diff --git a/checker/check_api_removed.go b/checker/check_api_removed.go index 422bf118..3b6dadb2 100644 --- a/checker/check_api_removed.go +++ b/checker/check_api_removed.go @@ -10,38 +10,57 @@ import ( const ( APIPathRemovedWithoutDeprecationId = "api-path-removed-without-deprecation" + APIPathRemovedWithDeprecationId = "api-path-removed-with-deprecation" APIPathSunsetParseId = "api-path-sunset-parse" APIPathRemovedBeforeSunsetId = "api-path-removed-before-sunset" APIRemovedWithoutDeprecationId = "api-removed-without-deprecation" + APIRemovedWithDeprecationId = "api-removed-with-deprecation" APIRemovedBeforeSunsetId = "api-removed-before-sunset" ) 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) - if change := checkAPIRemoval(config, APIPathRemovedWithoutDeprecationId, APIPathRemovedBeforeSunsetId, op, operationsSources, operation, path); change != nil { + 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 +} - for path, pathItem := range diffReport.PathsDiff.Modified { +func checkRemovedOperations(pathsDiff *diff.PathsDiff, operationsSources *diff.OperationsSourcesMap, config *Config) Changes { + if pathsDiff == nil { + return nil + } + + result := make(Changes, 0) + + for path, pathItem := range pathsDiff.Modified { if pathItem.OperationsDiff == nil { continue } for _, operation := range pathItem.OperationsDiff.Deleted { op := pathItem.Base.GetOperation(operation) - if change := checkAPIRemoval(config, APIRemovedWithoutDeprecationId, APIRemovedBeforeSunsetId, op, operationsSources, operation, path); change != nil { + if change := checkAPIRemoval(config, false, op, operationsSources, operation, path); change != nil { result = append(result, change) } } @@ -50,10 +69,10 @@ func APIRemovedCheck(diffReport *diff.Diff, operationsSources *diff.OperationsSo return result } -func checkAPIRemoval(config *Config, deprecationId, sunsetId string, op *openapi3.Operation, operationsSources *diff.OperationsSourcesMap, method, path string) Change { +func checkAPIRemoval(config *Config, isPath bool, op *openapi3.Operation, operationsSources *diff.OperationsSourcesMap, method, path string) Change { if !op.Deprecated { return NewApiChange( - deprecationId, + getWithoutDeprecationId(isPath), config, nil, "", @@ -65,8 +84,16 @@ func checkAPIRemoval(config *Config, deprecationId, sunsetId string, op *openapi } sunset, ok := getSunset(op.Extensions) if !ok { - // No sunset date, allow removal - return nil + return NewApiChange( + getWithDeprecationId(isPath), + config, + nil, + "", + operationsSources, + op, + method, + path, + ) } date, err := getSunsetDate(sunset) @@ -76,7 +103,7 @@ func checkAPIRemoval(config *Config, deprecationId, sunsetId string, op *openapi if civil.DateOf(time.Now()).Before(date) { return NewApiChange( - sunsetId, + getBeforeSunsetId(isPath), config, []any{date}, "", @@ -101,3 +128,24 @@ func getAPIPathSunsetParse(config *Config, operation *openapi3.Operation, operat path, ) } + +func getWithDeprecationId(isPath bool) string { + if isPath { + return APIPathRemovedWithDeprecationId + } + return APIRemovedWithDeprecationId +} + +func getWithoutDeprecationId(isPath bool) string { + if isPath { + return APIPathRemovedWithoutDeprecationId + } + return APIRemovedWithoutDeprecationId +} + +func getBeforeSunsetId(isPath bool) string { + if isPath { + return APIPathRemovedBeforeSunsetId + } + return APIRemovedBeforeSunsetId +} diff --git a/checker/check_api_removed_test.go b/checker/check_api_removed_test.go index bc52ce33..a019999d 100644 --- a/checker/check_api_removed_test.go +++ b/checker/check_api_removed_test.go @@ -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")) @@ -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 @@ -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) { diff --git a/checker/checker.go b/checker/checker.go index b22fc500..55c42f0b 100644 --- a/checker/checker.go +++ b/checker/checker.go @@ -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) diff --git a/checker/config_test.go b/checker/config_test.go index eac24fdc..f32a9fa8 100644 --- a/checker/config_test.go +++ b/checker/config_test.go @@ -9,7 +9,7 @@ import ( const ( numOfChecks = 91 - numOfIds = 262 + numOfIds = 264 ) func TestNewConfig(t *testing.T) { diff --git a/checker/localizations/localizations.go b/checker/localizations/localizations.go index 9dbb9d5f..e3c0d0d6 100644 --- a/checker/localizations/localizations.go +++ b/checker/localizations/localizations.go @@ -1,6 +1,6 @@ // Code generated by go-localize; DO NOT EDIT. // This file was generated by robots at -// 2024-07-17 10:54:08.151759 +0300 IDT m=+0.013463284 +// 2024-10-12 21:56:08.28391 +0300 IDT m=+0.005721042 package localizations @@ -32,12 +32,14 @@ var localizations = map[string]string{ "en.messages.api-operation-id-removed-description": "operation ID deleted from an endpoint", "en.messages.api-path-removed-before-sunset": "api path removed before the sunset date %s", "en.messages.api-path-removed-before-sunset-description": "path and endpoint deleted before sunset date", + "en.messages.api-path-removed-with-deprecation": "api path removed with deprecation", "en.messages.api-path-removed-without-deprecation": "api path removed without deprecation", "en.messages.api-path-removed-without-deprecation-description": "path and endpoint deleted without deprecation", "en.messages.api-path-sunset-parse": "failed to parse sunset date: %v", "en.messages.api-path-sunset-parse-description": "path and endpoint deleted with invalid or missing sunset date", "en.messages.api-removed-before-sunset": "api removed before the sunset date %s", "en.messages.api-removed-before-sunset-description": "endpoint deleted before sunset date", + "en.messages.api-removed-with-deprecation": "api removed with deprecation", "en.messages.api-removed-without-deprecation": "api removed without deprecation", "en.messages.api-removed-without-deprecation-description": "endpoint deleted without deprecation", "en.messages.api-schema-removed": "removed the schema %s", @@ -572,9 +574,11 @@ var localizations = map[string]string{ "ru.messages.api-path-deprecated": "API path deprecated", "ru.messages.api-path-reactivated": "API path реактивирован", "ru.messages.api-path-removed-before-sunset": "API path удалён до даты sunset %s", - "ru.messages.api-path-removed-without-deprecation": "api path удалён без процедуры deprecation", + "ru.messages.api-path-removed-with-deprecation": "API path удалён с процедурой deprecation", + "ru.messages.api-path-removed-without-deprecation": "API path удалён без процедуры deprecation", "ru.messages.api-path-sunset-parse": "не удалось проанализировать дату заката: %v", "ru.messages.api-removed-before-sunset": "API удалёг до даты sunset %s", + "ru.messages.api-removed-with-deprecation": "API удалён с процедурой deprecation", "ru.messages.api-removed-without-deprecation": "API удалён без deprecation", "ru.messages.api-schema-removed": "удалена схема %s", "ru.messages.api-security-added": "схема безопасности точки доступа %s была добавлена к API", diff --git a/checker/localizations_src/en/messages.yaml b/checker/localizations_src/en/messages.yaml index 804b7ec1..1b1f4f0e 100644 --- a/checker/localizations_src/en/messages.yaml +++ b/checker/localizations_src/en/messages.yaml @@ -23,8 +23,10 @@ endpoint-added: endpoint added endpoint-deprecated: endpoint deprecated endpoint-reactivated: endpoint reactivated api-path-removed-without-deprecation: api path removed without deprecation +api-path-removed-with-deprecation: api path removed with deprecation api-path-removed-before-sunset: api path removed before the sunset date %s api-removed-without-deprecation: api removed without deprecation +api-removed-with-deprecation: api removed with deprecation api-removed-before-sunset: api removed before the sunset date %s api-operation-id-removed: api operation id %s removed and replaced with %s api-operation-id-added: api operation id %s was added diff --git a/checker/localizations_src/ru/messages.yaml b/checker/localizations_src/ru/messages.yaml index 33f1756b..4bf65e57 100644 --- a/checker/localizations_src/ru/messages.yaml +++ b/checker/localizations_src/ru/messages.yaml @@ -22,9 +22,11 @@ api-sunset-date-too-small: дата API sunset date %s слишком рання api-path-added: API path добавлено api-path-deprecated: API path deprecated api-path-reactivated: API path реактивирован -api-path-removed-without-deprecation: api path удалён без процедуры deprecation +api-path-removed-without-deprecation: API path удалён без процедуры deprecation +api-path-removed-with-deprecation: API path удалён с процедурой deprecation api-path-removed-before-sunset: API path удалён до даты sunset %s api-removed-without-deprecation: API удалён без deprecation +api-removed-with-deprecation: API удалён с процедурой deprecation api-removed-before-sunset: API удалёг до даты sunset %s api-operation-id-removed: Идентификатор операции API %s удален и заменен на %s api-tag-removed: Тег API %s удален diff --git a/checker/rules.go b/checker/rules.go index 99321f76..d4e594db 100644 --- a/checker/rules.go +++ b/checker/rules.go @@ -100,9 +100,11 @@ func GetAllRules() BackwardCompatibilityRules { newBackwardCompatibilityRule(EndpointDeprecatedId, INFO, APIDeprecationCheck, DirectionNone, LocationNone, ActionNone), // APIRemovedCheck newBackwardCompatibilityRule(APIPathRemovedWithoutDeprecationId, ERR, APIRemovedCheck, DirectionNone, LocationNone, ActionRemove), + newBackwardCompatibilityRule(APIPathRemovedWithDeprecationId, INFO, APIRemovedCheck, DirectionNone, LocationNone, ActionRemove), newBackwardCompatibilityRule(APIPathSunsetParseId, ERR, APIRemovedCheck, DirectionNone, LocationNone, ActionNone), newBackwardCompatibilityRule(APIPathRemovedBeforeSunsetId, ERR, APIRemovedCheck, DirectionNone, LocationNone, ActionRemove), newBackwardCompatibilityRule(APIRemovedWithoutDeprecationId, ERR, APIRemovedCheck, DirectionNone, LocationNone, ActionRemove), + newBackwardCompatibilityRule(APIRemovedWithDeprecationId, INFO, APIRemovedCheck, DirectionNone, LocationNone, ActionRemove), newBackwardCompatibilityRule(APIRemovedBeforeSunsetId, ERR, APIRemovedCheck, DirectionNone, LocationNone, ActionRemove), // APISunsetChangedCheck newBackwardCompatibilityRule(APISunsetDeletedId, ERR, APISunsetChangedCheck, DirectionNone, LocationNone, ActionRemove), diff --git a/data/deprecation/deprecated-path-no-sunset.yaml b/data/deprecation/deprecated-path-no-sunset.yaml new file mode 100644 index 00000000..35b250ca --- /dev/null +++ b/data/deprecation/deprecated-path-no-sunset.yaml @@ -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 diff --git a/docs/BREAKING-CHANGES-EXAMPLES.md b/docs/BREAKING-CHANGES-EXAMPLES.md index 991c8caa..228ee97a 100644 --- a/docs/BREAKING-CHANGES-EXAMPLES.md +++ b/docs/BREAKING-CHANGES-EXAMPLES.md @@ -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) @@ -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) @@ -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) @@ -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) diff --git a/docs/DEPRECATION.md b/docs/DEPRECATION.md index 17d98899..f2131f2e 100644 --- a/docs/DEPRECATION.md +++ b/docs/DEPRECATION.md @@ -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: