Skip to content

Commit 46274a2

Browse files
askptaskpt
authored
refactor: Improve EventExecutor (#393)
<!-- Please use this template for your pull request. --> <!-- Please use the sections that you need and delete other sections --> ## This PR <!-- add the description of the PR here --> This pull request to `src/OpenFeature/EventExecutor.cs` includes changes to improve code readability and performance. The most important changes include replacing `List` and `Dictionary` initializations with shorthand syntax, switching from `Thread` to `Task` for asynchronous operations, and refactoring methods for better clarity and maintainability. Improvements to code readability and performance: * [`src/OpenFeature/EventExecutor.cs`](diffhunk://#diff-f563baadcf750bb66efaea76d7ec4783320e6efdc45c468effb531c948a2292cL13-R28): Replaced `List` and `Dictionary` initializations with shorthand syntax `[]`. [[1]](diffhunk://#diff-f563baadcf750bb66efaea76d7ec4783320e6efdc45c468effb531c948a2292cL13-R28) [[2]](diffhunk://#diff-f563baadcf750bb66efaea76d7ec4783320e6efdc45c468effb531c948a2292cL45-R41) [[3]](diffhunk://#diff-f563baadcf750bb66efaea76d7ec4783320e6efdc45c468effb531c948a2292cL73-R75) * [`src/OpenFeature/EventExecutor.cs`](diffhunk://#diff-f563baadcf750bb66efaea76d7ec4783320e6efdc45c468effb531c948a2292cL149-R144): Changed from using `Thread` to `Task` for asynchronous operations to improve performance and simplify the code. [[1]](diffhunk://#diff-f563baadcf750bb66efaea76d7ec4783320e6efdc45c468effb531c948a2292cL149-R144) [[2]](diffhunk://#diff-f563baadcf750bb66efaea76d7ec4783320e6efdc45c468effb531c948a2292cL219-R213) [[3]](diffhunk://#diff-f563baadcf750bb66efaea76d7ec4783320e6efdc45c468effb531c948a2292cL237-R257) Refactoring for better clarity and maintainability: * [`src/OpenFeature/EventExecutor.cs`](diffhunk://#diff-f563baadcf750bb66efaea76d7ec4783320e6efdc45c468effb531c948a2292cL189-L204): Refactored `EmitOnRegistration` method to use a switch expression for setting the message based on provider status and event type, improving readability. * [`src/OpenFeature/EventExecutor.cs`](diffhunk://#diff-f563baadcf750bb66efaea76d7ec4783320e6efdc45c468effb531c948a2292cR266-R275): Split `ProcessEventAsync` method into smaller methods (`ProcessApiHandlers`, `ProcessClientHandlers`, `ProcessDefaultProviderHandlers`) for better organization and maintainability. [[1]](diffhunk://#diff-f563baadcf750bb66efaea76d7ec4783320e6efdc45c468effb531c948a2292cR266-R275) [[2]](diffhunk://#diff-f563baadcf750bb66efaea76d7ec4783320e6efdc45c468effb531c948a2292cL281-R298) [[3]](diffhunk://#diff-f563baadcf750bb66efaea76d7ec4783320e6efdc45c468effb531c948a2292cL305-R311) * [`src/OpenFeature/EventExecutor.cs`](diffhunk://#diff-f563baadcf750bb66efaea76d7ec4783320e6efdc45c468effb531c948a2292cL219-R213): Converted `ProcessFeatureProviderEventsAsync` and `ProcessEventAsync` from `void` to `Task` to follow best practices for asynchronous methods. [[1]](diffhunk://#diff-f563baadcf750bb66efaea76d7ec4783320e6efdc45c468effb531c948a2292cL219-R213) [[2]](diffhunk://#diff-f563baadcf750bb66efaea76d7ec4783320e6efdc45c468effb531c948a2292cL237-R257) ### Related Issues <!-- add here the GitHub issue that this PR resolves if applicable --> Reference #358 --------- Signed-off-by: André Silva <[email protected]>
1 parent 9b6feab commit 46274a2

File tree

3 files changed

+101
-106
lines changed

3 files changed

+101
-106
lines changed

‎src/OpenFeature/EventExecutor.cs

+100-101
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
using System;
22
using System.Collections.Generic;
3-
using System.Threading;
43
using System.Threading.Channels;
54
using System.Threading.Tasks;
65
using Microsoft.Extensions.Logging;
@@ -10,26 +9,23 @@
109

1110
namespace OpenFeature
1211
{
13-
internal delegate Task ShutdownDelegate(CancellationToken cancellationToken);
14-
1512
internal sealed partial class EventExecutor : IAsyncDisposable
1613
{
17-
private readonly object _lockObj = new object();
14+
private readonly object _lockObj = new();
1815
public readonly Channel<object> EventChannel = Channel.CreateBounded<object>(1);
1916
private FeatureProvider? _defaultProvider;
20-
private readonly Dictionary<string, FeatureProvider> _namedProviderReferences = new Dictionary<string, FeatureProvider>();
21-
private readonly List<FeatureProvider> _activeSubscriptions = new List<FeatureProvider>();
17+
private readonly Dictionary<string, FeatureProvider> _namedProviderReferences = [];
18+
private readonly List<FeatureProvider> _activeSubscriptions = [];
2219

23-
private readonly Dictionary<ProviderEventTypes, List<EventHandlerDelegate>> _apiHandlers = new Dictionary<ProviderEventTypes, List<EventHandlerDelegate>>();
24-
private readonly Dictionary<string, Dictionary<ProviderEventTypes, List<EventHandlerDelegate>>> _clientHandlers = new Dictionary<string, Dictionary<ProviderEventTypes, List<EventHandlerDelegate>>>();
20+
private readonly Dictionary<ProviderEventTypes, List<EventHandlerDelegate>> _apiHandlers = [];
21+
private readonly Dictionary<string, Dictionary<ProviderEventTypes, List<EventHandlerDelegate>>> _clientHandlers = [];
2522

2623
private ILogger _logger;
2724

2825
public EventExecutor()
2926
{
3027
this._logger = NullLogger<EventExecutor>.Instance;
31-
var eventProcessing = new Thread(this.ProcessEventAsync);
32-
eventProcessing.Start();
28+
Task.Run(this.ProcessEventAsync);
3329
}
3430

3531
public ValueTask DisposeAsync() => new(this.ShutdownAsync());
@@ -42,7 +38,7 @@ internal void AddApiLevelHandler(ProviderEventTypes eventType, EventHandlerDeleg
4238
{
4339
if (!this._apiHandlers.TryGetValue(eventType, out var eventHandlers))
4440
{
45-
eventHandlers = new List<EventHandlerDelegate>();
41+
eventHandlers = [];
4642
this._apiHandlers[eventType] = eventHandlers;
4743
}
4844

@@ -70,13 +66,13 @@ internal void AddClientHandler(string client, ProviderEventTypes eventType, Even
7066
// check if there is already a list of handlers for the given client and event type
7167
if (!this._clientHandlers.TryGetValue(client, out var registry))
7268
{
73-
registry = new Dictionary<ProviderEventTypes, List<EventHandlerDelegate>>();
69+
registry = [];
7470
this._clientHandlers[client] = registry;
7571
}
7672

7773
if (!this._clientHandlers[client].TryGetValue(eventType, out var eventHandlers))
7874
{
79-
eventHandlers = new List<EventHandlerDelegate>();
75+
eventHandlers = [];
8076
this._clientHandlers[client][eventType] = eventHandlers;
8177
}
8278

@@ -127,16 +123,15 @@ internal void RegisterClientFeatureProvider(string client, FeatureProvider? prov
127123
}
128124
lock (this._lockObj)
129125
{
130-
var newProvider = provider;
131126
FeatureProvider? oldProvider = null;
132127
if (this._namedProviderReferences.TryGetValue(client, out var foundOldProvider))
133128
{
134129
oldProvider = foundOldProvider;
135130
}
136131

137-
this._namedProviderReferences[client] = newProvider;
132+
this._namedProviderReferences[client] = provider;
138133

139-
this.StartListeningAndShutdownOld(newProvider, oldProvider);
134+
this.StartListeningAndShutdownOld(provider, oldProvider);
140135
}
141136
}
142137

@@ -146,8 +141,7 @@ private void StartListeningAndShutdownOld(FeatureProvider newProvider, FeaturePr
146141
if (!this.IsProviderActive(newProvider))
147142
{
148143
this._activeSubscriptions.Add(newProvider);
149-
var featureProviderEventProcessing = new Thread(this.ProcessFeatureProviderEventsAsync);
150-
featureProviderEventProcessing.Start(newProvider);
144+
Task.Run(() => this.ProcessFeatureProviderEventsAsync(newProvider));
151145
}
152146

153147
if (oldProvider != null && !this.IsProviderBound(oldProvider))
@@ -186,42 +180,37 @@ private void EmitOnRegistration(FeatureProvider? provider, ProviderEventTypes ev
186180
}
187181
var status = provider.Status;
188182

189-
var message = "";
190-
if (status == ProviderStatus.Ready && eventType == ProviderEventTypes.ProviderReady)
191-
{
192-
message = "Provider is ready";
193-
}
194-
else if (status == ProviderStatus.Error && eventType == ProviderEventTypes.ProviderError)
183+
var message = status switch
195184
{
196-
message = "Provider is in error state";
197-
}
198-
else if (status == ProviderStatus.Stale && eventType == ProviderEventTypes.ProviderStale)
185+
ProviderStatus.Ready when eventType == ProviderEventTypes.ProviderReady => "Provider is ready",
186+
ProviderStatus.Error when eventType == ProviderEventTypes.ProviderError => "Provider is in error state",
187+
ProviderStatus.Stale when eventType == ProviderEventTypes.ProviderStale => "Provider is in stale state",
188+
_ => string.Empty
189+
};
190+
191+
if (string.IsNullOrWhiteSpace(message))
199192
{
200-
message = "Provider is in stale state";
193+
return;
201194
}
202195

203-
if (message != "")
196+
try
204197
{
205-
try
198+
handler.Invoke(new ProviderEventPayload
206199
{
207-
handler.Invoke(new ProviderEventPayload
208-
{
209-
ProviderName = provider.GetMetadata()?.Name,
210-
Type = eventType,
211-
Message = message
212-
});
213-
}
214-
catch (Exception exc)
215-
{
216-
this.ErrorRunningHandler(exc);
217-
}
200+
ProviderName = provider.GetMetadata()?.Name,
201+
Type = eventType,
202+
Message = message
203+
});
204+
}
205+
catch (Exception exc)
206+
{
207+
this.ErrorRunningHandler(exc);
218208
}
219209
}
220210

221-
private async void ProcessFeatureProviderEventsAsync(object? providerRef)
211+
private async Task ProcessFeatureProviderEventsAsync(FeatureProvider provider)
222212
{
223-
var typedProviderRef = (FeatureProvider?)providerRef;
224-
if (typedProviderRef?.GetEventChannel() is not { Reader: { } reader })
213+
if (provider.GetEventChannel() is not { Reader: { } reader })
225214
{
226215
return;
227216
}
@@ -234,82 +223,92 @@ private async void ProcessFeatureProviderEventsAsync(object? providerRef)
234223
switch (item)
235224
{
236225
case ProviderEventPayload eventPayload:
237-
this.UpdateProviderStatus(typedProviderRef, eventPayload);
238-
await this.EventChannel.Writer.WriteAsync(new Event { Provider = typedProviderRef, EventPayload = eventPayload }).ConfigureAwait(false);
226+
UpdateProviderStatus(provider, eventPayload);
227+
await this.EventChannel.Writer.WriteAsync(new Event { Provider = provider, EventPayload = eventPayload }).ConfigureAwait(false);
239228
break;
240229
}
241230
}
242231
}
243232

244233
// Method to process events
245-
private async void ProcessEventAsync()
234+
private async Task ProcessEventAsync()
246235
{
247236
while (await this.EventChannel.Reader.WaitToReadAsync().ConfigureAwait(false))
248237
{
249238
if (!this.EventChannel.Reader.TryRead(out var item))
239+
{
250240
continue;
241+
}
251242

252-
switch (item)
243+
if (item is not Event e)
253244
{
254-
case Event e:
255-
lock (this._lockObj)
256-
{
257-
if (e.EventPayload?.Type != null && this._apiHandlers.TryGetValue(e.EventPayload.Type, out var eventHandlers))
258-
{
259-
foreach (var eventHandler in eventHandlers)
260-
{
261-
this.InvokeEventHandler(eventHandler, e);
262-
}
263-
}
264-
265-
// look for client handlers and call invoke method there
266-
foreach (var keyAndValue in this._namedProviderReferences)
267-
{
268-
if (keyAndValue.Value == e.Provider && keyAndValue.Key != null)
269-
{
270-
if (this._clientHandlers.TryGetValue(keyAndValue.Key, out var clientRegistry))
271-
{
272-
if (e.EventPayload?.Type != null && clientRegistry.TryGetValue(e.EventPayload.Type, out var clientEventHandlers))
273-
{
274-
foreach (var eventHandler in clientEventHandlers)
275-
{
276-
this.InvokeEventHandler(eventHandler, e);
277-
}
278-
}
279-
}
280-
}
281-
}
282-
283-
if (e.Provider != this._defaultProvider)
284-
{
285-
break;
286-
}
287-
// handling the default provider - invoke event handlers for clients which are not bound
288-
// to a particular feature provider
289-
foreach (var keyAndValues in this._clientHandlers)
290-
{
291-
if (this._namedProviderReferences.TryGetValue(keyAndValues.Key, out _))
292-
{
293-
// if there is an association for the client to a specific feature provider, then continue
294-
continue;
295-
}
296-
if (e.EventPayload?.Type != null && keyAndValues.Value.TryGetValue(e.EventPayload.Type, out var clientEventHandlers))
297-
{
298-
foreach (var eventHandler in clientEventHandlers)
299-
{
300-
this.InvokeEventHandler(eventHandler, e);
301-
}
302-
}
303-
}
304-
}
305-
break;
245+
continue;
246+
}
247+
248+
lock (this._lockObj)
249+
{
250+
this.ProcessApiHandlers(e);
251+
this.ProcessClientHandlers(e);
252+
this.ProcessDefaultProviderHandlers(e);
253+
}
254+
}
255+
}
256+
257+
private void ProcessApiHandlers(Event e)
258+
{
259+
if (e.EventPayload?.Type != null && this._apiHandlers.TryGetValue(e.EventPayload.Type, out var eventHandlers))
260+
{
261+
foreach (var eventHandler in eventHandlers)
262+
{
263+
this.InvokeEventHandler(eventHandler, e);
264+
}
265+
}
266+
}
267+
268+
private void ProcessClientHandlers(Event e)
269+
{
270+
foreach (var keyAndValue in this._namedProviderReferences)
271+
{
272+
if (keyAndValue.Value == e.Provider
273+
&& this._clientHandlers.TryGetValue(keyAndValue.Key, out var clientRegistry)
274+
&& e.EventPayload?.Type != null
275+
&& clientRegistry.TryGetValue(e.EventPayload.Type, out var clientEventHandlers))
276+
{
277+
foreach (var eventHandler in clientEventHandlers)
278+
{
279+
this.InvokeEventHandler(eventHandler, e);
280+
}
281+
}
282+
}
283+
}
284+
285+
private void ProcessDefaultProviderHandlers(Event e)
286+
{
287+
if (e.Provider != this._defaultProvider)
288+
{
289+
return;
290+
}
291+
292+
foreach (var keyAndValues in this._clientHandlers)
293+
{
294+
if (this._namedProviderReferences.ContainsKey(keyAndValues.Key))
295+
{
296+
continue;
306297
}
307298

299+
if (e.EventPayload?.Type != null && keyAndValues.Value.TryGetValue(e.EventPayload.Type, out var clientEventHandlers))
300+
{
301+
foreach (var eventHandler in clientEventHandlers)
302+
{
303+
this.InvokeEventHandler(eventHandler, e);
304+
}
305+
}
308306
}
309307
}
310308

309+
311310
// map events to provider status as per spec: https://openfeature.dev/specification/sections/events/#requirement-535
312-
private void UpdateProviderStatus(FeatureProvider provider, ProviderEventPayload eventPayload)
311+
private static void UpdateProviderStatus(FeatureProvider provider, ProviderEventPayload eventPayload)
313312
{
314313
switch (eventPayload.Type)
315314
{

‎src/OpenFeature/Providers/Memory/InMemoryProvider.cs

+1
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ public async Task UpdateFlagsAsync(IDictionary<string, Flag>? flags = null)
6565
FlagsChanged = changed, // emit all
6666
Message = "flags changed",
6767
};
68+
6869
await this.EventChannel.Writer.WriteAsync(@event).ConfigureAwait(false);
6970
}
7071

‎test/OpenFeature.Tests/TestImplementations.cs

-5
Original file line numberDiff line numberDiff line change
@@ -152,10 +152,5 @@ internal ValueTask SendEventAsync(ProviderEventTypes eventType, CancellationToke
152152
{
153153
return this.EventChannel.Writer.WriteAsync(new ProviderEventPayload { Type = eventType, ProviderName = this.GetMetadata().Name, }, cancellationToken);
154154
}
155-
156-
internal ValueTask SendEventAsync(ProviderEventPayload payload, CancellationToken cancellationToken = default)
157-
{
158-
return this.EventChannel.Writer.WriteAsync(payload, cancellationToken);
159-
}
160155
}
161156
}

0 commit comments

Comments
 (0)