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

avoid breaking-change on alpha/draft stability level endpoints when path is deleted #623

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
5 changes: 5 additions & 0 deletions checker/check_api_removed.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ func checkRemovedPaths(pathsDiff *diff.PathsDiff, operationsSources *diff.Operat

for operation := range pathsDiff.Base.Value(path).Operations() {
op := pathsDiff.Base.Value(path).GetOperation(operation)
stability, err := getStabilityLevel(op.Extensions)
if err != nil || stability == STABILITY_ALPHA || stability == STABILITY_DRAFT {
continue
}

if change := checkAPIRemoval(config, true, op, operationsSources, operation, path); change != nil {
result = append(result, change)
}
Expand Down
24 changes: 7 additions & 17 deletions checker/check_api_removed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,46 +44,36 @@ func TestBreaking_DeprecationNoSunset(t *testing.T) {
require.Equal(t, checker.INFO, errs[0].GetLevel())
}

// BC: removing the path without a deprecation policy and without specifying sunset date is breaking if some APIs are not alpha stability level
// BC: removing the path without a deprecation policy and without specifying sunset date is breaking for endpoints with non draft/alpha stability level
func TestBreaking_RemovedPathForAlphaBreaking(t *testing.T) {
s1, err := open(getDeprecationFile("base-alpha-stability.yaml"))
require.NoError(t, err)

s2, err := open(getDeprecationFile("base-alpha-stability.yaml"))
s2, err := open(getDeprecationFile("sunset-path.yaml"))
require.NoError(t, err)

s2.Spec.Paths.Delete("/api/test")

d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibility(singleCheckConfig(checker.APIRemovedCheck), d, osm)
require.Len(t, errs, 2)
require.Len(t, errs, 1)
require.Equal(t, checker.APIPathRemovedWithoutDeprecationId, errs[0].GetId())
require.Equal(t, "api path removed without deprecation", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer()))
require.Equal(t, checker.APIPathRemovedWithoutDeprecationId, errs[1].GetId())
require.Equal(t, "api path removed without deprecation", errs[1].GetUncolorizedText(checker.NewDefaultLocalizer()))
}

// BC: removing the path without a deprecation policy and without specifying sunset date is breaking if some APIs are not draft stability level
// BC: removing the path without a deprecation policy and without specifying sunset date is breaking for endpoints with non draft/alpha stability level
func TestBreaking_RemovedPathForDraftBreaking(t *testing.T) {
s1, err := open(getDeprecationFile("base-alpha-stability.yaml"))
s1, err := open(getDeprecationFile("base-draft-stability.yaml"))
require.NoError(t, err)
draft := toJson(t, checker.STABILITY_DRAFT)
s1.Spec.Paths.Value("/api/test").Get.Extensions["x-stability-level"] = draft

s2, err := open(getDeprecationFile("base-alpha-stability.yaml"))
s2, err := open(getDeprecationFile("sunset-path.yaml"))
require.NoError(t, err)

s2.Spec.Paths.Delete("/api/test")

d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibility(singleCheckConfig(checker.APIRemovedCheck), d, osm)
require.Len(t, errs, 2)
require.Len(t, errs, 1)
require.Equal(t, checker.APIPathRemovedWithoutDeprecationId, errs[0].GetId())
require.Equal(t, "api path removed without deprecation", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer()))
require.Equal(t, checker.APIPathRemovedWithoutDeprecationId, errs[1].GetId())
require.Equal(t, "api path removed without deprecation", errs[1].GetUncolorizedText(checker.NewDefaultLocalizer()))
}

