From a2bcffe29f367f0a3b4181e45cba4f4b9ec53351 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Sat, 1 Feb 2020 17:52:57 +0800 Subject: [PATCH 1/3] Fix the issue that WebImage's onSuccess does not get called when memory cache hit for new created View Struct --- SDWebImageSwiftUI/Classes/ImageManager.swift | 2 ++ SDWebImageSwiftUI/Classes/WebImage.swift | 11 +++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/SDWebImageSwiftUI/Classes/ImageManager.swift b/SDWebImageSwiftUI/Classes/ImageManager.swift index 6a752c29..df80d7d3 100644 --- a/SDWebImageSwiftUI/Classes/ImageManager.swift +++ b/SDWebImageSwiftUI/Classes/ImageManager.swift @@ -19,6 +19,7 @@ class ImageManager : ObservableObject { weak var currentOperation: SDWebImageOperation? = nil var isSuccess: Bool = false // true means request for this URL is ended forever, load() do nothing var isIncremental: Bool = false // true means during incremental loading + var isFirstLoad: Bool = true // false after first call `load()` var url: URL? var options: SDWebImageOptions @@ -39,6 +40,7 @@ class ImageManager : ObservableObject { } func load() { + isFirstLoad = false if currentOperation != nil { return } diff --git a/SDWebImageSwiftUI/Classes/WebImage.swift b/SDWebImageSwiftUI/Classes/WebImage.swift index e2f47ac2..a258e18e 100644 --- a/SDWebImageSwiftUI/Classes/WebImage.swift +++ b/SDWebImageSwiftUI/Classes/WebImage.swift @@ -58,13 +58,16 @@ public struct WebImage : View { } } self.imageManager = ImageManager(url: url, options: options, context: context) - // load remote image here, SwiftUI sometimes will create a new View struct without calling `onAppear` (like enter EditMode) :) - // this can ensure we load the image, SDWebImage take care of the duplicated query - self.imageManager.load() } public var body: some View { - Group { + // load remote image when first called `body`, SwiftUI sometimes will create a new View struct without calling `onAppear` (like enter EditMode) :) + // this can ensure we load the image, and display image synchronously when memory cache hit to avoid flashing + // called once per struct, SDWebImage take care of the duplicated query + if imageManager.isFirstLoad { + imageManager.load() + } + return Group { if imageManager.image != nil { if isAnimating && !self.imageManager.isIncremental { if currentFrame != nil { From 3363162b036bd9c8f7f7a55163111c0f6be5ea80 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Sat, 1 Feb 2020 17:53:28 +0800 Subject: [PATCH 2/3] Added the test case `testWebImageOnSuccessWhenMemoryCacheHit` --- SDWebImageSwiftUI.xcodeproj/project.pbxproj | 8 +++++++ Tests/AnimatedImageTests.swift | 24 +++------------------ Tests/TestUtils.swift | 18 ++++++++++++++++ Tests/WebImageTests.swift | 19 +++++++++++++++- 4 files changed, 47 insertions(+), 22 deletions(-) create mode 100644 Tests/TestUtils.swift diff --git a/SDWebImageSwiftUI.xcodeproj/project.pbxproj b/SDWebImageSwiftUI.xcodeproj/project.pbxproj index 48e3759c..0ec7543e 100644 --- a/SDWebImageSwiftUI.xcodeproj/project.pbxproj +++ b/SDWebImageSwiftUI.xcodeproj/project.pbxproj @@ -28,6 +28,9 @@ 321C1D6B23DEDB98009CF62A /* WebImageTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3211F84F23DE98E300FC757F /* WebImageTests.swift */; }; 321C1D6C23DEDB98009CF62A /* AnimatedImageTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3211F84623DE984D00FC757F /* AnimatedImageTests.swift */; }; 321C1D6D23DEDB98009CF62A /* WebImageTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3211F84F23DE98E300FC757F /* WebImageTests.swift */; }; + 322E0F4823E57F09006836DC /* TestUtils.swift in Sources */ = {isa = PBXBuildFile; fileRef = 322E0F4723E57F09006836DC /* TestUtils.swift */; }; + 322E0F4923E57F09006836DC /* TestUtils.swift in Sources */ = {isa = PBXBuildFile; fileRef = 322E0F4723E57F09006836DC /* TestUtils.swift */; }; + 322E0F4A23E57F09006836DC /* TestUtils.swift in Sources */ = {isa = PBXBuildFile; fileRef = 322E0F4723E57F09006836DC /* TestUtils.swift */; }; 326B84822363350C0011BDFB /* Indicator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 326B84812363350C0011BDFB /* Indicator.swift */; }; 326B84832363350C0011BDFB /* Indicator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 326B84812363350C0011BDFB /* Indicator.swift */; }; 326B84842363350C0011BDFB /* Indicator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 326B84812363350C0011BDFB /* Indicator.swift */; }; @@ -160,6 +163,7 @@ 3211F85423DE9D2700FC757F /* Images.bundle */ = {isa = PBXFileReference; lastKnownFileType = "wrapper.plug-in"; path = Images.bundle; sourceTree = ""; }; 321C1D3B23DEC17D009CF62A /* SDWebImageSwiftUITests macOS.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = "SDWebImageSwiftUITests macOS.xctest"; sourceTree = BUILT_PRODUCTS_DIR; }; 321C1D4A23DEC185009CF62A /* SDWebImageSwiftUITests tvOS.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = "SDWebImageSwiftUITests tvOS.xctest"; sourceTree = BUILT_PRODUCTS_DIR; }; + 322E0F4723E57F09006836DC /* TestUtils.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestUtils.swift; sourceTree = ""; }; 326B84812363350C0011BDFB /* Indicator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Indicator.swift; sourceTree = ""; }; 326B8486236335110011BDFB /* ActivityIndicator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ActivityIndicator.swift; sourceTree = ""; }; 326B848B236335400011BDFB /* ProgressIndicator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ProgressIndicator.swift; sourceTree = ""; }; @@ -259,6 +263,7 @@ 3211F84623DE984D00FC757F /* AnimatedImageTests.swift */, 3211F84F23DE98E300FC757F /* WebImageTests.swift */, 32BD9C4623E03B08008D5F6A /* IndicatorTests.swift */, + 322E0F4723E57F09006836DC /* TestUtils.swift */, ); path = Tests; sourceTree = ""; @@ -711,6 +716,7 @@ 3211F85023DE98E300FC757F /* WebImageTests.swift in Sources */, 32BD9C4723E03B08008D5F6A /* IndicatorTests.swift in Sources */, 3211F84723DE984D00FC757F /* AnimatedImageTests.swift in Sources */, + 322E0F4823E57F09006836DC /* TestUtils.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -721,6 +727,7 @@ 321C1D6B23DEDB98009CF62A /* WebImageTests.swift in Sources */, 32BD9C4823E03B08008D5F6A /* IndicatorTests.swift in Sources */, 321C1D6A23DEDB98009CF62A /* AnimatedImageTests.swift in Sources */, + 322E0F4923E57F09006836DC /* TestUtils.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -731,6 +738,7 @@ 321C1D6D23DEDB98009CF62A /* WebImageTests.swift in Sources */, 32BD9C4923E03B08008D5F6A /* IndicatorTests.swift in Sources */, 321C1D6C23DEDB98009CF62A /* AnimatedImageTests.swift in Sources */, + 322E0F4A23E57F09006836DC /* TestUtils.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/Tests/AnimatedImageTests.swift b/Tests/AnimatedImageTests.swift index 1c503883..59ea71a7 100644 --- a/Tests/AnimatedImageTests.swift +++ b/Tests/AnimatedImageTests.swift @@ -34,7 +34,7 @@ class AnimatedImageTests: XCTestCase { func testAnimatedImageWithName() throws { let expectation = self.expectation(description: "AnimatedImage name initializer") - let imageView = AnimatedImage(name: "TestImage.gif", bundle: testImageBundle()) + let imageView = AnimatedImage(name: "TestImage.gif", bundle: TestUtils.testImageBundle()) let introspectView = imageView.introspectAnimatedImage { animatedImageView in if let animatedImage = animatedImageView.image as? SDAnimatedImage { XCTAssertEqual(animatedImage.animatedImageLoopCount, 0) @@ -51,7 +51,7 @@ class AnimatedImageTests: XCTestCase { func testAnimatedImageWithData() throws { let expectation = self.expectation(description: "AnimatedImage data initializer") - let imageData = try XCTUnwrap(testImageData(name: "TestImageAnimated.apng")) + let imageData = try XCTUnwrap(TestUtils.testImageData(name: "TestImageAnimated.apng")) let imageView = AnimatedImage(data: imageData) let introspectView = imageView.introspectAnimatedImage { animatedImageView in if let animatedImage = animatedImageView.image as? SDAnimatedImage { @@ -91,7 +91,7 @@ class AnimatedImageTests: XCTestCase { let expectation = self.expectation(description: "AnimatedImage binding control") let binding = Binding(wrappedValue: true) var context: AnimatedImage.Context? - let imageView = AnimatedImage(name: "TestLoopCount.gif", bundle: testImageBundle(), isAnimating: binding) + let imageView = AnimatedImage(name: "TestLoopCount.gif", bundle: TestUtils.testImageBundle(), isAnimating: binding) .onViewCreate { _, c in context = c } @@ -171,22 +171,4 @@ class AnimatedImageTests: XCTestCase { self.waitForExpectations(timeout: 5, handler: nil) AnimatedImage.onViewDestroy() } - - // MARK: Helper - func testBundle() -> Bundle { - Bundle(for: type(of: self)) - } - - func testImageBundle() -> Bundle { - let imagePath = (testBundle().resourcePath! as NSString).appendingPathComponent("Images.bundle") - return Bundle(path: imagePath)! - } - - func testImageData(name: String) -> Data? { - guard let url = testImageBundle().url(forResource: name, withExtension: nil) else { - return nil - } - return try? Data(contentsOf: url) - } - } diff --git a/Tests/TestUtils.swift b/Tests/TestUtils.swift new file mode 100644 index 00000000..c8aa257c --- /dev/null +++ b/Tests/TestUtils.swift @@ -0,0 +1,18 @@ +import XCTest +import SwiftUI + +class TestUtils { + static var testBundle = Bundle(for: TestUtils.self) + + class func testImageBundle() -> Bundle { + let imagePath = (testBundle.resourcePath! as NSString).appendingPathComponent("Images.bundle") + return Bundle(path: imagePath)! + } + + class func testImageData(name: String) -> Data? { + guard let url = testImageBundle().url(forResource: name, withExtension: nil) else { + return nil + } + return try? Data(contentsOf: url) + } +} diff --git a/Tests/WebImageTests.swift b/Tests/WebImageTests.swift index 8f497b6f..4eef0edc 100644 --- a/Tests/WebImageTests.swift +++ b/Tests/WebImageTests.swift @@ -15,7 +15,6 @@ class WebImageTests: XCTestCase { override func tearDown() { // Put teardown code here. This method is called after the invocation of each test method in the class. super.tearDown() - SDImageCache.shared.clear(with: .all) } func testWebImageWithStaticURL() throws { @@ -107,4 +106,22 @@ class WebImageTests: XCTestCase { self.waitForExpectations(timeout: 5, handler: nil) } + func testWebImageOnSuccessWhenMemoryCacheHit() throws { + let expectation = self.expectation(description: "WebImage onSuccess when memory hit") + let imageUrl = URL(string: "https://foo.bar/buzz.png") + let cacheKey = SDWebImageManager.shared.cacheKey(for: imageUrl) + let testImage = UIImage(named: "TestImage", in: TestUtils.testImageBundle(), compatibleWith: nil) + SDImageCache.shared.storeImage(toMemory: testImage, forKey: cacheKey) + let imageView = WebImage(url: imageUrl) + let introspectView = imageView.onSuccess { image, cacheType in + XCTAssert(cacheType == .memory) + XCTAssertNotNil(image) + XCTAssertEqual(image, testImage) + expectation.fulfill() + } + _ = try introspectView.inspect(WebImage.self) + ViewHosting.host(view: introspectView) + self.waitForExpectations(timeout: 5, handler: nil) + } + } From f56c39aab2085c296e58abf89e9214ba789fffe9 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Sat, 1 Feb 2020 18:18:52 +0800 Subject: [PATCH 3/3] Fix the test case compile issue on macOS --- Tests/WebImageTests.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Tests/WebImageTests.swift b/Tests/WebImageTests.swift index 4eef0edc..91a2e418 100644 --- a/Tests/WebImageTests.swift +++ b/Tests/WebImageTests.swift @@ -110,7 +110,11 @@ class WebImageTests: XCTestCase { let expectation = self.expectation(description: "WebImage onSuccess when memory hit") let imageUrl = URL(string: "https://foo.bar/buzz.png") let cacheKey = SDWebImageManager.shared.cacheKey(for: imageUrl) + #if os(macOS) + let testImage = TestUtils.testImageBundle().image(forResource: "TestImage") + #else let testImage = UIImage(named: "TestImage", in: TestUtils.testImageBundle(), compatibleWith: nil) + #endif SDImageCache.shared.storeImage(toMemory: testImage, forKey: cacheKey) let imageView = WebImage(url: imageUrl) let introspectView = imageView.onSuccess { image, cacheType in