mirror of
https://gitee.com/mateos/mateclaw.git
synced 2026-09-15 20:08:18 +08:00
fix(cron): persist the delivery channel and target when a job is edited
This commit is contained in:
parent
ed7f6f5c19
commit
e8d5e73825
@ -64,7 +64,12 @@ public class CronJobEntity {
|
|||||||
* RFC-063r §2.9: originating channel binding. Null when this job was
|
* RFC-063r §2.9: originating channel binding. Null when this job was
|
||||||
* created from the web (no proactive delivery target). The single
|
* created from the web (no proactive delivery target). The single
|
||||||
* indexed column lets ops query "all jobs delivering to channel X".
|
* indexed column lets ops query "all jobs delivering to channel X".
|
||||||
|
*
|
||||||
|
* <p>{@code FieldStrategy.ALWAYS} so clearing the binding from the edit
|
||||||
|
* form actually writes NULL — the default NOT_NULL strategy drops the
|
||||||
|
* column from the UPDATE and the old channel silently survives.
|
||||||
*/
|
*/
|
||||||
|
@TableField(updateStrategy = FieldStrategy.ALWAYS)
|
||||||
private Long channelId;
|
private Long channelId;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -72,7 +77,7 @@ public class CronJobEntity {
|
|||||||
* persisted as JSON via MyBatis Plus JacksonTypeHandler so future fields
|
* persisted as JSON via MyBatis Plus JacksonTypeHandler so future fields
|
||||||
* don't require schema migrations.
|
* don't require schema migrations.
|
||||||
*/
|
*/
|
||||||
@TableField(typeHandler = JacksonTypeHandler.class)
|
@TableField(typeHandler = JacksonTypeHandler.class, updateStrategy = FieldStrategy.ALWAYS)
|
||||||
private DeliveryConfig deliveryConfig;
|
private DeliveryConfig deliveryConfig;
|
||||||
|
|
||||||
@TableField(fill = FieldFill.INSERT)
|
@TableField(fill = FieldFill.INSERT)
|
||||||
|
|||||||
@ -1,8 +1,12 @@
|
|||||||
package vip.mate.cron.repository;
|
package vip.mate.cron.repository;
|
||||||
|
|
||||||
import com.baomidou.mybatisplus.core.mapper.BaseMapper;
|
import com.baomidou.mybatisplus.core.mapper.BaseMapper;
|
||||||
|
import com.baomidou.mybatisplus.extension.handlers.JacksonTypeHandler;
|
||||||
import org.apache.ibatis.annotations.Mapper;
|
import org.apache.ibatis.annotations.Mapper;
|
||||||
import org.apache.ibatis.annotations.Param;
|
import org.apache.ibatis.annotations.Param;
|
||||||
|
import org.apache.ibatis.annotations.Result;
|
||||||
|
import org.apache.ibatis.annotations.ResultMap;
|
||||||
|
import org.apache.ibatis.annotations.Results;
|
||||||
import org.apache.ibatis.annotations.Select;
|
import org.apache.ibatis.annotations.Select;
|
||||||
import vip.mate.cron.model.CronJobEntity;
|
import vip.mate.cron.model.CronJobEntity;
|
||||||
|
|
||||||
@ -31,6 +35,18 @@ public interface CronJobMapper extends BaseMapper<CronJobEntity> {
|
|||||||
* <p>Filters out logically-deleted rows and orders by create_time DESC
|
* <p>Filters out logically-deleted rows and orders by create_time DESC
|
||||||
* to mirror the existing {@code list()} ordering.
|
* to mirror the existing {@code list()} ordering.
|
||||||
*/
|
*/
|
||||||
|
// Shared result map for every hand-written query in this mapper. The
|
||||||
|
// typeHandler declared on CronJobEntity.deliveryConfig only reaches the
|
||||||
|
// result map MyBatis Plus generates for the injected BaseMapper methods;
|
||||||
|
// annotation-driven statements build their own, and auto-mapping finds no
|
||||||
|
// handler for the DeliveryConfig record, so MyBatis silently skips the
|
||||||
|
// column (default AutoMappingUnknownColumnBehavior.NONE) and every job
|
||||||
|
// read through these queries came back with a null deliveryConfig.
|
||||||
|
// Restating the handler here fixes it — the other columns still auto-map.
|
||||||
|
@Results(id = "cronJobResultMap", value = {
|
||||||
|
@Result(column = "delivery_config", property = "deliveryConfig",
|
||||||
|
typeHandler = JacksonTypeHandler.class)
|
||||||
|
})
|
||||||
@Select("""
|
@Select("""
|
||||||
SELECT j.*,
|
SELECT j.*,
|
||||||
(SELECT r.delivery_status FROM mate_cron_job_run r
|
(SELECT r.delivery_status FROM mate_cron_job_run r
|
||||||
@ -51,6 +67,7 @@ public interface CronJobMapper extends BaseMapper<CronJobEntity> {
|
|||||||
* (cross-workspace access returns null → caller throws not_found, matching
|
* (cross-workspace access returns null → caller throws not_found, matching
|
||||||
* the "deleted" shape so workspace existence isn't enumerable).
|
* the "deleted" shape so workspace existence isn't enumerable).
|
||||||
*/
|
*/
|
||||||
|
@ResultMap("cronJobResultMap")
|
||||||
@Select("""
|
@Select("""
|
||||||
SELECT j.*,
|
SELECT j.*,
|
||||||
(SELECT r.delivery_status FROM mate_cron_job_run r
|
(SELECT r.delivery_status FROM mate_cron_job_run r
|
||||||
@ -70,6 +87,7 @@ public interface CronJobMapper extends BaseMapper<CronJobEntity> {
|
|||||||
* toggle / runNow). Skips the delivery-status subquery — those paths
|
* toggle / runNow). Skips the delivery-status subquery — those paths
|
||||||
* don't need it and pay for the correlated lookup otherwise.
|
* don't need it and pay for the correlated lookup otherwise.
|
||||||
*/
|
*/
|
||||||
|
@ResultMap("cronJobResultMap")
|
||||||
@Select("SELECT * FROM mate_cron_job WHERE id = #{id} AND deleted = 0 AND workspace_id = #{workspaceId}")
|
@Select("SELECT * FROM mate_cron_job WHERE id = #{id} AND deleted = 0 AND workspace_id = #{workspaceId}")
|
||||||
CronJobEntity selectByIdAndWorkspace(@Param("id") Long id,
|
CronJobEntity selectByIdAndWorkspace(@Param("id") Long id,
|
||||||
@Param("workspaceId") Long workspaceId);
|
@Param("workspaceId") Long workspaceId);
|
||||||
|
|||||||
@ -353,6 +353,13 @@ public class CronJobService implements ApplicationRunner {
|
|||||||
existing.setTaskType(dto.getTaskType());
|
existing.setTaskType(dto.getTaskType());
|
||||||
existing.setTriggerMessage(dto.getTriggerMessage());
|
existing.setTriggerMessage(dto.getTriggerMessage());
|
||||||
existing.setRequestBody(dto.getRequestBody());
|
existing.setRequestBody(dto.getRequestBody());
|
||||||
|
// The edit form always submits the full delivery binding (channel +
|
||||||
|
// target + suppress flag), so the request is authoritative for both
|
||||||
|
// fields — including a null pair, which means "unbind this job from
|
||||||
|
// its channel". FieldStrategy.ALWAYS on the entity lets the null
|
||||||
|
// through to the UPDATE.
|
||||||
|
existing.setChannelId(dto.getChannelId());
|
||||||
|
existing.setDeliveryConfig(dto.getDeliveryConfig());
|
||||||
if (dto.getEnabled() != null) {
|
if (dto.getEnabled() != null) {
|
||||||
existing.setEnabled(dto.getEnabled());
|
existing.setEnabled(dto.getEnabled());
|
||||||
}
|
}
|
||||||
|
|||||||
@ -10,7 +10,9 @@ import org.springframework.lang.Nullable;
|
|||||||
import org.springframework.stereotype.Component;
|
import org.springframework.stereotype.Component;
|
||||||
import vip.mate.agent.context.ChatOrigin;
|
import vip.mate.agent.context.ChatOrigin;
|
||||||
import vip.mate.cron.model.CronJobDTO;
|
import vip.mate.cron.model.CronJobDTO;
|
||||||
|
import vip.mate.cron.model.DeliveryConfig;
|
||||||
import vip.mate.cron.service.CronJobService;
|
import vip.mate.cron.service.CronJobService;
|
||||||
|
import vip.mate.tool.ConcurrencyUnsafe;
|
||||||
|
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
import java.util.LinkedHashMap;
|
import java.util.LinkedHashMap;
|
||||||
@ -25,7 +27,7 @@ import java.util.Map;
|
|||||||
* from natural language (e.g. "every day at 9am" → "0 9 * * *").
|
* from natural language (e.g. "every day at 9am" → "0 9 * * *").
|
||||||
*
|
*
|
||||||
* @author MateClaw Team
|
* @author MateClaw Team
|
||||||
* @see vip.mate.cron.service.CronJobService
|
* @see CronJobService
|
||||||
*/
|
*/
|
||||||
@Slf4j
|
@Slf4j
|
||||||
@Component
|
@Component
|
||||||
@ -42,7 +44,7 @@ public class CronJobTool {
|
|||||||
*/
|
*/
|
||||||
private final ObjectMapper objectMapper;
|
private final ObjectMapper objectMapper;
|
||||||
|
|
||||||
@vip.mate.tool.ConcurrencyUnsafe("cron job creation persists to mate_cron_job; concurrent creates can race on name")
|
@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 — "
|
@Tool(description = "Create a scheduled task that asks the agent to do something at a specific time — "
|
||||||
+ "the trigger message is sent to the LLM, which can use tools (search, weather, etc.) to produce the answer. "
|
+ "the trigger message is sent to the LLM, which can use tools (search, weather, etc.) to produce the answer. "
|
||||||
+ "Use this for queries like 'every morning give me a weather report' or 'daily news summary'. "
|
+ "Use this for queries like 'every morning give me a weather report' or 'daily news summary'. "
|
||||||
@ -119,7 +121,7 @@ public class CronJobTool {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@vip.mate.tool.ConcurrencyUnsafe("reminder creation persists to mate_cron_job; concurrent creates can race on name")
|
@ConcurrencyUnsafe("reminder creation persists to mate_cron_job; concurrent creates can race on name")
|
||||||
@Tool(description = "Create a scheduled REMINDER. The reminder text is delivered to the user verbatim at the "
|
@Tool(description = "Create a scheduled REMINDER. The reminder text is delivered to the user verbatim at the "
|
||||||
+ "scheduled time — no LLM call, no rephrasing, no token cost. "
|
+ "scheduled time — no LLM call, no rephrasing, no token cost. "
|
||||||
+ "Use this when the user wants a notification with specific content (e.g. 'remind me at 3pm to leave for the meeting' → "
|
+ "Use this when the user wants a notification with specific content (e.g. 'remind me at 3pm to leave for the meeting' → "
|
||||||
@ -211,7 +213,7 @@ public class CronJobTool {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@vip.mate.tool.ConcurrencyUnsafe("toggles row state in mate_cron_job; serialize to keep enabled/disabled deterministic")
|
@ConcurrencyUnsafe("toggles row state in mate_cron_job; serialize to keep enabled/disabled deterministic")
|
||||||
@Tool(description = "Enable or disable a scheduled task by its job ID. "
|
@Tool(description = "Enable or disable a scheduled task by its job ID. "
|
||||||
+ "Use list_cron_jobs first to find the job ID.")
|
+ "Use list_cron_jobs first to find the job ID.")
|
||||||
public String toggle_cron_job(
|
public String toggle_cron_job(
|
||||||
@ -236,7 +238,7 @@ public class CronJobTool {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@vip.mate.tool.ConcurrencyUnsafe("destructive — removes row from mate_cron_job")
|
@ConcurrencyUnsafe("destructive — removes row from mate_cron_job")
|
||||||
@Tool(description = "Delete a scheduled task by its job ID. This action requires user approval. "
|
@Tool(description = "Delete a scheduled task by its job ID. This action requires user approval. "
|
||||||
+ "Use list_cron_jobs first to find the job ID.")
|
+ "Use list_cron_jobs first to find the job ID.")
|
||||||
public String delete_cron_job(
|
public String delete_cron_job(
|
||||||
@ -306,7 +308,7 @@ public class CronJobTool {
|
|||||||
// store it as session.targetId, which never equals the cron's own
|
// store it as session.targetId, which never equals the cron's own
|
||||||
// chatId/senderId-derived targetId — the senderId match is the
|
// chatId/senderId-derived targetId — the senderId match is the
|
||||||
// stable common key.
|
// stable common key.
|
||||||
dto.setDeliveryConfig(vip.mate.cron.model.DeliveryConfig.from(
|
dto.setDeliveryConfig(DeliveryConfig.from(
|
||||||
origin.channelTarget(), origin.requesterId()));
|
origin.channelTarget(), origin.requesterId()));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@ -0,0 +1,123 @@
|
|||||||
|
package vip.mate.cron.service;
|
||||||
|
|
||||||
|
import org.junit.jupiter.api.DisplayName;
|
||||||
|
import org.junit.jupiter.api.Test;
|
||||||
|
import org.springframework.beans.factory.annotation.Autowired;
|
||||||
|
import org.springframework.boot.test.context.SpringBootTest;
|
||||||
|
import org.springframework.test.context.TestPropertySource;
|
||||||
|
import vip.mate.MateClawApplication;
|
||||||
|
import vip.mate.cron.model.CronJobDTO;
|
||||||
|
import vip.mate.cron.model.DeliveryConfig;
|
||||||
|
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertNull;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Regression coverage for the delivery binding on the cron CRUD path:
|
||||||
|
* {@code channelId} and {@code deliveryConfig} must survive a create, be
|
||||||
|
* readable back through the list/detail queries, and be mutable through
|
||||||
|
* {@code update()} — including clearing the binding entirely.
|
||||||
|
*/
|
||||||
|
@SpringBootTest(
|
||||||
|
classes = MateClawApplication.class,
|
||||||
|
webEnvironment = SpringBootTest.WebEnvironment.NONE
|
||||||
|
)
|
||||||
|
@TestPropertySource(properties = {
|
||||||
|
"spring.datasource.url=jdbc:h2:mem:cron_delivery_test_${random.uuid};MODE=MySQL;DATABASE_TO_LOWER=TRUE;CASE_INSENSITIVE_IDENTIFIERS=TRUE;DB_CLOSE_DELAY=-1",
|
||||||
|
"spring.ai.dashscope.api-key=test-key",
|
||||||
|
"spring.main.web-application-type=none"
|
||||||
|
})
|
||||||
|
class CronJobDeliveryPersistenceTest {
|
||||||
|
|
||||||
|
private static final long WORKSPACE_ID = 1L;
|
||||||
|
|
||||||
|
@Autowired
|
||||||
|
private CronJobService cronJobService;
|
||||||
|
|
||||||
|
private CronJobDTO newDto(String name, Long channelId, DeliveryConfig deliveryConfig) {
|
||||||
|
CronJobDTO dto = new CronJobDTO();
|
||||||
|
dto.setName(name);
|
||||||
|
dto.setCronExpression("*/5 * * * *");
|
||||||
|
dto.setTimezone("Asia/Shanghai");
|
||||||
|
dto.setAgentId(9001L);
|
||||||
|
dto.setTaskType("text");
|
||||||
|
dto.setTriggerMessage("ping");
|
||||||
|
dto.setEnabled(false);
|
||||||
|
dto.setChannelId(channelId);
|
||||||
|
dto.setDeliveryConfig(deliveryConfig);
|
||||||
|
return dto;
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@DisplayName("create persists the delivery binding and the read path returns it")
|
||||||
|
void createPersistsDeliveryBinding() {
|
||||||
|
CronJobDTO created = cronJobService.create(
|
||||||
|
newDto("delivery-create", 7001L,
|
||||||
|
new DeliveryConfig("target-1", null, null, "user-1", Boolean.FALSE)),
|
||||||
|
WORKSPACE_ID);
|
||||||
|
|
||||||
|
CronJobDTO loaded = cronJobService.getById(created.getId(), WORKSPACE_ID);
|
||||||
|
assertEquals(7001L, loaded.getChannelId());
|
||||||
|
assertNotNull(loaded.getDeliveryConfig(), "deliveryConfig must round-trip through the detail query");
|
||||||
|
assertEquals("target-1", loaded.getDeliveryConfig().targetId());
|
||||||
|
assertEquals("user-1", loaded.getDeliveryConfig().userId());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@DisplayName("update rewrites channelId and deliveryConfig")
|
||||||
|
void updateRewritesDeliveryBinding() {
|
||||||
|
CronJobDTO created = cronJobService.create(
|
||||||
|
newDto("delivery-update", 7001L,
|
||||||
|
new DeliveryConfig("target-1", null, null, "user-1", Boolean.FALSE)),
|
||||||
|
WORKSPACE_ID);
|
||||||
|
|
||||||
|
CronJobDTO patch = newDto("delivery-update", 7002L,
|
||||||
|
new DeliveryConfig("target-2", null, null, "user-2", Boolean.TRUE));
|
||||||
|
cronJobService.update(created.getId(), patch, WORKSPACE_ID);
|
||||||
|
|
||||||
|
CronJobDTO loaded = cronJobService.getById(created.getId(), WORKSPACE_ID);
|
||||||
|
assertEquals(7002L, loaded.getChannelId(), "channel rebinding must persist");
|
||||||
|
assertNotNull(loaded.getDeliveryConfig());
|
||||||
|
assertEquals("target-2", loaded.getDeliveryConfig().targetId());
|
||||||
|
assertEquals("user-2", loaded.getDeliveryConfig().userId());
|
||||||
|
assertTrue(loaded.getDeliveryConfig().isAgentReplySuppressed(),
|
||||||
|
"suppressAgentReply toggle must persist");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@DisplayName("toggle preserves the delivery binding")
|
||||||
|
void togglePreservesDeliveryBinding() {
|
||||||
|
CronJobDTO created = cronJobService.create(
|
||||||
|
newDto("delivery-toggle", 7001L,
|
||||||
|
new DeliveryConfig("target-1", null, null, "user-1", Boolean.TRUE)),
|
||||||
|
WORKSPACE_ID);
|
||||||
|
|
||||||
|
cronJobService.toggle(created.getId(), true, WORKSPACE_ID);
|
||||||
|
cronJobService.toggle(created.getId(), false, WORKSPACE_ID);
|
||||||
|
|
||||||
|
CronJobDTO loaded = cronJobService.getById(created.getId(), WORKSPACE_ID);
|
||||||
|
assertEquals(7001L, loaded.getChannelId());
|
||||||
|
assertNotNull(loaded.getDeliveryConfig(),
|
||||||
|
"enable/disable must not wipe the delivery binding");
|
||||||
|
assertEquals("target-1", loaded.getDeliveryConfig().targetId());
|
||||||
|
assertTrue(loaded.getDeliveryConfig().isAgentReplySuppressed());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@DisplayName("update can clear the delivery binding")
|
||||||
|
void updateClearsDeliveryBinding() {
|
||||||
|
CronJobDTO created = cronJobService.create(
|
||||||
|
newDto("delivery-clear", 7001L,
|
||||||
|
new DeliveryConfig("target-1", null, null, "user-1", Boolean.FALSE)),
|
||||||
|
WORKSPACE_ID);
|
||||||
|
|
||||||
|
CronJobDTO patch = newDto("delivery-clear", null, null);
|
||||||
|
cronJobService.update(created.getId(), patch, WORKSPACE_ID);
|
||||||
|
|
||||||
|
CronJobDTO loaded = cronJobService.getById(created.getId(), WORKSPACE_ID);
|
||||||
|
assertNull(loaded.getChannelId(), "unbinding a channel must persist");
|
||||||
|
assertNull(loaded.getDeliveryConfig(), "clearing the delivery target must persist");
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue
Block a user