baloodb: Use complete access filtering for all outputs
Needs ReviewPublic

Authored by michaelh on Apr 15 2018, 11:52 AM.

Details

Reviewers
None
Group Reviewers
Baloo
Frameworks
Maniphest Tasks
T8250: Sanitize the database
Summary

Add mounted-only option to 'list' and 'devices' command
Use a common function to determine if a device should be ignored

Depends on D11753

Diff Detail

Repository
R293 Baloo
Branch
sanitize-dry (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
michaelh created this revision.Apr 15 2018, 11:52 AM
Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptApr 15 2018, 11:52 AM
michaelh requested review of this revision.Apr 15 2018, 11:52 AM
michaelh edited the summary of this revision. (Show Details)
michaelh added inline comments.Apr 15 2018, 12:51 PM
src/tools/experimental/baloodb/main.cpp
160

Ideas for better formatting this are welcome.

bruns added a subscriber: bruns.Apr 16 2018, 5:06 PM
bruns added inline comments.
src/engine/experimental/databasesanitizer.cpp
304–305

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());
michaelh added inline comments.Apr 16 2018, 7:15 PM
src/engine/experimental/databasesanitizer.cpp
304–305

Cool!

michaelh updated this revision to Diff 32321.Apr 16 2018, 7:16 PM
  • Use remove_if
michaelh added inline comments.Apr 16 2018, 7:19 PM
src/engine/experimental/databasesanitizer.cpp
180

How can this work? quint64 + bool

bruns added inline comments.Apr 16 2018, 10:43 PM
src/engine/experimental/databasesanitizer.cpp
179

missing space

180

bool is automatically cast to int, false -> 0, true -> 1.
Although probably better to do it explicitly:
summary.accessible += info.accessible ? 1 : 0;

michaelh updated this revision to Diff 32422.EditedApr 17 2018, 7:39 PM
  • Apply suggested changes
  • Repair cleaning
  • Comment some decisions

I went I little overboard with remove_if and had to repair.
Device filtering is also broken see inline comment.

src/engine/experimental/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.

michaelh updated this revision to Diff 32426.Apr 17 2018, 8:47 PM
  • Filter by device id
michaelh marked an inline comment as done.Apr 17 2018, 8:51 PM
michaelh added inline comments.
src/engine/experimental/databasesanitizer.cpp
160

On the wrong track

222

~3 hrs for this, phew.

michaelh marked 6 inline comments as done.Apr 17 2018, 8:52 PM
bruns added inline comments.Apr 17 2018, 11:42 PM
src/engine/experimental/databasesanitizer.cpp
179

isMounted(id) can be implemented as getStorageInfo(id).isValid()

183

unused, please remove

215

I can not see what this function is useful for. Just inline the FileInfo -> DeviceIds reduction here, and ...

218

... skip the deviceId if it should be ignored:

const auto storageInfo = getStorageInfo(id);
if (isDeviceIgnored(storageInfo, accessFilter)) {
    continue;
}
221
if (info.deviceId != deviceId) {
  return false;
}
summary.accessible += info.accessible ? 1 : 0;
return true;
michaelh updated this revision to Diff 32578.Apr 19 2018, 5:14 PM
michaelh marked 5 inline comments as done.
  • Revert most previous changes
  • Use isMounted() and isObscured()
  • Remove documents recursively
michaelh updated this revision to Diff 32579.Apr 19 2018, 5:19 PM
  • Correct typo
michaelh updated this revision to Diff 32580.Apr 19 2018, 5:22 PM
  • That was no typo
michaelh marked an inline comment as done.Apr 19 2018, 5:35 PM

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.

@bruns this still applies cleanly to current master, do you think this and the dependent revision are still worth it to merge?

Restricted Application added a subscriber: kde-frameworks-devel. · View Herald TranscriptApr 13 2019, 11:09 AM