From f64599c36155be139c4896bb3db672bb559147e5 Mon Sep 17 00:00:00 2001 From: Ian Foote Date: Thu, 27 Mar 2014 15:20:13 +0000 Subject: [PATCH 1/2] Reduce indentation in BaseSerializer.errors This makes the method more linear and easier to read. --- rest_framework/serializers.py | 108 ++++++++++++++++++---------------- 1 file changed, 58 insertions(+), 50 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 88972e257..1c2d1233c 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -493,58 +493,66 @@ class BaseSerializer(WritableField): Run deserialization and return error data, setting self.object if no errors occurred. """ - if self._errors is None: - data, files = self.init_data, self.init_files + if self._errors is not None: + return self._errors - if self.many is not None: - many = self.many - else: - many = hasattr(data, '__iter__') and not isinstance(data, (Page, dict, six.text_type)) - if many: - warnings.warn('Implicit list/queryset serialization is deprecated. ' - 'Use the `many=True` flag when instantiating the serializer.', - DeprecationWarning, stacklevel=3) + data, files = self.init_data, self.init_files + if self.many is not None: + many = self.many + else: + many = hasattr(data, '__iter__') and not isinstance(data, (Page, dict, six.text_type)) if many: - ret = RelationsList() - errors = [] - update = self.object is not None + warnings.warn('Implicit list/queryset serialization is deprecated. ' + 'Use the `many=True` flag when instantiating the serializer.', + DeprecationWarning, stacklevel=3) - if update: - # If this is a bulk update we need to map all the objects - # to a canonical identity so we can determine which - # individual object is being updated for each item in the - # incoming data - objects = self.object - identities = [self.get_identity(self.to_native(obj)) for obj in objects] - identity_to_objects = dict(zip(identities, objects)) - - if hasattr(data, '__iter__') and not isinstance(data, (dict, six.text_type)): - for item in data: - if update: - # Determine which object we're updating - identity = self.get_identity(item) - self.object = identity_to_objects.pop(identity, None) - if self.object is None and not self.allow_add_remove: - ret.append(None) - errors.append({'non_field_errors': ['Cannot create a new item, only existing items may be updated.']}) - continue - - ret.append(self.from_native(item, None)) - errors.append(self._errors) - - if update and self.allow_add_remove: - ret._deleted = identity_to_objects.values() - - self._errors = any(errors) and errors or [] - else: - self._errors = {'non_field_errors': ['Expected a list of items.']} - else: - ret = self.from_native(data, files) + if not many: + ret = self.from_native(data, files) if not self._errors: self.object = ret + return self._errors + + ret = RelationsList() + errors = [] + update = self.object is not None + + if update: + # If this is a bulk update we need to map all the objects + # to a canonical identity so we can determine which + # individual object is being updated for each item in the + # incoming data + objects = self.object + identities = [self.get_identity(self.to_native(obj)) for obj in objects] + identity_to_objects = dict(zip(identities, objects)) + + if hasattr(data, '__iter__') and not isinstance(data, (dict, six.text_type)): + for item in data: + if update: + # Determine which object we're updating + identity = self.get_identity(item) + self.object = identity_to_objects.pop(identity, None) + if self.object is None and not self.allow_add_remove: + ret.append(None) + errors.append({'non_field_errors': [ + 'Cannot create a new item, only existing items may be updated.']}) + continue + + ret.append(self.from_native(item, None)) + errors.append(self._errors) + + if update and self.allow_add_remove: + ret._deleted = identity_to_objects.values() + + self._errors = any(errors) and errors or [] + else: + self._errors = {'non_field_errors': ['Expected a list of items.']} + + if not self._errors: + self.object = ret + return self._errors def is_valid(self): @@ -757,9 +765,9 @@ class ModelSerializer(Serializer): field.read_only = True ret[accessor_name] = field - + # Ensure that 'read_only_fields' is an iterable - assert isinstance(self.opts.read_only_fields, (list, tuple)), '`read_only_fields` must be a list or tuple' + assert isinstance(self.opts.read_only_fields, (list, tuple)), '`read_only_fields` must be a list or tuple' # Add the `read_only` flag to any fields that have been specified # in the `read_only_fields` option @@ -774,10 +782,10 @@ class ModelSerializer(Serializer): "on serializer '%s'." % (field_name, self.__class__.__name__)) ret[field_name].read_only = True - + # Ensure that 'write_only_fields' is an iterable - assert isinstance(self.opts.write_only_fields, (list, tuple)), '`write_only_fields` must be a list or tuple' - + assert isinstance(self.opts.write_only_fields, (list, tuple)), '`write_only_fields` must be a list or tuple' + for field_name in self.opts.write_only_fields: assert field_name not in self.base_fields.keys(), ( "field '%s' on serializer '%s' specified in " @@ -788,7 +796,7 @@ class ModelSerializer(Serializer): "Non-existant field '%s' specified in `write_only_fields` " "on serializer '%s'." % (field_name, self.__class__.__name__)) - ret[field_name].write_only = True + ret[field_name].write_only = True return ret From c0c8833185cee1b1cdcac3939ddca5d4943999f5 Mon Sep 17 00:00:00 2001 From: Ian Foote Date: Thu, 27 Mar 2014 15:38:21 +0000 Subject: [PATCH 2/2] Split errors into errors and _errors_many Reduce indentation and linearize code to improve readability. --- rest_framework/serializers.py | 50 ++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 1c2d1233c..d3087484f 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -507,12 +507,21 @@ class BaseSerializer(WritableField): 'Use the `many=True` flag when instantiating the serializer.', DeprecationWarning, stacklevel=3) - if not many: - ret = self.from_native(data, files) + if many: + return self._errors_many(data) - if not self._errors: - self.object = ret + ret = self.from_native(data, files) + if not self._errors: + self.object = ret + + return self._errors + + def _errors_many(self, data): + """Run deserialization of bulk data.""" + + if not hasattr(data, '__iter__') or isinstance(data, (dict, six.text_type)): + self._errors = {'non_field_errors': ['Expected a list of items.']} return self._errors ret = RelationsList() @@ -528,27 +537,24 @@ class BaseSerializer(WritableField): identities = [self.get_identity(self.to_native(obj)) for obj in objects] identity_to_objects = dict(zip(identities, objects)) - if hasattr(data, '__iter__') and not isinstance(data, (dict, six.text_type)): - for item in data: - if update: - # Determine which object we're updating - identity = self.get_identity(item) - self.object = identity_to_objects.pop(identity, None) - if self.object is None and not self.allow_add_remove: - ret.append(None) - errors.append({'non_field_errors': [ - 'Cannot create a new item, only existing items may be updated.']}) - continue + for item in data: + if update: + # Determine which object we're updating + identity = self.get_identity(item) + self.object = identity_to_objects.pop(identity, None) + if self.object is None and not self.allow_add_remove: + ret.append(None) + errors.append({'non_field_errors': [ + 'Cannot create a new item, only existing items may be updated.']}) + continue - ret.append(self.from_native(item, None)) - errors.append(self._errors) + ret.append(self.from_native(item, None)) + errors.append(self._errors) - if update and self.allow_add_remove: - ret._deleted = identity_to_objects.values() + if update and self.allow_add_remove: + ret._deleted = identity_to_objects.values() - self._errors = any(errors) and errors or [] - else: - self._errors = {'non_field_errors': ['Expected a list of items.']} + self._errors = any(errors) and errors or [] if not self._errors: self.object = ret