From b968e43774654f1f8b875c01845591be631e3b23 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Thu, 6 Oct 2022 23:59:15 -0400 Subject: [PATCH 1/8] fix: add read/write locks to client/api Signed-off-by: Todd Baert --- .../dev/openfeature/sdk/OpenFeatureAPI.java | 51 +++++++++++++------ .../openfeature/sdk/OpenFeatureClient.java | 20 +++++--- .../sdk/internal/AutoCloseableLock.java | 10 ++++ .../AutoCloseableReentrantReadWriteLock.java | 28 ++++++++++ 4 files changed, 88 insertions(+), 21 deletions(-) create mode 100644 src/main/java/dev/openfeature/sdk/internal/AutoCloseableLock.java create mode 100644 src/main/java/dev/openfeature/sdk/internal/AutoCloseableReentrantReadWriteLock.java diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index 5918fa085..46c3df6fd 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -1,21 +1,24 @@ package dev.openfeature.sdk; -import lombok.Getter; -import lombok.Setter; - -import javax.annotation.Nullable; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import javax.annotation.Nullable; + +import dev.openfeature.sdk.internal.AutoCloseableLock; +import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock; +import lombok.Getter; +import lombok.Setter; + /** * A global singleton which holds base configuration for the OpenFeature library. * Configuration here will be shared across all {@link Client}s. */ public class OpenFeatureAPI { - private static OpenFeatureAPI api; + // package-private multi-read/single-write lock + static AutoCloseableReentrantReadWriteLock rwLock = new AutoCloseableReentrantReadWriteLock(); @Getter - @Setter private FeatureProvider provider; @Getter @Setter @@ -23,21 +26,20 @@ public class OpenFeatureAPI { @Getter private List apiHooks; - public OpenFeatureAPI() { + private OpenFeatureAPI() { this.apiHooks = new ArrayList<>(); } + private static class SingletonHolder { + private static final OpenFeatureAPI INSTANCE = new OpenFeatureAPI(); + } + /** * Provisions the {@link OpenFeatureAPI} singleton (if needed) and returns it. * @return The singleton instance. */ public static OpenFeatureAPI getInstance() { - synchronized (OpenFeatureAPI.class) { - if (api == null) { - api = new OpenFeatureAPI(); - } - } - return api; + return SingletonHolder.INSTANCE; } public Metadata getProviderMetadata() { @@ -56,11 +58,30 @@ public Client getClient(@Nullable String name, @Nullable String version) { return new OpenFeatureClient(this, name, version); } + /** + * {@inheritDoc} + */ + public void setProvider(FeatureProvider provider) { + try (AutoCloseableLock __ = rwLock.writeLockAutoCloseable()) { + this.provider = provider; + } + } + + /** + * {@inheritDoc} + */ public void addHooks(Hook... hooks) { - this.apiHooks.addAll(Arrays.asList(hooks)); + try (AutoCloseableLock __ = rwLock.writeLockAutoCloseable()) { + this.apiHooks.addAll(Arrays.asList(hooks)); + } } + /** + * {@inheritDoc} + */ public void clearHooks() { - this.apiHooks.clear(); + try (AutoCloseableLock __ = rwLock.writeLockAutoCloseable()) { + this.apiHooks.clear(); + } } } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index 0f99e4941..30a983739 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -8,6 +8,8 @@ import dev.openfeature.sdk.exceptions.GeneralError; import dev.openfeature.sdk.exceptions.OpenFeatureError; +import dev.openfeature.sdk.internal.AutoCloseableLock; +import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock; import dev.openfeature.sdk.internal.ObjectUtils; import lombok.Getter; import lombok.Setter; @@ -25,6 +27,7 @@ public class OpenFeatureClient implements Client { @Getter private final List clientHooks; private final HookSupport hookSupport; + private AutoCloseableReentrantReadWriteLock rwLock = new AutoCloseableReentrantReadWriteLock(); @Getter @Setter @@ -48,7 +51,9 @@ public OpenFeatureClient(OpenFeatureAPI openFeatureAPI, String name, String vers @Override public void addHooks(Hook... hooks) { - this.clientHooks.addAll(Arrays.asList(hooks)); + try (AutoCloseableLock __ = this.rwLock.writeLockAutoCloseable()) { + this.clientHooks.addAll(Arrays.asList(hooks)); + } } private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key, T defaultValue, @@ -57,16 +62,19 @@ private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key () -> FlagEvaluationOptions.builder().build()); Map hints = Collections.unmodifiableMap(flagOptions.getHookHints()); ctx = ObjectUtils.defaultIfNull(ctx, () -> new MutableContext()); - FeatureProvider provider = ObjectUtils.defaultIfNull(openfeatureApi.getProvider(), () -> { - log.debug("No provider configured, using no-op provider."); - return new NoOpProvider(); - }); + FlagEvaluationDetails details = null; List mergedHooks = null; HookContext hookCtx = null; - try { + try (AutoCloseableLock __ = OpenFeatureAPI.rwLock.readLockAutoCloseable(); + AutoCloseableLock ___ = this.rwLock.readLockAutoCloseable()) { + + FeatureProvider provider = ObjectUtils.defaultIfNull(openfeatureApi.getProvider(), () -> { + log.debug("No provider configured, using no-op provider."); + return new NoOpProvider(); + }); hookCtx = HookContext.from(key, type, this.getMetadata(), openfeatureApi.getProvider().getMetadata(), ctx, defaultValue); diff --git a/src/main/java/dev/openfeature/sdk/internal/AutoCloseableLock.java b/src/main/java/dev/openfeature/sdk/internal/AutoCloseableLock.java new file mode 100644 index 000000000..41fb5dc90 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/internal/AutoCloseableLock.java @@ -0,0 +1,10 @@ +package dev.openfeature.sdk.internal; + +public interface AutoCloseableLock extends AutoCloseable { + + /** + * Override the exception in AutoClosable. + */ + @Override + void close(); +} diff --git a/src/main/java/dev/openfeature/sdk/internal/AutoCloseableReentrantReadWriteLock.java b/src/main/java/dev/openfeature/sdk/internal/AutoCloseableReentrantReadWriteLock.java new file mode 100644 index 000000000..92827ef68 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/internal/AutoCloseableReentrantReadWriteLock.java @@ -0,0 +1,28 @@ +package dev.openfeature.sdk.internal; + +import java.util.concurrent.locks.ReentrantReadWriteLock; + +/** + * A utility class that wraps a multi-read/single-write lock construct as AutoCloseable, so it can + * be used in a try-with-resources. + */ +public class AutoCloseableReentrantReadWriteLock extends ReentrantReadWriteLock { + + /** + * Get the single write lock as an AutoCloseableLock. + * @return unlock method ref + */ + public AutoCloseableLock writeLockAutoCloseable() { + this.writeLock().lock(); + return this.writeLock()::unlock; + } + + /** + * Get the multi read lock as an AutoCloseableLock. + * @return unlock method ref + */ + public AutoCloseableLock readLockAutoCloseable() { + this.readLock().lock(); + return this.readLock()::unlock; + } +} \ No newline at end of file From 4ff9296da91158b8d6eee17dd28bf0c7026f66b6 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Fri, 7 Oct 2022 11:48:35 -0400 Subject: [PATCH 2/8] dont lock entire evaluation Signed-off-by: Todd Baert --- .../openfeature/sdk/OpenFeatureClient.java | 49 +++++++++++-------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index 30a983739..ce827645f 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -67,32 +67,39 @@ private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key FlagEvaluationDetails details = null; List mergedHooks = null; HookContext hookCtx = null; - - try (AutoCloseableLock __ = OpenFeatureAPI.rwLock.readLockAutoCloseable(); - AutoCloseableLock ___ = this.rwLock.readLockAutoCloseable()) { - - FeatureProvider provider = ObjectUtils.defaultIfNull(openfeatureApi.getProvider(), () -> { - log.debug("No provider configured, using no-op provider."); - return new NoOpProvider(); - }); - - hookCtx = HookContext.from(key, type, this.getMetadata(), - openfeatureApi.getProvider().getMetadata(), ctx, defaultValue); - - mergedHooks = ObjectUtils.merge(provider.getProviderHooks(), flagOptions.getHooks(), clientHooks, - openfeatureApi.getApiHooks()); + FeatureProvider provider = null; + + try { + EvaluationContext apiContext; + EvaluationContext clientContext; + + // lock while getting the provider and hooks + try (AutoCloseableLock __ = OpenFeatureAPI.rwLock.readLockAutoCloseable(); + AutoCloseableLock ___ = this.rwLock.readLockAutoCloseable()) { + provider = ObjectUtils.defaultIfNull(openfeatureApi.getProvider(), () -> { + log.debug("No provider configured, using no-op provider."); + return new NoOpProvider(); + }); + + mergedHooks = ObjectUtils.merge(provider.getProviderHooks(), flagOptions.getHooks(), clientHooks, + openfeatureApi.getApiHooks()); + + hookCtx = HookContext.from(key, type, this.getMetadata(), + provider.getMetadata(), ctx, defaultValue); + + // merge of: API.context, client.context, invocation.context + apiContext = openfeatureApi.getEvaluationContext() != null + ? openfeatureApi.getEvaluationContext() + : new MutableContext(); + clientContext = openfeatureApi.getEvaluationContext() != null + ? this.getEvaluationContext() + : new MutableContext(); + } EvaluationContext ctxFromHook = hookSupport.beforeHooks(type, hookCtx, mergedHooks, hints); EvaluationContext invocationCtx = ctx.merge(ctxFromHook); - // merge of: API.context, client.context, invocation.context - EvaluationContext apiContext = openfeatureApi.getEvaluationContext() != null - ? openfeatureApi.getEvaluationContext() - : new MutableContext(); - EvaluationContext clientContext = openfeatureApi.getEvaluationContext() != null - ? this.getEvaluationContext() - : new MutableContext(); EvaluationContext mergedCtx = apiContext.merge(clientContext.merge(invocationCtx)); ProviderEvaluation providerEval = (ProviderEvaluation) createProviderEvaluation(type, key, From 8de427c923d97a423188c6dfd37aa86c8704b258 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Mon, 10 Oct 2022 23:15:21 -0400 Subject: [PATCH 3/8] add tests Signed-off-by: Todd Baert --- pom.xml | 18 +++ spotbugs-exclusions.xml | 7 +- .../dev/openfeature/sdk/OpenFeatureAPI.java | 11 +- .../openfeature/sdk/OpenFeatureClient.java | 18 ++- .../sdk/DeveloperExperienceTest.java | 36 +++++- .../openfeature/sdk/DoSomethingProvider.java | 3 +- .../sdk/FlagEvaluationSpecTest.java | 2 +- .../java/dev/openfeature/sdk/LockingTest.java | 117 ++++++++++++++++++ 8 files changed, 202 insertions(+), 10 deletions(-) create mode 100644 src/test/java/dev/openfeature/sdk/LockingTest.java diff --git a/pom.xml b/pom.xml index e15949da6..9225287f6 100644 --- a/pom.xml +++ b/pom.xml @@ -164,6 +164,21 @@ + + org.codehaus.mojo + build-helper-maven-plugin + 3.3.0 + + + validate + get-cpu-count + + cpu-count + + + + + org.cyclonedx cyclonedx-maven-plugin @@ -231,6 +246,9 @@ ${surefireArgLine} + + ${cpu.count} + false ${testExclusions} diff --git a/spotbugs-exclusions.xml b/spotbugs-exclusions.xml index 8675964d5..de2b4c8e6 100644 --- a/spotbugs-exclusions.xml +++ b/spotbugs-exclusions.xml @@ -9,11 +9,16 @@ - + + + + + + diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index 46c3df6fd..b840b87d7 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -9,7 +9,6 @@ import dev.openfeature.sdk.internal.AutoCloseableLock; import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock; import lombok.Getter; -import lombok.Setter; /** * A global singleton which holds base configuration for the OpenFeature library. @@ -21,7 +20,6 @@ public class OpenFeatureAPI { @Getter private FeatureProvider provider; @Getter - @Setter private EvaluationContext evaluationContext; @Getter private List apiHooks; @@ -58,6 +56,15 @@ public Client getClient(@Nullable String name, @Nullable String version) { return new OpenFeatureClient(this, name, version); } + /** + * {@inheritDoc} + */ + public void setEvaluationContext(EvaluationContext evaluationContext) { + try (AutoCloseableLock __ = rwLock.writeLockAutoCloseable()) { + this.evaluationContext = evaluationContext; + } + } + /** * {@inheritDoc} */ diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index ce827645f..d12fa8024 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -12,7 +12,6 @@ import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock; import dev.openfeature.sdk.internal.ObjectUtils; import lombok.Getter; -import lombok.Setter; import lombok.extern.slf4j.Slf4j; @Slf4j @@ -27,10 +26,9 @@ public class OpenFeatureClient implements Client { @Getter private final List clientHooks; private final HookSupport hookSupport; - private AutoCloseableReentrantReadWriteLock rwLock = new AutoCloseableReentrantReadWriteLock(); + AutoCloseableReentrantReadWriteLock rwLock = new AutoCloseableReentrantReadWriteLock(); @Getter - @Setter private EvaluationContext evaluationContext; /** @@ -49,6 +47,9 @@ public OpenFeatureClient(OpenFeatureAPI openFeatureAPI, String name, String vers this.hookSupport = new HookSupport(); } + /** + * {@inheritDoc} + */ @Override public void addHooks(Hook... hooks) { try (AutoCloseableLock __ = this.rwLock.writeLockAutoCloseable()) { @@ -56,6 +57,16 @@ public void addHooks(Hook... hooks) { } } + /** + * {@inheritDoc} + */ + @Override + public void setEvaluationContext(EvaluationContext evaluationContext) { + try (AutoCloseableLock __ = rwLock.writeLockAutoCloseable()) { + this.evaluationContext = evaluationContext; + } + } + private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key, T defaultValue, EvaluationContext ctx, FlagEvaluationOptions options) { FlagEvaluationOptions flagOptions = ObjectUtils.defaultIfNull(options, @@ -74,6 +85,7 @@ private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key EvaluationContext clientContext; // lock while getting the provider and hooks + // the retrieval any mutable state on client/API MUST be done in this block. try (AutoCloseableLock __ = OpenFeatureAPI.rwLock.readLockAutoCloseable(); AutoCloseableLock ___ = this.rwLock.readLockAutoCloseable()) { provider = ObjectUtils.defaultIfNull(openfeatureApi.getProvider(), () -> { diff --git a/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java b/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java index 454f16709..12e03abe1 100644 --- a/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java +++ b/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java @@ -6,12 +6,14 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import java.util.Arrays; +import java.util.Map; +import java.util.Optional; + import org.junit.jupiter.api.Test; import dev.openfeature.sdk.fixtures.HookFixtures; -import java.util.Arrays; - class DeveloperExperienceTest implements HookFixtures { transient String flagKey = "mykey"; @@ -90,4 +92,34 @@ class DeveloperExperienceTest implements HookFixtures { assertEquals(Reason.ERROR.toString(), retval.getReason()); assertFalse(retval.getValue()); } + + @Test + void providerLockedPerTransaction() throws InterruptedException { + + class MutatingHook implements Hook { + + @Override + // change the provider during a before hook - this should not impact the evaluation in progress + public Optional before(HookContext ctx, Map hints) { + OpenFeatureAPI.getInstance().setProvider(new NoOpProvider()); + return Optional.empty(); + } + } + + final String defaultValue = "string-value"; + final OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + final Client client = api.getClient(); + api.setProvider(new DoSomethingProvider()); + api.addHooks(new MutatingHook()); + + // if provider is changed during an evaluation transaction it should proceed with the original provider + String doSomethingValue = client.getStringValue("val", defaultValue); + assertEquals(new StringBuilder(defaultValue).reverse().toString(), doSomethingValue); + + api.clearHooks(); + + // subsequent evaluations should now use new provider set by hook + String noOpValue = client.getStringValue("val", defaultValue); + assertEquals(noOpValue, defaultValue); + } } diff --git a/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java b/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java index 37fd20f4e..d87fa3749 100644 --- a/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java +++ b/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java @@ -2,6 +2,7 @@ public class DoSomethingProvider implements FeatureProvider { + public static final String name = "Something"; private EvaluationContext savedContext; public EvaluationContext getMergedContext() { @@ -10,7 +11,7 @@ public EvaluationContext getMergedContext() { @Override public Metadata getMetadata() { - return () -> "test"; + return () -> name; } @Override diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java index 75b6e5bb2..92cebea2b 100644 --- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java @@ -53,7 +53,7 @@ private Client _client() { @Test void provider_metadata() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.setProvider(new DoSomethingProvider()); - assertEquals("test", api.getProviderMetadata().getName()); + assertEquals(DoSomethingProvider.name, api.getProviderMetadata().getName()); } @Specification(number="1.1.3", text="The API MUST provide a function to add hooks which accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks. When new hooks are added, previously added hooks are not removed.") diff --git a/src/test/java/dev/openfeature/sdk/LockingTest.java b/src/test/java/dev/openfeature/sdk/LockingTest.java new file mode 100644 index 000000000..296a71c34 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/LockingTest.java @@ -0,0 +1,117 @@ +package dev.openfeature.sdk; + +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.concurrent.locks.ReentrantReadWriteLock; + +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock; + +class LockingTest { + + private static OpenFeatureAPI api; + private OpenFeatureClient client; + private AutoCloseableReentrantReadWriteLock apiRwLock; + private AutoCloseableReentrantReadWriteLock clientRwLock; + private ReentrantReadWriteLock.ReadLock mockClientReadLock; + private ReentrantReadWriteLock.WriteLock mockClientWriteLock; + private ReentrantReadWriteLock.ReadLock mockApiReadLock; + private ReentrantReadWriteLock.WriteLock mockApiWriteLock; + + @BeforeAll + static void beforeAll() { + api = OpenFeatureAPI.getInstance(); + } + + @BeforeEach + void beforeEach() { + client = (OpenFeatureClient)api.getClient(); + + // mock the inner read and write locks + mockClientReadLock = mockInnerReadLock(); + mockClientWriteLock = mockInnerWriteLock(); + mockApiReadLock = mockInnerReadLock(); + mockApiWriteLock = mockInnerWriteLock(); + + // mock the client rwLock + clientRwLock = mock(AutoCloseableReentrantReadWriteLock.class); + when(clientRwLock.readLockAutoCloseable()).thenCallRealMethod(); + when(clientRwLock.readLock()).thenReturn(mockClientReadLock); + when(clientRwLock.writeLockAutoCloseable()).thenCallRealMethod(); + when(clientRwLock.writeLock()).thenReturn(mockClientWriteLock); + client.rwLock = clientRwLock; + + // mock the API rwLock + apiRwLock = mock(AutoCloseableReentrantReadWriteLock.class); + when(apiRwLock.readLockAutoCloseable()).thenCallRealMethod(); + when(apiRwLock.readLock()).thenReturn(mockApiReadLock); + when(apiRwLock.writeLockAutoCloseable()).thenCallRealMethod(); + when(apiRwLock.writeLock()).thenReturn(mockApiWriteLock); + OpenFeatureAPI.rwLock = apiRwLock; + } + + @Test + void evaluationShouldReadLockandReadUnlockClientAndApi() { + client.getBooleanValue("a-key", false); + verify(mockApiReadLock).lock(); + verify(mockApiReadLock).unlock(); + verify(mockClientReadLock).lock(); + verify(mockClientReadLock).unlock(); + } + + @Test + void addHooksShouldWriteLockAndUnlock() { + client.addHooks(new Hook(){}); + verify(mockClientWriteLock).lock(); + verify(mockClientWriteLock).unlock(); + + api.addHooks(new Hook(){}); + verify(mockApiWriteLock).lock(); + verify(mockApiWriteLock).unlock(); + } + + @Test + void setContextShouldWriteLockAndUnlock() { + client.setEvaluationContext(new MutableContext()); + verify(mockClientWriteLock).lock(); + verify(mockClientWriteLock).unlock(); + + api.setEvaluationContext(new MutableContext()); + verify(mockApiWriteLock).lock(); + verify(mockApiWriteLock).unlock(); + } + + @Test + void setProviderShouldWriteLockAndUnlock() { + api.setProvider(new DoSomethingProvider()); + verify(mockApiWriteLock).lock(); + verify(mockApiWriteLock).unlock(); + } + + @Test + void clearHooksShouldWriteLockAndUnlock() { + api.clearHooks(); + verify(mockApiWriteLock).lock(); + verify(mockApiWriteLock).unlock(); + } + + private static ReentrantReadWriteLock.ReadLock mockInnerReadLock() { + ReentrantReadWriteLock.ReadLock readLockMock = mock(ReentrantReadWriteLock.ReadLock.class); + doNothing().when(readLockMock).lock(); + doNothing().when(readLockMock).unlock(); + return readLockMock; + } + + private static ReentrantReadWriteLock.WriteLock mockInnerWriteLock() { + ReentrantReadWriteLock.WriteLock writeLockMock = mock(ReentrantReadWriteLock.WriteLock.class); + doNothing().when(writeLockMock).lock(); + doNothing().when(writeLockMock).unlock(); + return writeLockMock; + } +} \ No newline at end of file From 7ed974cb19091c24a220088d26962ae203835b73 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Mon, 10 Oct 2022 23:16:58 -0400 Subject: [PATCH 4/8] fixup comment Signed-off-by: Todd Baert --- src/main/java/dev/openfeature/sdk/OpenFeatureClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index d12fa8024..9930ac8ec 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -85,7 +85,7 @@ private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key EvaluationContext clientContext; // lock while getting the provider and hooks - // the retrieval any mutable state on client/API MUST be done in this block. + // the retrieval of any mutable state on client/API MUST be done in this block. try (AutoCloseableLock __ = OpenFeatureAPI.rwLock.readLockAutoCloseable(); AutoCloseableLock ___ = this.rwLock.readLockAutoCloseable()) { provider = ObjectUtils.defaultIfNull(openfeatureApi.getProvider(), () -> { From 03485faa96ba37d6a8fe794db4b36d88b0483090 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Tue, 11 Oct 2022 11:45:00 -0400 Subject: [PATCH 5/8] fixup pom comment Signed-off-by: Todd Baert --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 9225287f6..443da5cef 100644 --- a/pom.xml +++ b/pom.xml @@ -246,7 +246,7 @@ ${surefireArgLine} - + ${cpu.count} false From 1e6c182ccd54f4220baa6fa23216bfa24e85c5cf Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Tue, 11 Oct 2022 17:14:52 -0400 Subject: [PATCH 6/8] increase lock granularity, imporove tests Signed-off-by: Todd Baert --- src/main/java/dev/openfeature/sdk/Client.java | 2 +- .../dev/openfeature/sdk/OpenFeatureAPI.java | 43 +++++-- .../openfeature/sdk/OpenFeatureClient.java | 36 ++++-- .../sdk/FlagEvaluationSpecTest.java | 10 +- .../java/dev/openfeature/sdk/LockingTest.java | 111 ++++++++++-------- .../sdk/OpenFeatureClientTest.java | 2 +- 6 files changed, 130 insertions(+), 74 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/Client.java b/src/main/java/dev/openfeature/sdk/Client.java index 07015a633..a4ccf26f9 100644 --- a/src/main/java/dev/openfeature/sdk/Client.java +++ b/src/main/java/dev/openfeature/sdk/Client.java @@ -32,5 +32,5 @@ public interface Client extends Features { * Fetch the hooks associated to this client. * @return A list of {@link Hook}s. */ - List getClientHooks(); + List getHooks(); } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index b840b87d7..e85f4e130 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -8,7 +8,6 @@ import dev.openfeature.sdk.internal.AutoCloseableLock; import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock; -import lombok.Getter; /** * A global singleton which holds base configuration for the OpenFeature library. @@ -16,12 +15,11 @@ */ public class OpenFeatureAPI { // package-private multi-read/single-write lock - static AutoCloseableReentrantReadWriteLock rwLock = new AutoCloseableReentrantReadWriteLock(); - @Getter + static AutoCloseableReentrantReadWriteLock hooksLock = new AutoCloseableReentrantReadWriteLock(); + static AutoCloseableReentrantReadWriteLock providerLock = new AutoCloseableReentrantReadWriteLock(); + static AutoCloseableReentrantReadWriteLock contextLock = new AutoCloseableReentrantReadWriteLock(); private FeatureProvider provider; - @Getter private EvaluationContext evaluationContext; - @Getter private List apiHooks; private OpenFeatureAPI() { @@ -60,34 +58,61 @@ public Client getClient(@Nullable String name, @Nullable String version) { * {@inheritDoc} */ public void setEvaluationContext(EvaluationContext evaluationContext) { - try (AutoCloseableLock __ = rwLock.writeLockAutoCloseable()) { + try (AutoCloseableLock __ = contextLock.writeLockAutoCloseable()) { this.evaluationContext = evaluationContext; } } + /** + * {@inheritDoc} + */ + public EvaluationContext getEvaluationContext() { + try (AutoCloseableLock __ = contextLock.readLockAutoCloseable()) { + return this.evaluationContext; + } + } + /** * {@inheritDoc} */ public void setProvider(FeatureProvider provider) { - try (AutoCloseableLock __ = rwLock.writeLockAutoCloseable()) { + try (AutoCloseableLock __ = providerLock.writeLockAutoCloseable()) { this.provider = provider; } } + /** + * {@inheritDoc} + */ + public FeatureProvider getProvider() { + try (AutoCloseableLock __ = providerLock.readLockAutoCloseable()) { + return this.provider; + } + } + /** * {@inheritDoc} */ public void addHooks(Hook... hooks) { - try (AutoCloseableLock __ = rwLock.writeLockAutoCloseable()) { + try (AutoCloseableLock __ = hooksLock.writeLockAutoCloseable()) { this.apiHooks.addAll(Arrays.asList(hooks)); } } + /** + * {@inheritDoc} + */ + public List getHooks() { + try (AutoCloseableLock __ = hooksLock.readLockAutoCloseable()) { + return this.apiHooks; + } + } + /** * {@inheritDoc} */ public void clearHooks() { - try (AutoCloseableLock __ = rwLock.writeLockAutoCloseable()) { + try (AutoCloseableLock __ = hooksLock.writeLockAutoCloseable()) { this.apiHooks.clear(); } } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index 9930ac8ec..e7ec56044 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -23,12 +23,10 @@ public class OpenFeatureClient implements Client { private final String name; @Getter private final String version; - @Getter private final List clientHooks; private final HookSupport hookSupport; - AutoCloseableReentrantReadWriteLock rwLock = new AutoCloseableReentrantReadWriteLock(); - - @Getter + AutoCloseableReentrantReadWriteLock hooksLock = new AutoCloseableReentrantReadWriteLock(); + AutoCloseableReentrantReadWriteLock contextLock = new AutoCloseableReentrantReadWriteLock(); private EvaluationContext evaluationContext; /** @@ -52,21 +50,41 @@ public OpenFeatureClient(OpenFeatureAPI openFeatureAPI, String name, String vers */ @Override public void addHooks(Hook... hooks) { - try (AutoCloseableLock __ = this.rwLock.writeLockAutoCloseable()) { + try (AutoCloseableLock __ = this.hooksLock.writeLockAutoCloseable()) { this.clientHooks.addAll(Arrays.asList(hooks)); } } + /** + * {@inheritDoc} + */ + @Override + public List getHooks() { + try (AutoCloseableLock __ = this.hooksLock.readLockAutoCloseable()) { + return this.clientHooks; + } + } + /** * {@inheritDoc} */ @Override public void setEvaluationContext(EvaluationContext evaluationContext) { - try (AutoCloseableLock __ = rwLock.writeLockAutoCloseable()) { + try (AutoCloseableLock __ = contextLock.writeLockAutoCloseable()) { this.evaluationContext = evaluationContext; } } + /** + * {@inheritDoc} + */ + @Override + public EvaluationContext getEvaluationContext() { + try (AutoCloseableLock __ = contextLock.readLockAutoCloseable()) { + return this.evaluationContext; + } + } + private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key, T defaultValue, EvaluationContext ctx, FlagEvaluationOptions options) { FlagEvaluationOptions flagOptions = ObjectUtils.defaultIfNull(options, @@ -86,15 +104,15 @@ private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key // lock while getting the provider and hooks // the retrieval of any mutable state on client/API MUST be done in this block. - try (AutoCloseableLock __ = OpenFeatureAPI.rwLock.readLockAutoCloseable(); - AutoCloseableLock ___ = this.rwLock.readLockAutoCloseable()) { + try (AutoCloseableLock __ = this.contextLock.readLockAutoCloseable(); + AutoCloseableLock ___ = this.hooksLock.readLockAutoCloseable()) { provider = ObjectUtils.defaultIfNull(openfeatureApi.getProvider(), () -> { log.debug("No provider configured, using no-op provider."); return new NoOpProvider(); }); mergedHooks = ObjectUtils.merge(provider.getProviderHooks(), flagOptions.getHooks(), clientHooks, - openfeatureApi.getApiHooks()); + openfeatureApi.getHooks()); hookCtx = HookContext.from(key, type, this.getMetadata(), provider.getMetadata(), ctx, defaultValue); diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java index 92cebea2b..0bf9a6d4e 100644 --- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java @@ -63,12 +63,12 @@ private Client _client() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.addHooks(h1); - assertEquals(1, api.getApiHooks().size()); - assertEquals(h1, api.getApiHooks().get(0)); + assertEquals(1, api.getHooks().size()); + assertEquals(h1, api.getHooks().get(0)); api.addHooks(h2); - assertEquals(2, api.getApiHooks().size()); - assertEquals(h2, api.getApiHooks().get(1)); + assertEquals(2, api.getHooks().size()); + assertEquals(h2, api.getHooks().get(1)); } @Specification(number="1.1.5", text="The API MUST provide a function for creating a client which accepts the following options: - name (optional): A logical string identifier for the client.") @@ -85,7 +85,7 @@ private Client _client() { Hook m2 = mock(Hook.class); c.addHooks(m1); c.addHooks(m2); - List hooks = c.getClientHooks(); + List hooks = c.getHooks(); assertEquals(2, hooks.size()); assertTrue(hooks.contains(m1)); assertTrue(hooks.contains(m2)); diff --git a/src/test/java/dev/openfeature/sdk/LockingTest.java b/src/test/java/dev/openfeature/sdk/LockingTest.java index 296a71c34..90ed00669 100644 --- a/src/test/java/dev/openfeature/sdk/LockingTest.java +++ b/src/test/java/dev/openfeature/sdk/LockingTest.java @@ -17,13 +17,12 @@ class LockingTest { private static OpenFeatureAPI api; private OpenFeatureClient client; - private AutoCloseableReentrantReadWriteLock apiRwLock; - private AutoCloseableReentrantReadWriteLock clientRwLock; - private ReentrantReadWriteLock.ReadLock mockClientReadLock; - private ReentrantReadWriteLock.WriteLock mockClientWriteLock; - private ReentrantReadWriteLock.ReadLock mockApiReadLock; - private ReentrantReadWriteLock.WriteLock mockApiWriteLock; - + private AutoCloseableReentrantReadWriteLock apiContextLock; + private AutoCloseableReentrantReadWriteLock apiHooksLock; + private AutoCloseableReentrantReadWriteLock apiProviderLock; + private AutoCloseableReentrantReadWriteLock clientContextLock; + private AutoCloseableReentrantReadWriteLock clientHooksLock; + @BeforeAll static void beforeAll() { api = OpenFeatureAPI.getInstance(); @@ -31,74 +30,77 @@ static void beforeAll() { @BeforeEach void beforeEach() { - client = (OpenFeatureClient)api.getClient(); - - // mock the inner read and write locks - mockClientReadLock = mockInnerReadLock(); - mockClientWriteLock = mockInnerWriteLock(); - mockApiReadLock = mockInnerReadLock(); - mockApiWriteLock = mockInnerWriteLock(); - - // mock the client rwLock - clientRwLock = mock(AutoCloseableReentrantReadWriteLock.class); - when(clientRwLock.readLockAutoCloseable()).thenCallRealMethod(); - when(clientRwLock.readLock()).thenReturn(mockClientReadLock); - when(clientRwLock.writeLockAutoCloseable()).thenCallRealMethod(); - when(clientRwLock.writeLock()).thenReturn(mockClientWriteLock); - client.rwLock = clientRwLock; - - // mock the API rwLock - apiRwLock = mock(AutoCloseableReentrantReadWriteLock.class); - when(apiRwLock.readLockAutoCloseable()).thenCallRealMethod(); - when(apiRwLock.readLock()).thenReturn(mockApiReadLock); - when(apiRwLock.writeLockAutoCloseable()).thenCallRealMethod(); - when(apiRwLock.writeLock()).thenReturn(mockApiWriteLock); - OpenFeatureAPI.rwLock = apiRwLock; + client = (OpenFeatureClient) api.getClient(); + + apiContextLock = setupLock(apiContextLock, mockInnerReadLock(), mockInnerWriteLock()); + apiProviderLock = setupLock(apiProviderLock, mockInnerReadLock(), mockInnerWriteLock()); + apiHooksLock = setupLock(apiHooksLock, mockInnerReadLock(), mockInnerWriteLock()); + OpenFeatureAPI.contextLock = apiContextLock; + OpenFeatureAPI.providerLock = apiProviderLock; + OpenFeatureAPI.hooksLock = apiHooksLock; + + clientContextLock = setupLock(clientContextLock, mockInnerReadLock(), mockInnerWriteLock()); + clientHooksLock = setupLock(clientHooksLock, mockInnerReadLock(), mockInnerWriteLock()); + client.contextLock = clientContextLock; + client.hooksLock = clientHooksLock; } @Test void evaluationShouldReadLockandReadUnlockClientAndApi() { client.getBooleanValue("a-key", false); - verify(mockApiReadLock).lock(); - verify(mockApiReadLock).unlock(); - verify(mockClientReadLock).lock(); - verify(mockClientReadLock).unlock(); + verify(clientHooksLock.readLock()).lock(); + verify(clientHooksLock.readLock()).unlock(); + verify(clientContextLock.readLock()).lock(); + verify(clientContextLock.readLock()).unlock(); } @Test void addHooksShouldWriteLockAndUnlock() { - client.addHooks(new Hook(){}); - verify(mockClientWriteLock).lock(); - verify(mockClientWriteLock).unlock(); - - api.addHooks(new Hook(){}); - verify(mockApiWriteLock).lock(); - verify(mockApiWriteLock).unlock(); + client.addHooks(new Hook() { + }); + verify(clientHooksLock.writeLock()).lock(); + verify(clientHooksLock.writeLock()).unlock(); + + api.addHooks(new Hook() { + }); + verify(apiHooksLock.writeLock()).lock(); + verify(apiHooksLock.writeLock()).unlock(); } @Test void setContextShouldWriteLockAndUnlock() { client.setEvaluationContext(new MutableContext()); - verify(mockClientWriteLock).lock(); - verify(mockClientWriteLock).unlock(); + verify(clientContextLock.writeLock()).lock(); + verify(clientContextLock.writeLock()).unlock(); api.setEvaluationContext(new MutableContext()); - verify(mockApiWriteLock).lock(); - verify(mockApiWriteLock).unlock(); + verify(apiContextLock.writeLock()).lock(); + verify(apiContextLock.writeLock()).unlock(); + } + + @Test + void getContextShouldReadLockAndUnlock() { + client.getEvaluationContext(); + verify(clientContextLock.readLock()).lock(); + verify(clientContextLock.readLock()).unlock(); + + api.getEvaluationContext(); + verify(apiContextLock.readLock()).lock(); + verify(apiContextLock.readLock()).unlock(); } @Test void setProviderShouldWriteLockAndUnlock() { api.setProvider(new DoSomethingProvider()); - verify(mockApiWriteLock).lock(); - verify(mockApiWriteLock).unlock(); + verify(apiProviderLock.writeLock()).lock(); + verify(apiProviderLock.writeLock()).unlock(); } @Test void clearHooksShouldWriteLockAndUnlock() { api.clearHooks(); - verify(mockApiWriteLock).lock(); - verify(mockApiWriteLock).unlock(); + verify(apiHooksLock.writeLock()).lock(); + verify(apiHooksLock.writeLock()).unlock(); } private static ReentrantReadWriteLock.ReadLock mockInnerReadLock() { @@ -114,4 +116,15 @@ private static ReentrantReadWriteLock.WriteLock mockInnerWriteLock() { doNothing().when(writeLockMock).unlock(); return writeLockMock; } + + private AutoCloseableReentrantReadWriteLock setupLock(AutoCloseableReentrantReadWriteLock lock, + AutoCloseableReentrantReadWriteLock.ReadLock readlock, + AutoCloseableReentrantReadWriteLock.WriteLock writeLock) { + lock = mock(AutoCloseableReentrantReadWriteLock.class); + when(lock.readLockAutoCloseable()).thenCallRealMethod(); + when(lock.readLock()).thenReturn(readlock); + when(lock.writeLockAutoCloseable()).thenCallRealMethod(); + when(lock.writeLock()).thenReturn(writeLock); + return lock; + } } \ No newline at end of file diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java index 14446c1aa..eab962ac2 100644 --- a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java +++ b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java @@ -20,7 +20,7 @@ void shouldNotThrowExceptionIfHookHasDifferentTypeArgumentThanHookContext() { TEST_LOGGER.clear(); OpenFeatureAPI api = mock(OpenFeatureAPI.class); when(api.getProvider()).thenReturn(new DoSomethingProvider()); - when(api.getApiHooks()).thenReturn(Arrays.asList(mockBooleanHook(), mockStringHook())); + when(api.getHooks()).thenReturn(Arrays.asList(mockBooleanHook(), mockStringHook())); OpenFeatureClient client = new OpenFeatureClient(api, "name", "version"); From 5ca966361667d11bd473ccec84f3abc10e839750 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Tue, 11 Oct 2022 17:21:27 -0400 Subject: [PATCH 7/8] fix spotbugs Signed-off-by: Todd Baert --- spotbugs-exclusions.xml | 12 ++++- .../openfeature/sdk/OpenFeatureClient.java | 48 +++++++++---------- .../java/dev/openfeature/sdk/LockingTest.java | 27 +++++++---- 3 files changed, 52 insertions(+), 35 deletions(-) diff --git a/spotbugs-exclusions.xml b/spotbugs-exclusions.xml index de2b4c8e6..673bf4b55 100644 --- a/spotbugs-exclusions.xml +++ b/spotbugs-exclusions.xml @@ -9,17 +9,27 @@ - + + + + + + + + + + + diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index e7ec56044..3759ecea7 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -99,32 +99,28 @@ private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key FeatureProvider provider = null; try { - EvaluationContext apiContext; - EvaluationContext clientContext; - - // lock while getting the provider and hooks - // the retrieval of any mutable state on client/API MUST be done in this block. - try (AutoCloseableLock __ = this.contextLock.readLockAutoCloseable(); - AutoCloseableLock ___ = this.hooksLock.readLockAutoCloseable()) { - provider = ObjectUtils.defaultIfNull(openfeatureApi.getProvider(), () -> { - log.debug("No provider configured, using no-op provider."); - return new NoOpProvider(); - }); - - mergedHooks = ObjectUtils.merge(provider.getProviderHooks(), flagOptions.getHooks(), clientHooks, - openfeatureApi.getHooks()); - - hookCtx = HookContext.from(key, type, this.getMetadata(), - provider.getMetadata(), ctx, defaultValue); - - // merge of: API.context, client.context, invocation.context - apiContext = openfeatureApi.getEvaluationContext() != null - ? openfeatureApi.getEvaluationContext() - : new MutableContext(); - clientContext = openfeatureApi.getEvaluationContext() != null - ? this.getEvaluationContext() - : new MutableContext(); - } + final EvaluationContext apiContext; + final EvaluationContext clientContext; + + // openfeatureApi.getProvider() must be called once to maintain a consistent reference + provider = ObjectUtils.defaultIfNull(openfeatureApi.getProvider(), () -> { + log.debug("No provider configured, using no-op provider."); + return new NoOpProvider(); + }); + + mergedHooks = ObjectUtils.merge(provider.getProviderHooks(), flagOptions.getHooks(), clientHooks, + openfeatureApi.getHooks()); + + hookCtx = HookContext.from(key, type, this.getMetadata(), + provider.getMetadata(), ctx, defaultValue); + + // merge of: API.context, client.context, invocation.context + apiContext = openfeatureApi.getEvaluationContext() != null + ? openfeatureApi.getEvaluationContext() + : new MutableContext(); + clientContext = openfeatureApi.getEvaluationContext() != null + ? this.getEvaluationContext() + : new MutableContext(); EvaluationContext ctxFromHook = hookSupport.beforeHooks(type, hookCtx, mergedHooks, hints); diff --git a/src/test/java/dev/openfeature/sdk/LockingTest.java b/src/test/java/dev/openfeature/sdk/LockingTest.java index 90ed00669..f511d8bba 100644 --- a/src/test/java/dev/openfeature/sdk/LockingTest.java +++ b/src/test/java/dev/openfeature/sdk/LockingTest.java @@ -45,14 +45,14 @@ void beforeEach() { client.hooksLock = clientHooksLock; } - @Test - void evaluationShouldReadLockandReadUnlockClientAndApi() { - client.getBooleanValue("a-key", false); - verify(clientHooksLock.readLock()).lock(); - verify(clientHooksLock.readLock()).unlock(); - verify(clientContextLock.readLock()).lock(); - verify(clientContextLock.readLock()).unlock(); - } + // @Test + // void evaluationShouldReadLockandReadUnlockClientAndApi() { + // client.getBooleanValue("a-key", false); + // verify(clientHooksLock.readLock()).lock(); + // verify(clientHooksLock.readLock()).unlock(); + // verify(clientContextLock.readLock()).lock(); + // verify(clientContextLock.readLock()).unlock(); + // } @Test void addHooksShouldWriteLockAndUnlock() { @@ -67,6 +67,17 @@ void addHooksShouldWriteLockAndUnlock() { verify(apiHooksLock.writeLock()).unlock(); } + @Test + void getHooksShouldReadLockAndUnlock() { + client.getHooks(); + verify(clientHooksLock.readLock()).lock(); + verify(clientHooksLock.readLock()).unlock(); + + api.getHooks(); + verify(apiHooksLock.readLock()).lock(); + verify(apiHooksLock.readLock()).unlock(); + } + @Test void setContextShouldWriteLockAndUnlock() { client.setEvaluationContext(new MutableContext()); From bf96c76bf43b5215b667f2548e03dfa80f629d76 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Tue, 11 Oct 2022 18:32:04 -0400 Subject: [PATCH 8/8] remove commented test Signed-off-by: Todd Baert --- src/test/java/dev/openfeature/sdk/LockingTest.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/LockingTest.java b/src/test/java/dev/openfeature/sdk/LockingTest.java index f511d8bba..1a8dac9c3 100644 --- a/src/test/java/dev/openfeature/sdk/LockingTest.java +++ b/src/test/java/dev/openfeature/sdk/LockingTest.java @@ -45,15 +45,6 @@ void beforeEach() { client.hooksLock = clientHooksLock; } - // @Test - // void evaluationShouldReadLockandReadUnlockClientAndApi() { - // client.getBooleanValue("a-key", false); - // verify(clientHooksLock.readLock()).lock(); - // verify(clientHooksLock.readLock()).unlock(); - // verify(clientContextLock.readLock()).lock(); - // verify(clientContextLock.readLock()).unlock(); - // } - @Test void addHooksShouldWriteLockAndUnlock() { client.addHooks(new Hook() {