Differential D11745 Diff 31811 src/engine/databasesanitizer.cpp

Changeset View

Standalone View

View Options

src/engine/databasesanitizer.cpp

Show All 24 Lines
25#include "idutils.h" 25​#include "idutils.h"
26 26​
27#include <sys/sysmacros.h> 27​#include <sys/sysmacros.h>
28#include <unordered_map> 28​#include <unordered_map>
29 29​
30#include <KLocalizedString> 30​#include <KLocalizedString>
31#include <QFileInfo> 31​#include <QFileInfo>
32#include <QStorageInfo> 32​#include <QStorageInfo>
33​#include <QDebug>
33 34​
34namespace Baloo 35​namespace Baloo
35{ 36​{
36 37​
37class DatabaseSanitizerImpl { 38​class DatabaseSanitizerImpl {
38public: 39​public:
39 DatabaseSanitizerImpl(const Database& db, Transaction::TransactionType type) 40​ DatabaseSanitizerImpl(const Database& db, Transaction::TransactionType type)
40 : m_transaction(new Transaction(db, type)) 41​ : m_transaction(new Transaction(db, type))
Show All 27 Lines45public:
68 * and every item is collected. 69​ * and every item is collected.
69 * Positive numbers are including filters collecting only the mentioned device ids. 70​ * Positive numbers are including filters collecting only the mentioned device ids.
70 * Negative numbers are excluding filters collecting everything but the mentioned device ids. 71​ * Negative numbers are excluding filters collecting everything but the mentioned device ids.
71 * 72​ *
72 * \p missingOnly Only inaccessible items are collected. 73​ * \p missingOnly Only inaccessible items are collected.
73 * 74​ *
74 * \p urlFilter Filter result urls. Default is null = Collect everything. 75​ * \p urlFilter Filter result urls. Default is null = Collect everything.
75 */ 76​ */
76 QVector<FileInfo> createList( 77​ QPair<QVector<FileInfo>, QString> createList(
77 const QVector<qint64>& deviceIds, 78​ const QVector<qint64>& deviceIds,
78 const bool purging, 79​ const DatabaseSanitizer::ItemAccessFilters accessFilter,
79 const QSharedPointer<QRegularExpression>& urlFilter 80​ const QSharedPointer<QRegularExpression>& urlFilter
80 ) const 81​ ) const
81 { 82​ {
82 Q_ASSERT(m_transaction); 83​ Q_ASSERT(m_transaction);
83 84​
84 const auto docUrlDb = DocumentUrlDB(m_transaction->m_dbis.idTreeDbi, 85​ const auto docUrlDb = DocumentUrlDB(m_transaction->m_dbis.idTreeDbi,
85 m_transaction->m_dbis.idFilenameDbi, 86​ m_transaction->m_dbis.idFilenameDbi,
86 m_transaction->m_txn); 87​ m_transaction->m_txn);
87 const auto map = docUrlDb.toTestMap(); 88​ const auto map = docUrlDb.toTestMap();
88 const auto keys = map.keys(); 89​ const auto keys = map.keys();
89 QVector<FileInfo> result; 90​ QVector<FileInfo> result;
90 result.reserve(keys.count()); 91​ result.reserve(keys.count());
91 uint i = 0; 92​ uint i = 0;
92 uint max = keys.count(); 93​ uint max = keys.count();
93 QVector<quint32> includeIds; 94​ QVector<quint32> includeIds;
94 QVector<quint32> excludeIds; 95​ QVector<quint32> excludeIds;
95 for (qint64 deviceId : deviceIds) { 96​ for (qint64 deviceId : deviceIds) {
96 if (deviceId > 0) { 97​ if (deviceId > 0) {
97 includeIds.append(deviceId); 98​ includeIds.append(deviceId);
98 } else if (deviceId < 0) { 99​ } else if (deviceId < 0) {
99 excludeIds.append(-deviceId); 100​ excludeIds.append(-deviceId);
100 } 101​ }
101 } 102​ }
102 103​ int ignoredCount = 0;
104​ int accessibleCount = 0;
103 QTextStream err(stderr); 105​ QTextStream err(stderr);
104 for (quint64 id: keys) { 106​ for (auto it = map.constBegin(), end = map.constEnd(); it != end; it++) {
brunsUnsubmitted
Done

As you are not iterating over sorted keys, just directly iterate on the map.

bruns: As you are not iterating over sorted keys, just directly iterate on the map.
105 printProgress(err, i, max, 100); 107​ printProgress(err, i, max, 100);
106 108​ quint64 id = it.key();
107 const quint32* arr = reinterpret_cast<quint32*>(&id); 109​ const quint32* arr = reinterpret_cast<quint32*>(&id);
108 const auto url = docUrlDb.get(id); ​
109 FileInfo info; 110​ FileInfo info;
brunsUnsubmitted
Done

Why are you fetching the url a second time here? url == map[key].second.

bruns: Why are you fetching the url a second time here? url == map[key].second.
110 info.deviceId = arr[0]; 111​ info.deviceId = arr[0];
111 info.inode = arr[1]; 112​ info.inode = arr[1];
112 info.url = url; 113​ info.url = it.value();
113 info.accessible = !url.isEmpty() && QFileInfo::exists(url); 114​ info.accessible = !info.url.isEmpty() && QFileInfo::exists(info.url);
114 if ((!includeIds.isEmpty() && !includeIds.contains(info.deviceId)) 115​ if ((!includeIds.isEmpty() && !includeIds.contains(info.deviceId))
115 || (!excludeIds.isEmpty() && excludeIds.contains(info.deviceId)) 116​ || (!excludeIds.isEmpty() && excludeIds.contains(info.deviceId))
brunsUnsubmitted
Not Done

I think this becomes a little bit clearer if you move it to the top of the map iteration and use a continue; to process the next item, e.g.:

quint64 id= it.key();
deviceId = idToDeviceId(id);  // idutils.h

if (!includeIds.isEmpty() && !includeIds.contains(deviceId)) {
    continue;
} else if (excludeIds.contains(info.deviceId)) {
    continue;
} else if (urlFilter && !urlFilter->match(it.value()).hasMatch())
    continue;
}

checkedFiles += 1;
FileInfo info; 
....
if (info.accessible)
   accessibleCount++

if ((info.accessible && !(accessFilter & IgnoreAvailable)) ||
    (!info.accessible && !(accessFilter & IgnoreUnavailable))) {
    result.append(info);
}

ignoredCount = map.size() - checkedFiles
inaccessibleCount = checkedFiles - accessibleCount

bruns: I think this becomes a little bit clearer if you move it to the top of the map iteration and…
116 || (purging && info.accessible) ​
117 || (urlFilter && !urlFilter->match(info.url).hasMatch()) 117​ || (urlFilter && !urlFilter->match(info.url).hasMatch())
118 ) { 118​ ) {
119 continue; 119​ ignoredCount++;
120 } 120​ } else if (!info.accessible) {
121 result.append(info); 121​ result.append(info);
122​ } else if (accessFilter & DatabaseSanitizer::IgnoreAvailable) {
123​ accessibleCount++;
124​ } else {
125​ result.append(info);
126​ accessibleCount++;
122 } 127​ }
123 return result; 128​ }
129​ return {result, i18n("Total: %1, Ignored: %2, Accessible: %3, Inaccessible: %4",
130​ map.size(),
131​ ignoredCount,
132​ accessibleCount,
133​ map.size() - ignoredCount - accessibleCount
134​ )};
michaelhAuthorUnsubmitted
Not Done

I'm not content with this solution. Still, it's the best I could do without bloating the code too much.

michaelh: I'm not content with this solution. Still, it's the best I could do without bloating the code…
brunsUnsubmitted
Not Done

for the second, return

struct summary {
    quint64 ignored;
    quint64 accessible;
    quint64 inaccessible;
};
bruns: for the second, return ``` struct summary { quint64 ignored; quint64 accessible…
124 } 135​ }
125 136​
126 struct DeviceInfo { 137​ struct DeviceInfo {
127 quint32 id = 0; 138​ quint32 id = 0;
128 int items = 0; 139​ int items = 0;
129 FSUtils::DeviceInfo fsInfo = {}; 140​ FSUtils::DeviceInfo fsInfo = {};
130 bool mounted = false; 141​ bool mounted = false;
131 }; 142​ };
Show All 20 Lines162 if (deviceInfos.count(id) == 1) {
152 const DeviceInfo devInf = deviceInfos[deviceInfo.id]; 163​ const DeviceInfo devInf = deviceInfos[deviceInfo.id];
153 deviceInfo.fsInfo = devInf.fsInfo ; 164​ deviceInfo.fsInfo = devInf.fsInfo ;
154 deviceInfo.mounted = devInf.mounted; 165​ deviceInfo.mounted = devInf.mounted;
155 return 0; 166​ return 0;
156 } 167​ }
157 return 1; 168​ return 1;
158 } 169​ }
159 170​
160 QMap<quint32, DeviceInfo> createDeviceList(const QVector<FileInfo>& infos) 171​ QMap<quint32, DeviceInfo> createDeviceList(
172​ const QVector<FileInfo>& infos,
173​ const DatabaseSanitizer::ItemAccessFilters accessFilter = DatabaseSanitizer::IgnoreNone)
161 { 174​ {
162 QMap<quint32, DeviceInfo> usedDevices; 175​ QMap<quint32, DeviceInfo> usedDevices;
163 for (const auto& info : infos) { 176​ for (const auto& info : infos) {
164 usedDevices[info.deviceId].items++; 177​ usedDevices[info.deviceId].items++;
165 } 178​ }
166 for (auto it = usedDevices.begin(), end = usedDevices.end(); it != end; it++) { 179​ for (auto it = usedDevices.begin(), end = usedDevices.end(); it != end;) {
167 fillInDeviceInfo(it.key(), it.value()); 180​ fillInDeviceInfo(it.key(), it.value());
181​
182​ if ((it.value().mounted && !(accessFilter & DatabaseSanitizer::IgnoreMounted))
183​ || (!it.value().mounted && !(accessFilter & DatabaseSanitizer::IgnoreUnmounted))
brunsUnsubmitted
Not Done

You can omit DatabaseSanitizer:: here

bruns: You can omit `DatabaseSanitizer::` here
184​ ) {
185​ it++;
186​ } else {
187​ usedDevices.erase(it++);
188​ }
168 } 189​ }
169 return usedDevices; 190​ return usedDevices;
170 } 191​ }
171 192​
172private: 193​private:
173 Transaction* m_transaction; 194​ Transaction* m_transaction;
174}; 195​};
175} 196​}
Show All 26 Lines
202* Negative numbers are excluding filters printing everything but the mentioned device ids. 223​* Negative numbers are excluding filters printing everything but the mentioned device ids.
203* 224​*
204* \p missingOnly Simulate purging operation. Only inaccessible items are printed. 225​* \p missingOnly Simulate purging operation. Only inaccessible items are printed.
205* 226​*
206* \p urlFilter Filter result urls. Default is null = Print everything. 227​* \p urlFilter Filter result urls. Default is null = Print everything.
207*/ 228​*/
208 void DatabaseSanitizer::printList( 229​ void DatabaseSanitizer::printList(
209 const QVector<qint64>& deviceIds, 230​ const QVector<qint64>& deviceIds,
210 const bool missingOnly, 231​ const ItemAccessFilters accessFilter,
211 const QSharedPointer<QRegularExpression>& urlFilter) 232​ const QSharedPointer<QRegularExpression>& urlFilter)
212{ 233​{
213 auto infos = m_pimpl->createList(deviceIds, missingOnly, urlFilter); 234​ auto infos = m_pimpl->createList(deviceIds, accessFilter, urlFilter);
214 const auto sep = QLatin1Char(' '); 235​ const auto sep = QLatin1Char(' ');
215 QTextStream out(stdout); 236​ QTextStream out(stdout);
216 QTextStream err(stderr); 237​ QTextStream err(stderr);
217 for (const auto& info: infos) { 238​ for (const auto& info: infos.first) {
brunsUnsubmitted
Done

You filter a second time here (first time in createList).

I would propose to use the same format independent of the Ignore(Un)Available.

bruns: You filter a second time here (first time in createList). I would propose to use the same…
218 if (!missingOnly) { 239​ out << QStringLiteral("%1").arg(info.accessible ? "+" : "!")
219 out << QStringLiteral("%1").arg(info.accessible ? "+" : "!") << sep; 240​ << sep << QStringLiteral("device: %1").arg(info.deviceId)
220 } else if (!info.accessible) { ​
221 out << i18n("Missing:") << sep; ​
222 } else { ​
223 Q_ASSERT(false); ​
224 continue; ​
225 } ​
226 out << QStringLiteral("device: %1").arg(info.deviceId) ​
227 << sep << QStringLiteral("inode: %1").arg(info.inode) 241​ << sep << QStringLiteral("inode: %1").arg(info.inode)
228 << sep << QStringLiteral("url: %1").arg(info.url) 242​ << sep << QStringLiteral("url: %1").arg(info.url)
229 << endl; 243​ << endl;
230 } 244​ }
231 err << i18n("Found %1 matching items", infos.count()) << endl; 245​ err << infos.second << endl;
brunsUnsubmitted
Not Done

This needs improvement - if you only print the inaccessible ones, the count is off.
Better explicitly mention number of Total, Accessible, Inaccessible.

bruns: This needs improvement - if you only print the inaccessible ones, the count is off. Better…
michaelhAuthorUnsubmitted
Not Done

see above

michaelh: see above
232 246​
233} 247​}
234 248​
235void DatabaseSanitizer::printDevices(const QVector<qint64>& deviceIds, const bool missingOnly) 249​void DatabaseSanitizer::printDevices(const QVector<qint64>& deviceIds, const ItemAccessFilters accessFilter)
236{ 250​{
237 auto infos = m_pimpl->createList(deviceIds, false, nullptr); 251​ auto infos = m_pimpl->createList(deviceIds, accessFilter, nullptr);
238 auto usedDevices = m_pimpl->createDeviceList(infos); 252​ auto usedDevices = m_pimpl->createDeviceList(infos.first, accessFilter);
239 253​
240 const auto sep = QLatin1Char(' '); 254​ const auto sep = QLatin1Char(' ');
241 QTextStream out(stdout); 255​ QTextStream out(stdout);
242 QTextStream err(stderr); 256​ QTextStream err(stderr);
243 int matchCount = 0; 257​ int matchCount = 0;
244 for (const auto& dev : usedDevices) { 258​ for (const auto& dev : usedDevices) {
245 if (missingOnly && dev.mounted) { 259​ if ((accessFilter & IgnoreMounted) && dev.mounted) {
246 continue; 260​ continue;
247 } 261​ }
248 matchCount++; 262​ matchCount++;
249 // TODO coloring would be nice, but "...|grep '^!'" does not work with it. 263​ // TODO coloring would be nice, but "...|grep '^!'" does not work with it.
250 // out << QStringLiteral("%1").arg(dev.mounted ? "+" : "\033[1;31m!") 264​ // out << QStringLiteral("%1").arg(dev.mounted ? "+" : "\033[1;31m!")
251 // Can be done, see: https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/global/qlogging.cpp#n263 265​ // Can be done, see: https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/global/qlogging.cpp#n263
252 out << QStringLiteral("%1").arg(dev.mounted ? "+" : "!") 266​ out << QStringLiteral("%1").arg(dev.mounted ? "+" : "!")
253 << sep << QStringLiteral("device:%1").arg(dev.id) 267​ << sep << QStringLiteral("device:%1").arg(dev.id)
Show All 16 Lines