Skip to content

refactor: Introduce pre-consume quota and unify relay handlers #1594

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Calcium-Ion merged 12 commits into from
Aug 15, 2025
Merged

refactor: Introduce pre-consume quota and unify relay handlers #1594

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
e2037ad
refactor: Introduce pre-consume quota and unify relay handlers
Calcium-Ion Aug 14, 2025
6748b00
refactor: centralize logging and update resource initialization
Calcium-Ion Aug 14, 2025
89caccd
refactor: enhance quota handling and logging for pre-consume operations
Calcium-Ion Aug 14, 2025
7f1f368
refactor: improve channel base URL handling and enhance RelayInfo log…
Calcium-Ion Aug 14, 2025
44e9b02
refactor: enhance error handling and masking for model not found scen…
Calcium-Ion Aug 15, 2025
03fc89d
refactor: add email masking function and enhance RelayInfo logging
Calcium-Ion Aug 15, 2025
5fe1ce8
refactor: improve request type validation and enhance sensitive infor…
Calcium-Ion Aug 15, 2025
2f25e44
refactor: update token type handling and improve token counting logic
Calcium-Ion Aug 15, 2025
edeb479
Merge branch 'alpha' into refactor_relay
Calcium-Ion Aug 15, 2025
067be37
refactor: simplify domain masking logic by removing URL check
Calcium-Ion Aug 15, 2025
cc4f73d
refactor: enhance logging messages for user quota handling in pre-con…
Calcium-Ion Aug 15, 2025
1d4850e
refactor: improve logging for channel operations with detailed context
Calcium-Ion Aug 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 0 additions & 99 deletions common/logger.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,52 +1,12 @@
package common

import (
"context"
"encoding/json"
"fmt"
"github.com/bytedance/gopkg/util/gopool"
"github.com/gin-gonic/gin"
"io"
"log"
"os"
"path/filepath"
"sync"
"time"
)

const (
loggerINFO = "INFO"
loggerWarn = "WARN"
loggerError = "ERR"
)

const maxLogCount = 1000000

var logCount int
var setupLogLock sync.Mutex
var setupLogWorking bool

