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

Bug when flattening with a non-existent sequence and colander.drop #299

Open
iwillau opened this issue Aug 31, 2017 · 1 comment
Open

Bug when flattening with a non-existent sequence and colander.drop #299

iwillau opened this issue Aug 31, 2017 · 1 comment

Comments

@iwillau
Copy link

iwillau commented Aug 31, 2017

Hi All!

I have been using colander for year now, but I've never come across this one. I am using Colander to validate some incoming JSON and then flatten that structure. Several portions of the JSON are optional and if they are missing from the input struct I want to ensure they are also missing from the deserialized and subsequently flattened data, hence I don't want to use a default of any kind.

When applying the missing=colander.drop to a SequenceSchema and then flattening a struct that does not contain the sequence it triggers a bug, trying to iterate colander.null. I am presuming that this should do what I am trying, which is to omit that key from flattened output entirely.

Example code to trigger the bug:

import colander
from pprint import pprint

class MySeq(colander.SequenceSchema):
    key = colander.SchemaNode(colander.String())

class MySchema(colander.MappingSchema):
    title = colander.SchemaNode(colander.String())
    items = MySeq(missing=colander.drop)

schema = MySchema()


def example(cstruct):
    print('===================================')
    pprint(cstruct, width=40)
    try:
        deserialized = schema.deserialize(cstruct)
        pprint(deserialized, width=40)
        pprint(schema.flatten(deserialized), width=40)
    except Exception as err:
        print('Error')
        print(err)

cstruct_original = {
    'title': 'My Things',
    'items': ['one', 'two'],
}

cstruct_empty_items = {
    'title': 'My Things',
    'items': [],
}

cstruct_empty_none_items = {
    'title': 'My Things',
    'items': None,
}

cstruct_no_items = {
    'title': 'My Things',
}

example(cstruct_original)
example(cstruct_empty_items)
example(cstruct_empty_none_items)
example(cstruct_no_items)

I have added some tests and fixed the problem in my fork: 65afce1

I'd be happy to create a PR if you want to: master...iwillau:master

Below is the output of the script above before and after my changes:

(env) wwheatley solo:colander $ python example.py 
===================================
{'items': ['one', 'two'],
 'title': 'My Things'}
{'items': ['one', 'two'],
 'title': 'My Things'}
{'items.0': 'one',
 'items.1': 'two',
 'title': 'My Things'}
===================================
{'items': [], 'title': 'My Things'}
{'items': [], 'title': 'My Things'}
{'title': 'My Things'}
===================================
{'items': None, 'title': 'My Things'}
Error
{'items': '"None" is not iterable'}
===================================
{'title': 'My Things'}
{'title': 'My Things'}
Error
'_null' object is not iterable
(env) wwheatley solo:colander $ git checkout master
Switched to branch 'master'
Your branch is ahead of 'origin/master' by 1 commit.
  (use "git push" to publish your local commits)
(env) wwheatley solo:colander $ python example.py 
===================================
{'items': ['one', 'two'],
 'title': 'My Things'}
{'items': ['one', 'two'],
 'title': 'My Things'}
{'items.0': 'one',
 'items.1': 'two',
 'title': 'My Things'}
===================================
{'items': [], 'title': 'My Things'}
{'items': [], 'title': 'My Things'}
{'items': [], 'title': 'My Things'}
===================================
{'items': None, 'title': 'My Things'}
Error
{'items': '"None" is not iterable'}
===================================
{'title': 'My Things'}
{'title': 'My Things'}
{'title': 'My Things'}
@fmigneault
Copy link

For anyone interested about an "easy" solution to handle this, my solution was to insert the following definitions to drop the mapping and sequences with minimal code refactoring when missing=drop.

from colander import MappingSchema as MapSchema, SequenceSchema as SeqSchema

class DropableSchema(colander.SchemaNode):
    def deserialize(self, cstruct):
        if self.default is colander.null and self.missing is colander.drop and cstruct is None:
            return colander.drop
        return super(DropableSchema, self).deserialize(cstruct)

class MappingSchema(DropableSchema, MapSchema):
    """Override the default :class:`colander.MappingSchema` to auto-handle dropping missing definition as required."""


class SequenceSchema(DropableSchema, SeqSchema):
    """Override the default :class:`colander.SequenceSchema` to auto-handle dropping missing definition as required."""

Following will then resolve as expected by dropping the missing s1 definition:

class SchemaA(MappingSchema):
    field = SchemaNode(String())

class SchemaB(MappingSchema):
    s1 = SchemaA(missing=drop)   # optional
    s2 = SchemaA()               # required

SchemaB().deserialize({"s1": {"field": "ok"}, "s2": {"field": "ok"}})
# {'s1': {'field': 'ok'}, 's2': {'field': 'ok'}}
SchemaB().deserialize({"s1": None, "s2": {"field": "ok"}})   # this would raise normally
# {'s2': {'field': 'ok'}}
SchemaB().deserialize({"s2": {"field": "ok"}})
# {'s2': {'field': 'ok'}}

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

2 participants