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

retrieve_related fails for toOne relationship #489

Closed
n2ygk opened this issue Oct 8, 2018 · 7 comments
Closed

retrieve_related fails for toOne relationship #489

n2ygk opened this issue Oct 8, 2018 · 7 comments
Labels

Comments

@n2ygk
Copy link
Contributor

n2ygk commented Oct 8, 2018

@Anton-Shutik Using your new #451 support for related links, using{'get': 'retrieve_related'} works fine for a serializer ResourceRelatedField where many=True but fails for a toOne relationship where many=False.

Using the old style, I had to override get_queryset() for many=True views and get_object() for many=False views so I expect there is a similar need for another "flavor" of retrieve_related for toOne relationships. Does this make sense?

Here's my old view code for reference:

class CourseTermViewSet(CourseBaseViewSet):
    queryset = CourseTerm.objects.all()
    serializer_class = CourseTermSerializer

    def get_queryset(self, *args, **kwargs):
        """
        Implement `related` view:
        Override `.list` if course_pk kwarg is present since course is toMany to course_term.
        """
        course_pk = self.kwargs.get('course_pk', None)
        if course_pk is not None:
            return self.queryset.filter(course_id=course_pk)
        return super(CourseTermViewSet, self).get_queryset()
class CourseViewSet(CourseBaseViewSet):
    queryset = Course.objects.all()
    serializer_class = CourseSerializer

    def get_object(self):
        """
        Implement `related` view:
        Override `.retrieve` if course_term_pk kwarg is present since course_term is toOne to course.
        """
        course_term_pk = self.kwargs.get('course_term_pk', None)
        if course_term_pk is not None:
            return CourseTerm.objects.get(id=course_term_pk).course
        return super(CourseViewSet, self).get_object()

The new code (eliminates those overrides, sets the kwarg to pk and updates the related_link_view_names to course-related and course_term-related, respectively and throws this error when retrieved:

AttributeError at /v1/course_terms/a1d34785-cc25-4c1c-9806-9d05a98068c7/course/
Got AttributeError when attempting to get a value for field `school_bulletin_prefix_code` on serializer `CourseSerializer`.
The serializer field might be named incorrectly and not match any attribute or key on the `PKOnlyObject` instance.
Original exception text was: 'PKOnlyObject' object has no attribute 'school_bulletin_prefix_code'.

...
Traceback:

File "/Users/alan/src/django-training/env/lib/python3.6/site-packages/rest_framework/fields.py" in get_attribute
  441.             return get_attribute(instance, self.source_attrs)

File "/Users/alan/src/django-training/env/lib/python3.6/site-packages/rest_framework/fields.py" in get_attribute
  100.                 instance = getattr(instance, attr)

During handling of the above exception ('PKOnlyObject' object has no attribute 'school_bulletin_prefix_code'), another exception occurred:

File "/Users/alan/src/django-training/env/lib/python3.6/site-packages/django/core/handlers/exception.py" in inner
  34.             response = get_response(request)

File "/Users/alan/src/django-training/env/lib/python3.6/site-packages/django/core/handlers/base.py" in _get_response
  126.                 response = self.process_exception_by_middleware(e, request)

File "/Users/alan/src/django-training/env/lib/python3.6/site-packages/django/core/handlers/base.py" in _get_response
  124.                 response = wrapped_callback(request, *callback_args, **callback_kwargs)

File "/Users/alan/src/django-training/env/lib/python3.6/site-packages/django/views/decorators/csrf.py" in wrapped_view
  54.         return view_func(*args, **kwargs)

File "/Users/alan/src/django-training/env/lib/python3.6/site-packages/rest_framework/viewsets.py" in view
  103.             return self.dispatch(request, *args, **kwargs)

File "/Users/alan/src/django-training/env/lib/python3.6/site-packages/rest_framework/views.py" in dispatch
  483.             response = self.handle_exception(exc)

File "/Users/alan/src/django-training/env/lib/python3.6/site-packages/rest_framework/views.py" in handle_exception
  443.             self.raise_uncaught_exception(exc)

File "/Users/alan/src/django-training/env/lib/python3.6/site-packages/rest_framework/views.py" in dispatch
  480.             response = handler(request, *args, **kwargs)

File "/Users/alan/src/django-training/env/lib/python3.6/site-packages/rest_framework_json_api/views.py" in retrieve_related
  128.         return Response(serializer.data)

File "/Users/alan/src/django-training/env/lib/python3.6/site-packages/rest_framework/serializers.py" in data
  560.         ret = super(Serializer, self).data

File "/Users/alan/src/django-training/env/lib/python3.6/site-packages/rest_framework/serializers.py" in data
  262.                 self._data = self.to_representation(self.instance)

File "/Users/alan/src/django-training/env/lib/python3.6/site-packages/rest_framework/serializers.py" in to_representation
  514.                 attribute = field.get_attribute(instance)

File "/Users/alan/src/django-training/env/lib/python3.6/site-packages/rest_framework/fields.py" in get_attribute
  462.             raise type(exc)(msg)

@n2ygk n2ygk added the bug label Oct 8, 2018
@n2ygk
Copy link
Contributor Author

n2ygk commented Oct 8, 2018

my old urls looked like this:

    url(r'^v1/courses/(?P<course_pk>[^/.]+)/course_terms/',
        views.CourseTermViewSet.as_view({'get': 'list'}),
        name='course-course_terms'),
    url(r'^v1/course_terms/(?P<course_term_pk>[^/.]+)/course/',
        views.CourseViewSet.as_view({'get': 'retrieve'}), # a toOne relationship
        name='course_terms-course'),

and here are the models:

class Course(CommonModel):
    school_bulletin_prefix_code = models.CharField(max_length=10)
    suffix_two = models.CharField(max_length=2)
    subject_area_code = models.CharField(max_length=10)
    course_number = models.CharField(max_length=10)
    course_identifier = models.CharField(max_length=10, unique=True)
    course_name = models.CharField(max_length=80)
    course_description = models.TextField()

    class Meta:
        ordering = ["course_number"]

    def __str__(self):
        return '%s,%s,%s,%s' % (
            self.id,
            self.course_number,
            self.course_identifier,
            self.course_name
        )


class CourseTerm(CommonModel):
    term_identifier = models.TextField(max_length=10)
    audit_permitted_code = models.PositiveIntegerField(blank=True, default=0)
    exam_credit_flag = models.BooleanField(default=True)
    course = models.ForeignKey('myapp.Course', related_name='course_terms', on_delete=models.CASCADE, null=True,
                               default=None)

    class Meta:
        ordering = ["term_identifier"]

    def __str__(self):
        return '%s,%s,%s' % (self.id, self.term_identifier, self.course.course_identifier)

@n2ygk
Copy link
Contributor Author

n2ygk commented Oct 8, 2018

Here's where it happens but not sure what the fix is:

class RelatedMixin(object):
    """
    This mixin handles all related entities, whose Serializers are declared in "related_serializers"
    """

    def retrieve_related(self, request, *args, **kwargs):
        serializer_kwargs = {}
        instance = self.get_related_instance()

        returns PKOnlyObject instead of the full object.
--------
This is because ResourceRelatedField use_pk_only_optimization returns True.
---------

in rest_framework_json_api/views.py L160:

    def get_related_instance(self):
        parent_obj = self.get_object()
        parent_serializer = self.serializer_class(parent_obj)
        field_name = self.get_related_field_name()
        field = parent_serializer.fields.get(field_name, None)

        if field is not None:
            return field.get_attribute(parent_obj)
        else:
            try:
                return getattr(parent_obj, field_name)
            except AttributeError:
                raise NotFound

-------

in rest_framework/relations.py L162: self.use_pk_only_optimization() returns true from ResourceRelatedField.use_pk_only_optimization.

    def get_attribute(self, instance):
        if self.use_pk_only_optimization() and self.source_attrs:
            # Optimized case, return a mock object only containing the pk attribute.
            try:
                instance = get_attribute(instance, self.source_attrs[:-1])
                value = instance.serializable_value(self.source_attrs[-1])
                if is_simple_callable(value):
                    # Handle edge case where the relationship `source` argument
                    # points to a `get_relationship()` method on the model
                    value = value().pk
                return PKOnlyObject(pk=value)
            except AttributeError:
                pass


--------

in rest_framework_json_api/relations.py L197 ResourceRelatedField:

    def use_pk_only_optimization(self):
        # We need the real object to determine its type...
        return self.get_resource_type_from_included_serializer() is not None

@n2ygk
Copy link
Contributor Author

n2ygk commented Oct 8, 2018

This works around the problem:

class RRF(ResourceRelatedField):
    def use_pk_only_optimization(self):
        return False

@n2ygk
Copy link
Contributor Author

n2ygk commented Oct 8, 2018

Looks like this commit: b4dffb8 in #374

@santiavenda2 perhaps you can take a look.

commit b4dffb8291d5c5ad7e9dae52a6942709082ff77a
Author: santiavenda <[email protected]>
Date:   Mon Jan 8 18:02:47 2018 -0800

    Avoid query explosion for related fields where I can user pk_only_opt… (#374)
    
    * Fix factory boy dependency
    
    * fix factory-boy version in setup.py and requirements-development.txt
    
    * Fix setup.py factory boy dependency
    
    * Avoid query explosion for related fields where I can user pk_only_optimization
    
    * Fix autoPrefetchMixin for ReverseOneToOneDescriptor
    
    * Fix code style
    
    * Avoid query objects in ModelSerializer to_representation method
    
    * Fix code queality error
    
    * Fix problem that makes None related objects not being renderer
    
    * Fix problem that makes None related objects not being renderer
    
    * _readable_field property is missing in drf<3.2

@Anton-Shutik
Copy link
Contributor

@n2ygk Hi, can you show how the fields are declared on your CourseSerializer ?

@n2ygk
Copy link
Contributor Author

n2ygk commented Oct 23, 2018

@Anton-Shutik It's here:

class CourseSerializer(HyperlinkedModelSerializer):
"""
(de-)serialize the Course.
"""
terms = relations.ResourceRelatedField(
model=Term,
many=True,
read_only=False,
allow_null=True,
required=False,
queryset=Term.objects.all(),
self_link_view_name='course-relationships',
related_link_view_name='course-related',
)
# 'included' support (also used for `related_serializers` for DJA 2.6.0)
included_serializers = {
'terms': 'example.serializers.TermSerializer',
}
class Meta:
model = Course
fields = (
'url',
'school_bulletin_prefix_code', 'suffix_two', 'subject_area_code',
'course_number', 'course_identifier', 'course_name', 'course_description',
'effective_start_date', 'effective_end_date',
'last_mod_user_name', 'last_mod_date',
'terms')

@n2ygk
Copy link
Contributor Author

n2ygk commented Oct 23, 2018

@Anton-Shutik and see also here for the test case:

# the following test reproduces/confirm fix for this bug:
# https://github.com/django-json-api/django-rest-framework-json-api/issues/489
def test_term_related_course(self):
"""
confirm that the related child data reference the parent
"""
term_id = self.term.first().pk
kwargs = {'pk': term_id, 'related_field': 'course'}
url = reverse('term-related', kwargs=kwargs)
with mock.patch('rest_framework_json_api.views.RelatedMixin.override_pk_only_optimization',
True):
resp = self.client.get(url)
self.assertEqual(resp.status_code, 200)
dja_response = resp.json()
back_reference = dja_response['data']['relationships']['terms']['data']
self.assertIn({"type": "terms", "id": str(term_id)}, back_reference)
# the following raises AttributeError:
with self.assertRaises(AttributeError) as ae:
with mock.patch(
'rest_framework_json_api.views.RelatedMixin.override_pk_only_optimization',
False):
resp = self.client.get(url)
print(ae.exception)
self.assertIn('`PKOnlyObject`', ae.exception.args[0])

n2ygk added a commit to n2ygk/django-rest-framework-json-api that referenced this issue Nov 12, 2018
n2ygk added a commit to n2ygk/django-rest-framework-json-api that referenced this issue Nov 12, 2018
…`retrieve_related` of a to-one relationship
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants