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

Add serializer name to cache namespace #193

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benwalsh
Copy link

Current Behaviour

When different serializations of the same object are created, the cache namespace and key used is the same, which can cause the wrong representation to be retrieved.

For example, if an "index" page for "Articles" uses a lightweight "preview" representation of each article to show a thumbnail image, and headline, but the "show" page uses a full serialization including the article content, metadata, author information, etc. then on navigating to the "show" page the cache will return the "preview" serialization.

The exception to this is a serialization using parse fieldsets, which modifies the cache namespace to avoid collisions between the default serialization and the serialization using a specified fieldset.

New Behaviour

The name of the serializer being invoked is added to the namespace (in addition to the fieldset, if it exists), which will avoid these collisions.

References

This should address these issues from the beforetime:

Netflix/fast_jsonapi/issues/300
Netflix/fast_jsonapi/issues/389
Netflix/fast_jsonapi/issues/394

@stas
Copy link
Collaborator

stas commented Jul 13, 2021

@benwalsh thank you very much for the PR, but I'm a bit reluctant to merge this.

The thing is that it's not the first time when folks request all kind of customizations around the caching strategy and I'd rather encourage everyone to use the available API than try to accommodate everyone's needs.

This being said, please take a look at an example how to customize the caching parameters and let me know what you think:
#148 (comment)

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

Successfully merging this pull request may close these issues.

None yet

2 participants