[Runtime] OpenAPIObjectContainer hashing violates Hashable requirements

[andrii-vysotskyi-maudau]

Description

OpenAPIObjectContainer’s hash(into:) depends on dictionary iteration order. This can cause equal values to produce different hashes.

Reproduction

func testHashing_objectOrderIndependence_success() throws {
   let container1 = try OpenAPIObjectContainer(
      unvalidatedValue: ["foo": 0, "bar": 1]
   )
   let container2 = try OpenAPIObjectContainer(
      unvalidatedValue: ["bar": 1, "foo": 0]
   )
   XCTAssertEqual(container1, container2)
   XCTAssertEqual(container1.hashValue, container2.hashValue)
}

Package version(s)

├── swift-openapi-generator<https://github.com/apple/swift-openapi-generator@1.13.0>(traits: default)
│   ├── swift-algorithms<https://github.com/apple/swift-algorithms@1.2.1>(traits: default)
│   │   └── swift-numerics<https://github.com/apple/swift-numerics.git@1.0.3>(traits: default)
│   ├── openapikit<https://github.com/mattpolzin/OpenAPIKit@6.2.0>
│   ├── yams<https://github.com/jpsim/Yams@6.0.1>(traits: default)
│   └── swift-argument-parser<https://github.com/apple/swift-argument-parser@1.5.1>(traits: default)
├── swift-openapi-runtime<https://github.com/apple/swift-openapi-runtime@1.12.0>(traits: FullFoundation)
│   └── swift-http-types<https://github.com/apple/swift-http-types@1.4.0>(traits: default)
└── swift-openapi-urlsession<https://github.com/apple/swift-openapi-urlsession@1.3.1>(traits: default)
    ├── swift-openapi-runtime<https://github.com/apple/swift-openapi-runtime@1.12.0>(traits: FullFoundation)
    │   └── swift-http-types<https://github.com/apple/swift-http-types@1.4.0>(traits: default)
    ├── swift-http-types<https://github.com/apple/swift-http-types@1.4.0>(traits: default)
    └── swift-collections<https://github.com/apple/swift-collections@1.2.0>(traits: default)

Expected behavior

OpenAPIObjectContainer’s hash should return same value for equal containers.

Environment

swift-driver version: 1.148.6 Apple Swift version 6.3.3 (swiftlang-6.3.3.1.3 clang-2100.1.1.101)
Target: arm64-apple-macosx26.0

Additional information

No response

[simonjbeaumont]

Thanks for filing this. I agree this looks like an issue. The simplest fix would be to iterate the keys in order when combining the elements into the hash. This probably affects both OpenAPIValueContainer and OpenAPIObjectContainer.

[andrii-vysotskyi-maudau]

Ahh you are right, it affects both.

I'm not quite sure how we could enforce a specific order while iterating. I was thinking to reuse the same approach Swift's Dictionary is using.

public func hash(into hasher: inout Hasher) {
    var commutativeHash = 0
    for (k, v) in self {
      // Note that we use a copy of our own hasher here. This makes hash values
      // dependent on its state, eliminating static collision patterns.
      var elementHasher = hasher
      elementHasher.combine(k)
      elementHasher.combine(v)
      commutativeHash ^= elementHasher._finalize()
    }
    hasher.combine(commutativeHash)
  }

[simonjbeaumont]

We could. But it might just be simpler to iterate over .keys.sorted(). Are we expecting this to be very perf sensitive?

[andrii-vysotskyi-maudau]

Are we expecting this to be very perf sensitive?

Probably depends on actual use case, so quite hard to predict. Anyway for some reason I missed that key is String, hence comparable, so sure we could use sort as well.

[EhsanAzish80]

I’d like to work on this. I’ll update both OpenAPIObjectContainer and OpenAPIValueContainer to hash object keys in a stable order, add regression coverage proving equal containers produce equal hashes regardless of insertion order, and run the package test suite before opening a PR.

[EhsanAzish80]

I found the existing implementation in apple/swift-openapi-runtime#195 after tracing these types to the runtime repository. I’m standing down to avoid duplicating that work. Sorry—I missed it because it wasn’t linked from this issue.

[andrii-vysotskyi-maudau]

Hi @simonjbeaumont , updated apple/swift-openapi-runtime#195 to rely on sorted keys as discussed. Would you mind taking a look?

[simonjbeaumont]

@andrii-vysotskyi-maudau I reviewed the PR. We can continue the discussion over there. Thanks for the contribution 🙏