Skip to content

Conversation

@wangrong1069
Copy link
Contributor

No description provided.

Refactor the configuration loading logic into a new `update_config()` private method
to centralize configuration fetching and processing. This ensures that all
configuration values are consistently reloaded when the D-Bus configuration changes.

Improvements to path handling include:
- `process_blacklist_paths()`: Canonicalize absolute paths, remove duplicate entries
    using an `unordered_set`, and explicitly ensure `.cache` is always part of
    the blacklist for more robust event filtering.
- `indexing_paths_`: Properly resolve `$HOME` and filter out invalid relative paths.

These changes enhance the robustness and maintainability of the configuration
management within the daemon.
- Add new index job type 'refresh' for index management
- Refactor event_handler_config from shared_ptr to value semantics
- Implement config change handling in base_event_handler and default_event_handler
- Add thread-safe config access with mutex protection
- Enhance blacklist path processing with absolute path detection
- Improve file index manager refresh with NRT (Near Real-Time) support
- Add config event queue for handling configuration changes asynchronously
- Support blacklist path additions/removals without full index restart
- Add config event types for different blacklist operations
- Implement string list diff algorithm for config change detection

Task: https://pms.uniontech.com/task-view-385089.html
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.

Sorry @wangrong1069, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot
Copy link

deepin pr auto review

我来对这段代码的变更进行审查,主要从语法逻辑、代码质量、性能和安全几个方面进行分析:

  1. 语法逻辑改进建议:
  • 在 base_event_handler.h 中,将 std::shared_ptr<event_handler_config> 改为 event_handler_config 可能会增加内存开销,建议评估是否必要
  • 新增的 refresh 枚举值应该添加相应的注释说明其用途
  • handle_config_change 方法的返回值类型应该使用更明确的枚举类型,而不是简单的 bool
  1. 代码质量改进建议:
  • 在 default_event_handler.cpp 中,新增的配置事件处理逻辑较复杂,建议拆分成更小的函数
  • process_blacklist_paths 函数中的路径处理逻辑可以提取为单独的工具函数
  • 建议为新增的配置事件类型(ACT_BLACKLIST_ADD_*)添加更详细的文档注释
  1. 性能优化建议:
  • get_string_list_diff 函数使用了 unordered_set,但每次调用都要重新构建,建议考虑缓存机制
  • 在处理大量配置变更时,可以考虑批量处理而不是逐个处理
  • refresh_indexes 函数新增的 nrt 参数应该有更清晰的文档说明其性能影响
  1. 安全性改进建议:
  • determine_blacklist_path 函数中,应该增加对路径遍历攻击的防护
  • push_config_event 函数中的路径复制应该增加长度检查
  • 配置变更处理应该增加权限检查,确保只有授权用户可以修改配置
  1. 其他建议:
  • 建议添加更多的错误处理和日志记录
  • 配置变更应该支持事务性操作,确保配置的一致性
  • 考虑添加配置变更的回滚机制
  1. 具体代码改进:
// 在 base_event_handler.h 中
enum class ConfigChangeResult {
    SUCCESS,
    NOT_SUPPORTED,
    INVALID_VALUE,
    ACCESS_DENIED
};

// 在 default_event_handler.cpp 中
class ConfigEventHandler {
public:
    explicit ConfigEventHandler(GAsyncQueue* queue);
    bool handleBlacklistChange(const std::vector<std::string>& old_paths, 
                             const std::vector<std::string>& new_paths);
private:
    void processPathAddition(const std::string& path);
    void processPathRemoval(const std::string& path);
    GAsyncQueue* queue_;
};

这些改进建议旨在提高代码的可维护性、性能和安全性。建议根据实际需求和资源情况逐步实施这些改进。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@wangrong1069
Copy link
Contributor Author

/merge

@deepin-bot deepin-bot bot merged commit 07c95a0 into linuxdeepin:develop/snipe Dec 25, 2025
17 checks passed
@wangrong1069 wangrong1069 deleted the pr1222 branch December 25, 2025 05:43
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