From 6f2c4e2c46bfa18088747903b7cce0c356107f88 Mon Sep 17 00:00:00 2001
From: Michael Dowling <mtdowling@gmail.com>
Date: Thu, 28 May 2020 15:54:50 -0700
Subject: [PATCH] Remove required entries when property is removed

---
 CHANGELOG.md                                    |  7 +++++++
 .../amazon/smithy/jsonschema/Schema.java        | 10 ++++++++--
 .../amazon/smithy/jsonschema/SchemaTest.java    | 17 +++++++++++++++++
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8fc77be8d9d..8a2a93b335e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,12 @@
 # Smithy Changelog
 
+## 1.0.4 (TBD)
+
+### Features
+
+* Ensure that when a property is removed from a JSON schema object, that a corresponding "required"
+  entry is also removed.
+
 ## 1.0.3 (2020-05-26)
 
 ### Bug Fixes
diff --git a/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/Schema.java b/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/Schema.java
index 13b035faa0f..5d6b61b5c25 100644
--- a/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/Schema.java
+++ b/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/Schema.java
@@ -15,6 +15,7 @@
 
 package software.amazon.smithy.jsonschema;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -562,7 +563,7 @@ public static final class Builder implements SmithyBuilder<Schema> {
 
         private Integer maxProperties;
         private Integer minProperties;
-        private Collection<String> required = ListUtils.of();
+        private Collection<String> required = new ArrayList<>();
         private Map<String, Schema> properties = new HashMap<>();
         private Schema additionalProperties;
         private Schema propertyNames;
@@ -688,7 +689,11 @@ public Builder minProperties(Integer minProperties) {
         }
 
         public Builder required(Collection<String> required) {
-            this.required = required == null ? ListUtils.of() : required;
+            if (required == null) {
+                this.required.clear();
+            } else {
+                this.required = new ArrayList<>(required);
+            }
             return this;
         }
 
@@ -709,6 +714,7 @@ public Builder putProperty(String key, Schema value) {
 
         public Builder removeProperty(String key) {
             properties.remove(key);
+            required.remove(key);
             return this;
         }
 
diff --git a/smithy-jsonschema/src/test/java/software/amazon/smithy/jsonschema/SchemaTest.java b/smithy-jsonschema/src/test/java/software/amazon/smithy/jsonschema/SchemaTest.java
index 801a3401b01..451e78184f6 100644
--- a/smithy-jsonschema/src/test/java/software/amazon/smithy/jsonschema/SchemaTest.java
+++ b/smithy-jsonschema/src/test/java/software/amazon/smithy/jsonschema/SchemaTest.java
@@ -16,6 +16,7 @@
 package software.amazon.smithy.jsonschema;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.not;
 
@@ -24,6 +25,7 @@
 import java.util.Set;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
+import software.amazon.smithy.utils.ListUtils;
 import software.amazon.smithy.utils.SetUtils;
 
 public class SchemaTest {
@@ -180,4 +182,19 @@ public void ignoresNegativeNumericPositions() {
 
         assertThat(schema.selectSchema("allOf", "-1"), equalTo(Optional.empty()));
     }
+
+    @Test
+    public void removingPropertiesRemovesRequiredPropertiesToo() {
+        Schema schema = Schema.builder()
+                .removeProperty("notThere")
+                .required(null)
+                .putProperty("foo", Schema.builder().build())
+                .putProperty("bar", Schema.builder().build())
+                .required(ListUtils.of("foo", "bar"))
+                .removeProperty("foo")
+                .build();
+
+        assertThat(schema.getProperties().keySet(), contains("bar"));
+        assertThat(schema.getRequired(), contains("bar"));
+    }
 }