asyncapi / spec

The AsyncAPI specification allows you to create machine-readable definitions of your asynchronous APIs.
https://www.asyncapi.com
Apache License 2.0
4.25k stars 269 forks source link

[FEATURE REQUEST] allow $schema #377

Open ripienaar opened 4 years ago

ripienaar commented 4 years ago

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 commented 4 years ago

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:

derberg commented 4 years ago

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

ghost commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

I asked a few questions to the schemastore community -> https://github.com/SchemaStore/schemastore/issues/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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity :sleeping: 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 :heart:

derberg commented 3 years ago

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