Repository: smithy-lang/smithy
Pull request #2022 (2023-10-27 14:43:55)

Add unknown union malformed request test
Addresses #1222

*Issue #, if available:*

*Description of changes:*


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlNodeParser.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlNodeParser.java
index 2aee9178a..fa35f6556 100644
--- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlNodeParser.java
+++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlNodeParser.java
@@ -74,7 +74,7 @@ final class IdlNodeParser {
                 return result;
             case IDENTIFIER:
                 String shapeId = loader.internString(IdlShapeIdParser.expectAndSkipShapeId(tokenizer));
-                return parseIdentifier(loader, shapeId, location);
+                return createIdentifier(loader, shapeId, location);
             case NUMBER:
                 Number number = tokenizer.getCurrentTokenNumberValue();
                 tokenizer.next();
@@ -95,10 +95,10 @@ final class IdlNodeParser {
      * @param location   Source location to assign to the identifier.
      * @return Returns the parsed identifier.
      */
-    static Node parseIdentifier(IdlModelLoader loader, String identifier, SourceLocation location) {
+    static Node createIdentifier(IdlModelLoader loader, String identifier, SourceLocation location) {
         Keyword keyword = Keyword.from(identifier);
         return keyword == null
-               ? parseSyntacticShapeId(loader, identifier, location)
+               ? createSyntacticShapeId(loader, identifier, location)
                : keyword.createNode(location);
     }
 
@@ -138,7 +138,7 @@ final class IdlNodeParser {
         }
     }
 
-    private static Node parseSyntacticShapeId(
+    private static Node createSyntacticShapeId(
             IdlModelLoader loader,
             String identifier,
             SourceLocation location
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlTraitParser.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlTraitParser.java
index 0d8006e2b..82b52de57 100644
--- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlTraitParser.java
+++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlTraitParser.java
@@ -193,19 +193,30 @@ final class IdlTraitParser {
                 }
             case IDENTIFIER:
             default:
-                String identifier = loader.internString(tokenizer.getCurrentTokenLexeme());
-                tokenizer.next();
+                // Handle: `foo`, `foo$bar`, `foo.bar#baz`, `foo.bar#baz$bam`, `foo: bam`
+                String identifier = loader.internString(IdlShapeIdParser.expectAndSkipShapeId(tokenizer));
                 tokenizer.skipWsAndDocs();
-                if (tokenizer.getCurrentToken() == IdlToken.COLON) {
+                if (tokenizer.getCurrentToken() == IdlToken.RPAREN || isItDefinitelyShapeId(identifier)) {
+                    return IdlNodeParser.createIdentifier(loader, identifier, location);
+                } else {
+                    tokenizer.expect(IdlToken.COLON);
                     tokenizer.next();
                     tokenizer.skipWsAndDocs();
                     return parseStructuredTrait(loader, new StringNode(identifier, location));
-                } else {
-                    return IdlNodeParser.parseIdentifier(loader, identifier, location);
                 }
         }
     }
 
+    private static boolean isItDefinitelyShapeId(String identifier) {
+        for (int i = 0; i < identifier.length(); i++) {
+            char c = identifier.charAt(i);
+            if (c == '.' || c == '$' || c == '#') {
+                return true;
+            }
+        }
+        return false;
+    }
+
     private static ObjectNode parseStructuredTrait(IdlModelLoader loader, StringNode firstKey) {
         IdlInternalTokenizer tokenizer = loader.getTokenizer();
         loader.increaseNestingLevel();


Test patch:
diff --git a/smithy-aws-protocol-tests/model/restJson1/malformedRequests/malformed-union.smithy b/smithy-aws-protocol-tests/model/restJson1/malformedRequests/malformed-union.smithy
index f5ac1bb44..cd0ca8cd4 100644
--- a/smithy-aws-protocol-tests/model/restJson1/malformedRequests/malformed-union.smithy
+++ b/smithy-aws-protocol-tests/model/restJson1/malformedRequests/malformed-union.smithy
@@ -15,14 +15,14 @@ apply MalformedUnion @httpMalformedRequestTests([
     {
         id: "RestJsonMalformedUnionMultipleFieldsSet",
         documentation: """
-        When the union has multiple fields set, the response should be a 400
-        SerializationException.""",
+            When the union has multiple fields set, the response should be a 400
+            SerializationException.""",
         protocol: restJson1,
         request: {
             method: "POST",
             uri: "/MalformedUnion",
             body: """
-            { "union" : { "int": 2, "string": "three" } }""",
+                { "union" : { "int": 2, "string": "three" } }""",
             headers: {
                 "content-type": "application/json"
             }
@@ -37,8 +37,8 @@ apply MalformedUnion @httpMalformedRequestTests([
     {
         id: "RestJsonMalformedUnionKnownAndUnknownFieldsSet",
         documentation: """
-        When the union has multiple fields set, even when only one is modeled,
-        the response should be a 400 SerializationException.""",
+            When the union has multiple fields set, even when only one is modeled,
+            the response should be a 400 SerializationException.""",
         protocol: restJson1,
         request: {
             method: "POST",
@@ -59,14 +59,14 @@ apply MalformedUnion @httpMalformedRequestTests([
     {
         id: "RestJsonMalformedUnionNoFieldsSet",
         documentation: """
-        When the union has no fields set, the response should be a 400
-        SerializationException.""",
+            When the union has no fields set, the response should be a 400
+            SerializationException.""",
         protocol: restJson1,
         request: {
             method: "POST",
             uri: "/MalformedUnion",
             body: """
-            { "union" : { "int": null } }""",
+                { "union" : { "int": null } }""",
             headers: {
                 "content-type": "application/json"
             }
@@ -81,14 +81,14 @@ apply MalformedUnion @httpMalformedRequestTests([
     {
         id: "RestJsonMalformedUnionValueIsArray",
         documentation: """
-        When the union value is actually an array, the response should be a 400
-        SerializationException.""",
+            When the union value is actually an array, the response should be a 400
+            SerializationException.""",
         protocol: restJson1,
         request: {
             method: "POST",
             uri: "/MalformedUnion",
             body: """
-            { "union" : ["int"] }""",
+                { "union" : ["int"] }""",
             headers: {
                 "content-type": "application/json"
             }
@@ -99,7 +99,33 @@ apply MalformedUnion @httpMalformedRequestTests([
                 "x-amzn-errortype": "SerializationException"
             }
         }
-    },
+    }
+    {
+        id: "RestJsonMalformedUnionUnknownMember",
+        documentation: """
+            When an unknown union member is received, the response should be a 400
+            SerializationException."""
+        protocol: restJson1
+        request: {
+            method: "POST"
+            uri: "/MalformedUnion"
+            body: """
+                {
+                    "union": {
+                        "unknown": "hello"
+                    }
+                }""",
+            headers: {
+                "content-type": "application/json"
+            }
+        }
+        response: {
+            code: 400
+            headers: {
+                "x-amzn-errortype": "SerializationException"
+            }
+        }
+    }
 ])
 
 structure MalformedUnionInput {
@@ -111,4 +137,3 @@ union SimpleUnion {
 
     string: String
 }
-
diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/invalid/traits/annotation-unclosed-object1.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/invalid/traits/annotation-unclosed-object1.smithy
index 87564f759..74952506c 100644
--- a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/invalid/traits/annotation-unclosed-object1.smithy
+++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/invalid/traits/annotation-unclosed-object1.smithy
@@ -1,4 +1,4 @@
-// Syntax error at line 4, column 8: Expected RPAREN(')') but found IDENTIFIER('MyString') | Model
+// Syntax error at line 4, column 8: Expected COLON(':') but found IDENTIFIER('MyString') | Model
 namespace com.foo
 @foo(
 string MyString
diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/invalid/traits/invalid-trait-shape-id-key.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/invalid/traits/invalid-trait-shape-id-key.smithy
index 6d3ac8643..83e4c9e55 100644
--- a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/invalid/traits/invalid-trait-shape-id-key.smithy
+++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/invalid/traits/invalid-trait-shape-id-key.smithy
@@ -1,4 +1,4 @@
-// Syntax error at line 4, column 30: Expected RPAREN(')') but found DOT('.') | Model
+// Syntax error at line 4, column 42: Expected RPAREN(')') but found COLON(':') | Model
 namespace smithy.example
 
 @externalDocumentation(smithy.example#foo: "bar")
diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/traits/reference-shape-from-trait.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/traits/reference-shape-from-trait.json
new file mode 100644
index 000000000..e49630c64
--- /dev/null
+++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/traits/reference-shape-from-trait.json
@@ -0,0 +1,53 @@
+{
+    "smithy": "2.0",
+    "shapes": {
+        "smithy.example#StructA": {
+            "type": "structure",
+            "members": {
+                "id": {
+                    "target": "smithy.api#String"
+                }
+            }
+        },
+        "smithy.example#StructB": {
+            "type": "structure",
+            "members": {
+                "structAId": {
+                    "target": "smithy.api#String",
+                    "traits": {
+                        "smithy.example#link": "smithy.example#StructA$id"
+                    }
+                }
+            }
+        },
+        "smithy.example#StructC": {
+            "type": "structure",
+            "members": {
+                "structAId": {
+                    "target": "smithy.api#String",
+                    "traits": {
+                        "smithy.example#link": "smithy.example#StructA$id"
+                    }
+                }
+            }
+        },
+        "smithy.example#StructD": {
+            "type": "structure",
+            "members": {
+                "structAId": {
+                    "target": "smithy.api#String",
+                    "traits": {
+                        "smithy.example#link": "smithy.example#StructA"
+                    }
+                }
+            }
+        },
+        "smithy.example#link": {
+            "type": "string",
+            "traits": {
+                "smithy.api#idRef": {},
+                "smithy.api#trait": {}
+            }
+        }
+    }
+}
diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/traits/reference-shape-from-trait.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/traits/reference-shape-from-trait.smithy
new file mode 100644
index 000000000..a23e03dcf
--- /dev/null
+++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/traits/reference-shape-from-trait.smithy
@@ -0,0 +1,26 @@
+$version: "2.0"
+
+namespace smithy.example
+
+@trait
+@idRef
+string link
+
+structure StructA {
+    id: String
+}
+
+structure StructB {
+    @link(StructA$id)
+    structAId: String
+}
+
+structure StructC {
+    @link(smithy.example#StructA$id)
+    structAId: String
+}
+
+structure StructD {
+    @link(smithy.example#StructA)
+    structAId: String
+}