Skip to content

Commit ff5bccf

Browse files
committed
fixup: fixing nit, and adding information for fallthrough
Signed-off-by: Simon Schrottner <[email protected]>
1 parent 92a7729 commit ff5bccf

File tree

7 files changed

+20
-52
lines changed

7 files changed

+20
-52
lines changed

Diff for: .gitmodules

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
[submodule "providers/flagd/test-harness"]
55
path = providers/flagd/test-harness
66
url = https://github.com/open-feature/test-harness.git
7-
branch = v0.5.21
7+
branch = v1.1.0
88
[submodule "providers/flagd/spec"]
99
path = providers/flagd/spec
1010
url = https://github.com/open-feature/spec.git

Diff for: providers/flagd/pom.xml

+1-2
Original file line numberDiff line numberDiff line change
@@ -156,14 +156,13 @@
156156
<scope>test</scope>
157157
</dependency>
158158
<!-- uncomment for logoutput during test runs -->
159-
<!--
159+
160160
<dependency>
161161
<groupId>org.slf4j</groupId>
162162
<artifactId>slf4j-simple</artifactId>
163163
<version>2.0.16</version>
164164
<scope>test</scope>
165165
</dependency>
166-
-->
167166
</dependencies>
168167

169168
<dependencyManagement>

Diff for: providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java

+16-5
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ public synchronized void initialize(EvaluationContext evaluationContext) throws
114114
// block till ready - this works with deadline fine for rpc, but with in_process we also need to take parsing
115115
// into the equation
116116
// TODO: evaluate where we are losing time, so we can remove this magic number - follow up
117-
Util.busyWaitAndCheck(this.deadline + 200, () -> initialized);
117+
Util.busyWaitAndCheck(this.deadline + 500, () -> initialized);
118118
}
119119

120120
@Override
@@ -195,15 +195,19 @@ private void onProviderEvent(FlagdProviderEvent flagdProviderEvent) {
195195
enrichedContext = contextEnricher.apply(flagdProviderEvent.getSyncMetadata());
196196
}
197197

198+
/*
199+
We only use Error and Ready as previous states.
200+
As error will first be emitted as Stale, and only turns after a while into an emitted Error.
201+
Ready is needed, as the InProcessResolver does not have a dedicated ready event, hence we need to
202+
forward a configuration changed to the ready, if we are not in the ready state.
203+
*/
198204
switch (flagdProviderEvent.getEvent()) {
199205
case PROVIDER_CONFIGURATION_CHANGED:
200206
if (previousEvent == ProviderEvent.PROVIDER_READY) {
201-
this.emitProviderConfigurationChanged(ProviderEventDetails.builder()
202-
.flagsChanged(flagdProviderEvent.getFlagsChanged())
203-
.message("configuration changed")
204-
.build());
207+
onConfigurationChanged(flagdProviderEvent);
205208
break;
206209
}
210+
// intentional fall through, a not-ready change will trigger a ready.
207211
case PROVIDER_READY:
208212
onReady();
209213
previousEvent = ProviderEvent.PROVIDER_READY;
@@ -220,6 +224,13 @@ private void onProviderEvent(FlagdProviderEvent flagdProviderEvent) {
220224
}
221225
}
222226

