Skip to content

Conversation

@lichaofan2008
Copy link

@lichaofan2008 lichaofan2008 commented Sep 1, 2025

  • Added functionality to monitor newly created directories and handle pending directories.
  • Implemented parent directory monitoring for non-existing paths.
  • Improved logging for directory changes and monitoring status.

fix: 提高文件监听能力,可监听暂时不存在的潜在目录

Bug: https://pms.uniontech.com/bug-view-328595.html

Summary by Sourcery

Allow monitoring of directories that do not exist at startup by watching parent paths, add automatic detection and watching of newly created subdirectories, and improve logging for directory monitoring events.

New Features:

  • Monitor non-existent target directories by watching their parent directories and activating direct monitoring once they appear
  • Automatically detect and add newly created subdirectories under existing monitored directories

Bug Fixes:

  • Fix inability to monitor directories that were missing at initialization

Enhancements:

  • Add detailed debug and info logs for directory changes, pending directories, and watcher status
  • Clear and reset internal pending, current, and parent watcher state when stopping watchers

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • clear() deletes the QTimer instance but methods like checkPendingDirectories and directoryChanged handler still call m_timer->start without guarding for null—add null checks or avoid deleting the timer until all callbacks are detached to prevent crashes.
  • The method name addWather seems to be a typo; renaming it to addWatcher would improve readability and consistency.
  • There are many qDebug/qInfo statements flooding the code path—consider consolidating logs or introducing a verbosity flag to reduce console noise in production.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- clear() deletes the QTimer instance but methods like checkPendingDirectories and directoryChanged handler still call m_timer->start without guarding for null—add null checks or avoid deleting the timer until all callbacks are detached to prevent crashes.
- The method name addWather seems to be a typo; renaming it to addWatcher would improve readability and consistency.
- There are many qDebug/qInfo statements flooding the code path—consider consolidating logs or introducing a verbosity flag to reduce console noise in production.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@sourcery-ai
Copy link

sourcery-ai bot commented Sep 1, 2025

Reviewer's Guide

This PR extends FileInotify to support monitoring of newly created and currently non-existent directories by tracking pending paths and parent watchers, restructures addWather to categorize paths, adds detailed logging across directory events, improves resource cleanup, and streamlines the FileInotifyGroup API.

File-Level Changes

Change Details Files
Enhanced logging for directory events and monitoring actions
  • Log detected directory changes in the directoryChanged callback
  • Emit debug/info messages when adding or removing watches
  • Log parent watcher additions and pending directory handling
  • Log failures when parent directories do not exist
src/album/fileinotify.cpp
Added support for pending and parent directory monitoring
  • Introduce m_pendingDirs, m_parentDirs, and m_parentToChildren to track non-existing targets
  • Implement addParentWatcher to watch parent paths and map to target children
  • Implement checkPendingDirectories to detect and promote newly created directories
src/album/fileinotify.cpp
src/album/fileinotify.h
Restructured addWather to handle existing vs. non-existing paths
  • Separate given paths into existingPaths and nonExistingPaths
  • Add direct watches for existing directories
  • Set up parent watches for directories that don’t yet exist and enqueue them as pending
src/album/fileinotify.cpp
Improved cleanup in clear() to reset new monitoring state
  • Clear m_pendingDirs, m_parentDirs, m_parentToChildren, m_currentDirs, and m_deleteFile
  • Safely stop and delete the timer only if it’s active
src/album/fileinotify.cpp
Simplified FileInotifyGroup.startWatch API
  • Removed pre-filtering of existing paths
  • Pass full path list directly to addWather
src/album/fileinotifygroup.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

- Added functionality to monitor newly created directories and handle pending directories.
- Implemented parent directory monitoring for non-existing paths.
- Improved logging for directory changes and monitoring status.

fix: 提高文件监听能力,可监听暂时不存在的潜在目录

Bug: https://pms.uniontech.com/bug-view-328595.html
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查报告

总体评价

这个代码实现了一个基于Qt的文件系统监控功能,主要使用QFileSystemWatcher来监控目录变化。代码结构清晰,功能完整,但有一些可以改进的地方。

语法逻辑

  1. 函数命名错误

    • fileinotify.h 中函数名从 addWather 改为 addWatcher,但在 fileinotifygroup.cpp 中仍然使用了旧的函数名 addWather。这会导致编译错误。
    • 建议:统一使用正确的函数名 addWatcher
  2. 代码重复

    • clear() 函数中,m_Supported.clear() 被调用了两次。
    • 建议:删除重复的调用。

代码质量

  1. 日志输出

    • 代码中添加了大量的 qDebugqInfo 日志输出,这对于调试很有帮助,但在生产环境中可能会产生过多日志。
    • 建议:考虑使用Qt的日志系统,根据不同级别(DEBUG, INFO, WARNING)输出日志,并允许配置日志级别。
  2. 空指针检查

    • 虽然已经添加了一些对 m_timer 的空指针检查,但在其他地方(如 m_watcher)仍然需要类似的检查。
    • 建议:对所有可能为空的成员变量进行空指针检查。

代码性能

  1. 定时器使用

    • 使用定时器来延迟处理文件变化是合理的,但可以考虑使用更精确的定时器或事件驱动的方式。
    • 建议:评估是否需要使用 QTimer::singleShot 替代 start/stop 模式。
  2. 目录监控效率

    • 当前代码对每个目录都单独监控,当监控大量目录时可能会影响性能。
    • 建议:考虑批量处理目录监控,减少系统调用。

代码安全

  1. 路径处理

    • 代码中使用了大量的文件路径操作,但没有对路径进行规范化处理。
    • 建议:使用 QDir::cleanPath() 规范化路径,避免路径解析问题。
  2. 资源管理

    • clear() 函数中,定时器被直接删除,但没有确保没有其他地方正在使用它。
    • 建议:使用Qt的父子对象机制或智能指针来管理资源。
  3. 并发安全

    • 代码中没有明显的线程同步机制,如果多个线程同时访问共享数据可能会导致问题。
    • 建议:如果需要在多线程环境中使用,应添加适当的同步机制。

具体改进建议

  1. addWatcher 函数

    • 路径分类逻辑很好,但可以添加更多的错误处理和日志记录。
    • 建议:对每个路径添加更详细的验证,确保路径格式正确。
  2. checkPendingDirectories 函数

    • 函数功能清晰,但可以考虑将目录检查逻辑提取为单独的函数,提高代码复用性。
    • 建议:添加超时机制,避免无限等待目录创建。
  3. onNeedSendPictures 函数

    • 可以考虑使用更高效的数据结构来存储文件列表,如 QSet 替代 QStringList 以提高查找效率。
    • 建议:添加批量处理机制,减少频繁的信号发送。
  4. 内存管理

    • 建议使用Qt的父子对象机制,让父对象自动管理子对象的生命周期。
    • 对于动态分配的对象,考虑使用 QPointerQSharedPointer 来避免悬空指针。

总体而言,这段代码实现了基本功能,但可以通过上述建议进行改进,以提高代码的健壮性、性能和可维护性。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lichaofan2008, lzwind

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

@lichaofan2008
Copy link
Author

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Sep 2, 2025

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 00830bb into linuxdeepin:release/eagle Sep 2, 2025
13 of 14 checks passed
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