Merge lp:~ken-vandine/content-hub/gsettings_type_check into lp:content-hub

[Ken VanDine]

Ensure m_defaultSources is a StringList before attempting to use
it as a list

Ensure m_defaultSources is a StringList before attempting to use
it as a list

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:82
http://jenkins.qa.ubuntu.com/job/content-hub-ci/138/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/content-hub-trusty-amd64-ci/41
    SUCCESS: http://jenkins.qa.ubuntu.com/job/content-hub-trusty-armhf-ci/41
        deb: http://jenkins.qa.ubuntu.com/job/content-hub-trusty-armhf-ci/41/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/content-hub-trusty-i386-ci/41

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/content-hub-ci/138/rebuild

review: Approve (continuous-integration)

[Ken VanDine (ken-vandine)]

Are there any related MPs required for this MP to build/function as expected? Please list.

 * No

Is your branch in sync with latest trunk (e.g. bzr pull lp:trunk -> no changes)

 * Yes

Did you perform an exploratory manual test run of your code change and any related functionality on device or emulator?

 * Yes

Did you successfully run all tests found in your component's Test Plan (https://wiki.ubuntu.com/Process/Merges/TestPlan/content-hub) on device or emulator?

 * Yes

If you changed the UI, was the change specified/approved by design?

 * No changes

If you changed the packaging (debian), did you subscribe a core-dev to this MP?

 * No changes

[Michael Sheldon (michael-sheldon)]

Did you perform an exploratory manual test run of the code change and any related functionality on device or emulator?

 * Yes

Did CI run pass? If not, please explain why.

 * Yes

Have you checked that submitter has accurately filled out the submitter checklist and has taken no shortcut?

 * Yes

review: Approve

=== modified file 'src/com/ubuntu/content/service/main.cpp'
--- src/com/ubuntu/content/service/main.cpp	2014-03-04 21:47:48 +0000
+++ src/com/ubuntu/content/service/main.cpp	2014-03-27 16:05:34 +0000
@@ -33,22 +33,6 @@
 namespace cuc = com::ubuntu::content;
 
 namespace {
-    void list(QSharedPointer<cucd::PeerRegistry> registry)
-    {
-        /* list known peers for pictures */
-        QStringList result;
-        registry->enumerate_known_sources_for_type(
-            cuc::Type::Known::pictures(),
-            [&result](const cuc::Peer& peer)
-            {
-                result.append(peer.id());
-            });
-
-        foreach (QString r, result) {
-            TRACE() << "PEER: " << r;
-        }
-    }
-
     void shutdown(int sig)
     {
         TRACE() << Q_FUNC_INFO << sig;
@@ -100,12 +84,8 @@
 
     if (ret == 1)
         app->exit(ret);
-    else
-    {
-        /* list known peers */
-        list(registry);
+    else        
         ret = app->exec();
-    }
 
     TRACE() << "Server exiting, cleaning up";
     delete server;

=== modified file 'src/com/ubuntu/content/service/registry.cpp'
--- src/com/ubuntu/content/service/registry.cpp	2014-03-18 15:23:16 +0000
+++ src/com/ubuntu/content/service/registry.cpp	2014-03-27 16:05:34 +0000
@@ -37,19 +37,35 @@
     {
         if (m_defaultSources->keys().contains(type.id()))
         {
-            QString peer_id = m_defaultSources->get(type.id()).toString();
-            QStringList as(m_defaultSources->get(type.id()).toStringList());
-            if (!as.isEmpty())
-            {
-                std::string pkg = as[0].toStdString();
-                std::string app = as[1].toStdString();
-                std::string ver = as[2].toStdString();
-                cuc::Peer peer;
-                if (app.empty() || ver.empty())
-                    peer = QString::fromStdString(pkg);
-                else
-                    peer = QString::fromLocal8Bit(upstart_app_launch_triplet_to_app_id(pkg.c_str(), app.c_str(), ver.c_str()));
-                install_source_for_type(type, cuc::Peer{peer.id(), true});
+            QVariant peer_v = m_defaultSources->get(type.id());
+            // If default isn't a StringList, attempt to reset
+            if (peer_v.type() != QVariant::StringList)
+            {
+                TRACE() << Q_FUNC_INFO << "Default type for" << type.id() << "is incorrect, resetting";
+                m_defaultSources->reset(type.id());
+                // After reset, lets get a new QVariant
+                peer_v = m_defaultSources->get(type.id());
+            }
+
+            /* Only assume the default is correct if the type is a StringList
+             * The reset above should have ensured it was reset, lets double
+             * check anyway to prevent crashes
+             */
+            if (peer_v.type() == QVariant::StringList)
+            {
+                QStringList as(peer_v.toStringList());
+                if (!as.isEmpty())
+                {
+                    std::string pkg = as[0].toStdString();
+                    std::string app = as[1].toStdString();
+                    std::string ver = as[2].toStdString();
+                    cuc::Peer peer;
+                    if (app.empty() || ver.empty())
+                        peer = QString::fromStdString(pkg);
+                    else
+                        peer = QString::fromLocal8Bit(upstart_app_launch_triplet_to_app_id(pkg.c_str(), app.c_str(), ver.c_str()));
+                    install_source_for_type(type, cuc::Peer{peer.id(), true});
+                }
             }
         }
     }
@@ -62,13 +78,15 @@
     TRACE() << Q_FUNC_INFO << type.id();
     if (m_defaultSources->keys().contains(type.id()))
     {
-        QStringList as(m_defaultSources->get(type.id()).toStringList());
+        QVariant peer_v = m_defaultSources->get(type.id());
+        if (peer_v.type() != QVariant::StringList)
+            return cuc::Peer(peer_v.toString());
+        QStringList as(peer_v.toStringList());
         if (!as.isEmpty())
         {
             std::string pkg = as[0].toStdString();
             std::string app = as[1].toStdString();
             std::string ver = as[2].toStdString();
-            cuc::Peer peer;
             if (app.empty() || ver.empty())
                 return cuc::Peer(QString::fromStdString(pkg));
             return cuc::Peer(QString::fromLocal8Bit(upstart_app_launch_triplet_to_app_id(pkg.c_str(), app.c_str(), ver.c_str())), true);
@@ -122,16 +140,20 @@
     {
         TRACE() << Q_FUNC_INFO << k;
         bool defaultPeer = false;
-        QStringList as(m_defaultSources->get(type.id()).toStringList());
-        if (!as.isEmpty())
-        {   
-            std::string pkg = as[0].toStdString();
-            std::string app = as[1].toStdString();
-            std::string ver = as[2].toStdString();
-            if (app.empty() || ver.empty())
-                defaultPeer = QString::fromStdString(pkg) == k;
-            else
-                defaultPeer = QString::fromLocal8Bit(upstart_app_launch_triplet_to_app_id(pkg.c_str(), app.c_str(), ver.c_str())) == k;
+        QVariant peer_v = m_defaultSources->get(type.id());
+        if (peer_v.type() == QVariant::StringList)
+        {
+            QStringList as(peer_v.toStringList());
+            if (!as.isEmpty())
+            {
+                std::string pkg = as[0].toStdString();
+                std::string app = as[1].toStdString();
+                std::string ver = as[2].toStdString();
+                if (app.empty() || ver.empty())
+                    defaultPeer = QString::fromStdString(pkg) == k;
+                else
+                    defaultPeer = QString::fromLocal8Bit(upstart_app_launch_triplet_to_app_id(pkg.c_str(), app.c_str(), ver.c_str())) == k;
+            }
         }
         for_each(cuc::Peer{k, defaultPeer});
     }