eclipse / microprofile-open-api

Microprofile open api
Apache License 2.0
132 stars 82 forks source link

[OAS 3.1.0] New restrictions on ServerVariable.enum #593

Closed Azquelt closed 4 months ago

Azquelt commented 8 months ago

Two new restrictions are added to ServerVariable.enum:

We should

benjamin-confino commented 5 months ago

tck/src/main/java/org/eclipse/microprofile/openapi/tck/ModelConstructionTest.java

This test modifies the enum list, but never sets a default so the ServerVariable will be in an illegal state.

(It also raises the question of intermediate values. If you want to remove the defualt from the enum, add a new value, snd set the new value to the default. Is it required to ensure the default is in the enum at every step of the process?)

The following tests use ServerVariable in line with the rule

tck/src/main/java/org/eclipse/microprofile/openapi/apps/airlines/JAXRSApp.java - enumeration is unset.

tck/src/main/java/org/eclipse/microprofile/openapi/apps/airlines/resources/ReviewResource.java - enumeration is set, default is set and in the enumeration.

tck/src/main/java/org/eclipse/microprofile/openapi/reader/MyOASModelReaderImpl.java - enumeration is set, default is set and in the enumeration.

The following files only mention ServerVariable

api/src/main/java/org/eclipse/microprofile/openapi/models/servers/Server.java api/src/main/java/org/eclipse/microprofile/openapi/models/servers/ServerVariable.java api/src/main/java/org/eclipse/microprofile/openapi/models/servers/package-info.java api/src/main/java/org/eclipse/microprofile/openapi/OASFactory.java api/src/main/java/org/eclipse/microprofile/openapi/annotations/servers/Server.java api/src/main/java/org/eclipse/microprofile/openapi/annotations/servers/ServerVariable.java api/src/main/java/org/eclipse/microprofile/openapi/annotations/servers/package-info.java

Azquelt commented 5 months ago

(It also raises the question of intermediate values. If you want to remove the defualt from the enum, add a new value, snd set the new value to the default. Is it required to ensure the default is in the enum at every step of the process?)

No, I don't think this is required, the object being in an invalid intermediate state is fine.

benjamin-confino commented 5 months ago

I spoke with Andrew yesterday and learned that we do not add TCK tests for illegal states. Given that, and the results of my audit above, I think this issue only needs an update to the documentation.

Azquelt commented 4 months ago

I think we should doc that default must be one of the values in enum if enum is set. Do this on the default field in the ServerVariable model interface and annotation.