[stable34] fix(files_external): S3 folder mtime updated on every read-only access#61977
Open
backportbot[bot] wants to merge 4 commits into
Open
[stable34] fix(files_external): S3 folder mtime updated on every read-only access#61977backportbot[bot] wants to merge 4 commits into
backportbot[bot] wants to merge 4 commits into
Conversation
normalizePath('') returns '.' for S3 object keys, but the filecache stores the
storage root under the key ''. getDirectoryMetaData() was calling getCache()->get('.')
which looks up by md5('.'), never matching the root entry stored at md5('').
The cache miss caused getDirectoryMetaData to return synthetic data with time() as
mtime/storage_mtime and uniqid() as etag on every call. The scanner then saw a
storage_mtime mismatch and wrote the fabricated timestamps back to the cache on every
occ files:scan run, regardless of whether any S3 content had changed.
Introduce getCachePath() to translate the normalized root path '.' back to '' before
any filecache lookup.
Assisted-by: ClaudeCode:claude-sonnet-4-6
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
…rs 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>
…changed Storage backends like AmazonS3 whose hasUpdated() always returns true (S3 has no cheap global change-detection mechanism) caused propagateChange() to be called on every watcher->update() after a folder was browsed, even when the underlying storage_mtime and etag were identical to the cached values. Before the fix, getCacheEntry() would call watcher->update() and then unconditionally call propagateChange() whenever the watcher reported a change. For S3 directories this meant every read bumped the parent mtime chain. After the fix, getCacheEntry() snapshots the cache entry before watcher->update() and compares it with the entry after. propagateChange() is only invoked when at least one metadata field (mtime, storage_mtime, size, or etag) actually changed. Assisted-by: ClaudeCode:claude-sonnet-4-6 Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Add null-safety checks to handle S3 responses that don't include
LastModified and ETag fields. This prevents 'Undefined array key'
warnings and deprecation notices when processing directory metadata
or incomplete S3 responses.
- objectToMetaData(): Check if LastModified/ETag exist before accessing
- getMetaData(): Check if LastModified exists before using in strtotime()
Fixes test failures in testStat where hasUpdated('/', time) would fail
when encountering S3 objects without complete metadata.
Assisted-by: ClaudeCode:claude-sonnet-4-6
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
backportbot
Bot
requested review from
a team,
Antreesy,
CarlSchwan,
come-nc,
icewind1991,
leftybournes,
provokateurin and
salmart-dev
and removed request for
a team,
come-nc,
icewind1991,
provokateurin and
salmart-dev
backportbot
Bot
added
bug
3. to review
Antreesy
approved these changes
Jul 10, 2026
Antreesy
left a comment
Antreesy
left a comment
Contributor
There was a problem hiding this comment.
Tested (docker-dev, rust-fs as external S3)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of PR #59539