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

Can't access Parameter Reference #25

Closed
TristanSpeakEasy opened this issue Dec 1, 2022 · 13 comments
Closed

Can't access Parameter Reference #25

TristanSpeakEasy opened this issue Dec 1, 2022 · 13 comments

Comments

@TristanSpeakEasy
Copy link
Contributor

Hi it doesn't seem like I can access the reference string of a parameter or check its from a reference.

For example:

openapi: 3.0.3
info:
  title: Test
  version: 0.1.0
paths:
  /test:
    get:
      operationId: getTest
      parameters:
        - $ref: '#/components/parameters/test'
      responses:
        "200":
          description: OK
components:
  parameters:
    test:
      name: test
      in: query
      required: true
      schema:
        type: string
        enum:
          - test
          - test2

I can't figure out how to get #/components/parameters/test and determine this parameter is a reference.

I would expect to be able to get the reference for at least each component type listed here https://swagger.io/docs/specification/components/#structure

@daveshanley
Copy link
Member

Hi Tristan,

Everything that isn't a schema is auto-resolved, as in it's pulled into the model automatically. This was a design choice as part of the model. The 'reference' no longer exists (for everything that isn't a schema).

The index may help here; however, it keeps track of every reference found. The GetAllParameters() method should return all the original references.

https://github.com/pb33f/libopenapi/blob/main/index/spec_index.go#L425

@TristanSpeakEasy
Copy link
Contributor Author

How would I know though that a parameter I am iterating through in the document was a reference and not inline in the original document?

For example if two different operations had parameters with the same name and one was a reference and the other is defined inline and that name matched a parameter in the components how do I determine which one was originally a reference or not?

@daveshanley
Copy link
Member

The only way to tell would be to check the KeyNode (from the low model) line/col reference to see if it can match one of the line/col references found in the index. The ValueNode is replaced with the actual value node of the reference vs the string reference representation.

From the model perspective, inline vs. references should not matter because, once resolved, they are the same thing. When you use the resolver to resolve a doc, the node structure underneath is modified to represent this.

Schemas are treated differently because they are extraordinarily prone to circular references through polymorphism and recursive properties.

A new feature could be added to track the original reference string in the low model. It might a bit for me to get to it, however.

@TristanSpeakEasy
Copy link
Contributor Author

TristanSpeakEasy commented Dec 1, 2022

For my use case I am generating SDKs from the OpenAPI documents and a core part of that is being able to identify whether types are "shared" ie defined as a reference or defined inline so I don't duplicate types while generating, without ease of access to that knowledge wherever a reference can be used, the job to determine if the types are shared becomes quite tough (as I will need to do a lookup into the original doc as suggested or try to validate schemas are equal)

@daveshanley
Copy link
Member

I see.

I did start to build out this feature; you can see the start of it at:

https://github.com/pb33f/libopenapi/blob/main/datamodel/low/reference.go#L89

However, I don't think I got very far. I am not sure if this works yet - if I did, would it be what you're looking for?

@TristanSpeakEasy
Copy link
Contributor Author

Yeah I would need to know if it was a reference and what the "$ref" value actually was ie "#/components/parameters/test" as I use that string to determine the type of component and the name of the ref.

I was just looking at how I could achieve this at the moment with the current version and I can't figure out how to get a KeyNode from a v3.Parameter as this is an item in an array.

I haven't used the yaml.Node stuff before so my knowledge is a bit lacking (will make sure to read up on it) but it looks like a Parameter like this is built from a yaml.Node should that be stored in the low level so you can access the line number of where that particular item lives in the document? So for example if I discover I can't handle a parameter for some reason (we don't support all openapi features in our SDK generator), it would be great to surface back to the use the line that parameter was defined in their document.

daveshanley added a commit that referenced this issue Dec 3, 2022
When building a document, everything that IS NOT a schema is auto-resolved in the model, this is very convenient because it creates a nice tree to explore and there is no need to perform lookups to when using `$ref` instead of inline definitions.

The issue is however being able to determine if the node was originally a reference or not, that data was lost, including the name of the reference used. This use case surfaced in issue #25, where the need to know what is and what is not referenced has different requirements for different applications.

This update now tracks that data and makes it available in `NodeReference` and `ValueReference` for every property.

Signed-off-by: Dave Shanley <dave@quobix.com>
daveshanley added a commit that referenced this issue Dec 3, 2022
Forgot a section of code to add new feature to, just so happened to be the one I wasn't testing.

Signed-off-by: Dave Shanley <dave@quobix.com>
@daveshanley
Copy link
Member

daveshanley commented Dec 3, 2022

I have extended the low level model to support tracking anything that is a reference. Two new properties are available in low.NodeReference and low.ValueReference. They are Reference (which holds the value the node initially pointed to), and IsReference which is a bool value that will be true if the object was a reference.

This is how to use it.

var data = `
openapi: "3.1"
components:
  parameters:
    Param1:
      description: "I am a param"
paths:
  /something:
    get:
      parameters:
        - $ref: '#/components/parameters/Param1'`

  doc, err := libopenapi.NewDocument([]byte(data))
  if err != nil {
      panic(err)
  }

  result, errs := doc.BuildV3Model()
  if len(errs) > 0 {
      panic(errs)
  }

  // extract operation.
  operation := result.Model.Paths.PathItems["/something"].Get

  // extract params.
  params := operation.Parameters

  // get low level params
  paramsLow := operation.GoLow().Parameters

  // extract high level param from op.
  param1High := params[0]

  // extract low level param from op.
  param1Low := paramsLow.Value[0]

  // print it out.
  fmt.Printf("param1: %s, is reference? %t, original reference %s",
      param1High.Description, param1Low.IsReference, param1Low.Reference)
 

This will print:

param1: I am a param, is reference? true, original reference #/components/parameters/Param1

Let me know how this works for you.

@TristanSpeakEasy
Copy link
Contributor Author

TristanSpeakEasy commented Dec 5, 2022

Hi Dave, thanks for taking a look at this, this will definitely help me move forward but the API is a little awkward here as I will have to pass both low and high level arguments to various functions.

In an ideal world I could do something like: op.Parameters[0].GoLow().IsReference as I am generally trying to iterate through a document using the high API and then just going low in the few places I need to, to access additional information

@daveshanley
Copy link
Member

daveshanley commented Dec 5, 2022

I see what you're doing. Hmmm OK, I will think about this some more. There are a few ways to make this happen, and none of them are particularly lightweight however, as it means building IsReference() into all low-level models.

@TristanSpeakEasy
Copy link
Contributor Author

TristanSpeakEasy commented Dec 12, 2022

Just an example of what I needed to do to workaround this:

type parameter struct {
	v3High.Parameter
	low low.ValueReference[*v3Low.Parameter]
}

func getParameters(high []*v3High.Parameter, low low.NodeReference[[]low.ValueReference[*v3Low.Parameter]]) []parameter {
	params := []parameter{}

	for i, p := range high {
		params = append(params, parameter{
			Parameter: *p,
			low:       low.Value[i],
		})
	}

	return params
}

// usage
params := getParameters(op.Parameters, op.GoLow().Parameters)

while not terrible it does add to the integration burden

@daveshanley
Copy link
Member

I see, OK. I'll jump back on this ASAP.

@daveshanley
Copy link
Member

I looked at the code again with some fresh eyes. I re-wrote the example without changing any code.

    var data = `
openapi: "3.1"
components:
  parameters:
    Param1:
      description: "I am a param"
paths:
  /something:
    get:
      parameters:
        - $ref: '#/components/parameters/Param1'`

    doc, err := NewDocument([]byte(data))
    if err != nil {
        panic(err)
    }

    result, errs := doc.BuildV3Model()
    if len(errs) > 0 {
        panic(errs)
    }

    // extract operation.
    operation := result.Model.Paths.PathItems["/something"].Get

    // print it out.
    fmt.Printf("param1: %s, is reference? %t, original reference %s",
        operation.Parameters[0].Description, operation.GoLow().Parameters.Value[0].IsReference,
        operation.GoLow().Parameters.Value[0].Reference)

Does this help see things slightly differently?

The challenge with adding IsReference() directly to any object, means it will need to know which parent ValueReference or NodeReference it's attached to (which is where the reference is stored). This two-way binding isn't there in the low-level model and it would have to be added (which is a significant lift, every referenceable model would need a retro-fit).

Let me know if this alternative way helps - if not, a new feature request would be needed to add IsReference() and GetReference() to every low-level model that can then keep track of it's parent's state.

@TristanSpeakEasy
Copy link
Contributor Author

I would still need the workaround I showed above, but that works for us for now.

I do think the API would be much easier to work with if you could GoLow() from any high level model and from the accompanying low model determine if it was resolved from a reference as we need to understand if really any object we are working with in a document came from a ref so I have the above workaround for each type we are dealing with (not just parameters).

I will close this ticket and add a feature request for IsReference() and GetReference() methods on the low level models as I do believe it would make the library easier to work with in the long run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants