From 5dea2e29a61a4d84ae57a107a821bfcdb1d24c72 Mon Sep 17 00:00:00 2001 From: Reuven Date: Tue, 30 Jan 2024 22:59:21 +0200 Subject: [PATCH 01/21] inherit params from path to operation --- checker/checker_not_breaking_test.go | 6 ++- data/param-inheritance/path_base.yaml | 56 +++++++++++++++++++++++ data/param-inheritance/path_revision.yaml | 53 +++++++++++++++++++++ diff/diff_test.go | 20 ++++++++ diff/method_diff.go | 10 ++-- diff/operations_diff.go | 8 ++-- diff/params_inherit.go | 19 ++++++++ report/example_test.go | 2 + 8 files changed, 165 insertions(+), 9 deletions(-) create mode 100644 data/param-inheritance/path_base.yaml create mode 100644 data/param-inheritance/path_revision.yaml create mode 100644 diff/params_inherit.go diff --git a/checker/checker_not_breaking_test.go b/checker/checker_not_breaking_test.go index c1876abe..840f68b7 100644 --- a/checker/checker_not_breaking_test.go +++ b/checker/checker_not_breaking_test.go @@ -167,19 +167,21 @@ func TestBreaking_NewRequiredResponseHeader(t *testing.T) { // BC: changing operation ID is not breaking func TestBreaking_OperationID(t *testing.T) { r := d(t, getConfig(), 3, 1) - require.Len(t, r, 3) + require.Len(t, r, 4) require.Equal(t, checker.RequestParameterMaxLengthDecreasedId, r[0].GetId()) require.Equal(t, checker.RequestParameterEnumValueRemovedId, r[1].GetId()) require.Equal(t, checker.RequestParameterPatternAddedId, r[2].GetId()) + require.Equal(t, checker.RequestParameterRemovedId, r[3].GetId()) } // BC: changing a link to operation ID is not breaking func TestBreaking_LinkOperationID(t *testing.T) { r := d(t, getConfig(), 3, 1) - require.Len(t, r, 3) + require.Len(t, r, 4) require.Equal(t, checker.RequestParameterMaxLengthDecreasedId, r[0].GetId()) require.Equal(t, checker.RequestParameterEnumValueRemovedId, r[1].GetId()) require.Equal(t, checker.RequestParameterPatternAddedId, r[2].GetId()) + require.Equal(t, checker.RequestParameterRemovedId, r[3].GetId()) } // BC: adding a media-type to response is not breaking diff --git a/data/param-inheritance/path_base.yaml b/data/param-inheritance/path_base.yaml new file mode 100644 index 00000000..2fd4779a --- /dev/null +++ b/data/param-inheritance/path_base.yaml @@ -0,0 +1,56 @@ +openapi: 3.0.0 +info: + title: Some API + version: 1.0.0 + description: Some desc + contact: + name: '#onetwothree' + url: 'https://test.com' +servers: + - description: local + url: 'http://localhost:8080' +tags: + - name: One + - name: Two +paths: + '/admin/v0/abc/{id}': + parameters: + - $ref: '#/components/parameters/tenant_id' + - $ref: '#/components/parameters/id' + get: + summary: Get abc + tags: + - Two + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/abc' +components: + schemas: + abc: + title: abc + x-stoplight: + id: lzwt7se3t6ab2 + type: object + properties: + details: + type: string + + parameters: + tenant_id: + schema: + type: string + in: header + required: true + name: tenant-id + description: 'Tenant IDs' + id: + name: id + in: path + required: true + schema: + type: string + description: 'The ID' \ No newline at end of file diff --git a/data/param-inheritance/path_revision.yaml b/data/param-inheritance/path_revision.yaml new file mode 100644 index 00000000..0da06466 --- /dev/null +++ b/data/param-inheritance/path_revision.yaml @@ -0,0 +1,53 @@ +openapi: 3.0.0 +info: + title: Some API + version: 1.0.0 + description: Some desc + contact: + name: '#onetwothree' + url: 'https://test.com22' +servers: + - description: local + url: 'http://localhost:8080' +tags: + - name: One + - name: Two +paths: + '/admin/v0/abc/{id}': + get: + summary: Get abc + tags: + - Two + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/abc' +components: + schemas: + abc: + title: abc + x-stoplight: + id: lzwt7se3t6ab2 + type: object + properties: + details: + type: string + + parameters: + tenant_id: + schema: + type: string + in: header + required: true + name: tenant-id + description: 'Tenant IDs' + id: + name: id + in: path + required: true + schema: + type: string + description: 'The ID' \ No newline at end of file diff --git a/diff/diff_test.go b/diff/diff_test.go index 7fe7b8ee..d6fa2d19 100644 --- a/diff/diff_test.go +++ b/diff/diff_test.go @@ -918,3 +918,23 @@ func TestDiff_DifferentComponentSameHeader(t *testing.T) { require.NoError(t, err) require.Empty(t, d) } + +func TestDiff_MovedParam(t *testing.T) { + loader := openapi3.NewLoader() + + s1, err := loader.LoadFromFile("../data/param-inheritance/path_base.yaml") + require.NoError(t, err) + + s2, err := loader.LoadFromFile("../data/param-inheritance/path_revision.yaml") + require.NoError(t, err) + + d, _, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), + &load.SpecInfo{ + Spec: s1, + }, + &load.SpecInfo{ + Spec: s2, + }) + require.NoError(t, err) + require.NotEmpty(t, d.EndpointsDiff) +} diff --git a/diff/method_diff.go b/diff/method_diff.go index 1045f3da..c2806230 100644 --- a/diff/method_diff.go +++ b/diff/method_diff.go @@ -36,9 +36,10 @@ func (methodDiff *MethodDiff) Empty() bool { return *methodDiff == MethodDiff{Base: methodDiff.Base, Revision: methodDiff.Revision} } -func getMethodDiff(config *Config, state *state, operation1, operation2 *openapi3.Operation, pathParamsMap PathParamsMap) (*MethodDiff, error) { +func getMethodDiff(config *Config, state *state, operation1, operation2 *openapi3.Operation, pathParamsMap PathParamsMap, + pathParams1, pathParams2 openapi3.Parameters) (*MethodDiff, error) { - diff, err := getMethodDiffInternal(config, state, operation1, operation2, pathParamsMap) + diff, err := getMethodDiffInternal(config, state, operation1, operation2, pathParamsMap, pathParams1, pathParams2) if err != nil { return nil, err @@ -51,7 +52,8 @@ func getMethodDiff(config *Config, state *state, operation1, operation2 *openapi return diff, nil } -func getMethodDiffInternal(config *Config, state *state, operation1, operation2 *openapi3.Operation, pathParamsMap PathParamsMap) (*MethodDiff, error) { +func getMethodDiffInternal(config *Config, state *state, operation1, operation2 *openapi3.Operation, pathParamsMap PathParamsMap, + pathParams1, pathParams2 openapi3.Parameters) (*MethodDiff, error) { result := newMethodDiff() var err error @@ -61,7 +63,7 @@ func getMethodDiffInternal(config *Config, state *state, operation1, operation2 result.SummaryDiff = getValueDiffConditional(config.IsExcludeSummary(), operation1.Summary, operation2.Summary) result.DescriptionDiff = getValueDiffConditional(config.IsExcludeDescription(), operation1.Description, operation2.Description) result.OperationIDDiff = getValueDiff(operation1.OperationID, operation2.OperationID) - result.ParametersDiff, err = getParametersDiffByLocation(config, state, operation1.Parameters, operation2.Parameters, pathParamsMap) + result.ParametersDiff, err = getParametersDiffByLocation(config, state, paramsInherit(pathParams1, operation1.Parameters), paramsInherit(pathParams2, operation2.Parameters), pathParamsMap) if err != nil { return nil, err } diff --git a/diff/operations_diff.go b/diff/operations_diff.go index d0f4ba9e..f4c3ee85 100644 --- a/diff/operations_diff.go +++ b/diff/operations_diff.go @@ -76,7 +76,8 @@ func getOperationsDiffInternal(config *Config, state *state, pathItemPair *pathI var err error for _, op := range operations { - err = result.diffOperation(config, state, pathItemPair.PathItem1.GetOperation(op), pathItemPair.PathItem2.GetOperation(op), op, pathItemPair.PathParamsMap) + err = result.diffOperation(config, state, pathItemPair.PathItem1.GetOperation(op), pathItemPair.PathItem2.GetOperation(op), op, pathItemPair.PathParamsMap, + pathItemPair.PathItem1.Parameters, pathItemPair.PathItem2.Parameters) if err != nil { return nil, err } @@ -85,7 +86,8 @@ func getOperationsDiffInternal(config *Config, state *state, pathItemPair *pathI return result, nil } -func (operationsDiff *OperationsDiff) diffOperation(config *Config, state *state, operation1, operation2 *openapi3.Operation, method string, pathParamsMap PathParamsMap) error { +func (operationsDiff *OperationsDiff) diffOperation(config *Config, state *state, operation1, operation2 *openapi3.Operation, method string, pathParamsMap PathParamsMap, + pathParams1, pathParams2 openapi3.Parameters) error { if operation1 == nil && operation2 == nil { return nil } @@ -100,7 +102,7 @@ func (operationsDiff *OperationsDiff) diffOperation(config *Config, state *state return nil } - diff, err := getMethodDiff(config, state, operation1, operation2, pathParamsMap) + diff, err := getMethodDiff(config, state, operation1, operation2, pathParamsMap, pathParams1, pathParams2) if err != nil { return err } diff --git a/diff/params_inherit.go b/diff/params_inherit.go new file mode 100644 index 00000000..0aec03ec --- /dev/null +++ b/diff/params_inherit.go @@ -0,0 +1,19 @@ +package diff + +import "github.com/getkin/kin-openapi/openapi3" + +func paramsInherit(pathParams, opParams openapi3.Parameters) openapi3.Parameters { + for _, pathParam := range pathParams { + opParams = paramInherit(opParams, pathParam.Value) + } + return opParams +} + +func paramInherit(opParams openapi3.Parameters, pathParam *openapi3.Parameter) openapi3.Parameters { + if opParams.GetByInAndName(pathParam.In, pathParam.Name) == nil { + opParams = append(opParams, &openapi3.ParameterRef{ + Value: pathParam, + }) + } + return opParams +} diff --git a/report/example_test.go b/report/example_test.go index 004fde38..9a90e9a7 100644 --- a/report/example_test.go +++ b/report/example_test.go @@ -70,6 +70,7 @@ func ExampleGetTextReportAsString() { // - Deleted response: 400 // // GET /api/{domain}/{project}/install-command + // - New header param: name // - Deleted header param: network-policies // - Modified path param: project // - Schema changed @@ -193,6 +194,7 @@ func ExampleGetHTMLReportAsString() { // //

GET /api/{domain}/{project}/install-command

//