kin-openapi icon indicating copy to clipboard operation
kin-openapi copied to clipboard

Local reference inside relative file reference missing path information

Open mikesep opened this issue 4 years ago • 6 comments

Hi! I'm trying to split my OpenAPI spec across multiple files, and I'm not entirely sure whether what I'm doing should or shouldn't be supported.

Here's my testcase:

# main.yaml
openapi: "3.0.0"
info:
  title: "test file"
  version: "n/a"
paths:
  /testpath:
    $ref: "testpath.yaml#/paths/~1testpath"
# testpath.yaml
paths:
  /testpath:
    get:
      responses:
        "200":
          $ref: "#/components/responses/testpath_200_response"

components:
  responses:
    testpath_200_response:
      description: a custom response
      content:
        application/json:
          schema:
            type: string

main.yaml is the root of the spec. The definition of the /testpath path is in testpath.yaml, and it refers to a named response in that same file.

Here's a small test driver that parses the spec and prints it out in JSON:

package main

import (
	"fmt"

	"github.com/getkin/kin-openapi/openapi3"
)

func main() {
	sl := openapi3.NewSwaggerLoader()
	sl.IsExternalRefsAllowed = true
	s, err := sl.LoadSwaggerFromFile("main.yaml")
	if err != nil {
		panic(err)
	}

	bs, err := s.MarshalJSON()
	if err != nil {
		panic(err)
	}

	fmt.Printf("%s\n", bs)
}

When I run it, I get the following:

$ go run main.go | jq
{
  "components": {},
  "info": {
    "title": "test file",
    "version": "n/a"
  },
  "openapi": "3.0.0",
  "paths": {
    "/testpath": {
      "get": {
        "responses": {
          "200": {
            "$ref": "#/components/responses/testpath_200_response"
          }
        }
      }
    }
  }
}

The reference to testpath_200_response does not have testpath.yaml at the front, so it seems to be referring to a component in the same document, but components is empty in this doc. I think the path where the reference originated is getting lost along the way.

The language in the OpenAPI spec is fairly dense, so I'm not 100% sure whether I'm doing this right. That being said, swagger-cli validate main.yaml says that it's valid, and using Swagger UI on main.yaml follows the reference to the response type.

Thanks for reading this far, and thanks for all the work you've done on this project. 😃

mikesep avatar Apr 17 '21 00:04 mikesep

As shown in https://github.com/getkin/kin-openapi/pull/342 the refs are resolved.

Serialization does not merge the referenced contents into a single spec though, if that's what you want there's https://github.com/getkin/kin-openapi/issues/277

fenollp avatar Apr 17 '21 09:04 fenollp

Hi @fenollp, thanks for taking a look so quickly!

Value.Type for this object is showing accurate information, as you've demonstrated. 👍

I don't think I'm looking for no refs at all, which seems to be where #277 ended up.

I'm trying to get deepmap/oapi-codegen to generate code for my split spec, but I think it doesn't have enough information from kin-openapi to do what I hope.

I think oapi-codegen use references for types where it can. It lets me generate types for testpath.yaml into their own package (say, testpath) and then supply a mapping from file name to package when I generate things from main.yaml. The import mappings let it see testpath.yaml#/components/responses/foo and turn it into testpath.Foo in Go code. I suspect that because testpath.yaml is missing from the reference, oapi-codegen doesn't add a package prefix to the Go code it generates (which then refers to Foo instead of testpath.Foo).

I hope that makes it a bit clearer. I don't see another attribute in the struct tree that someone could use to tell that this type comes from another file, so I'm still wondering if adding the relative path to the fragment reference would be the correct thing to do here.

mikesep avatar Apr 17 '21 15:04 mikesep

I agree that kin-openapi requires modification in order to keep track of which files definitions come from but I don't have a clear API in mind. Do you?

This info could be stored in ExtensionProps however, but only if you can afford to add this to each of your YAML files.

fenollp avatar Apr 17 '21 16:04 fenollp

I've been digging through the swagger loader code to better understand what's going on. While I won't say I understand it perfectly, I think I get some of it.

Not sure what the best name is for this data, which is the ref path written with respect to the root document.

For an API, I think having an rooted ref name in addition to the original ref name could be good. I'm not sure if this means adding another string field to structs wherever Ref string appears or whether it would be better to make Ref a struct with two members. Adding a new field is certainly more backwards-compatible.

In trying to figure out how I would rewrite the refs, I found that documentPath tends to have the path up to the doc but not including it, so it's hard to tell which doc you're currently working on. The Swagger struct doesn't contain this information either. I might try to see whether it's feasible to keep the filename in the documentPath args/vars as a way to carry that data forward.

mikesep avatar Apr 21 '21 12:04 mikesep

I'm working on this documentPath issue myself and should have a fix soon. It will be the path to the resource being visited.

Back to your point: let's have this as a sibling field to Ref and Value for now, it'd make the diff smaller. I'm not sure this Source field should be a string. An interface could be interesting (there is another issue asking to cache access to remote parts of a document).

fenollp avatar Apr 21 '21 14:04 fenollp

Fixed my documentPath issue. The test I derived from your issue now passes: https://github.com/getkin/kin-openapi/pull/342

Feel free to continue discussing that .Source field thing here or to open a dedicated issue :)

fenollp avatar Apr 23 '21 11:04 fenollp