Workaround for caching with iOS 8 and NSURLSession.

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.
twitter-archive · Dec 11, 2015 · ff61f58 · ff61f58
1 parent 11d02ec
commit ff61f58
Show file tree

Hide file tree

Showing 4 changed files with 239 additions and 14 deletions.
diff --git a/SPDY/SPDYStream.m b/SPDY/SPDYStream.m
@@ -72,6 +72,10 @@ @implementation SPDYStream
 
     NSURLRequest *_pushRequest;  // stored because we need a strong reference, _request is weak.
     NSHTTPURLResponse *_response;
+
+    NSURLCacheStoragePolicy _cachePolicy;
+    NSMutableData *_cacheDataBuffer; // only used for manual caching.
+    NSInteger _cacheMaxItemSize;
 }
 
 - (instancetype)initWithProtocol:(SPDYProtocol *)protocol
@@ -332,6 +336,9 @@ - (void)_close
     [self markUnblocked];  // just in case. safe if already unblocked.
     _metadata.blockedMs = _blockedElapsed * 1000;
 
+    // Manually make the willCacheResponse callback when needed
+    [self _storeCacheResponse];
+
     [_client URLProtocolDidFinishLoading:_protocol];
 
     if (_delegate && [_delegate respondsToSelector:@selector(streamClosed:)]) {
@@ -585,11 +592,24 @@ - (void)didReceiveResponse
         return;
     }
 
-    if (_client) {
-        NSURLCacheStoragePolicy cachePolicy = SPDYCacheStoragePolicy(_request, _response);
-        [_client URLProtocol:_protocol
-          didReceiveResponse:_response
-          cacheStoragePolicy:cachePolicy];
+    _cachePolicy = SPDYCacheStoragePolicy(_request, _response);
+    [_client URLProtocol:_protocol
+      didReceiveResponse:_response
+      cacheStoragePolicy:_cachePolicy];
+
+    // Prepare for manual caching, if needed
+    NSURLCache *cache = _protocol.associatedSession.configuration.URLCache;
+    if (_cachePolicy != NSURLCacheStorageNotAllowed &&  // cacheable?
+        [self _shouldUseManualCaching] &&                // hack needed and NSURLSession used?
+        cache != nil &&                                 // cache configured (NSURLSession-specific)?
+        _local) {                                       // not a push request?
+
+        // The NSURLCache has a heuristic to limit the max size of items based on the capacity of the
+        // cache. This is our attempt to mimic that behavior and prevent unlimited buffering of large
+        // responses. These numbers were found by manually experimenting and are only approximate.
+        // See SPDYNSURLCachingTest testResponseNearItemCacheSize_DoesUseCache.
+        _cacheDataBuffer = [[NSMutableData alloc] init];
+        _cacheMaxItemSize = MAX(cache.memoryCapacity * 0.05, cache.diskCapacity * 0.01);
     }
 }
 
@@ -689,7 +709,7 @@ - (void)didLoadData:(NSData *)data
             NSUInteger inflatedLength = DECOMPRESSED_CHUNK_LENGTH - _zlibStream.avail_out;
             inflatedData.length = inflatedLength;
             if (inflatedLength > 0) {
-                [_client URLProtocol:_protocol didLoadData:inflatedData];
+                [self _didLoadDataChunk:inflatedData];
             }
 
             // This can happen if the decompressed data is size N * DECOMPRESSED_CHUNK_LENGTH,
@@ -711,7 +731,69 @@ - (void)didLoadData:(NSData *)data
         }
     } else {
         NSData *dataCopy = [[NSData alloc] initWithBytes:data.bytes length:dataLength];
-        [_client URLProtocol:_protocol didLoadData:dataCopy];
+        [self _didLoadDataChunk:dataCopy];
+    }
+}
+
+- (void)_didLoadDataChunk:(NSData *)data
+{
+    [_client URLProtocol:_protocol didLoadData:data];
+
+    if (_cacheDataBuffer) {
+        NSUInteger bufferSize = _cacheDataBuffer.length + data.length;
+        if (bufferSize < _cacheMaxItemSize) {
+            [_cacheDataBuffer appendData:data];
+        } else {
+            // Throw away anything already buffered, it's going to be too big
+            _cacheDataBuffer = nil;
+        }
+    }
+}
+
+// NSURLSession on iOS 8 and iOS 7 does not cache items. Seems to be a bug with its interation
+// with NSURLProtocol. This flag represents whether our workaround, to insert into the cache
+// ourselves, is turned on.
+- (bool)_shouldUseManualCaching
+{
+    NSInteger osVersion = 0;
+    NSProcessInfo *processInfo = [NSProcessInfo processInfo];
+    if ([processInfo respondsToSelector:@selector(operatingSystemVersion)]) {
+        osVersion = [processInfo operatingSystemVersion].majorVersion;
+    }
+
+    // iOS 8 and earlier and using NSURLSession
+    return (osVersion <= 8 && _protocol.associatedSession != nil && _protocol.associatedSessionTask != nil);
+}
+
+- (void)_storeCacheResponse
+{
+    if (_cacheDataBuffer == nil) {
+        return;
+    }
+
+    NSCachedURLResponse *cachedResponse;
+    cachedResponse = [[NSCachedURLResponse alloc] initWithResponse:_response
+                                                              data:_cacheDataBuffer
+                                                          userInfo:nil
+                                                     storagePolicy:_cachePolicy];
+    NSURLCache *cache = _protocol.associatedSession.configuration.URLCache;
+    NSURLSessionDataTask *dataTask = (NSURLSessionDataTask *)_protocol.associatedSessionTask;
+
+    // Make "official" willCacheResponse callback to app, bypassing the NSURL loading system.
+    id<NSURLSessionDataDelegate> delegate = (id)_protocol.associatedSession.delegate;
+    if ([delegate respondsToSelector:@selector(URLSession:dataTask:willCacheResponse:completionHandler:)]) {
+        NSOperationQueue *queue = _protocol.associatedSession.delegateQueue;
+        [(queue) ?: [NSOperationQueue mainQueue] addOperationWithBlock:^{
+            [delegate URLSession:_protocol.associatedSession dataTask:dataTask willCacheResponse:cachedResponse completionHandler:^(NSCachedURLResponse * __nullable cachedResponse) {
+                // This block may execute asynchronously at any time. No need to come back to the SPDY/NSURL thread
+                if (cachedResponse) {
+                    [cache storeCachedResponse:cachedResponse forDataTask:dataTask];
+                }
+            }];
+        }];
+    } else {
+        // willCacheResponse delegate not implemented. Default behavior is to cache.
+        [cache storeCachedResponse:cachedResponse forDataTask:dataTask];
     }
 }
 

diff --git a/SPDYUnitTests/SPDYIntegrationTestHelper.h b/SPDYUnitTests/SPDYIntegrationTestHelper.h
@@ -29,11 +29,13 @@
 - (BOOL)didLoadFromNetwork;
 - (BOOL)didLoadFromCache;
 - (BOOL)didGetResponse;
+- (BOOL)didLoadData;
 - (BOOL)didGetError;
 - (BOOL)didCacheResponse;
 
 - (void)reset;
 - (void)loadRequest:(NSURLRequest *)request;
+- (void)provideResponseWithStatus:(NSUInteger)status cacheControl:(NSString *)cacheControl date:(NSDate *)date dataChunks:(NSArray *)dataChunks;
 - (void)provideResponseWithStatus:(NSUInteger)status cacheControl:(NSString *)cacheControl date:(NSDate *)date;
 - (void)provideBasicUncacheableResponse;
 - (void)provideBasicCacheableResponse;

