Skip to content

Commit

Permalink
Merge branch 'main' into panic_in_RequestPropertyTypeChangedCheck
Browse files Browse the repository at this point in the history
  • Loading branch information
Reuven Harrison authored Aug 8, 2023
2 parents 52ff94c + 30738ef commit b238ffa
Show file tree
Hide file tree
Showing 17 changed files with 361 additions and 74 deletions.
21 changes: 14 additions & 7 deletions BREAKING-CHANGES-EXAMPLES.md
Original file line number Diff line number Diff line change
@@ -1,7 +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 'allOf' schema to the request body or request body property is breaking](checker/checker_breaking_test.go?plain=1#L694)
[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 @@ -63,16 +63,17 @@ 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)
[removing 'allOf' schema from the request body or request body property is breaking with warn](checker/checker_breaking_test.go?plain=1#L716)
[removing 'anyOf' schema from the request body or request body property is breaking](checker/checker_breaking_test.go?plain=1#L651)
[removing 'oneOf' schema from the request body or request body property is breaking](checker/checker_breaking_test.go?plain=1#L673)
[removing a media type from request body is breaking](checker/checker_breaking_test.go?plain=1#L635)
[removing a success status is breaking](checker/check-response-status-updated_test.go?plain=1#L89)
[removing an existing optional response header is breaking as warn](checker/checker_breaking_test.go?plain=1#L410)
[removing an existing required response header is breaking as error](checker/checker_breaking_test.go?plain=1#L227)
[removing an existing response with non-successful status is breaking (optional)](checker/checker_breaking_test.go?plain=1#L264)
[removing an existing response with successful status is breaking](checker/checker_breaking_test.go?plain=1#L246)
[removing an schema object from components is breaking (optional)](checker/checker_breaking_test.go?plain=1#L592)
[removing an schema object from components is breaking (optional)](checker/checker_breaking_test.go?plain=1#L610)
[removing the default value of an optional request parameter is breaking](checker/checker_breaking_test.go?plain=1#L573)
[removing the path without a deprecation policy and without specifying sunset date is breaking if some APIs are not alpha stability level](checker/checker_deprecation_test.go?plain=1#L137)
[removing the path without a deprecation policy and without specifying sunset date is breaking if some APIs are not draft stability level](checker/checker_deprecation_test.go?plain=1#L191)
[removing/updating a property enum in response is breaking (optional)](checker/checker_breaking_test.go?plain=1#L324)
Expand Down Expand Up @@ -126,7 +127,7 @@ These examples are automatically generated from unit tests.
[increasing max length in request is not breaking](checker/checker_breaking_min_max_test.go?plain=1#L76)
[increasing min items in response is not breaking](checker/checker_breaking_min_max_test.go?plain=1#L250)
[modifying a pattern to ".*" in a schema is not breaking](checker/checker_breaking_test.go?plain=1#L523)
[modifying the default value of a required request parameter is not breaking](checker/checker_breaking_test.go?plain=1#L573)
[modifying the default value of a required request parameter is not breaking](checker/checker_breaking_test.go?plain=1#L591)
[new optional header param is not breaking](checker/checker_not_breaking_test.go?plain=1#L119)
[new optional property in request header is not breaking](checker/checker_breaking_property_test.go?plain=1#L38)
[new required response header param is not breaking](checker/checker_not_breaking_test.go?plain=1#L153)
Expand Down Expand Up @@ -171,8 +172,11 @@ These examples are automatically generated from unit tests.
[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 body default value or request property default value](checker/check-request-property-default-value-changed_test.go?plain=1#L58)
[adding request parameter default value](checker/check-request-parameters-default-value-changed_test.go?plain=1#L34)
[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)
[adding response body default value or response body property default value](checker/check-response-property-default-value-changed_test.go?plain=1#L66)
[adding response property pattern](checker/check-response-pattern-added-or-changed_test.go?plain=1#L37)
[adding two new request properties, one required, one optional](checker/check-request-property-updated_test.go?plain=1#L33)
[changing a response property schema format](checker/check-response-property-type-changed_test.go?plain=1#L59)
Expand Down Expand Up @@ -292,8 +296,11 @@ These examples are automatically generated from unit tests.
[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 body default value or request property default value](checker/check-request-property-default-value-changed_test.go?plain=1#L92)
[removing request parameter default value](checker/check-request-parameters-default-value-changed_test.go?plain=1#L59)
[removing request property enum values](checker/check-request-property-enum-value-updated_test.go?plain=1#L11)
[removing request property pattern](checker/check-request-property-pattern-added-or-changed_test.go?plain=1#L59)
[removing response body default value or response body property default value](checker/check-response-property-default-value-changed_test.go?plain=1#L101)
[removing response property pattern](checker/check-response-pattern-added-or-changed_test.go?plain=1#L63)
[updating an existing operation id](checker/check-api-operation-id-updated_test.go?plain=1#L36)
[updating an existing tag](checker/check-api-tag-updated_test.go?plain=1#L62)
2 changes: 1 addition & 1 deletion checker/check-request-parameter-enum-value-updated.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func RequestParameterEnumValueUpdatedCheck(diffReport *diff.Diff, operationsSour
result = append(result, ApiChange{
Id: "request-parameter-enum-value-removed",
Level: ERR,
Text: fmt.Sprintf(config.i18n("request-parameter-enum-value-removed"), enumVal, ColorizedValue(paramLocation), ColorizedValue(paramName)),
Text: fmt.Sprintf(config.i18n("request-parameter-enum-value-removed"), ColorizedValue(enumVal), ColorizedValue(paramLocation), ColorizedValue(paramName)),
Operation: operation,
OperationId: operationItem.Revision.OperationID,
Path: path,
Expand Down
2 changes: 1 addition & 1 deletion checker/check-request-parameter-enum-value-updated_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestRequestParameterEnumValueRemovedCheck(t *testing.T) {
require.Len(t, errs, 1)
require.Equal(t, checker.ApiChange{
Id: "request-parameter-enum-value-removed",
Text: "removed the enum value available from the 'query' request parameter 'status'",
Text: "removed the enum value 'available' from the 'query' request parameter 'status'",
Level: checker.ERR,
Operation: "GET",
Path: "/test",
Expand Down
30 changes: 19 additions & 11 deletions checker/check-request-parameters-default-value-changed.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,18 @@ func RequestParameterDefaultValueChanged(diffReport *diff.Diff, operationsSource
if operationItem.ParametersDiff == nil {
continue
}
source := (*operationsSources)[operationItem.Revision]
appendResultItem := func(messageId string, a ...any) {
result = append(result, ApiChange{
Id: messageId,
Level: ERR,
Text: fmt.Sprintf(config.i18n(messageId), a...),
Operation: operation,
OperationId: operationItem.Revision.OperationID,
Path: path,
Source: source,
})
}
for paramLocation, paramDiffs := range operationItem.ParametersDiff.Modified {
for paramName, paramDiff := range paramDiffs {

Expand All @@ -41,17 +53,13 @@ func RequestParameterDefaultValueChanged(diffReport *diff.Diff, operationsSource
continue
}

source := (*operationsSources)[operationItem.Revision]

result = append(result, ApiChange{
Id: "request-parameter-default-value-changed",
Level: ERR,
Text: fmt.Sprintf(config.i18n("request-parameter-default-value-changed"), ColorizedValue(paramLocation), ColorizedValue(paramName), ColorizedValue(defaultValueDiff.From), ColorizedValue(defaultValueDiff.To)),
Operation: operation,
OperationId: operationItem.Revision.OperationID,
Path: path,
Source: source,
})
if defaultValueDiff.From == nil {
appendResultItem("request-parameter-default-value-added", ColorizedValue(paramLocation), ColorizedValue(paramName), ColorizedValue(defaultValueDiff.To))
} else if defaultValueDiff.To == nil {
appendResultItem("request-parameter-default-value-removed", ColorizedValue(paramLocation), ColorizedValue(paramName), ColorizedValue(defaultValueDiff.From))
} else {
appendResultItem("request-parameter-default-value-changed", ColorizedValue(paramLocation), ColorizedValue(paramName), ColorizedValue(defaultValueDiff.From), ColorizedValue(defaultValueDiff.To))
}
}
}
}
Expand Down
50 changes: 50 additions & 0 deletions checker/check-request-parameters-default-value-changed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,53 @@ func TestRequestParameterDefaultValueChanged(t *testing.T) {
OperationId: "createOneGroup",
}, errs[0])
}

// CL: adding request parameter default value
func TestRequestParameterDefaultValueAdded(t *testing.T) {
s1, err := open("../data/checker/request_parameter_default_value_changed_base.yaml")
require.NoError(t, err)
s2, err := open("../data/checker/request_parameter_default_value_changed_base.yaml")
require.NoError(t, err)

s1.Spec.Paths["/api/v1.0/groups"].Post.Parameters[1].Value.Schema.Value.Default = nil

d, osm, err := diff.GetWithOperationsSourcesMap(getConfig(), s1, s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibilityUntilLevel(singleCheckConfig(checker.RequestParameterDefaultValueChanged), d, osm, checker.ERR)
require.Len(t, errs, 1)
require.Equal(t, checker.ApiChange{
Id: "request-parameter-default-value-added",
Text: "for the 'query' request parameter 'category', default value 'default_category' was added",
Comment: "",
Level: checker.ERR,
Operation: "POST",
Path: "/api/v1.0/groups",
Source: "../data/checker/request_parameter_default_value_changed_base.yaml",
OperationId: "createOneGroup",
}, errs[0])
}

// CL: removing request parameter default value
func TestRequestParameterDefaultValueRemoved(t *testing.T) {
s1, err := open("../data/checker/request_parameter_default_value_changed_base.yaml")
require.NoError(t, err)
s2, err := open("../data/checker/request_parameter_default_value_changed_base.yaml")
require.NoError(t, err)

s2.Spec.Paths["/api/v1.0/groups"].Post.Parameters[1].Value.Schema.Value.Default = nil

d, osm, err := diff.GetWithOperationsSourcesMap(getConfig(), s1, s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibilityUntilLevel(singleCheckConfig(checker.RequestParameterDefaultValueChanged), d, osm, checker.ERR)
require.Len(t, errs, 1)
require.Equal(t, checker.ApiChange{
Id: "request-parameter-default-value-removed",
Text: "for the 'query' request parameter 'category', default value 'default_category' was removed",
Comment: "",
Level: checker.ERR,
Operation: "POST",
Path: "/api/v1.0/groups",
Source: "../data/checker/request_parameter_default_value_changed_base.yaml",
OperationId: "createOneGroup",
}, errs[0])
}
45 changes: 27 additions & 18 deletions checker/check-request-property-default-value-changed.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,30 @@ func RequestPropertyDefaultValueChangedCheck(diffReport *diff.Diff, operationsSo
}
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,
})
}

modifiedMediaTypes := operationItem.RequestBodyDiff.ContentDiff.MediaTypeModified
for mediaType, mediaTypeDiff := range modifiedMediaTypes {
if mediaTypeDiff.SchemaDiff != nil && mediaTypeDiff.SchemaDiff.DefaultDiff != nil {
defaultValueDiff := mediaTypeDiff.SchemaDiff.DefaultDiff
result = append(result, ApiChange{
Id: "request-body-default-value-changed",
Level: INFO,
Text: fmt.Sprintf(config.i18n("request-body-default-value-changed"), ColorizedValue(mediaType), empty2none(defaultValueDiff.From), empty2none(defaultValueDiff.To)),
Operation: operation,
OperationId: operationItem.Revision.OperationID,
Path: path,
Source: source,
})

if defaultValueDiff.From == nil {
appendResultItem("request-body-default-value-added", ColorizedValue(mediaType), empty2none(defaultValueDiff.To))
} else if defaultValueDiff.To == nil {
appendResultItem("request-body-default-value-removed", ColorizedValue(mediaType), empty2none(defaultValueDiff.From))
} else {
appendResultItem("request-body-default-value-changed", ColorizedValue(mediaType), empty2none(defaultValueDiff.From), empty2none(defaultValueDiff.To))
}
}

CheckModifiedPropertiesDiff(
Expand All @@ -47,15 +58,13 @@ func RequestPropertyDefaultValueChangedCheck(diffReport *diff.Diff, operationsSo

defaultValueDiff := propertyDiff.DefaultDiff

result = append(result, ApiChange{
Id: "request-property-default-value-changed",
Level: INFO,
Text: fmt.Sprintf(config.i18n("request-property-default-value-changed"), ColorizedValue(propertyName), empty2none(defaultValueDiff.From), empty2none(defaultValueDiff.To)),
Operation: operation,
OperationId: operationItem.Revision.OperationID,
Path: path,
Source: source,
})
if defaultValueDiff.From == nil {
appendResultItem("request-property-default-value-added", ColorizedValue(propertyName), empty2none(defaultValueDiff.To))
} else if defaultValueDiff.To == nil {
appendResultItem("request-property-default-value-removed", ColorizedValue(propertyName), empty2none(defaultValueDiff.From))
} else {
appendResultItem("request-property-default-value-changed", ColorizedValue(propertyName), empty2none(defaultValueDiff.From), empty2none(defaultValueDiff.To))
}
})
}
}
Expand Down
68 changes: 68 additions & 0 deletions checker/check-request-property-default-value-changed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,71 @@ func TestRequestPropertyDefaultValueChanged(t *testing.T) {
OperationId: "createProduct",
}, errs[0])
}

// CL: adding request body default value or request property default value
func TestRequestBodyDefaultValueAdded(t *testing.T) {
s1, err := open("../data/checker/request_body_default_value_changed_base.yaml")
require.NoError(t, err)
s2, err := open("../data/checker/request_body_default_value_changed_base.yaml")
require.NoError(t, err)

s1.Spec.Paths["/products"].Post.RequestBody.Value.Content["text/plain"].Schema.Value.Default = nil
s1.Spec.Paths["/products"].Post.RequestBody.Value.Content["application/json"].Schema.Value.Properties["price"].Value.Default = nil

d, osm, err := diff.GetWithOperationsSourcesMap(getConfig(), s1, s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibilityUntilLevel(singleCheckConfig(checker.RequestPropertyDefaultValueChangedCheck), d, osm, checker.INFO)
require.Len(t, errs, 2)
require.ElementsMatch(t, []checker.ApiChange{{
Id: "request-body-default-value-added",
Text: "the request body 'text/plain' default value 'Default' was added",
Comment: "",
Level: checker.INFO,
Operation: "POST",
Path: "/products",
Source: "../data/checker/request_body_default_value_changed_base.yaml",
OperationId: "createProduct",
}, {
Id: "request-property-default-value-added",
Text: "the 'price' request property default value '10.00' was added",
Level: checker.INFO,
Operation: "POST",
Path: "/products",
Source: "../data/checker/request_body_default_value_changed_base.yaml",
OperationId: "createProduct",
}}, errs)
}

// CL: removing request body default value or request property default value
func TestRequestBodyDefaultValueRemoving(t *testing.T) {
s1, err := open("../data/checker/request_body_default_value_changed_base.yaml")
require.NoError(t, err)
s2, err := open("../data/checker/request_body_default_value_changed_base.yaml")
require.NoError(t, err)

s2.Spec.Paths["/products"].Post.RequestBody.Value.Content["text/plain"].Schema.Value.Default = nil
s2.Spec.Paths["/products"].Post.RequestBody.Value.Content["application/json"].Schema.Value.Properties["price"].Value.Default = nil

d, osm, err := diff.GetWithOperationsSourcesMap(getConfig(), s1, s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibilityUntilLevel(singleCheckConfig(checker.RequestPropertyDefaultValueChangedCheck), d, osm, checker.INFO)
require.Len(t, errs, 2)
require.ElementsMatch(t, []checker.ApiChange{{
Id: "request-body-default-value-removed",
Text: "the request body 'text/plain' default value 'Default' was removed",
Comment: "",
Level: checker.INFO,
Operation: "POST",
Path: "/products",
Source: "../data/checker/request_body_default_value_changed_base.yaml",
OperationId: "createProduct",
}, {
Id: "request-property-default-value-removed",
Text: "the 'price' request property default value '10.00' was removed",
Level: checker.INFO,
Operation: "POST",
Path: "/products",
Source: "../data/checker/request_body_default_value_changed_base.yaml",
OperationId: "createProduct",
}}, errs)
}
2 changes: 1 addition & 1 deletion checker/check-request-property-enum-value-updated.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func RequestPropertyEnumValueUpdatedCheck(diffReport *diff.Diff, operationsSourc
result = append(result, ApiChange{
Id: "request-property-enum-value-removed",
Level: level,
Text: fmt.Sprintf(config.i18n("request-property-enum-value-removed"), enumVal, ColorizedValue(propertyFullName(propertyPath, propertyName))),
Text: fmt.Sprintf(config.i18n("request-property-enum-value-removed"), ColorizedValue(enumVal), ColorizedValue(propertyFullName(propertyPath, propertyName))),
Operation: operation,
OperationId: operationItem.Revision.OperationID,
Path: path,
Expand Down
Loading

0 comments on commit b238ffa

Please sign in to comment.