Server Variable's enum now MUST not be empty

[mattpolzin]

See the release notes, and new spec.

Work off of the release/3_0 branch and work on types in the OpenAPIKit (not OpenAPIKit_3_0) module.

[mattpolzin]

This is probably a good candidate for a new on-by-default validation instead of failure to encode/decode. See other validations in Validation+Builtins.swift and the list of on-by-default validations in the Validator type's 0-arity initializer.

[mihaelamj]

@mattpolzin This issue encompasses three things, in my opinion:

1. Server Variable's enum should change from a required property (that is sometimes empty currently) to an optional property of the variable.
2. If enum is present as a property of Variable, it must not be empty.
3. The default property of the variable should not be empty and should contain one value from the enum array (if the variable has enum property).

[mattpolzin]

@mihaelamj good points; I did not spot the possibility for the enum to be unspecified (omitted) but upon rereading you are absolutely right.

1. Server Variable's enum should change from a required property (that is sometimes empty currently) to an optional property of the variable.

Sounds right.

2. If enum is present as a property of Variable, it must not be empty.

Yep; and I think the best way to accomplish this is via a default-on validation (I mentioned this in an above comment, just reiterating here.

3. The default property of the variable should not be empty and should contain one value from the enum array (if the variable has enum property).

Yep; I originally missed this requirement, but you are quite right. This strikes me as another thing that would make a good default-on validation.

The reason I think we should go for validations for both (2) and (3) is that tackling the logic around those requirements inside decoding logic means if someone violates them then the whole document fails to decode -- I think these are minor enough infractions for it to be worth successfully decoding the OpenAPI but then failing validation (and subsequently allowing for some opportunity for tooling to correct the invalid state before re-encoding).

[mihaelamj]

The reason I think we should go for validations for both (2) and (3) is that tackling the logic around those requirements inside decoding logic means if someone violates them then the whole document fails to decode -- I think these are minor enough infractions for it to be worth successfully decoding the OpenAPI but then failing validation (and subsequently allowing for some opportunity for tooling to correct the invalid state before re-encoding).

Agree. It is very important for the user to be able to see their failing document decoded, with rich errors that enable the user to fix them.

[mihaelamj]

@mattpolzin I'm working on validators, and I've noticed that Server Variable's default is required: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.1.0.md#serverVariableObject

The way it is coded now is that the initializer will throw if the default value is missing.

default = try container.decode(String.self, forKey: .default)

Should we make it default to empty string

default = try container.decodeIfPresent(String.self, forKey: .default) ?? ""

And add a new validator for empty default:

public static var serverVariablesDefaultIsNotEmpty: Validation<OpenAPI.Server.Variable>

[mihaelamj]

@mattpolzin I've put up a PR for this: https://github.com/mattpolzin/OpenAPIKit/pull/214

I did not make the changes to default obviously. If we decide to do it, it may be a new PR.

[mattpolzin]

@mattpolzin I'm working on validators, and I've noticed that Server Variable's default is required: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.1.0.md#serverVariableObject

The way it is coded now is that the initializer will throw if the default value is missing.

default = try container.decode(String.self, forKey: .default)

Should we make it default to empty string

default = try container.decodeIfPresent(String.self, forKey: .default) ?? ""

And add a new validator for empty default:

public static var serverVariablesDefaultIsNotEmpty: Validation<OpenAPI.Server.Variable>

This seems like a good idea (as a separate ticket/PR as you suggest). As much as I want the structure to only be valid if the OpenAPI document is valid, it is always a bummer when an otherwise-ok document cannot be worked with at all because of some really small detail like a missing default on a server variable!

If you're willing to create a ticket for this that would be great! Otherwise I will try to remember to create the ticket myself when I get the time. There's no obligation to take this work on yourself regardless of whether you create the ticket; I appreciate the good idea either way!