From 0fed6dc1444877a216f18acb1e612a8745e77f8e Mon Sep 17 00:00:00 2001 From: mlogclub Date: Sat, 2 May 2026 19:29:04 +0800 Subject: [PATCH] fix(ticket): close ticket number concurrency gap --- ...008_sync_lightweight_ticket_permissions.go | 2 +- .../notification_permissions_test.go | 77 ++++++++++++++++++- internal/services/ticket_no_service.go | 23 +++--- internal/services/ticket_service.go | 44 +++++++---- internal/services/ticket_service_test.go | 50 ++++++++++++ 5 files changed, 164 insertions(+), 32 deletions(-) diff --git a/internal/migration/000008_sync_lightweight_ticket_permissions.go b/internal/migration/000008_sync_lightweight_ticket_permissions.go index 98f9081..58f99ce 100644 --- a/internal/migration/000008_sync_lightweight_ticket_permissions.go +++ b/internal/migration/000008_sync_lightweight_ticket_permissions.go @@ -8,7 +8,7 @@ import ( ) func init() { - register(8, "sync lightweight ticket permissions", func() error { + register(8, "sync lightweight ticket permissions and reset ticket data", func() error { return sqls.WithTransaction(func(ctx *sqls.TxContext) error { if err := resetLightweightTicketData(ctx.Tx); err != nil { return err diff --git a/internal/migration/notification_permissions_test.go b/internal/migration/notification_permissions_test.go index 510b5cc..6add982 100644 --- a/internal/migration/notification_permissions_test.go +++ b/internal/migration/notification_permissions_test.go @@ -1,6 +1,15 @@ package migration -import "testing" +import ( + "testing" + "time" + + "cs-agent/internal/models" + + "github.com/glebarez/sqlite" + "gorm.io/gorm" + "gorm.io/gorm/schema" +) func TestNotificationPermissionMigrationRegistered(t *testing.T) { migration, ok := migrationFuncs[7] @@ -17,7 +26,71 @@ func TestLightweightTicketPermissionMigrationRegistered(t *testing.T) { if !ok { t.Fatalf("expected migration version 8 to be registered") } - if migration.Remark != "sync lightweight ticket permissions" { + if migration.Remark != "sync lightweight ticket permissions and reset ticket data" { t.Fatalf("unexpected migration remark: %q", migration.Remark) } } + +func TestLightweightTicketMigrationResetDeletesTicketData(t *testing.T) { + db, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory&cache=shared"), &gorm.Config{ + NamingStrategy: schema.NamingStrategy{ + TablePrefix: "t_", + SingularTable: true, + }, + }) + if err != nil { + t.Fatalf("open sqlite db: %v", err) + } + sqlDB, err := db.DB() + if err != nil { + t.Fatalf("get sqlite db: %v", err) + } + t.Cleanup(func() { + _ = sqlDB.Close() + }) + + if err := db.AutoMigrate(&models.Ticket{}, &models.TicketTag{}, &models.TicketProgress{}, &models.TicketNoSequence{}); err != nil { + t.Fatalf("AutoMigrate() error = %v", err) + } + + now := time.Now() + ticket := &models.Ticket{ + TicketNo: "TK2026050200001", + Title: "legacy ticket", + Description: "legacy ticket description", + AuditFields: models.AuditFields{CreatedAt: now, UpdatedAt: now}, + } + if err := db.Create(ticket).Error; err != nil { + t.Fatalf("create ticket error = %v", err) + } + if err := db.Create(&models.TicketTag{TicketID: ticket.ID, TagID: 1, AuditFields: models.AuditFields{CreatedAt: now, UpdatedAt: now}}).Error; err != nil { + t.Fatalf("create ticket tag error = %v", err) + } + if err := db.Create(&models.TicketProgress{TicketID: ticket.ID, Content: "legacy progress", CreatedAt: now}).Error; err != nil { + t.Fatalf("create ticket progress error = %v", err) + } + if err := db.Create(&models.TicketNoSequence{DateKey: "20260502", NextSeq: 2, CreatedAt: now, UpdatedAt: now}).Error; err != nil { + t.Fatalf("create ticket no sequence error = %v", err) + } + + if err := resetLightweightTicketData(db); err != nil { + t.Fatalf("resetLightweightTicketData() error = %v", err) + } + + assertTableCount(t, db, &models.Ticket{}, 0) + assertTableCount(t, db, &models.TicketTag{}, 0) + assertTableCount(t, db, &models.TicketProgress{}, 0) + assertTableCount(t, db, &models.TicketNoSequence{}, 0) +} + +func assertTableCount(t *testing.T, db *gorm.DB, model any, expected int64) { + t.Helper() + + var count int64 + if err := db.Model(model).Count(&count).Error; err != nil { + t.Fatalf("count %T error = %v", model, err) + } + if count != expected { + t.Fatalf("expected %T count %d, got %d", model, expected, count) + } +} diff --git a/internal/services/ticket_no_service.go b/internal/services/ticket_no_service.go index 8916e0a..9bfcf5b 100644 --- a/internal/services/ticket_no_service.go +++ b/internal/services/ticket_no_service.go @@ -38,7 +38,7 @@ func (s *ticketNoService) nextWithRetry(tx *gorm.DB, now time.Time) (string, err for attempt := 0; attempt < 100; attempt++ { current, err := repositories.TicketNoSequenceRepository.GetByDateKeyForUpdate(tx, dateKey) if err != nil { - if isRetriableTicketNoError(err) { + if isRetriableTicketNoError(tx, err) { sleepTicketNoRetry(attempt) continue } @@ -55,13 +55,13 @@ func (s *ticketNoService) nextWithRetry(tx *gorm.DB, now time.Time) (string, err if err == nil { return formatTicketNo(dateKey, 1), nil } - if !isRetriableTicketNoError(err) { + if !isRetriableTicketNoError(tx, err) { return "", err } current, err = repositories.TicketNoSequenceRepository.GetByDateKeyForUpdate(tx, dateKey) if err != nil { - if isRetriableTicketNoError(err) { + if isRetriableTicketNoError(tx, err) { sleepTicketNoRetry(attempt) continue } @@ -75,7 +75,7 @@ func (s *ticketNoService) nextWithRetry(tx *gorm.DB, now time.Time) (string, err allocated := current.NextSeq ok, err := repositories.TicketNoSequenceRepository.UpdateNextSeq(tx, current.ID, allocated, allocated+1, now) if err != nil { - if isRetriableTicketNoError(err) { + if isRetriableTicketNoError(tx, err) { sleepTicketNoRetry(attempt) continue } @@ -109,20 +109,19 @@ func isDuplicateKeyError(err error) bool { return strings.Contains(message, "duplicate") || strings.Contains(message, "unique") || strings.Contains(message, "constraint failed") } -func isRetriableTicketNoError(err error) bool { - return isDuplicateKeyError(err) || isDatabaseLockedError(err) +func isRetriableTicketNoError(tx *gorm.DB, err error) bool { + if isDuplicateKeyError(err) { + return true + } + return tx != nil && tx.Dialector.Name() == "sqlite" && isSQLiteDatabaseLockedError(err) } -func isDatabaseLockedError(err error) bool { +func isSQLiteDatabaseLockedError(err error) bool { if err == nil { return false } message := strings.ToLower(err.Error()) 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") + strings.Contains(message, "database is busy") } diff --git a/internal/services/ticket_service.go b/internal/services/ticket_service.go index c0c6f0b..bd740f9 100644 --- a/internal/services/ticket_service.go +++ b/internal/services/ticket_service.go @@ -178,23 +178,25 @@ func (s *ticketService) CreateTicket(req request.CreateTicketRequest, operator * } ticket.UpdatedAt = now - if err := sqls.WithTransaction(func(ctx *sqls.TxContext) error { - ticketNo, err := TicketNoService.Next(ctx.Tx, now) - if err != nil { - return err - } - ticket.TicketNo = ticketNo - if err := repositories.TicketRepository.Create(ctx.Tx, ticket); err != nil { - return err - } - if err := TicketTagService.ReplaceTicketTags(ctx.Tx, ticket.ID, tagIDs, operator); err != nil { - return err - } - return repositories.TicketProgressRepository.Create(ctx.Tx, &models.TicketProgress{ - TicketID: ticket.ID, - Content: "创建工单", - AuthorID: operator.UserID, - CreatedAt: now, + if err := withSQLiteTicketCreateLock(sqls.DB(), func() error { + return sqls.WithTransaction(func(ctx *sqls.TxContext) error { + ticketNo, err := TicketNoService.nextWithRetry(ctx.Tx, now) + if err != nil { + return err + } + ticket.TicketNo = ticketNo + if err := repositories.TicketRepository.Create(ctx.Tx, ticket); err != nil { + return err + } + if err := TicketTagService.ReplaceTicketTags(ctx.Tx, ticket.ID, tagIDs, operator); err != nil { + return err + } + return repositories.TicketProgressRepository.Create(ctx.Tx, &models.TicketProgress{ + TicketID: ticket.ID, + Content: "创建工单", + AuthorID: operator.UserID, + CreatedAt: now, + }) }) }); err != nil { return nil, err @@ -207,6 +209,14 @@ func (s *ticketService) CreateTicket(req request.CreateTicketRequest, operator * return s.Get(ticket.ID), nil } +func withSQLiteTicketCreateLock(db *gorm.DB, fn func() error) error { + if db != nil && db.Dialector.Name() == "sqlite" { + ticketNoSQLiteMu.Lock() + defer ticketNoSQLiteMu.Unlock() + } + return fn() +} + func (s *ticketService) CreateFromConversation(req request.CreateTicketFromConversationRequest, operator *dto.AuthPrincipal) (*models.Ticket, error) { if operator == nil { return nil, errorsx.Unauthorized("未登录或登录已过期") diff --git a/internal/services/ticket_service_test.go b/internal/services/ticket_service_test.go index 48c5424..cd9b97e 100644 --- a/internal/services/ticket_service_test.go +++ b/internal/services/ticket_service_test.go @@ -368,6 +368,56 @@ func TestTicketServiceTicketNoNextConcurrent(t *testing.T) { } } +func TestTicketServiceCreateTicketConcurrentAllocatesUniqueTicketNos(t *testing.T) { + setupTicketTestDBWithMaxOpenConns(t, 8) + operator := createTestOperator(t, "concurrent-create-operator") + + const count = 50 + results := make(chan string, count) + errs := make(chan error, count) + var wg sync.WaitGroup + + for i := 0; i < count; i++ { + wg.Add(1) + go func(index int) { + defer wg.Done() + ticket, err := services.TicketService.CreateTicket(request.CreateTicketRequest{ + Title: fmt.Sprintf("concurrent ticket %d", index), + Description: fmt.Sprintf("concurrent ticket %d description", index), + }, operator) + if err != nil { + errs <- err + return + } + results <- ticket.TicketNo + }(i) + } + + wg.Wait() + close(results) + close(errs) + + for err := range errs { + if err != nil { + t.Fatalf("CreateTicket() concurrent error = %v", err) + } + } + + seen := make(map[string]struct{}, count) + for ticketNo := range results { + if ticketNo == "" { + t.Fatalf("expected non-empty ticket number") + } + if _, ok := seen[ticketNo]; ok { + t.Fatalf("duplicate ticket number generated: %s", ticketNo) + } + seen[ticketNo] = struct{}{} + } + if len(seen) != count { + t.Fatalf("expected %d unique ticket numbers, got %d", count, len(seen)) + } +} + func setupTicketTestDB(t *testing.T) { setupTicketTestDBWithMaxOpenConns(t, 0) }