Make cache updating when making changes work...but it probably needs …

…some refactoring.

auslib/db.py

@@ -1025,6 +1025,9 @@ def updateRelease(self, name, changed_by, old_data_version, product=None, versio
                 raise ValueError("Release blob contains forbidden domain.")
             what['data'] = blob.getJSON()
         self.update(where=[self.name == name], what=what, changed_by=changed_by, old_data_version=old_data_version, transaction=transaction)
+        new_data_version = old_data_version + 1
+        cache.put("blob", name, {"data_version": new_data_version, "blob": blob})
+        cache.put("blob_version", name, new_data_version)
 
     def addLocaleToRelease(self, name, platform, locale, data, old_data_version, changed_by, transaction=None, alias=None):
         """Adds or update's the existing data for a specific platform + locale
@@ -1064,6 +1067,9 @@ def addLocaleToRelease(self, name, platform, locale, data, old_data_version, cha
         what = dict(data=releaseBlob.getJSON())
         self.update(where=where, what=what, changed_by=changed_by, old_data_version=old_data_version,
                     transaction=transaction)
+        new_data_version = old_data_version + 1
+        cache.put("blob", name, {"data_version": new_data_version, "blob": releaseBlob})
+        cache.put("blob_version", name, new_data_version)
 
     def getLocale(self, name, platform, locale, transaction=None):
         try:
@@ -1082,6 +1088,8 @@ def localeExists(self, name, platform, locale, transaction=None):
     def deleteRelease(self, changed_by, name, old_data_version, transaction=None):
         where = [self.name == name]
         self.delete(changed_by=changed_by, where=where, old_data_version=old_data_version, transaction=transaction)
+        cache.invalidate("blob", name)
+        cache.invalidate("blob_version", name)
 
 
 class Permissions(AUSTable):

auslib/test/test_db.py

@@ -1099,17 +1099,10 @@ def testGetReleaseBlobCachingWithDataVersionChange(self):
             self._checkCacheStats(cache.caches["blob"], 3, 2, 1)
             self._checkCacheStats(cache.caches["blob_version"], 3, 2, 1)
 
-            # Now change it, which will change data_version.
+            # Now change it, which should update the caches
             newBlob = ReleaseBlobV1(name="b", appv="2")
             self.releases.updateRelease("b", "bob", 1, blob=newBlob)
 
-            # Because the ttl of the blob_version cache is 4 and t is only at 3
-            # we need to retrieve the blob one more time before we will get the
-            # updated version.
-            blob = self.releases.getReleaseBlob(name="b")
-            self.assertTrue("appv" not in blob)
-            t.return_value += 1
-
             # Ensure that we have the updated version, not the originally
             # cached one.
             blob = self.releases.getReleaseBlob(name="b")
@@ -1123,13 +1116,65 @@ def testGetReleaseBlobCachingWithDataVersionChange(self):
             t.return_value += 1
             self.releases.getReleaseBlob(name="b")
 
-            # Because getReleaseBlob can't decide whether or not the cached
-            # blob is fresh enough until after it retrieves it (and adjusts
-            # the statistics), these numbers are a bit of a lie. In an ideal
-            # world we'd adjust these to be 100% accurate.
-            self._checkCacheStats(cache.caches["blob"], 8, 7, 1)
-            # Data version hit counts are 100% accurate though.
-            self._checkCacheStats(cache.caches["blob_version"], 8, 6, 2)
+            self._checkCacheStats(cache.caches["blob"], 7, 6, 1)
+            self._checkCacheStats(cache.caches["blob_version"], 7, 6, 1)
+
+    def testUpdateReleaseUpdatesCaches(self):
+        with mock.patch("time.time") as t:
+            t.return_value = 0
+            self.releases.getReleaseBlob(name="b")
+            t.return_value += 1
+            self.releases.getReleaseBlob(name="b")
+            t.return_value += 1
+            newBlob = ReleaseBlobV1(name="b", appv="2")
+            self.releases.updateRelease("b", "bob", 1, blob=newBlob)
+            t.return_value += 1
+            blob = self.releases.getReleaseBlob(name="b")
+
+            self.assertEquals(blob, newBlob)
+            self._checkCacheStats(cache.caches["blob"], 3, 2, 1)
+            self._checkCacheStats(cache.caches["blob_version"], 3, 2, 1)
+
+    def testDeleteReleaseClobbersCache(self):
+        with mock.patch("time.time") as t:
+            t.return_value = 0
+            self.releases.getReleaseBlob(name="b")
+            t.return_value += 1
+            self.releases.getReleaseBlob(name="b")
+            t.return_value += 1
+            self.releases.deleteRelease("bob", "b", 1)
+            t.return_value += 1
+
+            self._checkCacheStats(cache.caches["blob"], 2, 1, 1)
+            self._checkCacheStats(cache.caches["blob_version"], 2, 1, 1)
+            self.assertRaises(KeyError, self.releases.getReleaseBlob, name="b")
+
+    def testAddLocaleToReleaseUpdatesCaches(self):
+        with mock.patch("time.time") as t:
+            t.return_value = 0
+            self.releases.getReleaseBlob(name="b")
+            t.return_value += 1
+            self.releases.addLocaleToRelease("b", "win", "zu", dict(buildID=123), 1, "bob")
+            t.return_value += 1
+            blob = self.releases.getReleaseBlob(name="b")
+
+            newBlob = {
+                "schema_version": 1,
+                "name": "b",
+                "platforms": {
+                    "win": {
+                        "locales": {
+                            "zu": {
+                                "buildID": 123,
+                            }
+                        }
+                    }
+                }
+            }
+            # XXX: THIS check should fail right now. why doesn't it?!
+            self.assertEquals(blob, newBlob)
+            self._checkCacheStats(cache.caches["blob"], 3, 2, 1)
+            self._checkCacheStats(cache.caches["blob_version"], 3, 2, 1)
 
 #    def testGetReleaseBlobDataChangesBetweenCacheLooksup(self):
 #        """Makes sure that data changing between retrieval of data version

auslib/util/cache.py

@@ -1,3 +1,5 @@
+from copy import deepcopy
+
 from repoze.lru import ExpiringLRUCache
 
 
@@ -41,12 +43,19 @@ def get(self, name, key, value_getter=None):
             value = value_getter()
             self.put(name, key, value)
 
+        # Copy the value to make sure the caller can't accidentally update the
+        # cached version. If they want to update it, they should call "put"
+        # explicitly.
+        value = deepcopy(value)
         return value
 
     def put(self, name, key, value):
         if name not in self.caches:
             return
 
+        # Copy the value to make sure the caller can't accicdentally update the
+        # cached version.
+        value = deepcopy(value)
         return self.caches[name].put(key, value)
 
     def clear(self, name=None):

uwsgi/admin.wsgi

@@ -27,7 +27,6 @@ from auslib.global_state import cache, dbo
 auslib.log.cef_config = auslib.log.get_cef_config("syslog")
 
 cache.make_cache("blob", 500, 3600)
-cache.make_cache("blob_version", 500, 60)
 
 dbo.setDb(os.environ["DBURI"])
 dbo.setupChangeMonitors(SYSTEM_ACCOUNTS)