Clarify Extensible name (with/without x-)

[jbee]

The javadoc of Extensible.addExtension states:

@param name the key used to access the extension object. Always prefixed by "x-".

This can be understood in two ways:

1. The name passed is plain and the extensible object will prefix it with x- in its map.
2. The name should start with x-. Does this mean other should be ignored?

In the TCK examples the annotations use names with the x- prefix already in the name. I'd think that from the user perspective these should be plain names without the x- as it is more readable and avoids using illegal names by design.

[EricWittmann]

@arthurdm - can you add this to the agenda for next meeting?

[jmini]

I think accepting a key that starts with an "x-" is valid and should be accepted.

What can be changed:

if the name does not start with "x-" we can make the TCK ensure that the implementations add the missing prefix:

e.addExtension("custom", "value");

results in "x-custom" key being added with the value "value".

If we go this way, then to be consistent the same should be applied for setExtensions(Map<String, Object>) and for me also for removeExtension(String).

And we should have TCK test for all combinations.

---

As raised by @EricWittmann in smallrye/smallrye-open-api#77:

There are also all sorts of invalid things I could do when creating my OpenAPI document.

The problem raised here is just one of them... We could also agree to do nothing if the API is used wrongly.

An other approach would be to accept the illegal value and report a validation issue. See: #331

[EricWittmann]

I think this is a pretty good summary of the options! :)

[arthurdm]

I believe the OAS3 says the extension keys MUST start with x-, as per https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#specification-extensions

[EricWittmann]

It does, but I think the idea here is that the MP+OAI spec could support leaving out the x- bit of the key from the annotation, but that the platform would add that prefix in when generating the OpenAPI document.

[EricWittmann]

I think my approach would be to leave it as-is but update the javadoc (if it's not already clear) to indicate that the extension key should begin with x- or else an invalid document will be produced. @jmini correctly quotes my default position (until convinced otherwise) that we'll never be able to prevent users from doing something that will produce an invalid OpenAPI document. That said, it doesn't mean we shouldn't consider sensible helpful constraints or platform features. So we can discuss and decide at the next meeting. :)

[jmini]

@arthurdm : (out of topic for this specific issue, but same idea when it comes to validation and/or enforcement) the spec also says for Paths Object: The field name MUST begin with a slash (e.g. /{path}) but the MP-OpenAPI Spec/TCK do not enforce it...

[EricWittmann]

That's an example of what I mean - there are many ways to mess up. Off the top of my head:

• Operations IDs must be unique
• Tag definitions must be unique
• The allowEmptyValue property on Parameter can only be used for Query Params
• A Header Parameter named Content-Type is not allowed (but any other properly formatted name is OK)

The list is very long. In Apicurio I currently have 127 separate validation rules just for Spec Compliance - and I'm sure I've missed some. Constructing a document from annotations or from a model reader would allow a user to violate pretty much any rule. So I think it's obvious that the MP platform can't be expected to enforce compliance. Instead, I think it goes back to the fact that platform impls should be able to provide validation of the generated document.

[arthurdm]

sounds good, thanks guys. We can definitely chat about it in the next hangout.

My only worry is that the mutation of an extension key in a {key, value} pair may come as a surprise to the developer - for example, I define the extension key mySecretKey and not realizing that it got generated into OAS3 as x-mySecretKey I fetch for mySecretKey in the incoming JSON..but my client, being a good citizen, sent me x-mySecretKey. maybe that's not realistic, just a thought that occurred to me. :D

[EricWittmann]

We reviewed this issue in the MP hangout last week. The consensus was that we would not do any automatic conversion of non x- to x- extension names. However, we will ensure that the javadoc appropriately describes the requirements of using the @Extension annotation - specifically that the names of all extensions MUST begin with a x- or else an invalid document will be potentially created.

We have two main reasons for this.

1. Any consumers of the extension properties will need to refer to those properties with their proper x- prefixed names, so it would be weird to define them in @Extension without the prefix.
2. There are many ways to produce an invalid OpenAPI document using annotations, and it would be very difficult (perhaps impossible) for the platform implementation to check for every possible way in which the developer might mess up.

The best solution for this problem is for the platform to support validation of the resulting OpenAPI document. Optional validation support is something we will be adding to the MP+OAI spec in the near future.

[EricWittmann]

Note: I am leaving this open until we have reviewed the javadoc to ensure it is accurate and complete with respect to documenting @Extension. Once that is done (either because it's already sufficient or a PR is submitted) we can close this issue.

If anyone strongly disagrees with the committee's decision on this matter, please don't hesitate to make your case! It would be best to join a future MP hangout to do this, but if you cannot attend the hangouts then you can make a case here.

[MikeEdgar]

The plan is to update the JavaDoc as suggested above for the MP 3.1 milestone.

[jbee]

There are many ways to produce an invalid OpenAPI document using annotations, and it would be very difficult (perhaps impossible) for the platform implementation to check for every possible way in which the developer might mess up.

With the same logic you could argue that static types in a language are "not needed" as developers have many ways (e.g. casts) to produce an invalid program. Why even have the compiler check it since there are many ways mess up, right?. Just saying.