From 152e7333c57003034b8e863b635e718ce42e1932 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Tue, 1 Dec 2020 02:40:09 -0800 Subject: [PATCH 01/23] draft for waiter support --- .../smithy/go/codegen/SmithyGoDependency.java | 9 + .../go/codegen/integration/Waiters.java | 610 ++++++++++++++++++ internal/waiters/waiter.go | 154 +++++ 3 files changed, 773 insertions(+) create mode 100644 codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java create mode 100644 internal/waiters/waiter.go diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java index 071669b93..05fb81551 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java @@ -53,12 +53,16 @@ public final class SmithyGoDependency { public static final GoDependency SMITHY_RAND = smithy("rand", "smithyrand"); public static final GoDependency SMITHY_TESTING = smithy("testing", "smithytesting"); public static final GoDependency SMITHY_XML = smithy("xml", "smithyxml"); + public static final GoDependency SMITHY_WAITERS = smithy("waiters", "smithywaiters"); public static final GoDependency GO_CMP = goCmp("cmp"); public static final GoDependency GO_CMP_OPTIONS = goCmp("cmp/cmpopts"); + public static final GoDependency GO_JMESPATH = goJmespath(""); + private static final String SMITHY_SOURCE_PATH = "github.com/awslabs/smithy-go"; private static final String GO_CMP_SOURCE_PATH = "github.com/google/go-cmp"; + private static final String GO_JMESPATH_SOURCE_PATH = "github.com/jmespath/go-jmespath"; private SmithyGoDependency() { } @@ -94,6 +98,10 @@ private static GoDependency goCmp(String relativePath) { return relativePackage(GO_CMP_SOURCE_PATH, relativePath, Versions.GO_CMP, null); } + private static GoDependency goJmespath(String relativePath) { + return relativePackage(GO_JMESPATH_SOURCE_PATH, relativePath, Versions.GO_JMESPATH, null); + } + private static GoDependency relativePackage( String moduleImportPath, String relativePath, @@ -111,5 +119,6 @@ private static final class Versions { private static final String GO_STDLIB = "1.15"; private static final String GO_CMP = "v0.5.4"; private static final String SMITHY_GO = "v0.4.0"; + private static final String GO_JMESPATH = "v0.4.0"; } } diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java new file mode 100644 index 000000000..85a965b3a --- /dev/null +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java @@ -0,0 +1,610 @@ +/* + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.smithy.go.codegen.integration; + +import java.util.Map; +import java.util.Optional; +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.GoWriter; +import software.amazon.smithy.go.codegen.SmithyGoDependency; +import software.amazon.smithy.go.codegen.SymbolUtils; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.knowledge.TopDownIndex; +import software.amazon.smithy.model.shapes.OperationShape; +import software.amazon.smithy.model.shapes.ServiceShape; +import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.model.shapes.StructureShape; +import software.amazon.smithy.utils.StringUtils; +import software.amazon.smithy.waiters.Acceptor; +import software.amazon.smithy.waiters.Matcher; +import software.amazon.smithy.waiters.PathComparator; +import software.amazon.smithy.waiters.WaitableTrait; +import software.amazon.smithy.waiters.Waiter; + +/** + * Implements support for WaitableTrait. + */ +public class Waiters implements GoIntegration { + private static final String WAITER_INVOKER_FUNCTION_NAME = "Wait"; + private static final String WAITER_MIDDLEWARE_NAME = "WaiterRetrier"; + private static final int DEFAULT_MAX_WAIT_TIME = 300; + private static final int DEFAULT_MAX_ATTEMPTS = 8; + + + @Override + public void writeAdditionalFiles( + GoSettings settings, + Model model, + SymbolProvider symbolProvider, + GoDelegator goDelegator + ) { + ServiceShape serviceShape = settings.getService(model); + TopDownIndex topDownIndex = TopDownIndex.of(model); + + topDownIndex.getContainedOperations(serviceShape).stream() + .forEach(operation -> { + if (!operation.hasTrait(WaitableTrait.ID)) { + return; + } + + Map waiters = operation.expectTrait(WaitableTrait.class).getWaiters(); + + goDelegator.useShapeWriter(operation, writer -> { + generateOperationWaiter(model, symbolProvider, writer, operation, waiters); + }); + }); + } + + + /** + * Generates all waiter components used for the operation. + */ + private void generateOperationWaiter( + Model model, + SymbolProvider symbolProvider, + GoWriter writer, + OperationShape operation, + Map waiters + ) { + // write client interface + generateAPIClientInterface(model, symbolProvider, writer, operation); + + // generate waiter function + waiters.forEach((name, waiter) -> { + // write waiter options + generateWaiterOptions(model, symbolProvider, writer, operation, name); + + // write waiter client + generateWaiterClient(model, symbolProvider, writer, operation, name, waiter); + + // write waiter specific invoker + generateWaiterInvoker(model, symbolProvider, writer, operation, name, waiter); + + // write waiter state mutator for each waiter + generateWaiterStateMutator(model, symbolProvider, writer, operation, name, waiter); + }); + } + + /** + * Generates interface to satisfy service client and invoke the relevant operation. + */ + private final void generateAPIClientInterface( + Model model, + SymbolProvider symbolProvider, + GoWriter writer, + OperationShape operationShape + ) { + StructureShape inputShape = model.expectShape( + operationShape.getInput().get(), StructureShape.class + ); + StructureShape outputShape = model.expectShape( + operationShape.getOutput().get(), StructureShape.class + ); + + Symbol operationSymbol = symbolProvider.toSymbol(operationShape); + Symbol inputSymbol = symbolProvider.toSymbol(inputShape); + Symbol outputSymbol = symbolProvider.toSymbol(outputShape); + + String interfaceName = generateAPIClientInterfaceName(operationSymbol); + + writer.write(""); + writer.writeDocs( + String.format("%s is a client that implements %s operation", interfaceName, operationSymbol.getName()) + ); + writer.openBlock("type $L interface {", "}", + interfaceName, () -> { + writer.addUseImports(SmithyGoDependency.CONTEXT); + writer.write( + "$L(context.Context, $P, ...func(*Options)) ($P, error)", + operationSymbol, inputSymbol, outputSymbol + ); + } + ); + writer.write(""); + } + + /** + * Generates waiter options to configure a waiter client. + */ + private final void generateWaiterOptions( + Model model, + SymbolProvider symbolProvider, + GoWriter writer, + OperationShape operationShape, + String waiterName + ) { + String optionsName = generateWaiterOptionsName(waiterName); + String waiterClientName = generateWaiterClientName(waiterName); + + writer.write(""); + writer.writeDocs( + String.format("%s are waiter options for %s", optionsName, waiterClientName) + ); + + writer.openBlock("type $L struct {", "}", + optionsName, () -> { + writer.addUseImports(SmithyGoDependency.TIME); + + writer.write(""); + writer.writeDocs("MinDelay is the minimum amount of time to delay between retries in seconds"); + writer.write("MinDelay time.Duration"); + + writer.write(""); + writer.writeDocs("MaxDelay is the maximum amount of time to delay between retries in seconds"); + writer.write("MaxDelay time.Duration"); + + writer.write(""); + writer.writeDocs("MaxWaitTime is the maximum amount of wait time in seconds before a waiter is " + + "forced to return"); + writer.write("MaxWaitTime time.Duration"); + + writer.write(""); + writer.writeDocs("MaxAttempts is the maximum number of attempts to fetch a terminal waiter state"); + writer.write("MaxAttempts int64"); + + writer.write(""); + writer.writeDocs("EnableLogger is used to enable logging for waiter retry attempts"); + writer.write("EnableLogger bool"); + + writer.write(""); + writer.writeDocs( + "WaiterStateMutator is mutator function option that can be used to override the " + + "service defined waiter-behavior based on operation output, or returned error. " + + "The mutator function is used by the waiter to decide if a state is retryable " + + "or a terminal state.\n\nBy default service-modeled waiter state mutators " + + "will populate this option. This option can thus be used to define a custom waiter " + + "state with fall-back to service-modeled waiter state mutators." + ); + writer.write( + "WaiterStateMutator func(context.Context, *OperationInput, *OperationOutput, error) " + + "(bool, error)"); + } + ); + writer.write(""); + } + + + /** + * Generates waiter client used to invoke waiter function. The waiter client is specific to a modeled waiter. + * Each waiter client is unique within a enclosure of a service. + * This function also generates a waiter client constructor that takes in a API client interface, and waiter options + * to configure a waiter client. + */ + private final void generateWaiterClient( + Model model, + SymbolProvider symbolProvider, + GoWriter writer, + OperationShape operationShape, + String waiterName, + Waiter waiter + ) { + StructureShape inputShape = model.expectShape( + operationShape.getInput().get(), StructureShape.class + ); + + Symbol operationSymbol = symbolProvider.toSymbol(operationShape); + String clientName = generateWaiterClientName(waiterName); + + writer.write(""); + writer.writeDocs( + String.format("%s defines the waiters for %s", clientName, waiterName) + ); + writer.openBlock("type $L struct {", "}", + clientName, () -> { + writer.write(""); + writer.write("client $L", generateAPIClientInterfaceName(operationSymbol)); + + writer.write(""); + writer.write("options $L", generateWaiterOptionsName(waiterName)); + }); + + writer.write(""); + + String constructorName = String.format("New%s", clientName); + + Symbol waiterOptionsSymbol = SymbolUtils.createPointableSymbolBuilder( + generateWaiterOptionsName(waiterName) + ).build(); + + Symbol clientSymbol = SymbolUtils.createPointableSymbolBuilder( + clientName + ).build(); + + writer.writeDocs( + String.format("%s constructs a %s.", constructorName, clientName) + ); + writer.openBlock("func $L(client $L, optFns ...func($P)) ($P, error) {", + constructorName, waiterOptionsSymbol, clientSymbol, + "}", () -> { + writer.write("options := $T{}", waiterOptionsSymbol); + writer.addUseImports(SmithyGoDependency.TIME); + + // set defaults + writer.write("options.MinDelay = $L * time.Second", waiter.getMinDelay()); + writer.write("options.MaxDelay = $L * time.Second", waiter.getMaxDelay()); + // set defaults not defined in model + writer.write("options.MaxWaitTime = $L * time.Second", DEFAULT_MAX_WAIT_TIME); + writer.write("options.MaxAttempts = $L ", DEFAULT_MAX_ATTEMPTS); + writer.write("options.WaiterStateMutator = $L", generateWaiterStateMutatorName(waiterName)); + writer.write(""); + + writer.openBlock("for _, fn := range optFns {", + "}", () -> { + writer.write("fn(&options)"); + }); + + writer.openBlock("return &$T {", "}", clientSymbol, () -> { + writer.write("client: client, "); + writer.write("options: options, "); + }); + }); + } + + /** + * Generates waiter invoker functions to call specific operation waiters + * These waiter invoker functions is defined on each modeled waiter client. + * The invoker function takes in a context, along with operation input, and + * optional functional options for the waiter. + */ + private final void generateWaiterInvoker( + Model model, + SymbolProvider symbolProvider, + GoWriter writer, + OperationShape operationShape, + String waiterName, + Waiter waiter + ) { + StructureShape inputShape = model.expectShape( + operationShape.getInput().get(), StructureShape.class + ); + + Symbol operationSymbol = symbolProvider.toSymbol(operationShape); + Symbol inputSymbol = symbolProvider.toSymbol(inputShape); + + Symbol waiterOptionsSymbol = SymbolUtils.createPointableSymbolBuilder( + generateWaiterOptionsName(waiterName) + ).build(); + + Symbol clientSymbol = SymbolUtils.createPointableSymbolBuilder( + generateWaiterClientName(waiterName) + ).build(); + + Symbol waiterMiddlewareSymbol = SymbolUtils.createValueSymbolBuilder( + WAITER_MIDDLEWARE_NAME, SmithyGoDependency.SMITHY_WAITERS + ).build(); + + writer.write(""); + writer.addUseImports(SmithyGoDependency.CONTEXT); + writer.writeDocs( + String.format("%s calls the waiter function for %s waiter", WAITER_INVOKER_FUNCTION_NAME, waiterName) + ); + writer.openBlock("func (w $P) $L(ctx context.Context, params $P, optFns ...func($P)) error {", "}", + clientSymbol, WAITER_INVOKER_FUNCTION_NAME, inputSymbol, waiterOptionsSymbol, + () -> { + writer.write("options := $T{}", waiterOptionsSymbol); + writer.write("options.MinDelay = w.options.MinDelay"); + writer.write("options.MaxDelay = w.options.MaxDelay"); + writer.write("options.MaxWaitTime = w.options.MaxWaitTime"); + writer.write("options.MaxAttempts = w.options.MaxAttempts"); + writer.write("options.EnableLogger = w.options.EnableLogger"); + writer.write("options.WaiterStateMutator = w.options.WaiterStateMutator"); + + writer.openBlock("for _, fn := range optFns {", + "}", () -> { + writer.write("fn(&options)"); + }); + + writer.openBlock("_, err := client.$L(ctx, params, func (o *Options) { ", "})", + operationSymbol, () -> { + writer.openBlock("o.APIOptions = append(o.APIOptions, func(stack *middleware.Stack) {", + "})", + () -> { + writer.openBlock("stack.Finalize.Add(&$L{", "}, middleware.Before)", + waiterMiddlewareSymbol, + () -> { + writer.write("MinDelay : options.MinDelay, "); + writer.write("MaxDelay : options.MaxDelay, "); + writer.write("MaxWaitTime : options.MaxWaitTime, "); + writer.write("MaxAttempts : options.MaxAttempts, "); + writer.write("EnableLogger : options.EnableLogger, "); + writer.write( + "WaiterStateMutator : options.WaiterStateMutator, "); + writer.addUseImports(SmithyGoDependency.SMITHY_HTTP_TRANSPORT); + writer.write("RequestCloner : http.RequestCloner "); + }); + + }); + }); + writer.write(""); + + writer.addUseImports(SmithyGoDependency.FMT); + writer.write("if err != nil { return fmt.Errorf(\"$L errored with error %w\", err) }", + clientSymbol); + writer.write("return nil"); + }); + } + + /** + * Generates a waiter state mutator function which is used by the WaiterRetrier Middleware to mutate + * waiter state as per the defined logic and returned operation response. + * + * @param model the smithy model + * @param symbolProvider symbol provider + * @param writer the Gowriter + * @param operationShape operation shape on which the waiter is modeled + * @param waiterName the waiter name + * @param waiter the waiter structure that contains info on modeled waiter + */ + private final void generateWaiterStateMutator( + Model model, + SymbolProvider symbolProvider, + GoWriter writer, + OperationShape operationShape, + String waiterName, + Waiter waiter + ) { + StructureShape inputShape = model.expectShape( + operationShape.getInput().get(), StructureShape.class + ); + StructureShape outputShape = model.expectShape( + operationShape.getOutput().get(), StructureShape.class + ); + + Symbol inputSymbol = symbolProvider.toSymbol(inputShape); + Symbol outputSymbol = symbolProvider.toSymbol(outputShape); + + writer.openBlock("func $L(ctx context.Context, input $P, output $P, err error) (bool, error) {", + generateWaiterStateMutatorName(waiterName), inputSymbol, outputSymbol, () -> { + waiter.getAcceptors().forEach(acceptor -> { + Matcher matcher = acceptor.getMatcher(); + switch (matcher.getMemberName()) { + case "output": + writer.write(""); + writer.addUseImports(SmithyGoDependency.GO_JMESPATH); + writer.addUseImports(SmithyGoDependency.FMT); + + writer.write("if err != nil { return true, nil }"); + + Matcher.OutputMember outputMember = (Matcher.OutputMember) matcher; + String path = outputMember.getValue().getPath(); + String expectedValue = outputMember.getValue().getExpected(); + PathComparator comparator = outputMember.getValue().getComparator(); + + writer.write("pathValue, err := jmespath.Search($L, output)", path); + writer.openBlock("if err != nil {", "}", () -> { + writer.write( + "return false, fmt.Errorf(\"error evaluating waiter state: %w\", err)"); + }).write(""); + writeWaiterComparator(writer, acceptor, comparator, "pathValue", expectedValue); + break; + + case "inputOutput": + writer.write(""); + writer.addUseImports(SmithyGoDependency.GO_JMESPATH); + writer.addUseImports(SmithyGoDependency.FMT); + + writer.write("if err != nil { return true, nil }"); + + Matcher.InputOutputMember ioMember = (Matcher.InputOutputMember) matcher; + path = ioMember.getValue().getPath(); + expectedValue = ioMember.getValue().getExpected(); + comparator = ioMember.getValue().getComparator(); + + writer.openBlock("type wrapper struct {", "}", () -> { + writer.write("Input $P", inputSymbol); + writer.write("Output $P", outputSymbol); + }); + + writer.write("pathValue, err := jmespath.Search($L, &wrapper{ " + + "Input: input,\n Output: output,\n })", path); + writer.openBlock("if err != nil {", "}", () -> { + writer.write( + "return false, fmt.Errorf(\"error evaluating waiter state: %w\", err)\")"); + }); + writer.write(""); + writeWaiterComparator(writer, acceptor, comparator, "pathValue", expectedValue); + break; + + case "success": + writer.write(""); + + Matcher.SuccessMember successMember = (Matcher.SuccessMember) matcher; + + writer.openBlock("if err != nil {", "}", + () -> { + writer.write("return true, nil"); + }); + + writeMatchedAcceptorReturn(writer, acceptor); + writer.write(""); + break; + + case "errorType": + writer.write(""); + writer.write("if err == nil { return true, nil }"); + + Matcher.ErrorTypeMember errorTypeMember = (Matcher.ErrorTypeMember) matcher; + String errorType = errorTypeMember.getValue(); + + Optional errorShape = operationShape.getErrors().stream().filter(shapeId -> { + return shapeId.getName().equalsIgnoreCase(errorType); + }).findFirst(); + + writer.write("var modeledError bool"); + if (errorShape.isPresent()) { + Symbol modeledErrorSymbol = SymbolUtils.createValueSymbolBuilder( + errorShape.get().getName() + ).build(); + writer.write("modeledError = errors.As(err, &$L{})", modeledErrorSymbol); + } + + writer.addUseImports(SmithyGoDependency.STRINGS); + writer.openBlock("if strings.Contains(err.Error(), errorType) || modeledError {", "}", + () -> { + writeMatchedAcceptorReturn(writer, acceptor); + }); + + writer.write("return true, nil"); + writer.write(""); + break; + } + }); + }); + } + + /** + * writes comparators for a given waiter. The comparators are defined within the waiter acceptor. + * + * @param writer the Gowriter + * @param acceptor the waiter acceptor that defines the comparator and acceptor states + * @param comparator the comparator + * @param actual the variable carrying the actual value obtained. This may be computed via a jmespath expression or + * from operation response (success/failure) + * @param expected the variable carrying the expected value. This value is as per the modeled waiter. + */ + private final void writeWaiterComparator( + GoWriter writer, + Acceptor acceptor, + PathComparator comparator, + String actual, + String expected + ) { + switch (comparator) { + case STRING_EQUALS: + writer.addUseImports(SmithyGoDependency.STRINGS); + writer.openBlock("if Strings.EqualFold($L, $L) {", "}", actual, expected, () -> { + writeMatchedAcceptorReturn(writer, acceptor); + }); + writer.write("return true, nil"); + break; + + case BOOLEAN_EQUALS: + writer.addUseImports(SmithyGoDependency.STRCONV); + writer.write("bv, err := strconv.ParseBool($L)", expected); + writer.write( + "if err != nil { return false, fmt.Errorf(\"error parsing boolean from string %w\", err)}"); + writer.openBlock("if $L == bv {", "}", () -> { + writeMatchedAcceptorReturn(writer, acceptor); + }); + writer.write("return true, nil"); + break; + + case ALL_STRING_EQUALS: + writer.openBlock("for _, v := range actual {", "}", () -> { + writer.addUseImports(SmithyGoDependency.STRINGS); + writer.write("if !Strings.EqualFold($L, $L) { return true, nil }"); + }); + writer.write(""); + writeMatchedAcceptorReturn(writer, acceptor); + break; + + case ANY_STRING_EQUALS: + writer.openBlock("for _, v := range actual {", "}", () -> { + writer.addUseImports(SmithyGoDependency.STRINGS); + writer.openBlock("if Strings.EqualFold($L, $L) {", "}", () -> { + writeMatchedAcceptorReturn(writer, acceptor); + }); + }); + writer.write("return true, nil"); + break; + + default: + throw new CodegenException( + String.format("Found unknown waiter path comparator, %s", comparator.toString())); + } + + writer.write(""); + } + + + /** + * Writes return statement for state where a waiter's acceptor state is a match + * @param writer the Go writer + * @param acceptor the waiter acceptor who's state is used to write an appropriate return statement. + */ + private final void writeMatchedAcceptorReturn(GoWriter writer, Acceptor acceptor) { + switch (acceptor.getState()) { + case SUCCESS: + writer.write("return false, nil"); + break; + + case FAILURE: + writer.addUseImports(SmithyGoDependency.FMT); + writer.write("return false, fmt.Errorf(\"waiter state transitioned to Failure\")"); + break; + + case RETRY: + writer.write("return true, nil"); + break; + + default: + throw new CodegenException("unknown acceptor state defined for the waiter"); + } + } + + + private String generateAPIClientInterfaceName( + Symbol operationSymbol + ) { + return String.format("%sWaiterAPIClient", operationSymbol.getName()); + } + + private String generateWaiterOptionsName( + String waiterName + ) { + waiterName = StringUtils.capitalize(waiterName); + return String.format("%sWaiterOptions", waiterName); + } + + private String generateWaiterClientName( + String waiterName + ) { + waiterName = StringUtils.capitalize(waiterName); + return String.format("%sWaiter", waiterName); + } + + private String generateWaiterStateMutatorName( + String waiterName + ) { + waiterName = StringUtils.uncapitalize(waiterName); + return String.format("%sStateMutator", waiterName); + } + +} diff --git a/internal/waiters/waiter.go b/internal/waiters/waiter.go new file mode 100644 index 000000000..6810efc9a --- /dev/null +++ b/internal/waiters/waiter.go @@ -0,0 +1,154 @@ +package waiters + +import ( + "context" + "fmt" + "math" + "time" + + "github.com/awslabs/smithy-go/logging" + "github.com/awslabs/smithy-go/middleware" +) + +// WaiterRetrier represents waiter retrier middleware struct +type WaiterRetrier struct { + + // MinDelay is the minimum amount of time to delay between retries in seconds + MinDelay time.Duration + + // MaxDelay is the maximum amount of time to delay between retries in seconds. + MaxDelay time.Duration + + // MaxWaitTime is the maximum amount of wait time before a waiter is forced to return. + MaxWaitTime time.Duration + + // MaxAttempts is the maximum number of attempts to fetch a success/failure waiter state. + MaxAttempts int64 + + // Enable logger for logging waiter workflow + EnableLogger bool + + // WaiterStateMutator is mutator function option that can be used to override the + // service defined waiter-behavior based on operation output, or returned error. + // The mutator function is used by the waiter to decide if a state is retryable + // or a terminal state. + // + // This option is by default backfilled, to use service-modeled waiter state mutators. + // This option can thus be used to define a custom waiter state with fall-back to + // service-modeled waiter state mutators. + WaiterStateMutator func(ctx context.Context, input, output interface{}, err error) (bool, error) + + // RequestCloner function is a transport-agnositic accessor function that is intended to take in a request, + // and return a cloned request + RequestCloner func(interface{}) interface{} +} + +// ID is the waiterRetrier middleware identifier +func (*WaiterRetrier) ID() string { + return "WaiterRetrier" +} + +// HandleFinalize handles the finalize middleware step +func (m *WaiterRetrier) HandleFinalize( + ctx context.Context, in middleware.FinalizeInput, next middleware.FinalizeHandler, +) ( + out middleware.FinalizeOutput, metadata middleware.Metadata, err error, +) { + // fetch logger + logger := middleware.GetLogger(ctx) + + // current attempt, delay + var attempt int64 + var delay time.Duration + var remainingTime = m.MaxWaitTime + + for { + + // check number of attempts + if m.MaxAttempts == attempt { + break + } + + attempt++ + + attemptInput := in + attemptInput.Request = m.RequestCloner(attemptInput.Request) + + if attempt > 1 { + // compute exponential backoff between retries + delay = computeDelay(m.MinDelay, m.MaxDelay, remainingTime, attempt) + // update the remaining time + remainingTime = remainingTime - delay + + // rewind transport stream + if rewindable, ok := in.Request.(interface{ RewindStream() error }); ok { + if err := rewindable.RewindStream(); err != nil { + return out, metadata, fmt.Errorf("failed to rewind transport stream for retry, %w", err) + } + } + + // sleep for the delay amount before invoking a request + if err = sleepWithContext(ctx, delay); err != nil { + return out, metadata, fmt.Errorf("request cancelled while waiting, %w", err) + } + + // log retry attempt + if m.EnableLogger { + logger.Logf(logging.Debug, fmt.Sprintf("retrying waiter request, attempt count: %d", attempt)) + } + } + + // attempt request + out, metadata, err = next.HandleFinalize(ctx, attemptInput) + + // check for state mutation + retryable, err := m.WaiterStateMutator(ctx, attemptInput, out, err) + if !retryable || err != nil { + if m.EnableLogger { + if err != nil { + logger.Logf(logging.Debug, "waiter transitioned to a failed state with unretryable error %v", err) + } else { + logger.Logf(logging.Debug, "waiter transitioned to a success state") + } + } + return out, metadata, err + } + } + + if m.EnableLogger { + logger.Logf(logging.Debug, "max retry attempts exhausted, max %d", attempt) + } + return out, metadata, fmt.Errorf("exhausted all retry attempts while waiting for the resource state") +} + +// computeDelay will compute delay between waiter attempts using the min delay, max delay, remaining time and +// current attempt count. Will return a delay value in seconds. +func computeDelay(minDelay, maxDelay, remainingTime time.Duration, attempt int64) time.Duration { + delay := time.Duration(math.Min( + maxDelay.Seconds(), + minDelay.Seconds()*math.Pow(2, float64(attempt-1))), + ) * time.Second + + if remainingTime-delay <= minDelay { + delay = remainingTime - minDelay + } + + return delay +} + +// sleepWithContext will wait for the timer duration to expire, or the context +// is canceled. Which ever happens first. If the context is canceled the +// Context's error will be returned. +func sleepWithContext(ctx context.Context, dur time.Duration) error { + t := time.NewTimer(dur) + defer t.Stop() + + select { + case <-t.C: + break + case <-ctx.Done(): + return ctx.Err() + } + + return nil +} From 5a80be15e75c908d0ec827017c36957c07602489 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Tue, 1 Dec 2020 11:31:20 -0800 Subject: [PATCH 02/23] fix dep import, codegen and code styling issues --- codegen/smithy-go-codegen/build.gradle.kts | 1 + .../go/codegen/integration/Waiters.java | 60 ++++++++++--------- ...mithy.go.codegen.integration.GoIntegration | 1 + {internal/waiters => waiters}/waiter.go | 0 4 files changed, 33 insertions(+), 29 deletions(-) rename {internal/waiters => waiters}/waiter.go (100%) diff --git a/codegen/smithy-go-codegen/build.gradle.kts b/codegen/smithy-go-codegen/build.gradle.kts index 1d472d60a..dcea37619 100644 --- a/codegen/smithy-go-codegen/build.gradle.kts +++ b/codegen/smithy-go-codegen/build.gradle.kts @@ -19,6 +19,7 @@ extra["moduleName"] = "software.amazon.smithy.go.codegen" dependencies { api("software.amazon.smithy:smithy-codegen-core:[1.3.0,2.0.0[") + implementation("software.amazon.smithy:smithy-waiters:[1.4.0,2.0.0[") compile("com.atlassian.commonmark:commonmark:0.15.2") api("org.jsoup:jsoup:1.13.1") implementation("software.amazon.smithy:smithy-protocol-test-traits:[1.3.0,2.0.0[") diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java index 85a965b3a..69d0f6d30 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java @@ -84,7 +84,7 @@ private void generateOperationWaiter( Map waiters ) { // write client interface - generateAPIClientInterface(model, symbolProvider, writer, operation); + generateApiClientInterface(model, symbolProvider, writer, operation); // generate waiter function waiters.forEach((name, waiter) -> { @@ -105,7 +105,7 @@ private void generateOperationWaiter( /** * Generates interface to satisfy service client and invoke the relevant operation. */ - private final void generateAPIClientInterface( + private void generateApiClientInterface( Model model, SymbolProvider symbolProvider, GoWriter writer, @@ -122,7 +122,7 @@ private final void generateAPIClientInterface( Symbol inputSymbol = symbolProvider.toSymbol(inputShape); Symbol outputSymbol = symbolProvider.toSymbol(outputShape); - String interfaceName = generateAPIClientInterfaceName(operationSymbol); + String interfaceName = generateApiClientInterfaceName(operationSymbol); writer.write(""); writer.writeDocs( @@ -132,7 +132,7 @@ private final void generateAPIClientInterface( interfaceName, () -> { writer.addUseImports(SmithyGoDependency.CONTEXT); writer.write( - "$L(context.Context, $P, ...func(*Options)) ($P, error)", + "$T(context.Context, $P, ...func(*Options)) ($P, error)", operationSymbol, inputSymbol, outputSymbol ); } @@ -143,7 +143,7 @@ private final void generateAPIClientInterface( /** * Generates waiter options to configure a waiter client. */ - private final void generateWaiterOptions( + private void generateWaiterOptions( Model model, SymbolProvider symbolProvider, GoWriter writer, @@ -171,8 +171,8 @@ private final void generateWaiterOptions( writer.write("MaxDelay time.Duration"); writer.write(""); - writer.writeDocs("MaxWaitTime is the maximum amount of wait time in seconds before a waiter is " + - "forced to return"); + writer.writeDocs("MaxWaitTime is the maximum amount of wait time in seconds before a waiter is " + + "forced to return"); writer.write("MaxWaitTime time.Duration"); writer.write(""); @@ -189,8 +189,8 @@ private final void generateWaiterOptions( + "service defined waiter-behavior based on operation output, or returned error. " + "The mutator function is used by the waiter to decide if a state is retryable " + "or a terminal state.\n\nBy default service-modeled waiter state mutators " - + "will populate this option. This option can thus be used to define a custom waiter " - + "state with fall-back to service-modeled waiter state mutators." + + "will populate this option. This option can thus be used to define a custom " + + "waiter state with fall-back to service-modeled waiter state mutators." ); writer.write( "WaiterStateMutator func(context.Context, *OperationInput, *OperationOutput, error) " @@ -207,7 +207,7 @@ private final void generateWaiterOptions( * This function also generates a waiter client constructor that takes in a API client interface, and waiter options * to configure a waiter client. */ - private final void generateWaiterClient( + private void generateWaiterClient( Model model, SymbolProvider symbolProvider, GoWriter writer, @@ -215,10 +215,6 @@ private final void generateWaiterClient( String waiterName, Waiter waiter ) { - StructureShape inputShape = model.expectShape( - operationShape.getInput().get(), StructureShape.class - ); - Symbol operationSymbol = symbolProvider.toSymbol(operationShape); String clientName = generateWaiterClientName(waiterName); @@ -229,7 +225,7 @@ private final void generateWaiterClient( writer.openBlock("type $L struct {", "}", clientName, () -> { writer.write(""); - writer.write("client $L", generateAPIClientInterfaceName(operationSymbol)); + writer.write("client $L", generateApiClientInterfaceName(operationSymbol)); writer.write(""); writer.write("options $L", generateWaiterOptionsName(waiterName)); @@ -250,9 +246,9 @@ private final void generateWaiterClient( writer.writeDocs( String.format("%s constructs a %s.", constructorName, clientName) ); - writer.openBlock("func $L(client $L, optFns ...func($P)) ($P, error) {", - constructorName, waiterOptionsSymbol, clientSymbol, - "}", () -> { + writer.openBlock("func $L(client $L, optFns ...func($P)) ($P, error) {", "}", + constructorName, generateApiClientInterfaceName(operationSymbol), waiterOptionsSymbol, clientSymbol, + () -> { writer.write("options := $T{}", waiterOptionsSymbol); writer.addUseImports(SmithyGoDependency.TIME); @@ -283,7 +279,7 @@ private final void generateWaiterClient( * The invoker function takes in a context, along with operation input, and * optional functional options for the waiter. */ - private final void generateWaiterInvoker( + private void generateWaiterInvoker( Model model, SymbolProvider symbolProvider, GoWriter writer, @@ -336,7 +332,7 @@ private final void generateWaiterInvoker( writer.openBlock("o.APIOptions = append(o.APIOptions, func(stack *middleware.Stack) {", "})", () -> { - writer.openBlock("stack.Finalize.Add(&$L{", "}, middleware.Before)", + writer.openBlock("stack.Finalize.Add(&$T{", "}, middleware.Before)", waiterMiddlewareSymbol, () -> { writer.write("MinDelay : options.MinDelay, "); @@ -347,7 +343,7 @@ private final void generateWaiterInvoker( writer.write( "WaiterStateMutator : options.WaiterStateMutator, "); writer.addUseImports(SmithyGoDependency.SMITHY_HTTP_TRANSPORT); - writer.write("RequestCloner : http.RequestCloner "); + writer.write("RequestCloner : http.RequestCloner, "); }); }); @@ -372,7 +368,7 @@ private final void generateWaiterInvoker( * @param waiterName the waiter name * @param waiter the waiter structure that contains info on modeled waiter */ - private final void generateWaiterStateMutator( + private void generateWaiterStateMutator( Model model, SymbolProvider symbolProvider, GoWriter writer, @@ -391,7 +387,7 @@ private final void generateWaiterStateMutator( Symbol outputSymbol = symbolProvider.toSymbol(outputShape); writer.openBlock("func $L(ctx context.Context, input $P, output $P, err error) (bool, error) {", - generateWaiterStateMutatorName(waiterName), inputSymbol, outputSymbol, () -> { + "}", generateWaiterStateMutatorName(waiterName), inputSymbol, outputSymbol, () -> { waiter.getAcceptors().forEach(acceptor -> { Matcher matcher = acceptor.getMatcher(); switch (matcher.getMemberName()) { @@ -432,8 +428,8 @@ private final void generateWaiterStateMutator( writer.write("Output $P", outputSymbol); }); - writer.write("pathValue, err := jmespath.Search($L, &wrapper{ " + - "Input: input,\n Output: output,\n })", path); + writer.write("pathValue, err := jmespath.Search($L, &wrapper{ " + + "Input: input,\n Output: output,\n })", path); writer.openBlock("if err != nil {", "}", () -> { writer.write( "return false, fmt.Errorf(\"error evaluating waiter state: %w\", err)\")"); @@ -484,6 +480,11 @@ private final void generateWaiterStateMutator( writer.write("return true, nil"); writer.write(""); break; + + default: + throw new CodegenException( + String.format("Unknown waiter state : %v", matcher.getMemberName()) + ); } }); }); @@ -499,7 +500,7 @@ private final void generateWaiterStateMutator( * from operation response (success/failure) * @param expected the variable carrying the expected value. This value is as per the modeled waiter. */ - private final void writeWaiterComparator( + private void writeWaiterComparator( GoWriter writer, Acceptor acceptor, PathComparator comparator, @@ -555,11 +556,12 @@ private final void writeWaiterComparator( /** - * Writes return statement for state where a waiter's acceptor state is a match + * Writes return statement for state where a waiter's acceptor state is a match. + * * @param writer the Go writer * @param acceptor the waiter acceptor who's state is used to write an appropriate return statement. */ - private final void writeMatchedAcceptorReturn(GoWriter writer, Acceptor acceptor) { + private void writeMatchedAcceptorReturn(GoWriter writer, Acceptor acceptor) { switch (acceptor.getState()) { case SUCCESS: writer.write("return false, nil"); @@ -580,7 +582,7 @@ private final void writeMatchedAcceptorReturn(GoWriter writer, Acceptor acceptor } - private String generateAPIClientInterfaceName( + private String generateApiClientInterfaceName( Symbol operationSymbol ) { return String.format("%sWaiterAPIClient", operationSymbol.getName()); diff --git a/codegen/smithy-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration b/codegen/smithy-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration index 1c670e6da..aa4002376 100644 --- a/codegen/smithy-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration +++ b/codegen/smithy-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration @@ -5,3 +5,4 @@ software.amazon.smithy.go.codegen.integration.RequiresLengthTraitSupport software.amazon.smithy.go.codegen.integration.EndpointHostPrefixMiddleware software.amazon.smithy.go.codegen.integration.ClientLogger software.amazon.smithy.go.codegen.integration.Paginators +software.amazon.smithy.go.codegen.integration.Waiters diff --git a/internal/waiters/waiter.go b/waiters/waiter.go similarity index 100% rename from internal/waiters/waiter.go rename to waiters/waiter.go From 30a4e83a760b2d5d666d88bde507fa1d6f7d9c57 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Tue, 1 Dec 2020 14:44:25 -0800 Subject: [PATCH 03/23] additional waiter changes and fixes to support mutliple acceptors --- .../smithy-go-codegen-test/model/main.smithy | 44 ++++ .../smithy/go/codegen/SmithyGoDependency.java | 4 +- .../go/codegen/integration/Waiters.java | 214 ++++++++++-------- 3 files changed, 169 insertions(+), 93 deletions(-) diff --git a/codegen/smithy-go-codegen-test/model/main.smithy b/codegen/smithy-go-codegen-test/model/main.smithy index 7f1952135..85e6d239f 100644 --- a/codegen/smithy-go-codegen-test/model/main.smithy +++ b/codegen/smithy-go-codegen-test/model/main.smithy @@ -3,6 +3,7 @@ namespace example.weather use smithy.test#httpRequestTests use smithy.test#httpResponseTests +use smithy.waiters#waitable /// Provides weather forecasts. @fakeProtocol @@ -36,6 +37,49 @@ resource CityImage { string CityId @readonly +@waitable( + CityExists: { + description: "Waits until a city has been created", + acceptors: [ + // Fail-fast if the thing transitions to a "failed" state. + { + state: "failure", + matcher: { + errorType: "NoSuchResource" + } + }, + // Succeed when the city image value is not empty i.e. enters into a "success" state. + { + state: "success", + matcher: { + success: true + } + }, + // Retry if city id input is of same length as city name in output + { + state: "retry", + matcher: { + inputOutput: { + path: "length(input.cityId) == length(output.name)", + comparator: "booleanEquals", + expected: "true", + } + } + }, + // Success if city name in output is seattle + { + state: "success", + matcher: { + output: { + path: "name", + comparator: "stringEquals", + expected: "seattle", + } + } + } + ] + } +) @http(method: "GET", uri: "/cities/{cityId}") operation GetCity { input: GetCityInput, diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java index 05fb81551..c79dcc02c 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java @@ -58,7 +58,7 @@ public final class SmithyGoDependency { public static final GoDependency GO_CMP = goCmp("cmp"); public static final GoDependency GO_CMP_OPTIONS = goCmp("cmp/cmpopts"); - public static final GoDependency GO_JMESPATH = goJmespath(""); + public static final GoDependency GO_JMESPATH = goJmespath(null); private static final String SMITHY_SOURCE_PATH = "github.com/awslabs/smithy-go"; private static final String GO_CMP_SOURCE_PATH = "github.com/google/go-cmp"; @@ -118,7 +118,7 @@ private static GoDependency relativePackage( private static final class Versions { private static final String GO_STDLIB = "1.15"; private static final String GO_CMP = "v0.5.4"; - private static final String SMITHY_GO = "v0.4.0"; + private static final String SMITHY_GO = "v0.4.1-0.20201201204143-5a80be15e75c"; private static final String GO_JMESPATH = "v0.4.0"; } } diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java index 69d0f6d30..c75a89b8f 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java @@ -193,7 +193,7 @@ private void generateWaiterOptions( + "waiter state with fall-back to service-modeled waiter state mutators." ); writer.write( - "WaiterStateMutator func(context.Context, *OperationInput, *OperationOutput, error) " + "WaiterStateMutator func(context.Context, interface{}, interface{}, error) " + "(bool, error)"); } ); @@ -246,7 +246,7 @@ private void generateWaiterClient( writer.writeDocs( String.format("%s constructs a %s.", constructorName, clientName) ); - writer.openBlock("func $L(client $L, optFns ...func($P)) ($P, error) {", "}", + writer.openBlock("func $L(client $L, optFns ...func($P)) $P {", "}", constructorName, generateApiClientInterfaceName(operationSymbol), waiterOptionsSymbol, clientSymbol, () -> { writer.write("options := $T{}", waiterOptionsSymbol); @@ -327,12 +327,12 @@ private void generateWaiterInvoker( writer.write("fn(&options)"); }); - writer.openBlock("_, err := client.$L(ctx, params, func (o *Options) { ", "})", + writer.openBlock("_, err := w.client.$T(ctx, params, func (o *Options) { ", "})", operationSymbol, () -> { - writer.openBlock("o.APIOptions = append(o.APIOptions, func(stack *middleware.Stack) {", - "})", + writer.openBlock("o.APIOptions = append(o.APIOptions, " + + "func(stack *middleware.Stack) error {", "})", () -> { - writer.openBlock("stack.Finalize.Add(&$T{", "}, middleware.Before)", + writer.openBlock("return stack.Finalize.Add(&$T{", "}, middleware.Before)", waiterMiddlewareSymbol, () -> { writer.write("MinDelay : options.MinDelay, "); @@ -343,7 +343,7 @@ private void generateWaiterInvoker( writer.write( "WaiterStateMutator : options.WaiterStateMutator, "); writer.addUseImports(SmithyGoDependency.SMITHY_HTTP_TRANSPORT); - writer.write("RequestCloner : http.RequestCloner, "); + writer.write("RequestCloner : smithyhttp.RequestCloner, "); }); }); @@ -361,12 +361,12 @@ private void generateWaiterInvoker( * Generates a waiter state mutator function which is used by the WaiterRetrier Middleware to mutate * waiter state as per the defined logic and returned operation response. * - * @param model the smithy model + * @param model the smithy model * @param symbolProvider symbol provider - * @param writer the Gowriter + * @param writer the Gowriter * @param operationShape operation shape on which the waiter is modeled - * @param waiterName the waiter name - * @param waiter the waiter structure that contains info on modeled waiter + * @param waiterName the waiter name + * @param waiter the waiter structure that contains info on modeled waiter */ private void generateWaiterStateMutator( Model model, @@ -386,8 +386,32 @@ private void generateWaiterStateMutator( Symbol inputSymbol = symbolProvider.toSymbol(inputShape); Symbol outputSymbol = symbolProvider.toSymbol(outputShape); - writer.openBlock("func $L(ctx context.Context, input $P, output $P, err error) (bool, error) {", - "}", generateWaiterStateMutatorName(waiterName), inputSymbol, outputSymbol, () -> { + writer.openBlock("func $L(ctx context.Context, inputIface, outputIface interface{}, err error) (bool, error) {", + "}", generateWaiterStateMutatorName(waiterName), () -> { + + writer.write("input, ok := inputIface.($P)", inputSymbol); + writer.addUseImports(SmithyGoDependency.FMT); + writer.write("if !ok { return false, fmt.Errorf(\"expected input to be of type $P\")}", + inputSymbol + ); + writer.write("_ = input").write(""); + + writer.write("output, ok := outputIface.($P)", outputSymbol); + writer.addUseImports(SmithyGoDependency.FMT); + writer.write("if !ok { return false, fmt.Errorf(\"expected output to be of type $P\")}", + outputSymbol + ); + writer.write("_ = output").write(""); + + + writer.openBlock("type wrapper struct {", "}", () -> { + writer.write("Input $P", inputSymbol); + writer.write("Output $P", outputSymbol); + }); + + writer.write("wrappedIO := &wrapper{ Input: input,\n Output: output,\n}"); + writer.write("_ = wrappedIO"); + waiter.getAcceptors().forEach(acceptor -> { Matcher matcher = acceptor.getMatcher(); switch (matcher.getMemberName()) { @@ -396,19 +420,22 @@ private void generateWaiterStateMutator( writer.addUseImports(SmithyGoDependency.GO_JMESPATH); writer.addUseImports(SmithyGoDependency.FMT); - writer.write("if err != nil { return true, nil }"); - - Matcher.OutputMember outputMember = (Matcher.OutputMember) matcher; - String path = outputMember.getValue().getPath(); - String expectedValue = outputMember.getValue().getExpected(); - PathComparator comparator = outputMember.getValue().getComparator(); - - writer.write("pathValue, err := jmespath.Search($L, output)", path); - writer.openBlock("if err != nil {", "}", () -> { - writer.write( - "return false, fmt.Errorf(\"error evaluating waiter state: %w\", err)"); - }).write(""); - writeWaiterComparator(writer, acceptor, comparator, "pathValue", expectedValue); + writer.openBlock("{", "}", () -> { + writer.write("if err != nil { return true, nil }"); + + Matcher.OutputMember outputMember = (Matcher.OutputMember) matcher; + String path = outputMember.getValue().getPath(); + String expectedValue = outputMember.getValue().getExpected(); + PathComparator comparator = outputMember.getValue().getComparator(); + + writer.write("pathValue, err := jmespath.Search($S, output)", path); + writer.openBlock("if err != nil {", "}", () -> { + writer.write( + "return false, fmt.Errorf(\"error evaluating waiter state: %w\", err)"); + }).write(""); + writer.write("expectedValue := $S", expectedValue); + writeWaiterComparator(writer, acceptor, comparator, "pathValue", "expectedValue"); + }); break; case "inputOutput": @@ -416,74 +443,81 @@ private void generateWaiterStateMutator( writer.addUseImports(SmithyGoDependency.GO_JMESPATH); writer.addUseImports(SmithyGoDependency.FMT); - writer.write("if err != nil { return true, nil }"); - - Matcher.InputOutputMember ioMember = (Matcher.InputOutputMember) matcher; - path = ioMember.getValue().getPath(); - expectedValue = ioMember.getValue().getExpected(); - comparator = ioMember.getValue().getComparator(); - - writer.openBlock("type wrapper struct {", "}", () -> { - writer.write("Input $P", inputSymbol); - writer.write("Output $P", outputSymbol); - }); - - writer.write("pathValue, err := jmespath.Search($L, &wrapper{ " - + "Input: input,\n Output: output,\n })", path); - writer.openBlock("if err != nil {", "}", () -> { - writer.write( - "return false, fmt.Errorf(\"error evaluating waiter state: %w\", err)\")"); + writer.openBlock("{", "}", () -> { + writer.write("if err != nil { return true, nil }"); + + Matcher.InputOutputMember ioMember = (Matcher.InputOutputMember) matcher; + String path = ioMember.getValue().getPath(); + String expectedValue = ioMember.getValue().getExpected(); + PathComparator comparator = ioMember.getValue().getComparator(); + + writer.write("pathValue, err := jmespath.Search($S, wrappedIO)", path); + writer.openBlock("if err != nil {", "}", () -> { + writer.write( + "return false, fmt.Errorf(\"error evaluating waiter state: %w\", err)"); + }); + writer.write(""); + writer.write("expectedValue := $S", expectedValue); + writeWaiterComparator(writer, acceptor, comparator, "pathValue", "expectedValue"); }); - writer.write(""); - writeWaiterComparator(writer, acceptor, comparator, "pathValue", expectedValue); break; case "success": writer.write(""); Matcher.SuccessMember successMember = (Matcher.SuccessMember) matcher; + writer.openBlock("{", "}", () -> { + writer.openBlock("if err != nil {", "}", + () -> { + writer.write("return true, nil"); + }); - writer.openBlock("if err != nil {", "}", - () -> { - writer.write("return true, nil"); - }); - - writeMatchedAcceptorReturn(writer, acceptor); - writer.write(""); + writeMatchedAcceptorReturn(writer, acceptor); + }); break; case "errorType": - writer.write(""); writer.write("if err == nil { return true, nil }"); + writer.write(""); - Matcher.ErrorTypeMember errorTypeMember = (Matcher.ErrorTypeMember) matcher; - String errorType = errorTypeMember.getValue(); - - Optional errorShape = operationShape.getErrors().stream().filter(shapeId -> { - return shapeId.getName().equalsIgnoreCase(errorType); - }).findFirst(); - - writer.write("var modeledError bool"); - if (errorShape.isPresent()) { - Symbol modeledErrorSymbol = SymbolUtils.createValueSymbolBuilder( - errorShape.get().getName() - ).build(); - writer.write("modeledError = errors.As(err, &$L{})", modeledErrorSymbol); - } - - writer.addUseImports(SmithyGoDependency.STRINGS); - writer.openBlock("if strings.Contains(err.Error(), errorType) || modeledError {", "}", - () -> { - writeMatchedAcceptorReturn(writer, acceptor); - }); - - writer.write("return true, nil"); + writer.openBlock("{", "}", () -> { + Matcher.ErrorTypeMember errorTypeMember = (Matcher.ErrorTypeMember) matcher; + String errorType = errorTypeMember.getValue(); + + // identify if this is a modeled error shape + Optional errorShape = operationShape.getErrors().stream().filter( + shapeId -> { + return shapeId.getName().equalsIgnoreCase(errorType); + }).findFirst(); + + // if modeled error shape + if (errorShape.isPresent()) { + Symbol modeledErrorSymbol = SymbolUtils.createValueSymbolBuilder( + errorShape.get().getName(), "types" + ).build(); + writer.addUseImports(SmithyGoDependency.ERRORS); + writer.write("var errorType *$T", modeledErrorSymbol); + writer.openBlock("if errors.As(err, &errorType) {", "}", + () -> { + writeMatchedAcceptorReturn(writer, acceptor); + }); + } else { + // fall back to un-modeled error shape matching + writer.addUseImports(SmithyGoDependency.STRINGS); + writer.openBlock("if strings.Contains(err.Error(), $S) {", + "}", errorType, () -> { + writeMatchedAcceptorReturn(writer, acceptor); + }); + } + + writer.write("return true, nil"); + }); writer.write(""); break; default: throw new CodegenException( - String.format("Unknown waiter state : %v", matcher.getMemberName()) + String.format("unknown waiter state : %v", matcher.getMemberName()) ); } }); @@ -493,12 +527,12 @@ private void generateWaiterStateMutator( /** * writes comparators for a given waiter. The comparators are defined within the waiter acceptor. * - * @param writer the Gowriter - * @param acceptor the waiter acceptor that defines the comparator and acceptor states + * @param writer the Gowriter + * @param acceptor the waiter acceptor that defines the comparator and acceptor states * @param comparator the comparator - * @param actual the variable carrying the actual value obtained. This may be computed via a jmespath expression or - * from operation response (success/failure) - * @param expected the variable carrying the expected value. This value is as per the modeled waiter. + * @param actual the variable carrying the actual value obtained. + * This may be computed via a jmespath expression or operation response status (success/failure) + * @param expected the variable carrying the expected value. This value is as per the modeled waiter. */ private void writeWaiterComparator( GoWriter writer, @@ -509,8 +543,7 @@ private void writeWaiterComparator( ) { switch (comparator) { case STRING_EQUALS: - writer.addUseImports(SmithyGoDependency.STRINGS); - writer.openBlock("if Strings.EqualFold($L, $L) {", "}", actual, expected, () -> { + writer.openBlock("if $L == $L {", "}", actual, expected, () -> { writeMatchedAcceptorReturn(writer, acceptor); }); writer.write("return true, nil"); @@ -521,7 +554,7 @@ private void writeWaiterComparator( writer.write("bv, err := strconv.ParseBool($L)", expected); writer.write( "if err != nil { return false, fmt.Errorf(\"error parsing boolean from string %w\", err)}"); - writer.openBlock("if $L == bv {", "}", () -> { + writer.openBlock("if $L == bv {", "}", actual, () -> { writeMatchedAcceptorReturn(writer, acceptor); }); writer.write("return true, nil"); @@ -529,8 +562,7 @@ private void writeWaiterComparator( case ALL_STRING_EQUALS: writer.openBlock("for _, v := range actual {", "}", () -> { - writer.addUseImports(SmithyGoDependency.STRINGS); - writer.write("if !Strings.EqualFold($L, $L) { return true, nil }"); + writer.write("if v != $L { return true, nil }", expected); }); writer.write(""); writeMatchedAcceptorReturn(writer, acceptor); @@ -538,10 +570,10 @@ private void writeWaiterComparator( case ANY_STRING_EQUALS: writer.openBlock("for _, v := range actual {", "}", () -> { - writer.addUseImports(SmithyGoDependency.STRINGS); - writer.openBlock("if Strings.EqualFold($L, $L) {", "}", () -> { - writeMatchedAcceptorReturn(writer, acceptor); - }); + writer.openBlock("if v == $L {", "}", + expected, () -> { + writeMatchedAcceptorReturn(writer, acceptor); + }); }); writer.write("return true, nil"); break; @@ -558,7 +590,7 @@ private void writeWaiterComparator( /** * Writes return statement for state where a waiter's acceptor state is a match. * - * @param writer the Go writer + * @param writer the Go writer * @param acceptor the waiter acceptor who's state is used to write an appropriate return statement. */ private void writeMatchedAcceptorReturn(GoWriter writer, Acceptor acceptor) { From 1ba08af0096d3b6b2e9b369c8b39b37fcb0037c6 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Tue, 1 Dec 2020 21:53:52 -0800 Subject: [PATCH 04/23] update waiter util to export utils and remove hand-written waiter middleware --- waiters/waiter.go | 123 ++-------------------------------------------- 1 file changed, 4 insertions(+), 119 deletions(-) diff --git a/waiters/waiter.go b/waiters/waiter.go index 6810efc9a..b4e0bcc50 100644 --- a/waiters/waiter.go +++ b/waiters/waiter.go @@ -2,128 +2,13 @@ package waiters import ( "context" - "fmt" "math" "time" - - "github.com/awslabs/smithy-go/logging" - "github.com/awslabs/smithy-go/middleware" ) -// WaiterRetrier represents waiter retrier middleware struct -type WaiterRetrier struct { - - // MinDelay is the minimum amount of time to delay between retries in seconds - MinDelay time.Duration - - // MaxDelay is the maximum amount of time to delay between retries in seconds. - MaxDelay time.Duration - - // MaxWaitTime is the maximum amount of wait time before a waiter is forced to return. - MaxWaitTime time.Duration - - // MaxAttempts is the maximum number of attempts to fetch a success/failure waiter state. - MaxAttempts int64 - - // Enable logger for logging waiter workflow - EnableLogger bool - - // WaiterStateMutator is mutator function option that can be used to override the - // service defined waiter-behavior based on operation output, or returned error. - // The mutator function is used by the waiter to decide if a state is retryable - // or a terminal state. - // - // This option is by default backfilled, to use service-modeled waiter state mutators. - // This option can thus be used to define a custom waiter state with fall-back to - // service-modeled waiter state mutators. - WaiterStateMutator func(ctx context.Context, input, output interface{}, err error) (bool, error) - - // RequestCloner function is a transport-agnositic accessor function that is intended to take in a request, - // and return a cloned request - RequestCloner func(interface{}) interface{} -} - -// ID is the waiterRetrier middleware identifier -func (*WaiterRetrier) ID() string { - return "WaiterRetrier" -} - -// HandleFinalize handles the finalize middleware step -func (m *WaiterRetrier) HandleFinalize( - ctx context.Context, in middleware.FinalizeInput, next middleware.FinalizeHandler, -) ( - out middleware.FinalizeOutput, metadata middleware.Metadata, err error, -) { - // fetch logger - logger := middleware.GetLogger(ctx) - - // current attempt, delay - var attempt int64 - var delay time.Duration - var remainingTime = m.MaxWaitTime - - for { - - // check number of attempts - if m.MaxAttempts == attempt { - break - } - - attempt++ - - attemptInput := in - attemptInput.Request = m.RequestCloner(attemptInput.Request) - - if attempt > 1 { - // compute exponential backoff between retries - delay = computeDelay(m.MinDelay, m.MaxDelay, remainingTime, attempt) - // update the remaining time - remainingTime = remainingTime - delay - - // rewind transport stream - if rewindable, ok := in.Request.(interface{ RewindStream() error }); ok { - if err := rewindable.RewindStream(); err != nil { - return out, metadata, fmt.Errorf("failed to rewind transport stream for retry, %w", err) - } - } - - // sleep for the delay amount before invoking a request - if err = sleepWithContext(ctx, delay); err != nil { - return out, metadata, fmt.Errorf("request cancelled while waiting, %w", err) - } - - // log retry attempt - if m.EnableLogger { - logger.Logf(logging.Debug, fmt.Sprintf("retrying waiter request, attempt count: %d", attempt)) - } - } - - // attempt request - out, metadata, err = next.HandleFinalize(ctx, attemptInput) - - // check for state mutation - retryable, err := m.WaiterStateMutator(ctx, attemptInput, out, err) - if !retryable || err != nil { - if m.EnableLogger { - if err != nil { - logger.Logf(logging.Debug, "waiter transitioned to a failed state with unretryable error %v", err) - } else { - logger.Logf(logging.Debug, "waiter transitioned to a success state") - } - } - return out, metadata, err - } - } - - if m.EnableLogger { - logger.Logf(logging.Debug, "max retry attempts exhausted, max %d", attempt) - } - return out, metadata, fmt.Errorf("exhausted all retry attempts while waiting for the resource state") -} - -// computeDelay will compute delay between waiter attempts using the min delay, max delay, remaining time and +// ComputeDelay will compute delay between waiter attempts using the min delay, max delay, remaining time and // current attempt count. Will return a delay value in seconds. -func computeDelay(minDelay, maxDelay, remainingTime time.Duration, attempt int64) time.Duration { +func ComputeDelay(minDelay, maxDelay, remainingTime time.Duration, attempt int64) time.Duration { delay := time.Duration(math.Min( maxDelay.Seconds(), minDelay.Seconds()*math.Pow(2, float64(attempt-1))), @@ -136,10 +21,10 @@ func computeDelay(minDelay, maxDelay, remainingTime time.Duration, attempt int64 return delay } -// sleepWithContext will wait for the timer duration to expire, or the context +// SleepWithContext will wait for the timer duration to expire, or the context // is canceled. Which ever happens first. If the context is canceled the // Context's error will be returned. -func sleepWithContext(ctx context.Context, dur time.Duration) error { +func SleepWithContext(ctx context.Context, dur time.Duration) error { t := time.NewTimer(dur) defer t.Stop() From 81faf8293077d6521a0c5b245577701f8fbc4cf1 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Tue, 1 Dec 2020 22:42:23 -0800 Subject: [PATCH 05/23] update codegen to generate waiter specific middleware --- .../GoStackStepMiddlewareGenerator.java | 17 + .../smithy/go/codegen/SmithyGoDependency.java | 2 +- .../go/codegen/integration/Waiters.java | 324 +++++++++++++----- 3 files changed, 257 insertions(+), 86 deletions(-) diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoStackStepMiddlewareGenerator.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoStackStepMiddlewareGenerator.java index a53816e99..af4d5326a 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoStackStepMiddlewareGenerator.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoStackStepMiddlewareGenerator.java @@ -114,6 +114,23 @@ public static GoStackStepMiddlewareGenerator createDeserializeStepMiddleware(Str .build()); } + /** + * Create a new Finalize step middleware generator with the provided type name. + * + * @param name is the type name to identify the middleware. + * @param id the unique ID for the middleware. + * @return the middleware generator. + */ + public static GoStackStepMiddlewareGenerator createFinalizeStepMiddleware(String name, MiddlewareIdentifier id) { + return createMiddleware(name, + id, + "HandleFinalize", + SymbolUtils.createValueSymbolBuilder("FinalizeInput", SmithyGoDependency.SMITHY_MIDDLEWARE).build(), + SymbolUtils.createValueSymbolBuilder("FinalizeOutput", SmithyGoDependency.SMITHY_MIDDLEWARE).build(), + SymbolUtils.createValueSymbolBuilder("FinalizeHandler", SmithyGoDependency.SMITHY_MIDDLEWARE) + .build()); + } + /** * Generates a new step middleware generator. * diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java index c79dcc02c..678467426 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java @@ -118,7 +118,7 @@ private static GoDependency relativePackage( private static final class Versions { private static final String GO_STDLIB = "1.15"; private static final String GO_CMP = "v0.5.4"; - private static final String SMITHY_GO = "v0.4.1-0.20201201204143-5a80be15e75c"; + private static final String SMITHY_GO = "v0.4.1-0.20201202055352-1ba08af0096d"; private static final String GO_JMESPATH = "v0.4.0"; } } diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java index c75a89b8f..fa3d611f4 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java @@ -22,7 +22,9 @@ 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.MiddlewareIdentifier; import software.amazon.smithy.go.codegen.SmithyGoDependency; import software.amazon.smithy.go.codegen.SymbolUtils; import software.amazon.smithy.model.Model; @@ -43,11 +45,10 @@ */ public class Waiters implements GoIntegration { private static final String WAITER_INVOKER_FUNCTION_NAME = "Wait"; - private static final String WAITER_MIDDLEWARE_NAME = "WaiterRetrier"; - private static final int DEFAULT_MAX_WAIT_TIME = 300; + private static final String WAITER_MIDDLEWARE_ID = "WaiterRetrier"; + private static final int DEFAULT_MAX_WAIT_TIME_IN_SECONDS = 300; private static final int DEFAULT_MAX_ATTEMPTS = 8; - @Override public void writeAdditionalFiles( GoSettings settings, @@ -99,6 +100,9 @@ private void generateOperationWaiter( // write waiter state mutator for each waiter generateWaiterStateMutator(model, symbolProvider, writer, operation, name, waiter); + + // write waiter middleware for each waiter + generateWaiterMiddleware(model, symbolProvider, writer, operation, name, waiter); }); } @@ -153,6 +157,16 @@ private void generateWaiterOptions( String optionsName = generateWaiterOptionsName(waiterName); String waiterClientName = generateWaiterClientName(waiterName); + StructureShape inputShape = model.expectShape( + operationShape.getInput().get(), StructureShape.class + ); + StructureShape outputShape = model.expectShape( + operationShape.getOutput().get(), StructureShape.class + ); + + Symbol inputSymbol = symbolProvider.toSymbol(inputShape); + Symbol outputSymbol = symbolProvider.toSymbol(outputShape); + writer.write(""); writer.writeDocs( String.format("%s are waiter options for %s", optionsName, waiterClientName) @@ -193,8 +207,8 @@ private void generateWaiterOptions( + "waiter state with fall-back to service-modeled waiter state mutators." ); writer.write( - "WaiterStateMutator func(context.Context, interface{}, interface{}, error) " - + "(bool, error)"); + "WaiterStateMutator func(context.Context, $P, $P, error) " + + "(bool, error)", inputSymbol, outputSymbol); } ); writer.write(""); @@ -256,7 +270,7 @@ constructorName, generateApiClientInterfaceName(operationSymbol), waiterOptionsS writer.write("options.MinDelay = $L * time.Second", waiter.getMinDelay()); writer.write("options.MaxDelay = $L * time.Second", waiter.getMaxDelay()); // set defaults not defined in model - writer.write("options.MaxWaitTime = $L * time.Second", DEFAULT_MAX_WAIT_TIME); + writer.write("options.MaxWaitTime = $L * time.Second", DEFAULT_MAX_WAIT_TIME_IN_SECONDS); writer.write("options.MaxAttempts = $L ", DEFAULT_MAX_ATTEMPTS); writer.write("options.WaiterStateMutator = $L", generateWaiterStateMutatorName(waiterName)); writer.write(""); @@ -302,10 +316,6 @@ private void generateWaiterInvoker( generateWaiterClientName(waiterName) ).build(); - Symbol waiterMiddlewareSymbol = SymbolUtils.createValueSymbolBuilder( - WAITER_MIDDLEWARE_NAME, SmithyGoDependency.SMITHY_WAITERS - ).build(); - writer.write(""); writer.addUseImports(SmithyGoDependency.CONTEXT); writer.writeDocs( @@ -332,18 +342,19 @@ private void generateWaiterInvoker( writer.openBlock("o.APIOptions = append(o.APIOptions, " + "func(stack *middleware.Stack) error {", "})", () -> { - writer.openBlock("return stack.Finalize.Add(&$T{", "}, middleware.Before)", - waiterMiddlewareSymbol, + writer.openBlock("return stack.Finalize.Add(&$L{", "}, middleware.Before)", + generateWaiterMiddlewareName(waiterName), () -> { - writer.write("MinDelay : options.MinDelay, "); - writer.write("MaxDelay : options.MaxDelay, "); - writer.write("MaxWaitTime : options.MaxWaitTime, "); - writer.write("MaxAttempts : options.MaxAttempts, "); - writer.write("EnableLogger : options.EnableLogger, "); + writer.write("minDelay : options.MinDelay, "); + writer.write("maxDelay : options.MaxDelay, "); + writer.write("maxWaitTime : options.MaxWaitTime, "); + writer.write("maxAttempts : options.MaxAttempts, "); + writer.write("enableLogger : options.EnableLogger, "); writer.write( - "WaiterStateMutator : options.WaiterStateMutator, "); + "waiterStateMutator : options.WaiterStateMutator, "); writer.addUseImports(SmithyGoDependency.SMITHY_HTTP_TRANSPORT); - writer.write("RequestCloner : smithyhttp.RequestCloner, "); + writer.write("requestCloner : smithyhttp.RequestCloner, "); + writer.write("params: params, "); }); }); @@ -358,17 +369,16 @@ private void generateWaiterInvoker( } /** - * Generates a waiter state mutator function which is used by the WaiterRetrier Middleware to mutate - * waiter state as per the defined logic and returned operation response. + * Generates a waiter retrier middleware to trigger waiter state validation and retry behavior. * * @param model the smithy model * @param symbolProvider symbol provider - * @param writer the Gowriter - * @param operationShape operation shape on which the waiter is modeled + * @param writer the GoWriter + * @param operationShape operation shape on which waiter is modeled * @param waiterName the waiter name * @param waiter the waiter structure that contains info on modeled waiter */ - private void generateWaiterStateMutator( + private void generateWaiterMiddleware( Model model, SymbolProvider symbolProvider, GoWriter writer, @@ -386,41 +396,179 @@ private void generateWaiterStateMutator( Symbol inputSymbol = symbolProvider.toSymbol(inputShape); Symbol outputSymbol = symbolProvider.toSymbol(outputShape); - writer.openBlock("func $L(ctx context.Context, inputIface, outputIface interface{}, err error) (bool, error) {", - "}", generateWaiterStateMutatorName(waiterName), () -> { + GoStackStepMiddlewareGenerator middlewareGenerator = + GoStackStepMiddlewareGenerator.createFinalizeStepMiddleware( + generateWaiterMiddlewareName(waiterName), + MiddlewareIdentifier.builder() + .name(WAITER_MIDDLEWARE_ID) + .build() + ); - writer.write("input, ok := inputIface.($P)", inputSymbol); - writer.addUseImports(SmithyGoDependency.FMT); - writer.write("if !ok { return false, fmt.Errorf(\"expected input to be of type $P\")}", - inputSymbol - ); - writer.write("_ = input").write(""); + writer.write(""); - writer.write("output, ok := outputIface.($P)", outputSymbol); - writer.addUseImports(SmithyGoDependency.FMT); - writer.write("if !ok { return false, fmt.Errorf(\"expected output to be of type $P\")}", - outputSymbol - ); - writer.write("_ = output").write(""); + middlewareGenerator.writeMiddleware(writer, (generator, w) -> { + w.write(""); + w.addUseImports(SmithyGoDependency.SMITHY_LOGGING); + w.writeDocs("fetch logger from context"); + w.write("logger := middleware.GetLogger(ctx)"); + w.write(""); + + w.writeDocs("current attempt, delay"); + w.write("var attempt int64"); + w.write("var delay time.Duration"); + w.write("var remainingTime = m.maxWaitTime"); + + // start loop + w.write("for {"); + + w.writeDocs("check number of attempts"); + w.write("if m.maxAttempts == attempt { break }"); + w.write(""); + w.write("attempt++"); + w.write(""); + + w.write("attemptInput := in"); + w.write("attemptInput.Request = m.requestCloner(attemptInput.Request)"); + + w.write(""); + + // start retry only behavior + w.write("if attempt > 1 {"); + + Symbol computeDelaySymbol = SymbolUtils.createValueSymbolBuilder( + "ComputeDelay", SmithyGoDependency.SMITHY_WAITERS + ).build(); + w.writeDocs("compute exponential backoff between waiter retries"); + w.write("delay = $T(m.minDelay, m.maxDelay, remainingTime, attempt)", computeDelaySymbol); + w.write(""); + + w.writeDocs("update the remaining time"); + w.write("remainingTime = remainingTime - delay"); + w.write(""); + + w.writeDocs("rewind transport stream"); + w.write("if rewindable, ok := in.Request.(interface{ RewindStream() error }); ok {"); + w.write("if err := rewindable.RewindStream(); err != nil {"); + w.addUseImports(SmithyGoDependency.FMT); + w.write("return out, metadata, fmt.Errorf(\"failed to rewind transport stream for retry, %w\", err)\n}"); + w.write("}"); + w.write(""); + + Symbol sleepWithContextSymbol = SymbolUtils.createValueSymbolBuilder( + "SleepWithContext", SmithyGoDependency.SMITHY_WAITERS + ).build(); + w.writeDocs("sleep for the delay amount before invoking a request"); + w.openBlock("if err = $T(ctx, delay); err != nil {", "}", + sleepWithContextSymbol, () -> { + w.write("return out, metadata, fmt.Errorf(\"request cancelled while waiting, %w\", err)"); + }); + // enable logger + w.writeDocs("log attempt"); + w.openBlock("if m.enableLogger {", "}", () -> { + w.write("logger.Logf(logging.Debug, " + + "fmt.Sprintf(\"retrying waiter request, attempt count: %d\", attempt))\n"); + }); + w.write(""); + // end retry only behavior + w.write("}"); + + w.write(""); + w.writeDocs("attempt request"); + w.write("out, metadata, err = next.HandleFinalize(ctx, attemptInput)"); + w.write(""); + + w.writeDocs("check for state mutation"); + w.write("output := out.Result.($P)", outputSymbol); + w.write("retryable, err := m.waiterStateMutator(ctx, m.params, output, err)"); + w.write(""); + + w.openBlock("if !retryable || err != nil {", "}", () -> { + w.write("if m.enableLogger {"); + w.openBlock("if err != nil {", "} else {", () -> { + w.write("logger.Logf(logging.Debug, " + + "\"waiter transitioned to a failed state with unretryable error %v\", err)"); + }); + w.write("logger.Logf(logging.Debug, \"waiter transitioned to a success state\")}"); + w.write("}"); + w.write("return out, metadata, err"); + }); + // end loop + w.write("}"); + + w.openBlock("if m.enableLogger {", "}", () -> { + w.write("logger.Logf(logging.Debug, \"max retry attempts exhausted, max %d\", attempt)"); + }); + + w.addUseImports(SmithyGoDependency.FMT); + w.write("return out, metadata, " + + "fmt.Errorf(\"exhausted all retry attempts while waiting for the resource state\")"); + + w.write(""); + }, (generator, w) -> { + w.write(""); + w.addUseImports(SmithyGoDependency.TIME); + w.write("minDelay time.Duration "); + w.write("maxDelay time.Duration "); + w.write("maxWaitTime time.Duration"); + w.write("maxAttempts int64 "); + w.write("enableLogger bool "); + + w.addUseImports(SmithyGoDependency.CONTEXT); + w.write( + "waiterStateMutator func(context.Context, $P, $P, error) (bool, error)", + inputSymbol, outputSymbol + ); + + w.addUseImports(SmithyGoDependency.SMITHY_HTTP_TRANSPORT); + w.write("requestCloner func(interface{}) interface{}"); + + w.write("params $P", inputSymbol); + w.write(""); + }); + } - writer.openBlock("type wrapper struct {", "}", () -> { - writer.write("Input $P", inputSymbol); - writer.write("Output $P", outputSymbol); - }); + /** + * Generates a waiter state mutator function which is used by the waiter retrier Middleware to mutate + * waiter state as per the defined logic and returned operation response. + * + * @param model the smithy model + * @param symbolProvider symbol provider + * @param writer the Gowriter + * @param operationShape operation shape on which the waiter is modeled + * @param waiterName the waiter name + * @param waiter the waiter structure that contains info on modeled waiter + */ + private void generateWaiterStateMutator( + Model model, + SymbolProvider symbolProvider, + GoWriter writer, + OperationShape operationShape, + String waiterName, + Waiter waiter + ) { + StructureShape inputShape = model.expectShape( + operationShape.getInput().get(), StructureShape.class + ); + StructureShape outputShape = model.expectShape( + operationShape.getOutput().get(), StructureShape.class + ); - writer.write("wrappedIO := &wrapper{ Input: input,\n Output: output,\n}"); - writer.write("_ = wrappedIO"); + Symbol inputSymbol = symbolProvider.toSymbol(inputShape); + Symbol outputSymbol = symbolProvider.toSymbol(outputShape); + writer.openBlock("func $L(ctx context.Context, input $P, output $P, err error) (bool, error) {", + "}", generateWaiterStateMutatorName(waiterName), inputSymbol, outputSymbol, () -> { waiter.getAcceptors().forEach(acceptor -> { - Matcher matcher = acceptor.getMatcher(); - switch (matcher.getMemberName()) { - case "output": - writer.write(""); - writer.addUseImports(SmithyGoDependency.GO_JMESPATH); - writer.addUseImports(SmithyGoDependency.FMT); - - writer.openBlock("{", "}", () -> { + // scope each acceptor to avoid name collisions + writer.openBlock("{", "}", () -> { + Matcher matcher = acceptor.getMatcher(); + switch (matcher.getMemberName()) { + case "output": + writer.write(""); + writer.addUseImports(SmithyGoDependency.GO_JMESPATH); + writer.addUseImports(SmithyGoDependency.FMT); + writer.write("if err != nil { return true, nil }"); Matcher.OutputMember outputMember = (Matcher.OutputMember) matcher; @@ -435,23 +583,26 @@ private void generateWaiterStateMutator( }).write(""); writer.write("expectedValue := $S", expectedValue); writeWaiterComparator(writer, acceptor, comparator, "pathValue", "expectedValue"); - }); - break; + break; - case "inputOutput": - writer.write(""); - writer.addUseImports(SmithyGoDependency.GO_JMESPATH); - writer.addUseImports(SmithyGoDependency.FMT); + case "inputOutput": + writer.write(""); + writer.addUseImports(SmithyGoDependency.GO_JMESPATH); + writer.addUseImports(SmithyGoDependency.FMT); - writer.openBlock("{", "}", () -> { writer.write("if err != nil { return true, nil }"); Matcher.InputOutputMember ioMember = (Matcher.InputOutputMember) matcher; - String path = ioMember.getValue().getPath(); - String expectedValue = ioMember.getValue().getExpected(); - PathComparator comparator = ioMember.getValue().getComparator(); - - writer.write("pathValue, err := jmespath.Search($S, wrappedIO)", path); + path = ioMember.getValue().getPath(); + expectedValue = ioMember.getValue().getExpected(); + comparator = ioMember.getValue().getComparator(); + + writer.openBlock("pathValue, err := jmespath.Search($S, &struct{", + "})", path, () -> { + writer.write("Input $P \n Output $P \n }{", inputSymbol, + outputSymbol); + writer.write("Input: input, \n Output: output, \n"); + }); writer.openBlock("if err != nil {", "}", () -> { writer.write( "return false, fmt.Errorf(\"error evaluating waiter state: %w\", err)"); @@ -459,28 +610,24 @@ private void generateWaiterStateMutator( writer.write(""); writer.write("expectedValue := $S", expectedValue); writeWaiterComparator(writer, acceptor, comparator, "pathValue", "expectedValue"); - }); - break; + break; - case "success": - writer.write(""); + case "success": + writer.write(""); - Matcher.SuccessMember successMember = (Matcher.SuccessMember) matcher; - writer.openBlock("{", "}", () -> { + Matcher.SuccessMember successMember = (Matcher.SuccessMember) matcher; writer.openBlock("if err != nil {", "}", () -> { writer.write("return true, nil"); }); writeMatchedAcceptorReturn(writer, acceptor); - }); - break; + break; - case "errorType": - writer.write("if err == nil { return true, nil }"); - writer.write(""); + case "errorType": + writer.write("if err == nil { return true, nil }"); + writer.write(""); - writer.openBlock("{", "}", () -> { Matcher.ErrorTypeMember errorTypeMember = (Matcher.ErrorTypeMember) matcher; String errorType = errorTypeMember.getValue(); @@ -511,15 +658,15 @@ private void generateWaiterStateMutator( } writer.write("return true, nil"); - }); - writer.write(""); - break; - - default: - throw new CodegenException( - String.format("unknown waiter state : %v", matcher.getMemberName()) - ); - } + writer.write(""); + break; + + default: + throw new CodegenException( + String.format("unknown waiter state : %v", matcher.getMemberName()) + ); + } + }); }); }); } @@ -641,4 +788,11 @@ private String generateWaiterStateMutatorName( return String.format("%sStateMutator", waiterName); } + private String generateWaiterMiddlewareName( + String waiterName + ) { + waiterName = StringUtils.uncapitalize(waiterName); + return String.format("%sWaiterRetrier", waiterName); + } + } From e18ca4fbbcffa0c66d2a389572fb400408c770a8 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Wed, 2 Dec 2020 10:29:20 -0800 Subject: [PATCH 06/23] simplify waiter state mutator and fix issue with multi acceptor waiters --- .../go/codegen/integration/Waiters.java | 137 +++++++++--------- 1 file changed, 66 insertions(+), 71 deletions(-) diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java index fa3d611f4..98b96c0df 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java @@ -560,105 +560,99 @@ private void generateWaiterStateMutator( writer.openBlock("func $L(ctx context.Context, input $P, output $P, err error) (bool, error) {", "}", generateWaiterStateMutatorName(waiterName), inputSymbol, outputSymbol, () -> { waiter.getAcceptors().forEach(acceptor -> { + writer.write(""); // scope each acceptor to avoid name collisions - writer.openBlock("{", "}", () -> { Matcher matcher = acceptor.getMatcher(); switch (matcher.getMemberName()) { case "output": - writer.write(""); writer.addUseImports(SmithyGoDependency.GO_JMESPATH); writer.addUseImports(SmithyGoDependency.FMT); - writer.write("if err != nil { return true, nil }"); - Matcher.OutputMember outputMember = (Matcher.OutputMember) matcher; String path = outputMember.getValue().getPath(); String expectedValue = outputMember.getValue().getExpected(); PathComparator comparator = outputMember.getValue().getComparator(); + writer.openBlock("if err == nil {", "}", () -> { + writer.write("pathValue, err := jmespath.Search($S, output)", path); + writer.openBlock("if err != nil {", "}", () -> { + writer.write( + "return false, " + + "fmt.Errorf(\"error evaluating waiter state: %w\", err)"); + }).write(""); + writer.write("expectedValue := $S", expectedValue); + writeWaiterComparator(writer, acceptor, comparator, "pathValue", + "expectedValue"); + }); - writer.write("pathValue, err := jmespath.Search($S, output)", path); - writer.openBlock("if err != nil {", "}", () -> { - writer.write( - "return false, fmt.Errorf(\"error evaluating waiter state: %w\", err)"); - }).write(""); - writer.write("expectedValue := $S", expectedValue); - writeWaiterComparator(writer, acceptor, comparator, "pathValue", "expectedValue"); break; case "inputOutput": - writer.write(""); writer.addUseImports(SmithyGoDependency.GO_JMESPATH); writer.addUseImports(SmithyGoDependency.FMT); - writer.write("if err != nil { return true, nil }"); - Matcher.InputOutputMember ioMember = (Matcher.InputOutputMember) matcher; path = ioMember.getValue().getPath(); expectedValue = ioMember.getValue().getExpected(); comparator = ioMember.getValue().getComparator(); - - writer.openBlock("pathValue, err := jmespath.Search($S, &struct{", - "})", path, () -> { - writer.write("Input $P \n Output $P \n }{", inputSymbol, - outputSymbol); - writer.write("Input: input, \n Output: output, \n"); - }); - writer.openBlock("if err != nil {", "}", () -> { - writer.write( - "return false, fmt.Errorf(\"error evaluating waiter state: %w\", err)"); + writer.openBlock("if err == nil {", "}", () -> { + writer.openBlock("pathValue, err := jmespath.Search($S, &struct{", + "})", path, () -> { + writer.write("Input $P \n Output $P \n }{", inputSymbol, + outputSymbol); + writer.write("Input: input, \n Output: output, \n"); + }); + writer.openBlock("if err != nil {", "}", () -> { + writer.write( + "return false, " + + "fmt.Errorf(\"error evaluating waiter state: %w\", err)"); + }); + writer.write(""); + writer.write("expectedValue := $S", expectedValue); + writeWaiterComparator(writer, acceptor, comparator, "pathValue", + "expectedValue"); }); - writer.write(""); - writer.write("expectedValue := $S", expectedValue); - writeWaiterComparator(writer, acceptor, comparator, "pathValue", "expectedValue"); break; case "success": - writer.write(""); - Matcher.SuccessMember successMember = (Matcher.SuccessMember) matcher; - writer.openBlock("if err != nil {", "}", + writer.openBlock("if err == nil {", "}", () -> { - writer.write("return true, nil"); + writeMatchedAcceptorReturn(writer, acceptor); }); - - writeMatchedAcceptorReturn(writer, acceptor); break; case "errorType": - writer.write("if err == nil { return true, nil }"); - writer.write(""); - Matcher.ErrorTypeMember errorTypeMember = (Matcher.ErrorTypeMember) matcher; String errorType = errorTypeMember.getValue(); - // identify if this is a modeled error shape - Optional errorShape = operationShape.getErrors().stream().filter( - shapeId -> { - return shapeId.getName().equalsIgnoreCase(errorType); - }).findFirst(); - - // if modeled error shape - if (errorShape.isPresent()) { - Symbol modeledErrorSymbol = SymbolUtils.createValueSymbolBuilder( - errorShape.get().getName(), "types" - ).build(); - writer.addUseImports(SmithyGoDependency.ERRORS); - writer.write("var errorType *$T", modeledErrorSymbol); - writer.openBlock("if errors.As(err, &errorType) {", "}", - () -> { - writeMatchedAcceptorReturn(writer, acceptor); - }); - } else { - // fall back to un-modeled error shape matching - writer.addUseImports(SmithyGoDependency.STRINGS); - writer.openBlock("if strings.Contains(err.Error(), $S) {", - "}", errorType, () -> { - writeMatchedAcceptorReturn(writer, acceptor); - }); - } + writer.openBlock("if err != nil {", "}", () -> { - writer.write("return true, nil"); - writer.write(""); + // identify if this is a modeled error shape + Optional errorShape = operationShape.getErrors().stream().filter( + shapeId -> { + return shapeId.getName().equalsIgnoreCase(errorType); + }).findFirst(); + + // if modeled error shape + if (errorShape.isPresent()) { + Symbol modeledErrorSymbol = SymbolUtils.createValueSymbolBuilder( + errorShape.get().getName(), "types" + ).build(); + writer.addUseImports(SmithyGoDependency.ERRORS); + writer.write("var errorType *$T", modeledErrorSymbol); + writer.openBlock("if errors.As(err, &errorType) {", "}", + () -> { + writeMatchedAcceptorReturn(writer, acceptor); + }); + } else { + // fall back to un-modeled error shape matching + writer.addUseImports(SmithyGoDependency.STRINGS); + writer.openBlock("if strings.Contains(err.Error(), $S) {", "}", + errorType, () -> { + writeMatchedAcceptorReturn(writer, acceptor); + }); + } + }); break; default: @@ -667,7 +661,9 @@ private void generateWaiterStateMutator( ); } }); - }); + + writer.write(""); + writer.write("return true, nil"); }); } @@ -693,7 +689,6 @@ private void writeWaiterComparator( writer.openBlock("if $L == $L {", "}", actual, expected, () -> { writeMatchedAcceptorReturn(writer, acceptor); }); - writer.write("return true, nil"); break; case BOOLEAN_EQUALS: @@ -704,15 +699,18 @@ private void writeWaiterComparator( writer.openBlock("if $L == bv {", "}", actual, () -> { writeMatchedAcceptorReturn(writer, acceptor); }); - writer.write("return true, nil"); break; case ALL_STRING_EQUALS: + writer.write("match = true"); + writer.write("if length(actual.([]string)) == 0 { match = false }"); writer.openBlock("for _, v := range actual {", "}", () -> { - writer.write("if v != $L { return true, nil }", expected); + writer.write("if v != $L { equality = false }", expected); }); writer.write(""); - writeMatchedAcceptorReturn(writer, acceptor); + writer.openBlock("if match {", "}", () -> { + writeMatchedAcceptorReturn(writer, acceptor); + }); break; case ANY_STRING_EQUALS: @@ -722,15 +720,12 @@ private void writeWaiterComparator( writeMatchedAcceptorReturn(writer, acceptor); }); }); - writer.write("return true, nil"); break; default: throw new CodegenException( String.format("Found unknown waiter path comparator, %s", comparator.toString())); } - - writer.write(""); } From 53751d04471184602e3b53a85e28137a337f8e6c Mon Sep 17 00:00:00 2001 From: skotambkar Date: Wed, 2 Dec 2020 11:28:13 -0800 Subject: [PATCH 07/23] more model driven testing, and fine-tuning waiters for different comparators --- .../smithy-go-codegen-test/model/main.smithy | 29 +++ .../go/codegen/integration/Waiters.java | 223 +++++++++--------- 2 files changed, 146 insertions(+), 106 deletions(-) diff --git a/codegen/smithy-go-codegen-test/model/main.smithy b/codegen/smithy-go-codegen-test/model/main.smithy index 85e6d239f..3ce76aff6 100644 --- a/codegen/smithy-go-codegen-test/model/main.smithy +++ b/codegen/smithy-go-codegen-test/model/main.smithy @@ -222,6 +222,35 @@ apply NoSuchResource @httpResponseTests([ // return truncated results. @readonly @paginated(items: "items") +@waitable( + "ListContainsCity": { + description: "Wait until ListCities operation response matches a given state", + acceptors: [ + // failure in case all items returned match to seattle + { + state: "failure", + matcher: { + output: { + path: "items", + comparator: "allStringEquals", + expected: "seattle", + } + } + }, + // success in case any items returned match to NewYork + { + state: "success", + matcher: { + output: { + path: "items", + comparator: "anyStringEquals", + expected: "NewYork", + } + } + } + ] + } +) @http(method: "GET", uri: "/cities") operation ListCities { input: ListCitiesInput, diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java index 98b96c0df..537d93d77 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java @@ -557,110 +557,111 @@ private void generateWaiterStateMutator( Symbol inputSymbol = symbolProvider.toSymbol(inputShape); Symbol outputSymbol = symbolProvider.toSymbol(outputShape); + writer.write(""); writer.openBlock("func $L(ctx context.Context, input $P, output $P, err error) (bool, error) {", "}", generateWaiterStateMutatorName(waiterName), inputSymbol, outputSymbol, () -> { waiter.getAcceptors().forEach(acceptor -> { writer.write(""); // scope each acceptor to avoid name collisions - Matcher matcher = acceptor.getMatcher(); - switch (matcher.getMemberName()) { - case "output": - writer.addUseImports(SmithyGoDependency.GO_JMESPATH); - writer.addUseImports(SmithyGoDependency.FMT); - - Matcher.OutputMember outputMember = (Matcher.OutputMember) matcher; - String path = outputMember.getValue().getPath(); - String expectedValue = outputMember.getValue().getExpected(); - PathComparator comparator = outputMember.getValue().getComparator(); - writer.openBlock("if err == nil {", "}", () -> { - writer.write("pathValue, err := jmespath.Search($S, output)", path); - writer.openBlock("if err != nil {", "}", () -> { - writer.write( - "return false, " - + "fmt.Errorf(\"error evaluating waiter state: %w\", err)"); - }).write(""); - writer.write("expectedValue := $S", expectedValue); - writeWaiterComparator(writer, acceptor, comparator, "pathValue", - "expectedValue"); - }); - - break; - - case "inputOutput": - writer.addUseImports(SmithyGoDependency.GO_JMESPATH); - writer.addUseImports(SmithyGoDependency.FMT); - - Matcher.InputOutputMember ioMember = (Matcher.InputOutputMember) matcher; - path = ioMember.getValue().getPath(); - expectedValue = ioMember.getValue().getExpected(); - comparator = ioMember.getValue().getComparator(); - writer.openBlock("if err == nil {", "}", () -> { - writer.openBlock("pathValue, err := jmespath.Search($S, &struct{", - "})", path, () -> { - writer.write("Input $P \n Output $P \n }{", inputSymbol, - outputSymbol); - writer.write("Input: input, \n Output: output, \n"); - }); - writer.openBlock("if err != nil {", "}", () -> { - writer.write( - "return false, " - + "fmt.Errorf(\"error evaluating waiter state: %w\", err)"); - }); - writer.write(""); - writer.write("expectedValue := $S", expectedValue); - writeWaiterComparator(writer, acceptor, comparator, "pathValue", - "expectedValue"); - }); - break; - - case "success": - Matcher.SuccessMember successMember = (Matcher.SuccessMember) matcher; - writer.openBlock("if err == nil {", "}", - () -> { - writeMatchedAcceptorReturn(writer, acceptor); + Matcher matcher = acceptor.getMatcher(); + switch (matcher.getMemberName()) { + case "output": + writer.addUseImports(SmithyGoDependency.GO_JMESPATH); + writer.addUseImports(SmithyGoDependency.FMT); + + Matcher.OutputMember outputMember = (Matcher.OutputMember) matcher; + String path = outputMember.getValue().getPath(); + String expectedValue = outputMember.getValue().getExpected(); + PathComparator comparator = outputMember.getValue().getComparator(); + writer.openBlock("if err == nil {", "}", () -> { + writer.write("pathValue, err := jmespath.Search($S, output)", path); + writer.openBlock("if err != nil {", "}", () -> { + writer.write( + "return false, " + + "fmt.Errorf(\"error evaluating waiter state: %w\", err)"); + }).write(""); + writer.write("expectedValue := $S", expectedValue); + writeWaiterComparator(writer, acceptor, comparator, "pathValue", + "expectedValue"); + }); + + break; + + case "inputOutput": + writer.addUseImports(SmithyGoDependency.GO_JMESPATH); + writer.addUseImports(SmithyGoDependency.FMT); + + Matcher.InputOutputMember ioMember = (Matcher.InputOutputMember) matcher; + path = ioMember.getValue().getPath(); + expectedValue = ioMember.getValue().getExpected(); + comparator = ioMember.getValue().getComparator(); + writer.openBlock("if err == nil {", "}", () -> { + writer.openBlock("pathValue, err := jmespath.Search($S, &struct{", + "})", path, () -> { + writer.write("Input $P \n Output $P \n }{", inputSymbol, + outputSymbol); + writer.write("Input: input, \n Output: output, \n"); }); - break; - - case "errorType": - Matcher.ErrorTypeMember errorTypeMember = (Matcher.ErrorTypeMember) matcher; - String errorType = errorTypeMember.getValue(); - writer.openBlock("if err != nil {", "}", () -> { - - // identify if this is a modeled error shape - Optional errorShape = operationShape.getErrors().stream().filter( - shapeId -> { - return shapeId.getName().equalsIgnoreCase(errorType); - }).findFirst(); - - // if modeled error shape - if (errorShape.isPresent()) { - Symbol modeledErrorSymbol = SymbolUtils.createValueSymbolBuilder( - errorShape.get().getName(), "types" - ).build(); - writer.addUseImports(SmithyGoDependency.ERRORS); - writer.write("var errorType *$T", modeledErrorSymbol); - writer.openBlock("if errors.As(err, &errorType) {", "}", - () -> { - writeMatchedAcceptorReturn(writer, acceptor); - }); - } else { - // fall back to un-modeled error shape matching - writer.addUseImports(SmithyGoDependency.STRINGS); - writer.openBlock("if strings.Contains(err.Error(), $S) {", "}", - errorType, () -> { - writeMatchedAcceptorReturn(writer, acceptor); - }); - } + writer.write( + "return false, " + + "fmt.Errorf(\"error evaluating waiter state: %w\", err)"); }); - break; - - default: - throw new CodegenException( - String.format("unknown waiter state : %v", matcher.getMemberName()) - ); - } - }); + writer.write(""); + writer.write("expectedValue := $S", expectedValue); + writeWaiterComparator(writer, acceptor, comparator, "pathValue", + "expectedValue"); + }); + break; + + case "success": + Matcher.SuccessMember successMember = (Matcher.SuccessMember) matcher; + writer.openBlock("if err == nil {", "}", + () -> { + writeMatchedAcceptorReturn(writer, acceptor); + }); + break; + + case "errorType": + Matcher.ErrorTypeMember errorTypeMember = (Matcher.ErrorTypeMember) matcher; + String errorType = errorTypeMember.getValue(); + + writer.openBlock("if err != nil {", "}", () -> { + + // identify if this is a modeled error shape + Optional errorShape = operationShape.getErrors().stream().filter( + shapeId -> { + return shapeId.getName().equalsIgnoreCase(errorType); + }).findFirst(); + + // if modeled error shape + if (errorShape.isPresent()) { + Symbol modeledErrorSymbol = SymbolUtils.createValueSymbolBuilder( + errorShape.get().getName(), "types" + ).build(); + writer.addUseImports(SmithyGoDependency.ERRORS); + writer.write("var errorType *$T", modeledErrorSymbol); + writer.openBlock("if errors.As(err, &errorType) {", "}", + () -> { + writeMatchedAcceptorReturn(writer, acceptor); + }); + } else { + // fall back to un-modeled error shape matching + writer.addUseImports(SmithyGoDependency.STRINGS); + writer.openBlock("if strings.Contains(err.Error(), $S) {", "}", + errorType, () -> { + writeMatchedAcceptorReturn(writer, acceptor); + }); + } + }); + break; + + default: + throw new CodegenException( + String.format("unknown waiter state : %v", matcher.getMemberName()) + ); + } + }); writer.write(""); writer.write("return true, nil"); @@ -686,7 +687,9 @@ private void writeWaiterComparator( ) { switch (comparator) { case STRING_EQUALS: - writer.openBlock("if $L == $L {", "}", actual, expected, () -> { + writer.write("value := $L.(string)", actual); + writer.addUseImports(SmithyGoDependency.STRINGS); + writer.openBlock("if strings.EqualFold(value, $L) {", "}", expected, () -> { writeMatchedAcceptorReturn(writer, acceptor); }); break; @@ -702,10 +705,15 @@ private void writeWaiterComparator( break; case ALL_STRING_EQUALS: - writer.write("match = true"); - writer.write("if length(actual.([]string)) == 0 { match = false }"); - writer.openBlock("for _, v := range actual {", "}", () -> { - writer.write("if v != $L { equality = false }", expected); + writer.write("var match = true"); + writer.write("listOfValues := $L.([]string)", actual); + writer.write(""); + + writer.write("if len(listOfValues) == 0 { match = false }"); + + writer.addUseImports(SmithyGoDependency.STRINGS); + writer.openBlock("for _, v := range listOfValues {", "}", () -> { + writer.write("if !strings.EqualFold(v, $L) { match = false }", expected); }); writer.write(""); writer.openBlock("if match {", "}", () -> { @@ -714,11 +722,14 @@ private void writeWaiterComparator( break; case ANY_STRING_EQUALS: - writer.openBlock("for _, v := range actual {", "}", () -> { - writer.openBlock("if v == $L {", "}", - expected, () -> { - writeMatchedAcceptorReturn(writer, acceptor); - }); + writer.write("listOfValues := $L.([]string)", actual); + writer.write(""); + + writer.addUseImports(SmithyGoDependency.STRINGS); + writer.openBlock("for _, v := range listOfValues {", "}", () -> { + writer.openBlock("if strings.EqualFold(v, $L) {", "}", expected, () -> { + writeMatchedAcceptorReturn(writer, acceptor); + }); }); break; From ecdac4f8f31e7680d5498338e5f5813c41bf0b36 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Wed, 2 Dec 2020 13:54:41 -0800 Subject: [PATCH 08/23] make comparators case sensitive, update unmodeled error handling and change model for it --- .../smithy-go-codegen-test/model/main.smithy | 7 ++ .../go/codegen/integration/Waiters.java | 65 ++++++++++++++----- 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/codegen/smithy-go-codegen-test/model/main.smithy b/codegen/smithy-go-codegen-test/model/main.smithy index 3ce76aff6..e340feb30 100644 --- a/codegen/smithy-go-codegen-test/model/main.smithy +++ b/codegen/smithy-go-codegen-test/model/main.smithy @@ -48,6 +48,13 @@ string CityId errorType: "NoSuchResource" } }, + // Fail-fast if the thing transitions to a "failed" state. + { + state: "failure", + matcher: { + errorType: "UnModeledError" + } + }, // Succeed when the city image value is not empty i.e. enters into a "success" state. { state: "success", diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java index 537d93d77..456dd8fcb 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java @@ -641,14 +641,24 @@ private void generateWaiterStateMutator( ).build(); writer.addUseImports(SmithyGoDependency.ERRORS); writer.write("var errorType *$T", modeledErrorSymbol); - writer.openBlock("if errors.As(err, &errorType) {", "}", - () -> { - writeMatchedAcceptorReturn(writer, acceptor); - }); + writer.openBlock("if errors.As(err, &errorType) {", "}", () -> { + writeMatchedAcceptorReturn(writer, acceptor); + }); } else { // fall back to un-modeled error shape matching - writer.addUseImports(SmithyGoDependency.STRINGS); - writer.openBlock("if strings.Contains(err.Error(), $S) {", "}", + writer.addUseImports(SmithyGoDependency.SMITHY); + writer.addUseImports(SmithyGoDependency.ERRORS); + + // assert unmodeled error to smithy's API error + writer.write("var apiErr smithy.APIError"); + writer.write("ok := errors.As(err, &apiErr)"); + writer.openBlock("if !ok {", "}", () -> { + writer.write("return false, " + + "fmt.Errorf(\"expected err to be of type smithy.APIError\")"); + }); + writer.write(""); + + writer.openBlock("if $S == apiErr.ErrorCode() {", "}", errorType, () -> { writeMatchedAcceptorReturn(writer, acceptor); }); @@ -687,9 +697,14 @@ private void writeWaiterComparator( ) { switch (comparator) { case STRING_EQUALS: - writer.write("value := $L.(string)", actual); - writer.addUseImports(SmithyGoDependency.STRINGS); - writer.openBlock("if strings.EqualFold(value, $L) {", "}", expected, () -> { + writer.write("value, ok := $L.(string)", actual); + writer.openBlock(" if !ok {", "}", () -> { + writer.write("return false, " + + "fmt.Errorf(\"waiter comparator expected string value got %T\", $L)", actual); + }); + writer.write(""); + + writer.openBlock("if value == $L {", "}", expected, () -> { writeMatchedAcceptorReturn(writer, acceptor); }); break; @@ -698,36 +713,52 @@ private void writeWaiterComparator( writer.addUseImports(SmithyGoDependency.STRCONV); writer.write("bv, err := strconv.ParseBool($L)", expected); writer.write( - "if err != nil { return false, fmt.Errorf(\"error parsing boolean from string %w\", err)}"); - writer.openBlock("if $L == bv {", "}", actual, () -> { + "if err != nil { return false, " + + "fmt.Errorf(\"error parsing boolean from string %w\", err)}"); + + writer.write("value, ok := $L.(bool)", actual); + writer.openBlock(" if !ok {", "}", () -> { + writer.write("return false, " + + "fmt.Errorf(\"waiter comparator expected bool value got %T\", $L)", actual); + }); + writer.write(""); + + writer.openBlock("if value == bv {", "}", () -> { writeMatchedAcceptorReturn(writer, acceptor); }); break; case ALL_STRING_EQUALS: writer.write("var match = true"); - writer.write("listOfValues := $L.([]string)", actual); + writer.write("listOfValues, ok := $L.([]string)", actual); + writer.openBlock(" if !ok {", "}", () -> { + writer.write("return false, " + + "fmt.Errorf(\"waiter comparator expected []string value got %T\", $L)", actual); + }); writer.write(""); writer.write("if len(listOfValues) == 0 { match = false }"); - writer.addUseImports(SmithyGoDependency.STRINGS); writer.openBlock("for _, v := range listOfValues {", "}", () -> { - writer.write("if !strings.EqualFold(v, $L) { match = false }", expected); + writer.write("if v != $L { match = false }", expected); }); writer.write(""); + writer.openBlock("if match {", "}", () -> { writeMatchedAcceptorReturn(writer, acceptor); }); break; case ANY_STRING_EQUALS: - writer.write("listOfValues := $L.([]string)", actual); + writer.write("listOfValues, ok := $L.([]string)", actual); + writer.openBlock(" if !ok {", "}", () -> { + writer.write("return false, " + + "fmt.Errorf(\"waiter comparator expected []string value got %T\", $L)", actual); + }); writer.write(""); - writer.addUseImports(SmithyGoDependency.STRINGS); writer.openBlock("for _, v := range listOfValues {", "}", () -> { - writer.openBlock("if strings.EqualFold(v, $L) {", "}", expected, () -> { + writer.openBlock("if v == $L {", "}", expected, () -> { writeMatchedAcceptorReturn(writer, acceptor); }); }); From 96d63bb8c2ff277b4aae0fe886f3fcf83a284769 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Thu, 3 Dec 2020 01:41:42 -0800 Subject: [PATCH 09/23] move sleep with context method to smithy time package --- time/time.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/time/time.go b/time/time.go index 241c555b2..0a3fbe48c 100644 --- a/time/time.go +++ b/time/time.go @@ -1,6 +1,7 @@ package time import ( + "context" "time" ) @@ -53,3 +54,20 @@ func FormatEpochSeconds(value time.Time) float64 { func ParseEpochSeconds(value float64) time.Time { return time.Unix(0, int64(value*float64(time.Second))).UTC() } + +// SleepWithContext will wait for the timer duration to expire, or the context +// is canceled. Which ever happens first. If the context is canceled the +// Context's error will be returned. +func SleepWithContext(ctx context.Context, dur time.Duration) error { + t := time.NewTimer(dur) + defer t.Stop() + + select { + case <-t.C: + break + case <-ctx.Done(): + return ctx.Err() + } + + return nil +} From 3adc0fc999d35b2e52b25c8df0b1a57ad2a03798 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Thu, 3 Dec 2020 02:21:39 -0800 Subject: [PATCH 10/23] feedback and change of design --- .../GoStackStepMiddlewareGenerator.java | 17 - .../smithy/go/codegen/SmithyGoDependency.java | 2 +- .../integration/InterfaceGenerator.java | 126 ++++++ .../go/codegen/integration/Paginators.java | 32 +- .../go/codegen/integration/Waiters.java | 376 +++++------------- ...mithy.go.codegen.integration.GoIntegration | 1 + waiters/waiter.go | 18 - 7 files changed, 230 insertions(+), 342 deletions(-) create mode 100644 codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/InterfaceGenerator.java diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoStackStepMiddlewareGenerator.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoStackStepMiddlewareGenerator.java index af4d5326a..a53816e99 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoStackStepMiddlewareGenerator.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoStackStepMiddlewareGenerator.java @@ -114,23 +114,6 @@ public static GoStackStepMiddlewareGenerator createDeserializeStepMiddleware(Str .build()); } - /** - * Create a new Finalize step middleware generator with the provided type name. - * - * @param name is the type name to identify the middleware. - * @param id the unique ID for the middleware. - * @return the middleware generator. - */ - public static GoStackStepMiddlewareGenerator createFinalizeStepMiddleware(String name, MiddlewareIdentifier id) { - return createMiddleware(name, - id, - "HandleFinalize", - SymbolUtils.createValueSymbolBuilder("FinalizeInput", SmithyGoDependency.SMITHY_MIDDLEWARE).build(), - SymbolUtils.createValueSymbolBuilder("FinalizeOutput", SmithyGoDependency.SMITHY_MIDDLEWARE).build(), - SymbolUtils.createValueSymbolBuilder("FinalizeHandler", SmithyGoDependency.SMITHY_MIDDLEWARE) - .build()); - } - /** * Generates a new step middleware generator. * diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java index 678467426..606a99669 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java @@ -118,7 +118,7 @@ private static GoDependency relativePackage( private static final class Versions { private static final String GO_STDLIB = "1.15"; private static final String GO_CMP = "v0.5.4"; - private static final String SMITHY_GO = "v0.4.1-0.20201202055352-1ba08af0096d"; + private static final String SMITHY_GO = "v0.4.1-0.20201203094142-96d63bb8c2ff"; private static final String GO_JMESPATH = "v0.4.0"; } } diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/InterfaceGenerator.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/InterfaceGenerator.java new file mode 100644 index 000000000..6c70d3cc5 --- /dev/null +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/InterfaceGenerator.java @@ -0,0 +1,126 @@ +/* + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.smithy.go.codegen.integration; + +import java.util.Set; +import java.util.TreeSet; +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.GoWriter; +import software.amazon.smithy.go.codegen.SmithyGoDependency; +import software.amazon.smithy.go.codegen.SymbolUtils; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.knowledge.TopDownIndex; +import software.amazon.smithy.model.shapes.OperationShape; +import software.amazon.smithy.model.shapes.ServiceShape; +import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.model.traits.PaginatedTrait; +import software.amazon.smithy.waiters.WaitableTrait; + +/** + * Generates API client Interfaces as per API operation. + */ +public class InterfaceGenerator implements GoIntegration { + + private static Set listOfClientInterfaceOperations = new TreeSet<>(); + + @Override + public void processFinalizedModel( + GoSettings settings, + Model model + ) { + ServiceShape serviceShape = settings.getService(model); + TopDownIndex topDownIndex = TopDownIndex.of(model); + + // fetch operations for which paginated trait is applied + topDownIndex.getContainedOperations(serviceShape).stream() + .filter(operationShape -> operationShape.hasTrait(PaginatedTrait.class)) + .forEach(operationShape -> listOfClientInterfaceOperations.add(operationShape.getId())); + + if (serviceShape.hasTrait(PaginatedTrait.class)) { + topDownIndex.getContainedOperations(serviceShape).stream() + .forEach(operationShape -> listOfClientInterfaceOperations.add(operationShape.getId())); + } + + // fetch operations for which waitable trait is applied + topDownIndex.getContainedOperations(serviceShape).stream() + .filter(operationShape -> operationShape.hasTrait(WaitableTrait.class)) + .forEach(operationShape -> listOfClientInterfaceOperations.add(operationShape.getId())); + + if (listOfClientInterfaceOperations.isEmpty()) { + throw new CodegenException("empty operations"); + } + } + + + @Override + public void writeAdditionalFiles( + GoSettings settings, + Model model, + SymbolProvider symbolProvider, + GoDelegator goDelegator + ) { + listOfClientInterfaceOperations.stream().forEach(shapeId -> { + OperationShape operationShape = model.expectShape(shapeId, OperationShape.class); + goDelegator.useShapeWriter(operationShape, writer -> { + generateApiClientInterface(writer, model, symbolProvider, operationShape); + }); + }); + } + + private void generateApiClientInterface( + GoWriter writer, + Model model, + SymbolProvider symbolProvider, + OperationShape operationShape + ) { + Symbol contextSymbol = SymbolUtils.createValueSymbolBuilder("Context", SmithyGoDependency.CONTEXT) + .build(); + + Symbol operationSymbol = symbolProvider.toSymbol(operationShape); + + Symbol interfaceSymbol = SymbolUtils.createValueSymbolBuilder(getApiClientInterfaceName(operationSymbol)) + .build(); + + Symbol inputSymbol = symbolProvider.toSymbol(model.expectShape(operationShape.getInput().get())); + Symbol outputSymbol = symbolProvider.toSymbol(model.expectShape(operationShape.getOutput().get())); + + writer.writeDocs(String.format("%s is a client that implements the %s operation.", + interfaceSymbol.getName(), operationSymbol.getName())); + writer.openBlock("type $T interface {", "}", interfaceSymbol, () -> { + writer.write("$L($T, $P, ...func(*Options)) ($P, error)", operationSymbol.getName(), contextSymbol, + inputSymbol, outputSymbol); + }); + writer.write(""); + writer.write("var _ $T = (*Client)(nil)", interfaceSymbol); + writer.write(""); + } + + /** + * Returns name of an API client interface. + * + * @param operationSymbol Symbol of operation shape for which Api client interface is being generated. + * @return name of the interface. + */ + public static String getApiClientInterfaceName( + Symbol operationSymbol + ) { + return String.format("%sAPIClient", operationSymbol.getName()); + } +} diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Paginators.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Paginators.java index 89a88788b..0ba2cde3c 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Paginators.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Paginators.java @@ -72,14 +72,14 @@ private void generateOperationPaginator( ) { Symbol operationSymbol = symbolProvider.toSymbol(paginationInfo.getOperation()); - Symbol interfaceSymbol = SymbolUtils.createValueSymbolBuilder(String.format("%sAPIClient", - operationSymbol.getName())).build(); + Symbol interfaceSymbol = SymbolUtils.createValueSymbolBuilder( + InterfaceGenerator.getApiClientInterfaceName(operationSymbol) + ).build(); Symbol paginatorSymbol = SymbolUtils.createPointableSymbolBuilder(String.format("%sPaginator", operationSymbol.getName())).build(); Symbol optionsSymbol = SymbolUtils.createPointableSymbolBuilder(String.format("%sOptions", paginatorSymbol.getName())).build(); - writeClientOperationInterface(writer, symbolProvider, paginationInfo, interfaceSymbol); writePaginatorOptions(writer, model, symbolProvider, paginationInfo, operationSymbol, optionsSymbol); writePaginator(writer, model, symbolProvider, paginationInfo, interfaceSymbol, paginatorSymbol, optionsSymbol); } @@ -249,30 +249,4 @@ private void writePaginatorOptions( }); writer.write(""); } - - private void writeClientOperationInterface( - GoWriter writer, - SymbolProvider symbolProvider, - PaginationInfo paginationInfo, - Symbol interfaceSymbol - ) { - Symbol contextSymbol = SymbolUtils.createValueSymbolBuilder("Context", SmithyGoDependency.CONTEXT) - .build(); - - Symbol operationSymbol = symbolProvider.toSymbol(paginationInfo.getOperation()); - Symbol inputSymbol = symbolProvider.toSymbol(paginationInfo.getInput()); - Symbol outputSymbol = symbolProvider.toSymbol(paginationInfo.getOutput()); - - writer.writeDocs(String.format("%s is a client that implements the %s operation.", - interfaceSymbol.getName(), operationSymbol.getName())); - writer.openBlock("type $T interface {", "}", interfaceSymbol, () -> { - writer.write("$L($T, $P, ...func(*Options)) ($P, error)", operationSymbol.getName(), contextSymbol, - inputSymbol, outputSymbol); - }); - writer.write(""); - writer.write("var _ $T = (*Client)(nil)", interfaceSymbol); - writer.write(""); - } - - } diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java index 456dd8fcb..006cfc74e 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java @@ -22,9 +22,7 @@ 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.MiddlewareIdentifier; import software.amazon.smithy.go.codegen.SmithyGoDependency; import software.amazon.smithy.go.codegen.SymbolUtils; import software.amazon.smithy.model.Model; @@ -45,9 +43,6 @@ */ public class Waiters implements GoIntegration { private static final String WAITER_INVOKER_FUNCTION_NAME = "Wait"; - private static final String WAITER_MIDDLEWARE_ID = "WaiterRetrier"; - private static final int DEFAULT_MAX_WAIT_TIME_IN_SECONDS = 300; - private static final int DEFAULT_MAX_ATTEMPTS = 8; @Override public void writeAdditionalFiles( @@ -84,9 +79,6 @@ private void generateOperationWaiter( OperationShape operation, Map waiters ) { - // write client interface - generateApiClientInterface(model, symbolProvider, writer, operation); - // generate waiter function waiters.forEach((name, waiter) -> { // write waiter options @@ -99,51 +91,11 @@ private void generateOperationWaiter( generateWaiterInvoker(model, symbolProvider, writer, operation, name, waiter); // write waiter state mutator for each waiter - generateWaiterStateMutator(model, symbolProvider, writer, operation, name, waiter); + generateRetryable(model, symbolProvider, writer, operation, name, waiter); - // write waiter middleware for each waiter - generateWaiterMiddleware(model, symbolProvider, writer, operation, name, waiter); }); } - /** - * Generates interface to satisfy service client and invoke the relevant operation. - */ - private void generateApiClientInterface( - Model model, - SymbolProvider symbolProvider, - GoWriter writer, - OperationShape operationShape - ) { - StructureShape inputShape = model.expectShape( - operationShape.getInput().get(), StructureShape.class - ); - StructureShape outputShape = model.expectShape( - operationShape.getOutput().get(), StructureShape.class - ); - - Symbol operationSymbol = symbolProvider.toSymbol(operationShape); - Symbol inputSymbol = symbolProvider.toSymbol(inputShape); - Symbol outputSymbol = symbolProvider.toSymbol(outputShape); - - String interfaceName = generateApiClientInterfaceName(operationSymbol); - - writer.write(""); - writer.writeDocs( - String.format("%s is a client that implements %s operation", interfaceName, operationSymbol.getName()) - ); - writer.openBlock("type $L interface {", "}", - interfaceName, () -> { - writer.addUseImports(SmithyGoDependency.CONTEXT); - writer.write( - "$T(context.Context, $P, ...func(*Options)) ($P, error)", - operationSymbol, inputSymbol, outputSymbol - ); - } - ); - writer.write(""); - } - /** * Generates waiter options to configure a waiter client. */ @@ -177,37 +129,43 @@ private void generateWaiterOptions( writer.addUseImports(SmithyGoDependency.TIME); writer.write(""); - writer.writeDocs("MinDelay is the minimum amount of time to delay between retries in seconds"); - writer.write("MinDelay time.Duration"); - - writer.write(""); - writer.writeDocs("MaxDelay is the maximum amount of time to delay between retries in seconds"); - writer.write("MaxDelay time.Duration"); + writer.writeDocs( + "Set of options to modify how an operation is invoked. These apply to all operations " + + "invoked for this client. Use functional options on operation call to modify " + + "this list for per operation behavior." + ); + Symbol stackSymbol = SymbolUtils.createPointableSymbolBuilder("Stack", + SmithyGoDependency.SMITHY_MIDDLEWARE) + .build(); + writer.write("APIOptions []func($P) error", stackSymbol); writer.write(""); - writer.writeDocs("MaxWaitTime is the maximum amount of wait time in seconds before a waiter is " - + "forced to return"); - writer.write("MaxWaitTime time.Duration"); + writer.writeDocs("MinDelay is the minimum amount of time to delay between retries"); + writer.write("MinDelay time.Duration"); writer.write(""); - writer.writeDocs("MaxAttempts is the maximum number of attempts to fetch a terminal waiter state"); - writer.write("MaxAttempts int64"); + writer.writeDocs("MaxDelay is the maximum amount of time to delay between retries"); + writer.write("MaxDelay time.Duration"); writer.write(""); - writer.writeDocs("EnableLogger is used to enable logging for waiter retry attempts"); - writer.write("EnableLogger bool"); + writer.writeDocs("LogWaitAttempts is used to enable logging for waiter retry attempts"); + writer.write("LogWaitAttempts bool"); writer.write(""); writer.writeDocs( - "WaiterStateMutator is mutator function option that can be used to override the " + "Retryable is function that can be used to override the " + "service defined waiter-behavior based on operation output, or returned error. " - + "The mutator function is used by the waiter to decide if a state is retryable " - + "or a terminal state.\n\nBy default service-modeled waiter state mutators " + + "This function is used by the waiter to decide if a state is retryable " + + "or a terminal state.\n\nBy default service-modeled logic " + "will populate this option. This option can thus be used to define a custom " + "waiter state with fall-back to service-modeled waiter state mutators." + + "The function returns an error in case of a failure state. " + + "In case of retry state, this function returns a bool value of true and " + + "nil error, while in case of success it returns a bool value of false and " + + "nil error." ); writer.write( - "WaiterStateMutator func(context.Context, $P, $P, error) " + "Retryable func(context.Context, $P, $P, error) " + "(bool, error)", inputSymbol, outputSymbol); } ); @@ -239,7 +197,7 @@ private void generateWaiterClient( writer.openBlock("type $L struct {", "}", clientName, () -> { writer.write(""); - writer.write("client $L", generateApiClientInterfaceName(operationSymbol)); + writer.write("client $L", InterfaceGenerator.getApiClientInterfaceName(operationSymbol)); writer.write(""); writer.write("options $L", generateWaiterOptionsName(waiterName)); @@ -261,18 +219,15 @@ private void generateWaiterClient( String.format("%s constructs a %s.", constructorName, clientName) ); writer.openBlock("func $L(client $L, optFns ...func($P)) $P {", "}", - constructorName, generateApiClientInterfaceName(operationSymbol), waiterOptionsSymbol, clientSymbol, - () -> { + constructorName, InterfaceGenerator.getApiClientInterfaceName(operationSymbol), + waiterOptionsSymbol, clientSymbol, () -> { writer.write("options := $T{}", waiterOptionsSymbol); writer.addUseImports(SmithyGoDependency.TIME); // set defaults writer.write("options.MinDelay = $L * time.Second", waiter.getMinDelay()); writer.write("options.MaxDelay = $L * time.Second", waiter.getMaxDelay()); - // set defaults not defined in model - writer.write("options.MaxWaitTime = $L * time.Second", DEFAULT_MAX_WAIT_TIME_IN_SECONDS); - writer.write("options.MaxAttempts = $L ", DEFAULT_MAX_ATTEMPTS); - writer.write("options.WaiterStateMutator = $L", generateWaiterStateMutatorName(waiterName)); + writer.write("options.Retryable = $L", generateRetryableName(waiterName)); writer.write(""); writer.openBlock("for _, fn := range optFns {", @@ -318,214 +273,96 @@ private void generateWaiterInvoker( writer.write(""); writer.addUseImports(SmithyGoDependency.CONTEXT); + writer.addUseImports(SmithyGoDependency.TIME); writer.writeDocs( String.format("%s calls the waiter function for %s waiter", WAITER_INVOKER_FUNCTION_NAME, waiterName) ); - writer.openBlock("func (w $P) $L(ctx context.Context, params $P, optFns ...func($P)) error {", "}", + writer.openBlock( + "func (w $P) $L(ctx context.Context, params $P, maxWaitTime time.Duration, optFns ...func($P)) error {", + "}", clientSymbol, WAITER_INVOKER_FUNCTION_NAME, inputSymbol, waiterOptionsSymbol, () -> { + writer.openBlock("if maxWaitTime == 0 {", "}", () -> { + writer.addUseImports(SmithyGoDependency.FMT); + writer.write("fmt.Errorf(\"maximum wait time for waiter must be greater than zero\")"); + }).write(""); + writer.write("options := $T{}", waiterOptionsSymbol); + writer.write("options.APIOptions = w.options.APIOptions"); writer.write("options.MinDelay = w.options.MinDelay"); writer.write("options.MaxDelay = w.options.MaxDelay"); - writer.write("options.MaxWaitTime = w.options.MaxWaitTime"); - writer.write("options.MaxAttempts = w.options.MaxAttempts"); - writer.write("options.EnableLogger = w.options.EnableLogger"); - writer.write("options.WaiterStateMutator = w.options.WaiterStateMutator"); + writer.write("options.LogWaitAttempts = w.options.LogWaitAttempts"); + writer.write("options.Retryable = w.options.Retryable"); writer.openBlock("for _, fn := range optFns {", "}", () -> { writer.write("fn(&options)"); }); - writer.openBlock("_, err := w.client.$T(ctx, params, func (o *Options) { ", "})", - operationSymbol, () -> { - writer.openBlock("o.APIOptions = append(o.APIOptions, " - + "func(stack *middleware.Stack) error {", "})", - () -> { - writer.openBlock("return stack.Finalize.Add(&$L{", "}, middleware.Before)", - generateWaiterMiddlewareName(waiterName), - () -> { - writer.write("minDelay : options.MinDelay, "); - writer.write("maxDelay : options.MaxDelay, "); - writer.write("maxWaitTime : options.MaxWaitTime, "); - writer.write("maxAttempts : options.MaxAttempts, "); - writer.write("enableLogger : options.EnableLogger, "); - writer.write( - "waiterStateMutator : options.WaiterStateMutator, "); - writer.addUseImports(SmithyGoDependency.SMITHY_HTTP_TRANSPORT); - writer.write("requestCloner : smithyhttp.RequestCloner, "); - writer.write("params: params, "); - }); + writer.addUseImports(SmithyGoDependency.TIME); + writer.addUseImports(SmithyGoDependency.CONTEXT); - }); - }); - writer.write(""); + writer.write("logger := middleware.GetLogger(ctx)").write(""); + writer.write("var attempt int64"); + writer.write("var remainingTime = maxWaitTime"); - writer.addUseImports(SmithyGoDependency.FMT); - writer.write("if err != nil { return fmt.Errorf(\"$L errored with error %w\", err) }", - clientSymbol); - writer.write("return nil"); - }); - } + writer.write("deadline := time.Now().Add(maxWaitTime)"); + writer.write("ctx, cancelFn := context.WithDeadline(ctx, deadline)"); + writer.write("defer cancelFn()"); + writer.openBlock("for {", "}", () -> { + writer.write(""); - /** - * Generates a waiter retrier middleware to trigger waiter state validation and retry behavior. - * - * @param model the smithy model - * @param symbolProvider symbol provider - * @param writer the GoWriter - * @param operationShape operation shape on which waiter is modeled - * @param waiterName the waiter name - * @param waiter the waiter structure that contains info on modeled waiter - */ - private void generateWaiterMiddleware( - Model model, - SymbolProvider symbolProvider, - GoWriter writer, - OperationShape operationShape, - String waiterName, - Waiter waiter - ) { - StructureShape inputShape = model.expectShape( - operationShape.getInput().get(), StructureShape.class - ); - StructureShape outputShape = model.expectShape( - operationShape.getOutput().get(), StructureShape.class - ); + writer.addUseImports(SmithyGoDependency.FMT); + writer.openBlock("if remainingTime <= 0 {", "}", () -> { + writer.write("return fmt.Errorf(\"exceeded maximum wait time for $L waiter\")", + waiterName); + }); + writer.write(""); - Symbol inputSymbol = symbolProvider.toSymbol(inputShape); - Symbol outputSymbol = symbolProvider.toSymbol(outputShape); + writer.write("attempt++").write(""); + + Symbol computeDelaySymbol = SymbolUtils.createValueSymbolBuilder( + "ComputeDelay", SmithyGoDependency.SMITHY_WAITERS + ).build(); + writer.writeDocs("compute exponential backoff between waiter retries"); + writer.openBlock("delay := $T(", ")", computeDelaySymbol, () -> { + writer.write("options.MinDelay, options.MaxDelay, remainingTime, attempt,"); + }).write(""); + + writer.write("remainingTime -= delay"); + + Symbol sleepWithContextSymbol = SymbolUtils.createValueSymbolBuilder( + "SleepWithContext", SmithyGoDependency.SMITHY_TIME + ).build(); + writer.writeDocs("sleep for the delay amount before invoking a request"); + writer.openBlock("if err := $T(ctx, delay); err != nil {", "}", + sleepWithContextSymbol, () -> { + writer.write("return fmt.Errorf(\"request cancelled while waiting, %w\", err)"); + }).write(""); + + // enable logger + writer.openBlock("if options.LogWaitAttempts {", "}", () -> { + writer.addUseImports(SmithyGoDependency.SMITHY_LOGGING); + writer.write("logger.Logf(logging.Debug, " + + "fmt.Sprintf(\"attempting waiter request, attempt count: %d\", attempt))\n"); + }); + writer.write(""); - GoStackStepMiddlewareGenerator middlewareGenerator = - GoStackStepMiddlewareGenerator.createFinalizeStepMiddleware( - generateWaiterMiddlewareName(waiterName), - MiddlewareIdentifier.builder() - .name(WAITER_MIDDLEWARE_ID) - .build() - ); + // make a request + writer.openBlock("out, err := w.client.$T(ctx, params, func (o *Options) { ", "})", + operationSymbol, () -> { + writer.write("o.APIOptions = append(o.APIOptions, options.APIOptions...)"); + }); + writer.write(""); - writer.write(""); - middlewareGenerator.writeMiddleware(writer, (generator, w) -> { - w.write(""); - w.addUseImports(SmithyGoDependency.SMITHY_LOGGING); - w.writeDocs("fetch logger from context"); - w.write("logger := middleware.GetLogger(ctx)"); - w.write(""); - - w.writeDocs("current attempt, delay"); - w.write("var attempt int64"); - w.write("var delay time.Duration"); - w.write("var remainingTime = m.maxWaitTime"); - - // start loop - w.write("for {"); - - w.writeDocs("check number of attempts"); - w.write("if m.maxAttempts == attempt { break }"); - w.write(""); - w.write("attempt++"); - w.write(""); - - w.write("attemptInput := in"); - w.write("attemptInput.Request = m.requestCloner(attemptInput.Request)"); - - w.write(""); - - // start retry only behavior - w.write("if attempt > 1 {"); - - Symbol computeDelaySymbol = SymbolUtils.createValueSymbolBuilder( - "ComputeDelay", SmithyGoDependency.SMITHY_WAITERS - ).build(); - w.writeDocs("compute exponential backoff between waiter retries"); - w.write("delay = $T(m.minDelay, m.maxDelay, remainingTime, attempt)", computeDelaySymbol); - w.write(""); - - w.writeDocs("update the remaining time"); - w.write("remainingTime = remainingTime - delay"); - w.write(""); - - w.writeDocs("rewind transport stream"); - w.write("if rewindable, ok := in.Request.(interface{ RewindStream() error }); ok {"); - w.write("if err := rewindable.RewindStream(); err != nil {"); - w.addUseImports(SmithyGoDependency.FMT); - w.write("return out, metadata, fmt.Errorf(\"failed to rewind transport stream for retry, %w\", err)\n}"); - w.write("}"); - w.write(""); - - Symbol sleepWithContextSymbol = SymbolUtils.createValueSymbolBuilder( - "SleepWithContext", SmithyGoDependency.SMITHY_WAITERS - ).build(); - w.writeDocs("sleep for the delay amount before invoking a request"); - w.openBlock("if err = $T(ctx, delay); err != nil {", "}", - sleepWithContextSymbol, () -> { - w.write("return out, metadata, fmt.Errorf(\"request cancelled while waiting, %w\", err)"); + // handle response and identify waiter state + writer.write("retryable, err := options.Retryable(ctx, params, out, err)"); + writer.write("if err != nil { return err }").write(""); + writer.write("if !retryable { return nil }"); }); - - // enable logger - w.writeDocs("log attempt"); - w.openBlock("if m.enableLogger {", "}", () -> { - w.write("logger.Logf(logging.Debug, " - + "fmt.Sprintf(\"retrying waiter request, attempt count: %d\", attempt))\n"); - }); - w.write(""); - // end retry only behavior - w.write("}"); - - w.write(""); - w.writeDocs("attempt request"); - w.write("out, metadata, err = next.HandleFinalize(ctx, attemptInput)"); - w.write(""); - - w.writeDocs("check for state mutation"); - w.write("output := out.Result.($P)", outputSymbol); - w.write("retryable, err := m.waiterStateMutator(ctx, m.params, output, err)"); - w.write(""); - - w.openBlock("if !retryable || err != nil {", "}", () -> { - w.write("if m.enableLogger {"); - w.openBlock("if err != nil {", "} else {", () -> { - w.write("logger.Logf(logging.Debug, " - + "\"waiter transitioned to a failed state with unretryable error %v\", err)"); + writer.write("return fmt.Errorf(\"exceeded max wait time for $L waiter \")", waiterName); }); - w.write("logger.Logf(logging.Debug, \"waiter transitioned to a success state\")}"); - w.write("}"); - w.write("return out, metadata, err"); - }); - // end loop - w.write("}"); - - w.openBlock("if m.enableLogger {", "}", () -> { - w.write("logger.Logf(logging.Debug, \"max retry attempts exhausted, max %d\", attempt)"); - }); - - w.addUseImports(SmithyGoDependency.FMT); - w.write("return out, metadata, " - + "fmt.Errorf(\"exhausted all retry attempts while waiting for the resource state\")"); - - w.write(""); - }, (generator, w) -> { - w.write(""); - w.addUseImports(SmithyGoDependency.TIME); - w.write("minDelay time.Duration "); - w.write("maxDelay time.Duration "); - w.write("maxWaitTime time.Duration"); - w.write("maxAttempts int64 "); - w.write("enableLogger bool "); - - w.addUseImports(SmithyGoDependency.CONTEXT); - w.write( - "waiterStateMutator func(context.Context, $P, $P, error) (bool, error)", - inputSymbol, outputSymbol - ); - - w.addUseImports(SmithyGoDependency.SMITHY_HTTP_TRANSPORT); - w.write("requestCloner func(interface{}) interface{}"); - - w.write("params $P", inputSymbol); - w.write(""); - }); } /** @@ -539,7 +376,7 @@ private void generateWaiterMiddleware( * @param waiterName the waiter name * @param waiter the waiter structure that contains info on modeled waiter */ - private void generateWaiterStateMutator( + private void generateRetryable( Model model, SymbolProvider symbolProvider, GoWriter writer, @@ -559,7 +396,7 @@ private void generateWaiterStateMutator( writer.write(""); writer.openBlock("func $L(ctx context.Context, input $P, output $P, err error) (bool, error) {", - "}", generateWaiterStateMutatorName(waiterName), inputSymbol, outputSymbol, () -> { + "}", generateRetryableName(waiterName), inputSymbol, outputSymbol, () -> { waiter.getAcceptors().forEach(acceptor -> { writer.write(""); // scope each acceptor to avoid name collisions @@ -797,13 +634,6 @@ private void writeMatchedAcceptorReturn(GoWriter writer, Acceptor acceptor) { } } - - private String generateApiClientInterfaceName( - Symbol operationSymbol - ) { - return String.format("%sWaiterAPIClient", operationSymbol.getName()); - } - private String generateWaiterOptionsName( String waiterName ) { @@ -818,18 +648,10 @@ private String generateWaiterClientName( return String.format("%sWaiter", waiterName); } - private String generateWaiterStateMutatorName( + private String generateRetryableName( String waiterName ) { waiterName = StringUtils.uncapitalize(waiterName); - return String.format("%sStateMutator", waiterName); + return String.format("%sStateRetryable", waiterName); } - - private String generateWaiterMiddlewareName( - String waiterName - ) { - waiterName = StringUtils.uncapitalize(waiterName); - return String.format("%sWaiterRetrier", waiterName); - } - } diff --git a/codegen/smithy-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration b/codegen/smithy-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration index aa4002376..35e3e7359 100644 --- a/codegen/smithy-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration +++ b/codegen/smithy-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration @@ -4,5 +4,6 @@ software.amazon.smithy.go.codegen.integration.AddChecksumRequiredMiddleware software.amazon.smithy.go.codegen.integration.RequiresLengthTraitSupport software.amazon.smithy.go.codegen.integration.EndpointHostPrefixMiddleware software.amazon.smithy.go.codegen.integration.ClientLogger +software.amazon.smithy.go.codegen.integration.InterfaceGenerator software.amazon.smithy.go.codegen.integration.Paginators software.amazon.smithy.go.codegen.integration.Waiters diff --git a/waiters/waiter.go b/waiters/waiter.go index b4e0bcc50..39ddac7ce 100644 --- a/waiters/waiter.go +++ b/waiters/waiter.go @@ -1,7 +1,6 @@ package waiters import ( - "context" "math" "time" ) @@ -20,20 +19,3 @@ func ComputeDelay(minDelay, maxDelay, remainingTime time.Duration, attempt int64 return delay } - -// SleepWithContext will wait for the timer duration to expire, or the context -// is canceled. Which ever happens first. If the context is canceled the -// Context's error will be returned. -func SleepWithContext(ctx context.Context, dur time.Duration) error { - t := time.NewTimer(dur) - defer t.Stop() - - select { - case <-t.C: - break - case <-ctx.Done(): - return ctx.Err() - } - - return nil -} From cc1c95c6692eadfeac49c17cdf6e51c0245bbb64 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Thu, 3 Dec 2020 02:47:40 -0800 Subject: [PATCH 11/23] update compute delay waiter util --- .../amazon/smithy/go/codegen/integration/Waiters.java | 7 +------ waiters/waiter.go | 8 ++++++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java index 006cfc74e..701bdae48 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java @@ -287,12 +287,7 @@ private void generateWaiterInvoker( writer.write("fmt.Errorf(\"maximum wait time for waiter must be greater than zero\")"); }).write(""); - writer.write("options := $T{}", waiterOptionsSymbol); - writer.write("options.APIOptions = w.options.APIOptions"); - writer.write("options.MinDelay = w.options.MinDelay"); - writer.write("options.MaxDelay = w.options.MaxDelay"); - writer.write("options.LogWaitAttempts = w.options.LogWaitAttempts"); - writer.write("options.Retryable = w.options.Retryable"); + writer.write("options := w.options"); writer.openBlock("for _, fn := range optFns {", "}", () -> { diff --git a/waiters/waiter.go b/waiters/waiter.go index 39ddac7ce..a8bdba380 100644 --- a/waiters/waiter.go +++ b/waiters/waiter.go @@ -6,11 +6,15 @@ import ( ) // ComputeDelay will compute delay between waiter attempts using the min delay, max delay, remaining time and -// current attempt count. Will return a delay value in seconds. +// current attempt count. Will return a computed delay. func ComputeDelay(minDelay, maxDelay, remainingTime time.Duration, attempt int64) time.Duration { + + // [0.0, 1.0) * 2 ^ attempt-1 + ri := float64(1 << uint64(attempt-1)) + delay := time.Duration(math.Min( maxDelay.Seconds(), - minDelay.Seconds()*math.Pow(2, float64(attempt-1))), + minDelay.Seconds()*ri), ) * time.Second if remainingTime-delay <= minDelay { From 06ecf6cecb0e630b6b2336d22c30abad0be8e0b9 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Thu, 3 Dec 2020 02:52:50 -0800 Subject: [PATCH 12/23] minor fix --- waiters/waiter.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/waiters/waiter.go b/waiters/waiter.go index a8bdba380..7908b6abd 100644 --- a/waiters/waiter.go +++ b/waiters/waiter.go @@ -10,11 +10,11 @@ import ( func ComputeDelay(minDelay, maxDelay, remainingTime time.Duration, attempt int64) time.Duration { // [0.0, 1.0) * 2 ^ attempt-1 - ri := float64(1 << uint64(attempt-1)) + ri := 1 << uint64(attempt-1) delay := time.Duration(math.Min( maxDelay.Seconds(), - minDelay.Seconds()*ri), + minDelay.Seconds()*float64(ri)), ) * time.Second if remainingTime-delay <= minDelay { From 48917fb92c9c9477615b07665b9d44eb2d2012f0 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Thu, 3 Dec 2020 15:50:03 -0800 Subject: [PATCH 13/23] update waiter util, naming and add tests --- ....java => OperationInterfaceGenerator.java} | 0 waiter/waiter.go | 41 ++++++++ waiter/waiter_test.go | 95 +++++++++++++++++++ waiters/waiter.go | 25 ----- 4 files changed, 136 insertions(+), 25 deletions(-) rename codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/{InterfaceGenerator.java => OperationInterfaceGenerator.java} (100%) create mode 100644 waiter/waiter.go create mode 100644 waiter/waiter_test.go delete mode 100644 waiters/waiter.go diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/InterfaceGenerator.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/OperationInterfaceGenerator.java similarity index 100% rename from codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/InterfaceGenerator.java rename to codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/OperationInterfaceGenerator.java diff --git a/waiter/waiter.go b/waiter/waiter.go new file mode 100644 index 000000000..f6c969103 --- /dev/null +++ b/waiter/waiter.go @@ -0,0 +1,41 @@ +package waiter + +import ( + "fmt" + "math" + "time" +) + +// ComputeDelay will compute delay between waiter attempts using the min delay, max delay, remaining time and +// current attempt count. The attempt count is the request call count. +// The zeroth attempt takes no delay. +func ComputeDelay(minDelay, maxDelay, remainingTime time.Duration, attempt int64) (time.Duration, error) { + // validation + if minDelay > maxDelay { + return 0, fmt.Errorf("maximum delay must be greater than minimum delay") + } + + // zeroth attempt, no delay + if attempt <= 0 { + return 0, nil + } + + // remainingTime is zero or less, no delay + if remainingTime <= 0 { + return 0, nil + } + + // [0.0, 1.0) * 2 ^ attempt-1 + ri := 1 << uint64(attempt-1) + + delay := time.Duration(math.Min( + maxDelay.Seconds(), + minDelay.Seconds()*float64(ri)), + ) * time.Second + + if remainingTime-delay <= minDelay { + delay = remainingTime - minDelay + } + + return delay, nil +} diff --git a/waiter/waiter_test.go b/waiter/waiter_test.go new file mode 100644 index 000000000..d2577387e --- /dev/null +++ b/waiter/waiter_test.go @@ -0,0 +1,95 @@ +package waiter + +import ( + "strings" + "testing" + "time" +) + +func TestComputeDelay(t *testing.T) { + cases := map[string]struct { + totalAttempts int64 + minDelay time.Duration + maxDelay time.Duration + maxWaitTime time.Duration + expectedDelays []time.Duration + expectedError string + }{ + "standard": { + totalAttempts: 8, + minDelay: 2 * time.Second, + maxDelay: 120 * time.Second, + maxWaitTime: 300 * time.Second, + expectedDelays: []time.Duration{2, 4, 8, 16, 32, 64, 120, 52}, + }, + "zero minDelay": { + totalAttempts: 3, + minDelay: 0, + maxDelay: 120 * time.Second, + maxWaitTime: 300 * time.Second, + expectedDelays: []time.Duration{0, 0, 0}, + }, + "zero maxDelay": { + totalAttempts: 3, + minDelay: 10 * time.Second, + maxDelay: 0, + maxWaitTime: 300 * time.Second, + expectedDelays: []time.Duration{0, 0, 0}, + expectedError: "maximum delay must be greater than minimum delay", + }, + "zero remaining time": { + totalAttempts: 3, + minDelay: 10 * time.Second, + maxWaitTime: 0, + expectedDelays: []time.Duration{0, 0, 0}, + }, + } + + for name, c := range cases { + t.Run(name, func(t *testing.T) { + var err error + var attempt int64 + var delays = make([]time.Duration, c.totalAttempts) + + remainingTime := c.maxWaitTime + + for { + attempt++ + + if c.totalAttempts < attempt { + break + } + + if remainingTime <= 0 { + break + } + + delay, e := ComputeDelay(c.minDelay, c.maxDelay, remainingTime, attempt) + if e != nil { + err = e + break + } + delays[attempt-1] = delay + + remainingTime -= delay + } + + if len(c.expectedError) != 0 { + if err == nil { + t.Fatalf("expected error, got none") + } + if e, a := c.expectedError, err.Error(); !strings.Contains(a, e) { + t.Fatalf("expected error %v, got %v instead", e, a) + } + } else if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + for i, expectedDelay := range c.expectedDelays { + if e, a := expectedDelay*time.Second, delays[i]; e != a { + t.Fatalf("attempt %d : expected delay to be %v, got %v", i+1, e, a) + } + } + }) + } +} diff --git a/waiters/waiter.go b/waiters/waiter.go deleted file mode 100644 index 7908b6abd..000000000 --- a/waiters/waiter.go +++ /dev/null @@ -1,25 +0,0 @@ -package waiters - -import ( - "math" - "time" -) - -// ComputeDelay will compute delay between waiter attempts using the min delay, max delay, remaining time and -// current attempt count. Will return a computed delay. -func ComputeDelay(minDelay, maxDelay, remainingTime time.Duration, attempt int64) time.Duration { - - // [0.0, 1.0) * 2 ^ attempt-1 - ri := 1 << uint64(attempt-1) - - delay := time.Duration(math.Min( - maxDelay.Seconds(), - minDelay.Seconds()*float64(ri)), - ) * time.Second - - if remainingTime-delay <= minDelay { - delay = remainingTime - minDelay - } - - return delay -} From 865d004ee7f1583578f4c7f1f414dca031c46df3 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Thu, 3 Dec 2020 16:16:18 -0800 Subject: [PATCH 14/23] feedback around naming of java class and go package --- .../smithy/go/codegen/SmithyGoDependency.java | 4 +- .../OperationInterfaceGenerator.java | 2 +- .../go/codegen/integration/Paginators.java | 2 +- .../go/codegen/integration/Waiters.java | 57 +++++++++++-------- ...mithy.go.codegen.integration.GoIntegration | 2 +- 5 files changed, 39 insertions(+), 28 deletions(-) diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java index 606a99669..5e8cea1fd 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java @@ -53,7 +53,7 @@ public final class SmithyGoDependency { public static final GoDependency SMITHY_RAND = smithy("rand", "smithyrand"); public static final GoDependency SMITHY_TESTING = smithy("testing", "smithytesting"); public static final GoDependency SMITHY_XML = smithy("xml", "smithyxml"); - public static final GoDependency SMITHY_WAITERS = smithy("waiters", "smithywaiters"); + public static final GoDependency SMITHY_WAITERS = smithy("waiter", "smithywaiter"); public static final GoDependency GO_CMP = goCmp("cmp"); public static final GoDependency GO_CMP_OPTIONS = goCmp("cmp/cmpopts"); @@ -118,7 +118,7 @@ private static GoDependency relativePackage( private static final class Versions { private static final String GO_STDLIB = "1.15"; private static final String GO_CMP = "v0.5.4"; - private static final String SMITHY_GO = "v0.4.1-0.20201203094142-96d63bb8c2ff"; + private static final String SMITHY_GO = "v0.4.1-0.20201203235003-48917fb92c9c"; private static final String GO_JMESPATH = "v0.4.0"; } } diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/OperationInterfaceGenerator.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/OperationInterfaceGenerator.java index 6c70d3cc5..58b008d0a 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/OperationInterfaceGenerator.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/OperationInterfaceGenerator.java @@ -36,7 +36,7 @@ /** * Generates API client Interfaces as per API operation. */ -public class InterfaceGenerator implements GoIntegration { +public class OperationInterfaceGenerator implements GoIntegration { private static Set listOfClientInterfaceOperations = new TreeSet<>(); diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Paginators.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Paginators.java index 0ba2cde3c..165af5ba7 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Paginators.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Paginators.java @@ -73,7 +73,7 @@ private void generateOperationPaginator( Symbol operationSymbol = symbolProvider.toSymbol(paginationInfo.getOperation()); Symbol interfaceSymbol = SymbolUtils.createValueSymbolBuilder( - InterfaceGenerator.getApiClientInterfaceName(operationSymbol) + OperationInterfaceGenerator.getApiClientInterfaceName(operationSymbol) ).build(); Symbol paginatorSymbol = SymbolUtils.createPointableSymbolBuilder(String.format("%sPaginator", operationSymbol.getName())).build(); diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java index 701bdae48..2c68c8263 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java @@ -197,7 +197,7 @@ private void generateWaiterClient( writer.openBlock("type $L struct {", "}", clientName, () -> { writer.write(""); - writer.write("client $L", InterfaceGenerator.getApiClientInterfaceName(operationSymbol)); + writer.write("client $L", OperationInterfaceGenerator.getApiClientInterfaceName(operationSymbol)); writer.write(""); writer.write("options $L", generateWaiterOptionsName(waiterName)); @@ -219,7 +219,7 @@ private void generateWaiterClient( String.format("%s constructs a %s.", constructorName, clientName) ); writer.openBlock("func $L(client $L, optFns ...func($P)) $P {", "}", - constructorName, InterfaceGenerator.getApiClientInterfaceName(operationSymbol), + constructorName, OperationInterfaceGenerator.getApiClientInterfaceName(operationSymbol), waiterOptionsSymbol, clientSymbol, () -> { writer.write("options := $T{}", waiterOptionsSymbol); writer.addUseImports(SmithyGoDependency.TIME); @@ -314,32 +314,41 @@ private void generateWaiterInvoker( }); writer.write(""); - writer.write("attempt++").write(""); + // handle retry attempt behavior + writer.openBlock("if attempt > 0 {", "}", () -> { + writer.write(""); - Symbol computeDelaySymbol = SymbolUtils.createValueSymbolBuilder( - "ComputeDelay", SmithyGoDependency.SMITHY_WAITERS - ).build(); - writer.writeDocs("compute exponential backoff between waiter retries"); - writer.openBlock("delay := $T(", ")", computeDelaySymbol, () -> { - writer.write("options.MinDelay, options.MaxDelay, remainingTime, attempt,"); - }).write(""); + Symbol computeDelaySymbol = SymbolUtils.createValueSymbolBuilder( + "ComputeDelay", SmithyGoDependency.SMITHY_WAITERS + ).build(); + + writer.writeDocs("compute exponential backoff between waiter retries"); + writer.openBlock("delay, err := $T(", ")", computeDelaySymbol, () -> { + writer.write("options.MinDelay, options.MaxDelay, remainingTime, attempt,"); + }); - writer.write("remainingTime -= delay"); + writer.write( + "if err != nil { return fmt.Errorf(\"error computing waiter delay, %w\", err)}"); + writer.write(""); - Symbol sleepWithContextSymbol = SymbolUtils.createValueSymbolBuilder( - "SleepWithContext", SmithyGoDependency.SMITHY_TIME - ).build(); - writer.writeDocs("sleep for the delay amount before invoking a request"); - writer.openBlock("if err := $T(ctx, delay); err != nil {", "}", - sleepWithContextSymbol, () -> { - writer.write("return fmt.Errorf(\"request cancelled while waiting, %w\", err)"); - }).write(""); + writer.write("remainingTime -= delay"); + + Symbol sleepWithContextSymbol = SymbolUtils.createValueSymbolBuilder( + "SleepWithContext", SmithyGoDependency.SMITHY_TIME + ).build(); + writer.writeDocs("sleep for the delay amount before invoking a request"); + writer.openBlock("if err := $T(ctx, delay); err != nil {", "}", sleepWithContextSymbol, + () -> { + writer.write( + "return fmt.Errorf(\"request cancelled while waiting, %w\", err)"); + }); + }).write(""); // enable logger writer.openBlock("if options.LogWaitAttempts {", "}", () -> { writer.addUseImports(SmithyGoDependency.SMITHY_LOGGING); writer.write("logger.Logf(logging.Debug, " - + "fmt.Sprintf(\"attempting waiter request, attempt count: %d\", attempt))\n"); + + "fmt.Sprintf(\"attempting waiter request, attempt count: %d\", attempt+1))"); }); writer.write(""); @@ -350,13 +359,15 @@ private void generateWaiterInvoker( }); writer.write(""); - // handle response and identify waiter state writer.write("retryable, err := options.Retryable(ctx, params, out, err)"); writer.write("if err != nil { return err }").write(""); - writer.write("if !retryable { return nil }"); + writer.write("if !retryable { return nil }").write(""); + + // increment the attempt counter for retry + writer.write("attempt++"); }); - writer.write("return fmt.Errorf(\"exceeded max wait time for $L waiter \")", waiterName); + writer.write("return fmt.Errorf(\"exceeded max wait time for $L waiter\")", waiterName); }); } diff --git a/codegen/smithy-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration b/codegen/smithy-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration index 35e3e7359..fccd43e43 100644 --- a/codegen/smithy-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration +++ b/codegen/smithy-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration @@ -4,6 +4,6 @@ software.amazon.smithy.go.codegen.integration.AddChecksumRequiredMiddleware software.amazon.smithy.go.codegen.integration.RequiresLengthTraitSupport software.amazon.smithy.go.codegen.integration.EndpointHostPrefixMiddleware software.amazon.smithy.go.codegen.integration.ClientLogger -software.amazon.smithy.go.codegen.integration.InterfaceGenerator +software.amazon.smithy.go.codegen.integration.OperationInterfaceGenerator software.amazon.smithy.go.codegen.integration.Paginators software.amazon.smithy.go.codegen.integration.Waiters From 6f950470a81fdc65c95964c711fbbbad9f7a023b Mon Sep 17 00:00:00 2001 From: skotambkar Date: Fri, 4 Dec 2020 18:50:28 -0800 Subject: [PATCH 15/23] updates jitter and delay computation for waiter --- time/time.go | 8 +++ waiter/waiter.go | 61 ++++++++++++++----- waiter/waiter_test.go | 137 +++++++++++++++++++++++++----------------- 3 files changed, 135 insertions(+), 71 deletions(-) diff --git a/time/time.go b/time/time.go index 0a3fbe48c..277adb136 100644 --- a/time/time.go +++ b/time/time.go @@ -71,3 +71,11 @@ func SleepWithContext(ctx context.Context, dur time.Duration) error { return nil } + +// DurationMin compares two time.Duration input values and returns the minimum time.Duration value +func DurationMin(a, b time.Duration) time.Duration { + if a < b { + return a + } + return b +} diff --git a/waiter/waiter.go b/waiter/waiter.go index f6c969103..a61bb1d2b 100644 --- a/waiter/waiter.go +++ b/waiter/waiter.go @@ -3,39 +3,68 @@ package waiter import ( "fmt" "math" + "math/rand" "time" + + smithytime "github.com/awslabs/smithy-go/time" ) -// ComputeDelay will compute delay between waiter attempts using the min delay, max delay, remaining time and -// current attempt count. The attempt count is the request call count. -// The zeroth attempt takes no delay. -func ComputeDelay(minDelay, maxDelay, remainingTime time.Duration, attempt int64) (time.Duration, error) { +// ComputeDelay computes delay between waiter attempts. The function takes in a current attempt count, +// minimum delay, maximum delay, and remaining wait time for waiter as input. +// +// Returns the computed delay and if next attempt count is possible within the given input time constraints. +// Note that the zeroth attempt results in no delay. +func ComputeDelay(attempt int64, minDelay, maxDelay, remainingTime time.Duration) (delay time.Duration, done bool, err error) { // validation if minDelay > maxDelay { - return 0, fmt.Errorf("maximum delay must be greater than minimum delay") + return 0, true, fmt.Errorf("maximum delay must be greater than minimum delay") } // zeroth attempt, no delay if attempt <= 0 { - return 0, nil + return 0, true, nil } // remainingTime is zero or less, no delay if remainingTime <= 0 { - return 0, nil + return 0, true, nil + } + + // as we use log, ensure min delay and maxdelay are atleast 1 ns + if minDelay < 1 { + minDelay = 1 } - // [0.0, 1.0) * 2 ^ attempt-1 - ri := 1 << uint64(attempt-1) + // if max delay is less than 1 ns, return 0 as delay + if maxDelay < 1 { + return 0, true, nil + } - delay := time.Duration(math.Min( - maxDelay.Seconds(), - minDelay.Seconds()*float64(ri)), - ) * time.Second + // check if this is the last attempt possible and compute delay accordingly + defer func() { + if remainingTime-delay <= minDelay { + delay = remainingTime - minDelay + done = true + } + }() + + // Get attempt ceiling to prevent integer overflow. + attemptCeiling := (math.Log(float64(maxDelay/minDelay)) / math.Log(2)) + 1 + + if attempt > int64(attemptCeiling) { + delay = maxDelay + } else { + // Compute exponential delay based on attempt. + // [0.0, 1.0) * 2 ^ attempt-1 + ri := 1 << uint64(attempt-1) + // compute delay + delay = smithytime.DurationMin(maxDelay, minDelay*time.Duration(ri)) + } - if remainingTime-delay <= minDelay { - delay = remainingTime - minDelay + if delay != minDelay { + // randomize to get jitter between min delay and delay value + delay = time.Duration(rand.Int63n(int64(delay-minDelay))) + minDelay } - return delay, nil + return delay, done, nil } diff --git a/waiter/waiter_test.go b/waiter/waiter_test.go index d2577387e..ed35b8f2c 100644 --- a/waiter/waiter_test.go +++ b/waiter/waiter_test.go @@ -8,71 +8,65 @@ import ( func TestComputeDelay(t *testing.T) { cases := map[string]struct { - totalAttempts int64 - minDelay time.Duration - maxDelay time.Duration - maxWaitTime time.Duration - expectedDelays []time.Duration - expectedError string + totalAttempts int64 + minDelay time.Duration + maxDelay time.Duration + maxWaitTime time.Duration + expectedMaxDelays []time.Duration + expectedError string + expectedMinAttempts int }{ "standard": { - totalAttempts: 8, - minDelay: 2 * time.Second, - maxDelay: 120 * time.Second, - maxWaitTime: 300 * time.Second, - expectedDelays: []time.Duration{2, 4, 8, 16, 32, 64, 120, 52}, + totalAttempts: 8, + minDelay: 2 * time.Second, + maxDelay: 120 * time.Second, + maxWaitTime: 300 * time.Second, + expectedMaxDelays: []time.Duration{2, 4, 8, 16, 32, 64, 120, 120}, + expectedMinAttempts: 8, }, "zero minDelay": { - totalAttempts: 3, - minDelay: 0, - maxDelay: 120 * time.Second, - maxWaitTime: 300 * time.Second, - expectedDelays: []time.Duration{0, 0, 0}, + totalAttempts: 3, + minDelay: 0, + maxDelay: 120 * time.Second, + maxWaitTime: 300 * time.Second, + expectedMaxDelays: []time.Duration{1, 1, 1}, + expectedMinAttempts: 3, }, "zero maxDelay": { - totalAttempts: 3, - minDelay: 10 * time.Second, - maxDelay: 0, - maxWaitTime: 300 * time.Second, - expectedDelays: []time.Duration{0, 0, 0}, - expectedError: "maximum delay must be greater than minimum delay", + totalAttempts: 3, + minDelay: 10 * time.Second, + maxDelay: 0, + maxWaitTime: 300 * time.Second, + expectedError: "maximum delay must be greater than minimum delay", }, "zero remaining time": { - totalAttempts: 3, - minDelay: 10 * time.Second, - maxWaitTime: 0, - expectedDelays: []time.Duration{0, 0, 0}, + totalAttempts: 3, + minDelay: 10 * time.Second, + maxDelay: 20 * time.Second, + maxWaitTime: 0, + expectedMaxDelays: []time.Duration{0}, + expectedMinAttempts: 1, + }, + "large minDelay": { + totalAttempts: 80, + minDelay: 150 * time.Minute, + maxDelay: 200 * time.Minute, + maxWaitTime: 250 * time.Minute, + expectedMinAttempts: 1, + }, + "large maxDelay": { + totalAttempts: 80, + minDelay: 15 * time.Minute, + maxDelay: 2000 * time.Minute, + maxWaitTime: 250 * time.Minute, + expectedMinAttempts: 5, }, } for name, c := range cases { t.Run(name, func(t *testing.T) { - var err error - var attempt int64 - var delays = make([]time.Duration, c.totalAttempts) - - remainingTime := c.maxWaitTime - - for { - attempt++ - - if c.totalAttempts < attempt { - break - } - - if remainingTime <= 0 { - break - } - - delay, e := ComputeDelay(c.minDelay, c.maxDelay, remainingTime, attempt) - if e != nil { - err = e - break - } - delays[attempt-1] = delay - - remainingTime -= delay - } + // mock waiter call + delays, err := mockwait(c.totalAttempts, c.minDelay, c.maxDelay, c.maxWaitTime) if len(c.expectedError) != 0 { if err == nil { @@ -85,11 +79,44 @@ func TestComputeDelay(t *testing.T) { t.Fatalf("expected no error, got %v", err) } - for i, expectedDelay := range c.expectedDelays { - if e, a := expectedDelay*time.Second, delays[i]; e != a { - t.Fatalf("attempt %d : expected delay to be %v, got %v", i+1, e, a) + if e, a := c.expectedMinAttempts, len(delays); e > a { + t.Logf("%v", delays) + t.Fatalf("expected minimum attempts to be %v, got %v", e, a) + } + + for i, expectedDelay := range c.expectedMaxDelays { + if e, a := expectedDelay*time.Second, delays[i]; e < a { + t.Fatalf("attempt %d : expected delay to be less than %v, got %v", i+1, e, a) } } }) } } + +func mockwait(maxAttempts int64, minDelay, maxDelay, maxWaitTime time.Duration) ([]time.Duration, error) { + delays := make([]time.Duration, 0) + remainingTime := maxWaitTime + var attempt int64 + + for { + attempt++ + + if maxAttempts < attempt { + break + } + + delay, done, err := ComputeDelay(attempt, minDelay, maxDelay, remainingTime) + if err != nil { + return delays, err + } + + delays = append(delays, delay) + if done { + break + } + + remainingTime -= delay + } + + return delays, nil +} From 0ff9d3f6150aa1addc6354424ec7fa9e644ccc41 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Fri, 4 Dec 2020 19:09:10 -0800 Subject: [PATCH 16/23] update to use change delay computation function --- .../amazon/smithy/go/codegen/SmithyGoDependency.java | 2 +- .../amazon/smithy/go/codegen/integration/Waiters.java | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java index 5e8cea1fd..7d7a2ab48 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java @@ -118,7 +118,7 @@ private static GoDependency relativePackage( private static final class Versions { private static final String GO_STDLIB = "1.15"; private static final String GO_CMP = "v0.5.4"; - private static final String SMITHY_GO = "v0.4.1-0.20201203235003-48917fb92c9c"; + private static final String SMITHY_GO = "v0.4.1-0.20201205025028-6f950470a81f"; private static final String GO_JMESPATH = "v0.4.0"; } } diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java index 2c68c8263..3e9564ac2 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java @@ -323,14 +323,18 @@ private void generateWaiterInvoker( ).build(); writer.writeDocs("compute exponential backoff between waiter retries"); - writer.openBlock("delay, err := $T(", ")", computeDelaySymbol, () -> { - writer.write("options.MinDelay, options.MaxDelay, remainingTime, attempt,"); + writer.openBlock("delay, done, err := $T(", ")", computeDelaySymbol, () -> { + writer.write("attempt, options.MinDelay, options.MaxDelay, remainingTime,"); }); writer.write( "if err != nil { return fmt.Errorf(\"error computing waiter delay, %w\", err)}"); writer.write(""); + writer.openBlock("if done {", "}", () -> { + writer.write("break"); + }); + writer.write("remainingTime -= delay"); Symbol sleepWithContextSymbol = SymbolUtils.createValueSymbolBuilder( From c4c9a5d17dbbf833d54b6329835b25694108e16c Mon Sep 17 00:00:00 2001 From: skotambkar Date: Fri, 4 Dec 2020 19:47:14 -0800 Subject: [PATCH 17/23] fix operation interface generator for incorrect exceptions --- .../OperationInterfaceGenerator.java | 52 +++++++++++-------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/OperationInterfaceGenerator.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/OperationInterfaceGenerator.java index 58b008d0a..ded793b6b 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/OperationInterfaceGenerator.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/OperationInterfaceGenerator.java @@ -15,9 +15,10 @@ package software.amazon.smithy.go.codegen.integration; +import java.util.HashMap; +import java.util.Map; import java.util.Set; import java.util.TreeSet; -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; @@ -38,7 +39,19 @@ */ public class OperationInterfaceGenerator implements GoIntegration { - private static Set listOfClientInterfaceOperations = new TreeSet<>(); + private static Map> mapOfClientInterfaceOperations = new HashMap<>(); + + /** + * Returns name of an API client interface. + * + * @param operationSymbol Symbol of operation shape for which Api client interface is being generated. + * @return name of the interface. + */ + public static String getApiClientInterfaceName( + Symbol operationSymbol + ) { + return String.format("%sAPIClient", operationSymbol.getName()); + } @Override public void processFinalizedModel( @@ -47,6 +60,7 @@ public void processFinalizedModel( ) { ServiceShape serviceShape = settings.getService(model); TopDownIndex topDownIndex = TopDownIndex.of(model); + Set listOfClientInterfaceOperations = new TreeSet<>(); // fetch operations for which paginated trait is applied topDownIndex.getContainedOperations(serviceShape).stream() @@ -55,7 +69,7 @@ public void processFinalizedModel( if (serviceShape.hasTrait(PaginatedTrait.class)) { topDownIndex.getContainedOperations(serviceShape).stream() - .forEach(operationShape -> listOfClientInterfaceOperations.add(operationShape.getId())); + .forEach(operationShape -> listOfClientInterfaceOperations.add(operationShape.toShapeId())); } // fetch operations for which waitable trait is applied @@ -63,12 +77,11 @@ public void processFinalizedModel( .filter(operationShape -> operationShape.hasTrait(WaitableTrait.class)) .forEach(operationShape -> listOfClientInterfaceOperations.add(operationShape.getId())); - if (listOfClientInterfaceOperations.isEmpty()) { - throw new CodegenException("empty operations"); + if (!listOfClientInterfaceOperations.isEmpty()) { + mapOfClientInterfaceOperations.put(serviceShape.getId(), listOfClientInterfaceOperations); } } - @Override public void writeAdditionalFiles( GoSettings settings, @@ -76,12 +89,17 @@ public void writeAdditionalFiles( SymbolProvider symbolProvider, GoDelegator goDelegator ) { - listOfClientInterfaceOperations.stream().forEach(shapeId -> { - OperationShape operationShape = model.expectShape(shapeId, OperationShape.class); - goDelegator.useShapeWriter(operationShape, writer -> { - generateApiClientInterface(writer, model, symbolProvider, operationShape); + ShapeId serviceId = settings.getService(model).getId(); + + if (mapOfClientInterfaceOperations.containsKey(serviceId)) { + Set listOfClientInterfaceOperations = mapOfClientInterfaceOperations.get(serviceId); + listOfClientInterfaceOperations.stream().forEach(shapeId -> { + OperationShape operationShape = model.expectShape(shapeId, OperationShape.class); + goDelegator.useShapeWriter(operationShape, writer -> { + generateApiClientInterface(writer, model, symbolProvider, operationShape); + }); }); - }); + } } private void generateApiClientInterface( @@ -111,16 +129,4 @@ private void generateApiClientInterface( writer.write("var _ $T = (*Client)(nil)", interfaceSymbol); writer.write(""); } - - /** - * Returns name of an API client interface. - * - * @param operationSymbol Symbol of operation shape for which Api client interface is being generated. - * @return name of the interface. - */ - public static String getApiClientInterfaceName( - Symbol operationSymbol - ) { - return String.format("%sAPIClient", operationSymbol.getName()); - } } From b8cdbaa577ffb49e7d12da2c32b206f742ff656e Mon Sep 17 00:00:00 2001 From: skotambkar Date: Tue, 8 Dec 2020 15:29:24 -0800 Subject: [PATCH 18/23] update waiter compute delay util and adds waiter logger middleware --- rand/rand.go | 31 ++++++++++++++++++++++++ time/time.go | 8 ------- waiter/logger.go | 35 +++++++++++++++++++++++++++ waiter/waiter.go | 56 ++++++++++++++++++++----------------------- waiter/waiter_test.go | 33 ++++++++++++++++--------- 5 files changed, 114 insertions(+), 49 deletions(-) create mode 100644 rand/rand.go create mode 100644 waiter/logger.go diff --git a/rand/rand.go b/rand/rand.go new file mode 100644 index 000000000..4dc176d7d --- /dev/null +++ b/rand/rand.go @@ -0,0 +1,31 @@ +package rand + +import ( + "crypto/rand" + "fmt" + "io" + "math/big" +) + +func init() { + Reader = rand.Reader +} + +// Reader provides a random reader that can reset during testing. +var Reader io.Reader + +// Int63n returns a int64 between zero and value of max, read from an io.Reader source. +func Int63n(reader io.Reader, max int64) (int64, error) { + bi, err := rand.Int(reader, big.NewInt(max)) + if err != nil { + return 0, fmt.Errorf("failed to read random value, %w", err) + } + + return bi.Int64(), nil +} + +// CryptoRandInt63n returns a random int64 between zero and value of max +//obtained from the crypto rand source. +func CryptoRandInt63n(max int64) (int64, error) { + return Int63n(Reader, max) +} diff --git a/time/time.go b/time/time.go index 277adb136..0a3fbe48c 100644 --- a/time/time.go +++ b/time/time.go @@ -71,11 +71,3 @@ func SleepWithContext(ctx context.Context, dur time.Duration) error { return nil } - -// DurationMin compares two time.Duration input values and returns the minimum time.Duration value -func DurationMin(a, b time.Duration) time.Duration { - if a < b { - return a - } - return b -} diff --git a/waiter/logger.go b/waiter/logger.go new file mode 100644 index 000000000..4853ace01 --- /dev/null +++ b/waiter/logger.go @@ -0,0 +1,35 @@ +package waiter + +import ( + "context" + "fmt" + "github.com/awslabs/smithy-go/logging" + "github.com/awslabs/smithy-go/middleware" +) + +// Logger is the Logger middleware used by the waiter to log an attempt +type Logger struct { + // Attempt is the current attempt to be logged + Attempt int64 +} + +// ID representing the Logger middleware +func (*Logger) ID() string { + return "WaiterLogger" +} + +// HandleInitialize performs handling of request in initialize stack step +func (m *Logger) HandleInitialize(ctx context.Context, in middleware.InitializeInput, next middleware.InitializeHandler) ( + out middleware.InitializeOutput, metadata middleware.Metadata, err error, +) { + logger := middleware.GetLogger(ctx) + + logger.Logf(logging.Debug, fmt.Sprintf("attempting waiter request, attempt count: %d", m.Attempt)) + + return next.HandleInitialize(ctx, in) +} + +// AddLogger is helper util to add waiter logger after `SetLogger` middleware in +func (m Logger) AddLogger(stack *middleware.Stack) error { + return stack.Initialize.Insert(&m, "SetLogger", middleware.After) +} diff --git a/waiter/waiter.go b/waiter/waiter.go index a61bb1d2b..05b71cd30 100644 --- a/waiter/waiter.go +++ b/waiter/waiter.go @@ -2,52 +2,38 @@ package waiter import ( "fmt" + "github.com/awslabs/smithy-go/rand" "math" - "math/rand" "time" - - smithytime "github.com/awslabs/smithy-go/time" ) // ComputeDelay computes delay between waiter attempts. The function takes in a current attempt count, -// minimum delay, maximum delay, and remaining wait time for waiter as input. +// minimum delay, maximum delay, and remaining wait time for waiter as input. The inputs minDelay and maxDelay +// must always be greater than 0, along with minDelay lesser than or equal to maxDelay. // // Returns the computed delay and if next attempt count is possible within the given input time constraints. // Note that the zeroth attempt results in no delay. -func ComputeDelay(attempt int64, minDelay, maxDelay, remainingTime time.Duration) (delay time.Duration, done bool, err error) { - // validation - if minDelay > maxDelay { - return 0, true, fmt.Errorf("maximum delay must be greater than minimum delay") - } - +func ComputeDelay(attempt int64, minDelay, maxDelay, remainingTime time.Duration) (delay time.Duration, err error) { // zeroth attempt, no delay if attempt <= 0 { - return 0, true, nil + return 0, nil } // remainingTime is zero or less, no delay if remainingTime <= 0 { - return 0, true, nil + return 0, nil } - // as we use log, ensure min delay and maxdelay are atleast 1 ns - if minDelay < 1 { - minDelay = 1 + // validate min delay is greater than 0 + if minDelay == 0 { + return 0, fmt.Errorf("minDelay must be greater than zero when computing Delay") } - // if max delay is less than 1 ns, return 0 as delay - if maxDelay < 1 { - return 0, true, nil + // validate max delay is greater than 0 + if maxDelay == 0 { + return 0, fmt.Errorf("maxDelay must be greater than zero when computing Delay") } - // check if this is the last attempt possible and compute delay accordingly - defer func() { - if remainingTime-delay <= minDelay { - delay = remainingTime - minDelay - done = true - } - }() - // Get attempt ceiling to prevent integer overflow. attemptCeiling := (math.Log(float64(maxDelay/minDelay)) / math.Log(2)) + 1 @@ -55,16 +41,26 @@ func ComputeDelay(attempt int64, minDelay, maxDelay, remainingTime time.Duration delay = maxDelay } else { // Compute exponential delay based on attempt. - // [0.0, 1.0) * 2 ^ attempt-1 ri := 1 << uint64(attempt-1) // compute delay - delay = smithytime.DurationMin(maxDelay, minDelay*time.Duration(ri)) + delay = minDelay * time.Duration(ri) } if delay != minDelay { // randomize to get jitter between min delay and delay value - delay = time.Duration(rand.Int63n(int64(delay-minDelay))) + minDelay + // [0.0, 1.0) * [minDelay, delay] + d, err := rand.CryptoRandInt63n(int64(delay - minDelay)) + if err != nil { + return 0, fmt.Errorf("error computing retry jitter, %w", err) + } + + delay = time.Duration(d) + minDelay + } + + // check if this is the last attempt possible and compute delay accordingly + if remainingTime-delay <= minDelay { + delay = remainingTime - minDelay } - return delay, done, nil + return delay, nil } diff --git a/waiter/waiter_test.go b/waiter/waiter_test.go index ed35b8f2c..949c50dfd 100644 --- a/waiter/waiter_test.go +++ b/waiter/waiter_test.go @@ -25,19 +25,18 @@ func TestComputeDelay(t *testing.T) { expectedMinAttempts: 8, }, "zero minDelay": { - totalAttempts: 3, - minDelay: 0, - maxDelay: 120 * time.Second, - maxWaitTime: 300 * time.Second, - expectedMaxDelays: []time.Duration{1, 1, 1}, - expectedMinAttempts: 3, + totalAttempts: 3, + minDelay: 0, + maxDelay: 120 * time.Second, + maxWaitTime: 300 * time.Second, + expectedError: "minDelay must be greater than zero", }, "zero maxDelay": { totalAttempts: 3, minDelay: 10 * time.Second, maxDelay: 0, maxWaitTime: 300 * time.Second, - expectedError: "maximum delay must be greater than minimum delay", + expectedError: "maxDelay must be greater than zero", }, "zero remaining time": { totalAttempts: 3, @@ -47,6 +46,14 @@ func TestComputeDelay(t *testing.T) { expectedMaxDelays: []time.Duration{0}, expectedMinAttempts: 1, }, + "max wait time is less than min delay": { + totalAttempts: 3, + minDelay: 10 * time.Second, + maxDelay: 20 * time.Second, + maxWaitTime: 5 * time.Second, + expectedMaxDelays: []time.Duration{0}, + expectedMinAttempts: 1, + }, "large minDelay": { totalAttempts: 80, minDelay: 150 * time.Minute, @@ -88,6 +95,10 @@ func TestComputeDelay(t *testing.T) { if e, a := expectedDelay*time.Second, delays[i]; e < a { t.Fatalf("attempt %d : expected delay to be less than %v, got %v", i+1, e, a) } + + if e, a := c.minDelay, delays[i]; e > a && c.maxWaitTime > c.minDelay { + t.Fatalf("attempt %d : expected delay to be more than %v, got %v", i+1, e, a) + } } }) } @@ -105,17 +116,17 @@ func mockwait(maxAttempts int64, minDelay, maxDelay, maxWaitTime time.Duration) break } - delay, done, err := ComputeDelay(attempt, minDelay, maxDelay, remainingTime) + delay, err := ComputeDelay(attempt, minDelay, maxDelay, remainingTime) if err != nil { return delays, err } delays = append(delays, delay) - if done { - break - } remainingTime -= delay + if remainingTime < minDelay { + break + } } return delays, nil From 3806a96ca1bb34297706d732a3fb9e9b4c706a68 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Tue, 8 Dec 2020 15:47:20 -0800 Subject: [PATCH 19/23] update java code as per feedback --- .../smithy/go/codegen/SmithyGoDependency.java | 2 +- .../go/codegen/integration/Waiters.java | 87 ++++++++++++------- 2 files changed, 56 insertions(+), 33 deletions(-) diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java index 7d7a2ab48..db7906f07 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java @@ -118,7 +118,7 @@ private static GoDependency relativePackage( private static final class Versions { private static final String GO_STDLIB = "1.15"; private static final String GO_CMP = "v0.5.4"; - private static final String SMITHY_GO = "v0.4.1-0.20201205025028-6f950470a81f"; + private static final String SMITHY_GO = "v0.4.1-0.20201208232924-b8cdbaa577ff"; private static final String GO_JMESPATH = "v0.4.0"; } } diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java index 3e9564ac2..7f13c4f66 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java @@ -82,7 +82,7 @@ private void generateOperationWaiter( // generate waiter function waiters.forEach((name, waiter) -> { // write waiter options - generateWaiterOptions(model, symbolProvider, writer, operation, name); + generateWaiterOptions(model, symbolProvider, writer, operation, name, waiter); // write waiter client generateWaiterClient(model, symbolProvider, writer, operation, name, waiter); @@ -104,7 +104,8 @@ private void generateWaiterOptions( SymbolProvider symbolProvider, GoWriter writer, OperationShape operationShape, - String waiterName + String waiterName, + Waiter waiter ) { String optionsName = generateWaiterOptionsName(waiterName); String waiterClientName = generateWaiterClientName(waiterName); @@ -140,11 +141,21 @@ private void generateWaiterOptions( writer.write("APIOptions []func($P) error", stackSymbol); writer.write(""); - writer.writeDocs("MinDelay is the minimum amount of time to delay between retries"); + writer.writeDocs( + String.format("MinDelay is the minimum amount of time to delay between retries. " + + "If unset, %s will use default minimum delay of %s seconds. " + + "Note that MinDelay must resolve to a value lesser than or equal " + + "to the MaxDelay.", waiterName, waiter.getMinDelay()) + ); writer.write("MinDelay time.Duration"); writer.write(""); - writer.writeDocs("MaxDelay is the maximum amount of time to delay between retries"); + writer.writeDocs( + String.format("MaxDelay is the maximum amount of time to delay between retries. " + + "If unset or set to zero, %s will use default max delay of %s seconds. " + + "Note that MaxDelay must resolve to value greater than or equal " + + "to the MinDelay.", waiter.getMaxDelay(), waiterName) + ); writer.write("MaxDelay time.Duration"); writer.write(""); @@ -275,14 +286,17 @@ private void generateWaiterInvoker( writer.addUseImports(SmithyGoDependency.CONTEXT); writer.addUseImports(SmithyGoDependency.TIME); writer.writeDocs( - String.format("%s calls the waiter function for %s waiter", WAITER_INVOKER_FUNCTION_NAME, waiterName) + String.format( + "%s calls the waiter function for %s waiter. The maxWaitDur is the maximum wait duration " + + "the waiter will wait. The maxWaitDur is required and must be greater than zero.", + WAITER_INVOKER_FUNCTION_NAME, waiterName) ); writer.openBlock( - "func (w $P) $L(ctx context.Context, params $P, maxWaitTime time.Duration, optFns ...func($P)) error {", + "func (w $P) $L(ctx context.Context, params $P, maxWaitDur time.Duration, optFns ...func($P)) error {", "}", clientSymbol, WAITER_INVOKER_FUNCTION_NAME, inputSymbol, waiterOptionsSymbol, () -> { - writer.openBlock("if maxWaitTime == 0 {", "}", () -> { + writer.openBlock("if maxWaitDur == 0 {", "}", () -> { writer.addUseImports(SmithyGoDependency.FMT); writer.write("fmt.Errorf(\"maximum wait time for waiter must be greater than zero\")"); }).write(""); @@ -293,29 +307,42 @@ private void generateWaiterInvoker( "}", () -> { writer.write("fn(&options)"); }); + writer.write(""); - writer.addUseImports(SmithyGoDependency.TIME); - writer.addUseImports(SmithyGoDependency.CONTEXT); + // validate values for MaxDelay from options + writer.openBlock("if options.MaxDelay == 0 {", "}", () -> { + writer.write("options.MaxDelay = $L * time.Second", waiter.getMaxDelay()); + }); + writer.write(""); - writer.write("logger := middleware.GetLogger(ctx)").write(""); - writer.write("var attempt int64"); - writer.write("var remainingTime = maxWaitTime"); + // validate that MinDelay is lesser than or equal to resolved MaxDelay + writer.openBlock("if options.MinDelay > options.MaxDelay {", "}", () -> { + writer.addUseImports(SmithyGoDependency.FMT); + writer.write("return fmt.Errorf(\"minimum waiter delay %v must be lesser than or equal to " + + "maximum waiter delay of %v.\", options.MinDelay, options.MaxDelay)"); + }).write(""); - writer.write("deadline := time.Now().Add(maxWaitTime)"); - writer.write("ctx, cancelFn := context.WithDeadline(ctx, deadline)"); + + writer.addUseImports(SmithyGoDependency.CONTEXT); + writer.write("ctx, cancelFn := context.WithTimeout(ctx, maxWaitDur)"); writer.write("defer cancelFn()"); - writer.openBlock("for {", "}", () -> { - writer.write(""); + writer.write(""); - writer.addUseImports(SmithyGoDependency.FMT); - writer.openBlock("if remainingTime <= 0 {", "}", () -> { - writer.write("return fmt.Errorf(\"exceeded maximum wait time for $L waiter\")", - waiterName); - }); + Symbol loggerMiddleware = SymbolUtils.createValueSymbolBuilder( + "Logger", SmithyGoDependency.SMITHY_WAITERS + ).build(); + writer.write("logger := $T{}", loggerMiddleware); + writer.write("remainingTime := maxWaitDur").write(""); + + writer.write("var attempt int64"); + writer.openBlock("for {", "}", () -> { writer.write(""); - // handle retry attempt behavior - writer.openBlock("if attempt > 0 {", "}", () -> { + // handle retry delay computation, sleep. Skip if minDelay is lesser than or equal to 0. + writer.openBlock("if attempt > 0 && options.MinDelay > 0 {", "}", () -> { + writer.openBlock("if remainingTime < options.MinDelay || remainingTime <= 0 {", "}", () -> { + writer.write("break"); + }); writer.write(""); Symbol computeDelaySymbol = SymbolUtils.createValueSymbolBuilder( @@ -323,18 +350,15 @@ private void generateWaiterInvoker( ).build(); writer.writeDocs("compute exponential backoff between waiter retries"); - writer.openBlock("delay, done, err := $T(", ")", computeDelaySymbol, () -> { + writer.openBlock("delay, err := $T(", ")", computeDelaySymbol, () -> { writer.write("attempt, options.MinDelay, options.MaxDelay, remainingTime,"); }); + writer.addUseImports(SmithyGoDependency.FMT); writer.write( "if err != nil { return fmt.Errorf(\"error computing waiter delay, %w\", err)}"); writer.write(""); - writer.openBlock("if done {", "}", () -> { - writer.write("break"); - }); - writer.write("remainingTime -= delay"); Symbol sleepWithContextSymbol = SymbolUtils.createValueSymbolBuilder( @@ -348,11 +372,10 @@ private void generateWaiterInvoker( }); }).write(""); - // enable logger + // add waiter logger middleware to log an attempt, if LogWaitAttempts is enabled. writer.openBlock("if options.LogWaitAttempts {", "}", () -> { - writer.addUseImports(SmithyGoDependency.SMITHY_LOGGING); - writer.write("logger.Logf(logging.Debug, " - + "fmt.Sprintf(\"attempting waiter request, attempt count: %d\", attempt+1))"); + writer.write("logger.Attempt = attempt +1"); + writer.write("options.APIOptions = append(options.APIOptions, logger.AddLogger)"); }); writer.write(""); From 23a971924cc1b30bc064c2a59359339c6a602dcb Mon Sep 17 00:00:00 2001 From: skotambkar Date: Tue, 8 Dec 2020 17:57:50 -0800 Subject: [PATCH 20/23] use paginationIndex for paginator interface generation --- .../OperationInterfaceGenerator.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/OperationInterfaceGenerator.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/OperationInterfaceGenerator.java index ded793b6b..57827cf0e 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/OperationInterfaceGenerator.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/OperationInterfaceGenerator.java @@ -17,6 +17,7 @@ import java.util.HashMap; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.TreeSet; import software.amazon.smithy.codegen.core.Symbol; @@ -27,11 +28,11 @@ import software.amazon.smithy.go.codegen.SmithyGoDependency; import software.amazon.smithy.go.codegen.SymbolUtils; import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.knowledge.PaginatedIndex; import software.amazon.smithy.model.knowledge.TopDownIndex; import software.amazon.smithy.model.shapes.OperationShape; import software.amazon.smithy.model.shapes.ServiceShape; import software.amazon.smithy.model.shapes.ShapeId; -import software.amazon.smithy.model.traits.PaginatedTrait; import software.amazon.smithy.waiters.WaitableTrait; /** @@ -60,17 +61,16 @@ public void processFinalizedModel( ) { ServiceShape serviceShape = settings.getService(model); TopDownIndex topDownIndex = TopDownIndex.of(model); + PaginatedIndex paginatedIndex = PaginatedIndex.of(model); + Set listOfClientInterfaceOperations = new TreeSet<>(); - // fetch operations for which paginated trait is applied + // fetch operations for which paginators are generated topDownIndex.getContainedOperations(serviceShape).stream() - .filter(operationShape -> operationShape.hasTrait(PaginatedTrait.class)) - .forEach(operationShape -> listOfClientInterfaceOperations.add(operationShape.getId())); - - if (serviceShape.hasTrait(PaginatedTrait.class)) { - topDownIndex.getContainedOperations(serviceShape).stream() - .forEach(operationShape -> listOfClientInterfaceOperations.add(operationShape.toShapeId())); - } + .map(operationShape -> paginatedIndex.getPaginationInfo(serviceShape, operationShape)) + .filter(Optional::isPresent) + .map(Optional::get) + .forEach(paginationInfo -> listOfClientInterfaceOperations.add(paginationInfo.getOperation().getId())); // fetch operations for which waitable trait is applied topDownIndex.getContainedOperations(serviceShape).stream() From 6cc9c9626d2d128dbb03a757eef583dd4d3e1fb1 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Wed, 9 Dec 2020 10:18:48 -0800 Subject: [PATCH 21/23] doc update --- .../amazon/smithy/go/codegen/integration/Waiters.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java index 7f13c4f66..70d07fd8e 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java @@ -145,7 +145,7 @@ private void generateWaiterOptions( String.format("MinDelay is the minimum amount of time to delay between retries. " + "If unset, %s will use default minimum delay of %s seconds. " + "Note that MinDelay must resolve to a value lesser than or equal " - + "to the MaxDelay.", waiterName, waiter.getMinDelay()) + + "to the MaxDelay.", waiterClientName, waiter.getMinDelay()) ); writer.write("MinDelay time.Duration"); @@ -154,7 +154,7 @@ private void generateWaiterOptions( String.format("MaxDelay is the maximum amount of time to delay between retries. " + "If unset or set to zero, %s will use default max delay of %s seconds. " + "Note that MaxDelay must resolve to value greater than or equal " - + "to the MinDelay.", waiter.getMaxDelay(), waiterName) + + "to the MinDelay.", waiterClientName, waiter.getMaxDelay()) ); writer.write("MaxDelay time.Duration"); From 2bbbd84b5faf3dbda83e658e8f43afd9c3277b90 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Wed, 9 Dec 2020 11:16:12 -0800 Subject: [PATCH 22/23] feedback around remainingTime computation --- .../amazon/smithy/go/codegen/integration/Waiters.java | 10 ++++++---- waiter/waiter.go | 1 - waiter/waiter_test.go | 10 ++++++++++ 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java index 70d07fd8e..9affdaf1d 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java @@ -296,7 +296,7 @@ private void generateWaiterInvoker( "}", clientSymbol, WAITER_INVOKER_FUNCTION_NAME, inputSymbol, waiterOptionsSymbol, () -> { - writer.openBlock("if maxWaitDur == 0 {", "}", () -> { + writer.openBlock("if maxWaitDur <= 0 {", "}", () -> { writer.addUseImports(SmithyGoDependency.FMT); writer.write("fmt.Errorf(\"maximum wait time for waiter must be greater than zero\")"); }).write(""); @@ -310,7 +310,7 @@ private void generateWaiterInvoker( writer.write(""); // validate values for MaxDelay from options - writer.openBlock("if options.MaxDelay == 0 {", "}", () -> { + writer.openBlock("if options.MaxDelay <= 0 {", "}", () -> { writer.write("options.MaxDelay = $L * time.Second", waiter.getMaxDelay()); }); writer.write(""); @@ -337,6 +337,7 @@ private void generateWaiterInvoker( writer.write("var attempt int64"); writer.openBlock("for {", "}", () -> { writer.write(""); + writer.write("start := time.Now()"); // handle retry delay computation, sleep. Skip if minDelay is lesser than or equal to 0. writer.openBlock("if attempt > 0 && options.MinDelay > 0 {", "}", () -> { @@ -359,8 +360,6 @@ private void generateWaiterInvoker( "if err != nil { return fmt.Errorf(\"error computing waiter delay, %w\", err)}"); writer.write(""); - writer.write("remainingTime -= delay"); - Symbol sleepWithContextSymbol = SymbolUtils.createValueSymbolBuilder( "SleepWithContext", SmithyGoDependency.SMITHY_TIME ).build(); @@ -391,6 +390,9 @@ private void generateWaiterInvoker( writer.write("if err != nil { return err }").write(""); writer.write("if !retryable { return nil }").write(""); + // update remaining time + writer.write("remainingTime -= time.Since(start)"); + // increment the attempt counter for retry writer.write("attempt++"); }); diff --git a/waiter/waiter.go b/waiter/waiter.go index 05b71cd30..4b35a96ff 100644 --- a/waiter/waiter.go +++ b/waiter/waiter.go @@ -48,7 +48,6 @@ func ComputeDelay(attempt int64, minDelay, maxDelay, remainingTime time.Duration if delay != minDelay { // randomize to get jitter between min delay and delay value - // [0.0, 1.0) * [minDelay, delay] d, err := rand.CryptoRandInt63n(int64(delay - minDelay)) if err != nil { return 0, fmt.Errorf("error computing retry jitter, %w", err) diff --git a/waiter/waiter_test.go b/waiter/waiter_test.go index 949c50dfd..9f749869b 100644 --- a/waiter/waiter_test.go +++ b/waiter/waiter_test.go @@ -1,6 +1,8 @@ package waiter import ( + "github.com/awslabs/smithy-go/rand" + mathrand "math/rand" "strings" "testing" "time" @@ -72,6 +74,13 @@ func TestComputeDelay(t *testing.T) { for name, c := range cases { t.Run(name, func(t *testing.T) { + // mock smithy-go rand/#Reader + r := rand.Reader + defer func() { + rand.Reader = r + }() + rand.Reader = mathrand.New(mathrand.NewSource(1)) + // mock waiter call delays, err := mockwait(c.totalAttempts, c.minDelay, c.maxDelay, c.maxWaitTime) @@ -100,6 +109,7 @@ func TestComputeDelay(t *testing.T) { t.Fatalf("attempt %d : expected delay to be more than %v, got %v", i+1, e, a) } } + t.Logf("delays : %v", delays) }) } } From 1d741e4fd4c12673ebf6e5e799a3cc28f495d1c8 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Wed, 9 Dec 2020 13:22:20 -0800 Subject: [PATCH 23/23] update waiter call workflow --- .../go/codegen/integration/Waiters.java | 96 ++++++++++--------- 1 file changed, 49 insertions(+), 47 deletions(-) diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java index 9affdaf1d..1d07dd2b2 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/Waiters.java @@ -143,18 +143,18 @@ private void generateWaiterOptions( writer.write(""); writer.writeDocs( String.format("MinDelay is the minimum amount of time to delay between retries. " - + "If unset, %s will use default minimum delay of %s seconds. " - + "Note that MinDelay must resolve to a value lesser than or equal " - + "to the MaxDelay.", waiterClientName, waiter.getMinDelay()) + + "If unset, %s will use default minimum delay of %s seconds. " + + "Note that MinDelay must resolve to a value lesser than or equal " + + "to the MaxDelay.", waiterClientName, waiter.getMinDelay()) ); writer.write("MinDelay time.Duration"); writer.write(""); writer.writeDocs( String.format("MaxDelay is the maximum amount of time to delay between retries. " - + "If unset or set to zero, %s will use default max delay of %s seconds. " - + "Note that MaxDelay must resolve to value greater than or equal " - + "to the MinDelay.", waiterClientName, waiter.getMaxDelay()) + + "If unset or set to zero, %s will use default max delay of %s seconds. " + + "Note that MaxDelay must resolve to value greater than or equal " + + "to the MinDelay.", waiterClientName, waiter.getMaxDelay()) ); writer.write("MaxDelay time.Duration"); @@ -322,7 +322,6 @@ private void generateWaiterInvoker( + "maximum waiter delay of %v.\", options.MinDelay, options.MaxDelay)"); }).write(""); - writer.addUseImports(SmithyGoDependency.CONTEXT); writer.write("ctx, cancelFn := context.WithTimeout(ctx, maxWaitDur)"); writer.write("defer cancelFn()"); @@ -337,64 +336,67 @@ private void generateWaiterInvoker( writer.write("var attempt int64"); writer.openBlock("for {", "}", () -> { writer.write(""); - writer.write("start := time.Now()"); - - // handle retry delay computation, sleep. Skip if minDelay is lesser than or equal to 0. - writer.openBlock("if attempt > 0 && options.MinDelay > 0 {", "}", () -> { - writer.openBlock("if remainingTime < options.MinDelay || remainingTime <= 0 {", "}", () -> { - writer.write("break"); - }); - writer.write(""); - - Symbol computeDelaySymbol = SymbolUtils.createValueSymbolBuilder( - "ComputeDelay", SmithyGoDependency.SMITHY_WAITERS - ).build(); + writer.write("attempt++"); - writer.writeDocs("compute exponential backoff between waiter retries"); - writer.openBlock("delay, err := $T(", ")", computeDelaySymbol, () -> { - writer.write("attempt, options.MinDelay, options.MaxDelay, remainingTime,"); - }); - - writer.addUseImports(SmithyGoDependency.FMT); - writer.write( - "if err != nil { return fmt.Errorf(\"error computing waiter delay, %w\", err)}"); - writer.write(""); - - Symbol sleepWithContextSymbol = SymbolUtils.createValueSymbolBuilder( - "SleepWithContext", SmithyGoDependency.SMITHY_TIME - ).build(); - writer.writeDocs("sleep for the delay amount before invoking a request"); - writer.openBlock("if err := $T(ctx, delay); err != nil {", "}", sleepWithContextSymbol, - () -> { - writer.write( - "return fmt.Errorf(\"request cancelled while waiting, %w\", err)"); - }); - }).write(""); + writer.write("apiOptions := options.APIOptions"); + writer.write("start := time.Now()").write(""); // add waiter logger middleware to log an attempt, if LogWaitAttempts is enabled. writer.openBlock("if options.LogWaitAttempts {", "}", () -> { - writer.write("logger.Attempt = attempt +1"); - writer.write("options.APIOptions = append(options.APIOptions, logger.AddLogger)"); - }); - writer.write(""); + writer.write("logger.Attempt = attempt"); + writer.write( + "apiOptions = append([]func(*middleware.Stack) error{}, options.APIOptions...)"); + writer.write("apiOptions = append(apiOptions, logger.AddLogger)"); + }).write(""); // make a request writer.openBlock("out, err := w.client.$T(ctx, params, func (o *Options) { ", "})", operationSymbol, () -> { - writer.write("o.APIOptions = append(o.APIOptions, options.APIOptions...)"); + writer.write("o.APIOptions = append(o.APIOptions, apiOptions...)"); }); writer.write(""); // handle response and identify waiter state writer.write("retryable, err := options.Retryable(ctx, params, out, err)"); - writer.write("if err != nil { return err }").write(""); + writer.write("if err != nil { return err }"); writer.write("if !retryable { return nil }").write(""); // update remaining time writer.write("remainingTime -= time.Since(start)"); - // increment the attempt counter for retry - writer.write("attempt++"); + // check if next iteration is possible + writer.openBlock("if remainingTime < options.MinDelay || remainingTime <= 0 {", "}", () -> { + writer.write("break"); + }); + writer.write(""); + + // handle retry delay computation, sleep. + Symbol computeDelaySymbol = SymbolUtils.createValueSymbolBuilder( + "ComputeDelay", SmithyGoDependency.SMITHY_WAITERS + ).build(); + writer.writeDocs("compute exponential backoff between waiter retries"); + writer.openBlock("delay, err := $T(", ")", computeDelaySymbol, () -> { + writer.write("attempt, options.MinDelay, options.MaxDelay, remainingTime,"); + }); + + writer.addUseImports(SmithyGoDependency.FMT); + writer.write( + "if err != nil { return fmt.Errorf(\"error computing waiter delay, %w\", err)}"); + writer.write(""); + + // update remaining time as per computed delay + writer.write("remainingTime -= delay"); + + // sleep for delay + Symbol sleepWithContextSymbol = SymbolUtils.createValueSymbolBuilder( + "SleepWithContext", SmithyGoDependency.SMITHY_TIME + ).build(); + writer.writeDocs("sleep for the delay amount before invoking a request"); + writer.openBlock("if err := $T(ctx, delay); err != nil {", "}", sleepWithContextSymbol, + () -> { + writer.write( + "return fmt.Errorf(\"request cancelled while waiting, %w\", err)"); + }); }); writer.write("return fmt.Errorf(\"exceeded max wait time for $L waiter\")", waiterName); });