Changeset View
Changeset View
Standalone View
Standalone View
src/file/unindexedfileiterator.cpp
| Show All 29 Lines | |||||
| 30 | using namespace Baloo; | 30 | using namespace Baloo; | ||
| 31 | 31 | | |||
| 32 | UnIndexedFileIterator::UnIndexedFileIterator(const FileIndexerConfig* config, Transaction* transaction, const QString& folder) | 32 | UnIndexedFileIterator::UnIndexedFileIterator(const FileIndexerConfig* config, Transaction* transaction, const QString& folder) | ||
| 33 | : m_config(config) | 33 | : m_config(config) | ||
| 34 | , m_transaction(transaction) | 34 | , m_transaction(transaction) | ||
| 35 | , m_iter(config, folder, FilteredDirIterator::FilesAndDirs) | 35 | , m_iter(config, folder, FilteredDirIterator::FilesAndDirs) | ||
| 36 | , m_mTimeChanged(false) | 36 | , m_mTimeChanged(false) | ||
| 37 | , m_cTimeChanged(false) | 37 | , m_cTimeChanged(false) | ||
| 38 | , m_nameChanged(false) | ||||
| 38 | { | 39 | { | ||
| 39 | } | 40 | } | ||
| 40 | 41 | | |||
| 41 | UnIndexedFileIterator::~UnIndexedFileIterator() | 42 | UnIndexedFileIterator::~UnIndexedFileIterator() | ||
| 42 | { | 43 | { | ||
| 43 | } | 44 | } | ||
| 44 | 45 | | |||
| 45 | QString UnIndexedFileIterator::filePath() const | 46 | QString UnIndexedFileIterator::filePath() const | ||
| Show All 11 Lines | 57 | { | |||
| 57 | return m_mTimeChanged; | 58 | return m_mTimeChanged; | ||
| 58 | } | 59 | } | ||
| 59 | 60 | | |||
| 60 | bool UnIndexedFileIterator::cTimeChanged() const | 61 | bool UnIndexedFileIterator::cTimeChanged() const | ||
| 61 | { | 62 | { | ||
| 62 | return m_cTimeChanged; | 63 | return m_cTimeChanged; | ||
| 63 | } | 64 | } | ||
| 64 | 65 | | |||
| 66 | bool UnIndexedFileIterator::nameChanged() const | ||||
| 67 | { | ||||
| 68 | return m_nameChanged; | ||||
| 69 | } | ||||
| 70 | | ||||
| 65 | QString UnIndexedFileIterator::next() | 71 | QString UnIndexedFileIterator::next() | ||
| 66 | { | 72 | { | ||
| 67 | while (1) { | 73 | while (1) { | ||
| 68 | const QString filePath = m_iter.next(); | 74 | const QString filePath = m_iter.next(); | ||
| 69 | m_mTimeChanged = false; | 75 | m_mTimeChanged = false; | ||
| 70 | m_cTimeChanged = false; | 76 | m_cTimeChanged = false; | ||
| 77 | m_nameChanged = false; | ||||
| 71 | 78 | | |||
| 72 | if (filePath.isEmpty()) { | 79 | if (filePath.isEmpty()) { | ||
| 73 | m_mimetype.clear(); | 80 | m_mimetype.clear(); | ||
| 74 | return QString(); | 81 | return QString(); | ||
| 75 | } | 82 | } | ||
| 76 | 83 | | |||
| 77 | // This mimetype may not be completely accurate, but that's okay. This is | 84 | // This mimetype may not be completely accurate, but that's okay. This is | ||
| 78 | // just the initial phase of indexing. The second phase can try to find | 85 | // just the initial phase of indexing. The second phase can try to find | ||
| Show All 19 Lines | 96 | { | |||
| 98 | quint64 fileId = filePathToId(QFile::encodeName(filePath)); | 105 | quint64 fileId = filePathToId(QFile::encodeName(filePath)); | ||
| 99 | Q_ASSERT_X(fileId, "UnIndexedFileIterator::shouldIndex", "file id is 0"); | 106 | Q_ASSERT_X(fileId, "UnIndexedFileIterator::shouldIndex", "file id is 0"); | ||
| 100 | if (!fileId) { | 107 | if (!fileId) { | ||
| 101 | return true; | 108 | return true; | ||
| 102 | } | 109 | } | ||
| 103 | 110 | | |||
| 104 | DocumentTimeDB::TimeInfo timeInfo = m_transaction->documentTimeInfo(fileId); | 111 | DocumentTimeDB::TimeInfo timeInfo = m_transaction->documentTimeInfo(fileId); | ||
| 105 | 112 | | |||
| 106 | // A folders mtime is updated when a new file is added / removed / renamed | | |||
| 107 | // we don't really need to reindex a folder when that happens | | |||
| 108 | // In fact, we never need to reindex a folder | | |||
| 109 | if (timeInfo.mTime && fileInfo.isDir()) { | | |||
| 110 | return false; | | |||
| 111 | } | | |||
| 112 | | ||||
| 113 | if (timeInfo.mTime != fileInfo.lastModified().toTime_t()) { | 113 | if (timeInfo.mTime != fileInfo.lastModified().toTime_t()) { | ||
| 114 | m_mTimeChanged = true; | 114 | m_mTimeChanged = true; | ||
| 115 | } | 115 | } | ||
| 116 | 116 | | |||
| 117 | auto fileMTime = fileInfo.metadataChangeTime().toTime_t(); | 117 | auto fileMTime = fileInfo.metadataChangeTime().toTime_t(); | ||
| 118 | if (timeInfo.cTime != fileMTime) { | 118 | if (timeInfo.cTime != fileMTime) { | ||
poboiko: I wanted to note that this line is messed up a bit: either `fileMTime` should be renamed to… | |||||
| 119 | m_cTimeChanged = true; | 119 | m_cTimeChanged = true; | ||
| 120 | } | 120 | } | ||
| 121 | 121 | | |||
| 122 | if (m_mTimeChanged) { | ||||
thats wrong: $:/tmp> touch dir $:/tmp> stat dir File: dir Size: 0 Blocks: 0 IO Block: 4096 regular empty file Device: 3fh/63d Inode: 1892961 Links: 1 Access: (0644/-rw-r--r--) Uid: ( 1000/ sbruens) Gid: ( 100/ users) Access: 2019-06-03 19:21:57.764626372 +0200 Modify: 2019-06-03 19:21:57.764626372 +0200 Change: 2019-06-03 19:21:57.764626372 +0200 Birth: 2019-06-03 19:21:57.764626372 +0200 $:/tmp> mv dir dir_renamed $:/tmp> stat dir_renamed File: dir_renamed Size: 0 Blocks: 0 IO Block: 4096 regular empty file Device: 3fh/63d Inode: 1892961 Links: 1 Access: (0644/-rw-r--r--) Uid: ( 1000/ sbruens) Gid: ( 100/ users) Access: 2019-06-03 19:21:57.764626372 +0200 Modify: 2019-06-03 19:21:57.764626372 +0200 Change: 2019-06-03 19:22:07.096601727 +0200 Birth: 2019-06-03 19:21:57.764626372 +0200 bruns: thats wrong:
```
$:/tmp> touch dir
$:/tmp> stat dir
File: dir
Size: 0 Blocks… | |||||
poboiko: Right, sorry, I've mixed them up. Damn russian locale. | |||||
| 123 | // Since documentUrl is pretty expensive, we want to calculate it only | ||||
| 124 | // if we suspect it could have changed | ||||
| 125 | const QString oldName = m_transaction->documentUrl(fileId); | ||||
| 126 | if (oldName != filePath) { | ||||
| 127 | m_nameChanged = true; | ||||
| 128 | qCDebug(BALOO) << "name changed:" << oldName << filePath; | ||||
| 129 | } | ||||
| 130 | // A folders mtime is updated when a new file is added / removed / renamed | ||||
The comment does not match - the mtime changes when something in the directory is modified, when the directory is renamed, the ctime changes. bruns: The comment does not match - the mtime changes when something **in** the directory is modified… | |||||
The comment is not mine - but that's precisely what it says. We don't want to reindex a directory every time something inside it modifies. poboiko: The comment is not mine - but that's precisely what it says. We don't want to reindex a… | |||||
| 131 | // we don't really need to reindex a folder when that happens | ||||
| 132 | // We only want to reindex folder if it was renamed | ||||
| 133 | if (fileInfo.isDir() && !m_nameChanged) { | ||||
| 134 | return false; | ||||
| 135 | } | ||||
| 136 | } | ||||
| 137 | | ||||
| 122 | if (m_mTimeChanged || m_cTimeChanged) { | 138 | if (m_mTimeChanged || m_cTimeChanged) { | ||
| 123 | qCDebug(BALOO) << "mtime/ctime changed:" | 139 | qCDebug(BALOO) << "mtime/ctime changed:" | ||
| 124 | << timeInfo.mTime << fileInfo.lastModified().toTime_t() | 140 | << timeInfo.mTime << fileInfo.lastModified().toTime_t() | ||
| 125 | << timeInfo.cTime << fileMTime; | 141 | << timeInfo.cTime << fileMTime; | ||
| 142 | | ||||
bruns: This whole block belongs into a separate block, executed `if (m_cTimeChanged)`. | |||||
poboiko: Did you mean `m_mTimeChanged`? | |||||
bruns: obviously not ... | |||||
bruns: unrelated whitespace change | |||||
| 126 | return true; | 143 | return true; | ||
| 127 | } | 144 | } | ||
| 128 | 145 | | |||
| 129 | return false; | 146 | return false; | ||
| 130 | } | 147 | } | ||
I wanted to note that this line is messed up a bit: either fileMTime should be renamed to fileCTime, or even removed, just like three lines above.
(it looks like the variable was introduced some time ago inside #ifdef QT < 5.10.0, which was removed recently with Qt dependency bump).
Since that's just some cosmetic change, I can change it in this patch as well. Or should I make a new one?