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

Unifying when reference can be used #650

Closed
jonaslagoni opened this issue Nov 10, 2021 · 9 comments
Closed

Unifying when reference can be used #650

jonaslagoni opened this issue Nov 10, 2021 · 9 comments
Labels
👻 Needs Champion RFC Needs a champion to progress (See CONTRIBUTING.md) stale 💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md)

Comments

@jonaslagoni
Copy link
Member

This issue is different from #649 as this issue only focuses on when a reference can be used, and not the internal behavior of such reference.

The problem

There is a large discrepancy between when a $ref can be used in the specification.

The current structure kinda makes sense if we assume that all the referenced must be within the very same document, or at least reference an AsyncAPI document variant (referencing to a component). The problem is $ref could reference an external file that, if seen in isolation, has nothing to do with AsyncAPI. And it is not until it is bundled together that it forms a valid AsyncAPI document.

Example:

# asyncapi.yaml
asyncapi: '2.2.0'
info:
  title: Test overriding dereferenced objects 
  version: '1.0.0'
servers:
  test:
    $ref: 'testServer.yaml'

# testServer.yaml
url: ws://mycompany.com/ws
protocol: ws

The example above is currently not supported in the spec, our JS-parser however incorrectly allows such behavior (which does not help with the confusion).

Here is a list of the immediate problems I found in the spec:

Ways to address it

I see a couple of ways we can address this, the following explains those suggested solutions:

Allow reference anywhere

One option could be to uniform references in such a way that the Reference Object is allowed within any keyword of the specification.

Allow references within any JSON object

Another option would be to allow the Reference Object to be used anywhere a JSON object is expected.

My suggestion

I suggest that we go with the second suggestion, allowing references anywhere a JSON object is expected.

Related issues

@jonaslagoni jonaslagoni added the 💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md) label Nov 10, 2021
@jonaslagoni jonaslagoni added the 👻 Needs Champion RFC Needs a champion to progress (See CONTRIBUTING.md) label Nov 11, 2021
@magicmatatjahu
Copy link
Member

Allow reference anywhere
One option could be to uniform references in such a way that the Reference Object is allowed within any keyword of the specification.

Allow references within any JSON object
Another option would be to allow the Reference Object to be used anywhere a JSON object is expected.

Please have in mind (or you had it writing this issue) that string, number, boolean, object and array (and null) are JSON instances (RFC 8259), so if you write about JSON object you should have in mind JSON object, not JSON instance, because if not, then your first proposition is exactly this same as second.

However, I prefer the first proposal in order to simplify the use of references in general, as well as to allow referencing of primitives - the description of the AsyncAPI document in the info/description field comes to mind. If description can be described in CommonMark format, then we should also be able to refer to a markdown file :)

@char0n
Copy link
Collaborator

char0n commented Jan 17, 2022

I found an article https://www.asyncapi.com/blog/the-reference-rabbit-hole and it was linking to this issue. Id' like to join the discussions with my observations.

@jonaslagoni we had similar discussion with you and other maintainers in: #552 and #579. I'll be quoting sections from the article and expressing my thoughts about quotes.

This is where the confusion starts, what behavior does the $ref keyword follow? More precisely, which specification?

This is what the specification tell us about it:

image

image

This clearly defines what Reference Object is and that it can be used only in places where specification allows. As for Schema Object, the specification clearly says that when $ref keyword is intercepted, the Schema Object containing the $ref keyword is replaced by Reference Object and the resolution is then driven by Reference Object resolution rules.

was using a Reference Object for a server, which was not allowed. More specifically, it was this example where he references the mosquitto server

Specification clearly states here, that Servers Object only allows Server Object as the only type for it's fields. There shouldn't be a confusion here, this case is IMHO just the result of incomplete understanding of the spec.

What about $id keyword

IMHO this whole section is not what specification tells us. As mentioned above every Schema Object that contains $ref field is replaced by Reference Object and Reference Object only recognizes $ref field and follows JSON Reference spec. Given this evidence, $id inside Schema Object has no semantic meaning. This has been stipulated already in #552. The resolution as described in the example in the article should never happen.

What about extra keywords?

I mostly agree with observation of this section. But it should be pointed out, that there is only one referencing mechanism allowed in the AsyncAPI spec and that is Reference Object. It shouldn't matter what the behavior of Schema Object.$ref field (and others like $id) is.

But, what if I use one of the newer JSON Schema versions, what then?

As mentioned above, it shouldn't really matter as the specification clearly says that Schema Object with $ref field are replaced with Reference Object:

image

Hard to find tooling

There is actually a full implementation of alterantive AsyncAPI 2.2.x parser: https://github.com/swagger-api/apidom. It comes with reference implementation of the spec and fully implemented mechanism of resolution and dereferencing.

Summary

This has been the behavior of OpenAPI <= 3.0.x specs. As the AsyncAPI spec was borned from OpenAPI it comes with the same or similar mechanisms. OpenAPI 3.1.x now clearly states that the Schema Object has it's own rules of resolution/dereferencing defined by JSON Schema specification and not by OpenAPI spec. According to my understanding AsyncAPI has currently exclusively two mechanism of internal/external referencing:

@jonaslagoni
Copy link
Member Author

jonaslagoni commented Jan 17, 2022

@char0n thanks for taking the time to provide feedback to the article!

Specification clearly states here, that Servers Object only allows Server Object as the only type for it's fields. There shouldn't be a confusion here, this case is IMHO just the result of incomplete understanding of the spec.

Absolutely, just my own confusion 😄 Don't see the article as anything but the timeline of thoughts as we reflected on the knowledge and tried answering the questions as they came up.

IMHO this whole section is not what specification tells us. As mentioned above every Schema Object that contains $ref field is replaced by Reference Object and Reference Object only recognizes $ref field and follows JSON Reference spec. Given this evidence, $id inside Schema Object has no semantic meaning. This has been stipulated already in #552. The resolution as described in the example in the article should never happen.

Hmm, I see I haven't formulated myself well enough in the article. One of the points it's missing is a deeper explanation of the schemaFormat. When you define it as schemaFormat: 'application/vnd.aai.asyncapi+yaml;version=2.2.0' (or simply leave it out) means that the payload follows the Schema Object and Reference Object. And all your comments definitely make sense there.

However, and this is where I didn't seem to have explained it as when defined as schemaFormat: 'application/schema+json;version=draft-07' it means that you no longer follow the Schema Object (application/vnd.aai.asyncapi+yaml;version=2.2.0), but JSON Schema schema format. No longer related to the Schema Object and Reference Object.

This is also why the bug report to the parser contains an AsyncAPI document with that specific format.

This means that technically, parsers have to understand both ($id,$ref and extra properties) behaviors, and interpret them accurately.

There is actually a full implementation of alterantive AsyncAPI 2.2.x parser: https://github.com/swagger-api/apidom. It comes with reference implementation of the spec and fully implemented mechanism of resolution and dereferencing.

Uhh, nice! Does your parser take into account the different schemaFormats and resolve references relative to that?

Any reason it is not on the tools list? I see we don't have a section for parsers, but that can be added 😄

However, what I tried to refer to, is core tooling that everyone can use for dereferencing i.e. build their own parsers upon 😄 So implementors don't have to figure out the small details of the specs.

@char0n
Copy link
Collaborator

char0n commented Jan 17, 2022

Hmm, I see I haven't formulated myself well enough in the article. One of the points it's missing is a deeper explanation of the schemaFormat. When you define it as schemaFormat: 'application/vnd.aai.asyncapi+yaml;version=2.2.0' (or simply leave it out) means that the payload follows the Schema Object and Reference Object. And all your comments definitely make sense there.
However, and this is where I didn't seem to have explained it as when defined as schemaFormat: 'application/schema+json;version=draft-07' it means that you no longer follow the Schema Object (application/vnd.aai.asyncapi+yaml;version=2.2.0), but JSON Schema schema format. No longer related to the Schema Object and Reference Object.
This is also why the bug report to the parser contains an AsyncAPI document with that specific format.
This means that technically, parsers have to understand both ($id,$ref and extra properties) behaviors, and interpret them accurately.

Right. I missed that the example is based on Message Object which can use schemaFormat (along with Message Trait Object). In that case I'd say your observations there are correct and the resolution/dereference are bound to the the specification as defined in schemaFormat and this is completely separate from AsyncAPI specification rules.

Uhh, nice! Does your parser take into account the different schemaFormats and resolve references relative to that?

We currently support only AsyncAPI 2.2.0 Schema Object. If consumer provides other schemaFormat, generic object will be the result of the parsing; this switching of schemas has so many implications that we're still trying to map it all. It technically means we have to fully support the specs supported in schemaFormat field (or subset of the spec) and resolve issues associated with it - like crossing document boundaries etc...

However, what I tried to refer to, is core tooling that everyone can use for dereferencing i.e. build their own parsers upon smile So implementors don't have to figure out the small details of the specs.

Yes I think I got it. The parser I mentioned can semantically parse the spec, has querying capability and provides higher order algorithms like resolution or dereferencing - so consumer can just use it and not deal with complex intricacies of the spec. Bundling is not yet supported as well as schemaFormat.

@jonaslagoni
Copy link
Member Author

jonaslagoni commented Jan 31, 2022

Yes I think I got it. The parser I mentioned can semantically parse the spec, has querying capability and provides higher order algorithms like resolution or dereferencing - so consumer can just use it and not deal with complex intricacies of the spec. Bundling is not yet supported as well as schemaFormat.

@char0n would be great if we could all use an underlying library that resolves references for us, so regardless of how specs define the behavior it can adapt it internally instead of being static. But yea, out of scope for this issue 😆

@github-actions
Copy link

github-actions bot commented Jun 1, 2022

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Jun 1, 2022
@derberg derberg removed the stale label Jun 7, 2022
@github-actions
Copy link

github-actions bot commented Oct 6, 2022

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Oct 6, 2022
@derberg
Copy link
Member

derberg commented Oct 11, 2022

@jonaslagoni can we close this one if favor of #829?

@jonaslagoni
Copy link
Member Author

As I understand #829 is only for figuring out if we go with $ref or name for example referencing servers in channels.

But honestly, I don't see this as an issue anymore, and the suggestions are kinda useless tbh as there are no use-cases behind it (and the suggestion I made does not really make sense), just confusion based on the parser implementation.

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👻 Needs Champion RFC Needs a champion to progress (See CONTRIBUTING.md) stale 💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md)
Projects
None yet
Development

No branches or pull requests

4 participants