From b3677f4a82399c8cd423e32e492c81718816557f Mon Sep 17 00:00:00 2001 From: Jonathan Rosenberg <96974219+Jonathan-Rosenberg@users.noreply.github.com> Date: Sat, 16 Sep 2023 12:08:55 +0300 Subject: [PATCH] Breaking change check: New request required default param on existing path (#376) --- BREAKING-CHANGES-EXAMPLES.md | 1 + ...-new-request-non-path-default-parameter.go | 49 ++++++++ ...request-non-path-default-parameter_test.go | 117 ++++++++++++++++++ checker/default_checks.go | 1 + checker/localizations/localizations.go | 6 +- checker/localizations_src/en/messages.yaml | 2 + checker/localizations_src/ru/messages.yaml | 2 + data/request_params/base.yaml | 9 ++ .../required-request-params.yaml | 81 ++++++++++++ 9 files changed, 267 insertions(+), 1 deletion(-) create mode 100644 checker/check-new-request-non-path-default-parameter.go create mode 100644 checker/check-new-request-non-path-default-parameter_test.go create mode 100644 data/request_params/required-request-params.yaml diff --git a/BREAKING-CHANGES-EXAMPLES.md b/BREAKING-CHANGES-EXAMPLES.md index 7aff36e5..fbe9f0c0 100644 --- a/BREAKING-CHANGES-EXAMPLES.md +++ b/BREAKING-CHANGES-EXAMPLES.md @@ -57,6 +57,7 @@ These examples are automatically generated from unit tests. [modifying a pattern in a schema is breaking](checker/checker_breaking_test.go?plain=1#L491) [modifying a pattern in request parameter is breaking](checker/checker_breaking_test.go?plain=1#L507) [modifying the default value of an optional request parameter is breaking](checker/checker_breaking_test.go?plain=1#L537) +[new header, query and cookie required request default param is breaking](checker/check-new-request-non-path-default-parameter_test.go?plain=1#L11) [new required header param is breaking](checker/checker_breaking_test.go?plain=1#L171) [new required path param is breaking](checker/checker_breaking_test.go?plain=1#L155) [new required property in request header is breaking](checker/checker_breaking_property_test.go?plain=1#L17) diff --git a/checker/check-new-request-non-path-default-parameter.go b/checker/check-new-request-non-path-default-parameter.go new file mode 100644 index 00000000..e81ad49e --- /dev/null +++ b/checker/check-new-request-non-path-default-parameter.go @@ -0,0 +1,49 @@ +package checker + +import ( + "github.com/tufin/oasdiff/diff" +) + +func NewRequestNonPathDefaultParameterCheck(diffReport *diff.Diff, operationsSources *diff.OperationsSourcesMap, config Config) Changes { + result := make(Changes, 0) + if diffReport.PathsDiff == nil || len(diffReport.PathsDiff.Modified) == 0 { + return result + } + for path, pathItem := range diffReport.PathsDiff.Modified { + if pathItem.ParametersDiff == nil || pathItem.Revision == nil || len(pathItem.Revision.Operations()) == 0 { + continue + } + + for paramLoc := range pathItem.ParametersDiff.Added { + if paramLoc == "path" { + continue + } + + for _, param := range pathItem.Revision.Parameters { + + id := "new-required-request-default-parameter-to-existing-path" + level := ERR + if !param.Value.Required { + id = "new-optional-request-default-parameter-to-existing-path" + level = INFO + } + + for operation, operationItem := range pathItem.Revision.Operations() { + + // TODO: if base operation had this param individually (not through the path) - continue + + result = append(result, ApiChange{ + Id: id, + Level: level, + Text: config.Localize(id, ColorizedValue(paramLoc), ColorizedValue(param.Value.Name)), + Operation: operation, + OperationId: operationItem.OperationID, + Path: path, + Source: (*operationsSources)[operationItem], + }) + } + } + } + } + return result +} diff --git a/checker/check-new-request-non-path-default-parameter_test.go b/checker/check-new-request-non-path-default-parameter_test.go new file mode 100644 index 00000000..b1f71088 --- /dev/null +++ b/checker/check-new-request-non-path-default-parameter_test.go @@ -0,0 +1,117 @@ +package checker_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/tufin/oasdiff/checker" + "github.com/tufin/oasdiff/diff" +) + +// BC: new header, query and cookie required request default param is breaking +func TestNewRequestNonPathParameter_DetectsNewRequiredPathsAndNewOperations(t *testing.T) { + s1, err := open("../data/request_params/base.yaml") + require.NoError(t, err) + + s2, err := open("../data/request_params/required-request-params.yaml") + require.NoError(t, err) + + d, osm, err := diff.GetWithOperationsSourcesMap(&diff.Config{}, s1, s2) + require.NoError(t, err) + + errs := checker.CheckBackwardCompatibilityUntilLevel(singleCheckConfig(checker.NewRequestNonPathDefaultParameterCheck), d, osm, checker.INFO) + require.NotEmpty(t, errs) + require.Len(t, errs, 9) + + require.ElementsMatch(t, []checker.ApiChange{ + { + Id: "new-required-request-default-parameter-to-existing-path", + Text: "added the new required 'query' request parameter 'version' to all path's operations", + Comment: "", + Level: 3, + Operation: "GET", + OperationId: "getTest", + Path: "/api/test1", + Source: "../data/request_params/required-request-params.yaml", + }, + { + Id: "new-required-request-default-parameter-to-existing-path", + Text: "added the new required 'query' request parameter 'version' to all path's operations", + Comment: "", + Level: 3, + Operation: "POST", + OperationId: "", + Path: "/api/test1", + Source: "../data/request_params/required-request-params.yaml", + }, + { + Id: "new-required-request-default-parameter-to-existing-path", + Text: "added the new required 'header' request parameter 'id' to all path's operations", + Comment: "", + Level: 3, + Operation: "GET", + OperationId: "getTest", + Path: "/api/test2", + Source: "../data/request_params/required-request-params.yaml", + }, + { + Id: "new-required-request-default-parameter-to-existing-path", + Text: "added the new required 'query' request parameter 'id' to all path's operations", + Comment: "", + Level: 3, + Operation: "GET", + OperationId: "getTest", + Path: "/api/test2", + Source: "../data/request_params/required-request-params.yaml", + }, + { + Id: "new-required-request-default-parameter-to-existing-path", + Text: "added the new required 'header' request parameter 'If-None-Match' to all path's operations", + Comment: "", + Level: 3, + Operation: "GET", + OperationId: "getTest", + Path: "/api/test3", + Source: "../data/request_params/required-request-params.yaml", + }, + { + Id: "new-optional-request-default-parameter-to-existing-path", + Text: "added the new optional 'query' request parameter 'optionalQueryParam' to all path's operations", + Comment: "", + Level: 1, + Operation: "GET", + OperationId: "getTest", + Path: "/api/test1", + Source: "../data/request_params/required-request-params.yaml", + }, + { + Id: "new-optional-request-default-parameter-to-existing-path", + Text: "added the new optional 'query' request parameter 'optionalQueryParam' to all path's operations", + Comment: "", + Level: 1, + Operation: "POST", + OperationId: "", + Path: "/api/test1", + Source: "../data/request_params/required-request-params.yaml", + }, + { + Id: "new-optional-request-default-parameter-to-existing-path", + Text: "added the new optional 'header' request parameter 'optionalHeaderParam' to all path's operations", + Comment: "", + Level: 1, + Operation: "GET", + OperationId: "getTest", + Path: "/api/test2", + Source: "../data/request_params/required-request-params.yaml", + }, + { + Id: "new-optional-request-default-parameter-to-existing-path", + Text: "added the new optional 'query' request parameter 'optionalHeaderParam' to all path's operations", + Comment: "", + Level: 1, + Operation: "GET", + OperationId: "getTest", + Path: "/api/test2", + Source: "../data/request_params/required-request-params.yaml", + }}, errs) +} diff --git a/checker/default_checks.go b/checker/default_checks.go index cd360923..3fb0a2c3 100644 --- a/checker/default_checks.go +++ b/checker/default_checks.go @@ -67,6 +67,7 @@ func defaultChecks() []BackwardCompatibilityCheck { APISecurityUpdatedCheck, APISunsetChangedCheck, AddedRequiredRequestBodyCheck, + NewRequestNonPathDefaultParameterCheck, NewRequestNonPathParameterCheck, NewRequestPathParameterCheck, NewRequiredRequestHeaderPropertyCheck, diff --git a/checker/localizations/localizations.go b/checker/localizations/localizations.go index d9c16b87..9d00fd47 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 -// 2023-08-04 10:55:40.9774 +0100 IST m=+0.005745834 +// 2023-09-11 16:39:50.19002 +0300 IDT m=+0.007249459 package localizations @@ -46,9 +46,11 @@ var localizations = map[string]string{ "en.messages.endpoint-deprecated": "endpoint deprecated", "en.messages.endpoint-reactivated": "endpoint reactivated", "en.messages.in": "in", + "en.messages.new-optional-request-default-parameter-to-existing-path": "added the new optional %s request parameter %s to all path's operations", "en.messages.new-optional-request-parameter": "added the new optional %s request parameter %s", "en.messages.new-optional-request-property": "added the new optional request property %s", "en.messages.new-request-path-parameter": "added the new path request parameter %s", + "en.messages.new-required-request-default-parameter-to-existing-path": "added the new required %s request parameter %s to all path's operations", "en.messages.new-required-request-header-property": "added the new required %s request header's property %s", "en.messages.new-required-request-parameter": "added the new required %s request parameter %s", "en.messages.new-required-request-property": "added the new required request property %s", @@ -310,9 +312,11 @@ var localizations = map[string]string{ "ru.messages.api-tag-removed": "Тег API %s удален", "ru.messages.at": "в", "ru.messages.in": "в", + "ru.messages.new-optional-request-default-parameter-to-existing-path": "добавлен новый необязательный %s параметр запроса %s ко всем операциям пути", "ru.messages.new-optional-request-parameter": "добавлен новый необязательный %s параметр зароса %s", "ru.messages.new-optional-request-property": "добавлено новое необязательное поле запроса %s", "ru.messages.new-request-path-parameter": "добален новый path параметр запроса %s", + "ru.messages.new-required-request-default-parameter-to-existing-path": "добавлен новый обязательный %s параметр запроса %s для всех операций пути", "ru.messages.new-required-request-header-property": "в заголовке запроса %s добавлено новое обязательное поле %s", "ru.messages.new-required-request-parameter": "добавлен новый обязательный %s параметр зароса %s", "ru.messages.new-required-request-property": "добавлено новое обязательное поле запроса %s", diff --git a/checker/localizations_src/en/messages.yaml b/checker/localizations_src/en/messages.yaml index d214b45f..39440083 100644 --- a/checker/localizations_src/en/messages.yaml +++ b/checker/localizations_src/en/messages.yaml @@ -262,3 +262,5 @@ request-optional-property-became-not-write-only: the request optional property % request-optional-property-became-not-read-only: the request optional property %s became not read-only request-required-property-became-write-only: the request required property %s became write-only request-required-property-became-not-write-only: the request required property %s became not write-only +new-required-request-default-parameter-to-existing-path: added the new required %s request parameter %s to all path's operations +new-optional-request-default-parameter-to-existing-path: added the new optional %s request parameter %s to all path's operations diff --git a/checker/localizations_src/ru/messages.yaml b/checker/localizations_src/ru/messages.yaml index bb2cf59f..ca8edb41 100644 --- a/checker/localizations_src/ru/messages.yaml +++ b/checker/localizations_src/ru/messages.yaml @@ -262,3 +262,5 @@ request-optional-property-became-not-write-only: необязательное п request-optional-property-became-not-read-only: необязательное поле запроса %s перестало быть только для чтения request-required-property-became-write-only: обязательное поле запроса %s стало только для записи request-required-property-became-not-write-only: обязательное поле запроса %s перестало быть только для записи +new-required-request-default-parameter-to-existing-path: добавлен новый обязательный %s параметр запроса %s для всех операций пути +new-optional-request-default-parameter-to-existing-path: добавлен новый необязательный %s параметр запроса %s ко всем операциям пути diff --git a/data/request_params/base.yaml b/data/request_params/base.yaml index 09cc74e6..1ae909f4 100644 --- a/data/request_params/base.yaml +++ b/data/request_params/base.yaml @@ -27,3 +27,12 @@ paths: responses: 200: description: OK + + /api/test4: + parameters: + - in: query + name: emptyPath + required: false + schema: + type: string + diff --git a/data/request_params/required-request-params.yaml b/data/request_params/required-request-params.yaml new file mode 100644 index 00000000..1439c12e --- /dev/null +++ b/data/request_params/required-request-params.yaml @@ -0,0 +1,81 @@ +info: + title: Tufin + version: 1.0.0 +openapi: 3.0.3 +paths: + /api/test1: + parameters: + - in: query + name: version + required: true + schema: + type: string + - in: query + name: optionalQueryParam + required: false + schema: + type: string + get: + operationId: getTest + parameters: + - in: header + name: X-NewRequestHeaderParam + required: false + schema: + type: string + responses: + 200: + description: OK + post: + responses: + 201: + description: OK + + /api/test2: + parameters: + - in: query + name: id + required: true + schema: + type: string + - in: header + name: optionalHeaderParam + required: false + schema: + type: string + get: + operationId: getTest + parameters: + - in: query + name: newQueryParam + required: false + schema: + type: string + responses: + 200: + description: OK + + /api/test3: + parameters: + - in: header + name: If-None-Match + required: true + get: + operationId: getTest + parameters: + - in: cookie + name: csrf-token + required: false + schema: + type: string + responses: + 200: + description: OK + + /api/test4: + parameters: + - in: query + name: emptyPath2 + required: true + schema: + type: string