fix(preview): Fix files:app-data-scan for previews

And add unit tests for both migrated files and non-migrated files

Signed-off-by: Carl Schwan <carlschwan@kde.org>
This commit is contained in:
Carl Schwan 2026-02-10 17:44:13 +01:00
parent 935cd2910f
commit 3d18cd7cc5
No known key found for this signature in database
GPG key ID: 02325448204E452A
8 changed files with 324 additions and 96 deletions

View file

@ -53,6 +53,16 @@ class ScanAppData extends Base {
$this->addArgument('folder', InputArgument::OPTIONAL, 'The appdata subfolder to scan', '');
}
protected function getScanner(OutputInterface $output): Scanner {
$connection = $this->reconnectToDatabase($output);
return new Scanner(
null,
new ConnectionAdapter($connection),
Server::get(IEventDispatcher::class),
Server::get(LoggerInterface::class),
);
}
protected function scanFiles(OutputInterface $output, string $folder): int {
if ($folder === 'preview' || $folder === '') {
$this->previewsCounter = $this->previewStorage->scan();
@ -79,13 +89,7 @@ class ScanAppData extends Base {
}
}
$connection = $this->reconnectToDatabase($output);
$scanner = new Scanner(
null,
new ConnectionAdapter($connection),
Server::get(IEventDispatcher::class),
Server::get(LoggerInterface::class)
);
$scanner = $this->getScanner($output);
# check on each file/folder if there was a user interrupt (ctrl-c) and throw an exception
$scanner->listen('\OC\Files\Utils\Scanner', 'scanFile', function ($path) use ($output): void {
@ -142,6 +146,9 @@ class ScanAppData extends Base {
$folder = $input->getArgument('folder');
// Start the timer
$this->execTime = -microtime(true);
$this->initTools();
$exitCode = $this->scanFiles($output, $folder);
@ -155,8 +162,6 @@ class ScanAppData extends Base {
* Initialises some useful tools for the Command
*/
protected function initTools(): void {
// Start the timer
$this->execTime = -microtime(true);
// Convert PHP errors to exceptions
set_error_handler([$this, 'exceptionErrorHandler'], E_ALL);
}
@ -210,6 +215,11 @@ class ScanAppData extends Base {
$rows[] = $this->filesCounter;
$rows[] = $niceDate;
}
$this->displayTable($output, $headers, $rows);
}
protected function displayTable($output, $headers, $rows): void {
$table = new Table($output);
$table
->setHeaders($headers)
@ -250,9 +260,9 @@ class ScanAppData extends Base {
* @throws NotFoundException
*/
private function getAppDataFolder(): Node {
$instanceId = $this->config->getSystemValue('instanceid', null);
$instanceId = $this->config->getSystemValueString('instanceid', '');
if ($instanceId === null) {
if ($instanceId === '') {
throw new NotFoundException();
}

View file

@ -0,0 +1,216 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH
* SPDX-FileContributor: Carl Schwan
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCA\Files\Tests\Command;
use OC\Files\Utils\Scanner;
use OC\Preview\Db\Preview;
use OC\Preview\Db\PreviewMapper;
use OC\Preview\PreviewService;
use OC\Preview\Storage\StorageFactory;
use OCA\Files\Command\ScanAppData;
use OCP\Files\Folder;
use OCP\Files\IMimeTypeDetector;
use OCP\Files\IMimeTypeLoader;
use OCP\Files\IRootFolder;
use OCP\Files\ISetupManager;
use OCP\Files\NotFoundException;
use OCP\IAppConfig;
use OCP\IConfig;
use OCP\IUserManager;
use OCP\IUserSession;
use OCP\Server;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\Group;
use PHPUnit\Framework\MockObject\MockObject;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Test\TestCase;
#[Group(name: 'DB')]
class ScanAppDataTest extends TestCase {
private IRootFolder $rootFolder;
private IConfig $config;
private StorageFactory $storageFactory;
private OutputInterface&MockObject $output;
private InputInterface&MockObject $input;
private Scanner&MockObject $internalScanner;
private ScanAppData $scanner;
private string $user;
public function setUp(): void {
$this->config = Server::get(IConfig::class);
$this->rootFolder = Server::get(IRootFolder::class);
$this->storageFactory = Server::get(StorageFactory::class);
$this->user = static::getUniqueID('user');
$user = Server::get(IUserManager::class)->createUser($this->user, 'test');
Server::get(ISetupManager::class)->setupForUser($user);
Server::get(IUserSession::class)->setUser($user);
$this->output = $this->createMock(OutputInterface::class);
$this->input = $this->createMock(InputInterface::class);
$this->scanner = $this->getMockBuilder(ScanAppData::class)
->onlyMethods(['displayTable', 'initTools', 'getScanner'])
->setConstructorArgs([$this->rootFolder, $this->config, $this->storageFactory])
->getMock();
$this->internalScanner = $this->getMockBuilder(Scanner::class)
->onlyMethods(['scan'])
->disableOriginalConstructor()
->getMock();
$this->scanner->method('getScanner')->willReturn($this->internalScanner);
$this->scanner->method('initTools')
->willReturnCallback(function () {});
try {
$this->rootFolder->get($this->rootFolder->getAppDataDirectoryName() . '/preview')->delete();
} catch (NotFoundException) {
}
Server::get(PreviewService::class)->deleteAll();
try {
$appDataFolder = $this->rootFolder->get($this->rootFolder->getAppDataDirectoryName());
} catch (NotFoundException) {
$appDataFolder = $this->rootFolder->newFolder($this->rootFolder->getAppDataDirectoryName());
}
$appDataFolder->newFolder('preview');
}
public function tearDown(): void {
Server::get(IUserManager::class)->get($this->user)->delete();
Server::get(IUserSession::class)->setUser(null);
$this->rootFolder->get($this->rootFolder->getAppDataDirectoryName())->delete();
parent::tearDown();
}
public function testScanAppDataRoot(): void {
$this->input->method('getArgument')->with('folder')->willReturn('');
$this->internalScanner->method('scan')->willReturnCallback(function () {
$this->internalScanner->emit('\OC\Files\Utils\Scanner', 'scanFile', ['path42']);
$this->internalScanner->emit('\OC\Files\Utils\Scanner', 'scanFolder', ['path42']);
$this->internalScanner->emit('\OC\Files\Utils\Scanner', 'scanFolder', ['path42']);
});
$this->scanner->expects($this->once())->method('displayTable')
->willReturnCallback(function (OutputInterface $output, array $headers, array $rows): void {
$this->assertEquals($this->output, $output);
$this->assertEquals(['Previews', 'Folders', 'Files', 'Elapsed time'], $headers);
$this->assertEquals(0, $rows[0]);
$this->assertEquals(2, $rows[1]);
$this->assertEquals(1, $rows[2]);
});
$errorCode = $this->invokePrivate($this->scanner, 'execute', [$this->input, $this->output]);
$this->assertEquals(ScanAppData::SUCCESS, $errorCode);
}
public static function scanPreviewLocalData(): \Generator {
yield 'initial migration done' => [true, null];
yield 'initial migration not done' => [false, false];
yield 'initial migration not done with legacy paths' => [false, true];
}
#[DataProvider(methodName: 'scanPreviewLocalData')]
public function testScanAppDataPreviewOnlyLocalFile(bool $migrationDone, ?bool $legacy): void {
$this->input->method('getArgument')->with('folder')->willReturn('preview');
$file = $this->rootFolder->getUserFolder($this->user)->newFile('myfile.jpeg');
if ($migrationDone) {
Server::get(IAppConfig::class)->setValueBool('core', 'previewMovedDone', true);
$preview = new Preview();
$preview->generateId();
$preview->setFileId($file->getId());
$preview->setStorageId($file->getStorage()->getCache()->getNumericStorageId());
$preview->setEtag('abc');
$preview->setMtime(1);
$preview->setWidth(1024);
$preview->setHeight(1024);
$preview->setMimeType('image/jpeg');
$preview->setSize($this->storageFactory->writePreview($preview, 'preview content'));
Server::get(PreviewMapper::class)->insert($preview);
$preview = new Preview();
$preview->generateId();
$preview->setFileId($file->getId());
$preview->setStorageId($file->getStorage()->getCache()->getNumericStorageId());
$preview->setEtag('abc');
$preview->setMtime(1);
$preview->setWidth(2024);
$preview->setHeight(2024);
$preview->setMax(true);
$preview->setMimeType('image/jpeg');
$preview->setSize($this->storageFactory->writePreview($preview, 'preview content'));
Server::get(PreviewMapper::class)->insert($preview);
$preview = new Preview();
$preview->generateId();
$preview->setFileId($file->getId());
$preview->setStorageId($file->getStorage()->getCache()->getNumericStorageId());
$preview->setEtag('abc');
$preview->setMtime(1);
$preview->setWidth(2024);
$preview->setHeight(2024);
$preview->setMax(true);
$preview->setCropped(true);
$preview->setMimeType('image/jpeg');
$preview->setSize($this->storageFactory->writePreview($preview, 'preview content'));
Server::get(PreviewMapper::class)->insert($preview);
$previews = Server::get(PreviewService::class)->getAvailablePreviews([$file->getId()]);
$this->assertCount(3, $previews[$file->getId()]);
} else {
Server::get(IAppConfig::class)->setValueBool('core', 'previewMovedDone', false);
/** @var Folder $previewFolder */
$previewFolder = $this->rootFolder->get($this->rootFolder->getAppDataDirectoryName() . '/preview');
if (!$legacy) {
foreach (str_split(substr(md5((string)$file->getId()), 0, 7)) as $subPath) {
$previewFolder = $previewFolder->newFolder($subPath);
}
}
$previewFolder = $previewFolder->newFolder((string)$file->getId());
$previewFolder->newFile('1024-1024.jpg');
$previewFolder->newFile('2024-2024-max.jpg');
$previewFolder->newFile('2024-2024-max-crop.jpg');
$this->assertCount(3, $previewFolder->getDirectoryListing());
$previews = Server::get(PreviewService::class)->getAvailablePreviews([$file->getId()]);
$this->assertCount(0, $previews[$file->getId()]);
}
$mimetypeDetector = $this->createMock(IMimeTypeDetector::class);
$mimetypeDetector->method('detectPath')->willReturn('image/jpeg');
$appConfig = $this->createMock(IAppConfig::class);
$appConfig->method('getValueBool')->with('core', 'previewMovedDone')->willReturn($migrationDone);
$mimetypeLoader = $this->createMock(IMimeTypeLoader::class);
$mimetypeLoader->method('getMimetypeById')->willReturn('image/jpeg');
$this->scanner->expects($this->once())->method('displayTable')
->willReturnCallback(function ($output, array $headers, array $rows): void {
$this->assertEquals($output, $this->output);
$this->assertEquals(['Previews', 'Folders', 'Files', 'Elapsed time'], $headers);
$this->assertEquals(3, $rows[0]);
$this->assertEquals(0, $rows[1]);
$this->assertEquals(0, $rows[2]);
});
$errorCode = $this->invokePrivate($this->scanner, 'execute', [$this->input, $this->output]);
$this->assertEquals(ScanAppData::SUCCESS, $errorCode);
/** @var Folder $previewFolder */
$previewFolder = $this->rootFolder->get($this->rootFolder->getAppDataDirectoryName() . '/preview');
$children = $previewFolder->getDirectoryListing();
$this->assertCount(0, $children);
Server::get(PreviewService::class)->deleteAll();
}
}

View file

@ -1287,9 +1287,6 @@
<InvalidArgument>
<code><![CDATA[[$this, 'exceptionErrorHandler']]]></code>
</InvalidArgument>
<NullArgument>
<code><![CDATA[null]]></code>
</NullArgument>
</file>
<file src="apps/files/lib/Controller/ApiController.php">
<DeprecatedMethod>

View file

@ -438,7 +438,7 @@ class Scanner extends BasicEmitter implements IScanner {
* @param bool|IScanner::SCAN_RECURSIVE_INCOMPLETE $recursive
*/
private function handleChildren(string $path, $recursive, int $reuse, int $folderId, bool $lock, int|float &$size, bool &$etagChanged): array {
// we put this in it's own function so it cleans up the memory before we start recursing
// we put this in its own function so it cleans up the memory before we start recursing
$existingChildren = $this->getExistingChildren($folderId);
$newChildren = iterator_to_array($this->storage->getDirectoryContent($path));

View file

@ -57,7 +57,7 @@ class Scanner extends PublicEmitter {
protected int $entriesToCommit = 0;
public function __construct(
private string $user,
private ?string $user,
protected ?IDBConnection $db,
private IEventDispatcher $dispatcher,
protected LoggerInterface $logger,

View file

@ -100,20 +100,30 @@ class Preview extends SnowflakeAwareEntity {
$preview->setFileId((int)basename(dirname($path)));
$fileName = pathinfo($path, PATHINFO_FILENAME) . '.' . pathinfo($path, PATHINFO_EXTENSION);
$ok = preg_match('/(([A-Za-z0-9\+\/]+)-)?([0-9]+)-([0-9]+)(-(max))?(-(crop))?\.([a-z]{3,4})/', $fileName, $matches);
$ok = preg_match('/(([A-Za-z0-9\+\/]+)-)?([0-9]+)-([0-9]+)(-(crop))?(-(max))?\.([a-z]{3,4})$/', $fileName, $matches);
if ($ok !== 1) {
return false;
$ok = preg_match('/(([A-Za-z0-9\+\/]+)-)?([0-9]+)-([0-9]+)(-(max))?(-(crop))?\.([a-z]{3,4})$/', $fileName, $matches);
if ($ok !== 1) {
return false;
}
[
2 => $version,
3 => $width,
4 => $height,
6 => $max,
8 => $crop,
] = $matches;
} else {
[
2 => $version,
3 => $width,
4 => $height,
6 => $crop,
8 => $max,
] = $matches;
}
[
2 => $version,
3 => $width,
4 => $height,
6 => $max,
8 => $crop,
] = $matches;
$preview->setMimeType($mimeTypeDetector->detectPath($fileName));
$preview->setWidth((int)$width);

View file

@ -66,11 +66,11 @@ class PreviewMigrationService {
$path = $fileId . '/' . $previewFile->getName();
/** @var SimpleFile $previewFile */
$preview = Preview::fromPath($path, $this->mimeTypeDetector);
$preview->generateId();
if (!$preview) {
if ($preview === false) {
$this->logger->error('Unable to import old preview at path.');
continue;
}
$preview->generateId();
$preview->setSize($previewFile->getSize());
$preview->setMtime($previewFile->getMtime());
$preview->setOldFileId($previewFile->getId());
@ -93,17 +93,17 @@ class PreviewMigrationService {
->setMaxResults(1);
$result = $qb->executeQuery();
$result = $result->fetchAllAssociative();
$result = $result->fetchAssociative();
if (count($result) > 0) {
if ($result !== false) {
foreach ($previewFiles as $previewFile) {
/** @var Preview $preview */
$preview = $previewFile['preview'];
/** @var SimpleFile $file */
$file = $previewFile['file'];
$preview->setStorageId($result[0]['storage']);
$preview->setEtag($result[0]['etag']);
$preview->setSourceMimeType($this->mimeTypeLoader->getMimetypeById((int)$result[0]['mimetype']));
$preview->setStorageId($result['storage']);
$preview->setEtag($result['etag']);
$preview->setSourceMimeType($this->mimeTypeLoader->getMimetypeById((int)$result['mimetype']));
$preview->generateId();
try {
$preview = $this->previewMapper->insert($preview);

View file

@ -18,21 +18,18 @@ use OC\Preview\Db\PreviewMapper;
use OCP\DB\Exception;
use OCP\Files\IMimeTypeDetector;
use OCP\Files\IMimeTypeLoader;
use OCP\Files\IRootFolder;
use OCP\Files\NotFoundException;
use OCP\Files\NotPermittedException;
use OCP\IAppConfig;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\Snowflake\ISnowflakeGenerator;
use Override;
use Psr\Log\LoggerInterface;
use RecursiveDirectoryIterator;
use RecursiveIteratorIterator;
class LocalPreviewStorage implements IPreviewStorage {
private readonly string $rootFolder;
private readonly string $instanceId;
public function __construct(
private readonly IConfig $config,
private readonly PreviewMapper $previewMapper,
@ -40,11 +37,9 @@ class LocalPreviewStorage implements IPreviewStorage {
private readonly IDBConnection $connection,
private readonly IMimeTypeDetector $mimeTypeDetector,
private readonly LoggerInterface $logger,
private readonly ISnowflakeGenerator $generator,
private readonly IMimeTypeLoader $mimeTypeLoader,
private readonly IRootFolder $rootFolder,
) {
$this->instanceId = $this->config->getSystemValueString('instanceid');
$this->rootFolder = $this->config->getSystemValue('datadirectory', OC::$SERVERROOT . '/data');
}
#[Override]
@ -72,8 +67,12 @@ class LocalPreviewStorage implements IPreviewStorage {
}
}
public function getRootFolder(): string {
return $this->config->getSystemValueString('datadirectory', OC::$SERVERROOT . '/data');
}
public function getPreviewRootFolder(): string {
return $this->rootFolder . '/appdata_' . $this->instanceId . '/preview/';
return $this->getRootFolder() . '/' . $this->rootFolder->getAppDataDirectoryName() . '/preview/';
}
private function constructPath(Preview $preview): string {
@ -113,16 +112,19 @@ class LocalPreviewStorage implements IPreviewStorage {
public function scan(): int {
$checkForFileCache = !$this->appConfig->getValueBool('core', 'previewMovedDone');
if (!file_exists($this->getPreviewRootFolder())) {
return 0;
}
$scanner = new RecursiveDirectoryIterator($this->getPreviewRootFolder());
$previewsFound = 0;
$skipFiles = [];
foreach (new RecursiveIteratorIterator($scanner) as $file) {
if ($file->isFile()) {
if ($file->isFile() && !in_array((string)$file, $skipFiles, true)) {
$preview = Preview::fromPath((string)$file, $this->mimeTypeDetector);
if ($preview === false) {
$this->logger->error('Unable to parse preview information for ' . $file->getRealPath());
continue;
}
$preview->generateId();
try {
$preview->setSize($file->getSize());
$preview->setMtime($file->getMtime());
@ -139,23 +141,34 @@ class LocalPreviewStorage implements IPreviewStorage {
if ($result === false) {
// original file is deleted
$this->logger->warning('Original file ' . $preview->getFileId() . ' was not found. Deleting preview at ' . $file->getRealPath());
@unlink($file->getRealPath());
continue;
}
if ($checkForFileCache) {
$relativePath = str_replace($this->rootFolder . '/', '', $file->getRealPath());
$rowAffected = $qb->delete('filecache')
$relativePath = str_replace($this->getRootFolder() . '/', '', $file->getRealPath());
$qb = $this->connection->getQueryBuilder();
$result2 = $qb->select('fileid', 'storage', 'etag', 'mimetype', 'parent')
->from('filecache')
->where($qb->expr()->eq('path_hash', $qb->createNamedParameter(md5($relativePath))))
->executeStatement();
if ($rowAffected > 0) {
$this->deleteParentsFromFileCache(dirname($relativePath));
->runAcrossAllShards()
->setMaxResults(1)
->executeQuery()
->fetchAssociative();
if ($result2 !== false) {
$qb->delete('filecache')
->where($qb->expr()->eq('fileid', $qb->createNamedParameter($result2['fileid'])))
->andWhere($qb->expr()->eq('storage', $qb->createNamedParameter($result2['storage'])))
->executeStatement();
$this->deleteParentsFromFileCache((int)$result2['parent'], (int)$result2['storage']);
}
}
$preview->setStorageId($result['storage']);
$preview->setStorageId((int)$result['storage']);
$preview->setEtag($result['etag']);
$preview->setSourceMimetype($this->mimeTypeLoader->getMimetypeById($result['mimetype']));
$preview->setSourceMimetype($this->mimeTypeLoader->getMimetypeById((int)$result['mimetype']));
$preview->generateId();
// try to insert, if that fails the preview is already in the DB
$this->previewMapper->insert($preview);
@ -169,6 +182,8 @@ class LocalPreviewStorage implements IPreviewStorage {
if (!$ok) {
throw new LogicException('Failed to move ' . $file->getRealPath() . ' to ' . $previewPath);
}
$skipFiles[] = $previewPath;
}
} catch (Exception $e) {
if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
@ -182,65 +197,45 @@ class LocalPreviewStorage implements IPreviewStorage {
return $previewsFound;
}
private function deleteParentsFromFileCache(string $dirname): void {
/**
* Recursive method that deletes the folder and its parent folders if it's not
* empty.
*/
private function deleteParentsFromFileCache(int $folderId, int $storageId): void {
$qb = $this->connection->getQueryBuilder();
$result = $qb->select('fileid', 'path', 'storage', 'parent')
->from('filecache')
->where($qb->expr()->eq('path_hash', $qb->createNamedParameter(md5($dirname))))
->where($qb->expr()->eq('parent', $qb->createNamedParameter($folderId)))
->setMaxResults(1)
->runAcrossAllShards()
->executeQuery()
->fetchAll();
->fetchAssociative();
if (empty($result)) {
if ($result !== false) {
// there are other files in the directory, don't delete yet
return;
}
$this->connection->beginTransaction();
// Get new parent
$qb = $this->connection->getQueryBuilder();
$result = $qb->select('fileid', 'path', 'parent')
->from('filecache')
->where($qb->expr()->eq('fileid', $qb->createNamedParameter($folderId)))
->andWhere($qb->expr()->eq('storage', $qb->createNamedParameter($storageId)))
->setMaxResults(1)
->executeQuery()
->fetchAssociative();
$parentId = $result[0]['parent'];
$fileId = $result[0]['fileid'];
$storage = $result[0]['storage'];
if ($result !== false) {
$parentFolderId = (int)$result['parent'];
try {
while (true) {
$qb = $this->connection->getQueryBuilder();
$childs = $qb->select('fileid', 'path', 'storage')
->from('filecache')
->where($qb->expr()->eq('parent', $qb->createNamedParameter($fileId)))
->hintShardKey('storage', $storage)
->executeQuery()
->fetchAll();
$qb = $this->connection->getQueryBuilder();
$qb->delete('filecache')
->where($qb->expr()->eq('fileid', $qb->createNamedParameter($folderId)))
->andWhere($qb->expr()->eq('storage', $qb->createNamedParameter($storageId)))
->executeStatement();
if (!empty($childs)) {
break;
}
$qb = $this->connection->getQueryBuilder();
$qb->delete('filecache')
->where($qb->expr()->eq('fileid', $qb->createNamedParameter($fileId)))
->hintShardKey('storage', $result[0]['storage'])
->executeStatement();
$qb = $this->connection->getQueryBuilder();
$result = $qb->select('fileid', 'path', 'storage', 'parent')
->from('filecache')
->where($qb->expr()->eq('fileid', $qb->createNamedParameter($parentId)))
->setMaxResults(1)
->hintShardKey('storage', $storage)
->executeQuery()
->fetchAll();
if (empty($result)) {
break;
}
$fileId = $parentId;
$parentId = $result[0]['parent'];
}
} finally {
$this->connection->commit();
$this->deleteParentsFromFileCache($parentFolderId, $storageId);
}
}
}