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

Events changes #721

Merged
merged 25 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ apply plugin: 'kotlin-android'
apply from: 'spec.gradle'

ext {
splitVersion = '5.1.0-alpha.1'
splitVersion = '5.1.0-alpha.2'
}

android {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ public void testNoReadyFromCache() throws Exception {
latch.await(40, TimeUnit.SECONDS);

Assert.assertTrue(client.isReady());
Assert.assertFalse(readyFromCacheTask.isOnPostExecutionCalled);
Assert.assertTrue(readyFromCacheTask.isOnPostExecutionCalled);
Assert.assertTrue(readyTask.isOnPostExecutionCalled);
Assert.assertFalse(readyTimeOutTask.isOnPostExecutionCalled);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public void test() throws Exception {
Assert.fail("Impressions request timeout");
}

Assert.assertFalse(readyFromCacheTask.isOnPostExecutionCalled);
Assert.assertTrue(readyFromCacheTask.isOnPostExecutionCalled);
Assert.assertEquals("on_s1", treatments.get(0));
Assert.assertEquals("on_s1", treatments.get(1));
Assert.assertEquals("on_s1", treatments.get(2));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public void setUp() {

@Test
public void expirationPeriodIsUsed() throws InterruptedException {
test(getTimestampDaysAgo(1), RolloutCacheConfiguration.builder().expiration(1));
test(getTimestampDaysAgo(1), RolloutCacheConfiguration.builder().expirationDays(1));
}

@Test
Expand All @@ -75,7 +75,7 @@ public void clearOnInitClearsCacheOnStartup() throws InterruptedException {

@Test
public void repeatedInitWithClearOnInitSetToTrueDoesNotClearIfMinDaysHasNotElapsed() throws InterruptedException {
// Preload DB with update timestamp of 1 day ago
// Preload DB with update timestamp of now
long oldTimestamp = System.currentTimeMillis();
preloadDb(oldTimestamp, 0L, 8000L);

Expand All @@ -102,6 +102,7 @@ public void repeatedInitWithClearOnInitSetToTrueDoesNotClearIfMinDaysHasNotElaps
factory.client().on(SplitEvent.SDK_READY, TestingHelper.testTask(readyLatch));
boolean readyAwait = readyLatch.await(10, TimeUnit.SECONDS);

// Destroy factory
factory.destroy();
mRequestCountdownLatch = new CountDownLatch(1);

Expand Down Expand Up @@ -141,6 +142,23 @@ public void repeatedInitWithClearOnInitSetToTrueDoesNotClearIfMinDaysHasNotElaps
.getByName("rolloutCacheLastClearTimestamp").getLongValue());
}

@Test
public void sdkReadyFromCacheIsEmittedOnFreshInit() throws InterruptedException {
SplitFactory splitFactory = getSplitFactory(RolloutCacheConfiguration.builder().build());

CountDownLatch latch1 = new CountDownLatch(1);
CountDownLatch latch2 = new CountDownLatch(1);
splitFactory.client().on(SplitEvent.SDK_READY_FROM_CACHE, TestingHelper.testTask(latch1));
splitFactory.client("two").on(SplitEvent.SDK_READY_FROM_CACHE, TestingHelper.testTask(latch2));
mRequestCountdownLatch.countDown();

boolean await1 = latch1.await(10, TimeUnit.SECONDS);
boolean await2 = latch2.await(10, TimeUnit.SECONDS);

assertTrue(await1);
assertTrue(await2);
}

private void test(long timestampDaysAgo, RolloutCacheConfiguration.Builder configBuilder) throws InterruptedException {
// Preload DB with update timestamp of 1 day ago
long oldTimestamp = timestampDaysAgo;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ public void scheduleWorkSchedulesSplitsJob() {
mWrapper.scheduleWork();

Data inputData = new Data.Builder()
.putLong("splitCacheExpiration", 864000)
.putString("endpoint", "https://test.split.io/api")
.putBoolean("shouldRecordTelemetry", true)
.putStringArray("configuredFilterValues", new String[]{"set_1", "set_2"})
Expand Down Expand Up @@ -252,7 +251,6 @@ public void schedulingWithoutCertificatePinning() {
mWrapper.scheduleWork();

Data inputData = new Data.Builder()
.putLong("splitCacheExpiration", 864000)
.putString("endpoint", "https://test.split.io/api")
.putBoolean("shouldRecordTelemetry", true)
.putStringArray("configuredFilterValues", new String[]{"set_1", "set_2"})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@

public class RolloutCacheConfiguration {

private final int mExpiration;
private final int mExpirationDays;
private final boolean mClearOnInit;

private RolloutCacheConfiguration(int expiration, boolean clearOnInit) {
mExpiration = expiration;
mExpirationDays = expiration;
mClearOnInit = clearOnInit;
}

public int getExpiration() {
return mExpiration;
public int getExpirationDays() {
return mExpirationDays;
}

public boolean isClearOnInit() {
Expand All @@ -38,15 +38,15 @@ private Builder() {

/**
* Set the expiration time for the rollout definitions cache, in days. Default is 10 days.
* @param expiration in days
* @param expirationDays in days
* @return This builder
*/
public Builder expiration(int expiration) {
if (expiration < MIN_EXPIRATION_DAYS) {
public Builder expirationDays(int expirationDays) {
if (expirationDays < MIN_EXPIRATION_DAYS) {
Logger.w("Cache expiration must be at least 1 day. Using default value.");
mExpiration = ServiceConstants.DEFAULT_ROLLOUT_CACHE_EXPIRATION;
} else {
mExpiration = expiration;
mExpiration = expirationDays;
}

return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ public String trafficType() {

@Deprecated
public long cacheExpirationInSeconds() {
return TimeUnit.DAYS.toSeconds(rolloutCacheConfiguration().getExpiration());
return TimeUnit.DAYS.toSeconds(rolloutCacheConfiguration().getExpirationDays());
}

public long eventFlushInterval() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ private void triggerSdkReadyIfNeeded() {
if ((wasTriggered(SplitInternalEvent.MY_SEGMENTS_UPDATED) || wasTriggered(SplitInternalEvent.MY_SEGMENTS_FETCHED) || wasTriggered(SplitInternalEvent.MY_LARGE_SEGMENTS_UPDATED)) &&
(wasTriggered(SplitInternalEvent.SPLITS_UPDATED) || wasTriggered(SplitInternalEvent.SPLITS_FETCHED)) &&
!isTriggered(SplitEvent.SDK_READY)) {
if (!isTriggered(SplitEvent.SDK_READY_FROM_CACHE)) {
trigger(SplitEvent.SDK_READY_FROM_CACHE);
}
trigger(SplitEvent.SDK_READY);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ public class ServiceConstants {
public final static String WORKER_PARAM_ENDPOINT = "endpoint";
public final static String WORKER_PARAM_IMPRESSIONS_PER_PUSH = "impressionsPerPush";
public final static String WORKER_PARAM_EVENTS_PER_PUSH = "eventsPerPush";
public final static String WORKER_PARAM_SPLIT_CACHE_EXPIRATION = "splitCacheExpiration";
public static final String WORKER_PARAM_UNIQUE_KEYS_PER_PUSH = "unique_keys_per_push";
public static final String WORKER_PARAM_UNIQUE_KEYS_ESTIMATED_SIZE_IN_BYTES = "unique_keys_estimated_size_in_bytes";
public static final String WORKER_PARAM_ENCRYPTION_ENABLED = "encryptionEnabled";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,8 @@ public ImpressionsRecorderTask createImpressionsRecorderTask() {

@Override
public SplitsSyncTask createSplitsSyncTask(boolean checkCacheExpiration) {
return SplitsSyncTask.build(mSplitsSyncHelper, mSplitsStorageContainer.getSplitsStorage(), checkCacheExpiration,
mSplitClientConfig.cacheExpirationInSeconds(), mSplitsFilterQueryStringFromConfig, mEventsManager, mSplitsStorageContainer.getTelemetryStorage());
return SplitsSyncTask.build(mSplitsSyncHelper, mSplitsStorageContainer.getSplitsStorage(),
mSplitsFilterQueryStringFromConfig, mEventsManager, mSplitsStorageContainer.getTelemetryStorage());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,17 +185,6 @@ private void updateStorage(boolean clearBeforeUpdate, SplitChange splitChange) {
mSplitsStorage.update(mSplitChangeProcessor.process(splitChange));
}

public boolean cacheHasExpired(long storedChangeNumber, long updateTimestamp, long cacheExpirationInSeconds) {
long elapsed = now() - TimeUnit.MILLISECONDS.toSeconds(updateTimestamp);
return storedChangeNumber > -1
&& updateTimestamp > 0
&& (elapsed > cacheExpirationInSeconds);
}

private long now() {
return TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis());
}

private void logError(String message) {
Logger.e("Error while executing splits sync/update task: " + message);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ public class SplitsSyncTask implements SplitTask {
private final String mSplitsFilterQueryStringFromConfig;

private final SplitsStorage mSplitsStorage;
private final boolean mCheckCacheExpiration;
private final long mCacheExpirationInSeconds;
private final SplitsSyncHelper mSplitsSyncHelper;
@Nullable
private final ISplitEventsManager mEventsManager; // Should only be null on background sync
Expand All @@ -32,36 +30,28 @@ public class SplitsSyncTask implements SplitTask {

public static SplitsSyncTask build(@NonNull SplitsSyncHelper splitsSyncHelper,
@NonNull SplitsStorage splitsStorage,
boolean checkCacheExpiration,
long cacheExpirationInSeconds,
String splitsFilterQueryString,
@NonNull ISplitEventsManager eventsManager,
@NonNull TelemetryRuntimeProducer telemetryRuntimeProducer) {
return new SplitsSyncTask(splitsSyncHelper, splitsStorage, checkCacheExpiration, cacheExpirationInSeconds, splitsFilterQueryString, telemetryRuntimeProducer, eventsManager, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES);
return new SplitsSyncTask(splitsSyncHelper, splitsStorage, splitsFilterQueryString, telemetryRuntimeProducer, eventsManager, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES);
}

public static SplitTask buildForBackground(@NonNull SplitsSyncHelper splitsSyncHelper,
@NonNull SplitsStorage splitsStorage,
boolean checkCacheExpiration,
long cacheExpirationInSeconds,
String splitsFilterQueryString,
String splitsFilterQueryString,
@NonNull TelemetryRuntimeProducer telemetryRuntimeProducer) {
return new SplitsSyncTask(splitsSyncHelper, splitsStorage, checkCacheExpiration, cacheExpirationInSeconds, splitsFilterQueryString, telemetryRuntimeProducer, null, 1);
return new SplitsSyncTask(splitsSyncHelper, splitsStorage, splitsFilterQueryString, telemetryRuntimeProducer, null, 1);
}

private SplitsSyncTask(@NonNull SplitsSyncHelper splitsSyncHelper,
@NonNull SplitsStorage splitsStorage,
boolean checkCacheExpiration,
long cacheExpirationInSeconds,
String splitsFilterQueryString,
@NonNull TelemetryRuntimeProducer telemetryRuntimeProducer,
@Nullable ISplitEventsManager eventsManager,
int onDemandFetchBackoffMaxRetries) {

mSplitsStorage = checkNotNull(splitsStorage);
mSplitsSyncHelper = checkNotNull(splitsSyncHelper);
mCacheExpirationInSeconds = cacheExpirationInSeconds;
mCheckCacheExpiration = checkCacheExpiration;
mSplitsFilterQueryStringFromConfig = splitsFilterQueryString;
mEventsManager = eventsManager;
mChangeChecker = new SplitsChangeChecker();
Expand All @@ -73,10 +63,6 @@ private SplitsSyncTask(@NonNull SplitsSyncHelper splitsSyncHelper,
@NonNull
public SplitTaskExecutionInfo execute() {
long storedChangeNumber = mSplitsStorage.getTill();
long updateTimestamp = mSplitsStorage.getUpdateTimestamp();

boolean shouldClearExpiredCache = mCheckCacheExpiration &&
mSplitsSyncHelper.cacheHasExpired(storedChangeNumber, updateTimestamp, mCacheExpirationInSeconds);

boolean splitsFilterHasChanged = splitsFilterHasChanged(mSplitsStorage.getSplitsFilterQueryString());

Expand All @@ -87,8 +73,8 @@ public SplitTaskExecutionInfo execute() {

long startTime = System.currentTimeMillis();
SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(storedChangeNumber,
splitsFilterHasChanged || shouldClearExpiredCache,
splitsFilterHasChanged || shouldClearExpiredCache, mOnDemandFetchBackoffMaxRetries);
splitsFilterHasChanged,
splitsFilterHasChanged, mOnDemandFetchBackoffMaxRetries);
mTelemetryRuntimeProducer.recordSyncLatency(OperationType.SPLITS, System.currentTimeMillis() - startTime);

if (result.getStatus() == SplitTaskExecutionStatus.SUCCESS) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ private boolean validateExpiration() {
// calculate elapsed time since last update
long daysSinceLastUpdate = TimeUnit.MILLISECONDS.toDays(System.currentTimeMillis() - lastUpdateTimestamp);

if (lastUpdateTimestamp > 0 && daysSinceLastUpdate >= mConfig.getExpiration()) {
if (lastUpdateTimestamp > 0 && daysSinceLastUpdate >= mConfig.getExpirationDays()) {
Logger.v("Clearing rollout definitions cache due to expiration");
return true;
} else if (mConfig.isClearOnInit()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ private SplitTaskType taskTypeFromTags(Set<String> tags) {

private Data buildSplitSyncInputData() {
Data.Builder dataBuilder = new Data.Builder();
dataBuilder.putLong(ServiceConstants.WORKER_PARAM_SPLIT_CACHE_EXPIRATION, mSplitClientConfig.cacheExpirationInSeconds());
dataBuilder.putString(ServiceConstants.WORKER_PARAM_ENDPOINT, mSplitClientConfig.endpoint());
dataBuilder.putBoolean(ServiceConstants.SHOULD_RECORD_TELEMETRY, mSplitClientConfig.shouldRecordTelemetry());
dataBuilder.putString(ServiceConstants.WORKER_PARAM_CONFIGURED_FILTER_TYPE, (mFilter != null) ? mFilter.getType().queryStringField() : null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ public abstract class SplitWorker extends Worker {
private final SplitRoomDatabase mDatabase;
private final HttpClient mHttpClient;
private final String mEndpoint;
private final long mCacheExpirationInSeconds;

protected SplitTask mSplitTask;

Expand All @@ -36,8 +35,6 @@ public SplitWorker(@NonNull Context context,
String apiKey = inputData.getString(ServiceConstants.WORKER_PARAM_API_KEY);
mEndpoint = inputData.getString(ServiceConstants.WORKER_PARAM_ENDPOINT);
mDatabase = SplitRoomDatabase.getDatabase(context, databaseName);
mCacheExpirationInSeconds = inputData.getLong(ServiceConstants.WORKER_PARAM_SPLIT_CACHE_EXPIRATION,
ServiceConstants.DEFAULT_SPLITS_CACHE_EXPIRATION_IN_SECONDS);
mHttpClient = buildHttpClient(apiKey, buildCertPinningConfig(inputData.getString(ServiceConstants.WORKER_PARAM_CERTIFICATE_PINS)));
}

Expand All @@ -64,10 +61,6 @@ public String getEndPoint() {
return mEndpoint;
}

public long getCacheExpirationInSeconds() {
return mCacheExpirationInSeconds;
}

private static HttpClient buildHttpClient(String apiKey, @Nullable CertificatePinningConfiguration certificatePinningConfiguration) {
HttpClientImpl.Builder builder = new HttpClientImpl.Builder();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ public SplitsSyncWorker(@NonNull Context context,
new FetcherProvider(getHttpClient(), getEndPoint()),
new SplitChangeProcessorProvider().provideSplitChangeProcessor(params.configuredFilterType(), params.configuredFilterValues()),
new SyncHelperProvider(),
getCacheExpirationInSeconds(),
params.flagsSpec());

mSplitTask = builder.getTask();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
class SplitsSyncWorkerTaskBuilder {

private final long mCacheExpirationInSeconds;
private final StorageProvider mStorageProvider;
private final FetcherProvider mFetcherProvider;
private final SplitChangeProcessor mSplitChangeProcessor;
Expand All @@ -26,13 +25,11 @@ class SplitsSyncWorkerTaskBuilder {
FetcherProvider fetcherProvider,
SplitChangeProcessor splitChangeProcessor,
SyncHelperProvider splitsSyncHelperProvider,
long cacheExpirationInSeconds,
String flagsSpec) {
mStorageProvider = storageProvider;
mFetcherProvider = fetcherProvider;
mSplitsSyncHelperProvider = splitsSyncHelperProvider;
mSplitChangeProcessor = splitChangeProcessor;
mCacheExpirationInSeconds = cacheExpirationInSeconds;
mFlagsSpec = flagsSpec;
}

Expand All @@ -51,8 +48,6 @@ SplitTask getTask() {

return SplitsSyncTask.buildForBackground(splitsSyncHelper,
splitsStorage,
false,
mCacheExpirationInSeconds,
splitsFilterQueryString,
telemetryStorage);
} catch (URISyntaxException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,16 @@ public class RolloutCacheConfigurationTest {
@Test
public void defaultValues() {
RolloutCacheConfiguration config = RolloutCacheConfiguration.builder().build();
assertEquals(10, config.getExpiration());
assertEquals(10, config.getExpirationDays());
assertFalse(config.isClearOnInit());
}

@Test
public void expirationIsCorrectlySet() {
RolloutCacheConfiguration.Builder builder = RolloutCacheConfiguration.builder();
builder.expiration(1);
builder.expirationDays(1);
RolloutCacheConfiguration config = builder.build();
assertEquals(1, config.getExpiration());
assertEquals(1, config.getExpirationDays());
}

@Test
Expand All @@ -34,8 +34,8 @@ public void clearOnInitIsCorrectlySet() {
@Test
public void negativeExpirationIsSetToDefault() {
RolloutCacheConfiguration.Builder builder = RolloutCacheConfiguration.builder();
builder.expiration(-1);
builder.expirationDays(-1);
RolloutCacheConfiguration config = builder.build();
assertEquals(10, config.getExpiration());
assertEquals(10, config.getExpirationDays());
}
}
Loading
Loading