Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Dec 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/go-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
105 changes: 105 additions & 0 deletions fixtures/bugs/1898/swagger.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
6 changes: 6 additions & 0 deletions flatten.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
14 changes: 9 additions & 5 deletions flatten_name.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand All @@ -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
}

Expand Down
27 changes: 27 additions & 0 deletions flatten_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
38 changes: 31 additions & 7 deletions internal/flatten/replace/replace.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package replace

import (
"encoding/json"
"fmt"
"net/url"
"os"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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()))

Expand All @@ -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()))
Expand All @@ -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
}
}

Expand Down
2 changes: 1 addition & 1 deletion internal/flatten/sortref/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading