From f16b6574b8cf1b98577278fd984c742bba8df4d3 Mon Sep 17 00:00:00 2001 From: tcdsv Date: Tue, 24 Oct 2023 10:23:14 +0300 Subject: [PATCH] handle circular refs in properties, items, anyof, oneof --- flatten/merge_allof.go | 42 +++++- flatten/merge_allof_test.go | 120 ++++++++++++++---- flatten/testdata/circular-refs-allof.yaml | 25 ++++ flatten/testdata/circular-refs-anyof.yaml | 41 ++++++ flatten/testdata/circular-refs-items.yaml | 43 +++++++ flatten/testdata/circular-refs-oneof.yaml | 41 ++++++ .../testdata/circular-refs-properties.yaml | 71 +++++++++++ 7 files changed, 357 insertions(+), 26 deletions(-) create mode 100644 flatten/testdata/circular-refs-allof.yaml create mode 100644 flatten/testdata/circular-refs-anyof.yaml create mode 100644 flatten/testdata/circular-refs-items.yaml create mode 100644 flatten/testdata/circular-refs-oneof.yaml create mode 100644 flatten/testdata/circular-refs-properties.yaml diff --git a/flatten/merge_allof.go b/flatten/merge_allof.go index 41d5d94f..84f28a13 100644 --- a/flatten/merge_allof.go +++ b/flatten/merge_allof.go @@ -266,7 +266,6 @@ func resolveNumberRange(schema *openapi3.Schema, collection *SchemaCollection) * } func resolveItems(state *state, schema *openapi3.Schema, collection *SchemaCollection) (*openapi3.Schema, error) { - items := openapi3.SchemaRefs{} for _, sref := range collection.Items { if sref != nil { @@ -278,7 +277,31 @@ func resolveItems(state *state, schema *openapi3.Schema, collection *SchemaColle return schema, nil } - res, err := flattenSchemas(state, items, openapi3.NewSchema()) + mergedItems := openapi3.SchemaRefs{} + for _, item := range items { + m, err := mergeInternal(state, *item) + if err != nil { + return nil, err + } + mergedItems = append(mergedItems, m) + } + + if len(items) == 1 { + /*r, err := mergeInternal(state, *items[0]) + if err != nil { + return nil, err + }*/ + schema.Items = mergedItems[0] + return schema, nil + } + + // Any list of Items that includes a circular reference is considered invalid, + // except a single circular ref Item. + if len(mergedItems) > 1 && isCircular(state, mergedItems) { + return nil, errors.New("could not merge items with circular refs") + } + + res, err := flattenSchemas(state, mergedItems, openapi3.NewSchema()) if err != nil { return nil, err } @@ -476,7 +499,9 @@ func mergeProps(state *state, schema *openapi3.Schema, collection *SchemaCollect continue } - // TODO: If schemas of some property contain a loop, do not flatten them. + if isCircular(state, schemas) { + return nil, errors.New("could not merge circular ref property") + } mergedProp, err := flattenSchemas(state, schemas, openapi3.NewSchema()) if err != nil { @@ -879,7 +904,7 @@ func resolveCombinations(state *state, collection []openapi3.SchemaRefs) (openap } groups, err := mergeSchemaRefs(state, groups) if err != nil { - return openapi3.SchemaRefs{}, err + return nil, err } // there is only one schema, no need for calculating combinations. if len(groups) == 1 { @@ -887,6 +912,15 @@ func resolveCombinations(state *state, collection []openapi3.SchemaRefs) (openap } combinations := getCombinations(groups) + + // Any combination that includes a circular reference is considered invalid, + // except a combination that includes a single circular ref, without additional schemas. + for _, c := range combinations { + if len(c) > 1 && isCircular(state, c) { + return nil, errors.New("could not merge OneOf/AnyOf with circular ref") + } + } + mergedCombinations, err := mergeCombinations(state, combinations) if err != nil { return nil, err diff --git a/flatten/merge_allof_test.go b/flatten/merge_allof_test.go index 0f088887..27259bf5 100644 --- a/flatten/merge_allof_test.go +++ b/flatten/merge_allof_test.go @@ -1653,12 +1653,7 @@ func TestMerge_Required(t *testing.T) { } func TestMerge_SimpleCircularAllOf(t *testing.T) { - ctx := context.Background() - sl := openapi3.NewLoader() - doc, err := sl.LoadFromFile("testdata/circular1.yaml") - require.NoError(t, err, "loading test file") - err = doc.Validate(ctx) - require.NoError(t, err) + doc := loadSpec(t, "testdata/circular-refs-allof.yaml") result, err := flatten.Merge(*doc.Components.Schemas["Circular_1"]) require.NoError(t, err) require.NotEmpty(t, result.AllOf) @@ -1666,33 +1661,114 @@ func TestMerge_SimpleCircularAllOf(t *testing.T) { require.Equal(t, result, result.AllOf[0].Value) } -func TestMerge_CircularAllOfProps(t *testing.T) { - - ctx := context.Background() - sl := openapi3.NewLoader() - doc, err := sl.LoadFromFile("testdata/circular1.yaml") +func TestMerge_CircularRefsPropsFailure(t *testing.T) { + doc := loadSpec(t, "testdata/circular-refs-properties.yaml") + _, err := flatten.Merge(*doc.Components.Schemas["Failed_Merge_1"]) + require.Error(t, err) +} - require.NoError(t, err, "loading test file") - err = doc.Validate(ctx) - require.NoError(t, err) - result, err := flatten.Merge(*doc.Components.Schemas["Circular_2"]) +func TestMerge_CircularRefsProps(t *testing.T) { + doc := loadSpec(t, "testdata/circular-refs-properties.yaml") + result, err := flatten.Merge(*doc.Components.Schemas["Successful_Merge_2"]) require.NoError(t, err) require.Len(t, result.AllOf, 2) require.Equal(t, result, result.AllOf[0].Value) - require.Equal(t, "#/components/schemas/Circular_2", result.AllOf[0].Ref) + require.Equal(t, "#/components/schemas/Successful_Merge_2", result.AllOf[0].Ref) require.Equal(t, result, result.AllOf[1].Value.Properties["test"].Value) - require.Equal(t, "#/components/schemas/Circular_2", result.AllOf[1].Value.Properties["test"].Ref) + require.Equal(t, "#/components/schemas/Successful_Merge_2", result.AllOf[1].Value.Properties["test"].Ref) - result, err = flatten.Merge(*doc.Components.Schemas["Circular_3"]) + result, err = flatten.Merge(*doc.Components.Schemas["Successful_Merge_3"]) require.NoError(t, err) - require.Equal(t, "#/components/schemas/Circular_3", result.Properties["test"].Value.AllOf[0].Ref) + require.Equal(t, "#/components/schemas/Successful_Merge_3", result.Properties["test"].Value.AllOf[0].Ref) require.Equal(t, result, result.Properties["test"].Value.AllOf[0].Value) - result, err = flatten.Merge(*doc.Components.Schemas["Circular_4"]) + result, err = flatten.Merge(*doc.Components.Schemas["Successful_Merge_4"]) require.NoError(t, err) - require.Equal(t, "#/components/schemas/Circular_4", result.AllOf[0].Ref) + require.Equal(t, "#/components/schemas/Successful_Merge_4", result.AllOf[0].Ref) require.Equal(t, result, result.AllOf[0].Value) - require.Equal(t, "#/components/schemas/Circular_4", result.AllOf[1].Value.Properties["test"].Value.Properties["b"].Ref) + require.Equal(t, "#/components/schemas/Successful_Merge_4", result.AllOf[1].Value.Properties["test"].Value.Properties["b"].Ref) require.Equal(t, result, result.AllOf[1].Value.Properties["test"].Value.Properties["b"].Value) } + +// Circular property field could not be merged with other schemas +func TestMerge_CircularRefPropFailure(t *testing.T) { + doc := loadSpec(t, "testdata/circular-refs-properties.yaml") + _, err := flatten.Merge(*doc.Components.Schemas["Failed_Merge_1"]) + require.Error(t, err) +} + +// Circular item could not be merged with other schemas +func TestMerge_CircularRefItemFailure(t *testing.T) { + doc := loadSpec(t, "testdata/circular-refs-items.yaml") + _, err := flatten.Merge(*doc.Components.Schemas["Failed_Merge_1"]) + require.Error(t, err) +} + +func TestMerge_CircularRefItem(t *testing.T) { + doc := loadSpec(t, "testdata/circular-refs-items.yaml") + result, err := flatten.Merge(*doc.Components.Schemas["Successful_Merge_1"]) + require.NoError(t, err) + require.Equal(t, "#/components/schemas/Successful_Merge_1", result.Properties["test"].Value.Items.Ref) + require.Equal(t, result, result.Properties["test"].Value.Items.Value) +} + +func TestMerge_CircularRefAnyOf(t *testing.T) { + doc := loadSpec(t, "testdata/circular-refs-anyof.yaml") + result, err := flatten.Merge(*doc.Components.Schemas["Successful_Merge_1"]) + require.NoError(t, err) + require.Equal(t, "#/components/schemas/Successful_Merge_1", result.AnyOf[0].Ref) + require.Equal(t, result, result.AnyOf[0].Value) + require.Equal(t, "integer", result.Properties["test"].Value.Type) +} + +func TestMerge_CircularRefAnyOf_Failure(t *testing.T) { + doc := loadSpec(t, "testdata/circular-refs-anyof.yaml") + _, err := flatten.Merge(*doc.Components.Schemas["Failed_Merge_1"]) + require.Error(t, err) +} + +func TestMerge_CircularRefOneOf(t *testing.T) { + doc := loadSpec(t, "testdata/circular-refs-oneof.yaml") + result, err := flatten.Merge(*doc.Components.Schemas["Successful_Merge_1"]) + require.NoError(t, err) + require.Equal(t, "#/components/schemas/Successful_Merge_1", result.OneOf[0].Ref) + require.Equal(t, result, result.OneOf[0].Value) + require.Equal(t, "integer", result.Properties["test"].Value.Type) +} + +func TestMerge_CircularRefOneOf_Failure(t *testing.T) { + doc := loadSpec(t, "testdata/circular-refs-oneof.yaml") + _, err := flatten.Merge(*doc.Components.Schemas["Failed_Merge_1"]) + require.Error(t, err) +} + +func loadSpec(t *testing.T, path string) *openapi3.T { + ctx := context.Background() + sl := openapi3.NewLoader() + doc, err := sl.LoadFromFile(path) + require.NoError(t, err, "loading test file") + err = doc.Validate(ctx) + require.NoError(t, err) + return doc +} + +// doc, err := sl.LoadFromFile("testdata/circular-refs-allof-validation.yaml") + +func TestMerge_CircularRef_Success(t *testing.T) { + doc := loadSpec(t, "testdata/circular-refs-allof-validation.yaml") + schema_A, err := flatten.Merge(*doc.Components.Schemas["A"]) + require.NoError(t, err) + + require.Equal(t, "#/components/schemas/B", schema_A.OneOf[0].Ref) + + schema_B := schema_A.OneOf[0].Value + + require.Equal(t, "#/components/schemas/A", schema_B.AllOf[0].Ref) + require.Equal(t, schema_A, schema_B.AllOf[0].Value) + + require.Equal(t, "number", schema_A.Properties["prop1"].Value.Type) + require.Equal(t, "integer", schema_B.AllOf[1].Value.Properties["prop1"].Value.Type) + + // require.Equal(t, "integer", result.Properties["test"].Value.Type) +} diff --git a/flatten/testdata/circular-refs-allof.yaml b/flatten/testdata/circular-refs-allof.yaml new file mode 100644 index 00000000..3de43444 --- /dev/null +++ b/flatten/testdata/circular-refs-allof.yaml @@ -0,0 +1,25 @@ +openapi: 3.0.0 +info: + title: Circular Reference Example + version: 1.0.0 +paths: + /endpoint: + get: + summary: Example endpoint + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Circular_1' + +components: + schemas: + Circular_1: + type: object + allOf: + - $ref: '#/components/schemas/Circular_1' + required: + - name + description: simple circular allof \ No newline at end of file diff --git a/flatten/testdata/circular-refs-anyof.yaml b/flatten/testdata/circular-refs-anyof.yaml new file mode 100644 index 00000000..f5639ea9 --- /dev/null +++ b/flatten/testdata/circular-refs-anyof.yaml @@ -0,0 +1,41 @@ +openapi: 3.0.0 +info: + title: Successful merge of Items with circular refs + version: 1.0.0 +paths: + /endpoint: + get: + summary: Example endpoint + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Successful_Merge_1' + +components: + schemas: + Successful_Merge_1: + type: object + allOf: + - type: object + anyOf: + - $ref: '#/components/schemas/Successful_Merge_1' + - type: object + properties: + test: + type: integer + + Failed_Merge_1: + type: object + allOf: + - type: object + anyOf: + - $ref: '#/components/schemas/Failed_Merge_1' + - type: object + anyOf: + - type: object + properties: + test: + type: integer \ No newline at end of file diff --git a/flatten/testdata/circular-refs-items.yaml b/flatten/testdata/circular-refs-items.yaml new file mode 100644 index 00000000..9e49077e --- /dev/null +++ b/flatten/testdata/circular-refs-items.yaml @@ -0,0 +1,43 @@ +openapi: 3.0.0 +info: + title: Successful merge of Items with circular refs + version: 1.0.0 +paths: + /endpoint: + get: + summary: Example endpoint + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Successful_Merge_1' + +components: + schemas: + Successful_Merge_1: + type: object + allOf: + - type: object + properties: + test: + type: array + items: + $ref: '#/components/schemas/Successful_Merge_1' + + Failed_Merge_1: + type: object + allOf: + - type: object + properties: + test: + type: array + items: + type: integer + - type: object + properties: + test: + type: array + items: + $ref: '#/components/schemas/Failed_Merge_1' diff --git a/flatten/testdata/circular-refs-oneof.yaml b/flatten/testdata/circular-refs-oneof.yaml new file mode 100644 index 00000000..c6c761bd --- /dev/null +++ b/flatten/testdata/circular-refs-oneof.yaml @@ -0,0 +1,41 @@ +openapi: 3.0.0 +info: + title: Successful merge of Items with circular refs + version: 1.0.0 +paths: + /endpoint: + get: + summary: Example endpoint + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Successful_Merge_1' + +components: + schemas: + Successful_Merge_1: + type: object + allOf: + - type: object + oneOf: + - $ref: '#/components/schemas/Successful_Merge_1' + - type: object + properties: + test: + type: integer + + Failed_Merge_1: + type: object + allOf: + - type: object + oneOf: + - $ref: '#/components/schemas/Failed_Merge_1' + - type: object + oneOf: + - type: object + properties: + test: + type: integer \ No newline at end of file diff --git a/flatten/testdata/circular-refs-properties.yaml b/flatten/testdata/circular-refs-properties.yaml new file mode 100644 index 00000000..6955324a --- /dev/null +++ b/flatten/testdata/circular-refs-properties.yaml @@ -0,0 +1,71 @@ +openapi: 3.0.0 +info: + title: Circular Reference Example + version: 1.0.0 +paths: + /endpoint: + get: + summary: Example endpoint + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Successful_Merge_2' + +components: + schemas: + + Successful_Merge_2: + type: object + allOf: + - $ref: '#/components/schemas/Successful_Merge_2' + - type: object + properties: + test: + $ref: '#/components/schemas/Successful_Merge_2' + + Successful_Merge_3: + type: object + properties: + test: + type: object + allOf: + - $ref: '#/components/schemas/Successful_Merge_3' + + Successful_Merge_4: + type: object + allOf: + - $ref: '#/components/schemas/Successful_Merge_4' + - type: object + properties: + test: + type: object + allOf: + - type: object + properties: + a: + type: object + description: a + - type: object + properties: + b: + $ref: '#/components/schemas/Successful_Merge_4' + description: b + + Failed_Merge_1: + type: object + allOf: + - type: object + properties: + test: + type: array + items: + type: integer + - type: object + properties: + test: + type: array + items: + $ref: '#/components/schemas/Failed_Merge_1'