Contrary to the documentation, 'default' affects

[michaellenaghan]

Colander has a page devoted to describing Colander's use of Null and Drop values. The product's current behaviour doesn't match that description. Specifically, default should only affect serialization—but it currently affects deserialization too, if default is set to colander.drop.

Consider this code:

import colander

class Person(colander.MappingSchema):
    name = colander.SchemaNode(
        colander.String()
    )
    age = colander.SchemaNode(
        colander.Int(),
        validator=colander.Range(0, 200)
    )
    hair_color = colander.SchemaNode(
        colander.String(),
        default=u'default brown',
        missing=u'missing brown'
    )

schema = Person()

print('serialized = {}'.format(schema.serialize({
    'name': 'Fred',
    'age': 20
})))

print('deserialized = {}'.format(schema.deserialize({
    'name': 'Fred',
    'age': 20
})))

It produces:

serialized = {'age': '20', 'name': u'Fred', 'hair_color': u'default brown'}
deserialized = {'age': 20, 'name': u'Fred', 'hair_color': u'missing brown'}

That's exactly what we expect: 'hair_color' wasn't passed in, so during serialization we got the default value and during deserialization we got the missing value.

Now let's change default to colander.drop:

import colander

class Person(colander.MappingSchema):
    name = colander.SchemaNode(
        colander.String()
    )
    age = colander.SchemaNode(
        colander.Int(),
        validator=colander.Range(0, 200)
    )
    hair_color = colander.SchemaNode(
        colander.String(),
        default=colander.drop,
        missing=u'missing brown'
    )

schema = Person()

print('serialized = {}'.format(schema.serialize({
    'name': 'Fred',
    'age': 20
})))

print('deserialized = {}'.format(schema.deserialize({
    'name': 'Fred',
    'age': 20
})))

It produces:

serialized = {'age': '20', 'name': u'Fred'}
deserialized = {'age': 20, 'name': u'Fred'}

hair_color was dropped during serialization—but it was also dropped during deserialization. We should have gotten the missing value instead.

The problem seems to be that Mapping.serialize() and Mapping.deserialize() both call Mapping._impl(), and Mapping._impl() consults default. There's a similar problem in Sequence: Sequence.serialize() and Sequence.deserialize() both call Sequence._impl(), and Sequence._impl() consults default. Deserialization should never consult default.

At least where mappings are concerned, this is a very old bug. It was introduced in this commit in 2014. That commit was an attempt to fix this bug. That bug actually describes what I'm trying to do: have a default value of colander.drop and a missing value of None. (The Sequence bug was introduced in this commit which appears to have copied the Mapping code.)

My current workaround is to use deferred() and bind() to instantiate one variation of the schema for serialization and another one for deserialization.

[tisdall]

does any of #237 help?

[michaellenaghan]

Not sure what you mean by "help". :-)

I believe the original design of colander (as extensively documented!) is reasonable and correct. I also believe that using colander.drop in default is reasonable and correct. What I think is wrong is that one specific value of default, colander.drop, affects _de_serialization. I think that was an inadvertent change caused by shared code and a hole in the test suite.

I was able to find a workaround—not based on deferred() and bind() as it turns out—so I'm not stuck. But I still think this is a bug worth fixing.

Here's a fix I was considering but didn't try. serialize() and deserialize() both call _impl() (in both Mapping and Sequence). _impl() then checks default. I was thinking that it might work to pass a second parameter into _impl(), the name of the attribute that should be checked: "default" for serialize(), "missing" for deserialize(). I didn't try it because i'm very new to colander—I just started with it last week. I don't even know what I don't know yet.

[tisdall]

Yeah, that does seem like a bug. I'm curious if any of the existing tests
fail after fixing this. It may have been missed because 'drop' wasn't well
documented for 'default' until recently.
On May 23, 2016 5:23 PM, "michaellenaghan" notifications@github.com wrote:

Not sure what you mean by "help". :-)

I believe the original design of colander (as extensively documented!) is
reasonable and correct. I also believe that using colander.drop in default
is reasonable and correct. What I think is wrong is that one specific value
of default, colander.drop, affects _de_serialization. I think that was an
inadvertent change caused by shared code and a hole in the test suite.

I was able to find a workaround—not based on deferred() and bind() as it
turns out—so I'm not stuck. But I still think this is a bug worth fixing.

Here's a fix I was considering but didn't try. serialize() and
deserialize() both call _impl() (in both Mapping and Sequence). _impl()
then checks default. I was thinking that it might work to pass a second
parameter into _impl(), the name of the attribute that should be checked:
"default" for serialize(), "missing" for deserialize(). I'm very new to
colander, though—just started with it last week—so I don't know enough to
know whether or not that actually makes sense.

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#262 (comment)