Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Ivy233 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's GuideThis PR defers starting the notification expiration timer until a bubble is actually inserted into the UI model, wiring a new bubbleDisplayed signal through BubblePanel to NotificationManager, which now schedules timeouts based on the stored client expire timeout only when notifications are displayed, with thread-safe forwarding from the applet and added unit tests. Sequence diagram for deferred notification timeout start when bubble is displayedsequenceDiagram
participant BubbleModel
participant BubblePanel
participant NotifyServerApplet
participant NotificationManager
BubbleModel->>BubbleModel: insertBubble / replaceBubble
BubbleModel-->>BubblePanel: bubbleDisplayed(id)
BubblePanel->>NotifyServerApplet: notificationDisplayed(id)
NotifyServerApplet->>NotificationManager: notificationDisplayed(id)
NotificationManager->>NotificationManager: fetchEntity(id)
NotificationManager->>NotificationManager: pushPendingEntity(entity, entity.timeout())
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
430dd63 to
fcf70d8
Compare
0b0eb8c to
2ae625f
Compare
2ae625f to
5ef467e
Compare
|
|
||
| Q_EMIT NotificationStateChanged(entity.id(), entity.processedType()); | ||
|
|
||
| bool critical = false; |
There was a problem hiding this comment.
服务端定时器逻辑整体移除后，超时计算完全由前端 ExpireTimer 负责，ExpireTimer 内部通过 NotifyEntity::timeout()/urgency() 自行推导有效超时时间（-1 默认 5000ms，0/Critical 永不过期），因此无需再把 expireTimeout 参数单独传给定时器。
|
|
||
| bool contains(qint64 key) const; | ||
| // Milliseconds left for key, or 0 when it is not tracked. | ||
| int remaining(qint64 key) const; |
There was a problem hiding this comment.
按操作来定义接口吧，不用搞这么通用的，然后让调用者去组合，
这里只有start，stop，clear吧，resume类似传递需要停止的entity，逻辑在内部封装，
| * that key. All bookkeeping lives in hash maps, so no QTimer is allocated per | ||
| * key and nothing leaks when a key expires or is stopped. | ||
| */ | ||
| class ExpireTimer : public QObject |
There was a problem hiding this comment.
暂存区和通知横幅的是不是共用同一个定时器管理的呀，不然这里会不会一个通知有两个定时器在弄呀，
There was a problem hiding this comment.
是共用的。ExpireTimer 是进程级单例，仅一个共享 QTimer，气泡与暂存区都通过它管理。同一通知 id 同时在气泡和暂存区显示时，ExpireTimer::push 发现 id 已在跟踪中会保持原截止时间而不重建倒计时，不会出现一个通知被两个定时器同时计时的现象。
5ef467e to
aa5a30c
Compare
aa5a30c to
fadfda6
Compare
| return &expireTimer; | ||
| } | ||
|
|
||
| void ExpireTimer::start(qint64 key, const NotifyEntity &entity) |
There was a problem hiding this comment.
这里里的key就是entity的id吧，直接从entity里获取就行了，这个key名称换成id吧，
| if (id == m_blockedId) | ||
| return; | ||
|
|
||
| if (m_blockedId != NotifyEntity::InvalidId) |
There was a problem hiding this comment.
这个hover就blocked的逻辑可以放在ExpirreTimer里实现吧，不暴露resume和pause了，
There was a problem hiding this comment.
已经把block逻辑迁移到expiretimer里面，并替代掉resume和pause。
| // A bubble that has been pushed off the display (overflow) is no longer | ||
| // in the model, but its countdown still finishes and the notification | ||
| // should still be closed, so fall back to the persisted bubble id. | ||
| Q_EMIT bubbleExpired(id, DataAccessorProxy::instance()->fetchEntity(id).bubbleId()); |
There was a problem hiding this comment.
既然bubbleExpired需要bubbleId，那在ExpireTimer::expired里就直接添加这个参数就行了，
There was a problem hiding this comment.
已按建议处理。ExpireTimer::expired 的签名就是 expired(qint64 id, uint bubbleId)，到期时从实体读取 bubbleId 随信号一并发出，BubbleModel::bubbleExpired(id, bubbleId) 直接转发，无需自行拼接。
| if (it == m_deadlines.cend()) | ||
| return; | ||
|
|
||
| m_paused.insert(key, static_cast<int>(qMax<qint64>(0, it.value() - QDateTime::currentMSecsSinceEpoch()))); |
There was a problem hiding this comment.
pause的只会是一个吧，最起码目前可以只是一个，
| if (m_deadlines.contains(key) || m_paused.contains(key)) | ||
| return; | ||
|
|
||
| m_deadlines.insert(key, QDateTime::currentMSecsSinceEpoch() + interval); |
There was a problem hiding this comment.
这个还是解决不了在通知横幅定时器跑了一半的时候，再切到暂存区定时器又重新计时的问题呀，
There was a problem hiding this comment.
已解决。ExpireTimer::remove 在气泡移出模型时挂起该 id 的倒计时（把绝对截止时间记入 m_retired），随后暂存区 NotifyStagingModel::push 调用 ExpireTimer::push 时从 m_retired 恢复原截止时间而不是重新计时；若暂停期间已过截止点，恢复时会按当前时间立即到期，不再从横幅跑了一半的地方重头开始。
There was a problem hiding this comment.
补充说明（方案有更新）：最终实现没有引入 m_retired，改为更简单的幂等 push——ExpireTimer::push 检测到相同 id + cTime 已在跟踪时直接返回，保留首次（横幅显示时）写入的绝对截止时间。因此同一通知随后进入/切换到暂存区时复用原截止时间、不会重新计时；只有服务端关闭通知（NotificationStateChanged → ExpireTimer::remove）或替换通知（按 bubbleId 换槽）才会结束/重建计时。效果与之前描述的 m_retired 方案等价：横幅跑了一半切到暂存区，剩余时间照旧倒数，过期点不变。
| return; | ||
|
|
||
| // Critical notifications must not disappear on their own. | ||
| if (reason == NotifyEntity::Expired && entity.urgency() == NotifyEntity::Critical) |
There was a problem hiding this comment.
Expired的不会是Critical类型的吧，不需要这个判断吧，
fadfda6 to
e12655f
Compare
| if (id == m_blockedId) | ||
| return; | ||
|
|
||
| m_blockedId = id; |
There was a problem hiding this comment.
BubbleModel里不需要m_blockedId了吧，
There was a problem hiding this comment.
已删除。悬停冻结逻辑整体下沉到 ExpireTimer::setBlockId（内部维护单一悬停 id），BubblePanel::setHoveredId 直接调用 ExpireTimer::instance()->setBlockId(id)，BubbleModel 不再保存 m_blockedId。
| // times out, close it and notify the server so it moves the notification | ||
| // from the in-memory store to the center database and emits the signals. | ||
| connect(m_bubbles, &BubbleModel::bubbleExpired, this, [this](qint64 id, uint bubbleId) { | ||
| closeBubble(id); |
There was a problem hiding this comment.
不需要这里close吧，它会被server发送过来的吧，
There was a problem hiding this comment.
已去掉。到期时 BubblePanel 只向服务端发送 notificationClosed(id, bubbleId, Expired)，服务端处理后发出 NotificationStateChanged，气泡经由 onNotificationStateChanged 的正常流程关闭，不再本地 closeBubble。
| const auto replaceIndex = replaceBubbleIndex(bubble); | ||
| const auto oldBubble = m_bubbles[replaceIndex]; | ||
|
|
||
| ExpireTimer::instance()->stop(oldBubble->id()); |
There was a problem hiding this comment.
这个替换的逻辑，放在ExpireTimer里吧，只需要跟insert一样，push进去就行，
There was a problem hiding this comment.
已迁移。replaceBubble 现在与 insertBubble 一样只调用 ExpireTimer::push；替换逻辑（取消同一气泡槽位旧通知的倒计时、悬停块转移给新通知）封装在 push 内部的 cancelReplacement 中统一处理。
There was a problem hiding this comment.
已按建议修改：BubbleModel::replaceBubble 现在只调用 ExpireTimer::instance()->push(bubble->entity())，与 insertBubble 完全一致。替换语义全部下沉到 ExpireTimer::push 内部：isReplace() 的实体会按 bubbleId 取消旧槽位的倒计时（悬停 block id 同步转移到新 id）后，再按新实体的 timeout/urgency 启动新计时。
| // Absolute deadlines of ids stopped by stop(); restored by start() so a | ||
| // context switch (bubble <-> staging) resumes the countdown. | ||
| QHash<qint64, qint64> m_retired; | ||
| QHash<qint64, uint> m_bubbleIds; |
There was a problem hiding this comment.
不需要m_bubbleIds和m_deadlines两个吧，放一个QHash<qint64, NotifyEntity>是不是就可以了，
There was a problem hiding this comment.
已合并。m_bubbleIds 已删除，m_deadlines 改为 QHash<qint64, Deadline>，Deadline 同时保存实体与绝对截止时间（point），到期时所需的 bubbleId 直接从实体读取。
There was a problem hiding this comment.
已修改：现在只保留一个容器 QMultiHash<qint64, NotifyEntity> m_pendingEntities，key 为该通知的绝对截止时间（共享 QTimer 按最近的截止时间调度），id/bubbleId/cTime 都由 value 中的 entity 携带，不再有 m_bubbleIds/m_deadlines 两个结构。
e12655f to
0c427f4
Compare
| connect(NotifyAccessor::instance(), &NotifyAccessor::stagingEntityClosed, this, &NotifyStagingModel::onEntityClosed); | ||
| connect(NotifySetting::instance(), &NotifySetting::contentRowCountChanged, this, &NotifyStagingModel::updateContentRowCount); | ||
|
|
||
| connect(ExpireTimer::instance(), &ExpireTimer::expired, this, [this](qint64 id, uint bubbleId) { |
There was a problem hiding this comment.
一个通知发送了expired后，会不会被通知横幅和暂存区都close一次呀，
是不是可以直接由ExpireTimer调用server的close呀，
There was a problem hiding this comment.
已处理。
-
关于"会不会 close 两次"：确实存在这个竞态，已修复。之前暂存区在 ExpireTimer::expired 时本地 remove + 补位，而服务端关闭是异步投递到 worker 线程的，导致刚过期的通知在补位时仍被当作 NotProcessed 重新插入并重新计时，一个倒计时周期后再次触发 notificationClosed（被幂等检查挡住，但会多打一条日志、多一次空跑）。现在暂存区不再响应 expired，只在服务端 stagingEntityClosed 回环时移除行，此时实体已标记 Processed，不会被补位重新选中。
-
关于"由 ExpireTimer 直接调用 server 的 close"：采用等价且不破坏分层的做法——ExpireTimer 保持纯前端、不感知服务端，由 NotifyServerApplet 连接 ExpireTimer::expired，通过 Qt::QueuedConnection 统一转发给 NotificationManager::notificationClosed，作为过期关闭的唯一入口；notificationClosed 保留幂等检查兜底。横幅和暂存区各自只移除自己的视图，服务端每个通知只处理一次。
a5757b6 to
c7b598c
Compare
|
|
||
| const auto entity = notifyById(id); | ||
| if (entity.isValid()) | ||
| ExpireTimer::instance()->remove(entity); |
There was a problem hiding this comment.
最新版本已按建议调整：NotifyStagingModel::remove 已不再直接调用 ExpireTimer::remove。通知生命周期统一由服务端管理——关闭/归档时服务端发出 NotificationStateChanged(Processed/Removed)，由 NotifyServerApplet 统一调用 ExpireTimer::remove(id) 停止倒计时；前端各视图只负责移除自己的行，删除逻辑不再散落在前端模型里。
顺带说明一下新增的 m_opened 标志的用途：NotifyStagingModel 在通知中心面板隐藏时依然存活，stagingEntityReceived 随时会触发 doEntityReceived。若面板隐藏期间不拦截，通知会立即 push() 进入 ExpireTimer 启动倒计时，还没显示就过期，违背"实际显示后才开始计时"的原则。m_opened 由 open()/close()（Panel.visibleChanged 驱动）维护，作为"暂存区是否可见"的标志：doEntityReceived 中 if (!m_opened) return; 确保只有面板可见、暂存区实际显示时才启动倒计时；open() 中的 if (m_opened) return; 作为重复进入保护。
0ea9c62 to
24d7693
Compare
| qDebug(notifyLog) << "Receive entity" << id; | ||
|
|
||
| // The model exists while the panel is hidden. A hidden staging item has not | ||
| // been displayed and must not start the expiration countdown. |
24d7693 to
8c366c9
Compare
|
回复 auto review 提出的多线程竞态问题：经逐一核实，
另外，示例中在持锁状态下调用 |
|
/test github-pr-review-ci |
|
TAG Bot New tag: 2.0.53 |
| const auto entity = m_persistence->fetchEntity(id); | ||
| // A notification can be tracked by more than one expire timer (the bubble |
There was a problem hiding this comment.
不会被多个计时吧，超时了也是从这里更新状态，发送NotificationClosed的吧，
|
TAG Bot New tag: 2.0.54 |
8c366c9 to
cdd2040
Compare
|
TAG Bot New tag: 2.0.55 |
|
TAG Bot New tag: 2.0.56 |
1. Move the pending-timeout machinery from NotificationManager to the frontend ExpireTimer singleton, preserving one shared QTimer and absolute deadlines in a QMultiHash while removing the server-side timer state 2. Start a countdown only when a notification is inserted into the bubble model or the visible staging model, so queued, hidden and overflow notifications do not expire before they are displayed 3. Make push idempotent for the same entity so the bubble and staging views share the first deadline, and replace the old bubble-slot countdown when a new entity replaces it 4. Keep hover blocking inside ExpireTimer and give an unblocked notification a short grace period without spinning the timer at a zero interval 5. Keep the server authoritative for notification lifecycle: forward expiration through NotifyServerApplet with a queued call, and stop countdowns from server state changes instead of frontend model removal 6. Derive effective timeouts from NotifyEntity (0/Critical never expires and -1 uses the 5000 ms default) and carry bubbleId in the expiration signal 7. Remove the staging expiry race by waiting for the server close round trip before refilling rows; hidden-panel updates remain gated by NotifyAccessor 8. Remove the obsolete NotificationManager pending-timeout code and its applet tests 9. Match staging notification replacements by bubble id (the key preserved across a replace), exactly like BubbleModel::replaceBubbleIndex: a replacement may carry a fresh entity id (marked-processed then re-added) or the original entity id (replaceEntity), so matching on the entity id left the old staging row stale and inserted a duplicate instead of updating in place 10. Update the SPDX copyright year of notifystagingmodel.h to 2024-2026 Log: Start notification expiration after actual display and keep lifecycle cleanup owned by the server Influence: 1. Verify normal notifications disappear after their configured/default timeout 2. Verify queued and hidden notifications do not expire before being displayed 3. Verify opening the notification center starts only visible staging countdowns 4. Verify hovering prevents expiration and leaves a short grace period on exit 5. Verify replacement and shared bubble/staging notifications keep one countdown 6. Verify close, invoke and expiry paths stop countdowns through server state 7. Verify a replaced staging notification updates in place instead of leaving a stale row and inserting a duplicate fix: 通知实际显示后才开始过期计时 1. 将待超时机制从 NotificationManager 迁移到前端 ExpireTimer 单例，保留一个 共享 QTimer，并以 QMultiHash 保存绝对截止时间，同时移除服务端定时器状态 2. 仅在通知插入横幅模型或可见暂存区模型时启动倒计时，排队、隐藏及未展示的 重叠通知不会在实际显示前过期 3. 同一实体重复 push 时保持首次截止时间，使横幅与暂存区共享一个倒计时；新实体 替换通知时则移除同一气泡槽位的旧倒计时 4. 将悬停阻塞封装在 ExpireTimer 内，解除阻塞后保留短暂宽限期，并避免定时器以 零间隔空转 5. 由服务端统一决定通知生命周期：NotifyServerApplet 通过队列调用转发过期关闭， 并根据服务端状态变化停止倒计时，不再由前端模型移除操作清理 6. 由 NotifyEntity 推导有效超时（0/Critical 永不过期，-1 使用默认 5000ms）， 并在过期信号中携带 bubbleId 7. 暂存区等待服务端关闭回环后再补位，避免过期通知被重新插入；面板隐藏期间的 更新继续由 NotifyAccessor 统一屏蔽 8. 移除 NotificationManager 中废弃的待超时代码及对应 applet 测试 9. 暂存区通知替换改为按 bubbleId 匹配（与 BubbleModel::replaceBubbleIndex 一致）：替换可能携带全新实体 id（旧实体标记已处理后再新增）或原实体 id （replaceEntity），按实体 id 匹配会导致旧行残留并插入重复行，无法原地更新 10. 更新 notifystagingmodel.h 的 SPDX 版权年份为 2024-2026 Log: 通知实际显示后开始过期计时，并由服务端统一管理生命周期清理 Influence: 1. 验证普通通知按配置或默认超时时间消失 2. 验证排队及隐藏通知不会在显示前过期 3. 验证打开通知中心时仅启动实际可见暂存通知的倒计时 4. 验证悬停阻止通知过期，移开后保留短暂宽限期 5. 验证替换通知以及横幅/暂存区共享通知只保留一个倒计时 6. 验证关闭、调用动作及过期路径均通过服务端状态停止倒计时 7. 验证被替换的暂存区通知原地更新，而非残留旧行并插入重复行 PMS: BUG-372279
fix: 修复 actionInvoked 跨线程竞态并补充 ExpireTimer 单元测试 1. Changed Qt::DirectConnection to Qt::QueuedConnection in both actionInvoked overloads to eliminate cross-thread race conditions, consistent with the already-fixed notificationClosed. 将 actionInvoked 两个重载的 Qt::DirectConnection 改为 Qt::QueuedConnection，消除跨线程竞态，与已修复的 notificationClosed 保持一致。 2. Restored private: access modifier for member variables in bubblemodel.h that was accidentally removed. 恢复 bubblemodel.h 中被误移除的成员变量 private: 访问修饰符。 3. Added ExpireTimer unit tests covering expiration timing, idempotent push, hover blocking, replacement cancellation, and critical urgency non-expiration. 新增 ExpireTimer 单元测试，覆盖过期计时、幂等 push、悬停阻断、 替换取消、Critical 紧急度不过期等场景。 PMS: BUG-372279 Log: 修复通知 actionInvoked 跨线程竞态，恢复 private 访问修饰符，补充 ExpireTimer 单元测试 Influence: 通知中心 actionInvoked 调用安全性提升，ExpireTimer 测试覆盖率提升
6e0e511 to
61ccf38
Compare
deepin pr auto reviewAI 代码审查报告
总体评价
本次 PR 将通知过期计时机制从服务端 NotificationManager 迁移到前端 ExpireTimer 单例，确保通知仅在实际显示后才开始倒计时。代码质量优秀，设计思路清晰，线程安全处理得当，同时修复了跨线程竞态和暂存区替换匹配等多个缺陷，并补充了完整的单元测试。 漏洞统计
漏洞对比统计：新增漏洞 0 个，减少漏洞 0 个，持平 0 个 四维度评分维度1：语法逻辑（25/25）✓
评估要点:
无编译错误，无内存泄漏（Qt 父子对象所有权管理正确），无空指针解引用，边界条件处理完善。 维度2：代码质量（25/25）✓
评估要点:
维度3：代码性能（20/20）✓
评估要点:
维度4：代码安全（30/30）✓
评估要点:
改进建议本次代码审查未发现需要修复的问题。以下为可选的优化建议：
审查检查清单
关联信息
|
3 participants
NotificationManagerto a frontendExpireTimersingleton, keeping one shared QTimer and absolute deadlines in a singleQMultiHashwhile removing all server-side timer stateExpireTimer::pushidempotent for the same entity (same id + cTime), so the bubble and the staging area share the first deadline instead of restarting it, and cancel the old bubble-slot countdown when a replacement arrivesExpireTimer::setBlockIdwith a short grace period, without spinning the shared timer at a zero intervalNotificationManager::notificationClosedvia a queued invocation, and countdowns are stopped from the server'sNotificationStateChangedinstead of frontend model removal;notificationClosednow reports a close only once per notificationNotifyEntity(timeout 0 or Critical urgency never expires, -1 falls back to the 5000 ms default) and carrybubbleIdin theexpiredsignalBubbleModel::replaceBubbleIndexNotificationManager/NotifyServerAppletand its outdated testsdataChangedinDockGlobalElementModel, fix refresh of multi-mapped rows inRoleCombineModel)Log: Start the notification expire countdown only after the notification is displayed
Influence:
fix: 通知显示后才启动过期计时
NotificationManager中的 pending-timeout 机制整体迁移到前端ExpireTimer单例：保留单个共享 QTimer 与按绝对截止时间组织的QMultiHash，移除服务端全部定时器状态ExpireTimer::push对同一通知（相同 id + cTime）幂等：气泡与暂存区共享首次截止时间、不重复计时；替换通知按 bubbleId 取消旧槽位倒计时后再启动新计时ExpireTimer::setBlockId，移开悬停后有短暂宽限期，共享 QTimer 不会以 0 间隔空转NotificationManager::notificationClosed，前端根据服务端NotificationStateChanged停止计时而非自行移除；notificationClosed增加去重，避免同一通知重复上报关闭NotifyEntity推导（timeout 为 0 或 Critical 紧急级别永不过期，-1 回退默认 5000ms），expired信号携带 bubbleIdBubbleModel::replaceBubbleIndex保持一致NotificationManager/NotifyServerApplet中过时的 pending-timeout 代码及其旧测试DockGlobalElementModel转发 dataChanged，RoleCombineModel修复多对一映射行的刷新）Log: 通知显示后才启动过期计时
Influence:
PMS: BUG-372279
Summary by Sourcery
Start notification expiration only after display while coordinating shared countdowns with server-owned notification state.
Bug Fixes:
Enhancements:
Tests:
Chores: