feat: add check for api removed with deprecation (#621)

* add check for api removed with deprecation

* add test

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
+}

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) {
 

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)
 

checker/config_test.go

@@ -9,7 +9,7 @@ import (
 
 const (
 	numOfChecks = 91
-	numOfIds    = 262
+	numOfIds    = 264
 )
 
 func TestNewConfig(t *testing.T) {

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

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 удален

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),

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