Merge lp:~alecu/ubuntuone-client/unbreak-connect into lp:ubuntuone-client

[Alejandro J. Cura]

Fix CredentialsManager so connect and restart now work (LP: #811307)

Fix CredentialsManager so connect and restart now work (LP: #811307)

[Natalia Bidart (nataliabidart)]

Can we have somehow this branch managing the fact that

token = yield self._request_token(autoconnecting=autoconnecting)

can return the empty dict when autoconnecting is True and there are no credentials in the system?
In that case, the current linux behavior is to also raise NoAccessToken.

review: Needs Information

[Alejandro J. Cura (alecu)]

Fixed empty dictionary as suggested by Natalia.

[lp:~alecu/ubuntuone-client/unbreak-connect]

1060. By Alejandro J. Cura on 2011-07-20

handle empty credentials as NoAccessToken

1061. By Alejandro J. Cura on 2011-07-20

spaces after the colon in lambdas

[Natalia Bidart (nataliabidart)]

Great branch!

review: Approve

[Roberto Alsina (ralsina)]

review: Approve

=== modified file 'tests/platform/windows/test_credentials.py'
--- tests/platform/windows/test_credentials.py	2011-07-04 15:02:47 +0000
+++ tests/platform/windows/test_credentials.py	2011-07-20 15:02:53 +0000
@@ -61,10 +61,26 @@
             setattr(self, callback_name, None)
 
 
-    def find_credentials(self, app_name, options, **kwargs):
+    def find_credentials(self, app_name, options):
         """Ask the U1 credentials."""
         return defer.succeed(TEST_CREDENTIALS)
 
+    def clear_credentials(self, app_name, options):
+        """Clear the U1 credentials."""
+        return defer.succeed(None)
+
+    def store_credentials(self, app_name, options):
+        """Store the U1 credentials."""
+        return defer.succeed(None)
+
+    def register(self, app_name, options):
+        """Store the U1 credentials."""
+        return defer.succeed(None)
+
+    def login(self, app_name, options):
+        """Store the U1 credentials."""
+        return defer.succeed(None)
+
 
 class RemovableSignalTestCase(TestCase):
     """Tests for RemovableSignal."""
@@ -111,6 +127,39 @@
         self.cm.find_credentials(reply_handler=ok, error_handler=error)
         return d
 
+    def test_clear_credentials(self):
+        """Test the clear_credentials method."""
+        d = defer.Deferred()
+        ok = lambda: d.callback("ok")
+        error = lambda *args: d.errback(args)
+        self.cm.clear_credentials(reply_handler=ok, error_handler=error)
+        return d
+
+    def test_store_credentials(self):
+        """Test the store_credentials method."""
+        d = defer.Deferred()
+        ok = lambda: d.callback("ok")
+        error = lambda *args: d.errback(args)
+        self.cm.store_credentials(TEST_CREDENTIALS, reply_handler=ok,
+                                  error_handler=error)
+        return d
+
+    def test_register(self):
+        """Test the register method."""
+        d = defer.Deferred()
+        ok = lambda: d.callback("ok")
+        error = lambda *args: d.errback(args)
+        self.cm.register({}, reply_handler=ok, error_handler=error)
+        return d
+
+    def test_login(self):
+        """Test the login method."""
+        d = defer.Deferred()
+        ok = lambda: d.callback("ok")
+        error = lambda *args: d.errback(args)
+        self.cm.login({}, reply_handler=ok, error_handler=error)
+        return d
+
     def test_register_to_credentials_found(self):
         """Test the register_to_credentials_found method."""
         cb = lambda creds: self.assertEqual(creds, TEST_CREDENTIALS)

=== modified file 'tests/platform/windows/test_ipc.py'
--- tests/platform/windows/test_ipc.py	2011-07-15 17:09:43 +0000
+++ tests/platform/windows/test_ipc.py	2011-07-20 15:02:53 +0000
@@ -3263,13 +3263,20 @@
             return defer.fail(RandomException())
         return defer.succeed(self.fake_credentials)
 
-    def register(self, *args):
+    def register(self, window_id=0):
         """Register a new user."""
+        assert isinstance(window_id, (str, int))
         if self.should_fail:
             return defer.fail(RandomException())
         return defer.succeed(self.fake_credentials)
 
 
+class EmptyCredentialsManagementTool(FakeCredentialsManagementTool):
+    """A Fake CredentialsManagementTool that returns empty credentials."""
+
+    fake_credentials = {}
+
+
 class FakeCredentialsManagementToolFails(FakeCredentialsManagementTool):
     """A Fake CredentialsManagementTool that consistently fails."""
 
@@ -3338,6 +3345,22 @@
         token = self.ipc.main.event_q.pushed_events[0][1]["access_token"]
         self.assertEqual(token, expected_token)
 
+class IPCInterfaceEmptyCredentialsTestCase(IPCInterfaceTestCaseBase):
+    """Tests for the IPCInterface with empty credentials."""
+
+    cmt_class = EmptyCredentialsManagementTool
+
+    @defer.inlineCallbacks
+    def test_connect(self):
+        """Test the connect method fails."""
+        yield self.assertFailure(self.ipc.connect(), ipc.NoAccessToken)
+
+    @defer.inlineCallbacks
+    def test_connect_autoconnecting(self):
+        """Test the connect method when autoconnecting fails."""
+        d = self.ipc.connect(autoconnecting=True)
+        yield self.assertFailure(d, ipc.NoAccessToken)
+
 
 class IPCInterfaceFailingTestCase(IPCInterfaceTestCaseBase):
     """Tests for the IPCInterface on a rainy day."""

=== modified file 'ubuntuone/platform/windows/credentials.py'
--- ubuntuone/platform/windows/credentials.py	2011-07-04 15:02:47 +0000
+++ ubuntuone/platform/windows/credentials.py	2011-07-20 15:02:53 +0000
@@ -72,34 +72,34 @@
     def find_credentials(self, reply_handler=NO_OP, error_handler=NO_OP):
         """Ask the Ubuntu One credentials."""
         d = self.sso_proxy.find_credentials(APP_NAME, {})
-        d.addCallbacks(lambda _:reply_handler(), error_handler)
+        d.addCallbacks(lambda _: reply_handler(), error_handler)
 
     def clear_credentials(self, reply_handler=NO_OP, error_handler=NO_OP):
         """Clear the Ubuntu One credentials."""
-        self.sso_proxy.clear_credentials(APP_NAME, {},
-            reply_handler=reply_handler, error_handler=error_handler)
+        d = self.sso_proxy.clear_credentials(APP_NAME, {})
+        d.addCallbacks(lambda _: reply_handler(), error_handler)
 
     def store_credentials(self, credentials,
                           reply_handler=NO_OP, error_handler=NO_OP):
         """Store the token for Ubuntu One application."""
-        self.sso_proxy.store_credentials(APP_NAME, credentials,
-            reply_handler=reply_handler, error_handler=error_handler)
+        d = self.sso_proxy.store_credentials(APP_NAME, credentials)
+        d.addCallbacks(lambda _: reply_handler(), error_handler)
 
     def register(self, args, reply_handler=NO_OP, error_handler=NO_OP):
         """Get credentials if found else prompt to register to Ubuntu One."""
         params = {HELP_TEXT_KEY: DESCRIPTION, TC_URL_KEY: TC_URL,
                   PING_URL_KEY: PING_URL}
         params.update(args)
-        self.sso_proxy.register(APP_NAME, params,
-            reply_handler=reply_handler, error_handler=error_handler)
+        d = self.sso_proxy.register(APP_NAME, params)
+        d.addCallbacks(lambda _: reply_handler(), error_handler)
 
     def login(self, args, reply_handler=NO_OP, error_handler=NO_OP):
         """Get credentials if found else prompt to login to Ubuntu One."""
         params = {HELP_TEXT_KEY: DESCRIPTION, TC_URL_KEY: TC_URL,
                   PING_URL_KEY: PING_URL}
         params.update(args)
-        self.sso_proxy.login(APP_NAME, params,
-            reply_handler=reply_handler, error_handler=error_handler)
+        d = self.sso_proxy.login(APP_NAME, params)
+        d.addCallbacks(lambda _: reply_handler(), error_handler)
 
     def register_to_credentials_stored(self, callback):
         """Register to the CredentialsStored dbus signal."""

=== modified file 'ubuntuone/platform/windows/ipc.py'
--- ubuntuone/platform/windows/ipc.py	2011-07-19 17:29:51 +0000
+++ ubuntuone/platform/windows/ipc.py	2011-07-20 15:02:53 +0000
@@ -1102,23 +1102,23 @@
                 token = yield self._request_token(
                                             autoconnecting=autoconnecting)
             except Exception, e:
+                logger.exception('failure while getting the token')
                 raise NoAccessToken(e)
 
+            if not token:
+                raise NoAccessToken("got empty credentials.")
+
         self.main.event_q.push('SYS_USER_CONNECT', access_token=token)
 
     def _request_token(self, autoconnecting):
         """Request to SSO auth service to fetch the token."""
         # call ubuntu sso
-        try:
-            management = CredentialsManagementTool()
-            # return the deferred, since we are no longer using signals
-            if autoconnecting:
-                return management.find_credentials()
-            else:
-                return management.register({'window_id': '0'})  # no window ID
-        except:
-            logger.exception('failure while getting the token')
-            raise
+        management = CredentialsManagementTool()
+        # return the deferred, since we are no longer using signals
+        if autoconnecting:
+            return management.find_credentials()
+        else:
+            return management.register(window_id=0)  # no window ID
 
     def disconnect(self):
         """ Push the SYS_USER_DISCONNECT event. """

=== modified file 'ubuntuone/platform/windows/tools.py'
--- ubuntuone/platform/windows/tools.py	2011-07-15 17:03:13 +0000
+++ ubuntuone/platform/windows/tools.py	2011-07-20 15:02:53 +0000
@@ -38,7 +38,8 @@
     """Check if there is a syncdaemon instance running."""
     #TODO: Do not start two instances of this process
     #      https://launchpad.net/bugs/803672
-    return False
+    #NOTE: Changed to True so SD can be restarted while this bug is fixed
+    return True
 
 
 class SyncDaemonTool(object):