Skip to content

Commit

Permalink
Enhance image freshness check before stored into cache (#23226)
Browse files Browse the repository at this point in the history 
Summary:
This is a re-submit of D13895627 which got landed but didn't include a fix to Instagram's code. The sheriffs were unsure how it got landed without running the build.

Currently, before we store the image to cache, we only respect `Cache-Control`, actually, we also may need to check `Expires`、`Last-Modified`, refer to [MDN docs](https://developer.mozilla.org/en-US/docs/Web/HTTP/Caching#Freshness), and  [okhttp](https://github.com/square/okhttp/blob/568a91c44a118b2c2ba62d310a331582c567b24a/okhttp/src/main/java/okhttp3/internal/cache/CacheStrategy.java#L268) respect the `MDN`, so in iOS, we can also respect this.

[iOS] [Fixed] - Respect `MDN` cache strategy before cache the image.

Reviewed By: shergin

Differential Revision: D13896822

Pulled By: cpojer

fbshipit-source-id: 8c1714f4a17ad40496146806cff3e188a60be93c
  • Loading branch information
@zhongwuzw @facebook-github-bot
zhongwuzw authored and facebook-github-bot committed Feb 6, 2019
1 parent a4840e7 commit fb8ba3f
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 47 deletions.
60 changes: 38 additions & 22 deletions Libraries/Image/RCTImageCache.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,33 +106,49 @@ - (void)addImageToCache:(UIImage *)image
size:(CGSize)size
scale:(CGFloat)scale
resizeMode:(RCTResizeMode)resizeMode
responseDate:(NSString *)responseDate
cacheControl:(NSString *)cacheControl
response:(NSURLResponse *)response
{
NSString *cacheKey = RCTCacheKeyForImage(url, size, scale, resizeMode);
BOOL shouldCache = YES;
NSDate *staleTime;
NSArray<NSString *> *components = [cacheControl componentsSeparatedByString:@","];
for (NSString *component in components) {
if ([component containsString:@"no-cache"] || [component containsString:@"no-store"] || [component hasSuffix:@"max-age=0"]) {
shouldCache = NO;
break;
} else {
NSRange range = [component rangeOfString:@"max-age="];
if (range.location != NSNotFound) {
NSInteger seconds = [[component substringFromIndex:range.location + range.length] integerValue];
NSDate *originalDate = [self dateWithHeaderString:responseDate];
staleTime = [originalDate dateByAddingTimeInterval:(NSTimeInterval)seconds];
if ([response isKindOfClass:[NSHTTPURLResponse class]]) {
NSString *cacheKey = RCTCacheKeyForImage(url, size, scale, resizeMode);
BOOL shouldCache = YES;
NSString *responseDate = ((NSHTTPURLResponse *)response).allHeaderFields[@"Date"];
NSDate *originalDate = [self dateWithHeaderString:responseDate];
NSString *cacheControl = ((NSHTTPURLResponse *)response).allHeaderFields[@"Cache-Control"];
NSDate *staleTime;
NSArray<NSString *> *components = [cacheControl componentsSeparatedByString:@","];
for (NSString *component in components) {
if ([component containsString:@"no-cache"] || [component containsString:@"no-store"] || [component hasSuffix:@"max-age=0"]) {
shouldCache = NO;
break;
} else {
NSRange range = [component rangeOfString:@"max-age="];
if (range.location != NSNotFound) {
NSInteger seconds = [[component substringFromIndex:range.location + range.length] integerValue];
staleTime = [originalDate dateByAddingTimeInterval:(NSTimeInterval)seconds];
}
}
}
}
if (shouldCache) {
if (staleTime) {
@synchronized(_cacheStaleTimes) {
_cacheStaleTimes[cacheKey] = staleTime;
if (shouldCache) {
if (!staleTime && originalDate) {
NSString *expires = ((NSHTTPURLResponse *)response).allHeaderFields[@"Expires"];
NSString *lastModified = ((NSHTTPURLResponse *)response).allHeaderFields[@"Last-Modified"];
if (expires) {
staleTime = [self dateWithHeaderString:expires];
} else if (lastModified) {
NSDate *lastModifiedDate = [self dateWithHeaderString:lastModified];
if (lastModifiedDate) {
NSTimeInterval interval = [originalDate timeIntervalSinceDate:lastModifiedDate] / 10;
staleTime = [originalDate dateByAddingTimeInterval:interval];
}
}
}
if (staleTime) {
@synchronized(_cacheStaleTimes) {
_cacheStaleTimes[cacheKey] = staleTime;
}
}
return [self addImageToCache:image forKey:cacheKey];
}
return [self addImageToCache:image forKey:cacheKey];
}
}

Expand Down
3 changes: 1 addition & 2 deletions Libraries/Image/RCTImageLoader.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ typedef dispatch_block_t RCTImageLoaderCancellationBlock;
size:(CGSize)size
scale:(CGFloat)scale
resizeMode:(RCTResizeMode)resizeMode
responseDate:(NSString *)responseDate
cacheControl:(NSString *)cacheControl;
response:(NSURLResponse *)response;

@end

Expand Down
40 changes: 17 additions & 23 deletions Libraries/Image/RCTImageLoader.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ - (RCTImageLoaderCancellationBlock)_loadImageOrDataWithURLRequest:(NSURLRequest
resizeMode:(RCTResizeMode)resizeMode
progressBlock:(RCTImageLoaderProgressBlock)progressHandler
partialLoadBlock:(RCTImageLoaderPartialLoadBlock)partialLoadHandler
completionBlock:(void (^)(NSError *error, id imageOrData, BOOL cacheResult, NSString *fetchDate, NSString *cacheControl))completionBlock
completionBlock:(void (^)(NSError *error, id imageOrData, BOOL cacheResult, NSURLResponse *response))completionBlock
{
{
NSMutableURLRequest *mutableRequest = [request mutableCopy];
Expand Down Expand Up @@ -375,7 +375,7 @@ - (RCTImageLoaderCancellationBlock)_loadImageOrDataWithURLRequest:(NSURLRequest
__block atomic_bool cancelled = ATOMIC_VAR_INIT(NO);
// TODO: Protect this variable shared between threads.
__block dispatch_block_t cancelLoad = nil;
void (^completionHandler)(NSError *, id, NSString *, NSString *) = ^(NSError *error, id imageOrData, NSString *fetchDate, NSString *cacheControl) {
void (^completionHandler)(NSError *, id, NSURLResponse *) = ^(NSError *error, id imageOrData, NSURLResponse *response) {
cancelLoad = nil;

// If we've received an image, we should try to set it synchronously,
Expand All @@ -385,11 +385,11 @@ - (RCTImageLoaderCancellationBlock)_loadImageOrDataWithURLRequest:(NSURLRequest
// expecting it, and may do expensive post-processing in the callback
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
if (!atomic_load(&cancelled)) {
completionBlock(error, imageOrData, cacheResult, fetchDate, cacheControl);
completionBlock(error, imageOrData, cacheResult, response);
}
});
} else if (!atomic_load(&cancelled)) {
completionBlock(error, imageOrData, cacheResult, fetchDate, cacheControl);
completionBlock(error, imageOrData, cacheResult, response);
}
};

Expand All @@ -403,7 +403,7 @@ - (RCTImageLoaderCancellationBlock)_loadImageOrDataWithURLRequest:(NSURLRequest
progressHandler:progressHandler
partialLoadHandler:partialLoadHandler
completionHandler:^(NSError *error, UIImage *image){
completionHandler(error, image, nil, nil);
completionHandler(error, image, nil);
}];
}

Expand All @@ -427,7 +427,7 @@ - (RCTImageLoaderCancellationBlock)_loadImageOrDataWithURLRequest:(NSURLRequest
progressHandler:progressHandler
partialLoadHandler:partialLoadHandler
completionHandler:^(NSError *error, UIImage *image) {
completionHandler(error, image, nil, nil);
completionHandler(error, image, nil);
}];
} else {
UIImage *image;
Expand All @@ -439,7 +439,7 @@ - (RCTImageLoaderCancellationBlock)_loadImageOrDataWithURLRequest:(NSURLRequest
}

if (image) {
completionHandler(nil, image, nil, nil);
completionHandler(nil, image, nil);
} else {
// Use networking module to load image
cancelLoad = [strongSelf _loadURLRequest:request
Expand All @@ -464,7 +464,7 @@ - (RCTImageLoaderCancellationBlock)_loadImageOrDataWithURLRequest:(NSURLRequest

- (RCTImageLoaderCancellationBlock)_loadURLRequest:(NSURLRequest *)request
progressBlock:(RCTImageLoaderProgressBlock)progressHandler
completionBlock:(void (^)(NSError *error, id imageOrData, NSString *fetchDate, NSString *cacheControl))completionHandler
completionBlock:(void (^)(NSError *error, id imageOrData, NSURLResponse *response))completionHandler
{
// Check if networking module is available
if (RCT_DEBUG && ![_bridge respondsToSelector:@selector(networking)]) {
Expand All @@ -486,36 +486,31 @@ - (RCTImageLoaderCancellationBlock)_loadURLRequest:(NSURLRequest *)request
RCTURLRequestCompletionBlock processResponse = ^(NSURLResponse *response, NSData *data, NSError *error) {
// Check for system errors
if (error) {
completionHandler(error, nil, nil, nil);
completionHandler(error, nil, response);
return;
} else if (!response) {
completionHandler(RCTErrorWithMessage(@"Response metadata error"), nil, nil, nil);
completionHandler(RCTErrorWithMessage(@"Response metadata error"), nil, response);
return;
} else if (!data) {
completionHandler(RCTErrorWithMessage(@"Unknown image download error"), nil, nil, nil);
completionHandler(RCTErrorWithMessage(@"Unknown image download error"), nil, response);
return;
}

// Check for http errors
NSString *responseDate;
NSString *cacheControl;
if ([response isKindOfClass:[NSHTTPURLResponse class]]) {
NSInteger statusCode = ((NSHTTPURLResponse *)response).statusCode;
if (statusCode != 200) {
NSString *errorMessage = [NSString stringWithFormat:@"Failed to load %@", response.URL];
NSDictionary *userInfo = @{NSLocalizedDescriptionKey: errorMessage};
completionHandler([[NSError alloc] initWithDomain:NSURLErrorDomain
code:statusCode
userInfo:userInfo], nil, nil, nil);
userInfo:userInfo], nil, response);
return;
}

responseDate = ((NSHTTPURLResponse *)response).allHeaderFields[@"Date"];
cacheControl = ((NSHTTPURLResponse *)response).allHeaderFields[@"Cache-Control"];
}

// Call handler
completionHandler(nil, data, responseDate, cacheControl);
completionHandler(nil, data, response);
};

// Download image
Expand All @@ -537,7 +532,7 @@ - (RCTImageLoaderCancellationBlock)_loadURLRequest:(NSURLRequest *)request
} else {
someError = RCTErrorWithMessage(@"Unknown image download error");
}
completionHandler(someError, nil, nil, nil);
completionHandler(someError, nil, response);
[strongSelf dequeueTasks];
return;
}
Expand Down Expand Up @@ -603,7 +598,7 @@ - (RCTImageLoaderCancellationBlock)loadImageWithURLRequest:(NSURLRequest *)image
};

__weak RCTImageLoader *weakSelf = self;
void (^completionHandler)(NSError *, id, BOOL, NSString *, NSString *) = ^(NSError *error, id imageOrData, BOOL cacheResult, NSString *fetchDate, NSString *cacheControl) {
void (^completionHandler)(NSError *, id, BOOL, NSURLResponse *) = ^(NSError *error, id imageOrData, BOOL cacheResult, NSURLResponse *response) {
__typeof(self) strongSelf = weakSelf;
if (atomic_load(&cancelled) || !strongSelf) {
return;
Expand All @@ -623,8 +618,7 @@ - (RCTImageLoaderCancellationBlock)loadImageWithURLRequest:(NSURLRequest *)image
size:size
scale:scale
resizeMode:resizeMode
responseDate:fetchDate
cacheControl:cacheControl];
response:response];
}

cancelLoad = nil;
Expand Down Expand Up @@ -758,7 +752,7 @@ - (RCTImageLoaderCancellationBlock)decodeImageData:(NSData *)data
- (RCTImageLoaderCancellationBlock)getImageSizeForURLRequest:(NSURLRequest *)imageURLRequest
block:(void(^)(NSError *error, CGSize size))callback
{
void (^completion)(NSError *, id, BOOL, NSString *, NSString *) = ^(NSError *error, id imageOrData, BOOL cacheResult, NSString *fetchDate, NSString *cacheControl) {
void (^completion)(NSError *, id, BOOL, NSURLResponse *) = ^(NSError *error, id imageOrData, BOOL cacheResult, NSURLResponse *response) {
CGSize size;
if ([imageOrData isKindOfClass:[NSData class]]) {
NSDictionary *meta = RCTGetImageMetadata(imageOrData);
Expand Down

0 comments on commit fb8ba3f

Please sign in to comment.