Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move files app to use WebDAV instead of custom PHP endpoints #12353

Closed
PVince81 opened this issue Nov 21, 2014 · 47 comments · Fixed by #16902
Closed

Move files app to use WebDAV instead of custom PHP endpoints #12353

PVince81 opened this issue Nov 21, 2014 · 47 comments · Fixed by #16902
Milestone

Comments

@PVince81
Copy link
Contributor

Currently we have at least two endpoints to manage files:

  1. WebDAV
  2. Private PHP ajax endpoints in the files app

To reduce code duplication and endpoint duplication one idea would be to make the files app use the WebDAV endpoint instead, just like any other external client does (mobile, desktop, etc)

An existing JS library could be used (for example: https://github.com/sara-nl/js-webdav-client) by the files app.

We need to make sure that every custom property (ex: "icon", "isPreviewAvailabe", etc) is available on the WebDAV endpoint to keep the existing functionality working.

There might be other challenges like batch file deletion, where the JS code would need to send multiple "DELETE" calls, one for every selected file, instead of deleting a series of files in one call.

@DeepDiver1975 @icewind1991

@dragotin
Copy link
Contributor

Yes.

We will see speed issues. But we can fix these, and each fix will also improve the clients. which in turn will improve the overall ownCloud experience. From my POV this is a good step towards a more unified development of all parts.

About the specific batch problem you mentioned: I highly suggest that we add batch deletes and maybe other operations like move because that is a problem for the clients as well.

@PVince81
Copy link
Contributor Author

@icewind1991 also raised a concern that IE8 might not support verbs like PROPFIND, etc. But we can have some tests about that.

@PVince81
Copy link
Contributor Author

On a side-note I'd also love if ownCloud itself could get rid of internal ajax APIs and only rely on OCS public APIs (some kind of dogfooding).

@PVince81
Copy link
Contributor Author

PVince81 commented Feb 4, 2015

The public page can then use the "public.php/webdav" endpoint for public link file operations (#6635)

@PVince81
Copy link
Contributor Author

PVince81 commented Feb 4, 2015

Note that moving to WebDAV will also have the advantage to automatically use part files internally and solve concurrency issues like #13755

@PVince81
Copy link
Contributor Author

PVince81 commented Feb 4, 2015

Changing the whole file UI is a big task, so this should rather be done step by step:

@PVince81
Copy link
Contributor Author

PVince81 commented Feb 4, 2015

  • read quota/available space value from the list PROPFIND instead of using "getstoragestats.php"

@PVince81 PVince81 added this to the backlog milestone Feb 4, 2015
@PVince81
Copy link
Contributor Author

PVince81 commented Feb 4, 2015

@DeepDiver1975 I'm not sure whether there's time for this in 8.1, but as said above we can split it in smaller pieces.

@PVince81
Copy link
Contributor Author

PVince81 commented Feb 4, 2015

  • make sure upload still works in IE8 (it might be using a workaround for uploads)

@PVince81
Copy link
Contributor Author

PVince81 commented Feb 4, 2015

Got some ideas: currently the JS code for the file list is still quite ugly and it would bring some advantages to move the file handling logic to a separate class/API anyway.
Introducing the JS Files API: #13905

It could be written to be based directly off WebDAV and can be injected and used by JS apps too to access files. This way we not only gain the advantage of having the files web UI based on WebDAV, but also any app that needs access/manipulate files from JS.

So the first step would be to first design and write that new JS Files API class, which itself can use/wrap a WebDAV library.

Then the files web UI can be ported progressively to use that JS Files API.

@PVince81
Copy link
Contributor Author

Anoter challenge:

@PVince81
Copy link
Contributor Author

  • parse Webdav exception and pass it back instead of just the HTTP status code. Can be useful to distinguish error messages.

@PVince81
Copy link
Contributor Author

  • future: also add convenience methods on OC.Files.Client, even if they still need ajax endpoint ? (ex: downloading multiple files)

@PVince81
Copy link
Contributor Author

Two other regressions that have been missed when testing due to the multiple code paths: #17594 and #18006

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 1, 2015

Discussed with @DeepDiver1975, moving to 9.0

@PVince81
Copy link
Contributor Author

  • make sure that apps from owncloud/apps and file viewers are also using the webdav endpoint (or ideally the new FilesClient API)

@MTRichards
Copy link
Contributor

@DeepDiver1975
Copy link
Member

#5939

@MTRichards I did set the milestone to 9.0 - okay?

@MTRichards
Copy link
Contributor

Yep, thank you.

@PVince81
Copy link
Contributor Author

Not fully done, upload is still t obe done.

@PVince81 PVince81 reopened this Nov 23, 2015
@PVince81
Copy link
Contributor Author

also downloading as zip, etc. Keeping this ticket open.

@dragotin
Copy link
Contributor

yeah!!! ☀️

@PVince81
Copy link
Contributor Author

but no upload or download as zip yet (not wanting to darken the mood)

@karlitschek
Copy link
Contributor

Awesome :-)
Gret work @PVince81 and everyone!

@lock
Copy link

lock bot commented Aug 7, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants