From 492375a4eb3c198270be24518eb8e8b7784e15f2 Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Tue, 25 Mar 2025 23:54:10 -0300 Subject: [PATCH 01/10] Rename file --- EssentialFeed/EssentialFeed.xcodeproj/project.pbxproj | 8 ++++---- ...r.swift => CoreDataFeedStore+FeedImageDataStore.swift} | 0 2 files changed, 4 insertions(+), 4 deletions(-) rename EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/{CoreDataFeedStore+FeedImageDataLoader.swift => CoreDataFeedStore+FeedImageDataStore.swift} (100%) diff --git a/EssentialFeed/EssentialFeed.xcodeproj/project.pbxproj b/EssentialFeed/EssentialFeed.xcodeproj/project.pbxproj index 78300dd..2d7d020 100644 --- a/EssentialFeed/EssentialFeed.xcodeproj/project.pbxproj +++ b/EssentialFeed/EssentialFeed.xcodeproj/project.pbxproj @@ -129,7 +129,7 @@ 5BC4F6CB2CDAF0B20002D4CF /* CoreDataHelpers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5BC4F6CA2CDAF0B20002D4CF /* CoreDataHelpers.swift */; }; 5BC4F6CD2CDAF1B30002D4CF /* ManagedCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5BC4F6CC2CDAF1B30002D4CF /* ManagedCache.swift */; }; 5BC4F6CF2CDAF1C60002D4CF /* ManagedFeedImage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5BC4F6CE2CDAF1C60002D4CF /* ManagedFeedImage.swift */; }; - 5BDE3C652D6C19D8005D520D /* CoreDataFeedStore+FeedImageDataLoader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5BDE3C642D6C19D8005D520D /* CoreDataFeedStore+FeedImageDataLoader.swift */; }; + 5BDE3C652D6C19D8005D520D /* CoreDataFeedStore+FeedImageDataStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5BDE3C642D6C19D8005D520D /* CoreDataFeedStore+FeedImageDataStore.swift */; }; 5BDE3C672D6C225A005D520D /* CoreDataFeedStore+FeedStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5BDE3C662D6C225A005D520D /* CoreDataFeedStore+FeedStore.swift */; }; 5BE36BA62CD5845700ACC57C /* FeedCachePolicy.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5BE36BA52CD5845700ACC57C /* FeedCachePolicy.swift */; }; 5BF9F2F92CD9961400C8DB96 /* FeedStoreSpecs.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5BF9F2F82CD9961400C8DB96 /* FeedStoreSpecs.swift */; }; @@ -313,7 +313,7 @@ 5BC4F6CC2CDAF1B30002D4CF /* ManagedCache.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ManagedCache.swift; sourceTree = ""; }; 5BC4F6CE2CDAF1C60002D4CF /* ManagedFeedImage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ManagedFeedImage.swift; sourceTree = ""; }; 5BDE3C632D6C1672005D520D /* FeedStore2.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = FeedStore2.xcdatamodel; sourceTree = ""; }; - 5BDE3C642D6C19D8005D520D /* CoreDataFeedStore+FeedImageDataLoader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CoreDataFeedStore+FeedImageDataLoader.swift"; sourceTree = ""; }; + 5BDE3C642D6C19D8005D520D /* CoreDataFeedStore+FeedImageDataStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CoreDataFeedStore+FeedImageDataStore.swift"; sourceTree = ""; }; 5BDE3C662D6C225A005D520D /* CoreDataFeedStore+FeedStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CoreDataFeedStore+FeedStore.swift"; sourceTree = ""; }; 5BE36BA52CD5845700ACC57C /* FeedCachePolicy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedCachePolicy.swift; sourceTree = ""; }; 5BF9F2F82CD9961400C8DB96 /* FeedStoreSpecs.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedStoreSpecs.swift; sourceTree = ""; }; @@ -890,7 +890,7 @@ children = ( 5BF9F3092CDAD24D00C8DB96 /* CoreDataFeedStore.swift */, 5BDE3C662D6C225A005D520D /* CoreDataFeedStore+FeedStore.swift */, - 5BDE3C642D6C19D8005D520D /* CoreDataFeedStore+FeedImageDataLoader.swift */, + 5BDE3C642D6C19D8005D520D /* CoreDataFeedStore+FeedImageDataStore.swift */, 5BC4F6CA2CDAF0B20002D4CF /* CoreDataHelpers.swift */, 5BC4F6CC2CDAF1B30002D4CF /* ManagedCache.swift */, 5BC4F6CE2CDAF1C60002D4CF /* ManagedFeedImage.swift */, @@ -1213,7 +1213,7 @@ 5B88290A2D6A7B76006E0BD7 /* ResourceErrorViewModel.swift in Sources */, 5B8829142D6BAE59006E0BD7 /* FeedImageDataLoader.swift in Sources */, 5B8829082D6A7B12006E0BD7 /* ResourceLoadingViewModel.swift in Sources */, - 5BDE3C652D6C19D8005D520D /* CoreDataFeedStore+FeedImageDataLoader.swift in Sources */, + 5BDE3C652D6C19D8005D520D /* CoreDataFeedStore+FeedImageDataStore.swift in Sources */, 5B88290F2D6A94C3006E0BD7 /* FeedImagePresenter.swift in Sources */, 5BBDA00E2D6FCCF000D68DF0 /* FeedCache.swift in Sources */, 5BC4F6CF2CDAF1C60002D4CF /* ManagedFeedImage.swift in Sources */, diff --git a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedImageDataLoader.swift b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedImageDataStore.swift similarity index 100% rename from EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedImageDataLoader.swift rename to EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedImageDataStore.swift From 2d2a40c2f698d07a0af7a772c6c31ed67850d92f Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Tue, 25 Mar 2025 23:57:23 -0300 Subject: [PATCH 02/10] Remove duplication --- .../CoreData/CoreDataFeedStore+FeedStore.swift | 2 +- .../Feed Cache/Infrastructure/CoreData/ManagedCache.swift | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedStore.swift b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedStore.swift index b3e1b56..a05127f 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedStore.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedStore.swift @@ -31,7 +31,7 @@ extension CoreDataFeedStore: FeedStore { public func deleteCachedFeed(completion: @escaping DeletionCompletion) { perform { context in completion(Result { - try ManagedCache.find(in: context).map(context.delete).map(context.save) + try ManagedCache.deleteCache(in: context) }) } } diff --git a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/ManagedCache.swift b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/ManagedCache.swift index 0063fa1..318b383 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/ManagedCache.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/ManagedCache.swift @@ -18,8 +18,12 @@ extension ManagedCache { return try context.fetch(request).first } + static func deleteCache(in context: NSManagedObjectContext) throws { + try find(in: context).map(context.delete).map(context.save) + } + static func newUniqueInstance(in context: NSManagedObjectContext) throws -> ManagedCache { - try find(in: context).map(context.delete) + try deleteCache(in: context) return ManagedCache(context: context) } From 135bf7d51a5c8a87c93abd70f822a4b87aa200fd Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Tue, 25 Mar 2025 23:59:20 -0300 Subject: [PATCH 03/10] Add helper to find image data for a given URL --- .../CoreData/CoreDataFeedStore+FeedImageDataStore.swift | 2 +- .../Feed Cache/Infrastructure/CoreData/ManagedFeedImage.swift | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedImageDataStore.swift b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedImageDataStore.swift index c11c1da..1844930 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedImageDataStore.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/CoreDataFeedStore+FeedImageDataStore.swift @@ -20,7 +20,7 @@ extension CoreDataFeedStore: FeedImageDataStore { public func retrieve(dataForURL url: URL, completion: @escaping (FeedImageDataStore.RetrievalResult) -> Void) { perform { context in completion(Result { - try ManagedFeedImage.first(with: url, in: context)?.data + try ManagedFeedImage.data(with: url, in: context) }) } } diff --git a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/ManagedFeedImage.swift b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/ManagedFeedImage.swift index bd44501..4f8fe7e 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/ManagedFeedImage.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/ManagedFeedImage.swift @@ -16,6 +16,10 @@ class ManagedFeedImage: NSManagedObject { } extension ManagedFeedImage { + static func data(with url: URL, in context: NSManagedObjectContext) throws -> Data? { + return try first(with: url, in: context)?.data + } + static func first(with url: URL, in context: NSManagedObjectContext) throws -> ManagedFeedImage? { let request = NSFetchRequest(entityName: entity().name!) request.predicate = NSPredicate(format: "%K = %@", argumentArray: [#keyPath(ManagedFeedImage.url), url]) From 5510b21d8d3aeb473bd7c39c603dd82496aaecf5 Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Wed, 26 Mar 2025 00:02:50 -0300 Subject: [PATCH 04/10] Use `NullStore` when we can't create a `CoreDataFeedStore` instance --- EssentialApp/EssentialApp/NullStore.swift | 29 +++++++++++++++++++ EssentialApp/EssentialApp/SceneDelegate.swift | 12 +++++--- 2 files changed, 37 insertions(+), 4 deletions(-) create mode 100644 EssentialApp/EssentialApp/NullStore.swift diff --git a/EssentialApp/EssentialApp/NullStore.swift b/EssentialApp/EssentialApp/NullStore.swift new file mode 100644 index 0000000..2db00b1 --- /dev/null +++ b/EssentialApp/EssentialApp/NullStore.swift @@ -0,0 +1,29 @@ +// +// Created by Rodrigo Porto. +// Copyright © 2025 PortoCode. All Rights Reserved. +// + +import Foundation +import EssentialFeed + +class NullStore: FeedStore & FeedImageDataStore { + func deleteCachedFeed(completion: @escaping DeletionCompletion) { + completion(.success(())) + } + + func insert(_ feed: [LocalFeedImage], timestamp: Date, completion: @escaping InsertionCompletion) { + completion(.success(())) + } + + func retrieve(completion: @escaping RetrievalCompletion) { + completion(.success(.none)) + } + + func insert(_ data: Data, for url: URL, completion: @escaping (InsertionResult) -> Void) { + completion(.success(())) + } + + func retrieve(dataForURL url: URL, completion: @escaping (FeedImageDataStore.RetrievalResult) -> Void) { + completion(.success(.none)) + } +} diff --git a/EssentialApp/EssentialApp/SceneDelegate.swift b/EssentialApp/EssentialApp/SceneDelegate.swift index 72ef3eb..997dd45 100644 --- a/EssentialApp/EssentialApp/SceneDelegate.swift +++ b/EssentialApp/EssentialApp/SceneDelegate.swift @@ -16,10 +16,14 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate { }() private lazy var store: FeedStore & FeedImageDataStore = { - try! CoreDataFeedStore( - storeURL: NSPersistentContainer - .defaultDirectoryURL() - .appendingPathComponent("feed-store.sqlite")) + do { + return try CoreDataFeedStore( + storeURL: NSPersistentContainer + .defaultDirectoryURL() + .appendingPathComponent("feed-store.sqlite")) + } catch { + return NullStore() + } }() private lazy var localFeedLoader: LocalFeedLoader = { From 6490eeed891fa56ede24a4f44e6e0d7e49fc77a8 Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Wed, 26 Mar 2025 00:04:04 -0300 Subject: [PATCH 05/10] Add `assertionFailure` to catch CoreData issues in Debug builds --- EssentialApp/EssentialApp/SceneDelegate.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/EssentialApp/EssentialApp/SceneDelegate.swift b/EssentialApp/EssentialApp/SceneDelegate.swift index 997dd45..57a8be2 100644 --- a/EssentialApp/EssentialApp/SceneDelegate.swift +++ b/EssentialApp/EssentialApp/SceneDelegate.swift @@ -22,6 +22,7 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate { .defaultDirectoryURL() .appendingPathComponent("feed-store.sqlite")) } catch { + assertionFailure("Failed to instantiate CoreData store with error: \(error.localizedDescription)") return NullStore() } }() From 151583c80ea22b31f85cf6d6c8d3806235682146 Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Wed, 26 Mar 2025 00:15:42 -0300 Subject: [PATCH 06/10] Log fault when we can't create a `CoreDataFeedStore` instance --- EssentialApp/EssentialApp/SceneDelegate.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/EssentialApp/EssentialApp/SceneDelegate.swift b/EssentialApp/EssentialApp/SceneDelegate.swift index 57a8be2..406cf2f 100644 --- a/EssentialApp/EssentialApp/SceneDelegate.swift +++ b/EssentialApp/EssentialApp/SceneDelegate.swift @@ -3,6 +3,7 @@ // Copyright © 2025 PortoCode. All Rights Reserved. // +import os import UIKit import CoreData import Combine @@ -15,6 +16,8 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate { URLSessionHTTPClient(session: URLSession(configuration: .ephemeral)) }() + private lazy var logger = Logger(subsystem: "portocode.EssentialApp", category: "main") + private lazy var store: FeedStore & FeedImageDataStore = { do { return try CoreDataFeedStore( @@ -23,6 +26,7 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate { .appendingPathComponent("feed-store.sqlite")) } catch { assertionFailure("Failed to instantiate CoreData store with error: \(error.localizedDescription)") + logger.fault("Failed to instantiate CoreData store with error: \(error.localizedDescription)") return NullStore() } }() From 98c6405ea837b45c9af3eefd7fa5adfccbe7e4eb Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Wed, 26 Mar 2025 21:35:30 -0300 Subject: [PATCH 07/10] Optimize CoreData implementation to reduce cache misses --- .../Infrastructure/CoreData/ManagedFeedImage.swift | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/ManagedFeedImage.swift b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/ManagedFeedImage.swift index 4f8fe7e..4064fb9 100644 --- a/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/ManagedFeedImage.swift +++ b/EssentialFeed/EssentialFeed/Feed Cache/Infrastructure/CoreData/ManagedFeedImage.swift @@ -17,6 +17,8 @@ class ManagedFeedImage: NSManagedObject { extension ManagedFeedImage { static func data(with url: URL, in context: NSManagedObjectContext) throws -> Data? { + if let data = context.userInfo[url] as? Data { return data } + return try first(with: url, in: context)?.data } @@ -29,17 +31,26 @@ extension ManagedFeedImage { } static func images(from localFeed: [LocalFeedImage], in context: NSManagedObjectContext) -> NSOrderedSet { - return NSOrderedSet(array: localFeed.map { local in + let images = NSOrderedSet(array: localFeed.map { local in let managed = ManagedFeedImage(context: context) managed.id = local.id managed.imageDescription = local.description managed.location = local.location managed.url = local.url + managed.data = context.userInfo[local.url] as? Data return managed }) + context.userInfo.removeAllObjects() + return images } var local: LocalFeedImage { return LocalFeedImage(id: id, description: imageDescription, location: location, url: url) } + + override func prepareForDeletion() { + super.prepareForDeletion() + + managedObjectContext?.userInfo[url] = data + } } From 79618eb1ad35d8a75e78069b8045d6d51bbe1a80 Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Wed, 26 Mar 2025 22:13:38 -0300 Subject: [PATCH 08/10] Prevent loading resources again until a running request completes --- .../LoadResourcePresentationAdapter.swift | 9 ++++++ .../CommentsUIIntegrationTests.swift | 6 ++++ .../FeedUIIntegrationTests.swift | 30 +++++++++++++++++-- .../FeedUIIntegrationTests+LoaderSpy.swift | 1 + 4 files changed, 44 insertions(+), 2 deletions(-) diff --git a/EssentialApp/EssentialApp/LoadResourcePresentationAdapter.swift b/EssentialApp/EssentialApp/LoadResourcePresentationAdapter.swift index 744ea3b..53282ab 100644 --- a/EssentialApp/EssentialApp/LoadResourcePresentationAdapter.swift +++ b/EssentialApp/EssentialApp/LoadResourcePresentationAdapter.swift @@ -10,6 +10,7 @@ import EssentialFeediOS final class LoadResourcePresentationAdapter { private let loader: () -> AnyPublisher private var cancellable: Cancellable? + private var isLoading = false var presenter: LoadResourcePresenter? init(loader: @escaping () -> AnyPublisher) { @@ -17,10 +18,16 @@ final class LoadResourcePresentationAdapter { } func loadResource() { + guard !isLoading else { return } + presenter?.didStartLoading() + isLoading = true cancellable = loader() .dispatchOnMainQueue() + .handleEvents(receiveCancel: { [weak self] in + self?.isLoading = false + }) .sink( receiveCompletion: { [weak self] completion in switch completion { @@ -29,6 +36,8 @@ final class LoadResourcePresentationAdapter { case let .failure(error): self?.presenter?.didFinishLoading(with: error) } + + self?.isLoading = false }, receiveValue: { [weak self] resource in self?.presenter?.didFinishLoading(with: resource) }) diff --git a/EssentialApp/EssentialAppTests/CommentsUIIntegrationTests.swift b/EssentialApp/EssentialAppTests/CommentsUIIntegrationTests.swift index 821b554..1e1c9ef 100644 --- a/EssentialApp/EssentialAppTests/CommentsUIIntegrationTests.swift +++ b/EssentialApp/EssentialAppTests/CommentsUIIntegrationTests.swift @@ -27,9 +27,14 @@ class CommentsUIIntegrationTests: XCTestCase { sut.simulateAppearance() XCTAssertEqual(loader.loadCommentsCallCount, 1, "Expected a loading request once view appears") + sut.simulateUserInitiatedReload() + XCTAssertEqual(loader.loadCommentsCallCount, 1, "Expected no request until previous completes") + + loader.completeCommentsLoading(at: 0) sut.simulateUserInitiatedReload() XCTAssertEqual(loader.loadCommentsCallCount, 2, "Expected another loading request once user initiates a reload") + loader.completeCommentsLoading(at: 1) sut.simulateUserInitiatedReload() XCTAssertEqual(loader.loadCommentsCallCount, 3, "Expected yet another loading request once user initiates another reload") } @@ -205,6 +210,7 @@ class CommentsUIIntegrationTests: XCTestCase { func completeCommentsLoading(with comments: [ImageComment] = [], at index: Int = 0) { requests[index].send(comments) + requests[index].send(completion: .finished) } func completeCommentsLoadingWithError(at index: Int = 0) { diff --git a/EssentialApp/EssentialAppTests/FeedUIIntegrationTests.swift b/EssentialApp/EssentialAppTests/FeedUIIntegrationTests.swift index 5420a6d..1602bcf 100644 --- a/EssentialApp/EssentialAppTests/FeedUIIntegrationTests.swift +++ b/EssentialApp/EssentialAppTests/FeedUIIntegrationTests.swift @@ -37,14 +37,19 @@ class FeedUIIntegrationTests: XCTestCase { func test_loadFeedActions_requestFeedFromLoader() { let (sut, loader) = makeSUT() - XCTAssertEqual(loader.loadFeedCallCount, 0, "Expected no loading requests before view appears") + XCTAssertEqual(loader.loadFeedCallCount, 0, "Expected no loading requests before view is loaded") sut.simulateAppearance() - XCTAssertEqual(loader.loadFeedCallCount, 1, "Expected a loading request once view appears") + XCTAssertEqual(loader.loadFeedCallCount, 1, "Expected a loading request once view is loaded") + sut.simulateUserInitiatedReload() + XCTAssertEqual(loader.loadFeedCallCount, 1, "Expected no request until previous completes") + + loader.completeFeedLoading(at: 0) sut.simulateUserInitiatedReload() XCTAssertEqual(loader.loadFeedCallCount, 2, "Expected another loading request once user initiates a reload") + loader.completeFeedLoading(at: 1) sut.simulateUserInitiatedReload() XCTAssertEqual(loader.loadFeedCallCount, 3, "Expected yet another loading request once user initiates another reload") } @@ -531,6 +536,27 @@ class FeedUIIntegrationTests: XCTestCase { wait(for: [exp], timeout: 1.0) } + func test_feedImageView_doesNotLoadImageAgainUntilPreviousRequestCompletes() { + let image = makeImage(url: URL(string: "http://url-0.com")!) + let (sut, loader) = makeSUT() + sut.simulateAppearance() + loader.completeFeedLoading(with: [image]) + + sut.simulateFeedImageViewNearVisible(at: 0) + XCTAssertEqual(loader.loadedImageURLs, [image.url], "Expected first request when near visible") + + sut.simulateFeedImageViewVisible(at: 0) + XCTAssertEqual(loader.loadedImageURLs, [image.url], "Expected no request until previous completes") + + loader.completeImageLoading(at: 0) + sut.simulateFeedImageViewVisible(at: 0) + XCTAssertEqual(loader.loadedImageURLs, [image.url, image.url], "Expected second request when visible after previous complete") + + sut.simulateFeedImageViewNotVisible(at: 0) + sut.simulateFeedImageViewVisible(at: 0) + XCTAssertEqual(loader.loadedImageURLs, [image.url, image.url, image.url], "Expected third request when visible after canceling previous complete") + } + // MARK: - Helpers private func makeSUT( diff --git a/EssentialApp/EssentialAppTests/Helpers/FeedUIIntegrationTests+LoaderSpy.swift b/EssentialApp/EssentialAppTests/Helpers/FeedUIIntegrationTests+LoaderSpy.swift index 49e4cb5..41de2a1 100644 --- a/EssentialApp/EssentialAppTests/Helpers/FeedUIIntegrationTests+LoaderSpy.swift +++ b/EssentialApp/EssentialAppTests/Helpers/FeedUIIntegrationTests+LoaderSpy.swift @@ -30,6 +30,7 @@ extension FeedUIIntegrationTests { feedRequests[index].send(Paginated(items: feed, loadMorePublisher: { [weak self] in self?.loadMorePublisher() ?? Empty().eraseToAnyPublisher() })) + feedRequests[index].send(completion: .finished) } func completeFeedLoadingWithError(at index: Int = 0) { From 624b391dc9fb156d3a2a4b17955a99c898eb220f Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Wed, 26 Mar 2025 23:22:56 -0300 Subject: [PATCH 09/10] Reuse existing cell controllers to avoid unnecessary reallocation of resources --- .../EssentialApp/FeedViewAdapter.swift | 26 +++++++++++++++---- .../FeedUIIntegrationTests.swift | 5 ++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/EssentialApp/EssentialApp/FeedViewAdapter.swift b/EssentialApp/EssentialApp/FeedViewAdapter.swift index 8116a9a..55e0787 100644 --- a/EssentialApp/EssentialApp/FeedViewAdapter.swift +++ b/EssentialApp/EssentialApp/FeedViewAdapter.swift @@ -11,18 +11,27 @@ final class FeedViewAdapter: ResourceView { private weak var controller: ListViewController? private let imageLoader: (URL) -> FeedImageDataLoader.Publisher private let selection: (FeedImage) -> Void + private let currentFeed: [FeedImage: CellController] private typealias ImageDataPresentationAdapter = LoadResourcePresentationAdapter> private typealias LoadMorePresentationAdapter = LoadResourcePresentationAdapter, FeedViewAdapter> - init(controller: ListViewController? = nil, imageLoader: @escaping (URL) -> FeedImageDataLoader.Publisher, selection: @escaping (FeedImage) -> Void) { + init(currentFeed: [FeedImage: CellController] = [:], controller: ListViewController? = nil, imageLoader: @escaping (URL) -> FeedImageDataLoader.Publisher, selection: @escaping (FeedImage) -> Void) { + self.currentFeed = currentFeed self.controller = controller self.imageLoader = imageLoader self.selection = selection } func display(_ viewModel: Paginated) { + guard let controller = controller else { return } + + var currentFeed = self.currentFeed let feed: [CellController] = viewModel.items.map { model in + if let controller = currentFeed[model] { + return controller + } + let adapter = ImageDataPresentationAdapter(loader: { [imageLoader] in imageLoader(model.url) }) @@ -40,11 +49,13 @@ final class FeedViewAdapter: ResourceView { errorView: WeakRefVirtualProxy(view), mapper: UIImage.tryMake) - return CellController(id: model, view) + let controller = CellController(id: model, view) + currentFeed[model] = controller + return controller } guard let loadMorePublisher = viewModel.loadMorePublisher else { - controller?.display(feed) + controller.display(feed) return } @@ -52,13 +63,18 @@ final class FeedViewAdapter: ResourceView { let loadMore = LoadMoreCellController(callback: loadMoreAdapter.loadResource) loadMoreAdapter.presenter = LoadResourcePresenter( - resourceView: self, + resourceView: FeedViewAdapter( + currentFeed: currentFeed, + controller: controller, + imageLoader: imageLoader, + selection: selection + ), loadingView: WeakRefVirtualProxy(loadMore), errorView: WeakRefVirtualProxy(loadMore)) let loadMoreSection = [CellController(id: UUID(), loadMore)] - controller?.display(feed, loadMoreSection) + controller.display(feed, loadMoreSection) } } diff --git a/EssentialApp/EssentialAppTests/FeedUIIntegrationTests.swift b/EssentialApp/EssentialAppTests/FeedUIIntegrationTests.swift index 1602bcf..df95ae1 100644 --- a/EssentialApp/EssentialAppTests/FeedUIIntegrationTests.swift +++ b/EssentialApp/EssentialAppTests/FeedUIIntegrationTests.swift @@ -555,6 +555,11 @@ class FeedUIIntegrationTests: XCTestCase { sut.simulateFeedImageViewNotVisible(at: 0) sut.simulateFeedImageViewVisible(at: 0) XCTAssertEqual(loader.loadedImageURLs, [image.url, image.url, image.url], "Expected third request when visible after canceling previous complete") + + sut.simulateLoadMoreFeedAction() + loader.completeLoadMore(with: [image, makeImage()]) + sut.simulateFeedImageViewVisible(at: 0) + XCTAssertEqual(loader.loadedImageURLs, [image.url, image.url, image.url], "Expected no request until previous completes") } // MARK: - Helpers From 591719b538775d29fef8eec0e60fb3dbb19e9133 Mon Sep 17 00:00:00 2001 From: Rodrigo Porto Date: Wed, 26 Mar 2025 23:53:08 -0300 Subject: [PATCH 10/10] Fix for iOS 14.5 prefetching behavior change --- .../FeedUIIntegrationTests.swift | 21 +++++++++++++++++++ .../Controllers/FeedImageCellController.swift | 2 ++ 2 files changed, 23 insertions(+) diff --git a/EssentialApp/EssentialAppTests/FeedUIIntegrationTests.swift b/EssentialApp/EssentialAppTests/FeedUIIntegrationTests.swift index df95ae1..8698061 100644 --- a/EssentialApp/EssentialAppTests/FeedUIIntegrationTests.swift +++ b/EssentialApp/EssentialAppTests/FeedUIIntegrationTests.swift @@ -509,6 +509,27 @@ class FeedUIIntegrationTests: XCTestCase { XCTAssertEqual(view0?.isShowingImageLoadingIndicator, false, "Expected no loading indicator when image loads successfully after view becomes visible again") } + func test_feedImageView_configuresViewCorrectlyWhenTransitioningFromNearVisibleToVisibleWhileStillPreloadingImage() { + let (sut, loader) = makeSUT() + + sut.simulateAppearance() + loader.completeFeedLoading(with: [makeImage()]) + + sut.simulateFeedImageViewNearVisible(at: 0) + let view0 = sut.simulateFeedImageViewVisible(at: 0) + + XCTAssertEqual(view0?.renderedImage, nil, "Expected no rendered image when view becomes visible while still preloading image") + XCTAssertEqual(view0?.isShowingRetryAction, false, "Expected no retry action when view becomes visible while still preloading image") + XCTAssertEqual(view0?.isShowingImageLoadingIndicator, true, "Expected loading indicator when view becomes visible while still preloading image") + + let imageData = UIImage.make(withColor: .red).pngData()! + loader.completeImageLoading(with: imageData, at: 0) + + XCTAssertEqual(view0?.renderedImage, imageData, "Expected rendered image after image preloads successfully") + XCTAssertEqual(view0?.isShowingRetryAction, false, "Expected no retry action after image preloads successfully") + XCTAssertEqual(view0?.isShowingImageLoadingIndicator, false, "Expected no loading indicator after image preloads successfully") + } + func test_feedImageView_doesNotRenderLoadedImageWhenNotVisibleAnymore() { let (sut, loader) = makeSUT() diff --git a/EssentialFeed/EssentialFeediOS/Feed UI/Controllers/FeedImageCellController.swift b/EssentialFeed/EssentialFeediOS/Feed UI/Controllers/FeedImageCellController.swift index cc8934c..001c1f9 100644 --- a/EssentialFeed/EssentialFeediOS/Feed UI/Controllers/FeedImageCellController.swift +++ b/EssentialFeed/EssentialFeediOS/Feed UI/Controllers/FeedImageCellController.swift @@ -35,6 +35,8 @@ extension FeedImageCellController: UITableViewDataSource, UITableViewDelegate, U cell?.locationLabel.text = viewModel.location cell?.descriptionLabel.text = viewModel.description cell?.feedImageView.image = nil + cell?.feedImageContainer.isShimmering = true + cell?.feedImageRetryButton.isHidden = true cell?.onRetry = { [weak self] in self?.delegate.didRequestImage() }