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

Reset RC per-instance userdefaults if config database was not found. #4896

Merged
merged 4 commits into from
Feb 14, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions FirebaseRemoteConfig/Sources/RCNConfigDBManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,7 @@ typedef void (^RCNDBLoadCompletion)(BOOL success,
/// Remove all the records from experiment table with given key.
/// @param key The key of experiment data belongs to, which are defined in RCNConfigDefines.h.
- (void)deleteExperimentTableForKey:(NSString *)key;

/// Returns true if this a new install of the Config database.
- (BOOL)isNewDatabase;
@end
7 changes: 7 additions & 0 deletions FirebaseRemoteConfig/Sources/RCNConfigDBManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#define RCNTableNameInternalMetadata "internal_metadata"
#define RCNTableNameExperiment "experiment"

static BOOL gIsNewDatabase;
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be made thread-safe?

Copy link
Contributor Author

@dmandar dmandar Feb 13, 2020

Choose a reason for hiding this comment

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

it is. The variable is used on the database serial queue.

/// SQLite file name in versions 0, 1 and 2.
static NSString *const RCNDatabaseName = @"RemoteConfig.sqlite3";
/// The application support sub-directory that the Remote Config database resides in.
Expand Down Expand Up @@ -77,6 +78,7 @@ static BOOL RemoteConfigCreateFilePathIfNotExist(NSString *filePath) {
}
NSFileManager *fileManager = [NSFileManager defaultManager];
if (![fileManager fileExistsAtPath:filePath]) {
gIsNewDatabase = YES;
NSError *error;
[fileManager createDirectoryAtPath:[filePath stringByDeletingLastPathComponent]
withIntermediateDirectories:YES
Expand Down Expand Up @@ -203,6 +205,7 @@ - (void)createOrOpenDatabase {
NSString *dbPath = [RCNConfigDBManager remoteConfigPathForDatabase];
FIRLogInfo(kFIRLoggerRemoteConfig, @"I-RCN000062", @"Loading database at path %@", dbPath);
const char *databasePath = dbPath.UTF8String;

// Create or open database path.
if (!RemoteConfigCreateFilePathIfNotExist(dbPath)) {
return;
Expand Down Expand Up @@ -1036,4 +1039,8 @@ - (BOOL)logErrorWithSQL:(const char *)SQL
return returnValue;
}

- (BOOL)isNewDatabase {
return gIsNewDatabase;
}

@end
8 changes: 8 additions & 0 deletions FirebaseRemoteConfig/Sources/RCNConfigSettings.m
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,14 @@ - (instancetype)initWithDatabaseManager:(RCNConfigDBManager *)manager
_userDefaultsManager = [[RCNUserDefaultsManager alloc] initWithAppName:appName
bundleID:_bundleIdentifier
namespace:_FIRNamespace];

// Check if the config database is new. If so, clear the configs saved in userDefaults.
if ([_DBManager isNewDatabase]) {
Copy link
Member

Choose a reason for hiding this comment

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

How can this test ever fail? I don't see gIsNewDatabase ever set to NO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test case setup method above creates a new test database. We just want to verify that this flag is set to true whenever an existing database is not found.

FIRLogNotice(kFIRLoggerRemoteConfig, @"I-RCN000072",
@"New config database created. Resetting user defaults.");
[_userDefaultsManager resetUserDefaults];
}

_isFetchInProgress = NO;
}
return self;
Expand Down
2 changes: 2 additions & 0 deletions FirebaseRemoteConfig/Sources/RCNUserDefaultsManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ NS_ASSUME_NONNULL_BEGIN
__attribute__((unavailable("Use `initWithAppName:bundleID:namespace:` instead.")));
// NOLINTEND

/// Delete all saved userdefaults for this instance.
- (void)resetUserDefaults;
@end

NS_ASSUME_NONNULL_END
18 changes: 18 additions & 0 deletions FirebaseRemoteConfig/Sources/RCNUserDefaultsManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,11 @@ - (void)setCurrentThrottlingRetryIntervalSeconds:(NSTimeInterval)throttlingRetry
forKey:kRCNUserDefaultsKeyNamecurrentThrottlingRetryInterval];
}

#pragma mark Public methods.
- (void)resetUserDefaults {
[self resetInstanceUserDefaults];
}

#pragma mark Private methods.

// There is a nested hierarchy for the userdefaults as follows:
Expand Down Expand Up @@ -212,4 +217,17 @@ - (void)setInstanceUserDefaultsValue:(NSObject *)value forKey:(NSString *)key {
}
}

// Delete any existing userdefaults for this instance.
- (void)resetInstanceUserDefaults {
@synchronized(_userDefaults) {
NSMutableDictionary *appUserDefaults = [[self appUserDefaults] mutableCopy];
NSMutableDictionary *appNamespaceUserDefaults = [[self instanceUserDefaults] mutableCopy];
[appNamespaceUserDefaults removeAllObjects];
[appUserDefaults setObject:appNamespaceUserDefaults forKey:_firebaseNamespace];
[_userDefaults setObject:appUserDefaults forKey:_firebaseAppName];
// We need to synchronize to have this value updated for the extension.
[_userDefaults synchronize];
}
}

@end
3 changes: 2 additions & 1 deletion FirebaseRemoteConfig/Tests/Unit/RCNRemoteConfigTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ - (void)setUp {
namespace:fullyQualifiedNamespace
options:currentOptions]);

OCMStub([_configFetch[i] fetchConfigWithExpirationDuration:43200 completionHandler:OCMOCK_ANY])
OCMStub([_configFetch[i] fetchConfigWithExpirationDuration:0 completionHandler:OCMOCK_ANY])
.ignoringNonObjectArgs()
.andDo(^(NSInvocation *invocation) {
void (^handler)(FIRRemoteConfigFetchStatus status, NSError *_Nullable error) = nil;
[invocation getArgument:&handler atIndex:3];
Expand Down
10 changes: 10 additions & 0 deletions FirebaseRemoteConfig/Tests/Unit/RCNUserDefaultsManagerTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -170,4 +170,14 @@ - (void)testUserDefaultsForMultipleNamespaces {
RCNUserDefaultsSampleTimeStamp - 2.0);
}

- (void)testUserDefaultsReset {
RCNUserDefaultsManager* manager =
[[RCNUserDefaultsManager alloc] initWithAppName:@"TESTING"
bundleID:[NSBundle mainBundle].bundleIdentifier
namespace:@"testNamespace1"];
[manager setLastETag:@"testETag"];
[manager resetUserDefaults];
XCTAssertNil([manager lastETag]);
}

@end