fix: 1) adds supports for Diff to report on changed extension values #2984 and 2) detects request param extension changes #2983

Signed-off-by: Ahmad Moudani <ahmad.moudani@crowdstrike.com>

Author: Ahmad Moudani

diff/compatibility.go

@@ -36,6 +36,7 @@ func init() {
 			AddedConstraint:         NonBreaking,
 			DeletedExtension:        Warning,
 			AddedExtension:          Warning,
+			ChangedExtensionValue:   Warning,
 		},
 		ForRequest: map[SpecChangeCode]Compatibility{
 			AddedRequiredProperty:     Breaking,
@@ -70,6 +71,7 @@ func init() {
 			ChangedCollectionFormat:   Breaking,
 			DeletedExtension:          Warning,
 			AddedExtension:            Warning,
+			ChangedExtensionValue:     Warning,
 		},
 		ForChange: map[SpecChangeCode]Compatibility{
 			NoChangeDetected:          NonBreaking,
@@ -96,6 +98,7 @@ func init() {
 			DeletedDefinition:         NonBreaking,
 			DeletedExtension:          Warning,
 			AddedExtension:            Warning,
+			ChangedExtensionValue:     Warning,
 		},
 	}
 }

diff/difftypes.go

@@ -117,6 +117,8 @@ const (
 	DeletedExtension
 	// AddedExtension added an extension
 	AddedExtension
+	// ChangedExtensionValue changed an extension value
+	ChangedExtensionValue
 )
 
 var toLongStringSpecChangeCode = map[SpecChangeCode]string{
@@ -173,6 +175,7 @@ var toLongStringSpecChangeCode = map[SpecChangeCode]string{
 	ChangedCollectionFormat:   "Changed collection format",
 	DeletedExtension:          "Deleted Extension",
 	AddedExtension:            "Added Extension",
+	ChangedExtensionValue:     "Changed Extension Value",
 }
 
 var toStringSpecChangeCode = map[SpecChangeCode]string{
@@ -229,6 +232,7 @@ var toStringSpecChangeCode = map[SpecChangeCode]string{
 	ChangedCollectionFormat:   "ChangedCollectionFormat",
 	DeletedExtension:          "DeletedExtension",
 	AddedExtension:            "AddedExtension",
+	ChangedExtensionValue:     "ChangedExtensionValue",
 }
 
 var toIDSpecChangeCode = map[string]SpecChangeCode{}

diff/spec_analyser.go

@@ -2,6 +2,7 @@ package diff
 
 import (
 	"fmt"
+	"reflect"
 	"strings"
 
 	"github.com/go-openapi/spec"
@@ -293,6 +294,7 @@ func (sd *SpecAnalyser) analyseExtensions(spec1, spec2 *spec.Swagger) {
 	specLoc := DifferenceLocation{Node: &Node{Field: "Spec"}}
 	sd.checkAddedExtensions(spec1.Extensions, spec2.Extensions, specLoc, "")
 	sd.checkDeletedExtensions(spec1.Extensions, spec2.Extensions, specLoc, "")
+	sd.checkChangedExtensions(spec1.Extensions, spec2.Extensions, specLoc, "")
 
 	sd.analyzeInfoExtensions()
 	sd.analyzeTagExtensions(spec1, spec2)
@@ -302,19 +304,27 @@ func (sd *SpecAnalyser) analyseExtensions(spec1, spec2 *spec.Swagger) {
 }
 
 func (sd *SpecAnalyser) analyzeOperationExtensions() {
+	pathsIterated := make(map[string]struct{})
 	for urlMethod, op2 := range sd.urlMethods2 {
 		pathAndMethodLoc := DifferenceLocation{URL: urlMethod.Path, Method: urlMethod.Method}
 		if op1, ok := sd.urlMethods1[urlMethod]; ok {
-			sd.checkAddedExtensions(op1.Extensions, op2.Extensions, DifferenceLocation{URL: urlMethod.Path}, "")
+			if _, ok := pathsIterated[urlMethod.Path]; !ok {
+				sd.checkAddedExtensions(op1.Extensions, op2.Extensions, DifferenceLocation{URL: urlMethod.Path}, "")
+				sd.checkChangedExtensions(op1.Extensions, op2.Extensions, DifferenceLocation{URL: urlMethod.Path}, "")
+				pathsIterated[urlMethod.Path] = struct{}{}
+			}
 			sd.checkAddedExtensions(op1.Operation.Responses.Extensions, op2.Operation.Responses.Extensions, pathAndMethodLoc, "Responses")
+			sd.checkChangedExtensions(op1.Operation.Responses.Extensions, op2.Operation.Responses.Extensions, pathAndMethodLoc, "Responses")
 			sd.checkAddedExtensions(op1.Operation.Extensions, op2.Operation.Extensions, pathAndMethodLoc, "")
-
+			sd.checkChangedExtensions(op1.Operation.Extensions, op2.Operation.Extensions, pathAndMethodLoc, "")
+			sd.checkParamExtensions(op1, op2, urlMethod)
 			for code, resp := range op1.Operation.Responses.StatusCodeResponses {
 				for hdr, h := range resp.Headers {
 					op2StatusCode, ok := op2.Operation.Responses.StatusCodeResponses[code]
 					if ok {
 						if _, ok = op2StatusCode.Headers[hdr]; ok {
 							sd.checkAddedExtensions(h.Extensions, op2StatusCode.Headers[hdr].Extensions, DifferenceLocation{URL: urlMethod.Path, Method: urlMethod.Method, Node: getNameOnlyDiffNode("Headers")}, hdr)
+							sd.checkChangedExtensions(h.Extensions, op2StatusCode.Headers[hdr].Extensions, DifferenceLocation{URL: urlMethod.Path, Method: urlMethod.Method, Node: getNameOnlyDiffNode("Headers")}, hdr)
 						}
 					}
 				}
@@ -326,10 +336,14 @@ func (sd *SpecAnalyser) analyzeOperationExtensions() {
 		}
 	}
 
+	pathsIterated = make(map[string]struct{})
 	for urlMethod, op1 := range sd.urlMethods1 {
 		pathAndMethodLoc := DifferenceLocation{URL: urlMethod.Path, Method: urlMethod.Method}
 		if op2, ok := sd.urlMethods2[urlMethod]; ok {
-			sd.checkDeletedExtensions(op1.Extensions, op2.Extensions, DifferenceLocation{URL: urlMethod.Path}, "")
+			if _, ok := pathsIterated[urlMethod.Path]; !ok {
+				sd.checkDeletedExtensions(op1.Extensions, op2.Extensions, DifferenceLocation{URL: urlMethod.Path}, "")
+				pathsIterated[urlMethod.Path] = struct{}{}
+			}
 			sd.checkDeletedExtensions(op1.Operation.Responses.Extensions, op2.Operation.Responses.Extensions, pathAndMethodLoc, "Responses")
 			sd.checkDeletedExtensions(op1.Operation.Extensions, op2.Operation.Extensions, pathAndMethodLoc, "")
 			for code, resp := range op1.Operation.Responses.StatusCodeResponses {
@@ -346,11 +360,42 @@ func (sd *SpecAnalyser) analyzeOperationExtensions() {
 	}
 }
 
+func (sd *SpecAnalyser) checkParamExtensions(op1 *PathItemOp, op2 *PathItemOp, urlMethod URLMethod) {
+	locations := []string{"query", "path", "body", "header", "formData"}
+	titles := []string{"Query", "Path", "Body", "Header", "FormData"}
+
+	for i, paramLocation := range locations {
+		rootNode := getNameOnlyDiffNode(titles[i])
+		params1 := getParams(op1.ParentPathItem.Parameters, op1.Operation.Parameters, paramLocation)
+		params2 := getParams(op2.ParentPathItem.Parameters, op2.Operation.Parameters, paramLocation)
+
+		location := DifferenceLocation{URL: urlMethod.Path, Method: urlMethod.Method, Node: rootNode}
+		// detect deleted param extensions
+		for paramName1, param1 := range params1 {
+			if param2, ok := params2[paramName1]; ok {
+				childLocation := location.AddNode(getSchemaDiffNode(paramName1, &param1.SimpleSchema))
+				sd.checkDeletedExtensions(param1.Extensions, param2.Extensions, childLocation, "")
+			}
+		}
+
+		// detect added changed params
+		for paramName2, param2 := range params2 {
+			// changed?
+			if param1, ok := params1[paramName2]; ok {
+				childLocation := location.AddNode(getSchemaDiffNode(paramName2, &param1.SimpleSchema))
+				sd.checkAddedExtensions(param1.Extensions, param2.Extensions, childLocation, "")
+				sd.checkChangedExtensions(param1.Extensions, param2.Extensions, childLocation, "")
+			}
+		}
+	}
+}
+
 func (sd *SpecAnalyser) analyzeSecurityDefinitionExtensions(spec1 *spec.Swagger, spec2 *spec.Swagger) {
 	securityDefLoc := DifferenceLocation{Node: &Node{Field: "Security Definitions"}}
-	for key, securityDef := range spec1.SecurityDefinitions {
+	for key, securityDef1 := range spec1.SecurityDefinitions {
 		if securityDef2, ok := spec2.SecurityDefinitions[key]; ok {
-			sd.checkAddedExtensions(securityDef.Extensions, securityDef2.Extensions, securityDefLoc, "")
+			sd.checkAddedExtensions(securityDef1.Extensions, securityDef2.Extensions, securityDefLoc, "")
+			sd.checkChangedExtensions(securityDef1.Extensions, securityDef2.Extensions, securityDefLoc, "")
 		}
 	}
 
@@ -365,6 +410,7 @@ func (sd *SpecAnalyser) analyzeSchemaExtensions(schema1, schema2 *spec.Schema, c
 	if schema1 != nil && schema2 != nil {
 		diffLoc := DifferenceLocation{Response: code, URL: urlMethod.Path, Method: urlMethod.Method, Node: getSchemaDiffNode("Body", schema2)}
 		sd.checkAddedExtensions(schema1.Extensions, schema2.Extensions, diffLoc, "")
+		sd.checkChangedExtensions(schema1.Extensions, schema2.Extensions, diffLoc, "")
 		sd.checkDeletedExtensions(schema1.Extensions, schema2.Extensions, diffLoc, "")
 		if schema1.Items != nil && schema2.Items != nil {
 			sd.analyzeSchemaExtensions(schema1.Items.Schema, schema2.Items.Schema, code, urlMethod)
@@ -384,15 +430,18 @@ func (sd *SpecAnalyser) analyzeInfoExtensions() {
 		diffLocation := DifferenceLocation{Node: &Node{Field: "Spec Info"}}
 		sd.checkAddedExtensions(sd.Info1.Extensions, sd.Info2.Extensions, diffLocation, "")
 		sd.checkDeletedExtensions(sd.Info1.Extensions, sd.Info2.Extensions, diffLocation, "")
+		sd.checkChangedExtensions(sd.Info1.Extensions, sd.Info2.Extensions, diffLocation, "")
 		if sd.Info1.Contact != nil && sd.Info2.Contact != nil {
 			diffLocation = DifferenceLocation{Node: &Node{Field: "Spec Info.Contact"}}
 			sd.checkAddedExtensions(sd.Info1.Contact.Extensions, sd.Info2.Contact.Extensions, diffLocation, "")
 			sd.checkDeletedExtensions(sd.Info1.Contact.Extensions, sd.Info2.Contact.Extensions, diffLocation, "")
+			sd.checkChangedExtensions(sd.Info1.Contact.Extensions, sd.Info2.Contact.Extensions, diffLocation, "")
 		}
 		if sd.Info1.License != nil && sd.Info2.License != nil {
 			diffLocation = DifferenceLocation{Node: &Node{Field: "Spec Info.License"}}
 			sd.checkAddedExtensions(sd.Info1.License.Extensions, sd.Info2.License.Extensions, diffLocation, "")
 			sd.checkDeletedExtensions(sd.Info1.License.Extensions, sd.Info2.License.Extensions, diffLocation, "")
+			sd.checkChangedExtensions(sd.Info1.License.Extensions, sd.Info2.License.Extensions, diffLocation, "")
 		}
 	}
 }
@@ -403,6 +452,7 @@ func (sd *SpecAnalyser) analyzeTagExtensions(spec1 *spec.Swagger, spec2 *spec.Sw
 		for _, spec1Tag := range spec1.Tags {
 			if spec2Tag.Name == spec1Tag.Name {
 				sd.checkAddedExtensions(spec1Tag.Extensions, spec2Tag.Extensions, diffLocation, "")
+				sd.checkChangedExtensions(spec1Tag.Extensions, spec2Tag.Extensions, diffLocation, "")
 			}
 		}
 	}
@@ -430,6 +480,21 @@ func (sd *SpecAnalyser) checkAddedExtensions(extensions1 spec.Extensions, extens
 	}
 }
 
+func (sd *SpecAnalyser) checkChangedExtensions(extensions1 spec.Extensions, extensions2 spec.Extensions, diffLocation DifferenceLocation, fieldPrefix string) {
+	for extKey, ext2Val := range extensions2 {
+		if ext1Val, ok := extensions1[extKey]; ok && !reflect.DeepEqual(ext1Val, ext2Val) {
+			if fieldPrefix != "" {
+				extKey = fmt.Sprintf("%s.%s", fieldPrefix, extKey)
+			}
+			sd.Diffs = sd.Diffs.addDiff(SpecDifference{
+				DifferenceLocation: diffLocation.AddNode(&Node{Field: extKey}),
+				Code:               ChangedExtensionValue,
+				Compatibility:      Warning, // this could potentially be a breaking change
+			})
+		}
+	}
+}
+
 func (sd *SpecAnalyser) checkDeletedExtensions(extensions1 spec.Extensions, extensions2 spec.Extensions, diffLocation DifferenceLocation, fieldPrefix string) {
 	for extKey := range extensions1 {
 		if _, ok := extensions2[extKey]; !ok {