Skip to content

Commit

Permalink
Correctly resolve path of yaml resource if double referenced.
Browse files Browse the repository at this point in the history
Where previously following path was resolved `testdata/components/models/error.yaml`, missing `recursiveRef` folder.
  • Loading branch information
d-sauer committed Sep 23, 2022
1 parent fa5d9a9 commit 301c5ca
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 9 deletions.
16 changes: 8 additions & 8 deletions openapi3/internalize_refs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package openapi3

import (
"context"
"io/ioutil"
"os"
"regexp"
"testing"

Expand Down Expand Up @@ -41,25 +41,25 @@ func TestInternalizeRefs(t *testing.T) {
err = doc.Validate(ctx)
require.NoError(t, err, "validating internalized spec")

data, err := doc.MarshalJSON()
actual, err := doc.MarshalJSON()
require.NoError(t, err, "marshalling internalized spec")

// run a static check over the file, making sure each occurence of a
// reference is followed by a #
numRefs := len(regexpRef.FindAll(data, -1))
numInternalRefs := len(regexpRefInternal.FindAll(data, -1))
numRefs := len(regexpRef.FindAll(actual, -1))
numInternalRefs := len(regexpRefInternal.FindAll(actual, -1))
require.Equal(t, numRefs, numInternalRefs, "checking all references are internal")

// load from data, but with the path set to the current directory
doc2, err := sl.LoadFromData(data)
// load from actual, but with the path set to the current directory
doc2, err := sl.LoadFromData(actual)
require.NoError(t, err, "reloading spec")
err = doc2.Validate(ctx)
require.NoError(t, err, "validating reloaded spec")

// compare with expected
data0, err := ioutil.ReadFile(test.filename + ".internalized.yml")
expected, err := os.ReadFile(test.filename + ".internalized.yml")
require.NoError(t, err)
require.JSONEq(t, string(data), string(data0))
require.JSONEq(t, string(expected), string(actual))
})
}
}
9 changes: 8 additions & 1 deletion openapi3/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,14 @@ func join(basePath *url.URL, relativePath *url.URL) (*url.URL, error) {
if err != nil {
return nil, fmt.Errorf("cannot copy path: %q", basePath.String())
}
newPath.Path = path.Join(path.Dir(newPath.Path), relativePath.Path)

// check if basePath is reference to the file
if strings.LastIndex(basePath.Path, ".") > strings.LastIndex(basePath.Path, "/") {
newPath.Path = path.Join(path.Dir(newPath.Path), relativePath.Path)
} else {
newPath.Path = path.Join(newPath.Path, relativePath.Path)
}

return newPath, nil
}

Expand Down
39 changes: 39 additions & 0 deletions openapi3/loader_recursive_ref_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package openapi3

import (
"net/url"
"testing"

"github.com/stretchr/testify/require"
Expand All @@ -14,6 +15,44 @@ func TestLoaderSupportsRecursiveReference(t *testing.T) {
err = doc.Validate(loader.Context)
require.NoError(t, err)
require.Equal(t, "bar", doc.Paths["/foo"].Get.Responses.Get(200).Value.Content.Get("application/json").Schema.Value.Properties["foo2"].Value.Properties["foo"].Value.Properties["bar"].Value.Example)
require.Equal(t, "ErrorDetails", doc.Paths["/foo"].Get.Responses.Get(400).Value.Content.Get("application/json").Schema.Value.Title)
require.Equal(t, "ErrorDetails", doc.Paths["/double-ref-foo"].Get.Responses.Get(400).Value.Content.Get("application/json").Schema.Value.Title)
}

func TestResolvePath(t *testing.T) {
// relative path of folder
var b = &url.URL{Path: "testdata/recursiveRef"}
var c = &url.URL{Path: "./components/models/error.yaml"}
u, err := resolvePath(b, c)

require.NoError(t, err)
require.Equal(t, &url.URL{Path: "testdata/recursiveRef/components/models/error.yaml"}, u)

// relative path of specification file
b = &url.URL{Path: "testdata/recursiveRef/openapi.yaml"}
c = &url.URL{Path: "./components/models/error.yaml"}
u, err = resolvePath(b, c)

require.NoError(t, err)
require.Equal(t, &url.URL{Path: "testdata/recursiveRef/components/models/error.yaml"}, u)

// URL point to root of the resource
b, err = url.Parse("https://testdata/recursiveRef")
c = &url.URL{Path: "./components/models/error.yaml"}
u, err = resolvePath(b, c)

require.NoError(t, err)
var expected, _ = url.Parse("https://testdata/recursiveRef/components/models/error.yaml")
require.Equal(t, expected, u)

// URL point to the resource
b, err = url.Parse("https://testdata/recursiveRef/openapi.yaml")
c = &url.URL{Path: "./components/models/error.yaml"}
u, err = resolvePath(b, c)

require.NoError(t, err)
expected, _ = url.Parse("https://testdata/recursiveRef/components/models/error.yaml")
require.Equal(t, expected, u)
}

func TestIssue447(t *testing.T) {
Expand Down
2 changes: 2 additions & 0 deletions openapi3/testdata/recursiveRef/components/models/error.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
type: object
title: ErrorDetails
16 changes: 16 additions & 0 deletions openapi3/testdata/recursiveRef/openapi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ info:
paths:
/foo:
$ref: ./paths/foo.yml
/double-ref-foo:
get:
summary: Double ref response
description: Reference response with double reference.
responses:
"400":
$ref: "#/components/responses/400"
components:
schemas:
Foo:
Expand All @@ -15,3 +22,12 @@ components:
$ref: ./components/Bar.yml
Cat:
$ref: ./components/Cat.yml
Error:
$ref: ./components/models/error.yaml
responses:
"400":
description: 400 Bad Request
content:
application/json:
schema:
$ref: "#/components/schemas/Error"
34 changes: 34 additions & 0 deletions openapi3/testdata/recursiveRef/openapi.yml.internalized.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,27 @@
}
}
},
"responses": {
"400": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/Error"
}
}
},
"description": "400 Bad Request"
}
},
"schemas": {
"Bar": {
"example": "bar",
"type": "string"
},
"Error":{
"title":"ErrorDetails",
"type":"object"
},
"Foo": {
"properties": {
"bar": {
Expand All @@ -30,6 +46,10 @@
},
"type": "object"
},
"error":{
"title":"ErrorDetails",
"type":"object"
},
"Cat": {
"properties": {
"cat": {
Expand All @@ -46,6 +66,17 @@
},
"openapi": "3.0.3",
"paths": {
"/double-ref-foo": {
"get": {
"description": "Reference response with double reference.",
"responses": {
"400": {
"$ref": "#/components/responses/400"
}
},
"summary": "Double ref response"
}
},
"/foo": {
"get": {
"responses": {
Expand All @@ -63,6 +94,9 @@
}
},
"description": "OK"
},
"400": {
"$ref": "#/components/responses/400"
}
}
},
Expand Down
2 changes: 2 additions & 0 deletions openapi3/testdata/recursiveRef/paths/foo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ get:
properties:
foo2:
$ref: ../openapi.yml#/components/schemas/Foo2
"400":
$ref: "../openapi.yml#/components/responses/400"

0 comments on commit 301c5ca

Please sign in to comment.