r106630 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r106629‎ | r106630 | r106631 >
Date:08:47, 19 December 2011
Author:aaron
Status:deferred
Tags:
Comment:
In SwiftFileBackend:
* Updated code due to function renames in base classes
* Removed functions that can use the parent implementation
* Added TODO/FIXME items
* Comment tweaks and cleanup
* Broke long lines and made some code style cleanups
In FSFileBackend:
* Various comment cleanups
Modified paths:
  • /branches/FileBackend/phase3/includes/filerepo/backend/FSFileBackend.php (modified) (history)
  • /branches/FileBackend/phase3/includes/filerepo/backend/SwiftFileBackend.php (modified) (history)

Diff [purge]

Index: branches/FileBackend/phase3/includes/filerepo/backend/FSFileBackend.php
——@@ -19,6 +19,9 @@
2020
2121 /**
2222 * @see FileBackend::__construct()
 23+ * Additional $config params include:
 24+ * containerPaths : Map of container names to absolute file system paths
 25+ * fileMode : Octal UNIX file permissions to use on files stored
2326 */
2427 function __construct( array $config ) {
2528 parent::__construct( $config );
Index: branches/FileBackend/phase3/includes/filerepo/backend/SwiftFileBackend.php
——@@ -9,22 +9,35 @@
1010 * Status messages should avoid mentioning the Swift account name
1111 * Likewise, error suppression should be used to avoid path disclosure.
1212 *
 13+ * @FIXME: resuse connections with auto-connect and don't let connection
 14+ * exceptions bubble up from read/write operations.
 15+ *
1316 * @ingroup FileBackend
1417 */
1518 class SwiftFileBackend extends FileBackend {
16 - protected $swiftuser,
17 - $swiftkey,
18 - $authurl,
19 - $container;
 19+ protected $swiftUser; // string
 20+ protected $swiftKey; // string
 21+ protected $swiftAuthUrl; // string
 22+ protected $swiftProxyUser; // string
2023
 24+ /**
 25+ * @see FileBackend::__construct()
 26+ * Additional $config params include:
 27+ * swiftAuthUrl : Swift authentication server URL
 28+ * swiftUser : Swift user used by MediaWiki
 29+ * swiftKey : Authentication key for the above user (used to get sessions)
 30+ * swiftProxyUser : Swift user used for end-user hits to proxy server
 31+ */
2132 function __construct( array $config ) {
2233 parent::__construct( $config );
23 -
2434 // Required settings
25 - $this->swiftuser = $config['user'];
26 - $this->swiftkey = $config['key'];
27 - $this->authurl = $config['authurl'];
28 - $this->container = $config['container'];
 35+ $this->swiftUser = $config['swiftUser'];
 36+ $this->swiftKey = $config['swiftKey'];
 37+ $this->swiftAuthUrl = $config['swiftAuthUrl'];
 38+ // Optional settings
 39+ $this->swiftProxyUser = isset( $config['swiftProxyUser'] )
 40+ ? $config['swiftProxyUser']
 41+ : '';
2942 }
3043
3144 /**
——@@ -33,13 +46,14 @@
3447 * @return CF_Connection
3548 */
3649 protected function connect() {
37 - $auth = new CF_Authentication( $this->swiftuser, $this->swiftkey, NULL, $this->authurl );
 50+ $auth = new CF_Authentication(
 51+ $this->swiftUser, $this->swiftKey, NULL, $this->swiftAuthUrl );
3852 try {
3953 $auth->authenticate();
4054 } catch ( AuthenticationException $e ) {
4155 throw new MWException( "We can't authenticate ourselves." );
4256 # } catch (InvalidResponseException $e) {
43 - # throw new MWException( __METHOD__ . "unexpected response '$e'" );
 57+ # throw new MWException( __METHOD__ . "unexpected response '$e'" );
4458 }
4559 return new CF_Connection( $auth );
4660 }
——@@ -58,12 +72,13 @@
5973 } catch ( NoSuchContainerException $e ) {
6074 throw new MWException( "A container we thought existed, doesn't." );
6175 # } catch (InvalidResponseException $e) {
62 - # throw new MWException( __METHOD__ . "unexpected response '$e'" );
 76+ # throw new MWException( __METHOD__ . "unexpected response '$e'" );
6377 }
6478 }
6579
6680 /**
6781 * Copy a file from one place to another place
 82+ *
6883 * @param $srcContainer CF_Container
6984 * @param $srcRel String: relative path to the source file.
7085 * @param $dstContainer CF_Container
——@@ -87,25 +102,31 @@
88103 try {
89104 $obj = $dstContainer->get_object( $dstRel );
90105 } catch ( NoSuchObjectException $e ) {
91 - throw new MWException( 'The object we just created does not exist: ' . $dstContainer->name . "/$dstRel: $e" );
 106+ throw new MWException( 'The object we just created does not exist: ' .
 107+ $dstContainer->name . "/$dstRel: $e" );
92108 }
93109
94110 try {
95111 $srcObj = $srcContainer->get_object( $srcRel );
96112 } catch ( NoSuchObjectException $e ) {
97 - throw new MWException( 'Source file does not exist: ' . $srcContainer->name . "/$srcRel: $e" );
 113+ throw new MWException( 'Source file does not exist: ' .
 114+ $srcContainer->name . "/$srcRel: $e" );
98115 }
99116
100117 try {
101118 $dstContainer->copy_object_from($srcObj,$srcContainer,$dstRel);
102119 } catch ( SyntaxException $e ) {
103 - throw new MWException( 'Source file does not exist: ' . $srcContainer->name . "/$srcRel: $e" );
 120+ throw new MWException( 'Source file does not exist: ' .
 121+ $srcContainer->name . "/$srcRel: $e" );
104122 } catch ( MisMatchedChecksumException $e ) {
105123 throw new MWException( "Checksums do not match: $e" );
106124 }
107125 }
108126
109 - function store( array $params ) {
 127+ /**
 128+ * @see FileBackend::doStore()
 129+ */
 130+ function doStore( array $params ) {
110131 $status = Status::newGood();
111132
112133 list( $destc, $dest ) = $this->resolveStoragePath( $params['dst'] );
——@@ -123,7 +144,7 @@
124145 return $status;
125146 }
126147 $exists = true;
127 - } catch (NoSuchObjectException $e) {
 148+ } catch ( NoSuchObjectException $e ) {
128149 $exists = false;
129150 }
130151
——@@ -142,7 +163,10 @@
143164 return $status;
144165 }
145166
146 - function copy( array $params ) {
 167+ /**
 168+ * @see FileBackend::doCopy()
 169+ */
 170+ function doCopy( array $params ) {
147171 $status = Status::newGood();
148172
149173 list( $sourcec, $source ) = $this->resolveStoragePath( $params['src'] );
——@@ -168,22 +192,21 @@
169193 return $status;
170194 }
171195 $exists = true;
172 - } catch (NoSuchObjectException $e) {
 196+ } catch ( NoSuchObjectException $e ) {
173197 $exists = false;
174198 }
175199 try {
176200 $this->swiftcopy( $srcc, $source, $dstc, $dest );
177 - } catch (InvalidResponseException $e ) {
 201+ } catch ( InvalidResponseException $e ) {
178202 $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] );
179 - }
 203+ }
180204 return $status;
181205 }
182206
183 - function canMove( array $params ) {
184 - return false;
185 - }
186 -
187 - function delete( array $params ) {
 207+ /**
 208+ * @see FileBackend::doDelete()
 209+ */
 210+ function doDelete( array $params ) {
188211 $status = Status::newGood();
189212
190213 list( $sourcec, $source ) = $this->resolveStoragePath( $params['src'] );
——@@ -196,30 +219,33 @@
197220 $container = $this->get_container( $conn, $sourcec );
198221
199222 try {
200 - $obj = $container->get_object( $source);
 223+ $obj = $container->get_object( $source );
201224 $exists = true;
202 - } catch (NoSuchObjectException $e) {
 225+ } catch ( NoSuchObjectException $e ) {
203226 if ( empty( $params['ignoreMissingSource'] ) ) {
204227 $status->fatal( 'backend-fail-delete', $params['src'] );
205228 }
206229 $exists = false;
207230 }
208231
209 - if ($exists) {
 232+ if ( $exists ) {
210233 try {
211234 $container->delete_object( $source );
212235 } catch ( SyntaxException $e ) {
213236 throw new MWException( "Swift object name not well-formed: '$e'" );
214237 } catch ( NoSuchObjectException $e ) {
215238 throw new MWException( "Swift object we are trying to delete does not exist: '$e'" );
216 - } catch (InvalidResponseException $e) {
 239+ } catch ( InvalidResponseException $e ) {
217240 $status->fatal( 'backend-fail-delete', $params['src'] );
218241 }
219242 }
220243 return $status; // do nothing; either OK or bad status
221244 }
222245
223 - function concatenate( array $params ) {
 246+ /**
 247+ * @see FileBackend::doConcatenate()
 248+ */
 249+ function doConcatenate( array $params ) {
224250 $status = Status::newGood();
225251
226252 list( $destc, $dest ) = $this->resolveStoragePath( $params['dst'] );
——@@ -238,17 +264,17 @@
239265 return $status;
240266 }
241267 $exists = true;
242 - } catch (NoSuchObjectException $e) {
 268+ } catch ( NoSuchObjectException $e ) {
243269 $exists = false;
244270 }
245 -
 271+
246272 try {
247273 $biggie = $dstc->create_object( $dest );
248 - } catch (InvalidResponseException $e) {
 274+ } catch ( InvalidResponseException $e ) {
249275 $status->fatal( 'backend-fail-opentemp', $tmpPath );
250276 return $status;
251277 }
252 -
 278+
253279 foreach ( $params['srcs'] as $virtualSource ) {
254280 list( $sourcec, $source ) = $this->resolveStoragePath( $virtualSource );
255281 if ( $source === null ) {
——@@ -256,13 +282,16 @@
257283 return $status;
258284 }
259285 $srcc = $this->get_container( $conn, $sourcec );
260 - $obj = $srcc->get_object( $source);
 286+ $obj = $srcc->get_object( $source );
261287 $biggie->write( $obj->read() );
262288 }
263289 return $status;
264290 }
265291
266 - function create( array $params ) {
 292+ /**
 293+ * @see FileBackend::doCopy()
 294+ */
 295+ function doCreate( array $params ) {
267296 $status = Status::newGood();
268297
269298 list( $destc, $dest ) = $this->resolveStoragePath( $params['dst'] );
——@@ -281,17 +310,18 @@
282311 return $status;
283312 }
284313 $exists = true;
285 - } catch (NoSuchObjectException $e) {
 314+ } catch ( NoSuchObjectException $e ) {
286315 $exists = false;
287316 }
288317
289318 $obj = $dstc->create_object( $dest );
290319 //FIXME how do we know what the content type is?
 320+ // This *should* work...cloudfiles can figure content type from strings too.
291321 $obj->content_type = 'text/plain';
292322
293323 try {
294324 $obj->write( $params['content'] );
295 - } catch (InvalidResponseException $e ) {
 325+ } catch ( InvalidResponseException $e ) {
296326 $status->fatal( 'backend-fail-create', $params['dst'] );
297327 return $status;
298328 }
——@@ -299,21 +329,18 @@
300330 return $status;
301331 }
302332
303 - function prepare( array $params ) {
304 - $status = Status::newGood();
305 - return $status; // we good. we so good, we BAD.
306 - }
307 -
 333+ /**
 334+ * @see FileBackend::secure()
 335+ */
