From f334ae71c41beb0bdb27b85c6bb959bfba725def Mon Sep 17 00:00:00 2001 From: Michael Dowling Date: Wed, 23 Mar 2022 16:13:08 -0700 Subject: [PATCH] Allow jsonName trait on union members Closes #953 --- docs/source/1.0/spec/core/protocol-traits.rst | 11 +- .../model/restJson1/unions.smithy | 153 ++++++++++++++++++ .../validators/JsonNameValidator.java | 69 ++++++++ ...e.amazon.smithy.model.validation.Validator | 1 + .../amazon/smithy/model/loader/prelude.smithy | 2 +- .../errorfiles/validators/json-name.errors | 2 +- .../validators/jsonName-conflict.errors | 2 + .../validators/jsonName-conflict.json | 45 ++++++ 8 files changed, 280 insertions(+), 5 deletions(-) create mode 100644 smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/JsonNameValidator.java create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/jsonName-conflict.errors create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/jsonName-conflict.json diff --git a/docs/source/1.0/spec/core/protocol-traits.rst b/docs/source/1.0/spec/core/protocol-traits.rst index 5f59c7000fd..92a06b8f429 100644 --- a/docs/source/1.0/spec/core/protocol-traits.rst +++ b/docs/source/1.0/spec/core/protocol-traits.rst @@ -143,11 +143,11 @@ support configuration settings. Summary Allows a serialized object property name in a JSON document to differ from - a structure member name used in the model. + a structure or union member name used in the model. Trait selector - ``structure > member`` + ``:is(structure, union) > member`` - *Any structure member* + *Any structure or union member* Value type ``string`` @@ -203,6 +203,11 @@ following document: "bar": "def" } +.. note:: + + No two members of the same structure or union can use the + same case-sensitive ``@jsonName``. + .. smithy-trait:: smithy.api#mediaType .. _mediaType-trait: diff --git a/smithy-aws-protocol-tests/model/restJson1/unions.smithy b/smithy-aws-protocol-tests/model/restJson1/unions.smithy index 09c3703013c..0d942f4f2d4 100644 --- a/smithy-aws-protocol-tests/model/restJson1/unions.smithy +++ b/smithy-aws-protocol-tests/model/restJson1/unions.smithy @@ -512,3 +512,156 @@ apply PostPlayerAction @httpResponseTests([ } } ]) + + +/// This operation defines a union that uses jsonName on some members. +@http(uri: "/PostUnionWithJsonName", method: "POST") +operation PostUnionWithJsonName { + input: PostUnionWithJsonNameInput, + output: PostUnionWithJsonNameOutput +} + +@input +structure PostUnionWithJsonNameInput { + @required + value: UnionWithJsonName +} + +@output +structure PostUnionWithJsonNameOutput { + @required + value: UnionWithJsonName +} + +union UnionWithJsonName { + @jsonName("FOO") + foo: String, + + bar: String, + + @jsonName("_baz") + baz: String +} + +apply PostUnionWithJsonName @httpRequestTests([ + { + id: "PostUnionWithJsonNameRequest1", + documentation: "Tests that jsonName works with union members.", + protocol: restJson1, + method: "POST", + uri: "/PostUnionWithJsonName", + body: """ + { + "value": { + "FOO": "hi" + } + }""", + bodyMediaType: "application/json", + headers: {"Content-Type": "application/json"}, + params: { + value: { + foo: "hi" + } + } + }, + { + id: "PostUnionWithJsonNameRequest2", + documentation: "Tests that jsonName works with union members.", + protocol: restJson1, + method: "POST", + uri: "/PostUnionWithJsonName", + body: """ + { + "value": { + "_baz": "hi" + } + }""", + bodyMediaType: "application/json", + headers: {"Content-Type": "application/json"}, + params: { + value: { + baz: "hi" + } + } + }, + { + id: "PostUnionWithJsonNameRequest3", + documentation: "Tests that jsonName works with union members.", + protocol: restJson1, + method: "POST", + uri: "/PostUnionWithJsonName", + body: """ + { + "value": { + "bar": "hi" + } + }""", + bodyMediaType: "application/json", + headers: {"Content-Type": "application/json"}, + params: { + value: { + bar: "hi" + } + } + } +]) + +apply PostUnionWithJsonName @httpResponseTests([ + { + id: "PostUnionWithJsonNameResponse1", + documentation: "Tests that jsonName works with union members.", + protocol: restJson1, + code: 200, + body: """ + { + "value": { + "FOO": "hi" + } + }""", + bodyMediaType: "application/json", + headers: {"Content-Type": "application/json"}, + params: { + value: { + foo: "hi" + } + } + }, + { + id: "PostUnionWithJsonNameResponse2", + documentation: "Tests that jsonName works with union members.", + protocol: restJson1, + code: 200, + body: """ + { + "value": { + "_baz": "hi" + } + }""", + bodyMediaType: "application/json", + headers: {"Content-Type": "application/json"}, + params: { + value: { + baz: "hi" + } + } + }, + { + id: "PostUnionWithJsonNameResponse3", + documentation: "Tests that jsonName works with union members.", + protocol: restJson1, + code: 200, + body: """ + { + "value": { + "bar": "hi" + } + }""", + bodyMediaType: "application/json", + headers: {"Content-Type": "application/json"}, + params: { + value: { + bar: "hi" + } + } + } +]) diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/JsonNameValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/JsonNameValidator.java new file mode 100644 index 00000000000..1a09c43420d --- /dev/null +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/JsonNameValidator.java @@ -0,0 +1,69 @@ +/* + * Copyright 2022 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.model.validation.validators; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; +import java.util.TreeSet; +import java.util.stream.Collectors; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.shapes.MemberShape; +import software.amazon.smithy.model.shapes.Shape; +import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.model.traits.JsonNameTrait; +import software.amazon.smithy.model.validation.AbstractValidator; +import software.amazon.smithy.model.validation.ValidationEvent; + +public final class JsonNameValidator extends AbstractValidator { + @Override + public List validate(Model model) { + List events = new ArrayList<>(); + Set visitedContainers = new HashSet<>(); + + // Find every member marked with a jsonName trait. The containing shapes of these members are + // the only structure/union shapes that need to be validated. + for (MemberShape member : model.getMemberShapesWithTrait(JsonNameTrait.class)) { + // If the container hasn't been visited yet, then validate it's members. + if (visitedContainers.add(member.getContainer())) { + validateMembersOfContainer(model.expectShape(member.getContainer()), events); + } + } + return events; + } + + private void validateMembersOfContainer(Shape container, List events) { + Map> memberMappings = new TreeMap<>(); + for (MemberShape m : container.members()) { + String jsonName = m.getTrait(JsonNameTrait.class) + .map(JsonNameTrait::getValue) + .orElseGet(m::getMemberName); + memberMappings.computeIfAbsent(jsonName, n -> new TreeSet<>()).add(m); + } + + for (Map.Entry> entry : memberMappings.entrySet()) { + if (entry.getValue().size() > 1) { + events.add(error(container, String.format( + "This shape contains members with conflicting JSON names that resolve to '%s': %s", + entry.getKey(), + entry.getValue().stream().map(MemberShape::getMemberName).collect(Collectors.joining(", "))))); + } + } + } +} diff --git a/smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator b/smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator index 72853f70660..a3a377bf40c 100644 --- a/smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator +++ b/smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator @@ -16,6 +16,7 @@ software.amazon.smithy.model.validation.validators.HttpQueryParamsTraitValidator software.amazon.smithy.model.validation.validators.HttpQueryTraitValidator software.amazon.smithy.model.validation.validators.HttpResponseCodeSemanticsValidator software.amazon.smithy.model.validation.validators.HttpUriConflictValidator +software.amazon.smithy.model.validation.validators.JsonNameValidator software.amazon.smithy.model.validation.validators.LengthTraitValidator software.amazon.smithy.model.validation.validators.MediaTypeValidator software.amazon.smithy.model.validation.validators.NoInlineDocumentSupportValidator diff --git a/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy b/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy index 8ec4dd6368a..44d9d1b9454 100644 --- a/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy +++ b/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy @@ -293,7 +293,7 @@ structure internal {} /// The jsonName trait allows a serialized object property name to differ /// from a structure member name used in the model. -@trait(selector: "structure > member") +@trait(selector: ":is(structure, union) > member") @tags(["diff.error.const"]) string jsonName diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/json-name.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/json-name.errors index 86c24165423..54f457c62df 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/json-name.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/json-name.errors @@ -1 +1 @@ -[ERROR] ns.foo#B: Trait `jsonName` cannot be applied to `ns.foo#B`. This trait may only be applied to shapes that match the following selector: structure > member | TraitTarget +[ERROR] ns.foo#B: Trait `jsonName` cannot be applied to `ns.foo#B`. This trait may only be applied to shapes that match the following selector: :is(structure, union) | TraitTarget diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/jsonName-conflict.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/jsonName-conflict.errors new file mode 100644 index 00000000000..bf5e4328e1b --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/jsonName-conflict.errors @@ -0,0 +1,2 @@ +[ERROR] ns.foo#A: This shape contains members with conflicting JSON names that resolve to 'b': a, b | JsonName +[ERROR] ns.foo#B: This shape contains members with conflicting JSON names that resolve to 'b': a, b | JsonName diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/jsonName-conflict.json b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/jsonName-conflict.json new file mode 100644 index 00000000000..6681f5bd4f3 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/jsonName-conflict.json @@ -0,0 +1,45 @@ +{ + "smithy": "1.0", + "shapes": { + "ns.foo#A": { + "type": "structure", + "members": { + "a": { + "target": "smithy.api#String", + "traits": { + "smithy.api#jsonName": "b" + } + }, + "b": { + "target": "smithy.api#String" + }, + "c": { + "target": "smithy.api#String", + "traits": { + "smithy.api#jsonName": "a" + } + } + } + }, + "ns.foo#B": { + "type": "union", + "members": { + "a": { + "target": "smithy.api#String", + "traits": { + "smithy.api#jsonName": "b" + } + }, + "b": { + "target": "smithy.api#String" + }, + "c": { + "target": "smithy.api#String", + "traits": { + "smithy.api#jsonName": "a" + } + } + } + } + } +}