Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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 Microsoft.Mcp.slnx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
<Folder Name="/core/Microsoft.Mcp.Core/tests/">
<Project Path="core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Core.Tests/Microsoft.Mcp.Core.Tests.csproj" />
<Project Path="core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Tests/Microsoft.Mcp.Tests.csproj" />
<Project Path="core/Microsoft.Mcp.Core/tests/TrimmedOptionContainerApp/TrimmedOptionContainerApp.csproj" />
</Folder>
<Folder Name="/core/Microsoft.ModelContextProtocol.HttpServer.Distributed/" />
<Folder Name="/core/Microsoft.ModelContextProtocol.HttpServer.Distributed/src/">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ public sealed class BaseGroupOptions : ISubscriptionOption
[Option(Description = OptionDescriptions.Tenant)]
public string? Tenant { get; set; }

[OptionContainer(Prefix = "retry")]
[OptionContainer<RetryPolicyOptions>(Prefix = "retry")]
public RetryPolicyOptions? RetryPolicy { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ public sealed class ResourceListOptions : ISubscriptionOption
[Option(Description = OptionDescriptions.Tenant)]
public string? Tenant { get; set; }

[OptionContainer(Prefix = "retry")]
[OptionContainer<RetryPolicyOptions>(Prefix = "retry")]
public RetryPolicyOptions? RetryPolicy { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ public sealed class SubscriptionListOptions
[Option(Description = OptionDescriptions.Tenant)]
public string? Tenant { get; set; }

[OptionContainer(Prefix = "retry")]
[OptionContainer<RetryPolicyOptions>(Prefix = "retry")]
public RetryPolicyOptions? RetryPolicy { get; set; }
}
16 changes: 16 additions & 0 deletions core/Microsoft.Mcp.Core/src/Options/IOptionContainerMetadata.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Diagnostics.CodeAnalysis;

namespace Microsoft.Mcp.Core.Options;

internal interface IOptionContainerMetadata
{
string? Prefix { get; }

[DynamicallyAccessedMembers(
DynamicallyAccessedMemberTypes.PublicProperties |
DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]
Type ContainerType { get; }
}
11 changes: 5 additions & 6 deletions core/Microsoft.Mcp.Core/src/Options/OptionBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,9 @@ public static class OptionBinder
parentInstances ??= [];
if (!parentInstances.TryGetValue(handler.Descriptor.ParentProperty, out var parent))
{
parent = CreateInstance(handler.Descriptor.ParentProperty.PropertyType);
var parentType = handler.Descriptor.ParentType
?? throw new InvalidOperationException("Nested option descriptor is missing its parent type.");
parent = CreateInstance(parentType);
parentInstances[handler.Descriptor.ParentProperty] = parent;
}
handler.Descriptor.TargetProperty.SetValue(parent, value);
Expand Down Expand Up @@ -124,11 +126,8 @@ public static class OptionBinder
return instance;
}

[UnconditionalSuppressMessage("Trimming", "IL2067:UnrecognizedReflectionPattern",
Justification = "Nested option types are rooted by the application via property references.")]
[UnconditionalSuppressMessage("AOT", "IL3050:RequiresDynamicCode",
Justification = "Nested option types use parameterless constructors rooted by the application.")]
private static object CreateInstance(Type type)
private static object CreateInstance(
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] Type type)
{
return Activator.CreateInstance(type)
?? throw new InvalidOperationException($"Failed to create instance of nested options type '{type.Name}'. Ensure it has a public parameterless constructor.");
Expand Down
24 changes: 0 additions & 24 deletions core/Microsoft.Mcp.Core/src/Options/OptionContainerAttribute.cs
Comment thread
LarryOsterman marked this conversation as resolved.
Outdated

This file was deleted.

Comment thread
LarryOsterman marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Diagnostics.CodeAnalysis;

namespace Microsoft.Mcp.Core.Options;

/// <summary>
/// Identifies a complex option container and preserves the members required to discover and bind its options.
/// </summary>
/// <typeparam name="TContainer">The type of the option container.</typeparam>
[AttributeUsage(AttributeTargets.Property, Inherited = true, AllowMultiple = false)]
public sealed class OptionContainerAttribute<
[DynamicallyAccessedMembers(ContainerMembers)] TContainer> : Attribute, IOptionContainerMetadata
{
private const DynamicallyAccessedMemberTypes ContainerMembers =
DynamicallyAccessedMemberTypes.PublicProperties |
DynamicallyAccessedMemberTypes.PublicParameterlessConstructor;

/// <summary>
/// The prefix to use for the options in the container.
/// If null, the property name (in kebab-case) is used as the prefix.
/// </summary>
public string? Prefix { get; init; }

[DynamicallyAccessedMembers(ContainerMembers)]
Type IOptionContainerMetadata.ContainerType => typeof(TContainer);
Comment thread
LarryOsterman marked this conversation as resolved.
Outdated
}
36 changes: 28 additions & 8 deletions core/Microsoft.Mcp.Core/src/Options/OptionDescriptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,24 @@ public class OptionDescriptor
public required PropertyInfo TargetProperty { get; init; }
public required Type Type { get; init; }
public PropertyInfo? ParentProperty { get; init; }
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]
public Type? ParentType { get; init; }

public static OptionDescriptor[] FromType<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T>() where T : class
{
List<OptionDescriptor> optionDescriptors = [];
CollectDescriptors(typeof(T), null, optionDescriptors, new(), true, null);
CollectDescriptors(typeof(T), null, optionDescriptors, new(), true, null, null);
return [.. optionDescriptors];
}

[UnconditionalSuppressMessage("Trimming", "IL2070:UnrecognizedReflectionPattern",
Justification = "Nested option types are rooted by the application.")]
private static void CollectDescriptors(
Type type,
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] Type type,
string? prefix,
List<OptionDescriptor> descriptors,
NullabilityInfoContext nullabilityContext,
bool parentRequired,
PropertyInfo? parentProperty)
PropertyInfo? parentProperty,
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] Type? parentType)
{
PropertyInfo[] allProperties = type.GetProperties(BindingFlags.Public | BindingFlags.Instance);

Expand All @@ -60,7 +61,9 @@ private static void CollectDescriptors(
}

var optionAttribute = property.GetCustomAttribute<OptionAttribute>();
var optionContainerAttribute = property.GetCustomAttribute<OptionContainerAttribute>();
var optionContainerAttribute = property.GetCustomAttributes()
.OfType<IOptionContainerMetadata>()
.SingleOrDefault();
// Only include properties with [Option] or [OptionContainer]
if (optionAttribute == null && optionContainerAttribute == null)
{
Expand Down Expand Up @@ -100,7 +103,8 @@ private static void CollectDescriptors(
DefaultValue = optionAttribute.DefaultValue,
AllowEmptyOrWhiteSpaceString = optionAttribute.AllowEmptyOrWhiteSpaceString,
TargetProperty = property,
ParentProperty = parentProperty
ParentProperty = parentProperty,
ParentType = parentType
});
}

Expand All @@ -110,8 +114,24 @@ private static void CollectDescriptors(
{
throw new InvalidOperationException("Non-complex properties cannot use [OptionContainer] attribute. Use [Option] instead.");
}
Comment thread
LarryOsterman marked this conversation as resolved.

var containerType = optionContainerAttribute.ContainerType;
var declaredContainerType = Nullable.GetUnderlyingType(property.PropertyType) ?? property.PropertyType;
if (containerType != declaredContainerType)
{
throw new InvalidOperationException(
$"Option container type '{containerType.Name}' does not match property type '{declaredContainerType.Name}'.");
}

// Flatten nested complex types with a prefix.
CollectDescriptors(property.PropertyType, GetNameOrPrefix(optionContainerAttribute.Prefix, prefix, property.Name), descriptors, nullabilityContext, required, property);
CollectDescriptors(
containerType,
GetNameOrPrefix(optionContainerAttribute.Prefix, prefix, property.Name),
descriptors,
nullabilityContext,
required,
property,
containerType);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,17 +123,17 @@ private sealed class NestedOptional
{
[Option(Description = "The name of the item.")]
public string? Name { get; set; }
[OptionContainer]
[OptionContainer<NetworkSettings>]
public NetworkSettings? Optional { get; set; }
[OptionContainer]
[OptionContainer<NetworkSettings>]
public required NetworkSettings Required { get; set; }
}

private sealed class NestedRequired
{
[Option(Description = "The name of the item.")]
public string? Name { get; set; }
[OptionContainer]
[OptionContainer<RequiredNetworkSettings>]
public required RequiredNetworkSettings Required { get; set; }
}

Expand Down Expand Up @@ -168,7 +168,7 @@ private sealed class EmptyOrWhiteSpaceOptions
private sealed class InvalidAttributesCombinationOptions
{
[Option(Description = "Invalid attribute combination.")]
[OptionContainer]
[OptionContainer<NetworkSettings>]
public NetworkSettings? Invalid { get; set; }
}

Expand All @@ -180,10 +180,16 @@ private sealed class InvalidOptionAttributeOptions

private sealed class InvalidOptionContainerAttributeOptions
{
[OptionContainer]
[OptionContainer<string>]
public string? Invalid { get; set; }
}

private sealed class MismatchedOptionContainerOptions
{
[OptionContainer<NetworkSettings>]
public RequiredNetworkSettings? Invalid { get; set; }
}

#endregion

#region RegisterOptions Tests
Expand Down Expand Up @@ -300,6 +306,17 @@ public void RegisterOptions_InvalidOptionContainerAttribute_Throws()
Assert.Contains("Non-complex properties cannot use [OptionContainer] attribute. Use [Option] instead.", ex.Message);
}

[Fact]
public void RegisterOptions_MismatchedOptionContainerType_Throws()
{
var command = new Command("test");

var ex = Assert.Throws<InvalidOperationException>(
() => OptionBinder.RegisterOptions<MismatchedOptionContainerOptions>(command));

Assert.Contains("does not match property type", ex.Message);
}

#endregion

#region BindOptions Tests
Expand Down Expand Up @@ -739,6 +756,14 @@ public void TrimAnnotations_CommandAnnotations_IncludePublicPropertiesAndParamet
Assert.True(annotations.HasFlag(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor));
}

[Fact]
public void OptionContainerAttribute_GenericType_PreservesBindingMembers()
{
var containerType = typeof(OptionContainerAttribute<>).GetGenericArguments()[0];

AssertHasRequiredMemberAnnotations(containerType);
}

private static void AssertHasRequiredMemberAnnotations(MemberInfo member)
{
var attribute = member.GetCustomAttribute<DynamicallyAccessedMembersAttribute>();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Diagnostics;
using System.Reflection;
using System.Runtime.InteropServices;
using Xunit;

namespace Microsoft.Mcp.Core.Tests.Options;

public sealed class OptionBinderTrimmedTests
{
[Fact]
public async Task TrimmedPublish_PreservesNestedOptionContainerProperties()
{
var sourcePath = Assembly.GetExecutingAssembly()
.GetCustomAttributes<AssemblyMetadataAttribute>()
.Single(attribute => attribute.Key == "SourcePath")
.Value!;
var projectPath = Path.GetFullPath(
Path.Combine(sourcePath, "..", "TrimmedOptionContainerApp", "TrimmedOptionContainerApp.csproj"));
var publishPath = Path.Combine(Path.GetTempPath(), nameof(OptionBinderTrimmedTests), Guid.NewGuid().ToString("N"));

try
{
var publishResult = await RunProcessAsync(
"dotnet",
[
"publish",
projectPath,
"--configuration", "Release",
"--runtime", RuntimeInformation.RuntimeIdentifier,
"--self-contained", "true",
"--output", publishPath,
"/p:PublishTrimmed=true"
]);

Assert.True(
publishResult.ExitCode == 0,
$"Trimmed publish failed.{Environment.NewLine}{publishResult.StandardOutput}{Environment.NewLine}{publishResult.StandardError}");

var executableName = OperatingSystem.IsWindows()
? "TrimmedOptionContainerApp.exe"
: "TrimmedOptionContainerApp";
var executablePath = Path.Combine(publishPath, executableName);
var executionResult = await RunProcessAsync(executablePath, []);

Assert.True(
executionResult.ExitCode == 0,
$"Trimmed executable did not discover and bind nested retry options.{Environment.NewLine}" +
$"{executionResult.StandardOutput}{Environment.NewLine}{executionResult.StandardError}");
}
finally
{
if (Directory.Exists(publishPath))
{
Directory.Delete(publishPath, recursive: true);
}
}
}

private static async Task<(int ExitCode, string StandardOutput, string StandardError)> RunProcessAsync(
string fileName,
string[] arguments)
{
var startInfo = new ProcessStartInfo
{
FileName = fileName,
UseShellExecute = false,
RedirectStandardOutput = true,
RedirectStandardError = true,
CreateNoWindow = true
};

foreach (var argument in arguments)
{
startInfo.ArgumentList.Add(argument);
}

using var process = Process.Start(startInfo)
?? throw new InvalidOperationException($"Failed to start '{fileName}'.");
var standardOutput = process.StandardOutput.ReadToEndAsync();
var standardError = process.StandardError.ReadToEndAsync();

await process.WaitForExitAsync(TestContext.Current.CancellationToken);
return (process.ExitCode, await standardOutput, await standardError);
}
}
Loading
Loading