Skip to content

Commit

Permalink
Remove file URL error, add support for downloads (#3342)
Browse files Browse the repository at this point in the history
* Remove file URL error, add support for downloads.

* Add documentation note about file downloads.
  • Loading branch information
jshier committed Oct 25, 2020
1 parent 097e1f0 commit bcbc769
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 20 deletions.
13 changes: 11 additions & 2 deletions Source/Request.swift
Expand Up @@ -1459,8 +1459,11 @@ public class DownloadRequest: Request {

/// A closure executed once a `DownloadRequest` has successfully completed in order to determine where to move the
/// temporary file written to during the download process. The closure takes two arguments: the temporary file URL
/// and the URL response, and returns a two arguments: the file URL where the temporary file should be moved and
/// and the `HTTPURLResponse`, and returns two values: the file URL where the temporary file should be moved and
/// the options defining how the file should be moved.
///
/// - Note: Downloads from a local `file://` `URL`s do not use the `Destination` closure, as those downloads do not
/// return an `HTTPURLResponse`. Instead the file is merely moved within the temporary directory.
public typealias Destination = (_ temporaryURL: URL,
_ response: HTTPURLResponse) -> (destinationURL: URL, options: Options)

Expand Down Expand Up @@ -1489,10 +1492,16 @@ public class DownloadRequest: Request {
/// with this destination must be additionally moved if they should survive the system reclamation of temporary
/// space.
static let defaultDestination: Destination = { url, _ in
(defaultDestinationURL(url), [])
}

/// Default `URL` creation closure. Creates a `URL` in the temporary directory with `Alamofire_` prepended to the
/// provided file name.
static let defaultDestinationURL: (URL) -> URL = { url in
let filename = "Alamofire_\(url.lastPathComponent)"
let destination = url.deletingLastPathComponent().appendingPathComponent(filename)

return (destination, [])
return destination
}

// MARK: Downloadable
Expand Down
10 changes: 6 additions & 4 deletions Source/SessionDelegate.swift
Expand Up @@ -298,12 +298,14 @@ extension SessionDelegate: URLSessionDownloadDelegate {
return
}

guard let response = request.response else {
fatalError("URLSessionDownloadTask finished downloading with no response.")
let (destination, options): (URL, DownloadRequest.Options)
if let response = request.response {
(destination, options) = request.destination(location, response)
} else {
// If there's no response this is likely a local file download, so generate the temporary URL directly.
(destination, options) = (DownloadRequest.defaultDestinationURL(location), [])
}

let (destination, options) = (request.destination)(location, response)

eventMonitor?.request(request, didCreateDestinationURL: destination)

do {
Expand Down
5 changes: 0 additions & 5 deletions Source/URLRequest+Alamofire.swift
Expand Up @@ -32,11 +32,6 @@ extension URLRequest {
}

public func validate() throws {
if let url = url, url.isFileURL {
// This should become another urlRequestValidationFailed error in Alamofire 6.
throw AFError.invalidURL(url: url)
}

if method == .get, let bodyData = httpBody {
throw AFError.urlRequestValidationFailed(reason: .bodyDataInGETRequest(bodyData))
}
Expand Down
18 changes: 9 additions & 9 deletions Tests/RequestTests.swift
Expand Up @@ -1184,12 +1184,12 @@ final class RequestLifetimeTests: BaseTestCase {

// MARK: -

class RequestInvalidURLTestCase: BaseTestCase {
#if !SWIFT_PACKAGE
#if !SWIFT_PACKAGE
final class RequestInvalidURLTestCase: BaseTestCase {
func testThatDataRequestWithFileURLThrowsError() {
// Given
let fileURL = url(forResource: "valid_data", withExtension: "json")
let expectation = self.expectation(description: "Request should fail with invalid URL error.")
let expectation = self.expectation(description: "Request should succeed.")
var response: DataResponse<Data?, AFError>?

// When
Expand All @@ -1202,13 +1202,13 @@ class RequestInvalidURLTestCase: BaseTestCase {
waitForExpectations(timeout: timeout)

// Then
XCTAssertEqual(response?.error?.isInvalidURLError, true)
XCTAssertEqual(response?.result.isSuccess, true)
}

func testThatDownloadRequestWithFileURLThrowsError() {
// Given
let fileURL = url(forResource: "valid_data", withExtension: "json")
let expectation = self.expectation(description: "Request should fail with invalid URL error.")
let expectation = self.expectation(description: "Request should succeed.")
var response: DownloadResponse<URL?, AFError>?

// When
Expand All @@ -1221,13 +1221,13 @@ class RequestInvalidURLTestCase: BaseTestCase {
waitForExpectations(timeout: timeout)

// Then
XCTAssertEqual(response?.error?.isInvalidURLError, true)
XCTAssertEqual(response?.result.isSuccess, true)
}

func testThatDataStreamRequestWithFileURLThrowsError() {
// Given
let fileURL = url(forResource: "valid_data", withExtension: "json")
let expectation = self.expectation(description: "Request should fail with invalid URL error.")
let expectation = self.expectation(description: "Request should succeed.")
var response: DataStreamRequest.Completion?

// When
Expand All @@ -1242,7 +1242,7 @@ class RequestInvalidURLTestCase: BaseTestCase {
waitForExpectations(timeout: timeout)

// Then
XCTAssertEqual(response?.error?.isInvalidURLError, true)
XCTAssertNil(response?.response)
}
#endif
}
#endif

0 comments on commit bcbc769

Please sign in to comment.