Allow to loads undeclared field with meta attribute (fix #934) #957

noirbizarre · 2015-04-13T16:52:10Z

This pull request try to fix #934 by adding an optionnal meta attribute: strict.

This attribute disable field existence check on data load, but not on constructor calls.

It also document both behaviors.

landscape-bot · 2015-04-13T16:58:42Z

Code quality remained the same when pulling ddbcc8e on noirbizarre:metastrict into 19dc312 on MongoEngine:master.

coveralls · 2015-04-13T17:13:01Z

Coverage decreased (-0.0%) to 73.02% when pulling ddbcc8e on noirbizarre:metastrict into 19dc312 on MongoEngine:master.

landscape-bot · 2015-04-13T20:32:51Z

Code quality remained the same when pulling 52c7b68 on noirbizarre:metastrict into 19dc312 on MongoEngine:master.

gordol · 2015-04-13T21:06:36Z

Why a duplicate fix for the same issue?

Check my pull request over here: https://github.com/MongoEngine/mongoengine/pull/953/files#diff-e3e316e6686a3db66d2e679634f57f7f

I think you might need to refactor a bit... This test fails on your branch:

    def test_undefined_field_allowed_read(self):
        """
        Test read from data wtih undefined fields #934 #953
        """

        class A(Document):
            test = StringField(required=True)
            test2 = StringField(required=True)
            meta = {'collection': 'undefined_test'}

        class B(Document):
            test = StringField(required=True)
            meta = {'collection': 'undefined_test', 'strict': True}

        a = A()
        a.test = 'one'
        a.test2 = 'two'
        a.save()

        try:
            b = B.objects.get()
            self.assertEqual(b.test, a.test)
        except Exception:
            self.fail()

gordol · 2015-04-13T21:10:38Z

Oh wait, nevermind... I see what you did. B() meta should set strict to False, my mistake. I pulled the test from my branch and replaced 'ignore_undefined_fields' with 'strict', not thinking about the logic being inverse.

noirbizarre · 2015-04-13T21:52:00Z

I submit the pull-request because I started it before you submitted yours and before I saw the discussion.
Both solutions are a little bit differents so I submitted mine too.

Feel free to pick some code or documentation.
The only thing that matter to me is that there is a proper fix and both behaviors are specified and documented.

Yeah, you should set strict to False if you want to load some unspecified fields.
The constructor is always checked, even with strict set to False.

By the way, you should avoid the following pattern:

try:
    # do whatever raises an exception
except Exception:
    self.fail()

Simply let the exception be catched by the testing framework.
It will have the same effect but the stacktrace wil be way more useful.

gordol · 2015-04-13T21:58:33Z

Simply let the exception be catched by the testing framework.
It will have the same effect but the stacktrace will be way more useful.

Indeed.

Thanks.

coveralls · 2015-04-13T23:16:36Z