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

Resource Identifier and URL #93

Closed
RuiAAPeres opened this issue Aug 13, 2015 · 3 comments
Closed

Resource Identifier and URL #93

RuiAAPeres opened this issue Aug 13, 2015 · 3 comments

Comments

@RuiAAPeres
Copy link

With the current API we are forced to use the URL as the resource identifier. This won't work in non-trivial scenarios, because we might want the identifier to be different from the URL. For example: creating a thumbnail out of an image.

The image itself as a specific URL (e.g. http://abc.jpg). But I want to cache both http://abc.jpg and http://abc.jpg_thumbnail (this _thumbnail is generated from the original one). The problem is when I want to fetch the thumbnail from the cache. Because if I pass http://abc.jpg_thumbnail and it's not in the cache, it will try to fetch http://abc.jpg_thumbnail which will fail. If I pass http://abc.jpg then it will never touch the thumbnail and get the full size one.

The fundamental problem is: the cache identifier and the resource's URL are coupled. A quick and easy solution would be to use a struct like this:

struct Resource {
let key : String
let resourceURL : NSURL
}

We would then pass this struct, instead of the URL. For example:

public func retrieveImageWithURL(URL: NSURL, optionsInfo: Kingfisher.KingfisherOptionsInfo?, progressBlock: Kingfisher.DownloadProgressBlock?, completionHandler: Kingfisher.CompletionHandler?) -> Kingfisher.RetrieveImageTask

to:

public func retrieveImageWithResource(resource: Resource, optionsInfo: Kingfisher.KingfisherOptionsInfo?, progressBlock: Kingfisher.DownloadProgressBlock?, completionHandler: Kingfisher.CompletionHandler?) -> Kingfisher.RetrieveImageTask

Still, for this kind of cases, we can just leave as is. There is no point on passing a Resource.

public func storeImage(image: UIImage, forKey key: String)

Likewise, when the interaction is with the ImageDownloader it's fine to use a NSURL, since the ImageDownload should be responsible only for interactions with the network.

Without seeing all the code and assuming the code respects the SOLID principles, only the KingfisherManager would have to be updated.

@dmcrodrigues
Copy link

Yeah I think this is really important, for example URL can contain dynamic parameters which can break the cache. I think the key can be the URL's absolute string by default but we should be able to customise it somehow to handle our specific use-case.

The approach suggested by @RuiAAPeres seems great to handle this. 👍

@RuiAAPeres
Copy link
Author

I think the key can be the URL's absolute string by default

struct Resource {
    let key : String
    let url : NSURL

    init(key: String, url: NSURL) {
        self.key = key
        self.url = url
    }

    init(url: NSURL) {
        self.init(key: url.absoluteString, url: url)
    }
}

@onevcat
Copy link
Owner

onevcat commented Aug 14, 2015

Hi, @RuiAAPeres @dmcrodrigues

Great suggestion for decoupling the key and URL. It is very easy to adjust current code for it. I will handle it soon.

@onevcat onevcat closed this as completed Aug 24, 2015
nicoster pushed a commit to nicoster/Kingfisher that referenced this issue Nov 3, 2022
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