Code

Ticket #17: 17_no_tests.diff

File 17_no_tests.diff, 10.8 KB (added by PhiR, 7 years ago)

cleanup and comments. Not working but comments welcome

Line 
1Index: django/db/models/base.py
2===================================================================
3--- django/db/models/base.py    (revision 6250)
4+++ django/db/models/base.py    (working copy)
5@@ -15,6 +15,7 @@
6 from django.utils.encoding import smart_str, force_unicode, smart_unicode
7 from django.conf import settings
8 from itertools import izip
9+from weakref import WeakValueDictionary
10 import types
11 import sys
12 import os
13@@ -77,6 +78,35 @@
14         # registered version.
15         return get_model(new_class._meta.app_label, name, False)
16 
17+    def __call__(cls, *args, **kwargs):
18+        """
19+        this method will either create an instance (by calling the default implementation)
20+        or try to retrieve one from the class-wide cache by infering the pk value from
21+        args and kwargs. If instance caching is enabled for this class, the cache is
22+        populated whenever possible (ie when it is possible to infer the pk value).
23+        """
24+        def new_instance():
25+            return super(ModelBase, cls).__call__(*args, **kwargs)
26+       
27+        # we always pop those settings from kwargs not to pollute the instance
28+        instance_caching_enabled = kwargs.pop('meta__instance_caching', False) or cls.meta__instance_caching
29+        # simplest case, just create a new instance every time
30+        if not instance_caching_enabled:
31+            return new_instance()
32+       
33+        instance_key = cls._get_cache_key(args, kwargs)
34+        # depending on the arguments, we might not be able to infer the PK, so in that case we create a new instance
35+        if instance_key is None:
36+            return new_instance()
37+
38+        cached_instance = cls.get_cached_instance(key)
39+        if cached_instance is None:
40+            cached_instance = new_instance()
41+            # FIXME test cached_instance._get_pk_val() ==  instance_key
42+            cls.cache_instance(cached_instance)
43+           
44+        return cached_instance
45+
46 class Model(object):
47     __metaclass__ = ModelBase
48 
49@@ -97,6 +127,50 @@
50     def __ne__(self, other):
51         return not self.__eq__(other)
52 
53+    def _get_cache_key(cls, args, kwargs):
54+        result = None
55+        pk = cls._meta.pk
56+        # get the index of the pk in the class fields. this should be calculated *once*, but isn't atm
57+        pk_position = cls._meta.fields.index(pk)
58+        # if it's in the args, we can get it easily by index
59+        if len(args) > pk_position:
60+            result = args[pk_position]
61+        # retrieve the pk value. Note that we use attname instead of name, to handle the case where the pk is a
62+        # a ForeignKey.
63+        elif pk.attname in kwargs:
64+            result = kwargs[pk.attname]
65+        # ok we couldn't find the value, but maybe it's a FK and we can find the corresponding object instead
66+        elif pk.name != pk.attname and pk.name in kwargs:
67+            result = kwargs[pk.name]
68+        # if the pk value happens to be a model (which can happen wich a FK), we'd rather use its own pk as the key
69+        if result is not None and isinstance(result, Model):
70+            result = result._get_pk_val()
71+        return result
72+    _get_cache_key = classmethod(_get_cache_key)
73+
74+    def get_cached_instance(cls, id):
75+        """
76+        Method to retrieve a cached instance by pk value. Returns None when not found (which will always be the case when caching is disabled for this class).
77+        """
78+        return cls.__instance_cache__.get(id)
79+    get_cached_instance = classmethod(get_cached_instance)
80+
81+    def cache_instance(cls, instance):
82+        """
83+        Method to store an instance in the cache. The instance will only be stored if 'instance.meta__instance_cache' is 'True', which means it is
84+        possible to override the class-wide settings in the instance.
85+        """
86+        if instance.meta__instance_cache and instance._get_pk_val() is not None:
87+            cls.__instance_cache__[instance._get_pk_val()] = instance
88+    cache_instance = classmethod(cache_instance)
89+
90+    def flush_cached_instance(cls, instance):
91+        """
92+        Method to flush an instance from the cache. The instance will always be flushed from the cache, since this is most likely called from delete().
93+        We do not test the pk value because delete() does it and it will fail silently anyway.
94+        """
95+        self.__instance_cache__.pop(_get_pk_val(), None)
96+
97     def __init__(self, *args, **kwargs):
98         dispatcher.send(signal=signals.pre_init, sender=self.__class__, args=args, kwargs=kwargs)
99 
100@@ -197,6 +271,10 @@
101         if hasattr(cls, 'get_absolute_url'):
102             cls.get_absolute_url = curry(get_absolute_url, opts, cls.get_absolute_url)
103 
104+        cls.__instance_cache__ = WeakValueDictionary()
105+        # enable the cache according to user preferences (off by default)
106+        cls.meta__instance_caching = getattr(cls, 'meta__instance_caching', False)
107+
108         dispatcher.send(signal=signals.class_prepared, sender=cls)
109 
110     _prepare = classmethod(_prepare)
111@@ -260,6 +338,9 @@
112                 setattr(self, self._meta.pk.attname, connection.ops.last_insert_id(cursor, self._meta.db_table, self._meta.pk.column))
113         transaction.commit_unless_managed()
114 
115+        # if we're a new instance that hasn't been written in; save ourself.
116+        self.__class__.cache_instance(self)
117+
118         # Run any post-save hooks.
119         dispatcher.send(signal=signals.post_save, sender=self.__class__, instance=self)
120 
121@@ -319,6 +400,8 @@
122         seen_objs = SortedDict()
123         self._collect_sub_objects(seen_objs)
124 
125+        # remove ourself from the cache
126+        self.__class__.flush_cached_instance(self)
127         # Actually delete the objects
128         delete_objects(seen_objs)
129 
130Index: django/db/models/fields/related.py
131===================================================================
132--- django/db/models/fields/related.py  (revision 6250)
133+++ django/db/models/fields/related.py  (working copy)
134@@ -165,12 +165,15 @@
135                 if self.field.null:
136                     return None
137                 raise self.field.rel.to.DoesNotExist
138-            other_field = self.field.rel.get_related_field()
139-            if other_field.rel:
140-                params = {'%s__pk' % self.field.rel.field_name: val}
141-            else:
142-                params = {'%s__exact' % self.field.rel.field_name: val}
143-            rel_obj = self.field.rel.to._default_manager.get(**params)
144+            # try to get a cached instance, and if that fails retrieve it from the db
145+            rel_obj = self.field.rel.to.get_cached_instance(val)
146+            if rel_obj is None:
147+                other_field = self.field.rel.get_related_field()
148+                if other_field.rel:
149+                    params = {'%s__pk' % self.field.rel.field_name: val}
150+                else:
151+                    params = {'%s__exact' % self.field.rel.field_name: val}
152+                rel_obj = self.field.rel.to._default_manager.get(**params)
153             setattr(instance, cache_name, rel_obj)
154             return rel_obj
155 
156Index: django/db/models/query.py
157===================================================================
158--- django/db/models/query.py   (revision 6250)
159+++ django/db/models/query.py   (working copy)
160@@ -1134,6 +1134,11 @@
161             dispatcher.send(signal=signals.pre_delete, sender=cls, instance=instance)
162 
163         pk_list = [pk for pk,instance in seen_objs[cls]]
164+        # we wipe the cache now; it's *possible* some form of a __get__ lookup may reintroduce an item after
165+        # the fact with the same pk (extremely unlikely)
166+        for instance in seen_objs.values():
167+            cls.flush_cached_instance(instance)
168+
169         for related in cls._meta.get_all_related_many_to_many_objects():
170             if not isinstance(related.field, generic.GenericRelation):
171                 for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
172@@ -1167,6 +1172,8 @@
173     for cls in ordered_classes:
174         seen_objs[cls].reverse()
175         pk_list = [pk for pk,instance in seen_objs[cls]]
176+        for instance in seen_objs.values():
177+            cls.flush_cached_instance(instance)
178         for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
179             cursor.execute("DELETE FROM %s WHERE %s IN (%s)" % \
180                 (qn(cls._meta.db_table), qn(cls._meta.pk.column),
181Index: django/core/serializers/xml_serializer.py
182===================================================================
183--- django/core/serializers/xml_serializer.py   (revision 6250)
184+++ django/core/serializers/xml_serializer.py   (working copy)
185@@ -176,7 +176,7 @@
186                 else:
187                     value = field.to_python(getInnerText(field_node).strip())
188                 data[field.name] = value
189-
190+        data["meta__instance_cache"] = False
191         # Return a DeserializedObject so that the m2m data has a place to live.
192         return base.DeserializedObject(Model(**data), m2m_data)
193 
194@@ -234,4 +234,3 @@
195         else:
196            pass
197     return u"".join(inner_text)
198-
199Index: django/core/serializers/python.py
200===================================================================
201--- django/core/serializers/python.py   (revision 6250)
202+++ django/core/serializers/python.py   (working copy)
203@@ -88,7 +88,7 @@
204             # Handle all other fields
205             else:
206                 data[field.name] = field.to_python(field_value)
207-
208+        data["meta__instance_cache"] = False
209         yield base.DeserializedObject(Model(**data), m2m_data)
210 
211 def _get_model(model_identifier):
212Index: tests/modeltests/select_related/models.py
213===================================================================
214--- tests/modeltests/select_related/models.py   (revision 6250)
215+++ tests/modeltests/select_related/models.py   (working copy)
216@@ -107,13 +107,13 @@
217 1
218 
219 # select_related() also of course applies to entire lists, not just items.
220-# Without select_related()
221+# Without select_related() (note instance caching still reduces this from 9 to 5)
222 >>> db.reset_queries()
223 >>> world = Species.objects.all()
224 >>> [o.genus.family for o in world]
225 [<Family: Drosophilidae>, <Family: Hominidae>, <Family: Fabaceae>, <Family: Amanitacae>]
226 >>> len(db.connection.queries)
227-9
228+5
229 
230 # With select_related():
231 >>> db.reset_queries()
232@@ -129,23 +129,23 @@
233 >>> pea.genus.family.order.klass.phylum.kingdom.domain
234 <Domain: Eukaryota>
235 
236-# Notice: one few query than above because of depth=1
237+# notice: instance caching saves the day; would be 7 without.
238 >>> len(db.connection.queries)
239-7
240+1
241 
242 >>> db.reset_queries()
243 >>> pea = Species.objects.select_related(depth=5).get(name="sativum")
244 >>> pea.genus.family.order.klass.phylum.kingdom.domain
245 <Domain: Eukaryota>
246 >>> len(db.connection.queries)
247-3
248+1
249 
250 >>> db.reset_queries()
251 >>> world = Species.objects.all().select_related(depth=2)
252 >>> [o.genus.family.order for o in world]
253 [<Order: Diptera>, <Order: Primates>, <Order: Fabales>, <Order: Agaricales>]
254 >>> len(db.connection.queries)
255-5
256+1
257 
258 # Reset DEBUG to where we found it.
259 >>> settings.DEBUG = False