Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

Add support for NSURLCache. #141

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Add support for NSURLCache. #141

wants to merge 5 commits into from

Conversation

kgoodier
Copy link
Contributor

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.

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.
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.
@NSProgrammer
Copy link
Collaborator

Looks great! I have a number of comments that should be addressed though. Thanks, Kevin!

Simplify some things, handle edge cases, handle OSX.
@NSProgrammer
Copy link
Collaborator

+1

if (self != nil) {
}
return self;
}
Copy link

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?

Copy link
Contributor Author

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
@CLAassistant
Copy link

CLAassistant commented Jul 18, 2019

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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

Successfully merging this pull request may close these issues.

None yet

4 participants