Skip to content

Commit d4a5ddd

Browse files
committed
Fix several serde issues for http binding protocols
This commit fixes several issues related to typing for shapes bound to http headers on input and output. It also fixes an issue where payload shapes would not be properly redirected to their respective serde function.
1 parent b4b4153 commit d4a5ddd

File tree

1 file changed

+125
-11
lines changed

1 file changed

+125
-11
lines changed

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java

Lines changed: 125 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434
import software.amazon.smithy.model.shapes.BooleanShape;
3535
import software.amazon.smithy.model.shapes.CollectionShape;
3636
import software.amazon.smithy.model.shapes.DocumentShape;
37+
import software.amazon.smithy.model.shapes.DoubleShape;
38+
import software.amazon.smithy.model.shapes.FloatShape;
3739
import software.amazon.smithy.model.shapes.MemberShape;
3840
import software.amazon.smithy.model.shapes.NumberShape;
3941
import software.amazon.smithy.model.shapes.OperationShape;
@@ -42,6 +44,7 @@
4244
import software.amazon.smithy.model.shapes.StringShape;
4345
import software.amazon.smithy.model.shapes.StructureShape;
4446
import software.amazon.smithy.model.shapes.TimestampShape;
47+
import software.amazon.smithy.model.shapes.UnionShape;
4548
import software.amazon.smithy.model.traits.HttpTrait;
4649
import software.amazon.smithy.model.traits.TimestampFormatTrait.Format;
4750
import software.amazon.smithy.typescript.codegen.ApplicationProtocol;
@@ -280,7 +283,7 @@ private void writeHeaders(
280283
Shape target = index.getShape(binding.getMember().getTarget()).get();
281284
String headerValue = getInputValue(context, binding.getLocation(), "input." + memberName,
282285
binding.getMember(), target);
283-
writer.write("headers['$L'] = $L;", binding.getLocationName(), headerValue);
286+
writer.write("headers[$S] = $L;", binding.getLocationName(), headerValue);
284287
});
285288
}
286289

@@ -294,7 +297,7 @@ private void writeHeaders(
294297
String headerValue = getInputValue(context, binding.getLocation(),
295298
"input." + memberName + "[suffix]", binding.getMember(), target);
296299
// Append the suffix to the defined prefix and serialize the value in to that key.
297-
writer.write("headers['$L' + suffix] = $L;", binding.getLocationName(), headerValue);
300+
writer.write("headers[$S + suffix] = $L;", binding.getLocationName(), headerValue);
298301
});
299302
});
300303
}
@@ -318,14 +321,18 @@ private List<HttpBinding> writeRequestBody(
318321
return documentBindings;
319322
}
320323
if (!payloadBindings.isEmpty()) {
321-
// TODO Validate payload structures are handled correctly.
322324
SymbolProvider symbolProvider = context.getSymbolProvider();
323325
// There can only be one payload binding.
324326
HttpBinding binding = payloadBindings.get(0);
325327
String memberName = symbolProvider.toMemberName(binding.getMember());
326-
Shape target = context.getModel().getShapeIndex().getShape(binding.getMember().getTarget()).get();
327-
writer.write("let body: any = $L;", getInputValue(
328-
context, Location.PAYLOAD, "input." + memberName, binding.getMember(), target));
328+
329+
// Write the default `body` property.
330+
writer.write("let body: any = {};");
331+
writer.openBlock("if (input.$L !== undefined) {", "}", memberName, () -> {
332+
Shape target = context.getModel().getShapeIndex().getShape(binding.getMember().getTarget()).get();
333+
writer.write("body = $L;", getInputValue(
334+
context, Location.PAYLOAD, "input." + memberName, binding.getMember(), target));
335+
});
329336
return payloadBindings;
330337
}
331338

@@ -363,6 +370,8 @@ private String getInputValue(
363370
return getBlobInputParam(bindingType, dataSource);
364371
} else if (target instanceof CollectionShape) {
365372
return getCollectionInputParam(bindingType, dataSource);
373+
} else if (target instanceof StructureShape || target instanceof UnionShape) {
374+
return getNamedMembersInputParam(context, bindingType, dataSource, target);
366375
}
367376

