Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

AFURLCache - disk cache for iOS #25

Closed
wants to merge 15 commits into from
Closed

AFURLCache - disk cache for iOS #25

wants to merge 15 commits into from

Conversation

steipete
Copy link
Contributor

The current implementation of NSURLCache doesn't save to disk, even if requests are marked this way.
With AFURLCache, images and requests that are marked as cacheable are now really cached to disk, loaded almost instantaneous. Even etags are parsed. Check it out. I'd love to see this in main.

I took https://github.com/rs/SDURLCache and modified it to be crazy fast, using async gcd wherever possible. (also fixed some retain cycle and disabled the cache memory policy). Still, Olivier Poitrey did amazing work here, huge thanks.

From the Documentation: "Important: iOS ignores this cache policy, and instead treats it as NSURLCacheStorageAllowedInMemoryOnly." (http://developer.apple.com/library/iOS/#documentation/Cocoa/Reference/Foundation/Classes/NSCachedURLResponse_Class/Reference/Reference.html)

@mattt
Copy link
Contributor

mattt commented Sep 15, 2011

Finally got a chance to give this a proper once-over, and the code looks pretty solid. It's a great improvement to SDURLCache. Seeing as how it's completely stand-alone, without any explicit AFNetworking inter-dependencies, I wonder if it makes sense to make this a part of core, or keep it as a standalone library.

In the process of writing Gowalla 4, @sco added disk cacheing to image requests, since we weren't as concerned with / had other cacheing mechanisms (i.e. Core Data) for regular HTTP requests. We're still trying to reconcile this; whether it should be pushed upstream, and how it fits it with AFURLCache...

Anyway, sorry to keep you waiting on this. And again, fantastic job, @steipete :)

@steipete
Copy link
Contributor Author

You're correct, this would run standalone and works as well with basic NSURLRequest's. Still, I see AFNetworking as "the whole package" and a disk cache is something I expect from it. More people will use it if it gets in core, and in the end we all get better apps - most people probably not even know that NSURLCache doesn't do disk caching, especially because it has the initWithMemoryCapacity:diskCapacity:diskPath: method, which even I supposed to really enable disk caching.
I'm all for the "Rails way" of setting good defaults, people still can easily opt-out of it, as it has no dependencies.

As for explicit disk caching for images - that may be a bit faster, if you load them synchronously and because they're already stored in png format - but I still like the idea of a generic cache more, same feature set, just more flexible.

I'd love it to be merged with core, but you're the boss :)

@myell0w
Copy link
Contributor

myell0w commented Sep 21, 2011

I think this would be a valid addition. maybe gowalla could host it as a standalone repository since it's independent of AFNetworking and just add a note to the readme/include a sample on how to integrate.

@steipete: have you thought about bringing your improvements back to SDURLCache?

@NSElvis
Copy link

NSElvis commented Sep 21, 2011

I'd love to see this in main, too. I see a huge advantage on caching things because you don't have to download your images every time.

@steipete
Copy link
Contributor Author

@myell0w, If they don't integrate it, i'll rename it to PSURLCache and will host it myself, or try to merge the efforts with the original project. I just fear that people are lazy and won't use it as much, if it's a separate repository. I'm all for good defaults. About giving back - I notified Oliver but he hasn't responded yet.

@mattt
Copy link
Contributor

mattt commented Sep 21, 2011

Can't tell you enough how awesome this is, and I hate to keep you in limbo on all of this. It's just that we're completely split on whether this should be included, or if it's separate. Even after a week of mulling it over, there's no clear consensus.

We're at an interesting turning point with AFNetworking. The API is stabilizing, so decisions we make now will really determine what kind of library it is. My current thoughts are that of removing features, paring things down to get to the core functionality. But at the same time, the Rails way of strong defaults is also appealing. Gah!

If it's any consolation, CocoaPods looks like it will take a lot of the pain away from dependency management, so we could move away from kitchen sink frameworks out of convenience.

@sbandol
Copy link

sbandol commented Sep 22, 2011

We all know the pros and cons, but since it so easy to use it or not to (weak dependency) , I think it shall be merged.

AFURLCache really should be be autoreleased, even if it's only removed when the process quits.
@jogu
Copy link

jogu commented Sep 23, 2011

This cache sounds excellent - I'd love to see this merged, or at the worst, having an official list of "known good things that work well with AFNetworking".

I suspect there's going to be a lot of ASIHTTPRequest (which has caching built in) users switching over to AFNetworking over the next few months, and having a clear message "if you want a cache, this is probably the best one, it's being used by a fair number of people so should be bug free, and it's likely to be maintained going forwards". Those kind of recommendations are priceless when you're learning something new.

(I'm also all for making it clear that this is a separate module that you don't have to use and keeping it obviously separate from the core of AFNetworking.)

@mattt
Copy link
Contributor

mattt commented Sep 23, 2011

@steipete I think I finally came up with a good solution for how everything should fit together. Tell me what you think of this:

AFURLCache, like NSURLCache, functions completely on its own, without any coupling or dependencies, and with minimal setup:

AFURLCache *URLCache = [[[NSURLCache alloc] initWithMemoryCapacity:1024 * 1024 diskCapacity:1024 * 1024 * 5 diskPath:nil] autorelease];
[NSURLCache setSharedURLCache:URLCache];

AFNetworking doesn't interact with URLCache at all, except indirectly with the one NSURLConnection delegate method connect:willCacheResponse:. As such, any NSURLCache can be swapped out for another easily.

From a the perspective of library maintenance, it does not make logistical sense to have them in the same project.

HOWEVER, although the components are fully decoupled, they do pair extremely well together. It's a lot like those jars of peanut butter and jelly already mixed together. PB&J are great together, but should be stored separately.

Not to extend the metaphor too far, but the example app is like a nice peanut butter and jelly sandwich made for people to try out, to see how they like it. That's why I'd like to include AFURLCache (or rather PSURLCache) in the example project. NSURLCache in general is a thing of beauty, and something that a surprising few number of developers are privy to, so it'd be great to have that as an example.

I'm constantly impressed and humbled by your open-source contributions, @steipete. Thank you :)

If this all seems amicable, I'd love to get that out as soon as possible (what, with the wave of ASI converts coming through)

}
}

if (status == 302 || status == 307) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is already tested at the very beginning of the method.

@cte
Copy link

cte commented Oct 6, 2011

Just wanted to say that I've been using AFURLCache for a few days now in the development of an image intensive iPad app that hugely benefits from disk caching, and I love it. Thanks for the excellent work!

@grgcombs
Copy link

grgcombs commented Oct 6, 2011

Yep, I'm playing around with it too and it's dead easy...

@mattt
Copy link
Contributor

mattt commented Oct 6, 2011

It's a fantastic piece of code, and I'm looking forward to incorporating it into the example project.

@steipete: Any updates on this? As far as I can tell, it'd be as simple as Find-And-Replacing AF with PS and creating a new repository, right?

@steipete
Copy link
Contributor Author

steipete commented Oct 6, 2011

Pretty much so. And then, do we add it as a submodule into the example dir?

(Sorry for the delay. Working hard to get my university degree to finally move to San Francisco.
Sucks out all of my time, can't fail here.)

@ALL - thanks for the great feedback!

@mattt
Copy link
Contributor

mattt commented Oct 6, 2011

Submodules would be great, but only if they don't end up being the pain in the @$$ that I've come to know them for.

Really awesome to hear that you'll be heading to SF! All the best as you get everything ready for that!

@steipete
Copy link
Contributor Author

steipete commented Oct 6, 2011

Yeah, submodules can be a PITA sometimes. But if we copy the files, that's just like hosting it in this repo anyways.

Just now when people clone and try out the example, they'll get an error unless they make the submodule dance.
Guess that's also something we don't want. Or we don't use it in default, that would make it less visible.

@grgcombs
Copy link

grgcombs commented Oct 7, 2011

Yep, I like the fake submodule approach for small repositories. You still get the ability to keep them up to date as needed, but without the headaches of a traditional submodule.

@steipete
Copy link
Contributor Author

steipete commented Oct 7, 2011

Ahh, that's pretty neat, exactly the solution we're searching here.
So with fake submodules, the actual .git directory is not committed? Is it excluded per default?

@grgcombs
Copy link

grgcombs commented Oct 7, 2011

Yes, that's right. It's almost magical how easy it is ... I'm sure there's some drawbacks to it, but I haven't run into one yet. I certainly feel like I have quite a bit more insurance against detached heads and lost changes as a result...

@mattt
Copy link
Contributor

mattt commented Oct 12, 2011

Alright, I'm going to close this issue, since I think we figured out a resolution to the original point. Feel free, as always, to continue any further discussion here.

@steipete I'm planning on releasing 0.7 tomorrow, but I'm looking forward to including PSURLCache in the 0.7.x release soon.

@mattt mattt closed this Oct 12, 2011
@grgcombs
Copy link

Whatever became of this? Is the end-developer's solution just to pull in Pete's url cache repository? Is there a blessed pathway?

@mattt
Copy link
Contributor

mattt commented Dec 29, 2011

AFNetworking does and will not include a URL cache, but should you want that functionality, I strongly endorse @steipete's SDURLCache fork, or whatever it becomes if he extracts that into a separate project.

@grgcombs
Copy link

Fair enough. I've done just that and it's playing quite nicely with the master branch here. I don't know if there's anything additional I should be doing on the AF side to take full advantage, but so far so good.

@mattt
Copy link
Contributor

mattt commented Dec 29, 2011

Good to hear. Yeah, if you've set up a shared URL cache, AFNetworking is already taking full advantage of it.

@gerardp
Copy link

gerardp commented Feb 4, 2012

Anybody can provide a demo using SDURLcache with AFNetworking?

@grgcombs
Copy link

grgcombs commented Feb 4, 2012

It's one line of code at the beginning of your startup. The readme for the URL cache shows the setup. That's all it takes.

Sent from my iPhone.

On Feb 4, 2012, at 10:48 AM, gerardpreply@reply.github.com wrote:

Anybody can provide a demo using SDURLcache with AFNetworking?


Reply to this email directly or view it on GitHub:
#25 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
10 participants