Skip to content

Commit 6c23ae5

Browse files
committed
Test review
1 parent 42f7b3e commit 6c23ae5

File tree

7 files changed

+165
-185
lines changed

7 files changed

+165
-185
lines changed

dev/io.openliberty.mcp.internal/bnd.bnd

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,11 @@ Bundle-Localization: \
5050

5151

5252
-testpath: \
53-
org.hamcrest:hamcrest-all;version=1.3, \
54-
../build.sharedResources/lib/junit/old/junit.jar;version=file, \
55-
io.openliberty.jakarta.jsonb.3.0;version=latest,\
56-
io.openliberty.org.eclipse.yasson.3.0;version=latest,\
57-
io.openliberty.org.eclipse.parsson.1.1;version=latest,\
58-
io.openliberty.mcp;version=latest,\
59-
org.json:json;version=20080701,\
60-
org.skyscreamer:jsonassert
53+
org.hamcrest:hamcrest-all;version='1.3',\
54+
../build.sharedResources/lib/junit/old/junit.jar;version=file,\
55+
io.openliberty.jakarta.jsonb.3.0;version=latest,\
56+
io.openliberty.org.eclipse.yasson.3.0;version=latest,\
57+
io.openliberty.org.eclipse.parsson.1.1;version=latest,\
58+
io.openliberty.mcp;version=latest,\
59+
org.json:json;version=20080701,\
60+
org.skyscreamer:jsonassert

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

