marshmallow-code / marshmallow-code/marshmallow-oneofschema

Child schema's unknown setting in Meta class is ignored

Open
#216 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
142
Forks
44
Avg merge
7h 7m
Merged PRs (30d)
3

Description

The unknown value for a schema's Meta class is overridden and ignored.

import marshmallow
import marshmallow.fields
from marshmallow_oneofschema import OneOfSchema


class Foo:
    def __init__(self, foo):
        self.foo = foo

    def __str__(self) -> str:
        return f"{self.foo=}"

class FooUnknown:
    def __init__(self, foo, extras):
        self.foo = foo
        self.extras = extras

    def __str__(self) -> str:
        return f"{self.foo=} - {self.extras=}"


class FooSchema(marshmallow.Schema):
    foo = marshmallow.fields.String(required=True)

    @marshmallow.post_load
    def make_foo(self, data, **kwargs):
        return Foo(**data)

class FooSchemaUnkown(marshmallow.Schema):
    class Meta:
        unknown = marshmallow.INCLUDE

    foo = marshmallow.fields.String(required=True)

    @marshmallow.post_dump(pass_original=True)
    def add_unknowns(self, output, orig, **kwargs):
        output["extras"] = orig.extras
        return output

    # def load(self, data, *, many=None, partial=None, unknown=None):
    #     """Overriding load to ensure unknown is set to INCLUDE."""
    #     return super().load(
    #         data, many=many, partial=partial, unknown=marshmallow.INCLUDE
    #     )

    @marshmallow.post_load
    def make_foo(self, data, **kwargs):
        return FooUnknown(**data)


class MyUberSchema(OneOfSchema):
    type_schemas = {"foo": FooSchema, "foo-unknown": FooSchemaUnkown}

    def get_obj_type(self, obj):
        if isinstance(obj, Foo):
            return "foo"
        elif isinstance(obj, FooUnknown):
            return "foo-unknown"
        else:
            raise Exception("Unknown object type: {}".format(obj.__class__.__name__))


dump_response = MyUberSchema().dump(Foo(foo="hello"))
assert dump_response == {"foo": "hello", "type": "foo"}

dump_response = MyUberSchema().dump(FooUnknown(foo="hello", extras={"spam": "spam"}))
assert dump_response == {"foo": "hello", "extras": {"spam": "spam"}, "type": "foo-unknown"}

load_response = MyUberSchema().load(
    {"type": "foo", "foo": "hello"}
)
assert isinstance(load_response, Foo)
assert load_response.foo == "hello"

load_response = MyUberSchema().load(
    {"type": "foo-unknown", "foo": "hello", "extras": {"spam": "spam"}}
)
assert isinstance(load_response, FooUnknown)
assert load_response.foo == "hello"
assert load_response.extras == {"spam": "spam"}

With the Meta class in the FooUnknown class I'd expect these asserts to pass. With the current way OneOfSchema works it fails when attempting to load with "extras" set with an error.

marshmallow.exceptions.ValidationError: {'extras': ['Unknown field.']}

However if I uncomment the overridden load function, which forces the load function to be passed the correct unknown field value, all asserts pass correctly.

In my own code I'm using the overriding of load workaround, but I believe the unknown field's value on the child schema should be honored. Would appreciate any advice to the contrary if I'm mistaken however.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the OneOfSchema load path and compare how it forwards the unknown option to the selected child schema. Reproduce the example with FooSchemaUnkown and its Meta.unknown setting; done means the extras input loads successfully without requiring the child load override, while the other assertions continue to pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
50/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.