mirror of
https://gitee.com/mateos/mateclaw.git
synced 2026-09-13 03:13:41 +08:00
fix(tool): keep Snowflake ids precise across the tool boundary (#319)
This commit is contained in:
parent
bad216d686
commit
5235e30138
@ -1,8 +1,6 @@
|
||||
package vip.mate.tool.builtin;
|
||||
|
||||
import cn.hutool.json.JSONArray;
|
||||
import cn.hutool.json.JSONObject;
|
||||
import cn.hutool.json.JSONUtil;
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
import lombok.RequiredArgsConstructor;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.springframework.ai.chat.model.ToolContext;
|
||||
@ -14,7 +12,10 @@ import vip.mate.agent.context.ChatOrigin;
|
||||
import vip.mate.cron.model.CronJobDTO;
|
||||
import vip.mate.cron.service.CronJobService;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
/**
|
||||
* Built-in tool: scheduled task (cron job) management via chat.
|
||||
@ -32,6 +33,14 @@ import java.util.List;
|
||||
public class CronJobTool {
|
||||
|
||||
private final CronJobService cronJobService;
|
||||
/**
|
||||
* The application ObjectMapper serializes every {@code Long} as a JSON string
|
||||
* (see {@code JacksonConfig}). Tool output goes through it so a 19-digit
|
||||
* Snowflake {@code jobId} reaches the model as a string — never a JSON number
|
||||
* that loses its low digits in a double / JS Number round-trip on the way
|
||||
* back into toggle_cron_job / delete_cron_job.
|
||||
*/
|
||||
private final ObjectMapper objectMapper;
|
||||
|
||||
@vip.mate.tool.ConcurrencyUnsafe("cron job creation persists to mate_cron_job; concurrent creates can race on name")
|
||||
@Tool(description = "Create a scheduled task that asks the agent to do something at a specific time — "
|
||||
@ -94,15 +103,15 @@ public class CronJobTool {
|
||||
Long workspaceId = origin.workspaceId() != null ? origin.workspaceId() : 1L;
|
||||
CronJobDTO created = cronJobService.create(dto, workspaceId);
|
||||
|
||||
JSONObject result = new JSONObject();
|
||||
result.set("success", true);
|
||||
result.set("jobId", created.getId());
|
||||
result.set("name", created.getName());
|
||||
result.set("cronExpression", created.getCronExpression());
|
||||
result.set("timezone", created.getTimezone());
|
||||
result.set("nextRunTime", created.getNextRunTime() != null ? created.getNextRunTime().toString() : "");
|
||||
result.set("enabled", created.getEnabled());
|
||||
return JSONUtil.toJsonPrettyStr(result);
|
||||
Map<String, Object> result = new LinkedHashMap<>();
|
||||
result.put("success", true);
|
||||
result.put("jobId", created.getId());
|
||||
result.put("name", created.getName());
|
||||
result.put("cronExpression", created.getCronExpression());
|
||||
result.put("timezone", created.getTimezone());
|
||||
result.put("nextRunTime", created.getNextRunTime() != null ? created.getNextRunTime().toString() : "");
|
||||
result.put("enabled", created.getEnabled());
|
||||
return writeJson(result);
|
||||
|
||||
} catch (Exception e) {
|
||||
log.error("[CronJobTool] create failed: {}", e.getMessage());
|
||||
@ -154,16 +163,16 @@ public class CronJobTool {
|
||||
Long workspaceId = origin.workspaceId() != null ? origin.workspaceId() : 1L;
|
||||
CronJobDTO created = cronJobService.create(dto, workspaceId);
|
||||
|
||||
JSONObject result = new JSONObject();
|
||||
result.set("success", true);
|
||||
result.set("jobId", created.getId());
|
||||
result.set("name", created.getName());
|
||||
result.set("taskType", "reminder");
|
||||
result.set("cronExpression", created.getCronExpression());
|
||||
result.set("timezone", created.getTimezone());
|
||||
result.set("nextRunTime", created.getNextRunTime() != null ? created.getNextRunTime().toString() : "");
|
||||
result.set("enabled", created.getEnabled());
|
||||
return JSONUtil.toJsonPrettyStr(result);
|
||||
Map<String, Object> result = new LinkedHashMap<>();
|
||||
result.put("success", true);
|
||||
result.put("jobId", created.getId());
|
||||
result.put("name", created.getName());
|
||||
result.put("taskType", "reminder");
|
||||
result.put("cronExpression", created.getCronExpression());
|
||||
result.put("timezone", created.getTimezone());
|
||||
result.put("nextRunTime", created.getNextRunTime() != null ? created.getNextRunTime().toString() : "");
|
||||
result.put("enabled", created.getEnabled());
|
||||
return writeJson(result);
|
||||
|
||||
} catch (Exception e) {
|
||||
log.error("[CronJobTool] create_reminder failed: {}", e.getMessage());
|
||||
@ -179,23 +188,23 @@ public class CronJobTool {
|
||||
// sees the cron jobs of the workspace it's running in.
|
||||
Long workspaceId = workspaceFromContext(ctx);
|
||||
List<CronJobDTO> jobs = cronJobService.list(workspaceId);
|
||||
JSONArray arr = new JSONArray();
|
||||
List<Map<String, Object>> arr = new ArrayList<>();
|
||||
for (CronJobDTO job : jobs) {
|
||||
JSONObject obj = new JSONObject();
|
||||
obj.set("jobId", job.getId());
|
||||
obj.set("name", job.getName());
|
||||
obj.set("cronExpression", job.getCronExpression());
|
||||
obj.set("timezone", job.getTimezone());
|
||||
obj.set("enabled", job.getEnabled());
|
||||
obj.set("nextRunTime", job.getNextRunTime() != null ? job.getNextRunTime().toString() : "");
|
||||
obj.set("lastRunTime", job.getLastRunTime() != null ? job.getLastRunTime().toString() : "");
|
||||
obj.set("agentName", job.getAgentName());
|
||||
Map<String, Object> obj = new LinkedHashMap<>();
|
||||
obj.put("jobId", job.getId());
|
||||
obj.put("name", job.getName());
|
||||
obj.put("cronExpression", job.getCronExpression());
|
||||
obj.put("timezone", job.getTimezone());
|
||||
obj.put("enabled", job.getEnabled());
|
||||
obj.put("nextRunTime", job.getNextRunTime() != null ? job.getNextRunTime().toString() : "");
|
||||
obj.put("lastRunTime", job.getLastRunTime() != null ? job.getLastRunTime().toString() : "");
|
||||
obj.put("agentName", job.getAgentName());
|
||||
arr.add(obj);
|
||||
}
|
||||
JSONObject result = new JSONObject();
|
||||
result.set("totalJobs", jobs.size());
|
||||
result.set("jobs", arr);
|
||||
return JSONUtil.toJsonPrettyStr(result);
|
||||
Map<String, Object> result = new LinkedHashMap<>();
|
||||
result.put("totalJobs", jobs.size());
|
||||
result.put("jobs", arr);
|
||||
return writeJson(result);
|
||||
} catch (Exception e) {
|
||||
log.error("[CronJobTool] list failed: {}", e.getMessage());
|
||||
return errorResult("Failed to list cron jobs: " + e.getMessage());
|
||||
@ -214,13 +223,13 @@ public class CronJobTool {
|
||||
Long workspaceId = workspaceFromContext(ctx);
|
||||
cronJobService.toggle(jobId, enabled, workspaceId);
|
||||
CronJobDTO updated = cronJobService.getById(jobId, workspaceId);
|
||||
JSONObject result = new JSONObject();
|
||||
result.set("success", true);
|
||||
result.set("jobId", jobId);
|
||||
result.set("name", updated.getName());
|
||||
result.set("enabled", updated.getEnabled());
|
||||
result.set("nextRunTime", updated.getNextRunTime() != null ? updated.getNextRunTime().toString() : "");
|
||||
return JSONUtil.toJsonPrettyStr(result);
|
||||
Map<String, Object> result = new LinkedHashMap<>();
|
||||
result.put("success", true);
|
||||
result.put("jobId", jobId);
|
||||
result.put("name", updated.getName());
|
||||
result.put("enabled", updated.getEnabled());
|
||||
result.put("nextRunTime", updated.getNextRunTime() != null ? updated.getNextRunTime().toString() : "");
|
||||
return writeJson(result);
|
||||
} catch (Exception e) {
|
||||
log.error("[CronJobTool] toggle failed: {}", e.getMessage());
|
||||
return errorResult("Failed to toggle cron job: " + e.getMessage());
|
||||
@ -239,10 +248,10 @@ public class CronJobTool {
|
||||
CronJobDTO job = cronJobService.getById(jobId, workspaceId);
|
||||
String jobName = job.getName();
|
||||
cronJobService.delete(jobId, workspaceId);
|
||||
JSONObject result = new JSONObject();
|
||||
result.set("success", true);
|
||||
result.set("deleted", jobName);
|
||||
return JSONUtil.toJsonPrettyStr(result);
|
||||
Map<String, Object> result = new LinkedHashMap<>();
|
||||
result.put("success", true);
|
||||
result.put("deleted", jobName);
|
||||
return writeJson(result);
|
||||
} catch (Exception e) {
|
||||
log.error("[CronJobTool] delete failed: {}", e.getMessage());
|
||||
return errorResult("Failed to delete cron job: " + e.getMessage());
|
||||
@ -250,10 +259,25 @@ public class CronJobTool {
|
||||
}
|
||||
|
||||
private String errorResult(String message) {
|
||||
JSONObject result = new JSONObject();
|
||||
result.set("success", false);
|
||||
result.set("error", message);
|
||||
return JSONUtil.toJsonPrettyStr(result);
|
||||
Map<String, Object> result = new LinkedHashMap<>();
|
||||
result.put("success", false);
|
||||
result.put("error", message);
|
||||
return writeJson(result);
|
||||
}
|
||||
|
||||
/**
|
||||
* Serialize tool output through the id-safe application ObjectMapper so every
|
||||
* {@code Long} (notably {@code jobId}) is rendered as a string. Falls back to
|
||||
* a minimal literal on the rare serialization failure rather than throwing
|
||||
* out of a tool call.
|
||||
*/
|
||||
private String writeJson(Object value) {
|
||||
try {
|
||||
return objectMapper.writerWithDefaultPrettyPrinter().writeValueAsString(value);
|
||||
} catch (Exception e) {
|
||||
log.error("[CronJobTool] result serialization failed: {}", e.getMessage());
|
||||
return "{\"success\":false,\"error\":\"result serialization failed\"}";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@ -1,8 +1,8 @@
|
||||
package vip.mate.tool.builtin;
|
||||
|
||||
import cn.hutool.json.JSONArray;
|
||||
import cn.hutool.json.JSONObject;
|
||||
import cn.hutool.json.JSONUtil;
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
import lombok.RequiredArgsConstructor;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.springframework.ai.tool.annotation.Tool;
|
||||
@ -14,7 +14,9 @@ import vip.mate.datasource.service.DatasourceService;
|
||||
|
||||
import java.sql.*;
|
||||
import java.util.ArrayList;
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
/**
|
||||
@ -32,6 +34,14 @@ public class DatasourceTool {
|
||||
|
||||
private final DatasourceService datasourceService;
|
||||
private final DatasourceConnectionManager connectionManager;
|
||||
/**
|
||||
* The application ObjectMapper, which serializes every {@code Long} as a JSON
|
||||
* string (see {@code JacksonConfig}). Tool output goes through it so 19-digit
|
||||
* Snowflake ids reach the model as strings — exactly like the HTTP API — and
|
||||
* never as JSON numbers that lose their low digits in a double/JS-number
|
||||
* round-trip on the way back into a tool call.
|
||||
*/
|
||||
private final ObjectMapper objectMapper;
|
||||
|
||||
/** SQL identifier whitelist: letters, digits, underscore, dot, hyphen only */
|
||||
private static final Pattern SAFE_IDENTIFIER = Pattern.compile("^[a-zA-Z0-9_][a-zA-Z0-9_.\\-]{0,127}$");
|
||||
@ -67,22 +77,25 @@ public class DatasourceTool {
|
||||
}
|
||||
}
|
||||
|
||||
private String listDatasources() {
|
||||
private String listDatasources() throws Exception {
|
||||
List<DatasourceEntity> list = datasourceService.listEnabled();
|
||||
JSONArray arr = new JSONArray();
|
||||
List<Map<String, Object>> rows = new ArrayList<>();
|
||||
for (DatasourceEntity ds : list) {
|
||||
JSONObject obj = new JSONObject();
|
||||
obj.set("id", ds.getId());
|
||||
obj.set("name", ds.getName());
|
||||
obj.set("dbType", ds.getDbType());
|
||||
obj.set("databaseName", ds.getDatabaseName());
|
||||
obj.set("description", ds.getDescription());
|
||||
arr.add(obj);
|
||||
Map<String, Object> obj = new LinkedHashMap<>();
|
||||
// ds.getId() is a Long; the shared ObjectMapper renders it as a JSON
|
||||
// string so the model copies an exact id back into list_tables /
|
||||
// execute_sql / describe_table calls.
|
||||
obj.put("id", ds.getId());
|
||||
obj.put("name", ds.getName());
|
||||
obj.put("dbType", ds.getDbType());
|
||||
obj.put("databaseName", ds.getDatabaseName());
|
||||
obj.put("description", ds.getDescription());
|
||||
rows.add(obj);
|
||||
}
|
||||
JSONObject result = new JSONObject();
|
||||
result.set("datasources", arr);
|
||||
result.set("count", arr.size());
|
||||
return result.toStringPretty();
|
||||
Map<String, Object> result = new LinkedHashMap<>();
|
||||
result.put("datasources", rows);
|
||||
result.put("count", rows.size());
|
||||
return objectMapper.writerWithDefaultPrettyPrinter().writeValueAsString(result);
|
||||
}
|
||||
|
||||
private String listTables(Long datasourceId) throws SQLException {
|
||||
|
||||
@ -0,0 +1,55 @@
|
||||
package vip.mate.tool.builtin;
|
||||
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
import com.fasterxml.jackson.databind.json.JsonMapper;
|
||||
import com.fasterxml.jackson.databind.module.SimpleModule;
|
||||
import com.fasterxml.jackson.databind.ser.std.ToStringSerializer;
|
||||
import org.junit.jupiter.api.DisplayName;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import vip.mate.cron.model.CronJobDTO;
|
||||
import vip.mate.cron.service.CronJobService;
|
||||
|
||||
import java.util.List;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
/**
|
||||
* Issue #319 (same class as the datasource fix): a 19-digit Snowflake {@code jobId}
|
||||
* must reach the model as a JSON string, not a number — otherwise it loses its low
|
||||
* digits when the model copies it back into toggle_cron_job / delete_cron_job and
|
||||
* the wrong (or no) job is hit.
|
||||
*/
|
||||
class CronJobToolIdPrecisionTest {
|
||||
|
||||
private static ObjectMapper idSafeMapper() {
|
||||
SimpleModule m = new SimpleModule();
|
||||
m.addSerializer(Long.class, ToStringSerializer.instance);
|
||||
m.addSerializer(Long.TYPE, ToStringSerializer.instance);
|
||||
return JsonMapper.builder().addModule(m).build();
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("list_cron_jobs emits jobId as a quoted JSON string, never a bare number")
|
||||
void listCronJobs_jobIdIsString() {
|
||||
long bigId = 2064875200729235458L;
|
||||
CronJobDTO job = new CronJobDTO();
|
||||
job.setId(bigId);
|
||||
job.setName("Daily summary");
|
||||
job.setEnabled(true);
|
||||
|
||||
CronJobService service = mock(CronJobService.class);
|
||||
when(service.list(any())).thenReturn(List.of(job));
|
||||
CronJobTool tool = new CronJobTool(service, idSafeMapper());
|
||||
|
||||
String out = tool.list_cron_jobs(null);
|
||||
|
||||
assertTrue(out.contains("\"" + bigId + "\""),
|
||||
"jobId must appear as a quoted string so its 19 digits survive; got: " + out);
|
||||
assertFalse(out.contains(": " + bigId) || out.contains(":" + bigId),
|
||||
"jobId must NOT appear as a bare JSON number");
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,59 @@
|
||||
package vip.mate.tool.builtin;
|
||||
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
import com.fasterxml.jackson.databind.json.JsonMapper;
|
||||
import com.fasterxml.jackson.databind.ser.std.ToStringSerializer;
|
||||
import com.fasterxml.jackson.databind.module.SimpleModule;
|
||||
import org.junit.jupiter.api.DisplayName;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import vip.mate.datasource.model.DatasourceEntity;
|
||||
import vip.mate.datasource.service.DatasourceConnectionManager;
|
||||
import vip.mate.datasource.service.DatasourceService;
|
||||
|
||||
import java.util.List;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
/**
|
||||
* Issue #319: a 19-digit Snowflake datasource id must reach the model as a JSON
|
||||
* string, not a number. As a number it loses its low digits when it round-trips
|
||||
* through a double / JS Number on the way back into a follow-up tool call (and in
|
||||
* the chat UI), so the looked-up datasource is "not found". The tool serializes
|
||||
* through the application ObjectMapper, which renders every Long as a string —
|
||||
* the same id-safety policy the HTTP API already applies.
|
||||
*/
|
||||
class DatasourceToolIdPrecisionTest {
|
||||
|
||||
/** Mirrors the application ObjectMapper: every Long serializes as a string. */
|
||||
private static ObjectMapper idSafeMapper() {
|
||||
SimpleModule m = new SimpleModule();
|
||||
m.addSerializer(Long.class, ToStringSerializer.instance);
|
||||
m.addSerializer(Long.TYPE, ToStringSerializer.instance);
|
||||
return JsonMapper.builder().addModule(m).build();
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("list_datasources emits the id as a quoted JSON string, never a bare number")
|
||||
void listDatasources_idIsString() {
|
||||
long bigId = 2064875200729235458L;
|
||||
DatasourceEntity ds = new DatasourceEntity();
|
||||
ds.setId(bigId);
|
||||
ds.setName("prod-mysql");
|
||||
ds.setDbType("mysql");
|
||||
ds.setDatabaseName("app");
|
||||
|
||||
DatasourceService service = mock(DatasourceService.class);
|
||||
when(service.listEnabled()).thenReturn(List.of(ds));
|
||||
DatasourceTool tool = new DatasourceTool(service, mock(DatasourceConnectionManager.class), idSafeMapper());
|
||||
|
||||
String out = tool.query_datasource("list_datasources", null, null);
|
||||
|
||||
assertTrue(out.contains("\"" + bigId + "\""),
|
||||
"id must appear as a quoted string so its 19 digits survive the round-trip; got: " + out);
|
||||
assertFalse(out.contains(": " + bigId) || out.contains(":" + bigId),
|
||||
"id must NOT appear as a bare JSON number (precision-lossy across double/JS Number)");
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user