Items in Schema should be a single SchemaProxy instead of an array

[TristanSpeakEasy]

https://json-schema.org/understanding-json-schema/reference/array.html

Items can either be a single schema (for v2, v3.0 and v3.1) or a boolean (v3.1).

And along with PrefixItems there should be a Contains field as well which is a single schema along with minContains / maxContains fields.

[daveshanley]

Yes, this is a mistake on my part. It was during some copy pasta work. Good call on the missing JSON Schema properties ( I expect there are a few others missing also).

I'll look into this as soon as possible.

[daveshanley]

I've changed this in the high level model so Items is a single SchemaProxy and added contains and the min/max properties also. However when I was thinking about the or a boolean part, I hit a crossroad and i'd like some of your input.

In order to handle these dynamic values, in the low level model I used a SchemaDynamicValue that can hold two types (A or B) https://github.com/pb33f/libopenapi/blob/main/datamodel/low/base/schema.go#L15

This API is awkward, and there is a reason I didn't bring it up high, but it allows multiple types to sit in a single value across versions. The question I have is:

Would this API be appropriate up high? because to use a single property, the model would need to use this dynamic type approach or use interfaces.

Another question: What kind of signature/interface/design would you like to see?

I have the rest of the code ready in a branch: https://github.com/pb33f/libopenapi/blob/issue28-contains/datamodel/high/base/schema.go#L58

Let me know what you think.

[TristanSpeakEasy]

So kin-openapi as an example adds an additional field to the structure when a field can be represented by multiple types, here is an example for AdditionalProperties https://github.com/getkin/kin-openapi/blob/master/openapi3/schema.go#L165 which could be one option but does mean the model doesn't match the spec exactly.

Or a container type that has a .Type() method which then allows you to case the .Value (any type) to the correct type to access the value?

[daveshanley]

I'm open to suggestions on this one. Personally, it makes more sense to me to create a MultiType container (which is what the low-level does), but it might be too ugly for a high-level API.

Which of the two designs do you prefer? Another property or a multi-type container?

@danielgtaylor, what do you think?

[TristanSpeakEasy]

I don't mind working with a container if it helps reduce some of the boiler plate around casting to types, maybe taking advantage of generics to return the correct type from a get method or similar

[danielgtaylor]

There's already some high-level precedent for using separate fields, e.g. see the exclusive min/max:

    // In versions 2 and 3.0, this ExclusiveMaximum can only be a boolean.
    ExclusiveMaximumBool *bool

    // In version 3.1, ExclusiveMaximum is an integer.
    ExclusiveMaximum *int64

    // In versions 2 and 3.0, this ExclusiveMinimum can only be a boolean.
    ExclusiveMinimum *int64

    // In version 3.1, ExclusiveMinimum is an integer.
    ExclusiveMinimumBool *bool

The downside is it's harder to tell which one was set vs. a multi-type container like the low-level model provides where you can query for whether a or b is set. I don't have a strong preference on this one but we may want to think about consistency of the model, i.e. if Items gets a multi-type container then maybe the exclusive values should as well.

Another option is to have less of a one-to-one mapping in the high-level model via abstraction. items: false is semantically saying that no additional items are allowed when used with prefixItems so you could add a high-level additionalItems bool. Same with the exclusive min/max - rather than me managing all that mess you could just provide a convenience IsMinimumExclusive() bool or something along those lines. I guess it's a question of how much abstraction we can stomach knowing things may change again with Open API 3.2.

I know I'm an early adopter (you can see libopenapi in action right now in Restish 0.15.0) so don't mind if we decide it's best to break and I'll do some updating for the next release :+1: My test coverage of the Open API code is pretty good at this point.

[daveshanley]

Please check PR #40

[daveshanley]

This is resolved in v0.4.0 And the use of DynamicValue will allow high-level models to support multi-types moving foward.