Try something a little more decoupled and consistent.

4 files changed, 112 insertions, 88 deletions

diff --git a/pyramid/cachebust.py b/pyramid/cachebust.py
deleted file mode 100644
index 69c7eb1d2..000000000
--- a/pyramid/cachebust.py
+++ /dev/null
@@ -1,34 +0,0 @@
-import hashlib
-import pkg_resources
-
-from zope.interface import implementer
-
-from .interfaces import ICacheBuster
-
-from pyramid.asset import resolve_asset_spec
-
-
-def generate_md5(spec):
-    package, filename = resolve_asset_spec(spec)
-    md5 = hashlib.md5()
-    with pkg_resources.resource_stream(package, filename) as stream:
-        for block in iter(lambda: stream.read(4096), ''):
-            md5.update(block)
-    return md5.hexdigest()
-
-
-@implementer(ICacheBuster)
-class DefaultCacheBuster(object):
-
-    def generate_token(self, request, pathspec):
-        token_cache = request.registry.setdefault('md5-token-cache', {})
-        token = token_cache.get(pathspec)
-        if not token:
-            token_cache[pathspec] = token = generate_md5(pathspec)
-        return token
-
-    def pregenerate_url(self, request, token, subpath, kw):
-        return token + '/' + subpath, kw
-
-    def match_url(self, request, path_elements):
-        return path_elements[1:]
diff --git a/pyramid/config/views.py b/pyramid/config/views.py
index 78c415b14..c09ddc73d 100644
--- a/pyramid/config/views.py
+++ b/pyramid/config/views.py
@@ -1,6 +1,8 @@
+import hashlib
 import inspect
 import operator
 import os
+import pkg_resources
 import warnings
 
 from zope.interface import (
@@ -34,7 +36,7 @@ from pyramid.interfaces import (
     )
 
 from pyramid import renderers
-from pyramid.cachebust import DefaultCacheBuster
+from pyramid.asset import resolve_asset_spec
 
 from pyramid.compat import (
     string_types,
@@ -45,11 +47,6 @@ from pyramid.compat import (
     is_nonstr_iter
     )
 
-from pyramid.encode import (
-    quote_plus,
-    urlencode,
-)
-
 from pyramid.exceptions import (
     ConfigurationError,
     PredicateMismatch,
@@ -1907,15 +1904,13 @@ class StaticURLInfo(object):
         except AttributeError: # bw compat (for tests)
             registry = get_current_registry()
         registrations = self._get_registrations(registry)
-        for (url, spec, route_name, cachebust) in registrations:
+        for (url, spec, route_name, cachebuster) in registrations:
             if path.startswith(spec):
                 subpath = path[len(spec):]
                 if WIN: # pragma: no cover
                     subpath = subpath.replace('\\', '/') # windows
-                if cachebust:
-                    token = cachebust.generate_token(request, spec + subpath)
-                    subpath, kw = cachebust.pregenerate_url(
-                        request, token, subpath, kw)
+                if cachebuster:
+                    subpath, kw = cachebuster(subpath, kw)
                 if url is None:
                     kw['subpath'] = subpath
                     return request.route_url(route_name, **kw)
@@ -1955,9 +1950,22 @@ class StaticURLInfo(object):
             # make sure it ends with a slash
             name = name + '/'
 
-        cachebust = extra.pop('cachebust', None)
-        if cachebust is True:
-            cachebust = DefaultCacheBuster()
+        cb = extra.pop('cachebust', None)
+        if cb is True:
+            cb_token, cb_pregen, cb_match = DefaultCacheBuster()
+        elif cb:
+            cb_token, cb_pregen, cb_match = cb
+        else:
+            cb_token = cb_pregen = cb_match = None
+
+        if cb_token and cb_pregen:
+            def cachebuster(subpath, kw):
+                token = cb_token(spec + subpath)
+                subpath_tuple = tuple(subpath.split('/'))
+                subpath_tuple, kw = cb_pregen(token, subpath_tuple, kw)
+                return '/'.join(subpath_tuple), kw
+        else:
+            cachebuster = None
 
         if url_parse(name).netloc:
             # it's a URL
@@ -1968,12 +1976,12 @@ class StaticURLInfo(object):
             # it's a view name
             url = None
             cache_max_age = extra.pop('cache_max_age', None)
-            if cache_max_age is None and cachebust:
+            if cache_max_age is None and cb:
                 cache_max_age = 10 * 365 * 24 * 60 * 60  # Ten(ish) years
 
             # create a view
             view = static_view(spec, cache_max_age=cache_max_age,
-                               use_subpath=True, cachebust=cachebust)
+                               use_subpath=True, cachebust_match=cb_match)
 
             # Mutate extra to allow factory, etc to be passed through here.
             # Treat permission specially because we'd like to default to
@@ -2014,7 +2022,7 @@ class StaticURLInfo(object):
                 registrations.pop(idx)
 
             # url, spec, route_name
-            registrations.append((url, spec, route_name, cachebust))
+            registrations.append((url, spec, route_name, cachebuster))
 
         intr = config.introspectable('static views',
                                      name,
@@ -2026,3 +2034,40 @@ class StaticURLInfo(object):
         config.action(None, callable=register, introspectables=(intr,))
 
 
+def _generate_md5(spec):
+    package, filename = resolve_asset_spec(spec)
+    md5 = hashlib.md5()
+    with pkg_resources.resource_stream(package, filename) as stream:
+        for block in iter(lambda: stream.read(4096), ''):
+            md5.update(block)
+    return md5.hexdigest()
+
+
+def DefaultCacheBuster():
+    token_cache = {}
+
+    def generate_token(pathspec):
+        # An astute observer will notice that this use of token_cache doesn't
+        # look particular thread safe.  Basic read/write operations on Python
+        # dicts, however, are atomic, so simply accessing and writing values
+        # to the dict shouldn't cause a segfault or other catastrophic failure.
+        # (See: http://effbot.org/pyfaq/what-kinds-of-global-value-mutation-are-thread-safe.htm)
+        #
+        # We do have a race condition that could result in the same md5
+        # checksum getting computed twice or more times in parallel.  Since
+        # the program would still function just fine if this were to occur,
+        # the extra overhead of using locks to serialize access to the dict
+        # seems an unnecessary burden.
+        #
+        token = token_cache.get(pathspec)
+        if not token:
+            token_cache[pathspec] = token = _generate_md5(pathspec)
+        return token
+
+    def pregenerate_url(token, subpath, kw):
+        return (token,) + subpath, kw
+
+    def match_url(subpath):
+        return subpath[1:]
+
+    return (generate_token, pregenerate_url, match_url)
diff --git a/pyramid/interfaces.py b/pyramid/interfaces.py
index e60898dbc..84a6ad833 100644
--- a/pyramid/interfaces.py
+++ b/pyramid/interfaces.py
@@ -1164,47 +1164,60 @@ class IJSONAdapter(Interface):
 class IPredicateList(Interface):
     """ Interface representing a predicate list """
 
-class ICacheBuster(Interface):
-    """
-    An instance of a class which implements this interface may be passed as the
-    ``cachebust`` argument to
-    :meth:`pyramid.config.Configurator.add_static_view` to add cache busting
-    capability to a static view.
-    """
-    def generate_token(request, pathspec):
+class ICachebustTokenGenerator(Interface):
+    def __call__(pathspec):
         """
-        Return a token string for a static asset to be used to rewrite a
-        static asset URL for cache busting.
+        A function which computes and returns a token string used for cache
+        busting.  ``pathspec`` is the path specification for the resource to be
+        cache busted.  Often a cachebust token might be computed for a specific
+        asset (e.g. an md5 checksum), but probably just as often people use
+        schemes where a single cachebust token is used globally.  It could be a
+        git commit sha1, a timestamp, or something configured manually. A
+        pattern that can be useful is to use to a factory function and a
+        closure to return a function that depends on some configuration.  For
+        example:
 
-        The ``pathspec`` argument is the path specification for the asset we're
-        generating a token for.
+        .. code-block:: python
+           :linenos:
+
+           def use_configured_cachebust_token(config):
+               # config is an instance of pyramid.config.Configurator
+               token = config.registry.settings['myapp.cachebust_token']
+               def cachebust_token(pathspec):
+                   return token
+               return cachebust_token
         """
 
-    def pregenerate_url(request, token, subpath, kw):
+class ICachebustURLPregenerator(Interface):
+    def __call__(token, subpath, kw):
         """
-        Modifies the elements and/or keywords used to generate the URL for a
-        given static asset.
-
-        The ``token`` argument is the result of calling
-        :meth:`~pyramid.interfaces.ICacheBuster.generate_token` for a static
-        asset.
-
-        The ``subpath`` argument is the subpath in the static asset URL that
-        would normally be generated without cache busting.  The ``kw``
-        argument is the keywords dict that would be passed to
-        :meth:`~pyramid.request.Request.route_url`.
-        The return value should be a two-tuple of elements ``(subpath, kw)``
-        which are modified from the incoming arguments.
+        A function which modifies a subpath and/or keyword arguments from which
+        a static asset URL will be computed during URL generation.  The
+        ``token`` argument is a token string computed by an instance of
+        :class:`~pyramid.interfaces.ICachebustTokenGenerator` for a particular
+        asset.  The ``subpath`` argument is a tuple of path elements that
+        represent the portion of the asset URL which is used to find the asset.
+        The ``kw`` argument is a dict of keywords that are to be passed
+        eventually to :meth:`~pyramid.request.Request.route_url` for URL
+        generation.  The return value of this function should be two-tuple of
+        ``(subpath, kw)`` which are versions of the same arguments modified to
+        include the cachebust token in the generated URL.
         """
 
-    def match_url(request, path_elements):
+class ICachebustURLMatcher(Interface):
+    def __call__(subpath):
         """
-        Undo any modification to the subpath which may have been done by
-        :meth:`~pyramid.interfaces.ICacheBuster.pregenerate_url`.  The
-        ``path_elements`` argument is a tuple of path elements that represent
-        the subpath of the asset request URL.  The return value should be
-        a modified (or not) version of ``path_elements``, which will be used
-        ultimately to find the asset.
+        A function which performs the logical inverse of an
+        :class:`~pyramid.interfaces.ICacheBustURLPregenerator`, by taking a
+        subpath from a cache busted URL and removing the cachebust token, so
+        that :app:`Pyramid` can find the underlying asset.  If the cache
+        busting scheme in use doesn't specifically modify the path portion of
+        the generated URL (e.g. it adds a query string), a function which
+        implements this interface may not be necessary.
+
+        ``subpath`` is the subpath portion of the URL for an incoming request
+        for a static asset.  The return value should be the same tuple with the
+        cache busting token elided.
         """
 
 # configuration phases: a lower phase number means the actions associated
diff --git a/pyramid/static.py b/pyramid/static.py
index be191971a..87bbcd34c 100644
--- a/pyramid/static.py
+++ b/pyramid/static.py
@@ -78,7 +78,7 @@ class static_view(object):
     """
 
     def __init__(self, root_dir, cache_max_age=3600, package_name=None,
-                 use_subpath=False, index='index.html', cachebust=None):
+                 use_subpath=False, index='index.html', cachebust_match=None):
         # package_name is for bw compat; it is preferred to pass in a
         # package-relative path as root_dir
         # (e.g. ``anotherpackage:foo/static``).
@@ -91,15 +91,15 @@ class static_view(object):
         self.docroot = docroot
         self.norm_docroot = normcase(normpath(docroot))
         self.index = index
-        self.cachebust = cachebust
+        self.cachebust_match = cachebust_match
 
     def __call__(self, context, request):
         if self.use_subpath:
             path_tuple = request.subpath
         else:
             path_tuple = traversal_path_info(request.environ['PATH_INFO'])
-        if self.cachebust:
-            path_tuple = self.cachebust.match_url(request, path_tuple)
+        if self.cachebust_match:
+            path_tuple = self.cachebust_match(path_tuple)
         path = _secure_path(path_tuple)
 
         if path is None: