edits

Author: timgraham

django_mongodb_backend/cache.py

@@ -59,19 +59,17 @@ def collection_for_write(self):
         db = router.db_for_write(self.cache_model_class)
         return connections[db].get_collection(self._collection_name)
 
-    def get(self, key, default=None, version=None):
-        return self.get_many([key], version).get(key, default)
-
     def _filter_expired(self, expired=False):
         """
-        Create a filter to exclude expired data by default
-        or include only expired data if `expired` is True.
+        Return MQL to exclude expired entries (needed because the MongoDB
+        daemon does not remove expired entries precisely when it expires).
+        If expired=True, return MQL to include only expired entries.
         """
-        return (
-            {"expires_at": {"$lt": datetime.utcnow()}}
-            if expired
-            else {"expires_at": {"$gte": datetime.utcnow()}}
-        )
+        op = "$lt" if expired else "$gte"
+        return {"expires_at": {op: datetime.utcnow()}}
+
+    def get(self, key, default=None, version=None):
+        return self.get_many([key], version).get(key, default)
 
     def get_many(self, keys, version=None):
         if not keys:
@@ -93,7 +91,7 @@ def set(self, key, value, timeout=DEFAULT_TIMEOUT, version=None):
                 "$set": {
                     "key": key,
                     "value": self.serializer.dumps(value),
-                    "expires_at": self._get_expiration_time(timeout),
+                    "expires_at": self.get_backend_timeout(timeout),
                 }
             },
             upsert=True,
@@ -111,7 +109,7 @@ def add(self, key, value, timeout=DEFAULT_TIMEOUT, version=None):
                     "$set": {
                         "key": key,
                         "value": self.serializer.dumps(value),
-                        "expires_at": self._get_expiration_time(timeout),
+                        "expires_at": self.get_backend_timeout(timeout),
                     }
                 },
                 upsert=True,
@@ -124,10 +122,13 @@ def _cull(self, num):
         if self._cull_frequency == 0:
             self.clear()
         else:
+            # The fraction of entries that are culled when MAX_ENTRIES is
+            # reached is 1 / CULL_FREQUENCY. For example, in the default case
+            # of CULL_FREQUENCY=3, 2/3 of the entries are kept.
             keep_num = num - num // self._cull_frequency
             try:
