Dorthu / openapi3

A Python3 OpenAPI 3 Spec Parser
BSD 3-Clause "New" or "Revised" License
118 stars 47 forks source link

Support for additionalProperties #63

Open mephinet opened 2 years ago

mephinet commented 2 years ago

This pull request adds support for validation of OpenAPI specs that make use of the additionalProperties feature. For an type=object, additionalProperties and properties can both be specified, or only one of them. When a schema defines additionalProperties, no__slots__ is set in the generated Model class, so that the additional attributes can be stored in the __dict__ of the instance. As with normal properties, no type checking for string/int/... is performed.

Dorthu commented 2 years ago

Thanks for the submission!

Looking at the OpenAPI definition and the relevant parts of JSON Schema, it looks like additionalProperties is intended to be either false (to explicitly say no additional properties are allowed) or a schema describing what is allowed (i.e. {"type": "string"}). I couldn't find any examples of an "additionalProperties": true that allowed all additional properties for a schema. However, JSON Schema's reference article notes that by default, any additional properties are allowed.

I'm going to do more research as to how to interpret what the intended behavior is. However, testing this change, I notice some inconsistencies between the behavior of classes with additionalProperties and those without:

In [4]: # r has additionalProperties set to true
   ...: print(r)
{}

In [5]: # lt does not
   ...: print(lt)
{'id': 'g6-nanode-1', 'label': 'Nanode 1GB', 'disk': 25600, 'class': 'nanode', 'price': {'hourly': 0.0075, 'monthly': 5.0}, 'addons': {'backups': {'price': {'hourly': 0.003, 'monthly': 2.0}}}, 'network_out': 1000, 'memory': 1024, 'successor': None, 'transfer': 1000, 'vcpus': 1, 'gpus': 0}

In [6]: r.__slots__
Out[6]: ['_raw_data', '_schema']

In [7]: lt.__slots__
Out[7]: dict_keys(['id', 'label', 'disk', 'class', 'price', 'addons', 'network_out', 'memory', 'successor', 'transfer', 'vcpus', 'gpus'])

In [8]: r._schema

In [9]: lt._schema
Out[9]: <<class 'openapi3.schemas.Schema'> ['components', 'schemas', 'LinodeType']>

In [10]: r._raw_data

In [11]: lt._raw_data
Out[11]:
{'id': 'g6-nanode-1',
 'label': 'Nanode 1GB',
 'price': {'hourly': 0.0075, 'monthly': 5.0},
 'addons': {'backups': {'price': {'hourly': 0.003, 'monthly': 2.0}}},
 'memory': 1024,
 'disk': 25600,
 'transfer': 1000,
 'vcpus': 1,
 'gpus': 0,
 'network_out': 1000,
 'class': 'nanode',
 'successor': None}

I think that at the very least, these should behave the same way when used as above.

commonism commented 2 years ago

https://github.com/APIs-guru/openapi-directory/search?q=additionalProperties

I'm rolling with

aiopenapi3/v31/schemas.py:    additionalProperties: Optional[Union[bool, "Schema"]] = Field(default=None)
aiopenapi3/v20/schemas.py:    additionalProperties: Optional[Union[bool, "Schema", Reference]] = Field(default=None)
aiopenapi3/v30/schemas.py:    additionalProperties: Optional[Union[bool, "Schema", Reference]] = Field(default=None)
chriswhite199 commented 2 years ago

@Dorthu:

Looking at the OpenAPI definition and the relevant parts of JSON Schema, it looks like additionalProperties is intended to be either false (to explicitly say no additional properties are allowed) or a schema describing what is allowed (i.e. {"type": "string"}). I couldn't find any examples of an "additionalProperties": true that allowed all additional properties for a schema. However, JSON Schema's reference article notes that by default, any additional properties are allowed.

The swagger.io spec (https://swagger.io/specification/#properties) does note the following:

additionalProperties - Value can be boolean or object. Inline or referenced schema MUST be of a Schema Object and not a standard JSON Schema. Consistent with JSON Schema, additionalProperties defaults to true

The Understanding JSON article does note:

By default any additional properties are allowed.

So this would imply that additionalProperties: true is the default (as the swagger spec noted), and you only need to reach for false (when you want to be strict) or a schema when you want to be somewhat lenient.

Dorthu commented 2 years ago

Thanks for the excellent references @chriswhite199. In light of that, I think the correct behavior is to: