Skip to content

Commit aed1ca6

Browse files
joewyzjoewyz
authored
Fix trait codegen for list with uniqueitems trait (#2706)
* Convert stream style to for loop style for map and list
1 parent 9c21bef commit aed1ca6

15 files changed

Lines changed: 464 additions & 157 deletions

File tree

‎smithy-trait-codegen/src/it/java/software/amazon/smithy/traitcodegen/test/CreatesTraitTest.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,14 @@
1616
import com.example.traits.enums.SuitTrait;
1717
import com.example.traits.lists.DocumentListTrait;
1818
import com.example.traits.lists.ListMember;
19+
import com.example.traits.lists.NestedListTrait;
20+
import com.example.traits.lists.NestedUniqueItemsListTrait;
1921
import com.example.traits.lists.NumberListTrait;
2022
import com.example.traits.lists.StringListTrait;
2123
import com.example.traits.lists.StructureListTrait;
2224
import com.example.traits.maps.MapValue;
25+
import com.example.traits.maps.NestedMapTrait;
26+
import com.example.traits.maps.NestedStringUniqueItemMapTrait;
2327
import com.example.traits.maps.StringDocumentMapTrait;
2428
import com.example.traits.maps.StringStringMapTrait;
2529
import com.example.traits.maps.StringToStructMapTrait;
@@ -37,6 +41,8 @@
3741
import com.example.traits.structures.BasicAnnotationTrait;
3842
import com.example.traits.structures.NestedA;
3943
import com.example.traits.structures.NestedB;
44+
import com.example.traits.structures.StructWithListOfMapTrait;
45+
import com.example.traits.structures.StructWithUniqueItemsListTrait;
4046
import com.example.traits.structures.StructureTrait;
4147
import com.example.traits.timestamps.DateTimeTimestampTrait;
4248
import com.example.traits.timestamps.EpochSecondsTimestampTrait;
@@ -59,6 +65,7 @@
5965
import software.amazon.smithy.model.traits.TraitFactory;
6066
import software.amazon.smithy.utils.ListUtils;
6167
import software.amazon.smithy.utils.MapUtils;
68+
import software.amazon.smithy.utils.SetUtils;
6269

6370
public class CreatesTraitTest {
6471
private static final ShapeId DUMMY_ID = ShapeId.from("ns.foo#foo");
@@ -103,6 +110,14 @@ static Stream<Arguments> createTraitTests() {
103110
ArrayNode.fromNodes(
104111
ObjectNode.builder().withMember("a", "b").build(),
105112
ObjectNode.builder().withMember("c", "d").withMember("e", "f").build())),
113+
Arguments.of(NestedListTrait.ID,
114+
ArrayNode.fromNodes(
115+
ArrayNode.fromNodes(
116+
ArrayNode.fromNodes(Node.from("1"), Node.from("2"), Node.from("3"))))),
117+
Arguments.of(NestedUniqueItemsListTrait.ID,
118+
ArrayNode.fromNodes(
119+
ArrayNode.fromNodes(
120+
ArrayNode.fromNodes(Node.from("1"), Node.from("2"), Node.from("3"))))),
106121
// Maps
107122
Arguments.of(StringStringMapTrait.ID,
108123
StringStringMapTrait.builder()
@@ -124,6 +139,19 @@ static Stream<Arguments> createTraitTests() {
124139
.putValues("number", Node.from(1))
125140
.build()
126141
.toNode()),
142+
Arguments.of(NestedMapTrait.ID,
143+
NestedMapTrait.builder()
144+
.putValues("1", MapUtils.of("1", MapUtils.of("2", "3")))
145+
.build()
146+
.toNode()),
147+
Arguments.of(NestedStringUniqueItemMapTrait.ID,
148+
NestedStringUniqueItemMapTrait.builder()
149+
.putValues("1",
150+
SetUtils.of(
151+
MapUtils.of("2", "3"),
152+
MapUtils.of("4", "5")))
153+
.build()
154+
.toNode()),
127155
// Mixins
128156
Arguments.of(StructureListWithMixinMemberTrait.ID,
129157
ArrayNode.fromNodes(ObjectNode.builder().withMember("a", "a").withMember("d", "d").build())),
@@ -161,6 +189,17 @@ static Stream<Arguments> createTraitTests() {
161189
.fieldE(MapUtils.of("a", "one", "b", "two"))
162190
.build()
163191
.toNode()),
192+
Arguments.of(StructWithListOfMapTrait.ID,
193+
StructWithListOfMapTrait.builder()
194+
.addItems(MapUtils.of("1", "2"))
195+
.addItems(MapUtils.of("3", "4"))
196+
.build()
197+
.toNode()),
198+
Arguments.of(StructWithUniqueItemsListTrait.ID,
199+
StructWithUniqueItemsListTrait.builder()
200+
.addItems(SetUtils.of("a", "b", "c"))
201+
.build()
202+
.toNode()),
164203
// Timestamps
165204
Arguments.of(TimestampTrait.ID, Node.from("1985-04-12T23:20:50.52Z")),
166205
Arguments.of(DateTimeTimestampTrait.ID, Node.from("1985-04-12T23:20:50.52Z")),

‎smithy-trait-codegen/src/it/java/software/amazon/smithy/traitcodegen/test/LoadsFromModelTest.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.example.traits.lists.DocumentListTrait;
2626
import com.example.traits.lists.ListMember;
2727
import com.example.traits.lists.NestedListTrait;
28+
import com.example.traits.lists.NestedUniqueItemsListTrait;
2829
import com.example.traits.lists.NumberListTrait;
2930
import com.example.traits.lists.StructureListTrait;
3031
import com.example.traits.maps.MapValue;
@@ -48,6 +49,7 @@
4849
import com.example.traits.structures.NestedA;
4950
import com.example.traits.structures.NestedB;
5051
import com.example.traits.structures.StructWithListOfMapTrait;
52+
import com.example.traits.structures.StructWithUniqueItemsListTrait;
5153
import com.example.traits.structures.StructureTrait;
5254
import com.example.traits.timestamps.DateTimeTimestampTrait;
5355
import com.example.traits.timestamps.EpochSecondsTimestampTrait;
@@ -162,6 +164,11 @@ static Stream<Arguments> loadsModelTests() {
162164
NestedListTrait.class,
163165
MapUtils.of("getValues",
164166
ListUtils.of(ListUtils.of(ListUtils.of("a"))))),
167+
Arguments.of("lists/nested-uniqueitems-list-trait.smithy",
168+
NestedUniqueItemsListTrait.class,
169+
MapUtils.of("getValues",
170+
SetUtils.of(SetUtils.of(SetUtils.of("a", "ab", "c", "bc"),
171+
SetUtils.of("b", "ba", "ab", "aa"))))),
165172
// Maps
166173
Arguments.of("maps/string-string-map-trait.smithy",
167174
StringStringMapTrait.class,
@@ -310,6 +317,13 @@ static Stream<Arguments> loadsModelTests() {
310317
Optional.of(
311318
ListUtils.of(
312319
MapUtils.of("b", "c"))))),
320+
Arguments.of("structures/struct-with-uniqueitems-list-trait.smithy",
321+
StructWithUniqueItemsListTrait.class,
322+
MapUtils.of(
323+
"getName",
324+
Optional.of("a"),
325+
"getItems",
326+
Optional.of(SetUtils.of(SetUtils.of("b", "c"), SetUtils.of("d", "e"))))),
313327
// Timestamps
314328
Arguments.of("timestamps/struct-with-nested-timestamps.smithy",
315329
StructWithNestedTimestampsTrait.class,

‎smithy-trait-codegen/src/it/resources/software/amazon/smithy/traitcodegen/test/lists/nested-uniqueitems-list-trait.smithy‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
$version: "2.0"
2+
3+
namespace test.smithy.traitcodegen
4+
5+
use test.smithy.traitcodegen.lists#NestedUniqueItemsListTrait
6+
7+
@NestedUniqueItemsListTrait([
8+
[
9+
["a", "ab", "c", "bc"]
10+
["b", "ba", "ab", "aa"]
11+
]
12+
])
13+
structure myStruct {}

‎smithy-trait-codegen/src/it/resources/software/amazon/smithy/traitcodegen/test/maps/nested-string-uniqueitems-map-trait.smithy‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
$version: "2.0"
2+
3+
namespace test.smithy.traitcodegen
4+
5+
use test.smithy.traitcodegen.maps#NestedStringUniqueItemMapTrait
6+
7+
@NestedStringUniqueItemMapTrait({
8+
a: [
9+
{
10+
b: "c"
11+
}
12+
]
13+
})
14+
structure myStruct {}

‎smithy-trait-codegen/src/it/resources/software/amazon/smithy/traitcodegen/test/structures/struct-with-uniqueitems-list-trait.smithy‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
$version: "2.0"
2+
3+
namespace test.smithy.traitcodegen
4+
5+
use test.smithy.traitcodegen.structures#StructWithUniqueItemsListTrait
6+
7+
@StructWithUniqueItemsListTrait(
8+
name: "a"
9+
items: [
10+
["b", "c"]
11+
["d", "e"]
12+
]
13+
)
14+
structure myStruct {}

‎smithy-trait-codegen/src/main/java/software/amazon/smithy/traitcodegen/generators/FromNodeGenerator.java‎

Lines changed: 41 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import java.time.Instant;
88
import java.util.Iterator;
9+
import java.util.LinkedHashSet;
910
import software.amazon.smithy.codegen.core.Symbol;
1011
import software.amazon.smithy.codegen.core.SymbolProvider;
1112
import software.amazon.smithy.model.Model;
@@ -35,10 +36,10 @@
3536
import software.amazon.smithy.model.shapes.TimestampShape;
3637
import software.amazon.smithy.model.shapes.UnionShape;
3738
import software.amazon.smithy.model.traits.TimestampFormatTrait;
39+
import software.amazon.smithy.model.traits.UniqueItemsTrait;
3840
import software.amazon.smithy.traitcodegen.SymbolProperties;
3941
import software.amazon.smithy.traitcodegen.TraitCodegenUtils;
4042
import software.amazon.smithy.traitcodegen.writer.TraitCodegenWriter;
41-
import software.amazon.smithy.utils.StringUtils;
4243

4344
/**
4445
* Generates the static {@code fromNode} method to deserialize a smithy node into an instance of a Java class.
@@ -82,7 +83,8 @@ public Void listShape(ListShape shape) {
8283
Node.class,
8384
() -> {
8485
writer.writeWithNoFormatting("Builder builder = builder();");
85-
shape.accept(new FromNodeMapperVisitor(writer, model, "node"));
86+
shape.accept(new FromNodeMapperVisitor(writer, model, "node", symbolProvider));
87+
writer.write("builder.values(value0);");
8688
writer.writeWithNoFormatting("return builder.build();");
8789
});
8890
writer.newLine();
@@ -99,7 +101,7 @@ public Void mapShape(MapShape shape) {
99101
Node.class,
100102
() -> {
101103
writer.writeWithNoFormatting("Builder builder = builder();");
102-
shape.accept(new FromNodeMapperVisitor(writer, model, "node"));
104+
shape.accept(new FromNodeMapperVisitor(writer, model, "node", symbolProvider));
103105
writer.writeWithNoFormatting("return builder.build();");
104106
});
105107
writer.newLine();
@@ -169,7 +171,7 @@ public Void structureShape(StructureShape shape) {
169171
if (memberIterator.hasNext()) {
170172
writer.writeInlineWithNoFormatting("\n");
171173
} else {
172-
writer.writeWithNoFormatting(";\n");
174+
writer.writeInlineWithNoFormatting(";\n");
173175
}
174176
}
175177
writer.dedent();
@@ -200,7 +202,7 @@ private void writeTimestampDeser() {
200202
if (shape.hasTrait(TimestampFormatTrait.ID)) {
201203
writer.write("return new $T($C, node.getSourceLocation());",
202204
symbol,
203-
(Runnable) () -> shape.accept(new FromNodeMapperVisitor(writer, model, "node")));
205+
(Runnable) () -> shape.accept(new FromNodeMapperVisitor(writer, model, "node", symbolProvider)));
204206
} else {
205207
writer.openBlock("if (node.isNumberNode()) {", "}", () -> {
206208
writer.write("return new $T($T.ofEpochSecond(node.expectNumberNode().getValue().longValue()),",
@@ -251,10 +253,30 @@ public Void booleanShape(BooleanShape shape) {
251253

252254
@Override
253255
public Void listShape(ListShape shape) {
254-
writer.writeInline(memberPrefix + "ArrayMember($1S, n -> $3C, builder::$2L)",
255-
fieldName,
256-
memberName,
257-
(Runnable) () -> shape.getMember().accept(new FromNodeMapperVisitor(writer, model, "n", 1)));
256+
writer.writeInline(memberPrefix + "ArrayMember($1S, n -> ", fieldName);
257+
258+
Shape memberTarget = model.expectShape(shape.getMember().getTarget());
259+
boolean isMemberCollection = memberTarget.isListShape() || memberTarget.isMapShape();
260+
if (isMemberCollection) {
261+
writer.indent();
262+
writer.writeInline("{\n$C\n",
263+
(Runnable) () -> shape.getMember()
264+
.accept(new FromNodeMapperVisitor(writer, model, "n", 1, symbolProvider)));
265+
writer.writeInlineWithNoFormatting("return value1;\n");
266+
writer.dedent();
267+
writer.writeInlineWithNoFormatting("}, ");
268+
} else {
269+
writer.writeInline("$C, ",
270+
(Runnable) () -> shape.getMember()
271+
.accept(new FromNodeMapperVisitor(writer, model, "n", 1, symbolProvider)));
272+
}
273+
274+
boolean isSet = shape.hasTrait(UniqueItemsTrait.ID);
275+
if (isSet) {
276+
writer.writeInline("l -> builder.$L(new $T<>(l)))", fieldName, LinkedHashSet.class);
277+
} else {
278+
writer.writeInline("builder::$L)", fieldName);
279+
}
258280
return null;
259281
}
260282

@@ -335,29 +357,13 @@ public Void bigDecimalShape(BigDecimalShape shape) {
335357

336358
@Override
337359
public Void mapShape(MapShape shape) {
338-
writer.disableNewlines();
339-
writer.openBlock(memberPrefix
340-
+ "ObjectMember($S, o -> o.getMembers().forEach((k, v) -> {\n",
341-
"}))",
342-
fieldName,
343-
() -> {
344-
writer.write("builder.put$L(\n", StringUtils.capitalize(memberName));
345-
writer.indent();
346-
writer.write("$C,\n",
347-
(Runnable) () -> shape.getKey()
348-
.accept(new FromNodeMapperVisitor(writer,
349-
model,
350-
"k")));
351-
writer.write("$C\n",
352-
(Runnable) () -> shape.getValue()
353-
.accept(new FromNodeMapperVisitor(writer,
354-
model,
355-
"v",
356-
1)));
357-
writer.dedent();
358-
writer.write(");\n");
359-
});
360-
writer.enableNewlines();
360+
writer.writeInline(memberPrefix + "ObjectMember($S, o -> {\n", fieldName);
361+
writer.indent();
362+
writer.writeInline("$C\n",
363+
(Runnable) () -> shape.accept(new FromNodeMapperVisitor(writer, model, "o", 1, symbolProvider)));
364+
writer.writeInline("builder.$L(value1);", memberName);
365+
writer.dedent();
366+
writer.writeInlineWithNoFormatting("\n})");
361367
return null;
362368
}
363369

@@ -378,7 +384,7 @@ public Void stringShape(StringShape shape) {
378384
writer.writeInline(memberPrefix + "Member($1S, n -> $3C, builder::$2L)",
379385
fieldName,
380386
memberName,
381-
(Runnable) () -> shape.accept(new FromNodeMapperVisitor(writer, model, "n")));
387+
(Runnable) () -> shape.accept(new FromNodeMapperVisitor(writer, model, "n", symbolProvider)));
382388
}
383389
return null;
384390
}
@@ -397,7 +403,7 @@ public Void structureShape(StructureShape shape) {
397403
writer.writeInline(memberPrefix + "Member($1S, n -> $3C, builder::$2L)",
398404
fieldName,
399405
memberName,
400-
(Runnable) () -> shape.accept(new FromNodeMapperVisitor(writer, model, "n")));
406+
(Runnable) () -> shape.accept(new FromNodeMapperVisitor(writer, model, "n", symbolProvider)));
401407
return null;
402408
}
403409

@@ -406,7 +412,7 @@ public Void timestampShape(TimestampShape shape) {
406412
writer.writeInline(memberPrefix + "Member($1S, n -> $3C, builder::$2L)",
407413
fieldName,
408414
memberName,
409-
(Runnable) () -> shape.accept(new FromNodeMapperVisitor(writer, model, "n")));
415+
(Runnable) () -> shape.accept(new FromNodeMapperVisitor(writer, model, "n", symbolProvider)));
410416
return null;
411417
}
412418

0 commit comments

Comments
 (0)