From 060a0358a1d61ca951c4db3414fb454de4d726e3 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Thu, 7 Jan 2021 17:39:13 -0800 Subject: [PATCH 1/4] add codegen customization for getbucketlocation deserializers, also make xmlProtocolUtils class public --- .../aws/go/codegen/XmlProtocolUtils.java | 2 +- .../customization/S3GetBucketLocation.java | 211 ++++++++++++++++++ ...mithy.go.codegen.integration.GoIntegration | 1 + 3 files changed, 213 insertions(+), 1 deletion(-) create mode 100644 codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/customization/S3GetBucketLocation.java diff --git a/codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/XmlProtocolUtils.java b/codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/XmlProtocolUtils.java index 4be0852ce5e..e9464c5b00e 100644 --- a/codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/XmlProtocolUtils.java +++ b/codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/XmlProtocolUtils.java @@ -24,7 +24,7 @@ import software.amazon.smithy.model.traits.XmlNamespaceTrait; import software.amazon.smithy.aws.traits.protocols.RestXmlTrait; -final class XmlProtocolUtils { +public final class XmlProtocolUtils { private XmlProtocolUtils() { } diff --git a/codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/customization/S3GetBucketLocation.java b/codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/customization/S3GetBucketLocation.java new file mode 100644 index 00000000000..0e164612a60 --- /dev/null +++ b/codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/customization/S3GetBucketLocation.java @@ -0,0 +1,211 @@ +package software.amazon.smithy.aws.go.codegen.customization; + +import java.util.List; +import software.amazon.smithy.aws.go.codegen.XmlProtocolUtils; +import software.amazon.smithy.aws.traits.ServiceTrait; +import software.amazon.smithy.codegen.core.CodegenException; +import software.amazon.smithy.codegen.core.Symbol; +import software.amazon.smithy.codegen.core.SymbolProvider; +import software.amazon.smithy.go.codegen.GoDelegator; +import software.amazon.smithy.go.codegen.GoSettings; +import software.amazon.smithy.go.codegen.GoStackStepMiddlewareGenerator; +import software.amazon.smithy.go.codegen.GoWriter; +import software.amazon.smithy.go.codegen.SmithyGoDependency; +import software.amazon.smithy.go.codegen.SymbolUtils; +import software.amazon.smithy.go.codegen.integration.GoIntegration; +import software.amazon.smithy.go.codegen.integration.MiddlewareRegistrar; +import software.amazon.smithy.go.codegen.integration.ProtocolGenerator; +import software.amazon.smithy.go.codegen.integration.ProtocolUtils; +import software.amazon.smithy.go.codegen.integration.RuntimeClientPlugin; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.shapes.OperationShape; +import software.amazon.smithy.model.shapes.ServiceShape; +import software.amazon.smithy.model.shapes.Shape; +import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.utils.ListUtils; +import sun.jvm.hotspot.debugger.cdbg.Sym; + + +/** + * This integration generates a custom deserializer for GetBucketLocation response. + * Amazon S3 service does not wrap the GetBucketLocation response with Operation + * name xml tags, and thus custom deserialization is required. + *

+ * Related to aws/aws-sdk-go-v2#908 + */ +public class S3GetBucketLocation implements GoIntegration { + + private final String protocolName = "awsRestxml"; + private final String swapDeserializerFuncName = "swapDeserializerHelper"; + private final String getBucketLocationOpID = "GetBucketLocation"; + + /** + * Return true if service is Amazon S3. + * + * @param model is the generation model. + * @param service is the service shape being audited. + */ + private static boolean isS3Service(Model model, ServiceShape service) { + String serviceId = service.expectTrait(ServiceTrait.class).getSdkId(); + return serviceId.equalsIgnoreCase("S3"); + } + + /** + * returns name of the deserializer middleware written wrt this customization. + * + * @param operation the operation for which custom deserializer is generated. + */ + private String getDeserializeMiddlewareName(OperationShape operation) { + return ProtocolGenerator.getDeserializeMiddlewareName(operation.getId(), protocolName) + "_custom"; + } + + @Override + public List getClientPlugins() { + return ListUtils.of( + RuntimeClientPlugin.builder() + .operationPredicate((model, service, operation) -> { + return isS3Service(model, service) && operation.getId().getName() + .equals(getBucketLocationOpID); + }) + .registerMiddleware(MiddlewareRegistrar.builder() + .resolvedFunction( + SymbolUtils.createValueSymbolBuilder(swapDeserializerFuncName).build()) + .build()) + .build() + ); + } + + @Override + public void writeAdditionalFiles( + GoSettings settings, + Model model, + SymbolProvider symbolProvider, + GoDelegator goDelegator + ) { + ShapeId serviceId = settings.getService(); + ServiceShape service = model.expectShape(serviceId, ServiceShape.class); + if (!isS3Service(model, service)) { + return; + } + + for (ShapeId operationId : service.getAllOperations()) { + if (!(operationId.getName().equals(getBucketLocationOpID))) { + continue; + } + + OperationShape operation = model.expectShape(operationId, OperationShape.class); + goDelegator.useShapeWriter(operation, writer -> { + writeCustomDeserializer(writer, model, symbolProvider, operation); + writeDeserializerSwapFunction(writer, operation); + }); + } + + } + + /** + * writes helper function to swap deserialization middleware with the generated + * custom deserializer middleware. + * + * @param writer is the go writer used + * @param operation is the operation for which swap function is written. + */ + private void writeDeserializerSwapFunction( + GoWriter writer, + OperationShape operation + ) { + writer.writeDocs("$L is helper to swap in a custom deserializer"); + writer.openBlock("func $L(stack *middleware.Stack) error{", "}", + swapDeserializerFuncName, () -> { + writer.write("_, err := stack.Deserialize.Swap($S, &$L{})", + ProtocolUtils.OPERATION_DESERIALIZER_MIDDLEWARE_ID.getString(), + getDeserializeMiddlewareName(operation) + ); + writer.write("if err != nil { return err }"); + writer.write("return nil"); + }); + } + + /** + * writes a custom deserializer middleware for the provided operation. + * + * @param goWriter is the go writer used. + * @param model is the generation model. + * @param symbolProvider is the symbol provider. + * @param operation is the operation shape for which custom deserializer is written. + */ + private void writeCustomDeserializer( + GoWriter goWriter, + Model model, + SymbolProvider symbolProvider, + OperationShape operation + ) { + + GoStackStepMiddlewareGenerator middleware = GoStackStepMiddlewareGenerator.createDeserializeStepMiddleware( + getDeserializeMiddlewareName(operation), ProtocolUtils.OPERATION_DESERIALIZER_MIDDLEWARE_ID); + + String errorFunctionName = ProtocolGenerator.getOperationErrorDeserFunctionName( + operation, protocolName); + + middleware.writeMiddleware(goWriter, (generator, writer) -> { + writer.addUseImports(SmithyGoDependency.FMT); + + writer.write("out, metadata, err = next.$L(ctx, in)", generator.getHandleMethodName()); + writer.write("if err != nil { return out, metadata, err }"); + writer.write(""); + + writer.addUseImports(SmithyGoDependency.SMITHY_HTTP_TRANSPORT); + writer.write("response, ok := out.RawResponse.(*smithyhttp.Response)"); + writer.openBlock("if !ok {", "}", () -> { + writer.addUseImports(SmithyGoDependency.SMITHY); + writer.write(String.format("return out, metadata, &smithy.DeserializationError{Err: %s}", + "fmt.Errorf(\"unknown transport type %T\", out.RawResponse)")); + }); + writer.write(""); + + writer.openBlock("if response.StatusCode < 200 || response.StatusCode >= 300 {", "}", () -> { + writer.write("return out, metadata, $L(response, &metadata)", errorFunctionName); + }); + + Shape outputShape = model.expectShape(operation.getOutput() + .orElseThrow(() -> new CodegenException("expect output shape for operation: " + operation.getId())) + ); + + Symbol outputSymbol = symbolProvider.toSymbol(outputShape); + + // initialize out.Result as output structure shape + writer.write("output := &$T{}", outputSymbol); + writer.write("out.Result = output"); + writer.write(""); + + writer.addUseImports(SmithyGoDependency.XML); + writer.addUseImports(SmithyGoDependency.SMITHY_XML); + writer.addUseImports(SmithyGoDependency.IO); + writer.addUseImports(SmithyGoDependency.SMITHY_IO); + + writer.write("var buff [1024]byte"); + writer.write("ringBuffer := smithyio.NewRingBuffer(buff[:])"); + writer.write("body := io.TeeReader(response.Body, ringBuffer)"); + writer.write("rootDecoder := xml.NewDecoder(body)"); + + // define a decoder with empty start element since we s3 does not wrap Location Constraint + // xml tag with operation specific xml tag. + writer.write("decoder := smithyxml.WrapNodeDecoder(rootDecoder, xml.StartElement{})"); + + String deserFuncName = ProtocolGenerator.getDocumentDeserializerFunctionName(outputShape, protocolName); + writer.addUseImports(SmithyGoDependency.IO); + + // delegate to already generated inner body deserializer function. + writer.write("err = $L(&output, decoder)", deserFuncName); + + // EOF error is valid in this case, as we provide a NOP start element at start. + // Note that we correctly handle unexpected EOF. + writer.addUseImports(SmithyGoDependency.IO); + writer.write("if err == io.EOF { err = nil }"); + + XmlProtocolUtils.handleDecodeError(writer, "out, metadata,"); + + writer.write(""); + writer.write("return out, metadata, err"); + }); + } +} diff --git a/codegen/smithy-aws-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration b/codegen/smithy-aws-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration index 7f26e7341e2..d49e1781ab0 100644 --- a/codegen/smithy-aws-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration +++ b/codegen/smithy-aws-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration @@ -33,3 +33,4 @@ software.amazon.smithy.aws.go.codegen.RequestResponseLogging software.amazon.smithy.aws.go.codegen.customization.S3ControlEndpointResolver software.amazon.smithy.aws.go.codegen.AwsHttpPresignURLClientGenerator software.amazon.smithy.aws.go.codegen.ResolveClientConfig +software.amazon.smithy.aws.go.codegen.customization.S3GetBucketLocation From 34d86ffe331b995f3f044a911f277b0dd0fa66a5 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Thu, 7 Jan 2021 17:40:03 -0800 Subject: [PATCH 2/4] add generated custom deserializer, and unit tests --- service/s3/api_op_GetBucketLocation.go | 66 ++++++++++++++++ .../s3/internal/customizations/unit_test.go | 79 +++++++++++++++++++ 2 files changed, 145 insertions(+) diff --git a/service/s3/api_op_GetBucketLocation.go b/service/s3/api_op_GetBucketLocation.go index 2c2371403be..afbe08206ca 100644 --- a/service/s3/api_op_GetBucketLocation.go +++ b/service/s3/api_op_GetBucketLocation.go @@ -3,13 +3,20 @@ package s3 import ( + "bytes" "context" + "encoding/xml" + "fmt" awsmiddleware "github.com/aws/aws-sdk-go-v2/aws/middleware" "github.com/aws/aws-sdk-go-v2/aws/signer/v4" s3cust "github.com/aws/aws-sdk-go-v2/service/s3/internal/customizations" "github.com/aws/aws-sdk-go-v2/service/s3/types" + smithy "github.com/aws/smithy-go" + smithyxml "github.com/aws/smithy-go/encoding/xml" + smithyio "github.com/aws/smithy-go/io" "github.com/aws/smithy-go/middleware" smithyhttp "github.com/aws/smithy-go/transport/http" + "io" ) // Returns the Region the bucket resides in. You set the bucket's Region using the @@ -106,6 +113,9 @@ func addOperationGetBucketLocationMiddlewares(stack *middleware.Stack, options O if err = smithyhttp.AddCloseResponseBodyMiddleware(stack); err != nil { return err } + if err = swapDeserializerHelper(stack); err != nil { + return err + } if err = addOpGetBucketLocationValidationMiddleware(stack); err != nil { return err } @@ -133,6 +143,62 @@ func addOperationGetBucketLocationMiddlewares(stack *middleware.Stack, options O return nil } +type awsRestxml_deserializeOpGetBucketLocation_custom struct { +} + +func (*awsRestxml_deserializeOpGetBucketLocation_custom) ID() string { + return "OperationDeserializer" +} + +func (m *awsRestxml_deserializeOpGetBucketLocation_custom) HandleDeserialize(ctx context.Context, in middleware.DeserializeInput, next middleware.DeserializeHandler) ( + out middleware.DeserializeOutput, metadata middleware.Metadata, err error, +) { + out, metadata, err = next.HandleDeserialize(ctx, in) + if err != nil { + return out, metadata, err + } + + response, ok := out.RawResponse.(*smithyhttp.Response) + if !ok { + return out, metadata, &smithy.DeserializationError{Err: fmt.Errorf("unknown transport type %T", out.RawResponse)} + } + + if response.StatusCode < 200 || response.StatusCode >= 300 { + return out, metadata, awsRestxml_deserializeOpErrorGetBucketLocation(response, &metadata) + } + output := &GetBucketLocationOutput{} + out.Result = output + + var buff [1024]byte + ringBuffer := smithyio.NewRingBuffer(buff[:]) + body := io.TeeReader(response.Body, ringBuffer) + rootDecoder := xml.NewDecoder(body) + decoder := smithyxml.WrapNodeDecoder(rootDecoder, xml.StartElement{}) + err = awsRestxml_deserializeOpDocumentGetBucketLocationOutput(&output, decoder) + if err == io.EOF { + err = nil + } + if err != nil { + var snapshot bytes.Buffer + io.Copy(&snapshot, ringBuffer) + return out, metadata, &smithy.DeserializationError{ + Err: fmt.Errorf("failed to decode response body, %w", err), + Snapshot: snapshot.Bytes(), + } + } + + return out, metadata, err +} + +// $L is helper to swap in a custom deserializer +func swapDeserializerHelper(stack *middleware.Stack) error { + _, err := stack.Deserialize.Swap("OperationDeserializer", &awsRestxml_deserializeOpGetBucketLocation_custom{}) + if err != nil { + return err + } + return nil +} + func newServiceMetadataMiddleware_opGetBucketLocation(region string) *awsmiddleware.RegisterServiceMetadata { return &awsmiddleware.RegisterServiceMetadata{ Region: region, diff --git a/service/s3/internal/customizations/unit_test.go b/service/s3/internal/customizations/unit_test.go index 4bf401c1f70..8fd87acc017 100644 --- a/service/s3/internal/customizations/unit_test.go +++ b/service/s3/internal/customizations/unit_test.go @@ -5,6 +5,7 @@ import ( "errors" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -79,3 +80,81 @@ func Test_EmptyResponse(t *testing.T) { }) } } + +func TestBucketLocationPopulation(t *testing.T) { + cases := map[string]struct { + responseBody string + expectLocation string + expectError string + }{ + "empty location": { + responseBody: ``, + expectLocation: "", + }, + "EU location": { + responseBody: `EU`, + expectLocation: "EU", + }, + "AfSouth1 location": { + responseBody: `af-south-1`, + expectLocation: "af-south-1", + }, + "IncompleteResponse": { + responseBody: ``, + expectError: "unexpected EOF", + }, + } + + for name, c := range cases { + t.Run(name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + w.Write([]byte(c.responseBody)) + })) + defer server.Close() + + ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelFn() + + cfg := aws.Config{ + Region: "us-east-1", + EndpointResolver: aws.EndpointResolverFunc(func(service, region string) (aws.Endpoint, error) { + return aws.Endpoint{ + URL: server.URL, + SigningName: "s3", + }, nil + }), + Retryer: aws.NopRetryer{}, + } + + client := s3.NewFromConfig(cfg, func(options *s3.Options) { + options.UsePathStyle = true + }) + + params := &s3.GetBucketLocationInput{ + Bucket: aws.String("aws-sdk-go-data"), + } + resp, err := client.GetBucketLocation(ctx, params) + if len(c.expectError) != 0 && err == nil { + t.Fatal("expect error, got none") + } + + if err != nil && len(c.expectError) == 0 { + t.Fatalf("expect no error, got %v", err) + } else { + if err != nil { + if !strings.Contains(err.Error(), c.expectError) { + t.Fatalf("expect error to be %v, got %v", err.Error(), c.expectError) + } + return + } + } + + if e, a := c.expectLocation, resp.LocationConstraint; !strings.EqualFold(e, string(a)) { + t.Fatalf("expected location constraint to be deserialized as %v, got %v", e, a) + } + }) + } + +} From f27f164148d742caaa3487e2ad341e5804944e29 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Thu, 7 Jan 2021 22:27:22 -0800 Subject: [PATCH 3/4] remove unused import --- .../codegen/customization/S3GetBucketLocation.java | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/customization/S3GetBucketLocation.java b/codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/customization/S3GetBucketLocation.java index 0e164612a60..e8398710d25 100644 --- a/codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/customization/S3GetBucketLocation.java +++ b/codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/customization/S3GetBucketLocation.java @@ -23,8 +23,6 @@ import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.model.shapes.ShapeId; import software.amazon.smithy.utils.ListUtils; -import sun.jvm.hotspot.debugger.cdbg.Sym; - /** * This integration generates a custom deserializer for GetBucketLocation response. @@ -42,7 +40,7 @@ public class S3GetBucketLocation implements GoIntegration { /** * Return true if service is Amazon S3. * - * @param model is the generation model. + * @param model is the generation model. * @param service is the service shape being audited. */ private static boolean isS3Service(Model model, ServiceShape service) { @@ -106,14 +104,14 @@ public void writeAdditionalFiles( * writes helper function to swap deserialization middleware with the generated * custom deserializer middleware. * - * @param writer is the go writer used + * @param writer is the go writer used * @param operation is the operation for which swap function is written. */ private void writeDeserializerSwapFunction( GoWriter writer, OperationShape operation ) { - writer.writeDocs("$L is helper to swap in a custom deserializer"); + writer.writeDocs("Helper to swap in a custom deserializer"); writer.openBlock("func $L(stack *middleware.Stack) error{", "}", swapDeserializerFuncName, () -> { writer.write("_, err := stack.Deserialize.Swap($S, &$L{})", @@ -128,10 +126,10 @@ private void writeDeserializerSwapFunction( /** * writes a custom deserializer middleware for the provided operation. * - * @param goWriter is the go writer used. - * @param model is the generation model. + * @param goWriter is the go writer used. + * @param model is the generation model. * @param symbolProvider is the symbol provider. - * @param operation is the operation shape for which custom deserializer is written. + * @param operation is the operation shape for which custom deserializer is written. */ private void writeCustomDeserializer( GoWriter goWriter, From e790084b9d018c3e4d6dcb5501dabd5b1b4658e2 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Mon, 11 Jan 2021 12:11:47 -0800 Subject: [PATCH 4/4] fix comment --- service/s3/api_op_GetBucketLocation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/s3/api_op_GetBucketLocation.go b/service/s3/api_op_GetBucketLocation.go index afbe08206ca..6776ca755ca 100644 --- a/service/s3/api_op_GetBucketLocation.go +++ b/service/s3/api_op_GetBucketLocation.go @@ -190,7 +190,7 @@ func (m *awsRestxml_deserializeOpGetBucketLocation_custom) HandleDeserialize(ctx return out, metadata, err } -// $L is helper to swap in a custom deserializer +// Helper to swap in a custom deserializer func swapDeserializerHelper(stack *middleware.Stack) error { _, err := stack.Deserialize.Swap("OperationDeserializer", &awsRestxml_deserializeOpGetBucketLocation_custom{}) if err != nil {