decouple cache buster from top-level asset spec

[mmerickel]

Currently the cache buster api is coupled to the asset spec defined in config.add_static_view('myapp:static', '/static', cachebust=CacheBuster()). This does not play well with config.override_assets(to_override='myapp:static/theme/', override_with='mytheme:static/') (a desirable pattern for static views). The issue is that some assets are pulled in from other packages which are not at myapp:static and thus cannot be properly cache busted without knowing what is overridden and what is not.

1. The ICacheBuster interface receives no information about the fact that this asset is overridden. The spec it received is myapp:static, never mytheme:static.
2. The 10-year cache expiration time is applied whether or not the asset is actually cache busted.

This should be decoupled such that raw paths can be busted. I'm proposing config.add_cache_buster('mytheme:static/', cache_buster, cache_max_age).

Two problems with this API that we'll need to solve:

1. In multiple calls to add_cache_buster which cache buster is used on the matching side when a URL comes into the system?
2. For assets that match a cache busted pattern, how do we tell whether the asset is actually cache busted or not? Should we enhance ICacheBuster.pregenerate to return some info about whether it is busted? Check whether the parameters are mutated?

ping @bertjwregeer, @dstufft

[digitalresistor]

I was going to work on this over the weekend, but I am stuck on number 1 that you've listed. Do we go in reverse order of registration, so that later cache busters override earlier ones? The cache buster doesn't really know for what path it was registered and whether it originally "busted" the URL or not.

[mmerickel]

Yeah later defs should override newer ones but two cache busters with the same path should conflict. Ideally also if you define a buster on a specific subpath/file and then add a newer buster covering a superset of those paths it should also conflict. That's not directly covered by the conflict api though so it would need to be done manually in the action.

[digitalresistor]

I was planning on working on this, but then spent the next two hours thinking about it before hitting up @mmerickel on IRC and coming to the following list of issues:

• Creating the URL that is cache busted is simple, the reverse however is not true
• Static view knows about the asset spec, but cache busters may be registered for longer or even single file asset specs
• Finding the reverse of /static/some/file/asset-busted.png is fairly simple, but current cache buster allows the cache buster to really mess with the URL, and /static/cachebustingtoken/some/file/asset.png is not easily unbusted.
• static_view only works on folders (and anything in that folder is served), however we want to allow overriding a single asset making things more complicated: for example static view for myapp:static/ but asset override and cache busting for myapp:static/logo.png, and we'd thus need to be able to take a cache busted URL and find the appropriate cachebuster for it. [1]

[1]: Single file override already works as is, it is adding the cachebusting that is an issue.

[mmerickel]

After talking to @dstufft in IRC I think we came to a conclusion that should work well. We can simply remove the match side of the cache buster and only support generating urls. I think this leads to a more consistent api with static views as they:

1. Do not always host the assets themselves anyway as add_static_view may point to an external host.
2. It's easy to override the matching side with custom tweens/middlewares/notfound views already if absolutely necessary.
3. Most people who are cache busting are actually naming the file on disk with the token in it or using query strings - so un-busting the file is unnecessary.

[digitalresistor]

@mmerickel Ok.

[mmerickel]

Progress update:

import os.path
import posixpath

from pyramid.config import Configurator

here = os.path.dirname(os.path.abspath(__file__))

def cachebust(token):
    def cachebust(spec, subpath, kw):
        print('spec=%s' % (spec,))
        print('subpath=%s' % (subpath,))
        fname, ext = posixpath.splitext(subpath)
        return fname + token + ext, kw
    return cachebust

def main(global_config, **app_settings):
    config = Configurator()
    config.add_static_view('static', 'myapp:static')
    config.override_asset('myapp:static/theme/', here)
    config.override_asset('myapp:static/a.png', 'theme:static/a-override.png')
    config.override_asset('myapp:static/c.png', 'theme:static/c-override.png')

    config.add_cache_buster('theme:static', cachebust('-1111'))
    config.add_cache_buster(here, cachebust('-2222'))
    config.add_cache_buster('myapp:static', cachebust('-3333'))

    config.add_view(some_view)
    return config.make_wsgi_app()

def some_view(request):
    print(request.static_url('myapp:static/a.png'))
    print(request.static_url('myapp:static/images/b.png'))
    print(request.static_url('myapp:static/theme/c.png'))
    print(request.static_url('myapp:static/d.png'))
    return request.response

if __name__ == '__main__':
    from waitress import serve
    app = main({})
    serve(app, host='127.0.0.1', port=8080)

playground❯ env/bin/python play.py
serving on http://127.0.0.1:8080
spec=myapp:static/a.png
subpath=a.png
http://localhost:8080/static/a-1111.png
spec=myapp:static/images/b.png
subpath=images/b.png
http://localhost:8080/static/images/b-3333.png
spec=myapp:static/theme/c.png
subpath=theme/c.png
http://localhost:8080/static/theme/c-2222.png
spec=myapp:static/d.png
subpath=d.png
http://localhost:8080/static/d-3333.png

[mmerickel]

Supporting absolute path overrides is a little bit of a quirk. I'm going back and forth right now but I think I will need to require a trailing / when adding a cache buster for a folder in order to avoid applying a cache buster registered for /foo to some other asset /foobar/something.png.