Rename illigal args to missing args, rename source set for consistency, review comments

Author: ttmunyunguma

dev/io.openliberty.mcp.internal/src/io/openliberty/mcp/internal/McpCdiExtension.java

@@ -63,10 +63,10 @@ void afterDeploymentValidation(@Observes AfterDeploymentValidation afterDeployme
     private void reportOnToolArgEdgeCases(AfterDeploymentValidation afterDeploymentValidation) {
         StringBuilder sbBlankArgs = new StringBuilder("Blank arguments found in MCP Tool:");
         StringBuilder sbDuplicateArgs = new StringBuilder("Duplicate arguments found in MCP Tool:");
-        StringBuilder sbIllegalArgs = new StringBuilder("Illegal arguments found in MCP Tool:");
+        StringBuilder sbMissingArgs = new StringBuilder("Missing arguments found in MCP Tool:");
         boolean blankArgumentsFound = false;
         boolean duplicateArgumentsFound = false;
-        boolean illegalArgumentsFound = false;
+        boolean missingArgumentName = false;
 
         for (ToolMetadata tool : tools.getAllTools()) {
             Map<String, ArgumentMetadata> arguments = tool.arguments();
@@ -78,9 +78,10 @@ private void reportOnToolArgEdgeCases(AfterDeploymentValidation afterDeploymentV
                 } else if (arguments.get(argName).isDuplicate()) {
                     sbDuplicateArgs.append("\n").append("Tool: " + tool.getToolQualifiedName() + " -  Argument: " + argName);
                     duplicateArgumentsFound = true;
-                } else if (argName.equals(ToolMetadata.ILLEGAL_TOOL_ARG_NAME)) {
-                    sbIllegalArgs.append("\n").append("Tool: " + tool.getToolQualifiedName());
-                    illegalArgumentsFound = true;
+                } else if (argName.equals(ToolMetadata.MISSING_TOOL_ARG_NAME)) {
+                    sbMissingArgs.append("\n").append("Tool: " + tool.getToolQualifiedName());
+                    sbMissingArgs.append("\n Tool argument name was not provided for the parameter. Either add a name to the @ToolArg annotation, or to add the -parameters compiler option to use the parameter name");
+                    missingArgumentName = true;
                 }
             }
         }
@@ -90,8 +91,8 @@ private void reportOnToolArgEdgeCases(AfterDeploymentValidation afterDeploymentV
         if (duplicateArgumentsFound) {
             afterDeploymentValidation.addDeploymentProblem(new Exception(sbDuplicateArgs.toString()));
         }
-        if (illegalArgumentsFound) {
-            afterDeploymentValidation.addDeploymentProblem(new Exception(sbIllegalArgs.toString()));
+        if (missingArgumentName) {
+            afterDeploymentValidation.addDeploymentProblem(new Exception(sbMissingArgs.toString()));
         }
     }
 

dev/io.openliberty.mcp.internal/src/io/openliberty/mcp/internal/ToolMetadata.java

@@ -29,7 +29,7 @@ public record ToolMetadata(Tool annotation, Bean<?> bean, AnnotatedMethod<?> met
                            String name, String title, String description,
                            List<Class<? extends Throwable>> businessExceptions) {
 
-    public static final String ILLEGAL_TOOL_ARG_NAME = "<<<ILLEGAL NAME>>>";
+    public static final String MISSING_TOOL_ARG_NAME = "<<<MISSING TOOL_ARG NAME>>>";
 
     public record ArgumentMetadata(Type type, int index, String description, boolean required, boolean isDuplicate) {}
 
@@ -86,7 +86,7 @@ private static String resolveArgumentName(AnnotatedParameter<?> param, ToolArg a
             return param.getJavaParameter().getName();
         }
 
-        return ILLEGAL_TOOL_ARG_NAME;
+        return MISSING_TOOL_ARG_NAME;
     }
 
     private static List<SpecialArgumentMetadata> getSpecialArgumentList(AnnotatedMethod<?> method) {

dev/io.openliberty.mcp.internal_fat/.classpath

@@ -5,7 +5,7 @@
 			<attribute name="test" value="true"/>
 		</attributes>
 	</classpathentry>
-	<classpathentry kind="src" path="noParamTest">
+	<classpathentry kind="src" path="noParamNames">
 		<attributes>
 			<attribute name="test" value="true"/>
 		</attributes>

dev/io.openliberty.mcp.internal_fat/build.gradle

@@ -28,9 +28,9 @@ sourceSets {
 dependencies {
   requiredLibs 'org.json:json:20080701',
                'org.skyscreamer:jsonassert:2.0-rc1'
-  noParamNamesTestCompileOnly project(':io.openliberty.mcp'),
+  noParamNamesCompileOnly project(':io.openliberty.mcp'),
                               project(path: ':io.openliberty.jakarta', configuration: 'cdi40')
-  implementation sourceSets.noParamNamesTest.output
+  implementation sourceSets.noParamNames.output
 }
 
 /* Added to allow method parmaters to be available for MCP Tool Server*/ 
@@ -39,14 +39,14 @@ compileJava {
   options.compilerArgs << '-parameters'
 }
 
-// Build the noParamNamesTest code into a jar
-tasks.register('noParamNamesTestJar', Jar) {
-  dependsOn noParamNamesTestClasses
-  from sourceSets.noParamNamesTest.output
-  archiveFileName = 'noParamNamesTest.jar'
+// Build the noParamNames code into a jar
+tasks.register('noParamNamesJar', Jar) {
+  dependsOn noParamNamesClasses
+  from sourceSets.noParamNames.output
+  archiveFileName = 'noParamNames.jar'
 }
 
-// Build the noParamNamesTest jar before the autoFVT zip so that it gets included in the zip
+// Build the noParamNames jar before the autoFVT zip so that it gets included in the zip
 tasks.named('autoFVT') {
-  dependsOn noParamNamesTestJar
+  dependsOn noParamNamesJar
 }

dev/io.openliberty.mcp.internal_fat/fat/src/io/openliberty/mcp/internal/fat/tool/NoParamNameTest.java

@@ -39,8 +39,8 @@ public static void teardown() throws Exception {
 
     @Test
     public void testNoParamNameToolArg() throws Exception {
-        String expectedErrorHeader = "Illegal arguments found in MCP Tool:";
-        List<String> expectedErrorList = List.of("io.openliberty.mcp.internal.fat.noparamtool.NoParamTools.illegalToolArgNameTool");
-        ExpectedAppFailureValidator.findAndAssertExpectedErrorsInLogs("Illegal Tool Arg: ", expectedErrorHeader, expectedErrorList, server);
+        String expectedErrorHeader = "Missing arguments found in MCP Tool:";
+        List<String> expectedErrorList = List.of("io.openliberty.mcp.internal.fat.noparamtool.NoParamTools.missingToolArgNameTool");
+        ExpectedAppFailureValidator.findAndAssertExpectedErrorsInLogs("Missing arguments found in MCP Tool: ", expectedErrorHeader, expectedErrorList, server);
     }
 }

dev/io.openliberty.mcp.internal_fat/noParamTest/io/openliberty/mcp/internal/fat/noparamtool/NoParamTools.java renamed to dev/io.openliberty.mcp.internal_fat/noParamNames/io/openliberty/mcp/internal/fat/noparamtool/NoParamTools.java

@@ -16,8 +16,8 @@
 @ApplicationScoped
 public class NoParamTools {
 
-    @Tool(name = "illegalToolArgNameTool", title = "illegal ToolArgName Tool", description = "thows illegalArgumentExeption because of the ToolArgName")
-    public String illegalToolArgNameTool(@ToolArg(name = Tool.ELEMENT_NAME, description = "input to echo") String input) {
+    @Tool(name = "missingToolArgNameTool", title = "missing ToolArgName Tool", description = "ToolArgName is missing so app wont start")
+    public String missingToolArgNameTool(@ToolArg(description = "input to echo") String input) {
         return input;
     }
 }

dev/io.openliberty.mcp.internal_fat/noParamNames/io/openliberty/mcp/internal/fat/noparamtool/README.md

@@ -0,0 +1,4 @@
+# No Param Names
+
+This source set contains a basic tool called *missingToolArgNameTool*. The reason the tool has been separated out of the the rest of the application is because we need this to be compiled without the -parameters flag. This allows us to test that if parameter names are not included, then we report an error.
+