Skip to content

Commit fc37feb

Browse files
committed
fix(preview): skip object-store existence checks
ObjectExists() is a network round-trip per preview. Treat the DB row as authoritative for now so bulk listing stays cheap. Missing objects still fail later on read. Local storage keeps a real filesystem check. Assisted-by: Grok:grok-4.6 Signed-off-by: Ray Vincent <rayhvincent@gmail.com>
1 parent 6e0eac7 commit fc37feb

2 files changed

Lines changed: 13 additions & 55 deletions

File tree

lib/private/Preview/Storage/ObjectStorePreviewStorage.php

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -176,20 +176,11 @@ public function deleteUnreferencedPreview(Preview $preview): void {
176176

177177
#[Override]
178178
public function previewExists(Preview $preview): bool {
179-
if ($preview->getLocationId() === null) {
180-
// Without a location the bucket cannot be resolved, which is the case
181-
// for rows written before a move to an object store and for the dummy
182-
// previews of the unit tests. Never report those as missing, or they
183-
// would be dropped on every request.
184-
return true;
185-
}
186-
187-
[
188-
'urn' => $urn,
189-
'store' => $store,
190-
] = $this->getObjectStoreInfoForExistingPreview($preview);
191-
192-
return $store->objectExists($urn);
179+
// Avoid a network round-trip per preview. Object-store
180+
// existence checks are expensive; treat the DB row as
181+
// authoritative for now. Missing objects will still fail
182+
// later on read and can be cleaned up by other paths.
183+
return true;
193184
}
194185

195186
public function getUrn(Preview $preview, array $config): string {

tests/lib/Preview/Storage/ObjectStorePreviewStorageTest.php

Lines changed: 8 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
use OC\Preview\Db\Preview;
1414
use OC\Preview\Db\PreviewMapper;
1515
use OC\Preview\Storage\ObjectStorePreviewStorage;
16-
use OCP\Files\ObjectStore\IObjectStore;
1716
use OCP\IConfig;
1817
use PHPUnit\Framework\MockObject\MockObject;
1918
use PHPUnit\Framework\TestCase;
@@ -44,42 +43,27 @@ protected function setUp(): void {
4443
}
4544

4645
/**
47-
* Rows without a location cannot resolve a bucket. Treating them as missing
48-
* would drop dummy and pre-migration previews on every request.
46+
* Object-store existence checks are a network round-trip per preview, so
47+
* the DB row is treated as authoritative until a cheaper bulk check exists.
4948
*/
50-
public function testPreviewExistsWhenLocationIdIsNull(): void {
51-
$preview = $this->makePreview(locationId: null);
49+
public function testPreviewExistsAlwaysReturnsTrueWithoutQueryingObjectStore(): void {
50+
$preview = $this->makePreview(locationId: 'loc-1');
5251

5352
$this->objectStoreConfig->expects($this->never())->method('getObjectStoreConfiguration');
5453
$this->objectStoreConfig->expects($this->never())->method('buildObjectStore');
5554

5655
$this->assertTrue($this->storage->previewExists($preview));
5756
}
5857

59-
public function testPreviewExistsWhenObjectIsPresent(): void {
60-
$preview = $this->makePreview(locationId: 'loc-1');
61-
$store = $this->mockObjectStoreForPreview($preview);
58+
public function testPreviewExistsWhenLocationIdIsNull(): void {
59+
$preview = $this->makePreview(locationId: null);
6260

63-
$store->expects($this->once())
64-
->method('objectExists')
65-
->with('urn:oid:preview:' . $preview->getId())
66-
->willReturn(true);
61+
$this->objectStoreConfig->expects($this->never())->method('getObjectStoreConfiguration');
62+
$this->objectStoreConfig->expects($this->never())->method('buildObjectStore');
6763

6864
$this->assertTrue($this->storage->previewExists($preview));
6965
}
7066

71-
public function testPreviewExistsWhenObjectIsMissing(): void {
72-
$preview = $this->makePreview(locationId: 'loc-1');
73-
$store = $this->mockObjectStoreForPreview($preview);
74-
75-
$store->expects($this->once())
76-
->method('objectExists')
77-
->with('urn:oid:preview:' . $preview->getId())
78-
->willReturn(false);
79-
80-
$this->assertFalse($this->storage->previewExists($preview));
81-
}
82-
8367
private function makePreview(?string $locationId): Preview {
8468
$preview = new Preview();
8569
$preview->id = 'preview-id-1';
@@ -96,21 +80,4 @@ private function makePreview(?string $locationId): Preview {
9680
}
9781
return $preview;
9882
}
99-
100-
private function mockObjectStoreForPreview(Preview $preview): IObjectStore&MockObject {
101-
$store = $this->createMock(IObjectStore::class);
102-
$this->objectStoreConfig->method('getObjectStoreConfiguration')
103-
->with($preview->getObjectStoreName())
104-
->willReturn([
105-
'class' => IObjectStore::class,
106-
'arguments' => [
107-
'multibucket' => false,
108-
'bucket' => 'preview-bucket',
109-
'objectPrefix' => 'urn:oid:',
110-
],
111-
]);
112-
$this->objectStoreConfig->method('buildObjectStore')
113-
->willReturn($store);
114-
return $store;
115-
}
11683
}

0 commit comments

Comments
 (0)