Skip to content

Commit b069f66

Browse files
committed
feat(Db): Use SnowflakeId for previews
Allow to get an id for the storing the preview on disk before inserting the preview on the DB. Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>
1 parent 6766d0a commit b069f66

11 files changed

Lines changed: 169 additions & 44 deletions

File tree

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace OC\Core\Migrations;
10+
11+
use Closure;
12+
use OCP\DB\ISchemaWrapper;
13+
use OCP\DB\Types;
14+
use OCP\IDBConnection;
15+
use OCP\Migration\Attributes\AddIndex;
16+
use OCP\Migration\Attributes\CreateTable;
17+
use OCP\Migration\Attributes\IndexType;
18+
use OCP\Migration\Attributes\ModifyColumn;
19+
use OCP\Migration\IOutput;
20+
use OCP\Migration\SimpleMigrationStep;
21+
22+
/**
23+
* Migrate away from auto-increment
24+
*/
25+
#[AddIndex(table: 'preview_locations', type: IndexType::UNIQUE)]
26+
#[ModifyColumn(table: 'preview_locations', name: 'id', description: 'Remove auto-increment')]
27+
#[ModifyColumn(table: 'previews', name: 'id', description: 'Remove auto-increment')]
28+
#[ModifyColumn(table: 'preview_versions', name: 'id', description: 'Remove auto-increment')]
29+
class Version33000Date20251023110529 extends SimpleMigrationStep {
30+
public function __construct(
31+
private readonly IDBConnection $connection,
32+
) {}
33+
34+
/**
35+
* @param Closure(): ISchemaWrapper $schemaClosure The `\Closure` returns a `ISchemaWrapper`
36+
*/
37+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
38+
/** @var ISchemaWrapper $schema */
39+
$schema = $schemaClosure();
40+
41+
if ($schema->hasTable('preview_locations')) {
42+
$table = $schema->getTable('preview_locations');
43+
$table->modifyColumn('id', ['autoincrement' => false]);
44+
$table->addUniqueIndex(['bucket_name', 'object_store_name'], 'unique_bucket_object_store_name');
45+
}
46+
47+
if ($schema->hasTable('preview_versions')) {
48+
$table = $schema->getTable('preview_versions');
49+
$table->modifyColumn('id', ['autoincrement' => false]);
50+
}
51+
52+
if ($schema->hasTable('previews')) {
53+
$table = $schema->getTable('previews');
54+
$table->modifyColumn('id', ['autoincrement' => false]);
55+
}
56+
57+
return $schema;
58+
}
59+
60+
public function preSchemaChange(IOutput $output, \Closure $schemaClosure, array $options) {
61+
// This code should never be run on a production instance but this might be needed on daily/git version.
62+
$qb = $this->connection->getQueryBuilder();
63+
$qb->select('*')
64+
->from('preview_locations')
65+
->groupBy('bucket_name', 'object_store_name')
66+
->having('COUNT(*) > 1');
67+
68+
$result = $qb->executeQuery();
69+
while ($row = $result->fetch()) {
70+
// Iterate over all the rows with duplicated rows
71+
$id = $row['id'];
72+
73+
$qb = $this->connection->getQueryBuilder();
74+
$qb->select('id')
75+
->from('preview_locations')
76+
->where($qb->expr()->eq('bucket_name', $qb->createNamedParameter($row['bucket_name'])))
77+
->andWhere($qb->expr()->eq('object_store_name', $qb->createNamedParameter($row['object_store_name'])))
78+
->andWhere($qb->expr()->neq('id', $qb->createNamedParameter($row['id'])));
79+
80+
$result = $qb->executeQuery();
81+
while ($row = $result->fetch()) {
82+
// Update previews entries to the now de-duplicated id
83+
$qb = $this->connection->getQueryBuilder();
84+
$qb->update('previews')
85+
->set('location_id', $qb->createNamedParameter($id))
86+
->where($qb->expr()->eq('id', $qb->createNamedParameter($row['id'])));
87+
$qb->executeStatement();
88+
89+
$qb = $this->connection->getQueryBuilder();
90+
$qb->delete('preview_locations')
91+
->where($qb->expr()->eq('id', $qb->createNamedParameter($row['id'])));
92+
$qb->executeStatement();
93+
}
94+
}
95+
}
96+
}

