From db5307eb9404a3f1f33a9da6591c9e6ff0d24def Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Sat, 30 Dec 2017 02:16:52 +0800 Subject: [PATCH 1/2] Change our imageCache `storeImageDataToDisk` to internal use IO-queue. And also change error from POSIX errno to Cocoa file error --- SDWebImage/SDImageCache.h | 6 ++--- SDWebImage/SDImageCache.m | 40 +++++++++++++++++++---------- SDWebImage/SDWebImageImageIOCoder.m | 2 -- Tests/Tests/SDImageCacheTests.m | 15 +++++++---- Tests/Tests/SDMockFileManager.h | 3 ++- Tests/Tests/SDMockFileManager.m | 27 +++++++++---------- 6 files changed, 54 insertions(+), 39 deletions(-) diff --git a/SDWebImage/SDImageCache.h b/SDWebImage/SDImageCache.h index 4b1dbaa5..1fd3f3c9 100644 --- a/SDWebImage/SDImageCache.h +++ b/SDWebImage/SDImageCache.h @@ -150,15 +150,13 @@ typedef void(^SDWebImageCompletionWithPossibleErrorBlock)(NSError * _Nullable er /** * Synchronously store image NSData into disk cache at the given key. * - * @warning This method is synchronous, make sure to call it from the ioQueue - * * @param imageData The image data to store * @param key The unique image cache key, usually it's image absolute URL - * @param errorPtr NSError pointer. If error occurs then (*errorPtr) != nil. + * @param error NSError pointer, for possible file I/O errors, See FoundationErrors.h */ - (BOOL)storeImageDataToDisk:(nullable NSData *)imageData forKey:(nullable NSString *)key - error:(NSError * _Nullable * _Nullable)errorPtr; + error:(NSError * _Nullable * _Nullable)error; #pragma mark - Query and Retrieve Ops diff --git a/SDWebImage/SDImageCache.m b/SDWebImage/SDImageCache.m index 7701a1f5..d8f09885 100644 --- a/SDWebImage/SDImageCache.m +++ b/SDWebImage/SDImageCache.m @@ -137,14 +137,6 @@ FOUNDATION_STATIC_INLINE NSUInteger SDCacheCostForImage(UIImage *image) { [[NSNotificationCenter defaultCenter] removeObserver:self]; } -- (void)checkIfQueueIsIOQueue { - const char *currentQueueLabel = dispatch_queue_get_label(DISPATCH_CURRENT_QUEUE_LABEL); - const char *ioQueueLabel = dispatch_queue_get_label(self.ioQueue); - if (strcmp(currentQueueLabel, ioQueueLabel) != 0) { - NSLog(@"This method should be called from the ioQueue"); - } -} - #pragma mark - Cache paths - (void)addReadOnlyCachePath:(nonnull NSString *)path { @@ -233,7 +225,7 @@ FOUNDATION_STATIC_INLINE NSUInteger SDCacheCostForImage(UIImage *image) { } data = [[SDWebImageCodersManager sharedInstance] encodedDataWithImage:image format:format]; } - [self storeImageDataToDisk:data forKey:key error:&writeError]; + [self safeStoreImageDataToDisk:data forKey:key error:&writeError]; } if (completionBlock) { @@ -251,15 +243,34 @@ FOUNDATION_STATIC_INLINE NSUInteger SDCacheCostForImage(UIImage *image) { - (BOOL)storeImageDataToDisk:(nullable NSData *)imageData forKey:(nullable NSString *)key - error:(NSError * _Nullable * _Nullable)errorPtr { + error:(NSError * _Nullable __autoreleasing * _Nullable)error { if (!imageData || !key) { return NO; } + __autoreleasing NSError *fileError; + if (!error) { + error = &fileError; + } - [self checkIfQueueIsIOQueue]; + __block BOOL success = YES; + void(^storeImageDataBlock)(void) = ^{ + success = [self safeStoreImageDataToDisk:imageData forKey:key error:error]; + }; + dispatch_sync(self.ioQueue, storeImageDataBlock); + return success; +} + +- (BOOL)safeStoreImageDataToDisk:(nullable NSData *)imageData + forKey:(nullable NSString *)key + error:(NSError * _Nullable __autoreleasing * _Nonnull)error { + if (!imageData || !key) { + return NO; + } if (![_fileManager fileExistsAtPath:_diskCachePath]) { - [_fileManager createDirectoryAtPath:_diskCachePath withIntermediateDirectories:YES attributes:nil error:NULL]; + if (![_fileManager createDirectoryAtPath:_diskCachePath withIntermediateDirectories:YES attributes:nil error:error]) { + return NO; + } } // get cache Path for image key @@ -267,8 +278,9 @@ FOUNDATION_STATIC_INLINE NSUInteger SDCacheCostForImage(UIImage *image) { // transform to NSUrl NSURL *fileURL = [NSURL fileURLWithPath:cachePathForKey]; - if (![_fileManager createFileAtPath:cachePathForKey contents:imageData attributes:nil] && errorPtr) { - *errorPtr = [[NSError alloc] initWithDomain:NSPOSIXErrorDomain code:errno userInfo:nil]; + // NSFileManager's `createFileAtPath:` is used just for old code compatibility and will not trigger any delegate methods, so it's useless for custom NSFileManager at all. + // And also, NSFileManager's `createFileAtPath:` can only grab underlying POSIX errno, but NSData can grab errors defined in NSCocoaErrorDomain, which is better for user to check. + if (![imageData writeToFile:cachePathForKey options:NSDataWritingAtomic error:error]) { return NO; } diff --git a/SDWebImage/SDWebImageImageIOCoder.m b/SDWebImage/SDWebImageImageIOCoder.m index 25c2d194..f86de89c 100644 --- a/SDWebImage/SDWebImageImageIOCoder.m +++ b/SDWebImage/SDWebImageImageIOCoder.m @@ -540,8 +540,6 @@ static const CGFloat kDestSeemOverlap = 2.0f; // the numbers of pixels to over result = [SDWebImageCoderHelper imageOrientationFromEXIFOrientation:exifOrientation]; } // else - if it's not set it remains at up CFRelease((CFTypeRef) properties); - } else { - //NSLog(@"NO PROPERTIES, FAIL"); } CFRelease(imageSource); } diff --git a/Tests/Tests/SDImageCacheTests.m b/Tests/Tests/SDImageCacheTests.m index 6066555e..efec8f06 100644 --- a/Tests/Tests/SDImageCacheTests.m +++ b/Tests/Tests/SDImageCacheTests.m @@ -309,23 +309,28 @@ NSString *kImageTestKey = @"TestImageKey.jpg"; - (void)test41StoreImageDataToDiskWithError { NSData *imageData = [NSData dataWithContentsOfFile:[self testImagePath]]; - NSError * error = nil; + NSError *targetError = [NSError errorWithDomain:NSCocoaErrorDomain code:NSFileWriteNoPermissionError userInfo:nil]; + NSError *error = nil; + SDMockFileManager *fileManager = [[SDMockFileManager alloc] init]; + fileManager.mockSelectors = @{NSStringFromSelector(@selector(createDirectoryAtPath:withIntermediateDirectories:attributes:error:)) : targetError}; SDImageCache *cache = [[SDImageCache alloc] initWithNamespace:@"test" diskCacheDirectory:@"/" - fileManager:[[SDMockFileManager alloc] initWithError:EACCES]]; + fileManager:fileManager]; [cache storeImageDataToDisk:imageData forKey:kImageTestKey error:&error]; - XCTAssertEqual(error.code, EACCES); + XCTAssertEqual(error.code, NSFileWriteNoPermissionError); } - (void)test42StoreImageDataToDiskWithoutError { NSData *imageData = [NSData dataWithContentsOfFile:[self testImagePath]]; - NSError * error = nil; + NSError *error = nil; + SDMockFileManager *fileManager = [[SDMockFileManager alloc] init]; + fileManager.mockSelectors = @{NSStringFromSelector(@selector(createDirectoryAtPath:withIntermediateDirectories:attributes:error:)) : [NSNull null]}; SDImageCache *cache = [[SDImageCache alloc] initWithNamespace:@"test" diskCacheDirectory:@"/" - fileManager:[[SDMockFileManager alloc] initWithError:0]]; + fileManager:fileManager]; [cache storeImageDataToDisk:imageData forKey:kImageTestKey error:&error]; diff --git a/Tests/Tests/SDMockFileManager.h b/Tests/Tests/SDMockFileManager.h index e5d4edfc..d93d1ac4 100644 --- a/Tests/Tests/SDMockFileManager.h +++ b/Tests/Tests/SDMockFileManager.h @@ -8,8 +8,9 @@ #import +// This is a mock class to provide custom error for methods @interface SDMockFileManager : NSFileManager -- (id)initWithError:(int)errorNumber; +@property (nonatomic, copy, nullable) NSDictionary *mockSelectors; // used to specify mocked selectors which will return NO with specify error instead of normal process. If you specify a NSNull, will use nil instead. @end diff --git a/Tests/Tests/SDMockFileManager.m b/Tests/Tests/SDMockFileManager.m index 60202f4b..472797fb 100644 --- a/Tests/Tests/SDMockFileManager.m +++ b/Tests/Tests/SDMockFileManager.m @@ -10,24 +10,25 @@ @interface SDMockFileManager () -@property (nonatomic, assign) int errorNumber; - @end @implementation SDMockFileManager -- (id)initWithError:(int)errorNumber { - self = [super init]; - if (self) { - _errorNumber = errorNumber; +- (BOOL)createDirectoryAtPath:(NSString *)path withIntermediateDirectories:(BOOL)createIntermediates attributes:(NSDictionary *)attributes error:(NSError * _Nullable __autoreleasing *)error { + NSError *mockError = [self.mockSelectors objectForKey:NSStringFromSelector(_cmd)]; + if ([mockError isEqual:[NSNull null]]) { + if (error) { + *error = nil; + } + return NO; + } else if (mockError) { + if (error) { + *error = mockError; + } + return NO; + } else { + return [super createDirectoryAtPath:path withIntermediateDirectories:createIntermediates attributes:attributes error:error]; } - - return self; -} - -- (BOOL)createFileAtPath:(NSString *)path contents:(NSData *)data attributes:(NSDictionary *)attr { - errno = self.errorNumber; - return (self.errorNumber == 0); } @end From 0d7c93d7b77ef9f2fe0f9eb8e938849c520f1553 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Sat, 30 Dec 2017 03:09:55 +0800 Subject: [PATCH 2/2] Add a `diskCacheWritingOptions` to allow user to specify disk data writing options --- SDWebImage/SDImageCache.m | 2 +- SDWebImage/SDImageCacheConfig.h | 8 +++++++- SDWebImage/SDImageCacheConfig.m | 1 + 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/SDWebImage/SDImageCache.m b/SDWebImage/SDImageCache.m index d8f09885..88b37c17 100644 --- a/SDWebImage/SDImageCache.m +++ b/SDWebImage/SDImageCache.m @@ -280,7 +280,7 @@ FOUNDATION_STATIC_INLINE NSUInteger SDCacheCostForImage(UIImage *image) { // NSFileManager's `createFileAtPath:` is used just for old code compatibility and will not trigger any delegate methods, so it's useless for custom NSFileManager at all. // And also, NSFileManager's `createFileAtPath:` can only grab underlying POSIX errno, but NSData can grab errors defined in NSCocoaErrorDomain, which is better for user to check. - if (![imageData writeToFile:cachePathForKey options:NSDataWritingAtomic error:error]) { + if (![imageData writeToFile:cachePathForKey options:self.config.diskCacheWritingOptions error:error]) { return NO; } diff --git a/SDWebImage/SDImageCacheConfig.h b/SDWebImage/SDImageCacheConfig.h index 20f2d108..d3cb5421 100644 --- a/SDWebImage/SDImageCacheConfig.h +++ b/SDWebImage/SDImageCacheConfig.h @@ -29,10 +29,16 @@ /** * The reading options while reading cache from disk. - * Defaults to 0. You can set this to mapped file to improve performance. + * Defaults to 0. You can set this to `NSDataReadingMappedIfSafe` to improve performance. */ @property (assign, nonatomic) NSDataReadingOptions diskCacheReadingOptions; +/** + * The writing options while writing cache to disk. + * Defaults to `NSDataWritingAtomic`. You can set this to `NSDataWritingWithoutOverwriting` to prevent overwriting an existing file. + */ +@property (assign, nonatomic) NSDataWritingOptions diskCacheWritingOptions; + /** * The maximum length of time to keep an image in the cache, in seconds. */ diff --git a/SDWebImage/SDImageCacheConfig.m b/SDWebImage/SDImageCacheConfig.m index 7a5af6cb..923506d0 100644 --- a/SDWebImage/SDImageCacheConfig.m +++ b/SDWebImage/SDImageCacheConfig.m @@ -18,6 +18,7 @@ static const NSInteger kDefaultCacheMaxCacheAge = 60 * 60 * 24 * 7; // 1 week _shouldDisableiCloud = YES; _shouldCacheImagesInMemory = YES; _diskCacheReadingOptions = 0; + _diskCacheWritingOptions = NSDataWritingAtomic; _maxCacheAge = kDefaultCacheMaxCacheAge; _maxCacheSize = 0; }