feat: downloadFile progress reporting support

[jkbz64]

Attempt at partially implementing #2

• web support
• android support
• ios support

Introduction

The pull requests covers only downloadFile progress (but also places some foundation for uploadFile implementation and maybe even request support) since it seems like the best candidate for checking progress - when we use it we are probably downloading bigger files using it and it was easiest one to implement (no deep buried logic).

Pull request is marked as draft since I'm not sure if it should or can be merged without at least uploadFile support.
Will also try to implement ios support after getting convention/implementation/approval feedback.

For downloadFile only support we can drop generic interfaces in favor of ones like download-progress event or just only use DOWNLOAD ProgressType.

Naming conventions and implementation methods of course are subject to change based on feedback.

This draft might aswell go nowhere if it's not good enough, no hard feelings :)

Usage

Http.addListener("progress", (e) => {
  console.log(e.type);
  console.log(e.url);
  console.log(e.bytes + " / " + e.contentLength);
});


Http.downloadFile({
  url: "https://upload.wikimedia.org/wikipedia/commons/6/69/Linus_Torvalds.jpeg",
  filePath: "linus.jpeg",
  fileDirectory: "DOWNLOADS",
  method: "GET",
  progress: true
});

API and definitions concers

ProgressType is 'DOWNLOAD' | 'UPLOAD' and so does HttpProgressListener indicate generic interface but only 'DOWNLOAD' is supported at this moment - should it be 'DOWNLOAD' only?

Web implementation concerns and decisions

There is no built-in (AFAIK) progress reporting mechanism in standard fetch API so there are two ways:

• Use some custom fetch library e.g axios, fetch-progress or other to offload support for chunk combining logic but at the cost of breaking change (webFetchExtra arguments may be be different depending on library)
• Write custom one - at the obvious cost of reinventing the wheel (debugging and maintaining it)

For the initial support I went with writing simple chunk combiner by using streams and it also is opt-in - default behaviour happens when the progress is set to false or is not set at all.

Not sure if the implementation should be placed in web.ts.
Event notifying in the initial implementation is not in any way throttled.
Content-Type of response is currently also not being added to Blob.

It works for most cases I tried, would appreciate some testing though to make sure it works good enough.

Note: The current (simple) implementation can be hard for memory since chunks are indeed stored in memory (should take at most memory equal to the downloaded file size).
It can probably be optimized to not consume that much memory, if it's required to be merged I can try to optimize it.

Android implementation concerns and decisions

For the android support I went with notyfing listeners in Http.java file by using @FunctionalInterface lambda passed down to HttpRequestHandler

ProgressEmitter was defined in HttpRequestHandler, but in order to support requests and uploadFile it would need to be probably defined and passed even farther down in CapacitorHttpUrlConnection.java or require some kind of rewrite.

Just like it is in the web case, android implementation is also not throttled in the initial implementation.

[thomasvidas]

Oh wow! Great to see someone tackling this. I'd be fine merging this in without uploadFile support as long as the downloadFile function also has an iOS implementation. And also if it works 😄 I haven't tested it yet but just peeking at the code I'm not seeing any glaring issues

[jkbz64]

After some time the iOS implementation is there, took some tweaking and testing to finally get it right, this is my first time writing in swift so I might have missed something...

iOS implementation concerns and decisions

After some research first thing that seemed to be really easy to implement was to use task.progress.observe but \.fractionComplete (returns 0.0 - 1.0 progress) without access to content-length header is not helpful (not possible to calculate currently downloaded bytes) and \.totalUnitCount (which returns total bytes only when using progress on File) / \.completedUnitCount are not available for network requests....

I ended up using custom URLSessionDownloadDelegate for progress requests which I don't really know everything about but the results seem the same (I think) as URLSession.shared default delegate which is currently used.

The iOS implementation is not in any way throttled just like other implementations.

I might tweak/polish some things before opening the draft as pull request.
One question, before opening the draft do you want me to rebase against current master and/or squash some commits?

[muuvmuuv]

Is there no way in returning an Observable instead of a Promise to report the progress? I don't like the event listener approach here because it would be very difficult to track parallel uploads/downloads with that.

[jkbz64]

Is there no way in returning an Observable instead of a Promise to report the progress?

Do you mean downloadFile returning something other than Promise<HttpDownloadFileResult>? If so then it's a breaking change and it would probably need to wait for major release and probably at least uploadFile progress would need to have it's implementation done by that time to make it somewhat consistent.

I don't like the event listener approach

Neither do I, but it's the most non-invasive/non-breaking approach I thought was possible at that moment.
Sadly I don't think there is (or is there?) PluginCall.getFunction method or something similar in Capacitor, that would make it possible to take advantage of callbacks and do something like that:

Http.downloadFile({ onProgress: (...) })

Would that be sufficient for your use-case?

I'm going to do some digging into the capacitor plugin internals to check if there is a way to achieve at least that (geolocation plugin looks promising).

The "Observable" approach you mention is a concept that would need to be implemented (your angular/rxjs example in the issue is of course not part of the javascript standard) even before implementing downloadFile progress too or the library would need to adapt some kind of implementation of that... It's kind of outside of the scope of this PR.

it would be very difficult to track parallel uploads/downloads with that.

In my opinion its not that __difficult__, you get the type and url of the downloaded file in the event, assuming that url is unique then you can easily dispatch your own events.

Sure it's not hassle-free but something like that should work, no?
It would be even easier with onProgress callback.

function observedDownload(options) {
  const { url } = options;
  
  let promise;

  const progress = new Observable(observer => {
    // This part probably doesn't have to be created for every request
    // for the sake of the example lets leave it like that
    const listener = Http.addListener('progress', e => {
      if (e.type === "DOWNLOAD" && e.url === url)
        observer.next(/** e.bytes or whatever **/)  
    });

    promise = Http.downloadFile({ ...options, progress: true });
    
    // assuming success
    promise.then(() => {
        listener.remove();
        observer.complete();
    });
  }
    
  return [promise, progress];
}

// Somewhere else
const [result, progress] = observedDownload({ url: ... });

progress.subscribe({
  next(...) { },
  complete() { }
});

await result;

// Or you could even move subscribing to function definition and end up with
const result = await observedDownload({ url: ... }, { next(...) {}, complete() });

Disclaimer: I don't have any experience with using observables and my example can be totally wrong.

[muuvmuuv]

@jkbz64 get the point. That was also a thought of mine that Observables are non-standard but would work very well in that case. The callback method would be even better. But for the release now I am fine with everything tbh to just get started ^^ The workaround you shared is not perfect but a good start. I really didn't think about wrapping all in an Observable, good catch! As soon as I am able to work with it I can share an improved one.

[zLinz]

any progress in this feat?

[jkbz64]

@zLinz
The pull request is implementation complete(maybe lacks some polishing here and there) and awaiting testing/review from maintainers.

@muuvmuuv
After digging into the capacitor plugin internals I couldn't find a way to make the callbacks possible (although I found out workaround but it's rather drastic, I definitely won't fight to implement it here 😄 ) so I have opened up discussion/feature request in capacitor repository here ionic-team/capacitor#4976

I'm going to open up the PR for review since I couldn't find a way to use callbacks in options and callback method doesn't work there (and breaks).
Let me know if I should squash/merge some commits

[zLinz]

Thanks for your work, bro!

@zLinz
The pull request is implementation complete(maybe lacks some polishing here and there) and awaiting testing/review from maintainers.

@muuvmuuv
After digging into the capacitor plugin internals I couldn't find a way to make the callbacks possible (although I found out workaround but it's rather drastic, I definitely won't fight to implement it here 😄 ) so I have opened up discussion/feature request in capacitor repository here ionic-team/capacitor#4976

I'm going to open up the PR for review since I couldn't find a way to use callbacks in options and callback method doesn't work there (and breaks).
Let me know if I should squash/merge some commits

Thanks bro! This feature is really awesome,I hope it can be a part of the plugins as soon.

[bazuka5801]

@jkbz64 I tested your PR on Android & iOS, everything works correctly.
🌟 Awesome work!
👍 Thanks for your contributing.

[bazuka5801]

@thomasvidas Can we merge this PR? I'm sure we need to test it on the Web platform.

[thomasvidas]

I've been out on vacation for most of this month but I'll be sure to test, merge, and release as soon as I have a bit of time to do so 😄