lib/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1530,6 +1530,7 @@
15301530
'OC\\Core\\Migrations\\Version32000Date20250731062008' => $baseDir . '/core/Migrations/Version32000Date20250731062008.php',
15311531
'OC\\Core\\Migrations\\Version32000Date20250806110519' => $baseDir . '/core/Migrations/Version32000Date20250806110519.php',
15321532
'OC\\Core\\Migrations\\Version33000Date20250819110529' => $baseDir . '/core/Migrations/Version33000Date20250819110529.php',
1533+
'OC\\Core\\Migrations\\Version33000Date20251023110529' => $baseDir . '/core/Migrations/Version33000Date20251023110529.php',
15331534
'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php',
15341535
'OC\\Core\\ResponseDefinitions' => $baseDir . '/core/ResponseDefinitions.php',
15351536
'OC\\Core\\Service\\CronService' => $baseDir . '/core/Service/CronService.php',

lib/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1571,6 +1571,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
15711571
'OC\\Core\\Migrations\\Version32000Date20250731062008' => __DIR__ . '/../../..' . '/core/Migrations/Version32000Date20250731062008.php',
15721572
'OC\\Core\\Migrations\\Version32000Date20250806110519' => __DIR__ . '/../../..' . '/core/Migrations/Version32000Date20250806110519.php',
15731573
'OC\\Core\\Migrations\\Version33000Date20250819110529' => __DIR__ . '/../../..' . '/core/Migrations/Version33000Date20250819110529.php',
1574+
'OC\\Core\\Migrations\\Version33000Date20251023110529' => __DIR__ . '/../../..' . '/core/Migrations/Version33000Date20251023110529.php',
15741575
'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php',
15751576
'OC\\Core\\ResponseDefinitions' => __DIR__ . '/../../..' . '/core/ResponseDefinitions.php',
15761577
'OC\\Core\\Service\\CronService' => __DIR__ . '/../../..' . '/core/Service/CronService.php',

