feat: allow to define Schema Object in Server variables

[magicmatatjahu]

---

title: "Allow to define Schema Object in Server variables"

---

Related issue(s):
#583

---

As I described in the #583 issue, I'm wondering why we have inconsistencies for schemas in server.variables and channel.parameters. This PR introduces change of shape of single Server Variable Object:

• instead of having the 3 words enum, default, examples, which work identically to the JSON Schema keywords, we will now have the word schema, which is Schema Object

• description is still available, to be consistent with Channel Parameter Object.

• we can remove description field (it is also defined in Schema Object) and flat the schema so definition of single variable can have shape like:

  variables:
    username:
      type: string
      description: This value is assigned by the service provider, in this example `gigantic-server.com`
      default: demo

• another solution is to add the possibility to define other keywords like format, pattern, minLength etc and for numeric values (integer and number types), but in the current proposal someone can define a schema for server variable in components/schemas and we are reusing things that already exist in spec, not creating something from scratch.

• with current proposal, in the future, when we will implement and resolve problems (Avro schemas are not supported in the components/schemas section #528 [2.0.0 REVIEW] Clarify usage of JSON References (/Pointers) for non-JSON data structures #216), user will be able to define variables in different formats than JSON Schema.

Comments are more than welcome!

BTW. It's a breaking change, so it should be (hopefully) merged in the 3.0.0.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[fmvilas]

I think we'll need to specify how to serialize them to strings (probably on channel parameters too) so when someone does something like this:

variables:
  example:
    schema:
      type: object
      properties:
        a:
          type: string
          default: 'hello'

How is it serialized into the URL? As {"a": "something"}? Maybe as ?a=something?

The reason you don't have a full schema object is to prevent people from using other things than a simple string. We can probably improve it and let people use other basic types like number and boolean which are pretty straightforward to serialize as a string inside the URL.

Food for thought 🤔

[fmvilas]

Also, is there a use case driving this proposal? Would be great to know why you want such a change.

[magicmatatjahu]

The reason you don't have a full schema object is to prevent people from using other things than a simple string. We can probably improve it and let people use other basic types like number and boolean which are pretty straightforward to serialize as a string inside the URL.

I know that object and array in described case is unnecessary but my PR is like normal Strawman (I think that better is discuss under the PR than issue - you can see changes and how it can works), so using all keywords from Schema isn't good decision. As you wrote and I suggested in PR description, we can allow only a keywords for string, number, integer, boolean types - it's another good solution. With this approach we will still have possibility to define the Server Variables in the components/schemas and reuse them.

How is it serialized into the URL? As {"a": "something"}? Maybe as ?a=something?

I don't think so if it should be serialized. In my opinion it's invalid. Like in Parameter Object, currently you have this possibility to define parameter as object (or array), but it's invalid usage, so we should also handled that problem in this proposal, for Server Variable and Parameter Object.

Also, is there a use case driving this proposal? Would be great to know why you want such a change.

I don't have (for me) explicit use case, but I see that at the moment if you want define more specific constraints for your Server Variable, you must write examples in the examples array or describe constraints in the description field. When you will have more keywords which you can use, you can even make this:

variable:
  type: integer
  minimum: 0
  maximum: 100 

rather than:

variable:
  examples: ['0', '1', '2' ... '100']
  # or:
  enum: ['0', '1', '2' .... '100']

So at the end (after discussion) we should have answer for question: allow all keywords for the Server Variables? If yes, fine, if not, then we should chose the most important and change also the schema shape for Channel Parameter Object.

[magicmatatjahu]

How is it serialized into the URL? As {"a": "something"}? Maybe as ?a=something?

I am now wondering if in server url user should have possibility to define query for configuration purposes 🤔 This may be off-proposal but it is quite an interesting topic.

[derberg]

@fmvilas what do you mean by serialized into url? variables are provided under url and then described one by one under Variables object. So there is no need to say how is this going to be serialized into the url. As @magicmatatjahu write, we just need to clearly describe-restrict it to basic types only. We do it like this for query in WS Binding https://github.com/asyncapi/bindings/tree/master/websockets#channel-binding-object and this is completely fine.

For now I don't like it is not a specific use case driven, but still it is a good change that makes Variables consistent with Parameters. I would compare it to #547, so like, nobody cares much (I mean, it is like it is, and things still work), but the change makes sense and we should strongly consider it.

Nevertheless, would be cool to hear community here.

[fmvilas]

Yeah, sorry I should have been clearer. I meant what actual strings in the URL should be matching the validation described in the schema. So for instance, if you have a URL template like ws://localhost/{username} and you try to connect to a URL like ws://localhost/["one", 2, false], would it be valid given a schema like the following?

variables:
  username:
    schema:
      type: array

If so, why are we assuming that URL variables should be deserialized as JSON? Why not a list of values like ws://localhost/one,2,false? JSON Schema is meant for validating JSON data but, in this case, I don't think it's JSON but just some strings and maybe with the ability to provide numbers, booleans, etc.

My point is, don't you think it's weird? And don't get me wrong, I think we should align with Parameter Object but probably by removing the schema keyword there because of the same issue.

Of course, listening from the community would be needed here. Maybe nobody is using the schema keyword beyond type: string and it's safe to remove it. Or maybe not. We need insights before proceeding.

[magicmatatjahu]

... I don't think it's JSON but just some strings and maybe with the ability to provide numbers, booleans, etc.

My point is, don't you think it's weird? And don't get me wrong, I think we should align with Parameter Object but probably by removing the schema keyword there because of the same issue.

Of course, listening from the community would be needed here. Maybe nobody is using the schema keyword beyond type: string and it's safe to remove it. Or maybe not. We need insights before proceeding.

I agree with you that's it's weird that in the current proposal you can define also the object and array type, but as you mentioned it's still possible in the Parameter Object, so we should standardize it, I think not only for Server Variable and Parameter but also in the other parts of spec - for example, the message's headers field has Schema Object shape, but in docs is mentioned that it should use only the object type:

But however if I define for headers the type: string as in example:

it still works. I know that spec =/= tooling (parser) but we should handle that problems.

[derberg]

I think we should align with Parameter Object but probably by removing the schema keyword there because of the same issue.

very good point.

for example, the message's headers field

Strange, I was fixing it in the schema in the past and it was working as expected -> asyncapi/spec-json-schemas#10

Yeah, just fix the schema here https://github.com/asyncapi/asyncapi-node/blob/master/schemas/2.1.0.json#L644-L657 😄

[magicmatatjahu]

@derberg Sorry, you're right, if you define another type that object inside the Message Object then you will have error, but if you define in trait it's ok -> https://github.com/asyncapi/asyncapi-node/blob/master/schemas/2.1.0.json#L885-L892 Traits are merged after validation so then I can see in playground:

I will create an issue in the asyncapi-node repo.

EDIT: Issue -> asyncapi/spec-json-schemas#82

[fmvilas]

but as you mentioned it's still possible in the Parameter Object, so we should standardize it

Yeah, I agree we should standardize it. I just don't agree to extend server variables but "cut" the parameters object instead. It's weird in both places. And regarding headers, you're right, it's another example. The reason it is like that is that I couldn't find a better way to do it. In this case (including the Parameter Object case), I think our best choice is to actually remove options to keep it simple. My two cents.

[github-actions]

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

[magicmatatjahu]

Still valid. We will need this in the next major release.

[smoya]

I think we should align with Parameter Object but probably by removing the schema keyword there because of the same issue.

+1

@magicmatatjahu would it make sense to turn this issue scope into what that suggestion says?

[magicmatatjahu]

@smoya but in ParameterObject you have also location field, so schema have to be inside ParemeterObject, but it should allows only string, number, boolean types with their constraints.

[smoya]

@smoya but in ParameterObject you have also location field, so schema have to be inside ParemeterObject, but it should allows only string, number, boolean types with their constraints.

What I understood (maybe wrongly) with that suggestion is that we drop support completely for using Schema Object (and schema field obviously).

[magicmatatjahu]

@smoya

What I understood (maybe wrongly) with that suggestion is that we drop support completely for using Schema Object (and schema field obviously).

And how people will define constraints for parameter? 😅 Looking at this idea, removing the schema field will not be a good idea. We should just limit it to 3 types, string, number and boolean with all their constraints.

[smoya]

@smoya

What I understood (maybe wrongly) with that suggestion is that we drop support completely for using Schema Object (and schema field obviously).

And how people will define constraints for parameter? 😅 Looking at this idea, removing the schema field will not be a good idea. We should just limit it to 3 types, string, number and boolean with all their constraints.

No idea. I thought this was part of what it should be done. But as I mentioned, I think I did a wrong assumption so please discard it :)

[github-actions]

This pull request 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 pull request, add a comment with detailed explanation.

There can be many reasons why some specific pull request 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 pull request forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

[magicmatatjahu]

At the moment, I don't see the point of keeping that PR open. If I find another way to use Schema Object in Server Variable, I will make an another proposal.