From 0a92ef7655f6c897ada15c75ad0b2470bdd40ce1 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 20 Aug 2019 04:15:35 -0400 Subject: [PATCH] [packagechooser] Refactor fromApp*() - These don't have to be static methods of PackageItem, a free function is more convenient. - Since it's not API of PackageItem anymore, need to - update tests not to use API - do API-not-available warnings in consumers --- src/modules/packagechooser/ItemAppData.cpp | 2 +- src/modules/packagechooser/ItemAppData.h | 37 ++++++++++++++++++ src/modules/packagechooser/ItemAppStream.cpp | 4 +- src/modules/packagechooser/ItemAppStream.h | 38 +++++++++++++++++++ .../packagechooser/PackageChooserViewStep.cpp | 18 ++++++++- src/modules/packagechooser/PackageModel.cpp | 20 ---------- src/modules/packagechooser/PackageModel.h | 27 ------------- src/modules/packagechooser/Tests.cpp | 12 ++++-- 8 files changed, 102 insertions(+), 56 deletions(-) create mode 100644 src/modules/packagechooser/ItemAppData.h create mode 100644 src/modules/packagechooser/ItemAppStream.h diff --git a/src/modules/packagechooser/ItemAppData.cpp b/src/modules/packagechooser/ItemAppData.cpp index 7c80a1c3d..ed0ba9223 100644 --- a/src/modules/packagechooser/ItemAppData.cpp +++ b/src/modules/packagechooser/ItemAppData.cpp @@ -186,7 +186,7 @@ getNameAndSummary( const QDomNode& n ) } PackageItem -PackageItem::fromAppData( const QVariantMap& item_map ) +fromAppData( const QVariantMap& item_map ) { QString fileName = CalamaresUtils::getString( item_map, "appdata" ); if ( fileName.isEmpty() ) diff --git a/src/modules/packagechooser/ItemAppData.h b/src/modules/packagechooser/ItemAppData.h new file mode 100644 index 000000000..72617ff0c --- /dev/null +++ b/src/modules/packagechooser/ItemAppData.h @@ -0,0 +1,37 @@ +/* === This file is part of Calamares - === + * + * Copyright 2019, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#ifndef ITEMAPPDATA_H +#define ITEMAPPDATA_H + +#include "PackageModel.h" + +/** @brief Loads an AppData XML file and returns a PackageItem + * + * The @p map must have a key *appdata*. That is used as the + * primary source of information, but keys *id* and *screenshotPath* + * may be used to override parts of the AppData -- so that the + * ID is under the control of Calamares, and the screenshot can be + * forced to a local path available on the installation medium. + * + * Requires XML support in libcalamares, if not present will + * return invalid PackageItems. + */ +PackageItem fromAppData( const QVariantMap& map ); + +#endif diff --git a/src/modules/packagechooser/ItemAppStream.cpp b/src/modules/packagechooser/ItemAppStream.cpp index 392797846..3ee49f7d0 100644 --- a/src/modules/packagechooser/ItemAppStream.cpp +++ b/src/modules/packagechooser/ItemAppStream.cpp @@ -71,7 +71,7 @@ fromComponent( AppStream::Component& component ) map.insert( "id", component.id() ); map.insert( "name", component.name() ); map.insert( "description", component.description() ); - map.insert( "package", component.packageNames().join(",") ); + map.insert( "package", component.packageNames().join( "," ) ); auto screenshots = component.screenshots(); if ( screenshots.count() > 0 ) @@ -96,7 +96,7 @@ fromComponent( AppStream::Component& component ) } PackageItem -PackageItem::fromAppStream( const QVariantMap& item_map ) +fromAppStream( const QVariantMap& item_map ) { QString appstreamId = CalamaresUtils::getString( item_map, "appstream" ); if ( appstreamId.isEmpty() ) diff --git a/src/modules/packagechooser/ItemAppStream.h b/src/modules/packagechooser/ItemAppStream.h new file mode 100644 index 000000000..7d820400f --- /dev/null +++ b/src/modules/packagechooser/ItemAppStream.h @@ -0,0 +1,38 @@ +/* === This file is part of Calamares - === + * + * Copyright 2019, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#ifndef ITEMAPPSTREAM_H +#define ITEMAPPSTREAM_H + +#include "PackageModel.h" + +/** @brief Loads an item from AppStream data. + * + * The @p map must have a key *appstream*. That is used as the + * primary source of information from the AppStream cache, but + * keys *id* and *screenshotPath* may be used to override parts + * of the AppStream data -- so that the ID is under the control + * of Calamares, and the screenshot can be forced to a local path + * available on the installation medium. + * + * Requires AppStreamQt, if not present will return invalid + * PackageItems. + */ +PackageItem fromAppStream( const QVariantMap& map ); + +#endif diff --git a/src/modules/packagechooser/PackageChooserViewStep.cpp b/src/modules/packagechooser/PackageChooserViewStep.cpp index 412658394..bf45c6726 100644 --- a/src/modules/packagechooser/PackageChooserViewStep.cpp +++ b/src/modules/packagechooser/PackageChooserViewStep.cpp @@ -18,6 +18,12 @@ #include "PackageChooserViewStep.h" +#ifdef HAVE_XML +#include "ItemAppData.h" +#endif +#ifdef HAVE_APPSTREAM +#include "ItemAppStream.h" +#endif #include "PackageChooserPage.h" #include "PackageModel.h" @@ -217,11 +223,19 @@ PackageChooserViewStep::fillModel( const QVariantList& items ) if ( item_map.contains( "appdata" ) ) { - m_model->addPackage( PackageItem::fromAppData( item_map ) ); +#ifdef HAVE_XML + m_model->addPackage( fromAppData( item_map ) ); +#else + cWarning() << "Loading AppData XML is not supported."; +#endif } else if ( item_map.contains( "appstream" ) ) { - m_model->addPackage( PackageItem::fromAppStream( item_map ) ); +#ifdef HAVE_APPSTREAM + m_model->addPackage( fromAppStream( item_map ) ); +#else + cWarning() << "Loading AppStream data is not supported."; +#endif } else { diff --git a/src/modules/packagechooser/PackageModel.cpp b/src/modules/packagechooser/PackageModel.cpp index bc80109f3..12995fad5 100644 --- a/src/modules/packagechooser/PackageModel.cpp +++ b/src/modules/packagechooser/PackageModel.cpp @@ -88,26 +88,6 @@ PackageItem::PackageItem::PackageItem( const QVariantMap& item_map ) } } - -#ifndef HAVE_XML -PackageItem -PackageItem::fromAppData( const QVariantMap& item_map ) -{ - cWarning() << "Loading AppData XML is not supported."; - return PackageItem(); -} -#endif - -#ifndef HAVE_APPSTREAM -PackageItem -PackageItem::fromAppStream( const QVariantMap& item_map ) -{ - cWarning() << "Loading AppStream data is not supported."; - return PackageItem(); -} -#endif - - PackageListModel::PackageListModel( QObject* parent ) : QAbstractListModel( parent ) { diff --git a/src/modules/packagechooser/PackageModel.h b/src/modules/packagechooser/PackageModel.h index 49b4bc0dd..8362bee33 100644 --- a/src/modules/packagechooser/PackageModel.h +++ b/src/modules/packagechooser/PackageModel.h @@ -80,33 +80,6 @@ struct PackageItem * A valid item has an untranslated name available. */ bool isValid() const { return !name.isEmpty(); } - - /** @brief Loads an AppData XML file and returns a PackageItem - * - * The @p map must have a key *appdata*. That is used as the - * primary source of information, but keys *id* and *screenshotPath* - * may be used to override parts of the AppData -- so that the - * ID is under the control of Calamares, and the screenshot can be - * forced to a local path available on the installation medium. - * - * Requires XML support in libcalamares, if not present will - * return invalid PackageItems. - */ - static PackageItem fromAppData( const QVariantMap& map ); - - /** @brief Loads an item from AppStream data. - * - * The @p map must have a key *appstream*. That is used as the - * primary source of information from the AppStream cache, but - * keys *id* and *screenshotPath* may be used to override parts - * of the AppStream data -- so that the ID is under the control - * of Calamares, and the screenshot can be forced to a local path - * available on the installation medium. - * - * Requires AppStreamQt, if not present will return invalid - * PackageItems. - */ - static PackageItem fromAppStream( const QVariantMap& map ); }; using PackageList = QVector< PackageItem >; diff --git a/src/modules/packagechooser/Tests.cpp b/src/modules/packagechooser/Tests.cpp index 537ecbd3c..639d06d65 100644 --- a/src/modules/packagechooser/Tests.cpp +++ b/src/modules/packagechooser/Tests.cpp @@ -18,6 +18,12 @@ #include "Tests.h" +#ifdef HAVE_XML +#include "ItemAppData.h" +#endif +#ifdef HAVE_APPSTREAM +#include "ItemAppStream.h" +#endif #include "PackageModel.h" #include "utils/Logger.h" @@ -62,8 +68,8 @@ PackageChooserTests::testAppData() QVariantMap m; m.insert( "appdata", appdataName ); - PackageItem p1 = PackageItem::fromAppData( m ); #ifdef HAVE_XML + PackageItem p1 = fromAppData( m ); QVERIFY( p1.isValid() ); QCOMPARE( p1.id, "io.calamares.calamares.desktop" ); QCOMPARE( p1.name.get(), "Calamares" ); @@ -76,12 +82,10 @@ PackageChooserTests::testAppData() m.insert( "id", "calamares" ); m.insert( "screenshot", ":/images/calamares.png" ); - PackageItem p2 = PackageItem::fromAppData( m ); + PackageItem p2 = fromAppData( m ); QVERIFY( p2.isValid() ); QCOMPARE( p2.id, "calamares" ); QCOMPARE( p2.description.get( QLocale( "nl" ) ), "Calamares is een installatieprogramma voor Linux distributies." ); QVERIFY( !p2.screenshot.isNull() ); -#else - QVERIFY( !p1.isValid() ); #endif }