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

Drop ResilienceStrategyBuilderOptions #1117

Merged
merged 1 commit into from
Apr 13, 2023
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

This file was deleted.

52 changes: 26 additions & 26 deletions src/Polly.Core.Tests/Builder/ResilienceStrategyBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,23 @@
using Moq;
using Polly.Builder;
using Polly.Telemetry;
using Polly.Utils;

namespace Polly.Core.Tests.Builder;

public class ResilienceStrategyBuilderTests
{
[Fact]
public void Ctor_EnsureDefaults()
{
var builder = new ResilienceStrategyBuilder();

builder.BuilderName.Should().Be("");
builder.Properties.Should().NotBeNull();
builder.TimeProvider.Should().Be(TimeProvider.System);
builder.TelemetryFactory.Should().Be(NullResilienceTelemetryFactory.Instance);
}

[Fact]
public void AddStrategy_Single_Ok()
{
Expand Down Expand Up @@ -143,26 +155,20 @@ public void AddStrategy_AfterUsed_Throws()
.WithMessage("Cannot add any more resilience strategies to the builder after it has been used to build a strategy once.");
}

[Fact]
public void Options_SetNull_Throws()
{
var builder = new ResilienceStrategyBuilder();

builder.Invoking(b => b.Options = null!).Should().Throw<ArgumentNullException>();
}

[Fact]
public void Build_InvalidBuilderOptions_Throw()
{
var builder = new ResilienceStrategyBuilder();
builder.Options.BuilderName = null!;
var builder = new ResilienceStrategyBuilder
{
BuilderName = null!
};

builder.Invoking(b => b.Build())
.Should()
.Throw<ValidationException>()
.WithMessage(
"""
The 'ResilienceStrategyBuilderOptions' options are not valid.
The 'ResilienceStrategyBuilder' configuration is invalid.

Validation Errors:
The BuilderName field is required.
Expand Down Expand Up @@ -246,11 +252,8 @@ public void BuildStrategy_EnsureCorrectContext()

var builder = new ResilienceStrategyBuilder
{
Options = new ResilienceStrategyBuilderOptions
{
BuilderName = "builder-name",
TimeProvider = new FakeTimeProvider().Object
}
BuilderName = "builder-name",
TimeProvider = new FakeTimeProvider().Object
};

builder.AddStrategy(
Expand All @@ -259,10 +262,10 @@ public void BuildStrategy_EnsureCorrectContext()
context.BuilderName.Should().Be("builder-name");
context.StrategyName.Should().Be("strategy-name");
context.StrategyType.Should().Be("strategy-type");
context.BuilderProperties.Should().BeSameAs(builder.Options.Properties);
context.BuilderProperties.Should().BeSameAs(builder.Properties);
context.Telemetry.Should().NotBeNull();
context.Telemetry.Should().Be(NullResilienceTelemetry.Instance);
context.TimeProvider.Should().Be(builder.Options.TimeProvider);
context.TimeProvider.Should().Be(builder.TimeProvider);
verified1 = true;

return new TestResilienceStrategy();
Expand All @@ -275,10 +278,10 @@ public void BuildStrategy_EnsureCorrectContext()
context.BuilderName.Should().Be("builder-name");
context.StrategyName.Should().Be("strategy-name-2");
context.StrategyType.Should().Be("strategy-type-2");
context.BuilderProperties.Should().BeSameAs(builder.Options.Properties);
context.BuilderProperties.Should().BeSameAs(builder.Properties);
context.Telemetry.Should().NotBeNull();
context.Telemetry.Should().Be(NullResilienceTelemetry.Instance);
context.TimeProvider.Should().Be(builder.Options.TimeProvider);
context.TimeProvider.Should().Be(builder.TimeProvider);
verified2 = true;

return new TestResilienceStrategy();
Expand All @@ -300,11 +303,8 @@ public void BuildStrategy_EnsureTelemetryFactoryInvoked()
var factory = new Mock<ResilienceTelemetryFactory>(MockBehavior.Strict);
var builder = new ResilienceStrategyBuilder
{
Options = new ResilienceStrategyBuilderOptions
{
BuilderName = "builder-name",
TelemetryFactory = factory.Object
},
BuilderName = "builder-name",
TelemetryFactory = factory.Object
};

factory
Expand All @@ -315,7 +315,7 @@ public void BuildStrategy_EnsureTelemetryFactoryInvoked()
context.BuilderName.Should().Be("builder-name");
context.StrategyName.Should().Be("strategy-name");
context.StrategyType.Should().Be("strategy-type");
context.BuilderProperties.Should().BeSameAs(builder.Options.Properties);
context.BuilderProperties.Should().BeSameAs(builder.Properties);
});

builder.AddStrategy(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public void AddBuilder_GetStrategy_EnsureCalled()
registry.TryAddBuilder(StrategyId.Create("A"), (key, builder) =>
{
builder.AddStrategy(new TestResilienceStrategy());
builder.Options.Properties.Set(StrategyId.ResilienceKey, key);
builder.Properties.Set(StrategyId.ResilienceKey, key);
called++;
});

Expand Down
46 changes: 32 additions & 14 deletions src/Polly.Core/Builder/ResilienceStrategyBuilder.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.ComponentModel.DataAnnotations;
using Polly.Telemetry;

namespace Polly.Builder;
Expand All @@ -13,17 +14,34 @@ namespace Polly.Builder;
public class ResilienceStrategyBuilder
{
private readonly List<Entry> _entries = new();
private ResilienceStrategyBuilderOptions _options = new();
private bool _used;

/// <summary>
/// Gets or sets the builder options.
/// Gets or sets the name of the builder.
/// </summary>
public ResilienceStrategyBuilderOptions Options
{
get => _options;
set => _options = Guard.NotNull(value);
}
/// <remarks>This property is also included in the telemetry that is produced by the individual resilience strategies.</remarks>
[Required(AllowEmptyStrings = true)]
public string BuilderName { get; set; } = string.Empty;

/// <summary>
/// Gets the custom properties attached to builder options.
/// </summary>
public ResilienceProperties Properties { get; } = new();

/// <summary>
/// Gets or sets an instance of <see cref="TelemetryFactory"/>.
/// </summary>
[Required]
public ResilienceTelemetryFactory TelemetryFactory { get; set; } = NullResilienceTelemetryFactory.Instance;

/// <summary>
/// Gets or sets a <see cref="TimeProvider"/> that is used by strategies that work with time.
/// </summary>
/// <remarks>
/// This property is internal until we switch to official System.TimeProvider.
/// </remarks>
[Required]
internal TimeProvider TimeProvider { get; set; } = TimeProvider.System;

/// <summary>
/// Adds an already created strategy instance to the builder.
Expand Down Expand Up @@ -69,7 +87,7 @@ public ResilienceStrategyBuilder AddStrategy(Func<ResilienceStrategyBuilderConte
/// <returns>An instance of <see cref="ResilienceStrategy"/>.</returns>
public ResilienceStrategy Build()
{
ValidationHelper.ValidateObject(Options, $"The '{nameof(ResilienceStrategyBuilderOptions)}' options are not valid.");
ValidationHelper.ValidateObject(this, $"The '{nameof(ResilienceStrategyBuilder)}' configuration is invalid.");

_used = true;

Expand All @@ -92,20 +110,20 @@ private ResilienceStrategy CreateResilienceStrategy(Entry entry)
{
var telemetryContext = new ResilienceTelemetryFactoryContext
{
BuilderName = Options.BuilderName,
BuilderProperties = Options.Properties,
BuilderName = BuilderName,
BuilderProperties = Properties,
StrategyName = entry.Properties.StrategyName,
StrategyType = entry.Properties.StrategyType
};

var context = new ResilienceStrategyBuilderContext
{
BuilderName = Options.BuilderName,
BuilderProperties = Options.Properties,
BuilderName = BuilderName,
BuilderProperties = Properties,
StrategyName = entry.Properties.StrategyName,
StrategyType = entry.Properties.StrategyType,
Telemetry = Options.TelemetryFactory.Create(telemetryContext),
TimeProvider = Options.TimeProvider
Telemetry = TelemetryFactory.Create(telemetryContext),
TimeProvider = TimeProvider
};

return entry.Factory(context);
Expand Down
66 changes: 0 additions & 66 deletions src/Polly.Core/Builder/ResilienceStrategyBuilderOptions.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -70,29 +70,18 @@ public void AddResilienceStrategy_EnsureContextFilled()
}

[Fact]
public void AddResilienceStrategy_EnsureResilienceStrategyBuilderOptionsApplied()
public void AddResilienceStrategy_EnsureResilienceStrategyBuilderResolvedCorrectly()
{
var telemetry = Mock.Of<ResilienceTelemetry>();
var telemetryFactory = Mock.Of<ResilienceTelemetryFactory>(v => v.Create(It.IsAny<ResilienceTelemetryFactoryContext>()) == telemetry);
var asserted = false;
var key = new ResiliencePropertyKey<int>("A");
ResilienceStrategyBuilderOptions? globalOptions = null;

_services.Configure<ResilienceStrategyBuilderOptions>(options =>
{
options.BuilderName = "dummy";
options.TelemetryFactory = telemetryFactory;
options.Properties.Set(key, 123);
globalOptions = options;
});
_services.AddSingleton(telemetryFactory);

AddResilienceStrategy(Key, context =>
{
context.BuilderProperties.Should().NotBeSameAs(globalOptions!.Properties);
context.BuilderName.Should().Be("dummy");
context.Telemetry.Should().Be(telemetry);
context.BuilderProperties.TryGetValue(key, out var val).Should().BeTrue();
val.Should().Be(123);
asserted = true;
});

Expand Down Expand Up @@ -166,7 +155,7 @@ public void AddResilienceStrategy_CustomTelemetryFactory_EnsureUsed()
Key,
context =>
{
context.Builder.Options.TelemetryFactory.Should().Be(factory.Object);
context.Builder.TelemetryFactory.Should().Be(factory.Object);
context.Builder.AddStrategy(context =>
{
context.Telemetry.Should().Be(telemetry.Object);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,27 +95,16 @@ private static IServiceCollection AddResilienceStrategyRegistry<TKey>(this IServ

private static void AddResilienceStrategyBuilder(this IServiceCollection services)
{
services
.AddOptions<ResilienceStrategyBuilderOptions>()
.PostConfigure<IServiceProvider>((options, serviceProvider) =>
{
if (options.TelemetryFactory == NullResilienceTelemetryFactory.Instance &&
serviceProvider.GetService<ResilienceTelemetryFactory>() is ResilienceTelemetryFactory factory)
{
options.TelemetryFactory = factory;
}

options.Properties.Set(PollyDependencyInjectionKeys.ServiceProvider, serviceProvider);
});

services.TryAddTransient(serviceProvider =>
{
var globalOptions = serviceProvider.GetRequiredService<IOptions<ResilienceStrategyBuilderOptions>>().Value;

return new ResilienceStrategyBuilder
var builder = new ResilienceStrategyBuilder();
if (serviceProvider.GetService<ResilienceTelemetryFactory>() is ResilienceTelemetryFactory factory)
{
Options = new ResilienceStrategyBuilderOptions(globalOptions)
};
builder.TelemetryFactory = factory;
}

builder.Properties.Set(PollyDependencyInjectionKeys.ServiceProvider, serviceProvider);
return builder;
});
}

Expand Down