227+
private void onConfigurationChanged(FlagdProviderEvent flagdProviderEvent) {
228+
this.emitProviderConfigurationChanged(ProviderEventDetails.builder()
229+
.flagsChanged(flagdProviderEvent.getFlagsChanged())
230+
.message("configuration changed")
231+
.build());
232+
}
233+
223234
private void onReady() {
224235
if (!initialized) {
225236
initialized = true;

Diff for: providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,6 @@ void observeEventStream(final BlockingQueue<QueuePayload> writeTo, final AtomicB
112112
try {
113113
metadataResponse = grpcConnector
114114
.getResolver()
115-
.withDeadlineAfter(deadline, TimeUnit.MILLISECONDS)
116115
.getMetadata(metadataRequest.build());
117116
} catch (Exception e) {
118117
// the chances this call fails but the syncRequest does not are slim
@@ -122,6 +121,7 @@ void observeEventStream(final BlockingQueue<QueuePayload> writeTo, final AtomicB
122121
metadataException = e;
123122
}
124123

124+
log.info("stream");
125125
while (!shutdown.get()) {
126126
final GrpcResponseModel response = streamReceiver.take();
127127
if (response.isComplete()) {

Diff for: providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java

-8
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@
6161
import java.util.function.Consumer;
6262
import java.util.function.Function;
6363
import org.junit.jupiter.api.BeforeAll;
64-
import org.junit.jupiter.api.Disabled;
6564
import org.junit.jupiter.api.Test;
6665
import org.mockito.MockedConstruction;
6766

@@ -320,13 +319,6 @@ void resolvers_should_not_cache_responses_if_not_static() {
320319
do_resolvers_cache_responses(DEFAULT.toString(), true, false);
321320
}
322321

323-
@Test
324-
@Disabled(
325-
"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")
326-
void resolvers_should_not_cache_responses_if_event_stream_not_alive() {
327-
do_resolvers_cache_responses(STATIC_REASON, false, false);
328-
}
329-
330322
@Test
331323
void context_is_parsed_and_passed_to_grpc_service() {
332324
final String BOOLEAN_ATTR_KEY = "bool-attr";

Diff for: providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnectorTest.java

-34
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import org.junit.jupiter.api.AfterEach;
1818
import org.junit.jupiter.api.Assertions;
1919
import org.junit.jupiter.api.BeforeEach;
20-
import org.junit.jupiter.api.Disabled;
2120
import org.junit.jupiter.api.Test;
2221
import org.mockito.Mock;
2322
import org.mockito.MockitoAnnotations;
@@ -71,39 +70,6 @@ private void tearDownGrpcServer() throws InterruptedException {
7170
}
7271
}
7372

74-
@Test
75-
@Disabled("not sure this test makes sense in this kind of way")
76-
void whenShuttingDownAndRestartingGrpcServer_ConsumerReceivesDisconnectedAndConnectedEvent() throws Exception {
77-
CountDownLatch sync = new CountDownLatch(2);
78-
ArrayList<Boolean> connectionStateChanges = Lists.newArrayList();
79-
Consumer<FlagdProviderEvent> testConsumer = event -> {
80-
connectionStateChanges.add(!event.isDisconnected());
81-
sync.countDown();
82-
};
83-
84-
GrpcConnector<ServiceGrpc.ServiceStub, ServiceGrpc.ServiceBlockingStub> instance = new GrpcConnector<>(
85-
FlagdOptions.builder().build(),
86-
ServiceGrpc::newStub,
87-
ServiceGrpc::newBlockingStub,
88-
testConsumer,
89-
stub -> stub.eventStream(Evaluation.EventStreamRequest.getDefaultInstance(), mockEventStreamObserver),
90-
testChannel);
91-
92-
instance.initialize();
93-
94-
// when shutting down server
95-
testServer.shutdown();
96-
testServer.awaitTermination(1, TimeUnit.SECONDS);
97-
98-
// when restarting server
99-
setupTestGrpcServer();
100-
101-
// then consumer received DISCONNECTED and CONNECTED event
102-
boolean finished = sync.await(10, TimeUnit.SECONDS);
103-
Assertions.assertTrue(finished);
104-
Assertions.assertEquals(Lists.newArrayList(DISCONNECTED, CONNECTED), connectionStateChanges);
105-
}
106-
10773
@Test
10874
void whenShuttingDownGrpcConnector_ConsumerReceivesDisconnectedEvent() throws Exception {
10975
CountDownLatch sync = new CountDownLatch(1);

Diff for: providers/flagd/test-harness

0 commit comments

Comments
 (0)