jazzband / jsonmodels

jsonmodels is library to make it easier for you to deal with structures that are converted to, or read from JSON.
http://jsonmodels.readthedocs.org/en/latest/
BSD 3-Clause "New" or "Revised" License
334 stars 51 forks source link

Support for additionalProperties=False #106

Open smurfix opened 6 years ago

smurfix commented 6 years ago

Even though the generated JSON schema contains "additionalProperties: False" for every structure, actually setting a property that's not in the model does not cause a validation error.

It should. in the meantime, emitting "additionalProperties: False" is wrong, strictly speaking.

avrahamshukron commented 6 years ago

Well - although you can set additional attributes to an instance, these attribute won't make it to the actual JSON.

from jsonmodels.models import Base
from jsonmodels.fields import *

class Foo(Base):
    age = IntField()

f = Foo()
f.bar = "hello"
f.age = 4
f.to_struct()

The above code will yield {'age': 4} so it is not really possible to add additional properties that are not defined on the model.

You could make the code raise an exception if a new attribute is introduces, but this is not very "pythonic", and does not provide a real value IMO.


Notice that you can add new fields to an existing models.Base subclass, and that new field will be reflected in the JSON, but that is not "additionalProperties". You are actually changing the schema at runtime and adding actual properties:

class Foo(Base):
    age = IntField()

Foo.to_json_schema()
Out[1]: 
{'additionalProperties': False,
 'properties': {'age': {'type': 'number'}},
 'type': 'object'}

Foo.bar = StringField()
Foo.to_json_schema()
Out[2]: 
{'additionalProperties': False,
 'properties': {'age': {'type': 'number'}, 'bar': {'type': 'string'}},
 'type': 'object'}
beregond commented 6 years ago

Hey @smurfix, thanks for the feedback!

Answering to your notice - well, the intention was different for that. "additionalProperties: False" is intended only for structures, when you are dealing with json itself. In models you may find it usefull to actually have extra properties (and functions) - that is why model ignores all that are NOT fields.

Imagine case like this (pseudocode):

class Foo(Base):

    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.birth_date = datetime.now() - relativedelta(years=self.age)

    age = IntField()
    birth_date = None

foo = Foo(age=4)

and with this you could easily use birth date even if it isn't given explicite by your data source without calculating it each time in many places in code. This approach allows for flexibility in extending models for your use cases.

smurfix commented 6 years ago

OK, fair enough. However, models should describe their data exhaustively. Thus it makes sense to forbid random elements. The easiest way to specify that is to not mention it in the class. Thus, in your example, if birth_date = None was missing, then assigning to self.birth_date should be an error.

beregond commented 6 years ago

Hmm, additionalProperties: false is feature only for json schema actually, so yes, we could add some parser that would fail during json -> jsonmodel translation 🤔

illeatmyhat commented 5 years ago

For those getting here from google, who have models where it is emphatically impossible to describe their data exhaustively (i.e. for a set of user-defined parameters) I've sufficiently simulated additionalProperties: True by simply making a DictField like so

class DictField(fields.BaseField):
    types = dict

Since it's a Field instead of a model, it requires that all of your arbitrary properties be confined into their own scope rather than mingling with the rest of the validated fields.