refactor(channelserver): standardize on BeginTxx for all repository transactions

Replace db.Begin() with db.BeginTxx(context.Background(), nil) across all
8 remaining call sites in repo_guild.go, repo_guild_rp.go, repo_festa.go,
and repo_event.go. Use deferred Rollback() instead of explicit rollback
at each error return, eliminating 15 manual rollback calls.
This commit is contained in:
Houmgaor
2026-02-22 16:55:59 +01:00
parent 2acbb5d03a
commit 59fd722d37
5 changed files with 30 additions and 38 deletions

View File

@@ -114,19 +114,9 @@ The pattern `m.Field, _ = stringsupport.SJISToUTF8(...)` appears across:
A malformed SJIS string from a client yields an empty string with no log output, making garbled text impossible to debug. The error return should at least be logged at debug level. A malformed SJIS string from a client yields an empty string with no log output, making garbled text impossible to debug. The error return should at least be logged at debug level.
### 6. Inconsistent transaction API ### ~~6. Inconsistent transaction API~~ (Fixed)
`repo_guild.go` mixes two transaction styles in the same file: **Status:** Fixed. All transaction call sites now use `BeginTxx(context.Background(), nil)` with deferred rollback, replacing the old `Begin()` + manual rollback pattern across `repo_guild.go`, `repo_guild_rp.go`, `repo_festa.go`, and `repo_event.go`.
```go
// Line 175 — no context, old style
tx, err := r.db.Begin()
// Line 518 — sqlx-idiomatic, with context
tx, err := r.db.BeginTxx(context.Background(), nil)
```
Should standardize on `BeginTxx` throughout. The `Begin()` calls cannot carry a context for cancellation or timeout.
### ~~7. `LoopDelay` config has no Viper default~~ (Fixed) ### ~~7. `LoopDelay` config has no Viper default~~ (Fixed)
@@ -162,4 +152,4 @@ Based on impact and the momentum from recent repo-interface refactoring:
5. ~~**Fix `fmt.Sprintf` in logger calls**~~**Done** 5. ~~**Fix `fmt.Sprintf` in logger calls**~~**Done**
6. ~~**Add `LoopDelay` Viper default**~~**Done** 6. ~~**Add `LoopDelay` Viper default**~~**Done**
7. **Log SJIS decoding errors** — improves debuggability for text issues 7. **Log SJIS decoding errors** — improves debuggability for text issues
8. **Standardize on `BeginTxx`**consistency fix in `repo_guild.go` 8. ~~**Standardize on `BeginTxx`**~~**Done**

View File

@@ -1,6 +1,7 @@
package channelserver package channelserver
import ( import (
"context"
"time" "time"
"github.com/jmoiron/sqlx" "github.com/jmoiron/sqlx"
@@ -79,13 +80,14 @@ func (r *EventRepository) UpdateEventQuestStartTimes(updates []EventQuestUpdate)
if len(updates) == 0 { if len(updates) == 0 {
return nil return nil
} }
tx, err := r.db.Begin() tx, err := r.db.BeginTxx(context.Background(), nil)
if err != nil { if err != nil {
return err return err
} }
defer func() { _ = tx.Rollback() }()
for _, u := range updates { for _, u := range updates {
if _, err := tx.Exec("UPDATE event_quests SET start_time = $1 WHERE id = $2", u.StartTime, u.ID); err != nil { if _, err := tx.Exec("UPDATE event_quests SET start_time = $1 WHERE id = $2", u.StartTime, u.ID); err != nil {
_ = tx.Rollback()
return err return err
} }
} }

View File

@@ -1,6 +1,7 @@
package channelserver package channelserver
import ( import (
"context"
"database/sql" "database/sql"
"github.com/jmoiron/sqlx" "github.com/jmoiron/sqlx"
@@ -181,16 +182,17 @@ func (r *FestaRepository) RegisterGuild(guildID uint32, team string) error {
// SubmitSouls records soul submissions for a character within a transaction. // SubmitSouls records soul submissions for a character within a transaction.
func (r *FestaRepository) SubmitSouls(charID, guildID uint32, souls []uint16) error { func (r *FestaRepository) SubmitSouls(charID, guildID uint32, souls []uint16) error {
tx, err := r.db.Begin() tx, err := r.db.BeginTxx(context.Background(), nil)
if err != nil { if err != nil {
return err return err
} }
defer func() { _ = tx.Rollback() }()
for i, s := range souls { for i, s := range souls {
if s == 0 { if s == 0 {
continue continue
} }
if _, err := tx.Exec(`INSERT INTO festa_submissions VALUES ($1, $2, $3, $4, now())`, charID, guildID, i, s); err != nil { if _, err := tx.Exec(`INSERT INTO festa_submissions VALUES ($1, $2, $3, $4, now())`, charID, guildID, i, s); err != nil {
_ = tx.Rollback()
return err return err
} }
} }

View File

@@ -1,6 +1,7 @@
package channelserver package channelserver
import ( import (
"context"
"database/sql" "database/sql"
"errors" "errors"
"fmt" "fmt"
@@ -168,10 +169,11 @@ func (r *GuildRepository) ListAll() ([]*Guild, error) {
// Create creates a new guild and adds the leader as its first member. // Create creates a new guild and adds the leader as its first member.
func (r *GuildRepository) Create(leaderCharID uint32, guildName string) (int32, error) { func (r *GuildRepository) Create(leaderCharID uint32, guildName string) (int32, error) {
tx, err := r.db.Begin() tx, err := r.db.BeginTxx(context.Background(), nil)
if err != nil { if err != nil {
return 0, err return 0, err
} }
defer func() { _ = tx.Rollback() }()
var guildID int32 var guildID int32
err = tx.QueryRow( err = tx.QueryRow(
@@ -179,13 +181,11 @@ func (r *GuildRepository) Create(leaderCharID uint32, guildName string) (int32,
guildName, leaderCharID, guildName, leaderCharID,
).Scan(&guildID) ).Scan(&guildID)
if err != nil { if err != nil {
_ = tx.Rollback()
return 0, err return 0, err
} }
_, err = tx.Exec(`INSERT INTO guild_characters (guild_id, character_id) VALUES ($1, $2)`, guildID, leaderCharID) _, err = tx.Exec(`INSERT INTO guild_characters (guild_id, character_id) VALUES ($1, $2)`, guildID, leaderCharID)
if err != nil { if err != nil {
_ = tx.Rollback()
return 0, err return 0, err
} }
@@ -207,10 +207,11 @@ func (r *GuildRepository) Save(guild *Guild) error {
// Disband removes a guild, its members, and cleans up alliance references. // Disband removes a guild, its members, and cleans up alliance references.
func (r *GuildRepository) Disband(guildID uint32) error { func (r *GuildRepository) Disband(guildID uint32) error {
tx, err := r.db.Begin() tx, err := r.db.BeginTxx(context.Background(), nil)
if err != nil { if err != nil {
return err return err
} }
defer func() { _ = tx.Rollback() }()
stmts := []string{ stmts := []string{
"DELETE FROM guild_characters WHERE guild_id = $1", "DELETE FROM guild_characters WHERE guild_id = $1",
@@ -219,17 +220,14 @@ func (r *GuildRepository) Disband(guildID uint32) error {
} }
for _, stmt := range stmts { for _, stmt := range stmts {
if _, err := tx.Exec(stmt, guildID); err != nil { if _, err := tx.Exec(stmt, guildID); err != nil {
_ = tx.Rollback()
return err return err
} }
} }
if _, err := tx.Exec("UPDATE guild_alliances SET sub1_id=sub2_id, sub2_id=NULL WHERE sub1_id=$1", guildID); err != nil { if _, err := tx.Exec("UPDATE guild_alliances SET sub1_id=sub2_id, sub2_id=NULL WHERE sub1_id=$1", guildID); err != nil {
_ = tx.Rollback()
return err return err
} }
if _, err := tx.Exec("UPDATE guild_alliances SET sub2_id=NULL WHERE sub2_id=$1", guildID); err != nil { if _, err := tx.Exec("UPDATE guild_alliances SET sub2_id=NULL WHERE sub2_id=$1", guildID); err != nil {
_ = tx.Rollback()
return err return err
} }
@@ -244,13 +242,13 @@ func (r *GuildRepository) RemoveCharacter(charID uint32) error {
// AcceptApplication deletes the application and adds the character to the guild. // AcceptApplication deletes the application and adds the character to the guild.
func (r *GuildRepository) AcceptApplication(guildID, charID uint32) error { func (r *GuildRepository) AcceptApplication(guildID, charID uint32) error {
tx, err := r.db.Begin() tx, err := r.db.BeginTxx(context.Background(), nil)
if err != nil { if err != nil {
return err return err
} }
defer func() { _ = tx.Rollback() }()
if _, err := tx.Exec(`DELETE FROM guild_applications WHERE character_id = $1`, charID); err != nil { if _, err := tx.Exec(`DELETE FROM guild_applications WHERE character_id = $1`, charID); err != nil {
_ = tx.Rollback()
return err return err
} }
@@ -258,7 +256,6 @@ func (r *GuildRepository) AcceptApplication(guildID, charID uint32) error {
INSERT INTO guild_characters (guild_id, character_id, order_index) INSERT INTO guild_characters (guild_id, character_id, order_index)
VALUES ($1, $2, (SELECT MAX(order_index) + 1 FROM guild_characters WHERE guild_id = $1)) VALUES ($1, $2, (SELECT MAX(order_index) + 1 FROM guild_characters WHERE guild_id = $1))
`, guildID, charID); err != nil { `, guildID, charID); err != nil {
_ = tx.Rollback()
return err return err
} }
@@ -275,18 +272,18 @@ func (r *GuildRepository) CreateApplication(guildID, charID, actorID uint32, app
// CreateApplicationWithMail atomically creates an application and sends a notification mail. // CreateApplicationWithMail atomically creates an application and sends a notification mail.
func (r *GuildRepository) CreateApplicationWithMail(guildID, charID, actorID uint32, appType GuildApplicationType, mailSenderID, mailRecipientID uint32, mailSubject, mailBody string) error { func (r *GuildRepository) CreateApplicationWithMail(guildID, charID, actorID uint32, appType GuildApplicationType, mailSenderID, mailRecipientID uint32, mailSubject, mailBody string) error {
tx, err := r.db.Begin() tx, err := r.db.BeginTxx(context.Background(), nil)
if err != nil { if err != nil {
return err return err
} }
defer func() { _ = tx.Rollback() }()
if _, err := tx.Exec( if _, err := tx.Exec(
`INSERT INTO guild_applications (guild_id, character_id, actor_id, application_type) VALUES ($1, $2, $3, $4)`, `INSERT INTO guild_applications (guild_id, character_id, actor_id, application_type) VALUES ($1, $2, $3, $4)`,
guildID, charID, actorID, appType); err != nil { guildID, charID, actorID, appType); err != nil {
_ = tx.Rollback()
return err return err
} }
if _, err := tx.Exec(mailInsertQuery, mailSenderID, mailRecipientID, mailSubject, mailBody, 0, 0, true, false); err != nil { if _, err := tx.Exec(mailInsertQuery, mailSenderID, mailRecipientID, mailSubject, mailBody, 0, 0, true, false); err != nil {
_ = tx.Rollback()
return err return err
} }
return tx.Commit() return tx.Commit()
@@ -312,14 +309,14 @@ func (r *GuildRepository) RejectApplication(guildID, charID uint32) error {
// ArrangeCharacters reorders guild members by updating their order_index values. // ArrangeCharacters reorders guild members by updating their order_index values.
func (r *GuildRepository) ArrangeCharacters(charIDs []uint32) error { func (r *GuildRepository) ArrangeCharacters(charIDs []uint32) error {
tx, err := r.db.Begin() tx, err := r.db.BeginTxx(context.Background(), nil)
if err != nil { if err != nil {
return err return err
} }
defer func() { _ = tx.Rollback() }()
for i, id := range charIDs { for i, id := range charIDs {
if _, err := tx.Exec("UPDATE guild_characters SET order_index = $1 WHERE character_id = $2", 2+i, id); err != nil { if _, err := tx.Exec("UPDATE guild_characters SET order_index = $1 WHERE character_id = $2", 2+i, id); err != nil {
_ = tx.Rollback()
return err return err
} }
} }

View File

@@ -1,6 +1,9 @@
package channelserver package channelserver
import "time" import (
"context"
"time"
)
// AddMemberDailyRP adds RP to a member's daily total. // AddMemberDailyRP adds RP to a member's daily total.
func (r *GuildRepository) AddMemberDailyRP(charID uint32, amount uint16) error { func (r *GuildRepository) AddMemberDailyRP(charID uint32, amount uint16) error {
@@ -56,36 +59,34 @@ func (r *GuildRepository) SetRoomExpiry(guildID uint32, expiry time.Time) error
// then updates the guild's rp_reset_at timestamp. // then updates the guild's rp_reset_at timestamp.
// Uses SELECT FOR UPDATE to prevent concurrent rollovers from racing. // Uses SELECT FOR UPDATE to prevent concurrent rollovers from racing.
func (r *GuildRepository) RolloverDailyRP(guildID uint32, noon time.Time) error { func (r *GuildRepository) RolloverDailyRP(guildID uint32, noon time.Time) error {
tx, err := r.db.Begin() tx, err := r.db.BeginTxx(context.Background(), nil)
if err != nil { if err != nil {
return err return err
} }
defer func() { _ = tx.Rollback() }()
// Lock the guild row and re-check whether rollover is still needed. // Lock the guild row and re-check whether rollover is still needed.
var rpResetAt time.Time var rpResetAt time.Time
if err := tx.QueryRow( if err := tx.QueryRow(
`SELECT COALESCE(rp_reset_at, '2000-01-01'::timestamptz) FROM guilds WHERE id = $1 FOR UPDATE`, `SELECT COALESCE(rp_reset_at, '2000-01-01'::timestamptz) FROM guilds WHERE id = $1 FOR UPDATE`,
guildID, guildID,
).Scan(&rpResetAt); err != nil { ).Scan(&rpResetAt); err != nil {
_ = tx.Rollback()
return err return err
} }
if !rpResetAt.Before(noon) { if !rpResetAt.Before(noon) {
// Another goroutine already rolled over; nothing to do. // Another goroutine already rolled over; nothing to do.
_ = tx.Rollback()
return nil return nil
} }
if _, err := tx.Exec( if _, err := tx.Exec(
`UPDATE guild_characters SET rp_yesterday = rp_today, rp_today = 0 WHERE guild_id = $1`, `UPDATE guild_characters SET rp_yesterday = rp_today, rp_today = 0 WHERE guild_id = $1`,
guildID, guildID,
); err != nil { ); err != nil {
_ = tx.Rollback()
return err return err
} }
if _, err := tx.Exec( if _, err := tx.Exec(
`UPDATE guilds SET rp_reset_at = $1 WHERE id = $2`, `UPDATE guilds SET rp_reset_at = $1 WHERE id = $2`,
noon, guildID, noon, guildID,
); err != nil { ); err != nil {
_ = tx.Rollback()
return err return err
} }
return tx.Commit() return tx.Commit()