Hi folks,
Exec summary: I'm looking for help on a possible change needed to FileRepo::publish() API, since it currently makes some assumptions that seem to be incompatible with fixing bug 4421 ("Image file extension should not be part of the name"): https://bugzilla.wikimedia.org/show_bug.cgi?id=4421
I'm also more generally looking for feedback on my approach to 4421.
Details: A while back, I decided to try my hand at fixing bug 4421. It's a longstanding usability wart, and I figured it'd be a good way to re-familiarize myself with the codebase. It's mostly working, though working through all of the edge cases is a bit of a game of whack-a-mole. Most of the complexity comes from needing to store the files on the filesystem with appropriate file extensions, since these get served directly from the filesystem from Apache. Thus, there's a lot of convoluted logic for tacking on the file extension in the appropriate spots. You can read the comments on 4421 to understand why things are the way they are now.
An example of something that isn't working that I need advice on is this: with my modified version, it's possible to upload a jpeg to a location without an extension, then upload a png to that same location. The problem comes in LocalFile::publish(). Here's the call it makes from that function: $status = $this->repo->publish( $srcPath, $dstRel, $archiveRel, $flags );
This causes two things to happen: 1. copy $dstRel to $archiveRel 2. copy $srcPath to $dstRel.
The problem here is with uploading a png over the top of a jpg. For example, if the title name is "File:Foo", then the filename for the first version of the file will be "Foo.jpg", and the replacement will be "Foo.png". So, if we pass "Foo.jpg" to $dstRel, then step 1 works, but step 2 fails. If we pass "Foo.png", then the opposite problem occurs.
The only solution it would seem is to rework FileRepo::publish() to accommodate this case (adding another parameter). That doesn't seem *so* bad, though being an interface change. Here's what I'm hoping folks will weigh in on: a. If reworking FileRepo::publish() is the way to do it. b. If my general approach on 4421 seems sound, and to get a sense if there's going to be objections to landing this in trunk soon (presumably after 1.16). I've just updated my latest version here: https://bugzilla.wikimedia.org/attachment.cgi?id=6885&action=diff
Feel free to answer here or to ping me on IRC ("robla" on Freenode #mediawiki)
Thanks! Rob