fix: correct notification ID handling and validation

[18202781743]

1. Fixed BubbleModel to use entity id instead of bubble id for data
   retrieval
2. Updated BubblePanel to check for InvalidId (0) instead of id > 0 when
   closing bubbles
3. Modified DataAccessorProxy to return empty entity instead of
   cascading fetch when primary source fails
4. Changed MemoryAccessor to generate unique negative IDs for in-memory
   entities to avoid conflicts
5. Updated NotifyEntity to use InvalidId constant (0) instead of -1 for
   invalid state
6. Added InvalidId constant definition in NotifyEntity header
7. Fixed NotificationManager to properly handle replace operations and
   return false when replace fails
8. Ensured consistent ID validation across all components using
   InvalidId constant

Log: Fixed notification display issues where incorrect IDs caused
mismatched notifications

Influence:

1. Test notification display in bubble panel with various notification
   types
2. Verify bubble closing functionality works correctly
3. Test notification replacement scenarios
4. Check memory-only notification handling
5. Validate notification persistence and retrieval from database
6. Test edge cases with invalid notification IDs
7. Verify notification matching and display consistency

fix: 修正通知ID处理和验证逻辑

1. 修复BubbleModel使用实体ID而非气泡ID进行数据检索
2. 更新BubblePanel在关闭气泡时检查InvalidId(0)而非id>0
3. 修改DataAccessorProxy在主数据源失败时返回空实体而非级联查询
4. 更改MemoryAccessor为内存实体生成唯一的负ID以避免冲突
5. 更新NotifyEntity使用InvalidId常量(0)而非-1表示无效状态
6. 在NotifyEntity头文件中添加InvalidId常量定义
7. 修复NotificationManager正确处理替换操作并在替换失败时返回false
8. 确保所有组件使用InvalidId常量进行一致的ID验证

Log: 修复通知显示问题，错误的ID导致通知匹配混乱

Influence:

1. 测试气泡面板中各种通知类型的显示
2. 验证气泡关闭功能正常工作
3. 测试通知替换场景
4. 检查仅内存通知的处理
5. 验证通知持久化和从数据库检索
6. 测试无效通知ID的边缘情况
7. 验证通知匹配和显示一致性

PMS: BUG-349053

Summary by Sourcery

Standardize notification ID handling and invalid ID semantics across notification storage, in-memory access, and UI bubbles to fix mismatched and failing notification operations.

Bug Fixes:

• Correct notification replacement logic to fail fast and report when a replace target does not exist or DB persistence fails.
• Fix bubble closing behavior to use a shared invalid ID sentinel instead of relying on positive ID checks, preventing incorrect close attempts.
• Resolve incorrect notification lookups in the bubble model by exposing the entity ID rather than the bubble ID for the Id role.
• Prevent cascading data fetches when the primary data accessor fails by returning an empty notification entity instead.
• Ensure in-memory notifications use unique negative IDs to avoid conflicts with persisted IDs and invalid IDs.
• Align NotifyEntity validity checks and initial IDs with a shared InvalidId constant so invalid entities are consistently detected.

Enhancements:

• Introduce a central NotifyEntity::InvalidId constant and propagate its usage across notification manager, entities, and UI components for consistent ID validation semantics.

[deepin-ci-robot]

deepin pr auto review

这份代码修改主要涉及通知系统中 ID 的处理逻辑，将原本基于 > 0 的 ID 有效性判断改为基于特定常量 InvalidId 的判断，并统一了 ID 的获取方式。整体来看，这是一次旨在提高代码健壮性和可维护性的重构。

以下是对各部分代码的详细审查意见：

1. 整体逻辑与设计

优点：

• 语义明确：引入 NotifyEntity::InvalidId = 0 替代了原本散落在代码各处的 0 或 -1，提高了代码的可读性。
• 统一接口：将 bubbleId() 统一修改为 id()，消除了命名上的不一致，使得模型层和数据层的交互更清晰。

潜在问题：

• ID 生成策略冲突：在 memoryaccessor.cpp 中，内存 ID 生成逻辑被修改为递减的负数（--g_Id），而在 notifyentity.h 中定义 InvalidId = 0。这意味着 ID 的有效范围变成了 != 0（即正数和负数均有效）。然而，在某些数据库或持久化场景下，负数 ID 可能会引起问题（例如作为数组索引或自增主键）。需要确认持久化层（如 DB）是否支持负数 ID，或者内存中的负数 ID 是否会在后续逻辑中导致混淆。

2. 文件具体审查

panels/notification/bubble/bubblemodel.cpp

-        return m_bubbles[row]->bubbleId();
+        return m_bubbles[row]->id();

• 评价：良好。统一了访问器名称。

panels/notification/bubble/bubblepanel.cpp

-    if (id > 0) {
+    if (id != NotifyEntity::InvalidId) {

• 评价：逻辑正确。将硬编码的数值判断改为常量判断，符合防御性编程原则。

panels/notification/common/dataaccessorproxy.cpp

     auto entity = m_impl->fetchLastEntity(notifyId);
     if (entity.isValid())
         return entity;
-
-    return m_source->fetchLastEntity(notifyId);
+    return {};

• 评价：逻辑有风险。
  • 原代码逻辑是：优先从内存（m_impl）获取，如果内存没有，则从持久化源（m_source）获取。
  • 新代码逻辑是：仅从内存获取，如果内存没有，直接返回空实体。
  • 改进建议：除非业务需求明确变更（不再回源查找），否则这可能是一个 Bug。如果确实不再需要回源，建议添加注释说明原因。

panels/notification/common/memoryaccessor.cpp

 qint64 MemoryAccessor::addEntity(const NotifyEntity &entity)
 {
+    static qint64 g_Id = NotifyEntity::InvalidId; // use negative id for in-memory entities
     QMutexLocker locker(&m_mutex);
     m_entities << entity;
-    return entity.bubbleId();
+    return --g_Id;
 }

• 评价：存在安全隐患。
  • 安全性：使用 static 局部变量生成 ID。虽然 QMutexLocker 保证了线程安全，但在程序长期运行的情况下，g_Id 会不断递减。虽然 qint64 范围很大，但在极端高频场景下仍存在溢出风险（变为正数，甚至回绕到 InvalidId）。
  • 代码质量：g_Id 初始化为 InvalidId (0)，第一次调用返回 -1。这与 InvalidId = 0 的定义不冲突，但逻辑上略显别扭（从 0 开始减）。
  • 改进建议：建议初始化为一个较大的负数（如 -1），或者使用 QAtomicInt 等原子操作配合更复杂的生成策略（如 UUID 或雪花算法），如果仅仅是内存临时使用，目前的做法尚可，但需注意溢出。

panels/notification/common/notifyentity.cpp

-    qint64 id = -1;
+    qint64 id = NotifyEntity::InvalidId;
...
-    return d && d->id > 0 && d->cTime > 0;
+    return d && d->id != NotifyEntity::InvalidId && d->cTime > 0;

• 评价：良好。统一了无效 ID 的定义和校验逻辑。

panels/notification/server/notificationmanager.cpp

     if (entity.isReplace()) {
         ...
         } else {
             qWarning() << "Not exist notification to replace for the replaceId" << entity.replacesId();
+            return false;
         }
     }

• 评价：逻辑改进。
  • 原代码在替换失败（找不到被替换对象）时，会继续往下走尝试执行 addEntity。这可能导致同一个 replacesId 被当作新通知添加，不符合"替换"的语义。
  • 新代码直接返回 false，逻辑更严谨。

3. 总结与建议

1. 关键问题：请务必确认 dataaccessorproxy.cpp 中移除 m_source->fetchLastEntity 是否符合预期。这可能导致持久化存储的通知无法被加载。
2. ID 范围一致性：确认整个系统（包括数据库表设计）是否接受负数 ID 作为有效 ID。如果数据库只接受正数自增 ID，那么 MemoryAccessor 生成的负数 ID 在尝试写入数据库时可能会失败或产生冲突。
3. 常量定义位置：InvalidId 定义在 notifyentity.h 中是合理的，确保所有引用该头文件的地方都能使用该常量。

总体而言，除了 dataaccessorproxy.cpp 的逻辑变动需要再次确认外，其他修改提升了代码的规范性和健壮性。

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• In MemoryAccessor::addEntity, the static local g_Id is shared across all MemoryAccessor instances and updated without its own synchronization; consider making it a member variable (initialized once per accessor) protected by m_mutex to avoid cross-instance coupling and potential race conditions if addEntity is ever called concurrently across instances.
• Now that in-memory entities intentionally use negative IDs and persistent ones use positive IDs, it may be worth enforcing this invariant at the boundaries (e.g., in addEntity/replaceEntity or NotifyEntity construction) to catch incorrect ID assignments early rather than relying only on InvalidId checks.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In MemoryAccessor::addEntity, the static local g_Id is shared across all MemoryAccessor instances and updated without its own synchronization; consider making it a member variable (initialized once per accessor) protected by m_mutex to avoid cross-instance coupling and potential race conditions if addEntity is ever called concurrently across instances.
- Now that in-memory entities intentionally use negative IDs and persistent ones use positive IDs, it may be worth enforcing this invariant at the boundaries (e.g., in addEntity/replaceEntity or NotifyEntity construction) to catch incorrect ID assignments early rather than relying only on InvalidId checks.

## Individual Comments

### Comment 1
<location> `panels/notification/common/memoryaccessor.cpp:18-21` </location>
<code_context>

 qint64 MemoryAccessor::addEntity(const NotifyEntity &entity)
 {
+    static qint64 g_Id = NotifyEntity::InvalidId; // use negative id for in-memory entities
     QMutexLocker locker(&m_mutex);
     m_entities << entity;
-    return entity.bubbleId();
+    return --g_Id;
 }

</code_context>

<issue_to_address>
**issue (bug_risk):** Consider updating the stored entity's id to match the generated in-memory id.

In `addEntity`, we store `entity` unchanged but return a decremented `g_Id`. Unless `NotifyEntity` is updated elsewhere, `entity.id()` will stay `InvalidId` while callers may rely on the returned negative id. Any code that later uses `entity.id()` (e.g., for comparisons or lookups) will then see a different id than the one returned here. Consider cloning/updating the entity with the generated id before storing it so the stored entity and returned id are consistent.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue (bug_risk): Consider updating the stored entity's id to match the generated in-memory id.

In addEntity, we store entity unchanged but return a decremented g_Id. Unless NotifyEntity is updated elsewhere, entity.id() will stay InvalidId while callers may rely on the returned negative id. Any code that later uses entity.id() (e.g., for comparisons or lookups) will then see a different id than the one returned here. Consider cloning/updating the entity with the generated id before storing it so the stored entity and returned id are consistent.

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, yixinshark

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment