Skip to content

Commit

Permalink
add test
Browse files Browse the repository at this point in the history
  • Loading branch information
reuvenharrison committed Oct 13, 2024
1 parent 4e52e6c commit e96dac9
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 17 deletions.
33 changes: 25 additions & 8 deletions checker/check_api_removed.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,42 @@ const (
)

func APIRemovedCheck(diffReport *diff.Diff, operationsSources *diff.OperationsSourcesMap, config *Config) Changes {
result := make(Changes, 0)
if diffReport.PathsDiff == nil {
return result
return append(
checkRemovedPaths(diffReport.PathsDiff, operationsSources, config),
checkRemovedOperations(diffReport.PathsDiff, operationsSources, config)...,
)
}

func checkRemovedPaths(pathsDiff *diff.PathsDiff, operationsSources *diff.OperationsSourcesMap, config *Config) Changes {

if pathsDiff == nil {
return nil
}

for _, path := range diffReport.PathsDiff.Deleted {
if diffReport.PathsDiff.Base.Value(path) == nil {
result := make(Changes, 0)
for _, path := range pathsDiff.Deleted {
if pathsDiff.Base.Value(path) == nil {
continue
}

for operation := range diffReport.PathsDiff.Base.Value(path).Operations() {
op := diffReport.PathsDiff.Base.Value(path).GetOperation(operation)
for operation := range pathsDiff.Base.Value(path).Operations() {
op := pathsDiff.Base.Value(path).GetOperation(operation)
if change := checkAPIRemoval(config, true, op, operationsSources, operation, path); change != nil {
result = append(result, change)
}
}
}
return result
}

func checkRemovedOperations(pathsDiff *diff.PathsDiff, operationsSources *diff.OperationsSourcesMap, config *Config) Changes {
if pathsDiff == nil {
return nil
}

result := make(Changes, 0)

for path, pathItem := range diffReport.PathsDiff.Modified {
for path, pathItem := range pathsDiff.Modified {
if pathItem.OperationsDiff == nil {
continue
}
Expand Down
32 changes: 29 additions & 3 deletions checker/check_api_removed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func TestBreaking_RemoveBeforeSunset(t *testing.T) {
require.Equal(t, "api removed before the sunset date '9999-08-10'", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer()))
}

// BC: deleting an operation without sunset date is not breaking
// BC: deleting a deprecated operation without sunset date is not breaking
func TestBreaking_DeprecationNoSunset(t *testing.T) {

s1, err := open(getDeprecationFile("deprecated-no-sunset.yaml"))
Expand All @@ -36,9 +36,12 @@ func TestBreaking_DeprecationNoSunset(t *testing.T) {
require.NoError(t, err)

d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
errs := checker.CheckBackwardCompatibility(singleCheckConfig(checker.APIRemovedCheck), d, osm)
errs := checker.CheckBackwardCompatibilityUntilLevel(singleCheckConfig(checker.APIRemovedCheck), d, osm, checker.INFO)
require.NoError(t, err)
require.Empty(t, errs)
require.Len(t, errs, 1)
require.Equal(t, checker.APIRemovedWithDeprecationId, errs[0].GetId())
require.Equal(t, "api removed with deprecation", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer()))
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
Expand Down Expand Up @@ -101,6 +104,29 @@ func TestBreaking_DeprecationPathMixed(t *testing.T) {
require.Equal(t, "api path removed before the sunset date '9999-08-10'", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer()))
}

// BC: deleting a path with deprecated operations without sunset date is not breaking
func TestBreaking_PathDeprecationNoSunset(t *testing.T) {

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

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

d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
errs := checker.CheckBackwardCompatibilityUntilLevel(singleCheckConfig(checker.APIRemovedCheck), d, osm, checker.INFO)
require.NoError(t, err)
require.Len(t, errs, 2)

require.Equal(t, checker.APIPathRemovedWithDeprecationId, errs[0].GetId())
require.Equal(t, "api path removed with deprecation", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer()))
require.Equal(t, checker.INFO, errs[0].GetLevel())

require.Equal(t, checker.APIPathRemovedWithDeprecationId, errs[1].GetId())
require.Equal(t, "api path removed with deprecation", errs[1].GetUncolorizedText(checker.NewDefaultLocalizer()))
require.Equal(t, checker.INFO, errs[1].GetLevel())
}

// BC: removing a deprecated enpoint with an invalid date is breaking
func TestBreaking_RemoveEndpointWithInvalidSunset(t *testing.T) {

Expand Down
2 changes: 2 additions & 0 deletions checker/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ const (
APIStabilityDecreasedId = "api-stability-decreased"
)

// CheckBackwardCompatibility runs the checks with level WARN and ERR
func CheckBackwardCompatibility(config *Config, diffReport *diff.Diff, operationsSources *diff.OperationsSourcesMap) Changes {
return CheckBackwardCompatibilityUntilLevel(config, diffReport, operationsSources, WARN)
}

// CheckBackwardCompatibilityUntilLevel runs the checks with level equal or higher than the given level
func CheckBackwardCompatibilityUntilLevel(config *Config, diffReport *diff.Diff, operationsSources *diff.OperationsSourcesMap, level Level) Changes {
result := make(Changes, 0)

Expand Down
21 changes: 21 additions & 0 deletions data/deprecation/deprecated-path-no-sunset.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
info:
title: Tufin
version: 1.0.0
openapi: 3.0.3
paths:
/api/placeholder:
get:
responses:
200:
description: OK
/api/test:
get:
deprecated: true
responses:
200:
description: OK
post:
deprecated: true
responses:
201:
description: OK
11 changes: 6 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#L86)
[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 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#L104)
[removing a deprecated enpoint with an invalid date is breaking](../checker/check_api_removed_test.go?plain=1#L130)
[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,8 @@ 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#L44)
[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#L64)
[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/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 @@ -132,12 +132,13 @@ These examples are automatically generated from unit tests.
[changing response's body schema type from number to integer is not breaking](../checker/check_breaking_response_type_changed_test.go?plain=1#L52)
[changing response's body schema type from number/none to integer/int32 is not breaking](../checker/check_breaking_response_type_changed_test.go?plain=1#L90)
[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 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)
[deleting an operation after sunset date is not breaking](../checker/check_api_deprecation_test.go?plain=1#L33)
[deleting an operation without sunset date is not breaking](../checker/check_api_removed_test.go?plain=1#L29)
[deleting other extension (not sunset) header for a deprecated endpoint is not breaking](../checker/check_api_sunset_changed_test.go?plain=1#L84)
[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)
Expand Down
3 changes: 2 additions & 1 deletion docs/DEPRECATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ Sometimes APIs need to be removed, for example, when we replace an old API by a
As API owners, we want a process that will allow us to phase out the old API version and transition to the new one smoothly as possible and with minimal disruptions to business.

OpenAPI specification supports a ```deprecated``` flag which can be used to mark operations and other object types as deprecated.
Normally, deprecation **is not** considered a breaking change since it doesn't break the client but only serves as an indication of an intent to remove something in the future, in contrast, the eventual removal of a resource **is** considered a breaking change.
Deprecating a resource **isn't** considered a breaking change since it doesn't break the client but only serves as an indication of an intent to remove something in the future.
After deprecating a resource, it can be removed without triggering a breaking change since the client already knows it is going to be removed.

### Deprecation without a sunset date
Oasdiff allows you to gracefully remove a resource without getting a breaking change error, as follows:
Expand Down

0 comments on commit e96dac9

Please sign in to comment.