-
Notifications
You must be signed in to change notification settings - Fork 9
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.
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
feat: add cache and optimize logic in user.GetInfo and term.TermList #170
Conversation
怎么感觉怪怪的,你开 commit 签名了吗 |
昨天token过期了 |
Codecov ReportAttention: Patch coverage is @@ Coverage Diff @@
## main #170 +/- ##
========================================
+ Coverage 3.21% 3.31% +0.09%
========================================
Files 200 209 +9
Lines 41224 41359 +135
========================================
+ Hits 1327 1371 +44
- Misses 39818 39905 +87
- Partials 79 83 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
图里面几个过期时间设置的时间写在 pr 里哈 |
这个pr可以合了吗 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 11 out of 26 changed files in this pull request and generated no comments.
Files not reviewed (15)
- cmd/common/main.go: Evaluated as low risk
- pkg/cache/course/course.go: Evaluated as low risk
- pkg/cache/common/set_term_list.go: Evaluated as low risk
- pkg/cache/common/common.go: Evaluated as low risk
- pkg/cache/cache.go: Evaluated as low risk
- internal/user/service/service.go: Evaluated as low risk
- internal/common/service/service.go: Evaluated as low risk
- internal/course/service/service.go: Evaluated as low risk
- cmd/user/main.go: Evaluated as low risk
- cmd/course/main.go: Evaluated as low risk
- internal/user/service/get_info_test.go: Evaluated as low risk
- internal/course/service/get_terms.go: Evaluated as low risk
- internal/course/service/get_terms_test.go: Evaluated as low risk
- internal/common/service/term.go: Evaluated as low risk
- internal/user/service/get_info.go: Evaluated as low risk
internal/common/service/term.go
Outdated
"github.com/west2-online/jwch" | ||
) | ||
|
||
func (s *CommonService) GetTermList() (*jwch.SchoolCalendar, error) { | ||
exist := s.cache.IsKeyExist(s.ctx, constants.TermListKey) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个改成 if s.cache.IsKeyExist(s.ctx, constants.TermListKey) {}
就可以
internal/course/service/get_terms.go
Outdated
@@ -37,12 +37,23 @@ func (s *CourseService) GetTermsList(req *course.TermListRequest) ([]string, err | |||
return nil, fmt.Errorf("service.GetTermList: Get login data fail: %w", err) | |||
} | |||
|
|||
stu := jwch.NewStudent().WithLoginData(loginData.GetId(), utils.ParseCookies(loginData.GetCookies())) | |||
e := s.cache.IsKeyExist(s.ctx, loginData.GetId()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
同上
// 查询数据库是否存入此学生信息 | ||
// 查询cache | ||
existCache := s.cache.IsKeyExist(s.ctx, stuId) | ||
if existCache { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
同上
exist, stuInfo, err := s.db.User.GetStudentById(s.ctx, stuId) | ||
IsUpdate := false | ||
if err != nil { | ||
return nil, fmt.Errorf("service.GetUserInfo: %w", err) | ||
} | ||
if exist { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个不用删掉 exist,因为逻辑比前面那个复杂
自查 PR 结构
PR 标题符合这个格式: <type>(optional scope): <description>
此 PR 标题的描述以用户为导向,足够清晰,其他人可以理解。
我已经对所有 commit 提供了签名(GPG 密钥签名、SSH 密钥签名)
这个 PR 属于强制变更/破坏性更改
这个 PR 的类型是什么?
feat && fix
这个 PR 做了什么 / 我们为什么需要这个 PR?
(可选)这个 PR 解决了哪个/些 issue?
stuInfo(#149)
对 Reviewer 预留的一些提醒