Merge pull request #91 from essentialdevelopercom/swift-concurrency

Migrate Image Loading composition to async/await

Author: mapostolakis

EssentialApp/EssentialApp.xcodeproj/project.pbxproj

@@ -26,6 +26,7 @@
 		0832C9D0238D2811002314C9 /* SceneDelegateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0832C9CF238D2811002314C9 /* SceneDelegateTests.swift */; };
 		0835BF6D24850F9800A793D2 /* CombineHelpers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0835BF6C24850F9800A793D2 /* CombineHelpers.swift */; };
 		08367CD82486FB51009CD536 /* UIView+TestHelpers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 08367CD72486FB51009CD536 /* UIView+TestHelpers.swift */; };
+		084BE5342EB38EC5006886E9 /* LoaderSpy.swift in Sources */ = {isa = PBXBuildFile; fileRef = 084BE5332EB38EC5006886E9 /* LoaderSpy.swift */; };
 		0851CDAC239AB13100C19B1D /* HTTPClientStub.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0851CDAB239AB13100C19B1D /* HTTPClientStub.swift */; };
 		088B441925309AA300D75AAD /* CommentsUIIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 088B441825309AA300D75AAD /* CommentsUIIntegrationTests.swift */; };
 		088B441C25309B6E00D75AAD /* CommentsUIComposer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 088B441B25309B6E00D75AAD /* CommentsUIComposer.swift */; };
@@ -84,6 +85,7 @@
 		0832C9CF238D2811002314C9 /* SceneDelegateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SceneDelegateTests.swift; sourceTree = "<group>"; };
 		0835BF6C24850F9800A793D2 /* CombineHelpers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CombineHelpers.swift; sourceTree = "<group>"; };
 		08367CD72486FB51009CD536 /* UIView+TestHelpers.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "UIView+TestHelpers.swift"; sourceTree = "<group>"; };
+		084BE5332EB38EC5006886E9 /* LoaderSpy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoaderSpy.swift; sourceTree = "<group>"; };
 		0851CDAB239AB13100C19B1D /* HTTPClientStub.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HTTPClientStub.swift; sourceTree = "<group>"; };
 		088B441825309AA300D75AAD /* CommentsUIIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CommentsUIIntegrationTests.swift; sourceTree = "<group>"; };
 		088B441B25309B6E00D75AAD /* CommentsUIComposer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CommentsUIComposer.swift; sourceTree = "<group>"; };
@@ -137,6 +139,7 @@
 				082C00032359E46C008927D3 /* XCTestCase+MemoryLeakTracking.swift */,
 				082C00052359E4C6008927D3 /* SharedTestHelpers.swift */,
 				0851CDAB239AB13100C19B1D /* HTTPClientStub.swift */,
+				084BE5332EB38EC5006886E9 /* LoaderSpy.swift */,
 			);
 			path = Helpers;
 			sourceTree = "<group>";
@@ -325,6 +328,7 @@
 				082C00062359E4C6008927D3 /* SharedTestHelpers.swift in Sources */,
 				088B441925309AA300D75AAD /* CommentsUIIntegrationTests.swift in Sources */,
 				08073B57238D2E1000A75DC6 /* UIControl+TestHelpers.swift in Sources */,
+				084BE5342EB38EC5006886E9 /* LoaderSpy.swift in Sources */,
 				08073B56238D2E1000A75DC6 /* FeedUIIntegrationTests+Assertions.swift in Sources */,
 				0832C9D0238D2811002314C9 /* SceneDelegateTests.swift in Sources */,
 				08073B5B238D2E1000A75DC6 /* ListViewController+TestHelpers.swift in Sources */,

EssentialApp/EssentialApp/FeedUIComposer.swift

