Check for string lists and multi-values in property map
ClosedPublic

Authored by astippich on Apr 13 2019, 8:51 AM.

Details

Summary

Currently, KFileMetaData stores multi-values (e.g. multiple arists)
as multiple entries in the property map, while Baloo combines them to List
before serialization. More information in D19087 (your opinion is also appreciated there!)
or T8196.
Consider both cases such that the metadata retrieved by the baloo indexer and the file
indexer is the same.

Diff Detail

Repository
R255 Elisa
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
astippich requested review of this revision.Apr 13 2019, 8:51 AM
astippich created this revision.

I will have a look thanks.
I will also have a look at your other suggestions.
I hope to be able to do it this week-end if time permits.

astippich updated this revision to Diff 56120.Apr 13 2019, 9:25 AM
  • use localized strings
mgallien requested changes to this revision.Apr 13 2019, 8:08 PM
mgallien added inline comments.
src/filescanner.cpp
119

There is a typo in the variable name

258

I am afraid that for most cases the concerned properties will be single valued and this code path will be slower than previous code.
Would int QMap::count(const Key &key) const makes sense to detect if going through QMap::values is required ?

This revision now requires changes to proceed.Apr 13 2019, 8:08 PM
astippich updated this revision to Diff 56172.Apr 14 2019, 9:10 AM
  • fix typo, check entry count

I am missing time to investigate deep enough in KFileMetaData and Baloo but if D20526 lands, why this diff would still be needed ?

It will become less relevant as there will be less lists inserted, but still possible. Vorbis comments explicitly allows to insert multiple tags so there is a chance that there will be multiple entries in the map extracted from KFileMetadata, or lists when obtained via Baloo.
Also note that all existing data in the baloo database is currently not updated when extractors change.

mgallien accepted this revision.Apr 22 2019, 8:56 PM

It will become less relevant as there will be less lists inserted, but still possible. Vorbis comments explicitly allows to insert multiple tags so there is a chance that there will be multiple entries in the map extracted from KFileMetadata, or lists when obtained via Baloo.

OK, thanks for the explanations.

Also note that all existing data in the baloo database is currently not updated when extractors change.

Yes I know that. I vaguely remember some work going into that direction (I was talking about D19109).
In the future, Elisa will need to make good use of that.

This revision is now accepted and ready to land.Apr 22 2019, 8:56 PM
This revision was automatically updated to reflect the committed changes.