Skip to content

fix(files): add FsNode.etag_unquoted, keep etag as the server sent it - #456

Merged
oleksandr-nc merged 1 commit into
mainfrom
fix/etag-quotes
Aug 3, 2026
Merged

fix(files): add FsNode.etag_unquoted, keep etag as the server sent it#456
oleksandr-nc merged 1 commit into
mainfrom
fix/etag-quotes

Conversation

@oleksandr-nc

@oleksandr-nc oleksandr-nc commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

FsNode.etag keeps what the server sent, quotes included, so it can be passed to an If-Match/If-None-Match header unchanged. Stripping them there would break that, measured against NC 35: the server answers 412 to a conditional write and silently ignores the precondition on a conditional read (200 instead of 304). The quotes are part of the entity tag (RFC 9110 §8.8.3).

  • add FsNode.etag_unquoted for callers that want the bare tag, which is what Quoted E-Tags may be stripped #448 asks for
  • FsNode.etag is always a str now: the trashbin sends an empty <d:getetag/>, which used to arrive as None
  • 9 unit tests over every source shape, plus an integration test that sends the unmodified etag back to the server and expects 304/2xx

Closes #448

Summary by CodeRabbit

  • New Features

    • Added etag_unquoted for retrieving ETags without surrounding quotation marks.
    • etag now preserves the server-provided format for direct use in conditional request headers.
    • Missing ETags consistently return as empty strings, including for trashbin entries.
  • Bug Fixes

    • Standardized ETag handling across file listings, responses, and file metadata.
    • Improved coverage for quoted, unquoted, and missing ETag values.

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

FsNode.etag now preserves server-provided quotes and always returns a string. FsNode.etag_unquoted provides the quote-stripped value. Unit and integration tests cover ETag sources and conditional DAV requests.

Changes

ETag normalization

Layer / File(s) Summary
Normalize FsNode ETags
nc_py_api/files/__init__.py, CHANGELOG.md
FsNode preserves quoted ETags, converts missing or None values to "", and exposes etag_unquoted. Documentation and the changelog describe the updated behavior.
Validate ETag sources
tests_unit/test_etag_normalization.py, tests/actual_tests/files_test.py
Tests cover direct construction, Propfind data, response headers, ActionFileInfo conversion, filesystem listings, and conditional DAV requests.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant DAVServer
  participant FsNode
  participant Client
  DAVServer->>FsNode: Return server-provided quoted ETag
  FsNode->>Client: Expose etag and etag_unquoted
  Client->>DAVServer: Send quoted ETag in conditional DAV headers
  DAVServer-->>Client: Return 304 or accept conditional PUT
Loading
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR preserves quoted FsNode.etag and adds etag_unquoted, but issue #448 requests stripping quotes from the exposed ETag. Make FsNode.etag unquoted as requested by #448, or update the issue and compatibility requirements to approve the dual-property API.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Out of Scope Changes check ✅ Passed The code, changelog, and tests directly support ETag representation and conditional request behavior.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main API change: adding FsNode.etag_unquoted while preserving the server-provided FsNode.etag value.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/etag-quotes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

The trashbin answers with an empty `<d:getetag/>`, which arrived as None
despite the str annotation, so FsNode.etag was not always a string.

Normalize that to "" and add FsNode.etag_unquoted for callers that want the
bare tag. `etag` itself keeps exactly what the server sent, quotes included,
so it can still be passed to an If-Match/If-None-Match header unchanged.
Stripping the quotes from `etag` would have broken that: the server answers
412 to a conditional write and silently ignores the precondition on a
conditional read.

Closes #448

Signed-off-by: Oleksandr Piskun <oleksandr2088@icloud.com>
@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.79%. Comparing base (126a31e) to head (9611972).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #456      +/-   ##
==========================================
+ Coverage   94.69%   94.79%   +0.10%     
==========================================
  Files          48       50       +2     
  Lines        5650     5767     +117     
==========================================
+ Hits         5350     5467     +117     
  Misses        300      300              
Files with missing lines Coverage Δ
nc_py_api/files/__init__.py 99.72% <100.00%> (+<0.01%) ⬆️
tests_unit/test_etag_normalization.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@oleksandr-nc oleksandr-nc changed the title fix(files): expose FsNode.etag without the surrounding quotes fix(files): add FsNode.etag_unquoted, keep etag as the server sent it Aug 3, 2026
@oleksandr-nc
oleksandr-nc merged commit c6b96fd into main Aug 3, 2026
13 checks passed
@oleksandr-nc
oleksandr-nc deleted the fix/etag-quotes branch August 3, 2026 15:22
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.

Quoted E-Tags may be stripped

1 participant