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

feat: add check for api removed with deprecation #621

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
78 changes: 63 additions & 15 deletions checker/check_api_removed.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand All @@ -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,
"",
Expand All @@ -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)
Expand All @@ -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},
"",
Expand All @@ -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
}
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
2 changes: 1 addition & 1 deletion checker/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

const (
numOfChecks = 91
numOfIds = 262
numOfIds = 264
)

func TestNewConfig(t *testing.T) {
Expand Down
8 changes: 6 additions & 2 deletions checker/localizations/localizations.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions checker/localizations_src/en/messages.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion checker/localizations_src/ru/messages.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 удален
Expand Down
2 changes: 2 additions & 0 deletions checker/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
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
Loading
Loading