Merge pull request #1010 from bugsnag/PLAT-5401/synchronized-store

Create synchronized store for user information

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)
 

bugsnag-android-core/detekt-baseline.xml

@@ -34,6 +34,7 @@
     <ID>TooGenericExceptionCaught:ManifestConfigLoader.kt$ManifestConfigLoader$exc: Exception</ID>
     <ID>TooGenericExceptionCaught:PluginClient.kt$PluginClient$exc: Throwable</ID>
     <ID>TooGenericExceptionCaught:Stacktrace.kt$Stacktrace$lineEx: Exception</ID>
+    <ID>TooGenericExceptionCaught:SynchronizedStreamableStore.kt$SynchronizedStreamableStore$exc: Throwable</ID>
     <ID>TooGenericExceptionThrown:BreadcrumbStateTest.kt$BreadcrumbStateTest$throw Exception("Oh no")</ID>
     <ID>TooManyFunctions:ConfigInternal.kt$ConfigInternal : CallbackAwareMetadataAwareUserAware</ID>
     <ID>TooManyFunctions:DeviceDataCollector.kt$DeviceDataCollector</ID>

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<Context>()
+        val file = File(ctx.cacheDir, "no-such-file.json")
+        val store = SynchronizedStreamableStore<User>(file)
+        store.persist(user)
+        assertEquals(user, store.load(User.Companion::fromReader))
+    }
+
+    @Test
+    fun testPersistWritableFile() {
+        val file = File.createTempFile("test", "json")
+        val store = SynchronizedStreamableStore<User>(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<User>(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<CrashyStreamable>(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<User>(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<User>(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<ThreadTestStreamable>(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<ThreadTestStreamable> {
+        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<CrashyStreamable> {
+        override fun fromReader(reader: JsonReader) = TODO("coffee break...")
+    }
+}

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<T : JsonStream.Streamable> {
+
+    /**
+     * Constructs an object from a JSON input.
+     */
+    fun fromReader(reader: JsonReader): T
+}

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<T : JsonStream.Streamable>(
+    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))
+            }
+        }
+    }
+}

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<User> {
+        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

bugsnag-plugin-android-ndk/detekt-baseline.xml

@@ -7,10 +7,8 @@
     <ID>MaxLineLength:NativeBridge.kt$NativeBridge$is AddBreadcrumb -&gt; addBreadcrumb(makeSafe(msg.message), makeSafe(msg.type.toString()), makeSafe(msg.timestamp), msg.metadata)</ID>
     <ID>MaxLineLength:NativeBridge.kt$NativeBridge$is StartSession -&gt; startedSession(makeSafe(msg.id), makeSafe(msg.startedAt), msg.handledCount, msg.unhandledCount)</ID>
     <ID>NestedBlockDepth:NativeBridge.kt$NativeBridge$private fun deliverPendingReports()</ID>
-    <ID>NewLineAtEndOfFile:VerifyUtils.kt$com.bugsnag.android.ndk.VerifyUtils.kt</ID>
     <ID>ReturnCount:NativeBridge.kt$NativeBridge$private fun isInvalidMessage(msg: Any?): Boolean</ID>
     <ID>TooGenericExceptionCaught:NativeBridge.kt$NativeBridge$ex: Exception</ID>
     <ID>TooManyFunctions:NativeBridge.kt$NativeBridge : Observer</ID>
-    <ID>WildcardImport:NativeBridge.kt$import com.bugsnag.android.StateEvent.*</ID>
   </CurrentIssues>
 </SmellBaseline>