From fa9d431cbe3efc65f21518a74cc0aaa533902f87 Mon Sep 17 00:00:00 2001 From: mlogclub Date: Sat, 2 May 2026 19:19:18 +0800 Subject: [PATCH] fix(ticket): harden lightweight backend core --- ...008_sync_lightweight_ticket_permissions.go | 69 ++++++++++++++++++ .../notification_permissions_test.go | 10 +++ .../ticket_no_sequence_repository.go | 14 ++++ .../repositories/ticket_view_repository.go | 8 +-- .../services/ticket_no_sequence_service.go | 27 +------ internal/services/ticket_no_service.go | 60 +++++++++++++--- internal/services/ticket_service.go | 18 +++-- internal/services/ticket_service_test.go | 72 +++++++++++++++++-- internal/services/ticket_view_service.go | 5 +- 9 files changed, 232 insertions(+), 51 deletions(-) create mode 100644 internal/migration/000008_sync_lightweight_ticket_permissions.go diff --git a/internal/migration/000008_sync_lightweight_ticket_permissions.go b/internal/migration/000008_sync_lightweight_ticket_permissions.go new file mode 100644 index 0000000..98f9081 --- /dev/null +++ b/internal/migration/000008_sync_lightweight_ticket_permissions.go @@ -0,0 +1,69 @@ +package migration + +import ( + "cs-agent/internal/models" + + "github.com/mlogclub/simple/sqls" + "gorm.io/gorm" +) + +func init() { + register(8, "sync lightweight ticket permissions", func() error { + return sqls.WithTransaction(func(ctx *sqls.TxContext) error { + if err := resetLightweightTicketData(ctx.Tx); err != nil { + return err + } + + permissions, err := ensurePermissions(ctx.Tx) + if err != nil { + return err + } + + roles, err := ensureRoles(ctx.Tx) + if err != nil { + return err + } + + return ensureRolePermissions(ctx.Tx, roles, permissions) + }) + }) +} + +func resetLightweightTicketData(tx *gorm.DB) error { + for _, table := range lightweightTicketResetTables(tx) { + if !tx.Migrator().HasTable(table) { + continue + } + if err := tx.Exec("DELETE FROM " + table).Error; err != nil { + return err + } + } + return nil +} + +func lightweightTicketResetTables(tx *gorm.DB) []string { + return []string{ + "t_ticket_sla_record", + "t_ticket_resolution_code", + "t_ticket_priority_config", + "t_ticket_watcher", + "t_ticket_collaborator", + "t_ticket_mention", + "t_ticket_event_log", + "t_ticket_relation", + "t_ticket_comment", + tableName(tx, &models.TicketProgress{}, "t_ticket_progress"), + tableName(tx, &models.TicketTag{}, "t_ticket_tag"), + tableName(tx, &models.Ticket{}, "t_ticket"), + tableName(tx, &models.TicketNoSequence{}, "t_ticket_no_sequence"), + tableName(tx, &models.TicketView{}, "t_ticket_view"), + } +} + +func tableName(tx *gorm.DB, model any, fallback string) string { + stmt := &gorm.Statement{DB: tx} + if err := stmt.Parse(model); err != nil { + return fallback + } + return stmt.Schema.Table +} diff --git a/internal/migration/notification_permissions_test.go b/internal/migration/notification_permissions_test.go index 5c11d38..510b5cc 100644 --- a/internal/migration/notification_permissions_test.go +++ b/internal/migration/notification_permissions_test.go @@ -11,3 +11,13 @@ func TestNotificationPermissionMigrationRegistered(t *testing.T) { t.Fatalf("unexpected migration remark: %q", migration.Remark) } } + +func TestLightweightTicketPermissionMigrationRegistered(t *testing.T) { + migration, ok := migrationFuncs[8] + if !ok { + t.Fatalf("expected migration version 8 to be registered") + } + if migration.Remark != "sync lightweight ticket permissions" { + t.Fatalf("unexpected migration remark: %q", migration.Remark) + } +} diff --git a/internal/repositories/ticket_no_sequence_repository.go b/internal/repositories/ticket_no_sequence_repository.go index b480507..013f35a 100644 --- a/internal/repositories/ticket_no_sequence_repository.go +++ b/internal/repositories/ticket_no_sequence_repository.go @@ -2,11 +2,13 @@ package repositories import ( "cs-agent/internal/models" + "errors" "time" "github.com/mlogclub/simple/sqls" "github.com/mlogclub/simple/web/params" "gorm.io/gorm" + "gorm.io/gorm/clause" ) var TicketNoSequenceRepository = newTicketNoSequenceRepository() @@ -74,6 +76,18 @@ func (r *ticketNoSequenceRepository) GetByDateKey(db *gorm.DB, dateKey string) * return ret } +func (r *ticketNoSequenceRepository) GetByDateKeyForUpdate(db *gorm.DB, dateKey string) (*models.TicketNoSequence, error) { + ret := &models.TicketNoSequence{} + err := db.Clauses(clause.Locking{Strength: "UPDATE"}).Take(ret, "date_key = ?", dateKey).Error + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil, nil + } + if err != nil { + return nil, err + } + return ret, nil +} + func (r *ticketNoSequenceRepository) Create(db *gorm.DB, t *models.TicketNoSequence) error { return db.Create(t).Error } diff --git a/internal/repositories/ticket_view_repository.go b/internal/repositories/ticket_view_repository.go index 6072d5a..2e0e039 100644 --- a/internal/repositories/ticket_view_repository.go +++ b/internal/repositories/ticket_view_repository.go @@ -25,7 +25,7 @@ func (r *ticketViewRepository) Get(db *gorm.DB, id int64) *models.TicketView { return ret } -func (r *ticketViewRepository) Take(db *gorm.DB, where ...interface{}) *models.TicketView { +func (r *ticketViewRepository) Take(db *gorm.DB, where ...any) *models.TicketView { ret := &models.TicketView{} if err := db.Take(ret, where...).Error; err != nil { return nil @@ -74,10 +74,10 @@ func (r *ticketViewRepository) Update(db *gorm.DB, t *models.TicketView) error { return db.Save(t).Error } -func (r *ticketViewRepository) Updates(db *gorm.DB, id int64, columns map[string]interface{}) error { +func (r *ticketViewRepository) Updates(db *gorm.DB, id int64, columns map[string]any) error { return db.Model(&models.TicketView{}).Where("id = ?", id).Updates(columns).Error } -func (r *ticketViewRepository) Delete(db *gorm.DB, id int64) { - db.Delete(&models.TicketView{}, "id = ?", id) +func (r *ticketViewRepository) Delete(db *gorm.DB, id int64) error { + return db.Delete(&models.TicketView{}, "id = ?", id).Error } diff --git a/internal/services/ticket_no_sequence_service.go b/internal/services/ticket_no_sequence_service.go index 2af1382..c322768 100644 --- a/internal/services/ticket_no_sequence_service.go +++ b/internal/services/ticket_no_sequence_service.go @@ -1,7 +1,6 @@ package services import ( - "fmt" "time" "cs-agent/internal/models" @@ -70,29 +69,5 @@ func (s *ticketNoSequenceService) Delete(id int64) { } func (s *ticketNoSequenceService) Next(db *gorm.DB, now time.Time) (string, error) { - dateKey := now.Format("20060102") - for i := 0; i < 5; i++ { - current := repositories.TicketNoSequenceRepository.GetByDateKey(db, dateKey) - if current == nil { - item := &models.TicketNoSequence{ - DateKey: dateKey, - NextSeq: 2, - CreatedAt: now, - UpdatedAt: now, - } - if err := repositories.TicketNoSequenceRepository.Create(db, item); err != nil { - continue - } - return fmt.Sprintf("TK%s%04d", dateKey, int64(1)), nil - } - seq := current.NextSeq - ok, err := repositories.TicketNoSequenceRepository.UpdateNextSeq(db, current.ID, seq, seq+1, now) - if err != nil { - return "", err - } - if ok { - return fmt.Sprintf("TK%s%04d", dateKey, seq), nil - } - } - return "", fmt.Errorf("generate ticket number failed") + return TicketNoService.Next(db, now) } diff --git a/internal/services/ticket_no_service.go b/internal/services/ticket_no_service.go index 53f81be..8916e0a 100644 --- a/internal/services/ticket_no_service.go +++ b/internal/services/ticket_no_service.go @@ -5,6 +5,7 @@ import ( "cs-agent/internal/repositories" "fmt" "strings" + "sync" "time" "gorm.io/gorm" @@ -12,6 +13,8 @@ import ( var TicketNoService = newTicketNoService() +var ticketNoSQLiteMu sync.Mutex + func newTicketNoService() *ticketNoService { return &ticketNoService{} } @@ -22,9 +25,25 @@ func (s *ticketNoService) Next(tx *gorm.DB, now time.Time) (string, error) { if tx == nil { return "", fmt.Errorf("ticket number transaction is required") } + if tx.Dialector.Name() == "sqlite" { + ticketNoSQLiteMu.Lock() + defer ticketNoSQLiteMu.Unlock() + return s.nextWithRetry(tx, now) + } + return s.nextWithRetry(tx, now) +} + +func (s *ticketNoService) nextWithRetry(tx *gorm.DB, now time.Time) (string, error) { dateKey := now.Format("20060102") - for attempt := 0; attempt < 20; attempt++ { - current := repositories.TicketNoSequenceRepository.GetByDateKey(tx, dateKey) + for attempt := 0; attempt < 100; attempt++ { + current, err := repositories.TicketNoSequenceRepository.GetByDateKeyForUpdate(tx, dateKey) + if err != nil { + if isRetriableTicketNoError(err) { + sleepTicketNoRetry(attempt) + continue + } + return "", err + } if current == nil { item := &models.TicketNoSequence{ DateKey: dateKey, @@ -39,14 +58,25 @@ func (s *ticketNoService) Next(tx *gorm.DB, now time.Time) (string, error) { if !isRetriableTicketNoError(err) { return "", err } - time.Sleep(time.Duration(attempt+1) * 10 * time.Millisecond) - continue + + current, err = repositories.TicketNoSequenceRepository.GetByDateKeyForUpdate(tx, dateKey) + if err != nil { + if isRetriableTicketNoError(err) { + sleepTicketNoRetry(attempt) + continue + } + return "", err + } + if current == nil { + sleepTicketNoRetry(attempt) + continue + } } allocated := current.NextSeq - ok, err := repositories.TicketNoSequenceRepository.UpdateNextSeq(tx, current.ID, current.NextSeq, current.NextSeq+1, now) + ok, err := repositories.TicketNoSequenceRepository.UpdateNextSeq(tx, current.ID, allocated, allocated+1, now) if err != nil { if isRetriableTicketNoError(err) { - time.Sleep(time.Duration(attempt+1) * 10 * time.Millisecond) + sleepTicketNoRetry(attempt) continue } return "", err @@ -54,11 +84,19 @@ func (s *ticketNoService) Next(tx *gorm.DB, now time.Time) (string, error) { if ok { return formatTicketNo(dateKey, allocated), nil } - time.Sleep(time.Duration(attempt+1) * 10 * time.Millisecond) + sleepTicketNoRetry(attempt) } return "", fmt.Errorf("failed to allocate ticket number") } +func sleepTicketNoRetry(attempt int) { + delay := time.Duration(attempt+1) * 10 * time.Millisecond + if delay > 200*time.Millisecond { + delay = 200 * time.Millisecond + } + time.Sleep(delay) +} + func formatTicketNo(dateKey string, seq int64) string { return fmt.Sprintf("TK%s%05d", dateKey, seq) } @@ -80,5 +118,11 @@ func isDatabaseLockedError(err error) bool { return false } message := strings.ToLower(err.Error()) - return strings.Contains(message, "database is locked") || strings.Contains(message, "database table is locked") + return strings.Contains(message, "database is locked") || + strings.Contains(message, "database table is locked") || + strings.Contains(message, "database is busy") || + strings.Contains(message, "lock wait timeout") || + strings.Contains(message, "deadlock") || + strings.Contains(message, "try restarting transaction") || + strings.Contains(message, "could not serialize") } diff --git a/internal/services/ticket_service.go b/internal/services/ticket_service.go index 386eb1e..c0c6f0b 100644 --- a/internal/services/ticket_service.go +++ b/internal/services/ticket_service.go @@ -59,7 +59,7 @@ func (s *ticketService) Get(id int64) *models.Ticket { return repositories.TicketRepository.Get(sqls.DB(), id) } -func (s *ticketService) Take(where ...interface{}) *models.Ticket { +func (s *ticketService) Take(where ...any) *models.Ticket { return repositories.TicketRepository.Take(sqls.DB(), where...) } @@ -96,11 +96,11 @@ func (s *ticketService) Update(t *models.Ticket) error { return repositories.TicketRepository.Update(sqls.DB(), t) } -func (s *ticketService) Updates(id int64, columns map[string]interface{}) error { +func (s *ticketService) Updates(id int64, columns map[string]any) error { return repositories.TicketRepository.Updates(sqls.DB(), id, columns) } -func (s *ticketService) UpdateColumn(id int64, name string, value interface{}) error { +func (s *ticketService) UpdateColumn(id int64, name string, value any) error { return repositories.TicketRepository.UpdateColumn(sqls.DB(), id, name, value) } @@ -576,8 +576,14 @@ func (s *ticketService) validateTicketRefs(customerID, conversationID, assigneeI if customerID > 0 && CustomerService.Get(customerID) == nil { return errorsx.InvalidParam("客户不存在") } - if conversationID > 0 && ConversationService.Get(conversationID) == nil { - return errorsx.InvalidParam("会话不存在") + if conversationID > 0 { + conversation := ConversationService.Get(conversationID) + if conversation == nil { + return errorsx.InvalidParam("会话不存在") + } + if customerID > 0 && conversation.CustomerID != customerID { + return errorsx.InvalidParam("会话与客户不匹配") + } } return s.validateAssignee(assigneeID) } @@ -594,7 +600,7 @@ func (s *ticketService) validateRequiredAssignee(userID int64) error { return errorsx.InvalidParam("负责人不存在") } user := UserService.Get(userID) - if user == nil || user.Status == enums.StatusDeleted { + if user == nil || user.Status != enums.StatusOk { return errorsx.InvalidParam("负责人不存在") } return nil diff --git a/internal/services/ticket_service_test.go b/internal/services/ticket_service_test.go index 1b5a8a1..48c5424 100644 --- a/internal/services/ticket_service_test.go +++ b/internal/services/ticket_service_test.go @@ -208,6 +208,43 @@ func TestTicketServiceAssignTicketRequiresTargetUser(t *testing.T) { } } +func TestTicketServiceAssignTicketRejectsDisabledUser(t *testing.T) { + setupTicketTestDB(t) + operator := createTestOperator(t, "assign-disabled-operator") + disabledUserID := createTestUserWithStatus(t, "assign-disabled-user", enums.StatusDisabled) + ticket, err := services.TicketService.CreateTicket(createTestTicketRequest("assign-disabled-ticket"), operator) + if err != nil { + t.Fatalf("CreateTicket() error = %v", err) + } + + err = services.TicketService.AssignTicket(request.AssignTicketRequest{ + TicketID: ticket.ID, + ToUserID: disabledUserID, + Reason: "disabled assignment", + }, operator) + if err == nil { + t.Fatalf("expected AssignTicket() to reject disabled target user") + } +} + +func TestTicketServiceCreateTicketRejectsMismatchedCustomerConversation(t *testing.T) { + setupTicketTestDB(t) + operator := createTestOperator(t, "mismatch-operator") + customerID := createTestCustomer(t, "mismatch-customer") + otherCustomerID := createTestCustomer(t, "mismatch-other-customer") + conversationID := createTestConversation(t, otherCustomerID, "mismatch-conversation") + + _, err := services.TicketService.CreateTicket(request.CreateTicketRequest{ + Title: "mismatch ticket", + Description: "mismatch ticket description", + CustomerID: customerID, + ConversationID: conversationID, + }, operator) + if err == nil { + t.Fatalf("expected CreateTicket() to reject mismatched customer and conversation") + } +} + func TestTicketServiceSummaryCountsStaleTickets(t *testing.T) { setupTicketTestDB(t) operator := createTestOperator(t, "summary-operator") @@ -284,9 +321,9 @@ func TestTicketServiceFindPageAggregateEnrichesLookups(t *testing.T) { } func TestTicketServiceTicketNoNextConcurrent(t *testing.T) { - setupTicketTestDB(t) + setupTicketTestDBWithMaxOpenConns(t, 8) - const count = 20 + const count = 50 results := make(chan string, count) errs := make(chan error, count) var wg sync.WaitGroup @@ -332,6 +369,10 @@ func TestTicketServiceTicketNoNextConcurrent(t *testing.T) { } func setupTicketTestDB(t *testing.T) { + setupTicketTestDBWithMaxOpenConns(t, 0) +} + +func setupTicketTestDBWithMaxOpenConns(t *testing.T, maxOpenConns int) { t.Helper() dbPath := filepath.Join(t.TempDir(), "ticket-test.db") @@ -339,7 +380,7 @@ func setupTicketTestDB(t *testing.T) { Type: "sqlite", DSN: "file:" + dbPath + "?_busy_timeout=5000", MaxIdleConns: 1, - MaxOpenConns: 1, + MaxOpenConns: maxOpenConns, }) if err != nil { t.Fatalf("InitDB() error = %v", err) @@ -369,13 +410,17 @@ func createTestOperator(t *testing.T, prefix string) *dto.AuthPrincipal { } func createTestUser(t *testing.T, prefix string) int64 { + return createTestUserWithStatus(t, prefix, enums.StatusOk) +} + +func createTestUserWithStatus(t *testing.T, prefix string, status enums.Status) int64 { t.Helper() now := time.Now() username := fmt.Sprintf("%s_%d", prefix, now.UnixNano()) user := &models.User{ Username: username, Nickname: prefix, - Status: enums.StatusOk, + Status: status, AuditFields: models.AuditFields{ CreatedAt: now, CreateUserID: 1, @@ -391,6 +436,25 @@ func createTestUser(t *testing.T, prefix string) int64 { return user.ID } +func createTestConversation(t *testing.T, customerID int64, prefix string) int64 { + t.Helper() + + now := time.Now() + item := &models.Conversation{ + CustomerID: customerID, + CustomerName: prefix, + Status: enums.IMConversationStatusActive, + ServiceMode: enums.IMConversationServiceModeAIOnly, + LastMessageAt: now, + LastActiveAt: now, + AuditFields: models.AuditFields{CreatedAt: now, UpdatedAt: now}, + } + if err := repositories.ConversationRepository.Create(sqls.DB(), item); err != nil { + t.Fatalf("create conversation error = %v", err) + } + return item.ID +} + func createTestCustomer(t *testing.T, prefix string) int64 { t.Helper() diff --git a/internal/services/ticket_view_service.go b/internal/services/ticket_view_service.go index 8ae5ef7..e6ff87c 100644 --- a/internal/services/ticket_view_service.go +++ b/internal/services/ticket_view_service.go @@ -57,7 +57,7 @@ func (s *ticketViewService) Save(req request.SaveTicketViewRequest, operator *dt if item == nil || item.UserID != operator.UserID { return nil, errorsx.InvalidParam("视图不存在") } - if err := repositories.TicketViewRepository.Updates(sqls.DB(), req.ID, map[string]interface{}{ + if err := repositories.TicketViewRepository.Updates(sqls.DB(), req.ID, map[string]any{ "name": name, "filters_json": string(filtersJSON), "update_user_id": operator.UserID, @@ -88,6 +88,5 @@ func (s *ticketViewService) Delete(id int64, operator *dto.AuthPrincipal) error if item == nil || item.UserID != operator.UserID { return errorsx.InvalidParam("视图不存在") } - repositories.TicketViewRepository.Delete(sqls.DB(), id) - return nil + return repositories.TicketViewRepository.Delete(sqls.DB(), id) }