dedicated diff for components.params (#354)

Tufin · Aug 2, 2023 · f40f595 · f40f595
1 parent 3a7f646
commit f40f595
Show file tree

Hide file tree

Showing 17 changed files with 424 additions and 67 deletions.
diff --git a/checker/checker.go b/checker/checker.go
@@ -231,7 +231,7 @@ func (d *BCDiff) AddModifiedPath(path string) *diff.PathDiff {
 func (diffBC *BCDiff) AddModifiedParameter(path string, operation string, paramLocation string, paramName string) *diff.ParameterDiff {
 	opDiff := diffBC.AddModifiedOperation(path, operation)
 	if opDiff.ParametersDiff == nil {
-		opDiff.ParametersDiff = &diff.ParametersDiff{}
+		opDiff.ParametersDiff = &diff.ParametersDiffByLocation{}
 	}
 	if opDiff.ParametersDiff.Modified == nil {
 		opDiff.ParametersDiff.Modified = make(diff.ParamDiffByLocation)

diff --git a/data/different_component_modified_parameter.yaml b/data/different_component_modified_parameter.yaml
@@ -0,0 +1,25 @@
+openapi: 3.0.3
+
+components:
+  parameters:
+    differentComponentName_A:
+      name: sameParamName
+      in: query
+      required: true
+      schema:
+        type: string
+        enum:
+          - option_1
+          - option_2
+        default: option_1
+      description: Let's mention both options, option_1 and option_2.
+    differentComponentName_B:
+      name: sameParamName
+      in: query
+      required: false
+      schema:
+        type: string
+        enum:
+          - option_1
+        default: option_1
+      description: Let's mention the only option, option_1.
diff --git a/data/different_component_same_header.yaml b/data/different_component_same_header.yaml
@@ -0,0 +1,20 @@
+openapi: 3.0.3
+
+components:
+  headers:
+    differentComponentName_A:
+      schema:
+        type: string
+        enum:
+          - option_1
+          - option_2
+        default: option_1
+      description: Let's mention both options, option_1 and option_2.
+    differentComponentName_B:
+      schema:
+        type: string
+        enum:
+          - option_1
+        default: option_1
+      description: Let's mention the only option, option_1.
+
diff --git a/data/different_component_same_parameter.yaml b/data/different_component_same_parameter.yaml
@@ -0,0 +1,25 @@
+openapi: 3.0.3
+
+components:
+  parameters:
+    differentComponentName_A:
+      name: sameParamName
+      in: header
+      required: true
+      schema:
+        type: string
+        enum:
+          - option_1
+          - option_2
+        default: option_1
+      description: Let's mention both options, option_1 and option_2.
+    differentComponentName_B:
+      name: sameParamName
+      in: query
+      required: false
+      schema:
+        type: string
+        enum:
+          - option_1
+        default: option_1
+      description: Let's mention the only option, option_1.
diff --git a/data/different_component_same_schema.yaml b/data/different_component_same_schema.yaml
@@ -0,0 +1,10 @@
+openapi: 3.0.3
+
+components:
+  schemas:
+    differentComponentName_A:
+      name: sameParamName
+      type: string
+    differentComponentName_B:
+      name: sameParamName
+      type: string
diff --git a/data/param-rename/op-base-ref.yaml b/data/param-rename/op-base-ref.yaml
@@ -0,0 +1,51 @@
+openapi: 3.0.0
+
+info:
+  title: My API
+  version: '0.1'
+
+paths:
+  /books/{bookId}:
+
+    get:
+      summary: get book details
+      description: get details for a book
+
+      parameters:
+        - $ref: "#/components/parameters/bookId"
+
+      responses:
+        '200':
+          description: successfully retrieved book details
+
+          content:
+            application/json:
+              schema:
+                title: book details
+                type: object
+
+                required:
+                  - author
+                  - title
+
+                properties:
+                  author:
+                    title: book author name
+                    type: string
+
+                  title:
+                    title: book title
+                    type: string
+
+components:
+  parameters:
+    bookId:
+      in: path
+      name: bookId
+      description: book ID
+      required: true
+
+      schema:
+        title: ID
+        type: string
+
diff --git a/data/param-rename/op-revision-ref.yaml b/data/param-rename/op-revision-ref.yaml
@@ -0,0 +1,51 @@
+openapi: 3.0.0
+
+info:
+  title: My API
+  version: '0.1'
+
+paths:
+  /books/{id}:
+
+    get:
+      summary: get book details
+      description: get details for a book
+
+      parameters:
+        - $ref: "#/components/parameters/id"
+
+      responses:
+        '200':
+          description: successfully retrieved book details
+
+          content:
+            application/json:
+              schema:
+                title: book details
+                type: object
+
+                required:
+                  - author
+                  - title
+
+                properties:
+                  author:
+                    title: book author name
+                    type: string
+
+                  title:
+                    title: book title
+                    type: string
+
+components:
+  parameters:
+    id:
+        in: path
+        name: id
+        description: book ID
+        required: true
+
+        schema:
+          title: ID
+          type: string
+
diff --git a/diff/components_diff.go b/diff/components_diff.go
@@ -36,7 +36,7 @@ func getComponentsDiffInternal(config *Config, state *state, s1, s2 openapi3.Com
 		return result, err
 	}
 
-	result.ParametersDiff, err = getParametersDiff(config, state, toParameters(s1.Parameters), toParameters(s2.Parameters), PathParamsMap{})
+	result.ParametersDiff, err = getParametersDiff(config, state, s1.Parameters, s2.Parameters, PathParamsMap{})
 	if err != nil {
 		return result, err
 	}

diff --git a/diff/diff_test.go b/diff/diff_test.go
@@ -41,7 +41,7 @@ func TestDiff_Empty(t *testing.T) {
 	require.True(t, (*diff.ExtensionsDiff)(nil).Empty())
 	require.True(t, (*diff.HeadersDiff)(nil).Empty())
 	require.True(t, (*diff.OperationsDiff)(nil).Empty())
-	require.True(t, (*diff.ParametersDiff)(nil).Empty())
+	require.True(t, (*diff.ParametersDiffByLocation)(nil).Empty())
 	require.True(t, (*diff.RequestBodiesDiff)(nil).Empty())
 	require.True(t, (*diff.ResponsesDiff)(nil).Empty())
 	require.True(t, (*diff.SchemasDiff)(nil).Empty())
@@ -772,6 +772,29 @@ func TestDiff_PathParamInOperationRenamed(t *testing.T) {
 	require.Equal(t, "id", dd.To)
 }
 
+func TestDiff_PathParamRefInOperationRenamed(t *testing.T) {
+	loader := openapi3.NewLoader()
+
+	s1, err := loader.LoadFromFile("../data/param-rename/op-base.yaml")
+	require.NoError(t, err)
+
+	s2, err := loader.LoadFromFile("../data/param-rename/op-revision-ref.yaml")
+	require.NoError(t, err)
+
+	d, _, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(),
+		&load.SpecInfo{
+			Spec: s1,
+		},
+		&load.SpecInfo{
+			Spec: s2,
+		})
+	require.NoError(t, err)
+
+	dd := d.PathsDiff.Modified["/books/{bookId}"].OperationsDiff.Modified["GET"].ParametersDiff.Modified["path"]["bookId"].NameDiff
+	require.Equal(t, "bookId", dd.From)
+	require.Equal(t, "id", dd.To)
+}
+
 func TestDiff_TwoPathParamsRenamed(t *testing.T) {
 	loader := openapi3.NewLoader()
 
@@ -821,3 +844,76 @@ func TestDiff_TwoPathParamsOneRenamed(t *testing.T) {
 	require.Equal(t, "libraryId", dd.From)
 	require.Equal(t, "otherId", dd.To)
 }
+
+func TestDiff_DifferentComponentSameParam(t *testing.T) {
+	loader := openapi3.NewLoader()
+
+	s1, err := loader.LoadFromFile("../data/different_component_same_parameter.yaml")
+	require.NoError(t, err)
+
+	d, _, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(),
+		&load.SpecInfo{
+			Spec: s1,
+		},
+		&load.SpecInfo{
+			Spec: s1,
+		})
+	require.NoError(t, err)
+	require.Empty(t, d)
+}
+
+func TestDiff_DifferentComponentModifiedParam(t *testing.T) {
+	loader := openapi3.NewLoader()
+
+	s1, err := loader.LoadFromFile("../data/different_component_same_parameter.yaml")
+	require.NoError(t, err)
+
+	s2, err := loader.LoadFromFile("../data/different_component_modified_parameter.yaml")
+	require.NoError(t, err)
+
+	d, _, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(),
+		&load.SpecInfo{
+			Spec: s1,
+		},
+		&load.SpecInfo{
+			Spec: s2,
+		})
+	require.NoError(t, err)
+	dd := d.ComponentsDiff.ParametersDiff.Modified["differentComponentName_A"].InDiff
+	require.Equal(t, "header", dd.From)
+	require.Equal(t, "query", dd.To)
+}
+
+func TestDiff_DifferentComponentSameSchema(t *testing.T) {
+	loader := openapi3.NewLoader()
+
+	s1, err := loader.LoadFromFile("../data/different_component_same_schema.yaml")
+	require.NoError(t, err)
+
+	d, _, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(),
+		&load.SpecInfo{
+			Spec: s1,
+		},
+		&load.SpecInfo{
+			Spec: s1,
+		})
+	require.NoError(t, err)
+	require.Empty(t, d)
+}
+
+func TestDiff_DifferentComponentSameHeader(t *testing.T) {
+	loader := openapi3.NewLoader()
+
+	s1, err := loader.LoadFromFile("../data/different_component_same_header.yaml")
+	require.NoError(t, err)
+
+	d, _, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(),
+		&load.SpecInfo{
+			Spec: s1,
+		},
+		&load.SpecInfo{
+			Spec: s1,
+		})
+	require.NoError(t, err)
+	require.Empty(t, d)
+}
diff --git a/diff/headers_diff.go b/diff/headers_diff.go
@@ -78,9 +78,9 @@ func getHeadersDiffInternal(config *Config, state *state, headers1, headers2 ope
 		}
 	}
 
