r105636 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r105635‎ | r105636 | r105637 >
Date:01:54, 9 December 2011
Author:aaron
Status:deferred (Comments)
Tags:
Comment:
* In FileOp:
** Fixed 'overwriteSame' parameter checks and made it actually work
** Added functions to just get read/changed storage paths separately
** Made doPrecheck() have a default implementation
** Renamed member checkDest => destAlreadyExists
* Removed deadlock comment from DBLockManager
* A few other random fixes
Modified paths:
  • /branches/FileBackend/phase3/includes/filerepo/backend/FileBackend.php (modified) (history)
  • /branches/FileBackend/phase3/includes/filerepo/backend/FileBackendMultiWrite.php (modified) (history)
  • /branches/FileBackend/phase3/includes/filerepo/backend/FileOp.php (modified) (history)
  • /branches/FileBackend/phase3/includes/filerepo/backend/LockManager.php (modified) (history)

Diff [purge]

Index: branches/FileBackend/phase3/includes/filerepo/backend/FileBackendMultiWrite.php
——@@ -84,7 +84,7 @@
8585 }
8686
8787 // Try to lock those files for the scope of this function...
88 - $scopedLock = $this->getScopedFileLocks( $filesToLock, $status );
 88+ $scopedLock = $this->getScopedFileLocks( $filesToLock, LockManager::LOCK_EX, $status );
8989 if ( !$status->isOK() ) {
9090 return $status; // abort
9191 }
Index: branches/FileBackend/phase3/includes/filerepo/backend/FileOp.php
——@@ -18,11 +18,12 @@
1919 protected $params;
2020 /** $var FileBackendBase */
2121 protected $backend;
22 - /** @var TempLocalFile|null */
 22+ /** @var TempFSFile|null */
2323 protected $tmpSourceFile, $tmpDestFile;
2424
25 - protected $state;
26 - protected $failed;
 25+ protected $state; // integer
 26+ protected $failed; // boolean
 27+ protected $destSameAsSource; // boolean
2728
2829 /* Object life-cycle */
2930 const STATE_NEW = 1;
——@@ -41,10 +42,11 @@
4243 $this->params = $params;
4344 $this->state = self::STATE_NEW;
4445 $this->failed = false;
 46+ $this->destSameAsSource = false;
