Skip to content

fix: Implement reconcileIncompleteIndex to manage stale filetime entries#620

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:master
May 20, 2026
Merged

fix: Implement reconcileIncompleteIndex to manage stale filetime entries#620
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:master

Conversation

@dengzhongyuan365-dev

Copy link
Copy Markdown
Member
  • Added a new method to reconcile incomplete index entries in the database, ensuring that stale filetime entries are cleared when no corresponding search data exists.
  • Introduced helper function to parse application language from metadata path.
  • Enhanced filetime entry management in the database by adding methods for inserting single filetime entries and counting search entries.

Log: Implement reconcileIncompleteIndex for better filetime management.

bug: https://pms.uniontech.com/bug-view-362095.html

lzwind
lzwind previously approved these changes May 20, 2026
- Added a new method to reconcile incomplete index entries in the database, ensuring that stale filetime entries are cleared when no corresponding search data exists.
- Introduced helper function to parse application language from metadata path.
- Enhanced filetime entry management in the database by adding methods for inserting single filetime entries and counting search entries.

Log: Implement reconcileIncompleteIndex for better filetime management.

bug: https://pms.uniontech.com/bug-view-362095.html
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已经仔细审查了你提供的Git Diff。这次代码变更的主要目的是修复搜索索引不一致的问题(即文件时间记录存在,但实际的搜索索引数据丢失),通过新增 reconcileIncompleteIndex 方法在启动时检测并清理脏数据,并将 filetime 的写入时机从解析前推迟到解析成功后,以确保数据的一致性。

总体来说,这个修复逻辑非常清晰且必要。但我在代码语法、逻辑、性能和安全性方面发现了一些需要改进的地方,特别是一个会导致编译失败的严重Bug

以下是详细的审查意见:

1. 语法与逻辑

🚨 严重问题:未声明的变量 systemType

parseAppLangFromMdPath 函数中,有这样一段代码:

if (systemType.contains(list.at(list.count() - 4))) {

这里的 systemType 没有在函数内定义,也没有作为参数传入,更不是全局变量(它被放在匿名命名空间中)。这会导致编译失败
改进意见:请确认 systemType 的来源。如果它是一个常量集合,请将其作为参数传入,或者在匿名命名空间中定义它。如果它是类成员变量,由于这是一个静态/自由函数,它也无法直接访问。

逻辑隐患:路径解析的脆弱性

parseAppLangFromMdPath 函数通过硬编码索引(如 count() - 4, count() - 2)来解析路径。如果路径末尾带有斜杠 /(如 /path/to/app/lang/md/),split("/") 会产生空字符串,导致解析出错误的 appName 和 lang。
改进意见:在分割路径前,先移除末尾的斜杠。

const QStringList list = path.endsWith('/') ? path.chopped(1).split("/") : path.split("/");

逻辑改进:handleDb 中的冗余条件移除

helpermanager.cpp 中,移除了 !addTime.isEmpty() 的判断:

-    if (!list.isEmpty() && !addTime.isEmpty()) {
+    if (!list.isEmpty()) {

这是合理的,因为 filetime 的写入被推迟到了 onRecvParseMsg 中。但请确保 addTime 变量在这个作用域内不再被使用,否则可能会引发其他问题。从Diff来看,addTime 似乎已经变成了死代码,建议一并清理。

2. 代码性能

性能瓶颈:循环内的数据库查询

reconcileIncompleteIndex 中,你在 for 循环内对每个文件执行了数据库查询:

if (dbObj->searchEntryCount(appName, lang) == 0) { ... }

如果 mapFile 包含数百个文件,这会导致数百次同步的数据库 I/O 操作,严重影响启动性能。
改进意见

  1. 批量查询:在 SearchDb 中新增一个方法,一次性查询出所有存在索引的 appNamelang 的集合(如 QSet<QString>),然后在内存中进行 contains 判断,这将极大地提升性能。
  2. 限制检查范围:是否真的需要每次启动都遍历所有文件?能否只对最近修改的文件或新增的文件做一致性校验?

性能优化:避免 QStringList 的隐式共享失效

parseAppLangFromMdPath 中:

const QStringList list = path.split("/");

由于 list 在后续被多次通过 at() 访问,且函数结束即销毁,这没有大问题。但在 C++11 及以上版本中,如果不需要修改,使用 const auto& 可以避免潜在的拷贝(尽管 Qt 有隐式共享机制,但引用更保险):

const QStringList list = path.split("/");
// 后续访问: list[list.size() - 4]

3. 代码安全

路径遍历与注入风险

mdPath 来自文件系统,最终被传入 SQL 查询。虽然 SearchDb::searchEntryCount 使用了 query.bindValue,这很好地防范了SQL注入,值得表扬。
但是,appNamelang 是直接从路径中解析出来的。恶意的路径名(如包含特殊字符、极长字符串)可能会对数据库或下游逻辑造成风险。
改进意见:在 parseAppLangFromMdPath 解析出 appNamelang 后,增加基本的合法性校验(例如:不为空,符合正则规范等)。

文件时间格式的硬编码

代码中多次出现 fileInfo.lastModified().toString("yyyy-MM-dd hh:mm:ss.zzz")
改进意见:建议将时间格式字符串提取为常量,避免“魔法字符串”,并确保整个项目的时间格式统一。

4. 重构建议代码

针对上述问题,我为你重构了部分代码,请参考:

修正并增强 parseAppLangFromMdPath

namespace {

// 假设 systemType 是一个全局的 QStringList,建议将其定义移到这里或作为参数传入
const QStringList SYSTEM_TYPES = {"system1", "system2"}; 

bool parseAppLangFromMdPath(const QString &path, QString &appName, QString &lang)
{
    if (path.contains("video-guide")) {
        return false;
    }

    // 移除末尾的斜杠,防止 split 产生空字符串导致索引偏移
    QString cleanPath = path;
    while (cleanPath.endsWith('/')) {
        cleanPath.chop(1);
    }

    const QStringList list = cleanPath.split("/");
    if (list.count() < 4) {
        return false;
    }

    // 建议使用 size() 替代 count() (C++11 标准容器习惯)
    if (SYSTEM_TYPES.contains(list.at(list.size() - 4))) {
        lang = list.at(list.size() - 2);
        appName = list.at(list.size() - 3);
        return true;
    }

    if (list.contains("application") || list.contains("system")) {
        lang = list.at(list.size() - 2);
        appName = list.at(list.size() - 4);
        // 增加安全校验:解析出的字段不应为空
        if (appName.isEmpty() || lang.isEmpty()) {
            return false;
        }
        return true;
    }

    return false;
}

} // namespace

优化 SearchDb 的一致性检查(性能优化方案建议):

// 在 search_db.h 中新增:
QSet<QString> selectExistingAppLangKeys();

// 在 search_db.cpp 中实现:
QSet<QString> SearchDb::selectExistingAppLangKeys() {
    Q_ASSERT(p_->db.isOpen());
    QSet<QString> existingKeys;
    QSqlQuery query(p_->db);
    if (query.exec("SELECT appName || '_' || lang FROM search")) {
        while (query.next()) {
            existingKeys.insert(query.value(0).toString());
        }
    } else {
        qCWarning(app) << "selectExistingAppLangKeys failed:" << query.lastError().text();
    }
    return existingKeys;
}

// 然后在 reconcileIncompleteIndex 中:
void helperManager::reconcileIncompleteIndex(QMap<QString, QString> &mapFile,
                                             const QMap<QString, QString> &mapNow)
{
    // 一次性查出所有存在的索引键值对,避免 N+1 查询问题
    QSet<QString> existingKeys = dbObj->selectExistingAppLangKeys();
    
    QStringList stalePaths;
    // ... 循环内判断:
    QString key = appName + "_" + lang;
    if (!existingKeys.contains(key)) {
        stalePaths.append(mdPath);
    }
}

总结

  1. 最高优先级:修复 systemType 未定义的编译错误。
  2. 高优先级:优化循环内的数据库查询逻辑,避免启动卡顿。
  3. 中优先级:增强路径解析的鲁棒性(处理末尾斜杠、校验解析结果)。
  4. 低优先级:清理死代码(如 addTime),提取时间格式魔法字符串为常量。

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev, 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

@dengzhongyuan365-dev

Copy link
Copy Markdown
Member Author

/forcemerge

@deepin-bot

deepin-bot Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

This pr force merged! (status: unstable)

@deepin-bot deepin-bot Bot merged commit f9a71c1 into linuxdeepin:master May 20, 2026
17 of 18 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