// BC: deleting a path with some operations having sunset date in the future is breaking
Expand Down
15 changes: 15 additions & 0 deletions data/deprecation/base-draft-stability.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
info:
title: Tufin
version: 1.0.0
openapi: 3.0.3
paths:
/api/test:
post:
responses:
201:
description: OK
get:
x-stability-level: draft
responses:
200:
description: OK
9 changes: 4 additions & 5 deletions docs/BREAKING-CHANGES-EXAMPLES.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ These examples are automatically generated from unit tests.
[deleting a media-type from response is breaking](../checker/check_breaking_test.go?plain=1#L442)
[deleting a non-required non-write-only property in response body is breaking with warning](../checker/check_breaking_property_test.go?plain=1#L512)
[deleting a path is breaking](../checker/check_breaking_test.go?plain=1#L37)
[deleting a path with some operations having sunset date in the future is breaking](../checker/check_api_removed_test.go?plain=1#L89)
[deleting a path with some operations having sunset date in the future is breaking](../checker/check_api_removed_test.go?plain=1#L79)
[deleting a required property in request is breaking with warn](../checker/check_breaking_property_test.go?plain=1#L369)
[deleting a required property in response body is breaking](../checker/check_breaking_property_test.go?plain=1#L421)
[deleting a required property under AllOf in response body is breaking](../checker/check_breaking_property_test.go?plain=1#L451)
Expand Down Expand Up @@ -80,7 +80,7 @@ These examples are automatically generated from unit tests.
[removing 'allOf' subschema from the request body or request body property is breaking with warn](../checker/check_breaking_test.go?plain=1#L749)
[removing 'anyOf' schema from the request body or request body property is breaking](../checker/check_breaking_test.go?plain=1#L684)
[removing 'oneOf' schema from the request body or request body property is breaking](../checker/check_breaking_test.go?plain=1#L706)
[removing a deprecated enpoint with an invalid date is breaking](../checker/check_api_removed_test.go?plain=1#L130)
[removing a deprecated enpoint with an invalid date is breaking](../checker/check_api_removed_test.go?plain=1#L120)
[removing a media type from request body is breaking](../checker/check_breaking_test.go?plain=1#L668)
[removing a success status is breaking](../checker/check_response_status_updated_test.go?plain=1#L87)
[removing an existing optional response header is breaking as warn](../checker/check_breaking_test.go?plain=1#L422)
Expand All @@ -89,8 +89,7 @@ These examples are automatically generated from unit tests.
[removing an existing response with successful status is breaking](../checker/check_breaking_test.go?plain=1#L251)
[removing an schema object from components is breaking (optional)](../checker/check_breaking_test.go?plain=1#L643)
[removing the default value of an optional request parameter is breaking](../checker/check_breaking_test.go?plain=1#L606)
[removing the path without a deprecation policy and without specifying sunset date is breaking if some APIs are not alpha stability level](../checker/check_api_removed_test.go?plain=1#L47)
[removing the path without a deprecation policy and without specifying sunset date is breaking if some APIs are not draft stability level](../checker/check_api_removed_test.go?plain=1#L67)
[removing the path without a deprecation policy and without specifying sunset date is breaking for endpoints with non draft/alpha stability level](../checker/check_api_removed_test.go?plain=1#L47)
[removing/updating a property enum in response is breaking (optional)](../checker/check_breaking_test.go?plain=1#L333)
[removing/updating a tag is breaking (optional)](../checker/check_breaking_test.go?plain=1#L351)
[removing/updating an enum in request body is breaking (optional)](../checker/check_breaking_test.go?plain=1#L310)
Expand Down Expand Up @@ -134,7 +133,7 @@ These examples are automatically generated from unit tests.
[changing servers is not breaking](../checker/check_not_breaking_test.go?plain=1#L252)
[deleting a deprecated operation without sunset date is not breaking](../checker/check_api_removed_test.go?plain=1#L29)
[deleting a path after sunset date of all contained operations is not breaking](../checker/check_api_deprecation_test.go?plain=1#L255)
[deleting a path with deprecated operations without sunset date is not breaking](../checker/check_api_removed_test.go?plain=1#L107)
[deleting a path with deprecated operations without sunset date is not breaking](../checker/check_api_removed_test.go?plain=1#L97)
[deleting a pattern from a schema is not breaking](../checker/check_breaking_test.go?plain=1#L459)
[deleting a required write-only property in response body is not breaking](../checker/check_breaking_property_test.go?plain=1#L495)
[deleting a tag is not breaking](../checker/check_not_breaking_test.go?plain=1#L71)
Expand Down
Loading