Skip to content

Commit c1976f9

Browse files
codeboyzhoucodeboyzhou
committed
refactor: use builder pattern for CallToolResult and simplify lambda expressions in tests
1 parent 6e9af40 commit c1976f9

File tree

3 files changed

+85
-55
lines changed

3 files changed

+85
-55
lines changed

‎mcp-core/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -851,7 +851,7 @@ void testThrowingToolCallIsCaughtBeforeTimeout(String clientType) {
851851

852852
@ParameterizedTest(name = "{0} : {displayName} ")
853853
@MethodSource("clientsForTesting")
854-
void testToolCallSuccessWithTranportContextExtraction(String clientType) {
854+
void testToolCallSuccessWithTransportContextExtraction(String clientType) {
855855

856856
var clientBuilder = clientBuilders.get(clientType);
857857

‎mcp-core/src/test/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java‎

Lines changed: 71 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -78,14 +78,14 @@ void testConstructorWithInvalidArguments() {
7878
void testGracefulShutdown() {
7979
var mcpSyncServer = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0").build();
8080

81-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
81+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
8282
}
8383

8484
@Test
8585
void testImmediateClose() {
8686
var mcpSyncServer = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0").build();
8787

88-
assertThatCode(() -> mcpSyncServer.close()).doesNotThrowAnyException();
88+
assertThatCode(mcpSyncServer::close).doesNotThrowAnyException();
8989
}
9090

9191
@Test
@@ -94,7 +94,7 @@ void testGetAsyncServer() {
9494

9595
assertThat(mcpSyncServer.getAsyncServer()).isNotNull();
9696

97-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
97+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
9898
}
9999

100100
// ---------------------------------------
@@ -117,7 +117,7 @@ void testAddTool() {
117117
(exchange, args) -> new CallToolResult(List.of(), false))))
118118
.doesNotThrowAnyException();
119119

120-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
120+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
121121
}
122122

123123
@Test
@@ -134,10 +134,10 @@ void testAddToolCall() {
134134

135135
assertThatCode(() -> mcpSyncServer.addTool(McpServerFeatures.SyncToolSpecification.builder()
136136
.tool(newTool)
137-
.callHandler((exchange, request) -> new CallToolResult(List.of(), false))
137+
.callHandler((exchange, request) -> CallToolResult.builder().content(List.of()).isError(false).build())
138138
.build())).doesNotThrowAnyException();
139139

140-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
140+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
141141
}
142142

143143
@Test
@@ -158,7 +158,7 @@ void testAddDuplicateTool() {
158158
(exchange, args) -> new CallToolResult(List.of(), false))))
159159
.doesNotThrowAnyException();
160160

161-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
161+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
162162
}
163163

164164
@Test
@@ -171,15 +171,16 @@ void testAddDuplicateToolCall() {
171171

172172
var mcpSyncServer = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0")
173173
.capabilities(ServerCapabilities.builder().tools(true).build())
174-
.toolCall(duplicateTool, (exchange, request) -> new CallToolResult(List.of(), false))
174+
.toolCall(duplicateTool,
175+
(exchange, request) -> CallToolResult.builder().content(List.of()).isError(false).build())
175176
.build();
176177

177178
assertThatCode(() -> mcpSyncServer.addTool(McpServerFeatures.SyncToolSpecification.builder()
178179
.tool(duplicateTool)
179-
.callHandler((exchange, request) -> new CallToolResult(List.of(), false))
180+
.callHandler((exchange, request) -> CallToolResult.builder().content(List.of()).isError(false).build())
180181
.build())).doesNotThrowAnyException();
181182

182-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
183+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
183184
}
184185

185186
@Test
@@ -192,8 +193,10 @@ void testDuplicateToolCallDuringBuilding() {
192193

193194
assertThatThrownBy(() -> prepareSyncServerBuilder().serverInfo("test-server", "1.0.0")
194195
.capabilities(ServerCapabilities.builder().tools(true).build())
195-
.toolCall(duplicateTool, (exchange, request) -> new CallToolResult(List.of(), false))
196-
.toolCall(duplicateTool, (exchange, request) -> new CallToolResult(List.of(), false)) // Duplicate!
196+
.toolCall(duplicateTool,
197+
(exchange, request) -> CallToolResult.builder().content(List.of()).isError(false).build())
198+
.toolCall(duplicateTool,
199+
(exchange, request) -> CallToolResult.builder().content(List.of()).isError(false).build()) // Duplicate!
197200
.build()).isInstanceOf(IllegalArgumentException.class)
198201
.hasMessage("Tool with name 'duplicate-build-toolcall' is already registered.");
199202
}
@@ -208,11 +211,13 @@ void testDuplicateToolsInBatchListRegistration() {
208211
List<McpServerFeatures.SyncToolSpecification> specs = List.of(
209212
McpServerFeatures.SyncToolSpecification.builder()
210213
.tool(duplicateTool)
211-
.callHandler((exchange, request) -> new CallToolResult(List.of(), false))
214+
.callHandler(
215+
(exchange, request) -> CallToolResult.builder().content(List.of()).isError(false).build())
212216
.build(),
213217
McpServerFeatures.SyncToolSpecification.builder()
214218
.tool(duplicateTool)
215-
.callHandler((exchange, request) -> new CallToolResult(List.of(), false))
219+
.callHandler(
220+
(exchange, request) -> CallToolResult.builder().content(List.of()).isError(false).build())
216221
.build() // Duplicate!
217222
);
218223

@@ -235,11 +240,12 @@ void testDuplicateToolsInBatchVarargsRegistration() {
235240
.capabilities(ServerCapabilities.builder().tools(true).build())
236241
.tools(McpServerFeatures.SyncToolSpecification.builder()
237242
.tool(duplicateTool)
238-
.callHandler((exchange, request) -> new CallToolResult(List.of(), false))
243+
.callHandler((exchange, request) -> CallToolResult.builder().content(List.of()).isError(false).build())
239244
.build(),
240245
McpServerFeatures.SyncToolSpecification.builder()
241246
.tool(duplicateTool)
242-
.callHandler((exchange, request) -> new CallToolResult(List.of(), false))
247+
.callHandler((exchange,
248+
request) -> CallToolResult.builder().content(List.of()).isError(false).build())
243249
.build() // Duplicate!
244250
)
245251
.build()).isInstanceOf(IllegalArgumentException.class)
@@ -256,12 +262,12 @@ void testRemoveTool() {
256262

257263
var mcpSyncServer = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0")
258264
.capabilities(ServerCapabilities.builder().tools(true).build())
259-
.toolCall(tool, (exchange, args) -> new CallToolResult(List.of(), false))
265+
.toolCall(tool, (exchange, args) -> CallToolResult.builder().content(List.of()).isError(false).build())
260266
.build();
261267

262268
assertThatCode(() -> mcpSyncServer.removeTool(TEST_TOOL_NAME)).doesNotThrowAnyException();
263269

264-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
270+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
265271
}
266272

267273
@Test
@@ -272,16 +278,16 @@ void testRemoveNonexistentTool() {
272278

273279
assertThatCode(() -> mcpSyncServer.removeTool("nonexistent-tool")).doesNotThrowAnyException();
274280

275-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
281+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
276282
}
277283

278284
@Test
279285
void testNotifyToolsListChanged() {
280286
var mcpSyncServer = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0").build();
281287

282-
assertThatCode(() -> mcpSyncServer.notifyToolsListChanged()).doesNotThrowAnyException();
288+
assertThatCode(mcpSyncServer::notifyToolsListChanged).doesNotThrowAnyException();
283289

284-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
290+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
285291
}
286292

287293
// ---------------------------------------
@@ -292,9 +298,9 @@ void testNotifyToolsListChanged() {
292298
void testNotifyResourcesListChanged() {
293299
var mcpSyncServer = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0").build();
294300

295-
assertThatCode(() -> mcpSyncServer.notifyResourcesListChanged()).doesNotThrowAnyException();
301+
assertThatCode(mcpSyncServer::notifyResourcesListChanged).doesNotThrowAnyException();
296302

297-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
303+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
298304
}
299305

300306
@Test
@@ -305,7 +311,7 @@ void testNotifyResourcesUpdated() {
305311
.notifyResourcesUpdated(new McpSchema.ResourcesUpdatedNotification(TEST_RESOURCE_URI)))
306312
.doesNotThrowAnyException();
307313

308-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
314+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
309315
}
310316

311317
@Test
@@ -314,14 +320,18 @@ void testAddResource() {
314320
.capabilities(ServerCapabilities.builder().resources(true, false).build())
315321
.build();
316322

317-
Resource resource = new Resource(TEST_RESOURCE_URI, "Test Resource", "text/plain", "Test resource description",
318-
null);
323+
Resource resource = Resource.builder()
324+
.uri(TEST_RESOURCE_URI)
325+
.name("Test Resource")
326+
.mimeType("text/plain")
327+
.description("Test resource description")
328+
.build();
319329
McpServerFeatures.SyncResourceSpecification specification = new McpServerFeatures.SyncResourceSpecification(
320330
resource, (exchange, req) -> new ReadResourceResult(List.of()));
321331

322332
assertThatCode(() -> mcpSyncServer.addResource(specification)).doesNotThrowAnyException();
323333

324-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
334+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
325335
}
326336

327337
@Test
@@ -334,15 +344,19 @@ void testAddResourceWithNullSpecification() {
334344
.isInstanceOf(IllegalArgumentException.class)
335345
.hasMessage("Resource must not be null");
336346

337-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
347+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
338348
}
339349

340350
@Test
341351
void testAddResourceWithoutCapability() {
342352
var serverWithoutResources = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0").build();
343353

344-
Resource resource = new Resource(TEST_RESOURCE_URI, "Test Resource", "text/plain", "Test resource description",
345-
null);
354+
Resource resource = Resource.builder()
355+
.uri(TEST_RESOURCE_URI)
356+
.name("Test Resource")
357+
.mimeType("text/plain")
358+
.description("Test resource description")
359+
.build();
346360
McpServerFeatures.SyncResourceSpecification specification = new McpServerFeatures.SyncResourceSpecification(
347361
resource, (exchange, req) -> new ReadResourceResult(List.of()));
348362

@@ -366,8 +380,12 @@ void testListResources() {
366380
.capabilities(ServerCapabilities.builder().resources(true, false).build())
367381
.build();
368382

369-
Resource resource = new Resource(TEST_RESOURCE_URI, "Test Resource", "text/plain", "Test resource description",
370-
null);
383+
Resource resource = Resource.builder()
384+
.uri(TEST_RESOURCE_URI)
385+
.name("Test Resource")
386+
.mimeType("text/plain")
387+
.description("Test resource description")
388+
.build();
371389
McpServerFeatures.SyncResourceSpecification specification = new McpServerFeatures.SyncResourceSpecification(
372390
resource, (exchange, req) -> new ReadResourceResult(List.of()));
373391

@@ -377,7 +395,7 @@ void testListResources() {
377395
assertThat(resources).hasSize(1);
378396
assertThat(resources.get(0).uri()).isEqualTo(TEST_RESOURCE_URI);
379397

380-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
398+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
381399
}
382400

383401
@Test
@@ -386,15 +404,19 @@ void testRemoveResource() {
386404
.capabilities(ServerCapabilities.builder().resources(true, false).build())
387405
.build();
388406

389-
Resource resource = new Resource(TEST_RESOURCE_URI, "Test Resource", "text/plain", "Test resource description",
390-
null);
407+
Resource resource = Resource.builder()
408+
.uri(TEST_RESOURCE_URI)
409+
.name("Test Resource")
410+
.mimeType("text/plain")
411+
.description("Test resource description")
412+
.build();
391413
McpServerFeatures.SyncResourceSpecification specification = new McpServerFeatures.SyncResourceSpecification(
392414
resource, (exchange, req) -> new ReadResourceResult(List.of()));
393415

394416
mcpSyncServer.addResource(specification);
395417
assertThatCode(() -> mcpSyncServer.removeResource(TEST_RESOURCE_URI)).doesNotThrowAnyException();
396418

397-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
419+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
398420
}
399421

400422
@Test
@@ -407,7 +429,7 @@ void testRemoveNonexistentResource() {
407429
// as per the new implementation that just logs a warning
408430
assertThatCode(() -> mcpSyncServer.removeResource("nonexistent://resource")).doesNotThrowAnyException();
409431

410-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
432+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
411433
}
412434

413435
// ---------------------------------------
@@ -432,7 +454,7 @@ void testAddResourceTemplate() {
432454

433455
assertThatCode(() -> mcpSyncServer.addResourceTemplate(specification)).doesNotThrowAnyException();
434456

435-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
457+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
436458
}
437459

438460
@Test
@@ -474,7 +496,7 @@ void testRemoveResourceTemplate() {
474496

475497
assertThatCode(() -> mcpSyncServer.removeResourceTemplate("test://template/{id}")).doesNotThrowAnyException();
476498

477-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
499+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
478500
}
479501

480502
@Test
@@ -496,7 +518,7 @@ void testRemoveNonexistentResourceTemplate() {
496518
assertThatCode(() -> mcpSyncServer.removeResourceTemplate("nonexistent://template/{id}"))
497519
.doesNotThrowAnyException();
498520

499-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
521+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
500522
}
501523

502524
@Test
@@ -520,7 +542,7 @@ void testListResourceTemplates() {
520542

521543
assertThat(templates).isNotNull();
522544

523-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
545+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
524546
}
525547

526548
// ---------------------------------------
@@ -531,9 +553,9 @@ void testListResourceTemplates() {
531553
void testNotifyPromptsListChanged() {
532554
var mcpSyncServer = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0").build();
533555

534-
assertThatCode(() -> mcpSyncServer.notifyPromptsListChanged()).doesNotThrowAnyException();
556+
assertThatCode(mcpSyncServer::notifyPromptsListChanged).doesNotThrowAnyException();
535557

536-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
558+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
537559
}
538560

539561
@Test
@@ -584,7 +606,7 @@ void testRemovePrompt() {
584606

585607
assertThatCode(() -> mcpSyncServer.removePrompt(TEST_PROMPT_NAME)).doesNotThrowAnyException();
586608

587-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
609+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
588610
}
589611

590612
@Test
@@ -595,7 +617,7 @@ void testRemoveNonexistentPrompt() {
595617

596618
assertThatCode(() -> mcpSyncServer.removePrompt("nonexistent://template/{id}")).doesNotThrowAnyException();
597619

598-
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
620+
assertThatCode(mcpSyncServer::closeGracefully).doesNotThrowAnyException();
599621
}
600622

601623
// ---------------------------------------
@@ -617,7 +639,7 @@ void testRootsChangeHandlers() {
617639
}))
618640
.build();
619641
assertThat(singleConsumerServer).isNotNull();
620-
assertThatCode(() -> singleConsumerServer.closeGracefully()).doesNotThrowAnyException();
642+
assertThatCode(singleConsumerServer::closeGracefully).doesNotThrowAnyException();
621643
onClose();
622644

623645
// Test with multiple consumers
@@ -633,7 +655,7 @@ void testRootsChangeHandlers() {
633655
.build();
634656

635657
assertThat(multipleConsumersServer).isNotNull();
636-
assertThatCode(() -> multipleConsumersServer.closeGracefully()).doesNotThrowAnyException();
658+
assertThatCode(multipleConsumersServer::closeGracefully).doesNotThrowAnyException();
637659
onClose();
638660

639661
// Test error handling
@@ -644,14 +666,14 @@ void testRootsChangeHandlers() {
644666
.build();
645667

646668
assertThat(errorHandlingServer).isNotNull();
647-
assertThatCode(() -> errorHandlingServer.closeGracefully()).doesNotThrowAnyException();
669+
assertThatCode(errorHandlingServer::closeGracefully).doesNotThrowAnyException();
648670
onClose();
649671

650672
// Test without consumers
651673
var noConsumersServer = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0").build();
652674

653675
assertThat(noConsumersServer).isNotNull();
654-
assertThatCode(() -> noConsumersServer.closeGracefully()).doesNotThrowAnyException();
676+
assertThatCode(noConsumersServer::closeGracefully).doesNotThrowAnyException();
655677
}
656678

657679
}

0 commit comments

Comments
 (0)