marshmallow-oneofschema icon indicating copy to clipboard operation
marshmallow-oneofschema copied to clipboard

Allow use of Schema hooks on OneOfSchema

Open sirosen opened this issue 3 years ago • 2 comments

NOTE: this change is very mildly backwards-incompatible, but since 3.0 is coming up for this lib, I think it's okay.

Rather than overriding the dump() and load() methods of the Schema class, override _serialize and _deserialize, which are the "concrete" steps in schema loading and dumping which handle loading or dumping on a field-by-field basis.

This still uses load() and dump() methods of the type schemas being used, but it happens between the various hooks which may run on the OneOfSchema instance.

Add a test that checks that a post_dump hook to remove the type field works.

The most significant downside of this change is that it makes use of several private APIs within marshmallow. Not only are _serialize and _deserialize private methods, but the error_store object which is used here is also considered private (per marshmallow docs).

In order to better guarantee behavior near-identical to marshmallow, several methods from marshmallow.utils have been copied in-tree here.

One notable API change here is that arbitrary keyword arguments are no longer being passed from OneOfSchema.load() and OneOfSchema.dump() down into the type schemas' load and dump methods. As a result, you cannot specify a load or dump parameter here and expect it to take effect. With the switch to overriding _serialize and _deserialize, there is no practical way to pass parameters like that.

closes #4

sirosen avatar Nov 10 '20 20:11 sirosen

Reviewing existing issues, I believe this will also resolve #74 , since it allows marshmallow.Schema._do_load to be called.

sirosen avatar Nov 10 '20 21:11 sirosen

Something which has been bothering me about this since I opened it is that load and dump were passing through arbitrary kwargs, which seemed odd. I just noticed that this was added in #111 to allow the arguments to load and dump to get to the child schemas. This change will break that use-case, and I'm not sure what to do about it.

On the one hand, this implementation strikes me as much simpler and makes all of the hooks work for free. On the other hand, I don't see how to make this work for the case cited in #111 without additional upstream changes in marshmallow. (If kwargs to load passed all the way down to _deserialize, we could use them that way.)

sirosen avatar Nov 23 '20 21:11 sirosen