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

Fix panic when modifying path params while also renaming them #658

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions checker/check_request_parameter_sunset_changed.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ func RequestParameterSunsetChangedCheck(diffReport *diff.Diff, operationsSources
for paramLocation, paramItems := range operationDiff.ParametersDiff.Modified {
for paramName, paramItem := range paramItems {

paramBase := opBase.Parameters.GetByInAndName(paramLocation, paramName)
paramRevision := opRevision.Parameters.GetByInAndName(paramLocation, paramName)
paramBase := paramItem.Base
paramRevision := paramItem.Revision

if !paramRevision.Deprecated {
continue
Expand Down
18 changes: 18 additions & 0 deletions checker/check_request_parameter_sunset_changed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,24 @@ func TestBreaking_SunsetDeletedForDeprecatedParameter(t *testing.T) {
require.Equal(t, "'query' request parameter 'id' sunset date deleted, but deprecated=true kept", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer()))
}

// BC: deleting sunset header for a deprecated parameter is breaking even if the parameter is renamed
func TestBreaking_SunsetDeletedForDeprecatedAndRenamedParameter(t *testing.T) {

s1, err := open(getParameterDeprecationFile("deprecated-with-sunset-path.yaml"))
require.NoError(t, err)

s2, err := open(getParameterDeprecationFile("deprecated-no-sunset-path-renamed.yaml"))
require.NoError(t, err)

d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibility(singleCheckConfig(checker.RequestParameterSunsetChangedCheck), d, osm)
require.NotEmpty(t, errs)
require.Len(t, errs, 1)
require.Equal(t, checker.RequestParameterSunsetDeletedId, errs[0].GetId())
require.Equal(t, "'path' request parameter 'id' sunset date deleted, but deprecated=true kept", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer()))
}

// BC: changing sunset to an earlier date for a deprecated parameter with a deprecation policy is breaking
func TestBreaking_SunsetModifiedForDeprecatedParameter(t *testing.T) {

Expand Down
4 changes: 2 additions & 2 deletions checker/check_request_parameters_default_value_changed.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ func RequestParameterDefaultValueChangedCheck(diffReport *diff.Diff, operationsS
for paramLocation, paramDiffs := range operationItem.ParametersDiff.Modified {
for paramName, paramDiff := range paramDiffs {

baseParam := operationItem.Base.Parameters.GetByInAndName(paramLocation, paramName)
baseParam := paramDiff.Base
if baseParam == nil || baseParam.Required {
continue
}

revisionParam := operationItem.Revision.Parameters.GetByInAndName(paramLocation, paramName)
revisionParam := paramDiff.Revision
if revisionParam == nil || revisionParam.Required {
continue
}
Expand Down
22 changes: 22 additions & 0 deletions checker/check_request_parameters_default_value_changed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,28 @@ func TestRequestParameterDefaultValueChanged(t *testing.T) {
}, errs[0])
}

// CL: changing request parameter default value, while the param is also renamed
func TestRequestParameterDefaultValueChangedAndRenamedParameter(t *testing.T) {
s1, err := open("../data/checker/request_parameter_default_value_changed_base_renamed.yaml")
require.NoError(t, err)
s2, err := open("../data/checker/request_parameter_default_value_changed_revision_renamed.yaml")
require.NoError(t, err)

d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibilityUntilLevel(singleCheckConfig(checker.RequestParameterDefaultValueChangedCheck), d, osm, checker.ERR)
require.Len(t, errs, 1)
require.Equal(t, checker.ApiChange{
Id: checker.RequestParameterDefaultValueChangedId,
Args: []any{"path", "group_id", "2", "1"},
Level: checker.ERR,
Operation: "POST",
Path: "/api/v1.0/groups/{group_id}",
Source: load.NewSource("../data/checker/request_parameter_default_value_changed_revision_renamed.yaml"),
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")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
openapi: 3.0.1
info:
title: Tufin
version: "2.0"
servers:
- url: https://localhost:9080
paths:
/api/v1.0/groups/{group_id}:
post:
operationId: createOneGroup
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/GroupView'
description: Creates one project.
required: true
parameters:
- in: path
name: group_id
schema:
type: string
default: "2"
- in: query
name: category
schema:
type: string
responses:
"200":
content:
application/json:
schema:
$ref: '#/components/schemas/GroupView'
description: OK
"409":
content:
application/json:
schema:
$ref: '#/components/schemas/GroupView'
description: Conflict
summary: Create One Project
components:
schemas:
GroupView:
type: object
properties:
data:
type: object
properties:
created:
type: string
format: date-time
readOnly: true
pattern: "^[a-z]+$"
id:
type: string
readOnly: true
name:
type: string
required:
- name
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
openapi: 3.0.1
info:
title: Tufin
version: "2.0"
servers:
- url: https://localhost:9080
paths:
/api/v1.0/groups/{groupId}:
post:
operationId: createOneGroup
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/GroupView'
description: Creates one project.
required: true
parameters:
- in: path
name: groupId
schema:
type: string
default: "1"
- in: query
name: category
schema:
type: string
responses:
"200":
content:
application/json:
schema:
$ref: '#/components/schemas/GroupView'
description: OK
"409":
content:
application/json:
schema:
$ref: '#/components/schemas/GroupView'
description: Conflict
summary: Create One Project
components:
schemas:
GroupView:
type: object
properties:
data:
type: object
properties:
created:
type: string
format: date-time
readOnly: true
pattern: "^[a-z]+$"
id:
type: string
readOnly: true
name:
type: string
required:
- name
22 changes: 22 additions & 0 deletions data/param-deprecation/deprecated-no-sunset-path-renamed.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
info:
title: Tufin
version: 1.0.0
openapi: 3.0.3
paths:
/api/test/{ParamID}:
get:
parameters:
- $ref: '#/components/parameters/id'
responses:
200:
description: OK
components:
parameters:
id:
name: ParamID
in: path
required: true
schema:
type: string
description: 'The ID'
deprecated: true
23 changes: 23 additions & 0 deletions data/param-deprecation/deprecated-with-sunset-path.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
info:
title: Tufin
version: 1.0.0
openapi: 3.0.3
paths:
/api/test/{id}:
get:
parameters:
- $ref: '#/components/parameters/id'
responses:
200:
description: OK
components:
parameters:
id:
name: id
in: path
required: true
schema:
type: string
description: 'The ID'
deprecated: true
x-sunset: "2023-08-02"
16 changes: 9 additions & 7 deletions docs/BREAKING-CHANGES-EXAMPLES.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ These examples are automatically generated from unit tests.
[changing response's body schema type from string to number is breaking](../checker/check_breaking_response_type_changed_test.go?plain=1#L12)
[changing response's embedded property schema type from string/none to integer/int32 is breaking](../checker/check_breaking_response_type_changed_test.go?plain=1#L109)
[changing sunset from an invalid date for a deprecated endpoint is breaking](../checker/check_api_sunset_changed_test.go?plain=1#L65)
[changing sunset from an invalid date for a deprecated parameter is breaking](../checker/check_request_parameter_sunset_changed_test.go?plain=1#L66)
[changing sunset from an invalid date for a deprecated parameter is breaking](../checker/check_request_parameter_sunset_changed_test.go?plain=1#L84)
[changing sunset to an earlier date for a deprecated endpoint with a deprecation policy is breaking](../checker/check_api_sunset_changed_test.go?plain=1#L29)
[changing sunset to an earlier date for a deprecated parameter with a deprecation policy is breaking](../checker/check_request_parameter_sunset_changed_test.go?plain=1#L29)
[changing sunset to an earlier date for a deprecated parameter with a deprecation policy is breaking](../checker/check_request_parameter_sunset_changed_test.go?plain=1#L47)
[changing sunset to an invalid date for a deprecated endpoint is breaking](../checker/check_api_sunset_changed_test.go?plain=1#L47)
[changing sunset to an invalid date for a deprecated parameter is breaking](../checker/check_request_parameter_sunset_changed_test.go?plain=1#L47)
[changing sunset to an invalid date for a deprecated parameter is breaking](../checker/check_request_parameter_sunset_changed_test.go?plain=1#L65)
[decreasing maxItems of common request parameters with --flatten-params is breaking](../checker/check_request_parameters_max_items_updated_test.go?plain=1#L72)
[decreasing stability level is breaking](../checker/checker_test.go?plain=1#L11)
[deleting a media-type from response is breaking](../checker/check_breaking_test.go?plain=1#L418)
Expand All @@ -65,6 +65,7 @@ These examples are automatically generated from unit tests.
[deleting an operation before sunset date is breaking](../checker/check_api_removed_test.go?plain=1#L51)
[deleting an operation without deprecation is breaking](../checker/check_api_removed_test.go?plain=1#L20)
[deleting sunset header for a deprecated endpoint is breaking](../checker/check_api_sunset_changed_test.go?plain=1#L11)
[deleting sunset header for a deprecated parameter is breaking even if the parameter is renamed](../checker/check_request_parameter_sunset_changed_test.go?plain=1#L29)
[deleting sunset header for a deprecated parameter is breaking](../checker/check_request_parameter_sunset_changed_test.go?plain=1#L11)
[deprecating a parameter with a deprecation policy and an invalid sunset date is breaking](../checker/check_request_parameter_deprecation_test.go?plain=1#L18)
[deprecating a parameter with a deprecation policy and sunset date before required deprecation period is breaking](../checker/check_request_parameter_deprecation_test.go?plain=1#L102)
Expand Down Expand Up @@ -153,7 +154,7 @@ These examples are automatically generated from unit tests.
[deleting a tag is not breaking](../checker/check_not_breaking_test.go?plain=1#L71)
[deleting an operation after sunset date is not breaking](../checker/check_api_removed_test.go?plain=1#L36)
[deleting other extension (not sunset) header for a deprecated endpoint is not breaking](../checker/check_api_sunset_changed_test.go?plain=1#L84)
[deleting other extension (not sunset) header for a deprecated parameter is not breaking](../checker/check_request_parameter_sunset_changed_test.go?plain=1#L85)
[deleting other extension (not sunset) header for a deprecated parameter is not breaking](../checker/check_request_parameter_sunset_changed_test.go?plain=1#L103)
[deprecating a header is not breaking](../checker/check_not_breaking_test.go?plain=1#L226)
[deprecating a parameter is not breaking](../checker/check_not_breaking_test.go?plain=1#L213)
[deprecating a parameter with a default deprecation policy but without specifying sunset date is not breaking](../checker/check_request_parameter_deprecation_test.go?plain=1#L71)
Expand All @@ -177,7 +178,7 @@ These examples are automatically generated from unit tests.
[new required response header param is not breaking](../checker/check_not_breaking_test.go?plain=1#L152)
[no change is not breaking](../checker/check_not_breaking_test.go?plain=1#L27)
[no change to headers for a deprecated endpoint is not breaking](../checker/check_api_sunset_changed_test.go?plain=1#L99)
[no change to headers for a deprecated parameter is not breaking](../checker/check_request_parameter_sunset_changed_test.go?plain=1#L100)
[no change to headers for a deprecated parameter is not breaking](../checker/check_request_parameter_sunset_changed_test.go?plain=1#L118)
[reducing max in response is not breaking](../checker/check_breaking_min_max_test.go?plain=1#L281)
[reducing max length in response is not breaking](../checker/check_breaking_min_max_test.go?plain=1#L31)
[reducing min items in request is not breaking](../checker/check_breaking_min_max_test.go?plain=1#L206)
Expand Down Expand Up @@ -220,7 +221,7 @@ These examples are automatically generated from unit tests.
[adding discriminator to the response body or response property](../checker/check_response_discriminator_updated_test.go?plain=1#L13)
[adding pattern to request parameters](../checker/check_request_parameter_pattern_added_or_changed_test.go?plain=1#L60)
[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 parameter default value](../checker/check_request_parameters_default_value_changed_test.go?plain=1#L56)
[adding request property enum values](../checker/check_request_property_enum_value_updated_test.go?plain=1#L67)
[adding request property pattern](../checker/check_request_property_pattern_added_or_changed_test.go?plain=1#L60)
[adding response body default value or response body property default value](../checker/check_response_property_default_value_changed_test.go?plain=1#L64)
Expand Down Expand Up @@ -257,6 +258,7 @@ These examples are automatically generated from unit tests.
[changing request body type](../checker/check_request_property_type_changed_test.go?plain=1#L14)
[changing request header parameter format](../checker/check_request_parameters_type_changed_test.go?plain=1#L134)
[changing request header parameter type](../checker/check_request_parameters_type_changed_test.go?plain=1#L62)
[changing request parameter default value, while the param is also renamed](../checker/check_request_parameters_default_value_changed_test.go?plain=1#L34)
[changing request parameter default value](../checker/check_request_parameters_default_value_changed_test.go?plain=1#L12)
[changing request parameter type to enum](../checker/check_request_parameter_became_enum_test.go?plain=1#L12)
[changing request path parameter format](../checker/check_request_parameters_type_changed_test.go?plain=1#L86)
Expand Down Expand Up @@ -360,7 +362,7 @@ These examples are automatically generated from unit tests.
[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#L83)
[removing request body default value or request property default value](../checker/check_request_property_default_value_changed_test.go?plain=1#L91)
[removing request parameter default value](../checker/check_request_parameters_default_value_changed_test.go?plain=1#L58)
[removing request parameter default value](../checker/check_request_parameters_default_value_changed_test.go?plain=1#L80)
[removing request property enum values](../checker/check_request_property_enum_value_updated_test.go?plain=1#L12)
[removing request property pattern](../checker/check_request_property_pattern_added_or_changed_test.go?plain=1#L83)
[removing request read-only property enum values](../checker/check_request_property_enum_value_updated_test.go?plain=1#L39)
Expand Down
Loading