Skip to content

Commit 478f6cb

Browse files
committed
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 c0bd0ed commit 478f6cb

2 files changed

Lines changed: 45 additions & 0 deletions

File tree

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,25 @@ private function getLastModified(string $path): string {
359359
return 'now';
360360
}
361361

362+
public function getMetaData(string $path): ?array {
363+
$data = parent::getMetaData($path);
364+
if ($data !== null && $data['mimetype'] === FileInfo::MIMETYPE_FOLDER) {
365+
// Common::getMetaData sets storage_mtime = mtime, but for S3 virtual directories
366+
// mtime may have been updated by mtime propagation while storage_mtime should
367+
// reflect the actual last storage change. Without this override the scanner sees
368+
// data['storage_mtime'] != cacheData['storage_mtime'] and re-writes the cache,
369+
// causing View::getCacheEntry to trigger propagateChange on every read.
370+
$path = $this->normalizePath($path);
371+
$cacheEntry = $this->getCache()->get($this->getCachePath($path));
372+
if ($cacheEntry instanceof CacheEntry) {
373+
$data['storage_mtime'] = $cacheEntry->getStorageMTime();
374+
} elseif (!$this->isRoot($path) && $directoryMarker = $this->headObject($path . '/')) {
375+
$data['storage_mtime'] = strtotime($directoryMarker['LastModified']);
376+
}
377+
}
378+
return $data;
379+
}
380+
362381
public function is_dir(string $path): bool {
363382
$path = $this->normalizePath($path);
364383

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)