fix(ticket): close ticket number concurrency gap
This commit is contained in:
@@ -8,7 +8,7 @@ import (
|
|||||||
)
|
)
|
||||||
|
|
||||||
func init() {
|
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 {
|
return sqls.WithTransaction(func(ctx *sqls.TxContext) error {
|
||||||
if err := resetLightweightTicketData(ctx.Tx); err != nil {
|
if err := resetLightweightTicketData(ctx.Tx); err != nil {
|
||||||
return err
|
return err
|
||||||
|
|||||||
@@ -1,6 +1,15 @@
|
|||||||
package migration
|
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) {
|
func TestNotificationPermissionMigrationRegistered(t *testing.T) {
|
||||||
migration, ok := migrationFuncs[7]
|
migration, ok := migrationFuncs[7]
|
||||||
@@ -17,7 +26,71 @@ func TestLightweightTicketPermissionMigrationRegistered(t *testing.T) {
|
|||||||
if !ok {
|
if !ok {
|
||||||
t.Fatalf("expected migration version 8 to be registered")
|
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)
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -38,7 +38,7 @@ func (s *ticketNoService) nextWithRetry(tx *gorm.DB, now time.Time) (string, err
|
|||||||
for attempt := 0; attempt < 100; attempt++ {
|
for attempt := 0; attempt < 100; attempt++ {
|
||||||
current, err := repositories.TicketNoSequenceRepository.GetByDateKeyForUpdate(tx, dateKey)
|
current, err := repositories.TicketNoSequenceRepository.GetByDateKeyForUpdate(tx, dateKey)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if isRetriableTicketNoError(err) {
|
if isRetriableTicketNoError(tx, err) {
|
||||||
sleepTicketNoRetry(attempt)
|
sleepTicketNoRetry(attempt)
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
@@ -55,13 +55,13 @@ func (s *ticketNoService) nextWithRetry(tx *gorm.DB, now time.Time) (string, err
|
|||||||
if err == nil {
|
if err == nil {
|
||||||
return formatTicketNo(dateKey, 1), nil
|
return formatTicketNo(dateKey, 1), nil
|
||||||
}
|
}
|
||||||
if !isRetriableTicketNoError(err) {
|
if !isRetriableTicketNoError(tx, err) {
|
||||||
return "", err
|
return "", err
|
||||||
}
|
}
|
||||||
|
|
||||||
current, err = repositories.TicketNoSequenceRepository.GetByDateKeyForUpdate(tx, dateKey)
|
current, err = repositories.TicketNoSequenceRepository.GetByDateKeyForUpdate(tx, dateKey)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if isRetriableTicketNoError(err) {
|
if isRetriableTicketNoError(tx, err) {
|
||||||
sleepTicketNoRetry(attempt)
|
sleepTicketNoRetry(attempt)
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
@@ -75,7 +75,7 @@ func (s *ticketNoService) nextWithRetry(tx *gorm.DB, now time.Time) (string, err
|
|||||||
allocated := current.NextSeq
|
allocated := current.NextSeq
|
||||||
ok, err := repositories.TicketNoSequenceRepository.UpdateNextSeq(tx, current.ID, allocated, allocated+1, now)
|
ok, err := repositories.TicketNoSequenceRepository.UpdateNextSeq(tx, current.ID, allocated, allocated+1, now)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if isRetriableTicketNoError(err) {
|
if isRetriableTicketNoError(tx, err) {
|
||||||
sleepTicketNoRetry(attempt)
|
sleepTicketNoRetry(attempt)
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
@@ -109,20 +109,19 @@ func isDuplicateKeyError(err error) bool {
|
|||||||
return strings.Contains(message, "duplicate") || strings.Contains(message, "unique") || strings.Contains(message, "constraint failed")
|
return strings.Contains(message, "duplicate") || strings.Contains(message, "unique") || strings.Contains(message, "constraint failed")
|
||||||
}
|
}
|
||||||
|
|
||||||
func isRetriableTicketNoError(err error) bool {
|
func isRetriableTicketNoError(tx *gorm.DB, err error) bool {
|
||||||
return isDuplicateKeyError(err) || isDatabaseLockedError(err)
|
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 {
|
if err == nil {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
message := strings.ToLower(err.Error())
|
message := strings.ToLower(err.Error())
|
||||||
return strings.Contains(message, "database is locked") ||
|
return strings.Contains(message, "database is locked") ||
|
||||||
strings.Contains(message, "database table is locked") ||
|
strings.Contains(message, "database table is locked") ||
|
||||||
strings.Contains(message, "database is busy") ||
|
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")
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -178,23 +178,25 @@ func (s *ticketService) CreateTicket(req request.CreateTicketRequest, operator *
|
|||||||
}
|
}
|
||||||
ticket.UpdatedAt = now
|
ticket.UpdatedAt = now
|
||||||
|
|
||||||
if err := sqls.WithTransaction(func(ctx *sqls.TxContext) error {
|
if err := withSQLiteTicketCreateLock(sqls.DB(), func() error {
|
||||||
ticketNo, err := TicketNoService.Next(ctx.Tx, now)
|
return sqls.WithTransaction(func(ctx *sqls.TxContext) error {
|
||||||
if err != nil {
|
ticketNo, err := TicketNoService.nextWithRetry(ctx.Tx, now)
|
||||||
return err
|
if err != nil {
|
||||||
}
|
return err
|
||||||
ticket.TicketNo = ticketNo
|
}
|
||||||
if err := repositories.TicketRepository.Create(ctx.Tx, ticket); err != nil {
|
ticket.TicketNo = ticketNo
|
||||||
return err
|
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
|
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,
|
return repositories.TicketProgressRepository.Create(ctx.Tx, &models.TicketProgress{
|
||||||
Content: "创建工单",
|
TicketID: ticket.ID,
|
||||||
AuthorID: operator.UserID,
|
Content: "创建工单",
|
||||||
CreatedAt: now,
|
AuthorID: operator.UserID,
|
||||||
|
CreatedAt: now,
|
||||||
|
})
|
||||||
})
|
})
|
||||||
}); err != nil {
|
}); err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
@@ -207,6 +209,14 @@ func (s *ticketService) CreateTicket(req request.CreateTicketRequest, operator *
|
|||||||
return s.Get(ticket.ID), nil
|
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) {
|
func (s *ticketService) CreateFromConversation(req request.CreateTicketFromConversationRequest, operator *dto.AuthPrincipal) (*models.Ticket, error) {
|
||||||
if operator == nil {
|
if operator == nil {
|
||||||
return nil, errorsx.Unauthorized("未登录或登录已过期")
|
return nil, errorsx.Unauthorized("未登录或登录已过期")
|
||||||
|
|||||||
@@ -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) {
|
func setupTicketTestDB(t *testing.T) {
|
||||||
setupTicketTestDBWithMaxOpenConns(t, 0)
|
setupTicketTestDBWithMaxOpenConns(t, 0)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user