Merge lp:~mvo/software-center/dbus-idle-timeout into lp:software-center

[Michael Vogt]

This branch addresses bug #1058567 to ensure that the data provider
exits after a certain amount of time.

[Gary Lasker (gary-lasker)]

Hi Michael, thanks for your branch! Strangely, I'm getting a bunch of errors when I run the associated unit test, test_dataprovider.py, as shown at the following pastebin:

  http://paste.ubuntu.com/1255206/

I'm not seeing these errors in current trunk. Do you have an idea about why I might be seeing these?

Many thanks!

[lp:~mvo/software-center/dbus-idle-timeout]

3212. By Michael Vogt on 2012-10-02

move implementation of the dbus methods into a seperate function so that we can add decorators - the dbus.service.method decorator seems to not like additional decroators

[Michael Vogt (mvo)]

Thanks, indeed, it looks like the dbus decorator does not like other decorators at the same time. I fixed this now and the tests work now on my box.

[Michael Vogt (mvo)]

Anything holding this one back?

[Gary Lasker (gary-lasker)]

Hi Michael, I'm sorry for the delay on this one! Ok, I just tried it and I'm still getting an error in the test. It seems that SoftwareCenterDataProvider's __init__ doesn't like the Mock self.IDLE_CHECK_INTERVAL being passed into the GObject.timeout_add_seconds() that happens in test_idle_timeout:

======================================================================
ERROR: test_idle_timeout (__main__.IdleTimeoutTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/test_dataprovider.py", line 195, in test_idle_timeout
    self.bus_name, main_loop=self.loop)
  File "/home/tremolux/Projects/quantal/software-center_review_mvo_dbus_idle_timeout/dbus-idle-timeout/softwarecenter/db/dataprovider.py", line 89, in __init__
    self.IDLE_CHECK_INTERVAL, self._check_inactivity)
TypeError: an integer is required

----------------------------------------------------------------------
Ran 15 tests in 8.076s

FAILED (errors=1)

[Gary Lasker (gary-lasker)]

Note that the above is when testing on Precise. I'll try this on a Quantal machine and see if I still get the error.

[Gary Lasker (gary-lasker)]

Meanwhile I will approve this as the only issue is the failing test case. Michael, please feel free to merge this branch when the test is ready.

Thanks!

review: Approve

[Michael Vogt (mvo)]

Thanks for the review, I just double checked, it fails on precise for me as well but works on quantal.

=== modified file 'software-center-dbus'
--- software-center-dbus	2012-07-18 12:45:55 +0000
+++ software-center-dbus	2012-10-02 07:44:21 +0000
@@ -18,9 +18,11 @@
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
 import logging
+import sys
 from softwarecenter.db.dataprovider import dbus_main
 
 
 if __name__ == "__main__":
-    logging.basicConfig(level=logging.DEBUG)
+    if "--debug" in sys.argv:
+        logging.basicConfig(level=logging.DEBUG)
     dbus_main()
 
=== modified file 'softwarecenter/db/dataprovider.py'
--- softwarecenter/db/dataprovider.py	2012-09-18 11:36:15 +0000
+++ softwarecenter/db/dataprovider.py	2012-10-02 07:44:21 +0000
@@ -19,6 +19,7 @@
 import dbus
 import dbus.service
 import logging
+import time
 
 from dbus.mainloop.glib import DBusGMainLoop
 DBusGMainLoop(set_as_default=True)
@@ -50,11 +51,27 @@
 DBUS_DATA_PROVIDER_PATH = '/com/ubuntu/SoftwareCenterDataProvider'
 
 
+def update_activity_timestamp(fn):
+    def wrapped(*args, **kwargs):
+        self = args[0]
+        self._update_activity_timestamp()
+        return fn(*args, **kwargs)
+    return wrapped
+
+
 class SoftwareCenterDataProvider(dbus.service.Object):
 
-    def __init__(self, bus_name, object_path=DBUS_DATA_PROVIDER_PATH):
+    # 5 min by default
+    IDLE_TIMEOUT = 60 * 5
+    IDLE_CHECK_INTERVAL = 60
+
+    def __init__(self, bus_name, object_path=DBUS_DATA_PROVIDER_PATH,
+                 main_loop=None):
         dbus.service.Object.__init__(self, bus_name, object_path)
         self.bus_name = bus_name
+        if main_loop is None:
+            main_loop = GObject.MainLoop(GObject.main_context_default())
+        self.main_loop = main_loop
         # the database
         self.db = StoreDatabase()
         self.db.open()
@@ -66,16 +83,41 @@
         self.review_loader.refresh_review_stats()
         # ensure we query new applications
         run_software_center_agent(self.db)
+        # setup inactivity timer
+        self._update_activity_timestamp()
+        self._idle_timeout = GObject.timeout_add_seconds(
+            self.IDLE_CHECK_INTERVAL, self._check_inactivity)
 
     def stop(self):
         """ stop the dbus controller and remove from the bus """
-        self.remove_from_connection()
-
+        LOG.debug("stop() called")
+        self.main_loop.quit()
+        LOG.debug("exited")
+
+    # internal helper
+    def _check_inactivity(self):
+        """ Check for activity """
+        now = time.time()
+        if (self._activity_timestamp + self.IDLE_TIMEOUT) < now:
+            LOG.info("stopping after %s inactivity" % self.IDLE_TIMEOUT)
+            self.stop()
+        return True
+
+    def _update_activity_timestamp(self):
+        self._activity_timestamp = time.time()
+
+    # public dbus methods with their implementations, the dbus decorator
+    # does not like additional decorators so we use a seperate function
+    # for the actual implementation
     @dbus.service.method(DBUS_DATA_PROVIDER_IFACE,
                          in_signature='ss', out_signature='a{sv}')
     def GetAppDetails(self, appname, pkgname):
         LOG.debug("GetAppDetails() called with ('%s', '%s')" % (
                 appname, pkgname))
+        return self._get_app_details(appname, pkgname)
+
+    @update_activity_timestamp
+    def _get_app_details(self, appname, pkgname):
         app = Application(appname, pkgname)
         appdetails = app.get_details(self.db)
         return appdetails.as_dbus_property_dict()
@@ -84,12 +126,20 @@
                          in_signature='', out_signature='as')
     def GetAvailableCategories(self):
         LOG.debug("GetAvailableCategories() called")
+        return self._get_available_categories()
+
+    @update_activity_timestamp    
+    def _get_available_categories(self):
         return [cat.name for cat in self.categories]
 
     @dbus.service.method(DBUS_DATA_PROVIDER_IFACE,
                          in_signature='s', out_signature='as')
     def GetAvailableSubcategories(self, category_name):
         LOG.debug("GetAvailableSubcategories() called")
+        return self._get_available_subcategories(category_name)
+
+    @update_activity_timestamp
+    def _get_available_subcategories(self, category_name):
         cat = get_category_by_name(self.categories, category_name)
         return [subcat.name for subcat in cat.subcategories]
 
@@ -97,6 +147,10 @@
                          in_signature='s', out_signature='a(ssss)')
     def GetItemsForCategory(self, category_name):
         LOG.debug("GetItemsForCategory() called with ('%s')" % category_name)
+        return self._get_items_for_category(category_name)
+
+    @update_activity_timestamp
+    def _get_items_for_category(self, category_name):
         result = []
         cat = get_category_by_name(self.categories, category_name)
         for doc in cat.get_documents(self.db):
@@ -112,10 +166,13 @@
 def dbus_main(bus=None):
     if bus is None:
         bus = dbus.SessionBus()
+
+    main_context = GObject.main_context_default()
+    main_loop = GObject.MainLoop(main_context)
+
     bus_name = dbus.service.BusName(DBUS_BUS_NAME, bus)
-    data_provider = SoftwareCenterDataProvider(bus_name)
+    data_provider = SoftwareCenterDataProvider(bus_name, main_loop=main_loop)
     data_provider  # pyflakes
 
-    main_context = GObject.main_context_default()
-    main_loop = GObject.MainLoop(main_context)
+    # run it
     main_loop.run()
 
=== modified file 'tests/test_dataprovider.py'
--- tests/test_dataprovider.py	2012-09-20 01:05:10 +0000
+++ tests/test_dataprovider.py	2012-10-02 07:44:21 +0000
@@ -4,10 +4,15 @@
 import time
 import unittest
 
+from gi.repository import GObject
+
 from dbus.mainloop.glib import DBusGMainLoop
 DBusGMainLoop(set_as_default=True)
 
-from mock import Mock
+from mock import (
+    Mock,
+    patch,
+    )
 
 from tests.utils import (
     kill_process,
@@ -162,6 +167,47 @@
         self.assertEqual(len(result), 20)
 
 
+class IdleTimeoutTestCase(unittest.TestCase):
+
+    def setUp(self):
+        self.loop = GObject.MainLoop(GObject.main_context_default())
+
+        # setup bus
+        dbus_service_name = DBUS_BUS_NAME 
+        proc, dbus_address = start_dbus_daemon()
+        bus = dbus.bus.BusConnection(dbus_address)
+        # run the checks
+        self.bus_name = dbus.service.BusName(dbus_service_name, bus)
+
+    def tearDown(self):
+        self.loop.quit()
+
+    @patch.object(SoftwareCenterDataProvider, "IDLE_TIMEOUT")
+    @patch.object(SoftwareCenterDataProvider, "IDLE_CHECK_INTERVAL")
+    def test_idle_timeout(self, mock_timeout, mock_interval):
+        mock_timeout = 1
+        mock_timeout  # pyflakes
+        mock_interval = 1
+        mock_interval  # pyflakes
+
+        now = time.time()
+        provider = SoftwareCenterDataProvider(
+            self.bus_name, main_loop=self.loop)
+        provider  # pyflakes
+        self.loop.run()
+        # ensure this exited within a reasonable timeout
+        self.assertTrue((time.time() - now) < 5)
+
+    def test_idle_timeout_updates(self):
+        provider = SoftwareCenterDataProvider(
+            self.bus_name, main_loop=self.loop)
+        t1 = provider._activity_timestamp
+        time.sleep(0.1)
+        provider.GetAvailableCategories()
+        t2 = provider._activity_timestamp
+        self.assertTrue(t1 < t2)
+
+
 if __name__ == "__main__":
     #import logging
     #logging.basicConfig(level=logging.DEBUG)