Lines changed: 33 additions & 143 deletions
Original file line numberDiff line numberDiff line change
@@ -94,107 +94,6 @@ public record SpecialArgumentMetadata(SpecialArgumentType.Resolution typeResolut
9494
* @param jsonb the jsonb to use to serialize structured content
9595
* @return the created tool metadata
9696
*/
97-
// public static ToolMetadata createFrom(Tool annotation, Bean<?> bean, AnnotatedMethod<?> method, BeanManager bm, Jsonb jsonb) {
98-
// String name = annotation.name().equals(Tool.ELEMENT_NAME) ? method.getJavaMember().getName() : annotation.name();
99-
// String title = annotation.title().isEmpty() ? null : annotation.title();
100-
// String description = annotation.description().isEmpty() ? null : annotation.description();
101-
//
102-
// Type returnType = method.getJavaMember().getGenericReturnType();
103-
// Type outputType = unwrapOutputType(resolveOutputType(returnType));
104-
// Type actualOutputType = unwrapOutputType(outputType);
105-
//
106-
// Class<?> returnTypeClass = method.getJavaMember().getReturnType();
107-
//
108-
// WrapBusinessError wrapAnnotation = method.getAnnotation(WrapBusinessError.class);
109-
// List<Class<? extends Throwable>> businessExceptions = (wrapAnnotation != null) ? List.of(wrapAnnotation.value()) : Collections.emptyList();
110-
// boolean returnsCompletionStage = CompletionStage.class.isAssignableFrom(returnTypeClass);
111-
// SchemaRegistry sr = SchemaRegistry.get();
112-
//
113-
// JsonObject inputSchema = sr.getToolInputSchema(method);
114-
// JsonObject outputSchema = null;
115-
//
116-
// boolean isString = outputType.equals(String.class);
117-
// boolean isContent = outputType.equals(Content.class);
118-
// boolean isToolResponse = outputType instanceof Class<?> c &&
119-
// ToolResponse.class.isAssignableFrom(c);
120-
//
121-
// System.out.println("\n======= DEBUG: ToolMetadata.createFrom =======");
122-
// System.out.println("Tool Name: " + name);
123-
// System.out.println("structuredContent: " + annotation.structuredContent());
124-
// System.out.println("Return Type: " + method.getJavaMember().getGenericReturnType().getTypeName());
125-
// System.out.println("Resolved Output Type: " + outputType.getTypeName());
126-
// System.out.println("Is CompletionStage: " + CompletionStage.class.isAssignableFrom(returnTypeClass));
127-
// System.out.println("isToolResponse: " + (outputType instanceof Class<?> &&
128-
// ToolResponse.class.isAssignableFrom((Class<?>) outputType)));
129-
//
130-
// boolean isListOfSerializableType = false;
131-
//
132-
// // Handle raw ToolResponse types
133-
// if (ToolResponse.class.isAssignableFrom(returnTypeClass)) {
134-
// if (outputType instanceof Class<?>) {
135-
// System.err.println("Skipping schema generation for raw ToolResponse return type in: " + name);
136-
// outputSchema = null;
137-
// } else if (outputType instanceof ParameterizedType pt &&
138-
// ToolResponse.class.isAssignableFrom((Class<?>) pt.getRawType())) {
139-
// actualOutputType = pt.getActualTypeArguments()[0];
140-
// }
141-
// }
142-
//
143-
// // Determine if we should generate schema
144-
// boolean hasOutputSchema = annotation.structuredContent()
145-
// && !(isString || isContent || isToolResponse)
146-
// && (actualOutputType instanceof Class<?> || isListOfSerializableType);
147-
//
148-
// // Special case - ToolResponse<T> with @Schema
149-
// if (!hasOutputSchema &&
150-
// returnTypeClass.isAssignableFrom(ToolResponse.class) &&
151-
// annotation.structuredContent() &&
152-
// method.isAnnotationPresent(Schema.class) &&
153-
// method.getAnnotation(Schema.class).value() != Schema.UNSET) {
154-
// hasOutputSchema = true;
155-
// }
156-
//
157-
// // Generate outputSchema
158-
// if (hasOutputSchema) {
159-
// try {
160-
// System.out.println("Unwrapped output type used for schema: " + actualOutputType.getTypeName());
161-
// outputSchema = generateOutputSchema(actualOutputType, method);
162-
// System.out.println("Generated Output Schema: " + outputSchema);
163-
// if (outputSchema == null || outputSchema.isEmpty()) {
164-
// System.out.println("Output Schema is null or empty for tool: " + name);
165-
// }
166-
// } catch (Exception e) {
167-
// System.err.println("Schema generation failed for tool: " + name + ", type: " + outputType.getTypeName());
168-
// e.printStackTrace();
169-
// }
170-
// }
171-
//
172-
// outputSchema = (outputSchema == null || outputSchema.isEmpty()) ? null : outputSchema;
173-
//
174-
// ToolAnnotations annotations = readAnnotations(annotation.annotations());
175-
//
176-
// Map<String, ArgumentMetadata> argumentMap = getArgumentMap(method);
177-
//
178-
// MethodMetadata methodMetadata = new MethodMetadata(name,
179-
// bean,
180-
// method.getJavaMember(),
181-
// hasOutputSchema,
182-
// businessExceptions,
183-
// getSpecialArgumentList(method),
184-
// getArgNameArray(method, argumentMap));
185-
//
186-
// SyncBeanMethodHandler handler = null;
187-
// AsyncBeanMethodHandler asyncHandler = null;
188-
// if (returnsCompletionStage) {
189-
// asyncHandler = new AsyncBeanMethodHandler(jsonb, bm, methodMetadata);
190-
// } else {
191-
// handler = new SyncBeanMethodHandler(jsonb, bm, methodMetadata);
192-
// }
193-
//
194-
// return new ToolMetadata(name, title, description, getArgumentMap(method), annotations, returnsCompletionStage, inputSchema, outputSchema, handler, asyncHandler,
195-
// Optional.of(methodMetadata));
196-
//
197-
// }
19897
public static ToolMetadata createFrom(Tool annotation, Bean<?> bean, AnnotatedMethod<?> method, BeanManager bm, Jsonb jsonb) {
19998
String name = annotation.name().equals(Tool.ELEMENT_NAME) ? method.getJavaMember().getName() : annotation.name();
20099
String title = annotation.title().isEmpty() ? null : annotation.title();
@@ -205,30 +104,20 @@ public static ToolMetadata createFrom(Tool annotation, Bean<?> bean, AnnotatedMe
205104
Type unwrappedOutputType = unwrapOutputType(outputType);
206105

207106
Class<?> returnTypeClass = method.getJavaMember().getReturnType();
208-
boolean returnsCompletionStage = CompletionStage.class.isAssignableFrom(returnTypeClass);
209-
boolean isToolResponse = ToolResponse.class.isAssignableFrom(returnTypeClass);
210107

108+
WrapBusinessError wrapAnnotation = method.getAnnotation(WrapBusinessError.class);
109+
List<Class<? extends Throwable>> businessExceptions = (wrapAnnotation != null) ? List.of(wrapAnnotation.value()) : Collections.emptyList();
110+
boolean returnsCompletionStage = CompletionStage.class.isAssignableFrom(returnTypeClass);
211111
SchemaRegistry sr = SchemaRegistry.get();
212-
JsonObject inputSchema = sr.getToolInputSchema(method);
213-
JsonObject outputSchema = null;
214112

113+
JsonObject inputSchema = sr.getToolInputSchema(method);
215114
boolean isString = unwrappedOutputType.equals(String.class);
216115
boolean isContent = unwrappedOutputType.equals(Content.class);
217116

218-
// Business exceptions
219-
WrapBusinessError wrapAnnotation = method.getAnnotation(WrapBusinessError.class);
220-
List<Class<? extends Throwable>> businessExceptions = (wrapAnnotation != null) ? List.of(wrapAnnotation.value()) : Collections.emptyList();
221-
222-
System.out.println("\n======= DEBUG: ToolMetadata.createFrom =======");
223-
System.out.println("Tool Name: " + name);
224-
System.out.println("structuredContent: " + annotation.structuredContent());
225-
System.out.println("Return Type: " + returnType.getTypeName());
226-
System.out.println("Resolved Output Type: " + outputType.getTypeName());
227-
System.out.println("Unwrapped Output Type: " + unwrappedOutputType.getTypeName());
228-
System.out.println("Is CompletionStage: " + returnsCompletionStage);
229-
System.out.println("Is ToolResponse: " + isToolResponse);
230-
231-
boolean hasOutputSchema = false;
117+
boolean hasContentListReturn = (returnType instanceof ParameterizedType pt && ((Class<?>) pt.getRawType()).isAssignableFrom(List.class)
118+
&& (pt.getActualTypeArguments()[0] instanceof Class<?>) && ((Class<?>) pt.getActualTypeArguments()[0]).isAssignableFrom(Content.class));
119+
boolean hasOutputSchema = (!returnTypeClass.isAssignableFrom(ToolResponse.class) && !hasContentListReturn && !returnTypeClass.isAssignableFrom(Content.class)
120+
&& !returnTypeClass.isAssignableFrom(String.class) && annotation.structuredContent());
232121

233122
if (annotation.structuredContent()) {
234123
// Exclude basic types that shouldn't get a schema
@@ -238,43 +127,35 @@ public static ToolMetadata createFrom(Tool annotation, Bean<?> bean, AnnotatedMe
238127
// Unwrap ToolResponse<T> -> T
239128
Type inner = pt.getActualTypeArguments()[0];
240129
unwrappedOutputType = unwrapOutputType(inner);
241-
hasOutputSchema = true;
130+
// hasOutputSchema = true;
242131
} else {
243132
// if it's not a String/Content, we want schema
244133
hasOutputSchema = true;
245134
}
246135
}
247136
}
137+
if (!hasOutputSchema && returnTypeClass.isAssignableFrom(ToolResponse.class) && annotation.structuredContent()
138+
&& method.isAnnotationPresent(Schema.class)
139+
&& method.getAnnotation(Schema.class).value() != Schema.UNSET) {
248140

249-
// user manually added @Schema(...) on method
250-
if (!hasOutputSchema &&
251-
annotation.structuredContent() &&
252-
method.isAnnotationPresent(Schema.class) &&
253-
!Schema.UNSET.equals(method.getAnnotation(Schema.class).value())) {
254141
hasOutputSchema = true;
255-
}
256142

257-
// Generate output schema
258-
if (hasOutputSchema) {
259-
try {
260-
System.out.println("Generating schema for type: " + unwrappedOutputType.getTypeName());
261-
outputSchema = sr.getToolOutputSchema(method, outputType);
262-
System.out.println("Generated schema: " + outputSchema);
263-
} catch (Exception e) {
264-
System.err.println("Failed to generate schema for: " + name + ", type: " + unwrappedOutputType.getTypeName());
265-
e.printStackTrace();
266-
}
267143
}
144+
JsonObject outputSchema = hasOutputSchema ? sr.getToolOutputSchema(method, unwrappedOutputType) : null;
268145

269-
// Cleanup if empty
270146
outputSchema = (outputSchema == null || outputSchema.isEmpty()) ? null : outputSchema;
271147

272148
ToolAnnotations annotations = readAnnotations(annotation.annotations());
149+
273150
Map<String, ArgumentMetadata> argumentMap = getArgumentMap(method);
274151

275-
MethodMetadata methodMetadata = new MethodMetadata(
276-
name, bean, method.getJavaMember(), hasOutputSchema,
277-
businessExceptions, getSpecialArgumentList(method), getArgNameArray(method, argumentMap));
152+
MethodMetadata methodMetadata = new MethodMetadata(name,
153+
bean,
154+
method.getJavaMember(),
155+
hasOutputSchema,
156+
businessExceptions,
157+
getSpecialArgumentList(method),
158+
getArgNameArray(method, argumentMap));
278159

279160
SyncBeanMethodHandler handler = null;
280161
AsyncBeanMethodHandler asyncHandler = null;
@@ -284,9 +165,18 @@ public static ToolMetadata createFrom(Tool annotation, Bean<?> bean, AnnotatedMe
284165
handler = new SyncBeanMethodHandler(jsonb, bm, methodMetadata);
285166
}
286167

287-
return new ToolMetadata(name, title, description, argumentMap, annotations,
288-
returnsCompletionStage, inputSchema, outputSchema,
289-
handler, asyncHandler, Optional.of(methodMetadata));
168+
return new ToolMetadata(name,
169+
title,
170+
description,
171+
getArgumentMap(method),
172+
annotations,
173+
returnsCompletionStage,
174+
inputSchema,
175+
outputSchema,
176+
handler,
177+
asyncHandler,
178+
Optional.of(methodMetadata));
179+
290180
}
291181

292182
/**

dev/io.openliberty.mcp.internal/test/io/openliberty/mcp/internal/test/AsyncToolSchemaTest.java

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,33 @@
1-
/*******************************************************************************
2-
* Copyright (c) 2025 IBM Corporation and others.
3-
* All rights reserved. This program and the accompanying materials
4-
* are made available under the terms of the Eclipse Public License 2.0
5-
* which accompanies this distribution, and is available at
6-
* http://www.eclipse.org/legal/epl-2.0/
7-
*
8-
* SPDX-License-Identifier: EPL-2.0
9-
*******************************************************************************/
10-
package io.openliberty.mcp.internal.test;
11-
12-
//public class AsyncToolSchemaTest {
1+
///*******************************************************************************
2+
// * Copyright (c) 2025 IBM Corporation and others.
3+
// * All rights reserved. This program and the accompanying materials
4+
// * are made available under the terms of the Eclipse Public License 2.0
5+
// * which accompanies this distribution, and is available at
6+
// * http://www.eclipse.org/legal/epl-2.0/
7+
// *
8+
// * SPDX-License-Identifier: EPL-2.0
9+
// *******************************************************************************/
10+
//package io.openliberty.mcp.internal.test;
1311
//
14-
// @Before
15-
// public void setup() throws Exception {
16-
// ToolRegistry registry = new ToolRegistry();
12+
//import static org.junit.Assert.assertFalse;
13+
//import static org.junit.Assert.assertNotNull;
14+
//import static org.junit.Assert.assertNull;
15+
//import static org.junit.Assert.assertTrue;
1716
//
18-
// Method asyncObjectToolMethod = AsyncTools.class.getMethod("asyncObjectTool", String.class);
19-
// AnnotatedMethodStub stub = new AnnotatedMethodStub(AsyncTools.class, asyncObjectToolMethod);
17+
//import org.junit.Before;
18+
//import org.junit.Test;
2019
//
21-
// SchemaRegistry schemaRegistry = new SchemaRegistry();
22-
// Jsonb jsonb = JsonbBuilder.create();
20+
//import io.openliberty.mcp.internal.ToolMetadata;
21+
//import io.openliberty.mcp.internal.ToolRegistry;
22+
//import io.openliberty.mcp.internal.tools.BeanMethodHandler.MethodMetadata;
2323
//
24-
// ToolMetadata metadata = ToolMetadata.createFrom(
25-
// asyncObjectToolMethod.getAnnotation(Tool.class),
26-
// null,
27-
// stub,
28-
// null,
29-
// jsonb,
30-
// schemaRegistry);
24+
//public class AsyncToolSchemaTest {
3125
//
32-
// registry.addTool(metadata);
26+
// private ToolRegistry registry;
27+
//
28+
// @Before
29+
// public void setup() throws Exception {
30+
// registry = new ToolRegistry();
3331
// ToolRegistry.set(registry);
3432
// }
3533
//

dev/io.openliberty.mcp.internal/test/io/openliberty/mcp/internal/test/schema/SchemaTest.java

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
*******************************************************************************/
1010
package io.openliberty.mcp.internal.test.schema;
1111

12+
import java.lang.reflect.Type;
1213
import java.util.List;
1314
import java.util.Map;
1415
import java.util.Optional;
@@ -330,8 +331,9 @@ public void testToolInputSchema() throws NoSuchMethodException, SecurityExceptio
330331
@Test
331332
public void testToolOutputSchema() throws NoSuchMethodException, SecurityException {
332333
MockAnnotatedMethod<Object> toolMethod = TestUtils.findMethod(SchemaTest.class, "updateWidget");
334+
Type returnType = toolMethod.getJavaMember().getGenericReturnType();
333335

334-
String toolInputSchema = registry.getToolOutputSchema(toolMethod).toString();
336+
String toolInputSchema = registry.getToolOutputSchema(toolMethod, returnType).toString();
335337
String expectedSchema = """
336338
{
337339
"type": "object",
@@ -414,7 +416,9 @@ public void testToolInputRecursive() {
414416
@Test
415417
public void testToolOutputRecursive() {
416418
MockAnnotatedMethod<Object> toolMethod = TestUtils.findMethod(SchemaTest.class, "combineWidgets");
417-
String toolInputSchema = registry.getToolOutputSchema(toolMethod).toString();
419+
Type returnType = toolMethod.getJavaMember().getGenericReturnType();
420+
421+
String toolInputSchema = registry.getToolOutputSchema(toolMethod, returnType).toString();
418422
String expectedSchema = """
419423
{
420424
"$defs": {
@@ -999,7 +1003,9 @@ public void testPersonAddtoListToolInputSchema() throws NoSuchMethodException, S
9991003
@Test
10001004
public void testPersonAddtoListToolOutputSchema() throws NoSuchMethodException, SecurityException {
10011005
MockAnnotatedMethod<Object> toolMethod = TestUtils.findMethod(SchemaTest.class, "addPersonToList");
1002-
String response = registry.getToolOutputSchema(toolMethod).toString();
1006+
Type returnType = toolMethod.getJavaMember().getGenericReturnType();
1007+
1008+
String response = registry.getToolOutputSchema(toolMethod, returnType).toString();
10031009
String expectedResponseString = """
10041010
{
10051011
"$defs": {
@@ -2258,9 +2264,10 @@ public int[] primitiveArrayTest(@ToolArg(name = "name", description = "name") St
22582264
@Test
22592265
public void testPrimitiveArray() {
22602266
MockAnnotatedMethod<Object> toolMethod = TestUtils.findMethod(SchemaTest.class, "primitiveArrayTest");
2261-
String response = registry.getToolOutputSchema(toolMethod).toString();
2267+
Type returnType = toolMethod.getJavaMember().getGenericReturnType();
2268+
String response = registry.getToolOutputSchema(toolMethod, returnType).toString();
22622269
String expectedResponseString = """
2263-
{"type":"array","items":{"type":"integer"}}
2270+
{"type":"array","items":{"type":"integer"}}
22642271
""";
22652272
JSONAssert.assertEquals(expectedResponseString, response, true);
22662273
}

dev/io.openliberty.mcp.internal_fat/bnd.bnd

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,6 @@ tested.features: \
3636
io.openliberty.jakarta.jsonb.3.0;version=latest,\
3737
io.openliberty.jakarta.jsonp.2.1;version=latest,\
3838
io.openliberty.jakarta.annotation.2.1,\
39-
io.openliberty.jakarta.concurrency.3.0
39+
io.openliberty.jakarta.concurrency.3.0,\
40+
io.openliberty.jakarta.jsonb.2.0,\
41+
jakarta.json.bind-api

0 commit comments

Comments
 (0)