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

Cache registered properties #51

Merged
merged 6 commits into from
Oct 18, 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: 2 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
name: 'Build & Test'
on:
push:
branches:
- main
pull_request:

jobs:
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/gradle-wrapper-validation.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
name: 'Validate Gradle Wrapper'
on:
push:
branches:
- main
pull_request:

jobs:
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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.")
Expand All @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
3 changes: 3 additions & 0 deletions posthog/api/posthog.api
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down
47 changes: 26 additions & 21 deletions posthog/src/main/java/com/posthog/PostHog.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 ->
Expand All @@ -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.")
Expand Down Expand Up @@ -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 ?: ""
Expand All @@ -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(
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
}
}

Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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()
Expand All @@ -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 {
Expand Down
7 changes: 5 additions & 2 deletions posthog/src/main/java/com/posthog/PostHogInterface.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
}
}
Loading