4547 $this->initialize();
4648 }
4749
48 - /*
 50+ /**
4951 * Get the value of the parameter with the given name.
5052 * Returns null if the parameter is not set.
5153 *
——@@ -148,15 +150,33 @@
149151 }
150152
151153 /**
152 - * Get a list of storage paths to lock for this operation
 154+ * Get a list of storage paths used by this operation
153155 *
154156 * @return Array
155157 */
156 - public function storagePathsUsed() {
 158+ final public function storagePathsUsed() {
 159+ return array_merge( $this->storagePathsRead(), $this->storagePathsChanged() );
 160+ }
 161+
 162+ /**
 163+ * Get a list of storage paths read from for this operation
 164+ *
 165+ * @return Array
 166+ */
 167+ public function storagePathsRead() {
157168 return array();
158169 }
159170
160171 /**
 172+ * Get a list of storage paths written to for this operation
 173+ *
 174+ * @return Array
 175+ */
 176+ public function storagePathsChanged() {
 177+ return array();
 178+ }
 179+
 180+ /**
161181 * @return void
162182 */
163183 protected function initialize() {}
——@@ -164,7 +184,9 @@
165185 /**
166186 * @return Status
167187 */
168 - abstract protected function doPrecheck( array &$predicates );
 188+ protected function doPrecheck( array &$predicates ) {
 189+ return Status::newGood();
 190+ }
169191
170192 /**
171193 * @return Status
——@@ -224,19 +246,19 @@
225247 return $status;
226248 }
227249 } elseif ( $this->getParam( 'overwriteSame' ) ) {
228 - // Get the source content hash (if there is a single source)
229250 $shash = $this->getSourceMD5();
230251 // If there is a single source, then we can do some checks already.
231 - // For things like concatenate(), we need to build a temp file first.
 252+ // For things like concatenate(), we would need to build a temp file
 253+ // first and thus don't support 'overwriteSame' ($shash is null).
232254 if ( $shash !== null ) {
233255 $dhash = $this->getFileMD5( $this->params['dst'] );
234256 if ( !strlen( $shash ) || !strlen( $dhash ) ) {
235257 $status->fatal( 'backend-fail-hashes' );
236 - return $status;
237 - }
238 - // Give an error if the files are not identical
239 - if ( $shash !== $dhash ) {
 258+ } elseif ( $shash !== $dhash ) {
 259+ // Give an error if the files are not identical
240260 $status->fatal( 'backend-fail-notsame', $this->params['dst'] );
 261+ } else {
 262+ $this->destSameAsSource = true;
241263 }
242264 return $status; // do nothing; either OK or bad status
243265 }
——@@ -263,13 +285,13 @@
264286 *
265287 * @return string|false False on failure
266288 */
267 - final protected function getFileMD5( $path ) {
 289+ protected function getFileMD5( $path ) {
268290 // Source file is in backend
269291 if ( FileBackend::isStoragePath( $path ) ) {
270292 // For some backends (e.g. Swift, Azure) we can get
271293 // standard hashes to use for this types of comparisons.
272294 if ( $this->backend->getHashType() === 'md5' ) {
273 - $hash = $this->backend->getFileHash( $path );
 295+ $hash = $this->backend->getFileHash( array( 'src' => $path ) );
274296 } else {
275297 $tmp = $this->backend->getLocalCopy( array( 'src' => $path ) );
276298 if ( !$tmp ) {
——@@ -327,13 +349,13 @@
328350 }
329351
330352 /**
331 - * Check if a file will exist when this operation is attempted
 353+ * Check if a file will exist in storage when this operation is attempted
332354 *
333 - * @param $source string
 355+ * @param $source string Storage path
334356 * @param $predicates Array
335357 * @return bool
336358 */
337 - final protected function fileExists( $source, $predicates ) {
 359+ final protected function fileExists( $source, array $predicates ) {
338360 if ( isset( $predicates['exists'][$source] ) ) {
339361 return $predicates['exists'][$source]; // previous op assures this
340362 } else {
——@@ -351,21 +373,21 @@
352374 * overwriteSame : override any existing file at destination
353375 */
354376 class StoreFileOp extends FileOp {
355 - protected $checkDest = true;
 377+ protected $destAlreadyExists = true;
356378
357379 protected function doPrecheck( array &$predicates ) {
358380 $status = Status::newGood();
359381 // Check if destination file exists
360382 if ( $this->fileExists( $this->params['dst'], $predicates ) ) {
361 - if ( !$this->getParam( 'overwriteDest' ) ) {
 383+ if ( !$this->getParam( 'overwriteDest' ) && !$this->getParam( 'overwriteSame' ) ) {
362384 $status->fatal( 'backend-fail-alreadyexists', $this->params['dst'] );
363385 return $status;
364386 }
365387 } else {
366 - $this->checkDest = false;
 388+ $this->destAlreadyExists = false;
367389 }
368390 // Check if the source file exists on the file system
369 - if ( !file_exists( $this->params['src'] ) ) {
 391+ if ( !is_file( $this->params['src'] ) ) {
370392 $status->fatal( 'backend-fail-notexists', $this->params['src'] );
371393 return $status;
372394 }
——@@ -377,26 +399,31 @@
378400 protected function doAttempt() {
379401 $status = Status::newGood();
380402 // Create a destination backup copy as needed
381 - if ( $this->checkDest ) {
 403+ if ( $this->destAlreadyExists ) {
382404 $status->merge( $this->checkAndBackupDest() );
383405 if ( !$status->isOK() ) {
384406 return $status;
385407 }
386408 }
387409 // Store the file at the destination
388 - $status->merge( $this->backend->store( $this->params ) );
 410+ if ( !$this->destSameAsSource ) {
 411+ $status->merge( $this->backend->store( $this->params ) );
 412+ }
389413 return $status;
390414 }
391415
392416 protected function doRevert() {
393 - // Remove the file saved to the destination
394 - $params = array( 'src' => $this->params['dst'] );
395 - $status = $this->backend->delete( $params );
396 - if ( !$status->isOK() ) {
397 - return $status; // also can't restore any dest file
 417+ $status = Status::newGood();
 418+ if ( !$this->destSameAsSource ) {
 419+ // Remove the file saved to the destination
 420+ $params = array( 'src' => $this->params['dst'] );
 421+ $status->merge( $this->backend->delete( $params ) );
 422+ if ( !$status->isOK() ) {
 423+ return $status; // also can't restore any dest file
 424+ }
 425+ // Restore any file that was at the destination
 426+ $status->merge( $this->restoreDest() );
398427 }
399 - // Restore any file that was at the destination
400 - $status->merge( $this->restoreDest() );
401428 return $status;
402429 }
403430
——@@ -404,7 +431,7 @@
405432 return md5_file( $this->params['src'] );
406433 }
407434
408 - function storagePathsUsed() {
 435+ public function storagePathsChanged() {
409436 return array( $this->params['dst'] );
410437 }
411438 }
——@@ -418,18 +445,18 @@
419446 * overwriteSame : override any existing file at destination
420447 */
421448 class CreateFileOp extends FileOp {
422 - protected $checkDest = true;
 449+ protected $destAlreadyExists = true;
423450
424451 protected function doPrecheck( array &$predicates ) {
425452 $status = Status::newGood();
426453 // Check if destination file exists
427454 if ( $this->fileExists( $this->params['dst'], $predicates ) ) {
428 - if ( !$this->getParam( 'overwriteDest' ) ) {
 455+ if ( !$this->getParam( 'overwriteDest' ) && !$this->getParam( 'overwriteSame' ) ) {
429456 $status->fatal( 'backend-fail-alreadyexists', $this->params['dst'] );
430457 return $status;
431458 }
432459 } else {
433 - $this->checkDest = false;
 460+ $this->destAlreadyExists = false;
434461 }
435462 // Update file existence predicates
436463 $predicates['exists'][$this->params['dst']] = true;
——@@ -439,26 +466,31 @@
440467 protected function doAttempt() {
441468 $status = Status::newGood();
442469 // Create a destination backup copy as needed
443 - if ( $this->checkDest ) {
 470+ if ( $this->destAlreadyExists ) {
444471 $status->merge( $this->checkAndBackupDest() );
445472 if ( !$status->isOK() ) {
446473 return $status;
447474 }
448475 }
449476 // Create the file at the destination
450 - $status->merge( $this->backend->create( $this->params ) );
 477+ if ( !$this->destSameAsSource ) {
 478+ $status->merge( $this->backend->create( $this->params ) );
 479+ }
451480 return $status;
452481 }
453482
454483 protected function doRevert() {
455 - // Remove the file saved to the destination
456 - $params = array( 'src' => $this->params['dst'] );
457 - $status = $this->backend->delete( $params );
458 - if ( !$status->isOK() ) {
459 - return $status; // also can't restore any dest file
 484+ $status = Status::newGood();
 485+ if ( !$this->destSameAsSource ) {
 486+ // Remove the file saved to the destination
 487+ $params = array( 'src' => $this->params['dst'] );
 488+ $status->merge( $this->backend->delete( $params ) );
 489+ if ( !$status->isOK() ) {
 490+ return $status; // also can't restore any dest file
 491+ }
 492+ // Restore any file that was at the destination
 493+ $status->merge( $this->restoreDest() );
460494 }
461 - // Restore any file that was at the destination
462 - $status->merge( $this->restoreDest() );
463495 return $status;
464496 }
465497
——@@ -466,7 +498,7 @@
467499 return md5( $this->params['content'] );
468500 }
469501
470 - function storagePathsUsed() {
 502+ public function storagePathsChanged() {
471503 return array( $this->params['dst'] );
472504 }
473505 }
——@@ -480,18 +512,18 @@
481513 * overwriteSame : override any existing file at destination
482514 */
483515 class CopyFileOp extends FileOp {
484 - protected $checkDest = true;
 516+ protected $destAlreadyExists = true;
485517
486518 protected function doPrecheck( array &$predicates ) {
487519 $status = Status::newGood();
488520 // Check if destination file exists
489521 if ( $this->fileExists( $this->params['dst'], $predicates ) ) {
490 - if ( !$this->getParam( 'overwriteDest' ) ) {
 522+ if ( !$this->getParam( 'overwriteDest' ) && !$this->getParam( 'overwriteSame' ) ) {
491523 $status->fatal( 'backend-fail-alreadyexists', $this->params['dst'] );
492524 return $status;
493525 }
494526 } else {
495 - $this->checkDest = false;
 527+ $this->destAlreadyExists = false;
496528 }
497529 // Check if the source file exists
498530 if ( !$this->fileExists( $this->params['src'], $predicates ) ) {
——@@ -506,27 +538,31 @@
507539 protected function doAttempt() {
508540 $status = Status::newGood();
509541 // Create a destination backup copy as needed
510 - if ( $this->checkDest ) {
 542+ if ( $this->destAlreadyExists ) {
511543 $status->merge( $this->checkAndBackupDest() );
512544 if ( !$status->isOK() ) {
513545 return $status;
514546 }
515547 }
516548 // Copy the file into the destination
517 - $status->merge( $this->backend->copy( $this->params ) );
 549+ if ( !$this->destSameAsSource ) {
 550+ $status->merge( $this->backend->copy( $this->params ) );
 551+ }
518552 return $status;
519553 }
520554
521555 protected function doRevert() {
522556 $status = Status::newGood();
523 - // Remove the file saved to the destination
524 - $params = array( 'src' => $this->params['dst'] );
525 - $status->merge( $this->backend->delete( $params ) );
526 - if ( !$status->isOK() ) {
527 - return $status; // also can't restore any dest file
 557+ if ( !$this->destSameAsSource ) {
 558+ // Remove the file saved to the destination
 559+ $params = array( 'src' => $this->params['dst'] );
 560+ $status->merge( $this->backend->delete( $params ) );
 561+ if ( !$status->isOK() ) {
 562+ return $status; // also can't restore any dest file
 563+ }
 564+ // Restore any file that was at the destination
 565+ $status->merge( $this->restoreDest() );
528566 }
529 - // Restore any file that was at the destination
530 - $status->merge( $this->restoreDest() );
531567 return $status;
532568 }
533569
——@@ -534,9 +570,13 @@
535571 return $this->getFileMD5( $this->params['src'] );
536572 }
537573
538 - function storagePathsUsed() {
539 - return array( $this->params['src'], $this->params['dst'] );
 574+ public function storagePathsRead() {
 575+ return array( $this->params['src'] );
540576 }
 577+
 578+ public function storagePathsChanged() {
 579+ return array( $this->params['dst'] );
 580+ }
541581 }
542582
543583 /**
——@@ -549,7 +589,7 @@
550590 */
551591 class MoveFileOp extends FileOp {
552592 protected $usingMove = false; // using backend move() function?
553 - protected $checkDest = true;
 593+ protected $destAlreadyExists = true;
554594
555595 function initialize() {
556596 // Use faster, native, move() if applicable
——@@ -560,12 +600,12 @@
561601 $status = Status::newGood();
562602 // Check if destination file exists
563603 if ( $this->fileExists( $this->params['dst'], $predicates ) ) {
564 - if ( !$this->getParam( 'overwriteDest' ) ) {
 604+ if ( !$this->getParam( 'overwriteDest' ) && !$this->getParam( 'overwriteSame' ) ) {
565605 $status->fatal( 'backend-fail-alreadyexists', $this->params['dst'] );
566606 return $status;
567607 }
568608 } else {
569 - $this->checkDest = false;
 609+ $this->destAlreadyExists = false;
570610 }
571611 // Check if the source file exists
572612 if ( !$this->fileExists( $this->params['src'], $predicates ) ) {
——@@ -581,23 +621,37 @@
582622 protected function doAttempt() {
583623 $status = Status::newGood();
584624 // Create a destination backup copy as needed
585 - if ( $this->checkDest ) {
 625+ if ( $this->destAlreadyExists ) {
586626 $status->merge( $this->checkAndBackupDest() );
587627 if ( !$status->isOK() ) {
588628 return $status;
589629 }
590630 }
591 - // Native moves: move the file into the destination
592 - if ( $this->usingMove ) {
593 - $status->merge( $this->backend->move( $this->params ) );
594 - // Non-native moves: copy the file into the destination & delete source
 631+ if ( !$this->destSameAsSource ) {
 632+ // Native moves: move the file into the destination
 633+ if ( $this->usingMove ) {
 634+ $status->merge( $this->backend->move( $this->params ) );
 635+ // Non-native moves: copy the file into the destination & delete source
 636+ } else {
 637+ // Copy source to dest
 638+ $status->merge( $this->backend->copy( $this->params ) );
 639+ if ( !$status->isOK() ) {
 640+ return $status;
 641+ }
 642+ // Delete source
 643+ $params = array( 'src' => $this->params['src'] );
 644+ $status->merge( $this->backend->delete( $params ) );
 645+ if ( !$status->isOK() ) {
 646+ return $status;
 647+ }
 648+ }
595649 } else {
596 - // Copy source to dest
597 - $status->merge( $this->backend->copy( $this->params ) );
 650+ // Create a source backup copy as needed
 651+ $status->merge( $this->backupSource() );
598652 if ( !$status->isOK() ) {
599653 return $status;
600654 }
601 - // Delete source
 655+ // Just delete source as the destination needs no changes
602656 $params = array( 'src' => $this->params['src'] );
603657 $status->merge( $this->backend->delete( $params ) );
604658 if ( !$status->isOK() ) {
——@@ -609,33 +663,39 @@
610664
611665 protected function doRevert() {
612666 $status = Status::newGood();
613 - // Native moves: move the file back to the source
614 - if ( $this->usingMove ) {
615 - $params = array(
616 - 'src' => $this->params['dst'],
617 - 'dst' => $this->params['src']
618 - );
619 - $status->merge( $this->backend->move( $params ) );
620 - if ( !$status->isOK() ) {
621 - return $status; // also can't restore any dest file
 667+ if ( !$this->destSameAsSource ) {
 668+ // Native moves: move the file back to the source
 669+ if ( $this->usingMove ) {
 670+ $params = array(
 671+ 'src' => $this->params['dst'],
 672+ 'dst' => $this->params['src']
 673+ );
 674+ $status->merge( $this->backend->move( $params ) );
 675+ if ( !$status->isOK() ) {
 676+ return $status; // also can't restore any dest file
 677+ }
 678+ // Non-native moves: remove the file saved to the destination
 679+ } else {
 680+ // Copy destination back to source
 681+ $params = array( 'src' => $this->params['dst'], 'dst' => $this->params['src'] );
 682+ $status = $this->backend->copy( $params );
 683+ if ( !$status->isOK() ) {
 684+ return $status; // also can't restore any dest file
 685+ }
 686+ // Delete destination
 687+ $params = array( 'src' => $this->params['dst'] );
 688+ $status = $this->backend->delete( $params );
 689+ if ( !$status->isOK() ) {
 690+ return $status; // also can't restore any dest file
 691+ }
622692 }
623 - // Non-native moves: remove the file saved to the destination
 693+ // Restore any file that was at the destination
 694+ $status->merge( $this->restoreDest() );
624695 } else {
625 - // Copy destination back to source
626 - $params = array( 'src' => $this->params['dst'], 'dst' => $this->params['src'] );
627 - $status = $this->backend->copy( $params );
628 - if ( !$status->isOK() ) {
629 - return $status; // also can't restore any dest file
630 - }
631 - // Delete destination
632 - $params = array( 'src' => $this->params['dst'] );
633 - $status = $this->backend->delete( $params );
634 - if ( !$status->isOK() ) {
635 - return $status; // also can't restore any dest file
636 - }
 696+ // Restore any source file
 697+ return $this->restoreSource();
637698 }
638 - // Restore any file that was at the destination
639 - $status->merge( $this->restoreDest() );
 699+
640700 return $status;
641701 }
642702
——@@ -643,9 +703,13 @@
644704 return $this->getFileMD5( $this->params['src'] );
645705 }
646706
647 - function storagePathsUsed() {
648 - return array( $this->params['src'], $this->params['dst'] );
 707+ public function storagePathsRead() {
 708+ return array( $this->params['src'] );
649709 }
 710+
 711+ public function storagePathsChanged() {
 712+ return array( $this->params['dst'] );
 713+ }
650714 }
651715
652716 /**
——@@ -656,7 +720,7 @@
657721 * overwriteDest : do nothing and pass if an identical file exists at destination
658722 */
659723 class ConcatenateFileOp extends FileOp {
660 - protected $checkDest = true;
 724+ protected $destAlreadyExists = true;
661725
662726 protected function doPrecheck( array &$predicates ) {
663727 $status = Status::newGood();
——@@ -667,7 +731,7 @@
668732 return $status;
669733 }
670734 } else {
671 - $this->checkDest = false;
 735+ $this->destAlreadyExists = false;
672736 }
673737 // Check that source files exists
674738 foreach ( $this->params['srcs'] as $source ) {
——@@ -684,7 +748,7 @@
685749 protected function doAttempt() {
686750 $status = Status::newGood();
687751 // Create a destination backup copy as needed
688 - if ( $this->checkDest ) {
 752+ if ( $this->destAlreadyExists ) {
689753 $status->merge( $this->checkAndBackupDest() );
690754 if ( !$status->isOK() ) {
691755 return $status;
——@@ -711,9 +775,13 @@
712776 return null; // defer this until we finish building the new file
713777 }
714778
715 - function storagePathsUsed() {
716 - return array_merge( $this->params['srcs'], $this->params['dst'] );
 779+ public function storagePathsRead() {
 780+ return $this->params['srcs'];
717781 }
 782+
 783+ public function storagePathsChanged() {
 784+ return array( $this->params['dst'] );
 785+ }
718786 }
719787
720788 /**
——@@ -762,7 +830,7 @@
763831 return $this->restoreSource();
764832 }
765833
766 - function storagePathsUsed() {
 834+ public function storagePathsChanged() {
767835 return array( $this->params['src'] );
768836 }
769837 }
——@@ -771,10 +839,6 @@
772840 * Placeholder operation that has no params and does nothing
773841 */
774842 class NullFileOp extends FileOp {
775 - protected function doPrecheck( array &$predicates ) {
776 - return Status::newGood();
777 - }
778 -
779843 protected function doAttempt() {
780844 return Status::newGood();
781845 }
Index: branches/FileBackend/phase3/includes/filerepo/backend/LockManager.php
——@@ -267,9 +267,8 @@
268268 * cache can be setup to track servers that recently missed queries; such
269269 * servers will not be trusted for obtaining locks.
270270 *
271 - * For performance, deadlock detection should be disabled and a small
272 - * lock-wait timeout should be set via server config. In innoDB, this can
273 - * done via the innodb_deadlock_detect and innodb_lock_wait_timeout settings.
 271+ * For performance, a small lock-wait timeout should be set via server config.
 272+ * In innoDB, this can done via the innodb_lock_wait_timeout setting.
274273 */
275274 class DBLockManager extends LockManager {
276275 /** @var Array Map of bucket indexes to peer sets */
Index: branches/FileBackend/phase3/includes/filerepo/backend/FileBackend.php
——@@ -254,10 +254,11 @@
255255 * Avoid using this function outside of FileBackendScopedLock.
256256 *
257257 * @param $paths Array Storage paths
 258+ * @param $type integer LockManager::LOCK_EX, LockManager::LOCK_SH
258259 * @return Status
259260 */
260 - final public function lockFiles( array $paths ) {
261 - return $this->lockManager->lock( $paths, LockManager::LOCK_EX );
 261+ final public function lockFiles( array $paths, $type ) {
 262+ return $this->lockManager->lock( $paths, $type );
262263 }
263264
264265 /**
——@@ -266,10 +267,11 @@
267268 * Avoid using this function outside of FileBackendScopedLock.
268269 *
269270 * @param $paths Array Storage paths
 271+ * @param $type integer LockManager::LOCK_EX, LockManager::LOCK_SH
270272 * @return Status
271273 */
272 - final public function unlockFiles( array $paths ) {
273 - return $this->lockManager->unlock( $paths, LockManager::LOCK_EX );
 274+ final public function unlockFiles( array $paths, $type ) {
 275+ return $this->lockManager->unlock( $paths, $type );
274276 }
275277
276278 /**
——@@ -281,11 +283,12 @@
282284 * the status updated. Unlock fatals will not change the status "OK" value.
283285 *
284286 * @param $paths Array Storage paths
 287+ * @param $type integer LockManager::LOCK_EX, LockManager::LOCK_SH
285288 * @param $status Status Status to update on lock/unlock
286289 * @return FileBackendScopedLock|null Returns null on failure
287290 */
288 - final public function getScopedFileLocks( array $paths, Status $status ) {
289 - return FileBackendScopedLock::factory( $this, $paths, $status );
 291+ final public function getScopedFileLocks( array $paths, $type, Status $status ) {
 292+ return FileBackendScopedLock::factory( $this, $paths, $type, $status );
290293 }
291294 }
292295
——@@ -499,7 +502,7 @@
500503 }
501504
502505 // Try to lock those files for the scope of this function...
503 - $scopedLock = $this->getScopedFileLocks( $filesToLock, $status );
 506+ $scopedLock = $this->getScopedFileLocks( $filesToLock, LockManager::LOCK_EX, $status );
504507 if ( !$status->isOK() ) {
505508 return $status; // abort
506509 }
——@@ -686,16 +689,21 @@
687690 protected $status;
688691 /** @var Array List of storage paths*/
689692 protected $paths;
 693+ protected $type; // integer lock type
690694
691695 /**
692696 * @param $backend FileBackendBase
693697 * @param $paths Array List of storage paths
 698+ * @param $type integer LockManager::LOCK_EX, LockManager::LOCK_SH
694699 * @param $status Status
695700 */
696 - protected function __construct( FileBackendBase $backend, array $paths, Status $status ) {
 701+ protected function __construct(
 702+ FileBackendBase $backend, array $paths, $type, Status $status
 703+ ) {
697704 $this->backend = $backend;
698705 $this->paths = $paths;
699706 $this->status = $status;
 707+ $this->type = $type;
700708 }
701709
702710 protected function __clone() {}
——@@ -708,21 +716,24 @@
709717 *
710718 * @param $backend FileBackendBase
711719 * @param $files Array List of storage paths
 720+ * @param $type integer LockManager::LOCK_EX, LockManager::LOCK_SH
712721 * @param $status Status
713722 * @return FileBackendScopedLock|null
714723 */
715 - public static function factory( FileBackendBase $backend, array $files, Status $status ) {
716 - $lockStatus = $backend->lockFiles( $files );
 724+ public static function factory(
 725+ FileBackendBase $backend, array $files, $type, Status $status
 726+ ) {
 727+ $lockStatus = $backend->lockFiles( $files, $type );
717728 $status->merge( $lockStatus );
718729 if ( $lockStatus->isOK() ) {
719 - return new self( $backend, $files, $status );
 730+ return new self( $backend, $files, $type, $status );
720731 }
721732 return null;
722733 }
723734
724735 function __destruct() {
725736 $wasOk = $this->status->isOK();
726 - $this->status->merge( $this->backend->unlockFiles( $this->paths ) );
 737+ $this->status->merge( $this->backend->unlockFiles( $this->paths, $this->type ) );
727738 if ( $wasOk ) {
728739 // Make sure status is OK, despite any unlockFiles() fatals
729740 $this->status->setResult( true, $this->status->value );

Comments

#Comment by Aaron Schulz (talk | contribs)   01:56, 9 December 2011

This also pushes lock type specifications up...which I will actually probably remove later (EX only). That kind slipped in there by mistake.

Status & tagging log