Skip to content

Commit 24dc8a1

Browse files
CarlSchwanCarlSchwan
committed
perf(preview): Adapt BackgroundCleanupJob to new previews table
Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>
1 parent e55c56d commit 24dc8a1

File tree

7 files changed

+128
-235
lines changed

7 files changed

+128
-235
lines changed

‎build/psalm-baseline.xml‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4090,16 +4090,6 @@
40904090
<code><![CDATA[\OCA\Notifications\App]]></code>
40914091
</UndefinedClass>
40924092
</file>
4093-
<file src="lib/private/Preview/BackgroundCleanupJob.php">
4094-
<InvalidReturnStatement>
4095-
<code><![CDATA[[]]]></code>
4096-
</InvalidReturnStatement>
4097-
</file>
4098-
<file src="lib/private/Preview/Generator.php">
4099-
<LessSpecificReturnType>
4100-
<code><![CDATA[null|string]]></code>
4101-
</LessSpecificReturnType>
4102-
</file>
41034093
<file src="lib/private/Preview/ProviderV1Adapter.php">
41044094
<InvalidReturnStatement>
41054095
<code><![CDATA[$thumbnail === false ? null: $thumbnail]]></code>
@@ -4401,14 +4391,6 @@
44014391
<code><![CDATA[array{X-Request-Id: string, Cache-Control: string, Content-Security-Policy: string, Feature-Policy: string, X-Robots-Tag: string, Last-Modified?: string, ETag?: string, ...H}]]></code>
44024392
</MoreSpecificReturnType>
44034393
</file>
4404-
<file src="lib/public/Preview/BeforePreviewFetchedEvent.php">
4405-
<LessSpecificReturnStatement>
4406-
<code><![CDATA[$this->mode]]></code>
4407-
</LessSpecificReturnStatement>
4408-
<MoreSpecificReturnType>
4409-
<code><![CDATA[null|IPreview::MODE_FILL|IPreview::MODE_COVER]]></code>
4410-
</MoreSpecificReturnType>
4411-
</file>
44124394
<file src="ocs-provider/index.php">
44134395
<DeprecatedMethod>
44144396
<code><![CDATA[getAppManager]]></code>

‎lib/private/Preview/BackgroundCleanupJob.php‎

Lines changed: 75 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -8,112 +8,63 @@
88
*/
99
namespace OC\Preview;
1010

11-
use OC\Preview\Storage\Root;
11+
use OC\Preview\Db\PreviewMapper;
12+
use OC\Preview\Storage\StorageFactory;
1213
use OCP\AppFramework\Utility\ITimeFactory;
1314
use OCP\BackgroundJob\TimedJob;
1415
use OCP\DB\QueryBuilder\IQueryBuilder;
15-
use OCP\Files\IMimeTypeLoader;
16-
use OCP\Files\NotFoundException;
17-
use OCP\Files\NotPermittedException;
1816
use OCP\IDBConnection;
1917

18+
/**
19+
* @psalm-type FileId int
20+
* @psalm-type StorageId int
21+
*/
2022
class BackgroundCleanupJob extends TimedJob {
2123

2224
public function __construct(
2325
ITimeFactory $timeFactory,
24-
private IDBConnection $connection,
25-
private Root $previewFolder,
26-
private IMimeTypeLoader $mimeTypeLoader,
27-
private bool $isCLI,
26+
readonly private IDBConnection $connection,
27+
readonly private PreviewMapper $previewMapper,
28+
readonly private StorageFactory $storageFactory,
29+
readonly private bool $isCLI,
2830
) {
2931
parent::__construct($timeFactory);
3032
// Run at most once an hour
3133
$this->setInterval(60 * 60);
3234
$this->setTimeSensitivity(self::TIME_INSENSITIVE);
3335
}
3436

35-
public function run($argument) {
36-
foreach ($this->getDeletedFiles() as $fileId) {
37-
try {
38-
$preview = $this->previewFolder->getFolder((string)$fileId);
39-
$preview->delete();
40-
} catch (NotFoundException $e) {
41-
// continue
42-
} catch (NotPermittedException $e) {
43-
// continue
37+
public function run($argument): void {
38+
foreach ($this->getDeletedFiles() as $chunk) {
39+
foreach ($chunk as $storage => $fileIds) {
40+
foreach ($this->previewMapper->getByFileIds($storage, $fileIds) as $previews) {
41+
$previewIds = [];
42+
foreach ($previews as $preview) {
43+
$previewIds[] = $preview->getId();
44+
$this->storageFactory->deletePreview($preview);
45+
}
46+
47+
$this->previewMapper->deleteByIds($storage, $previewIds);
48+
};
4449
}
4550
}
4651
}
4752

53+
/**
54+
* @return \Iterator<array<StorageId, FileId[]>>
55+
*/
4856
private function getDeletedFiles(): \Iterator {
49-
yield from $this->getOldPreviewLocations();
50-
yield from $this->getNewPreviewLocations();
51-
}
52-
53-
private function getOldPreviewLocations(): \Iterator {
54-
if ($this->connection->getShardDefinition('filecache')) {
55-
// sharding is new enough that we don't need to support this
56-
return;
57-
}
58-
59-
$qb = $this->connection->getQueryBuilder();
60-
$qb->select('a.name')
61-
->from('filecache', 'a')
62-
->leftJoin('a', 'filecache', 'b', $qb->expr()->eq(
63-
$qb->expr()->castColumn('a.name', IQueryBuilder::PARAM_INT), 'b.fileid'
64-
))
65-
->where(
66-
$qb->expr()->andX(
67-
$qb->expr()->isNull('b.fileid'),
68-
$qb->expr()->eq('a.storage', $qb->createNamedParameter($this->previewFolder->getStorageId())),
69-
$qb->expr()->eq('a.parent', $qb->createNamedParameter($this->previewFolder->getId())),
70-
$qb->expr()->like('a.name', $qb->createNamedParameter('__%')),
71-
$qb->expr()->eq('a.mimetype', $qb->createNamedParameter($this->mimeTypeLoader->getId('httpd/unix-directory')))
72-
)
73-
);
74-
75-
if (!$this->isCLI) {
76-
$qb->setMaxResults(10);
77-
}
78-
79-
$cursor = $qb->executeQuery();
80-
81-
while ($row = $cursor->fetch()) {
82-
yield $row['name'];
83-
}
84-
85-
$cursor->closeCursor();
86-
}
87-
88-
private function getNewPreviewLocations(): \Iterator {
89-
$qb = $this->connection->getQueryBuilder();
90-
$qb->select('path', 'mimetype')
91-
->from('filecache')
92-
->where($qb->expr()->eq('fileid', $qb->createNamedParameter($this->previewFolder->getId())));
93-
$cursor = $qb->executeQuery();
94-
$data = $cursor->fetch();
95-
$cursor->closeCursor();
96-
97-
if ($data === null) {
98-
return [];
99-
}
100-
10157
if ($this->connection->getShardDefinition('filecache')) {
102-
$chunks = $this->getAllPreviewIds($data['path'], 1000);
58+
$chunks = $this->getAllPreviewIds(1000);
10359
foreach ($chunks as $chunk) {
104-
yield from $this->findMissingSources($chunk);
60+
foreach ($chunk as $storage => $preview) {
61+
yield [$storage => $this->findMissingSources($storage, $preview)];
62+
}
10563
}
10664

10765
return;
10866
}
10967

110-
/*
111-
* This lovely like is the result of the way the new previews are stored
112-
* We take the md5 of the name (fileid) and split the first 7 chars. That way
113-
* there are not a gazillion files in the root of the preview appdata.
114-
*/
115-
$like = $this->connection->escapeLikeParameter($data['path']) . '/_/_/_/_/_/_/_/%';
116-
11768
/*
11869
* Deleting a file will not delete related previews right away.
11970
*
@@ -130,71 +81,85 @@ private function getNewPreviewLocations(): \Iterator {
13081
* If the related file is deleted, b.fileid will be null and the preview folder can be deleted.
13182
*/
13283
$qb = $this->connection->getQueryBuilder();
133-
$qb->select('a.name')
134-
->from('filecache', 'a')
135-
->leftJoin('a', 'filecache', 'b', $qb->expr()->eq(
136-
$qb->expr()->castColumn('a.name', IQueryBuilder::PARAM_INT), 'b.fileid'
84+
$qb->select('p.storage_id', 'p.file_id')
85+
->from('previews', 'p')
86+
->leftJoin('p', 'filecache', 'f', $qb->expr()->eq(
87+
'p.file_id', 'f.fileid'
13788
))
138-
->where(
139-
$qb->expr()->andX(
140-
$qb->expr()->eq('a.storage', $qb->createNamedParameter($this->previewFolder->getStorageId())),
141-
$qb->expr()->isNull('b.fileid'),
142-
$qb->expr()->like('a.path', $qb->createNamedParameter($like)),
143-
$qb->expr()->eq('a.mimetype', $qb->createNamedParameter($this->mimeTypeLoader->getId('httpd/unix-directory')))
144-
)
145-
);
89+
->where($qb->expr()->isNull('f.fileid'));
14690

14791
if (!$this->isCLI) {
14892
$qb->setMaxResults(10);
14993
}
15094

15195
$cursor = $qb->executeQuery();
15296

97+
$lastStorageId = null;
98+
/** @var FileId[] $tmpResult */
99+
$tmpResult = [];
153100
while ($row = $cursor->fetch()) {
154-
yield $row['name'];
101+
if ($lastStorageId === null) {
102+
$lastStorageId = $row['storage_id'];
103+
} else if ($lastStorageId !== $row['storage_id']) {
104+
yield [$lastStorageId => $tmpResult];
105+
$tmpResult = [];
106+
$lastStorageId = $row['storage_id'];
107+
}
108+
$tmpResult[] = $row['file_id'];
109+
}
110+
111+
if (!empty($tmpResult)) {
112+
yield [$lastStorageId => $tmpResult];
155113
}
156114

157115
$cursor->closeCursor();
158116
}
159117

160-
private function getAllPreviewIds(string $previewRoot, int $chunkSize): \Iterator {
161-
// See `getNewPreviewLocations` for some more info about the logic here
162-
$like = $this->connection->escapeLikeParameter($previewRoot) . '/_/_/_/_/_/_/_/%';
163-
118+
/**
119+
* @return \Iterator<array<StorageId, FileId[]>>
120+
*/
121+
private function getAllPreviewIds(int $chunkSize): \Iterator {
164122
$qb = $this->connection->getQueryBuilder();
165-
$qb->select('name', 'fileid')
166-
->from('filecache')
123+
$qb->select('id', 'file_id', 'storage_id')
124+
->from('previews')
167125
->where(
168-
$qb->expr()->andX(
169-
$qb->expr()->eq('storage', $qb->createNamedParameter($this->previewFolder->getStorageId())),
170-
$qb->expr()->like('path', $qb->createNamedParameter($like)),
171-
$qb->expr()->eq('mimetype', $qb->createNamedParameter($this->mimeTypeLoader->getId('httpd/unix-directory'))),
172-
$qb->expr()->gt('fileid', $qb->createParameter('min_id')),
173-
)
126+
$qb->expr()->gt('id', $qb->createParameter('min_id')),
174127
)
175-
->orderBy('fileid', 'ASC')
128+
->orderBy('id', 'ASC')
176129
->setMaxResults($chunkSize);
177130

178131
$minId = 0;
179132
while (true) {
180133
$qb->setParameter('min_id', $minId);
181134
$rows = $qb->executeQuery()->fetchAll();
182135
if (count($rows) > 0) {
183-
$minId = $rows[count($rows) - 1]['fileid'];
184-
yield array_map(function ($row) {
185-
return (int)$row['name'];
186-
}, $rows);
136+
$minId = $rows[count($rows) - 1]['id'];
137+
$result = [];
138+
foreach ($rows as $row) {
139+
if (!isset($result[$row['storage_id']])) {
140+
$result[$row['storage_id']] = [];
141+
}
142+
$result[$row['storage_id']][] = $row['file_id'];
143+
}
144+
yield $result;
187145
} else {
188146
break;
189147
}
190148
}
191149
}
192150

193-
private function findMissingSources(array $ids): array {
151+
/**
152+
* @param FileId[] $ids
153+
* @return FileId[]
154+
*/
155+
private function findMissingSources(int $storage, array $ids): array {
194156
$qb = $this->connection->getQueryBuilder();
195157
$qb->select('fileid')
196158
->from('filecache')
197-
->where($qb->expr()->in('fileid', $qb->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY)));
159+
->where($qb->expr()->andX(
160+
$qb->expr()->in('fileid', $qb->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY)),
161+
$qb->expr()->eq('storage', $qb->createNamedParameter($storage, IQueryBuilder::PARAM_INT)),
162+
));
198163
$found = $qb->executeQuery()->fetchAll(\PDO::FETCH_COLUMN);
199164
return array_diff($ids, $found);
200165
}

‎lib/private/Preview/Db/PreviewMapper.php‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,4 +63,34 @@ public function getPreview(int $fileId, int $width, int $height, string $mode, i
6363
return null;
6464
}
6565
}
66+
67+
/**
68+
* @param int[] $fileIds
69+
* @return array<int, Preview[]>
70+
*/
71+
public function getByFileIds(int $storageId, array $fileIds): array {
72+
$selectQb = $this->db->getQueryBuilder();
73+
$selectQb->select('*')
74+
->from(self::TABLE_NAME)
75+
->where($selectQb->expr()->andX(
76+
$selectQb->expr()->in('file_id', $selectQb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY)),
77+
));
78+
$previews = array_fill_keys($fileIds, []);
79+
foreach ($this->yieldEntities($selectQb) as $preview) {
80+
$previews[$preview->getFileId()][] = $preview;
81+
}
82+
return $previews;
83+
}
84+
85+
/**
86+
* @param int[] $previewIds
87+
*/
88+
public function deleteByIds(int $storageId, array $previewIds): void {
89+
$qb = $this->db->getQueryBuilder();
90+
$qb->delete(self::TABLE_NAME)
91+
->where($qb->expr()->andX(
92+
$qb->expr()->eq('storage_id', $qb->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)),
93+
$qb->expr()->in('id', $qb->createNamedParameter($previewIds, IQueryBuilder::PARAM_INT_ARRAY))
94+
))->executeStatement();
95+
}
6696
}

‎lib/private/Preview/Generator.php‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,6 @@ public function generatePreviews(File $file, array $specifications, ?string $mim
174174
if ($maxPreviewImage === null) {
175175
$maxPreviewImage = $this->helper->getImage(new PreviewFile($maxPreview, $this->storageFactory, $this->previewMapper));
176176
}
177-
assert($maxPreviewImage);
178177

179178
$this->logger->debug('Cached preview not found for file {path}, generating a new preview.', ['path' => $file->getPath()]);
180179
$previewFile = $this->generatePreview($file, $maxPreviewImage, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion, $cacheResult);
@@ -508,7 +507,6 @@ private function generatePreview(
508507
self::unguardWithSemaphore($sem);
509508
}
510509

511-
512510
$path = $this->generatePath($width, $height, $crop, false, $preview->dataMimeType(), $version);
513511
if ($cacheResult) {
514512
$previewEntry = $this->savePreview($file, $width, $height, $crop, false, $preview, $version);
@@ -519,11 +517,9 @@ private function generatePreview(
519517
}
520518

521519
/**
522-
* @param string $mimeType
523-
* @return null|string
524520
* @throws \InvalidArgumentException
525521
*/
526-
private function getExtension($mimeType) {
522+
private function getExtension(string $mimeType): string {
527523
switch ($mimeType) {
528524
case 'image/png':
529525
return 'png';

‎lib/public/AppFramework/Db/QBMapper.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,6 @@ public function delete(Entity $entity): Entity {
8484
return $entity;
8585
}
8686

87-
8887
/**
8988
* Creates a new entry in the db from an entity
9089
*

‎lib/public/Preview/BeforePreviewFetchedEvent.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
*/
2222
class BeforePreviewFetchedEvent extends \OCP\EventDispatcher\Event {
2323
/**
24+
* @param null|IPreview::MODE_FILL|IPreview::MODE_COVER $mode
2425
* @since 25.0.1
2526
*/
2627
public function __construct(

0 commit comments

Comments
 (0)