fix: enriched context lost on some events (#1226)

Signed-off-by: Todd Baert <[email protected]>
open-feature · Feb 18, 2025 · aefa941 · aefa941
1 parent 020c9a1
commit aefa941
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 24 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
@@ -10,7 +10,6 @@
 import dev.openfeature.sdk.EventProvider;
 import dev.openfeature.sdk.Hook;
 import dev.openfeature.sdk.ImmutableContext;
-import dev.openfeature.sdk.ImmutableStructure;
 import dev.openfeature.sdk.Metadata;
 import dev.openfeature.sdk.ProviderEvaluation;
 import dev.openfeature.sdk.ProviderEvent;
@@ -184,19 +183,6 @@ public ProviderEvaluation<Value> getObjectEvaluation(String key, Value defaultVa
         return flagResolver.objectEvaluation(key, defaultValue, ctx);
     }
 
-    /**
-     * An unmodifiable view of a Structure representing the latest result of the
-     * SyncMetadata.
-     * Set on initial connection and updated with every reconnection.
-     * see:
-     * https://buf.build/open-feature/flagd/docs/main:flagd.sync.v1#flagd.sync.v1.FlagSyncService.GetMetadata
-     *
-     * @return Object map representing sync metadata
-     */
-    protected Structure getSyncMetadata() {
-        return new ImmutableStructure(eventsLock.syncMetadata.asMap());
-    }
-
     /**
      * The updated context mixed into all evaluations based on the sync-metadata.
      *
@@ -211,10 +197,6 @@ private void onProviderEvent(FlagdProviderEvent flagdProviderEvent) {
 
         synchronized (eventsLock) {
             log.info("FlagdProviderEvent: {}", flagdProviderEvent.getEvent());
-            eventsLock.syncMetadata = flagdProviderEvent.getSyncMetadata();
-            if (flagdProviderEvent.getSyncMetadata() != null) {
-                eventsLock.enrichedContext = contextEnricher.apply(flagdProviderEvent.getSyncMetadata());
-            }
 
             /*
              * We only use Error and Ready as previous states.
@@ -233,6 +215,13 @@ private void onProviderEvent(FlagdProviderEvent flagdProviderEvent) {
                     }
                     // intentional fall through, a not-ready change will trigger a ready.
                 case PROVIDER_READY:
+                    /*
+                     * Sync metadata is used to enrich the context, and is immutable in flagd,
+                     * so we only need it to be fetched once at READY.
+                     */
+                    if (flagdProviderEvent.getSyncMetadata() != null) {
+                        eventsLock.enrichedContext = contextEnricher.apply(flagdProviderEvent.getSyncMetadata());
+                    }
                     onReady();
                     eventsLock.previousEvent = ProviderEvent.PROVIDER_READY;
                     break;
@@ -304,7 +293,6 @@ private void onError() {
      */
     static class EventsLock {
         volatile ProviderEvent previousEvent = null;
-        volatile Structure syncMetadata = new ImmutableStructure();
         volatile boolean initialized = false;
         volatile EvaluationContext enrichedContext = new ImmutableContext();
     }

diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java
@@ -669,7 +669,6 @@ void updatesSyncMetadataWithCallback() throws Exception {
             provider.initialize(ctx);
 
             // the onConnectionEvent should have updated the sync metadata and the
-            assertEquals(val, provider.getSyncMetadata().getValue(key).asString());
             assertEquals(val, provider.getEnrichedContext().getValue(key).asString());
 
             // call the hook manually and make sure the enriched context is returned

diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFileTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFileTest.java
@@ -28,7 +28,7 @@
 @ConfigurationParameter(key = GLUE_PROPERTY_NAME, value = "dev.openfeature.contrib.providers.flagd.e2e.steps")
 @ConfigurationParameter(key = OBJECT_FACTORY_PROPERTY_NAME, value = "io.cucumber.picocontainer.PicoFactory")
 @IncludeTags("file")
-@ExcludeTags({"unixsocket", "targetURI", "reconnect", "customCert", "events", "flagdcontext"})
+@ExcludeTags({"unixsocket", "targetURI", "reconnect", "customCert", "events", "contextEnrichment"})
 @Testcontainers
 public class RunFileTest {
 

diff --git a/...ers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java b/...ers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java
@@ -17,6 +17,8 @@
 @Slf4j
 public class EventSteps extends AbstractSteps {
 
+    private static final int EVENT_TIMEOUT_MS = 12_000;
+
     public EventSteps(State state) {
         super(state);
         state.events = new LinkedList<>();
@@ -47,12 +49,12 @@ public void a_stale_event_handler(String eventType) {
 
     @When("a {} event was fired")
     public void eventWasFired(String eventType) throws InterruptedException {
-        eventHandlerShouldBeExecutedWithin(eventType, 8000);
+        eventHandlerShouldBeExecutedWithin(eventType, EVENT_TIMEOUT_MS);
     }
 
     @Then("the {} event handler should have been executed")
     public void eventHandlerShouldBeExecuted(String eventType) {
-        eventHandlerShouldBeExecutedWithin(eventType, 10000);
+        eventHandlerShouldBeExecutedWithin(eventType, EVENT_TIMEOUT_MS);
     }
 
     @Then("the {} event handler should have been executed within {int}ms")

diff --git a/providers/flagd/test-harness b/providers/flagd/test-harness