Merge lp:~nataliabidart/ubuntuone-control-panel/dont-start-syncdaemon into lp:ubuntuone-control-panel

[Natalia Bidart]

- Fixed (LP: #744731) with:
- Avoid creating a control-panel-backend in the OverviewPanel.
- Avoid FileSyncService and FileSyncStatus to start syncdaemon until load() is called.
- Make the dbus layer for the control panel backend not set the FileStatusChanged signal until is actually needed

[Roman Yepishev (rye)]

SD is not started immediately upon launching. One has actually enter the credentials first for syncdaemon to start doing things.

review: Approve (fieldtest)

[Shane Fagan (shanepatrickfagan)]

review: Approve

=== modified file 'ubuntuone/controlpanel/backend.py'
--- ubuntuone/controlpanel/backend.py	2011-04-12 13:13:58 +0000
+++ ubuntuone/controlpanel/backend.py	2011-04-13 21:06:35 +0000
@@ -108,7 +108,6 @@
         self.shutdown_func = shutdown_func
         self.wc = WebClient(dbus_client.get_credentials)
         self._status_changed_handler = None
-        self.status_changed_handler = lambda *a: None
 
         self._volumes = {}  # cache last known volume info
         self.file_sync_disabled = False
@@ -175,7 +174,7 @@
         self._status_changed_handler = process_and_callback
         dbus_client.set_status_changed_handler(process_and_callback)
 
-    def _get_status_changed_handler(self, handler):
+    def _get_status_changed_handler(self):
         """Return the handler to be called when file sync status changes."""
         return self._status_changed_handler
 

=== modified file 'ubuntuone/controlpanel/dbus_service.py'
--- ubuntuone/controlpanel/dbus_service.py	2011-04-07 14:30:20 +0000
+++ ubuntuone/controlpanel/dbus_service.py	2011-04-13 21:06:35 +0000
@@ -118,11 +118,9 @@
         """Create this instance of the backend."""
         super(ControlPanelBackend, self).__init__(*args, **kwargs)
         self.backend = backend
-        self.backend.status_changed_handler = self.process_status
         self.transform = lambda f: transform_failure(f, self.UnauthorizedError)
-        logger.debug('ControlPanelBackend: created with %r, %r.\n'
-                     'status_changed_handler is %r.',
-                     args, kwargs, self.process_status)
+        logger.debug('ControlPanelBackend: created with %r, %r.',
+                     args, kwargs)
 
     # pylint: disable=C0103
 
@@ -243,6 +241,9 @@
     @method(dbus_interface=DBUS_PREFERENCES_IFACE, in_signature="")
     def file_sync_status(self):
         """Get the status of the file sync service."""
+        if self.backend.status_changed_handler is None:
+            self.backend.status_changed_handler = self.process_status
+
         d = self.backend.file_sync_status()
         d.addCallback(self.process_status)
         d.addErrback(self.transform(self.FileSyncStatusError))

=== modified file 'ubuntuone/controlpanel/gtk/gui.py'
--- ubuntuone/controlpanel/gtk/gui.py	2011-04-12 13:46:26 +0000
+++ ubuntuone/controlpanel/gtk/gui.py	2011-04-13 21:06:35 +0000
@@ -161,18 +161,21 @@
 class ControlPanelMixin(object):
     """The main interface for the Ubuntu One control panel."""
 
-    def __init__(self, filename=None):
-        bus = dbus.SessionBus()
-        try:
-            obj = bus.get_object(DBUS_BUS_NAME,
-                                 DBUS_PREFERENCES_PATH,
-                                 follow_name_owner_changes=True)
-            iface = DBUS_PREFERENCES_IFACE
-            self.backend = dbus.Interface(obj, dbus_interface=iface)
-        except dbus.exceptions.DBusException:
-            logger.exception('Can not connect to DBus at %r',
-                             (DBUS_BUS_NAME, DBUS_PREFERENCES_PATH))
-            raise
+    def __init__(self, filename=None, backend_instance=None):
+        if backend_instance is not None:
+            self.backend = backend_instance
+        else:
+            bus = dbus.SessionBus()
+            try:
+                obj = bus.get_object(DBUS_BUS_NAME,
+                                     DBUS_PREFERENCES_PATH,
+                                     follow_name_owner_changes=True)
+                iface = DBUS_PREFERENCES_IFACE
+                self.backend = dbus.Interface(obj, dbus_interface=iface)
+            except dbus.exceptions.DBusException:
+                logger.exception('Can not connect to DBus at %r',
+                                 (DBUS_BUS_NAME, DBUS_PREFERENCES_PATH))
+                raise
 
         if filename is not None:
             builder = gtk.Builder()
@@ -284,39 +287,42 @@
 
     def __init__(self, main_window):
         GreyableBin.__init__(self)
-        ControlPanelMixin.__init__(self, filename='overview.ui')
-        self.add(self.itself)
-        self.warning_label.set_text('')
-        self.warning_label.set_property('xalign', 0.5)
-
-        self.connect_button.set_uri(self.CONNECT)
-
-        self.main_window = main_window
-        self._credentials_are_new = False
-        self.show()
-
+
+        sso_backend = None
         bus = dbus.SessionBus()
         try:
             obj = bus.get_object(ubuntu_sso.DBUS_BUS_NAME,
                                  ubuntu_sso.DBUS_CREDENTIALS_PATH,
                                  follow_name_owner_changes=True)
             iface = ubuntu_sso.DBUS_CREDENTIALS_IFACE
-            self.sso_backend = dbus.Interface(obj, dbus_interface=iface)
+            sso_backend = dbus.Interface(obj, dbus_interface=iface)
         except dbus.exceptions.DBusException:
             logger.exception('Can not connect to DBus at %r',
                              (ubuntu_sso.DBUS_BUS_NAME,
                               ubuntu_sso.DBUS_CREDENTIALS_PATH))
             raise
 
-        # Disconnect signals!!! (LP: #673667)
-        self.sso_backend.connect_to_signal('CredentialsFound',
-                                           self.on_credentials_found)
-        self.sso_backend.connect_to_signal('CredentialsNotFound',
-                                           self.on_credentials_not_found)
-        self.sso_backend.connect_to_signal('CredentialsError',
-                                           self.on_credentials_error)
-        self.sso_backend.connect_to_signal('AuthorizationDenied',
-                                           self.on_authorization_denied)
+        ControlPanelMixin.__init__(self, filename='overview.ui',
+                                   backend_instance=sso_backend)
+        self.add(self.itself)
+        self.warning_label.set_text('')
+        self.warning_label.set_property('xalign', 0.5)
+
+        self.connect_button.set_uri(self.CONNECT)
+
+        self.main_window = main_window
+        self._credentials_are_new = False
+        self.show()
+
+        self.backend.connect_to_signal('CredentialsFound',
+                                       self.on_credentials_found)
+        self.backend.connect_to_signal('CredentialsNotFound',
+                                       self.on_credentials_not_found)
+        self.backend.connect_to_signal('CredentialsError',
+                                       self.on_credentials_error)
+        self.backend.connect_to_signal('AuthorizationDenied',
+                                       self.on_authorization_denied)
+
         kw = dict(result_cb=self.on_network_state_changed)
         self.network_manager_state = networkstate.NetworkManagerState(**kw)
         self.network_manager_state.find_online_state()
@@ -348,7 +354,7 @@
         settings = {TC_URL_KEY: U1_TC_URL, HELP_TEXT_KEY: U1_DESCRIPTION,
                     WINDOW_ID_KEY: str(self.main_window.window.xid),
                     PING_URL_KEY: U1_PING_URL}
-        self.sso_backend.register(U1_APP_NAME, settings,
+        self.backend.register(U1_APP_NAME, settings,
             reply_handler=NO_OP, error_handler=error_handler)
         self.set_property('greyed', True)
         self.warning_label.set_text('')
@@ -358,7 +364,7 @@
         settings = {TC_URL_KEY: U1_TC_URL, HELP_TEXT_KEY: U1_DESCRIPTION,
                     WINDOW_ID_KEY: str(self.main_window.window.xid),
                     PING_URL_KEY: U1_PING_URL}
-        self.sso_backend.login(U1_APP_NAME, settings,
+        self.backend.login(U1_APP_NAME, settings,
             reply_handler=NO_OP, error_handler=error_handler)
         self.set_property('greyed', True)
         self.warning_label.set_text('')
@@ -405,7 +411,7 @@
         else:
             self.set_sensitive(True)
             self.warning_label.set_text(msg)
-            self.sso_backend.find_credentials(U1_APP_NAME, {},
+            self.backend.find_credentials(U1_APP_NAME, {},
                 reply_handler=NO_OP, error_handler=error_handler)
 
 
@@ -1080,9 +1086,6 @@
         self.backend.connect_to_signal('FilesEnabled', self.on_files_enabled)
         self.backend.connect_to_signal('FilesDisabled', self.on_files_disabled)
 
-        self.backend.file_sync_status(reply_handler=NO_OP,
-                                      error_handler=error_handler)
-
     @log_call(logger.debug)
     def on_file_sync_status_changed(self, status):
         """File Sync status changed."""
@@ -1117,6 +1120,11 @@
             self.backend.disable_files(reply_handler=NO_OP,
                                        error_handler=error_handler)
 
+    def load(self):
+        """Load the information."""
+        self.backend.file_sync_status(reply_handler=NO_OP,
+                                      error_handler=error_handler)
+
 
 class DesktopcouchService(Service):
     """A desktopcouch service."""
@@ -1248,6 +1256,7 @@
     @log_call(logger.debug)
     def load(self):
         """Load info."""
+        self.file_sync_service.load()
         self.replications.hide()
         if self.install_box is not None:
             self.itself.remove(self.install_box)

=== modified file 'ubuntuone/controlpanel/gtk/tests/test_gui_basic.py'
--- ubuntuone/controlpanel/gtk/tests/test_gui_basic.py	2011-04-11 17:30:04 +0000
+++ ubuntuone/controlpanel/gtk/tests/test_gui_basic.py	2011-04-13 21:06:35 +0000
@@ -445,17 +445,17 @@
 
     def test_sso_backend(self):
         """Has a correct SSO backend."""
-        self.assertIsInstance(self.ui.sso_backend, FakedSSOBackend)
+        self.assertIsInstance(self.ui.backend, FakedSSOBackend)
 
     def test_sso_backend_signals(self):
         """The proper signals are connected to the backend."""
-        self.assertEqual(self.ui.sso_backend._signals['CredentialsFound'],
+        self.assertEqual(self.ui.backend._signals['CredentialsFound'],
                          [self.ui.on_credentials_found])
-        self.assertEqual(self.ui.sso_backend._signals['CredentialsNotFound'],
+        self.assertEqual(self.ui.backend._signals['CredentialsNotFound'],
                          [self.ui.on_credentials_not_found])
-        self.assertEqual(self.ui.sso_backend._signals['CredentialsError'],
+        self.assertEqual(self.ui.backend._signals['CredentialsError'],
                          [self.ui.on_credentials_error])
-        self.assertEqual(self.ui.sso_backend._signals['AuthorizationDenied'],
+        self.assertEqual(self.ui.backend._signals['AuthorizationDenied'],
                          [self.ui.on_authorization_denied])
 
 
@@ -507,8 +507,7 @@
     def test_find_credentials_is_called(self):
         """Credentials are asked to SSO backend."""
         self.assertFalse(self.ui._credentials_are_new)
-        self.assert_backend_called('find_credentials', (gui.U1_APP_NAME, {}),
-                                   backend=self.ui.sso_backend)
+        self.assert_backend_called('find_credentials', (gui.U1_APP_NAME, {}))
 
     def test_on_credentials_found(self):
         """Callback 'on_credentials_found' is correct."""
@@ -630,8 +629,7 @@
                  gui.HELP_TEXT_KEY: gui.U1_DESCRIPTION,
                  gui.WINDOW_ID_KEY: str(window_id),
                  gui.PING_URL_KEY: gui.U1_PING_URL})
-        self.assert_backend_called('register', args,
-                                   backend=self.ui.sso_backend)
+        self.assert_backend_called('register', args)
 
     def test_connect_button_clicked(self):
         """Test the 'join now' button callback."""
@@ -646,8 +644,7 @@
                  gui.HELP_TEXT_KEY: gui.U1_DESCRIPTION,
                  gui.WINDOW_ID_KEY: str(window_id),
                  gui.PING_URL_KEY: gui.U1_PING_URL})
-        self.assert_backend_called('login', args,
-                                   backend=self.ui.sso_backend)
+        self.assert_backend_called('login', args)
 
     def test_join_now_button_clicked_set_greyed(self):
         """Clicking on 'join_now' self is greyed."""

=== modified file 'ubuntuone/controlpanel/integrationtests/test_dbus_service.py'
--- ubuntuone/controlpanel/integrationtests/test_dbus_service.py	2011-04-06 19:31:04 +0000
+++ ubuntuone/controlpanel/integrationtests/test_dbus_service.py	2011-04-13 21:06:35 +0000
@@ -122,11 +122,11 @@
         dbus_service.MSG_KEY: 'test me please',
         dbus_service.STATUS_KEY: dbus_service.FILE_SYNC_IDLE,
     }
-    status_changed_handler = None
     shutdown_func = None
 
     def __init__(self, shutdown_func=None):
         MockBackend.shutdown_func = shutdown_func
+        self._status_changed_handler = []
 
     def _process(self, result):
         """Process the request with the given result."""
@@ -135,6 +135,24 @@
             return defer.fail(self.exception(result))
         return defer.succeed(result)
 
+    def _set_status_changed_handler(self, handler):
+        """Set 'handler' to be called when file sync status changes."""
+        if handler is not None:
+            self._status_changed_handler.append(handler)
+        else:
+            self._status_changed_handler = []
+
+    def _get_status_changed_handler(self):
+        """Return the handler to be called when file sync status changes."""
+        if len(self._status_changed_handler) > 0:
+            result = self._status_changed_handler[-1]
+        else:
+            result = None
+        return result
+
+    status_changed_handler = property(_get_status_changed_handler,
+                                      _set_status_changed_handler)
+
     def account_info(self):
         """Get the user account info."""
         return self._process(SAMPLE_ACCOUNT_INFO)
@@ -737,10 +755,28 @@
     def test_status_changed_handler(self):
         """The status changed handler is properly set."""
         be = MockBackend()
+        dbus_service.ControlPanelBackend(backend=be)
+
+        self.assertEqual(be.status_changed_handler, None)
+
+    def test_status_changed_handler_after_status_requested(self):
+        """The status changed handler is properly set."""
+        be = MockBackend()
         cpbe = dbus_service.ControlPanelBackend(backend=be)
+        cpbe.file_sync_status()
 
         self.assertEqual(be.status_changed_handler, cpbe.process_status)
 
+    def test_status_changed_handler_after_status_requested_twice(self):
+        """The status changed handler is properly set."""
+        be = MockBackend()
+        cpbe = dbus_service.ControlPanelBackend(backend=be)
+        cpbe.file_sync_status()
+        cpbe.file_sync_status()
+
+        # pylint: disable=W0212
+        self.assertEqual(be._status_changed_handler, [cpbe.process_status])
+
 
 class ShutdownTestCase(BaseTestCase):
     """Test for the DBus service shurdown."""