This is an automated email from the ASF dual-hosted git repository. yuluo-yx pushed a commit to branch 0721-yuluo/feat in repository https://gitbox.apache.org/repos/asf/hertzbeat.git
commit e2694796b4dc9ed4334e361f5c7913a88807c4fd Author: yuluo-yx <[email protected]> AuthorDate: Tue Jul 21 23:09:14 2026 +0800 fix(ai): improve SOP execution and provider config handling Signed-off-by: yuluo-yx <[email protected]> --- .../impl/ChatClientProviderServiceImpl.java | 41 ++++--- .../hertzbeat/ai/sop/engine/SopEngineImpl.java | 110 ++++++++++--------- .../hertzbeat/ai/sop/registry/SopToolCallback.java | 81 ++++++++++---- .../hertzbeat/ai/sop/registry/ToolRegistry.java | 26 +++-- .../src/main/resources/skills/README_ZH.md | 8 +- .../impl/ChatClientProviderServiceImplTest.java | 74 +++++++++++++ .../hertzbeat/ai/sop/engine/SopEngineImplTest.java | 115 ++++++++++++++++++++ .../ai/sop/registry/SopToolCallbackTest.java | 119 +++++++++++++++++++++ .../ai/sop/registry/ToolRegistryTest.java | 63 +++++++++++ 9 files changed, 544 insertions(+), 93 deletions(-) diff --git a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/service/impl/ChatClientProviderServiceImpl.java b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/service/impl/ChatClientProviderServiceImpl.java index dec9bfc90c..220a95c816 100644 --- a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/service/impl/ChatClientProviderServiceImpl.java +++ b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/service/impl/ChatClientProviderServiceImpl.java @@ -48,6 +48,7 @@ import org.springframework.ai.tool.ToolCallbackProvider; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.ApplicationContext; +import org.springframework.util.StringUtils; import reactor.core.publisher.Flux; import java.io.IOException; @@ -69,7 +70,7 @@ public class ChatClientProviderServiceImpl implements ChatClientProviderService private final GeneralConfigDao generalConfigDao; - private ModelProviderConfig modelProviderConfig; + private volatile ModelProviderConfig modelProviderConfig; private final SkillRegistry skillRegistry; @@ -78,7 +79,9 @@ public class ChatClientProviderServiceImpl implements ChatClientProviderService @Qualifier("hertzbeatTools") private ToolCallbackProvider toolCallbackProvider; - private boolean isConfigured = false; + private volatile boolean configured; + + private volatile boolean configurationLoaded; @Value("classpath:/prompt/system-message.st") private Resource systemResource; @@ -150,7 +153,8 @@ public class ChatClientProviderServiceImpl implements ChatClientProviderService .replace(CONVERSATION_ID_PLACEHOLDER, String.valueOf(conversationId)); // add extra prompt for protected model to guide it to use protected tools - if (Objects.equals(modelProviderConfig.getParticipationModel(), "PROTECTED")) { + ModelProviderConfig currentConfig = modelProviderConfig; + if (currentConfig != null && Objects.equals(currentConfig.getParticipationModel(), "PROTECTED")) { Map<String, Object> metadata = new HashMap<>(); metadata.put("conversationId", conversationId); return template + SystemPromptTemplate.builder().resource(extraResourceProtected).build() @@ -202,19 +206,32 @@ public class ChatClientProviderServiceImpl implements ChatClientProviderService @EventListener(AiProviderConfigChangeEvent.class) public void onAiProviderConfigChange(AiProviderConfigChangeEvent event) { - GeneralConfig providerConfig = generalConfigDao.findByType("provider"); - this.modelProviderConfig = JsonUtil.fromJson(providerConfig.getContent(), ModelProviderConfig.class); + refreshProviderConfiguration(); } @Override public boolean isConfigured() { - if (!isConfigured) { - GeneralConfig providerConfig = generalConfigDao.findByType("provider"); - ModelProviderConfig modelProviderConfig = JsonUtil.fromJson(providerConfig.getContent(), - ModelProviderConfig.class); - isConfigured = modelProviderConfig != null && modelProviderConfig.getApiKey() != null; - this.modelProviderConfig = modelProviderConfig; + if (!configurationLoaded) { + synchronized (this) { + if (!configurationLoaded) { + refreshProviderConfiguration(); + } + } + } + return configured; + } + + /** + * Atomically refreshes the configuration snapshot after enabling, disabling, or switching providers. + */ + private synchronized void refreshProviderConfiguration() { + GeneralConfig providerConfig = generalConfigDao.findByType("provider"); + ModelProviderConfig refreshedConfig = null; + if (providerConfig != null && StringUtils.hasText(providerConfig.getContent())) { + refreshedConfig = JsonUtil.fromJson(providerConfig.getContent(), ModelProviderConfig.class); } - return isConfigured; + modelProviderConfig = refreshedConfig; + configured = refreshedConfig != null && StringUtils.hasText(refreshedConfig.getApiKey()); + configurationLoaded = true; } } diff --git a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/engine/SopEngineImpl.java b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/engine/SopEngineImpl.java index 44ac80da12..af38c1fdd0 100644 --- a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/engine/SopEngineImpl.java +++ b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/engine/SopEngineImpl.java @@ -41,9 +41,6 @@ import reactor.core.publisher.Flux; @Service public class SopEngineImpl implements SopEngine { - // Thread-local context for each execution - private static final ThreadLocal<Map<String, Object>> CONTEXT_BUS = ThreadLocal.withInitial(HashMap::new); - private final List<SopExecutor> executors; @Autowired @@ -55,22 +52,12 @@ public class SopEngineImpl implements SopEngine { public Flux<String> execute(SopDefinition definition, Map<String, Object> inputParams) { return Flux.create(sink -> { try { + OutputConfig outputConfig = resolveOutputConfig(definition); + Map<String, Object> context = prepareContext(definition, inputParams, outputConfig); + log.info("Starting execution of SOP: {}", definition.getName()); sink.next("Starting SOP: " + definition.getName() + " (v" + definition.getVersion() + ")"); - // Initialize context with input parameters - Map<String, Object> context = CONTEXT_BUS.get(); - context.clear(); - context.putAll(inputParams); - - // Add language configuration to context - OutputConfig outputConfig = definition.getOutput(); - if (outputConfig != null) { - context.put("_language", outputConfig.getLanguageCode()); - } else { - context.put("_language", "zh"); - } - for (SopStep step : definition.getSteps()) { sink.next("Executing step [" + step.getId() + "]: " + step.getType()); @@ -106,10 +93,9 @@ public class SopEngineImpl implements SopEngine { sink.next("SOP " + definition.getName() + " completed successfully."); sink.complete(); } catch (Exception e) { - log.error("Error executing SOP {}: {}", definition.getName(), e.getMessage(), e); + String sopName = definition == null ? "unknown" : definition.getName(); + log.error("Error executing SOP {}: {}", sopName, e.getMessage(), e); sink.error(e); - } finally { - CONTEXT_BUS.remove(); } }); } @@ -118,40 +104,20 @@ public class SopEngineImpl implements SopEngine { public SopResult executeSync(SopDefinition definition, Map<String, Object> inputParams) { long startTime = System.currentTimeMillis(); List<StepResult> stepResults = new ArrayList<>(); - Map<String, Object> context = new HashMap<>(inputParams); - - // Apply default values for parameters that are not provided - if (definition.getParameters() != null) { - for (SopParameter param : definition.getParameters()) { - if (!context.containsKey(param.getName()) && param.getDefaultValue() != null) { - context.put(param.getName(), param.getDefaultValue()); - } - } - } - - // Get output configuration first - OutputConfig outputConfig = definition.getOutput(); - if (outputConfig == null) { - outputConfig = OutputConfig.builder() - .type("simple") - .format("text") - .language("zh") - .build(); - } - - // Add language configuration to context - context.put("_language", outputConfig.getLanguageCode()); - + SopResult.SopResultBuilder resultBuilder = SopResult.builder() - .sopName(definition.getName()) - .sopVersion(definition.getVersion()) + .sopName(definition == null ? null : definition.getName()) + .sopVersion(definition == null ? null : definition.getVersion()) .startTime(startTime); - - resultBuilder.outputType(outputConfig.getOutputType()); - resultBuilder.outputFormat(outputConfig.getFormat() != null ? outputConfig.getFormat() : "text"); - resultBuilder.language(outputConfig.getLanguageCode()); - + try { + OutputConfig outputConfig = resolveOutputConfig(definition); + Map<String, Object> context = prepareContext(definition, inputParams, outputConfig); + + resultBuilder.outputType(outputConfig.getOutputType()); + resultBuilder.outputFormat(outputConfig.getFormat() != null ? outputConfig.getFormat() : "text"); + resultBuilder.language(outputConfig.getLanguageCode()); + log.info("Starting sync execution of SOP: {}", definition.getName()); for (SopStep step : definition.getSteps()) { @@ -217,6 +183,50 @@ public class SopEngineImpl implements SopEngine { return buildFailedResult(resultBuilder, stepResults, e.getMessage(), startTime); } } + + /** + * Applies defaults and validates required parameters consistently for every execution entry point. + */ + private Map<String, Object> prepareContext(SopDefinition definition, Map<String, Object> inputParams, + OutputConfig outputConfig) { + if (definition.getSteps() == null || definition.getSteps().isEmpty()) { + throw new IllegalArgumentException("SOP must contain at least one step"); + } + + Map<String, Object> context = inputParams == null ? new HashMap<>() : new HashMap<>(inputParams); + if (definition.getParameters() != null) { + for (SopParameter parameter : definition.getParameters()) { + Object value = context.get(parameter.getName()); + if (isMissing(value) && parameter.getDefaultValue() != null) { + context.put(parameter.getName(), parameter.getDefaultValue()); + value = parameter.getDefaultValue(); + } + if (parameter.isRequired() && isMissing(value)) { + throw new IllegalArgumentException("Required SOP parameter is missing: " + parameter.getName()); + } + } + } + context.put("_language", outputConfig.getLanguageCode()); + return context; + } + + private boolean isMissing(Object value) { + return value == null || value instanceof String text && text.isBlank(); + } + + private OutputConfig resolveOutputConfig(SopDefinition definition) { + if (definition == null) { + throw new IllegalArgumentException("SOP definition must not be null"); + } + if (definition.getOutput() != null) { + return definition.getOutput(); + } + return OutputConfig.builder() + .type("simple") + .format("text") + .language("zh") + .build(); + } private SopResult buildFailedResult(SopResult.SopResultBuilder builder, List<StepResult> stepResults, String error, long startTime) { diff --git a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/registry/SopToolCallback.java b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/registry/SopToolCallback.java index 90ce21e7c2..21eb124c5f 100644 --- a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/registry/SopToolCallback.java +++ b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/registry/SopToolCallback.java @@ -17,8 +17,16 @@ package org.apache.hertzbeat.ai.sop.registry; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; import org.apache.hertzbeat.ai.sop.engine.SopEngine; import org.apache.hertzbeat.ai.sop.model.SopDefinition; +import org.apache.hertzbeat.ai.sop.model.SopParameter; +import org.apache.hertzbeat.ai.sop.model.SopResult; +import org.apache.hertzbeat.common.util.JsonUtil; import org.springframework.ai.tool.ToolCallback; import org.springframework.ai.tool.definition.ToolDefinition; @@ -49,33 +57,66 @@ public class SopToolCallback implements ToolCallback { @Override public String call(String arguments) { - // TODO: Parse arguments and execute SOP - return "SOP " + definition.getName() + " execution started."; + Map<String, Object> inputParams = parseArguments(arguments); + SopResult result = sopEngine.executeSync(definition, inputParams); + return result.toAiResponse(); } private String buildInputSchema() { - // Build JSON Schema for SOP parameters - StringBuilder schema = new StringBuilder(); - schema.append("{\"type\":\"object\",\"properties\":{"); - - if (definition.getParameters() != null && !definition.getParameters().isEmpty()) { - boolean first = true; - for (var param : definition.getParameters()) { - if (!first) { - schema.append(","); + Map<String, Object> schema = new LinkedHashMap<>(); + Map<String, Object> properties = new LinkedHashMap<>(); + List<String> required = new ArrayList<>(); + + if (definition.getParameters() != null) { + for (SopParameter parameter : definition.getParameters()) { + Map<String, Object> property = new LinkedHashMap<>(); + property.put("type", mapType(parameter.getType())); + if (parameter.getDescription() != null) { + property.put("description", parameter.getDescription()); + } + if (parameter.getDefaultValue() != null) { + property.put("default", convertDefaultValue(parameter)); } - schema.append("\"").append(param.getName()).append("\":"); - schema.append("{\"type\":\"").append(mapType(param.getType())).append("\""); - if (param.getDescription() != null) { - schema.append(",\"description\":\"").append(param.getDescription()).append("\""); + properties.put(parameter.getName(), property); + if (parameter.isRequired()) { + required.add(parameter.getName()); } - schema.append("}"); - first = false; } } - - schema.append("}}"); - return schema.toString(); + + schema.put("type", "object"); + schema.put("properties", properties); + if (!required.isEmpty()) { + schema.put("required", required); + } + schema.put("additionalProperties", false); + return JsonUtil.toJson(schema); + } + + @SuppressWarnings("unchecked") + private Map<String, Object> parseArguments(String arguments) { + if (arguments == null || arguments.isBlank()) { + return new HashMap<>(); + } + Map<String, Object> inputParams = JsonUtil.fromJson(arguments, Map.class); + if (inputParams == null) { + throw new IllegalArgumentException("SOP tool arguments must be a valid JSON object"); + } + return inputParams; + } + + private Object convertDefaultValue(SopParameter parameter) { + String defaultValue = parameter.getDefaultValue(); + try { + return switch (mapType(parameter.getType())) { + case "boolean" -> Boolean.valueOf(defaultValue); + case "integer" -> Long.valueOf(defaultValue); + case "number" -> Double.valueOf(defaultValue); + default -> defaultValue; + }; + } catch (NumberFormatException e) { + throw new IllegalArgumentException("Invalid default value for SOP parameter: " + parameter.getName(), e); + } } private String mapType(String type) { diff --git a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/registry/ToolRegistry.java b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/registry/ToolRegistry.java index 77f3a68bfa..bf72956ee7 100644 --- a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/registry/ToolRegistry.java +++ b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/registry/ToolRegistry.java @@ -129,7 +129,7 @@ public class ToolRegistry { */ public Set<String> getToolNames() { ensureInitialized(); - return tools.keySet(); + return Set.copyOf(tools.keySet()); } /** @@ -176,15 +176,19 @@ public class ToolRegistry { * Invoke this tool method with the given arguments. */ public String invoke(Map<String, Object> args) { - try { - Object[] methodArgs = new Object[paramInfos.size()]; - - for (int i = 0; i < paramInfos.size(); i++) { - ParamInfo paramInfo = paramInfos.get(i); - Object value = args.get(paramInfo.name); - methodArgs[i] = convertValue(value, paramInfo.type); + Map<String, Object> safeArgs = args == null ? Map.of() : args; + Object[] methodArgs = new Object[paramInfos.size()]; + + for (int i = 0; i < paramInfos.size(); i++) { + ParamInfo paramInfo = paramInfos.get(i); + Object value = safeArgs.get(paramInfo.name); + if (paramInfo.required && isMissing(value)) { + throw new IllegalArgumentException("Required tool parameter is missing: " + paramInfo.name); } - + methodArgs[i] = convertValue(value, paramInfo.type); + } + + try { Object result = method.invoke(bean, methodArgs); return result != null ? result.toString() : ""; @@ -194,6 +198,10 @@ public class ToolRegistry { } } + private boolean isMissing(Object value) { + return value == null || value instanceof String text && text.isBlank(); + } + private Object convertValue(Object value, Class<?> targetType) { if (value == null) { return null; diff --git a/hertzbeat-ai/src/main/resources/skills/README_ZH.md b/hertzbeat-ai/src/main/resources/skills/README_ZH.md index dfad433185..75ec5f8dd3 100644 --- a/hertzbeat-ai/src/main/resources/skills/README_ZH.md +++ b/hertzbeat-ai/src/main/resources/skills/README_ZH.md @@ -65,12 +65,16 @@ POST /api/ai/sop/execute/{skillName}/ai ``` YAML 定义 → SkillRegistry → SopEngine → Executors → SopResult - ↓ - ToolExecutor / LlmExecutor + ↓ ↓ + SopToolCallback ToolExecutor / LlmExecutor ↓ ToolRegistry（自动发现 @Tool 方法） ``` +所有执行入口都由 `SopEngine` 统一补齐默认参数并校验必填参数。技能注册为 Spring AI +工具时，`SopToolCallback` 会根据参数定义生成 JSON Schema、解析模型传入的 JSON 对象， +同步执行 SOP 并返回 `SopResult`；调用方不应在控制器或工具外层重复实现参数校验。 + ## 添加新工具 只需添加 `@Tool` 注解，无需修改其他代码： diff --git a/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/service/impl/ChatClientProviderServiceImplTest.java b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/service/impl/ChatClientProviderServiceImplTest.java new file mode 100644 index 0000000000..d1f5f2b4bc --- /dev/null +++ b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/service/impl/ChatClientProviderServiceImplTest.java @@ -0,0 +1,74 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hertzbeat.ai.service.impl; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.reflect.Proxy; +import java.util.concurrent.atomic.AtomicReference; +import org.apache.hertzbeat.base.dao.GeneralConfigDao; +import org.apache.hertzbeat.common.entity.dto.ModelProviderConfig; +import org.apache.hertzbeat.common.entity.manager.GeneralConfig; +import org.apache.hertzbeat.common.support.event.AiProviderConfigChangeEvent; +import org.apache.hertzbeat.common.util.JsonUtil; +import org.junit.jupiter.api.Test; +import org.springframework.context.support.StaticApplicationContext; + +/** + * Verifies that the provider configuration cache reacts to enable and disable events. + */ +class ChatClientProviderServiceImplTest { + + @Test + void configurationChangeShouldRefreshConfiguredState() { + AtomicReference<GeneralConfig> currentConfig = new AtomicReference<>(); + GeneralConfigDao configDao = configDao(currentConfig); + ChatClientProviderServiceImpl service = new ChatClientProviderServiceImpl(null, configDao, null); + + assertFalse(service.isConfigured()); + + currentConfig.set(providerConfig("sk-test")); + service.onAiProviderConfigChange(changeEvent()); + assertTrue(service.isConfigured()); + + currentConfig.set(providerConfig(" ")); + service.onAiProviderConfigChange(changeEvent()); + assertFalse(service.isConfigured()); + } + + private GeneralConfigDao configDao(AtomicReference<GeneralConfig> currentConfig) { + return (GeneralConfigDao) Proxy.newProxyInstance( + GeneralConfigDao.class.getClassLoader(), + new Class<?>[]{GeneralConfigDao.class}, + (proxy, method, args) -> "findByType".equals(method.getName()) ? currentConfig.get() : null); + } + + private GeneralConfig providerConfig(String apiKey) { + ModelProviderConfig modelConfig = new ModelProviderConfig(); + modelConfig.setApiKey(apiKey); + return GeneralConfig.builder() + .type("provider") + .content(JsonUtil.toJson(modelConfig)) + .build(); + } + + private AiProviderConfigChangeEvent changeEvent() { + return new AiProviderConfigChangeEvent(new StaticApplicationContext()); + } +} diff --git a/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/engine/SopEngineImplTest.java b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/engine/SopEngineImplTest.java new file mode 100644 index 0000000000..94b281d699 --- /dev/null +++ b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/engine/SopEngineImplTest.java @@ -0,0 +1,115 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hertzbeat.ai.sop.engine; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; +import org.apache.hertzbeat.ai.sop.executor.SopExecutor; +import org.apache.hertzbeat.ai.sop.model.SopDefinition; +import org.apache.hertzbeat.ai.sop.model.SopParameter; +import org.apache.hertzbeat.ai.sop.model.SopResult; +import org.apache.hertzbeat.ai.sop.model.SopStep; +import org.junit.jupiter.api.Test; + +/** + * Verifies that all SOP execution entry points follow the same parameter contract. + */ +class SopEngineImplTest { + + @Test + void executeSyncShouldRejectMissingRequiredParameter() { + SopEngineImpl engine = new SopEngineImpl(List.of(new RecordingExecutor())); + + SopResult result = engine.executeSync(definition(requiredParameter()), Map.of()); + + assertEquals("FAILED", result.getStatus()); + assertTrue(result.getError().contains("monitorId")); + assertTrue(result.getSteps().isEmpty()); + } + + @Test + void executeSyncShouldApplyDefaultParameter() { + AtomicReference<Map<String, Object>> capturedContext = new AtomicReference<>(); + SopEngineImpl engine = new SopEngineImpl(List.of(new RecordingExecutor(capturedContext))); + + SopResult result = engine.executeSync(definition(defaultParameter()), Map.of()); + + assertEquals("SUCCESS", result.getStatus()); + assertEquals("10", capturedContext.get().get("limit")); + assertEquals("zh", capturedContext.get().get("_language")); + assertEquals("10", result.getData().get("limit")); + } + + @Test + void executeStreamShouldApplyTheSameParameterRules() { + AtomicReference<Map<String, Object>> capturedContext = new AtomicReference<>(); + SopEngineImpl engine = new SopEngineImpl(List.of(new RecordingExecutor(capturedContext))); + + List<String> events = engine.execute(definition(defaultParameter()), null).collectList().block(); + + assertNotNull(events); + assertTrue(events.getLast().contains("completed successfully")); + assertEquals("10", capturedContext.get().get("limit")); + } + + private SopDefinition definition(SopParameter parameter) { + return SopDefinition.builder() + .name("test-sop") + .version("1.0") + .parameters(List.of(parameter)) + .steps(List.of(SopStep.builder().id("query").type("tool").build())) + .build(); + } + + private SopParameter requiredParameter() { + return SopParameter.builder().name("monitorId").type("long").required(true).build(); + } + + private SopParameter defaultParameter() { + return SopParameter.builder().name("limit").type("integer").defaultValue("10").build(); + } + + private static final class RecordingExecutor implements SopExecutor { + + private final AtomicReference<Map<String, Object>> capturedContext; + + private RecordingExecutor() { + this(new AtomicReference<>()); + } + + private RecordingExecutor(AtomicReference<Map<String, Object>> capturedContext) { + this.capturedContext = capturedContext; + } + + @Override + public boolean support(String type) { + return "tool".equals(type); + } + + @Override + public Object execute(SopStep step, Map<String, Object> context) { + capturedContext.set(Map.copyOf(context)); + return "ok"; + } + } +} diff --git a/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/registry/SopToolCallbackTest.java b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/registry/SopToolCallbackTest.java new file mode 100644 index 0000000000..0c81533410 --- /dev/null +++ b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/registry/SopToolCallbackTest.java @@ -0,0 +1,119 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hertzbeat.ai.sop.registry; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; +import org.apache.hertzbeat.ai.sop.engine.SopEngine; +import org.apache.hertzbeat.ai.sop.model.OutputType; +import org.apache.hertzbeat.ai.sop.model.SopDefinition; +import org.apache.hertzbeat.ai.sop.model.SopParameter; +import org.apache.hertzbeat.ai.sop.model.SopResult; +import org.apache.hertzbeat.common.util.JsonUtil; +import org.junit.jupiter.api.Test; +import reactor.core.publisher.Flux; + +/** + * Verifies the input contract and execution behavior of a SOP exposed as a Spring AI tool. + */ +class SopToolCallbackTest { + + @Test + @SuppressWarnings("unchecked") + void schemaShouldDeclareRequiredParametersAndEscapeDescriptions() { + SopDefinition definition = definition(); + SopToolCallback callback = new SopToolCallback(definition, new RecordingEngine()); + + Map<String, Object> schema = JsonUtil.fromJson(callback.getToolDefinition().inputSchema(), Map.class); + Map<String, Object> properties = (Map<String, Object>) schema.get("properties"); + Map<String, Object> monitorId = (Map<String, Object>) properties.get("monitorId"); + Map<String, Object> limit = (Map<String, Object>) properties.get("limit"); + + assertEquals(List.of("monitorId"), schema.get("required")); + assertEquals(false, schema.get("additionalProperties")); + assertEquals("integer", monitorId.get("type")); + assertEquals("包含 \"引号\" 的说明", monitorId.get("description")); + assertEquals(10, limit.get("default")); + } + + @Test + void callShouldParseArgumentsAndExecuteSop() { + RecordingEngine engine = new RecordingEngine(); + SopToolCallback callback = new SopToolCallback(definition(), engine); + + String response = callback.call("{\"monitorId\":42}"); + + assertEquals(42, engine.inputParams.get().get("monitorId")); + assertTrue(response.contains("SUCCESS")); + assertFalse(response.contains("execution started")); + } + + @Test + void callShouldRejectInvalidJson() { + SopToolCallback callback = new SopToolCallback(definition(), new RecordingEngine()); + + assertThrows(IllegalArgumentException.class, () -> callback.call("not-json")); + } + + private SopDefinition definition() { + SopParameter monitorId = SopParameter.builder() + .name("monitorId") + .type("long") + .description("包含 \"引号\" 的说明") + .required(true) + .build(); + SopParameter limit = SopParameter.builder() + .name("limit") + .type("integer") + .defaultValue("10") + .build(); + return SopDefinition.builder() + .name("diagnose") + .description("诊断工具") + .version("1.0") + .parameters(List.of(monitorId, limit)) + .build(); + } + + private static final class RecordingEngine implements SopEngine { + + private final AtomicReference<Map<String, Object>> inputParams = new AtomicReference<>(); + + @Override + public Flux<String> execute(SopDefinition definition, Map<String, Object> inputParams) { + return Flux.empty(); + } + + @Override + public SopResult executeSync(SopDefinition definition, Map<String, Object> inputParams) { + this.inputParams.set(inputParams); + return SopResult.builder() + .sopName(definition.getName()) + .sopVersion(definition.getVersion()) + .status("SUCCESS") + .outputType(OutputType.SIMPLE) + .build(); + } + } +} diff --git a/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/registry/ToolRegistryTest.java b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/registry/ToolRegistryTest.java new file mode 100644 index 0000000000..c949877762 --- /dev/null +++ b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/registry/ToolRegistryTest.java @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hertzbeat.ai.sop.registry; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.lang.reflect.Method; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.springframework.ai.tool.annotation.Tool; +import org.springframework.ai.tool.annotation.ToolParam; + +/** + * Verifies parameter validation and type conversion for registered tool methods. + */ +class ToolRegistryTest { + + @Test + void invokeShouldRejectMissingRequiredParameter() throws NoSuchMethodException { + ToolRegistry.ToolMethod toolMethod = toolMethod(); + + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, + () -> toolMethod.invoke(Map.of())); + + assertEquals("Required tool parameter is missing: monitorId", error.getMessage()); + } + + @Test + void invokeShouldConvertNumericParameter() throws NoSuchMethodException { + ToolRegistry.ToolMethod toolMethod = toolMethod(); + + assertEquals("42", toolMethod.invoke(Map.of("monitorId", "42"))); + } + + private ToolRegistry.ToolMethod toolMethod() throws NoSuchMethodException { + Method method = ToolFixture.class.getMethod("query", Long.class); + return new ToolRegistry.ToolMethod(new ToolFixture(), method, method.getAnnotation(Tool.class)); + } + + private static final class ToolFixture { + + @Tool(name = "query", description = "测试工具") + public String query(@ToolParam(description = "监控 ID", required = true) Long monitorId) { + return String.valueOf(monitorId); + } + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
