Skip to content

Commit

Permalink
chore: revert breaking setProvider (#190)
Browse files Browse the repository at this point in the history 
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: André Silva <2493377+askpt@users.noreply.github.com>
Co-authored-by: Austin Drenski <austin@austindrenski.io>
  • Loading branch information
@toddbaert @askpt @austindrenski
3 people authored Jan 23, 2024
1 parent 26cd5cd commit 2919c2f
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 73 deletions.
30 changes: 27 additions & 3 deletions src/OpenFeature/Api.cs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
Expand Down Expand Up @@ -36,25 +37,48 @@ static Api() { }
private Api() { }

/// <summary>
/// Sets the feature provider. In order to wait for the provider to be set, and initialization to complete,
/// Sets the default feature provider to given clientName without awaiting its initialization.
/// </summary>
/// <remarks>The provider cannot be set to null. Attempting to set the provider to null has no effect.</remarks>
/// <param name="featureProvider">Implementation of <see cref="FeatureProvider"/></param>
[Obsolete("Will be removed in later versions; use SetProviderAsync, which can be awaited")]
public void SetProvider(FeatureProvider featureProvider)
{
this._eventExecutor.RegisterDefaultFeatureProvider(featureProvider);
_ = this._repository.SetProvider(featureProvider, this.GetContext());
}

/// <summary>
/// Sets the default feature provider. In order to wait for the provider to be set, and initialization to complete,
/// await the returned task.
/// </summary>
/// <remarks>The provider cannot be set to null. Attempting to set the provider to null has no effect.</remarks>
/// <param name="featureProvider">Implementation of <see cref="FeatureProvider"/></param>
public async Task SetProvider(FeatureProvider featureProvider)
public async Task SetProviderAsync(FeatureProvider featureProvider)
{
this._eventExecutor.RegisterDefaultFeatureProvider(featureProvider);
await this._repository.SetProvider(featureProvider, this.GetContext()).ConfigureAwait(false);
}

/// <summary>
/// Sets the feature provider to given clientName without awaiting its initialization.
/// </summary>
/// <param name="clientName">Name of client</param>
/// <param name="featureProvider">Implementation of <see cref="FeatureProvider"/></param>
[Obsolete("Will be removed in later versions; use SetProviderAsync, which can be awaited")]
public void SetProvider(string clientName, FeatureProvider featureProvider)
{
this._eventExecutor.RegisterClientFeatureProvider(clientName, featureProvider);
_ = this._repository.SetProvider(clientName, featureProvider, this.GetContext());
}

/// <summary>
/// Sets the feature provider to given clientName. In order to wait for the provider to be set, and
/// initialization to complete, await the returned task.
/// </summary>
/// <param name="clientName">Name of client</param>
/// <param name="featureProvider">Implementation of <see cref="FeatureProvider"/></param>
public async Task SetProvider(string clientName, FeatureProvider featureProvider)
public async Task SetProviderAsync(string clientName, FeatureProvider featureProvider)
{
this._eventExecutor.RegisterClientFeatureProvider(clientName, featureProvider);
await this._repository.SetProvider(clientName, featureProvider, this.GetContext()).ConfigureAwait(false);
Expand Down
2 changes: 1 addition & 1 deletion test/OpenFeature.E2ETests/Steps/EvaluationStepDefinitions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public EvaluationStepDefinitions(ScenarioContext scenarioContext)
{
_scenarioContext = scenarioContext;
var flagdProvider = new FlagdProvider();
Api.Instance.SetProvider(flagdProvider).Wait();
Api.Instance.SetProviderAsync(flagdProvider).Wait();
client = Api.Instance.GetClient();
}

Expand Down
2 changes: 1 addition & 1 deletion test/OpenFeature.Tests/ClearOpenFeatureInstanceFixture.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ public ClearOpenFeatureInstanceFixture()
{
Api.Instance.SetContext(null);
Api.Instance.ClearHooks();
Api.Instance.SetProvider(new NoOpFeatureProvider()).Wait();
Api.Instance.SetProviderAsync(new NoOpFeatureProvider()).Wait();
}
}
}
22 changes: 11 additions & 11 deletions test/OpenFeature.Tests/OpenFeatureClientTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public async Task OpenFeatureClient_Should_Allow_Flag_Evaluation()
var defaultStructureValue = fixture.Create<Value>();
var emptyFlagOptions = new FlagEvaluationOptions(ImmutableList<Hook>.Empty, ImmutableDictionary<string, object>.Empty);

await Api.Instance.SetProvider(new NoOpFeatureProvider());
await Api.Instance.SetProviderAsync(new NoOpFeatureProvider());
var client = Api.Instance.GetClient(clientName, clientVersion);

(await client.GetBooleanValue(flagName, defaultBoolValue)).Should().Be(defaultBoolValue);
Expand Down Expand Up @@ -121,7 +121,7 @@ public async Task OpenFeatureClient_Should_Allow_Details_Flag_Evaluation()
var defaultStructureValue = fixture.Create<Value>();
var emptyFlagOptions = new FlagEvaluationOptions(ImmutableList<Hook>.Empty, ImmutableDictionary<string, object>.Empty);

await Api.Instance.SetProvider(new NoOpFeatureProvider());
await Api.Instance.SetProviderAsync(new NoOpFeatureProvider());
var client = Api.Instance.GetClient(clientName, clientVersion);

var boolFlagEvaluationDetails = new FlagEvaluationDetails<bool>(flagName, defaultBoolValue, ErrorType.None, NoOpProvider.ReasonNoOp, NoOpProvider.Variant);
Expand Down Expand Up @@ -172,7 +172,7 @@ public async Task OpenFeatureClient_Should_Return_DefaultValue_When_Type_Mismatc
mockedFeatureProvider.GetMetadata().Returns(new Metadata(fixture.Create<string>()));
mockedFeatureProvider.GetProviderHooks().Returns(ImmutableList<Hook>.Empty);

await Api.Instance.SetProvider(mockedFeatureProvider);
await Api.Instance.SetProviderAsync(mockedFeatureProvider);
var client = Api.Instance.GetClient(clientName, clientVersion, mockedLogger);

var evaluationDetails = await client.GetObjectDetails(flagName, defaultValue);
Expand Down Expand Up @@ -202,7 +202,7 @@ public async Task Should_Resolve_BooleanValue()
featureProviderMock.GetMetadata().Returns(new Metadata(fixture.Create<string>()));
featureProviderMock.GetProviderHooks().Returns(ImmutableList<Hook>.Empty);

await Api.Instance.SetProvider(featureProviderMock);
await Api.Instance.SetProviderAsync(featureProviderMock);
var client = Api.Instance.GetClient(clientName, clientVersion);

(await client.GetBooleanValue(flagName, defaultValue)).Should().Be(defaultValue);
Expand All @@ -224,7 +224,7 @@ public async Task Should_Resolve_StringValue()
featureProviderMock.GetMetadata().Returns(new Metadata(fixture.Create<string>()));
featureProviderMock.GetProviderHooks().Returns(ImmutableList<Hook>.Empty);

await Api.Instance.SetProvider(featureProviderMock);
await Api.Instance.SetProviderAsync(featureProviderMock);
var client = Api.Instance.GetClient(clientName, clientVersion);

(await client.GetStringValue(flagName, defaultValue)).Should().Be(defaultValue);
Expand All @@ -246,7 +246,7 @@ public async Task Should_Resolve_IntegerValue()
featureProviderMock.GetMetadata().Returns(new Metadata(fixture.Create<string>()));
featureProviderMock.GetProviderHooks().Returns(ImmutableList<Hook>.Empty);

await Api.Instance.SetProvider(featureProviderMock);
await Api.Instance.SetProviderAsync(featureProviderMock);
var client = Api.Instance.GetClient(clientName, clientVersion);

(await client.GetIntegerValue(flagName, defaultValue)).Should().Be(defaultValue);
Expand All @@ -268,7 +268,7 @@ public async Task Should_Resolve_DoubleValue()
featureProviderMock.GetMetadata().Returns(new Metadata(fixture.Create<string>()));
featureProviderMock.GetProviderHooks().Returns(ImmutableList<Hook>.Empty);

await Api.Instance.SetProvider(featureProviderMock);
await Api.Instance.SetProviderAsync(featureProviderMock);
var client = Api.Instance.GetClient(clientName, clientVersion);

(await client.GetDoubleValue(flagName, defaultValue)).Should().Be(defaultValue);
Expand All @@ -290,7 +290,7 @@ public async Task Should_Resolve_StructureValue()
featureProviderMock.GetMetadata().Returns(new Metadata(fixture.Create<string>()));
featureProviderMock.GetProviderHooks().Returns(ImmutableList<Hook>.Empty);

await Api.Instance.SetProvider(featureProviderMock);
await Api.Instance.SetProviderAsync(featureProviderMock);
var client = Api.Instance.GetClient(clientName, clientVersion);

(await client.GetObjectValue(flagName, defaultValue)).Should().Be(defaultValue);
Expand All @@ -313,7 +313,7 @@ public async Task When_Error_Is_Returned_From_Provider_Should_Return_Error()
featureProviderMock.GetMetadata().Returns(new Metadata(fixture.Create<string>()));
featureProviderMock.GetProviderHooks().Returns(ImmutableList<Hook>.Empty);

await Api.Instance.SetProvider(featureProviderMock);
await Api.Instance.SetProviderAsync(featureProviderMock);
var client = Api.Instance.GetClient(clientName, clientVersion);
var response = await client.GetObjectDetails(flagName, defaultValue);

Expand All @@ -338,7 +338,7 @@ public async Task When_Exception_Occurs_During_Evaluation_Should_Return_Error()
featureProviderMock.GetMetadata().Returns(new Metadata(fixture.Create<string>()));
featureProviderMock.GetProviderHooks().Returns(ImmutableList<Hook>.Empty);

await Api.Instance.SetProvider(featureProviderMock);
await Api.Instance.SetProviderAsync(featureProviderMock);
var client = Api.Instance.GetClient(clientName, clientVersion);
var response = await client.GetObjectDetails(flagName, defaultValue);

Expand All @@ -351,7 +351,7 @@ public async Task When_Exception_Occurs_During_Evaluation_Should_Return_Error()
[Fact]
public async Task Should_Use_No_Op_When_Provider_Is_Null()
{
await Api.Instance.SetProvider(null);
await Api.Instance.SetProviderAsync(null);
var client = new FeatureClient("test", "test");
(await client.GetIntegerValue("some-key", 12)).Should().Be(12);
}
Expand Down
60 changes: 43 additions & 17 deletions test/OpenFeature.Tests/OpenFeatureEventTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public async Task API_Level_Event_Handlers_Should_Be_Registered()
Api.Instance.AddHandler(ProviderEventTypes.ProviderStale, eventHandler);

var testProvider = new TestProvider();
await Api.Instance.SetProvider(testProvider);
await Api.Instance.SetProviderAsync(testProvider);

testProvider.SendEvent(ProviderEventTypes.ProviderConfigurationChanged);
testProvider.SendEvent(ProviderEventTypes.ProviderError);
Expand Down Expand Up @@ -117,7 +117,33 @@ public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Ready_State_
var eventHandler = Substitute.For<EventHandlerDelegate>();

var testProvider = new TestProvider();
await Api.Instance.SetProvider(testProvider);
await Api.Instance.SetProviderAsync(testProvider);

Api.Instance.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);

await Utils.AssertUntilAsync(_ => eventHandler
.Received()
.Invoke(
Arg.Is<ProviderEventPayload>(
payload => payload.ProviderName == testProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderReady
)));
}

[Fact]
[Specification("5.1.2", "When a `provider` signals the occurrence of a particular `event`, the associated `client` and `API` event handlers MUST run.")]
[Specification("5.2.2", "The `API` MUST provide a function for associating `handler functions` with a particular `provider event type`.")]
[Specification("5.2.3", "The `event details` MUST contain the `provider name` associated with the event.")]
[Specification("5.2.4", "The `handler function` MUST accept a `event details` parameter.")]
[Specification("5.3.1", "If the provider's `initialize` function terminates normally, `PROVIDER_READY` handlers MUST run.")]
[Specification("5.3.3", "Handlers attached after the provider is already in the associated state, MUST run immediately.")]
public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Ready_State_After_Registering_Provider_Ready_Sync()
{
var eventHandler = Substitute.For<EventHandlerDelegate>();

var testProvider = new TestProvider();
#pragma warning disable CS0618 // Type or member is obsolete
Api.Instance.SetProvider(testProvider);
#pragma warning restore CS0618 // Type or member is obsolete

Api.Instance.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);

Expand All @@ -141,7 +167,7 @@ public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Error_State_
var eventHandler = Substitute.For<EventHandlerDelegate>();

var testProvider = new TestProvider();
await Api.Instance.SetProvider(testProvider);
await Api.Instance.SetProviderAsync(testProvider);

testProvider.SetStatus(ProviderStatus.Error);

Expand All @@ -166,7 +192,7 @@ public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Stale_State_
var eventHandler = Substitute.For<EventHandlerDelegate>();

var testProvider = new TestProvider();
await Api.Instance.SetProvider(testProvider);
await Api.Instance.SetProviderAsync(testProvider);

testProvider.SetStatus(ProviderStatus.Stale);

Expand Down Expand Up @@ -194,12 +220,12 @@ public async Task API_Level_Event_Handlers_Should_Be_Exchangeable()
Api.Instance.AddHandler(ProviderEventTypes.ProviderConfigurationChanged, eventHandler);

var testProvider = new TestProvider();
await Api.Instance.SetProvider(testProvider);
await Api.Instance.SetProviderAsync(testProvider);

testProvider.SendEvent(ProviderEventTypes.ProviderConfigurationChanged);

var newTestProvider = new TestProvider();
await Api.Instance.SetProvider(newTestProvider);
await Api.Instance.SetProviderAsync(newTestProvider);

newTestProvider.SendEvent(ProviderEventTypes.ProviderConfigurationChanged);

Expand All @@ -223,13 +249,13 @@ public async Task API_Level_Event_Handlers_Should_Be_Removable()
Api.Instance.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);

var testProvider = new TestProvider();
await Api.Instance.SetProvider(testProvider);
await Api.Instance.SetProviderAsync(testProvider);

Thread.Sleep(1000);
Api.Instance.RemoveHandler(ProviderEventTypes.ProviderReady, eventHandler);

var newTestProvider = new TestProvider();
await Api.Instance.SetProvider(newTestProvider);
await Api.Instance.SetProviderAsync(newTestProvider);

eventHandler.Received(1).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
}
Expand All @@ -254,7 +280,7 @@ public async Task API_Level_Event_Handlers_Should_Be_Executed_When_Other_Handler
Api.Instance.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);

var testProvider = new TestProvider(fixture.Create<string>());
await Api.Instance.SetProvider(testProvider);
await Api.Instance.SetProviderAsync(testProvider);

await Utils.AssertUntilAsync(
_ => failingEventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name))
Expand All @@ -277,7 +303,7 @@ public async Task Client_Level_Event_Handlers_Should_Be_Registered()
var myClient = Api.Instance.GetClient(fixture.Create<string>());

var testProvider = new TestProvider();
await Api.Instance.SetProvider(myClient.GetMetadata().Name, testProvider);
await Api.Instance.SetProviderAsync(myClient.GetMetadata().Name, testProvider);

myClient.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);

Expand Down Expand Up @@ -306,7 +332,7 @@ public async Task Client_Level_Event_Handlers_Should_Be_Executed_When_Other_Hand
myClient.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);

var testProvider = new TestProvider();
await Api.Instance.SetProvider(myClient.GetMetadata().Name, testProvider);
await Api.Instance.SetProviderAsync(myClient.GetMetadata().Name, testProvider);

await Utils.AssertUntilAsync(
_ => failingEventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name))
Expand Down Expand Up @@ -335,9 +361,9 @@ public async Task Client_Level_Event_Handlers_Should_Be_Registered_To_Default_Pr
var clientProvider = new TestProvider(fixture.Create<string>());

// set the default provider on API level, but not specifically to the client
await Api.Instance.SetProvider(apiProvider);
await Api.Instance.SetProviderAsync(apiProvider);
// set the other provider specifically for the client
await Api.Instance.SetProvider(myClientWithBoundProvider.GetMetadata().Name, clientProvider);
await Api.Instance.SetProviderAsync(myClientWithBoundProvider.GetMetadata().Name, clientProvider);

myClientWithNoBoundProvider.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);
myClientWithBoundProvider.AddHandler(ProviderEventTypes.ProviderReady, clientEventHandler);
Expand Down Expand Up @@ -367,7 +393,7 @@ public async Task Client_Level_Event_Handlers_Should_Be_Receive_Events_From_Name
var clientProvider = new TestProvider(fixture.Create<string>());

// set the default provider
await Api.Instance.SetProvider(defaultProvider);
await Api.Instance.SetProviderAsync(defaultProvider);

client.AddHandler(ProviderEventTypes.ProviderConfigurationChanged, clientEventHandler);

Expand All @@ -379,7 +405,7 @@ await Utils.AssertUntilAsync(
);

// set the other provider specifically for the client
await Api.Instance.SetProvider(client.GetMetadata().Name, clientProvider);
await Api.Instance.SetProviderAsync(client.GetMetadata().Name, clientProvider);

// now, send another event for the default handler
defaultProvider.SendEvent(ProviderEventTypes.ProviderConfigurationChanged);
Expand Down Expand Up @@ -410,7 +436,7 @@ public async Task Client_Level_Event_Handlers_Should_Be_Informed_About_Ready_Sta
var myClient = Api.Instance.GetClient(fixture.Create<string>());

var testProvider = new TestProvider();
await Api.Instance.SetProvider(myClient.GetMetadata().Name, testProvider);
await Api.Instance.SetProviderAsync(myClient.GetMetadata().Name, testProvider);

// add the event handler after the provider has already transitioned into the ready state
myClient.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);
Expand All @@ -435,7 +461,7 @@ public async Task Client_Level_Event_Handlers_Should_Be_Removable()
myClient.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);

var testProvider = new TestProvider();
await Api.Instance.SetProvider(myClient.GetMetadata().Name, testProvider);
await Api.Instance.SetProviderAsync(myClient.GetMetadata().Name, testProvider);

// wait for the first event to be received
await Utils.AssertUntilAsync(_ => myClient.RemoveHandler(ProviderEventTypes.ProviderReady, eventHandler));
Expand Down
Loading

0 comments on commit 2919c2f

Please sign in to comment.