From 7ae26e7feb8f548a8e26842ec0b0f3faa292bc2a Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Wed, 25 Nov 2020 17:11:56 +0000 Subject: [PATCH] feat: implement synchronized streamable store --- CHANGELOG.md | 3 + bugsnag-android-core/detekt-baseline.xml | 1 + .../SynchronizedStreamableStoreTest.kt | 141 ++++++++++++++++++ .../java/com/bugsnag/android/JsonReadable.kt | 14 ++ .../android/SynchronizedStreamableStore.kt | 33 ++++ .../src/main/java/com/bugsnag/android/User.kt | 36 ++++- .../detekt-baseline.xml | 2 - 7 files changed, 225 insertions(+), 5 deletions(-) create mode 100644 bugsnag-android-core/src/androidTest/java/com/bugsnag/android/SynchronizedStreamableStoreTest.kt create mode 100644 bugsnag-android-core/src/main/java/com/bugsnag/android/JsonReadable.kt create mode 100644 bugsnag-android-core/src/main/java/com/bugsnag/android/SynchronizedStreamableStore.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 72a42f565d..497c90546c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## TBD +* Create synchronized store for user information + [#1010](https://github.com/bugsnag/bugsnag-android/pull/1010) + * Add persistenceDirectory config option for controlling event/session storage [#998](https://github.com/bugsnag/bugsnag-android/pull/998) diff --git a/bugsnag-android-core/detekt-baseline.xml b/bugsnag-android-core/detekt-baseline.xml index 1a2ecda262..6dc0d187eb 100644 --- a/bugsnag-android-core/detekt-baseline.xml +++ b/bugsnag-android-core/detekt-baseline.xml @@ -34,6 +34,7 @@ TooGenericExceptionCaught:ManifestConfigLoader.kt$ManifestConfigLoader$exc: Exception TooGenericExceptionCaught:PluginClient.kt$PluginClient$exc: Throwable TooGenericExceptionCaught:Stacktrace.kt$Stacktrace$lineEx: Exception + TooGenericExceptionCaught:SynchronizedStreamableStore.kt$SynchronizedStreamableStore$exc: Throwable TooGenericExceptionThrown:BreadcrumbStateTest.kt$BreadcrumbStateTest$throw Exception("Oh no") TooManyFunctions:ConfigInternal.kt$ConfigInternal : CallbackAwareMetadataAwareUserAware TooManyFunctions:DeviceDataCollector.kt$DeviceDataCollector diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/SynchronizedStreamableStoreTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/SynchronizedStreamableStoreTest.kt new file mode 100644 index 0000000000..002f74813d --- /dev/null +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/SynchronizedStreamableStoreTest.kt @@ -0,0 +1,141 @@ +package com.bugsnag.android + +import android.content.Context +import android.util.JsonReader +import androidx.test.core.app.ApplicationProvider +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test +import java.io.EOFException +import java.io.File +import java.io.FileNotFoundException +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors + +internal class SynchronizedStreamableStoreTest { + + private val user = User("123", "test@example.com", "Tess Tng") + + @Test + fun testPersistNonExistingFile() { + val ctx = ApplicationProvider.getApplicationContext() + val file = File(ctx.cacheDir, "no-such-file.json") + val store = SynchronizedStreamableStore(file) + store.persist(user) + assertEquals(user, store.load(User.Companion::fromReader)) + } + + @Test + fun testPersistWritableFile() { + val file = File.createTempFile("test", "json") + val store = SynchronizedStreamableStore(file) + store.persist(user) + assertEquals(user, store.load(User.Companion::fromReader)) + } + + @Test(expected = FileNotFoundException::class) + fun testPersistNonWritableFile() { + val file = File.createTempFile("test", "json").apply { + setWritable(false) + } + val store = SynchronizedStreamableStore(file) + store.persist(user) + assertNull(store.load(User.Companion::fromReader)) + } + + @Test(expected = NotImplementedError::class) + fun testPersistExceptionInStreamable() { + val file = File.createTempFile("test", "json") + val store = SynchronizedStreamableStore(file) + store.persist(CrashyStreamable()) + assertNull(store.load(CrashyStreamable.Companion::fromReader)) + } + + @Test(expected = FileNotFoundException::class) + fun testReadNonExistingFile() { + val file = File("no-such-file.bmp") + val store = SynchronizedStreamableStore(file) + assertNull(store.load(User.Companion::fromReader)) + } + + @Test(expected = EOFException::class) + fun testReadNonWritableFile() { + val file = File.createTempFile("test", "json").apply { + setWritable(false) + } + val store = SynchronizedStreamableStore(file) + assertNull(store.load(User.Companion::fromReader)) + } + + /** + * Reads the same file concurrently to assert that a [ReadWriteLock] is used + */ + @Test(timeout = 2000) + fun testConcurrentReadsPossible() { + // persist some initial data + val file = File.createTempFile("test", "json") + val store = SynchronizedStreamableStore(file) + store.persist(ThreadTestStreamable("some_val")) + + // read file on bg thread, triggered halfway through reading file on main thread + var alreadyReadingBgThread = false + ThreadTestStreamable.readCallback = { + if (!alreadyReadingBgThread) { + alreadyReadingBgThread = true + val reader = JsonReader(file.reader()) + val latch = CountDownLatch(1) + + Executors.newSingleThreadExecutor().execute { + val bgThreadObj = ThreadTestStreamable.fromReader(reader) + assertEquals("some_val", bgThreadObj.id) + latch.countDown() + } + latch.await() + } + } + + // read the file on the main thread + val reader = JsonReader(file.reader()) + val mainThreadObj = ThreadTestStreamable.fromReader(reader) + assertEquals("some_val", mainThreadObj.id) + } +} + +internal class ThreadTestStreamable( + val id: String, + val writeCallback: () -> Unit = {} +) : JsonStream.Streamable { + + override fun toStream(stream: JsonStream) { + with(stream) { + beginObject() + name("test") + writeCallback() + value(id) + endObject() + } + } + + companion object : JsonReadable { + var readCallback: () -> Unit = {} + + override fun fromReader(reader: JsonReader): ThreadTestStreamable { + with(reader) { + beginObject() + nextName() + readCallback() + val obj = ThreadTestStreamable(nextString()) + endObject() + return obj + } + } + } +} + +internal class CrashyStreamable : JsonStream.Streamable { + override fun toStream(stream: JsonStream) = TODO("I'll handle this later...") + + companion object: JsonReadable { + override fun fromReader(reader: JsonReader) = TODO("coffee break...") + } +} diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/JsonReadable.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/JsonReadable.kt new file mode 100644 index 0000000000..dc68f3e82f --- /dev/null +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/JsonReadable.kt @@ -0,0 +1,14 @@ +package com.bugsnag.android + +import android.util.JsonReader + +/** + * Classes which implement this interface are capable of deserializing a JSON input. + */ +internal interface JsonReadable { + + /** + * Constructs an object from a JSON input. + */ + fun fromReader(reader: JsonReader): T +} diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/SynchronizedStreamableStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/SynchronizedStreamableStore.kt new file mode 100644 index 0000000000..a2e8eb72b0 --- /dev/null +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/SynchronizedStreamableStore.kt @@ -0,0 +1,33 @@ +package com.bugsnag.android + +import android.util.JsonReader +import java.io.File +import java.io.IOException +import java.util.concurrent.locks.ReentrantReadWriteLock +import kotlin.concurrent.withLock + +internal class SynchronizedStreamableStore( + private val file: File +) { + + private val lock = ReentrantReadWriteLock() + + @Throws(IOException::class) + fun persist(streamable: T) { + lock.writeLock().withLock { + file.writer().use { + streamable.toStream(JsonStream(it)) + true + } + } + } + + @Throws(IOException::class) + fun load(loadCallback: (JsonReader) -> T): T { + lock.readLock().withLock { + return file.reader().use { + loadCallback(JsonReader(it)) + } + } + } +} diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/User.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/User.kt index 8ce21e6b9d..c32d3a8ddc 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/User.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/User.kt @@ -1,5 +1,6 @@ package com.bugsnag.android +import android.util.JsonReader import java.io.IOException /** @@ -25,12 +26,41 @@ class User @JvmOverloads internal constructor( @Throws(IOException::class) override fun toStream(writer: JsonStream) { writer.beginObject() - writer.name("id").value(id) - writer.name("email").value(email) - writer.name("name").value(name) + writer.name(KEY_ID).value(id) + writer.name(KEY_EMAIL).value(email) + writer.name(KEY_NAME).value(name) writer.endObject() } + internal companion object: JsonReadable { + private const val KEY_ID = "id" + private const val KEY_NAME = "name" + private const val KEY_EMAIL = "email" + + override fun fromReader(reader: JsonReader): User { + var user: User + with(reader) { + beginObject() + var id: String? = null + var email: String? = null + var name: String? = null + + while (hasNext()) { + val key = nextName() + val value = nextString() + when (key) { + KEY_ID -> id = value + KEY_EMAIL -> email = value + KEY_NAME -> name = value + } + } + user = User(id, email, name) + endObject() + } + return user + } + } + override fun equals(other: Any?): Boolean { if (this === other) return true if (javaClass != other?.javaClass) return false diff --git a/bugsnag-plugin-android-ndk/detekt-baseline.xml b/bugsnag-plugin-android-ndk/detekt-baseline.xml index 582fa0af87..51aa1e990d 100644 --- a/bugsnag-plugin-android-ndk/detekt-baseline.xml +++ b/bugsnag-plugin-android-ndk/detekt-baseline.xml @@ -7,10 +7,8 @@ MaxLineLength:NativeBridge.kt$NativeBridge$is AddBreadcrumb -> addBreadcrumb(makeSafe(msg.message), makeSafe(msg.type.toString()), makeSafe(msg.timestamp), msg.metadata) MaxLineLength:NativeBridge.kt$NativeBridge$is StartSession -> startedSession(makeSafe(msg.id), makeSafe(msg.startedAt), msg.handledCount, msg.unhandledCount) NestedBlockDepth:NativeBridge.kt$NativeBridge$private fun deliverPendingReports() - NewLineAtEndOfFile:VerifyUtils.kt$com.bugsnag.android.ndk.VerifyUtils.kt ReturnCount:NativeBridge.kt$NativeBridge$private fun isInvalidMessage(msg: Any?): Boolean TooGenericExceptionCaught:NativeBridge.kt$NativeBridge$ex: Exception TooManyFunctions:NativeBridge.kt$NativeBridge : Observer - WildcardImport:NativeBridge.kt$import com.bugsnag.android.StateEvent.*