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

[3.0.0] Allow references to be used in any part of the specification #778

Open
magicmatatjahu opened this issue May 4, 2022 · 12 comments
Open
Labels
💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md)

Comments

@magicmatatjahu
Copy link
Member

magicmatatjahu commented May 4, 2022

The current version of AsyncAPI (2.x.x) has strictly defined places where users can use references in a document. We should extend the available places to every part of AsyncAPI.

Benefits:

  • no PR and issue like Extending Server.variables field type #775 (after every release we have at least 1 because we forgot to update places where we can use references)
  • easier support in tooling - currently in our official tooling we don't check where references were used, but I don't see the benefits of such checking
  • people wouldn't be confused if a reference is allowed or not

It seems to me that instead of giving the type as the union of the object and reference in each place, we could add a section in the spec explaining how to use references (at the beginning) and they are allowed to be use everywhere.

This is a breaking change, so I'm assigning this issue for 3.0.0.

@magicmatatjahu magicmatatjahu added the 💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md) label May 4, 2022
@char0n
Copy link
Collaborator

char0n commented May 4, 2022

HI @magicmatatjahu,

Very quickly couple of points I can think of:


The Reference Object comes originally from OpenAPI spec and as far as I understand is was introduced to have full control over how definitions written using the spec look like (structure) and which parts do make sense to be reusable and which not.


This proposal will IMHO introduce problematic ambiguity. We'll also have problem recognizing definition author intentions. How do we tell if $ref was meant as a arbitrary field name or as a Reference Object?

{
  "components": {
    "schemas": {
      "$ref": { }
    }
  }
}

There will be problem with identifying resolution rules. Every version of the spec can have specific resolution rules. 2.4 uses JSON Reference, 3.0 can use RFC3986.

Having following definition:

asyncapi: 
  $ref: 'external-document.yaml#/path/to/version',
info:
  title: Streetlights Kafka API
  version: '1.0.0'
  description: |
    The Smartylighting Streetlights API allows you to remotely manage the city lights.

In order to know what are the resolution rules for the Reference Object, I have to determine what is the version of the AsyncAPI spec used to write this definition. I cannot resolve the reference as I don't know the spec version and and I cannot get the spec version as I don't know the resolution rules. We got into classical chicken and egg situation which cannot be resolved.

@magicmatatjahu
Copy link
Member Author

@char0n Thanks for comment.

The Reference Object comes originally from OpenAPI spec and as far as I understand is was introduced to have full control over how definitions written using the spec look like (structure) and which parts do make sense to be reusable and which not.

I had in mind that when I wrote issue, but in my opinion, saying which things should be reusable and which should not only causes misunderstanding of the community. In my opinion every part should be reusable without exception. If someone creates very complex dependencies that are difficult to read/resolve, that's the user's problem. People who understand how references work use them in ways that are not described in the specification, e.g. referencing md documents in description fields - currently it is possible but few people know about it, why not standardize it?

This proposal will IMHO introduce problematic ambiguity. We'll also have problem recognizing definition author intentions. How do we tell if $ref was meant as a arbitrary field name or as a Reference Object?

I know about this problem, but it occurs even in the current specification, or rather in the tooling - for now we don't check the specification before dereferencing and I don't know if we should and it makes sense.

Additionally, looking at this example, I wonder if someone would like functionality for merging keys in an object. For example something like this:

{
  "components": {
    "schemas": {
      "$ref": "...",
      "mySchema1": {},
      "mySchema2": {}
    }
  }
}

where you merge reference with additional keys - we have even issue for that #649
IMHO we should try to add this possibility to override some keys from referenced object to make reusable objects more friendly.

In order to know what are the resolution rules for the Reference Object, I have to determine what is the version of the AsyncAPI spec used to write this definition. I cannot resolve the reference as I don't know the spec version and and I cannot get the spec version as I don't know the resolution rules. We got into classical chicken and egg situation which cannot be resolved.

tbh I had this problem in mind when I wrote this issue 😄 , but I decided that it was so abstract that there was no point in dwelling on it. This case would be allowed, but it doesn't make any sense - it's like in programming languages, you can write hard to understand code, it will work, but nobody will accept it in PR, so we should think the same here:

something that is allowed doesn't mean it's a good idea

@char0n
Copy link
Collaborator

char0n commented May 6, 2022

Hi @magicmatatjahu,

I had in mind that when I wrote issue, but in my opinion, saying which things should be reusable and which should not only causes misunderstanding of the community.

I wonder what are cases of community confusion? Could you provide example issues?

In my opinion every part should be reusable without exception. If someone creates very complex dependencies that are difficult to read/resolve, that's the user's problem.

Specification now clearly states where references are allowed and how they are resolved. After #782 and #779 are accepted, it is crystal clear how references are handled and there is no question about they can be used and how they are resolved.

People who understand how references work use them in ways that are not described in the specification e.g. referencing md documents in description fields - currently it is possible but few people know about it, why not standardize it?