308336 function secure( array $params ) {
309337 $status = Status::newGood();
 338+ // @TODO: restrict container from $this->swiftProxyUser
310339 return $status; // badgers? We don't need no steenking badgers!
311340 }
312341
313 - function clean( array $params ) {
314 - $status = Status::newGood();
315 - return $status; // can't delete what doesn't exist.
316 - }
317 -
 342+ /**
 343+ * @see FileBackend::fileExists()
 344+ */
318345 function fileExists( array $params ) {
319346 list( $sourcec, $source ) = $this->resolveStoragePath( $params['src'] );
320347 if ( $source === null ) {
——@@ -329,7 +356,10 @@
330357 }
331358 return $exists;
332359 }
333 -
 360+
 361+ /**
 362+ * @see FileBackend::getFileTimestamp()
 363+ */
334364 function getFileTimestamp( array $params ) {
335365 list( $sourcec, $source ) = $this->resolveStoragePath( $params['src'] );
336366 if ( $source === null ) {
——@@ -345,14 +375,19 @@
346376 }
347377 if ( $obj ) {
348378 $thumbTime = $obj->last_modified;
 379+ // @FIXME: strptime() UNIX-only (http://php.net/manual/en/function.strptime.php)
349380 $tm = strptime( $thumbTime, '%a, %d %b %Y %H:%M:%S GMT' );
350 - $thumbGMT = gmmktime( $tm['tm_hour'], $tm['tm_min'], $tm['tm_sec'], $tm['tm_mon'] + 1, $tm['tm_mday'], $tm['tm_year'] + 1900 );
 381+ $thumbGMT = gmmktime( $tm['tm_hour'], $tm['tm_min'], $tm['tm_sec'],
 382+ $tm['tm_mon'] + 1, $tm['tm_mday'], $tm['tm_year'] + 1900 );
351383 return ( gmdate( 'YmdHis', $thumbGMT ) );
352384 } else {
353385 return false; // file not found.
354386 }
355387 }
356388
 389+ /**
 390+ * @see FileBackend::getFileList()
 391+ */
