From cacf9004205ace10c07562f91480fe80a3255dfe Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Thu, 27 Mar 2025 21:18:52 -0300 Subject: [PATCH 01/25] Deprecate async APIs in favor of new sync APIs --- .../Feed Cache/FeedImageDataStore.swift | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/EssentialFeed/EssentialFeed/Feed Cache/FeedImageDataStore.swift b/EssentialFeed/EssentialFeed/Feed Cache/FeedImageDataStore.swift index eb76cad..4c0ffb9 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/FeedImageDataStore.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/FeedImageDataStore.swift @@ -9,6 +9,41 @@ public protocol FeedImageDataStore { typealias RetrievalResult = Swift.Result typealias InsertionResult = Swift.Result + func insert(_ data: Data, for url: URL) throws + func retrieve(dataForURL url: URL) throws -> Data? + + @available(*, deprecated) func insert(_ data: Data, for url: URL, completion: @escaping (InsertionResult) -> Void) + + @available(*, deprecated) func retrieve(dataForURL url: URL, completion: @escaping (RetrievalResult) -> Void) } + +public extension FeedImageDataStore { + func insert(_ data: Data, for url: URL) throws { + let group = DispatchGroup() + group.enter() + var result: InsertionResult! + insert(data, for: url) { + result = $0 + group.leave() + } + group.wait() + return try result.get() + } + + func retrieve(dataForURL url: URL) throws -> Data? { + let group = DispatchGroup() + group.enter() + var result: RetrievalResult! + retrieve(dataForURL: url) { + result = $0 + group.leave() + } + group.wait() + return try result.get() + } + + func insert(_ data: Data, for url: URL, completion: @escaping (InsertionResult) -> Void) {} + func retrieve(dataForURL url: URL, completion: @escaping (RetrievalResult) -> Void) {} +} From d4a8f48a796cd716459702589e3f89d42da9d235 Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Thu, 27 Mar 2025 23:21:34 -0300 Subject: [PATCH 02/25] Save images synchronously --- .../Feed Cache/LocalFeedImageDataLoader.swift | 8 +++----- .../CacheFeedImageDataUseCaseTests.swift | 15 +-------------- .../Helpers/FeedImageDataStoreSpy.swift | 14 +++++++------- 3 files changed, 11 insertions(+), 26 deletions(-) diff --git a/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedImageDataLoader.swift b/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedImageDataLoader.swift index 631541a..8788af2 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedImageDataLoader.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedImageDataLoader.swift @@ -21,11 +21,9 @@ extension LocalFeedImageDataLoader: FeedImageDataCache { } public func save(_ data: Data, for url: URL, completion: @escaping (SaveResult) -> Void) { - store.insert(data, for: url) { [weak self] result in - guard self != nil else { return } - - completion(result.mapError { _ in SaveError.failed }) - } + completion(SaveResult { + try store.insert(data, for: url) + }.mapError { _ in SaveError.failed }) } } diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedImageDataUseCaseTests.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedImageDataUseCaseTests.swift index 00fe85e..b52361e 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedImageDataUseCaseTests.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedImageDataUseCaseTests.swift @@ -41,19 +41,6 @@ class CacheFeedImageDataUseCaseTests: XCTestCase { }) } - func test_saveImageDataFromURL_doesNotDeliverResultAfterSUTInstanceHasBeenDeallocated() { - let store = FeedImageDataStoreSpy() - var sut: LocalFeedImageDataLoader? = LocalFeedImageDataLoader(store: store) - - var received = [LocalFeedImageDataLoader.SaveResult]() - sut?.save(anyData(), for: anyURL()) { received.append($0) } - - sut = nil - store.completeInsertionSuccessfully() - - XCTAssertTrue(received.isEmpty, "Expected no received results after instance has been deallocated") - } - // MARK: - Helpers private func makeSUT(file: StaticString = #file, line: UInt = #line) -> (sut: LocalFeedImageDataLoader, store: FeedImageDataStoreSpy) { @@ -70,6 +57,7 @@ class CacheFeedImageDataUseCaseTests: XCTestCase { private func expect(_ sut: LocalFeedImageDataLoader, toCompleteWith expectedResult: LocalFeedImageDataLoader.SaveResult, when action: () -> Void, file: StaticString = #file, line: UInt = #line) { let exp = expectation(description: "Wait for load completion") + action() sut.save(anyData(), for: anyURL()) { receivedResult in switch (receivedResult, expectedResult) { @@ -87,7 +75,6 @@ class CacheFeedImageDataUseCaseTests: XCTestCase { exp.fulfill() } - action() wait(for: [exp], timeout: 1.0) } diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/Helpers/FeedImageDataStoreSpy.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/Helpers/FeedImageDataStoreSpy.swift index 9b9cb12..f7b3717 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/Helpers/FeedImageDataStoreSpy.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/Helpers/FeedImageDataStoreSpy.swift @@ -14,11 +14,11 @@ class FeedImageDataStoreSpy: FeedImageDataStore { private(set) var receivedMessages = [Message]() private var retrievalCompletions = [(FeedImageDataStore.RetrievalResult) -> Void]() - private var insertionCompletions = [(FeedImageDataStore.InsertionResult) -> Void]() + private var insertionResult: Result? - func insert(_ data: Data, for url: URL, completion: @escaping (FeedImageDataStore.InsertionResult) -> Void) { + func insert(_ data: Data, for url: URL) throws { receivedMessages.append(.insert(data: data, for: url)) - insertionCompletions.append(completion) + try insertionResult?.get() } func retrieve(dataForURL url: URL, completion: @escaping (FeedImageDataStore.RetrievalResult) -> Void) { @@ -34,11 +34,11 @@ class FeedImageDataStoreSpy: FeedImageDataStore { retrievalCompletions[index](.success(data)) } - func completeInsertion(with error: Error, at index: Int = 0) { - insertionCompletions[index](.failure(error)) + func completeInsertion(with error: Error) { + insertionResult = .failure(error) } - func completeInsertionSuccessfully(at index: Int = 0) { - insertionCompletions[index](.success(())) + func completeInsertionSuccessfully() { + insertionResult = .success(()) } } From 4ec2287d5cc6b712495178f99fee9a5f2e6df637 Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Thu, 27 Mar 2025 23:39:18 -0300 Subject: [PATCH 03/25] Load images synchronously --- .../Feed Cache/LocalFeedImageDataLoader.swift | 11 +++---- .../Helpers/FeedImageDataStoreSpy.swift | 14 ++++----- ...adFeedImageDataFromCacheUseCaseTests.swift | 30 +------------------ 3 files changed, 14 insertions(+), 41 deletions(-) diff --git a/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedImageDataLoader.swift b/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedImageDataLoader.swift index 8788af2..cec3c1c 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedImageDataLoader.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedImageDataLoader.swift @@ -57,15 +57,16 @@ extension LocalFeedImageDataLoader: FeedImageDataLoader { public func loadImageData(from url: URL, completion: @escaping (FeedImageDataLoader.Result) -> Void) -> FeedImageDataLoaderTask { let task = LoadImageDataTask(completion) - store.retrieve(dataForURL: url) { [weak self] result in - guard self != nil else { return } - - task.complete(with: result + + task.complete( + with: Swift.Result { + try store.retrieve(dataForURL: url) + } .mapError { _ in LoadError.failed } .flatMap { data in data.map { .success($0) } ?? .failure(LoadError.notFound) }) - } + return task } } diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/Helpers/FeedImageDataStoreSpy.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/Helpers/FeedImageDataStoreSpy.swift index f7b3717..0f91d29 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/Helpers/FeedImageDataStoreSpy.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/Helpers/FeedImageDataStoreSpy.swift @@ -13,7 +13,7 @@ class FeedImageDataStoreSpy: FeedImageDataStore { } private(set) var receivedMessages = [Message]() - private var retrievalCompletions = [(FeedImageDataStore.RetrievalResult) -> Void]() + private var retrievalResult: Result? private var insertionResult: Result? func insert(_ data: Data, for url: URL) throws { @@ -21,17 +21,17 @@ class FeedImageDataStoreSpy: FeedImageDataStore { try insertionResult?.get() } - func retrieve(dataForURL url: URL, completion: @escaping (FeedImageDataStore.RetrievalResult) -> Void) { + func retrieve(dataForURL url: URL) throws -> Data? { receivedMessages.append(.retrieve(dataFor: url)) - retrievalCompletions.append(completion) + return try retrievalResult?.get() } - func completeRetrieval(with error: Error, at index: Int = 0) { - retrievalCompletions[index](.failure(error)) + func completeRetrieval(with error: Error) { + retrievalResult = .failure(error) } - func completeRetrieval(with data: Data?, at index: Int = 0) { - retrievalCompletions[index](.success(data)) + func completeRetrieval(with data: Data?) { + retrievalResult = .success(data) } func completeInsertion(with error: Error) { diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/LoadFeedImageDataFromCacheUseCaseTests.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/LoadFeedImageDataFromCacheUseCaseTests.swift index 61733e0..834ac65 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/LoadFeedImageDataFromCacheUseCaseTests.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/LoadFeedImageDataFromCacheUseCaseTests.swift @@ -49,34 +49,6 @@ class LoadFeedImageDataFromCacheUseCaseTests: XCTestCase { }) } - func test_loadImageDataFromURL_doesNotDeliverResultAfterCancellingTask() { - let (sut, store) = makeSUT() - let foundData = anyData() - - var received = [FeedImageDataLoader.Result]() - let task = sut.loadImageData(from: anyURL()) { received.append($0) } - task.cancel() - - store.completeRetrieval(with: foundData) - store.completeRetrieval(with: .none) - store.completeRetrieval(with: anyNSError()) - - XCTAssertTrue(received.isEmpty, "Expected no received results after cancelling task") - } - - func test_loadImageDataFromURL_doesNotDeliverResultAfterSUTInstanceHasBeenDeallocated() { - let store = FeedImageDataStoreSpy() - var sut: LocalFeedImageDataLoader? = LocalFeedImageDataLoader(store: store) - - var received = [FeedImageDataLoader.Result]() - _ = sut?.loadImageData(from: anyURL()) { received.append($0) } - - sut = nil - store.completeRetrieval(with: anyData()) - - XCTAssertTrue(received.isEmpty, "Expected no received results after instance has been deallocated") - } - // MARK: - Helpers private func makeSUT(currentDate: @escaping () -> Date = Date.init, file: StaticString = #file, line: UInt = #line) -> (sut: LocalFeedImageDataLoader, store: FeedImageDataStoreSpy) { @@ -97,6 +69,7 @@ class LoadFeedImageDataFromCacheUseCaseTests: XCTestCase { private func expect(_ sut: LocalFeedImageDataLoader, toCompleteWith expectedResult: FeedImageDataLoader.Result, when action: () -> Void, file: StaticString = #file, line: UInt = #line) { let exp = expectation(description: "Wait for load completion") + action() _ = sut.loadImageData(from: anyURL()) { receivedResult in switch (receivedResult, expectedResult) { @@ -114,7 +87,6 @@ class LoadFeedImageDataFromCacheUseCaseTests: XCTestCase { exp.fulfill() } - action() wait(for: [exp], timeout: 1.0) } From abb106f7b382d09b653b07763ccc14b36aa5af73 Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Fri, 28 Mar 2025 00:07:33 -0300 Subject: [PATCH 04/25] Make FeedImageDataCache sync --- .../EssentialApp/CombineHelpers.swift | 2 +- .../Feed Cache/LocalFeedImageDataLoader.swift | 10 +++--- .../Feed Feature/FeedImageDataCache.swift | 4 +-- .../EssentialFeedCacheIntegrationTests.swift | 11 +++---- .../CacheFeedImageDataUseCaseTests.swift | 33 ++++++++----------- 5 files changed, 25 insertions(+), 35 deletions(-) diff --git a/EssentialApp/EssentialApp/CombineHelpers.swift b/EssentialApp/EssentialApp/CombineHelpers.swift index ff872f1..ca04aa0 100644 --- a/EssentialApp/EssentialApp/CombineHelpers.swift +++ b/EssentialApp/EssentialApp/CombineHelpers.swift @@ -76,7 +76,7 @@ extension Publisher where Output == Data { private extension FeedImageDataCache { func saveIgnoringResult(_ data: Data, for url: URL) { - save(data, for: url) { _ in } + try? save(data, for: url) } } diff --git a/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedImageDataLoader.swift b/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedImageDataLoader.swift index cec3c1c..036da89 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedImageDataLoader.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedImageDataLoader.swift @@ -14,16 +14,16 @@ public final class LocalFeedImageDataLoader { } extension LocalFeedImageDataLoader: FeedImageDataCache { - public typealias SaveResult = FeedImageDataCache.Result - public enum SaveError: Error { case failed } - public func save(_ data: Data, for url: URL, completion: @escaping (SaveResult) -> Void) { - completion(SaveResult { + public func save(_ data: Data, for url: URL) throws { + do { try store.insert(data, for: url) - }.mapError { _ in SaveError.failed }) + } catch { + throw SaveError.failed + } } } diff --git a/EssentialFeed/EssentialFeed/Feed Feature/FeedImageDataCache.swift b/EssentialFeed/EssentialFeed/Feed Feature/FeedImageDataCache.swift index 4935331..9e27406 100644 --- a/EssentialFeed/EssentialFeed/Feed Feature/FeedImageDataCache.swift +++ b/EssentialFeed/EssentialFeed/Feed Feature/FeedImageDataCache.swift @@ -6,7 +6,5 @@ import Foundation public protocol FeedImageDataCache { - typealias Result = Swift.Result - - func save(_ data: Data, for url: URL, completion: @escaping (Result) -> Void) + func save(_ data: Data, for url: URL) throws } diff --git a/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift b/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift index 59a749b..25ee5f6 100644 --- a/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift +++ b/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift @@ -166,14 +166,11 @@ class EssentialFeedCacheIntegrationTests: XCTestCase { } private func save(_ data: Data, for url: URL, with loader: LocalFeedImageDataLoader, file: StaticString = #file, line: UInt = #line) { - let saveExp = expectation(description: "Wait for save completion") - loader.save(data, for: url) { result in - if case let Result.failure(error) = result { - XCTFail("Expected to save image data successfully, got error: \(error)", file: file, line: line) - } - saveExp.fulfill() + do { + try loader.save(data, for: url) + } catch { + XCTFail("Expected to save image data successfully, got error: \(error)", file: file, line: line) } - wait(for: [saveExp], timeout: 1.0) } private func expect(_ sut: LocalFeedImageDataLoader, toLoad expectedData: Data, for url: URL, file: StaticString = #file, line: UInt = #line) { diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedImageDataUseCaseTests.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedImageDataUseCaseTests.swift index b52361e..0574377 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedImageDataUseCaseTests.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedImageDataUseCaseTests.swift @@ -19,7 +19,7 @@ class CacheFeedImageDataUseCaseTests: XCTestCase { let url = anyURL() let data = anyData() - sut.save(data, for: url) { _ in } + try? sut.save(data, for: url) XCTAssertEqual(store.receivedMessages, [.insert(data: data, for: url)]) } @@ -51,31 +51,26 @@ class CacheFeedImageDataUseCaseTests: XCTestCase { return (sut, store) } - private func failed() -> LocalFeedImageDataLoader.SaveResult { + private func failed() -> Result { return .failure(LocalFeedImageDataLoader.SaveError.failed) } - private func expect(_ sut: LocalFeedImageDataLoader, toCompleteWith expectedResult: LocalFeedImageDataLoader.SaveResult, when action: () -> Void, file: StaticString = #file, line: UInt = #line) { - let exp = expectation(description: "Wait for load completion") + private func expect(_ sut: LocalFeedImageDataLoader, toCompleteWith expectedResult: Result, when action: () -> Void, file: StaticString = #file, line: UInt = #line) { action() - sut.save(anyData(), for: anyURL()) { receivedResult in - switch (receivedResult, expectedResult) { - case (.success, .success): - break - - case (.failure(let receivedError as LocalFeedImageDataLoader.SaveError), - .failure(let expectedError as LocalFeedImageDataLoader.SaveError)): - XCTAssertEqual(receivedError, expectedError, file: file, line: line) - - default: - XCTFail("Expected result \(expectedResult), got \(receivedResult) instead", file: file, line: line) - } + let receivedResult = Result { try sut.save(anyData(), for: anyURL()) } + + switch (receivedResult, expectedResult) { + case (.success, .success): + break + + case (.failure(let receivedError as LocalFeedImageDataLoader.SaveError), + .failure(let expectedError as LocalFeedImageDataLoader.SaveError)): + XCTAssertEqual(receivedError, expectedError, file: file, line: line) - exp.fulfill() + default: + XCTFail("Expected result \(expectedResult), got \(receivedResult) instead", file: file, line: line) } - - wait(for: [exp], timeout: 1.0) } } From 23e1c297ab4e63a2f822096c76bc6b6621f4ac9e Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Fri, 28 Mar 2025 01:00:49 -0300 Subject: [PATCH 05/25] Make FeedImageDataLoader sync --- .../EssentialApp/CombineHelpers.swift | 7 ++-- .../FeedUIIntegrationTests+LoaderSpy.swift | 26 +++++------- .../Feed Cache/LocalFeedImageDataLoader.swift | 41 ++++--------------- .../Feed Feature/FeedImageDataLoader.swift | 8 +--- .../EssentialFeedAPIEndToEndTests.swift | 4 +- .../EssentialFeedCacheIntegrationTests.swift | 17 +++----- ...adFeedImageDataFromCacheUseCaseTests.swift | 35 +++++++--------- 7 files changed, 45 insertions(+), 93 deletions(-) diff --git a/EssentialApp/EssentialApp/CombineHelpers.swift b/EssentialApp/EssentialApp/CombineHelpers.swift index ca04aa0..056627f 100644 --- a/EssentialApp/EssentialApp/CombineHelpers.swift +++ b/EssentialApp/EssentialApp/CombineHelpers.swift @@ -54,14 +54,13 @@ public extension FeedImageDataLoader { typealias Publisher = AnyPublisher func loadImageDataPublisher(from url: URL) -> Publisher { - var task: FeedImageDataLoaderTask? - return Deferred { Future { completion in - task = self.loadImageData(from: url, completion: completion) + completion(Result { + try self.loadImageData(from: url) + }) } } - .handleEvents(receiveCancel: { task?.cancel() }) .eraseToAnyPublisher() } } diff --git a/EssentialApp/EssentialAppTests/Helpers/FeedUIIntegrationTests+LoaderSpy.swift b/EssentialApp/EssentialAppTests/Helpers/FeedUIIntegrationTests+LoaderSpy.swift index 41de2a1..100c4e7 100644 --- a/EssentialApp/EssentialAppTests/Helpers/FeedUIIntegrationTests+LoaderSpy.swift +++ b/EssentialApp/EssentialAppTests/Helpers/FeedUIIntegrationTests+LoaderSpy.swift @@ -10,7 +10,7 @@ import Combine extension FeedUIIntegrationTests { - class LoaderSpy: FeedImageDataLoader { + class LoaderSpy { // MARK: - FeedLoader @@ -65,14 +65,7 @@ extension FeedUIIntegrationTests { // MARK: - FeedImageDataLoader - private struct TaskSpy: FeedImageDataLoaderTask { - let cancelCallback: () -> Void - func cancel() { - cancelCallback() - } - } - - private var imageRequests = [(url: URL, completion: (FeedImageDataLoader.Result) -> Void)]() + private var imageRequests = [(url: URL, publisher: PassthroughSubject)]() var loadedImageURLs: [URL] { return imageRequests.map { $0.url } @@ -80,18 +73,21 @@ extension FeedUIIntegrationTests { private(set) var cancelledImageURLs = [URL]() - func loadImageData(from url: URL, completion: @escaping (FeedImageDataLoader.Result) -> Void) -> FeedImageDataLoaderTask { - imageRequests.append((url, completion)) - return TaskSpy { [weak self] in self?.cancelledImageURLs.append(url) } + func loadImageDataPublisher(from url: URL) -> AnyPublisher { + let publisher = PassthroughSubject() + imageRequests.append((url, publisher)) + return publisher.handleEvents(receiveCancel: { [weak self] in + self?.cancelledImageURLs.append(url) + }).eraseToAnyPublisher() } func completeImageLoading(with imageData: Data = Data(), at index: Int = 0) { - imageRequests[index].completion(.success(imageData)) + imageRequests[index].publisher.send(imageData) + imageRequests[index].publisher.send(completion: .finished) } func completeImageLoadingWithError(at index: Int = 0) { - let error = NSError(domain: "an error", code: 404) - imageRequests[index].completion(.failure(error)) + imageRequests[index].publisher.send(completion: .failure(anyNSError())) } } diff --git a/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedImageDataLoader.swift b/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedImageDataLoader.swift index 036da89..6d978c9 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedImageDataLoader.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedImageDataLoader.swift @@ -28,45 +28,20 @@ extension LocalFeedImageDataLoader: FeedImageDataCache { } extension LocalFeedImageDataLoader: FeedImageDataLoader { - public typealias LoadResult = FeedImageDataLoader.Result - public enum LoadError: Swift.Error { case failed case notFound } - private final class LoadImageDataTask: FeedImageDataLoaderTask { - private var completion: ((FeedImageDataLoader.Result) -> Void)? - - init(_ completion: @escaping (FeedImageDataLoader.Result) -> Void) { - self.completion = completion - } - - func complete(with result: FeedImageDataLoader.Result) { - completion?(result) - } - - func cancel() { - preventFurtherCompletions() - } - - private func preventFurtherCompletions() { - completion = nil - } - } - - public func loadImageData(from url: URL, completion: @escaping (FeedImageDataLoader.Result) -> Void) -> FeedImageDataLoaderTask { - let task = LoadImageDataTask(completion) - - task.complete( - with: Swift.Result { - try store.retrieve(dataForURL: url) + public func loadImageData(from url: URL) throws -> Data { + do { + if let imageData = try store.retrieve(dataForURL: url) { + return imageData } - .mapError { _ in LoadError.failed } - .flatMap { data in - data.map { .success($0) } ?? .failure(LoadError.notFound) - }) + } catch { + throw LoadError.failed + } - return task + throw LoadError.notFound } } diff --git a/EssentialFeed/EssentialFeed/Feed Feature/FeedImageDataLoader.swift b/EssentialFeed/EssentialFeed/Feed Feature/FeedImageDataLoader.swift index ed53bfe..e2b375e 100644 --- a/EssentialFeed/EssentialFeed/Feed Feature/FeedImageDataLoader.swift +++ b/EssentialFeed/EssentialFeed/Feed Feature/FeedImageDataLoader.swift @@ -5,12 +5,6 @@ import Foundation -public protocol FeedImageDataLoaderTask { - func cancel() -} - public protocol FeedImageDataLoader { - typealias Result = Swift.Result - - func loadImageData(from url: URL, completion: @escaping (Result) -> Void) -> FeedImageDataLoaderTask + func loadImageData(from url: URL) throws -> Data } diff --git a/EssentialFeed/EssentialFeedAPIEndToEndTests/EssentialFeedAPIEndToEndTests.swift b/EssentialFeed/EssentialFeedAPIEndToEndTests/EssentialFeedAPIEndToEndTests.swift index e269914..dc02121 100644 --- a/EssentialFeed/EssentialFeedAPIEndToEndTests/EssentialFeedAPIEndToEndTests.swift +++ b/EssentialFeed/EssentialFeedAPIEndToEndTests/EssentialFeedAPIEndToEndTests.swift @@ -64,12 +64,12 @@ final class EssentialFeedAPIEndToEndTests: XCTestCase { return receivedResult } - private func getFeedImageDataResult(file: StaticString = #file, line: UInt = #line) -> FeedImageDataLoader.Result? { + private func getFeedImageDataResult(file: StaticString = #file, line: UInt = #line) -> Result? { let client = ephemeralClient() let url = feedTestServerURL.appendingPathComponent("73A7F70C-75DA-4C2E-B5A3-EED40DC53AA6/image") let exp = expectation(description: "Wait for load completion") - var receivedResult: FeedImageDataLoader.Result? + var receivedResult: Result? client.get(from: url) { result in receivedResult = result.flatMap { (data, response) in do { diff --git a/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift b/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift index 25ee5f6..77cd8c5 100644 --- a/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift +++ b/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift @@ -174,19 +174,12 @@ class EssentialFeedCacheIntegrationTests: XCTestCase { } private func expect(_ sut: LocalFeedImageDataLoader, toLoad expectedData: Data, for url: URL, file: StaticString = #file, line: UInt = #line) { - let exp = expectation(description: "Wait for load completion") - _ = sut.loadImageData(from: url) { result in - switch result { - case let .success(loadedData): - XCTAssertEqual(loadedData, expectedData, file: file, line: line) - - case let .failure(error): - XCTFail("Expected successful image data result, got \(error) instead", file: file, line: line) - } - - exp.fulfill() + do { + let loadedData = try sut.loadImageData(from: url) + XCTAssertEqual(loadedData, expectedData, file: file, line: line) + } catch { + XCTFail("Expected successful image data result, got \(error) instead", file: file, line: line) } - wait(for: [exp], timeout: 1.0) } private func setupEmptyStoreState() { diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/LoadFeedImageDataFromCacheUseCaseTests.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/LoadFeedImageDataFromCacheUseCaseTests.swift index 834ac65..6cf4aea 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/LoadFeedImageDataFromCacheUseCaseTests.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/LoadFeedImageDataFromCacheUseCaseTests.swift @@ -18,7 +18,7 @@ class LoadFeedImageDataFromCacheUseCaseTests: XCTestCase { let (sut, store) = makeSUT() let url = anyURL() - _ = sut.loadImageData(from: url) { _ in } + _ = try? sut.loadImageData(from: url) XCTAssertEqual(store.receivedMessages, [.retrieve(dataFor: url)]) } @@ -59,35 +59,30 @@ class LoadFeedImageDataFromCacheUseCaseTests: XCTestCase { return (sut, store) } - private func failed() -> FeedImageDataLoader.Result { + private func failed() -> Result { return .failure(LocalFeedImageDataLoader.LoadError.failed) } - private func notFound() -> FeedImageDataLoader.Result { + private func notFound() -> Result { return .failure(LocalFeedImageDataLoader.LoadError.notFound) } - private func expect(_ sut: LocalFeedImageDataLoader, toCompleteWith expectedResult: FeedImageDataLoader.Result, when action: () -> Void, file: StaticString = #file, line: UInt = #line) { - let exp = expectation(description: "Wait for load completion") + private func expect(_ sut: LocalFeedImageDataLoader, toCompleteWith expectedResult: Result, when action: () -> Void, file: StaticString = #file, line: UInt = #line) { action() - _ = sut.loadImageData(from: anyURL()) { receivedResult in - switch (receivedResult, expectedResult) { - case let (.success(receivedData), .success(expectedData)): - XCTAssertEqual(receivedData, expectedData, file: file, line: line) - - case (.failure(let receivedError as LocalFeedImageDataLoader.LoadError), - .failure(let expectedError as LocalFeedImageDataLoader.LoadError)): - XCTAssertEqual(receivedError, expectedError, file: file, line: line) - - default: - XCTFail("Expected result \(expectedResult), got \(receivedResult) instead", file: file, line: line) - } + let receivedResult = Result { try sut.loadImageData(from: anyURL()) } + + switch (receivedResult, expectedResult) { + case let (.success(receivedData), .success(expectedData)): + XCTAssertEqual(receivedData, expectedData, file: file, line: line) + + case (.failure(let receivedError as LocalFeedImageDataLoader.LoadError), + .failure(let expectedError as LocalFeedImageDataLoader.LoadError)): + XCTAssertEqual(receivedError, expectedError, file: file, line: line) - exp.fulfill() + default: + XCTFail("Expected result \(expectedResult), got \(receivedResult) instead", file: file, line: line) } - - wait(for: [exp], timeout: 1.0) } } From 37c8cbbfbda8572a84bbbb6652a46c2b864064c5 Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Fri, 28 Mar 2025 01:04:45 -0300 Subject: [PATCH 06/25] Rename method to clarify intent --- .../CoreData/CoreDataFeedStore+FeedImageDataStore.swift | 4 ++-- .../CoreData/CoreDataFeedStore+FeedStore.swift | 6 +++--- .../Infrastructure/CoreData/CoreDataFeedStore.swift | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedImageDataStore.swift b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedImageDataStore.swift index 1844930..a73e1ea 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedImageDataStore.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedImageDataStore.swift @@ -8,7 +8,7 @@ import Foundation extension CoreDataFeedStore: FeedImageDataStore { public func insert(_ data: Data, for url: URL, completion: @escaping (FeedImageDataStore.InsertionResult) -> Void) { - perform { context in + performAsync { context in completion(Result { try ManagedFeedImage.first(with: url, in: context) .map { $0.data = data } @@ -18,7 +18,7 @@ extension CoreDataFeedStore: FeedImageDataStore { } public func retrieve(dataForURL url: URL, completion: @escaping (FeedImageDataStore.RetrievalResult) -> Void) { - perform { context in + performAsync { context in completion(Result { try ManagedFeedImage.data(with: url, in: context) }) diff --git a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedStore.swift b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedStore.swift index a05127f..52311fb 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedStore.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedStore.swift @@ -8,7 +8,7 @@ import CoreData extension CoreDataFeedStore: FeedStore { public func retrieve(completion: @escaping RetrievalCompletion) { - perform { context in + performAsync { context in completion(Result { try ManagedCache.find(in: context).map { CachedFeed(feed: $0.localFeed, timestamp: $0.timestamp) @@ -18,7 +18,7 @@ extension CoreDataFeedStore: FeedStore { } public func insert(_ feed: [LocalFeedImage], timestamp: Date, completion: @escaping InsertionCompletion) { - perform { context in + performAsync { context in completion(Result { let managedCache = try ManagedCache.newUniqueInstance(in: context) managedCache.timestamp = timestamp @@ -29,7 +29,7 @@ extension CoreDataFeedStore: FeedStore { } public func deleteCachedFeed(completion: @escaping DeletionCompletion) { - perform { context in + performAsync { context in completion(Result { try ManagedCache.deleteCache(in: context) }) diff --git a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore.swift b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore.swift index a85f280..aa60558 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore.swift @@ -30,7 +30,7 @@ public final class CoreDataFeedStore { } } - func perform(_ action: @escaping (NSManagedObjectContext) -> Void) { + func performAsync(_ action: @escaping (NSManagedObjectContext) -> Void) { let context = self.context context.perform { action(context) } } From 32c51451592eb5733000c5cded6e7a3c758bea0f Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Fri, 28 Mar 2025 07:42:34 -0300 Subject: [PATCH 07/25] Make CoreData FeedImageDataStore implementation sync --- ...CoreDataFeedStore+FeedImageDataStore.swift | 16 ++++---- .../CoreData/CoreDataFeedStore.swift | 7 ++++ .../CoreDataFeedImageDataStoreTests.swift | 39 ++++++++----------- 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedImageDataStore.swift b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedImageDataStore.swift index a73e1ea..032b52b 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedImageDataStore.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedImageDataStore.swift @@ -7,21 +7,21 @@ import Foundation extension CoreDataFeedStore: FeedImageDataStore { - public func insert(_ data: Data, for url: URL, completion: @escaping (FeedImageDataStore.InsertionResult) -> Void) { - performAsync { context in - completion(Result { + public func insert(_ data: Data, for url: URL) throws { + try performSync { context in + Result { try ManagedFeedImage.first(with: url, in: context) .map { $0.data = data } .map(context.save) - }) + } } } - public func retrieve(dataForURL url: URL, completion: @escaping (FeedImageDataStore.RetrievalResult) -> Void) { - performAsync { context in - completion(Result { + public func retrieve(dataForURL url: URL) throws -> Data? { + try performSync { context in + Result { try ManagedFeedImage.data(with: url, in: context) - }) + } } } diff --git a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore.swift b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore.swift index aa60558..e999b42 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore.swift @@ -35,6 +35,13 @@ public final class CoreDataFeedStore { context.perform { action(context) } } + func performSync(_ action: (NSManagedObjectContext) -> Result) throws -> R { + let context = self.context + var result: Result! + context.performAndWait { result = action(context) } + return try result.get() + } + private func cleanUpReferencesToPersistentStores() { context.performAndWait { let coordinator = self.container.persistentStoreCoordinator diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/CoreDataFeedImageDataStoreTests.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/CoreDataFeedImageDataStoreTests.swift index c8d87ef..bd324ea 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/CoreDataFeedImageDataStoreTests.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/CoreDataFeedImageDataStoreTests.swift @@ -84,38 +84,33 @@ class CoreDataFeedImageDataStoreTests: XCTestCase { } private func expect(_ sut: CoreDataFeedStore, toCompleteRetrievalWith expectedResult: FeedImageDataStore.RetrievalResult, for url: URL, file: StaticString = #file, line: UInt = #line) { - let exp = expectation(description: "Wait for load completion") - sut.retrieve(dataForURL: url) { receivedResult in - switch (receivedResult, expectedResult) { - case let (.success( receivedData), .success(expectedData)): - XCTAssertEqual(receivedData, expectedData, file: file, line: line) - - default: - XCTFail("Expected \(expectedResult), got \(receivedResult) instead", file: file, line: line) - } - exp.fulfill() + let receivedResult = Result { try sut.retrieve(dataForURL: url) } + + switch (receivedResult, expectedResult) { + case let (.success( receivedData), .success(expectedData)): + XCTAssertEqual(receivedData, expectedData, file: file, line: line) + + default: + XCTFail("Expected \(expectedResult), got \(receivedResult) instead", file: file, line: line) } - wait(for: [exp], timeout: 1.0) } private func insert(_ data: Data, for url: URL, into sut: CoreDataFeedStore, file: StaticString = #file, line: UInt = #line) { let exp = expectation(description: "Wait for cache insertion") let image = localImage(url: url) sut.insert([image], timestamp: Date()) { result in - switch result { - case let .failure(error): + if case let .failure(error) = result { XCTFail("Failed to save \(image) with error \(error)", file: file, line: line) - exp.fulfill() - - case .success: - sut.insert(data, for: url) { result in - if case let Result.failure(error) = result { - XCTFail("Failed to insert \(data) with error \(error)", file: file, line: line) - } - exp.fulfill() - } } + + exp.fulfill() } wait(for: [exp], timeout: 1.0) + + do { + try sut.insert(data, for: url) + } catch { + XCTFail("Failed to insert \(data) with error \(error)", file: file, line: line) + } } } From 5babace270c716fac0b8d834891ef1cf88d59957 Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Fri, 28 Mar 2025 07:48:27 -0300 Subject: [PATCH 08/25] Refactor NullStore class into extensions --- EssentialApp/EssentialApp/NullStore.swift | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/EssentialApp/EssentialApp/NullStore.swift b/EssentialApp/EssentialApp/NullStore.swift index 2db00b1..60eec30 100644 --- a/EssentialApp/EssentialApp/NullStore.swift +++ b/EssentialApp/EssentialApp/NullStore.swift @@ -6,7 +6,9 @@ import Foundation import EssentialFeed -class NullStore: FeedStore & FeedImageDataStore { +class NullStore {} + +extension NullStore: FeedStore { func deleteCachedFeed(completion: @escaping DeletionCompletion) { completion(.success(())) } @@ -18,7 +20,9 @@ class NullStore: FeedStore & FeedImageDataStore { func retrieve(completion: @escaping RetrievalCompletion) { completion(.success(.none)) } - +} + +extension NullStore: FeedImageDataStore { func insert(_ data: Data, for url: URL, completion: @escaping (InsertionResult) -> Void) { completion(.success(())) } From 5897c3869d8f6bb45249000f74bc48f6be661d4e Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Fri, 28 Mar 2025 07:50:14 -0300 Subject: [PATCH 09/25] Make NullStore FeedImageDataStore implementation sync --- EssentialApp/EssentialApp/NullStore.swift | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/EssentialApp/EssentialApp/NullStore.swift b/EssentialApp/EssentialApp/NullStore.swift index 60eec30..1ee5f05 100644 --- a/EssentialApp/EssentialApp/NullStore.swift +++ b/EssentialApp/EssentialApp/NullStore.swift @@ -23,11 +23,7 @@ extension NullStore: FeedStore { } extension NullStore: FeedImageDataStore { - func insert(_ data: Data, for url: URL, completion: @escaping (InsertionResult) -> Void) { - completion(.success(())) - } + func insert(_ data: Data, for url: URL) throws {} - func retrieve(dataForURL url: URL, completion: @escaping (FeedImageDataStore.RetrievalResult) -> Void) { - completion(.success(.none)) - } + func retrieve(dataForURL url: URL) throws -> Data? { .none } } From be545d96e3ee492f7fe165e9c138e4156c3031bc Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Fri, 28 Mar 2025 07:53:14 -0300 Subject: [PATCH 10/25] Make in memory FeedImageDataStore implementation sync --- .../EssentialAppTests/Helpers/InMemoryFeedStore.swift | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/EssentialApp/EssentialAppTests/Helpers/InMemoryFeedStore.swift b/EssentialApp/EssentialAppTests/Helpers/InMemoryFeedStore.swift index a0e3446..05596ab 100644 --- a/EssentialApp/EssentialAppTests/Helpers/InMemoryFeedStore.swift +++ b/EssentialApp/EssentialAppTests/Helpers/InMemoryFeedStore.swift @@ -32,13 +32,12 @@ extension InMemoryFeedStore: FeedStore { } extension InMemoryFeedStore: FeedImageDataStore { - func insert(_ data: Data, for url: URL, completion: @escaping (FeedImageDataStore.InsertionResult) -> Void) { + func insert(_ data: Data, for url: URL) throws { feedImageDataCache[url] = data - completion(.success(())) } - func retrieve(dataForURL url: URL, completion: @escaping (FeedImageDataStore.RetrievalResult) -> Void) { - completion(.success(feedImageDataCache[url])) + func retrieve(dataForURL url: URL) throws -> Data? { + feedImageDataCache[url] } } From aeca19f96ddd07cf91bc89ce42b1fdfe049f4a8b Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Fri, 28 Mar 2025 07:56:28 -0300 Subject: [PATCH 11/25] Remove unused async methods --- .../Feed Cache/FeedImageDataStore.swift | 38 ------------------- .../CoreDataFeedImageDataStoreTests.swift | 22 ++--------- 2 files changed, 3 insertions(+), 57 deletions(-) diff --git a/EssentialFeed/EssentialFeed/Feed Cache/FeedImageDataStore.swift b/EssentialFeed/EssentialFeed/Feed Cache/FeedImageDataStore.swift index 4c0ffb9..642c178 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/FeedImageDataStore.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/FeedImageDataStore.swift @@ -6,44 +6,6 @@ import Foundation public protocol FeedImageDataStore { - typealias RetrievalResult = Swift.Result - typealias InsertionResult = Swift.Result - func insert(_ data: Data, for url: URL) throws func retrieve(dataForURL url: URL) throws -> Data? - - @available(*, deprecated) - func insert(_ data: Data, for url: URL, completion: @escaping (InsertionResult) -> Void) - - @available(*, deprecated) - func retrieve(dataForURL url: URL, completion: @escaping (RetrievalResult) -> Void) -} - -public extension FeedImageDataStore { - func insert(_ data: Data, for url: URL) throws { - let group = DispatchGroup() - group.enter() - var result: InsertionResult! - insert(data, for: url) { - result = $0 - group.leave() - } - group.wait() - return try result.get() - } - - func retrieve(dataForURL url: URL) throws -> Data? { - let group = DispatchGroup() - group.enter() - var result: RetrievalResult! - retrieve(dataForURL: url) { - result = $0 - group.leave() - } - group.wait() - return try result.get() - } - - func insert(_ data: Data, for url: URL, completion: @escaping (InsertionResult) -> Void) {} - func retrieve(dataForURL url: URL, completion: @escaping (RetrievalResult) -> Void) {} } diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/CoreDataFeedImageDataStoreTests.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/CoreDataFeedImageDataStoreTests.swift index bd324ea..a46034d 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/CoreDataFeedImageDataStoreTests.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/CoreDataFeedImageDataStoreTests.swift @@ -46,22 +46,6 @@ class CoreDataFeedImageDataStoreTests: XCTestCase { expect(sut, toCompleteRetrievalWith: found(lastStoredData), for: url) } - func test_sideEffects_runSerially() { - let sut = makeSUT() - let url = anyURL() - - let op1 = expectation(description: "Operation 1") - sut.insert([localImage(url: url)], timestamp: Date()) { _ in op1.fulfill() } - - let op2 = expectation(description: "Operation 2") - sut.insert(anyData(), for: url) { _ in op2.fulfill() } - - let op3 = expectation(description: "Operation 3") - sut.insert(anyData(), for: url) { _ in op3.fulfill() } - - wait(for: [op1, op2, op3], timeout: 5.0, enforceOrder: true) - } - // - MARK: Helpers private func makeSUT(file: StaticString = #file, line: UInt = #line) -> CoreDataFeedStore { @@ -71,11 +55,11 @@ class CoreDataFeedImageDataStoreTests: XCTestCase { return sut } - private func notFound() -> FeedImageDataStore.RetrievalResult { + private func notFound() -> Result { return .success(.none) } - private func found(_ data: Data) -> FeedImageDataStore.RetrievalResult { + private func found(_ data: Data) -> Result { return .success(data) } @@ -83,7 +67,7 @@ class CoreDataFeedImageDataStoreTests: XCTestCase { return LocalFeedImage(id: UUID(), description: "any", location: "any", url: url) } - private func expect(_ sut: CoreDataFeedStore, toCompleteRetrievalWith expectedResult: FeedImageDataStore.RetrievalResult, for url: URL, file: StaticString = #file, line: UInt = #line) { + private func expect(_ sut: CoreDataFeedStore, toCompleteRetrievalWith expectedResult: Result, for url: URL, file: StaticString = #file, line: UInt = #line) { let receivedResult = Result { try sut.retrieve(dataForURL: url) } switch (receivedResult, expectedResult) { From b547dd8bebdea32df4b20ab7573c6e0de1bbb789 Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Sun, 30 Mar 2025 23:18:22 -0300 Subject: [PATCH 12/25] Add AnyScheduler --- .../EssentialApp/CombineHelpers.swift | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/EssentialApp/EssentialApp/CombineHelpers.swift b/EssentialApp/EssentialApp/CombineHelpers.swift index 056627f..14e18b8 100644 --- a/EssentialApp/EssentialApp/CombineHelpers.swift +++ b/EssentialApp/EssentialApp/CombineHelpers.swift @@ -169,3 +169,49 @@ extension DispatchQueue { } } } + +typealias AnyDispatchQueueScheduler = AnyScheduler + +extension AnyDispatchQueueScheduler { + static var immediateOnMainQueue: Self { + DispatchQueue.immediateWhenOnMainQueueScheduler.eraseToAnyScheduler() + } +} + +extension Scheduler { + func eraseToAnyScheduler() -> AnyScheduler { + AnyScheduler(self) + } +} + +struct AnyScheduler: Scheduler where SchedulerTimeType.Stride: SchedulerTimeIntervalConvertible { + private let _now: () -> SchedulerTimeType + private let _minimumTolerance: () -> SchedulerTimeType.Stride + private let _schedule: (SchedulerOptions?, @escaping () -> Void) -> Void + private let _scheduleAfter: (SchedulerTimeType, SchedulerTimeType.Stride, SchedulerOptions?, @escaping () -> Void) -> Void + private let _scheduleAfterInterval: (SchedulerTimeType, SchedulerTimeType.Stride, SchedulerTimeType.Stride, SchedulerOptions?, @escaping () -> Void) -> Cancellable + + init(_ scheduler: S) where SchedulerTimeType == S.SchedulerTimeType, SchedulerOptions == S.SchedulerOptions, S: Scheduler { + _now = { scheduler.now } + _minimumTolerance = { scheduler.minimumTolerance } + _schedule = scheduler.schedule(options:_:) + _scheduleAfter = scheduler.schedule(after:tolerance:options:_:) + _scheduleAfterInterval = scheduler.schedule(after:interval:tolerance:options:_:) + } + + var now: SchedulerTimeType { _now() } + + var minimumTolerance: SchedulerTimeType.Stride { _minimumTolerance() } + + func schedule(options: SchedulerOptions?, _ action: @escaping () -> Void) { + _schedule(options, action) + } + + func schedule(after date: SchedulerTimeType, tolerance: SchedulerTimeType.Stride, options: SchedulerOptions?, _ action: @escaping () -> Void) { + _scheduleAfter(date, tolerance, options, action) + } + + func schedule(after date: SchedulerTimeType, interval: SchedulerTimeType.Stride, tolerance: SchedulerTimeType.Stride, options: SchedulerOptions?, _ action: @escaping () -> Void) -> Cancellable { + _scheduleAfterInterval(date, interval, tolerance, options, action) + } +} From 8eac75a66f78e9d43d91ba40668adb2a6b4207fb Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Sun, 30 Mar 2025 23:23:50 -0300 Subject: [PATCH 13/25] Subscribe upstream store subscriptions in a background queue to avoid blocking the main queue (tests still run synchronously in the main queue with the immediate scheduler) --- EssentialApp/EssentialApp/SceneDelegate.swift | 15 +++++++++++++-- .../EssentialAppTests/FeedAcceptanceTests.swift | 4 ++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/EssentialApp/EssentialApp/SceneDelegate.swift b/EssentialApp/EssentialApp/SceneDelegate.swift index 406cf2f..32d936f 100644 --- a/EssentialApp/EssentialApp/SceneDelegate.swift +++ b/EssentialApp/EssentialApp/SceneDelegate.swift @@ -12,6 +12,12 @@ import EssentialFeed class SceneDelegate: UIResponder, UIWindowSceneDelegate { var window: UIWindow? + private lazy var scheduler: AnyDispatchQueueScheduler = DispatchQueue( + label: "portocode.infra.queue", + qos: .userInitiated, + attributes: .concurrent + ).eraseToAnyScheduler() + private lazy var httpClient: HTTPClient = { URLSessionHTTPClient(session: URLSession(configuration: .ephemeral)) }() @@ -43,10 +49,11 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate { imageLoader: makeLocalImageLoaderWithRemoteFallback, selection: showComments)) - convenience init(httpClient: HTTPClient, store: FeedStore & FeedImageDataStore) { + convenience init(httpClient: HTTPClient, store: FeedStore & FeedImageDataStore, scheduler: AnyDispatchQueueScheduler) { self.init() self.httpClient = httpClient self.store = store + self.scheduler = scheduler } func scene(_ scene: UIScene, willConnectTo session: UISceneSession, options connectionOptions: UIScene.ConnectionOptions) { @@ -121,11 +128,15 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate { return localImageLoader .loadImageDataPublisher(from: url) - .fallback(to: { [httpClient] in + .fallback(to: { [httpClient, scheduler] in httpClient .getPublisher(url: url) .tryMap(FeedImageDataMapper.map) .caching(to: localImageLoader, using: url) + .subscribe(on: scheduler) + .eraseToAnyPublisher() }) + .subscribe(on: scheduler) + .eraseToAnyPublisher() } } diff --git a/EssentialApp/EssentialAppTests/FeedAcceptanceTests.swift b/EssentialApp/EssentialAppTests/FeedAcceptanceTests.swift index 1cad5c2..cd60660 100644 --- a/EssentialApp/EssentialAppTests/FeedAcceptanceTests.swift +++ b/EssentialApp/EssentialAppTests/FeedAcceptanceTests.swift @@ -87,7 +87,7 @@ class FeedAcceptanceTests: XCTestCase { httpClient: HTTPClientStub = .offline, store: InMemoryFeedStore = .empty ) -> ListViewController { - let sut = SceneDelegate(httpClient: httpClient, store: store) + let sut = SceneDelegate(httpClient: httpClient, store: store, scheduler: .immediateOnMainQueue) sut.window = UIWindow(frame: CGRect(x: 0, y: 0, width: 390, height: 1)) sut.configureWindow() @@ -98,7 +98,7 @@ class FeedAcceptanceTests: XCTestCase { } private func enterBackground(with store: InMemoryFeedStore) { - let sut = SceneDelegate(httpClient: HTTPClientStub.offline, store: store) + let sut = SceneDelegate(httpClient: HTTPClientStub.offline, store: store, scheduler: .immediateOnMainQueue) sut.sceneWillResignActive(UIApplication.shared.connectedScenes.first!) } From 600b4d69f0676097f579866ac4f1cb4b2d9c5502 Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Mon, 31 Mar 2025 21:32:26 -0300 Subject: [PATCH 14/25] Deprecate async APIs in favor of new sync APIs --- .../EssentialFeed/Feed Cache/FeedStore.swift | 55 +++++++++++++++++-- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/EssentialFeed/EssentialFeed/Feed Cache/FeedStore.swift b/EssentialFeed/EssentialFeed/Feed Cache/FeedStore.swift index fde7f39..82c9795 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/FeedStore.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/FeedStore.swift @@ -8,6 +8,10 @@ import Foundation public typealias CachedFeed = (feed: [LocalFeedImage], timestamp: Date) public protocol FeedStore { + func deleteCachedFeed() throws + func insert(_ feed: [LocalFeedImage], timestamp: Date) throws + func retrieve() throws -> CachedFeed? + typealias DeletionResult = Result typealias DeletionCompletion = (DeletionResult) -> Void @@ -17,15 +21,54 @@ public protocol FeedStore { typealias RetrievalResult = Result typealias RetrievalCompletion = (RetrievalResult) -> Void - /// The completion handler can be invoked in any thread. - /// Clients are responsible to dispatch to appropriate threads, if needed. + @available(*, deprecated) func deleteCachedFeed(completion: @escaping DeletionCompletion) - /// The completion handler can be invoked in any thread. - /// Clients are responsible to dispatch to appropriate threads, if needed. + @available(*, deprecated) func insert(_ feed: [LocalFeedImage], timestamp: Date, completion: @escaping InsertionCompletion) - /// The completion handler can be invoked in any thread. - /// Clients are responsible to dispatch to appropriate threads, if needed. + @available(*, deprecated) func retrieve(completion: @escaping RetrievalCompletion) } + +public extension FeedStore { + func deleteCachedFeed() throws { + let group = DispatchGroup() + group.enter() + var result: DeletionResult! + deleteCachedFeed { + result = $0 + group.leave() + } + group.wait() + return try result.get() + } + + func insert(_ feed: [LocalFeedImage], timestamp: Date) throws { + let group = DispatchGroup() + group.enter() + var result: InsertionResult! + insert(feed, timestamp: timestamp) { + result = $0 + group.leave() + } + group.wait() + return try result.get() + } + + func retrieve() throws -> CachedFeed? { + let group = DispatchGroup() + group.enter() + var result: RetrievalResult! + retrieve { + result = $0 + group.leave() + } + group.wait() + return try result.get() + } + + func deleteCachedFeed(completion: @escaping DeletionCompletion) {} + func insert(_ feed: [LocalFeedImage], timestamp: Date, completion: @escaping InsertionCompletion) {} + func retrieve(completion: @escaping RetrievalCompletion) {} +} From 86d0a0d3a643020c684a3402771b5083e25bc3aa Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Mon, 31 Mar 2025 22:07:11 -0300 Subject: [PATCH 15/25] Perform feed cache operations synchronously --- .../Feed Cache/LocalFeedLoader.swift | 63 ++++++------------- .../Feed Cache/CacheFeedUseCaseTests.swift | 41 +----------- .../Feed Cache/Helpers/FeedStoreSpy.swift | 46 +++++++------- .../LoadFeedFromCacheUseCaseTests.swift | 25 ++------ .../ValidateFeedCacheUseCaseTests.swift | 24 ++----- 5 files changed, 57 insertions(+), 142 deletions(-) diff --git a/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedLoader.swift b/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedLoader.swift index 593db5a..939c78b 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedLoader.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedLoader.swift @@ -19,24 +19,10 @@ extension LocalFeedLoader: FeedCache { public typealias SaveResult = FeedCache.Result public func save(_ feed: [FeedImage], completion: @escaping (SaveResult) -> Void) { - store.deleteCachedFeed { [weak self] deletionResult in - guard let self = self else { return } - - switch deletionResult { - case .success: - self.cache(feed, with: completion) - case let .failure(error): - completion(.failure(error)) - } - } - } - - private func cache(_ feed: [FeedImage], with completion: @escaping (SaveResult) -> Void) { - store.insert(feed.toLocal(), timestamp: self.currentDate()) { [weak self] error in - guard self != nil else { return } - - completion(error) - } + completion(SaveResult { + try store.deleteCachedFeed() + try store.insert(feed.toLocal(), timestamp: currentDate()) + }) } } @@ -44,41 +30,30 @@ extension LocalFeedLoader { public typealias LoadResult = Swift.Result<[FeedImage], Error> public func load(completion: @escaping (LoadResult) -> Void) { - store.retrieve { [weak self] result in - guard let self = self else { return } - - switch result { - case let .failure(error): - completion(.failure(error)) - - case let .success(.some(cache)) where FeedCachePolicy.validate(cache.timestamp, against: self.currentDate()): - completion(.success(cache.feed.toModels())) - - case .success: - completion(.success([])) + completion(LoadResult { + if let cache = try store.retrieve(), FeedCachePolicy.validate(cache.timestamp, against: currentDate()) { + return cache.feed.toModels() } - } + return [] + }) } } extension LocalFeedLoader { public typealias ValidationResult = Result + private struct InvalidCache: Error {} + public func validateCache(completion: @escaping (ValidationResult) -> Void) { - store.retrieve { [weak self] result in - guard let self = self else { return } - - switch result { - case .failure: - self.store.deleteCachedFeed(completion: completion) - - case let .success(.some(cache)) where !FeedCachePolicy.validate(cache.timestamp, against: self.currentDate()): - self.store.deleteCachedFeed(completion: completion) - - case .success: - completion(.success(())) + completion(ValidationResult { + do { + if let cache = try store.retrieve(), !FeedCachePolicy.validate(cache.timestamp, against: currentDate()) { + throw InvalidCache() + } + } catch { + try store.deleteCachedFeed() } - } + }) } } diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedUseCaseTests.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedUseCaseTests.swift index e41aab6..8aa1968 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedUseCaseTests.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedUseCaseTests.swift @@ -14,20 +14,12 @@ class CacheFeedUseCaseTests: XCTestCase { XCTAssertEqual(store.receivedMessages, []) } - func test_save_requestsCacheDeletion() { - let (sut, store) = makeSUT() - - sut.save(uniqueImageFeed().models) { _ in } - - XCTAssertEqual(store.receivedMessages, [.deleteCachedFeed]) - } - func test_save_doesNotRequestCacheInsertionOnDeletionError() { let (sut, store) = makeSUT() let deletionError = anyNSError() + store.completeDeletion(with: deletionError) sut.save(uniqueImageFeed().models) { _ in } - store.completeDeletion(with: deletionError) XCTAssertEqual(store.receivedMessages, [.deleteCachedFeed]) } @@ -36,9 +28,9 @@ class CacheFeedUseCaseTests: XCTestCase { let timestamp = Date() let feed = uniqueImageFeed() let (sut, store) = makeSUT(currentDate: { timestamp }) + store.completeDeletionSuccessfully() sut.save(feed.models) { _ in } - store.completeDeletionSuccessfully() XCTAssertEqual(store.receivedMessages, [.deleteCachedFeed, .insert(feed.local, timestamp)]) } @@ -71,33 +63,6 @@ class CacheFeedUseCaseTests: XCTestCase { }) } - func test_save_doesNotDeliverDeletionErrorAfterSUTInstanceHasBeenDeallocated() { - let store = FeedStoreSpy() - var sut: LocalFeedLoader? = LocalFeedLoader(store: store, currentDate: Date.init) - - var receivedResults = [LocalFeedLoader.SaveResult]() - sut?.save(uniqueImageFeed().models) { receivedResults.append($0) } - - sut = nil - store.completeDeletion(with: anyNSError()) - - XCTAssertTrue(receivedResults.isEmpty) - } - - func test_save_doesNotDeliverInsertionErrorAfterSUTInstanceHasBeenDeallocated() { - let store = FeedStoreSpy() - var sut: LocalFeedLoader? = LocalFeedLoader(store: store, currentDate: Date.init) - - var receivedResults = [LocalFeedLoader.SaveResult]() - sut?.save(uniqueImageFeed().models) { receivedResults.append($0) } - - store.completeDeletionSuccessfully() - sut = nil - store.completeInsertion(with: anyNSError()) - - XCTAssertTrue(receivedResults.isEmpty) - } - // MARK: - Helpers private func makeSUT(currentDate: @escaping () -> Date = Date.init, file: StaticString = #filePath, line: UInt = #line) -> (sut: LocalFeedLoader, store: FeedStoreSpy) { @@ -110,6 +75,7 @@ class CacheFeedUseCaseTests: XCTestCase { private func expect(_ sut: LocalFeedLoader, toCompleteWithError expectedError: NSError?, when action: () -> Void, file: StaticString = #filePath, line: UInt = #line) { let exp = expectation(description: "Wait for save completion") + action() var receivedError: Error? sut.save(uniqueImageFeed().models) { result in @@ -117,7 +83,6 @@ class CacheFeedUseCaseTests: XCTestCase { exp.fulfill() } - action() wait(for: [exp], timeout: 1.0) XCTAssertEqual(receivedError as NSError?, expectedError, file: file, line: line) diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/Helpers/FeedStoreSpy.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/Helpers/FeedStoreSpy.swift index c00197f..b7edad2 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/Helpers/FeedStoreSpy.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/Helpers/FeedStoreSpy.swift @@ -15,50 +15,50 @@ class FeedStoreSpy: FeedStore { private(set) var receivedMessages = [ReceivedMessage]() - private var deletionCompletions = [DeletionCompletion]() - private var insertionCompletions = [InsertionCompletion]() - private var retrievalCompletions = [RetrievalCompletion]() + private var deletionResult: Result? + private var insertionResult: Result? + private var retrievalResult: Result? - func deleteCachedFeed(completion: @escaping DeletionCompletion) { - deletionCompletions.append(completion) + func deleteCachedFeed() throws { receivedMessages.append(.deleteCachedFeed) + try deletionResult?.get() } - func completeDeletion(with error: Error, at index: Int = 0) { - deletionCompletions[index](.failure(error)) + func completeDeletion(with error: Error) { + deletionResult = .failure(error) } - func completeDeletionSuccessfully(at index: Int = 0) { - deletionCompletions[index](.success(())) + func completeDeletionSuccessfully() { + deletionResult = .success(()) } - func insert(_ feed: [LocalFeedImage], timestamp: Date, completion: @escaping InsertionCompletion) { - insertionCompletions.append(completion) + func insert(_ feed: [LocalFeedImage], timestamp: Date) throws { receivedMessages.append(.insert(feed, timestamp)) + try insertionResult?.get() } - func completeInsertion(with error: Error, at index: Int = 0) { - insertionCompletions[index](.failure(error)) + func completeInsertion(with error: Error) { + insertionResult = .failure(error) } - func completeInsertionSuccessfully(at index: Int = 0) { - insertionCompletions[index](.success(())) + func completeInsertionSuccessfully() { + insertionResult = .success(()) } - func retrieve(completion: @escaping RetrievalCompletion) { - retrievalCompletions.append(completion) + func retrieve() throws -> CachedFeed? { receivedMessages.append(.retrieve) + return try retrievalResult?.get() } - func completeRetrieval(with error: Error, at index: Int = 0) { - retrievalCompletions[index](.failure(error)) + func completeRetrieval(with error: Error) { + retrievalResult = .failure(error) } - func completeRetrievalWithEmptyCache(at index: Int = 0) { - retrievalCompletions[index](.success(.none)) + func completeRetrievalWithEmptyCache() { + retrievalResult = .success(.none) } - func completeRetrieval(with feed: [LocalFeedImage], timestamp: Date, at index: Int = 0) { - retrievalCompletions[index](.success(CachedFeed(feed: feed, timestamp: timestamp))) + func completeRetrieval(with feed: [LocalFeedImage], timestamp: Date) { + retrievalResult = .success(CachedFeed(feed: feed, timestamp: timestamp)) } } diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/LoadFeedFromCacheUseCaseTests.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/LoadFeedFromCacheUseCaseTests.swift index 0971ad2..fc84edf 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/LoadFeedFromCacheUseCaseTests.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/LoadFeedFromCacheUseCaseTests.swift @@ -74,18 +74,18 @@ class LoadFeedFromCacheUseCaseTests: XCTestCase { func test_load_hasNoSideEffectsOnRetrievalError() { let (sut, store) = makeSUT() + store.completeRetrieval(with: anyNSError()) sut.load { _ in } - store.completeRetrieval(with: anyNSError()) XCTAssertEqual(store.receivedMessages, [.retrieve]) } func test_load_hasNoSideEffectsOnEmptyCache() { let (sut, store) = makeSUT() + store.completeRetrievalWithEmptyCache() sut.load { _ in } - store.completeRetrievalWithEmptyCache() XCTAssertEqual(store.receivedMessages, [.retrieve]) } @@ -95,9 +95,9 @@ class LoadFeedFromCacheUseCaseTests: XCTestCase { let fixedCurrentDate = Date() let nonExpiredTimestamp = fixedCurrentDate.minusFeedCacheMaxAge().adding(seconds: 1) let (sut, store) = makeSUT(currentDate: { fixedCurrentDate } ) + store.completeRetrieval(with: feed.local, timestamp: nonExpiredTimestamp) sut.load { _ in } - store.completeRetrieval(with: feed.local, timestamp: nonExpiredTimestamp) XCTAssertEqual(store.receivedMessages, [.retrieve]) } @@ -107,9 +107,9 @@ class LoadFeedFromCacheUseCaseTests: XCTestCase { let fixedCurrentDate = Date() let expirationTimestamp = fixedCurrentDate.minusFeedCacheMaxAge() let (sut, store) = makeSUT(currentDate: { fixedCurrentDate } ) + store.completeRetrieval(with: feed.local, timestamp: expirationTimestamp) sut.load { _ in } - store.completeRetrieval(with: feed.local, timestamp: expirationTimestamp) XCTAssertEqual(store.receivedMessages, [.retrieve]) } @@ -119,26 +119,13 @@ class LoadFeedFromCacheUseCaseTests: XCTestCase { let fixedCurrentDate = Date() let expiredTimestamp = fixedCurrentDate.minusFeedCacheMaxAge().adding(seconds: -1) let (sut, store) = makeSUT(currentDate: { fixedCurrentDate } ) + store.completeRetrieval(with: feed.local, timestamp: expiredTimestamp) sut.load { _ in } - store.completeRetrieval(with: feed.local, timestamp: expiredTimestamp) XCTAssertEqual(store.receivedMessages, [.retrieve]) } - func test_load_doesNotDeliverResultAfterSUTInstanceHasBeenDeallocated() { - let store = FeedStoreSpy() - var sut: LocalFeedLoader? = LocalFeedLoader(store: store, currentDate: Date.init) - - var receivedResults = [LocalFeedLoader.LoadResult]() - sut?.load { receivedResults.append($0) } - - sut = nil - store.completeRetrievalWithEmptyCache() - - XCTAssertTrue(receivedResults.isEmpty) - } - // MARK: - Helpers private func makeSUT(currentDate: @escaping () -> Date = Date.init, file: StaticString = #filePath, line: UInt = #line) -> (sut: LocalFeedLoader, store: FeedStoreSpy) { @@ -151,6 +138,7 @@ class LoadFeedFromCacheUseCaseTests: XCTestCase { private func expect(_ sut: LocalFeedLoader, toCompletion expectedResult: LocalFeedLoader.LoadResult, when action: () -> Void, file: StaticString = #filePath, line: UInt = #line) { let exp = expectation(description: "Wait for load completion") + action() sut.load { receivedResult in switch (receivedResult, expectedResult) { @@ -167,7 +155,6 @@ class LoadFeedFromCacheUseCaseTests: XCTestCase { exp.fulfill() } - action() wait(for: [exp], timeout: 1.0) } diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/ValidateFeedCacheUseCaseTests.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/ValidateFeedCacheUseCaseTests.swift index 085affb..ab15ddf 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/ValidateFeedCacheUseCaseTests.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/ValidateFeedCacheUseCaseTests.swift @@ -16,18 +16,18 @@ class ValidateFeedCacheUseCaseTests: XCTestCase { func test_validateCache_deletesCacheOnRetrievalError() { let (sut, store) = makeSUT() + store.completeRetrieval(with: anyNSError()) sut.validateCache { _ in } - store.completeRetrieval(with: anyNSError()) XCTAssertEqual(store.receivedMessages, [.retrieve, .deleteCachedFeed]) } func test_validateCache_doesNotDeleteCacheOnEmptyCache() { let (sut, store) = makeSUT() + store.completeRetrievalWithEmptyCache() sut.validateCache { _ in } - store.completeRetrievalWithEmptyCache() XCTAssertEqual(store.receivedMessages, [.retrieve]) } @@ -37,9 +37,9 @@ class ValidateFeedCacheUseCaseTests: XCTestCase { let fixedCurrentDate = Date() let nonExpiredTimestamp = fixedCurrentDate.minusFeedCacheMaxAge().adding(seconds: 1) let (sut, store) = makeSUT(currentDate: { fixedCurrentDate } ) + store.completeRetrieval(with: feed.local, timestamp: nonExpiredTimestamp) sut.validateCache { _ in } - store.completeRetrieval(with: feed.local, timestamp: nonExpiredTimestamp) XCTAssertEqual(store.receivedMessages, [.retrieve]) } @@ -49,9 +49,9 @@ class ValidateFeedCacheUseCaseTests: XCTestCase { let fixedCurrentDate = Date() let expirationTimestamp = fixedCurrentDate.minusFeedCacheMaxAge() let (sut, store) = makeSUT(currentDate: { fixedCurrentDate } ) + store.completeRetrieval(with: feed.local, timestamp: expirationTimestamp) sut.validateCache { _ in } - store.completeRetrieval(with: feed.local, timestamp: expirationTimestamp) XCTAssertEqual(store.receivedMessages, [.retrieve, .deleteCachedFeed]) } @@ -61,9 +61,9 @@ class ValidateFeedCacheUseCaseTests: XCTestCase { let fixedCurrentDate = Date() let expiredTimestamp = fixedCurrentDate.minusFeedCacheMaxAge().adding(seconds: -1) let (sut, store) = makeSUT(currentDate: { fixedCurrentDate } ) + store.completeRetrieval(with: feed.local, timestamp: expiredTimestamp) sut.validateCache { _ in } - store.completeRetrieval(with: feed.local, timestamp: expiredTimestamp) XCTAssertEqual(store.receivedMessages, [.retrieve, .deleteCachedFeed]) } @@ -131,18 +131,6 @@ class ValidateFeedCacheUseCaseTests: XCTestCase { }) } - func test_validateCache_doesNotDeleteInvalidCacheAfterSUTInstanceHasBeenDeallocated() { - let store = FeedStoreSpy() - var sut: LocalFeedLoader? = LocalFeedLoader(store: store, currentDate: Date.init) - - sut?.validateCache{ _ in } - - sut = nil - store.completeRetrieval(with: anyNSError()) - - XCTAssertEqual(store.receivedMessages, [.retrieve]) - } - // MARK: - Helpers private func makeSUT(currentDate: @escaping () -> Date = Date.init, file: StaticString = #filePath, line: UInt = #line) -> (sut: LocalFeedLoader, store: FeedStoreSpy) { @@ -155,6 +143,7 @@ class ValidateFeedCacheUseCaseTests: XCTestCase { private func expect(_ sut: LocalFeedLoader, toCompleteWith expectedResult: LocalFeedLoader.ValidationResult, when action: () -> Void, file: StaticString = #file, line: UInt = #line) { let exp = expectation(description: "Wait for load completion") + action() sut.validateCache { receivedResult in switch (receivedResult, expectedResult) { @@ -171,7 +160,6 @@ class ValidateFeedCacheUseCaseTests: XCTestCase { exp.fulfill() } - action() wait(for: [exp], timeout: 1.0) } From f18701601234561486d9e50df17d4ef6bda920ca Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Mon, 31 Mar 2025 22:16:05 -0300 Subject: [PATCH 16/25] Make FeedCache sync --- EssentialApp/EssentialApp/CombineHelpers.swift | 2 +- .../Feed Cache/LocalFeedLoader.swift | 10 +++------- .../EssentialFeed/Feed Feature/FeedCache.swift | 4 +--- .../EssentialFeedCacheIntegrationTests.swift | 11 ++++------- .../Feed Cache/CacheFeedUseCaseTests.swift | 17 ++++++----------- 5 files changed, 15 insertions(+), 29 deletions(-) diff --git a/EssentialApp/EssentialApp/CombineHelpers.swift b/EssentialApp/EssentialApp/CombineHelpers.swift index 14e18b8..6278674 100644 --- a/EssentialApp/EssentialApp/CombineHelpers.swift +++ b/EssentialApp/EssentialApp/CombineHelpers.swift @@ -108,7 +108,7 @@ extension Publisher { private extension FeedCache { func saveIgnoringResult(_ feed: [FeedImage]) { - save(feed) { _ in } + try? save(feed) } func saveIgnoringResult(_ page: Paginated) { diff --git a/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedLoader.swift b/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedLoader.swift index 939c78b..304f8a1 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedLoader.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedLoader.swift @@ -16,13 +16,9 @@ public final class LocalFeedLoader { } extension LocalFeedLoader: FeedCache { - public typealias SaveResult = FeedCache.Result - - public func save(_ feed: [FeedImage], completion: @escaping (SaveResult) -> Void) { - completion(SaveResult { - try store.deleteCachedFeed() - try store.insert(feed.toLocal(), timestamp: currentDate()) - }) + public func save(_ feed: [FeedImage]) throws { + try store.deleteCachedFeed() + try store.insert(feed.toLocal(), timestamp: currentDate()) } } diff --git a/EssentialFeed/EssentialFeed/Feed Feature/FeedCache.swift b/EssentialFeed/EssentialFeed/Feed Feature/FeedCache.swift index 0b52bf0..51f33cf 100644 --- a/EssentialFeed/EssentialFeed/Feed Feature/FeedCache.swift +++ b/EssentialFeed/EssentialFeed/Feed Feature/FeedCache.swift @@ -6,7 +6,5 @@ import Foundation public protocol FeedCache { - typealias Result = Swift.Result - - func save(_ feed: [FeedImage], completion: @escaping (Result) -> Void) + func save(_ feed: [FeedImage]) throws } diff --git a/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift b/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift index 77cd8c5..5a235e1 100644 --- a/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift +++ b/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift @@ -125,14 +125,11 @@ class EssentialFeedCacheIntegrationTests: XCTestCase { } private func save(_ feed: [FeedImage], with loader: LocalFeedLoader, file: StaticString = #file, line: UInt = #line) { - let saveExp = expectation(description: "Wait for save completion") - loader.save(feed) { result in - if case let Result.failure(error) = result { - XCTFail("Expected to save feed successfully, got error: \(error)", file: file, line: line) - } - saveExp.fulfill() + do { + try loader.save(feed) + } catch { + XCTFail("Expected to save feed successfully, got error: \(error)", file: file, line: line) } - wait(for: [saveExp], timeout: 1.0) } private func validateCache(with loader: LocalFeedLoader, file: StaticString = #file, line: UInt = #line) { diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedUseCaseTests.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedUseCaseTests.swift index 8aa1968..607c474 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedUseCaseTests.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedUseCaseTests.swift @@ -19,7 +19,7 @@ class CacheFeedUseCaseTests: XCTestCase { let deletionError = anyNSError() store.completeDeletion(with: deletionError) - sut.save(uniqueImageFeed().models) { _ in } + try? sut.save(uniqueImageFeed().models) XCTAssertEqual(store.receivedMessages, [.deleteCachedFeed]) } @@ -30,7 +30,7 @@ class CacheFeedUseCaseTests: XCTestCase { let (sut, store) = makeSUT(currentDate: { timestamp }) store.completeDeletionSuccessfully() - sut.save(feed.models) { _ in } + try? sut.save(feed.models) XCTAssertEqual(store.receivedMessages, [.deleteCachedFeed, .insert(feed.local, timestamp)]) } @@ -74,18 +74,13 @@ class CacheFeedUseCaseTests: XCTestCase { } private func expect(_ sut: LocalFeedLoader, toCompleteWithError expectedError: NSError?, when action: () -> Void, file: StaticString = #filePath, line: UInt = #line) { - let exp = expectation(description: "Wait for save completion") action() - var receivedError: Error? - sut.save(uniqueImageFeed().models) { result in - if case let Result.failure(error) = result { receivedError = error } - exp.fulfill() + do { + try sut.save(uniqueImageFeed().models) + } catch { + XCTAssertEqual(error as NSError?, expectedError, file: file, line: line) } - - wait(for: [exp], timeout: 1.0) - - XCTAssertEqual(receivedError as NSError?, expectedError, file: file, line: line) } } From ec42cc8869f2068b643abc6923dd34ace3133661 Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Mon, 31 Mar 2025 23:37:59 -0300 Subject: [PATCH 17/25] Make LocalFeedLoader.load sync --- .../EssentialApp/CombineHelpers.swift | 4 +- .../Feed Cache/LocalFeedLoader.swift | 14 +++---- .../EssentialFeedCacheIntegrationTests.swift | 20 +++------- .../LoadFeedFromCacheUseCaseTests.swift | 39 ++++++++----------- 4 files changed, 30 insertions(+), 47 deletions(-) diff --git a/EssentialApp/EssentialApp/CombineHelpers.swift b/EssentialApp/EssentialApp/CombineHelpers.swift index 6278674..c8d51d6 100644 --- a/EssentialApp/EssentialApp/CombineHelpers.swift +++ b/EssentialApp/EssentialApp/CombineHelpers.swift @@ -84,7 +84,9 @@ public extension LocalFeedLoader { func loadPublisher() -> Publisher { Deferred { - Future(self.load) + Future { completion in + completion(Result{ try self.load() }) + } } .eraseToAnyPublisher() } diff --git a/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedLoader.swift b/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedLoader.swift index 304f8a1..8ac5af0 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedLoader.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedLoader.swift @@ -23,15 +23,11 @@ extension LocalFeedLoader: FeedCache { } extension LocalFeedLoader { - public typealias LoadResult = Swift.Result<[FeedImage], Error> - - public func load(completion: @escaping (LoadResult) -> Void) { - completion(LoadResult { - if let cache = try store.retrieve(), FeedCachePolicy.validate(cache.timestamp, against: currentDate()) { - return cache.feed.toModels() - } - return [] - }) + public func load() throws -> [FeedImage] { + if let cache = try store.retrieve(), FeedCachePolicy.validate(cache.timestamp, against: currentDate()) { + return cache.feed.toModels() + } + return [] } } diff --git a/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift b/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift index 5a235e1..0195023 100644 --- a/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift +++ b/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift @@ -144,22 +144,12 @@ class EssentialFeedCacheIntegrationTests: XCTestCase { } private func expect(_ sut: LocalFeedLoader, toLoad expectedFeed: [FeedImage], file: StaticString = #file, line: UInt = #line) { - let exp = expectation(description: "Wait for load completion") - sut.load { result in - switch result { - case let .success(loadedFeed): - XCTAssertEqual(loadedFeed, expectedFeed, file: file, line: line) - - case let .failure(error): - XCTFail("Expected successful feed result, got \(error) instead", file: file, line: line) - - @unknown default: - XCTFail("Unknown enum case") - } - - exp.fulfill() + do { + let loadedFeed = try sut.load() + XCTAssertEqual(loadedFeed, expectedFeed, file: file, line: line) + } catch { + XCTFail("Expected successful feed result, got \(error) instead", file: file, line: line) } - wait(for: [exp], timeout: 1.0) } private func save(_ data: Data, for url: URL, with loader: LocalFeedImageDataLoader, file: StaticString = #file, line: UInt = #line) { diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/LoadFeedFromCacheUseCaseTests.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/LoadFeedFromCacheUseCaseTests.swift index fc84edf..fab03c7 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/LoadFeedFromCacheUseCaseTests.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/LoadFeedFromCacheUseCaseTests.swift @@ -17,7 +17,7 @@ class LoadFeedFromCacheUseCaseTests: XCTestCase { func test_load_requestsCacheRetrieval() { let (sut, store) = makeSUT() - sut.load() { _ in } + _ = try? sut.load() XCTAssertEqual(store.receivedMessages, [.retrieve]) } @@ -76,7 +76,7 @@ class LoadFeedFromCacheUseCaseTests: XCTestCase { let (sut, store) = makeSUT() store.completeRetrieval(with: anyNSError()) - sut.load { _ in } + _ = try? sut.load() XCTAssertEqual(store.receivedMessages, [.retrieve]) } @@ -85,7 +85,7 @@ class LoadFeedFromCacheUseCaseTests: XCTestCase { let (sut, store) = makeSUT() store.completeRetrievalWithEmptyCache() - sut.load { _ in } + _ = try? sut.load() XCTAssertEqual(store.receivedMessages, [.retrieve]) } @@ -97,7 +97,7 @@ class LoadFeedFromCacheUseCaseTests: XCTestCase { let (sut, store) = makeSUT(currentDate: { fixedCurrentDate } ) store.completeRetrieval(with: feed.local, timestamp: nonExpiredTimestamp) - sut.load { _ in } + _ = try? sut.load() XCTAssertEqual(store.receivedMessages, [.retrieve]) } @@ -109,7 +109,7 @@ class LoadFeedFromCacheUseCaseTests: XCTestCase { let (sut, store) = makeSUT(currentDate: { fixedCurrentDate } ) store.completeRetrieval(with: feed.local, timestamp: expirationTimestamp) - sut.load { _ in } + _ = try? sut.load() XCTAssertEqual(store.receivedMessages, [.retrieve]) } @@ -121,7 +121,7 @@ class LoadFeedFromCacheUseCaseTests: XCTestCase { let (sut, store) = makeSUT(currentDate: { fixedCurrentDate } ) store.completeRetrieval(with: feed.local, timestamp: expiredTimestamp) - sut.load { _ in } + _ = try? sut.load() XCTAssertEqual(store.receivedMessages, [.retrieve]) } @@ -136,26 +136,21 @@ class LoadFeedFromCacheUseCaseTests: XCTestCase { return (sut, store) } - private func expect(_ sut: LocalFeedLoader, toCompletion expectedResult: LocalFeedLoader.LoadResult, when action: () -> Void, file: StaticString = #filePath, line: UInt = #line) { - let exp = expectation(description: "Wait for load completion") + private func expect(_ sut: LocalFeedLoader, toCompletion expectedResult: Result<[FeedImage], Error>, when action: () -> Void, file: StaticString = #filePath, line: UInt = #line) { action() - sut.load { receivedResult in - switch (receivedResult, expectedResult) { - case let (.success(receivedImages), .success(expectedImages)): - XCTAssertEqual(receivedImages, expectedImages, file: file, line: line) - - case let (.failure(receivedError as NSError), .failure(expectedError as NSError)): - XCTAssertEqual(receivedError, expectedError, file: file, line: line) - - default: - XCTFail("Expected result \(expectedResult), got \(receivedResult) instead") - } + let receivedResult = Result { try sut.load() } + + switch (receivedResult, expectedResult) { + case let (.success(receivedImages), .success(expectedImages)): + XCTAssertEqual(receivedImages, expectedImages, file: file, line: line) + + case let (.failure(receivedError as NSError), .failure(expectedError as NSError)): + XCTAssertEqual(receivedError, expectedError, file: file, line: line) - exp.fulfill() + default: + XCTFail("Expected result \(expectedResult), got \(receivedResult) instead") } - - wait(for: [exp], timeout: 1.0) } } From 43746081d2fda33920323fab9c02e03d1e8af9ea Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Mon, 31 Mar 2025 23:45:09 -0300 Subject: [PATCH 18/25] Make LocalFeedLoader.validateCache sync --- EssentialApp/EssentialApp/SceneDelegate.swift | 6 ++- .../Feed Cache/LocalFeedLoader.swift | 18 ++++----- .../EssentialFeedCacheIntegrationTests.swift | 11 ++---- .../ValidateFeedCacheUseCaseTests.swift | 37 ++++++++----------- 4 files changed, 32 insertions(+), 40 deletions(-) diff --git a/EssentialApp/EssentialApp/SceneDelegate.swift b/EssentialApp/EssentialApp/SceneDelegate.swift index 32d936f..dae87ef 100644 --- a/EssentialApp/EssentialApp/SceneDelegate.swift +++ b/EssentialApp/EssentialApp/SceneDelegate.swift @@ -69,7 +69,11 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate { } func sceneWillResignActive(_ scene: UIScene) { - localFeedLoader.validateCache { _ in } + do { + try localFeedLoader.validateCache() + } catch { + logger.error("Failed to validate cache with error: \(error.localizedDescription)") + } } private func showComments(for image: FeedImage) { diff --git a/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedLoader.swift b/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedLoader.swift index 8ac5af0..794ecd4 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedLoader.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/LocalFeedLoader.swift @@ -32,20 +32,16 @@ extension LocalFeedLoader { } extension LocalFeedLoader { - public typealias ValidationResult = Result - private struct InvalidCache: Error {} - public func validateCache(completion: @escaping (ValidationResult) -> Void) { - completion(ValidationResult { - do { - if let cache = try store.retrieve(), !FeedCachePolicy.validate(cache.timestamp, against: currentDate()) { - throw InvalidCache() - } - } catch { - try store.deleteCachedFeed() + public func validateCache() throws { + do { + if let cache = try store.retrieve(), !FeedCachePolicy.validate(cache.timestamp, against: currentDate()) { + throw InvalidCache() } - }) + } catch { + try store.deleteCachedFeed() + } } } diff --git a/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift b/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift index 0195023..a438965 100644 --- a/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift +++ b/EssentialFeed/EssentialFeedCacheIntegrationTests/EssentialFeedCacheIntegrationTests.swift @@ -133,14 +133,11 @@ class EssentialFeedCacheIntegrationTests: XCTestCase { } private func validateCache(with loader: LocalFeedLoader, file: StaticString = #file, line: UInt = #line) { - let saveExp = expectation(description: "Wait for save completion") - loader.validateCache() { result in - if case let Result.failure(error) = result { - XCTFail("Expected to validate feed successfully, got error: \(error)", file: file, line: line) - } - saveExp.fulfill() + do { + try loader.validateCache() + } catch { + XCTFail("Expected to validate feed successfully, got error: \(error)", file: file, line: line) } - wait(for: [saveExp], timeout: 1.0) } private func expect(_ sut: LocalFeedLoader, toLoad expectedFeed: [FeedImage], file: StaticString = #file, line: UInt = #line) { diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/ValidateFeedCacheUseCaseTests.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/ValidateFeedCacheUseCaseTests.swift index ab15ddf..c81a4b3 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/ValidateFeedCacheUseCaseTests.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/ValidateFeedCacheUseCaseTests.swift @@ -18,7 +18,7 @@ class ValidateFeedCacheUseCaseTests: XCTestCase { let (sut, store) = makeSUT() store.completeRetrieval(with: anyNSError()) - sut.validateCache { _ in } + try? sut.validateCache() XCTAssertEqual(store.receivedMessages, [.retrieve, .deleteCachedFeed]) } @@ -27,7 +27,7 @@ class ValidateFeedCacheUseCaseTests: XCTestCase { let (sut, store) = makeSUT() store.completeRetrievalWithEmptyCache() - sut.validateCache { _ in } + try? sut.validateCache() XCTAssertEqual(store.receivedMessages, [.retrieve]) } @@ -39,7 +39,7 @@ class ValidateFeedCacheUseCaseTests: XCTestCase { let (sut, store) = makeSUT(currentDate: { fixedCurrentDate } ) store.completeRetrieval(with: feed.local, timestamp: nonExpiredTimestamp) - sut.validateCache { _ in } + try? sut.validateCache() XCTAssertEqual(store.receivedMessages, [.retrieve]) } @@ -51,7 +51,7 @@ class ValidateFeedCacheUseCaseTests: XCTestCase { let (sut, store) = makeSUT(currentDate: { fixedCurrentDate } ) store.completeRetrieval(with: feed.local, timestamp: expirationTimestamp) - sut.validateCache { _ in } + try? sut.validateCache() XCTAssertEqual(store.receivedMessages, [.retrieve, .deleteCachedFeed]) } @@ -63,7 +63,7 @@ class ValidateFeedCacheUseCaseTests: XCTestCase { let (sut, store) = makeSUT(currentDate: { fixedCurrentDate } ) store.completeRetrieval(with: feed.local, timestamp: expiredTimestamp) - sut.validateCache { _ in } + try? sut.validateCache() XCTAssertEqual(store.receivedMessages, [.retrieve, .deleteCachedFeed]) } @@ -141,26 +141,21 @@ class ValidateFeedCacheUseCaseTests: XCTestCase { return (sut, store) } - private func expect(_ sut: LocalFeedLoader, toCompleteWith expectedResult: LocalFeedLoader.ValidationResult, when action: () -> Void, file: StaticString = #file, line: UInt = #line) { - let exp = expectation(description: "Wait for load completion") + private func expect(_ sut: LocalFeedLoader, toCompleteWith expectedResult: Result, when action: () -> Void, file: StaticString = #file, line: UInt = #line) { action() - sut.validateCache { receivedResult in - switch (receivedResult, expectedResult) { - case (.success, .success): - break - - case let (.failure(receivedError as NSError), .failure(expectedError as NSError)): - XCTAssertEqual(receivedError, expectedError, file: file, line: line) - - default: - XCTFail("Expected result \(expectedResult), got \(receivedResult) instead", file: file, line: line) - } + let receivedResult = Result { try sut.validateCache() } + + switch (receivedResult, expectedResult) { + case (.success, .success): + break + + case let (.failure(receivedError as NSError), .failure(expectedError as NSError)): + XCTAssertEqual(receivedError, expectedError, file: file, line: line) - exp.fulfill() + default: + XCTFail("Expected result \(expectedResult), got \(receivedResult) instead", file: file, line: line) } - - wait(for: [exp], timeout: 1.0) } } From ac871303f719c274d00f90187145dfac4ec9f98c Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Tue, 1 Apr 2025 00:02:16 -0300 Subject: [PATCH 19/25] Make CoreData FeedStore implementation sync --- .../CoreDataFeedStore+FeedStore.swift | 24 +++--- .../CoreData/CoreDataFeedStore.swift | 5 -- .../CoreDataFeedImageDataStoreTests.swift | 14 +--- .../Feed Cache/CoreDataFeedStoreTests.swift | 5 -- .../FeedStoreSpecs/FeedStoreSpecs.swift | 2 - .../XCTestCase+FeedStoreSpecs.swift | 80 +++++-------------- 6 files changed, 37 insertions(+), 93 deletions(-) diff --git a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedStore.swift b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedStore.swift index 52311fb..7b53624 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedStore.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedStore.swift @@ -7,32 +7,32 @@ import CoreData extension CoreDataFeedStore: FeedStore { - public func retrieve(completion: @escaping RetrievalCompletion) { - performAsync { context in - completion(Result { + public func retrieve() throws -> CachedFeed? { + try performSync { context in + Result { try ManagedCache.find(in: context).map { CachedFeed(feed: $0.localFeed, timestamp: $0.timestamp) } - }) + } } } - public func insert(_ feed: [LocalFeedImage], timestamp: Date, completion: @escaping InsertionCompletion) { - performAsync { context in - completion(Result { + public func insert(_ feed: [LocalFeedImage], timestamp: Date) throws { + try performSync { context in + Result { let managedCache = try ManagedCache.newUniqueInstance(in: context) managedCache.timestamp = timestamp managedCache.feed = ManagedFeedImage.images(from: feed, in: context) try context.save() - }) + } } } - public func deleteCachedFeed(completion: @escaping DeletionCompletion) { - performAsync { context in - completion(Result { + public func deleteCachedFeed() throws { + try performSync { context in + Result { try ManagedCache.deleteCache(in: context) - }) + } } } diff --git a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore.swift b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore.swift index e999b42..d44a3c8 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore.swift @@ -30,11 +30,6 @@ public final class CoreDataFeedStore { } } - func performAsync(_ action: @escaping (NSManagedObjectContext) -> Void) { - let context = self.context - context.perform { action(context) } - } - func performSync(_ action: (NSManagedObjectContext) -> Result) throws -> R { let context = self.context var result: Result! diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/CoreDataFeedImageDataStoreTests.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/CoreDataFeedImageDataStoreTests.swift index a46034d..56798b2 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/CoreDataFeedImageDataStoreTests.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/CoreDataFeedImageDataStoreTests.swift @@ -80,21 +80,13 @@ class CoreDataFeedImageDataStoreTests: XCTestCase { } private func insert(_ data: Data, for url: URL, into sut: CoreDataFeedStore, file: StaticString = #file, line: UInt = #line) { - let exp = expectation(description: "Wait for cache insertion") - let image = localImage(url: url) - sut.insert([image], timestamp: Date()) { result in - if case let .failure(error) = result { - XCTFail("Failed to save \(image) with error \(error)", file: file, line: line) - } - - exp.fulfill() - } - wait(for: [exp], timeout: 1.0) - do { + let image = localImage(url: url) + try sut.insert([image], timestamp: Date()) try sut.insert(data, for: url) } catch { XCTFail("Failed to insert \(data) with error \(error)", file: file, line: line) } } + } diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/CoreDataFeedStoreTests.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/CoreDataFeedStoreTests.swift index a48d110..cb094d8 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/CoreDataFeedStoreTests.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/CoreDataFeedStoreTests.swift @@ -74,11 +74,6 @@ class CoreDataFeedStoreTests: XCTestCase, FeedStoreSpecs { assertThatDeleteEmptiesPreviouslyInsertedCache(on: sut) } - func test_storeSideEffects_runSerially() throws { - let sut = try makeSUT() - - assertThatSideEffectsRunSerially(on: sut) - } // MARK: - Helpers diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/FeedStoreSpecs/FeedStoreSpecs.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/FeedStoreSpecs/FeedStoreSpecs.swift index eb2352f..5dad022 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/FeedStoreSpecs/FeedStoreSpecs.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/FeedStoreSpecs/FeedStoreSpecs.swift @@ -19,8 +19,6 @@ protocol FeedStoreSpecs { func test_delete_hasNoSideEffectsOnEmptyCache() throws func test_delete_deliversNoErrorOnNonEmptyCache() throws func test_delete_emptiesPreviouslyInsertedCache() throws - - func test_storeSideEffects_runSerially() throws } protocol FailableRetrieveFeedStoreSpecs: FeedStoreSpecs { diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/FeedStoreSpecs/XCTestCase+FeedStoreSpecs.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/FeedStoreSpecs/XCTestCase+FeedStoreSpecs.swift index 5093c8e..3099ca1 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/FeedStoreSpecs/XCTestCase+FeedStoreSpecs.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/FeedStoreSpecs/XCTestCase+FeedStoreSpecs.swift @@ -87,57 +87,27 @@ extension FeedStoreSpecs where Self: XCTestCase { expect(sut, toRetrieve: .success(.none), file: file, line: line) } - func assertThatSideEffectsRunSerially(on sut: FeedStore, file: StaticString = #file, line: UInt = #line) { - var completedOperationsInOrder = [XCTestExpectation]() - - let op1 = expectation(description: "Operation 1") - sut.insert(uniqueImageFeed().local, timestamp: Date()) { _ in - completedOperationsInOrder.append(op1) - op1.fulfill() - } - - let op2 = expectation(description: "Operation 2") - sut.deleteCachedFeed { _ in - completedOperationsInOrder.append(op2) - op2.fulfill() - } - - let op3 = expectation(description: "Operation 3") - sut.insert(uniqueImageFeed().local, timestamp: Date()) { _ in - completedOperationsInOrder.append(op3) - op3.fulfill() - } - - waitForExpectations(timeout: 5.0) - - XCTAssertEqual(completedOperationsInOrder, [op1, op2, op3], "Expected side-effects to run serially but operations finished in the wrong order", file: file, line: line) - } - } extension FeedStoreSpecs where Self: XCTestCase { @discardableResult func insert(_ cache: (feed: [LocalFeedImage], timestamp: Date), to sut: FeedStore) -> Error? { - let exp = expectation(description: "Wait for cache insertion") - var insertionError: Error? - sut.insert(cache.feed, timestamp: cache.timestamp) { result in - if case let Result.failure(error) = result { insertionError = error } - exp.fulfill() + do { + try sut.insert(cache.feed, timestamp: cache.timestamp) + return nil + } catch { + return error } - wait(for: [exp], timeout: 1.0) - return insertionError } @discardableResult func deleteCache(from sut: FeedStore) -> Error? { - let exp = expectation(description: "Wait for cache deletion") - var deletionError: Error? - sut.deleteCachedFeed { result in - if case let Result.failure(error) = result { deletionError = error } - exp.fulfill() + do { + try sut.deleteCachedFeed() + return nil + } catch { + return error } - wait(for: [exp], timeout: 1.0) - return deletionError } func expect(_ sut: FeedStore, toRetrieveTwice expectedResult: FeedStore.RetrievalResult, file: StaticString = #file, line: UInt = #line) { @@ -146,25 +116,19 @@ extension FeedStoreSpecs where Self: XCTestCase { } func expect(_ sut: FeedStore, toRetrieve expectedResult: FeedStore.RetrievalResult, file: StaticString = #file, line: UInt = #line) { - let exp = expectation(description: "Wait for cache retrieval") - - sut.retrieve { retrievedResult in - switch (expectedResult, retrievedResult) { - case (.success(.none), .success(.none)), - (.failure, .failure): - break - - case let (.success(.some(expected)), .success(.some(retrieved))): - XCTAssertEqual(retrieved.feed, expected.feed, file: file, line: line) - XCTAssertEqual(retrieved.timestamp, expected.timestamp, file: file, line: line) - - default: - XCTFail("Expected to retrieve \(expectedResult), got \(retrievedResult) instead", file: file, line: line) - } + let retrievedResult = Result { try sut.retrieve() } + + switch (expectedResult, retrievedResult) { + case (.success(.none), .success(.none)), + (.failure, .failure): + break + + case let (.success(.some(expected)), .success(.some(retrieved))): + XCTAssertEqual(retrieved.feed, expected.feed, file: file, line: line) + XCTAssertEqual(retrieved.timestamp, expected.timestamp, file: file, line: line) - exp.fulfill() + default: + XCTFail("Expected to retrieve \(expectedResult), got \(retrievedResult) instead", file: file, line: line) } - - wait(for: [exp], timeout: 1.0) } } From b2425282c2e07261dd80ca83d458145eab7c6bf4 Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Tue, 1 Apr 2025 00:04:10 -0300 Subject: [PATCH 20/25] Make NullStore FeedStore implementation sync --- EssentialApp/EssentialApp/NullStore.swift | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/EssentialApp/EssentialApp/NullStore.swift b/EssentialApp/EssentialApp/NullStore.swift index 1ee5f05..629d313 100644 --- a/EssentialApp/EssentialApp/NullStore.swift +++ b/EssentialApp/EssentialApp/NullStore.swift @@ -9,17 +9,11 @@ import EssentialFeed class NullStore {} extension NullStore: FeedStore { - func deleteCachedFeed(completion: @escaping DeletionCompletion) { - completion(.success(())) - } + func deleteCachedFeed() throws {} - func insert(_ feed: [LocalFeedImage], timestamp: Date, completion: @escaping InsertionCompletion) { - completion(.success(())) - } + func insert(_ feed: [LocalFeedImage], timestamp: Date) throws {} - func retrieve(completion: @escaping RetrievalCompletion) { - completion(.success(.none)) - } + func retrieve() throws -> CachedFeed? { .none } } extension NullStore: FeedImageDataStore { From 2e239c312323dd2b2c5551963d33109f9870fab6 Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Tue, 1 Apr 2025 00:07:18 -0300 Subject: [PATCH 21/25] Make in memory FeedStore implementation sync --- .../EssentialAppTests/Helpers/InMemoryFeedStore.swift | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/EssentialApp/EssentialAppTests/Helpers/InMemoryFeedStore.swift b/EssentialApp/EssentialAppTests/Helpers/InMemoryFeedStore.swift index 05596ab..99e6250 100644 --- a/EssentialApp/EssentialAppTests/Helpers/InMemoryFeedStore.swift +++ b/EssentialApp/EssentialAppTests/Helpers/InMemoryFeedStore.swift @@ -16,18 +16,16 @@ class InMemoryFeedStore { } extension InMemoryFeedStore: FeedStore { - func deleteCachedFeed(completion: @escaping FeedStore.DeletionCompletion) { + func deleteCachedFeed() throws { feedCache = nil - completion(.success(())) } - func insert(_ feed: [LocalFeedImage], timestamp: Date, completion: @escaping FeedStore.InsertionCompletion) { + func insert(_ feed: [LocalFeedImage], timestamp: Date) throws { feedCache = CachedFeed(feed: feed, timestamp: timestamp) - completion(.success(())) } - func retrieve(completion: @escaping FeedStore.RetrievalCompletion) { - completion(.success(feedCache)) + func retrieve() throws -> CachedFeed? { + feedCache } } From db3b01f181b369713de14bc51ab7094b4fb7b1b7 Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Tue, 1 Apr 2025 00:10:26 -0300 Subject: [PATCH 22/25] Remove unused async methods --- .../EssentialFeed/Feed Cache/FeedStore.swift | 60 ------------------- .../XCTestCase+FeedStoreSpecs.swift | 4 +- 2 files changed, 2 insertions(+), 62 deletions(-) diff --git a/EssentialFeed/EssentialFeed/Feed Cache/FeedStore.swift b/EssentialFeed/EssentialFeed/Feed Cache/FeedStore.swift index 82c9795..db51e4b 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/FeedStore.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/FeedStore.swift @@ -11,64 +11,4 @@ public protocol FeedStore { func deleteCachedFeed() throws func insert(_ feed: [LocalFeedImage], timestamp: Date) throws func retrieve() throws -> CachedFeed? - - typealias DeletionResult = Result - typealias DeletionCompletion = (DeletionResult) -> Void - - typealias InsertionResult = Result - typealias InsertionCompletion = (InsertionResult) -> Void - - typealias RetrievalResult = Result - typealias RetrievalCompletion = (RetrievalResult) -> Void - - @available(*, deprecated) - func deleteCachedFeed(completion: @escaping DeletionCompletion) - - @available(*, deprecated) - func insert(_ feed: [LocalFeedImage], timestamp: Date, completion: @escaping InsertionCompletion) - - @available(*, deprecated) - func retrieve(completion: @escaping RetrievalCompletion) -} - -public extension FeedStore { - func deleteCachedFeed() throws { - let group = DispatchGroup() - group.enter() - var result: DeletionResult! - deleteCachedFeed { - result = $0 - group.leave() - } - group.wait() - return try result.get() - } - - func insert(_ feed: [LocalFeedImage], timestamp: Date) throws { - let group = DispatchGroup() - group.enter() - var result: InsertionResult! - insert(feed, timestamp: timestamp) { - result = $0 - group.leave() - } - group.wait() - return try result.get() - } - - func retrieve() throws -> CachedFeed? { - let group = DispatchGroup() - group.enter() - var result: RetrievalResult! - retrieve { - result = $0 - group.leave() - } - group.wait() - return try result.get() - } - - func deleteCachedFeed(completion: @escaping DeletionCompletion) {} - func insert(_ feed: [LocalFeedImage], timestamp: Date, completion: @escaping InsertionCompletion) {} - func retrieve(completion: @escaping RetrievalCompletion) {} } diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/FeedStoreSpecs/XCTestCase+FeedStoreSpecs.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/FeedStoreSpecs/XCTestCase+FeedStoreSpecs.swift index 3099ca1..55436b3 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/FeedStoreSpecs/XCTestCase+FeedStoreSpecs.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/FeedStoreSpecs/XCTestCase+FeedStoreSpecs.swift @@ -110,12 +110,12 @@ extension FeedStoreSpecs where Self: XCTestCase { } } - func expect(_ sut: FeedStore, toRetrieveTwice expectedResult: FeedStore.RetrievalResult, file: StaticString = #file, line: UInt = #line) { + func expect(_ sut: FeedStore, toRetrieveTwice expectedResult: Result, file: StaticString = #file, line: UInt = #line) { expect(sut, toRetrieve: expectedResult, file: file, line: line) expect(sut, toRetrieve: expectedResult, file: file, line: line) } - func expect(_ sut: FeedStore, toRetrieve expectedResult: FeedStore.RetrievalResult, file: StaticString = #file, line: UInt = #line) { + func expect(_ sut: FeedStore, toRetrieve expectedResult: Result, file: StaticString = #file, line: UInt = #line) { let retrievedResult = Result { try sut.retrieve() } switch (expectedResult, retrievedResult) { From 4f7bd8598a9b0789f5f8c65c3720ec054768066e Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Tue, 1 Apr 2025 00:15:55 -0300 Subject: [PATCH 23/25] Subscribe upstream store subscriptions in a background queue to avoid blocking the main queue (tests still run synchronously in the main queue with the immediate scheduler) --- EssentialApp/EssentialApp/SceneDelegate.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/EssentialApp/EssentialApp/SceneDelegate.swift b/EssentialApp/EssentialApp/SceneDelegate.swift index dae87ef..bc5177d 100644 --- a/EssentialApp/EssentialApp/SceneDelegate.swift +++ b/EssentialApp/EssentialApp/SceneDelegate.swift @@ -96,6 +96,7 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate { .caching(to: localFeedLoader) .fallback(to: localFeedLoader.loadPublisher) .map(makeFirstPage) + .subscribe(on: scheduler) .eraseToAnyPublisher() } @@ -104,8 +105,11 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate { .zip(makeRemoteFeedLoader(after: last)) .map { (cachedItems, newItems) in (cachedItems + newItems, newItems.last) - }.map(makePage) + } + .map(makePage) .caching(to: localFeedLoader) + .subscribe(on: scheduler) + .eraseToAnyPublisher() } private func makeRemoteFeedLoader(after: FeedImage? = nil) -> AnyPublisher<[FeedImage], Error> { From 420e8429a5ef83e7f9e59693ad2ef3586faa762d Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Tue, 1 Apr 2025 00:21:58 -0300 Subject: [PATCH 24/25] Increase coverage by checking errors even when save method doesn't throw an error. If the save method doesn't throw an error, the catch block is never executed, so the error assertion inside the catch block wouldn't execute. To ensure the error assertions always run, we need to move it outside the catch block. --- .../Feed Cache/CacheFeedUseCaseTests.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedUseCaseTests.swift b/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedUseCaseTests.swift index 607c474..22d8012 100644 --- a/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedUseCaseTests.swift +++ b/EssentialFeed/EssentialFeedTests/Feed Cache/CacheFeedUseCaseTests.swift @@ -76,11 +76,15 @@ class CacheFeedUseCaseTests: XCTestCase { private func expect(_ sut: LocalFeedLoader, toCompleteWithError expectedError: NSError?, when action: () -> Void, file: StaticString = #filePath, line: UInt = #line) { action() + var receivedError: NSError? + do { try sut.save(uniqueImageFeed().models) } catch { - XCTAssertEqual(error as NSError?, expectedError, file: file, line: line) + receivedError = error as NSError? } + + XCTAssertEqual(receivedError, expectedError, file: file, line: line) } } From d3a792befa0d505e9948cf70a7390f4f186d5dda Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Tue, 1 Apr 2025 00:28:55 -0300 Subject: [PATCH 25/25] Ensures all store operations run in the same scheduler Our Core Data store is thread-safe, so it can be safely used from any queue. However, we may want to perform store operations in a dedicated infra queue to avoid doing too much work in other queues. For example, the URLSessionHTTPClient sends results in its URLSession delegate queue. If we store the result received from the URLSessionHTTPClient in the same queue as we receive it, we may be doing too much work in the URLSession delegate queue - which could slow down the processing of other URLSessionHTTPClient results. To avoid this problem, we're now calling all store methods in a specific infra queue by using `receive(on: scheduler)` when performing a store operation after receiving results from another infra operation. --- EssentialApp/EssentialApp/SceneDelegate.swift | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/EssentialApp/EssentialApp/SceneDelegate.swift b/EssentialApp/EssentialApp/SceneDelegate.swift index bc5177d..6aad13c 100644 --- a/EssentialApp/EssentialApp/SceneDelegate.swift +++ b/EssentialApp/EssentialApp/SceneDelegate.swift @@ -69,10 +69,12 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate { } func sceneWillResignActive(_ scene: UIScene) { - do { - try localFeedLoader.validateCache() - } catch { - logger.error("Failed to validate cache with error: \(error.localizedDescription)") + scheduler.schedule { [localFeedLoader, logger] in + do { + try localFeedLoader.validateCache() + } catch { + logger.error("Failed to validate cache with error: \(error.localizedDescription)") + } } } @@ -93,10 +95,10 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate { private func makeRemoteFeedLoaderWithLocalFallback() -> AnyPublisher, Error> { makeRemoteFeedLoader() + .receive(on: scheduler) .caching(to: localFeedLoader) .fallback(to: localFeedLoader.loadPublisher) .map(makeFirstPage) - .subscribe(on: scheduler) .eraseToAnyPublisher() } @@ -107,6 +109,7 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate { (cachedItems + newItems, newItems.last) } .map(makePage) + .receive(on: scheduler) .caching(to: localFeedLoader) .subscribe(on: scheduler) .eraseToAnyPublisher() @@ -140,8 +143,8 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate { httpClient .getPublisher(url: url) .tryMap(FeedImageDataMapper.map) + .receive(on: scheduler) .caching(to: localImageLoader, using: url) - .subscribe(on: scheduler) .eraseToAnyPublisher() }) .subscribe(on: scheduler)