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

get_name() call for hyperlinked relationships not required for JSON - should be lazy. #4476

Closed
5 of 6 tasks
melinath opened this issue Sep 8, 2016 · 3 comments
Closed
5 of 6 tasks
Labels
Milestone

Comments

@melinath
Copy link

melinath commented Sep 8, 2016

Checklist

  • I have verified that that issue exists against the master branch of Django REST framework.
  • I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
  • This is not a usage question. (Those should be directed to the discussion group instead.)
  • This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
  • I have reduced the issue to the simplest possible case.
  • I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)

Steps to reproduce

  1. Set up a model with a related model.
  2. Use that related model in the __str__ method of the model.
  3. Set up a viewset / HyperlinkedModelSerializer / etc. for the model with no nesting and no select_related() call.

Expected behavior

The API should run one query to fetch the data to be serialized to JSON.

Actual behavior

The API runs one query to fetch the data to be serialized. And then it runs N queries by calling get_name on each object (which calls __str__, which references the related model).

I suppose you could say that this is my fault for not having a 100% pure __str__ method - but sometimes practicality beats purity. __str__ for me is primarily supposed to make it easy for me to tell what an object is when introspecting it in the shell or some other situation. I would also expect it to be called, say, when I'm in the admin looking at a list of objects and their representations.

I would not expect it to be called when I'm looking at a JSON serialized version of the model that doesn't even use the results of the method anywhere. This seems like a pretty big potential pitfall that people might not even realize they're walking into.

Potential solutions:

  • make it lazy somehow?
  • don't call it if the format is JSON?
@decentral1se
Copy link
Contributor

decentral1se commented Sep 9, 2016

and no select_related() call.

Isn't the danger mitigated when you would use select_related on your serializer?

This could be another motivation to get #1977 out.

@tomchristie
Copy link
Member

Good point, yup, we return Hyperlink(url, name), rather than the raw url, so as you say a __str__ on the model that requires a lookup will be a problem.

We should make the call lazy.

@tomchristie tomchristie added the Bug label Sep 9, 2016
@tomchristie tomchristie added this to the 3.5.0 Release milestone Sep 9, 2016
@tomchristie tomchristie changed the title Default DRF get_name causes potentially massive query bloat get_name() call for hyperlinked relationships not required for JSON - should be lazy. Sep 9, 2016
@tomchristie
Copy link
Member

Retitled to better match the intent.

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

3 participants