Hardening model whilst testing what-changed feature.

Checking and sampling, and fixing bugs while working through testing.

Signed-off-by: Dave Shanley <dshanley@splunk.com>

datamodel/low/model_interfaces.go

@@ -98,6 +98,7 @@ type OpenAPIParameter interface {
 type SharedOperations interface {
 	//HasDescription
 	//HasExternalDocs
+	GetOperationId() NodeReference[string]
 	GetExternalDocs() NodeReference[any]
 	GetDescription() NodeReference[string]
 	GetTags() NodeReference[[]ValueReference[string]]

datamodel/low/v3/path_item.go

@@ -124,7 +124,7 @@ func (p *PathItem) Build(root *yaml.Node, idx *index.SpecIndex) error {
 		}
 	}
 
-	_, ln, vn = utils.FindKeyNodeFull(ServersLabel, root.Content)
+	_, ln, vn = utils.FindKeyNodeFullTop(ServersLabel, root.Content)
 	if vn != nil {
 		if utils.IsNodeArray(vn) {
 			var servers []low.ValueReference[*Server]

test_specs/burgershop.openapi-modified.yaml

@@ -9,22 +9,25 @@ info:
     email: buckaroo@pb33f.io
     url: https://pb33f.io
   license:
-    name: pb33f
+    name: pb33f-internal
     url: https://pb33f.io/made-up
   version: "1.2"
 security:
   - OAuthScheme:
       - read:burgers
       - write:burgers
+      - read:books
 tags:
+  - name: HotDogs
+    description: a new type of burger, its a thin round one in a bun.
   - name: "Burgers"
-    description: "All kinds of yummy burgers."
+    description: All kinds of yummy burgers, the very best in the world.
     externalDocs:
       description: "Find out more"
       url: "https://pb33f.io"
     x-internal-ting: somethingSpecial
     x-internal-tong: 1
-    x-internal-tang: 1.2
+    x-internal-tang: 1.2.3
     x-internal-tung: true
     x-internal-arr:
       - one
@@ -42,10 +45,10 @@ tags:
       url: "https://pb33f.io"
 servers:
   - url: "{scheme}://api.pb33f.io"
-    description: "this is our main API server, for all fun API things."
+    description: "this is our main API server, for all fun API things. updated"
     variables:
       scheme:
-        enum: [https, wss]
+        enum: [https]
         default: https
         description: this is a server variable for the scheme
   - url: "https://{domain}.{host}.com"
@@ -58,15 +61,16 @@ servers:
         default: "pb33f.io"
         description: the default host for this API is 'pb33f.io'
 paths:
-  x-milky-milk: milky
+  x-milky-milk: milky updated
   /burgers:
-    x-burger-meta: meaty
+    x-burger-meta: meaty pop
     post:
-      operationId: createBurger
+      operationId: createBurgerChanged
       tags:
         - "Burgers"
-      summary:  Create a new burger
-      description: A new burger for our menu, yummy yum yum.
+        - "HotDogs"
+      summary:  Create a new burger the changed
+      description: A new burger for our menu
       requestBody:
         $ref: '#/components/requestBodies/BurgerRequest'
       responses:
@@ -74,7 +78,7 @@ paths:
           headers:
             UseOil:
               $ref: '#/components/headers/UseOil'
-          description: A tasty burger for you to eat.
+          description: A tasty burger for you to eat. update
           content:
             application/json:
               schema:
@@ -83,11 +87,13 @@ paths:
                 quarterPounder:
                   $ref: '#/components/examples/QuarterPounder'
                 filetOFish:
-                  summary: a cripsy fish sammich filled with ocean goodness.
+                  summary: a cripsy fish sammich filled with ocean goodness. changed
                   value:
                     name: Filet-O-Fish
-                    numPatties: 1
+                    numPatties: 22
           links:
+            DuplicateLocateBurger:
+              $ref: '#/components/links/LocateBurger'
             LocateBurger:
               $ref: '#/components/links/LocateBurger'
             AnotherLocateBurger:
@@ -103,24 +109,16 @@ paths:
                   summary: oh my goodness
                   value:
                     message: something went terribly wrong my friend, no new burger for you. mate.
-        "422":
-          description: Unprocessable entity
-          content:
-            application/json:
-              schema:
-                $ref: '#/components/schemas/Error'
-              examples:
-                unexpectedError:
-                  summary: invalid request
-                  value:
-                    message: unable to accept this request, looks bad, missing something.
       security:
         - OAuthScheme:
             - read:burgers
             - write:burgers
+            - read:books
       servers:
         - url: https://pb33f.io
           description: this is an alternative server for this operation.
+        - url: https://pb33f.io/new
+          description: this is an alternative server, copy pasta.
   /burgers/{burgerId}:
     get:
       callbacks:

what-changed/model/document.go

@@ -15,7 +15,7 @@ type DocumentChanges struct {
 	PropertyChanges
 	InfoChanges                *InfoChanges
 	PathsChanges               *PathsChanges
-	TagChanges                 *TagChanges
+	TagChanges                 []*TagChanges
 	ExternalDocChanges         *ExternalDocChanges
 	WebhookChanges             map[string]*PathItemChanges
 	ServerChanges              []*ServerChanges
@@ -32,8 +32,8 @@ func (d *DocumentChanges) TotalChanges() int {
 	if d.PathsChanges != nil {
 		c += d.PathsChanges.TotalChanges()
 	}
-	if d.TagChanges != nil {
-		c += d.TagChanges.TotalChanges()
+	for k := range d.TagChanges {
+		c += d.TagChanges[k].TotalChanges()
 	}
 	if d.ExternalDocChanges != nil {
 		c += d.ExternalDocChanges.TotalChanges()
@@ -64,8 +64,8 @@ func (d *DocumentChanges) TotalBreakingChanges() int {
 	if d.PathsChanges != nil {
 		c += d.PathsChanges.TotalBreakingChanges()
 	}
-	if d.TagChanges != nil {
-		c += d.TagChanges.TotalBreakingChanges()
+	for k := range d.TagChanges {
+		c += d.TagChanges[k].TotalBreakingChanges()
 	}
 	if d.ExternalDocChanges != nil {
 		c += d.ExternalDocChanges.TotalBreakingChanges()
@@ -213,7 +213,7 @@ func CompareDocuments(l, r any) *DocumentChanges {
 		}
 
 		// compare servers
-		if n := checkServers(lDoc.Servers, rDoc.Servers, &changes); n != nil {
+		if n := checkServers(lDoc.Servers, rDoc.Servers); n != nil {
 			dc.ServerChanges = n
 		}
 

what-changed/model/operation.go

@@ -16,13 +16,9 @@ import (
 
 type OperationChanges struct {
 	PropertyChanges
-	ExternalDocChanges *ExternalDocChanges
-	ParameterChanges   []*ParameterChanges
-	ResponsesChanges   *ResponsesChanges
-
-	// SecurityRequirementChanges are defined differently between v2 and v3. Both are defined
-	// as the same data type, however the objects are structured differently. A slice is the cleanest
-	// way to compose both models.
+	ExternalDocChanges         *ExternalDocChanges
+	ParameterChanges           []*ParameterChanges
+	ResponsesChanges           *ResponsesChanges
 	SecurityRequirementChanges []*SecurityRequirementChanges
 
 	// v3
@@ -105,6 +101,10 @@ func addSharedOperationProperties(left, right low.SharedOperations, changes *[]*
 	addPropertyCheck(&props, left.GetDeprecated().ValueNode, right.GetDeprecated().ValueNode,
 		left.GetDeprecated(), right.GetDeprecated(), changes, v3.DeprecatedLabel, false)
 
+	// operation id
+	addPropertyCheck(&props, left.GetOperationId().ValueNode, right.GetOperationId().ValueNode,
+		left.GetOperationId(), right.GetOperationId(), changes, v3.OperationIdLabel, true)
+
 	return props
 }
 
@@ -343,7 +343,7 @@ func CompareOperations(l, r any) *OperationChanges {
 		}
 
 		// servers
-		oc.ServerChanges = checkServers(lOperation.Servers, rOperation.Servers, &changes)
+		oc.ServerChanges = checkServers(lOperation.Servers, rOperation.Servers)
 		oc.ExtensionChanges = CompareExtensions(lOperation.Extensions, rOperation.Extensions)
 
 		// todo: callbacks
@@ -353,8 +353,7 @@ func CompareOperations(l, r any) *OperationChanges {
 	return oc
 }
 
-func checkServers(lServers, rServers low.NodeReference[[]low.ValueReference[*v3.Server]],
-	changes *[]*Change) []*ServerChanges {
+func checkServers(lServers, rServers low.NodeReference[[]low.ValueReference[*v3.Server]]) []*ServerChanges {
 
 	var serverChanges []*ServerChanges
 
@@ -383,34 +382,58 @@ func checkServers(lServers, rServers low.NodeReference[[]low.ValueReference[*v3.
 		}
 
 		for k := range lv {
+
+			var changes []*Change
+
 			if _, ok := rv[k]; ok {
 				if !low.AreEqual(lv[k].Value, rv[k].Value) {
 					serverChanges = append(serverChanges, CompareServers(lv[k].Value, rv[k].Value))
 				}
 				continue
 			}
-			CreateChange(changes, ObjectRemoved, v3.ServersLabel,
+			lv[k].ValueNode.Value = lv[k].Value.URL.Value
+			CreateChange(&changes, ObjectRemoved, v3.ServersLabel,
 				lv[k].ValueNode, nil, true, lv[k].Value.URL.Value,
 				nil)
+			sc := new(ServerChanges)
+			sc.Changes = changes
+			serverChanges = append(serverChanges, sc)
+
 		}
+
 		for k := range rv {
+
 			if _, ok := lv[k]; !ok {
-				CreateChange(changes, ObjectAdded, v3.ServersLabel,
-					rv[k].ValueNode, nil, false, rv[k].Value.URL.Value,
-					nil)
+
+				var changes []*Change
+				rv[k].ValueNode.Value = rv[k].Value.URL.Value
+				CreateChange(&changes, ObjectAdded, v3.ServersLabel,
+					nil, rv[k].ValueNode, false, nil,
+					rv[k].Value.URL.Value)
+
+				sc := new(ServerChanges)
+				sc.Changes = changes
+				serverChanges = append(serverChanges, sc)
 			}
+
 		}
 	}
+	var changes []*Change
+	sc := new(ServerChanges)
 	if !lServers.IsEmpty() && rServers.IsEmpty() {
-		CreateChange(changes, PropertyRemoved, v3.ServersLabel,
-			lServers.ValueNode, nil, true, lServers.ValueNode,
+		CreateChange(&changes, PropertyRemoved, v3.ServersLabel,
+			lServers.ValueNode, nil, true, lServers.Value,
 			nil)
 	}
 	if lServers.IsEmpty() && !rServers.IsEmpty() {
-		CreateChange(changes, PropertyAdded, v3.ServersLabel,
+		CreateChange(&changes, PropertyAdded, v3.ServersLabel,
 			nil, rServers.ValueNode, false, nil,
 			rServers.Value)
 	}
+	sc.Changes = changes
+	if len(changes) > 0 {
+		serverChanges = append(serverChanges, sc)
+	}
 	if len(serverChanges) <= 0 {
 		return nil
 	}

what-changed/model/path_item.go

@@ -526,7 +526,7 @@ func compareOpenAPIPathItem(lPath, rPath *v3.PathItem, changes *[]*Change, pc *P
 	}
 
 	// servers
-	pc.ServerChanges = checkServers(lPath.Servers, rPath.Servers, changes)
+	pc.ServerChanges = checkServers(lPath.Servers, rPath.Servers)
 
 	// parameters
 	if !lPath.Parameters.IsEmpty() && !rPath.Parameters.IsEmpty() {

what-changed/model/tags.go

@@ -7,7 +7,6 @@ import (
 	"github.com/pb33f/libopenapi/datamodel/low"
 	"github.com/pb33f/libopenapi/datamodel/low/base"
 	"github.com/pb33f/libopenapi/datamodel/low/v3"
-	"strings"
 )
 
 // TagChanges represents changes made to the Tags object of an OpenAPI document.
@@ -37,25 +36,28 @@ func (t *TagChanges) TotalBreakingChanges() int {
 // CompareTags will compare a left (original) and a right (new) slice of ValueReference nodes for
 // any changes between them. If there are changes, a pointer to TagChanges is returned, if not then
 // nil is returned instead.
-func CompareTags(l, r []low.ValueReference[*base.Tag]) *TagChanges {
-	tc := new(TagChanges)
+func CompareTags(l, r []low.ValueReference[*base.Tag]) []*TagChanges {
+
+	var tagResults []*TagChanges
 
 	// look at the original and then look through the new.
 	seenLeft := make(map[string]*low.ValueReference[*base.Tag])
 	seenRight := make(map[string]*low.ValueReference[*base.Tag])
 	for i := range l {
 		h := l[i]
-		seenLeft[strings.ToLower(l[i].Value.Name.Value)] = &h
+		seenLeft[l[i].Value.Name.Value] = &h
 	}
 	for i := range r {
 		h := r[i]
-		seenRight[strings.ToLower(r[i].Value.Name.Value)] = &h
+		seenRight[r[i].Value.Name.Value] = &h
 	}
 
-	var changes []*Change
+	//var changes []*Change
 
 	// check for removals, modifications and moves
 	for i := range seenLeft {
+		tc := new(TagChanges)
+		var changes []*Change
 
 		CheckForObjectAdditionOrRemoval[*base.Tag](seenLeft, seenRight, i, &changes, false, true)
 
@@ -104,20 +106,32 @@ func CompareTags(l, r []low.ValueReference[*base.Tag]) *TagChanges {
 			}
 
 			// check extensions
-			tc.ExtensionChanges = CheckExtensions(seenLeft[i].GetValue(), seenRight[i].GetValue())
-
+			tc.ExtensionChanges = CompareExtensions(seenLeft[i].Value.Extensions, seenRight[i].Value.Extensions)
+			tc.Changes = changes
+			if tc.TotalChanges() > 0 {
+				tagResults = append(tagResults, tc)
+			}
+			continue
 		}
-	}
 
+		if len(changes) > 0 {
+			tc.Changes = changes
+			tagResults = append(tagResults, tc)
+		}
+
+	}
 	for i := range seenRight {
 		if seenLeft[i] == nil {
+			tc := new(TagChanges)
+			var changes []*Change
+
 			CreateChange(&changes, ObjectAdded, i, nil, seenRight[i].GetValueNode(),
 				false, nil, seenRight[i].GetValue())
+
+			tc.Changes = changes
+			tagResults = append(tagResults, tc)
+
 		}
 	}
-	tc.Changes = changes
-	if tc.TotalChanges() <= 0 {
-		return nil
-	}
-	return tc
+	return tagResults
 }

what-changed/model/tags_test.go

@@ -40,12 +40,12 @@ tags:
 	changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value)
 
 	// evaluate.
-	assert.Len(t, changes.Changes, 1)
-	assert.Len(t, changes.ExternalDocs.Changes, 2)
-	assert.Len(t, changes.ExtensionChanges.Changes, 1)
-	assert.Equal(t, 4, changes.TotalChanges())
+	assert.Len(t, changes[0].Changes, 1)
+	assert.Len(t, changes[0].ExternalDocs.Changes, 2)
+	assert.Len(t, changes[0].ExtensionChanges.Changes, 1)
+	assert.Equal(t, 4, changes[0].TotalChanges())
 
-	descChange := changes.Changes[0]
+	descChange := changes[0].Changes[0]
 	assert.Equal(t, "a lovelier tag description", descChange.New)
 	assert.Equal(t, "a lovely tag", descChange.Original)
 	assert.Equal(t, Modified, descChange.ChangeType)
@@ -84,10 +84,10 @@ tags:
 	changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value)
 
 	// evaluate.
-	assert.Len(t, changes.Changes, 1)
-	assert.Equal(t, 1, changes.TotalChanges())
+	assert.Len(t, changes[0].Changes, 1)
+	assert.Equal(t, 1, changes[0].TotalChanges())
 
-	descChange := changes.Changes[0]
+	descChange := changes[0].Changes[0]
 	assert.Equal(t, ObjectAdded, descChange.ChangeType)
 }
 
@@ -117,12 +117,10 @@ tags:
 	changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value)
 
 	// evaluate.
-	assert.Len(t, changes.Changes, 2)
-	assert.Equal(t, 2, changes.TotalChanges())
-
-	assert.Equal(t, ObjectRemoved, changes.Changes[0].ChangeType)
-	assert.Equal(t, ObjectAdded, changes.Changes[1].ChangeType)
-	assert.Equal(t, 1, changes.TotalBreakingChanges())
+	assert.Len(t, changes, 2)
+	assert.Equal(t, 1, changes[0].TotalChanges())
+	assert.Equal(t, 1, changes[1].TotalChanges())
+	assert.Equal(t, 1, changes[0].TotalBreakingChanges())
 }
 
 func TestCompareTags_DescriptionMoved(t *testing.T) {
@@ -222,10 +220,10 @@ tags:
 	changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value)
 
 	// evaluate.
-	assert.Len(t, changes.Changes, 1)
-	assert.Equal(t, 1, changes.TotalChanges())
+	assert.Len(t, changes[0].Changes, 1)
+	assert.Equal(t, 1, changes[0].TotalChanges())
 
-	descChange := changes.Changes[0]
+	descChange := changes[0].Changes[0]
 	assert.Equal(t, Modified, descChange.ChangeType)
 	assert.Equal(t, "a lovelier tag description", descChange.Original)
 	assert.Equal(t, "a different tag description", descChange.New)
@@ -289,8 +287,8 @@ tags:
 	changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value)
 
 	// evaluate.
-	assert.Equal(t, 1, changes.TotalChanges())
-	assert.Equal(t, ObjectAdded, changes.Changes[0].ChangeType)
+	assert.Equal(t, 1, changes[0].TotalChanges())
+	assert.Equal(t, ObjectAdded, changes[0].Changes[0].ChangeType)
 
 }
 
@@ -317,7 +315,7 @@ tags:
 	changes := CompareTags(rDoc.Tags.Value, lDoc.Tags.Value)
 
 	// evaluate.
-	assert.Equal(t, 1, changes.TotalChanges())
-	assert.Equal(t, ObjectRemoved, changes.Changes[0].ChangeType)
+	assert.Equal(t, 1, changes[0].TotalChanges())
+	assert.Equal(t, ObjectRemoved, changes[0].Changes[0].ChangeType)
 
 }

what-changed/what_changed_test.go

@@ -22,8 +22,8 @@ func TestCompareOpenAPIDocuments(t *testing.T) {
 	modDoc, _ := v3.CreateDocument(infoMod)
 
 	changes := CompareOpenAPIDocuments(origDoc, modDoc)
-	assert.Equal(t, 1, changes.TotalChanges())
-	assert.Equal(t, 0, changes.TotalBreakingChanges())
+	assert.Equal(t, 21, changes.TotalChanges())
+	assert.Equal(t, 3, changes.TotalBreakingChanges())
 
 }