Conversation
- 创建统一的错误处理机制(domain/errors) - 创建日志接口和实现(infrastructure/logger) - 重构配置管理，使用依赖注入(infrastructure/config) - 添加配置验证逻辑
- 创建课程领域实体和仓储接口 - 创建摘要领域实体和仓储接口 - 创建用户领域实体和服务接口
- 实现课程和摘要的Repository - 实现外部服务(用户服务、课程服务、视频认证服务) - 使用新的日志接口替代全局变量
- 实现课程应用服务，协调领域对象 - 使用依赖注入传递仓储和日志
- 使用Gin框架重构路由系统 - 创建DTO和请求验证 - 实现统一的错误处理和响应格式 - 创建课程、摘要和健康检查处理器
- 使用依赖注入初始化所有组件 - 集成新的配置、日志和路由系统 - 更新数据库包使用新的config包
- 移除对全局Logger的依赖 - 使用依赖注入传递logger - 更新所有日志调用使用新的接口
- 使用loggerPkg别名避免命名冲突 - 修复所有logger.String函数调用
- 删除旧的handler、router、config和middleware/logger - 修复所有编译错误 - 修复导入冲突和未使用的导入 - 确保代码完全使用新架构
- 修复JSON解包后的类型断言，添加存在性和类型检查 - 修复liveCourseData字段提取的类型断言问题 - 修复url.Parse错误被忽略的问题 - 防止外部API响应异常时导致panic
- 将asr、cos、util迁移到infrastructure/external - 重构summary job使用新架构 - 删除internal/service、internal/util、internal/asr、internal/cos、internal/router目录
- Bug 1: 修复当Task为'new'且Asr不为空时asrText未初始化的问题 - Bug 2: 修复NewCOSService和NewASRService错误被忽略的问题 - Bug 3: 修复GetUserInfo未验证响应Code字段的问题 - 确保所有错误都被正确处理，防止nil指针panic
There was a problem hiding this comment.
Pull request overview
This pull request implements a major architectural refactoring of the application, transitioning from a simple layered architecture to a Domain-Driven Design (DDD) structure with clean architecture principles.
Key Changes:
- Architecture: Reorganized code into Domain, Application, Infrastructure, and Interfaces layers
- Web Framework: Migrated from net/http to Gin framework
- Logging: Replaced custom logger with a structured logger interface
- Error Handling: Introduced domain-specific error types with proper HTTP status mapping
- Configuration: Added validation and improved error handling for config loading
Reviewed changes
Copilot reviewed 52 out of 54 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
cmd/main.go |
Refactored entry point with dependency injection and new layer structure |
internal/infrastructure/config/config.go |
Added config validation and changed LoadConfig to return error |
internal/infrastructure/logger/logger.go |
New structured logger interface implementation |
internal/domain/* |
New domain layer with entities, repositories, and errors |
internal/application/* |
New application services layer |
internal/infrastructure/external/* |
Refactored external service integrations |
internal/infrastructure/persistence/* |
New repository implementations |
internal/interfaces/http/* |
New HTTP handlers using Gin framework |
internal/middleware/queue.go |
Updated queue middleware with logger injection |
go.mod |
Added Gin framework and dependencies |
Comments suppressed due to low confidence (5)
internal/infrastructure/config/config.go:99
- The Temperature field parsing logic is inconsistent with other fields. If the environment variable contains an invalid float value, the error is silently ignored and the default value is used. This could mask configuration errors. Consider returning an error or logging a warning when parsing fails.
internal/infrastructure/config/config.go:55 - The config.LoadConfig() function signature has changed to return an error, but this change is incompatible with existing code that may call this function without expecting an error return. This is a breaking API change that could cause issues for other parts of the codebase or external consumers.
internal/infrastructure/external/asr_service.go:81 - The ASR service creation error handling at line 78 returns an empty string instead of returning the error properly. This line should be "return "", errors.NewExternalError("asr", fmt.Errorf("task failed: %s", errorMsg))" to match the pattern used elsewhere.
internal/infrastructure/external/asr_service.go:28 - The error returned from NewASRService is ignored when creating the ASR client. If client creation fails, the function will panic later when attempting to use a nil client. The error should be checked and returned properly.
internal/infrastructure/config/config.go:135 - The database connection strings and sensitive credentials (OpenAI key, Tencent credentials) from environment variables are not validated for emptiness before use in the Validate function. While the Validate function checks these fields, there's a risk of using uninitialized or default values if environment variables are missing. The validation should happen immediately after loading config.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // 处理其他错误 | ||
| c.JSON(http.StatusInternalServerError, dto.ErrorResponse(http.StatusInternalServerError, "internal server error")) |
There was a problem hiding this comment.
The error response always returns "internal server error" for unhandled errors, which masks the actual error from the client. While this is good for security, the actual error should be logged before returning the generic message to aid in debugging.
| // 计算MD5 | ||
| parsedURL, err := url.Parse(courseEntity.Video) | ||
| if err != nil { | ||
| c.Error(errors.NewInternalError("failed to parse video URL", err)) | ||
| return | ||
| } | ||
| md5Input := fmt.Sprintf("%s%d%d%s%d", parsedURL.Path, userInfo.ID, userInfo.TenantID, reversedPhone, timestamp) | ||
| md5Hash := fmt.Sprintf("%x", md5.Sum([]byte(md5Input))) |
There was a problem hiding this comment.
MD5 is used for generating video authentication tokens. MD5 is cryptographically broken and should not be used for security-sensitive operations. Consider using SHA-256 or another secure hashing algorithm instead.
| } | ||
|
|
||
| type JobLoader func([]byte, *config.Config) (Job, error) | ||
| type JobLoader func([]byte, *config.Config, loggerPkg.Logger) (Job, error) |
There was a problem hiding this comment.
The JobLoader function signature has changed to include a logger parameter, which is a breaking change to the middleware API. Any external code registering job loaders will fail to compile. Consider documenting this breaking change or providing a migration guide.
| randIdx := rand.Intn(len(j.config.TencentSecretId)) | ||
| asrSvc, err := external.NewASRService(j.config.TencentSecretId[randIdx], j.config.TencentSecretKey[randIdx], j.logger) |
There was a problem hiding this comment.
Using rand.Intn without seeding could result in predictable selection of Tencent credentials. Consider using crypto/rand for security-sensitive operations or at least seed the random number generator. Additionally, selecting credentials randomly may lead to uneven load distribution across credential sets.
1 participant
No description provided.