From 87f156be7b1979c95a1ec77c555f155310f1135d Mon Sep 17 00:00:00 2001 From: Ciprian Tibulca Date: Tue, 13 Jun 2023 20:39:23 +0100 Subject: [PATCH] changelog: cover not required request params (#296) * changelog: cover optional non-path request params * git --------- Co-authored-by: Reuven --- BREAKING-CHANGES-EXAMPLES.md | 1 + ...> check-new-request-non-path-parameter.go} | 29 +++++++----- ...eck-new-request-non-path-parameter_test.go | 43 +++++++++++++++++ checker/default_checks.go | 2 +- checker/localizations/localizations.go | 4 +- checker/localizations_src/en/messages.yaml | 1 + checker/localizations_src/ru/messages.yaml | 1 + data/request_params/base.yaml | 29 ++++++++++++ .../optional-request-params.yaml | 47 +++++++++++++++++++ 9 files changed, 143 insertions(+), 14 deletions(-) rename checker/{check-new-request-required-non-path-parameter.go => check-new-request-non-path-parameter.go} (52%) create mode 100644 checker/check-new-request-non-path-parameter_test.go create mode 100644 data/request_params/base.yaml create mode 100644 data/request_params/optional-request-params.yaml diff --git a/BREAKING-CHANGES-EXAMPLES.md b/BREAKING-CHANGES-EXAMPLES.md index 51db1fa9..3c015d9e 100644 --- a/BREAKING-CHANGES-EXAMPLES.md +++ b/BREAKING-CHANGES-EXAMPLES.md @@ -138,6 +138,7 @@ These examples are automatically generated from unit tests. [changing an existing header param from required to optional](checker/checker_request_parameter_required_value_updated_test.go?plain=1#L36) [changing an existing header param to optional](checker/checker_not_breaking_test.go?plain=1#L116) [deprecating an operation with sunset greater than min](checker/checker_not_breaking_test.go?plain=1#L182) +[new header, query and cookie request params](checker/check-new-request-non-path-parameter_test.go?plain=1#L11) [new paths or path operations](checker/check-api-added_test.go?plain=1#L11) [path operations that became deprecated](checker/checker_deprecation_test.go?plain=1#L324) [path operations that were re-activated](checker/checker_deprecation_test.go?plain=1#L344) diff --git a/checker/check-new-request-required-non-path-parameter.go b/checker/check-new-request-non-path-parameter.go similarity index 52% rename from checker/check-new-request-required-non-path-parameter.go rename to checker/check-new-request-non-path-parameter.go index 061af827..4d8b2e0f 100644 --- a/checker/check-new-request-required-non-path-parameter.go +++ b/checker/check-new-request-non-path-parameter.go @@ -6,7 +6,7 @@ import ( "github.com/tufin/oasdiff/diff" ) -func NewRequiredRequestNonPathParameterCheck(diffReport *diff.Diff, operationsSources *diff.OperationsSourcesMap, config BackwardCompatibilityCheckConfig) []BackwardCompatibilityError { +func NewRequestNonPathParameterCheck(diffReport *diff.Diff, operationsSources *diff.OperationsSourcesMap, config BackwardCompatibilityCheckConfig) []BackwardCompatibilityError { result := make([]BackwardCompatibilityError, 0) if diffReport.PathsDiff == nil { return result @@ -28,18 +28,23 @@ func NewRequiredRequestNonPathParameterCheck(diffReport *diff.Diff, operationsSo for _, paramName := range paramItems { for _, param := range operationItem.Revision.Parameters { if param.Value.Name == paramName { - if param.Value.Required { - source := (*operationsSources)[operationItem.Revision] - result = append(result, BackwardCompatibilityError{ - Id: "new-required-request-parameter", - Level: ERR, - Text: fmt.Sprintf(config.i18n("new-required-request-parameter"), ColorizedValue(paramLocation), ColorizedValue(paramName)), - Operation: operation, - OperationId: operationItem.Revision.OperationID, - Path: path, - Source: source, - }) + id := "new-required-request-parameter" + level := ERR + if !param.Value.Required { + id = "new-optional-request-parameter" + level = INFO } + source := (*operationsSources)[operationItem.Revision] + result = append(result, BackwardCompatibilityError{ + Id: id, + Level: level, + Text: fmt.Sprintf(config.i18n(id), ColorizedValue(paramLocation), ColorizedValue(paramName)), + Operation: operation, + OperationId: operationItem.Revision.OperationID, + Path: path, + Source: source, + }) + break } } diff --git a/checker/check-new-request-non-path-parameter_test.go b/checker/check-new-request-non-path-parameter_test.go new file mode 100644 index 00000000..475c020c --- /dev/null +++ b/checker/check-new-request-non-path-parameter_test.go @@ -0,0 +1,43 @@ +package checker_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/tufin/oasdiff/checker" + "github.com/tufin/oasdiff/diff" +) + +// CL: new header, query and cookie request params +func TestNewRequestNonPathParameter_DetectsNewPathsAndNewOperations(t *testing.T) { + s1, err := open("../data/request_params/base.yaml") + require.NoError(t, err) + + s2, err := open("../data/request_params/optional-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.NewRequestNonPathParameterCheck), d, osm, checker.INFO) + require.NotEmpty(t, errs) + require.Len(t, errs, 3) + + require.Equal(t, "new-optional-request-parameter", errs[0].Id) + require.Equal(t, "GET", errs[0].Operation) + require.Equal(t, "/api/test1", errs[0].Path) + require.Equal(t, checker.INFO, errs[0].Level) + require.Contains(t, errs[0].Text, "X-NewRequestHeaderParam") + + require.Equal(t, "new-optional-request-parameter", errs[1].Id) + require.Equal(t, "GET", errs[1].Operation) + require.Equal(t, "/api/test2", errs[1].Path) + require.Equal(t, checker.INFO, errs[1].Level) + require.Contains(t, errs[1].Text, "newQueryParam") + + require.Equal(t, "new-optional-request-parameter", errs[2].Id) + require.Equal(t, "GET", errs[2].Operation) + require.Equal(t, "/api/test3", errs[2].Path) + require.Equal(t, checker.INFO, errs[2].Level) + require.Contains(t, errs[2].Text, "csrf-token") +} diff --git a/checker/default_checks.go b/checker/default_checks.go index a35fdc29..ff004f72 100644 --- a/checker/default_checks.go +++ b/checker/default_checks.go @@ -82,7 +82,7 @@ func defaultChecks() []BackwardCompatibilityCheck { ResponseSuccessStatusRemoved, ResponseMediaTypeRemoved, NewRequestPathParameterCheck, - NewRequiredRequestNonPathParameterCheck, + NewRequestNonPathParameterCheck, NewRequiredRequestHeaderPropertyCheck, ResponseRequiredPropertyRemovedCheck, UncheckedRequestAllOfWarnCheck, diff --git a/checker/localizations/localizations.go b/checker/localizations/localizations.go index c2113dac..9f7a1479 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-06-08 10:29:39.819262 +0100 IST m=+0.001862709 +// 2023-06-12 15:35:28.982549 +0100 IST m=+0.003894001 package localizations @@ -28,6 +28,7 @@ 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-parameter": "added the new optional %s request parameter %s", "en.messages.new-request-path-parameter": "added the new path request parameter %s", "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", @@ -148,6 +149,7 @@ var localizations = map[string]string{ "ru.messages.api-tag-removed": "Тег API %s удален", "ru.messages.at": "в", "ru.messages.in": "в", + "ru.messages.new-optional-request-parameter": "добавлен новый необязательный %s параметр зароса %s", "ru.messages.new-request-path-parameter": "добален новый path параметр запроса %s", "ru.messages.new-required-request-header-property": "в заголовке запроса %s добавлено новое обязательное поле %s", "ru.messages.new-required-request-parameter": "добавлен новый обязательный %s параметр зароса %s", diff --git a/checker/localizations_src/en/messages.yaml b/checker/localizations_src/en/messages.yaml index bbbd4101..9e2a9908 100644 --- a/checker/localizations_src/en/messages.yaml +++ b/checker/localizations_src/en/messages.yaml @@ -22,6 +22,7 @@ api-schema-removed: removed the schema %s from openapi components sunset-deleted: api sunset date deleted, but deprecated=true kept api-sunset-date-changed-too-small: api sunset date changed to earlier date from %s to %s, new sunset date must be not earlier than %s at least %d days from now new-required-request-parameter: added the new required %s request parameter %s +new-optional-request-parameter: added the new optional %s request parameter %s new-required-request-property: added the new required request property %s new-required-request-header-property: added the new required %s request header's property %s request-body-became-required: request body became required diff --git a/checker/localizations_src/ru/messages.yaml b/checker/localizations_src/ru/messages.yaml index cb2f11a6..ae90efe2 100644 --- a/checker/localizations_src/ru/messages.yaml +++ b/checker/localizations_src/ru/messages.yaml @@ -22,6 +22,7 @@ api-schema-removed: удалена схема %s из компонентов ope sunset-deleted: удалена дата sunset date у API, но сохранён deprecated=true api-sunset-date-changed-too-small: дата sunset у API изменена на более раннюю с %s на %s, новая дата sunset должна быть либо не раньше %s, либо, как минимум, %d дней от текущего дня new-required-request-parameter: добавлен новый обязательный %s параметр зароса %s +new-optional-request-parameter: добавлен новый необязательный %s параметр зароса %s new-required-request-property: добавлено новле обязательное поле запроса %s new-required-request-header-property: в заголовке запроса %s добавлено новое обязательное поле %s request-body-became-required: тело запроса стало обязательным diff --git a/data/request_params/base.yaml b/data/request_params/base.yaml new file mode 100644 index 00000000..09cc74e6 --- /dev/null +++ b/data/request_params/base.yaml @@ -0,0 +1,29 @@ +info: + title: Tufin + version: 1.0.0 +openapi: 3.0.3 +paths: + /api/test1: + get: + operationId: getTest + responses: + 200: + description: OK + post: + responses: + 201: + description: OK + + /api/test2: + get: + operationId: getTest + responses: + 200: + description: OK + + /api/test3: + get: + operationId: getTest + responses: + 200: + description: OK diff --git a/data/request_params/optional-request-params.yaml b/data/request_params/optional-request-params.yaml new file mode 100644 index 00000000..27a2abc4 --- /dev/null +++ b/data/request_params/optional-request-params.yaml @@ -0,0 +1,47 @@ +info: + title: Tufin + version: 1.0.0 +openapi: 3.0.3 +paths: + /api/test1: + 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: + get: + operationId: getTest + parameters: + - in: query + name: newQueryParam + required: false + schema: + type: string + responses: + 200: + description: OK + + /api/test3: + get: + operationId: getTest + parameters: + - in: cookie + name: csrf-token + required: false + schema: + type: string + responses: + 200: + description: OK