Cache registered properties (#51)

PostHog · Oct 18, 2023 · a51ff9b · a51ff9b
1 parent 1cc8200
commit a51ff9b
Show file tree

Hide file tree

Showing 12 changed files with 143 additions and 31 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -1,6 +1,8 @@
 name: 'Build & Test'
 on:
   push:
+    branches:
+      - main
   pull_request:
 
 jobs:

diff --git a/.github/workflows/gradle-wrapper-validation.yml b/.github/workflows/gradle-wrapper-validation.yml
@@ -1,6 +1,8 @@
 name: 'Validate Gradle Wrapper'
 on:
   push:
+    branches:
+      - main
   pull_request:
 
 jobs:

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,7 @@
 ## Next
 
+- Registered keys are cached in the disk preferences ([#51](https://github.com/PostHog/posthog-android/pull/51))
+
 ## 3.0.0-alpha.8 - 2023-10-12
 
 - SDK only sends the `$feature_flag_called` event once per flag ([#47](https://github.com/PostHog/posthog-android/pull/47))

diff --git a/posthog-android/src/main/java/com/posthog/android/internal/PostHogSharedPreferences.kt b/posthog-android/src/main/java/com/posthog/android/internal/PostHogSharedPreferences.kt
@@ -5,7 +5,9 @@ import android.content.Context.MODE_PRIVATE
 import android.content.SharedPreferences
 import com.posthog.android.PostHogAndroidConfig
 import com.posthog.internal.PostHogPreferences
+import com.posthog.internal.PostHogPreferences.Companion.ALL_INTERNAL_KEYS
 import com.posthog.internal.PostHogPreferences.Companion.GROUPS
+import com.posthog.internal.PostHogPreferences.Companion.STRINGIFIED_KEYS
 
 /**
  * Reads and writes to the SDKs shared preferences
@@ -29,10 +31,18 @@ internal class PostHogSharedPreferences(
             value = sharedPreferences.all[key] ?: defaultValue
         }
 
+        val stringifiedKeys = getStringifiedKeys()
+        return convertValue(key, value, stringifiedKeys)
+    }
+
+    private fun convertValue(key: String, value: Any?, keys: Set<String>): Any? {
         return when (value) {
             is String -> {
                 // we only want to deserialize special keys
-                if (SPECIAL_KEYS.contains(key)) {
+                // or keys that were stringified.
+                if (SPECIAL_KEYS.contains(key) ||
+                    keys.contains(key)
+                ) {
                     deserializeObject(value)
                 } else {
                     value
@@ -108,10 +118,32 @@ internal class PostHogSharedPreferences(
         }
     }
 
+    private fun addToStringifiedKeys(key: String, editor: SharedPreferences.Editor) {
+        val stringifiedKeys = getStringifiedKeys() + key
+        editor.putStringSet(STRINGIFIED_KEYS, stringifiedKeys)
+    }
+
+    private fun removeFromStringifiedKeys(key: String, editor: SharedPreferences.Editor) {
+        val keys = getStringifiedKeys().toMutableSet()
+        if (!keys.contains(key)) {
+            return
+        }
+        keys.remove(key)
+        editor.putStringSet(STRINGIFIED_KEYS, keys)
+    }
+
+    private fun getStringifiedKeys(): Set<String> {
+        return sharedPreferences.getStringSet(STRINGIFIED_KEYS, setOf()) ?: setOf()
+    }
+
     private fun serializeObject(key: String, value: Any, editor: SharedPreferences.Editor) {
         try {
             config.serializer.serializeObject(value)?.let {
                 editor.putString(key, it)
+
+                addToStringifiedKeys(key, editor)
+            } ?: run {
+                config.logger.log("Value type: ${value.javaClass.name} and value: $value isn't valid.")
             }
         } catch (e: Throwable) {
             config.logger.log("Value type: ${value.javaClass.name} and value: $value isn't valid.")
@@ -133,16 +165,28 @@ internal class PostHogSharedPreferences(
         val edit = sharedPreferences.edit()
         synchronized(lock) {
             edit.remove(key)
+            removeFromStringifiedKeys(key, edit)
             edit.apply()
         }
     }
 
     override fun getAll(): Map<String, Any> {
-        val preferences: Map<String, Any>
+        val allPreferences: Map<String, Any>
         synchronized(lock) {
             @Suppress("UNCHECKED_CAST")
-            preferences = sharedPreferences.all.toMap() as? Map<String, Any> ?: emptyMap()
+            allPreferences = sharedPreferences.all.toMap() as? Map<String, Any> ?: emptyMap()
+        }
+        val filteredPreferences = allPreferences.filterKeys { key ->
+            !ALL_INTERNAL_KEYS.contains(key)
         }
+        val preferences = mutableMapOf<String, Any>()
+        val stringifiedKeys = getStringifiedKeys()
+        for ((key, value) in filteredPreferences) {
+            convertValue(key, value, stringifiedKeys)?.let {
+                preferences[key] = it
+            }
+        }
+
         return preferences
     }
 

diff --git a/posthog-android/src/test/java/com/posthog/android/internal/PostHogSharedPreferencesTests.kt b/posthog-android/src/test/java/com/posthog/android/internal/PostHogSharedPreferencesTests.kt
@@ -6,6 +6,7 @@ import com.posthog.android.FakeSharedPreferences
 import com.posthog.android.PostHogAndroidConfig
 import com.posthog.android.apiKey
 import com.posthog.internal.PostHogPreferences.Companion.GROUPS
+import com.posthog.internal.PostHogPreferences.Companion.STRINGIFIED_KEYS
 import org.junit.runner.RunWith
 import org.mockito.kotlin.mock
 import kotlin.test.Test
@@ -100,7 +101,8 @@ internal class PostHogSharedPreferencesTests {
 
         sut.setValue("key", Any())
 
-        assertEquals("{}", sut.getValue("key"))
+        @Suppress("UNCHECKED_CAST")
+        assertEquals(emptyMap(), sut.getValue("key") as? Map<String, Any>)
     }
 
     @Test
@@ -114,14 +116,17 @@ internal class PostHogSharedPreferencesTests {
     }
 
     @Test
-    fun `preferences fallback to stringified version if not special key`() {
+    fun `preferences fallback to stringified version if not special and not stringified key`() {
         val sut = getSut()
 
         val props = mapOf("key" to "value")
-        sut.setValue("key", props)
+        sut.setValue("myJson", props)
+
+        // removing to make it testable
+        sut.remove(STRINGIFIED_KEYS)
 
         val json = """{"key":"value"}"""
-        assertEquals(json, sut.getValue("key"))
+        assertEquals(json, sut.getValue("myJson"))
     }
 
     @Test

diff --git a/...g-samples/posthog-android-sample/src/main/java/com/posthog/android/sample/MainActivity.kt b/...g-samples/posthog-android-sample/src/main/java/com/posthog/android/sample/MainActivity.kt
@@ -41,6 +41,7 @@ fun Greeting(name: String, modifier: Modifier = Modifier) {
 //            PostHog.optOut()
 //            PostHog.optIn()
 //            PostHog.identify("my_distinct_id", properties = mapOf("my_property" to 1), userProperties = mapOf("name" to "hello"))
+            PostHog.register("test", mapOf("one" to "two"))
             PostHog.capture("testEvent", properties = mapOf("testProperty" to "testValue"))
 //            PostHog.reloadFeatureFlagsRequest()
 //            PostHog.isFeatureEnabled("sessionRecording")

diff --git a/posthog/api/posthog.api b/posthog/api/posthog.api
@@ -210,6 +210,7 @@ public abstract interface class com/posthog/internal/PostHogPreferences {
 	public static final field BUILD Ljava/lang/String;
 	public static final field Companion Lcom/posthog/internal/PostHogPreferences$Companion;
 	public static final field GROUPS Ljava/lang/String;
+	public static final field STRINGIFIED_KEYS Ljava/lang/String;
 	public static final field VERSION Ljava/lang/String;
 	public abstract fun clear (Ljava/util/List;)V
 	public abstract fun getAll ()Ljava/util/Map;
@@ -221,7 +222,9 @@ public abstract interface class com/posthog/internal/PostHogPreferences {
 public final class com/posthog/internal/PostHogPreferences$Companion {
 	public static final field BUILD Ljava/lang/String;
 	public static final field GROUPS Ljava/lang/String;
+	public static final field STRINGIFIED_KEYS Ljava/lang/String;
 	public static final field VERSION Ljava/lang/String;
+	public final fun getALL_INTERNAL_KEYS ()Ljava/util/Set;
 }
 
 public final class com/posthog/internal/PostHogPreferences$DefaultImpls {

diff --git a/posthog/src/main/java/com/posthog/PostHog.kt b/posthog/src/main/java/com/posthog/PostHog.kt
@@ -4,6 +4,8 @@ import com.posthog.internal.PostHogApi
 import com.posthog.internal.PostHogCalendarDateProvider
 import com.posthog.internal.PostHogFeatureFlags
 import com.posthog.internal.PostHogMemoryPreferences
+import com.posthog.internal.PostHogPreferences
+import com.posthog.internal.PostHogPreferences.Companion.ALL_INTERNAL_KEYS
 import com.posthog.internal.PostHogPreferences.Companion.ANONYMOUS_ID
 import com.posthog.internal.PostHogPreferences.Companion.BUILD
 import com.posthog.internal.PostHogPreferences.Companion.DISTINCT_ID
@@ -57,15 +59,15 @@ public class PostHog private constructor(
                     config.logger.log("API Key: ${config.apiKey} already has a PostHog instance.")
                 }
 
-                val cachePreferences = config.cachePreferences ?: PostHogMemoryPreferences()
+                val cachePreferences = config.cachePreferences ?: memoryPreferences
                 config.cachePreferences = cachePreferences
                 val dateProvider = PostHogCalendarDateProvider()
                 val api = PostHogApi(config, dateProvider)
                 val queue = PostHogQueue(config, api, dateProvider, queueExecutor)
                 val featureFlags = PostHogFeatureFlags(config, api, featureFlagsExecutor)
 
                 // no need to lock optOut here since the setup is locked already
-                val optOut = config.cachePreferences?.getValue(
+                val optOut = getPreferences().getValue(
                     OPT_OUT,
                     defaultValue = config.optOut,
                 ) as? Boolean
@@ -110,8 +112,12 @@ public class PostHog private constructor(
         }
     }
 
+    private fun getPreferences(): PostHogPreferences {
+        return config?.cachePreferences ?: memoryPreferences
+    }
+
     private fun legacyPreferences(config: PostHogConfig, serializer: PostHogSerializer) {
-        val cachedPrefs = config.cachePreferences?.getValue(config.apiKey) as? String
+        val cachedPrefs = getPreferences().getValue(config.apiKey) as? String
         cachedPrefs?.let {
             try {
                 serializer.deserialize<Map<String, Any>?>(it.reader())?.let { props ->
@@ -125,7 +131,7 @@ public class PostHog private constructor(
                         this.distinctId = distId
                     }
 
-                    config.cachePreferences?.remove(config.apiKey)
+                    getPreferences().remove(config.apiKey)
                 }
             } catch (e: Throwable) {
                 config.logger.log("Legacy cached prefs: $cachedPrefs failed to parse: $e.")
@@ -164,7 +170,7 @@ public class PostHog private constructor(
         get() {
             var anonymousId: String?
             synchronized(anonymousLock) {
-                anonymousId = config?.cachePreferences?.getValue(ANONYMOUS_ID) as? String
+                anonymousId = getPreferences().getValue(ANONYMOUS_ID) as? String
                 if (anonymousId == null) {
                     anonymousId = UUID.randomUUID().toString()
                     this.anonymousId = anonymousId ?: ""
@@ -173,18 +179,18 @@ public class PostHog private constructor(
             return anonymousId ?: ""
         }
         set(value) {
-            config?.cachePreferences?.setValue(ANONYMOUS_ID, value)
+            getPreferences().setValue(ANONYMOUS_ID, value)
         }
 
     private var distinctId: String
         get() {
-            return config?.cachePreferences?.getValue(
+            return getPreferences().getValue(
                 DISTINCT_ID,
                 defaultValue = anonymousId,
             ) as? String ?: ""
         }
         set(value) {
-            config?.cachePreferences?.setValue(DISTINCT_ID, value)
+            getPreferences().setValue(DISTINCT_ID, value)
         }
 
     private fun buildProperties(
@@ -197,7 +203,7 @@ public class PostHog private constructor(
         val props = mutableMapOf<String, Any>()
 
         if (appendSharedProps) {
-            val registeredPrefs = memoryPreferences.getAll()
+            val registeredPrefs = getPreferences().getAll()
             if (registeredPrefs.isNotEmpty()) {
                 props.putAll(registeredPrefs)
             }
@@ -287,7 +293,7 @@ public class PostHog private constructor(
 
         synchronized(lockOptOut) {
             config?.optOut = false
-            config?.cachePreferences?.setValue(OPT_OUT, false)
+            getPreferences().setValue(OPT_OUT, false)
         }
     }
 
@@ -298,7 +304,7 @@ public class PostHog private constructor(
 
         synchronized(lockOptOut) {
             config?.optOut = true
-            config?.cachePreferences?.setValue(OPT_OUT, true)
+            getPreferences().setValue(OPT_OUT, true)
         }
     }
 
@@ -393,8 +399,7 @@ public class PostHog private constructor(
             props["\$group_set"] = it
         }
 
-        // just defensive, if there's no cachePreferences, we fallback to in memory
-        val preferences = config?.cachePreferences ?: memoryPreferences
+        val preferences = getPreferences()
 
         @Suppress("UNCHECKED_CAST")
         val groups = preferences.getValue(GROUPS) as? Map<String, Any>
@@ -429,11 +434,8 @@ public class PostHog private constructor(
     }
 
     private fun loadFeatureFlagsRequest(onFeatureFlags: PostHogOnFeatureFlags?) {
-        // just defensive, if there's no config.cachePreferences, we fallback to in memory
-        val preferences = config?.cachePreferences ?: memoryPreferences
-
         @Suppress("UNCHECKED_CAST")
-        val groups = preferences.getValue(GROUPS) as? Map<String, Any>
+        val groups = getPreferences().getValue(GROUPS) as? Map<String, Any>
 
         featureFlags?.loadFeatureFlags(distinctId, anonymousId, groups, onFeatureFlags)
     }
@@ -496,8 +498,7 @@ public class PostHog private constructor(
         // only remove properties, preserve BUILD and VERSION keys in order to to fix over-sending
         // of 'Application Installed' events and under-sending of 'Application Updated' events
         val except = listOf(VERSION, BUILD)
-        memoryPreferences.clear(except = except)
-        config?.cachePreferences?.clear(except = except)
+        getPreferences().clear(except = except)
         featureFlags?.clear()
         queue?.clear()
         featureFlagsCalled.clear()
@@ -514,14 +515,18 @@ public class PostHog private constructor(
         if (!isEnabled()) {
             return
         }
-        memoryPreferences.setValue(key, value)
+        if (ALL_INTERNAL_KEYS.contains(key)) {
+            config?.logger?.log("Key: $key is reserved for internal use.")
+            return
+        }
+        getPreferences().setValue(key, value)
     }
 
     public override fun unregister(key: String) {
         if (!isEnabled()) {
             return
         }
-        memoryPreferences.remove(key)
+        getPreferences().remove(key)
     }
 
     override fun distinctId(): String {

diff --git a/posthog/src/main/java/com/posthog/PostHogInterface.kt b/posthog/src/main/java/com/posthog/PostHogInterface.kt
@@ -129,14 +129,17 @@ public interface PostHogInterface {
     public fun isOptOut(): Boolean
 
     /**
-     * Register a property to always be sent within this session
+     * Register a property to always be sent with all the following events until you call
+     * [unregister] with the same key
+     * PostHogPreferences.ALL_INTERNAL_KEYS are not allowed since they are internal and used by
+     * the SDK only.
      * @param key the Key
      * @param value the Value
      */
     public fun register(key: String, value: Any)
 
     /**
-     * Unregisters the previously set property to be sent within this session
+     * Unregisters the previously set property to be sent with all the following events
      * @param key the Key
      */
     public fun unregister(key: String)

diff --git a/posthog/src/main/java/com/posthog/internal/PostHogMemoryPreferences.kt b/posthog/src/main/java/com/posthog/internal/PostHogMemoryPreferences.kt
@@ -47,6 +47,8 @@ public class PostHogMemoryPreferences : PostHogPreferences {
         synchronized(lock) {
             props = preferences.toMap()
         }
-        return props
+        return props.filterKeys { key ->
+            !PostHogPreferences.ALL_INTERNAL_KEYS.contains(key)
+        }
     }
 }
diff --git a/posthog/src/main/java/com/posthog/internal/PostHogPreferences.kt b/posthog/src/main/java/com/posthog/internal/PostHogPreferences.kt
@@ -25,5 +25,16 @@ public interface PostHogPreferences {
         internal const val OPT_OUT = "opt-out"
         public const val VERSION: String = "version"
         public const val BUILD: String = "build"
+        public const val STRINGIFIED_KEYS: String = "stringifiedKeys"
+
+        public val ALL_INTERNAL_KEYS: Set<String> = setOf(
+            GROUPS,
+            ANONYMOUS_ID,
+            DISTINCT_ID,
+            OPT_OUT,
+            VERSION,
+            BUILD,
+            STRINGIFIED_KEYS,
+        )
     }
 }