-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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.
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
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
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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; | ||||||||||||||||||||||||
|
@@ -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) { | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||
$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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||
$rows = $this->findBrokenTreeBits(); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
$output->writeln("Found " . count($rows) . " file entries with an invalid path"); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if ($fix) { | ||||||||||||||||||||||||
|
@@ -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']); | ||||||||||||||||||||||||
|
@@ -89,8 +131,6 @@ | |||||||||||||||||||||||
if ($fix) { | ||||||||||||||||||||||||
$this->connection->commit(); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
return 0; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
private function getFileId(int $storage, string $path) { | ||||||||||||||||||||||||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
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(); | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
Check notice
Code scanning / Psalm
MissingReturnType Note