Skip to content

Commit

Permalink
add message generator
Browse files Browse the repository at this point in the history
  • Loading branch information
@reuvenharrison
reuvenharrison committed Aug 30, 2024
1 parent 05bb6a2 commit a25eacd
Show file tree
Hide file tree
Showing 15 changed files with 429 additions and 108 deletions.
2 changes: 1 addition & 1 deletion checker/check_breaking_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ func TestBreaking_OperationIdRemoved(t *testing.T) {
require.Len(t, errs, 1)
require.Equal(t, checker.APIOperationIdRemovedId, errs[0].GetId())
require.Equal(t, "api operation id 'GetSecurityScores' removed and replaced with 'newOperationId'", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer()))
verifyNonBreakingChangeIsChangelogEntry(t, d, osm, checker.APIOperationIdRemovedId)
verifyNonBreakingChangeIsChangelogEntry(t, d, osm, checker.APIOperationIdRemovedId, "api operation id 'GetSecurityScores' removed and replaced with 'newOperationId'")
}

// BC: removing/updating an enum in request body is breaking (optional): request-body-enum-value-removed
Expand Down
32 changes: 14 additions & 18 deletions checker/check_not_breaking_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"github.com/tufin/oasdiff/diff"
)

func verifyNonBreakingChangeIsChangelogEntry(t *testing.T, d *diff.Diff, osm *diff.OperationsSourcesMap, changeId string) {
func verifyNonBreakingChangeIsChangelogEntry(t *testing.T, d *diff.Diff, osm *diff.OperationsSourcesMap, changeId string, text string) {
t.Helper()

// Check no breaking change is detected
Expand All @@ -22,6 +22,7 @@ func verifyNonBreakingChangeIsChangelogEntry(t *testing.T, d *diff.Diff, osm *di
require.Len(t, errs, 1)
require.Equal(t, checker.INFO, errs[0].GetLevel())
require.Equal(t, changeId, errs[0].GetId())
require.Equal(t, text, errs[0].GetUncolorizedText(checker.NewDefaultLocalizer()))
}

// BC: no change is not breaking
Expand Down Expand Up @@ -50,7 +51,7 @@ func TestBreaking_AddingOptionalRequestBody(t *testing.T) {
require.Empty(t, errs)
}

// CL: changing an existing request body from required to optional
// CL: changing an existing request body from required to optional: request-body-became-optional
func TestBreaking_RequestBodyRequiredDisabled(t *testing.T) {
s1 := l(t, 1)
s2 := l(t, 1)
Expand All @@ -65,7 +66,7 @@ func TestBreaking_RequestBodyRequiredDisabled(t *testing.T) {

d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
require.NoError(t, err)
verifyNonBreakingChangeIsChangelogEntry(t, d, osm, checker.RequestBodyBecameOptionalId)
verifyNonBreakingChangeIsChangelogEntry(t, d, osm, checker.RequestBodyBecameOptionalId, "request body became optional")
}

// BC: deleting a tag is not breaking
Expand Down Expand Up @@ -129,7 +130,7 @@ func TestBreaking_NewOptionalHeaderParam(t *testing.T) {
require.Empty(t, errs)
}

// CL: changing an existing header param to optional
// CL: changing an existing header param to optional: request-parameter-became-optional
func TestBreaking_HeaderParamRequiredDisabled(t *testing.T) {
s1 := l(t, 1)
s2 := l(t, 1)
Expand All @@ -139,10 +140,7 @@ func TestBreaking_HeaderParamRequiredDisabled(t *testing.T) {

d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
require.NoError(t, err)
changes := checker.CheckBackwardCompatibilityUntilLevel(allChecksConfig(), d, osm, checker.INFO)
require.NotEmpty(t, changes)
require.Equal(t, checker.RequestParameterBecomeOptionalId, changes[0].GetId())
require.Len(t, changes, 1)
verifyNonBreakingChangeIsChangelogEntry(t, d, osm, checker.RequestParameterBecomeOptionalId, "'header' request parameter 'network-policies' became optional")
}

func deleteResponseHeader(response *openapi3.Response, name string) {
Expand Down Expand Up @@ -195,7 +193,7 @@ func TestBreaking_ResponseAddMediaType(t *testing.T) {
require.Empty(t, errs)
}

// CL: deprecating an operation with sunset greater than min
// CL: deprecating an operation with sunset greater than min: endpoint-deprecated
func TestBreaking_DeprecatedOperation(t *testing.T) {
s1 := l(t, 1)
s2 := l(t, 1)
Expand All @@ -205,9 +203,7 @@ func TestBreaking_DeprecatedOperation(t *testing.T) {

d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibilityUntilLevel(allChecksConfig(), d, osm, checker.INFO)
require.Len(t, errs, 1)
require.Equal(t, errs[0].GetLevel(), checker.INFO)
verifyNonBreakingChangeIsChangelogEntry(t, d, osm, checker.EndpointDeprecatedId, "endpoint deprecated")
}

// BC: deprecating a parameter is not breaking
Expand Down Expand Up @@ -263,18 +259,18 @@ func TestBreaking_Servers(t *testing.T) {
require.Empty(t, errs)
}

// BC: adding a tag is not breaking
// BC: adding a tag is not breaking: api-tag-added
func TestBreaking_TagAdded(t *testing.T) {
s1 := l(t, 1)
s2 := l(t, 1)

s2.Spec.Paths.Value(securityScorePath).Get.Tags = append(s2.Spec.Paths.Value(securityScorePath).Get.Tags, "newTag")
d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
require.NoError(t, err)
verifyNonBreakingChangeIsChangelogEntry(t, d, osm, checker.APITagAddedId)
verifyNonBreakingChangeIsChangelogEntry(t, d, osm, checker.APITagAddedId, "added api tag 'newTag'")
}

// BC: adding an operation ID is not breaking
// BC: adding an operation ID is not breaking: api-operation-id-added
func TestBreaking_OperationIdAdded(t *testing.T) {
s1 := l(t, 1)
s2 := l(t, 1)
Expand All @@ -283,10 +279,10 @@ func TestBreaking_OperationIdAdded(t *testing.T) {

d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
require.NoError(t, err)
verifyNonBreakingChangeIsChangelogEntry(t, d, osm, checker.APIOperationIdAddId)
verifyNonBreakingChangeIsChangelogEntry(t, d, osm, checker.APIOperationIdAddId, "api operation id 'GetSecurityScores' was added")
}

// BC: adding a required property to response is not breaking
// BC: adding a required property to response is not breaking: response-required-property-added
func TestBreaking_RequiredResponsePropertyAdded(t *testing.T) {
s1, err := open("../data/checker/response_required_property_added_base.yaml")
require.NoError(t, err)
Expand All @@ -296,5 +292,5 @@ func TestBreaking_RequiredResponsePropertyAdded(t *testing.T) {

d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
require.NoError(t, err)
verifyNonBreakingChangeIsChangelogEntry(t, d, osm, "response-required-property-added")
verifyNonBreakingChangeIsChangelogEntry(t, d, osm, checker.ResponseRequiredPropertyAddedId, "added required property 'data/new' to response status '200'")
}
6 changes: 4 additions & 2 deletions checker/check_request_body_mediatype_updated_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/tufin/oasdiff/load"
)

// CL: adding a new media type to request body
// CL: adding a new media type to request body: request-body-media-type-added
func TestRequestBodyMediaTypeAdded(t *testing.T) {
s1, err := open("../data/checker/request_body_media_type_updated_base.yaml")
require.NoError(t, err)
Expand All @@ -29,9 +29,10 @@ func TestRequestBodyMediaTypeAdded(t *testing.T) {
Source: load.NewSource("../data/checker/request_body_media_type_updated_revision.yaml"),
OperationId: "createOneGroup",
}, errs[0])
require.Equal(t, "added media type 'application/json' to the request body", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer()))
}

// CL: removing media type from request body
// CL: removing media type from request body: request-body-media-type-removed
func TestRequestBodyMediaTypeRemoved(t *testing.T) {
s1, err := open("../data/checker/request_body_media_type_updated_revision.yaml")
require.NoError(t, err)
Expand All @@ -51,4 +52,5 @@ func TestRequestBodyMediaTypeRemoved(t *testing.T) {
Source: load.NewSource("../data/checker/request_body_media_type_updated_base.yaml"),
OperationId: "createOneGroup",
}, errs[0])
require.Equal(t, "removed media type 'application/json' from the request body", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer()))
}
6 changes: 4 additions & 2 deletions checker/check_request_body_required_value_updated_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/tufin/oasdiff/load"
)

// CL: changing request's body to required is breaking
// CL: changing request's body to required is breaking: request-body-became-required
func TestRequestBodyBecameRequired(t *testing.T) {
s1, err := open("../data/checker/request_body_became_required_base.yaml")
require.NoError(t, err)
Expand All @@ -30,9 +30,10 @@ func TestRequestBodyBecameRequired(t *testing.T) {
Source: load.NewSource("../data/checker/request_body_became_required_base.yaml"),
OperationId: "createOneGroup",
}, errs[0])
require.Equal(t, "request body became required", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer()))
}

