diff --git a/AlamofireImage.podspec b/AlamofireImage.podspec index c0de8999..e3cdc4d8 100644 --- a/AlamofireImage.podspec +++ b/AlamofireImage.podspec @@ -18,5 +18,5 @@ Pod::Spec.new do |s| s.swift_version = '5.0' - s.dependency 'Alamofire', '~> 5.0.0-beta.6' + s.dependency 'Alamofire', '~> 5.0.0-beta.7' end diff --git a/AlamofireImage.xcodeproj/project.pbxproj b/AlamofireImage.xcodeproj/project.pbxproj index b072286d..c90aa0bd 100644 --- a/AlamofireImage.xcodeproj/project.pbxproj +++ b/AlamofireImage.xcodeproj/project.pbxproj @@ -496,6 +496,9 @@ 4C86C1121DA0B3400032ECC3 /* UIImageViewTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4C62D30F1B96C1500011B036 /* UIImageViewTests.swift */; }; 4C86C1131DA0B3440032ECC3 /* UIImage+AlamofireImageTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4C0893EB1B936A7A005125D9 /* UIImage+AlamofireImageTests.swift */; }; 4C96A4781AAE9488008AE0B6 /* ImageDownloader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4C96A4771AAE9488008AE0B6 /* ImageDownloader.swift */; }; + 4CBE8FB72316008300782A2E /* ImageDownloaderStressTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4CBE8FB62316008300782A2E /* ImageDownloaderStressTests.swift */; }; + 4CBE8FB82316008300782A2E /* ImageDownloaderStressTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4CBE8FB62316008300782A2E /* ImageDownloaderStressTests.swift */; }; + 4CBE8FB92316008300782A2E /* ImageDownloaderStressTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4CBE8FB62316008300782A2E /* ImageDownloaderStressTests.swift */; }; 4CD5BCEA1D7F9D1E0055E232 /* AFIError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4CD5BCE91D7F9D1E0055E232 /* AFIError.swift */; }; 4CD5BCEB1D7F9D1E0055E232 /* AFIError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4CD5BCE91D7F9D1E0055E232 /* AFIError.swift */; }; 4CD5BCEC1D7F9D1E0055E232 /* AFIError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4CD5BCE91D7F9D1E0055E232 /* AFIError.swift */; }; @@ -745,6 +748,7 @@ 4C9043821AABBFC5001B4E60 /* AlamofireImage iOS Tests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = "AlamofireImage iOS Tests.xctest"; sourceTree = BUILT_PRODUCTS_DIR; }; 4C96A4771AAE9488008AE0B6 /* ImageDownloader.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ImageDownloader.swift; sourceTree = ""; }; 4CB2B2F31C0270C500B442EA /* Info-tvOS.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = "Info-tvOS.plist"; sourceTree = ""; }; + 4CBE8FB62316008300782A2E /* ImageDownloaderStressTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageDownloaderStressTests.swift; sourceTree = ""; }; 4CD5BCE91D7F9D1E0055E232 /* AFIError.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AFIError.swift; sourceTree = ""; }; 4CD5BCF41D7FBE380055E232 /* AFError+AlamofireImageTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "AFError+AlamofireImageTests.swift"; sourceTree = ""; }; 4CE611321AABC24E00D35044 /* AlamofireImage.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = AlamofireImage.framework; sourceTree = BUILT_PRODUCTS_DIR; }; @@ -1236,6 +1240,14 @@ name = Products; sourceTree = ""; }; + 4CBE8FB52316006100782A2E /* Stress Tests */ = { + isa = PBXGroup; + children = ( + 4CBE8FB62316008300782A2E /* ImageDownloaderStressTests.swift */, + ); + name = "Stress Tests"; + sourceTree = ""; + }; 4CD5BCF21D7FBE110055E232 /* Tests */ = { isa = PBXGroup; children = ( @@ -1248,6 +1260,7 @@ 4C0893F41B937404005125D9 /* UIImageTests.swift */, 4C62D30F1B96C1500011B036 /* UIImageViewTests.swift */, 4C0893EA1B936A4D005125D9 /* Extensions */, + 4CBE8FB52316006100782A2E /* Stress Tests */, ); name = Tests; sourceTree = ""; @@ -2064,6 +2077,7 @@ 4C16B3AE1BA93ADB00A66EF0 /* UIImage+AlamofireImageTests.swift in Sources */, 4C16B3AA1BA93ADB00A66EF0 /* ImageFilterTests.swift in Sources */, 4C16B3A71BA93ADB00A66EF0 /* BaseTestCase.swift in Sources */, + 4CBE8FB92316008300782A2E /* ImageDownloaderStressTests.swift in Sources */, 4C16B3AD1BA93ADB00A66EF0 /* UIImageViewTests.swift in Sources */, 4C16B3A91BA93ADB00A66EF0 /* ImageDownloaderTests.swift in Sources */, 4C16B3AC1BA93ADB00A66EF0 /* UIImageTests.swift in Sources */, @@ -2114,6 +2128,7 @@ 4C5872CA1B93DEAC00407E58 /* ImageFilterTests.swift in Sources */, 4CEBB5401B93C622001391DE /* ImageCacheTests.swift in Sources */, 4C0893EC1B936A7A005125D9 /* UIImage+AlamofireImageTests.swift in Sources */, + 4CBE8FB72316008300782A2E /* ImageDownloaderStressTests.swift in Sources */, 4C1624851AABE8E600A0385D /* BaseTestCase.swift in Sources */, 4C0893F51B937404005125D9 /* UIImageTests.swift in Sources */, 4C5874861B93F81800407E58 /* ImageDownloaderTests.swift in Sources */, @@ -2148,6 +2163,7 @@ 4C1624861AABE8E600A0385D /* BaseTestCase.swift in Sources */, 4CEBB5411B93C622001391DE /* ImageCacheTests.swift in Sources */, 4C86C1121DA0B3400032ECC3 /* UIImageViewTests.swift in Sources */, + 4CBE8FB82316008300782A2E /* ImageDownloaderStressTests.swift in Sources */, 4C5874871B93F81800407E58 /* ImageDownloaderTests.swift in Sources */, 4C86C1131DA0B3440032ECC3 /* UIImage+AlamofireImageTests.swift in Sources */, 4C0897EC1B93BC0D005125D9 /* RequestTests.swift in Sources */, diff --git a/Carthage/Checkouts/Alamofire b/Carthage/Checkouts/Alamofire index 163db025..a6241748 160000 --- a/Carthage/Checkouts/Alamofire +++ b/Carthage/Checkouts/Alamofire @@ -1 +1 @@ -Subproject commit 163db02519e6f89dcbc57126bd5b17f729b81988 +Subproject commit a6241748b5c5f38ca53fa8286788d2f90fa9c666 diff --git a/Source/ImageDownloader.swift b/Source/ImageDownloader.swift index b5fcdd49..e164fd01 100644 --- a/Source/ImageDownloader.swift +++ b/Source/ImageDownloader.swift @@ -322,19 +322,18 @@ open class ImageDownloader { request.response( queue: self.responseQueue, responseSerializer: imageResponseSerializer, - completionHandler: { [weak self] response in - guard let strongSelf = self, let request = response.request else { return } - + completionHandler: { response in defer { - strongSelf.safelyDecrementActiveRequestCount() - strongSelf.safelyStartNextRequestIfNecessary() + self.safelyDecrementActiveRequestCount() + self.safelyStartNextRequestIfNecessary() } // Early out if the request has changed out from under us - let handler = strongSelf.safelyFetchResponseHandler(withURLIdentifier: urlID) - guard handler?.handlerID == handlerID else { return } - - guard let responseHandler = strongSelf.safelyRemoveResponseHandler(withURLIdentifier: urlID) else { + guard + let handler = self.safelyFetchResponseHandler(withURLIdentifier: urlID), + handler.handlerID == handlerID, + let responseHandler = self.safelyRemoveResponseHandler(withURLIdentifier: urlID) + else { return } @@ -356,7 +355,9 @@ open class ImageDownloader { filteredImage = image } - strongSelf.imageCache?.add(filteredImage, for: request, withIdentifier: filter?.identifier) + if let request = response.request { + self.imageCache?.add(filteredImage, for: request, withIdentifier: filter?.identifier) + } DispatchQueue.main.async { let response = DataResponse( @@ -476,7 +477,7 @@ open class ImageDownloader { DispatchQueue.main.async { operation.completion?(response) } } - if responseHandler.operations.isEmpty && requestReceipt.request.task?.state == .suspended { + if responseHandler.operations.isEmpty { requestReceipt.request.cancel() self.responseHandlers.removeValue(forKey: urlID) } @@ -509,22 +510,15 @@ open class ImageDownloader { synchronizationQueue.sync { guard self.isActiveRequestCountBelowMaximumLimit() else { return } - let states: Set = [.initialized, .suspended] + guard let request = self.dequeue() else { return } - while !self.queuedRequests.isEmpty { - if let request = self.dequeue(), states.contains(request.state) { - self.start(request) - break - } - } + self.start(request) } } func safelyDecrementActiveRequestCount() { - self.synchronizationQueue.sync { - if self.activeRequestCount > 0 { - self.activeRequestCount -= 1 - } + synchronizationQueue.sync { + self.activeRequestCount -= 1 } } diff --git a/Tests/ImageDownloaderStressTests.swift b/Tests/ImageDownloaderStressTests.swift new file mode 100644 index 00000000..033ba42d --- /dev/null +++ b/Tests/ImageDownloaderStressTests.swift @@ -0,0 +1,199 @@ +// +// ImageDownloaderStressTests.swift +// +// Copyright (c) 2019 Alamofire Software Foundation (http://alamofire.org/) +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +// + +@testable import Alamofire +@testable import AlamofireImage +import Foundation +import XCTest + +class ImageDownloaderStressTestCase: BaseTestCase { + let imageCount = 10 + var kittenCache: Set = [] + + // MARK: - Setup and Teardown + + override func setUp() { + super.setUp() + kittenCache.removeAll() + } + + // MARK: - Tests - Common Use Cases + + func testThatItCanDownloadManyImagesInParallel() { + // Given + let imageRequests = (1...imageCount).map { _ in randomKittenURLRequest() } + let imageDownloader = ImageDownloader(configuration: .ephemeral) + + let expect = expectation(description: "all requests should complete") + expect.expectedFulfillmentCount = imageRequests.count + + var receipts: [RequestReceipt] = [] + var responses: [DataResponse] = [] + + // When + for imageRequest in imageRequests { + let receipt = imageDownloader.download(imageRequest) { response in + responses.append(response) + expect.fulfill() + } + + receipt.flatMap { receipts.append($0) } + } + + waitForExpectations(timeout: 10, handler: nil) + + // Then + XCTAssertEqual(receipts.count, imageCount) + XCTAssertEqual(responses.count, imageCount) + responses.forEach { XCTAssertTrue($0.result.isSuccess) } + } + + func testThatItCanDownloadManyImagesInParallelWhileCancellingRequests() { + // Given + let cancelledImageCount = 4 + let imageRequests = (1...imageCount).map { _ in randomKittenURLRequest() } + let imageDownloader = ImageDownloader(configuration: .ephemeral) + + let expect = expectation(description: "all requests should complete") + expect.expectedFulfillmentCount = imageRequests.count + + var receipts: [RequestReceipt] = [] + var responses: [DataResponse] = [] + + // When + for imageRequest in imageRequests { + let receipt = imageDownloader.download(imageRequest) { response in + responses.append(response) + expect.fulfill() + } + + receipt.flatMap { receipts.append($0) } + } + + receipts.suffix(cancelledImageCount).forEach { imageDownloader.cancelRequest(with: $0) } + + waitForExpectations(timeout: 10, handler: nil) + + // Then + XCTAssertEqual(receipts.count, imageCount) + XCTAssertEqual(responses.count, imageCount) + + let successCount = responses.reduce(0) { count, response in response.result.isSuccess ? count + 1 : count } + let failureCount = responses.reduce(0) { count, response in response.result.isFailure ? count + 1 : count } + + XCTAssertEqual(successCount, imageCount - cancelledImageCount) + XCTAssertEqual(failureCount, cancelledImageCount) + } + + // MARK: - Tests - Uncommon Use Cases (External Abuse) + + func testThatItCanDownloadManyImagesInParallelWhileResumingRequestsExternally() { + // Given + let imageRequests = (1...imageCount).map { _ in randomKittenURLRequest() } + let imageDownloader = ImageDownloader(configuration: .ephemeral) + + let expect = expectation(description: "all requests should complete") + expect.expectedFulfillmentCount = imageRequests.count + + var receipts: [RequestReceipt] = [] + var responses: [DataResponse] = [] + + // When + for imageRequest in imageRequests { + let receipt = imageDownloader.download(imageRequest) { response in + responses.append(response) + expect.fulfill() + } + + receipt.flatMap { receipts.append($0) } + } + + receipts.suffix(4).forEach { $0.request.resume() } + + waitForExpectations(timeout: 10, handler: nil) + + // Then + XCTAssertEqual(receipts.count, imageCount) + XCTAssertEqual(responses.count, imageCount) + responses.forEach { XCTAssertTrue($0.result.isSuccess) } + } + + func testThatItCanDownloadManyImagesInParallelWhileCancellingRequestsExternally() { + // Given + let cancelledImageCount = 4 + let imageRequests = (1...imageCount).map { _ in randomKittenURLRequest() } + let imageDownloader = ImageDownloader(configuration: .ephemeral) + + let expect = expectation(description: "all requests should complete") + expect.expectedFulfillmentCount = imageRequests.count + + var receipts: [RequestReceipt] = [] + var responses: [DataResponse] = [] + + // When + for imageRequest in imageRequests { + let receipt = imageDownloader.download(imageRequest) { response in + responses.append(response) + expect.fulfill() + } + + receipt.flatMap { receipts.append($0) } + } + + receipts.suffix(cancelledImageCount).forEach { $0.request.cancel() } + + waitForExpectations(timeout: 10, handler: nil) + + // Then + XCTAssertEqual(receipts.count, imageCount) + XCTAssertEqual(responses.count, imageCount) + + let successCount = responses.reduce(0) { count, response in response.result.isSuccess ? count + 1 : count } + let failureCount = responses.reduce(0) { count, response in response.result.isFailure ? count + 1 : count } + + XCTAssertEqual(successCount, imageCount - cancelledImageCount) + XCTAssertEqual(failureCount, cancelledImageCount) + } + + private func randomKittenURLRequest() -> URLRequest { + let urlString = uniqueKittenURLString() + let url = URL(string: urlString)! + + return URLRequest(url: url) + } + + private func uniqueKittenURLString() -> String { + let width = Int.random(in: 100...400) + let height = Int.random(in: 100...400) + + let urlString = "https://placekitten.com/\(width)/\(height)" + + if kittenCache.contains(urlString) { + return uniqueKittenURLString() + } else { + kittenCache.insert(urlString) + return urlString + } + } +} diff --git a/Tests/ImageDownloaderTests.swift b/Tests/ImageDownloaderTests.swift index d0953b85..ac4d680d 100644 --- a/Tests/ImageDownloaderTests.swift +++ b/Tests/ImageDownloaderTests.swift @@ -187,16 +187,16 @@ class ImageDownloaderTestCase: BaseTestCase { let urlRequest2 = try! URLRequest(url: "https://httpbin.org/image/png", method: .get) let expectation = self.expectation(description: "both downloads should succeed") - var completedDownloads = 0 + expectation.expectedFulfillmentCount = 2 + var completedDownloads = 0 var results: [AFResult] = [] // When downloader.download([urlRequest1, urlRequest2], filter: nil) { closureResponse in results.append(closureResponse.result) - completedDownloads += 1 - if completedDownloads == 2 { expectation.fulfill() } + expectation.fulfill() } let activeRequestCount = downloader.activeRequestCount @@ -488,7 +488,7 @@ class ImageDownloaderTestCase: BaseTestCase { let downloader = ImageDownloader() let urlRequest = try! URLRequest(url: "https://httpbin.org/image/jpeg", method: .get) - let expectation = self.expectation(description: "download request should succeed") + let expectation = self.expectation(description: "download request should cancel") var response: DataResponse?