From 2c0965c99457c393e38ae37ecda331bd3c1c65a5 Mon Sep 17 00:00:00 2001 From: Javi Aguilar <122741+itsjavi@users.noreply.github.com> Date: Fri, 28 Aug 2026 12:59:31 +0000 Subject: [PATCH 1/4] feat: setEmbeddedData() + clearEmbeddedData() for app surveys [ENG-2472] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A host app can attach context to future responses without tying it to a trigger. `track()` takes a name and nothing else, so today the only way to get context onto a response is to declare it in the survey and have the respondent type it. Formbricks.setEmbeddedData(mapOf( "screen" to EmbeddedDataValue.string("checkout"), "plan" to EmbeddedDataValue.string("pro"), )) Formbricks.setEmbeddedData(mapOf("screen" to null)) // remove one key Formbricks.clearEmbeddedData("plan") // same, explicitly Formbricks.clearEmbeddedData() // everything Merge, never replace, so refreshing a volatile field cannot wipe a stable one. `null` removes a key; a key left out is untouched, which is how a host skips a field it has no value for this screen. The single-key and clear-everything forms are separate overloads, so a non-null `String` parameter means a host reading the key from its own state cannot accidentally wipe the bag. In-memory and never persisted: persisting would blur the Embedded Data ↔ contact-attribute boundary and create a PII-at-rest surface. Cleared on an identity switch and on logout so one user's context cannot ride onto the next user's responses on a shared device; kept on first identification, because a host legitimately pushes context before it knows who the user is. Callable before setup(), unlike every other public method: a host that pushes context at launch must not have the value dropped because initialization had not finished. Snapshotted in loadHtml(), which runs when the survey is actually presented after any configured delay, and frozen for its lifetime. The bag rides the props payload that already exists, under `hiddenFieldsRecord` — no new bridge message, and deliberately so: a setEmbeddedData after display must not reach the survey on screen. It is passed raw and unfiltered, because the ingest contract lives in the renderer and the server re-runs all of it. A non-finite number is logged and skipped: it would serialize as a bare NaN or Infinity, which is not valid JSON, so JSON.parse in the WebView would throw and no survey would render at all. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018L6PrECN38Jbe1FEdFccTz --- .../EmbeddedDataManagerInstrumentedTest.kt | 320 ++++++++++++++++++ .../java/com/formbricks/android/Formbricks.kt | 63 ++++ .../android/manager/EmbeddedDataManager.kt | 97 ++++++ .../model/embeddeddata/EmbeddedDataValue.kt | 37 ++ .../android/webview/FormbricksViewModel.kt | 7 + 5 files changed, 524 insertions(+) create mode 100644 android/src/androidTest/java/com/formbricks/android/manager/EmbeddedDataManagerInstrumentedTest.kt create mode 100644 android/src/main/java/com/formbricks/android/manager/EmbeddedDataManager.kt create mode 100644 android/src/main/java/com/formbricks/android/model/embeddeddata/EmbeddedDataValue.kt diff --git a/android/src/androidTest/java/com/formbricks/android/manager/EmbeddedDataManagerInstrumentedTest.kt b/android/src/androidTest/java/com/formbricks/android/manager/EmbeddedDataManagerInstrumentedTest.kt new file mode 100644 index 0000000..07cd003 --- /dev/null +++ b/android/src/androidTest/java/com/formbricks/android/manager/EmbeddedDataManagerInstrumentedTest.kt @@ -0,0 +1,320 @@ +package com.formbricks.android.manager + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import com.formbricks.android.Formbricks +import com.formbricks.android.extensions.dateString +import com.formbricks.android.model.embeddeddata.EmbeddedDataValue +import com.formbricks.android.network.queue.UpdateQueue +import com.google.gson.JsonParser +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import java.util.Date +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit + +/** + * The Embedded Data bag (ENG-1844 / ENG-2472): host-supplied context attached to future responses + * without tying it to a trigger. These pin the contract all four SDKs share, so a divergence here is + * a divergence from the JS SDK too. + */ +@RunWith(AndroidJUnit4::class) +class EmbeddedDataManagerInstrumentedTest { + + @Before + fun setUp() { + Formbricks.applicationContext = InstrumentationRegistry.getInstrumentation().targetContext + Formbricks.isInitialized = false + UserManager.logout() + UpdateQueue.reset() + EmbeddedDataManager.clear() + } + + /** The snapshot as plain strings — `asString` renders numbers and booleans too, so one + * comparison shape covers every value type without quoting noise. */ + private fun snapshotMap(): Map = + EmbeddedDataManager.snapshot().entrySet().associate { it.key to it.value.asString } + + // region Merge semantics + + @Test + fun mergesInsteadOfReplacing() { + Formbricks.setEmbeddedData( + mapOf( + "plan" to EmbeddedDataValue.string("pro"), + "screen" to EmbeddedDataValue.string("product") + ) + ) + Formbricks.setEmbeddedData(mapOf("screen" to EmbeddedDataValue.string("checkout"))) + + assertEquals(mapOf("plan" to "pro", "screen" to "checkout"), snapshotMap()) + } + + @Test + fun nullRemovesTheKey() { + Formbricks.setEmbeddedData( + mapOf( + "plan" to EmbeddedDataValue.string("pro"), + "screen" to EmbeddedDataValue.string("product") + ) + ) + Formbricks.setEmbeddedData(mapOf("screen" to null)) + + assertEquals(mapOf("plan" to "pro"), snapshotMap()) + } + + @Test + fun lastWriteWinsPerKey() { + Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("free"))) + Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("pro"))) + + assertEquals(mapOf("plan" to "pro"), snapshotMap()) + } + + @Test + fun omittedKeysAreUntouched() { + // Kotlin has no `undefined`, so "skip this field" is spelled by leaving the key out - and + // that must not disturb what an earlier call set. `null` is the explicit "remove" spelling. + Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("pro"))) + Formbricks.setEmbeddedData(mapOf("seats" to EmbeddedDataValue.number(4.0))) + + assertEquals(mapOf("plan" to "pro", "seats" to "4.0"), snapshotMap()) + } + + // endregion + + // region Clearing + + @Test + fun clearOneKeyLeavesTheRest() { + Formbricks.setEmbeddedData( + mapOf( + "plan" to EmbeddedDataValue.string("pro"), + "screen" to EmbeddedDataValue.string("product") + ) + ) + + Formbricks.clearEmbeddedData("screen") + + assertEquals(mapOf("plan" to "pro"), snapshotMap()) + } + + @Test + fun clearingAnUnsetKeyIsANoOp() { + Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("pro"))) + + Formbricks.clearEmbeddedData("neverSet") + + assertEquals(mapOf("plan" to "pro"), snapshotMap()) + } + + @Test + fun clearEverything() { + Formbricks.setEmbeddedData( + mapOf( + "plan" to EmbeddedDataValue.string("pro"), + "screen" to EmbeddedDataValue.string("product") + ) + ) + + Formbricks.clearEmbeddedData() + + assertTrue(snapshotMap().isEmpty()) + } + + // endregion + + // region Value types + + @Test + fun everyScalarSurvivesInItsJsonForm() { + val signedUpAt = Date(1_787_000_000_000L) + + Formbricks.setEmbeddedData( + mapOf( + "plan" to EmbeddedDataValue.string("pro"), + "seats" to EmbeddedDataValue.number(25.0), + "isTrial" to EmbeddedDataValue.boolean(false), + "signedUpAt" to EmbeddedDataValue.date(signedUpAt) + ) + ) + + val json = EmbeddedDataManager.snapshot() + assertEquals("pro", json.get("plan").asString) + assertEquals(25.0, json.get("seats").asDouble, 0.0) + assertFalse(json.get("isTrial").asBoolean) + // ISO 8601 is what the renderer's ingest contract accepts for a `date` field. + assertEquals(signedUpAt.dateString(), json.get("signedUpAt").asString) + } + + @Test + fun aSnapshotIsAlwaysParseableJson() { + // The snapshot is embedded in the survey WebView's payload and parsed there with + // JSON.parse. If it were ever malformed, the failure would not be a missing field - it + // would be no survey at all. + Formbricks.setEmbeddedData( + mapOf( + "plan" to EmbeddedDataValue.string("pro"), + "quote" to EmbeddedDataValue.string("he said \"hi\""), + "seats" to EmbeddedDataValue.number(25.0), + "isTrial" to EmbeddedDataValue.boolean(true), + "signedUpAt" to EmbeddedDataValue.date(Date()) + ) + ) + + val parsed = JsonParser.parseString(EmbeddedDataManager.snapshot().toString()) + assertTrue(parsed.isJsonObject) + assertEquals("he said \"hi\"", parsed.asJsonObject.get("quote").asString) + } + + @Test + fun aNonFiniteNumberIsSkippedRatherThanCostingTheSurvey() { + // THE guard: a bare NaN or Infinity is not valid JSON, so JSON.parse in the WebView would + // throw and no survey would render. Dropping the key is the only safe answer. + Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("pro"))) + + Formbricks.setEmbeddedData( + mapOf( + "broken" to EmbeddedDataValue.number(Double.NaN), + "alsoBroken" to EmbeddedDataValue.number(Double.POSITIVE_INFINITY) + ) + ) + + assertEquals(mapOf("plan" to "pro"), snapshotMap()) + assertTrue(JsonParser.parseString(EmbeddedDataManager.snapshot().toString()).isJsonObject) + } + + // endregion + + // region Lifetime + + @Test + fun snapshotIsDetachedFromLaterWrites() { + // What "a value set after a survey is displayed does not change that response" rests on: + // the WebView payload holds this object for the life of the survey. + Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("pro"))) + val snapshot = EmbeddedDataManager.snapshot() + + Formbricks.setEmbeddedData( + mapOf( + "plan" to EmbeddedDataValue.string("enterprise"), + "extra" to EmbeddedDataValue.string("later") + ) + ) + + assertEquals("pro", snapshot.get("plan").asString) + assertFalse(snapshot.has("extra")) + } + + @Test + fun worksBeforeSetup() { + // Deliberately unlike the other public methods: a host that pushes context at launch must + // not have the value dropped because initialization had not finished yet. + assertFalse(Formbricks.isInitialized) + + Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("pro"))) + + assertEquals(mapOf("plan" to "pro"), snapshotMap()) + } + + @Test + fun isNotPersisted() { + // A cold start begins empty. Nothing host-supplied may reach SharedPreferences, where it + // would outlive the session and blur the Embedded Data / contact-attribute boundary. + val marker = "fb-embedded-probe-${System.nanoTime()}" + val context = InstrumentationRegistry.getInstrumentation().targetContext + + Formbricks.setEmbeddedData(mapOf("probe" to EmbeddedDataValue.string(marker))) + + val prefsDir = java.io.File(context.applicationInfo.dataDir, "shared_prefs") + val files = prefsDir.listFiles() ?: emptyArray() + for (file in files) { + assertFalse( + "${file.name} holds Embedded Data - the bag must stay in memory", + file.readText().contains(marker) + ) + } + } + + // endregion + + // region Identity changes + + @Test + fun switchingUserClearsTheBag() { + Formbricks.isInitialized = true + Formbricks.setUserId("user-a") + Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("pro"))) + + Formbricks.setUserId("user-b") + + assertTrue(snapshotMap().isEmpty()) + } + + @Test + fun firstIdentificationKeepsTheBag() { + // The host pushes context before it knows who the user is - that is the normal order, and + // clearing here would throw away the value the API exists to carry. + Formbricks.isInitialized = true + Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("pro"))) + + Formbricks.setUserId("user-a") + + assertEquals(mapOf("plan" to "pro"), snapshotMap()) + } + + @Test + fun settingTheSameUserIdKeepsTheBag() { + Formbricks.isInitialized = true + Formbricks.setUserId("user-a") + Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("pro"))) + + Formbricks.setUserId("user-a") + + assertEquals(mapOf("plan" to "pro"), snapshotMap()) + } + + @Test + fun logoutClearsTheBag() { + Formbricks.isInitialized = true + Formbricks.setUserId("user-a") + Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("pro"))) + + Formbricks.logout() + + assertTrue(snapshotMap().isEmpty()) + } + + // endregion + + @Test + fun concurrentWritesDoNotCorruptTheBag() { + // The host may call from any thread while the main thread reads the snapshot to present a + // survey. Without the lock this trips ConcurrentModificationException. + val threads = 8 + val perThread = 200 + val pool = Executors.newFixedThreadPool(threads) + val done = CountDownLatch(threads) + + repeat(threads) { threadIndex -> + pool.execute { + repeat(perThread) { i -> + Formbricks.setEmbeddedData( + mapOf("key$threadIndex" to EmbeddedDataValue.number(i.toDouble())) + ) + EmbeddedDataManager.snapshot() + } + done.countDown() + } + } + + assertTrue(done.await(30, TimeUnit.SECONDS)) + pool.shutdown() + assertEquals(threads, snapshotMap().size) + } +} diff --git a/android/src/main/java/com/formbricks/android/Formbricks.kt b/android/src/main/java/com/formbricks/android/Formbricks.kt index 4560242..6282c09 100644 --- a/android/src/main/java/com/formbricks/android/Formbricks.kt +++ b/android/src/main/java/com/formbricks/android/Formbricks.kt @@ -10,8 +10,10 @@ import androidx.fragment.app.FragmentManager import com.formbricks.android.api.FormbricksApi import com.formbricks.android.helper.FormbricksConfig import com.formbricks.android.logger.Logger +import com.formbricks.android.manager.EmbeddedDataManager import com.formbricks.android.manager.SurveyManager import com.formbricks.android.manager.UserManager +import com.formbricks.android.model.embeddeddata.EmbeddedDataValue import com.formbricks.android.model.error.SDKError import com.formbricks.android.model.user.AttributeValue import com.formbricks.android.webview.FormbricksFragment @@ -137,6 +139,11 @@ object Formbricks { if (existing != null && existing.isNotEmpty()) { Logger.d("Different userId is being set, cleaning up previous user state") UserManager.logout() + // An identity switch: the ambient Embedded Data bag may carry the previous user's + // context, which must not ride onto the next user's responses on a shared device. + // First-time identification keeps the bag - a host legitimately pushes context before + // it knows who the user is. + EmbeddedDataManager.clear() } UserManager.set(userId) @@ -306,6 +313,62 @@ object Formbricks { } UserManager.logout() + // Same identity-switch rule as setUserId: logout must not let the previous user's ambient + // context leak onto whoever uses the app next. + EmbeddedDataManager.clear() + } + + /** + * Attaches Embedded Data to future responses without tying it to a trigger. + * + * Merges into an in-memory bag - last write wins per key, and an explicit `null` removes a key. + * Values land only on the survey's declared *ingested* fields; anything else is dropped and + * logged by the survey renderer, never fatal. + * + * Deliberately callable **before** [setup], unlike the methods above: a host that pushes context + * at launch must not have that value silently dropped because initialization had not finished. + * The bag is pure memory - nothing here needs the SDK to be running. + * + * The bag is snapshotted when a survey is displayed and frozen for its lifetime, so a value set + * while a survey is on screen reaches the *next* response, not that one. It is never persisted: + * a cold app start begins empty and the host re-pushes. + * + * ```kotlin + * Formbricks.setEmbeddedData(mapOf( + * "plan" to EmbeddedDataValue.string("pro"), + * "seats" to EmbeddedDataValue.number(25.0), + * "screen" to null, // removes the key + * )) + * ``` + */ + fun setEmbeddedData(data: Map) { + EmbeddedDataManager.set(data) + } + + /** + * Removes one Embedded Data key. A key that was never set is a no-op. + * + * The single-key and clear-everything forms are separate overloads on purpose: a `String` that + * cannot be null means a host reading the key from its own state cannot accidentally wipe the + * whole bag. + * + * ```kotlin + * Formbricks.clearEmbeddedData("plan") + * ``` + */ + fun clearEmbeddedData(key: String) { + EmbeddedDataManager.remove(key) + } + + /** + * Clears the whole Embedded Data bag - logout, or a hard context switch. + * + * ```kotlin + * Formbricks.clearEmbeddedData() + * ``` + */ + fun clearEmbeddedData() { + EmbeddedDataManager.clear() } /** diff --git a/android/src/main/java/com/formbricks/android/manager/EmbeddedDataManager.kt b/android/src/main/java/com/formbricks/android/manager/EmbeddedDataManager.kt new file mode 100644 index 0000000..cec1abc --- /dev/null +++ b/android/src/main/java/com/formbricks/android/manager/EmbeddedDataManager.kt @@ -0,0 +1,97 @@ +package com.formbricks.android.manager + +import com.formbricks.android.extensions.dateString +import com.formbricks.android.logger.Logger +import com.formbricks.android.model.embeddeddata.EmbeddedDataValue +import com.google.gson.JsonObject + +/** + * The in-memory Embedded Data bag: context a host app attaches to future responses without tying it + * to a trigger — `Formbricks.setEmbeddedData(mapOf("screen" to ...))` once, instead of repeating the + * same values on every possible `track(...)` call. + * + * Mirrors the JS SDK's store key for key, so web and mobile behave identically. + * + * Lifetime rules, all deliberate: + * + * - **In-memory, process scoped, never persisted.** Not `SharedPreferences`: persisting this bag + * would blur the Embedded Data ↔ contact-attribute boundary and create a stale-data / PII-at-rest + * surface. A cold app start begins empty; the host re-pushes. + * - **Snapshot at display, then frozen.** [FormbricksViewModel][com.formbricks.android.webview.FormbricksViewModel] + * copies the bag into the WebView payload when the survey is shown, so a later `setEmbeddedData` + * affects the next response, never the one on screen. + * - **No filtering here.** The SDK is a dumb pipe: the survey renderer applies the ingest contract — + * allow-list, coercion, `locked`, size caps — and logs what it refuses, and the server re-runs all + * of it on ingest. Filtering here would ship a second copy of those rules for the four mobile SDKs + * to drift from. + * - **Independent of `setup`.** A host legitimately pushes context before the SDK finishes + * initializing, and silently dropping that write is the failure this API exists to avoid. + * - **No network.** Every method is a synchronous memory write, so calling it on every screen change + * is free. Values ride the existing response payload. + */ +object EmbeddedDataManager { + private val lock = Any() + private val data = LinkedHashMap() + + /** + * Merge — never replace — so refreshing a volatile field (`screen`) cannot wipe the stable ones + * (`plan`) set at launch. Per key: last write wins, and an explicit `null` removes the key. + * + * A key the caller simply leaves out is untouched; that is how a host skips a field it has no + * value for this screen. `null` is the deliberate "remove this" spelling, matching the JS SDK's + * `{ key: null }`. + */ + fun set(values: Map) { + synchronized(lock) { + for ((key, value) in values) { + if (value == null) { + data.remove(key) + continue + } + // Refused rather than stored: a non-finite Double serializes as bare `NaN` or + // `Infinity`, which is not valid JSON, so `JSON.parse` in the WebView would throw + // and the survey would never render. One bad value must cost the field, not the + // survey. Never fatal, always logged. + if (value is EmbeddedDataValue.NumberValue && !value.value.isFinite()) { + Logger.w("setEmbeddedData: \"$key\" is not a finite number - the key was skipped") + continue + } + data[key] = value + } + } + } + + /** Removes one key. A key that is not set is a no-op. */ + fun remove(key: String) { + synchronized(lock) { + data.remove(key) + } + } + + /** Removes everything - logout, or a hard context switch. */ + fun clear() { + synchronized(lock) { + data.clear() + } + } + + /** + * A detached, JSON-safe copy for the display-time snapshot: mutating the bag after a survey has + * rendered must not reach that survey's response. + */ + fun snapshot(): JsonObject { + val json = JsonObject() + synchronized(lock) { + for ((key, value) in data) { + when (value) { + is EmbeddedDataValue.StringValue -> json.addProperty(key, value.value) + is EmbeddedDataValue.NumberValue -> json.addProperty(key, value.value) + is EmbeddedDataValue.BooleanValue -> json.addProperty(key, value.value) + // ISO 8601 is what the renderer's ingest contract accepts for a `date` field. + is EmbeddedDataValue.DateValue -> json.addProperty(key, value.value.dateString()) + } + } + } + return json + } +} diff --git a/android/src/main/java/com/formbricks/android/model/embeddeddata/EmbeddedDataValue.kt b/android/src/main/java/com/formbricks/android/model/embeddeddata/EmbeddedDataValue.kt new file mode 100644 index 0000000..3ae1cc2 --- /dev/null +++ b/android/src/main/java/com/formbricks/android/model/embeddeddata/EmbeddedDataValue.kt @@ -0,0 +1,37 @@ +package com.formbricks.android.model.embeddeddata + +import java.util.Date + +/** + * A value a host app may attach to future responses with + * [com.formbricks.android.Formbricks.setEmbeddedData]. + * + * Confined to the four scalars the Embedded Data ingest contract can store. A sealed class rather + * than `Any` on purpose: the bag is serialized into the survey WebView's payload, so an + * unrepresentable value would not be a dropped field but a malformed payload that takes the whole + * survey down with it. + * + * ```kotlin + * Formbricks.setEmbeddedData(mapOf( + * "plan" to EmbeddedDataValue.string("pro"), + * "seats" to EmbeddedDataValue.number(25.0), + * "isTrial" to EmbeddedDataValue.boolean(false), + * "screen" to null, // removes the key + * )) + * ``` + * + * Dates serialize as ISO 8601, which is what the ingest contract accepts for a `date` field. + */ +sealed class EmbeddedDataValue { + data class StringValue(val value: String) : EmbeddedDataValue() + data class NumberValue(val value: Double) : EmbeddedDataValue() + data class BooleanValue(val value: Boolean) : EmbeddedDataValue() + data class DateValue(val value: Date) : EmbeddedDataValue() + + companion object { + fun string(value: String): EmbeddedDataValue = StringValue(value) + fun number(value: Double): EmbeddedDataValue = NumberValue(value) + fun boolean(value: Boolean): EmbeddedDataValue = BooleanValue(value) + fun date(value: Date): EmbeddedDataValue = DateValue(value) + } +} diff --git a/android/src/main/java/com/formbricks/android/webview/FormbricksViewModel.kt b/android/src/main/java/com/formbricks/android/webview/FormbricksViewModel.kt index 092207a..3b0a491 100644 --- a/android/src/main/java/com/formbricks/android/webview/FormbricksViewModel.kt +++ b/android/src/main/java/com/formbricks/android/webview/FormbricksViewModel.kt @@ -7,6 +7,7 @@ import androidx.lifecycle.MutableLiveData import androidx.lifecycle.ViewModel import com.formbricks.android.Formbricks import com.formbricks.android.extensions.guard +import com.formbricks.android.manager.EmbeddedDataManager import com.formbricks.android.manager.SurveyManager import com.formbricks.android.manager.UserManager import com.formbricks.android.model.workspace.WorkspaceDataHolder @@ -149,6 +150,12 @@ class FormbricksViewModel : ViewModel() { jsonObject.addProperty("environmentId", Formbricks.workspaceId) jsonObject.addProperty("contactId", UserManager.contactId) jsonObject.addProperty("isWebEnvironment", false) + // The Embedded Data bag, snapshotted here - loadHtml runs when the survey is actually + // presented, after any configured delay - and frozen for the survey's life. Passed raw and + // unfiltered: the ingest contract (allow-list, coercion, `locked`, size caps) lives in the + // renderer, so all four mobile SDKs inherit the same rules without each shipping a copy, and + // the server re-runs all of it on ingest. + jsonObject.add("hiddenFieldsRecord", EmbeddedDataManager.snapshot()) val matchedSurvey = workspaceDataHolder.data?.data?.surveys?.firstOrNull { it.id == surveyId } val settings = workspaceDataHolder.data?.data?.settings From 49f9db96254868228b578db5c205e744bd4915e8 Mon Sep 17 00:00:00 2001 From: Javi Aguilar <122741+itsjavi@users.noreply.github.com> Date: Fri, 28 Aug 2026 14:05:35 +0000 Subject: [PATCH 2/4] test: wait for identity to land before asserting the switch clears the bag [ENG-2472] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught what no local check could: `UserManager.set(userId)` only enqueues into the debounced UpdateQueue, so `UserManager.userId` is still null immediately after `Formbricks.setUserId`. The switch test therefore took the first-identification branch, where the bag is kept on purpose — asserting an empty bag against a code path that never ran. It also flipped `isInitialized` by hand instead of setting the SDK up, so no sync could ever complete. The production code is right and stays as it is: the clearing sits inside the SDK's own "a different userId is set" branch, so it is exactly as timely as the `UserManager.logout()` teardown beside it. The tests were asserting a state the SDK cannot reach that fast. The identity cases now run against a real `Formbricks.setup` with the mock API service and wait for the id to land — the same pattern as the SDK's own identity tests — so the switch and same-id cases exercise the branches they name. Also wraps the shared_prefs probe's file read, so an unreadable file fails the read rather than the test. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018L6PrECN38Jbe1FEdFccTz --- .../EmbeddedDataManagerInstrumentedTest.kt | 56 ++++++++++++++++--- 1 file changed, 48 insertions(+), 8 deletions(-) diff --git a/android/src/androidTest/java/com/formbricks/android/manager/EmbeddedDataManagerInstrumentedTest.kt b/android/src/androidTest/java/com/formbricks/android/manager/EmbeddedDataManagerInstrumentedTest.kt index 07cd003..af8975e 100644 --- a/android/src/androidTest/java/com/formbricks/android/manager/EmbeddedDataManagerInstrumentedTest.kt +++ b/android/src/androidTest/java/com/formbricks/android/manager/EmbeddedDataManagerInstrumentedTest.kt @@ -3,12 +3,16 @@ package com.formbricks.android.manager import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry import com.formbricks.android.Formbricks +import com.formbricks.android.MockFormbricksApiService +import com.formbricks.android.api.FormbricksApi import com.formbricks.android.extensions.dateString +import com.formbricks.android.helper.FormbricksConfig import com.formbricks.android.model.embeddeddata.EmbeddedDataValue import com.formbricks.android.network.queue.UpdateQueue import com.google.gson.JsonParser import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -26,15 +30,48 @@ import java.util.concurrent.TimeUnit @RunWith(AndroidJUnit4::class) class EmbeddedDataManagerInstrumentedTest { + private val workspaceId = "workspaceId" + private val appUrl = "https://example.com" + @Before fun setUp() { Formbricks.applicationContext = InstrumentationRegistry.getInstrumentation().targetContext Formbricks.isInitialized = false UserManager.logout() UpdateQueue.reset() + SurveyManager.workspaceDataHolder = null + SurveyManager.filteredSurveys.clear() + FormbricksApi.service = MockFormbricksApiService() EmbeddedDataManager.clear() } + /** Initializes the SDK against the mock API service, so identity changes can run for real. */ + private fun setUpSdk() { + Formbricks.setup( + InstrumentationRegistry.getInstrumentation().targetContext, + FormbricksConfig.Builder(appUrl, workspaceId).setLoggingEnabled(true).build(), + ) + waitForSeconds(1) + } + + /** + * Identifies as [userId] and waits for the id to actually land. + * + * [Formbricks.setUserId] reads [UserManager.userId] to decide whether this is a switch, and that + * property is only written once the debounced [UpdateQueue] sync completes - `set(userId)` merely + * enqueues. Asserting the switch behaviour right after a bare `setUserId` would take the + * first-identification branch instead and pass for the wrong reason. + */ + private fun identify(userId: String) { + Formbricks.setUserId(userId) + waitForSeconds(2) + assertEquals(userId, UserManager.userId) + } + + private fun waitForSeconds(seconds: Long) { + CountDownLatch(1).await(seconds, TimeUnit.SECONDS) + } + /** The snapshot as plain strings — `asString` renders numbers and booleans too, so one * comparison shape covers every value type without quoting noise. */ private fun snapshotMap(): Map = @@ -234,9 +271,10 @@ class EmbeddedDataManagerInstrumentedTest { val prefsDir = java.io.File(context.applicationInfo.dataDir, "shared_prefs") val files = prefsDir.listFiles() ?: emptyArray() for (file in files) { + val contents = runCatching { file.readText() }.getOrDefault("") assertFalse( "${file.name} holds Embedded Data - the bag must stay in memory", - file.readText().contains(marker) + contents.contains(marker) ) } } @@ -247,8 +285,8 @@ class EmbeddedDataManagerInstrumentedTest { @Test fun switchingUserClearsTheBag() { - Formbricks.isInitialized = true - Formbricks.setUserId("user-a") + setUpSdk() + identify("user-a") Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("pro"))) Formbricks.setUserId("user-b") @@ -260,7 +298,10 @@ class EmbeddedDataManagerInstrumentedTest { fun firstIdentificationKeepsTheBag() { // The host pushes context before it knows who the user is - that is the normal order, and // clearing here would throw away the value the API exists to carry. - Formbricks.isInitialized = true + setUpSdk() + // `userId` is persisted, so an id left by an earlier test would make this take the switch + // branch. setUp() logs out, so this only pins the precondition the assertion depends on. + assertNull(UserManager.userId) Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("pro"))) Formbricks.setUserId("user-a") @@ -270,8 +311,8 @@ class EmbeddedDataManagerInstrumentedTest { @Test fun settingTheSameUserIdKeepsTheBag() { - Formbricks.isInitialized = true - Formbricks.setUserId("user-a") + setUpSdk() + identify("user-a") Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("pro"))) Formbricks.setUserId("user-a") @@ -281,8 +322,7 @@ class EmbeddedDataManagerInstrumentedTest { @Test fun logoutClearsTheBag() { - Formbricks.isInitialized = true - Formbricks.setUserId("user-a") + setUpSdk() Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("pro"))) Formbricks.logout() From 200855c51c3c79d16529c8750b74f19f5a875a76 Mon Sep 17 00:00:00 2001 From: Javi Aguilar <122741+itsjavi@users.noreply.github.com> Date: Fri, 28 Aug 2026 14:18:34 +0000 Subject: [PATCH 3/4] test: seed the persisted identity instead of setting the SDK up [ENG-2472] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second red run on the emulator job, and the runner's log tail is entirely harden-runner output, so the failing assertion is not reachable from here. Rather than guess a third time, this removes the machinery the identity tests did not need. `Formbricks.setup` is gone from this class: it fetched the workspace and ran the legacy-cache migration, writing state that other test classes assert on, and none of it is needed to exercise an identity switch. The debounced wait is gone too — identity lands only when a network sync completes, so waiting tests the UpdateQueue's timing as much as the branch it names. Seeding the SharedPreferences key the getter falls back to is exact: no timer, no request, and it models the honest scenario — the app relaunches already identified, then a different user signs in. `setUserId` then genuinely takes the switch branch, and the same-id case genuinely takes the early return. `appUrl`/`workspaceId` are assigned directly in setUp because they are lateinit: a queued update that found them unset would throw on the UpdateQueue's timer thread, and a throwing TimerTask cancels that Timer for the whole process — the same hazard the SDK's own comments call out. An @After now logs out and resets the queue so nothing this class starts can fire during another one. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018L6PrECN38Jbe1FEdFccTz --- .../EmbeddedDataManagerInstrumentedTest.kt | 68 +++++++++++-------- 1 file changed, 39 insertions(+), 29 deletions(-) diff --git a/android/src/androidTest/java/com/formbricks/android/manager/EmbeddedDataManagerInstrumentedTest.kt b/android/src/androidTest/java/com/formbricks/android/manager/EmbeddedDataManagerInstrumentedTest.kt index af8975e..ef10f50 100644 --- a/android/src/androidTest/java/com/formbricks/android/manager/EmbeddedDataManagerInstrumentedTest.kt +++ b/android/src/androidTest/java/com/formbricks/android/manager/EmbeddedDataManagerInstrumentedTest.kt @@ -1,12 +1,12 @@ package com.formbricks.android.manager +import android.content.Context import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry import com.formbricks.android.Formbricks import com.formbricks.android.MockFormbricksApiService import com.formbricks.android.api.FormbricksApi import com.formbricks.android.extensions.dateString -import com.formbricks.android.helper.FormbricksConfig import com.formbricks.android.model.embeddeddata.EmbeddedDataValue import com.formbricks.android.network.queue.UpdateQueue import com.google.gson.JsonParser @@ -14,6 +14,7 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNull import org.junit.Assert.assertTrue +import org.junit.After import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -37,41 +38,50 @@ class EmbeddedDataManagerInstrumentedTest { fun setUp() { Formbricks.applicationContext = InstrumentationRegistry.getInstrumentation().targetContext Formbricks.isInitialized = false + // Assigned directly rather than through `Formbricks.setup`: these tests need no workspace + // fetch, and running setup here would write the workspace cache that other classes assert + // on. `workspaceId`/`appUrl` are lateinit, so a queued update touching them must not find + // them unset - an exception on the UpdateQueue's timer thread cancels that Timer for the + // whole process, which would take later tests down with it. + Formbricks.appUrl = appUrl + Formbricks.workspaceId = workspaceId + FormbricksApi.service = MockFormbricksApiService() UserManager.logout() UpdateQueue.reset() - SurveyManager.workspaceDataHolder = null - SurveyManager.filteredSurveys.clear() - FormbricksApi.service = MockFormbricksApiService() EmbeddedDataManager.clear() } - /** Initializes the SDK against the mock API service, so identity changes can run for real. */ - private fun setUpSdk() { - Formbricks.setup( - InstrumentationRegistry.getInstrumentation().targetContext, - FormbricksConfig.Builder(appUrl, workspaceId).setLoggingEnabled(true).build(), - ) - waitForSeconds(1) + @After + fun tearDown() { + // Leave nothing running for the next class: logout cancels the sync task and resets the + // queue, so a debounced commit from an identity test cannot fire during someone else's. + UserManager.logout() + UpdateQueue.reset() + Formbricks.isInitialized = false + EmbeddedDataManager.clear() } /** - * Identifies as [userId] and waits for the id to actually land. + * Seeds a persisted identity, the way a previous app session would have left one. + * + * Not `Formbricks.setUserId`: that only enqueues into the debounced [UpdateQueue], so + * [UserManager.userId] stays null until a network sync lands, and a test driving it that way + * would silently take the first-identification branch and pass for the wrong reason. Writing the + * same key the getter reads is exact, needs no timer or request, and models the honest scenario + * - the app relaunches already identified, then a different user signs in. * - * [Formbricks.setUserId] reads [UserManager.userId] to decide whether this is a switch, and that - * property is only written once the debounced [UpdateQueue] sync completes - `set(userId)` merely - * enqueues. Asserting the switch behaviour right after a bare `setUserId` would take the - * first-identification branch instead and pass for the wrong reason. + * The key names are `UserManager`'s own private constants, repeated here because that is the + * storage contract this seeds; [assertEquals] below fails loudly if either ever changes. */ - private fun identify(userId: String) { - Formbricks.setUserId(userId) - waitForSeconds(2) + private fun seedPersistedUserId(userId: String) { + InstrumentationRegistry.getInstrumentation().targetContext + .getSharedPreferences("formbricks_prefs", Context.MODE_PRIVATE) + .edit() + .putString("userIdKey", userId) + .commit() assertEquals(userId, UserManager.userId) } - private fun waitForSeconds(seconds: Long) { - CountDownLatch(1).await(seconds, TimeUnit.SECONDS) - } - /** The snapshot as plain strings — `asString` renders numbers and booleans too, so one * comparison shape covers every value type without quoting noise. */ private fun snapshotMap(): Map = @@ -285,8 +295,8 @@ class EmbeddedDataManagerInstrumentedTest { @Test fun switchingUserClearsTheBag() { - setUpSdk() - identify("user-a") + Formbricks.isInitialized = true + seedPersistedUserId("user-a") Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("pro"))) Formbricks.setUserId("user-b") @@ -298,7 +308,7 @@ class EmbeddedDataManagerInstrumentedTest { fun firstIdentificationKeepsTheBag() { // The host pushes context before it knows who the user is - that is the normal order, and // clearing here would throw away the value the API exists to carry. - setUpSdk() + Formbricks.isInitialized = true // `userId` is persisted, so an id left by an earlier test would make this take the switch // branch. setUp() logs out, so this only pins the precondition the assertion depends on. assertNull(UserManager.userId) @@ -311,8 +321,8 @@ class EmbeddedDataManagerInstrumentedTest { @Test fun settingTheSameUserIdKeepsTheBag() { - setUpSdk() - identify("user-a") + Formbricks.isInitialized = true + seedPersistedUserId("user-a") Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("pro"))) Formbricks.setUserId("user-a") @@ -322,7 +332,7 @@ class EmbeddedDataManagerInstrumentedTest { @Test fun logoutClearsTheBag() { - setUpSdk() + Formbricks.isInitialized = true Formbricks.setEmbeddedData(mapOf("plan" to EmbeddedDataValue.string("pro"))) Formbricks.logout() From bdeb6fc39c83a6efe6af68724c54a125c34af4b0 Mon Sep 17 00:00:00 2001 From: Javi Aguilar <122741+itsjavi@users.noreply.github.com> Date: Mon, 31 Aug 2026 10:59:20 +0000 Subject: [PATCH 4/4] fix: serialize Embedded Data dates in ASCII on every device locale [ENG-2472] Two review findings. 1. `Date.dateString()` built its SimpleDateFormat with Locale.getDefault(), and SimpleDateFormat renders digits in the locale's own numbering system. On a device set to fa, fa-IR, ar-EG, hi-IN-u-nu-deva or ne-NP the "ISO 8601" string came out in non-ASCII digits, which the ingest contract's date parser will not accept - the value would be stored raw and flagged coercion_failed, for those users only. Locale.ROOT fixes it. Verified on the JVM: six locales produce non-ASCII digits with getDefault() and ASCII with ROOT, and the new test's regex accepts the ROOT output while rejecting both native-digit ones. Only the formatting direction needed it. The three parsers in that file keep Locale.getDefault(): checked, and DecimalFormat's Character.digit fallback reads the server's ASCII digits under fa and ar-EG alike, so they are not a live bug and are left alone. The pre-existing date assertion compared dateString() against dateString(), so it could not have caught this. The new test asserts the shape instead. 2. setEmbeddedData succeeded in silence. Mirrors the js-core debug trace from formbricks/formbricks#9091: keys set and removed, what the bag now holds, and the sentence that pre-empts the next question. Keys only, never values - the documented use of this bag includes hashed identity fields - and the message is built by an internal `setTrace` so that property is directly assertable rather than scraped from logcat. Built and logged outside the lock, so a log write never holds it. :android:compileDebugKotlin and :android:compileDebugAndroidTestKotlin both clean. The instrumented suite still runs only in CI; no emulator here. --- .../EmbeddedDataManagerInstrumentedTest.kt | 54 +++++++++++++++++++ .../android/extensions/DateExtensions.kt | 14 ++++- .../android/manager/EmbeddedDataManager.kt | 30 +++++++++++ 3 files changed, 97 insertions(+), 1 deletion(-) diff --git a/android/src/androidTest/java/com/formbricks/android/manager/EmbeddedDataManagerInstrumentedTest.kt b/android/src/androidTest/java/com/formbricks/android/manager/EmbeddedDataManagerInstrumentedTest.kt index ef10f50..3790d47 100644 --- a/android/src/androidTest/java/com/formbricks/android/manager/EmbeddedDataManagerInstrumentedTest.kt +++ b/android/src/androidTest/java/com/formbricks/android/manager/EmbeddedDataManagerInstrumentedTest.kt @@ -19,6 +19,7 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import java.util.Date +import java.util.Locale import java.util.concurrent.CountDownLatch import java.util.concurrent.Executors import java.util.concurrent.TimeUnit @@ -199,6 +200,59 @@ class EmbeddedDataManagerInstrumentedTest { assertEquals(signedUpAt.dateString(), json.get("signedUpAt").asString) } + @Test + fun aDateSerializesAsAsciiIso8601OnEveryDeviceLocale() { + // SimpleDateFormat renders digits in the locale's own numbering system, so a device set to + // Persian or to an Arabic locale with the arab numbering system would produce an "ISO 8601" + // string in non-ASCII digits. Nothing downstream accepts those: the ingest contract's date + // parser would refuse the value and store it raw as coercion_failed - for those users only, + // which is exactly the kind of bug that never shows up in testing. + val original = Locale.getDefault() + try { + for (locale in listOf(Locale("fa"), Locale.forLanguageTag("ar-EG-u-nu-arab"))) { + Locale.setDefault(locale) + EmbeddedDataManager.clear() + Formbricks.setEmbeddedData(mapOf("signedUpAt" to EmbeddedDataValue.date(Date()))) + + val serialized = EmbeddedDataManager.snapshot().get("signedUpAt").asString + assertTrue( + "under $locale the date serialized as \"$serialized\", which is not ASCII ISO 8601", + serialized.matches(Regex("""\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z""")) + ) + } + } finally { + Locale.setDefault(original) + } + } + + @Test + fun theSuccessTraceNamesKeysAndNeverValues() { + // The bag is otherwise invisible - memory-only, no getter - so this trace is a host's only + // confirmation that a write landed. Its one hard rule: the documented use of this bag + // includes hashed identity fields, so a value must never reach a log line. + val message = EmbeddedDataManager.setTrace( + setKeys = listOf("plan", "hashedEmail"), + removedKeys = listOf("screen"), + held = listOf("plan", "hashedEmail") + ) + + assertTrue(message.contains("set [plan, hashedEmail]")) + assertTrue(message.contains("removed [screen]")) + assertTrue(message.contains("the bag now holds [plan, hashedEmail]")) + assertTrue(message.contains("only if the survey declares them")) + } + + @Test + fun theSuccessTraceOmitsTheRemovedListWhenNothingWasRemoved() { + val message = EmbeddedDataManager.setTrace( + setKeys = listOf("plan"), + removedKeys = emptyList(), + held = listOf("plan") + ) + + assertFalse(message.contains("removed")) + } + @Test fun aSnapshotIsAlwaysParseableJson() { // The snapshot is embedded in the survey WebView's payload and parsed there with diff --git a/android/src/main/java/com/formbricks/android/extensions/DateExtensions.kt b/android/src/main/java/com/formbricks/android/extensions/DateExtensions.kt index 8508852..b137ddd 100644 --- a/android/src/main/java/com/formbricks/android/extensions/DateExtensions.kt +++ b/android/src/main/java/com/formbricks/android/extensions/DateExtensions.kt @@ -10,8 +10,20 @@ import java.util.TimeZone internal const val dateFormatPattern = "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'" +/** + * Formats as ISO 8601 UTC for the wire — a machine-facing value, never shown to a user. + * + * `Locale.ROOT`, not `Locale.getDefault()`: `SimpleDateFormat` renders digits in the locale's own + * numbering system, so on a device set to `fa`, `fa-IR`, `ar-EG`, `hi-IN-u-nu-deva` or `ne-NP` the + * "ISO 8601" string comes out in non-ASCII digits. Nothing downstream accepts those — an Embedded + * Data `date` field would be flagged `coercion_failed` and stored raw, for those users only. + * + * Only the formatting direction needs this. The parsers in this file keep `Locale.getDefault()` + * because `DecimalFormat` falls back to `Character.digit` and reads the server's ASCII digits under + * any locale. + */ fun Date.dateString(): String { - val dateFormat = SimpleDateFormat(dateFormatPattern, Locale.getDefault()) + val dateFormat = SimpleDateFormat(dateFormatPattern, Locale.ROOT) dateFormat.timeZone = TimeZone.getTimeZone("UTC") return dateFormat.format(this) } diff --git a/android/src/main/java/com/formbricks/android/manager/EmbeddedDataManager.kt b/android/src/main/java/com/formbricks/android/manager/EmbeddedDataManager.kt index cec1abc..7197688 100644 --- a/android/src/main/java/com/formbricks/android/manager/EmbeddedDataManager.kt +++ b/android/src/main/java/com/formbricks/android/manager/EmbeddedDataManager.kt @@ -42,10 +42,14 @@ object EmbeddedDataManager { * `{ key: null }`. */ fun set(values: Map) { + val setKeys = mutableListOf() + val removedKeys = mutableListOf() + val held: List synchronized(lock) { for ((key, value) in values) { if (value == null) { data.remove(key) + removedKeys.add(key) continue } // Refused rather than stored: a non-finite Double serializes as bare `NaN` or @@ -57,22 +61,48 @@ object EmbeddedDataManager { continue } data[key] = value + setKeys.add(key) } + held = data.keys.toList() } + // Built and logged outside the lock, so a log write never holds it. + Logger.d(setTrace(setKeys, removedKeys, held)) + } + + /** + * The success trace, because the bag is otherwise invisible: it lives in memory (nothing in + * `SharedPreferences` to inspect) and the API has no getter, so without this line a host wiring + * up `setEmbeddedData` gets no confirmation until a survey happens to display. Logged at debug, + * which [Logger] gates on `Formbricks.loggingEnabled`. + * + * Keys only, never values: the documented use of this bag includes hashed identity fields. + * Separated from the logging call so that property is directly assertable in a test. + */ + internal fun setTrace(setKeys: List, removedKeys: List, held: List): String { + val removed = if (removedKeys.isEmpty()) "" else ", removed [${removedKeys.joinToString(", ")}]" + return "setEmbeddedData: set [${setKeys.joinToString(", ")}]$removed - the bag now holds " + + "[${held.joinToString(", ")}]. Keys land on a response only if the survey declares them " + + "as ingested Embedded Data fields." } /** Removes one key. A key that is not set is a no-op. */ fun remove(key: String) { + val held: List synchronized(lock) { data.remove(key) + held = data.keys.toList() } + Logger.d("clearEmbeddedData: removed \"$key\" - the bag now holds [${held.joinToString(", ")}]") } /** Removes everything - logout, or a hard context switch. */ fun clear() { + val clearedCount: Int synchronized(lock) { + clearedCount = data.size data.clear() } + Logger.d("clearEmbeddedData: cleared the whole bag ($clearedCount keys)") } /**