handle circular refs in properties, items, anyof, oneof

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,14 +904,23 @@ 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 {
 		return groups[0], nil
 	}
 
 	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

flatten/merge_allof_test.go

@@ -1653,46 +1653,122 @@ 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)
 	require.Equal(t, "#/components/schemas/Circular_1", result.AllOf[0].Ref)
 	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)
+}

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

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

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'

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