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..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 @@ -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,245 @@ public void identifyHooksRunForEachEnvironment() throws Exception { } } + @Test + 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 were already live when register threw, so they stay live. + ldClient.boolVariation("test-flag", false); + assertEquals(1, testHook.beforeEvaluationCalls.size()); + assertEquals(1, 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 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. + 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 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(2, 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()); + + 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 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, and its own hooks were already live + // by then, as they are for a plugin configured up front. + assertEquals(1, testPlugin.registerCalls.size()); + 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(); + } + } + + @Test + public void registerPluginDoesNotCallOnPluginsReady() throws Exception { + MockPlugin testPlugin = new MockPlugin(Collections.emptyList()); + + try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(null), ldContext, 1)) { + ldClient.registerPlugin(testPlugin); + + // 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(); + } + } + + @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()); + + // 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(1, testHook.beforeEvaluationCalls.size()); + assertEquals(1, testHook.afterEvaluationCalls.size()); + + // 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"); + } + } + + @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 +434,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 +481,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 +492,61 @@ 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; } + + // 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( + "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/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 d593d25d..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 @@ -80,9 +80,18 @@ 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; + // 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 @@ -133,7 +142,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 +210,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); @@ -215,44 +225,18 @@ 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 - for (Map.Entry entry : instanceMetadatas.entrySet()) { - LDClient instance = entry.getKey(); - EnvironmentMetadata metadata = entry.getValue(); - - 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("Exception thrown getting hooks for plugin " + plugin.getMetadata().getName() + ". Unable to get hooks, plugin will not be registered."); - } - } + // after instances have been created, register each instance's plugins + for (LDClient instance : createdInstances) { + EnvironmentMetadata metadata = instance.environmentMetadata; - List pluginFailures = new ArrayList<>(); - for (Plugin plugin : instance.plugins) { - 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() + "."); - } - } + List pluginFailures = + instance.registerPlugins(instance.plugins, metadata); RegistrationCompleteResult pluginsRegistrationResult = pluginFailures.isEmpty() ? 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()); @@ -895,4 +879,102 @@ 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"); + } + + registerPlugins(Collections.singletonList(plugin), environmentMetadata); + } + + /** + * Activates the hooks contributed by {@code plugins}, then registers each of those plugins with + * this client. + *

+ * 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. + * + * @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<>(); + List pluginsToRegister = 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; + } + + 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)); + } + } + + return failures; + } + + /** + * 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)); + } + } + } + + /** + * 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 static 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..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 @@ -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,20 @@ 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 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. + * + * @param plugin the plugin to register; must not be null + */ + void registerPlugin(Plugin plugin); } 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..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 @@ -48,6 +48,16 @@ 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 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) { // default: do nothing } 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";