Skip to content

Commit

Permalink
fixup: fixing nit, and adding information for fallthrough
Browse files Browse the repository at this point in the history
Signed-off-by: Simon Schrottner <[email protected]>
  • Loading branch information
aepfli committed Jan 16, 2025
1 parent 92a7729 commit ff5bccf
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 52 deletions.
2 changes: 1 addition & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 1 addition & 2 deletions providers/flagd/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -156,14 +156,13 @@
<scope>test</scope>
</dependency>
<!-- uncomment for logoutput during test runs -->
<!--

<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-simple</artifactId>
<version>2.0.16</version>
<scope>test</scope>
</dependency>
-->
</dependencies>

<dependencyManagement>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ void observeEventStream(final BlockingQueue<QueuePayload> 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
Expand All @@ -122,6 +121,7 @@ void observeEventStream(final BlockingQueue<QueuePayload> writeTo, final AtomicB
metadataException = e;
}

log.info("stream");
while (!shutdown.get()) {
final GrpcResponseModel response = streamReceiver.take();
if (response.isComplete()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Boolean> connectionStateChanges = Lists.newArrayList();
Consumer<FlagdProviderEvent> testConsumer = event -> {
connectionStateChanges.add(!event.isDisconnected());
sync.countDown();
};

GrpcConnector<ServiceGrpc.ServiceStub, ServiceGrpc.ServiceBlockingStub> 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);
Expand Down
2 changes: 1 addition & 1 deletion providers/flagd/test-harness

0 comments on commit ff5bccf

Please sign in to comment.