已开启
优化批量查询接口GetReminderInfoByBundles,改为真实批量查询 #5036
优化批量查询接口GetReminderInfoByBundles,改为真实批量查询 #5036
已开启
luxin创建于 9月9日
luxin
luxin
9月9日

一、内容说明(相关的Issue)

二、建议测试周期和提测地址

建议测试完成时间:xxxx.xx.xx
投产上线时间:xxxx.xx.xx
提测地址:CI环境/压测环境
测试账号:

三、变更内容

  • 3.1 关联PR列表

  • 3.2 数据库和部署说明

    1. 常规更新
    2. 重启unicorn
    3. 重启sidekiq
    4. 迁移任务:是否有迁移任务,没有写 "无"
    5. rake脚本:bundle exec xxx RAILS_ENV = production;没有写 "无"
  • 3.4 其他技术优化内容(做了什么,变更了什么)

    • 重构了 xxxx 代码
    • xxxx 算法优化
  • 3.5 废弃通知(什么字段、方法弃用?)

  • 3.6 后向不兼容变更(是否有无法向后兼容的变更?)

四、研发自测点(自测哪些?冒烟用例全部自测?)

自测测试结论:

五、测试关注点(需要提醒QA重点关注的、可能会忽略的地方)

检查点:

需求名称 是否影响xx公共模块 是否需要xx功能 需求升级是否依赖其他子产品
xxx 否 需要 不需要

接口测试:

性能测试:

并发测试:

其他:

likedislike
合并受阻
afwk_helper成员
9月9日 评论:

开始进行AI检视!

AI review has been started, please wait...

likedislike
afwk_helper成员
9月9日 评论:
check type result report
start ai_review pass -
likedislike
openharmony_ciopenharmony_ci成员
9月9日 添加了label:waiting_on_author
openharmony_ci
openharmony_ci成员
9月9日 评论:

感谢提交 Pull Requests!如果您提交的PR已经开发完毕,请评论 "start build" 触发门禁,更多交互操作,请访问OpenHarmony社区支持命令清单。如果需要调整订阅PR、Issue的变更状态,请访问订阅链接。


Thanks for submitting the pull request. If your Pull Request has already been developed, you can leave a "start build" comment to trigger the gated system. For more commands, please visit OpenHarmony Command List. If you need to change the subscription of a Pull Request or Issue, please visit the link.

likedislike
openharmony_ciopenharmony_ci成员
9月9日 添加了label:dco检查成功
afwk_helper成员
9月9日 评论:

🚨 🤖 AI 代码检视报告 🚨

总体评估: CRITICAL

问题统计:

  • 总问题数: 5
  • 严重问题: 2
  • 高危问题: 2

摘要:
本PR在重构批量查询接口时引入了严重的编译错误(缺少分号)、空指针解引用风险以及静默提醒状态的逻辑回归,建议进行全面修复后再提交。

📊 详细报告
查看完整的审查详情,包括具体的问题描述、建议和代码位置:
🔗 查看详细报告


此评论由 OpenHarmony Insight 代码审查系统自动生成

likedislike
afwk_helper成员9月9日进行代码检视3
services/ans/src/advanced_notification_reminder_service.cpp
已过期
@@ -201,3 +186,1 @@
201- reminder.SetBundleOption(*validBundle);
202- reminder.SetReminderFlags(flags);
203- reminder.SetSilentReminderEnabled(silentReminderEnabled);
186+ reminder.SetBundleOption(*(resolvedBundles[i]));
afwk_helper9月9日评论:

🤖 AI 代码检视意见(回复本评论可解决检视意见,点击被检视代码行左侧的小头像可收起检视意见)


🔴 空指针解引用风险

位置: L186 | 严重程度: Critical

❓ 问题描述

在 GetReminderInfoByBundles 方法的循环中,直接对 resolvedBundles[i] 进行解引用 *(resolvedBundles[i]),未检查其是否为空指针。如果底层返回空指针对象,此处将导致严重的空指针解引用崩溃。原代码中有空指针检查,重构后丢失了该保护。

💡 修复建议

修改建议:在解引用前增加空指针及越界检查,若为空则跳过当前循环。

184: for (size_t i = 0; i < resolvedBundles.size(); ++i) {
185: if (resolvedBundles[i] == nullptr || i >= reminderInfos.size()) {
186: continue;
187: }
188: NotificationReminderInfo reminder;
189: reminder.SetBundleOption(*(resolvedBundles[i]));
190: reminder.SetReminderFlags(reminderInfos[i].first);
191: reminder.SetSilentReminderEnabled(
192: reminderInfos[i].second == NotificationConstant::SWITCH_STATE::USER_MODIFIED_ON);
193: reminderInfo.emplace_back(reminder);
194: }


likedislike
System
系统消息系统
9月10日 评论:

changed this line on 68d60e60 view diff detail

luxin
luxin
9月10日 评论:

已修改添加

afwk_helper成员9月9日进行代码检视3
services/ans/src/advanced_notification_reminder_service.cpp
已过期
@@ -203,0 +186,4 @@
186+ reminder.SetBundleOption(*(resolvedBundles[i]));
187+ reminder.SetReminderFlags(reminderInfos[i].first);
188+ reminder.SetSilentReminderEnabled(
189+ reminderInfos[i].second == NotificationConstant::SWITCH_STATE::USER_MODIFIED_ON);
afwk_helper9月9日评论:

🤖 AI 代码检视意见(回复本评论可解决检视意见,点击被检视代码行左侧的小头像可收起检视意见)


🟠 逻辑回归:丢失了系统默认开启状态的处理

位置: L189 | 严重程度: High

❓ 问题描述

原代码判断 SilentReminderEnabled 时,当状态为 SYSTEM_DEFAULT_ON 或 USER_MODIFIED_ON 均返回 true。而新代码仅判断 USER_MODIFIED_ON,丢失了对 SYSTEM_DEFAULT_ON 的处理。这会导致当系统默认静默提醒开启时,新接口错误地返回 false,引发业务行为变更。

💡 修复建议

修改建议:恢复对 SYSTEM_DEFAULT_ON 状态的判断,确保逻辑与原有一致。

188: reminder.SetSilentReminderEnabled(
189: reminderInfos[i].second == NotificationConstant::SWITCH_STATE::SYSTEM_DEFAULT_ON ||
190: reminderInfos[i].second == NotificationConstant::SWITCH_STATE::USER_MODIFIED_ON);


likedislike
System
系统消息系统
9月10日 评论:

changed this line on 68d60e60 view diff detail

luxin
luxin
9月10日 评论:

已修改添加

afwk_helper成员9月9日进行代码检视1
services/ans/src/advanced_notification_reminder_service.cpp
@@ -158,49 +158,35 @@ ErrCode AdvancedNotificationService::GetReminderInfoByBundles(
158158 return result;
159159 }
160160 HaMetaMessage message = HaMetaMessage(EventSceneId::SCENE_26, EventBranchId::BRANCH_3);
161+ auto resolvedBundles = ResolveBundleOptionUids(bundles);
afwk_helper9月9日评论:

🤖 AI 代码检视意见(回复本评论可解决检视意见,点击被检视代码行左侧的小头像可收起检视意见)


🟡 线程安全与设计问题:将 Bundle 解析移出任务队列

位置: L161 | 严重程度: Medium

❓ 问题描述

原代码将 GenerateValidBundleOption 调用放在了 notificationSvrQueue_.SyncSubmit 的 lambda 内部,保证了相关操作在专用线程上下文中执行。新代码将 ResolveBundleOptionUids 移到了 SyncSubmit 之外执行,这违背了原有的线程安全设计,可能在调用线程(如 IPC 线程)引发潜在的并发问题或数据竞争。

💡 修复建议

修改建议:将 ResolveBundleOptionUids 及相关变量的初始化移入 notificationSvrQueue_.SyncSubmit 的 lambda 内部,保持与原设计一致的线程模型。

159: HaMetaMessage message = HaMetaMessage(EventSceneId::SCENE_26, EventBranchId::BRANCH_3);
160: result = ERR_OK;
161: auto submitResult = notificationSvrQueue_.SyncSubmit(std::bind(& {
162: ANS_LOGD("ffrt enter!");
163: auto resolvedBundles = ResolveBundleOptionUids(bundles);
164: int32_t userId = -1;
165: if (OsAccountManagerHelper::GetInstance().GetCurrentCallingUserId(userId) != ERR_OK) {


likedislike
afwk_helper成员9月9日进行代码检视3
services/ans/src/notification_preferences_database.cpp
已过期
@@ -1270,0 +1299,4 @@
1299+ ANS_LOGE("Query reminder info for bundles failed.");
1300+ return false;
1301+ }
1302+ reminderInfos.clear()
afwk_helper9月9日评论:

🤖 AI 代码检视意见(回复本评论可解决检视意见,点击被检视代码行左侧的小头像可收起检视意见)


🔴 语法错误:缺少分号导致编译失败

位置: L1302 | 严重程度: Critical

❓ 问题描述

在 GetReminderInfoForBundles 方法中,reminderInfos.clear() 语句末尾缺少分号,这将导致编译错误,使得整个模块无法构建。

💡 修复建议

修改建议:在 reminderInfos.clear() 语句末尾补充分号。

1302: reminderInfos.clear();


likedislike
System
系统消息系统
9月10日 评论:

changed this line on 68d60e60 view diff detail

luxin
luxin
9月10日 评论:

已修复

afwk_helper成员9月9日进行代码检视2
services/ans/src/notification_preferences_database.cpp
@@ -1270,0 +1301,4 @@
1301+ }
1302+ reminderInfos.clear()
1303+ reminderInfos.reserve(bundleOptions.size());
1304+ for (const auto &bundleOption : bundleOptions) {
afwk_helper9月9日评论:

🤖 AI 代码检视意见(回复本评论可解决检视意见,点击被检视代码行左侧的小头像可收起检视意见)


🟠 空指针处理逻辑不一致导致索引对齐风险

位置: L1304-L1321 | 严重程度: High

❓ 问题描述

在 GetReminderInfoForBundles 的第一个循环中,遇到 nullptr 会跳过生成键;但在第二个循环中,遇到 nullptr 却不跳过,而是直接插入默认值。这种处理逻辑使得 reminderInfos 的索引与 bundleOptions 强对齐,但在上层调用者 advanced_notification_reminder_service.cpp 中直接解引用 resolvedBundles[i] 会导致空指针崩溃。此设计非常脆弱且与测试用例的期望存在矛盾。

💡 修复建议

修改建议:统一空指针处理策略。若决定在输入端过滤空指针,则在数据库层不应保留空位对齐逻辑,同时上层调用者需保证不传入空指针;并修正测试用例中相互矛盾的期望。

1304: for (const auto &bundleOption : bundleOptions) {
1305: if (bundleOption == nullptr) {
1306: continue;
1307: }
1308: uint32_t slotFlags = defaultSlotFlags;
1309: NotificationConstant::SWITCH_STATE silentState = NotificationConstant::SWITCH_STATE::SYSTEM_DEFAULT_OFF;
1310: std::string bundleKey = bundleOption->GetBundleName() + std::to_string(bundleOption->GetUid());
1311: auto slotFlagsIter = results.find(GenerateBundleKey(bundleKey, KEY_BUNDLE_SLOTFLGS_TYPE));
1312: if (slotFlagsIter != results.end()) {
1313: slotFlags = static_cast<uint32_t>(AnsCommonUtils::StringToInt(slotFlagsIter->second));
1314: }
1315: auto silentIter = results.find(GenerateBundleKey(bundleKey, KEY_SILENT_REMINDER_ENABLE_NOTIFICATION));
1316: if (silentIter != results.end()) {
1317: silentState = AnsCommonUtils::StringToInt(silentIter->second) ?
1318: NotificationConstant::SWITCH_STATE::USER_MODIFIED_ON :
1319: NotificationConstant::SWITCH_STATE::USER_MODIFIED_OFF;
1320: }
1321: reminderInfos.emplace_back(slotFlags, silentState);
1322: }


likedislike
luxin
luxin
9月10日 评论:

已修改

luxinluxin
9月9日 关联了issue:优化批量查询接口GetReminderInfoByBundles,改为真实批量查询
此处折叠了11条事件消息 查看更多
luxinluxin
9月10日 强制推送  1 个提交:68d60e60-Co-Authored-By: luxin
openharmony_ci
openharmony_ci成员
9月10日 评论:

感谢提交 Pull Requests!如果您提交的PR已经开发完毕,请评论 "start build" 触发门禁,更多交互操作,请访问OpenHarmony社区支持命令清单。如果需要调整订阅PR、Issue的变更状态,请访问订阅链接。


Thanks for submitting the pull request. If your Pull Request has already been developed, you can leave a "start build" comment to trigger the gated system. For more commands, please visit OpenHarmony Command List. If you need to change the subscription of a Pull Request or Issue, please visit the link.

likedislike
luxin
luxin
9月10日 评论:

check comments

likedislike
afwk_helper成员
9月10日 评论:
检视意见(Review Comments)
代码行数 总检视意见数 AI检视意见数 AI严重意见数 未确认严重意见数 检查结果
498 5 5 4 0 pass

检视意见成功。
Review Comments passed.

likedislike
luxin
luxin
9月10日 评论:

static-check

likedislike
openharmony_ci
openharmony_ci成员
9月10日 评论:


静态检查:

# check type result report
1 codeCheck noPass >>>

likedislike
luxinluxin
9月10日 审查状态已重置,审查人: afwk_helper
luxinluxin
9月10日 强制推送  1 个提交:54af88e5-B
luxin
luxin
9月10日 评论:

static-check

likedislike
openharmony_ci
openharmony_ci成员
9月10日 评论:

感谢提交 Pull Requests!如果您提交的PR已经开发完毕,请评论 "start build" 触发门禁,更多交互操作,请访问OpenHarmony社区支持命令清单。如果需要调整订阅PR、Issue的变更状态,请访问订阅链接。


Thanks for submitting the pull request. If your Pull Request has already been developed, you can leave a "start build" comment to trigger the gated system. For more commands, please visit OpenHarmony Command List. If you need to change the subscription of a Pull Request or Issue, please visit the link.

likedislike
openharmony_ci
openharmony_ci成员
9月10日 评论:


静态检查:

# check type result report
1 codeCheck pass >>>

likedislike
openharmony_dcp
openharmony_dcp成员
11 天前 评论:

您好, @qq_50992459 该PR需要您响应,已过去25天未响应,请根据检视意见进行修改,如5天内未响应检视意见,此PR会被自动关闭。关闭后的PR,如有需要,您可以自行打开该PR。

likedislike