Skip to content

Comments

fix(file-backend): return correct last modified in head object#861

Merged
ferhatelmas merged 1 commit intomasterfrom
ferhat/file-backend-last-mod
Feb 19, 2026
Merged

fix(file-backend): return correct last modified in head object#861
ferhatelmas merged 1 commit intomasterfrom
ferhat/file-backend-last-mod

Conversation

@ferhatelmas
Copy link
Member

@ferhatelmas ferhatelmas commented Feb 16, 2026

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

last modified is wrong for head object in file backend

What is the new behavior?

last modified is correct as in get object

Additional context

closes #858

simplify date construction as well

@ferhatelmas ferhatelmas requested a review from a team as a code owner February 16, 2026 13:40
@coderabbitai
Copy link

coderabbitai bot commented Feb 16, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉


📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • lastModified now consistently reflects the file system's actual modification time (mtime) across all object metadata responses.
  • Tests

    • Added tests that verify lastModified is derived from the file's mtime and returned correctly for both head and get object metadata queries.

Walkthrough

Replaced internal lastModified computation in the file backend to use the filesystem modification time (data.mtime) consistently for getObject, headObject, and all related metadata responses (200/304 and range responses). Added tests that set a file's mtime and assert that headObject(...).lastModified and getObject(...).metadata.lastModified match the filesystem mtime. No public interfaces, eTag logic, or response structures were changed.

Assessment against linked issues

Objective Addressed Explanation
Ensure headObject returns the correct last-modified (matches GET/read) so HEAD and GET are consistent [#858]
🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing the last modified value returned in HEAD object requests for the file backend.
Linked Issues check ✅ Passed The PR successfully addresses issue #858 by correcting the lastModified source from a placeholder to the actual file mtime, ensuring HEAD and GET requests return consistent values.
Out of Scope Changes check ✅ Passed All changes are within scope: file.ts modifications fix the lastModified logic, and test additions validate the fix against the stated requirement.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into master

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/test/file-backend.test.ts (1)

191-215: Use withOptionalVersion to avoid hardcoding the file layout.

This keeps the test resilient if the versioning path format changes.

♻️ Proposed change
-    const filePath = path.join(tmpDir, `${bucket}/${key}/${version}`)
+    const filePath = path.join(tmpDir, withOptionalVersion(`${bucket}/${key}`, version))

@coveralls
Copy link

coveralls commented Feb 16, 2026

Pull Request Test Coverage Report for Build 22154238163

Details

  • 4 of 6 (66.67%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.3%) to 76.263%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/storage/backend/file.ts 4 6 66.67%
Totals Coverage Status
Change from base Build 22154212801: 0.3%
Covered Lines: 26286
Relevant Lines: 34161

💛 - Coveralls

@ferhatelmas ferhatelmas force-pushed the ferhat/file-backend-last-mod branch 4 times, most recently from 371a425 to 0c62610 Compare February 18, 2026 19:21
closes #858

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
@ferhatelmas ferhatelmas force-pushed the ferhat/file-backend-last-mod branch from 0c62610 to 19d84d9 Compare February 19, 2026 15:38
@ferhatelmas ferhatelmas merged commit 2c131f3 into master Feb 19, 2026
2 checks passed
@ferhatelmas ferhatelmas deleted the ferhat/file-backend-last-mod branch February 19, 2026 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

File backend head object returns wrong last-modified

3 participants