Skip to content

Conversation

@aajtodd
Copy link
Contributor

@aajtodd aajtodd commented Jan 31, 2022

Issue #

Related to: smithy-lang/smithy-kotlin#579
closes #449

Description of changes

Refactored protocols to be implemented using the new StructuredDataSerializerGenerator and StructuredDataParserGenerator)
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@aajtodd aajtodd requested a review from a team as a code owner January 31, 2022 20:22
@aajtodd aajtodd requested review from ianbotsf and kggilmer January 31, 2022 20:24
@github-actions
Copy link

A new generated diff is ready to view: __generated-main...__generated-refactor-protocol-codegen

@github-actions
Copy link

A new generated diff is ready to view: __generated-main...__generated-refactor-protocol-codegen

}
.write("")

if (op.errors.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment: Now that we're attaching metadata in all cases, I think we could simplify this code by moving the metadata/throwing to a single place:

writer.withBlock("val ex = when (errorDetails.code) {", "}") {
    op.errors.forEach { err ->
        val errSymbol = ctx.symbolProvider.toSymbol(ctx.model.expectShape(err))
        val errDeserializerSymbol = buildSymbol {
            name = "${errSymbol.name}Deserializer"
            namespace = "${ctx.settings.pkg.name}.transform"
        }
        write("#S -> #T.deserialize(context, wrappedResponse)", getErrorCode(ctx, err), errDeserializerSymbol)
    }
    
    write("else -> #T(errorDetails.message)", exceptionBaseSymbol)
}

writer.write("#T(ex, wrappedResponse, errorDetails)", AwsRuntimeTypes.Http.setAseErrorMetadata)
writer.write("throw ex")

}
.write("")

if (op.errors.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment: Duplicated code with AwsHttpBindingProtocolGenerator.kt.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the improvements you suggested I'm going to leave some duplication here since the S3 error codegen is handling a few S3 specific behaviors

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 2, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
9.1% 9.1% Duplication

@github-actions
Copy link

github-actions bot commented Feb 2, 2022

A new generated diff is ready to view: __generated-main...__generated-refactor-protocol-codegen

Copy link
Contributor

@kggilmer kggilmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loving all the red

@aajtodd aajtodd merged commit fbb9e45 into main Feb 2, 2022
@aajtodd aajtodd deleted the refactor-protocol-codegen branch February 2, 2022 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error metadata not set on unmodeled errors

3 participants