Skip to content

Replace per-request timestamp cache-busting with file mtime versioning for media URLs#1706

Draft
Copilot wants to merge 5 commits intomasterfrom
copilot/alternative-cache-busting-strategy
Draft

Replace per-request timestamp cache-busting with file mtime versioning for media URLs#1706
Copilot wants to merge 5 commits intomasterfrom
copilot/alternative-cache-busting-strategy

Conversation

Copy link
Contributor

Copilot AI commented Mar 12, 2026

?v={% now 'YmdHis' %} appended to every video/image URL generated a new cache key on every page load, effectively disabling browser caching for all media assets.

Changes

  • New media_version template filter (signbank/dictionary/templatetags/cache_control.py): returns the file's mtime as an integer. Accepts FileField objects (via .path) or URL-encoded path strings (decoded and resolved against WRITABLE_FOLDER). Returns 0 on failure.

  • gloss.html and gloss_detail.html: replaced all 15 occurrences of {% now 'YmdHis' %} with |media_version. Also removed the cache-busting parameter from external source URL hyperlinks where it was semantically meaningless.

<!-- Before: new cache key on every page load -->
<video src="{{ protected_media_url }}{{ gloss.get_video_url }}?v={% now 'YmdHis' %}">

<!-- After: cache key changes only when the file changes -->
<video src="{{ protected_media_url }}{{ gloss.get_video_url }}?v={{ gloss.get_video_url|media_version }}">

The filter falls back to 0 when a file is missing, keeping URLs well-formed without breaking anything.

Original prompt

This section details on the original issue you should resolve

<issue_title>Alternative for cache-busting query parameter</issue_title>
<issue_description>This was suggested by CoPilot

Adding a cache-busting query parameter ?v={% now 'YmdHis' %} to video URLs prevents browser caching, which could significantly increase bandwidth usage and slow down the user experience. Consider using more targeted cache invalidation strategies, such as versioned URLs or ETags, that only bust the cache when videos actually change.

Originally posted by @copilot in #1686 (comment)
</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits March 12, 2026 10:56
Co-authored-by: Woseseltops <1909317+Woseseltops@users.noreply.github.com>
Co-authored-by: Woseseltops <1909317+Woseseltops@users.noreply.github.com>
Co-authored-by: Woseseltops <1909317+Woseseltops@users.noreply.github.com>
Copilot AI changed the title [WIP] Consider alternative for cache-busting query parameter Replace per-request timestamp cache-busting with file mtime versioning for media URLs Mar 12, 2026
@Woseseltops Woseseltops requested a review from susanodd March 19, 2026 11:49
@Woseseltops
Copy link
Collaborator

@vanlummelhuizen susan suggest you also review this, you agree?

@vanlummelhuizen
Copy link
Collaborator

My assessment is that the url parameter v is never used and therefore there is no change necessary, other than removing that url parameter.

Reasoning:

  1. protected_media_url is constructed here:
    {% url 'dictionary:protected_media' '' as protected_media_url %}
    (and in other templates)
  2. ... which refers to
    re_path(r'protected_media/(?P<filename>.*)$', signbank.dictionary.views.protected_media, name='protected_media'),
  3. ... which refers to
    def protected_media(request, filename, document_root=WRITABLE_FOLDER, show_indexes=False):
    if not request.user.is_authenticated:
    # If we are not logged in, try to find if this maybe belongs to a gloss that is free to see for everbody?
    (name, ext) = os.path.splitext(os.path.basename(filename))
    if 'annotatedvideo' in filename:
    # check that the sentence exists
    try:
    file = os.path.basename(filename)
    sentence_pk = int(file.split('.')[0])
    except IndexError:
    return HttpResponse(status=401)
    lookup_sentence = AnnotatedSentence.objects.filter(pk=sentence_pk).first()
    if not lookup_sentence:
    return HttpResponse(status=401)
    pass
    elif 'handshape' in name:
    # handshape images are allowed to be seen in Show All Handshapes
    pass
    else:
    glosspk = extract_glossid_from_filename(filename)
    if glosspk is None or not Gloss.objects.filter(pk=glosspk, archived=False, inWeb=True).count() == 1:
    return HttpResponse(status=401)
    # If we got here, the gloss was found and in the web dictionary, so we can continue
    filename = os.path.normpath(filename)
    dir_path = WRITABLE_FOLDER
    path = dir_path.encode('utf-8') + filename.encode('utf-8')
    try:
    exists = os.path.exists(path)
    except (OSError, FileExistsError):
    exists = False
    if not exists:
    # quote the filename instead to resolve special characters in the url
    (head, tail) = os.path.split(filename)
    quoted_filename = quote(tail, safe='')
    quoted_path = os.path.join(dir_path, head, quoted_filename)
    exists = os.path.exists(quoted_path)
    if not exists:
    response = HttpResponse()
    return response
    else:
    filename = quoted_filename
    path = quoted_path
    if USE_X_SENDFILE:
    if filename.split('.')[-1] == 'mp4':
    response = HttpResponse(content_type='video/mp4')
    elif filename.split('.')[-1] == 'png':
    response = HttpResponse(content_type='image/png')
    elif filename.split('.')[-1] == 'jpg':
    response = HttpResponse(content_type='image/jpg')
    else:
    response = HttpResponse()
    response['Content-Disposition'] = 'inline;filename='+filename+';filename*=UTF-8'
    response['X-Sendfile'] = path
    return response
    else:
    return serve(request, filename, document_root, show_indexes)

And this view does not use any url parameter.

If my assessment is correct, CoPilot is just making stuff up and is basically wasting our time.

@susanodd
Copy link
Collaborator

You and Jetske were the ones that added the "?v={% now 'YmdHis' %}" to the urls in the gloss_detail.html template. I tried removing this before. But If one uploads a new video, the site keeps showing the old video rather than the new one. (Search the master branch on "\?v")

@susanodd
Copy link
Collaborator

susanodd commented Mar 20, 2026

If I was working on this, I probably would have removed the existing "now" code and instead when a new video is uploaded dynamically stick that into the template using jQuery and force the page to reload. (Which sounds like a bad solution.)

You can see on the gloss_videos.html page (video buttons branch #1686) that I added this "now" and forced the page to reload (a lot). So whatever solution ends up implemented is needed there too.

I have yet to test this solution.

@vanlummelhuizen
Copy link
Collaborator

Reassessment: What I now realize is that the ?v= part is only meant to have the browser not use the locally cached version of the video so that, when the video was changed, the new video is downloaded. Using now 'YmdHis' make the browser always download the video and not use the cached version. CoPilot is correct that this busts the caching.

Copilot's proposal is an option, but I don't know if checking the mtime is the best solution. An alternative is adding the ID of the GlossVideo in the ?v= part, because that also changes when a new video is uploaded (or a video is deleted and and backed-up video is used).

@susanodd @Woseseltops what do you think is the best solution?

@susanodd
Copy link
Collaborator

I like that idea!

I am trying to use CoPilot's solution but something broke with fetching the videos when combined with the new functionality for restoring a video. (Now it's fetching a video with "bak" in the name for some reason. So something happened to the path. Probably I did not merge it correctly.)

@Woseseltops
Copy link
Collaborator

Quick summary:

  • Current solution uses timestamp to prevent browser caching
  • Copilot suggests using the video mtime
  • @vanlummelhuizen suggests using the database id

If the solution by @vanlummelhuizen works it seems cleaner to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Alternative for cache-busting query parameter

4 participants