Skip to content

Commit

Permalink
Merge pull request #77 from SDWebImage/bugfix_onSuccess_when_memory_c…
Browse files Browse the repository at this point in the history
…ache_hit

Fix the issue that WebImage's onSuccess does not get called when memory cache hit for new created View Struct
  • Loading branch information
dreampiggy authored Feb 1, 2020
2 parents 4336985 + f56c39a commit 7891347
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 26 deletions.
8 changes: 8 additions & 0 deletions SDWebImageSwiftUI.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -160,6 +163,7 @@
3211F85423DE9D2700FC757F /* Images.bundle */ = {isa = PBXFileReference; lastKnownFileType = "wrapper.plug-in"; path = Images.bundle; sourceTree = "<group>"; };
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 = "<group>"; };
326B84812363350C0011BDFB /* Indicator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Indicator.swift; sourceTree = "<group>"; };
326B8486236335110011BDFB /* ActivityIndicator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ActivityIndicator.swift; sourceTree = "<group>"; };
326B848B236335400011BDFB /* ProgressIndicator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ProgressIndicator.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -259,6 +263,7 @@
3211F84623DE984D00FC757F /* AnimatedImageTests.swift */,
3211F84F23DE98E300FC757F /* WebImageTests.swift */,
32BD9C4623E03B08008D5F6A /* IndicatorTests.swift */,
322E0F4723E57F09006836DC /* TestUtils.swift */,
);
path = Tests;
sourceTree = "<group>";
Expand Down Expand Up @@ -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;
};
Expand All @@ -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;
};
Expand All @@ -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;
};
Expand Down
2 changes: 2 additions & 0 deletions SDWebImageSwiftUI/Classes/ImageManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -39,6 +40,7 @@ class ImageManager : ObservableObject {
}

func load() {
isFirstLoad = false
if currentOperation != nil {
return
}
Expand Down
11 changes: 7 additions & 4 deletions SDWebImageSwiftUI/Classes/WebImage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
24 changes: 3 additions & 21 deletions Tests/AnimatedImageTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -91,7 +91,7 @@ class AnimatedImageTests: XCTestCase {
let expectation = self.expectation(description: "AnimatedImage binding control")
let binding = Binding<Bool>(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
}
Expand Down Expand Up @@ -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)
}

}
18 changes: 18 additions & 0 deletions Tests/TestUtils.swift
Original file line number Diff line number Diff line change
@@ -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)
}
}
23 changes: 22 additions & 1 deletion Tests/WebImageTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -107,4 +106,26 @@ 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)
#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
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)
}

}

0 comments on commit 7891347

Please sign in to comment.