357392 function getFileList( array $params ) {
358393 list( $dirc, $dir ) = $this->resolveStoragePath( $params['dir'] );
359394 if ( $dir === null ) { // invalid storage path
——@@ -360,21 +395,16 @@
361396 }
362397
363398 $conn = $this->connect();
 399+ // @TODO: return an Iterator class that pages via list_objects()
364400 $container = $this->get_container( $conn, $dirc );
365 - $files = $container->list_objects( 0, NULL, $dir);
 401+ $files = $container->list_objects( 0, NULL, $dir );
366402 // if there are no files matching the prefix, return empty array
367403 return $files;
368404 }
369405
370 - function getLocalReference( array $params ) {
371 - list( $c, $source ) = $this->resolveStoragePath( $params['src'] );
372 - if ( $source === null ) {
373 - return null;
374 - }
375 - // FIXME!
376 - return new FSFile( $source );
377 - }
378 -
 406+ /**
 407+ * @see FileBackend::getLocalCopy()
 408+ */
379409 function getLocalCopy( array $params ) {
380410 list( $sourcec, $source ) = $this->resolveStoragePath( $params['src'] );
381411 if ( $source === null ) {
——@@ -392,7 +422,7 @@
393423 $tmpPath = $tmpFile->getPath();
394424
395425 $conn = $this->connect();
396 - $cont = $this->get_container( $conn, $sourcec);
 426+ $cont = $this->get_container( $conn, $sourcec );
397427
398428 try {
399429 $obj = $cont->get_object( $source );
——@@ -411,5 +441,4 @@
412442 }
413443 return $tmpFile;
414444 }
415 -
416445 }

Status & tagging log