Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: cleanup code #277

Merged
merged 6 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions src/OpenFeature/Api.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public sealed class Api : IEventBus
public static Api Instance { get; } = new Api();

// Explicit static constructor to tell C# compiler
// not to mark type as beforefieldinit
// not to mark type as beforeFieldInit
// IE Lazy way of ensuring this is thread safe without using locks
static Api() { }
private Api() { }
Expand All @@ -46,7 +46,7 @@ private Api() { }
public async Task SetProviderAsync(FeatureProvider featureProvider)
{
this._eventExecutor.RegisterDefaultFeatureProvider(featureProvider);
await this._repository.SetProviderAsync(featureProvider, this.GetContext(), AfterInitialization, AfterError).ConfigureAwait(false);
await this._repository.SetProviderAsync(featureProvider, this.GetContext(), this.AfterInitialization, this.AfterError).ConfigureAwait(false);
}

/// <summary>
Expand All @@ -62,7 +62,7 @@ public async Task SetProviderAsync(string clientName, FeatureProvider featurePro
throw new ArgumentNullException(nameof(clientName));
}
this._eventExecutor.RegisterClientFeatureProvider(clientName, featureProvider);
await this._repository.SetProviderAsync(clientName, featureProvider, this.GetContext(), AfterInitialization, AfterError).ConfigureAwait(false);
await this._repository.SetProviderAsync(clientName, featureProvider, this.GetContext(), this.AfterInitialization, this.AfterError).ConfigureAwait(false);
}

/// <summary>
Expand Down Expand Up @@ -101,15 +101,15 @@ public FeatureProvider GetProvider(string clientName)
/// </para>
/// </summary>
/// <returns><see cref="ClientMetadata"/></returns>
public Metadata GetProviderMetadata() => this.GetProvider().GetMetadata();
public Metadata? GetProviderMetadata() => this.GetProvider().GetMetadata();

/// <summary>
/// Gets providers metadata assigned to the given clientName. If the clientName has no provider
/// assigned to it the default provider will be returned
/// </summary>
/// <param name="clientName">Name of client</param>
/// <returns>Metadata assigned to provider</returns>
public Metadata GetProviderMetadata(string clientName) => this.GetProvider(clientName).GetMetadata();
public Metadata? GetProviderMetadata(string clientName) => this.GetProvider(clientName).GetMetadata();

/// <summary>
/// Create a new instance of <see cref="FeatureClient"/> using the current provider
Expand All @@ -121,7 +121,7 @@ public FeatureProvider GetProvider(string clientName)
/// <returns><see cref="FeatureClient"/></returns>
public FeatureClient GetClient(string? name = null, string? version = null, ILogger? logger = null,
EvaluationContext? context = null) =>
new FeatureClient(() => _repository.GetProvider(name), name, version, logger, context);
new FeatureClient(() => this._repository.GetProvider(name), name, version, logger, context);

/// <summary>
/// Appends list of hooks to global hooks list
Expand Down Expand Up @@ -277,7 +277,7 @@ private async Task AfterInitialization(FeatureProvider provider)
{
Type = ProviderEventTypes.ProviderReady,
Message = "Provider initialization complete",
ProviderName = provider.GetMetadata().Name,
ProviderName = provider.GetMetadata()?.Name,
};

await this._eventExecutor.EventChannel.Writer.WriteAsync(new Event { Provider = provider, EventPayload = eventPayload }).ConfigureAwait(false);
Expand All @@ -286,10 +286,9 @@ private async Task AfterInitialization(FeatureProvider provider)
/// <summary>
/// Update the provider state to ERROR and emit an ERROR after failed init.
/// </summary>
private async Task AfterError(FeatureProvider provider, Exception ex)

