Alternative API without callbacks or without auto-closing

[grote]

I was hoping to use this library for a native WebDAV connection for the Seedvault backup app. However, after digging more into it, I noticed that all requests made via DavCollection are using a callback and that the okhttp Response gets auto-closed after the callback was invoked.

This is really bad for using these requests in an existing complicated codebase, because all stream operations (reading/writing) have to happen inside this callback. A more flexible approach would be to give the API user access to the streams/sinks and don't auto-close them.

To illustrate where I am coming from: The Android back API is calling my code repeatedly to give me bytes of app's backup data in small chunks. I have no control over how I am called, but I need to write the bytes away for safe-keeping. So there's no way of doing this inside a callback of an dav4jvm request. If I'd simply get an OutputStream from the library, I could keep writing to it until I am done and then close it.

So I could probably use suspendCoroutine() to work around the callback, but then the auto-closing is really breaking my neck here.

Is it imaginable that this library accommodates my use-case in a not too far away future? This may or may not be related to #2

[rfc2822]

Is it about sending the data to the server or about reading from the server?

If you want upload data from a stream, you can generate the RequestBody from a stream like here:

https://github.com/bitfireAT/davx5-ose/blob/5a5023bf542755a746b546e46c5aef2894475fb7/app/src/main/kotlin/at/bitfire/davdroid/webdav/StreamingFileDescriptor.kt#L162-L195

Would that be an option?

Otherwise yes maybe async methods (I'd also like to have them, but okhttp is still in alpha for that), or an optional parameter that allows getting the response without closing? But if it's only for uploading, that shouldn't be needed. And for streamed downloading, this is normally done with GET (which you can control yourself anyway) or in small in-line packages (like in calendar-multiget), then the callback should be fine.

[grote]

Thanks for the quick response!

Is it about sending the data to the server or about reading from the server?

It is about both. In simplified terms: For backup I need to send data, for restore I need to read data.

If you want upload data from a stream, you can generate the RequestBody from a stream like here:

Thanks, this looks similar to the stream recipe in the okhttp documentation. I actually also have a ParcelFileDescriptor to work with. However, as hinted at above, I need to work with a peculiar API that seems to do some sort of pumping of data into that FD. So I am getting callbacks for reading n bytes of data from the FD and can't read before that callback comes in.

If you are interested, here's the API and here how they implemented it.

So I don't really see a way how this would work with inheriting RequestBody. Currently, I hold on to an OutputStream and write to it when I can, as callback arrive. Then after I am told that I am done, I am closing the stream. Maybe I can hold on to the RequestBody and have its writeTo() method block until I have more data available for writing.

for streamed downloading, this is normally done with GET (which you can control yourself anyway)

Oh I haven't seen this. It looked like I had to read from the response body stream before the callback terminates. Or are you suggesting to make my own GET requests with okhttp and not using the library?

I haven't looked into the restore use-case in detail, but a cursory look suggests that it may be more feasible to do inside a callback (only that I'd have to refactor my entire plugin API and the code that uses it). So getting the response body stream and being allowed to close it myself when done would be helpful I think.

[rfc2822]

Ah I see. I have made a few thoughts, but I don't really know the backup API nor whether this is compatible with your architecture. But what do you think about this:

Uploading to WebDAV

• Create a PipedInputStream, where the DavResource.put() will then read from.
• Create a PipedOutputStream that is connected to the PipedInputStream and where you pipe your backup data in.
• When the backup is started, call DavResource.put() in a separate coroutine (running in Dispatchers.IO), remember the async block, and provide the PipedInputStream to the RequestBody.
• When data arrives from the backup API, pipe it to the PipedOutputStream.
• The coroutine in the IO thread copies the data blocks from the PipedInputStream into the WebDAV request body (like the mentioned code in the DAVx⁵ StreamingFileDescriptor.uploadNow()). If an error occurs, it should be propagated to the main thread/backup API somehow.
• When the backup is finished, flush/close the streams and put() will finish. You'll receive the result in the coroutine and wait for it in the main thread with something like asyncBlock.get().

(Or the other way, call put() in the main thread which will then block, and pipe data into it from a separate thread, if that is supported by the backup API).