That is obviously an invalid usage of the specification and it's a issue of tooling implementation that it allows it. Tooling should adhere to the specification and if somebody decide to go outside of the specification in tooling implementation, well then they have to cope with implications.

I know about this problem, but it occurs even in the current specification, or rather in the tooling - for now we don't check the specification before dereferencing and I don't know if we should and it makes sense.

Right, the issue is in the tooling and not in the specification. Specification says that it's not allowed and tooling should respect that.

Additionally, looking at this example, I wonder if someone would like functionality for merging keys in an object. For example something like this:

I have no idea if someone wants to do that. Given 2.4 version of the spec this is not allowed and should be considered by the tooling as arbitrary $ref field.

tbh I had this problem in mind when I wrote this issue smile , but I decided that it was so abstract that there was no point in dwelling on it. This case would be allowed, but it doesn't make any sense - it's like in programming languages, you can write hard to understand code, it will work, but nobody will accept it in PR, so we should think the same here:

The specification should be clear and strive to eliminate ambiguities and limit invalid usage of the specification. By adopting idea described in this issue, specification itself will introduce undefined situations that in some cases are unresolvable.


My overall opinion is that this proposal will unnecessarily increase the complexity and decrease the end-user experience. It will also make creating tooling, specifically strict semantic parsing ambiguous as the intention of the author is lost in definition.

@github-actions
Copy link

github-actions bot commented Sep 4, 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
Copy link

github-actions bot commented Jan 4, 2023

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 Jan 4, 2023
@handrews
Copy link

A few thoughts as JSON Schema has gone through several iterations of how referencing works:

  • Doing this would completely break compatibility with JSON Schema. In draft-07, the breakage would just involve ambiguous reference-or-not locations, but in 2019-09 and later, references are evaluated in the context of schema behavior which would not work at all with this
  • Outside of JSON Schema, merging is possible (OpenAPI 3.1 supports overwriting summary or description), but it is not allowed in JSON Schema and would often contradict the specified behavior (in all drafts)
  • @char0n you noted "There will be problem with identifying resolution rules. Every version of the spec can have specific resolution rules. 2.4 uses JSON Reference, 3.0 can use RFC3986." but I am unaware of any differences in behavior. Could you clarify? It might be something I need to address in the unified referencing proposals.

I'll also direct folks to the unified referencing discussion repository. There's no need to update it with proposals that aren't being adopted, but it would be great to file any AsyncAPI referencing changes that will be adopted as issues so that we can keep track of how references are used everywhere. The discussion also encompasses standalone referencing tooling.

@github-actions github-actions bot removed the stale label Jan 11, 2023
@github-actions
Copy link

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 ❤️

@eivindga
Copy link

eivindga commented Jun 1, 2023

As of v1.1.0 of the asyncApi-generator, the tooling is now conforming to the specifications. Meaning most refs no longer work. 😵

This causes major headaches to your users when the api-specifications and text gets big enough.

There is no technical reasons why the spec could not describe the handling of a asyncapi-file as a two part action:

  1. resolve all refs (depth first) into a yaml / json
  2. do the validation

I find it hard to believe the end users want to keep long markdown-texts and examples in one big single yaml-file. That makes for a terrible maintenance job.

@github-actions github-actions bot removed the stale label Jun 2, 2023
@github-actions
Copy link

github-actions bot commented Oct 1, 2023

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 1, 2023
@berry120
Copy link

berry120 commented Dec 7, 2023

@char0n Even if it's not introduced everywhere, there's still a lot of places (the servers for example) that would commonly be identical between APIs that can't be reused at the moment. Could we still entertain the idea of significantly expanding the places $ref can be used, even if not everywhere?

@github-actions github-actions bot removed the stale label Dec 8, 2023
Copy link

github-actions bot commented Apr 7, 2024

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 Apr 7, 2024
@char0n
Copy link
Collaborator

char0n commented Jun 18, 2024

@char0n Even if it's not introduced everywhere, there's still a lot of places (the servers for example) that would commonly be identical between APIs that can't be reused at the moment. Could we still entertain the idea of significantly expanding the places $ref can be used, even if not everywhere?

@berry120 sure, a lot of spec objects could be referenced or expanded to be referenced, if it makes sense.

@char0n you noted "There will be problem with identifying resolution rules. Every version of the spec can have specific resolution rules. 2.4 uses JSON Reference, 3.0 can use RFC3986." but I am unaware of any differences in behavior. Could you clarify? It might be something I need to address in the unified referencing proposals.

@handrews bad example. Yes you're absolutely right. JSON Reference is referring back to RFC3986. What I was trying to say is that with this proposal the version of AsyncAPI specification could in theory be referenced as well and when different versions of AsyncAPI use different resolution mechanisms it becomes problematic to resolve the reference.

@github-actions github-actions bot removed the stale label Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md)
Projects
None yet
Development

No branches or pull requests

5 participants