Skip to content

Conversation

@sujaygarlanka
Copy link
Contributor

@sujaygarlanka sujaygarlanka commented Apr 23, 2020

Goals ⚽

Implementation Details 🚧

  • A BoxDownloadTask returned for every download request. This task contains the URLSessionTask that is associated with that request. BoxDownloadTask has a cancel method on it that can be called. When called, it cancels the URLSessionTask associated with, resulting in a cancelled request.
  • A BoxUploadTask returned for every upload request. BoxUploadTask has a cancel method on it that can be called.

Testing Details 🔍

  • Added unit tests

sujaygarlanka and others added 7 commits April 23, 2020 20:50
Co-Authored-By: James Lawton <jlawton@box.com>
Co-Authored-By: James Lawton <jlawton@box.com>
Co-Authored-By: James Lawton <jlawton@box.com>
Co-Authored-By: James Lawton <jlawton@box.com>
Co-Authored-By: James Lawton <jlawton@box.com>
Co-Authored-By: James Lawton <jlawton@box.com>
Co-Authored-By: James Lawton <jlawton@box.com>
jlawton
jlawton previously approved these changes Apr 24, 2020
@sujaygarlanka sujaygarlanka changed the title Add ability to cancel downloads Add ability to cancel uploads and downloads Apr 29, 2020
jlawton
jlawton previously approved these changes May 15, 2020
Copy link
Collaborator

@jlawton jlawton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it works to me, and seems like a reasonable evolution of the code.

I think that Combine (which I know we can't use) has a slightly cleaner solution to this sort of thing, but it hinges on the fact that Publishers separate initialization and subscription, and that they are chained, rather than using callbacks. I think moving in that direction would be too large of a change.

This wouldn't be totally trivial to implement. Just to document the idea in case you're interested in an alternative, it would look something like:

class NetworkTask<T> {
    /// Run a task
    func subscribe(_ completion: @escaping Callback<T>) -> Cancellable
    /// Chain tasks
    func then<U>(_ nextTask: (T) -> NetworkTask<U>) -> NetworkTask<U>
}

func preflightCheck() -> NetworkTask<Void>
func upload() -> NetworkTask<File>

func uploadWithPreflightCheck() -> NetworkTask<File> {
    return preflightCheck().then { upload() }
}

func uploadWithPreflightCheck(, _ completion: @escaping Callback<File>) -> Cancellable {
    return uploadWithPreflightCheck().subscribe(completion)
}

PJSimon
PJSimon previously approved these changes May 15, 2020
@sujaygarlanka sujaygarlanka dismissed stale reviews from PJSimon and jlawton via 92c8a8a May 15, 2020 20:30
@sujaygarlanka sujaygarlanka merged commit c2bf1f7 into master May 15, 2020
@sujaygarlanka sujaygarlanka deleted the cancel-requests branch May 15, 2020 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

How to cancel a file download session

3 participants