spec icon indicating copy to clipboard operation
spec copied to clipboard

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

Open magicmatatjahu opened this issue 3 years ago • 12 comments

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 https://github.com/asyncapi/spec/issues/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 avatar May 04 '22 08:05 magicmatatjahu

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.

char0n avatar May 04 '22 09:05 char0n

@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 https://github.com/asyncapi/spec/issues/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

magicmatatjahu avatar May 04 '22 09:05 magicmatatjahu

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 https://github.com/asyncapi/spec/pull/782 and https://github.com/asyncapi/spec/pull/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.

char0n avatar May 06 '22 08:05 char0n

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

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 :heart:

github-actions[bot] avatar Sep 04 '22 00:09 github-actions[bot]

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

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 :heart:

github-actions[bot] avatar Jan 04 '23 00:01 github-actions[bot]

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.

handrews avatar Jan 10 '23 20:01 handrews

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

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 :heart:

github-actions[bot] avatar May 12 '23 00:05 github-actions[bot]

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.

eivindga avatar Jun 01 '23 09:06 eivindga

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

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 :heart:

github-actions[bot] avatar Oct 01 '23 00:10 github-actions[bot]

@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 avatar Dec 07 '23 08:12 berry120

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

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 :heart:

github-actions[bot] avatar Apr 07 '24 00:04 github-actions[bot]