Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sd_setImageWithURL calls completion block twice when used with SDWebImageRefreshCached #1090

Closed
pronebird opened this issue Mar 23, 2015 · 18 comments

Comments

@pronebird
Copy link

Hi again,

Same code with SDWebImageRefreshCached, calls completion block twice.

[self.photoImageView sd_setImageWithURL:url
                       placeholderImage:nil
                                options:SDWebImageRetryFailed | SDWebImageRefreshCached
                               progress:^(NSInteger receivedSize, NSInteger expectedSize) {
                                   dispatch_async(dispatch_get_main_queue(), ^{
                                       NSLog(@"receivedSize = %d, expectedSize = %d", receivedSize, expectedSize);
                                   });
                               }
                              completed:^(UIImage *image, NSError *error, SDImageCacheType cacheType, NSURL *imageURL) {
                                  NSLog(@"Complete = %d, error = %@", cacheType, error);
                              }];

Log:

2015-03-23 12:38:09.112 App[50401:1875672] Complete = 2, error = (null)
2015-03-23 12:38:09.113 App[50401:1875672] receivedSize = 0, expectedSize = -1
2015-03-23 12:38:11.378 App[50401:1875672] receivedSize = 0, expectedSize = 34220
2015-03-23 12:38:11.378 App[50401:1875672] receivedSize = 1432, expectedSize = 34220
2015-03-23 12:38:11.378 App[50401:1875672] receivedSize = 2864, expectedSize = 34220
2015-03-23 12:38:11.378 App[50401:1875672] receivedSize = 4296, expectedSize = 34220
2015-03-23 12:38:11.380 App[50401:1875672] receivedSize = 5728, expectedSize = 34220
2015-03-23 12:38:11.380 App[50401:1875672] receivedSize = 7160, expectedSize = 34220
2015-03-23 12:38:11.380 App[50401:1875672] receivedSize = 8592, expectedSize = 34220
2015-03-23 12:38:11.381 App[50401:1875672] receivedSize = 10024, expectedSize = 34220
2015-03-23 12:38:11.381 App[50401:1875672] receivedSize = 11456, expectedSize = 34220
2015-03-23 12:38:11.381 App[50401:1875672] receivedSize = 12888, expectedSize = 34220
2015-03-23 12:38:11.572 App[50401:1875672] receivedSize = 14320, expectedSize = 34220
2015-03-23 12:38:11.572 App[50401:1875672] receivedSize = 15752, expectedSize = 34220
2015-03-23 12:38:11.583 App[50401:1875672] receivedSize = 18616, expectedSize = 34220
2015-03-23 12:38:11.597 App[50401:1875672] receivedSize = 21480, expectedSize = 34220
2015-03-23 12:38:11.598 App[50401:1875672] receivedSize = 22912, expectedSize = 34220
2015-03-23 12:38:11.598 App[50401:1875672] receivedSize = 24344, expectedSize = 34220
2015-03-23 12:38:11.599 App[50401:1875672] receivedSize = 25776, expectedSize = 34220
2015-03-23 12:38:11.599 App[50401:1875672] receivedSize = 27208, expectedSize = 34220
2015-03-23 12:38:11.599 App[50401:1875672] receivedSize = 28640, expectedSize = 34220
2015-03-23 12:38:11.599 App[50401:1875672] receivedSize = 30072, expectedSize = 34220
2015-03-23 12:38:11.601 App[50401:1875672] receivedSize = 32936, expectedSize = 34220
2015-03-23 12:38:11.602 App[50401:1875672] receivedSize = 34220, expectedSize = 34220
2015-03-23 12:38:11.618 App[50401:1875672] Complete = 0, error = (null)

Is this a desired behavior? It seems like a bug to me.

@pronebird
Copy link
Author

Update:

I removed second download request I had for the same UIImageView which happened earlier tho and now it's even worse, I get completion callback called, then two updates, then silence.

