Hi Brian, I'm assuming you're making progress on this, but let me know if you need help. Thanks!

bawolff: How are things going?
Any news, or do you need help from somebody (who)?

Bryan is going to try having a go at it, since this is relatively self contained.

Reproduced on test2 with:

• upload https://test2.wikipedia.org/wiki/File:BDavis-Our-Beach.jpeg
• upload replacement with image flipped upside down
• Open first version: https://upload.wikimedia.org/wikipedia/test2/archive/f/f0/20130814212216%21BDavis-Our-Beach.jpeg
• Delete first version
• Hard reload https://upload.wikimedia.org/wikipedia/test2/archive/f/f0/20130814212216%21BDavis-Our-Beach.jpeg
  
  Response Headers
  
  HTTP/1.1 304 Not Modified Server: nginx/1.1.19 Date: Wed, 14 Aug 2013 21:37:56 GMT Content-Type: image/jpeg Connection: keep-alive X-Object-Meta-Sha1base36: q6tjiuuvytupp30yb76eav509pl464c Last-Modified: Wed, 14 Aug 2013 21:22:16 GMT Etag: f046823df66ca83b706e67c31a6d2420 X-Timestamp: 1376515336.85862 x-varnish: 176829175 176335682, 3224550997 Via: 1.1 varnish, 1.1 varnish Accept-Ranges: bytes Age: 242 X-Cache: cp1061 hit (5), cp1062 frontend miss (0) access-control-allow-origin: *

Created attachment 13103
Call SquidUpdate::purge() when deleteing

I think this patch will send the appropriate squid/varnish purge messages.

I tested it locally by running a sniffer to watch for the HTCP packets being sent. You can get the sniffer I used from https://github.com/bd808/htcpsnoop. I have not tested with an actual squid/varnish cache layer.

attachment bug51064.patch ignored as obsolete

I almost posted a review to gerrit and then I remembered this was marked as "security".

Aaron, can you look at the patch too, since this is part of filerepo?

I've tested and read over this patch. It looks good to me.

Some notes:

*body of the foreach loop should be indented
*There's still a code path with moving files where the old name of the file isn't purged, but that could be a separate issue, since we're not trying to hide the old filename, and long term we want the old filename to give an http redirect anyhow, so really this code path probably doesn't matter.
*The revision delete code path (RevDel_FileList::doPostCommitUpdates) still has this issue. This is a much more important code path then the move file (This includes things like oversight of files), but still could be considered a mildly separate bug. Nonetheless, I would recommend fixing this code path at the same time.

*body of the foreach loop should be indented

Easy fix. I think my current listchars setting in vim is making it hard for me to see some of these indent issues. I'll work on that.

*There's still a code path with moving files where the old name of the file
isn't purged, but that could be a separate issue, since we're not trying to
hide the old filename, and long term we want the old filename to give an http
redirect anyhow, so really this code path probably doesn't matter.

I think that the call to purgeEverything() in move() _should_ purge the squid. File::purgeEverything() calls LocalFile::purgeCache() which in turn calls SquidUpdate::purge(array($this->getURL())).

*The revision delete code path (RevDel_FileList::doPostCommitUpdates) still
has this issue. This is a much more important code path then the move file
(This includes things like oversight of files), but still could be considered
a mildly separate bug. Nonetheless, I would recommend fixing this code path
at the same time.

I'm looking at this now. Unfortunately the level of abstraction here and my very limited understanding of the codebase may make this take longer than it could/should. The original patch took me 2 days to create. :)

Created attachment 13129
Call SquidUpdate::purge() when deleteing

Updated patch that fixes indent issue and adds a call to SquidUpdate::purge() from RevDel_FileList::doPostCommitUpdates().

Attached:

(In reply to comment #10)

Created attachment 13129 [details]
Call SquidUpdate::purge() when deleteing

Updated patch that fixes indent issue and adds a call to SquidUpdate::purge()
from RevDel_FileList::doPostCommitUpdates().

This looks good to me

Attached:

Change 79842 had a related patch set uploaded by BryanDavis:
Purge upstream caches when deleting file assets.

https://gerrit.wikimedia.org/r/79842

Change 79842 merged by jenkins-bot:
Purge upstream caches when deleting file assets.

https://gerrit.wikimedia.org/r/79842

Change 79843 had a related patch set uploaded by BryanDavis:
Purge upstream caches when deleting file assets.

https://gerrit.wikimedia.org/r/79843

Change 79844 had a related patch set uploaded by BryanDavis:
Purge upstream caches when deleting file assets.

https://gerrit.wikimedia.org/r/79844

The patch for master has been merged, so marking fix. The patches for stable still need to be merged.

Change 79915 had a related patch set uploaded by BryanDavis:
Release note update for bug 51064.

https://gerrit.wikimedia.org/r/79915

Change 79914 had a related patch set uploaded by BryanDavis:
Release note update for bug 51064.

https://gerrit.wikimedia.org/r/79914

Change 79918 had a related patch set uploaded by BryanDavis:
Release note update for bug 51064.

https://gerrit.wikimedia.org/r/79918

Change 79914 merged by jenkins-bot:
Release note update for bug 51064.

https://gerrit.wikimedia.org/r/79914

Chris: did you want me to merge the backports of Bryan's patch?

Yes please!

Change 79844 merged by jenkins-bot:
Purge upstream caches when deleting file assets.

https://gerrit.wikimedia.org/r/79844

Change 79915 merged by jenkins-bot:
Release note update for bug 51064.

https://gerrit.wikimedia.org/r/79915

Change 79843 merged by Brian Wolff:
Purge upstream caches when deleting file assets.

https://gerrit.wikimedia.org/r/79843

Change 79918 merged by jenkins-bot:
Release note update for bug 51064.

https://gerrit.wikimedia.org/r/79918

Change 80515 had a related patch set uploaded by Brian Wolff:
Backport purge upstream caches when deleting file assets.

https://gerrit.wikimedia.org/r/80515

Change 80515 merged by jenkins-bot:
Backport purge upstream caches when deleting file assets.

https://gerrit.wikimedia.org/r/80515