[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

@@ -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() )

src/modules/packagechooser/ItemAppData.h

@@ -0,0 +1,37 @@
+/* === This file is part of Calamares - <https://github.com/calamares> ===
+ *
+ *   Copyright 2019, Adriaan de Groot <groot@kde.org>
+ *
+ *   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 <http://www.gnu.org/licenses/>.
+ */
+
+#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

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() )

src/modules/packagechooser/ItemAppStream.h

@@ -0,0 +1,38 @@
+/* === This file is part of Calamares - <https://github.com/calamares> ===
+ *
+ *   Copyright 2019, Adriaan de Groot <groot@kde.org>
+ *
+ *   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 <http://www.gnu.org/licenses/>.
+ */
+
+#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

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
         {

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 )
 {

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 >;

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
 }