Skip to content

Commit c827d5c

Browse files
fix(codegen): fix set deserialization in SSDKs (#3322)
Using JavaScript's Set to detect duplicate items does not work for non-primitive types. There's a function in server-common to detect duplicates of any type, and detecting them in server deserialization is much more important than client deserialization, so the check has been moved to server-only.
1 parent 7ddf66d commit c827d5c

File tree

1 file changed

+14
-18
lines changed

1 file changed

+14
-18
lines changed

codegen/smithy-aws-typescript-codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/JsonShapeDeserVisitor.java

+14-18
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import software.amazon.smithy.model.traits.SparseTrait;
3434
import software.amazon.smithy.model.traits.TimestampFormatTrait.Format;
3535
import software.amazon.smithy.typescript.codegen.CodegenUtils;
36+
import software.amazon.smithy.typescript.codegen.TypeScriptDependency;
3637
import software.amazon.smithy.typescript.codegen.TypeScriptSettings.ArtifactType;
3738
import software.amazon.smithy.typescript.codegen.TypeScriptWriter;
3839
import software.amazon.smithy.typescript.codegen.integration.DocumentMemberDeserVisitor;
@@ -83,11 +84,7 @@ protected void deserializeCollection(GenerationContext context, CollectionShape
8384
potentialFilter = ".filter((e: any) => e != null)";
8485
}
8586

86-
if (shape.isSetShape()) {
87-
writer.write("const uniqueValues = new Set<any>();");
88-
}
89-
90-
writer.openBlock("return (output || [])$L.map((entry: any) => {", "});", potentialFilter, () -> {
87+
writer.openBlock("const retVal = (output || [])$L.map((entry: any) => {", "});", potentialFilter, () -> {
9188
// Short circuit null values from serialization.
9289
writer.openBlock("if (entry === null) {", "}", () -> {
9390
// In the SSDK we want to be very strict about not accepting nulls in non-sparse lists.
@@ -99,20 +96,19 @@ protected void deserializeCollection(GenerationContext context, CollectionShape
9996
}
10097
});
10198

102-
if (shape.isSetShape()) {
103-
writer.write("const parsedEntry = $L$L;",
104-
target.accept(getMemberVisitor(shape.getMember(), "entry")),
105-
usesExpect(target) ? " as any" : "");
106-
writer.write("if (uniqueValues.has(parsedEntry)) { throw new "
107-
+ "TypeError('All elements of the set $S must be unique.'); } else { "
108-
+ "uniqueValues.add(parsedEntry)\nreturn parsedEntry; }",
109-
shape.getId());
110-
} else {
111-
// Dispatch to the output value provider for any additional handling.
112-
writer.write("return $L$L;", target.accept(getMemberVisitor(shape.getMember(), "entry")),
113-
usesExpect(target) ? " as any" : "");
114-
}
99+
// Dispatch to the output value provider for any additional handling.
100+
writer.write("return $L$L;", target.accept(getMemberVisitor(shape.getMember(), "entry")),
101+
usesExpect(target) ? " as any" : "");
115102
});
103+
if (shape.isSetShape() && artifactType.equals(ArtifactType.SSDK)) {
104+
writer.addDependency(TypeScriptDependency.SERVER_COMMON);
105+
writer.addImport("findDuplicates", "__findDuplicates", "@aws-smithy/server-common");
106+
writer.openBlock("if (__findDuplicates(retVal).length > 0) {", "}", () -> {
107+
writer.write("throw new TypeError('All elements of the set $S must be unique.');",
108+
shape.getId());
109+
});
110+
}
111+
writer.write("return retVal;");
116112
}
117113

118114
@Override

0 commit comments

Comments
 (0)