Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for additionalProperties=False #106

Open
smurfix opened this issue Jan 24, 2018 · 5 comments
Open

Support for additionalProperties=False #106

smurfix opened this issue Jan 24, 2018 · 5 comments

Comments

@smurfix
Copy link

smurfix commented Jan 24, 2018

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
Copy link
Contributor

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
Copy link
Collaborator

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
Copy link
Author

smurfix commented Jan 26, 2018

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
Copy link
Collaborator

beregond commented Feb 3, 2018

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
Copy link

illeatmyhat commented Aug 29, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants