Merge lp:~ted/pay-service/verify-after-purchase into lp:pay-service/14.10

[Ted Gould]

Verify purchase status after running Purchase UI

This makes it so that we always verify the status after each time we run the UI so that we don't have to trust its return code. It also fixes some bugs found with multiple verify calls.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:25
http://jenkins.qa.ubuntu.com/job/pay-service-ci/32/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/pay-service-utopic-amd64-ci/33
    SUCCESS: http://jenkins.qa.ubuntu.com/job/pay-service-utopic-armhf-ci/33
        deb: http://jenkins.qa.ubuntu.com/job/pay-service-utopic-armhf-ci/33/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/pay-service-ci/32/rebuild

review: Approve (continuous-integration)

[dobey (dobey)]

Solves the hang I was seeing in my testing, and does result in verification when the payui exits regardless of exit code.

review: Approve

=== modified file 'libpay/pay-package.h'
--- libpay/pay-package.h	2014-06-14 02:56:52 +0000
+++ libpay/pay-package.h	2014-07-14 18:39:31 +0000
@@ -33,7 +33,8 @@
  * The states that an purchased item can be in.
  */
 typedef enum
-{ /*< prefix=PAY_PACKAGE_ITEM_STATUS */
+{
+    /*< prefix=PAY_PACKAGE_ITEM_STATUS */
     PAY_PACKAGE_ITEM_STATUS_UNKNOWN,       /*< nick=unknown */
     PAY_PACKAGE_ITEM_STATUS_VERIFYING,     /*< nick=verifying */
     PAY_PACKAGE_ITEM_STATUS_PURCHASED,     /*< nick=purchased */
 
=== modified file 'service/item-memory.cpp'
--- service/item-memory.cpp	2014-05-30 12:50:42 +0000
+++ service/item-memory.cpp	2014-07-14 18:39:31 +0000
@@ -60,16 +60,16 @@
 
     bool verify (void)
     {
-        if (vitem != nullptr)
-        {
-            return true;
-        }
         if (!vfactory->running())
         {
             return false;
         }
 
-        vitem = vfactory->verifyItem(app, id);
+        if (vitem == nullptr)
+        {
+            vitem = vfactory->verifyItem(app, id);
+        }
+
         if (vitem == nullptr)
         {
             /* Uhg, failed */
@@ -121,17 +121,11 @@
 
             pitem->purchaseComplete.connect([this](Purchase::Item::Status status)
             {
-                switch (status)
+                /* Verifying on each time the purchase UI runs right now because
+                   we're not getting reliable status back from them. */
+                if (!verify())
                 {
-                    case Purchase::Item::PURCHASED:
-                        setStatus(Item::Status::PURCHASED);
-                        break;
-                    case Purchase::Item::ERROR:
-                    case Purchase::Item::NOT_PURCHASED:
-                    default: /* Fall through, an error is same as status we don't know */
-                        /* We know we were not purchased before, so let's stay that way */
-                        setStatus(Item::Status::NOT_PURCHASED);
-                        break;
+                    setStatus(Item::Status::NOT_PURCHASED);
                 }
                 return;
             });
 
=== modified file 'service/verification-curl.cpp'
--- service/verification-curl.cpp	2014-07-10 21:11:03 +0000
+++ service/verification-curl.cpp	2014-07-14 18:39:31 +0000
@@ -87,25 +87,32 @@
 
     ~CurlItem (void)
     {
+        stopThread();
+
+        curl_easy_cleanup(handle);
+
+        if (curlHeaders != nullptr)
+        {
+            curl_slist_free_all (curlHeaders) ;
+            curlHeaders = nullptr;
+        }
+    }
+
+    void stopThread (void)
+    {
         stop = true;
 
         if (exec.joinable())
         {
             exec.join();
         }
-
-        curl_easy_cleanup(handle);
-
-        if (curlHeaders != nullptr)
-        {
-            curl_slist_free_all (curlHeaders) ;
-            curlHeaders = nullptr;
-        }
     }
 
     virtual bool run (void)
     {
+        stopThread();
         transferBuffer.clear();
+        stop = false;
 
         /* Do the execution in another thread so we can wait on the
            network socket. */
 
=== modified file 'tests/item-memory-tests.cpp'
--- tests/item-memory-tests.cpp	2014-05-08 21:42:53 +0000
+++ tests/item-memory-tests.cpp	2014-07-14 18:39:31 +0000
@@ -180,12 +180,24 @@
 	usleep(50 * 1000);
 	EXPECT_EQ(Item::Item::Status::NOT_PURCHASED, item->getStatus());
 
+	/* Assume the purchase UI is a liar */
+	std::string fitemname("falsely-purchased-item");
+	pfactory->test_setPurchase(appname, fitemname, true);
+	auto fitem = store->getItem(appname, fitemname);
+
+	ASSERT_TRUE(fitem->verify());
+	usleep(50 * 1000);
+	ASSERT_TRUE(fitem->purchase());
+	usleep(50 * 1000);
+
+	EXPECT_EQ(Item::Item::Status::NOT_PURCHASED, fitem->getStatus());
+
+	/* Legit purchase with verification */
 	std::string pitemname("purchased-item");
 	pfactory->test_setPurchase(appname, pitemname, true);
+	vfactory->test_setPurchase(appname, pitemname, true);
+
 	auto pitem = store->getItem(appname, pitemname);
-
-	ASSERT_TRUE(pitem->verify());
-	usleep(50 * 1000);
 	ASSERT_TRUE(pitem->purchase());
 	usleep(50 * 1000);
 
 
=== modified file 'tests/verification-curl-tests.cpp'
--- tests/verification-curl-tests.cpp	2014-06-12 20:55:13 +0000
+++ tests/verification-curl-tests.cpp	2014-07-14 18:39:31 +0000
@@ -67,6 +67,15 @@
 
 	EXPECT_EQ(Verification::Item::Status::NOT_PURCHASED, status);
 
+	/* Verify it can be run twice on the same item */
+	status = Verification::Item::Status::ERROR;
+
+	ASSERT_TRUE(item->run());
+	usleep(20 * 1000);
+
+	EXPECT_EQ(Verification::Item::Status::NOT_PURCHASED, status);
+
+	/* Bad App ID */
 	std::string badappid("bad");
 	auto baditem = verify->verifyItem(badappid, itemid);
 	ASSERT_NE(nullptr, item);
 
=== modified file 'tests/verification-test.h'
--- tests/verification-test.h	2014-05-05 18:51:08 +0000
+++ tests/verification-test.h	2014-07-14 18:39:31 +0000
@@ -38,6 +38,9 @@
 	}
 
 	virtual bool run (void) {
+		if (t.joinable())
+			t.join();
+
 		t = std::thread([this]() {
 			/* Fastest website in the world */
 			usleep(10 * 1000);