Skip to content

Commit 52a36af

Browse files
ianbotsfianbotsf
authored
* fix: replace service model sets with lists (#673)
1 parent cb4388e commit 52a36af

File tree

6 files changed

+44
-56
lines changed

6 files changed

+44
-56
lines changed

‎.changes/9bbbf88e-193f-4efa-955a-10c17c1d7e83.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"id": "9bbbf88e-193f-4efa-955a-10c17c1d7e83",
3+
"type": "bugfix",
4+
"description": "**Breaking**: Generate `List<T>` members for all collection types. (Previously, `Set<T>` would be generated for lists decorated with `@uniqueItems`.)"
5+
}

‎gradle.properties‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ kotlin.native.ignoreDisabledTargets=true
99
kotlin.mpp.hierarchicalStructureSupport=false
1010

1111
# SDK
12-
sdkVersion=0.11.3-SNAPSHOT
12+
sdkVersion=0.12.0-SNAPSHOT
1313

1414
# kotlin
1515
kotlinVersion=1.7.0
@@ -46,4 +46,4 @@ kotlinLoggingVersion=2.1.21
4646
slf4jVersion=1.7.36
4747

4848
# crt
49-
crtKotlinVersion=0.6.2
49+
crtKotlinVersion=0.6.2

‎smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinSymbolProvider.kt‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import software.amazon.smithy.model.shapes.*
1313
import software.amazon.smithy.model.traits.BoxTrait
1414
import software.amazon.smithy.model.traits.SparseTrait
1515
import software.amazon.smithy.model.traits.StreamingTrait
16-
import software.amazon.smithy.model.traits.UniqueItemsTrait
1716
import java.util.logging.Logger
1817

