Skip to content

Commit

Permalink
this runs through the background and removes any experiments that are… (
Browse files Browse the repository at this point in the history
#302)

* this runs through the background and removes any experiments that are not in a fresh datafile

* cleanup naming and add try catch

* add unit tests for remove invalid experiments from user profile service

* add more tests for removeInvalidExperimentsForAllUsers and cleanUserProfileService. fix typos

* correct typos and remove extra blank lines
  • Loading branch information
thomaszurkan-optimizely authored Sep 7, 2018
1 parent d962ac5 commit 12bf48a
Show file tree
Hide file tree
Showing 11 changed files with 136 additions and 4 deletions.
44 changes: 43 additions & 1 deletion OptimizelySDKShared/OptimizelySDKShared/OPTLYManagerBase.m
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
#import "OPTLYLogger.h"
#import "OPTLYLoggerMessages.h"
#else
#import <OptimizelySDKCore/OPTLYExperiment.h>
#import <OptimizelySDKCore/OPTLYProjectConfig.h>
#import <OptimizelySDKCore/OPTLYErrorHandler.h>
#import <OptimizelySDKCore/OPTLYEventDispatcherBasic.h>
#import <OptimizelySDKCore/OPTLYLogger.h>
Expand Down Expand Up @@ -67,6 +69,29 @@ @interface OPTLYManagerBase()

@implementation OPTLYManagerBase

- (void)cleanUserProfileService:(NSArray<OPTLYExperiment> *)experiments {
if (experiments == nil) return;

if (_userProfileService != nil && [(NSObject *)_userProfileService respondsToSelector:@selector(removeInvalidExperimentsForAllUsers:)]) {
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
if (self.userProfileService != nil) {
NSMutableArray<NSString*> *ids = [[NSMutableArray alloc] initWithCapacity:experiments.count];
for (int i = 0; i < experiments.count; i++) {
NSString *exKey = ((OPTLYExperiment *)experiments[i]).experimentId;
[ids addObject:exKey];
}

@try {
[(NSObject *)self.userProfileService performSelector:@selector(removeInvalidExperimentsForAllUsers:) withObject:ids];
}
@catch(NSException *e) {
[self.logger logMessage:@"Error cleaning up user profile service" withLevel:OptimizelyLogLevelError];
}
}
});
}
}

#pragma mark - Constructors

- (instancetype)init {
Expand Down Expand Up @@ -102,14 +127,22 @@ - (OPTLYClient *)initialize {

// attempt to get the cached datafile
NSData *data = [self.datafileManager getSavedDatafile:nil];

BOOL cleanUserProfile = NO;
// fall back to the datafile provided by the manager builder if we can't get the saved datafile
if (data == nil) {
data = self.datafile;
[self.logger logMessage:OPTLYLoggerMessagesManagerBundledDataLoaded withLevel:OptimizelyLogLevelInfo];
}
else {
// cleanup user profile service in background
cleanUserProfile = YES;
}

OPTLYClient *client = [self initializeWithDatafile:data];

if (cleanUserProfile) {
[self cleanUserProfileService:client.optimizely.config.experiments];
}

return client;
}
Expand Down Expand Up @@ -138,13 +171,22 @@ - (void)initializeWithCallback:(void (^)(NSError * _Nullable, OPTLYClient * _Nul
}

// fall back to the datafile provided by the manager builder if we can't get the saved datafile
BOOL cleanUserProfileService = NO;
if (data == nil) {
data = self.datafile;
[self.logger logMessage:OPTLYLoggerMessagesManagerBundledDataLoaded withLevel:OptimizelyLogLevelInfo];
}
else {
// update user profile service in the background.
cleanUserProfileService = YES;
}

OPTLYClient *client = [self initializeWithDatafile:data];

if (cleanUserProfileService) {
[self cleanUserProfileService:client.optimizely.config.experiments];
}

if (callback) {
callback(error, client);
}
Expand Down
44 changes: 44 additions & 0 deletions OptimizelySDKShared/OptimizelySDKSharedTests/OPTLYManagerTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@
static NSString * const kClientEngine = @"tvos-sdk";
#endif

@interface OPTLYManagerBase(Tests)
- (void)cleanUserProfileService:(NSArray<OPTLYExperiment> *)experiments;
@end

@interface OPTLYManagerTest : XCTestCase
@property (nonatomic, strong) NSData *defaultDatafile;
@property (nonatomic, strong) NSData *alternateDatafile;
Expand Down Expand Up @@ -74,13 +78,21 @@ - (void)testInitializeWithCachedDatafile
builder.projectId = kAlternateProjectId;
}]];

id managerMock = OCMPartialMock(manager);

/* run code under test */

// save the datafile
[manager.datafileManager saveDatafile:self.alternateDatafile];
OPTLYClient *client = [manager initialize];

[self isClientValid:client
datafile:self.alternateDatafile];
[self checkConfigIsUsingAlternativeDatafile:client.optimizely.config];

NSArray<OPTLYExperiment> *experiments = [[[manager getOptimizely] optimizely].config experiments];
OCMVerify([managerMock cleanUserProfileService:experiments]);

}

// If no datafile is cached, the client should be initialized with
Expand All @@ -94,11 +106,16 @@ - (void)testInitializeNoCachedDatafile
}]];
id partialMockManager = OCMPartialMock(manager);

NSArray<OPTLYExperiment> *experiments = [[NSArray<OPTLYExperiment> alloc] init];

[[partialMockManager reject] cleanUserProfileService:experiments];

