Skip to content

Commit 95b775b

Browse files
authored
fix(core): Fix NPE in Toolkit.getTool when tool name is null (agentscope-ai#982)
## AgentScope-Java Version 1.0.11 ## Description * Closes: agentscope-ai#973 * Fix NPE in Toolkit.getTool when tool name is null ## Checklist Please check the following items before code is ready to be reviewed. - [x] Code has been formatted with `mvn spotless:apply` - [x] All tests are passing (`mvn test`) - [x] Javadoc comments are complete and follow project conventions - [x] Related documentation has been updated (e.g. links, examples, etc.) - [x] Code is ready for review
1 parent 38fbbe3 commit 95b775b

2 files changed

Lines changed: 74 additions & 0 deletions

File tree

agentscope-core/src/main/java/io/agentscope/core/tool/ToolRegistry.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ class ToolRegistry {
5050
* @param registered RegisteredToolFunction wrapper with metadata
5151
*/
5252
void registerTool(String toolName, AgentTool tool, RegisteredToolFunction registered) {
53+
if (toolName == null || toolName.isBlank()) {
54+
throw new IllegalArgumentException("Tool name cannot be null or blank");
55+
}
5356
tools.put(toolName, tool);
5457
registeredTools.put(toolName, registered);
5558
}
@@ -61,6 +64,9 @@ void registerTool(String toolName, AgentTool tool, RegisteredToolFunction regist
6164
* @return AgentTool or null if not found
6265
*/
6366
AgentTool getTool(String name) {
67+
if (name == null || name.isBlank()) {
68+
return null;
69+
}
6470
return tools.get(name);
6571
}
6672

@@ -71,6 +77,9 @@ AgentTool getTool(String name) {
7177
* @return RegisteredToolFunction or null if not found
7278
*/
7379
RegisteredToolFunction getRegisteredTool(String name) {
80+
if (name == null || name.isBlank()) {
81+
return null;
82+
}
7483
return registeredTools.get(name);
7584
}
7685

@@ -98,6 +107,9 @@ Map<String, RegisteredToolFunction> getAllRegisteredTools() {
98107
* @param toolName Tool name to remove
99108
*/
100109
void removeTool(String toolName) {
110+
if (toolName == null || toolName.isBlank()) {
111+
throw new IllegalArgumentException("Tool name cannot be null or blank");
112+
}
101113
tools.remove(toolName);
102114
registeredTools.remove(toolName);
103115
}

agentscope-core/src/test/java/io/agentscope/core/tool/ToolRegistryTest.java

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,15 @@
1919
import static org.junit.jupiter.api.Assertions.assertEquals;
2020
import static org.junit.jupiter.api.Assertions.assertNotSame;
2121
import static org.junit.jupiter.api.Assertions.assertNull;
22+
import static org.junit.jupiter.api.Assertions.assertThrows;
2223
import static org.junit.jupiter.api.Assertions.assertTrue;
2324

2425
import io.agentscope.core.message.ToolResultBlock;
2526
import java.util.HashMap;
2627
import java.util.Map;
2728
import java.util.Set;
2829
import org.junit.jupiter.api.BeforeEach;
30+
import org.junit.jupiter.api.DisplayName;
2931
import org.junit.jupiter.api.Test;
3032
import reactor.core.publisher.Mono;
3133

@@ -297,4 +299,64 @@ void testConcurrentAccess() {
297299
// Assert
298300
assertEquals(200, registry.getToolNames().size());
299301
}
302+
303+
@Test
304+
@DisplayName("Should throw IllegalArgumentException register null name tool")
305+
void testRegisterNullNameTool() {
306+
assertThrows(
307+
IllegalArgumentException.class,
308+
() -> registry.registerTool(null, mockTool1, registered1));
309+
}
310+
311+
@Test
312+
@DisplayName("Should throw IllegalArgumentException register empty name tool")
313+
void testRegisterEmptyNameTool() {
314+
assertThrows(
315+
IllegalArgumentException.class,
316+
() -> registry.registerTool("", mockTool1, registered1));
317+
}
318+
319+
@Test
320+
@DisplayName("Should throw IllegalArgumentException register blank name tool")
321+
void testRegisterBlankNameTool() {
322+
assertThrows(
323+
IllegalArgumentException.class,
324+
() -> registry.registerTool(" ", mockTool1, registered1));
325+
}
326+
327+
@Test
328+
@DisplayName("Should throw IllegalArgumentException remove null name tool")
329+
void testRemoveNullNameTool() {
330+
assertThrows(IllegalArgumentException.class, () -> registry.removeTool(null));
331+
}
332+
333+
@Test
334+
@DisplayName("Should throw IllegalArgumentException remove empty name tool")
335+
void testRemoveEmptyNameTool() {
336+
assertThrows(IllegalArgumentException.class, () -> registry.removeTool(""));
337+
}
338+
339+
@Test
340+
@DisplayName("Should throw IllegalArgumentException remove blank name tool")
341+
void testRemoveBlankNameTool() {
342+
assertThrows(IllegalArgumentException.class, () -> registry.removeTool(" "));
343+
}
344+
345+
@Test
346+
@DisplayName("Should return null get null name tool")
347+
void testGetNullNameTool() {
348+
assertNull(registry.getTool(null));
349+
}
350+
351+
@Test
352+
@DisplayName("Should return null get empty name tool")
353+
void testGetEmptyNameTool() {
354+
assertNull(registry.getTool(""));
355+
}
356+
357+
@Test
358+
@DisplayName("Should return null get blank name tool")
359+
void testGetBlankNameTool() {
360+
assertNull(registry.getTool(" "));
361+
}
300362
}

0 commit comments

Comments
 (0)