From 0124e392c7b36eaf6ed287c342ed582117f9a027 Mon Sep 17 00:00:00 2001 From: tamakyi Date: Thu, 29 Oct 2026 16:28:34 +0800 Subject: [PATCH] =?UTF-8?q?fix(correctness):=20=E4=B8=89=E5=A4=84=E5=B9=82?= =?UTF-8?q?=E7=AD=89=E7=BC=BA=E9=99=B7=E3=80=81=E7=A7=81=E8=81=8A=E9=A6=96?= =?UTF-8?q?=E6=9D=A1=E4=BA=8B=E5=8A=A1=E5=8C=96=E3=80=81=E9=AA=8C=E8=AF=81?= =?UTF-8?q?=E7=A0=81=E5=A4=B1=E8=B4=A5=E4=B8=8D=E5=86=8D=E4=B8=A2=E6=AD=A3?= =?UTF-8?q?=E6=96=87?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 四处都属于「并发/失败路径下状态不一致」,用户侧表现为「点了没反应」「发了没收到」。 1) 删除会话重复回退配额 MarkDeletedByOwner 是 check-then-act:双击时两个请求都读到 is_deleted_by_owner=false, 都执行 UPDATE、都执行一次 IncrementQuota(-1)。计数不会变负,但会白吃掉其他会话的额度, 表现为定量收集提前关闭,箱主重置计数也校准不回来。 → 改条件更新 + RowsAffected 决定是否回退。 2) 双击产生两条重复追问 「上一条追问必须先被回复」原来只在 handler 里判断(事务外):两个请求都能通过那次检查, 随后被 CreateIfBelowLimit 的问题行锁串行化,各自 count 都 < 3 —— 两条相同追问都建成, 吃掉 2/3 名额,箱主还收两封邮件。 → 判定挪进事务,新增 ErrFollowUpAwaitingReply;handler 的预检查保留作为快速失败。 3) 双击置顶结果与预期相反 SetPinned(id, !question.IsPinned) 是 read-modify-write:两个请求都读到 false 都写 true, 用户点两次期望「置顶后取消」,实际仍是「置顶」。 → 新增 SetPinnedIfCurrent(current, next),第二次因状态已变而 0 行,语义是「只切换一次」。 4) 私聊首条三步写入无事务 建会话 → 写首条消息 → 计数是三次独立写入,第二步失败会留下 message_count=0 的空会话, 箱主列表里能点进去却什么都没有;而 BeforeCreate 已占掉一个 6 字符 token(唯一索引), 孤儿会话永久占用它。 → 新增 CreateWithFirstMessage,三步收进一个事务。 5) 验证码失败丢正文 该分支原来走 SetErrorFlash + Redirect,而其它失败分支都是 SetError + Success 回填表单。 验证码失败恰恰是最常见的失败(token 过期、网络抖动),结果用户写好的正文整段丢失。 → 改为同请求渲染并回填;错误文案不再透传 err.Error()(可能含上游原始信息)。 6) 5 处裸类型断言改带 ok(share.go ×3 / context.go ×1 / user/vote.go ×1), 断言失败时丢弃脏缓存并走重建,而不是 panic 掉整个请求。share.go 的 ttlCache 补 Delete。 验证:内部 DB 测试 3 项(并发删除只回退一次配额 5→4、双击只产生一条追问、 双击只切换一次置顶);verify_round3.py 6/6(验证码失败 200+正文回填+不建提问、 私聊首条 message_count 与实际消息数一致)。 --- internal/context/context.go | 5 +- internal/db/conversation.go | 66 ++++++++++++- internal/db/follow_ups.go | 20 ++++ internal/db/idempotency_test.go | 162 ++++++++++++++++++++++++++++++++ internal/db/questions.go | 18 ++++ route/question/follow_up.go | 9 ++ route/question/page.go | 50 +++++----- route/question/question.go | 7 +- route/question/share.go | 30 +++++- 9 files changed, 332 insertions(+), 35 deletions(-) create mode 100644 internal/db/idempotency_test.go diff --git a/internal/context/context.go b/internal/context/context.go index bb7eb01..676170a 100644 --- a/internal/context/context.go +++ b/internal/context/context.go @@ -66,7 +66,10 @@ func (c *Context) HasError() bool { if !ok { return false } - return hasErr.(bool) + // 带 ok 的断言:Data 是 map[string]interface{},任何写入方都可能塞进非 bool 值, + // 裸断言遇到那种情况会直接 panic 掉整个请求。非 bool 一律视为「无错误」。 + v, isBool := hasErr.(bool) + return isBool && v } func (c *Context) SetError(err error, f ...interface{}) { diff --git a/internal/db/conversation.go b/internal/db/conversation.go index 3541609..40c4609 100644 --- a/internal/db/conversation.go +++ b/internal/db/conversation.go @@ -19,6 +19,9 @@ var Conversations ConversationStore type ConversationStore interface { Create(ctx context.Context, opts CreateConversationOptions) (*Conversation, error) + // CreateWithFirstMessage 事务化地建会话 + 首条消息 + 消息计数。 + // 单独写首条消息(用于「发第一条私聊」这类一次性动作),避免失败时留下空会话。 + CreateWithFirstMessage(ctx context.Context, opts CreateConversationOptions, senderID uint, content string) (*Conversation, *Message, error) GetByID(ctx context.Context, id uint) (*Conversation, error) GetByToken(ctx context.Context, token string) (*Conversation, error) GetByIDAndOwner(ctx context.Context, id, ownerID uint) (*Conversation, error) @@ -83,6 +86,52 @@ func (db *conversations) Create(ctx context.Context, opts CreateConversationOpti return conversation, nil } +// CreateWithFirstMessage 在一个事务里完成「建会话 + 写首条消息 + 消息计数」。 +// +// 原来这三步是三次独立写入:第二步失败时会话已经提交,于是留下一条 +// message_count=0、没有任何消息的「空会话」——箱主的私聊列表里能看到它, +// 点进去什么都没有;更麻烦的是 Conversations.Create 的 BeforeCreate 已经 +// 生成并占用了一个 6 字符 token(唯一索引),孤儿会话会永久占着这个 token。 +func (db *conversations) CreateWithFirstMessage( + ctx context.Context, opts CreateConversationOptions, senderID uint, content string, +) (*Conversation, *Message, error) { + var conv *Conversation + var msg *Message + + err := db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + conv = &Conversation{ + OwnerID: opts.OwnerID, + AskerID: opts.AskerID, + MaxMessages: opts.MaxMessages, + } + // 走 tx 而不是 db:BeforeCreate 里生成 token 的 hook 同样会被触发。 + if err := tx.Create(conv).Error; err != nil { + return errors.Wrap(err, "create conversation") + } + + msg = &Message{ + ConversationID: conv.ID, + SenderID: senderID, + Content: content, + } + if err := tx.Create(msg).Error; err != nil { + return errors.Wrap(err, "create first message") + } + + // 原子自增,与 IncrementMessageCount 保持同一写法 + if err := tx.Model(&Conversation{}). + Where("id = ?", conv.ID). + UpdateColumn("message_count", gorm.Expr("message_count + 1")).Error; err != nil { + return errors.Wrap(err, "increment message count") + } + return nil + }) + if err != nil { + return nil, nil, err + } + return conv, msg, nil +} + func (db *conversations) GetByID(ctx context.Context, id uint) (*Conversation, error) { var conv Conversation if err := db.WithContext(ctx).First(&conv, id).Error; err != nil { @@ -216,8 +265,21 @@ func (db *conversations) MarkDeletedByOwner(ctx context.Context, id uint) error return nil } - if err := db.WithContext(ctx).Model(&Conversation{}).Where("id = ?", id).Update("is_deleted_by_owner", true).Error; err != nil { - return errors.Wrap(err, "mark conversation deleted by owner") + // 用「条件更新 + RowsAffected」代替纯 Update: + // 原来的写法是 check-then-act,双击「删除会话」或前端重复提交时,两个请求 + // 都能读到 is_deleted_by_owner=false,于是都执行 UPDATE、都执行一次 + // IncrementQuota(-1)。IncrementQuota 有 quota_count>0 兜底不会变负, + // 但会白吃掉本用户其他会话的计数 —— 表现为定量收集提前关闭, + // 而箱主重置计数也校准不回来。 + res := db.WithContext(ctx).Model(&Conversation{}). + Where("id = ? AND is_deleted_by_owner = ?", id, false). + Update("is_deleted_by_owner", true) + if res.Error != nil { + return errors.Wrap(res.Error, "mark conversation deleted by owner") + } + if res.RowsAffected == 0 { + // 并发里被另一个请求抢先删掉了:它已经做过配额回退,这里不能再来一次。 + return nil } // 回退定量收集计数:删除私聊会话等价于少一条已收集内容, diff --git a/internal/db/follow_ups.go b/internal/db/follow_ups.go index 0c2dbec..65ddd06 100644 --- a/internal/db/follow_ups.go +++ b/internal/db/follow_ups.go @@ -18,6 +18,9 @@ var ( ErrFollowUpLimitReached = errors.New("follow up limit reached") ErrFollowUpNotFound = errors.New("follow up not found") ErrFollowUpAlreadyAnswered = errors.New("follow up already answered") + // ErrFollowUpAwaitingReply 上一条追问尚未被箱主回复,不允许继续追问。 + // 由 CreateIfBelowLimit 在事务内判定(见该方法的注释)。 + ErrFollowUpAwaitingReply = errors.New("previous follow up awaiting reply") ) var FollowUps FollowUpsStore @@ -84,6 +87,23 @@ func (db *followUps) CreateIfBelowLimit(ctx context.Context, opts CreateFollowUp return ErrFollowUpLimitReached } + // 「上一条追问必须先被回复」也必须在事务内判断。 + // + // 这段校验原来只在 handler 里做(事务外):双击提交时两个请求都读到 + // 「最新一条还没回复」,随后被这行 FOR UPDATE 串行化,各自 count 都 < 3, + // 于是两条内容相同的追问都创建成功 —— 吃掉 2/3 名额,箱主还会收两封邮件。 + // 行锁只保护「条数」,不保护「业务前置条件」,所以判断必须挪进来。 + var latest FollowUp + if err := tx.Where("question_id = ?", opts.QuestionID). + Order("id DESC"). // 用 id 而非 created_at:同毫秒写入时时间排序不稳定 + Limit(1). + Find(&latest).Error; err != nil { + return errors.Wrap(err, "get latest follow up for reply check") + } + if latest.ID != 0 && latest.Answer == "" { + return ErrFollowUpAwaitingReply + } + f := &FollowUp{ QuestionID: opts.QuestionID, Content: opts.Content, diff --git a/internal/db/idempotency_test.go b/internal/db/idempotency_test.go new file mode 100644 index 0000000..ab4250b --- /dev/null +++ b/internal/db/idempotency_test.go @@ -0,0 +1,162 @@ +// Copyright 2026 tamakyi. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package db + +import ( + "context" + "sync" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestSetPinnedIfCurrentOnlyAppliesOnce 覆盖「双击置顶」这个场景。 +// +// 原来的 handler 是 read-modify-write:SetPinned(id, !question.IsPinned)。 +// 双击时两个请求都读到 is_pinned=false,都写 true,于是用户点了两次、期望 +// 「置顶后取消」,结果仍是「置顶」。现在加 current 条件,第二次会因为状态 +// 已变而影响 0 行 —— 语义变成「只切换一次」,符合用户本意(手抖点两下)。 +func TestSetPinnedIfCurrentOnlyAppliesOnce(t *testing.T) { + database, cleanup := newTestDB(t) + ctx := context.Background() + t.Cleanup(func() { + if err := cleanup("questions"); err != nil { + t.Fatal(err) + } + }) + + store := NewQuestionsStore(database) + q, err := store.Create(ctx, CreateQuestionOptions{ + FromIP: "114.5.1.4", + UserID: 1, + Content: "试试置顶", + }) + require.NoError(t, err) + require.False(t, q.IsPinned) + + // 第一次切换:false -> true,生效 + applied, err := store.SetPinnedIfCurrent(ctx, q.ID, false, true) + require.NoError(t, err) + require.True(t, applied, "第一次切换应生效") + + // 第二次切换:请求方仍然以为当前是 false(它读到的旧值),写入 true + // 此时数据库已经是 true,条件不成立 -> 0 行 + applied, err = store.SetPinnedIfCurrent(ctx, q.ID, false, true) + require.NoError(t, err) + require.False(t, applied, "状态已变,第二次切换不应再生效") + + got, err := store.GetByID(ctx, q.ID) + require.NoError(t, err) + require.True(t, got.IsPinned, "最终状态应仍是置顶,而不是被双击反转") + + // 正常取消置顶(条件匹配) + applied, err = store.SetPinnedIfCurrent(ctx, q.ID, true, false) + require.NoError(t, err) + require.True(t, applied) + got, err = store.GetByID(ctx, q.ID) + require.NoError(t, err) + require.False(t, got.IsPinned) +} + +// TestFollowUpRejectsSecondWhilePreviousUnanswered 覆盖「追问必须等主人回复」。 +// +// 这条规则原来只在 handler 里判断(事务外)。双击提交时两个请求都能通过那次检查, +// 随后被 CreateIfBelowLimit 的问题行锁串行化,各自 count 都 < 3 —— 两条内容相同的 +// 追问都创建成功,吃掉 2/3 名额,箱主还会收到两封邮件。 +// 现在判定挪进事务,第二个请求会拿到 ErrFollowUpAwaitingReply。 +func TestFollowUpRejectsSecondWhilePreviousUnanswered(t *testing.T) { + database, cleanup := newTestDB(t) + ctx := context.Background() + t.Cleanup(func() { + if err := cleanup("follow_ups", "questions"); err != nil { + t.Fatal(err) + } + }) + + qStore := NewQuestionsStore(database) + q, err := qStore.Create(ctx, CreateQuestionOptions{ + FromIP: "114.5.1.4", + UserID: 1, + Content: "允许我追问吗", + }) + require.NoError(t, err) + + fStore := NewFollowUpsStore(database) + + created, err := fStore.CreateIfBelowLimit(ctx, CreateFollowUpOptions{ + QuestionID: q.ID, + Content: "第一次追问", + }) + require.NoError(t, err) + require.True(t, created) + + // 第一条还没被回复:第二次追问必须被拒 + _, err = fStore.CreateIfBelowLimit(ctx, CreateFollowUpOptions{ + QuestionID: q.ID, + Content: "第一次追问", // 内容相同,模拟双击 + }) + require.ErrorIs(t, err, ErrFollowUpAwaitingReply) + + // 确认库里只有一条(没有因为双击写入两条) + list, err := fStore.GetByQuestionID(ctx, q.ID) + require.NoError(t, err) + require.Len(t, list, 1, "双击不应产生两条追问") + + // 主人回复后,允许继续追问 + require.NoError(t, fStore.AnswerByID(ctx, q.ID, list[0].ID, "答:可以")) + created, err = fStore.CreateIfBelowLimit(ctx, CreateFollowUpOptions{ + QuestionID: q.ID, + Content: "第二次追问", + }) + require.NoError(t, err) + require.True(t, created, "上一条已回复后应允许继续追问") +} + +// TestMarkDeletedByOwnerConcurrentDecrementsQuotaOnce 覆盖并发删除会话时的配额回退。 +// +// 原来是 check-then-act:两次并发请求都读到 is_deleted_by_owner=false, +// 都执行 UPDATE、都执行一次 IncrementQuota(-1)。计数不会变负,但会白吃掉 +// 其他会话的额度,表现为定量收集提前关闭。 +func TestMarkDeletedByOwnerConcurrentDecrementsQuotaOnce(t *testing.T) { + database, cleanup := newTestDB(t) + ctx := context.Background() + t.Cleanup(func() { + if err := cleanup("conversations", "feature_controls"); err != nil { + t.Fatal(err) + } + }) + + const ownerID = 7 + + // 先把私聊配额计数做到 5 + fcStore := NewFeatureControlsStore(database) + require.NoError(t, fcStore.IncrementQuota(ctx, ownerID, FeatureChat, 5)) + before, err := fcStore.Get(ctx, ownerID, FeatureChat) + require.NoError(t, err) + require.EqualValues(t, 5, before.QuotaCount) + + convStore := NewConversationStore(database) + conv, err := convStore.Create(ctx, CreateConversationOptions{OwnerID: ownerID}) + require.NoError(t, err) + + // 并发两次删除(模拟双击 / 前端重复提交) + var wg sync.WaitGroup + errs := make([]error, 2) + for i := 0; i < 2; i++ { + wg.Add(1) + go func(idx int) { + defer wg.Done() + errs[idx] = convStore.MarkDeletedByOwner(ctx, conv.ID) + }(i) + } + wg.Wait() + require.NoError(t, errs[0]) + require.NoError(t, errs[1]) + + after, err := fcStore.Get(ctx, ownerID, FeatureChat) + require.NoError(t, err) + require.EqualValues(t, 4, after.QuotaCount, + "两个并发删除请求只应回退一次配额(5 -> 4),而不是两次(5 -> 3)") +} diff --git a/internal/db/questions.go b/internal/db/questions.go index 15c0231..0d4cb7c 100644 --- a/internal/db/questions.go +++ b/internal/db/questions.go @@ -42,6 +42,13 @@ type QuestionsStore interface { SetPrivate(ctx context.Context, id uint) error SetPublic(ctx context.Context, id uint) error SetPinned(ctx context.Context, id uint, pinned bool) error + // SetPinnedIfCurrent 仅在「当前置顶状态 == current」时才写成 pinned,返回是否真的写入。 + // + // 用于「切换」这类 read-modify-write:handler 拿到的 is_pinned 是本次请求早期读的, + // 双击或并发提交时两个请求都会算出「取反」并写同一个值,状态机不是原子的 + // (用户点两次期望「置顶后又取消」,实际仍是「置顶」)。 + // 加上 current 条件后,第二次会因为状态已变而 0 行 —— 语义上「只切换一次」。 + SetPinnedIfCurrent(ctx context.Context, id uint, current, pinned bool) (bool, error) SetRead(ctx context.Context, id uint) error SetUnread(ctx context.Context, id uint) error // NextUnread 取该用户最早的一条未读提问(未读 = is_read=false 且未回答)。 @@ -512,6 +519,17 @@ func (db *questions) SetPinned(ctx context.Context, id uint, pinned bool) error return db.WithContext(ctx).Model(&Question{}).Where("id = ?", id).Update("is_pinned", pinned).Error } +// SetPinnedIfCurrent 条件更新置顶状态(详见接口注释)。 +func (db *questions) SetPinnedIfCurrent(ctx context.Context, id uint, current, pinned bool) (bool, error) { + res := db.WithContext(ctx).Model(&Question{}). + Where("id = ? AND is_pinned = ?", id, current). + Update("is_pinned", pinned) + if res.Error != nil { + return false, errors.Wrap(res.Error, "set question pinned if current") + } + return res.RowsAffected > 0, nil +} + func (db *questions) SetPublic(ctx context.Context, id uint) error { return db.WithContext(ctx).Model(&Question{}).Where("id = ?", id).Update("is_private", false).Error } diff --git a/route/question/follow_up.go b/route/question/follow_up.go index dacd3b0..0b3824b 100644 --- a/route/question/follow_up.go +++ b/route/question/follow_up.go @@ -82,6 +82,9 @@ func PublishFollowUp(ctx context.Context, pageUser *db.User, question *db.Questi // 服务端校验"等主人回复":模板仅在前一条被回复后才展示追问表单, // 但 POST 可被直接构造绕过,因此必须检查最新一条追问是否已回复。 + // + // 这一段只是「快速失败」省掉一次事务开销;并发/双击下的正确性由 + // db.FollowUps.CreateIfBelowLimit 在事务内重新判定(见 ErrFollowUpAwaitingReply)。 latest, err := db.FollowUps.GetLatestByQuestionID(ctx.Request().Context(), question.ID) if err != nil { logrus.WithContext(ctx.Request().Context()).WithError(err).Error("Failed to get latest follow up") @@ -118,6 +121,12 @@ func PublishFollowUp(ctx context.Context, pageUser *db.User, question *db.Questi ctx.Redirect(fmt.Sprintf("/_/%s/%d?t=%s", pageUser.Domain, question.ID, question.Token)) return } + if errors.Is(err, db.ErrFollowUpAwaitingReply) { + // 事务层的兜底(并发双击时会走到这里) + ctx.SetErrorFlash(i18n.T(ctx.Lang(), "followup.errWaitReply")) + ctx.Redirect(fmt.Sprintf("/_/%s/%d?t=%s", pageUser.Domain, question.ID, question.Token)) + return + } logrus.WithContext(ctx.Request().Context()).WithError(err).Error("Failed to create follow up") ctx.SetInternalErrorFlash() ctx.Redirect(fmt.Sprintf("/_/%s/%d?t=%s", pageUser.Domain, question.ID, question.Token)) diff --git a/route/question/page.go b/route/question/page.go index 728e951..504b2dc 100644 --- a/route/question/page.go +++ b/route/question/page.go @@ -518,34 +518,28 @@ func New(ctx context.Context, f form.NewQuestion, pageUser *db.User, recaptcha r maxMessages = conf.Limits.ChatMaxMessages } - conv, err := db.Conversations.Create(ctx.Request().Context(), db.CreateConversationOptions{ - OwnerID: pageUser.ID, - AskerID: ctx.User.ID, - MaxMessages: maxMessages, - }) + // 建会话 + 首条消息,一个事务写完。 + // 原来三步分开写:第二步失败时会话已经落库, + // 留下一条 message_count=0 、没有任何消息的「空会话」 + // (箱主的私聊列表里能看到、点进去什么都没有), + // 而且 BeforeCreate 已经占掉一个 6 字符 token(唯一索引),孤儿会话会永久占用它。 + conv, _, err := db.Conversations.CreateWithFirstMessage( + ctx.Request().Context(), + db.CreateConversationOptions{ + OwnerID: pageUser.ID, + AskerID: ctx.User.ID, + MaxMessages: maxMessages, + }, + ctx.User.ID, + f.Content, + ) if err != nil { - logrus.WithContext(ctx.Request().Context()).WithError(err).Error("Failed to create conversation") + logrus.WithContext(ctx.Request().Context()).WithError(err).Error("Failed to create conversation with first message") ctx.SetInternalError(f) ctx.Success("question/list") return } - _, err = db.Messages.Create(ctx.Request().Context(), db.CreateMessageOptions{ - ConversationID: conv.ID, - SenderID: ctx.User.ID, - Content: f.Content, - }) - if err != nil { - logrus.WithContext(ctx.Request().Context()).WithError(err).Error("Failed to create first message") - ctx.SetInternalError(f) - ctx.Success("question/list") - return - } - - if err := db.Conversations.IncrementMessageCount(ctx.Request().Context(), conv.ID); err != nil { - logrus.WithContext(ctx.Request().Context()).WithError(err).Error("Failed to increment message count") - } - // 私聊定量收集:新建会话即计一条(后续往来消息不重复计数) if err := db.FeatureControls.IncrementQuota(ctx.Request().Context(), pageUser.ID, db.FeatureChat, 1); err != nil { logrus.WithContext(ctx.Request().Context()).WithError(err).Warn("Failed to increment chat quota") @@ -607,13 +601,17 @@ func New(ctx context.Context, f form.NewQuestion, pageUser *db.User, recaptcha r ok, err := captcha.Verify(f.Recaptcha, shared.ResolveClientIP(ctx.Request().Request), ctx.Session, recaptcha, ctx.Lang()) if err != nil { logrus.WithContext(ctx.Request().Context()).WithError(err).Error("Failed to check captcha") - ctx.SetErrorFlash(err.Error()) - ctx.Redirect("/_/" + pageUser.Domain) + // 不把 err.Error() 直接给用户:里面可能带上游返回的原始信息。 + ctx.SetError(errors.New(i18n.T(ctx.Lang(), "captcha.error")), f) + ctx.Success("question/list") return } if !ok { - ctx.SetErrorFlash(i18n.T(ctx.Lang(), "captcha.error")) - ctx.Redirect("/_/" + pageUser.Domain) + // 验证码失败是这里最常见的失败(token 过期、网络抖动)。 + // 原来走 Flash + Redirect 会把用户已经写好的正文整段丢掉; + // 这里与其他失败分支一致,用 SetError + Success 回填表单。 + ctx.SetError(errors.New(i18n.T(ctx.Lang(), "captcha.error")), f) + ctx.Success("question/list") return } diff --git a/route/question/question.go b/route/question/question.go index 37b7298..5e7a637 100644 --- a/route/question/question.go +++ b/route/question/question.go @@ -370,7 +370,12 @@ func TogglePin(ctx context.Context, pageUser *db.User, question *db.Question, ca return } - if err := db.Questions.SetPinned(ctx.Request().Context(), question.ID, !question.IsPinned); err != nil { + // 条件更新:只有当前状态仍是这次读到的值时才写入。 + // 原来直接 SetPinned(!question.IsPinned) 是 read-modify-write,双击时两个请求 + // 都读到 false、都写 true,结果停在「置顶」与用户预期的「取消」 + // 相反。现在第二次会因状态已变而影响 0 行,即「只切换一次」。 + if _, err := db.Questions.SetPinnedIfCurrent( + ctx.Request().Context(), question.ID, question.IsPinned, !question.IsPinned); err != nil { logrus.WithContext(ctx.Request().Context()).WithError(err).Error("Failed to toggle question pinned") ctx.SetInternalError() ctx.Success("question/item") diff --git a/route/question/share.go b/route/question/share.go index 0effadb..83cfeb6 100644 --- a/route/question/share.go +++ b/route/question/share.go @@ -369,6 +369,15 @@ func (c *ttlCache) Get(key string) (interface{}, bool) { return item.value, true } +// Delete 移除单条缓存。 +// 用于读到「类型不符的脏数据」时丢弃这一条 —— 没有它就只能等下次 Set 覆盖, +// 而重建路径万一再次失败,这条脏数据会一直留在缓存里被反复读到。 +func (c *ttlCache) Delete(key string) { + c.mu.Lock() + defer c.mu.Unlock() + delete(c.items, key) +} + func (c *ttlCache) Set(key string, value interface{}, ttl time.Duration) { c.mu.Lock() defer c.mu.Unlock() @@ -447,8 +456,13 @@ func ShareCard(ctx context.Context) { // 命中缓存直接返回,避免重复下载图片与重新绘制 cacheKey := shareCardCacheKey(user) if cached, ok := shareCardCache.Get(cacheKey); ok { - writeShareCardResponse(ctx, cached.([]byte)) - return + if data, isBytes := cached.([]byte); isBytes { + writeShareCardResponse(ctx, data) + return + } + // 类型不符:理论上不会发生(key 由本文件独占写入),但裸断言遇到不符就是 panic。 + // 丢弃这条脏缓存,走下面重新绘制。 + shareCardCache.Delete(cacheKey) } canvas := image.NewRGBA(image.Rect(0, 0, CardWidth, CardHeight)) @@ -712,7 +726,10 @@ func loadImage(url string, w, h int) (image.Image, error) { key := fmt.Sprintf("%s|%dx%d", url, w, h) if cached, ok := imageCache.Get(key); ok { - return cached.(image.Image), nil + if img, isImage := cached.(image.Image); isImage { + return img, nil + } + imageCache.Delete(key) } resp, err := shareHTTPClient.Get(url) @@ -1087,8 +1104,11 @@ func QuestionShareCard(ctx context.Context) { cacheKey := questionShareCardCacheKey(user, question) if cached, ok := questionShareCache.Get(cacheKey); ok { - writeShareCardResponse(ctx, cached.([]byte)) - return + if data, isBytes := cached.([]byte); isBytes { + writeShareCardResponse(ctx, data) + return + } + questionShareCache.Delete(cacheKey) } canvas := image.NewRGBA(image.Rect(0, 0, CardWidth, CardHeight))