diff --git a/backend/internal/handler/api/v1/captain_skill_handler_test.go b/backend/internal/handler/api/v1/captain_skill_handler_test.go index c0d481a7..24a18832 100644 --- a/backend/internal/handler/api/v1/captain_skill_handler_test.go +++ b/backend/internal/handler/api/v1/captain_skill_handler_test.go @@ -5,8 +5,10 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "path/filepath" "strconv" "testing" + "time" "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" @@ -25,8 +27,12 @@ import ( func newCaptainSkillHandlerTestRouter(t *testing.T) (*gin.Engine, *gorm.DB, *model.CaptainAssistant) { t.Helper() gin.SetMode(gin.TestMode) - db, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory&cache=shared"), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)}) + db, err := gorm.Open(sqlite.Open("file:"+filepath.Join(t.TempDir(), "captain-skill-handler.db")+"?_busy_timeout=5000&_journal_mode=WAL"), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)}) require.NoError(t, err) + sqlDB, err := db.DB() + require.NoError(t, err) + sqlDB.SetMaxOpenConns(2) + t.Cleanup(func() { require.NoError(t, sqlDB.Close()) }) require.NoError(t, db.AutoMigrate(&model.CaptainAssistant{}, &model.CaptainSkill{}, &model.CaptainSkillReference{}, &model.CaptainAssistantSkill{})) assistant := &model.CaptainAssistant{AccountID: 1, Name: "Assistant", Status: model.AssistantStatusActive} require.NoError(t, db.Create(assistant).Error) @@ -145,3 +151,63 @@ func TestCaptainSkillHandlerValidationAndConflictStatuses(t *testing.T) { w = captainSkillRequest(t, router, http.MethodDelete, base+"/skills/"+skillID, "administrator", nil) assert.Equal(t, http.StatusNoContent, w.Code) } + +func TestCaptainSkillHandlerConcurrentUpdateCASConflict(t *testing.T) { + router, db, _ := newCaptainSkillHandlerTestRouter(t) + base := "/api/v1/accounts/1/captain/skills" + w := captainSkillRequest(t, router, http.MethodPost, base, "administrator", captainSkillBody("active")) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + var created map[string]any + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &created)) + + ready := make(chan struct{}, 2) + release := make(chan struct{}) + rowsAffected := make(chan int64, 2) + barrierName := "test:captain_skill_handler_cas_barrier" + rowsName := "test:captain_skill_handler_cas_rows" + require.NoError(t, db.Callback().Update().Before("gorm:update").Register(barrierName, func(tx *gorm.DB) { + if tx.Statement.Table == "captain_skills" { + ready <- struct{}{} + <-release + } + })) + require.NoError(t, db.Callback().Update().After("gorm:update").Register(rowsName, func(tx *gorm.DB) { + if tx.Statement.Table == "captain_skills" { + rowsAffected <- tx.RowsAffected + } + })) + defer func() { + require.NoError(t, db.Callback().Update().Remove(barrierName)) + require.NoError(t, db.Callback().Update().Remove(rowsName)) + }() + + responses := make(chan *httptest.ResponseRecorder, 2) + for i := range 2 { + body := captainSkillBody("active") + body["skill"].(map[string]any)["description"] = "Concurrent update " + strconv.Itoa(i) + body["skill"].(map[string]any)["expected_version"] = created["version"] + go func() { + responses <- captainSkillRequest(t, router, http.MethodPut, base+"/"+strconv.Itoa(int(created["id"].(float64))), "administrator", body) + }() + } + for range 2 { + select { + case <-ready: + case <-time.After(5 * time.Second): + close(release) + t.Fatal("both HTTP updates did not reach the repository CAS") + } + } + close(release) + + statuses := make([]int, 0, 2) + for range 2 { + response := <-responses + statuses = append(statuses, response.Code) + if response.Code == http.StatusConflict { + assert.Contains(t, response.Body.String(), repository.ErrCaptainSkillVersionConflict.Error()) + } + } + assert.ElementsMatch(t, []int{http.StatusOK, http.StatusConflict}, statuses) + assert.ElementsMatch(t, []int64{1, 0}, []int64{<-rowsAffected, <-rowsAffected}) +} diff --git a/backend/internal/handler/api/v1/profile_handler_test.go b/backend/internal/handler/api/v1/profile_handler_test.go index b9341822..c5e327f0 100644 --- a/backend/internal/handler/api/v1/profile_handler_test.go +++ b/backend/internal/handler/api/v1/profile_handler_test.go @@ -150,14 +150,12 @@ func (s *ProfileHandlerTestSuite) buildRouter() *gin.Engine { func (s *ProfileHandlerTestSuite) SetupTest() { s.db.Where("user_id = ?", s.userID).Delete(&model.UserSession{}) - passwordDigest, err := crypto.HashPassword("oldpassword") - s.Require().NoError(err) // Reset user to original state before each test s.db.Model(&model.User{}).Where("id = ?", s.userID).Updates(map[string]interface{}{ "name": "ProfileUser", "email": "profile@example.com", - "password": passwordDigest, - "password_digest": passwordDigest, + "password": s.user.PasswordDigest, + "password_digest": s.user.PasswordDigest, "avatar_url": "", "available": false, "display_name": "Profile Display", diff --git a/backend/internal/service/captain_skill_service_test.go b/backend/internal/service/captain_skill_service_test.go index 1fce8570..a8d81c0f 100644 --- a/backend/internal/service/captain_skill_service_test.go +++ b/backend/internal/service/captain_skill_service_test.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "os" + "path/filepath" "strings" "testing" "time" @@ -22,10 +23,14 @@ import ( func newCaptainSkillTestService(t *testing.T) (*CaptainSkillService, *gorm.DB, *model.CaptainAssistant, *model.CaptainAssistant) { t.Helper() - db, err := gorm.Open(sqlite.Open("file:"+strings.ReplaceAll(t.Name(), "/", "_")+"?mode=memory&cache=shared"), &gorm.Config{ + db, err := gorm.Open(sqlite.Open("file:"+filepath.Join(t.TempDir(), "captain-skill.db")+"?_busy_timeout=5000&_journal_mode=WAL"), &gorm.Config{ Logger: logger.Default.LogMode(logger.Silent), }) require.NoError(t, err) + sqlDB, err := db.DB() + require.NoError(t, err) + sqlDB.SetMaxOpenConns(2) + t.Cleanup(func() { require.NoError(t, sqlDB.Close()) }) require.NoError(t, db.AutoMigrate( &model.CaptainAssistant{}, &model.CaptainSkill{}, @@ -121,13 +126,10 @@ func TestCaptainSkillServiceManagementFlow(t *testing.T) { func TestCaptainSkillServiceConcurrentUpdateConflict(t *testing.T) { svc, db, assistant, _ := newCaptainSkillTestService(t) - sqlDB, err := db.DB() - require.NoError(t, err) - sqlDB.SetMaxOpenConns(1) skill, err := svc.Create(context.Background(), assistant.AccountID, validCaptainSkillRequest()) require.NoError(t, err) t.Run("content and references", func(t *testing.T) { - assertConcurrentCaptainSkillUpdate(t, svc, skill, false) + assertConcurrentCaptainSkillUpdate(t, db, svc, skill, false) }) t.Run("status only", func(t *testing.T) { draftRequest := validCaptainSkillRequest() @@ -135,7 +137,17 @@ func TestCaptainSkillServiceConcurrentUpdateConflict(t *testing.T) { draftRequest.Status = model.CaptainSkillStatusDraft draft, err := svc.Create(context.Background(), assistant.AccountID, draftRequest) require.NoError(t, err) - assertConcurrentCaptainSkillUpdate(t, svc, draft, true) + assertConcurrentCaptainSkillUpdate(t, db, svc, draft, true) + }) + t.Run("repository CAS", func(t *testing.T) { + request := validCaptainSkillRequest() + request.Name = "repository-cas" + current, err := svc.Create(context.Background(), assistant.AccountID, request) + require.NoError(t, err) + next := *current + next.Version++ + require.NoError(t, svc.repo.Update(context.Background(), &next, current.Version, false, false)) + assert.ErrorIs(t, svc.repo.Update(context.Background(), &next, current.Version, false, false), repository.ErrCaptainSkillVersionConflict) }) } @@ -177,7 +189,7 @@ func TestCaptainSkillServicePostgresSmoke(t *testing.T) { require.NoError(t, err) assert.Equal(t, uint(2), skill.Version) assert.Equal(t, "standard-policy", skill.References[0].ReferenceKey) - assertConcurrentCaptainSkillUpdate(t, svc, skill, false) + assertConcurrentCaptainSkillUpdate(t, db, svc, skill, false) require.NoError(t, svc.Bind(context.Background(), 1, assistant.ID, skill.ID)) items, err := svc.List(context.Background(), 1, &assistant.ID) require.NoError(t, err) @@ -188,11 +200,21 @@ func TestCaptainSkillServicePostgresSmoke(t *testing.T) { draftRequest.Status = model.CaptainSkillStatusDraft draft, err := svc.Create(context.Background(), 1, draftRequest) require.NoError(t, err) - assertConcurrentCaptainSkillUpdate(t, svc, draft, true) + assertConcurrentCaptainSkillUpdate(t, db, svc, draft, true) } -func assertConcurrentCaptainSkillUpdate(t *testing.T, svc *CaptainSkillService, skill *model.CaptainSkill, statusOnly bool) { +func assertConcurrentCaptainSkillUpdate(t *testing.T, db *gorm.DB, svc *CaptainSkillService, skill *model.CaptainSkill, statusOnly bool) { t.Helper() + ready := make(chan struct{}, 2) + release := make(chan struct{}) + callbackName := "test:captain_skill_cas_barrier:" + t.Name() + require.NoError(t, db.Callback().Update().Before("gorm:update").Register(callbackName, func(tx *gorm.DB) { + if tx.Statement.Table == "captain_skills" { + ready <- struct{}{} + <-release + } + })) + defer func() { require.NoError(t, db.Callback().Update().Remove(callbackName)) }() type updateResult struct { skill *model.CaptainSkill err error @@ -216,12 +238,22 @@ func assertConcurrentCaptainSkillUpdate(t *testing.T, svc *CaptainSkillService, }() } close(start) + for range 2 { + select { + case <-ready: + case <-time.After(5 * time.Second): + close(release) + t.Fatal("concurrent updates did not both reach the repository CAS") + } + } + close(release) var winner *model.CaptainSkill conflicts := 0 for i := 0; i < 2; i++ { result := <-results if errors.Is(result.err, ErrCaptainSkillConflict) { + assert.ErrorContains(t, result.err, repository.ErrCaptainSkillVersionConflict.Error()) conflicts++ continue }