diff --git a/SPDYUnitTests/SPDYIntegrationTestHelper.m b/SPDYUnitTests/SPDYIntegrationTestHelper.m
@@ -43,19 +43,24 @@ - (instancetype)init
 
 - (BOOL)didLoadFromNetwork
 {
-    return self.didGetResponse && _stream != nil;
+    return self.didGetResponse && self.didLoadData && _stream != nil;
 }
 
 - (BOOL)didLoadFromCache
 {
-    return self.didGetResponse && _stream == nil;
+    return self.didGetResponse && self.didLoadData && _stream == nil;
 }
 
 - (BOOL)didGetResponse
 {
     return _response != nil;
 }
 
+- (BOOL)didLoadData
+{
+    return _data.length > 0;
+}
+
 - (BOOL)didGetError
 {
     return _connectionError != nil;
@@ -100,12 +105,10 @@ - (NSString *)dateHeaderValue:(NSDate *)date
     return string;
 }
 
-- (void)provideResponseWithStatus:(NSUInteger)status cacheControl:(NSString *)cacheControl date:(NSDate *)date
+- (void)provideResponseWithStatus:(NSUInteger)status cacheControl:(NSString *)cacheControl date:(NSDate *)date dataChunks:(NSArray *)dataChunks
 {
     [SPDYMockSessionManager shared].streamQueuedBlock = ^(SPDYStream *stream) {
         _stream = stream;
-        uint8_t dataBytes[] = {1};
-        NSData *data = [NSData dataWithBytes:dataBytes length:1];
         NSMutableDictionary *headers = [NSMutableDictionary dictionaryWithDictionary:@{
                 @":status": [@(status) stringValue],
                 @":version": @"1.1",
@@ -119,10 +122,19 @@ - (void)provideResponseWithStatus:(NSUInteger)status cacheControl:(NSString *)ca
 
         [stream mergeHeaders:headers];
         [stream didReceiveResponse];
-        [stream didLoadData:data];
+        for (NSData *data in dataChunks) {
+            [stream didLoadData:data];
+        }
     };
 }
 
+- (void)provideResponseWithStatus:(NSUInteger)status cacheControl:(NSString *)cacheControl date:(NSDate *)date
+{
+    uint8_t dataBytes[] = {1};
+    NSArray *dataChunks = @[ [NSData dataWithBytes:dataBytes length:1] ];
+    [self provideResponseWithStatus:status cacheControl:cacheControl date:date dataChunks:dataChunks];
+}
+
 - (void)provideBasicUncacheableResponse
 {
     [self provideResponseWithStatus:200 cacheControl:@"no-store, no-cache, must-revalidate" date:[NSDate date]];
@@ -157,6 +169,7 @@ - (NSString *)description
     NSDictionary *params = @{
                              @"didLoadFromNetwork": @([self didLoadFromNetwork]),
                              @"didGetResponse": @([self didGetResponse]),
+                             @"didLoadData": @([self didLoadData]),
                              @"didGetError": @([self didGetError]),
                              @"didCacheResponse": @([self didCacheResponse]),
                              @"stream": _stream ?: @"<nil>",
@@ -237,6 +250,7 @@ - (instancetype)init
     }
     return self;
 }
+
 - (void)loadRequest:(NSMutableURLRequest *)request
 {
     [super loadRequest:request];

diff --git a/SPDYUnitTests/SPDYNSURLCachingTest.m b/SPDYUnitTests/SPDYNSURLCachingTest.m
@@ -27,6 +27,8 @@ + (void)setUp
     [SPDYIntegrationTestHelper setUp];
 
     [SPDYURLConnectionProtocol registerOrigin:@"http://example.com"];
+
+    [NSURLCache setSharedURLCache:[[NSURLCache alloc] initWithMemoryCapacity:1000000 diskCapacity:10000000 diskPath:nil]];
 }
 
 + (void)tearDown
@@ -105,7 +107,7 @@ - (void)testCacheableResponse_DoesInsertAndLoadFromCache
     }
 }
 
-- (void)testCachedItem_DoesHaveMetadata
+- (void)testCachedItem_DoesHaveFreshMetadata
 {
     for (NSArray *testParams in [self parameterizedTestInputs]) {
         GET_TEST_PARAMS;
@@ -124,6 +126,7 @@ - (void)testCachedItem_DoesHaveMetadata
         SPDYMetadata *metadata = [SPDYProtocol metadataForResponse:testHelper.response];
         XCTAssertNotNil(metadata, @"%@", testHelper);
         XCTAssertEqual(metadata.loadSource, SPDYLoadSourceNetwork, @"%@", testHelper);
+        XCTAssertEqual(metadata.streamId, 1, @"%@", testHelper);
 
         // Now make request again. Should pull from cache.
         [testHelper reset];
@@ -136,6 +139,7 @@ - (void)testCachedItem_DoesHaveMetadata
         metadata = [SPDYProtocol metadataForResponse:testHelper.response];
         XCTAssertNotNil(metadata, @"%@", testHelper);
         XCTAssertEqual(metadata.loadSource,  shouldPullFromCache ? SPDYLoadSourceCache : SPDYLoadSourceNetwork, @"%@", testHelper);
+        XCTAssertEqual(metadata.streamId, shouldPullFromCache ? 0 : 1, @"%@", testHelper);
 
         // Special logic for metadata provided by SPDYProtocolContext
         if ([testHelper isMemberOfClass:[SPDYURLSessionIntegrationTestHelper class]]) {
@@ -144,6 +148,7 @@ - (void)testCachedItem_DoesHaveMetadata
 
             XCTAssertNotNil(metadata2, @"%@", testHelper);
             XCTAssertEqual(metadata2.loadSource,  shouldPullFromCache ? SPDYLoadSourceCache : SPDYLoadSourceNetwork, @"%@", testHelper);
+            XCTAssertEqual(metadata2.streamId, 0, @"%@", testHelper);
         }
     }
 }
@@ -476,4 +481,126 @@ - (void)testPOSTRequest_DoesNotUseCache
     }
 }
 
+- (void)testResponseNearItemCacheSize_DoesUseCache
+{
+    for (NSArray *testParams in [self parameterizedTestInputs]) {
+        GET_TEST_PARAMS;
+        NSLog(@"- using %@, policy %tu", [testHelper class], cachePolicy);
+
+        [self resetSharedCache];
+
+        NSMutableURLRequest *request = [[NSMutableURLRequest alloc] initWithURL:[NSURL URLWithString:@"http://example.com/test/path"]];
+        request.cachePolicy = cachePolicy;
+
+        // Note: per observational manual testing, NSURL system has a much lower limit for item size.
+        // Seems to be less than 1% of max capacity.
+        // Base memory capacity 1000000, base disk capacity 10000000:
+        // - Response size 50000 is ok.
+        // - Response size 50001 is not ok.
+        // - If memory capacity goes from 1000000 to 999999, 50000 is no longer ok.
+        // - If disk capacity goes from 10000000 to 6000000, 50000 is still ok.
+        // - If disk capacity goes from 10000000 to 5000000, 50000 is no longer ok.
+        // So we're walking the line here with little understanding of Apple's heuristic. I thought
+        // 10% was the limit, but apparently its somewhere less than 1% depending on memory/disk
+        // setttings.
+        // This test may need to change if this starts failing.
+        NSArray *dataChunks = @[
+                                [NSMutableData dataWithLength:30000],
+                                [NSMutableData dataWithLength:20000],
+                                ];
+
+        [testHelper provideResponseWithStatus:200 cacheControl:@"public, max-age=1200" date:[NSDate date] dataChunks:dataChunks];
+        [testHelper loadRequest:request];
+
+        XCTAssertTrue(testHelper.didLoadFromNetwork, @"%@", testHelper);
+        XCTAssertTrue(testHelper.didCacheResponse, @"%@", testHelper);
+
+        // Now make request again. Should pull from cache.
+        [testHelper reset];
+        [testHelper loadRequest:request];
+
+        XCTAssertEqual(testHelper.didLoadFromCache, shouldPullFromCache, @"%@", testHelper);
+    }
+}
+
+- (void)testResponseExceedsItemCacheSize_DoesNotUseCache
+{
+    for (NSArray *testParams in [self parameterizedTestInputs]) {
+        GET_TEST_PARAMS;
+        NSLog(@"- using %@, policy %tu", [testHelper class], cachePolicy);
+
+        [self resetSharedCache];
+
+        NSMutableURLRequest *request = [[NSMutableURLRequest alloc] initWithURL:[NSURL URLWithString:@"http://example.com/test/path"]];
+        request.cachePolicy = cachePolicy;
+
+        // 10% of cache. Limit is 10%. Disk capacity is set to 10M.
+        NSArray *dataChunks = @[
+                                [NSMutableData dataWithLength:500000],
+                                [NSMutableData dataWithLength:500000],
+                                ];
+
+        [testHelper provideResponseWithStatus:200 cacheControl:@"public, max-age=1200" date:[NSDate date] dataChunks:dataChunks];
+        [testHelper loadRequest:request];
+
+        XCTAssertTrue(testHelper.didLoadFromNetwork, @"%@", testHelper);
+        XCTAssertFalse(testHelper.didCacheResponse, @"%@", testHelper);
+    }
+}
+
+- (void)testCacheDoesStoreAndLoadCorrectData
+{
+    for (NSArray *testParams in [self parameterizedTestInputs]) {
+        GET_TEST_PARAMS;
+        NSLog(@"- using %@, policy %tu", [testHelper class], cachePolicy);
+
+        [self resetSharedCache];
+
+        NSMutableURLRequest *request = [[NSMutableURLRequest alloc] initWithURL:[NSURL URLWithString:@"http://example.com/test/path"]];
+        request.cachePolicy = cachePolicy;
+
+        uint8_t buffer1[50] = {0};
+        uint8_t buffer2[3] = {0};
+        uint8_t buffer3[50] = {0};
+        const NSUInteger expectedLength = sizeof(buffer1) + sizeof(buffer2) + sizeof(buffer3);
+        uint8_t finalBuffer[expectedLength];
+
+        int i = 0;
+        for (int j = 0; j < sizeof(buffer1); j++) {
+            finalBuffer[i] = i % 256;
+            buffer1[j] = i++ % 256;
+        }
+        for (int j = 0; j < sizeof(buffer2); j++) {
+            finalBuffer[i] = i % 256;
+            buffer2[j] = i++ % 256;
+        }
+        for (int j = 0; j < sizeof(buffer3); j++) {
+            finalBuffer[i] = i % 256;
+            buffer3[j] = i++ % 256;
+        }
+        NSData *finalData = [NSData dataWithBytes:finalBuffer length:expectedLength];
+
+        NSArray *dataChunks = @[
+                                [NSMutableData dataWithBytes:buffer1 length:sizeof(buffer1)],
+                                [NSMutableData dataWithBytes:buffer2 length:sizeof(buffer2)],
+                                [NSMutableData dataWithBytes:buffer3 length:sizeof(buffer3)],
+                                ];
+
+        [testHelper provideResponseWithStatus:200 cacheControl:@"public, max-age=1200" date:[NSDate date] dataChunks:dataChunks];
+        [testHelper loadRequest:request];
+
+        XCTAssertTrue(testHelper.didLoadFromNetwork, @"%@", testHelper);
+        XCTAssertTrue(testHelper.didCacheResponse, @"%@", testHelper);
+
+        // Now make request again. Should pull from cache.
+        [testHelper reset];
+        [testHelper loadRequest:request];
+
+        XCTAssertEqual(testHelper.didLoadFromCache, shouldPullFromCache, @"%@", testHelper);
+        XCTAssertEqual(testHelper.data.length, expectedLength, @"%@", testHelper);
+
+        XCTAssertEqualObjects(testHelper.data, finalData, @"%@", testHelper);
+    }
+}
+
 @end