Skip to content

Commit

Permalink
Revert "global serializer cache"
Browse files Browse the repository at this point in the history
  • Loading branch information
ryochiji committed Feb 6, 2016
1 parent 0ce1ec0 commit cae44c7
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 19 deletions.
1 change: 0 additions & 1 deletion .isort.cfg
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
[settings]
multi_line_output=3
known_third_party=django,pylru,rest_framework
6 changes: 1 addition & 5 deletions dynamic_rest/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,7 @@

# PAGE_SIZE_QUERY_PARAM: global setting for the page size query parameter.
# Can be overriden at the viewset level.
'PAGE_SIZE_QUERY_PARAM': 'per_page',

# SERIALIZER_CACHE_MAX_COUNT: maximum number of serializers to cache
'SERIALIZER_CACHE_MAX_COUNT': 1000,

'PAGE_SIZE_QUERY_PARAM': 'per_page'
}


Expand Down
21 changes: 12 additions & 9 deletions dynamic_rest/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import importlib
import pickle

import pylru
from django.utils import six
from django.utils.functional import cached_property
from rest_framework import fields
Expand All @@ -14,10 +13,6 @@
from dynamic_rest.conf import settings
from dynamic_rest.meta import is_field_remote

serializer_cache = pylru.lrucache(
settings.SERIALIZER_CACHE_MAX_COUNT
)


class DynamicField(fields.Field):

Expand Down Expand Up @@ -169,10 +164,18 @@ def root_serializer(self):
def _get_cached_serializer(self, args, init_args):
enabled = settings.ENABLE_SERIALIZER_CACHE

if not self.parent or not self.field_name or not enabled:
root = self.root_serializer
if not root or not self.field_name or not enabled:
# Not enough info to use cache.
return self.serializer_class(*args, **init_args)

if not hasattr(root, '_descendant_serializer_cache'):
# Initialize dict to use as cache on root serializer.
# Arguably this is a Serializer concern, but we'll do it
# here so it's agnostic to the exact type of the root
# serializer (i.e. it could be a DRF serializer).
root._descendant_serializer_cache = {}

key_dict = {
'parent': self.parent.__class__.__name__,
'field': self.field_name,
Expand All @@ -181,14 +184,14 @@ def _get_cached_serializer(self, args, init_args):
}
cache_key = hash(pickle.dumps(key_dict))

if cache_key not in serializer_cache:
if cache_key not in root._descendant_serializer_cache:
szr = self.serializer_class(
*args,
**init_args
)
serializer_cache[cache_key] = szr
root._descendant_serializer_cache[cache_key] = szr

return serializer_cache[cache_key]
return root._descendant_serializer_cache[cache_key]

def _get_request_fields_from_parent(self):
"""Get request fields from the parent serializer."""
Expand Down
1 change: 0 additions & 1 deletion install_requires.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
Django>=1.7,<=1.9
djangorestframework>=3.1.0,<=3.3.0
pylru==1.0.9
6 changes: 3 additions & 3 deletions tests/test_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -647,18 +647,18 @@ def test_same_serializer_class_different_fields(self):
)
)

def test_different_roots_share_cache(self):
def test_different_roots(self):
serializer2 = CatSerializer(
request_fields={'home': {}, 'backup_home': {}}
)

home1 = self.serializer.fields['home']
home2 = serializer2.fields['home']

self.assertIs(
self.assertIsNot(
home1.serializer,
home2.serializer,
'Different root serializers should share instances.'
'Different root serializers should yield different instances.'
)

def test_root_serializer_cycle_busting(self):
Expand Down

0 comments on commit cae44c7

Please sign in to comment.