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

Jump to bottom

update occ files:repair-tree command to also handle cases where the parent is -1 when it shouldn't be #39231

Closed
wants to merge 1 commit into from
Closed

update occ files:repair-tree command to also handle cases where the parent is -1 when it shouldn't be #39231

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
92 changes: 87 additions & 5 deletions apps/files/lib/Command/RepairTree.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

namespace OCA\Files\Command;

use OC\DB\QueryBuilder\Literal;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
Expand All @@ -51,9 +54,48 @@
}

public function execute(InputInterface $input, OutputInterface $output): int {
$rows = $this->findBrokenTreeBits();
$fix = !$input->getOption('dry-run');

$this->repairParent($fix, $output);
$this->repairPaths($fix, $output);

return 0;
}

private function repairParent(bool $fix, OutputInterface $output) {

Check notice

Code scanning / Psalm

MissingReturnType Note

Method OCA\Files\Command\RepairTree::repairParent does not have a return type, expecting void
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private function repairParent(bool $fix, OutputInterface $output) {
private function repairParent(bool $fix, OutputInterface $output): void {

$rows = $this->findMissingParents();

$output->writeln("Found " . count($rows) . " file entries with an invalid parent");

if ($fix) {
$this->connection->beginTransaction();
}

$query = $this->connection->getQueryBuilder();
$query->update('filecache')
->set('parent', $query->createParameter('parent'))
->where($query->expr()->eq('fileid', $query->createParameter('fileid')));
Comment on lines +72 to +77
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
$query = $this->connection->getQueryBuilder();
$query->update('filecache')
->set('parent', $query->createParameter('parent'))
->where($query->expr()->eq('fileid', $query->createParameter('fileid')));
$query = $this->connection->getQueryBuilder();
$query->update('filecache')
->set('parent', $query->createParameter('parent'))
->where($query->expr()->eq('fileid', $query->createParameter('fileid')));
}

The query is only used for fixing, no?


foreach ($rows as $row) {
$output->writeln("Parent of file <info>{$row['fileid']}</info> (<info>{$row['path']}</info>) is <info>-1</info> but should be <info>{$row['parent_id']}</info> based on its path", OutputInterface::VERBOSITY_VERBOSE);

if ($fix) {
$query->setParameters([
'fileid' => $row['fileid'],
'parent' => $row['parent_id'],
]);
$query->execute();

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\DB\QueryBuilder\IQueryBuilder::execute has been marked as deprecated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$query->execute();
$query->executeStatement();

}
}

if ($fix) {
$this->connection->commit();
}
}

private function repairPaths(bool $fix, OutputInterface $output) {

Check notice

Code scanning / Psalm

MissingReturnType Note

Method OCA\Files\Command\RepairTree::repairPaths does not have a return type, expecting void
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private function repairPaths(bool $fix, OutputInterface $output) {
private function repairPaths(bool $fix, OutputInterface $output): void {

$rows = $this->findBrokenTreeBits();

$output->writeln("Found " . count($rows) . " file entries with an invalid path");

if ($fix) {
Expand All @@ -68,7 +110,7 @@
->where($query->expr()->eq('fileid', $query->createParameter('fileid')));

foreach ($rows as $row) {
$output->writeln("Path of file {$row['fileid']} is {$row['path']} but should be {$row['parent_path']}/{$row['name']} based on its parent", OutputInterface::VERBOSITY_VERBOSE);
$output->writeln("Path of file <info>{$row['fileid']}</info> is <info>{$row['path']}</info> but should be <info>{$row['parent_path']}/{$row['name']}</info> based on its parent", OutputInterface::VERBOSITY_VERBOSE);

if ($fix) {
$fileId = $this->getFileId((int)$row['parent_storage'], $row['parent_path'] . '/' . $row['name']);
Expand All @@ -89,8 +131,6 @@
if ($fix) {
$this->connection->commit();
}

return 0;
}

private function getFileId(int $storage, string $path) {
Expand Down Expand Up @@ -127,7 +167,49 @@
$query->expr()->neq('f.path', 'f.name')
),
$query->expr()->neq('f.storage', 'p.storage')
));
))
->andWhere($query->expr()->neq('f.parent', $query->createNamedParameter(-1, IQueryBuilder::PARAM_INT)));

return $query->execute()->fetchAll();

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\DB\QueryBuilder\IQueryBuilder::execute has been marked as deprecated

Check notice

Code scanning / Psalm

PossiblyInvalidMethodCall Note

Cannot call method on possible int variable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return $query->execute()->fetchAll();
return $query->executeQuery()->fetchAll();

}

private function findMissingParents(): array {
$query = $this->connection->getQueryBuilder();

// find all items with parent == -1 && path !== ''
// and also find the matching parent item by p.path == dirname(f.path)
//
// we implement dirname in sql by doing substr(1, max(0, strlen(path) - strlen(name) - 1))
// 1 because sql substr starts at 1 instead of 0
// -1 for the slash
// max(0, ...) to handle the cases where path==name

$query->select('f.fileid', 'f.path')
->selectAlias('p.fileid', 'parent_id')
->from('filecache', 'f')
->innerJoin('f', 'filecache', 'p', $query->expr()->andX(
$query->expr()->eq(
'p.path_hash',
$query->func()->md5($query->func()->substring(
'f.path',
$query->createNamedParameter(1, IQueryBuilder::PARAM_INT),
$query->func()->greatest(
$query->func()->subtract(
$query->func()->subtract(
$query->func()->charLength('f.path'),
$query->func()->charLength('f.name'),
),
$query->createNamedParameter(1, IQueryBuilder::PARAM_INT),
),
$query->createNamedParameter(0, IQueryBuilder::PARAM_INT),
)

))
),
$query->expr()->eq('f.storage', 'p.storage')
))
->where($query->expr()->neq('f.path_hash', $query->createNamedParameter(md5(''))))
->andWhere($query->expr()->eq('f.parent', $query->createNamedParameter(-1, IQueryBuilder::PARAM_INT)));

return $query->execute()->fetchAll();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return $query->execute()->fetchAll();
return $query->executeQuery()->fetchAll();

}
Expand Down
Loading