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

[FEATURE REQUEST] allow $schema #377

Open
ripienaar opened this issue Apr 21, 2020 · 16 comments
Open

[FEATURE REQUEST] allow $schema #377

ripienaar opened this issue Apr 21, 2020 · 16 comments
Labels
keep-open Prevents stale bot from closing this issue or PR

Comments

@ripienaar
Copy link

Is your feature request related to a problem? Please describe.

When editing AsyncAPI documents it would be useful to allow $schema since modern IDEs will do code completion etc based on whats valid in the schema.

Can't it be tackled using specification extensions?

nope

Describe the solution you'd like

Allow $schema key in the top level.

Describe alternatives you've considered

Manual editing of IDEs, not nice.

Additional context

Publishing the schemas on well known URLs would be good if not already.

@ghost
Copy link

ghost commented Apr 29, 2020

Hi @ripienaar ,

thanks for bringing this up! This is excactly the problem I'm currently facing as well.

I've prepared this commit as an example how this could be solved at the JSON Schema document level: https://github.com/I522722/asyncapi/commit/dede4318a3425e5541a47832bf2cd87147508f1f

Probably we can make the following distinctions for solving this problem:

  • Just allowing the $schema property without any further restrictions or explanations to open up the JSON Schema usecase of the $schema attribute
  • Further making $schema an enum and therefore a discriminator. Adding $id to the Schema Document. See my commit example. This would be still optional to not break the spec. In theory, the existence of a mandatory $schema would make the asyncapi attribute obsolete as the link (should) describe both the identity and the version of the document.
  • Additionally we could consider publishing it to http://schemastore.org/json/. OpenAPI e.g. is already published there.

@derberg
Copy link
Member

derberg commented May 8, 2020

For others interested in this topic. There is a lot of clarification here https://asyncapi.slack.com/archives/C34F2JV0U/p1588926494498200

@ghost
Copy link

ghost commented May 11, 2020

For the simplest intermediary solution, we could just add $schema without a const or enum restriction to the top-level properties. Since it's just a technical attribute imho it does not need to be added to the human readable documentation.

  "$schema": {
      "type": "string",
      "format": "uri",
      "description": "The $schema keyword is used to declare that this JSON document is validated against the JSON Schema of the AsyncAPI 2.0.0 Specification. Adding this allows for automated tool support and code intelligence. See https://json-schema.org/understanding-json-schema/reference/schema.html"
  } 

This would be a very quick fix to enable the use case. There could be a follow-up issue whether this should be taken up to be more explicitly supported and e.g. the Schema officially published at a schema registry.

@derberg What's your oppinion on it? I could create a PR for just this change. Or do you want to discuss it further / go for a more complete solution right from the start?

@derberg
Copy link
Member

derberg commented May 11, 2020

@I522722 Hi Simon. It is a spec we are talking here about so quick fix is not something that will be quick :) I mean that quick fix will land quickly in the master branch, but will not be quickly released so better to work on a complete solution that could get into 2.1 release of the speck.

There is one thing I started to think about. $schema means that most of the tools that people use will fetch the schema, will call some URI. And I immediately started to think about one important subject that we do not how to solve with @fmvilas. We are basically not able to determine how many people use AsyncAPI now at the moment. We know there are many companies adopting it in their products, like SAP in its Enterprise Messaging and Kyma, or Solace in their Event Portal. We also know many people use it for other purposes, but we only know about those that talk to us about it on different channels.

And I was thinking that $schema sounds like the best solution here. That using sponsors money we could host the schema on our own and collect information on how many times and how ofter the file is fetched. This way we would have an estimated number of users, the closest we could get, comparing to other options.

So basically have a const here. We would of course transparently say that this server is collecting information about how many times and when a file is fetched, and if possible, take from headers info what editor is used so we know which one is most popular and what editor plugins to focus on first.

@ripienaar @I522722 what do you think about it? Or maybe there is some service like this for schemas that is giving what we need 🤔

@ghost
Copy link

ghost commented May 11, 2020

Hi @derberg ,

yes, quick was definately the wrong word for it. I wasn't expecting a quick release of this in any way :) Let me rephrase it to: uncomplicated to add and without the introduction of breaking changes.

This is an interesting point you make here. You could definitely get at least some analytics - but it's probably very hard to guess how big the percentage of tools is that would actually resolve this URI. It's good that you are transparent about the implications 👍

There is already a service for this, but I'm not sure whether it provides analytics that are available. If it uses a CDN for static file hosting this is most likely not the case. See https://github.com/SchemaStore/schemastore

The question then is whether this property should be mandatory then. If you make it mandatory AND add a const condition on it, it automatically also indicates additional information: This is an AsyncAPI document of this specific version. Therefore the asyncapi attribute would be technically obsolete.

If you keep it optional, my guess is that most people would not understand it or the usefulness of it. But those who do can easily add it and enjoy the benefits and there is no need to introduce a breaking change to the spec.

@derberg
Copy link
Member

derberg commented May 15, 2020

Hey @I522722,

IntelliJ and VSCode support $schema for sure, and they have loads of users so this should already bring us some good numbers.
This part is super curious for me https://schemastore.azurewebsites.net/json/#editors. Maybe editors are not so eager to fetch schemas from any source as we would anyway have to submit it to some knows domain... 🤔 we will have to check that.

