Fixes #3401 - Unbreak bust_cache

[miketaylr]

We unintentionally regressed behavior for non-LOCALHOST envs in #3355

This is sort of a bandaid to get it working - reverting a line in bust_cache and getting the one failing test to pass.

If we think the previous absolute path behavior is better, we need to update staging and production STATIC_PATH and do some testing, I think.

[miketaylr]

r? @karlcow

[miketaylr]

(I was going to deploy this to staging, but it appears I can't reach the server :S -- others can see it, so possibly a regional thing)

edit: ok, back up for me. weird.

[miketaylr]

checksums are back on staging after deploying this branch.

[karlcow]

added context on #3401 (comment)

[karlcow]

@miketaylr some analysis and suggestion for this PR.

And thanks for spotting this. missing_file worked in some ways :) by making us noticing it.

[karlcow]

OK let's see, to test locally I did:

    # if app.config['LOCALHOST']:
    #     return file_path
    absolute_path = os.path.join(STATIC_PATH, file_path)
    print('-----EXHIBIT A')
    print('STATIC_PATH: ', STATIC_PATH)
    print('file_path: ', file_path)
    print('absolute_path: ', absolute_path)
    print('CHECKSUM: ', get_checksum(absolute_path))
    print('-----EXHIBIT B')
    print('STATIC_PATH: ', STATIC_PATH)
    print('file_path: ', file_path)
    print('static + file_path: ', STATIC_PATH + file_path)
    print('CHECKSUM: ', get_checksum(STATIC_PATH + file_path))
    return file_path + '?' + get_checksum(absolute_path)

And it gives me

-----EXHIBIT A
STATIC_PATH:  /Users/karl/code/webcompat.com-karlcow/webcompat/static
file_path:  /dist/index.js
absolute_path:  /dist/index.js
GET_CHECKSUM {'/dist/vendor.js': 'missing_file', '/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/vendor.js': '0a12387d091507afc835d02c48807bdb', '/dist/webcompat.js': 'missing_file', '/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/webcompat.js': '23003fc31f870e799cb91d79906e56ab', '/dist/index.js': 'missing_file', '/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/index.js': '28732cb8ce2520ea383aa149f5558d9c'}
CHECKSUM:  missing_file

-----EXHIBIT B
STATIC_PATH:  /Users/karl/code/webcompat.com-karlcow/webcompat/static
file_path:  /dist/index.js
static + file_path:  /Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/index.js
GET_CHECKSUM {'/dist/vendor.js': 'missing_file', '/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/vendor.js': '0a12387d091507afc835d02c48807bdb', '/dist/webcompat.js': 'missing_file', '/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/webcompat.js': '23003fc31f870e799cb91d79906e56ab', '/dist/index.js': 'missing_file', '/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/index.js': '28732cb8ce2520ea383aa149f5558d9c'}
CHECKSUM:  28732cb8ce2520ea383aa149f5558d9c
GET_CHECKSUM {'/dist/vendor.js': 'missing_file', '/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/vendor.js': '0a12387d091507afc835d02c48807bdb', '/dist/webcompat.js': 'missing_file', '/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/webcompat.js': '23003fc31f870e799cb91d79906e56ab', '/dist/index.js': 'missing_file', '/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/index.js': '28732cb8ce2520ea383aa149f5558d9c'}

ah understood. because the two string starts with /, os.path.join() think it's two absolute paths, so this is logical and this can be fixed.

>>> import os.path
>>> os.path.join('/root/blah', '/somewhere/booh')
'/somewhere/booh'
>>> os.path.join('/root/blah', 'somewhere/booh')
'/root/blah/somewhere/booh'
>>> os.path.join('/root/blah', '/somewhere/booh'[1:])
'/root/blah/somewhere/booh'

This can be fixed.

Let's try this. Now EXHIBIT A should work, and EXHIBIT B should fail.

    # if app.config['LOCALHOST']:
    #     return file_path
    if file_path.startswith('/'):
        file_path = file_path[1:]
    absolute_path = os.path.join(STATIC_PATH, file_path)

ok. Good.

