feat(files): request extra WebDAV properties and expose them on FsNode - #461
feat(files): request extra WebDAV properties and expose them on FsNode#461oleksandr-nc wants to merge 3 commits into
Conversation
FsNode models a fixed set of properties and the parser dropped everything else, so there was no way to read anything the library does not know about. Mutating PROPFIND_PROPERTIES used to be the only lever, and that is gone since it became a tuple. Add an extra_properties argument to the methods that issue a PROPFIND, SEARCH or REPORT, and collect every returned property FsNode does not model itself in FsNode.extra_properties. Values are kept as the server sent them: a str, None for the empty ones and a dict for the nested ones such as oc:share-types. That also surfaces the properties that were already requested and then thrown away, oc:share-types, oc:checksums, oc:dDC and nc:is-encrypted. Properties are validated against the namespaces the requests declare and deduplicated against the ones asked for anyway. Signed-off-by: Oleksandr Piskun <oleksandr2088@icloud.com>
|
Warning Review limit reached
Next review available in: 3 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe file APIs now accept validated extra WebDAV properties. Returned unmapped properties are preserved in ChangesWebDAV extra-property support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant FilesAPI
participant WebDAV
participant FsNode
Caller->>FilesAPI: request extra_properties
FilesAPI->>WebDAV: send validated property query
WebDAV-->>FilesAPI: return property values
FilesAPI->>FsNode: store unmapped values in extra_properties
FilesAPI-->>Caller: return FsNode
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nc_py_api/files/files.py`:
- Around line 302-314: Update trashbin_list in nc_py_api/files/files.py (lines
302-314) and its async counterpart in nc_py_api/files/files_async.py (lines
310-322) to build the complete TrashBin property list, including the three
TrashBin-specific properties, before calling _validate_extra_properties;
validate extras against that complete list so duplicate requested properties are
excluded in both implementations.
- Around line 82-85: Document the extra_properties parameter in the public
by_path methods: add the same :param extra_properties: description used by the
other changed public methods in nc_py_api/files/files.py lines 82-85 and
nc_py_api/files/files_async.py lines 84-88.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fff9aa91-c54c-4342-ba89-710e70dd282b
📒 Files selected for processing (7)
CHANGELOG.mdnc_py_api/files/__init__.pync_py_api/files/_files.pync_py_api/files/files.pync_py_api/files/files_async.pytests/actual_tests/files_test.pytests_unit/test_extra_properties.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #461 +/- ##
==========================================
+ Coverage 94.89% 94.95% +0.06%
==========================================
Files 50 51 +1
Lines 5875 5947 +72
==========================================
+ Hits 5575 5647 +72
Misses 300 300
🚀 New features to boost your workflow:
|
The integration tests only exercised listdir, by_path and find, so dropping the argument in by_id, list_by_criteria, trashbin_list or get_versions kept the suite green. Mutating each of the seven call sites now fails a test, in both the sync and the async API. While covering trashbin_list: it deduplicated the requested properties against PROPFIND_PROPERTIES only, so asking for one of the three TrashBin properties sent that element twice. Deduplicate against the full list instead and assert the request carries no duplicates. Also document the argument on by_path, which the other methods already had. Signed-off-by: Oleksandr Piskun <oleksandr2088@icloud.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/actual_tests/files_test.py`:
- Around line 1402-1407: Extend the asynchronous test around
AsyncFilesAPI.trashbin_list to assert that duplicate TrashBin extra properties
are deduplicated and that the default oc:share-types property is exposed,
matching the synchronous test’s coverage. Keep the existing nc:has-preview,
plain-file, and invalid-property assertions unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c98e9619-8a2c-4755-b8cd-47363779a874
📒 Files selected for processing (4)
nc_py_api/files/files.pync_py_api/files/files_async.pytests/actual_tests/files_test.pytests_unit/test_extra_properties.py
The TrashBin deduplication and the default oc:share-types exposure were only checked in the sync test, so reintroducing the duplicate in AsyncFilesAPI kept the suite green. Signed-off-by: Oleksandr Piskun <oleksandr2088@icloud.com>
1 participant
FsNodemodels a fixed set of WebDAV properties and the parser dropped everything else, so there was no way to read anything the library does not know about. MutatingPROPFIND_PROPERTIESwas the only lever, and #453 closed it by making the constants tuples.extra_propertiesonlistdir,by_path,by_id,find,list_by_criteria,trashbin_listandget_versions, sync and asyncFsNode.extra_propertiesholds every returned propertyFsNodedoes not model itself, values as the server sent them:str,Nonewhen empty,dictfor nested ones such asoc:share-typesoc:share-types,oc:checksums,oc:dDC,nc:is-encryptedd,oc,nc) and deduplicated against the defaultsNote that
extra_propertiesis therefore rarely empty: a normal listing carriesoc:share-typeseven when nothing was requested.10 unit tests and integration tests for both APIs; value shapes were taken from a live server, not assumed.
Summary by CodeRabbit
New Features
extra_properties.Documentation
Tests