diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index e1a95c7..139caf4 100644 --- a/.github/workflows/go-test.yml +++ b/.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 diff --git a/fixtures/bugs/1898/swagger.json b/fixtures/bugs/1898/swagger.json new file mode 100644 index 0000000..158b35e --- /dev/null +++ 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 index 6e5b8d5..1a0a3a4 100644 --- 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 index c47e75a..c7d7938 100644 --- a/flatten_name.go +++ b/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 } diff --git a/flatten_test.go b/flatten_test.go index df1f10a..bb50357 100644 --- a/flatten_test.go +++ b/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) diff --git a/go.mod b/go.mod index 5d3b660..fc65d04 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 5337a01..4b1df74 100644 --- a/go.sum +++ b/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= diff --git a/internal/flatten/replace/replace.go b/internal/flatten/replace/replace.go index 545483b..c0f43e7 100644 --- 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: 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 } } diff --git a/internal/flatten/sortref/keys.go b/internal/flatten/sortref/keys.go index 18e552e..ac80fc2 100644 --- 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