-	for headerValue2 := range headers2 {
-		if _, ok := headers1[headerValue2]; !ok {
-			result.Added = append(result.Added, headerValue2)
+	for headerName2 := range headers2 {
+		if _, ok := headers1[headerName2]; !ok {
+			result.Added = append(result.Added, headerName2)
 		}
 	}
 

diff --git a/diff/method_diff.go b/diff/method_diff.go
@@ -11,7 +11,7 @@ type MethodDiff struct {
 	SummaryDiff      *ValueDiff                `json:"summary,omitempty" yaml:"summary,omitempty"`
 	DescriptionDiff  *ValueDiff                `json:"description,omitempty" yaml:"description,omitempty"`
 	OperationIDDiff  *ValueDiff                `json:"operationID,omitempty" yaml:"operationID,omitempty"`
-	ParametersDiff   *ParametersDiff           `json:"parameters,omitempty" yaml:"parameters,omitempty"`
+	ParametersDiff   *ParametersDiffByLocation `json:"parameters,omitempty" yaml:"parameters,omitempty"`
 	RequestBodyDiff  *RequestBodyDiff          `json:"requestBody,omitempty" yaml:"requestBody,omitempty"`
 	ResponsesDiff    *ResponsesDiff            `json:"responses,omitempty" yaml:"responses,omitempty"`
 	CallbacksDiff    *CallbacksDiff            `json:"callbacks,omitempty" yaml:"callbacks,omitempty"`
@@ -61,7 +61,7 @@ func getMethodDiffInternal(config *Config, state *state, operation1, operation2
 	result.SummaryDiff = getValueDiffConditional(config.IsExcludeSummary(), operation1.Summary, operation2.Summary)
 	result.DescriptionDiff = getValueDiffConditional(config.IsExcludeDescription(), operation1.Description, operation2.Description)
 	result.OperationIDDiff = getValueDiff(operation1.OperationID, operation2.OperationID)
-	result.ParametersDiff, err = getParametersDiff(config, state, operation1.Parameters, operation2.Parameters, pathParamsMap)
+	result.ParametersDiff, err = getParametersDiffByLocation(config, state, operation1.Parameters, operation2.Parameters, pathParamsMap)
 	if err != nil {
 		return nil, err
 	}

diff --git a/diff/parameter_diff.go b/diff/parameter_diff.go
@@ -7,6 +7,7 @@ import (
 // ParameterDiff describes the changes between a pair of parameter objects: https://swagger.io/specification/#parameter-object
 type ParameterDiff struct {
 	NameDiff            *ValueDiff          `json:"name,omitempty" yaml:"name,omitempty"`
+	InDiff              *ValueDiff          `json:"in,omitempty" yaml:"in,omitempty"`
 	ExtensionsDiff      *ExtensionsDiff     `json:"extensions,omitempty" yaml:"extensions,omitempty"`
 	DescriptionDiff     *ValueDiff          `json:"description,omitempty" yaml:"description,omitempty"`
 	StyleDiff           *ValueDiff          `json:"style,omitempty" yaml:"style,omitempty"`
@@ -47,6 +48,7 @@ func getParameterDiffInternal(config *Config, state *state, param1, param2 *open
 	var err error
 
 	result.NameDiff = getValueDiff(param1.Name, param2.Name)
+	result.InDiff = getValueDiff(param1.In, param2.In)
 	result.ExtensionsDiff = getExtensionsDiff(config, state, param1.Extensions, param2.Extensions)
 	result.DescriptionDiff = getValueDiffConditional(config.IsExcludeDescription(), param1.Description, param2.Description)
 	result.StyleDiff = getValueDiff(param1.Style, param2.Style)

diff --git a/diff/parameters_aux.go b/diff/parameters_aux.go