Skip to content

Commit

Permalink
chore: revert breaking setProvider
Browse files Browse the repository at this point in the history 
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
  • Loading branch information
@toddbaert
toddbaert committed Jan 17, 2024
1 parent 950775b commit 4ea63e3
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 60 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 @@ -38,25 +39,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()).ConfigureAwait(false);
}

/// <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()).ConfigureAwait(false);
}

Check warning on line 75 in src/OpenFeature/Api.cs

View check run for this annotation

Codecov / codecov/patch

src/OpenFeature/Api.cs#L72-L75 

Added lines #L72 - L75 were not covered by tests

/// <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
59 changes: 42 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 @@ -118,7 +118,32 @@ 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);

Thread.Sleep(1000);
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 void API_Level_Event_Handlers_Should_Be_Informed_About_Ready_State_After_Registering_Provider_Ready_Sync()
{
var eventHandler = Substitute.For<EventHandlerDelegate>();

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

Check failure on line 146 in test/OpenFeature.Tests/OpenFeatureEventTests.cs

View workflow job for this annotation

GitHub Actions / unit-tests-linux 

'Api.SetProvider(FeatureProvider)' is obsolete: 'Will be removed in later versions; use SetProviderAsync, which can be awaited'

Check failure on line 146 in test/OpenFeature.Tests/OpenFeatureEventTests.cs

View workflow job for this annotation

GitHub Actions / unit-tests-linux 

'Api.SetProvider(FeatureProvider)' is obsolete: 'Will be removed in later versions; use SetProviderAsync, which can be awaited'

Check failure on line 146 in test/OpenFeature.Tests/OpenFeatureEventTests.cs

View workflow job for this annotation

GitHub Actions / unit-tests-linux 

'Api.SetProvider(FeatureProvider)' is obsolete: 'Will be removed in later versions; use SetProviderAsync, which can be awaited'

Check failure on line 146 in test/OpenFeature.Tests/OpenFeatureEventTests.cs

View workflow job for this annotation

GitHub Actions / unit-tests-windows 

'Api.SetProvider(FeatureProvider)' is obsolete: 'Will be removed in later versions; use SetProviderAsync, which can be awaited'

Check failure on line 146 in test/OpenFeature.Tests/OpenFeatureEventTests.cs

View workflow job for this annotation

GitHub Actions / unit-tests-windows 

'Api.SetProvider(FeatureProvider)' is obsolete: 'Will be removed in later versions; use SetProviderAsync, which can be awaited'

Check failure on line 146 in test/OpenFeature.Tests/OpenFeatureEventTests.cs

View workflow job for this annotation

GitHub Actions / unit-tests-windows 

'Api.SetProvider(FeatureProvider)' is obsolete: 'Will be removed in later versions; use SetProviderAsync, which can be awaited'

Check failure on line 146 in test/OpenFeature.Tests/OpenFeatureEventTests.cs

View workflow job for this annotation

GitHub Actions / unit-tests-windows 

'Api.SetProvider(FeatureProvider)' is obsolete: 'Will be removed in later versions; use SetProviderAsync, which can be awaited'

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

Expand All @@ -143,7 +168,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 @@ -169,7 +194,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 @@ -198,12 +223,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 @@ -224,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 @@ -255,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);

failingEventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
eventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
Expand All @@ -274,7 +299,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 @@ -303,7 +328,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);

Thread.Sleep(1000);

Expand All @@ -330,9 +355,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 @@ -362,7 +387,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 @@ -374,7 +399,7 @@ public async Task Client_Level_Event_Handlers_Should_Be_Receive_Events_From_Name
clientEventHandler.Received(1).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == defaultProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderConfigurationChanged));

// 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 @@ -403,7 +428,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 @@ -428,7 +453,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
Thread.Sleep(1000);
Expand Down
Loading

0 comments on commit 4ea63e3

Please sign in to comment.