chunk getting invalid paths and reuse queries

Signed-off-by: Robin Appelman <robin@icewind.nl>

lib/private/Repair/NC13/RepairInvalidPaths.php

@@ -22,17 +22,25 @@
 namespace OC\Repair\NC13;
 
 
+use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\Migration\IOutput;
 use OCP\Migration\IRepairStep;
 
 class RepairInvalidPaths implements IRepairStep {
+	const MAX_ROWS = 1000;
+
 	/** @var IDBConnection */
 	private $connection;
 	/** @var IConfig */
 	private $config;
 
+	private $getIdQuery;
+	private $updateQuery;
+	private $reparentQuery;
+	private $deleteQuery;
+
 	public function __construct(IDBConnection $connection, IConfig $config) {
 		$this->connection = $connection;
 		$this->config = $config;
@@ -58,51 +66,77 @@ class RepairInvalidPaths implements IRepairStep {
 				$builder->expr()->eq('f.parent', 'p.fileid'),
 				$builder->expr()->neq('p.name', $builder->createNamedParameter(''))
 			))
-			->where($builder->expr()->neq('f.path', $computedPath));
-
-		$result = $query->execute();
-		while ($row = $result->fetch()) {
-			yield $row;
-		}
+			->where($builder->expr()->neq('f.path', $computedPath))
+			->setMaxResults(self::MAX_ROWS);
+
+		do {
+			$result = $query->execute();
+			$rows = $result->fetchAll();
+			foreach ($rows as $row) {
+				yield $row;
+			}
+			$result->closeCursor();
+		} while (count($rows) >= self::MAX_ROWS);
 	}
 
 	private function getId($storage, $path) {
-		$builder = $this->connection->getQueryBuilder();
+		if (!$this->getIdQuery) {
+			$builder = $this->connection->getQueryBuilder();
+
+			$this->getIdQuery = $builder->select('fileid')
+				->from('filecache')
+				->where($builder->expr()->eq('storage', $builder->createParameter('storage')))
+				->andWhere($builder->expr()->eq('path', $builder->createParameter('path')));
+		}
 
-		$query = $builder->select('fileid')
-			->from('filecache')
-			->where($builder->expr()->eq('storage', $builder->createNamedParameter($storage)))
-			->andWhere($builder->expr()->eq('path', $builder->createNamedParameter($path)));
+		$this->getIdQuery->setParameter('storage', $storage, IQueryBuilder::PARAM_INT);
+		$this->getIdQuery->setParameter('path', $path);
 
-		return $query->execute()->fetchColumn();
+		return $this->getIdQuery->execute()->fetchColumn();
 	}
 
 	private function update($fileid, $newPath) {
-		$builder = $this->connection->getQueryBuilder();
+		if (!$this->updateQuery) {
+			$builder = $this->connection->getQueryBuilder();
 
-		$query = $builder->update('filecache')
-			->set('path', $builder->createNamedParameter($newPath))
-			->set('path_hash', $builder->createNamedParameter(md5($newPath)))
-			->where($builder->expr()->eq('fileid', $builder->createNamedParameter($fileid)));
+			$this->updateQuery = $builder->update('filecache')
+				->set('path', $builder->createParameter('newpath'))
+				->set('path_hash', $builder->func()->md5($builder->createParameter('newpath')))
+				->where($builder->expr()->eq('fileid', $builder->createParameter('fileid')));
+		}
 
-		$query->execute();
+		$this->updateQuery->setParameter('newpath', $newPath);
+		$this->updateQuery->setParameter('fileid', $fileid, IQueryBuilder::PARAM_INT);
+
+		$this->updateQuery->execute();
 	}
 
 	private function reparent($from, $to) {
-		$builder = $this->connection->getQueryBuilder();
+		if (!$this->reparentQuery) {
+			$builder = $this->connection->getQueryBuilder();
+
+			$this->reparentQuery = $builder->update('filecache')
+				->set('parent', $builder->createParameter('to'))
+				->where($builder->expr()->eq('fileid', $builder->createParameter('from')));
+		}
+
+		$this->reparentQuery->setParameter('from', $from);
+		$this->reparentQuery->setParameter('to', $to);
 
-		$query = $builder->update('filecache')
-			->set('parent', $builder->createNamedParameter($to))
-			->where($builder->expr()->eq('fileid', $builder->createNamedParameter($from)));
-		$query->execute();
+		$this->reparentQuery->execute();
 	}
 
 	private function delete($fileid) {
-		$builder = $this->connection->getQueryBuilder();
+		if (!$this->deleteQuery) {
+			$builder = $this->connection->getQueryBuilder();
+
+			$this->deleteQuery = $builder->delete('filecache')
+				->where($builder->expr()->eq('fileid', $builder->createParameter('fileid')));
+		}
+
+		$this->deleteQuery->setParameter('fileid', $fileid, IQueryBuilder::PARAM_INT);
 
-		$query = $builder->delete('filecache')
-			->where($builder->expr()->eq('fileid', $builder->createNamedParameter($fileid)));
-		$query->execute();
+		$this->deleteQuery->execute();
 	}
 
 	private function repair() {

tests/lib/Repair/RepairInvalidPathsTest.php

@@ -114,4 +114,42 @@ class RepairInvalidPathsTest extends TestCase {
 		$this->assertEquals($this->cache->getId('foo2/bar'), $this->cache->get('foo2/bar/asd')['parent']);
 		$this->assertEquals($this->cache->getId('foo2/bar/asd'), $this->cache->get('foo2/bar/asd/foo')['parent']);
 	}
+
+	public function testRepairMultipleNonDuplicate() {
+		$this->storage->mkdir('foo/bar/asd');
+		$this->storage->mkdir('foo/bar2/asd');
+		$this->storage->mkdir('foo2');
+		$this->storage->getScanner()->scan('');
+
+		$folderId1 = $this->cache->getId('foo/bar');
+		$folderId2 = $this->cache->getId('foo/bar2');
+		$newParentFolderId = $this->cache->getId('foo2');
+		// failed rename, moved entry is updated but not it's children
+		$this->cache->update($folderId1, ['path' => 'foo2/bar', 'parent' => $newParentFolderId]);
+		$this->cache->update($folderId2, ['path' => 'foo2/bar2', 'parent' => $newParentFolderId]);
+
+		$this->assertTrue($this->cache->inCache('foo2/bar'));
+		$this->assertTrue($this->cache->inCache('foo2/bar2'));
+		$this->assertTrue($this->cache->inCache('foo/bar/asd'));
+		$this->assertTrue($this->cache->inCache('foo/bar2/asd'));
+		$this->assertFalse($this->cache->inCache('foo2/bar/asd'));
+		$this->assertFalse($this->cache->inCache('foo2/bar2/asd'));
+
+		$this->assertEquals($folderId1, $this->cache->get('foo/bar/asd')['parent']);
+		$this->assertEquals($folderId2, $this->cache->get('foo/bar2/asd')['parent']);
+
+		$this->repair->run($this->createMock(IOutput::class));
+
+		$this->assertTrue($this->cache->inCache('foo2/bar'));
+		$this->assertTrue($this->cache->inCache('foo2/bar2'));
+		$this->assertTrue($this->cache->inCache('foo2/bar/asd'));
+		$this->assertTrue($this->cache->inCache('foo2/bar2/asd'));
+		$this->assertFalse($this->cache->inCache('foo/bar/asd'));
+		$this->assertFalse($this->cache->inCache('foo/bar2/asd'));
+
+		$this->assertEquals($folderId1, $this->cache->get('foo2/bar/asd')['parent']);
+		$this->assertEquals($folderId2, $this->cache->get('foo2/bar2/asd')['parent']);
+		$this->assertEquals($folderId1, $this->cache->getId('foo2/bar'));
+		$this->assertEquals($folderId2, $this->cache->getId('foo2/bar2'));
+	}
 }