func SetupLogger() {
if *LogDir != "" {
ok := setupLogLock.TryLock()
if !ok {
log.Println("setup log is already working")
return
}
defer func() {
setupLogLock.Unlock()
setupLogWorking = false
}()
logPath := filepath.Join(*LogDir, fmt.Sprintf("oneapi-%s.log", time.Now().Format("20060102150405")))
fd, err := os.OpenFile(logPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
if err != nil {
log.Fatal("failed to open log file")
}
gin.DefaultWriter = io.MultiWriter(os.Stdout, fd)
gin.DefaultErrorWriter = io.MultiWriter(os.Stderr, fd)
}
}

func SysLog(s string) {
t := time.Now()
_, _ = fmt.Fprintf(gin.DefaultWriter, "[SYS] %v | %s \n", t.Format("2006/01/02 - 15:04:05"), s)
Expand All @@ -57,67 +17,8 @@ func SysError(s string) {
_, _ = fmt.Fprintf(gin.DefaultErrorWriter, "[SYS] %v | %s \n", t.Format("2006/01/02 - 15:04:05"), s)
}

func LogInfo(ctx context.Context, msg string) {
logHelper(ctx, loggerINFO, msg)
}

func LogWarn(ctx context.Context, msg string) {
logHelper(ctx, loggerWarn, msg)
}

func LogError(ctx context.Context, msg string) {
logHelper(ctx, loggerError, msg)
}

func logHelper(ctx context.Context, level string, msg string) {
writer := gin.DefaultErrorWriter
if level == loggerINFO {
writer = gin.DefaultWriter
}
id := ctx.Value(RequestIdKey)
if id == nil {
id = "SYSTEM"
}
now := time.Now()
_, _ = fmt.Fprintf(writer, "[%s] %v | %s | %s \n", level, now.Format("2006/01/02 - 15:04:05"), id, msg)
logCount++ // we don't need accurate count, so no lock here
if logCount > maxLogCount && !setupLogWorking {
logCount = 0
setupLogWorking = true
gopool.Go(func() {
SetupLogger()
})
}
}

func FatalLog(v ...any) {
t := time.Now()
_, _ = fmt.Fprintf(gin.DefaultErrorWriter, "[FATAL] %v | %v \n", t.Format("2006/01/02 - 15:04:05"), v)
os.Exit(1)
}

func LogQuota(quota int) string {
if DisplayInCurrencyEnabled {
return fmt.Sprintf("＄%.6f 额度", float64(quota)/QuotaPerUnit)
} else {
return fmt.Sprintf("%d 点额度", quota)
}
}

func FormatQuota(quota int) string {
if DisplayInCurrencyEnabled {
return fmt.Sprintf("＄%.6f", float64(quota)/QuotaPerUnit)
} else {
return fmt.Sprintf("%d", quota)
}
}

// LogJson 仅供测试使用 only for test
func LogJson(ctx context.Context, msg string, obj any) {
jsonStr, err := json.Marshal(obj)
if err != nil {
LogError(ctx, fmt.Sprintf("json marshal failed: %s", err.Error()))
return
}
LogInfo(ctx, fmt.Sprintf("%s | %s", msg, string(jsonStr)))
}
5 changes: 5 additions & 0 deletions common/quota.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package common

func GetTrustQuota() int {
return int(10 * QuotaPerUnit)
}
Comment thread
Calcium-Ion marked this conversation as resolved.
Resolved
2 changes: 2 additions & 0 deletions constant/context_key.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package constant
type ContextKey string

const (
ContextKeyPromptTokens ContextKey = "prompt_tokens"

ContextKeyOriginalModel ContextKey = "original_model"
ContextKeyRequestStartTime ContextKey = "request_start_time"

Expand Down
16 changes: 13 additions & 3 deletions controller/channel-test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,17 @@ func testChannel(channel *model.Channel, testModel string) testResult {
newAPIError: newAPIError,
}
}
request := buildTestRequest(testModel)

info, err := relaycommon.GenRelayInfo(c, types.RelayFormatOpenAI, request, nil)

info := relaycommon.GenRelayInfo(c)
if err != nil {
return testResult{
context: c,
localErr: err,
newAPIError: types.NewError(err, types.ErrorCodeGenRelayInfoFailed),
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

err = helper.ModelMappedHelper(c, info, nil)
if err != nil {
Expand All @@ -143,7 +152,9 @@ func testChannel(channel *model.Channel, testModel string) testResult {
newAPIError: types.NewError(err, types.ErrorCodeChannelModelMappedError),
}
}

testModel = info.UpstreamModelName
request.Model = testModel

apiType, _ := common.ChannelType2APIType(channel.Type)
adaptor := relay.GetAdaptor(apiType)
Expand All @@ -155,13 +166,12 @@ func testChannel(channel *model.Channel, testModel string) testResult {
}
}

request := buildTestRequest(testModel)
// 创建一个用于日志的 info 副本，移除 ApiKey
logInfo := *info
logInfo.ApiKey = ""
common.SysLog(fmt.Sprintf("testing channel %d with model %s , info %+v ", channel.Id, testModel, logInfo))

Comment thread
Calcium-Ion marked this conversation as resolved.
Resolved
priceData, err := helper.ModelPriceHelper(c, info, 0, int(request.GetMaxTokens()))
priceData, err := helper.ModelPriceHelper(c, info, 0, request.GetTokenCountMeta())
if err != nil {
return testResult{
context: c,
Expand Down
183 changes: 92 additions & 91 deletions controller/console_migrate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,101 +3,102 @@
package controller

import (
"encoding/json"
"net/http"
"one-api/common"
"one-api/model"
"github.com/gin-gonic/gin"
"encoding/json"
"net/http"
"one-api/common"
"one-api/model"

"github.com/gin-gonic/gin"
)

// MigrateConsoleSetting 迁移旧的控制台相关配置到 console_setting.*
func MigrateConsoleSetting(c *gin.Context) {
// 读取全部 option
opts, err := model.AllOption()
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
return
}
// 建立 map
valMap := map[string]string{}
for _, o := range opts {
valMap[o.Key] = o.Value
}
// 读取全部 option
opts, err := model.AllOption()
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
return
}
// 建立 map
valMap := map[string]string{}
for _, o := range opts {
valMap[o.Key] = o.Value
}
Comment on lines +16 to +26

@coderabbitai coderabbitai Bot Aug 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Preallocate the map and prepare to track migrated keys.

Micro-optimization and forward-prep for safe deletion later. Also, a dedicated slice for migrated keys will help avoid deleting non-migrated data.

Apply this diff:

-	// 建立 map
-	valMap := map[string]string{}
+	// 建立 map
+	valMap := make(map[string]string, len(opts))
 	for _, o := range opts {
 		valMap[o.Key] = o.Value
 	}
+
+	// 收集成功迁移的旧键（用于最后仅删除已成功迁移的键，避免数据丢失）
+	migratedKeys := make([]string, 0, 5)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// 读取全部 option
opts, err := model.AllOption()
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
return
}
// 建立 map
valMap := map[string]string{}
for _, o := range opts {
valMap[o.Key] = o.Value
}
// 读取全部 option
opts, err := model.AllOption()
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
return
}
// 建立 map
valMap := make(map[string]string, len(opts))
for _, o := range opts {
valMap[o.Key] = o.Value
}
// 收集成功迁移的旧键（用于最后仅删除已成功迁移的键，避免数据丢失）
migratedKeys := make([]string, 0, 5)
🤖 Prompt for AI Agents 
In controller/console_migrate.go around lines 16 to 26, the options map is
created without preallocation and there is no collection of migrated keys for
safe deletion; change creation of valMap to preallocate capacity using len(opts)
(e.g., make(map[string]string, len(opts))) and add a migratedKeys slice (e.g.,
var migratedKeys = make([]string, 0, len(opts))) then, as you iterate opts,
populate valMap[o.Key]=o.Value and append o.Key to migratedKeys so you can later
delete only those migrated keys safely.


// 处理 APIInfo
if v := valMap["ApiInfo"]; v != "" {
var arr []map[string]interface{}
if err := json.Unmarshal([]byte(v), &arr); err == nil {
if len(arr) > 50 {
arr = arr[:50]
}
bytes, _ := json.Marshal(arr)
model.UpdateOption("console_setting.api_info", string(bytes))
}
model.UpdateOption("ApiInfo", "")
}
// Announcements 直接搬
if v := valMap["Announcements"]; v != "" {
model.UpdateOption("console_setting.announcements", v)
model.UpdateOption("Announcements", "")
}
// FAQ 转换
if v := valMap["FAQ"]; v != "" {
var arr []map[string]interface{}
if err := json.Unmarshal([]byte(v), &arr); err == nil {
out := []map[string]interface{}{}
for _, item := range arr {
q, _ := item["question"].(string)
if q == "" {
q, _ = item["title"].(string)
}
a, _ := item["answer"].(string)
if a == "" {
a, _ = item["content"].(string)
}
if q != "" && a != "" {
out = append(out, map[string]interface{}{"question": q, "answer": a})
}
}
if len(out) > 50 {
out = out[:50]
}
bytes, _ := json.Marshal(out)
model.UpdateOption("console_setting.faq", string(bytes))
}
model.UpdateOption("FAQ", "")
}
// Uptime Kuma 迁移到新的 groups 结构（console_setting.uptime_kuma_groups）
url := valMap["UptimeKumaUrl"]
slug := valMap["UptimeKumaSlug"]
if url != "" && slug != "" {
// 仅当同时存在 URL 与 Slug 时才进行迁移
groups := []map[string]interface{}{
{
"id": 1,
"categoryName": "old",
"url": url,
"slug": slug,
"description": "",
},
}
bytes, _ := json.Marshal(groups)
model.UpdateOption("console_setting.uptime_kuma_groups", string(bytes))
}
// 清空旧键内容
if url != "" {
model.UpdateOption("UptimeKumaUrl", "")
}
if slug != "" {
model.UpdateOption("UptimeKumaSlug", "")
}
// 处理 APIInfo
if v := valMap["ApiInfo"]; v != "" {
var arr []map[string]interface{}
if err := json.Unmarshal([]byte(v), &arr); err == nil {
if len(arr) > 50 {
arr = arr[:50]
}
bytes, _ := json.Marshal(arr)
model.UpdateOption("console_setting.api_info", string(bytes))
}
model.UpdateOption("ApiInfo", "")
}
Comment on lines +28 to +39

@coderabbitai coderabbitai Bot Aug 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Avoid data loss when ApiInfo JSON is invalid; handle errors from UpdateOption and json.Marshal.

Currently the code clears the old key even if JSON parsing/marshalling fails, causing irreversible data loss. Also, UpdateOption errors are ignored.

Apply this diff:

 	// 处理 APIInfo
 	if v := valMap["ApiInfo"]; v != "" {
 		var arr []map[string]interface{}
-		if err := json.Unmarshal([]byte(v), &arr); err == nil {
-			if len(arr) > 50 {
-				arr = arr[:50]
-			}
-			bytes, _ := json.Marshal(arr)
-			model.UpdateOption("console_setting.api_info", string(bytes))
-		}
-		model.UpdateOption("ApiInfo", "")
+		if err := json.Unmarshal([]byte(v), &arr); err == nil {
+			if len(arr) > 50 {
+				arr = arr[:50]
+			}
+			if data, err := json.Marshal(arr); err == nil {
+				if err := model.UpdateOption("console_setting.api_info", string(data)); err != nil {
+					c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
+					return
+				}
+				if err := model.UpdateOption("ApiInfo", ""); err != nil {
+					c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
+					return
+				}
+				migratedKeys = append(migratedKeys, "ApiInfo")
+			} // marshalling failed: 保留旧值，不清空
+		} // parsing failed: 保留旧值，不清空
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// 处理 APIInfo
if v := valMap["ApiInfo"]; v != "" {
var arr []map[string]interface{}
if err := json.Unmarshal([]byte(v), &arr); err == nil {
if len(arr) > 50 {
arr = arr[:50]
}
bytes, _ := json.Marshal(arr)
model.UpdateOption("console_setting.api_info", string(bytes))
}
model.UpdateOption("ApiInfo", "")
}
// 处理 APIInfo
if v := valMap["ApiInfo"]; v != "" {
var arr []map[string]interface{}
if err := json.Unmarshal([]byte(v), &arr); err == nil {
if len(arr) > 50 {
arr = arr[:50]
}
if data, err := json.Marshal(arr); err == nil {
if err := model.UpdateOption("console_setting.api_info", string(data)); err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
return
}
if err := model.UpdateOption("ApiInfo", ""); err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
return
}
migratedKeys = append(migratedKeys, "ApiInfo")
} // marshalling failed: 保留旧值，不清空
} // parsing failed: 保留旧值，不清空
}
🤖 Prompt for AI Agents 
In controller/console_migrate.go around lines 28-39, the code currently clears
the original "ApiInfo" key even when json.Unmarshal or json.Marshal fails and
ignores errors returned by model.UpdateOption; change the flow so that you only
update "console_setting.api_info" and clear "ApiInfo" after successful
json.Unmarshal and json.Marshal, keep the original data if parsing fails (do not
blank it), check and handle errors returned by json.Unmarshal, json.Marshal and
model.UpdateOption (log and return or propagate the error as appropriate), and
ensure the trimming to 50 items happens before marshalling and only proceed to
call UpdateOption when marshalling succeeds.

// Announcements 直接搬
if v := valMap["Announcements"]; v != "" {
model.UpdateOption("console_setting.announcements", v)
model.UpdateOption("Announcements", "")
}
Comment on lines +41 to +44

@coderabbitai coderabbitai Bot Aug 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Handle UpdateOption errors and only delete keys that migrated successfully (Announcements).

Errors are ignored and the old key is always cleared. This risks data loss on failure.

Apply this diff:

 	// Announcements 直接搬
 	if v := valMap["Announcements"]; v != "" {
-		model.UpdateOption("console_setting.announcements", v)
-		model.UpdateOption("Announcements", "")
+		if err := model.UpdateOption("console_setting.announcements", v); err != nil {
+			c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
+			return
+		}
+		if err := model.UpdateOption("Announcements", ""); err != nil {
+			c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
+			return
+		}
+		migratedKeys = append(migratedKeys, "Announcements")
 	}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In controller/console_migrate.go around lines 41 to 44, the call to
model.UpdateOption("console_setting.announcements", v) ignores errors and always
clears the old "Announcements" key, risking data loss; change the code to
capture the error returned by UpdateOption for the new key, only call
model.UpdateOption("Announcements", "") when the first call returned nil, and
handle/propagate or log the error if the update failed so the old value is
preserved when migration does not succeed.

// FAQ 转换
if v := valMap["FAQ"]; v != "" {
var arr []map[string]interface{}
if err := json.Unmarshal([]byte(v), &arr); err == nil {
out := []map[string]interface{}{}
for _, item := range arr {
q, _ := item["question"].(string)
if q == "" {
q, _ = item["title"].(string)
}
a, _ := item["answer"].(string)
if a == "" {
a, _ = item["content"].(string)
}
if q != "" && a != "" {
out = append(out, map[string]interface{}{"question": q, "answer": a})
}
}
if len(out) > 50 {
out = out[:50]
}
bytes, _ := json.Marshal(out)
model.UpdateOption("console_setting.faq", string(bytes))
}
model.UpdateOption("FAQ", "")
}
Comment on lines +46 to +70

@coderabbitai coderabbitai Bot Aug 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Avoid data loss on FAQ parse/marshal failure; check errors and only clear/delete on success.

Same issues as ApiInfo: clearing on failure and ignoring UpdateOption/marshal errors can drop user data.

Apply this diff:

 	// FAQ 转换
 	if v := valMap["FAQ"]; v != "" {
 		var arr []map[string]interface{}
-		if err := json.Unmarshal([]byte(v), &arr); err == nil {
-			out := []map[string]interface{}{}
-			for _, item := range arr {
+		if err := json.Unmarshal([]byte(v), &arr); err == nil {
+			out := []map[string]interface{}{}
+			for _, item := range arr {
 				q, _ := item["question"].(string)
 				if q == "" {
 					q, _ = item["title"].(string)
 				}
 				a, _ := item["answer"].(string)
 				if a == "" {
 					a, _ = item["content"].(string)
 				}
 				if q != "" && a != "" {
 					out = append(out, map[string]interface{}{"question": q, "answer": a})
 				}
-			}
-			if len(out) > 50 {
-				out = out[:50]
-			}
-			bytes, _ := json.Marshal(out)
-			model.UpdateOption("console_setting.faq", string(bytes))
-		}
-		model.UpdateOption("FAQ", "")
+			}
+			if len(out) > 50 {
+				out = out[:50]
+			}
+			if data, err := json.Marshal(out); err == nil {
+				if err := model.UpdateOption("console_setting.faq", string(data)); err != nil {
+					c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
+					return
+				}
+				if err := model.UpdateOption("FAQ", ""); err != nil {
+					c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
+					return
+				}
+				migratedKeys = append(migratedKeys, "FAQ")
+			} // marshalling failed: 保留旧值，不清空
+		} // parsing failed: 保留旧值，不清空
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if v := valMap["FAQ"]; v != "" {
var arr []map[string]interface{}
if err := json.Unmarshal([]byte(v), &arr); err == nil {
out := []map[string]interface{}{}
for _, item := range arr {
q, _ := item["question"].(string)
if q == "" {
q, _ = item["title"].(string)
}
a, _ := item["answer"].(string)
if a == "" {
a, _ = item["content"].(string)
}
if q != "" && a != "" {
out = append(out, map[string]interface{}{"question": q, "answer": a})
}
}
if len(out) > 50 {
out = out[:50]
}
bytes, _ := json.Marshal(out)
model.UpdateOption("console_setting.faq", string(bytes))
}
model.UpdateOption("FAQ", "")
}
if v := valMap["FAQ"]; v != "" {
var arr []map[string]interface{}
if err := json.Unmarshal([]byte(v), &arr); err == nil {
out := []map[string]interface{}{}
for _, item := range arr {
q, _ := item["question"].(string)
if q == "" {
q, _ = item["title"].(string)
}
a, _ := item["answer"].(string)
if a == "" {
a, _ = item["content"].(string)
}
if q != "" && a != "" {
out = append(out, map[string]interface{}{"question": q, "answer": a})
}
}
if len(out) > 50 {
out = out[:50]
}
if data, err := json.Marshal(out); err == nil {
if err := model.UpdateOption("console_setting.faq", string(data)); err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
return
}
if err := model.UpdateOption("FAQ", ""); err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
return
}
migratedKeys = append(migratedKeys, "FAQ")
} // marshalling failed: 保留旧值，不清空
} // parsing failed: 保留旧值，不清空
}
🤖 Prompt for AI Agents 
In controller/console_migrate.go around lines 46 to 70, the FAQ migration
currently ignores Unmarshal/Marshal and UpdateOption errors and clears the
original "FAQ" option even on failure; change the flow to only clear or
overwrite "FAQ" after successful parsing and successful model.UpdateOption
calls: first attempt json.Unmarshal and if it fails do nothing (do not call
model.UpdateOption("FAQ", "")). Build the filtered out slice, cap to 50, then
json.Marshal and check its error; if Marshal succeeds, call
model.UpdateOption("console_setting.faq", string(bytes)) and check its returned
error — only if that UpdateOption call succeeds, call model.UpdateOption("FAQ",
"") to clear the old key; return or log errors from
Unmarshal/Marshal/UpdateOption instead of silently ignoring them.

// Uptime Kuma 迁移到新的 groups 结构（console_setting.uptime_kuma_groups）
url := valMap["UptimeKumaUrl"]
slug := valMap["UptimeKumaSlug"]
if url != "" && slug != "" {
// 仅当同时存在 URL 与 Slug 时才进行迁移
groups := []map[string]interface{}{
{
"id": 1,
"categoryName": "old",
"url": url,
"slug": slug,
"description": "",
},
}
bytes, _ := json.Marshal(groups)
model.UpdateOption("console_setting.uptime_kuma_groups", string(bytes))
}
// 清空旧键内容
if url != "" {
model.UpdateOption("UptimeKumaUrl", "")
}
if slug != "" {
model.UpdateOption("UptimeKumaSlug", "")
}
Comment on lines +72 to +94

@coderabbitai coderabbitai Bot Aug 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Uptime Kuma: Only clear old keys if migration succeeded; current logic can erase half-configured data.

If only one of url/slug exists, the code does not migrate but still clears whichever one is set. This causes data loss.

Apply this diff:

 	// Uptime Kuma 迁移到新的 groups 结构（console_setting.uptime_kuma_groups）
 	url := valMap["UptimeKumaUrl"]
 	slug := valMap["UptimeKumaSlug"]
 	if url != "" && slug != "" {
 		// 仅当同时存在 URL 与 Slug 时才进行迁移
 		groups := []map[string]interface{}{
 			{
 				"id":           1,
 				"categoryName": "old",
 				"url":          url,
 				"slug":         slug,
 				"description":  "",
 			},
 		}
-		bytes, _ := json.Marshal(groups)
-		model.UpdateOption("console_setting.uptime_kuma_groups", string(bytes))
-	}
-	// 清空旧键内容
-	if url != "" {
-		model.UpdateOption("UptimeKumaUrl", "")
-	}
-	if slug != "" {
-		model.UpdateOption("UptimeKumaSlug", "")
-	}
+		if data, err := json.Marshal(groups); err == nil {
+			if err := model.UpdateOption("console_setting.uptime_kuma_groups", string(data)); err != nil {
+				c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
+				return
+			}
+			if err := model.UpdateOption("UptimeKumaUrl", ""); err != nil {
+				c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
+				return
+			}
+			if err := model.UpdateOption("UptimeKumaSlug", ""); err != nil {
+				c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
+				return
+			}
+			migratedKeys = append(migratedKeys, "UptimeKumaUrl", "UptimeKumaSlug")
+		} // marshalling failed: 保留旧值，不清空
+	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
url := valMap["UptimeKumaUrl"]
slug := valMap["UptimeKumaSlug"]
if url != "" && slug != "" {
// 仅当同时存在 URL 与 Slug 时才进行迁移
groups := []map[string]interface{}{
{
"id": 1,
"categoryName": "old",
"url": url,
"slug": slug,
"description": "",
},
}
bytes, _ := json.Marshal(groups)
model.UpdateOption("console_setting.uptime_kuma_groups", string(bytes))
}
// 清空旧键内容
if url != "" {
model.UpdateOption("UptimeKumaUrl", "")
}
if slug != "" {
model.UpdateOption("UptimeKumaSlug", "")
}
url := valMap["UptimeKumaUrl"]
slug := valMap["UptimeKumaSlug"]
if url != "" && slug != "" {
// 仅当同时存在 URL 与 Slug 时才进行迁移
groups := []map[string]interface{}{
{
"id": 1,
"categoryName": "old",
"url": url,
"slug": slug,
"description": "",
},
}
if data, err := json.Marshal(groups); err == nil {
if err := model.UpdateOption("console_setting.uptime_kuma_groups", string(data)); err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
return
}
if err := model.UpdateOption("UptimeKumaUrl", ""); err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
return
}
if err := model.UpdateOption("UptimeKumaSlug", ""); err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"success": false, "message": err.Error()})
return
}
migratedKeys = append(migratedKeys, "UptimeKumaUrl", "UptimeKumaSlug")
} // marshalling failed: 保留旧值，不清空
}


// 删除旧键记录
oldKeys := []string{"ApiInfo", "Announcements", "FAQ", "UptimeKumaUrl", "UptimeKumaSlug"}
model.DB.Where("key IN ?", oldKeys).Delete(&model.Option{})
// 删除旧键记录
oldKeys := []string{"ApiInfo", "Announcements", "FAQ", "UptimeKumaUrl", "UptimeKumaSlug"}
model.DB.Where("key IN ?", oldKeys).Delete(&model.Option{})

Comment on lines +96 to 99

@coderabbitai coderabbitai Bot Aug 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Delete only successfully migrated keys; avoid blanket deletion that can drop data.

Unconditionally deleting all old keys risks losing data if any migration step failed or was skipped.

Apply this diff:

-	// 删除旧键记录
-	oldKeys := []string{"ApiInfo", "Announcements", "FAQ", "UptimeKumaUrl", "UptimeKumaSlug"}
-	model.DB.Where("key IN ?", oldKeys).Delete(&model.Option{})
+	// 删除旧键记录（仅删除成功迁移的）
+	if len(migratedKeys) > 0 {
+		model.DB.Where("key IN ?", migratedKeys).Delete(&model.Option{})
+	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// 删除旧键记录
oldKeys := []string{"ApiInfo", "Announcements", "FAQ", "UptimeKumaUrl", "UptimeKumaSlug"}
model.DB.Where("key IN ?", oldKeys).Delete(&model.Option{})
// 删除旧键记录（仅删除成功迁移的）
if len(migratedKeys) > 0 {
model.DB.Where("key IN ?", migratedKeys).Delete(&model.Option{})
}

// 重新加载 OptionMap
model.InitOptionMap()
common.SysLog("console setting migrated")
c.JSON(http.StatusOK, gin.H{"success": true, "message": "migrated"})
}
// 重新加载 OptionMap
model.InitOptionMap()
common.SysLog("console setting migrated")
c.JSON(http.StatusOK, gin.H{"success": true, "message": "migrated"})
}
Comment on lines +100 to +104

@coderabbitai coderabbitai Bot Aug 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider transactional safety for the entire migration.

Multiple UpdateOption calls + subsequent deletes should ideally be atomic to prevent partial migrations on failure or concurrent requests. Given UpdateOption uses the global DB, you might add Tx variants (e.g., UpdateOptionTx) and wrap the function body in model.DB.Transaction to guarantee all-or-nothing behavior.

I can draft a Tx-aware UpdateOption and wire this function into a transaction to ensure atomicity across all steps. Do you want me to provide that patch?

🤖 Prompt for AI Agents 
In controller/console_migrate.go around lines 100 to 104, the migration performs
multiple UpdateOption and delete operations against the global DB without
transactional protection; wrap the entire migration in a DB transaction
(model.DB.Transaction or equivalent) so all updates/deletes are atomic, add or
use Tx-aware variants of UpdateOption/DeleteOption that accept the transaction
handle (e.g., UpdateOptionTx / DeleteOptionTx or pass *gorm.DB tx into existing
functions), perform all changes using the tx, and only call
model.InitOptionMap() and emit the success log after the transaction commits;
ensure proper rollback on error and return the error response if the transaction
fails.

Loading