Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Code adjustments according to PR review
* Delete unnecessary function docs
* Rename parameters to 'since' and 'until'
* Style: use '&&' instead of 'and'
* Add types

Signed-off-by: GitHub <[email protected]>
  • Loading branch information
R0Wi authored and backportbot-nextcloud[bot] committed Sep 22, 2023
commit 9630de5674871d4c40f94f9dfe01504db27d40b5
86 changes: 32 additions & 54 deletions apps/files_trashbin/lib/Command/RestoreAllFiles.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
namespace OCA\Files_Trashbin\Command;

use OC\Core\Command\Base;
use OCA\Files_Trashbin\Trash\ITrashItem;
use OCA\Files_Trashbin\Trash\ITrashManager;
use OCP\Files\IRootFolder;
use OCP\IDBConnection;
Expand All @@ -39,7 +38,7 @@ class RestoreAllFiles extends Base {
private const SCOPE_USER = 1;
private const SCOPE_GROUPFOLDERS = 2;

private static $SCOPE_MAP = [
private static array $SCOPE_MAP = [
'user' => self::SCOPE_USER,
'groupfolders' => self::SCOPE_GROUPFOLDERS,
'all' => self::SCOPE_ALL
Expand All @@ -54,8 +53,7 @@ class RestoreAllFiles extends Base {
/** @var \OCP\IDBConnection */
protected $dbConnection;

/** @var ITrashManager */
protected $trashManager;
protected ITrashManager $trashManager;

/** @var IL10N */
protected $l10n;
Expand Down Expand Up @@ -100,14 +98,14 @@ protected function configure(): void {
'user'
)
->addOption(
'restore-from',
'f',
'since',
null,
InputOption::VALUE_OPTIONAL,
'Only restore files deleted after the given timestamp'
)
->addOption(
'restore-to',
't',
'until',
null,
InputOption::VALUE_OPTIONAL,
'Only restore files deleted before the given timestamp'
)
Expand All @@ -122,16 +120,16 @@ protected function configure(): void {
protected function execute(InputInterface $input, OutputInterface $output): int {
/** @var string[] $users */
$users = $input->getArgument('user_id');
if ((!empty($users)) and ($input->getOption('all-users'))) {
if ((!empty($users)) && ($input->getOption('all-users'))) {
throw new InvalidOptionException('Either specify a user_id or --all-users');
}

[$scope, $restoreFrom, $restoreTo, $dryRun] = $this->parseArgs($input);
[$scope, $since, $until, $dryRun] = $this->parseArgs($input);

if (!empty($users)) {
foreach ($users as $user) {
$output->writeln("Restoring deleted files for user <info>$user</info>");
$this->restoreDeletedFiles($user, $scope, $restoreFrom, $restoreTo, $dryRun, $output);
$this->restoreDeletedFiles($user, $scope, $since, $until, $dryRun, $output);
}
} elseif ($input->getOption('all-users')) {
$output->writeln('Restoring deleted files for all users');
Expand All @@ -147,7 +145,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$users = $backend->getUsers('', $limit, $offset);
foreach ($users as $user) {
$output->writeln("<info>$user</info>");
$this->restoreDeletedFiles($user, $scope, $restoreFrom, $restoreTo, $dryRun, $output);
$this->restoreDeletedFiles($user, $scope, $since, $until, $dryRun, $output);
}
$offset += $limit;
} while (count($users) >= $limit);
Expand All @@ -159,16 +157,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}

/**
* Restore deleted files for the given user
*
* @param string $uid
* @param int $scope
* @param int|null $restoreFrom
* @param int|null $restoreTo
* @param bool $dryRun
* @param OutputInterface $output
* Restore deleted files for the given user according to the given filters
*/
protected function restoreDeletedFiles(string $uid, int $scope, ?int $restoreFrom, ?int $restoreTo, bool $dryRun, OutputInterface $output): void {
protected function restoreDeletedFiles(string $uid, int $scope, ?int $since, ?int $until, bool $dryRun, OutputInterface $output): void {
\OC_Util::tearDownFS();
\OC_Util::setupFS($uid);
\OC_User::setUserId($uid);
Expand All @@ -183,13 +174,13 @@ protected function restoreDeletedFiles(string $uid, int $scope, ?int $restoreFro
$userTrashItems = $this->filterTrashItems(
$this->trashManager->listTrashRoot($user),
$scope,
$restoreFrom,
$restoreTo,
$since,
$until,
$output);

$trashCount = count($userTrashItems);
if ($trashCount == 0) {
$output->writeln("User has no deleted files in the trashbin");
$output->writeln("User has no deleted files in the trashbin matching the given filters");
return;
}
$prepMsg = $dryRun ? 'Would restore' : 'Preparing to restore';
Expand All @@ -213,13 +204,12 @@ protected function restoreDeletedFiles(string $uid, int $scope, ?int $restoreFro
try {
$trashItem->getTrashBackend()->restoreItem($trashItem);
} catch (\Throwable $e) {
$output->writeln(" <error>failed</error>");
$output->writeln("<error>" . $e->getMessage() . "</error>");
$output->writeln("<error>" . $e->getTraceAsString() . "</error>", OutputInterface::VERBOSITY_VERY_VERBOSE);
$output->writeln(" <error>Failed: " . $e->getMessage() . "</error>");
$output->writeln(" <error>" . $e->getTraceAsString() . "</error>", OutputInterface::VERBOSITY_VERY_VERBOSE);
continue;
}

$count = $count + 1;
$count++;
$output->writeln(" <info>success</info>");
}

Expand All @@ -229,25 +219,21 @@ protected function restoreDeletedFiles(string $uid, int $scope, ?int $restoreFro
}

protected function parseArgs(InputInterface $input): array {
$restoreFrom = $this->parseTimestamp($input->getOption('restore-from'));
$restoreTo = $this->parseTimestamp($input->getOption('restore-to'));
$since = $this->parseTimestamp($input->getOption('since'));
$until = $this->parseTimestamp($input->getOption('until'));

if ($restoreFrom !== null and $restoreTo !== null and $restoreFrom > $restoreTo) {
throw new InvalidOptionException('restore-from must be before restore-to');
if ($since !== null && $until !== null && $since > $until) {
throw new InvalidOptionException('since must be before until');
}

return [
$this->parseScope($input->getOption('scope')),
$restoreFrom,
$restoreTo,
$since,
$until,
$input->getOption('dry-run')
];
}

/**
* @param string $scope
* @return int
*/
protected function parseScope(string $scope): int {
if (isset(self::$SCOPE_MAP[$scope])) {
return self::$SCOPE_MAP[$scope];
Expand All @@ -256,10 +242,6 @@ protected function parseScope(string $scope): int {
throw new InvalidOptionException("Invalid scope '$scope'");
}

/**
* @param string|null $timestamp
* @return int|null
*/
protected function parseTimestamp(?string $timestamp): ?int {
if ($timestamp === null) {
return null;
Expand All @@ -271,15 +253,7 @@ protected function parseTimestamp(?string $timestamp): ?int {
return $timestamp;
}

/**
* @param ITrashItem[] $trashItems
* @param int $scope
* @param int|null $restoreFrom
* @param int|null $restoreTo
* @param OutputInterface $output
* @return ITrashItem[]
*/
protected function filterTrashItems(array $trashItems, int $scope, ?int $restoreFrom, ?int $restoreTo, OutputInterface $output): array {
protected function filterTrashItems(array $trashItems, int $scope, ?int $since, ?int $until, OutputInterface $output): array {
$filteredTrashItems = [];
foreach ($trashItems as $trashItem) {
$trashItemClass = get_class($trashItem);
Expand All @@ -290,20 +264,24 @@ protected function filterTrashItems(array $trashItems, int $scope, ?int $restore
continue;
}

// Check scope for groupfolders by string because the groupfolders app might not be installed
/**
* Check scope for groupfolders by string because the groupfolders app might not be installed.
* That's why PSALM doesn't know the class GroupTrashItem.
* @psalm-suppress RedundantCondition
*/
if ($scope === self::SCOPE_GROUPFOLDERS && $trashItemClass !== 'OCA\GroupFolders\Trash\GroupTrashItem') {
$output->writeln("Skipping <info>" . $trashItem->getName() . "</info> because it is not a groupfolders trash item", OutputInterface::VERBOSITY_VERBOSE);
continue;
}

// Check left timestamp boundary
if ($restoreFrom !== null && $trashItem->getDeletedTime() <= $restoreFrom) {
if ($since !== null && $trashItem->getDeletedTime() <= $since) {
$output->writeln("Skipping <info>" . $trashItem->getName() . "</info> because it was deleted before the restore-from timestamp", OutputInterface::VERBOSITY_VERBOSE);
continue;
}

// Check right timestamp boundary
if ($restoreTo !== null && $trashItem->getDeletedTime() >= $restoreTo) {
if ($until !== null && $trashItem->getDeletedTime() >= $until) {
$output->writeln("Skipping <info>" . $trashItem->getName() . "</info> because it was deleted after the restore-to timestamp", OutputInterface::VERBOSITY_VERBOSE);
continue;
}
Expand Down
6 changes: 0 additions & 6 deletions build/psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1242,12 +1242,6 @@
<code><![CDATA[isset($_['hideFileList']) && $_['hideFileList'] !== true]]></code>
</RedundantCondition>
</file>
<file src="apps/files_trashbin/lib/Command/RestoreAllFiles.php">
<RedundantCondition>
<code><![CDATA[$scope === self::SCOPE_GROUPFOLDERS && $trashItemClass !== 'OCA\GroupFolders\Trash\GroupTrashItem']]></code>
<code><![CDATA[$trashItemClass !== 'OCA\GroupFolders\Trash\GroupTrashItem']]></code>
</RedundantCondition>
</file>
<file src="apps/files_trashbin/lib/Listeners/LoadAdditionalScripts.php">
<MissingTemplateParam>
<code>IEventListener</code>
Expand Down