From 1f129a22cbc8546b89dd4b49b2f567c1f758a968 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 23 Nov 2013 13:51:32 -0800 Subject: [PATCH] filebackend: cleaned up the FileBackend constructor * Moved some of the graph construction work to FileBackendGroup. This helps the code in not depending on the rest of MW so much. * Updated tests and FileBackendMultiwrite, which are the only things directly constructing FileBackend objects. Change-Id: I188a053c70ce088ce34613d5db40e6708e3ea9b7 --- includes/filebackend/FileBackend.php | 20 ++++++++----------- includes/filebackend/FileBackendGroup.php | 8 ++++++++ .../filebackend/FileBackendMultiWrite.php | 2 +- tests/parser/parserTest.inc | 4 ++-- tests/phpunit/includes/LocalFileTest.php | 2 +- .../includes/filebackend/FileBackendTest.php | 11 +++++----- .../includes/filerepo/FileRepoTest.php | 2 +- .../includes/filerepo/StoreBatchTest.php | 4 +++- .../includes/media/ExifRotationTest.php | 2 +- .../includes/media/FormatMetadataTest.php | 2 +- tests/phpunit/includes/media/GIFTest.php | 2 +- tests/phpunit/includes/media/JpegTest.php | 2 +- tests/phpunit/includes/media/PNGTest.php | 2 +- tests/phpunit/includes/media/SVGTest.php | 2 +- .../phpunit/includes/parser/NewParserTest.php | 4 +++- .../phpunit/suites/UploadFromUrlTestSuite.php | 2 +- 16 files changed, 39 insertions(+), 32 deletions(-) diff --git a/includes/filebackend/FileBackend.php b/includes/filebackend/FileBackend.php index d042dc4ba3..f007cae5d9 100644 --- a/includes/filebackend/FileBackend.php +++ b/includes/filebackend/FileBackend.php @@ -91,13 +91,13 @@ abstract class FileBackend { * This name should not be changed after use (e.g. with journaling). * Note that the name is *not* used in actual container names. * - wikiId : Prefix to container names that is unique to this backend. - * If not provided, this defaults to the current wiki ID. * It should only consist of alphanumberic, '-', and '_' characters. * This ID is what avoids collisions if multiple logical backends * use the same storage system, so this should be set carefully. - * - lockManager : Registered name of a file lock manager to use. - * - fileJournal : File journal configuration; see FileJournal::factory(). - * Journals simply log changes to files stored in the backend. + * - lockManager : LockManager object to use for any file locking. + * If not provided, then no file locking will be enforced. + * - fileJournal : FileJournal object to use for logging changes to files. + * If not provided, then change journaling will be disabled. * - readOnly : Write operations are disallowed if this is a non-empty string. * It should be an explanation for the backend being read-only. * - parallelize : When to do file operations in parallel (when possible). @@ -110,16 +110,12 @@ abstract class FileBackend { if ( !preg_match( '!^[a-zA-Z0-9-_]{1,255}$!', $this->name ) ) { throw new MWException( "Backend name `{$this->name}` is invalid." ); } - $this->wikiId = isset( $config['wikiId'] ) - ? $config['wikiId'] - : wfWikiID(); // e.g. "my_wiki-en_" - $this->lockManager = ( $config['lockManager'] instanceof LockManager ) + $this->wikiId = $config['wikiId']; // e.g. "my_wiki-en_" + $this->lockManager = isset( $config['lockManager'] ) ? $config['lockManager'] - : LockManagerGroup::singleton( $this->wikiId )->get( $config['lockManager'] ); + : new NullLockManager( array() ); $this->fileJournal = isset( $config['fileJournal'] ) - ? ( ( $config['fileJournal'] instanceof FileJournal ) - ? $config['fileJournal'] - : FileJournal::factory( $config['fileJournal'], $this->name ) ) + ? $config['fileJournal'] : FileJournal::factory( array( 'class' => 'NullFileJournal' ), $this->name ); $this->readOnly = isset( $config['readOnly'] ) ? (string)$config['readOnly'] diff --git a/includes/filebackend/FileBackendGroup.php b/includes/filebackend/FileBackendGroup.php index 491424bf61..e451e3c102 100644 --- a/includes/filebackend/FileBackendGroup.php +++ b/includes/filebackend/FileBackendGroup.php @@ -152,6 +152,14 @@ class FileBackendGroup { if ( !isset( $this->backends[$name]['instance'] ) ) { $class = $this->backends[$name]['class']; $config = $this->backends[$name]['config']; + $config['wikiId'] = isset( $config['wikiId'] ) + ? $config['wikiId'] + : wfWikiID(); // e.g. "my_wiki-en_" + $config['lockManager'] = + LockManagerGroup::singleton( $config['wikiId'] )->get( $config['lockManager'] ); + $config['fileJournal'] = isset( $config['fileJournal'] ) + ? FileJournal::factory( $config['fileJournal'], $name ) + : FileJournal::factory( array( 'class' => 'NullFileJournal' ), $name ); $this->backends[$name]['instance'] = new $class( $config ); } diff --git a/includes/filebackend/FileBackendMultiWrite.php b/includes/filebackend/FileBackendMultiWrite.php index b3c46c6121..cb7a58e79e 100644 --- a/includes/filebackend/FileBackendMultiWrite.php +++ b/includes/filebackend/FileBackendMultiWrite.php @@ -125,8 +125,8 @@ class FileBackendMultiWrite extends FileBackend { // Alter certain sub-backend settings for sanity unset( $config['readOnly'] ); // use proxy backend setting unset( $config['fileJournal'] ); // use proxy backend journal + unset( $config['lockManager'] ); // lock under proxy backend $config['wikiId'] = $this->wikiId; // use the proxy backend wiki ID - $config['lockManager'] = 'nullLockManager'; // lock under proxy backend if ( !empty( $config['isMultiMaster'] ) ) { if ( $this->masterIndex >= 0 ) { throw new MWException( 'More than one master backend defined.' ); diff --git a/tests/parser/parserTest.inc b/tests/parser/parserTest.inc index 58ea1ed0b1..39fa09e24d 100644 --- a/tests/parser/parserTest.inc +++ b/tests/parser/parserTest.inc @@ -170,7 +170,7 @@ class ParserTest { 'transformVia404' => false, 'backend' => new FSFileBackend( array( 'name' => 'local-backend', - 'lockManager' => 'fsLockManager', + 'wikiId' => wfWikiId(), 'containerPaths' => array( 'local-public' => wfTempDir() . '/test-repo/public', 'local-thumb' => wfTempDir() . '/test-repo/thumb', @@ -738,7 +738,7 @@ class ParserTest { 'transformVia404' => false, 'backend' => new FSFileBackend( array( 'name' => 'local-backend', - 'lockManager' => 'fsLockManager', + 'wikiId' => wfWikiId(), 'containerPaths' => array( 'local-public' => $this->uploadDir, 'local-thumb' => $this->uploadDir . '/thumb', diff --git a/tests/phpunit/includes/LocalFileTest.php b/tests/phpunit/includes/LocalFileTest.php index 694f4aef35..d210ce51b5 100644 --- a/tests/phpunit/includes/LocalFileTest.php +++ b/tests/phpunit/includes/LocalFileTest.php @@ -21,7 +21,7 @@ class LocalFileTest extends MediaWikiTestCase { 'transformVia404' => false, 'backend' => new FSFileBackend( array( 'name' => 'local-backend', - 'lockManager' => 'fsLockManager', + 'wikiId' => wfWikiId(), 'containerPaths' => array( 'cont1' => "/testdir/local-backend/tempimages/cont1", 'cont2' => "/testdir/local-backend/tempimages/cont2" diff --git a/tests/phpunit/includes/filebackend/FileBackendTest.php b/tests/phpunit/includes/filebackend/FileBackendTest.php index fcfa724fc5..072cb7c524 100644 --- a/tests/phpunit/includes/filebackend/FileBackendTest.php +++ b/tests/phpunit/includes/filebackend/FileBackendTest.php @@ -37,6 +37,8 @@ class FileBackendTest extends MediaWikiTestCase { $useConfig['shardViaHashLevels'] = array( // test sharding 'unittest-cont1' => array( 'levels' => 1, 'base' => 16, 'repeat' => 1 ) ); + $useConfig['fileJournal'] = FileJournal::factory( $config['fileJournal'], $name ); + $useConfig['lockManager'] = LockManagerGroup::singleton()->get( $useConfig['lockManager'] ); $class = $useConfig['class']; self::$backendToUse = new $class( $useConfig ); $this->singleBackend = self::$backendToUse; @@ -44,9 +46,8 @@ class FileBackendTest extends MediaWikiTestCase { } else { $this->singleBackend = new FSFileBackend( array( 'name' => 'localtesting', - 'lockManager' => 'fsLockManager', - #'parallelize' => 'implicit', - 'wikiId' => wfWikiID() . $uniqueId, + 'lockManager' => LockManagerGroup::singleton()->get( 'fsLockManager' ), + 'wikiId' => wfWikiID(), 'containerPaths' => array( 'unittest-cont1' => "{$tmpPrefix}-localtesting-cont1", 'unittest-cont2' => "{$tmpPrefix}-localtesting-cont2" ) @@ -54,14 +55,13 @@ class FileBackendTest extends MediaWikiTestCase { } $this->multiBackend = new FileBackendMultiWrite( array( 'name' => 'localtesting', - 'lockManager' => 'fsLockManager', + 'lockManager' => LockManagerGroup::singleton()->get( 'fsLockManager' ), 'parallelize' => 'implicit', 'wikiId' => wfWikiId() . $uniqueId, 'backends' => array( array( 'name' => 'localmultitesting1', 'class' => 'FSFileBackend', - 'lockManager' => 'nullLockManager', 'containerPaths' => array( 'unittest-cont1' => "{$tmpPrefix}-localtestingmulti1-cont1", 'unittest-cont2' => "{$tmpPrefix}-localtestingmulti1-cont2" ), @@ -70,7 +70,6 @@ class FileBackendTest extends MediaWikiTestCase { array( 'name' => 'localmultitesting2', 'class' => 'FSFileBackend', - 'lockManager' => 'nullLockManager', 'containerPaths' => array( 'unittest-cont1' => "{$tmpPrefix}-localtestingmulti2-cont1", 'unittest-cont2' => "{$tmpPrefix}-localtestingmulti2-cont2" ), diff --git a/tests/phpunit/includes/filerepo/FileRepoTest.php b/tests/phpunit/includes/filerepo/FileRepoTest.php index e3a755673d..a196dca83b 100644 --- a/tests/phpunit/includes/filerepo/FileRepoTest.php +++ b/tests/phpunit/includes/filerepo/FileRepoTest.php @@ -46,7 +46,7 @@ class FileRepoTest extends MediaWikiTestCase { 'name' => 'FileRepoTestRepository', 'backend' => new FSFileBackend( array( 'name' => 'local-testing', - 'lockManager' => 'nullLockManager', + 'wikiId' => 'test_wiki', 'containerPaths' => array() ) ) ) ); diff --git a/tests/phpunit/includes/filerepo/StoreBatchTest.php b/tests/phpunit/includes/filerepo/StoreBatchTest.php index b33c1bbb39..8fb85b6946 100644 --- a/tests/phpunit/includes/filerepo/StoreBatchTest.php +++ b/tests/phpunit/includes/filerepo/StoreBatchTest.php @@ -25,13 +25,15 @@ class StoreBatchTest extends MediaWikiTestCase { $useConfig = $conf; } } + $useConfig['lockManager'] = LockManagerGroup::singleton()->get( $useConfig['lockManager'] ); + unset( $useConfig['fileJournal'] ); $useConfig['name'] = 'local-testing'; // swap name $class = $useConfig['class']; $backend = new $class( $useConfig ); } else { $backend = new FSFileBackend( array( 'name' => 'local-testing', - 'lockManager' => 'nullLockManager', + 'wikiId' => wfWikiID(), 'containerPaths' => array( 'unittests-public' => "{$tmpPrefix}-public", 'unittests-thumb' => "{$tmpPrefix}-thumb", diff --git a/tests/phpunit/includes/media/ExifRotationTest.php b/tests/phpunit/includes/media/ExifRotationTest.php index f4f415428b..118dc851fd 100644 --- a/tests/phpunit/includes/media/ExifRotationTest.php +++ b/tests/phpunit/includes/media/ExifRotationTest.php @@ -24,7 +24,7 @@ class ExifRotationTest extends MediaWikiTestCase { 'url' => 'http://localhost/thumbtest', 'backend' => new FSFileBackend( array( 'name' => 'localtesting', - 'lockManager' => 'nullLockManager', + 'wikiId' => wfWikiId(), 'containerPaths' => array( 'temp-thumb' => $tmpDir, 'data' => $filePath ) ) ) ) ); diff --git a/tests/phpunit/includes/media/FormatMetadataTest.php b/tests/phpunit/includes/media/FormatMetadataTest.php index a4f71db547..44d8f004f0 100644 --- a/tests/phpunit/includes/media/FormatMetadataTest.php +++ b/tests/phpunit/includes/media/FormatMetadataTest.php @@ -16,7 +16,7 @@ class FormatMetadataTest extends MediaWikiTestCase { $filePath = __DIR__ . '/../../data/media'; $this->backend = new FSFileBackend( array( 'name' => 'localtesting', - 'lockManager' => 'nullLockManager', + 'wikiId' => wfWikiId(), 'containerPaths' => array( 'data' => $filePath ) ) ); $this->repo = new FSRepo( array( diff --git a/tests/phpunit/includes/media/GIFTest.php b/tests/phpunit/includes/media/GIFTest.php index 4350cbbad3..7bd04b7c90 100644 --- a/tests/phpunit/includes/media/GIFTest.php +++ b/tests/phpunit/includes/media/GIFTest.php @@ -16,7 +16,7 @@ class GIFHandlerTest extends MediaWikiTestCase { $this->filePath = __DIR__ . '/../../data/media'; $this->backend = new FSFileBackend( array( 'name' => 'localtesting', - 'lockManager' => 'nullLockManager', + 'wikiId' => wfWikiId(), 'containerPaths' => array( 'data' => $this->filePath ) ) ); $this->repo = new FSRepo( array( diff --git a/tests/phpunit/includes/media/JpegTest.php b/tests/phpunit/includes/media/JpegTest.php index bff64bbe74..a5bf1dcf4a 100644 --- a/tests/phpunit/includes/media/JpegTest.php +++ b/tests/phpunit/includes/media/JpegTest.php @@ -18,7 +18,7 @@ class JpegTest extends MediaWikiTestCase { $this->backend = new FSFileBackend( array( 'name' => 'localtesting', - 'lockManager' => 'nullLockManager', + 'wikiId' => wfWikiId(), 'containerPaths' => array( 'data' => $this->filePath ) ) ); $this->repo = new FSRepo( array( diff --git a/tests/phpunit/includes/media/PNGTest.php b/tests/phpunit/includes/media/PNGTest.php index 2cb7426062..a47dc4a5a8 100644 --- a/tests/phpunit/includes/media/PNGTest.php +++ b/tests/phpunit/includes/media/PNGTest.php @@ -16,7 +16,7 @@ class PNGHandlerTest extends MediaWikiTestCase { $this->filePath = __DIR__ . '/../../data/media'; $this->backend = new FSFileBackend( array( 'name' => 'localtesting', - 'lockManager' => 'nullLockManager', + 'wikiId' => wfWikiId(), 'containerPaths' => array( 'data' => $this->filePath ) ) ); $this->repo = new FSRepo( array( diff --git a/tests/phpunit/includes/media/SVGTest.php b/tests/phpunit/includes/media/SVGTest.php index b28ee56c9b..eb790986ad 100644 --- a/tests/phpunit/includes/media/SVGTest.php +++ b/tests/phpunit/includes/media/SVGTest.php @@ -10,7 +10,7 @@ class SVGTest extends MediaWikiTestCase { $this->backend = new FSFileBackend( array( 'name' => 'localtesting', - 'lockManager' => 'nullLockManager', + 'wikiId' => wfWikiId(), 'containerPaths' => array( 'data' => $this->filePath ) ) ); $this->repo = new FSRepo( array( diff --git a/tests/phpunit/includes/parser/NewParserTest.php b/tests/phpunit/includes/parser/NewParserTest.php index eac4de5c0c..124b477ffc 100644 --- a/tests/phpunit/includes/parser/NewParserTest.php +++ b/tests/phpunit/includes/parser/NewParserTest.php @@ -306,6 +306,8 @@ class NewParserTest extends MediaWikiTestCase { } } $useConfig['name'] = 'local-backend'; // swap name + unset( $useConfig['lockManager'] ); + unset( $useConfig['fileJournal'] ); $class = $conf['class']; self::$backendToUse = new $class( $useConfig ); $backend = self::$backendToUse; @@ -316,7 +318,7 @@ class NewParserTest extends MediaWikiTestCase { # informations. $backend = new MockFileBackend( array( 'name' => 'local-backend', - 'lockManager' => 'nullLockManager', + 'wikiId' => wfWikiId(), 'containerPaths' => array( 'local-public' => "$uploadDir", 'local-thumb' => "$uploadDir/thumb", diff --git a/tests/phpunit/suites/UploadFromUrlTestSuite.php b/tests/phpunit/suites/UploadFromUrlTestSuite.php index 7eb599e393..fb7780d0ec 100644 --- a/tests/phpunit/suites/UploadFromUrlTestSuite.php +++ b/tests/phpunit/suites/UploadFromUrlTestSuite.php @@ -36,7 +36,7 @@ class UploadFromUrlTestSuite extends PHPUnit_Framework_TestSuite { 'transformVia404' => false, 'backend' => new FSFileBackend( array( 'name' => 'local-backend', - 'lockManager' => 'fsLockManager', + 'wikiId' => wfWikiId(), 'containerPaths' => array( 'local-public' => wfTempDir() . '/test-repo/public', 'local-thumb' => wfTempDir() . '/test-repo/thumb', -- 2.20.1