Introduce sanitizer class
ClosedPublic
Actions

Authored by michaelh on Mar 13 2018, 2:15 PM.

Details

Reviewers

ngraham
mlaurent

Group Reviewers

Baloo
Frameworks

Maniphest Tasks

T8250: Sanitize the database

Commits

R293:9de78becc063: Introduce sanitizer class

Summary

Due to device ids being inconstant duplicates are introduced to the database. I. e. multiple document ids pointing to the same entity.
This class shall eventually sanitize the database. Currently it just displays the issues should there be any.

Diff Detail

Repository

R293 Baloo

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

michaelh created this revision.Mar 13 2018, 2:15 PM

Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptMar 13 2018, 2:15 PM

michaelh requested review of this revision.Mar 13 2018, 2:15 PM

michaelh added a dependent revision: D11287: Introduce baloodb CLI tool.Mar 13 2018, 2:23 PM

michaelh mentioned this in D11213: balooctl: Introduce list command.

Looks good to me!

This revision is now accepted and ready to land.Mar 14 2018, 7:03 PM

michaelh added inline comments.Mar 14 2018, 7:57 PM

src/engine/databasesanitizer.cpp
201	This is the best I could think of to make `getDocuments` accessible to DatabaseSanitizerImpl. But I don't like it. This method definitely should not be part of the interface.

michaelh mentioned this in D11287: Introduce baloodb CLI tool.Mar 14 2018, 8:36 PM

mlaurent requested changes to this revision.Mar 15 2018, 9:12 AM

mlaurent added a subscriber: mlaurent.

mlaurent added inline comments.

src/engine/databasesanitizer.cpp
45	initialize value by default please
108	includeIds.count() > 0 => use !isEmpty
154	const QSharedPointer<QRegularExpression> & ------------------------------------------------------^
166	use qCDebug(BALOO)
175	as you add a i18n in engine directory you need to extract them. you missed to add a Message.sh in this subdirectory and loading message file.

This revision now requires changes to proceed.Mar 15 2018, 9:12 AM

Is it possible to create an autotest for this class ?

Apply suggested changes
Put method descriptions in their correct place

There were some conflicts I had to solve with this. Because it is not completely clear to me where this is going I wanted to be safe and used a d-pointer.
As a consequence this class does the printing which I prefer to be in the cli.
Secondly I can't make DatabaseSanitizerImpl a friend of the Transaction class. Which leads to the getDocuments() in DatabaseSanitizer, which should not be part of the interface. In the future more of similar functions will be needed.
Is there a better way to accomplish this?

src/engine/databasesanitizer.cpp
166	I get a linker error `databasesanitizer.cpp:189: undefined reference to` BALOO()'` Currently its not worth the trouble fixing it, because that method is going to change anyway. Also I don't like to mix debug statement and printing to stderr.

In D11285#226298, @mlaurent wrote:

Is it possible to create an autotest for this class ?

This is where it gets complicated.
Essentially I can test only for sane databases, but that's useless.
Or I need to mock an external drive and have control over its device id, which seems to be very difficult if at all possible.

michaelh added inline comments.Mar 15 2018, 1:51 PM

src/engine/databasesanitizer.cpp
164	Before landing this will become Create a list of \a FileInfo items and print it to stdout.

mlaurent requested changes to this revision.Mar 16 2018, 1:54 PM

mlaurent added inline comments.

src/engine/databasesanitizer.cpp
47	you can replace this constructor by initialize directly variable > bool accessible = true; etc. > it avoids to create this constructor
100	const auto & for a quint32 it's better to use directly quint32 without const'ref

This revision now requires changes to proceed.Mar 16 2018, 1:54 PM

Turn FileInfo into a struct again

michaelh marked 3 inline comments as done.Mar 16 2018, 2:44 PM

+2 for me but I don't know baloo code :)

This revision is now accepted and ready to land.Mar 16 2018, 2:52 PM

Closed by commit R293:9de78becc063: Introduce sanitizer class (authored by michaelh). · Explain WhyMar 16 2018, 2:54 PM

This revision was automatically updated to reflect the committed changes.

michaelh mentioned this in D11038: balooctl: Add clean option to remove stale file index entries.Mar 18 2018, 8:37 AM

michaelh mentioned this in D10203: documenturldb: Temporarily remove Q_ASSERT, ignore empty filenames instead.Mar 19 2018, 6:59 AM

michaelh added a task: T8250: Sanitize the database.Apr 12 2018, 9:08 PM

Revision Contents
Changeset List

		Path
M		src/engine/CMakeLists.txt (2 lines)
A	M	src/engine/Messages.sh (2 lines)
A	M	src/engine/databasesanitizer.h (77 lines)
A	M	src/engine/databasesanitizer.cpp (217 lines)
M		src/engine/documenturldb.cpp (6 lines)
M		src/engine/transaction.h (1 line)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	29405	2bab68b		Mar 13 2018, 2:11 PM	★	★
Diff 2	29594	2bab68b	- Apply suggested changes	Mar 15 2018, 1:24 PM	★	★
Diff 3	29683	2bab68b	- Turn FileInfo into a struct again	Mar 16 2018, 2:43 PM	★	★
Diff 4	29684	5bacac6	R293:9de78becc06346a734279c2dc13d081dc5861624	Mar 16 2018, 2:56 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
c699fb5d1683	84566ff3deaf	9defc4e959ef	Michael Heidelbach	Turn FileInfo into a struct again	Mar 16 2018, 2:41 PM
9defc4e959ef	6c4924cc54b0	4326304a13c2	Michael Heidelbach	Put method descriptions in their correct place	Mar 15 2018, 1:26 PM
4326304a13c2	8b0e60bfd1a9	68557453f159	Michael Heidelbach	Apply more suggested changes	Mar 15 2018, 1:04 PM
68557453f159	8d82047651f6	d9eaf6118352	Michael Heidelbach	Apply suggested changes	Mar 15 2018, 12:58 PM
d9eaf6118352	d92d510ac051	c8ad2a6152f0	Michael Heidelbach	Clean up code	Mar 13 2018, 2:09 PM
c8ad2a6152f0	27f93c63c121	48c6697d4565	Michael Heidelbach	sanitize: Introduce printProgress()	Mar 13 2018, 1:51 PM
48c6697d4565	13937d328bee	5325655a118c	Michael Heidelbach	documenturldb: Remove Q_ASSERTs	Mar 13 2018, 1:50 PM
5325655a118c	726a2fe2ad26	58504b86e16b	Michael Heidelbach	Apply minor adjustments	Mar 13 2018, 1:13 PM
58504b86e16b	05c3333de992	2bab68bccf62	Michael Heidelbach	Introduce sanitizer class (Show More…)	Mar 13 2018, 12:30 PM

Status	Author	Revision
Accepted	michaelh	D12044 baloodb: Improve interface
Needs Review	michaelh	D12222 baloodb: Use complete access filtering for all outputs
Closed	michaelh	D11753 baloodb: Add clean command
Closed	michaelh	D11745 databasesanitizer: Use flags for filtering
Closed	bruns	D12165 Use QStorageInfo instead of a homegrown implementation
Closed	michaelh	D11452 sanitizer: Improve device listing
Closed	michaelh	D11287 Introduce baloodb CLI tool
Closed	michaelh	D11285 Introduce sanitizer class