Skip to content

Make URLSession and URLSessionTask more source-compatible with Darwin #2587

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 16, 2020
Merged

Make URLSession and URLSessionTask more source-compatible with Darwin #2587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 53 additions & 60 deletions Foundation/URLSession/URLSession.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ open class URLSession : NSObject {
}

open private(set) var delegateQueue: OperationQueue
open var delegate: URLSessionDelegate?
open private(set) var delegate: URLSessionDelegate?
Copy link
Contributor Author

@broadwaylamb broadwaylamb Dec 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

URLSession.delegate is get-only in Darwin Foundation.

Therefore, we should be able to override it with a get-only property. We can't do this with swift-corelibs-foundation now.

open private(set) var configuration: URLSessionConfiguration

/*
Expand Down Expand Up @@ -370,8 +370,13 @@ open class URLSession : NSObject {
}
}

@available(*, unavailable, renamed: "getTasksWithCompletionHandler(_:)")
open func getTasksWithCompletionHandler(completionHandler: @escaping ([URLSessionDataTask], [URLSessionUploadTask], [URLSessionDownloadTask]) -> Void) {
getTasksWithCompletionHandler(completionHandler)
}

/* invokes completionHandler with outstanding data, upload and download tasks. */
open func getTasksWithCompletionHandler(completionHandler: @escaping ([URLSessionDataTask], [URLSessionUploadTask], [URLSessionDownloadTask]) -> Void) {
open func getTasksWithCompletionHandler(_ completionHandler: @escaping ([URLSessionDataTask], [URLSessionUploadTask], [URLSessionDownloadTask]) -> Void) {
Comment on lines +373 to +379
Copy link
Contributor Author

@broadwaylamb broadwaylamb Dec 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Darwin Foundation doesn't use an argument label.

Marked the old version as unavailable instead of deprecated, because in the latter case we have ambiguity if this method is used with trailing closure syntax.

workQueue.async {
self.delegateQueue.addOperation {
var dataTasks = [URLSessionDataTask]()
Expand Down Expand Up @@ -419,6 +424,22 @@ open class URLSession : NSObject {
open func dataTask(with url: URL) -> URLSessionDataTask {
return dataTask(with: _Request(url), behaviour: .callDelegate)
}

/*
* data task convenience methods. These methods create tasks that
* bypass the normal delegate calls for response and data delivery,
* and provide a simple cancelable asynchronous interface to receiving
* data. Errors will be returned in the NSURLErrorDomain,
* see <Foundation/NSURLError.h>. The delegate, if any, will still be
* called for authentication challenges.
*/
open func dataTask(with request: URLRequest, completionHandler: @escaping (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask {
return dataTask(with: _Request(request), behaviour: .dataCompletionHandler(completionHandler))
}

open func dataTask(with url: URL, completionHandler: @escaping (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask {
return dataTask(with: _Request(url), behaviour: .dataCompletionHandler(completionHandler))
}
Comment on lines +427 to +442
Copy link
Contributor Author

@broadwaylamb broadwaylamb Dec 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Moved these from an extension to the class itself. With Darwin Foundation we're able to override these methods in subclasses. Same with downloadTask and uploadTask methods.

Copy link
Contributor

Choose a reason for hiding this comment

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

SE-0117 strikes again.

I think you can mark each method in the extension with open, and that should reduce the size of this PR and keep changes easier to see.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They were already open, it’s just that we can’t override methods declared in extensions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I didn't double checked my proposal.

I just tested in a small Xcode (Mac) project that reproduces the problem, and I get this error "Overriding non-@objc declarations from extensions is not supported". I tried adding a simple @objc in front of the extension in the Swift framework, and it seems to fix the problem.

Do you think that might be a better solution? I don't mind moving the methods into the main class code, but if someone wrote them as an extension for organization, I would prefer to keep them that one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’m afraid @objc won’t work on non-Darwin platforms.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's true. What a strange limitation of extensions. I wonder why open there doesn't generate a warning, if it cannot be overriden anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK this has to do with method dispatch. We can't retroactively add methods to class vtable, unless we use some completely different dispatching mechanism, like message passing in Objective-C. That's why @objc works — it relies on Objective-C runtime.


/* Creates an upload task with the given request. The body of the request will be created from the file referenced by fileURL */
open func uploadTask(with request: URLRequest, fromFile fileURL: URL) -> URLSessionUploadTask {
Expand All @@ -437,6 +458,18 @@ open class URLSession : NSObject {
let r = URLSession._Request(request)
return uploadTask(with: r, body: nil, behaviour: .callDelegate)
}

/*
* upload convenience method.
*/
open func uploadTask(with request: URLRequest, fromFile fileURL: URL, completionHandler: @escaping (Data?, URLResponse?, Error?) -> Void) -> URLSessionUploadTask {
let fileData = try! Data(contentsOf: fileURL)
return uploadTask(with: request, from: fileData, completionHandler: completionHandler)
}

open func uploadTask(with request: URLRequest, from bodyData: Data?, completionHandler: @escaping (Data?, URLResponse?, Error?) -> Void) -> URLSessionUploadTask {
return uploadTask(with: _Request(request), body: .data(createDispatchData(bodyData!)), behaviour: .dataCompletionHandler(completionHandler))
}

/* Creates a download task with the given request. */
open func downloadTask(with request: URLRequest) -> URLSessionDownloadTask {
Expand All @@ -453,6 +486,24 @@ open class URLSession : NSObject {
open func downloadTask(withResumeData resumeData: Data) -> URLSessionDownloadTask {
return invalidDownloadTask(behavior: .callDelegate)
}

/*
* download task convenience methods. When a download successfully
* completes, the URL will point to a file that must be read or
* copied during the invocation of the completion routine. The file
* will be removed automatically.
*/
open func downloadTask(with request: URLRequest, completionHandler: @escaping (URL?, URLResponse?, Error?) -> Void) -> URLSessionDownloadTask {
return downloadTask(with: _Request(request), behavior: .downloadCompletionHandler(completionHandler))
}

open func downloadTask(with url: URL, completionHandler: @escaping (URL?, URLResponse?, Error?) -> Void) -> URLSessionDownloadTask {
return downloadTask(with: _Request(url), behavior: .downloadCompletionHandler(completionHandler))
}

open func downloadTask(withResumeData resumeData: Data, completionHandler: @escaping (URL?, URLResponse?, Error?) -> Void) -> URLSessionDownloadTask {
return invalidDownloadTask(behavior: .downloadCompletionHandler(completionHandler))
}

/* Creates a bidirectional stream task to a given host and port.
*/
Expand Down Expand Up @@ -556,64 +607,6 @@ fileprivate extension URLSession {
}
}


/*
* URLSession convenience routines deliver results to
* a completion handler block. These convenience routines
* are not available to URLSessions that are configured
* as background sessions.
*
* Task objects are always created in a suspended state and
* must be sent the -resume message before they execute.
*/
extension URLSession {
/*
* data task convenience methods. These methods create tasks that
* bypass the normal delegate calls for response and data delivery,
* and provide a simple cancelable asynchronous interface to receiving
* data. Errors will be returned in the NSURLErrorDomain,
* see <Foundation/NSURLError.h>. The delegate, if any, will still be
* called for authentication challenges.
*/
open func dataTask(with request: URLRequest, completionHandler: @escaping (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask {
return dataTask(with: _Request(request), behaviour: .dataCompletionHandler(completionHandler))
}

open func dataTask(with url: URL, completionHandler: @escaping (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask {
return dataTask(with: _Request(url), behaviour: .dataCompletionHandler(completionHandler))
}

/*
* upload convenience method.
*/
open func uploadTask(with request: URLRequest, fromFile fileURL: URL, completionHandler: @escaping (Data?, URLResponse?, Error?) -> Void) -> URLSessionUploadTask {
let fileData = try! Data(contentsOf: fileURL)
return uploadTask(with: request, from: fileData, completionHandler: completionHandler)
}

open func uploadTask(with request: URLRequest, from bodyData: Data?, completionHandler: @escaping (Data?, URLResponse?, Error?) -> Void) -> URLSessionUploadTask {
return uploadTask(with: _Request(request), body: .data(createDispatchData(bodyData!)), behaviour: .dataCompletionHandler(completionHandler))
}

/*
* download task convenience methods. When a download successfully
* completes, the URL will point to a file that must be read or
* copied during the invocation of the completion routine. The file
* will be removed automatically.
*/
open func downloadTask(with request: URLRequest, completionHandler: @escaping (URL?, URLResponse?, Error?) -> Void) -> URLSessionDownloadTask {
return downloadTask(with: _Request(request), behavior: .downloadCompletionHandler(completionHandler))
}

open func downloadTask(with url: URL, completionHandler: @escaping (URL?, URLResponse?, Error?) -> Void) -> URLSessionDownloadTask {
return downloadTask(with: _Request(url), behavior: .downloadCompletionHandler(completionHandler))
}

open func downloadTask(withResumeData resumeData: Data, completionHandler: @escaping (URL?, URLResponse?, Error?) -> Void) -> URLSessionDownloadTask {
return invalidDownloadTask(behavior: .downloadCompletionHandler(completionHandler))
}
}

internal extension URLSession {
/// The kind of callback / delegate behaviour of a task.
///
Expand Down
2 changes: 1 addition & 1 deletion Foundation/URLSession/URLSessionTask.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ open class URLSessionTask : NSObject, NSCopying {
/*
* The current state of the task within the session.
*/
open var state: URLSessionTask.State {
open fileprivate(set) var state: URLSessionTask.State {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

URLSessionTask.state is get-only in Darwin Foundation.

get {
return self.syncQ.sync { self._state }
}
Expand Down