2015-03-23 13:21:10.588 App[52095:1907863] Complete = 2, error = (null)
2015-03-23 13:21:10.606 App[52095:1907863] receivedSize = 0, expectedSize = -1
2015-03-23 13:21:10.606 App[52095:1907863] receivedSize = 0, expectedSize = 34220
2015-03-23 13:21:10.606 App[52095:1907863] receivedSize = 34220, expectedSize = 34220

This method that provides progress/completion blocks is totally useless.

@mythodeia
Copy link
Contributor

the progress block depends on your server setup as well. if your server setup returns the size of each segment upload then you can use it to show download with percentage.
try and download the same image with chrome.
if you see the a progress showing size downloaded and total size then please share either a link or a small demo so we can debug it

@pronebird
Copy link
Author

I don't see the problem with download from Parse which I use. The progress seems legit. What does not seem legit is that progress is reported after completion block or completion block is never called...

@mythodeia
Copy link
Contributor

are you using the latest build?

@pronebird
Copy link
Author

Using SDWebImage (3.7.2) according to pod install

@pronebird
Copy link
Author

Sometimes it just calls completion block right away, reports that image is cached in memory and then reloads it from server because SDWebImageRefreshCached was set. But it does not call completion block after image is refreshed from server.

Could you please test and confirm (or not?) the same problem on your end if you use the same image view and force load the same image twice? Can be in sequence after image is loaded first time.

@mythodeia
Copy link
Contributor

i tried in the demo project and you are right on the fact that it calls the completion block 1st before showing the progress report when SDWebImageRefreshCached is set.
if you remove the SDWebImageRefreshCached it works fine and reports progress before calling the completion block

@pronebird
Copy link
Author

Thanks, yeah it seems odd, I think it should because that's the whole point of SDWebImageRefreshCached I guess..

@mythodeia
Copy link
Contributor

from the comments inside the SDWebImagemanager.m line 163

If image was found in the cache & SDWebImageRefreshCached is provided, 
notify about the cached image AND try to re-download it in order to let a chance
 to NSURLCache to refresh it from server.

thats why you see a completion block 1st and then a small progress when you set the SDWebImageRefreshCached option

@pronebird
Copy link
Author

Yeah so even if I ignore cached image in completion handler, I am still not being notified when image is refreshed from server.

@mythodeia
Copy link
Contributor

if there was a new image in the server then it should be downloaded.
After the 1st completion block, the one that notifies you that the image exists in cache, the manager tries to re-download it again from the web in order to give NSURLCache a chance to refresh it from server.
logically it all depends on the headers you supply back as a response.

@pronebird
Copy link
Author

I see, in my case it hits 304 for sure. I still think that this method defeats its purpose, e.g. too much of internal implementation details exposed, threading issues (#1089) and inconsistency that makes it totally useless in case if you want to animate anything alongside. The only way to get anything useful out of it is to use progress handler and completely forget about completion handler, which is supposed to be called on completion, not randomly. I apologize if I push too hard here.

@mythodeia
Copy link
Contributor

no worries. Productive criticism is welcome.
In your case since it hits 304, it does not re-download the image again as it has not been modified.
i am using it without any issues so far to show progress but i agree that there are bits and pieces that need to be addressed.
Please if you find some time and have anything to offer, submit a pull request and we will be happy to take a look at it.
Closing this.

@timkoma
Copy link

timkoma commented Mar 23, 2015

@pronebird: I tried to fix issues you are complaining here(#1089 #1090 and some others)

You can find it here:
https://github.com/timkoma/SDWebImage/tree/SDWebImageOperation%2BCanceling/SDWebImage

I think that it will call completedBlock with cancelled error code in case '304', but it can be easy fixed.

I will be happy if some body can test it.:-)

@pronebird
Copy link
Author

@mythodeia thanks for discussing that matter with me.

@timkoma did you send PR already? I'll take a look at your fix.

@timkoma
Copy link

timkoma commented Mar 24, 2015

#1069

@timkoma
Copy link

timkoma commented Apr 6, 2015

@pronebird Did you have a time for review or test ?

@pronebird
Copy link
Author

@timkoma Sorry, I've been working a lot on other projects and I had no time to deal with that bug or review your fix so I approached the problem differently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants