Merge pull request #534 from bugsnag/v6-blocks

Make all callbacks return boolean values
bugsnag · Apr 16, 2020 · f7cb9cc · f7cb9cc
2 parents 7d50435 + ecec977
commit f7cb9cc
Show file tree

Hide file tree

Showing 11 changed files with 76 additions and 31 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,9 @@ Bugsnag Notifiers on other platforms.
 
 ## Enhancements
 
+* Make all callbacks return boolean values
+  [#534](https://github.com/bugsnag/bugsnag-cocoa/pull/534)
+
 * Add `originalError` property to `BugsnagEvent`
   [#541](https://github.com/bugsnag/bugsnag-cocoa/pull/541)
 

diff --git a/Source/BugsnagConfiguration.h b/Source/BugsnagConfiguration.h
@@ -59,7 +59,7 @@ typedef void (^BugsnagOnErrorBlock)(BugsnagEvent *_Nonnull event);
  *
  *  @return YES if the event should be sent
  */
-typedef bool (^BugsnagOnSendBlock)(BugsnagEvent *_Nonnull event);
+typedef BOOL (^BugsnagOnSendBlock)(BugsnagEvent *_Nonnull event);
 
 /**
  *  A configuration block for modifying a captured breadcrumb
@@ -73,7 +73,7 @@ typedef BOOL (^BugsnagOnBreadcrumbBlock)(BugsnagBreadcrumb *_Nonnull breadcrumb)
  *
  * @param sessionPayload The session about to be delivered
  */
-typedef void(^BugsnagOnSessionBlock)(NSMutableDictionary *_Nonnull sessionPayload);
+typedef BOOL (^BugsnagOnSessionBlock)(NSMutableDictionary *_Nonnull sessionPayload);
 
 typedef NS_OPTIONS(NSUInteger, BSGErrorType) {
     BSGErrorTypesNone         NS_SWIFT_NAME(None)         = 0,

diff --git a/Source/BugsnagSessionTracker.m b/Source/BugsnagSessionTracker.m
@@ -32,6 +32,7 @@ - (void)resume;
 
 @interface BugsnagConfiguration ()
 @property(readonly, retain, nullable) NSURL *sessionURL;
+@property(nonatomic, readwrite, strong) NSMutableArray *onSessionBlocks;
 @end
 
 @interface BugsnagSessionTracker ()
@@ -116,11 +117,20 @@ - (void)startNewSessionWithAutoCaptureValue:(BOOL)isAutoCaptured {
         return;
     }
 
+    // TODO JL: refactor to pass as structured data into callback
+    BugsnagSessionTrackingPayload *sessions = [[BugsnagSessionTrackingPayload alloc] initWithSessions:@[]
+                                                                                               config:self.config];
+    NSMutableDictionary *sessionsJson = [sessions toJson];
+    for (BugsnagOnSessionBlock onSessionBlock in self.config.onSessionBlocks) {
+        if (!onSessionBlock(sessionsJson)) {
+            return;
+        }
+    }
+
     self.currentSession = [[BugsnagSession alloc] initWithId:[[NSUUID UUID] UUIDString]
                                                    startDate:[NSDate date]
                                                         user:self.config.user
                                                 autoCaptured:isAutoCaptured];
-
     [self.sessionStore write:self.currentSession];
 
     if (self.callback) {

diff --git a/Source/BugsnagSessionTrackingApiClient.m b/Source/BugsnagSessionTrackingApiClient.m
@@ -49,10 +49,6 @@ - (void)deliverSessionsInStore:(BugsnagSessionFileStore *)store {
         NSUInteger sessionCount = payload.sessions.count;
         NSMutableDictionary *data = [payload toJson];
 
-        for (BugsnagOnSessionBlock cb in self.config.onSessionBlocks) {
-            cb(data);
-        }
-
         if (sessionCount > 0) {
             NSDictionary *HTTPHeaders = @{
                                           @"Bugsnag-Payload-Version": @"1.0",

diff --git a/Tests/BugsnagBaseUnitTest.m b/Tests/BugsnagBaseUnitTest.m
@@ -44,7 +44,7 @@ -(void)setUpBugsnagWillCallNotify:(bool)willNotify
     [configuration setPersistUser:willPersistUser];
 
     if (willNotify) {
-        [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }];
+        [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }];
     }
     [Bugsnag startBugsnagWithConfiguration:configuration];
 }

diff --git a/Tests/BugsnagBreadcrumbsTest.m b/Tests/BugsnagBreadcrumbsTest.m
@@ -302,7 +302,7 @@ - (void)testBreadcrumbFromDict {
 - (void)testCallbackFreeConstructors2 {
     // Prevent sending events
     BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
-    [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }];
+    [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }];
     [Bugsnag startBugsnagWithConfiguration:configuration];
 
     NSDictionary *md1 = @{ @"x" : @"y"};
@@ -369,7 +369,7 @@ - (void)testCallbackFreeConstructors2 {
 - (void)testCallbackFreeConstructors3 {
     // Prevent sending events
     BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
-    [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }];
+    [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }];
     [Bugsnag startBugsnagWithConfiguration:configuration];
 
     [Bugsnag leaveBreadcrumbWithMessage:@"message1"];

diff --git a/Tests/BugsnagClientTests.m b/Tests/BugsnagClientTests.m
@@ -47,7 +47,7 @@ @implementation BugsnagClientTests
 -(void)setUpBugsnagWillCallNotify:(bool)willNotify {
     BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
     if (willNotify) {
-        [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }];
+        [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }];
     }
     [Bugsnag startBugsnagWithConfiguration:configuration];
 }

diff --git a/Tests/BugsnagConfigurationTests.m b/Tests/BugsnagConfigurationTests.m
@@ -108,9 +108,10 @@ - (void)testAddOnSessionBlock {
     BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
     [config setEndpointsForNotify:@"http://notreal.bugsnag.com" sessions:@"http://notreal.bugsnag.com"];
     XCTAssertEqual([[config onSessionBlocks] count], 0);
-    BugsnagOnSessionBlock sessionBlock = ^(NSMutableDictionary * _Nonnull sessionPayload) {
+    BugsnagOnSessionBlock sessionBlock = ^BOOL(NSMutableDictionary * _Nonnull sessionPayload) {
         // We expect the session block to be called
         [expectation fulfill];
+        return true;
     };
     [config addOnSessionBlock:sessionBlock];
     XCTAssertEqual([[config onSessionBlocks] count], 1);
@@ -132,8 +133,9 @@ - (void)testRemoveOnSessionBlock {
     BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
     [config setEndpointsForNotify:@"http://notreal.bugsnag.com" sessions:@"http://notreal.bugsnag.com"];
     XCTAssertEqual([[config onSessionBlocks] count], 0);
-    BugsnagOnSessionBlock sessionBlock = ^(NSMutableDictionary * _Nonnull sessionPayload) {
+    BugsnagOnSessionBlock sessionBlock = ^BOOL(NSMutableDictionary * _Nonnull sessionPayload) {
         [calledExpectation fulfill];
+        return true;
     };
 
     // It's there (and from other tests we know it gets called) and then it's not there
@@ -166,7 +168,7 @@ - (void)testAddOnSessionBlockThenRemove {
     [config setEndpointsForNotify:@"http://notreal.bugsnag.com" sessions:@"http://notreal.bugsnag.com"];
     XCTAssertEqual([[config onSessionBlocks] count], 0);
 
-    BugsnagOnSessionBlock sessionBlock = ^(NSMutableDictionary * _Nonnull sessionPayload) {
+    BugsnagOnSessionBlock sessionBlock = ^BOOL(NSMutableDictionary * _Nonnull sessionPayload) {
         switch (called) {
         case 0:
             [expectation1 fulfill];
@@ -183,6 +185,7 @@ - (void)testAddOnSessionBlockThenRemove {
             [expectation4 fulfill];
             break;
         }
+        return true;
     };
 
     [config addOnSessionBlock:sessionBlock];
@@ -219,8 +222,8 @@ - (void)testAddOnSessionBlockThenRemove {
 - (void)testRemoveNonexistentOnSessionBlocks {
     BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
     XCTAssertEqual([[config onSessionBlocks] count], 0);
-    BugsnagOnSessionBlock sessionBlock1 = ^(NSMutableDictionary * _Nonnull sessionPayload) {};
-    BugsnagOnSessionBlock sessionBlock2 = ^(NSMutableDictionary * _Nonnull sessionPayload) {};
+    BugsnagOnSessionBlock sessionBlock1 = ^BOOL(NSMutableDictionary * _Nonnull sessionPayload) { return true; };
+    BugsnagOnSessionBlock sessionBlock2 = ^BOOL(NSMutableDictionary * _Nonnull sessionPayload) { return true; };
 
     [config addOnSessionBlock:sessionBlock1];
     XCTAssertEqual([[config onSessionBlocks] count], 1);
@@ -731,7 +734,7 @@ - (void) testRemoveOnSendBlock {
     BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
     XCTAssertEqual([[configuration onSendBlocks] count], 0);
 
-    BugsnagOnSendBlock block = ^bool(BugsnagEvent * _Nonnull event) { return false; };
+    BugsnagOnSendBlock block = ^BOOL(BugsnagEvent * _Nonnull event) { return false; };
 
     [configuration addOnSendBlock:block];
     [Bugsnag startBugsnagWithConfiguration:configuration];
@@ -750,8 +753,8 @@ - (void) testClearOnSendBlock {
     BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
     XCTAssertEqual([[configuration onSendBlocks] count], 0);
 
-    BugsnagOnSendBlock block1 = ^bool(BugsnagEvent * _Nonnull event) { return false; };
-    BugsnagOnSendBlock block2 = ^bool(BugsnagEvent * _Nonnull event) { return false; };
+    BugsnagOnSendBlock block1 = ^BOOL(BugsnagEvent * _Nonnull event) { return false; };
+    BugsnagOnSendBlock block2 = ^BOOL(BugsnagEvent * _Nonnull event) { return false; };
 
     // Add more than one
     [configuration addOnSendBlock:block1];
@@ -772,8 +775,8 @@ - (void)testNSCopying {
     [config setContext:@"context1"];
     [config setAppType:@"The most amazing app, a brilliant app, the app to end all apps"];
     [config setPersistUser:YES];
-    BugsnagOnSendBlock onSendBlock1 = ^bool(BugsnagEvent * _Nonnull event) { return true; };
-    BugsnagOnSendBlock onSendBlock2 = ^bool(BugsnagEvent * _Nonnull event) { return true; };
+    BugsnagOnSendBlock onSendBlock1 = ^BOOL(BugsnagEvent * _Nonnull event) { return true; };
+    BugsnagOnSendBlock onSendBlock2 = ^BOOL(BugsnagEvent * _Nonnull event) { return true; };
 
     NSArray *sendBlocks = @[ onSendBlock1, onSendBlock2 ];
     [config setOnSendBlocks:[sendBlocks mutableCopy]]; // Mutable arg required

diff --git a/Tests/BugsnagSessionTrackerTest.m b/Tests/BugsnagSessionTrackerTest.m
@@ -24,6 +24,10 @@ @interface BugsnagConfiguration ()
 - (void)deletePersistedUserData;
 @end
 
+@interface BugsnagSessionTracker ()
+@property (strong, readwrite) BugsnagSession *currentSession;
+@end
+
 @interface BugsnagSessionTrackerTest : XCTestCase
 @property BugsnagConfiguration *configuration;
 @property BugsnagSessionTracker *sessionTracker;
@@ -139,4 +143,31 @@ - (void)testIncrementCounts {
     XCTAssertEqual(1, session.unhandledCount);
 }
 
+- (void)testOnSendBlockFalse {
+    self.configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
+    [self.configuration addOnSessionBlock:^BOOL(NSMutableDictionary *sessionPayload) {
+        return NO;
+    }];
+    self.sessionTracker = [[BugsnagSessionTracker alloc] initWithConfig:self.configuration
+                                                     postRecordCallback:nil];
+    [self.sessionTracker startNewSession];
+    XCTAssertNil(self.sessionTracker.currentSession);
+}
+
+- (void)testOnSendBlockTrue {
+    __block XCTestExpectation *expectation = [self expectationWithDescription:@"Session block is invoked"];
+
+    self.configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
+    [self.configuration addOnSessionBlock:^BOOL(NSMutableDictionary *sessionPayload) {
+        [expectation fulfill];
+        return YES;
+    }];
+    self.sessionTracker = [[BugsnagSessionTracker alloc] initWithConfig:self.configuration
+                                                     postRecordCallback:nil];
+    [self.sessionTracker startNewSession];
+    [self waitForExpectations:@[expectation] timeout:2];
+    XCTAssertNotNil(self.sessionTracker.currentSession);
+}
+
+
 @end
diff --git a/Tests/BugsnagTests.m b/Tests/BugsnagTests.m
@@ -44,7 +44,7 @@ @implementation BugsnagTests
 -(void)setUpBugsnagWillCallNotify:(bool)willNotify {
     BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
     if (willNotify) {
-        [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }];
+        [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }];
     }
     [Bugsnag startBugsnagWithConfiguration:configuration];
 }
@@ -147,7 +147,7 @@ - (void)testGetMetadata {
  */
 -(void)testBugsnagPauseSession {
     BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
-    [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }];
+    [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }];
 
     [Bugsnag startBugsnagWithConfiguration:configuration];
 
@@ -164,7 +164,7 @@ - (void)testMutableContext {
 
     BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
     [configuration setContext:@"firstContext"];
-    [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }];
+    [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }];
 
     [Bugsnag startBugsnagWithConfiguration:configuration];
 
@@ -247,9 +247,9 @@ - (void)testRemoveOnSessionBlock {
     BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
 
     // non-sending bugsnag
-    [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }];
+    [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }];
 
-    BugsnagOnSessionBlock sessionBlock = ^(NSMutableDictionary * _Nonnull sessionPayload) {
+    BugsnagOnSessionBlock sessionBlock = ^BOOL(NSMutableDictionary * _Nonnull sessionPayload) {
         switch (called) {
         case 0:
             [expectation1 fulfill];
@@ -258,6 +258,7 @@ - (void)testRemoveOnSessionBlock {
             [expectation2 fulfill];
             break;
         }
+        return true;
     };
 
     [configuration addOnSessionBlock:sessionBlock];
@@ -287,9 +288,9 @@ - (void)testAddOnSessionBlock {
     configuration.autoTrackSessions = NO;
 
     // non-sending bugsnag
-    [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }];
+    [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }];
 
-    BugsnagOnSessionBlock sessionBlock = ^(NSMutableDictionary * _Nonnull sessionPayload) {
+    BugsnagOnSessionBlock sessionBlock = ^BOOL(NSMutableDictionary * _Nonnull sessionPayload) {
         switch (called) {
         case 0:
             [expectation1 fulfill];
@@ -298,6 +299,7 @@ - (void)testAddOnSessionBlock {
             [expectation2 fulfill];
             break;
         }
+        return true;
     };
 
     // NOTE: Due to test conditions the state of the Bugsnag/client class is indeterminate.
@@ -348,7 +350,7 @@ - (void) testOnSendBlocks {
     expectation6.inverted = YES;
 
     // Two blocks that will get called (or not) when we notify()
-    BugsnagOnSendBlock block1 = ^bool(BugsnagEvent * _Nonnull event)
+    BugsnagOnSendBlock block1 = ^BOOL(BugsnagEvent * _Nonnull event)
     {
         switch (called) {
             case 0:
@@ -372,7 +374,7 @@ - (void) testOnSendBlocks {
         return false;
     };
 
-    BugsnagOnSendBlock block2 = ^bool(BugsnagEvent * _Nonnull event)
+    BugsnagOnSendBlock block2 = ^BOOL(BugsnagEvent * _Nonnull event)
     {
         switch (called) {
             case 0:

diff --git a/features/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/OOMScenario.m b/features/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/OOMScenario.m
@@ -6,7 +6,7 @@ @implementation OOMScenario
 - (void)startBugsnag {
     self.config.autoTrackSessions = NO;
     self.config.releaseStage = @"alpha";
-    [self.config addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) {
+    [self.config addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) {
         [event addMetadata:@{ @"shape": @"line" } toSection:@"extra"];
         return YES;
     }];