lib/private/AppFramework/Http/Dispatcher.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ private function executeController(Controller $controller, string $methodName):
204204
try {
205205
$response = \call_user_func_array([$controller, $methodName], $arguments);
206206
} catch (\TypeError $e) {
207-
// Only intercept TypeErrors occuring on the first line, meaning that the invocation of the controller method failed.
207+
// Only intercept TypeErrors occurring on the first line, meaning that the invocation of the controller method failed.
208208
// Any other TypeError happens inside the controller method logic and should be logged as normal.
209209
if ($e->getFile() === $this->reflector->getFile() && $e->getLine() === $this->reflector->getStartLine()) {
210210
$this->logger->debug('Failed to call controller method: ' . $e->getMessage(), ['exception' => $e]);

lib/private/Preview/Db/Preview.php

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,22 @@
1313
use OCP\AppFramework\Db\Entity;
1414
use OCP\DB\Types;
1515
use OCP\Files\IMimeTypeDetector;
16+
use OCP\Server;
17+
use OCP\Snowflake\IGenerator;
1618

1719
/**
1820
* Preview entity mapped to the oc_previews and oc_preview_locations table.
1921
*
22+
* @method string getId()
23+
* @method void setId(string $id)
2024
* @method int getFileId() Get the file id of the original file.
2125
* @method void setFileId(int $fileId)
2226
* @method int getStorageId() Get the storage id of the original file.
2327
* @method void setStorageId(int $fileId)
2428
* @method int getOldFileId() Get the old location in the file-cache table, for legacy compatibility.
2529
* @method void setOldFileId(int $oldFileId)
26-
* @method int getLocationId() Get the location id in the preview_locations table. Only set when using an object store as primary storage.
27-
* @method void setLocationId(int $locationId)
30+
* @method string getLocationId() Get the location id in the preview_locations table. Only set when using an object store as primary storage.
31+
* @method void setLocationId(string $locationId)
2832
* @method string|null getBucketName() Get the bucket name where the preview is stored. This is stored in the preview_locations table.
2933
* @method string|null getObjectStoreName() Get the object store name where the preview is stored. This is stored in the preview_locations table.
3034
* @method int getWidth() Get the width of the preview.
@@ -46,7 +50,7 @@
4650
* @method string getEtag() Get the etag of the preview.
4751
* @method void setEtag(string $etag)
4852
* @method string|null getVersion() Get the version for files_versions_s3
49-
* @method void setVersionId(int $versionId)
53+
* @method void setVersionId(string $versionId)
5054
* @method bool|null getIs() Get the version for files_versions_s3
5155
* @method bool isEncrypted() Get whether the preview is encrypted. At the moment every preview is unencrypted.
5256
* @method void setEncrypted(bool $encrypted)
@@ -57,7 +61,7 @@ class Preview extends Entity {
5761
protected ?int $fileId = null;
5862
protected ?int $oldFileId = null;
5963
protected ?int $storageId = null;
60-
protected ?int $locationId = null;
64+
protected ?string $locationId = null;
6165
protected ?string $bucketName = null;
6266
protected ?string $objectStoreName = null;
6367
protected ?int $width = null;
@@ -72,14 +76,15 @@ class Preview extends Entity {
7276
protected ?bool $cropped = null;
7377
protected ?string $etag = null;
7478
protected ?string $version = null;
75-
protected ?int $versionId = null;
79+
protected ?string $versionId = null;
7680
protected ?bool $encrypted = null;
7781

7882
public function __construct() {
83+
$this->addType('id', Types::STRING);
7984
$this->addType('fileId', Types::BIGINT);
8085
$this->addType('storageId', Types::BIGINT);
8186
$this->addType('oldFileId', Types::BIGINT);
82-
$this->addType('locationId', Types::BIGINT);
87+
$this->addType('locationId', Types::STRING);
8388
$this->addType('width', Types::INTEGER);
8489
$this->addType('height', Types::INTEGER);
8590
$this->addType('mimetypeId', Types::INTEGER);
@@ -91,6 +96,7 @@ public function __construct() {
9196
$this->addType('encrypted', Types::BOOLEAN);
9297
$this->addType('etag', Types::STRING);
9398
$this->addType('versionId', Types::STRING);
99+
$this->setId(Server::get(IGenerator::class)->nextId());
94100
}
95101

96102
public static function fromPath(string $path, IMimeTypeDetector $mimeTypeDetector): Preview|false {

lib/private/Preview/Db/PreviewMapper.php

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use OCP\DB\QueryBuilder\IQueryBuilder;
1616
use OCP\Files\IMimeTypeLoader;
1717
use OCP\IDBConnection;
18+
use OCP\Snowflake\IGenerator;
1819
use Override;
1920

2021
/**
@@ -29,6 +30,7 @@ class PreviewMapper extends QBMapper {
2930
public function __construct(
3031
IDBConnection $db,
3132
private readonly IMimeTypeLoader $mimeTypeLoader,
33+
private readonly IGenerator $snowflake,
3234
) {
3335
parent::__construct($db, self::TABLE_NAME, Preview::class);
3436
}
@@ -50,13 +52,15 @@ public function insert(Entity $entity): Entity {
5052

5153
if ($preview->getVersion() !== null && $preview->getVersion() !== '') {
5254
$qb = $this->db->getQueryBuilder();
55+
$id = $this->snowflake->nextId();
5356
$qb->insert(self::VERSION_TABLE_NAME)
5457
->values([
58+
'id' => $id,
5559
'version' => $preview->getVersion(),
5660
'file_id' => $preview->getFileId(),
5761
])
5862
->executeStatement();
59-
$entity->setVersionId($qb->getLastInsertId());
63+
$entity->setVersionId($id);
6064
}
6165
return parent::insert($preview);
6266
}
@@ -148,7 +152,13 @@ protected function joinLocation(IQueryBuilder $qb): IQueryBuilder {
148152
));
149153
}
150154

151-
public function getLocationId(string $bucket, string $objectStore): int {
155+
/**
156+
* Get the location id corresponding to the $bucket and $objectStore. Create one
157+
* if not existing yet.
158+
*
159+
* @throws Exception
160+
*/
161+
public function getLocationId(string $bucket, string $objectStore): string {
152162
$qb = $this->db->getQueryBuilder();
153163
$result = $qb->select('id')
154164
->from(self::LOCATION_TABLE_NAME)
@@ -157,14 +167,33 @@ public function getLocationId(string $bucket, string $objectStore): int {
157167
->executeQuery();
158168
$data = $result->fetchOne();
159169
if ($data) {
160-
return $data;
170+
return (string)$data;
161171
} else {
162-
$qb->insert(self::LOCATION_TABLE_NAME)
163-
->values([
164-
'bucket_name' => $qb->createNamedParameter($bucket),
165-
'object_store_name' => $qb->createNamedParameter($objectStore),
166-
])->executeStatement();
167-
return $qb->getLastInsertId();
172+
try {
173+
$id = $this->snowflake->nextId();
174+
$qb->insert(self::LOCATION_TABLE_NAME)
175+
->values([
176+
'id' => $qb->createNamedParameter($id),
177+
'bucket_name' => $qb->createNamedParameter($bucket),
178+
'object_store_name' => $qb->createNamedParameter($objectStore),
179+
])->executeStatement();
180+
return $id;
181+
} catch (Exception $e) {
182+
if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
183+
// Fetch again as there seems to be another entry added meanwhile
184+
$result = $qb->select('id')
185+
->from(self::LOCATION_TABLE_NAME)
186+
->where($qb->expr()->eq('bucket_name', $qb->createNamedParameter($bucket)))
187+
->andWhere($qb->expr()->eq('object_store_name', $qb->createNamedParameter($objectStore)))
188+
->executeQuery();
189+
$data = $result->fetchOne();
190+
if ($data) {
191+
return (string)$data;
192+
}
193+
}
194+
195+
throw $e;
196+
}
168197
}
169198
}
170199

lib/private/Preview/Generator.php

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,6 @@ private function generateProviderPreview(File $file, int $width, int $height, bo
360360
$previewEntry->setMimetype($preview->dataMimeType());
361361
$previewEntry->setEtag($file->getEtag());
362362
$previewEntry->setMtime((new \DateTime())->getTimestamp());
363-
$previewEntry->setSize(0);
364363
return $this->savePreview($previewEntry, $preview);
365364
} catch (NotPermittedException) {
366365
throw new NotFoundException();
@@ -514,7 +513,6 @@ private function generatePreview(
514513
$previewEntry->setMimeType($preview->dataMimeType());
515514
$previewEntry->setEtag($file->getEtag());
516515
$previewEntry->setMtime((new \DateTime())->getTimestamp());
517-
$previewEntry->setSize(0);
518516
if ($cacheResult) {
519517
$previewEntry = $this->savePreview($previewEntry, $preview);
520518
return new PreviewFile($previewEntry, $this->storageFactory, $this->previewMapper);
@@ -530,26 +528,20 @@ private function generatePreview(
530528
* @throws \OCP\DB\Exception
531529
*/
532530
public function savePreview(Preview $previewEntry, IImage $preview): Preview {
533-
$previewEntry = $this->previewMapper->insert($previewEntry);
534-
535531
// we need to save to DB first
536-
try {
537-
if ($preview instanceof IStreamImage) {
538-
$size = $this->storageFactory->writePreview($previewEntry, $preview->resource());
539-
} else {
540-
$stream = fopen('php://temp', 'w+');
541-
fwrite($stream, $preview->data());
542-
rewind($stream);
543-
$size = $this->storageFactory->writePreview($previewEntry, $stream);
544-
}
545-
if (!$size) {
546-
throw new \RuntimeException('Unable to write preview file');
547-
}
548-
} catch (\Exception $e) {
549-
$this->previewMapper->delete($previewEntry);
550-
throw $e;
532+
if ($preview instanceof IStreamImage) {
533+
$size = $this->storageFactory->writePreview($previewEntry, $preview->resource());
534+
} else {
535+
$stream = fopen('php://temp', 'w+');
536+
fwrite($stream, $preview->data());
537+
rewind($stream);
538+
$size = $this->storageFactory->writePreview($previewEntry, $stream);
539+
}
540+
if (!$size) {
541+
throw new \RuntimeException('Unable to write preview file');
551542
}
552543
$previewEntry->setSize($size);
553-
return $this->previewMapper->update($previewEntry);
544+
545+
return $this->previewMapper->insert($previewEntry);
554546
}
555547
}

lib/private/Preview/PreviewService.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public function getAvailableFileIds(): \Generator {
5252

5353
// Previews next to each others in the database are likely in the same storage, so group them
5454
while ($row = $result->fetch()) {
55-
if ($lastStorageId !== $row['storage_id']) {
55+
if ($lastStorageId !== (int)$row['storage_id']) {
5656
if ($lastStorageId !== -1) {
5757
yield ['storageId' => $lastStorageId, 'fileIds' => $fileIds];
5858
$fileIds = [];

lib/public/AppFramework/Db/Entity.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,8 @@
1919
* @psalm-consistent-constructor
2020
*/
2121
abstract class Entity {
22-
/**
23-
* @var int
24-
*/
25-
public $id;
22+
/** @var int $id */
23+
public $id = null;
2624

2725
private array $_updatedFields = [];
2826
/** @var array<string, \OCP\DB\Types::*> */

tests/lib/Preview/PreviewMapperTest.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use OC\Preview\Db\PreviewMapper;
1515
use OCP\IDBConnection;
1616
use OCP\Server;
17+
use OCP\Snowflake\IGenerator;
1718
use Test\TestCase;
1819

1920
/**
@@ -22,10 +23,12 @@
2223
class PreviewMapperTest extends TestCase {
2324
private PreviewMapper $previewMapper;
2425
private IDBConnection $connection;
26+
private IGenerator $snowflake;
2527

2628
public function setUp(): void {
2729
$this->previewMapper = Server::get(PreviewMapper::class);
2830
$this->connection = Server::get(IDBConnection::class);
31+
$this->snowflake = Server::get(IGenerator::class);
2932
}
3033

3134
public function testGetAvailablePreviews(): void {
@@ -53,13 +56,14 @@ private function createPreviewForFileId(int $fileId, ?int $bucket = null): void
5356
$locationId = null;
5457
if ($bucket) {
5558
$qb = $this->connection->getQueryBuilder();
59+
$locationId = $this->snowflake->nextId();
5660
$qb->insert('preview_locations')
5761
->values([
62+
'id' => $locationId,
5863
'bucket_name' => $qb->createNamedParameter('preview-' . $bucket),
5964
'object_store_name' => $qb->createNamedParameter('default'),
6065
]);
6166
$qb->executeStatement();
62-
$locationId = $qb->getLastInsertId();
6367
}
6468
$preview = new Preview();
6569
$preview->setFileId($fileId);

0 commit comments

Comments
 (0)