Skip to content

Commit e26a23d

Browse files
Nightsd01jkasten2
authored andcommitted
Add Timeout
• Adds a 25 second timeout to make sure that even if an app implements the OSNotificationDisplayTypeDelegate but doesn't execute the callback, the SDK will timeout and will still show the notification • The complicating factor is that multiple notifications can arrive rapidly, so the app needs to be able to map the correct display types to previously received notifications. • TODO: Before this PR gets reviewed, I've been working on a test for the timeout edge case but it is a complex timing situation so this test will be added in the next day or two.
1 parent e694cb9 commit e26a23d

File tree

5 files changed

+82
-16
lines changed

5 files changed

+82
-16
lines changed

iOS_SDK/OneSignalSDK/Source/OneSignal.m

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,9 @@ @implementation OneSignal
201201
// Display type is used in multiple areas of the SDK
202202
// To avoid calling the delegate multiple times, we store
203203
// the type and notification ID for each notification
204+
// These data structures *MUST* be accessed on the main thread only
204205
static NSMutableDictionary<NSString *, NSNumber *> *_displayTypeMap;
206+
static NSMutableDictionary<NSString *, NSMutableArray<OSNotificationDisplayTypeResponse> *> *_pendingDisplayTypeCallbacks;
205207

206208
static OSNotificationDisplayType _inFocusDisplayType = OSNotificationDisplayTypeInAppAlert;
207209
+ (void)setInFocusDisplayType:(OSNotificationDisplayType)value {
@@ -403,6 +405,7 @@ + (void)clearStatics {
403405

404406
_displayDelegate = nil;
405407
_displayTypeMap = [NSMutableDictionary new];
408+
_pendingDisplayTypeCallbacks = [NSMutableDictionary new];
406409
}
407410

408411
// Set to false as soon as it's read.
@@ -1759,6 +1762,12 @@ + (void)setLastnonActiveMessageId:(NSString*)value { _lastnonActiveMessageId = v
17591762

17601763
+ (void)displayTypeForNotificationPayload:(NSDictionary *)payload withCompletion:(OSNotificationDisplayTypeResponse)completion {
17611764
[OneSignalHelper runOnMainThread:^{
1765+
if (!_displayTypeMap)
1766+
_displayTypeMap = [NSMutableDictionary new];
1767+
1768+
if (!_pendingDisplayTypeCallbacks)
1769+
_pendingDisplayTypeCallbacks = [NSMutableDictionary new];
1770+
17621771
var type = self.inFocusDisplayType;
17631772

17641773
// check to make sure the app is in focus and it's a OneSignal notification
@@ -1773,23 +1782,64 @@ + (void)displayTypeForNotificationPayload:(NSDictionary *)payload withCompletion
17731782
let notificationId = osPayload.notificationID;
17741783

17751784
// Prevent calling the delegate multiple times for the same payload
1776-
if (_displayTypeMap[osPayload.notificationID]) {
1785+
// Checks to see if there is a pending delegate request
1786+
if (_pendingDisplayTypeCallbacks[notificationId]) {
1787+
[_pendingDisplayTypeCallbacks[notificationId] addObject:completion];
1788+
// checks to see if the delegate already responded for this notification
1789+
} else if (_displayTypeMap[osPayload.notificationID]) {
17771790
type = (OSNotificationDisplayType)[_displayTypeMap[osPayload.notificationID] intValue];
17781791
completion(type);
17791792
} else if (_displayDelegate) {
1793+
NSMutableArray *callbacks = [NSMutableArray new];
1794+
[callbacks addObject:completion];
1795+
[_pendingDisplayTypeCallbacks setObject:callbacks forKey:notificationId];
1796+
1797+
NSTimer *watchdogTimer = [NSTimer scheduledTimerWithTimeInterval:CUSTOM_DISPLAY_TYPE_TIMEOUT
1798+
target:self
1799+
selector:@selector(watchdogTimerFired:)
1800+
userInfo:notificationId
1801+
repeats:false];
1802+
17801803
[_displayDelegate willPresentInFocusNotificationWithPayload:osPayload withDefaultDisplayType:type withCompletion:^(OSNotificationDisplayType displayType) {
17811804
[OneSignalHelper runOnMainThread:^{
1805+
NSMutableArray<OSNotificationDisplayTypeResponse> *callbacks = _pendingDisplayTypeCallbacks[notificationId];
1806+
1807+
if (!callbacks || callbacks.count == 0)
1808+
return;
1809+
1810+
[watchdogTimer invalidate];
1811+
[_pendingDisplayTypeCallbacks removeObjectForKey:notificationId];
17821812
_displayTypeMap[notificationId] = @((int)displayType);
1783-
completion(displayType);
1813+
1814+
for (OSNotificationDisplayTypeResponse callback in callbacks)
1815+
callback(displayType);
17841816
}];
17851817
}];
17861818
} else {
1819+
// No delegate is set; uses the main display type set with OneSignal.setInFocusDisplayType()
17871820
_displayTypeMap[notificationId] = @((int)type);
17881821
completion(type);
17891822
}
17901823
}];
17911824
}
17921825

1826+
// If this is called, it means OSNotificationDisplayTypeDelegate did not execute the callback
1827+
// within the max time range, and timed out. We must display the notification anyways
1828+
// using the default display type to avoid dropping notifications.
1829+
+ (void)watchdogTimerFired:(NSTimer *)timer {
1830+
NSString *notificationId = (NSString *)timer.userInfo;
1831+
NSMutableArray<OSNotificationDisplayTypeResponse> *callbacks = _pendingDisplayTypeCallbacks[notificationId];
1832+
1833+
// Check just in case the app called the callback just as this timer fired
1834+
if (!callbacks || callbacks.count == 0)
1835+
return;
1836+
1837+
[_pendingDisplayTypeCallbacks removeObjectForKey:notificationId];
1838+
1839+
for (OSNotificationDisplayTypeResponse completion in callbacks)
1840+
completion(_inFocusDisplayType);
1841+
}
1842+
17931843
// Entry point for the following:
17941844
// - 1. (iOS all) - Opening notifications
17951845
// - 2. Notification received

iOS_SDK/OneSignalSDK/Source/OneSignalCommonDefines.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,10 @@ typedef enum {GET, POST, HEAD, PUT, DELETE, OPTIONS, CONNECT, TRACE} HTTPMethod;
130130

131131
// the max number of UNNotificationCategory ID's the SDK will register
132132
#define MAX_CATEGORIES_SIZE 128
133+
134+
// Defines how long the SDK will wait for OSNotificationDisplayTypeDelegate to execute
135+
// the callback to set the display type for a given notification
136+
#define CUSTOM_DISPLAY_TYPE_TIMEOUT 25.0
133137
#else
134138
// Test defines for API Client
135139
#define REATTEMPT_DELAY 0.004
@@ -142,6 +146,10 @@ typedef enum {GET, POST, HEAD, PUT, DELETE, OPTIONS, CONNECT, TRACE} HTTPMethod;
142146

143147
// the max number of UNNotificationCategory ID's the SDK will register
144148
#define MAX_CATEGORIES_SIZE 5
149+
150+
// Defines how long the SDK will wait for OSNotificationDisplayTypeDelegate to execute
151+
// the callback to set the display type for a given notification
152+
#define CUSTOM_DISPLAY_TYPE_TIMEOUT 0.2
145153
#endif
146154

147155
// A max timeout for a request, which might include multiple reattempts

iOS_SDK/OneSignalSDK/UnitTests/DummyNotificationDisplayTypeDelegate.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ NS_ASSUME_NONNULL_BEGIN
3636

3737
@property (nonatomic) OSNotificationDisplayType overrideDisplayType;
3838

39+
@property (nonatomic) BOOL shouldFireCompletionBlock;
40+
3941
// Each time the OSNotificationDisplayTypeDelegate.willPresentNotificationWithPayload()
4042
// method is called, this int is incremented
4143
@property (nonatomic) int numberOfCalls;

iOS_SDK/OneSignalSDK/UnitTests/DummyNotificationDisplayTypeDelegate.m

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,19 @@
3131
@implementation DummyNotificationDisplayTypeDelegate
3232

3333
- (instancetype)init {
34-
if (self = [super init])
34+
if (self = [super init]) {
3535
_numberOfCalls = 0;
36+
_shouldFireCompletionBlock = false;
37+
}
3638

3739
return self;
3840
}
3941

4042
- (void)willPresentInFocusNotificationWithPayload:(OSNotificationPayload *)payload withDefaultDisplayType:(OSNotificationDisplayType)displayType withCompletion:(OSNotificationDisplayTypeResponse)completion {
4143
_notificationId = payload.notificationID;
4244

43-
completion(_overrideDisplayType);
45+
if (_shouldFireCompletionBlock)
46+
completion(_overrideDisplayType);
4447

4548
_numberOfCalls += 1;
4649
}

iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2076,7 +2076,7 @@ - (void)testRegistersAfterNoApnsResponse {
20762076
[expectation fulfill];
20772077
});
20782078

2079-
[self waitForExpectations:@[expectation] timeout:0.1];
2079+
[self waitForExpectations:@[expectation] timeout:0.2];
20802080

20812081
// If APNS didn't respond within X seconds, the SDK
20822082
// should have registered the user with OneSignal
@@ -2278,9 +2278,7 @@ - (void)testNotificationWithButtonsRegistersUniqueCategory {
22782278
XCTAssertEqualObjects(content.categoryIdentifier, @"__onesignal__dynamic__b2f7f966-d8cc-11e4-bed1-df8f05be55ba");
22792279
}
22802280

2281-
- (void)testOverrideNotificationDisplayType {
2282-
let dummyDelegate = [DummyNotificationDisplayTypeDelegate new];
2283-
2281+
- (NSDictionary *)setUpNotificationDisplayTypeTestWithDummyDelegate:(DummyNotificationDisplayTypeDelegate *)dummyDelegate withDisplayType:(OSNotificationDisplayType)displayType {
22842282
id userInfo = @{@"aps": @{
22852283
@"mutable-content": @1,
22862284
@"alert": @{@"body": @"Message Body", @"title": @"title"},
@@ -2291,25 +2289,30 @@ - (void)testOverrideNotificationDisplayType {
22912289
@"buttons": @[@{@"i": @"id1", @"n": @"text1"}],
22922290
}};
22932291

2294-
__block BOOL openedWasFire = false;
2295-
id receiveBlock = ^(OSNotificationOpenedResult *result) {
2296-
openedWasFire = true;
2297-
};
2298-
2299-
[OneSignal initWithLaunchOptions:nil appId:@"b2f7f966-d8cc-11e4-bed1-df8f05be55ba" handleNotificationAction:receiveBlock];
2292+
[OneSignal initWithLaunchOptions:nil appId:@"b2f7f966-d8cc-11e4-bed1-df8f05be55ba" handleNotificationAction:nil];
23002293

23012294
[OneSignal setNotificationDisplayTypeDelegate:dummyDelegate];
23022295

2303-
[OneSignal setInFocusDisplayType:OSNotificationDisplayTypeNone];
2296+
[OneSignal setInFocusDisplayType:displayType];
23042297

23052298
UIApplicationOverrider.currentUIApplicationState = UIApplicationStateActive;
23062299

23072300
[UnitTestCommonMethods resumeApp];
23082301
[UnitTestCommonMethods runBackgroundThreads];
23092302

2310-
// Even though the display type is set to None, this should
2303+
return userInfo;
2304+
}
2305+
2306+
- (void)testOverrideNotificationDisplayType {
2307+
let dummyDelegate = [DummyNotificationDisplayTypeDelegate new];
2308+
2309+
// Even though the display type will be set to None, this should
23112310
// cause the SDK to present this notification as an alert
23122311
dummyDelegate.overrideDisplayType = OSNotificationDisplayTypeInAppAlert;
2312+
dummyDelegate.shouldFireCompletionBlock = true;
2313+
2314+
let userInfo = [self setUpNotificationDisplayTypeTestWithDummyDelegate:dummyDelegate
2315+
withDisplayType:OSNotificationDisplayTypeNone];
23132316

23142317
id notifResponse = [UnitTestCommonMethods createBasiciOSNotificationResponseWithPayload:userInfo];
23152318
[notifResponse setValue:@"id1" forKeyPath:@"actionIdentifier"];

0 commit comments

Comments
 (0)