feat!: Thread safe hooks, provider, and context

src/OpenFeatureSDK/FeatureProvider.cs

@@ -1,5 +1,4 @@
-using System;
-using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Threading.Tasks;
 using OpenFeatureSDK.Model;
 
@@ -22,8 +21,8 @@ public abstract class FeatureProvider
         /// error (if applicable): Provider, Invocation, Client, API
         /// finally: Provider, Invocation, Client, API
         /// </summary>
-        /// <returns></returns>
-        public virtual IReadOnlyList<Hook> GetProviderHooks() => Array.Empty<Hook>();
+        /// <returns>Immutable list of hooks</returns>
+        public virtual IImmutableList<Hook> GetProviderHooks() => ImmutableList<Hook>.Empty;
 
         /// <summary>
         /// Metadata describing the provider.

src/OpenFeatureSDK/Model/FlagEvaluationOptions.cs

@@ -1,4 +1,4 @@
-using System.Collections.Generic;
+using System.Collections.Immutable;
 
 namespace OpenFeatureSDK.Model
 {
@@ -12,33 +12,33 @@ public class FlagEvaluationOptions
         /// <summary>
         /// A immutable list of <see cref="Hook"/>
         /// </summary>
-        public IReadOnlyList<Hook> Hooks { get; }
+        public IImmutableList<Hook> Hooks { get; }
 
         /// <summary>
         /// A immutable dictionary of hook hints
         /// </summary>
-        public IReadOnlyDictionary<string, object> HookHints { get; }
+        public IImmutableDictionary<string, object> HookHints { get; }
 
         /// <summary>
         /// Initializes a new instance of the <see cref="FlagEvaluationOptions"/> class.
         /// </summary>
-        /// <param name="hooks"></param>
+        /// <param name="hooks">An immutable list of hooks to use during evaluation</param>
         /// <param name="hookHints">Optional - a list of hints that are passed through the hook lifecycle</param>
-        public FlagEvaluationOptions(IReadOnlyList<Hook> hooks, IReadOnlyDictionary<string, object> hookHints = null)
+        public FlagEvaluationOptions(IImmutableList<Hook> hooks, IImmutableDictionary<string, object> hookHints = null)
         {
             this.Hooks = hooks;
-            this.HookHints = hookHints ?? new Dictionary<string, object>();
+            this.HookHints = hookHints ?? ImmutableDictionary<string, object>.Empty;
         }
 
         /// <summary>
         /// Initializes a new instance of the <see cref="FlagEvaluationOptions"/> class.
         /// </summary>
-        /// <param name="hook"></param>
+        /// <param name="hook">A hook to use during the evaluation</param>
         /// <param name="hookHints">Optional - a list of hints that are passed through the hook lifecycle</param>
-        public FlagEvaluationOptions(Hook hook, IReadOnlyDictionary<string, object> hookHints = null)
+        public FlagEvaluationOptions(Hook hook, ImmutableDictionary<string, object> hookHints = null)
         {
-            this.Hooks = new[] { hook };
-            this.HookHints = hookHints ?? new Dictionary<string, object>();
+            this.Hooks = ImmutableList.Create(hook);
+            this.HookHints = hookHints ?? ImmutableDictionary<string, object>.Empty;
         }
     }
 }

src/OpenFeatureSDK/OpenFeature.cs

@@ -1,4 +1,6 @@
+using System.Collections.Concurrent;
 using System.Collections.Generic;
+using System.Linq;
 using Microsoft.Extensions.Logging;
 using OpenFeatureSDK.Model;
 
@@ -11,9 +13,15 @@ namespace OpenFeatureSDK
     /// <seealso href="https://github.com/open-feature/spec/blob/main/specification/flag-evaluation.md#flag-evaluation-api"/>
     public sealed class OpenFeature
     {
-        private EvaluationContext _evaluationContext = EvaluationContext.Empty;
-        private FeatureProvider _featureProvider = new NoOpFeatureProvider();
-        private readonly List<Hook> _hooks = new List<Hook>();
+        /// References are atomic in C#, but it is possible to get an old value due to register caching.
+        /// Making it volatile will result in reads bypassing that cache. It doesn't provide any ordering
+        /// guarantee. Considering this is just a method to replace the value, regardless of the current value,
+        /// this should be sufficient.
+        /// https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/language-specification/variables#96-atomicity-of-variable-references
+        /// https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/volatile
+        private volatile EvaluationContext _evaluationContext = EvaluationContext.Empty;
+        private volatile FeatureProvider _featureProvider = new NoOpFeatureProvider();
+        private readonly ConcurrentStack<Hook> _hooks = new ConcurrentStack<Hook>();
 
         /// <summary>
         /// Singleton instance of OpenFeature
@@ -53,25 +61,30 @@ private OpenFeature() { }
         /// <param name="context">Context given to this client</param>
         /// <returns><see cref="FeatureClient"/></returns>
         public FeatureClient GetClient(string name = null, string version = null, ILogger logger = null, EvaluationContext context = null) =>
-            new FeatureClient(this._featureProvider, name, version, logger, context);
+            new FeatureClient(name, version, logger, context);
 
         /// <summary>
         /// Appends list of hooks to global hooks list
         /// </summary>
         /// <param name="hooks">A list of <see cref="Hook"/></param>
-        public void AddHooks(IEnumerable<Hook> hooks) => this._hooks.AddRange(hooks);
+        public void AddHooks(IEnumerable<Hook> hooks) => this._hooks.PushRange(hooks.ToArray());
 
         /// <summary>
         /// Adds a hook to global hooks list
         /// </summary>
         /// <param name="hook">A list of <see cref="Hook"/></param>
-        public void AddHooks(Hook hook) => this._hooks.Add(hook);
+        public void AddHooks(Hook hook) => this._hooks.Push(hook);
 
         /// <summary>
-        /// Returns the global immutable hooks list
+        /// Enumerates the global hooks.
+        /// <para>
+        /// The items enumerated will reflect the registered hooks
+        /// at the start of enumeration. Hooks added during enumeration
+        /// will not be included.
+        /// </para>
         /// </summary>
-        /// <returns>A immutable list of <see cref="Hook"/></returns>
-        public IReadOnlyList<Hook> GetHooks() => this._hooks.AsReadOnly();
+        /// <returns>Enumeration of <see cref="Hook"/></returns>
+        public IEnumerable<Hook> GetHooks() => this._hooks.Reverse();
 
         /// <summary>
         /// Removes all hooks from global hooks list

src/OpenFeatureSDK/OpenFeatureClient.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
@@ -17,8 +18,7 @@ namespace OpenFeatureSDK
     public sealed class FeatureClient : IFeatureClient
     {
         private readonly ClientMetadata _metadata;
-        private readonly FeatureProvider _featureProvider;
-        private readonly List<Hook> _hooks = new List<Hook>();
+        private readonly ConcurrentStack<Hook> _hooks = new ConcurrentStack<Hook>();
         private readonly ILogger _logger;
         private EvaluationContext _evaluationContext;
 
@@ -36,15 +36,13 @@ public sealed class FeatureClient : IFeatureClient
         /// <summary>
         /// Initializes a new instance of the <see cref="FeatureClient"/> class.
         /// </summary>
-        /// <param name="featureProvider">Feature provider used by client <see cref="FeatureProvider"/></param>
         /// <param name="name">Name of client <see cref="ClientMetadata"/></param>
         /// <param name="version">Version of client <see cref="ClientMetadata"/></param>
         /// <param name="logger">Logger used by client</param>
         /// <param name="context">Context given to this client</param>
         /// <exception cref="ArgumentNullException">Throws if any of the required parameters are null</exception>
-        public FeatureClient(FeatureProvider featureProvider, string name, string version, ILogger logger = null, EvaluationContext context = null)
+        public FeatureClient(string name, string version, ILogger logger = null, EvaluationContext context = null)
         {
-            this._featureProvider = featureProvider ?? throw new ArgumentNullException(nameof(featureProvider));
             this._metadata = new ClientMetadata(name, version);
             this._logger = logger ?? new Logger<OpenFeature>(new NullLoggerFactory());
             this._evaluationContext = context ?? EvaluationContext.Empty;
@@ -60,19 +58,24 @@ public FeatureClient(FeatureProvider featureProvider, string name, string versio
         /// Add hook to client
         /// </summary>
         /// <param name="hook">Hook that implements the <see cref="Hook"/> interface</param>
-        public void AddHooks(Hook hook) => this._hooks.Add(hook);
+        public void AddHooks(Hook hook) => this._hooks.Push(hook);
 
         /// <summary>
         /// Appends hooks to client
         /// </summary>
         /// <param name="hooks">A list of Hooks that implement the <see cref="Hook"/> interface</param>
-        public void AddHooks(IEnumerable<Hook> hooks) => this._hooks.AddRange(hooks);
+        public void AddHooks(IEnumerable<Hook> hooks) => this._hooks.PushRange(hooks.ToArray());
 
         /// <summary>
-        /// Return a immutable list of hooks that are registered against the client
+        /// Enumerates the global hooks.
+        /// <para>
+        /// The items enumerated will reflect the registered hooks
+        /// at the start of enumeration. Hooks added during enumeration
+        /// will not be included.
+        /// </para>
         /// </summary>
-        /// <returns>A list of immutable hooks</returns>
-        public IReadOnlyList<Hook> GetHooks() => this._hooks.ToList().AsReadOnly();
+        /// <returns>Enumeration of <see cref="Hook"/></returns>
+        public IEnumerable<Hook> GetHooks() => this._hooks.Reverse();
 
         /// <summary>
         /// Removes all hooks from the client
@@ -101,7 +104,7 @@ public async Task<bool> GetBooleanValue(string flagKey, bool defaultValue, Evalu
         /// <returns>Resolved flag details <see cref="FlagEvaluationDetails{T}"/></returns>
         public async Task<FlagEvaluationDetails<bool>> GetBooleanDetails(string flagKey, bool defaultValue,
             EvaluationContext context = null, FlagEvaluationOptions config = null) =>
-            await this.EvaluateFlag(this._featureProvider.ResolveBooleanValue, FlagValueType.Boolean, flagKey,
+            await this.EvaluateFlag(OpenFeature.Instance.GetProvider().ResolveBooleanValue, FlagValueType.Boolean, flagKey,
                 defaultValue, context, config);
 
         /// <summary>
@@ -126,7 +129,7 @@ public async Task<string> GetStringValue(string flagKey, string defaultValue, Ev
         /// <returns>Resolved flag details <see cref="FlagEvaluationDetails{T}"/></returns>
         public async Task<FlagEvaluationDetails<string>> GetStringDetails(string flagKey, string defaultValue,
             EvaluationContext context = null, FlagEvaluationOptions config = null) =>
-            await this.EvaluateFlag(this._featureProvider.ResolveStringValue, FlagValueType.String, flagKey,
+            await this.EvaluateFlag(OpenFeature.Instance.GetProvider().ResolveStringValue, FlagValueType.String, flagKey,
                 defaultValue, context, config);
 
         /// <summary>
@@ -151,7 +154,7 @@ public async Task<int> GetIntegerValue(string flagKey, int defaultValue, Evaluat
         /// <returns>Resolved flag details <see cref="FlagEvaluationDetails{T}"/></returns>
         public async Task<FlagEvaluationDetails<int>> GetIntegerDetails(string flagKey, int defaultValue,
             EvaluationContext context = null, FlagEvaluationOptions config = null) =>
-            await this.EvaluateFlag(this._featureProvider.ResolveIntegerValue, FlagValueType.Number, flagKey,
+            await this.EvaluateFlag(OpenFeature.Instance.GetProvider().ResolveIntegerValue, FlagValueType.Number, flagKey,
                 defaultValue, context, config);
 
         /// <summary>
@@ -177,7 +180,7 @@ public async Task<double> GetDoubleValue(string flagKey, double defaultValue,
         /// <returns>Resolved flag details <see cref="FlagEvaluationDetails{T}"/></returns>
         public async Task<FlagEvaluationDetails<double>> GetDoubleDetails(string flagKey, double defaultValue,
             EvaluationContext context = null, FlagEvaluationOptions config = null) =>
-            await this.EvaluateFlag(this._featureProvider.ResolveDoubleValue, FlagValueType.Number, flagKey,
+            await this.EvaluateFlag(OpenFeature.Instance.GetProvider().ResolveDoubleValue, FlagValueType.Number, flagKey,
                 defaultValue, context, config);
 
         /// <summary>
@@ -202,7 +205,7 @@ public async Task<Value> GetObjectValue(string flagKey, Value defaultValue, Eval
         /// <returns>Resolved flag details <see cref="FlagEvaluationDetails{T}"/></returns>
         public async Task<FlagEvaluationDetails<Value>> GetObjectDetails(string flagKey, Value defaultValue,
             EvaluationContext context = null, FlagEvaluationOptions config = null) =>
-            await this.EvaluateFlag(this._featureProvider.ResolveStructureValue, FlagValueType.Object, flagKey,
+            await this.EvaluateFlag(OpenFeature.Instance.GetProvider().ResolveStructureValue, FlagValueType.Object, flagKey,
                 defaultValue, context, config);
 
         private async Task<FlagEvaluationDetails<T>> EvaluateFlag<T>(
@@ -225,9 +228,9 @@ private async Task<FlagEvaluationDetails<T>> EvaluateFlag<T>(
 
             var allHooks = new List<Hook>()
                 .Concat(OpenFeature.Instance.GetHooks())
-                .Concat(this._hooks)
+                .Concat(this.GetHooks())
                 .Concat(options?.Hooks ?? Enumerable.Empty<Hook>())
-                .Concat(this._featureProvider.GetProviderHooks())
+                .Concat(OpenFeature.Instance.GetProvider().GetProviderHooks())
                 .ToList()
                 .AsReadOnly();