-                # Find the first entry beyond the retention limit,
-                # prioritizing earlier expiration dates.
+                # Find the first cache entry beyond the retention limit,
+                # culling entries that expire the soonest.
                 deleted_from = next(
                     self.collection_for_write.aggregate(
                         [
@@ -139,16 +140,19 @@ def _cull(self, num):
                     )
                 )
             except StopIteration:
-                # No entries found beyond the retention limit, nothing to delete.
+                # If no entries are found, there is nothing to delete. It may
+                # happen if the database removes expired entries between the
+                # query to get `num` and the query to get `deleted_from`.
                 pass
             else:
-                # Delete all entries with an earlier expiration date.
-                # If multiple entries share the same expiration date,
-                # delete those with a greater or equal key.
+                # Cull the cache.
                 self.collection_for_write.delete_many(
                     {
                         "$or": [
+                            # Delete keys that expire before `deleted_from`...
                             {"expires_at": {"$lt": deleted_from["expires_at"]}},
+                            # and the entries that share an expiration with
+                            # `deleted_from` but are alphabetically after it.
                             {
                                 "$and": [
                                     {"expires_at": deleted_from["expires_at"]},
@@ -162,7 +166,7 @@ def _cull(self, num):
     def touch(self, key, timeout=DEFAULT_TIMEOUT, version=None):
         key = self.make_and_validate_key(key, version=version)
         res = self.collection_for_write.update_one(
-            {"key": key}, {"$set": {"expires_at": self._get_expiration_time(timeout)}}
+            {"key": key}, {"$set": {"expires_at": self.get_backend_timeout(timeout)}}
         )
         return res.matched_count > 0
 
@@ -185,10 +189,10 @@ def incr(self, key, delta=1, version=None):
             raise ValueError(f"Key '{key}' not found.") from None
         return updated["value"]
 
-    def _get_expiration_time(self, timeout=None):
+    def get_backend_timeout(self, timeout=DEFAULT_TIMEOUT):
         if timeout is None:
             return datetime.max
-        timestamp = self.get_backend_timeout(timeout)
+        timestamp = super().get_backend_timeout(timeout)
         return datetime.fromtimestamp(timestamp, tz=timezone.utc)
 
     def delete(self, key, version=None):
@@ -205,12 +209,10 @@ def _delete_many(self, keys, version=None):
 
     def has_key(self, key, version=None):
         key = self.make_and_validate_key(key, version=version)
-        return (
-            self.collection_for_read.count_documents(
-                {"key": key, **self._filter_expired(expired=False)}
-            )
-            > 0
+        num = self.collection_for_read.count_documents(
+            {"key": key, **self._filter_expired(expired=False)}
         )
+        return num > 0
 
     def clear(self):
         self.collection_for_write.delete_many({})

django_mongodb_backend/management/commands/createcachecollection.py

@@ -12,7 +12,6 @@
 
 class Command(BaseCommand):
     help = "Creates the collections needed to use the MongoDB cache backend."
-
     requires_system_checks = []
 
     def add_arguments(self, parser):

docs/source/topics/known-issues.rst

@@ -102,4 +102,5 @@ Caching
 :djadmin:`createcachetable`.
 
 Secondly, you must use the :class:`django_mongodb_backend.cache.MongoDBCache`
-backend rather than Django's built-in database cache backend.
+backend rather than Django's built-in database cache backend,
+``django.core.cache.backends.db.DatabaseCache``).

tests/cache_/tests.py

@@ -92,12 +92,31 @@ def caches_setting_for_tests(base=None, exclude=None, **params):
     return setting
 
 
-class BaseCacheTests:
+@override_settings(
+    CACHES=caches_setting_for_tests(
+        BACKEND="django_mongodb_backend.cache.MongoDBCache",
+        # Spaces are used in the name to ensure quoting/escaping works.
+        LOCATION="test cache collection",
+    ),
+)
+@modify_settings(
+    INSTALLED_APPS={"prepend": "django_mongodb_backend"},
+)
+class CacheTests(TestCase):
     factory = RequestFactory()
     incr_decr_type_error_msg = "Cannot apply %s() to a value of non-numeric type."
 
-    def tearDown(self):
-        cache.clear()
+    def setUp(self):
+        # The super calls needs to happen first for the settings override.
+        super().setUp()
+        self.create_cache_collection()
+        self.addCleanup(self.drop_collection)
+
+    def create_cache_collection(self):
+        management.call_command("createcachecollection", verbosity=0)
+
+    def drop_collection(self):
+        cache.collection_for_write.drop()
 
     def test_simple(self):
         # Simple cache set/get works
@@ -930,34 +949,10 @@ def test_collection_has_indexes(self):
             )
         )
 
-
-@override_settings(
-    CACHES=caches_setting_for_tests(
-        BACKEND="django_mongodb_backend.cache.MongoDBCache",
-        # Spaces are used in the table name to ensure quoting/escaping is working
-        LOCATION="test cache table",
-    ),
-)
-@modify_settings(
-    INSTALLED_APPS={"prepend": "django_mongodb_backend"},
-)
-class DBCacheTests(BaseCacheTests, TestCase):
-    def setUp(self):
-        # The super calls needs to happen first for the settings override.
-        super().setUp()
-        self.create_cache_collection()
-        self.addCleanup(self.drop_collection)
-
-    def drop_collection(self):
-        cache.collection_for_write.drop()
-
-    def create_cache_collection(self):
-        management.call_command("createcachecollection", verbosity=0)
-
-
-@override_settings(USE_TZ=True)
-class DBCacheWithTimeZoneTests(DBCacheTests):
-    pass
+    def test_serializer_dumps(self):
+        self.assertEqual(cache.serializer.dumps(123), 123)
+        self.assertIsInstance(cache.serializer.dumps(True), bytes)
+        self.assertIsInstance(cache.serializer.dumps("abc"), bytes)
 
 
 class DBCacheRouter:
@@ -1001,6 +996,5 @@ def test_createcachetable_observes_database_router(self):
         # cache table should be created on 'other'
         # Queries:
         #   1: Create indexes
-        num = 1
-        with self.assertNumQueries(num, using="other"):
+        with self.assertNumQueries(1, using="other"):
             management.call_command("createcachecollection", database="other", verbosity=0)