Skip to content

[feature/10.0][WIP] Use options validation source generator #8254

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

Draft
wants to merge 15 commits into
base: feature/10.0
Choose a base branch
from
Draft
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
1 change: 1 addition & 0 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
<PackageVersion Include="Microsoft.Extensions.Logging" Version="$(MicrosoftExtensionsLoggingVersion)" />
<PackageVersion Include="Microsoft.Extensions.Logging.Abstractions" Version="$(MicrosoftExtensionsLoggingAbstractionsVersion)" />
<PackageVersion Include="Microsoft.Extensions.Logging.Console" Version="$(MicrosoftExtensionsLoggingConsoleVersion)" />
<PackageVersion Include="Microsoft.Extensions.Options" Version="$(MicrosoftExtensionsOptionsVersion)" />
<PackageVersion Include="Microsoft.FileFormats" Version="$(MicrosoftFileFormatsVersion)" />
<PackageVersion Include="Microsoft.Identity.Web" Version="$(MicrosoftIdentityWebVersion)" />
<PackageVersion Include="Microsoft.OpenApi.Readers" Version="$(MicrosoftOpenApiReadersVersion)" />
Expand Down
3 changes: 3 additions & 0 deletions eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
<MicrosoftExtensionsLoggingVersion>$(MicrosoftExtensionsLogging80Version)</MicrosoftExtensionsLoggingVersion>
<MicrosoftExtensionsLoggingAbstractionsVersion>$(MicrosoftExtensionsLoggingAbstractions80Version)</MicrosoftExtensionsLoggingAbstractionsVersion>
<MicrosoftExtensionsLoggingConsoleVersion>$(MicrosoftExtensionsLoggingConsole80Version)</MicrosoftExtensionsLoggingConsoleVersion>
<MicrosoftExtensionsOptionsVersion>$(MicrosoftExtensionsOptions80Version)</MicrosoftExtensionsOptionsVersion>
<SystemTextJsonVersion>$(SystemTextJson80Version)</SystemTextJsonVersion>
</PropertyGroup>
<PropertyGroup Label=".NET 9 Dependent" Condition=" '$(TargetFramework)' == 'net9.0' ">
Expand All @@ -95,6 +96,7 @@
<MicrosoftExtensionsLoggingVersion>$(MicrosoftExtensionsLogging90Version)</MicrosoftExtensionsLoggingVersion>
<MicrosoftExtensionsLoggingAbstractionsVersion>$(MicrosoftExtensionsLoggingAbstractions90Version)</MicrosoftExtensionsLoggingAbstractionsVersion>
<MicrosoftExtensionsLoggingConsoleVersion>$(MicrosoftExtensionsLoggingConsole90Version)</MicrosoftExtensionsLoggingConsoleVersion>
<MicrosoftExtensionsOptionsVersion>$(MicrosoftExtensionsOptions90Version)</MicrosoftExtensionsOptionsVersion>
<SystemTextJsonVersion>$(SystemTextJson90Version)</SystemTextJsonVersion>
<MicrosoftAspNetCoreOpenApiVersion>$(MicrosoftAspNetCoreApp90Version)</MicrosoftAspNetCoreOpenApiVersion>
</PropertyGroup>
Expand All @@ -105,6 +107,7 @@
<MicrosoftExtensionsLoggingVersion>$(MicrosoftNETCoreApp100Version)</MicrosoftExtensionsLoggingVersion>
<MicrosoftExtensionsLoggingAbstractionsVersion>$(MicrosoftNETCoreApp100Version)</MicrosoftExtensionsLoggingAbstractionsVersion>
<MicrosoftExtensionsLoggingConsoleVersion>$(MicrosoftNETCoreApp100Version)</MicrosoftExtensionsLoggingConsoleVersion>
<MicrosoftExtensionsOptionsVersion>$(MicrosoftNETCoreApp100Version)</MicrosoftExtensionsOptionsVersion>
<SystemTextJsonVersion>$(MicrosoftNETCoreApp100Version)</SystemTextJsonVersion>
<MicrosoftAspNetCoreOpenApiVersion>$(MicrosoftAspNetCoreApp100Version)</MicrosoftAspNetCoreOpenApiVersion>
</PropertyGroup>
Expand Down
1 change: 1 addition & 0 deletions eng/dependabot/net8.0/Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<PackageReference Include="Microsoft.Extensions.Logging" Version="$(MicrosoftExtensionsLogging80Version)" />
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="$(MicrosoftExtensionsLoggingAbstractions80Version)" />
<PackageReference Include="Microsoft.Extensions.Logging.Console" Version="$(MicrosoftExtensionsLoggingConsole80Version)" />
<PackageReference Include="Microsoft.Extensions.Options" Version="$(MicrosoftExtensionsOptions80Version)" />
<!--
We want to update "Microsoft.NETCore.App.Runtime.*" but those packages are considered platform packages and cannot be added
as a package reference. Use Microsoft.NETCore.DotNetHost instead which is released in lockstep with the runtime packages and
Expand Down
2 changes: 2 additions & 0 deletions eng/dependabot/net8.0/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
<MicrosoftExtensionsLoggingAbstractions80Version>8.0.3</MicrosoftExtensionsLoggingAbstractions80Version>
<!-- Microsoft.Extensions.Logging.Console -->
<MicrosoftExtensionsLoggingConsole80Version>8.0.1</MicrosoftExtensionsLoggingConsole80Version>
<!-- Microsoft.Extensions.Options -->
<MicrosoftExtensionsOptions80Version>8.0.2</MicrosoftExtensionsOptions80Version>
<!-- Microsoft.NETCore.App -->
<MicrosoftNETCoreApp80Version>8.0.15</MicrosoftNETCoreApp80Version>
<!-- System.Text.Json -->
Expand Down
2 changes: 2 additions & 0 deletions eng/dependabot/net9.0/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
<MicrosoftExtensionsLoggingAbstractions90Version>9.0.4</MicrosoftExtensionsLoggingAbstractions90Version>
<!-- Microsoft.Extensions.Logging.Console -->
<MicrosoftExtensionsLoggingConsole90Version>9.0.4</MicrosoftExtensionsLoggingConsole90Version>
<!-- Microsoft.Extensions.Options -->
<MicrosoftExtensionsOptions90Version>9.0.4</MicrosoftExtensionsOptions90Version>
<!-- Microsoft.NETCore.App -->
<MicrosoftNETCoreApp90Version>9.0.4</MicrosoftNETCoreApp90Version>
<!-- System.Text.Json -->
Expand Down
10 changes: 10 additions & 0 deletions src/Directory.Build.targets
Original file line number Diff line number Diff line change
Expand Up @@ -126,4 +126,14 @@
</ItemGroup>
</Target>

<!-- Disable options validation source generator, to let us use a checked-in copy
of generated sources that include a fix for https://github.com/dotnet/runtime/issues/115347 -->
<Target Name="DisableOptionsValidationGenerator"
BeforeTargets="CoreCompile"
Condition="'$(DisableOptionsValidationGenerator)' == 'true'">
<ItemGroup>
<Analyzer Remove="@(Analyzer)" Condition="'%(Filename)' == 'Microsoft.Extensions.Options.SourceGeneration'" />
</ItemGroup>
</Target>

</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ private static ServiceCollection CreateServices<TOptions>(ExtensionEgressPayload
.Build()
.Bind(options);
});
services.AddSingleton<IValidateOptions<TOptions>, DataAnnotationValidateOptions<TOptions>>();

services.AddSingleton(new EgressProperties(payload.Properties));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@
<IsShippingAssembly>true</IsShippingAssembly>
</PropertyGroup>

<ItemGroup>
<Compile Include="..\Tools\dotnet-monitor\DataAnnotationValidateOptions.cs" Link="DataAnnotationValidateOptions.cs" />
</ItemGroup>

<ItemGroup>
<InternalsVisibleTo Include="Microsoft.Diagnostics.Monitoring.AzureBlobStorageTests.UnitTests" />
<InternalsVisibleTo Include="Microsoft.Diagnostics.Monitoring.S3StorageTests.UnitTests" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
using System.ComponentModel.DataAnnotations;
using System.Globalization;
using System.Linq;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;

namespace Microsoft.Diagnostics.Monitoring.WebApi
{
Expand Down Expand Up @@ -52,19 +54,19 @@ partial class GlobalCounterOptions : IValidatableObject
{
public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
{
var providerValidator = validationContext.GetRequiredService<IValidateOptions<GlobalProviderOptions>>();
var results = new List<ValidationResult>();

if (Providers != null)
{
var providerResults = new List<ValidationResult>();
foreach ((string provider, GlobalProviderOptions options) in Providers)
{
providerResults.Clear();
if (!Validator.TryValidateObject(options, new ValidationContext(options), providerResults, true))
ValidateOptionsResult providerResults = providerValidator.Validate(provider, options);
if (providerResults.Failed)
{
// We prefix the validation error with the provider.
results.AddRange(providerResults.Select(r => new ValidationResult(
string.Format(CultureInfo.CurrentCulture, OptionsDisplayStrings.ErrorMessage_NestedProviderValidationError, provider, r.ErrorMessage))));
results.AddRange(providerResults.Failures.Select(r => new ValidationResult(
string.Format(CultureInfo.CurrentCulture, OptionsDisplayStrings.ErrorMessage_NestedProviderValidationError, provider, r))));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.Extensions.Options;
using System;
using System.Collections.Generic;
using System.ComponentModel;
Expand Down Expand Up @@ -43,6 +44,7 @@ public class MetricsOptions
[Display(
ResourceType = typeof(OptionsDisplayStrings),
Description = nameof(OptionsDisplayStrings.DisplayAttributeDescription_MetricsOptions_Providers))]
[ValidateEnumeratedItems]
public List<MetricProvider> Providers { get; set; } = [];

[Display(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<!--
Expand All @@ -9,6 +9,7 @@
<IsShippingAssembly>true</IsShippingAssembly>
<OutputType>Library</OutputType>
<Nullable>enable</Nullable>
<DisableOptionsValidationGenerator>true</DisableOptionsValidationGenerator>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Configuration;
using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options.Actions;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Options;
using System;
using System.Collections.Generic;
using System.Threading;
Expand Down Expand Up @@ -75,17 +76,19 @@ public ICollectionRuleAction Create(IProcessInfo processInfo, BaseRecordOptions
}
}

internal sealed class CallbackActionDescriptor : ICollectionRuleActionDescriptor
internal sealed partial class CallbackActionDescriptor : ICollectionRuleActionDescriptor<BaseRecordOptions, CallbackActionFactory>
{
public string ActionName => CallbackAction.ActionName;
public Type OptionsType => typeof(BaseRecordOptions);
public Type FactoryType => typeof(CallbackActionFactory);

public void BindOptions(IConfigurationSection settingsSection, out object settings)
public void BindOptions(IConfigurationSection settingsSection, out BaseRecordOptions options)
{
BaseRecordOptions options = new();
options = new();
settingsSection.Bind(options);
settings = options;
}

public ValidateOptionsResult Validate(string name, BaseRecordOptions options)
{
return ValidateOptionsResult.Success;
}
}

Expand Down Expand Up @@ -127,17 +130,19 @@ public Task<CollectionRuleActionResult> WaitForCompletionAsync(CancellationToken
}
}

internal sealed class DelayedCallbackActionDescriptor : ICollectionRuleActionDescriptor
internal sealed partial class DelayedCallbackActionDescriptor : ICollectionRuleActionDescriptor<BaseRecordOptions, DelayedCallbackActionFactory>
{
public string ActionName => DelayedCallbackAction.ActionName;
public Type OptionsType => typeof(BaseRecordOptions);
public Type FactoryType => typeof(DelayedCallbackActionFactory);

public void BindOptions(IConfigurationSection settingsSection, out object settings)
public void BindOptions(IConfigurationSection settingsSection, out BaseRecordOptions options)
{
BaseRecordOptions options = new();
options = new();
settingsSection.Bind(options);
settings = options;
}

public ValidateOptionsResult Validate(string name, BaseRecordOptions options)
{
return ValidateOptionsResult.Success;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Configuration;
using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options.Actions;
using Microsoft.Extensions.Configuration;
using System;
using Microsoft.Extensions.Options;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -56,17 +56,19 @@ public Task<CollectionRuleActionResult> WaitForCompletionAsync(CancellationToken
}
}

internal sealed class PassThroughActionDescriptor : ICollectionRuleActionDescriptor
internal sealed partial class PassThroughActionDescriptor : ICollectionRuleActionDescriptor<PassThroughOptions, PassThroughActionFactory>
{
public string ActionName => nameof(PassThroughAction);
public Type OptionsType => typeof(PassThroughOptions);
public Type FactoryType => typeof(PassThroughActionFactory);

public void BindOptions(IConfigurationSection settingsSection, out object settings)
public void BindOptions(IConfigurationSection settingsSection, out PassThroughOptions options)
{
PassThroughOptions options = new();
options = new();
settingsSection.Bind(options);
settings = options;
}

public ValidateOptionsResult Validate(string name, PassThroughOptions options)
{
return ValidateOptionsResult.Success;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,11 @@ await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions =>
}, host =>
{
OptionsValidationException invalidOptionsException = Assert.Throws<OptionsValidationException>(
() => ActionTestsHelper.GetActionOptions<OptionsValidationException>(host, DefaultRuleName));
() => ActionTestsHelper.GetActionOptions<CollectDumpOptions>(host, DefaultRuleName));

Assert.Equal(string.Format(OptionsDisplayStrings.ErrorMessage_NoDefaultEgressProvider), invalidOptionsException.Message);
string message = string.Format(OptionsDisplayStrings.ErrorMessage_NoDefaultEgressProvider);

Assert.Equal($"{nameof(CollectDumpOptions.Egress)}: {message}", invalidOptionsException.Message);
});
}

Expand Down Expand Up @@ -143,7 +145,7 @@ await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions =>
OptionsValidationException invalidOptionsException = Assert.Throws<OptionsValidationException>(
() => TriggerTestsHelper.GetTriggerOptions<AspNetRequestCountOptions>(host, DefaultRuleName));

VerifyRangeMessage<int>(invalidOptionsException, nameof(AspNetRequestCountOptions.RequestCount), "1", int.MaxValue.ToString());
VerifyRangeMessage<int>(invalidOptionsException, nameof(AspNetRequestCountOptions), nameof(AspNetRequestCountOptions.RequestCount), "1", int.MaxValue.ToString());
});
}

Expand Down Expand Up @@ -217,7 +219,7 @@ await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions =>
OptionsValidationException invalidOptionsException = Assert.Throws<OptionsValidationException>(
() => TriggerTestsHelper.GetTriggerOptions<AspNetRequestDurationOptions>(host, DefaultRuleName));

VerifyRangeMessage<int>(invalidOptionsException, nameof(AspNetRequestDurationOptions.RequestCount), "1", int.MaxValue.ToString());
VerifyRangeMessage<int>(invalidOptionsException, nameof(AspNetRequestDurationOptions), nameof(AspNetRequestDurationOptions.RequestCount), "1", int.MaxValue.ToString());
});
}

Expand Down Expand Up @@ -297,7 +299,7 @@ await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions =>
OptionsValidationException invalidOptionsException = Assert.Throws<OptionsValidationException>(
() => TriggerTestsHelper.GetTriggerOptions<AspNetResponseStatusOptions>(host, DefaultRuleName));

VerifyRangeMessage<int>(invalidOptionsException, nameof(AspNetResponseStatusOptions.ResponseCount), "1", int.MaxValue.ToString());
VerifyRangeMessage<int>(invalidOptionsException, nameof(AspNetResponseStatusOptions), nameof(AspNetResponseStatusOptions.ResponseCount), "1", int.MaxValue.ToString());
});
}

Expand Down Expand Up @@ -550,11 +552,11 @@ await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions =>
});
}

private static void VerifyRangeMessage<T>(Exception ex, string fieldName, string min, string max)
private static void VerifyRangeMessage<T>(Exception ex, string typeName, string fieldName, string min, string max)
{
string message = (new RangeAttribute(typeof(T), min, max)).FormatErrorMessage(fieldName);
string message = (new RangeAttribute(typeof(T), min, max)).FormatErrorMessage($"{typeName}.{fieldName}");

Assert.Equal(message, ex.Message);
Assert.Equal($"{fieldName}: {message}", ex.Message);
}
}
}
Loading
Loading