Skip to content

Commit 9ec92a8

Browse files
Antreesybackportbot[bot]
authored andcommitted
fix(AmazonS3#getMetaData): preserve storage_mtime after parent clobbers it
Common::getMetaData (Common.php:667) always sets storage_mtime = mtime before returning. For S3 virtual directories this is wrong: mtime can be bumped by child propagation while storage_mtime should remain the actual last S3 change. When the scanner later calls getMetaData() it compares data['storage_mtime'] against cacheData['storage_mtime']. If they differ it writes the value back, triggering View::getCacheEntry to fire propagateChange on every read even when nothing on S3 changed. Override getMetaData() to restore storage_mtime from the live cache entry (or, for non-root directories not yet in the cache, from the S3 directory marker LastModified header) after the parent call. Assisted-by: ClaudeCode:claude-sonnet-4-6 Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
1 parent 7a81700 commit 9ec92a8

2 files changed

Lines changed: 46 additions & 0 deletions

File tree

apps/files_external/lib/Lib/Storage/AmazonS3.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,26 @@ public function stat(string $path): array|false {
331331
return $stat;
332332
}
333333

334+
#[\Override]
335+
public function getMetaData(string $path): ?array {
336+
$data = parent::getMetaData($path);
337+
if ($data !== null && $data['mimetype'] === FileInfo::MIMETYPE_FOLDER) {
338+
// Common::getMetaData sets storage_mtime = mtime, but for S3 virtual directories
339+
// mtime may have been updated by mtime propagation while storage_mtime should
340+
// reflect the actual last storage change. Without this override the scanner sees
341+
// data['storage_mtime'] != cacheData['storage_mtime'] and re-writes the cache,
342+
// causing View::getCacheEntry to trigger propagateChange on every read.
343+
$path = $this->normalizePath($path);
344+
$cacheEntry = $this->getCache()->get($this->getCachePath($path));
345+
if ($cacheEntry instanceof CacheEntry) {
346+
$data['storage_mtime'] = $cacheEntry->getStorageMTime();
347+
} elseif (!$this->isRoot($path) && $directoryMarker = $this->headObject($path . '/')) {
348+
$data['storage_mtime'] = strtotime($directoryMarker['LastModified']);
349+
}
350+
}
351+
return $data;
352+
}
353+
334354
#[\Override]
335355
public function is_dir(string $path): bool {
336356
$path = $this->normalizePath($path);

apps/files_external/tests/Storage/Amazons3Test.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,30 @@ public function testStatRootPreservesStorageMtimeFromCache(): void {
6464
);
6565
}
6666

67+
/**
68+
* Regression test: Common::getMetaData sets storage_mtime = mtime, but for S3 virtual
69+
* directories mtime may have been bumped by propagation while storage_mtime should stay
70+
* stable. The override restores storage_mtime from the cache entry so the scanner does
71+
* not see a spurious mismatch and re-write the cache on every scan.
72+
*/
73+
public function testGetMetaDataDirectoryPreservesStorageMtimeSeparateFromMtime(): void {
74+
$this->instance->getScanner()->scan('', Scanner::SCAN_SHALLOW);
75+
76+
$cachedRoot = $this->instance->getCache()->get('');
77+
$this->assertNotFalse($cachedRoot, 'Root entry must exist in cache after scan');
78+
79+
// Simulate propagation bumping mtime without touching storage_mtime
80+
$originalStorageMtime = $cachedRoot['storage_mtime'];
81+
$this->instance->getCache()->update($cachedRoot->getId(), [
82+
'mtime' => $originalStorageMtime + 9999,
83+
]);
84+
85+
$meta = $this->instance->getMetaData('');
86+
$this->assertNotNull($meta, 'getMetaData(\'\') must return data');
87+
$this->assertEquals(
88+
$originalStorageMtime,
89+
$meta['storage_mtime'],
90+
'getMetaData must return storage_mtime from cache, not the propagated mtime'
91+
);
92+
}
6793
}

0 commit comments

Comments
 (0)