Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

codegen: Update generated serde to skip unmodeled input/outputs #261

Merged
merged 2 commits into from
Jan 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ public static Model execute(Model model, ShapeId serviceShapeId) {
private static StructureShape emptyOperationStructure(ShapeId opShapeId, String suffix) {
return StructureShape.builder()
.id(ShapeId.fromParts(CodegenUtils.getSyntheticTypeNamespace(), opShapeId.getName() + suffix))
.addTrait(SyntheticClone.builder().build())
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
import java.util.function.Predicate;
import java.util.logging.Logger;
import software.amazon.smithy.codegen.core.CodegenException;
Expand Down Expand Up @@ -145,6 +146,22 @@ public static String getSyntheticTypeNamespace() {
return CodegenUtils.SYNTHETIC_NAMESPACE;
}

/**
* Get if the passed in shape is decorated as a synthetic clone, but there is no other shape the clone is
* created from.
*
* @param shape the shape to check if its a stubbed synthetic clone without an archetype.
* @return if the shape is synthetic clone, but not based on a specific shape.
*/
public static boolean isStubSyntheticClone(Shape shape) {
Optional<SyntheticClone> optional = shape.getTrait(SyntheticClone.class);
if (!optional.isPresent()) {
return false;
}

SyntheticClone synthClone = optional.get();
return !synthClone.getArchetype().isPresent();
}

/**
* Returns the operand decorated with an &amp; if the address of the shape type can be taken.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

package software.amazon.smithy.go.codegen;

import java.util.Optional;
import software.amazon.smithy.codegen.core.CodegenException;
import software.amazon.smithy.model.node.Node;
import software.amazon.smithy.model.shapes.ShapeId;
Expand All @@ -25,15 +26,15 @@

/**
* Defines a shape as being a clone of another modeled shape.
*
* <p>
* Must only be used as a runtime trait-only applied to shapes based on model processing
*/
public final class SyntheticClone extends AbstractTrait implements ToSmithyBuilder<SyntheticClone> {
public static final ShapeId ID = ShapeId.from("smithy.go.traits#SyntheticClone");

private static final String ARCHETYPE = "archetype";

private final ShapeId archetype;
private final Optional<ShapeId> archetype;

private SyntheticClone(Builder builder) {
super(ID, builder.getSourceLocation());
Expand All @@ -45,7 +46,7 @@ private SyntheticClone(Builder builder) {
*
* @return the original archetype shape
*/
public ShapeId getArchetype() {
public Optional<ShapeId> getArchetype() {
return archetype;
}

Expand All @@ -56,8 +57,10 @@ protected Node createNode() {

@Override
public SmithyBuilder<SyntheticClone> toBuilder() {
return builder()
.archetype(getArchetype());
Builder builder = builder();
getArchetype().ifPresent(builder::archetype);

return builder;
}

/**
Expand All @@ -71,7 +74,7 @@ public static Builder builder() {
* Builder for {@link SyntheticClone}.
*/
public static final class Builder extends AbstractTraitBuilder<SyntheticClone, Builder> {
private ShapeId archetype;
private Optional<ShapeId> archetype = Optional.empty();

private Builder() {
}
Expand All @@ -82,7 +85,12 @@ public SyntheticClone build() {
}

public Builder archetype(ShapeId archetype) {
this.archetype = archetype;
this.archetype = Optional.ofNullable(archetype);
return this;
}

public Builder removeArchetype() {
this.archetype = Optional.empty();
return this;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,11 @@ public void generateRequestSerializers(GenerationContext context) {
private void generateOperationSerializer(GenerationContext context, OperationShape operation) {
generateOperationSerializerMiddleware(context, operation);
generateOperationHttpBindingSerializer(context, operation);
generateOperationDocumentSerializer(context, operation);
addOperationDocumentShapeBindersForSerializer(context, operation);

if (!CodegenUtils.isStubSyntheticClone(ProtocolUtils.expectInput(context.getModel(), operation))) {
generateOperationDocumentSerializer(context, operation);
addOperationDocumentShapeBindersForSerializer(context, operation);
}
}

/**
Expand Down Expand Up @@ -240,32 +243,36 @@ private void generateOperationSerializerMiddleware(GenerationContext context, Op
writer.write("");
}

// document bindings vs payload bindings
HttpBindingIndex httpBindingIndex = model.getKnowledge(HttpBindingIndex.class);
boolean hasDocumentBindings = !httpBindingIndex
.getRequestBindings(operation, HttpBinding.Location.DOCUMENT)
.isEmpty();
Optional<HttpBinding> payloadBinding = httpBindingIndex.getRequestBindings(operation,
HttpBinding.Location.PAYLOAD).stream().findFirst();


if (hasDocumentBindings) {
// delegate the setup and usage of the document serializer function for the protocol
writeMiddlewareDocumentSerializerDelegator(context, operation, generator);

} else if (payloadBinding.isPresent()) {
// delegate the setup and usage of the payload serializer function for the protocol
MemberShape memberShape = payloadBinding.get().getMember();
writeMiddlewarePayloadSerializerDelegator(context, memberShape);
// Don't consider serializing the body if the input shape is a stubbed synthetic clone, without an
// archetype.
if (!CodegenUtils.isStubSyntheticClone(ProtocolUtils.expectInput(model, operation))) {
// document bindings vs payload bindings
HttpBindingIndex httpBindingIndex = model.getKnowledge(HttpBindingIndex.class);
boolean hasDocumentBindings = !httpBindingIndex
.getRequestBindings(operation, HttpBinding.Location.DOCUMENT)
.isEmpty();
Optional<HttpBinding> payloadBinding = httpBindingIndex.getRequestBindings(operation,
HttpBinding.Location.PAYLOAD).stream().findFirst();

if (hasDocumentBindings) {
// delegate the setup and usage of the document serializer function for the protocol
writeMiddlewareDocumentSerializerDelegator(context, operation, generator);

} else if (payloadBinding.isPresent()) {
// delegate the setup and usage of the payload serializer function for the protocol
MemberShape memberShape = payloadBinding.get().getMember();
writeMiddlewarePayloadSerializerDelegator(context, memberShape);
}
writer.write("");
}

writer.write("");
// Serialize HTTP request with payload, if set.
writer.openBlock("if request.Request, err = restEncoder.Encode(request.Request); err != nil {", "}", () -> {
writer.write("return out, metadata, &smithy.SerializationError{Err: err}");
});
// Ensure the request value is updated.
writer.write("in.Request = request");
writer.write("");

writer.write("return next.$L(ctx, in)", generator.getHandleMethodName());
});
}
Expand Down Expand Up @@ -331,12 +338,21 @@ private void generateOperationDeserializerMiddleware(GenerationContext context,
writer.write("");
}

// Output Shape Document Binding middleware generation
if (isShapeWithResponseBindings(model, operation, HttpBinding.Location.DOCUMENT)
// Discard without deserializing the response if the input shape is a stubbed synthetic clone
// without an archetype.
if (CodegenUtils.isStubSyntheticClone(ProtocolUtils.expectOutput(model, operation))) {
writer.addUseImports(SmithyGoDependency.IOUTIL);
writer.openBlock("if _, err = io.Copy(ioutil.Discard, response.Body); err != nil {", "}", () -> {
writer.openBlock("return out, metadata, &smithy.DeserializationError{", "}", () -> {
writer.write("Err: fmt.Errorf(\"failed to discard response body, %w\", err),");
});
});
} else if (isShapeWithResponseBindings(model, operation, HttpBinding.Location.DOCUMENT)
|| isShapeWithResponseBindings(model, operation, HttpBinding.Location.PAYLOAD)) {
// Output Shape Document Binding middleware generation
writeMiddlewareDocumentDeserializerDelegator(context, operation, generator);
writer.write("");
}
writer.write("");

writer.write("return out, metadata, err");
});
Expand Down Expand Up @@ -800,8 +816,11 @@ public void generateResponseDeserializers(GenerationContext context) {
for (OperationShape operation : getHttpBindingOperations(context)) {
generateOperationDeserializerMiddleware(context, operation);
generateHttpBindingDeserializer(context, operation);
generateOperationDocumentDeserializer(context, operation);
addOperationDocumentShapeBindersForDeserializer(context, operation);

if (!CodegenUtils.isStubSyntheticClone(ProtocolUtils.expectOutput(context.getModel(), operation))) {
generateOperationDocumentDeserializer(context, operation);
addOperationDocumentShapeBindersForDeserializer(context, operation);
}
}

for (StructureShape error : deserializingErrorShapes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import software.amazon.smithy.codegen.core.Symbol;
import software.amazon.smithy.codegen.core.SymbolProvider;
import software.amazon.smithy.go.codegen.ApplicationProtocol;
import software.amazon.smithy.go.codegen.CodegenUtils;
import software.amazon.smithy.go.codegen.GoStackStepMiddlewareGenerator;
import software.amazon.smithy.go.codegen.GoWriter;
import software.amazon.smithy.go.codegen.SmithyGoDependency;
Expand All @@ -38,7 +39,8 @@ public abstract class HttpRpcProtocolGenerator implements ProtocolGenerator {
/**
* Creates an Http RPC protocol generator.
*/
public HttpRpcProtocolGenerator() { }
public HttpRpcProtocolGenerator() {
}

@Override
public ApplicationProtocol getApplicationProtocol() {
Expand Down Expand Up @@ -111,6 +113,7 @@ private void generateOperationSerializer(GenerationContext context, OperationSha

// Cast the input parameters to the operation request type and check for errors.
writer.write("input, ok := in.Parameters.($P)", inputSymbol);
writer.write("_ = input");
writer.openBlock("if !ok {", "}", () -> {
writer.write("return out, metadata, "
+ "&smithy.SerializationError{Err: fmt.Errorf(\"unknown input parameters type %T\","
Expand All @@ -129,13 +132,18 @@ private void generateOperationSerializer(GenerationContext context, OperationSha

// delegate the setup and usage of the document serializer function for the protocol
serializeInputDocument(context, operation);
serializingDocumentShapes.add(ProtocolUtils.expectInput(model, operation));
// Skipping calling serializer method for the input shape is responsibility of the
// serializeInputDocument implementation.
if (!CodegenUtils.isStubSyntheticClone(ProtocolUtils.expectInput(context.getModel(), operation))) {
serializingDocumentShapes.add(ProtocolUtils.expectInput(model, operation));
}

writer.write("");

writer.openBlock("if request.Request, err = httpBindingEncoder.Encode(request.Request); err != nil {",
"}", () -> {
writer.write("return out, metadata, &smithy.SerializationError{Err: err}");
});
writer.write("return out, metadata, &smithy.SerializationError{Err: err}");
});
// Ensure the request value is updated if modified for a document.
writer.write("in.Request = request");

Expand Down Expand Up @@ -164,7 +172,8 @@ private void writeRequestHeaders(GenerationContext context, OperationShape opera
* @param operation The operation being generated.
* @param writer The writer to use.
*/
protected void writeDefaultHeaders(GenerationContext context, OperationShape operation, GoWriter writer) {}
protected void writeDefaultHeaders(GenerationContext context, OperationShape operation, GoWriter writer) {
}

/**
* Provides the request path for the operation.
Expand Down Expand Up @@ -256,8 +265,20 @@ private void generateOperationDeserializer(GenerationContext context, OperationS
writer.write("out.Result = output");
writer.write("");

deserializeOutputDocument(context, operation);
deserializingDocumentShapes.add(ProtocolUtils.expectOutput(model, operation));
// Discard without deserializing the response if the input shape is a stubbed synthetic clone
// without an archetype.
if (CodegenUtils.isStubSyntheticClone(ProtocolUtils.expectOutput(model, operation))) {
writer.addUseImports(SmithyGoDependency.IOUTIL);
writer.openBlock("if _, err = io.Copy(ioutil.Discard, response.Body); err != nil {", "}",
() -> {
writer.openBlock("return out, metadata, &smithy.DeserializationError{", "}", () -> {
writer.write("Err: fmt.Errorf(\"failed to discard response body, %w\", err),");
});
});
} else {
deserializeOutputDocument(context, operation);
deserializingDocumentShapes.add(ProtocolUtils.expectOutput(model, operation));
}
writer.write("");

writer.write("return out, metadata, err");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,14 @@ public void testOperationWithoutInputOutput() {
shape.isPresent(), Matchers.is(true));
MatcherAssert.assertThat(shapeId + " shape must have no members",
shape.get().members().size(), Matchers.equalTo(0));
if (shape.get().getTrait(SyntheticClone.class).isPresent()) {
MatcherAssert.assertThat("shape is not synthetic clone", false);

Optional<SyntheticClone> opSynthClone = shape.get().getTrait(SyntheticClone.class);
if (opSynthClone.isPresent()) {
SyntheticClone synthClone = opSynthClone.get();
MatcherAssert.assertThat(shapeId + " shape must not have archetype",
synthClone.getArchetype().isPresent(), Matchers.equalTo(false));
} else {
MatcherAssert.assertThat(shapeId + " shape must be synthetic clone", false);
}
});
}
Expand Down Expand Up @@ -118,14 +124,15 @@ public void testOperationWithExistingInputOutput() {
MatcherAssert.assertThat("foo member present", fooMember.isPresent(), Matchers.is(true));

ShapeId id = fooMember.get().getId();
MatcherAssert.assertThat("foo is correct namespace", id.getNamespace(), Matchers.equalTo(shapeId.getNamespace()));
MatcherAssert.assertThat("foo is correct namespace", id.getNamespace(),
Matchers.equalTo(shapeId.getNamespace()));
MatcherAssert.assertThat("foo is correct parent", id.getName(), Matchers.equalTo(shapeId.getName()));

Optional<SyntheticClone> syntheticClone = shape.get().getTrait(SyntheticClone.class);
if (!syntheticClone.isPresent()) {
MatcherAssert.assertThat("shape must be marked as synthetic clone", false);
MatcherAssert.assertThat(shapeId + " shape must be marked as synthetic clone", false);
} else {
MatcherAssert.assertThat(syntheticClone.get().getArchetype().toString(),
MatcherAssert.assertThat(syntheticClone.get().getArchetype().get().toString(),
Matchers.is(Matchers.oneOf(NAMESPACE + "#TestOperationRequest",
NAMESPACE + "#TestOperationResponse")));
}
Expand Down Expand Up @@ -161,7 +168,7 @@ public void testOperationWithExistingInputOutputWithConflitcs() {
ShapeId expInputRename = ShapeId.fromParts(CodegenUtils.getSyntheticTypeNamespace(), "TestOperationInput");
ShapeId expOutputRename = ShapeId.fromParts(CodegenUtils.getSyntheticTypeNamespace(), "TestOperationOutput");

ListUtils.of(expInputRename, expOutputRename, inputConflict.getId(), outputConflict.getId())
ListUtils.of(inputConflict.getId(), outputConflict.getId())
.forEach(shapeId -> {
Optional<Shape> shape = processedModel.getShape(shapeId);
MatcherAssert.assertThat(shapeId + " shape must be present in model",
Expand All @@ -171,5 +178,21 @@ public void testOperationWithExistingInputOutputWithConflitcs() {
MatcherAssert.assertThat("shape must not be marked as synthetic clone", false);
}
});

ListUtils.of(expInputRename, expOutputRename)
.forEach(shapeId -> {
Optional<Shape> shape = processedModel.getShape(shapeId);
MatcherAssert.assertThat(shapeId + " shape must be present in model",
shape.isPresent(), Matchers.is(true));

Optional<SyntheticClone> opSynthClone = shape.get().getTrait(SyntheticClone.class);
if (opSynthClone.isPresent()) {
SyntheticClone synthClone = opSynthClone.get();
MatcherAssert.assertThat(shapeId + " shape must not have archetype",
synthClone.getArchetype().isPresent(), Matchers.equalTo(false));
} else {
MatcherAssert.assertThat(shapeId + " shape must be synthetic clone", false);
}
});
}
}