Even though the possible analytics info we could get from $schema I don't believe it should be a mandatory field. We should only clearly describe in the documentation what are the benefits for the use and for the spec maintainers.

@ghost
Copy link

ghost commented May 15, 2020

Hi @derberg ,

thanks for the update! Sounds reasonable :)

Regarding the editor support of Schemastore: It would be interesting to know if this support is automatic or is used on a case-by-case basis. If the $schema is not explicitly stated in a .json file, there would need to be some clear mapping between the schemastore schema and the file is actually validated against. Most likely, each editor or editor extension will have a certain ruleset for this?

But if AsyncAPI is added in schemastore, it will have the public visibility so it is more likely to be considered for such rules.

Btw. a nice extension to bring Schema support for YAML is this one: https://marketplace.visualstudio.com/items?itemName=redhat.vscode-yaml

Since you can have a per-project config for this extension, you can define those rules (which file matches a schema) there via configuration yourself.

E.g.

"yaml.schemas": {
    "http://json.schemastore.org/composer": ["/*"],
    "file:///home/johnd/some-schema.json": ["some.yaml"],
    "../relative/path/schema.json": ["/config*.yaml"],
    "/Users/johnd/some-schema.json": ["some.yaml"],
}

This is an interesting discussion 👍

If I may come back to my original suggestion and the motivation behind it: The "minimal" solution that would enable those use cases for me is that $schema is listed as allowed property in the AsyncAPI schema. If that is the case, I can use it either directly via $schema or via the configured rules mentioned above

@derberg
Copy link
Member

derberg commented May 24, 2020

I asked a few questions to the schemastore community -> SchemaStore/schemastore#1048. This looks very awesome.

We can store schema on our servers and do our analytics if we want, and at the same time we can register schema at the schemastore and point to our servers and it will work like a charm.

Once we enable above, I don't see really why would one need $schema. I haven't found a single specification where you can specify $schema parameter. This is a pretty technical property.

Don't you think it is actually enough to have schemastore setup in place?

@ripienaar
Copy link
Author

I am confused. How you did not find any? Even JSON Schema lets you add it to tell editors and tools what the authored document comply to.

It’s a common case and the reasons JSON Schema allows it is the same as why it would be allowed here.

@derberg
Copy link
Member

derberg commented May 24, 2020

ok, JSON Schema of course allows, this is the only one that was pretty obvious for me and I didn't mention it, for me it is an exception that just confirms what I wrote. Json Schema for OpenAPI also doesn't support it and also disallows additional properties

@ripienaar
Copy link
Author

ripienaar commented May 24, 2020

Does AsyncAPI dictate the file name for a api description so it would be compatible with file match? Documentation says it’s by convention.

@derberg
Copy link
Member

derberg commented May 24, 2020

No, it doesn't, we would probably have to do something like openapi

    {

      "name": "openapi.json",

      "description": "A JSON schema for Open API documentation files",

      "fileMatch": [

        "openapi.json",

        "openapi.yml",

        "openapi.yaml"

      ],

      "url": "https://raw.githubusercontent.com/OAI/OpenAPI-Specification/master/schemas/v3.0/schema.json"

    }

@derberg
Copy link
Member

derberg commented May 24, 2020

On the other hand, if we allow x- for extensions we could also allow $ as technical props.
This way we do not have to allow $schema explicitly and document it explicitly 🤔
It would just be a matter of extending existing patternProperties https://github.com/asyncapi/asyncapi/blob/master/versions/2.0.0/schema.json#L11 🤔

@ghost
Copy link

ghost commented May 25, 2020

On the other hand, if we allow x- for extensions we could also allow $ as technical props.

Yes, I'd be highly in favor of this. Currently, it's not really possible to define $schema explicitly. But I'm rather sure that Schemstore support doesn't come automatically active the moment you add your schema there. Having this explicit $schema is something that will always work.

My PR proposal would have added $schema at least as a known property so it may be explicitly added without it immediately marking itself as invalid. Whitelisting all $ props would be even better.

It's also from JSON Schema side a best practice if I interpret it correctly? At least JSON Schema always states its $schema. This has also been very common with XML and XML schema. In CloudEvents you also do something similar via the data and dataschema attributes. Basically what is does is always attach to the data what the relevant schema is and where to fetch it.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴
It will be closed in 30 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions ❤️

@github-actions github-actions bot added the stale label Jul 25, 2020
@fmvilas fmvilas added keep-open Prevents stale bot from closing this issue or PR and removed stale labels Jul 25, 2020
@fmvilas fmvilas added this to the AsyncAPI specification 2.1.0 milestone Jan 31, 2021
@fmvilas fmvilas removed this from the Next specification version milestone May 12, 2021
@github-actions github-actions bot added the stale label Oct 5, 2021
@asyncapi asyncapi deleted a comment from github-actions bot Oct 5, 2021
@derberg derberg removed the stale label Oct 5, 2021
@derberg
Copy link
Member

derberg commented Oct 5, 2021

I'm in progress with AsyncAPI schema store, hope to have something working this year

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep-open Prevents stale bot from closing this issue or PR
Projects
None yet
Development

No branches or pull requests

3 participants