Skip to content

Commit

Permalink
fix(codegen): fix set deserialization in SSDKs (#3322)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
adamthom-amzn committed Feb 19, 2022
1 parent 7ddf66d commit c827d5c
Showing 1 changed file with 14 additions and 18 deletions.
Expand Up @@ -33,6 +33,7 @@
import software.amazon.smithy.model.traits.SparseTrait;
import software.amazon.smithy.model.traits.TimestampFormatTrait.Format;
import software.amazon.smithy.typescript.codegen.CodegenUtils;
import software.amazon.smithy.typescript.codegen.TypeScriptDependency;
import software.amazon.smithy.typescript.codegen.TypeScriptSettings.ArtifactType;
import software.amazon.smithy.typescript.codegen.TypeScriptWriter;
import software.amazon.smithy.typescript.codegen.integration.DocumentMemberDeserVisitor;
Expand Down Expand Up @@ -83,11 +84,7 @@ protected void deserializeCollection(GenerationContext context, CollectionShape
potentialFilter = ".filter((e: any) => e != null)";
}

if (shape.isSetShape()) {
writer.write("const uniqueValues = new Set<any>();");
}

writer.openBlock("return (output || [])$L.map((entry: any) => {", "});", potentialFilter, () -> {
writer.openBlock("const retVal = (output || [])$L.map((entry: any) => {", "});", potentialFilter, () -> {
// Short circuit null values from serialization.
writer.openBlock("if (entry === null) {", "}", () -> {
// In the SSDK we want to be very strict about not accepting nulls in non-sparse lists.
Expand All @@ -99,20 +96,19 @@ protected void deserializeCollection(GenerationContext context, CollectionShape
}
});

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

@Override
Expand Down

0 comments on commit c827d5c

Please sign in to comment.