Conversation
📝 WalkthroughWalkthrough注入并使用 ModelPriceRepository,扩展备份数据结构以包含模型价格与 Provider.logo,更新备份导出/导入逻辑以处理 modelPrices 并增强冲突检测与导入顺序,同时新增相关单元/集成测试并在启动/路由处完成依赖连接与路径修正。 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AdminHandler as Admin Handler
participant BackupSvc as Backup Service
participant ProviderRepo as Provider Repo
participant ModelPriceRepo as ModelPrice Repo
participant Storage as DB/File
Client->>AdminHandler: POST /admin/backup/import or GET /admin/backup/export
AdminHandler->>BackupSvc: Import(payload)/Export()
BackupSvc->>ProviderRepo: load/save providers (include logo)
BackupSvc->>ModelPriceRepo: load/save modelPrices
BackupSvc->>Storage: persist other entities (routes, mappings, tokens...)
ProviderRepo-->>BackupSvc: providers data
ModelPriceRepo-->>BackupSvc: modelPrices data
BackupSvc-->>AdminHandler: export payload / import result
AdminHandler-->>Client: HTTP response (file or status)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@internal/service/backup.go`:
- Around line 934-947: The current buildModelMappingKey uses "|" which can
collide if any mapping field (e.g., ProviderName, Pattern, Target) contains that
character; change buildModelMappingKey to produce an unambiguous deterministic
key by serializing the relevant fields (Scope, ClientType, ProviderType,
ProviderName, ProjectSlug, RouteName, APITokenName, Pattern, Target, Priority)
and then hashing the serialization (e.g., SHA256 hex) or alternatively
JSON-marshal the struct and use that string as the key; update
buildModelMappingKey to assemble the selected fields into a struct or slice,
marshal it (or hash the marshaled bytes) and return that as the key so embedded
'|' cannot cause collisions.
🧹 Nitpick comments (3)
internal/handler/admin.go (1)
103-113: 尾部斜杠修复正确,解决了/admin/providers/import/和/admin/providers/export/的路由问题。在
HasSuffix检查之前先TrimSuffix去除尾部/,使得带尾部斜杠的请求路径能正确匹配/export和/import后缀。不过请注意,其他类似的子路由处理函数(如
handleModelMappings第 1129 行、handleUsageStats第 1267 行、handleModelPrices第 1527 行)仍然直接使用r.URL.Path做HasSuffix检查,没有做相同的尾部斜杠规范化处理,存在同样的潜在问题。可以考虑统一处理。,
♻️ 建议对其他 handler 应用相同的尾部斜杠修复
// handleModelMappings - path := r.URL.Path + path := strings.TrimSuffix(r.URL.Path, "/")// handleUsageStats - path := r.URL.Path + path := strings.TrimSuffix(r.URL.Path, "/")// handleModelPrices - path := r.URL.Path + path := strings.TrimSuffix(r.URL.Path, "/")internal/handler/admin_import_export_test.go (1)
59-79: 构造函数参数脆弱:大量nil占位参数难以维护。
NewAdminService使用了 18 个参数,其中大部分为nil。如果构造函数签名再次变化(如本 PR 添加modelPriceRepo时),此处需要同步修改且编译器提示不够直观。建议后续考虑使用 options struct 或在测试中添加注释标注每个nil对应的参数名。internal/service/backup.go (1)
319-322: 反向查找 map 的构建可与上方循环合并。
providerIDToName、projectIDToSlug、apiTokenIDToName分别用了独立循环,但上方已有遍历同一 slice 的循环(如 316-318、329-331、351-353)。可将两个循环合并以减少冗余遍历。♻️ 示例:合并 provider 循环
+ providerIDToName := make(map[uint64]string, len(providers)) for _, p := range providers { ctx.providerNameToID[p.Name] = p.ID + providerIDToName[p.ID] = p.Name } - providerIDToName := make(map[uint64]string, len(providers)) - for _, p := range providers { - providerIDToName[p.ID] = p.Name - }Also applies to: 332-335, 354-357
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
cmd/maxx/main.gointernal/core/database.gointernal/domain/backup.gointernal/handler/admin.gointernal/handler/admin_import_export_test.gointernal/repository/sqlite/models.gointernal/repository/sqlite/provider.gointernal/service/backup.gointernal/service/backup_test.goweb/src/lib/transport/types.ts
🧰 Additional context used
🧬 Code graph analysis (4)
internal/domain/backup.go (1)
web/src/lib/transport/types.ts (1)
BackupModelPrice(822-835)
internal/repository/sqlite/provider.go (1)
internal/repository/sqlite/models.go (2)
LongText(17-17)LongText(20-27)
internal/service/backup_test.go (6)
internal/service/backup.go (2)
BackupService(13-24)NewBackupService(27-51)internal/repository/sqlite/provider.go (1)
NewProviderRepository(15-17)internal/repository/sqlite/system_setting.go (1)
NewSystemSettingRepository(16-18)internal/repository/sqlite/model_price.go (1)
NewModelPriceRepository(15-17)internal/domain/model.go (2)
ClientTypeOpenAI(15-15)ClientTypeClaude(12-12)internal/domain/backup.go (1)
ImportOptions(122-125)
web/src/lib/transport/types.ts (1)
internal/domain/backup.go (1)
BackupModelPrice(106-119)
🔇 Additional comments (17)
internal/domain/backup.go (2)
105-119: BackupModelPrice 结构体与前端类型定义一致,设计合理。新增的
BackupModelPrice结构体字段与前端web/src/lib/transport/types.ts中的BackupModelPrice接口完全对齐,JSON tag 命名一致。omitempty在BackupData.ModelPrices上的使用保证了与旧版备份文件的向后兼容性。
26-26: 新增字段向后兼容,LGTM。
ModelPrices和Logo均使用了omitempty,旧版备份文件在反序列化时不会出错。BackupVersion保持"1.0"不变是合理的,因为这些都是纯增量的可选字段。Also applies to: 39-39
internal/core/database.go (1)
336-347:ModelPriceRepo注入 BackupService,接线正确。
repos.ModelPriceRepo作为新参数传入NewBackupService,与backup.go中NewBackupService构造函数签名一致。,
AI 摘要声称
NewRouter签名也更新以包含ModelPriceRepository,但实际代码(第 237-243 行)中NewRouter仍然只接收 5 个参数,并未包含ModelPriceRepo。internal/repository/sqlite/provider.go (1)
85-85: Logo 字段的持久化实现正确,与现有模式一致。
LongText类型适合存储可能较大的 Logo 数据(如 base64 编码的 data URI),直接类型转换(无需 JSON 序列化/反序列化)对于纯字符串字段是正确的做法。Also applies to: 101-101
internal/repository/sqlite/models.go (1)
67-67: LGTM!
Logo LongText字段添加与 Provider 结构体中的其他LongText字段(Config、SupportedClientTypes、SupportModels)风格一致,GORM AutoMigrate 会自动处理新列的添加。cmd/maxx/main.go (1)
287-298: BackupService 接线与internal/core/database.go保持一致,LGTM。
modelPriceRepo在两个入口点(独立二进制cmd/maxx/main.go和internal/core/database.go)中均正确传入NewBackupService,确保了备份功能在不同启动模式下的一致性。internal/service/backup_test.go (3)
167-227: 测试覆盖全面,验证了关键的备份往返保真性。测试覆盖了完整的 export → import → re-export 流程,并验证了以下关键数据的保真性:
- Provider Logo
- ModelPrice(包括具体字段值)
- API Token(包括明文 token)
- ModelMapping(包括路由引用)
这组测试能有效防止备份数据丢失的回归问题。
229-251: 重复检测测试逻辑清晰。向同一数据库导入已存在的数据并验证
Skipped > 0,是验证ConflictStrategy: "skip"对 model mappings 生效的有效方式。
26-41: 无需修改 —adapterRefresher已有空值保护。第 508 行已存在
if s.adapterRefresher != nil {的保护,在调用RefreshAdapter()前进行了空值检查。测试传入nil是安全的,不会导致 panic。internal/handler/admin_import_export_test.go (2)
84-116: Import 测试逻辑清晰,LGTM。测试覆盖了 trailing slash 路由、响应状态码、导入计数和仓库状态,断言合理。
118-150: Export 测试逻辑清晰,LGTM。验证了 trailing slash 路由、
Content-Dispositionheader 和响应体内容,覆盖到位。web/src/lib/transport/types.ts (2)
822-835:BackupModelPrice接口与 Go 后端domain.BackupModelPrice字段完全对齐,LGTM。12 个字段及其类型均与后端 JSON tag 一致。
751-751:BackupData和BackupProvider扩展合理,LGTM。
modelPrices和logo均为可选字段,保持了向后兼容。Also applies to: 762-762
internal/service/backup.go (4)
238-258: ModelPrices 导出逻辑清晰,LGTM。使用
ListCurrentPrices获取有效价格并逐字段映射到BackupModelPrice,与其他实体的导出模式一致。
850-928:importModelPrices实现完整,冲突策略覆盖全面,LGTM。skip/error/overwrite 三种策略均正确处理,overwrite 保留了原记录的 ID 和 CreatedAt,新增记录也同步写入
existingByModelID以防同一批次内重复。
373-392: 新增的 model mapping 冲突检测逻辑合理。通过加载现有 mappings 并构建 composite key,在导入时实现了重复检测。与 route 冲突检测的模式一致。
764-779: Model mapping 导入冲突检测和批次内去重均已覆盖,LGTM。导入前检查
modelMappingKeys,导入成功后将 key 加入集合(line 843),有效防止同一备份文件中的重复条目。Also applies to: 843-843
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
fc1cba9 to
7f5a1e5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@internal/service/backup.go`:
- Around line 964-967: buildModelMappingKey currently swallows json.Marshal
errors and returns an empty string, causing distinct mappings to collide in
modelMappingKeys; update buildModelMappingKey to surface failures instead of
returning "" — either change its signature to return (string, error) and
propagate the error to callers (so callers of buildModelMappingKey can
handle/fail explicitly) or, if this truly is an unrecoverable invariant, replace
the empty-string return with a panic that includes the payload and marshal
error; ensure all call sites that populate modelMappingKeys handle the new error
or accept the panic approach so collisions/hidden skips no longer occur.
🧹 Nitpick comments (1)
internal/service/backup.go (1)
933-935:buildRouteKey使用:分隔符,与 Export 中RouteName的拼接格式一致,但存在理论冲突风险。若
providerName或projectSlug含:,可能产生 key 碰撞(例如"a:b"+"openai"+""与"a"+"b:openai"+"")。当前 Export(Line 230-233)也使用相同格式写入RouteName,因此导入导出是一致的。如果 provider/project 命名有校验限制则无需改动,否则可考虑与buildModelMappingKey同样采用 JSON+hash 方式。
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/service/backup.gointernal/service/backup_test.go
🧰 Additional context used
🧬 Code graph analysis (2)
internal/service/backup.go (3)
internal/repository/interfaces.go (1)
ModelPriceRepository(246-269)internal/domain/backup.go (5)
BackupModelPrice(106-119)BackupModelMapping(92-103)ImportOptions(122-125)ImportResult(135-140)ImportSummary(128-132)internal/domain/model.go (3)
ClientType(9-9)ModelPrice(515-534)ModelMappingScope(743-743)
internal/service/backup_test.go (2)
internal/service/backup.go (2)
BackupService(16-27)NewBackupService(30-54)internal/domain/backup.go (2)
ImportOptions(122-125)BackupModelMapping(92-103)
🔇 Additional comments (9)
internal/service/backup.go (5)
241-261: ModelPrices 导出逻辑清晰,LGTM。字段映射完整,与
BackupModelPrice结构对齐,错误处理一致。
853-931:importModelPrices实现完整,支持 skip/overwrite/error 三种策略。有一个小细节值得注意:overwrite 时使用
existing.ID和existing.CreatedAt保留原记录标识,新建时将 price 加入existingByModelID避免同批次重复——逻辑正确。
376-395: 加载现有 ModelMapping 用于冲突检测的逻辑合理。通过构造
BackupModelMapping并调用buildModelMappingKey生成一致的 key,与导入时的比较逻辑对称。注意:当ProviderID/ProjectID/RouteID/APITokenID为 0 时,对应的 reverse lookup map 会返回零值空字符串,这与BackupModelMapping中omitempty字段为空的行为一致,不会导致 key 不匹配。
112-119: Provider 导出现已包含Logo字段,导入端(Line 498)也同步设置——roundtrip 对称性良好。
766-782: ModelMapping 冲突检测:导入新 mapping 后也同步更新ctx.modelMappingKeys(Line 846),确保同批次内的重复也能检测到。internal/service/backup_test.go (4)
167-227: Roundtrip 测试覆盖了核心新增功能(Logo、ModelPrices、Token 原值还原、ModelMapping 路由引用),LGTM。可选改进:考虑补充对
Routes、Projects、RetryConfigs、SystemSettings等实体数量和关键字段的断言,以增强回归保障。
253-278: Key 碰撞测试有效验证了先前|分隔符冲突已修复。
12-41: 测试辅助函数简洁清晰,t.Cleanup确保 DB 资源释放。
229-251: 重复导入跳过测试验证了 ModelMapping 冲突检测在 "skip" 策略下的行为。注意该测试依赖于所有上游实体(Provider、Project 等)也被 skip,从而使
ctx中的 name→ID 映射来自loadExistingMappings。这正好验证了loadExistingMappings中新增的 reverse lookup 逻辑的正确性。
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| encoded, err := json.Marshal(payload) | ||
| if err != nil { | ||
| return "" | ||
| } |
There was a problem hiding this comment.
buildModelMappingKey 在序列化失败时返回空字符串,可能导致误判冲突。
json.Marshal 对纯基础类型结构体几乎不会失败,但若返回 "",多个不同 mapping 会被视为同一 key,在 modelMappingKeys 中产生假冲突,导致数据被静默跳过或报错。建议在此处 panic 或返回 error,因为这属于"不应发生"的编程错误。
🛡️ 建议修改
encoded, err := json.Marshal(payload)
if err != nil {
- return ""
+ // 基础类型序列化不应失败;若发生则属编程错误
+ panic(fmt.Sprintf("buildModelMappingKey: marshal failed: %v", err))
}📝 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.
| encoded, err := json.Marshal(payload) | |
| if err != nil { | |
| return "" | |
| } | |
| encoded, err := json.Marshal(payload) | |
| if err != nil { | |
| // 基础类型序列化不应失败;若发生则属编程错误 | |
| panic(fmt.Sprintf("buildModelMappingKey: marshal failed: %v", err)) | |
| } |
🤖 Prompt for AI Agents
In `@internal/service/backup.go` around lines 964 - 967, buildModelMappingKey
currently swallows json.Marshal errors and returns an empty string, causing
distinct mappings to collide in modelMappingKeys; update buildModelMappingKey to
surface failures instead of returning "" — either change its signature to return
(string, error) and propagate the error to callers (so callers of
buildModelMappingKey can handle/fail explicitly) or, if this truly is an
unrecoverable invariant, replace the empty-string return with a panic that
includes the payload and marshal error; ensure all call sites that populate
modelMappingKeys handle the new error or accept the panic approach so
collisions/hidden skips no longer occur.
|
已处理这条 review comment,修复如下:
这样即使字段内包含 另外补充了回归测试:
验证:
已推送到当前 PR 分支。 |
变更摘要
provider.logo与modelPrices的导入导出验证
go test ./internal/handler -run TestAdminHandler_ProvidersImport_WithTrailingSlash -count=1go test ./internal/handler -run TestAdminHandler_ProvidersExport_WithTrailingSlash -count=1go test ./internal/service -run TestBackupService_ -count=1go test ./...Summary by CodeRabbit
发布说明
新功能
问题修复
测试