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 <[email protected]>
fredbi · Dec 21, 2023 · f7563d0 · f7563d0
1 parent a50691f
commit f7563d0
Show file tree

Hide file tree

Showing 8 changed files with 183 additions and 15 deletions.
diff --git a/fixtures/bugs/1898/swagger.json b/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"
+    }
+  }
+}
diff --git a/flatten.go b/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 {

diff --git a/flatten_name.go b/flatten_name.go
@@ -39,6 +39,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)
@@ -149,13 +150,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))
@@ -169,6 +172,7 @@ func namesFromKey(parts sortref.SplitKey, aschema *AnalyzedSchema, operations ma
 	}
 	sort.Strings(result)
 
+	debugLog("names from parts: %v => %v", parts, result)
 	return result
 }
 

diff --git a/flatten_test.go b/flatten_test.go
@@ -1332,6 +1332,33 @@ func TestFlatten_2113(t *testing.T) {
 	require.JSONEq(t, string(expected), jazon)
 }
 
+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)

diff --git a/go.mod b/go.mod
@@ -1,5 +1,7 @@
 module github.com/go-openapi/analysis
 
+replace github.com/go-openapi/jsonpointer => github.com/fredbi/jsonpointer v0.20.2-fred
+
 require (
 	github.com/go-openapi/jsonpointer v0.20.1
 	github.com/go-openapi/spec v0.20.12

diff --git a/go.sum b/go.sum
@@ -2,10 +2,10 @@ github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3d
 github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw=
 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/fredbi/jsonpointer v0.20.2-fred h1:u+zu3MdibDkp55vuOFsgR04BQuwCG1Bo3ES/NKZEp3k=
+github.com/fredbi/jsonpointer v0.20.2-fred/go.mod h1:bHen+N0u1KEO3YlmqOjTT9Adn1RfD91Ar825/PuiRVs=
 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/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=

diff --git a/internal/flatten/replace/replace.go b/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: attemps 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
 		}
 	}
 

diff --git a/internal/flatten/sortref/keys.go b/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