Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement request/response allOf checkers #340

Merged
merged 9 commits into from
Jul 27, 2023
6 changes: 6 additions & 0 deletions BREAKING-CHANGES-EXAMPLES.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Examples of Breaking and Non-Breaking Changes
These examples are automatically generated from unit tests.
## Examples of breaking changes
[adding 'allOf' schema to the request body or request body property is breaking](checker/checker_breaking_test.go?plain=1#L676)
[adding a new required property in request body is breaking](checker/checker_breaking_property_test.go?plain=1#L352)
[adding a pattern to a schema is breaking for recursive properties](checker/checker_breaking_test.go?plain=1#L475)
[adding a pattern to a schema is breaking](checker/checker_breaking_test.go?plain=1#L459)
Expand Down Expand Up @@ -62,6 +63,7 @@ These examples are automatically generated from unit tests.
[reducing max length in request is breaking](checker/checker_breaking_min_max_test.go?plain=1#L12)
[reducing min items in response is breaking](checker/checker_breaking_min_max_test.go?plain=1#L220)
[reducing min length in response is breaking](checker/checker_breaking_min_max_test.go?plain=1#L62)
[removing 'allOf' schema from the request body or request body property is breaking with warn](checker/checker_breaking_test.go?plain=1#L698)
[removing 'anyOf' schema from the request body or request body property is breaking](checker/checker_breaking_test.go?plain=1#L633)
[removing 'oneOf' schema from the request body or request body property is breaking](checker/checker_breaking_test.go?plain=1#L655)
[removing a media type from request body is breaking](checker/checker_breaking_test.go?plain=1#L617)
Expand Down Expand Up @@ -140,6 +142,8 @@ These examples are automatically generated from unit tests.
[renaming a path parameter is not breaking](checker/checker_breaking_test.go?plain=1#L135)

## Examples of info-level changes for changelog
[adding 'allOf' schema to the request body or request body property](checker/check-request-property-all-of-updated_test.go?plain=1#L11)
[adding 'allOf' schema to the response body or response body property](checker/check-response-property-all-of-updated_test.go?plain=1#L11)
[adding 'anyOf' schema to the request body or request body property](checker/check-request-property-any-of-updated_test.go?plain=1#L11)
[adding 'anyOf' schema to the response body or response body property](checker/check-response-property-any-of-updated_test.go?plain=1#L11)
[adding 'oneOf' schema to the request body or request body property](checker/check-request-property-one-of-updated_test.go?plain=1#L11)
Expand Down Expand Up @@ -202,6 +206,8 @@ These examples are automatically generated from unit tests.
[new paths or path operations](checker/check-api-added_test.go?plain=1#L11)
[path operations that became deprecated](checker/checker_deprecation_test.go?plain=1#L324)
[path operations that were re-activated](checker/checker_deprecation_test.go?plain=1#L346)
[removing 'allOf' schema from the request body or request body property](checker/check-request-property-all-of-updated_test.go?plain=1#L47)
[removing 'allOf' schema from the response body or response body property](checker/check-response-property-all-of-updated_test.go?plain=1#L47)
[removing 'anyOf' schema from the request body or request body property](checker/check-request-property-any-of-updated_test.go?plain=1#L47)
[removing 'anyOf' schema from the response body or response body property](checker/check-response-property-any-of-updated_test.go?plain=1#L47)
[removing 'oneOf' schema from the request body or request body property](checker/check-request-property-one-of-updated_test.go?plain=1#L47)
Expand Down
103 changes: 103 additions & 0 deletions checker/check-request-property-all-of-updated.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package checker

import (
"fmt"

"github.com/tufin/oasdiff/diff"
)

func RequestPropertyAllOfUpdated(diffReport *diff.Diff, operationsSources *diff.OperationsSourcesMap, config Config) Changes {
result := make(Changes, 0)
if diffReport.PathsDiff == nil {
return result
}

for path, pathItem := range diffReport.PathsDiff.Modified {
if pathItem.OperationsDiff == nil {
continue
}

for operation, operationItem := range pathItem.OperationsDiff.Modified {
if operationItem.RequestBodyDiff == nil ||
operationItem.RequestBodyDiff.ContentDiff == nil ||
operationItem.RequestBodyDiff.ContentDiff.MediaTypeModified == nil {
continue
}
source := (*operationsSources)[operationItem.Revision]

modifiedMediaTypes := operationItem.RequestBodyDiff.ContentDiff.MediaTypeModified
for _, mediaTypeDiff := range modifiedMediaTypes {
if mediaTypeDiff.SchemaDiff == nil {
continue
}

if mediaTypeDiff.SchemaDiff.AllOfDiff != nil && len(mediaTypeDiff.SchemaDiff.AllOfDiff.Added) > 0 {
result = append(result, ApiChange{
Id: "request-body-all-of-added",
Level: ERR,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

breaking change

Text: fmt.Sprintf(
config.i18n("request-body-all-of-added"),
ColorizedValue(mediaTypeDiff.SchemaDiff.AllOfDiff.Added.String())),
Operation: operation,
OperationId: operationItem.Revision.OperationID,
Path: path,
Source: source,
})
}

if mediaTypeDiff.SchemaDiff.AllOfDiff != nil && len(mediaTypeDiff.SchemaDiff.AllOfDiff.Deleted) > 0 {
result = append(result, ApiChange{
Id: "request-body-all-of-removed",
Level: WARN,
Text: fmt.Sprintf(
config.i18n("request-body-all-of-removed"),
ColorizedValue(mediaTypeDiff.SchemaDiff.AllOfDiff.Deleted.String())),
Operation: operation,
OperationId: operationItem.Revision.OperationID,
Path: path,
Source: source,
})
}

CheckModifiedPropertiesDiff(
mediaTypeDiff.SchemaDiff,
func(propertyPath string, propertyName string, propertyDiff *diff.SchemaDiff, parent *diff.SchemaDiff) {
if propertyDiff.AllOfDiff == nil {
return
}

if len(propertyDiff.AllOfDiff.Added) > 0 {
result = append(result, ApiChange{
Id: "request-property-all-of-added",
Level: ERR,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

breaking change

Text: fmt.Sprintf(
config.i18n("request-property-all-of-added"),
ColorizedValue(propertyDiff.AllOfDiff.Added.String()),
ColorizedValue(propertyFullName(propertyPath, propertyName))),
Operation: operation,
OperationId: operationItem.Revision.OperationID,
Path: path,
Source: source,
})
}

if len(propertyDiff.AllOfDiff.Deleted) > 0 {
result = append(result, ApiChange{
Id: "request-property-all-of-removed",
Level: WARN,
Text: fmt.Sprintf(
config.i18n("request-property-all-of-removed"),
ColorizedValue(propertyDiff.AllOfDiff.Deleted.String()),
ColorizedValue(propertyFullName(propertyPath, propertyName))),
Operation: operation,
OperationId: operationItem.Revision.OperationID,
Path: path,
Source: source,
})
}
})
}
}
}
return result
}
81 changes: 81 additions & 0 deletions checker/check-request-property-all-of-updated_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package checker_test

import (
"testing"

"github.com/stretchr/testify/require"
"github.com/tufin/oasdiff/checker"
"github.com/tufin/oasdiff/diff"
)

// CL: adding 'allOf' schema to the request body or request body property
func TestRequestPropertyAllOfAdded(t *testing.T) {
s1, err := open("../data/checker/request_property_all_of_added_base.yaml")
require.NoError(t, err)
s2, err := open("../data/checker/request_property_all_of_added_revision.yaml")
require.NoError(t, err)

d, osm, err := diff.GetWithOperationsSourcesMap(getConfig(), s1, s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibilityUntilLevel(singleCheckConfig(checker.RequestPropertyAllOfUpdated), d, osm, checker.INFO)

require.Len(t, errs, 2)

require.ElementsMatch(t, []checker.ApiChange{
{
Id: "request-body-all-of-added",
Text: "added 'Rabbit' to the request body 'allOf' list",
Comment: "",
Level: checker.ERR,
Operation: "POST",
Path: "/pets",
Source: "../data/checker/request_property_all_of_added_revision.yaml",
OperationId: "updatePets",
},
{
Id: "request-property-all-of-added",
Text: "added 'Breed3' to the '/allOf[#/components/schemas/Dog]/breed' request property 'allOf' list",
Comment: "",
Level: checker.ERR,
Operation: "POST",
Path: "/pets",
Source: "../data/checker/request_property_all_of_added_revision.yaml",
OperationId: "updatePets",
}}, errs)
}

// CL: removing 'allOf' schema from the request body or request body property
func TestRequestPropertyAllOfRemoved(t *testing.T) {
s1, err := open("../data/checker/request_property_all_of_removed_base.yaml")
require.NoError(t, err)
s2, err := open("../data/checker/request_property_all_of_removed_revision.yaml")
require.NoError(t, err)

d, osm, err := diff.GetWithOperationsSourcesMap(getConfig(), s1, s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibilityUntilLevel(singleCheckConfig(checker.RequestPropertyAllOfUpdated), d, osm, checker.INFO)

require.Len(t, errs, 2)

require.ElementsMatch(t, []checker.ApiChange{
{
Id: "request-body-all-of-removed",
Text: "removed 'Rabbit' from the request body 'allOf' list",
Comment: "",
Level: checker.WARN,
Operation: "POST",
Path: "/pets",
Source: "../data/checker/request_property_all_of_removed_revision.yaml",
OperationId: "updatePets",
},
{
Id: "request-property-all-of-removed",
Text: "removed 'Breed3' from the '/allOf[#/components/schemas/Dog]/breed' request property 'allOf' list",
Comment: "",
Level: checker.WARN,
Operation: "POST",
Path: "/pets",
Source: "../data/checker/request_property_all_of_removed_revision.yaml",
OperationId: "updatePets",
}}, errs)
}
111 changes: 111 additions & 0 deletions checker/check-response-property-all-of-updated.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package checker

import (
"fmt"

"github.com/tufin/oasdiff/diff"
)

func ResponsePropertyAllOfUpdated(diffReport *diff.Diff, operationsSources *diff.OperationsSourcesMap, config Config) Changes {
result := make(Changes, 0)
if diffReport.PathsDiff == nil {
return result
}

for path, pathItem := range diffReport.PathsDiff.Modified {
if pathItem.OperationsDiff == nil {
continue
}

for operation, operationItem := range pathItem.OperationsDiff.Modified {
if operationItem.ResponsesDiff == nil || operationItem.ResponsesDiff.Modified == nil {
continue
}
source := (*operationsSources)[operationItem.Revision]

for responseStatus, responsesDiff := range operationItem.ResponsesDiff.Modified {
if responsesDiff.ContentDiff == nil || responsesDiff.ContentDiff.MediaTypeModified == nil {
continue
}

modifiedMediaTypes := responsesDiff.ContentDiff.MediaTypeModified
for _, mediaTypeDiff := range modifiedMediaTypes {
if mediaTypeDiff.SchemaDiff == nil {
continue
}

if mediaTypeDiff.SchemaDiff.AllOfDiff != nil && len(mediaTypeDiff.SchemaDiff.AllOfDiff.Added) > 0 {
result = append(result, ApiChange{
Id: "response-body-all-of-added",
Level: INFO,
Text: fmt.Sprintf(
config.i18n("response-body-all-of-added"),
ColorizedValue(mediaTypeDiff.SchemaDiff.AllOfDiff.Added.String()),
responseStatus),
Operation: operation,
OperationId: operationItem.Revision.OperationID,
Path: path,
Source: source,
})
}

if mediaTypeDiff.SchemaDiff.AllOfDiff != nil && len(mediaTypeDiff.SchemaDiff.AllOfDiff.Deleted) > 0 {
result = append(result, ApiChange{
Id: "response-body-all-of-removed",
Level: INFO,
Text: fmt.Sprintf(
config.i18n("response-body-all-of-removed"),
ColorizedValue(mediaTypeDiff.SchemaDiff.AllOfDiff.Deleted.String()),
responseStatus),
Operation: operation,
OperationId: operationItem.Revision.OperationID,
Path: path,
Source: source,
})
}

CheckModifiedPropertiesDiff(
mediaTypeDiff.SchemaDiff,
func(propertyPath string, propertyName string, propertyDiff *diff.SchemaDiff, parent *diff.SchemaDiff) {
if propertyDiff.AllOfDiff == nil {
return
}

if len(propertyDiff.AllOfDiff.Added) > 0 {
result = append(result, ApiChange{
Id: "response-property-all-of-added",
Level: INFO,
Text: fmt.Sprintf(
config.i18n("response-property-all-of-added"),
ColorizedValue(propertyDiff.AllOfDiff.Added.String()),
ColorizedValue(propertyFullName(propertyPath, propertyName)),
responseStatus),
Operation: operation,
OperationId: operationItem.Revision.OperationID,
Path: path,
Source: source,
})
}

if len(propertyDiff.AllOfDiff.Deleted) > 0 {
result = append(result, ApiChange{
Id: "response-property-all-of-removed",
Level: INFO,
Text: fmt.Sprintf(
config.i18n("response-property-all-of-removed"),
ColorizedValue(propertyDiff.AllOfDiff.Deleted.String()),
ColorizedValue(propertyFullName(propertyPath, propertyName)),
responseStatus),
Operation: operation,
OperationId: operationItem.Revision.OperationID,
Path: path,
Source: source,
})
}
})
}
}
}
}
return result
}
Loading