From 2f5186be87deb7a2b8ea1bd884b4dc2b882f3ccd Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Tue, 17 Dec 2019 10:55:03 +0200 Subject: [PATCH] Use a metaclass, generate fields on definition time, _fields_cache -> base_fields --- docs/api-guide/serializers.md | 2 +- rest_framework/serializers.py | 170 ++++++++++++++++++--------------- tests/test_model_serializer.py | 98 ++++++++----------- 3 files changed, 131 insertions(+), 139 deletions(-) diff --git a/docs/api-guide/serializers.md b/docs/api-guide/serializers.md index d4d8177b2..29c19014b 100644 --- a/docs/api-guide/serializers.md +++ b/docs/api-guide/serializers.md @@ -587,7 +587,7 @@ For full details see the [serializer relations][relations] documentation. ## Customizing field mappings -The ModelSerializer class also exposes a set of class attributes that you can override in order to alter how serializer fields are automatically determined when first instantiating the serializer. +The ModelSerializer class also exposes a set of class attributes that you can override in order to alter how serializer fields are automatically determined when generating the fields. Normally if a `ModelSerializer` does not generate the fields you need by default then you should either add them to the class explicitly, or simply use a regular `Serializer` class instead. However in some cases you may want to create a new base class that defines how the serializer fields are created for any given model. diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index c1b4f0290..727fd37fd 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -851,7 +851,97 @@ def raise_errors_on_nested_writes(method_name, serializer, validated_data): ) -class ModelSerializer(Serializer): +class ModelSerializerMetaclass(SerializerMetaclass): + """ + This metaclass sets a dictionary named `base_fields` on the class. + + `base_fields` include the declared fields (see SerializerMetaclass), + and fields generated for the model. A deepcopy of `base_fields` is + used for each instance of the ModelSerializer. + """ + + @classmethod + def _get_fields(cls, new_class): + # XXX + if not hasattr(new_class, 'Meta'): + return OrderedDict() + + assert hasattr(new_class, 'Meta'), ( + 'Class {serializer_class} missing "Meta" attribute'.format( + serializer_class=new_class.__name__ + ) + ) + assert hasattr(new_class.Meta, 'model'), ( + 'Class {serializer_class} missing "Meta.model" attribute'.format( + serializer_class=new_class.__name__ + ) + ) + if model_meta.is_abstract_model(new_class.Meta.model): + raise ValueError( + 'Cannot use ModelSerializer with Abstract Models.' + ) + + if new_class.url_field_name is None: + new_class.url_field_name = api_settings.URL_FIELD_NAME + + declared_fields = new_class._declared_fields + model = getattr(new_class.Meta, 'model') + depth = getattr(new_class.Meta, 'depth', 0) + + if depth is not None: + assert depth >= 0, "'depth' may not be negative." + assert depth <= 10, "'depth' may not be greater than 10." + + # Retrieve metadata about fields & relationships on the model class. + info = model_meta.get_field_info(model) + field_names = new_class.get_field_names(declared_fields, info) + + # Determine any extra field arguments and hidden fields that + # should be included + extra_kwargs = new_class.get_extra_kwargs() + extra_kwargs, hidden_fields = new_class.get_uniqueness_extra_kwargs( + field_names, declared_fields, extra_kwargs + ) + + # Determine the fields that should be included on the serializer. + fields = OrderedDict() + + for field_name in field_names: + # If the field is explicitly declared on the class then use that. + if field_name in declared_fields: + fields[field_name] = declared_fields[field_name] + continue + + extra_field_kwargs = extra_kwargs.get(field_name, {}) + source = extra_field_kwargs.get('source', '*') + if source == '*': + source = field_name + + # Determine the serializer field class and keyword arguments. + field_class, field_kwargs = new_class.build_field( + source, info, model, depth + ) + + # Include any kwargs defined in `Meta.extra_kwargs` + field_kwargs = new_class.include_extra_kwargs( + field_kwargs, extra_field_kwargs + ) + + # Create the serializer field. + fields[field_name] = field_class(**field_kwargs) + + # Add in any hidden fields. + fields.update(hidden_fields) + + return fields + + def __new__(cls, name, bases, attrs): + new_class = super().__new__(cls, name, bases, attrs) + new_class.base_fields = cls._get_fields(new_class) + return new_class + + +class ModelSerializer(Serializer, metaclass=ModelSerializerMetaclass): """ A `ModelSerializer` is just a regular `Serializer`, except that: @@ -1002,88 +1092,12 @@ class ModelSerializer(Serializer): # Determine the fields to apply... - @classmethod - def _get_fields(cls): - if cls.url_field_name is None: - cls.url_field_name = api_settings.URL_FIELD_NAME - - assert hasattr(cls, 'Meta'), ( - 'Class {serializer_class} missing "Meta" attribute'.format( - serializer_class=cls.__name__ - ) - ) - assert hasattr(cls.Meta, 'model'), ( - 'Class {serializer_class} missing "Meta.model" attribute'.format( - serializer_class=cls.__name__ - ) - ) - if model_meta.is_abstract_model(cls.Meta.model): - raise ValueError( - 'Cannot use ModelSerializer with Abstract Models.' - ) - - declared_fields = cls._declared_fields - model = getattr(cls.Meta, 'model') - depth = getattr(cls.Meta, 'depth', 0) - - if depth is not None: - assert depth >= 0, "'depth' may not be negative." - assert depth <= 10, "'depth' may not be greater than 10." - - # Retrieve metadata about fields & relationships on the model class. - info = model_meta.get_field_info(model) - field_names = cls.get_field_names(declared_fields, info) - - # Determine any extra field arguments and hidden fields that - # should be included - extra_kwargs = cls.get_extra_kwargs() - extra_kwargs, hidden_fields = cls.get_uniqueness_extra_kwargs( - field_names, declared_fields, extra_kwargs - ) - - # Determine the fields that should be included on the serializer. - fields = OrderedDict() - - for field_name in field_names: - # If the field is explicitly declared on the class then use that. - if field_name in declared_fields: - fields[field_name] = declared_fields[field_name] - continue - - extra_field_kwargs = extra_kwargs.get(field_name, {}) - source = extra_field_kwargs.get('source', '*') - if source == '*': - source = field_name - - # Determine the serializer field class and keyword arguments. - field_class, field_kwargs = cls.build_field( - source, info, model, depth - ) - - # Include any kwargs defined in `Meta.extra_kwargs` - field_kwargs = cls.include_extra_kwargs( - field_kwargs, extra_field_kwargs - ) - - # Create the serializer field. - fields[field_name] = field_class(**field_kwargs) - - # Add in any hidden fields. - fields.update(hidden_fields) - - return fields - def get_fields(self): """ Return the dict of field names -> field instances that should be used for `self.fields` when instantiating the serializer. """ - cls = self.__class__ - # We don't want the cache to traverse the MRO, since each subclass - # has its own fields; hence the cls comparison. - if not hasattr(cls, "_fields_cache") or cls._fields_cache[0] != cls: - cls._fields_cache = cls, cls._get_fields() - return copy.deepcopy(cls._fields_cache[1]) + return copy.deepcopy(self.base_fields) # Methods for determining the set of field names to include... diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py index fbb562792..852b5a180 100644 --- a/tests/test_model_serializer.py +++ b/tests/test_model_serializer.py @@ -144,18 +144,12 @@ class TestModelSerializer(TestCase): class Meta: abstract = True - class TestSerializer(serializers.ModelSerializer): - class Meta: - model = AbstractModel - fields = ('afield',) - - serializer = TestSerializer(data={ - 'afield': 'foo', - }) - msginitial = 'Cannot use ModelSerializer with Abstract Models.' with self.assertRaisesMessage(ValueError, msginitial): - serializer.is_valid() + class TestSerializer(serializers.ModelSerializer): + class Meta: + model = AbstractModel + fields = ('afield',) class TestRegularFieldMappings(TestCase): @@ -307,33 +301,29 @@ class TestRegularFieldMappings(TestCase): Field names that do not map to a model field or relationship should raise a configuration error. """ - class TestSerializer(serializers.ModelSerializer): - class Meta: - model = RegularFieldsModel - fields = ('auto_field', 'invalid') - expected = 'Field name `invalid` is not valid for model `RegularFieldsModel`.' with self.assertRaisesMessage(ImproperlyConfigured, expected): - TestSerializer().fields + class TestSerializer(serializers.ModelSerializer): + class Meta: + model = RegularFieldsModel + fields = ('auto_field', 'invalid') def test_missing_field(self): """ Fields that have been declared on the serializer class must be included in the `Meta.fields` if it exists. """ - class TestSerializer(serializers.ModelSerializer): - missing = serializers.ReadOnlyField() - - class Meta: - model = RegularFieldsModel - fields = ('auto_field',) - expected = ( "The field 'missing' was declared on serializer TestSerializer, " "but has not been included in the 'fields' option." ) with self.assertRaisesMessage(AssertionError, expected): - TestSerializer().fields + class TestSerializer(serializers.ModelSerializer): + missing = serializers.ReadOnlyField() + + class Meta: + model = RegularFieldsModel + fields = ('auto_field',) def test_missing_superclass_field(self): """ @@ -921,51 +911,43 @@ class MetaClassTestModel(models.Model): class TestSerializerMetaClass(TestCase): def test_meta_class_fields_option(self): - class ExampleSerializer(serializers.ModelSerializer): - class Meta: - model = MetaClassTestModel - fields = 'text' - msginitial = "The `fields` option must be a list or tuple" with self.assertRaisesMessage(TypeError, msginitial): - ExampleSerializer().fields + class ExampleSerializer(serializers.ModelSerializer): + class Meta: + model = MetaClassTestModel + fields = 'text' def test_meta_class_exclude_option(self): - class ExampleSerializer(serializers.ModelSerializer): - class Meta: - model = MetaClassTestModel - exclude = 'text' - msginitial = "The `exclude` option must be a list or tuple" with self.assertRaisesMessage(TypeError, msginitial): - ExampleSerializer().fields + class ExampleSerializer(serializers.ModelSerializer): + class Meta: + model = MetaClassTestModel + exclude = 'text' def test_meta_class_fields_and_exclude_options(self): - class ExampleSerializer(serializers.ModelSerializer): - class Meta: - model = MetaClassTestModel - fields = ('text',) - exclude = ('text',) - msginitial = "Cannot set both 'fields' and 'exclude' options on serializer ExampleSerializer." with self.assertRaisesMessage(AssertionError, msginitial): - ExampleSerializer().fields + class ExampleSerializer(serializers.ModelSerializer): + class Meta: + model = MetaClassTestModel + fields = ('text',) + exclude = ('text',) def test_declared_fields_with_exclude_option(self): - class ExampleSerializer(serializers.ModelSerializer): - text = serializers.CharField() - - class Meta: - model = MetaClassTestModel - exclude = ('text',) - expected = ( "Cannot both declare the field 'text' and include it in the " "ExampleSerializer 'exclude' option. Remove the field or, if " "inherited from a parent serializer, disable with `text = None`." ) with self.assertRaisesMessage(AssertionError, expected): - ExampleSerializer().fields + class ExampleSerializer(serializers.ModelSerializer): + text = serializers.CharField() + + class Meta: + model = MetaClassTestModel + exclude = ('text',) class Issue2704TestCase(TestCase): @@ -1177,16 +1159,12 @@ class Issue3674Test(TestCase): class Issue4897TestCase(TestCase): def test_should_assert_if_writing_readonly_fields(self): - class TestSerializer(serializers.ModelSerializer): - class Meta: - model = OneFieldModel - fields = ('char_field',) - readonly_fields = fields - - obj = OneFieldModel.objects.create(char_field='abc') - with pytest.raises(AssertionError) as cm: - TestSerializer(obj).fields + class TestSerializer(serializers.ModelSerializer): + class Meta: + model = OneFieldModel + fields = ('char_field',) + readonly_fields = fields cm.match(r'readonly_fields')