Refactory the current thumbnail && transformer about cache key. Developer should have the API to calcualte the cache key from thumbnail or transformer, not hard-coded. #2966
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dreampiggy
commented
Mar 31, 2020
| @return The transformed image, or nil if transform failed | ||
| */ | ||
| - (nullable UIImage *)transformedImageWithImage:(nonnull UIImage *)image forKey:(nonnull NSString *)key; | ||
| - (nullable UIImage *)transformedImageWithImage:(nonnull UIImage *)image forKey:(nonnull NSString *)key API_DEPRECATED("The key arg will be removed in the future. Update your code and don't rely on that.", macos(10.10, API_TO_BE_DEPRECATED), ios(8.0, API_TO_BE_DEPRECATED), tvos(9.0, API_TO_BE_DEPRECATED), watchos(2.0, API_TO_BE_DEPRECATED)); |
There was a problem hiding this comment.
Can we change the method signature to nullable for this key arg ?
There was a problem hiding this comment.
It's okey to change in nullable.
There was a problem hiding this comment.
Seems change it to nullable will cause API break for exist custom protocol for Swift User. (Objective-C does not cause issue). So let keep it until 6.0.0
|
Could you update some test case? |
dreampiggy
force-pushed
the
bugfix_refactory_thumbnail_cache_key
branch
2 times, most recently
from
3d7ddae
to
499540c
Compare
Codecov Report
@@ Coverage Diff @@
## master #2966 +/- ##
==========================================
- Coverage 83.83% 83.71% -0.13%
==========================================
Files 69 69
Lines 7286 7312 +26
==========================================
+ Hits 6108 6121 +13
- Misses 1178 1191 +13
Continue to review full report at Codecov.
|
dreampiggy
force-pushed
the
bugfix_refactory_thumbnail_cache_key
branch
2 times, most recently
from
93b665c
to
fac9535
Compare
…oper should have the API to calcualte the cache key from thumbnail or transformer, not hard-coded.
…ass null to disable this.
dreampiggy
force-pushed
the
bugfix_refactory_thumbnail_cache_key
branch
from
fac9535
to
b766c58
Compare
dreampiggy
commented
Apr 2, 2020
SDWebImage/Core/SDImageCache.h
Outdated
| @@ -227,7 +227,7 @@ typedef NS_OPTIONS(NSUInteger, SDImageCacheOptions) { | |||
| #pragma mark - Query and Retrieve Ops | |||
|
|
|||
| /** | |||
| * Asynchronously queries the cache with operation and call the completion when done. | |||
| * Synchronously queries the cache with operation and call the completion when done. | |||
There was a problem hiding this comment.
This documentation needs update...
kinarobin
approved these changes
Apr 3, 2020
…eeded if you have animated image/transformer/thumbnail usage
dreampiggy
force-pushed
the
bugfix_refactory_thumbnail_cache_key
branch
from
b766c58
to
8c2141e
Compare
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 / refers to the following issues: ...
Pull Request Description
Motivation
Current Thumbnail (introduced in 5.6.0) and Transformer with cache key is not works correct. For example:
imageFromMemoryForKey:) need to care about and generate cache key manually"Thumbnail({%f,%f},%d)) does not have any public way to access this unless you harcode. Which means, you can not directly get the thumbnail image from cache by yourself without SDWebImageManager.Changes
New API:
SDThumbnailedKeyForKey()Used for public API to generate thumbnail cache key, if you access the cache directly without SDWebImageManager.
+
cacheKeyForURL:context:Used for user who don't want to care about what context in it (it may have thumbnail pixel size, it may have cache filter, it may have transformer...etc). Just pass in the context, we give you the final cache key which can directly passed to
SDImageCache