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

feat: switch storage for better thread safety #181

Merged
merged 22 commits into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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
9 changes: 5 additions & 4 deletions .github/workflows/docs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ jobs:
build-and-deploy:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Set up JDK 11
uses: actions/setup-java@v3
uses: actions/setup-java@v4
with:
java-version: '11'
distribution: 'zulu'

- name: Checkout
uses: actions/checkout@v3
cache: 'gradle'

- name: Setup Docs
run: ./gradlew dokkaHtmlMultiModule
Expand Down
7 changes: 4 additions & 3 deletions .github/workflows/pull-request-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@ jobs:
test:
runs-on: ubuntu-20.04
steps:
- uses: actions/checkout@v4
- name: Set up JDK 11
uses: actions/setup-java@v3
uses: actions/setup-java@v4
with:
java-version: '11'
distribution: 'temurin'
- uses: actions/checkout@v3
cache: 'gradle'
- name: Build
run: ./gradlew build
- name: Unit Test
Expand All @@ -20,7 +21,7 @@ jobs:
run: ./gradlew ktlintCheck
- name: Upload build results
if: always()
uses: actions/upload-artifact@v2
uses: actions/upload-artifact@v4
with:
# artifacts name
name: build-results
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,14 @@ jobs:
needs: [authorize]
steps:
- name: Checkout
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Set up JDK 11
uses: actions/setup-java@v3
uses: actions/setup-java@v4
with:
java-version: '11'
distribution: 'zulu'
cache: 'gradle'

- name: Build
run: ./gradlew build
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,22 @@ import com.amplitude.android.Configuration
import com.amplitude.android.utilities.AndroidStorage

class ApiKeyStorageMigration(
private val amplitude: Amplitude
private val amplitude: Amplitude,
) {
suspend fun execute() {
val configuration = amplitude.configuration as Configuration
val logger = amplitude.logger

val storage = amplitude.storage as? AndroidStorage
if (storage != null) {
val apiKeyStorage = AndroidStorage(configuration.context, configuration.apiKey, logger, storage.prefix)
val apiKeyStorage = AndroidStorage(configuration.context, configuration.apiKey, logger, storage.prefix, amplitude.diagnostics)
StorageKeyMigration(apiKeyStorage, storage, logger).execute()
}

val identifyInterceptStorage = amplitude.identifyInterceptStorage as? AndroidStorage
if (identifyInterceptStorage != null) {
val apiKeyStorage = AndroidStorage(configuration.context, configuration.apiKey, logger, identifyInterceptStorage.prefix)
val apiKeyStorage =
AndroidStorage(configuration.context, configuration.apiKey, logger, identifyInterceptStorage.prefix, amplitude.diagnostics)
StorageKeyMigration(apiKeyStorage, identifyInterceptStorage, logger).execute()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import com.amplitude.core.Storage
import com.amplitude.core.StorageProvider
import com.amplitude.core.events.BaseEvent
import com.amplitude.core.platform.EventPipeline
import com.amplitude.core.utilities.Diagnostics
import com.amplitude.core.utilities.EventsFileManager
import com.amplitude.core.utilities.EventsFileStorage
import com.amplitude.core.utilities.FileResponseHandler
Expand All @@ -24,9 +25,9 @@ class AndroidStorage(
context: Context,
val storageKey: String,
private val logger: Logger,
internal val prefix: String?
internal val prefix: String?,
private val diagnostics: Diagnostics,
) : Storage, EventsFileStorage {

companion object {
const val STORAGE_PREFIX = "amplitude-android"
}
Expand All @@ -35,7 +36,7 @@ class AndroidStorage(
context.getSharedPreferences("${getPrefix()}-$storageKey", Context.MODE_PRIVATE)
private val storageDirectory: File = context.getDir(getDir(), Context.MODE_PRIVATE)
private val eventsFile =
EventsFileManager(storageDirectory, storageKey, AndroidKVS(sharedPreferences))
EventsFileManager(storageDirectory, storageKey, AndroidKVS(sharedPreferences), logger, diagnostics)
private val eventCallbacksMap = mutableMapOf<String, EventCallBack>()

override suspend fun writeEvent(event: BaseEvent) {
Expand All @@ -47,7 +48,10 @@ class AndroidStorage(
}
}

override suspend fun write(key: Storage.Constants, value: String) {
override suspend fun write(
key: Storage.Constants,
value: String,
) {
sharedPreferences.edit().putString(key.rawVal, value).apply()
}

Expand Down Expand Up @@ -87,7 +91,7 @@ class AndroidStorage(
configuration,
scope,
dispatcher,
logger
logger,
)
}

Expand All @@ -103,7 +107,10 @@ class AndroidStorage(
eventCallbacksMap.remove(insertId)
}

override fun splitEventFile(filePath: String, events: JSONArray) {
override fun splitEventFile(
filePath: String,
events: JSONArray,
) {
eventsFile.splitFile(filePath, events)
}

Expand All @@ -120,13 +127,17 @@ class AndroidStorage(
}

class AndroidStorageProvider : StorageProvider {
override fun getStorage(amplitude: Amplitude, prefix: String?): Storage {
override fun getStorage(
amplitude: Amplitude,
prefix: String?,
): Storage {
val configuration = amplitude.configuration as com.amplitude.android.Configuration
return AndroidStorage(
configuration.context,
configuration.instanceName,
configuration.loggerProvider.getLogger(amplitude),
prefix
prefix,
amplitude.diagnostics,
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import com.amplitude.android.utilities.AndroidStorage
import com.amplitude.common.jvm.ConsoleLogger
import com.amplitude.core.Storage
import com.amplitude.core.events.BaseEvent
import com.amplitude.core.utilities.Diagnostics
import kotlinx.coroutines.runBlocking
import org.junit.Test
import org.junit.jupiter.api.Assertions
Expand All @@ -16,13 +17,15 @@ import java.util.UUID

@RunWith(RobolectricTestRunner::class)
class StorageKeyMigrationTest {
private val testDiagnostics = Diagnostics()

@Test
fun `simple values should be migrated`() {
val context = ApplicationProvider.getApplicationContext<Context>()
val logger = ConsoleLogger()

val source = AndroidStorage(context, UUID.randomUUID().toString(), logger, null)
val destination = AndroidStorage(context, UUID.randomUUID().toString(), logger, null)
val source = AndroidStorage(context, UUID.randomUUID().toString(), logger, null, testDiagnostics)
val destination = AndroidStorage(context, UUID.randomUUID().toString(), logger, null, testDiagnostics)
val sourceFileIndexKey = "amplitude.events.file.index.${source.storageKey}"
val destinationFileIndexKey = "amplitude.events.file.index.${destination.storageKey}"

Expand Down Expand Up @@ -74,8 +77,8 @@ class StorageKeyMigrationTest {
val context = ApplicationProvider.getApplicationContext<Context>()
val logger = ConsoleLogger()

val source = AndroidStorage(context, UUID.randomUUID().toString(), logger, null)
val destination = AndroidStorage(context, UUID.randomUUID().toString(), logger, null)
val source = AndroidStorage(context, UUID.randomUUID().toString(), logger, null, testDiagnostics)
val destination = AndroidStorage(context, UUID.randomUUID().toString(), logger, null, testDiagnostics)

runBlocking {
source.writeEvent(createEvent(1))
Expand Down Expand Up @@ -117,8 +120,8 @@ class StorageKeyMigrationTest {
val context = ApplicationProvider.getApplicationContext<Context>()
val logger = ConsoleLogger()

val source = AndroidStorage(context, UUID.randomUUID().toString(), logger, null)
val destination = AndroidStorage(context, UUID.randomUUID().toString(), logger, null)
val source = AndroidStorage(context, UUID.randomUUID().toString(), logger, null, testDiagnostics)
val destination = AndroidStorage(context, UUID.randomUUID().toString(), logger, null, testDiagnostics)

var destinationPreviousSessionId = destination.read(Storage.Constants.PREVIOUS_SESSION_ID)
var destinationLastEventTime = destination.read(Storage.Constants.LAST_EVENT_TIME)
Expand Down Expand Up @@ -150,8 +153,8 @@ class StorageKeyMigrationTest {
val context = ApplicationProvider.getApplicationContext<Context>()
val logger = ConsoleLogger()

val source = AndroidStorage(context, UUID.randomUUID().toString(), logger, null)
val destination = AndroidStorage(context, UUID.randomUUID().toString(), logger, null)
val source = AndroidStorage(context, UUID.randomUUID().toString(), logger, null, testDiagnostics)
val destination = AndroidStorage(context, UUID.randomUUID().toString(), logger, null, testDiagnostics)

runBlocking {
source.writeEvent(createEvent(1))
Expand Down
Loading