From: Faraz Samapoor Date: Tue, 4 Jul 2023 18:43:32 +0000 (+0330) Subject: Refactors files app commands. X-Git-Tag: v28.0.0beta1~609^2~3 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=5d242aa2f86f003bb5f0400188a440d2d5eea68f;p=nextcloud-server.git Refactors files app commands. To improve code readability. Signed-off-by: Faraz Samapoor --- diff --git a/apps/files/lib/Command/Delete.php b/apps/files/lib/Command/Delete.php index da535568702..f491b67ae1f 100644 --- a/apps/files/lib/Command/Delete.php +++ b/apps/files/lib/Command/Delete.php @@ -35,10 +35,9 @@ use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Question\ConfirmationQuestion; class Delete extends Command { - private FileUtils $fileUtils; - - public function __construct(FileUtils $fileUtils) { - $this->fileUtils = $fileUtils; + public function __construct( + private FileUtils $fileUtils, + ) { parent::__construct(); } @@ -58,7 +57,7 @@ class Delete extends Command { if (!$node) { $output->writeln("file $fileInput not found"); - return 1; + return self::FAILURE; } $deleteConfirmed = $force; @@ -72,7 +71,7 @@ class Delete extends Command { $question = new ConfirmationQuestion("$fileInput in a shared file, do you want to unshare the file from $user instead of deleting the source file? [Y/n] ", true); if ($helper->ask($input, $output, $question)) { $storage->unshareStorage(); - return 0; + return self::SUCCESS; } else { $node = $storage->getShare()->getNode(); $output->writeln(""); @@ -110,7 +109,6 @@ class Delete extends Command { } } - return 0; + return self::SUCCESS; } - } diff --git a/apps/files/lib/Command/DeleteOrphanedFiles.php b/apps/files/lib/Command/DeleteOrphanedFiles.php index e3305fe3b9b..4b7179271f5 100644 --- a/apps/files/lib/Command/DeleteOrphanedFiles.php +++ b/apps/files/lib/Command/DeleteOrphanedFiles.php @@ -35,17 +35,13 @@ use Symfony\Component\Console\Output\OutputInterface; class DeleteOrphanedFiles extends Command { public const CHUNK_SIZE = 200; - /** - * @var IDBConnection - */ - protected $connection; - - public function __construct(IDBConnection $connection) { - $this->connection = $connection; + public function __construct( + protected IDBConnection $connection, + ) { parent::__construct(); } - protected function configure() { + protected function configure(): void { $this ->setName('files:cleanup') ->setDescription('cleanup filecache'); @@ -81,10 +77,10 @@ class DeleteOrphanedFiles extends Command { $deletedMounts = $this->cleanupOrphanedMounts(); $output->writeln("$deletedMounts orphaned mount entries deleted"); - return 0; + return self::SUCCESS; } - private function cleanupOrphanedMounts() { + private function cleanupOrphanedMounts(): int { $deletedEntries = 0; $query = $this->connection->getQueryBuilder(); diff --git a/apps/files/lib/Command/Get.php b/apps/files/lib/Command/Get.php index 7bdb4cb59ee..6b21f94e8cf 100644 --- a/apps/files/lib/Command/Get.php +++ b/apps/files/lib/Command/Get.php @@ -32,10 +32,9 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; class Get extends Command { - private FileUtils $fileUtils; - - public function __construct(FileUtils $fileUtils) { - $this->fileUtils = $fileUtils; + public function __construct( + private FileUtils $fileUtils, + ) { parent::__construct(); } @@ -54,36 +53,35 @@ class Get extends Command { if (!$node) { $output->writeln("file $fileInput not found"); - return 1; + return self::FAILURE; } - if ($node instanceof File) { - $isTTY = stream_isatty(STDOUT); - if ($outputName === null && $isTTY && $node->getMimePart() !== 'text') { - $output->writeln([ - "Warning: Binary output can mess up your terminal", - " Use occ files:get $fileInput - to output it to the terminal anyway", - " Or occ files:get $fileInput to save to a file instead" - ]); - return 1; - } - $source = $node->fopen('r'); - if (!$source) { - $output->writeln("Failed to open $fileInput for reading"); - return 1; - } - $target = ($outputName === null || $outputName === '-') ? STDOUT : fopen($outputName, 'w'); - if (!$target) { - $output->writeln("Failed to open $outputName for reading"); - return 1; - } - - stream_copy_to_stream($source, $target); - return 0; - } else { + if (!($node instanceof File)) { $output->writeln("$fileInput is a directory"); - return 1; + return self::FAILURE; + } + + $isTTY = stream_isatty(STDOUT); + if ($outputName === null && $isTTY && $node->getMimePart() !== 'text') { + $output->writeln([ + "Warning: Binary output can mess up your terminal", + " Use occ files:get $fileInput - to output it to the terminal anyway", + " Or occ files:get $fileInput to save to a file instead" + ]); + return self::FAILURE; + } + $source = $node->fopen('r'); + if (!$source) { + $output->writeln("Failed to open $fileInput for reading"); + return self::FAILURE; + } + $target = ($outputName === null || $outputName === '-') ? STDOUT : fopen($outputName, 'w'); + if (!$target) { + $output->writeln("Failed to open $outputName for reading"); + return self::FAILURE; } - } + stream_copy_to_stream($source, $target); + return self::SUCCESS; + } } diff --git a/apps/files/lib/Command/Object/Delete.php b/apps/files/lib/Command/Object/Delete.php index 9742778e271..5000eeb6ee5 100644 --- a/apps/files/lib/Command/Object/Delete.php +++ b/apps/files/lib/Command/Object/Delete.php @@ -34,10 +34,9 @@ use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Question\ConfirmationQuestion; class Delete extends Command { - private ObjectUtil $objectUtils; - - public function __construct(ObjectUtil $objectUtils) { - $this->objectUtils = $objectUtils; + public function __construct( + private ObjectUtil $objectUtils, + ) { parent::__construct(); } @@ -73,6 +72,6 @@ class Delete extends Command { if ($helper->ask($input, $output, $question)) { $objectStore->deleteObject($object); } - return 0; + return self::SUCCESS; } } diff --git a/apps/files/lib/Command/Object/Get.php b/apps/files/lib/Command/Object/Get.php index c07a64b20e2..cad1f3d98ac 100644 --- a/apps/files/lib/Command/Object/Get.php +++ b/apps/files/lib/Command/Object/Get.php @@ -31,10 +31,9 @@ use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; class Get extends Command { - private ObjectUtil $objectUtils; - - public function __construct(ObjectUtil $objectUtils) { - $this->objectUtils = $objectUtils; + public function __construct( + private ObjectUtil $objectUtils, + ) { parent::__construct(); } @@ -52,29 +51,29 @@ class Get extends Command { $outputName = $input->getArgument('output'); $objectStore = $this->objectUtils->getObjectStore($input->getOption("bucket"), $output); if (!$objectStore) { - return 1; + return self::FAILURE; } if (!$objectStore->objectExists($object)) { $output->writeln("Object $object does not exist"); - return 1; - } else { - try { - $source = $objectStore->readObject($object); - } catch (\Exception $e) { - $msg = $e->getMessage(); - $output->writeln("Failed to read $object from object store: $msg"); - return 1; - } - $target = $outputName === '-' ? STDOUT : fopen($outputName, 'w'); - if (!$target) { - $output->writeln("Failed to open $outputName for writing"); - return 1; - } + return self::FAILURE; + } - stream_copy_to_stream($source, $target); - return 0; + try { + $source = $objectStore->readObject($object); + } catch (\Exception $e) { + $msg = $e->getMessage(); + $output->writeln("Failed to read $object from object store: $msg"); + return self::FAILURE; } + $target = $outputName === '-' ? STDOUT : fopen($outputName, 'w'); + if (!$target) { + $output->writeln("Failed to open $outputName for writing"); + return self::FAILURE; + } + + stream_copy_to_stream($source, $target); + return self::SUCCESS; } } diff --git a/apps/files/lib/Command/Object/ObjectUtil.php b/apps/files/lib/Command/Object/ObjectUtil.php index b7359dfa193..5d278cdf668 100644 --- a/apps/files/lib/Command/Object/ObjectUtil.php +++ b/apps/files/lib/Command/Object/ObjectUtil.php @@ -30,12 +30,10 @@ use OCP\IDBConnection; use Symfony\Component\Console\Output\OutputInterface; class ObjectUtil { - private IConfig $config; - private IDBConnection $connection; - - public function __construct(IConfig $config, IDBConnection $connection) { - $this->config = $config; - $this->connection = $connection; + public function __construct( + private IConfig $config, + private IDBConnection $connection, + ) { } private function getObjectStoreConfig(): ?array { @@ -50,9 +48,9 @@ class ObjectUtil { $config['multibucket'] = false; } return $config; - } else { - return null; } + + return null; } public function getObjectStore(?string $bucket, OutputInterface $output): ?IObjectStore { @@ -91,20 +89,21 @@ class ObjectUtil { * Check if an object is referenced in the database */ public function objectExistsInDb(string $object): int|false { - if (str_starts_with($object, 'urn:oid:')) { - $fileId = (int)substr($object, strlen('urn:oid:')); - $query = $this->connection->getQueryBuilder(); - $query->select('fileid') - ->from('filecache') - ->where($query->expr()->eq('fileid', $query->createNamedParameter($fileId, IQueryBuilder::PARAM_INT))); - $result = $query->executeQuery(); - if ($result->fetchOne() !== false) { - return $fileId; - } else { - return false; - } - } else { + if (!str_starts_with($object, 'urn:oid:')) { return false; } + + $fileId = (int)substr($object, strlen('urn:oid:')); + $query = $this->connection->getQueryBuilder(); + $query->select('fileid') + ->from('filecache') + ->where($query->expr()->eq('fileid', $query->createNamedParameter($fileId, IQueryBuilder::PARAM_INT))); + $result = $query->executeQuery(); + + if ($result->fetchOne() === false) { + return false; + } + + return $fileId; } } diff --git a/apps/files/lib/Command/Object/Put.php b/apps/files/lib/Command/Object/Put.php index dabc2b1ffc3..852d7c2225e 100644 --- a/apps/files/lib/Command/Object/Put.php +++ b/apps/files/lib/Command/Object/Put.php @@ -33,12 +33,10 @@ use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Question\ConfirmationQuestion; class Put extends Command { - private ObjectUtil $objectUtils; - private IMimeTypeDetector $mimeTypeDetector; - - public function __construct(ObjectUtil $objectUtils, IMimeTypeDetector $mimeTypeDetector) { - $this->objectUtils = $objectUtils; - $this->mimeTypeDetector = $mimeTypeDetector; + public function __construct( + private ObjectUtil $objectUtils, + private IMimeTypeDetector $mimeTypeDetector, + ) { parent::__construct(); } @@ -75,10 +73,10 @@ class Put extends Command { $source = $inputName === '-' ? STDIN : fopen($inputName, 'r'); if (!$source) { $output->writeln("Failed to open $inputName"); - return 1; + return self::FAILURE; } $objectStore->writeObject($object, $source, $this->mimeTypeDetector->detectPath($inputName)); - return 0; + return self::SUCCESS; } } diff --git a/apps/files/lib/Command/Put.php b/apps/files/lib/Command/Put.php index f89ab8fb436..73261e47fb6 100644 --- a/apps/files/lib/Command/Put.php +++ b/apps/files/lib/Command/Put.php @@ -34,12 +34,10 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; class Put extends Command { - private FileUtils $fileUtils; - private IRootFolder $rootFolder; - - public function __construct(FileUtils $fileUtils, IRootFolder $rootFolder) { - $this->fileUtils = $fileUtils; - $this->rootFolder = $rootFolder; + public function __construct( + private FileUtils $fileUtils, + private IRootFolder $rootFolder, + ) { parent::__construct(); } @@ -58,29 +56,28 @@ class Put extends Command { if ($node instanceof Folder) { $output->writeln("$fileOutput is a folder"); - return 1; + return self::FAILURE; } if (!$node and is_numeric($fileOutput)) { $output->writeln("$fileOutput not found"); - return 1; + return self::FAILURE; } $source = ($inputName === null || $inputName === '-') ? STDIN : fopen($inputName, 'r'); if (!$source) { $output->writeln("Failed to open $inputName"); - return 1; + return self::FAILURE; } if ($node instanceof File) { $target = $node->fopen('w'); if (!$target) { $output->writeln("Failed to open $fileOutput"); - return 1; + return self::FAILURE; } stream_copy_to_stream($source, $target); } else { $this->rootFolder->newFile($fileOutput, $source); } - return 0; + return self::SUCCESS; } - } diff --git a/apps/files/lib/Command/RepairTree.php b/apps/files/lib/Command/RepairTree.php index 521fe3d6a4a..7e7c40b4e00 100644 --- a/apps/files/lib/Command/RepairTree.php +++ b/apps/files/lib/Command/RepairTree.php @@ -33,17 +33,13 @@ use Symfony\Component\Console\Output\OutputInterface; class RepairTree extends Command { public const CHUNK_SIZE = 200; - /** - * @var IDBConnection - */ - protected $connection; - - public function __construct(IDBConnection $connection) { - $this->connection = $connection; + public function __construct( + protected IDBConnection $connection, + ) { parent::__construct(); } - protected function configure() { + protected function configure(): void { $this ->setName('files:repair-tree') ->setDescription('Try and repair malformed filesystem tree structures') @@ -90,7 +86,7 @@ class RepairTree extends Command { $this->connection->commit(); } - return 0; + return self::SUCCESS; } private function getFileId(int $storage, string $path) { @@ -102,7 +98,7 @@ class RepairTree extends Command { return $query->execute()->fetch(\PDO::FETCH_COLUMN); } - private function deleteById(int $fileId) { + private function deleteById(int $fileId): void { $query = $this->connection->getQueryBuilder(); $query->delete('filecache') ->where($query->expr()->eq('fileid', $query->createNamedParameter($fileId))); diff --git a/apps/files/lib/Command/Scan.php b/apps/files/lib/Command/Scan.php index 6c7a607d2af..8c3574d7608 100644 --- a/apps/files/lib/Command/Scan.php +++ b/apps/files/lib/Command/Scan.php @@ -54,26 +54,20 @@ use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; class Scan extends Base { - private IUserManager $userManager; protected float $execTime = 0; protected int $foldersCounter = 0; protected int $filesCounter = 0; protected int $errorsCounter = 0; - private IRootFolder $root; - private MetadataManager $metadataManager; public function __construct( - IUserManager $userManager, - IRootFolder $rootFolder, - MetadataManager $metadataManager + private IUserManager $userManager, + private IRootFolder $rootFolder, + private MetadataManager $metadataManager ) { - $this->userManager = $userManager; parent::__construct(); - $this->root = $rootFolder; - $this->metadataManager = $metadataManager; } - protected function configure() { + protected function configure(): void { parent::configure(); $this @@ -134,7 +128,7 @@ class Scan extends Base { ++$this->filesCounter; $this->abortIfInterrupted(); if ($scanMetadata) { - $node = $this->root->get($path); + $node = $this->rootFolder->get($path); if ($node instanceof File) { $this->metadataManager->generateMetadata($node, false); } @@ -181,7 +175,7 @@ class Scan extends Base { } } - public function filterHomeMount(IMountPoint $mountPoint) { + public function filterHomeMount(IMountPoint $mountPoint): bool { // any mountpoint inside '/$user/files/' return substr_count($mountPoint->getMountPoint(), '/') <= 3; } @@ -202,7 +196,7 @@ class Scan extends Base { $users_total = count($users); if ($users_total === 0) { $output->writeln('Please specify the user id to scan, --all to scan for all users or --path=...'); - return 1; + return self::FAILURE; } $this->initTools($output); @@ -212,7 +206,7 @@ class Scan extends Base { if (is_object($user)) { $user = $user->getUID(); } - $path = $inputPath ? $inputPath : '/' . $user; + $path = $inputPath ?: '/' . $user; ++$user_count; if ($this->userManager->userExists($user)) { $output->writeln("Starting scan for user $user_count out of $users_total ($user)"); @@ -231,13 +225,13 @@ class Scan extends Base { } $this->presentStats($output); - return 0; + return self::SUCCESS; } /** * Initialises some useful tools for the Command */ - protected function initTools(OutputInterface $output) { + protected function initTools(OutputInterface $output): void { // Start the timer $this->execTime = -microtime(true); // Convert PHP errors to exceptions @@ -270,10 +264,7 @@ class Scan extends Base { return true; } - /** - * @param OutputInterface $output - */ - protected function presentStats(OutputInterface $output) { + protected function presentStats(OutputInterface $output): void { // Stop the timer $this->execTime += microtime(true); @@ -299,11 +290,9 @@ class Scan extends Base { /** - * Formats microtime into a human readable format - * - * @return string + * Formats microtime into a human-readable format */ - protected function formatExecTime() { + protected function formatExecTime(): string { $secs = (int)round($this->execTime); # convert seconds into HH:MM:SS form return sprintf('%02d:%02d:%02d', (int)($secs / 3600), ((int)($secs / 60) % 60), $secs % 60); diff --git a/apps/files/lib/Command/ScanAppData.php b/apps/files/lib/Command/ScanAppData.php index 63e13733b2a..ddd3986aa28 100644 --- a/apps/files/lib/Command/ScanAppData.php +++ b/apps/files/lib/Command/ScanAppData.php @@ -46,26 +46,20 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; class ScanAppData extends Base { + protected float $execTime = 0; - /** @var IRootFolder */ - protected $root; - /** @var IConfig */ - protected $config; - /** @var float */ - protected $execTime = 0; - /** @var int */ - protected $foldersCounter = 0; - /** @var int */ - protected $filesCounter = 0; - - public function __construct(IRootFolder $rootFolder, IConfig $config) { - parent::__construct(); + protected int $foldersCounter = 0; + + protected int $filesCounter = 0; - $this->root = $rootFolder; - $this->config = $config; + public function __construct( + protected IRootFolder $rootFolder, + protected IConfig $config, + ) { + parent::__construct(); } - protected function configure() { + protected function configure(): void { parent::configure(); $this @@ -80,7 +74,7 @@ class ScanAppData extends Base { $appData = $this->getAppDataFolder(); } catch (NotFoundException $e) { $output->writeln('NoAppData folder found'); - return 1; + return self::FAILURE; } if ($folder !== '') { @@ -88,7 +82,7 @@ class ScanAppData extends Base { $appData = $appData->get($folder); } catch (NotFoundException $e) { $output->writeln('Could not find folder: ' . $folder . ''); - return 1; + return self::FAILURE; } } @@ -126,21 +120,21 @@ class ScanAppData extends Base { } catch (ForbiddenException $e) { $output->writeln('Storage not writable'); $output->writeln('Make sure you\'re running the scan command only as the user the web server runs as'); - return 1; + return self::FAILURE; } catch (InterruptedException $e) { # exit the function if ctrl-c has been pressed $output->writeln('Interrupted by user'); - return 1; + return self::FAILURE; } catch (NotFoundException $e) { $output->writeln('Path not found: ' . $e->getMessage() . ''); - return 1; + return self::FAILURE; } catch (\Exception $e) { $output->writeln('Exception during scan: ' . $e->getMessage() . ''); $output->writeln('' . $e->getTraceAsString() . ''); - return 1; + return self::FAILURE; } - return 0; + return self::SUCCESS; } @@ -167,7 +161,7 @@ class ScanAppData extends Base { /** * Initialises some useful tools for the Command */ - protected function initTools() { + protected function initTools(): void { // Start the timer $this->execTime = -microtime(true); // Convert PHP errors to exceptions @@ -186,7 +180,7 @@ class ScanAppData extends Base { * * @throws \ErrorException */ - public function exceptionErrorHandler($severity, $message, $file, $line) { + public function exceptionErrorHandler($severity, $message, $file, $line): void { if (!(error_reporting() & $severity)) { // This error code is not included in error_reporting return; @@ -194,10 +188,7 @@ class ScanAppData extends Base { throw new \ErrorException($message, 0, $severity, $file, $line); } - /** - * @param OutputInterface $output - */ - protected function presentStats(OutputInterface $output) { + protected function presentStats(OutputInterface $output): void { // Stop the timer $this->execTime += microtime(true); @@ -213,9 +204,8 @@ class ScanAppData extends Base { * * @param string[] $headers * @param string[] $rows - * @param OutputInterface $output */ - protected function showSummary($headers, $rows, OutputInterface $output) { + protected function showSummary($headers, $rows, OutputInterface $output): void { $niceDate = $this->formatExecTime(); if (!$rows) { $rows = [ @@ -233,11 +223,9 @@ class ScanAppData extends Base { /** - * Formats microtime into a human readable format - * - * @return string + * Formats microtime into a human-readable format */ - protected function formatExecTime() { + protected function formatExecTime(): string { $secs = round($this->execTime); # convert seconds into HH:MM:SS form return sprintf('%02d:%02d:%02d', (int)($secs / 3600), ((int)($secs / 60) % 60), (int)$secs % 60); @@ -273,6 +261,6 @@ class ScanAppData extends Base { throw new NotFoundException(); } - return $this->root->get('appdata_'.$instanceId); + return $this->rootFolder->get('appdata_'.$instanceId); } } diff --git a/apps/files/lib/Command/TransferOwnership.php b/apps/files/lib/Command/TransferOwnership.php index 50aa0b21a5f..2007c23f256 100644 --- a/apps/files/lib/Command/TransferOwnership.php +++ b/apps/files/lib/Command/TransferOwnership.php @@ -46,26 +46,15 @@ use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; class TransferOwnership extends Command { - - /** @var IUserManager */ - private $userManager; - - /** @var OwnershipTransferService */ - private $transferService; - - /** @var IConfig */ - private $config; - - public function __construct(IUserManager $userManager, - OwnershipTransferService $transferService, - IConfig $config) { + public function __construct( + private IUserManager $userManager, + private OwnershipTransferService $transferService, + private IConfig $config, + ) { parent::__construct(); - $this->userManager = $userManager; - $this->transferService = $transferService; - $this->config = $config; } - protected function configure() { + protected function configure(): void { $this ->setName('files:transfer-ownership') ->setDescription('All files and folders are moved to another user - outgoing shares and incoming user file shares (optionally) are moved as well.') @@ -107,7 +96,7 @@ class TransferOwnership extends Command { if ($input->getArgument(('source-user')) === $input->getArgument('destination-user')) { $output->writeln("Ownership can't be transferred when Source and Destination users are the same user. Please check your input."); - return 1; + return self::FAILURE; } $sourceUserObject = $this->userManager->get($input->getArgument('source-user')); @@ -115,12 +104,12 @@ class TransferOwnership extends Command { if (!$sourceUserObject instanceof IUser) { $output->writeln("Unknown source user " . $input->getArgument('source-user') . ""); - return 1; + return self::FAILURE; } if (!$destinationUserObject instanceof IUser) { $output->writeln("Unknown destination user " . $input->getArgument('destination-user') . ""); - return 1; + return self::FAILURE; } try { @@ -137,13 +126,12 @@ class TransferOwnership extends Command { $includeIncoming = $this->config->getSystemValue('transferIncomingShares', false); if (gettype($includeIncoming) !== 'boolean') { $output->writeln(" config.php: 'transfer-incoming-shares': wrong usage. Transfer aborted."); - return 1; + return self::FAILURE; } break; default: $output->writeln("Option --transfer-incoming-shares: wrong usage. Transfer aborted."); - return 1; - break; + return self::FAILURE; } $this->transferService->transfer( @@ -157,9 +145,9 @@ class TransferOwnership extends Command { ); } catch (TransferOwnershipException $e) { $output->writeln("" . $e->getMessage() . ""); - return $e->getCode() !== 0 ? $e->getCode() : 1; + return $e->getCode() !== 0 ? $e->getCode() : self::FAILURE; } - return 0; + return self::SUCCESS; } }