private async Task AfterError(FeatureProvider provider, Exception? ex)
{
provider.Status = typeof(ProviderFatalException) == ex.GetType() ? ProviderStatus.Fatal : ProviderStatus.Error;
provider.Status = typeof(ProviderFatalException) == ex?.GetType() ? ProviderStatus.Fatal : ProviderStatus.Error;
var eventPayload = new ProviderEventPayload
{
Type = ProviderEventTypes.ProviderError,
Expand Down
16 changes: 8 additions & 8 deletions src/OpenFeature/Constant/Reason.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,42 +9,42 @@ public static class Reason
/// <summary>
/// Use when the flag is matched based on the evaluation context user data
/// </summary>
public static string TargetingMatch = "TARGETING_MATCH";
public const string TargetingMatch = "TARGETING_MATCH";

/// <summary>
/// Use when the flag is matched based on a split rule in the feature flag provider
/// </summary>
public static string Split = "SPLIT";
public const string Split = "SPLIT";

/// <summary>
/// Use when the flag is disabled in the feature flag provider
/// </summary>
public static string Disabled = "DISABLED";
public const string Disabled = "DISABLED";

/// <summary>
/// Default reason when evaluating flag
/// </summary>
public static string Default = "DEFAULT";
public const string Default = "DEFAULT";

/// <summary>
/// The resolved value is static (no dynamic evaluation)
/// </summary>
public static string Static = "STATIC";
public const string Static = "STATIC";

/// <summary>
/// The resolved value was retrieved from cache
/// </summary>
public static string Cached = "CACHED";
public const string Cached = "CACHED";

/// <summary>
/// Use when an unknown reason is encountered when evaluating flag.
/// An example of this is if the feature provider returns a reason that is not defined in the spec
/// </summary>
public static string Unknown = "UNKNOWN";
public const string Unknown = "UNKNOWN";

/// <summary>
/// Use this flag when abnormal execution is encountered.
/// </summary>
public static string Error = "ERROR";
public const string Error = "ERROR";
}
}
2 changes: 1 addition & 1 deletion src/OpenFeature/Error/ProviderFatalException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

namespace OpenFeature.Error
{
/// <summary> the
/// <summary>
/// An exception that signals the provider has entered an irrecoverable error state.
/// </summary>
[ExcludeFromCodeCoverage]
Expand Down
3 changes: 2 additions & 1 deletion src/OpenFeature/EventExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ private void EmitOnRegistration(FeatureProvider? provider, ProviderEventTypes ev
{
handler.Invoke(new ProviderEventPayload
{
ProviderName = provider.GetMetadata().Name,
ProviderName = provider.GetMetadata()?.Name,
Type = eventType,
Message = message
});
Expand Down Expand Up @@ -322,6 +322,7 @@ private void UpdateProviderStatus(FeatureProvider provider, ProviderEventPayload
case ProviderEventTypes.ProviderError:
provider.Status = eventPayload.ErrorType == ErrorType.ProviderFatal ? ProviderStatus.Fatal : ProviderStatus.Error;
break;
case ProviderEventTypes.ProviderConfigurationChanged:
default: break;
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/OpenFeature/FeatureProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ namespace OpenFeature
{
/// <summary>
/// The provider interface describes the abstraction layer for a feature flag provider.
/// A provider acts as the translates layer between the generic feature flag structure to a target feature flag system.
/// A provider acts as it translates layer between the generic feature flag structure to a target feature flag system.
/// </summary>
/// <seealso href="https://github.com/open-feature/spec/blob/v0.5.2/specification/sections/02-providers.md">Provider specification</seealso>
public abstract class FeatureProvider
{
/// <summary>
/// Gets a immutable list of hooks that belong to the provider.
/// By default return a empty list
/// Gets an immutable list of hooks that belong to the provider.
/// By default, return an empty list
///
/// Executed in the order of hooks
/// before: API, Client, Invocation, Provider
Expand All @@ -38,7 +38,7 @@ public abstract class FeatureProvider
/// Metadata describing the provider.
/// </summary>
/// <returns><see cref="Metadata"/></returns>
public abstract Metadata GetMetadata();
public abstract Metadata? GetMetadata();

/// <summary>
/// Resolves a boolean feature flag
Expand Down
4 changes: 2 additions & 2 deletions src/OpenFeature/Model/EvaluationContextBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,9 @@ public EvaluationContextBuilder Merge(EvaluationContext context)
{
string? newTargetingKey = "";

if (!string.IsNullOrWhiteSpace(TargetingKey))
if (!string.IsNullOrWhiteSpace(this.TargetingKey))
{
newTargetingKey = TargetingKey;
newTargetingKey = this.TargetingKey;
}

if (!string.IsNullOrWhiteSpace(context.TargetingKey))
Expand Down
4 changes: 2 additions & 2 deletions src/OpenFeature/Model/FlagEvaluationOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ namespace OpenFeature.Model
public sealed class FlagEvaluationOptions
{
/// <summary>
/// A immutable list of <see cref="Hook"/>
/// An immutable list of <see cref="Hook"/>
/// </summary>
public IImmutableList<Hook> Hooks { get; }

/// <summary>
/// A immutable dictionary of hook hints
/// An immutable dictionary of hook hints
/// </summary>
public IImmutableDictionary<string, object> HookHints { get; }

Expand Down
1 change: 0 additions & 1 deletion src/OpenFeature/Model/ImmutableMetadata.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using System.Collections.Generic;
using System.Collections.Immutable;

#nullable enable
namespace OpenFeature.Model;

/// <summary>
Expand Down
18 changes: 9 additions & 9 deletions src/OpenFeature/Model/Value.cs
Original file line number Diff line number Diff line change
Expand Up @@ -139,49 +139,49 @@ public Value(Object value)
public object? AsObject => this._innerValue;

/// <summary>
/// Returns the underlying int value
/// Value will be null if it isn't a integer
/// Returns the underlying int value.
/// Value will be null if it isn't an integer
/// </summary>
/// <returns>Value as int</returns>
public int? AsInteger => this.IsNumber ? (int?)Convert.ToInt32((double?)this._innerValue) : null;
public int? AsInteger => this.IsNumber ? Convert.ToInt32((double?)this._innerValue) : null;

/// <summary>
/// Returns the underlying bool value
/// Returns the underlying bool value.
/// Value will be null if it isn't a bool
/// </summary>
/// <returns>Value as bool</returns>
public bool? AsBoolean => this.IsBoolean ? (bool?)this._innerValue : null;

/// <summary>
/// Returns the underlying double value
/// Returns the underlying double value.
/// Value will be null if it isn't a double
/// </summary>
/// <returns>Value as int</returns>
public double? AsDouble => this.IsNumber ? (double?)this._innerValue : null;

/// <summary>
/// Returns the underlying string value
/// Returns the underlying string value.
/// Value will be null if it isn't a string
/// </summary>
/// <returns>Value as string</returns>
public string? AsString => this.IsString ? (string?)this._innerValue : null;

/// <summary>
/// Returns the underlying Structure value
/// Returns the underlying Structure value.
/// Value will be null if it isn't a Structure
/// </summary>
/// <returns>Value as Structure</returns>
public Structure? AsStructure => this.IsStructure ? (Structure?)this._innerValue : null;

/// <summary>
/// Returns the underlying List value
/// Returns the underlying List value.
/// Value will be null if it isn't a List
/// </summary>
/// <returns>Value as List</returns>
public IImmutableList<Value>? AsList => this.IsList ? (IImmutableList<Value>?)this._innerValue : null;

/// <summary>
/// Returns the underlying DateTime value
/// Returns the underlying DateTime value.
/// Value will be null if it isn't a DateTime
/// </summary>
/// <returns>Value as DateTime</returns>
Expand Down
15 changes: 6 additions & 9 deletions src/OpenFeature/OpenFeatureClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,8 @@ private async Task<FlagEvaluationDetails<T>> EvaluateFlagAsync<T>(
var resolveValueDelegate = providerInfo.Item1;
var provider = providerInfo.Item2;

// New up a evaluation context if one was not provided.
if (context == null)
{
context = EvaluationContext.Empty;
}
// New up an evaluation context if one was not provided.
context ??= EvaluationContext.Empty;

// merge api, client, and invocation context.
var evaluationContext = Api.Instance.GetContext();
Expand Down Expand Up @@ -253,11 +250,11 @@ private async Task<FlagEvaluationDetails<T>> EvaluateFlagAsync<T>(
var contextFromHooks = await this.TriggerBeforeHooksAsync(allHooks, hookContext, options, cancellationToken).ConfigureAwait(false);

// short circuit evaluation entirely if provider is in a bad state
if (provider.Status == ProviderStatus.NotReady)
if (provider.Status == ProviderStatus.NotReady)
{
throw new ProviderNotReadyException("Provider has not yet completed initialization.");
}
else if (provider.Status == ProviderStatus.Fatal)
}
else if (provider.Status == ProviderStatus.Fatal)
{
throw new ProviderFatalException("Provider is in an irrecoverable error state.");
}
Expand Down Expand Up @@ -349,7 +346,7 @@ private async Task TriggerFinallyHooksAsync<T>(IReadOnlyList<Hook> hooks, HookCo
}
catch (Exception e)
{
this._logger.LogError(e, "Error while executing Finally hook {HookName}", hook.GetType().Name);
this.FinallyHookError(hook.GetType().Name, e);
}
}
}
Expand Down
26 changes: 12 additions & 14 deletions src/OpenFeature/ProviderRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ namespace OpenFeature
/// <summary>
/// This class manages the collection of providers, both default and named, contained by the API.
/// </summary>
internal sealed class ProviderRepository : IAsyncDisposable
internal sealed partial class ProviderRepository : IAsyncDisposable
{
private ILogger _logger;
private ILogger _logger = NullLogger<EventExecutor>.Instance;

private FeatureProvider _defaultProvider = new NoOpFeatureProvider();

Expand All @@ -26,20 +26,15 @@ internal sealed class ProviderRepository : IAsyncDisposable
/// The reader/writer locks is not disposed because the singleton instance should never be disposed.
///
/// Mutations of the _defaultProvider or _featureProviders are done within this lock even though
/// _featureProvider is a concurrent collection. This is for a couple reasons, the first is that
/// _featureProvider is a concurrent collection. This is for a couple of reasons, the first is that
/// a provider should only be shutdown if it is not in use, and it could be in use as either a named or
/// default provider.
///
/// The second is that a concurrent collection doesn't provide any ordering so we could check a provider
/// The second is that a concurrent collection doesn't provide any ordering, so we could check a provider
/// as it was being added or removed such as two concurrent calls to SetProvider replacing multiple instances
/// of that provider under different names..
/// of that provider under different names.
private readonly ReaderWriterLockSlim _providersLock = new ReaderWriterLockSlim();

public ProviderRepository()
{
this._logger = NullLogger<EventExecutor>.Instance;
}

public async ValueTask DisposeAsync()
{
using (this._providersLock)
Expand Down Expand Up @@ -201,15 +196,15 @@ private async Task ShutdownIfUnusedAsync(
return;
}

await SafeShutdownProviderAsync(targetProvider).ConfigureAwait(false);
await this.SafeShutdownProviderAsync(targetProvider).ConfigureAwait(false);
}

/// <remarks>
/// <para>
/// Shut down the provider and capture any exceptions thrown.
/// </para>
/// <para>
/// The provider is set either to a name or default before the old provider it shutdown, so
/// The provider is set either to a name or default before the old provider it shut down, so
/// it would not be meaningful to emit an error.
/// </para>
/// </remarks>
Expand All @@ -226,7 +221,7 @@ private async Task SafeShutdownProviderAsync(FeatureProvider? targetProvider)
}
catch (Exception ex)
{
this._logger.LogError(ex, $"Error shutting down provider: {targetProvider.GetMetadata().Name}");
this.ErrorShuttingDownProvider(targetProvider.GetMetadata()?.Name, ex);
}
}

Expand Down Expand Up @@ -287,8 +282,11 @@ public async Task ShutdownAsync(Action<FeatureProvider, Exception>? afterError =
foreach (var targetProvider in providers)
{
// We don't need to take any actions after shutdown.
await SafeShutdownProviderAsync(targetProvider).ConfigureAwait(false);
await this.SafeShutdownProviderAsync(targetProvider).ConfigureAwait(false);
}
}

[LoggerMessage(EventId = 105, Level = LogLevel.Error, Message = "Error shutting down provider: {TargetProviderName}`")]
partial void ErrorShuttingDownProvider(string? targetProviderName, Exception exception);
}
}
Loading