mirror of
https://github.com/infiniflow/ragflow.git
synced 2026-07-08 20:34:48 +08:00
fix(go): agent settings update clearing DSL (#16495)
### Summary This PR fixes a Go backend bug where updating agent settings, such as description, could clear the agent DSL. Root cause: PUT /api/v1/agents/:canvas_id only bound the dsl field in Go. When the frontend submitted settings without dsl, the service still updated the canvas with an empty DSL value. Changes: - Treat agent updates as partial patches. - Preserve existing DSL when dsl is not present in the request. - Update only specified user_canvas fields instead of saving the full row. - Add a regression test for settings updates preserving DSL. Test: `go test ./internal/service ./internal/handler` Co-authored-by: Zhichang Yu <yuzhichang@gmail.com>
This commit is contained in:
@@ -494,6 +494,12 @@ func (dao *UserCanvasDAO) UpdateDSL(canvasID string, dsl entity.JSONMap) (int64,
|
||||
return result.RowsAffected, result.Error
|
||||
}
|
||||
|
||||
// UpdateFields updates only the supplied user_canvas columns.
|
||||
func (dao *UserCanvasDAO) UpdateFields(canvasID string, fields map[string]interface{}) (int64, error) {
|
||||
result := DB.Model(&entity.UserCanvas{}).Where("id = ?", canvasID).Updates(fields)
|
||||
return result.RowsAffected, result.Error
|
||||
}
|
||||
|
||||
// UpdateTags updates a canvas's comma-separated tags by canvas ID.
|
||||
func (dao *UserCanvasDAO) UpdateTags(canvasID, tags string) (int64, error) {
|
||||
result := DB.Model(&entity.UserCanvas{}).Where("id = ?", canvasID).Update("tags", tags)
|
||||
|
||||
@@ -306,17 +306,15 @@ func (h *AgentHandler) GetAgent(c *gin.Context) {
|
||||
}
|
||||
|
||||
// updateAgentRequest is the wire shape for PUT /api/v1/agents/:canvas_id.
|
||||
type updateAgentRequest struct {
|
||||
DSL entity.JSONMap `json:"dsl"`
|
||||
}
|
||||
type updateAgentRequest map[string]interface{}
|
||||
|
||||
// UpdateAgent writes a new draft DSL to the canvas (no version created).
|
||||
// @Summary Update Agent (Draft)
|
||||
// UpdateAgent applies a partial update to the canvas draft.
|
||||
// @Summary Update Agent
|
||||
// @Tags agents
|
||||
// @Accept json
|
||||
// @Produce json
|
||||
// @Param canvas_id path string true "canvas id"
|
||||
// @Param request body updateAgentRequest true "draft DSL payload"
|
||||
// @Param request body updateAgentRequest true "agent update payload"
|
||||
// @Success 200 {object} map[string]interface{}
|
||||
// @Router /api/v1/agents/{canvas_id} [put]
|
||||
func (h *AgentHandler) UpdateAgent(c *gin.Context) {
|
||||
@@ -331,7 +329,10 @@ func (h *AgentHandler) UpdateAgent(c *gin.Context) {
|
||||
jsonError(c, common.CodeArgumentError, "Invalid request: "+err.Error())
|
||||
return
|
||||
}
|
||||
if err := h.agentService.UpdateAgent(c.Request.Context(), user.ID, canvasID, req.DSL); err != nil {
|
||||
if req == nil {
|
||||
req = updateAgentRequest{}
|
||||
}
|
||||
if err := h.agentService.UpdateAgent(c.Request.Context(), user.ID, canvasID, map[string]interface{}(req)); err != nil {
|
||||
ec, em := mapAgentError(err)
|
||||
jsonError(c, ec, em)
|
||||
return
|
||||
|
||||
@@ -441,7 +441,7 @@ func (f *fullFakeAgentService) GetAgent(context.Context, string, string) (*entit
|
||||
}
|
||||
return f.getRow, nil
|
||||
}
|
||||
func (f *fullFakeAgentService) UpdateAgent(context.Context, string, string, entity.JSONMap) error {
|
||||
func (f *fullFakeAgentService) UpdateAgent(context.Context, string, string, map[string]interface{}) error {
|
||||
return nil
|
||||
}
|
||||
func (f *fullFakeAgentService) DeleteAgent(context.Context, string, string) error {
|
||||
|
||||
@@ -84,7 +84,7 @@ func (f *waitFakeAgentService) CreateAgent(context.Context, *service.CreateAgent
|
||||
func (f *waitFakeAgentService) GetAgent(context.Context, string, string) (*entity.UserCanvas, error) {
|
||||
return &entity.UserCanvas{ID: "canvas-wait"}, nil
|
||||
}
|
||||
func (f *waitFakeAgentService) UpdateAgent(context.Context, string, string, entity.JSONMap) error {
|
||||
func (f *waitFakeAgentService) UpdateAgent(context.Context, string, string, map[string]interface{}) error {
|
||||
return nil
|
||||
}
|
||||
func (f *waitFakeAgentService) DeleteAgent(context.Context, string, string) error {
|
||||
|
||||
@@ -418,17 +418,40 @@ func (s *AgentService) GetAgent(ctx context.Context, userID, canvasID string) (*
|
||||
return s.loadCanvasForUser(ctx, userID, canvasID)
|
||||
}
|
||||
|
||||
// UpdateAgent writes a new DSL to the draft (user_canvas.dsl) and toggles
|
||||
// release=false. The call does NOT create a new user_canvas_version row —
|
||||
// versions are produced only by PublishAgent.
|
||||
func (s *AgentService) UpdateAgent(ctx context.Context, userID, canvasID string, dsl entity.JSONMap) error {
|
||||
row, err := s.loadCanvasForUser(ctx, userID, canvasID)
|
||||
if err != nil {
|
||||
// UpdateAgent applies a draft patch to user_canvas. Settings updates may omit
|
||||
// dsl; in that case the existing draft DSL must be preserved.
|
||||
func (s *AgentService) UpdateAgent(ctx context.Context, userID, canvasID string, patch map[string]interface{}) error {
|
||||
if _, err := s.loadCanvasForUser(ctx, userID, canvasID); err != nil {
|
||||
return err
|
||||
}
|
||||
row.DSL = dslpkg.NormalizeForCanvas(dsl)
|
||||
row.Release = false
|
||||
if err := s.canvasDAO.Update(row); err != nil {
|
||||
|
||||
updates := map[string]interface{}{
|
||||
"release": false,
|
||||
}
|
||||
for _, key := range []string{"title", "avatar", "description", "permission", "canvas_type", "canvas_category"} {
|
||||
if value, ok := patch[key]; ok && value != nil {
|
||||
if key == "title" {
|
||||
if title, ok := value.(string); ok {
|
||||
value = strings.TrimSpace(title)
|
||||
}
|
||||
}
|
||||
updates[key] = value
|
||||
}
|
||||
}
|
||||
if dsl, ok := patch["dsl"]; ok && dsl != nil {
|
||||
dslMap, ok := dsl.(map[string]interface{})
|
||||
if !ok {
|
||||
if typed, ok := dsl.(entity.JSONMap); ok {
|
||||
dslMap = map[string]interface{}(typed)
|
||||
} else {
|
||||
return fmt.Errorf("update agent %s: dsl must be an object", canvasID)
|
||||
}
|
||||
}
|
||||
updates["dsl"] = entity.JSONMap(dslpkg.NormalizeForCanvas(entity.JSONMap(dslMap)))
|
||||
}
|
||||
|
||||
_, err := s.canvasDAO.UpdateFields(canvasID, updates)
|
||||
if err != nil {
|
||||
return fmt.Errorf("update agent %s: %w", canvasID, err)
|
||||
}
|
||||
return nil
|
||||
|
||||
@@ -1186,6 +1186,92 @@ func TestResetAgentServiceNotFound(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdateAgentSettingsPreservesDSL(t *testing.T) {
|
||||
setupAgentSessionServiceTest(t)
|
||||
|
||||
originalDSL := entity.JSONMap{
|
||||
"graph": map[string]any{
|
||||
"nodes": []any{map[string]any{"id": "begin"}},
|
||||
"edges": []any{},
|
||||
},
|
||||
"components": map[string]any{
|
||||
"begin": map[string]any{
|
||||
"obj": map[string]any{"component_name": "Begin"},
|
||||
},
|
||||
},
|
||||
}
|
||||
if err := dao.DB.Create(&entity.UserCanvas{
|
||||
ID: "canvas-settings",
|
||||
UserID: "user-1",
|
||||
Title: sptr("Settings Agent"),
|
||||
Description: sptr("old description"),
|
||||
CanvasCategory: "agent_canvas",
|
||||
DSL: originalDSL,
|
||||
}).Error; err != nil {
|
||||
t.Fatalf("failed to seed canvas: %v", err)
|
||||
}
|
||||
|
||||
err := NewAgentService().UpdateAgent(context.Background(), "user-1", "canvas-settings", map[string]interface{}{
|
||||
"description": "new description",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("UpdateAgent failed: %v", err)
|
||||
}
|
||||
|
||||
persisted, err := dao.NewUserCanvasDAO().GetByID("canvas-settings")
|
||||
if err != nil {
|
||||
t.Fatalf("failed to reload canvas: %v", err)
|
||||
}
|
||||
if persisted.Description == nil || *persisted.Description != "new description" {
|
||||
t.Fatalf("Description = %v, want new description", persisted.Description)
|
||||
}
|
||||
if _, ok := persisted.DSL["graph"]; !ok {
|
||||
t.Fatalf("DSL graph was removed: %#v", persisted.DSL)
|
||||
}
|
||||
if _, ok := persisted.DSL["components"]; !ok {
|
||||
t.Fatalf("DSL components were removed: %#v", persisted.DSL)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdateAgentPersistsDSLAsJSONMap(t *testing.T) {
|
||||
setupAgentSessionServiceTest(t)
|
||||
|
||||
if err := dao.DB.Create(&entity.UserCanvas{
|
||||
ID: "canvas-dsl-update",
|
||||
UserID: "user-1",
|
||||
Title: sptr("DSL Agent"),
|
||||
CanvasCategory: "agent_canvas",
|
||||
DSL: entity.JSONMap{},
|
||||
}).Error; err != nil {
|
||||
t.Fatalf("failed to seed canvas: %v", err)
|
||||
}
|
||||
|
||||
err := NewAgentService().UpdateAgent(context.Background(), "user-1", "canvas-dsl-update", map[string]interface{}{
|
||||
"dsl": map[string]interface{}{
|
||||
"graph": map[string]interface{}{
|
||||
"nodes": []interface{}{map[string]interface{}{"id": "begin"}},
|
||||
"edges": []interface{}{},
|
||||
},
|
||||
"components": map[string]interface{}{
|
||||
"begin": map[string]interface{}{
|
||||
"obj": map[string]interface{}{"component_name": "Begin"},
|
||||
},
|
||||
},
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("UpdateAgent failed: %v", err)
|
||||
}
|
||||
|
||||
persisted, err := dao.NewUserCanvasDAO().GetByID("canvas-dsl-update")
|
||||
if err != nil {
|
||||
t.Fatalf("failed to reload canvas: %v", err)
|
||||
}
|
||||
if _, ok := persisted.DSL["graph"]; !ok {
|
||||
t.Fatalf("DSL graph was not persisted: %#v", persisted.DSL)
|
||||
}
|
||||
}
|
||||
|
||||
// TestResetAgentServiceOtherTenant asserts the access-denied path:
|
||||
// a canvas owned by user-2 is not visible to user-1, so the same
|
||||
// not-found error type is returned. The service layer does not
|
||||
|
||||
Reference in New Issue
Block a user