fix(files_external): S3 folder mtime updated on every read-only access

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

@@ -70,6 +70,11 @@ private function isRoot(string $path): bool {
 		return $path === '.';
 	}
 
+	private function getCachePath(string $path): string {
+		// normalizePath() converts '' to '.' for S3 object keys, but filecache stores the root as ''
+		return $this->isRoot($path) ? '' : $path;
+	}
+
 	private function cleanKey(string $path): string {
 		if ($this->isRoot($path)) {
 			return '/';
@@ -326,6 +331,28 @@ public function stat(string $path): array|false {
 		return $stat;
 	}
 
+	#[\Override]
+	public function getMetaData(string $path): ?array {
+		$data = parent::getMetaData($path);
+		if ($data !== null && $data['mimetype'] === FileInfo::MIMETYPE_FOLDER) {
+			// Common::getMetaData sets storage_mtime = mtime, but for S3 virtual directories
+			// mtime may have been updated by mtime propagation while storage_mtime should
+			// reflect the actual last storage change. Without this override the scanner sees
+			// data['storage_mtime'] != cacheData['storage_mtime'] and re-writes the cache,
+			// causing View::getCacheEntry to trigger propagateChange on every read.
+			$path = $this->normalizePath($path);
+			$cacheEntry = $this->getCache()->get($this->getCachePath($path));
+			if ($cacheEntry instanceof CacheEntry) {
+				$data['storage_mtime'] = $cacheEntry->getStorageMTime();
+			} elseif (!$this->isRoot($path) && $directoryMarker = $this->headObject($path . '/')) {
+				if (isset($directoryMarker['LastModified'])) {
+					$data['storage_mtime'] = strtotime($directoryMarker['LastModified']);
+				}
+			}
+		}
+		return $data;
+	}
+
 	#[\Override]
 	public function is_dir(string $path): bool {
 		$path = $this->normalizePath($path);
@@ -649,12 +676,14 @@ public function getDirectoryContent(string $directory): \Traversable {
 	}
 
 	private function objectToMetaData(array $object): array {
+		$mtime = isset($object['LastModified']) ? strtotime($object['LastModified']) : time();
+		$etag = isset($object['ETag']) ? trim($object['ETag'], '"') : '';
 		return [
 			'name' => basename($object['Key']),
 			'mimetype' => $this->mimeDetector->detectPath($object['Key']),
-			'mtime' => strtotime($object['LastModified']),
-			'storage_mtime' => strtotime($object['LastModified']),
-			'etag' => trim($object['ETag'], '"'),
+			'mtime' => $mtime,
+			'storage_mtime' => $mtime,
+			'etag' => $etag,
 			'permissions' => Constants::PERMISSION_ALL - Constants::PERMISSION_CREATE,
 			'size' => (int)($object['Size'] ?? $object['ContentLength']),
 		];
@@ -667,7 +696,7 @@ private function getDirectoryMetaData(string $path): ?array {
 		if ($this->versioningEnabled() && !$this->doesDirectoryExist($path)) {
 			return null;
 		}
-		$cacheEntry = $this->getCache()->get($path);
+		$cacheEntry = $this->getCache()->get($this->getCachePath($path));
 		if ($cacheEntry instanceof CacheEntry) {
 			return $cacheEntry->getData();
 		} else {

apps/files_external/tests/Storage/Amazons3Test.php

@@ -8,6 +8,7 @@
  */
 namespace OCA\Files_External\Tests\Storage;
 
+use OC\Files\Cache\Scanner;
 use OCA\Files_External\Lib\Storage\AmazonS3;
 
 /**
@@ -38,7 +39,56 @@ protected function tearDown(): void {
 		parent::tearDown();
 	}
 
-	public function testStat(): void {
-		$this->markTestSkipped('S3 doesn\'t update the parents folder mtime');
+	/**
+	 * Regression test for the '.' vs '' root path mismatch in getDirectoryMetaData.
+	 *
+	 * normalizePath('') returns '.' for S3 object keys, but the filecache stores the
+	 * storage root under the key ''. Before the fix, getCache()->get('.') returned false,
+	 * causing getDirectoryMetaData to return a fabricated time() on every call, which
+	 * made getCacheEntry always see a changed storage_mtime and fire propagateChange.
+	 */
+	public function testStatRootPreservesStorageMtimeFromCache(): void {
+		$this->instance->getScanner()->scan('', Scanner::SCAN_SHALLOW);
+
+		$cachedRoot = $this->instance->getCache()->get('');
+		$this->assertNotFalse($cachedRoot, 'Root entry must exist in cache after scan');
+
+		$cachedStorageMtime = $cachedRoot['storage_mtime'];
+
+		$stat = $this->instance->stat('');
+		$this->assertNotFalse($stat, 'stat(\'\') must return data');
+		$this->assertEquals(
+			$cachedStorageMtime,
+			$stat['storage_mtime'],
+			'stat(\'\') must return storage_mtime from the cache entry, not a fabricated time()'
+		);
 	}
+
+	/**
+	 * Regression test: Common::getMetaData sets storage_mtime = mtime, but for S3 virtual
+	 * directories mtime may have been bumped by propagation while storage_mtime should stay
+	 * stable. The override restores storage_mtime from the cache entry so the scanner does
+	 * not see a spurious mismatch and re-write the cache on every scan.
+	 */
+	public function testGetMetaDataDirectoryPreservesStorageMtimeSeparateFromMtime(): void {
+		$this->instance->getScanner()->scan('', Scanner::SCAN_SHALLOW);
+
+		$cachedRoot = $this->instance->getCache()->get('');
+		$this->assertNotFalse($cachedRoot, 'Root entry must exist in cache after scan');
+
+		// Simulate propagation bumping mtime without touching storage_mtime
+		$originalStorageMtime = $cachedRoot['storage_mtime'];
+		$this->instance->getCache()->update($cachedRoot->getId(), [
+			'mtime' => $originalStorageMtime + 9999,
+		]);
+
+		$meta = $this->instance->getMetaData('');
+		$this->assertNotNull($meta, 'getMetaData(\'\') must return data');
+		$this->assertEquals(
+			$originalStorageMtime,
+			$meta['storage_mtime'],
+			'getMetaData must return storage_mtime from cache, not the propagated mtime'
+		);
+	}
+
 }

lib/private/Files/View.php

@@ -1410,9 +1410,17 @@ private function getCacheEntry($storage, $internalPath, $relativePath) {
 				$data = $cache->get($internalPath);
 			} elseif (!Scanner::isPartialFile($internalPath) && $watcher->needsUpdate($internalPath, $data)) {
 				$this->lockFile($relativePath, ILockingProvider::LOCK_SHARED);
+				$cacheDataBefore = $data instanceof CacheEntry ? $data->getData() : false;
 				$watcher->update($internalPath, $data);
-				$storage->getPropagator()->propagateChange($internalPath, time());
 				$data = $cache->get($internalPath);
+				$cacheDataAfter = $data instanceof CacheEntry ? $data->getData() : false;
+
+				// Only propagate mtime change to parent folders if the scanner actually changed the cached metadata,
+				// to avoid updating folder mtimes on every read for backends that conservatively report directories as updated (e.g. S3)
+				if ($cacheDataAfter !== $cacheDataBefore) {
+					$storage->getPropagator()->propagateChange($internalPath, time());
+					$data = $cache->get($internalPath);
+				}
 				$this->unlockFile($relativePath, ILockingProvider::LOCK_SHARED);
 			}
 		} catch (LockedException $e) {

tests/lib/Files/ViewTest.php

@@ -80,6 +80,16 @@ public function instanceOfStorage(string $class): bool {
 	}
 }
 
+/**
+ * Storage that always reports hasUpdated() = true for any path, simulating the
+ * behavior of Amazon S3 external storage (which has no real directory objects).
+ */
+class TemporaryAlwaysUpdated extends Temporary {
+	public function hasUpdated(string $path, int $time): bool {
+		return true;
+	}
+}
+
 class TestEventHandler {
 	public function umount() {
 	}
@@ -456,6 +466,46 @@ public function testWatcher(): void {
 		$this->assertEquals(3, $cachedData['size']);
 	}
 
+	/**
+	 * Regression test for View::getCacheEntry unconditionally calling propagateChange
+	 * when the watcher reports needsUpdate = true, regardless of whether the scanner
+	 * found any actual storage change.
+	 *
+	 * Backends like Amazon S3 always return hasUpdated() = true for directory paths
+	 * because S3 has no real directory objects. Before the fix, every getFileInfo()
+	 * call on a folder fired propagateChange(path, time()), stamping all ancestor
+	 * folders in the filecache with the current request timestamp.
+	 *
+	 * After the fix, propagateChange is only called when watcher->update() actually
+	 * changes the cached metadata for the path.
+	 */
+	public function testWatcherDoesNotPropagateWhenStorageMtimeUnchanged(): void {
+		$storage = $this->getTestStorage(true, TemporaryAlwaysUpdated::class);
+		Filesystem::mount($storage, [], '/');
+		$storage->getWatcher()->setPolicy(Watcher::CHECK_ALWAYS);
+
+		$rootView = new View('');
+
+		// Note the root mtime right after the initial scan.
+		$rootMtimeBefore = $storage->getCache()->get('')['mtime'];
+
+		// Access a subfolder. The watcher will fire (hasUpdated always returns true),
+		// but the scanner leaves the cached metadata for 'folder' unchanged.
+		// getCacheEntry must therefore NOT call propagateChange('folder', time()),
+		// which would update the root entry's mtime to the current timestamp.
+		$rootView->getFileInfo('folder');
+
+		// Read the root mtime directly from the cache to avoid triggering another watcher cycle.
+		$rootMtimeAfter = $storage->getCache()->get('')['mtime'];
+
+		$this->assertEquals(
+			$rootMtimeBefore,
+			$rootMtimeAfter,
+			'Root folder mtime must not be updated when the watcher fires but cached metadata has not changed'
+		);
+	}
+
+
 	public function testCopyBetweenStorageNoCross(): void {
 		$storage1 = $this->getTestStorage(true, TemporaryNoCross::class);
 		$storage2 = $this->getTestStorage(true, TemporaryNoCross::class);