Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

Replace Jackrabbit-webdav and deprecated/end-of-life HttpClient 3.1 libs with active and trustable WebDAV + HTTP client libraries #51

Closed
77 tasks done
ghost opened this issue Jan 23, 2015 · 10 comments

Comments

@ghost
Copy link

ghost commented Jan 23, 2015

Hi,

it seems the android-library is currently using the HttpClient libs in version 3.1 released in 2007. This has reached End-Of-Life in 2011 (https://hc.apache.org/httpclient-legacy/index.html).

Have you considered to move to the still maintananced https://hc.apache.org/ ?


Edited by @davigonz from here on


ACs


TOPICS TO DISCUSS

  • How to update OwnCloudClient:
    • Option 1: Extend OkHttpClient instead of HttpClient.
    • Option 2: Do not extend OkHttpClient and decouple it from OwnCloudClient, so that OkHttpClient can be easily used regardless of implementation details and OwnCloudClient is not too affected when upgrading OkHttpClient to future versions.
  • How to implement the new WebDAV methods
    • Option 1: use a new wrapper for all the methods.

TASKS

  • Look for active and trustable WebDAV + HTTP client libraries.
    Candidates:
    1. OKHttp 3.10.0: https://github.com/square/okhttp
    2. Sardine: https://github.com/lookfirst/sardine Not enough active nor trustable
    3. HttpUrlConnection: https://developer.android.com/reference/java/net/HttpURLConnection.html
  • Include new network library dependency.
  • Use our sample_client app to perform some tests:
    • GET to retrieve server status.
    • PROPFIND to read remote folder.
  • Separate the previous tests in one branch for each candidate library:
  • Implement basic requests by using the sample client app and the new endpoint, without depending on ownCloud Android library => PR: Test OkHttp and new endpoint before network library replacement #187
    • Check server (GET): show the server version
    • Refresh (PROPFIND): show the xml contained in propfind response with the list of files
    • Upload (PUT): show successful message if the upload is properly performed.
    • Download (GET): show successful message if the download is properly performed. The downloaded file won't be created locally in this stage.
    • Delete remote (DELETE): show successful message if the deletion is properly performed.
  • Having a look at the code of a different library and testing it
  • Create wrapper with generic WebDAV and non WebDAV methods.
    • WebDAV
      • Propfind
      • Copy
      • Move
      • Put
      • MkCol
    • Non WebDAV
      • Get
      • Post
      • Put
      • Delete
  • Requests to implement, with the headers and properties required
    • Files (WEBDAV - /remote.php/dav/files/davUser/):
      • UploadRemoteFileOperation => PUT
        • Upload working
        • Upload progress in notification bar and uploads view
        • Cancel upload
        • Retry upload
      • ChunkedUploadRemoteFileOperation, use new chunking=> PUT
        • Upload working
        • Upload progress in notification bar and uploads view
        • Cancel chunked upload
          • Delete remote chunks folder when canceling upload
        • Retry chunked upload
      • DownloadRemoteFileOperation
      • RemoveRemoteFileOperation
      • CopyRemoteFileOperation => COPY
      • RenameRemoteFileOperation => LOCALMOVE (MOVE)
      • CreateRemoteFolderOperation => MKCOL
      • ExistenceCheckRemoteOperation => PROPFIND
      • MoveRemoteFileOperation => MOVE
      • ReadRemoteFileOperation => PROPFIND
      • ReadRemoteFolderOperation => PROPFIND
        • Request properties:
          • creationdate
          • quota-used-bytes
          • permissions
          • quota-available-bytes
          • id
          • getcontentlength
          • size
          • privatelink
    • OAuth:
      • OAuth2GetAccessTokenOperation
      • OAuth2RefreshAccessTokenOperation
    • Shares /ocs/v1.php/apps/files_sharing/api/v1/shares:
      • CreateRemoteShareOperation => POST
      • GetRemoteShareesOperation => GET
      • GetRemoteShareOperation => GET
      • GetRemoteSharesForFileOperation => GET
      • GetRemoteSharesOperation => GET
      • RemoveRemoteShareOperation => DELETE
      • UpdateRemoteShareOperation => PUT
    • Status:
      • GetRemoteStatusOperation /status.php => GET
      • GetRemoteCapabilitiesOperation /ocs/v1.php/cloud/capabilities => GET
    • Users:
      • GetRemoteUserAvatarOperation /index.php/avatar/=> GET
      • GetRemoteUserInfoOperation /ocs/v1.php/cloud/user => GET
      • GetRemoteUserQuotaOperation /ocs/v1.php/cloud/users/username => GET
  • Create custom interceptors to put user agent and credentials headers in all the requests.
  • Merge non ownCloud changes to dav4android by creating separate MRs
  • Update sample client app to use the current library operations.

TESTS

[TO DO]


RELATED ISSUES

@codeling
Copy link

codeling commented Jun 1, 2015

Or maybe even better, moving to the HttpURLConnection already included in android? But I guess this would mean also having to rewrite the jackrabbit-webdav library to use something else, right?

@codeling
Copy link

codeling commented Jun 1, 2015

@masensio @davivel what's your take on this?
Further question: Is it possible that a dependency to such an old library causes gradle builds to fail? see mendhak/gpslogger#316

@davivel
Copy link
Contributor

davivel commented Aug 14, 2015

@RealRancor, @codeling, sorry for the lack of response.

As @codeling said, the key point why HttpClient 3.1 is kept is that Jackrabbit-WebDAV depends on it. And depends on HttpClient 3.x, HttpClient 4.x is not backwards compatible with 3.x.

We just updated the version of Jackrabbit-Webdav (ready in master) and checked all the buld environments, including Gradle. We found no special problem.

About considering a change: of course, several times. We searched for replacements of Jackrabbit-Webdav that depend on newer HTTP software, but had a hard time finding alternatives easy-to-trust. We almost decided to go with https://github.com/lookfirst/sardine , but in the end didn't find the appropiate moment to go for it.

We'd rather not writing the code to parse WebDAV responses ourselves, since would take more time and would be prone to errors.

@davivel davivel changed the title Using deprecated/end-of-life HttpClient 3.1 libs Replace Jackrabbit-webdav and deprecated/end-of-life HttpClient 3.1 libs with active and trustable WebDAV + HTTP client libraries Aug 14, 2015
@davigonz davigonz added this to the 0.9.20 milestone Jan 24, 2018
@davigonz davigonz self-assigned this Jan 24, 2018
@davigonz
Copy link
Contributor

but in the end didn't find the appropiate moment to go for it.

This moment has arrived 🙌 , let's create some tasks...

@davigonz
Copy link
Contributor

After playing a bit with OkHttp by using the test app included in our Android library, I conclude that it's possible to use Webdav methods but, as we expected time ago, we would need extra work to handle Webdav requests and responses since OkHttp doesn't include specific methods for Webdav as Jackrabbit-webdav did. Apart from that, the OkHttp library looks nice and it could fit in our app, but with some changes.

Related to the other library, Sardine, is not as active as OkHttp and in addition to not including any reference to compatibility with Android, it depends on apache httpclient and there are some opened issues about apache httpclient conflicts, similar to what I explained in #182, so IMHO, is not a trustable option.

The request I've performed with our test app is one of our main requests, which is a PROPFIND to read the remote folder and works as expected, I will keep working on new requests with the test app before including the OkHttp library in our Android library.

CC / @jesmrec @michaelstingl

@michaelstingl
Copy link
Contributor

is not a trustable option.

@davigonz Thank you for checking it!

@davigonz How could we test early if OkHttp plays well with MI-wrapping? /cc @ChrisEdS

@davigonz
Copy link
Contributor

davigonz commented Jan 29, 2018

How could we test early if OkHttp plays well with MI-wrapping?

@michaelstingl I can adapt to OkHttp the first request the app performs to check the server, if the server is not reachable, tunneling is not working.

I could even use the test app for that purpose.

@ChrisEdS
Copy link

Yes @davigonz let's try with the test app. Please send it - i will try with wrapping and tunneling.

@davigonz
Copy link
Contributor

davigonz commented Oct 9, 2018

This is ready and to be included in 0.9.21 version of the library and 2.9.0 version of the app.

@davigonz davigonz closed this as completed Oct 9, 2018
@theScrabi
Copy link
Contributor

Awesome :D

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants