Merge lp:~nataliabidart/ubuntu-sso-client/shutdown into lp:ubuntu-sso-client

[Natalia Bidart]

The service should shutdown when unused (LP: #701606).

To test, run from this branch:

killall ubuntu-sso-login; DEBUG=True PYTHONPATH=. ./bin/ubuntu-sso-login

And open d-feet and play with the com.ubuntuone.Credentials interface. You should expect to see the service quitting once there is no more requests to process.

[Eric Casteleijn (thisfred)]

Code looks good, and works as well!

review: Approve

[Roberto Alsina (ralsina)]

I like it.

review: Approve

=== modified file 'bin/ubuntu-sso-login'
--- bin/ubuntu-sso-login	2010-12-20 14:29:18 +0000
+++ bin/ubuntu-sso-login	2011-01-11 22:44:07 +0000
@@ -94,6 +94,8 @@
     bus_name = dbus.service.BusName(DBUS_BUS_NAME, bus=dbus.SessionBus())
     SSOLogin(bus_name, object_path=DBUS_ACCOUNT_PATH)
     SSOCredentials(bus_name, object_path=DBUS_CRED_PATH)
-    CredentialsManagement(bus_name, object_path=DBUS_CREDENTIALS_PATH)
+    CredentialsManagement(timeout_func=gtk.timeout_add,
+                          shutdown_func=gtk.main_quit,
+                          bus_name=bus_name, object_path=DBUS_CREDENTIALS_PATH)
 
     gtk.main()

=== modified file 'ubuntu_sso/logger.py'
--- ubuntu_sso/logger.py	2010-10-01 15:21:25 +0000
+++ ubuntu_sso/logger.py	2011-01-11 22:44:07 +0000
@@ -57,6 +57,7 @@
     logger.addHandler(MAIN_HANDLER)
     if os.environ.get('DEBUG'):
         debug_handler = logging.StreamHandler(sys.stderr)
+        debug_handler.setFormatter(logging.Formatter(fmt=FMT))
         logger.addHandler(debug_handler)
 
     return logger

=== modified file 'ubuntu_sso/main.py'
--- ubuntu_sso/main.py	2011-01-02 03:34:23 +0000
+++ ubuntu_sso/main.py	2011-01-11 22:44:07 +0000
@@ -50,6 +50,7 @@
 
 logger = setup_logging("ubuntu_sso.main")
 U1_PING_URL = "https://one.ubuntu.com/oauth/sso-finished-so-get-tokens/"
+TIMEOUT_INTERVAL = 500
 
 
 class SSOLoginProcessor(Account):
@@ -421,6 +422,12 @@
 
     """
 
+    def __init__(self, timeout_func, shutdown_func, *args, **kwargs):
+        super(CredentialsManagement, self).__init__(*args, **kwargs)
+        self._ref_count = 0
+        self.timeout_func = timeout_func
+        self.shutdown_func = shutdown_func
+
     # Operator not preceded by a space (fails with dbus decorators)
     # pylint: disable=C0322
 
@@ -440,39 +447,66 @@
         """Process the 'failure' and emit CredentialsError."""
         self.CredentialsError(app_name, except_to_errdict(failure.value))
 
+    def _get_ref_count(self):
+        """Get value of ref_count."""
+        logger.debug('ref_count is %r.', self._ref_count)
+        return self._ref_count
+
+    def _set_ref_count(self, new_value):
+        """Set a new value to ref_count."""
+        logger.debug('ref_count is %r, changing value to %r.',
+                     self._ref_count, new_value)
+        if new_value < 0:
+            self._ref_count = 0
+            msg = 'Attempting to decrease ref_count to a negative value (%r).'
+            logger.warning(msg, new_value)
+        else:
+            self._ref_count = new_value
+
+        if self._ref_count == 0:
+            self.timeout_func(TIMEOUT_INTERVAL, self.shutdown_func)
+
+    ref_count = property(fget=_get_ref_count, fset=_set_ref_count)
+
     @dbus.service.signal(DBUS_CREDENTIALS_IFACE, signature='s')
     def AuthorizationDenied(self, app_name):
         """Signal thrown when the user denies the authorization."""
+        self.ref_count -= 1
         logger.info('%s: emitting AuthorizationDenied with app_name "%s".',
                     self.__class__.__name__, app_name)
 
     @dbus.service.signal(DBUS_CREDENTIALS_IFACE, signature='sa{ss}')
     def CredentialsFound(self, app_name, credentials):
         """Signal thrown when the credentials are found."""
+        self.ref_count -= 1
         logger.info('%s: emitting CredentialsFound with app_name "%s".',
                     self.__class__.__name__, app_name)
 
     @dbus.service.signal(DBUS_CREDENTIALS_IFACE, signature='s')
     def CredentialsNotFound(self, app_name):
         """Signal thrown when the credentials are not found."""
+        self.ref_count -= 1
         logger.info('%s: emitting CredentialsNotFound with app_name "%s".',
                     self.__class__.__name__, app_name)
 
     @dbus.service.signal(DBUS_CREDENTIALS_IFACE, signature='s')
     def CredentialsCleared(self, app_name):
         """Signal thrown when the credentials were cleared."""
+        self.ref_count -= 1
         logger.info('%s: emitting CredentialsCleared with app_name "%s".',
                     self.__class__.__name__, app_name)
 
     @dbus.service.signal(DBUS_CREDENTIALS_IFACE, signature='s')
     def CredentialsStored(self, app_name):
         """Signal thrown when the credentials were cleared."""
+        self.ref_count -= 1
         logger.info('%s: emitting CredentialsStored with app_name "%s".',
                     self.__class__.__name__, app_name)
 
     @dbus.service.signal(DBUS_CREDENTIALS_IFACE, signature='sa{ss}')
     def CredentialsError(self, app_name, error_dict):
         """Signal thrown when there is a problem getting the credentials."""
+        self.ref_count -= 1
         logger.error('%s: emitting CredentialsError with app_name "%s" and '
                      'error_dict %r.', self.__class__.__name__, app_name,
                      error_dict)
@@ -488,6 +522,7 @@
         - 'args' is a dictionary, currently not used.
 
         """
+        self.ref_count += 1
 
         def success_cb(credentials):
             """Find credentials and notify using signals."""
@@ -513,6 +548,8 @@
         - 'args' is a dictionary, currently not used.
 
         """
+        self.ref_count += 1
+
         obj = Credentials(app_name)
         d = obj.clear_credentials()
         # pylint: disable=E1101
@@ -532,6 +569,8 @@
         'consumer_secret'.
 
         """
+        self.ref_count += 1
+
         obj = Credentials(app_name)
         d = obj.store_credentials(args)
         # pylint: disable=E1101
@@ -542,6 +581,8 @@
                          in_signature='sa{ss}', out_signature='')
     def register(self, app_name, args):
         """Get credentials if found else prompt GUI to register."""
