Skip to content

Commit 008a1ce

Browse files
garrettmoongarrettmoon
authored
Revert Adds support for specifying a quality indexed array of URLs (facebookarchive#699)
* Revert "Adds support for specifying a quality indexed array of URLs (facebookarchive#557)" This reverts commit 3c77d4a. * Add CHANGELOG entry
1 parent 0b6d41f commit 008a1ce

File tree

5 files changed

+83
-176
lines changed

5 files changed

+83
-176
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
## master
22
* Add your own contributions to the next release on the line below this with your name.
3+
- [ASNetworkImageNode] Deprecates .URLs in favor of .URL [Garrett Moon](https://github.com/garrettmoon) [#699](https://github.com/TextureGroup/Texture/pull/699)
34
- [iOS11] Update project settings and fix errors [Eke](https://github.com/Eke) [#676](https://github.com/TextureGroup/Texture/pull/676)
45
- [ASCollectionView] Improve performance and behavior of rotation / bounds changes. [Scott Goodson](https://github.com/appleguy) [#431](https://github.com/TextureGroup/Texture/pull/431)
56
- [ASCollectionView] Improve index space translation of Flow Layout Delegate methods. [Scott Goodson](https://github.com/appleguy)

‎Source/ASNetworkImageNode.h‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,13 +83,16 @@ NS_ASSUME_NONNULL_BEGIN
8383
@property (nullable, nonatomic, strong, readwrite) NSURL *URL;
8484

8585
/**
86-
* An array of URLs of increasing cost to download.
87-
*
88-
* @discussion By setting an array of URLs, the image property of this node will be managed internally. This means previously
89-
* directly set images to the image property will be cleared out and replaced by the placeholder (<defaultImage>) image
90-
* while loading and the final image after the new image data was downloaded and processed.
91-
*/
92-
@property (nullable, nonatomic, strong, readwrite) NSArray <NSURL *> *URLs;
86+
* An array of URLs of increasing cost to download.
87+
*
88+
* @discussion By setting an array of URLs, the image property of this node will be managed internally. This means previously
89+
* directly set images to the image property will be cleared out and replaced by the placeholder (<defaultImage>) image
90+
* while loading and the final image after the new image data was downloaded and processed.
91+
*
92+
* @deprecated This API has been removed for now due to the increased complexity to the class that it brought.
93+
* Please use .URL instead.
94+
*/
95+
@property (nullable, nonatomic, strong, readwrite) NSArray <NSURL *> *URLs ASDISPLAYNODE_DEPRECATED_MSG("Please use URL instead.");
9396

9497
/**
9598
* Download and display a new image.

‎Source/ASNetworkImageNode.mm‎

Lines changed: 66 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ @interface ASNetworkImageNode ()
3737
// Only access any of these with __instanceLock__.
3838
__weak id<ASNetworkImageNodeDelegate> _delegate;
3939

40-
NSArray *_URLs;
40+
NSURL *_URL;
4141
UIImage *_defaultImage;
4242

4343
NSUUID *_cacheUUID;
@@ -67,15 +67,13 @@ @interface ASNetworkImageNode ()
6767
unsigned int downloaderImplementsSetPriority:1;
6868
unsigned int downloaderImplementsAnimatedImage:1;
6969
unsigned int downloaderImplementsCancelWithResume:1;
70-
unsigned int downloaderImplementsDownloadURLs:1;
7170
} _downloaderFlags;
7271

7372
// Immutable and set on init only. We don't need to lock in this case.
7473
__weak id<ASImageCacheProtocol> _cache;
7574
struct {
7675
unsigned int cacheSupportsClearing:1;
7776
unsigned int cacheSupportsSynchronousFetch:1;
78-
unsigned int cacheSupportsCachedURLs:1;
7977
} _cacheFlags;
8078
}
8179

@@ -97,11 +95,9 @@ - (instancetype)initWithCache:(id<ASImageCacheProtocol>)cache downloader:(id<ASI
9795
_downloaderFlags.downloaderImplementsSetPriority = [downloader respondsToSelector:@selector(setPriority:withDownloadIdentifier:)];
9896
_downloaderFlags.downloaderImplementsAnimatedImage = [downloader respondsToSelector:@selector(animatedImageWithData:)];
9997
_downloaderFlags.downloaderImplementsCancelWithResume = [downloader respondsToSelector:@selector(cancelImageDownloadWithResumePossibilityForIdentifier:)];
100-
_downloaderFlags.downloaderImplementsDownloadURLs = [downloader respondsToSelector:@selector(downloadImageWithURLs:callbackQueue:downloadProgress:completion:)];
10198

10299
_cacheFlags.cacheSupportsClearing = [cache respondsToSelector:@selector(clearFetchedImageFromCacheWithURL:)];
103100
_cacheFlags.cacheSupportsSynchronousFetch = [cache respondsToSelector:@selector(synchronouslyFetchedCachedImageWithURL:)];
104-
_cacheFlags.cacheSupportsCachedURLs = [cache respondsToSelector:@selector(cachedImageWithURLs:callbackQueue:completion:)];
105101

106102
_shouldCacheImage = YES;
107103
_shouldRenderProgressImages = YES;
@@ -139,8 +135,8 @@ - (void)_locked_setImage:(UIImage *)image
139135
BOOL shouldCancelAndClear = imageWasSetExternally && (imageWasSetExternally != _imageWasSetExternally);
140136
_imageWasSetExternally = imageWasSetExternally;
141137
if (shouldCancelAndClear) {
142-
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.");
143-
_URLs = nil;
138+
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.");
139+
_URL = nil;
144140
[self _locked_cancelDownloadAndClearImageWithResumePossibility:NO];
145141
}
146142

@@ -159,40 +155,29 @@ - (void)_locked__setImage:(UIImage *)image
159155
[super _locked_setImage:image];
160156
}
161157

162-
- (void)setURL:(NSURL *)URL
163-
{
164-
if (URL) {
165-
[self setURLs:@[URL]];
166-
} else {
167-
[self setURLs:nil];
168-
}
169-
}
170-
171-
- (void)setURL:(NSURL *)URL resetToDefault:(BOOL)reset
158+
// Deprecated
159+
- (void)setURLs:(NSArray<NSURL *> *)URLs
172160
{
173-
if (URL) {
174-
[self setURLs:@[URL] resetToDefault:reset];
175-
} else {
176-
[self setURLs:nil resetToDefault:reset];
177-
}
161+
[self setURL:[URLs firstObject]];
178162
}
179163

180-
- (NSURL *)URL
164+
// Deprecated
165+
- (NSArray<NSURL *> *)URLs
181166
{
182-
return [self.URLs lastObject];
167+
return @[self.URL];
183168
}
184169

185-
- (void)setURLs:(NSArray <NSURL *> *)URLs
170+
- (void)setURL:(NSURL *)URL
186171
{
187-
[self setURLs:URLs resetToDefault:YES];
172+
[self setURL:URL resetToDefault:YES];
188173
}
189174

190-
- (void)setURLs:(NSArray <NSURL *> *)URLs resetToDefault:(BOOL)reset
175+
- (void)setURL:(NSURL *)URL resetToDefault:(BOOL)reset
191176
{
192177
{
193178
ASDN::MutexLocker l(__instanceLock__);
194179

195-
if (ASObjectIsEqual(URLs, _URLs)) {
180+
if (ASObjectIsEqual(URL, _URL)) {
196181
return;
197182
}
198183

@@ -201,25 +186,25 @@ - (void)setURLs:(NSArray <NSURL *> *)URLs resetToDefault:(BOOL)reset
201186
_imageWasSetExternally = NO;
202187

203188
[self _locked_cancelImageDownloadWithResumePossibility:NO];
204-
189+
205190
_imageLoaded = NO;
206191

207-
_URLs = URLs;
192+
_URL = URL;
208193

209-
BOOL hasURL = (_URLs.count == 0);
194+
BOOL hasURL = (_URL == nil);
210195
if (reset || hasURL) {
211196
[self _locked_setCurrentImageQuality:(hasURL ? 0.0 : 1.0)];
212197
[self _locked__setImage:_defaultImage];
213198
}
214199
}
215-
200+
216201
[self setNeedsPreload];
217202
}
218203

219-
- (NSArray <NSURL *>*)URLs
204+
- (NSURL *)URL
220205
{
221206
ASDN::MutexLocker l(__instanceLock__);
222-
return _URLs;
207+
return _URL;
223208
}
224209

225210
- (void)setDefaultImage:(UIImage *)defaultImage
@@ -238,7 +223,7 @@ - (void)_locked_setDefaultImage:(UIImage *)defaultImage
238223
_defaultImage = defaultImage;
239224

240225
if (!_imageLoaded) {
241-
[self _locked_setCurrentImageQuality:((_URLs.count == 0) ? 0.0 : 1.0)];
226+
[self _locked_setCurrentImageQuality:((_URL == nil) ? 0.0 : 1.0)];
242227
[self _locked__setImage:defaultImage];
243228

244229
}
@@ -337,7 +322,7 @@ - (BOOL)shouldRenderProgressImages
337322
- (BOOL)placeholderShouldPersist
338323
{
339324
ASDN::MutexLocker l(__instanceLock__);
340-
return (self.image == nil && self.animatedImage == nil && _URLs.count != 0);
325+
return (self.image == nil && self.animatedImage == nil && _URL != nil);
341326
}
342327

343328
/* displayWillStartAsynchronously: in ASMultiplexImageNode has a very similar implementation. Changes here are likely necessary
@@ -349,25 +334,22 @@ - (void)displayWillStartAsynchronously:(BOOL)asynchronously
349334
if (asynchronously == NO && _cacheFlags.cacheSupportsSynchronousFetch) {
350335
ASDN::MutexLocker l(__instanceLock__);
351336

352-
if (_imageLoaded == NO && _URLs.count > 0 && _downloadIdentifier == nil) {
353-
for (NSURL *url in [_URLs reverseObjectEnumerator]) {
354-
UIImage *result = [[_cache synchronouslyFetchedCachedImageWithURL:url] asdk_image];
355-
if (result) {
356-
[self _locked_setCurrentImageQuality:1.0];
357-
[self _locked__setImage:result];
358-
_imageLoaded = YES;
359-
360-
// Call out to the delegate.
361-
if (_delegateFlags.delegateDidLoadImageWithInfo) {
362-
ASDN::MutexUnlocker l(__instanceLock__);
363-
ASNetworkImageNodeDidLoadInfo info = {};
364-
info.imageSource = ASNetworkImageSourceSynchronousCache;
365-
[_delegate imageNode:self didLoadImage:result info:info];
366-
} else if (_delegateFlags.delegateDidLoadImage) {
367-
ASDN::MutexUnlocker l(__instanceLock__);
368-
[_delegate imageNode:self didLoadImage:result];
369-
}
370-
break;
337+
if (_imageLoaded == NO && _URL && _downloadIdentifier == nil) {
338+
UIImage *result = [[_cache synchronouslyFetchedCachedImageWithURL:_URL] asdk_image];
339+
if (result) {
340+
[self _locked_setCurrentImageQuality:1.0];
341+
[self _locked__setImage:result];
342+
_imageLoaded = YES;
343+
344+
// Call out to the delegate.
345+
if (_delegateFlags.delegateDidLoadImageWithInfo) {
346+
ASDN::MutexUnlocker l(__instanceLock__);
347+
ASNetworkImageNodeDidLoadInfo info = {};
348+
info.imageSource = ASNetworkImageSourceSynchronousCache;
349+
[_delegate imageNode:self didLoadImage:result info:info];
350+
} else if (_delegateFlags.delegateDidLoadImage) {
351+
ASDN::MutexUnlocker l(__instanceLock__);
352+
[_delegate imageNode:self didLoadImage:result];
371353
}
372354
}
373355
}
@@ -538,11 +520,9 @@ - (void)_locked_cancelDownloadAndClearImageWithResumePossibility:(BOOL)storeResu
538520
_imageLoaded = NO;
539521

540522
if (_cacheFlags.cacheSupportsClearing) {
541-
if (_URLs.count != 0) {
542-
as_log_verbose(ASImageLoadingLog(), "Clearing cached image for %@ url: %@", self, _URLs);
543-
for (NSURL *url in _URLs) {
544-
[_cache clearFetchedImageFromCacheWithURL:url];
545-
}
523+
if (_URL != nil) {
524+
as_log_verbose(ASImageLoadingLog(), "Clearing cached image for %@ url: %@", self, _URL);
525+
[_cache clearFetchedImageFromCacheWithURL:_URL];
546526
}
547527
}
548528
}
@@ -576,7 +556,7 @@ - (void)_locked_cancelImageDownloadWithResumePossibility:(BOOL)storeResume
576556
- (void)_downloadImageWithCompletion:(void (^)(id <ASImageContainerProtocol> imageContainer, NSError*, id downloadIdentifier))finished
577557
{
578558
ASPerformBlockOnBackgroundThread(^{
579-
NSArray <NSURL *> *urls;
559+
NSURL *url;
580560
id downloadIdentifier;
581561
BOOL cancelAndReattempt = NO;
582562

@@ -585,34 +565,23 @@ - (void)_downloadImageWithCompletion:(void (^)(id <ASImageContainerProtocol> ima
585565
// it and try again.
586566
{
587567
ASDN::MutexLocker l(__instanceLock__);
588-
urls = _URLs;
568+
url = _URL;
589569
}
590570

591-
if (_downloaderFlags.downloaderImplementsDownloadURLs) {
592-
downloadIdentifier = [_downloader downloadImageWithURLs:urls
593-
callbackQueue:dispatch_get_main_queue()
594-
downloadProgress:NULL
595-
completion:^(id <ASImageContainerProtocol> _Nullable imageContainer, NSError * _Nullable error, id _Nullable downloadIdentifier) {
596-
if (finished != NULL) {
597-
finished(imageContainer, error, downloadIdentifier);
598-
}
599-
}];
600-
} else {
601-
downloadIdentifier = [_downloader downloadImageWithURL:[urls lastObject]
602-
callbackQueue:dispatch_get_main_queue()
603-
downloadProgress:NULL
604-
completion:^(id <ASImageContainerProtocol> _Nullable imageContainer, NSError * _Nullable error, id _Nullable downloadIdentifier) {
605-
if (finished != NULL) {
606-
finished(imageContainer, error, downloadIdentifier);
607-
}
608-
}];
609-
}
610-
571+
572+
downloadIdentifier = [_downloader downloadImageWithURL:url
573+
callbackQueue:dispatch_get_main_queue()
574+
downloadProgress:NULL
575+
completion:^(id <ASImageContainerProtocol> _Nullable imageContainer, NSError * _Nullable error, id _Nullable downloadIdentifier) {
576+
if (finished != NULL) {
577+
finished(imageContainer, error, downloadIdentifier);
578+
}
579+
}];
611580
as_log_verbose(ASImageLoadingLog(), "Downloading image for %@ url: %@", self, url);
612581

613582
{
614583
ASDN::MutexLocker l(__instanceLock__);
615-
if (ASObjectIsEqual(_URLs, urls)) {
584+
if (ASObjectIsEqual(_URL, url)) {
616585
// The download we kicked off is correct, no need to do any more work.
617586
_downloadIdentifier = downloadIdentifier;
618587
} else {
@@ -641,36 +610,34 @@ - (void)_lazilyLoadImageIfNecessary
641610
__weak id<ASNetworkImageNodeDelegate> delegate = _delegate;
642611
BOOL delegateDidStartFetchingData = _delegateFlags.delegateDidStartFetchingData;
643612
BOOL isImageLoaded = _imageLoaded;
644-
NSArray <NSURL *>*URLs = _URLs;
613+
NSURL *URL = _URL;
645614
id currentDownloadIdentifier = _downloadIdentifier;
646615
__instanceLock__.unlock();
647616

648-
if (!isImageLoaded && URLs.count > 0 && currentDownloadIdentifier == nil) {
617+
if (!isImageLoaded && URL != nil && currentDownloadIdentifier == nil) {
649618
if (delegateDidStartFetchingData) {
650619
[delegate imageNodeDidStartFetchingData:self];
651620
}
652621

653-
// We only support file URLs if there is one URL currently
654-
if (URLs.count == 1 && [URLs lastObject].isFileURL) {
622+
if (URL.isFileURL) {
655623
dispatch_async(dispatch_get_main_queue(), ^{
656624
ASDN::MutexLocker l(__instanceLock__);
657625

658626
// Bail out if not the same URL anymore
659-
if (!ASObjectIsEqual(URLs, _URLs)) {
627+
if (!ASObjectIsEqual(URL, _URL)) {
660628
return;
661629
}
662630

663-
NSURL *URL = [URLs lastObject];
664631
if (_shouldCacheImage) {
665-
[self _locked__setImage:[UIImage imageNamed:URL.path.lastPathComponent]];
632+
[self _locked__setImage:[UIImage imageNamed:_URL.path.lastPathComponent]];
666633
} else {
667634
// First try to load the path directly, for efficiency assuming a developer who
668635
// doesn't want caching is trying to be as minimal as possible.
669-
UIImage *nonAnimatedImage = [UIImage imageWithContentsOfFile:URL.path];
636+
UIImage *nonAnimatedImage = [UIImage imageWithContentsOfFile:_URL.path];
670637
if (nonAnimatedImage == nil) {
671638
// If we couldn't find it, execute an -imageNamed:-like search so we can find resources even if the
672639
// extension is not provided in the path. This allows the same path to work regardless of shouldCacheImage.
673-
NSString *filename = [[NSBundle mainBundle] pathForResource:URL.path.lastPathComponent ofType:nil];
640+
NSString *filename = [[NSBundle mainBundle] pathForResource:_URL.path.lastPathComponent ofType:nil];
674641
if (filename != nil) {
675642
nonAnimatedImage = [UIImage imageWithContentsOfFile:filename];
676643
}
@@ -679,7 +646,7 @@ - (void)_lazilyLoadImageIfNecessary
679646
// If the file may be an animated gif and then created an animated image.
680647
id<ASAnimatedImageProtocol> animatedImage = nil;
681648
if (_downloaderFlags.downloaderImplementsAnimatedImage) {
682-
NSData *data = [NSData dataWithContentsOfURL:URL];
649+
NSData *data = [NSData dataWithContentsOfURL:_URL];
683650
if (data != nil) {
684651
animatedImage = [_downloader animatedImageWithData:data];
685652

@@ -719,7 +686,7 @@ - (void)_lazilyLoadImageIfNecessary
719686
return;
720687
}
721688

722-
as_log_verbose(ASImageLoadingLog(), "Downloaded image for %@ img: %@ urls: %@", self, [imageContainer asdk_image], URLs);
689+
as_log_verbose(ASImageLoadingLog(), "Downloaded image for %@ img: %@ url: %@", self, [imageContainer asdk_image], URL);
723690

724691
// Grab the lock for the rest of the block
725692
ASDN::MutexLocker l(strongSelf->__instanceLock__);
@@ -784,7 +751,7 @@ - (void)_lazilyLoadImageIfNecessary
784751
_cacheUUID = cacheUUID;
785752
__instanceLock__.unlock();
786753

787-
as_log_verbose(ASImageLoadingLog(), "Decaching image for %@ urls: %@", self, URLs);
754+
as_log_verbose(ASImageLoadingLog(), "Decaching image for %@ url: %@", self, URL);
788755

789756
ASImageCacherCompletion completion = ^(id <ASImageContainerProtocol> imageContainer) {
790757
// If the cache UUID changed, that means this request was cancelled.
@@ -801,20 +768,13 @@ - (void)_lazilyLoadImageIfNecessary
801768
finished(imageContainer, error, downloadIdentifier, ASNetworkImageSourceDownload);
802769
}];
803770
} else {
804-
as_log_verbose(ASImageLoadingLog(), "Decached image for %@ img: %@ urls: %@", self, [imageContainer asdk_image], URLs);
771+
as_log_verbose(ASImageLoadingLog(), "Decached image for %@ img: %@ url: %@", self, [imageContainer asdk_image], URL);
805772
finished(imageContainer, nil, nil, ASNetworkImageSourceAsynchronousCache);
806773
}
807774
};
808-
809-
if (_cacheFlags.cacheSupportsCachedURLs) {
810-
[_cache cachedImageWithURLs:URLs
811-
callbackQueue:dispatch_get_main_queue()
812-
completion:completion];
813-
} else {
814-
[_cache cachedImageWithURL:[URLs lastObject]
815-
callbackQueue:dispatch_get_main_queue()
816-
completion:completion];
817-
}
775+
[_cache cachedImageWithURL:URL
776+
callbackQueue:dispatch_get_main_queue()
777+
completion:completion];
818778
} else {
819779
[self _downloadImageWithCompletion:^(id<ASImageContainerProtocol> imageContainer, NSError *error, id downloadIdentifier) {
820780
finished(imageContainer, error, downloadIdentifier, ASNetworkImageSourceDownload);

0 commit comments

Comments
 (0)