From 72a43a17f557a04d7b925d159c9dfccd20f20fef Mon Sep 17 00:00:00 2001
From: Dario Mehlich <d.mehlich@gmail.com>
Date: Mon, 27 Jan 2025 10:16:08 +0100
Subject: [PATCH] fix(preview): Filter for folders in cleanup old preview job
Fixes #35936.
When running `OC\Preview\BackgroundCleanupJob`, the main iteration loop
in `run()` expects a folder, however, `getOldPreviewLocations()`
currently does not filter by mimetype and therefore can yield a
non-folder entry which causes an Exception when constructing the Folder
impl.
Filtering for `httpd/unix-directory`, as `getNewPreviewLocations()`
already does, fixes this issue.
Signed-off-by: Dario Mehlich <d.mehlich@gmail.com>
---
lib/private/Preview/BackgroundCleanupJob.php | 14 ++++-----
.../lib/Preview/BackgroundCleanupJobTest.php | 31 +++++++++++++++++++
2 files changed, 38 insertions(+), 7 deletions(-)
diff --git a/lib/private/Preview/BackgroundCleanupJob.php b/lib/private/Preview/BackgroundCleanupJob.php
index e816ae2274303..3138abb1bf988 100644
--- a/lib/private/Preview/BackgroundCleanupJob.php
+++ b/lib/private/Preview/BackgroundCleanupJob.php
@@ -63,13 +63,13 @@ private function getOldPreviewLocations(): \Iterator {
$qb->expr()->castColumn('a.name', IQueryBuilder::PARAM_INT), 'b.fileid'
))
->where(
- $qb->expr()->isNull('b.fileid')
- )->andWhere(
- $qb->expr()->eq('a.storage', $qb->createNamedParameter($this->previewFolder->getStorageId()))
- )->andWhere(
- $qb->expr()->eq('a.parent', $qb->createNamedParameter($this->previewFolder->getId()))
- )->andWhere(
- $qb->expr()->like('a.name', $qb->createNamedParameter('__%'))
+ $qb->expr()->andX(
+ $qb->expr()->isNull('b.fileid'),
+ $qb->expr()->eq('a.storage', $qb->createNamedParameter($this->previewFolder->getStorageId())),
+ $qb->expr()->eq('a.parent', $qb->createNamedParameter($this->previewFolder->getId())),
+ $qb->expr()->like('a.name', $qb->createNamedParameter('__%')),
+ $qb->expr()->eq('a.mimetype', $qb->createNamedParameter($this->mimeTypeLoader->getId('httpd/unix-directory')))
+ )
);
if (!$this->isCLI) {
diff --git a/tests/lib/Preview/BackgroundCleanupJobTest.php b/tests/lib/Preview/BackgroundCleanupJobTest.php
index 9c521376af5ad..945366cde9f4a 100644
--- a/tests/lib/Preview/BackgroundCleanupJobTest.php
+++ b/tests/lib/Preview/BackgroundCleanupJobTest.php
@@ -191,13 +191,44 @@ public function testOldPreviews(): void {
$f2 = $appdata->newFolder((string)PHP_INT_MAX - 1);
$f2->newFile('foo.jpg', 'foo');
+ /*
+ * Cleanup of OldPreviewLocations should only remove numeric folders on AppData level,
+ * therefore these files should stay untouched.
+ */
+ $appdata->getFolder('/')->newFile('not-a-directory', 'foo');
+ $appdata->getFolder('/')->newFile('133742', 'bar');
+
$appdata = \OC::$server->getAppDataDir('preview');
+ // AppData::getDirectoryListing filters all non-folders
$this->assertSame(3, count($appdata->getDirectoryListing()));
+ try {
+ $appdata->getFolder('/')->getFile('not-a-directory');
+ } catch (NotFoundException) {
+ $this->fail('Could not find file \'not-a-directory\'');
+ }
+ try {
+ $appdata->getFolder('/')->getFile('133742');
+ } catch (NotFoundException) {
+ $this->fail('Could not find file \'133742\'');
+ }
$job = new BackgroundCleanupJob($this->timeFactory, $this->connection, $this->getRoot(), $this->mimeTypeLoader, true);
$job->run([]);
$appdata = \OC::$server->getAppDataDir('preview');
+
+ // Check if the files created above are still present
+ // Remember: AppData::getDirectoryListing filters all non-folders
$this->assertSame(0, count($appdata->getDirectoryListing()));
+ try {
+ $appdata->getFolder('/')->getFile('not-a-directory');
+ } catch (NotFoundException) {
+ $this->fail('Could not find file \'not-a-directory\'');
+ }
+ try {
+ $appdata->getFolder('/')->getFile('133742');
+ } catch (NotFoundException) {
+ $this->fail('Could not find file \'133742\'');
+ }
}
}