Adds support for specifying a quality indexed array of URLs (TextureG…

…roup#557) * Add support for downloading a set of URLs on ASNetworkImageNode * Should be building now; * Remove old unused code * Add a changelog message * Bump PINRemoteImage * Huy's comments
AtomTickets · Apr 25, 2018 · 4dac032 · 4dac032
1 parent 81e5de5
commit 4dac032
Show file tree

Hide file tree

Showing 7 changed files with 226 additions and 97 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,7 @@
 - Fixed a memory corruption issue in the ASImageNode display system. [Adlai Holler](https://github.com/Adlai-Holler) [#555](https://github.com/TextureGroup/Texture/pull/555)
 - [Breaking] Rename ASCollectionGalleryLayoutSizeProviding to ASCollectionGalleryLayoutPropertiesProviding. Besides a fixed item size, it now can provide interitem and line spacings, as well as section inset [Huy Nguyen](https://github.com/nguyenhuy) [#496](https://github.com/TextureGroup/Texture/pull/496) [#533](https://github.com/TextureGroup/Texture/pull/533)
 - Deprecate `-[ASDisplayNode displayWillStart]` in favor of `-displayWillStartAsynchronously:` [Huy Nguyen](https://github.com/nguyenhuy) [536](https://github.com/TextureGroup/Texture/pull/536)
+- Add support for URLs on ASNetworkImageNode. [Garrett Moon](https://github.com/garrettmoon)
 
 ##2.4
 - Fix an issue where inserting/deleting sections could lead to inconsistent supplementary element behavior. [Adlai Holler](https://github.com/Adlai-Holler)

diff --git a/Cartfile b/Cartfile
@@ -1,2 +1,2 @@
-github "pinterest/PINRemoteImage" "3.0.0-beta.11"
+github "pinterest/PINRemoteImage" "3.0.0-beta.12"
 github "pinterest/PINCache" "3.0.1-beta.5"
diff --git a/Source/ASNetworkImageNode.h b/Source/ASNetworkImageNode.h
@@ -82,6 +82,15 @@ NS_ASSUME_NONNULL_BEGIN
  */
 @property (nullable, nonatomic, strong, readwrite) NSURL *URL;
 
+/**
+ * An array of URLs of increasing cost to download.
+ *
+ * @discussion By setting an array of URLs, the image property of this node will be managed internally. This means previously
+ * directly set images to the image property will be cleared out and replaced by the placeholder (<defaultImage>) image
+ * while loading and the final image after the new image data was downloaded and processed.
+ */
+@property (nullable, nonatomic, strong, readwrite) NSArray <NSURL *> *URLs;
+
 /**
  * Download and display a new image.
  *

diff --git a/Source/ASNetworkImageNode.mm b/Source/ASNetworkImageNode.mm
@@ -39,7 +39,7 @@ @interface ASNetworkImageNode ()
   // Only access any of these with __instanceLock__.
   __weak id<ASNetworkImageNodeDelegate> _delegate;
 
-  NSURL *_URL;
+  NSArray *_URLs;
   UIImage *_defaultImage;
 
   NSUUID *_cacheUUID;
@@ -68,13 +68,15 @@ @interface ASNetworkImageNode ()
     unsigned int downloaderImplementsSetPriority:1;
     unsigned int downloaderImplementsAnimatedImage:1;
     unsigned int downloaderImplementsCancelWithResume:1;
+    unsigned int downloaderImplementsDownloadURLs:1;
   } _downloaderFlags;
 
   // Immutable and set on init only. We don't need to lock in this case.
   __weak id<ASImageCacheProtocol> _cache;
   struct {
     unsigned int cacheSupportsClearing:1;
     unsigned int cacheSupportsSynchronousFetch:1;
+    unsigned int cacheSupportsCachedURLs:1;
   } _cacheFlags;
 }
 
@@ -96,9 +98,11 @@ - (instancetype)initWithCache:(id<ASImageCacheProtocol>)cache downloader:(id<ASI
   _downloaderFlags.downloaderImplementsSetPriority = [downloader respondsToSelector:@selector(setPriority:withDownloadIdentifier:)];
   _downloaderFlags.downloaderImplementsAnimatedImage = [downloader respondsToSelector:@selector(animatedImageWithData:)];
   _downloaderFlags.downloaderImplementsCancelWithResume = [downloader respondsToSelector:@selector(cancelImageDownloadWithResumePossibilityForIdentifier:)];
+  _downloaderFlags.downloaderImplementsDownloadURLs = [downloader respondsToSelector:@selector(downloadImageWithURLs:callbackQueue:downloadProgress:completion:)];
 
   _cacheFlags.cacheSupportsClearing = [cache respondsToSelector:@selector(clearFetchedImageFromCacheWithURL:)];
   _cacheFlags.cacheSupportsSynchronousFetch = [cache respondsToSelector:@selector(synchronouslyFetchedCachedImageWithURL:)];
+  _cacheFlags.cacheSupportsCachedURLs = [cache respondsToSelector:@selector(cachedImageWithURLs:callbackQueue:completion:)];
 
   _shouldCacheImage = YES;
   _shouldRenderProgressImages = YES;
@@ -136,8 +140,8 @@ - (void)_locked_setImage:(UIImage *)image
   BOOL shouldCancelAndClear = imageWasSetExternally && (imageWasSetExternally != _imageWasSetExternally);
   _imageWasSetExternally = imageWasSetExternally;
   if (shouldCancelAndClear) {
-    ASDisplayNodeAssertNil(_URL, @"Directly setting an image on an ASNetworkImageNode causes it to behave like an ASImageNode instead of an ASNetworkImageNode. If this is what you want, set the URL to nil first.");
-    _URL = nil;
+    ASDisplayNodeAssert(_URLs == nil || _URLs.count == 0, @"Directly setting an image on an ASNetworkImageNode causes it to behave like an ASImageNode instead of an ASNetworkImageNode. If this is what you want, set the URL to nil first.");
+    _URLs = nil;
     [self _locked_cancelDownloadAndClearImageWithResumePossibility:NO];
   }
 
@@ -158,15 +162,38 @@ - (void)_locked__setImage:(UIImage *)image
 
 - (void)setURL:(NSURL *)URL
 {
-  [self setURL:URL resetToDefault:YES];
+  if (URL) {
+    [self setURLs:@[URL]];
+  } else {
+    [self setURLs:nil];
+  }
 }
 
 - (void)setURL:(NSURL *)URL resetToDefault:(BOOL)reset
+{
+  if (URL) {
+    [self setURLs:@[URL] resetToDefault:reset];
+  } else {
+    [self setURLs:nil resetToDefault:reset];
+  }
+}
+
+- (NSURL *)URL
+{
+  return [self.URLs lastObject];
+}
+
+- (void)setURLs:(NSArray <NSURL *> *)URLs
+{
+  [self setURLs:URLs resetToDefault:YES];
+}
+
+- (void)setURLs:(NSArray <NSURL *> *)URLs resetToDefault:(BOOL)reset
 {
   {
     ASDN::MutexLocker l(__instanceLock__);
 
-    if (ASObjectIsEqual(URL, _URL)) {
+    if (ASObjectIsEqual(URLs, _URLs)) {
       return;
     }
 
@@ -175,25 +202,25 @@ - (void)setURL:(NSURL *)URL resetToDefault:(BOOL)reset
     _imageWasSetExternally = NO;
 
     [self _locked_cancelImageDownloadWithResumePossibility:NO];
-
+    
     _imageLoaded = NO;
 
-    _URL = URL;
+    _URLs = URLs;
 
-    BOOL hasURL = (_URL == nil);
+    BOOL hasURL = (_URLs.count == 0);
     if (reset || hasURL) {
       [self _locked_setCurrentImageQuality:(hasURL ? 0.0 : 1.0)];
       [self _locked__setImage:_defaultImage];
     }
   }
-
+  
   [self setNeedsPreload];
 }
 
-- (NSURL *)URL
+- (NSArray <NSURL *>*)URLs
 {
   ASDN::MutexLocker l(__instanceLock__);
-  return _URL;
+  return _URLs;
 }
 
 - (void)setDefaultImage:(UIImage *)defaultImage
@@ -212,7 +239,7 @@ - (void)_locked_setDefaultImage:(UIImage *)defaultImage
   _defaultImage = defaultImage;
 
   if (!_imageLoaded) {
-    [self _locked_setCurrentImageQuality:((_URL == nil) ? 0.0 : 1.0)];
+    [self _locked_setCurrentImageQuality:((_URLs.count == 0) ? 0.0 : 1.0)];
     [self _locked__setImage:defaultImage];
 
   }
@@ -310,7 +337,7 @@ - (BOOL)shouldRenderProgressImages
 - (BOOL)placeholderShouldPersist
 {
   ASDN::MutexLocker l(__instanceLock__);
-  return (self.image == nil && self.animatedImage == nil && _URL != nil);
+  return (self.image == nil && self.animatedImage == nil && _URLs.count != 0);
 }
 
 /* displayWillStartAsynchronously: in ASMultiplexImageNode has a very similar implementation. Changes here are likely necessary
@@ -322,13 +349,16 @@ - (void)displayWillStartAsynchronously:(BOOL)asynchronously
   if (asynchronously == NO && _cacheFlags.cacheSupportsSynchronousFetch) {
     ASDN::MutexLocker l(__instanceLock__);
 
-    if (_imageLoaded == NO && _URL && _downloadIdentifier == nil) {
-      UIImage *result = [[_cache synchronouslyFetchedCachedImageWithURL:_URL] asdk_image];
-      if (result) {
-        [self _locked_setCurrentImageQuality:1.0];
-        [self _locked__setImage:result];
-
-        _imageLoaded = YES;
+    if (_imageLoaded == NO && _URLs.count > 0 && _downloadIdentifier == nil) {
+      for (NSURL *url in [_URLs reverseObjectEnumerator]) {
+        UIImage *result = [[_cache synchronouslyFetchedCachedImageWithURL:url] asdk_image];
+        if (result) {
+          [self _locked_setCurrentImageQuality:1.0];
+          [self _locked__setImage:result];
+
+          _imageLoaded = YES;
+          break;
+        }
       }
     }
   }
@@ -510,9 +540,11 @@ - (void)_locked_cancelDownloadAndClearImageWithResumePossibility:(BOOL)storeResu
   _imageLoaded = NO;
 
   if (_cacheFlags.cacheSupportsClearing) {
-    if (_URL != nil) {
-      as_log_verbose(ASImageLoadingLog(), "Clearing cached image for %@ url: %@", self, _URL);
-      [_cache clearFetchedImageFromCacheWithURL:_URL];
+    if (_URLs.count != 0) {
+      as_log_verbose(ASImageLoadingLog(), "Clearing cached image for %@ url: %@", self, _URLs);
+      for (NSURL *url in _URLs) {
+        [_cache clearFetchedImageFromCacheWithURL:url];
+      }
     }
   }
 }
@@ -546,7 +578,7 @@ - (void)_locked_cancelImageDownloadWithResumePossibility:(BOOL)storeResume
 - (void)_downloadImageWithCompletion:(void (^)(id <ASImageContainerProtocol> imageContainer, NSError*, id downloadIdentifier))finished
 {
   ASPerformBlockOnBackgroundThread(^{
-    NSURL *url;
+    NSArray <NSURL *> *urls;
     id downloadIdentifier;
     BOOL cancelAndReattempt = NO;
 
@@ -555,23 +587,34 @@ - (void)_downloadImageWithCompletion:(void (^)(id <ASImageContainerProtocol> ima
     // it and try again.
     {
       ASDN::MutexLocker l(__instanceLock__);
-      url = _URL;
+      urls = _URLs;
     }
 
-
-    downloadIdentifier = [_downloader downloadImageWithURL:url
-                                             callbackQueue:dispatch_get_main_queue()
-                                          downloadProgress:NULL
-                                                completion:^(id <ASImageContainerProtocol> _Nullable imageContainer, NSError * _Nullable error, id  _Nullable downloadIdentifier) {
-                                                  if (finished != NULL) {
-                                                    finished(imageContainer, error, downloadIdentifier);
-                                                  }
-                                                }];
+    if (_downloaderFlags.downloaderImplementsDownloadURLs) {
+      downloadIdentifier = [_downloader downloadImageWithURLs:urls
+                                                callbackQueue:dispatch_get_main_queue()
+                                             downloadProgress:NULL
+                                                   completion:^(id <ASImageContainerProtocol> _Nullable imageContainer, NSError * _Nullable error, id  _Nullable downloadIdentifier) {
+                                                     if (finished != NULL) {
+                                                       finished(imageContainer, error, downloadIdentifier);
+                                                     }
+                                                   }];
+    } else {
+      downloadIdentifier = [_downloader downloadImageWithURL:[urls lastObject]
+                                               callbackQueue:dispatch_get_main_queue()
+                                            downloadProgress:NULL
+                                                  completion:^(id <ASImageContainerProtocol> _Nullable imageContainer, NSError * _Nullable error, id  _Nullable downloadIdentifier) {
+                                                    if (finished != NULL) {
+                                                      finished(imageContainer, error, downloadIdentifier);
+                                                    }
+                                                  }];
+    }
+
     as_log_verbose(ASImageLoadingLog(), "Downloading image for %@ url: %@", self, url);
 
     {
       ASDN::MutexLocker l(__instanceLock__);
-      if (ASObjectIsEqual(_URL, url)) {
+      if (ASObjectIsEqual(_URLs, urls)) {
         // The download we kicked off is correct, no need to do any more work.
         _downloadIdentifier = downloadIdentifier;
       } else {
@@ -600,34 +643,36 @@ - (void)_lazilyLoadImageIfNecessary
     __weak id<ASNetworkImageNodeDelegate> delegate = _delegate;
     BOOL delegateDidStartFetchingData = _delegateFlags.delegateDidStartFetchingData;
     BOOL isImageLoaded = _imageLoaded;
-    NSURL *URL = _URL;
+    NSArray <NSURL *>*URLs = _URLs;
     id currentDownloadIdentifier = _downloadIdentifier;
   __instanceLock__.unlock();
 
-  if (!isImageLoaded && URL != nil && currentDownloadIdentifier == nil) {
+  if (!isImageLoaded && URLs.count > 0 && currentDownloadIdentifier == nil) {
     if (delegateDidStartFetchingData) {
       [delegate imageNodeDidStartFetchingData:self];
     }
 
-    if (URL.isFileURL) {
+    // We only support file URLs if there is one URL currently
+    if (URLs.count == 1 && [URLs lastObject].isFileURL) {
       dispatch_async(dispatch_get_main_queue(), ^{
         ASDN::MutexLocker l(__instanceLock__);
 
         // Bail out if not the same URL anymore
-        if (!ASObjectIsEqual(URL, _URL)) {
+        if (!ASObjectIsEqual(URLs, _URLs)) {
           return;
         }
 
+        NSURL *URL = [URLs lastObject];
         if (_shouldCacheImage) {
-          [self _locked__setImage:[UIImage imageNamed:_URL.path.lastPathComponent]];
+          [self _locked__setImage:[UIImage imageNamed:URL.path.lastPathComponent]];
         } else {
           // First try to load the path directly, for efficiency assuming a developer who
           // doesn't want caching is trying to be as minimal as possible.
-          UIImage *nonAnimatedImage = [UIImage imageWithContentsOfFile:_URL.path];
+          UIImage *nonAnimatedImage = [UIImage imageWithContentsOfFile:URL.path];
           if (nonAnimatedImage == nil) {
             // If we couldn't find it, execute an -imageNamed:-like search so we can find resources even if the
             // extension is not provided in the path.  This allows the same path to work regardless of shouldCacheImage.
-            NSString *filename = [[NSBundle mainBundle] pathForResource:_URL.path.lastPathComponent ofType:nil];
+            NSString *filename = [[NSBundle mainBundle] pathForResource:URL.path.lastPathComponent ofType:nil];
             if (filename != nil) {
               nonAnimatedImage = [UIImage imageWithContentsOfFile:filename];
             }
@@ -636,7 +681,7 @@ - (void)_lazilyLoadImageIfNecessary
           // If the file may be an animated gif and then created an animated image.
           id<ASAnimatedImageProtocol> animatedImage = nil;
           if (_downloaderFlags.downloaderImplementsAnimatedImage) {
-            NSData *data = [NSData dataWithContentsOfURL:_URL];
+            NSData *data = [NSData dataWithContentsOfURL:URL];
             if (data != nil) {
               animatedImage = [_downloader animatedImageWithData:data];
 
@@ -671,7 +716,7 @@ - (void)_lazilyLoadImageIfNecessary
           return;
         }
 
-        as_log_verbose(ASImageLoadingLog(), "Downloaded image for %@ img: %@ url: %@", self, [imageContainer asdk_image], URL);
+        as_log_verbose(ASImageLoadingLog(), "Downloaded image for %@ img: %@ urls: %@", self, [imageContainer asdk_image], URLs);
 
         // Grab the lock for the rest of the block
         ASDN::MutexLocker l(strongSelf->__instanceLock__);
@@ -714,26 +759,35 @@ - (void)_lazilyLoadImageIfNecessary
           _cacheUUID = cacheUUID;
         __instanceLock__.unlock();
 
-        as_log_verbose(ASImageLoadingLog(), "Decaching image for %@ url: %@", self, URL);
-        [_cache cachedImageWithURL:URL
-                     callbackQueue:dispatch_get_main_queue()
-                        completion:^(id <ASImageContainerProtocol> imageContainer) {
-                          // If the cache UUID changed, that means this request was cancelled.
-                          __instanceLock__.lock();
-                            NSUUID *currentCacheUUID = _cacheUUID;
-                          __instanceLock__.unlock();
-
-                          if (!ASObjectIsEqual(currentCacheUUID, cacheUUID)) {
-                            return;
-                          }
-
-                          if ([imageContainer asdk_image] == nil && _downloader != nil) {
-                            [self _downloadImageWithCompletion:finished];
-                          } else {
-                            as_log_verbose(ASImageLoadingLog(), "Decached image for %@ img: %@ url: %@", self, [imageContainer asdk_image], URL);
-                            finished(imageContainer, nil, nil);
-                          }
-                        }];
+        as_log_verbose(ASImageLoadingLog(), "Decaching image for %@ urls: %@", self, URLs);
+
+        ASImageCacherCompletion completion = ^(id <ASImageContainerProtocol> imageContainer) {
+          // If the cache UUID changed, that means this request was cancelled.
+          __instanceLock__.lock();
+          NSUUID *currentCacheUUID = _cacheUUID;
+          __instanceLock__.unlock();
+
+          if (!ASObjectIsEqual(currentCacheUUID, cacheUUID)) {
+            return;
+          }
+
+          if ([imageContainer asdk_image] == nil && _downloader != nil) {
+            [self _downloadImageWithCompletion:finished];
+          } else {
+            as_log_verbose(ASImageLoadingLog(), "Decached image for %@ img: %@ urls: %@", self, [imageContainer asdk_image], URLs);
+            finished(imageContainer, nil, nil);
+          }
+        };
+
+        if (_cacheFlags.cacheSupportsCachedURLs) {
+          [_cache cachedImageWithURLs:URLs
+                        callbackQueue:dispatch_get_main_queue()
+                           completion:completion];
+        } else {
+          [_cache cachedImageWithURL:[URLs lastObject]
+                       callbackQueue:dispatch_get_main_queue()
+                          completion:completion];
+        }
       } else {
         [self _downloadImageWithCompletion:finished];
       }