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

Prefix config #553

Merged
merged 4 commits into from
Nov 14, 2023
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 @@ -16,7 +16,7 @@ apply plugin: 'signing'
apply plugin: 'kotlin-android'

ext {
splitVersion = '3.4.0'
splitVersion = '3.5.0-alpha-1'
}

android {
Expand Down
3 changes: 2 additions & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
ossrhUsername=
ossrhPassword=
android.enableJetifier=true
android.useAndroidX=true

kotlin.stdlib.default.dependency=false
10 changes: 9 additions & 1 deletion src/androidTest/java/helper/TestableSplitConfigBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ public class TestableSplitConfigBuilder {
private long mDefaultSSEConnectionDelayInSecs = ServiceConstants.DEFAULT_SSE_CONNECTION_DELAY_SECS;
private long mSSEDisconnectionDelayInSecs = 60L;

private String mPrefix = "";

public TestableSplitConfigBuilder() {
mServiceEndpoints = ServiceEndpoints.builder().build();
}
Expand Down Expand Up @@ -249,6 +251,11 @@ public TestableSplitConfigBuilder sseDisconnectionDelayInSecs(long seconds) {
return this;
}

public TestableSplitConfigBuilder prefix(String prefix) {
this.mPrefix = prefix;
return this;
}

public SplitClientConfig build() {
Constructor constructor = SplitClientConfig.class.getDeclaredConstructors()[0];
constructor.setAccessible(true);
Expand Down Expand Up @@ -300,7 +307,8 @@ public SplitClientConfig build() {
mUserConsent,
mEncryptionEnabled,
mDefaultSSEConnectionDelayInSecs,
mSSEDisconnectionDelayInSecs);
mSSEDisconnectionDelayInSecs,
mPrefix);
return config;
} catch (Exception e) {
Logger.e("Error creating Testable Split client builder: "
Expand Down
24 changes: 22 additions & 2 deletions src/main/java/io/split/android/client/SplitClientConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ public class SplitClientConfig {
private int mLogLevel = SplitLogLevel.NONE;
private UserConsent mUserConsent;
private boolean mEncryptionEnabled = false;
private final String mPrefix;
private final long mDefaultSSEConnectionDelayInSecs;
private final long mSSEDisconnectionDelayInSecs;

Expand Down Expand Up @@ -172,7 +173,8 @@ private SplitClientConfig(String endpoint,
UserConsent userConsent,
boolean encryptionEnabled,
long defaultSSEConnectionDelayInSecs,
long sseDisconnectionDelayInSecs) {
long sseDisconnectionDelayInSecs,
String prefix) {
mEndpoint = endpoint;
mEventsEndpoint = eventsEndpoint;
mTelemetryEndpoint = telemetryEndpoint;
Expand Down Expand Up @@ -226,6 +228,7 @@ private SplitClientConfig(String endpoint,
mEncryptionEnabled = encryptionEnabled;
mDefaultSSEConnectionDelayInSecs = defaultSSEConnectionDelayInSecs;
mSSEDisconnectionDelayInSecs = sseDisconnectionDelayInSecs;
mPrefix = prefix;

Logger.instance().setLevel(mLogLevel);
}
Expand Down Expand Up @@ -350,6 +353,10 @@ String defaultDataFolder() {
return DEFAULT_DATA_FOLDER;
}

String prefix() {
return mPrefix;
}

public String ip() {
return mIp;
}
Expand Down Expand Up @@ -521,6 +528,8 @@ public static final class Builder {

private final long mSSEDisconnectionDelayInSecs = 60L;

private String mPrefix = "";

public Builder() {
mServiceEndpoints = ServiceEndpoints.builder().build();
}
Expand Down Expand Up @@ -1022,6 +1031,16 @@ public Builder encryptionEnabled(boolean enabled) {
return this;
}

/**
* Optional prefix for the database name.
* @param prefix Prefix for the database name.
* @return This builder
*/
public Builder prefix(String prefix) {
mPrefix = (prefix == null) ? "" : prefix.trim();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TBD: validation

return this;
}

public SplitClientConfig build() {


Expand Down Expand Up @@ -1127,7 +1146,8 @@ public SplitClientConfig build() {
mUserConsent,
mEncryptionEnabled,
mDefaultSSEConnectionDelayInSecs,
mSSEDisconnectionDelayInSecs);
mSSEDisconnectionDelayInSecs,
mPrefix);
}

private HttpProxy parseProxyHost(String proxyUri) {
Expand Down
13 changes: 10 additions & 3 deletions src/main/java/io/split/android/client/SplitFactoryHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,20 @@ String getDatabaseName(SplitClientConfig config, String apiToken, Context contex
}

private String buildDatabaseName(SplitClientConfig config, String apiToken) {
int apiTokenLength = apiToken.length();
if (apiToken == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the SDK keys is validated previously in Input Validation somewhere, so this check wouldn't be necessary.

throw new IllegalArgumentException("SDK key cannot be null");
}

final int apiTokenLength = apiToken.length();
final String prefix = (config.prefix() == null) ? "" : config.prefix();

if (apiTokenLength > DB_MAGIC_CHARS_COUNT) {
String begin = apiToken.substring(0, DB_MAGIC_CHARS_COUNT);
String end = apiToken.substring(apiTokenLength - DB_MAGIC_CHARS_COUNT);
return begin + end;
return prefix + begin + end;
}
return config.defaultDataFolder();

return prefix + config.defaultDataFolder();
}

private String buildLegacyDatabaseName(SplitClientConfig splitClientConfig, String apiToken) {
Expand Down
113 changes: 111 additions & 2 deletions src/test/java/io/split/android/client/SplitFactoryHelperTest.kt
Original file line number Diff line number Diff line change
@@ -1,35 +1,53 @@
package io.split.android.client

import android.content.Context
import io.split.android.client.service.executor.SplitTaskExecutionListener
import io.split.android.client.service.executor.SplitTaskExecutor
import io.split.android.client.storage.cipher.EncryptionMigrationTask
import io.split.android.client.storage.db.SplitRoomDatabase
import junit.framework.TestCase.assertEquals
import org.junit.After
import org.junit.Before
import org.junit.Test
import org.mockito.Mock
import org.mockito.Mockito
import org.mockito.Mockito.any
import org.mockito.Mockito.argThat
import org.mockito.Mockito.mock
import org.mockito.Mockito.verify
import org.mockito.Mockito.`when`
import org.mockito.MockitoAnnotations
import java.io.File
import java.lang.IllegalArgumentException

class SplitFactoryHelperTest {

private lateinit var mocks: AutoCloseable

@Mock
private lateinit var splitRoomDatabase: SplitRoomDatabase
@Mock
private lateinit var splitTaskExecutor: SplitTaskExecutor
@Mock
private lateinit var taskListener: SplitTaskExecutionListener
@Mock
private lateinit var context: Context

private lateinit var helper: SplitFactoryHelper

@Before
fun setup() {
MockitoAnnotations.openMocks(this)
mocks = MockitoAnnotations.openMocks(this)
helper = SplitFactoryHelper()
}

@After
fun tearDown() {
mocks.close()
}

@Test
fun testMigrateEncryption() {
fun migrateEncryption() {

helper.migrateEncryption(
"abcdedfghijklmnopqrstuvwxyz",
Expand All @@ -43,4 +61,95 @@ class SplitFactoryHelperTest {
argThat { it is EncryptionMigrationTask },
argThat { it?.equals(taskListener) == true })
}

@Test
fun generateDatabaseNameWithoutPrefixAndKeyLongerThan4() {
val path = mock(File::class.java)
`when`(path.exists()).thenReturn(true)
`when`(context.getDatabasePath("abcdwxyz")).thenReturn(path)
val databaseName = helper.getDatabaseName(
SplitClientConfig.builder().build(),
"abcdedfghijklmnopqrstuvwxyz",
context
)

assertEquals("abcdwxyz", databaseName)
}

@Test
fun generateDatabaseNameWithoutPrefixAndKeyShorterThan4() {
val path = mock(File::class.java)
`when`(path.exists()).thenReturn(true)
`when`(context.getDatabasePath("split_data")).thenReturn(path)
val databaseName = helper.getDatabaseName(
SplitClientConfig.builder().build(),
"abc",
context
)

assertEquals("split_data", databaseName)
}

@Test
fun generateDatabaseNameWithPrefixAndKeyLongerThan4() {
val path = mock(File::class.java)
`when`(path.exists()).thenReturn(true)
`when`(context.getDatabasePath("mydbabcdwxyz")).thenReturn(path)
val databaseName = helper.getDatabaseName(
SplitClientConfig.builder().prefix("mydb").build(),
"abcdedfghijklmnopqrstuvwxyz",
context
)

assertEquals("mydbabcdwxyz", databaseName)
}

@Test
fun generateDatabaseNameWithPrefixAndKeyShorterThan4() {
val path = mock(File::class.java)
`when`(path.exists()).thenReturn(true)
`when`(context.getDatabasePath("mydbsplit_data")).thenReturn(path)
val databaseName = helper.getDatabaseName(
SplitClientConfig.builder().prefix("mydb").build(),
"abc",
context
)

assertEquals("mydbsplit_data", databaseName)
}

@Test
fun generateDatabaseNameWithNullKeyThrowsIllegalArgumentException() {
val path = mock(File::class.java)
`when`(path.exists()).thenReturn(true)
`when`(context.getDatabasePath(Mockito.anyString())).thenReturn(path)

try {
helper.getDatabaseName(
SplitClientConfig.builder().build(),
null,
context
)
} catch (e: IllegalArgumentException) {
assertEquals("SDK key cannot be null", e.message)
}
}

@Test
fun legacyDbIsRenamedIfExists() {
val nonExistingPath = mock(File::class.java)
val existingPath = mock(File::class.java)
`when`(nonExistingPath.exists()).thenReturn(false)
`when`(existingPath.exists()).thenReturn(true)
`when`(context.getDatabasePath(any())).thenReturn(existingPath);
`when`(context.getDatabasePath("abcdwxyz")).thenReturn(nonExistingPath)
val databaseName = helper.getDatabaseName(
SplitClientConfig.builder().build(),
"abcdfghijklmnopqrstuvwxyz",
context
)

verify(existingPath).renameTo(nonExistingPath)
assertEquals("abcdwxyz", databaseName)
}
}