Changeset 1511

Timestamp:

03/28/08 08:51:04 (2 years ago)

Author:

JensDiemer

Message:

Updating the cache middleware:

• Splitting pagestats from cache is a good idea, after all ;)
• add a debug information in the response
• verify the shortcut from the url
• update unittests

Location:

trunk/pylucid

Files:

• PyLucid/middlewares/cache.py (modified) (8 diffs)
• PyLucid/middlewares/pagestats.py (added)
• PyLucid/settings_example.py (modified) (2 diffs)
• PyLucid/tools/shortcuts.py (modified) (1 diff)
• tests/cache.py (modified) (3 diffs)
• tests/security.py (modified) (4 diffs)

trunk/pylucid/PyLucid/middlewares/cache.py

@@ -12 +12 @@
         - Build the cache key based on the page shortcuts
 
-    If request.debug == True: We added set response["from_cache"] = "yes" if
-    the response comes from the cache.
+    If request.debug == True: We added added information about the cache status
+    in _insert_cache_info():
+        - set response["from_cache"] to "yes" / "no" (Not True/False!)
+        - Replace the pagestats TAG with cache debug information.
 
     The page statistics part bases on:
@@ -35 +37 @@
 from django.core.cache import cache
 
+from PyLucid.models import Page
+from PyLucid.tools.shortcuts import verify_shortcut
+from PyLucid.middlewares.pagestats import TAG
 from PyLucid.template_addons.filters import human_duration
-from PyLucid.models import Page
 
 CACHE_TIMEOUT = settings.CACHE_MIDDLEWARE_SECONDS
 
-# Save the start time of the current running pyhon instance
-start_overall = time.time()
-
-TAG = u"<!-- script_duration -->"
-
-FMT = (
-    u'render time: %(total_time)s -'
-    ' overall: %(overall_time)s -'
-    ' queries: %(queries)d'
-)
+CACHED_INFO_YES = u"from cache"
+CACHED_INFO_NO = u"not cached"
 
 # RFC1123 date format as specified by HTTP RFC2616 section 3.3.1:
@@ -70 +66 @@
         shortcut = url.rsplit("/", 1)[-1]
 
+    # Check a shortcut. A AssertionError whould be raised if something seems to
+    # be wrong.
+    # Normaly the url-re of the index view filters bad things out. But
+    # process_request in the middleware whould becalled before this done.
+    verify_shortcut(shortcut)
+
     cache_key = settings.PAGE_CACHE_PREFIX + shortcut
     return shortcut, cache_key
@@ -75 +77 @@
 
 class CacheMiddleware(object):
+    def __init__(self):
+        self.cache_key = None
+
     def process_request(self, request):
         """
@@ -81 +86 @@
         user makes a GET or HEAD requests.
         """
-        # save start time and the number of db queries before we do anything
-        self.start_time = time.time()
-        self.old_queries = len(connection.queries)
+        request._from_cache = False
 
         # cache only GET or HEAD requests
@@ -99 +102 @@
         # Build the cache key based on the page shortcuts
         url = request.path
-        shortcut, self.cache_key = build_cache_key(url)
+        try:
+            shortcut, self.cache_key = build_cache_key(url)
+        except AssertionError, e:
+            # Something is wrong with the given url
+            request._use_cache = False
+            return
 
         # Get the page data from the cache. If not exist response is None.
         response = cache.get(self.cache_key)
 
-        if response:
-            # The page data exist in the cache
-            assert isinstance(response, HttpResponse)
-            if request.debug:
-                #print "Use cached page version. (key: '%s')" % self.cache_key
-                response["from_cache"] = "yes"
-            self.insert_page_stats(request, response)
-        #elif settings.DEBUG:
-            #print "Page not in cache found. (key: '%s')" % self.cache_key
+        if response == None:
+            # The page data doesn't exist in the cache
+            return
+
+        # The page data exist in the cache
+        assert isinstance(response, HttpResponse)
+        request._from_cache = True
+
+        if request.debug:
+            # Add the cache debug information.
+            response = self._insert_cache_info(request, response, True)
 
         return response
