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.

Sign up for GitHub

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

remove initializerQueue, move db logic from init to initializeApiKey,… #109

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

remove initializerQueue, move db logic from init to initializeApiKey,… #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
128 changes: 78 additions & 50 deletions Amplitude/Amplitude.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
@interface Amplitude ()

@property (nonatomic, strong) NSOperationQueue *backgroundQueue;
@property (nonatomic, strong) NSOperationQueue *initializerQueue;
@property (nonatomic, strong) AMPDatabaseHelper *dbHelper;
@property (nonatomic, assign) BOOL initialized;
@property (nonatomic, assign) BOOL sslPinningEnabled;
Expand Down Expand Up @@ -226,7 +225,6 @@ - (id)initWithInstanceName:(NSString*) instanceName
_backoffUpload = NO;
_offline = NO;
_instanceName = SAFE_ARC_RETAIN(instanceName);
_dbHelper = SAFE_ARC_RETAIN([AMPDatabaseHelper getDatabaseHelper:instanceName]);

self.eventUploadThreshold = kAMPEventUploadThreshold;
self.eventMaxCount = kAMPEventMaxCount;
Expand All @@ -235,31 +233,28 @@ - (id)initWithInstanceName:(NSString*) instanceName
self.minTimeBetweenSessionsMillis = kAMPMinTimeBetweenSessionsMillis;
_backoffUploadBatchSize = self.eventUploadMaxBatchSize;

_initializerQueue = [[NSOperationQueue alloc] init];
_backgroundQueue = [[NSOperationQueue alloc] init];
// Force method calls to happen in FIFO order by only allowing 1 concurrent operation
[_backgroundQueue setMaxConcurrentOperationCount:1];
// Ensure initialize finishes running asynchronously before other calls are run
[_backgroundQueue setSuspended:YES];
// Name the queue so runOnBackgroundQueue can tell which queue an operation is running
_backgroundQueue.name = BACKGROUND_QUEUE_NAME;

[_initializerQueue addOperationWithBlock:^{
[_backgroundQueue addOperationWithBlock:^{
Copy link
Member

Choose a reason for hiding this comment

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

want to make sure here that this is guaranteed to be called before any other _backgroundQueue addOperationWithBlock calls can be made

Copy link
Member Author

Choose a reason for hiding this comment

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

I just searched through the code and all addOperationWithBlock and runOnBackgroundQueue calls are protected with an apiKey check, so they need to call initializeApiKey before any of those methods work.


_deviceInfo = [[AMPDeviceInfo alloc] init];

_uploadTaskID = UIBackgroundTaskInvalid;


// resolve path strings
NSString *eventsDataDirectory = [NSSearchPathForDirectoriesInDomains(NSLibraryDirectory, NSUserDomainMask, YES) objectAtIndex: 0];
NSString *propertyListPath = [eventsDataDirectory stringByAppendingPathComponent:@"com.amplitude.plist"];
if (![_instanceName isEqualToString:kAMPDefaultInstance]) {
propertyListPath = [NSString stringWithFormat:@"%@_%@", propertyListPath, _instanceName]; // namespace pList with instance name
}
_propertyListPath = SAFE_ARC_RETAIN(propertyListPath);
_eventsDataPath = SAFE_ARC_RETAIN([eventsDataDirectory stringByAppendingPathComponent:@"com.amplitude.archiveDict"]);
[self upgradePrefs];
[self upgradePrefs]; // migrate legacy prefs file

// Load propertyList object
// Load propertyList object, which contains current db version
_propertyList = SAFE_ARC_RETAIN([self deserializePList:_propertyListPath]);
if (!_propertyList) {
_propertyList = SAFE_ARC_RETAIN([NSMutableDictionary dictionary]);
Expand All @@ -271,42 +266,6 @@ - (id)initWithInstanceName:(NSString*) instanceName
} else {
AMPLITUDE_LOG(@"Loaded from %@", _propertyListPath);
}

// update database if necessary
int oldDBVersion = 1;
NSNumber *oldDBVersionSaved = [_propertyList objectForKey:DATABASE_VERSION];
if (oldDBVersionSaved != nil) {
oldDBVersion = [oldDBVersionSaved intValue];
}

// update the database
if (oldDBVersion < kAMPDBVersion) {
if ([self.dbHelper upgrade:oldDBVersion newVersion:kAMPDBVersion]) {
[_propertyList setObject:[NSNumber numberWithInt:kAMPDBVersion] forKey:DATABASE_VERSION];
[self savePropertyList];
}
}

// only on default instance, migrate all of old _eventsData object to database store if database just created
if ([_instanceName isEqualToString:kAMPDefaultInstance] && oldDBVersion < kAMPDBFirstVersion) {
if ([self migrateEventsDataToDB]) {
// delete events data so don't need to migrate next time
if ([[NSFileManager defaultManager] fileExistsAtPath:_eventsDataPath]) {
[[NSFileManager defaultManager] removeItemAtPath:_eventsDataPath error:NULL];
}
}
}
SAFE_ARC_RELEASE(_eventsDataPath);

// try to restore previous session
long long previousSessionId = [self previousSessionId];
if (previousSessionId >= 0) {
_sessionId = previousSessionId;
}

[self initializeDeviceId];

[_backgroundQueue setSuspended:NO];
}];

// CLLocationManager must be created on the main thread
Expand All @@ -317,8 +276,6 @@ - (id)initWithInstanceName:(NSString*) instanceName
SEL setDelegate = NSSelectorFromString(@"setDelegate:");
[_locationManager performSelector:setDelegate withObject:_locationManagerDelegate];
});

[self addObservers];
}
return self;
}
Expand Down Expand Up @@ -411,12 +368,12 @@ - (void) dealloc {

// Release instance variables
SAFE_ARC_RELEASE(_deviceInfo);
SAFE_ARC_RELEASE(_initializerQueue);
SAFE_ARC_RELEASE(_lastKnownLocation);
SAFE_ARC_RELEASE(_locationManager);
SAFE_ARC_RELEASE(_locationManagerDelegate);
SAFE_ARC_RELEASE(_propertyList);
SAFE_ARC_RELEASE(_propertyListPath);
SAFE_ARC_RELEASE(_eventsDataPath);
SAFE_ARC_RELEASE(_dbHelper);
SAFE_ARC_RELEASE(_instanceName);

Expand Down Expand Up @@ -464,15 +421,51 @@ - (void)initializeApiKey:(NSString*) apiKey userId:(NSString*) userId setUserId:
SAFE_ARC_RETAIN(apiKey);
SAFE_ARC_RELEASE(_apiKey);
_apiKey = apiKey;
_dbHelper = SAFE_ARC_RETAIN([AMPDatabaseHelper getDatabaseHelper:_instanceName]);

[self runOnBackgroundQueue:^{
// update database if necessary
int oldDBVersion = 1;
NSNumber *oldDBVersionSaved = [_propertyList objectForKey:DATABASE_VERSION];
if (oldDBVersionSaved != nil) {
oldDBVersion = [oldDBVersionSaved intValue];
}

// update the database
if (oldDBVersion < kAMPDBVersion) {
if ([self.dbHelper upgrade:oldDBVersion newVersion:kAMPDBVersion]) {
[_propertyList setObject:[NSNumber numberWithInt:kAMPDBVersion] forKey:DATABASE_VERSION];
[self savePropertyList];
}
}

// only on default instance, migrate all of old _eventsData object to database store if database just created
if ([_instanceName isEqualToString:kAMPDefaultInstance] && oldDBVersion < kAMPDBFirstVersion) {
if ([self migrateEventsDataToDB]) {
// delete events data so don't need to migrate next time
if ([[NSFileManager defaultManager] fileExistsAtPath:_eventsDataPath]) {
[[NSFileManager defaultManager] removeItemAtPath:_eventsDataPath error:NULL];
}
}
}

// try to restore previous session
long long previousSessionId = [self previousSessionId];
if (previousSessionId >= 0) {
_sessionId = previousSessionId;
}
[self initializeDeviceId];

if (setUserId) {
[self setUserId:userId];
} else {
_userId = SAFE_ARC_RETAIN([self.dbHelper getValue:USER_ID]);
}
}];

// now we can add observers after setting apikey since enterBackground saves timestamp to DB
[self addObservers];

UIApplication *app = [self getSharedApplication];
if (app != nil) {
UIApplicationState state = app.applicationState;
Expand Down Expand Up @@ -1218,6 +1211,11 @@ - (void)startSession

- (void)identify:(AMPIdentify *)identify
{
if (_apiKey == nil) {
AMPLITUDE_ERROR(@"ERROR: apiKey cannot be nil or empty, set apiKey with initializeApiKey: before calling identify");
return;
}

if (identify == nil || [identify.userPropertyOperations count] == 0) {
return;
}
Expand All @@ -1228,6 +1226,11 @@ - (void)identify:(AMPIdentify *)identify

- (void)setUserProperties:(NSDictionary*) userProperties
{
if (_apiKey == nil) {
AMPLITUDE_ERROR(@"ERROR: apiKey cannot be nil or empty, set apiKey with initializeApiKey: before calling setUserProperties");
return;
}

if (userProperties == nil || ![self isArgument:userProperties validType:[NSDictionary class] methodName:@"setUserProperties:"] || [userProperties count] == 0) {
return;
}
Expand All @@ -1252,6 +1255,11 @@ - (void)setUserProperties:(NSDictionary*) userProperties replace:(BOOL) replace

- (void)clearUserProperties
{
if (_apiKey == nil) {
AMPLITUDE_ERROR(@"ERROR: apiKey cannot be nil or empty, set apiKey with initializeApiKey: before calling clearUserProperties");
return;
}

AMPIdentify *identify = [[AMPIdentify identify] clearAll];
[self identify:identify];
}
Expand All @@ -1276,6 +1284,11 @@ - (void)setGroup:(NSString *)groupType groupName:(NSObject *)groupName

- (void)setUserId:(NSString*) userId
{
if (_apiKey == nil) {
AMPLITUDE_ERROR(@"ERROR: apiKey cannot be nil or empty, set apiKey with initializeApiKey: before calling setUserId");
return;
}

if (!(userId == nil || [self isArgument:userId validType:[NSString class] methodName:@"setUserId:"])) {
return;
}
Expand All @@ -1290,6 +1303,11 @@ - (void)setUserId:(NSString*) userId

- (void)setOptOut:(BOOL)enabled
{
if (_apiKey == nil) {
AMPLITUDE_ERROR(@"ERROR: apiKey cannot be nil or empty, set apiKey with initializeApiKey: before calling setOptOut");
return;
}

[self runOnBackgroundQueue:^{
NSNumber *value = [NSNumber numberWithBool:enabled];
(void) [self.dbHelper insertOrReplaceKeyLongValue:OPT_OUT value:value];
Expand All @@ -1300,7 +1318,7 @@ - (void)setOffline:(BOOL)offline
{
_offline = offline;

if (!_offline) {
if (!_offline && _apiKey != nil) {
[self uploadEvents];
}
}
Expand All @@ -1313,12 +1331,22 @@ - (void)setEventUploadMaxBatchSize:(int) eventUploadMaxBatchSize

- (BOOL)optOut
{
if (_apiKey == nil) {
AMPLITUDE_ERROR(@"ERROR: apiKey cannot be nil or empty, set apiKey with initializeApiKey: before toggling and fetching optOut");
return NO;
}
return [[self.dbHelper getLongValue:OPT_OUT] boolValue];
}

- (void)setDeviceId:(NSString*)deviceId
{
if (_apiKey == nil) {
AMPLITUDE_ERROR(@"ERROR: apiKey cannot be nil or empty, set apiKey with initializeApiKey: before calling setDeviceId");
return;
}

if (![self isValidDeviceId:deviceId]) {
AMPLITUDE_ERROR(@"ERROR: invalid deviceId '%@', skipping setDeviceId", deviceId);
return;
}

Expand Down
6 changes: 4 additions & 2 deletions AmplitudeTests/AmplitudeTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ - (void)testInstanceWithName {

- (void)testInitWithInstanceName {
Amplitude *a = [Amplitude instanceWithName:@"APP1"];
[a flushQueueWithQueue:a.initializerQueue];
[a flushQueue];
XCTAssertEqualObjects(a.instanceName, @"app1");
XCTAssertTrue([a.propertyListPath rangeOfString:@"com.amplitude.plist_app1"].location != NSNotFound);

Amplitude *b = [Amplitude instanceWithName:[kAMPDefaultInstance uppercaseString]];
[b flushQueueWithQueue:b.initializerQueue];
[b flushQueue];
XCTAssertEqualObjects(b.instanceName, kAMPDefaultInstance);
XCTAssertTrue([b.propertyListPath rangeOfString:@"com.amplitude.plist"].location != NSNotFound);
XCTAssertTrue([ b.propertyListPath rangeOfString:@"com.amplitude.plist_"].location == NSNotFound);
Expand Down Expand Up @@ -110,6 +110,8 @@ - (void)testSeparateInstancesLogEventsSeparate {
[newDBHelper2 resetDB:NO];

// setup existing database file, init default instance
[[Amplitude instance] initializeApiKey:apiKey];
[[Amplitude instance] flushQueue];
[oldDbHelper insertOrReplaceKeyLongValue:@"sequence_number" value:[NSNumber numberWithLongLong:1000]];
[oldDbHelper addEvent:@"{\"event_type\":\"oldEvent\"}"];
[oldDbHelper addIdentify:@"{\"event_type\":\"$identify\"}"];
Expand Down
2 changes: 1 addition & 1 deletion AmplitudeTests/BaseTestCase.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ - (void)setUp {
XCTAssertTrue([self.databaseHelper resetDB:NO]);

[self.amplitude init];
[self.amplitude flushQueue];
self.amplitude.sslPinningEnabled = NO;
}

- (void)tearDown {
// Ensure all background operations are done
[self.amplitude flushQueueWithQueue:self.amplitude.initializerQueue];
[self.amplitude flushQueue];
SAFE_ARC_RELEASE(_amplitude);
SAFE_ARC_RELEASE(_databaseHelper);
Expand Down
7 changes: 1 addition & 6 deletions AmplitudeTests/SessionTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ - (void)testSessionAutoStartedBackground {
id mockAmplitude = [OCMockObject partialMockForObject:self.amplitude];
[[mockAmplitude reject] enterForeground];
[mockAmplitude initializeApiKey:apiKey];
[mockAmplitude flushQueueWithQueue:[mockAmplitude initializerQueue]];
[mockAmplitude flushQueue];
[mockAmplitude verify];
XCTAssertEqual([mockAmplitude queuedEventCount], 0);
Expand All @@ -56,7 +55,6 @@ - (void)testSessionAutoStartedInactive {
id mockAmplitude = [OCMockObject partialMockForObject:self.amplitude];
[[mockAmplitude expect] enterForeground];
[mockAmplitude initializeApiKey:apiKey];
[mockAmplitude flushQueueWithQueue:[mockAmplitude initializerQueue]];
[mockAmplitude flushQueue];
[mockAmplitude verify];
XCTAssertEqual([mockAmplitude queuedEventCount], 0);
Expand All @@ -70,7 +68,6 @@ - (void)testSessionHandling {
[[[mockAmplitude expect] andReturnValue:OCMOCK_VALUE(date)] currentTime];

[mockAmplitude initializeApiKey:apiKey userId:nil];
[mockAmplitude flushQueueWithQueue:[mockAmplitude initializerQueue]];
[mockAmplitude flushQueue];
XCTAssertEqual([mockAmplitude queuedEventCount], 0);
XCTAssertEqual([mockAmplitude sessionId], 1000000);
Expand Down Expand Up @@ -140,7 +137,7 @@ - (void)testSessionHandling {

- (void)testEnterBackgroundDoesNotTrackEvent {
[self.amplitude initializeApiKey:apiKey userId:nil];
[self.amplitude flushQueueWithQueue:self.amplitude.initializerQueue];
[self.amplitude flushQueue];

NSNotificationCenter *center = [NSNotificationCenter defaultCenter];
[center postNotificationName:UIApplicationDidEnterBackgroundNotification object:nil userInfo:nil];
Expand All @@ -156,7 +153,6 @@ - (void)testTrackSessionEvents {
[mockAmplitude setTrackingSessionEvents:YES];

[mockAmplitude initializeApiKey:apiKey userId:nil];
[mockAmplitude flushQueueWithQueue:[mockAmplitude initializerQueue]];
[mockAmplitude flushQueue];

XCTAssertEqual([mockAmplitude queuedEventCount], 1);
Expand Down Expand Up @@ -192,7 +188,6 @@ - (void)testSessionEventsOn32BitDevices {
[mockAmplitude setTrackingSessionEvents:YES];

[mockAmplitude initializeApiKey:apiKey userId:nil];
[mockAmplitude flushQueueWithQueue:[mockAmplitude initializerQueue]];
[mockAmplitude flushQueue];

XCTAssertEqual([mockAmplitude queuedEventCount], 1);
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## Unreleased

* Migrate all database logic from `init` to `initializeApiKey`.
* Run `init` logic on `backgroundQueue`, removing need for separate `initializerQueue`.

### 3.8.3 (July 18, 2016)

* Fix overflow bug for long long values saved to Sqlite DB on 32-bit devices.
Expand Down