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

compat.dump raises ValidationError when nullable Nested fields (with required subfields) without dump_only of a ResponseSchema are None #305

Open
fcr-- opened this issue Feb 28, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@fcr--
Copy link
Contributor

fcr-- commented Feb 28, 2024

We found this issue when observed a ValidationError being thrown when serializing the response of an endpoint. The curiosity of this issue is that it happens in the compat.dump when all of these are true:

  • There is a Nested field in a ResponseSchema with allow_none=True,
  • The None value is used for that field.
  • That field has a schema with required subfields (in this case it was an id str subfield without a default).

Then by reading the compat.dump code and the called functions, we've seen that setting dump_only=True on the Nested field served as a workaround.

More info:

This bug happens at least on version 3.2.0

@fcr-- fcr-- changed the title compat.dump raises ValidationError when nullable Nested fields (with required subfields) without dump_only are None compat.dump raises ValidationError when nullable Nested fields (with required subfields) without dump_only of a ResponseSchema are None Feb 28, 2024
@fcr--
Copy link
Contributor Author

fcr-- commented Feb 28, 2024

This is how to reproduce the bug:

from flask_rebar import compat, ResponseSchema
from marshmallow import Schema, fields


class N(Schema):
    n = fields.Integer(required=True)


class S(ResponseSchema):
    sub = fields.Nested(N(), required=False, allow_none=True)


obj = {"sub": None}

print("default dump:", S().dump(obj))
print("compat.dump:", compat.dump(S(), obj))

which when run:

(venv) francisco_castro@GLOBQW4HV7DF7V bridge % python3 repro.py
default dump: {'sub': None}
Traceback (most recent call last):
  File "/Users/francisco_castro/proj/bridge/repro.py", line 16, in <module>
    print("compat.dump:", compat.dump(S(), obj))
                          ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/francisco_castro/proj/bridge/venv/lib/python3.11/site-packages/flask_rebar/compat.py", line 51, in dump
    schema.load(filtered.loadable)  # trigger validation
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/francisco_castro/proj/bridge/venv/lib/python3.11/site-packages/marshmallow/schema.py", line 723, in load
    return self._do_load(
           ^^^^^^^^^^^^^^
  File "/Users/francisco_castro/proj/bridge/venv/lib/python3.11/site-packages/marshmallow/schema.py", line 910, in _do_load
    raise exc
marshmallow.exceptions.ValidationError: {'sub': {'n': ['Missing data for required field.']}}

@fcr--
Copy link
Contributor Author

fcr-- commented Feb 28, 2024

So, this is where the empty dict came from:

loadable = dict()
for k, v in non_dump_only.items():
field = schema.fields[output_to_input_keys.get(k, k)]
# see if we have a nested schema (using either Nested(many=True) or List(Nested())
field_schema = None
if isinstance(field, fields.Nested):
field_schema = field.schema
elif isinstance(field, fields.List) and isinstance(
field.inner, fields.Nested
):
field_schema = field.inner.schema
if field_schema is None:
loadable[k] = v
else:
field_filtered = filter_dump_only(field_schema, v)
loadable[k] = field_filtered.loadable

Which then reaches compat.dump's schema.load(filtered.loadable), resulting on the error shown here.

@airstandley airstandley added the bug Something isn't working label Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants