Skip to content

feat: 完善推送能力#399

Merged
renbaoshuo merged 13 commits intomainfrom
fix/push
Mar 6, 2026
Merged

feat: 完善推送能力#399
renbaoshuo merged 13 commits intomainfrom
fix/push

Conversation

@linrongda
Copy link
Member

@linrongda linrongda commented Feb 4, 2026

自查 PR 结构

  • PR 标题符合这个格式: <type>(optional scope): <description>

  • 此 PR 标题的描述以用户为导向,足够清晰,其他人可以理解。

  • 我已经对所有 commit 提供了签名(GPG 密钥签名、SSH 密钥签名)

  • 这个 PR 属于强制变更/破坏性更改

如果是,请在 PR 标题中添加 BREAKING CHANGE 前缀,并在 PR 描述中详细说明。

这个 PR 的类型是什么?

feat

这个 PR 做了什么 / 我们为什么需要这个 PR?

  • 增加厂商推送
  • 完善限流措施

(可选)这个 PR 解决了哪个/些 issue?

对 Reviewer 预留的一些提醒

@codecov
Copy link

codecov bot commented Feb 4, 2026

Codecov Report

❌ Patch coverage is 78.68852% with 26 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/umeng/groupcast.go 61.90% 24 Missing ⚠️
internal/academic/service/get_scores.go 66.66% 1 Missing and 1 partial ⚠️
@@            Coverage Diff             @@
##             main     #399      +/-   ##
==========================================
+ Coverage   62.65%   63.64%   +0.98%     
==========================================
  Files         209      210       +1     
  Lines        5278     5328      +50     
==========================================
+ Hits         3307     3391      +84     
+ Misses       1869     1833      -36     
- Partials      102      104       +2     
Flag Coverage Δ
unittest 63.64% <78.68%> (+0.98%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
internal/course/service/get_course_list.go 83.89% <ø> (-1.36%) ⬇️
pkg/umeng/dispatcher.go 100.00% <100.00%> (ø)
internal/academic/service/get_scores.go 82.50% <66.66%> (-0.57%) ⬇️
pkg/umeng/groupcast.go 45.83% <61.90%> (+45.83%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@linrongda linrongda marked this pull request as ready for review March 3, 2026 05:53
Copilot AI review requested due to automatic review settings March 3, 2026 05:53
@linrongda linrongda requested review from a team, mutezebra and ozline as code owners March 3, 2026 05:53
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the Umeng push notification capability by introducing support for multiple vendor push channels (OPPO, VIVO, Xiaomi, Huawei, etc.), splitting the Policy struct into AndroidPolicy and IOSPolicy, and replacing synchronous in-line rate limiting (time.Sleep) with a new asynchronous single-goroutine dispatcher (asyncDispatcher) that enforces both per-message interval and daily quotas globally.

Changes:

  • Async dispatcher with rate limiting: pkg/umeng/dispatcher.go introduces a singleton asyncDispatcher with a queue, per-message interval control (1 min), and a daily send quota (500/day), replacing the previous blocking time.Sleep approach.
  • Extended model and groupcast API: pkg/umeng/model.go and groupcast.go add new vendor channel properties (OPPO private templates, Huawei, VIVO, Xiaomi, local channel), split Policy into AndroidPolicy/IOSPolicy, and remove app key/secret parameters from public API (now read from config).
  • Updated callers: internal/academic/service/get_scores.go, internal/course/service/get_course_list.go, and cmd/common/main.go are updated to use EnqueueAsync and the new function signatures.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
pkg/umeng/dispatcher.go New async dispatcher with queue, interval, and daily-limit rate control
pkg/umeng/model.go Added vendor channel properties structs, split Policy into Android/iOS variants
pkg/umeng/groupcast.go Updated send functions to read config directly; added vendor channel properties
pkg/constants/umeng.go New constants for queue size, daily limit; rate limit delay raised to 1 min
config/types.go New oppo, huawei, localProperties, and updated vendors config structs
internal/academic/service/get_scores.go Switched to EnqueueAsync; simplified sendNotifications
internal/academic/service/get_scores_test.go Updated test tag to be long enough for tag[:12] slicing
internal/course/service/get_course_list.go Switched to EnqueueAsync; simplified sendNotifications
cmd/common/main.go Switched notice push to EnqueueAsync

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ozline
Copy link
Member

ozline commented Mar 4, 2026


Code Review: fix/push 分支

本次 PR 的核心目标是将友盟推送从同步阻塞 + time.Sleep 限流改造为异步队列 + 调度器限流,并适配 OPPO/荣耀/vivo/小米/华为的私信渠道配置。整体方向正确,架构改进明显。


  1. sendGroupcast 创建无超时 HTTP 客户端
  • 优先级:重要 Major
  • 位置:pkg/umeng/groupcast.go:183
  • 原因:client := &http.Client{} 没有设置 Timeout,Go 默认的 http.Client 超时为无限制。在 asyncDispatcher 的串行消费模式下,一旦友盟 API
    出现网络问题或响应超慢,run() 协程将永久阻塞在该请求上,导致整个推送队列后续所有任务全部堆积,直到 ch 满后所有新通知被静默丢弃。
  • 建议:
    // 在 package 级别定义共享客户端
    var httpClient = &http.Client{Timeout: 15 * time.Second}
  • 同时将 sendGroupcast 中的 client := &http.Client{} 替换为 httpClient,既解决超时问题,又通过连接复用提升性能。
  • 影响:友盟 API 一次超时即可导致后续所有推送任务全部阻塞并丢失。

  1. OppoPrivateMsgTemplate 在未配置时仍被序列化
  • 优先级:重要 Major
  • 位置:pkg/umeng/model.go:69,pkg/umeng/groupcast.go:45-53
  • 原因:AndroidChannelProperties 中的 OppoPrivateMsgTemplate 字段是嵌套结构体,没有 omitempty。当 OPPO 私信渠道未配置时(PrivateMsgTemplateID
    为空字符串),该字段依然会被序列化为:
    "oppo_private_msg_template": {
    "private_msg_template_id": "",
    "private_title_parameters": {"title": "xxx"},
    "private_content_parameters": {"content": ""}
    }
  • 友盟 API 可能因收到空 template ID 而拒绝请求或触发未定义行为。同样的问题存在于 LocalProperties、ChannelActivity 等字段——空字符串会覆盖厂商默认配置。
  • 建议:对嵌套对象使用指针 + omitempty,或在 getChannelProperties 中判断后条件赋值:
    // 方案一:指针 + omitempty
    OppoPrivateMsgTemplate *OppoPrivateMsgTemplate json:"oppo_private_msg_template,omitempty"

// 方案二:条件构造(推荐,无需改动 model)
if config.Vendors.Oppo.PrivateMsgTemplate.PrivateMsgTemplateID != "" {
props.OppoPrivateMsgTemplate = &OppoPrivateMsgTemplate{...}
}

  • 同时,ChannelActivity、XiaoMiChannelID 等字符串字段也应加 omitempty,避免空值干扰厂商通道。
  • 影响:可能导致部分厂商通道的推送请求被拒绝或行为异常。

  1. sendNotifications 始终返回 nil,错误被静默吞掉
  • 优先级:重要 Major
  • 位置:internal/academic/service/get_scores.go:203-215
  • 原因:sendNotifications 函数即使 Android 或 iOS 推送均失败,仍然 return nil。包装它的 EnqueueAsync 闭包:
    if ok := umeng.EnqueueAsync(func() error {
    return s.sendNotifications(scores[i].Name, tag)
    }); !ok {
  • 永远不会触发 dispatcher.run() 中的失败日志 "umeng async task failed",失去了异步错误感知能力。cmd/common/main.go
    中的教务处通知推送也存在同样问题——两个发送失败后直接 return nil。
  • 建议:明确错误处理语义,统一返回最后一个错误(或用 errors.Join 合并多个错误),让 dispatcher 能感知失败:
    func (s *AcademicService) sendNotifications(courseName, tag string) error {
    var errs []error
    if err := umeng.SendAndroidGroupcastWithGoApp(...); err != nil {
    errs = append(errs, fmt.Errorf("android: %w", err))
    }
    if err := umeng.SendIOSGroupcast(...); err != nil {
    errs = append(errs, fmt.Errorf("ios: %w", err))
    }
    return errors.Join(errs...)
    }
  • 影响:推送失败无法被上层感知,调试困难,日志监控失效。

  1. Dispatcher 缺少优雅停机机制
  • 优先级:一般 Minor
  • 位置:pkg/umeng/dispatcher.go:565-572
  • 原因:run() 使用 for task := range d.ch 模式,只有在 ch 被关闭后才会退出。但目前没有任何地方关闭该 channel,也没有接受 context.Context 来响应程序停止信号。
  • 建议:暴露一个 Shutdown(ctx context.Context) error 方法,通过 close(d.ch) 触发 goroutine 退出,并等待当前任务执行完毕。或者在 getDispatcher()
    中注册到程序的生命周期管理(如 hertz 的 OnShutdown 钩子)。
  • 影响:程序优雅停机时,队列中已入队但未执行的通知会全部丢失;集成测试中会有 goroutine 泄漏。

  1. 全局单例 dispatcherOnce 导致单元测试隔离困难
  • 优先级:一般 Minor
  • 位置:pkg/umeng/dispatcher.go:499-516
  • 原因:sync.Once 保证了整个进程内只初始化一次 dispatcher,但也导致:1)跑多个测试时 dispatcher goroutine 一直在后台运行;2)无法在测试中注入不同配置的
    dispatcher(如更短的 interval 来加速测试)。
  • 建议:为 dispatcher.go 补充一个 _test.go 文件中可调用的 resetDispatcher() 函数(仅用于测试),或在 newAsyncDispatcher 参数上提供更好的可注入性。
  • 影响:测试存在隐性 goroutine 泄漏,CI 中使用 -race flag 时可能出现非预期报告。

  1. daily limit 超限等待后未重新检查 sameDay
  • 优先级:一般 Minor
  • 位置:pkg/umeng/dispatcher.go:587-593
  • 原因:当触发每日限额阻塞时:
    nextDay := time.Date(now.Year(), now.Month(), now.Day()+1, 0, 0, 0, 0, now.Location())
    time.Sleep(time.Until(nextDay))
    d.dailyCount = 0
    d.lastResetDate = time.Now()
  • 这里的 now 是函数入口处采样的时间。如果 time.Sleep 到达 nextDay 后,下一次 wait() 调用中 sameDay(time.Now(), d.lastResetDate)
    确实会检测到新的一天并重置,不会双重计数。但当前 wait() 调用直接进入了间隔检查,没有再走 sameDay 逻辑——这在逻辑上是正确的(因为已手动重置),但有些脆弱。若
    time.Sleep 精度偏差导致醒来时仍是同一天(极端场景),会触发 0 >= 500 的判断(dailyCount 已重置为 0),不会再次进入 daily limit 分支,所以实际上没有
    bug。但代码的意图不够清晰,建议加一行注释说明这里不需要重新走 sameDay 检查的原因。
  • 影响:无运行时问题,但逻辑脆弱,后续修改容易引入 bug。

  1. Category: 1 硬编码魔法数字
  • 优先级:建议 Suggestion
  • 位置:pkg/umeng/groupcast.go:90, 128
  • 原因:Category: 1 在两处广播函数中被硬编码,未使用命名常量。根据友盟文档,category 字段代表消息分类,不同值对应不同推送策略。
  • 建议:在 pkg/constants/umeng.go 中定义:
    UmengCategoryNormal = 1 // 普通消息
  • 并替换两处硬编码。
  • 影响:降低代码可读性,未来修改时容易遗漏。

  1. 删除了 sendGroupcast 成功日志,降低可观测性
  • 优先级:建议 Suggestion
  • 位置:pkg/umeng/groupcast.go(删除了原 logger.Infof("Groupcast sent successfully! MsgID: %s\n", response.Data.MsgID))
  • 原因:删除成功日志使得在排查推送问题时无法通过日志确认某条消息是否实际发出,以及其对应的 MsgID。
  • 建议:使用 Debug 级别保留此日志(而非 Info),既不污染生产日志,又保留调试能力:
    logger.Debugf("umeng groupcast sent, msg_id: %s", response.Data.MsgID)
  • 影响:降低问题排查效率。

  1. newAsyncDispatcher 参数注释与行为不一致
  • 优先级:建议 Suggestion
  • 位置:pkg/umeng/dispatcher.go:524
  • 原因:注释说"interval:发送最小间隔",但不像 queueSize 和 dailyLimit 有 <=0 使用默认值 的保护逻辑,interval 传入 0 会导致完全不限流(d.interval = 0,每次 elapsed

= 0 == interval,间隔检查恒为 false)。

  • 建议:补充对 interval <= 0 的默认值保护:
    if interval <= 0 {
    interval = constants.UmengRateLimitDelay
    }
  • 影响:调用方传入 0 间隔时行为符合预期(不限流),但与其他参数的防御性校验不一致,容易误用。

总结与总体评价

有部分重要问题需要修复后再合并。

本次 PR 的架构改进思路是正确的——用异步队列替代同步阻塞 + sleep,并统一了厂商推送配置的管理方式。代码注释完善,dispatcher.go 设计清晰。

核心需要修复的问题是:

  1. sendGroupcast 的 HTTP 无超时问题(Major)—— 可能导致整个推送队列永久阻塞
  2. OppoPrivateMsgTemplate 空值序列化问题(Major)—— 可能导致未配置私信渠道时请求被拒绝
  3. sendNotifications 错误静默吞掉(Major)—— 失去了异步错误感知能力

其余问题优先级较低,可在后续迭代中处理。

Copy link
Member

@renbaoshuo renbaoshuo left a comment

Choose a reason for hiding this comment

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

LGTM

@renbaoshuo renbaoshuo merged commit e75d927 into main Mar 6, 2026
10 checks passed
@renbaoshuo renbaoshuo deleted the fix/push branch March 6, 2026 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants