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: revert breaking setProvider #190

Merged
merged 22 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
38a8bd4
chore: revert breaking setProvider
toddbaert Jan 17, 2024
d663052
fixup: disable warning
toddbaert Jan 17, 2024
a4d9e6b
fixup: formatting
toddbaert Jan 17, 2024
e9bece4
fixup: review
toddbaert Jan 18, 2024
d34331a
fixup: more awaits in tests
toddbaert Jan 18, 2024
9837d32
Merge branch 'main' into chore/revert-breaking-setProvider-change
askpt Jan 22, 2024
5dddf65
fixup: merge conflicts
toddbaert Jan 22, 2024
952b002
fixup: use new util
toddbaert Jan 22, 2024
4343571
Update test/OpenFeature.Tests/ProviderRepositoryTests.cs
toddbaert Jan 22, 2024
b96117e
Update test/OpenFeature.Tests/ProviderRepositoryTests.cs
toddbaert Jan 22, 2024
de971f7
Update test/OpenFeature.Tests/ProviderRepositoryTests.cs
toddbaert Jan 22, 2024
08b4f48
Update test/OpenFeature.Tests/OpenFeatureTests.cs
toddbaert Jan 22, 2024
aa98784
Update test/OpenFeature.Tests/OpenFeatureTests.cs
toddbaert Jan 22, 2024
ca9c815
Update test/OpenFeature.Tests/OpenFeatureTests.cs
toddbaert Jan 22, 2024
5df17d8
Update test/OpenFeature.Tests/OpenFeatureTests.cs
toddbaert Jan 22, 2024
4d815ef
Update test/OpenFeature.Tests/OpenFeatureTests.cs
toddbaert Jan 22, 2024
36c0089
Update test/OpenFeature.Tests/OpenFeatureEventTests.cs
toddbaert Jan 22, 2024
8d227c6
Update test/OpenFeature.Tests/OpenFeatureTests.cs
toddbaert Jan 22, 2024
5e008dd
Update test/OpenFeature.Tests/OpenFeatureTests.cs
toddbaert Jan 22, 2024
ea26bc4
Update test/OpenFeature.Tests/OpenFeatureTests.cs
toddbaert Jan 22, 2024
528b723
Update test/OpenFeature.Tests/OpenFeatureTests.cs
toddbaert Jan 22, 2024
a9d9fbc
Update test/OpenFeature.Tests/OpenFeatureTests.cs
toddbaert Jan 22, 2024
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
30 changes: 27 additions & 3 deletions src/OpenFeature/Api.cs
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 @@
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)
toddbaert marked this conversation as resolved.
Show resolved Hide resolved
{
this._eventExecutor.RegisterDefaultFeatureProvider(featureProvider);
await this._repository.SetProvider(featureProvider, this.GetContext()).ConfigureAwait(false);
toddbaert marked this conversation as resolved.
Show resolved Hide resolved
}

/// <summary>
/// Sets the feature provider to given clientName without awaiting its initialization.
toddbaert marked this conversation as resolved.
Show resolved Hide resolved
/// </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());
}

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

View check run for this annotation

Codecov / codecov/patch

src/OpenFeature/Api.cs#L70-L73

Added lines #L70 - L73 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)
toddbaert marked this conversation as resolved.
Show resolved Hide resolved
{
this._eventExecutor.RegisterClientFeatureProvider(clientName, featureProvider);
await this._repository.SetProvider(clientName, featureProvider, this.GetContext()).ConfigureAwait(false);
toddbaert marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
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();
toddbaert marked this conversation as resolved.
Show resolved Hide resolved
client = Api.Instance.GetClient();
}

Expand Down
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();
toddbaert marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
22 changes: 11 additions & 11 deletions test/OpenFeature.Tests/OpenFeatureClientTests.cs
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
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