r62231 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r62230‎ | r62231 | r62232 >
Date:10:36, 10 February 2010
Author:mah
Status:resolved (Comments)
Tags:
Comment:
* new FauxResponse class to help with unit testing
* Add append() method to FileRepo classes to enable chunked uploading
* Change chunksessionkey to chunksession
* Remove echo json stuff
* Fix a multitude of bugs in my own code
* still to test: mwEmbed use of chunked upload
Modified paths:
  • /trunk/phase3/includes/AutoLoader.php (modified) (history)
  • /trunk/phase3/includes/WebRequest.php (modified) (history)
  • /trunk/phase3/includes/WebResponse.php (modified) (history)
  • /trunk/phase3/includes/api/ApiUpload.php (modified) (history)
  • /trunk/phase3/includes/filerepo/FSRepo.php (modified) (history)
  • /trunk/phase3/includes/filerepo/FileRepo.php (modified) (history)
  • /trunk/phase3/includes/filerepo/ForeignAPIRepo.php (modified) (history)
  • /trunk/phase3/includes/filerepo/NullRepo.php (modified) (history)
  • /trunk/phase3/includes/upload/UploadFromChunks.php (modified) (history)
  • /trunk/phase3/maintenance/tests/UploadFromChunksTest.php (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/tests/UploadFromChunksTest.php
——@@ -9,8 +9,40 @@
1010
1111 $wgEnableUploads=true;
1212 ini_set('file_loads', true);
 13+ parent::setup();
 14+
1315 }
1416
 17+ function makeChunk() {
 18+ $file = tempnam( wfTempDir(), "" );
 19+ $fh = fopen($file, "w");
 20+ if($fh == false) {
 21+ $this->markTestIncomplete("Couldn't open $file!\n");
 22+ return;
 23+ }
 24+ fwrite($fh, "123");
 25+ fclose($fh);
 26+
 27+ $_FILES['chunk']['tmp_name'] = $file;
 28+ $_FILES['chunk']['size'] = 3;
 29+ $_FILES['chunk']['error'] = null;
 30+ $_FILES['chunk']['name'] = "test.txt";
 31+ }
 32+
 33+ function cleanChunk() {
 34+ if(file_exists($_FILES['chunk']['tmp_name']))
 35+ unlink($_FILES['chunk']['tmp_name']);
 36+ }
 37+
 38+ function doApiRequest($params) {
 39+ $session = isset( $_SESSION ) ? $_SESSION : array();
 40+ $req = new FauxRequest($params, true, $session);
 41+ $module = new ApiMain($req, true);
 42+ $module->execute();
 43+
 44+ return $module->getResultData();
 45+ }
 46+
