Permalink
Browse files

Fixes to ensure failed requests are not resent. refs #628, fixes #744,

…closes #720

Additional changes and test coverage to handle the case of loading an invalid URL
or otherwise immediately encountering an error condition:

* Restores use of removeLoadingRequest: within the queue
* Updates fragile tests to ensure better coverage for error cases
* Sets isLoaded to YES during error callbacks to prevent duplicated dispatching
  • Loading branch information...
1 parent f8c35cc commit 938304d5422f6883f7017ad0e07423a0d0701357 @blakewatters blakewatters committed May 17, 2012
View
@@ -159,50 +159,21 @@ typedef void(^RKRequestDidFailLoadWithErrorBlock)(NSError *error);
Models the request portion of an HTTP request/response cycle.
*/
@interface RKRequest : NSObject {
- NSURL *_URL;
- NSMutableURLRequest *_URLRequest;
- NSURLConnection *_connection;
- NSDictionary *_additionalHTTPHeaders;
- NSObject<RKRequestSerializable> *_params;
- NSObject<RKRequestDelegate> *_delegate;
- NSObject<RKConfigurationDelegate> *_configurationDelegate;
- id _userData;
- RKRequestAuthenticationType _authenticationType;
- NSString *_username;
- NSString *_password;
- NSString *_OAuth1ConsumerKey;
- NSString *_OAuth1ConsumerSecret;
- NSString *_OAuth1AccessToken;
- NSString *_OAuth1AccessTokenSecret;
- NSString *_OAuth2AccessToken;
- NSString *_OAuth2RefreshToken;
- RKRequestMethod _method;
- BOOL _isLoading;
- BOOL _isLoaded;
- RKRequestCachePolicy _cachePolicy;
BOOL _sentSynchronously;
- RKRequestCache *_cache;
- NSTimeInterval _cacheTimeoutInterval;
- RKRequestQueue *_queue;
- RKReachabilityObserver *_reachabilityObserver;
+ NSURLConnection *_connection;
+ id<RKRequestDelegate> _delegate;
NSTimer *_timeoutTimer;
- NSStringEncoding _defaultHTTPEncoding;
-
- NSSet *_additionalRootCertificates;
- BOOL _disableCertificateValidation;
- BOOL _followRedirect;
- NSString *_runLoopMode;
-
- #if TARGET_OS_IPHONE
- RKRequestBackgroundPolicy _backgroundPolicy;
- UIBackgroundTaskIdentifier _backgroundTaskIdentifier;
- #endif
+ RKRequestCachePolicy _cachePolicy;
RKRequestDidLoadResponseBlock _onDidLoadResponse;
RKRequestDidFailLoadWithErrorBlock _onDidFailLoadWithError;
+
+#if TARGET_OS_IPHONE
+ RKRequestBackgroundPolicy _backgroundPolicy;
+ UIBackgroundTaskIdentifier _backgroundTaskIdentifier;
+#endif
}
-
///-----------------------------------------------------------------------------
/// @name Creating a Request
///-----------------------------------------------------------------------------
@@ -770,12 +741,12 @@ typedef void(^RKRequestDidFailLoadWithErrorBlock)(NSError *error);
/**
Returns YES when this request is in-progress
*/
-- (BOOL)isLoading;
+@property (nonatomic, assign, readonly, getter = isLoading) BOOL loading;
/**
Returns YES when this request has been completed
*/
-- (BOOL)isLoaded;
+@property (nonatomic, assign, readonly, getter = isLoaded) BOOL loaded;
/**
Returns YES when this request has not yet been sent
View
@@ -85,6 +85,12 @@ RKRequestMethod RKRequestMethodTypeFromName(NSString *methodName) {
#undef RKLogComponent
#define RKLogComponent lcl_cRestKitNetwork
+@interface RKRequest ()
+@property (nonatomic, assign, readwrite, getter = isLoaded) BOOL loaded;
+@property (nonatomic, assign, readwrite, getter = isLoading) BOOL loading;
+@property (nonatomic, assign, readwrite) BOOL canceled;
+@end
+
@implementation RKRequest
@class GCOAuth;
@@ -119,9 +125,13 @@ @implementation RKRequest
@synthesize cancelled = _cancelled;
@synthesize followRedirect = _followRedirect;
@synthesize runLoopMode = _runLoopMode;
+@synthesize loaded = _loaded;
+@synthesize loading = _loading;
+@synthesize canceled = _canceled;
#if TARGET_OS_IPHONE
-@synthesize backgroundPolicy = _backgroundPolicy, backgroundTaskIdentifier = _backgroundTaskIdentifier;
+@synthesize backgroundPolicy = _backgroundPolicy;
+@synthesize backgroundTaskIdentifier = _backgroundTaskIdentifier;
#endif
+ (RKRequest*)requestWithURL:(NSURL*)URL {
@@ -161,7 +171,7 @@ - (id)init {
}
- (void)reset {
- if (_isLoading) {
+ if (self.isLoading) {
RKLogWarning(@"Request was reset while loading: %@. Canceling.", self);
[self cancel];
}
@@ -170,9 +180,9 @@ - (void)reset {
[_URLRequest setCachePolicy:NSURLRequestReloadIgnoringCacheData];
[_connection release];
_connection = nil;
- _isLoading = NO;
- _isLoaded = NO;
- _cancelled = NO;
+ self.loading = NO;
+ self.loaded = NO;
+ self.canceled = NO;
}
- (void)cleanupBackgroundTask {
@@ -398,7 +408,7 @@ - (void)cancelAndInformDelegate:(BOOL)informDelegate {
[_connection release];
_connection = nil;
[self invalidateTimeoutTimer];
- _isLoading = NO;
+ self.loading = NO;
if (informDelegate && [_delegate respondsToSelector:@selector(requestDidCancelLoad:)]) {
[_delegate requestDidCancelLoad:self];
@@ -428,10 +438,9 @@ - (NSString*)HTTPMethod {
}
}
-// TODO: We may want to eliminate the coupling between the request queue and individual request instances.
-// We could factor the knowledge about the queue out of RKRequest entirely, but it will break behavior.
+// NOTE: We could factor the knowledge about the queue out of RKRequest entirely, but it will break behavior.
- (void)send {
- NSAssert(NO == _isLoading || NO == _isLoaded, @"Cannot send a request that is loading or loaded without resetting it first.");
+ NSAssert(NO == self.isLoading || NO == self.isLoaded, @"Cannot send a request that is loading or loaded without resetting it first.");
if (self.queue) {
[self.queue addRequest:self];
} else {
@@ -446,7 +455,7 @@ - (void)fireAsynchronousRequest {
return;
}
- _isLoading = YES;
+ self.loading = YES;
if ([self.delegate respondsToSelector:@selector(requestDidStartLoad:)]) {
[self.delegate requestDidStartLoad:self];
@@ -489,11 +498,11 @@ - (BOOL)shouldDispatchRequest {
}
- (void)sendAsynchronously {
- NSAssert(NO == _isLoading || NO == _isLoaded, @"Cannot send a request that is loading or loaded without resetting it first.");
+ NSAssert(NO == self.loading || NO == self.loaded, @"Cannot send a request that is loading or loaded without resetting it first.");
_sentSynchronously = NO;
if ([self shouldLoadFromCache]) {
RKResponse* response = [self loadResponseFromCache];
- _isLoading = YES;
+ self.loading = YES;
[self performSelector:@selector(didFinishLoad:) withObject:response afterDelay:0];
} else if ([self shouldDispatchRequest]) {
[self createTimeoutTimer];
@@ -539,26 +548,24 @@ - (void)sendAsynchronously {
if (_cachePolicy & RKRequestCachePolicyLoadIfOffline &&
[self.cache hasResponseForRequest:self]) {
-
- _isLoading = YES;
-
+ self.loading = YES;
[self didFinishLoad:[self loadResponseFromCache]];
-
} else {
+ self.loading = YES;
+
RKLogError(@"Failed to send request to %@ due to unreachable network. Reachability observer = %@", [[self URL] absoluteString], self.reachabilityObserver);
NSString* errorMessage = [NSString stringWithFormat:@"The client is unable to contact the resource at %@", [[self URL] absoluteString]];
NSDictionary *userInfo = [NSDictionary dictionaryWithObjectsAndKeys:
errorMessage, NSLocalizedDescriptionKey,
nil];
- NSError* error = [NSError errorWithDomain:RKErrorDomain code:RKRequestBaseURLOfflineError userInfo:userInfo];
- _isLoading = YES;
+ NSError* error = [NSError errorWithDomain:RKErrorDomain code:RKRequestBaseURLOfflineError userInfo:userInfo];
[self performSelector:@selector(didFailLoadWithError:) withObject:error afterDelay:0];
}
}
}
- (RKResponse*)sendSynchronously {
- NSAssert(NO == _isLoading || NO == _isLoaded, @"Cannot send a request that is loading or loaded without resetting it first.");
+ NSAssert(NO == self.loading || NO == self.loaded, @"Cannot send a request that is loading or loaded without resetting it first.");
NSHTTPURLResponse* URLResponse = nil;
NSError* error;
NSData* payload = nil;
@@ -567,7 +574,7 @@ - (RKResponse*)sendSynchronously {
if ([self shouldLoadFromCache]) {
response = [self loadResponseFromCache];
- _isLoading = YES;
+ self.loading = YES;
[self didFinishLoad:response];
} else if ([self shouldDispatchRequest]) {
RKLogDebug(@"Sending synchronous %@ request to URL %@.", [self HTTPMethod], [[self URL] absoluteString]);
@@ -579,7 +586,7 @@ - (RKResponse*)sendSynchronously {
[[NSNotificationCenter defaultCenter] postNotificationName:RKRequestSentNotification object:self userInfo:nil];
- _isLoading = YES;
+ self.loading = YES;
if ([self.delegate respondsToSelector:@selector(requestDidStartLoad:)]) {
[self.delegate requestDidStartLoad:self];
}
@@ -649,7 +656,8 @@ - (void)didFailLoadWithError:(NSError*)error {
[self didFinishLoad:[self loadResponseFromCache]];
} else {
- _isLoading = NO;
+ self.loaded = YES;
+ self.loading = NO;
if ([_delegate respondsToSelector:@selector(request:didFailLoadWithError:)]) {
[_delegate request:self didFailLoadWithError:error];
@@ -678,8 +686,8 @@ - (void)updateInternalCacheDate {
}
- (void)didFinishLoad:(RKResponse*)response {
- _isLoading = NO;
- _isLoaded = YES;
+ self.loading = NO;
+ self.loaded = YES;
RKLogInfo(@"Status Code: %ld", (long) [response statusCode]);
RKLogDebug(@"Body: %@", [response bodyAsString]);
@@ -739,16 +747,8 @@ - (BOOL)isHEAD {
return _method == RKRequestMethodHEAD;
}
-- (BOOL)isLoading {
- return _isLoading;
-}
-
-- (BOOL)isLoaded {
- return _isLoaded;
-}
-
- (BOOL)isUnsent {
- return _isLoading == NO && _isLoaded == NO;
+ return self.loading == NO && self.loaded == NO;
}
- (NSString*)resourcePath {
@@ -500,7 +500,7 @@ - (void)processRequestDidFailWithErrorNotification:(NSNotification *)notificatio
[_delegate requestQueue:self didFailRequest:request withError:error];
}
- [self removeRequest:request];
+ [self removeLoadingRequest:request];
[self loadNextInQueue];
}
@@ -37,19 +37,28 @@ - (void)updateInternalCacheDate;
- (void)postRequestDidFailWithErrorNotification:(NSError *)error;
@end
+@interface RKObjectLoader ()
+@property (nonatomic, assign, readwrite, getter = isLoaded) BOOL loaded;
+@property (nonatomic, assign, readwrite, getter = isLoading) BOOL loading;
+@end
+
@implementation RKObjectLoader
-@synthesize mappingProvider = _mappingProvider, response = _response;
-@synthesize targetObject = _targetObject, objectMapping = _objectMapping;
+@synthesize mappingProvider = _mappingProvider;
+@synthesize response = _response;
+@synthesize targetObject = _targetObject;
+@synthesize objectMapping = _objectMapping;
@synthesize result = _result;
@synthesize serializationMapping = _serializationMapping;
@synthesize serializationMIMEType = _serializationMIMEType;
@synthesize sourceObject = _sourceObject;
@synthesize mappingQueue = _mappingQueue;
-@synthesize onDidFailWithError;
-@synthesize onDidLoadObject;
-@synthesize onDidLoadObjects;
-@synthesize onDidLoadObjectsDictionary;
+@synthesize onDidFailWithError = _onDidFailWithError;
+@synthesize onDidLoadObject = _onDidLoadObject;
+@synthesize onDidLoadObjects = _onDidLoadObjects;
+@synthesize onDidLoadObjectsDictionary = _onDidLoadObjectsDictionary;
+@dynamic loaded;
+@dynamic loading;
+ (id)loaderWithURL:(RKURL *)URL mappingProvider:(RKObjectMappingProvider *)mappingProvider {
return [[[self alloc] initWithURL:URL mappingProvider:mappingProvider] autorelease];
@@ -82,14 +91,14 @@ - (void)dealloc {
_serializationMIMEType = nil;
[_serializationMapping release];
_serializationMapping = nil;
- [onDidFailWithError release];
- onDidFailWithError = nil;
- [onDidLoadObject release];
- onDidLoadObject = nil;
- [onDidLoadObjects release];
- onDidLoadObjects = nil;
- [onDidLoadObjectsDictionary release];
- onDidLoadObjectsDictionary = nil;
+ [_onDidFailWithError release];
+ _onDidFailWithError = nil;
+ [_onDidLoadObject release];
+ _onDidLoadObject = nil;
+ [_onDidLoadObjects release];
+ _onDidLoadObjects = nil;
+ [_onDidLoadObjectsDictionary release];
+ _onDidLoadObjectsDictionary = nil;
[super dealloc];
}
@@ -115,8 +124,8 @@ - (void)informDelegateOfError:(NSError *)error {
// NOTE: This method is significant because the notifications posted are used by
// RKRequestQueue to remove requests from the queue. All requests need to be finalized.
- (void)finalizeLoad:(BOOL)successful {
- _isLoading = NO;
- _isLoaded = successful;
+ self.loading = NO;
+ self.loaded = successful;
if ([self.delegate respondsToSelector:@selector(objectLoaderDidFinishLoading:)]) {
[(NSObject<RKObjectLoaderDelegate>*)self.delegate performSelectorOnMainThread:@selector(objectLoaderDidFinishLoading:)
@@ -258,15 +258,25 @@ - (void)testShouldRemoveItemsFromTheQueueWithAnUnmappableResponse {
- (void)testThatSendingRequestToInvalidURLDoesNotGetSentTwice {
RKRequestQueue *queue = [RKRequestQueue requestQueue];
- NSURL *URL = [NSURL URLWithString:@"http://bix.gg/RKRequestQueueExample"];
+ NSURL *URL = [NSURL URLWithString:@"http://localhost:7662/RKRequestQueueExample"];
RKRequest *request = [RKRequest requestWithURL:URL];
RKTestResponseLoader *responseLoader = [RKTestResponseLoader responseLoader];
id mockResponseLoader = [OCMockObject partialMockForObject:responseLoader];
[[[mockResponseLoader expect] andForwardToRealObject] request:request didFailLoadWithError:OCMOCK_ANY];
request.delegate = responseLoader;
+ id mockQueueDelegate = [OCMockObject niceMockForProtocol:@protocol(RKRequestQueueDelegate)];
+ __block NSUInteger invocationCount = 0;
+ [[mockQueueDelegate stub] requestQueue:queue willSendRequest:[OCMArg checkWithBlock:^BOOL(id request) {
+ invocationCount++;
+ return YES;
+ }]];
[queue addRequest:request];
+ queue.delegate = mockQueueDelegate;
[queue start];
[mockResponseLoader waitForResponse];
+ [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:1.0]];
+ [mockResponseLoader verify];
+ assertThatInteger(invocationCount, is(equalToInteger(1)));
}
@end
@@ -947,4 +947,34 @@ - (void)testInvocationOfDidReceiveResponse {
[loaderMock verify];
}
+- (void)testThatIsLoadingIsNoDuringDidFailWithErrorCallback {
+ NSURL *URL = [[NSURL alloc] initWithString:@"http://localhost:8765"];
+ RKTestResponseLoader* loader = [RKTestResponseLoader responseLoader];
+
+ RKClient *client = [RKClient clientWithBaseURL:URL];
+ RKRequest *request = [client requestWithResourcePath:@"/invalid"];
+ request.method = RKRequestMethodGET;
+ request.delegate = loader;
+ request.onDidFailLoadWithError = ^(NSError *error) {
+ assertThatBool([request isLoading], is(equalToBool(NO)));
+ };
+ [request sendAsynchronously];
+ [loader waitForResponse];
+}
+
+- (void)testThatIsLoadedIsYesDuringDidFailWithErrorCallback {
+ NSURL *URL = [[NSURL alloc] initWithString:@"http://localhost:8765"];
+ RKTestResponseLoader* loader = [RKTestResponseLoader responseLoader];
+
+ RKClient *client = [RKClient clientWithBaseURL:URL];
+ RKRequest *request = [client requestWithResourcePath:@"/invalid"];
+ request.method = RKRequestMethodGET;
+ request.delegate = loader;
+ request.onDidFailLoadWithError = ^(NSError *error) {
+ assertThatBool([request isLoaded], is(equalToBool(YES)));
+ };
+ [request sendAsynchronously];
+ [loader waitForResponse];
+}
+
@end

0 comments on commit 938304d

Please sign in to comment.