+        self.ref_count += 1
+
         obj = Credentials(app_name, **self._parse_args(args))
         obj.register()
 
@@ -549,5 +590,7 @@
                          in_signature='sa{ss}', out_signature='')
     def login(self, app_name, args):
         """Get credentials if found else prompt GUI to login."""
+        self.ref_count += 1
+
         obj = Credentials(app_name, **self._parse_args(args))
         obj.login()

=== modified file 'ubuntu_sso/tests/__init__.py'
--- ubuntu_sso/tests/__init__.py	2010-11-19 19:53:22 +0000
+++ ubuntu_sso/tests/__init__.py	2011-01-11 22:44:07 +0000
@@ -18,6 +18,8 @@
 
 import os
 
+from twisted.trial import unittest
+
 from ubuntu_sso.keyring import get_token_name
 
 APP_NAME = 'The Super App!'
@@ -45,3 +47,15 @@
 TOKEN_NAME = get_token_name(APP_NAME)
 TC_URL = 'http://localhost/'
 WINDOW_ID = 5
+
+
+class TestCase(unittest.TestCase):
+    """Customized test case that keeps tracks of method calls."""
+
+    def setUp(self):
+        super(TestCase, self).setUp()
+        self._called = False
+
+    def _set_called(self, *args, **kwargs):
+        """Keep track of a method call."""
+        self._called = (args, kwargs)

=== modified file 'ubuntu_sso/tests/test_main.py'
--- ubuntu_sso/tests/test_main.py	2011-01-02 03:34:23 +0000
+++ ubuntu_sso/tests/test_main.py	2011-01-11 22:44:07 +0000
@@ -34,14 +34,15 @@
 
 from ubuntu_sso import DBUS_CREDENTIALS_IFACE
 from ubuntu_sso.keyring import U1_APP_NAME
-from ubuntu_sso.main import (U1_PING_URL, blocking, except_to_errdict,
+from ubuntu_sso.main import (U1_PING_URL, TIMEOUT_INTERVAL,
+    blocking, except_to_errdict,
     CredentialsManagement, SSOCredentials, SSOLogin)
 from ubuntu_sso.main import (HELP_TEXT_KEY, PING_URL_KEY,
     TC_URL_KEY, UI_CLASS_KEY, UI_MODULE_KEY, WINDOW_ID_KEY,
     SUCCESS_CB_KEY, ERROR_CB_KEY, DENIAL_CB_KEY)
 from ubuntu_sso.tests import (APP_NAME, TC_URL, HELP_TEXT, CAPTCHA_ID,
     CAPTCHA_SOLUTION, EMAIL, EMAIL_TOKEN, PASSWORD, PING_URL, TOKEN,
-    TOKEN_NAME, WINDOW_ID)
+    TOKEN_NAME, WINDOW_ID, TestCase)
 
 
 # Access to a protected member 'yyy' of a client class
@@ -709,15 +710,23 @@
                 }
 
     def setUp(self):
+        super(CredentialsManagementTestCase, self).setUp()
+
         self.mocker = Mocker()
-        self.client = CredentialsManagement()
+        self.client = CredentialsManagement(timeout_func=lambda *a: None,
+                                            shutdown_func=lambda *a: None)
         self.args = {}
         self.cred_args = {}
 
+        self.memento = MementoHandler()
+        self.memento.setLevel(logging.DEBUG)
+        ubuntu_sso.main.logger.addHandler(self.memento)
+
     def tearDown(self):
         """Verify the mocking stuff and shut it down."""
         self.mocker.verify()
         self.mocker.restore()
+        super(CredentialsManagementTestCase, self).tearDown()
 
     def assert_dbus_method_correct(self, method):
         """Check that 'method' is a dbus method with proper signatures."""
@@ -740,6 +749,166 @@
         self.assertIsInstance(self.client, ubuntu_sso.main.dbus.service.Object)
 
 