368377
throw new CodegenException(String.format(
@@ -418,6 +427,35 @@ private String getCollectionInputParam(
418427
}
419428
}
420429

430+
/**
431+
* Given context and a source of data, generate an input value provider for the
432+
* shape. This redirects to a serialization function for payloads,
433+
* and fails otherwise.
434+
*
435+
* @param context The generation context.
436+
* @param bindingType How this value is bound to the operation input.
437+
* @param dataSource The in-code location of the data to provide an input of
438+
* ({@code input.foo}, {@code entry}, etc.)
439+
* @param target The shape of the value being provided.
440+
* @return Returns a value or expression of the input shape.
441+
*/
442+
private String getNamedMembersInputParam(
443+
GenerationContext context,
444+
Location bindingType,
445+
String dataSource,
446+
Shape target
447+
) {
448+
switch (bindingType) {
449+
case PAYLOAD:
450+
// Redirect to a serialization function.
451+
Symbol symbol = context.getSymbolProvider().toSymbol(target);
452+
return ProtocolGenerator.getSerFunctionName(symbol, context.getProtocolName())
453+
+ "(" + dataSource + ", context)";
454+
default:
455+
throw new CodegenException("Unexpected named member shape binding location `" + bindingType + "`");
456+
}
457+
}
458+
421459
/**
422460
* Writes the code needed to serialize the input document of a request.
423461
*
@@ -517,10 +555,10 @@ private void readHeaders(
517555
ShapeIndex index = context.getModel().getShapeIndex();
518556
for (HttpBinding binding : bindingIndex.getRequestBindings(operation, Location.HEADER)) {
519557
String memberName = symbolProvider.toMemberName(binding.getMember());
520-
writer.openBlock("if (output.headers[$L] !== undefined) {", "}", binding.getLocationName(), () -> {
558+
writer.openBlock("if (output.headers[$S] !== undefined) {", "}", binding.getLocationName(), () -> {
521559
Shape target = index.getShape(binding.getMember().getTarget()).get();
522560
String headerValue = getOutputValue(context, binding.getLocation(),
523-
"output.headers[" + binding.getLocationName() + "]", binding.getMember(), target);
561+
"output.headers['" + binding.getLocationName() + "']", binding.getMember(), target);
524562
writer.write("contents.$L = $L;", memberName, headerValue);
525563
});
526564
}
@@ -567,11 +605,10 @@ private List<HttpBinding> readResponseBody(
567605
return documentBindings;
568606
}
569607
if (!payloadBindings.isEmpty()) {
570-
// TODO Validate payload structures are handled correctly.
571608
// There can only be one payload binding.
572609
HttpBinding binding = payloadBindings.get(0);
573610
Shape target = context.getModel().getShapeIndex().getShape(binding.getMember().getTarget()).get();
574-
writer.write("output.$L = $L;", binding.getMemberName(), getOutputValue(context,
611+
writer.write("contents.$L = $L;", binding.getMemberName(), getOutputValue(context,
575612
Location.PAYLOAD, "data", binding.getMember(), target));
576613
return payloadBindings;
577614
}
@@ -599,7 +636,11 @@ private String getOutputValue(
599636
MemberShape member,
600637
Shape target
601638
) {
602-
if (isNativeSimpleType(target)) {
639+
if (target instanceof NumberShape) {
640+
return getNumberOutputParam(bindingType, dataSource, target);
641+
} else if (target instanceof BooleanShape) {
642+
return getBooleanOutputParam(bindingType, dataSource);
643+
} else if (target instanceof StringShape || target instanceof DocumentShape) {
603644
return dataSource;
604645
} else if (target instanceof TimestampShape) {
605646
HttpBindingIndex httpIndex = context.getModel().getKnowledge(HttpBindingIndex.class);
@@ -609,13 +650,34 @@ private String getOutputValue(
609650
return getBlobOutputParam(bindingType, dataSource);
610651
} else if (target instanceof CollectionShape) {
611652
return getCollectionOutputParam(bindingType, dataSource);
653+
} else if (target instanceof StructureShape || target instanceof UnionShape) {
654+
return getNamedMembersOutputParam(context, bindingType, dataSource, target);
612655
}
613656

614657
throw new CodegenException(String.format(
615658
"Unsupported %s binding of %s to %s in %s using the %s protocol",
616659
bindingType, member.getMemberName(), target.getType(), member.getContainer(), getName()));
617660
}
618661

662+
/**
663+
* Given context and a source of data, generate an output value provider for the
664+
* boolean. By default, this checks strict equality to 'true'in headers and passes
665+
* through for documents.
666+
*
667+
* @param bindingType How this value is bound to the operation output.
668+
* @param dataSource The in-code location of the data to provide an output of
669+
* ({@code output.foo}, {@code entry}, etc.)
670+
* @return Returns a value or expression of the output boolean.
671+
*/
672+
private String getBooleanOutputParam(Location bindingType, String dataSource) {
673+
switch (bindingType) {
674+
case HEADER:
675+
return dataSource + " === 'true'";
676+
default:
677+
throw new CodegenException("Unexpected blob binding location `" + bindingType + "`");
678+
}
679+
}
680+
619681
/**
620682
* Given context and a source of data, generate an output value provider for the
621683
* blob. By default, this base64 decodes content in headers and passes through
@@ -660,6 +722,58 @@ private String getCollectionOutputParam(
660722
}
661723
}
662724

725+
/**
726+
* Given context and a source of data, generate an output value provider for the
727+
* shape. This redirects to a deserialization function for documents and payloads,
728+
* and fails otherwise.
729+
*
730+
* @param context The generation context.
731+
* @param bindingType How this value is bound to the operation output.
732+
* @param dataSource The in-code location of the data to provide an output of
733+
* ({@code output.foo}, {@code entry}, etc.)
734+
* @param target The shape of the value being provided.
735+
* @return Returns a value or expression of the output shape.
736+
*/
737+
private String getNamedMembersOutputParam(
738+
GenerationContext context,
739+
Location bindingType,
740+
String dataSource,
741+
Shape target
742+
) {
743+
switch (bindingType) {
744+
case PAYLOAD:
745+
// Redirect to a deserialization function.
746+
Symbol symbol = context.getSymbolProvider().toSymbol(target);
747+
return ProtocolGenerator.getDeserFunctionName(symbol, context.getProtocolName())
748+
+ "(" + dataSource + ", context)";
749+
default:
750+
throw new CodegenException("Unexpected named member shape binding location `" + bindingType + "`");
751+
}
752+
}
753+
754+
/**
755+
* Given context and a source of data, generate an output value provider for the
756+
* number. By default, invokes parseInt on byte/short/integer/long types in headers,
757+
* invokes parseFloat on float/double types in headers, and fails otherwise.
758+
*
759+
* @param bindingType How this value is bound to the operation output.
760+
* @param dataSource The in-code location of the data to provide an output of
761+
* ({@code output.foo}, {@code entry}, etc.)
762+
* @param target The shape of the value being provided.
763+
* @return Returns a value or expression of the output number.
764+
*/
765+
private String getNumberOutputParam(Location bindingType, String dataSource, Shape target) {
766+
switch (bindingType) {
767+
case HEADER:
768+
if (target instanceof FloatShape || target instanceof DoubleShape) {
769+
return "parseFloat(" + dataSource + ", 10)";
770+
}
771+
return "parseInt(" + dataSource + ", 10)";
772+
default:
773+
throw new CodegenException("Unexpected number binding location `" + bindingType + "`");
774+
}
775+
}
776+
663777
/**
664778
* Writes the code that loads an {@code errorCode} String with the content used
665779
* to dispatch errors to specific serializers.

0 commit comments

Comments
 (0)