Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor files_sharing app commands #39898

Closed
wants to merge 2 commits into from
Closed

Refactor files_sharing app commands #39898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8e35fc2
Refactors files_sharing app commands.
Aug 16, 2023
a159263
Uses early returns.
Aug 16, 2023
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
68 changes: 31 additions & 37 deletions apps/files_sharing/lib/Command/CleanupRemoteStorages.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,24 +38,14 @@
* shares_external table.
*/
class CleanupRemoteStorages extends Command {

/**
* @var IDBConnection
*/
protected $connection;

/**
* @var ICloudIdManager
*/
private $cloudIdManager;

public function __construct(IDBConnection $connection, ICloudIdManager $cloudIdManager) {
$this->connection = $connection;
$this->cloudIdManager = $cloudIdManager;
public function __construct(
protected IDBConnection $connection,
private ICloudIdManager $cloudIdManager,
) {
parent::__construct();
}

protected function configure() {
protected function configure(): void {
$this
->setName('sharing:cleanup-remote-storages')
->setDescription('Cleanup shared storage entries that have no matching entry in the shares_external table')
Expand All @@ -77,34 +67,38 @@ public function execute(InputInterface $input, OutputInterface $output): int {
$output->writeln(count($remoteShareIds) . ' remote share(s) exist');

foreach ($remoteShareIds as $id => $remoteShareId) {
if (isset($remoteStorages[$remoteShareId])) {
if ($input->getOption('dry-run') || $output->isVerbose()) {
$output->writeln("<info>$remoteShareId belongs to remote share $id</info>");
}

unset($remoteStorages[$remoteShareId]);
} else {
if (!isset($remoteStorages[$remoteShareId])) {
$output->writeln("<comment>$remoteShareId for share $id has no matching storage, yet</comment>");
continue;
}

if ($input->getOption('dry-run') || $output->isVerbose()) {
$output->writeln("<info>$remoteShareId belongs to remote share $id</info>");
}

unset($remoteStorages[$remoteShareId]);
}

if (empty($remoteStorages)) {
$output->writeln('<info>no storages deleted</info>');
} else {
$dryRun = $input->getOption('dry-run');
foreach ($remoteStorages as $id => $numericId) {
if ($dryRun) {
$output->writeln("<error>$id [$numericId] can be deleted</error>");
$this->countFiles($numericId, $output);
} else {
$this->deleteStorage($id, $numericId, $output);
}
return self::SUCCESS;
}

$dryRun = $input->getOption('dry-run');
foreach ($remoteStorages as $id => $numericId) {
if (!$dryRun) {
$this->deleteStorage($id, $numericId, $output);
continue;
}

$output->writeln("<error>$id [$numericId] can be deleted</error>");
$this->countFiles($numericId, $output);
}
return 0;

return self::SUCCESS;
}

public function countFiles($numericId, OutputInterface $output) {
public function countFiles($numericId, OutputInterface $output): void {

Check notice

Code scanning / Psalm

MissingParamType

Parameter $numericId has no provided type
$queryBuilder = $this->connection->getQueryBuilder();
$queryBuilder->select($queryBuilder->func()->count('fileid'))
->from('filecache')
Expand All @@ -118,7 +112,7 @@ public function countFiles($numericId, OutputInterface $output) {
$output->writeln("$count files can be deleted for storage $numericId");
}

public function deleteStorage($id, $numericId, OutputInterface $output) {
public function deleteStorage($id, $numericId, OutputInterface $output): void {

Check notice

Code scanning / Psalm

MissingParamType

Parameter $id has no provided type

Check notice

Code scanning / Psalm

MissingParamType

Parameter $numericId has no provided type
$queryBuilder = $this->connection->getQueryBuilder();
$queryBuilder->delete('storages')
->where($queryBuilder->expr()->eq(
Expand All @@ -132,7 +126,7 @@ public function deleteStorage($id, $numericId, OutputInterface $output) {
$this->deleteFiles($numericId, $output);
}

public function deleteFiles($numericId, OutputInterface $output) {
public function deleteFiles($numericId, OutputInterface $output): void {

Check notice

Code scanning / Psalm

MissingParamType

Parameter $numericId has no provided type
$queryBuilder = $this->connection->getQueryBuilder();
$queryBuilder->delete('filecache')
->where($queryBuilder->expr()->eq(
Expand All @@ -145,7 +139,7 @@ public function deleteFiles($numericId, OutputInterface $output) {
$output->writeln("deleted $count files");
}

public function getRemoteStorages() {
public function getRemoteStorages(): array {
$queryBuilder = $this->connection->getQueryBuilder();
$queryBuilder->select(['id', 'numeric_id'])
->from('storages')
Expand All @@ -172,7 +166,7 @@ public function getRemoteStorages() {
return $remoteStorages;
}

public function getRemoteShareIds() {
public function getRemoteShareIds(): array {
$queryBuilder = $this->connection->getQueryBuilder();
$queryBuilder->select(['id', 'share_token', 'owner', 'remote'])
->from('share_external');
Expand Down
31 changes: 16 additions & 15 deletions apps/files_sharing/lib/Command/DeleteOrphanShares.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,10 @@
use Symfony\Component\Console\Output\OutputInterface;

class DeleteOrphanShares extends Base {
private OrphanHelper $orphanHelper;

public function __construct(OrphanHelper $orphanHelper) {
public function __construct(
private OrphanHelper $orphanHelper,
) {
parent::__construct();
$this->orphanHelper = $orphanHelper;
}

protected function configure(): void {
Expand All @@ -58,23 +57,25 @@ public function execute(InputInterface $input, OutputInterface $output): int {

$orphans = [];
foreach ($shares as $share) {
if (!$this->orphanHelper->isShareValid($share['owner'], $share['fileid'])) {
$orphans[] = $share['id'];
$exists = $this->orphanHelper->fileExists($share['fileid']);
$output->writeln("<info>{$share['target']}</info> owned by <info>{$share['owner']}</info>");
if ($exists) {
$output->writeln(" file still exists but the share owner lost access to it, run <info>occ info:file {$share['fileid']}</info> for more information about the file");
} else {
$output->writeln(" file no longer exists");
}
if ($this->orphanHelper->isShareValid($share['owner'], $share['fileid'])) {
continue;
}

$orphans[] = $share['id'];
$exists = $this->orphanHelper->fileExists($share['fileid']);
$output->writeln("<info>{$share['target']}</info> owned by <info>{$share['owner']}</info>");
if ($exists) {
$output->writeln(" file still exists but the share owner lost access to it, run <info>occ info:file {$share['fileid']}</info> for more information about the file");
} else {
$output->writeln(" file no longer exists");
}
}

$count = count($orphans);

if ($count === 0) {
$output->writeln("No orphan shares detected");
return 0;
return self::SUCCESS;
}

if ($force) {
Expand All @@ -91,6 +92,6 @@ public function execute(InputInterface $input, OutputInterface $output): int {
$this->orphanHelper->deleteShares($orphans);
}

return 0;
return self::SUCCESS;
}
}
30 changes: 9 additions & 21 deletions apps/files_sharing/lib/Command/ExiprationNotification.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,28 +36,16 @@
use Symfony\Component\Console\Output\OutputInterface;

class ExiprationNotification extends Command {
/** @var NotificationManager */
private $notificationManager;
/** @var IDBConnection */
private $connection;
/** @var ITimeFactory */
private $time;
/** @var ShareManager */
private $shareManager;

public function __construct(ITimeFactory $time,
NotificationManager $notificationManager,
IDBConnection $connection,
ShareManager $shareManager) {
public function __construct(
private ITimeFactory $time,
private NotificationManager $notificationManager,
private IDBConnection $connection,
private ShareManager $shareManager,
) {
parent::__construct();

$this->notificationManager = $notificationManager;
$this->connection = $connection;
$this->time = $time;
$this->shareManager = $shareManager;
}

protected function configure() {
protected function configure(): void {
$this
->setName('sharing:expiration-notification')
->setDescription('Notify share initiators when a share will expire the next day.');
Expand All @@ -67,7 +55,7 @@ public function execute(InputInterface $input, OutputInterface $output): int {
//Current time
$minTime = $this->time->getDateTime();
$minTime->add(new \DateInterval('P1D'));
$minTime->setTime(0,0,0);
$minTime->setTime(0, 0, 0);

$maxTime = clone $minTime;
$maxTime->setTime(23, 59, 59);
Expand All @@ -94,6 +82,6 @@ public function execute(InputInterface $input, OutputInterface $output): int {
$notification->setUser($share->getSharedBy());
$this->notificationManager->notify($notification);
}
return 0;
return self::SUCCESS;
}
}