refactor: 后端代码质量优化 - 复用公共库、使用标准库、类型安全错误判断
## 高优先级修复 - stats_service_impl: 使用 strings.SplitN 替代错误的索引分割 - provider_handler: 使用 errors.Is(err, gorm.ErrDuplicatedKey) 替代字符串匹配 - client: 重写 isNetworkError 使用 errors.As/Is 类型安全判断 - proxy_handler: 使用 encoding/json 标准库解析 JSON(extractModelName、isStreamRequest) ## 中优先级修复 - stats_handler: 添加 parseDateParam 辅助函数消除重复日期解析 - pkg/errors: 新增 ErrRequestCreate/Send/ResponseRead 错误类型和 WithCause 方法 - client: 使用结构化错误替代 fmt.Errorf - ConversionEngine: logger 依赖注入,替换所有 zap.L() 调用 ## 低优先级修复 - encoder: 删除 joinStrings,使用 strings.Join - adapter: 删除 modelInfoRegex 正则,使用 isModelInfoPath 字符串函数 ## 文档更新 - README.md: 添加公共库使用指南和编码规范章节 - specs: 同步 delta specs 到 main specs(error-handling、structured-logging、request-validation) ## 归档 - openspec/changes/archive/2026-04-20-refactor-backend-code-quality/
This commit is contained in:
@@ -13,6 +13,7 @@ import (
|
||||
"github.com/gin-gonic/gin"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"gorm.io/gorm"
|
||||
|
||||
"nex/backend/internal/domain"
|
||||
"nex/backend/internal/provider"
|
||||
@@ -250,3 +251,25 @@ func formatMapErrors(errs map[string]string) string {
|
||||
}
|
||||
return "请求验证失败: " + strings.Join(parts, "; ")
|
||||
}
|
||||
|
||||
// ============ 错误类型判断测试 ============
|
||||
|
||||
func TestProviderHandler_CreateProvider_DuplicatedKey(t *testing.T) {
|
||||
h := NewProviderHandler(&mockProviderService{
|
||||
err: gorm.ErrDuplicatedKey,
|
||||
})
|
||||
|
||||
body, _ := json.Marshal(map[string]string{
|
||||
"id": "p1",
|
||||
"name": "Test",
|
||||
"api_key": "sk-test",
|
||||
"base_url": "https://test.com",
|
||||
})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("POST", "/api/providers", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
h.CreateProvider(c)
|
||||
assert.Equal(t, 409, w.Code)
|
||||
}
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
package handler
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"net/http"
|
||||
"strings"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
"gorm.io/gorm"
|
||||
@@ -55,7 +55,7 @@ func (h *ProviderHandler) CreateProvider(c *gin.Context) {
|
||||
|
||||
err := h.providerService.Create(provider)
|
||||
if err != nil {
|
||||
if strings.Contains(err.Error(), "UNIQUE constraint failed") {
|
||||
if errors.Is(err, gorm.ErrDuplicatedKey) {
|
||||
c.JSON(http.StatusConflict, gin.H{
|
||||
"error": "供应商 ID 已存在",
|
||||
})
|
||||
|
||||
@@ -2,6 +2,7 @@ package handler
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"encoding/json"
|
||||
"io"
|
||||
"net/http"
|
||||
"strings"
|
||||
@@ -213,18 +214,14 @@ func (h *ProxyHandler) isStreamRequest(body []byte, clientProtocol, nativePath s
|
||||
if ifaceType != conversion.InterfaceTypeChat {
|
||||
return false
|
||||
}
|
||||
for i, b := range body {
|
||||
if b == '"' && i+8 <= len(body) {
|
||||
if string(body[i:i+8]) == `"stream"` {
|
||||
for j := i + 8; j < len(body) && j < i+20; j++ {
|
||||
if body[j] == 't' && j+3 < len(body) && string(body[j:j+4]) == "true" {
|
||||
return true
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
var req struct {
|
||||
Stream bool `json:"stream"`
|
||||
}
|
||||
return false
|
||||
if err := json.Unmarshal(body, &req); err != nil {
|
||||
return false
|
||||
}
|
||||
return req.Stream
|
||||
}
|
||||
|
||||
// writeConversionError 写入转换错误
|
||||
@@ -312,51 +309,13 @@ func (h *ProxyHandler) forwardPassthrough(c *gin.Context, inSpec conversion.HTTP
|
||||
|
||||
// extractModelName 从 JSON body 中提取 model
|
||||
func extractModelName(body []byte) string {
|
||||
inQuote := false
|
||||
escaped := false
|
||||
keyStart := -1
|
||||
keyEnd := -1
|
||||
lookingForKey := true
|
||||
lookingForValue := false
|
||||
valueStart := -1
|
||||
|
||||
for i := 0; i < len(body); i++ {
|
||||
b := body[i]
|
||||
if escaped {
|
||||
escaped = false
|
||||
continue
|
||||
}
|
||||
if b == '\\' {
|
||||
escaped = true
|
||||
continue
|
||||
}
|
||||
if b == '"' {
|
||||
if !inQuote {
|
||||
inQuote = true
|
||||
if lookingForKey {
|
||||
keyStart = i + 1
|
||||
}
|
||||
if lookingForValue {
|
||||
valueStart = i + 1
|
||||
}
|
||||
} else {
|
||||
inQuote = false
|
||||
if lookingForKey && keyStart >= 0 {
|
||||
keyEnd = i
|
||||
if string(body[keyStart:keyEnd]) == "model" {
|
||||
lookingForKey = false
|
||||
lookingForValue = true
|
||||
}
|
||||
} else if lookingForValue && valueStart >= 0 {
|
||||
return string(body[valueStart:i])
|
||||
}
|
||||
}
|
||||
}
|
||||
if !inQuote && lookingForValue && b == ':' {
|
||||
// 等待值开始
|
||||
}
|
||||
var req struct {
|
||||
Model string `json:"model"`
|
||||
}
|
||||
return ""
|
||||
if err := json.Unmarshal(body, &req); err != nil {
|
||||
return ""
|
||||
}
|
||||
return req.Model
|
||||
}
|
||||
|
||||
// extractHeaders 从 Gin context 提取请求头
|
||||
|
||||
@@ -84,8 +84,9 @@ func (m *mockProxyStatsService) Aggregate(stats []domain.UsageStats, groupBy str
|
||||
func setupProxyEngine(t *testing.T) *conversion.ConversionEngine {
|
||||
t.Helper()
|
||||
registry := conversion.NewMemoryRegistry()
|
||||
engine := conversion.NewConversionEngine(registry)
|
||||
engine := conversion.NewConversionEngine(registry, nil)
|
||||
require.NoError(t, registry.Register(openai.NewAdapter()))
|
||||
require.NoError(t, registry.Register(anthropic.NewAdapter()))
|
||||
return engine
|
||||
}
|
||||
|
||||
@@ -321,27 +322,6 @@ func TestProxyHandler_ForwardPassthrough_NoProviders(t *testing.T) {
|
||||
assert.Equal(t, 404, w.Code)
|
||||
}
|
||||
|
||||
func TestExtractModelName(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
body string
|
||||
want string
|
||||
}{
|
||||
{"basic", `{"model":"gpt-4","messages":[]}`, "gpt-4"},
|
||||
{"nested", `{"stream":true,"model":"claude-3","messages":[]}`, "claude-3"},
|
||||
{"no_model", `{"messages":[]}`, ""},
|
||||
{"empty", "", ""},
|
||||
{"escaped", `{"model":"gpt\"4","messages":[]}`, `gpt\"4`},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := extractModelName([]byte(tt.body))
|
||||
assert.Equal(t, tt.want, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractHeaders(t *testing.T) {
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
@@ -354,30 +334,6 @@ func TestExtractHeaders(t *testing.T) {
|
||||
assert.Equal(t, "application/json", headers["Content-Type"])
|
||||
}
|
||||
|
||||
func TestIsStreamRequest(t *testing.T) {
|
||||
engine := setupProxyEngine(t)
|
||||
h := newTestProxyHandler(engine, &mockProxyProviderClient{}, &mockProxyRoutingService{}, &mockProxyProviderService{})
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
body string
|
||||
path string
|
||||
expected bool
|
||||
}{
|
||||
{"stream true chat", `{"model":"gpt-4","stream":true}`, "/v1/chat/completions", true},
|
||||
{"stream false chat", `{"model":"gpt-4","stream":false}`, "/v1/chat/completions", false},
|
||||
{"no stream field", `{"model":"gpt-4"}`, "/v1/chat/completions", false},
|
||||
{"stream true non-chat", `{"model":"gpt-4","stream":true}`, "/v1/models", false},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
result := h.isStreamRequest([]byte(tt.body), "openai", tt.path)
|
||||
assert.Equal(t, tt.expected, result)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestProxyHandler_HandleProxy_ProviderProtocolDefault(t *testing.T) {
|
||||
engine := setupProxyEngine(t)
|
||||
routingSvc := &mockProxyRoutingService{
|
||||
@@ -529,7 +485,7 @@ func TestProxyHandler_HandleStream_FlushOutput(t *testing.T) {
|
||||
|
||||
func TestProxyHandler_HandleStream_CreateStreamConverterError(t *testing.T) {
|
||||
registry := conversion.NewMemoryRegistry()
|
||||
engine := conversion.NewConversionEngine(registry)
|
||||
engine := conversion.NewConversionEngine(registry, nil)
|
||||
err := registry.Register(openai.NewAdapter())
|
||||
require.NoError(t, err)
|
||||
|
||||
@@ -552,7 +508,7 @@ func TestProxyHandler_HandleStream_CreateStreamConverterError(t *testing.T) {
|
||||
|
||||
func TestProxyHandler_HandleStream_ConvertRequestError(t *testing.T) {
|
||||
registry := conversion.NewMemoryRegistry()
|
||||
engine := conversion.NewConversionEngine(registry)
|
||||
engine := conversion.NewConversionEngine(registry, nil)
|
||||
require.NoError(t, registry.Register(openai.NewAdapter()))
|
||||
|
||||
routingSvc := &mockProxyRoutingService{
|
||||
@@ -574,7 +530,7 @@ func TestProxyHandler_HandleStream_ConvertRequestError(t *testing.T) {
|
||||
|
||||
func TestProxyHandler_HandleNonStream_ConvertResponseError(t *testing.T) {
|
||||
registry := conversion.NewMemoryRegistry()
|
||||
engine := conversion.NewConversionEngine(registry)
|
||||
engine := conversion.NewConversionEngine(registry, nil)
|
||||
require.NoError(t, registry.Register(openai.NewAdapter()))
|
||||
require.NoError(t, registry.Register(anthropic.NewAdapter()))
|
||||
|
||||
@@ -636,7 +592,7 @@ func TestProxyHandler_HandleNonStream_ResponseHeaders(t *testing.T) {
|
||||
|
||||
func TestProxyHandler_ForwardPassthrough_CrossProtocol(t *testing.T) {
|
||||
registry := conversion.NewMemoryRegistry()
|
||||
engine := conversion.NewConversionEngine(registry)
|
||||
engine := conversion.NewConversionEngine(registry, nil)
|
||||
require.NoError(t, registry.Register(openai.NewAdapter()))
|
||||
|
||||
anthropicAdapter := anthropic.NewAdapter()
|
||||
@@ -759,3 +715,119 @@ func TestProxyHandler_HandleProxy_RouteEmptyBody_NoModel(t *testing.T) {
|
||||
h.HandleProxy(c)
|
||||
assert.Equal(t, 200, w.Code)
|
||||
}
|
||||
|
||||
// ============ extractModelName 测试 ============
|
||||
|
||||
func TestExtractModelName(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
body []byte
|
||||
expected string
|
||||
}{
|
||||
{
|
||||
name: "valid model",
|
||||
body: []byte(`{"model": "gpt-4", "messages": []}`),
|
||||
expected: "gpt-4",
|
||||
},
|
||||
{
|
||||
name: "empty body",
|
||||
body: []byte(`{}`),
|
||||
expected: "",
|
||||
},
|
||||
{
|
||||
name: "invalid json",
|
||||
body: []byte(`{invalid}`),
|
||||
expected: "",
|
||||
},
|
||||
{
|
||||
name: "nested structure",
|
||||
body: []byte(`{"model": "claude-3", "messages": [{"role": "user", "content": "hello"}]}`),
|
||||
expected: "claude-3",
|
||||
},
|
||||
{
|
||||
name: "model with special chars",
|
||||
body: []byte(`{"model": "gpt-4-0125-preview", "stream": true}`),
|
||||
expected: "gpt-4-0125-preview",
|
||||
},
|
||||
{
|
||||
name: "empty body bytes",
|
||||
body: []byte{},
|
||||
expected: "",
|
||||
},
|
||||
{
|
||||
name: "model is null",
|
||||
body: []byte(`{"model": null}`),
|
||||
expected: "",
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
result := extractModelName(tt.body)
|
||||
assert.Equal(t, tt.expected, result)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// ============ isStreamRequest 测试 ============
|
||||
|
||||
func TestIsStreamRequest(t *testing.T) {
|
||||
engine := setupProxyEngine(t)
|
||||
h := &ProxyHandler{engine: engine}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
body []byte
|
||||
clientProtocol string
|
||||
nativePath string
|
||||
expected bool
|
||||
}{
|
||||
{
|
||||
name: "stream true",
|
||||
body: []byte(`{"model": "gpt-4", "stream": true}`),
|
||||
clientProtocol: "openai",
|
||||
nativePath: "/v1/chat/completions",
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "stream false",
|
||||
body: []byte(`{"model": "gpt-4", "stream": false}`),
|
||||
clientProtocol: "openai",
|
||||
nativePath: "/v1/chat/completions",
|
||||
expected: false,
|
||||
},
|
||||
{
|
||||
name: "no stream field",
|
||||
body: []byte(`{"model": "gpt-4"}`),
|
||||
clientProtocol: "openai",
|
||||
nativePath: "/v1/chat/completions",
|
||||
expected: false,
|
||||
},
|
||||
{
|
||||
name: "invalid json",
|
||||
body: []byte(`{invalid}`),
|
||||
clientProtocol: "openai",
|
||||
nativePath: "/v1/chat/completions",
|
||||
expected: false,
|
||||
},
|
||||
{
|
||||
name: "not chat endpoint",
|
||||
body: []byte(`{"model": "gpt-4", "stream": true}`),
|
||||
clientProtocol: "openai",
|
||||
nativePath: "/v1/models",
|
||||
expected: false,
|
||||
},
|
||||
{
|
||||
name: "anthropic stream",
|
||||
body: []byte(`{"model": "claude-3", "stream": true}`),
|
||||
clientProtocol: "anthropic",
|
||||
nativePath: "/v1/messages",
|
||||
expected: true,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
result := h.isStreamRequest(tt.body, tt.clientProtocol, tt.nativePath)
|
||||
assert.Equal(t, tt.expected, result)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package handler
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"time"
|
||||
|
||||
@@ -23,31 +24,16 @@ func NewStatsHandler(statsService service.StatsService) *StatsHandler {
|
||||
func (h *StatsHandler) GetStats(c *gin.Context) {
|
||||
providerID := c.Query("provider_id")
|
||||
modelName := c.Query("model_name")
|
||||
startDateStr := c.Query("start_date")
|
||||
endDateStr := c.Query("end_date")
|
||||
|
||||
var startDate, endDate *time.Time
|
||||
|
||||
if startDateStr != "" {
|
||||
t, err := time.Parse("2006-01-02", startDateStr)
|
||||
if err != nil {
|
||||
c.JSON(http.StatusBadRequest, gin.H{
|
||||
"error": "无效的 start_date 格式,应为 YYYY-MM-DD",
|
||||
})
|
||||
return
|
||||
}
|
||||
startDate = &t
|
||||
startDate, err := parseDateParam(c, "start_date")
|
||||
if err != nil {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
|
||||
if endDateStr != "" {
|
||||
t, err := time.Parse("2006-01-02", endDateStr)
|
||||
if err != nil {
|
||||
c.JSON(http.StatusBadRequest, gin.H{
|
||||
"error": "无效的 end_date 格式,应为 YYYY-MM-DD",
|
||||
})
|
||||
return
|
||||
}
|
||||
endDate = &t
|
||||
endDate, err := parseDateParam(c, "end_date")
|
||||
if err != nil {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
|
||||
stats, err := h.statsService.Get(providerID, modelName, startDate, endDate)
|
||||
@@ -65,32 +51,17 @@ func (h *StatsHandler) GetStats(c *gin.Context) {
|
||||
func (h *StatsHandler) AggregateStats(c *gin.Context) {
|
||||
providerID := c.Query("provider_id")
|
||||
modelName := c.Query("model_name")
|
||||
startDateStr := c.Query("start_date")
|
||||
endDateStr := c.Query("end_date")
|
||||
groupBy := c.Query("group_by")
|
||||
|
||||
var startDate, endDate *time.Time
|
||||
|
||||
if startDateStr != "" {
|
||||
t, err := time.Parse("2006-01-02", startDateStr)
|
||||
if err != nil {
|
||||
c.JSON(http.StatusBadRequest, gin.H{
|
||||
"error": "无效的 start_date 格式,应为 YYYY-MM-DD",
|
||||
})
|
||||
return
|
||||
}
|
||||
startDate = &t
|
||||
startDate, err := parseDateParam(c, "start_date")
|
||||
if err != nil {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
|
||||
if endDateStr != "" {
|
||||
t, err := time.Parse("2006-01-02", endDateStr)
|
||||
if err != nil {
|
||||
c.JSON(http.StatusBadRequest, gin.H{
|
||||
"error": "无效的 end_date 格式,应为 YYYY-MM-DD",
|
||||
})
|
||||
return
|
||||
}
|
||||
endDate = &t
|
||||
endDate, err := parseDateParam(c, "end_date")
|
||||
if err != nil {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
|
||||
stats, err := h.statsService.Get(providerID, modelName, startDate, endDate)
|
||||
@@ -104,3 +75,16 @@ func (h *StatsHandler) AggregateStats(c *gin.Context) {
|
||||
result := h.statsService.Aggregate(stats, groupBy)
|
||||
c.JSON(http.StatusOK, result)
|
||||
}
|
||||
|
||||
// parseDateParam 解析日期查询参数
|
||||
func parseDateParam(c *gin.Context, paramName string) (*time.Time, error) {
|
||||
dateStr := c.Query(paramName)
|
||||
if dateStr == "" {
|
||||
return nil, nil
|
||||
}
|
||||
t, err := time.Parse("2006-01-02", dateStr)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("无效的 %s 格式,应为 YYYY-MM-DD", paramName)
|
||||
}
|
||||
return &t, nil
|
||||
}
|
||||
|
||||
61
backend/internal/handler/stats_handler_test.go
Normal file
61
backend/internal/handler/stats_handler_test.go
Normal file
@@ -0,0 +1,61 @@
|
||||
package handler
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestParseDateParam(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
|
||||
t.Run("valid_date", func(t *testing.T) {
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest(http.MethodGet, "/?start_date=2024-01-15", nil)
|
||||
|
||||
result, err := parseDateParam(c, "start_date")
|
||||
assert.NoError(t, err)
|
||||
assert.NotNil(t, result)
|
||||
expected := time.Date(2024, 1, 15, 0, 0, 0, 0, time.UTC)
|
||||
assert.Equal(t, expected, *result)
|
||||
})
|
||||
|
||||
t.Run("empty_param", func(t *testing.T) {
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
|
||||
result, err := parseDateParam(c, "start_date")
|
||||
assert.NoError(t, err)
|
||||
assert.Nil(t, result)
|
||||
})
|
||||
|
||||
t.Run("invalid_format", func(t *testing.T) {
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest(http.MethodGet, "/?start_date=2024/01/15", nil)
|
||||
|
||||
result, err := parseDateParam(c, "start_date")
|
||||
assert.Error(t, err)
|
||||
assert.Nil(t, result)
|
||||
assert.Contains(t, err.Error(), "start_date")
|
||||
assert.Contains(t, err.Error(), "YYYY-MM-DD")
|
||||
})
|
||||
|
||||
t.Run("end_date", func(t *testing.T) {
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest(http.MethodGet, "/?end_date=2024-12-31", nil)
|
||||
|
||||
result, err := parseDateParam(c, "end_date")
|
||||
assert.NoError(t, err)
|
||||
assert.NotNil(t, result)
|
||||
expected := time.Date(2024, 12, 31, 0, 0, 0, 0, time.UTC)
|
||||
assert.Equal(t, expected, *result)
|
||||
})
|
||||
}
|
||||
Reference in New Issue
Block a user