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

Add a flushInterval configuration option #767

Merged
merged 2 commits into from
Jul 16, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 5 additions & 1 deletion Analytics/Classes/Internal/SEGSegmentIntegration.m
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,11 @@ - (id)initWithAnalytics:(SEGAnalytics *)analytics httpClient:(SEGHTTPClient *)ht

- (void)setupFlushTimer
{
self.flushTimer = [NSTimer scheduledTimerWithTimeInterval:30.0 target:self selector:@selector(flush) userInfo:nil repeats:YES];
self.flushTimer = [NSTimer scheduledTimerWithTimeInterval:self.configuration.flushInterval
target:self
selector:@selector(flush)
userInfo:nil
repeats:YES];
}

/*
Expand Down
4 changes: 4 additions & 0 deletions Analytics/Classes/SEGAnalyticsConfiguration.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ typedef NSMutableURLRequest *_Nonnull (^SEGRequestFactory)(NSURL *_Nonnull);
*/
@property (nonatomic, assign) NSUInteger flushAt;

/**
* The amount of time to wait before each tick of the flush timer. Smaller values will make events delivered in a more real-time manner and also use more battery. `30` by default.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

30 seconds will be clearer than just 30 I think

*/
@property (nonatomic, assign) NSTimeInterval flushInterval;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can someone set this to less than < 0?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually think something too low (1ms) may also be problematic. We should just say something like "Don't set this below 10seconds".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to Apple :

interval

The number of seconds between firings of the timer. If interval is less than or equal to 0.0, this method chooses the nonnegative value of 0.1 milliseconds instead.

I added this note :

A value smaller than 10 seconds will seriously degrade overall performance.


/**
* Whether the analytics client should automatically make a track call for application lifecycle events, such as "Application Installed", "Application Updated" and "Application Opened".
Expand Down
1 change: 1 addition & 0 deletions Analytics/Classes/SEGAnalyticsConfiguration.m
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ - (instancetype)init
self.enableAdvertisingTracking = YES;
self.shouldUseBluetooth = NO;
self.flushAt = 20;
self.flushInterval = 30;
_factories = [NSMutableArray array];
Class applicationClass = NSClassFromString(@"UIApplication");
if (applicationClass) {
Expand Down
24 changes: 24 additions & 0 deletions AnalyticsTests/AnalyticsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class AnalyticsTests: QuickSpec {

it("initialized correctly") {
expect(analytics.configuration.flushAt) == 20
expect(analytics.configuration.flushInterval) == 30
expect(analytics.configuration.writeKey) == "QUI5ydwIGeFFTa1IvCBUhxL9PyW5B0jE"
expect(analytics.configuration.shouldUseLocationServices) == false
expect(analytics.configuration.enableAdvertisingTracking) == true
Expand Down Expand Up @@ -120,6 +121,29 @@ class AnalyticsTests: QuickSpec {
let task = UIApplication.shared.beginBackgroundTask(expirationHandler: nil)
UIApplication.shared.endBackgroundTask(task)
}

it("flushes using flushTimer") {
let integration = analytics.test_integrationsManager()?.test_segmentIntegration()

analytics.track("test")

expect(integration?.test_flushTimer()).toEventuallyNot(beNil())
expect(integration?.test_batchRequest()).to(beNil())

integration?.test_flushTimer()?.fire()

expect(integration?.test_batchRequest()).toEventuallyNot(beNil())
}

it("respects flushInterval") {
let timer = analytics
.test_integrationsManager()?
.test_segmentIntegration()?
.test_flushTimer()

expect(timer).toNot(beNil())
expect(timer?.timeInterval) == config.flushInterval
}
}

}
6 changes: 6 additions & 0 deletions AnalyticsTests/Utils/TestUtils.swift
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ extension SEGSegmentIntegration {
func test_traits() -> [String: AnyObject]? {
return self.value(forKey: "traits") as? [String: AnyObject]
}
func test_flushTimer() -> Timer? {
return self.value(forKey: "flushTimer") as? Timer
}
func test_batchRequest() -> URLSessionUploadTask? {
return self.value(forKey: "batchRequest") as? URLSessionUploadTask
}
}


Expand Down