From dd75cc273fa4ee0ba478963384b3f2fc1fa0b304 Mon Sep 17 00:00:00 2001 From: Daniel Jih Date: Tue, 6 Oct 2015 16:30:13 -0700 Subject: [PATCH 1/2] fix database path --- Amplitude/AMPConstants.m | 4 ++-- Amplitude/AMPDatabaseHelper.m | 7 ++++--- Amplitude/Amplitude.m | 4 ++-- AmplitudeTests/AMPDatabaseHelperTests.m | 23 +++++++++++++++++++++-- 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/Amplitude/AMPConstants.m b/Amplitude/AMPConstants.m index ec0d93be..6c0fb43d 100644 --- a/Amplitude/AMPConstants.m +++ b/Amplitude/AMPConstants.m @@ -9,8 +9,8 @@ NSString *const kAMPEventLogDomain = @"api.amplitude.com"; NSString *const kAMPEventLogUrl = @"https://api.amplitude.com/"; const int kAMPApiVersion = 2; -const int kAMPDBVersion = 1; -const int kAMPDBFirstVersion = 1; // to detect if DB exists yet +const int kAMPDBVersion = 2; +const int kAMPDBFirstVersion = 2; // to detect if DB exists yet const int kAMPEventUploadThreshold = 30; const int kAMPEventUploadMaxBatchSize = 100; const int kAMPEventMaxCount = 1000; diff --git a/Amplitude/AMPDatabaseHelper.m b/Amplitude/AMPDatabaseHelper.m index 996fbb40..bf584f75 100644 --- a/Amplitude/AMPDatabaseHelper.m +++ b/Amplitude/AMPDatabaseHelper.m @@ -65,7 +65,7 @@ - (id) init if (self = [super init]) { NSString *databaseDirectory = [NSSearchPathForDirectoriesInDomains(NSLibraryDirectory, NSUserDomainMask, YES) objectAtIndex: 0]; - _databasePath = SAFE_ARC_RETAIN([databaseDirectory stringByAppendingString:@"com.amplitude.database"]); + _databasePath = SAFE_ARC_RETAIN([databaseDirectory stringByAppendingPathComponent:@"com.amplitude.database"]); _dbQueue = SAFE_ARC_RETAIN([FMDatabaseQueue databaseQueueWithPath:_databasePath flags:(SQLITE_OPEN_READWRITE|SQLITE_OPEN_CREATE)]); if (![[NSFileManager defaultManager] fileExistsAtPath:_databasePath]) { [self createTables]; @@ -122,7 +122,8 @@ - (void)upgrade:(int) oldVersion newVersion:(int) newVersion } switch (oldVersion) { - case 0: { + case 0: + case 1: { NSString *createEventsTable = [NSString stringWithFormat:CREATE_EVENT_TABLE, EVENT_TABLE_NAME, ID_FIELD, EVENT_FIELD]; [db executeUpdate:createEventsTable]; @@ -132,7 +133,7 @@ - (void)upgrade:(int) oldVersion newVersion:(int) newVersion NSString *createLongStoreTable = [NSString stringWithFormat:CREATE_LONG_STORE_TABLE, LONG_STORE_TABLE_NAME, KEY_FIELD, VALUE_FIELD]; [db executeUpdate:createLongStoreTable]; - if (newVersion <= 1) break; + if (newVersion <= 2) break; } default: success = NO; diff --git a/Amplitude/Amplitude.m b/Amplitude/Amplitude.m index 9b7dda95..937a40b8 100644 --- a/Amplitude/Amplitude.m +++ b/Amplitude/Amplitude.m @@ -209,7 +209,7 @@ - (id)init _propertyList = SAFE_ARC_RETAIN([self deserializePList:_propertyListPath]); if (!_propertyList) { _propertyList = SAFE_ARC_RETAIN([NSMutableDictionary dictionary]); - [_propertyList setObject:[NSNumber numberWithInt:0] forKey:DATABASE_VERSION]; + [_propertyList setObject:[NSNumber numberWithInt:1] forKey:DATABASE_VERSION]; BOOL success = [self savePropertyList]; if (!success) { NSLog(@"ERROR: Unable to save propertyList to file on initialization"); @@ -219,7 +219,7 @@ - (id)init } // update database if necessary - int oldDBVersion = 0; + int oldDBVersion = 1; NSNumber *oldDBVersionSaved = [_propertyList objectForKey:DATABASE_VERSION]; if (oldDBVersionSaved != nil) { oldDBVersion = [oldDBVersionSaved intValue]; diff --git a/AmplitudeTests/AMPDatabaseHelperTests.m b/AmplitudeTests/AMPDatabaseHelperTests.m index e30f5912..9f36839e 100644 --- a/AmplitudeTests/AMPDatabaseHelperTests.m +++ b/AmplitudeTests/AMPDatabaseHelperTests.m @@ -135,7 +135,7 @@ - (void)testGetNthEventId { XCTAssertEqual(-1, [self.databaseHelper getNthEventId:1]); } -- (void)testUpgradeFromVersion0ToVersion1{ +- (void)testUpgradeFromVersion1ToVersion2{ // inserts will fail since no tables exist [self.databaseHelper dropTables]; XCTAssertFalse([self.databaseHelper addEvent:@"test_event"]); @@ -148,7 +148,26 @@ - (void)testUpgradeFromVersion0ToVersion1{ // after upgrade, can insert into event, store, long_store [self.databaseHelper dropTables]; - [self.databaseHelper upgrade:0 newVersion:1]; + [self.databaseHelper upgrade:1 newVersion:2]; + XCTAssertTrue([self.databaseHelper addEvent:@"test"]); + XCTAssertTrue([self.databaseHelper insertOrReplaceKeyValue:@"key" value:@"value"]); + XCTAssertTrue([self.databaseHelper insertOrReplaceKeyLongValue:@"key" value:[NSNumber numberWithLongLong:0LL]]); +} + +- (void)testUpgradeFromVersion0ToVersion2{ + // inserts will fail since no tables exist + [self.databaseHelper dropTables]; + XCTAssertFalse([self.databaseHelper addEvent:@"test_event"]); + + [self.databaseHelper dropTables]; + XCTAssertFalse([self.databaseHelper insertOrReplaceKeyValue:@"test_key" value:@"test_value"]); + + [self.databaseHelper dropTables]; + XCTAssertFalse([self.databaseHelper insertOrReplaceKeyLongValue:@"test_key" value:[NSNumber numberWithInt:0]]); + + // after upgrade, can insert into event, store, long_store + [self.databaseHelper dropTables]; + [self.databaseHelper upgrade:0 newVersion:2]; XCTAssertTrue([self.databaseHelper addEvent:@"test"]); XCTAssertTrue([self.databaseHelper insertOrReplaceKeyValue:@"key" value:@"value"]); XCTAssertTrue([self.databaseHelper insertOrReplaceKeyLongValue:@"key" value:[NSNumber numberWithLongLong:0LL]]); From 145da67f20188f073103b6d0c7017945227729c4 Mon Sep 17 00:00:00 2001 From: Daniel Jih Date: Tue, 6 Oct 2015 17:09:51 -0700 Subject: [PATCH 2/2] added success checks before updating db number and deleting events archive --- Amplitude/AMPDatabaseHelper.h | 2 +- Amplitude/AMPDatabaseHelper.m | 15 +++++++---- Amplitude/Amplitude.m | 36 ++++++++++++++----------- AmplitudeTests/AMPDatabaseHelperTests.m | 12 +++++++-- 4 files changed, 41 insertions(+), 24 deletions(-) diff --git a/Amplitude/AMPDatabaseHelper.h b/Amplitude/AMPDatabaseHelper.h index 4374b4c7..e4ba4c78 100644 --- a/Amplitude/AMPDatabaseHelper.h +++ b/Amplitude/AMPDatabaseHelper.h @@ -11,7 +11,7 @@ + (AMPDatabaseHelper*)getDatabaseHelper; - (BOOL)createTables; - (BOOL)dropTables; -- (void)upgrade:(int) oldVersion newVersion:(int) newVersion; +- (BOOL)upgrade:(int) oldVersion newVersion:(int) newVersion; - (BOOL)resetDB:(BOOL) deleteDB; - (BOOL)deleteDB; diff --git a/Amplitude/AMPDatabaseHelper.m b/Amplitude/AMPDatabaseHelper.m index bf584f75..8b39f86b 100644 --- a/Amplitude/AMPDatabaseHelper.m +++ b/Amplitude/AMPDatabaseHelper.m @@ -109,7 +109,7 @@ - (BOOL)createTables return success; } -- (void)upgrade:(int) oldVersion newVersion:(int) newVersion +- (BOOL)upgrade:(int) oldVersion newVersion:(int) newVersion { __block BOOL success = YES; @@ -125,16 +125,19 @@ - (void)upgrade:(int) oldVersion newVersion:(int) newVersion case 0: case 1: { NSString *createEventsTable = [NSString stringWithFormat:CREATE_EVENT_TABLE, EVENT_TABLE_NAME, ID_FIELD, EVENT_FIELD]; - [db executeUpdate:createEventsTable]; + success &= [db executeUpdate:createEventsTable]; NSString *createStoreTable = [NSString stringWithFormat:CREATE_STORE_TABLE, STORE_TABLE_NAME, KEY_FIELD, VALUE_FIELD]; - [db executeUpdate:createStoreTable]; + success &= [db executeUpdate:createStoreTable]; NSString *createLongStoreTable = [NSString stringWithFormat:CREATE_LONG_STORE_TABLE, LONG_STORE_TABLE_NAME, KEY_FIELD, VALUE_FIELD]; - [db executeUpdate:createLongStoreTable]; + success &= [db executeUpdate:createLongStoreTable]; if (newVersion <= 2) break; } + case 2: { + if (newVersion <= 3) break; + } default: success = NO; } @@ -144,8 +147,10 @@ - (void)upgrade:(int) oldVersion newVersion:(int) newVersion if (!success) { NSLog(@"upgrade with unknown oldVersion %d", oldVersion); - [self resetDB:NO]; + return [self resetDB:NO]; } + + return success; } - (BOOL)dropTables diff --git a/Amplitude/Amplitude.m b/Amplitude/Amplitude.m index 937a40b8..711a9efb 100644 --- a/Amplitude/Amplitude.m +++ b/Amplitude/Amplitude.m @@ -227,14 +227,20 @@ - (id)init // update the database if (oldDBVersion < kAMPDBVersion) { - [[AMPDatabaseHelper getDatabaseHelper] upgrade:oldDBVersion newVersion:kAMPDBVersion]; - [_propertyList setObject:[NSNumber numberWithInt:kAMPDBVersion] forKey:DATABASE_VERSION]; - [self savePropertyList]; + if ([[AMPDatabaseHelper getDatabaseHelper] upgrade:oldDBVersion newVersion:kAMPDBVersion]) { + [_propertyList setObject:[NSNumber numberWithInt:kAMPDBVersion] forKey:DATABASE_VERSION]; + [self savePropertyList]; + } } // migrate all of old _eventsData object to database store if database just created if (oldDBVersion < kAMPDBFirstVersion) { - [self migrateEventsDataToDB]; + 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); @@ -263,50 +269,48 @@ - (id)init return self; }; -- (void) migrateEventsDataToDB +- (BOOL) migrateEventsDataToDB { NSDictionary *eventsData = [self unarchive:_eventsDataPath]; if (eventsData == nil) { - return; + return NO; } AMPDatabaseHelper *dbHelper = [AMPDatabaseHelper getDatabaseHelper]; + BOOL success = YES; // migrate events NSArray *events = [eventsData objectForKey:EVENTS]; for (id event in events) { NSData *jsonData = [NSJSONSerialization dataWithJSONObject:event options:0 error:NULL]; NSString *jsonString = [[NSString alloc] initWithData:jsonData encoding:NSUTF8StringEncoding]; - [dbHelper addEvent:jsonString]; + success &= [dbHelper addEvent:jsonString]; SAFE_ARC_RELEASE(jsonString); } // migrate remaining properties NSString *userId = [eventsData objectForKey:USER_ID]; if (userId != nil) { - [dbHelper insertOrReplaceKeyValue:USER_ID value:userId]; + success &= [dbHelper insertOrReplaceKeyValue:USER_ID value:userId]; } NSNumber *optOut = [eventsData objectForKey:OPT_OUT]; if (optOut != nil) { - [dbHelper insertOrReplaceKeyLongValue:OPT_OUT value:optOut]; + success &= [dbHelper insertOrReplaceKeyLongValue:OPT_OUT value:optOut]; } NSString *deviceId = [eventsData objectForKey:DEVICE_ID]; if (deviceId != nil) { - [dbHelper insertOrReplaceKeyValue:DEVICE_ID value:deviceId]; + success &= [dbHelper insertOrReplaceKeyValue:DEVICE_ID value:deviceId]; } NSNumber *previousSessionId = [eventsData objectForKey:PREVIOUS_SESSION_ID]; if (previousSessionId != nil) { - [dbHelper insertOrReplaceKeyLongValue:PREVIOUS_SESSION_ID value:previousSessionId]; + success &= [dbHelper insertOrReplaceKeyLongValue:PREVIOUS_SESSION_ID value:previousSessionId]; } NSNumber *previousSessionTime = [eventsData objectForKey:PREVIOUS_SESSION_TIME]; if (previousSessionTime != nil) { - [dbHelper insertOrReplaceKeyLongValue:PREVIOUS_SESSION_TIME value:previousSessionTime]; + success &= [dbHelper insertOrReplaceKeyLongValue:PREVIOUS_SESSION_TIME value:previousSessionTime]; } - // delete events data so don't need to migrate next time - if ([[NSFileManager defaultManager] fileExistsAtPath:_eventsDataPath]) { - [[NSFileManager defaultManager] removeItemAtPath:_eventsDataPath error:NULL]; - } + return success; } - (void) addObservers diff --git a/AmplitudeTests/AMPDatabaseHelperTests.m b/AmplitudeTests/AMPDatabaseHelperTests.m index 9f36839e..676cc790 100644 --- a/AmplitudeTests/AMPDatabaseHelperTests.m +++ b/AmplitudeTests/AMPDatabaseHelperTests.m @@ -148,7 +148,7 @@ - (void)testUpgradeFromVersion1ToVersion2{ // after upgrade, can insert into event, store, long_store [self.databaseHelper dropTables]; - [self.databaseHelper upgrade:1 newVersion:2]; + XCTAssertTrue([self.databaseHelper upgrade:1 newVersion:2]); XCTAssertTrue([self.databaseHelper addEvent:@"test"]); XCTAssertTrue([self.databaseHelper insertOrReplaceKeyValue:@"key" value:@"value"]); XCTAssertTrue([self.databaseHelper insertOrReplaceKeyLongValue:@"key" value:[NSNumber numberWithLongLong:0LL]]); @@ -167,7 +167,15 @@ - (void)testUpgradeFromVersion0ToVersion2{ // after upgrade, can insert into event, store, long_store [self.databaseHelper dropTables]; - [self.databaseHelper upgrade:0 newVersion:2]; + XCTAssertTrue([self.databaseHelper upgrade:0 newVersion:2]); + XCTAssertTrue([self.databaseHelper addEvent:@"test"]); + XCTAssertTrue([self.databaseHelper insertOrReplaceKeyValue:@"key" value:@"value"]); + XCTAssertTrue([self.databaseHelper insertOrReplaceKeyLongValue:@"key" value:[NSNumber numberWithLongLong:0LL]]); +} + +- (void)testUpgradeFromVersion2ToVersion2{ + // upgrade does nothing, can insert into event, store, long_store + XCTAssertTrue([self.databaseHelper upgrade:2 newVersion:2]); XCTAssertTrue([self.databaseHelper addEvent:@"test"]); XCTAssertTrue([self.databaseHelper insertOrReplaceKeyValue:@"key" value:@"value"]); XCTAssertTrue([self.databaseHelper insertOrReplaceKeyLongValue:@"key" value:[NSNumber numberWithLongLong:0LL]]);