Changeset View
Standalone View
src/engine/experimental/databasesanitizer.cpp
Show All 35 Lines | |||||
36 | class DatabaseSanitizerImpl { | 36 | class DatabaseSanitizerImpl { | ||
37 | public: | 37 | public: | ||
38 | DatabaseSanitizerImpl(const Database& db, Transaction::TransactionType type) | 38 | DatabaseSanitizerImpl(const Database& db, Transaction::TransactionType type) | ||
39 | : m_transaction(new Transaction(db, type)) | 39 | : m_transaction(new Transaction(db, type)) | ||
40 | { | 40 | { | ||
41 | } | 41 | } | ||
42 | 42 | | |||
43 | public: | 43 | public: | ||
44 | | ||||
44 | /** | 45 | /** | ||
45 | * \brief Basic info about database items | 46 | * \brief Basic info about database items | ||
46 | */ | 47 | */ | ||
47 | struct FileInfo { | 48 | struct FileInfo { | ||
48 | quint32 deviceId = 0; | 49 | quint32 deviceId = 0; | ||
49 | quint32 inode = 0; | 50 | quint32 inode = 0; | ||
50 | QString url = QString(); | 51 | quint64 id = 0; | ||
52 | bool isSymLink = false; | ||||
51 | bool accessible = true; | 53 | bool accessible = true; | ||
54 | QString url; | ||||
52 | }; | 55 | }; | ||
53 | 56 | | |||
54 | void printProgress(QTextStream& out, uint& cur, const uint max, const uint step) const | 57 | void printProgress(QTextStream& out, uint& cur, const uint max, const uint step) const | ||
55 | { | 58 | { | ||
56 | if (cur % step == 0) { | 59 | if (cur % step == 0) { | ||
57 | out << QStringLiteral("%1%2\r").arg(100 * cur / max, 6).arg("%", -16); | 60 | out << QStringLiteral("%1%2\r").arg(100 * cur / max, 6).arg("%", -16); | ||
58 | out.flush(); | 61 | out.flush(); | ||
59 | } | 62 | } | ||
▲ Show 20 Lines • Show All 62 Lines • ▼ Show 20 Line(s) | 117 | for (auto it = map.constBegin(), end = map.constEnd(); it != end; it++) { | |||
122 | } else if (urlFilter && !urlFilter->match(it.value()).hasMatch()) { | 125 | } else if (urlFilter && !urlFilter->match(it.value()).hasMatch()) { | ||
123 | continue; | 126 | continue; | ||
124 | } | 127 | } | ||
125 | 128 | | |||
126 | FileInfo info; | 129 | FileInfo info; | ||
127 | info.deviceId = deviceId; | 130 | info.deviceId = deviceId; | ||
128 | info.inode = idToInode(id); | 131 | info.inode = idToInode(id); | ||
129 | info.url = QFile::decodeName(it.value()); | 132 | info.url = QFile::decodeName(it.value()); | ||
130 | info.accessible = !info.url.isEmpty() && QFileInfo::exists(info.url); | 133 | info.id = id; | ||
134 | QFileInfo fileInfo(info.url); | ||||
135 | info.accessible = !info.url.isEmpty() && fileInfo.exists(); | ||||
131 | 136 | | |||
132 | if (info.accessible && (accessFilter & DatabaseSanitizer::IgnoreAvailable)) { | 137 | if (info.accessible && (accessFilter & DatabaseSanitizer::IgnoreAvailable)) { | ||
133 | continue; | 138 | continue; | ||
134 | } else if (!info.accessible && (accessFilter & DatabaseSanitizer::IgnoreUnavailable)) { | 139 | } else if (!info.accessible && (accessFilter & DatabaseSanitizer::IgnoreUnavailable)) { | ||
135 | continue; | 140 | continue; | ||
136 | } | 141 | } | ||
137 | 142 | | |||
143 | info.isSymLink = fileInfo.isSymLink(); | ||||
144 | | ||||
138 | result.append(info); | 145 | result.append(info); | ||
139 | summary.ignored--; | 146 | summary.ignored--; | ||
140 | if (info.accessible) { | 147 | if (info.accessible) { | ||
141 | summary.accessible++; | 148 | summary.accessible++; | ||
142 | } | 149 | } | ||
143 | } | 150 | } | ||
144 | return {result, summary}; | 151 | return {result, summary}; | ||
145 | } | 152 | } | ||
Show All 11 Lines | 155 | static QMap<quint32, QStorageInfo> storageInfos = []() { | |||
157 | } | 164 | } | ||
158 | return result; | 165 | return result; | ||
159 | }(); | 166 | }(); | ||
160 | 167 | | |||
161 | QStorageInfo info = storageInfos.value(id); | 168 | QStorageInfo info = storageInfos.value(id); | ||
162 | return info; | 169 | return info; | ||
163 | } | 170 | } | ||
164 | 171 | | |||
172 | | ||||
173 | QMap<quint32, bool> deviceFilters(QVector<FileInfo>& infos, const DatabaseSanitizer::ItemAccessFilters accessFilter) | ||||
bruns: Pass in a List/Set of deviceIds, and return the same type. | |||||
Your suggestion in D12222 can be reused. I'll do it there. michaelh: Your suggestion in D12222 can be reused. I'll do it there. | |||||
174 | { | ||||
175 | QMap<quint32, bool> result; | ||||
176 | for (const auto& info : infos) { | ||||
177 | result[info.deviceId] = false; | ||||
178 | } | ||||
179 | | ||||
180 | for (auto it = result.begin(), end = result.end(); it != end; it++) { | ||||
181 | const auto storageInfo = getStorageInfo(it.key()); | ||||
182 | it.value() = isIgnored(storageInfo, accessFilter); | ||||
183 | } | ||||
184 | return result; | ||||
185 | } | ||||
186 | | ||||
187 | bool isIgnored(const QStorageInfo& storageInfo, const DatabaseSanitizer::ItemAccessFilters accessFilter) | ||||
Are you planning to use this one anywhere else? Otherwise, inside deviceFilters(...): auto isIgnored = [&accessFilter](const QStorageInfo& storageInfo) { if (storageInfo.isValid() && ...) { ... } } bruns: Are you planning to use this one anywhere else?
Otherwise, inside `deviceFilters(...)`:
```… | |||||
Yes. For all output: D12222 michaelh: > Are you planning to use this one anywhere else?
Yes. For all output: D12222 | |||||
188 | { | ||||
189 | if (storageInfo.isValid() && (accessFilter & DatabaseSanitizer::IgnoreMounted)) { | ||||
bruns: `bool mounted = storageInfo.isValid()` | |||||
190 | return true; | ||||
191 | } else if (!storageInfo.isValid() && (accessFilter & DatabaseSanitizer::IgnoreUnmounted)) { | ||||
This else is ok, although not necessary, it emphasizes symmetry (mounted, not mounted) bruns: This `else` is ok, although not necessary, it emphasizes symmetry (mounted, not mounted) | |||||
192 | return true; | ||||
193 | } else if (storageInfo.fileSystemType() == QLatin1String("tmpfs")) { | ||||
Remove the else, and add a newline here ... } if (storageInfo.fileSystemType() == ...) { ... return true; } return false; bruns: Remove the `else`, and add a newline here
```
...
}
if (storageInfo.fileSystemType() == ... | |||||
194 | // Due to the volatility of device ids, an id known by baloo may | ||||
195 | // appear as mounted, but is not what baloo expects. | ||||
196 | // For example at indexing time 43 was the id of a smb share, but | ||||
197 | // at runtime 43 is the id of /run/media/<uid> when other users are | ||||
198 | // logged in. The latter have a type of 'tmpfs' and should be ignored. | ||||
199 | return true; | ||||
200 | } else { | ||||
201 | return false; | ||||
202 | } | ||||
203 | } | ||||
204 | | ||||
205 | void removeDocument(const quint64 id) { | ||||
206 | m_transaction->removeDocument(id); | ||||
207 | } | ||||
208 | | ||||
209 | void commit() { | ||||
210 | m_transaction->abort(); | ||||
bruns: ->commit() | |||||
211 | } | ||||
212 | | ||||
213 | void abort() { | ||||
214 | m_transaction->abort(); | ||||
215 | } | ||||
216 | | ||||
165 | private: | 217 | private: | ||
bruns: Why do you remove the private: here? | |||||
michaelh: 340: m_pimpl->m_transaction->removeDocument(info.id);
and more | |||||
166 | Transaction* m_transaction; | 218 | Transaction* m_transaction; | ||
219 | | ||||
167 | }; | 220 | }; | ||
168 | } | 221 | } | ||
bruns: Is this struct still used? | |||||
169 | 222 | | |||
170 | using namespace Baloo; | 223 | using namespace Baloo; | ||
171 | 224 | | |||
172 | 225 | | |||
173 | DatabaseSanitizer::DatabaseSanitizer(const Database& db, Baloo::Transaction::TransactionType type) | 226 | DatabaseSanitizer::DatabaseSanitizer(const Database& db, Baloo::Transaction::TransactionType type) | ||
174 | : m_pimpl(new DatabaseSanitizerImpl(db, type)) | 227 | : m_pimpl(new DatabaseSanitizerImpl(db, type)) | ||
175 | { | 228 | { | ||
176 | } | 229 | } | ||
bruns: duplicates code from idutils.h | |||||
177 | 230 | | |||
178 | DatabaseSanitizer::DatabaseSanitizer(Database* db, Transaction::TransactionType type) | 231 | DatabaseSanitizer::DatabaseSanitizer(Database* db, Transaction::TransactionType type) | ||
179 | : DatabaseSanitizer(*db, type) | 232 | : DatabaseSanitizer(*db, type) | ||
180 | { | 233 | { | ||
181 | } | 234 | } | ||
182 | 235 | | |||
183 | DatabaseSanitizer::~DatabaseSanitizer() | 236 | DatabaseSanitizer::~DatabaseSanitizer() | ||
184 | { | 237 | { | ||
▲ Show 20 Lines • Show All 89 Lines • ▼ Show 20 Line(s) | 299 | for (auto it = useCount.cbegin(); it != useCount.cend(); it++) { | |||
274 | } | 327 | } | ||
275 | // TODO: see above | 328 | // TODO: see above | ||
276 | // out << QStringLiteral("\033[0m") << endl; | 329 | // out << QStringLiteral("\033[0m") << endl; | ||
277 | out << endl; | 330 | out << endl; | ||
278 | } | 331 | } | ||
279 | 332 | | |||
280 | err << i18n("Found %1 matching in %2 devices", matchCount, useCount.size()) << endl; | 333 | err << i18n("Found %1 matching in %2 devices", matchCount, useCount.size()) << endl; | ||
281 | } | 334 | } | ||
335 | | ||||
336 | void DatabaseSanitizer::removeStaleEntries(const QVector<qint64>& deviceIds, | ||||
337 | const DatabaseSanitizer::ItemAccessFilters accessFilter, | ||||
338 | const bool dryRun, | ||||
339 | const QSharedPointer<QRegularExpression>& urlFilter) | ||||
340 | { | ||||
341 | auto listResult = m_pimpl->createList(deviceIds, IgnoreAvailable, urlFilter); | ||||
342 | | ||||
343 | const auto ignoredDevices = m_pimpl->deviceFilters(listResult.first, accessFilter); | ||||
344 | | ||||
345 | const auto sep = QLatin1Char(' '); | ||||
346 | auto& summary = listResult.second; | ||||
347 | QTextStream out(stdout); | ||||
348 | QTextStream err(stderr); | ||||
349 | for (const auto& info: listResult.first) { | ||||
350 | if (ignoredDevices[info.deviceId] == true) { | ||||
351 | summary.ignored++; | ||||
352 | } else { | ||||
353 | if (info.isSymLink) { | ||||
354 | out << i18n("IgnoredSymbolicLink:"); | ||||
355 | summary.ignored++; | ||||
356 | } else { | ||||
357 | m_pimpl->removeDocument(info.id); | ||||
do the calls to m_transaction using a wrapper, e.g. DatabaseSanitizer::removeDocument(info.id) bruns: do the calls to m_transaction using a wrapper, e.g. `DatabaseSanitizer::removeDocument(info.id)` | |||||
I'm assuming you meant DatabaseSanitizerImpl::removeDocument(info.id). If you meant DatabaseSanitizer::removeDocument, please explain what the advantage is. michaelh: I'm assuming you meant `DatabaseSanitizerImpl::removeDocument(info.id)`. If you meant… | |||||
bruns: Right. | |||||
358 | out << i18n("Removing:"); | ||||
359 | } | ||||
360 | out << sep << QStringLiteral("device: %1").arg(info.deviceId) | ||||
Restructure: bruns: Restructure:
Iterate over listResult.first to populate with deviceIds
Iterate over result to… | |||||
361 | << sep << QStringLiteral("inode: %1").arg(info.inode) | ||||
362 | << sep << QStringLiteral("url: %1").arg(info.url) | ||||
363 | << endl; | ||||
364 | } | ||||
365 | } | ||||
366 | if (dryRun) { | ||||
367 | m_pimpl->abort(); | ||||
bruns: dito | |||||
368 | } else { | ||||
369 | m_pimpl->commit(); | ||||
370 | } | ||||
371 | Q_ASSERT(summary.accessible == 0); | ||||
372 | err << i18nc("numbers", "Removed: %1, Total: %2, Ignored: %3", | ||||
373 | summary.total - summary.ignored, | ||||
bruns: Nitpick - one more space (align " and __s__ummary) | |||||
374 | summary.total, | ||||
375 | summary.ignored) | ||||
376 | << endl; | ||||
377 | } | ||||
bruns: remove `sep`, use `QStringLiteral(" device: %1") instead | |||||
I'd like to keep it this way.
michaelh: I'd like to keep it this way.
# For consistency with `printList` and `printDevices`
# I'm… | |||||
My preference is:
bruns: My preference is:
- Use **one** fixed format string for human readable output, which is… | |||||
bruns: dito `sep`, indent to align `<<` with line above |
Pass in a List/Set of deviceIds, and return the same type.