Skip to content

Chore refactor ep20 #1052

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 25, 2023
Merged

Chore refactor ep20 #1052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0dfd642
WIP broken upgrade of Smithy to 1.34.0-SNAPSHOT including endpoints r…
ianbotsf Jul 20, 2023
c93d4c3
Merge remote-tracking branch 'origin/main' into chore-refactor-ep20
ianbotsf Sep 14, 2023
7bed378
wip: update to latest pull of smithy/endpoints_refactor
ianbotsf Sep 15, 2023
4513180
ignore some new protocol tests
ianbotsf Sep 25, 2023
a0cc3e5
switch to released version of smithy
ianbotsf Sep 25, 2023
27cd29b
Merge remote-tracking branch 'origin/main' into chore-refactor-ep20
ianbotsf Sep 25, 2023
8ff508d
lint
ianbotsf Sep 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion codegen/smithy-aws-kotlin-codegen/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ dependencies {
api("software.amazon.smithy:smithy-aws-iam-traits:$smithyVersion")
api("software.amazon.smithy:smithy-aws-cloudformation-traits:$smithyVersion")
api("software.amazon.smithy:smithy-protocol-test-traits:$smithyVersion")
implementation("software.amazon.smithy:smithy-rules-engine:$smithyVersion")
implementation("software.amazon.smithy:smithy-aws-endpoints:$smithyVersion")

testImplementation("org.junit.jupiter:junit-jupiter:$junitVersion")
testImplementation("org.junit.jupiter:junit-jupiter-params:$junitVersion")
Expand Down
1 change: 1 addition & 0 deletions .../smithy-aws-kotlin-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/AwsEndpointDelegator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class AwsEndpointDelegator : EndpointDelegator {
},
)
}

if (rules != null) {
ctx.delegator.useFileWriter(defaultProviderSymbol) {
DefaultEndpointProviderGenerator(it, rules, defaultProviderSymbol, providerSymbol, paramsSymbol, ctx.settings, endpointFunctions, awsEndpointPropertyRenderers).render()
Expand Down
25 changes: 25 additions & 0 deletions .../src/main/kotlin/aws/sdk/kotlin/codegen/protocols/core/AwsHttpBindingProtocolGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,30 @@ abstract class AwsHttpBindingProtocolGenerator : HttpBindingProtocolGenerator()
}

override fun generateProtocolUnitTests(ctx: ProtocolGenerator.GenerationContext) {
val ignoredTests = TestMemberDelta(
setOf(
// FIXME - compression not yet supported, see https://github.com/awslabs/smithy-kotlin/issues/955
"SDKAppliedContentEncoding_awsJson1_0",
"SDKAppliedContentEncoding_awsJson1_1",
"SDKAppliedContentEncoding_awsQuery",
"SDKAppliedContentEncoding_ec2Query",
"SDKAppliedContentEncoding_restJson1",
"SDKAppliedContentEncoding_restXml",
"SDKAppendedGzipAfterProvidedEncoding_restJson1",
"SDKAppendedGzipAfterProvidedEncoding_restXml",
"SDKAppendsGzipAndIgnoresHttpProvidedEncoding_awsJson1_0",
"SDKAppendsGzipAndIgnoresHttpProvidedEncoding_awsJson1_1",
"SDKAppendsGzipAndIgnoresHttpProvidedEncoding_awsQuery",
"SDKAppendsGzipAndIgnoresHttpProvidedEncoding_ec2Query",

// FIXME - missing Content-Type on restXml empty payloads, see https://github.com/awslabs/aws-sdk-kotlin/issues/1050
"RestXmlHttpPayloadWithUnsetUnion",

// FIXME - xmlns incorrect in XML maps, see https://github.com/awslabs/smithy-kotlin/issues/957
"RestXmlXmlMapWithXmlNamespace",
),
)

// The following can be used to generate only a specific test by name.
// val targetedTest = TestMemberDelta(setOf("RestJsonComplexErrorWithNoMessage"), TestContainmentMode.RUN_TESTS)

Expand All @@ -64,6 +88,7 @@ abstract class AwsHttpBindingProtocolGenerator : HttpBindingProtocolGenerator()
requestTestBuilder,
responseTestBuilder,
errorTestBuilder,
ignoredTests,
).generateProtocolTests()
}

Expand Down
5 changes: 1 addition & 4 deletions .../src/main/kotlin/aws/sdk/kotlin/codegen/protocols/endpoints/DefaultEndpointProviderExt.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import software.amazon.smithy.kotlin.codegen.rendering.endpoints.ExpressionRende
import software.amazon.smithy.kotlin.codegen.utils.getOrNull
import software.amazon.smithy.model.node.Node
import software.amazon.smithy.model.node.ObjectNode
import software.amazon.smithy.rulesengine.language.syntax.expr.Expression
import software.amazon.smithy.rulesengine.language.syntax.expressions.Expression
import java.util.*

val awsEndpointFunctions = mapOf(
Expand All @@ -24,9 +24,6 @@ val awsEndpointPropertyRenderers = mapOf(
"authSchemes" to ::renderAuthSchemes,
)

// valid auth scheme names that can appear in a smithy endpoint's properties
private val validAuthSchemeNames = setOf("sigv4", "sigv4a")

private fun String.toSigningContextClassName(): String? =
when (this) {
"sigv4" -> "SigV4"
Expand Down
6 changes: 3 additions & 3 deletions gradle.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ org.gradle.jvmargs=-Xmx6g -XX:MaxMetaspaceSize=2G
sdkVersion=0.32.3-SNAPSHOT

# codegen
smithyVersion=1.29.0
smithyVersion=1.39.0
smithyGradleVersion=0.6.0
# smithy-kotlin codegen and runtime are versioned together
smithyKotlinVersion=0.27.4
smithyKotlinVersion=0.27.5-SNAPSHOT

# kotlin
kotlinVersion=1.8.22
Expand All @@ -37,4 +37,4 @@ mockkVersion=1.13.3
slf4jVersion=2.0.6

# dokka config (values specified at build-time as needed)
smithyKotlinDocBaseUrl=https://sdk.amazonaws.com/kotlin/api/smithy-kotlin/api/$smithyKotlinVersion/
smithyKotlinDocBaseUrl=https://sdk.amazonaws.com/kotlin/api/smithy-kotlin/api/$smithyKotlinVersion/
2 changes: 2 additions & 0 deletions gradle/sdk-plugins/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.Properties
buildscript {
repositories {
mavenLocal()
Copy link
Member

@lauzadis lauzadis Sep 25, 2023

Choose a reason for hiding this comment

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

Just wondering, why are these new mavenLocal() repositories required?

Copy link
Contributor Author

@ianbotsf ianbotsf Sep 25, 2023

Choose a reason for hiding this comment

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

They're not required but they were convenient while testing the unreleased changes in smithy because I could clone the branch and publish it locally. In general, having mavenLocal() in the repositories list makes development easier and we already have it in several places (e.g., our main build.gradle.kts).

mavenCentral()
}
}
Expand All @@ -15,6 +16,7 @@ plugins {
}

repositories {
mavenLocal()
mavenCentral()
gradlePluginPortal()
}
Expand Down
1 change: 1 addition & 0 deletions settings.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

pluginManagement {
repositories {
mavenLocal()
mavenCentral()
gradlePluginPortal()
}
Expand Down