Skip to content

Commit 0a6ebca

Browse files
committed
PR feedback
1 parent 607c517 commit 0a6ebca

8 files changed

Lines changed: 80 additions & 68 deletions

File tree

src/ModelContextProtocol.AspNetCore/HttpMcpServerBuilderExtensions.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public static IMcpServerBuilder WithHttpTransport(this IMcpServerBuilder builder
3535
builder.Services.AddHostedService<IdleTrackingBackgroundService>();
3636

3737
builder.Services.TryAddEnumerable(ServiceDescriptor.Transient<IPostConfigureOptions<McpServerOptions>, AuthorizationFilterSetup>());
38-
builder.Services.TryAddEnumerable(ServiceDescriptor.Transient<IPostConfigureOptions<HttpServerTransportOptions>, HttpServerTransportOptionsSetup>());
38+
builder.Services.TryAddEnumerable(ServiceDescriptor.Transient<IConfigureOptions<HttpServerTransportOptions>, HttpServerTransportOptionsSetup>());
3939

4040
if (configureOptions is not null)
4141
{
@@ -107,7 +107,7 @@ public static IMcpServerBuilder WithDistributedCacheEventStreamStore(this IMcpSe
107107
{
108108
ArgumentNullException.ThrowIfNull(builder);
109109

110-
builder.Services.TryAddSingleton<ISseEventStreamStore>(sp =>
110+
builder.Services.AddSingleton<ISseEventStreamStore>(sp =>
111111
{
112112
var resolvedCache = cache ?? sp.GetRequiredService<IDistributedCache>();
113113
var options = sp.GetService<IOptions<DistributedCacheEventStreamStoreOptions>>()?.Value;

src/ModelContextProtocol.AspNetCore/HttpServerTransportOptionsSetup.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ namespace ModelContextProtocol.AspNetCore;
88
/// Post-configures <see cref="HttpServerTransportOptions"/> by resolving services from DI
99
/// when they haven't been explicitly set on the options.
1010
/// </summary>
11-
internal sealed class HttpServerTransportOptionsSetup(IServiceProvider serviceProvider) : IPostConfigureOptions<HttpServerTransportOptions>
11+
internal sealed class HttpServerTransportOptionsSetup(IServiceProvider serviceProvider) : IConfigureOptions<HttpServerTransportOptions>
1212
{
13-
public void PostConfigure(string? name, HttpServerTransportOptions options)
13+
public void Configure(HttpServerTransportOptions options)
1414
{
1515
options.EventStreamStore ??= serviceProvider.GetService<ISseEventStreamStore>();
1616
options.SessionMigrationHandler ??= serviceProvider.GetService<ISessionMigrationHandler>();

src/ModelContextProtocol/McpServerOptionsPostSetup.cs

Lines changed: 0 additions & 17 deletions
This file was deleted.

src/ModelContextProtocol/McpServerOptionsSetup.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
1+
using Microsoft.Extensions.DependencyInjection;
12
using Microsoft.Extensions.Options;
23
using ModelContextProtocol.Server;
34

45
namespace ModelContextProtocol;
56

67
/// <summary>
7-
/// Configures the McpServerOptions using addition services from DI.
8+
/// Configures the McpServerOptions using additional services from DI.
89
/// </summary>
10+
/// <param name="serviceProvider">The service provider for resolving optional services.</param>
911
/// <param name="serverTools">The individually registered tools.</param>
1012
/// <param name="serverPrompts">The individually registered prompts.</param>
1113
/// <param name="serverResources">The individually registered resources.</param>
1214
internal sealed class McpServerOptionsSetup(
15+
IServiceProvider serviceProvider,
1316
IEnumerable<McpServerTool> serverTools,
1417
IEnumerable<McpServerPrompt> serverPrompts,
1518
IEnumerable<McpServerResource> serverResources) : IConfigureOptions<McpServerOptions>
@@ -23,6 +26,8 @@ public void Configure(McpServerOptions options)
2326
{
2427
Throw.IfNull(options);
2528

29+
options.TaskStore ??= serviceProvider.GetService<IMcpTaskStore>();
30+
2631
// Collect all of the provided tools into a tools collection. If the options already has
2732
// a collection, add to it, otherwise create a new one. We want to maintain the identity
2833
// of an existing collection in case someone has provided their own derived type, wants

src/ModelContextProtocol/McpServerServiceCollectionExtensions.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ public static IMcpServerBuilder AddMcpServer(this IServiceCollection services, A
2121
{
2222
services.AddOptions();
2323
services.TryAddEnumerable(ServiceDescriptor.Transient<IConfigureOptions<McpServerOptions>, McpServerOptionsSetup>());
24-
services.TryAddEnumerable(ServiceDescriptor.Transient<IPostConfigureOptions<McpServerOptions>, McpServerOptionsPostSetup>());
2524
if (configureOptions is not null)
2625
{
2726
services.Configure(configureOptions);

tests/ModelContextProtocol.AspNetCore.Tests/HttpMcpServerBuilderExtensionsTests.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,23 @@ public void EventStreamStore_RemainsNull_WhenNothingIsRegistered()
8888
Assert.Null(options.EventStreamStore);
8989
}
9090

91+
[Fact]
92+
public void EventStreamStore_CanBeOverriddenToNull_AfterDIRegistration()
93+
{
94+
Builder.Services.AddDistributedMemoryCache();
95+
Builder.Services
96+
.AddMcpServer()
97+
.WithHttpTransport()
98+
.WithDistributedCacheEventStreamStore();
99+
100+
Builder.Services.Configure<HttpServerTransportOptions>(options => options.EventStreamStore = null);
101+
102+
using var app = Builder.Build();
103+
104+
var options = app.Services.GetRequiredService<IOptions<HttpServerTransportOptions>>().Value;
105+
Assert.Null(options.EventStreamStore);
106+
}
107+
91108
[Fact]
92109
public void SessionMigrationHandler_IsPopulatedFromDI_ViaPostConfigure()
93110
{

tests/ModelContextProtocol.Tests/Configuration/McpServerOptionsPostSetupTests.cs

Lines changed: 0 additions & 45 deletions
This file was deleted.

tests/ModelContextProtocol.Tests/Configuration/McpServerOptionsSetupTests.cs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,4 +283,57 @@ public void Configure_WithCompleteHandler_CreatesCompletionsCapability()
283283
Assert.NotNull(options.Capabilities?.Completions);
284284
}
285285
#endregion
286+
287+
#region TaskStore Tests
288+
[Fact]
289+
public void TaskStore_IsPopulatedFromDI_WhenNotExplicitlySet()
290+
{
291+
var services = new ServiceCollection();
292+
services.AddMcpServer();
293+
services.AddSingleton<IMcpTaskStore, InMemoryMcpTaskStore>();
294+
295+
var options = services.BuildServiceProvider().GetRequiredService<IOptions<McpServerOptions>>().Value;
296+
297+
Assert.IsType<InMemoryMcpTaskStore>(options.TaskStore);
298+
}
299+
300+
[Fact]
301+
public void TaskStore_ExplicitOption_TakesPrecedenceOverDI()
302+
{
303+
var explicitStore = new InMemoryMcpTaskStore();
304+
305+
var services = new ServiceCollection();
306+
services.AddMcpServer(options => options.TaskStore = explicitStore);
307+
services.AddSingleton<IMcpTaskStore, InMemoryMcpTaskStore>();
308+
309+
var options = services.BuildServiceProvider().GetRequiredService<IOptions<McpServerOptions>>().Value;
310+
311+
Assert.Same(explicitStore, options.TaskStore);
312+
}
313+
314+
[Fact]
315+
public void TaskStore_RemainsNull_WhenNothingIsRegistered()
316+
{
317+
var services = new ServiceCollection();
318+
services.AddMcpServer();
319+
320+
var options = services.BuildServiceProvider().GetRequiredService<IOptions<McpServerOptions>>().Value;
321+
322+
Assert.Null(options.TaskStore);
323+
}
324+
325+
[Fact]
326+
public void TaskStore_CanBeOverriddenToNull_AfterDIRegistration()
327+
{
328+
var services = new ServiceCollection();
329+
services.AddMcpServer();
330+
services.AddSingleton<IMcpTaskStore, InMemoryMcpTaskStore>();
331+
332+
services.Configure<McpServerOptions>(options => options.TaskStore = null);
333+
334+
var options = services.BuildServiceProvider().GetRequiredService<IOptions<McpServerOptions>>().Value;
335+
336+
Assert.Null(options.TaskStore);
337+
}
338+
#endregion
286339
}

0 commit comments

Comments
 (0)