[issue-713] - Add a test to the TCKs in order to verify the usage of the "@ExternalDocumentation" annotation

[fabiobrz]

We recently found an issue in the SmallRye OpenAPI implementation, which reveals missing test coverage for the @ExternalDocumentation annotation usage in the MicroProfile OpenAPI TCKs.

This PR provides a new test to cover some basic cases.

Resolves #713

[eclipse-microprofile-bot]

Can one of the admins verify this patch?

[Azquelt]

I'm a bit surprised we allow @ExternalDocumentation to be applied to any type to add documentation at the top level, particularly as we also allow it under @OpenAPIDefinition, but that is what the Javadoc currently says (and has said since 1.0).

[MikeEdgar]

I'm a bit surprised we allow @ExternalDocumentation to be applied to any type to add documentation at the top level, particularly as we also allow it under @OpenAPIDefinition, but that is what the Javadoc currently says (and has said since 1.0).

I was thinking the same thing. For reference, the JavaDoc was added here. It's not clear if the use on any type was intentional. I wonder if we should investigate other implementations and if it's not supported anywhere, maybe we alter the JavaDoc accordingly.

[MikeEdgar]

We discussed this change on today's spec call and the thinking is that it make sense to go ahead with having the TCK check the use of @ExternalDocumentation on REST methods for the 4.2 release of the spec/API.

We're generally hesitant about support on any class (which is what the annotation specifies) and we're likely to make some adjustment to that part of the JavaDoc, perhaps limiting it to only REST resource classes. That would need to be in a new major version v5 since it's technically a break in the spec, even if that functionality is not supported by any implementation currently.

[fabiobrz]

Thanks @MikeEdgar - I didn't make it to the call, due to conflicts, but I generally agree with your considerations.
If it can help, while making changes to the WildFly integration in order to support multiple deployments contributing to the OpenAPI documentation, we noticed that annotations reflecting to "centralized" elements (i.e. root elements in the document) can actually be problematic.

[fabiobrz]

We discussed this change on today's spec call and the thinking is that it make sense to go ahead with having the TCK check the use of @ExternalDocumentation on REST methods for the 4.2 release of the spec/API.

IIUC, then the test which is checking the method should stay, correct?

We're generally hesitant about support on any class (which is what the annotation specifies) and we're likely to make some adjustment to that part of the JavaDoc, perhaps limiting it to only REST resource classes. That would need to be in a new major version v5 since it's technically a break in the spec, even if that functionality is not supported by any implementation currently.

Nevertheless, users could object that implementations do not honor what the spec Javadoc states.
That is the reason why IMHO the test that's checking what happens when a class is annotated should stay as well.

[MikeEdgar]

IIUC, then the test which is checking the method should stay, correct?

Yes, that's right. We would target that change for 4.2 (4.1.1 was just released yesterday).

Nevertheless, users could object that implementations do not honor what the spec Javadoc states.
That is the reason why IMHO the test that's checking what happens when a class is annotated should stay as well.

I see your point and it's true that users could object, but I'm dubious that allowing the annotation on any type as stated in the Javadoc was implemented by any runtime or even intended originally. It's not clear what use case it would cover that couldn't be supported using one of several other approaches to having external docs in the root of the produced OpenAPI document. It would probably be best to remove or somehow clarify it in v5 rather than enforcing something that isn't actually going to be used very much.

[Azquelt]

This looks sensible to me.

[Azquelt]

@eclipse-microprofile-bot test this please

[fabiobrz]

@eclipse-microprofile-bot test this please

@Azquelt I've fixed the missing license header and executed a mvn .. verify locally, which passed.
Do we need to trigger the bot again?

[MikeEdgar]

@eclipse-microprofile-bot test this please

[Azquelt]

Thanks, build has run cleanly now.