From 7c210a4855270ef0b91d8e5471479b04f24c65ae Mon Sep 17 00:00:00 2001 From: Luke Gao Date: Sat, 30 May 2026 15:33:54 +0800 Subject: [PATCH 01/25] 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 57a23d432..03b06701b 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 dac2b38fd..3bfb2e3ce 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 7a7adb681..c08f1e8d4 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 9a0d198b3..61f496500 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 125cdab22..e7495253b 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 14dea3470..da8f11b11 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 682a7b207..7fca2d50d 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 000000000..810c21011 --- /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 fd34278a1..60ec5d747 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 d095ac0e9..b7ddc6b10 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 308726e80..dbcec6c04 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 1e79ca7c4..4cbd24179 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 83ffe8f1e..e355e8041 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 99e21adc8..c6cc699ce 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 68085ab74277b399d3c6980a28d08eac1eef936f Mon Sep 17 00:00:00 2001 From: Luffy <52o@qq52o.cn> Date: Tue, 2 Jun 2026 11:50:54 +0800 Subject: [PATCH 02/25] fix: update license entries --- docs/release/LICENSE | 65 ++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/docs/release/LICENSE b/docs/release/LICENSE index 926d64b79..58aea229d 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) i18next (https://github.com/i18next/i18next) [link](./licenses/LICENSE-i18next-i18next.txt) - (MIT License) lodash (https://github.com/lodash/lodash) [link](./licenses/LICENSE-lodash-lodash.txt) - (MIT License) marked (https://github.com/markedjs/marked) [link](./licenses/LICENSE-markedjs-marked.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) 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) segmentfault-pacman (https://github.com/segmentfault/pacman) [link](./licenses/LICENSE-segmentfault-pacman.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) 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 e884bb61cb1b4cd43469bdd4688a948009fedb3d Mon Sep 17 00:00:00 2001 From: Ahmed Qasid Date: Wed, 3 Jun 2026 17:06:26 +0300 Subject: [PATCH 03/25] 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 000000000..8a7780fcc --- /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 89fd71460..11c3a8168 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 bf30d85b3..be61e11f6 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 e2e017c8d..929080838 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 39de9e960..bcedcb3c8 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 cece87f9dc6dc61eedc4157c514b14d4788cbc51 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/25] 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 000000000..7e844f78e --- /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 000000000..c01719fce --- /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 765cbf6be..1e8204d36 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 682811f769f4db6d24a0d1749901d95c2d93a7eb 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/25] 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 7e844f78e..02b1cbde7 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 c01719fce..58dbfdb0b 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 1e8204d36..8db557440 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 43a91313d8b323b5f79a0e5e69b69a244a2468f3 Mon Sep 17 00:00:00 2001 From: hgaol Date: Tue, 9 Jun 2026 15:26:11 +0000 Subject: [PATCH 06/25] 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 d7506d6a8..25b0050ea 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 000000000..3cfcb1056 --- /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 d93e31e92afa500aec40090f2f15d0c0cf3c5639 Mon Sep 17 00:00:00 2001 From: Artur Iusupov Date: Fri, 5 Jun 2026 00:42:00 +0400 Subject: [PATCH 07/25] 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 03b06701b..29e3a9622 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 3bfb2e3ce..2cc8f1305 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 c08f1e8d4..2f7e3754e 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 61f496500..5d1faa3e0 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 77c806e07..9552182ed 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 000000000..e48dc8636 --- /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 9dbe6eb8e..d5098dd0c 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 7fca2d50d..59fbb7bea 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 72765e372..bf785755a 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 000000000..1cc1895ba --- /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 000000000..e082ed158 --- /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 bdf2308d3..3cd27873a 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 d209c8f58..7d0c55a58 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 42a2efda7..1d7866f35 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 000000000..0d77b383d --- /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 1e25cbaa4..7656a1407 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 000000000..fc4bbdf70 --- /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 5e3964c0c..752ae0510 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 a87d427f2..430f45fff 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 dbcec6c04..8ab714230 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 a4a61152f..c596fc1c6 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 bffa44652..8dc30b965 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 4d894b6a8..0152983e7 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 cac34b4ff..f612bac93 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 7acf765ee..f49c394b8 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 3b6f981b81ca4376138734e4039eab59b1fea63c Mon Sep 17 00:00:00 2001 From: Artur Iusupov Date: Tue, 9 Jun 2026 01:39:27 +0400 Subject: [PATCH 08/25] 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 29e3a9622..4e48c88d0 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 2cc8f1305..a075dfe45 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 2f7e3754e..b3416a10e 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 9552182ed..531d88b45 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 e48dc8636..000000000 --- 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 1cc1895ba..245bb976d 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 3cd27873a..1d0b27ff6 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 000000000..e5413f4a9 --- /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 7d0c55a58..0683a5aff 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 1d7866f35..d0ebe8754 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 0d77b383d..d77a1c448 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 7656a1407..8b32b722e 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 fc4bbdf70..a3f834a53 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 1e329eb50132bce7f808820db02ea1f7e6304e27 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Thu, 2 Jul 2026 17:41:16 +0800 Subject: [PATCH 09/25] 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 987798d19..ab278cc0f 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 939a2a092..28aa5fc6a 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 b5cf85e86..12d2a339d 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 21389e9ca..5ec7de128 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 3f82179b7..2d209dbb5 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 e1d58ab6357766bbccb3da986bd6eabad8d5c924 Mon Sep 17 00:00:00 2001 From: Artur Iusupov Date: Wed, 17 Jun 2026 12:13:56 +0400 Subject: [PATCH 10/25] 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 0a5471873..bd323c7f9 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 000000000..22f17450f --- /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 000000000..4d1729a2f --- /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 d10e6aad701d857141da9fcebe35c99be5dc38ac Mon Sep 17 00:00:00 2001 From: Artur Iusupov Date: Sat, 27 Jun 2026 23:35:28 +0400 Subject: [PATCH 11/25] 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 | 472 ++++++++++----- 6 files changed, 1227 insertions(+), 211 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 425a8c2bb..5282cab0f 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 000000000..be3e6504d --- /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 000000000..302de4eb9 --- /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 bd323c7f9..43c5ff859 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 22f17450f..19bb1cea8 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 } - 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++ - } + if seconds > maxNewQuestionNotificationEmailSendIntervalSeconds { + return maxNewQuestionNotificationEmailSendInterval } + return time.Duration(seconds) * time.Second } diff --git a/internal/service/notification/new_question_notification_test.go b/internal/service/notification/new_question_notification_test.go index 4d1729a2f..e7db3814d 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) } + var task newQuestionEmailTask + select { + case task = <-service.newQuestionEmailWorker.tasks: + default: + t.Fatalf("expected enqueued new question email task") + } + 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) + 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") } - if codes := emailRepo.codesByUserID["author"]; len(codes) > 0 { - t.Fatalf("question author received notification codes: %v", codes) + 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 b70dda997a4e5d6c44559875c68c3ee49f5ef3e4 Mon Sep 17 00:00:00 2001 From: Artur Iusupov Date: Wed, 1 Jul 2026 13:30:44 +0400 Subject: [PATCH 12/25] 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 be3e6504d..c3df1beea 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 302de4eb9..e7ea4880d 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 9e8414cc15e8ba74d0cba05fe95048e89ff58b00 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Tue, 7 Jul 2026 12:18:55 +0800 Subject: [PATCH 13/25] 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 835680883..b932ba8b7 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 3f30215e716e78cbfc7787d288ff22cef300fed2 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Tue, 7 Jul 2026 14:33:21 +0800 Subject: [PATCH 14/25] 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 d0ebe8754..c1f800ff9 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 e424fce03fa049303afab5bceaca6c797aa77354 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Tue, 7 Jul 2026 15:05:47 +0800 Subject: [PATCH 15/25] 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 11c3a8168..7b68c180c 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 be61e11f6..1001f1da0 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 5d1b12b2d..a4e1b2f53 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 acc3e070152f22fdb252abb1a0fc0e6db50d0bdf Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Tue, 7 Jul 2026 16:26:45 +0800 Subject: [PATCH 16/25] 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 7b68c180c..5787c8b18 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 b3a8757a2..4389c2925 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 23f0fda75..b84cbde08 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 6a6dacc9d..d7798c636 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 1734f65c1..eb4fb91c7 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 474c1fcd5..73f66a4c1 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 bb00b828b..5b649354c 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 84c15d597..3edbc1b45 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 2d209dbb5..08f17ad41 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 c17e0c94c85716b94aa831ed47c06e69250cd96b Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Tue, 7 Jul 2026 19:03:27 +0800 Subject: [PATCH 17/25] 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 e7ea4880d..739dfe505 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 e7db3814d..3bb6a3dd8 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 9df5853942f17fccc81d0c3a449cbb7fc51d4ff4 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Tue, 7 Jul 2026 19:32:24 +0800 Subject: [PATCH 18/25] 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 739dfe505..c708ffd20 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 3bb6a3dd8..754481f6a 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 ef247af7a7f4ee848c14bf640b0d3a4af6c386c7 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Wed, 8 Jul 2026 10:31:16 +0800 Subject: [PATCH 19/25] 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 b90a1ac4d..a650f06e9 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 be08a3a1f7cd37dd8ca212c0125061b46e9f33df Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Wed, 8 Jul 2026 10:37:30 +0800 Subject: [PATCH 20/25] 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 c06f6f85b..ac461d4cf 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 4334d7eb5f45e4b0d919422961561656802a9ff1 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Wed, 8 Jul 2026 10:41:28 +0800 Subject: [PATCH 21/25] 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 754481f6a..3bb6a3dd8 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 74c7cf7fbed19c6c65609535d62e34d8e8308b81 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Wed, 8 Jul 2026 11:05:24 +0800 Subject: [PATCH 22/25] 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 315d7fefa..0f81013f6 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 8ccf7b4fe..459e0f3a2 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 da8390472..ad7deffb3 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 540f771b4463e0cc731aa02a3163d6bf484a9195 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Wed, 8 Jul 2026 12:27:43 +0800 Subject: [PATCH 23/25] 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 25b0050ea..bda7b582b 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 15d2d3f9d91ad15a09724cb87beaab0567cc2a85 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Wed, 8 Jul 2026 16:34:37 +0800 Subject: [PATCH 24/25] 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 d020e3e57..446f6cc0b 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 b40c58cf4..e24c1a546 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 52963c44c..42e3494a8 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 2309384a9..8fd9ed467 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 43c1294ce..2d154ebdb 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 fcced1c8b..29e338046 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 5050766490c2d0c7e5e6ac78d026efa63409db01 Mon Sep 17 00:00:00 2001 From: LinkinStars Date: Thu, 9 Jul 2026 17:29:44 +0800 Subject: [PATCH 25/25] 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 a02eae9ec..0623e1efd 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 3b8574aad..cf257aba2 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).