@@ -15,7 +15,7 @@ public final class FeedUIComposer {
 
 	public static func feedComposedWith(
 		feedLoader: @MainActor @escaping () -> AnyPublisher<Paginated<FeedImage>, Error>,
-		imageLoader: @MainActor @escaping (URL) -> FeedImageDataLoader.Publisher,
+		imageLoader: @MainActor @escaping (URL) async throws -> Data,
 		selection: @MainActor @escaping (FeedImage) -> Void = { _ in }
 	) -> ListViewController {
 		let presentationAdapter = FeedPresentationAdapter(loader: feedLoader)

EssentialApp/EssentialApp/FeedViewAdapter.swift

@@ -9,14 +9,14 @@ import EssentialFeediOS
 @MainActor
 final class FeedViewAdapter: ResourceView {
 	private weak var controller: ListViewController?
-	private let imageLoader: (URL) -> FeedImageDataLoader.Publisher
+	private let imageLoader: (URL) async throws -> Data
 	private let selection: (FeedImage) -> Void
 	private let currentFeed: [FeedImage: CellController]
 
-	private typealias ImageDataPresentationAdapter = LoadResourcePresentationAdapter<Data, WeakRefVirtualProxy<FeedImageCellController>>
+	private typealias ImageDataPresentationAdapter = AsyncLoadResourcePresentationAdapter<Data, WeakRefVirtualProxy<FeedImageCellController>>
 	private typealias LoadMorePresentationAdapter = LoadResourcePresentationAdapter<Paginated<FeedImage>, FeedViewAdapter>
 
-	init(currentFeed: [FeedImage: CellController] = [:], controller: ListViewController, imageLoader: @escaping (URL) -> FeedImageDataLoader.Publisher, selection: @escaping (FeedImage) -> Void) {
+	init(currentFeed: [FeedImage: CellController] = [:], controller: ListViewController, imageLoader: @escaping (URL) async throws -> Data, selection: @escaping (FeedImage) -> Void) {
 		self.currentFeed = currentFeed
 		self.controller = controller
 		self.imageLoader = imageLoader
@@ -33,7 +33,7 @@ final class FeedViewAdapter: ResourceView {
 			}
 
 			let adapter = ImageDataPresentationAdapter(loader: { [imageLoader] in
-				imageLoader(model.url)
+				try await imageLoader(model.url)
 			})
 
 			let view = FeedImageCellController(

EssentialApp/EssentialApp/LoadResourcePresentationAdapter.swift

@@ -6,6 +6,58 @@ import Combine
 import EssentialFeed
 import EssentialFeediOS
 
+@MainActor
+final class AsyncLoadResourcePresentationAdapter<Resource, View: ResourceView> {
+	private let loader: () async throws -> Resource
+	private var cancellable: Task<Void, Never>?
+	private var isLoading = false
+	
+	var presenter: LoadResourcePresenter<Resource, View>?
+	
+	init(loader: @escaping () async throws -> Resource) {
+		self.loader = loader
+	}
+	
+	func loadResource() {
+		guard !isLoading else { return }
+		
+		presenter?.didStartLoading()
+		isLoading = true
+		
+		cancellable = Task.immediate { @MainActor [weak self] in
+			defer { self?.isLoading = false }
+			
+			do {
+				if let resource = try await self?.loader() {
+					if Task.isCancelled { return }
+					
+					self?.presenter?.didFinishLoading(with: resource)
+				}
+			} catch {
+				if Task.isCancelled { return }
+				
+				self?.presenter?.didFinishLoading(with: error)
+			}
+		}
+	}
+	
+	deinit {
+		cancellable?.cancel()
+	}
+}
+
+extension AsyncLoadResourcePresentationAdapter: FeedImageCellControllerDelegate {
+	func didRequestImage() {
+		loadResource()
+	}
+	
+	func didCancelImageRequest() {
+		cancellable?.cancel()
+		cancellable = nil
+		isLoading = false
+	}
+}
+
 @MainActor
 final class LoadResourcePresentationAdapter<Resource, View: ResourceView> {
 	private let loader: () -> AnyPublisher<Resource, Error>

EssentialApp/EssentialApp/SceneDelegate.swift

@@ -28,7 +28,7 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate {
 
 	private lazy var logger = Logger(subsystem: "com.essentialdeveloper.EssentialAppCaseStudy", category: "main")
 
-	private lazy var store: FeedStore & FeedImageDataStore = {
+	private lazy var store: FeedStore & FeedImageDataStore & StoreScheduler & Sendable = {
 		do {
 			return try CoreDataFeedStore(
 				storeURL: NSPersistentContainer
@@ -50,10 +50,10 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate {
 	private lazy var navigationController = UINavigationController(
 		rootViewController: FeedUIComposer.feedComposedWith(
 			feedLoader: makeRemoteFeedLoaderWithLocalFallback,
-			imageLoader: makeLocalImageLoaderWithRemoteFallback,
+			imageLoader: loadLocalImageWithRemoteFallback,
 			selection: showComments))
 
-	convenience init(httpClient: HTTPClient, store: FeedStore & FeedImageDataStore) {
+	convenience init(httpClient: HTTPClient, store: FeedStore & FeedImageDataStore & StoreScheduler & Sendable) {
 		self.init()
 		self.httpClient = httpClient
 		self.store = store
@@ -137,20 +137,52 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate {
 		})
 	}
 
-	private func makeLocalImageLoaderWithRemoteFallback(url: URL) -> FeedImageDataLoader.Publisher {
-		let localImageLoader = LocalFeedImageDataLoader(store: store)
-		
-		return localImageLoader
-			.loadImageDataPublisher(from: url)
-			.fallback(to: { [httpClient, scheduler] in
-				httpClient
-					.getPublisher(url: url)
-					.tryMap(FeedImageDataMapper.map)
-					.receive(on: scheduler)
-					.caching(to: localImageLoader, using: url)
-					.eraseToAnyPublisher()
-			})
-			.subscribe(on: scheduler)
-			.eraseToAnyPublisher()
+	private func loadLocalImageWithRemoteFallback(url: URL) async throws -> Data {
+		do {
+			return try await loadLocalImage(url: url)
+		} catch {
+			return try await loadAndCacheRemoteImage(url: url)
+		}
+	}
+	
+	private func loadLocalImage(url: URL) async throws -> Data {
+		try await store.schedule { [store] in
+			let localImageLoader = LocalFeedImageDataLoader(store: store)
+			let imageData = try localImageLoader.loadImageData(from: url)
+			return imageData
+		}
+	}
+
+	private func loadAndCacheRemoteImage(url: URL) async throws -> Data {
+		let (data, response) = try await httpClient.get(from: url)
+		let imageData = try FeedImageDataMapper.map(data, from: response)
+		await store.schedule { [store] in
+			let localImageLoader = LocalFeedImageDataLoader(store: store)
+			try? localImageLoader.save(data, for: url)
+		}
+		return imageData
+	}
+}
+
+protocol StoreScheduler {
+	@MainActor
+	func schedule<T>(_ action: @escaping @Sendable () throws -> T) async rethrows -> T
+}
+
+extension CoreDataFeedStore: StoreScheduler {
+	@MainActor
+	func schedule<T>(_ action: @escaping @Sendable () throws -> T) async rethrows -> T {
+		if contextQueue == .main {
+			return try action()
+		} else {
+			return try await perform(action)
+		}
+	}
+}
+
+extension InMemoryFeedStore: StoreScheduler {
+	@MainActor
+	func schedule<T>(_ action: @escaping @Sendable () throws -> T) async rethrows -> T {
+		try action()
 	}
 }

EssentialApp/EssentialAppTests/FeedUIIntegrationTests.swift

@@ -263,7 +263,7 @@ class FeedUIIntegrationTests: XCTestCase {
 		XCTAssertEqual(loader.loadedImageURLs, [image0.url, image1.url], "Expected second image URL request once second view also becomes visible")
 	}
 
-	func test_feedImageView_cancelsImageLoadingWhenNotVisibleAnymore() {
+	func test_feedImageView_cancelsImageLoadingWhenNotVisibleAnymore() async throws {
 		let image0 = makeImage(url: URL(string: "http://url-0.com")!)
 		let image1 = makeImage(url: URL(string: "http://url-1.com")!)
 		let (sut, loader) = makeSUT()
@@ -273,9 +273,13 @@ class FeedUIIntegrationTests: XCTestCase {
 		XCTAssertEqual(loader.cancelledImageURLs, [], "Expected no cancelled image URL requests until image is not visible")
 
 		sut.simulateFeedImageViewNotVisible(at: 0)
+		let result0 = try await loader.imageResult(at: 0)
+		XCTAssertEqual(result0, .cancelled)
 		XCTAssertEqual(loader.cancelledImageURLs, [image0.url], "Expected one cancelled image URL request once first image is not visible anymore")
 
 		sut.simulateFeedImageViewNotVisible(at: 1)
+		let result1 = try await loader.imageResult(at: 1)
+		XCTAssertEqual(result1, .cancelled)
 		XCTAssertEqual(loader.cancelledImageURLs, [image0.url, image1.url], "Expected two cancelled image URL requests once second image is also not visible anymore")
 	}
 
@@ -420,7 +424,7 @@ class FeedUIIntegrationTests: XCTestCase {
 		XCTAssertEqual(loader.loadedImageURLs, [image0.url, image1.url], "Expected second image URL request once second image is near visible")
 	}
 
-	func test_feedImageView_cancelsImageURLPreloadingWhenNotNearVisibleAnymore() {
+	func test_feedImageView_cancelsImageURLPreloadingWhenNotNearVisibleAnymore() async throws {
 		let image0 = makeImage(url: URL(string: "http://url-0.com")!)
 		let image1 = makeImage(url: URL(string: "http://url-1.com")!)
 		let (sut, loader) = makeSUT()
@@ -430,9 +434,13 @@ class FeedUIIntegrationTests: XCTestCase {
 		XCTAssertEqual(loader.cancelledImageURLs, [], "Expected no cancelled image URL requests until image is not near visible")
 
 		sut.simulateFeedImageViewNotNearVisible(at: 0)
+		let result0 = try await loader.imageResult(at: 0)
+		XCTAssertEqual(result0, .cancelled)
 		XCTAssertEqual(loader.cancelledImageURLs, [image0.url], "Expected first cancelled image URL request once first image is not near visible anymore")
 
 		sut.simulateFeedImageViewNotNearVisible(at: 1)
+		let result1 = try await loader.imageResult(at: 1)
+		XCTAssertEqual(result1, .cancelled)
 		XCTAssertEqual(loader.cancelledImageURLs, [image0.url, image1.url], "Expected second cancelled image URL request once second image is not near visible anymore")
 	}
 
@@ -556,11 +564,16 @@ class FeedUIIntegrationTests: XCTestCase {
 		let loader = LoaderSpy()
 		let sut = FeedUIComposer.feedComposedWith(
 			feedLoader: loader.loadPublisher,
-			imageLoader: loader.loadImageDataPublisher,
+			imageLoader: loader.loadImageData,
 			selection: selection
 		)
 		trackForMemoryLeaks(loader, file: file, line: line)
 		trackForMemoryLeaks(sut, file: file, line: line)
+		
+		addTeardownBlock { [weak loader] in
+			try await loader?.cancelPendingRequests()
+		}
+		
 		return (sut, loader)
 	}
 

EssentialApp/EssentialAppTests/Helpers/FeedUIIntegrationTests+LoaderSpy.swift

@@ -65,29 +65,37 @@ extension FeedUIIntegrationTests {
 
 		// MARK: - FeedImageDataLoader
 
-		private var imageRequests = [(url: URL, publisher: PassthroughSubject<Data, Error>)]()
+		private var imageLoader = EssentialAppTests.LoaderSpy<URL, Data>()
 
 		var loadedImageURLs: [URL] {
-			return imageRequests.map { $0.url }
+			return imageLoader.requests.map { $0.param }
 		}
 
-		private(set) var cancelledImageURLs = [URL]()
+		var cancelledImageURLs: [URL] {
+			return imageLoader.requests.filter({ $0.result == .cancelled }).map { $0.param }
+		}
+		
+		private struct NoResponse: Error {}
+		private struct Timeout: Error {}
 
-		func loadImageDataPublisher(from url: URL) -> AnyPublisher<Data, Error> {
-			let publisher = PassthroughSubject<Data, Error>()
-			imageRequests.append((url, publisher))
-			return publisher.handleEvents(receiveCancel: { [weak self] in
-				self?.cancelledImageURLs.append(url)
-			}).eraseToAnyPublisher()
+		func loadImageData(from url: URL) async throws -> Data {
+			try await imageLoader.load(url)
 		}
 
 		func completeImageLoading(with imageData: Data = Data(), at index: Int = 0) {
-			imageRequests[index].publisher.send(imageData)
-			imageRequests[index].publisher.send(completion: .finished)
+			imageLoader.complete(with: imageData, at: index)
 		}
 
 		func completeImageLoadingWithError(at index: Int = 0) {
-			imageRequests[index].publisher.send(completion: .failure(anyNSError()))
+			imageLoader.fail(with: anyNSError(), at: index)
+		}
+		
+		func imageResult(at index: Int, timeout: TimeInterval = 1) async throws -> AsyncResult {
+			try await imageLoader.result(at: index, timeout: timeout)
+		}
+		
+		func cancelPendingRequests() async throws {
+			try await imageLoader.cancelPendingRequests()
 		}
 	}