From b02f0c7c5dd44069bacf3971dfc8c42c2103f550 Mon Sep 17 00:00:00 2001 From: Aron Cedercrantz Date: Mon, 22 Feb 2016 14:16:16 +0100 Subject: [PATCH 1/2] Add nullable to return type for methods that can return nil --- SPTDataLoader/SPTDataLoader.m | 2 +- .../SPTDataLoaderRequestTaskHandler.h | 3 ++- .../SPTDataLoaderRequestTaskHandler.m | 2 +- include/SPTDataLoader/SPTDataLoader.h | 21 ++++++++++++++----- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/SPTDataLoader/SPTDataLoader.m b/SPTDataLoader/SPTDataLoader.m index e77ca76a..28e10c45 100644 --- a/SPTDataLoader/SPTDataLoader.m +++ b/SPTDataLoader/SPTDataLoader.m @@ -70,7 +70,7 @@ - (void)executeDelegateBlock:(dispatch_block_t)block #pragma mark SPTDataLoader -- (id)performRequest:(SPTDataLoaderRequest *)request +- (nullable id)performRequest:(SPTDataLoaderRequest *)request { SPTDataLoaderRequest *copiedRequest = [request copy]; id delegate = self.delegate; diff --git a/SPTDataLoader/SPTDataLoaderRequestTaskHandler.h b/SPTDataLoader/SPTDataLoaderRequestTaskHandler.h index ce0bb634..ec89183d 100644 --- a/SPTDataLoader/SPTDataLoaderRequestTaskHandler.h +++ b/SPTDataLoader/SPTDataLoaderRequestTaskHandler.h @@ -66,8 +66,9 @@ NS_ASSUME_NONNULL_BEGIN /** * Tell the operation the URL session has completed the request * @param error An optional error to use if the request was not completed successfully + * @return The response object unless the request was cancelled or will be re-tried in which case `nil` is returned. */ -- (SPTDataLoaderResponse *)completeWithError:(nullable NSError *)error; +- (nullable SPTDataLoaderResponse *)completeWithError:(nullable NSError *)error; /** * Gets called whenever the original request was redirected. * Returns YES to allow redirect, NO to block it. diff --git a/SPTDataLoader/SPTDataLoaderRequestTaskHandler.m b/SPTDataLoader/SPTDataLoaderRequestTaskHandler.m index 77888d1a..778b0abe 100644 --- a/SPTDataLoader/SPTDataLoaderRequestTaskHandler.m +++ b/SPTDataLoader/SPTDataLoaderRequestTaskHandler.m @@ -112,7 +112,7 @@ - (void)receiveData:(NSData *)data }]; } -- (SPTDataLoaderResponse *)completeWithError:(nullable NSError *)error +- (nullable SPTDataLoaderResponse *)completeWithError:(nullable NSError *)error { id requestResponseHandler = self.requestResponseHandler; if (!self.response) { diff --git a/include/SPTDataLoader/SPTDataLoader.h b/include/SPTDataLoader/SPTDataLoader.h index 046c5d20..f3c9e35d 100644 --- a/include/SPTDataLoader/SPTDataLoader.h +++ b/include/SPTDataLoader/SPTDataLoader.h @@ -43,21 +43,32 @@ NS_ASSUME_NONNULL_BEGIN */ @interface SPTDataLoader : NSObject +#pragma mark Delegating Tasks + /** - * The object listening to the data loader + * The object listening to the data loader. */ @property (nonatomic, weak, nullable) id delegate; /** - * The queue to call the delegate selectors on - * @discussion By default this is the main queue + * The queue to call the delegate selectors on. + * @discussion By default this is the main queue. */ @property (nonatomic, strong) dispatch_queue_t delegateQueue; +#pragma mark Performing Requests + /** - * Performs a request + * Performs a request and returns a cancellation token associated with it. + * @discussion If the request can’t be performed `nil` will be returned and the receiver’s delegate will be sent the + * `dataLoader:didReceiveErrorResponse:`. The response object sent to the delegate will contain an `NSError` object + * describing what went wrong. * @param request The object describing the kind of request to be performed + * @return A cancellation token associated with the request, or `nil` if the request coulnd’t be performed. */ -- (id)performRequest:(SPTDataLoaderRequest *)request; +- (nullable id)performRequest:(SPTDataLoaderRequest *)request; + +#pragma mark Cancelling Loads + /** * Cancels all the currently operating and pending requests */ From 455d48db82cd8725a75e9dc5a2866d564dacedd8 Mon Sep 17 00:00:00 2001 From: Aron Cedercrantz Date: Mon, 22 Feb 2016 14:17:25 +0100 Subject: [PATCH 2/2] Fix methods returning nil when their return type is nonnull --- SPTDataLoader/SPTDataLoader.m | 20 ++++---- ...ataLoaderCancellationTokenImplementation.m | 9 ++-- SPTDataLoader/SPTDataLoaderFactory.m | 21 ++++----- SPTDataLoader/SPTDataLoaderRateLimiter.m | 17 +++---- SPTDataLoader/SPTDataLoaderRequest.m | 21 ++++----- .../SPTDataLoaderRequestTaskHandler.m | 31 ++++++------ SPTDataLoader/SPTDataLoaderResolverAddress.m | 14 +++--- SPTDataLoader/SPTDataLoaderResponse.m | 35 +++++++------- SPTDataLoader/SPTDataLoaderService.m | 47 +++++++++---------- 9 files changed, 100 insertions(+), 115 deletions(-) diff --git a/SPTDataLoader/SPTDataLoader.m b/SPTDataLoader/SPTDataLoader.m index 28e10c45..a1e8e572 100644 --- a/SPTDataLoader/SPTDataLoader.m +++ b/SPTDataLoader/SPTDataLoader.m @@ -30,8 +30,8 @@ @interface SPTDataLoader () -@property (nonatomic, strong) NSHashTable> *cancellationTokens; -@property (nonatomic, strong) NSMutableArray *requests; +@property (nonatomic, strong, readonly) NSHashTable> *cancellationTokens; +@property (nonatomic, strong, readonly) NSMutableArray *requests; @end @@ -46,16 +46,14 @@ + (instancetype)dataLoaderWithRequestResponseHandlerDelegate:(id)requestResponseHandlerDelegate { - if (!(self = [super init])) { - return nil; + self = [super init]; + if (self) { + _requestResponseHandlerDelegate = requestResponseHandlerDelegate; + + _cancellationTokens = [NSHashTable weakObjectsHashTable]; + _delegateQueue = dispatch_get_main_queue(); + _requests = [NSMutableArray new]; } - - _requestResponseHandlerDelegate = requestResponseHandlerDelegate; - - _cancellationTokens = [NSHashTable weakObjectsHashTable]; - _delegateQueue = dispatch_get_main_queue(); - _requests = [NSMutableArray new]; - return self; } diff --git a/SPTDataLoader/SPTDataLoaderCancellationTokenImplementation.m b/SPTDataLoader/SPTDataLoaderCancellationTokenImplementation.m index e2872215..efa536e3 100644 --- a/SPTDataLoader/SPTDataLoaderCancellationTokenImplementation.m +++ b/SPTDataLoader/SPTDataLoaderCancellationTokenImplementation.m @@ -41,13 +41,12 @@ + (instancetype)cancellationTokenImplementationWithDelegate:(id)delegate cancelObject:(nullable id)cancelObject { - if (!(self = [super init])) { - return nil; + self = [super init]; + if (self) { + _delegate = delegate; + _objectToCancel = cancelObject; } - _delegate = delegate; - _objectToCancel = cancelObject; - return self; } diff --git a/SPTDataLoader/SPTDataLoaderFactory.m b/SPTDataLoader/SPTDataLoaderFactory.m index 781a57f0..0d6e8b93 100644 --- a/SPTDataLoader/SPTDataLoaderFactory.m +++ b/SPTDataLoader/SPTDataLoaderFactory.m @@ -50,18 +50,17 @@ + (instancetype)dataLoaderFactoryWithRequestResponseHandlerDelegate:(nullable id - (instancetype)initWithRequestResponseHandlerDelegate:(nullable id)requestResponseHandlerDelegate authorisers:(nullable NSArray> *)authorisers { - if (!(self = [super init])) { - return nil; - } - - _requestResponseHandlerDelegate = requestResponseHandlerDelegate; - _authorisers = [authorisers copy]; - - _requestToRequestResponseHandler = [NSMapTable weakToWeakObjectsMapTable]; - _requestTimeoutQueue = dispatch_get_main_queue(); + self = [super init]; + if (self) { + _requestResponseHandlerDelegate = requestResponseHandlerDelegate; + _authorisers = [authorisers copy]; - for (id authoriser in _authorisers) { - authoriser.delegate = self; + _requestToRequestResponseHandler = [NSMapTable weakToWeakObjectsMapTable]; + _requestTimeoutQueue = dispatch_get_main_queue(); + + for (id authoriser in _authorisers) { + authoriser.delegate = self; + } } return self; diff --git a/SPTDataLoader/SPTDataLoaderRateLimiter.m b/SPTDataLoader/SPTDataLoaderRateLimiter.m index 1df6d8ab..cf11079c 100644 --- a/SPTDataLoader/SPTDataLoaderRateLimiter.m +++ b/SPTDataLoader/SPTDataLoaderRateLimiter.m @@ -45,15 +45,14 @@ + (instancetype)rateLimiterWithDefaultRequestsPerSecond:(double)requestsPerSecon - (instancetype)initWithDefaultRequestsPerSecond:(double)requestsPerSecond { - if (!(self = [super init])) { - return nil; + self = [super init]; + if (self) { + _requestsPerSecond = requestsPerSecond; + _serviceEndpointRequestsPerSecond = [NSMutableDictionary new]; + _serviceEndpointLastExecution = [NSMutableDictionary new]; + _serviceEndpointRetryAt = [NSMutableDictionary new]; } - _requestsPerSecond = requestsPerSecond; - _serviceEndpointRequestsPerSecond = [NSMutableDictionary new]; - _serviceEndpointLastExecution = [NSMutableDictionary new]; - _serviceEndpointRetryAt = [NSMutableDictionary new]; - return self; } @@ -135,10 +134,6 @@ - (double)requestsPerSecondForServiceKey:(NSString *)serviceKey - (NSString *)serviceKeyFromURL:(NSURL *)URL { - if (!URL) { - return nil; - } - NSURLComponents *requestComponents = [NSURLComponents componentsWithURL:URL resolvingAgainstBaseURL:NO]; NSURLComponents *serviceComponents = [NSURLComponents new]; serviceComponents.scheme = requestComponents.scheme; diff --git a/SPTDataLoader/SPTDataLoaderRequest.m b/SPTDataLoader/SPTDataLoaderRequest.m index 6f3ad3c9..ab5e15f6 100644 --- a/SPTDataLoader/SPTDataLoaderRequest.m +++ b/SPTDataLoader/SPTDataLoaderRequest.m @@ -51,17 +51,16 @@ - (instancetype)initWithURL:(NSURL *)URL sourceIdentifier:(nullable NSString *)s { static int64_t uniqueIdentifierBarrier = 0; - if (!(self = [super init])) { - return nil; - } - - _URL = URL; - _sourceIdentifier = sourceIdentifier; - - _mutableHeaders = [NSMutableDictionary new]; - _method = SPTDataLoaderRequestMethodGet; - @synchronized(self.class) { - _uniqueIdentifier = uniqueIdentifierBarrier++; + self = [super init]; + if (self) { + _URL = URL; + _sourceIdentifier = sourceIdentifier; + + _mutableHeaders = [NSMutableDictionary new]; + _method = SPTDataLoaderRequestMethodGet; + @synchronized(self.class) { + _uniqueIdentifier = uniqueIdentifierBarrier++; + } } return self; diff --git a/SPTDataLoader/SPTDataLoaderRequestTaskHandler.m b/SPTDataLoader/SPTDataLoaderRequestTaskHandler.m index 778b0abe..f30a726c 100644 --- a/SPTDataLoader/SPTDataLoaderRequestTaskHandler.m +++ b/SPTDataLoader/SPTDataLoaderRequestTaskHandler.m @@ -77,24 +77,23 @@ - (instancetype)initWithTask:(NSURLSessionTask *)task { const NSTimeInterval SPTDataLoaderRequestTaskHandlerMaximumTime = 60.0; const NSTimeInterval SPTDataLoaderRequestTaskHandlerInitialTime = 1.0; - - if (!(self = [super init])) { - return nil; + + self = [super init]; + if (self) { + _task = task; + _request = request; + _requestResponseHandler = requestResponseHandler; + _rateLimiter = rateLimiter; + + __weak __typeof(self) weakSelf = self; + _executionBlock = ^{ + [weakSelf checkRateLimiterAndExecute]; + }; + _exponentialTimer = [SPTDataLoaderExponentialTimer exponentialTimerWithInitialTime:SPTDataLoaderRequestTaskHandlerInitialTime + maxTime:SPTDataLoaderRequestTaskHandlerMaximumTime]; + _retryQueue = dispatch_get_main_queue(); } - _task = task; - _request = request; - _requestResponseHandler = requestResponseHandler; - _rateLimiter = rateLimiter; - - __weak __typeof(self) weakSelf = self; - _executionBlock = ^ { - [weakSelf checkRateLimiterAndExecute]; - }; - _exponentialTimer = [SPTDataLoaderExponentialTimer exponentialTimerWithInitialTime:SPTDataLoaderRequestTaskHandlerInitialTime - maxTime:SPTDataLoaderRequestTaskHandlerMaximumTime]; - _retryQueue = dispatch_get_main_queue(); - return self; } diff --git a/SPTDataLoader/SPTDataLoaderResolverAddress.m b/SPTDataLoader/SPTDataLoaderResolverAddress.m index ef449e13..f5085f83 100644 --- a/SPTDataLoader/SPTDataLoaderResolverAddress.m +++ b/SPTDataLoader/SPTDataLoaderResolverAddress.m @@ -24,8 +24,7 @@ @interface SPTDataLoaderResolverAddress () -@property (nonatomic, assign) NSTimeInterval stalePeriod; - +@property (nonatomic, assign, readonly) NSTimeInterval stalePeriod; @property (nonatomic, assign) CFAbsoluteTime lastFailedTime; @end @@ -53,14 +52,13 @@ + (instancetype)dataLoaderResolverAddressWithAddress:(NSString *)address - (instancetype)initWithAddress:(NSString *)address { const NSTimeInterval SPTDataLoaderResolverAddressDefaultStalePeriodOneHour = 60.0 * 60.0; - - if (!(self = [super init])) { - return nil; + + self = [super init]; + if (self) { + _address = address; + _stalePeriod = SPTDataLoaderResolverAddressDefaultStalePeriodOneHour; } - _address = address; - _stalePeriod = SPTDataLoaderResolverAddressDefaultStalePeriodOneHour; - return self; } diff --git a/SPTDataLoader/SPTDataLoaderResponse.m b/SPTDataLoader/SPTDataLoaderResponse.m index 0c20f270..df3a828a 100644 --- a/SPTDataLoader/SPTDataLoaderResponse.m +++ b/SPTDataLoader/SPTDataLoaderResponse.m @@ -49,27 +49,26 @@ + (instancetype)dataLoaderResponseWithRequest:(SPTDataLoaderRequest *)request re - (instancetype)initWithRequest:(SPTDataLoaderRequest *)request response:(nullable NSURLResponse *)response { - if (!(self = [super init])) { - return nil; - } - - _request = request; - _response = response; - - if ([response isKindOfClass:[NSHTTPURLResponse class]]) { - NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)response; - if (httpResponse.statusCode >= SPTDataLoaderResponseHTTPStatusCodeMovedMultipleChoices - || httpResponse.statusCode <= SPTDataLoaderResponseHTTPStatusCodeSwitchProtocols) { - _error = [NSError errorWithDomain:SPTDataLoaderResponseErrorDomain - code:httpResponse.statusCode - userInfo:nil]; + self = [super init]; + if (self) { + _request = request; + _response = response; + + if ([response isKindOfClass:[NSHTTPURLResponse class]]) { + NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)response; + if (httpResponse.statusCode >= SPTDataLoaderResponseHTTPStatusCodeMovedMultipleChoices + || httpResponse.statusCode <= SPTDataLoaderResponseHTTPStatusCodeSwitchProtocols) { + _error = [NSError errorWithDomain:SPTDataLoaderResponseErrorDomain + code:httpResponse.statusCode + userInfo:nil]; + } + _responseHeaders = httpResponse.allHeaderFields; + _statusCode = httpResponse.statusCode; } - _responseHeaders = httpResponse.allHeaderFields; - _statusCode = httpResponse.statusCode; + + _retryAfter = [self retryAfterForHeaders:_responseHeaders]; } - _retryAfter = [self retryAfterForHeaders:_responseHeaders]; - return self; } diff --git a/SPTDataLoader/SPTDataLoaderService.m b/SPTDataLoader/SPTDataLoaderService.m index 4b8ccd0a..d975282a 100644 --- a/SPTDataLoader/SPTDataLoaderService.m +++ b/SPTDataLoader/SPTDataLoaderService.m @@ -70,31 +70,30 @@ - (instancetype)initWithUserAgent:(NSString *)userAgent const NSUInteger SPTDataLoaderServiceMaxConcurrentOperations = 32; NSString * const SPTDataLoaderServiceUserAgentHeader = @"User-Agent"; - - if (!(self = [super init])) { - return nil; - } - - _rateLimiter = rateLimiter; - _resolver = resolver; - - NSURLSessionConfiguration *configuration = [NSURLSessionConfiguration defaultSessionConfiguration]; - configuration.timeoutIntervalForRequest = SPTDataLoaderServiceTimeoutInterval; - configuration.timeoutIntervalForResource = SPTDataLoaderServiceTimeoutInterval; - configuration.HTTPShouldUsePipelining = YES; - configuration.protocolClasses = customURLProtocolClasses; - if (userAgent) { - configuration.HTTPAdditionalHeaders = @{ SPTDataLoaderServiceUserAgentHeader : userAgent }; + + self = [super init]; + if (self) { + _rateLimiter = rateLimiter; + _resolver = resolver; + + NSURLSessionConfiguration *configuration = [NSURLSessionConfiguration defaultSessionConfiguration]; + configuration.timeoutIntervalForRequest = SPTDataLoaderServiceTimeoutInterval; + configuration.timeoutIntervalForResource = SPTDataLoaderServiceTimeoutInterval; + configuration.HTTPShouldUsePipelining = YES; + configuration.protocolClasses = customURLProtocolClasses; + if (userAgent) { + configuration.HTTPAdditionalHeaders = @{ SPTDataLoaderServiceUserAgentHeader : userAgent }; + } + + _cancellationTokenFactory = [SPTDataLoaderCancellationTokenFactoryImplementation new]; + _sessionQueue = [NSOperationQueue new]; + _sessionQueue.maxConcurrentOperationCount = SPTDataLoaderServiceMaxConcurrentOperations; + _sessionQueue.name = NSStringFromClass(self.class); + _session = [NSURLSession sessionWithConfiguration:configuration delegate:self delegateQueue:_sessionQueue]; + _handlers = [NSMutableArray new]; + _consumptionObservers = [NSMapTable weakToStrongObjectsMapTable]; } - _cancellationTokenFactory = [SPTDataLoaderCancellationTokenFactoryImplementation new]; - _sessionQueue = [NSOperationQueue new]; - _sessionQueue.maxConcurrentOperationCount = SPTDataLoaderServiceMaxConcurrentOperations; - _sessionQueue.name = NSStringFromClass(self.class); - _session = [NSURLSession sessionWithConfiguration:configuration delegate:self delegateQueue:_sessionQueue]; - _handlers = [NSMutableArray new]; - _consumptionObservers = [NSMapTable weakToStrongObjectsMapTable]; - return self; } @@ -121,7 +120,7 @@ - (void)removeConsumptionObserver:(id)consumpt } } -- (SPTDataLoaderRequestTaskHandler *)handlerForTask:(NSURLSessionTask *)task +- (nullable SPTDataLoaderRequestTaskHandler *)handlerForTask:(NSURLSessionTask *)task { NSArray *handlers = nil; @synchronized(self.handlers) {