Differential D17302 Diff 48688 autotests/unit/file/resulttest.cpp

Changeset View

Standalone View

View Options

autotests/unit/file/resulttest.cpp

  • This file was added.
1​/*
2​ * Copyright (C) 2018 Alexander Stippich <a.stippich@gmx.net>
3​ *
brunsUnsubmitted
Done

^_^

bruns: ^_^
4​ * This library is free software; you can redistribute it and/or
5​ * modify it under the terms of the GNU Lesser General Public
6​ * License as published by the Free Software Foundation; either
7​ * version 2.1 of the License, or (at your option) any later version.
8​ *
9​ * This library is distributed in the hope that it will be useful,
10​ * but WITHOUT ANY WARRANTY; without even the implied warranty of
11​ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
12​ * Lesser General Public License for more details.
13​ *
14​ * You should have received a copy of the GNU Lesser General Public
15​ * License along with this library; if not, write to the Free Software
16​ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
17​ *
18​ */
19​
20​#include "document.h"
21​#include "extractor/result.h"
22​
23​#include <KFileMetaData/Properties>
24​
25​#include <QTest>
26​
27​using namespace Baloo;
28​
29​class ResultTest : public QObject
30​{
31​ Q_OBJECT
32​
33​private Q_SLOTS:
34​ void testPropertyData()
35​ {
36​ Result result(QStringLiteral("test"), QStringLiteral("audio/mpeg3"),
37​ KFileMetaData::ExtractionResult::ExtractEverything);
38​
39​ result.add(KFileMetaData::Property::Artist, QVariant(QStringLiteral("artist1")));
40​ result.add(KFileMetaData::Property::Artist, QVariant(QStringLiteral("artist2")));
brunsUnsubmitted
Done

Spaces and linebreaks ...

bruns: Spaces and linebreaks ...
41​
brunsUnsubmitted
Not Done

No need to reset the default constructed Baloo::Document with a default constructed one.

bruns: No need to reset the default constructed `Baloo::Document` with a default constructed one.
astippichAuthorUnsubmitted
Not Done

Actually it is. The TermGenerators are otherwise only initialized with nullptr, and consequently will crash. That is something that should be fixed (separately).

astippich: Actually it is. The TermGenerators are otherwise only initialized with nullptr, and…
brunsUnsubmitted
Not Done

D17315

bruns: D17315
42​ result.add(KFileMetaData::Property::Genre, QVariant(QStringList({"genre1", "genre2"})));
43​
44​ result.add(KFileMetaData::Property::Keywords, QVariant(QStringList({"keyword1", "keyword2"})));
45​ result.add(KFileMetaData::Property::Keywords, QVariant(QStringLiteral("keyword3")));
46​
47​ result.add(KFileMetaData::Property::Author, QVariant(QStringLiteral("author")));
48​ result.add(KFileMetaData::Property::ReleaseYear, QVariant(2018));
49​ result.add(KFileMetaData::Property::PhotoGpsAltitude, QVariant(20.5));
50​
51​ result.finish();
52​
53​ auto artist = result.m_map.find(QString::number(KFileMetaData::Property::Artist));
54​ QCOMPARE(artist->type(), QVariant::List);
55​ QCOMPARE(artist->toStringList().join(", "), QStringLiteral("artist1, artist2"));
56​
57​ auto genre = result.m_map.find(QString::number(KFileMetaData::Property::Genre));
58​ QCOMPARE(genre->type(), QVariant::StringList);
59​ QCOMPARE(genre->toStringList().join(", "), QStringLiteral("genre1, genre2"));
60​
61​ auto keywords = result.m_map.find(QString::number(KFileMetaData::Property::Keywords));
62​ QCOMPARE(keywords->type(), QVariant::List);
63​ QCOMPARE(keywords->toList().count(), 2);
brunsUnsubmitted
Not Done

This looks wrong to me ...
How many items do you get when you append "keyword3" first and ["keyword1", "keyword2"] next?

bruns: This looks wrong to me ... How many items do you get when you append "keyword3" first and…
astippichAuthorUnsubmitted
Not Done

It's the same. The properties will get merged regardless of their form if the QVariantMap already contains an item with the same key, see https://phabricator.kde.org/source/baloo/browse/master/src/file/extractor/result.cpp$52 so this works as currently intended.
Right now, we rely on this behavior because KFileMetaData (incorrectly) outputs multiple properties with the same key instead of a single property made of a QStringList. I wrote the test to prepare the switch to string lists at some point.
My favorite solution is to remove this merging, once KFileMetaData outputs string lists when necessary. This requires a custom function for the conversion of the output JSON to a PropertyMap, see https://phabricator.kde.org/source/baloo/browse/master/src/lib/file.cpp$118
QJsonDocument::toVariantMap currently does not handle duplicated keys. IIRC JSON with duplicated keys is strictly not compliant, but since it is only internal, this is okay IMHO.
The own conversion function will save us an extra for loop, and makes sure that the same PropertyMap is generated when either querying via Baloo (file.load()) or directly via KFileMetaData's SimpleExtractionResult.

astippich: It's the same. The properties will get merged regardless of their form if the QVariantMap…
64​
65​ auto author = result.m_map.find(QString::number(KFileMetaData::Property::Author));
66​ QCOMPARE(author->type(), QVariant::String);
67​
68​ auto year = result.m_map.find(QString::number(KFileMetaData::Property::ReleaseYear));
69​ QCOMPARE(year->type(), QVariant::Int);
70​
71​ auto altitude = result.m_map.find(QString::number(KFileMetaData::Property::PhotoGpsAltitude));
72​ QCOMPARE(altitude->type(), QVariant::Double);
73​
74​
75​ }
76​};
77​
78​QTEST_MAIN(ResultTest)
79​
80​#include "resulttest.moc"