Page MenuHomePhabricator

Consider merging fileCacheExpiry and apiThumbCacheExpiry in ForeignAPIRepo
Open, Needs TriagePublicBUG REPORT

Description

I might be missing something with the logic.

In the event of an apiThumbCacheExpiry cache hit, MW immediately outputs the url of the thumbnail.

In the event of an apiThumbCacheExpiry cache miss, MW checks:

  • Fetches the metadata
  • Checks if the file has been modified since downloaded
  • checks if the file is older than fileCacheExpiry

If either of those 2 things aren't true, it redownloads the file

Fetching metadata is normally expensive, however as far as i can tell, to get to this stage of the process, ->transform() has been called on the File object, which means ->canRender() has been called, which means ->getWidth() has been called, which means' the file's metadata has been loaded, so the file metadata is already in cache. So fetching file metadata should be free in this case.

Thus unless there's something i am missing here, i don't see the point of having both this and fileCacheExpiry. Seems mostly to cause stale cache entries to stick around for no purpose. Thus i think we should merge the two settings.