Addressd issue #218

Signed-off-by: quobix <dave@quobix.com>
pb33f · Jan 18, 2024 · dc6aa76 · dc6aa76
1 parent a871b57
commit dc6aa76
Show file tree

Hide file tree

Showing 3 changed files with 139 additions and 10 deletions.
diff --git a/what-changed/model/schema.go b/what-changed/model/schema.go
@@ -347,18 +347,30 @@ func CompareSchemas(l, r *base.SchemaProxy) *SchemaChanges {
 
 		// changed from inline to ref
 		if !l.IsReference() && r.IsReference() {
-			CreateChange(&changes, Modified, v3.RefLabel,
-				l.GetValueNode(), r.GetValueNode().Content[1], true, l, r.GetReference())
-			sc.PropertyChanges = NewPropertyChanges(changes)
-			return sc // we're done here
+			// check if the referenced schema matches or not
+			// https://github.com/pb33f/libopenapi/issues/218
+			lHash := l.Schema().Hash()
+			rHash := r.Schema().Hash()
+			if lHash != rHash {
+				CreateChange(&changes, Modified, v3.RefLabel,
+					l.GetValueNode(), r.GetValueNode().Content[1], true, l, r.GetReference())
+				sc.PropertyChanges = NewPropertyChanges(changes)
+				return sc // we're done here
+			}
 		}
 
 		// changed from ref to inline
 		if l.IsReference() && !r.IsReference() {
-			CreateChange(&changes, Modified, v3.RefLabel,
-				l.GetValueNode().Content[1], r.GetValueNode(), true, l.GetReference(), r)
-			sc.PropertyChanges = NewPropertyChanges(changes)
-			return sc // done, nothing else to do.
+			// check if the referenced schema matches or not
+			// https://github.com/pb33f/libopenapi/issues/218
+			lHash := l.Schema().Hash()
+			rHash := r.Schema().Hash()
+			if lHash != rHash {
+				CreateChange(&changes, Modified, v3.RefLabel,
+					l.GetValueNode().Content[1], r.GetValueNode(), true, l.GetReference(), r)
+				sc.PropertyChanges = NewPropertyChanges(changes)
+				return sc // done, nothing else to do.
+			}
 		}
 
 		lSchema := l.Schema()

diff --git a/what-changed/model/schema_test.go b/what-changed/model/schema_test.go
@@ -2856,3 +2856,89 @@ func TestSchemaChanges_TotalBreakingChanges_NoNilPanic(t *testing.T) {
 func TestCompareSchemas_Nil(t *testing.T) {
 	assert.Nil(t, CompareSchemas(nil, nil))
 }
+
+// Test for issue https://github.com/pb33f/libopenapi/issues/218
+func TestCompareSchemas_PropertyRefChange_Identical(t *testing.T) {
+	left := `openapi: 3.0
+components:
+  schemas:
+    Yo:
+      type: int 
+    OK:
+      $ref: '#/components/schemas/Yo'`
+
+	right := `openapi: 3.0
+components:
+  schemas:
+    Yo:
+      type: int 
+    OK:
+      type: int`
+
+	leftDoc, rightDoc := test_BuildDoc(left, right)
+
+	// extract left reference schema and non reference schema.
+	lSchemaProxy := leftDoc.Components.Value.FindSchema("OK").Value
+	rSchemaProxy := rightDoc.Components.Value.FindSchema("OK").Value
+
+	changes := CompareSchemas(lSchemaProxy, rSchemaProxy)
+	assert.Nil(t, changes)
+
+}
+
+func TestCompareSchemas_PropertyRefChange_IdenticalReverse(t *testing.T) {
+	left := `openapi: 3.0
+components:
+  schemas:
+    Yo:
+      type: int 
+    OK:
+      $ref: '#/components/schemas/Yo'`
+
+	right := `openapi: 3.0
+components:
+  schemas:
+    Yo:
+      type: int 
+    OK:
+      type: int`
+
+	leftDoc, rightDoc := test_BuildDoc(right, left)
+
+	// extract left reference schema and non reference schema.
+	lSchemaProxy := leftDoc.Components.Value.FindSchema("OK").Value
+	rSchemaProxy := rightDoc.Components.Value.FindSchema("OK").Value
+
+	changes := CompareSchemas(lSchemaProxy, rSchemaProxy)
+	assert.Nil(t, changes)
+
+}
+
+func TestCompareSchemas_PropertyRefChange_Fail(t *testing.T) {
+	left := `openapi: 3.0
+components:
+  schemas:
+    Yo:
+      type: int 
+    OK:
+      $ref: '#/components/schemas/Yo'`
+
+	right := `openapi: 3.0
+components:
+  schemas:
+    Yo:
+      type: int 
+    OK:
+      type: string`
+
+	leftDoc, rightDoc := test_BuildDoc(left, right)
+
+	// extract left reference schema and non reference schema.
+	lSchemaProxy := leftDoc.Components.Value.FindSchema("OK").Value
+	rSchemaProxy := rightDoc.Components.Value.FindSchema("OK").Value
+
+	changes := CompareSchemas(lSchemaProxy, rSchemaProxy)
+	assert.NotNil(t, changes)
+	assert.Equal(t, 1, changes.TotalChanges())
+
+}
diff --git a/what-changed/what_changed_test.go b/what-changed/what_changed_test.go
@@ -27,8 +27,7 @@ func TestCompareOpenAPIDocuments(t *testing.T) {
 	changes := CompareOpenAPIDocuments(origDoc, modDoc)
 	assert.Equal(t, 75, changes.TotalChanges())
 	assert.Equal(t, 20, changes.TotalBreakingChanges())
-	//out, _ := json.MarshalIndent(changes, "", "  ")
-	//_ = os.WriteFile("outputv3.json", out, 0776)
+
 }
 
 func TestCompareSwaggerDocuments(t *testing.T) {
@@ -45,6 +44,38 @@ func TestCompareSwaggerDocuments(t *testing.T) {
 	assert.Equal(t, 52, changes.TotalChanges())
 	assert.Equal(t, 27, changes.TotalBreakingChanges())
 
+}
+
+func TestCompareRefs(t *testing.T) {
+
+	original := []byte(`openapi: 3.0
+components:
+  schemas:
+    Yo:
+      type: int 
+    OK:
+      $ref: '#/components/schemas/Yo'
+`)
+
+	modified := []byte(`openapi: 3.0
+components:
+  schemas:
+    Yo:
+      type: int 
+    OK:
+      type: int
+`)
+
+	infoOrig, _ := datamodel.ExtractSpecInfo(original)
+	infoMod, _ := datamodel.ExtractSpecInfo(modified)
+
+	origDoc, _ := v3.CreateDocumentFromConfig(infoOrig, datamodel.NewDocumentConfiguration())
+	modDoc, _ := v3.CreateDocumentFromConfig(infoMod, datamodel.NewDocumentConfiguration())
+
+	changes := CompareOpenAPIDocuments(origDoc, modDoc)
+	assert.Equal(t, 52, changes.TotalChanges())
+	assert.Equal(t, 27, changes.TotalBreakingChanges())
+
 	//out, _ := json.MarshalIndent(changes, "", "  ")
 	//_ = os.WriteFile("output.json", out, 0776)