From 64a95000788ded571c74a419ec02a3c6c6d9f823 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 5 Jun 2019 14:33:18 -0700 Subject: [PATCH] Add "bin/file migrate" options to support import of a local-disk backup for Phacility instances Summary: Ref T13306. Currently, there's no easy way to import a third-party local-disk file dump into a Phacility instance. Add some more options to `bin/files migrate` to support this. In particular, this enables: ``` $ ./bin/files --from-engine local-disk --engine amazon-s3 --local-disk-source path/to/backup ``` ...to import these files into S3 directly. These are general-purpose options and theoretically useful in other use cases, although realistically those cases are probably very rare. Test Plan: Used `bin/files` with the new options to move files in and out of local disk storage in an arbitrary backup directory. Got clean exports/imports. Reviewers: amckinley Maniphest Tasks: T13306 Differential Revision: https://secure.phabricator.com/D20571 --- ...bricatorFilesManagementMigrateWorkflow.php | 34 ++++++++++++++ .../PhabricatorFilesManagementWorkflow.php | 47 ++++++++++++++----- .../files/query/PhabricatorFileQuery.php | 13 +++++ 3 files changed, 83 insertions(+), 11 deletions(-) diff --git a/src/applications/files/management/PhabricatorFilesManagementMigrateWorkflow.php b/src/applications/files/management/PhabricatorFilesManagementMigrateWorkflow.php index 909e45c31f..58d5155aed 100644 --- a/src/applications/files/management/PhabricatorFilesManagementMigrateWorkflow.php +++ b/src/applications/files/management/PhabricatorFilesManagementMigrateWorkflow.php @@ -46,10 +46,44 @@ final class PhabricatorFilesManagementMigrateWorkflow 'name' => 'names', 'wildcard' => true, ), + array( + 'name' => 'from-engine', + 'param' => 'engine', + 'help' => pht('Migrate files from the named storage engine.'), + ), + array( + 'name' => 'local-disk-source', + 'param' => 'path', + 'help' => pht( + 'When migrating from a local disk source, use the specified '. + 'path as the root directory.'), + ), )); } public function execute(PhutilArgumentParser $args) { + + // See T13306. This flag allows you to import files from a backup of + // local disk storage into some other engine. When the caller provides + // the flag, we override the local disk engine configuration and treat + // it as though it is configured to use the specified location. + + $local_disk_source = $args->getArg('local-disk-source'); + if (strlen($local_disk_source)) { + $path = Filesystem::resolvePath($local_disk_source); + try { + Filesystem::assertIsDirectory($path); + } catch (FilesystemException $ex) { + throw new PhutilArgumentUsageException( + pht( + 'The "--local-disk-source" argument must point to a valid, '. + 'readable directory on local disk.')); + } + + $env = PhabricatorEnv::beginScopedEnv(); + $env->overrideEnvConfig('storage.local-disk.path', $path); + } + $target_key = $args->getArg('engine'); if (!$target_key) { throw new PhutilArgumentUsageException( diff --git a/src/applications/files/management/PhabricatorFilesManagementWorkflow.php b/src/applications/files/management/PhabricatorFilesManagementWorkflow.php index e94fa1d96a..44d43dc66a 100644 --- a/src/applications/files/management/PhabricatorFilesManagementWorkflow.php +++ b/src/applications/files/management/PhabricatorFilesManagementWorkflow.php @@ -4,23 +4,48 @@ abstract class PhabricatorFilesManagementWorkflow extends PhabricatorManagementWorkflow { protected function buildIterator(PhutilArgumentParser $args) { + $viewer = $this->getViewer(); $names = $args->getArg('names'); - if ($args->getArg('all')) { - if ($names) { - throw new PhutilArgumentUsageException( - pht( - 'Specify either a list of files or `%s`, but not both.', - '--all')); - } - return new LiskMigrationIterator(new PhabricatorFile()); + $is_all = $args->getArg('all'); + $from_engine = $args->getArg('from-engine'); + + $any_constraint = ($from_engine || $names); + + if (!$is_all && !$any_constraint) { + throw new PhutilArgumentUsageException( + pht( + 'Use "--all" to migrate all files, or choose files to migrate '. + 'with "--names" or "--from-engine".')); } + if ($is_all && $any_constraint) { + throw new PhutilArgumentUsageException( + pht( + 'You can not migrate all files with "--all" and also migrate only '. + 'a subset of files with "--from-engine" or "--names".')); + } + + // If we're migrating specific named files, convert the names into IDs + // first. + $ids = null; if ($names) { - return $this->loadFilesWithNames($names); + $files = $this->loadFilesWithNames($names); + $ids = mpull($files, 'getID'); } - return null; + $query = id(new PhabricatorFileQuery()) + ->setViewer($viewer); + + if ($ids) { + $query->withIDs($ids); + } + + if ($from_engine) { + $query->withStorageEngines(array($from_engine)); + } + + return new PhabricatorQueryIterator($query); } protected function loadFilesWithNames(array $names) { @@ -36,7 +61,7 @@ abstract class PhabricatorFilesManagementWorkflow if (empty($files[$name])) { throw new PhutilArgumentUsageException( pht( - "No file '%s' exists!", + 'No file "%s" exists.', $name)); } } diff --git a/src/applications/files/query/PhabricatorFileQuery.php b/src/applications/files/query/PhabricatorFileQuery.php index 4205ab5c5d..c19574acaa 100644 --- a/src/applications/files/query/PhabricatorFileQuery.php +++ b/src/applications/files/query/PhabricatorFileQuery.php @@ -19,6 +19,7 @@ final class PhabricatorFileQuery private $needTransforms; private $builtinKeys; private $isBuiltin; + private $storageEngines; public function withIDs(array $ids) { $this->ids = $ids; @@ -137,6 +138,11 @@ final class PhabricatorFileQuery $ngrams); } + public function withStorageEngines(array $engines) { + $this->storageEngines = $engines; + return $this; + } + public function showOnlyExplicitUploads($explicit_uploads) { $this->explicitUploads = $explicit_uploads; return $this; @@ -469,6 +475,13 @@ final class PhabricatorFileQuery } } + if ($this->storageEngines !== null) { + $where[] = qsprintf( + $conn, + 'storageEngine IN (%Ls)', + $this->storageEngines); + } + return $where; }