Skip to content

Commit

Permalink
fix(expand): parameters & responses should properly follow remote doc…
Browse files Browse the repository at this point in the history
… resolution when SKipSchema

* fixes go-openapi#182
* contributes go-swagger/go-swagger#2743

Signed-off-by: Frederic BIDON <[email protected]>
  • Loading branch information
fredbi committed Jan 8, 2024
1 parent b445199 commit 3121b63
Show file tree
Hide file tree
Showing 21 changed files with 225 additions and 24 deletions.
56 changes: 32 additions & 24 deletions expander.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,19 @@ func expandSchema(target Schema, parentRefs []string, resolver *schemaLoader, ba
}

if target.Ref.String() != "" {
return expandSchemaRef(target, parentRefs, resolver, basePath)
if !resolver.options.SkipSchemas {
return expandSchemaRef(target, parentRefs, resolver, basePath)
}

// when "expand" with SkipSchema, we just rebase the existing $ref without replacing
// the full schema.
rebasedRef, err := NewRef(normalizeURI(target.Ref.String(), basePath))
if err != nil {
return nil, err
}
target.Ref = denormalizeRef(&rebasedRef, resolver.context.basePath, resolver.context.rootID)

return &target, nil
}

for k := range target.Definitions {
Expand Down Expand Up @@ -525,21 +537,25 @@ func getRefAndSchema(input interface{}) (*Ref, *Schema, error) {
}

func expandParameterOrResponse(input interface{}, resolver *schemaLoader, basePath string) error {
ref, _, err := getRefAndSchema(input)
ref, sch, err := getRefAndSchema(input)
if err != nil {
return err
}

if ref == nil {
if ref == nil && sch == nil { // nothing to do
return nil
}

parentRefs := make([]string, 0, 10)
if err = resolver.deref(input, parentRefs, basePath); resolver.shouldStopOnError(err) {
return err
if ref != nil {
// dereference this $ref
if err = resolver.deref(input, parentRefs, basePath); resolver.shouldStopOnError(err) {
return err
}

ref, sch, _ = getRefAndSchema(input)
}

ref, sch, _ := getRefAndSchema(input)
if ref.String() != "" {
transitiveResolver := resolver.transitiveResolver(basePath, *ref)
basePath = resolver.updateBasePath(transitiveResolver, basePath)
Expand All @@ -551,6 +567,7 @@ func expandParameterOrResponse(input interface{}, resolver *schemaLoader, basePa
if ref != nil {
*ref = Ref{}
}

return nil
}

Expand All @@ -560,38 +577,29 @@ func expandParameterOrResponse(input interface{}, resolver *schemaLoader, basePa
return ern
}

switch {
case resolver.isCircular(&rebasedRef, basePath, parentRefs...):
if resolver.isCircular(&rebasedRef, basePath, parentRefs...) {
// this is a circular $ref: stop expansion
if !resolver.options.AbsoluteCircularRef {
sch.Ref = denormalizeRef(&rebasedRef, resolver.context.basePath, resolver.context.rootID)
} else {
sch.Ref = rebasedRef
}
case !resolver.options.SkipSchemas:
// schema expanded to a $ref in another root
sch.Ref = rebasedRef
debugLog("rebased to: %s", sch.Ref.String())
default:
// skip schema expansion but rebase $ref to schema
sch.Ref = denormalizeRef(&rebasedRef, resolver.context.basePath, resolver.context.rootID)
}
}

// $ref expansion or rebasing is performed by expandSchema below
if ref != nil {
*ref = Ref{}
}

// expand schema
if !resolver.options.SkipSchemas {
s, err := expandSchema(*sch, parentRefs, resolver, basePath)
if resolver.shouldStopOnError(err) {
return err
}
if s == nil {
// guard for when continuing on error
return nil
}
// yes, we do it even if options.SkipSchema is true: we have to go down that rabbit hole and rebase nested $ref)
s, err := expandSchema(*sch, parentRefs, resolver, basePath)
if resolver.shouldStopOnError(err) {
return err
}

if s != nil { // guard for when continuing on error
*sch = *s
}

Expand Down
7 changes: 7 additions & 0 deletions fixtures/bugs/2743/not-working/minimal.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
swagger: '2.0'
info:
version: 0.0.0
title: Simple API
paths:
/bar:
$ref: 'swagger/paths/bar.yml'
11 changes: 11 additions & 0 deletions fixtures/bugs/2743/not-working/spec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
swagger: '2.0'
info:
version: 0.0.0
title: Simple API
paths:
/foo:
$ref: 'swagger/paths/foo.yml'
/bar:
$ref: 'swagger/paths/bar.yml'
/nested:
$ref: 'swagger/paths/nested.yml#/response'
9 changes: 9 additions & 0 deletions fixtures/bugs/2743/not-working/swagger/definitions.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
ErrorPayload:
title: Error Payload
required:
- errors
properties:
errors:
type: array
items:
$ref: './items.yml#/ErrorDetailsItem'
15 changes: 15 additions & 0 deletions fixtures/bugs/2743/not-working/swagger/items.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
ErrorDetailsItem:
title: Error details item
description: Represents an item of the list of details of an error.
required:
- message
- code
properties:
message:
type: string
code:
type: string
details:
type: array
items:
type: string
8 changes: 8 additions & 0 deletions fixtures/bugs/2743/not-working/swagger/paths/bar.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
get:
responses:
200:
description: OK
schema:
type: array
items:
$ref: '../user/index.yml#/User' ## this doesn't work
8 changes: 8 additions & 0 deletions fixtures/bugs/2743/not-working/swagger/paths/foo.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
get:
responses:
200:
description: OK
500:
description: OK
schema:
$ref: '../definitions.yml#/ErrorPayload'
6 changes: 6 additions & 0 deletions fixtures/bugs/2743/not-working/swagger/paths/index.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/foo:
$ref: ./foo.yml
/bar:
$ref: ./bar.yml
/nested:
$ref: ./nested.yml#/response
14 changes: 14 additions & 0 deletions fixtures/bugs/2743/not-working/swagger/paths/nested.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
response:
get:
responses:
200:
description: OK
schema:
$ref: '#/definitions/SameFileReference'

definitions:
SameFileReference:
type: object
properties:
name:
type: string
2 changes: 2 additions & 0 deletions fixtures/bugs/2743/not-working/swagger/user/index.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
User:
$ref: './model.yml'
4 changes: 4 additions & 0 deletions fixtures/bugs/2743/not-working/swagger/user/model.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type: object
properties:
name:
type: string
11 changes: 11 additions & 0 deletions fixtures/bugs/2743/working/spec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
swagger: '2.0'
info:
version: 0.0.0
title: Simple API
paths:
/foo:
$ref: 'swagger/paths/foo.yml'
/bar:
$ref: 'swagger/paths/bar.yml'
/nested:
$ref: 'swagger/paths/nested.yml#/response'
9 changes: 9 additions & 0 deletions fixtures/bugs/2743/working/swagger/definitions.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
ErrorPayload:
title: Error Payload
required:
- errors
properties:
errors:
type: array
items:
$ref: './items.yml#/ErrorDetailsItem'
15 changes: 15 additions & 0 deletions fixtures/bugs/2743/working/swagger/items.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
ErrorDetailsItem:
title: Error details item
description: Represents an item of the list of details of an error.
required:
- message
- code
properties:
message:
type: string
code:
type: string
details:
type: array
items:
type: string
8 changes: 8 additions & 0 deletions fixtures/bugs/2743/working/swagger/paths/bar.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
get:
responses:
200:
description: OK
schema:
type: array
items:
$ref: './swagger/user/index.yml#/User' ## this works
8 changes: 8 additions & 0 deletions fixtures/bugs/2743/working/swagger/paths/foo.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
get:
responses:
200:
description: OK
500:
description: OK
schema:
$ref: '../definitions.yml#/ErrorPayload'
6 changes: 6 additions & 0 deletions fixtures/bugs/2743/working/swagger/paths/index.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/foo:
$ref: ./foo.yml
/bar:
$ref: ./bar.yml
/nested:
$ref: ./nested.yml#/response
14 changes: 14 additions & 0 deletions fixtures/bugs/2743/working/swagger/paths/nested.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
response:
get:
responses:
200:
description: OK
schema:
$ref: '#/definitions/SameFileReference'

definitions:
SameFileReference:
type: object
properties:
name:
type: string
2 changes: 2 additions & 0 deletions fixtures/bugs/2743/working/swagger/user/index.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
User:
$ref: './model.yml'
4 changes: 4 additions & 0 deletions fixtures/bugs/2743/working/swagger/user/model.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type: object
properties:
name:
type: string
32 changes: 32 additions & 0 deletions spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,38 @@ import (
)

// Test unitary fixture for dev and bug fixing

func TestSpec_Issue2743(t *testing.T) {
t.Run("should expand but produce unresolvable $ref", func(t *testing.T) {
path := filepath.Join("fixtures", "bugs", "2743", "working", "spec.yaml")
sp := loadOrFail(t, path)
require.NoError(t,
spec.ExpandSpec(sp, &spec.ExpandOptions{RelativeBase: path, SkipSchemas: true, PathLoader: testLoader}),
)

t.Run("all $ref do not resolve when expanding again", func(t *testing.T) {
err := spec.ExpandSpec(sp, &spec.ExpandOptions{RelativeBase: path, SkipSchemas: false, PathLoader: testLoader})
require.Error(t, err)
require.ErrorContains(t, err, "swagger/paths/swagger/user/index.yml: no such file or directory")
})
})

t.Run("should expand and produce resolvable $ref", func(t *testing.T) {
path := filepath.Join("fixtures", "bugs", "2743", "not-working", "spec.yaml")
sp := loadOrFail(t, path)
require.NoError(t,
spec.ExpandSpec(sp, &spec.ExpandOptions{RelativeBase: path, SkipSchemas: true, PathLoader: testLoader}),
)

t.Run("all $ref properly reolve when expanding again", func(t *testing.T) {
require.NoError(t,
spec.ExpandSpec(sp, &spec.ExpandOptions{RelativeBase: path, SkipSchemas: false, PathLoader: testLoader}),
)
require.NotContainsf(t, asJSON(t, sp), "$ref", "all $ref's should have been expanded properly")
})
})
}

func TestSpec_Issue1429(t *testing.T) {
path := filepath.Join("fixtures", "bugs", "1429", "swagger.yaml")

Expand Down

0 comments on commit 3121b63

Please sign in to comment.