eclipse/microprofile-open-api

TCK enforces deprecated schema `example` is merged with `examples`

Closed this issue · 4 comments

The model org.eclipse.microprofile.openapi.apps.airlines.model.User in the TCK airlines test includes several uses of the deprecated example property of @Schema. There are then several assertions in AirlinesAppTest that check that the example values have been mapped into the examples array. We should update these tests to allow the example to be present in either location.

vr.body("components.schemas.User.properties.phone.examples", not(contains("123-456-7890")));
vr.body("components.schemas.User.properties.phone.examples", contains("123-456-7891"));

vr.body("components.schemas.User.properties.phone.examples", contains("123-456-7891"));

vr.body("components.schemas.User.properties.password.examples", contains("bobSm37"));

As noted on the smallrye issue:

There's not a 1:1 mapping between the annotation and the model. IMO where something is deprecated with a complete replacement, I think we should target generating the new thing. This allows users with an existing code base to generate a good OpenAPI 3.1 document without code changes.

There's not a 1:1 mapping between the annotation and the model.

They both have the deprecated example as well as the array examples. Did you mean something else?

IMO where something is deprecated with a complete replacement, I think we should target generating the new thing. This allows users with an existing code base to generate a good OpenAPI 3.1 document without code changes.

I agree generally about targeting the new field, but I think having it enforced in the TCK or not something that can be configured at the implementation level may be too much.

In general, we don't have a 1:1 mapping between the annotations and the model. Therefore I don't see a problem with us deciding that example in the annotation should map to examples in the model.

In general, I think it will be much more common that someone updating to OpenAPI 3.1 will want a document that doesn't use deprecated features.

I agree generally about targeting the new field, but I think having it enforced in the TCK or not something that can be configured at the implementation level may be too much.

Having it enforced in the TCK does not prevent an implementation from making it configurable. It's always permitted to have options which allow you to deviate from the spec.

A better argument against this test might be that the spec must have a requirement for this behaviour for the TCK to assert it.

We've discussed this issue in our weekly call and determined that it's best to keep the tested behavior as-is in the TCK. If another implementation besides smallrye-open-api should raise a challenge to it, we will discuss further at that time.

For now, we can simply add an option to smallrye to disable the merging of example into examples. The default behavior will continue to merge them, however. Further discussion of the implementation details will continue in smallrye/smallrye-open-api#2076.