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 index c1a7c9297..0b4dcfc46 100644 --- 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 ProviderEvaluation 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 index 73a0eada2..c8d38fc30 100644 --- 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,11 @@ 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 index e8eca895a..ec39208a5 100644 --- 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 index 7a579fac2..ab357d657 100644 --- 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 index c188f1eb7..f14ac12d5 100644 --- 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/concurrent/ConcurrentUtils.java b/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/util/ConcurrentUtils.java similarity index 96% rename from providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/concurrent/ConcurrentUtils.java rename to providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/util/ConcurrentUtils.java index c9f3f908b..1f9bd80b5 100644 --- a/providers/go-feature-flag/src/main/java/dev/openfeature/contrib/providers/gofeatureflag/concurrent/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 index aa1643a89..027abc041 100644 --- 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 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;