Add download functionality

[mdbraber]

Initial commit for adding in-app download functionality. @joelekstrom there's a few settings that should be user-configurable (selecting download-directory and auto-opening extensions). Could you take a look at how to integrate those in the settings window? I've added a FIXME statement in places to check.

[joelekstrom]

Thanks for the PR! Much appreciated

[mdbraber]

@joelekstrom got it to work - see changes in the PR. Can you help check and also take a look at ideas for integrating this in the settings. Or do we not need to make any of this configurable (download location, auto-open)?

[joelekstrom]

@mdbraber I think as a first step it's good enough without settings - of course a setting would be nice but that could be another PR 👍

[joelekstrom]

Looks much better, with a few notes!

[mdbraber]

@joelekstrom I've created a final update to include download related settings and to use a FileDownloadManager to detect already running downloaded processes. Should be the last update before we can hopefully finish this. I might have done some illogical things as I'm still rather inexperienced coding in Obj-C :-) Thanks for your feedback so far.

[joelekstrom]

@mdbraber Great job! I've looked through the code and will come back with some suggestions, but all in all this seems like a real nice solution to me.

But one confusion - when I'm trying this locally I can clearly see the .fmdownload file, but I can't see any progress indicator on it in finder/dock or on my downloads folder. Do you see the actual progress locally? (and with progress I mean an animated progress bar, I can see the file size increasing)

[mdbraber]

Thanks Joel. Yes I can see the download progress bar. But I've found out that with sandboxed paths the progress bar doesn't always show up reliably. A suggestion to to try out, change line 118 of FileDownloadTask.m to this:
[[NSURL fileURLWithPath:self.downloadingPath] URLByResolvingSymlinksInPath], @"NSProgressFileURLKey", - does that fix things for you?

It would be good to get some more people (including ourselves) to test it as I imagine there might be some edge cases, e.g. a stalled download where for some reason the download doesn't progress can't be fixed by removing the fmdownload file, becuase FileDownloadManager will still think it's downloading as part of the sessions variable. Maybe there should be some other / additional logic to manage the current downloads?

[joelekstrom]

Hmm, no that still doesn't help for me. I will investigate a bit more when I get the time. I also think we can use the NSURLSessionDownloadTask to slim it down a bit, still just creating a tempt file at target location. I will experiment a bit and see if I can get it to work!

[mdbraber]

I've considered using NSURLSessionDownloadTask as it would be the likely designated class for this. My consideration is that a temporary file is created in a cache location and the progress of the download (e.g. byte size of the file) can't be seen if we create an empty .fmdownload file. I opted for streaming of the output, rather than 0-byte temp file.

Btw, this is how it looks for me:

[mdbraber]

@joelekstrom here's a small POC I've created to test the progress indicator:

#import <Foundation/Foundation.h>

// clang progress.mm -framework Cocoa -o progress
// ./progress

int main (int argc, const char * argv[])
{    
    NSDictionary* userInfo = @{
        NSProgressFileOperationKindKey : NSProgressFileOperationKindReceiving,
        NSProgressFileURLKey : [NSURL fileURLWithPath:@"/Users/mdbraber/Downloads/Test.pdf"],
    };

    NSProgress *progress = [[NSProgress alloc] initWithParent:nil userInfo:userInfo];    
    progress.kind = NSProgressKindFile;
    progress.pausable = NO;
    progress.cancellable = YES;
    progress.totalUnitCount = 10;

    [progress publish];

    for (NSUInteger completedUnits = 0; completedUnits < progress.totalUnitCount; ++completedUnits) {
        if ([progress isCancelled]) {
            NSLog(@"Cancelled!");
            break;
        }

        sleep(1);
        
        NSLog(@"Progress...");
        [progress setCompletedUnitCount:(completedUnits + 1)];
    }

    return 0;
}

[joelekstrom]

Interesting that I don't see it, must be something with my setup 🤔 will try a bit more. You can get progress from URLSessionDownloadTask using URLSessionDownloadDelegate: https://developer.apple.com/documentation/foundation/urlsessiondownloaddelegate/1409408-urlsession

I was thinking there's still a tmp .fmdownload file created where we post the progress, but then you don't need to handle the download buffering etc

[mdbraber]

Sorry @joelekstrom I accidentally posted the wrong POC - I've edited the comment above with the right POC and instructions (copied from https://gist.github.com/torarnv/0a4b6b4509bad7682b2dc0c561ec4437). Wondering if this POC would work for you. Main things I found that prevent the progress bar from showing are (1) fileURL paths with symlinked locations (2) forgetting to use publish

Regarding NSURLSessionDownloadTask I agree it means not having to handle download buffering, but the file size wouldn't be reflected in the Finder. I've taken a cue from Safari to see how they do it, and there the file is streamed directly to a download location (actually it's a bit different, the Safari .download file is actually a package file which holds the temporary file) Of course I think it's fine if you would prefer to switch to NSURLSessionDownloadTask but I think personally I prefer this approach.

[joelekstrom]

All right, sounds good let's keep this approach! 👍 I'll try to test the POC if I get some time layer today

[joelekstrom]

Interestingly, I do see the progress bar when running the POC, but not in Fastmate. Gonna have to try a bit more 🤔

[joelekstrom]

So strange, I got it to work once in Fastmate but then it stopped. I think something is acting up. Will debug a bit more tomorrow.

Btw, you can set up the progress object a bit more nicely this way:

    self.progress = [NSProgress progressWithTotalUnitCount:_totalBytes];
    self.progress.fileOperationKind = NSProgressFileOperationKindDownloading;
    self.progress.fileURL = [[NSURL fileURLWithPath:self.downloadingPath] URLByResolvingSymlinksInPath];
    self.progress.kind = NSProgressKindFile;
    self.progress.pausable = NO;
    self.progress.cancellable = YES;

[mdbraber]

Thanks for the update @joelekstrom, update the initialization. Any update on getting the progress bar to work on your side?

[joelekstrom]

Been a bit busy lately so haven't spent much time on it! However, if we can't get this to work perfectly that's fine. We could also ship it as a preference disabled by default to allow people to beta test it. Let people opt in to this new download experience and see how it goes. It might just be my computer acting up 😄

[mdbraber]

We could make the whole download functionality optional, I would than suggest switching it on by default qn making it possible to revert to the current behavior

[joelekstrom]

Yep, that sounds good to me! Would you like to add a preference for it? (Sorry for being so slow btw)

[mdbraber]

@joelekstrom yes, I'll add a preference for the download behavior in general, so people can toggle it on/off. No problem about the response time, I can relate :-) Would be nice to ship a version in the coming days / weeks, especially to get some feedback from people what can be improved.

[joelekstrom]

I agree! Would also be nice to ship the other patches you’ve made

[joelekstrom]

Hi @mdbraber! I feel like it's time we get this out. I rebased the branch added some minor changes including a setting to fall back to legacy downloads. Any thoughts on those? Otherwise I'll merge it and ship a new update soon. Thanks again for all the work on this!

[mdbraber]

@joelekstrom sorry for the delayed reply I was away for a bit. Thanks for merging this and adding a setting to revert to the legacy behavior. I think only one small detail to check now with users is if we can get the progress bar to show. It still shows on my install, but I've heard from at least one other user it (also, like with you) doesn't show up. Any ideas how we could get some user feedback on that?

[joelekstrom]

@mdbraber no problem! It does show on my new computer so it may have been something with my setup on the previous one