As soon as okhttp provides an async API (#2), the code could be refactored to use suspending coroutines instead of the blocking call in a separate thread. For now I think there's not really another way to do it because put() is always blocking and so can you can't send multiple blocks that you receive from multiple backup API calls to it, unless you build your own pipe.

As I understand it, it wouldn't help to just return the Response instead of calling the callback in put() because the problem comes from the blocking input. Or do I overlook something?

Downloading from WebDAV

I think in this case, there should be another DavResource.get() method that returns the streamable HTTP response instead of the callback:

# Caller is responsible for
# - processing HTTP error codes (it may call checkStatus())
# - closing the response
fun get(accept: String, headers: Headers?): Response

[grote]

But what do you think about this:

This sounds good. Thanks!

call DavResource.put() in a separate coroutine (running in Dispatchers.IO)

While a bit off-topic here, do you know if there's a guarantee that this actually executes in a different thread? What if I am already on an I/O thread and Kotlin decides it can just stay on this one?

When data arrives from the backup API, pipe it to the PipedOutputStream.

Do I need to be worried about timeouts? I guess as long as I keep writing to that OutputStream (without longer pauses) and then close it right after, the HTTP layer should be happy.

When the backup is finished, flush/close the streams and put() will finish. You'll receive the result in the coroutine and wait for it in the main thread with something like asyncBlock.get().

The way I imagine this to work is that I extend PipedOutputStream with a custom onClose action and do the waiting when the stream gets closed by the main thread. Then I need to define that exceptions in close() mean that we need to assume that data wasn't written (and ensure it really gets closed everywhere, as usual). The advantage is that I do not need to complicate my existing internal API and can put the pipe complexities encapsulated into my WebDAV plugin.

Btw. that last part where we wait for the writing to complete and the server tell us OK is something that Android's Storage Access Framework (SAF) is missing (last I checked). There, you just write into their stream and hope for the best. This is one of the main reasons why I am looking into a "native" WebDAV plugin.

As I understand it, it wouldn't help to just return the Response instead of calling the callback in put() because the problem comes from the blocking input. Or do I overlook something?

Right. That wouldn't help here.

I think in this case, there should be another DavResource.get() method that returns the streamable HTTP response instead of the callback:

That would be great, thanks!

[rfc2822]

While a bit off-topic here, do you know if there's a guarantee that this actually executes in a different thread? What if I am already on an I/O thread and Kotlin decides it can just stay on this one?

I'm not 100% sure, but the launch docs say that launch doesn't block. The block is enqueued to the queue (which is processed by the I/O thread pool), but you may change behavior with the start parameter.

When data arrives from the backup API, pipe it to the PipedOutputStream.

Do I need to be worried about timeouts? I guess as long as I keep writing to that OutputStream (without longer pauses) and then close it right after, the HTTP layer should be happy.

Guess so. You can vary timeouts when creating the OkHttpClient: https://github.com/bitfireAT/davx5-ose/blob/f6ac4e02d66f397cf536ce12275b4f46b221c98d/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClient.kt#L61-L70

But that of course won't save you from server-side timeouts.

When the backup is finished, flush/close the streams and put() will finish. You'll receive the result in the coroutine and wait for it in the main thread with something like asyncBlock.get().

The way I imagine this to work is that I extend PipedOutputStream with a custom onClose action and do the waiting when the stream gets closed by the main thread. Then I need to define that exceptions in close() mean that we need to assume that data wasn't written (and ensure it really gets closed everywhere, as usual). The advantage is that I do not need to complicate my existing internal API and can put the pipe complexities encapsulated into my WebDAV plugin.

Btw. that last part where we wait for the writing to complete and the server tell us OK is something that Android's Storage Access Framework (SAF) is missing (last I checked). There, you just write into their stream and hope for the best. This is one of the main reasons why I am looking into a "native" WebDAV plugin.

As I had understood it, you can write to the SAF API and when you close the stream (which would also flush the buffer and complete the HTTP request) and it still doesn't throw an (IO)Exception, everything is fine. If there's an error, DAVx5 closes the file descriptor with an error:

https://github.com/bitfireAT/davx5-ose/blob/f6ac4e02d66f397cf536ce12275b4f46b221c98d/app/src/main/kotlin/at/bitfire/davdroid/webdav/StreamingFileDescriptor.kt#L78

and I'd hope that the SAF API then throws an IOException. But I haven't tried it yet… did you experience problems with HTTP errors while the API didn't return an error?

I think in this case, there should be another DavResource.get() method that returns the streamable HTTP response instead of the callback:

That would be great, thanks!

See #38

[grote]

As I had understood it, you can write to the SAF API and when you close the stream (which would also flush the buffer and complete the HTTP request) and it still doesn't throw an (IO)Exception, everything is fine.
...
If there's an error, DAVx5 closes the file descriptor with an error:

Great! I should I have documented my findings better at the time. Unfortunately, I haven't, so I must assume that proper SAF implementations can and do expose write errors when closing the OutputStream. What had prompted the investigation back then was the Nextcloud app's SAF implementation writing streams to local cache and then adding the files to an internal upload queue. There, I am sure, we would not get notified when server uploads failed.

See #38

Awesome, thanks a lot! Do you still tag releases or do you expect library consumers to just use latest main via jitpack? I am asking, because last tag is quite old already.

[rfc2822]

Awesome, thanks a lot! Do you still tag releases or do you expect library consumers to just use latest main via jitpack? I am asking, because last tag is quite old already.

I recommend to use a specific commit ID (instead of a tag) over jitpack and then update the commit ID from time to time. This is how it's done in DAVx5.