Unverified Commit 5014e625 authored by Russell Cohen's avatar Russell Cohen Committed by GitHub
Browse files

Switch to BTreeSet to provide deterministic serialization (#42)

HashSet does not offer a deterministic iteration order which made serialization non-deterministic. The change to BTreeSet is otherwise transparent but provides deterministic sort order for members.
parent 16e513bd
Loading
Loading
Loading
Loading
+5 −1
Original line number Diff line number Diff line
@@ -48,7 +48,7 @@ sealed class RustType {

    data class HashSet(val member: RustType) : RustType() {
        // TODO: assert that underneath, the member is a String
        override val name: kotlin.String = "HashSet"
        override val name: kotlin.String = SetType
    }

    data class Reference(val lifetime: kotlin.String?, override val value: RustType) : RustType(), Container {
@@ -64,6 +64,10 @@ sealed class RustType {
    }

    data class Opaque(override val name: kotlin.String) : RustType()

    companion object {
        val SetType = "BTreeSet"
    }
}

fun RustType.render(): String = when (this) {
+1 −1
Original line number Diff line number Diff line
@@ -47,7 +47,7 @@ data class RuntimeType(val name: String, val dependency: RustDependency?, val na
        fun StdFmt(member: String) = RuntimeType("fmt::$member", dependency = null, namespace = "std")
        fun Std(member: String) = RuntimeType(member, dependency = null, namespace = "std")
        val StdError = RuntimeType("Error", dependency = null, namespace = "std::error")
        val HashSet = RuntimeType("HashSet", dependency = null, namespace = "std::collections")
        val HashSet = RuntimeType(RustType.SetType, dependency = null, namespace = "std::collections")
        val HashMap = RuntimeType("HashMap", dependency = null, namespace = "std::collections")

        fun Instant(runtimeConfig: RuntimeConfig) =
+4 −1
Original line number Diff line number Diff line
@@ -64,7 +64,10 @@ open class SymbolMetadataProvider(private val base: SymbolProvider) : WrappingSy
                    // enums must be hashable because string sets are hashable
                    RuntimeType.Std("hash::Hash") +
                    // enums can be eq because they can only contain strings
                    RuntimeType.Std("cmp::Eq")
                    RuntimeType.Std("cmp::Eq") +
                    // enums can be Ord because they can only contain strings
                    RuntimeType.Std("cmp::PartialOrd") +
                    RuntimeType.Std("cmp::Ord")
            ),
            public = true
        )
+1 −4
Original line number Diff line number Diff line
@@ -22,10 +22,7 @@ class HttpProtocolTestGenerator(private val protocolConfig: ProtocolConfig, priv
        "RestJsonListsSerializeNull",
        "AwsJson11MapsSerializeNullValues",
        "AwsJson11ListsSerializeNull",
        "RestJsonSerializesNullMapValues",
        // This test is fully disabled because it's flaky (it depends on the hash set iteration order)
        // https://github.com/awslabs/smithy-rs/issues/37
        "RestJsonInputAndOutputWithStringHeaders"
        "RestJsonSerializesNullMapValues"
    )

    // These tests fail due to shortcomings in our implementation.
+3 −2
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import software.amazon.smithy.model.shapes.StringShape
import software.amazon.smithy.model.shapes.StructureShape
import software.amazon.smithy.model.traits.ErrorTrait
import software.amazon.smithy.model.traits.SparseTrait
import software.amazon.smithy.rust.codegen.lang.RustType
import software.amazon.smithy.rust.codegen.lang.render
import software.amazon.smithy.rust.codegen.smithy.Errors
import software.amazon.smithy.rust.codegen.smithy.Operations
@@ -157,8 +158,8 @@ class SymbolBuilderTest {

        val provider: SymbolProvider = testSymbolProvider(model)
        val setSymbol = provider.toSymbol(set)
        setSymbol.rustType().render() shouldBe "HashSet<String>"
        setSymbol.referenceClosure().map { it.name } shouldBe listOf("HashSet", "String")
        setSymbol.rustType().render() shouldBe "${RustType.SetType}<String>"
        setSymbol.referenceClosure().map { it.name } shouldBe listOf(RustType.SetType, "String")
    }

    @Test
Loading