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

Add the SDWebImageContextStoreCacheType context option to specify target cache type when the image is downloaded by manager and will store to cache #2360

Merged

Conversation

dreampiggy
Copy link
Contributor

@dreampiggy dreampiggy commented Jun 15, 2018

This replace the previous SDWebImageCacheMemoryOnly option, which is not so accurate and lack of detail control for cache behavior.

New Pull Request Checklist

  • I have read and understood the CONTRIBUTING guide

  • I have read the Documentation

  • I have searched for a similar pull request in the project and found none

  • I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)

  • I have added the required tests to prove the fix/feature I am adding

  • I have updated the documentation (if necessary)

  • I have run the tests and they pass

  • I have run the lint and it passes (pod lib lint)

This merge request fixes / reffers to the following issues: ...

Pull Request Description

Reason

Current SDWebImageCacheMemoryOnly options is good for some big images, or tempory image which is no need to store in the disk cache.

However, there are still some people including me, hope to totally disable cache storage for some of images. For example, when query the image from Photos Library plugin, we don't want to store the image into disk cache or memory cache at all. Because the cost to load the image again is quite cheap.

We already done something like Custom Cache and allow user to sepcify 4 type of store cache behavior : store disk only, store memory only, store memory and disk, do not store. So I think now we can adopt a better solution for our user who need the detailed control for cache behavior.

Design

We should treat this cache type control as individual image request level. So the best choice is using SDWebImageContextOption. We just pass the target cache type from the user and use that to decide cache storage.

Implementation

/**
 A SDImageCacheType raw value which specify the cache type when the image has just been downloaded and will be stored to the cache. Specify `SDImageCacheTypeNone` to disable cache storage; `SDImageCacheTypeDisk` to store in disk cache only; `SDImageCacheTypeMemory` to store in memory only. And `SDImageCacheTypeAll` to store in both memory cache and disk cache.
 If not provide or the value is invalid, we will use `SDImageCacheTypeAll`. (NSNumber)
 */
FOUNDATION_EXPORT SDWebImageContextOption _Nonnull const SDWebImageContextStoreCacheType;

…etailed cache when the image is downloaded by manager and will store to cache.

This replace the previous `SDWebImageCacheMemoryOnly` option, which is not so accurate and lack of detail control for cache behavior.
@codecov-io
Copy link

Codecov Report

Merging #2360 into 5.x will decrease coverage by 0.11%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff            @@
##              5.x   #2360      +/-   ##
=========================================
- Coverage   69.22%   69.1%   -0.12%     
=========================================
  Files          48      48              
  Lines        6690    6690              
=========================================
- Hits         4631    4623       -8     
- Misses       2059    2067       +8
Flag Coverage Δ
#iOS 70.07% <50%> (-0.17%) ⬇️
#macOS 68.03% <50%> (+0.08%) ⬆️
Impacted Files Coverage Δ
SDWebImage/SDWebImageDefine.m 32.87% <ø> (ø) ⬆️
SDWebImage/SDWebImageManager.m 82.25% <50%> (ø) ⬆️
SDWebImage/SDWebImageDownloaderOperation.m 85.32% <0%> (-3.08%) ⬇️
SDWebImage/SDWebImageDownloader.m 81.5% <0%> (+0.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 997bf8f...e5bee68. Read the comment docs.

@dreampiggy
Copy link
Contributor Author

dreampiggy commented Jun 15, 2018

Current break change (Need change code)

SDWebImageDefine.h

  • SDWebImageCacheMemoryOnly REMOVED. Use SDWebImageContextStoreCacheType option with SDImageCacheTypeMemory instead.

Copy link
Contributor

@mythodeia mythodeia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@dreampiggy dreampiggy merged commit ab75235 into SDWebImage:5.x Jun 23, 2018
@dreampiggy dreampiggy deleted the feature_manager_store_cache_option branch June 23, 2018 05:45
@bpoplauschi
Copy link
Member

👍

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

Successfully merging this pull request may close these issues.

None yet

4 participants