From 0ddd14044a1d47b2b8c190e311968cbd10010e7e Mon Sep 17 00:00:00 2001 From: Jonathan Senecal Date: Tue, 15 Sep 2015 00:15:12 -0400 Subject: [PATCH 1/5] Removed extract_id as we now have access to the resource instance --- rest_framework_json_api/utils.py | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index 4729fb83..fa3fda17 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -131,7 +131,7 @@ def format_value(value, format_type=None): def build_json_resource_obj(fields, resource, resource_instance, resource_name): resource_data = [ ('type', resource_name), - ('id', extract_id(fields, resource)), + ('id', resource_instance.pk), ('attributes', extract_attributes(fields, resource)), ] relationships = extract_relationships(fields, resource, resource_instance) @@ -179,18 +179,10 @@ def extract_id_from_url(url): return encoding.force_text(match.kwargs['pk']) -def extract_id(fields, resource): - for field_name, field in six.iteritems(fields): - if field_name == 'id': - return encoding.force_text(resource.get(field_name)) - if field_name == api_settings.URL_FIELD_NAME: - return extract_id_from_url(resource.get(field_name)) - - def extract_attributes(fields, resource): data = OrderedDict() for field_name, field in six.iteritems(fields): - # ID is always provided in the root of JSON API so remove it from attrs + # ID is always provided in the root of JSON API so remove it from attributes if field_name == 'id': continue # Skip fields with relations @@ -282,14 +274,14 @@ def extract_relationships(fields, resource, resource_instance): relation_model = serializer.Meta.model relation_type = inflection.pluralize(relation_model.__name__).lower() - # Get the serializer fields - serializer_fields = get_serializer_fields(serializer) serializer_data = resource.get(field_name) if isinstance(serializer_data, list): - for serializer_resource in serializer_data: + for position in range(len(serializer_data)): + resource_instance_manager = getattr(resource_instance, field_name).all() + nested_resource_instance = resource_instance_manager[position] relation_data.append( OrderedDict([ - ('type', relation_type), ('id', extract_id(serializer_fields, serializer_resource)) + ('type', relation_type), ('id', nested_resource_instance.pk) ])) data.update({field_name: {'data': relation_data}}) @@ -299,15 +291,12 @@ def extract_relationships(fields, resource, resource_instance): relation_model = field.Meta.model relation_type = inflection.pluralize(relation_model.__name__).lower() - # Get the serializer fields - serializer_fields = get_serializer_fields(field) - serializer_data = resource.get(field_name) data.update({ field_name: { 'data': ( OrderedDict([ ('type', relation_type), - ('id', extract_id(serializer_fields, serializer_data)) + ('id', getattr(resource_instance, field_name).pk) ]) if resource.get(field_name) else None) } }) From 969543f82112be8c4dfb96233ef952b69fb52abc Mon Sep 17 00:00:00 2001 From: Jonathan Senecal Date: Tue, 15 Sep 2015 00:22:39 -0400 Subject: [PATCH 2/5] The values of the id and type members MUST be strings --- rest_framework_json_api/utils.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index fa3fda17..0f53cfd7 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -131,7 +131,7 @@ def format_value(value, format_type=None): def build_json_resource_obj(fields, resource, resource_instance, resource_name): resource_data = [ ('type', resource_name), - ('id', resource_instance.pk), + ('id', encoding.force_text(resource_instance.pk)), ('attributes', extract_attributes(fields, resource)), ] relationships = extract_relationships(fields, resource, resource_instance) @@ -212,7 +212,7 @@ def extract_relationships(fields, resource, resource_instance): relation_type = get_related_resource_type(field) related = getattr(resource_instance, field_name).all() for relation in related: - relation_data.append(OrderedDict([('type', relation_type), ('id', relation.pk)])) + relation_data.append(OrderedDict([('type', relation_type), ('id', encoding.force_text(relation.pk))])) data.update({field_name: { 'links': { @@ -239,7 +239,7 @@ def extract_relationships(fields, resource, resource_instance): { field_name: { 'data': (OrderedDict([ - ('type', relation_type), ('id', relation_id) + ('type', relation_type), ('id', encoding.force_text(relation_id)) ]) if relation_id is not None else None) } } @@ -281,7 +281,7 @@ def extract_relationships(fields, resource, resource_instance): nested_resource_instance = resource_instance_manager[position] relation_data.append( OrderedDict([ - ('type', relation_type), ('id', nested_resource_instance.pk) + ('type', relation_type), ('id', encoding.force_text(nested_resource_instance.pk)) ])) data.update({field_name: {'data': relation_data}}) @@ -296,7 +296,7 @@ def extract_relationships(fields, resource, resource_instance): 'data': ( OrderedDict([ ('type', relation_type), - ('id', getattr(resource_instance, field_name).pk) + ('id', encoding.force_text(getattr(resource_instance, field_name).pk)) ]) if resource.get(field_name) else None) } }) From d122fa39f0bd532ff55ad1cd60978d2091453d26 Mon Sep 17 00:00:00 2001 From: Jonathan Senecal Date: Tue, 15 Sep 2015 00:46:23 -0400 Subject: [PATCH 3/5] Removed extract_id_from_url as we now have access to the resource instance --- rest_framework_json_api/utils.py | 41 ++++++++------------------------ 1 file changed, 10 insertions(+), 31 deletions(-) diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index 0f53cfd7..4a89b409 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -166,19 +166,6 @@ def get_related_resource_type(relation): return inflection.pluralize(relation_model.__name__).lower() -def extract_id_from_url(url): - http_prefix = url.startswith(('http:', 'https:')) - if http_prefix: - # If needed convert absolute URLs to relative path - data = urlparse(url).path - prefix = urlresolvers.get_script_prefix() - if data.startswith(prefix): - url = '/' + data[len(prefix):] - - match = urlresolvers.resolve(url) - return encoding.force_text(match.kwargs['pk']) - - def extract_attributes(fields, resource): data = OrderedDict() for field_name, field in six.iteritems(fields): @@ -228,10 +215,7 @@ def extract_relationships(fields, resource, resource_instance): relation_type = get_related_resource_type(field) if resource.get(field_name) is not None: - if isinstance(field, PrimaryKeyRelatedField): - relation_id = encoding.force_text(resource.get(field_name)) - elif isinstance(field, HyperlinkedRelatedField): - relation_id = extract_id_from_url(resource.get(field_name)) + relation_id = getattr(resource_instance, field_name).id else: relation_id = None @@ -250,19 +234,14 @@ def extract_relationships(fields, resource, resource_instance): relation_data = list() relation = field.child_relation - relation_type = get_related_resource_type(relation) - - if isinstance(relation, HyperlinkedRelatedField): - for link in resource.get(field_name, list()): - relation_data.append(OrderedDict([('type', relation_type), ('id', extract_id_from_url(link))])) - - data.update({field_name: {'data': relation_data}}) - continue - - if isinstance(relation, PrimaryKeyRelatedField): - for pk in resource.get(field_name, list()): - relation_data.append(OrderedDict([('type', relation_type), ('id', encoding.force_text(pk))])) + nested_resource_queryset = getattr(resource_instance, field_name).all() + if isinstance(relation, (HyperlinkedRelatedField, PrimaryKeyRelatedField)): + for position in range(len(nested_resource_queryset)): + nested_resource_instance = nested_resource_queryset[position] + relation_data.append( + OrderedDict([('type', relation_type), ('id', encoding.force_text(nested_resource_instance.pk))]) + ) data.update({field_name: {'data': relation_data}}) continue @@ -275,10 +254,10 @@ def extract_relationships(fields, resource, resource_instance): relation_type = inflection.pluralize(relation_model.__name__).lower() serializer_data = resource.get(field_name) + resource_instance_queryset = getattr(resource_instance, field_name).all() if isinstance(serializer_data, list): for position in range(len(serializer_data)): - resource_instance_manager = getattr(resource_instance, field_name).all() - nested_resource_instance = resource_instance_manager[position] + nested_resource_instance = resource_instance_queryset[position] relation_data.append( OrderedDict([ ('type', relation_type), ('id', encoding.force_text(nested_resource_instance.pk)) From fb6196ae513e674d0bfcf94a38faa4ba63cccd8d Mon Sep 17 00:00:00 2001 From: Jonathan Senecal Date: Tue, 15 Sep 2015 09:08:36 -0400 Subject: [PATCH 4/5] DRY Fix for relation_type --- rest_framework_json_api/utils.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index b79f8b18..c7486382 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -198,10 +198,11 @@ def extract_relationships(fields, resource, resource_instance): if not isinstance(field, (RelatedField, ManyRelatedField, BaseSerializer)): continue + relation_type = get_related_resource_type(field) + if isinstance(field, HyperlinkedIdentityField): # special case for HyperlinkedIdentityField relation_data = list() - relation_type = get_related_resource_type(field) relation_manager = getattr(resource_instance, field_name) # Don't try to query an empty relation related = relation_manager.all() if relation_manager is not None else list() @@ -219,7 +220,6 @@ def extract_relationships(fields, resource, resource_instance): continue if isinstance(field, (PrimaryKeyRelatedField, HyperlinkedRelatedField)): - relation_type = get_related_resource_type(field) relation_id = getattr(resource_instance, field_name).pk if resource.get(field_name) else None relation_data = { @@ -256,7 +256,6 @@ def extract_relationships(fields, resource, resource_instance): if isinstance(field, ListSerializer): relation_data = list() - serializer = field.child relation_model = serializer.Meta.model relation_type = inflection.pluralize(relation_model.__name__).lower() From 3fa4c9e28f109e012dfe9eb333c3fdedf4d98ecd Mon Sep 17 00:00:00 2001 From: Jonathan Senecal Date: Tue, 15 Sep 2015 09:56:44 -0400 Subject: [PATCH 5/5] Readability, DRY and refactorisation --- rest_framework_json_api/utils.py | 56 +++++++++++++++++--------------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index c7486382..1d9ef502 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -199,15 +199,20 @@ def extract_relationships(fields, resource, resource_instance): continue relation_type = get_related_resource_type(field) + relation_instance_or_manager = getattr(resource_instance, field_name) if isinstance(field, HyperlinkedIdentityField): # special case for HyperlinkedIdentityField relation_data = list() - relation_manager = getattr(resource_instance, field_name) + # Don't try to query an empty relation - related = relation_manager.all() if relation_manager is not None else list() - for relation in related: - relation_data.append(OrderedDict([('type', relation_type), ('id', encoding.force_text(relation.pk))])) + relation_queryset = relation_instance_or_manager.all() \ + if relation_instance_or_manager is not None else list() + + for related_object in relation_queryset: + relation_data.append( + OrderedDict([('type', relation_type), ('id', encoding.force_text(related_object.pk))]) + ) data.update({field_name: { 'links': { @@ -220,26 +225,26 @@ def extract_relationships(fields, resource, resource_instance): continue if isinstance(field, (PrimaryKeyRelatedField, HyperlinkedRelatedField)): - relation_id = getattr(resource_instance, field_name).pk if resource.get(field_name) else None + relation_id = relation_instance_or_manager.pk if resource.get(field_name) else None relation_data = { - 'data': (OrderedDict([ - ('type', relation_type), ('id', encoding.force_text(relation_id)) - ]) if relation_id is not None else None) + 'data': ( + OrderedDict([('type', relation_type), ('id', encoding.force_text(relation_id))]) + if relation_id is not None else None) } relation_data.update( {'links': {'related': resource.get(field_name)}} - if isinstance(field, HyperlinkedRelatedField) and resource.get(field_name) else {} + if isinstance(field, HyperlinkedRelatedField) and resource.get(field_name) else dict() ) data.update({field_name: relation_data}) continue if isinstance(field, ManyRelatedField): relation_data = list() - relation = field.child_relation - relation_type = get_related_resource_type(relation) - for related_object in getattr(resource_instance, field_name).all(): + related_object = field.child_relation + relation_type = get_related_resource_type(related_object) + for related_object in relation_instance_or_manager.all(): relation_data.append(OrderedDict([ ('type', relation_type), ('id', encoding.force_text(related_object.pk)) @@ -261,14 +266,15 @@ def extract_relationships(fields, resource, resource_instance): relation_type = inflection.pluralize(relation_model.__name__).lower() serializer_data = resource.get(field_name) - resource_instance_queryset = getattr(resource_instance, field_name).all() + resource_instance_queryset = relation_instance_or_manager.all() if isinstance(serializer_data, list): for position in range(len(serializer_data)): nested_resource_instance = resource_instance_queryset[position] relation_data.append( - OrderedDict([ - ('type', relation_type), ('id', encoding.force_text(nested_resource_instance.pk)) - ])) + OrderedDict( + [('type', relation_type), ('id', encoding.force_text(nested_resource_instance.pk))] + ) + ) data.update({field_name: {'data': relation_data}}) continue @@ -282,7 +288,7 @@ def extract_relationships(fields, resource, resource_instance): 'data': ( OrderedDict([ ('type', relation_type), - ('id', encoding.force_text(getattr(resource_instance, field_name).pk)) + ('id', encoding.force_text(relation_instance_or_manager.pk)) ]) if resource.get(field_name) else None) } }) @@ -302,20 +308,21 @@ def extract_included(fields, resource, resource_instance): if not isinstance(field, BaseSerializer): continue - if isinstance(field, ListSerializer): + relation_instance_or_manager = getattr(resource_instance, field_name) + relation_queryset = relation_instance_or_manager.all() + serializer_data = resource.get(field_name) + if isinstance(field, ListSerializer): serializer = field.child model = serializer.Meta.model relation_type = inflection.pluralize(model.__name__).lower() # Get the serializer fields serializer_fields = get_serializer_fields(serializer) - serializer_data = resource.get(field_name) - if isinstance(serializer_data, list): + if serializer_data: for position in range(len(serializer_data)): serializer_resource = serializer_data[position] - resource_instance_manager = getattr(resource_instance, field_name).all() - nested_resource_instance = resource_instance_manager[position] + nested_resource_instance = relation_queryset[position] included_data.append( build_json_resource_obj( serializer_fields, serializer_resource, nested_resource_instance, relation_type @@ -323,17 +330,14 @@ def extract_included(fields, resource, resource_instance): ) if isinstance(field, ModelSerializer): - model = field.Meta.model relation_type = inflection.pluralize(model.__name__).lower() # Get the serializer fields serializer_fields = get_serializer_fields(field) - serializer_data = resource.get(field_name) - nested_resource_instance = getattr(resource_instance, field_name).all() if serializer_data: included_data.append( - build_json_resource_obj(serializer_fields, serializer_data, nested_resource_instance, relation_type) + build_json_resource_obj(serializer_fields, serializer_data, relation_queryset, relation_type) ) return format_keys(included_data)