diff --git a/.gitmodules b/.gitmodules index e439dea5d..b302279b9 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,7 +4,7 @@ [submodule "providers/flagd/test-harness"] path = providers/flagd/test-harness url = https://github.com/open-feature/test-harness.git - branch = v0.5.21 + branch = v1.1.0 [submodule "providers/flagd/spec"] path = providers/flagd/spec url = https://github.com/open-feature/spec.git diff --git a/providers/flagd/pom.xml b/providers/flagd/pom.xml index 138adc430..84410503d 100644 --- a/providers/flagd/pom.xml +++ b/providers/flagd/pom.xml @@ -156,14 +156,13 @@ test - 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 ea0ab6941..7f517a7c1 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 @@ -114,7 +114,7 @@ public synchronized void initialize(EvaluationContext evaluationContext) throws // block till ready - this works with deadline fine for rpc, but with in_process we also need to take parsing // into the equation // TODO: evaluate where we are losing time, so we can remove this magic number - follow up - Util.busyWaitAndCheck(this.deadline + 200, () -> initialized); + Util.busyWaitAndCheck(this.deadline + 500, () -> initialized); } @Override @@ -195,15 +195,19 @@ private void onProviderEvent(FlagdProviderEvent flagdProviderEvent) { enrichedContext = contextEnricher.apply(flagdProviderEvent.getSyncMetadata()); } + /* + We only use Error and Ready as previous states. + As error will first be emitted as Stale, and only turns after a while into an emitted Error. + Ready is needed, as the InProcessResolver does not have a dedicated ready event, hence we need to + forward a configuration changed to the ready, if we are not in the ready state. + */ switch (flagdProviderEvent.getEvent()) { case PROVIDER_CONFIGURATION_CHANGED: if (previousEvent == ProviderEvent.PROVIDER_READY) { - this.emitProviderConfigurationChanged(ProviderEventDetails.builder() - .flagsChanged(flagdProviderEvent.getFlagsChanged()) - .message("configuration changed") - .build()); + onConfigurationChanged(flagdProviderEvent); break; } + // intentional fall through, a not-ready change will trigger a ready. case PROVIDER_READY: onReady(); previousEvent = ProviderEvent.PROVIDER_READY; @@ -220,6 +224,13 @@ private void onProviderEvent(FlagdProviderEvent flagdProviderEvent) { } } + private void onConfigurationChanged(FlagdProviderEvent flagdProviderEvent) { + this.emitProviderConfigurationChanged(ProviderEventDetails.builder() + .flagsChanged(flagdProviderEvent.getFlagsChanged()) + .message("configuration changed") + .build()); + } + private void onReady() { if (!initialized) { initialized = true; diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java index 0a7ab91a0..3033d31f4 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java @@ -112,7 +112,6 @@ void observeEventStream(final BlockingQueue writeTo, final AtomicB try { metadataResponse = grpcConnector .getResolver() - .withDeadlineAfter(deadline, TimeUnit.MILLISECONDS) .getMetadata(metadataRequest.build()); } catch (Exception e) { // the chances this call fails but the syncRequest does not are slim @@ -122,6 +121,7 @@ void observeEventStream(final BlockingQueue writeTo, final AtomicB metadataException = e; } + log.info("stream"); while (!shutdown.get()) { final GrpcResponseModel response = streamReceiver.take(); if (response.isComplete()) { 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 index 43c9ac2e9..e719f4bcf 100644 --- 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 @@ -61,7 +61,6 @@ import java.util.function.Consumer; import java.util.function.Function; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.mockito.MockedConstruction; @@ -320,13 +319,6 @@ void resolvers_should_not_cache_responses_if_not_static() { do_resolvers_cache_responses(DEFAULT.toString(), true, false); } - @Test - @Disabled( - "This test seems to be wrong on the way, we are handling caching, as we return values as long as we are in stale mode") - void resolvers_should_not_cache_responses_if_event_stream_not_alive() { - do_resolvers_cache_responses(STATIC_REASON, false, false); - } - @Test void context_is_parsed_and_passed_to_grpc_service() { final String BOOLEAN_ATTR_KEY = "bool-attr"; diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnectorTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnectorTest.java index 0229b2660..596042bb5 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnectorTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnectorTest.java @@ -17,7 +17,6 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -71,39 +70,6 @@ private void tearDownGrpcServer() throws InterruptedException { } } - @Test - @Disabled("not sure this test makes sense in this kind of way") - void whenShuttingDownAndRestartingGrpcServer_ConsumerReceivesDisconnectedAndConnectedEvent() throws Exception { - CountDownLatch sync = new CountDownLatch(2); - ArrayList connectionStateChanges = Lists.newArrayList(); - Consumer testConsumer = event -> { - connectionStateChanges.add(!event.isDisconnected()); - sync.countDown(); - }; - - GrpcConnector instance = new GrpcConnector<>( - FlagdOptions.builder().build(), - ServiceGrpc::newStub, - ServiceGrpc::newBlockingStub, - testConsumer, - stub -> stub.eventStream(Evaluation.EventStreamRequest.getDefaultInstance(), mockEventStreamObserver), - testChannel); - - instance.initialize(); - - // when shutting down server - testServer.shutdown(); - testServer.awaitTermination(1, TimeUnit.SECONDS); - - // when restarting server - setupTestGrpcServer(); - - // then consumer received DISCONNECTED and CONNECTED event - boolean finished = sync.await(10, TimeUnit.SECONDS); - Assertions.assertTrue(finished); - Assertions.assertEquals(Lists.newArrayList(DISCONNECTED, CONNECTED), connectionStateChanges); - } - @Test void whenShuttingDownGrpcConnector_ConsumerReceivesDisconnectedEvent() throws Exception { CountDownLatch sync = new CountDownLatch(1); diff --git a/providers/flagd/test-harness b/providers/flagd/test-harness index a4faffc71..dd6b67067 160000 --- a/providers/flagd/test-harness +++ b/providers/flagd/test-harness @@ -1 +1 @@ -Subproject commit a4faffc71e632b734699503427db998f2ef86edf +Subproject commit dd6b67067a3c146f5b7dd414637905f7ec369901