@@ -121 +131 @@
         Cache the response and insert the page statistics.
         """
+        if request._from_cache == True:
+            # The content comes from the cache
+            return response
+
         if getattr(request, "_use_cache", False) != True:
             # Don't cache
-            self.insert_page_stats(request, response)
+            response = self._insert_cache_info(request, response, False)
             return response
 
         # cache the response
-        self.cache_response(request, response)
-
-        self.insert_page_stats(request, response)
+        self._cache_response(request, response)
+
+        response = self._insert_cache_info(request, response, False)
 
         return response
 
 
-    def cache_response(self, request, response):
-        """
-        Cache the given response.
-        """
-        # Add cache info headers to the response object
-        self.patch_response_headers(request, response)
-
-        # Save the page into the cache
-        cache.set(self.cache_key, response, CACHE_TIMEOUT)
-
-
-    def patch_response_headers(self, request, response):
-        """
-        Adds some useful response headers for the browser cache to the given
-        HttpResponse object.
-        Based on django.utils.cache.patch_response_headers() but here we use
-        the original page last update time.
-        """
-        # The the original page last update time
-        context = request.CONTEXT
-        current_page_obj = context["PAGE"]
-        lastupdatetime = current_page_obj.lastupdatetime
-
-        response['ETag'] = md5.new(self.cache_key).hexdigest()
-        response['Last-Modified'] = lastupdatetime.strftime(DATE_FORMAT)
-        now = datetime.datetime.utcnow()
-        expires = now + datetime.timedelta(0, CACHE_TIMEOUT)
-        response['Expires'] = expires.strftime(DATE_FORMAT)
-
-
-    def insert_page_stats(self, request, response):
-        """
-        calculate the statistic and replace it into the html page.
-        """
-        # Put only the statistic into HTML pages
-        if not "html" in response._headers["content-type"][1]:
-            # No HTML Page -> do nothing
-            return response
-
-        # compute the db time for the queries just run
-        # FIXME: In my shared webhosting environment the queries is always = 0
-        queries = len(connection.queries) - self.old_queries
-
-        total_time = human_duration(time.time() - self.start_time)
-        overall_time = human_duration(time.time() - start_overall)
-
-        # replace the comment if found
-        stat_info = FMT % {
-            'total_time' : total_time,
-            'overall_time' : overall_time,
-            'queries' : queries,
-        }
+    def _insert_cache_info(self, request, response, is_from_cache):
+        """
+        Add the cache debug information.
+
+        if request.debug == True, we added the information if the response
+        was from the cache or not. We "added" the information after the
+        pagestats >TAG< ;)
+        """
+        if request._from_cache == True:
+            response_msg = CACHED_INFO_YES
+            response["from_cache"] = "yes"
+        elif request._from_cache == False:
+            response_msg = CACHED_INFO_NO
+            response["from_cache"] = "no"
+        else:
+            raise AssertionError("wrong request._from_cache info.")
 
         content = response.content
@@ -197 +175 @@
                 return response
 
-        # insert the page statistic
-        new_content = content.replace(TAG, stat_info)
+        # Replace the pagestats TAG with the cache debug information.
+        message = u"%s - %s - cache key: %s" % (
+            TAG, response_msg, self.cache_key
+        )
+        new_content = content.replace(TAG, message)
         response.content = new_content
 
         return response
+
+
+    def _cache_response(self, request, response):
+        """
+        Cache the given response.
+        """
+        # Add cache info headers to the response object
+        self._patch_response_headers(request, response)
+
+        # Save the page into the cache
+        cache.set(self.cache_key, response, CACHE_TIMEOUT)
+
+
+    def _patch_response_headers(self, request, response):
+        """
+        Adds some useful response headers for the browser cache to the given
+        HttpResponse object.
+        Based on django.utils.cache.patch_response_headers() but here we use
+        the original page last update time.
+        """
+        # The the original page last update time
+        context = request.CONTEXT
+        current_page_obj = context["PAGE"]
+        lastupdatetime = current_page_obj.lastupdatetime
+
+        response['ETag'] = md5.new(self.cache_key).hexdigest()
+        response['Last-Modified'] = lastupdatetime.strftime(DATE_FORMAT)
+        now = datetime.datetime.utcnow()
+        expires = now + datetime.timedelta(0, CACHE_TIMEOUT)
+        response['Expires'] = expires.strftime(DATE_FORMAT)

trunk/pylucid/PyLucid/settings_example.py

@@ -84 +84 @@
 #
 MIDDLEWARE_CLASSES = (
+    # Insert a statistic line into the generated page:
+    'PyLucid.middlewares.pagestats.PageStatsMiddleware',
+
     # PyLucidCommonMiddleware loads the django middlewares:
     #    - 'django.contrib.sessions.middleware.SessionMiddleware'
@@ -90 +93 @@
     'PyLucid.middlewares.common.PyLucidCommonMiddleware',
 
+    # Cache all anonymous cms page request, if CACHE_BACKEND worked.
     'PyLucid.middlewares.cache.CacheMiddleware',
 

trunk/pylucid/PyLucid/tools/shortcuts.py

@@ -19 +19 @@
 ALLOW_CHARS = string.ascii_letters + string.digits + "_"
 
+def verify_shortcut(shortcut):
+    """
+    Check a shortcut. Raise AssertionError if something seems to be wrong.
+    But normaly the urls-re should only filter the bad thing from urls ;)
+    """
+    if shortcut=="":
+        raise AssertionError("Shortcut is empty!")
+
+    for char in shortcut:
+        if not char in ALLOW_CHARS:
+            raise AssertionError(
+                "Not allowed character in shortcut: '%r'" % char
+            )
 
 def makeUnique(item_name, name_list):

trunk/pylucid/tests/cache.py

@@ -135 +135 @@
             must_not_contain=("Traceback",)
         )
-
-        from_cache = response.get("from_cache", None)
-        self.failUnlessEqual(from_cache, None, msg)
+        self.failUnlessEqual(response["from_cache"], "no", msg)
 
     def failIfNotFromCache(self, response, msg=None):
@@ -157 +155 @@
             raise AssertionError("Wrong url?")
 
-        from_cache = response["from_cache"]
-        self.failUnlessEqual(from_cache, "yes", msg)
+        self.failUnlessEqual(response["from_cache"], "yes", msg)
 
     #--------------------------------------------------------------------------
@@ -314 +311 @@
         self._check_cache_after(test_func)
 
+    #--------------------------------------------------------------------------
+    
+    def test_wrong_url(self):
+        """
+        Test if the cache middleware checks bad characters in the url.
+        Normaly the urls-re doesn't match on bad urls. But the process_request
+        method of a middleware called before this.
+        """
+        content = "Should never comes from the cache!"
+        shortcut = "bad$char&here"
+        url = "/%s/" % shortcut
+        cache_key = settings.PAGE_CACHE_PREFIX + shortcut
+        
+        # Insert a cache entry, so the cache middleware can found the cached
+        # page and will be response the cached content, if the shortcut will
+        # not be veryfied.
+        response = HttpResponse(content)
+        cache.set(cache_key, response, 9999)
+        
+        # Request the cached page and check the response. It should not resonse
+        # the cached content. It should be apperar a 404 page not found.
+        response = self.client.get(url)
+        #debug_response(response)
+        self.failUnlessEqual(response.status_code, 404)
+        self.assertResponse(response, must_not_contain=(content,))
+
 
 if __name__ == "__main__":

trunk/pylucid/tests/security.py

@@ -37 +37 @@
 
 CHARACTER_VARIANTS = (
-    "<", "\x3c", "\x3C", u"\u003c", u"\u003C",
+    "<", r"\x3c", r"\x3C", r"\u003c", u"\u003c", u"\u003C",
 #    "&#60", "&#060", "&#0060", "&#00060", "&#000060", "&#0000060", "&#60;",
 #    "&#060;", "&#0060;", "&#00060;", "&#000060;", "&#0000060;",
@@ -143 +143 @@
         for char_variant in CHARACTER_VARIANTS:
             test_string = char_variant + VARIANTS_STRING
-            response = self.client.get("/" + test_string)
+            url = "/" + test_string
+            response = self.client.get(url)
             self.assertResponse(
                 response, must_not_contain=(test_string, "<" + VARIANTS_STRING,)
@@ -167 +168 @@
                     self.assertResponse(
                         response,
-                        must_not_contain=(test_string, "<" + VARIANTS_STRING,)
+                        must_not_contain=("<" + VARIANTS_STRING,)
                     )
 
@@ -184 +185 @@
                 self.assertResponse(
                     response,
-                    must_not_contain=(test_string, "<" + VARIANTS_STRING,)
+                    must_not_contain=("<" + VARIANTS_STRING,)
                 )