Skip to content

Add webdav trash bin endpoint#34690

Closed
DeepDiver1975 wants to merge 5 commits into
masterfrom
feature/webdav-trash
Closed

Add webdav trash bin endpoint#34690
DeepDiver1975 wants to merge 5 commits into
masterfrom
feature/webdav-trash

Conversation

@DeepDiver1975

@DeepDiver1975 DeepDiver1975 commented Mar 5, 2019

Copy link
Copy Markdown
Member

Description

We want a WebDAV API as discussed in #15646

This is how it looks like:

  • remote.php/dav/trash-bin is the root folder for all trashbins - each user will find their own trahsbin inside
  • remote.php/dav/trash-bin/admin is the users root for trashbins
  • remote.php/dav/trash-bin/items holds all deleted items - items are listed by file id
  • remote.php/dav/trash-bin/restore is a folder which is used to restore trashed items by moving them in there. This folder itself will not hold any items

Related Issue

How Has This Been Tested?

  • tests will follow

Screenshots (if appropriate):

Screenshot from 2019-06-21 13-25-17
Screenshot from 2019-06-21 13-25-28
Screenshot from 2019-06-21 13-25-48
Screenshot from 2019-06-21 13-26-48

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • Backport (if applicable set "backport-request" label and remove when the backport was done)

@codecov

codecov Bot commented Mar 5, 2019

Copy link
Copy Markdown

Codecov Report

Merging #34690 into master will decrease coverage by 0.1%.
The diff coverage is 7.08%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #34690      +/-   ##
============================================
- Coverage     65.25%   65.14%   -0.11%     
- Complexity    18458    18507      +49     
============================================
  Files          1207     1213       +6     
  Lines         69895    70022     +127     
  Branches       1280     1280              
============================================
+ Hits          45608    45617       +9     
- Misses        23915    24033     +118     
  Partials        372      372
Flag Coverage Δ Complexity Δ
#javascript 53.04% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 66.53% <7.08%> (-0.13%) 18507 <49> (+49)
Impacted Files Coverage Δ Complexity Δ
apps/dav/lib/TrashBin/TrashBinFolder.php 0% <0%> (ø) 11 <11> (?)
apps/dav/lib/TrashBin/TrashBinManager.php 0% <0%> (ø) 9 <9> (?)
apps/dav/lib/TrashBin/TrashBinFile.php 0% <0%> (ø) 9 <9> (?)
apps/dav/lib/TrashBin/TrashBinHome.php 0% <0%> (ø) 12 <12> (?)
apps/dav/lib/RootCollection.php 100% <100%> (ø) 1 <0> (ø) ⬇️
apps/dav/lib/Server.php 50% <100%> (+0.34%) 24 <0> (ø) ⬇️
apps/dav/lib/TrashBin/TrashBinPlugin.php 20% <20%> (ø) 6 <6> (?)
apps/dav/lib/TrashBin/RootCollection.php 50% <50%> (ø) 2 <2> (?)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 75e9d6f...6a9e284. Read the comment docs.

@codecov

codecov Bot commented Mar 5, 2019

Copy link
Copy Markdown

Codecov Report

Merging #34690 into master will decrease coverage by 0.43%.
The diff coverage is 5.52%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #34690      +/-   ##
============================================
- Coverage     65.68%   65.25%   -0.44%     
- Complexity    18775    18831      +56     
============================================
  Files          1222     1231       +9     
  Lines         70932    71095     +163     
  Branches       1289     1289              
============================================
- Hits          46594    46393     -201     
- Misses        23960    24324     +364     
  Partials        378      378
Flag Coverage Δ Complexity Δ
#javascript 53.7% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 66.57% <5.52%> (-0.49%) 18831 <56> (+56)
Impacted Files Coverage Δ Complexity Δ
apps/dav/lib/TrashBin/RestoreFolder.php 0% <0%> (ø) 4 <4> (?)
apps/dav/lib/TrashBin/TrashBinFolder.php 0% <0%> (ø) 8 <8> (?)
apps/dav/lib/TrashBin/AbstractTrashBinNode.php 0% <0%> (ø) 13 <13> (?)
apps/dav/lib/TrashBin/TrashBinManager.php 0% <0%> (ø) 9 <9> (?)
apps/dav/lib/TrashBin/TrashBinHome.php 0% <0%> (ø) 6 <6> (?)
apps/dav/lib/TrashBin/TrashBinItemsFolder.php 0% <0%> (ø) 5 <5> (?)
apps/dav/lib/Capabilities.php 0% <0%> (ø) 3 <0> (ø) ⬇️
apps/dav/lib/TrashBin/TrashBinFile.php 0% <0%> (ø) 3 <3> (?)
apps/dav/lib/Server.php 50.67% <100%> (+0.33%) 24 <0> (ø) ⬇️
apps/dav/lib/RootCollection.php 100% <100%> (ø) 1 <0> (ø) ⬇️
... and 26 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73ec7e3...26d8eec. Read the comment docs.

@DeepDiver1975
DeepDiver1975 force-pushed the feature/webdav-trash branch from 6a9e284 to 3e05eb1 Compare March 6, 2019 06:53
@DeepDiver1975
DeepDiver1975 force-pushed the feature/webdav-trash branch 2 times, most recently from d3e2256 to a01979c Compare June 20, 2019 19:46
@DeepDiver1975
DeepDiver1975 force-pushed the feature/webdav-trash branch from 55e30d7 to 26d8eec Compare June 24, 2019 09:27
@DeepDiver1975

Copy link
Copy Markdown
Member Author
* remote.php/dav/trash-bin/restore is a folder which is used to restore trashed items by moving them in there. This folder itself will not hold any items

I'm not too happy with this .... we will not get the etag back in the move header - which is required by the clients. Maybe we need to add special logic here.

$this->server->httpResponse->setHeader('ETag', $eTag);
}
} catch (NotFound $ex) {
// nothing to do then ....

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

destination will be /trash-bin/$user/restore/$original which is not know because the restore folder will not list the content.

Maybe we do a

mv trash-bin/$user/item/$fileId files/$user/$originalLocation

@DeepDiver1975 DeepDiver1975 mentioned this pull request Jun 24, 2019
11 tasks
@michaelstingl

Copy link
Copy Markdown

@davigonz @hosy @felix-schwarz Could this work for a mobile implementation? Any feedback? /cc @guruz @ckamm @ogoffart

@DeepDiver1975

Copy link
Copy Markdown
Member Author

As per discussion with @butonic we will go for the second approach which will give us more flexibility in terms of the restore location and naming.

@DeepDiver1975
DeepDiver1975 deleted the feature/webdav-trash branch June 25, 2019 06:54
@DeepDiver1975

Copy link
Copy Markdown
Member Author

@davigonz @hosy @felix-schwarz Could this work for a mobile implementation? Any feedback? /cc @guruz @ckamm @ogoffart

please follow up in #35636

@guruz

guruz commented Jun 25, 2019

Copy link
Copy Markdown
Contributor

I haven't looked at the code code but this sounds dangerous, what if I have a user called restore or items

remote.php/dav/trash-bin/admin is the users root for trashbins

Assuming user is called 'admin'

remote.php/dav/trash-bin/items holds all deleted items - items are listed by file id
remote.php/dav/trash-bin/restore is a folder which is used to restore trashed items by moving them in there. This folder itself will not hold any items

@guruz

guruz commented Jun 25, 2019

Copy link
Copy Markdown
Contributor

OK, seems like this is adressed, the description of #35636 looks without the items and restore

@lock lock Bot locked as resolved and limited conversation to collaborators Jun 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants