From 03de2758229706804a1a8d841abdb46a892940e4 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Wed, 22 Jan 2025 12:39:40 +0100 Subject: [PATCH] fixup! feat: improve wait logic to a more elegant solution #1160 Signed-off-by: christian.lutnik --- .../providers/flagd/FlagdProvider.java | 13 ++++++++++-- .../flagd/FlagdProviderSyncResources.java | 2 ++ .../flagd/FlagdProviderSyncResourcesTest.java | 20 +++++++++---------- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java index bdd1611fa..7d8e18a8a 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java @@ -208,8 +208,13 @@ EvaluationContext getEnrichedContext() { private void onProviderEvent(FlagdProviderEvent flagdProviderEvent) { synchronized (syncResources) { - log.info("FlagdProviderEvent: {} of type {}", flagdProviderEvent, flagdProviderEvent.getClass().getName()); - log.info("FlagdProviderEvent event {} ", flagdProviderEvent.getEvent().toString()); + log.info( + "FlagdProviderEvent: {} of type {}", + flagdProviderEvent, + flagdProviderEvent.getClass().getName()); + log.info( + "FlagdProviderEvent event {} ", + flagdProviderEvent.getEvent().toString()); syncResources.setSyncMetadata(flagdProviderEvent.getSyncMetadata()); if (flagdProviderEvent.getSyncMetadata() != null) { syncResources.setEnrichedContext(contextEnricher.apply(flagdProviderEvent.getSyncMetadata())); @@ -231,6 +236,9 @@ private void onProviderEvent(FlagdProviderEvent flagdProviderEvent) { break; } log.info("config change not ready"); + onReady(); + syncResources.setPreviousEvent(ProviderEvent.PROVIDER_READY); + break; // intentional fall through, a not-ready change will trigger a ready. case PROVIDER_READY: onReady(); @@ -270,6 +278,7 @@ private void onReady() { } this.emitProviderReady( ProviderEventDetails.builder().message("connected to flagd").build()); + log.info("post onready"); } private void onError() { 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 a5015dbd5..6cab637da 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 @@ -46,6 +46,7 @@ public synchronized boolean initialize() { } this.initialized = true; this.notifyAll(); + log.info("notified all"); return true; } @@ -77,6 +78,7 @@ public void waitForInitialization(long deadline) { long remaining = end - now; synchronized (this) { if (initialized) { // might have changed in the meantime + log.info("post wait for init in loop"); return; } if (isShutDown) { diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderSyncResourcesTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderSyncResourcesTest.java index 3180e5723..4da832249 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderSyncResourcesTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderSyncResourcesTest.java @@ -30,15 +30,11 @@ void waitForInitialization_waitsApproxForDeadline() { final AtomicLong start = new AtomicLong(); final AtomicLong end = new AtomicLong(); final long deadline = 45; - Assertions.assertThrows(GeneralError.class, () -> { - start.set(System.currentTimeMillis()); - try { - flagdProviderSyncResources.waitForInitialization(deadline); - } catch (Exception e) { - end.set(System.currentTimeMillis()); - throw e; - } - }); + + start.set(System.currentTimeMillis()); + Assertions.assertThrows(GeneralError.class, () -> flagdProviderSyncResources.waitForInitialization(deadline)); + end.set(System.currentTimeMillis()); + final long elapsed = end.get() - start.get(); // should wait at least for the deadline Assertions.assertTrue(elapsed >= deadline); @@ -54,7 +50,9 @@ void interruptingWaitingThread_isIgnored() throws InterruptedException { Thread waitingThread = new Thread(() -> { long start = System.currentTimeMillis(); isWaiting.set(true); - flagdProviderSyncResources.waitForInitialization(deadline); + Assertions.assertThrows( + GeneralError.class, () -> flagdProviderSyncResources.waitForInitialization(deadline)); + long end = System.currentTimeMillis(); long duration = end - start; // even though thread was interrupted, it still waited for the deadline @@ -110,7 +108,7 @@ void callingShutdown_wakesUpWaitingThreadWithException() throws InterruptedExcep long start = System.currentTimeMillis(); isWaiting.set(true); Assertions.assertThrows( - IllegalArgumentException.class, () -> flagdProviderSyncResources.waitForInitialization(10000)); + IllegalStateException.class, () -> flagdProviderSyncResources.waitForInitialization(10000)); long end = System.currentTimeMillis(); long duration = end - start;