From 8204d6b5cd5ea410cd752446a1d2eba50f6562ff Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Fri, 21 Aug 2026 12:52:25 -0700 Subject: [PATCH 1/7] feat: add registerPlugin so a plugin can be added after the client starts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plugins could only be supplied through LDConfig, so an integration that learns about a plugin later — or that wants to instrument a client it did not configure — had no way in. Hooks go live only once register succeeds, matching the .NET ordering, so a plugin whose registration failed never contributes hooks and a plugin's own hooks do not observe its register call. Retaining EnvironmentMetadata on the instance lets a plugin registered later be handed the same environment description as one configured up front. Co-authored-by: Cursor --- .../sdk/android/LDClientPluginsTest.java | 233 ++++++++++++++++++ .../launchdarkly/sdk/android/LDClient.java | 64 ++++- .../sdk/android/LDClientInterface.java | 14 ++ 3 files changed, 306 insertions(+), 5 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java b/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java index ce59793a..e9191956 100644 --- a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java +++ b/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java @@ -1,6 +1,8 @@ package com.launchdarkly.sdk.android; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import android.app.Application; @@ -18,6 +20,7 @@ import com.launchdarkly.sdk.android.integrations.IdentifySeriesResult; import com.launchdarkly.sdk.android.integrations.Plugin; import com.launchdarkly.sdk.android.integrations.PluginMetadata; +import com.launchdarkly.sdk.android.integrations.RegistrationCompleteResult; import com.launchdarkly.sdk.android.integrations.TrackSeriesContext; import org.junit.Before; @@ -175,6 +178,174 @@ public void identifyHooksRunForEachEnvironment() throws Exception { } } + @Test + public void registerPluginPassesClientAndEnvironmentMetadata() throws Exception { + MockPlugin testPlugin = new MockPlugin(Collections.emptyList()); + + try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(null), ldContext, 1)) { + ldClient.registerPlugin(testPlugin); + + assertEquals(1, testPlugin.getHooksCalls.size()); + assertEquals(1, testPlugin.registerCalls.size()); + assertEquals(ldClient, testPlugin.registerCalls.get(0).get("client")); + + EnvironmentMetadata metadata = (EnvironmentMetadata) testPlugin.registerCalls.get(0).get("environmentMetadata"); + assertEquals(mobileKey, metadata.getCredential()); + assertEquals("AndroidClient", metadata.getSdkMetadata().getName()); + + // The same environment description a plugin configured up front would have been given. + assertEquals(metadata, testPlugin.getHooksCalls.get(0).get("environmentMetadata")); + + logging.assertNoErrorsLogged(); + } + } + + @Test + public void registerPluginActivatesBundledHooks() throws Exception { + MockHook testHook = new MockHook(); + MockPlugin testPlugin = new MockPlugin(Collections.singletonList(testHook)); + + try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(null), ldContext, 1)) { + ldClient.registerPlugin(testPlugin); + + ldClient.boolVariation("test-flag", false); + assertEquals(1, testHook.beforeEvaluationCalls.size()); + assertEquals(1, testHook.afterEvaluationCalls.size()); + + ldClient.identify(LDContext.create("newUserKey")).get(); + // Only the identify made after registration: the implicit one during init predates the plugin. + assertEquals(1, testHook.beforeIdentifyCalls.size()); + assertEquals(1, testHook.afterIdentifyCalls.size()); + + ldClient.track("test-event"); + assertEquals(1, testHook.afterTrackCalls.size()); + + logging.assertNoErrorsLogged(); + } + } + + @Test + public void registerPluginDoesNotRunTheRegisteringPluginsOwnHooks() throws Exception { + MockHook testHook = new MockHook(); + EvaluateOnRegisterPlugin testPlugin = new EvaluateOnRegisterPlugin(testHook); + + try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(null), ldContext, 1)) { + ldClient.registerPlugin(testPlugin); + + // The plugin evaluated a flag from inside register, but its own hooks were not live yet. + assertEquals(1, testPlugin.registerCalls.size()); + assertEquals(0, testHook.beforeEvaluationCalls.size()); + assertEquals(0, testHook.afterEvaluationCalls.size()); + + // They do run for evaluations made once registration has completed. + ldClient.boolVariation("test-flag", false); + assertEquals(1, testHook.beforeEvaluationCalls.size()); + assertEquals(1, testHook.afterEvaluationCalls.size()); + + logging.assertNoErrorsLogged(); + } + } + + @Test + public void registerPluginReportsSuccessToOnPluginsReady() throws Exception { + MockPlugin testPlugin = new MockPlugin(Collections.emptyList()); + + try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(null), ldContext, 1)) { + ldClient.registerPlugin(testPlugin); + + assertEquals(1, testPlugin.onPluginsReadyCalls.size()); + assertEquals(RegistrationCompleteResult.success(), testPlugin.onPluginsReadyCalls.get(0).get("result")); + + EnvironmentMetadata metadata = (EnvironmentMetadata) testPlugin.onPluginsReadyCalls.get(0).get("environmentMetadata"); + assertEquals(mobileKey, metadata.getCredential()); + + logging.assertNoErrorsLogged(); + } + } + + @Test + public void registerPluginDoesNotRegisterPluginWhoseGetHooksThrows() throws Exception { + MockHook testHook = new MockHook(); + MockPlugin testPlugin = new MockPlugin(Collections.singletonList(testHook), true, false); + + try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(null), ldContext, 1)) { + ldClient.registerPlugin(testPlugin); + + assertEquals(0, testPlugin.registerCalls.size()); + assertEquals(0, testPlugin.onPluginsReadyCalls.size()); + + ldClient.boolVariation("test-flag", false); + assertEquals(0, testHook.beforeEvaluationCalls.size()); + + logging.assertErrorLogged("Unable to get hooks"); + } + } + + @Test + public void registerPluginToleratesRegisterThrowing() throws Exception { + MockHook testHook = new MockHook(); + MockPlugin testPlugin = new MockPlugin(Collections.singletonList(testHook), false, true); + + try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(null), ldContext, 1)) { + // The exception is logged rather than propagated. + ldClient.registerPlugin(testPlugin); + assertEquals(1, testPlugin.registerCalls.size()); + + // A plugin that failed to register contributes no hooks. + ldClient.boolVariation("test-flag", false); + assertEquals(0, testHook.beforeEvaluationCalls.size()); + assertEquals(0, testHook.afterEvaluationCalls.size()); + + assertEquals(1, testPlugin.onPluginsReadyCalls.size()); + RegistrationCompleteResult result = + (RegistrationCompleteResult) testPlugin.onPluginsReadyCalls.get(0).get("result"); + assertTrue(result instanceof RegistrationCompleteResult.Failure); + List failures = + ((RegistrationCompleteResult.Failure) result).getFailures(); + assertEquals(1, failures.size()); + assertEquals("mock-plugin-name", failures.get(0).getPluginName()); + + logging.assertErrorLogged("Exception thrown registering plugin"); + } + } + + @Test + public void registerPluginRejectsNullPlugin() throws Exception { + try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(null), ldContext, 1)) { + assertThrows(NullPointerException.class, () -> ldClient.registerPlugin(null)); + } + } + + @Test + public void registerPluginAppliesOnlyToTheClientItIsCalledOn() throws Exception { + MockHook testHook = new MockHook(); + MockPlugin testPlugin = new MockPlugin(Collections.singletonList(testHook)); + + LDConfig config = new LDConfig.Builder(LDConfig.Builder.AutoEnvAttributes.Disabled) + .mobileKey(mobileKey) + .secondaryMobileKeys(Map.of("secondaryEnvironment", secondaryMobileKey)) + .offline(true) + .events(Components.noEvents()) + .logAdapter(logging.logAdapter) + .build(); + + try (LDClient ldClient = LDClient.init(application, config, ldContext, 10)) { + ldClient.registerPlugin(testPlugin); + + assertEquals(1, testPlugin.registerCalls.size()); + assertEquals(ldClient, testPlugin.registerCalls.get(0).get("client")); + + ldClient.boolVariation("test-flag", false); + assertEquals(1, testHook.beforeEvaluationCalls.size()); + + // The other environment has its own client, which this plugin was not registered with. + LDClient.getForMobileKey("secondaryEnvironment").boolVariation("test-flag", false); + assertEquals(1, testHook.beforeEvaluationCalls.size()); + + logging.assertNoErrorsLogged(); + } + } + private LDConfig makeOfflineConfig(List plugins) { LDConfig.Builder builder = new LDConfig.Builder(LDConfig.Builder.AutoEnvAttributes.Disabled) .mobileKey(mobileKey) @@ -192,12 +363,21 @@ private LDConfig makeOfflineConfig(List plugins) { private static class MockPlugin extends Plugin { private final List hooks; + private final boolean throwOnGetHooks; + private final boolean throwOnRegister; public final List> getHooksCalls = new ArrayList<>(); public final List> registerCalls = new ArrayList<>(); + public final List> onPluginsReadyCalls = new ArrayList<>(); public MockPlugin(List hooks) { + this(hooks, false, false); + } + + public MockPlugin(List hooks, boolean throwOnGetHooks, boolean throwOnRegister) { this.hooks = hooks; + this.throwOnGetHooks = throwOnGetHooks; + this.throwOnRegister = throwOnRegister; } @NonNull @@ -230,6 +410,9 @@ public void register(LDClient client, EnvironmentMetadata metadata) { "client", client, "environmentMetadata", metadata )); + if (throwOnRegister) { + throw new RuntimeException("register failed for mock-plugin-name"); + } } @NonNull @@ -238,8 +421,58 @@ public List getHooks(EnvironmentMetadata metadata) { getHooksCalls.add(Map.of( "environmentMetadata", metadata )); + if (throwOnGetHooks) { + throw new RuntimeException("getHooks failed for mock-plugin-name"); + } return this.hooks; } + + @Override + public void onPluginsReady(RegistrationCompleteResult result, EnvironmentMetadata metadata) { + onPluginsReadyCalls.add(Map.of( + "result", result, + "environmentMetadata", metadata + )); + } + } + + /** + * Evaluates a flag from inside {@code register}, so a test can tell whether the plugin's own hooks were live at + * that point. + */ + private static class EvaluateOnRegisterPlugin extends Plugin { + + private final Hook hook; + + public final List> registerCalls = new ArrayList<>(); + + public EvaluateOnRegisterPlugin(Hook hook) { + this.hook = hook; + } + + @NonNull + @Override + public PluginMetadata getMetadata() { + return new PluginMetadata() { + @NonNull + @Override + public String getName() { + return "evaluate-on-register-plugin"; + } + }; + } + + @Override + public void register(LDClient client, EnvironmentMetadata metadata) { + registerCalls.add(Map.of("client", client, "environmentMetadata", metadata)); + client.boolVariation("test-flag", false); + } + + @NonNull + @Override + public List getHooks(EnvironmentMetadata metadata) { + return Collections.singletonList(hook); + } } private static class MockHook extends Hook { diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java index d593d25d..edea03b1 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java @@ -80,6 +80,7 @@ public class LDClient implements LDClientInterface, Closeable { // application may take on every redraw of a view. private final String mobileKeyHash; private List plugins; + private volatile EnvironmentMetadata environmentMetadata; // If 15 seconds or more is passed as a timeout to init, we will log a warning. private static final int EXCESSIVE_INIT_WAIT_SECONDS = 15; @@ -133,7 +134,7 @@ public static Future init(@NonNull Application application, LDContext modifiedContext; // used for plugin registration after instances are created - final Map instanceMetadatas = new HashMap<>(); + final List createdInstances = new ArrayList<>(); // Acquire the `initLock` to ensure that if `init()` is called multiple times, we will only // initialize the client(s) once. @@ -201,7 +202,8 @@ public static Future init(@NonNull Application application, } // metadata created per environment since mobile key varies - instanceMetadatas.put(instance, new EnvironmentMetadata(applicationInfo, sdkMetadata, mobileKey)); + instance.environmentMetadata = new EnvironmentMetadata(applicationInfo, sdkMetadata, mobileKey); + createdInstances.add(instance); } catch (LaunchDarklyException e) { resultFuture.setException(e); @@ -216,9 +218,8 @@ public static Future init(@NonNull Application application, } // after instances have been created, set up hooks for each plugin of the instance and call register - for (Map.Entry entry : instanceMetadatas.entrySet()) { - LDClient instance = entry.getKey(); - EnvironmentMetadata metadata = entry.getValue(); + for (LDClient instance : createdInstances) { + EnvironmentMetadata metadata = instance.environmentMetadata; for (Plugin plugin : instance.plugins) { // try is for each plugin so that if one plugin has an issue, the others will have an opportunity to be used @@ -895,4 +896,57 @@ static LDLogger getSharedLogger() { public void addHook(Hook hook) { hookRunner.addHook(hook); } + + @Override + public void registerPlugin(Plugin plugin) { + if (plugin == null) { + throw new NullPointerException("plugin must not be null"); + } + + EnvironmentMetadata metadata = environmentMetadata; + + List pluginHooks; + try { + pluginHooks = plugin.getHooks(metadata); + } catch (Exception e) { + logger.error("Exception thrown getting hooks for plugin " + pluginName(plugin) + ". Unable to get hooks, plugin will not be registered."); + return; + } + + RegistrationCompleteResult result; + try { + plugin.register(this, metadata); + result = RegistrationCompleteResult.success(); + } catch (Exception e) { + logger.error("Exception thrown registering plugin " + pluginName(plugin) + "."); + result = RegistrationCompleteResult.failure(Collections.singletonList( + new RegistrationCompleteResult.Failure.PluginFailure(pluginName(plugin), e.getMessage(), e))); + } + + // The hooks go live only once register has succeeded, so a plugin whose registration failed + // never contributes hooks, and a plugin's own hooks do not observe its register call. + if (result instanceof RegistrationCompleteResult.Success) { + for (Hook hook : pluginHooks) { + hookRunner.addHook(hook); + } + } + + try { + plugin.onPluginsReady(result, metadata); + } catch (Exception e) { + logger.error("Exception thrown executing onPluginsReady for plugin " + pluginName(plugin) + "."); + } + } + + /** + * Reads a plugin's name for a log message, tolerating a plugin whose metadata itself throws: + * otherwise reporting one failure would raise another out of the handler that reports it. + */ + private String pluginName(Plugin plugin) { + try { + return plugin.getMetadata().getName(); + } catch (Exception e) { + return "unknown"; + } + } } diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java index b7818b68..da407271 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java @@ -6,6 +6,7 @@ import com.launchdarkly.sdk.LDContext; import com.launchdarkly.sdk.LDValue; import com.launchdarkly.sdk.android.integrations.Hook; +import com.launchdarkly.sdk.android.integrations.Plugin; import java.io.Closeable; import java.util.Map; @@ -402,4 +403,17 @@ public interface LDClientInterface extends Closeable { * @param hook The hook to add. */ void addHook(Hook hook); + + /** + * Registers a single {@link Plugin} with this client after it has been created; to register + * plugins beforehand, use the {@code plugins} method of {@link LDConfig.Builder} instead. + *

+ * The plugin's hooks only start running once {@link Plugin#register} returns, so they do not + * observe what {@code register} itself does. Exceptions from {@link Plugin#getHooks} or + * {@code register} are logged rather than propagated, and leave the plugin contributing no + * hooks. Registration covers this client, and so this environment, alone. + * + * @param plugin the plugin to register; must not be null + */ + void registerPlugin(Plugin plugin); } From 723b0ce0bf0f4709614078330541a708eaa92bd1 Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Mon, 24 Aug 2026 14:38:42 -0700 Subject: [PATCH 2/7] address feadback --- .../sdk/android/LDClientPluginsTest.java | 26 ++++---- .../launchdarkly/sdk/android/LDClient.java | 65 +++++++++++-------- .../sdk/android/LDClientInterface.java | 2 + .../sdk/android/integrations/Plugin.java | 12 ++++ 4 files changed, 63 insertions(+), 42 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java b/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java index e9191956..9c7b5dfe 100644 --- a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java +++ b/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java @@ -2,7 +2,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import android.app.Application; @@ -247,17 +246,16 @@ public void registerPluginDoesNotRunTheRegisteringPluginsOwnHooks() throws Excep } @Test - public void registerPluginReportsSuccessToOnPluginsReady() throws Exception { + public void registerPluginDoesNotCallOnPluginsReady() throws Exception { MockPlugin testPlugin = new MockPlugin(Collections.emptyList()); try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(null), ldContext, 1)) { ldClient.registerPlugin(testPlugin); - assertEquals(1, testPlugin.onPluginsReadyCalls.size()); - assertEquals(RegistrationCompleteResult.success(), testPlugin.onPluginsReadyCalls.get(0).get("result")); - - EnvironmentMetadata metadata = (EnvironmentMetadata) testPlugin.onPluginsReadyCalls.get(0).get("environmentMetadata"); - assertEquals(mobileKey, metadata.getCredential()); + // onPluginsReady reports on a batch of plugins registered together, so registering a + // single plugin on its own has nothing to report and does not call it. + assertEquals(1, testPlugin.registerCalls.size()); + assertEquals(0, testPlugin.onPluginsReadyCalls.size()); logging.assertNoErrorsLogged(); } @@ -296,14 +294,9 @@ public void registerPluginToleratesRegisterThrowing() throws Exception { assertEquals(0, testHook.beforeEvaluationCalls.size()); assertEquals(0, testHook.afterEvaluationCalls.size()); - assertEquals(1, testPlugin.onPluginsReadyCalls.size()); - RegistrationCompleteResult result = - (RegistrationCompleteResult) testPlugin.onPluginsReadyCalls.get(0).get("result"); - assertTrue(result instanceof RegistrationCompleteResult.Failure); - List failures = - ((RegistrationCompleteResult.Failure) result).getFailures(); - assertEquals(1, failures.size()); - assertEquals("mock-plugin-name", failures.get(0).getPluginName()); + // The failure is reported in the log alone, since this path does not call + // onPluginsReady. + assertEquals(0, testPlugin.onPluginsReadyCalls.size()); logging.assertErrorLogged("Exception thrown registering plugin"); } @@ -427,6 +420,9 @@ public List getHooks(EnvironmentMetadata metadata) { return this.hooks; } + // Overridden despite the deprecation so tests can assert both that the configured-plugin + // path still calls it and that registerPlugin does not. + @SuppressWarnings("deprecation") @Override public void onPluginsReady(RegistrationCompleteResult result, EnvironmentMetadata metadata) { onPluginsReadyCalls.add(Map.of( diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java index edea03b1..0c25e20c 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java @@ -84,6 +84,14 @@ public class LDClient implements LDClientInterface, Closeable { // If 15 seconds or more is passed as a timeout to init, we will log a warning. private static final int EXCESSIVE_INIT_WAIT_SECONDS = 15; + // Shared by both registration paths, so that a plugin failing during init and the same plugin + // failing under registerPlugin are reported identically. + private static final String PLUGIN_GET_HOOKS_ERROR = + "Exception thrown getting hooks for plugin {}. Unable to get hooks, plugin will not be registered."; + private static final String PLUGIN_REGISTER_ERROR = "Exception thrown registering plugin {}."; + private static final String PLUGIN_ON_PLUGINS_READY_ERROR = + "Exception thrown executing onPluginsReady for plugin {}."; + /** * Initializes the singleton/primary instance. The result is a {@link Future} which @@ -229,7 +237,7 @@ public static Future init(@NonNull Application application, instance.hookRunner.addHook(hook); } } catch (Exception e) { - logger.error("Exception thrown getting hooks for plugin " + plugin.getMetadata().getName() + ". Unable to get hooks, plugin will not be registered."); + logger.error(PLUGIN_GET_HOOKS_ERROR, pluginName(plugin)); } } @@ -238,8 +246,8 @@ public static Future init(@NonNull Application application, try { plugin.register(instance, metadata); } catch (Exception e) { - pluginFailures.add(new RegistrationCompleteResult.Failure.PluginFailure(plugin.getMetadata().getName(), e.getMessage(), e)); - logger.error("Exception thrown registering plugin " + plugin.getMetadata().getName() + "."); + pluginFailures.add(new RegistrationCompleteResult.Failure.PluginFailure(pluginName(plugin), e.getMessage(), e)); + logger.error(PLUGIN_REGISTER_ERROR, pluginName(plugin)); } } @@ -247,13 +255,7 @@ public static Future init(@NonNull Application application, ? RegistrationCompleteResult.success() : RegistrationCompleteResult.failure(pluginFailures); - for (Plugin plugin : instance.plugins) { - try { - plugin.onPluginsReady(pluginsRegistrationResult, metadata); - } catch (Exception e) { - logger.error("Exception thrown executing onPluginsReady for plugin " + plugin.getMetadata().getName() + "."); - } - } + notifyPluginsReady(instance, metadata, pluginsRegistrationResult, logger); } final AtomicInteger initCounter = new AtomicInteger(config.getMobileKeys().size()); @@ -909,32 +911,41 @@ public void registerPlugin(Plugin plugin) { try { pluginHooks = plugin.getHooks(metadata); } catch (Exception e) { - logger.error("Exception thrown getting hooks for plugin " + pluginName(plugin) + ". Unable to get hooks, plugin will not be registered."); + logger.error(PLUGIN_GET_HOOKS_ERROR, pluginName(plugin)); return; } - RegistrationCompleteResult result; try { plugin.register(this, metadata); - result = RegistrationCompleteResult.success(); - } catch (Exception e) { - logger.error("Exception thrown registering plugin " + pluginName(plugin) + "."); - result = RegistrationCompleteResult.failure(Collections.singletonList( - new RegistrationCompleteResult.Failure.PluginFailure(pluginName(plugin), e.getMessage(), e))); - } - - // The hooks go live only once register has succeeded, so a plugin whose registration failed - // never contributes hooks, and a plugin's own hooks do not observe its register call. - if (result instanceof RegistrationCompleteResult.Success) { + // Adding the hooks only after register returns keeps them from observing the register + // call itself, and leaves a plugin whose registration threw contributing none. for (Hook hook : pluginHooks) { hookRunner.addHook(hook); } + } catch (Exception e) { + logger.error(PLUGIN_REGISTER_ERROR, pluginName(plugin)); } + } - try { - plugin.onPluginsReady(result, metadata); - } catch (Exception e) { - logger.error("Exception thrown executing onPluginsReady for plugin " + pluginName(plugin) + "."); + /** + * Tells every plugin configured on the environment how registering that whole batch went. + * + *

Kept in one place so that the SDK's only remaining call to the deprecated + * {@link Plugin#onPluginsReady} can be removed along with it. + */ + @SuppressWarnings("deprecation") + private static void notifyPluginsReady( + LDClient instance, + EnvironmentMetadata metadata, + RegistrationCompleteResult result, + LDLogger logger + ) { + for (Plugin plugin : instance.plugins) { + try { + plugin.onPluginsReady(result, metadata); + } catch (Exception e) { + logger.error(PLUGIN_ON_PLUGINS_READY_ERROR, pluginName(plugin)); + } } } @@ -942,7 +953,7 @@ public void registerPlugin(Plugin plugin) { * Reads a plugin's name for a log message, tolerating a plugin whose metadata itself throws: * otherwise reporting one failure would raise another out of the handler that reports it. */ - private String pluginName(Plugin plugin) { + private static String pluginName(Plugin plugin) { try { return plugin.getMetadata().getName(); } catch (Exception e) { diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java index da407271..c848cdf8 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java @@ -412,6 +412,8 @@ public interface LDClientInterface extends Closeable { * observe what {@code register} itself does. Exceptions from {@link Plugin#getHooks} or * {@code register} are logged rather than propagated, and leave the plugin contributing no * hooks. Registration covers this client, and so this environment, alone. + * {@link Plugin#onPluginsReady} is not called, because it reports on a batch of plugins + * registered together. * * @param plugin the plugin to register; must not be null */ diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/Plugin.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/Plugin.java index a4119611..401d4c37 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/Plugin.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/Plugin.java @@ -48,6 +48,18 @@ public List getHooks(EnvironmentMetadata metadata) { return Collections.emptyList(); } + /** + * Called once every plugin configured on {@link com.launchdarkly.sdk.android.LDConfig} has been + * registered, reporting whether they all succeeded. + * + * @param result the outcome of registering that batch of plugins + * @param metadata metadata about the environment where the plugin is running. + * @deprecated This reports on a batch of plugins registered together, so it has no meaning for + * {@link LDClient#registerPlugin(Plugin)}, which registers a single plugin and does + * not call it. Do work that needs the client in {@link #register} instead, which + * both paths call. + */ + @Deprecated public void onPluginsReady(RegistrationCompleteResult result, EnvironmentMetadata metadata) { // default: do nothing } From 49e9b192877291b9b4585ac61e16f4493f814f4e Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Mon, 24 Aug 2026 15:38:28 -0700 Subject: [PATCH 3/7] adding hook before --- .../sdk/android/LDClientPluginsTest.java | 22 ++++++++++--------- .../launchdarkly/sdk/android/LDClient.java | 11 +++++----- .../sdk/android/LDClientInterface.java | 9 ++++---- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java b/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java index 9c7b5dfe..ccda4b01 100644 --- a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java +++ b/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java @@ -224,23 +224,24 @@ public void registerPluginActivatesBundledHooks() throws Exception { } @Test - public void registerPluginDoesNotRunTheRegisteringPluginsOwnHooks() throws Exception { + public void registerPluginRunsTheRegisteringPluginsOwnHooks() throws Exception { MockHook testHook = new MockHook(); EvaluateOnRegisterPlugin testPlugin = new EvaluateOnRegisterPlugin(testHook); try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(null), ldContext, 1)) { ldClient.registerPlugin(testPlugin); - // The plugin evaluated a flag from inside register, but its own hooks were not live yet. + // The plugin evaluated a flag from inside register, and its own hooks were already live, + // as they would have been for a plugin configured up front. assertEquals(1, testPlugin.registerCalls.size()); - assertEquals(0, testHook.beforeEvaluationCalls.size()); - assertEquals(0, testHook.afterEvaluationCalls.size()); - - // They do run for evaluations made once registration has completed. - ldClient.boolVariation("test-flag", false); assertEquals(1, testHook.beforeEvaluationCalls.size()); assertEquals(1, testHook.afterEvaluationCalls.size()); + // And they keep running for evaluations made after registration. + ldClient.boolVariation("test-flag", false); + assertEquals(2, testHook.beforeEvaluationCalls.size()); + assertEquals(2, testHook.afterEvaluationCalls.size()); + logging.assertNoErrorsLogged(); } } @@ -289,10 +290,11 @@ public void registerPluginToleratesRegisterThrowing() throws Exception { ldClient.registerPlugin(testPlugin); assertEquals(1, testPlugin.registerCalls.size()); - // A plugin that failed to register contributes no hooks. + // The hooks were already live when register threw, so they stay live, as they do for a + // plugin configured up front whose register throws. ldClient.boolVariation("test-flag", false); - assertEquals(0, testHook.beforeEvaluationCalls.size()); - assertEquals(0, testHook.afterEvaluationCalls.size()); + assertEquals(1, testHook.beforeEvaluationCalls.size()); + assertEquals(1, testHook.afterEvaluationCalls.size()); // The failure is reported in the log alone, since this path does not call // onPluginsReady. diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java index 0c25e20c..f06d2584 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java @@ -915,13 +915,14 @@ public void registerPlugin(Plugin plugin) { return; } + // The hooks go live before register is called, as they do for a plugin configured on + // LDConfig, so that a plugin behaves the same however it was registered. + for (Hook hook : pluginHooks) { + hookRunner.addHook(hook); + } + try { plugin.register(this, metadata); - // Adding the hooks only after register returns keeps them from observing the register - // call itself, and leaves a plugin whose registration threw contributing none. - for (Hook hook : pluginHooks) { - hookRunner.addHook(hook); - } } catch (Exception e) { logger.error(PLUGIN_REGISTER_ERROR, pluginName(plugin)); } diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java index c848cdf8..48d53e50 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java @@ -408,10 +408,11 @@ public interface LDClientInterface extends Closeable { * Registers a single {@link Plugin} with this client after it has been created; to register * plugins beforehand, use the {@code plugins} method of {@link LDConfig.Builder} instead. *

- * The plugin's hooks only start running once {@link Plugin#register} returns, so they do not - * observe what {@code register} itself does. Exceptions from {@link Plugin#getHooks} or - * {@code register} are logged rather than propagated, and leave the plugin contributing no - * hooks. Registration covers this client, and so this environment, alone. + * The plugin's hooks start running before {@link Plugin#register} is called, as they do for a + * plugin configured up front, so they observe what {@code register} itself does. An exception + * from {@link Plugin#getHooks} is logged and leaves the plugin unregistered and contributing no + * hooks; one from {@code register} is logged but leaves the hooks in place. Neither propagates. + * Registration covers this client, and so this environment, alone. * {@link Plugin#onPluginsReady} is not called, because it reports on a batch of plugins * registered together. * From 69dc8a26f5f69d74e400bff641d83dbc6a652c6a Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Tue, 25 Aug 2026 11:49:24 -0700 Subject: [PATCH 4/7] fix comment --- .../com/launchdarkly/sdk/android/integrations/Plugin.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/Plugin.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/Plugin.java index 401d4c37..c2bc4d66 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/Plugin.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/Plugin.java @@ -54,10 +54,8 @@ public List getHooks(EnvironmentMetadata metadata) { * * @param result the outcome of registering that batch of plugins * @param metadata metadata about the environment where the plugin is running. - * @deprecated This reports on a batch of plugins registered together, so it has no meaning for - * {@link LDClient#registerPlugin(Plugin)}, which registers a single plugin and does - * not call it. Do work that needs the client in {@link #register} instead, which - * both paths call. + * @deprecated onPluginsReady was necessary in the past, + * but has since been determined to be problematic and is being removed in the next major version. */ @Deprecated public void onPluginsReady(RegistrationCompleteResult result, EnvironmentMetadata metadata) { From b1bb297a11f9b85b4020c4dd75f80e2f0ca388f6 Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Tue, 25 Aug 2026 14:49:50 -0700 Subject: [PATCH 5/7] refactor to run add hook after the fact --- .../sdk/android/LDClientPluginsTest.java | 100 +++++++++++++++--- .../launchdarkly/sdk/android/LDClient.java | 86 ++++++++------- .../sdk/android/LDClientInterface.java | 10 +- 3 files changed, 142 insertions(+), 54 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java b/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java index ccda4b01..6210781c 100644 --- a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java +++ b/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import android.app.Application; @@ -177,6 +178,81 @@ public void identifyHooksRunForEachEnvironment() throws Exception { } } + @Test + public void configuredPluginThatFailsToRegisterContributesNoHooks() throws Exception { + MockHook testHook = new MockHook(); + MockPlugin testPlugin = new MockPlugin(Collections.singletonList(testHook), false, true); + + try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(List.of(testPlugin)), ldContext, 1)) { + assertEquals(1, testPlugin.registerCalls.size()); + + // The hooks go live only once register has succeeded, so this plugin contributes none. + ldClient.boolVariation("test-flag", false); + assertEquals(0, testHook.beforeEvaluationCalls.size()); + assertEquals(0, testHook.afterEvaluationCalls.size()); + + assertEquals(1, testPlugin.onPluginsReadyCalls.size()); + RegistrationCompleteResult result = + (RegistrationCompleteResult) testPlugin.onPluginsReadyCalls.get(0).get("result"); + assertTrue(result instanceof RegistrationCompleteResult.Failure); + + logging.assertErrorLogged("Exception thrown registering plugin"); + } + } + + @Test + public void configuredPluginWhoseGetHooksThrowsIsNotRegistered() throws Exception { + MockHook testHook = new MockHook(); + MockPlugin testPlugin = new MockPlugin(Collections.singletonList(testHook), true, false); + + try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(List.of(testPlugin)), ldContext, 1)) { + // The logged message says the plugin will not be registered, and it is not. + assertEquals(0, testPlugin.registerCalls.size()); + + ldClient.boolVariation("test-flag", false); + assertEquals(0, testHook.beforeEvaluationCalls.size()); + + logging.assertErrorLogged("Unable to get hooks"); + } + } + + @Test + public void configuredPluginHooksDoNotObserveAnotherPluginsRegister() throws Exception { + MockHook firstHook = new MockHook(); + MockPlugin firstPlugin = new MockPlugin(Collections.singletonList(firstHook)); + // Registers after the first plugin, and evaluates a flag while doing so. + EvaluateOnRegisterPlugin secondPlugin = new EvaluateOnRegisterPlugin(new MockHook()); + + try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(List.of(firstPlugin, secondPlugin)), ldContext, 1)) { + assertEquals(1, secondPlugin.registerCalls.size()); + + // Hooks are activated only once every plugin has registered, so the first plugin's hooks + // did not observe the evaluation the second plugin made while registering. + assertEquals(0, firstHook.beforeEvaluationCalls.size()); + + ldClient.boolVariation("test-flag", false); + assertEquals(1, firstHook.beforeEvaluationCalls.size()); + + logging.assertNoErrorsLogged(); + } + } + + @Test + public void configuredPluginFailureDoesNotPreventOtherPlugins() throws Exception { + MockHook goodHook = new MockHook(); + MockPlugin badPlugin = new MockPlugin(Collections.emptyList(), false, true); + MockPlugin goodPlugin = new MockPlugin(Collections.singletonList(goodHook)); + + try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(List.of(badPlugin, goodPlugin)), ldContext, 1)) { + assertEquals(1, goodPlugin.registerCalls.size()); + + ldClient.boolVariation("test-flag", false); + assertEquals(1, goodHook.beforeEvaluationCalls.size()); + + logging.assertErrorLogged("Exception thrown registering plugin"); + } + } + @Test public void registerPluginPassesClientAndEnvironmentMetadata() throws Exception { MockPlugin testPlugin = new MockPlugin(Collections.emptyList()); @@ -224,23 +300,23 @@ public void registerPluginActivatesBundledHooks() throws Exception { } @Test - public void registerPluginRunsTheRegisteringPluginsOwnHooks() throws Exception { + public void registerPluginDoesNotRunTheRegisteringPluginsOwnHooks() throws Exception { MockHook testHook = new MockHook(); EvaluateOnRegisterPlugin testPlugin = new EvaluateOnRegisterPlugin(testHook); try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(null), ldContext, 1)) { ldClient.registerPlugin(testPlugin); - // The plugin evaluated a flag from inside register, and its own hooks were already live, - // as they would have been for a plugin configured up front. + // The plugin evaluated a flag from inside register, but its own hooks only go live once + // register has returned, as they do for a plugin configured up front. assertEquals(1, testPlugin.registerCalls.size()); - assertEquals(1, testHook.beforeEvaluationCalls.size()); - assertEquals(1, testHook.afterEvaluationCalls.size()); + assertEquals(0, testHook.beforeEvaluationCalls.size()); + assertEquals(0, testHook.afterEvaluationCalls.size()); - // And they keep running for evaluations made after registration. + // They do run for evaluations made once registration has completed. ldClient.boolVariation("test-flag", false); - assertEquals(2, testHook.beforeEvaluationCalls.size()); - assertEquals(2, testHook.afterEvaluationCalls.size()); + assertEquals(1, testHook.beforeEvaluationCalls.size()); + assertEquals(1, testHook.afterEvaluationCalls.size()); logging.assertNoErrorsLogged(); } @@ -290,11 +366,11 @@ public void registerPluginToleratesRegisterThrowing() throws Exception { ldClient.registerPlugin(testPlugin); assertEquals(1, testPlugin.registerCalls.size()); - // The hooks were already live when register threw, so they stay live, as they do for a - // plugin configured up front whose register throws. + // A plugin that failed to register contributes no hooks, as one configured up front + // whose register throws now also does not. ldClient.boolVariation("test-flag", false); - assertEquals(1, testHook.beforeEvaluationCalls.size()); - assertEquals(1, testHook.afterEvaluationCalls.size()); + assertEquals(0, testHook.beforeEvaluationCalls.size()); + assertEquals(0, testHook.afterEvaluationCalls.size()); // The failure is reported in the log alone, since this path does not call // onPluginsReady. diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java index f06d2584..7d5dcf76 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java @@ -225,31 +225,12 @@ public static Future init(@NonNull Application application, instances = newInstances; } - // after instances have been created, set up hooks for each plugin of the instance and call register + // after instances have been created, register each instance's plugins for (LDClient instance : createdInstances) { EnvironmentMetadata metadata = instance.environmentMetadata; - for (Plugin plugin : instance.plugins) { - // try is for each plugin so that if one plugin has an issue, the others will have an opportunity to be used - try { - List pluginHooks = plugin.getHooks(metadata); - for (Hook hook : pluginHooks) { - instance.hookRunner.addHook(hook); - } - } catch (Exception e) { - logger.error(PLUGIN_GET_HOOKS_ERROR, pluginName(plugin)); - } - } - - List pluginFailures = new ArrayList<>(); - for (Plugin plugin : instance.plugins) { - try { - plugin.register(instance, metadata); - } catch (Exception e) { - pluginFailures.add(new RegistrationCompleteResult.Failure.PluginFailure(pluginName(plugin), e.getMessage(), e)); - logger.error(PLUGIN_REGISTER_ERROR, pluginName(plugin)); - } - } + List pluginFailures = + instance.registerPlugins(instance.plugins, metadata); RegistrationCompleteResult pluginsRegistrationResult = pluginFailures.isEmpty() ? RegistrationCompleteResult.success() @@ -905,27 +886,58 @@ public void registerPlugin(Plugin plugin) { throw new NullPointerException("plugin must not be null"); } - EnvironmentMetadata metadata = environmentMetadata; + registerPlugins(Collections.singletonList(plugin), environmentMetadata); + } - List pluginHooks; - try { - pluginHooks = plugin.getHooks(metadata); - } catch (Exception e) { - logger.error(PLUGIN_GET_HOOKS_ERROR, pluginName(plugin)); - return; + /** + * Registers each of {@code plugins} with this client, then activates the hooks contributed by + * those that registered successfully. + *

+ * The hooks go live as the last step, once every plugin has registered, so that no plugin's + * hooks observe any plugin's {@link Plugin#register} call, and a plugin that failed to register + * contributes none. Shared by the plugins configured on {@link LDConfig} and by + * {@link #registerPlugin}, so that a plugin behaves the same however it was registered. + *

+ * Exceptions from a plugin are logged rather than propagated, so that one failing plugin does + * not stop the others being registered. + * + * @param plugins the plugins to register + * @param metadata the environment to describe to the plugins + * @return the failures to report to {@link Plugin#onPluginsReady}, empty if all succeeded + */ + private List registerPlugins( + List plugins, + EnvironmentMetadata metadata + ) { + List failures = new ArrayList<>(); + List hooksToActivate = new ArrayList<>(); + + for (Plugin plugin : plugins) { + List pluginHooks; + try { + pluginHooks = plugin.getHooks(metadata); + } catch (Exception e) { + logger.error(PLUGIN_GET_HOOKS_ERROR, pluginName(plugin)); + failures.add(new RegistrationCompleteResult.Failure.PluginFailure(pluginName(plugin), e.getMessage(), e)); + continue; + } + + try { + plugin.register(this, metadata); + } catch (Exception e) { + logger.error(PLUGIN_REGISTER_ERROR, pluginName(plugin)); + failures.add(new RegistrationCompleteResult.Failure.PluginFailure(pluginName(plugin), e.getMessage(), e)); + continue; + } + + hooksToActivate.addAll(pluginHooks); } - // The hooks go live before register is called, as they do for a plugin configured on - // LDConfig, so that a plugin behaves the same however it was registered. - for (Hook hook : pluginHooks) { + for (Hook hook : hooksToActivate) { hookRunner.addHook(hook); } - try { - plugin.register(this, metadata); - } catch (Exception e) { - logger.error(PLUGIN_REGISTER_ERROR, pluginName(plugin)); - } + return failures; } /** diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java index 48d53e50..a14cc718 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java @@ -408,11 +408,11 @@ public interface LDClientInterface extends Closeable { * Registers a single {@link Plugin} with this client after it has been created; to register * plugins beforehand, use the {@code plugins} method of {@link LDConfig.Builder} instead. *

- * The plugin's hooks start running before {@link Plugin#register} is called, as they do for a - * plugin configured up front, so they observe what {@code register} itself does. An exception - * from {@link Plugin#getHooks} is logged and leaves the plugin unregistered and contributing no - * hooks; one from {@code register} is logged but leaves the hooks in place. Neither propagates. - * Registration covers this client, and so this environment, alone. + * The plugin's hooks only start running once {@link Plugin#register} has returned, as they do + * for a plugin configured up front, so they do not observe what {@code register} itself does. + * Exceptions from {@link Plugin#getHooks} or {@code register} are logged rather than propagated, + * and leave the plugin contributing no hooks. Registration covers this client, and so this + * environment, alone. * {@link Plugin#onPluginsReady} is not called, because it reports on a batch of plugins * registered together. * From 0940c6b4a9a0119bde2b25d16f30952b70906a45 Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Tue, 25 Aug 2026 15:02:27 -0700 Subject: [PATCH 6/7] more --- .../launchdarkly/sdk/android/HookRunner.java | 20 ++++++++- .../launchdarkly/sdk/android/LDClient.java | 6 +-- .../sdk/android/HookRunnerTest.java | 42 +++++++++++++++++++ 3 files changed, 64 insertions(+), 4 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/HookRunner.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/HookRunner.java index 8a4f467e..7472e949 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/HookRunner.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/HookRunner.java @@ -12,6 +12,7 @@ import com.launchdarkly.sdk.android.integrations.TrackSeriesContext; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; @@ -73,8 +74,25 @@ private String getHookName(Hook hook) { * @param hook the hook to add */ public synchronized void addHook(Hook hook) { + addHooks(Collections.singletonList(hook)); + } + + /** + * Adds hooks, which the next series to begin will run. A series already under way runs the hooks it began with. + *

+ * The hooks become visible in a single step, so a series beginning on another thread runs either all of them or + * none of them, never part of the group. Adding the hooks a plugin contributes this way, rather than with repeated + * {@link #addHook(Hook)} calls, keeps hooks that are meant to work as a set from running half applied. + * + * @param hooksToAdd the hooks to add + */ + public synchronized void addHooks(Collection hooksToAdd) { + if (hooksToAdd.isEmpty()) { + return; + } + List updated = new ArrayList<>(hooks); - updated.add(hook); + updated.addAll(hooksToAdd); hooks = Collections.unmodifiableList(updated); } diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java index 7d5dcf76..4b18033e 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java @@ -933,9 +933,9 @@ private List registerPlugins( hooksToActivate.addAll(pluginHooks); } - for (Hook hook : hooksToActivate) { - hookRunner.addHook(hook); - } + // Added in one step, so that a series beginning on another thread cannot run a plugin's hooks + // half applied. + hookRunner.addHooks(hooksToActivate); return failures; } diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/HookRunnerTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/HookRunnerTest.java index 0a86f4da..839ba3e9 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/HookRunnerTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/HookRunnerTest.java @@ -592,6 +592,48 @@ public Map beforeEvaluation(EvaluationSeriesContext seriesContex logging.assertNothingLogged(); } + @Test + public void addsAGroupOfHooksAllAtOnce() throws InterruptedException { + int groupSize = 8; + int groups = 300; + HookRunner runner = new HookRunner(logging.logger, Collections.emptyList()); + AtomicInteger hooksObserved = new AtomicInteger(); + List partialGroups = new ArrayList<>(); + + Thread adder = new Thread(() -> { + for (int i = 0; i < groups; i++) { + List group = new ArrayList<>(groupSize); + for (int j = 0; j < groupSize; j++) { + group.add(new Hook("group-hook") { + @Override + public Map beforeEvaluation(EvaluationSeriesContext seriesContext, Map seriesData) { + hooksObserved.incrementAndGet(); + return seriesData; + } + }); + } + runner.addHooks(group); + } + }); + + EvaluationDetail evaluationResult = EvaluationDetail.fromValue(LDValue.of(true), 1, EvaluationReason.off()); + adder.start(); + while (adder.isAlive()) { + hooksObserved.set(0); + runner.withEvaluation("testMethod", "test-flag", LDContext.create("user-123"), LDValue.of(false), () -> evaluationResult); + int observed = hooksObserved.get(); + if (observed % groupSize != 0) { + partialGroups.add(observed); + } + } + adder.join(); + + // A group of hooks becomes visible in one step, so an evaluation racing the registration runs either + // all of a group's hooks or none of them, never part of one. + assertEquals(Collections.emptyList(), partialGroups); + logging.assertNothingLogged(); + } + @Test public void logsUnknownHookWhenGetMetadataThrows() { String method = "testMethod"; From c4e234d5f02ff433956e1c778a64a81fa7fda275 Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Wed, 26 Aug 2026 10:08:57 -0700 Subject: [PATCH 7/7] add hooks all together but before register --- .../sdk/android/LDClientPluginsTest.java | 42 +++++++++---------- .../launchdarkly/sdk/android/LDClient.java | 30 +++++++------ .../sdk/android/LDClientInterface.java | 10 ++--- 3 files changed, 43 insertions(+), 39 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java b/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java index 6210781c..1e7b092f 100644 --- a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java +++ b/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientPluginsTest.java @@ -179,17 +179,17 @@ public void identifyHooksRunForEachEnvironment() throws Exception { } @Test - public void configuredPluginThatFailsToRegisterContributesNoHooks() throws Exception { + public void configuredPluginThatFailsToRegisterKeepsItsHooks() throws Exception { MockHook testHook = new MockHook(); MockPlugin testPlugin = new MockPlugin(Collections.singletonList(testHook), false, true); try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(List.of(testPlugin)), ldContext, 1)) { assertEquals(1, testPlugin.registerCalls.size()); - // The hooks go live only once register has succeeded, so this plugin contributes none. + // The hooks were already live when register threw, so they stay live. ldClient.boolVariation("test-flag", false); - assertEquals(0, testHook.beforeEvaluationCalls.size()); - assertEquals(0, testHook.afterEvaluationCalls.size()); + assertEquals(1, testHook.beforeEvaluationCalls.size()); + assertEquals(1, testHook.afterEvaluationCalls.size()); assertEquals(1, testPlugin.onPluginsReadyCalls.size()); RegistrationCompleteResult result = @@ -217,7 +217,7 @@ public void configuredPluginWhoseGetHooksThrowsIsNotRegistered() throws Exceptio } @Test - public void configuredPluginHooksDoNotObserveAnotherPluginsRegister() throws Exception { + public void configuredPluginHooksObserveAnotherPluginsRegister() throws Exception { MockHook firstHook = new MockHook(); MockPlugin firstPlugin = new MockPlugin(Collections.singletonList(firstHook)); // Registers after the first plugin, and evaluates a flag while doing so. @@ -226,12 +226,12 @@ public void configuredPluginHooksDoNotObserveAnotherPluginsRegister() throws Exc try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(List.of(firstPlugin, secondPlugin)), ldContext, 1)) { assertEquals(1, secondPlugin.registerCalls.size()); - // Hooks are activated only once every plugin has registered, so the first plugin's hooks - // did not observe the evaluation the second plugin made while registering. - assertEquals(0, firstHook.beforeEvaluationCalls.size()); + // Hooks are activated before any plugin registers, so the first plugin's hooks observed + // the evaluation the second plugin made while registering. + assertEquals(1, firstHook.beforeEvaluationCalls.size()); ldClient.boolVariation("test-flag", false); - assertEquals(1, firstHook.beforeEvaluationCalls.size()); + assertEquals(2, firstHook.beforeEvaluationCalls.size()); logging.assertNoErrorsLogged(); } @@ -300,24 +300,24 @@ public void registerPluginActivatesBundledHooks() throws Exception { } @Test - public void registerPluginDoesNotRunTheRegisteringPluginsOwnHooks() throws Exception { + public void registerPluginRunsTheRegisteringPluginsOwnHooks() throws Exception { MockHook testHook = new MockHook(); EvaluateOnRegisterPlugin testPlugin = new EvaluateOnRegisterPlugin(testHook); try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(null), ldContext, 1)) { ldClient.registerPlugin(testPlugin); - // The plugin evaluated a flag from inside register, but its own hooks only go live once - // register has returned, as they do for a plugin configured up front. + // The plugin evaluated a flag from inside register, and its own hooks were already live + // by then, as they are for a plugin configured up front. assertEquals(1, testPlugin.registerCalls.size()); - assertEquals(0, testHook.beforeEvaluationCalls.size()); - assertEquals(0, testHook.afterEvaluationCalls.size()); - - // They do run for evaluations made once registration has completed. - ldClient.boolVariation("test-flag", false); assertEquals(1, testHook.beforeEvaluationCalls.size()); assertEquals(1, testHook.afterEvaluationCalls.size()); + // And they keep running for evaluations made after registration. + ldClient.boolVariation("test-flag", false); + assertEquals(2, testHook.beforeEvaluationCalls.size()); + assertEquals(2, testHook.afterEvaluationCalls.size()); + logging.assertNoErrorsLogged(); } } @@ -366,11 +366,11 @@ public void registerPluginToleratesRegisterThrowing() throws Exception { ldClient.registerPlugin(testPlugin); assertEquals(1, testPlugin.registerCalls.size()); - // A plugin that failed to register contributes no hooks, as one configured up front - // whose register throws now also does not. + // The hooks were already live when register threw, so they stay live, as they do for a + // plugin configured up front whose register throws. ldClient.boolVariation("test-flag", false); - assertEquals(0, testHook.beforeEvaluationCalls.size()); - assertEquals(0, testHook.afterEvaluationCalls.size()); + assertEquals(1, testHook.beforeEvaluationCalls.size()); + assertEquals(1, testHook.afterEvaluationCalls.size()); // The failure is reported in the log alone, since this path does not call // onPluginsReady. diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java index 4b18033e..f224668a 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java @@ -890,14 +890,15 @@ public void registerPlugin(Plugin plugin) { } /** - * Registers each of {@code plugins} with this client, then activates the hooks contributed by - * those that registered successfully. + * Activates the hooks contributed by {@code plugins}, then registers each of those plugins with + * this client. *

- * The hooks go live as the last step, once every plugin has registered, so that no plugin's - * hooks observe any plugin's {@link Plugin#register} call, and a plugin that failed to register - * contributes none. Shared by the plugins configured on {@link LDConfig} and by - * {@link #registerPlugin}, so that a plugin behaves the same however it was registered. + * The hooks go live before any of the plugins registers, so that a plugin's hooks observe its own + * {@link Plugin#register} call and those of the plugins registered alongside it. Shared by the + * plugins configured on {@link LDConfig} and by {@link #registerPlugin}, so that a plugin behaves + * the same however it was registered. *

+ * A plugin whose {@link Plugin#getHooks} throws contributes no hooks and is not registered. * Exceptions from a plugin are logged rather than propagated, so that one failing plugin does * not stop the others being registered. * @@ -911,6 +912,7 @@ private List registerPlugins( ) { List failures = new ArrayList<>(); List hooksToActivate = new ArrayList<>(); + List pluginsToRegister = new ArrayList<>(); for (Plugin plugin : plugins) { List pluginHooks; @@ -922,21 +924,23 @@ private List registerPlugins( continue; } + hooksToActivate.addAll(pluginHooks); + pluginsToRegister.add(plugin); + } + + // Added in one step, so that a series beginning on another thread cannot run a plugin's hooks + // half applied. + hookRunner.addHooks(hooksToActivate); + + for (Plugin plugin : pluginsToRegister) { try { plugin.register(this, metadata); } catch (Exception e) { logger.error(PLUGIN_REGISTER_ERROR, pluginName(plugin)); failures.add(new RegistrationCompleteResult.Failure.PluginFailure(pluginName(plugin), e.getMessage(), e)); - continue; } - - hooksToActivate.addAll(pluginHooks); } - // Added in one step, so that a series beginning on another thread cannot run a plugin's hooks - // half applied. - hookRunner.addHooks(hooksToActivate); - return failures; } diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java index a14cc718..dd62ec5c 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java @@ -408,11 +408,11 @@ public interface LDClientInterface extends Closeable { * Registers a single {@link Plugin} with this client after it has been created; to register * plugins beforehand, use the {@code plugins} method of {@link LDConfig.Builder} instead. *

- * The plugin's hooks only start running once {@link Plugin#register} has returned, as they do - * for a plugin configured up front, so they do not observe what {@code register} itself does. - * Exceptions from {@link Plugin#getHooks} or {@code register} are logged rather than propagated, - * and leave the plugin contributing no hooks. Registration covers this client, and so this - * environment, alone. + * The plugin's hooks start running before {@link Plugin#register} is called, as they do for a + * plugin configured up front, so they observe what {@code register} itself does. Exceptions from + * {@link Plugin#getHooks} or {@code register} are logged rather than propagated; one from + * {@code getHooks} leaves the plugin contributing no hooks and unregistered. Registration covers + * this client, and so this environment, alone. * {@link Plugin#onPluginsReady} is not called, because it reports on a batch of plugins * registered together. *