OPTLYClient *client = [partialMockManager initialize];

[self isClientValid:client
datafile:self.alternateDatafile];
[self checkConfigIsUsingAlternativeDatafile:client.optimizely.config];

}

// If a datafile is cached, but there is an error loading the datafile,
Expand Down Expand Up @@ -414,6 +431,33 @@ - (void)testInitializeWithCallbackDownloadErrorNoDatafile
[self waitForExpectationsWithTimeout:2 handler:nil];
}

#pragma mark - testCleanUserProfileService

- (void)testCleanUserProfileService
{
// need to mock the manager bundled datafile load to read from the test bundle
OPTLYManagerBasic *manager = [[OPTLYManagerBasic alloc] initWithBuilder:[OPTLYManagerBuilder builderWithBlock:^(OPTLYManagerBuilder * _Nullable builder) {
builder.projectId = kProjectId;
builder.datafile = self.alternateDatafile;
}]];
id partialMockManager = OCMPartialMock(manager);

NSArray<OPTLYExperiment> *experiments = [[NSArray<OPTLYExperiment> alloc] init];

OPTLYClient *client = [partialMockManager initialize];

[self isClientValid:client
datafile:self.alternateDatafile];
[self checkConfigIsUsingAlternativeDatafile:client.optimizely.config];

// pass in nil
[manager cleanUserProfileService:nil];
// pass in empty
[manager cleanUserProfileService:experiments];
// pass in experiements
[manager cleanUserProfileService:client.optimizely.config.experiments];
}

#pragma mark - isValidKeyString

-(void)testIsValidKeyString {
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ extern NSString *const OPTLYLoggerMessagesDatafileFetchIntervalInvalid;

// ---- Datafile Versioning ----
// warning
extern NSString *const OPTLYLoggerMessagesInvalidDatafileVersion;
extern NSString *const OPTLYLoggerMessagesDatafileVersion;

// ---- Event Builder ----
// debug
Expand Down
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ extern NSString *const OPTLYLoggerMessagesDatafileFetchIntervalInvalid;

// ---- Datafile Versioning ----
// warning
extern NSString *const OPTLYLoggerMessagesInvalidDatafileVersion;
extern NSString *const OPTLYLoggerMessagesDatafileVersion;

// ---- Event Builder ----
// debug
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,12 @@ __attribute((deprecated("Use OPTLYManager initWithBuilder method instead.")));
**/
- (void)removeAllUserExperimentRecords;

/**
* Clean up and remove experiments that are not in the valid experiment list passed in.
* This is called when initialized from a remote datafile to ensure that the UserProfileService
* does not grow indefinitely.
* @param validExperimentIds An array of valid experiment ids. If default user profile contains
* experiments not in this list, they are removed from user profile service.
**/
- (void)removeInvalidExperimentsForAllUsers:(NSArray<NSString *> *)validExperimentIds;
@end
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,26 @@ - (void)removeUserExperimentRecordsForUserId:(nonnull NSString *)userId {
- (void)removeAllUserExperimentRecords {
[self.dataStore removeAllUserData];
}

- (void)removeInvalidExperimentsForAllUsers:(NSArray<NSString *> *)validExperimentIds {

NSMutableDictionary*userProfileService = [[self.dataStore getUserDataForType:OPTLYDataStoreDataTypeUserProfileService] mutableCopy];

for (NSString *key in userProfileService.allKeys) {
NSMutableDictionary *userProfileDict = [userProfileService[key] mutableCopy];
NSDictionary * bucketMap = userProfileDict[@"experiment_bucket_map"];
NSMutableDictionary *newBucketMap = [bucketMap mutableCopy];
for (NSString *exId in bucketMap.allKeys) {
if (![validExperimentIds containsObject:exId]) {
[newBucketMap removeObjectForKey:exId];
}
}
userProfileDict[@"experiment_bucket_map"] = newBucketMap;
userProfileService[key] = userProfileDict;
}

[self.dataStore saveUserData:userProfileService type:OPTLYDataStoreDataTypeUserProfileService];
}

@end

Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,25 @@ - (void)testWhitelistingOverridesUserProfileAndStoresNothing {
XCTAssertEqualObjects(variation.variationId, kWhitelistingWhitelistedVariationId);
XCTAssertEqualObjects(variation.variationKey, kWhitelistingWhiteListedVariationKey);
}


- (void)testRemoveInvalidExperiments
{
if (self.userProfileService ) {
[self.userProfileService performSelector:@selector(removeInvalidExperimentsForAllUsers:) withObject:@[]];
[self.userProfileService performSelector:@selector(removeInvalidExperimentsForAllUsers:) withObject:nil];
[self.userProfileService performSelector:@selector(removeInvalidExperimentsForAllUsers:) withObject:@[kExperimentId3a]];
}
NSDictionary *userProfile1 = [self.userProfileService lookup:kUserId1];
XCTAssertNil(userProfile1[kExperimentId3a], @"User profile experiment entry should be removed");

NSDictionary *userProfile2 = [self.userProfileService lookup:kUserId2];
XCTAssertNil(userProfile2[kExperimentId3a], @"User profile experiment entry should be removed");

NSDictionary *userProfile3 = [self.userProfileService lookup:kUserId3];
XCTAssertNil(userProfile3[kExperimentId3a], @"User profile experiment entry should be removed");

}

- (void)testLegacyUserProfileMigration
{
[self saveUserId:kUserId1 experimentId:kExperimentId1 variationId:kVariationId1];
Expand Down

0 comments on commit 12bf48a

Please sign in to comment.