Skip to content

Commit a14931f

Browse files
committed
fixup: fixing synchronization in a strange way
Signed-off-by: Simon Schrottner <[email protected]>
1 parent 338f367 commit a14931f

File tree

5 files changed

+39
-36
lines changed

5 files changed

+39
-36
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 = v1.1.0
7+
branch = v1.1.1
88
[submodule "providers/flagd/spec"]
99
path = providers/flagd/spec
1010
url = https://github.com/open-feature/spec.git

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

+35-30
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ public class FlagdProvider extends EventProvider {
4141
private volatile EvaluationContext enrichedContext = new ImmutableContext();
4242
private final List<Hook> hooks = new ArrayList<>();
4343
private volatile ProviderEvent previousEvent = null;
44+
private final Object eventLock;
4445

4546
/**
4647
* An executor service responsible for emitting {@link ProviderEvent#PROVIDER_ERROR} after the provider went
@@ -97,6 +98,7 @@ public FlagdProvider(final FlagdOptions options) {
9798
this.errorExecutor = Executors.newSingleThreadScheduledExecutor();
9899
this.gracePeriod = options.getRetryGracePeriod();
99100
this.deadline = options.getDeadline();
101+
this.eventLock = new Object();
100102
}
101103

102104
@Override
@@ -188,39 +190,42 @@ EvaluationContext getEnrichedContext() {
188190
}
189191

190192
@SuppressWarnings("checkstyle:fallthrough")
191-
private synchronized void onProviderEvent(FlagdProviderEvent flagdProviderEvent) {
193+
private void onProviderEvent(FlagdProviderEvent flagdProviderEvent) {
192194

193-
syncMetadata = flagdProviderEvent.getSyncMetadata();
194-
if (flagdProviderEvent.getSyncMetadata() != null) {
195-
enrichedContext = contextEnricher.apply(flagdProviderEvent.getSyncMetadata());
196-
}
195+
synchronized (eventLock) {
196+
log.info("FlagdProviderEvent: {}", flagdProviderEvent);
197+
syncMetadata = flagdProviderEvent.getSyncMetadata();
198+
if (flagdProviderEvent.getSyncMetadata() != null) {
199+
enrichedContext = contextEnricher.apply(flagdProviderEvent.getSyncMetadata());
200+
}
197201

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-
*/
204-
switch (flagdProviderEvent.getEvent()) {
205-
case PROVIDER_CONFIGURATION_CHANGED:
206-
if (previousEvent == ProviderEvent.PROVIDER_READY) {
207-
onConfigurationChanged(flagdProviderEvent);
202+
/*
203+
We only use Error and Ready as previous states.
204+
As error will first be emitted as Stale, and only turns after a while into an emitted Error.
205+
Ready is needed, as the InProcessResolver does not have a dedicated ready event, hence we need to
206+
forward a configuration changed to the ready, if we are not in the ready state.
207+
*/
208+
switch (flagdProviderEvent.getEvent()) {
209+
case PROVIDER_CONFIGURATION_CHANGED:
210+
if (previousEvent == ProviderEvent.PROVIDER_READY) {
211+
onConfigurationChanged(flagdProviderEvent);
212+
break;
213+
}
214+
// intentional fall through, a not-ready change will trigger a ready.
215+
case PROVIDER_READY:
216+
onReady();
217+
previousEvent = ProviderEvent.PROVIDER_READY;
208218
break;
209-
}
210-
// intentional fall through, a not-ready change will trigger a ready.
211-
case PROVIDER_READY:
212-
onReady();
213-
previousEvent = ProviderEvent.PROVIDER_READY;
214-
break;
215219

216-
case PROVIDER_ERROR:
217-
if (previousEvent != ProviderEvent.PROVIDER_ERROR) {
218-
onError();
219-
}
220-
previousEvent = ProviderEvent.PROVIDER_ERROR;
221-
break;
222-
default:
223-
log.info("Unknown event {}", flagdProviderEvent.getEvent());
220+
case PROVIDER_ERROR:
221+
if (previousEvent != ProviderEvent.PROVIDER_ERROR) {
222+
onError();
223+
}
224+
previousEvent = ProviderEvent.PROVIDER_ERROR;
225+
break;
226+
default:
227+
log.info("Unknown event {}", flagdProviderEvent.getEvent());
228+
}
224229
}
225230
}
226231

@@ -258,7 +263,7 @@ private void onError() {
258263
if (!errorExecutor.isShutdown()) {
259264
errorTask = errorExecutor.schedule(
260265
() -> {
261-
if(previousEvent == ProviderEvent.PROVIDER_ERROR) {
266+
if (previousEvent == ProviderEvent.PROVIDER_ERROR) {
262267
log.debug(
263268
"Provider did not reconnect successfully within {}s. Emit ERROR event...",
264269
gracePeriod);

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

+1-3
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,7 @@ void observeEventStream(final BlockingQueue<QueuePayload> writeTo, final AtomicB
109109
try (CancellableContext context = Context.current().withCancellation()) {
110110

111111
try {
112-
metadataResponse = grpcConnector
113-
.getResolver()
114-
.getMetadata(metadataRequest.build());
112+
metadataResponse = grpcConnector.getResolver().getMetadata(metadataRequest.build());
115113
} catch (Exception e) {
116114
// the chances this call fails but the syncRequest does not are slim
117115
// it could be that the server doesn't implement this RPC

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
@ConfigurationParameter(key = GLUE_PROPERTY_NAME, value = "dev.openfeature.contrib.providers.flagd.e2e.steps")
2929
@ConfigurationParameter(key = OBJECT_FACTORY_PROPERTY_NAME, value = "io.cucumber.picocontainer.PicoFactory")
3030
@IncludeTags("in-process")
31-
@ExcludeTags({"unixsocket", "customCert", "targetURI"})
31+
@ExcludeTags({"unixsocket", "targetURI"})
3232
@Testcontainers
3333
public class RunInProcessTest {
3434

Diff for: providers/flagd/test-harness

0 commit comments

Comments
 (0)