D12222: baloodb: Use complete access filtering for all outputs
astippich added a comment. Herald added a subscriber: kde-frameworks-devel. @bruns this still applies cleanly to current master, do you think this and the dependent revision are still worth it to merge? REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D1 To: michaelh, #baloo, #frameworks Cc: kde-frameworks-devel, astippich, bruns, gennad, domson, ashaposhnikov, michaelh, spoorun, ngraham, abrahams
D12222: baloodb: Use complete access filtering for all outputs
michaelh marked an inline comment as done. michaelh added a comment. I'm sorry for making so much noise with all those stupid mistakes. It's probably best for me to let this rest for a few days, relax a little and gain some distance. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D1 To: michaelh, #baloo, #frameworks Cc: bruns, ashaposhnikov, michaelh, astippich, spoorun
D12222: baloodb: Use complete access filtering for all outputs
michaelh updated this revision to Diff 32580. michaelh added a comment. - That was no typo REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1?vs=32579=32580 BRANCH sanitize-dry (branched from master) REVISION DETAIL https://phabricator.kde.org/D1 AFFECTED FILES src/engine/experimental/databasesanitizer.cpp src/engine/experimental/databasesanitizer.h src/tools/experimental/baloodb/main.cpp To: michaelh, #baloo, #frameworks Cc: bruns, ashaposhnikov, michaelh, astippich, spoorun
D12222: baloodb: Use complete access filtering for all outputs
michaelh updated this revision to Diff 32579. michaelh added a comment. - Correct typo REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1?vs=32578=32579 BRANCH sanitize-dry (branched from master) REVISION DETAIL https://phabricator.kde.org/D1 AFFECTED FILES src/engine/experimental/databasesanitizer.cpp src/engine/experimental/databasesanitizer.h src/tools/experimental/baloodb/main.cpp To: michaelh, #baloo, #frameworks Cc: bruns, ashaposhnikov, michaelh, astippich, spoorun
D12222: baloodb: Use complete access filtering for all outputs
michaelh updated this revision to Diff 32578. michaelh marked 5 inline comments as done. michaelh added a comment. - Revert most previous changes - Use `isMounted()` and `isObscured()` - Remove documents recursively REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1?vs=32426=32578 BRANCH sanitize-dry (branched from master) REVISION DETAIL https://phabricator.kde.org/D1 AFFECTED FILES src/engine/experimental/databasesanitizer.cpp src/engine/experimental/databasesanitizer.h src/tools/experimental/baloodb/main.cpp To: michaelh, #baloo, #frameworks Cc: bruns, ashaposhnikov, michaelh, astippich, spoorun
D12222: baloodb: Use complete access filtering for all outputs
bruns added inline comments. INLINE COMMENTS > databasesanitizer.cpp:179 > + > +bool isMounted(const quint32 id) { > +return m_devices.mounted.contains(id); `isMounted(id)` can be implemented as `getStorageInfo(id).isValid()` > databasesanitizer.cpp:183 > + > +bool isUnMounted(const quint32 id) { > +return m_devices.unmounted.contains(id); unused, please remove > databasesanitizer.cpp:189 > { > -QMapresult; > +createDeviceStates(infos, accessFilter); > +auto tail = infos.end(); I can not see what this function is useful for. Just inline the FileInfo -> DeviceIds reduction here, and ... > databasesanitizer.cpp:192 > +Summary summary; > +for (const quint32 deviceId : m_devices.ignored) { > +tail = std::remove_if(infos.begin(), tail, ... skip the deviceId if it should be ignored: const auto storageInfo = getStorageInfo(id); if (isDeviceIgnored(storageInfo, accessFilter)) { continue; } > databasesanitizer.cpp:195 > + [deviceId, ] (const FileInfo& info) { > + summary.accessible += (info.deviceId == > deviceId && info.accessible ? 1 : 0); > + return info.deviceId == deviceId; if (info.deviceId != deviceId) { return false; } summary.accessible += info.accessible ? 1 : 0; return true; REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D1 To: michaelh, #baloo, #frameworks Cc: bruns, ashaposhnikov, michaelh, astippich, spoorun
D12222: baloodb: Use complete access filtering for all outputs
michaelh marked 6 inline comments as done. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D1 To: michaelh, #baloo, #frameworks Cc: bruns, ashaposhnikov, michaelh, astippich, spoorun
D12222: baloodb: Use complete access filtering for all outputs
michaelh marked an inline comment as done. michaelh added inline comments. INLINE COMMENTS > databasesanitizer.cpp:196 > + summary.accessible += (info.deviceId == > deviceId && info.accessible ? 1 : 0); > + return info.deviceId == deviceId; > + }); ~3 hrs for this, phew. > michaelh wrote in databasesanitizer.cpp:160 > $ baloodb list --mounted-only Einstein > Listing database contents... > ! device: 43 inode: 319 url: /media/circulans/(tv-analog)/L-Z/Was Einstein > noch nicht wusste.mkv > + device: 46 inode: 319 url: /media/circulans/(doku)/Was Einstein noch > nicht wusste.mkv > > 43 is the correct id for a share. The path is wrong. > 46 is a wrong id for the same share. It should not be listed. The path > however is correct. > > `QStorageInfo` is not enough, I'm afraid. For proper matching mtab must be > read. On the wrong track REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D1 To: michaelh, #baloo, #frameworks Cc: bruns, ashaposhnikov, michaelh, astippich, spoorun
D12222: baloodb: Use complete access filtering for all outputs
michaelh updated this revision to Diff 32426. michaelh added a comment. - Filter by device id REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1?vs=32422=32426 BRANCH sanitize-dry (branched from master) REVISION DETAIL https://phabricator.kde.org/D1 AFFECTED FILES src/engine/experimental/databasesanitizer.cpp src/engine/experimental/databasesanitizer.h src/tools/experimental/baloodb/main.cpp To: michaelh, #baloo, #frameworks Cc: bruns, ashaposhnikov, michaelh, astippich, spoorun
D12222: baloodb: Use complete access filtering for all outputs
michaelh added a comment. I went I little overboard with `remove_if` and had to repair. Device filtering is also broken see inline comment. INLINE COMMENTS > databasesanitizer.cpp:160 > const QByteArray rootPath = > QFile::encodeName(vol.rootPath()); > const auto fsinfo = filePathToStat(rootPath); > const quint32 id = static_cast(fsinfo.st_dev); $ baloodb list --mounted-only Einstein Listing database contents... ! device: 43 inode: 319 url: /media/circulans/(tv-analog)/L-Z/Was Einstein noch nicht wusste.mkv + device: 46 inode: 319 url: /media/circulans/(doku)/Was Einstein noch nicht wusste.mkv 43 is the correct id for a share. The path is wrong. 46 is a wrong id for the same share. It should not be listed. The path however is correct. `QStorageInfo` is not enough, I'm afraid. For proper matching mtab must be read. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D1 To: michaelh, #baloo, #frameworks Cc: bruns, ashaposhnikov, michaelh, astippich, spoorun
D12222: baloodb: Use complete access filtering for all outputs
michaelh updated this revision to Diff 32422. michaelh added a comment. - Apply suggested changes - Correct summary error - Repair cleaning - Comment some decisions REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1?vs=32321=32422 BRANCH sanitize-dry (branched from master) REVISION DETAIL https://phabricator.kde.org/D1 AFFECTED FILES src/engine/experimental/databasesanitizer.cpp src/engine/experimental/databasesanitizer.h src/tools/experimental/baloodb/main.cpp To: michaelh, #baloo, #frameworks Cc: bruns, ashaposhnikov, michaelh, astippich, spoorun
D12222: baloodb: Use complete access filtering for all outputs
bruns added inline comments. INLINE COMMENTS > databasesanitizer.cpp:179 > +tail = std::remove_if(infos.begin(), tail, > + [deviceId,] (const FileInfo& info) { > + summary.accessible += info.accessible; missing space > databasesanitizer.cpp:180 > + [deviceId,] (const FileInfo& info) { > + summary.accessible += info.accessible; > + return info.id == deviceId; bool is automatically cast to int, false -> 0, true -> 1. Although probably better to do it explicitly: `summary.accessible += info.accessible ? 1 : 0;` REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D1 To: michaelh, #baloo, #frameworks Cc: bruns, ashaposhnikov, michaelh, astippich, spoorun, ngraham
D12222: baloodb: Use complete access filtering for all outputs
michaelh added inline comments. INLINE COMMENTS > databasesanitizer.cpp:180 > + [deviceId,] (const FileInfo& info) { > + summary.accessible += info.accessible; > + return info.id == deviceId; How can this work? `quint64 + bool` REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D1 To: michaelh, #baloo, #frameworks Cc: bruns, ashaposhnikov, michaelh, astippich, spoorun, ngraham
D12222: baloodb: Use complete access filtering for all outputs
michaelh updated this revision to Diff 32321. michaelh added a comment. - Use remove_if REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1?vs=32176=32321 BRANCH sanitize-dry (branched from master) REVISION DETAIL https://phabricator.kde.org/D1 AFFECTED FILES src/engine/experimental/databasesanitizer.cpp src/tools/experimental/baloodb/main.cpp To: michaelh, #baloo, #frameworks Cc: bruns, ashaposhnikov, michaelh, astippich, spoorun, ngraham
D12222: baloodb: Use complete access filtering for all outputs
michaelh added inline comments. INLINE COMMENTS > bruns wrote in databasesanitizer.cpp:252 > If ignoredDevices is a Set/List, you can do a filter pass over the fileList > first. > > auto& fileList = listResult.first; > auto tail = fileList.end(); > for (auto deviceId : ignoredDevices) { > tail = std::remove_if(fileList.begin(), tail, > [deviceId] (const FileInfo& info) { > return info.id == deviceId; > }); > } > summary.ignored += fileList.end() - tail; > std::erase(tail, fileList.end()); Cool! REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D1 To: michaelh, #baloo, #frameworks Cc: bruns, ashaposhnikov, michaelh, astippich, spoorun, ngraham
D12222: baloodb: Use complete access filtering for all outputs
bruns added inline comments. INLINE COMMENTS > databasesanitizer.cpp:252 > +auto& summary = listResult.second; > for (const auto& info: listResult.first) { > +if (ignoredDevices[info.id] == false) { If ignoredDevices is a Set/List, you can do a filter pass over the fileList first. auto& fileList = listResult.first; auto tail = fileList.end(); for (auto deviceId : ignoredDevices) { tail = std::remove_if(fileList.begin(), tail, [deviceId] (const FileInfo& info) { return info.id == deviceId; }); } summary.ignored += fileList.end() - tail; std::erase(tail, fileList.end()); REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D1 To: michaelh, #baloo, #frameworks Cc: bruns, ashaposhnikov, michaelh, astippich, spoorun, ngraham
D12222: baloodb: Use complete access filtering for all outputs
michaelh added a dependent revision: D12044: baloodb: Improve interface. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D1 To: michaelh, #baloo, #frameworks Cc: ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin
D12222: baloodb: Use complete access filtering for all outputs
michaelh added inline comments. INLINE COMMENTS > main.cpp:161 > return i18n("\n\nCommands:\n%1", allCommandsStr); > } > Ideas for better formatting this are welcome. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D1 To: michaelh, #baloo, #frameworks Cc: ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin
D12222: baloodb: Use complete access filtering for all outputs
michaelh added a task: T8250: Sanitize the database. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D1 To: michaelh, #baloo, #frameworks Cc: ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin
D12222: baloodb: Use complete access filtering for all outputs
michaelh edited the summary of this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D1 To: michaelh, #baloo, #frameworks Cc: ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin
D12222: baloodb: Use complete access filtering for all outputs
michaelh added a dependency: D11753: baloodb: Add clean command. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D1 To: michaelh, #baloo, #frameworks Cc: ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin
D12222: baloodb: Use complete access filtering for all outputs
michaelh created this revision. michaelh added reviewers: Baloo, Frameworks. Restricted Application added projects: Frameworks, Baloo. michaelh requested review of this revision. REVISION SUMMARY Add `mounted-only` option to 'list' and 'devices' command Use a common function to determine if a device should be ignored REPOSITORY R293 Baloo BRANCH sanitize-dry (branched from master) REVISION DETAIL https://phabricator.kde.org/D1 AFFECTED FILES src/engine/experimental/databasesanitizer.cpp src/tools/experimental/baloodb/main.cpp To: michaelh, #baloo, #frameworks Cc: ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, alexeymin