Skip to content

bind() included resources #901

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

Closed
SafaAlfulaij opened this issue Mar 30, 2021 · 6 comments
Closed

bind() included resources #901

SafaAlfulaij opened this issue Mar 30, 2021 · 6 comments

Comments

@SafaAlfulaij
Copy link
Contributor

Included resources that are instantiated in extract_included under renders.py doesn't bind fields field_name and parent.
I don't have a definitive use case yet, but sometimes it's needed to understand the tree structure of serializers.

field = serializer_class(relation_instance, many=True, context=context)

field = serializer_class(relation_instance, many=many, context=context)

@sliverc
Copy link
Member

sliverc commented Mar 31, 2021

Thanks for your report.

The included feature of JSON API spec is a prefetching feature so basically an API user should be able to receive the same data in included as they would if they call the API of that entity separately.

If a field_name would be binded in a serializer a developer could distinguish between the two and will most likely fall into the pit to have render the data different in included then on the separated API. Caching of resources like many UI framework do such as ember.js would then not work.

So without a definitive user case I wary to make this change in DJA.

You said you do not have a definitive use case but do you have an idea of a use case where this could be helpful then?

@SafaAlfulaij
Copy link
Contributor Author

Let's say we have a PostDetailSerializer serializing fields including id, name, author, body (this is the detail endpoint /posts/id).
And have a UserListSerializer serializing fields including id, display_name, last_post (this is the list endpoint /users)

If we want to include the last_post of each user in a list, then we would get it's body, which can increase the load to get that big data and transfer it.
So we can just have some logic in the PostDetailSerializer: if it's included (or give the full freedom, if it's included in this kind of place): remove body field (both from fields and query).

Perhaps there is another solution that I don't know about...

@sliverc
Copy link
Member

sliverc commented Apr 2, 2021

JSON API spec resp. DJA follows REST principles. One of the REST constraint is uniform interface

Any single resource should contain each and everything in its representation.

The compound document feature in JSON API spec is basically a feature to avoid too many http request but the resource as such remains the same as when fetched through its own end point.

In your example PostDetailSerializer when called per include query parameter does not serialize everything in its representation (body is missing). An API user therefore has a resource which does not contain everything but how should a user know what is missing? They can't as the resource has a unique id and per REST guideline it is expected to always get the full resource (JS framework such as ember.js heavily rely on this convention for caching of resources for instance).

A pitfall to be avoided. If it is a real concern that the body is too large what I would do in your example is to create another resource postBody and move the content of the body to that resource. This can then additionally be included as post.body and the api user can decide whether they need it or not.

I hope this makes a bit clearer why those parameters are not binded.

I am closing this issue therefore. If there are any other comments or example on how to use those parameters please feel free to comment. We can always reopen the issue again.

@sliverc sliverc closed this as completed Apr 2, 2021
@SafaAlfulaij
Copy link
Contributor Author

Something just came to my mind. Not directly related, but in shares the scope.

Now as per JSON:API spec, included serializers must match with the data you get from the related URL.
As I know, we can change the serializer in the related view (get_serializer_class). Doesn't this violate the role as we can't select what the included serializer is, in a similar way to how viewset's get_serializer_class work?

@SafaAlfulaij
Copy link
Contributor Author

Another question:
We have Post which has category. We also have Category witch has posts.
Now if we'd include category in PostSerializer, isn't it a) redundant to include the post ids in the included category, b) waste of SQL queries? c) complex to understand what to select_related/prefetch_related the needed fields to not kill the performance?

@sliverc
Copy link
Member

sliverc commented Apr 7, 2021

I do not know your complete use case but I am not sure it is a good idea to have posts on Category as this might end up to be a really large request (with or without includes). Another solution is to have a filter category on the post end point which can be called when you want to show all posts of a certain category.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants