Skip to content

Commit 423fe2d

Browse files
Address PR feedback: convert if-else to switch and add default value mapping tests
Co-authored-by: eiriktsarpalis <[email protected]>
1 parent c6244fb commit 423fe2d

File tree

3 files changed

+157
-13
lines changed

3 files changed

+157
-13
lines changed

src/ModelContextProtocol.Core/Protocol/ElicitRequestParams.cs

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -162,17 +162,18 @@ public class Converter : JsonConverter<PrimitiveSchemaDefinition>
162162
case "default":
163163
// We need to handle different types for default values
164164
// Store the value based on the JSON token type
165-
if (reader.TokenType == JsonTokenType.True || reader.TokenType == JsonTokenType.False)
165+
switch (reader.TokenType)
166166
{
167-
defaultBool = reader.GetBoolean();
168-
}
169-
else if (reader.TokenType == JsonTokenType.Number)
170-
{
171-
defaultNumber = reader.GetDouble();
172-
}
173-
else if (reader.TokenType == JsonTokenType.String)
174-
{
175-
defaultString = reader.GetString();
167+
case JsonTokenType.True:
168+
case JsonTokenType.False:
169+
defaultBool = reader.GetBoolean();
170+
break;
171+
case JsonTokenType.Number:
172+
defaultNumber = reader.GetDouble();
173+
break;
174+
case JsonTokenType.String:
175+
defaultString = reader.GetString();
176+
break;
176177
}
177178
break;
178179

src/ModelContextProtocol.Core/Server/McpServer.Methods.cs

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
using Microsoft.Extensions.Logging;
33
using ModelContextProtocol.Protocol;
44
using System.Collections.Concurrent;
5+
using System.ComponentModel;
56
using System.Diagnostics.CodeAnalysis;
7+
using System.Reflection;
68
using System.Runtime.CompilerServices;
79
using System.Text;
810
using System.Text.Json;
@@ -257,9 +259,9 @@ public async ValueTask<ElicitResult<T>> ElicitAsync<T>(
257259
var dict = s_elicitResultSchemaCache.GetValue(serializerOptions, _ => new());
258260

259261
#if NET
260-
var schema = dict.GetOrAdd(typeof(T), static (t, s) => BuildRequestSchema(t, s), serializerOptions);
262+
var schema = dict.GetOrAdd(typeof(T), static (t, s) => BuildRequestSchemaHelper(t, s), serializerOptions);
261263
#else
262-
var schema = dict.GetOrAdd(typeof(T), type => BuildRequestSchema(type, serializerOptions));
264+
var schema = dict.GetOrAdd(typeof(T), type => BuildRequestSchemaHelper(type, serializerOptions));
263265
#endif
264266

265267
var request = new ElicitRequestParams
@@ -285,14 +287,21 @@ public async ValueTask<ElicitResult<T>> ElicitAsync<T>(
285287
return new ElicitResult<T> { Action = raw.Action, Content = typed };
286288
}
287289

290+
/// <summary>
291+
/// Helper method for BuildRequestSchema that can be called from lambdas without annotation issues.
292+
/// </summary>
293+
[UnconditionalSuppressMessage("Trimming", "IL2067", Justification = "Type T is preserved via JsonTypeInfo")]
294+
private static ElicitRequestParams.RequestSchema BuildRequestSchemaHelper(Type type, JsonSerializerOptions serializerOptions)
295+
=> BuildRequestSchema(type, serializerOptions);
296+
288297
/// <summary>
289298
/// Builds a request schema for elicitation based on the public serializable properties of <paramref name="type"/>.
290299
/// </summary>
291300
/// <param name="type">The type of the schema being built.</param>
292301
/// <param name="serializerOptions">The serializer options to use.</param>
293302
/// <returns>The built request schema.</returns>
294303
/// <exception cref="McpProtocolException"></exception>
295-
private static ElicitRequestParams.RequestSchema BuildRequestSchema(Type type, JsonSerializerOptions serializerOptions)
304+
private static ElicitRequestParams.RequestSchema BuildRequestSchema([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] Type type, JsonSerializerOptions serializerOptions)
296305
{
297306
var schema = new ElicitRequestParams.RequestSchema();
298307
var props = schema.Properties;
@@ -307,11 +316,47 @@ private static ElicitRequestParams.RequestSchema BuildRequestSchema(Type type, J
307316
foreach (JsonPropertyInfo pi in typeInfo.Properties)
308317
{
309318
var def = CreatePrimitiveSchema(pi.PropertyType, serializerOptions);
319+
320+
// Extract default value from DefaultValueAttribute if present
321+
var propInfo = type.GetProperty(pi.Name);
322+
if (propInfo != null)
323+
{
324+
var defaultValueAttr = propInfo.GetCustomAttribute<System.ComponentModel.DefaultValueAttribute>();
325+
if (defaultValueAttr?.Value != null)
326+
{
327+
SetDefaultValue(def, defaultValueAttr.Value);
328+
}
329+
}
330+
310331
props[pi.Name] = def;
311332
}
312333

313334
return schema;
314335
}
336+
337+
/// <summary>
338+
/// Sets the default value on a primitive schema definition based on the value type.
339+
/// </summary>
340+
/// <param name="schema">The schema to set the default value on.</param>
341+
/// <param name="value">The default value.</param>
342+
private static void SetDefaultValue(ElicitRequestParams.PrimitiveSchemaDefinition schema, object value)
343+
{
344+
switch (schema)
345+
{
346+
case ElicitRequestParams.StringSchema stringSchema:
347+
stringSchema.Default = value?.ToString();
348+
break;
349+
case ElicitRequestParams.NumberSchema numberSchema:
350+
numberSchema.Default = Convert.ToDouble(value);
351+
break;
352+
case ElicitRequestParams.BooleanSchema booleanSchema:
353+
booleanSchema.Default = Convert.ToBoolean(value);
354+
break;
355+
case ElicitRequestParams.EnumSchema enumSchema:
356+
enumSchema.Default = value?.ToString();
357+
break;
358+
}
359+
}
315360

316361
/// <summary>
317362
/// Creates a primitive schema definition for the specified type, if supported.

tests/ModelContextProtocol.Tests/Protocol/ElicitationTypedTests.cs

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,19 @@ await request.Server.ElicitAsync<string>(
8686
Content = [new TextContentBlock { Text = "unexpected" }],
8787
};
8888
}
89+
else if (request.Params!.Name == "TestElicitationWithDefaults")
90+
{
91+
var result = await request.Server.ElicitAsync<FormWithDefaults>(
92+
message: "Please provide information.",
93+
serializerOptions: ElicitationDefaultsJsonContext.Default.Options,
94+
cancellationToken: CancellationToken.None);
95+
96+
// The test will validate the schema in the client handler
97+
return new CallToolResult
98+
{
99+
Content = [new TextContentBlock { Text = "success" }],
100+
};
101+
}
89102
else
90103
{
91104
Assert.Fail($"Unexpected tool name: {request.Params!.Name}");
@@ -360,4 +373,89 @@ public sealed class Nested
360373
[JsonSerializable(typeof(UnsupportedForm.Nested))]
361374
[JsonSerializable(typeof(JsonElement))]
362375
internal partial class ElicitationUnsupportedJsonContext : JsonSerializerContext;
376+
377+
public sealed class FormWithDefaults
378+
{
379+
[System.ComponentModel.DefaultValue("John Doe")]
380+
public string Name { get; set; } = "John Doe";
381+
382+
[System.ComponentModel.DefaultValue(30)]
383+
public int Age { get; set; } = 30;
384+
385+
[System.ComponentModel.DefaultValue(85.5)]
386+
public double Score { get; set; } = 85.5;
387+
388+
[System.ComponentModel.DefaultValue(true)]
389+
public bool IsActive { get; set; } = true;
390+
391+
[System.ComponentModel.DefaultValue("active")]
392+
public string Status { get; set; } = "active";
393+
}
394+
395+
[JsonSerializable(typeof(FormWithDefaults))]
396+
[JsonSerializable(typeof(JsonElement))]
397+
internal partial class ElicitationDefaultsJsonContext : JsonSerializerContext;
398+
399+
[Fact]
400+
public async Task Elicit_Typed_With_Defaults_Maps_To_Schema_Defaults()
401+
{
402+
await using McpClient client = await CreateMcpClientForServer(new McpClientOptions
403+
{
404+
Handlers = new()
405+
{
406+
ElicitationHandler = async (request, cancellationToken) =>
407+
{
408+
Assert.NotNull(request);
409+
Assert.Equal("Please provide information.", request.Message);
410+
411+
Assert.Equal(5, request.RequestedSchema.Properties.Count);
412+
413+
// Verify that default values from the type are mapped to the schema
414+
foreach (var entry in request.RequestedSchema.Properties)
415+
{
416+
switch (entry.Key)
417+
{
418+
case nameof(FormWithDefaults.Name):
419+
var nameSchema = Assert.IsType<ElicitRequestParams.StringSchema>(entry.Value);
420+
Assert.Equal("John Doe", nameSchema.Default);
421+
break;
422+
423+
case nameof(FormWithDefaults.Age):
424+
var ageSchema = Assert.IsType<ElicitRequestParams.NumberSchema>(entry.Value);
425+
Assert.Equal(30, ageSchema.Default);
426+
break;
427+
428+
case nameof(FormWithDefaults.Score):
429+
var scoreSchema = Assert.IsType<ElicitRequestParams.NumberSchema>(entry.Value);
430+
Assert.Equal(85.5, scoreSchema.Default);
431+
break;
432+
433+
case nameof(FormWithDefaults.IsActive):
434+
var activeSchema = Assert.IsType<ElicitRequestParams.BooleanSchema>(entry.Value);
435+
Assert.True(activeSchema.Default);
436+
break;
437+
438+
case nameof(FormWithDefaults.Status):
439+
var statusSchema = Assert.IsType<ElicitRequestParams.StringSchema>(entry.Value);
440+
Assert.Equal("active", statusSchema.Default);
441+
break;
442+
443+
default:
444+
Assert.Fail($"Unexpected property: {entry.Key}");
445+
break;
446+
}
447+
}
448+
449+
return new ElicitResult
450+
{
451+
Action = "accept",
452+
Content = new Dictionary<string, JsonElement>()
453+
};
454+
},
455+
}
456+
});
457+
458+
var result = await client.CallToolAsync("TestElicitationWithDefaults", cancellationToken: TestContext.Current.CancellationToken);
459+
Assert.Equal("success", (result.Content[0] as TextContentBlock)?.Text);
460+
}
363461
}

0 commit comments

Comments
 (0)