From ebcb5ce46c3e8b5d02fb0374868029f034e77499 Mon Sep 17 00:00:00 2001 From: Ryan Prayogo <57620+ryanprayogo@users.noreply.github.com> Date: Tue, 20 Aug 2024 22:12:17 -0400 Subject: [PATCH 1/6] fix: Use ConcurrentHashMap for InMemoryProvider Signed-off-by: Ryan Prayogo <57620+ryanprayogo@users.noreply.github.com> --- .../providers/memory/InMemoryProvider.java | 6 +++--- .../memory/InMemoryProviderTest.java | 21 +++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java index 8cd9fc8dc..f6f897829 100644 --- a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java +++ b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java @@ -2,10 +2,10 @@ import java.util.ArrayList; import java.util.Arrays; -import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import dev.openfeature.sdk.EvaluationContext; import dev.openfeature.sdk.EventProvider; @@ -44,7 +44,7 @@ public Metadata getMetadata() { } public InMemoryProvider(Map> flags) { - this.flags = new HashMap<>(flags); + this.flags = new ConcurrentHashMap<>(flags); } /** @@ -67,7 +67,7 @@ public void updateFlags(Map> flags) { Set flagsChanged = new HashSet<>(); flagsChanged.addAll(this.flags.keySet()); flagsChanged.addAll(flags.keySet()); - this.flags = new HashMap<>(flags); + this.flags = new ConcurrentHashMap<>(flags); ProviderEventDetails details = ProviderEventDetails.builder() .flagsChanged(new ArrayList<>(flagsChanged)) .message("flags changed") diff --git a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java index ffdc31822..58426c24b 100644 --- a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java @@ -14,6 +14,8 @@ import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import static dev.openfeature.sdk.Structure.mapToStructure; import static dev.openfeature.sdk.testutils.TestFlagsUtils.buildFlags; @@ -105,4 +107,23 @@ void shouldThrowIfNotInitialized() { // ErrorCode.PROVIDER_NOT_READY should be returned when evaluated via the client assertThrows(ProviderNotReadyError.class, ()-> inMemoryProvider.getBooleanEvaluation("fail_not_initialized", false, new ImmutableContext())); } + + @Test + void multithreadedTest() { + ExecutorService executorService = Executors.newFixedThreadPool(100); + for (int i = 0; i < 10000; i++) { + String flagKey = "multithreadedFlag" + i; + executorService.submit(() -> { + provider.updateFlag(flagKey, Flag.builder() + .variant("on", true) + .variant("off", false) + .defaultVariant("on") + .build()); + }); + } + + for (int i = 0; i < 10000; i++) { + assertTrue(client.getBooleanValue("multithreadedFlag" + i, false)); + } + } } \ No newline at end of file From f2dbbbb226c1d5c33a909aa0e3bb20beb6d01f9f Mon Sep 17 00:00:00 2001 From: Ryan Prayogo <57620+ryanprayogo@users.noreply.github.com> Date: Wed, 21 Aug 2024 14:54:26 -0400 Subject: [PATCH 2/6] fix: make the flags field variable final Signed-off-by: Ryan Prayogo <57620+ryanprayogo@users.noreply.github.com> --- .../providers/memory/InMemoryProvider.java | 14 ++++---- .../memory/InMemoryProviderTest.java | 36 ++++++++++++++++--- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java index f6f897829..a193e8d66 100644 --- a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java +++ b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java @@ -33,7 +33,7 @@ public class InMemoryProvider extends EventProvider { @Getter private static final String NAME = "InMemoryProvider"; - private Map> flags; + private final Map> flags; @Getter private ProviderState state = ProviderState.NOT_READY; @@ -61,13 +61,13 @@ public void initialize(EvaluationContext evaluationContext) throws Exception { /** * Updating provider flags configuration, replacing existing flags. - * @param flags the flags to use instead of the previous flags. + * @param newFlags the flags to use instead of the previous flags. */ - public void updateFlags(Map> flags) { - Set flagsChanged = new HashSet<>(); - flagsChanged.addAll(this.flags.keySet()); - flagsChanged.addAll(flags.keySet()); - this.flags = new ConcurrentHashMap<>(flags); + public void updateFlags(Map> newFlags) { + Set flagsChanged = new HashSet<>(newFlags.keySet()); + flagsChanged.removeAll(this.flags.keySet()); + this.flags.putAll(newFlags); + ProviderEventDetails details = ProviderEventDetails.builder() .flagsChanged(new ArrayList<>(flagsChanged)) .message("flags changed") diff --git a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java index 58426c24b..e2c1e041d 100644 --- a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java @@ -2,6 +2,7 @@ import com.google.common.collect.ImmutableMap; import dev.openfeature.sdk.Client; +import dev.openfeature.sdk.EventDetails; import dev.openfeature.sdk.ImmutableContext; import dev.openfeature.sdk.OpenFeatureAPI; import dev.openfeature.sdk.Value; @@ -9,13 +10,17 @@ import dev.openfeature.sdk.exceptions.ProviderNotReadyError; import dev.openfeature.sdk.exceptions.TypeMismatchError; import lombok.SneakyThrows; -import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.function.Consumer; import static dev.openfeature.sdk.Structure.mapToStructure; import static dev.openfeature.sdk.testutils.TestFlagsUtils.buildFlags; @@ -23,6 +28,8 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -34,8 +41,8 @@ class InMemoryProviderTest { private static InMemoryProvider provider; @SneakyThrows - @BeforeAll - static void beforeAll() { + @BeforeEach + void beforeEach() { Map> flags = buildFlags(); provider = spy(new InMemoryProvider(flags)); OpenFeatureAPI.getInstance().onProviderConfigurationChanged(eventDetails -> {}); @@ -108,20 +115,39 @@ void shouldThrowIfNotInitialized() { assertThrows(ProviderNotReadyError.class, ()-> inMemoryProvider.getBooleanEvaluation("fail_not_initialized", false, new ImmutableContext())); } + @SuppressWarnings("unchecked") @Test - void multithreadedTest() { + void emitChangedFlagsOnlyIfThereAreChangedFlags() { + Consumer handler = mock(Consumer.class); + Map> flags = buildFlags(); + + OpenFeatureAPI.getInstance().onProviderConfigurationChanged(handler); + OpenFeatureAPI.getInstance().setProviderAndWait(provider); + + provider.updateFlags(flags); + + verify(handler, times(1)) + .accept(argThat(details -> details.getFlagsChanged().isEmpty())); + } + + @Test + void multithreadedTest() throws InterruptedException { ExecutorService executorService = Executors.newFixedThreadPool(100); + List> updates = new ArrayList<>(); for (int i = 0; i < 10000; i++) { String flagKey = "multithreadedFlag" + i; - executorService.submit(() -> { + updates.add(() -> { provider.updateFlag(flagKey, Flag.builder() .variant("on", true) .variant("off", false) .defaultVariant("on") .build()); + return null; }); } + executorService.invokeAll(updates); + for (int i = 0; i < 10000; i++) { assertTrue(client.getBooleanValue("multithreadedFlag" + i, false)); } From ce27c39e4011677dc4cc6fb20e15f20671930e36 Mon Sep 17 00:00:00 2001 From: Ryan Prayogo <57620+ryanprayogo@users.noreply.github.com> Date: Wed, 21 Aug 2024 14:56:10 -0400 Subject: [PATCH 3/6] chore: Use Collections.singletonList instead of Arrays.asList Signed-off-by: Ryan Prayogo <57620+ryanprayogo@users.noreply.github.com> --- .../sdk/providers/memory/InMemoryProvider.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java index a193e8d66..4b38f9d9b 100644 --- a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java +++ b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java @@ -1,12 +1,5 @@ package dev.openfeature.sdk.providers.memory; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; - import dev.openfeature.sdk.EvaluationContext; import dev.openfeature.sdk.EventProvider; import dev.openfeature.sdk.Metadata; @@ -24,6 +17,13 @@ import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + /** * In-memory provider. */ @@ -82,7 +82,7 @@ public void updateFlags(Map> newFlags) { public void updateFlag(String flagKey, Flag flag) { this.flags.put(flagKey, flag); ProviderEventDetails details = ProviderEventDetails.builder() - .flagsChanged(Arrays.asList(flagKey)) + .flagsChanged(Collections.singletonList(flagKey)) .message("flag added/updated") .build(); emitProviderConfigurationChanged(details); From 69f3976aa9832c19c87159f7eaefc02f8c9ff445 Mon Sep 17 00:00:00 2001 From: Ryan Prayogo <57620+ryanprayogo@users.noreply.github.com> Date: Wed, 21 Aug 2024 17:15:28 -0400 Subject: [PATCH 4/6] chore: Update javadoc and parameter name Signed-off-by: Ryan Prayogo <57620+ryanprayogo@users.noreply.github.com> --- .../sdk/providers/memory/InMemoryProvider.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java index 4b38f9d9b..3365b96a8 100644 --- a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java +++ b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java @@ -48,7 +48,7 @@ public InMemoryProvider(Map> flags) { } /** - * Initialize the provider. + * Initializes the provider. * @param evaluationContext evaluation context * @throws Exception on error */ @@ -60,8 +60,10 @@ public void initialize(EvaluationContext evaluationContext) throws Exception { } /** - * Updating provider flags configuration, replacing existing flags. - * @param newFlags the flags to use instead of the previous flags. + * Updates the provider flags configuration. + * For existing flags, the new configurations replace the old one. + * For new flags, they are added to the configuration. + * @param newFlags the new flag configurations */ public void updateFlags(Map> newFlags) { Set flagsChanged = new HashSet<>(newFlags.keySet()); @@ -76,11 +78,13 @@ public void updateFlags(Map> newFlags) { } /** - * Updating provider flags configuration with adding or updating a flag. - * @param flag the flag to update. If a flag with this key already exists, new flag replaces it. + * Updates a single provider flag configuration. + * For existing flag, the new configuration replaces the old one. + * For new flag, they are added to the configuration. + * @param newFlag the flag to update */ - public void updateFlag(String flagKey, Flag flag) { - this.flags.put(flagKey, flag); + public void updateFlag(String flagKey, Flag newFlag) { + this.flags.put(flagKey, newFlag); ProviderEventDetails details = ProviderEventDetails.builder() .flagsChanged(Collections.singletonList(flagKey)) .message("flag added/updated") From e139de4f5653f1db5dbea56eaa5f333ec9c63d31 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Thu, 22 Aug 2024 09:12:30 -0400 Subject: [PATCH 5/6] fixup: await verify Signed-off-by: Todd Baert --- .../sdk/providers/memory/InMemoryProviderTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java index e2c1e041d..747371d5d 100644 --- a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java @@ -27,6 +27,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.awaitility.Awaitility.await; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.mock; @@ -126,8 +127,8 @@ void emitChangedFlagsOnlyIfThereAreChangedFlags() { provider.updateFlags(flags); - verify(handler, times(1)) - .accept(argThat(details -> details.getFlagsChanged().isEmpty())); + await().untilAsserted(() -> verify(handler, times(1)) + .accept(argThat(details -> details.getFlagsChanged().isEmpty()))); } @Test From e7bdeceb751a8789860c33dd5db5ed0577f59a04 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Thu, 22 Aug 2024 15:45:23 -0400 Subject: [PATCH 6/6] fixup: remove test, key diffing Signed-off-by: Todd Baert --- .../providers/memory/InMemoryProvider.java | 1 - .../memory/InMemoryProviderTest.java | 30 +------------------ 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java index 3365b96a8..5b6d28702 100644 --- a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java +++ b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java @@ -67,7 +67,6 @@ public void initialize(EvaluationContext evaluationContext) throws Exception { */ public void updateFlags(Map> newFlags) { Set flagsChanged = new HashSet<>(newFlags.keySet()); - flagsChanged.removeAll(this.flags.keySet()); this.flags.putAll(newFlags); ProviderEventDetails details = ProviderEventDetails.builder() diff --git a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java index 747371d5d..cb7770d48 100644 --- a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java @@ -13,13 +13,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import java.util.ArrayList; import java.util.HashMap; -import java.util.List; import java.util.Map; -import java.util.concurrent.Callable; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import java.util.function.Consumer; import static dev.openfeature.sdk.Structure.mapToStructure; @@ -128,29 +123,6 @@ void emitChangedFlagsOnlyIfThereAreChangedFlags() { provider.updateFlags(flags); await().untilAsserted(() -> verify(handler, times(1)) - .accept(argThat(details -> details.getFlagsChanged().isEmpty()))); - } - - @Test - void multithreadedTest() throws InterruptedException { - ExecutorService executorService = Executors.newFixedThreadPool(100); - List> updates = new ArrayList<>(); - for (int i = 0; i < 10000; i++) { - String flagKey = "multithreadedFlag" + i; - updates.add(() -> { - provider.updateFlag(flagKey, Flag.builder() - .variant("on", true) - .variant("off", false) - .defaultVariant("on") - .build()); - return null; - }); - } - - executorService.invokeAll(updates); - - for (int i = 0; i < 10000; i++) { - assertTrue(client.getBooleanValue("multithreadedFlag" + i, false)); - } + .accept(argThat(details -> details.getFlagsChanged().size() == buildFlags().size()))); } } \ No newline at end of file