1918
/**
@@ -147,19 +146,11 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli
147146
override fun listShape(shape: ListShape): Symbol {
148147
val reference = toSymbol(shape.member)
149148
val valueType = if (shape.hasTrait<SparseTrait>()) "${reference.name}?" else reference.name
150-
return if (shape.hasTrait<UniqueItemsTrait>()) {
151-
createSymbolBuilder(shape, "Set<$valueType>", boxed = true)
152-
.addReference(reference)
153-
.putProperty(SymbolProperty.MUTABLE_COLLECTION_FUNCTION, "mutableSetOf<$valueType>")
154-
.putProperty(SymbolProperty.IMMUTABLE_COLLECTION_FUNCTION, "setOf<$valueType>")
155-
.build()
156-
} else {
157-
createSymbolBuilder(shape, "List<$valueType>", boxed = true)
158-
.addReferences(reference)
159-
.putProperty(SymbolProperty.MUTABLE_COLLECTION_FUNCTION, "mutableListOf<$valueType>")
160-
.putProperty(SymbolProperty.IMMUTABLE_COLLECTION_FUNCTION, "listOf<$valueType>")
161-
.build()
162-
}
149+
return createSymbolBuilder(shape, "List<$valueType>", boxed = true)
150+
.addReferences(reference)
151+
.putProperty(SymbolProperty.MUTABLE_COLLECTION_FUNCTION, "mutableListOf<$valueType>")
152+
.putProperty(SymbolProperty.IMMUTABLE_COLLECTION_FUNCTION, "listOf<$valueType>")
153+
.build()
163154
}
164155

165156
override fun mapShape(shape: MapShape): Symbol {

‎smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/protocol/HttpBindingProtocolGenerator.kt‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -750,7 +750,7 @@ abstract class HttpBindingProtocolGenerator : ProtocolGenerator {
750750
memberName, headerName, parseInstant("it", tsFormat)
751751
)
752752
}
753-
is CollectionShape -> {
753+
is ListShape -> {
754754
// member > boolean, number, string, or timestamp
755755
// headers are List<String>, get the internal mapping function contents (if any) to convert
756756
// to the target symbol type
@@ -789,12 +789,6 @@ abstract class HttpBindingProtocolGenerator : ProtocolGenerator {
789789
else -> throw CodegenException("invalid member type for header collection: binding: $hdrBinding; member: $memberName")
790790
}
791791

792-
val toCollectionType = when {
793-
memberTarget.isListShape && memberTarget.hasTrait<UniqueItemsTrait>() -> "?.toSet()"
794-
memberTarget.isListShape -> ""
795-
else -> throw CodegenException("unknown collection shape: $memberTarget")
796-
}
797-
798792
val mapFn = if (conversion.isNotEmpty()) {
799793
"?.map { $conversion }"
800794
} else {
@@ -804,7 +798,7 @@ abstract class HttpBindingProtocolGenerator : ProtocolGenerator {
804798
// writer.addImport("${KotlinDependency.CLIENT_RT_HTTP.namespace}.util", splitFn)
805799
writer
806800
.addImport(splitFn, KotlinDependency.HTTP, subpackage = "util")
807-
.write("builder.#L = response.headers.getAll(#S)?.flatMap(::$splitFn)${mapFn}$toCollectionType", memberName, headerName)
801+
.write("builder.#L = response.headers.getAll(#S)?.flatMap(::$splitFn)$mapFn", memberName, headerName)
808802
}
809803
else -> throw CodegenException("unknown deserialization: header binding: $hdrBinding; member: `$memberName`")
810804
}
@@ -838,11 +832,9 @@ abstract class HttpBindingProtocolGenerator : ProtocolGenerator {
838832
.write("val map = mutableMapOf<String, #T>()", targetValueSymbol)
839833
.openBlock("for (hdrKey in $keyCollName) {")
840834
.call {
841-
val getFn = when {
842-
targetValueShape.isStringShape -> "[hdrKey]"
843-
targetValueShape.isListShape && targetValueShape.hasTrait<UniqueItemsTrait>() ->
844-
".getAll(hdrKey)?.toSet()"
845-
targetValueShape.isListShape -> ".getAll(hdrKey)"
835+
val getFn = when (targetValueShape) {
836+
is StringShape -> "[hdrKey]"
837+
is ListShape -> ".getAll(hdrKey)"
846838
else -> throw CodegenException("invalid httpPrefixHeaders usage on ${binding.member}")
847839
}
848840
// get()/getAll() returns String? or List<String>?, this shouldn't ever trigger the continue though...

‎smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/core/SymbolProviderTest.kt‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ class SymbolProviderTest {
173173
}
174174

175175
@Test
176-
fun `creates sets`() {
176+
fun `creates lists even when @uniqueItems is present`() {
177177
val model = """
178178
structure Record { }
179179
@@ -184,15 +184,15 @@ class SymbolProviderTest {
184184
""".prependNamespaceAndService(namespace = "foo.bar").toSmithyModel()
185185

186186
val provider: SymbolProvider = KotlinCodegenPlugin.createSymbolProvider(model, rootNamespace = "foo.bar")
187-
val setShape = model.expectShape<ListShape>("foo.bar#Records")
188-
val setSymbol = provider.toSymbol(setShape)
187+
val listShape = model.expectShape<ListShape>("foo.bar#Records")
188+
val listSymbol = provider.toSymbol(listShape)
189189

190-
assertEquals("Set<Record>", setSymbol.name)
191-
assertEquals(true, setSymbol.isBoxed)
192-
assertEquals("null", setSymbol.defaultValue())
190+
assertEquals("List<Record>", listSymbol.name)
191+
assertEquals(true, listSymbol.isBoxed)
192+
assertEquals("null", listSymbol.defaultValue())
193193

194194
// collections should contain a reference to the member type
195-
assertEquals("Record", setSymbol.references[0].symbol.name)
195+
assertEquals("Record", listSymbol.references[0].symbol.name)
196196
}
197197

198198
@Test

‎smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/rendering/serde/DeserializeStructGeneratorTest.kt‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,7 @@ class DeserializeStructGeneratorTest {
546546
}
547547

548548
@Test
549-
fun `it deserializes a structure containing a set of primitive values`() {
549+
fun `it deserializes a structure containing a list of unique primitive values`() {
550550
val model = (
551551
modelPrefix + """
552552
structure FooResponse {
@@ -566,7 +566,7 @@ class DeserializeStructGeneratorTest {
566566
when (findNextFieldIndex()) {
567567
PAYLOAD_DESCRIPTOR.index -> builder.payload =
568568
deserializer.deserializeList(PAYLOAD_DESCRIPTOR) {
569-
val col0 = mutableSetOf<Int>()
569+
val col0 = mutableListOf<Int>()
570570
while (hasNextElement()) {
571571
val el0 = if (nextHasValue()) { deserializeInt() } else { deserializeNull(); continue }
572572
col0.add(el0)
@@ -586,15 +586,15 @@ class DeserializeStructGeneratorTest {
586586
}
587587

588588
@Test
589-
fun `it deserializes a structure containing a set of a map primitive values`() {
589+
fun `it deserializes a structure containing a list of unique maps`() {
590590
val model = (
591591
modelPrefix + """
592592
structure FooResponse {
593-
payload: IntegerSet
593+
payload: MapSet
594594
}
595595
596596
@uniqueItems
597-
list IntegerSet {
597+
list MapSet {
598598
member: StringMap
599599
}
600600
@@ -611,7 +611,7 @@ class DeserializeStructGeneratorTest {
611611
when (findNextFieldIndex()) {
612612
PAYLOAD_DESCRIPTOR.index -> builder.payload =
613613
deserializer.deserializeList(PAYLOAD_DESCRIPTOR) {
614-
val col0 = mutableSetOf<Map<String, String>>()
614+
val col0 = mutableListOf<Map<String, String>>()
615615
while (hasNextElement()) {
616616
val el0 = deserializer.deserializeMap(PAYLOAD_C0_DESCRIPTOR) {
617617
val map1 = mutableMapOf<String, String>()
@@ -639,15 +639,15 @@ class DeserializeStructGeneratorTest {
639639
}
640640

641641
@Test
642-
fun `it deserializes a structure containing a set of a list of primitive values`() {
642+
fun `it deserializes a structure containing a list of unique lists of primitive values`() {
643643
val model = (
644644
modelPrefix + """
645645
structure FooResponse {
646-
payload: IntegerSet
646+
payload: ListSet
647647
}
648648
649649
@uniqueItems
650-
list IntegerSet {
650+
list ListSet {
651651
member: StringList
652652
}
653653
@@ -663,7 +663,7 @@ class DeserializeStructGeneratorTest {
663663
when (findNextFieldIndex()) {
664664
PAYLOAD_DESCRIPTOR.index -> builder.payload =
665665
deserializer.deserializeList(PAYLOAD_DESCRIPTOR) {
666-
val col0 = mutableSetOf<List<String>>()
666+
val col0 = mutableListOf<List<String>>()
667667
while (hasNextElement()) {
668668
val el0 = deserializer.deserializeList(PAYLOAD_C0_DESCRIPTOR) {
669669
val col1 = mutableListOf<String>()
@@ -690,7 +690,7 @@ class DeserializeStructGeneratorTest {
690690
}
691691

692692
@Test
693-
fun `it deserializes a structure containing a nested structure of a set of primitive values`() {
693+
fun `it deserializes a structure containing a nested structure of a list of unique primitive values`() {
694694
val model = (
695695
modelPrefix + """
696696
structure FooResponse {
@@ -726,7 +726,7 @@ class DeserializeStructGeneratorTest {
726726
}
727727

728728
@Test
729-
fun `it deserializes a structure containing a list of a set of primitive values`() {
729+
fun `it deserializes a structure containing a list of a list of unique primitive values`() {
730730
val model = (
731731
modelPrefix + """
732732
structure FooResponse {
@@ -750,10 +750,10 @@ class DeserializeStructGeneratorTest {
750750
when (findNextFieldIndex()) {
751751
PAYLOAD_DESCRIPTOR.index -> builder.payload =
752752
deserializer.deserializeList(PAYLOAD_DESCRIPTOR) {
753-
val col0 = mutableListOf<Set<Int>>()
753+
val col0 = mutableListOf<List<Int>>()
754754
while (hasNextElement()) {
755755
val el0 = deserializer.deserializeList(PAYLOAD_C0_DESCRIPTOR) {
756-
val col1 = mutableSetOf<Int>()
756+
val col1 = mutableListOf<Int>()
757757
while (hasNextElement()) {
758758
val el1 = if (nextHasValue()) { deserializeInt() } else { deserializeNull(); continue }
759759
col1.add(el1)
@@ -777,7 +777,7 @@ class DeserializeStructGeneratorTest {
777777
}
778778

779779
@Test
780-
fun `it deserializes a structure containing a map of a set of primitive values`() {
780+
fun `it deserializes a structure containing a map of a list of unique primitive values`() {
781781
val model = (
782782
modelPrefix + """
783783
structure FooResponse {
@@ -802,13 +802,13 @@ class DeserializeStructGeneratorTest {
802802
when (findNextFieldIndex()) {
803803
PAYLOAD_DESCRIPTOR.index -> builder.payload =
804804
deserializer.deserializeMap(PAYLOAD_DESCRIPTOR) {
805-
val map0 = mutableMapOf<String, Set<Int>>()
805+
val map0 = mutableMapOf<String, List<Int>>()
806806
while (hasNextEntry()) {
807807
val k0 = key()
808808
val v0 =
809809
if (nextHasValue()) {
810810
deserializer.deserializeList(PAYLOAD_C0_DESCRIPTOR) {
811-
val col1 = mutableSetOf<Int>()
811+
val col1 = mutableListOf<Int>()
812812
while (hasNextElement()) {
813813
val el1 = if (nextHasValue()) { deserializeInt() } else { deserializeNull(); continue }
814814
col1.add(el1)
@@ -834,7 +834,7 @@ class DeserializeStructGeneratorTest {
834834
}
835835

836836
@Test
837-
fun `it deserializes a structure containing a sparse map of a set of primitive values`() {
837+
fun `it deserializes a structure containing a sparse map of a list of unique primitive values`() {
838838
val model = (
839839
modelPrefix + """
840840
structure FooResponse {
@@ -860,13 +860,13 @@ class DeserializeStructGeneratorTest {
860860
when (findNextFieldIndex()) {
861861
PAYLOAD_DESCRIPTOR.index -> builder.payload =
862862
deserializer.deserializeMap(PAYLOAD_DESCRIPTOR) {
863-
val map0 = mutableMapOf<String, Set<Int>?>()
863+
val map0 = mutableMapOf<String, List<Int>?>()
864864
while (hasNextEntry()) {
865865
val k0 = key()
866866
val v0 =
867867
if (nextHasValue()) {
868868
deserializer.deserializeList(PAYLOAD_C0_DESCRIPTOR) {
869-
val col1 = mutableSetOf<Int>()
869+
val col1 = mutableListOf<Int>()
870870
while (hasNextElement()) {
871871
val el1 = if (nextHasValue()) { deserializeInt() } else { deserializeNull(); continue }
872872
col1.add(el1)

0 commit comments

Comments
 (0)