Skip to content

Commit

Permalink
Fix bug overwriting explicitly declared fields
Browse files Browse the repository at this point in the history
Before, the `direct_fields` dict holding all fields declared in the custom
serializer was overwritten with all implicitly defined fields from the `Meta`
class inside the custom serializer.

That means if someone declared anything other than a standard Field for an
attribute of the model, this would have been overwritten with a standard Field
instance instead of keeping the explicitly declared field type, effectively
nullifying the effect of the explicit declaration.

Now, if someone was to declare a field explicitly, the SerializerMeta class
will not overwrite this field  with a standard Field instance in the
`direct_fields` dict, but will instead skip this field, thus preserving the
explicit declaration.

Also add some more comments explaining this in the code.
  • Loading branch information
jacobstoehr committed Dec 19, 2019
1 parent 2e53be0 commit d2b72b6
Showing 1 changed file with 30 additions and 13 deletions.
43 changes: 30 additions & 13 deletions serpy/serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,26 @@ def _compile_fields(field_map, serializer_cls):
]

@staticmethod
def _get_list_of_fields(model_fields, fields, exclude):
def _get_implicit_fields(model_fields, fields, exclude):
if fields == '__all__':
fields = model_fields
elif not fields and exclude:
fields = [
field for field in model_fields
if field.name not in exclude
]
# this implicitly handles the case when `fields` is set and `exclude`
# isn't. Then all fields declared will be returned without any
# modification.
return fields

@staticmethod
def _update_direct_fields(direct_fields, explicit_fields, implicit_fields):
for field in implicit_fields:
if field.name not in explicit_fields:
direct_fields[field.name] = Field()
return direct_fields

def __new__(cls, name, bases, attrs):
# Fields declared directly on the class.
direct_fields = {}
Expand Down Expand Up @@ -88,12 +98,19 @@ def __new__(cls, name, bases, attrs):
)
# Django models
if getattr(model, '_meta', None) is not None:
fields = cls._get_list_of_fields(model._meta.fields, fields, exclude)
direct_fields.update(
{
field.name: Field()
for field in fields
}
# Define `implicit_fields` as fields that have not been
# explicitly declared in the custom serializer class. They are
# deduced from the model declaration.
implicit_fields = cls._get_implicit_fields(
model._meta.fields, fields, exclude
)
# Define `explicit_fields` as fields that have been explicitly
# declared in custom serializer class. Dont update them with a
# simple `Field` instance. Instead, keep the Fieldtype that has
# been declared in the custom Serializer
explicit_fields = direct_fields.keys()
direct_fields = cls._update_direct_fields(
direct_fields, explicit_fields, implicit_fields
)
# SQLAlchemy model
# This has to be like this because SQLAlchemy has not implemented
Expand All @@ -102,12 +119,12 @@ def __new__(cls, name, bases, attrs):
# the getattr returned `None`, if yes, that would be an instance of
# `None`, so we can say that this is not an SQLAlchemy model.
elif not isinstance(getattr(model, '__table__', None), type(None)):
fields = cls._get_list_of_fields(model.__table__.columns, fields, exclude)
direct_fields.update(
{
field.name: Field()
for field in fields
}
implicit_fields = cls._get_implicit_fields(
model.__table__.columns, fields, exclude
)
explicit_fields = direct_fields.keys()
direct_fields = cls._update_direct_fields(
direct_fields, explicit_fields, implicit_fields
)
else:
raise RuntimeError(
Expand Down

0 comments on commit d2b72b6

Please sign in to comment.