Merge pull request #37 from invopop/unique-require

Avoid duplicate require entries when composing
invopop · Jul 25, 2022 · e8e691e · e8e691e
2 parents 6fa1420 + 4afb4d8
commit e8e691e
Show file tree

Hide file tree

Showing 10 changed files with 55 additions and 43 deletions.
diff --git a/fixtures/allow_additional_props.json b/fixtures/allow_additional_props.json
@@ -23,6 +23,9 @@
     },
     "TestUser": {
       "properties": {
+        "id": {
+          "type": "integer"
+        },
         "some_base_property": {
           "type": "integer"
         },
@@ -38,9 +41,6 @@
         "MapType": {
           "$ref": "#/$defs/MapType"
         },
-        "id": {
-          "type": "integer"
-        },
         "name": {
           "type": "string",
           "maxLength": 20,
@@ -202,12 +202,12 @@
       },
       "type": "object",
       "required": [
+        "id",
         "some_base_property",
         "grand",
         "SomeUntaggedBaseProperty",
         "PublicNonExported",
         "MapType",
-        "id",
         "name",
         "password",
         "TestFlag",

diff --git a/fixtures/defaults_expanded_toplevel.json b/fixtures/defaults_expanded_toplevel.json
@@ -23,6 +23,9 @@
     }
   },
   "properties": {
+    "id": {
+      "type": "integer"
+    },
     "some_base_property": {
       "type": "integer"
     },
@@ -38,9 +41,6 @@
     "MapType": {
       "$ref": "#/$defs/MapType"
     },
-    "id": {
-      "type": "integer"
-    },
     "name": {
       "type": "string",
       "maxLength": 20,
@@ -203,12 +203,12 @@
   "additionalProperties": false,
   "type": "object",
   "required": [
+    "id",
     "some_base_property",
     "grand",
     "SomeUntaggedBaseProperty",
     "PublicNonExported",
     "MapType",
-    "id",
     "name",
     "password",
     "TestFlag",

diff --git a/fixtures/ignore_type.json b/fixtures/ignore_type.json
@@ -17,6 +17,9 @@
     },
     "TestUser": {
       "properties": {
+        "id": {
+          "type": "integer"
+        },
         "some_base_property": {
           "type": "integer"
         },
@@ -32,9 +35,6 @@
         "MapType": {
           "$ref": "#/$defs/MapType"
         },
-        "id": {
-          "type": "integer"
-        },
         "name": {
           "type": "string",
           "maxLength": 20,
@@ -197,12 +197,12 @@
       "additionalProperties": false,
       "type": "object",
       "required": [
+        "id",
         "some_base_property",
         "grand",
         "SomeUntaggedBaseProperty",
         "PublicNonExported",
         "MapType",
-        "id",
         "name",
         "password",
         "TestFlag",

diff --git a/fixtures/no_reference.json b/fixtures/no_reference.json
@@ -2,6 +2,9 @@
   "$schema": "https://json-schema.org/draft/2020-12/schema",
   "$id": "https://github.com/invopop/jsonschema/test-user",
   "properties": {
+    "id": {
+      "type": "integer"
+    },
     "some_base_property": {
       "type": "integer"
     },
@@ -26,9 +29,6 @@
     "MapType": {
       "type": "object"
     },
-    "id": {
-      "type": "integer"
-    },
     "name": {
       "type": "string",
       "maxLength": 20,
@@ -192,12 +192,12 @@
   "additionalProperties": false,
   "type": "object",
   "required": [
+    "id",
     "some_base_property",
     "grand",
     "SomeUntaggedBaseProperty",
     "PublicNonExported",
     "MapType",
-    "id",
     "name",
     "password",
     "TestFlag",

diff --git a/fixtures/no_reference_anchor.json b/fixtures/no_reference_anchor.json
@@ -3,6 +3,9 @@
   "$id": "https://github.com/invopop/jsonschema/test-user",
   "$anchor": "TestUser",
   "properties": {
+    "id": {
+      "type": "integer"
+    },
     "some_base_property": {
       "type": "integer"
     },
@@ -28,9 +31,6 @@
     "MapType": {
       "type": "object"
     },
-    "id": {
-      "type": "integer"
-    },
     "name": {
       "type": "string",
       "maxLength": 20,
@@ -194,12 +194,12 @@
   "additionalProperties": false,
   "type": "object",
   "required": [
+    "id",
     "some_base_property",
     "grand",
     "SomeUntaggedBaseProperty",
     "PublicNonExported",
     "MapType",
-    "id",
     "name",
     "password",
     "TestFlag",

diff --git a/fixtures/required_from_jsontags.json b/fixtures/required_from_jsontags.json
@@ -24,6 +24,9 @@
     },
     "TestUser": {
       "properties": {
+        "id": {
+          "type": "integer"
+        },
         "some_base_property": {
           "type": "integer"
         },
@@ -39,9 +42,6 @@
         "MapType": {
           "$ref": "#/$defs/MapType"
         },
-        "id": {
-          "type": "integer"
-        },
         "name": {
           "type": "string",
           "maxLength": 20,

diff --git a/fixtures/test_user.json b/fixtures/test_user.json
@@ -24,6 +24,9 @@
     },
     "TestUser": {
       "properties": {
+        "id": {
+          "type": "integer"
+        },
         "some_base_property": {
           "type": "integer"
         },
@@ -39,9 +42,6 @@
         "MapType": {
           "$ref": "#/$defs/MapType"
         },
-        "id": {
-          "type": "integer"
-        },
         "name": {
           "type": "string",
           "maxLength": 20,
@@ -204,12 +204,12 @@
       "additionalProperties": false,
       "type": "object",
       "required": [
+        "id",
         "some_base_property",
         "grand",
         "SomeUntaggedBaseProperty",
         "PublicNonExported",
         "MapType",
-        "id",
         "name",
         "password",
         "TestFlag",

diff --git a/fixtures/test_user_assign_anchor.json b/fixtures/test_user_assign_anchor.json
@@ -26,6 +26,9 @@
     "TestUser": {
       "$anchor": "TestUser",
       "properties": {
+        "id": {
+          "type": "integer"
+        },
         "some_base_property": {
           "type": "integer"
         },
@@ -41,9 +44,6 @@
         "MapType": {
           "$ref": "#/$defs/MapType"
         },
-        "id": {
-          "type": "integer"
-        },
         "name": {
           "type": "string",
           "maxLength": 20,
@@ -206,12 +206,12 @@
       "additionalProperties": false,
       "type": "object",
       "required": [
+        "id",
         "some_base_property",
         "grand",
         "SomeUntaggedBaseProperty",
         "PublicNonExported",
         "MapType",
-        "id",
         "name",
         "password",
         "TestFlag",

diff --git a/reflect.go b/reflect.go
@@ -555,7 +555,7 @@ func (r *Reflector) reflectStructFields(st *Schema, definitions Definitions, t r
 
 		st.Properties.Set(name, property)
 		if required {
-			st.Required = append(st.Required, name)
+			st.Required = appendUniqueString(st.Required, name)
 		}
 	}
 
@@ -572,6 +572,15 @@ func (r *Reflector) reflectStructFields(st *Schema, definitions Definitions, t r
 	}
 }
 
+func appendUniqueString(base []string, value string) []string {
+	for _, v := range base {
+		if v == value {
+			return base
+		}
+	}
+	return append(base, value)
+}
+
 func (r *Reflector) lookupComment(t reflect.Type, name string) string {
 	if r.CommentMap == nil {
 		return ""

diff --git a/reflect_test.go b/reflect_test.go
@@ -29,7 +29,8 @@ type GrandfatherType struct {
 }
 
 type SomeBaseType struct {
-	SomeBaseProperty int `json:"some_base_property"`
+	ID               string `json:"id"` // to test composition override
+	SomeBaseProperty int    `json:"some_base_property"`
 	// The jsonschema required tag is nonsensical for private and ignored properties.
 	// Their presence here tests that the fields *will not* be required in the output
 	// schema, even if they are tagged required.
@@ -483,14 +484,16 @@ func TestArrayFormat(t *testing.T) {
 		TestURIs []string `jsonschema:"type=array,format=uri"`
 	}
 
-	var jsonReflector Reflector
-	schema := jsonReflector.Reflect(&URIArray{})
-
-	URIInterface, found := schema.Definitions["URIArray"].Properties.Get("TestURIs")
-	require.Equal(t, found, true)
-
-	var URIArrayProperties *Schema = URIInterface.(*Schema)
-
-	URIArrayType := URIArrayProperties.Items.Format
-	require.Equal(t, URIArrayType, "uri")
+	r := new(Reflector)
+	schema := r.Reflect(&URIArray{})
+	d := schema.Definitions["URIArray"]
+	require.NotNil(t, d)
+	props := d.Properties
+	require.NotNil(t, props)
+	i, found := props.Get("TestURIs")
+	require.True(t, found)
+
+	p := i.(*Schema)
+	pt := p.Items.Format
+	require.Equal(t, pt, "uri")
 }