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

[PLAT-5682] Make max persisted events/sessions spec compliant #966

Merged
merged 1 commit into from
Jan 12, 2021
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
28 changes: 6 additions & 22 deletions Bugsnag/Configuration/BugsnagConfiguration.m
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,9 @@ - (instancetype)initWithApiKey:(NSString *)apiKey {
_enabledReleaseStages = nil;
_redactedKeys = [NSSet setWithArray:@[@"password"]];
_enabledBreadcrumbTypes = BSGEnabledBreadcrumbTypeAll;
_maxPersistedEvents = 12;
_maxPersistedSessions = 32;
_maxBreadcrumbs = 25;
_maxPersistedEvents = 32;
_maxPersistedSessions = 128;
_autoTrackSessions = YES;
_sendThreads = BSGThreadSendPolicyAlways;
// Default to recording all error types
Expand Down Expand Up @@ -441,41 +441,25 @@ -(void)deletePersistedUserData {
// MARK: - Properties: Getters and Setters
// -----------------------------------------------------------------------------

@synthesize maxPersistedEvents = _maxPersistedEvents;

- (NSUInteger)maxPersistedEvents {
@synchronized (self) {
return _maxPersistedEvents;
}
}

- (void)setMaxPersistedEvents:(NSUInteger)maxPersistedEvents {
@synchronized (self) {
if (maxPersistedEvents >= 1 && maxPersistedEvents <= 100) {
if (maxPersistedEvents >= 1) {
_maxPersistedEvents = maxPersistedEvents;
} else {
bsg_log_err(@"Invalid configuration value detected. Option maxPersistedEvents "
"should be an integer between 1-100. Supplied value is %lu",
"should be a non-zero integer. Supplied value is %lu",
(unsigned long) maxPersistedEvents);
}
}
}

@synthesize maxPersistedSessions = _maxPersistedSessions;

- (NSUInteger)maxPersistedSessions {
@synchronized (self) {
return _maxPersistedSessions;
}
}

- (void)setMaxPersistedSessions:(NSUInteger)maxPersistedSessions {
@synchronized (self) {
if (maxPersistedSessions >= 1 && maxPersistedSessions <= 100) {
if (maxPersistedSessions >= 1) {
_maxPersistedSessions = maxPersistedSessions;
} else {
bsg_log_err(@"Invalid configuration value detected. Option maxPersistedSessions "
"should be an integer between 1-100. Supplied value is %lu",
"should be a non-zero integer. Supplied value is %lu",
(unsigned long) maxPersistedSessions);
}
}
Expand Down
4 changes: 2 additions & 2 deletions Bugsnag/include/Bugsnag/BugsnagConfiguration.h
Original file line number Diff line number Diff line change
Expand Up @@ -222,15 +222,15 @@ typedef BOOL (^BugsnagOnSessionBlock)(BugsnagSession *_Nonnull session);
* Sets the maximum number of events which will be stored. Once the threshold is reached,
* the oldest events will be deleted.
*
* By default, 12 events are stored: this can be amended up to a maximum of 100.
* By default, 32 events are persisted.
*/
@property (nonatomic) NSUInteger maxPersistedEvents;

/**
* Sets the maximum number of sessions which will be stored. Once the threshold is reached,
* the oldest sessions will be deleted.
*
* By default, 32 sessions are stored: this can be amended up to a maximum of 100.
* By default, 128 sessions are persisted.
*/
@property (nonatomic) NSUInteger maxPersistedSessions;

Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ Changelog

* Fix missing session information in OOM events.
[#963](https://github.com/bugsnag/bugsnag-cocoa/pull/963)
* Make `maxPersistedEvents` and `maxPersistedSessions` comply with the specification, with defaults of 32 and 128 respectively.
[#966](https://github.com/bugsnag/bugsnag-cocoa/pull/966)

## 6.5.0 (2021-01-06)

Expand Down
6 changes: 3 additions & 3 deletions Tests/BSGConfigurationBuilderTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ - (void)testDecodeDefaultValues {
XCTAssertNil(config.appVersion);
XCTAssertTrue(config.autoDetectErrors);
XCTAssertTrue(config.autoTrackSessions);
XCTAssertEqual(12, config.maxPersistedEvents);
XCTAssertEqual(32, config.maxPersistedSessions);
XCTAssertEqual(25, config.maxBreadcrumbs);
XCTAssertEqual(config.maxPersistedEvents, 32);
XCTAssertEqual(config.maxPersistedSessions, 128);
XCTAssertEqual(config.maxBreadcrumbs, 25);
XCTAssertTrue(config.persistUser);
XCTAssertEqualObjects(@[@"password"], [config.redactedKeys allObjects]);
XCTAssertEqual(BSGThreadSendPolicyAlways, config.sendThreads);
Expand Down
46 changes: 12 additions & 34 deletions Tests/BugsnagConfigurationTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -535,30 +535,19 @@ - (void)testSettingPersistUser {

- (void)testMaxPersistedEvents {
BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
XCTAssertEqual(12, config.maxPersistedEvents);
XCTAssertEqual(config.maxPersistedEvents, 32, @"maxPersistedEvents should default to 32");

// alter to valid value
config.maxPersistedEvents = 10;
XCTAssertEqual(10, config.maxPersistedEvents);
XCTAssertEqual(config.maxPersistedEvents, 10, @"Valid values should be accepted");

// alter to max value
config.maxPersistedEvents = 100;
XCTAssertEqual(100, config.maxPersistedEvents);
config.maxPersistedEvents = 1000;
XCTAssertEqual(config.maxPersistedEvents, 1000, @"No maximum bound should be applied");

// alter to min value
config.maxPersistedEvents = 1;
XCTAssertEqual(1, config.maxPersistedEvents);
XCTAssertEqual(config.maxPersistedEvents, 1, @"A value of 1 should be accepted");

config.maxPersistedEvents = 0;
XCTAssertEqual(1, config.maxPersistedEvents);

// alter to negative value
config.maxPersistedEvents = -1;
XCTAssertEqual(1, config.maxPersistedEvents);

// alter to > max value
config.maxPersistedEvents = 500;
XCTAssertEqual(1, config.maxPersistedEvents);
XCTAssertEqual(config.maxPersistedEvents, 1, @"Setting to zero should have no effect");
}

// =============================================================================
Expand All @@ -567,30 +556,19 @@ - (void)testMaxPersistedEvents {

- (void)testMaxPersistedSessions {
BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
XCTAssertEqual(32, config.maxPersistedSessions);
XCTAssertEqual(config.maxPersistedSessions, 128, @"maxPersistedSessions should default to 128");

// alter to valid value
config.maxPersistedSessions = 10;
XCTAssertEqual(10, config.maxPersistedSessions);
XCTAssertEqual(config.maxPersistedSessions, 10, @"Valid values should be accepted");

// alter to max value
config.maxPersistedSessions = 100;
XCTAssertEqual(100, config.maxPersistedSessions);
config.maxPersistedSessions = 1000;
XCTAssertEqual(config.maxPersistedSessions, 1000, @"No maximum bound should be applied");

// alter to min value
config.maxPersistedSessions = 1;
XCTAssertEqual(1, config.maxPersistedSessions);
XCTAssertEqual(config.maxPersistedSessions, 1, @"A value of 1 should be accepted");

config.maxPersistedSessions = 0;
XCTAssertEqual(1, config.maxPersistedSessions);

// alter to negative value
config.maxPersistedSessions = -1;
XCTAssertEqual(1, config.maxPersistedSessions);

// alter to > max value
config.maxPersistedSessions = 500;
XCTAssertEqual(1, config.maxPersistedSessions);
XCTAssertEqual(config.maxPersistedSessions, 1, @"Setting to zero should have no effect");
}

// =============================================================================
Expand Down
20 changes: 0 additions & 20 deletions Tests/ConfigurationApiValidationTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -137,16 +137,6 @@ - (void)testValidMaxPersistedEvents {
XCTAssertEqual(40, self.config.maxPersistedEvents);
}

- (void)testInvalidMaxPersistedEvents {
self.config.maxPersistedEvents = 1;
self.config.maxPersistedEvents = 0;
XCTAssertEqual(1, self.config.maxPersistedEvents);
self.config.maxPersistedEvents = -1;
XCTAssertEqual(1, self.config.maxPersistedEvents);
self.config.maxPersistedEvents = 590;
XCTAssertEqual(1, self.config.maxPersistedEvents);
}

- (void)testValidMaxPersistedSessions {
self.config.maxPersistedSessions = 1;
XCTAssertEqual(1, self.config.maxPersistedSessions);
Expand All @@ -156,16 +146,6 @@ - (void)testValidMaxPersistedSessions {
XCTAssertEqual(40, self.config.maxPersistedSessions);
}

- (void)testInvalidMaxPersistedSessions {
self.config.maxPersistedSessions = 1;
self.config.maxPersistedSessions = 0;
XCTAssertEqual(1, self.config.maxPersistedSessions);
self.config.maxPersistedSessions = -1;
XCTAssertEqual(1, self.config.maxPersistedSessions);
self.config.maxPersistedSessions = 590;
XCTAssertEqual(1, self.config.maxPersistedSessions);
}

- (void)testValidMaxBreadcrumbs {
self.config.maxBreadcrumbs = 0;
XCTAssertEqual(0, self.config.maxBreadcrumbs);
Expand Down