// CL: changing request's body to optional
// CL: changing request's body to optional: request-body-became-optional
func TestRequestBodyBecameOptional(t *testing.T) {
s1, err := open("../data/checker/request_body_became_optional_base.yaml")
require.NoError(t, err)
Expand All @@ -53,4 +54,5 @@ func TestRequestBodyBecameOptional(t *testing.T) {
Source: load.NewSource("../data/checker/request_body_became_optional_base.yaml"),
OperationId: "createOneGroup",
}, errs[0])
require.Equal(t, "request body became optional", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer()))
}
37 changes: 24 additions & 13 deletions checker/check_request_discriminator_updated.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,15 @@ func RequestDiscriminatorUpdatedCheck(diffReport *diff.Diff, operationsSources *
))
}

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

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

CheckModifiedPropertiesDiff(
Expand All @@ -66,6 +67,7 @@ func RequestDiscriminatorUpdatedCheck(diffReport *diff.Diff, operationsSources *
processDiscriminatorDiffForRequest(
propertyDiff.DiscriminatorDiff,
propertyFullName(propertyPath, propertyName),
mediaType,
appendResultItem)
})

Expand All @@ -78,6 +80,7 @@ func RequestDiscriminatorUpdatedCheck(diffReport *diff.Diff, operationsSources *
func processDiscriminatorDiffForRequest(
discriminatorDiff *diff.DiscriminatorDiff,
propertyName string,
mediaType string,
appendResultItem func(messageId string, a ...any)) {

if discriminatorDiff == nil {
Expand All @@ -91,17 +94,17 @@ func processDiscriminatorDiffForRequest(

if discriminatorDiff.Added {
if propertyName == "" {
appendResultItem(messageIdPrefix + "-added")
appendResultItem(messageIdPrefix+"-added", mediaType)
} else {
appendResultItem(messageIdPrefix+"-added", propertyName)
appendResultItem(messageIdPrefix+"-added", propertyName, mediaType)
}
return
}
if discriminatorDiff.Deleted {
if propertyName == "" {
appendResultItem(messageIdPrefix + "-removed")
appendResultItem(messageIdPrefix+"-removed", mediaType)
} else {
appendResultItem(messageIdPrefix+"-removed", propertyName)
appendResultItem(messageIdPrefix+"-removed", propertyName, mediaType)
}
return
}
Expand All @@ -110,35 +113,41 @@ func processDiscriminatorDiffForRequest(
if propertyName == "" {
appendResultItem(messageIdPrefix+"-property-name-changed",
discriminatorDiff.PropertyNameDiff.From,
discriminatorDiff.PropertyNameDiff.To)
discriminatorDiff.PropertyNameDiff.To,
mediaType)
} else {
appendResultItem(messageIdPrefix+"-property-name-changed",
propertyName,
discriminatorDiff.PropertyNameDiff.From,
discriminatorDiff.PropertyNameDiff.To)
discriminatorDiff.PropertyNameDiff.To,
mediaType)
}
}

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

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

Expand All @@ -147,13 +156,15 @@ func processDiscriminatorDiffForRequest(
appendResultItem(messageIdPrefix+"-mapping-changed",
k,
v.From,
v.To)
v.To,
mediaType)
} else {
appendResultItem(messageIdPrefix+"-mapping-changed",
k,
v.From,
v.To,
propertyName)
propertyName,
mediaType)

}
}
Expand Down
Loading

0 comments on commit a25eacd

Please sign in to comment.