diff --git a/CHANGELOG.md b/CHANGELOG.md index f3a48f6b9..9d6c30982 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -199,6 +199,9 @@ Bugsnag Notifiers on other platforms. * The metadata interface is now consistent across the `Bugsnag`, `BugsnagMetadata`, `BugsnagConfig`, `BugsnagClient` and `BugsnagEvent` classes. (#513)[https://github.com/bugsnag/bugsnag-cocoa/pull/513] + +* `BugsnagClient` now takes a shallow copy of the configuration passed in on initialisation. +(#524)[https://github.com/bugsnag/bugsnag-cocoa/pull/524] ## Bug fixes diff --git a/Source/BugsnagClient.m b/Source/BugsnagClient.m index cf8ae4168..f16e46fdd 100644 --- a/Source/BugsnagClient.m +++ b/Source/BugsnagClient.m @@ -319,7 +319,8 @@ - (id)initWithConfiguration:(BugsnagConfiguration *)initConfiguration { static NSString *const BSGWatchdogSentinelFileName = @"bugsnag_oom_watchdog.json"; static NSString *const BSGCrashSentinelFileName = @"bugsnag_handled_crash.txt"; if ((self = [super init])) { - self.configuration = initConfiguration; + // Take a shallow copy of the configuration + self.configuration = [initConfiguration copy]; self.state = [[BugsnagMetadata alloc] init]; NSString *notifierName = #if TARGET_OS_TV @@ -362,7 +363,7 @@ - (id)initWithConfiguration:(BugsnagConfiguration *)initConfiguration { [self initializeNotificationNameMap]; }); - self.sessionTracker = [[BugsnagSessionTracker alloc] initWithConfig:initConfiguration + self.sessionTracker = [[BugsnagSessionTracker alloc] initWithConfig:self.configuration postRecordCallback:^(BugsnagSession *session) { BSGWriteSessionCrashData(session); }]; diff --git a/Source/BugsnagConfiguration.m b/Source/BugsnagConfiguration.m index 5a50ae9d5..62b009476 100644 --- a/Source/BugsnagConfiguration.m +++ b/Source/BugsnagConfiguration.m @@ -55,6 +55,10 @@ @interface Bugsnag () + (BugsnagClient *)client; @end +@interface BugsnagMetadata () +- (NSDictionary *_Nonnull)toDictionary; +@end + @interface BugsnagConfiguration () /** @@ -88,8 +92,54 @@ @interface BugsnagConfiguration () @property(readonly, strong, nullable) BugsnagBreadcrumbs *breadcrumbs; @end +// ============================================================================= +// MARK: - BugsnagConfiguration +// ============================================================================= + @implementation BugsnagConfiguration +// ----------------------------------------------------------------------------- +// MARK: - +// ----------------------------------------------------------------------------- + +/** + * Produce a shallow copy of the BugsnagConfiguration object. + * + * @param zone This parameter is ignored. Memory zones are no longer used by Objective-C. + */ +- (nonnull id)copyWithZone:(nullable NSZone *)zone { + BugsnagConfiguration *copy = [[BugsnagConfiguration alloc] initWithApiKey:[NSMutableString stringWithString:self.apiKey]]; + // Omit apiKey - it's set explicitly in the line above + [copy setAppType:self.appType]; + [copy setAppVersion:self.appVersion]; + [copy setAutoDetectErrors:self.autoDetectErrors]; + [copy setAutoTrackSessions:self.autoTrackSessions]; + // Skip breadcrumbs - none should have been set + [copy setCodeBundleId:self.codeBundleId]; + [copy setConfig:[[BugsnagMetadata alloc] initWithDictionary:[[self.config toDictionary] mutableCopy]]]; + [copy setContext:self.context]; + [copy setEnabledBreadcrumbTypes:self.enabledBreadcrumbTypes]; + [copy setEnabledErrorTypes:self.enabledErrorTypes]; + [copy setEnabledReleaseStages:self.enabledReleaseStages]; + [copy setMaxBreadcrumbs:self.maxBreadcrumbs]; + [copy setMetadata: [[BugsnagMetadata alloc] initWithDictionary:[[self.metadata toDictionary] mutableCopy]]]; + [copy setEndpointsForNotify:self.notifyURL.absoluteString + sessions:self.sessionURL.absoluteString]; + [copy setOnBreadcrumbBlocks:[self.onBreadcrumbBlocks mutableCopy]]; + [copy setOnCrashHandler:self.onCrashHandler]; + [copy setOnSendBlocks:[self.onSendBlocks mutableCopy]]; + [copy setOnSessionBlocks:[self.onSessionBlocks mutableCopy]]; + [copy setPersistUser:self.persistUser]; + [copy setPlugins:[self.plugins copy]]; + [copy setReleaseStage:self.releaseStage]; + [copy setSession:[self.session copy]]; + [copy setUser:self.user.userId + withEmail:self.user.emailAddress + andName:self.user.name]; + + return copy; +} + // ----------------------------------------------------------------------------- // MARK: - Class Methods // ----------------------------------------------------------------------------- diff --git a/Tests/BugsnagClientTests.m b/Tests/BugsnagClientTests.m index f4ce775c4..9efe106f8 100644 --- a/Tests/BugsnagClientTests.m +++ b/Tests/BugsnagClientTests.m @@ -69,7 +69,7 @@ - (void)testAutomaticNotifyBreadcrumbData { // Check that the event is passed the apiKey [Bugsnag notify:ex block:^(BugsnagEvent * _Nonnull event) { - XCTAssertEqual(event.apiKey, DUMMY_APIKEY_32CHAR_1); + XCTAssertEqualObjects(event.apiKey, DUMMY_APIKEY_32CHAR_1); // Grab the values that end up in the event for later comparison eventErrorClass = [event errorClass]; diff --git a/Tests/BugsnagConfigurationTests.m b/Tests/BugsnagConfigurationTests.m index ae6b1edea..e7c133220 100644 --- a/Tests/BugsnagConfigurationTests.m +++ b/Tests/BugsnagConfigurationTests.m @@ -22,12 +22,12 @@ @interface BugsnagConfiguration () @property(nonatomic, readwrite, strong) NSMutableArray *onSendBlocks; @property(nonatomic, readwrite, strong) NSMutableArray *onSessionBlocks; +@property(readonly, retain, nullable) NSURL *sessionURL; +@property(readonly, retain, nullable) NSURL *notifyURL; - (void)deletePersistedUserData; - (BOOL)shouldSendReports; - (NSDictionary *_Nonnull)errorApiHeaders; - (NSDictionary *_Nonnull)sessionApiHeaders; -@property(readonly, retain, nullable) NSURL *sessionURL; -@property(readonly, retain, nullable) NSURL *notifyURL; @end @interface BugsnagCrashSentry () @@ -159,7 +159,6 @@ - (void)testAddOnSessionBlockThenRemove { __block XCTestExpectation *expectation1 = [self expectationWithDescription:@"Remove On Session Block 1"]; __block XCTestExpectation *expectation2 = [self expectationWithDescription:@"Remove On Session Block 2"]; __block XCTestExpectation *expectation3 = [self expectationWithDescription:@"Remove On Session Block 3"]; - expectation3.inverted = YES; __block XCTestExpectation *expectation4 = [self expectationWithDescription:@"Remove On Session Block 4"]; expectation4.inverted = YES; @@ -199,7 +198,7 @@ - (void)testAddOnSessionBlockThenRemove { [Bugsnag startSession]; [self waitForExpectations:@[expectation2] timeout:1.0]; - // Check it's NOT called once the block's deleted + // Check it's still called once the block's deleted (block has been copied in client init) [Bugsnag pauseSession]; called++; [config removeOnSessionBlock:sessionBlock]; @@ -763,6 +762,55 @@ - (void) testClearOnSendBlock { XCTAssertEqual([[configuration onSendBlocks] count], 2); } +- (void)testNSCopying { + BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; + + // Set some arbirtary values: + [config setUser:@"foo" withEmail:@"bar@baz.com" andName:@"Bill"]; + [config setApiKey:DUMMY_APIKEY_32CHAR_1]; + [config setAutoDetectErrors:YES]; + [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; }; + + NSArray *sendBlocks = @[ onSendBlock1, onSendBlock2 ]; + [config setOnSendBlocks:[sendBlocks mutableCopy]]; // Mutable arg required + + // Clone + BugsnagConfiguration *clone = [config copy]; + XCTAssertNotEqual(config, clone); + + // Change values + + // Object + [clone setUser:@"Cthulu" withEmail:@"hp@lovecraft.com" andName:@"Howard"]; + XCTAssertEqualObjects(config.user.userId, @"foo"); + XCTAssertEqualObjects(clone.user.userId, @"Cthulu"); + + // String + [clone setApiKey:DUMMY_APIKEY_32CHAR_2]; + XCTAssertEqualObjects(config.apiKey, DUMMY_APIKEY_32CHAR_1); + XCTAssertEqualObjects(clone.apiKey, DUMMY_APIKEY_32CHAR_2); + + // Bool + [clone setAutoDetectErrors:NO]; + XCTAssertTrue(config.autoDetectErrors); + XCTAssertFalse(clone.autoDetectErrors); + + // Block + [clone setOnCrashHandler:config.onCrashHandler]; + XCTAssertEqual(config.onCrashHandler, clone.onCrashHandler); + [clone setOnCrashHandler:(void *)^(const BSG_KSCrashReportWriter *_Nonnull writer){}]; + XCTAssertNotEqual(config.onCrashHandler, clone.onCrashHandler); + + // Array (of blocks) + XCTAssertNotEqual(config.onSendBlocks, clone.onSendBlocks); + XCTAssertEqual(config.onSendBlocks[0], clone.onSendBlocks[0]); + [clone setOnSendBlocks:[@[ onSendBlock2 ] mutableCopy]]; + XCTAssertNotEqual(config.onSendBlocks[0], clone.onSendBlocks[0]); +} - (void)testMetadataMutability { BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; diff --git a/Tests/BugsnagEventTests.m b/Tests/BugsnagEventTests.m index 8e1bab166..c964b4d77 100644 --- a/Tests/BugsnagEventTests.m +++ b/Tests/BugsnagEventTests.m @@ -485,23 +485,23 @@ - (void)testApiKey { // Check that the event is passed the apiKey [Bugsnag notify:ex block:^(BugsnagEvent * _Nonnull event) { - XCTAssertEqual(event.apiKey, DUMMY_APIKEY_32CHAR_1); + XCTAssertEqualObjects(event.apiKey, DUMMY_APIKEY_32CHAR_1); }]; // Check that we can change it [Bugsnag notify:ex block:^(BugsnagEvent * _Nonnull event) { - XCTAssertEqual(event.apiKey, DUMMY_APIKEY_32CHAR_1); + XCTAssertEqualObjects(event.apiKey, DUMMY_APIKEY_32CHAR_1); event.apiKey = DUMMY_APIKEY_32CHAR_2; XCTAssertEqual(event.apiKey, DUMMY_APIKEY_32CHAR_2); - XCTAssertEqual(Bugsnag.configuration.apiKey, DUMMY_APIKEY_32CHAR_1); + XCTAssertEqualObjects(Bugsnag.configuration.apiKey, DUMMY_APIKEY_32CHAR_1); }]; // Check that the global configuration is unaffected [Bugsnag notify:ex block:^(BugsnagEvent * _Nonnull event) { - XCTAssertEqual(event.apiKey, DUMMY_APIKEY_32CHAR_1); + XCTAssertEqualObjects(event.apiKey, DUMMY_APIKEY_32CHAR_1); event.apiKey = DUMMY_APIKEY_32CHAR_1; XCTAssertEqual(event.apiKey, DUMMY_APIKEY_32CHAR_1); - XCTAssertEqual(Bugsnag.configuration.apiKey, DUMMY_APIKEY_32CHAR_1); + XCTAssertEqualObjects(Bugsnag.configuration.apiKey, DUMMY_APIKEY_32CHAR_1); event.apiKey = DUMMY_APIKEY_32CHAR_3; XCTAssertEqual(event.apiKey, DUMMY_APIKEY_32CHAR_3); }]; diff --git a/Tests/BugsnagOnBreadcrumbTest.m b/Tests/BugsnagOnBreadcrumbTest.m index 06fd03940..394b86b76 100644 --- a/Tests/BugsnagOnBreadcrumbTest.m +++ b/Tests/BugsnagOnBreadcrumbTest.m @@ -13,6 +13,15 @@ #import "BugsnagTestConstants.h" #import "BugsnagBreadcrumbs.h" +@interface BugsnagClient () +@property(nonatomic, readwrite, retain) BugsnagConfiguration *_Nullable configuration; +@end + +@interface Bugsnag () ++ (BugsnagClient *)client; ++ (void)removeOnBreadcrumbBlock:(BugsnagOnBreadcrumbBlock _Nonnull)block; +@end + @interface BugsnagConfiguration () @property NSMutableArray *onBreadcrumbBlocks; @property BugsnagBreadcrumbs *breadcrumbs; @@ -120,7 +129,8 @@ - (void)testAddOnBreadcrumbBlockThenRemove { // Check it's NOT called once the block's deleted called++; - [config removeOnBreadcrumbBlock:crumbBlock]; + [Bugsnag removeOnBreadcrumbBlock:crumbBlock]; + [Bugsnag leaveBreadcrumbWithMessage:@"Hello"]; [self waitForExpectations:@[expectation2] timeout:1.0]; } @@ -171,8 +181,8 @@ - (void)testAddOnBreadcrumbMutation { // Call onbreadcrumb blocks [Bugsnag startBugsnagWithConfiguration:config]; XCTAssertEqual([[config onBreadcrumbBlocks] count], 1); - BugsnagBreadcrumb *crumb = [[config breadcrumbs].breadcrumbs firstObject]; - XCTAssertEqualObjects(@"Foo", crumb.message); + NSDictionary *crumb = [[[[[Bugsnag client] configuration] breadcrumbs] arrayValue] firstObject]; + XCTAssertEqualObjects(@"Foo", crumb[@"name"]); } /** diff --git a/Tests/BugsnagTests.m b/Tests/BugsnagTests.m index 7446bb099..ce4adc422 100644 --- a/Tests/BugsnagTests.m +++ b/Tests/BugsnagTests.m @@ -272,6 +272,9 @@ - (void)testRemoveOnSessionBlock { [self waitForExpectations:@[expectation2] timeout:1.0]; } +/** + * Test that we can add an onSession block, and that it's called correctly when a session starts + */ - (void)testAddOnSessionBlock { __block int called = 0; // A counter diff --git a/features/config_changes_after_start.feature b/features/config_changes_after_start.feature index 0953ccd4e..cfdb9fa57 100644 --- a/features/config_changes_after_start.feature +++ b/features/config_changes_after_start.feature @@ -5,19 +5,25 @@ Feature: Modifying configuration settings after start() is called Background: Given I set environment variable "BUGSNAG_API_KEY" to "a35a2a72bd230ac0aa0f52715bbdc6aa" + # autoTracksessions is on by default. Turning it on in config before crashing + # should not cause the event to be sent. Scenario: Turning on crash detection after start() When I crash the app using "TurnOnCrashDetectionAfterStartScenario" And I relaunch the app + And I wait for 10 seconds + Then I should receive 0 requests + + # autoTracksessions is on by default. Turning it off in config before crashing + # should not stop the event being sent. + Scenario: Turning off crash detection after start() + When I crash the app using "TurnOffCrashDetectionAfterStartScenario" + And I relaunch the app And I wait for a request Then the request is valid for the error reporting API And the "Bugsnag-API-Key" header equals "a35a2a72bd230ac0aa0f52715bbdc6aa" And the payload field "notifier.name" equals "iOS Bugsnag Notifier" And the payload field "events" is an array with 1 element And the exception "errorClass" equals "EXC_BAD_INSTRUCTION" - And the "method" of stack frame 0 equals "-[TurnOnCrashDetectionAfterStartScenario run]" - - Scenario: Turning off crash detection after start() - When I crash the app using "TurnOffCrashDetectionAfterStartScenario" - And I relaunch the app + And the "method" of stack frame 0 equals "-[TurnOffCrashDetectionAfterStartScenario run]" And I wait for 10 seconds - Then I should receive 0 requests + Then I should receive 1 request