Closed Bug 985290 Opened 10 years ago Closed 10 years ago

Always use hash instead of date modified for cachebusting app icons and screenshots

Categories

(Marketplace Graveyard :: Consumer Pages, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED
2014-04-08

People

(Reporter: cvan, Assigned: mat)

References

()

Details

(Keywords: perf, Whiteboard: [qa-])

For app icons and screenshots we're using the date modified (of the Webapp object) to cachebust icons <https://github.com/mozilla/zamboni/blob/2a7e8e8/apps/addons/models.py#L1004>:

    https://marketplace.cdn.mozilla.net/img/uploads/addon_icons/418/418206-64.png?modified=1395074901
    https://marketplace.cdn.mozilla.net/img/uploads/previews/thumbs/97/97216.png?modified=1372098680
    https://marketplace.cdn.mozilla.net/img/uploads/previews/full/97/97216.png?modified=1372098680

This is a bit silly because if an app developer changes the description, the modified timestamp changes, and now the URL for the app icon and screenshots has now just been changed and the cached copy is invalidated, causing another HTTP request to get an icon we already have in the browser cache.

We should instead keep a hash of the contents of the image and bust the URL off of that hash. No need to refetch an image that hasn't changed. But let's obviously be smart and store that offline so we don't need to recompute the hash on the fly.
See Also: → 985291
See Also: → 985292
See Also: → 985293
Summary: Always use hash instead of date modified for cachebusting app icons and screenshot → Always use hash instead of date modified for cachebusting app icons and screenshots
Assignee: nobody → mpillard
Screenshot previews are not an issue: the date used to build the cachebusting string comes from the Preview instance, not the Webapp, which is only updated when you modify images in the developer pages. It would still be nice to use a hash (because we re-save existing previews when you save changes in the edit media page) but that's less important ; I'll focus on the icons for now.
(In reply to Mathieu Pillard [:mat] from comment #1)
> Screenshot previews are not an issue: the date used to build the
> cachebusting string comes from the Preview instance, not the Webapp, which
> is only updated when you modify images in the developer pages.

Yes, but if you add a caption or change the order, the date modified changes, right? The date modified does not necessarily mean "date the image was changed."
You can't add captions anymore :) But yes, you're right. It's much less of a problem than the icons though, which depend on Webapp.modified. I'll probably end up patching both, currently fighting with icons tests.
Good point. Yeah, agreed, it probably doesn't cause issues that often. Thanks!
https://github.com/mozilla/zamboni/pull/1896 for icons
Status: NEW → ASSIGNED
Fixed in https://github.com/mozilla/zamboni/commit/b7225138748fd2909c1698abc6fe65db813614a4
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [qa-]
Target Milestone: --- → 2014-04-08
You need to log in before you can comment on or make changes to this bug.