From 82b542b7f3fc38f8409c12f18ed79aa4c0317960 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Tue, 21 Jan 2025 13:19:06 +0100 Subject: [PATCH] fixup! feat: improve wait logic to a more elegant solution #1160 Signed-off-by: christian.lutnik --- .../flagd/FlagdProviderSyncResources.java | 4 +++- ...va => FlagdProviderSyncResourcesTest.java} | 24 +++++++++---------- 2 files changed, 15 insertions(+), 13 deletions(-) rename providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/{EventsLockTest.java => FlagdProviderSyncResourcesTest.java} (84%) diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProviderSyncResources.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProviderSyncResources.java index 8a0c9c062..2a1221049 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProviderSyncResources.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProviderSyncResources.java @@ -1,5 +1,6 @@ package dev.openfeature.contrib.providers.flagd; +import com.google.common.annotations.VisibleForTesting; import dev.openfeature.sdk.EvaluationContext; import dev.openfeature.sdk.ImmutableContext; import dev.openfeature.sdk.ImmutableStructure; @@ -11,9 +12,10 @@ /** * Contains all fields we need to worry about locking, used as intrinsic lock - * for sync blocks. + * for sync blocks in the {@link FlagdProvider}. */ @Getter +@VisibleForTesting public class FlagdProviderSyncResources { @Setter private volatile ProviderEvent previousEvent = null; diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/EventsLockTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/FlagdProviderSyncResourcesTest.java similarity index 84% rename from providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/EventsLockTest.java rename to providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/FlagdProviderSyncResourcesTest.java index 466ad79cf..b311a2d5c 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/EventsLockTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/FlagdProviderSyncResourcesTest.java @@ -9,20 +9,20 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; -class EventsLockTest { +class FlagdProviderSyncResourcesTest { private static final long PERMISSIBLE_EPSILON = 20; - private FlagdProviderSyncResources eventsLock; + private FlagdProviderSyncResources flagdProviderSyncResources; @BeforeEach void setUp() { - eventsLock = new FlagdProviderSyncResources(); + flagdProviderSyncResources = new FlagdProviderSyncResources(); } @Timeout(2) @Test void waitForInitialization_failsWhenDeadlineElapses() { - Assertions.assertThrows(GeneralError.class, () -> eventsLock.waitForInitialization(2)); + Assertions.assertThrows(GeneralError.class, () -> flagdProviderSyncResources.waitForInitialization(2)); } @Timeout(2) @@ -34,7 +34,7 @@ void waitForInitialization_waitsApproxForDeadline() { Assertions.assertThrows(GeneralError.class, () -> { start.set(System.currentTimeMillis()); try { - eventsLock.waitForInitialization(deadline); + flagdProviderSyncResources.waitForInitialization(deadline); } catch (Exception e) { end.set(System.currentTimeMillis()); throw e; @@ -55,7 +55,7 @@ void interruptingWaitingThread_isIgnored() throws InterruptedException { Thread waitingThread = new Thread(() -> { long start = System.currentTimeMillis(); isWaiting.set(true); - eventsLock.waitForInitialization(deadline); + flagdProviderSyncResources.waitForInitialization(deadline); long end = System.currentTimeMillis(); long duration = end - start; // even though thread was interrupted, it still waited for the deadline @@ -85,7 +85,7 @@ void callingInitialize_wakesUpWaitingThread() throws InterruptedException { Thread waitingThread = new Thread(() -> { long start = System.currentTimeMillis(); isWaiting.set(true); - eventsLock.waitForInitialization(10000); + flagdProviderSyncResources.waitForInitialization(10000); long end = System.currentTimeMillis(); long duration = end - start; Assertions.assertTrue(duration < PERMISSIBLE_EPSILON); @@ -98,7 +98,7 @@ void callingInitialize_wakesUpWaitingThread() throws InterruptedException { Thread.sleep(PERMISSIBLE_EPSILON); // waitingThread should have started waiting in the meantime - eventsLock.initialize(); + flagdProviderSyncResources.initialize(); waitingThread.join(); } @@ -110,7 +110,7 @@ void callingShutdown_wakesUpWaitingThreadWithException() throws InterruptedExcep Thread waitingThread = new Thread(() -> { long start = System.currentTimeMillis(); isWaiting.set(true); - Assertions.assertThrows(IllegalArgumentException.class, () -> eventsLock.waitForInitialization(10000)); + Assertions.assertThrows(IllegalArgumentException.class, () -> flagdProviderSyncResources.waitForInitialization(10000)); long end = System.currentTimeMillis(); long duration = end - start; @@ -124,7 +124,7 @@ void callingShutdown_wakesUpWaitingThreadWithException() throws InterruptedExcep Thread.sleep(PERMISSIBLE_EPSILON); // waitingThread should have started waiting in the meantime - eventsLock.shutdown(); + flagdProviderSyncResources.shutdown(); waitingThread.join(); } @@ -132,9 +132,9 @@ void callingShutdown_wakesUpWaitingThreadWithException() throws InterruptedExcep @Timeout(2) @Test void waitForInitializationAfterCallingInitialize_returnsInstantly() { - eventsLock.initialize(); + flagdProviderSyncResources.initialize(); long start = System.currentTimeMillis(); - eventsLock.waitForInitialization(10000); + flagdProviderSyncResources.waitForInitialization(10000); long end = System.currentTimeMillis(); // do not use PERMISSIBLE_EPSILON here, this should happen faster than that Assertions.assertTrue(start + 1 >= end);