Fix OpenAPI required bug

This commit fixes an issue with the OpenAPI converter where path and
header parameters that are required would appear in the synthesized
object input with "required" properties that reference members that were
removed.

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Smithy Changelog
 
+## 1.0.3 (TBD)
+
+### Bug Fixes
+
+* Fix an issue with the OpenAPI conversion where synthesized structure inputs reference required properties that
+  were removed. ([#443](https://github.com/awslabs/smithy/pull/443))
+
 ## 1.0.2 (2020-05-18)
 
 ### Bug Fixes

smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/protocols/AwsRestJson1Protocol.java

@@ -66,22 +66,27 @@ Schema createDocumentSchema(
         ShapeId container = bindings.get(0).getMember().getContainer();
         StructureShape containerShape = context.getModel().expectShape(container, StructureShape.class);
 
-        // Create a schema from the original shape and remove unnecessary members.
-        Schema.Builder schemaBuilder = context.getJsonSchemaConverter()
-                .convertShape(containerShape)
-                .getRootSchema()
-                .toBuilder();
-
+        // Path parameters of requests are handled in "parameters" and headers are
+        // handled in headers, so this method must ensure that only members that
+        // are sent in the document payload are present in the structure when it is
+        // converted to OpenAPI. This ensures that any path parameters are removed
+        // before converting the structure to a synthesized JSON schema object.
+        // Doing this sanitation after converting the shape to JSON schema might
+        // result in things like "required" properties pointing to members that
+        // don't exist.
         Set<String> documentMemberNames = bindings.stream()
                 .map(HttpBinding::getMemberName)
                 .collect(Collectors.toSet());
 
+        // Remove non-document members.
+        StructureShape.Builder containerShapeBuilder = containerShape.toBuilder();
         for (String memberName : containerShape.getAllMembers().keySet()) {
             if (!documentMemberNames.contains(memberName)) {
-                schemaBuilder.removeProperty(memberName);
+                containerShapeBuilder.removeMember(memberName);
             }
         }
 
-        return schemaBuilder.build();
+        StructureShape cleanedShape = containerShapeBuilder.build();
+        return context.getJsonSchemaConverter().convertShape(cleanedShape).getRootSchema();
     }
 }

smithy-openapi/src/test/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverterTest.java

@@ -349,4 +349,23 @@ public void updateDefaultSettings(Model model, OpenApiConfig config) {
 
         assertThat(config.getExtensions().getMember("hello"), not(Optional.empty()));
     }
+
+    // The input structure needs a synthesized content structure. Since the
+    // path property is a "parameter" the synthesized structure must not list
+    // it as required because it is not part of the payload.
+    @Test
+    public void properlyRemovesRequiredPropertiesFromSynthesizedInput() {
+        Model model = Model.assembler()
+                .addImport(getClass().getResource("service-with-required-path.json"))
+                .discoverModels()
+                .assemble()
+                .unwrap();
+        OpenApiConfig config = new OpenApiConfig();
+        config.setService(ShapeId.from("example.rest#RestService"));
+        OpenApi result = OpenApiConverter.create().config(config).convert(model);
+        Node expectedNode = Node.parse(IoUtils.toUtf8String(
+                getClass().getResourceAsStream("service-with-required-path.openapi.json")));
+
+        Node.assertEquals(result, expectedNode);
+    }
 }

smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/service-with-required-path.json

@@ -0,0 +1,59 @@
+{
+    "smithy": "1.0",
+    "shapes": {
+        "example.rest#RestService": {
+            "type": "service",
+            "version": "2006-03-01",
+            "operations": [
+                {
+                    "target": "example.rest#PutDocumentPayload"
+                }
+            ],
+            "traits": {
+                "aws.protocols#restJson1": {}
+            }
+        },
+        "example.rest#PutDocumentPayload": {
+            "type": "operation",
+            "input": {
+                "target": "example.rest#PutDocumentPayloadInput"
+            },
+            "output": {
+                "target": "example.rest#PutDocumentPayloadOutput"
+            },
+            "traits": {
+                "smithy.api#idempotent": {},
+                "smithy.api#http": {
+                    "uri": "/payload/{path}",
+                    "method": "PUT"
+                }
+            }
+        },
+        "example.rest#PutDocumentPayloadInput": {
+            "type": "structure",
+            "members": {
+                "path": {
+                    "target": "smithy.api#String",
+                    "traits": {
+                        "smithy.api#httpLabel": {},
+                        "smithy.api#required": {}
+                    }
+                },
+                "foo": {
+                    "target": "smithy.api#String"
+                },
+                "baz": {
+                    "target": "smithy.api#String",
+                    "traits": {
+                        "smithy.api#httpHeader": "X-Baz",
+                        "smithy.api#required": {}
+                    }
+                }
+            }
+        },
+        "example.rest#PutDocumentPayloadOutput": {
+            "type": "structure",
+            "members": {}
+        }
+    }
+}

smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/service-with-required-path.openapi.json

@@ -0,0 +1,58 @@
+{
+    "openapi": "3.0.2",
+    "info": {
+        "title": "RestService",
+        "version": "2006-03-01"
+    },
+    "paths": {
+        "/payload/{path}": {
+            "put": {
+                "operationId": "PutDocumentPayload",
+                "requestBody": {
+                    "content": {
+                        "application/json": {
+                            "schema": {
+                                "$ref": "#/components/schemas/PutDocumentPayloadRequestContent"
+                            }
+                        }
+                    }
+                },
+                "parameters": [
+                    {
+                        "name": "path",
+                        "in": "path",
+                        "schema": {
+                            "type": "string"
+                        },
+                        "required": true
+                    },
+                    {
+                        "name": "X-Baz",
+                        "in": "header",
+                        "schema": {
+                            "type": "string"
+                        },
+                        "required": true
+                    }
+                ],
+                "responses": {
+                    "200": {
+                        "description": "PutDocumentPayload 200 response"
+                    }
+                }
+            }
+        }
+    },
+    "components": {
+        "schemas": {
+            "PutDocumentPayloadRequestContent": {
+                "type": "object",
+                "properties": {
+                    "foo": {
+                        "type": "string"
+                    }
+                }
+            }
+        }
+    }
+}