Fix OpenAPI required bug

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"
+                    }
+                }
+            }
+        }
+    }
+}