fuhrysteve / marshmallow-jsonschema

JSON Schema Draft v7 (http://json-schema.org/) formatting with marshmallow
MIT License
209 stars 72 forks source link

`additionalProperties` support #77

Closed atmo closed 5 years ago

atmo commented 5 years ago

This adds an ability to specify additionalProperties value for generated jsonschema either via Meta class:

class Meta:
    additional_properties = True/False

or for marshmallow 3 it can be deduced from unknown field: additionalProperties will be set to True for unknown=INCLUDE, to False for unknown=RAISE and an exception will be raised for unknown=EXCLUDE (as it is not possible to emulate such a behavior in jsonschema).

There is no automatic deduction for additionalProperties value for marshmallow 2, obviously.

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 47f09f0ee40a2742b6d258fde67450898af04ecc on atmo:additional_properties into 71614e0a7a4237afe132b5e63a844efcdc7401bc on fuhrysteve:master.

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 13033ecc6929fc999b2597a9ceb17eeab51a21ad on atmo:additional_properties into 71614e0a7a4237afe132b5e63a844efcdc7401bc on fuhrysteve:master.

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 13033ecc6929fc999b2597a9ceb17eeab51a21ad on atmo:additional_properties into 71614e0a7a4237afe132b5e63a844efcdc7401bc on fuhrysteve:master.

atmo commented 5 years ago

Things to think about:

atmo commented 5 years ago

After giving it a little bit more thought I've decided to set additionalProperties to False for both versions of marshmallow by default. The behavior is 100% equivalent for marshmallow 3 as it raises on unknown values. For marshmallow 2 the idea is that this change is backwards compatible and also more user friendly as users would not want to have unknown fields in their data in this case anyway.

atmo commented 5 years ago

@fuhrysteve , what are your thoughts on this?

fuhrysteve commented 5 years ago

Makes sense to me!