Skip to content

Commit 09f87a7

Browse files
committed
Fix validation generation and protocol tests
Streaming blob member inputs and design-time media types need special consideration during code generation, and the protocol tests were not accounting for the need for a validation customizer.
1 parent bdd942e commit 09f87a7

File tree

5 files changed

+69
-19
lines changed

5 files changed

+69
-19
lines changed

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/CodegenUtils.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,4 +134,20 @@ static void writeStreamingMemberType(
134134
+ " `%2$s` defined in {@link %1$s}", containerSymbol.getName(), memberName));
135135
writer.write("export interface $1L extends $1LType {}", typeName);
136136
}
137+
138+
/**
139+
* Ease the input streaming member restriction so that users don't need to construct a stream every time.
140+
* This is used for inline type declarations (such as parameters) that need to take more permissive inputs
141+
* Refer here for more rationales: https://github.com/aws/aws-sdk-js-v3/issues/843
142+
*/
143+
static void writeInlineStreamingMemberType(
144+
TypeScriptWriter writer,
145+
Symbol containerSymbol,
146+
MemberShape streamingMember
147+
) {
148+
String memberName = streamingMember.getMemberName();
149+
String optionalSuffix = streamingMember.isRequired() ? "" : "?";
150+
writer.writeInline("Omit<$1T, $2S> & { $2L$3L: $1T[$2S]|string|Uint8Array|Buffer }",
151+
containerSymbol, memberName, optionalSuffix);
152+
}
137153
}

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -293,15 +293,20 @@ private void generateServerRequestTest(OperationShape operation, HttpRequestTest
293293
// We use a partial here so that we don't have to define the entire service, but still get the advantages
294294
// the type checker, including excess property checking. Later on we'll use `as` to cast this to the
295295
// full service so that we can actually use it.
296-
writer.openBlock("const testService: Partial<$T> = {", "};", serviceSymbol, () -> {
297-
writer.write("$L: testFunction as $T,", operationSymbol.getName(), operationSymbol);
296+
writer.openBlock("const testService: Partial<$T<{}>> = {", "};", serviceSymbol, () -> {
297+
writer.write("$L: testFunction as $T<{}>,", operationSymbol.getName(), operationSymbol);
298298
});
299299

300300
String getHandlerName = "get" + handlerSymbol.getName();
301301
writer.addImport(getHandlerName, null, "./server/");
302+
writer.addImport("ValidationFailure", "__ValidationFailure", "@aws-smithy/server-common");
302303

303304
// Cast the service as any so TS will ignore the fact that the type being passed in is incomplete.
304-
writer.write("const handler = $L(testService as $T);", getHandlerName, serviceSymbol);
305+
writer.openBlock(
306+
"const handler = $L(testService as $T<{}>, (ctx: {}, failures: __ValidationFailure[]) => {",
307+
"});", getHandlerName, serviceSymbol,
308+
() -> writer.write("if (failures) { throw failures; } return undefined;")
309+
);
305310

306311
// Construct a new http request according to the test case definition.
307312
writer.openBlock("const request = new HttpRequest({", "});", () -> {
@@ -609,7 +614,9 @@ private void writeServerResponseTest(OperationShape operation, HttpResponseTestC
609614

610615
writer.addImport("serializeFrameworkException", null,
611616
"./protocols/" + ProtocolGenerator.getSanitizedName(protocolGenerator.getName()));
612-
writer.write("const handler = new $T(service, testMux, serFn, serializeFrameworkException);", handlerSymbol);
617+
writer.addImport("ValidationFailure", "__ValidationFailure", "@aws-smithy/server-common");
618+
writer.write("const handler = new $T(service, testMux, serFn, serializeFrameworkException, "
619+
+ "(ctx: {}, f: __ValidationFailure[]) => { if (f) { throw f; } return undefined;});", handlerSymbol);
613620
writer.write("let r = await handler.handle(request, {})").write("");
614621
writeHttpResponseAssertions(testCase);
615622
}

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/ServerCommandGenerator.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,11 @@ private void writeInputType(String typeName, Optional<StructureShape> inputShape
102102
} else {
103103
// If the input is non-existent, then use an empty object.
104104
writer.write("export interface $L {}", typeName);
105+
writer.openBlock("export namespace $L {", "}", typeName, () -> {
106+
writer.addImport("ValidationFailure", "__ValidationFailure", "@aws-smithy/server-common");
107+
writer.writeDocs("@internal");
108+
writer.write("export const validate: () => __ValidationFailure[] = () => [];");
109+
});
105110
}
106111
}
107112

@@ -110,8 +115,10 @@ private void renderNamespace(String typeName, StructureShape input) {
110115
writer.openBlock("export namespace $L {", "}", typeName, () -> {
111116
writer.addImport("ValidationFailure", "__ValidationFailure", "@aws-smithy/server-common");
112117
writer.writeDocs("@internal");
113-
writer.write("export const validate: (obj: $L) => __ValidationFailure[] = $T.validate;",
114-
symbol.getName(), symbol);
118+
// Streaming makes the type of the object being validated weird on occasion.
119+
// Using `Parameters` here means we don't have to try to derive the weird type twice
120+
writer.write("export const validate: (obj: Parameters<typeof $1T.validate>[0]) => "
121+
+ "__ValidationFailure[] = $1T.validate;", symbol);
115122
});
116123
}
117124

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructureGenerator.java

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,17 @@
1515

1616
package software.amazon.smithy.typescript.codegen;
1717

18+
import static software.amazon.smithy.typescript.codegen.CodegenUtils.getBlobStreamingMembers;
19+
import static software.amazon.smithy.typescript.codegen.CodegenUtils.writeInlineStreamingMemberType;
20+
21+
import java.util.List;
1822
import java.util.stream.Collectors;
1923
import java.util.stream.Stream;
2024
import software.amazon.smithy.codegen.core.Symbol;
2125
import software.amazon.smithy.codegen.core.SymbolProvider;
2226
import software.amazon.smithy.codegen.core.SymbolReference;
2327
import software.amazon.smithy.model.Model;
28+
import software.amazon.smithy.model.shapes.MemberShape;
2429
import software.amazon.smithy.model.shapes.StructureShape;
2530
import software.amazon.smithy.model.traits.ErrorTrait;
2631
import software.amazon.smithy.typescript.codegen.integration.HttpProtocolGeneratorUtils;
@@ -215,17 +220,17 @@ private void renderStructureNamespace(StructuredMemberWriter structuredMemberWri
215220

216221
writer.addImport("ValidationFailure", "__ValidationFailure", "@aws-smithy/server-common");
217222
writer.writeDocs("@internal");
218-
writer.openBlock("export const validate = ($L: $L, path: string = \"\"): __ValidationFailure[] => {", "}",
219-
objectParam, symbol.getName(),
220-
() -> {
221-
// TODO: move this somewhere so it only gets run once.
222-
// Putting it at the top of the namespace can result in runtime errors when
223-
// you have mutually recursive structures because the validator of one will
224-
// be defined before the validator of the other exists at all.
225-
structuredMemberWriter.writeMemberValidatorFactory(writer, "memberValidators");
226-
structuredMemberWriter.writeValidateMethodContents(writer, objectParam);
227-
}
228-
);
223+
List<MemberShape> blobStreamingMembers = getBlobStreamingMembers(model, shape);
224+
writer.writeInline("export const validate = ($L: ", objectParam);
225+
if (blobStreamingMembers.isEmpty()) {
226+
writer.writeInline("$L", symbol.getName());
227+
} else {
228+
writeInlineStreamingMemberType(writer, symbol, blobStreamingMembers.get(0));
229+
}
230+
writer.openBlock(", path: string = \"\"): __ValidationFailure[] => {", "}", () -> {
231+
structuredMemberWriter.writeMemberValidatorFactory(writer, "memberValidators");
232+
structuredMemberWriter.writeValidateMethodContents(writer, objectParam);
233+
});
229234
});
230235
}
231236
}

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import software.amazon.smithy.model.traits.EnumTrait;
3737
import software.amazon.smithy.model.traits.IdempotencyTokenTrait;
3838
import software.amazon.smithy.model.traits.LengthTrait;
39+
import software.amazon.smithy.model.traits.MediaTypeTrait;
3940
import software.amazon.smithy.model.traits.PatternTrait;
4041
import software.amazon.smithy.model.traits.RangeTrait;
4142
import software.amazon.smithy.model.traits.RequiredTrait;
@@ -347,8 +348,14 @@ void writeMemberValidatorFactory(TypeScriptWriter writer, String cacheName) {
347348
void writeValidateMethodContents(TypeScriptWriter writer, String param) {
348349
writer.openBlock("return [", "];", () -> {
349350
for (MemberShape member : members) {
350-
writer.write("...getMemberValidator($1S).validate($2L.$1L, `$${path}/$3L`),",
351-
getSanitizedMemberName(member), param, member.getMemberName());
351+
String optionalSuffix = "";
352+
if (member.getMemberTrait(model, MediaTypeTrait.class).isPresent()
353+
&& model.expectShape(member.getTarget()) instanceof StringShape) {
354+
// lazy JSON wrapper validation should be done based on the serialized form of the object
355+
optionalSuffix = "?.toString()";
356+
}
357+
writer.write("...getMemberValidator($1S).validate($2L.$1L$4L, `$${path}/$3L`),",
358+
getSanitizedMemberName(member), param, member.getMemberName(), optionalSuffix);
352359
}
353360
});
354361
}
@@ -553,6 +560,14 @@ private Symbol getSymbolForValidatedType(Shape shape) {
553560
return symbolProvider.toSymbol(model.expectShape(ShapeId.from("smithy.api#String")));
554561
}
555562

563+
// Streaming blob inputs can also take string, Uint8Array and Buffer, so we widen the symbol
564+
if (shape.isBlobShape() && shape.hasTrait(StreamingTrait.class)) {
565+
return symbolProvider.toSymbol(shape)
566+
.toBuilder()
567+
.name("Readable | ReadableStream | Blob | string | Uint8Array | Buffer")
568+
.build();
569+
}
570+
556571
return symbolProvider.toSymbol(shape);
557572
}
558573

0 commit comments

Comments
 (0)