+class CredentialsManagementRefCountingTestCase(CredentialsManagementTestCase):
+    """Tests for the CredentialsManagement ref counting."""
+
+    def test_ref_counting(self):
+        """Ref counting is in place."""
+        self.assertEqual(self.client.ref_count, 0)
+
+    def test_find_credentials(self):
+        """Keep proper track of on going requests."""
+        self.client.find_credentials(APP_NAME, self.args)
+
+        self.assertEqual(self.client.ref_count, 1)
+
+    def test_clear_credentials(self):
+        """Keep proper track of on going requests."""
+        self.client.clear_credentials(APP_NAME, self.args)
+
+        self.assertEqual(self.client.ref_count, 1)
+
+    def test_store_credentials(self):
+        """Keep proper track of on going requests."""
+        self.client.store_credentials(APP_NAME, self.args)
+
+        self.assertEqual(self.client.ref_count, 1)
+
+    def test_register(self):
+        """Keep proper track of on going requests."""
+        self.client.register(APP_NAME, self.args)
+
+        self.assertEqual(self.client.ref_count, 1)
+
+    def test_login(self):
+        """Keep proper track of on going requests."""
+        self.client.login(APP_NAME, self.args)
+
+        self.assertEqual(self.client.ref_count, 1)
+
+    def test_several_requests(self):
+        """Requests can be nested."""
+        self.client.login(APP_NAME, self.args)
+        self.client.clear_credentials(APP_NAME, self.args)
+        self.client.find_credentials(APP_NAME, self.args)
+        self.client.register(APP_NAME, self.args)
+        self.client.store_credentials(APP_NAME, self.args)
+
+        self.assertEqual(self.client.ref_count, 5)
+
+    def test_credentials_found(self):
+        """Ref counter is decreased when a signal is sent."""
+        self.client.ref_count = 3
+        self.client.CredentialsFound(APP_NAME, TOKEN)
+
+        self.assertEqual(self.client.ref_count, 2)
+
+    def test_credentials_not_found(self):
+        """Ref counter is decreased when a signal is sent."""
+        self.client.ref_count = 3
+        self.client.CredentialsNotFound(APP_NAME)
+
+        self.assertEqual(self.client.ref_count, 2)
+
+    def test_credentials_cleared(self):
+        """Ref counter is decreased when a signal is sent."""
+        self.client.ref_count = 3
+        self.client.CredentialsCleared(APP_NAME)
+
+        self.assertEqual(self.client.ref_count, 2)
+
+    def test_credentials_stored(self):
+        """Ref counter is decreased when a signal is sent."""
+        self.client.ref_count = 3
+        self.client.CredentialsStored(APP_NAME)
+
+        self.assertEqual(self.client.ref_count, 2)
+
+    def test_credentials_error(self):
+        """Ref counter is decreased when a signal is sent."""
+        self.client.ref_count = 3
+        self.client.CredentialsError(APP_NAME, {'error_type': 'test'})
+
+        self.assertEqual(self.client.ref_count, 2)
+
+    def test_authorization_denied(self):
+        """Ref counter is decreased when a signal is sent."""
+        self.client.ref_count = 3
+        self.client.AuthorizationDenied(APP_NAME)
+
+        self.assertEqual(self.client.ref_count, 2)
+
+    def test_credentials_found_when_ref_count_is_not_positive(self):
+        """Ref counter is decreased when a signal is sent."""
+        self.client._ref_count = -3
+        self.client.CredentialsFound(APP_NAME, TOKEN)
+
+        self.assertEqual(self.client.ref_count, 0)
+        msg = 'Attempting to decrease ref_count to a negative value (-4).'
+        self.assertTrue(self.memento.check_warning(msg))
+
+    def test_credentials_not_found_when_ref_count_is_not_positive(self):
+        """Ref counter is decreased when a signal is sent."""
+        self.client._ref_count = -3
+        self.client.CredentialsNotFound(APP_NAME)
+
+        self.assertEqual(self.client.ref_count, 0)
+        msg = 'Attempting to decrease ref_count to a negative value (-4).'
+        self.assertTrue(self.memento.check_warning(msg))
+
+    def test_credentials_cleared_when_ref_count_is_not_positive(self):
+        """Ref counter is decreased when a signal is sent."""
+        self.client._ref_count = -3
+        self.client.CredentialsCleared(APP_NAME)
+
+        self.assertEqual(self.client.ref_count, 0)
+        msg = 'Attempting to decrease ref_count to a negative value (-4).'
+        self.assertTrue(self.memento.check_warning(msg))
+
+    def test_credentials_stored_when_ref_count_is_not_positive(self):
+        """Ref counter is decreased when a signal is sent."""
+        self.client._ref_count = -3
+        self.client.CredentialsStored(APP_NAME)
+
+        self.assertEqual(self.client.ref_count, 0)
+        msg = 'Attempting to decrease ref_count to a negative value (-4).'
+        self.assertTrue(self.memento.check_warning(msg))
+
+    def test_credentials_error_when_ref_count_is_not_positive(self):
+        """Ref counter is decreased when a signal is sent."""
+        self.client._ref_count = -3
+        self.client.CredentialsError(APP_NAME, {'error_type': 'test'})
+
+        self.assertEqual(self.client.ref_count, 0)
+        msg = 'Attempting to decrease ref_count to a negative value (-4).'
+        self.assertTrue(self.memento.check_warning(msg))
+
+    def test_autorization_denied_when_ref_count_is_not_positive(self):
+        """Ref counter is decreased when a signal is sent."""
+        self.client._ref_count = -3
+        self.client.AuthorizationDenied(APP_NAME)
+
+        self.assertEqual(self.client.ref_count, 0)
+        msg = 'Attempting to decrease ref_count to a negative value (-4).'
+        self.assertTrue(self.memento.check_warning(msg))
+
+    def test_on_zero_ref_count_shutdown(self):
+        """When ref count reaches 0, queue shutdown op."""
+        self.client.timeout_func = self._set_called
+        self.client.find_credentials(APP_NAME, self.args)
+        self.client.CredentialsFound(APP_NAME, TOKEN)
+
+        self.assertEqual(self._called,
+                         ((TIMEOUT_INTERVAL, self.client.shutdown_func), {}))
+
+    def test_on_non_zero_ref_count_do_not_shutdown(self):
+        """If ref count is not 0, do not queue shutdown op."""
+        self.client.timeout_func = self._set_called
+        self.client.find_credentials(APP_NAME, self.args)
+
+        self.assertEqual(self._called, False)
+
+
 class CredentialsManagementFindTestCase(CredentialsManagementTestCase):
     """Tests for the CredentialsManagement find method."""
 
@@ -989,7 +1158,8 @@
     """Tests for the CredentialsManagement DBus signals."""
 
     def setUp(self):
-        self.client = CredentialsManagement()
+        self.client = CredentialsManagement(timeout_func=lambda *a: None,
+                                            shutdown_func=lambda *a: None)
 
         self.memento = MementoHandler()
         self.memento.setLevel(logging.DEBUG)