Skip to content

Commit e9fce36

Browse files
CarlSchwanCarlSchwan
committed
fix(preview): Make version column a string
And move it to a different table so that we don't have to pay the storage cost when not using it (most of the times). Signed-off-by: Carl Schwan <[email protected]>
1 parent 66f50bd commit e9fce36

22 files changed

+314
-196
lines changed

‎core/BackgroundJobs/MovePreviewJob.php‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use OCP\Files\NotFoundException;
2626
use OCP\Files\SimpleFS\ISimpleFolder;
2727
use OCP\IAppConfig;
28+
use OCP\IConfig;
2829
use OCP\IDBConnection;
2930
use Override;
3031
use Psr\Log\LoggerInterface;
@@ -35,6 +36,7 @@ class MovePreviewJob extends TimedJob {
3536
public function __construct(
3637
ITimeFactory $time,
3738
private readonly IAppConfig $appConfig,
39+
private readonly IConfig $config,
3840
private readonly PreviewMapper $previewMapper,
3941
private readonly StorageFactory $storageFactory,
4042
private readonly IDBConnection $connection,
@@ -109,12 +111,6 @@ protected function run(mixed $argument): void {
109111
}
110112
}
111113

112-
try {
113-
// Delete any leftover preview directory
114-
$this->appData->getFolder('.')->delete();
115-
} catch (NotFoundException) {
116-
// ignore
117-
}
118114
$this->appConfig->setValueBool('core', 'previewMovedDone', true);
119115
}
120116

@@ -133,7 +129,7 @@ private function processPreviews(int|string $fileId, bool $simplePaths): void {
133129
foreach ($folder->getDirectoryListing() as $previewFile) {
134130
$path = $fileId . '/' . $previewFile->getName();
135131
/** @var SimpleFile $previewFile */
136-
$preview = Preview::fromPath($path, $this->mimeTypeDetector, $this->mimeTypeLoader);
132+
$preview = Preview::fromPath($path, $this->mimeTypeDetector);
137133
if (!$preview) {
138134
$this->logger->error('Unable to import old preview at path.');
139135
continue;
@@ -160,12 +156,13 @@ private function processPreviews(int|string $fileId, bool $simplePaths): void {
160156

161157
if (count($result) > 0) {
162158
foreach ($previewFiles as $previewFile) {
159+
/** @var Preview $preview */
163160
$preview = $previewFile['preview'];
164161
/** @var SimpleFile $file */
165162
$file = $previewFile['file'];
166163
$preview->setStorageId($result[0]['storage']);
167164
$preview->setEtag($result[0]['etag']);
168-
$preview->setSourceMimetype($result[0]['mimetype']);
165+
$preview->setSourceMimeType($this->mimeTypeLoader->getMimetypeById($result[0]['mimetype']));
169166
try {
170167
$preview = $this->previewMapper->insert($preview);
171168
} catch (Exception $e) {
@@ -197,11 +194,15 @@ public static function getInternalFolder(string $name, bool $simplePaths): strin
197194
}
198195

199196
private function deleteFolder(string $path, ISimpleFolder $folder): void {
200-
$folder->delete();
201-
202197
$current = $path;
203198

204199
while (true) {
200+
$appDataPath = 'appdata_' . $this->config->getSystemValueString('instanceid') . '/preview/' . $current;
201+
$qb = $this->connection->getQueryBuilder();
202+
$qb->delete('filecache')
203+
->where($qb->expr()->eq('path_hash', $qb->createNamedParameter(md5($appDataPath))))
204+
->executeStatement();
205+
205206
$current = dirname($current);
206207
if ($current === '/' || $current === '.' || $current === '') {
207208
break;
@@ -212,7 +213,6 @@ private function deleteFolder(string $path, ISimpleFolder $folder): void {
212213
if (count($folder->getDirectoryListing()) !== 0) {
213214
break;
214215
}
215-
$folder->delete();
216216
}
217217
}
218218
}

‎core/Command/Preview/ResetRenderedTexts.php‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
use OC\Preview\Db\Preview;
1212
use OC\Preview\PreviewService;
13-
use OCP\Files\IMimeTypeLoader;
1413
use OCP\Files\NotFoundException;
1514
use OCP\Files\NotPermittedException;
1615
use OCP\IAvatarManager;
@@ -28,7 +27,6 @@ public function __construct(
2827
protected readonly IUserManager $userManager,
2928
protected readonly IAvatarManager $avatarManager,
3029
private readonly PreviewService $previewService,
31-
private readonly IMimeTypeLoader $mimeTypeLoader,
3230
) {
3331
parent::__construct();
3432
}
@@ -93,7 +91,7 @@ private function deletePreviews(OutputInterface $output, bool $dryMode): void {
9391
$previewsToDeleteCount = 0;
9492

9593
foreach ($this->getPreviewsToDelete() as $preview) {
96-
$output->writeln('Deleting preview ' . $preview->getName($this->mimeTypeLoader) . ' for fileId ' . $preview->getFileId(), OutputInterface::VERBOSITY_VERBOSE);
94+
$output->writeln('Deleting preview ' . $preview->getName() . ' for fileId ' . $preview->getFileId(), OutputInterface::VERBOSITY_VERBOSE);
9795

9896
$previewsToDeleteCount++;
9997

@@ -112,9 +110,9 @@ private function deletePreviews(OutputInterface $output, bool $dryMode): void {
112110
*/
113111
private function getPreviewsToDelete(): \Generator {
114112
return $this->previewService->getPreviewsForMimeTypes([
115-
$this->mimeTypeLoader->getId('text/plain'),
116-
$this->mimeTypeLoader->getId('text/markdown'),
117-
$this->mimeTypeLoader->getId('text/x-markdown'),
113+
'text/plain',
114+
'text/markdown',
115+
'text/x-markdown'
118116
]);
119117
}
120118
}

‎core/Migrations/Version33000Date20250819110529.php‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,14 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
3737
$table->setPrimaryKey(['id']);
3838
}
3939

40+
if (!$schema->hasTable('preview_versions')) {
41+
$table = $schema->createTable('preview_versions');
42+
$table->addColumn('id', Types::BIGINT, ['autoincrement' => true, 'notnull' => true, 'length' => 20, 'unsigned' => true]);
43+
$table->addColumn('file_id', Types::BIGINT, ['notnull' => true, 'length' => 20, 'unsigned' => true]);
44+
$table->addColumn('version', Types::STRING, ['notnull' => true, 'default' => '', 'length' => 1024]);
45+
$table->setPrimaryKey(['id']);
46+
}
47+
4048
if (!$schema->hasTable('previews')) {
4149
$table = $schema->createTable('previews');
4250
$table->addColumn('id', Types::BIGINT, ['autoincrement' => true, 'notnull' => true, 'length' => 20, 'unsigned' => true]);
@@ -46,18 +54,18 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
4654
$table->addColumn('location_id', Types::BIGINT, ['notnull' => false, 'length' => 20, 'unsigned' => true]);
4755
$table->addColumn('width', Types::INTEGER, ['notnull' => true, 'unsigned' => true]);
4856
$table->addColumn('height', Types::INTEGER, ['notnull' => true, 'unsigned' => true]);
49-
$table->addColumn('mimetype', Types::INTEGER, ['notnull' => true]);
50-
$table->addColumn('source_mimetype', Types::INTEGER, ['notnull' => true]);
57+
$table->addColumn('mimetype_id', Types::INTEGER, ['notnull' => true]);
58+
$table->addColumn('source_mimetype_id', Types::INTEGER, ['notnull' => true]);
5159
$table->addColumn('max', Types::BOOLEAN, ['notnull' => true, 'default' => false]);
5260
$table->addColumn('cropped', Types::BOOLEAN, ['notnull' => true, 'default' => false]);
5361
$table->addColumn('encrypted', Types::BOOLEAN, ['notnull' => true, 'default' => false]);
5462
$table->addColumn('etag', Types::STRING, ['notnull' => true, 'length' => 40, 'fixed' => true]);
5563
$table->addColumn('mtime', Types::INTEGER, ['notnull' => true, 'unsigned' => true]);
5664
$table->addColumn('size', Types::INTEGER, ['notnull' => true, 'unsigned' => true]);
57-
$table->addColumn('version', Types::BIGINT, ['notnull' => true, 'default' => -1]); // can not be null otherwise unique index doesn't work
65+
$table->addColumn('version_id', Types::BIGINT, ['notnull' => true, 'default' => -1]);
5866
$table->setPrimaryKey(['id']);
5967
$table->addIndex(['file_id']);
60-
$table->addUniqueIndex(['file_id', 'width', 'height', 'mimetype', 'cropped', 'version'], 'previews_file_uniq_idx');
68+
$table->addUniqueIndex(['file_id', 'width', 'height', 'mimetype_id', 'cropped', 'version_id'], 'previews_file_uniq_idx');
6169
}
6270

6371
return $schema;

‎lib/private/Files/Cache/LocalRootScanner.php‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
use OCP\IConfig;
1212
use OCP\Server;
13+
use Override;
1314

1415
class LocalRootScanner extends Scanner {
1516
private string $previewFolder;
@@ -20,6 +21,7 @@ public function __construct(\OC\Files\Storage\Storage $storage) {
2021
$this->previewFolder = 'appdata_' . $config->getSystemValueString('instanceid', '') . '/preview';
2122
}
2223

24+
#[Override]
2325
public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true, $data = null) {
2426
if ($this->shouldScanPath($file)) {
2527
return parent::scanFile($file, $reuseExisting, $parentId, $cacheData, $lock, $data);
@@ -28,6 +30,7 @@ public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData =
2830
}
2931
}
3032

33+
#[Override]
3134
public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $lock = true) {
3235
if ($this->shouldScanPath($path)) {
3336
return parent::scan($path, $recursive, $reuse, $lock);
@@ -36,11 +39,16 @@ public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $loc
3639
}
3740
}
3841

39-
private function shouldScanPath(string $path): bool {
40-
$path = trim($path, '/');
42+
#[Override]
43+
protected function scanChildren(string $path, $recursive, int $reuse, int $folderId, bool $lock, int|float $oldSize, &$etagChanged = false) {
4144
if (str_starts_with($path, $this->previewFolder)) {
42-
return false;
45+
return 0;
4346
}
47+
return parent::scanChildren($path, $recursive, $reuse, $folderId, $lock, $oldSize, $etagChanged);
48+
}
49+
50+
private function shouldScanPath(string $path): bool {
51+
$path = trim($path, '/');
4452
return $path === '' || str_starts_with($path, 'appdata_') || str_starts_with($path, '__groupfolders');
4553
}
4654
}

‎lib/private/Preview/BackgroundCleanupJob.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public function __construct(
3535
public function run($argument): void {
3636
foreach ($this->getDeletedFiles() as $fileId) {
3737
$previewIds = [];
38-
foreach ($this->previewService->getAvailablePreviewForFile($fileId) as $preview) {
38+
foreach ($this->previewService->getAvailablePreviewsForFile($fileId) as $preview) {
3939
$this->previewService->deletePreview($preview);
4040
}
4141
}

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

Lines changed: 45 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313
use OCP\AppFramework\Db\Entity;
1414
use OCP\DB\Types;
1515
use OCP\Files\IMimeTypeDetector;
16-
use OCP\Files\IMimeTypeLoader;
17-
use OCP\Server;
1816

1917
/**
2018
* Preview entity mapped to the oc_previews and oc_preview_locations table.
@@ -35,10 +33,10 @@
3533
* @method void setHeight(int $height)
3634
* @method bool isCropped() Get whether the preview is cropped or not.
3735
* @method void setCropped(bool $cropped)
38-
* @method void setMimetype(int $mimetype) Set the mimetype of the preview.
39-
* @method int getMimetype() Get the mimetype of the preview.
40-
* @method void setSourceMimetype(int $sourceMimetype) Set the mimetype of the source file.
41-
* @method int getSourceMimetype() Get the mimetype of the source file.
36+
* @method void setMimetypeId(int $mimetype) Set the mimetype of the preview.
37+
* @method int getMimetypeId() Get the mimetype of the preview.
38+
* @method void setSourceMimetypeId(int $sourceMimetype) Set the mimetype of the source file.
39+
* @method int getSourceMimetypeId() Get the mimetype of the source file.
4240
* @method int getMtime() Get the modification time of the preview.
4341
* @method void setMtime(int $mtime)
4442
* @method int getSize() Get the size of the preview.
@@ -47,8 +45,8 @@
4745
* @method void setMax(bool $max)
4846
* @method string getEtag() Get the etag of the preview.
4947
* @method void setEtag(string $etag)
50-
* @method int|null getVersion() Get the version for files_versions_s3
51-
* @method void setVersion(?int $version)
48+
* @method string|null getVersion() Get the version for files_versions_s3
49+
* @method void setVersionId(int $versionId)
5250
* @method bool|null getIs() Get the version for files_versions_s3
5351
* @method bool isEncrypted() Get whether the preview is encrypted. At the moment every preview is unencrypted.
5452
* @method void setEncrypted(bool $encrypted)
@@ -64,15 +62,17 @@ class Preview extends Entity {
6462
protected ?string $objectStoreName = null;
6563
protected ?int $width = null;
6664
protected ?int $height = null;
67-
protected ?int $mimetype = null;
68-
69-
protected ?int $sourceMimetype = null;
65+
protected ?int $mimetypeId = null;
66+
protected ?int $sourceMimetypeId = null;
67+
protected string $mimetype = 'application/octet-stream';
68+
protected string $sourceMimetype = 'application/octet-stream';
7069
protected ?int $mtime = null;
7170
protected ?int $size = null;
7271
protected ?bool $max = null;
7372
protected ?bool $cropped = null;
7473
protected ?string $etag = null;
75-
protected ?int $version = null;
74+
protected ?string $version = null;
75+
protected ?int $versionId = null;
7676
protected ?bool $encrypted = null;
7777

7878
public function __construct() {
@@ -82,23 +82,23 @@ public function __construct() {
8282
$this->addType('locationId', Types::BIGINT);
8383
$this->addType('width', Types::INTEGER);
8484
$this->addType('height', Types::INTEGER);
85-
$this->addType('mimetype', Types::INTEGER);
86-
$this->addType('sourceMimetype', Types::INTEGER);
85+
$this->addType('mimetypeId', Types::INTEGER);
86+
$this->addType('sourceMimetypeId', Types::INTEGER);
8787
$this->addType('mtime', Types::INTEGER);
8888
$this->addType('size', Types::INTEGER);
8989
$this->addType('max', Types::BOOLEAN);
9090
$this->addType('cropped', Types::BOOLEAN);
9191
$this->addType('encrypted', Types::BOOLEAN);
9292
$this->addType('etag', Types::STRING);
93-
$this->addType('version', Types::BIGINT);
93+
$this->addType('versionId', Types::STRING);
9494
}
9595

96-
public static function fromPath(string $path, IMimeTypeDetector $mimeTypeDetector, IMimeTypeLoader $mimeTypeLoader): Preview|false {
96+
public static function fromPath(string $path, IMimeTypeDetector $mimeTypeDetector): Preview|false {
9797
$preview = new self();
9898
$preview->setFileId((int)basename(dirname($path)));
9999

100100
$fileName = pathinfo($path, PATHINFO_FILENAME) . '.' . pathinfo($path, PATHINFO_EXTENSION);
101-
$ok = preg_match('/(([0-9]+)-)?([0-9]+)-([0-9]+)(-(max))?(-(crop))?\.([a-z]{3,4})/', $fileName, $matches);
101+
$ok = preg_match('/(([A-Za-z0-9\+\/]+)-)?([0-9]+)-([0-9]+)(-(max))?(-(crop))?\.([a-z]{3,4})/', $fileName, $matches);
102102

103103
if ($ok !== 1) {
104104
return false;
@@ -108,24 +108,24 @@ public static function fromPath(string $path, IMimeTypeDetector $mimeTypeDetecto
108108
2 => $version,
109109
3 => $width,
110110
4 => $height,
111-
6 => $crop,
112-
8 => $max,
111+
6 => $max,
112+
8 => $crop,
113113
] = $matches;
114114

115-
$preview->setMimetype($mimeTypeLoader->getId($mimeTypeDetector->detectPath($fileName)));
115+
$preview->setMimeType($mimeTypeDetector->detectPath($fileName));
116116

117117
$preview->setWidth((int)$width);
118118
$preview->setHeight((int)$height);
119119
$preview->setCropped($crop === 'crop');
120120
$preview->setMax($max === 'max');
121121

122122
if (!empty($version)) {
123-
$preview->setVersion((int)$version);
123+
$preview->setVersion($version);
124124
}
125125
return $preview;
126126
}
127127

128-
public function getName(IMimeTypeLoader $mimeTypeLoader): string {
128+
public function getName(): string {
129129
$path = ($this->getVersion() > -1 ? $this->getVersion() . '-' : '') . $this->getWidth() . '-' . $this->getHeight();
130130
if ($this->isCropped()) {
131131
$path .= '-crop';
@@ -134,13 +134,13 @@ public function getName(IMimeTypeLoader $mimeTypeLoader): string {
134134
$path .= '-max';
135135
}
136136

137-
$ext = $this->getExtension($mimeTypeLoader);
137+
$ext = $this->getExtension();
138138
$path .= '.' . $ext;
139139
return $path;
140140
}
141141

142-
public function getExtension(IMimeTypeLoader $mimeTypeLoader): string {
143-
return match ($this->getMimetypeValue($mimeTypeLoader)) {
142+
public function getExtension(): string {
143+
return match ($this->getMimeType()) {
144144
'image/png' => 'png',
145145
'image/gif' => 'gif',
146146
'image/jpeg' => 'jpg',
@@ -149,15 +149,31 @@ public function getExtension(IMimeTypeLoader $mimeTypeLoader): string {
149149
};
150150
}
151151

152-
public function getMimetypeValue(IMimeTypeLoader $mimeTypeLoader): string {
153-
return $mimeTypeLoader->getMimetypeById($this->mimetype) ?? 'image/jpeg';
154-
}
155-
156152
public function setBucketName(string $bucketName): void {
157153
$this->bucketName = $bucketName;
158154
}
159155

160156
public function setObjectStoreName(string $objectStoreName): void {
161157
$this->objectStoreName = $objectStoreName;
162158
}
159+
160+
public function setVersion(?string $version): void {
161+
$this->version = $version;
162+
}
163+
164+
public function getMimeType(): string {
165+
return $this->mimetype;
166+
}
167+
168+
public function setMimeType(string $mimeType): void {
169+
$this->mimetype = $mimeType;
170+
}
171+
172+
public function getSourceMimeType(): string {
173+
return $this->sourceMimetype;
174+
}
175+
176+
public function setSourceMimeType(string $mimeType): void {
177+
$this->sourceMimetype = $mimeType;
178+
}
163179
}

0 commit comments

Comments
 (0)