Bump sabre dav to 4.7.1 - #62405
Conversation
1a5d001 to
4f22769
Compare
4f22769 to
f8c7b3d
Compare
There was a problem hiding this comment.
Pull request overview
Updates Nextcloud’s DAV integration and test fixtures to align with the newer Sabre DAV/VObject behavior expected after the dependency bump referenced in nextcloud/3rdparty.
Changes:
- Update multiple ICS/VCF fixtures and calendar test XML payloads to the new Sabre VObject
PRODIDversion string. - Adjust Sabre connector method signatures (
ObjectTree::copy,Directory::copyInto) to match the updated Sabre DAV APIs that include adepthargument.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/lib/Calendar/ManagerTest.php | Updates expected VCALENDAR PRODID in free/busy responses. |
| tests/data/ics/free-busy-request.ics | Updates fixture PRODID string. |
| tests/data/ics/event-builder-without-attendees.ics | Updates fixture PRODID string. |
| tests/data/ics/event-builder-complete.ics | Updates fixture PRODID string. |
| apps/dav/tests/unit/test_fixtures/example-event-default-expected.ics | Updates fixture PRODID string used by DAV unit tests. |
| apps/dav/lib/ExampleContentFiles/exampleContact.vcf | Updates example VCF PRODID string. |
| apps/dav/lib/Connector/Sabre/ObjectTree.php | Updates copy() signature to accept depth (but currently does not use it). |
| apps/dav/lib/Connector/Sabre/Directory.php | Updates copyInto() signature to accept depth (but currently does not use it). |
Comments suppressed due to low confidence (1)
apps/dav/lib/Connector/Sabre/ObjectTree.php:162
- The new $depth argument is currently ignored. For WebDAV COPY on collections, Depth: 0 vs infinity changes semantics; always performing a recursive copy can violate the request and Sabre\DAV's expectations after the signature change.
public function copy($sourcePath, $destinationPath, int $depth = Server::DEPTH_INFINITY): void {
if (!$this->fileView) {
throw new \Sabre\DAV\Exception\ServiceUnavailable('filesystem not setup');
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Should we update the existing beforeCopy / afterCopy listeners for the new depth argument? |
Signed-off-by: Carl Schwan <carl@carlschwan.eu>
Signed-off-by: Carl Schwan <carl@carlschwan.eu>
Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
Signed-off-by: Carl Schwan <carl@carlschwan.eu>
3346cd8 to
a3e12d4
Compare
| $sourceNode = $this->getNodeForPath($sourcePath); | ||
|
|
||
| if ($sourceNode instanceof Directory && $depth !== Server::DEPTH_INFINITY) { | ||
| // Fallback to sabre slow default implementation with $depth support. | ||
| parent::copy($sourcePath, $destinationPath, $depth); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Does that mean that passing a depth is slower than passing no limit?
That sounds surprising, is the depth limit sent by webdav clients?
There was a problem hiding this comment.
@susnux since you are the one who extended the interface: sabre-io/dav#1495
There was a problem hiding this comment.
I also do not know why it should be slower, how did you measure it?
Summary
See nextcloud/3rdparty#2526
TODO
Checklist
3. to review, feature component)stable32)AI (if applicable)