Merge pull request #3866 from dreampiggy/bugfix/thumbnail_query_store_data

Fix the issue that thumbnail decoding will store the fulll image data into thumbnail key, which effect next time query from disk

SDWebImage/Core/SDImageCache.h

@@ -263,6 +263,7 @@ typedef NS_OPTIONS(NSUInteger, SDImageCacheOptions) {
  *
  * @param imageData  The image data to store
  * @param key        The unique image cache key, usually it's image absolute URL
+ * @note In history, when disk cache hit, the image will write back into memory cache, which is mostly unwanted behavior. From new version this behavior is removed.
  */
 - (void)storeImageDataToDisk:(nullable NSData *)imageData
                       forKey:(nullable NSString *)key;
@@ -312,6 +313,7 @@ typedef NS_OPTIONS(NSUInteger, SDImageCacheOptions) {
  * @param doneBlock The completion block. Will not get called if the operation is cancelled
  *
  * @return a SDImageCacheToken instance containing the cache operation, will callback immediately when cancelled
+ * @note In history, when disk cache hit, the image will write back into memory cache, which is mostly unwanted behavior. From new version this behavior is removed.
  */
 - (nullable SDImageCacheToken *)queryCacheOperationForKey:(nullable NSString *)key done:(nullable SDImageCacheQueryCompletionBlock)doneBlock;
 
@@ -324,6 +326,7 @@ typedef NS_OPTIONS(NSUInteger, SDImageCacheOptions) {
  *
  * @return a SDImageCacheToken instance containing the cache operation, will callback immediately when cancelled
  * @warning If you query with thumbnail cache key, you'd better not pass the thumbnail pixel size context, which is undefined behavior.
+ * @note In history, when disk cache hit, the image will write back into memory cache, which is mostly unwanted behavior. From new version this behavior is removed.
  */
 - (nullable SDImageCacheToken *)queryCacheOperationForKey:(nullable NSString *)key options:(SDImageCacheOptions)options done:(nullable SDImageCacheQueryCompletionBlock)doneBlock;
 
@@ -337,6 +340,7 @@ typedef NS_OPTIONS(NSUInteger, SDImageCacheOptions) {
  *
  * @return a SDImageCacheToken instance containing the cache operation, will callback immediately when cancellederation, will callback immediately when cancelled
  * @warning If you query with thumbnail cache key, you'd better not pass the thumbnail pixel size context, which is undefined behavior.
+ * @note In history, when disk cache hit, the image will write back into memory cache, which is mostly unwanted behavior. From new version this behavior is removed.
  */
 - (nullable SDImageCacheToken *)queryCacheOperationForKey:(nullable NSString *)key options:(SDImageCacheOptions)options context:(nullable SDWebImageContext *)context done:(nullable SDImageCacheQueryCompletionBlock)doneBlock;
 
@@ -351,6 +355,7 @@ typedef NS_OPTIONS(NSUInteger, SDImageCacheOptions) {
  *
  * @return a SDImageCacheToken instance containing the cache operation, will callback immediately when cancelled
  * @warning If you query with thumbnail cache key, you'd better not pass the thumbnail pixel size context, which is undefined behavior.
+ * @note In history, when disk cache hit, the image will write back into memory cache, which is mostly unwanted behavior. From new version this behavior is removed.
  */
 - (nullable SDImageCacheToken *)queryCacheOperationForKey:(nullable NSString *)key options:(SDImageCacheOptions)options context:(nullable SDWebImageContext *)context cacheType:(SDImageCacheType)queryCacheType done:(nullable SDImageCacheQueryCompletionBlock)doneBlock;
 

SDWebImage/Core/SDImageCache.m

@@ -9,6 +9,7 @@
 #import "SDImageCache.h"
 #import "SDInternalMacros.h"
 #import "NSImage+Compatibility.h"
+#import "UIImage+MultiFormat.h"
 #import "SDImageCodersManager.h"
 #import "SDImageCoderHelper.h"
 #import "SDAnimatedImage.h"
@@ -16,7 +17,6 @@
 #import "UIImage+Metadata.h"
 #import "UIImage+ExtendedCacheData.h"
 #import "SDCallbackQueue.h"
-#import "SDImageTransformer.h" // TODO, remove this
 
 // TODO, remove this
 static BOOL SDIsThumbnailKey(NSString *key) {
@@ -261,7 +261,10 @@ static NSString * _defaultDiskCacheDirectory;
         return;
     }
     NSData *data = imageData;
-    if (!data && [image respondsToSelector:@selector(animatedImageData)]) {
+    if (image.sd_isThumbnail && SDIsThumbnailKey(key)) {
+        // Currently we have no solid way to store thumbnail image's correct data
+        data = nil;
+    } else if (!data && [image respondsToSelector:@selector(animatedImageData)]) {
         // If image is custom animated image class, prefer its original animated data
         data = [((id<SDAnimatedImage>)image) animatedImageData];
     }
@@ -279,11 +282,17 @@ static NSString * _defaultDiskCacheDirectory;
                     format = [SDImageCoderHelper CGImageContainsAlpha:image.CGImage] ? SDImageFormatPNG : SDImageFormatJPEG;
                 }
             }
-            id<SDImageCoder> imageCoder = context[SDWebImageContextImageCoder];
-            if (!imageCoder) {
-                imageCoder = [SDImageCodersManager sharedManager];
+            NSData *encodedData;
+            if ([image respondsToSelector:@selector(animatedImageData)]) {
+                // This API will encode the animation for `SDAnimatedImage` with GIF format
+                encodedData = [image sd_imageDataAsFormat:format];
+            } else {
+                id<SDImageCoder> imageCoder = context[SDWebImageContextImageCoder];
+                if (!imageCoder) {
+                    imageCoder = [SDImageCodersManager sharedManager];
+                }
+                encodedData = [imageCoder encodedDataWithImage:image format:format options:context[SDWebImageContextImageEncodeOptions]];
             }
-            NSData *encodedData = [imageCoder encodedDataWithImage:image format:format options:context[SDWebImageContextImageEncodeOptions]];
             dispatch_async(self.ioQueue, ^{
                 [self _storeImageDataToDisk:encodedData forKey:key];
                 [self _archivedDataWithImage:image forKey:key];
@@ -438,16 +447,6 @@ static NSString * _defaultDiskCacheDirectory;
     }
     NSData *data = [self diskImageDataForKey:key];
     UIImage *diskImage = [self diskImageForKey:key data:data options:options context:context];
-    
-    BOOL shouldCacheToMemory = YES;
-    if (context[SDWebImageContextStoreCacheType]) {
-        SDImageCacheType cacheType = [context[SDWebImageContextStoreCacheType] integerValue];
-        shouldCacheToMemory = (cacheType == SDImageCacheTypeAll || cacheType == SDImageCacheTypeMemory);
-    }
-    if (shouldCacheToMemory) {
-        // check if we need sync logic
-        [self _syncDiskToMemoryWithImage:diskImage forKey:key];
-    }
 
     return diskImage;
 }
@@ -527,42 +526,6 @@ static NSString * _defaultDiskCacheDirectory;
     return image;
 }
 
-- (void)_syncDiskToMemoryWithImage:(UIImage *)diskImage forKey:(NSString *)key {
-    // earily check
-    if (!self.config.shouldCacheImagesInMemory) {
-        return;
-    }
-    if (!diskImage) {
-        return;
-    }
-    // The disk -> memory sync logic, which should only store thumbnail image with thumbnail key
-    // However, caller (like SDWebImageManager) will query full key, with thumbnail size, and get thubmnail image
-    // We should add a check here, currently it's a hack
-    if (diskImage.sd_isThumbnail && !SDIsThumbnailKey(key)) {
-        SDImageCoderOptions *options = diskImage.sd_decodeOptions;
-        CGSize thumbnailSize = CGSizeZero;
-        NSValue *thumbnailSizeValue = options[SDImageCoderDecodeThumbnailPixelSize];
-        if (thumbnailSizeValue != nil) {
-    #if SD_MAC
-            thumbnailSize = thumbnailSizeValue.sizeValue;
-    #else
-            thumbnailSize = thumbnailSizeValue.CGSizeValue;
-    #endif
-        }
-        BOOL preserveAspectRatio = YES;
-        NSNumber *preserveAspectRatioValue = options[SDImageCoderDecodePreserveAspectRatio];
-        if (preserveAspectRatioValue != nil) {
-            preserveAspectRatio = preserveAspectRatioValue.boolValue;
-        }
-        // Calculate the actual thumbnail key
-        NSString *thumbnailKey = SDThumbnailedKeyForKey(key, thumbnailSize, preserveAspectRatio);
-        // Override the sync key
-        key = thumbnailKey;
-    }
-    NSUInteger cost = diskImage.sd_memoryCost;
-    [self.memoryCache setObject:diskImage forKey:key cost:cost];
-}
-
 - (void)_unarchiveObjectWithImage:(UIImage *)image forKey:(NSString *)key {
     if (!image || !key) {
         return;
@@ -705,10 +668,6 @@ static NSString * _defaultDiskCacheDirectory;
             // decode image data only if in-memory cache missed
             if (!diskImage) {
                 diskImage = [self diskImageForKey:key data:diskData options:options context:context];
-                // check if we need sync logic
-                if (shouldCacheToMemory) {
-                    [self _syncDiskToMemoryWithImage:diskImage forKey:key];
-                }
             }
         }
         return diskImage;

SDWebImage/Core/SDWebImageManager.m

@@ -324,6 +324,12 @@ static id<SDImageLoader> _defaultImageLoader;
                     [self callOriginalCacheProcessForOperation:operation url:url options:options context:context progress:progressBlock completed:completedBlock];
                     return;
                 }
+            } else {
+                // Write back the disk image into memory cache, with the correct key
+                if (cacheType == SDImageCacheTypeDisk) {
+                    // Sync
+                    [imageCache storeImage:cachedImage imageData:nil forKey:key cacheType:SDImageCacheTypeMemory completion:nil];
+                }
             }
             // Continue download process
             [self callDownloadProcessForOperation:operation url:url options:options context:context cachedImage:cachedImage cachedData:cachedData cacheType:cacheType progress:progressBlock completed:completedBlock];
@@ -376,6 +382,12 @@ static id<SDImageLoader> _defaultImageLoader;
                 // Original image cache miss. Continue download process
                 [self callDownloadProcessForOperation:operation url:url options:options context:context cachedImage:nil cachedData:nil cacheType:SDImageCacheTypeNone progress:progressBlock completed:completedBlock];
                 return;
+            } else {
+                // Write back the disk image into memory cache, with the correct key
+                if (cacheType == SDImageCacheTypeDisk) {
+                    // Sync
+                    [imageCache storeImage:cachedImage imageData:nil forKey:key cacheType:SDImageCacheTypeMemory completion:nil];
+                }
             }
                         
             // Skip downloading and continue transform process, and ignore .refreshCached option for now

Tests/Tests/SDWebImageManagerTests.m

@@ -504,7 +504,8 @@
         // Check the thumbnail image should be in memory cache (because we have only full data + thumbnail image)
         expect([SDImageCache.sharedImageCache imageFromMemoryCacheForKey:fullSizeKey].size).equal(CGSizeZero);
         expect([SDImageCache.sharedImageCache imageFromDiskCacheForKey:fullSizeKey]).notTo.beNil();
-        expect([SDImageCache.sharedImageCache imageFromMemoryCacheForKey:thumbnailKey].size).equal(thumbnailSize);
+        // Store disk no longer store into memory
+//        expect([SDImageCache.sharedImageCache imageFromMemoryCacheForKey:thumbnailKey].size).equal(thumbnailSize);
         expect([SDImageCache.sharedImageCache imageFromDiskCacheForKey:thumbnailKey]).beNil();
         
         [SDImageCache.sharedImageCache removeImageFromDiskForKey:fullSizeKey];