[Repoze-checkins] r1130 - in repoze.accelerator/trunk/repoze/accelerator: . tests

Chris McDonough chrism at agendaless.com
Tue Jun 24 23:00:16 EDT 2008


Author: Chris McDonough <chrism at agendaless.com>
Date: Tue Jun 24 23:00:16 2008
New Revision: 1130

Log:
Better storage API.

Manufacture date if we're not proxying.


Modified:
   repoze.accelerator/trunk/repoze/accelerator/interfaces.py
   repoze.accelerator/trunk/repoze/accelerator/policy.py
   repoze.accelerator/trunk/repoze/accelerator/storage.py
   repoze.accelerator/trunk/repoze/accelerator/tests/test_policy.py
   repoze.accelerator/trunk/repoze/accelerator/tests/test_storage.py

Modified: repoze.accelerator/trunk/repoze/accelerator/interfaces.py
==============================================================================
--- repoze.accelerator/trunk/repoze/accelerator/interfaces.py	(original)
+++ repoze.accelerator/trunk/repoze/accelerator/interfaces.py	Tue Jun 24 23:00:16 2008
@@ -66,21 +66,25 @@
     """
     def fetch(url):
         """ Return a sequence of entries from the backing store for
-        the given 'url'.  An entry is in the form (status, headers, body_iter,
-        req_discrims, env_discrims).
+        the given 'url'.  Each entry is in the form
+        (discriminators, expires, status, headers, body_iter, extras).
 
         o Return None on a miss.
         """
 
-    def store(url, status, headers, req_discrims, env_discrims):
+    def store(url, discriminators, expires, status, headers, **extras):
         """ Prepare to cache a response to a backing store.
 
-        o 'url' is the key for the response used during fetch.
-          Two stored entries should "hash" to the same value
-          if the tuple (url, req_discrims, env_discrims) is equal
-          for both.
+        o 'url' is the key for the response used during fetch..
+
+        o 'discrims' is a squence of two-tuples further discriminating the
+          entity.  The combination of the url and the discriminators for
+          a resource identifies it unambiguously.
+
+        o 'expires' is a UNIX timestamp representing the UTC time
+          after which this storage entry will no longer be fresh.
         
-        o 'status' and 'headers' should be saved as well.
+        o 'status', 'headers', and **extras should be saved as well.
 
         o Return an object implementing IChunkHandler, which will
           be used to cache the response body chunks.

Modified: repoze.accelerator/trunk/repoze/accelerator/policy.py
==============================================================================
--- repoze.accelerator/trunk/repoze/accelerator/policy.py	(original)
+++ repoze.accelerator/trunk/repoze/accelerator/policy.py	Tue Jun 24 23:00:16 2008
@@ -152,10 +152,11 @@
             matching = self._discriminate(entries, request_headers, environ)
             if not matching:
                 return
-        
-            status, response_headers, body = matching
 
-            if self._isfresh(response_headers):
+            now = time.time()
+        
+            discrims, expires, status, response_headers, body, extras = matching
+            if expires > now:
                 return status, response_headers, body
 
             # XXX purge?
@@ -169,6 +170,9 @@
             return
         if not (status.startswith('200') or status.startswith('203')):
             return
+        if environ['wsgi.url_scheme'] == 'https':
+            if not self.store_https_responses:
+                return
         if self._check_no_cache(response_headers, environ):
             return
         cc_header = header_value(response_headers, 'Cache-Control')
@@ -179,12 +183,6 @@
                     return
             except ValueError:
                 return
-        if environ['wsgi.url_scheme'] == 'https':
-            if not self.store_https_responses:
-                return
-        date = header_value(response_headers, 'Date')
-        if not date:
-            return
 
         # if we didn't abort due to any condition above, store the response
         vary_header_names = []
@@ -198,55 +196,59 @@
         if '*' in vary_header_names:
             return
 
-        req_discrims = []
+        discriminators = []
         for header_name in vary_header_names:
             value = header_value(request_headers, header_name)
             if value is not None:
-                req_discrims.append((header_name, value))
-
-        env_discrims = []
+                discriminators.append(('vary', (header_name, value)))
         for varname in self.always_vary_on_environ:
             value = environ.get(varname)
             if value is not None:
-                env_discrims.append((varname, value))
+                discriminators.append(('env', (varname, value)))
 
-        env_discrims.sort()
-        req_discrims.sort()
-        
+        discriminators = tuple(sorted(discriminators))
         headers = endtoend(response_headers)
         url = construct_url(environ)
 
+        # Response headers won't have a date if we aren't proxying to
+        # another http server on our right hand side.
+        date = header_value(response_headers, 'Date')
+        if date is None:
+            date = time.time()
+        else:
+            date = parsedate_tz(date)
+
+        expires = self._expires(date, response_headers)
+
         return self.storage.store(
             url,
+            discriminators,
+            expires,
             status,
             headers,
-            req_discrims,
-            env_discrims,
             )
 
     def _discriminate(self, entries, request_headers, environ):
 
-        def req_getter(name):
-            return header_value(request_headers, name)
-
         matching_entries = entries[:]
 
         for entry in entries:
-            status, headers, body, req_discrims, env_discrims = entry
-            discrims = [ (environ.get, x) for x in env_discrims ]
-            discrims.extend([ (req_getter, x) for x in req_discrims ])
-
-            for (getter, discrim) in discrims:
-                stored_name, stored_value = discrim
-                strval = getter(stored_name)
+            discrims, expires, status, headers, body, extras = entry
+            for discrim in discrims:
+                typ, (stored_name, stored_value) = discrim
+                if typ == 'env':
+                    strval = environ.get(stored_name)
+                elif typ == 'vary':
+                    strval = header_value(request_headers, stored_name)
+                else:
+                    raise ValueError(discrim)
                 if strval is None or strval != stored_value:
                     matching_entries.remove(entry)
                     break
 
         if matching_entries:
             match = matching_entries[0] # this is essentially random
-            status, headers, body = match[:3]
-            return status, headers, body
+            return match
 
     def _check_no_cache(self, headers, environ):
         for nocache in ('Pragma', 'Cache-Control'):
@@ -255,44 +257,35 @@
                 return True
         return False
 
-    def _isfresh(self, response_headers):
-        date = header_value(response_headers, 'Date')
-        if not date:
-            return False
-        date = parsedate_tz(date)
-        if not date:
-            return False
-        date = calendar.timegm(date)
-        now = time.time()
-        current_age = max(0, now - date)
-        cc_header = header_value(response_headers, 'Cache-Control')
-        expires_header = header_value(response_headers, 'Expires')
-
-        # freshness logic stolen from httplib2
-        lifetime = 0
+    def _expires(self, date, headers):
+        cc_header = header_value(headers, 'Cache-Control')
+        expires_header = header_value(headers, 'Expires')
 
+        # logic stolen from httplib2
         if cc_header is not None:
             header_parts = parse_cache_control_header(cc_header)
             if 'max-age' in header_parts:
                 try:
                     lifetime = int(header_parts['max-age'])
                     if lifetime == 0:
-                        return False
+                        return date
+                    return date + lifetime
                 except ValueError:
-                    lifetime = 0
-
-        if not lifetime:
-            if expires_header is not None:
-                expires = parsedate_tz(expires_header)
-                if expires is None:
-                    lifetime = 0
-                else:
-                    lifetime = max(0, calendar.timegm(expires) - date)
+                    return date
 
-        if lifetime > current_age:
-            return True
+        if expires_header is not None:
+            expires = parsedate_tz(expires_header)
+            if expires is None:
+                return date
+            else:
+                return calendar.timegm(expires)
 
-        return False
+    def _isfresh(self, date, headers):
+        now = time.time()
+        current_age = max(0, now - date)
+        expires = self._expires(date, headers)
+        lifetime = max(0, expires - date)
+        return lifetime > current_age
 
 def make_accelerator_policy(logger, storage, config):
     allowed_methods = config.get('policy.allowed_methods', 'GET')

Modified: repoze.accelerator/trunk/repoze/accelerator/storage.py
==============================================================================
--- repoze.accelerator/trunk/repoze/accelerator/storage.py	(original)
+++ repoze.accelerator/trunk/repoze/accelerator/storage.py	Tue Jun 24 23:00:16 2008
@@ -15,11 +15,9 @@
         self.data = {}
         self.lock = lock
 
-    def store(self, url, status, headers, req_discrims, env_discrims):
+    def store(self, url, discriminators, expires, status, headers, **extras):
         body = []
         storage = self
-        req_discrims = tuple(req_discrims)
-        env_discrims = tuple(env_discrims)
 
         class SimpleHandler:
             implements(IChunkHandler)
@@ -29,20 +27,20 @@
             def close(self):
                 storage.lock.acquire()
                 try:
-                    discrims = storage.data.setdefault(url, {})
-                    discrims[(req_discrims, env_discrims)]=status, headers, body
+                    entries = storage.data.setdefault(url, {})
+                    entries[discriminators] = expires,status,headers,body,extras
                 finally:
                     storage.lock.release()
 
         return SimpleHandler()
                 
     def fetch(self, url):
-        discrims = self.data.get(url)
-        if discrims is None:
+        entries = self.data.get(url)
+        if entries is None:
             return None
         L = []
-        for (req_d, env_d), (status, headers, body) in discrims.items():
-            L.append((status, headers, body, req_d, env_d))
+        for discrims, (expires,status,headers,body,extras) in entries.items():
+            L.append((discrims, expires, status, headers, body, extras))
         return L
 
 def make_memory_storage(logger, config):

Modified: repoze.accelerator/trunk/repoze/accelerator/tests/test_policy.py
==============================================================================
--- repoze.accelerator/trunk/repoze/accelerator/tests/test_policy.py	(original)
+++ repoze.accelerator/trunk/repoze/accelerator/tests/test_policy.py	Tue Jun 24 23:00:16 2008
@@ -130,7 +130,7 @@
         self.assertEqual(result, True)
         self.assertEqual(storage.url, 'http://example.com')
         self.assertEqual(storage.status, '200 OK')
-        self.assertEqual(storage.outheaders, headers)
+        self.assertEqual(storage.headers, headers)
 
     def test_store_no_request_method_cacheable(self):
         storage = DummyStorage(store_result=True)
@@ -142,7 +142,7 @@
         self.assertEqual(result, True)
         self.assertEqual(storage.url, 'http://example.com')
         self.assertEqual(storage.status, '200 OK')
-        self.assertEqual(storage.outheaders, headers)
+        self.assertEqual(storage.headers, headers)
 
     def test_store_get_request_method_cacheable(self):
         storage = DummyStorage(store_result=True)
@@ -153,7 +153,7 @@
         self.assertEqual(result, True)
         self.assertEqual(storage.url, 'http://example.com')
         self.assertEqual(storage.status, '200 OK')
-        self.assertEqual(storage.outheaders, headers)
+        self.assertEqual(storage.headers, headers)
 
     def test_store_with_request_vary(self):
         storage = DummyStorage(store_result=True)
@@ -166,8 +166,11 @@
         self.assertEqual(result, True)
         self.assertEqual(storage.url, 'http://example.com')
         self.assertEqual(storage.status, '200 OK')
-        self.assertEqual(storage.outheaders, headers)
-        self.assertEqual(storage.req_discrims, [('cookie', '12345')])
+        self.assertEqual(storage.headers, headers)
+        discrims = storage.discrims
+        self.assertEqual(len(discrims), 2)
+        self.assertEqual(discrims[0], ('env', ('REQUEST_METHOD', 'GET')))
+        self.assertEqual(discrims[1], ('vary', ('cookie', '12345')))
 
     def test_store_with_always_request_vary(self):
         storage = DummyStorage(store_result=True)
@@ -180,8 +183,11 @@
         self.assertEqual(result, True)
         self.assertEqual(storage.url, 'http://example.com')
         self.assertEqual(storage.status, '200 OK')
-        self.assertEqual(storage.outheaders, headers)
-        self.assertEqual(storage.req_discrims, [('cookie', '12345')])
+        self.assertEqual(storage.headers, headers)
+        discrims = storage.discrims
+        self.assertEqual(len(discrims), 2)
+        self.assertEqual(discrims[0], ('env', ('REQUEST_METHOD', 'GET')))
+        self.assertEqual(discrims[1], ('vary', ('cookie', '12345')))
 
     def test_store_with_always_request_vary_and_plain_request_vary(self):
         storage = DummyStorage(store_result=True)
@@ -196,9 +202,12 @@
         self.assertEqual(result, True)
         self.assertEqual(storage.url, 'http://example.com')
         self.assertEqual(storage.status, '200 OK')
-        self.assertEqual(storage.outheaders, headers)
-        self.assertEqual(storage.req_discrims,
-                         [('cookie', '12345'), ('x-foo', 'xfoo')])
+        self.assertEqual(storage.headers, headers)
+        discrims = storage.discrims
+        self.assertEqual(len(discrims), 3)
+        self.assertEqual(discrims[0], ('env', ('REQUEST_METHOD', 'GET')))
+        self.assertEqual(discrims[1], ('vary', ('cookie', '12345')))
+        self.assertEqual(discrims[2], ('vary', ('x-foo', 'xfoo')))
 
     def test_store_with_always_request_vary_star(self):
         storage = DummyStorage(store_result=True)
@@ -220,8 +229,10 @@
         self.assertEqual(result, True)
         self.assertEqual(storage.url, 'http://example.com')
         self.assertEqual(storage.status, '200 OK')
-        self.assertEqual(storage.outheaders, headers)
-        self.assertEqual(storage.env_discrims, [('REMOTE_USER', '12345')])
+        self.assertEqual(storage.headers, headers)
+        discrims = storage.discrims
+        self.assertEqual(len(discrims), 1)
+        self.assertEqual(discrims[0], ('env', ('REMOTE_USER', '12345')))
 
     def test_store_with_environ_vary_and_req_vary(self):
         storage = DummyStorage(store_result=True)
@@ -236,9 +247,11 @@
         self.assertEqual(result, True)
         self.assertEqual(storage.url, 'http://example.com')
         self.assertEqual(storage.status, '200 OK')
-        self.assertEqual(storage.outheaders, headers)
-        self.assertEqual(storage.req_discrims, [('cookie', '12345')])
-        self.assertEqual(storage.env_discrims, [('REMOTE_USER', '12345')])
+        self.assertEqual(storage.headers, headers)
+        discrims = storage.discrims
+        self.assertEqual(len(discrims), 2)
+        self.assertEqual(discrims[0], ('env', ('REMOTE_USER', '12345')))
+        self.assertEqual(discrims[1], ('vary', ('cookie', '12345')))
 
     def test_fetch_fails_post_request_method(self):
         storage = DummyStorage(fetch_result=False)
@@ -300,63 +313,70 @@
         headers = self._makeHeaders()
         cc = 'max-age=4000'
         headers.append(('Cache-Control', cc))
-        expected = (200, headers, [], [], [])
+        import sys
+        expected = ([], sys.maxint, 200, headers, [], {})
         storage = DummyStorage(fetch_result=[expected])
         policy = self._makeOne(storage)
         environ = self._makeEnviron()
         del environ['REQUEST_METHOD']
         result = policy.fetch(environ)
-        self.assertEqual(result, expected[:3])
+        self.assertEqual(result, expected[2:-1])
 
     def test_fetch_succeeds_get_request_method(self):
         headers = self._makeHeaders()
         cc = 'max-age=4000'
         headers.append(('Cache-Control', cc))
-        expected = (200, headers, [], [], [])
+        import sys
+        expected = ([], sys.maxint, 200, headers, [], {})
         storage = DummyStorage(fetch_result=[expected])
         policy = self._makeOne(storage)
         environ = self._makeEnviron()
         result = policy.fetch(environ)
-        self.assertEqual(result, expected[:3])
+        self.assertEqual(result, expected[2:-1])
 
     def test_fetch_succeeds_more_than_one_response_from_storage(self):
         headers = self._makeHeaders()
         cc = 'max-age=4000'
         headers.append(('Cache-Control', cc))
-        expected1 = (200, headers, [], [], [])
-        expected2 = (200, headers, [], [], [])
+        import sys
+        expected1 = ([], sys.maxint, 200, headers, [], {})
+        expected2 = ([], sys.maxint, 200, headers, [], {})
         storage = DummyStorage(fetch_result=[expected1] + [expected2])
         policy = self._makeOne(storage)
         environ = self._makeEnviron()
         result = policy.fetch(environ)
-        self.assertEqual(result, expected1[:3])
+        self.assertEqual(result, expected1[2:-1])
 
     def test_fetch_succeeds_via_discrimination(self):
         headers = self._makeHeaders()
         cc = 'max-age=4000'
         headers.append(('Cache-Control', cc))
+        import sys
+        then = sys.maxint
         stored = [
-            (200, headers, [], [('Cookie', '12345')], []),
-            (200, headers, [], [('X-Foo', '12345'), ('Cookie', '12345')], []),
-            (200, headers, [], [('X-Bar', '123')], []),
-             ]
+            ([('vary', ('Cookie', '12345'))], then, 200, headers, [], {}),
+            ([('vary', ('Cookie', '12345')), ('vary', ('X-Foo', '12345'))],
+             then, 200, headers, [], {}),
+            ([('vary', ('X-Bar', '123'))], then, 200, headers, [], {}),
+            ]
         storage = DummyStorage(fetch_result=stored)
         policy = self._makeOne(storage)
         environ = self._makeEnviron()
         environ['HTTP_COOKIE'] = '12345'
         environ['HTTP_X_FOO'] = '12345'
         result = policy.fetch(environ)
-        self.assertEqual(result, stored[1][:3])
+        self.assertEqual(result, stored[0][2:-1])
 
     def test_fetch_fails_via_discrimination(self):
         headers = self._makeHeaders()
         cc = 'max-age=4000'
         headers.append(('Cache-Control', cc))
         stored = [
-            (200, headers, [], [('Cookie', '12345')], []),
-            (200, headers, [], [('X-Foo', '12345'), ('Cookie', '12345')], []),
-            (200, headers, [], [('X-Bar', '123')], []),
-             ]
+            ([('vary', ('Cookie', '12345'))], 0, 200, headers, [], {}),
+            ([('vary', ('Cookie', '12345')), ('vary', ('X-Foo', '12345'))],
+                  0, 200, headers, [], {}),
+            ([('vary', ('X-Bar', '123'))], 0, 200, headers, [], {}),
+            ]
         storage = DummyStorage(fetch_result=stored)
         policy = self._makeOne(storage)
         environ = self._makeEnviron()
@@ -378,7 +398,8 @@
     def test_fresh_via_max_age(self):
         headers = self._makeHeaders()
         headers.append(('Cache-Control', 'max-age=4000'))
-        expected = (200, headers, [], [], [])
+        import sys
+        expected = ([], sys.maxint, 200, headers, [], {})
         storage = DummyStorage(fetch_result=[expected])
         policy = self._makeOne(storage)
         environ = self._makeEnviron()
@@ -391,7 +412,8 @@
         import time
         expires = formatdate(time.time() + 5000)
         headers.append(('Expires', expires))
-        expected = (200, headers, [], [], [])
+        import sys
+        expected = ([], sys.maxint, 200, headers, [], {})
         storage = DummyStorage(fetch_result=[expected])
         policy = self._makeOne(storage)
         environ = self._makeEnviron()
@@ -404,7 +426,7 @@
         date = formatdate(time.time() - 5000)
         headers = [('Date', date)]
         headers.append(('Cache-Control', 'max-age=10'))
-        expected = (200, headers, [], [], [])
+        expected = ([], 0, 200, headers, [], {})
         storage = DummyStorage(fetch_result=[expected])
         policy = self._makeOne(storage)
         environ = self._makeEnviron()
@@ -417,7 +439,7 @@
         from email.Utils import formatdate
         expires = formatdate(time.time() - 5000)
         headers.append(('Expires', expires))
-        expected = (200, headers, [], [], [])
+        expected = ([], 0, 200, headers, [], {})
         storage = DummyStorage(fetch_result=[expected])
         policy = self._makeOne(storage)
         environ = self._makeEnviron()
@@ -454,12 +476,13 @@
         self.fetch_result = fetch_result
         self.store_result = store_result
 
-    def store(self, url, status, outheaders, req_discrims, env_discrims):
+    def store(self, url, discrims, expires, status, headers, **extras):
         self.url = url
+        self.discrims = discrims
+        self.expires = expires
         self.status = status
-        self.outheaders = outheaders
-        self.req_discrims = req_discrims
-        self.env_discrims = env_discrims
+        self.headers = headers
+        self.extras = extras
         return self.store_result
 
     def fetch(self, url):

Modified: repoze.accelerator/trunk/repoze/accelerator/tests/test_storage.py
==============================================================================
--- repoze.accelerator/trunk/repoze/accelerator/tests/test_storage.py	(original)
+++ repoze.accelerator/trunk/repoze/accelerator/tests/test_storage.py	Tue Jun 24 23:00:16 2008
@@ -32,14 +32,14 @@
         lock = DummyLock()
         storage = self._makeOne(lock)
         headers = [('Header1', 'value1')]
-        handler = storage.store('url', 'status', headers, [], [])
+        handler = storage.store('url', (), 0, 'status', headers)
         self.failIf(handler is None)
         chunks = ['chunk1', 'chunk2']
         for chunk in ('chunk1', 'chunk2'):
             handler.write(chunk)
         handler.close()
-        self.assertEqual(storage.data['url'][(), ()],
-                         ('status', headers, chunks))
+        self.assertEqual(storage.data['url'][()],
+                         (0, 'status', headers, chunks, {}))
         self.assertEqual(lock.acquired, 1)
         self.assertEqual(lock.released, 1)
 
@@ -49,14 +49,14 @@
         storage.data['url'] = {}
         storage.data['url'][(), ()] = ('otherstatus', (), ())
         headers = [('Header1', 'value1')]
-        handler = storage.store('url', 'status', headers, [], [])
+        handler = storage.store('url', (), 0, 'status', headers)
         self.failIf(handler is None)
         chunks = ['chunk1', 'chunk2']
         for chunk in ('chunk1', 'chunk2'):
             handler.write(chunk)
         handler.close()
-        self.assertEqual(storage.data['url'][(), ()],
-                         ('status', headers, chunks))
+        self.assertEqual(storage.data['url'][()],
+                         (0, 'status', headers, chunks, {}))
         self.assertEqual(lock.acquired, 1)
         self.assertEqual(lock.released, 1)
 
@@ -69,14 +69,20 @@
         lock = DummyLock()
         storage = self._makeOne(lock)
         storage.data['url'] = {
-            (1, 2):(200, [], []),
-            (3, 4):(203, [], [])
+            ('env', (1, 2)):(0, 200, [], [], {}),
+            ('env', (3, 4)):(0, 203, [], [], {}),
             }
         result = storage.fetch('url')
         result.sort()
         self.assertEqual(len(result), 2)
-        self.assertEqual(result[0], (200, [], [], 1, 2))
-        self.assertEqual(result[1], (203, [], [], 3, 4))
+        self.assertEqual(
+            result[0],
+            (('env', (1,2)), 0, 200, [], [], {})
+            )
+        self.assertEqual(
+            result[1],
+            (('env', (3,4)), 0, 203, [], [], {})
+            )
 
     def test_storage_factory_defaults(self):
         from repoze.accelerator.storage import make_memory_storage


More information about the Repoze-checkins mailing list