Lives here rather than in a subclass because the read is a base-class responsibility: this + * class restores the diagnostic context from it, and {@link #onSaveInstanceState(Bundle)} below + * round-trips it so a recreated fragment restores that context instead of blanking it. Keeping + * the save next to the read means a new subclass cannot forget it. + */ + protected String mCorrelationId; + /** * Whether the host opted in to MAM-CA install-referrer tagging for this request. */ @@ -171,13 +182,31 @@ user commit() rather than commitNow() that the fragment manager that we were rem * @param state a bundle containing data provided when the activity was created */ void extractState(@NonNull final Bundle state) { - setDiagnosticContextForNewThread(state.getString(DiagnosticContext.CORRELATION_ID)); + mCorrelationId = state.getString(DiagnosticContext.CORRELATION_ID); + setDiagnosticContextForNewThread(mCorrelationId); mMamCaInstallReferrerEnabled = state.getBoolean(MAM_CA_INSTALL_REFERRER_ENABLED, false); } + /** + * {@inheritDoc} + * + *
Round-trips the correlation id, which {@link #extractState(Bundle)} reads back on the + * recreation path. Without this the key is absent from the saved bundle and a recreated fragment + * blanks its diagnostic context, so every subsequent log line for the request loses its join + * key — and, for the WebView fragment, the onboarding recorder can no longer be resolved. + * + *
A null id is not written at all rather than stored as a null value. Storing it is harmless
+ * today — {@code RequestContext extends HashMap}, so the downstream {@code put} accepts null,
+ * and the only reader substitutes a random UUID — but that safety is load-bearing on the map
+ * type. Skipping the write keeps the absent case indistinguishable from "never saved", which is
+ * already handled, instead of relying on a null surviving every layer below.
+ */
@Override
public void onSaveInstanceState(@NonNull final Bundle outState) {
super.onSaveInstanceState(outState);
+ if (mCorrelationId != null) {
+ outState.putString(DiagnosticContext.CORRELATION_ID, mCorrelationId);
+ }
outState.putBoolean(MAM_CA_INSTALL_REFERRER_ENABLED, mMamCaInstallReferrerEnabled);
}
diff --git a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java
index db554c466c..b9afb6030e 100644
--- a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java
+++ b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java
@@ -75,6 +75,9 @@
import com.microsoft.identity.common.internal.ui.webview.WebViewUtil;
import com.microsoft.identity.common.internal.ui.webview.switchbrowser.SwitchBrowserStatusCallback;
import com.microsoft.identity.common.internal.ui.webview.switchbrowser.SwitchBrowserProtocolCoordinator;
+import com.microsoft.identity.common.internal.telemetry.OnboardingRecorderRegistry;
+import com.microsoft.identity.common.internal.telemetry.OnboardingTelemetryRecorder;
+import com.microsoft.identity.common.java.logging.DiagnosticContext;
import com.microsoft.identity.common.java.WarningType;
import com.microsoft.identity.common.java.constants.FidoConstants;
import com.microsoft.identity.common.java.exception.ClientException;
@@ -313,6 +316,7 @@ void extractState(@NonNull final Bundle state) {
@Nullable
@Override
public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup container, @Nullable Bundle savedInstanceState) {
+ final String methodTag = TAG + ":onCreateView";
final View view = inflater.inflate(R.layout.common_activity_authentication, container, false);
mProgressBar = view.findViewById(R.id.common_auth_webview_progressbar);
@@ -322,6 +326,26 @@ public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup c
}
mAADWebViewClient = createAADWebViewClient(activity);
setUpWebView(view, mAADWebViewClient);
+
+ // Onboarding telemetry (brokered): if AccountChooser seeded a recorder for this request,
+ // hand it to the WebView client so WebView-observed onboarding steps (MDM enrollment,
+ // Company Portal launch, broker install) and the Auth UX log_telemetry error code are
+ // recorded into the same blob the broker finalizes and returns. Keyed by correlationId via
+ // OnboardingRecorderRegistry (owner + WebView both run in the broker :auth process). No-op
+ // when the request seeded no recorder, or when the correlation id is unusable as a key.
+ // Must stay ahead of initializeAuthUxJavaScriptApi and launchWebView below, so the client
+ // already holds the recorder before the first page can reach the bridge. AB#3708195.
+ final OnboardingTelemetryRecorder onboardingRecorder =
+ OnboardingRecorderRegistry.get(mCorrelationId);
+ if (onboardingRecorder != null) {
+ Logger.info(methodTag, mCorrelationId,
+ "Onboarding telemetry: attaching recorder to WebView client");
+ mAADWebViewClient.setOnboardingTelemetryRecorder(onboardingRecorder);
+ } else {
+ Logger.verbose(methodTag, mCorrelationId,
+ "Onboarding telemetry: no recorder registered for this request");
+ }
+
mAADWebViewClient.initializeAuthUxJavaScriptApi(mWebView, mAuthorizationRequestUrl);
launchWebView(mAuthorizationRequestUrl, mRequestHeaders);
return view;
diff --git a/common/src/main/java/com/microsoft/identity/common/internal/telemetry/OnboardingRecorderRegistry.kt b/common/src/main/java/com/microsoft/identity/common/internal/telemetry/OnboardingRecorderRegistry.kt
new file mode 100644
index 0000000000..63413d34b6
--- /dev/null
+++ b/common/src/main/java/com/microsoft/identity/common/internal/telemetry/OnboardingRecorderRegistry.kt
@@ -0,0 +1,201 @@
+// Copyright (c) Microsoft Corporation.
+// All rights reserved.
+//
+// This code is licensed under the MIT License.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files(the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions :
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+package com.microsoft.identity.common.internal.telemetry
+
+import androidx.annotation.GuardedBy
+import androidx.annotation.VisibleForTesting
+import com.microsoft.identity.common.java.logging.DiagnosticContext
+import com.microsoft.identity.common.logging.Logger
+
+/**
+ * In-process handoff for the onboarding telemetry recorder between the component that OWNS it and
+ * the interactive WebView host that CONSUMES it.
+ *
+ * On the broker side the recorder is built per-request from the onboarding seed by
+ * `AccountChooser` (main broker logic), while the WebView that renders the interactive auth /
+ * remediation pages lives in a separate `AuthorizationActivity` created by the OS from an Intent.
+ * A live recorder instance cannot ride an Intent, but both components run in the broker `:auth`
+ * process, so a process-static registry keyed by the request **correlationId** bridges them:
+ *
+ * - `AccountChooser` [register]s its recorder when it seeds one.
+ * - [com.microsoft.identity.common.internal.providers.oauth2.WebViewAuthorizationFragment] looks it
+ * up by correlationId and hands it to
+ * [com.microsoft.identity.common.internal.ui.webview.AzureActiveDirectoryWebViewClient.setOnboardingTelemetryRecorder]
+ * so WebView-observed onboarding steps (MDM enrollment, Company Portal launch, broker install)
+ * and the Auth UX `log_telemetry` error code are recorded into the same blob that is finalized
+ * and returned in the broker result.
+ *
+ * Entries MUST be [unregister]ed on terminal outcome / teardown to avoid leaking recorders across
+ * requests. Stores the concrete [OnboardingTelemetryRecorder] because the WebView client's
+ * telemetry hooks (e.g. `setLastLoadedDomain`) use methods that are not on the common4j interface.
+ *
+ * **The correlation id must be a real request id.** [DiagnosticContext.UNSET_CORRELATION_ID] is the
+ * value every thread carries before its request context is set, and the authorization Intent extra
+ * is populated by reading the request context map directly rather than through
+ * `getThreadCorrelationId()` — so the raw sentinel can reach this class. It is shared by definition,
+ * so accepting it as a key would let two unrelated requests resolve to the same recorder and merge
+ * one flow's blocking errors into the other's uploaded blob. All three accessors reject it: the
+ * feature goes inert (and [register] warns) instead of silently mis-attributing telemetry, which for
+ * a component whose entire purpose is correct attribution is the only acceptable failure.
+ *
+ * Because this is process-static state in the long-lived broker `:auth` process, a missed
+ * [unregister] would leak a recorder permanently — the recorder object and its collected steps /
+ * blocking errors, not an Activity, since [OnboardingTelemetryRecorder] deliberately holds only the
+ * application context. The map is therefore capped at [MAX_ENTRIES]: once full, registering evicts
+ * the least-recently-used entry and logs a warning. That converts an unbounded leak into a bounded
+ * one and makes the underlying bug visible, rather than hiding it. The cap is far above real
+ * concurrency (the broker drives one interactive request at a time), so eviction should not happen
+ * at all; if the warning appears, it is the signal that a terminal path is failing to unregister.
+ *
+ * Eviction is least-recently-*used* rather than oldest-registered. In today's flow that is not
+ * load-bearing — the WebView host calls [get] exactly once per request and holds the reference
+ * thereafter, so evicting a live entry would not disturb the in-flight request anyway — but it is
+ * free, and it is the safer default if a future caller ever re-resolves mid-request.
+ */
+object OnboardingRecorderRegistry {
+
+ private val TAG = OnboardingRecorderRegistry::class.java.simpleName
+
+ /**
+ * Upper bound on concurrently-registered recorders. Generous relative to real concurrency; it
+ * exists to bound a leak from a missed [unregister], not to constrain legitimate use.
+ */
+ private const val MAX_ENTRIES = 16
+
+ // Access-ordered LinkedHashMap so eviction drops the least-recently-touched entry rather than
+ // simply the oldest-registered one. Guarded by its own monitor: LinkedHashMap is not
+ // thread-safe, and get() mutates access order, so reads need the lock too.
+ private val recorders = object : LinkedHashMap The registry's own suite proves storage in isolation; these tests prove the fragment side
+ * actually reaches it, including across activity recreation. Without them a regression in the
+ * correlation-id plumbing would leave every WebView onboarding hook inert while the registry tests
+ * still passed — the exact "green tests, dead feature" state this PR exists to fix.
+ *
+ * Follows the {@code AuthorizationFragmentUrlTrackingTest} pattern: a minimal concrete subclass
+ * exercising the base-class behaviour directly, rather than driving a full Fragment lifecycle.
+ */
+@RunWith(RobolectricTestRunner.class)
+public class AuthorizationFragmentCorrelationIdTest {
+
+ private static final String CORRELATION_ID = "e4f1a0c2-0000-4a1b-9f3e-000000000001";
+
+ /** Minimal concrete subclass; the behaviour under test all lives in the base class. */
+ private static class TestAuthorizationFragment extends AuthorizationFragment {
+ String correlationId() {
+ return mCorrelationId;
+ }
+ }
+
+ private TestAuthorizationFragment mFragment;
+
+ @Before
+ public void setUp() {
+ mFragment = new TestAuthorizationFragment();
+ OnboardingRecorderRegistry.clearForTest();
+ }
+
+ @After
+ public void tearDown() {
+ OnboardingRecorderRegistry.clearForTest();
+ }
+
+ @Test
+ public void testExtractState_CapturesCorrelationIdFromBundle() {
+ final Bundle state = new Bundle();
+ state.putString(DiagnosticContext.CORRELATION_ID, CORRELATION_ID);
+
+ mFragment.extractState(state);
+
+ assertEquals(CORRELATION_ID, mFragment.correlationId());
+ }
+
+ @Test
+ public void testExtractedCorrelationId_ResolvesTheRegisteredRecorder() {
+ // The whole point of capturing the id: it has to be the key the owner registered under.
+ final OnboardingTelemetryRecorder recorder = newRecorder();
+ OnboardingRecorderRegistry.register(CORRELATION_ID, recorder);
+
+ final Bundle state = new Bundle();
+ state.putString(DiagnosticContext.CORRELATION_ID, CORRELATION_ID);
+ mFragment.extractState(state);
+
+ assertSame(recorder, OnboardingRecorderRegistry.get(mFragment.correlationId()));
+ }
+
+ @Test
+ public void testCorrelationId_SurvivesSaveAndRestore() {
+ // The regression this guards: onSaveInstanceState originally did not round-trip
+ // CORRELATION_ID, so a recreated fragment (a config change AuthorizationActivity does not
+ // declare, e.g. uiMode) read null back, blanked its diagnostic context, and could no longer
+ // resolve its recorder. The Intent survives recreation but this fragment reads the bundle.
+ final OnboardingTelemetryRecorder recorder = newRecorder();
+ OnboardingRecorderRegistry.register(CORRELATION_ID, recorder);
+
+ final Bundle initialState = new Bundle();
+ initialState.putString(DiagnosticContext.CORRELATION_ID, CORRELATION_ID);
+ mFragment.extractState(initialState);
+
+ // Activity recreation: the framework hands back only what onSaveInstanceState wrote.
+ final Bundle savedState = new Bundle();
+ mFragment.onSaveInstanceState(savedState);
+
+ final TestAuthorizationFragment recreated = new TestAuthorizationFragment();
+ recreated.extractState(savedState);
+
+ assertEquals("the correlation id must round-trip through the saved bundle",
+ CORRELATION_ID, recreated.correlationId());
+ assertNotNull("the recorder must still be resolvable after recreation",
+ OnboardingRecorderRegistry.get(recreated.correlationId()));
+ assertSame(recorder, OnboardingRecorderRegistry.get(recreated.correlationId()));
+ }
+
+ @Test
+ public void testNullCorrelationId_IsNotWrittenToTheSavedBundle() {
+ // A fragment that never saw a correlation id (an MSAL client with no diagnostic context)
+ // must not persist a null under the key. Storing one is harmless today only because
+ // RequestContext extends HashMap and the sole reader substitutes a UUID for null — safety
+ // that would evaporate if the map type ever became a ConcurrentHashMap/Hashtable, where
+ // put(key, null) throws. Leaving the key absent keeps this identical to the already-handled
+ // "never saved" case rather than depending on null surviving every layer below.
+ final TestAuthorizationFragment fragment = new TestAuthorizationFragment();
+ fragment.extractState(new Bundle()); // no CORRELATION_ID present -> mCorrelationId is null
+
+ final Bundle savedState = new Bundle();
+ fragment.onSaveInstanceState(savedState);
+
+ assertFalse("a null correlation id must not be written under the key",
+ savedState.containsKey(DiagnosticContext.CORRELATION_ID));
+
+ // And the recreation path still behaves: absent key reads back as null, no throw.
+ final TestAuthorizationFragment recreated = new TestAuthorizationFragment();
+ recreated.extractState(savedState);
+ assertNull(recreated.correlationId());
+ }
+
+ @Test
+ public void testNoRecorderRegistered_ResolvesToNullNotThrow() {
+ // The MSAL-client path: no onboarding seed, so nothing is ever registered. The host resolves
+ // unconditionally and must simply stay inert.
+ final Bundle state = new Bundle();
+ state.putString(DiagnosticContext.CORRELATION_ID, CORRELATION_ID);
+ mFragment.extractState(state);
+
+ assertNull(OnboardingRecorderRegistry.get(mFragment.correlationId()));
+ }
+
+ @Test
+ public void testMissingCorrelationId_ResolvesToNullNotThrow() {
+ // A bundle without the key must not blow up the fragment or the registry lookup.
+ mFragment.extractState(new Bundle());
+
+ assertNull(mFragment.correlationId());
+ assertNull(OnboardingRecorderRegistry.get(mFragment.correlationId()));
+ }
+
+ @Test
+ public void testUnsetSentinelCorrelationId_DoesNotResolveAnotherRequestsRecorder() {
+ // A request whose thread never had a request context carries the shared UNSET sentinel into
+ // the Intent extra and therefore into this bundle. It must not act as a key.
+ OnboardingRecorderRegistry.register(CORRELATION_ID, newRecorder());
+
+ final Bundle state = new Bundle();
+ state.putString(DiagnosticContext.CORRELATION_ID, DiagnosticContext.UNSET_CORRELATION_ID);
+ mFragment.extractState(state);
+
+ assertNull("the sentinel must never resolve a recorder",
+ OnboardingRecorderRegistry.get(mFragment.correlationId()));
+ }
+
+ private OnboardingTelemetryRecorder newRecorder() {
+ return new OnboardingTelemetryRecorder(
+ "{\"schema_version\":\"1.0.0\","
+ + "\"session_correlation_id\":\"test-uuid-123\","
+ + "\"onboarding_mode\":\"brokered\"}",
+ "test-client-id",
+ "scope1",
+ ApplicationProvider.getApplicationContext());
+ }
+}
diff --git a/common/src/test/java/com/microsoft/identity/common/internal/telemetry/OnboardingRecorderRegistryTest.kt b/common/src/test/java/com/microsoft/identity/common/internal/telemetry/OnboardingRecorderRegistryTest.kt
new file mode 100644
index 0000000000..d68f8c3d2d
--- /dev/null
+++ b/common/src/test/java/com/microsoft/identity/common/internal/telemetry/OnboardingRecorderRegistryTest.kt
@@ -0,0 +1,251 @@
+// Copyright (c) Microsoft Corporation.
+// All rights reserved.
+//
+// This code is licensed under the MIT License.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files(the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions :
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+package com.microsoft.identity.common.internal.telemetry
+
+import android.content.Context
+import androidx.test.core.app.ApplicationProvider
+import com.microsoft.identity.common.java.logging.DiagnosticContext
+import org.junit.After
+import org.junit.Assert
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+
+/**
+ * AB#3708195: contract tests for the process-static recorder handoff between the broker's
+ * `AccountChooser` (owner) and `WebViewAuthorizationFragment` (consumer).
+ *
+ * This is a singleton holding recorders for the life of the broker `:auth` process, so the cases
+ * that matter are correlation-id keying (one request must never see another's recorder — including
+ * via the shared `UNSET` sentinel), removal (a missed unregister leaks the recorder and its
+ * collected steps / blocking errors until eviction), and absent-key behaviour (the no-seed path must
+ * get null rather than throw).
+ */
+@RunWith(RobolectricTestRunner::class)
+class OnboardingRecorderRegistryTest {
+
+ @Before
+ fun setup() = OnboardingRecorderRegistry.clearForTest()
+
+ @After
+ fun tearDown() = OnboardingRecorderRegistry.clearForTest()
+
+ // --- Keying ---
+
+ @Test
+ fun testRegisterThenGet_ReturnsSameInstance() {
+ val recorder = newRecorder()
+
+ OnboardingRecorderRegistry.register("correlation-a", recorder)
+
+ Assert.assertSame(recorder, OnboardingRecorderRegistry.get("correlation-a"))
+ }
+
+ @Test
+ fun testGet_WithDifferentCorrelationId_DoesNotCrossWire() {
+ // The failure this guards is two concurrent requests sharing a recorder, which would merge
+ // one flow's blocking errors and steps into the other's uploaded blob.
+ val recorderA = newRecorder()
+ val recorderB = newRecorder()
+ OnboardingRecorderRegistry.register("correlation-a", recorderA)
+ OnboardingRecorderRegistry.register("correlation-b", recorderB)
+
+ Assert.assertSame(recorderA, OnboardingRecorderRegistry.get("correlation-a"))
+ Assert.assertSame(recorderB, OnboardingRecorderRegistry.get("correlation-b"))
+ Assert.assertNotSame(
+ OnboardingRecorderRegistry.get("correlation-a"),
+ OnboardingRecorderRegistry.get("correlation-b")
+ )
+ }
+
+ @Test
+ fun testRegister_SameCorrelationIdTwice_LastWins() {
+ val first = newRecorder()
+ val second = newRecorder()
+
+ OnboardingRecorderRegistry.register("correlation-a", first)
+ OnboardingRecorderRegistry.register("correlation-a", second)
+
+ Assert.assertSame(second, OnboardingRecorderRegistry.get("correlation-a"))
+ Assert.assertEquals("re-registering must replace, not accumulate", 1, OnboardingRecorderRegistry.size())
+ }
+
+ // --- Absent keys / null-and-empty inputs ---
+
+ @Test
+ fun testGet_UnknownCorrelationId_ReturnsNullNotThrow() {
+ // The MSAL-client path: no onboarding seed, so nothing is ever registered. The WebView host
+ // calls get() unconditionally and must simply stay inert.
+ Assert.assertNull(OnboardingRecorderRegistry.get("never-registered"))
+ }
+
+ @Test
+ fun testGet_NullOrEmptyCorrelationId_ReturnsNull() {
+ OnboardingRecorderRegistry.register("correlation-a", newRecorder())
+
+ Assert.assertNull(OnboardingRecorderRegistry.get(null))
+ Assert.assertNull(OnboardingRecorderRegistry.get(""))
+ }
+
+ @Test
+ fun testRegister_NullOrEmptyCorrelationId_IsNoOp() {
+ OnboardingRecorderRegistry.register(null, newRecorder())
+ OnboardingRecorderRegistry.register("", newRecorder())
+
+ Assert.assertEquals(0, OnboardingRecorderRegistry.size())
+ }
+
+ @Test
+ fun testRegister_NullRecorder_IsNoOp() {
+ OnboardingRecorderRegistry.register("correlation-a", null)
+
+ Assert.assertEquals(0, OnboardingRecorderRegistry.size())
+ Assert.assertNull(OnboardingRecorderRegistry.get("correlation-a"))
+ }
+
+ // --- Removal / lifecycle ---
+
+ @Test
+ fun testUnregister_RemovesOnlyTheTargetedEntry() {
+ val recorderB = newRecorder()
+ OnboardingRecorderRegistry.register("correlation-a", newRecorder())
+ OnboardingRecorderRegistry.register("correlation-b", recorderB)
+
+ OnboardingRecorderRegistry.unregister("correlation-a")
+
+ Assert.assertNull("the released recorder must not be retrievable", OnboardingRecorderRegistry.get("correlation-a"))
+ Assert.assertSame("an unrelated request must be untouched", recorderB, OnboardingRecorderRegistry.get("correlation-b"))
+ Assert.assertEquals(1, OnboardingRecorderRegistry.size())
+ }
+
+ @Test
+ fun testUnregister_UnknownOrNullCorrelationId_IsSafeNoOp() {
+ OnboardingRecorderRegistry.register("correlation-a", newRecorder())
+
+ OnboardingRecorderRegistry.unregister("never-registered")
+ OnboardingRecorderRegistry.unregister(null)
+ OnboardingRecorderRegistry.unregister("")
+
+ Assert.assertEquals(1, OnboardingRecorderRegistry.size())
+ }
+
+ @Test
+ fun testUnregister_Twice_IsIdempotent() {
+ // The broker unregisters in a finally on the finalize path; a retried or doubled teardown
+ // must not throw.
+ OnboardingRecorderRegistry.register("correlation-a", newRecorder())
+
+ OnboardingRecorderRegistry.unregister("correlation-a")
+ OnboardingRecorderRegistry.unregister("correlation-a")
+
+ Assert.assertEquals(0, OnboardingRecorderRegistry.size())
+ }
+
+ // --- Leak bound ---
+
+ @Test
+ fun testRegistry_IsBoundedWhenCallersFailToUnregister() {
+ // This registry is process-static in the long-lived broker :auth process, so a terminal path
+ // that forgets to unregister would leak a recorder (and its Context) permanently. Simulate
+ // that: register far past the cap and never release.
+ repeat(100) { i -> OnboardingRecorderRegistry.register("correlation-$i", newRecorder()) }
+
+ Assert.assertEquals(
+ "a missed unregister must be a bounded leak, not an unbounded one",
+ 16, OnboardingRecorderRegistry.size()
+ )
+ Assert.assertNull("the oldest entries must have been evicted", OnboardingRecorderRegistry.get("correlation-0"))
+ Assert.assertNotNull("the newest entry must survive", OnboardingRecorderRegistry.get("correlation-99"))
+ }
+
+ @Test
+ fun testEviction_KeepsTheEntryStillBeingUsed() {
+ // Eviction is least-recently-USED, not least-recently-registered. Today's host resolves its
+ // recorder once and holds the reference, so this is not load-bearing for the current flow —
+ // it is the safer default if a future caller ever re-resolves mid-request.
+ val liveRecorder = newRecorder()
+ OnboardingRecorderRegistry.register("live-request", liveRecorder)
+
+ repeat(20) { i ->
+ OnboardingRecorderRegistry.register("leaked-$i", newRecorder())
+ // Simulates a caller that re-resolves rather than caching the reference.
+ Assert.assertSame(liveRecorder, OnboardingRecorderRegistry.get("live-request"))
+ }
+
+ Assert.assertSame(
+ "the entry in active use must survive eviction pressure",
+ liveRecorder, OnboardingRecorderRegistry.get("live-request")
+ )
+ }
+
+ // --- Correlation-id sentinel ---
+
+ @Test
+ fun testRegister_UnsetSentinelCorrelationId_IsRejected() {
+ // DiagnosticContext seeds every thread's request context with UNSET, and the authorization
+ // Intent extra is populated from that map directly rather than through
+ // getThreadCorrelationId(), so the raw sentinel can reach this registry. It is shared by
+ // definition: accepting it would let two unrelated requests resolve to the same recorder and
+ // merge one flow's blocking errors into the other's uploaded blob.
+ OnboardingRecorderRegistry.register(DiagnosticContext.UNSET_CORRELATION_ID, newRecorder())
+
+ Assert.assertEquals("the sentinel must never become a key", 0, OnboardingRecorderRegistry.size())
+ Assert.assertNull(OnboardingRecorderRegistry.get(DiagnosticContext.UNSET_CORRELATION_ID))
+ }
+
+ @Test
+ fun testUnsetSentinel_CannotCrossWireTwoRequests() {
+ // The concrete failure the guard prevents: two requests whose threads never had a request
+ // context both fall back to the sentinel. Without the guard the second registration silently
+ // displaces the first and BOTH WebView hosts resolve to request two's recorder.
+ OnboardingRecorderRegistry.register(DiagnosticContext.UNSET_CORRELATION_ID, newRecorder())
+ OnboardingRecorderRegistry.register(DiagnosticContext.UNSET_CORRELATION_ID, newRecorder())
+
+ Assert.assertEquals(0, OnboardingRecorderRegistry.size())
+ Assert.assertNull(
+ "inert is the correct failure; sharing a recorder would mis-attribute telemetry",
+ OnboardingRecorderRegistry.get(DiagnosticContext.UNSET_CORRELATION_ID)
+ )
+ }
+
+ @Test
+ fun testUnregister_UnsetSentinel_DoesNotDisturbRealEntries() {
+ val real = newRecorder()
+ OnboardingRecorderRegistry.register("correlation-a", real)
+
+ OnboardingRecorderRegistry.unregister(DiagnosticContext.UNSET_CORRELATION_ID)
+
+ Assert.assertSame(real, OnboardingRecorderRegistry.get("correlation-a"))
+ Assert.assertEquals(1, OnboardingRecorderRegistry.size())
+ }
+
+ private fun newRecorder(): OnboardingTelemetryRecorder = OnboardingTelemetryRecorder(
+ "{\"schema_version\":\"1.0.0\"," +
+ "\"session_correlation_id\":\"test-uuid-123\"," +
+ "\"onboarding_mode\":\"brokered\"}",
+ "test-client-id",
+ "scope1",
+ ApplicationProvider.getApplicationContext Deliberately independent of {@link DiagnosticContext#UNSET_CORRELATION_ID}, despite the
+ * identical text: this one also stands in for a missing thread id, and it is never used as a
+ * key or compared against. Callers that use the correlation id as a key must reject the
+ * {@code DiagnosticContext} sentinel instead — see its javadoc.
+ */
private static final String UNSET = "UNSET";
// Turn on the VERBOSE level logging by default.