fix(flatten): allows the use of pointers to schemas in x-... extensions

This PR allows for reusing schemas stored in x-... extensions of the
spec, via json pointers.

We have seen some specs using such extensions to create "alternate
definitions". The spec flattening canonicalizes such schemas by
reintroducing them as #/definitions, so they can be consumed by codegen.

* requires: go-openapi/jsonpointer#16
* contributes: go-swagger/go-swagger#1898

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>

.github/workflows/go-test.yml

@@ -38,7 +38,7 @@ jobs:
 
     - uses: actions/checkout@v3
 
-    - run: go test -v -race -coverprofile="coverage-${{ matrix.os }}.${{ matrix.go_version }}.out" -covermode=atomic ./...
+    - run: go test -v -race -coverprofile="coverage-${{ matrix.os }}.${{ matrix.go_version }}.out" -covermode=atomic -coverpkg=$(go list ./...) ./...
 
     - name: Upload coverage to codecov
       uses: codecov/codecov-action@v3

fixtures/bugs/1898/swagger.json

@@ -0,0 +1,105 @@
+{
+  "swagger": "2.0",
+  "info": {
+    "title": "example.proto",
+    "version": "version not set"
+  },
+  "schemes": [
+    "http",
+    "https"
+  ],
+  "consumes": [
+    "application/json"
+  ],
+  "produces": [
+    "application/json"
+  ],
+  "paths": {
+    "/example/v2/GetEvents": {
+      "get": {
+        "operationId": "GetEvents",
+        "responses": {
+          "200": {
+            "description": "A successful response.(streaming responses)",
+            "schema": {
+              "$ref": "#/x-stream-definitions/v2EventMsg"
+            }
+          }
+        },
+        "parameters": [
+          {
+            "name": "afterEventID",
+            "description": ".",
+            "in": "query",
+            "required": false,
+            "type": "string"
+          }
+        ],
+        "tags": [
+          "Matchmaking"
+        ]
+      }
+    }
+  },
+  "definitions": {
+    "protobufAny": {
+      "type": "object",
+      "properties": {
+        "type_url": {
+          "type": "string"
+        },
+        "value": {
+          "type": "string",
+          "format": "byte"
+        }
+      }
+    },
+    "runtimeStreamError": {
+      "type": "object",
+      "properties": {
+        "grpc_code": {
+          "type": "integer",
+          "format": "int32"
+        },
+        "http_code": {
+          "type": "integer",
+          "format": "int32"
+        },
+        "message": {
+          "type": "string"
+        },
+        "http_status": {
+          "type": "string"
+        },
+        "details": {
+          "type": "array",
+          "items": {
+            "$ref": "#/definitions/protobufAny"
+          }
+        }
+      }
+    },
+    "v2EventMsg": {
+      "type": "object",
+      "properties": {
+        "eventID": {
+          "type": "string"
+        }
+      }
+    }
+  },
+  "x-stream-definitions": {
+    "v2EventMsg": {
+      "type": "object",
+      "properties": {
+        "result": {
+          "$ref": "#/definitions/v2EventMsg"
+        },
+        "error": {
+          "$ref": "#/definitions/runtimeStreamError"
+        }
+      },
+      "title": "Stream result of v2EventMsg"
+    }
+  }
+}

flatten.go

