Bryan.TongMinh wrote:

I will have to break backwards compatibility in some hooks. For now it is UploadComplete which passes a reference to the SpecialUpload object. I think the best way to do this is moving UploadComplete to UploadFromBase and add a new hook SpecialUploadComplete to allow content modification.

Bryan.TongMinh wrote:

UploadFromMediawiki or UploadFromImport or something similar would also be nice.

Bryan.TongMinh wrote:

Work in progress - 1

Current work in progress

attachment patch.txt ignored as obsolete

Bryan.TongMinh wrote:

Work in progress - 1.1

Proper patch this time.

Note that it won't apply cleanly unless you svn cp includes/specials/SpecialUpload.php includes/UploadFromBase.php

attachment patch.txt ignored as obsolete

Bryan.TongMinh wrote:

Work in progress - 2

My second patch, which should be pretty much complete. Don't forget the

svn cp includes/specials/SpecialUpload.php includes/UploadFromBase.php

attachment patch.txt ignored as obsolete

Bryan.TongMinh wrote:

Done in r40091.

Bryan.TongMinh wrote:

Update

Reverted by Tim Starling.

I did some more work on it, but I don't have much time the coming weeks. So perhaps if somebody wants to test the code or take it over from here that would be great.

Some comments from Tim on the current state:

The only reasonable application, it seems to me, is to replace
Special:Upload with something else entirely -- say an API. The design
looks OK from that perspective, but I think this would be a missed
opportunity for generalisation. There's only a few more changes that
need to be made to allow upload sources to be extended dynamically.

I'd like to see multi-file upload in the core, and a Java applet
extension, but UploadBase isn't generic enough for that either.
UploadFromUrl actually calls $wgOut->showErrorPage(), which really kills
any kind of alternate UI. And the behaviour of
UploadBase::stashSession(), storing individual files under random keys
directly to $_SESSION, would appear very inelegant if it were applied to
a multi-file situation or alternate client situation.

attachment patch.txt ignored as obsolete

Bryan.TongMinh wrote:

Another update

Another update. I'm not really happy with it and also don't have much time for it. I would love if somebody who is better at engineering plugin interfaces could look at this.

Attached:

Tagging as need-review; calling on Tim to review this and see if it addresses the issues he brought up.

(In reply to comment #9)

Tagging as need-review; calling on Tim to review this and see if it addresses
the issues he brought up.

Bump. This is needed for a pretty major feature (API upload), and has been waiting to be reviewed for nearly 3 months now.

• Update the patch and Upload*.php, needs to incorporate SpecialUpload.php changes between r41253 and r44701
• UploadForm::internalProcessUpload() has too much boilerplate, which would have to be reproduced in each new UI component. I suggest merging verifyPermissions(), fetchFile(), verifyUpload(), checkWarnings(), performUpload() and cleanupTempFile() into a single function. It should accept an associative array as its parameter, with members such as initial page text, ignore warnings, comment and user. Preferably it would return a Status object.
• Get rid of the UploadBase status constants, they're not extensible. Use message names for status codes. You can wrap them in Status objects where convenient.
• Fix the UploadBase::BEFORE_PROCESSING case in processUpload()
• Fix stupid comment "It will show an error form on failure. No it will not."
• Rename UploadFromStash to UploadFromSession, document it to the point where ordinary humans might be able to work out how to use it. Rename stashSession() to createSession(), rename unsaveUploadedFile() to destroySession(). Document UploadFromSession::initialize(). Document the fact that these three functions go together.
• Suggest renaming "UploadFromUpload" to something less vague, maybe "UploadFromWebForm"
• "Upload handlers. Should probably just be a global" -- indeed it should, then it would be extensible. But the global should allow for arbitrary class names, instead of prepending "UploadFrom", so that extensions can maintain a naming scheme where class names start with the name of the extension.
• Document all functions, including parameter definitions, with an extension writer audience in mind. It won't take long.

Adding this as a blocker to 18563. This has been done in the new-upload branch and will become a part of core when that gets merged.

New-upload was merged to trunk, this is fixed there.