diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1648,7 +1648,6 @@ 'PhabricatorDataNotAttachedException' => 'infrastructure/storage/lisk/PhabricatorDataNotAttachedException.php', 'PhabricatorDatabaseSetupCheck' => 'applications/config/check/PhabricatorDatabaseSetupCheck.php', 'PhabricatorDebugController' => 'applications/system/controller/PhabricatorDebugController.php', - 'PhabricatorDefaultFileStorageEngineSelector' => 'applications/files/engineselector/PhabricatorDefaultFileStorageEngineSelector.php', 'PhabricatorDefaultSearchEngineSelector' => 'applications/search/selector/PhabricatorDefaultSearchEngineSelector.php', 'PhabricatorDestructibleInterface' => 'applications/system/interface/PhabricatorDestructibleInterface.php', 'PhabricatorDestructionEngine' => 'applications/system/engine/PhabricatorDestructionEngine.php', @@ -4850,7 +4849,6 @@ 'PhabricatorDataNotAttachedException' => 'Exception', 'PhabricatorDatabaseSetupCheck' => 'PhabricatorSetupCheck', 'PhabricatorDebugController' => 'PhabricatorController', - 'PhabricatorDefaultFileStorageEngineSelector' => 'PhabricatorFileStorageEngineSelector', 'PhabricatorDefaultSearchEngineSelector' => 'PhabricatorSearchEngineSelector', 'PhabricatorDestructionEngine' => 'Phobject', 'PhabricatorDeveloperConfigOptions' => 'PhabricatorApplicationConfigOptions', diff --git a/src/applications/files/config/PhabricatorFilesConfigOptions.php b/src/applications/files/config/PhabricatorFilesConfigOptions.php --- a/src/applications/files/config/PhabricatorFilesConfigOptions.php +++ b/src/applications/files/config/PhabricatorFilesConfigOptions.php @@ -142,7 +142,7 @@ $this->newOption( 'storage.engine-selector', 'class', - 'PhabricatorDefaultFileStorageEngineSelector') + 'PhabricatorFileStorageEngineSelector') ->setBaseClass('PhabricatorFileStorageEngineSelector') ->setSummary(pht('Storage engine selector.')) ->setDescription( diff --git a/src/applications/files/engine/PhabricatorFileStorageEngine.php b/src/applications/files/engine/PhabricatorFileStorageEngine.php --- a/src/applications/files/engine/PhabricatorFileStorageEngine.php +++ b/src/applications/files/engine/PhabricatorFileStorageEngine.php @@ -40,6 +40,10 @@ */ abstract public function getEngineIdentifier(); + /* abstract */ public function shouldBeEnabled($data, array $params) { + return false; + } + /* -( Managing File Data )------------------------------------------------- */ diff --git a/src/applications/files/engine/PhabricatorLocalDiskFileStorageEngine.php b/src/applications/files/engine/PhabricatorLocalDiskFileStorageEngine.php --- a/src/applications/files/engine/PhabricatorLocalDiskFileStorageEngine.php +++ b/src/applications/files/engine/PhabricatorLocalDiskFileStorageEngine.php @@ -22,6 +22,9 @@ return 'local-disk'; } + public function shouldBeEnabled($data, array $params) { + return (bool)PhabricatorEnv::getEnvConfig('storage.local-disk.path'); + } /** * Write the file data to local disk. Returns the relative path as the diff --git a/src/applications/files/engine/PhabricatorMySQLFileStorageEngine.php b/src/applications/files/engine/PhabricatorMySQLFileStorageEngine.php --- a/src/applications/files/engine/PhabricatorMySQLFileStorageEngine.php +++ b/src/applications/files/engine/PhabricatorMySQLFileStorageEngine.php @@ -25,6 +25,15 @@ return 'blob'; } + public function shouldBeEnabled($data, array $params) { + $length = strlen($data); + + $mysql_key = 'storage.mysql-engine.max-size'; + $mysql_limit = PhabricatorEnv::getEnvConfig($mysql_key); + + return $mysql_limit && $length <= $mysql_limit; + } + /** * Write file data into the big blob store table in MySQL. Returns the row diff --git a/src/applications/files/engine/PhabricatorS3FileStorageEngine.php b/src/applications/files/engine/PhabricatorS3FileStorageEngine.php --- a/src/applications/files/engine/PhabricatorS3FileStorageEngine.php +++ b/src/applications/files/engine/PhabricatorS3FileStorageEngine.php @@ -20,6 +20,10 @@ return 'amazon-s3'; } + public function shouldBeEnabled($data, array $params) { + return (bool)PhabricatorEnv::getEnvConfig('storage.s3.bucket'); + } + /** * Writes file data into Amazon S3. diff --git a/src/applications/files/engine/PhabricatorTestStorageEngine.php b/src/applications/files/engine/PhabricatorTestStorageEngine.php --- a/src/applications/files/engine/PhabricatorTestStorageEngine.php +++ b/src/applications/files/engine/PhabricatorTestStorageEngine.php @@ -3,8 +3,7 @@ /** * Test storage engine. Does not actually store files. Used for unit tests. */ -final class PhabricatorTestStorageEngine - extends PhabricatorFileStorageEngine { +final class PhabricatorTestStorageEngine extends PhabricatorFileStorageEngine { private static $storage = array(); private static $nextHandle = 1; @@ -13,6 +12,10 @@ return 'unit-test'; } + public function shouldBeEnabled($data, array $params) { + return true; + } + public function writeFile($data, array $params) { AphrontWriteGuard::willWrite(); self::$storage[self::$nextHandle] = $data; diff --git a/src/applications/files/engineselector/PhabricatorDefaultFileStorageEngineSelector.php b/src/applications/files/engineselector/PhabricatorDefaultFileStorageEngineSelector.php deleted file mode 100644 --- a/src/applications/files/engineselector/PhabricatorDefaultFileStorageEngineSelector.php +++ /dev/null @@ -1,52 +0,0 @@ -<?php - -/** - * Default storage engine selector. See - * @{class:PhabricatorFileStorageEngineSelector} and @{article:File Storage - * Technical Documentation} for more information. - */ -final class PhabricatorDefaultFileStorageEngineSelector - extends PhabricatorFileStorageEngineSelector { - - /** - * Select viable default storage engines according to configuration. We'll - * select the MySQL and Local Disk storage engines if they are configured - * to allow a given file. - */ - public function selectStorageEngines($data, array $params) { - $length = strlen($data); - - $mysql_key = 'storage.mysql-engine.max-size'; - $mysql_limit = PhabricatorEnv::getEnvConfig($mysql_key); - - $engines = array(); - if ($mysql_limit && $length <= $mysql_limit) { - $engines[] = new PhabricatorMySQLFileStorageEngine(); - } - - $local_key = 'storage.local-disk.path'; - $local_path = PhabricatorEnv::getEnvConfig($local_key); - if ($local_path) { - $engines[] = new PhabricatorLocalDiskFileStorageEngine(); - } - - $s3_key = 'storage.s3.bucket'; - if (PhabricatorEnv::getEnvConfig($s3_key)) { - $engines[] = new PhabricatorS3FileStorageEngine(); - } - - if ($mysql_limit && empty($engines)) { - // If we return no engines, an exception will be thrown but it will be - // a little vague ("No valid storage engines"). Since this is a default - // case, throw a more specific exception. - throw new Exception( - 'This file exceeds the configured MySQL storage engine filesize '. - 'limit, but no other storage engines are configured. Increase the '. - 'MySQL storage engine limit or configure a storage engine suitable '. - 'for larger files.'); - } - - return $engines; - } - -} diff --git a/src/applications/files/engineselector/PhabricatorFileStorageEngineSelector.php b/src/applications/files/engineselector/PhabricatorFileStorageEngineSelector.php --- a/src/applications/files/engineselector/PhabricatorFileStorageEngineSelector.php +++ b/src/applications/files/engineselector/PhabricatorFileStorageEngineSelector.php @@ -6,9 +6,10 @@ * of suitable @{class:PhabricatorFileStorageEngine}s. For more information, * see @{article:File Storage Technical Documentation}. * + * @concrete-extensible * @task select Selecting Storage Engines */ -abstract class PhabricatorFileStorageEngineSelector { +class PhabricatorFileStorageEngineSelector { final public function __construct() { // <empty> @@ -41,6 +42,28 @@ * preference. * @task select */ - abstract public function selectStorageEngines($data, array $params); + public function selectStorageEngines($data, array $params) { + $engines = id(new PhutilSymbolLoader()) + ->setAncestorClass('PhabricatorFileStorageEngine') + ->loadObjects(); + $engines = mfilter($engines, 'shouldBeEnabled'); + + $mysql_key = 'storage.mysql-engine.max-size'; + $mysql_limit = PhabricatorEnv::getEnvConfig($mysql_key); + + if ($mysql_limit && empty($engines)) { + // If we return no engines, an exception will be thrown but it will be + // a little vague ("No valid storage engines"). Since this is a default + // case, throw a more specific exception. + throw new Exception( + pht( + 'This file exceeds the configured MySQL storage engine filesize '. + 'limit, but no other storage engines are configured. Increase the '. + 'MySQL storage engine limit or configure a storage engine suitable '. + 'for larger files.')); + } + + return $engines; + } } diff --git a/src/docs/tech/files.diviner b/src/docs/tech/files.diviner --- a/src/docs/tech/files.diviner +++ b/src/docs/tech/files.diviner @@ -17,8 +17,8 @@ engine. When writing data, a @{class:PhabricatorFileStorageEngineSelector} is -instantiated (by default, @{class:PhabricatorDefaultFileStorageEngineSelector}, -but you can change this by setting the ##storage.engine-selector## key in your +instantiated (by default, @{class:PhabricatorFileStorageEngineSelector}, but +you can change this by setting the ##storage.engine-selector## key in your configuration). The selector returns a list of satisfactory @{class:PhabricatorFileStorageEngine}s, in order of preference.