[BUG] ModelUtils.getAdditionalProperties returns ObjectSchema for "true"

[padamstx]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator (example)?
• Have you tested with the latest master to confirm the issue still exists?
• Have you searched for related issues/PRs?
• What's the actual output vs expected output?
• [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

If a schema contains additionalProperties: true, that is supposed to be equivalent to additionalProperties: {}, which implies that arbitrary properties of ANY type (string, integer, object, etc.) may be added to the object/class/struct that is generated for that schema.
However, ModelUtils.getAdditionalProperties() will return an instance of ObjectSchema in this case rather than an instance of Schema. This would be equivalent to a schema with this value for the additionalProperties field:

 additionalProperties:
 type: object

This would imply that arbitrary properties added to the object/class/struct for that schema should be an "object" (i.e. Map<String,Object> in Java, map[string]interface{} in Go, etc.).

openapi-generator version

This was observed in an old version of the project (3.3.3) and is still present in the latest from the master branch.

OpenAPI declaration file content or url

Generation Details

Steps to reproduce

Related issues/PRs

Suggest a fix

Modify ModelUtils.getAdditionalProperties() so that it returns an instance of Schema instead of ObjectSchema in the scenario where additionalProperties: true is detected.

[DSuds]

{} is an Object with no properties. Implying to me that the additonalProperties is best considered a separate object whose properties should be considered as part of the schema.

The additional properties need to be a mapping of string property names to objects as constrained by the additionalProperties type so wouldn't they be stored in an object schema? If this works functionally I don't see why the implementation methodology is a bug. (But I'm new to this so be kind!)

[padamstx]

The value of a schema's additionalProperties field is either a schema or a boolean, where the value true is interpretted as {}.
So, additionalProperties: true is equivalent to additionalProperties: {} (i.e. an "empty" schema definition, which does not imply "object" schema IMO).

Technically from a JSON schema validation standpoint, the schema {} would allow values of ANY (json-supported) type (i.e. a string, an integer, a list, an object, etc.) simply because it omits the "type" field altogether.

So, if we interpret additionalProperties: {} as "arbitrary properties must be objects", that is an overly restrictive interpretation, IMO. Certainly, if we defined additionalProperties within a schema like this:

 ...
 additionalProperties:
 type: object

that would imply that arbitrary properties must be an "object" of some sort (e.g. { "foo": "bar" }).
But something like this:

 ...
 additionalProperties:
 description: a schema with no type field specified

would allow arbitrary properties of any type

[DSuds]

Well I think we are agreeing that , additionalProperties: description: a schema with no type field specified
is the same as additionalProperties: {"description": "a schema with no type field specified"} and like additionalProperties: {} would not restrict the type of parameters. I just think that the set of additional parameters all need to have a name and value. So given if an object

 name: 
 type : string
additionalProperties: true

and JSON
{"name":"myObj", "adProp1":"string", "adProp2":0.37, "adprop3":{"foo":"bar"}}
I would expect getAdditionalProperties to return an Object with the additional properties
{"adProp1":"string", "adProp2":0.37, "adprop3":{"foo":"bar"}}
which would follow an ObjectSchema.

[DSuds]

Figured out where I went sideways. :) My head was in instances not models.
To support boolean or a set of properties/constraint you would need an object, but since true is defined as {} and we can represent false with null an ObjectSchema seems like the best thing to hold all the allowable things in additonalProperites definition. I guess I think {} is an inline empty JSON object and not a schema in the same way that what follows properites is a set of <String, object> mappings.

Where you say

So, if we interpret additionalProperties: {} as "arbitrary properties must be objects",
I think we are only interpreting that additionalProperites will be described by objects and the set of addtionalProperties can be represented by an object but not constraining the type of the additionalProperites.

[padamstx]

My interpretation of the OpenAPI spec is that the additionalProperties field within a schema defines the type of VALUES that can be specified for arbitrary properties. Consider these schemas:

components:
 schemas:
 S1:
 description: allows for arbitrary properties of type string
 type: object
 properties:
 prop1:
 type: string
 additionalProperties:
 type: string

 S2:
 description: allows for arbitrary properties of "any" JSON-supported type
 type: object
 properties:
 prop1:
 type: string
 additionalProperties: {}

 S3:
 description: allows for arbitrary properties whose values are themselves objects (i.e. collection of properties/values)
 type: object
 properties:
 prop1:
 type: string
 additionalProperties:
 type: object

Example JSON instances for each of these schemas might look like this:

S1:
{ "prop1": "value1", "foo1": "bar1", "foo2": "bar2" }
(one defined string property, plus two arbitrary string properties)

S2:
{ "prop1": "value1", "foo1": true, "foo2": 32 }
(one defined string property, plus two arbitrary properties that could be of any type)

S3:
{ "prop1": "value1", "foo1": { "field1": "value1", "field2": "value2"}, "foo2": { "field3": "value3"}}
(one defined string property, plus two arbitrary properties whose values are objects)

My main point here is that the schema specified as the value of the additionalProperties field defines the type of arbitrary properties ("foo1", "foo2" in the examples above) that can be added to a model instance, above and beyond any explicitly-defined properties ("prop1" in the examples above).

Following this line of thinking, additionalProperties: true or additionalProperties: {} would imply that arbitrary properties should have a value whose type is any supported JSON type and not necessarily an "object", strictly speaking.
However, the behavior of ModelUtils.getAdditionalProperties() is such that it returns an instance of ObjectSchema for this scenario, but IMO it should instead return an instance of Schema (with no value for the "type" field, which would imply any supported type - included but not limited to "object").

[padamstx]

👍