Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request reattempt fix #418

Merged
merged 2 commits into from
Sep 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1147,6 +1147,11 @@
CLANG_WARN_SUSPICIOUS_MOVES = YES;
"CODE_SIGN_IDENTITY[sdk=iphoneos*]" = "iPhone Developer";
DEVELOPMENT_TEAM = 99SW8E36CT;
GCC_PREPROCESSOR_DEFINITIONS = (
"DEBUG=1",
"$(inherited)",
"OS_TEST=1",
);
INFOPLIST_FILE = UnitTests/Info.plist;
IPHONEOS_DEPLOYMENT_TARGET = 10.1;
LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks";
Expand Down
31 changes: 21 additions & 10 deletions iOS_SDK/OneSignalSDK/Source/OneSignalClient.m
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,7 @@
#import "UIApplicationDelegate+OneSignal.h"
#import "ReattemptRequest.h"
#import "OneSignal.h"

#define REATTEMPT_DELAY 30.0
#define REQUEST_TIMEOUT_REQUEST 60.0 //for most HTTP requests
#define REQUEST_TIMEOUT_RESOURCE 100.0 //for loading a resource like an image
#define MAX_ATTEMPT_COUNT 3
#import "OneSignalCommonDefines.h"

@interface OneSignal (OneSignalClientExtra)
+ (BOOL)shouldLogMissingPrivacyConsentErrorWithMethodName:(NSString *)methodName;
Expand Down Expand Up @@ -66,6 +62,14 @@ -(instancetype)init {
return self;
}

- (NSError *)privacyConsentErrorWithRequestType:(NSString *)type {
return [NSError errorWithDomain:@"OneSignal Error" code:0 userInfo:@{@"error" : [NSString stringWithFormat: @"Attempted to perform an HTTP request (%@) before the user provided privacy consent.", type]}];
}

- (NSError *)genericTimedOutError {
return [NSError errorWithDomain:@"OneSignal Error" code:0 userInfo:@{@"error" : @"The request timed out"}];
}

- (void)executeSimultaneousRequests:(NSDictionary<NSString *, OneSignalRequest *> *)requests withSuccess:(OSMultipleSuccessBlock)successBlock onFailure:(OSMultipleFailureBlock)failureBlock {
if (requests.allKeys.count == 0)
return;
Expand All @@ -92,7 +96,15 @@ - (void)executeSimultaneousRequests:(NSDictionary<NSString *, OneSignalRequest *
}];
}

dispatch_group_wait(group, dispatch_time(DISPATCH_TIME_NOW, REQUEST_TIMEOUT_REQUEST * NSEC_PER_SEC));
// Will wait for up to (maxTimeout) seconds and will then give up and call
// the failure block if the request times out.
let timedOut = (bool)(0 != dispatch_group_wait(group, dispatch_time(DISPATCH_TIME_NOW, MAX_TIMEOUT)));

// add a generic 'timed out' error if the request timed out
// and there are no other errors present.
if (timedOut && errors.allKeys.count == 0)
for (NSString *key in requests.allKeys)
errors[key] = [self genericTimedOutError];

//requests should all be completed at this point
dispatch_async(dispatch_get_main_queue(), ^{
Expand All @@ -109,9 +121,7 @@ - (void)executeRequest:(OneSignalRequest *)request onSuccess:(OSResultSuccessBlo

if (request.method != GET && [OneSignal shouldLogMissingPrivacyConsentErrorWithMethodName:nil]) {
if (failureBlock) {
failureBlock([NSError errorWithDomain:@"OneSignal Error" code:0
userInfo:@{@"error" : [NSString stringWithFormat:
@"Attempted to perform an HTTP request (%@) before the user provided privacy consent.", NSStringFromClass(request.class)]}]);
failureBlock([self privacyConsentErrorWithRequestType:NSStringFromClass(request.class)]);
}

return;
Expand Down Expand Up @@ -198,7 +208,8 @@ - (BOOL)willReattemptRequest:(int)statusCode withRequest:(OneSignalRequest *)req

if (async) {
//retry again in 15 seconds
[OneSignal onesignal_Log:ONE_S_LL_DEBUG message:[NSString stringWithFormat:@"Re-scheduling request (%@) to be re-attempted in %i seconds due to failed HTTP request with status code %i", NSStringFromClass([request class]), (int)REATTEMPT_DELAY, (int)statusCode]];
[OneSignal onesignal_Log:ONE_S_LL_DEBUG message:[NSString stringWithFormat:@"Re-scheduling request (%@) to be re-attempted in %.3f seconds due to failed HTTP request with status code %i", NSStringFromClass([request class]), REATTEMPT_DELAY, (int)statusCode]];
NSLog(@"Delay: %f", REATTEMPT_DELAY);

[OneSignalHelper performSelector:@selector(reattemptRequest:) onMainThreadOnObject:self withObject:reattempt afterDelay:REATTEMPT_DELAY];
} else {
Expand Down
23 changes: 23 additions & 0 deletions iOS_SDK/OneSignalSDK/Source/OneSignalCommonDefines.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,27 @@
// before registering the user anyways
#define APNS_TIMEOUT 25.0

#ifndef OS_TEST
// OneSignal API Client Defines
#define REATTEMPT_DELAY 30.0
#define REQUEST_TIMEOUT_REQUEST 120.0 //for most HTTP requests
#define REQUEST_TIMEOUT_RESOURCE 120.0 //for loading a resource like an image
#define MAX_ATTEMPT_COUNT 3

// Send tags batch delay
#define SEND_TAGS_DELAY 5.0
#else
// Test defines for API Client
#define REATTEMPT_DELAY 0.04
#define REQUEST_TIMEOUT_REQUEST 0.2 //for most HTTP requests
#define REQUEST_TIMEOUT_RESOURCE 0.2 //for loading a resource like an image
#define MAX_ATTEMPT_COUNT 3

// Send tags batch delay
#define SEND_TAGS_DELAY 0.05
#endif

// A max timeout for a request, which might include multiple reattempts
#define MAX_TIMEOUT ((REQUEST_TIMEOUT_REQUEST * MAX_ATTEMPT_COUNT) + (REATTEMPT_DELAY * MAX_ATTEMPT_COUNT)) * NSEC_PER_SEC

#endif /* OneSignalCommonDefines_h */
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,8 @@
@interface NSURLSessionOverrider : NSObject

@end


@interface MockNSURLSessionDataTask : NSURLSessionDataTask
-(void)resume;
@end
27 changes: 26 additions & 1 deletion iOS_SDK/OneSignalSDK/UnitTests/Shadows/NSURLSessionOverrider.m
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,18 @@
*/

#import "NSURLSessionOverrider.h"

#import "OneSignalSelectorHelpers.h"
#import "TestHelperFunctions.h"
#import "OneSignalHelper.h"



@implementation NSURLSessionOverrider

+ (void)load {
// Swizzle an injected method defined in OneSignalHelper
injectStaticSelector([NSURLSessionOverrider class], @selector(overrideDownloadItemAtURL:toFile:error:), [NSURLSession class], @selector(downloadItemAtURL:toFile:error:));
injectToProperClass(@selector(overrideDataTaskWithRequest:completionHandler:), @selector(dataTaskWithRequest:completionHandler:), @[], [NSURLSessionOverrider class], [NSURLSession class]);
}

// Override downloading of media attachment
Expand All @@ -52,4 +56,25 @@ + (NSString *)overrideDownloadItemAtURL:(NSURL*)url toFile:(NSString*)localPath
return @"image/jpg";
}

- (NSURLSessionDataTask *)overrideDataTaskWithRequest:(NSURLRequest *)request completionHandler:(void (^)(NSData * _Nullable data, NSURLResponse * _Nullable response, NSError * _Nullable error))completionHandler {

// mimics no active network connection
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(0.001 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
NSHTTPURLResponse *response = [[NSHTTPURLResponse alloc] initWithURL:request.URL statusCode:0 HTTPVersion:@"1.1" headerFields:@{}];
NSError *error = [NSError errorWithDomain:@"OneSignal Error" code:0 userInfo:@{@"error" : @"The user is not currently connected to the network."}];
NSLog(@"Calling completion handler");
completionHandler(nil, response, error);
});

return [MockNSURLSessionDataTask new];
}

@end

@implementation MockNSURLSessionDataTask

-(void)resume {
//unimplemented
}

@end
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,6 @@
+(NSString *)lastHTTPRequestType;
+(void)setRequiresEmailAuth:(BOOL)required;
+(BOOL)hasExecutedRequestOfType:(Class)type;
+ (void)disableExecuteRequestOverride:(BOOL)disable;
@end

12 changes: 12 additions & 0 deletions iOS_SDK/OneSignalSDK/UnitTests/Shadows/OneSignalClientOverrider.m
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ @implementation OneSignalClientOverrider
static NSString *lastHTTPRequestType;
static BOOL requiresEmailAuth = false;
static NSMutableArray<NSString *> *executedRequestTypes;
static BOOL disableOverride = false;

+ (void)load {
serialMockMainLooper = dispatch_queue_create("com.onesignal.unittest", DISPATCH_QUEUE_SERIAL);
Expand All @@ -63,7 +64,14 @@ + (void)load {
executedRequestTypes = [[NSMutableArray alloc] init];
}

// Calling this function twice results in reversing the swizzle
+ (void)disableExecuteRequestOverride:(BOOL)disable {
disableOverride = disable;
}

- (void)overrideExecuteSimultaneousRequests:(NSDictionary<NSString *, OneSignalRequest *> *)requests withSuccess:(OSMultipleSuccessBlock)successBlock onFailure:(OSMultipleFailureBlock)failureBlock {
if (disableOverride)
return [self overrideExecuteSimultaneousRequests:requests withSuccess:successBlock onFailure:failureBlock];

dispatch_semaphore_t semaphore = dispatch_semaphore_create(0);

Expand Down Expand Up @@ -94,6 +102,10 @@ - (void)overrideExecuteSimultaneousRequests:(NSDictionary<NSString *, OneSignalR
}

- (void)overrideExecuteRequest:(OneSignalRequest *)request onSuccess:(OSResultSuccessBlock)successBlock onFailure:(OSFailureBlock)failureBlock {

if (disableOverride)
return [self overrideExecuteRequest:request onSuccess:successBlock onFailure:failureBlock];

[executedRequestTypes addObject:NSStringFromClass([request class])];

if (executeInstantaneously) {
Expand Down
42 changes: 42 additions & 0 deletions iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -2051,4 +2051,46 @@ - (void)testRegistersAfterNoApnsResponse {
XCTAssertTrue(observer->last.to.userId != nil);
}

/*
To prevent tests from generating actual HTTP requests, we swizzle
a method called executeRequest() in the OneSignalClient class

However, this test ensures that HTTP retry logic occurs correctly.
We have additionally swizzled NSURLSession to prevent real HTTP
requests from being generated.

TODO: Remove the OneSignalClientOverrider mock entirely and
instead swizzle NSURLSession
*/
- (void)testHTTPClientTimeout {
[OneSignal initWithLaunchOptions:nil appId:@"b2f7f966-d8cc-11e4-bed1-df8f05be55ba"
handleNotificationAction:nil
settings:nil];

[UnitTestCommonMethods runBackgroundThreads];

// Switches from overriding OneSignalClient to using a
// swizzled NSURLSession instead. This results in NSURLSession
// mimicking a no-network connection state
[OneSignalClientOverrider disableExecuteRequestOverride:true];

let expectation = [self expectationWithDescription:@"timeout_test"];
expectation.expectedFulfillmentCount = 1;

[OneSignal sendTags:@{@"test_tag_key" : @"test_tag_value"} onSuccess:^(NSDictionary *result) {
XCTFail(@"Success should not be called");
} onFailure:^(NSError *error) {
[expectation fulfill];
}];

[NSObjectOverrider runPendingSelectors];
[UnitTestCommonMethods runBackgroundThreads];
[NSObjectOverrider runPendingSelectors];

[self waitForExpectations:@[expectation] timeout:10.0];

// revert the swizzle back to the standard state for tests
[OneSignalClientOverrider disableExecuteRequestOverride:false];
}

@end