Skip to content

Commit

Permalink
implement response discriminator checker (#347)
Browse files Browse the repository at this point in the history
  • Loading branch information
tibulca authored Aug 4, 2023
1 parent 9405cfd commit 4b24179
Show file tree
Hide file tree
Showing 21 changed files with 1,306 additions and 23 deletions.
10 changes: 9 additions & 1 deletion BREAKING-CHANGES-EXAMPLES.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ These examples are automatically generated from unit tests.
[adding an enum value to a response write-only property](checker/check-response-property-enum-value-added_test.go?plain=1#L36)
[adding an enum value to request parameter](checker/check-request-parameter-enum-value-updated_test.go?plain=1#L34)
[adding an optional write-only property to a response](checker/check-response-optional-property-updated_test.go?plain=1#L34)
[adding discriminator to the request body or request body property](checker/check-request-discriminator-updated_test.go?plain=1#L11)
[adding discriminator to the response body or response property](checker/check-response-discriminator-updated_test.go?plain=1#L11)
[adding pattern to request parameters](checker/check-request-parameter-pattern-added-or-changed_test.go?plain=1#L35)
[adding request property enum values](checker/check-request-property-enum-value-updated_test.go?plain=1#L37)
[adding request property pattern](checker/check-request-property-pattern-added-or-changed_test.go?plain=1#L36)
Expand All @@ -178,6 +180,10 @@ These examples are automatically generated from unit tests.
[changing an existing header param from required to optional](checker/checker_request_parameter_required_value_updated_test.go?plain=1#L35)
[changing an existing header param to optional](checker/checker_not_breaking_test.go?plain=1#L133)
[changing an existing request body from required to optional](checker/checker_not_breaking_test.go?plain=1#L53)
[changing discriminator mapping in the request body or request body property](checker/check-request-discriminator-updated_test.go?plain=1#L119)
[changing discriminator mapping in the response body or response property](checker/check-response-discriminator-updated_test.go?plain=1#L119)
[changing discriminator propertyName in the request body or request body property](checker/check-request-discriminator-updated_test.go?plain=1#L83)
[changing discriminator propertyName in the response body or response property](checker/check-response-discriminator-updated_test.go?plain=1#L83)
[changing optional request property to not read-only](checker/check-request-property-write-only-read-only_test.go?plain=1#L86)
[changing optional request property to not write-only](checker/check-request-property-write-only-read-only_test.go?plain=1#L36)
[changing optional request property to read-only](checker/check-request-property-write-only-read-only_test.go?plain=1#L61)
Expand Down Expand Up @@ -220,7 +226,7 @@ These examples are automatically generated from unit tests.
[changing required response property to optional](checker/check-response-property-became-optional_test.go?plain=1#L11)
[changing required response property to read-only](checker/check-response-required-property-write-only-read-only_test.go?plain=1#L63)
[changing required response property to write-only](checker/check-response-required-property-write-only-read-only_test.go?plain=1#L11)
[changing response body default value](checker/check-response-property-default-value-changed_test.go?plain=1#L33)
[changing response body default value](checker/check-response-property-default-value-changed_test.go?plain=1#L42)
[changing response body property default value](checker/check-response-property-default-value-changed_test.go?plain=1#L11)
[changing response property pattern](checker/check-response-pattern-added-or-changed_test.go?plain=1#L11)
[changing security component oauth's url](checker/check-components-security-updated_test.go?plain=1#L11)
Expand Down Expand Up @@ -280,6 +286,8 @@ These examples are automatically generated from unit tests.
[removing an existing operation id](checker/check-api-operation-id-updated_test.go?plain=1#L11)
[removing an existing tag](checker/check-api-tag-updated_test.go?plain=1#L36)
[removing an optional write-only property from a response](checker/check-response-optional-property-updated_test.go?plain=1#L11)
[removing discriminator from the request body or request body property](checker/check-request-discriminator-updated_test.go?plain=1#L47)
[removing discriminator from the response body or response property](checker/check-response-discriminator-updated_test.go?plain=1#L47)
[removing media type from request body](checker/check-request-body-mediatype-updated_test.go?plain=1#L34)
[removing pattern from request parameters](checker/check-request-parameter-pattern-added-or-changed_test.go?plain=1#L58)
[removing request property enum values](checker/check-request-property-enum-value-updated_test.go?plain=1#L11)
Expand Down
148 changes: 148 additions & 0 deletions checker/check-request-discriminator-updated.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
package checker

import (
"fmt"

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

func RequestDiscriminatorUpdatedCheck(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]

appendResultItem := func(messageId string, a ...any) {
result = append(result, ApiChange{
Id: messageId,
Level: INFO,
Text: fmt.Sprintf(config.i18n(messageId), a...),
Operation: operation,
OperationId: operationItem.Revision.OperationID,
Path: path,
Source: source,
})
}

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

processDiscriminatorDiffForRequest(
mediaTypeDiff.SchemaDiff.DiscriminatorDiff,
"",
appendResultItem)

CheckModifiedPropertiesDiff(
mediaTypeDiff.SchemaDiff,
func(propertyPath string, propertyName string, propertyDiff *diff.SchemaDiff, parent *diff.SchemaDiff) {
processDiscriminatorDiffForRequest(
propertyDiff.DiscriminatorDiff,
propertyFullName(propertyPath, propertyName),
appendResultItem)
})

}
}
}
return result
}

func processDiscriminatorDiffForRequest(
discriminatorDiff *diff.DiscriminatorDiff,
propertyName string,
appendResultItem func(messageId string, a ...any)) {

if discriminatorDiff == nil {
return
}

messageIdPrefix := "request-body-discriminator"
if propertyName != "" {
messageIdPrefix = "request-property-discriminator"
}

if discriminatorDiff.Added {
if propertyName == "" {
appendResultItem(messageIdPrefix + "-added")
} else {
appendResultItem(messageIdPrefix+"-added", ColorizedValue(propertyName))
}
return
}
if discriminatorDiff.Deleted {
if propertyName == "" {
appendResultItem(messageIdPrefix + "-removed")
} else {
appendResultItem(messageIdPrefix+"-removed", ColorizedValue(propertyName))
}
return
}

if discriminatorDiff.PropertyNameDiff != nil {
if propertyName == "" {
appendResultItem(messageIdPrefix+"-property-name-changed",
ColorizedValue(discriminatorDiff.PropertyNameDiff.From),
ColorizedValue(discriminatorDiff.PropertyNameDiff.To))
} else {
appendResultItem(messageIdPrefix+"-property-name-changed",
ColorizedValue(propertyName),
ColorizedValue(discriminatorDiff.PropertyNameDiff.From),
ColorizedValue(discriminatorDiff.PropertyNameDiff.To))
}
}

if discriminatorDiff.MappingDiff != nil {
if len(discriminatorDiff.MappingDiff.Added) > 0 {
if propertyName == "" {
appendResultItem(messageIdPrefix+"-mapping-added",
ColorizedValue(discriminatorDiff.MappingDiff.Added))
} else {
appendResultItem(messageIdPrefix+"-mapping-added",
ColorizedValue(discriminatorDiff.MappingDiff.Added),
ColorizedValue(propertyName))
}
}

if len(discriminatorDiff.MappingDiff.Deleted) > 0 {
if propertyName == "" {
appendResultItem(messageIdPrefix+"-mapping-deleted",
ColorizedValue(discriminatorDiff.MappingDiff.Deleted))
} else {
appendResultItem(messageIdPrefix+"-mapping-deleted",
ColorizedValue(discriminatorDiff.MappingDiff.Deleted),
ColorizedValue(propertyName))
}
}

for k, v := range discriminatorDiff.MappingDiff.Modified {
if propertyName == "" {
appendResultItem(messageIdPrefix+"-mapping-changed",
ColorizedValue(k),
ColorizedValue(v.From),
ColorizedValue(v.To))
} else {
appendResultItem(messageIdPrefix+"-mapping-changed",
ColorizedValue(k),
ColorizedValue(v.From),
ColorizedValue(v.To),
ColorizedValue(propertyName))

}
}
}
}
183 changes: 183 additions & 0 deletions checker/check-request-discriminator-updated_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
package checker_test

import (
"testing"

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

// CL: adding discriminator to the request body or request body property
func TestRequestDiscriminatorUpdatedCheckAdded(t *testing.T) {
s1, err := open("../data/checker/request_property_discriminator_added_base.yaml")
require.NoError(t, err)
s2, err := open("../data/checker/request_property_discriminator_added_revision.yaml")
require.NoError(t, err)

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

require.Len(t, errs, 2)

require.ElementsMatch(t, []checker.ApiChange{
{
Id: "request-body-discriminator-added",
Text: "added request discriminator",
Comment: "",
Level: checker.INFO,
Operation: "POST",
Path: "/pets",
Source: "../data/checker/request_property_discriminator_added_revision.yaml",
OperationId: "updatePets",
},
{
Id: "request-property-discriminator-added",
Text: "added discriminator to '/oneOf[#/components/schemas/Dog]/breed' request property",
Comment: "",
Level: checker.INFO,
Operation: "POST",
Path: "/pets",
Source: "../data/checker/request_property_discriminator_added_revision.yaml",
OperationId: "updatePets",
}}, errs)
}

// CL: removing discriminator from the request body or request body property
func TestRequestDiscriminatorUpdatedCheckRemoved(t *testing.T) {
s1, err := open("../data/checker/request_property_discriminator_added_revision.yaml")
require.NoError(t, err)
s2, err := open("../data/checker/request_property_discriminator_added_base.yaml")
require.NoError(t, err)

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

require.Len(t, errs, 2)

require.ElementsMatch(t, []checker.ApiChange{
{
Id: "request-body-discriminator-removed",
Text: "removed request discriminator",
Comment: "",
Level: checker.INFO,
Operation: "POST",
Path: "/pets",
Source: "../data/checker/request_property_discriminator_added_base.yaml",
OperationId: "updatePets",
},
{
Id: "request-property-discriminator-removed",
Text: "removed discriminator from '/oneOf[#/components/schemas/Dog]/breed' request property",
Comment: "",
Level: checker.INFO,
Operation: "POST",
Path: "/pets",
Source: "../data/checker/request_property_discriminator_added_base.yaml",
OperationId: "updatePets",
}}, errs)
}

// CL: changing discriminator propertyName in the request body or request body property
func TestRequestDiscriminatorUpdatedCheckPropertyNameChanging(t *testing.T) {
s1, err := open("../data/checker/request_property_discriminator_added_revision.yaml")
require.NoError(t, err)
s2, err := open("../data/checker/request_property_discriminator_added_property_name_changed.yaml")
require.NoError(t, err)

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

require.Len(t, errs, 2)

require.ElementsMatch(t, []checker.ApiChange{
{
Id: "request-body-discriminator-property-name-changed",
Text: "request discriminator property name changed from 'petType' to 'petType2'",
Comment: "",
Level: checker.INFO,
Operation: "POST",
Path: "/pets",
Source: "../data/checker/request_property_discriminator_added_property_name_changed.yaml",
OperationId: "updatePets",
},
{
Id: "request-property-discriminator-property-name-changed",
Text: "request discriminator property name changed for '/oneOf[#/components/schemas/Dog]/breed' request property from 'name' to 'name2'",
Comment: "",
Level: checker.INFO,
Operation: "POST",
Path: "/pets",
Source: "../data/checker/request_property_discriminator_added_property_name_changed.yaml",
OperationId: "updatePets",
}}, errs)
}

// CL: changing discriminator mapping in the request body or request body property
func TestRequestDiscriminatorUpdatedCheckMappingChanging(t *testing.T) {
s1, err := open("../data/checker/request_property_discriminator_added_revision.yaml")
require.NoError(t, err)
s2, err := open("../data/checker/request_property_discriminator_mapping_changed.yaml")
require.NoError(t, err)

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

require.Len(t, errs, 5)

require.ElementsMatch(t, []checker.ApiChange{
{
Id: "request-body-discriminator-mapping-added",
Text: "added '[cats]' mapping keys to the request discriminator",
Comment: "",
Level: checker.INFO,
Operation: "POST",
Path: "/pets",
Source: "../data/checker/request_property_discriminator_mapping_changed.yaml",
OperationId: "updatePets",
},
{
Id: "request-body-discriminator-mapping-deleted",
Text: "removed '[cat]' mapping keys from the request discriminator",
Comment: "",
Level: checker.INFO,
Operation: "POST",
Path: "/pets",
Source: "../data/checker/request_property_discriminator_mapping_changed.yaml",
OperationId: "updatePets",
},
{
Id: "request-property-discriminator-mapping-added",
Text: "added '[breed1Code]' discriminator mapping keys to the '/oneOf[#/components/schemas/Dog]/breed' request property",
Comment: "",
Level: checker.INFO,
Operation: "POST",
Path: "/pets",
Source: "../data/checker/request_property_discriminator_mapping_changed.yaml",
OperationId: "updatePets",
},
{
Id: "request-property-discriminator-mapping-changed",
Text: "mapped value for discriminator key 'breed2' changed from '#/components/schemas/Breed2' to '#/components/schemas/Breed3' for '/oneOf[#/components/schemas/Dog]/breed' request property",
Comment: "",
Level: checker.INFO,
Operation: "POST",
Path: "/pets",
Source: "../data/checker/request_property_discriminator_mapping_changed.yaml",
OperationId: "updatePets",
},
{
Id: "request-property-discriminator-mapping-deleted",
Text: "removed '[breed1]' discriminator mapping keys from the '/oneOf[#/components/schemas/Dog]/breed' request property",
Comment: "",
Level: checker.INFO,
Operation: "POST",
Path: "/pets",
Source: "../data/checker/request_property_discriminator_mapping_changed.yaml",
OperationId: "updatePets",
}}, errs)
}
Loading

0 comments on commit 4b24179

Please sign in to comment.