Merge lp:~stolowski/unity-scope-click/ensure-one-reader into lp:unity-scope-click/devel

[Paweł Stołowski]

Ensure click apps scope opens an existing database only and doesn't attempt to initialize it.

Ensure click apps scope opens an existing database only and doesn't attempt to initialize it - this is not needed since we provide an initialized and prepopulated database. Also make sure the reader process in the database stress test just reads from the db.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:347
http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-ci/223/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-amd64-ci/198
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-armhf-ci/197
        deb: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-armhf-ci/197/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-i386-ci/197

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-team-unity-scope-click-devel-ci/223/rebuild

review: Approve (continuous-integration)

[dobey (dobey)]

review: Needs Fixing

[lp:~stolowski/unity-scope-click/ensure-one-reader]

348. By Paweł Stołowski on 2014-07-21

Fixed error message.

[dobey (dobey)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:348
http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-ci/224/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-amd64-ci/199
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-armhf-ci/198
        deb: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-armhf-ci/198/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-i386-ci/198

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-team-unity-scope-click-devel-ci/224/rebuild

review: Approve (continuous-integration)

[Alejandro J. Cura (alecu)]

Tested on device, looks good.

review: Approve

=== modified file 'libclickscope/click/departments-db.cpp'
--- libclickscope/click/departments-db.cpp	2014-07-17 10:49:54 +0000
+++ libclickscope/click/departments-db.cpp	2014-07-21 13:53:45 +0000
@@ -39,21 +39,40 @@
 namespace click
 {
 
-std::unique_ptr<click::DepartmentsDb> DepartmentsDb::create_db()
+std::unique_ptr<click::DepartmentsDb> DepartmentsDb::open(bool create)
 {
     auto const path = QStandardPaths::writableLocation(QStandardPaths::CacheLocation);
     if (!path.isEmpty())
     {
         QDir("/").mkpath(path);
         const std::string dbpath = path.toStdString() + "/click-departments.db";
-        return std::unique_ptr<DepartmentsDb>(new DepartmentsDb(dbpath));
+        return std::unique_ptr<DepartmentsDb>(new DepartmentsDb(dbpath, create));
     }
     throw std::runtime_error("Cannot determine cache directory");
 }
 
-DepartmentsDb::DepartmentsDb(const std::string& name)
+DepartmentsDb::DepartmentsDb(const std::string& name, bool create)
 {
-    init_db(name);
+    db_ = QSqlDatabase::addDatabase("QSQLITE");
+    db_.setDatabaseName(QString::fromStdString(name));
+    if (!db_.open())
+    {
+        throw std::runtime_error("Cannot open departments database");
+    }
+
+    if (create)
+    {
+        init_db();
+    }
+    else
+    {
+        QSqlQuery query;
+        // check for existence of meta table to see if we're dealing with uninitialized database
+        if (!query.exec("SELECT 1 FROM meta"))
+        {
+            throw std::runtime_error("Invalid departments database");
+        }
+    }
 
     delete_pkgmap_query_.reset(new QSqlQuery(db_));
     delete_depts_query_.reset(new QSqlQuery(db_));
@@ -84,15 +103,8 @@
 {
 }
 
-void DepartmentsDb::init_db(const std::string& name)
+void DepartmentsDb::init_db()
 {
-    db_ = QSqlDatabase::addDatabase("QSQLITE");
-    db_.setDatabaseName(QString::fromStdString(name));
-    if (!db_.open())
-    {
-        throw std::runtime_error("Cannot open departments database");
-    }
-
     QSqlQuery query;
 
     // FIXME: for some reason enabling foreign keys gives errors about number of arguments of prepared queries when doing query.exec(); do not enable

=== modified file 'libclickscope/click/departments-db.h'
--- libclickscope/click/departments-db.h	2014-07-17 06:28:31 +0000
+++ libclickscope/click/departments-db.h	2014-07-21 13:53:45 +0000
@@ -59,7 +59,7 @@
         }
     };
 
-    DepartmentsDb(const std::string& name);
+    DepartmentsDb(const std::string& name, bool create = true);
     DepartmentsDb(const DepartmentsDb& other) = delete;
     DepartmentsDb& operator=(const DepartmentsDb&) = delete;
     virtual ~DepartmentsDb();
@@ -80,10 +80,10 @@
 
     void store_departments(const click::DepartmentList& depts, const std::string& locale);
 
-    static std::unique_ptr<DepartmentsDb> create_db();
+    static std::unique_ptr<DepartmentsDb> open(bool create = true);
 
 protected:
-    void init_db(const std::string& name);
+    void init_db();
     void store_departments_(const click::DepartmentList& depts, const std::string& locale);
     static void report_db_error(const QSqlError& error, const std::string& message);
 

=== modified file 'libclickscope/tests/test_departments-db.cpp'
--- libclickscope/tests/test_departments-db.cpp	2014-07-17 10:49:54 +0000
+++ libclickscope/tests/test_departments-db.cpp	2014-07-21 13:53:45 +0000
@@ -39,8 +39,8 @@
 class DepartmentsDbCheck : public DepartmentsDb
 {
 public:
-    DepartmentsDbCheck(const std::string& name)
-        : DepartmentsDb(name)
+    DepartmentsDbCheck(const std::string& name, bool create)
+        : DepartmentsDb(name, create)
     {
     }
 
@@ -63,11 +63,11 @@
 class DepartmentsDbTest: public ::testing::Test
 {
 public:
-    const std::string db_path = TEST_DIR "/departments-db-test.sqlite";
+    const std::string db_path = ":memory:";
 
     void SetUp() override
     {
-        db.reset(new DepartmentsDbCheck(db_path));
+        db.reset(new DepartmentsDbCheck(db_path, true));
         db->store_department_name("tools", "", "Tools");
         db->store_department_name("office", "", "Office");
 
@@ -90,11 +90,6 @@
         db->store_package_mapping("game2", "fps");
     }
 
-    void TearDown() override
-    {
-        unlink(db_path.c_str());
-    }
-
 protected:
     std::unique_ptr<DepartmentsDbCheck> db;
 };
@@ -329,7 +324,7 @@
 {
     // populate the db initially to make sure reader doesn't fail if it's faster than writer
     {
-        DepartmentsDb db(db_path);
+        DepartmentsDb db(db_path, true);
         populate_departments(&db, 1);
     }
 
@@ -342,7 +337,7 @@
     {
         SCOPED_TRACE("writer");
         std::unique_ptr<DepartmentsDb> db;
-        ASSERT_NO_THROW(db.reset(new DepartmentsDb(db_path)));
+        ASSERT_NO_THROW(db.reset(new DepartmentsDb(db_path, true)));
 
         populate_departments(db.get(), NUM_OF_WRITE_OPS);
 
@@ -359,7 +354,7 @@
         {
             SCOPED_TRACE("reader");
             std::unique_ptr<DepartmentsDb> db;
-            ASSERT_NO_THROW(db.reset(new DepartmentsDb(db_path)));
+            ASSERT_NO_THROW(db.reset(new DepartmentsDb(db_path, false)));
 
             for (int i = 0; i < NUM_OF_READ_OPS; i++)
             {
@@ -382,3 +377,10 @@
     }
 }
 
+TEST(DepartmentsDb, testOpenFailsOnUnitializedDb)
+{
+    ASSERT_THROW(
+            {
+                DepartmentsDb db(":memory:", false);
+            }, std::runtime_error);
+}

=== modified file 'scope/clickapps/apps-scope.cpp'
--- scope/clickapps/apps-scope.cpp	2014-07-15 11:56:05 +0000
+++ scope/clickapps/apps-scope.cpp	2014-07-21 13:53:45 +0000
@@ -52,11 +52,11 @@
     index.reset(new click::Index(client));
     try
     {
-        depts_db = click::DepartmentsDb::create_db();
+        depts_db = click::DepartmentsDb::open(false);
     }
     catch (const std::runtime_error& e)
     {
-        std::cerr << "Failed to get cache directory" << std::endl;
+        std::cerr << "Failed to open departments db: " << e.what() << std::endl;
     }
 }
 

=== modified file 'scope/clickstore/store-scope.cpp'
--- scope/clickstore/store-scope.cpp	2014-07-16 21:49:11 +0000
+++ scope/clickstore/store-scope.cpp	2014-07-21 13:53:45 +0000
@@ -58,11 +58,11 @@
 
     try
     {
-        depts_db = click::DepartmentsDb::create_db();
+        depts_db = click::DepartmentsDb::open(true);
     }
     catch (const std::runtime_error& e)
     {
-        std::cerr << "Failed to get cache directory" << std::endl;
+        std::cerr << "Failed to open departments db: " << e.what() << std::endl;
     }
 }
 

=== modified file 'scope/tests/test_apps_query.cpp'
--- scope/tests/test_apps_query.cpp	2014-07-17 20:05:57 +0000
+++ scope/tests/test_apps_query.cpp	2014-07-21 13:53:45 +0000
@@ -182,7 +182,7 @@
 {
     auto clickif = std::make_shared<MockClickInterface>();
     auto ptrCat = std::make_shared<FakeCategory>("id", "", "", renderer);
-    auto depts_db = std::make_shared<MockDepartmentsDb>(":memory:");
+    auto depts_db = std::make_shared<MockDepartmentsDb>(":memory:", true);
 
     // query for root of the departments tree
     {
@@ -220,7 +220,7 @@
 {
     auto clickif = std::make_shared<MockClickInterface>();
     auto ptrCat = std::make_shared<FakeCategory>("id", "", "", renderer);
-    auto depts_db = std::make_shared<MockDepartmentsDb>(":memory:");
+    auto depts_db = std::make_shared<MockDepartmentsDb>(":memory:", true);
 
     // query for a leaf department
     {

=== modified file 'scope/tests/test_helpers.h'
--- scope/tests/test_helpers.h	2014-07-16 11:53:51 +0000
+++ scope/tests/test_helpers.h	2014-07-21 13:53:45 +0000
@@ -56,8 +56,8 @@
 class MockDepartmentsDb : public click::DepartmentsDb
 {
 public:
-    MockDepartmentsDb(const std::string& name)
-        : click::DepartmentsDb(name)
+    MockDepartmentsDb(const std::string& name, bool create)
+        : click::DepartmentsDb(name, create)
     {
     }
 

=== modified file 'scope/tests/test_query.cpp'
--- scope/tests/test_query.cpp	2014-07-17 23:15:30 +0000
+++ scope/tests/test_query.cpp	2014-07-21 13:53:45 +0000
@@ -297,7 +297,7 @@
             std::make_shared<click::Department>("1-2", "Department three", "http://three.com", false)
             });
     DepartmentList init_departments({dept1});
-    auto depts_db = std::make_shared<MockDepartmentsDb>(":memory:");
+    auto depts_db = std::make_shared<MockDepartmentsDb>(":memory:", true);
 
     EXPECT_CALL(*depts_db, store_department_name(_, _, _)).Times(3);
     EXPECT_CALL(*depts_db, store_department_mapping(_, _)).Times(2);