This repository has been archived by the owner on Nov 1, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 232
Add support for NSURLCache. #141
Open
kgoodier
wants to merge
5
commits into
develop
Choose a base branch
from
feature/nsurlcache
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
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
The NSURL loading system has subtle and poorly documented behavior regarding caching. CocoaSPDY was not doing the right thing, thus there was no support for NSURLCache in the protocol. This patch adds basic support for both NSURLConnection and NSURLSession based requests. This is not yet a fully-featured client caching implementation. If the request specifies a NSURLRequest cachePolicy of: NSURLRequestUseProtocolCachePolicy - NSURL system does not provide a cached response to the protocol constructor. It is up to the protocol to load and validate the cached response. NSURLRequestReturnCacheDataElseLoad - NSURL system will provide the cached response, if available. The protocol is expected to validate the response, and load if not available or not valid. NSURLRequestReturnCacheDataDontLoad - NSURL system will provide the cached response, if available. The protocol is expected to validate the response. The protocol will not be loaded if no cached response is available, or if one is but is invalid, the protocol should not load the request. In the cases where the protocol has a cached response and it is valid, it is supposed to call URLProtocol:cachedResponseIsValid. CocoaSPDY was not doing this. Determining validity of the cached response is the job of the protocol, and a basic implementation has been provided here. CocoaSPDY also has to jump through some hoops whenever NSURLSession is being used, as Apple has not provided a way to get the NSURLSessionConfiguration and thus we cannot get the right NSURLCache. Fortunately we have already provided a workaround for this. SPDYMetadata has been extended to provide the source of the response.
Observationally, we've seen the NSURL loading system fail to insert items into the NSURLCache when using NSURLSession and iOS 7/8. NSURLConnection works fine on all these, and NSURLSession works fine on iOS 9. Best guess is there's a bug in the NSURLProtocol implementation that fails to insert into the cache. So, here we are creating a workaround for that specific case. CocoaSPDY will buffer the data internally, up to a certain size limit based on the cache size, and will manually insert into the cache when the response is done. This could potentially be expanded in the future for unclaimed, finished push responses, but for now those are excluded. This change also adds a few additional caching-related unit tests.
kgoodier
force-pushed
the
feature/nsurlcache
branch
from
December 11, 2015 20:09
ff61f58
to
2a2c2f8
Compare
On iOS 7, the protocol is required to *always* load its own cache item, even for the NSURLRequestReturnCacheDataDontLoad and NSURLRequestReturnCacheDataElseLoad policies. This applies to both NSURLConnection and NSURLSession. Found by unit testing on iOS 7 simulator with XCode 6.4.
Looks great! I have a number of comments that should be addressed though. Thanks, Kevin! |
Simplify some things, handle edge cases, handle OSX.
kgoodier
force-pushed
the
feature/nsurlcache
branch
from
December 12, 2015 00:41
5e6d02c
to
a758509
Compare
+1 |
if (self != nil) { | ||
} | ||
return self; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this init override?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. :) Leftover.
Feedback from code review: - parse quoted strings - use strptime_l - zero tm struct for strptime_l - allow requests with Authorization to be cached
kgoodier
force-pushed
the
feature/nsurlcache
branch
from
December 17, 2015 22:32
6224087
to
c3d7349
Compare
|
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
The NSURL loading system has subtle and poorly documented behavior
regarding caching. CocoaSPDY was not doing the right thing, thus
there was no support for NSURLCache in the protocol. This patch
adds basic support for both NSURLConnection and NSURLSession based
requests. This is not yet a fully-featured client caching
implementation.
If the request specifies a NSURLRequest cachePolicy of:
NSURLRequestUseProtocolCachePolicy
constructor. It is up to the protocol to load and validate the
cached response.
NSURLRequestReturnCacheDataElseLoad
protocol is expected to validate the response, and load if not
available or not valid.
NSURLRequestReturnCacheDataDontLoad
protocol is expected to validate the response. The protocol will not
be loaded if no cached response is available, or if one is but is
invalid, the protocol should not load the request.
In the cases where the protocol has a cached response and it is valid,
it is supposed to call URLProtocol:cachedResponseIsValid. CocoaSPDY
was not doing this. Determining validity of the cached response is the
job of the protocol, and a basic implementation has been provided here.
CocoaSPDY also has to jump through some hoops whenever NSURLSession is
being used, as Apple has not provided a way to get the
NSURLSessionConfiguration and thus we cannot get the right NSURLCache.
Fortunately we have already provided a workaround for this.
SPDYMetadata has been extended to provide the source of the response.
Observationally, we've seen the NSURL loading system fail to insert
items into the NSURLCache when using NSURLSession and iOS 7/8.
NSURLConnection works fine on all these, and NSURLSession works fine on
iOS 9. Best guess is there's a bug in the NSURLProtocol implementation
that fails to insert into the cache.
So, here we are creating a workaround for that specific case. CocoaSPDY
will buffer the data internally, up to a certain size limit based on the
cache size, and will manually insert into the cache when the response is
done. This could potentially be expanded in the future for unclaimed,
finished push responses, but for now those are excluded.