Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Allow 'null' variants in unions #3481

Merged
merged 2 commits into from
Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@ buildscript {
}

allprojects {
val allowLocalDeps: String by project
repositories {
// mavenLocal()
if (allowLocalDeps.toBoolean()) {
mavenLocal()
}
mavenCentral()
google()
}
Expand Down
1 change: 0 additions & 1 deletion buildSrc/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ plugins {
repositories {
mavenCentral()
google()
/* mavenLocal() */
}

// Load properties manually to avoid hard coding smithy version
Expand Down
2 changes: 0 additions & 2 deletions codegen-client-test/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,6 @@ val allCodegenTests = listOf(
),
ClientTest("aws.protocoltests.misc#QueryCompatService", "query-compat-test", dependsOn = listOf("aws-json-query-compat.smithy")),
).map(ClientTest::toCodegenTest)
// use this line to run just one test
// .filter { it.module == "query-compat-test" }

project.registerGenerateSmithyBuildTask(rootProject, pluginName, allCodegenTests)
project.registerGenerateCargoWorkspaceTask(rootProject, pluginName, allCodegenTests, workingDirUnderBuildDir)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.withBlockTemplate
import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext
import software.amazon.smithy.rust.codegen.core.smithy.CodegenTarget
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope
import software.amazon.smithy.rust.codegen.core.smithy.canUseDefault
import software.amazon.smithy.rust.codegen.core.smithy.customize.NamedCustomization
import software.amazon.smithy.rust.codegen.core.smithy.customize.Section
Expand Down Expand Up @@ -121,6 +122,7 @@ class JsonParserGenerator(
"skip_to_end" to smithyJson.resolve("deserialize::token::skip_to_end"),
"Token" to smithyJson.resolve("deserialize::Token"),
"or_empty" to orEmptyJson(),
*preludeScope,
)

/**
Expand Down Expand Up @@ -560,6 +562,15 @@ class JsonParserGenerator(
*codegenScope,
) {
objectKeyLoop(hasMembers = shape.members().isNotEmpty()) {
rustTemplate(
"""
if let #{Some}(#{Ok}(#{Token}::ValueNull { .. })) = tokens.peek() {
let _ = tokens.next().expect("peek returned a token")?;
continue;
}
""",
*codegenScope,
)
rustTemplate(
"""
let key = key.to_unescaped()?;
Expand Down Expand Up @@ -622,6 +633,16 @@ class JsonParserGenerator(
*codegenScope,
)
}
// If we've gotten to the point where the union had a `{ ... }` section, we can't return None
// anymore. If we didn't parse a union at this point, this is an error.
rustTemplate(
"""
if variant.is_none() {
return Err(#{Error}::custom("Union did not contain a valid variant."))
}
""",
*codegenScope,
)
rust("Ok(variant)")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,26 @@ class JsonParserGeneratorTest {
""",
)

unitTest(
"allow_null_for_variants",
"""
// __type field should be ignored during deserialization
let input = br#"{ "top": { "choice": { "blob": null, "boolean": null, "int": 5, "long": null, "__type": "value-should-be-ignored-anyway" } } }"#;
let output = ${format(operationGenerator)}(input, test_output::OpOutput::builder()).unwrap().build();
use test_model::Choice;
assert_eq!(Choice::Int(5), output.top.unwrap().choice);
""",
)

unitTest(
"all_variants_null",
"""
// __type field should be ignored during deserialization
let input = br#"{ "top": { "choice": { "blob": null, "boolean": null, "int": null, "long": null, "__type": "value-should-be-ignored-anyway" } } }"#;
let _err = ${format(operationGenerator)}(input, test_output::OpOutput::builder()).expect_err("invalid union");
""",
)

unitTest(
"empty_error",
"""
Expand Down
1 change: 1 addition & 0 deletions gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ kotlin.code.style=official
# codegen
smithyGradlePluginVersion=0.9.0
smithyVersion=1.45.0
allowLocalDeps=false

# kotlin
kotlinVersion=1.9.20
Expand Down
Loading