Skip to content

Commit

Permalink
627 support x-sunset and deprecation at parameter level (#643)
Browse files Browse the repository at this point in the history 
* use getDeprecationFile throughout tests

* param deprecation

* support simple deprecation

* fix build

* use allChecksConfig to check API removal

* update check counts

* move api removal tests to relevant file

* separate removal vs deprecation tests

* more checkers for param deprecation

* helper -> opInfo

* use opInfo

* finished deprecation in param removal

* parameter -> request-parameter

* handle param deprecation parsing

* enable more unit tests

* delete unneeded test

* test breaking-changes for common-params

* move test files

* completed RequestParameterSunsetChangedCheck

* more unit tests

* update docs

* update doc

* minor adjusments
  • Loading branch information
@reuvenharrison
reuvenharrison authored Feb 5, 2025
1 parent cd53a4d commit 5a2b098
Show file tree
Hide file tree
Showing 40 changed files with 1,573 additions and 254 deletions.
2 changes: 2 additions & 0 deletions checker/api_change.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ type ApiChange struct {
SourceColumnEnd int
}

// NewApiChange creates a new ApiChange
// TODO: use opInfo to simplify the function signature
func NewApiChange(id string, config *Config, args []any, comment string, operationsSources *diff.OperationsSourcesMap, operation *openapi3.Operation, method, path string) ApiChange {
return ApiChange{
Id: id,
Expand Down
2 changes: 1 addition & 1 deletion checker/check_api_deprecation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func APIDeprecationCheck(diffReport *diff.Diff, operationsSources *diff.Operatio
if !ok {
// if deprecation policy is defined and sunset is missing, it's a breaking change
if deprecationDays > 0 {
result = append(result, getAPIDeprecatedSunsetMissing(config, op, operationsSources, path, operation))
result = append(result, getAPIDeprecatedSunsetMissing(newOpInfo(config, op, operationsSources, operation, path)))
}
continue
}
Expand Down
79 changes: 6 additions & 73 deletions checker/check_api_deprecation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,6 @@ func allChecksConfig() *checker.Config {
return checker.NewConfig(checker.GetAllChecks())
}

// BC: deleting an operation after sunset date is not breaking
func TestBreaking_DeprecationPast(t *testing.T) {

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

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

d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibility(singleCheckConfig(checker.APIDeprecationCheck), d, osm)
require.Empty(t, errs)
}

// BC: deprecating an operation with a deprecation policy and an invalid sunset date is breaking
func TestBreaking_DeprecationWithInvalidSunset(t *testing.T) {

Expand Down Expand Up @@ -115,7 +100,7 @@ func TestBreaking_DeprecationWithoutSunsetWithPolicy(t *testing.T) {
errs := checker.CheckBackwardCompatibility(c, d, osm)
require.Len(t, errs, 1)
require.Equal(t, checker.APIDeprecatedSunsetMissingId, errs[0].GetId())
require.Equal(t, "sunset date is missing for deprecated API", errs[0].GetText(checker.NewDefaultLocalizer()))
require.Equal(t, "sunset date is missing for deprecated API", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer()))
}

// BC: deprecating an operation with a default deprecation policy but without specifying sunset date is not breaking
Expand Down Expand Up @@ -149,25 +134,6 @@ func TestBreaking_DeprecationForAlpha(t *testing.T) {
require.Empty(t, errs)
}

// BC: removing the path without a deprecation policy and without specifying sunset date is not breaking for alpha level
func TestBreaking_RemovedPathForAlpha(t *testing.T) {
s1, err := open(getDeprecationFile("base-alpha-stability.yaml"))
require.NoError(t, err)
alpha := toJson(t, checker.STABILITY_ALPHA)
s1.Spec.Paths.Value("/api/test").Get.Extensions["x-stability-level"] = alpha
s1.Spec.Paths.Value("/api/test").Post.Extensions = map[string]interface{}{"x-stability-level": alpha}

s2, err := open(getDeprecationFile("base-alpha-stability.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.APIDeprecationCheck), d, osm)
require.Empty(t, errs)
}

// BC: deprecating an operation without a deprecation policy and without specifying sunset date is not breaking for draft level
func TestBreaking_DeprecationForDraft(t *testing.T) {
s1, err := open(getDeprecationFile("base-alpha-stability.yaml"))
Expand All @@ -185,25 +151,6 @@ func TestBreaking_DeprecationForDraft(t *testing.T) {
require.Empty(t, errs)
}

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

s2, err := open(getDeprecationFile("base-alpha-stability.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.APIDeprecationCheck), d, osm)
require.Empty(t, errs)
}

func toJson(t *testing.T, value string) json.RawMessage {
t.Helper()
data, err := json.Marshal(value)
Expand Down Expand Up @@ -248,31 +195,17 @@ func TestBreaking_DeprecationWithProperSunset(t *testing.T) {
errs := checker.CheckBackwardCompatibilityUntilLevel(c, d, osm, checker.INFO)
require.Len(t, errs, 1)
// only a non-breaking change detected
require.Equal(t, checker.EndpointDeprecatedId, errs[0].GetId())
require.Equal(t, checker.INFO, errs[0].GetLevel())
require.Equal(t, "endpoint deprecated", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer()))
}

// BC: deleting a path after sunset date of all contained operations is not breaking
func TestBreaking_DeprecationPathPast(t *testing.T) {

s1, err := open(getDeprecationFile("deprecated-path-past.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)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibility(singleCheckConfig(checker.APIDeprecationCheck), d, osm)
require.Empty(t, errs)
}

// CL: path operations that became deprecated
func TestApiDeprecated_DetectsDeprecatedOperations(t *testing.T) {
s1, err := open("../data/deprecation/base.yaml")
s1, err := open(getDeprecationFile("base.yaml"))
require.NoError(t, err)

s2, err := open("../data/deprecation/deprecated-future.yaml")
s2, err := open(getDeprecationFile("deprecated-future.yaml"))
require.NoError(t, err)

d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
Expand All @@ -292,10 +225,10 @@ func TestApiDeprecated_DetectsDeprecatedOperations(t *testing.T) {

// CL: path operations that were re-activated
func TestApiDeprecated_DetectsReactivatedOperations(t *testing.T) {
s1, err := open("../data/deprecation/deprecated-future.yaml")
s1, err := open(getDeprecationFile("deprecated-future.yaml"))
require.NoError(t, err)

s2, err := open("../data/deprecation/base.yaml")
s2, err := open(getDeprecationFile("base.yaml"))
require.NoError(t, err)

d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
Expand Down
58 changes: 29 additions & 29 deletions checker/check_api_removed.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"time"

"cloud.google.com/go/civil"
"github.com/getkin/kin-openapi/openapi3"
"github.com/tufin/oasdiff/diff"
)

Expand Down Expand Up @@ -44,7 +43,8 @@ func checkRemovedPaths(pathsDiff *diff.PathsDiff, operationsSources *diff.Operat
continue
}

if change := checkAPIRemoval(config, true, op, operationsSources, operation, path); change != nil {
opInfo := newOpInfo(config, op, operationsSources, operation, path)
if change := checkAPIRemoval(opInfo, true); change != nil {
result = append(result, change)
}
}
Expand All @@ -64,8 +64,8 @@ func checkRemovedOperations(pathsDiff *diff.PathsDiff, operationsSources *diff.O
continue
}
for _, operation := range pathItem.OperationsDiff.Deleted {
op := pathItem.Base.GetOperation(operation)
if change := checkAPIRemoval(config, false, op, operationsSources, operation, path); change != nil {
opInfo := newOpInfo(config, pathItem.Base.GetOperation(operation), operationsSources, operation, path)
if change := checkAPIRemoval(opInfo, false); change != nil {
result = append(result, change)
}
}
Expand All @@ -74,63 +74,63 @@ func checkRemovedOperations(pathsDiff *diff.PathsDiff, operationsSources *diff.O
return result
}

func checkAPIRemoval(config *Config, isPath bool, op *openapi3.Operation, operationsSources *diff.OperationsSourcesMap, method, path string) Change {
if !op.Deprecated {
func checkAPIRemoval(opInfo opInfo, isPath bool) Change {
if !opInfo.operation.Deprecated {
return NewApiChange(
getWithoutDeprecationId(isPath),
config,
opInfo.config,
nil,
"",
operationsSources,
op,
method,
path,
opInfo.operationsSources,
opInfo.operation,
opInfo.method,
opInfo.path,
)
}
sunset, ok := getSunset(op.Extensions)
sunset, ok := getSunset(opInfo.operation.Extensions)
if !ok {
return NewApiChange(
getWithDeprecationId(isPath),
config,
opInfo.config,
nil,
"",
operationsSources,
op,
method,
path,
opInfo.operationsSources,
opInfo.operation,
opInfo.method,
opInfo.path,
)
}

date, err := getSunsetDate(sunset)
if err != nil {
return getAPIPathSunsetParse(config, op, operationsSources, method, path, err)
return getAPIPathSunsetParse(opInfo, err)
}

if civil.DateOf(time.Now()).Before(date) {
return NewApiChange(
getBeforeSunsetId(isPath),
config,
opInfo.config,
[]any{date},
"",
operationsSources,
op,
method,
path,
opInfo.operationsSources,
opInfo.operation,
opInfo.method,
opInfo.path,
)
}
return nil
}

func getAPIPathSunsetParse(config *Config, operation *openapi3.Operation, operationsSources *diff.OperationsSourcesMap, method string, path string, err error) Change {
func getAPIPathSunsetParse(opInfo opInfo, err error) Change {
return NewApiChange(
APIPathSunsetParseId,
config,
opInfo.config,
[]any{err},
"",
operationsSources,
operation,
method,
path,
opInfo.operationsSources,
opInfo.operation,
opInfo.method,
opInfo.path,
)
}

Expand Down
93 changes: 93 additions & 0 deletions checker/check_api_removed_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,51 @@ package checker_test
import (
"testing"

"github.com/getkin/kin-openapi/openapi3"
"github.com/stretchr/testify/require"
"github.com/tufin/oasdiff/checker"
"github.com/tufin/oasdiff/diff"
)

// BC: deleting a path without deprecation is breaking
func TestBreaking_DeletedPath(t *testing.T) {
errs := d(t, diff.NewConfig(), 1, 701)
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()))
}

// BC: deleting an operation without deprecation is breaking
func TestBreaking_DeletedOp(t *testing.T) {
s1 := l(t, 1)
s2 := l(t, 1)

s1.Spec.Paths.Value(installCommandPath).Put = openapi3.NewOperation()

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

// BC: deleting an operation after sunset date is not breaking
func TestBreaking_DeprecationPast(t *testing.T) {

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

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

d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibility(singleCheckConfig(checker.APIRemovedCheck), d, osm)
require.Empty(t, errs)
}

// BC: deleting an operation before sunset date is breaking
func TestBreaking_RemoveBeforeSunset(t *testing.T) {

Expand Down Expand Up @@ -44,6 +84,44 @@ 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 not breaking for alpha level
func TestBreaking_RemovedPathForAlpha(t *testing.T) {
s1, err := open(getDeprecationFile("base-alpha-stability.yaml"))
require.NoError(t, err)
alpha := toJson(t, checker.STABILITY_ALPHA)
s1.Spec.Paths.Value("/api/test").Get.Extensions["x-stability-level"] = alpha
s1.Spec.Paths.Value("/api/test").Post.Extensions = map[string]interface{}{"x-stability-level": alpha}

s2, err := open(getDeprecationFile("base-alpha-stability.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.Empty(t, errs)
}

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

s2, err := open(getDeprecationFile("base-alpha-stability.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.Empty(t, errs)
}

// 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"))
Expand Down Expand Up @@ -76,6 +154,21 @@ func TestBreaking_RemovedPathForDraftBreaking(t *testing.T) {
require.Equal(t, "api path removed without deprecation", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer()))
}

// BC: deleting a path after sunset date of all contained operations is not breaking
func TestBreaking_DeprecationPathPast(t *testing.T) {

s1, err := open(getDeprecationFile("deprecated-path-past.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)
require.NoError(t, err)
errs := checker.CheckBackwardCompatibility(singleCheckConfig(checker.APIRemovedCheck), d, osm)
require.Empty(t, errs)
}

// BC: deleting a path with some operations having sunset date in the future is breaking
func TestBreaking_DeprecationPathMixed(t *testing.T) {

Expand Down
Loading

0 comments on commit 5a2b098

Please sign in to comment.