Models: Are ServerVariables really Extensible?

[jmini]

org.eclipse.microprofile.openapi.models.servers.ServerVariables represents a map of org.eclipse.microprofile.openapi.models.servers.ServerVariable in org.eclipse.microprofile.openapi.models.servers.Server

See spec: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#server-variable-object

I do not think that the map <variable-name, instance> is extensible. I know Swagger-Core has implemented it, but Kaizen Model did not and I think this is correct.

• Swagger-Core: ServerVariables.java L57-L78
• KaiZen: Server.java is using Map<String, ServerVariable>, there is no ServerVariables interface.

Of course ServerVariable is extensible.

[EricWittmann]

You are right - the Server model has a simple map of variable name to ServerVariable. That map is not extensible. We should fix this.

[jmini]

Thank you for the quick feedback!

Well I guess that removing extends Extensible from the ServerVariables interface declaration is not a problem:

microprofile-open-api/api/src/main/java/org/eclipse/microprofile/openapi/models/servers/ServerVariables.java

Lines 25 to 30 in 7a28b39

	/**
	* ServerVariables
	*
	* @see <a href="https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#server-variable-object">ServerVariable Object</a>
	*/
	public interface ServerVariables extends Constructible, Extensible, Map<String, ServerVariable> {

I am wondering if this is possible for a 1.1 version (that should stay backward compatible). Maybe I did not get it.

[EricWittmann]

I think the ServerVariables model should be removed and the variables property of Server should instead just be a Map. What do you think, @arthurdm ?

[jmini]

@EricWittmann : I totally agree, but this is not only a ServerVariables question => #248.

[arthurdm]

hey guys - I totally agree here. Probably just legacy stuff we can improve upon.

How about we tackle this in #248, since that will probably eliminate ServerVariables along with other similar interfaces?

[jmini]

I am not sure anymore about this. Maybe ServerVariables is extensible after all.

I found the corresponding method in KaiZen-Parser:
https://github.com/RepreZen/KaiZen-OpenApi-Parser/blob/172ae39be53dc25187314e2a5f19ad9a709ef6cf/kaizen-openapi-parser/src/main/java/com/reprezen/kaizen/oasparser/model3/Server.java#L51-L74

Usage example:

openApi3.getServers().get(0).setVariablesExtension("xxxx", "yyyy");

[jmini]

The more I look at it, the more confuse I get.

I found the issue discussing if the ServerVariables are extensible or not: OAI/OpenAPI-Specification#975

=> My understanding from Issue and from associated PR is that it was decided to not have extensions for server-variables.

---

Current support in the java implementation

KaiZen-Parser supports it. #245 (comment)

Swagger-Core has code to support it but with commit swagger-api/swagger-core@5e61d6a they have mark it as deprecated and as far as I have tested it, Swagger-Parser do not parse server-variables extensions for the moment (could be an issue).

---

Test spec:

openapi: 3.0.1
info:
  title: Test extensions
  description: API under test
  version: 1.0.7
servers:
- url: https://api.test.com:{port}/{username}
  variables:
    x-test1: extension test
    port:
      default: "9999"
    username:
      default: alice
      enum:
      - alice
      - bob

---

My proposition:

Let ServerVariables extensible for the moment as the 2 java libs supports it.

[arthurdm]

Thanks for digging the OAS history on this @jmini!

I guess the summary is that ServerVariables is a legacy artifact that was removed from the OAS spec during the RC process, but some tools have adopted it during their early implementation.

Since it no longer exists in the spec, I think we should deprecate ServerVariables in 1.1 and remove it in 2.0. So then it becomes less important on whether it's extensible or not, if it goes away. =)

Each individual ServerVariable is still extensible, as per the spec.

[arthurdm]

Agreed on Sept. 10 hangout with conclusion from #245 (comment)

[jmini]

A Model Test for Server#setVariables(Map<String, ServerVariable> variables); should be added, because for the moment there are only tests for the method that is deprecated: (Server#setVariables(ServerVariables variables);)