Merge pull request #83 from fredbi/fix-circular-absolute

optional absolute path in circular $ref

Author: casualjim

.golangci.yml

@@ -18,3 +18,4 @@ linters:
   disable:
     - maligned
     - unparam
+    - lll

expander.go

@@ -32,9 +32,10 @@ import (
 
 // ExpandOptions provides options for expand.
 type ExpandOptions struct {
-	RelativeBase    string
-	SkipSchemas     bool
-	ContinueOnError bool
+	RelativeBase        string
+	SkipSchemas         bool
+	ContinueOnError     bool
+	AbsoluteCircularRef bool
 }
 
 // ResolutionCache a cache for resolving urls
@@ -444,7 +445,7 @@ func normalizeFileRef(ref *Ref, relativeBase string) *Ref {
 		return &r
 	}
 
-	debugLog("normalizing %s against %s (%s)", ref.String(), relativeBase, ref.GetURL().String())
+	debugLog("normalizing %s against %s", ref.String(), relativeBase)
 
 	s := normalizePaths(ref.String(), relativeBase)
 	r, _ := NewRef(s)
@@ -607,25 +608,35 @@ func shouldStopOnError(err error, opts *ExpandOptions) bool {
 	return false
 }
 
-// ExpandSchema expands the refs in the schema object with reference to the root object
-// go-openapi/validate uses this function
-// notice that it is impossible to reference a json scema in a different file other than root
-func ExpandSchema(schema *Schema, root interface{}, cache ResolutionCache) error {
-	// Only save the root to a tmp file if it isn't nil.
-	var base string
+// baseForRoot loads in the cache the root document and produces a fake "root" base path entry
+// for further $ref resolution
+func baseForRoot(root interface{}, cache ResolutionCache) string {
+	// cache the root document to resolve $ref's
+	const rootBase = "root"
 	if root != nil {
-		base, _ = absPath("root")
+		base, _ := absPath(rootBase)
+		normalizedBase := normalizeAbsPath(base)
+		debugLog("setting root doc in cache at: %s", normalizedBase)
 		if cache == nil {
 			cache = resCache
 		}
-		cache.Set(normalizeAbsPath(base), root)
-		base = "root"
+		cache.Set(normalizedBase, root)
+		return rootBase
 	}
+	return ""
+}
 
+// ExpandSchema expands the refs in the schema object with reference to the root object
+// go-openapi/validate uses this function
+// notice that it is impossible to reference a json schema in a different file other than root
+func ExpandSchema(schema *Schema, root interface{}, cache ResolutionCache) error {
 	opts := &ExpandOptions{
-		RelativeBase:    base,
+		// when a root is specified, cache the root as an in-memory document for $ref retrieval
+		RelativeBase:    baseForRoot(root, cache),
 		SkipSchemas:     false,
 		ContinueOnError: false,
+		// when no base path is specified, remaining $ref (circular) are rendered with an absolute path
+		AbsoluteCircularRef: true,
 	}
 	return ExpandSchemaWithBasePath(schema, cache, opts)
 }
@@ -734,6 +745,7 @@ func expandSchema(target Schema, parentRefs []string, resolver *schemaLoader, ba
 	   otherwise the basePath should inherit the parent's */
 	// important: ID can be relative path
 	if target.ID != "" {
+		debugLog("schema has ID: %s", target.ID)
 		// handling the case when id is a folder
 		// remember that basePath has to be a file
 		refPath := target.ID
@@ -757,11 +769,13 @@ func expandSchema(target Schema, parentRefs []string, resolver *schemaLoader, ba
 			// this means there is a cycle in the recursion tree: return the Ref
 			// - circular refs cannot be expanded. We leave them as ref.
 			// - denormalization means that a new local file ref is set relative to the original basePath
-			debugLog("shortcut circular ref")
-			debugLog("basePath: %s", basePath)
-			debugLog("normalized basePath: %s", normalizedBasePath)
-			debugLog("normalized ref: %s", normalizedRef.String())
-			target.Ref = *denormalizeFileRef(normalizedRef, normalizedBasePath, resolver.context.basePath)
+			debugLog("shortcut circular ref: basePath: %s, normalizedPath: %s, normalized ref: %s",
+				basePath, normalizedBasePath, normalizedRef.String())
+			if !resolver.options.AbsoluteCircularRef {
+				target.Ref = *denormalizeFileRef(normalizedRef, normalizedBasePath, resolver.context.basePath)
+			} else {
+				target.Ref = *normalizedRef
+			}
 			return &target, nil
 		}
 
@@ -1015,19 +1029,40 @@ func transitiveResolver(basePath string, ref Ref, resolver *schemaLoader) (*sche
 	return resolver, nil
 }
 
+// ExpandResponseWithRoot expands a response based on a root document, not a fetchable document
+func ExpandResponseWithRoot(response *Response, root interface{}, cache ResolutionCache) error {
+	opts := &ExpandOptions{
+		RelativeBase:    baseForRoot(root, cache),
+		SkipSchemas:     false,
+		ContinueOnError: false,
+		// when no base path is specified, remaining $ref (circular) are rendered with an absolute path
+		AbsoluteCircularRef: true,
+	}
+	resolver, err := defaultSchemaLoader(root, opts, nil, nil)
+	if err != nil {
+		return err
+	}
+
+	return expandResponse(response, resolver, opts.RelativeBase)
+}
+
 // ExpandResponse expands a response based on a basepath
 // This is the exported version of expandResponse
 // all refs inside response will be resolved relative to basePath
 func ExpandResponse(response *Response, basePath string) error {
+	var specBasePath string
+	if basePath != "" {
+		specBasePath, _ = absPath(basePath)
+	}
 	opts := &ExpandOptions{
-		RelativeBase: basePath,
+		RelativeBase: specBasePath,
 	}
 	resolver, err := defaultSchemaLoader(nil, opts, nil, nil)
 	if err != nil {
 		return err
 	}
 
-	return expandResponse(response, resolver, basePath)
+	return expandResponse(response, resolver, opts.RelativeBase)
 }
 
 func derefResponse(response *Response, parentRefs []string, resolver *schemaLoader, basePath string) error {
@@ -1058,7 +1093,6 @@ func expandResponse(response *Response, resolver *schemaLoader, basePath string)
 	if response == nil {
 		return nil
 	}
-
 	parentRefs := []string{}
 	if err := derefResponse(response, parentRefs, resolver, basePath); shouldStopOnError(err, resolver.options) {
 		return err
@@ -1094,19 +1128,40 @@ func expandResponse(response *Response, resolver *schemaLoader, basePath string)
 	return nil
 }
 
+// ExpandParameterWithRoot expands a parameter based on a root document, not a fetchable document
+func ExpandParameterWithRoot(parameter *Parameter, root interface{}, cache ResolutionCache) error {
+	opts := &ExpandOptions{
+		RelativeBase:    baseForRoot(root, cache),
+		SkipSchemas:     false,
+		ContinueOnError: false,
+		// when no base path is specified, remaining $ref (circular) are rendered with an absolute path
+		AbsoluteCircularRef: true,
+	}
+	resolver, err := defaultSchemaLoader(root, opts, nil, nil)
+	if err != nil {
+		return err
+	}
+
+	return expandParameter(parameter, resolver, opts.RelativeBase)
+}
+
 // ExpandParameter expands a parameter based on a basepath
 // This is the exported version of expandParameter
 // all refs inside parameter will be resolved relative to basePath
 func ExpandParameter(parameter *Parameter, basePath string) error {
+	var specBasePath string
+	if basePath != "" {
+		specBasePath, _ = absPath(basePath)
+	}
 	opts := &ExpandOptions{
-		RelativeBase: basePath,
+		RelativeBase: specBasePath,
 	}
 	resolver, err := defaultSchemaLoader(nil, opts, nil, nil)
 	if err != nil {
 		return err
 	}
 
-	return expandParameter(parameter, resolver, basePath)
+	return expandParameter(parameter, resolver, opts.RelativeBase)
 }
 
 func derefParameter(parameter *Parameter, parentRefs []string, resolver *schemaLoader, basePath string) error {

expander_test.go

@@ -32,6 +32,10 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+var (
+	rex = regexp.MustCompile(`"\$ref":\s*"(.+)"`)
+)
+
 func jsonDoc(path string) (json.RawMessage, error) {
 	data, err := swag.LoadFromFileOrHTTP(path)
 	if err != nil {
@@ -286,6 +290,39 @@ func TestExportedResponseExpansion(t *testing.T) {
 	// assert.Equal(t, expected, resp)
 }
 
+func TestExpandResponseAndParamWithRoot(t *testing.T) {
+	specDoc, err := jsonDoc("fixtures/bugs/1614/gitea.json")
+	if !assert.NoError(t, err) {
+		t.FailNow()
+		return
+	}
+	var spec Swagger
+	_ = json.Unmarshal(specDoc, &spec)
+
+	// check responses with $ref
+	resp := spec.Paths.Paths["/admin/users"].Post.Responses.StatusCodeResponses[201]
+	err = ExpandResponseWithRoot(&resp, spec, nil)
+	assert.NoError(t, err)
+	jazon, _ := json.MarshalIndent(resp, "", " ")
+	m := rex.FindAllStringSubmatch(string(jazon), -1)
+	assert.Nil(t, m)
+
+	resp = spec.Paths.Paths["/admin/users"].Post.Responses.StatusCodeResponses[403]
+	err = ExpandResponseWithRoot(&resp, spec, nil)
+	assert.NoError(t, err)
+	jazon, _ = json.MarshalIndent(resp, "", " ")
+	m = rex.FindAllStringSubmatch(string(jazon), -1)
+	assert.Nil(t, m)
+
+	// check param with $ref
+	param := spec.Paths.Paths["/admin/users"].Post.Parameters[0]
+	err = ExpandParameterWithRoot(&param, spec, nil)
+	assert.NoError(t, err)
+	jazon, _ = json.MarshalIndent(param, "", " ")
+	m = rex.FindAllStringSubmatch(string(jazon), -1)
+	assert.Nil(t, m)
+}
+
 func TestIssue3(t *testing.T) {
 	spec := new(Swagger)
 	specDoc, err := jsonDoc("fixtures/expansion/overflow.json")
@@ -428,7 +465,6 @@ func Test_MoreCircular(t *testing.T) {
 
 	fixturePath := "fixtures/more_circulars/spec.json"
 	jazon := expandThisOrDieTrying(t, fixturePath)
-	rex := regexp.MustCompile(`"\$ref":\s*"(.+)"`)
 	m := rex.FindAllStringSubmatch(jazon, -1)
 	if assert.NotNil(t, m) {
 		for _, matched := range m {
@@ -440,7 +476,6 @@ func Test_MoreCircular(t *testing.T) {
 
 	fixturePath = "fixtures/more_circulars/spec2.json"
 	jazon = expandThisOrDieTrying(t, fixturePath)
-	rex = regexp.MustCompile(`"\$ref":\s*"(.+)"`)
 	m = rex.FindAllStringSubmatch(jazon, -1)
 	if assert.NotNil(t, m) {
 		for _, matched := range m {
@@ -452,7 +487,6 @@ func Test_MoreCircular(t *testing.T) {
 
 	fixturePath = "fixtures/more_circulars/spec3.json"
 	jazon = expandThisOrDieTrying(t, fixturePath)
-	rex = regexp.MustCompile(`"\$ref":\s*"(.+)"`)
 	m = rex.FindAllStringSubmatch(jazon, -1)
 	if assert.NotNil(t, m) {
 		for _, matched := range m {
@@ -464,7 +498,6 @@ func Test_MoreCircular(t *testing.T) {
 
 	fixturePath = "fixtures/more_circulars/spec4.json"
 	jazon = expandThisOrDieTrying(t, fixturePath)
-	rex = regexp.MustCompile(`"\$ref":\s*"(.+)"`)
 	m = rex.FindAllStringSubmatch(jazon, -1)
 	if assert.NotNil(t, m) {
 		for _, matched := range m {
@@ -481,7 +514,6 @@ func Test_Issue957(t *testing.T) {
 	if assert.NotEmpty(t, jazon) {
 		assert.NotContainsf(t, jazon, "fixture-957.json#/",
 			"expected %s to be expanded with stripped circular $ref", fixturePath)
-		rex := regexp.MustCompile(`"\$ref":\s*"(.+)"`)
 		m := rex.FindAllStringSubmatch(jazon, -1)
 		if assert.NotNil(t, m) {
 			for _, matched := range m {
@@ -499,7 +531,6 @@ func Test_Bitbucket(t *testing.T) {
 
 	fixturePath := "fixtures/more_circulars/bitbucket.json"
 	jazon := expandThisOrDieTrying(t, fixturePath)
-	rex := regexp.MustCompile(`"\$ref":\s*"(.+)"`)
 	m := rex.FindAllStringSubmatch(jazon, -1)
 	if assert.NotNil(t, m) {
 		for _, matched := range m {
@@ -514,7 +545,6 @@ func Test_ExpandJSONSchemaDraft4(t *testing.T) {
 	fixturePath := filepath.Join("schemas", "jsonschema-draft-04.json")
 	jazon := expandThisSchemaOrDieTrying(t, fixturePath)
 	// assert all $ref maches  "$ref": "http://json-schema.org/draft-04/something"
-	rex := regexp.MustCompile(`"\$ref":\s*"(.+)"`)
 	m := rex.FindAllStringSubmatch(jazon, -1)
 	if assert.NotNil(t, m) {
 		for _, matched := range m {
@@ -529,7 +559,6 @@ func Test_ExpandSwaggerSchema(t *testing.T) {
 	fixturePath := filepath.Join("schemas", "v2", "schema.json")
 	jazon := expandThisSchemaOrDieTrying(t, fixturePath)
 	// assert all $ref maches  "$ref": "#/definitions/something"
-	rex := regexp.MustCompile(`"\$ref":\s*"(.+)"`)
 	m := rex.FindAllStringSubmatch(jazon, -1)
 	if assert.NotNil(t, m) {
 		for _, matched := range m {
@@ -1412,6 +1441,77 @@ func TestResolveForTransitiveRefs(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+const (
+	withoutSchemaID = "removed"
+	withSchemaID    = "schema"
+)
+
+func TestExpandSchemaWithRoot(t *testing.T) {
+	root := new(Swagger)
+	_ = json.Unmarshal(PetStoreJSONMessage, root)
+
+	// 1. remove ID from root definition
+	origPet := root.Definitions["Pet"]
+	newPet := origPet
+	newPet.ID = ""
+	root.Definitions["Pet"] = newPet
+	expandRootWithID(t, root, withoutSchemaID)
+
+	// 2. put back ID in Pet definition
+	// nested $ref should fail
+	//Debug = true
+	root.Definitions["Pet"] = origPet
+	expandRootWithID(t, root, withSchemaID)
+}
+
+func expandRootWithID(t *testing.T, root *Swagger, testcase string) {
+	t.Logf("case: expanding $ref to schema without ID, with nested $ref with %s ID", testcase)
+	sch := &Schema{
+		SchemaProps: SchemaProps{
+			Ref: MustCreateRef("#/definitions/newPet"),
+		},
+	}
+	err := ExpandSchema(sch, root, nil)
+	if testcase == withSchemaID {
+		assert.Errorf(t, err, "expected %s NOT to expand properly because of the ID in the parent schema", sch.Ref.String())
+	} else {
+		assert.NoErrorf(t, err, "expected %s to expand properly", sch.Ref.String())
+	}
+	if Debug {
+		bbb, _ := json.MarshalIndent(sch, "", " ")
+		t.Log(string(bbb))
+	}
+
+	t.Log("case: expanding $ref to schema without nested $ref")
+	sch = &Schema{
+		SchemaProps: SchemaProps{
+			Ref: MustCreateRef("#/definitions/Category"),
+		},
+	}
+	err = ExpandSchema(sch, root, nil)
+	assert.NoErrorf(t, err, "expected %s to expand properly", sch.Ref.String())
+	if Debug {
+		bbb, _ := json.MarshalIndent(sch, "", " ")
+		t.Log(string(bbb))
+	}
+	t.Logf("case: expanding $ref to schema with %s ID and nested $ref", testcase)
+	sch = &Schema{
+		SchemaProps: SchemaProps{
+			Ref: MustCreateRef("#/definitions/Pet"),
+		},
+	}
+	err = ExpandSchema(sch, root, nil)
+	if testcase == withSchemaID {
+		assert.Errorf(t, err, "expected %s NOT to expand properly because of the ID in the parent schema", sch.Ref.String())
+	} else {
+		assert.NoErrorf(t, err, "expected %s to expand properly", sch.Ref.String())
+	}
+	if Debug {
+		bbb, _ := json.MarshalIndent(sch, "", " ")
+		t.Log(string(bbb))
+	}
+}
+
 // PetStoreJSONMessage json raw message for Petstore20
 var PetStoreJSONMessage = json.RawMessage([]byte(PetStore20))