1547 function testGetTitle() {
1648 $filename = tempnam( wfTempDir(), "" );
1749 $c = new UploadFromChunks();
——@@ -22,89 +54,110 @@
2355 $this->assertEquals(Title::makeTitleSafe(NS_FILE, "Temp.png"), $c->getTitle());
2456 }
2557
26 - function testGetEditToken() {
 58+ function testLogin() {
 59+ $data = $this->doApiRequest(array('action' => 'login',
 60+ "lgname" => self::$userName,
 61+ "lgpassword" => self::$passWord ) );
 62+ $this->assertArrayHasKey("login", $data);
 63+ $this->assertArrayHasKey("result", $data['login']);
 64+ $this->assertEquals("Success", $data['login']['result']);
 65+
 66+ return $data;
2767 }
2868
29 - function testInitFromSessionKey() {
3069
31 - }
 70+ /**
 71+ * @depends testLogin
 72+ */
 73+ function testGetEditToken($data) {
 74+ global $wgUser;
 75+ $wgUser = User::newFromName(self::$userName);
 76+ $wgUser->load();
3277
33 - function testInitialize() {
 78+ $data = $this->doApiRequest(array('action' => 'query',
 79+ 'prop' => 'info',
 80+ 'intoken' => 'edit'));
3481 }
3582
3683 function testSetupChunkSession() {
3784 }
3885
3986
40 - function makeChunk() {
41 - $file = tempnam( wfTempDir(), "" );
42 - $fh = fopen($file, "w");
43 - if($fh == false) {
44 - $this->markTestIncomplete("Couldn't open $file!\n");
45 - return;
46 - }
47 - fwrite($fh, "123");
48 - fclose($fh);
49 -
50 - $_FILES['chunk']['tmp_name'] = $file;
51 - $_FILES['chunk']['size'] = 3;
52 - $_FILES['chunk']['error'] = null;
53 - $_FILES['chunk']['name'] = "test.txt";
54 - }
55 -
56 - function cleanChunk() {
57 - unlink($_FILES['chunk']['tmp_name']);
58 - }
59 -
6087 /**
6188 * @expectedException UsageException
6289 */
6390 function testPerformUploadInitError() {
6491 global $wgUser;
65 -
6692 $wgUser = User::newFromId(1);
67 - $token = $wgUser->editToken();
68 - $this->makeChunk();
6993
7094 $req = new FauxRequest(
7195 array('action' => 'upload',
7296 'enablechunks' => '1',
7397 'filename' => 'test.png',
74 - 'token' => $token,
7598 ));
7699 $module = new ApiMain($req, true);
77100 $module->execute();
78101 }
79102
80 - function testPerformUploadInitSuccess() {
 103+ /**
 104+ * @depends testLogin
 105+ */
 106+ function testPerformUploadInitSuccess($login) {
81107 global $wgUser;
82108
83 - $wgUser = User::newFromId(1);
 109+ $wgUser = User::newFromName(self::$userName);
84110 $token = $wgUser->editToken();
85 - $this->makeChunk();
86111
87 - $req = new FauxRequest(
 112+ $data = $this->doApiRequest(
88113 array('action' => 'upload',
89114 'enablechunks' => '1',
90115 'filename' => 'test.png',
91116 'token' => $token,
92117 ));
93 - $module = new ApiMain($req, true);
94 - $module->execute();
95 - }
96118
97 - function testAppendToUploadFile() {
98 - }
 119+ $this->assertArrayHasKey("upload", $data);
 120+ $this->assertArrayHasKey("uploadUrl", $data['upload']);
99121
100 - function testAppendChunk() {
 122+ return array('data' => $data, 'session' => $_SESSION, 'token' => $token);
101123 }
102124
103 - function testPeformUploadChunk() {
104 - }
 125+ /**
 126+ * @depends testPerformUploadInitSuccess
 127+ */
 128+ function testAppendChunk($combo) {
 129+ global $wgUser;
 130+ $data = $combo['data'];
 131+ $_SESSION = $combo['session'];
 132+ $wgUser = User::newFromName(self::$userName);
 133+ $token = $wgUser->editToken();
105134
106 - function testPeformUploadDone() {
 135+ $url = $data['upload']['uploadUrl'];
 136+ $params = wfCgiToArray(substr($url, strpos($url, "?")));
 137+
 138+ for($i=0;$i<10;$i++) {
 139+ $this->makeChunk();
 140+ $data = $this->doApiRequest($params);
 141+ $this->cleanChunk();
 142+ }
 143+
 144+ return array('data' => $data, 'session' => $_SESSION, 'token' => $token, 'params' => $params);
107145 }
108146
 147+ /**
 148+ * @depends testAppendChunk
 149+ */
 150+ function testUploadChunkDone($combo) {
 151+ global $wgUser;
 152+ $data = $combo['data'];
 153+ $params = $combo['params'];
 154+ $_SESSION = $combo['session'];
 155+ $wgUser = User::newFromName(self::$userName);
 156+ $token = $wgUser->editToken();
109157
 158+ $params['done'] = 1;
110159
 160+ $this->makeChunk();
 161+ $data = $this->doApiRequest($params);
 162+ $this->cleanChunk();
 163+ }
111164 }
Index: trunk/phase3/includes/upload/UploadFromChunks.php
——@@ -22,7 +22,6 @@
2323 protected $chunkMode; // INIT, CHUNK, DONE
2424 protected $sessionKey;
2525 protected $comment;
26 - protected $fileSize = 0;
2726 protected $repoPath;
2827 protected $pageText;
2928 protected $watch;
——@@ -37,9 +36,8 @@
3837 throw new MWException( 'not implemented' );
3938 }
4039
41 - public function initialize( $done, $filename, $sessionKey, $path,
42 - $fileSize, $sessionData )
43 - {
 40+ public function initialize( $done, $filename, $sessionKey, $path, $fileSize, $sessionData ) {
 41+ global $wgTmpDirectory;
4442 $this->status = new Status;
4543
4644 $this->initFromSessionKey( $sessionKey, $sessionData );
——@@ -47,7 +45,7 @@
4846 if ( !$this->sessionKey && !$done ) {
4947 // session key not set, init the chunk upload system:
5048 $this->chunkMode = self::INIT;
51 - $this->mDesiredDestName = $filename;
 49+ $this->initializePathInfo( $filename, $path, 0, true);
5250 } else if ( $this->sessionKey && !$done ) {
5351 $this->chunkMode = self::CHUNK;
5452 } else if ( $this->sessionKey && $done ) {
——@@ -55,7 +53,7 @@
5654 }
5755 if ( $this->chunkMode == self::CHUNK || $this->chunkMode == self::DONE ) {
5856 $this->mTempPath = $path;
59 - $this->fileSize += $fileSize;
 57+ $this->mFileSize += $fileSize;
6058 }
6159 }
6260
——@@ -128,24 +126,25 @@
129127 // a) the user must have requested the token to get here and
130128 // b) should only happen over POST
131129 // c) we need the token to validate chunks are coming from a non-xss request
132 - $token = urlencode( $wgUser->editToken() );
133 - echo FormatJson::encode( array(
134 - 'uploadUrl' => wfExpandUrl( wfScript( 'api' ) ) . "?action=upload&" .
135 - "token={$token}&format=json&enablechunks=true&chunksessionkey=" .
136 - $this->setupChunkSession( $comment, $pageText, $watch ) ) );
137 - $wgOut->disable();
 130+ return Status::newGood(
 131+ array('uploadUrl' => wfExpandUrl( wfScript( 'api' ) ) . "?" .
 132+ wfArrayToCGI(array('action' => 'upload',
 133+ 'token' => $wgUser->editToken(),
 134+ 'format' => 'json',
 135+ 'filename' => $pageText,
 136+ 'enablechunks' => 'true',
 137+ 'chunksession' => $this->setupChunkSession( $comment, $pageText, $watch ) ) ) ) );
138138 } else if ( $this->chunkMode == self::CHUNK ) {
139 - $status = $this->appendChunk();
140 - if ( !$status->isOK() ) {
141 - return $status;
 139+ $this->appendChunk();
 140+ if ( !$this->status->isOK() ) {
 141+ return $this->status;
142142 }
143143 // return success:
144144 // firefogg expects a specific result
145145 // http://www.firefogg.org/dev/chunk_post.html
146 - echo FormatJson::encode(
147 - array( 'result' => 1, 'filesize' => $this->fileSize )
 146+ return Status::newGood(
 147+ array( 'result' => 1, 'filesize' => $this->mFileSize )
148148 );
149 - $wgOut->disable();
150149 } else if ( $this->chunkMode == self::DONE ) {
151150 if ( !$comment )
152151 $comment = $this->comment;
——@@ -164,12 +163,9 @@
165164
166165 // firefogg expects a specific result
167166 // http://www.firefogg.org/dev/chunk_post.html
168 - echo FormatJson::encode( array(
169 - 'result' => 1,
170 - 'done' => 1,
171 - 'resultUrl' => $file->getDescriptionUrl() )
 167+ return Status::newGood(
 168+ array('result' => 1, 'done' => 1, 'resultUrl' => $file->getDescriptionUrl() )
172169 );
173 - $wgOut->disable();
174170 }
175171
176172 return Status::newGood();
——@@ -199,18 +195,18 @@
200196 if ( !$this->repoPath ) {
201197 $this->status = $this->saveTempUploadedFile( $this->mDesiredDestName, $this->mTempPath );
202198
203 - if ( $status->isOK() ) {
204 - $this->repoPath = $status->value;
 199+ if ( $this->status->isOK() ) {
 200+ $this->repoPath = $this->status->value;
205201 $_SESSION['wsUploadData'][$this->sessionKey]['repoPath'] = $this->repoPath;
206202 }
207 - return $status;
 203+ return;
208204 }
209205 if ( $this->getRealPath( $this->repoPath ) ) {
210206 $this->status = $this->appendToUploadFile( $this->repoPath, $this->mTempPath );
211207 } else {
212208 $this->status = Status::newFatal( 'filenotfound', $this->repoPath );
213209 }
214 - if ( $this->fileSize > $wgMaxUploadSize )
 210+ if ( $this->mFileSize > $wgMaxUploadSize )
215211 $this->status = Status::newFatal( 'largefileserver' );
216212 }
217213
Index: trunk/phase3/includes/filerepo/FSRepo.php
——@@ -227,6 +227,33 @@
228228 return $status;
229229 }
230230
 231+ function append( $srcPath, $toAppendPath ) {
 232+ $status = $this->newGood();
 233+
 234+ // Resolve the virtual URL
 235+ if ( self::isVirtualUrl( $srcPath ) ) {
 236+ $srcPath = $this->resolveVirtualUrl( $srcPath );
 237+ }
 238+ // Make sure the files are there
 239+ if ( !is_file( $srcPath ) )
 240+ $status->fatal( 'append-src-filenotfound', $srcPath );
 241+
 242+ if ( !is_file( $toAppendPath ) )
 243+ $status->fatal( 'append-toappend-filenotfound', $toAppendPath );
 244+
 245+ // Do the append
 246+ if( file_put_contents( $srcPath, file_get_contents( $toAppendPath ), FILE_APPEND ) ) {
 247+ $status->value = $srcPath;
 248+ } else {
 249+ $status->fatal( 'fileappenderror', $toAppendPath, $srcPath);
 250+ }
 251+
 252+ // Remove the source file
 253+ unlink( $toAppendPath );
 254+
 255+ return $status;
 256+ }
 257+
231258 /**
232259 * Checks existence of specified array of files.
233260 *
——@@ -575,7 +602,7 @@
576603 }
577604 return strtr( $param, $this->simpleCleanPairs );
578605 }
579 -
 606+
580607 /**
581608 * Chmod a file, supressing the warnings.
582609 * @param String $path The path to change
Index: trunk/phase3/includes/filerepo/NullRepo.php
——@@ -14,6 +14,9 @@
1515 function storeTemp( $originalName, $srcPath ) {
1616 return false;
1717 }
 18+ function append( $srcPath, $toAppendPath ){
 19+ return false;
 20+ }
1821 function publishBatch( $triplets, $flags = 0 ) {
1922 return false;
2023 }
Index: trunk/phase3/includes/filerepo/FileRepo.php
——@@ -30,7 +30,7 @@
3131 // Optional settings
3232 $this->initialCapital = MWNamespace::isCapitalized( NS_FILE );
3333 foreach ( array( 'descBaseUrl', 'scriptDirUrl', 'articleUrl', 'fetchDescription',
34 - 'thumbScriptUrl', 'initialCapital', 'pathDisclosureProtection',
 34+ 'thumbScriptUrl', 'initialCapital', 'pathDisclosureProtection',
3535 'descriptionCacheExpiry', 'hashLevels', 'url', 'thumbUrl' ) as $var )
3636 {
3737 if ( isset( $info[$var] ) ) {
——@@ -87,7 +87,7 @@
8888 *
8989 * ignoreRedirect: If true, do not follow file redirects
9090 *
91 - * private: If true, return restricted (deleted) files if the current
 91+ * private: If true, return restricted (deleted) files if the current
9292 * user is allowed to view them. Otherwise, such files will not
9393 * be found.
9494 */
——@@ -123,12 +123,12 @@
124124 }
125125 }
126126 }
127 -
 127+
128128 # Now try redirects
129129 if ( !empty( $options['ignoreRedirect'] ) ) {
130130 return false;
131131 }
132 - $redir = $this->checkRedirect( $title );
 132+ $redir = $this->checkRedirect( $title );
133133 if( $redir && $redir->getNamespace() == NS_FILE) {
134134 $img = $this->newFile( $redir );
135135 if( !$img ) {
——@@ -141,9 +141,9 @@
142142 }
143143 return false;
144144 }
145 -
 145+
146146 /*
147 - * Find many files at once.
 147+ * Find many files at once.
148148 * @param array $items, an array of titles, or an array of findFile() options with
149149 * the "title" option giving the title. Example:
150150 *
——@@ -168,7 +168,7 @@
169169 }
170170 return $result;
171171 }
172 -
 172+
173173 /**
174174 * Create a new File object from the local repository
175175 * @param mixed $sha1 SHA-1 key
——@@ -189,14 +189,14 @@
190190 return call_user_func( $this->fileFactoryKey, $sha1, $this );
191191 }
192192 }
193 -
 193+
194194 /**
195195 * Find an instance of the file with this key, created at the specified time
196196 * Returns false if the file does not exist. Repositories not supporting
197197 * version control should return false if the time is specified.
198198 *
199199 * @param string $sha1 string
200 - * @param array $options Option array, same as findFile().
 200+ * @param array $options Option array, same as findFile().
201201 */
202202 function findFileFromKey( $sha1, $options = array() ) {
203203 if ( !is_array( $options ) ) {
——@@ -234,7 +234,7 @@
235235 function getThumbScriptUrl() {
236236 return $this->thumbScriptUrl;
237237 }
238 -
 238+
239239 /**
240240 * Get the URL corresponding to one of the four basic zones
241241 * @param String $zone One of: public, deleted, temp, thumb
——@@ -280,7 +280,7 @@
281281 return $path;
282282 }
283283 }
284 -
 284+
285285 /**
286286 * Get a relative path including trailing slash, e.g. f/fa/
287287 * If the repo is not hashed, returns an empty string
——@@ -397,6 +397,8 @@
398398 */
399399 abstract function storeTemp( $originalName, $srcPath );
400400
 401+ abstract function append( $srcPath, $toAppendPath );
 402+
401403 /**
402404 * Remove a temporary file or mark it for garbage collection
403405 * @param string $virtualUrl The virtual URL returned by storeTemp
——@@ -587,14 +589,14 @@
588590 /**
589591 * Invalidates image redirect cache related to that image
590592 * Doesn't do anything for repositories that don't support image redirects.
591 - *
 593+ *
592594 * STUB
593595 * @param Title $title Title of image
594 - */
 596+ */
595597 function invalidateImageRedirect( $title ) {}
596 -
 598+
597599 /**
598 - * Get an array or iterator of file objects for files that have a given
 600+ * Get an array or iterator of file objects for files that have a given
599601 * SHA-1 content hash.
600602 *
601603 * STUB
——@@ -602,9 +604,9 @@
603605 function findBySha1( $hash ) {
604606 return array();
605607 }
606 -
 608+
607609 /**
608 - * Get the human-readable name of the repo.
 610+ * Get the human-readable name of the repo.
609611 * @return string
610612 */
611613 public function getDisplayName() {
——@@ -616,12 +618,12 @@
617619 if ( !wfEmptyMsg( 'shared-repo-name-' . $this->name, $repoName ) ) {
618620 return $repoName;
619621 }
620 - return wfMsg( 'shared-repo' );
 622+ return wfMsg( 'shared-repo' );
621623 }
622 -
 624+
623625 /**
624626 * Get a key on the primary cache for this repository.
625 - * Returns false if the repository's cache is not accessible at this site.
 627+ * Returns false if the repository's cache is not accessible at this site.
626628 * The parameters are the parts of the key, as for wfMemcKey().
627629 *
628630 * STUB
——@@ -631,7 +633,7 @@
632634 }
633635
634636 /**
635 - * Get a key for this repo in the local cache domain. These cache keys are
 637+ * Get a key for this repo in the local cache domain. These cache keys are
636638 * not shared with remote instances of the repo.
637639 * The parameters are the parts of the key, as for wfMemcKey().
638640 */
Index: trunk/phase3/includes/filerepo/ForeignAPIRepo.php
——@@ -22,7 +22,7 @@
2323 var $apiThumbCacheExpiry = 86400;
2424 protected $mQueryCache = array();
2525 protected $mFileExists = array();
26 -
 26+
2727 function __construct( $info ) {
2828 parent::__construct( $info );
2929 $this->mApiBase = $info['apibase']; // http://commons.wikimedia.org/w/api.php
——@@ -42,7 +42,7 @@
4343 $this->thumbUrl = $this->url . '/thumb';
4444 }
4545 }
46 -
 46+
4747 /**
4848 * Per docs in FileRepo, this needs to return false if we don't support versioned
4949 * files. Well, we don't.
——@@ -63,14 +63,17 @@
6464 function storeTemp( $originalName, $srcPath ) {
6565 return false;
6666 }
 67+ function append( $srcPath, $toAppendPath ){
 68+ return false;
 69+ }
6770 function publishBatch( $triplets, $flags = 0 ) {
6871 return false;
6972 }
7073 function deleteBatch( $sourceDestPairs ) {
7174 return false;
7275 }
73 -
7476
 77+
7578 function fileExistsBatch( $files, $flags = 0 ) {
7679 $results = array();
7780 foreach ( $files as $k => $f ) {
——@@ -99,10 +102,10 @@
100103 function getFileProps( $virtualUrl ) {
101104 return false;
102105 }
103 -
 106+
104107 protected function queryImage( $query ) {
105108 $data = $this->fetchImageQuery( $query );
106 -
 109+
107110 if( isset( $data['query']['pages'] ) ) {
108111 foreach( $data['query']['pages'] as $pageid => $info ) {
109112 if( isset( $info['imageinfo'][0] ) ) {
——@@ -112,10 +115,10 @@
113116 }
114117 return false;
115118 }
116 -
 119+
117120 protected function fetchImageQuery( $query ) {
118121 global $wgMemc;
119 -
 122+
120123 $url = $this->mApiBase .
121124 '?' .
122125 wfArrayToCgi(
——@@ -123,7 +126,7 @@
124127 array(
125128 'format' => 'json',
126129 'action' => 'query' ) ) );
127 -
 130+
128131 if( !isset( $this->mQueryCache[$url] ) ) {
129132 $key = $this->getLocalCacheKey( 'ForeignAPIRepo', 'Metadata', md5( $url ) );
130133 $data = $wgMemc->get( $key );
——@@ -143,14 +146,14 @@
144147 }
145148 return FormatJson::decode( $this->mQueryCache[$url], true );
146149 }
147 -
 150+
148151 function getImageInfo( $title, $time = false ) {
149152 return $this->queryImage( array(
150153 'titles' => 'Image:' . $title->getText(),
151154 'iiprop' => 'timestamp|user|comment|url|size|sha1|metadata|mime',
152155 'prop' => 'imageinfo' ) );
153156 }
154 -
 157+
155158 function findBySha1( $hash ) {
156159 $results = $this->fetchImageQuery( array(
157160 'aisha1base36' => $hash,
——@@ -164,7 +167,7 @@
165168 }
166169 return $ret;
167170 }
168 -
 171+
169172 function getThumbUrl( $name, $width=-1, $height=-1 ) {
170173 $info = $this->queryImage( array(
171174 'titles' => 'Image:' . $name,
——@@ -179,14 +182,14 @@
180183 return false;
181184 }
182185 }
183 -
 186+
184187 function getThumbUrlFromCache( $name, $width, $height ) {
185188 global $wgMemc, $wgUploadPath, $wgServer, $wgUploadDirectory;
186 -
 189+
187190 if ( !$this->canCacheThumbs() ) {
188191 return $this->getThumbUrl( $name, $width, $height );
189192 }
190 -
 193+
191194 $key = $this->getLocalCacheKey( 'ForeignAPIRepo', 'ThumbUrl', $name );
192195 if ( $thumbUrl = $wgMemc->get($key) ) {
193196 wfDebug("Got thumb from local cache. $thumbUrl \n");
——@@ -194,7 +197,7 @@
195198 }
196199 else {
197200 $foreignUrl = $this->getThumbUrl( $name, $width, $height );
198 -
 201+
199202 // We need the same filename as the remote one :)
200203 $fileName = rawurldecode( pathinfo( $foreignUrl, PATHINFO_BASENAME ) );
201204 $path = 'thumb/' . $this->getHashPath( $name ) . $name . "/";
——@@ -213,7 +216,7 @@
214217 return $localUrl;
215218 }
216219 }
217 -
 220+
218221 /**
219222 * @see FileRepo::getZoneUrl()
220223 */
Index: trunk/phase3/includes/api/ApiUpload.php
——@@ -40,6 +40,10 @@
4141 public function execute() {
4242 global $wgUser, $wgAllowCopyUploads;
4343
 44+ // Check whether upload is enabled
 45+ if ( !UploadBase::isEnabled() )
 46+ $this->dieUsageMsg( array( 'uploaddisabled' ) );
 47+
4448 $this->getMain()->isWriteMode();
4549 $this->mParams = $this->extractRequestParams();
4650 $request = $this->getMain()->getRequest();
——@@ -53,15 +57,27 @@
5458 // Add the uploaded file to the params array
5559 $this->mParams['file'] = $request->getFileName( 'file' );
5660
57 - // Check whether upload is enabled
58 - if ( !UploadBase::isEnabled() )
59 - $this->dieUsageMsg( array( 'uploaddisabled' ) );
60 -
6161 // One and only one of the following parameters is needed
6262 $this->requireOnlyOneParameter( $this->mParams,
6363 'sessionkey', 'file', 'url', 'enablechunks' );
6464
65 - if ( $this->mParams['sessionkey'] ) {
 65+ // Initialize $this->mUpload
 66+ if ( $this->mParams['enablechunks'] ) {
 67+ $this->mUpload = new UploadFromChunks();
 68+
 69+ $this->mUpload->initialize(
 70+ $request->getVal( 'done', null ),
 71+ $request->getVal( 'filename', null ),
 72+ $request->getVal( 'chunksession', null ),
 73+ $request->getFileTempName( 'chunk' ),
 74+ $request->getFileSize( 'chunk' ),
 75+ $request->getSessionData( 'wsUploadData' )
 76+ );
 77+
 78+ if ( !$this->mUpload->status->isOK() ) {
 79+ return $this->dieUsageMsg( $this->mUpload->status->getErrorsArray() );
 80+ }
 81+ } elseif ( $this->mParams['sessionkey'] ) {
6682 /**
6783 * Upload stashed in a previous request
6884 */
——@@ -72,30 +88,13 @@
7389 $this->mUpload = new UploadFromStash();
7490 $this->mUpload->initialize( $this->mParams['filename'],
7591 $_SESSION['wsUploadData'][$this->mParams['sessionkey']] );
76 - } else {
 92+ } elseif ( isset( $this->mParams['filename'] ) ) {
7793 /**
7894 * Upload from url, etc
7995 * Parameter filename is required
8096 */
81 - if ( !isset( $this->mParams['filename'] ) )
82 - $this->dieUsageMsg( array( 'missingparam', 'filename' ) );
8397
84 - // Initialize $this->mUpload
85 - if ( $this->mParams['enablechunks'] ) {
86 - $this->mUpload = new UploadFromChunks();
87 - $this->mUpload->initialize(
88 - $request->getVal( 'done', null ),
89 - $request->getVal( 'filename', null ),
90 - $request->getVal( 'chunksessionkey', null ),
91 - $request->getFileTempName( 'chunk' ),
92 - $request->getFileSize( 'chunk' ),
93 - $request->getSessionData( 'wsUploadData' )
94 - );
95 -
96 - if ( !$this->mUpload->status->isOK() ) {
97 - return $this->dieUsageMsg( $this->mUpload->status->getErrorsArray() );
98 - }
99 - } elseif ( isset( $this->mParams['file'] ) ) {
 98+ if ( isset( $this->mParams['file'] ) ) {
10099 $this->mUpload = new UploadFromFile();
101100 $this->mUpload->initialize(
102101 $this->mParams['filename'],
——@@ -120,7 +119,7 @@
121120 return $this->dieUsage( $status->getWikiText(), 'fetchfileerror' );
122121 }
123122 }
124 - }
 123+ } else $this->dieUsageMsg( array( 'missingparam', 'filename' ) );
125124
126125 if ( !isset( $this->mUpload ) )
127126 $this->dieUsage( 'No upload module set', 'nomodule' );
——@@ -242,6 +241,8 @@
243242 $this->getResult()->setIndexedTagName( $result['details'], 'error' );
244243
245244 $this->dieUsage( 'An internal error occurred', 'internal-error', 0, $error );
 245+ } elseif( isset($status->value->uploadUrl) ) {
 246+ return $status->value;
246247 }
247248
248249 $file = $this->mUpload->getLocalFile();
——@@ -272,7 +273,7 @@
273274 'ignorewarnings' => false,
274275 'file' => null,
275276 'enablechunks' => false,
276 - 'chunksessionkey' => null,
 277+ 'chunksession' => null,
277278 'chunk' => null,
278279 'done' => false,
279280 'url' => null,
——@@ -295,7 +296,7 @@
296297 'ignorewarnings' => 'Ignore any warnings',
297298 'file' => 'File contents',
298299 'enablechunks' => 'Set to use chunk mode; see http://firefogg.org/dev/chunk_post.html for protocol',
299 - 'chunksessionkey' => 'The session key, established on the first contact during the chunked upload',
 300+ 'chunksession' => 'The session key, established on the first contact during the chunked upload',
300301 'chunk' => 'The data in this chunk of a chunked upload',
301302 'done' => 'Set to 1 on the last chunk of a chunked upload',
302303 'url' => 'Url to fetch the file from',
Index: trunk/phase3/includes/WebRequest.php
——@@ -712,6 +712,7 @@
713713 class FauxRequest extends WebRequest {
714714 private $wasPosted = false;
715715 private $session = array();
 716+ private $response;
716717
717718 /**
718719 * @param $data Array of *non*-urlencoded key => value pairs, the
——@@ -767,9 +768,8 @@
768769 }
769770
770771 public function getSessionData( $key ) {
771 - if( !isset( $this->session[$key] ) )
772 - return null;
773 - return $this->session[$key];
 772+ if( isset( $this->session[$key] ) )
 773+ return $this->session[$key];
774774 }
775775
776776 public function setSessionData( $key, $data ) {
——@@ -780,4 +780,11 @@
781781 return false;
782782 }
783783
 784+ public function response() {
 785+ /* Lazy initialization of response object for this request */
 786+ if ( !is_object( $this->response ) ) {
 787+ $this->response = new FauxResponse;
 788+ }
 789+ return $this->response;
 790+ }
784791 }
Index: trunk/phase3/includes/AutoLoader.php
——@@ -84,6 +84,7 @@
8585 'FakeTitle' => 'includes/FakeTitle.php',
8686 'FakeMemCachedClient' => 'includes/ObjectCache.php',
8787 'FauxRequest' => 'includes/WebRequest.php',
 88+ 'FauxResponse' => 'includes/WebResponse.php',
8889 'FeedItem' => 'includes/Feed.php',
8990 'FeedUtils' => 'includes/FeedUtils.php',
9091 'FileDeleteForm' => 'includes/FileDeleteForm.php',
Index: trunk/phase3/includes/WebResponse.php
——@@ -6,7 +6,7 @@
77 */
88 class WebResponse {
99
10 - /**
 10+ /**
1111 * Output a HTTP header, wrapper for PHP's
1212 * header()
1313 * @param $string String: header to output
——@@ -58,3 +58,31 @@
5959 }
6060 }
6161 }
 62+
 63+
 64+class FauxResponse extends WebResponse {
 65+ private $headers;
 66+ private $cookies;
 67+
 68+ public function header($string, $replace=true) {
 69+ list($key, $val) = explode(":", $string, 2);
 70+
 71+ if($replace || !isset($this->headers[$key])) {
 72+ $this->headers[$key] = $val;
 73+ }
 74+ }
 75+
 76+ public function getheader($key) {
 77+ return $this->headers[$key];
 78+ }
 79+
 80+ public function setcookie( $name, $value, $expire = 0 ) {
 81+ $this->cookies[$name] = $value;
 82+ }
 83+
 84+ public function getcookie( $name ) {
 85+ if ( isset($this->cookies[$name]) ) {
 86+ return $this->cookies[$name];
 87+ }
 88+ }
 89+}
\ No newline at end of file

Follow-up revisions

RevisionCommit summaryAuthorDate
r62233Followup r62231, reduce code duplication in $request->response()demon10:49, 10 February 2010
r62235follow-up r62231 Document FileRepo::append()mah11:08, 10 February 2010
r62344Merge the remainder (r62231-r62338) of trunk revisions into LiquidThreads alphawerdna23:29, 11 February 2010
r62806follow up r62231, r61779, r62175...mah02:15, 22 February 2010

Comments

#Comment by 😂 (talk | contribs)   10:45, 10 February 2010
  1. Please keep whitespace changes out of commits with functional changes (ie: keep style fixes on their own).
  2. Document FileRepo::append()
  3. WebResponse doesn't have getcookie() or getheader() methods. They don't make sense here, as WebResponse is supposed to be about pushing data to the client, not getting data from it (that's what WebRequest is for!)
#Comment by MarkAHershberger (talk | contribs)   11:01, 10 February 2010

The FauxResponse class is specifically for testing, the get* methods are to be used for testing.

#Comment by Demontest (talk | contribs)   11:36, 10 February 2010

I can understand getheaders() (we really should camelCase these...). getcookie() should still be in WebRequest, IMHO.

I was toying with a FauxResponse class the other day for a completely different issue. If instead of calling header() and accessing $_GET|POST|COOKIE directly in code, all such calls should be going through WebRequest/WebResponse.

Right now, things can set headers without regard to whether it's a real request. We had this come up about a week ago when someone was making a FauxRequest to ApiMain and somewhere deeper in the code was setting headers and interfering with normal header output. This would be avoided if all this was encapsulated.

#Comment by MarkAHershberger (talk | contribs)   00:41, 11 February 2010

agreed, another benefit. Although, in production code, people should make calls directly instead of using FauxRequest.

#Comment by 😂 (talk | contribs)   19:41, 11 February 2010

Why? Making a FauxRequest to ApiMain is a perfectly legitimate way of getting data, and is documented at API:Calling_internally.

#Comment by MarkAHershberger (talk | contribs)   20:40, 11 February 2010

Just based on my own conversations with Tim where he has said he would prefer people make calls directly.

#Comment by Tim Starling (talk | contribs)   04:36, 16 February 2010 
+		global $wgTmpDirectory;

Declared but not used. 

+				array('uploadUrl' => wfExpandUrl( wfScript( 'api' ) ) . "?" .
+					  wfArrayToCGI(array('action' => 'upload',
+										 'token' => $wgUser->editToken(),
+										 'format' => 'json',
+										 'filename' => $pageText,
+										 'enablechunks' => 'true',
+										 'chunksession' => $this->setupChunkSession( $comment, $pageText, $watch ) ) ) ) );

Wrong whitespace style. See the Manual:Coding conventions section on indenting and alignment. 

+			$status->fatal( 'append-src-filenotfound', $srcPath );

Message does not exist. Two more instances. 

+		// Remove the source file
+		unlink( $toAppendPath );

You should follow the convention from store/publish and only delete the source file if that is specifically requested by the caller, with a FileRepo::DELETE_SOURCE flag. This avoids inadvertent data loss. 

+		if( file_put_contents( $srcPath, file_get_contents( $toAppendPath ), FILE_APPEND ) ) {

Fails to check for an error in file_get_contents(). May append nothing and return success.

#Comment by MarkAHershberger (talk | contribs)   03:15, 22 February 2010

fixed in r62806

Status & tagging log