@@ -649,6 +649,7 @@ func namePointers(opts *FlattenOpts) error {
 
 	refsToReplace := make(map[string]SchemaRef, len(opts.Spec.references.schemas))
 	for k, ref := range opts.Spec.references.allRefs {
+		debugLog("name pointers: %q => %#v", k, ref)
 		if path.Dir(ref.String()) == definitionsPath {
 			// this a ref to a top-level definition: ok
 			continue
@@ -766,6 +767,10 @@ func flattenAnonPointer(key string, v SchemaRef, refsToReplace map[string]Schema
 
 	// identifying edge case when the namer did nothing because we point to a non-schema object
 	// no definition is created and we expand the $ref for all callers
+	debugLog("decide what to do with the schema pointed to: asch.IsSimpleSchema=%t, len(callers)=%d, parts.IsSharedParam=%t, parts.IsSharedResponse=%t",
+		asch.IsSimpleSchema, len(callers), parts.IsSharedParam(), parts.IsSharedResponse(),
+	)
+
 	if (!asch.IsSimpleSchema || len(callers) > 1) && !parts.IsSharedParam() && !parts.IsSharedResponse() {
 		debugLog("replace JSON pointer at [%s] by definition: %s", key, v.Ref.String())
 		if err := namer.Name(v.Ref.String(), v.Schema, asch); err != nil {
@@ -788,6 +793,7 @@ func flattenAnonPointer(key string, v SchemaRef, refsToReplace map[string]Schema
 		return nil
 	}
 
+	// everything that is a simple schema and not factorizable is expanded
 	debugLog("expand JSON pointer for key=%s", key)
 
 	if err := replace.UpdateRefWithSchema(opts.Swagger(), key, v.Schema); err != nil {

flatten_name.go

@@ -40,6 +40,7 @@ func (isn *InlineSchemaNamer) Name(key string, schema *spec.Schema, aschema *Ana
 		sch := schutils.Clone(schema)
 
 		// replace values on schema
+		debugLog("rewriting schema to ref: key=%s with new name: %s", key, newName)
 		if err := replace.RewriteSchemaToRef(isn.Spec, key,
 			spec.MustCreateRef(path.Join(definitionsPath, newName))); err != nil {
 			return fmt.Errorf("error while creating definition %q from inline schema: %w", newName, err)
@@ -150,13 +151,15 @@ func namesFromKey(parts sortref.SplitKey, aschema *AnalyzedSchema, operations ma
 		startIndex int
 	)
 
-	if parts.IsOperation() {
+	switch {
+	case parts.IsOperation():
 		baseNames, startIndex = namesForOperation(parts, operations)
-	}
-
-	// definitions
-	if parts.IsDefinition() {
+	case parts.IsDefinition():
 		baseNames, startIndex = namesForDefinition(parts)
+	default:
+		// this a non-standard pointer: build a name by concatenating its parts
+		baseNames = [][]string{parts}
+		startIndex = len(baseNames) + 1
 	}
 
 	result := make([]string, 0, len(baseNames))
@@ -170,6 +173,7 @@ func namesFromKey(parts sortref.SplitKey, aschema *AnalyzedSchema, operations ma
 	}
 	sort.Strings(result)
 
+	debugLog("names from parts: %v => %v", parts, result)
 	return result
 }
 

flatten_test.go

@@ -1356,6 +1356,33 @@ func TestFlatten_2334(t *testing.T) {
 	assert.Contains(t, jazon, `"Baz":`)
 }
 
+func TestFlatten_1898(t *testing.T) {
+	log.SetOutput(io.Discard)
+	defer log.SetOutput(os.Stdout)
+
+	bp := filepath.Join("fixtures", "bugs", "1898", "swagger.json")
+	sp := antest.LoadOrFail(t, bp)
+	an := New(sp)
+
+	require.NoError(t, Flatten(FlattenOpts{
+		Spec: an, BasePath: bp, Verbose: true,
+		Expand:       false,
+		RemoveUnused: false,
+	}))
+	op, ok := an.OperationFor("GET", "/example/v2/GetEvents")
+	require.True(t, ok)
+
+	resp, _, ok := op.SuccessResponse()
+	require.True(t, ok)
+
+	require.Equal(t, "#/definitions/xStreamDefinitionsV2EventMsg", resp.Schema.Ref.String())
+	def, ok := sp.Definitions["xStreamDefinitionsV2EventMsg"]
+	require.True(t, ok)
+	require.Len(t, def.Properties, 2)
+	require.Contains(t, def.Properties, "error")
+	require.Contains(t, def.Properties, "result")
+}
+
 func getDefinition(t testing.TB, sp *spec.Swagger, key string) string {
 	d, ok := sp.Definitions[key]
 	require.Truef(t, ok, "Expected definition for %s", key)

go.mod

@@ -1,7 +1,7 @@
 module github.com/go-openapi/analysis
 
 require (
-	github.com/go-openapi/jsonpointer v0.20.1
+	github.com/go-openapi/jsonpointer v0.20.2
 	github.com/go-openapi/spec v0.20.12
 	github.com/go-openapi/strfmt v0.21.10
 	github.com/go-openapi/swag v0.22.5

go.sum

@@ -4,8 +4,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/go-openapi/errors v0.21.0 h1:FhChC/duCnfoLj1gZ0BgaBmzhJC2SL/sJr8a2vAobSY=
 github.com/go-openapi/errors v0.21.0/go.mod h1:jxNTMUxRCKj65yb/okJGEtahVd7uvWnuWfj53bse4ho=
-github.com/go-openapi/jsonpointer v0.20.1 h1:MkK4VEIEZMj4wT9PmjaUmGflVBr9nvud4Q4UVFbDoBE=
-github.com/go-openapi/jsonpointer v0.20.1/go.mod h1:bHen+N0u1KEO3YlmqOjTT9Adn1RfD91Ar825/PuiRVs=
+github.com/go-openapi/jsonpointer v0.20.2 h1:mQc3nmndL8ZBzStEo3JYF8wzmeWffDH4VbXz58sAx6Q=
+github.com/go-openapi/jsonpointer v0.20.2/go.mod h1:bHen+N0u1KEO3YlmqOjTT9Adn1RfD91Ar825/PuiRVs=
 github.com/go-openapi/jsonreference v0.20.3 h1:EjGcjTW8pD1mRis6+w/gmoBdqv5+RbE9B85D1NgDOVQ=
 github.com/go-openapi/jsonreference v0.20.3/go.mod h1:FviDZ46i9ivh810gqzFLl5NttD5q3tSlMLqLr6okedM=
 github.com/go-openapi/spec v0.20.12 h1:cgSLbrsmziAP2iais+Vz7kSazwZ8rsUZd6TUzdDgkVI=

internal/flatten/replace/replace.go

@@ -1,6 +1,7 @@
 package replace
 
 import (
+	"encoding/json"
 	"fmt"
 	"net/url"
 	"os"
@@ -40,6 +41,8 @@ func RewriteSchemaToRef(sp *spec.Swagger, key string, ref spec.Ref) error {
 		if refable.Schema != nil {
 			refable.Schema = &spec.Schema{SchemaProps: spec.SchemaProps{Ref: ref}}
 		}
+	case map[string]interface{}: // this happens e.g. if a schema points to an extension unmarshaled as map[string]interface{}
+		return rewriteParentRef(sp, key, ref)
 	default:
 		return fmt.Errorf("no schema with ref found at %s for %T", key, value)
 	}
@@ -120,6 +123,9 @@ func rewriteParentRef(sp *spec.Swagger, key string, ref spec.Ref) error {
 	case spec.SchemaProperties:
 		container[entry] = spec.Schema{SchemaProps: spec.SchemaProps{Ref: ref}}
 
+	case *interface{}:
+		*container = spec.Schema{SchemaProps: spec.SchemaProps{Ref: ref}}
+
 	// NOTE: can't have case *spec.SchemaOrBool = parent in this case is *Schema
 
 	default:
@@ -385,8 +391,9 @@ DOWNREF:
 			err := asSchema.UnmarshalJSON(asJSON)
 			if err != nil {
 				return nil,
-					fmt.Errorf("invalid type for resolved JSON pointer %s. Expected a schema a, got: %T",
-						currentRef.String(), value)
+					fmt.Errorf("invalid type for resolved JSON pointer %s. Expected a schema a, got: %T (%v)",
+						currentRef.String(), value, err,
+					)
 			}
 			warnings = append(warnings, fmt.Sprintf("found $ref %q (response) interpreted as schema", currentRef.String()))
 
@@ -402,8 +409,9 @@ DOWNREF:
 			var asSchema spec.Schema
 			if err := asSchema.UnmarshalJSON(asJSON); err != nil {
 				return nil,
-					fmt.Errorf("invalid type for resolved JSON pointer %s. Expected a schema a, got: %T",
-						currentRef.String(), value)
+					fmt.Errorf("invalid type for resolved JSON pointer %s. Expected a schema a, got: %T (%v)",
+						currentRef.String(), value, err,
+					)
 			}
 
 			warnings = append(warnings, fmt.Sprintf("found $ref %q (parameter) interpreted as schema", currentRef.String()))
@@ -414,9 +422,25 @@ DOWNREF:
 			currentRef = asSchema.Ref
 
 		default:
-			return nil,
-				fmt.Errorf("unhandled type to resolve JSON pointer %s. Expected a Schema, got: %T",
-					currentRef.String(), value)
+			// fallback: attempts to resolve the pointer as a schema
+			if refable == nil {
+				break DOWNREF
+			}
+
+			asJSON, _ := json.Marshal(refable)
+			var asSchema spec.Schema
+			if err := asSchema.UnmarshalJSON(asJSON); err != nil {
+				return nil,
+					fmt.Errorf("unhandled type to resolve JSON pointer %s. Expected a Schema, got: %T (%v)",
+						currentRef.String(), value, err,
+					)
+			}
+			warnings = append(warnings, fmt.Sprintf("found $ref %q (%T) interpreted as schema", currentRef.String(), refable))
+
+			if asSchema.Ref.String() == "" {
+				break DOWNREF
+			}
+			currentRef = asSchema.Ref
 		}
 	}
 

internal/flatten/sortref/keys.go

@@ -69,7 +69,7 @@ func KeyParts(key string) SplitKey {
 	return res
 }
 
-// SplitKey holds of the parts of a /-separated key, soi that their location may be determined.
+// SplitKey holds of the parts of a /-separated key, so that their location may be determined.
 type SplitKey []string
 
 // IsDefinition is true when the split key is in the #/definitions section of a spec