-----EXHIBIT A
STATIC_PATH:  /Users/karl/code/webcompat.com-karlcow/webcompat/static
file_path:  dist/index.js
absolute_path:  /Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/index.js
GET_CHECKSUM {'/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/vendor.js': '0a12387d091507afc835d02c48807bdb', '/Users/karl/code/webcompat.com-karlcow/webcompat/staticdist/vendor.js': 'missing_file', '/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/webcompat.js': '23003fc31f870e799cb91d79906e56ab', '/Users/karl/code/webcompat.com-karlcow/webcompat/staticdist/webcompat.js': 'missing_file', '/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/index.js': '28732cb8ce2520ea383aa149f5558d9c', '/Users/karl/code/webcompat.com-karlcow/webcompat/staticdist/index.js': 'missing_file'}
CHECKSUM:  28732cb8ce2520ea383aa149f5558d9c
-----EXHIBIT B
STATIC_PATH:  /Users/karl/code/webcompat.com-karlcow/webcompat/static
file_path:  dist/index.js
static + file_path:  /Users/karl/code/webcompat.com-karlcow/webcompat/staticdist/index.js
GET_CHECKSUM {'/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/vendor.js': '0a12387d091507afc835d02c48807bdb', '/Users/karl/code/webcompat.com-karlcow/webcompat/staticdist/vendor.js': 'missing_file', '/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/webcompat.js': '23003fc31f870e799cb91d79906e56ab', '/Users/karl/code/webcompat.com-karlcow/webcompat/staticdist/webcompat.js': 'missing_file', '/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/index.js': '28732cb8ce2520ea383aa149f5558d9c', '/Users/karl/code/webcompat.com-karlcow/webcompat/staticdist/index.js': 'missing_file'}
CHECKSUM:  missing_file
GET_CHECKSUM {'/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/vendor.js': '0a12387d091507afc835d02c48807bdb', '/Users/karl/code/webcompat.com-karlcow/webcompat/staticdist/vendor.js': 'missing_file', '/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/webcompat.js': '23003fc31f870e799cb91d79906e56ab', '/Users/karl/code/webcompat.com-karlcow/webcompat/staticdist/webcompat.js': 'missing_file', '/Users/karl/code/webcompat.com-karlcow/webcompat/static/dist/index.js': '28732cb8ce2520ea383aa149f5558d9c', '/Users/karl/code/webcompat.com-karlcow/webcompat/staticdist/index.js': 'missing_file'}

So my suggestion is to do:

@app.template_filter('bust_cache')
def bust_cache(file_path):
    """Jinja2 filter to add a cache busting param based on md5 checksum.

    Uses a simple cache_dict to we don't have to hash each file for every
    request. This is kept in-memory so it will be blown away when the app
    is restarted (which is when file changes would have been deployed).
    Doesn't return hash on development
    """
    if app.config['LOCALHOST']:
        return file_path
    if file_path.startswith('/'):
        file_path = file_path[1:]
    absolute_path = os.path.join(STATIC_PATH, file_path)
    return file_path + '?' + get_checksum(absolute_path)

which will make fail one test to fix in my previous code.

_______________________________________________________ test_bust_cache_production_missing_file _______________________________________________________

    def test_bust_cache_production_missing_file():
        """Test for cache_bust the path is missing."""
        expected = '/punkcat/nowhere?missing_file'
        webcompat.app.config['LOCALHOST'] = None
>       assert bust_cache('/punkcat/nowhere') == expected
E       AssertionError: assert 'punkcat/nowhere?missing_file' == '/punkcat/now...?missing_file'
E         - punkcat/nowhere?missing_file
E         + /punkcat/nowhere?missing_file
E         ? +

tests/unit/test_template.py:144: AssertionError
---------------------------------------------------------------- Captured stdout call -----------------------------------------------------------------
GET_CHECKSUM {'/Users/karl/code/webcompat.com-karlcow/tmp/pytest-of-karl/pytest-130/test_get_checksum_not_in_cache0/space.js': '501753e94c8bfbbbd53c792c9688c8b5'}

but that's an easy fix too.

[miketaylr]

OK, r? @karlcow

[karlcow]

Cool! Thanks.