Merge lp:~nataliabidart/ubuntuone-client/credentials-shutdown into lp:ubuntuone-client

[Natalia Bidart]

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

To test, run from this branch:

killall ubuntuone-login; DEBUG=True PYTHONPATH=. ./bin/ubuntuone-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)]

Looks good & works

review: Approve

[Roberto Alsina (ralsina)]

I like it.

review: Approve

[dobey (dobey)]

The attempt to merge lp:~nataliabidart/ubuntuone-client/credentials-shutdown into lp:ubuntuone-client failed. Below is the output from the failed tests.

/usr/bin/gnome-autogen.sh
checking for autoconf >= 2.53...
(B testing autoconf2.50... not found.
  testing autoconf... found 2.67
checking for automake >= 1.10...
(B testing automake-1.11... found 1.11.1
checking for libtool >= 1.5...
(B testing libtoolize... found 2.2.6b
checking for intltool >= 0.30...
(B testing intltoolize... found 0.41.1
checking for pkg-config >= 0.14.0...
(B testing pkg-config... found 0.25
checking for gtk-doc >= 1.0...
(B testing gtkdocize... found 1.15
Checking for required M4 macros...
(BChecking for forbidden M4 macros...
(BProcessing ./configure.ac
(BRunning libtoolize...
(Blibtoolize: putting auxiliary files in `.'.
libtoolize: copying file `./ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIR, `m4'.
libtoolize: copying file `m4/libtool.m4'
libtoolize: copying file `m4/ltoptions.m4'
libtoolize: copying file `m4/ltsugar.m4'
libtoolize: copying file `m4/ltversion.m4'
libtoolize: copying file `m4/lt~obsolete.m4'
Running intltoolize...
(BRunning gtkdocize...
(BRunning aclocal-1.11...
(BRunning autoconf...
(BRunning autoheader...
(BRunning automake-1.11...
(BRunning ./configure --enable-gtk-doc --enable-debug ...
(Bchecking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking for style of include used by make... GNU
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking dependency style of gcc... gcc3
checking for library containing strerror... none required
checking for gcc... (cached) gcc
checking whether we are using the GNU C compiler... (cached) yes
checking whether gcc accepts -g... (cached) yes
checking for gcc option to accept ISO C89... (cached) none needed
checking dependency style of gcc... (cached) gcc3
checking build system type... i686-pc-linux-gnu
checking host system type... i686-pc-linux-gnu
checking for a sed that does not truncate output... /bin/sed
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for fgrep... /bin/grep -F
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/nm -B
checking the name lister (/usr/bin/nm -B) interface... BSD nm
checking whether ln -s works... yes
checking the maximum length of command line arguments... 1572864
checking whether the shell understands some XSI constructs... yes
che...

[dobey (dobey)]

The attempt to merge lp:~nataliabidart/ubuntuone-client/credentials-shutdown into lp:ubuntuone-client failed. Below is the output from the failed tests.

/usr/bin/gnome-autogen.sh
checking for autoconf >= 2.53...
(B testing autoconf2.50... not found.
  testing autoconf... found 2.67
checking for automake >= 1.10...
(B testing automake-1.11... found 1.11.1
checking for libtool >= 1.5...
(B testing libtoolize... found 2.2.6b
checking for intltool >= 0.30...
(B testing intltoolize... found 0.41.1
checking for pkg-config >= 0.14.0...
(B testing pkg-config... found 0.25
checking for gtk-doc >= 1.0...
(B testing gtkdocize... found 1.15
Checking for required M4 macros...
(BChecking for forbidden M4 macros...
(BProcessing ./configure.ac
(BRunning libtoolize...
(Blibtoolize: putting auxiliary files in `.'.
libtoolize: copying file `./ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIR, `m4'.
libtoolize: copying file `m4/libtool.m4'
libtoolize: copying file `m4/ltoptions.m4'
libtoolize: copying file `m4/ltsugar.m4'
libtoolize: copying file `m4/ltversion.m4'
libtoolize: copying file `m4/lt~obsolete.m4'
Running intltoolize...
(BRunning gtkdocize...
(BRunning aclocal-1.11...
(BRunning autoconf...
(BRunning autoheader...
(BRunning automake-1.11...
(BRunning ./configure --enable-gtk-doc --enable-debug ...
(Bchecking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking for style of include used by make... GNU
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking dependency style of gcc... gcc3
checking for library containing strerror... none required
checking for gcc... (cached) gcc
checking whether we are using the GNU C compiler... (cached) yes
checking whether gcc accepts -g... (cached) yes
checking for gcc option to accept ISO C89... (cached) none needed
checking dependency style of gcc... (cached) gcc3
checking build system type... i686-pc-linux-gnu
checking host system type... i686-pc-linux-gnu
checking for a sed that does not truncate output... /bin/sed
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for fgrep... /bin/grep -F
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/nm -B
checking the name lister (/usr/bin/nm -B) interface... BSD nm
checking whether ln -s works... yes
checking the maximum length of command line arguments... 1572864
checking whether the shell understands some XSI constructs... yes
che...

=== modified file 'bin/ubuntuone-login'
--- bin/ubuntuone-login	2011-01-04 13:09:01 +0000
+++ bin/ubuntuone-login	2011-01-11 22:44:10 +0000
@@ -45,7 +45,9 @@
 
     logger.info("Starting Ubuntu One login manager for bus %r.", DBUS_BUS_NAME)
     bus_name = dbus.service.BusName(DBUS_BUS_NAME, bus=dbus.SessionBus())
-    CredentialsManagement(bus_name, object_path=DBUS_CREDENTIALS_PATH)
-
     mainloop = glib.MainLoop()
+    CredentialsManagement(timeout_func=glib.timeout_add,
+                          shutdown_func=mainloop.quit,
+                          bus_name=bus_name, object_path=DBUS_CREDENTIALS_PATH)
+
     mainloop.run()

=== modified file 'tests/credentials/test_credentials.py'
--- tests/credentials/test_credentials.py	2011-01-04 13:09:01 +0000
+++ tests/credentials/test_credentials.py	2011-01-11 22:44:10 +0000
@@ -25,7 +25,7 @@
 from ubuntuone.devtools.handlers import MementoHandler
 
 from ubuntuone.credentials import (dbus, logger, logging, ubuntu_sso,
-    CredentialsManagement,
+    CredentialsManagement, TIMEOUT_INTERVAL,
     DBUS_BUS_NAME, DBUS_CREDENTIALS_PATH, DBUS_CREDENTIALS_IFACE,
     APP_NAME, HELP_TEXT_KEY, DESCRIPTION, TC_URL_KEY, TC_URL,
     PING_URL_KEY, PING_URL, WINDOW_ID_KEY,
@@ -159,17 +159,15 @@
         self.register(app_name, args)
 
 
-class CredentialsManagementTestCase(TestCase):
-    """Test case for the DBus object that manages Ubuntu One credentials."""
+class BaseTestCase(TestCase):
+    """Base test case."""
 
     timeout = 2
     app_name = APP_NAME
     error_dict = None
-    signals = ('CredentialsFound', 'CredentialsNotFound', 'CredentialsCleared',
-               'CredentialsStored', 'CredentialsError', 'AuthorizationDenied')
 
     def setUp(self):
-        super(CredentialsManagementTestCase, self).setUp()
+        super(BaseTestCase, self).setUp()
         FakedSSOService.app_name = self.app_name
         FakedSSOService.error_dict = self.error_dict
 
@@ -181,12 +179,6 @@
         self.sso_server = self.register_server(ubuntu_sso.DBUS_BUS_NAME,
                                 ubuntu_sso.DBUS_CREDENTIALS_PATH,
                                 FakedSSOService)  # faked SSO server
-        self.creds_server = self.register_server(DBUS_BUS_NAME,
-                                DBUS_CREDENTIALS_PATH,
-                                CredentialsManagement)  # real service
-
-        self.deferred = Deferred()
-        self.proxy = self.get_creds_proxy()
 
     def register_server(self, bus_name, object_path, service_class):
         """Register a service on the session bus."""
@@ -209,6 +201,22 @@
                               object_path=ubuntu_sso.DBUS_CREDENTIALS_PATH,
                               dbus_interface=ubuntu_sso.DBUS_CREDENTIALS_IFACE)
 
+
+class CredentialsManagementTestCase(BaseTestCase):
+    """Test case for the DBus object that manages Ubuntu One credentials."""
+
+    signals = ('CredentialsFound', 'CredentialsNotFound', 'CredentialsCleared',
+               'CredentialsStored', 'CredentialsError', 'AuthorizationDenied')
+
+    def setUp(self):
+        super(CredentialsManagementTestCase, self).setUp()
+        self.creds_server = self.register_server(DBUS_BUS_NAME,
+                                DBUS_CREDENTIALS_PATH,
+                                CredentialsManagement)  # real service
+
+        self.deferred = Deferred()
+        self.proxy = self.get_creds_proxy()
+
     def get_creds_proxy(self):
         return self.get_proxy(bus_name=DBUS_BUS_NAME,
                               object_path=DBUS_CREDENTIALS_PATH,
@@ -578,3 +586,172 @@
 
     app_name = APP_NAME * 2
     error_dict = {'error_type': 'Test'}
+
+
+class RefCountingTestCase(BaseTestCase):
+    """Tests for the CredentialsManagement ref counting."""
+
+    def setUp(self):
+        super(RefCountingTestCase, self).setUp()
+        self._called = False
+        self.client = CredentialsManagement()
+
+    def _set_called(self, *args, **kwargs):
+        """Keep track of method calls."""
+        self._called = (args, kwargs)
+
+    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()
+
+        self.assertEqual(self.client.ref_count, 1)
+
+    def test_clear_credentials(self):
+        """Keep proper track of on going requests."""
+        self.client.clear_credentials()
+
+        self.assertEqual(self.client.ref_count, 1)
+
+    def test_store_credentials(self):
+        """Keep proper track of on going requests."""
+        self.client.store_credentials(FAKED_CREDENTIALS)
+
+        self.assertEqual(self.client.ref_count, 1)
+
+    def test_register(self):
+        """Keep proper track of on going requests."""
+        self.client.register()
+
+        self.assertEqual(self.client.ref_count, 1)
+
+    def test_login(self):
+        """Keep proper track of on going requests."""
+        self.client.login()
+
+        self.assertEqual(self.client.ref_count, 1)
+
+    def test_several_requests(self):
+        """Requests can be nested."""
+        self.client.login()
+        self.client.clear_credentials()
+        self.client.find_credentials()
+        self.client.register()
+        self.client.store_credentials(FAKED_CREDENTIALS)
+
+        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(FAKED_CREDENTIALS)
+
+        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()
+
+        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()
+
+        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()
+
+        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({'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()
+
+        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(FAKED_CREDENTIALS)
+
+        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()
+
+        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()
+
+        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()
+
+        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({'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()
+
+        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()
+        self.client.CredentialsFound(FAKED_CREDENTIALS)
+
+        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()
+
+        self.assertEqual(self._called, False)

=== modified file 'ubuntuone/credentials/__init__.py'
--- ubuntuone/credentials/__init__.py	2011-01-04 16:46:18 +0000
+++ ubuntuone/credentials/__init__.py	2011-01-11 22:44:10 +0000
@@ -35,6 +35,7 @@
 
 Q_ = lambda string: gettext.dgettext(GETTEXT_PACKAGE, string)
 NO_OP = lambda *args, **kwargs: None
+TIMEOUT_INTERVAL = 500
 
 LOG_LEVEL = logging.DEBUG
 path = os.path.join(LOGFOLDER, 'credentials.log')
@@ -68,8 +69,13 @@
 class CredentialsManagement(dbus.service.Object):
     """DBus object that manages Ubuntu One credentials."""
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, timeout_func=lambda *a: None,
+                 shutdown_func=lambda *a: None, *args, **kwargs):
         super(CredentialsManagement, self).__init__(*args, **kwargs)
+        self._ref_count = 0
+        self.timeout_func = timeout_func
+        self.shutdown_func = shutdown_func
+
         self.sso_match = None
         self.sso_proxy = self._get_sso_proxy()
 
@@ -113,42 +119,69 @@
 
         return proxy
 
+    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)
+
     # Operator not preceded by a space (fails with dbus decorators)
     # pylint: disable=C0322
 
     @dbus.service.signal(DBUS_CREDENTIALS_IFACE)
     def AuthorizationDenied(self):
         """Signal thrown when the user denies the authorization."""
+        self.ref_count -= 1
         logger.info('%s: emitting AuthorizationDenied.',
                     self.__class__.__name__)
 
     @dbus.service.signal(DBUS_CREDENTIALS_IFACE, signature='a{ss}')
     def CredentialsFound(self, credentials):
         """Signal thrown when the credentials are found."""
+        self.ref_count -= 1
         logger.info('%s: emitting CredentialsFound.',
                     self.__class__.__name__)
 
     @dbus.service.signal(DBUS_CREDENTIALS_IFACE)
     def CredentialsNotFound(self):
         """Signal thrown when the credentials are not found."""
+        self.ref_count -= 1
         logger.info('%s: emitting CredentialsNotFound.',
                     self.__class__.__name__)
 
     @dbus.service.signal(DBUS_CREDENTIALS_IFACE)
     def CredentialsCleared(self):
         """Signal thrown when the credentials were cleared."""
+        self.ref_count -= 1
         logger.info('%s: emitting CredentialsCleared.',
                     self.__class__.__name__)
 
     @dbus.service.signal(DBUS_CREDENTIALS_IFACE)
     def CredentialsStored(self):
         """Signal thrown when the credentials were cleared."""
+        self.ref_count -= 1
         logger.info('%s: emitting CredentialsStored.',
                     self.__class__.__name__)
 
     @dbus.service.signal(DBUS_CREDENTIALS_IFACE, signature='a{ss}')
     def CredentialsError(self, error_dict):
         """Signal thrown when there is a problem getting the credentials."""
+        self.ref_count -= 1
         logger.error('%s: emitting CredentialsError with error_dict %r.',
                      self.__class__.__name__, error_dict)
 
@@ -156,6 +189,7 @@
                          async_callbacks=("reply_handler", "error_handler"))
     def find_credentials(self, reply_handler=NO_OP, error_handler=NO_OP):
         """Ask the Ubuntu One credentials."""
+        self.ref_count += 1
         self.sso_proxy.find_credentials(APP_NAME, {},
             reply_handler=reply_handler, error_handler=error_handler)
 
@@ -164,6 +198,7 @@
                          async_callbacks=("reply_handler", "error_handler"))
     def clear_credentials(self, reply_handler=NO_OP, error_handler=NO_OP):
         """Clear the Ubuntu One credentials."""
+        self.ref_count += 1
         self.sso_proxy.clear_credentials(APP_NAME, {},
             reply_handler=reply_handler, error_handler=error_handler)
 
@@ -173,6 +208,7 @@
     def store_credentials(self, credentials,
                           reply_handler=NO_OP, error_handler=NO_OP):
         """Store the token for Ubuntu One application."""
+        self.ref_count += 1
         self.sso_proxy.store_credentials(APP_NAME, credentials,
             reply_handler=reply_handler, error_handler=error_handler)
 
@@ -180,6 +216,7 @@
                          async_callbacks=("reply_handler", "error_handler"))
     def register(self, reply_handler=NO_OP, error_handler=NO_OP):
         """Get credentials if found else prompt to register to Ubuntu One."""
+        self.ref_count += 1
         params = {HELP_TEXT_KEY: DESCRIPTION, TC_URL_KEY: TC_URL,
                   PING_URL_KEY: PING_URL, WINDOW_ID_KEY: '0'}
         self.sso_proxy.register(APP_NAME, params,