Skip to content

fix: fix regression by 9937c5456e16dd1dd764b946ed382c0095048c53#1672

Merged
dengbo11 merged 1 commit into
OpenAtom-Linyaps:masterfrom
reddevillg:fix_interaction
Jun 9, 2026
Merged

fix: fix regression by 9937c5456e16dd1dd764b946ed382c0095048c53#1672
dengbo11 merged 1 commit into
OpenAtom-Linyaps:masterfrom
reddevillg:fix_interaction

Conversation

@reddevillg

Copy link
Copy Markdown
Collaborator

ReplyInteraction signal may lost if Task object is not ready. Move it from per-task interface to the PackageManager1 interface. Add syncTaskProperties to fetch initial task state also.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the interaction mechanism between the Package Manager and the CLI by moving the ReplyInteraction method and RequestInteraction signal from the Task1 interface to the PackageManager1 interface. This simplifies PackageTask and centralizes interaction handling within the PackageManager. The review identified three critical issues: a potential compilation or semantic mismatch when passing reply.error().type() to LINGLONG_ERR, a thread-safety violation from emitting the RequestInteraction signal from a secondary thread, and a race condition in Cli::interaction where early interaction requests could be silently ignored before taskObjectPath is initialized.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread libs/linglong/src/linglong/cli/cli.cpp
Comment thread libs/linglong/src/linglong/package_manager/package_manager.cpp
Comment thread libs/linglong/src/linglong/cli/cli.cpp
@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

ReplyInteraction signal may lost if Task object is not ready. Move it
from per-task interface to the PackageManager1 interface. Add
syncTaskProperties to fetch initial task state also.

Signed-off-by: reddevillg <reddevillg@gmail.com>
@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

deepin pr auto review

代码审查意见

总体评价

这段代码主要涉及DBus接口定义和交互逻辑的修改,主要变更包括:

  1. 在DBus接口中添加了ReplyInteraction方法和RequestInteraction信号
  2. 重构了交互处理逻辑,从PackageTask移到了PackageManager
  3. 改进了任务状态同步和错误处理机制

具体改进建议

1. 代码质量

  • 优点

    • 代码结构清晰,职责划分明确
    • 使用了RAII管理资源
    • 错误处理较为完善
  • 改进建议

    • syncTaskProperties 函数中,建议添加对返回的 QVariantMap 的有效性检查
    • interaction 函数中的对象路径检查可以提取为单独的辅助函数,提高代码复用性

2. 代码性能

  • 优点

    • 使用了QEventLoop进行异步操作管理
    • 超时机制设置合理(180秒)
  • 改进建议

    • waitConfirm函数中,可以考虑使用std::future和std::promise替代QEventLoop,这样可以更好地与现代C++特性结合
    • 服务监视器(QDBusServiceWatcher)的创建和销毁可以考虑使用智能指针自动管理

3. 代码安全

  • 优点

    • 对DBus连接状态进行了监控
    • 实现了超时机制防止死锁
    • 对caller disconnected情况进行了处理
  • 改进建议

    • 在处理DBus消息时,建议添加更多的输入验证,特别是对messageID和additionalMessage的验证
    • ReplyInteraction方法中,建议对replies参数进行深度验证,确保符合预期的格式
    • 考虑添加日志记录,以便追踪交互过程中的关键事件

4. 逻辑设计

  • 优点

    • 将交互逻辑从PackageTask移到PackageManager,使职责更加集中
    • 实现了统一的交互处理机制
  • 改进建议

    • waitConfirm函数中,建议将超时时间作为参数配置,而不是硬编码
    • 考虑为交互类型添加枚举验证,确保只处理已知的交互类型
    • 在处理升级交互时,建议添加更多的上下文信息,帮助用户做出决策

5. 其他建议

  • 建议添加单元测试,特别是对交互逻辑的测试
  • 考虑添加交互日志记录功能,便于问题追踪
  • 可以考虑为交互添加重试机制,提高用户体验

总的来说,这次重构改进了代码的组织结构和错误处理机制,但在安全性和可维护性方面还有提升空间。建议按照上述建议进行改进,以提高代码的整体质量。

@reddevillg reddevillg requested review from ComixHe and dengbo11 June 9, 2026 09:34
@dengbo11 dengbo11 merged commit 6525cb7 into OpenAtom-Linyaps:master Jun 9, 2026
16 of 17 checks passed
@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengbo11, reddevillg

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

Details Needs approval from an approver in each of these files:

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

@reddevillg reddevillg deleted the fix_interaction branch June 9, 2026 12:00
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.

3 participants