From 09eaa5b7e3acefcde37cf6cce8a8317ad24568cf Mon Sep 17 00:00:00 2001 From: Luke Gao Date: Sat, 30 May 2026 15:33:54 +0800 Subject: [PATCH 01/27] feat: add reasoning content to AI conversation records and update related components (#1530) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #1524 Root cause DeepSeek's reasoning models stream reasoning_content alongside content. Answer ignored it, so follow-up requests failed with 400: The reasoning_content in the thinking mode must be passed back to the API, and the thinking text was never shown or saved. Fix - Capture reasoning_content from the stream and pass it back to theAPI on subsequent rounds. - Persist it with the conversation (new DB column via migrationv2.0.2). - Render it in the chat UI as a collapsible "Thinking…/Thoughts"panel above the answer. Compatibility Nullable column, omitempty field, UI hides the panel when empty — old conversations and non-reasoning models behave exactly as before. Demo https://github.com/user-attachments/assets/49b1a2a1-9133-4ac2-bbeb-860215a50285 --- docs/docs.go | 3 + docs/swagger.json | 3 + docs/swagger.yaml | 2 + i18n/en_US.yaml | 2 + internal/controller/ai_controller.go | 74 +++++++++++++------ internal/entity/ai_conversation_record.go | 1 + internal/migrations/migrations.go | 1 + internal/migrations/v33.go | 39 ++++++++++ internal/schema/ai_conversation_schema.go | 1 + .../ai_conversation_service.go | 9 +++ ui/src/common/interface.ts | 1 + ui/src/components/BubbleAi/index.tsx | 38 ++++++++++ .../components/DetailModal/index.tsx | 1 + ui/src/pages/AiAssistant/index.tsx | 15 +++- .../pages/Search/components/AiCard/index.tsx | 15 +++- 15 files changed, 175 insertions(+), 30 deletions(-) create mode 100644 internal/migrations/v33.go diff --git a/docs/docs.go b/docs/docs.go index 57a23d43..03b06701 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -8781,6 +8781,9 @@ const docTemplate = `{ "helpful": { "type": "integer" }, + "reasoning_content": { + "type": "string" + }, "role": { "type": "string" }, diff --git a/docs/swagger.json b/docs/swagger.json index dac2b38f..3bfb2e3c 100644 --- a/docs/swagger.json +++ b/docs/swagger.json @@ -8754,6 +8754,9 @@ "helpful": { "type": "integer" }, + "reasoning_content": { + "type": "string" + }, "role": { "type": "string" }, diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 7a7adb68..c08f1e8d 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -228,6 +228,8 @@ definitions: type: integer helpful: type: integer + reasoning_content: + type: string role: type: string unhelpful: diff --git a/i18n/en_US.yaml b/i18n/en_US.yaml index 9a0d198b..61f49650 100644 --- a/i18n/en_US.yaml +++ b/i18n/en_US.yaml @@ -867,6 +867,8 @@ ui: copy: Copy ask_a_follow_up: Ask a follow-up ask_placeholder: Ask a question + thinking: Thinking… + thoughts: Thoughts notifications: title: Notifications inbox: Inbox diff --git a/internal/controller/ai_controller.go b/internal/controller/ai_controller.go index 125cdab2..e7495253 100644 --- a/internal/controller/ai_controller.go +++ b/internal/controller/ai_controller.go @@ -143,8 +143,9 @@ type StreamChoice struct { } type Delta struct { - Role string `json:"role,omitempty"` - Content string `json:"content,omitempty"` + Role string `json:"role,omitempty"` + Content string `json:"content,omitempty"` + ReasoningContent string `json:"reasoning_content,omitempty"` } type Usage struct { @@ -443,14 +444,15 @@ func (c *AIController) handleAIConversation(ctx *gin.Context, w http.ResponseWri Stream: true, } - toolCalls, newMessages, finished, aiResponse := c.processAIStream(ctx, w, id, conversationCtx.Model, client, aiReq, messages) + toolCalls, newMessages, finished, aiResponse, reasoningContent := c.processAIStream(ctx, w, id, conversationCtx.Model, client, aiReq, messages) messages = newMessages log.Debugf("Round %d: toolCalls=%v", round+1, toolCalls) - if aiResponse != "" { + if aiResponse != "" || reasoningContent != "" { conversationCtx.Messages = append(conversationCtx.Messages, &ai_conversation.ConversationMessage{ - Role: "assistant", - Content: aiResponse, + Role: "assistant", + Content: aiResponse, + ReasoningContent: reasoningContent, }) } @@ -459,7 +461,7 @@ func (c *AIController) handleAIConversation(ctx *gin.Context, w http.ResponseWri } if len(toolCalls) > 0 { - messages = c.executeToolCalls(ctx, w, id, conversationCtx.Model, toolCalls, messages) + messages = c.executeToolCalls(ctx, w, id, conversationCtx.Model, toolCalls, messages, aiResponse, reasoningContent) } else { return } @@ -471,12 +473,12 @@ func (c *AIController) handleAIConversation(ctx *gin.Context, w http.ResponseWri // processAIStream func (c *AIController) processAIStream( _ *gin.Context, w http.ResponseWriter, id, model string, client *openai.Client, aiReq openai.ChatCompletionRequest, messages []openai.ChatCompletionMessage) ( - []openai.ToolCall, []openai.ChatCompletionMessage, bool, string) { + []openai.ToolCall, []openai.ChatCompletionMessage, bool, string, string) { stream, err := client.CreateChatCompletionStream(context.Background(), aiReq) if err != nil { log.Errorf("Failed to create stream: %v", err) c.sendErrorResponse(w, id, model, "Failed to create AI stream") - return nil, messages, true, "" + return nil, messages, true, "", "" } defer func() { _ = stream.Close() @@ -484,6 +486,7 @@ func (c *AIController) processAIStream( var currentToolCalls []openai.ToolCall var accumulatedContent strings.Builder + var accumulatedReasoning strings.Builder var accumulatedMessage openai.ChatCompletionMessage toolCallsMap := make(map[int]*openai.ToolCall) @@ -528,6 +531,27 @@ func (c *AIController) processAIStream( } } + if choice.Delta.ReasoningContent != "" { + accumulatedReasoning.WriteString(choice.Delta.ReasoningContent) + + reasoningResponse := StreamResponse{ + ChatCompletionID: id, + Object: "chat.completion.chunk", + Created: time.Now().Unix(), + Model: model, + Choices: []StreamChoice{ + { + Index: 0, + Delta: Delta{ + ReasoningContent: choice.Delta.ReasoningContent, + }, + FinishReason: nil, + }, + }, + } + sendStreamData(w, reasoningResponse) + } + if choice.Delta.Content != "" { accumulatedContent.WriteString(choice.Delta.Content) @@ -554,26 +578,30 @@ func (c *AIController) processAIStream( for _, toolCall := range toolCallsMap { currentToolCalls = append(currentToolCalls, *toolCall) } - return currentToolCalls, messages, false, accumulatedContent.String() + return currentToolCalls, messages, false, accumulatedContent.String(), accumulatedReasoning.String() } else { aiResponseContent := accumulatedContent.String() - if aiResponseContent != "" { + aiReasoningContent := accumulatedReasoning.String() + if aiResponseContent != "" || aiReasoningContent != "" { accumulatedMessage = openai.ChatCompletionMessage{ - Role: openai.ChatMessageRoleAssistant, - Content: aiResponseContent, + Role: openai.ChatMessageRoleAssistant, + Content: aiResponseContent, + ReasoningContent: aiReasoningContent, } messages = append(messages, accumulatedMessage) } - return nil, messages, true, aiResponseContent + return nil, messages, true, aiResponseContent, aiReasoningContent } } } aiResponseContent := accumulatedContent.String() - if aiResponseContent != "" { + aiReasoningContent := accumulatedReasoning.String() + if aiResponseContent != "" || aiReasoningContent != "" { accumulatedMessage = openai.ChatCompletionMessage{ - Role: openai.ChatMessageRoleAssistant, - Content: aiResponseContent, + Role: openai.ChatMessageRoleAssistant, + Content: aiResponseContent, + ReasoningContent: aiReasoningContent, } messages = append(messages, accumulatedMessage) } @@ -582,14 +610,14 @@ func (c *AIController) processAIStream( for _, toolCall := range toolCallsMap { currentToolCalls = append(currentToolCalls, *toolCall) } - return currentToolCalls, messages, false, aiResponseContent + return currentToolCalls, messages, false, aiResponseContent, aiReasoningContent } - return currentToolCalls, messages, len(currentToolCalls) == 0, aiResponseContent + return currentToolCalls, messages, len(currentToolCalls) == 0, aiResponseContent, aiReasoningContent } // executeToolCalls -func (c *AIController) executeToolCalls(ctx *gin.Context, _ http.ResponseWriter, _, _ string, toolCalls []openai.ToolCall, messages []openai.ChatCompletionMessage) []openai.ChatCompletionMessage { +func (c *AIController) executeToolCalls(ctx *gin.Context, _ http.ResponseWriter, _, _ string, toolCalls []openai.ToolCall, messages []openai.ChatCompletionMessage, assistantContent, reasoningContent string) []openai.ChatCompletionMessage { validToolCalls := make([]openai.ToolCall, 0) for _, toolCall := range toolCalls { if toolCall.ID == "" || toolCall.Function.Name == "" { @@ -611,8 +639,10 @@ func (c *AIController) executeToolCalls(ctx *gin.Context, _ http.ResponseWriter, } assistantMsg := openai.ChatCompletionMessage{ - Role: openai.ChatMessageRoleAssistant, - ToolCalls: validToolCalls, + Role: openai.ChatMessageRoleAssistant, + Content: assistantContent, + ReasoningContent: reasoningContent, + ToolCalls: validToolCalls, } messages = append(messages, assistantMsg) diff --git a/internal/entity/ai_conversation_record.go b/internal/entity/ai_conversation_record.go index 14dea347..da8f11b1 100644 --- a/internal/entity/ai_conversation_record.go +++ b/internal/entity/ai_conversation_record.go @@ -30,6 +30,7 @@ type AIConversationRecord struct { ChatCompletionID string `xorm:"not null VARCHAR(255) chat_completion_id"` Role string `xorm:"not null default '' VARCHAR(128) role"` Content string `xorm:"not null MEDIUMTEXT content"` + ReasoningContent string `xorm:"MEDIUMTEXT reasoning_content"` Helpful int `xorm:"not null default 0 INT(11) helpful"` Unhelpful int `xorm:"not null default 0 INT(11) unhelpful"` } diff --git a/internal/migrations/migrations.go b/internal/migrations/migrations.go index 682a7b20..7fca2d50 100644 --- a/internal/migrations/migrations.go +++ b/internal/migrations/migrations.go @@ -108,6 +108,7 @@ var migrations = []Migration{ NewMigration("v1.8.0", "change admin menu", updateAdminMenuSettings, true), NewMigration("v1.8.1", "ai feat", aiFeat, true), NewMigration("v2.0.1", "change avatar type to text", updateAvatarType, false), + NewMigration("v2.0.2", "add reasoning content to ai conversation record", addAIConversationReasoningContent, false), } func GetMigrations() []Migration { diff --git a/internal/migrations/v33.go b/internal/migrations/v33.go new file mode 100644 index 00000000..810c2101 --- /dev/null +++ b/internal/migrations/v33.go @@ -0,0 +1,39 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package migrations + +import ( + "context" + "fmt" + + "github.com/apache/answer/internal/entity" + "xorm.io/xorm" +) + +// addAIConversationReasoningContent adds a reasoning_content column to the +// ai_conversation_record table so that the chain-of-thought returned by +// reasoning/thinking-capable models (e.g. DeepSeek) is persisted along with +// the regular content and can be re-displayed when reloading a conversation. +func addAIConversationReasoningContent(ctx context.Context, x *xorm.Engine) error { + if err := x.Context(ctx).Sync(new(entity.AIConversationRecord)); err != nil { + return fmt.Errorf("sync ai_conversation_record table failed: %w", err) + } + return nil +} diff --git a/internal/schema/ai_conversation_schema.go b/internal/schema/ai_conversation_schema.go index fd34278a..60ec5d74 100644 --- a/internal/schema/ai_conversation_schema.go +++ b/internal/schema/ai_conversation_schema.go @@ -48,6 +48,7 @@ type AIConversationRecord struct { ChatCompletionID string `json:"chat_completion_id"` Role string `json:"role"` Content string `json:"content"` + ReasoningContent string `json:"reasoning_content,omitempty"` Helpful int `json:"helpful"` Unhelpful int `json:"unhelpful"` CreatedAt int64 `json:"created_at"` diff --git a/internal/service/ai_conversation/ai_conversation_service.go b/internal/service/ai_conversation/ai_conversation_service.go index d095ac0e..b7ddc6b1 100644 --- a/internal/service/ai_conversation/ai_conversation_service.go +++ b/internal/service/ai_conversation/ai_conversation_service.go @@ -51,6 +51,7 @@ type ConversationMessage struct { ChatCompletionID string `json:"chat_completion_id"` Role string `json:"role"` Content string `json:"content"` + ReasoningContent string `json:"reasoning_content,omitempty"` } // aiConversationService @@ -97,6 +98,7 @@ func (s *aiConversationService) SaveConversationRecords(ctx context.Context, con } content := strings.Builder{} + reasoning := strings.Builder{} for _, record := range records { if len(record.ChatCompletionID) > 0 { @@ -120,12 +122,17 @@ func (s *aiConversationService) SaveConversationRecords(ctx context.Context, con content.WriteString(record.Content) content.WriteString("\n") + if record.ReasoningContent != "" { + reasoning.WriteString(record.ReasoningContent) + reasoning.WriteString("\n") + } } aiRecord := &entity.AIConversationRecord{ ConversationID: conversationID, ChatCompletionID: chatcmplID, Role: "assistant", Content: content.String(), + ReasoningContent: reasoning.String(), Helpful: 0, Unhelpful: 0, } @@ -190,6 +197,7 @@ func (s *aiConversationService) GetConversationDetail(ctx context.Context, req * ChatCompletionID: record.ChatCompletionID, Role: record.Role, Content: record.Content, + ReasoningContent: record.ReasoningContent, Helpful: record.Helpful, Unhelpful: record.Unhelpful, CreatedAt: record.CreatedAt.Unix(), @@ -319,6 +327,7 @@ func (s *aiConversationService) GetConversationDetailForAdmin(ctx context.Contex ChatCompletionID: record.ChatCompletionID, Role: record.Role, Content: record.Content, + ReasoningContent: record.ReasoningContent, Helpful: record.Helpful, Unhelpful: record.Unhelpful, CreatedAt: record.CreatedAt.Unix(), diff --git a/ui/src/common/interface.ts b/ui/src/common/interface.ts index 308726e8..dbcec6c0 100644 --- a/ui/src/common/interface.ts +++ b/ui/src/common/interface.ts @@ -860,6 +860,7 @@ export interface AdminConversationListItem { export interface ConversationDetailItem { chat_completion_id: string; content: string; + reasoning_content?: string; role: string; helpful: number; unhelpful: number; diff --git a/ui/src/components/BubbleAi/index.tsx b/ui/src/components/BubbleAi/index.tsx index 1e79ca7c..4cbd2417 100644 --- a/ui/src/components/BubbleAi/index.tsx +++ b/ui/src/components/BubbleAi/index.tsx @@ -32,6 +32,7 @@ interface IProps { isLast: boolean; isCompleted: boolean; content: string; + reasoningContent?: string; minHeight?: number; actionData: { helpful: number; @@ -55,6 +56,7 @@ const BubbleAi: FC = ({ isLast, isCompleted, content, + reasoningContent = '', chatId = '', actionData, minHeight = 0, @@ -65,6 +67,7 @@ const BubbleAi: FC = ({ const [isHelpful, setIsHelpful] = useState(false); const [isUnhelpful, setIsUnhelpful] = useState(false); const [canShowAction, setCanShowAction] = useState(false); + const [isThinkingOpen, setIsThinkingOpen] = useState(true); const [safeHtml, setSafeHtml] = useState(''); const typewriterRef = useRef<{ timer: NodeJS.Timeout | null; @@ -255,6 +258,14 @@ const BubbleAi: FC = ({ setIsUnhelpful(actionData.unhelpful > 0); }, [actionData]); + // Auto-collapse the "Thinking" panel once the actual answer starts streaming + // (only while the message is being generated; users can still toggle manually). + useEffect(() => { + if (content && !isCompleted) { + setIsThinkingOpen(false); + } + }, [content, isCompleted]); + useEffect(() => { if (fmtContainer.current && isCompleted && safeHtml) { htmlRender(fmtContainer.current, { @@ -275,6 +286,33 @@ const BubbleAi: FC = ({ ref={containerRef} style={{ minHeight: `${minHeight}px`, overflowAnchor: 'none' }}>
+ {reasoningContent ? ( +
+ + {isThinkingOpen && ( +
+ {reasoningContent} +
+ )} +
+ ) : null} +
= ({ visible, id, onClose }) => { isLast={false} isCompleted content={item.content} + reasoningContent={item.reasoning_content || ''} actionData={{ helpful: item.helpful, unhelpful: item.unhelpful, diff --git a/ui/src/pages/AiAssistant/index.tsx b/ui/src/pages/AiAssistant/index.tsx index 83ffe8f1..e355e804 100644 --- a/ui/src/pages/AiAssistant/index.tsx +++ b/ui/src/pages/AiAssistant/index.tsx @@ -154,7 +154,10 @@ const Index = () => { await requestAi('/answer/api/v1/chat/completions', { body: JSON.stringify(params), onMessage: (res) => { - if (!res.choices[0].delta?.content) { + const delta = res.choices[0]?.delta; + const deltaContent = delta?.content || ''; + const deltaReasoning = delta?.reasoning_content || ''; + if (!deltaContent && !deltaReasoning) { return; } setIsLoading(false); @@ -165,13 +168,16 @@ const Index = () => { if (lastConversion?.chat_completion_id === res?.chat_completion_id) { updatedRecords[updatedRecords.length - 1] = { ...lastConversion, - content: lastConversion.content + res.choices[0].delta.content, + content: (lastConversion.content || '') + deltaContent, + reasoning_content: + (lastConversion.reasoning_content || '') + deltaReasoning, }; } else { updatedRecords.push({ chat_completion_id: res.chat_completion_id, - role: res.choices[0].delta.role || 'assistant', - content: res.choices[0].delta.content, + role: delta?.role || 'assistant', + content: deltaContent, + reasoning_content: deltaReasoning, helpful: 0, unhelpful: 0, created_at: Date.now(), @@ -330,6 +336,7 @@ const Index = () => { isLast={isLastMessage} isCompleted={!isGenerate || !isLastMessage} content={item.content} + reasoningContent={item.reasoning_content || ''} actionData={{ helpful: item.helpful, unhelpful: item.unhelpful, diff --git a/ui/src/pages/Search/components/AiCard/index.tsx b/ui/src/pages/Search/components/AiCard/index.tsx index 99e21adc..c6cc699c 100644 --- a/ui/src/pages/Search/components/AiCard/index.tsx +++ b/ui/src/pages/Search/components/AiCard/index.tsx @@ -78,7 +78,10 @@ const Index = () => { await requestAi('/answer/api/v1/chat/completions', { body: JSON.stringify(params), onMessage: (res) => { - if (!res.choices[0].delta?.content) { + const delta = res.choices[0]?.delta; + const deltaContent = delta?.content || ''; + const deltaReasoning = delta?.reasoning_content || ''; + if (!deltaContent && !deltaReasoning) { return; } setIsLoading(false); @@ -90,13 +93,16 @@ const Index = () => { if (lastConversion?.chat_completion_id === res?.chat_completion_id) { updatedRecords[updatedRecords.length - 1] = { ...lastConversion, - content: lastConversion.content + res.choices[0].delta.content, + content: (lastConversion.content || '') + deltaContent, + reasoning_content: + (lastConversion.reasoning_content || '') + deltaReasoning, }; } else { updatedRecords.push({ chat_completion_id: res.chat_completion_id, - role: res.choices[0].delta.role || 'assistant', - content: res.choices[0].delta.content, + role: delta?.role || 'assistant', + content: deltaContent, + reasoning_content: deltaReasoning, helpful: 0, unhelpful: 0, created_at: Date.now(), @@ -154,6 +160,7 @@ const Index = () => { isLast={isLastMessage} isCompleted={!isGenerate || !isLastMessage} content={item.content} + reasoningContent={item.reasoning_content || ''} actionData={{ helpful: item.helpful, unhelpful: item.unhelpful, From 4b8de1897b34bb040ec955538a5377a041afffe0 Mon Sep 17 00:00:00 2001 From: Luffy <52o@qq52o.cn> Date: Tue, 2 Jun 2026 11:50:54 +0800 Subject: [PATCH 02/27] fix: update license entries --- docs/release/LICENSE | 69 ++++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/docs/release/LICENSE b/docs/release/LICENSE index 926d64b7..58aea229 100644 --- a/docs/release/LICENSE +++ b/docs/release/LICENSE @@ -214,11 +214,12 @@ Apache 2.0 licenses The following components are provided under the Apache 2.0 License. - (Apache License, Version 2.0) react-helmet-async (https://github.com/staylor/react-helmet-async) [link](./licenses/LICENSE-staylor-react-helmet-async.txt) - (Apache License, Version 2.0) golang-mock (https://github.com/golang/mock) [link](./licenses/LICENSE-golang-mock.txt) + (Apache License, Version 2.0) gomock (https://github.com/uber-go/mock) [link](./licenses/LICENSE-uber-go-mock.txt) (Apache License, Version 2.0) google-wire (https://github.com/google/wire) [link](./licenses/LICENSE-google-wire.txt) (Apache License, Version 2.0) mojocn-base64Captcha (https://github.com/mojocn/base64Captcha) [link](./licenses/LICENSE-mojocn-base64Captcha.txt) (Apache License, Version 2.0) ory-dockertest (https://github.com/ory/dockertest) [link](./licenses/LICENSE-ory-dockertest.txt) + (Apache License, Version 2.0) react-helmet-async (https://github.com/staylor/react-helmet-async) [link](./licenses/LICENSE-staylor-react-helmet-async.txt) + (Apache License, Version 2.0) sashabaranov-go-openai (https://github.com/sashabaranov/go-openai) [link](./licenses/LICENSE-sashabaranov-go-openai.txt) (Apache License, Version 2.0) spf13-cobra (https://github.com/spf13/cobra) [link](./licenses/LICENSE-spf13-cobra.txt) ======================================================================== @@ -227,57 +228,61 @@ MIT licenses The following components are provided under the MIT License. See project link for details. - (MIT License) axios (https://github.com/axios/axios) [link](./licenses/LICENSE-axios-axios.txt) - (MIT License) bootstrap (https://github.com/twbs/bootstrap) [link](./licenses/LICENSE-twbs-bootstrap.txt) - (MIT License) icons (https://github.com/twbs/icons) [link](./licenses/LICENSE-twbs-icons.txt) - (MIT License) classnames (https://github.com/JedWatson/classnames) [link](./LICENSE-JedWatson-classnames.txt) - (MIT License) codemirror (https://github.com/codemirror/basic-setup) [link](./licenses/LICENSE-codemirror-basic-setup.txt) (MIT License) @codemirror/lang-markdown (https://github.com/codemirror/lang-markdown) [link](./licenses/LICENSE-codemirror-lang-markdown.txt) (MIT License) @codemirror/language-data (https://github.com/codemirror/language-data) [link](./licenses/LICENSE-codemirror-language-data.txt) (MIT License) @codemirror/state (https://github.com/codemirror/state) [link](./licenses/LICENSE-codemirror-state.txt) (MIT License) @codemirror/view (https://github.com/codemirror/view) [link](./licenses/LICENSE-codemirror-view.txt) + (MIT License) anargu-gin-brotli (https://github.com/anargu/gin-brotli) [link](./licenses/LICENSE-anargu-gin-brotli.txt) + (MIT License) asaskevich-govalidator (https://github.com/asaskevich/govalidator) [link](./licenses/LICENSE-asaskevich-govalidator.txt) + (MIT License) axios (https://github.com/axios/axios) [link](./licenses/LICENSE-axios-axios.txt) + (MIT License) bootstrap (https://github.com/twbs/bootstrap) [link](./licenses/LICENSE-twbs-bootstrap.txt) + (MIT License) classnames (https://github.com/JedWatson/classnames) [link](./LICENSE-JedWatson-classnames.txt) + (MIT License) codemirror (https://github.com/codemirror/basic-setup) [link](./licenses/LICENSE-codemirror-basic-setup.txt) (MIT License) color (https://github.com/Qix-/color) [link](./licenses/LICENSE-Qix--color.txt) (MIT License) copy-to-clipboard (https://github.com/sudodoki/copy-to-clipboard) [link](./licenses/LICENSE-sudodoki-copy-to-clipboard.txt) (MIT License) dayjs (https://github.com/iamkun/dayjs) [link](./licenses/LICENSE-iamkun-dayjs.txt) + (MIT License) disintegration-imaging (https://github.com/disintegration/imaging) [link](./licenses/LICENSE-disintegration-imaging.txt) + (MIT License) front-matter (https://github.com/jxson/front-matter) [link](./licenses/LICENSE-jxson-front-matter.txt) + (MIT License) gin-gonic-gin (https://github.com/gin-gonic/gin) [link](./licenses/LICENSE-gin-gonic-gin.txt) + (MIT License) go-gomail-gomail (https://gopkg.in/gomail.v2) [link](./licenses/LICENSE-go-gomail-gomail.txt) + (MIT License) go-playground-locales (https://github.com/go-playground/locales) [link](./licenses/LICENSE-go-playground-locales.txt) + (MIT License) go-playground-universal-translator (https://github.com/go-playground/universal-translator) [link](./licenses/LICENSE-go-playground-universal-translator.txt) + (MIT License) go-playground-validator (https://github.com/go-playground/validator) [link](./licenses/LICENSE-go-playground-validator.txt) + (MIT License) go-resty-resty (https://github.com/go-resty/resty) [link](./licenses/LICENSE-go-resty-resty.txt) + (MIT License) goccy-go-json (https://github.com/goccy/go-json) [link](./licenses/LICENSE-goccy-go-json.txt) (MIT License) i18next (https://github.com/i18next/i18next) [link](./licenses/LICENSE-i18next-i18next.txt) + (MIT License) icons (https://github.com/twbs/icons) [link](./licenses/LICENSE-twbs-icons.txt) + (MIT License) jinzhu-copier (https://github.com/jinzhu/copier) [link](./licenses/LICENSE-jinzhu-copier.txt) + (MIT License) jinzhu-now (https://github.com/jinzhu/now) [link](./licenses/LICENSE-jinzhu-now.txt) + (MIT License) joho-godotenv (https://github.com/joho/godotenv) [link](./licenses/LICENSE-joho-godotenv.txt) + (MIT License) jordan-wright-email (https://github.com/jordan-wright/email) [link](./licenses/LICENSE-jordan-wright-email.txt) + (MIT License) js-sha256 (https://github.com/emn178/js-sha256) [link](./licenses/LICENSE-emn178-js-sha256.txt) + (MIT License) lib-pq (https://github.com/lib/pq) [link](./licenses/LICENSE-lib-pq.txt) (MIT License) lodash (https://github.com/lodash/lodash) [link](./licenses/LICENSE-lodash-lodash.txt) + (MIT License) Machiel-slugify (https://github.com/Machiel/slugify) [link](./licenses/LICENSE-Machiel-slugify.txt) + (MIT License) mark3labs-mcp-go (https://github.com/mark3labs/mcp-go) [link](./licenses/LICENSE-mark3labs-mcp-go.txt) (MIT License) marked (https://github.com/markedjs/marked) [link](./licenses/LICENSE-markedjs-marked.txt) + (MIT License) Masterminds-semver (https://github.com/Masterminds/semver) [link](./licenses/LICENSE-Masterminds-semver.txt) + (MIT License) mattn-go-sqlite3 (https://github.com/mattn/go-sqlite3) [link](./licenses/LICENSE-mattn-go-sqlite3.txt) + (MIT License) mozillazg-go-pinyin (https://github.com/mozillazg/go-pinyin) [link](./licenses/LICENSE-mozillazg-go-pinyin.txt) (MIT License) next-share (https://github.com/Bunlong/next-share) [link](./licenses/LIcENSE-Bunlong-next-share.txt) (MIT License) node-qrcode (https://github.com/soldair/node-qrcode) [link](./licenses/LICENSE-soldair-qrcode.txt) (MIT License) react (https://github.com/facebook/react) [link](./licenses/LICENSE-facebook-react.txt) (MIT License) react-bootstrap (https://github.com/react-bootstrap/react-bootstrap) [link](./licenses/LICENSE-react-bootstrap-react-bootstrap.txt) (MIT License) react-i18next (https://github.com/i18next/react-i18next) [link](./licenses/LICENSE-i18next-react-i18next.txt) (MIT License) react-router (https://github.com/remix-run/react-router) [link](./licenses/LICENSE-remix-run-react-router.txt) - (MIT License) swr (https://github.com/vercel/swr) [link](./licenses/LICENSE-vercel-swr.txt) - (MIT License) zustand (https://github.com/pmndrs/zustand) [link](./licenses/LICENSE-pmndrs-zustand.txt) - (MIT License) mozillazg-go-pinyin (https://github.com/mozillazg/go-pinyin) [link](./licenses/LICENSE-mozillazg-go-pinyin.txt) - (MIT License) Machiel-slugify (https://github.com/Machiel/slugify) [link](./licenses/LICENSE-Machiel-slugify.txt) - (MIT License) Masterminds-semver (https://github.com/Masterminds/semver) [link](./licenses/LICENSE-Masterminds-semver.txt) - (MIT License) anargu-gin-brotli (https://github.com/anargu/gin-brotli) [link](./licenses/LICENSE-anargu-gin-brotli.txt) - (MIT License) asaskevich-govalidator (https://github.com/asaskevich/govalidator) [link](./licenses/LICENSE-asaskevich-govalidator.txt) - (MIT License) disintegration-imaging (https://github.com/disintegration/imaging) [link](./licenses/LICENSE-disintegration-imaging.txt) - (MIT License) gin-gonic-gin (https://github.com/gin-gonic/gin) [link](./licenses/LICENSE-gin-gonic-gin.txt) - (MIT License) go-playground-locales (https://github.com/go-playground/locales) [link](./licenses/LICENSE-go-playground-locales.txt) - (MIT License) go-playground-universal-translator (https://github.com/go-playground/universal-translator) [link](./licenses/LICENSE-go-playground-universal-translator.txt) - (MIT License) go-playground-validator (https://github.com/go-playground/validator) [link](./licenses/LICENSE-go-playground-validator.txt) - (MIT License) goccy-go-json (https://github.com/goccy/go-json) [link](./licenses/LICENSE-goccy-go-json.txt) - (MIT License) jinzhu-copier (https://github.com/jinzhu/copier) [link](./licenses/LICENSE-jinzhu-copier.txt) - (MIT License) jinzhu-now (https://github.com/jinzhu/now) [link](./licenses/LICENSE-jinzhu-now.txt) - (MIT License) jordan-wright-email (https://github.com/jordan-wright/email) [link](./licenses/LICENSE-jordan-wright-email.txt) - (MIT License) lib-pq (https://github.com/lib/pq) [link](./licenses/LICENSE-lib-pq.txt) - (MIT License) mattn-go-sqlite3 (https://github.com/mattn/go-sqlite3) [link](./licenses/LICENSE-mattn-go-sqlite3.txt) - (MIT License) segmentfault-pacman (https://github.com/segmentfault/pacman) [link](./licenses/LICENSE-segmentfault-pacman.txt) (MIT License) robfig-cron (https://github.com/robfig/cron) [link](./licenses/LICENSE-robfig-cron.txt) (MIT License) scottleedavis-go-exif-remove (https://github.com/scottleedavis/go-exif-remove) [link](./licenses/LICENSE-scottleedavis-go-exif-remove.txt) + (MIT License) segmentfault-pacman (https://github.com/segmentfault/pacman) [link](./licenses/LICENSE-segmentfault-pacman.txt) (MIT License) stretchr-testify (https://github.com/stretchr/testify) [link](./licenses/LICENSE-stretchr-testify.txt) (MIT License) swaggo-files (https://github.com/swaggo/files) [link](./licenses/LICENSE-swaggo-files.txt) (MIT License) swaggo-gin-swagger (https://github.com/swaggo/gin-swagger) [link](./licenses/LICENSE-swaggo-gin-swagger.txt) (MIT License) swaggo-swag (https://github.com/swaggo/swag) [link](./licenses/LICENSE-swaggo-swag.txt) + (MIT License) swr (https://github.com/vercel/swr) [link](./licenses/LICENSE-vercel-swr.txt) (MIT License) tidwall-gjson (https://github.com/tidwall/gjson) [link](./licenses/LICENSE-tidwall-gjson.txt) + (MIT License) uuidjs-uuid (https://github.com/uuidjs/uuid) [link](./licenses/LICENSE-uuidjs-uuid.txt) (MIT License) yuin-goldmark (https://github.com/yuin/goldmark) [link](./licenses/LICENSE-yuin-goldmark.txt) - (MIT License) go-gomail-gomail (https://gopkg.in/gomail.v2) [link](./licenses/LICENSE-go-gomail-gomail.txt) - (MIT License) front-matter (https://github.com/jxson/front-matter) [link](./licenses/LICENSE-jxson-front-matter.txt) - (MIT License) js-sha256 (https://github.com/emn178/js-sha256) [link](./licenses/LICENSE-emn178-js-sha256.txt) + (MIT License) zustand (https://github.com/pmndrs/zustand) [link](./licenses/LICENSE-pmndrs-zustand.txt) ======================================================================== BSD licenses @@ -286,13 +291,13 @@ BSD licenses The following components are provided under a BSD license. See project link for details. (BSD 2-Clause) bwmarrin-snowflake (https://github.com/bwmarrin/snowflake) [link](./licenses/LICENSE-bwmarrin-snowflake.txt) - (BSD 2-Clause) xorm (https://xorm.io/xorm) [link](./licenses/LICENSE-xorm.txt) + (BSD 3-Clause) cznic-sqlite (https://modernc.org/sqlite) [link](./licenses/LICENSE-cznic-sqlite.txt) (BSD 3-Clause) google-uuid (https://github.com/google/uuid) [link](./licenses/LICENSE-google-uuid.txt) (BSD 3-Clause) grokify-html-strip-tags-go (https://github.com/grokify/html-strip-tags-go) [link](./licenses/LICENSE-grokify-html-strip-tags-go.txt) - (BSD 3-Clause) microcosm-cc-bluemonday (https://github.com/microcosm-cc/bluemonday) [link](./licenses/LICENSE-microcosm-cc-bluemonday.txt) - (BSD 3-Clause) cznic-sqlite (https://modernc.org/sqlite) [link](./licenses/LICENSE-cznic-sqlite.txt) (BSD 3-Clause) jsdiff (https://github.com/kpdecker/jsdiff) [link](./licenses/LICENSE-kpdecker-jsdiff.txt) + (BSD 3-Clause) microcosm-cc-bluemonday (https://github.com/microcosm-cc/bluemonday) [link](./licenses/LICENSE-microcosm-cc-bluemonday.txt) (BSD 3-Clause) qs (https://github.com/ljharb/qs) [link](./licenses/LICENSE-ljharb-qs.txt) + (BSD 2-Clause) xorm (https://xorm.io/xorm) [link](./licenses/LICENSE-xorm.txt) ======================================================================== ISC licenses From a6d867e4ccc36b7519e65742b8039a9c685cd808 Mon Sep 17 00:00:00 2001 From: Ahmed Qasid Date: Wed, 3 Jun 2026 17:06:26 +0300 Subject: [PATCH 03/27] fix: avoid topic fallback for non-Latin titles via pragmatic ASCII transliteration (#1526) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # fix: avoid `topic` fallback for non-Latin titles via pragmatic ASCII transliteration > **Scope update (in response to review):** this PR is intentionally broader than its original "Arabic-only" framing. The implementation changes URL slug generation for **every non-Latin, non-CJK script** that `slugify` previously stripped — see *Scope* below for the explicit list. The goal is *not* linguistically correct romanization; it is "avoid collapsing to `/topic` by producing a usable ASCII slug." ## What this PR is (and isn't) **Goal:** when a question title contains characters outside Basic Latin / Latin Extended / CJK Han, generate a URL slug that is a deterministic ASCII approximation instead of letting `slugify` strip everything and falling back to the literal `"topic"`. **Non-goal:** this is *not* a linguistically correct multi-language romanizer. The output is a machine-acceptable ASCII slug, not what a native speaker would choose. For example, `こんにちは` → `konnichiha` (not the more natural `kon'nichiwa`), `ไทย` → `aithy` (not `thai`). Treat the slug as an opaque, stable, indexable identifier — the path-after-`/questions//` is for SEO and shareability, the canonical reference is always the ID. ## The bug Pure non-Latin titles previously got stripped by `slugify.Slugify`, hit the empty-result fallback in `htmltext.UrlTitle`, and collapsed to the literal slug `"topic"`. On a live multilingual site, every Arabic / Thai / Japanese-hiragana / Korean / Hebrew / Cyrillic question ended up at `/questions//topic`. ## The fix `UrlTitle()` gets a `convertNonLatin` pre-step that mirrors the existing `convertChinese` pre-step pattern, using `github.com/mozillazg/go-unidecode` (same author as `go-pinyin` already in the repo, to minimise new-dep friction). ``` UrlTitle(title) → convertChinese(title) // pre-existing: Han-block → pinyin → convertNonLatin(title) // NEW: detect non-Latin letters → unidecode to ASCII → clearEmoji / slugify / url.QueryEscape / cutLongTitle (unchanged) ``` The non-Latin detector skips ASCII, Latin-1 Supplement, Latin Extended-A/B, and CJK Han. Inputs that hit none of those non-Latin letter categories short-circuit and return unchanged, so Latin-only and Chinese-only inputs remain byte-identical (pinned by tests). ## Scope — what scripts are affected This PR changes behavior for **any** title containing letters in scripts that `slugify` doesn't handle. Confirmed by tests in `pkg/htmltext/htmltext_test.go`: | Script | Example title | Before | After | | --- | --- | --- | --- | | Arabic | `كيف حالك` | `topic` | `kyf-hlk` | | Mixed Latin + Arabic | `مرحبا hello` | `hello` | `mrhb-hello` | | Thai | `ไทย ไทย` | `topic` | `aithy-aithy` | | Japanese hiragana | `こんにちは` | `topic` | `konnichiha` | | Korean | `안녕하세요` | `topic` | `annyeonghaseyo` | | Hebrew | `שלום עולם` | `topic` | `shlvm-vlm` | | Cyrillic | `Привет мир` | `topic` | `privet-mir` | **Unchanged:** | Case | Behavior | | --- | --- | | Pure Latin (`hello world`) | unchanged → `hello-world` | | Pure Chinese (`这是一个，标题，title`) | unchanged → `zhe-shi-yi-ge-biao-ti` (pinyin path) | | Japanese with Han-block kanji (`日本`) | unchanged → `ri-ben` (caught by pre-existing pinyin path; treated as Chinese reading, not Japanese — a pre-existing limitation, **not** introduced by this PR) | | Emoji only (`😂😂😂`) | unchanged → `topic` | | Empty / whitespace | unchanged → `topic` | ## Transliteration quality — explicit acknowledgement `go-unidecode` is a generic Unicode → ASCII approximation. It is **not** a per-language romanization library. Specifically: - It will pick *one* approximation per codepoint regardless of language context. `ใ` → `ai` (Thai romanization is `i` or `ai` depending on standard), `한` → `han`, `語` → `Yu` (Chinese pinyin reading even when used in Japanese), etc. - The result is *good enough* to be a stable, URL-safe, human-recognizable handle, but speakers of the source language will not consider it "correct." - It is deterministic, so the same title always produces the same slug — important since `url_title` is recomputed on every request. If maintainers prefer to scope this PR more narrowly (e.g. Arabic only, and reject Thai/Hebrew/Cyrillic/etc.), the detector in `containsNonLatin` can be tightened to specific Unicode blocks — but that means the other scripts continue to collapse to `topic`, which is the bug we're trying to fix. I'd argue the broader fix is preferable to a piecemeal one, but happy to narrow if you want. ## Live deployment / real-world verification This patch has been running in production on **[ask.namasoft.com](https://ask.namasoft.com)** (an Apache Answer instance we operate) since deployment, built directly from this branch via `docker compose build`. The site hosts Arabic-language questions, so the fix exercises the affected code path on every page load. Sample question URL on the deployed instance: > `https://ask.namasoft.com/questions/10010000000000115` The slug in the URL is the transliterated Arabic title rather than `topic`. No data migration was needed since `url_title` is computed on every request from `Title` and never persisted (see *Why this is safe to ship* below). ## Admin-configurable The transliteration is gated by a package-level `atomic.Bool` (default **on**, since the current behavior is objectively broken for affected users): - `htmltext.SetTransliterateNonLatin(enabled bool)` - `htmltext.IsTransliterateNonLatinEnabled() bool` This is deliberately the minimum surface needed to satisfy "the setting must be readable from `UrlTitle()`". A follow-up PR can add an admin UI section that calls `SetTransliterateNonLatin` on save and on startup, without having to re-plumb every `htmltext.UrlTitle` call site through `context.Context`. **Default choice — please confirm:** I picked **default-on** because the existing `topic` behavior is a bug for affected users. If you'd prefer default-off for strict backward compat on existing installs, flip the `init()` in `pkg/htmltext/htmltext.go` to `Store(false)` and surface the toggle as opt-in. ## Why this is safe to ship - `url_title` is **not** a persisted column. It's not on the `Question` entity in `internal/entity/question_entity.go`, no migration has ever added/dropped it, and every call site (`question_service.go`, `revision_service.go`, `vote_service.go`, search/report/review/rank/comment services, controllers, repos) recomputes it from `Title` at response-build time via `htmltext.UrlTitle(...)`. - That means the fix is read-only: existing rows light up with correct slugs on the next request, with no migration and no data rewrite. - Rollback is just redeploying the prior image; nothing on disk changes. ## Test coverage `pkg/htmltext/htmltext_test.go`: - **`TestUrlTitleTable`** — table-driven, one case per affected script (the full matrix above), plus: - `empty` → `topic` - `pure latin unchanged` → byte-identical to pre-fix - `pure chinese unchanged` → byte-identical to pre-fix (pins existing pinyin behavior) - `japanese kanji goes through pinyin path unchanged` → documents the pre-existing Han-block limitation - `emoji only falls back to topic` → unchanged - `long arabic truncates at cutLongTitle boundary` → exercises the 150-byte cap and UTF-8 boundary safety - **`TestUrlTitleTransliterationToggle`** — with the toggle off, non-Latin titles collapse to `topic` (pre-fix behavior); with it on, they transliterate. - Existing `TestUrlTitle` left untouched. Test plan for reviewers: - [ ] `go test ./pkg/htmltext/...` — all pass - [ ] Visit the live sample URL above and confirm slug is transliterated, not `topic` - [ ] Verify Chinese / Latin / emoji-only / empty behavior is byte-identical to `main` (covered by table tests) ## Out of scope (intentionally) - No admin UI / site setting plumbing in this PR — see *Admin-configurable* above. Happy to do the React `Non-Latin Languages Handling` admin page + `SiteType` + service / controller / migration in a follow-up if maintainers want it. - No change to the `"topic"` empty-result fallback. - No plugin interface for slug generation — mirrored the existing `convertChinese` pre-step pattern instead. - No per-language romanization library — this is an explicit non-goal; see *Transliteration quality* above. ## Issues / discussion I didn't find an existing upstream issue covering this — happy to be pointed at one if there is. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: LinkinStars --- .../LICENSE-mozillazg-go-unidecode.txt | 21 ++++ go.mod | 1 + go.sum | 2 + pkg/htmltext/htmltext.go | 49 ++++++++ pkg/htmltext/htmltext_test.go | 105 ++++++++++++++++++ 5 files changed, 178 insertions(+) create mode 100644 docs/release/licenses/LICENSE-mozillazg-go-unidecode.txt diff --git a/docs/release/licenses/LICENSE-mozillazg-go-unidecode.txt b/docs/release/licenses/LICENSE-mozillazg-go-unidecode.txt new file mode 100644 index 00000000..8a7780fc --- /dev/null +++ b/docs/release/licenses/LICENSE-mozillazg-go-unidecode.txt @@ -0,0 +1,21 @@ +The MIT License (MIT) + +Copyright (c) 2016 mozillazg + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/go.mod b/go.mod index 89fd7146..11c3a816 100644 --- a/go.mod +++ b/go.mod @@ -43,6 +43,7 @@ require ( github.com/mark3labs/mcp-go v0.43.2 github.com/microcosm-cc/bluemonday v1.0.27 github.com/mozillazg/go-pinyin v0.20.0 + github.com/mozillazg/go-unidecode v0.2.0 github.com/ory/dockertest/v3 v3.11.0 github.com/robfig/cron/v3 v3.0.1 github.com/sashabaranov/go-openai v1.41.2 diff --git a/go.sum b/go.sum index bf30d85b..be61e11f 100644 --- a/go.sum +++ b/go.sum @@ -462,6 +462,8 @@ github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9G github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk= github.com/mozillazg/go-pinyin v0.20.0 h1:BtR3DsxpApHfKReaPO1fCqF4pThRwH9uwvXzm+GnMFQ= github.com/mozillazg/go-pinyin v0.20.0/go.mod h1:iR4EnMMRXkfpFVV5FMi4FNB6wGq9NV6uDWbUuPhP4Yc= +github.com/mozillazg/go-unidecode v0.2.0 h1:vFGEzAH9KSwyWmXCOblazEWDh7fOkpmy/Z4ArmamSUc= +github.com/mozillazg/go-unidecode v0.2.0/go.mod h1:zB48+/Z5toiRolOZy9ksLryJ976VIwmDmpQ2quyt1aA= github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= github.com/nats-io/jwt v0.3.0/go.mod h1:fRYCDE99xlTsqUzISS1Bi75UBJ6ljOJQOAAu5VglpSg= github.com/nats-io/jwt v0.3.2/go.mod h1:/euKqTS1ZD+zzjYrY7pseZrTtWQSjujC7xjPc8wL6eU= diff --git a/pkg/htmltext/htmltext.go b/pkg/htmltext/htmltext.go index e2e017c8..92908083 100644 --- a/pkg/htmltext/htmltext.go +++ b/pkg/htmltext/htmltext.go @@ -25,6 +25,8 @@ import ( "net/url" "regexp" "strings" + "sync/atomic" + "unicode" "unicode/utf8" "github.com/Machiel/slugify" @@ -32,6 +34,7 @@ import ( "github.com/apache/answer/pkg/converter" strip "github.com/grokify/html-strip-tags-go" "github.com/mozillazg/go-pinyin" + "github.com/mozillazg/go-unidecode" ) var ( @@ -47,8 +50,27 @@ var ( "\r", " ", "\t", " ", ) + + // Without this, pure non-Latin titles (Arabic, Cyrillic, Hebrew, ...) get + // stripped by slugify and collapse to the "topic" fallback. Chinese is + // handled separately by convertChinese. + transliterateNonLatin atomic.Bool ) +func init() { + transliterateNonLatin.Store(true) +} + +// SetTransliterateNonLatin toggles non-Latin script transliteration for URL slugs. +func SetTransliterateNonLatin(enabled bool) { + transliterateNonLatin.Store(enabled) +} + +// IsTransliterateNonLatinEnabled reports whether non-Latin transliteration is on. +func IsTransliterateNonLatinEnabled() bool { + return transliterateNonLatin.Load() +} + // ClearText clear HTML, get the clear text func ClearText(html string) string { if html == "" { @@ -66,6 +88,9 @@ func ClearText(html string) string { func UrlTitle(title string) (text string) { title = convertChinese(title) + if transliterateNonLatin.Load() { + title = convertNonLatin(title) + } title = clearEmoji(title) title = slugify.Slugify(title) title = url.QueryEscape(title) @@ -95,6 +120,30 @@ func convertChinese(content string) string { return strings.Join(pinyin.LazyConvert(content, nil), "-") } +// Short-circuits on Latin-only / Chinese-only input so existing slugs stay byte-identical. +func convertNonLatin(content string) string { + if !containsNonLatin(content) { + return content + } + return unidecode.Unidecode(content) +} + +func containsNonLatin(content string) bool { + for _, r := range content { + switch { + case r < 0x0080: // ASCII + continue + case r >= 0x0080 && r <= 0x024F: // Latin-1 Supplement, Latin Extended-A/B + continue + case unicode.Is(unicode.Han, r): // handled by convertChinese + continue + case unicode.IsLetter(r): + return true + } + } + return false +} + func cutLongTitle(title string) string { maxBytes := 150 if len(title) <= maxBytes { diff --git a/pkg/htmltext/htmltext_test.go b/pkg/htmltext/htmltext_test.go index 39de9e96..bcedcb3c 100644 --- a/pkg/htmltext/htmltext_test.go +++ b/pkg/htmltext/htmltext_test.go @@ -87,6 +87,111 @@ func TestUrlTitle(t *testing.T) { } } +func TestUrlTitleTable(t *testing.T) { + // Long pure-Arabic title: 50 copies of the same Arabic word, joined by spaces. + // Unidecode of "كيف" is "kyf", so the slug becomes "kyf-" repeated and + // exceeds cutLongTitle's 150-byte cap. + longArabic := strings.Repeat("كيف ", 50) + wantLongArabic := strings.Repeat("kyf-", 37) + "ky" // 37*4 + 2 = 150 bytes + + cases := []struct { + name string + title string + want string + }{ + { + name: "empty", + title: "", + want: "topic", + }, + { + name: "pure latin unchanged", + title: "hello world", + want: "hello-world", + }, + { + // Pinyin conversion drops Latin runes by design — matches pre-fix behavior. + name: "pure chinese unchanged", + title: "这是一个，标题，title", + want: "zhe-shi-yi-ge-biao-ti", + }, + { + // The fix: previously collapsed to "topic" for all of these scripts. + // Outputs are an ASCII approximation, not linguistically correct + // romanization — see PR description. + name: "arabic transliterated", + title: "كيف حالك", + want: "kyf-hlk", + }, + { + name: "mixed latin and arabic", + title: "مرحبا hello", + want: "mrhb-hello", + }, + { + name: "thai transliterated", + title: "ไทย ไทย", + want: "aithy-aithy", + }, + { + name: "japanese hiragana transliterated", + title: "こんにちは", + want: "konnichiha", + }, + { + // Japanese with Han-block kanji is caught by the pre-existing pinyin + // pre-step (Chinese reading, not Japanese), so this path is unchanged + // by this PR. Pinning to document the existing behavior. + name: "japanese kanji goes through pinyin path unchanged", + title: "日本", + want: "ri-ben", + }, + { + name: "korean transliterated", + title: "안녕하세요", + want: "annyeonghaseyo", + }, + { + name: "hebrew transliterated", + title: "שלום עולם", + want: "shlvm-vlm", + }, + { + name: "cyrillic transliterated", + title: "Привет мир", + want: "privet-mir", + }, + { + name: "emoji only falls back to topic", + title: "😂😂😂", + want: "topic", + }, + { + name: "long arabic truncates at cutLongTitle boundary", + title: longArabic, + want: wantLongArabic, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := UrlTitle(tc.title) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestUrlTitleTransliterationToggle(t *testing.T) { + defer SetTransliterateNonLatin(true) + + SetTransliterateNonLatin(false) + // With transliteration off, pure-Arabic titles collapse to the existing + // "topic" fallback (the pre-fix behavior). + assert.Equal(t, "topic", UrlTitle("كيف حالك")) + + SetTransliterateNonLatin(true) + assert.Equal(t, "kyf-hlk", UrlTitle("كيف حالك")) +} + func TestFindFirstMatchedWord(t *testing.T) { var ( expectedWord, From 10be96814ecf980d8993ef0ac23715f778945e63 Mon Sep 17 00:00:00 2001 From: hhc7 <169754973+hhc7@users.noreply.github.com> Date: Thu, 28 May 2026 18:04:27 +0800 Subject: [PATCH 04/27] feat: add recovery middleware to handle panic gracefully --- internal/base/middleware/recovery.go | 44 ++++++++++++++ internal/base/middleware/recovery_test.go | 71 +++++++++++++++++++++++ internal/base/server/http.go | 1 + 3 files changed, 116 insertions(+) create mode 100644 internal/base/middleware/recovery.go create mode 100644 internal/base/middleware/recovery_test.go diff --git a/internal/base/middleware/recovery.go b/internal/base/middleware/recovery.go new file mode 100644 index 00000000..7e844f78 --- /dev/null +++ b/internal/base/middleware/recovery.go @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package middleware + +import ( + "net/http" + "runtime/debug" + + "github.com/apache/answer/internal/base/handler" + "github.com/apache/answer/internal/base/reason" + "github.com/gin-gonic/gin" + "github.com/segmentfault/pacman/log" +) + +func Recovery() gin.HandlerFunc { + return func(ctx *gin.Context) { + defer func() { + if err := recover(); err != nil { + log.Errorf("panic recovered: %v\n%s", err, debug.Stack()) + ctx.AbortWithStatusJSON(http.StatusInternalServerError, + handler.NewRespBody(http.StatusInternalServerError, reason.UnknownError).TrMsg(handler.GetLangByCtx(ctx)), + ) + } + }() + ctx.Next() + } +} diff --git a/internal/base/middleware/recovery_test.go b/internal/base/middleware/recovery_test.go new file mode 100644 index 00000000..c01719fc --- /dev/null +++ b/internal/base/middleware/recovery_test.go @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package middleware + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" +) + +func TestRecovery_Panic(t *testing.T) { + gin.SetMode(gin.TestMode) + r := gin.New() + r.Use(Recovery()) + r.GET("/panic", func(ctx *gin.Context) { + panic("test panic") + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/panic", nil) + r.ServeHTTP(w, req) + + if w.Code != http.StatusInternalServerError { + t.Errorf("expected 500, got %d", w.Code) + } + + var body map[string]any + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { + t.Fatalf("response is not valid JSON: %v", err) + } + if body["reason"] != "base.unknown" { + t.Errorf("unexpected reason: %v", body["reason"]) + } +} + +func TestRecovery_NoPanic(t *testing.T) { + gin.SetMode(gin.TestMode) + r := gin.New() + r.Use(Recovery()) + r.GET("/ok", func(ctx *gin.Context) { + ctx.String(http.StatusOK, "ok") + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/ok", nil) + r.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d", w.Code) + } +} diff --git a/internal/base/server/http.go b/internal/base/server/http.go index 765cbf6b..1e8204d3 100644 --- a/internal/base/server/http.go +++ b/internal/base/server/http.go @@ -52,6 +52,7 @@ func NewHTTPServer(debug bool, gin.SetMode(gin.ReleaseMode) } r := gin.New() + r.Use(middleware.Recovery()) r.Use(func(ctx *gin.Context) { if strings.Contains(ctx.Request.URL.Path, "/chat/completions") { return From 9234d8024630a41400282f6dab619f716551f815 Mon Sep 17 00:00:00 2001 From: hhc7 <169754973+hhc7@users.noreply.github.com> Date: Tue, 2 Jun 2026 13:55:35 +0800 Subject: [PATCH 05/27] fix: scope JSON 500 to API routes, skip rewriting already-flushed responses --- internal/base/middleware/recovery.go | 26 ++++++++-- internal/base/middleware/recovery_test.go | 63 ++++++++++++++++++++--- internal/base/server/http.go | 5 +- 3 files changed, 82 insertions(+), 12 deletions(-) diff --git a/internal/base/middleware/recovery.go b/internal/base/middleware/recovery.go index 7e844f78..02b1cbde 100644 --- a/internal/base/middleware/recovery.go +++ b/internal/base/middleware/recovery.go @@ -22,6 +22,7 @@ package middleware import ( "net/http" "runtime/debug" + "strings" "github.com/apache/answer/internal/base/handler" "github.com/apache/answer/internal/base/reason" @@ -29,14 +30,31 @@ import ( "github.com/segmentfault/pacman/log" ) -func Recovery() gin.HandlerFunc { +func Recovery(apiPrefixes ...string) gin.HandlerFunc { return func(ctx *gin.Context) { defer func() { if err := recover(); err != nil { log.Errorf("panic recovered: %v\n%s", err, debug.Stack()) - ctx.AbortWithStatusJSON(http.StatusInternalServerError, - handler.NewRespBody(http.StatusInternalServerError, reason.UnknownError).TrMsg(handler.GetLangByCtx(ctx)), - ) + + // Headers/body already flushed (SSE or any streamed response). + // We can no longer rewrite the response cleanly; just stop the chain. + if ctx.Writer.Written() { + ctx.Abort() + return + } + + path := ctx.Request.URL.Path + for _, p := range apiPrefixes { + if strings.HasPrefix(path, p) { + ctx.AbortWithStatusJSON(http.StatusInternalServerError, + handler.NewRespBody(http.StatusInternalServerError, reason.UnknownError). + TrMsg(handler.GetLangByCtx(ctx)), + ) + return + } + } + + ctx.AbortWithStatus(http.StatusInternalServerError) } }() ctx.Next() diff --git a/internal/base/middleware/recovery_test.go b/internal/base/middleware/recovery_test.go index c01719fc..58dbfdb0 100644 --- a/internal/base/middleware/recovery_test.go +++ b/internal/base/middleware/recovery_test.go @@ -28,16 +28,17 @@ import ( "github.com/gin-gonic/gin" ) -func TestRecovery_Panic(t *testing.T) { +// Panic on an API path returns the project's unified JSON 500. +func TestRecovery_APIPathPanic(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() - r.Use(Recovery()) - r.GET("/panic", func(ctx *gin.Context) { + r.Use(Recovery("/api")) + r.GET("/api/panic", func(ctx *gin.Context) { panic("test panic") }) w := httptest.NewRecorder() - req, _ := http.NewRequest(http.MethodGet, "/panic", nil) + req, _ := http.NewRequest(http.MethodGet, "/api/panic", nil) r.ServeHTTP(w, req) if w.Code != http.StatusInternalServerError { @@ -53,16 +54,64 @@ func TestRecovery_Panic(t *testing.T) { } } +// Panic on a non-API path returns a bare 500 with no body, so the browser can +// render its own error page instead of showing raw JSON. +func TestRecovery_NonAPIPathPanic(t *testing.T) { + gin.SetMode(gin.TestMode) + r := gin.New() + r.Use(Recovery("/api")) + r.GET("/page", func(ctx *gin.Context) { + panic("test panic") + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/page", nil) + r.ServeHTTP(w, req) + + if w.Code != http.StatusInternalServerError { + t.Errorf("expected 500, got %d", w.Code) + } + if w.Body.Len() != 0 { + t.Errorf("expected empty body for non-API path, got: %q", w.Body.String()) + } +} + +// Panic after the response has already started writing (SSE / streamed +// responses). The middleware must not touch the response — status and body +// already on the wire stay untouched, no JSON gets appended. +func TestRecovery_PanicAfterResponseStarted(t *testing.T) { + gin.SetMode(gin.TestMode) + r := gin.New() + r.Use(Recovery("/api")) + r.GET("/api/stream", func(ctx *gin.Context) { + ctx.Writer.WriteHeader(http.StatusOK) + _, _ = ctx.Writer.Write([]byte("partial data")) + panic("test panic after write") + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/api/stream", nil) + r.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected status to remain 200 (already flushed), got %d", w.Code) + } + if w.Body.String() != "partial data" { + t.Errorf("expected body to remain 'partial data' (no error JSON appended), got: %q", w.Body.String()) + } +} + +// Normal requests pass through unaffected. func TestRecovery_NoPanic(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() - r.Use(Recovery()) - r.GET("/ok", func(ctx *gin.Context) { + r.Use(Recovery("/api")) + r.GET("/api/ok", func(ctx *gin.Context) { ctx.String(http.StatusOK, "ok") }) w := httptest.NewRecorder() - req, _ := http.NewRequest(http.MethodGet, "/ok", nil) + req, _ := http.NewRequest(http.MethodGet, "/api/ok", nil) r.ServeHTTP(w, req) if w.Code != http.StatusOK { diff --git a/internal/base/server/http.go b/internal/base/server/http.go index 1e8204d3..8db55744 100644 --- a/internal/base/server/http.go +++ b/internal/base/server/http.go @@ -52,7 +52,10 @@ func NewHTTPServer(debug bool, gin.SetMode(gin.ReleaseMode) } r := gin.New() - r.Use(middleware.Recovery()) + r.Use(middleware.Recovery( + uiConf.APIBaseURL+"/answer/api/v1", + uiConf.APIBaseURL+"/answer/admin/api", + )) r.Use(func(ctx *gin.Context) { if strings.Contains(ctx.Request.URL.Path, "/chat/completions") { return From 4aa52a6b521c5088b5706b1500a5d27c02fa04de Mon Sep 17 00:00:00 2001 From: hgaol Date: Tue, 9 Jun 2026 15:26:11 +0000 Subject: [PATCH 06/27] fix: accept answer fails when short links enabled (#1541) The ownership check added to AcceptAnswer compared the answer's QuestionID against the request's QuestionID directly. When short links are enabled, answerRepo.GetByID re-encodes QuestionID to its short form while the controller de-shorts req.QuestionID to its long form, so the two encodings of the same question never matched and every accept returned "Answer do not found". Normalize both ids via uid.DeShortID before comparing, preserving the privilege-escalation guard for answers that truly belong to another question. --- internal/service/content/answer_service.go | 8 +- .../service/content/answer_service_test.go | 81 +++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 internal/service/content/answer_service_test.go diff --git a/internal/service/content/answer_service.go b/internal/service/content/answer_service.go index d7506d6a..25b0050e 100644 --- a/internal/service/content/answer_service.go +++ b/internal/service/content/answer_service.go @@ -471,7 +471,7 @@ func (as *AnswerService) AcceptAnswer(ctx context.Context, req *schema.AcceptAns } // check answer belong to question - if acceptedAnswerInfo.QuestionID != req.QuestionID { + if !sameObjectID(acceptedAnswerInfo.QuestionID, req.QuestionID) { return errors.BadRequest(reason.AnswerNotFound) } acceptedAnswerInfo.ID = uid.DeShortID(acceptedAnswerInfo.ID) @@ -513,6 +513,12 @@ func (as *AnswerService) AcceptAnswer(ctx context.Context, req *schema.AcceptAns return nil } +// sameObjectID reports whether two object ids refer to the same row, +// regardless of whether each is in short-id or long-id form. +func sameObjectID(a, b string) bool { + return uid.DeShortID(a) == uid.DeShortID(b) +} + func (as *AnswerService) updateAnswerRank(ctx context.Context, userID string, questionInfo *entity.Question, newAnswerInfo *entity.Answer, oldAnswerInfo *entity.Answer, ) { diff --git a/internal/service/content/answer_service_test.go b/internal/service/content/answer_service_test.go new file mode 100644 index 00000000..3cfcb105 --- /dev/null +++ b/internal/service/content/answer_service_test.go @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package content + +import ( + "testing" + + "github.com/apache/answer/pkg/uid" +) + +// TestSameObjectID guards the AcceptAnswer ownership check (issue #1541). +// +// When short links are enabled, answerRepo.GetByID re-encodes the answer's +// QuestionID to its short form while the controller de-shorts req.QuestionID +// to its long form. The two encodings of the same question must be treated as +// equal, or accepting any answer fails with "Answer do not found". +func TestSameObjectID(t *testing.T) { + const longQID = "10010000000000001" + shortQID := uid.EnShortID(longQID) // e.g. "D1D1" + if shortQID == "" || shortQID == longQID { + t.Fatalf("precondition failed: EnShortID(%q)=%q, want a distinct short id", longQID, shortQID) + } + otherLongQID := "10010000000000002" + + tests := []struct { + name string + a string + b string + want bool + }{ + { + name: "short answer-side id vs long request-side id, same question (the bug)", + a: shortQID, + b: longQID, + want: true, + }, + { + name: "both long, same question (default permalink)", + a: longQID, + b: longQID, + want: true, + }, + { + name: "both short, same question", + a: shortQID, + b: shortQID, + want: true, + }, + { + name: "different questions must stay rejected (privilege-escalation guard)", + a: shortQID, + b: otherLongQID, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := sameObjectID(tt.a, tt.b); got != tt.want { + t.Errorf("sameObjectID(%q, %q) = %v, want %v", tt.a, tt.b, got, tt.want) + } + }) + } +} From 7123326515377a49b9f673dd8db6daa9c871ab47 Mon Sep 17 00:00:00 2001 From: Artur Iusupov Date: Fri, 5 Jun 2026 00:42:00 +0400 Subject: [PATCH 07/27] feat(site): allow disabling email verification --- docs/docs.go | 6 + docs/swagger.json | 6 + docs/swagger.yaml | 4 + i18n/en_US.yaml | 5 +- internal/controller/user_controller.go | 5 + internal/controller/user_controller_test.go | 37 ++++ internal/migrations/init.go | 7 +- internal/migrations/migrations.go | 1 + internal/migrations/v30.go | 9 +- internal/migrations/v34.go | 85 +++++++++ internal/migrations/v34_test.go | 172 ++++++++++++++++++ internal/schema/siteinfo_schema.go | 48 ++++- internal/schema/user_schema.go | 13 +- internal/service/content/user_service.go | 61 ++++++- internal/service/content/user_service_test.go | 155 ++++++++++++++++ internal/service/siteinfo/siteinfo_service.go | 22 ++- .../service/siteinfo/siteinfo_service_test.go | 104 +++++++++++ .../siteinfo_common/siteinfo_service.go | 2 +- .../siteinfo_common/siteinfo_service_test.go | 44 +++++ ui/src/common/interface.ts | 1 + ui/src/pages/Admin/Login/index.tsx | 15 ++ .../Register/components/SignUpForm/index.tsx | 13 +- ui/src/pages/Users/Register/index.tsx | 20 +- ui/src/services/common.ts | 5 +- ui/src/stores/loginSetting.ts | 1 + 25 files changed, 797 insertions(+), 44 deletions(-) create mode 100644 internal/controller/user_controller_test.go create mode 100644 internal/migrations/v34.go create mode 100644 internal/migrations/v34_test.go create mode 100644 internal/service/content/user_service_test.go create mode 100644 internal/service/siteinfo/siteinfo_service_test.go diff --git a/docs/docs.go b/docs/docs.go index 03b06701..29e3a962 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -12096,6 +12096,9 @@ const docTemplate = `{ }, "allow_password_login": { "type": "boolean" + }, + "require_email_verification": { + "type": "boolean" } } }, @@ -12116,6 +12119,9 @@ const docTemplate = `{ }, "allow_password_login": { "type": "boolean" + }, + "require_email_verification": { + "type": "boolean" } } }, diff --git a/docs/swagger.json b/docs/swagger.json index 3bfb2e3c..2cc8f130 100644 --- a/docs/swagger.json +++ b/docs/swagger.json @@ -12069,6 +12069,9 @@ }, "allow_password_login": { "type": "boolean" + }, + "require_email_verification": { + "type": "boolean" } } }, @@ -12089,6 +12092,9 @@ }, "allow_password_login": { "type": "boolean" + }, + "require_email_verification": { + "type": "boolean" } } }, diff --git a/docs/swagger.yaml b/docs/swagger.yaml index c08f1e8d..2f7e3754 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -2516,6 +2516,8 @@ definitions: type: boolean allow_password_login: type: boolean + require_email_verification: + type: boolean type: object schema.SiteLoginResp: properties: @@ -2529,6 +2531,8 @@ definitions: type: boolean allow_password_login: type: boolean + require_email_verification: + type: boolean type: object schema.SiteMCPReq: properties: diff --git a/i18n/en_US.yaml b/i18n/en_US.yaml index 61f49650..5d1faa3e 100644 --- a/i18n/en_US.yaml +++ b/i18n/en_US.yaml @@ -2241,6 +2241,10 @@ ui: title: Email registration label: Allow email registration text: Turn off to prevent anyone creating new account through email. + email_verification: + title: Email verification + label: Require email verification + text: When enabled, users must verify their email address before using the site. allowed_email_domains: title: Allowed email domains text: Email domains that users must register accounts with. One domain per line. Ignored when empty. @@ -2485,4 +2489,3 @@ ui: copied: Copied external_content_warning: External images/media are not displayed. - diff --git a/internal/controller/user_controller.go b/internal/controller/user_controller.go index 77c806e0..9552182e 100644 --- a/internal/controller/user_controller.go +++ b/internal/controller/user_controller.go @@ -279,6 +279,7 @@ func (uc *UserController) UserRegisterByEmail(ctx *gin.Context) { handler.HandleResponse(ctx, errors.BadRequest(reason.EmailIllegalDomainError), nil) return } + applyEmailVerificationSetting(req, siteInfo) req.IP = ctx.ClientIP() isAdmin := middleware.GetUserIsAdminModerator(ctx) if !isAdmin { @@ -305,6 +306,10 @@ func (uc *UserController) UserRegisterByEmail(ctx *gin.Context) { } } +func applyEmailVerificationSetting(req *schema.UserRegisterReq, siteInfo *schema.SiteLoginResp) { + req.SkipEmailVerification = !siteInfo.RequireEmailVerification +} + // UserVerifyEmail godoc // @Summary UserVerifyEmail // @Description UserVerifyEmail diff --git a/internal/controller/user_controller_test.go b/internal/controller/user_controller_test.go new file mode 100644 index 00000000..e48dc863 --- /dev/null +++ b/internal/controller/user_controller_test.go @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package controller + +import ( + "testing" + + "github.com/apache/answer/internal/schema" + "github.com/stretchr/testify/assert" +) + +func TestApplyEmailVerificationSetting(t *testing.T) { + req := &schema.UserRegisterReq{} + applyEmailVerificationSetting(req, &schema.SiteLoginResp{RequireEmailVerification: false}) + assert.True(t, req.SkipEmailVerification) + + req = &schema.UserRegisterReq{} + applyEmailVerificationSetting(req, &schema.SiteLoginResp{RequireEmailVerification: true}) + assert.False(t, req.SkipEmailVerification) +} diff --git a/internal/migrations/init.go b/internal/migrations/init.go index 9dbe6eb8..d5098dd0 100644 --- a/internal/migrations/init.go +++ b/internal/migrations/init.go @@ -226,9 +226,10 @@ func (m *Mentor) initSiteInfoGeneralData() { func (m *Mentor) initSiteInfoLoginConfig() { loginConfig := map[string]any{ - "allow_new_registrations": true, - "allow_email_registrations": true, - "allow_password_login": true, + "allow_new_registrations": true, + "allow_email_registrations": true, + "allow_password_login": true, + "require_email_verification": true, } loginConfigDataBytes, _ := json.Marshal(loginConfig) _, m.err = m.engine.Context(m.ctx).Insert(&entity.SiteInfo{ diff --git a/internal/migrations/migrations.go b/internal/migrations/migrations.go index 7fca2d50..59fbb7be 100644 --- a/internal/migrations/migrations.go +++ b/internal/migrations/migrations.go @@ -109,6 +109,7 @@ var migrations = []Migration{ NewMigration("v1.8.1", "ai feat", aiFeat, true), NewMigration("v2.0.1", "change avatar type to text", updateAvatarType, false), NewMigration("v2.0.2", "add reasoning content to ai conversation record", addAIConversationReasoningContent, false), + NewMigration("v2.0.3", "add require email verification login setting", addRequireEmailVerification, true), } func GetMigrations() []Migration { diff --git a/internal/migrations/v30.go b/internal/migrations/v30.go index 72765e37..bf785755 100644 --- a/internal/migrations/v30.go +++ b/internal/migrations/v30.go @@ -302,10 +302,11 @@ func splitLegalMenu(ctx context.Context, x *xorm.Engine) error { PrivacyPolicyParsedText: oldSiteLegal.PrivacyPolicyParsedText, } siteLogin := &schema.SiteLoginResp{ - AllowNewRegistrations: oldSiteLogin.AllowNewRegistrations, - AllowEmailRegistrations: oldSiteLogin.AllowEmailRegistrations, - AllowPasswordLogin: oldSiteLogin.AllowPasswordLogin, - AllowEmailDomains: oldSiteLogin.AllowEmailDomains, + AllowNewRegistrations: oldSiteLogin.AllowNewRegistrations, + AllowEmailRegistrations: oldSiteLogin.AllowEmailRegistrations, + AllowPasswordLogin: oldSiteLogin.AllowPasswordLogin, + AllowEmailDomains: oldSiteLogin.AllowEmailDomains, + RequireEmailVerification: true, } siteGeneral := &schema.SiteGeneralReq{ Name: oldSiteGeneral.Name, diff --git a/internal/migrations/v34.go b/internal/migrations/v34.go new file mode 100644 index 00000000..1cc1895b --- /dev/null +++ b/internal/migrations/v34.go @@ -0,0 +1,85 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package migrations + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "strings" + + "github.com/apache/answer/internal/base/constant" + "github.com/apache/answer/internal/entity" + "xorm.io/xorm" +) + +func addRequireEmailVerification(ctx context.Context, x *xorm.Engine) error { + loginSiteInfo := &entity.SiteInfo{} + exist, err := x.Context(ctx).Where("type = ?", constant.SiteTypeLogin).Get(loginSiteInfo) + if err != nil { + return fmt.Errorf("get login config failed: %w", err) + } + if !exist { + return nil + } + + content, err := backfillRequireEmailVerification(loginSiteInfo.Content) + if err != nil { + return fmt.Errorf("backfill login config failed: %w", err) + } + loginSiteInfo.Content = content + _, err = x.Context(ctx).ID(loginSiteInfo.ID).Cols("content").Update(loginSiteInfo) + if err != nil { + return fmt.Errorf("update login config failed: %w", err) + } + return nil +} + +func backfillRequireEmailVerification(content string) (string, error) { + if strings.TrimSpace(content) == "" { + content = "{}" + } + + loginConfig := map[string]json.RawMessage{} + if err := json.Unmarshal([]byte(content), &loginConfig); err != nil { + return "", err + } + if loginConfig == nil { + loginConfig = map[string]json.RawMessage{} + } + + requireEmailVerification, exists := loginConfig["require_email_verification"] + if !exists || bytes.Equal(bytes.TrimSpace(requireEmailVerification), []byte("null")) { + loginConfig["require_email_verification"] = json.RawMessage("true") + } else { + var value bool + if err := json.Unmarshal(requireEmailVerification, &value); err != nil { + return "", err + } + loginConfig["require_email_verification"] = requireEmailVerification + } + + data, err := json.Marshal(loginConfig) + if err != nil { + return "", err + } + return string(data), nil +} diff --git a/internal/migrations/v34_test.go b/internal/migrations/v34_test.go new file mode 100644 index 00000000..e082ed15 --- /dev/null +++ b/internal/migrations/v34_test.go @@ -0,0 +1,172 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package migrations + +import ( + "context" + "encoding/json" + "testing" + + "github.com/apache/answer/internal/base/constant" + "github.com/apache/answer/internal/entity" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "xorm.io/xorm" +) + +func TestBackfillRequireEmailVerification(t *testing.T) { + tests := []struct { + name string + content string + expected bool + }{ + { + name: "adds true for missing key", + content: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true}`, + expected: true, + }, + { + name: "converts null to true", + content: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"require_email_verification":null}`, + expected: true, + }, + { + name: "preserves false", + content: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"require_email_verification":false}`, + expected: false, + }, + { + name: "preserves true", + content: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"require_email_verification":true}`, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + content, err := backfillRequireEmailVerification(tt.content) + require.NoError(t, err) + + var result map[string]bool + require.NoError(t, json.Unmarshal([]byte(content), &result)) + assert.Equal(t, tt.expected, result["require_email_verification"]) + }) + } +} + +func TestAddRequireEmailVerificationAbsentLoginRow(t *testing.T) { + x, err := xorm.NewEngine("sqlite", ":memory:") + require.NoError(t, err) + defer func() { + _ = x.Close() + }() + require.NoError(t, x.Sync(new(entity.SiteInfo))) + + require.NoError(t, addRequireEmailVerification(context.TODO(), x)) +} + +func TestAddRequireEmailVerificationUpdatesLoginRow(t *testing.T) { + x, err := xorm.NewEngine("sqlite", ":memory:") + require.NoError(t, err) + defer func() { + _ = x.Close() + }() + require.NoError(t, x.Sync(new(entity.SiteInfo))) + + _, err = x.Insert(&entity.SiteInfo{ + Type: constant.SiteTypeLogin, + Content: `{"allow_new_registrations":true}`, + Status: 1, + }) + require.NoError(t, err) + + require.NoError(t, addRequireEmailVerification(context.TODO(), x)) + + login := &entity.SiteInfo{} + exist, err := x.Where("type = ?", constant.SiteTypeLogin).Get(login) + require.NoError(t, err) + require.True(t, exist) + + var result struct { + RequireEmailVerification bool `json:"require_email_verification"` + } + require.NoError(t, json.Unmarshal([]byte(login.Content), &result)) + assert.True(t, result.RequireEmailVerification) +} + +func TestSplitLegalMenuKeepsRequireEmailVerificationDefaultTrue(t *testing.T) { + x, err := xorm.NewEngine("sqlite", ":memory:") + require.NoError(t, err) + defer func() { + _ = x.Close() + }() + require.NoError(t, x.Sync(new(entity.SiteInfo))) + + _, err = x.Insert(&entity.SiteInfo{ + Type: constant.SiteTypeLegal, + Content: `{ + "terms_of_service_original_text":"tos", + "terms_of_service_parsed_text":"tos", + "privacy_policy_original_text":"privacy", + "privacy_policy_parsed_text":"privacy", + "external_content_display":"always_display" + }`, + Status: 1, + }) + require.NoError(t, err) + _, err = x.Insert(&entity.SiteInfo{ + Type: constant.SiteTypeLogin, + Content: `{ + "allow_new_registrations":true, + "allow_email_registrations":true, + "allow_password_login":true, + "login_required":false, + "allow_email_domains":[] + }`, + Status: 1, + }) + require.NoError(t, err) + _, err = x.Insert(&entity.SiteInfo{ + Type: constant.SiteTypeGeneral, + Content: `{ + "name":"site", + "short_description":"short", + "description":"description", + "site_url":"https://example.com", + "contact_email":"admin@example.com", + "check_update":true + }`, + Status: 1, + }) + require.NoError(t, err) + + require.NoError(t, splitLegalMenu(context.TODO(), x)) + + login := &entity.SiteInfo{} + exist, err := x.Where("type = ?", constant.SiteTypeLogin).Get(login) + require.NoError(t, err) + require.True(t, exist) + + var result struct { + RequireEmailVerification bool `json:"require_email_verification"` + } + require.NoError(t, json.Unmarshal([]byte(login.Content), &result)) + assert.True(t, result.RequireEmailVerification) +} diff --git a/internal/schema/siteinfo_schema.go b/internal/schema/siteinfo_schema.go index bdf2308d..3cd27873 100644 --- a/internal/schema/siteinfo_schema.go +++ b/internal/schema/siteinfo_schema.go @@ -21,6 +21,7 @@ package schema import ( "context" + "encoding/json" "fmt" "net/mail" "net/url" @@ -216,12 +217,48 @@ type SiteUsersReq struct { AllowUpdateLocation bool `json:"allow_update_location"` } +// OptionalBool preserves whether a JSON boolean field was omitted, set to null, +// or set to a concrete boolean value. +type OptionalBool struct { + Set bool `json:"-"` + Null bool `json:"-"` + Value bool `json:"-"` +} + +func (b *OptionalBool) UnmarshalJSON(data []byte) error { + b.Set = true + if string(data) == "null" { + b.Null = true + b.Value = false + return nil + } + b.Null = false + return json.Unmarshal(data, &b.Value) +} + +func (b OptionalBool) MarshalJSON() ([]byte, error) { + if !b.Set || b.Null { + return []byte("null"), nil + } + return json.Marshal(b.Value) +} + // SiteLoginReq site login request type SiteLoginReq struct { - AllowNewRegistrations bool `json:"allow_new_registrations"` - AllowEmailRegistrations bool `json:"allow_email_registrations"` - AllowPasswordLogin bool `json:"allow_password_login"` - AllowEmailDomains []string `json:"allow_email_domains"` + AllowNewRegistrations bool `json:"allow_new_registrations"` + AllowEmailRegistrations bool `json:"allow_email_registrations"` + AllowPasswordLogin bool `json:"allow_password_login"` + AllowEmailDomains []string `json:"allow_email_domains"` + RequireEmailVerification OptionalBool `json:"require_email_verification" swaggertype:"boolean"` +} + +// SiteLoginResp site login response +type SiteLoginResp struct { + AllowNewRegistrations bool `json:"allow_new_registrations"` + AllowEmailRegistrations bool `json:"allow_email_registrations"` + AllowPasswordLogin bool `json:"allow_password_login"` + AllowEmailDomains []string `json:"allow_email_domains"` + RequireEmailVerification bool `json:"require_email_verification"` } // SiteCustomCssHTMLReq site custom css html @@ -310,9 +347,6 @@ type SiteInterfaceResp SiteInterfaceReq // SiteBrandingResp site branding response type SiteBrandingResp SiteBrandingReq -// SiteLoginResp site login response -type SiteLoginResp SiteLoginReq - // SiteCustomCssHTMLResp site custom css html response type SiteCustomCssHTMLResp SiteCustomCssHTMLReq diff --git a/internal/schema/user_schema.go b/internal/schema/user_schema.go index d209c8f5..7d0c55a5 100644 --- a/internal/schema/user_schema.go +++ b/internal/schema/user_schema.go @@ -219,12 +219,13 @@ type UserEmailLoginReq struct { // UserRegisterReq user register request type UserRegisterReq struct { - Name string `validate:"required,gte=2,lte=30" json:"name"` - Email string `validate:"required,email,gt=0,lte=500" json:"e_mail" ` - Pass string `validate:"required,gte=8,lte=32" json:"pass"` - CaptchaID string `json:"captcha_id"` - CaptchaCode string `json:"captcha_code"` - IP string `json:"-" ` + Name string `validate:"required,gte=2,lte=30" json:"name"` + Email string `validate:"required,email,gt=0,lte=500" json:"e_mail" ` + Pass string `validate:"required,gte=8,lte=32" json:"pass"` + CaptchaID string `json:"captcha_id"` + CaptchaCode string `json:"captcha_code"` + IP string `json:"-" ` + SkipEmailVerification bool `json:"-"` } func (u *UserRegisterReq) Check() (errFields []*validator.FormErrorField, err error) { diff --git a/internal/service/content/user_service.go b/internal/service/content/user_service.go index 42a2efda..1d7866f3 100644 --- a/internal/service/content/user_service.go +++ b/internal/service/content/user_service.go @@ -518,18 +518,20 @@ func (us *UserService) UserRegisterByEmail(ctx context.Context, registerUserInfo log.Errorf("set default user notification config failed, err: %v", err) } - // send email - data := &schema.EmailCodeContent{ - Email: registerUserInfo.Email, - UserID: userInfo.ID, - } - code := token.GenerateToken() - verifyEmailURL := fmt.Sprintf("%s/users/account-activation?code=%s", us.getSiteUrl(ctx), code) - title, body, err := us.emailService.RegisterTemplate(ctx, verifyEmailURL) + err = applyRegistrationVerification(userInfo, registerUserInfo.SkipEmailVerification, registrationVerificationActions{ + sendActivationEmail: func() error { + return us.sendRegistrationActivationEmail(ctx, userInfo) + }, + activateUser: func() error { + return us.userActivity.UserActive(ctx, userInfo.ID) + }, + markEmailAvailable: func() error { + return us.userRepo.UpdateEmailStatus(ctx, userInfo.ID, entity.EmailStatusAvailable) + }, + }) if err != nil { return nil, nil, err } - go us.emailService.SendAndSaveCode(ctx, userInfo.ID, userInfo.EMail, title, body, code, data.ToJSONString()) roleID, err := us.userRoleService.GetUserRole(ctx, userInfo.ID) if err != nil { @@ -560,6 +562,47 @@ func (us *UserService) UserRegisterByEmail(ctx context.Context, registerUserInfo return resp, nil, nil } +type registrationVerificationActions struct { + sendActivationEmail func() error + activateUser func() error + markEmailAvailable func() error +} + +func applyRegistrationVerification( + userInfo *entity.User, skipEmailVerification bool, actions registrationVerificationActions, +) error { + userInfo.MailStatus = entity.EmailStatusToBeVerified + if !skipEmailVerification { + return actions.sendActivationEmail() + } + + if err := actions.activateUser(); err != nil { + log.Errorf("activate user during registration failed, fallback to email verification, err: %v", err) + return actions.sendActivationEmail() + } + if err := actions.markEmailAvailable(); err != nil { + log.Errorf("mark email available during registration failed, fallback to email verification, err: %v", err) + return actions.sendActivationEmail() + } + userInfo.MailStatus = entity.EmailStatusAvailable + return nil +} + +func (us *UserService) sendRegistrationActivationEmail(ctx context.Context, userInfo *entity.User) error { + data := &schema.EmailCodeContent{ + Email: userInfo.EMail, + UserID: userInfo.ID, + } + code := token.GenerateToken() + verifyEmailURL := fmt.Sprintf("%s/users/account-activation?code=%s", us.getSiteUrl(ctx), code) + title, body, err := us.emailService.RegisterTemplate(ctx, verifyEmailURL) + if err != nil { + return err + } + go us.emailService.SendAndSaveCode(ctx, userInfo.ID, userInfo.EMail, title, body, code, data.ToJSONString()) + return nil +} + func (us *UserService) UserVerifyEmailSend(ctx context.Context, userID string) error { userInfo, has, err := us.userRepo.GetByUserID(ctx, userID) if err != nil { diff --git a/internal/service/content/user_service_test.go b/internal/service/content/user_service_test.go new file mode 100644 index 00000000..0d77b383 --- /dev/null +++ b/internal/service/content/user_service_test.go @@ -0,0 +1,155 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package content + +import ( + "errors" + "testing" + + "github.com/apache/answer/internal/entity" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestApplyRegistrationVerification(t *testing.T) { + t.Run("enabled sends activation email and leaves user inactive", func(t *testing.T) { + userInfo := &entity.User{} + calls := map[string]int{} + + err := applyRegistrationVerification(userInfo, false, registrationVerificationActions{ + sendActivationEmail: func() error { + calls["sendActivationEmail"]++ + return nil + }, + activateUser: func() error { + calls["activateUser"]++ + return nil + }, + markEmailAvailable: func() error { + calls["markEmailAvailable"]++ + return nil + }, + }) + + require.NoError(t, err) + assert.Equal(t, entity.EmailStatusToBeVerified, userInfo.MailStatus) + assert.Equal(t, 1, calls["sendActivationEmail"]) + assert.Zero(t, calls["activateUser"]) + assert.Zero(t, calls["markEmailAvailable"]) + }) + + t.Run("disabled activates once and marks email available", func(t *testing.T) { + userInfo := &entity.User{} + calls := map[string]int{} + + err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{ + sendActivationEmail: func() error { + calls["sendActivationEmail"]++ + return nil + }, + activateUser: func() error { + calls["activateUser"]++ + return nil + }, + markEmailAvailable: func() error { + calls["markEmailAvailable"]++ + return nil + }, + }) + + require.NoError(t, err) + assert.Equal(t, entity.EmailStatusAvailable, userInfo.MailStatus) + assert.Zero(t, calls["sendActivationEmail"]) + assert.Equal(t, 1, calls["activateUser"]) + assert.Equal(t, 1, calls["markEmailAvailable"]) + }) + + t.Run("disabled user activation failure falls back to email verification", func(t *testing.T) { + userInfo := &entity.User{} + calls := map[string]int{} + + err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{ + sendActivationEmail: func() error { + calls["sendActivationEmail"]++ + return nil + }, + activateUser: func() error { + calls["activateUser"]++ + return errors.New("activate failed") + }, + markEmailAvailable: func() error { + calls["markEmailAvailable"]++ + return nil + }, + }) + + require.NoError(t, err) + assert.Equal(t, entity.EmailStatusToBeVerified, userInfo.MailStatus) + assert.Equal(t, 1, calls["sendActivationEmail"]) + assert.Equal(t, 1, calls["activateUser"]) + assert.Zero(t, calls["markEmailAvailable"]) + }) + + t.Run("disabled email status failure falls back to email verification", func(t *testing.T) { + userInfo := &entity.User{} + calls := map[string]int{} + + err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{ + sendActivationEmail: func() error { + calls["sendActivationEmail"]++ + return nil + }, + activateUser: func() error { + calls["activateUser"]++ + return nil + }, + markEmailAvailable: func() error { + calls["markEmailAvailable"]++ + return errors.New("update failed") + }, + }) + + require.NoError(t, err) + assert.Equal(t, entity.EmailStatusToBeVerified, userInfo.MailStatus) + assert.Equal(t, 1, calls["sendActivationEmail"]) + assert.Equal(t, 1, calls["activateUser"]) + assert.Equal(t, 1, calls["markEmailAvailable"]) + }) + + t.Run("fallback email failure returns before active status", func(t *testing.T) { + userInfo := &entity.User{} + expectedErr := errors.New("email failed") + + err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{ + sendActivationEmail: func() error { + return expectedErr + }, + activateUser: func() error { + return errors.New("activate failed") + }, + markEmailAvailable: func() error { + return nil + }, + }) + + require.ErrorIs(t, err, expectedErr) + assert.Equal(t, entity.EmailStatusToBeVerified, userInfo.MailStatus) + }) +} diff --git a/internal/service/siteinfo/siteinfo_service.go b/internal/service/siteinfo/siteinfo_service.go index 1e25cbaa..7656a140 100644 --- a/internal/service/siteinfo/siteinfo_service.go +++ b/internal/service/siteinfo/siteinfo_service.go @@ -291,7 +291,27 @@ func (s *SiteInfoService) SaveSiteSecurity(ctx context.Context, req *schema.Site // SaveSiteLogin save site legal configuration func (s *SiteInfoService) SaveSiteLogin(ctx context.Context, req *schema.SiteLoginReq) (err error) { - content, _ := json.Marshal(req) + requireEmailVerification := true + if req.RequireEmailVerification.Set { + if !req.RequireEmailVerification.Null { + requireEmailVerification = req.RequireEmailVerification.Value + } + } else { + currentLogin, err := s.GetSiteLogin(ctx) + if err != nil { + return err + } + requireEmailVerification = currentLogin.RequireEmailVerification + } + + loginConfig := &schema.SiteLoginResp{ + AllowNewRegistrations: req.AllowNewRegistrations, + AllowEmailRegistrations: req.AllowEmailRegistrations, + AllowPasswordLogin: req.AllowPasswordLogin, + AllowEmailDomains: req.AllowEmailDomains, + RequireEmailVerification: requireEmailVerification, + } + content, _ := json.Marshal(loginConfig) data := &entity.SiteInfo{ Type: constant.SiteTypeLogin, Content: string(content), diff --git a/internal/service/siteinfo/siteinfo_service_test.go b/internal/service/siteinfo/siteinfo_service_test.go new file mode 100644 index 00000000..fc4bbdf7 --- /dev/null +++ b/internal/service/siteinfo/siteinfo_service_test.go @@ -0,0 +1,104 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package siteinfo + +import ( + "context" + "encoding/json" + "testing" + + "github.com/apache/answer/internal/base/constant" + "github.com/apache/answer/internal/entity" + "github.com/apache/answer/internal/schema" + "github.com/apache/answer/internal/service/mock" + "github.com/apache/answer/internal/service/siteinfo_common" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" +) + +func TestSiteInfoService_SaveSiteLoginRequireEmailVerification(t *testing.T) { + tests := []struct { + name string + currentContent string + requestPayload string + expectGet bool + expectedRequire bool + }{ + { + name: "omitted preserves normalized default", + currentContent: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true}`, + requestPayload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[]}`, + expectGet: true, + expectedRequire: true, + }, + { + name: "omitted preserves current false", + currentContent: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"require_email_verification":false}`, + requestPayload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[]}`, + expectGet: true, + expectedRequire: false, + }, + { + name: "null normalizes true", + requestPayload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[],"require_email_verification":null}`, + expectedRequire: true, + }, + { + name: "explicit false persists false", + requestPayload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[],"require_email_verification":false}`, + expectedRequire: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctl := gomock.NewController(t) + defer ctl.Finish() + + repo := mock.NewMockSiteInfoRepo(ctl) + if tt.expectGet { + repo.EXPECT().GetByType(gomock.Any(), constant.SiteTypeLogin). + Return(&entity.SiteInfo{Content: tt.currentContent}, true, nil) + } + + var savedContent string + repo.EXPECT().SaveByType(gomock.Any(), constant.SiteTypeLogin, gomock.Any()). + DoAndReturn(func(_ context.Context, _ string, data *entity.SiteInfo) error { + savedContent = data.Content + return nil + }) + + req := &schema.SiteLoginReq{} + require.NoError(t, json.Unmarshal([]byte(tt.requestPayload), req)) + + service := &SiteInfoService{ + siteInfoRepo: repo, + siteInfoCommonService: siteinfo_common.NewSiteInfoCommonService(repo), + } + require.NoError(t, service.SaveSiteLogin(context.TODO(), req)) + assert.NotContains(t, savedContent, `"require_email_verification":null`) + + saved := &schema.SiteLoginResp{} + require.NoError(t, json.Unmarshal([]byte(savedContent), saved)) + assert.Equal(t, tt.expectedRequire, saved.RequireEmailVerification) + }) + } +} diff --git a/internal/service/siteinfo_common/siteinfo_service.go b/internal/service/siteinfo_common/siteinfo_service.go index 5e3964c0..752ae051 100644 --- a/internal/service/siteinfo_common/siteinfo_service.go +++ b/internal/service/siteinfo_common/siteinfo_service.go @@ -235,7 +235,7 @@ func (s *siteInfoCommonService) GetSiteSecurity(ctx context.Context) (resp *sche // GetSiteLogin get site login config func (s *siteInfoCommonService) GetSiteLogin(ctx context.Context) (resp *schema.SiteLoginResp, err error) { - resp = &schema.SiteLoginResp{} + resp = &schema.SiteLoginResp{RequireEmailVerification: true} if err = s.GetSiteInfoByType(ctx, constant.SiteTypeLogin, resp); err != nil { return nil, err } diff --git a/internal/service/siteinfo_common/siteinfo_service_test.go b/internal/service/siteinfo_common/siteinfo_service_test.go index a87d427f..430f45ff 100644 --- a/internal/service/siteinfo_common/siteinfo_service_test.go +++ b/internal/service/siteinfo_common/siteinfo_service_test.go @@ -50,3 +50,47 @@ func TestSiteInfoCommonService_GetSiteGeneral(t *testing.T) { require.NoError(t, err) assert.Equal(t, "name", resp.Name) } + +func TestSiteInfoCommonService_GetSiteLoginRequireEmailVerification(t *testing.T) { + tests := []struct { + name string + content string + expected bool + }{ + { + name: "missing key defaults true", + content: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true}`, + expected: true, + }, + { + name: "null defaults true", + content: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"require_email_verification":null}`, + expected: true, + }, + { + name: "explicit false is preserved", + content: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"require_email_verification":false}`, + expected: false, + }, + { + name: "explicit true is preserved", + content: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"require_email_verification":true}`, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctl := gomock.NewController(t) + defer ctl.Finish() + repo := mock.NewMockSiteInfoRepo(ctl) + repo.EXPECT().GetByType(gomock.Any(), constant.SiteTypeLogin). + Return(&entity.SiteInfo{Content: tt.content}, true, nil) + + siteInfoCommonService := NewSiteInfoCommonService(repo) + resp, err := siteInfoCommonService.GetSiteLogin(context.TODO()) + require.NoError(t, err) + assert.Equal(t, tt.expected, resp.RequireEmailVerification) + }) + } +} diff --git a/ui/src/common/interface.ts b/ui/src/common/interface.ts index dbcec6c0..8ab71423 100644 --- a/ui/src/common/interface.ts +++ b/ui/src/common/interface.ts @@ -488,6 +488,7 @@ export interface AdminSettingsLogin { allow_email_registrations: boolean; allow_email_domains: string[]; allow_password_login: boolean; + require_email_verification: boolean; } /** diff --git a/ui/src/pages/Admin/Login/index.tsx b/ui/src/pages/Admin/Login/index.tsx index a4a61152..c596fc1c 100644 --- a/ui/src/pages/Admin/Login/index.tsx +++ b/ui/src/pages/Admin/Login/index.tsx @@ -47,6 +47,12 @@ const Index: FC = () => { description: t('email_registration.text'), default: true, }, + require_email_verification: { + type: 'boolean', + title: t('email_verification.title'), + description: t('email_verification.text'), + default: true, + }, allow_password_login: { type: 'boolean', title: t('password_login.title'), @@ -73,6 +79,12 @@ const Index: FC = () => { label: t('email_registration.label'), }, }, + require_email_verification: { + 'ui:widget': 'switch', + 'ui:options': { + label: t('email_verification.label'), + }, + }, allow_password_login: { 'ui:widget': 'switch', 'ui:options': { @@ -105,6 +117,7 @@ const Index: FC = () => { allow_email_registrations: formData.allow_email_registrations.value, allow_email_domains: allowedEmailDomains, allow_password_login: formData.allow_password_login.value, + require_email_verification: formData.require_email_verification.value, }; putLoginSetting(reqParams) @@ -139,6 +152,8 @@ const Index: FC = () => { setting.allow_email_domains.join('\n'); } formMeta.allow_password_login.value = setting.allow_password_login; + formMeta.require_email_verification.value = + setting.require_email_verification; setFormData({ ...formMeta }); } }); diff --git a/ui/src/pages/Users/Register/components/SignUpForm/index.tsx b/ui/src/pages/Users/Register/components/SignUpForm/index.tsx index bffa4465..8dc30b96 100644 --- a/ui/src/pages/Users/Register/components/SignUpForm/index.tsx +++ b/ui/src/pages/Users/Register/components/SignUpForm/index.tsx @@ -23,14 +23,17 @@ import { Link } from 'react-router-dom'; import { Trans, useTranslation } from 'react-i18next'; import { useCaptchaPlugin } from '@/utils/pluginKit'; -import type { FormDataType, RegisterReqParams } from '@/common/interface'; +import type { + FormDataType, + RegisterReqParams, + UserInfoRes, +} from '@/common/interface'; import { register } from '@/services'; -import userStore from '@/stores/loggedUserInfo'; import { handleFormError, scrollToElementTop } from '@/utils'; import { useLegalClick } from '@/behaviour/useLegalClick'; interface Props { - callback: () => void; + callback: (user: UserInfoRes) => void; } const Index: React.FC = ({ callback }) => { @@ -53,7 +56,6 @@ const Index: React.FC = ({ callback }) => { }, }); - const updateUser = userStore((state) => state.update); const emailCaptcha = useCaptchaPlugin('email'); const nameRegex = /^[\w.-\s]{2,30}$/; @@ -139,8 +141,7 @@ const Index: React.FC = ({ callback }) => { register(reqParams) .then(async (res) => { await emailCaptcha?.close(); - updateUser(res); - callback(); + callback(res); }) .catch((err) => { if (err.isError) { diff --git a/ui/src/pages/Users/Register/index.tsx b/ui/src/pages/Users/Register/index.tsx index 4d894b6a..0152983e 100644 --- a/ui/src/pages/Users/Register/index.tsx +++ b/ui/src/pages/Users/Register/index.tsx @@ -20,12 +20,14 @@ import React, { useState } from 'react'; import { Container, Col } from 'react-bootstrap'; import { Trans, useTranslation } from 'react-i18next'; -import { Link } from 'react-router-dom'; +import { Link, useNavigate } from 'react-router-dom'; import { usePageTags } from '@/hooks'; +import type * as Type from '@/common/interface'; import { Unactivate, WelcomeTitle, PluginRender } from '@/components'; import { guard } from '@/utils'; -import { loginSettingStore } from '@/stores'; +import { loggedUserInfoStore, loginSettingStore } from '@/stores'; +import { setupAppTheme } from '@/utils/localize'; import { PluginType } from '@/utils/pluginKit/interface'; import SignUpForm from './components/SignUpForm'; @@ -33,9 +35,17 @@ import SignUpForm from './components/SignUpForm'; const Index: React.FC = () => { const [showForm, setShowForm] = useState(true); const { t } = useTranslation('translation', { keyPrefix: 'login' }); + const navigate = useNavigate(); const loginSetting = loginSettingStore((state) => state.login); - const onStep = () => { - setShowForm((bol) => !bol); + const updateUser = loggedUserInfoStore((state) => state.update); + const onRegister = (user: Type.UserInfoRes) => { + updateUser(user); + setupAppTheme(); + if (user.mail_status === 2) { + setShowForm(false); + return; + } + guard.handleLoginRedirect(navigate); }; usePageTags({ title: t('sign_up', { keyPrefix: 'page_title' }), @@ -60,7 +70,7 @@ const Index: React.FC = () => { slug_name="third_party_connector" className="mb-5" /> - {showSignupForm ? : null} + {showSignupForm ? : null}
Already have an account? Log in diff --git a/ui/src/services/common.ts b/ui/src/services/common.ts index cac34b4f..f612bac9 100644 --- a/ui/src/services/common.ts +++ b/ui/src/services/common.ts @@ -129,7 +129,10 @@ export const login = (params: Type.LoginReqParams) => { }; export const register = (params: Type.RegisterReqParams) => { - return request.post('/answer/api/v1/user/register/email', params); + return request.post( + '/answer/api/v1/user/register/email', + params, + ); }; export const logout = () => { diff --git a/ui/src/stores/loginSetting.ts b/ui/src/stores/loginSetting.ts index 7acf765e..f49c394b 100644 --- a/ui/src/stores/loginSetting.ts +++ b/ui/src/stores/loginSetting.ts @@ -32,6 +32,7 @@ const loginSetting = create((set) => ({ allow_email_registrations: true, allow_email_domains: [], allow_password_login: true, + require_email_verification: true, }, update: (params) => set(() => { From 3a5a15fe77319c62dd814fa07998f5bc53df35e1 Mon Sep 17 00:00:00 2001 From: Artur Iusupov Date: Tue, 9 Jun 2026 01:39:27 +0400 Subject: [PATCH 08/27] fix(site): require explicit email verification setting Replace OptionalBool with an explicit require_email_verification value for the login settings save request while keeping legacy read defaults intact. Use positive RequireEmailVerification naming through the registration flow and inline the site setting mapping. Add validation, save-path, and registration coverage for the explicit email verification setting. --- docs/docs.go | 3 + docs/swagger.json | 3 + docs/swagger.yaml | 2 + internal/controller/user_controller.go | 6 +- internal/controller/user_controller_test.go | 37 ---------- internal/migrations/v34.go | 2 + internal/schema/siteinfo_schema.go | 37 ++-------- internal/schema/siteinfo_schema_test.go | 72 +++++++++++++++++++ internal/schema/user_schema.go | 14 ++-- internal/service/content/user_service.go | 6 +- internal/service/content/user_service_test.go | 20 +++--- internal/service/siteinfo/siteinfo_service.go | 15 +--- .../service/siteinfo/siteinfo_service_test.go | 55 +++++++------- 13 files changed, 135 insertions(+), 137 deletions(-) delete mode 100644 internal/controller/user_controller_test.go create mode 100644 internal/schema/siteinfo_schema_test.go diff --git a/docs/docs.go b/docs/docs.go index 29e3a962..4e48c88d 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -12081,6 +12081,9 @@ const docTemplate = `{ }, "schema.SiteLoginReq": { "type": "object", + "required": [ + "require_email_verification" + ], "properties": { "allow_email_domains": { "type": "array", diff --git a/docs/swagger.json b/docs/swagger.json index 2cc8f130..a075dfe4 100644 --- a/docs/swagger.json +++ b/docs/swagger.json @@ -12054,6 +12054,9 @@ }, "schema.SiteLoginReq": { "type": "object", + "required": [ + "require_email_verification" + ], "properties": { "allow_email_domains": { "type": "array", diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 2f7e3754..b3416a10 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -2518,6 +2518,8 @@ definitions: type: boolean require_email_verification: type: boolean + required: + - require_email_verification type: object schema.SiteLoginResp: properties: diff --git a/internal/controller/user_controller.go b/internal/controller/user_controller.go index 9552182e..531d88b4 100644 --- a/internal/controller/user_controller.go +++ b/internal/controller/user_controller.go @@ -279,7 +279,7 @@ func (uc *UserController) UserRegisterByEmail(ctx *gin.Context) { handler.HandleResponse(ctx, errors.BadRequest(reason.EmailIllegalDomainError), nil) return } - applyEmailVerificationSetting(req, siteInfo) + req.RequireEmailVerification = siteInfo.RequireEmailVerification req.IP = ctx.ClientIP() isAdmin := middleware.GetUserIsAdminModerator(ctx) if !isAdmin { @@ -306,10 +306,6 @@ func (uc *UserController) UserRegisterByEmail(ctx *gin.Context) { } } -func applyEmailVerificationSetting(req *schema.UserRegisterReq, siteInfo *schema.SiteLoginResp) { - req.SkipEmailVerification = !siteInfo.RequireEmailVerification -} - // UserVerifyEmail godoc // @Summary UserVerifyEmail // @Description UserVerifyEmail diff --git a/internal/controller/user_controller_test.go b/internal/controller/user_controller_test.go deleted file mode 100644 index e48dc863..00000000 --- a/internal/controller/user_controller_test.go +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package controller - -import ( - "testing" - - "github.com/apache/answer/internal/schema" - "github.com/stretchr/testify/assert" -) - -func TestApplyEmailVerificationSetting(t *testing.T) { - req := &schema.UserRegisterReq{} - applyEmailVerificationSetting(req, &schema.SiteLoginResp{RequireEmailVerification: false}) - assert.True(t, req.SkipEmailVerification) - - req = &schema.UserRegisterReq{} - applyEmailVerificationSetting(req, &schema.SiteLoginResp{RequireEmailVerification: true}) - assert.False(t, req.SkipEmailVerification) -} diff --git a/internal/migrations/v34.go b/internal/migrations/v34.go index 1cc1895b..245bb976 100644 --- a/internal/migrations/v34.go +++ b/internal/migrations/v34.go @@ -67,6 +67,8 @@ func backfillRequireEmailVerification(content string) (string, error) { } requireEmailVerification, exists := loginConfig["require_email_verification"] + // Legacy configs that predate this setting should keep the safer behavior. + // Treat a missing or null value as requiring email verification. if !exists || bytes.Equal(bytes.TrimSpace(requireEmailVerification), []byte("null")) { loginConfig["require_email_verification"] = json.RawMessage("true") } else { diff --git a/internal/schema/siteinfo_schema.go b/internal/schema/siteinfo_schema.go index 3cd27873..1d0b27ff 100644 --- a/internal/schema/siteinfo_schema.go +++ b/internal/schema/siteinfo_schema.go @@ -21,7 +21,6 @@ package schema import ( "context" - "encoding/json" "fmt" "net/mail" "net/url" @@ -217,39 +216,13 @@ type SiteUsersReq struct { AllowUpdateLocation bool `json:"allow_update_location"` } -// OptionalBool preserves whether a JSON boolean field was omitted, set to null, -// or set to a concrete boolean value. -type OptionalBool struct { - Set bool `json:"-"` - Null bool `json:"-"` - Value bool `json:"-"` -} - -func (b *OptionalBool) UnmarshalJSON(data []byte) error { - b.Set = true - if string(data) == "null" { - b.Null = true - b.Value = false - return nil - } - b.Null = false - return json.Unmarshal(data, &b.Value) -} - -func (b OptionalBool) MarshalJSON() ([]byte, error) { - if !b.Set || b.Null { - return []byte("null"), nil - } - return json.Marshal(b.Value) -} - // SiteLoginReq site login request type SiteLoginReq struct { - AllowNewRegistrations bool `json:"allow_new_registrations"` - AllowEmailRegistrations bool `json:"allow_email_registrations"` - AllowPasswordLogin bool `json:"allow_password_login"` - AllowEmailDomains []string `json:"allow_email_domains"` - RequireEmailVerification OptionalBool `json:"require_email_verification" swaggertype:"boolean"` + AllowNewRegistrations bool `json:"allow_new_registrations"` + AllowEmailRegistrations bool `json:"allow_email_registrations"` + AllowPasswordLogin bool `json:"allow_password_login"` + AllowEmailDomains []string `json:"allow_email_domains"` + RequireEmailVerification *bool `validate:"required" json:"require_email_verification" swaggertype:"boolean"` } // SiteLoginResp site login response diff --git a/internal/schema/siteinfo_schema_test.go b/internal/schema/siteinfo_schema_test.go new file mode 100644 index 00000000..e5413f4a --- /dev/null +++ b/internal/schema/siteinfo_schema_test.go @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package schema + +import ( + "encoding/json" + "testing" + + "github.com/apache/answer/internal/base/validator" + "github.com/segmentfault/pacman/i18n" + "github.com/stretchr/testify/require" +) + +func TestSiteLoginReqRequireEmailVerificationValidation(t *testing.T) { + tests := []struct { + name string + payload string + expectError bool + }{ + { + name: "omitted is invalid", + payload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[]}`, + expectError: true, + }, + { + name: "null is invalid", + payload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[],"require_email_verification":null}`, + expectError: true, + }, + { + name: "false is valid", + payload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[],"require_email_verification":false}`, + expectError: false, + }, + { + name: "true is valid", + payload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[],"require_email_verification":true}`, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := &SiteLoginReq{} + require.NoError(t, json.Unmarshal([]byte(tt.payload), req)) + + _, err := validator.GetValidatorByLang(i18n.DefaultLanguage).Check(req) + if tt.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/internal/schema/user_schema.go b/internal/schema/user_schema.go index 7d0c55a5..0683a5af 100644 --- a/internal/schema/user_schema.go +++ b/internal/schema/user_schema.go @@ -219,13 +219,13 @@ type UserEmailLoginReq struct { // UserRegisterReq user register request type UserRegisterReq struct { - Name string `validate:"required,gte=2,lte=30" json:"name"` - Email string `validate:"required,email,gt=0,lte=500" json:"e_mail" ` - Pass string `validate:"required,gte=8,lte=32" json:"pass"` - CaptchaID string `json:"captcha_id"` - CaptchaCode string `json:"captcha_code"` - IP string `json:"-" ` - SkipEmailVerification bool `json:"-"` + Name string `validate:"required,gte=2,lte=30" json:"name"` + Email string `validate:"required,email,gt=0,lte=500" json:"e_mail" ` + Pass string `validate:"required,gte=8,lte=32" json:"pass"` + CaptchaID string `json:"captcha_id"` + CaptchaCode string `json:"captcha_code"` + IP string `json:"-" ` + RequireEmailVerification bool `json:"-"` } func (u *UserRegisterReq) Check() (errFields []*validator.FormErrorField, err error) { diff --git a/internal/service/content/user_service.go b/internal/service/content/user_service.go index 1d7866f3..d0ebe875 100644 --- a/internal/service/content/user_service.go +++ b/internal/service/content/user_service.go @@ -518,7 +518,7 @@ func (us *UserService) UserRegisterByEmail(ctx context.Context, registerUserInfo log.Errorf("set default user notification config failed, err: %v", err) } - err = applyRegistrationVerification(userInfo, registerUserInfo.SkipEmailVerification, registrationVerificationActions{ + err = applyRegistrationVerification(userInfo, registerUserInfo.RequireEmailVerification, registrationVerificationActions{ sendActivationEmail: func() error { return us.sendRegistrationActivationEmail(ctx, userInfo) }, @@ -569,10 +569,10 @@ type registrationVerificationActions struct { } func applyRegistrationVerification( - userInfo *entity.User, skipEmailVerification bool, actions registrationVerificationActions, + userInfo *entity.User, requireEmailVerification bool, actions registrationVerificationActions, ) error { userInfo.MailStatus = entity.EmailStatusToBeVerified - if !skipEmailVerification { + if requireEmailVerification { return actions.sendActivationEmail() } diff --git a/internal/service/content/user_service_test.go b/internal/service/content/user_service_test.go index 0d77b383..d77a1c44 100644 --- a/internal/service/content/user_service_test.go +++ b/internal/service/content/user_service_test.go @@ -29,11 +29,11 @@ import ( ) func TestApplyRegistrationVerification(t *testing.T) { - t.Run("enabled sends activation email and leaves user inactive", func(t *testing.T) { + t.Run("required sends activation email and leaves email pending", func(t *testing.T) { userInfo := &entity.User{} calls := map[string]int{} - err := applyRegistrationVerification(userInfo, false, registrationVerificationActions{ + err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{ sendActivationEmail: func() error { calls["sendActivationEmail"]++ return nil @@ -55,11 +55,11 @@ func TestApplyRegistrationVerification(t *testing.T) { assert.Zero(t, calls["markEmailAvailable"]) }) - t.Run("disabled activates once and marks email available", func(t *testing.T) { + t.Run("not required activates once and marks email available", func(t *testing.T) { userInfo := &entity.User{} calls := map[string]int{} - err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{ + err := applyRegistrationVerification(userInfo, false, registrationVerificationActions{ sendActivationEmail: func() error { calls["sendActivationEmail"]++ return nil @@ -81,11 +81,11 @@ func TestApplyRegistrationVerification(t *testing.T) { assert.Equal(t, 1, calls["markEmailAvailable"]) }) - t.Run("disabled user activation failure falls back to email verification", func(t *testing.T) { + t.Run("not required user activation failure falls back to email verification", func(t *testing.T) { userInfo := &entity.User{} calls := map[string]int{} - err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{ + err := applyRegistrationVerification(userInfo, false, registrationVerificationActions{ sendActivationEmail: func() error { calls["sendActivationEmail"]++ return nil @@ -107,11 +107,11 @@ func TestApplyRegistrationVerification(t *testing.T) { assert.Zero(t, calls["markEmailAvailable"]) }) - t.Run("disabled email status failure falls back to email verification", func(t *testing.T) { + t.Run("not required email status failure falls back to email verification", func(t *testing.T) { userInfo := &entity.User{} calls := map[string]int{} - err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{ + err := applyRegistrationVerification(userInfo, false, registrationVerificationActions{ sendActivationEmail: func() error { calls["sendActivationEmail"]++ return nil @@ -133,11 +133,11 @@ func TestApplyRegistrationVerification(t *testing.T) { assert.Equal(t, 1, calls["markEmailAvailable"]) }) - t.Run("fallback email failure returns before active status", func(t *testing.T) { + t.Run("fallback email failure returns before available email status", func(t *testing.T) { userInfo := &entity.User{} expectedErr := errors.New("email failed") - err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{ + err := applyRegistrationVerification(userInfo, false, registrationVerificationActions{ sendActivationEmail: func() error { return expectedErr }, diff --git a/internal/service/siteinfo/siteinfo_service.go b/internal/service/siteinfo/siteinfo_service.go index 7656a140..8b32b722 100644 --- a/internal/service/siteinfo/siteinfo_service.go +++ b/internal/service/siteinfo/siteinfo_service.go @@ -291,17 +291,8 @@ func (s *SiteInfoService) SaveSiteSecurity(ctx context.Context, req *schema.Site // SaveSiteLogin save site legal configuration func (s *SiteInfoService) SaveSiteLogin(ctx context.Context, req *schema.SiteLoginReq) (err error) { - requireEmailVerification := true - if req.RequireEmailVerification.Set { - if !req.RequireEmailVerification.Null { - requireEmailVerification = req.RequireEmailVerification.Value - } - } else { - currentLogin, err := s.GetSiteLogin(ctx) - if err != nil { - return err - } - requireEmailVerification = currentLogin.RequireEmailVerification + if req.RequireEmailVerification == nil { + return errors.BadRequest(reason.RequestFormatError) } loginConfig := &schema.SiteLoginResp{ @@ -309,7 +300,7 @@ func (s *SiteInfoService) SaveSiteLogin(ctx context.Context, req *schema.SiteLog AllowEmailRegistrations: req.AllowEmailRegistrations, AllowPasswordLogin: req.AllowPasswordLogin, AllowEmailDomains: req.AllowEmailDomains, - RequireEmailVerification: requireEmailVerification, + RequireEmailVerification: *req.RequireEmailVerification, } content, _ := json.Marshal(loginConfig) data := &entity.SiteInfo{ diff --git a/internal/service/siteinfo/siteinfo_service_test.go b/internal/service/siteinfo/siteinfo_service_test.go index fc4bbdf7..a3f834a5 100644 --- a/internal/service/siteinfo/siteinfo_service_test.go +++ b/internal/service/siteinfo/siteinfo_service_test.go @@ -28,7 +28,6 @@ import ( "github.com/apache/answer/internal/entity" "github.com/apache/answer/internal/schema" "github.com/apache/answer/internal/service/mock" - "github.com/apache/answer/internal/service/siteinfo_common" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" @@ -37,33 +36,17 @@ import ( func TestSiteInfoService_SaveSiteLoginRequireEmailVerification(t *testing.T) { tests := []struct { name string - currentContent string - requestPayload string - expectGet bool + requireEmail bool expectedRequire bool }{ { - name: "omitted preserves normalized default", - currentContent: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true}`, - requestPayload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[]}`, - expectGet: true, - expectedRequire: true, - }, - { - name: "omitted preserves current false", - currentContent: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"require_email_verification":false}`, - requestPayload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[]}`, - expectGet: true, - expectedRequire: false, - }, - { - name: "null normalizes true", - requestPayload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[],"require_email_verification":null}`, + name: "explicit true persists true", + requireEmail: true, expectedRequire: true, }, { name: "explicit false persists false", - requestPayload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[],"require_email_verification":false}`, + requireEmail: false, expectedRequire: false, }, } @@ -74,11 +57,6 @@ func TestSiteInfoService_SaveSiteLoginRequireEmailVerification(t *testing.T) { defer ctl.Finish() repo := mock.NewMockSiteInfoRepo(ctl) - if tt.expectGet { - repo.EXPECT().GetByType(gomock.Any(), constant.SiteTypeLogin). - Return(&entity.SiteInfo{Content: tt.currentContent}, true, nil) - } - var savedContent string repo.EXPECT().SaveByType(gomock.Any(), constant.SiteTypeLogin, gomock.Any()). DoAndReturn(func(_ context.Context, _ string, data *entity.SiteInfo) error { @@ -86,12 +64,15 @@ func TestSiteInfoService_SaveSiteLoginRequireEmailVerification(t *testing.T) { return nil }) - req := &schema.SiteLoginReq{} - require.NoError(t, json.Unmarshal([]byte(tt.requestPayload), req)) - service := &SiteInfoService{ - siteInfoRepo: repo, - siteInfoCommonService: siteinfo_common.NewSiteInfoCommonService(repo), + siteInfoRepo: repo, + } + req := &schema.SiteLoginReq{ + AllowNewRegistrations: true, + AllowEmailRegistrations: true, + AllowPasswordLogin: true, + AllowEmailDomains: []string{}, + RequireEmailVerification: &tt.requireEmail, } require.NoError(t, service.SaveSiteLogin(context.TODO(), req)) assert.NotContains(t, savedContent, `"require_email_verification":null`) @@ -102,3 +83,15 @@ func TestSiteInfoService_SaveSiteLoginRequireEmailVerification(t *testing.T) { }) } } + +func TestSiteInfoService_SaveSiteLoginRequiresEmailVerificationValue(t *testing.T) { + service := &SiteInfoService{} + req := &schema.SiteLoginReq{ + AllowNewRegistrations: true, + AllowEmailRegistrations: true, + AllowPasswordLogin: true, + AllowEmailDomains: []string{}, + } + + require.Error(t, service.SaveSiteLogin(context.TODO(), req)) +} From 06a0f742f22cb309908ab19cd73657368d9a0726 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Thu, 2 Jul 2026 17:41:16 +0800 Subject: [PATCH 09/27] fix external login account binding --- internal/base/constant/cache_key.go | 3 + internal/controller/connector_controller.go | 66 +++++++++- .../user_external_login_repo.go | 26 ++++ internal/schema/user_external_login_schema.go | 12 ++ .../user_external_login_service.go | 113 +++++++++++++++--- 5 files changed, 198 insertions(+), 22 deletions(-) diff --git a/internal/base/constant/cache_key.go b/internal/base/constant/cache_key.go index 987798d1..ab278cc0 100644 --- a/internal/base/constant/cache_key.go +++ b/internal/base/constant/cache_key.go @@ -42,6 +42,9 @@ const ( ConfigCacheTime = 1 * time.Hour ConnectorUserExternalInfoCacheKey = "answer:connector:" ConnectorUserExternalInfoCacheTime = 10 * time.Minute + ConnectorOAuthStateCacheKey = "answer:connector:oauth-state:" + ConnectorOAuthStateCacheTime = 10 * time.Minute + ConnectorOAuthBindStateCacheTime = 5 * time.Minute SiteMapQuestionCacheKeyPrefix = "answer:sitemap:question:%d" SiteMapQuestionCacheTime = time.Hour SitemapMaxSize = 50000 diff --git a/internal/controller/connector_controller.go b/internal/controller/connector_controller.go index 939a2a09..28aa5fc6 100644 --- a/internal/controller/connector_controller.go +++ b/internal/controller/connector_controller.go @@ -22,6 +22,7 @@ package controller import ( "fmt" "net/http" + "net/url" "github.com/apache/answer/internal/base/handler" "github.com/apache/answer/internal/base/middleware" @@ -106,6 +107,27 @@ func (cc *ConnectorController) ConnectorLogin(connector plugin.Connector) (fn fu return } + state := ctx.Query("state") + if len(state) > 0 { + stateInfo, err := cc.userExternalService.GetOAuthState(ctx, state) + if err != nil || stateInfo == nil || stateInfo.Provider != connector.ConnectorSlugName() { + log.Errorf("invalid connector oauth state for provider %s", connector.ConnectorSlugName()) + ctx.Redirect(http.StatusFound, "/50x") + return + } + } else { + state, err = cc.userExternalService.GenerateOAuthState(ctx, connector.ConnectorSlugName(), + schema.ExternalLoginOAuthStateLoginIntent, "") + if err != nil { + log.Errorf("generate connector oauth state failed: %v", err) + ctx.Redirect(http.StatusFound, "/50x") + return + } + q := ctx.Request.URL.Query() + q.Set("state", state) + ctx.Request.URL.RawQuery = q.Encode() + } + receiverURL := fmt.Sprintf("%s%s%s%s", general.SiteUrl, commonRouterPrefix, ConnectorRedirectRouterPrefix, connector.ConnectorSlugName()) redirectURL := connector.ConnectorSender(ctx, receiverURL) @@ -141,6 +163,27 @@ func (cc *ConnectorController) ConnectorRedirect(connector plugin.Connector) (fn Avatar: userInfo.Avatar, MetaInfo: userInfo.MetaInfo, } + stateInfo, err := cc.userExternalService.ConsumeOAuthState(ctx, ctx.Query("state")) + if err != nil { + log.Errorf("get connector oauth state failed: %v", err) + ctx.Redirect(http.StatusFound, "/50x") + return + } + if stateInfo != nil && stateInfo.Provider != connector.ConnectorSlugName() { + log.Errorf("connector oauth state provider mismatch: %s != %s", + stateInfo.Provider, connector.ConnectorSlugName()) + ctx.Redirect(http.StatusFound, "/50x") + return + } + if stateInfo != nil && stateInfo.Intent == schema.ExternalLoginOAuthStateBindIntent { + if err = cc.userExternalService.BindExternalLoginToUser(ctx, stateInfo.UserID, u); err != nil { + log.Errorf("bind external login failed: %v", err) + ctx.Redirect(http.StatusFound, "/50x") + return + } + ctx.Redirect(http.StatusFound, fmt.Sprintf("%s/users/settings/account", siteGeneral.SiteUrl)) + return + } resp, err := cc.userExternalService.ExternalLogin(ctx, u) if err != nil { log.Errorf("external login failed: %v", err) @@ -237,19 +280,32 @@ func (cc *ConnectorController) ConnectorsUserInfo(ctx *gin.Context) { } resp := make([]*schema.ConnectorUserInfoResp, 0) - _ = plugin.CallConnector(func(fn plugin.Connector) error { + err = plugin.CallConnector(func(fn plugin.Connector) error { externalID := userExternalLoginMapping[fn.ConnectorSlugName()] connectorName := fn.ConnectorName() + link := fmt.Sprintf("%s%s%s%s", general.SiteUrl, + commonRouterPrefix, ConnectorLoginRouterPrefix, fn.ConnectorSlugName()) + if len(externalID) == 0 { + state, err := cc.userExternalService.GenerateOAuthState(ctx, fn.ConnectorSlugName(), + schema.ExternalLoginOAuthStateBindIntent, userID) + if err != nil { + return err + } + link = fmt.Sprintf("%s?state=%s", link, url.QueryEscape(state)) + } resp = append(resp, &schema.ConnectorUserInfoResp{ - Name: connectorName.Translate(ctx), - Icon: fn.ConnectorLogoSVG(), - Link: fmt.Sprintf("%s%s%s%s", general.SiteUrl, - commonRouterPrefix, ConnectorLoginRouterPrefix, fn.ConnectorSlugName()), + Name: connectorName.Translate(ctx), + Icon: fn.ConnectorLogoSVG(), + Link: link, Binding: len(externalID) > 0, ExternalID: externalID, }) return nil }) + if err != nil { + handler.HandleResponse(ctx, err, nil) + return + } handler.HandleResponse(ctx, nil, resp) } diff --git a/internal/repo/user_external_login/user_external_login_repo.go b/internal/repo/user_external_login/user_external_login_repo.go index b5cf85e8..12d2a339 100644 --- a/internal/repo/user_external_login/user_external_login_repo.go +++ b/internal/repo/user_external_login/user_external_login_repo.go @@ -22,6 +22,7 @@ package user_external_login import ( "context" "encoding/json" + "time" "github.com/apache/answer/internal/base/constant" "github.com/apache/answer/internal/base/data" @@ -136,3 +137,28 @@ func (ur *userExternalLoginRepo) GetCacheUserExternalLoginInfo( _ = json.Unmarshal([]byte(res), &info) return info, nil } + +func (ur *userExternalLoginRepo) SetCacheOAuthState( + ctx context.Context, state string, info *schema.ExternalLoginOAuthState, duration time.Duration) (err error) { + cacheData, _ := json.Marshal(info) + return ur.data.Cache.SetString(ctx, constant.ConnectorOAuthStateCacheKey+state, + string(cacheData), duration) +} + +func (ur *userExternalLoginRepo) GetCacheOAuthState( + ctx context.Context, state string) (info *schema.ExternalLoginOAuthState, err error) { + res, exist, err := ur.data.Cache.GetString(ctx, constant.ConnectorOAuthStateCacheKey+state) + if err != nil { + return info, err + } + if !exist { + return nil, nil + } + info = &schema.ExternalLoginOAuthState{} + _ = json.Unmarshal([]byte(res), &info) + return info, nil +} + +func (ur *userExternalLoginRepo) DeleteCacheOAuthState(ctx context.Context, state string) (err error) { + return ur.data.Cache.Del(ctx, constant.ConnectorOAuthStateCacheKey+state) +} diff --git a/internal/schema/user_external_login_schema.go b/internal/schema/user_external_login_schema.go index 21389e9c..5ec7de12 100644 --- a/internal/schema/user_external_login_schema.go +++ b/internal/schema/user_external_login_schema.go @@ -19,6 +19,11 @@ package schema +const ( + ExternalLoginOAuthStateLoginIntent = "login" + ExternalLoginOAuthStateBindIntent = "bind" +) + // UserExternalLoginResp user external login resp type UserExternalLoginResp struct { BindingKey string `json:"binding_key"` @@ -75,6 +80,13 @@ type ExternalLoginUserInfoCache struct { Bio string } +// ExternalLoginOAuthState stores the local OAuth request state. +type ExternalLoginOAuthState struct { + Provider string `json:"provider"` + Intent string `json:"intent"` + UserID string `json:"user_id,omitempty"` +} + // ExternalLoginUnbindingReq external login unbinding user type ExternalLoginUnbindingReq struct { ExternalID string `validate:"required,gt=0,lte=128" json:"external_id"` diff --git a/internal/service/user_external_login/user_external_login_service.go b/internal/service/user_external_login/user_external_login_service.go index 3f82179b..2d209dbb 100644 --- a/internal/service/user_external_login/user_external_login_service.go +++ b/internal/service/user_external_login/user_external_login_service.go @@ -54,6 +54,10 @@ type UserExternalLoginRepo interface { DeleteUserExternalLoginByUserID(ctx context.Context, userID string) (err error) SetCacheUserExternalLoginInfo(ctx context.Context, key string, info *schema.ExternalLoginUserInfoCache) (err error) GetCacheUserExternalLoginInfo(ctx context.Context, key string) (info *schema.ExternalLoginUserInfoCache, err error) + SetCacheOAuthState(ctx context.Context, state string, info *schema.ExternalLoginOAuthState, + duration time.Duration) (err error) + GetCacheOAuthState(ctx context.Context, state string) (info *schema.ExternalLoginOAuthState, err error) + DeleteCacheOAuthState(ctx context.Context, state string) (err error) } // UserExternalLoginService user external login service @@ -88,6 +92,41 @@ func NewUserExternalLoginService( } } +func (us *UserExternalLoginService) GenerateOAuthState( + ctx context.Context, provider, intent, userID string) (state string, err error) { + state = token.GenerateToken() + duration := constant.ConnectorOAuthStateCacheTime + if intent == schema.ExternalLoginOAuthStateBindIntent { + duration = constant.ConnectorOAuthBindStateCacheTime + } + err = us.userExternalLoginRepo.SetCacheOAuthState(ctx, state, &schema.ExternalLoginOAuthState{ + Provider: provider, + Intent: intent, + UserID: userID, + }, duration) + return state, err +} + +func (us *UserExternalLoginService) GetOAuthState( + ctx context.Context, state string) (info *schema.ExternalLoginOAuthState, err error) { + if len(state) == 0 { + return nil, nil + } + return us.userExternalLoginRepo.GetCacheOAuthState(ctx, state) +} + +func (us *UserExternalLoginService) ConsumeOAuthState( + ctx context.Context, state string) (info *schema.ExternalLoginOAuthState, err error) { + info, err = us.GetOAuthState(ctx, state) + if err != nil || info == nil { + return info, err + } + if err = us.userExternalLoginRepo.DeleteCacheOAuthState(ctx, state); err != nil { + log.Errorf("delete oauth state failed: %v", err) + } + return info, nil +} + // ExternalLogin if user is already a member logged in func (us *UserExternalLoginService) ExternalLogin( ctx context.Context, externalUserInfo *schema.ExternalLoginUserInfoCache) ( @@ -151,12 +190,16 @@ func (us *UserExternalLoginService) ExternalLogin( if err != nil { return nil, err } + if exist { + return &schema.UserExternalLoginResp{ + ErrTitle: translator.Tr(handler.GetLangByCtx(ctx), reason.UserAccessDenied), + ErrMsg: translator.Tr(handler.GetLangByCtx(ctx), reason.UserAccessDenied), + }, nil + } // if user is not a member, register a new user - if !exist { - oldUserInfo, err = us.registerNewUser(ctx, externalUserInfo) - if err != nil { - return nil, err - } + oldUserInfo, err = us.registerNewUser(ctx, externalUserInfo) + if err != nil { + return nil, err } // bind external user info to user err = us.bindOldUser(ctx, externalUserInfo, oldUserInfo) @@ -176,10 +219,41 @@ func (us *UserExternalLoginService) ExternalLogin( } accessToken, _, err := us.userCommonService.CacheLoginUserInfo( - ctx, oldUserInfo.ID, newMailStatus, oldUserInfo.Status, oldExternalLoginUserInfo.ExternalID) + ctx, oldUserInfo.ID, newMailStatus, oldUserInfo.Status, externalUserInfo.ExternalID) return &schema.UserExternalLoginResp{AccessToken: accessToken}, err } +func (us *UserExternalLoginService) BindExternalLoginToUser(ctx context.Context, + userID string, externalUserInfo *schema.ExternalLoginUserInfoCache) error { + if len(userID) == 0 || len(externalUserInfo.ExternalID) == 0 { + return errors.BadRequest(reason.UserAccessDenied) + } + oldUserInfo, exist, err := us.userRepo.GetByUserID(ctx, userID) + if err != nil { + return err + } + if !exist || oldUserInfo.Status == entity.UserStatusDeleted { + return errors.BadRequest(reason.UserNotFound) + } + oldExternalLoginUserInfo, exist, err := us.userExternalLoginRepo.GetByExternalID(ctx, + externalUserInfo.Provider, externalUserInfo.ExternalID) + if err != nil { + return err + } + if exist && oldExternalLoginUserInfo.UserID != userID { + return errors.BadRequest(reason.UserAccessDenied) + } + currentExternalLoginUserInfo, exist, err := us.userExternalLoginRepo.GetByUserID(ctx, + externalUserInfo.Provider, userID) + if err != nil { + return err + } + if exist && currentExternalLoginUserInfo.ExternalID != externalUserInfo.ExternalID { + return errors.BadRequest(reason.UserAccessDenied) + } + return us.bindOldUser(ctx, externalUserInfo, oldUserInfo) +} + func (us *UserExternalLoginService) registerNewUser(ctx context.Context, externalUserInfo *schema.ExternalLoginUserInfoCache) (userInfo *entity.User, err error) { userInfo = &entity.User{} @@ -297,18 +371,20 @@ func (us *UserExternalLoginService) ExternalLoginBindingUserSendEmail( resp.EmailExistAndMustBeConfirmed = true return resp, nil } + if exist { + resp.EmailExistAndMustBeConfirmed = true + return resp, nil + } - if !exist { - externalLoginInfo.Email = req.Email - userInfo, err = us.registerNewUser(ctx, externalLoginInfo) - if err != nil { - return nil, err - } - resp.AccessToken, _, err = us.userCommonService.CacheLoginUserInfo( - ctx, userInfo.ID, userInfo.MailStatus, userInfo.Status, externalLoginInfo.ExternalID) - if err != nil { - log.Error(err) - } + externalLoginInfo.Email = req.Email + userInfo, err = us.registerNewUser(ctx, externalLoginInfo) + if err != nil { + return nil, err + } + resp.AccessToken, _, err = us.userCommonService.CacheLoginUserInfo( + ctx, userInfo.ID, userInfo.MailStatus, userInfo.Status, externalLoginInfo.ExternalID) + if err != nil { + log.Error(err) } err = us.userExternalLoginRepo.SetCacheUserExternalLoginInfo(ctx, req.BindingKey, externalLoginInfo) if err != nil { @@ -340,6 +416,9 @@ func (us *UserExternalLoginService) ExternalLoginBindingUser( if err != nil || externalLoginInfo == nil { return errors.BadRequest(reason.UserNotFound) } + if len(externalLoginInfo.Email) == 0 || externalLoginInfo.Email != oldUserInfo.EMail { + return errors.BadRequest(reason.UserAccessDenied) + } return us.bindOldUser(ctx, externalLoginInfo, oldUserInfo) } From 54b1b35df5aec3b80c834c35546cf8a35d927395 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Tue, 7 Jul 2026 12:18:55 +0800 Subject: [PATCH 10/27] Align revision audit permission checks --- internal/service/content/revision_service.go | 48 ++++++++++++-------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/internal/service/content/revision_service.go b/internal/service/content/revision_service.go index 83568088..b932ba8b 100644 --- a/internal/service/content/revision_service.go +++ b/internal/service/content/revision_service.go @@ -114,14 +114,20 @@ func (rs *RevisionService) RevisionAudit(ctx context.Context, req *schema.Revisi if revisioninfo.Status != entity.RevisionUnreviewedStatus { return } + objectType, objectTypeerr := obj.GetObjectTypeStrByObjectID(revisioninfo.ObjectID) + if objectTypeerr != nil { + return objectTypeerr + } if req.Operation == schema.RevisionAuditReject { + if err = checkRevisionAuditPermission(req, objectType); err != nil { + return err + } err = rs.revisionRepo.UpdateStatus(ctx, req.ID, entity.RevisionReviewRejectStatus, req.UserID) return } if req.Operation == schema.RevisionAuditApprove { - objectType, objectTypeerr := obj.GetObjectTypeStrByObjectID(revisioninfo.ObjectID) - if objectTypeerr != nil { - return objectTypeerr + if err = checkRevisionAuditPermission(req, objectType); err != nil { + return err } revisionitem := &schema.GetRevisionResp{} _ = copier.Copy(revisionitem, revisioninfo) @@ -129,23 +135,11 @@ func (rs *RevisionService) RevisionAudit(ctx context.Context, req *schema.Revisi var saveErr error switch objectType { case constant.QuestionObjectType: - if !req.CanReviewQuestion { - saveErr = errors.BadRequest(reason.RevisionNoPermission) - } else { - saveErr = rs.revisionAuditQuestion(ctx, revisionitem) - } + saveErr = rs.revisionAuditQuestion(ctx, revisionitem) case constant.AnswerObjectType: - if !req.CanReviewAnswer { - saveErr = errors.BadRequest(reason.RevisionNoPermission) - } else { - saveErr = rs.revisionAuditAnswer(ctx, revisionitem) - } + saveErr = rs.revisionAuditAnswer(ctx, revisionitem) case constant.TagObjectType: - if !req.CanReviewTag { - saveErr = errors.BadRequest(reason.RevisionNoPermission) - } else { - saveErr = rs.revisionAuditTag(ctx, revisionitem) - } + saveErr = rs.revisionAuditTag(ctx, revisionitem) } if saveErr != nil { return saveErr @@ -179,6 +173,24 @@ func (rs *RevisionService) RevisionAudit(ctx context.Context, req *schema.Revisi return nil } +func checkRevisionAuditPermission(req *schema.RevisionAuditReq, objectType string) error { + switch objectType { + case constant.QuestionObjectType: + if !req.CanReviewQuestion { + return errors.BadRequest(reason.RevisionNoPermission) + } + case constant.AnswerObjectType: + if !req.CanReviewAnswer { + return errors.BadRequest(reason.RevisionNoPermission) + } + case constant.TagObjectType: + if !req.CanReviewTag { + return errors.BadRequest(reason.RevisionNoPermission) + } + } + return nil +} + func (rs *RevisionService) revisionAuditQuestion(ctx context.Context, revisionitem *schema.GetRevisionResp) (err error) { questioninfo, ok := revisionitem.ContentParsed.(*schema.QuestionInfoResp) if ok { From b575fe893bda63ede1112d8262fdef85893b4e1e Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Tue, 7 Jul 2026 14:33:21 +0800 Subject: [PATCH 11/27] Harden avatar cleanup ownership checks --- internal/service/content/user_service.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/internal/service/content/user_service.go b/internal/service/content/user_service.go index d0ebe875..c1f800ff 100644 --- a/internal/service/content/user_service.go +++ b/internal/service/content/user_service.go @@ -360,7 +360,7 @@ func (us *UserService) UpdateInfo(ctx context.Context, req *schema.UpdateInfoReq cond := us.formatUserInfoForUpdateInfo(oldUserInfo, req) - us.cleanUpRemovedAvatar(ctx, oldUserInfo.Avatar, cond.Avatar) + us.cleanUpRemovedAvatar(ctx, req.UserID, oldUserInfo.Avatar, cond.Avatar) err = us.userRepo.UpdateInfo(ctx, cond) if err != nil { @@ -407,6 +407,7 @@ func (us *UserService) validateAvatarInfo( func (us *UserService) cleanUpRemovedAvatar( ctx context.Context, + updatingUserID string, oldAvatarJSON string, newAvatarJSON string, ) { @@ -434,6 +435,13 @@ func (us *UserService) cleanUpRemovedAvatar( log.Warn("no file record found for old avatar url:", oldAvatar.Custom) return } + if fileRecord.UserID != updatingUserID || fileRecord.Source != string(plugin.UserAvatar) { + log.Warnf( + "refuse to clean avatar url %q: file record owner/source mismatch (owner=%s source=%s updating_user=%s)", + oldAvatar.Custom, fileRecord.UserID, fileRecord.Source, updatingUserID, + ) + return + } if err := us.fileRecordService.DeleteAndMoveFileRecord(ctx, fileRecord); err != nil { log.Error(err) } From f92a0fe140328bd180e46792557fd93f03e2e651 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Tue, 7 Jul 2026 15:05:47 +0800 Subject: [PATCH 12/27] Harden Accept-Language parsing --- go.mod | 14 ++++----- go.sum | 32 ++++++++++----------- internal/base/middleware/accept_language.go | 6 ++++ 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/go.mod b/go.mod index 11c3a816..7b68c180 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,7 @@ module github.com/apache/answer -go 1.24.0 +go 1.25.0 require ( github.com/Machiel/slugify v1.0.1 @@ -62,11 +62,11 @@ require ( github.com/tidwall/gjson v1.17.3 github.com/yuin/goldmark v1.7.4 go.uber.org/mock v0.6.0 - golang.org/x/crypto v0.41.0 + golang.org/x/crypto v0.53.0 golang.org/x/image v0.20.0 - golang.org/x/net v0.43.0 - golang.org/x/term v0.34.0 - golang.org/x/text v0.28.0 + golang.org/x/net v0.56.0 + golang.org/x/term v0.44.0 + golang.org/x/text v0.39.0 gopkg.in/gomail.v2 v2.0.0-20160411212932-81ebce5c23df gopkg.in/yaml.v3 v3.0.1 modernc.org/sqlite v1.33.0 @@ -171,8 +171,8 @@ require ( go.uber.org/zap v1.27.0 // indirect golang.org/x/arch v0.10.0 // indirect golang.org/x/exp v0.0.0-20240909161429-701f63a606c0 // indirect - golang.org/x/sys v0.35.0 // indirect - golang.org/x/tools v0.36.0 // indirect + golang.org/x/sys v0.46.0 // indirect + golang.org/x/tools v0.47.0 // indirect google.golang.org/protobuf v1.34.2 // indirect gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc // indirect gopkg.in/ini.v1 v1.67.0 // indirect diff --git a/go.sum b/go.sum index be61e11f..1001f1da 100644 --- a/go.sum +++ b/go.sum @@ -705,8 +705,8 @@ golang.org/x/crypto v0.0.0-20201203163018-be400aefbc4c/go.mod h1:jdWPYTVW3xRLrWP golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4= golang.org/x/crypto v0.0.0-20210616213533-5ff15b29337e/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/crypto v0.41.0 h1:WKYxWedPGCTVVl5+WHSSrOBT0O8lx32+zxmHxijgXp4= -golang.org/x/crypto v0.41.0/go.mod h1:pO5AFd7FA68rFak7rOAGVuygIISepHftHnr8dr6+sUc= +golang.org/x/crypto v0.53.0 h1:QZ4Muo8THX6CizN2vPPd5fBGHyogrdK9fG4wLPFUsto= +golang.org/x/crypto v0.53.0/go.mod h1:DNLU434OwVakk9PzuwV8w62mAJpRJL3vsgcfp4Qnsio= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20240909161429-701f63a606c0 h1:e66Fs6Z+fZTbFBAxKfP3PALWBtpfqks2bwGcexMxgtk= golang.org/x/exp v0.0.0-20240909161429-701f63a606c0/go.mod h1:2TbTHSBQa924w8M6Xs1QcRcFwyucIwBGpK1p2f1YFFY= @@ -723,8 +723,8 @@ golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzB golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= -golang.org/x/mod v0.27.0 h1:kb+q2PyFnEADO2IEF935ehFUXlWiNjJWtRNgBLSfbxQ= -golang.org/x/mod v0.27.0/go.mod h1:rWI627Fq0DEoudcK+MBkNkCe0EetEaDSwJJkCcjpazc= +golang.org/x/mod v0.37.0 h1:vF1DjpVEshcIqoEaauuHebaLk1O1forxjxBaVn884JQ= +golang.org/x/mod v0.37.0/go.mod h1:m8S8VeM9r4dzDwjrKO0a1sZP3YjeMamRRlD+fmR2Q/0= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -751,8 +751,8 @@ golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwY golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= -golang.org/x/net v0.43.0 h1:lat02VYK2j4aLzMzecihNvTlJNQUq316m2Mr9rnM6YE= -golang.org/x/net v0.43.0/go.mod h1:vhO1fvI4dGsIjh73sWfUVjj3N7CA9WkKJNQm2svM6Jg= +golang.org/x/net v0.56.0 h1:Rw8j/hFzGvJUZwNBXnAtf5sVDVt+65SK2C7IxCxZt5o= +golang.org/x/net v0.56.0/go.mod h1:D3Ku6r+V6JROoZK144D2XfMHFcMq/0zSfLelVTCFKec= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -763,8 +763,8 @@ golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.16.0 h1:ycBJEhp9p4vXvUZNszeOq0kGTPghopOL8q0fq3vstxw= -golang.org/x/sync v0.16.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= +golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM= +golang.org/x/sync v0.21.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -799,14 +799,14 @@ golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.35.0 h1:vz1N37gP5bs89s7He8XuIYXpyY0+QlsKmzipCbUtyxI= -golang.org/x/sys v0.35.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= +golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw= +golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= -golang.org/x/term v0.34.0 h1:O/2T7POpk0ZZ7MAzMeWFSg6S5IpWd/RXDlM9hgM3DR4= -golang.org/x/term v0.34.0/go.mod h1:5jC53AEywhIVebHgPVeg0mj8OD3VO9OzclacVrqpaAw= +golang.org/x/term v0.44.0 h1:0rLvDRCtNj0gZkyIXhCyOb2OAzEhLVqc4B+hrsBhrmc= +golang.org/x/term v0.44.0/go.mod h1:7ze4MdzUzLXpSAoFP1H0bOI9aXDqveSvatT5vKcFh2Y= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= @@ -815,8 +815,8 @@ golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= -golang.org/x/text v0.28.0 h1:rhazDwis8INMIwQ4tpjLDzUhx6RlXqZNPEM0huQojng= -golang.org/x/text v0.28.0/go.mod h1:U8nCwOR8jO/marOQ0QbDiOngZVEBB7MAiitBuMjXiNU= +golang.org/x/text v0.39.0 h1:UbZz4pLOvn600D6Oh6GGEI6VAmndrEBLv8/6BEXzyus= +golang.org/x/text v0.39.0/go.mod h1:3UwRclnC2g0TU9x8PZiyfOajCd1zaUNHF9cvqcQZ+ZM= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.12.0 h1:ScB/8o8olJvc+CQPWrK3fPZNfh7qgwCrY0zJmoEQLSE= @@ -842,8 +842,8 @@ golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roY golang.org/x/tools v0.0.0-20201124115921-2c860bdd6e78/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= -golang.org/x/tools v0.36.0 h1:kWS0uv/zsvHEle1LbV5LE8QujrxB3wfQyxHfhOk0Qkg= -golang.org/x/tools v0.36.0/go.mod h1:WBDiHKJK8YgLHlcQPYQzNCkUxUypCaa5ZegCVutKm+s= +golang.org/x/tools v0.47.0 h1:7Kn5x/d1svx/PzryTsqeoZN4TZwqeH5pGWjefhLi/1Q= +golang.org/x/tools v0.47.0/go.mod h1:dFHnyTvFWY212G+h7ZY4Vsp/K3U4/7W9TyVaAul8uCA= golang.org/x/xerrors v0.0.0-20190410155217-1f06c39b4373/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/internal/base/middleware/accept_language.go b/internal/base/middleware/accept_language.go index 5d1b12b2..a4e1b2f5 100644 --- a/internal/base/middleware/accept_language.go +++ b/internal/base/middleware/accept_language.go @@ -29,10 +29,16 @@ import ( "golang.org/x/text/language" ) +const maxAcceptLanguageLength = 256 + // ExtractAndSetAcceptLanguage extract accept language from header and set to context func ExtractAndSetAcceptLanguage(ctx *gin.Context) { // The language of our front-end configuration, like en_US acceptLanguage := ctx.GetHeader(constant.AcceptLanguageFlag) + if len(acceptLanguage) > maxAcceptLanguageLength { + ctx.Set(constant.AcceptLanguageFlag, i18n.LanguageEnglish) + return + } tag, _, err := language.ParseAcceptLanguage(acceptLanguage) if err != nil || len(tag) == 0 { ctx.Set(constant.AcceptLanguageFlag, i18n.LanguageEnglish) From a00cb2d38c018187ca73d409400f3c0c8f15294c Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Tue, 7 Jul 2026 16:26:45 +0800 Subject: [PATCH 13/27] fix: update goroutine handling and context imports for consistency --- go.mod | 2 +- internal/base/queue/queue.go | 6 ++---- internal/base/queue/queue_test.go | 6 ++---- internal/controller/template_render/tags.go | 3 ++- .../controller/template_render/userinfo.go | 3 ++- internal/service/content/question_service.go | 2 +- internal/service/export/email_service.go | 2 +- internal/service/report/report_service.go | 2 +- .../user_external_login_service.go | 19 +++++++------------ 9 files changed, 19 insertions(+), 26 deletions(-) diff --git a/go.mod b/go.mod index 7b68c180..5787c8b1 100644 --- a/go.mod +++ b/go.mod @@ -64,7 +64,6 @@ require ( go.uber.org/mock v0.6.0 golang.org/x/crypto v0.53.0 golang.org/x/image v0.20.0 - golang.org/x/net v0.56.0 golang.org/x/term v0.44.0 golang.org/x/text v0.39.0 gopkg.in/gomail.v2 v2.0.0-20160411212932-81ebce5c23df @@ -171,6 +170,7 @@ require ( go.uber.org/zap v1.27.0 // indirect golang.org/x/arch v0.10.0 // indirect golang.org/x/exp v0.0.0-20240909161429-701f63a606c0 // indirect + golang.org/x/net v0.56.0 // indirect golang.org/x/sys v0.46.0 // indirect golang.org/x/tools v0.47.0 // indirect google.golang.org/protobuf v1.34.2 // indirect diff --git a/internal/base/queue/queue.go b/internal/base/queue/queue.go index b3a8757a..4389c292 100644 --- a/internal/base/queue/queue.go +++ b/internal/base/queue/queue.go @@ -102,13 +102,11 @@ func (q *Queue[T]) Close() { // startWorker starts the background goroutine that processes messages. func (q *Queue[T]) startWorker() { - q.wg.Add(1) - go func() { - defer q.wg.Done() + q.wg.Go(func() { for msg := range q.queue { q.processMessage(msg) } - }() + }) } // processMessage handles a single message with proper synchronization. diff --git a/internal/base/queue/queue_test.go b/internal/base/queue/queue_test.go index 23f0fda7..b84cbde0 100644 --- a/internal/base/queue/queue_test.go +++ b/internal/base/queue/queue_test.go @@ -200,13 +200,11 @@ func TestQueue_ConcurrentRegisterHandler(t *testing.T) { // Concurrently register handlers - should not race var wg sync.WaitGroup for range 10 { - wg.Add(1) - go func() { - defer wg.Done() + wg.Go(func() { q.RegisterHandler(func(ctx context.Context, msg *testMessage) error { return nil }) - }() + }) } wg.Wait() } diff --git a/internal/controller/template_render/tags.go b/internal/controller/template_render/tags.go index 6a6dacc9..d7798c63 100644 --- a/internal/controller/template_render/tags.go +++ b/internal/controller/template_render/tags.go @@ -20,10 +20,11 @@ package templaterender import ( + "context" + "github.com/apache/answer/internal/base/pager" "github.com/apache/answer/internal/schema" "github.com/jinzhu/copier" - "golang.org/x/net/context" ) func (q *TemplateRenderController) TagList(ctx context.Context, req *schema.GetTagWithPageReq) (resp *pager.PageModel, err error) { diff --git a/internal/controller/template_render/userinfo.go b/internal/controller/template_render/userinfo.go index 1734f65c..eb4fb91c 100644 --- a/internal/controller/template_render/userinfo.go +++ b/internal/controller/template_render/userinfo.go @@ -20,8 +20,9 @@ package templaterender import ( + "context" + "github.com/apache/answer/internal/schema" - "golang.org/x/net/context" ) func (q *TemplateRenderController) UserInfo(ctx context.Context, req *schema.GetOtherUserInfoByUsernameReq) (resp *schema.GetOtherUserInfoByUsernameResp, err error) { diff --git a/internal/service/content/question_service.go b/internal/service/content/question_service.go index 474c1fcd..73f66a4c 100644 --- a/internal/service/content/question_service.go +++ b/internal/service/content/question_service.go @@ -20,6 +20,7 @@ package content import ( + "context" "encoding/json" "fmt" "strings" @@ -64,7 +65,6 @@ import ( "github.com/jinzhu/copier" "github.com/segmentfault/pacman/errors" "github.com/segmentfault/pacman/log" - "golang.org/x/net/context" ) // QuestionRepo question repository diff --git a/internal/service/export/email_service.go b/internal/service/export/email_service.go index bb00b828..5b649354 100644 --- a/internal/service/export/email_service.go +++ b/internal/service/export/email_service.go @@ -20,6 +20,7 @@ package export import ( + "context" "crypto/tls" "encoding/json" "fmt" @@ -40,7 +41,6 @@ import ( "github.com/apache/answer/internal/service/siteinfo_common" "github.com/segmentfault/pacman/errors" "github.com/segmentfault/pacman/log" - "golang.org/x/net/context" "gopkg.in/gomail.v2" ) diff --git a/internal/service/report/report_service.go b/internal/service/report/report_service.go index 84c15d59..3edbc1b4 100644 --- a/internal/service/report/report_service.go +++ b/internal/service/report/report_service.go @@ -20,6 +20,7 @@ package report import ( + "context" "encoding/json" "github.com/apache/answer/internal/service/eventqueue" @@ -44,7 +45,6 @@ import ( "github.com/jinzhu/copier" "github.com/segmentfault/pacman/errors" "github.com/segmentfault/pacman/log" - "golang.org/x/net/context" ) // ReportService user service diff --git a/internal/service/user_external_login/user_external_login_service.go b/internal/service/user_external_login/user_external_login_service.go index 2d209dbb..08f17ad4 100644 --- a/internal/service/user_external_login/user_external_login_service.go +++ b/internal/service/user_external_login/user_external_login_service.go @@ -186,18 +186,16 @@ func (us *UserExternalLoginService) ExternalLogin( }, nil } - oldUserInfo, exist, err := us.userRepo.GetByEmail(ctx, externalUserInfo.Email) - if err != nil { + if _, exist, err := us.userRepo.GetByEmail(ctx, externalUserInfo.Email); err != nil { return nil, err - } - if exist { + } else if exist { return &schema.UserExternalLoginResp{ ErrTitle: translator.Tr(handler.GetLangByCtx(ctx), reason.UserAccessDenied), ErrMsg: translator.Tr(handler.GetLangByCtx(ctx), reason.UserAccessDenied), }, nil } // if user is not a member, register a new user - oldUserInfo, err = us.registerNewUser(ctx, externalUserInfo) + oldUserInfo, err := us.registerNewUser(ctx, externalUserInfo) if err != nil { return nil, err } @@ -363,21 +361,18 @@ func (us *UserExternalLoginService) ExternalLoginBindingUserSendEmail( return &schema.ExternalLoginBindingUserSendEmailResp{}, nil } - userInfo, exist, err := us.userRepo.GetByEmail(ctx, req.Email) - if err != nil { + if _, exist, err := us.userRepo.GetByEmail(ctx, req.Email); err != nil { return nil, err - } - if exist && !req.Must { + } else if exist && !req.Must { resp.EmailExistAndMustBeConfirmed = true return resp, nil - } - if exist { + } else if exist { resp.EmailExistAndMustBeConfirmed = true return resp, nil } externalLoginInfo.Email = req.Email - userInfo, err = us.registerNewUser(ctx, externalLoginInfo) + userInfo, err := us.registerNewUser(ctx, externalLoginInfo) if err != nil { return nil, err } From dda51232b9a61631dcb1b4e49bcefd31d83c8deb Mon Sep 17 00:00:00 2001 From: Artur Iusupov Date: Wed, 17 Jun 2026 12:13:56 +0400 Subject: [PATCH 14/27] feat(notification): add interval for new question emails --- .../notification/new_question_notification.go | 38 +- .../new_question_notification_interval.go | 92 +++ .../new_question_notification_test.go | 652 ++++++++++++++++++ 3 files changed, 766 insertions(+), 16 deletions(-) create mode 100644 internal/service/notification/new_question_notification_interval.go create mode 100644 internal/service/notification/new_question_notification_test.go diff --git a/internal/service/notification/new_question_notification.go b/internal/service/notification/new_question_notification.go index 0a547187..bd323c7f 100644 --- a/internal/service/notification/new_question_notification.go +++ b/internal/service/notification/new_question_notification.go @@ -28,7 +28,6 @@ import ( "github.com/apache/answer/internal/base/translator" "github.com/apache/answer/internal/schema" "github.com/apache/answer/pkg/display" - "github.com/apache/answer/pkg/token" "github.com/apache/answer/plugin" "github.com/jinzhu/copier" "github.com/segmentfault/pacman/i18n" @@ -50,27 +49,34 @@ func (ns *ExternalNotificationService) handleNewQuestionNotification(ctx context } log.Debugf("get subscribers %d for question %s", len(subscribers), msg.NewQuestionTemplateRawData.QuestionID) - for _, subscriber := range subscribers { - for _, channel := range subscriber.Channels { - if !channel.Enable { - continue - } - if channel.Key == constant.EmailChannel { - ns.sendNewQuestionNotificationEmail(ctx, subscriber.UserID, &schema.NewQuestionTemplateRawData{ - QuestionTitle: msg.NewQuestionTemplateRawData.QuestionTitle, - QuestionID: msg.NewQuestionTemplateRawData.QuestionID, - UnsubscribeCode: token.GenerateToken(), - Tags: msg.NewQuestionTemplateRawData.Tags, - TagIDs: msg.NewQuestionTemplateRawData.TagIDs, - }) - } - } + interval := newQuestionNotificationEmailSendInterval() + if interval > 0 { + ns.syncNewQuestionNotificationToPlugin(ctx, msg) + ns.sendNewQuestionNotificationEmails(ctx, subscribers, msg.NewQuestionTemplateRawData, interval) + return nil } + ns.sendNewQuestionNotificationEmails(ctx, subscribers, msg.NewQuestionTemplateRawData, interval) ns.syncNewQuestionNotificationToPlugin(ctx, msg) return nil } +func (ns *ExternalNotificationService) sendNewQuestionNotificationEmails( + ctx context.Context, + subscribers []*NewQuestionSubscriber, + rawData *schema.NewQuestionTemplateRawData, + interval time.Duration, +) { + sendNewQuestionNotificationEmailsWithInterval( + ctx, + subscribers, + rawData, + interval, + nil, + ns.sendNewQuestionNotificationEmail, + ) +} + func (ns *ExternalNotificationService) getNewQuestionSubscribers(ctx context.Context, msg *schema.ExternalNotificationMsg) ( subscribers []*NewQuestionSubscriber, err error) { subscribersMapping := make(map[string]*NewQuestionSubscriber) diff --git a/internal/service/notification/new_question_notification_interval.go b/internal/service/notification/new_question_notification_interval.go new file mode 100644 index 00000000..22f17450 --- /dev/null +++ b/internal/service/notification/new_question_notification_interval.go @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package notification + +import ( + "context" + "os" + "strconv" + "strings" + "time" + + "github.com/apache/answer/internal/base/constant" + "github.com/apache/answer/internal/schema" + "github.com/apache/answer/pkg/token" +) + +const newQuestionNotificationEmailSendIntervalEnv = "NEW_QUESTION_NOTIFICATION_EMAIL_SEND_INTERVAL_SECONDS" + +const maxNewQuestionNotificationEmailSendIntervalSeconds = int64(1<<63-1) / int64(time.Second) + +type newQuestionNotificationEmailSleeper func(time.Duration) + +type newQuestionNotificationEmailSender func(context.Context, string, *schema.NewQuestionTemplateRawData) + +func newQuestionNotificationEmailSendInterval() time.Duration { + return parseNewQuestionNotificationEmailSendInterval(os.Getenv(newQuestionNotificationEmailSendIntervalEnv)) +} + +func parseNewQuestionNotificationEmailSendInterval(value string) time.Duration { + value = strings.TrimSpace(value) + if len(value) == 0 { + return 0 + } + seconds, err := strconv.ParseInt(value, 10, 64) + if err != nil || seconds < 0 || seconds > maxNewQuestionNotificationEmailSendIntervalSeconds { + return 0 + } + return time.Duration(seconds) * time.Second +} + +func sendNewQuestionNotificationEmailsWithInterval( + ctx context.Context, + subscribers []*NewQuestionSubscriber, + rawData *schema.NewQuestionTemplateRawData, + interval time.Duration, + sleep newQuestionNotificationEmailSleeper, + send newQuestionNotificationEmailSender, +) { + if rawData == nil || send == nil { + return + } + if sleep == nil { + sleep = time.Sleep + } + + emailAttempts := 0 + for _, subscriber := range subscribers { + for _, channel := range subscriber.Channels { + if !channel.Enable || channel.Key != constant.EmailChannel { + continue + } + if interval > 0 && emailAttempts > 0 { + sleep(interval) + } + send(ctx, subscriber.UserID, &schema.NewQuestionTemplateRawData{ + QuestionTitle: rawData.QuestionTitle, + QuestionID: rawData.QuestionID, + UnsubscribeCode: token.GenerateToken(), + Tags: rawData.Tags, + TagIDs: rawData.TagIDs, + }) + emailAttempts++ + } + } +} diff --git a/internal/service/notification/new_question_notification_test.go b/internal/service/notification/new_question_notification_test.go new file mode 100644 index 00000000..4d1729a2 --- /dev/null +++ b/internal/service/notification/new_question_notification_test.go @@ -0,0 +1,652 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package notification + +import ( + "context" + "encoding/json" + "os" + "reflect" + "testing" + "time" + + "github.com/apache/answer/internal/base/constant" + basedata "github.com/apache/answer/internal/base/data" + "github.com/apache/answer/internal/entity" + "github.com/apache/answer/internal/schema" + "github.com/apache/answer/internal/service/config" + "github.com/apache/answer/internal/service/export" + "github.com/apache/answer/internal/service/mock" + "go.uber.org/mock/gomock" +) + +func TestNewQuestionNotificationEmailSendInterval(t *testing.T) { + tests := []struct { + name string + value string + set bool + want time.Duration + }{ + { + name: "unset", + want: 0, + }, + { + name: "empty", + value: "", + set: true, + want: 0, + }, + { + name: "positive integer", + value: "5", + set: true, + want: 5 * time.Second, + }, + { + name: "positive integer with whitespace", + value: " 5 ", + set: true, + want: 5 * time.Second, + }, + { + name: "invalid", + value: "not-a-number", + set: true, + want: 0, + }, + { + name: "negative", + value: "-1", + set: true, + want: 0, + }, + { + name: "duration overflow", + value: "9223372037", + set: true, + want: 0, + }, + { + name: "parse int overflow", + value: "9223372036854775808", + set: true, + want: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + setNewQuestionNotificationEmailSendIntervalEnv(t, tt.value, tt.set) + + got := newQuestionNotificationEmailSendInterval() + if got != tt.want { + t.Fatalf("newQuestionNotificationEmailSendInterval() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestSendNewQuestionNotificationEmailsWithInterval(t *testing.T) { + rawData := &schema.NewQuestionTemplateRawData{ + QuestionTitle: "question", + QuestionID: "1", + Tags: []string{"go"}, + TagIDs: []string{"tag-1"}, + } + interval := 3 * time.Second + + tests := []struct { + name string + interval time.Duration + subscribers []*NewQuestionSubscriber + wantSends []string + wantSleeps []time.Duration + wantEvents []string + }{ + { + name: "interval 0", + interval: 0, + subscribers: []*NewQuestionSubscriber{ + newQuestionSubscriber("user-1", newQuestionEmailChannel(true)), + newQuestionSubscriber("user-2", newQuestionEmailChannel(true)), + }, + wantSends: []string{"user-1", "user-2"}, + wantEvents: []string{ + "send:user-1", + "send:user-2", + }, + }, + { + name: "0 enabled email attempts", + interval: interval, + subscribers: []*NewQuestionSubscriber{ + newQuestionSubscriber("user-1", newQuestionEmailChannel(false)), + newQuestionSubscriber("user-2", newQuestionNonEmailChannel(true)), + }, + }, + { + name: "1 enabled email attempt", + interval: interval, + subscribers: []*NewQuestionSubscriber{ + newQuestionSubscriber("user-1", newQuestionEmailChannel(true)), + }, + wantSends: []string{"user-1"}, + wantEvents: []string{ + "send:user-1", + }, + }, + { + name: "N enabled email attempts", + interval: interval, + subscribers: []*NewQuestionSubscriber{ + newQuestionSubscriber("user-1", newQuestionEmailChannel(true)), + newQuestionSubscriber("user-2", newQuestionEmailChannel(true)), + newQuestionSubscriber("user-3", newQuestionEmailChannel(true)), + }, + wantSends: []string{"user-1", "user-2", "user-3"}, + wantSleeps: []time.Duration{interval, interval}, + wantEvents: []string{ + "send:user-1", + "sleep:3s", + "send:user-2", + "sleep:3s", + "send:user-3", + }, + }, + { + name: "disabled email channel does not add delay", + interval: interval, + subscribers: []*NewQuestionSubscriber{ + newQuestionSubscriber("user-1", newQuestionEmailChannel(true)), + newQuestionSubscriber("user-2", newQuestionEmailChannel(false)), + newQuestionSubscriber("user-3", newQuestionEmailChannel(true)), + }, + wantSends: []string{"user-1", "user-3"}, + wantSleeps: []time.Duration{interval}, + wantEvents: []string{ + "send:user-1", + "sleep:3s", + "send:user-3", + }, + }, + { + name: "non-email channel does not add delay", + interval: interval, + subscribers: []*NewQuestionSubscriber{ + newQuestionSubscriber("user-1", newQuestionEmailChannel(true)), + newQuestionSubscriber("user-2", newQuestionNonEmailChannel(true)), + newQuestionSubscriber("user-3", newQuestionEmailChannel(true)), + }, + wantSends: []string{"user-1", "user-3"}, + wantSleeps: []time.Duration{interval}, + wantEvents: []string{ + "send:user-1", + "sleep:3s", + "send:user-3", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var gotEvents []string + var gotSleeps []time.Duration + sleep := func(duration time.Duration) { + gotSleeps = append(gotSleeps, duration) + gotEvents = append(gotEvents, "sleep:"+duration.String()) + } + + var gotSends []string + var gotCodes []string + send := func(_ context.Context, userID string, rawData *schema.NewQuestionTemplateRawData) { + gotSends = append(gotSends, userID) + gotEvents = append(gotEvents, "send:"+userID) + if rawData.UnsubscribeCode == "" { + t.Fatalf("expected unsubscribe code for %s", userID) + } + gotCodes = append(gotCodes, rawData.UnsubscribeCode) + } + + sendNewQuestionNotificationEmailsWithInterval( + context.Background(), tt.subscribers, rawData, tt.interval, sleep, send) + + if !reflect.DeepEqual(gotSends, tt.wantSends) { + t.Fatalf("send calls = %v, want %v", gotSends, tt.wantSends) + } + if !reflect.DeepEqual(gotSleeps, tt.wantSleeps) { + t.Fatalf("sleep calls = %v, want %v", gotSleeps, tt.wantSleeps) + } + if !reflect.DeepEqual(gotEvents, tt.wantEvents) { + t.Fatalf("events = %v, want %v", gotEvents, tt.wantEvents) + } + assertUniqueNewQuestionUnsubscribeCodes(t, gotCodes) + }) + } +} + +func TestHandleNewQuestionNotificationSendsEmailsThroughFanOut(t *testing.T) { + setNewQuestionNotificationEmailSendIntervalEnv(t, "0", true) + + cache, cleanup, err := basedata.NewCache(&basedata.CacheConf{}) + if err != nil { + t.Fatalf("new cache: %v", err) + } + t.Cleanup(cleanup) + + ctrl := gomock.NewController(t) + siteInfoService := mock.NewMockSiteInfoCommonService(ctrl) + siteInfoService.EXPECT().GetSiteGeneral(gomock.Any()).Return(&schema.SiteGeneralResp{ + Name: "Answer", + SiteUrl: "https://answer.test", + ContactEmail: "support@answer.test", + }, nil).AnyTimes() + siteInfoService.EXPECT().GetSiteSeo(gomock.Any()).Return(&schema.SiteSeoResp{ + Permalink: constant.PermalinkQuestionIDAndTitle, + }, nil).AnyTimes() + + emailRepo := &newQuestionNotificationTestEmailRepo{ + codesByUserID: make(map[string][]string), + } + notificationConfigRepo := &newQuestionNotificationTestUserNotificationConfigRepo{ + followedTagConfigs: map[string]*entity.UserNotificationConfig{ + "tag-user": newQuestionNotificationConfig( + "tag-user", constant.AllNewQuestionForFollowingTagsSource, true), + "dup-user": newQuestionNotificationConfig( + "dup-user", constant.AllNewQuestionForFollowingTagsSource, true), + "author": newQuestionNotificationConfig( + "author", constant.AllNewQuestionForFollowingTagsSource, true), + }, + allQuestionConfigs: []*entity.UserNotificationConfig{ + newQuestionNotificationConfig("all-user", constant.AllNewQuestionSource, true), + newQuestionNotificationConfig("dup-user", constant.AllNewQuestionSource, true), + newQuestionNotificationConfig("author", constant.AllNewQuestionSource, true), + }, + } + service := &ExternalNotificationService{ + data: &basedata.Data{ + Cache: cache, + }, + userNotificationConfigRepo: notificationConfigRepo, + followRepo: &newQuestionNotificationTestFollowRepo{ + followersByObjectID: map[string][]string{ + "tag-1": {"tag-user", "dup-user", "author"}, + }, + }, + emailService: export.NewEmailService( + config.NewConfigService(newQuestionNotificationTestConfigRepo{}), + emailRepo, + siteInfoService, + ), + userRepo: &newQuestionNotificationTestUserRepo{ + users: map[string]*entity.User{ + "tag-user": newQuestionNotificationTestUser("tag-user"), + "dup-user": newQuestionNotificationTestUser("dup-user"), + "all-user": newQuestionNotificationTestUser("all-user"), + "author": newQuestionNotificationTestUser("author"), + }, + }, + siteInfoService: siteInfoService, + } + + err = service.handleNewQuestionNotification(context.Background(), &schema.ExternalNotificationMsg{ + NewQuestionTemplateRawData: &schema.NewQuestionTemplateRawData{ + QuestionTitle: "New question", + QuestionID: "1", + QuestionAuthorUserID: "author", + Tags: []string{"go"}, + TagIDs: []string{"tag-1"}, + }, + }) + if err != nil { + t.Fatalf("handleNewQuestionNotification() error = %v", err) + } + + wantUsers := []string{"all-user", "dup-user", "tag-user"} + assertStringSet(t, emailRepo.userIDs(), wantUsers) + for _, userID := range wantUsers { + codes := emailRepo.codesByUserID[userID] + if len(codes) != 1 { + t.Fatalf("saved codes for %s = %v, want exactly one code", userID, codes) + } + if codes[0] == "" { + t.Fatalf("saved empty code for %s", userID) + } + } + if codes := emailRepo.codesByUserID["author"]; len(codes) > 0 { + t.Fatalf("question author received notification codes: %v", codes) + } +} + +func assertUniqueNewQuestionUnsubscribeCodes(t *testing.T, codes []string) { + t.Helper() + + seen := make(map[string]bool) + for _, code := range codes { + if seen[code] { + t.Fatalf("duplicate unsubscribe code %q", code) + } + seen[code] = true + } +} + +func setNewQuestionNotificationEmailSendIntervalEnv(t *testing.T, value string, set bool) { + t.Helper() + + oldValue, oldSet := os.LookupEnv(newQuestionNotificationEmailSendIntervalEnv) + if set { + if err := os.Setenv(newQuestionNotificationEmailSendIntervalEnv, value); err != nil { + t.Fatalf("set env: %v", err) + } + } else { + if err := os.Unsetenv(newQuestionNotificationEmailSendIntervalEnv); err != nil { + t.Fatalf("unset env: %v", err) + } + } + t.Cleanup(func() { + if oldSet { + _ = os.Setenv(newQuestionNotificationEmailSendIntervalEnv, oldValue) + } else { + _ = os.Unsetenv(newQuestionNotificationEmailSendIntervalEnv) + } + }) +} + +func newQuestionSubscriber(userID string, channels ...*schema.NotificationChannelConfig) *NewQuestionSubscriber { + return &NewQuestionSubscriber{ + UserID: userID, + Channels: channels, + } +} + +func newQuestionEmailChannel(enable bool) *schema.NotificationChannelConfig { + return &schema.NotificationChannelConfig{ + Key: constant.EmailChannel, + Enable: enable, + } +} + +func newQuestionNonEmailChannel(enable bool) *schema.NotificationChannelConfig { + return &schema.NotificationChannelConfig{ + Key: constant.NotificationChannelKey("inbox"), + Enable: enable, + } +} + +func newQuestionNotificationConfig( + userID string, source constant.NotificationSource, emailEnabled bool) *entity.UserNotificationConfig { + channels := schema.NotificationChannels{ + newQuestionEmailChannel(emailEnabled), + } + return &entity.UserNotificationConfig{ + UserID: userID, + Source: string(source), + Channels: channels.ToJsonString(), + Enabled: emailEnabled, + } +} + +func newQuestionNotificationTestUser(userID string) *entity.User { + return &entity.User{ + ID: userID, + Username: userID, + DisplayName: userID, + EMail: userID + "@example.com", + Status: entity.UserStatusAvailable, + MailStatus: entity.EmailStatusAvailable, + } +} + +func assertStringSet(t *testing.T, got, want []string) { + t.Helper() + + gotSet := make(map[string]bool) + for _, value := range got { + gotSet[value] = true + } + wantSet := make(map[string]bool) + for _, value := range want { + wantSet[value] = true + } + if !reflect.DeepEqual(gotSet, wantSet) { + t.Fatalf("values = %v, want %v", got, want) + } +} + +type newQuestionNotificationTestFollowRepo struct { + followersByObjectID map[string][]string +} + +func (r *newQuestionNotificationTestFollowRepo) GetFollowIDs( + context.Context, string, string) ([]string, error) { + return nil, nil +} + +func (r *newQuestionNotificationTestFollowRepo) GetFollowAmount(context.Context, string) (int, error) { + return 0, nil +} + +func (r *newQuestionNotificationTestFollowRepo) GetFollowUserIDs( + _ context.Context, objectID string) ([]string, error) { + return r.followersByObjectID[objectID], nil +} + +func (r *newQuestionNotificationTestFollowRepo) IsFollowed(context.Context, string, string) (bool, error) { + return false, nil +} + +func (r *newQuestionNotificationTestFollowRepo) MigrateFollowers( + context.Context, string, string, string) error { + return nil +} + +type newQuestionNotificationTestUserNotificationConfigRepo struct { + followedTagConfigs map[string]*entity.UserNotificationConfig + allQuestionConfigs []*entity.UserNotificationConfig +} + +func (r *newQuestionNotificationTestUserNotificationConfigRepo) Add( + context.Context, []string, string, string) error { + return nil +} + +func (r *newQuestionNotificationTestUserNotificationConfigRepo) Save( + context.Context, *entity.UserNotificationConfig) error { + return nil +} + +func (r *newQuestionNotificationTestUserNotificationConfigRepo) GetByUserID( + context.Context, string) ([]*entity.UserNotificationConfig, error) { + return nil, nil +} + +func (r *newQuestionNotificationTestUserNotificationConfigRepo) GetBySource( + _ context.Context, source constant.NotificationSource) ([]*entity.UserNotificationConfig, error) { + if source == constant.AllNewQuestionSource { + return r.allQuestionConfigs, nil + } + return nil, nil +} + +func (r *newQuestionNotificationTestUserNotificationConfigRepo) GetByUserIDAndSource( + context.Context, string, constant.NotificationSource) (*entity.UserNotificationConfig, bool, error) { + return nil, false, nil +} + +func (r *newQuestionNotificationTestUserNotificationConfigRepo) GetByUsersAndSource( + _ context.Context, userIDs []string, source constant.NotificationSource) ( + []*entity.UserNotificationConfig, error) { + if source != constant.AllNewQuestionForFollowingTagsSource { + return nil, nil + } + configs := make([]*entity.UserNotificationConfig, 0, len(userIDs)) + for _, userID := range userIDs { + if config, ok := r.followedTagConfigs[userID]; ok { + configs = append(configs, config) + } + } + return configs, nil +} + +type newQuestionNotificationTestUserRepo struct { + users map[string]*entity.User +} + +func (r *newQuestionNotificationTestUserRepo) AddUser(context.Context, *entity.User) error { + return nil +} + +func (r *newQuestionNotificationTestUserRepo) IncreaseAnswerCount(context.Context, string, int) error { + return nil +} + +func (r *newQuestionNotificationTestUserRepo) IncreaseQuestionCount(context.Context, string, int) error { + return nil +} + +func (r *newQuestionNotificationTestUserRepo) UpdateQuestionCount(context.Context, string, int64) error { + return nil +} + +func (r *newQuestionNotificationTestUserRepo) UpdateAnswerCount(context.Context, string, int) error { + return nil +} + +func (r *newQuestionNotificationTestUserRepo) UpdateLastLoginDate(context.Context, string) error { + return nil +} + +func (r *newQuestionNotificationTestUserRepo) UpdateEmailStatus(context.Context, string, int) error { + return nil +} + +func (r *newQuestionNotificationTestUserRepo) UpdateNoticeStatus(context.Context, string, int) error { + return nil +} + +func (r *newQuestionNotificationTestUserRepo) UpdateEmail(context.Context, string, string) error { + return nil +} + +func (r *newQuestionNotificationTestUserRepo) UpdateUserInterface( + context.Context, string, string, string) error { + return nil +} + +func (r *newQuestionNotificationTestUserRepo) UpdatePass(context.Context, string, string) error { + return nil +} + +func (r *newQuestionNotificationTestUserRepo) UpdateInfo(context.Context, *entity.User) error { + return nil +} + +func (r *newQuestionNotificationTestUserRepo) UpdateUserProfile(context.Context, *entity.User) error { + return nil +} + +func (r *newQuestionNotificationTestUserRepo) GetByUserID( + _ context.Context, userID string) (*entity.User, bool, error) { + user, ok := r.users[userID] + return user, ok, nil +} + +func (r *newQuestionNotificationTestUserRepo) BatchGetByID( + context.Context, []string) ([]*entity.User, error) { + return nil, nil +} + +func (r *newQuestionNotificationTestUserRepo) GetByUsername( + context.Context, string) (*entity.User, bool, error) { + return nil, false, nil +} + +func (r *newQuestionNotificationTestUserRepo) GetByUsernames( + context.Context, []string) ([]*entity.User, error) { + return nil, nil +} + +func (r *newQuestionNotificationTestUserRepo) GetByEmail( + context.Context, string) (*entity.User, bool, error) { + return nil, false, nil +} + +func (r *newQuestionNotificationTestUserRepo) GetUserCount(context.Context) (int64, error) { + return 0, nil +} + +func (r *newQuestionNotificationTestUserRepo) SearchUserListByName( + context.Context, string, int, bool) ([]*entity.User, error) { + return nil, nil +} + +func (r *newQuestionNotificationTestUserRepo) IsAvatarFileUsed(context.Context, string) (bool, error) { + return false, nil +} + +type newQuestionNotificationTestConfigRepo struct{} + +func (newQuestionNotificationTestConfigRepo) GetConfigByID( + context.Context, int) (*entity.Config, error) { + return nil, nil +} + +func (newQuestionNotificationTestConfigRepo) GetConfigByKey( + context.Context, string) (*entity.Config, error) { + config := export.EmailConfig{ + FromEmail: "noreply@answer.test", + FromName: "Answer", + } + value, _ := json.Marshal(config) + return &entity.Config{ + Value: string(value), + }, nil +} + +func (newQuestionNotificationTestConfigRepo) GetConfigByKeyFromDB( + context.Context, string) (*entity.Config, error) { + return nil, nil +} + +func (newQuestionNotificationTestConfigRepo) UpdateConfig(context.Context, string, string) error { + return nil +} + +type newQuestionNotificationTestEmailRepo struct { + codesByUserID map[string][]string +} + +func (r *newQuestionNotificationTestEmailRepo) SetCode( + _ context.Context, userID, code, _ string, _ time.Duration) error { + r.codesByUserID[userID] = append(r.codesByUserID[userID], code) + return nil +} + +func (r *newQuestionNotificationTestEmailRepo) VerifyCode(context.Context, string) (string, error) { + return "", nil +} + +func (r *newQuestionNotificationTestEmailRepo) userIDs() []string { + userIDs := make([]string, 0, len(r.codesByUserID)) + for userID := range r.codesByUserID { + userIDs = append(userIDs, userID) + } + return userIDs +} From cb9739421154ad4e3c9d214ae5042343b10ef91c Mon Sep 17 00:00:00 2001 From: Artur Iusupov Date: Sat, 27 Jun 2026 23:35:28 +0400 Subject: [PATCH 15/27] fix(notification): move new question email throttling to worker --- .../notification/external_notification.go | 5 + .../notification/new_question_email_worker.go | 308 ++++++++++ .../new_question_email_worker_test.go | 558 ++++++++++++++++++ .../notification/new_question_notification.go | 48 +- .../new_question_notification_interval.go | 47 +- .../new_question_notification_test.go | 476 ++++++++++----- 6 files changed, 1229 insertions(+), 213 deletions(-) create mode 100644 internal/service/notification/new_question_email_worker.go create mode 100644 internal/service/notification/new_question_email_worker_test.go diff --git a/internal/service/notification/external_notification.go b/internal/service/notification/external_notification.go index 425a8c2b..5282cab0 100644 --- a/internal/service/notification/external_notification.go +++ b/internal/service/notification/external_notification.go @@ -45,6 +45,7 @@ type ExternalNotificationService struct { notificationQueueService noticequeue.ExternalService userExternalLoginRepo user_external_login.UserExternalLoginRepo siteInfoService siteinfo_common.SiteInfoCommonService + newQuestionEmailWorker *newQuestionEmailWorker } func NewExternalNotificationService( @@ -67,6 +68,10 @@ func NewExternalNotificationService( userExternalLoginRepo: userExternalLoginRepo, siteInfoService: siteInfoService, } + n.newQuestionEmailWorker = newQuestionEmailWorkerWithDefaults( + newQuestionNotificationEmailSendInterval, + n.sendNewQuestionNotificationEmail, + ) notificationQueueService.RegisterHandler(n.Handler) return n } diff --git a/internal/service/notification/new_question_email_worker.go b/internal/service/notification/new_question_email_worker.go new file mode 100644 index 00000000..be3e6504 --- /dev/null +++ b/internal/service/notification/new_question_email_worker.go @@ -0,0 +1,308 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package notification + +import ( + "context" + "sync" + "time" + + "github.com/apache/answer/internal/schema" + "github.com/apache/answer/pkg/token" + "github.com/segmentfault/pacman/log" +) + +const newQuestionEmailWorkerQueueSize = 128 + +type newQuestionEmailTask struct { + UserIDs []string + QuestionTitle string + QuestionID string + Tags []string + TagIDs []string +} + +type newQuestionEmailIntervalProvider func() time.Duration + +type newQuestionEmailTimer interface { + C() <-chan time.Time + Stop() +} + +type newQuestionEmailTimerFactory func(time.Duration) newQuestionEmailTimer + +type newQuestionEmailWorker struct { + tasks chan newQuestionEmailTask + send newQuestionNotificationEmailSender + interval newQuestionEmailIntervalProvider + timerFactory newQuestionEmailTimerFactory + ctx context.Context + cancel context.CancelFunc + mu sync.RWMutex + closed bool + wg sync.WaitGroup +} + +func newQuestionEmailWorkerWithDefaults( + interval newQuestionEmailIntervalProvider, + send newQuestionNotificationEmailSender, +) *newQuestionEmailWorker { + return newQuestionEmailWorkerWithBuffer( + interval, + send, + newRealNewQuestionEmailTimer, + newQuestionEmailWorkerQueueSize, + ) +} + +func newQuestionEmailWorkerWithBuffer( + interval newQuestionEmailIntervalProvider, + send newQuestionNotificationEmailSender, + timerFactory newQuestionEmailTimerFactory, + bufferSize int, +) *newQuestionEmailWorker { + if interval == nil { + interval = newQuestionNotificationEmailSendInterval + } + if timerFactory == nil { + timerFactory = newRealNewQuestionEmailTimer + } + ctx, cancel := context.WithCancel(context.Background()) + w := &newQuestionEmailWorker{ + tasks: make(chan newQuestionEmailTask, bufferSize), + send: send, + interval: interval, + timerFactory: timerFactory, + ctx: ctx, + cancel: cancel, + } + w.wg.Add(1) + go w.run() + return w +} + +func (w *newQuestionEmailWorker) TryEnqueue(task newQuestionEmailTask) bool { + if w == nil { + log.Warnf("[new_question_email] worker is nil, dropping new question email task") + return false + } + + task = copyNewQuestionEmailTask(task) + + w.mu.RLock() + defer w.mu.RUnlock() + + if w.closed { + log.Warnf("[new_question_email] worker is closed, dropping new question email task for question %s", task.QuestionID) + return false + } + + if w.ctx == nil { + log.Warnf("[new_question_email] worker context is nil, dropping new question email task for question %s", task.QuestionID) + return false + } + + select { + case <-w.ctx.Done(): + log.Warnf("[new_question_email] worker is canceled, dropping new question email task for question %s", task.QuestionID) + return false + default: + } + + select { + case w.tasks <- task: + log.Debugf("[new_question_email] enqueued task for question %s to %d users", task.QuestionID, len(task.UserIDs)) + return true + case <-w.ctx.Done(): + log.Warnf("[new_question_email] worker canceled while enqueueing task for question %s", task.QuestionID) + return false + default: + log.Warnf("[new_question_email] queue is full, dropping new question email task for question %s", task.QuestionID) + return false + } +} + +func (w *newQuestionEmailWorker) Close() { + if w == nil { + return + } + + w.mu.Lock() + if w.closed { + w.mu.Unlock() + return + } + w.closed = true + if w.cancel != nil { + w.cancel() + } + w.mu.Unlock() + + w.wg.Wait() + if dropped := w.dropPendingTasks(); dropped > 0 { + log.Warnf("[new_question_email] dropped %d pending tasks during shutdown", dropped) + } + log.Infof("[new_question_email] worker closed") +} + +func (w *newQuestionEmailWorker) run() { + defer w.wg.Done() + + emailAttemptSent := false + for { + if w.ctx.Err() != nil { + return + } + + select { + case <-w.ctx.Done(): + return + case task := <-w.tasks: + if w.ctx.Err() != nil { + return + } + if !w.processTask(task, &emailAttemptSent) { + return + } + } + } +} + +func (w *newQuestionEmailWorker) processTask(task newQuestionEmailTask, emailAttemptSent *bool) bool { + for _, userID := range task.UserIDs { + if w.ctx.Err() != nil { + return false + } + if *emailAttemptSent { + interval := w.interval() + if interval > 0 && !waitNewQuestionEmailInterval(w.ctx, interval, w.timerFactory) { + return false + } + } + if w.ctx.Err() != nil { + return false + } + if w.send == nil { + log.Errorf("[new_question_email] sender is nil, dropping email attempt for user %s question %s", userID, task.QuestionID) + *emailAttemptSent = true + continue + } + w.send(w.ctx, userID, task.newRawData()) + *emailAttemptSent = true + } + return true +} + +func (w *newQuestionEmailWorker) dropPendingTasks() int { + dropped := 0 + for { + select { + case <-w.tasks: + dropped++ + default: + return dropped + } + } +} + +func waitNewQuestionEmailInterval( + ctx context.Context, + interval time.Duration, + timerFactory newQuestionEmailTimerFactory, +) bool { + if interval <= 0 { + return true + } + if timerFactory == nil { + timerFactory = newRealNewQuestionEmailTimer + } + timer := timerFactory(interval) + defer timer.Stop() + + select { + case <-timer.C(): + return true + case <-ctx.Done(): + return false + } +} + +func (task newQuestionEmailTask) newRawData() *schema.NewQuestionTemplateRawData { + return &schema.NewQuestionTemplateRawData{ + QuestionTitle: task.QuestionTitle, + QuestionID: task.QuestionID, + UnsubscribeCode: token.GenerateToken(), + Tags: copyStringSlice(task.Tags), + TagIDs: copyStringSlice(task.TagIDs), + } +} + +func newQuestionEmailTaskFromRawData( + userIDs []string, + rawData *schema.NewQuestionTemplateRawData, +) newQuestionEmailTask { + if rawData == nil { + return newQuestionEmailTask{UserIDs: copyStringSlice(userIDs)} + } + return newQuestionEmailTask{ + UserIDs: copyStringSlice(userIDs), + QuestionTitle: rawData.QuestionTitle, + QuestionID: rawData.QuestionID, + Tags: copyStringSlice(rawData.Tags), + TagIDs: copyStringSlice(rawData.TagIDs), + } +} + +func copyNewQuestionEmailTask(task newQuestionEmailTask) newQuestionEmailTask { + task.UserIDs = copyStringSlice(task.UserIDs) + task.Tags = copyStringSlice(task.Tags) + task.TagIDs = copyStringSlice(task.TagIDs) + return task +} + +func copyStringSlice(values []string) []string { + if values == nil { + return nil + } + copied := make([]string, len(values)) + copy(copied, values) + return copied +} + +type realNewQuestionEmailTimer struct { + timer *time.Timer +} + +func newRealNewQuestionEmailTimer(interval time.Duration) newQuestionEmailTimer { + return &realNewQuestionEmailTimer{timer: time.NewTimer(interval)} +} + +func (t *realNewQuestionEmailTimer) C() <-chan time.Time { + return t.timer.C +} + +func (t *realNewQuestionEmailTimer) Stop() { + if !t.timer.Stop() { + select { + case <-t.timer.C: + default: + } + } +} diff --git a/internal/service/notification/new_question_email_worker_test.go b/internal/service/notification/new_question_email_worker_test.go new file mode 100644 index 00000000..302de4eb --- /dev/null +++ b/internal/service/notification/new_question_email_worker_test.go @@ -0,0 +1,558 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package notification + +import ( + "context" + "reflect" + "runtime" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/apache/answer/internal/schema" +) + +func TestNewQuestionEmailWorkerDelaysBetweenAttempts(t *testing.T) { + timerFactory := newFakeNewQuestionEmailTimerFactory() + sendCh := make(chan newQuestionEmailSendEvent, 2) + worker := newQuestionEmailWorkerWithBuffer( + func() time.Duration { return 3 * time.Second }, + newQuestionEmailSendRecorder(sendCh), + timerFactory.New, + 2, + ) + defer worker.Close() + + if !worker.TryEnqueue(newQuestionEmailWorkerTask("question-1", "user-1", "user-2")) { + t.Fatalf("TryEnqueue() = false, want true") + } + + first := receiveNewQuestionEmailSend(t, sendCh) + if first.userID != "user-1" { + t.Fatalf("first send user = %s, want user-1", first.userID) + } + timer := timerFactory.WaitForTimer(t) + assertNoNewQuestionEmailSend(t, sendCh) + timer.Fire() + + second := receiveNewQuestionEmailSend(t, sendCh) + if second.userID != "user-2" { + t.Fatalf("second send user = %s, want user-2", second.userID) + } + assertUniqueNewQuestionUnsubscribeCodes(t, []string{ + first.rawData.UnsubscribeCode, + second.rawData.UnsubscribeCode, + }) + if got := timerFactory.Durations(); !reflect.DeepEqual(got, []time.Duration{3 * time.Second}) { + t.Fatalf("timer durations = %v, want [3s]", got) + } +} + +func TestNewQuestionEmailWorkerDelayContinuesAcrossTaskBoundaries(t *testing.T) { + timerFactory := newFakeNewQuestionEmailTimerFactory() + sendCh := make(chan newQuestionEmailSendEvent, 2) + worker := newQuestionEmailWorkerWithBuffer( + func() time.Duration { return 5 * time.Second }, + newQuestionEmailSendRecorder(sendCh), + timerFactory.New, + 2, + ) + defer worker.Close() + + if !worker.TryEnqueue(newQuestionEmailWorkerTask("question-1", "user-1")) { + t.Fatalf("TryEnqueue() task 1 = false, want true") + } + first := receiveNewQuestionEmailSend(t, sendCh) + if first.userID != "user-1" { + t.Fatalf("first send user = %s, want user-1", first.userID) + } + + if !worker.TryEnqueue(newQuestionEmailWorkerTask("question-2", "user-2")) { + t.Fatalf("TryEnqueue() task 2 = false, want true") + } + timer := timerFactory.WaitForTimer(t) + assertNoNewQuestionEmailSend(t, sendCh) + timer.Fire() + + second := receiveNewQuestionEmailSend(t, sendCh) + if second.userID != "user-2" { + t.Fatalf("second send user = %s, want user-2", second.userID) + } +} + +func TestNewQuestionEmailWorkerZeroIntervalSendsWithoutTimers(t *testing.T) { + var timerCount int + sendCh := make(chan newQuestionEmailSendEvent, 3) + worker := newQuestionEmailWorkerWithBuffer( + func() time.Duration { return 0 }, + newQuestionEmailSendRecorder(sendCh), + func(time.Duration) newQuestionEmailTimer { + timerCount++ + return newFakeNewQuestionEmailTimer() + }, + 2, + ) + defer worker.Close() + + if !worker.TryEnqueue(newQuestionEmailWorkerTask("question-1", "user-1", "user-2", "user-3")) { + t.Fatalf("TryEnqueue() = false, want true") + } + + gotUsers := []string{ + receiveNewQuestionEmailSend(t, sendCh).userID, + receiveNewQuestionEmailSend(t, sendCh).userID, + receiveNewQuestionEmailSend(t, sendCh).userID, + } + if !reflect.DeepEqual(gotUsers, []string{"user-1", "user-2", "user-3"}) { + t.Fatalf("send users = %v, want [user-1 user-2 user-3]", gotUsers) + } + if timerCount != 0 { + t.Fatalf("timer count = %d, want 0", timerCount) + } +} + +func TestNewQuestionEmailWorkerCloseCancelsPendingWaitAndDropsQueuedTasks(t *testing.T) { + timerFactory := newFakeNewQuestionEmailTimerFactory() + sendCh := make(chan newQuestionEmailSendEvent, 3) + worker := newQuestionEmailWorkerWithBuffer( + func() time.Duration { return time.Hour }, + newQuestionEmailSendRecorder(sendCh), + timerFactory.New, + 2, + ) + + if !worker.TryEnqueue(newQuestionEmailWorkerTask("question-1", "user-1", "user-2")) { + t.Fatalf("TryEnqueue() task 1 = false, want true") + } + if !worker.TryEnqueue(newQuestionEmailWorkerTask("question-2", "user-3")) { + t.Fatalf("TryEnqueue() task 2 = false, want true") + } + + first := receiveNewQuestionEmailSend(t, sendCh) + if first.userID != "user-1" { + t.Fatalf("first send user = %s, want user-1", first.userID) + } + timer := timerFactory.WaitForTimer(t) + + worker.Close() + timer.AssertStopped(t) + assertNoNewQuestionEmailSend(t, sendCh) + if got := len(worker.tasks); got != 0 { + t.Fatalf("pending tasks after Close() = %d, want 0", got) + } + if worker.TryEnqueue(newQuestionEmailWorkerTask("question-3", "user-4")) { + t.Fatalf("TryEnqueue() after Close() = true, want false") + } +} + +func TestNewQuestionEmailWorkerProcessesSerially(t *testing.T) { + entered := make(chan string, 2) + releaseFirst := make(chan struct{}) + worker := newQuestionEmailWorkerWithBuffer( + func() time.Duration { return 0 }, + func(_ context.Context, userID string, _ *schema.NewQuestionTemplateRawData) { + entered <- userID + if userID == "user-1" { + <-releaseFirst + } + }, + nil, + 2, + ) + defer worker.Close() + + if !worker.TryEnqueue(newQuestionEmailWorkerTask("question-1", "user-1", "user-2")) { + t.Fatalf("TryEnqueue() = false, want true") + } + if got := receiveString(t, entered); got != "user-1" { + t.Fatalf("first send user = %s, want user-1", got) + } + assertNoString(t, entered) + + close(releaseFirst) + if got := receiveString(t, entered); got != "user-2" { + t.Fatalf("second send user = %s, want user-2", got) + } +} + +func TestNewQuestionEmailWorkerBuildsFreshRawDataPerAttempt(t *testing.T) { + sendCh := make(chan newQuestionEmailSendEvent, 2) + worker := newQuestionEmailWorkerWithBuffer( + func() time.Duration { return 0 }, + func(_ context.Context, userID string, rawData *schema.NewQuestionTemplateRawData) { + if rawData.QuestionAuthorUserID != "" { + t.Errorf("QuestionAuthorUserID = %q, want empty", rawData.QuestionAuthorUserID) + } + if userID == "user-1" { + rawData.Tags[0] = "mutated" + rawData.TagIDs[0] = "mutated" + } + sendCh <- newQuestionEmailSendEvent{userID: userID, rawData: rawData} + }, + nil, + 2, + ) + defer worker.Close() + + if !worker.TryEnqueue(newQuestionEmailTask{ + UserIDs: []string{"user-1", "user-2"}, + QuestionTitle: "Question", + QuestionID: "question-1", + Tags: []string{"go"}, + TagIDs: []string{"tag-1"}, + }) { + t.Fatalf("TryEnqueue() = false, want true") + } + + first := receiveNewQuestionEmailSend(t, sendCh) + second := receiveNewQuestionEmailSend(t, sendCh) + if first.rawData.UnsubscribeCode == "" || second.rawData.UnsubscribeCode == "" { + t.Fatalf("unsubscribe codes must be non-empty: %q %q", + first.rawData.UnsubscribeCode, second.rawData.UnsubscribeCode) + } + if first.rawData.UnsubscribeCode == second.rawData.UnsubscribeCode { + t.Fatalf("unsubscribe codes must be unique, both were %q", first.rawData.UnsubscribeCode) + } + if !reflect.DeepEqual(second.rawData.Tags, []string{"go"}) || + !reflect.DeepEqual(second.rawData.TagIDs, []string{"tag-1"}) { + t.Fatalf("second raw data tags = %v/%v, want original values", + second.rawData.Tags, second.rawData.TagIDs) + } +} + +func TestNewQuestionEmailWorkerTryEnqueueCopiesTaskAndFailsFast(t *testing.T) { + worker := newUnstartedNewQuestionEmailWorkerForTest(1) + task := newQuestionEmailTask{ + UserIDs: []string{"user-1"}, + QuestionTitle: "Question", + QuestionID: "question-1", + Tags: []string{"go"}, + TagIDs: []string{"tag-1"}, + } + if !worker.TryEnqueue(task) { + t.Fatalf("TryEnqueue() = false, want true") + } + task.UserIDs[0] = "mutated-user" + task.Tags[0] = "mutated-tag" + task.TagIDs[0] = "mutated-tag-id" + + queuedTask := <-worker.tasks + if !reflect.DeepEqual(queuedTask.UserIDs, []string{"user-1"}) || + !reflect.DeepEqual(queuedTask.Tags, []string{"go"}) || + !reflect.DeepEqual(queuedTask.TagIDs, []string{"tag-1"}) { + t.Fatalf("queued task was mutated: %+v", queuedTask) + } + + if !worker.TryEnqueue(newQuestionEmailWorkerTask("question-2", "user-2")) { + t.Fatalf("TryEnqueue() refill = false, want true") + } + if worker.TryEnqueue(newQuestionEmailWorkerTask("question-3", "user-3")) { + t.Fatalf("TryEnqueue() with full queue = true, want false") + } + + worker.Close() + if worker.TryEnqueue(newQuestionEmailWorkerTask("question-4", "user-4")) { + t.Fatalf("TryEnqueue() after Close() = true, want false") + } + + canceledWorker := newUnstartedNewQuestionEmailWorkerForTest(1) + canceledWorker.cancel() + if canceledWorker.TryEnqueue(newQuestionEmailWorkerTask("question-5", "user-5")) { + t.Fatalf("TryEnqueue() after cancel = true, want false") + } +} + +func TestNewQuestionEmailWorkerTryEnqueueConcurrentClose(t *testing.T) { + const ( + iterations = 100 + senders = 32 + ) + + for iteration := 0; iteration < iterations; iteration++ { + worker := newUnstartedNewQuestionEmailWorkerForTest(1) + if !worker.TryEnqueue(newQuestionEmailWorkerTask("already-queued", "queued-user")) { + t.Fatalf("iteration %d: pre-fill TryEnqueue() = false, want true", iteration) + } + + start := make(chan struct{}) + ready := make(chan struct{}, senders) + panicCh := make(chan any, senders) + var closeObserved atomic.Bool + var acceptedAfterCloseObserved atomic.Int64 + var wg sync.WaitGroup + + for sender := 0; sender < senders; sender++ { + wg.Add(1) + go func(sender int) { + defer wg.Done() + defer func() { + if recovered := recover(); recovered != nil { + panicCh <- recovered + } + }() + + ready <- struct{}{} + <-start + for { + accepted := worker.TryEnqueue(newQuestionEmailWorkerTask("question", "user")) + if accepted && closeObserved.Load() { + acceptedAfterCloseObserved.Add(1) + } + if closeObserved.Load() { + return + } + runtime.Gosched() + } + }(sender) + } + for sender := 0; sender < senders; sender++ { + <-ready + } + + closeDoneObserved := make(chan struct{}) + go func() { + <-worker.ctx.Done() + closeObserved.Store(true) + close(closeDoneObserved) + }() + + close(start) + runtime.Gosched() + + closeDone := make(chan struct{}) + go func() { + worker.Close() + close(closeDone) + }() + + select { + case <-closeDone: + case <-time.After(time.Second): + t.Fatalf("iteration %d: Close() did not return", iteration) + } + select { + case <-closeDoneObserved: + case <-time.After(time.Second): + t.Fatalf("iteration %d: close was not observed", iteration) + } + + wgDone := make(chan struct{}) + go func() { + wg.Wait() + close(wgDone) + }() + select { + case <-wgDone: + case <-time.After(time.Second): + t.Fatalf("iteration %d: TryEnqueue goroutines did not return", iteration) + } + + select { + case recovered := <-panicCh: + t.Fatalf("iteration %d: TryEnqueue panicked during Close(): %v", iteration, recovered) + default: + } + if got := acceptedAfterCloseObserved.Load(); got != 0 { + t.Fatalf("iteration %d: accepted %d enqueue attempts after close was observed, want 0", + iteration, got) + } + if worker.TryEnqueue(newQuestionEmailWorkerTask("after-close", "user")) { + t.Fatalf("iteration %d: TryEnqueue() after Close() = true, want false", iteration) + } + if got := len(worker.tasks); got != 0 { + t.Fatalf("iteration %d: pending tasks after Close() = %d, want 0", iteration, got) + } + } +} + +func TestWaitNewQuestionEmailIntervalCancel(t *testing.T) { + timerFactory := newFakeNewQuestionEmailTimerFactory() + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan bool, 1) + go func() { + done <- waitNewQuestionEmailInterval(ctx, time.Minute, timerFactory.New) + }() + + timer := timerFactory.WaitForTimer(t) + cancel() + + select { + case got := <-done: + if got { + t.Fatalf("waitNewQuestionEmailInterval() = true, want false") + } + case <-time.After(time.Second): + t.Fatalf("waitNewQuestionEmailInterval() did not return after cancellation") + } + timer.AssertStopped(t) +} + +type newQuestionEmailSendEvent struct { + userID string + rawData *schema.NewQuestionTemplateRawData +} + +func newQuestionEmailSendRecorder(sendCh chan<- newQuestionEmailSendEvent) newQuestionNotificationEmailSender { + return func(_ context.Context, userID string, rawData *schema.NewQuestionTemplateRawData) { + sendCh <- newQuestionEmailSendEvent{userID: userID, rawData: rawData} + } +} + +func newQuestionEmailWorkerTask(questionID string, userIDs ...string) newQuestionEmailTask { + return newQuestionEmailTask{ + UserIDs: userIDs, + QuestionTitle: "Question", + QuestionID: questionID, + Tags: []string{"go"}, + TagIDs: []string{"tag-1"}, + } +} + +func newUnstartedNewQuestionEmailWorkerForTest(bufferSize int) *newQuestionEmailWorker { + ctx, cancel := context.WithCancel(context.Background()) + return &newQuestionEmailWorker{ + tasks: make(chan newQuestionEmailTask, bufferSize), + interval: func() time.Duration { return 0 }, + timerFactory: newRealNewQuestionEmailTimer, + ctx: ctx, + cancel: cancel, + } +} + +func receiveNewQuestionEmailSend(t *testing.T, sendCh <-chan newQuestionEmailSendEvent) newQuestionEmailSendEvent { + t.Helper() + select { + case event := <-sendCh: + return event + case <-time.After(time.Second): + t.Fatalf("timed out waiting for new question email send") + return newQuestionEmailSendEvent{} + } +} + +func assertNoNewQuestionEmailSend(t *testing.T, sendCh <-chan newQuestionEmailSendEvent) { + t.Helper() + select { + case event := <-sendCh: + t.Fatalf("unexpected new question email send: %+v", event) + default: + } +} + +func receiveString(t *testing.T, ch <-chan string) string { + t.Helper() + select { + case value := <-ch: + return value + case <-time.After(time.Second): + t.Fatalf("timed out waiting for string") + return "" + } +} + +func assertNoString(t *testing.T, ch <-chan string) { + t.Helper() + select { + case value := <-ch: + t.Fatalf("unexpected string: %s", value) + default: + } +} + +type fakeNewQuestionEmailTimerFactory struct { + timers chan *fakeNewQuestionEmailTimer + mu sync.Mutex + durations []time.Duration +} + +func newFakeNewQuestionEmailTimerFactory() *fakeNewQuestionEmailTimerFactory { + return &fakeNewQuestionEmailTimerFactory{ + timers: make(chan *fakeNewQuestionEmailTimer, 16), + } +} + +func (f *fakeNewQuestionEmailTimerFactory) New(duration time.Duration) newQuestionEmailTimer { + timer := newFakeNewQuestionEmailTimer() + + f.mu.Lock() + f.durations = append(f.durations, duration) + f.mu.Unlock() + + f.timers <- timer + return timer +} + +func (f *fakeNewQuestionEmailTimerFactory) WaitForTimer(t *testing.T) *fakeNewQuestionEmailTimer { + t.Helper() + select { + case timer := <-f.timers: + return timer + case <-time.After(time.Second): + t.Fatalf("timed out waiting for timer") + return nil + } +} + +func (f *fakeNewQuestionEmailTimerFactory) Durations() []time.Duration { + f.mu.Lock() + defer f.mu.Unlock() + + durations := make([]time.Duration, len(f.durations)) + copy(durations, f.durations) + return durations +} + +type fakeNewQuestionEmailTimer struct { + ch chan time.Time + stopped chan struct{} + once sync.Once +} + +func newFakeNewQuestionEmailTimer() *fakeNewQuestionEmailTimer { + return &fakeNewQuestionEmailTimer{ + ch: make(chan time.Time, 1), + stopped: make(chan struct{}), + } +} + +func (t *fakeNewQuestionEmailTimer) C() <-chan time.Time { + return t.ch +} + +func (t *fakeNewQuestionEmailTimer) Stop() { + t.once.Do(func() { + close(t.stopped) + }) +} + +func (t *fakeNewQuestionEmailTimer) Fire() { + t.ch <- time.Now() +} + +func (t *fakeNewQuestionEmailTimer) AssertStopped(tb testing.TB) { + tb.Helper() + select { + case <-t.stopped: + case <-time.After(time.Second): + tb.Fatalf("timer was not stopped") + } +} diff --git a/internal/service/notification/new_question_notification.go b/internal/service/notification/new_question_notification.go index bd323c7f..43c5ff85 100644 --- a/internal/service/notification/new_question_notification.go +++ b/internal/service/notification/new_question_notification.go @@ -49,32 +49,42 @@ func (ns *ExternalNotificationService) handleNewQuestionNotification(ctx context } log.Debugf("get subscribers %d for question %s", len(subscribers), msg.NewQuestionTemplateRawData.QuestionID) - interval := newQuestionNotificationEmailSendInterval() - if interval > 0 { - ns.syncNewQuestionNotificationToPlugin(ctx, msg) - ns.sendNewQuestionNotificationEmails(ctx, subscribers, msg.NewQuestionTemplateRawData, interval) - return nil - } - - ns.sendNewQuestionNotificationEmails(ctx, subscribers, msg.NewQuestionTemplateRawData, interval) ns.syncNewQuestionNotificationToPlugin(ctx, msg) + ns.enqueueNewQuestionNotificationEmails(subscribers, msg.NewQuestionTemplateRawData) return nil } -func (ns *ExternalNotificationService) sendNewQuestionNotificationEmails( - ctx context.Context, +func (ns *ExternalNotificationService) enqueueNewQuestionNotificationEmails( subscribers []*NewQuestionSubscriber, rawData *schema.NewQuestionTemplateRawData, - interval time.Duration, ) { - sendNewQuestionNotificationEmailsWithInterval( - ctx, - subscribers, - rawData, - interval, - nil, - ns.sendNewQuestionNotificationEmail, - ) + task := newQuestionEmailTaskFromRawData(collectNewQuestionNotificationEmailUserIDs(subscribers), rawData) + if len(task.UserIDs) == 0 { + return + } + if ns.newQuestionEmailWorker == nil { + log.Warnf("[new_question_email] worker is nil, dropping task for question %s", task.QuestionID) + return + } + if !ns.newQuestionEmailWorker.TryEnqueue(task) { + log.Warnf("[new_question_email] failed to enqueue task for question %s", task.QuestionID) + } +} + +func collectNewQuestionNotificationEmailUserIDs(subscribers []*NewQuestionSubscriber) []string { + userIDs := make([]string, 0, len(subscribers)) + for _, subscriber := range subscribers { + if subscriber == nil { + continue + } + for _, channel := range subscriber.Channels { + if channel == nil || !channel.Enable || channel.Key != constant.EmailChannel { + continue + } + userIDs = append(userIDs, subscriber.UserID) + } + } + return userIDs } func (ns *ExternalNotificationService) getNewQuestionSubscribers(ctx context.Context, msg *schema.ExternalNotificationMsg) ( diff --git a/internal/service/notification/new_question_notification_interval.go b/internal/service/notification/new_question_notification_interval.go index 22f17450..19bb1cea 100644 --- a/internal/service/notification/new_question_notification_interval.go +++ b/internal/service/notification/new_question_notification_interval.go @@ -26,16 +26,14 @@ import ( "strings" "time" - "github.com/apache/answer/internal/base/constant" "github.com/apache/answer/internal/schema" - "github.com/apache/answer/pkg/token" ) const newQuestionNotificationEmailSendIntervalEnv = "NEW_QUESTION_NOTIFICATION_EMAIL_SEND_INTERVAL_SECONDS" -const maxNewQuestionNotificationEmailSendIntervalSeconds = int64(1<<63-1) / int64(time.Second) +const maxNewQuestionNotificationEmailSendInterval = 5 * time.Minute -type newQuestionNotificationEmailSleeper func(time.Duration) +const maxNewQuestionNotificationEmailSendIntervalSeconds = int64(maxNewQuestionNotificationEmailSendInterval / time.Second) type newQuestionNotificationEmailSender func(context.Context, string, *schema.NewQuestionTemplateRawData) @@ -49,44 +47,11 @@ func parseNewQuestionNotificationEmailSendInterval(value string) time.Duration { return 0 } seconds, err := strconv.ParseInt(value, 10, 64) - if err != nil || seconds < 0 || seconds > maxNewQuestionNotificationEmailSendIntervalSeconds { + if err != nil || seconds < 0 { return 0 } + if seconds > maxNewQuestionNotificationEmailSendIntervalSeconds { + return maxNewQuestionNotificationEmailSendInterval + } return time.Duration(seconds) * time.Second } - -func sendNewQuestionNotificationEmailsWithInterval( - ctx context.Context, - subscribers []*NewQuestionSubscriber, - rawData *schema.NewQuestionTemplateRawData, - interval time.Duration, - sleep newQuestionNotificationEmailSleeper, - send newQuestionNotificationEmailSender, -) { - if rawData == nil || send == nil { - return - } - if sleep == nil { - sleep = time.Sleep - } - - emailAttempts := 0 - for _, subscriber := range subscribers { - for _, channel := range subscriber.Channels { - if !channel.Enable || channel.Key != constant.EmailChannel { - continue - } - if interval > 0 && emailAttempts > 0 { - sleep(interval) - } - send(ctx, subscriber.UserID, &schema.NewQuestionTemplateRawData{ - QuestionTitle: rawData.QuestionTitle, - QuestionID: rawData.QuestionID, - UnsubscribeCode: token.GenerateToken(), - Tags: rawData.Tags, - TagIDs: rawData.TagIDs, - }) - emailAttempts++ - } - } -} diff --git a/internal/service/notification/new_question_notification_test.go b/internal/service/notification/new_question_notification_test.go index 4d1729a2..e7db3814 100644 --- a/internal/service/notification/new_question_notification_test.go +++ b/internal/service/notification/new_question_notification_test.go @@ -24,6 +24,7 @@ import ( "encoding/json" "os" "reflect" + "sync" "testing" "time" @@ -34,6 +35,7 @@ import ( "github.com/apache/answer/internal/service/config" "github.com/apache/answer/internal/service/export" "github.com/apache/answer/internal/service/mock" + "github.com/apache/answer/plugin" "go.uber.org/mock/gomock" ) @@ -79,11 +81,23 @@ func TestNewQuestionNotificationEmailSendInterval(t *testing.T) { want: 0, }, { - name: "duration overflow", - value: "9223372037", + name: "whitespace", + value: " ", set: true, want: 0, }, + { + name: "above max clamps to max", + value: "301", + set: true, + want: maxNewQuestionNotificationEmailSendInterval, + }, + { + name: "duration overflow clamps to max", + value: "9223372037", + set: true, + want: maxNewQuestionNotificationEmailSendInterval, + }, { name: "parse int overflow", value: "9223372036854775808", @@ -104,145 +118,7 @@ func TestNewQuestionNotificationEmailSendInterval(t *testing.T) { } } -func TestSendNewQuestionNotificationEmailsWithInterval(t *testing.T) { - rawData := &schema.NewQuestionTemplateRawData{ - QuestionTitle: "question", - QuestionID: "1", - Tags: []string{"go"}, - TagIDs: []string{"tag-1"}, - } - interval := 3 * time.Second - - tests := []struct { - name string - interval time.Duration - subscribers []*NewQuestionSubscriber - wantSends []string - wantSleeps []time.Duration - wantEvents []string - }{ - { - name: "interval 0", - interval: 0, - subscribers: []*NewQuestionSubscriber{ - newQuestionSubscriber("user-1", newQuestionEmailChannel(true)), - newQuestionSubscriber("user-2", newQuestionEmailChannel(true)), - }, - wantSends: []string{"user-1", "user-2"}, - wantEvents: []string{ - "send:user-1", - "send:user-2", - }, - }, - { - name: "0 enabled email attempts", - interval: interval, - subscribers: []*NewQuestionSubscriber{ - newQuestionSubscriber("user-1", newQuestionEmailChannel(false)), - newQuestionSubscriber("user-2", newQuestionNonEmailChannel(true)), - }, - }, - { - name: "1 enabled email attempt", - interval: interval, - subscribers: []*NewQuestionSubscriber{ - newQuestionSubscriber("user-1", newQuestionEmailChannel(true)), - }, - wantSends: []string{"user-1"}, - wantEvents: []string{ - "send:user-1", - }, - }, - { - name: "N enabled email attempts", - interval: interval, - subscribers: []*NewQuestionSubscriber{ - newQuestionSubscriber("user-1", newQuestionEmailChannel(true)), - newQuestionSubscriber("user-2", newQuestionEmailChannel(true)), - newQuestionSubscriber("user-3", newQuestionEmailChannel(true)), - }, - wantSends: []string{"user-1", "user-2", "user-3"}, - wantSleeps: []time.Duration{interval, interval}, - wantEvents: []string{ - "send:user-1", - "sleep:3s", - "send:user-2", - "sleep:3s", - "send:user-3", - }, - }, - { - name: "disabled email channel does not add delay", - interval: interval, - subscribers: []*NewQuestionSubscriber{ - newQuestionSubscriber("user-1", newQuestionEmailChannel(true)), - newQuestionSubscriber("user-2", newQuestionEmailChannel(false)), - newQuestionSubscriber("user-3", newQuestionEmailChannel(true)), - }, - wantSends: []string{"user-1", "user-3"}, - wantSleeps: []time.Duration{interval}, - wantEvents: []string{ - "send:user-1", - "sleep:3s", - "send:user-3", - }, - }, - { - name: "non-email channel does not add delay", - interval: interval, - subscribers: []*NewQuestionSubscriber{ - newQuestionSubscriber("user-1", newQuestionEmailChannel(true)), - newQuestionSubscriber("user-2", newQuestionNonEmailChannel(true)), - newQuestionSubscriber("user-3", newQuestionEmailChannel(true)), - }, - wantSends: []string{"user-1", "user-3"}, - wantSleeps: []time.Duration{interval}, - wantEvents: []string{ - "send:user-1", - "sleep:3s", - "send:user-3", - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - var gotEvents []string - var gotSleeps []time.Duration - sleep := func(duration time.Duration) { - gotSleeps = append(gotSleeps, duration) - gotEvents = append(gotEvents, "sleep:"+duration.String()) - } - - var gotSends []string - var gotCodes []string - send := func(_ context.Context, userID string, rawData *schema.NewQuestionTemplateRawData) { - gotSends = append(gotSends, userID) - gotEvents = append(gotEvents, "send:"+userID) - if rawData.UnsubscribeCode == "" { - t.Fatalf("expected unsubscribe code for %s", userID) - } - gotCodes = append(gotCodes, rawData.UnsubscribeCode) - } - - sendNewQuestionNotificationEmailsWithInterval( - context.Background(), tt.subscribers, rawData, tt.interval, sleep, send) - - if !reflect.DeepEqual(gotSends, tt.wantSends) { - t.Fatalf("send calls = %v, want %v", gotSends, tt.wantSends) - } - if !reflect.DeepEqual(gotSleeps, tt.wantSleeps) { - t.Fatalf("sleep calls = %v, want %v", gotSleeps, tt.wantSleeps) - } - if !reflect.DeepEqual(gotEvents, tt.wantEvents) { - t.Fatalf("events = %v, want %v", gotEvents, tt.wantEvents) - } - assertUniqueNewQuestionUnsubscribeCodes(t, gotCodes) - }) - } -} - -func TestHandleNewQuestionNotificationSendsEmailsThroughFanOut(t *testing.T) { +func TestHandleNewQuestionNotificationEnqueuesEmailTask(t *testing.T) { setNewQuestionNotificationEmailSendIntervalEnv(t, "0", true) cache, cleanup, err := basedata.NewCache(&basedata.CacheConf{}) @@ -305,6 +181,7 @@ func TestHandleNewQuestionNotificationSendsEmailsThroughFanOut(t *testing.T) { }, siteInfoService: siteInfoService, } + service.newQuestionEmailWorker = newUnstartedNewQuestionEmailWorkerForTest(1) err = service.handleNewQuestionNotification(context.Background(), &schema.ExternalNotificationMsg{ NewQuestionTemplateRawData: &schema.NewQuestionTemplateRawData{ @@ -319,19 +196,201 @@ func TestHandleNewQuestionNotificationSendsEmailsThroughFanOut(t *testing.T) { t.Fatalf("handleNewQuestionNotification() error = %v", err) } - wantUsers := []string{"all-user", "dup-user", "tag-user"} - assertStringSet(t, emailRepo.userIDs(), wantUsers) - for _, userID := range wantUsers { - codes := emailRepo.codesByUserID[userID] - if len(codes) != 1 { - t.Fatalf("saved codes for %s = %v, want exactly one code", userID, codes) - } - if codes[0] == "" { - t.Fatalf("saved empty code for %s", userID) - } + var task newQuestionEmailTask + select { + case task = <-service.newQuestionEmailWorker.tasks: + default: + t.Fatalf("expected enqueued new question email task") } - if codes := emailRepo.codesByUserID["author"]; len(codes) > 0 { - t.Fatalf("question author received notification codes: %v", codes) + + wantUsers := []string{"all-user", "dup-user", "tag-user"} + assertStringSet(t, task.UserIDs, wantUsers) + if task.QuestionTitle != "New question" || task.QuestionID != "1" { + t.Fatalf("task question data = %+v", task) + } + if !reflect.DeepEqual(task.Tags, []string{"go"}) || !reflect.DeepEqual(task.TagIDs, []string{"tag-1"}) { + t.Fatalf("task tags = %v/%v", task.Tags, task.TagIDs) + } + if len(emailRepo.codesByUserID) > 0 { + t.Fatalf("handler sent emails synchronously: %v", emailRepo.codesByUserID) + } +} + +func TestHandleNewQuestionNotificationSkipsEnqueueWithoutEnabledEmailAttempts(t *testing.T) { + cache, cleanup, err := basedata.NewCache(&basedata.CacheConf{}) + if err != nil { + t.Fatalf("new cache: %v", err) + } + t.Cleanup(cleanup) + + service := &ExternalNotificationService{ + data: &basedata.Data{Cache: cache}, + userNotificationConfigRepo: &newQuestionNotificationTestUserNotificationConfigRepo{ + followedTagConfigs: map[string]*entity.UserNotificationConfig{ + "tag-user": newQuestionNotificationConfig( + "tag-user", constant.AllNewQuestionForFollowingTagsSource, false), + }, + allQuestionConfigs: []*entity.UserNotificationConfig{ + newQuestionNotificationConfig("all-user", constant.AllNewQuestionSource, false), + }, + }, + followRepo: &newQuestionNotificationTestFollowRepo{ + followersByObjectID: map[string][]string{"tag-1": {"tag-user"}}, + }, + userRepo: &newQuestionNotificationTestUserRepo{ + users: map[string]*entity.User{ + "tag-user": newQuestionNotificationTestUser("tag-user"), + "all-user": newQuestionNotificationTestUser("all-user"), + }, + }, + newQuestionEmailWorker: newUnstartedNewQuestionEmailWorkerForTest(1), + } + + err = service.handleNewQuestionNotification(context.Background(), &schema.ExternalNotificationMsg{ + NewQuestionTemplateRawData: &schema.NewQuestionTemplateRawData{ + QuestionTitle: "New question", + QuestionID: "1", + Tags: []string{"go"}, + TagIDs: []string{"tag-1"}, + }, + }) + if err != nil { + t.Fatalf("handleNewQuestionNotification() error = %v", err) + } + select { + case task := <-service.newQuestionEmailWorker.tasks: + t.Fatalf("unexpected enqueued task: %+v", task) + default: + } +} + +func TestHandleNewQuestionNotificationReturnsWhenEmailWorkerQueueFull(t *testing.T) { + cache, cleanup, err := basedata.NewCache(&basedata.CacheConf{}) + if err != nil { + t.Fatalf("new cache: %v", err) + } + t.Cleanup(cleanup) + + worker := newUnstartedNewQuestionEmailWorkerForTest(1) + if !worker.TryEnqueue(newQuestionEmailWorkerTask("already-queued", "queued-user")) { + t.Fatalf("pre-fill TryEnqueue() = false, want true") + } + service := &ExternalNotificationService{ + data: &basedata.Data{Cache: cache}, + userNotificationConfigRepo: &newQuestionNotificationTestUserNotificationConfigRepo{ + allQuestionConfigs: []*entity.UserNotificationConfig{ + newQuestionNotificationConfig("all-user", constant.AllNewQuestionSource, true), + }, + }, + followRepo: &newQuestionNotificationTestFollowRepo{ + followersByObjectID: map[string][]string{}, + }, + userRepo: &newQuestionNotificationTestUserRepo{ + users: map[string]*entity.User{ + "all-user": newQuestionNotificationTestUser("all-user"), + }, + }, + newQuestionEmailWorker: worker, + } + + err = service.handleNewQuestionNotification(context.Background(), &schema.ExternalNotificationMsg{ + NewQuestionTemplateRawData: &schema.NewQuestionTemplateRawData{ + QuestionTitle: "New question", + QuestionID: "1", + }, + }) + if err != nil { + t.Fatalf("handleNewQuestionNotification() error = %v", err) + } + if got := len(worker.tasks); got != 1 { + t.Fatalf("worker queue length = %d, want 1", got) + } +} + +func TestHandleNewQuestionNotificationSyncsPluginBeforeEmailEnqueue(t *testing.T) { + cache, cleanup, err := basedata.NewCache(&basedata.CacheConf{}) + if err != nil { + t.Fatalf("new cache: %v", err) + } + t.Cleanup(cleanup) + + ctrl := gomock.NewController(t) + siteInfoService := mock.NewMockSiteInfoCommonService(ctrl) + siteInfoService.EXPECT().GetSiteGeneral(gomock.Any()).Return(&schema.SiteGeneralResp{ + Name: "Answer", + SiteUrl: "https://answer.test", + ContactEmail: "support@answer.test", + }, nil).AnyTimes() + siteInfoService.EXPECT().GetSiteSeo(gomock.Any()).Return(&schema.SiteSeoResp{ + Permalink: constant.PermalinkQuestionIDAndTitle, + }, nil).AnyTimes() + siteInfoService.EXPECT().GetSiteInterface(gomock.Any()).Return(&schema.SiteInterfaceSettingsResp{ + Language: "en", + }, nil).AnyTimes() + + notifyStarted := make(chan plugin.NotificationMessage, 1) + releaseNotify := make(chan struct{}) + enableNewQuestionNotificationTestPlugin(t, notifyStarted, releaseNotify) + + worker := newUnstartedNewQuestionEmailWorkerForTest(1) + service := &ExternalNotificationService{ + data: &basedata.Data{Cache: cache}, + userNotificationConfigRepo: &newQuestionNotificationTestUserNotificationConfigRepo{ + followedTagConfigs: map[string]*entity.UserNotificationConfig{ + "tag-user": newQuestionNotificationConfig( + "tag-user", constant.AllNewQuestionForFollowingTagsSource, true), + }, + }, + followRepo: &newQuestionNotificationTestFollowRepo{ + followersByObjectID: map[string][]string{"tag-1": {"tag-user"}}, + }, + userRepo: &newQuestionNotificationTestUserRepo{ + users: map[string]*entity.User{ + "tag-user": newQuestionNotificationTestUser("tag-user"), + }, + }, + userExternalLoginRepo: newQuestionNotificationTestUserExternalLoginRepo{}, + siteInfoService: siteInfoService, + newQuestionEmailWorker: worker, + } + + errCh := make(chan error, 1) + go func() { + errCh <- service.handleNewQuestionNotification(context.Background(), &schema.ExternalNotificationMsg{ + NewQuestionTemplateRawData: &schema.NewQuestionTemplateRawData{ + QuestionTitle: "New question", + QuestionID: "1", + Tags: []string{"go"}, + TagIDs: []string{"tag-1"}, + }, + }) + }() + + select { + case <-notifyStarted: + case <-time.After(time.Second): + t.Fatalf("plugin notification was not sent") + } + select { + case task := <-worker.tasks: + t.Fatalf("email task enqueued before plugin sync completed: %+v", task) + default: + } + close(releaseNotify) + + select { + case err := <-errCh: + if err != nil { + t.Fatalf("handleNewQuestionNotification() error = %v", err) + } + case <-time.After(time.Second): + t.Fatalf("handleNewQuestionNotification() did not return") + } + select { + case task := <-worker.tasks: + assertStringSet(t, task.UserIDs, []string{"tag-user"}) + default: + t.Fatalf("expected email task after plugin sync completed") } } @@ -650,3 +709,114 @@ func (r *newQuestionNotificationTestEmailRepo) userIDs() []string { } return userIDs } + +var ( + newQuestionNotificationTestPluginOnce sync.Once + newQuestionNotificationTestPluginInst = &newQuestionNotificationTestPlugin{} +) + +func enableNewQuestionNotificationTestPlugin( + t *testing.T, + notifyStarted chan plugin.NotificationMessage, + releaseNotify <-chan struct{}, +) { + t.Helper() + + newQuestionNotificationTestPluginInst.setChannels(notifyStarted, releaseNotify) + newQuestionNotificationTestPluginOnce.Do(func() { + plugin.Register(newQuestionNotificationTestPluginInst) + }) + plugin.StatusManager.Enable(newQuestionNotificationTestPluginInst.Info().SlugName, true) + t.Cleanup(func() { + plugin.StatusManager.Enable(newQuestionNotificationTestPluginInst.Info().SlugName, false) + newQuestionNotificationTestPluginInst.setChannels(nil, nil) + }) +} + +type newQuestionNotificationTestPlugin struct { + mu sync.Mutex + notifyStarted chan plugin.NotificationMessage + releaseNotify <-chan struct{} +} + +func (p *newQuestionNotificationTestPlugin) Info() plugin.Info { + return plugin.Info{SlugName: "new-question-notification-test-plugin"} +} + +func (p *newQuestionNotificationTestPlugin) GetNewQuestionSubscribers() []string { + return nil +} + +func (p *newQuestionNotificationTestPlugin) Notify(msg plugin.NotificationMessage) { + p.mu.Lock() + notifyStarted := p.notifyStarted + releaseNotify := p.releaseNotify + p.mu.Unlock() + + if notifyStarted != nil { + select { + case notifyStarted <- msg: + default: + } + } + if releaseNotify != nil { + <-releaseNotify + } +} + +func (p *newQuestionNotificationTestPlugin) setChannels( + notifyStarted chan plugin.NotificationMessage, + releaseNotify <-chan struct{}, +) { + p.mu.Lock() + defer p.mu.Unlock() + p.notifyStarted = notifyStarted + p.releaseNotify = releaseNotify +} + +type newQuestionNotificationTestUserExternalLoginRepo struct{} + +func (newQuestionNotificationTestUserExternalLoginRepo) AddUserExternalLogin( + context.Context, *entity.UserExternalLogin) error { + return nil +} + +func (newQuestionNotificationTestUserExternalLoginRepo) UpdateInfo( + context.Context, *entity.UserExternalLogin) error { + return nil +} + +func (newQuestionNotificationTestUserExternalLoginRepo) GetByExternalID( + context.Context, string, string) (*entity.UserExternalLogin, bool, error) { + return nil, false, nil +} + +func (newQuestionNotificationTestUserExternalLoginRepo) GetByUserID( + context.Context, string, string) (*entity.UserExternalLogin, bool, error) { + return nil, false, nil +} + +func (newQuestionNotificationTestUserExternalLoginRepo) GetUserExternalLoginList( + context.Context, string) ([]*entity.UserExternalLogin, error) { + return nil, nil +} + +func (newQuestionNotificationTestUserExternalLoginRepo) DeleteUserExternalLogin( + context.Context, string, string) error { + return nil +} + +func (newQuestionNotificationTestUserExternalLoginRepo) DeleteUserExternalLoginByUserID( + context.Context, string) error { + return nil +} + +func (newQuestionNotificationTestUserExternalLoginRepo) SetCacheUserExternalLoginInfo( + context.Context, string, *schema.ExternalLoginUserInfoCache) error { + return nil +} + +func (newQuestionNotificationTestUserExternalLoginRepo) GetCacheUserExternalLoginInfo( + context.Context, string) (*schema.ExternalLoginUserInfoCache, error) { + return nil, nil +} From e06311a51403224d9b9a7aace4d960c7960f6986 Mon Sep 17 00:00:00 2001 From: Artur Iusupov Date: Wed, 1 Jul 2026 13:30:44 +0400 Subject: [PATCH 16/27] fix(notification): make new question email queue configurable --- .../notification/new_question_email_worker.go | 30 +++- .../new_question_email_worker_test.go | 128 ++++++++++++++++++ 2 files changed, 156 insertions(+), 2 deletions(-) diff --git a/internal/service/notification/new_question_email_worker.go b/internal/service/notification/new_question_email_worker.go index be3e6504..c3df1bee 100644 --- a/internal/service/notification/new_question_email_worker.go +++ b/internal/service/notification/new_question_email_worker.go @@ -21,6 +21,9 @@ package notification import ( "context" + "os" + "strconv" + "strings" "sync" "time" @@ -29,7 +32,11 @@ import ( "github.com/segmentfault/pacman/log" ) -const newQuestionEmailWorkerQueueSize = 128 +const defaultNewQuestionEmailWorkerQueueSize = 1024 + +const maxNewQuestionEmailWorkerQueueSize = 65536 + +const newQuestionEmailWorkerQueueSizeEnv = "NEW_QUESTION_NOTIFICATION_EMAIL_QUEUE_SIZE" type newQuestionEmailTask struct { UserIDs []string @@ -68,10 +75,29 @@ func newQuestionEmailWorkerWithDefaults( interval, send, newRealNewQuestionEmailTimer, - newQuestionEmailWorkerQueueSize, + newQuestionEmailWorkerQueueSize(), ) } +func newQuestionEmailWorkerQueueSize() int { + return parseNewQuestionEmailWorkerQueueSize(os.Getenv(newQuestionEmailWorkerQueueSizeEnv)) +} + +func parseNewQuestionEmailWorkerQueueSize(value string) int { + value = strings.TrimSpace(value) + if len(value) == 0 { + return defaultNewQuestionEmailWorkerQueueSize + } + queueSize, err := strconv.ParseInt(value, 10, 64) + if err != nil || queueSize <= 0 { + return defaultNewQuestionEmailWorkerQueueSize + } + if queueSize > int64(maxNewQuestionEmailWorkerQueueSize) { + return maxNewQuestionEmailWorkerQueueSize + } + return int(queueSize) +} + func newQuestionEmailWorkerWithBuffer( interval newQuestionEmailIntervalProvider, send newQuestionNotificationEmailSender, diff --git a/internal/service/notification/new_question_email_worker_test.go b/internal/service/notification/new_question_email_worker_test.go index 302de4eb..e7ea4880 100644 --- a/internal/service/notification/new_question_email_worker_test.go +++ b/internal/service/notification/new_question_email_worker_test.go @@ -21,6 +21,7 @@ package notification import ( "context" + "os" "reflect" "runtime" "sync" @@ -31,6 +32,111 @@ import ( "github.com/apache/answer/internal/schema" ) +func TestParseNewQuestionEmailWorkerQueueSize(t *testing.T) { + tests := []struct { + name string + value string + want int + }{ + { + name: "empty", + value: "", + want: defaultNewQuestionEmailWorkerQueueSize, + }, + { + name: "whitespace", + value: " ", + want: defaultNewQuestionEmailWorkerQueueSize, + }, + { + name: "invalid", + value: "invalid", + want: defaultNewQuestionEmailWorkerQueueSize, + }, + { + name: "zero", + value: "0", + want: defaultNewQuestionEmailWorkerQueueSize, + }, + { + name: "negative", + value: "-1", + want: defaultNewQuestionEmailWorkerQueueSize, + }, + { + name: "parse int overflow", + value: "9223372036854775808", + want: defaultNewQuestionEmailWorkerQueueSize, + }, + { + name: "positive", + value: "2048", + want: 2048, + }, + { + name: "max", + value: "65536", + want: maxNewQuestionEmailWorkerQueueSize, + }, + { + name: "above max", + value: "65537", + want: maxNewQuestionEmailWorkerQueueSize, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := parseNewQuestionEmailWorkerQueueSize(tt.value) + if got != tt.want { + t.Fatalf("parseNewQuestionEmailWorkerQueueSize(%q) = %d, want %d", tt.value, got, tt.want) + } + }) + } +} + +func TestNewQuestionEmailWorkerQueueSizeUnsetEnv(t *testing.T) { + setNewQuestionEmailWorkerQueueSizeEnv(t, "", false) + + got := newQuestionEmailWorkerQueueSize() + if got != defaultNewQuestionEmailWorkerQueueSize { + t.Fatalf("newQuestionEmailWorkerQueueSize() = %d, want %d", + got, defaultNewQuestionEmailWorkerQueueSize) + } +} + +func TestNewQuestionEmailWorkerWithDefaultsUsesQueueSizeEnv(t *testing.T) { + tests := []struct { + name string + value string + want int + }{ + { + name: "configured", + value: "2048", + want: 2048, + }, + { + name: "invalid uses default", + value: "invalid", + want: defaultNewQuestionEmailWorkerQueueSize, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + setNewQuestionEmailWorkerQueueSizeEnv(t, tt.value, true) + + worker := newQuestionEmailWorkerWithDefaults(func() time.Duration { return 0 }, nil) + defer worker.Close() + + if got := cap(worker.tasks); got != tt.want { + t.Fatalf("cap(worker.tasks) = %d, want %d", got, tt.want) + } + }) + } +} + func TestNewQuestionEmailWorkerDelaysBetweenAttempts(t *testing.T) { timerFactory := newFakeNewQuestionEmailTimerFactory() sendCh := make(chan newQuestionEmailSendEvent, 2) @@ -438,6 +544,28 @@ func newUnstartedNewQuestionEmailWorkerForTest(bufferSize int) *newQuestionEmail } } +func setNewQuestionEmailWorkerQueueSizeEnv(t *testing.T, value string, set bool) { + t.Helper() + + oldValue, oldSet := os.LookupEnv(newQuestionEmailWorkerQueueSizeEnv) + if set { + if err := os.Setenv(newQuestionEmailWorkerQueueSizeEnv, value); err != nil { + t.Fatalf("set env: %v", err) + } + } else { + if err := os.Unsetenv(newQuestionEmailWorkerQueueSizeEnv); err != nil { + t.Fatalf("unset env: %v", err) + } + } + t.Cleanup(func() { + if oldSet { + _ = os.Setenv(newQuestionEmailWorkerQueueSizeEnv, oldValue) + } else { + _ = os.Unsetenv(newQuestionEmailWorkerQueueSizeEnv) + } + }) +} + func receiveNewQuestionEmailSend(t *testing.T, sendCh <-chan newQuestionEmailSendEvent) newQuestionEmailSendEvent { t.Helper() select { From f5e25dcb8d7c793316161ce1a6925edb596e1d2e Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Tue, 7 Jul 2026 19:03:27 +0800 Subject: [PATCH 17/27] fix(notification): remove buffer size parameter from new question email worker for test --- .../new_question_email_worker_test.go | 22 +++++---- .../new_question_notification_test.go | 45 ++++++++----------- 2 files changed, 29 insertions(+), 38 deletions(-) diff --git a/internal/service/notification/new_question_email_worker_test.go b/internal/service/notification/new_question_email_worker_test.go index e7ea4880..739dfe50 100644 --- a/internal/service/notification/new_question_email_worker_test.go +++ b/internal/service/notification/new_question_email_worker_test.go @@ -346,7 +346,7 @@ func TestNewQuestionEmailWorkerBuildsFreshRawDataPerAttempt(t *testing.T) { } func TestNewQuestionEmailWorkerTryEnqueueCopiesTaskAndFailsFast(t *testing.T) { - worker := newUnstartedNewQuestionEmailWorkerForTest(1) + worker := newUnstartedNewQuestionEmailWorkerForTest() task := newQuestionEmailTask{ UserIDs: []string{"user-1"}, QuestionTitle: "Question", @@ -380,7 +380,7 @@ func TestNewQuestionEmailWorkerTryEnqueueCopiesTaskAndFailsFast(t *testing.T) { t.Fatalf("TryEnqueue() after Close() = true, want false") } - canceledWorker := newUnstartedNewQuestionEmailWorkerForTest(1) + canceledWorker := newUnstartedNewQuestionEmailWorkerForTest() canceledWorker.cancel() if canceledWorker.TryEnqueue(newQuestionEmailWorkerTask("question-5", "user-5")) { t.Fatalf("TryEnqueue() after cancel = true, want false") @@ -393,8 +393,8 @@ func TestNewQuestionEmailWorkerTryEnqueueConcurrentClose(t *testing.T) { senders = 32 ) - for iteration := 0; iteration < iterations; iteration++ { - worker := newUnstartedNewQuestionEmailWorkerForTest(1) + for iteration := range iterations { + worker := newUnstartedNewQuestionEmailWorkerForTest() if !worker.TryEnqueue(newQuestionEmailWorkerTask("already-queued", "queued-user")) { t.Fatalf("iteration %d: pre-fill TryEnqueue() = false, want true", iteration) } @@ -406,10 +406,8 @@ func TestNewQuestionEmailWorkerTryEnqueueConcurrentClose(t *testing.T) { var acceptedAfterCloseObserved atomic.Int64 var wg sync.WaitGroup - for sender := 0; sender < senders; sender++ { - wg.Add(1) - go func(sender int) { - defer wg.Done() + for range senders { + wg.Go(func() { defer func() { if recovered := recover(); recovered != nil { panicCh <- recovered @@ -428,9 +426,9 @@ func TestNewQuestionEmailWorkerTryEnqueueConcurrentClose(t *testing.T) { } runtime.Gosched() } - }(sender) + }) } - for sender := 0; sender < senders; sender++ { + for range senders { <-ready } @@ -533,10 +531,10 @@ func newQuestionEmailWorkerTask(questionID string, userIDs ...string) newQuestio } } -func newUnstartedNewQuestionEmailWorkerForTest(bufferSize int) *newQuestionEmailWorker { +func newUnstartedNewQuestionEmailWorkerForTest() *newQuestionEmailWorker { ctx, cancel := context.WithCancel(context.Background()) return &newQuestionEmailWorker{ - tasks: make(chan newQuestionEmailTask, bufferSize), + tasks: make(chan newQuestionEmailTask, 1), interval: func() time.Duration { return 0 }, timerFactory: newRealNewQuestionEmailTimer, ctx: ctx, diff --git a/internal/service/notification/new_question_notification_test.go b/internal/service/notification/new_question_notification_test.go index e7db3814..3bb6a3dd 100644 --- a/internal/service/notification/new_question_notification_test.go +++ b/internal/service/notification/new_question_notification_test.go @@ -181,7 +181,7 @@ func TestHandleNewQuestionNotificationEnqueuesEmailTask(t *testing.T) { }, siteInfoService: siteInfoService, } - service.newQuestionEmailWorker = newUnstartedNewQuestionEmailWorkerForTest(1) + service.newQuestionEmailWorker = newUnstartedNewQuestionEmailWorkerForTest() err = service.handleNewQuestionNotification(context.Background(), &schema.ExternalNotificationMsg{ NewQuestionTemplateRawData: &schema.NewQuestionTemplateRawData{ @@ -243,7 +243,7 @@ func TestHandleNewQuestionNotificationSkipsEnqueueWithoutEnabledEmailAttempts(t "all-user": newQuestionNotificationTestUser("all-user"), }, }, - newQuestionEmailWorker: newUnstartedNewQuestionEmailWorkerForTest(1), + newQuestionEmailWorker: newUnstartedNewQuestionEmailWorkerForTest(), } err = service.handleNewQuestionNotification(context.Background(), &schema.ExternalNotificationMsg{ @@ -271,7 +271,7 @@ func TestHandleNewQuestionNotificationReturnsWhenEmailWorkerQueueFull(t *testing } t.Cleanup(cleanup) - worker := newUnstartedNewQuestionEmailWorkerForTest(1) + worker := newUnstartedNewQuestionEmailWorkerForTest() if !worker.TryEnqueue(newQuestionEmailWorkerTask("already-queued", "queued-user")) { t.Fatalf("pre-fill TryEnqueue() = false, want true") } @@ -332,7 +332,7 @@ func TestHandleNewQuestionNotificationSyncsPluginBeforeEmailEnqueue(t *testing.T releaseNotify := make(chan struct{}) enableNewQuestionNotificationTestPlugin(t, notifyStarted, releaseNotify) - worker := newUnstartedNewQuestionEmailWorkerForTest(1) + worker := newUnstartedNewQuestionEmailWorkerForTest() service := &ExternalNotificationService{ data: &basedata.Data{Cache: cache}, userNotificationConfigRepo: &newQuestionNotificationTestUserNotificationConfigRepo{ @@ -428,13 +428,6 @@ func setNewQuestionNotificationEmailSendIntervalEnv(t *testing.T, value string, }) } -func newQuestionSubscriber(userID string, channels ...*schema.NotificationChannelConfig) *NewQuestionSubscriber { - return &NewQuestionSubscriber{ - UserID: userID, - Channels: channels, - } -} - func newQuestionEmailChannel(enable bool) *schema.NotificationChannelConfig { return &schema.NotificationChannelConfig{ Key: constant.EmailChannel, @@ -442,13 +435,6 @@ func newQuestionEmailChannel(enable bool) *schema.NotificationChannelConfig { } } -func newQuestionNonEmailChannel(enable bool) *schema.NotificationChannelConfig { - return &schema.NotificationChannelConfig{ - Key: constant.NotificationChannelKey("inbox"), - Enable: enable, - } -} - func newQuestionNotificationConfig( userID string, source constant.NotificationSource, emailEnabled bool) *entity.UserNotificationConfig { channels := schema.NotificationChannels{ @@ -702,14 +688,6 @@ func (r *newQuestionNotificationTestEmailRepo) VerifyCode(context.Context, strin return "", nil } -func (r *newQuestionNotificationTestEmailRepo) userIDs() []string { - userIDs := make([]string, 0, len(r.codesByUserID)) - for userID := range r.codesByUserID { - userIDs = append(userIDs, userID) - } - return userIDs -} - var ( newQuestionNotificationTestPluginOnce sync.Once newQuestionNotificationTestPluginInst = &newQuestionNotificationTestPlugin{} @@ -820,3 +798,18 @@ func (newQuestionNotificationTestUserExternalLoginRepo) GetCacheUserExternalLogi context.Context, string) (*schema.ExternalLoginUserInfoCache, error) { return nil, nil } + +func (newQuestionNotificationTestUserExternalLoginRepo) SetCacheOAuthState( + context.Context, string, *schema.ExternalLoginOAuthState, time.Duration) error { + return nil +} + +func (newQuestionNotificationTestUserExternalLoginRepo) GetCacheOAuthState( + context.Context, string) (*schema.ExternalLoginOAuthState, error) { + return nil, nil +} + +func (newQuestionNotificationTestUserExternalLoginRepo) DeleteCacheOAuthState( + context.Context, string) error { + return nil +} From 88c288fdda2ccb4690f3bba3b6cc570389fe9e6b Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Tue, 7 Jul 2026 19:32:24 +0800 Subject: [PATCH 18/27] fix(tests): update goroutine handling in new question email worker tests --- .../new_question_email_worker_test.go | 7 +++++-- .../new_question_notification_test.go | 15 --------------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/internal/service/notification/new_question_email_worker_test.go b/internal/service/notification/new_question_email_worker_test.go index 739dfe50..c708ffd2 100644 --- a/internal/service/notification/new_question_email_worker_test.go +++ b/internal/service/notification/new_question_email_worker_test.go @@ -407,7 +407,10 @@ func TestNewQuestionEmailWorkerTryEnqueueConcurrentClose(t *testing.T) { var wg sync.WaitGroup for range senders { - wg.Go(func() { + //nolint:modernize // CI uses Go 1.23, which does not support WaitGroup.Go. + wg.Add(1) + go func() { + defer wg.Done() defer func() { if recovered := recover(); recovered != nil { panicCh <- recovered @@ -426,7 +429,7 @@ func TestNewQuestionEmailWorkerTryEnqueueConcurrentClose(t *testing.T) { } runtime.Gosched() } - }) + }() } for range senders { <-ready diff --git a/internal/service/notification/new_question_notification_test.go b/internal/service/notification/new_question_notification_test.go index 3bb6a3dd..754481f6 100644 --- a/internal/service/notification/new_question_notification_test.go +++ b/internal/service/notification/new_question_notification_test.go @@ -798,18 +798,3 @@ func (newQuestionNotificationTestUserExternalLoginRepo) GetCacheUserExternalLogi context.Context, string) (*schema.ExternalLoginUserInfoCache, error) { return nil, nil } - -func (newQuestionNotificationTestUserExternalLoginRepo) SetCacheOAuthState( - context.Context, string, *schema.ExternalLoginOAuthState, time.Duration) error { - return nil -} - -func (newQuestionNotificationTestUserExternalLoginRepo) GetCacheOAuthState( - context.Context, string) (*schema.ExternalLoginOAuthState, error) { - return nil, nil -} - -func (newQuestionNotificationTestUserExternalLoginRepo) DeleteCacheOAuthState( - context.Context, string) error { - return nil -} From 58dbe2c02759d60a6bdab5fe3711e2eb2c5b295b Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Wed, 8 Jul 2026 10:31:16 +0800 Subject: [PATCH 19/27] fix(build): streamline Docker image build process in CI configuration --- .github/workflows/build-image-for-test.yml | 41 +++++++++------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/.github/workflows/build-image-for-test.yml b/.github/workflows/build-image-for-test.yml index b90a1ac4..a650f06e 100644 --- a/.github/workflows/build-image-for-test.yml +++ b/.github/workflows/build-image-for-test.yml @@ -30,32 +30,23 @@ jobs: - name: Checkout uses: actions/checkout@v4 - - name: Docker meta - id: meta - uses: docker/metadata-action@c299e40c65443455700f0fdfc63efafe5b349051 # v5.10.0 - with: - images: apache/answer - tags: | - type=raw,value=test - - - name: Set up QEMU - uses: docker/setup-qemu-action@29109295f81e9208d7d86ff1c6c12d2833863392 # v3.6.0 + - name: Login to DockerHub + run: echo "${{ secrets.DOCKERHUB_TOKEN }}" | docker login --username "${{ secrets.DOCKERHUB_USER }}" --password-stdin - name: Set up Docker Buildx - uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3.12.0 - - - name: Login to DockerHub - uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3.7.0 - with: - username: ${{ secrets.DOCKERHUB_USER }} - password: ${{ secrets.DOCKERHUB_TOKEN }} + run: | + docker buildx create --name answer-builder --driver docker-container --use + docker buildx inspect --bootstrap - name: Build and push - uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6.19.2 - with: - context: . - file: ./Dockerfile - platforms: linux/amd64 - push: true - tags: ${{ steps.meta.outputs.tags }} - labels: ${{ steps.meta.outputs.labels }} + run: | + BUILD_DATE="$(date -u +"%Y-%m-%dT%H:%M:%SZ")" + docker buildx build \ + --file ./Dockerfile \ + --platform linux/amd64 \ + --push \ + --tag apache/answer:test \ + --label org.opencontainers.image.created="${BUILD_DATE}" \ + --label org.opencontainers.image.revision="${GITHUB_SHA}" \ + --label org.opencontainers.image.source="https://github.com/${GITHUB_REPOSITORY}" \ + . From c52e2eeb1f3893eed898213c48198432addfec77 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Wed, 8 Jul 2026 10:37:30 +0800 Subject: [PATCH 20/27] fix(build): update Go version in Dockerfile to 1.25-alpine --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index c06f6f85..ac461d4c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. -FROM golang:1.24-alpine AS golang-builder +FROM golang:1.25-alpine AS golang-builder LABEL maintainer="linkinstar@apache.org" ARG GOPROXY From 0705ea6baeaf2fc3a2abdff8ef75b6b5a0b26e67 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Wed, 8 Jul 2026 10:41:28 +0800 Subject: [PATCH 21/27] fix(tests): add mock methods for cache OAuth state in new question notification tests --- .../new_question_notification_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/internal/service/notification/new_question_notification_test.go b/internal/service/notification/new_question_notification_test.go index 754481f6..3bb6a3dd 100644 --- a/internal/service/notification/new_question_notification_test.go +++ b/internal/service/notification/new_question_notification_test.go @@ -798,3 +798,18 @@ func (newQuestionNotificationTestUserExternalLoginRepo) GetCacheUserExternalLogi context.Context, string) (*schema.ExternalLoginUserInfoCache, error) { return nil, nil } + +func (newQuestionNotificationTestUserExternalLoginRepo) SetCacheOAuthState( + context.Context, string, *schema.ExternalLoginOAuthState, time.Duration) error { + return nil +} + +func (newQuestionNotificationTestUserExternalLoginRepo) GetCacheOAuthState( + context.Context, string) (*schema.ExternalLoginOAuthState, error) { + return nil, nil +} + +func (newQuestionNotificationTestUserExternalLoginRepo) DeleteCacheOAuthState( + context.Context, string) error { + return nil +} From dc8443176818cd91e33939de999b0ef9f610b38b Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Wed, 8 Jul 2026 11:05:24 +0800 Subject: [PATCH 22/27] fix(build): update Docker build process to use buildx and streamline image tagging --- .../build-image-for-latest-release.yml | 41 ++++++++--------- .github/workflows/build-image-for-manual.yml | 42 ++++++++---------- .github/workflows/build-image-for-release.yml | 44 +++++++++---------- 3 files changed, 59 insertions(+), 68 deletions(-) diff --git a/.github/workflows/build-image-for-latest-release.yml b/.github/workflows/build-image-for-latest-release.yml index 315d7fef..0f81013f 100644 --- a/.github/workflows/build-image-for-latest-release.yml +++ b/.github/workflows/build-image-for-latest-release.yml @@ -35,32 +35,29 @@ jobs: - name: Checkout uses: actions/checkout@v4 - - name: Docker meta - id: meta - uses: docker/metadata-action@c299e40c65443455700f0fdfc63efafe5b349051 # v5.10.0 - with: - images: apache/answer - tags: | - type=raw,value=latest - - name: Set up QEMU - uses: docker/setup-qemu-action@29109295f81e9208d7d86ff1c6c12d2833863392 # v3.6.0 + run: | + sudo apt-get update + sudo apt-get install -y qemu-user-static - name: Set up Docker Buildx - uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3.12.0 + run: | + docker buildx create --name answer-builder --driver docker-container --use + docker buildx inspect --bootstrap - name: Login to DockerHub - uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3.7.0 - with: - username: ${{ secrets.DOCKERHUB_USER }} - password: ${{ secrets.DOCKERHUB_TOKEN }} + run: echo "${{ secrets.DOCKERHUB_TOKEN }}" | docker login --username "${{ secrets.DOCKERHUB_USER }}" --password-stdin - name: Build and push - uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6.19.2 - with: - context: . - platforms: linux/amd64,linux/arm64 - push: true - file: ./Dockerfile - tags: ${{ steps.meta.outputs.tags }} - labels: ${{ steps.meta.outputs.labels }} + run: | + BUILD_DATE="$(date -u +"%Y-%m-%dT%H:%M:%SZ")" + docker buildx build \ + --file ./Dockerfile \ + --platform linux/amd64,linux/arm64 \ + --push \ + --tag apache/answer:latest \ + --label org.opencontainers.image.created="${BUILD_DATE}" \ + --label org.opencontainers.image.revision="${GITHUB_SHA}" \ + --label org.opencontainers.image.source="https://github.com/${GITHUB_REPOSITORY}" \ + --label org.opencontainers.image.version="${GITHUB_REF_NAME#v}" \ + . diff --git a/.github/workflows/build-image-for-manual.yml b/.github/workflows/build-image-for-manual.yml index 8ccf7b4f..459e0f3a 100644 --- a/.github/workflows/build-image-for-manual.yml +++ b/.github/workflows/build-image-for-manual.yml @@ -33,33 +33,29 @@ jobs: - name: Checkout uses: actions/checkout@v4 - - name: Docker meta - id: meta - uses: docker/metadata-action@c299e40c65443455700f0fdfc63efafe5b349051 # v5.10.0 - with: - images: apache/answer - tags: | - type=ref,enable=true,priority=600,prefix=,suffix=,event=branch - type=semver,pattern={{version}} - - name: Set up QEMU - uses: docker/setup-qemu-action@29109295f81e9208d7d86ff1c6c12d2833863392 # v3.6.0 + run: | + sudo apt-get update + sudo apt-get install -y qemu-user-static - name: Set up Docker Buildx - uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3.12.0 + run: | + docker buildx create --name answer-builder --driver docker-container --use + docker buildx inspect --bootstrap - name: Login to DockerHub - uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3.7.0 - with: - username: ${{ secrets.DOCKERHUB_USER }} - password: ${{ secrets.DOCKERHUB_TOKEN }} + run: echo "${{ secrets.DOCKERHUB_TOKEN }}" | docker login --username "${{ secrets.DOCKERHUB_USER }}" --password-stdin - name: Build and push - uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6.19.2 - with: - context: . - platforms: linux/amd64,linux/arm64 - push: true - file: ./Dockerfile - tags: apache/answer:${{ inputs.tag_name }} - labels: ${{ steps.meta.outputs.labels }} + run: | + BUILD_DATE="$(date -u +"%Y-%m-%dT%H:%M:%SZ")" + docker buildx build \ + --file ./Dockerfile \ + --platform linux/amd64,linux/arm64 \ + --push \ + --tag "apache/answer:${{ inputs.tag_name }}" \ + --label org.opencontainers.image.created="${BUILD_DATE}" \ + --label org.opencontainers.image.revision="${GITHUB_SHA}" \ + --label org.opencontainers.image.source="https://github.com/${GITHUB_REPOSITORY}" \ + --label org.opencontainers.image.version="${{ inputs.tag_name }}" \ + . diff --git a/.github/workflows/build-image-for-release.yml b/.github/workflows/build-image-for-release.yml index da839047..ad7deffb 100644 --- a/.github/workflows/build-image-for-release.yml +++ b/.github/workflows/build-image-for-release.yml @@ -34,33 +34,31 @@ jobs: - name: Checkout uses: actions/checkout@v4 - - name: Docker meta - id: meta - uses: docker/metadata-action@c299e40c65443455700f0fdfc63efafe5b349051 # v5.10.0 - with: - images: apache/answer - tags: | - type=ref,enable=true,priority=600,prefix=,suffix=,event=branch - type=semver,pattern={{version}} - - name: Set up QEMU - uses: docker/setup-qemu-action@29109295f81e9208d7d86ff1c6c12d2833863392 # v3.6.0 + run: | + sudo apt-get update + sudo apt-get install -y qemu-user-static - name: Set up Docker Buildx - uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3.12.0 + run: | + docker buildx create --name answer-builder --driver docker-container --use + docker buildx inspect --bootstrap - name: Login to DockerHub - uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3.7.0 - with: - username: ${{ secrets.DOCKERHUB_USER }} - password: ${{ secrets.DOCKERHUB_TOKEN }} + run: echo "${{ secrets.DOCKERHUB_TOKEN }}" | docker login --username "${{ secrets.DOCKERHUB_USER }}" --password-stdin - name: Build and push - uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6.19.2 - with: - context: . - platforms: linux/amd64,linux/arm64 - push: true - file: ./Dockerfile - tags: ${{ steps.meta.outputs.tags }} - labels: ${{ steps.meta.outputs.labels }} + env: + IMAGE_TAG: ${{ github.ref_name }} + run: | + BUILD_DATE="$(date -u +"%Y-%m-%dT%H:%M:%SZ")" + docker buildx build \ + --file ./Dockerfile \ + --platform linux/amd64,linux/arm64 \ + --push \ + --tag "apache/answer:${IMAGE_TAG#v}" \ + --label org.opencontainers.image.created="${BUILD_DATE}" \ + --label org.opencontainers.image.revision="${GITHUB_SHA}" \ + --label org.opencontainers.image.source="https://github.com/${GITHUB_REPOSITORY}" \ + --label org.opencontainers.image.version="${IMAGE_TAG#v}" \ + . From c589b59dd5f16d59b389ec4da8f8e1a97ac8ef04 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Wed, 8 Jul 2026 12:27:43 +0800 Subject: [PATCH 23/27] fix: enhance answer visibility checks for user permissions --- internal/service/content/answer_service.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/service/content/answer_service.go b/internal/service/content/answer_service.go index 25b0050e..bda7b582 100644 --- a/internal/service/content/answer_service.go +++ b/internal/service/content/answer_service.go @@ -554,12 +554,18 @@ func (as *AnswerService) Get(ctx context.Context, answerID, loginUserID string, if !exist { return nil, nil, false, errors.NotFound(reason.AnswerNotFound) } + if (question.Status == entity.QuestionStatusDeleted || question.Status == entity.QuestionStatusPending || question.Show == entity.QuestionHide) && !isAdminModerator && question.UserID != loginUserID { return nil, nil, false, errors.NotFound(reason.AnswerNotFound) } + if (answerInfo.Status == entity.AnswerStatusDeleted || + answerInfo.Status == entity.AnswerStatusPending) && + !isAdminModerator && answerInfo.UserID != loginUserID { + return nil, nil, false, errors.NotFound(reason.AnswerNotFound) + } info := as.ShowFormat(ctx, answerInfo) // todo questionFunc questionInfo, err := as.questionCommon.Info(ctx, answerInfo.QuestionID, loginUserID) From 9d9f5d06a8a3d732b01458800fd9df9ca34aef6c Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Wed, 8 Jul 2026 16:34:37 +0800 Subject: [PATCH 24/27] fix: enhance user admin service to manage API keys and filter answer visibility --- cmd/wire_gen.go | 2 +- internal/controller/mcp_controller.go | 6 ++++++ internal/repo/answer/answer_repo.go | 8 ++++++-- internal/repo/api_key/api_key_repo.go | 8 ++++++++ internal/service/apikey/apikey_service.go | 5 +++++ internal/service/user_admin/user_backyard.go | 16 ++++++++++++++++ 6 files changed, 42 insertions(+), 3 deletions(-) diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index d020e3e5..446f6cc0 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -244,7 +244,7 @@ func initApplication(debug bool, serverConf *conf.Server, dbConf *data.Database, notificationRepo := notification2.NewNotificationRepo(dataData) pluginUserConfigRepo := plugin_config.NewPluginUserConfigRepo(dataData) badgeAwardRepo := badge_award.NewBadgeAwardRepo(dataData, uniqueIDRepo) - userAdminService := user_admin.NewUserAdminService(userAdminRepo, userRoleRelService, authService, userCommon, userActiveActivityRepo, siteInfoCommonService, emailService, questionRepo, answerRepo, commentCommonRepo, userExternalLoginRepo, notificationRepo, pluginUserConfigRepo, badgeAwardRepo) + userAdminService := user_admin.NewUserAdminService(userAdminRepo, userRoleRelService, authService, userCommon, userActiveActivityRepo, siteInfoCommonService, emailService, questionRepo, answerRepo, commentCommonRepo, userExternalLoginRepo, notificationRepo, pluginUserConfigRepo, badgeAwardRepo, apiKeyRepo) userAdminController := controller_admin.NewUserAdminController(userAdminService) reasonRepo := reason.NewReasonRepo(configService) reasonService := reason2.NewReasonService(reasonRepo) diff --git a/internal/controller/mcp_controller.go b/internal/controller/mcp_controller.go index b40c58cf..e24c1a54 100644 --- a/internal/controller/mcp_controller.go +++ b/internal/controller/mcp_controller.go @@ -176,6 +176,9 @@ func (c *MCPController) MCPAnswersHandler() func(ctx context.Context, request mc } resp := make([]*schema.MCPSearchAnswerInfoResp, 0) for _, answer := range answerList { + if answer.Status != entity.AnswerStatusAvailable { + continue + } t := &schema.MCPSearchAnswerInfoResp{ QuestionID: answer.QuestionID, AnswerID: answer.ID, @@ -195,6 +198,9 @@ func (c *MCPController) MCPAnswersHandler() func(ctx context.Context, request mc } resp := make([]*schema.MCPSearchAnswerInfoResp, 0) for _, answer := range answerList { + if answer.Status != entity.AnswerStatusAvailable { + continue + } t := &schema.MCPSearchAnswerInfoResp{ QuestionID: answer.QuestionID, AnswerID: answer.ID, diff --git a/internal/repo/answer/answer_repo.go b/internal/repo/answer/answer_repo.go index 52963c44..42e3494a 100644 --- a/internal/repo/answer/answer_repo.go +++ b/internal/repo/answer/answer_repo.go @@ -196,8 +196,12 @@ func (ar *answerRepo) GetAnswerCount(ctx context.Context) (count int64, err erro // GetAnswerList get answer list all func (ar *answerRepo) GetAnswerList(ctx context.Context, answer *entity.Answer) (answerList []*entity.Answer, err error) { answerList = make([]*entity.Answer, 0) - answer.ID = uid.DeShortID(answer.ID) - answer.QuestionID = uid.DeShortID(answer.QuestionID) + if len(answer.ID) > 0 { + answer.ID = uid.DeShortID(answer.ID) + } + if len(answer.QuestionID) > 0 { + answer.QuestionID = uid.DeShortID(answer.QuestionID) + } err = ar.data.DB.Context(ctx).Find(&answerList, answer) if err != nil { err = errors.InternalServer(reason.DatabaseError).WithError(err).WithStack() diff --git a/internal/repo/api_key/api_key_repo.go b/internal/repo/api_key/api_key_repo.go index 2309384a..8fd9ed46 100644 --- a/internal/repo/api_key/api_key_repo.go +++ b/internal/repo/api_key/api_key_repo.go @@ -81,3 +81,11 @@ func (ar *apiKeyRepo) DeleteAPIKey(ctx context.Context, id int) (err error) { } return } + +func (ar *apiKeyRepo) DeleteAPIKeysByUserID(ctx context.Context, userID string) (err error) { + _, err = ar.data.DB.Context(ctx).Where("user_id = ?", userID).Delete(&entity.APIKey{}) + if err != nil { + err = errors.InternalServer(reason.DatabaseError).WithError(err).WithStack() + } + return +} diff --git a/internal/service/apikey/apikey_service.go b/internal/service/apikey/apikey_service.go index 43c1294c..2d154ebd 100644 --- a/internal/service/apikey/apikey_service.go +++ b/internal/service/apikey/apikey_service.go @@ -35,6 +35,7 @@ type APIKeyRepo interface { UpdateAPIKey(ctx context.Context, apiKey entity.APIKey) (err error) AddAPIKey(ctx context.Context, apiKey entity.APIKey) (err error) DeleteAPIKey(ctx context.Context, id int) (err error) + DeleteAPIKeysByUserID(ctx context.Context, userID string) (err error) } type APIKeyService struct { @@ -114,3 +115,7 @@ func (s *APIKeyService) DeleteAPIKey(ctx context.Context, req *schema.DeleteAPIK } return nil } + +func (s *APIKeyService) DeleteUserAPIKeys(ctx context.Context, userID string) error { + return s.apiKeyRepo.DeleteAPIKeysByUserID(ctx, userID) +} diff --git a/internal/service/user_admin/user_backyard.go b/internal/service/user_admin/user_backyard.go index fcced1c8..29e33804 100644 --- a/internal/service/user_admin/user_backyard.go +++ b/internal/service/user_admin/user_backyard.go @@ -45,6 +45,7 @@ import ( "github.com/apache/answer/internal/entity" "github.com/apache/answer/internal/schema" "github.com/apache/answer/internal/service/activity" + "github.com/apache/answer/internal/service/apikey" "github.com/apache/answer/internal/service/auth" "github.com/apache/answer/internal/service/role" "github.com/apache/answer/internal/service/siteinfo_common" @@ -87,6 +88,7 @@ type UserAdminService struct { notificationRepo notificationcommon.NotificationRepo pluginUserConfigRepo plugin_common.PluginUserConfigRepo badgeAwardRepo badge.BadgeAwardRepo + apiKeyRepo apikey.APIKeyRepo } // NewUserAdminService new user admin service @@ -105,6 +107,7 @@ func NewUserAdminService( notificationRepo notificationcommon.NotificationRepo, pluginUserConfigRepo plugin_common.PluginUserConfigRepo, badgeAwardRepo badge.BadgeAwardRepo, + apiKeyRepo apikey.APIKeyRepo, ) *UserAdminService { return &UserAdminService{ userRepo: userRepo, @@ -121,6 +124,7 @@ func NewUserAdminService( notificationRepo: notificationRepo, pluginUserConfigRepo: pluginUserConfigRepo, badgeAwardRepo: badgeAwardRepo, + apiKeyRepo: apiKeyRepo, } } @@ -162,6 +166,11 @@ func (us *UserAdminService) UpdateUserStatus(ctx context.Context, req *schema.Up if err != nil { return err } + if req.IsInactive() || req.IsSuspended() || req.IsDeleted() { + if err := us.revokeUserAPIKeys(ctx, userInfo.ID); err != nil { + return err + } + } // remove all content that user created, such as question, answer, comment, etc. if req.RemoveAllContent { @@ -227,11 +236,18 @@ func (us *UserAdminService) UpdateUserRole(ctx context.Context, req *schema.Upda if err != nil { return err } + if err := us.revokeUserAPIKeys(ctx, req.UserID); err != nil { + return err + } us.authService.RemoveUserAllTokens(ctx, req.UserID) return } +func (us *UserAdminService) revokeUserAPIKeys(ctx context.Context, userID string) error { + return us.apiKeyRepo.DeleteAPIKeysByUserID(ctx, userID) +} + // AddUser add user func (us *UserAdminService) AddUser(ctx context.Context, req *schema.AddUserReq) (err error) { _, has, err := us.userRepo.GetUserInfoByEmail(ctx, req.Email) From 2758e23381541ed8a2a92d2079c3f447277c4d73 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Thu, 9 Jul 2026 17:29:44 +0800 Subject: [PATCH 25/27] fix: update version to 2.0.2 in Makefile and README --- Makefile | 2 +- README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index a02eae9e..0623e1ef 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ .PHONY: build clean ui -VERSION=2.0.1 +VERSION=2.0.2 BIN=answer DIR_SRC=./cmd/answer DOCKER_CMD=docker diff --git a/README.md b/README.md index 3b8574aa..cf257aba 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ To learn more about the project, visit [answer.apache.org](https://answer.apache ### Running with docker ```bash -docker run -d -p 9080:80 -v answer-data:/data --name answer apache/answer:2.0.1 +docker run -d -p 9080:80 -v answer-data:/data --name answer apache/answer:2.0.2 ``` For more information, see [Installation](https://answer.apache.org/docs/installation). From 897fc8c4d4961863c8fde6331aa6ebee9e31244d Mon Sep 17 00:00:00 2001 From: Luffy <52o@qq52o.cn> Date: Tue, 14 Jul 2026 16:03:24 +0800 Subject: [PATCH 26/27] fix: add missing license entry for mozillazg-go-unidecode (#1552) --- docs/release/LICENSE | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release/LICENSE b/docs/release/LICENSE index 58aea229..abfadafb 100644 --- a/docs/release/LICENSE +++ b/docs/release/LICENSE @@ -265,6 +265,7 @@ The following components are provided under the MIT License. See project link fo (MIT License) Masterminds-semver (https://github.com/Masterminds/semver) [link](./licenses/LICENSE-Masterminds-semver.txt) (MIT License) mattn-go-sqlite3 (https://github.com/mattn/go-sqlite3) [link](./licenses/LICENSE-mattn-go-sqlite3.txt) (MIT License) mozillazg-go-pinyin (https://github.com/mozillazg/go-pinyin) [link](./licenses/LICENSE-mozillazg-go-pinyin.txt) + (MIT License) mozillazg-go-unidecode (https://github.com/mozillazg/go-unidecode) [link](./licenses/LICENSE-mozillazg-go-unidecode.txt) (MIT License) next-share (https://github.com/Bunlong/next-share) [link](./licenses/LIcENSE-Bunlong-next-share.txt) (MIT License) node-qrcode (https://github.com/soldair/node-qrcode) [link](./licenses/LICENSE-soldair-qrcode.txt) (MIT License) react (https://github.com/facebook/react) [link](./licenses/LICENSE-facebook-react.txt) From 3b9f1370612e690a0b7f230f05e688930db4c6d3 Mon Sep 17 00:00:00 2001 From: Luffy <52o@qq52o.cn> Date: Fri, 17 Jul 2026 22:38:41 +0800 Subject: [PATCH 27/27] fix: edit question fails when short links enabled (#1554) Close #1553 --- internal/repo/tag/tag_rel_repo.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/repo/tag/tag_rel_repo.go b/internal/repo/tag/tag_rel_repo.go index a52b1bf5..1f2dd63f 100644 --- a/internal/repo/tag/tag_rel_repo.go +++ b/internal/repo/tag/tag_rel_repo.go @@ -195,6 +195,7 @@ func (tr *tagRelRepo) CountTagRelByTagID(ctx context.Context, tagID string) (cou // GetTagRelDefaultStatusByObjectID get tag rel default status func (tr *tagRelRepo) GetTagRelDefaultStatusByObjectID(ctx context.Context, objectID string) (status int, err error) { question := entity.Question{} + objectID = uid.DeShortID(objectID) exist, err := tr.data.DB.Context(ctx).ID(objectID).Cols("show", "status").Get(&question) if err != nil { err = errors.InternalServer(reason.DatabaseError).WithError(err).WithStack()