add unit test for flag cache invalidation

thomaspoignant · thomaspoignant · commit 1867697e0514 · 2024-06-11T20:58:53.000+02:00
Signed-off-by: Thomas Poignant &lt;thomas.poignant@gofeatureflag.org&gt;
diff --git a/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProvider.java b/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProvider.java
@@ -115,12 +115,15 @@ public void initialize(EvaluationContext evaluationContext) throws Exception {
 
         if (options.getEnableCache() == null || options.getEnableCache()) {
             this.cacheCtrl = CacheController.builder().options(options).build();
-            this.dataCollectorHook = new DataCollectorHook(DataCollectorHookOptions.builder()
-                    .flushIntervalMs(options.getFlushIntervalMs())
-                    .gofeatureflagController(this.gofeatureflagController)
-                    .maxPendingEvents(options.getMaxPendingEvents())
-                    .build());
-            this.hooks.add(this.dataCollectorHook);
+
+            if (!this.options.isDisableDataCollection()) {
+                this.dataCollectorHook = new DataCollectorHook(DataCollectorHookOptions.builder()
+                        .flushIntervalMs(options.getFlushIntervalMs())
+                        .gofeatureflagController(this.gofeatureflagController)
+                        .maxPendingEvents(options.getMaxPendingEvents())
+                        .build());
+                this.hooks.add(this.dataCollectorHook);
+            }
             this.flagChangeDisposable =
                     this.startCheckFlagConfigurationChangesDaemon();
         }
@@ -147,7 +150,7 @@ private Disposable startCheckFlagConfigurationChangesDaemon() {
                 .takeUntil(stopSignal)
                 .flatMap(tick -> Observable.fromCallable(() -> this.gofeatureflagController.configurationHasChanged())
                         .onErrorResumeNext(e -> {
-                            log.error("error while calling flag change API, error", e);
+                            log.error("error while calling flag change API", e);
                             if (e instanceof ConfigurationChangeEndpointNotFound) {
                                 // emit an item to stop the interval to stop the loop
                                 stopSignal.onNext(new Object());
@@ -164,6 +167,8 @@ private Disposable startCheckFlagConfigurationChangesDaemon() {
                                 this.cacheCtrl.invalidateAll();
                                 super.emitProviderConfigurationChanged(ProviderEventDetails.builder()
                                         .message("GO Feature Flag Configuration changed, clearing the cache").build());
+                            } else {
+                                log.debug("flag configuration has not changed: {}", response);
                             }
                         },
                         throwable -> log.error("error while calling flag change API, error: {}", throwable.getMessage())
@@ -242,7 +247,7 @@ private <T> ProviderEvaluation<T> getEvaluation(
 
     @Override
     public void shutdown() {
-        log.info("shutdown");
+        log.debug("shutdown");
         if (this.dataCollectorHook != null) {
             this.dataCollectorHook.shutdown();
         }
diff --git a/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProviderOptions.java b/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProviderOptions.java
@@ -86,4 +86,10 @@ public class GoFeatureFlagProviderOptions {
      * default: 120000
      */
     private Long flagChangePollingIntervalMs;
+
+    /**
+     * (optional) disableDataCollection set to true if you don't want to collect the usage of flags retrieved in the cache.
+     * default: false
+     */
+    private boolean disableDataCollection;
 }
diff --git a/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/bean/ConfigurationChange.java b/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/bean/ConfigurationChange.java
@@ -4,6 +4,7 @@
  * ConfigurationChange is an enum to represent the change of the configuration.
  */
 public enum ConfigurationChange {
+    FLAG_CONFIGURATION_INITIALIZED,
     FLAG_CONFIGURATION_UPDATED,
     FLAG_CONFIGURATION_NOT_CHANGED
 }
diff --git a/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/controller/GoFeatureFlagController.java b/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/controller/GoFeatureFlagController.java
@@ -253,7 +253,7 @@ public ConfigurationChange configurationHasChanged() throws GoFeatureFlagExcepti
         HttpUrl url = this.parsedEndpoint.newBuilder()
                 .addEncodedPathSegment("v1")
                 .addEncodedPathSegment("flag")
-                .addEncodedPathSegment("chang1e")
+                .addEncodedPathSegment("change")
                 .build();
 
         Request.Builder reqBuilder = new Request.Builder()
@@ -281,8 +281,11 @@ public ConfigurationChange configurationHasChanged() throws GoFeatureFlagExcepti
                 throw new ConfigurationChangeEndpointUnknownErr();
             }
 
-            this.etag = response.header("ETag");
-            return ConfigurationChange.FLAG_CONFIGURATION_UPDATED;
+            boolean isInitialConfiguration = this.etag == null;
+            this.etag = response.header(HttpHeaders.ETAG);
+            return isInitialConfiguration
+                    ? ConfigurationChange.FLAG_CONFIGURATION_INITIALIZED
+                    : ConfigurationChange.FLAG_CONFIGURATION_UPDATED;
         } catch (IOException e) {
             throw new ConfigurationChangeEndpointUnknownErr(e);
         }
diff --git a/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/hook/events/EventsPublisher.java b/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/hook/events/EventsPublisher.java
@@ -1,7 +1,7 @@
 package dev.openfeature.contrib.providers.gofeatureflag.hook.events;
 
 
-import dev.openfeature.contrib.providers.gofeatureflag.concurrent.ConcurrentUtils;
+import dev.openfeature.contrib.providers.gofeatureflag.util.ConcurrentUtils;
 import lombok.extern.slf4j.Slf4j;
 
 import java.util.ArrayList;
diff --git a/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/util/ConcurrentUtils.java b/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/util/ConcurrentUtils.java
@@ -1,4 +1,4 @@
-package dev.openfeature.contrib.providers.gofeatureflag.concurrent;
+package dev.openfeature.contrib.providers.gofeatureflag.util;
 
 import lombok.AccessLevel;
 import lombok.NoArgsConstructor;
diff --git a/providers/go-feature-flag/src/test/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProviderTest.java b/providers/go-feature-flag/src/test/java/dev/openfeature/contrib/providers/gofeatureflag/GoFeatureFlagProviderTest.java
@@ -11,6 +11,7 @@
 import java.util.Map;
 
 import com.google.common.cache.CacheBuilder;
+import com.google.common.net.HttpHeaders;
 import dev.openfeature.sdk.Client;
 import dev.openfeature.sdk.ErrorCode;
 import dev.openfeature.sdk.EvaluationContext;
@@ -49,6 +50,8 @@
 @Slf4j
 class GoFeatureFlagProviderTest {
     private int publishEventsRequestsReceived = 0;
+    private int flagChangeCallCounter = 0;
+    private boolean flagChanged404 = false;
 
     // Dispatcher is the configuration of the mock server to test the provider.
     final Dispatcher dispatcher = new Dispatcher() {
@@ -78,8 +81,21 @@ public MockResponse dispatch(RecordedRequest request) {
                 }
                 return new MockResponse().setResponseCode(200);
             }
-            if (request.getPath().startsWith("/v1/flag/change")) {
-                return new MockResponse().setResponseCode(200).setHeader("etag", "123456");
+            if (request.getPath().contains("/v1/flag/change")) {
+                flagChangeCallCounter++;
+                if (flagChanged404) {
+                    return new MockResponse().setResponseCode(404);
+                }
+                if (flagChangeCallCounter == 2) {
+                    return new MockResponse().setResponseCode(200).setHeader(HttpHeaders.ETAG, "7891011");
+                }
+                if (request.getHeader(HttpHeaders.IF_NONE_MATCH) != null
+                        && (request.getHeader(HttpHeaders.IF_NONE_MATCH).equals("123456")
+                        || request.getHeader(HttpHeaders.IF_NONE_MATCH).equals("7891011"))) {
+                    return new MockResponse().setResponseCode(304);
+                }
+
+                return new MockResponse().setResponseCode(200).setHeader(HttpHeaders.ETAG, "123456");
             }
             return new MockResponse().setResponseCode(404);
         }
@@ -98,6 +114,8 @@ public MockResponse dispatch(RecordedRequest request) {
 
     @BeforeEach
     void beforeEach(TestInfo testInfo) throws IOException {
+        this.flagChangeCallCounter = 0;
+        this.flagChanged404 = false;
         this.testName = testInfo.getDisplayName();
         this.server = new MockWebServer();
         this.server.setDispatcher(dispatcher);
@@ -777,6 +795,50 @@ void should_publish_events_context_without_anonymous() {
         assertEquals(3, publishEventsRequestsReceived, "We pass the flush interval, we should have 3 events");
     }
 
+    @SneakyThrows
+    @Test
+    void should_not_get_cached_value_if_flag_configuration_changed() {
+        this.evaluationContext = new MutableContext("d45e303a-38c2-11ed-a261-0242ac120002");
+        GoFeatureFlagProvider g = new GoFeatureFlagProvider(GoFeatureFlagProviderOptions.builder()
+                .endpoint(this.baseUrl.toString())
+                .timeout(1000)
+                .disableDataCollection(true)
+                .enableCache(true)
+                .flagChangePollingIntervalMs(100L)
+                .disableDataCollection(true)
+                .build());
+        String providerName = this.testName;
+        OpenFeatureAPI.getInstance().setProviderAndWait(providerName, g);
+        Client client = OpenFeatureAPI.getInstance().getClient(providerName);
+        FlagEvaluationDetails<Boolean> got = client.getBooleanDetails("bool_targeting_match", false, this.evaluationContext);
+        assertEquals(Reason.TARGETING_MATCH.name(), got.getReason());
+        got = client.getBooleanDetails("bool_targeting_match", false, this.evaluationContext);
+        assertEquals(Reason.CACHED.name(), got.getReason());
+        got = client.getBooleanDetails("bool_targeting_match", false, this.evaluationContext);
+        assertEquals(Reason.CACHED.name(), got.getReason());
+        Thread.sleep(200L);
+        got = client.getBooleanDetails("bool_targeting_match", false, this.evaluationContext);
+        assertEquals(Reason.TARGETING_MATCH.name(), got.getReason());
+    }
+
+    @SneakyThrows
+    @Test
+    void should_stop_calling_flag_change_if_receive_404() {
+        this.flagChanged404 = true;
+        this.evaluationContext = new MutableContext("d45e303a-38c2-11ed-a261-0242ac120002");
+        GoFeatureFlagProvider g = new GoFeatureFlagProvider(GoFeatureFlagProviderOptions.builder()
+                .endpoint(this.baseUrl.toString())
+                .timeout(1000)
+                .enableCache(true)
+                .flagChangePollingIntervalMs(10L)
+                .build());
+        String providerName = this.testName;
+        OpenFeatureAPI.getInstance().setProviderAndWait(providerName, g);
+        Client client = OpenFeatureAPI.getInstance().getClient(providerName);
+        Thread.sleep(150L);
+        assertEquals(1, this.flagChangeCallCounter);
+    }
+
     private String readMockResponse(String filename) throws Exception {
         URL url = getClass().getClassLoader().getResource("mock_responses/" + filename);
         assert url != null;

Original file line number	Diff line number	Diff line change
`@@ -4,6 +4,7 @@`
`4`	`4`	`* ConfigurationChange is an enum to represent the change of the configuration.`
`5`	`5`	`*/`
`6`	`6`	`public enum ConfigurationChange {`
	`7`	`+ FLAG_CONFIGURATION_INITIALIZED,`
`7`	`8`	`FLAG_CONFIGURATION_UPDATED,`
`8`	`9`	`FLAG_CONFIGURATION_NOT_CHANGED`
`9`	`10`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-package dev.openfeature.contrib.providers.gofeatureflag.concurrent;`
	`1`	`+package dev.openfeature.contrib.providers.gofeatureflag.util;`
`2`	`2`
`3`	`3`	`import lombok.AccessLevel;`
`4`	`4`	`import lombok.NoArgsConstructor;`