Merge lp:~nataliabidart/ubuntuone-client/login-email-password-for-everyone into lp:ubuntuone-client

[Natalia Bidart]

- CredentialsManagementTool provides login_email_password in both platforms (LP: #831043).
- Filtering signals by appname in credentials windows side (LP: #818190).

You can test this on both linux and windows by running:

PYTHONPATH=. contrib/login_email_password.py

You will be prompted by email and password, and the result will be printed in the terminal.
In windows, please be sure to have latest ussoc service running and in the PYTHONPATH.

[lp:~nataliabidart/ubuntuone-client/login-email-password-for-everyone]

1119. By Natalia Bidart on 2011-08-24

Typo.

[Alejandro J. Cura (alecu)]

Very nice cleanup branch.

Some issues:

----

There's a latent issue in making the variable "done" an instance variable in CredentialsManagementTool, and using it through all the inlineCallbacks generators in this class.

The "done" should be different for each of this generators, since more than one can be used at once.

I strongly believe instance variables *are not* clean to use here, and that they should be left as variables local to the generator. And perhaps we can have callback_done and errback_done get the "done" deferred from each generator via functools.partial.

----

This docstring is wrong (looks duplicated from the one above):

    def CredentialsStored(self):
        """Signal thrown when the credentials were cleared."""

----

The empty line should be restored before class CredentialsError.

----

review: Needs Fixing (just eyeballed the code)

[lp:~nataliabidart/ubuntuone-client/login-email-password-for-everyone]

1120. By Natalia Bidart on 2011-08-25

Merged trunk in.

1121. By Natalia Bidart on 2011-08-25

Fixes from review.

[Manuel de la Peña (mandel)]

Looks good to me.

review: Approve

[lp:~nataliabidart/ubuntuone-client/login-email-password-for-everyone]

1122. By Natalia Bidart on 2011-08-26

Merged trunk in.

[Alejandro J. Cura (alecu)]

Looks great!

review: Approve (ran tests)

=== modified file 'contrib/login_email_password.py' (properties changed: -x to +x)
--- contrib/login_email_password.py	2011-07-29 17:57:25 +0000
+++ contrib/login_email_password.py	2011-08-26 17:23:37 +0000
@@ -16,6 +16,12 @@
 # with this program.  If not, see <http://www.gnu.org/licenses/>.
 """Script that shows the qt gui."""
 
+import sys
+
+if sys.platform != 'win32':
+    from twisted.internet import glib2reactor
+    glib2reactor.install()
+
 # pylint: disable=F0401, E1101
 from twisted.internet import reactor
 from twisted.internet.defer import inlineCallbacks
@@ -23,18 +29,23 @@
 
 
 @inlineCallbacks
-def main():
+def main(email='[e-mail]', password='whatever'):
     """Perform a client request to be logged in."""
     credtool = CredentialsManagementTool()
-
-    creds = yield credtool.login_email_password(
-          email='[e-mail]',
-          password='whatever',
-    )
-    print "creds found", creds
+    print 'Trying to get credentials for email:', email
+    try:
+        creds = yield credtool.login_email_password(email=email,
+                                                    password=password)
+        print "creds found!", creds
+    except Exception, e:
+        print "creds error!", e
     reactor.stop()
 
 
 if __name__ == '__main__':
-    main()
+    print 'Enter email:'
+    email = raw_input()
+    print 'Enter password:'
+    password = raw_input()
+    main(email=email, password=password)
     reactor.run()

=== modified file 'tests/platform/linux/test_credentials.py'
--- tests/platform/linux/test_credentials.py	2011-08-16 13:44:20 +0000
+++ tests/platform/linux/test_credentials.py	2011-08-26 17:23:37 +0000
@@ -885,12 +885,9 @@
         """Find credentials and error."""
         FakedSSOService.error_dict = {'failure': 'really'}
 
-        try:
-            yield self.client.find_credentials()
-        except CredentialsError, e:
-            self.assertEqual(e[0], FakedSSOService.error_dict)
-        else:
-            self.fail('Must not succeed!')
+        e = yield self.assertFailure(self.client.find_credentials(),
+                                     CredentialsError)
+        self.assertEqual(e[0], FakedSSOService.error_dict)
 
     @inlineCallbacks
     def test_clear_credentials(self):
@@ -907,12 +904,9 @@
         """Clear credentials and error."""
         FakedSSOService.error_dict = {'failure': 'really'}
 
-        try:
-            yield self.client.clear_credentials()
-        except CredentialsError, e:
-            self.assertEqual(e[0], FakedSSOService.error_dict)
-        else:
-            self.fail('Must not succeed!')
+        e = yield self.assertFailure(self.client.clear_credentials(),
+                                     CredentialsError)
+        self.assertEqual(e[0], FakedSSOService.error_dict)
 
     @inlineCallbacks
     def test_store_credentials(self):
@@ -929,12 +923,9 @@
         """Store credentials and error."""
         FakedSSOService.error_dict = {'failure': 'really'}
 
-        try:
-            yield self.client.store_credentials({'test': 'me'})
-        except CredentialsError, e:
-            self.assertEqual(e[0], FakedSSOService.error_dict)
-        else:
-            self.fail('Must not succeed!')
+        e = yield self.assertFailure(self.client.store_credentials({'0': '1'}),
+                                     CredentialsError)
+        self.assertEqual(e[0], FakedSSOService.error_dict)
 
     @inlineCallbacks
     def test_register(self):
@@ -956,12 +947,9 @@
         """Register and error."""
         FakedSSOService.error_dict = {'failure': 'really'}
 
-        try:
-            yield self.client.register()
-        except CredentialsError, e:
-            self.assertEqual(e[0], FakedSSOService.error_dict)
-        else:
-            self.fail('Must not succeed!')
+        e = yield self.assertFailure(self.client.register(),
+                                     CredentialsError)
+        self.assertEqual(e[0], FakedSSOService.error_dict)
 
     @inlineCallbacks
     def test_login(self):
@@ -983,9 +971,6 @@
         """Login and error."""
         FakedSSOService.error_dict = {'failure': 'really'}
 
-        try:
-            yield self.client.login()
-        except CredentialsError, e:
-            self.assertEqual(e[0], FakedSSOService.error_dict)
-        else:
-            self.fail('Must not succeed!')
+        e = yield self.assertFailure(self.client.login(),
+                                     CredentialsError)
+        self.assertEqual(e[0], FakedSSOService.error_dict)

=== modified file 'tests/platform/test_credentials.py'
--- tests/platform/test_credentials.py	2011-08-18 13:13:37 +0000
+++ tests/platform/test_credentials.py	2011-08-26 17:23:37 +0000
@@ -22,127 +22,407 @@
 import urllib
 import urlparse
 
-from mocker import MockerTestCase, MATCH
+from collections import defaultdict
+from functools import wraps
+
 from twisted.internet import defer
 from twisted.trial.unittest import TestCase
 
 from ubuntuone import clientdefs
 from ubuntuone.platform.credentials import (
     BASE_PING_URL,
+    CredentialsError,
     CredentialsManagementTool,
+    NO_OP,
     PING_URL,
     platform_data,
 )
-
-
-class TestCredentialsManagementTool(MockerTestCase):
-    """Test the credentials management tool mocking the proxy."""
-
-    def setUp(self):
-        """Set the diff tests."""
-        super(TestCredentialsManagementTool, self).setUp()
-        self.get_cred_proxy = self.mocker.mock()
-        self.proxy = self.mocker.mock()
-        self.management_tool = CredentialsManagementTool()
-        self.management_tool.get_creds_proxy = self.get_cred_proxy
-
-    def test_find_credentials(self):
-        """Test that we do ask for the credentials correctly."""
-        signals = ['creds_found', 'creds_not_found', 'creds_erro']
-        self.get_cred_proxy()
-        self.mocker.result(self.proxy)
-        self.proxy.register_to_credentials_found(MATCH(callable))
-        self.mocker.result(signals[0])
-        self.proxy.register_to_credentials_not_found(MATCH(callable))
-        self.mocker.result(signals[1])
-        self.proxy.register_to_credentials_error(MATCH(callable))
-        self.mocker.result(signals[2])
-        self.proxy.find_credentials(reply_handler=MATCH(callable),
-                                    error_handler=MATCH(callable))
-        self.mocker.replay()
-        return self.management_tool.find_credentials()
-
-    def test_clear_credentials(self):
-        """Test that we clear the credentials correctly."""
-        signals = ['creds_cleared', 'creds_error']
-        self.get_cred_proxy()
-        self.mocker.result(self.proxy)
-        self.proxy.register_to_credentials_cleared(MATCH(callable))
-        self.mocker.result(signals[0])
-        self.proxy.register_to_credentials_error(MATCH(callable))
-        self.mocker.result(signals[1])
-        self.proxy.clear_credentials(reply_handler=MATCH(callable),
-                                     error_handler=MATCH(callable))
-        self.mocker.replay()
-        return self.management_tool.clear_credentials()
-
-    def test_store_credentials(self):
-        """Test that we store the credentials correctly."""
-        token = 'token'
-        signals = ['creds_stored','creds_error']
-        self.get_cred_proxy()
-        self.mocker.result(self.proxy)
-        self.proxy.register_to_credentials_stored(MATCH(callable))
-        self.mocker.result(signals[0])
-        self.proxy.register_to_credentials_error(MATCH(callable))
-        self.mocker.result(signals[1])
-        self.proxy.store_credentials(token, reply_handler=MATCH(callable),
-                                     error_handler=MATCH(callable))
-        self.mocker.replay()
-        return self.management_tool.store_credentials(token)
-
-    def test_register(self):
-        """Test that we correctly register."""
-        window_id = 10
-        window_id_dict = dict(window_id=str(window_id))
-        signals = ['creds_found', 'creds_denied', 'creds_error']
-        self.get_cred_proxy()
-        self.mocker.result(self.proxy)
-        self.proxy.register_to_credentials_found(MATCH(callable))
-        self.mocker.result(signals[0])
-        self.proxy.register_to_authorization_denied(MATCH(callable))
-        self.mocker.result(signals[1])
-        self.proxy.register_to_credentials_error(MATCH(callable))
-        self.mocker.result(signals[2])
-        self.proxy.register(window_id_dict, reply_handler=MATCH(callable),
-                            error_handler=MATCH(callable))
-        self.mocker.replay()
-        return self.management_tool.register(window_id=window_id)
-
-    def test_login_email_password(self):
-        """Test that we login correctly."""
-        email = 'email'
-        password = 'password'
-        arg_dict = dict(email=email, password=password)
-        signals = ['creds_found', 'creds_denied', 'creds_error']
-        self.get_cred_proxy()
-        self.mocker.result(self.proxy)
-        self.proxy.register_to_credentials_found(MATCH(callable))
-        self.mocker.result(signals[0])
-        self.proxy.register_to_credentials_error(MATCH(callable))
-        self.mocker.result(signals[1])
-        self.proxy.login_email_password(arg_dict, reply_handler=MATCH(callable),
-                            error_handler=MATCH(callable))
-        self.mocker.replay()
-        return self.management_tool.login_email_password(email=email,
-            password=password)
-
-    def test_cleanup_but_no_remove(self):
-        """The cleanup method works on handlers that have no remove method."""
-        self.management_tool._cleanup_signals = ["test signal 1", "test sig 2"]
-        self.management_tool.cleanup(None)
-
-
-class TestDeferredCredentialsManagementTool(TestCredentialsManagementTool):
-    """Test the credentials management tool mocking the proxy in a deferred."""
-
-    def setUp(self):
-        """Set the diff tests."""
-        super(TestDeferredCredentialsManagementTool, self).setUp()
-        real_get_cred_proxy = self.mocker.mock()
-        self.get_cred_proxy = lambda: defer.succeed(real_get_cred_proxy())
-        self.management_tool = CredentialsManagementTool()
-        self.management_tool.get_creds_proxy = self.get_cred_proxy
+from contrib.testing.testcase import FAKED_CREDENTIALS
+
+
+class FakedSignal(object):
+    """A faked signal."""
+
+    def __init__(self, name, callback):
+        self.name = name
+        self.callback = callback
+        self.removed = False
+        self.remove = lambda: setattr(self, 'removed', True)
+
+
+class FakedProxy(object):
+    """Fake a CredentialsManagement proxy."""
+
+    error_dict = None
+    error_handler = None
+
+    def __init__(self, *args, **kwargs):
+        self._credentials = None
+        self._args = None
+        self._kwargs = None
+        self._signals = []
+        self._receivers = defaultdict(list)
+        self._called = defaultdict(list)
+
+    def connect_to_signal(self, signal_name, callback):
+        """Keep track of connected signals."""
+        self._receivers[signal_name].append(callback)
+        result = FakedSignal(signal_name, callback)
+        self._signals.append(result)
+        return result
+
+    def maybe_emit_error(f):
+        """Decorator to fake a CredentialsError signal."""
+
+        @wraps(f)
+        def inner(self, *args, **kwargs):
+            """Fake a CredentialsError signal."""
+            if self.error_dict is None and self.error_handler is None:
+                f(self, *args, **kwargs)
+
+            reply_handler = kwargs.pop('reply_handler', NO_OP)
+            error_handler = kwargs.pop('error_handler', NO_OP)
+            if self.error_handler is not None:
+                error_handler(self.error_handler)
+            else:
+                reply_handler()
+
+            if self.error_dict is not None:
+                self.CredentialsError(self.error_dict)
+
+        return inner
+
+    def record_call(f):
+        """Decorator to record calls to 'f'."""
+
+        @wraps(f)
+        def inner(self, *a, **kw):
+            """Record the call to 'f' and call it."""
+            self._called[f.__name__].append((a, kw))
+            return f(self, *a, **kw)
+
+        return inner
+
+    def emit_signal(f):
+        """Decorator to emit a signal."""
+
+        @wraps(f)
+        def inner(self, *args, **kwargs):
+            """Emit the signal."""
+            for cb in self._receivers[f.__name__]:
+                cb(*args, **kwargs)
+
+        return inner
+
+    @emit_signal
+    def AuthorizationDenied(self):
+        """Signal thrown when the user denies the authorization."""
+
+    @emit_signal
+    def CredentialsFound(self, credentials):
+        """Signal thrown when the credentials are found."""
+
+    @emit_signal
+    def CredentialsNotFound(self):
+        """Signal thrown when the credentials are not found."""
+
+    @emit_signal
+    def CredentialsCleared(self):
+        """Signal thrown when the credentials were cleared."""
+
+    @emit_signal
+    def CredentialsStored(self):
+        """Signal thrown when the credentials were stored."""
+
+    @emit_signal
+    def CredentialsError(self, error_dict):
+        """Signal thrown when there is a problem getting the credentials."""
+
+    @record_call
+    @maybe_emit_error
+    def find_credentials(self, reply_handler=NO_OP, error_handler=NO_OP):
+        """Look for the credentials for an application."""
+        if self._credentials is not None:
+            self.CredentialsFound(self._credentials)
+        else:
+            self.CredentialsNotFound()
+
+    @record_call
+    @maybe_emit_error
+    def clear_credentials(self, reply_handler=NO_OP, error_handler=NO_OP):
+        """Clear the credentials for an application."""
+        self._credentials = None
+        self.CredentialsCleared()
+
+    @record_call
+    @maybe_emit_error
+    def store_credentials(self, credentials,
+                          reply_handler=NO_OP, error_handler=NO_OP):
+        """Store the token for an application."""
+        self._credentials = credentials
+        self.CredentialsStored()
+
+    @record_call
+    @maybe_emit_error
+    def register(self, dict_arg,
+                 reply_handler=NO_OP, error_handler=NO_OP):
+        """Get credentials if found else prompt GUI to register."""
+        creds = self._credentials
+        if creds is not None and len(creds) > 0:
+            self.CredentialsFound(creds)
+        elif creds == {}:
+            # fake an AuthorizationDenied
+            self.AuthorizationDenied()
+        elif creds is None:
+            # fake the adding of the credentials
+            self._credentials = FAKED_CREDENTIALS
+            self.CredentialsFound(FAKED_CREDENTIALS)
+
+    @record_call
+    @maybe_emit_error
+    def login(self, dict_arg,
+              reply_handler=NO_OP, error_handler=NO_OP):
+        """Get credentials if found else prompt GUI to login."""
+        self.register(dict_arg, reply_handler, error_handler)
+
+    @record_call
+    @maybe_emit_error
+    def login_email_password(self, dict_arg,
+                             reply_handler=NO_OP, error_handler=NO_OP):
+        """Fake login_email_password."""
+        self.register(dict_arg, reply_handler, error_handler)
+
+
+class FakedPlatformSource(object):
+    """Faked the platform source."""
+
+    def get_creds_proxy(self):
+        """Return a new faked proxy every time, so we can test proxy caching."""
+        return FakedProxy()
+
+
+class CredentialsManagementToolTestCase(TestCase):
+    """Test case for the object that manages Ubuntu One credentials."""
+
+    timeout = 3
+    error_dict = None
+    error_handler = None
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        yield super(CredentialsManagementToolTestCase, self).setUp()
+        self.cred_tool = CredentialsManagementTool()
+        self.patch(self.cred_tool, 'get_platform_source', FakedPlatformSource)
+        self.proxy = yield self.cred_tool.get_creds_proxy()
+        self.proxy.error_dict = self.error_dict
+        self.proxy.error_handler = self.error_handler
+
+        self.window_id_arg = {'window_id': '803'}
+        self.email_password_arg = {'email': '[e-mail]', 'password': 'yadda'}
+
+    @defer.inlineCallbacks
+    def test_proxy_is_reused(self):
+        """The inner proxy is re-used."""
+        proxy1 = yield self.cred_tool.get_creds_proxy()
+        proxy2 = yield self.cred_tool.get_creds_proxy()
+        self.assertIs(proxy1, proxy2)
+
+
+class ArgsTestCase(CredentialsManagementToolTestCase):
+    """Test case to check that proper arguments are passed to SSO backend."""
+
+    @defer.inlineCallbacks
+    def assert_method_called(self, method_name, *args, **kwargs):
+        """Test that 'method_name' was called once with 'args' and 'kwargs."""
+        self.assertIn(method_name, self.proxy._called)
+
+        calls = self.proxy._called[method_name]
+        msg = '%s must be called only once (got %s instead).'
+        self.assertEqual(len(calls), 1, msg % (method_name, len(calls)))
+
+        actual_args, actual_kwargs = calls[0]
+        self.assertEqual(args, actual_args)
+
+        reply_handler = actual_kwargs.pop('reply_handler', None)
+        self.assertIsNotNone(reply_handler, 'Must provide a reply_handler')
+        d = defer.Deferred()
+        reply_handler(deferred=d)
+        yield d
+        self.assertTrue(d.called)
+
+        error_handler = actual_kwargs.pop('error_handler', None)
+        self.assertIsNotNone(error_handler, 'Must provide a error_handler')
+        d = defer.Deferred()
+        error_handler(error='foo', deferred=d)
+        e = yield self.assertFailure(d, CredentialsError)
+        self.assertEqual(e.message, 'foo')
+
+        self.assertEqual(kwargs, actual_kwargs)
+
+    @defer.inlineCallbacks
+    def test_find_credentials(self):
+        """The find_credentials method calls proper method."""
+        yield self.cred_tool.find_credentials()
+
+        self.assert_method_called('find_credentials')
+
+    @defer.inlineCallbacks
+    def test_clear_credentials(self):
+        """The clear_credentials method calls proper method."""
+        yield self.cred_tool.clear_credentials()
+
+        self.assert_method_called('clear_credentials')
+
+    @defer.inlineCallbacks
+    def test_store_credentials(self):
+        """The store_credentials method calls proper method."""
+        yield self.cred_tool.store_credentials(FAKED_CREDENTIALS)
+
+        self.assert_method_called('store_credentials', FAKED_CREDENTIALS)
+
+    @defer.inlineCallbacks
+    def test_register(self):
+        """The register method calls proper method."""
+        yield self.cred_tool.register(**self.window_id_arg)
+
+        self.assert_method_called('register', self.window_id_arg)
+
+    @defer.inlineCallbacks
+    def test_login(self):
+        """The login method calls proper method."""
+        yield self.cred_tool.login(**self.window_id_arg)
+
+        self.assert_method_called('login', self.window_id_arg)
+
+    @defer.inlineCallbacks
+    def test_login_email_password(self):
+        """The login method calls proper method."""
+        yield self.cred_tool.login_email_password(**self.email_password_arg)
+
+        self.assert_method_called('login_email_password',
+                                  self.email_password_arg)
+
+
+class NoErrorWithCredsTestCase(CredentialsManagementToolTestCase):
+    """Test case when there was no error, and credentials are present."""
+
+    token = {'test': 'me'}
+
+    @defer.inlineCallbacks
+    def do_test(self, method, expected=None, **kwargs):
+        """Perform the test itself."""
+        if self.token is not None:
+            self.proxy.store_credentials(self.token)
+        else:
+            yield self.proxy.clear_credentials()
+        actual = yield method(**kwargs)
+        self.assertEqual(expected, actual)
+
+    @defer.inlineCallbacks
+    def test_find_credentials(self):
+        """The find_credentials method calls proper method."""
+        yield self.do_test(self.cred_tool.find_credentials, self.token)
+
+    @defer.inlineCallbacks
+    def test_clear_credentials(self):
+        """The clear_credentials method calls proper method."""
+        yield self.do_test(self.cred_tool.clear_credentials, None)
+
+    @defer.inlineCallbacks
+    def test_store_credentials(self):
+        """The store_credentials method calls proper method."""
+        yield self.do_test(self.cred_tool.store_credentials, None,
+                           token=FAKED_CREDENTIALS)
+
+    @defer.inlineCallbacks
+    def test_register(self):
+        """The register method calls proper method."""
+        yield self.do_test(self.cred_tool.register, self.token,
+                           **self.window_id_arg)
+
+    @defer.inlineCallbacks
+    def test_login(self):
+        """The login method calls proper method."""
+        yield self.do_test(self.cred_tool.login, self.token,
+                           **self.window_id_arg)
+
+    @defer.inlineCallbacks
+    def test_login_email_password(self):
+        """The login_email_password method calls proper method."""
+        yield self.do_test(self.cred_tool.login_email_password, self.token,
+                           **self.email_password_arg)
+
+
+class NoErrorNoCredsTestCase(NoErrorWithCredsTestCase):
+    """Test case when there was no error, and credentials are not present."""
+
+    token = None
+
+    @defer.inlineCallbacks
+    def test_find_credentials(self):
+        """The find_credentials method calls proper method."""
+        yield self.do_test(self.cred_tool.find_credentials, {})
+
+    @defer.inlineCallbacks
+    def test_register(self):
+        """The register method calls proper method."""
+        yield self.do_test(self.cred_tool.register, FAKED_CREDENTIALS,
+                           **self.window_id_arg)
+
+    @defer.inlineCallbacks
+    def test_register_authorization_denied(self):
+        """The register method calls proper method."""
+        self.token = {}
+        yield self.do_test(self.cred_tool.register, None,
+                           **self.window_id_arg)
+
+    @defer.inlineCallbacks
+    def test_login(self):
+        """The login method calls proper method."""
+        yield self.do_test(self.cred_tool.login, FAKED_CREDENTIALS,
+                           **self.window_id_arg)
+
+    @defer.inlineCallbacks
+    def test_login_authorization_denied(self):
+        """The login method calls proper method."""
+        self.token = {}
+        yield self.do_test(self.cred_tool.login, None, **self.window_id_arg)
+
+    @defer.inlineCallbacks
+    def test_login_email_password(self):
+        """The login_email_password method calls proper method."""
+        yield self.do_test(self.cred_tool.login_email_password,
+                           FAKED_CREDENTIALS, **self.email_password_arg)
+
+
+class WithCredentialsErrorTestCase(NoErrorNoCredsTestCase):
+    """Test case when there was a CredentialsError sent from the proxy."""
+
+    error_dict = expected = {'error_type': 'Test'}
+
+    @defer.inlineCallbacks
+    def do_test(self, method, expected=None, **kwargs):
+        """Perform the test itself."""
+        exc = yield self.assertFailure(method(**kwargs), Exception)
+        self.assertIsInstance(exc, CredentialsError)
+        self.assertEqual(self.expected, exc.args[0])
+
+
+class WithErrorHandlerCalledTestCase(WithCredentialsErrorTestCase):
+    """Test case when the error handler was called."""
+
+    error_dict = None
+    error_handler = expected = {'error_type': 'Another Test'}
+
+
+class SignalsRemovedTestCase(NoErrorWithCredsTestCase):
+
+    @defer.inlineCallbacks
+    def do_test(self, method, expected=None, **kwargs):
+        """Perform the test itself."""
+        yield method(**kwargs)
+        for signal in self.proxy._signals:
+            self.assertTrue(signal.removed)
 
 
 class PingURLPlatformDetails(TestCase):

=== modified file 'tests/platform/windows/test_credentials.py'
--- tests/platform/windows/test_credentials.py	2011-08-16 15:43:52 +0000
+++ tests/platform/windows/test_credentials.py	2011-08-26 17:23:37 +0000
@@ -20,6 +20,8 @@
 from twisted.internet import defer
 from twisted.trial.unittest import TestCase
 
+from contrib.testing.testcase import FAKED_CREDENTIALS
+from ubuntuone.platform.credentials import APP_NAME
 from ubuntuone.platform.credentials.windows import (
     CredentialsManagement,
     RemovableSignal,
@@ -27,9 +29,7 @@
 
 TEST_APP_NAME = "test application"
 TEST_ERROR_DICT = {}
-TEST_CREDENTIALS = {
-    "token": "1234token",
-}
+TEST_CREDENTIALS = FAKED_CREDENTIALS
 
 
 class FakeSSOProxy(object):
@@ -77,11 +77,15 @@
         return defer.succeed(None)
 
     def register(self, app_name, options):
-        """Store the U1 credentials."""
+        """Register."""
         return defer.succeed(None)
 
     def login(self, app_name, options):
-        """Store the U1 credentials."""
+        """Login."""
+        return defer.succeed(None)
+
+    def login_email_password(self, app_name, options):
+        """Login using email and password."""
         return defer.succeed(None)
 
 
@@ -92,13 +96,27 @@
         """Initialize this fake instance."""
         self.proxy = FakeSSOProxy()
 
+    def test_creation(self):
+        """When creating, bind properly to self.proxy."""
+        rs = RemovableSignal(self.proxy, "test", lambda *a: None)
+        self.assertIs(self.proxy.test, rs)
+
     def test_dunder_callable(self):
         """__call__ works as expected."""
         sample_store = []
-        test_cb = lambda app_name, creds: sample_store.append(creds)
-        rs = RemovableSignal(self.proxy, "on_credentials_found_cb", test_cb)
-        rs(TEST_APP_NAME, TEST_CREDENTIALS)
-        self.assertEqual(sample_store[0], TEST_CREDENTIALS)
+        expected = object()
+        test_cb = lambda res: sample_store.append(res)
+        rs = RemovableSignal(self.proxy, "on_credentials_found_cb", test_cb)
+        rs(APP_NAME, expected)
+        self.assertEqual(sample_store, [expected])
+
+    def test_dunder_filters_other_apps(self):
+        """__call__ filters by app_name."""
+        sample_store = []
+        test_cb = lambda res: sample_store.append(res)
+        rs = RemovableSignal(self.proxy, "on_credentials_found_cb", test_cb)
+        rs('other app name', object())
+        self.assertEqual(sample_store, [])
 
     def test_remove(self):
         """The signal has a .remove that removes the callback."""
@@ -114,12 +132,22 @@
     """Tests for CredentialsManagement."""
 
     timeout = 5
+    app_name = APP_NAME
 
     def setUp(self):
         """Initialize these tests."""
+        self._called = False
         self.proxy = FakeSSOProxy()
         self.cm = CredentialsManagement(self.proxy)
 
+    def _set_called(self, *args, **kwargs):
+        """Helper to keep track calls."""
+        self._called = (args, kwargs)
+
+    def assert_callback_called(self, expected):
+        """Test that _called helper holds 'expected'."""
+        self.assertEqual(self._called, expected)
+
     def test_find_credentials(self):
         """Test the find_credentials method."""
         d = defer.Deferred()
@@ -161,17 +189,50 @@
         self.cm.login({}, reply_handler=ok, error_handler=error)
         return d
 
+    def test_login_email_password(self):
+        """Test the login_email_password method."""
+        d = defer.Deferred()
+        ok = lambda: d.callback("ok")
+        error = lambda *args: d.errback(args)
+        self.cm.login_email_password({'email': 'foo', 'password': 'bar'},
+                                     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)
-        other_cb = self.cm.register_to_credentials_found(cb)
-        other_cb(TEST_APP_NAME, TEST_CREDENTIALS)
+        signal = self.cm.register_to_credentials_found(self._set_called)
+        signal(self.app_name, TEST_CREDENTIALS)
+        self.assert_callback_called(((TEST_CREDENTIALS,), {}))
 
     def test_register_to_credentials_not_found(self):
         """Test the register_to_credentials_not_found method."""
-        cb = lambda: None
-        other_cb = self.cm.register_to_credentials_not_found(cb)
-        other_cb(TEST_APP_NAME)
+        signal = self.cm.register_to_credentials_not_found(self._set_called)
+        signal(self.app_name)
+        self.assert_callback_called(((), {}))
+
+    def test_register_to_credentials_stored(self):
+        """Test the register_to_credentials_stored method."""
+        signal = self.cm.register_to_credentials_stored(self._set_called)
+        signal(self.app_name)
+        self.assert_callback_called(((), {}))
+
+    def test_register_to_credentials_cleared(self):
+        """Test the register_to_credentials_cleared method."""
+        signal = self.cm.register_to_credentials_cleared(self._set_called)
+        signal(self.app_name)
+        self.assert_callback_called(((), {}))
+
+    def test_register_to_credentials_error(self):
+        """Test the register_to_credentials_error method."""
+        signal = self.cm.register_to_credentials_error(self._set_called)
+        signal(self.app_name)
+        self.assert_callback_called(((), {}))
+
+    def test_register_to_authorization_denied(self):
+        """Test the register_to_authorization_denied method."""
+        signal = self.cm.register_to_authorization_denied(self._set_called)
+        signal(self.app_name, TEST_ERROR_DICT)
+        self.assert_callback_called(((TEST_ERROR_DICT,), {}))
 
     def _verify_not_called_twice(self, signal_name, *args):
         """Test that the callback is not called twice."""
@@ -199,7 +260,7 @@
 
     def test_not_called_twice_credentials_found(self):
         """Test that on_credentials_found is not called twice."""
-        self._verify_not_called_twice("credentials_found", TEST_APP_NAME,
+        self._verify_not_called_twice("credentials_found", self.app_name,
                                       TEST_CREDENTIALS)
 
     def test_not_called_twice_credentials_not_found(self):
@@ -213,3 +274,21 @@
     def test_not_called_twice_credentials_error(self):
         """Test that on_credentials_error is not called twice."""
         self._verify_not_called_twice("credentials_error", TEST_ERROR_DICT)
+
+    def test_connect_to_signal(self):
+        """The connect_to_signal method is correct."""
+        for signal_name in self.cm._SIGNAL_TO_CALLBACK_MAPPING:
+            match = self.cm.connect_to_signal(signal_name, self._set_called)
+            expected = object()
+            match(APP_NAME, expected)
+            self.assertEqual(self._called, ((expected,), {}))
+
+
+class CredentialsManagementOtherAppNameTestCase(CredentialsManagementTestCase):
+    """Tests for CredentialsManagement when the app name differs."""
+
+    app_name = 'other app name'
+
+    def assert_callback_called(self, expected):
+        """Test that _called helper does not hold 'expected'."""
+        self.assertEqual(self._called, False)

=== modified file 'ubuntuone/platform/credentials/__init__.py'
--- ubuntuone/platform/credentials/__init__.py	2011-08-18 15:30:53 +0000
+++ ubuntuone/platform/credentials/__init__.py	2011-08-26 17:23:37 +0000
@@ -25,6 +25,8 @@
 import urllib
 import sys
 
+from functools import partial
+
 from twisted.internet import defer
 
 from ubuntuone import clientdefs
@@ -83,6 +85,15 @@
 
     def __init__(self):
         self._cleanup_signals = []
+        self._proxy = None
+
+    def callback(self, result, deferred):
+        """Fire 'deferred' with success, sending 'result' as result."""
+        deferred.callback(result)
+
+    def errback(self, error, deferred):
+        """Fire 'deferred' with error sending a CredentialsError."""
+        deferred.errback(CredentialsError(error))
 
     def cleanup(self, _):
         """Disconnect all the DBus signals."""
@@ -93,16 +104,23 @@
 
         return _
 
-    def get_creds_proxy(self):
-        """Call the get_proxy_fn, and perhaps wrap the result in a defer."""
+    def get_platform_source(self):
+        """Platform-specific source."""
         if sys.platform == 'win32':
             from ubuntuone.platform.credentials import windows
             source = windows
         else:
             from ubuntuone.platform.credentials import linux
             source = linux
-        result = defer.maybeDeferred(source.get_creds_proxy)
-        return result
+        return source
+
+    @defer.inlineCallbacks
+    def get_creds_proxy(self):
+        """Call the platform-dependent get_creds_proxy caching the result."""
+        if self._proxy is None:
+            source = self.get_platform_source()
+            self._proxy = yield source.get_creds_proxy()
+        defer.returnValue(self._proxy)
 
     @log_call(logger.debug)
     @defer.inlineCallbacks
@@ -128,19 +146,21 @@
 
         proxy = yield self.get_creds_proxy()
 
-        sig = proxy.register_to_credentials_found(d.callback)
-        self._cleanup_signals.append(sig)
-
-        sig = proxy.register_to_credentials_not_found(lambda: d.callback({}))
-        self._cleanup_signals.append(sig)
-
-        sig = proxy.register_to_credentials_error(
-            lambda error_dict: d.errback(CredentialsError(error_dict)))
+        sig = proxy.connect_to_signal('CredentialsFound', d.callback)
+        self._cleanup_signals.append(sig)
+
+        sig = proxy.connect_to_signal('CredentialsNotFound',
+                    partial(self.callback, result={}, deferred=d))
+        self._cleanup_signals.append(sig)
+
+        sig = proxy.connect_to_signal('CredentialsError',
+                    partial(self.errback, deferred=d))
         self._cleanup_signals.append(sig)
 
         done = defer.Deferred()
-        proxy.find_credentials(reply_handler=lambda: done.callback(None),
-            error_handler=lambda *a: done.errback(a))
+        proxy.find_credentials(
+            reply_handler=partial(self.callback, result=None, deferred=done),
+            error_handler=partial(self.errback, deferred=done))
 
         yield done
 
@@ -161,16 +181,19 @@
         d.addBoth(self.cleanup)
 
         proxy = yield self.get_creds_proxy()
-        sig = proxy.register_to_credentials_cleared(lambda: d.callback(None))
+
+        sig = proxy.connect_to_signal('CredentialsCleared',
+                    partial(self.callback, result=None, deferred=d))
         self._cleanup_signals.append(sig)
 
-        sig = proxy.register_to_credentials_error(
-            lambda error_dict: d.errback(CredentialsError(error_dict)))
+        sig = proxy.connect_to_signal('CredentialsError',
+                    partial(self.errback, deferred=d))
         self._cleanup_signals.append(sig)
 
         done = defer.Deferred()
-        proxy.clear_credentials(reply_handler=lambda: done.callback(None),
-            error_handler=lambda *a: done.errback(a))
+        proxy.clear_credentials(
+            reply_handler=partial(self.callback, result=None, deferred=done),
+            error_handler=partial(self.errback, deferred=done))
 
         yield done
 
@@ -193,17 +216,19 @@
         d.addBoth(self.cleanup)
 
         proxy = yield self.get_creds_proxy()
-        sig = proxy.register_to_credentials_stored(lambda: d.callback(None))
+
+        sig = proxy.connect_to_signal('CredentialsStored',
+                    partial(self.callback, result=None, deferred=d))
         self._cleanup_signals.append(sig)
 
-        sig = proxy.register_to_credentials_error(
-            lambda error_dict: d.errback(CredentialsError(error_dict)))
+        sig = proxy.connect_to_signal('CredentialsError',
+                    partial(self.errback, deferred=d))
         self._cleanup_signals.append(sig)
 
         done = defer.Deferred()
         proxy.store_credentials(token,
-            reply_handler=lambda: done.callback(None),
-            error_handler=lambda *a: done.errback(a))
+            reply_handler=partial(self.callback, result=None, deferred=done),
+            error_handler=partial(self.errback, deferred=done))
 
         yield done
 
@@ -235,20 +260,21 @@
 
         proxy = yield self.get_creds_proxy()
 
-        sig = proxy.register_to_credentials_found(d.callback)
-        self._cleanup_signals.append(sig)
-
-        sig = proxy.register_to_authorization_denied(lambda: d.callback(None))
-        self._cleanup_signals.append(sig)
-
-        sig = proxy.register_to_credentials_error(
-            lambda error_dict: d.errback(CredentialsError(error_dict)))
+        sig = proxy.connect_to_signal('CredentialsFound', d.callback)
+        self._cleanup_signals.append(sig)
+
+        sig = proxy.connect_to_signal('AuthorizationDenied',
+                    partial(self.callback, result=None, deferred=d))
+        self._cleanup_signals.append(sig)
+
+        sig = proxy.connect_to_signal('CredentialsError',
+                    partial(self.errback, deferred=d))
         self._cleanup_signals.append(sig)
 
         done = defer.Deferred()
         proxy.register({'window_id': str(window_id)},
-            reply_handler=lambda: done.callback(None),
-            error_handler=lambda *a: done.errback(a))
+            reply_handler=partial(self.callback, result=None, deferred=done),
+            error_handler=partial(self.errback, deferred=done))
 
         yield done
 
@@ -282,20 +308,21 @@
 
         proxy = yield self.get_creds_proxy()
 
-        sig = proxy.register_to_credentials_found(d.callback)
-        self._cleanup_signals.append(sig)
-
-        sig = proxy.register_to_authorization_denied(lambda: d.callback(None))
-        self._cleanup_signals.append(sig)
-
-        sig = proxy.register_to_credentials_error(
-            lambda error_dict: d.errback(CredentialsError(error_dict)))
+        sig = proxy.connect_to_signal('CredentialsFound', d.callback)
+        self._cleanup_signals.append(sig)
+
+        sig = proxy.connect_to_signal('AuthorizationDenied',
+                    partial(self.callback, result=None, deferred=d))
+        self._cleanup_signals.append(sig)
+
+        sig = proxy.connect_to_signal('CredentialsError',
+                    partial(self.errback, deferred=d))
         self._cleanup_signals.append(sig)
 
         done = defer.Deferred()
         proxy.login({'window_id': str(window_id)},
-            reply_handler=lambda: done.callback(None),
-            error_handler=lambda *a: done.errback(a))
+            reply_handler=partial(self.callback, result=None, deferred=done),
+            error_handler=partial(self.errback, deferred=done))
 
         yield done
 
@@ -320,17 +347,17 @@
 
         proxy = yield self.get_creds_proxy()
 
-        sig = proxy.register_to_credentials_found(d.callback)
+        sig = proxy.connect_to_signal('CredentialsFound', d.callback)
         self._cleanup_signals.append(sig)
 
-        sig = proxy.register_to_credentials_error(
-            lambda x, error_dict: d.errback(CredentialsError(error_dict)))
+        sig = proxy.connect_to_signal('CredentialsError',
+                    partial(self.errback, deferred=d))
         self._cleanup_signals.append(sig)
 
         done = defer.Deferred()
         proxy.login_email_password({'email': email, 'password': password},
-            reply_handler=lambda: done.callback(None),
-            error_handler=lambda *a: done.errback(a))
+            reply_handler=partial(self.callback, result=None, deferred=done),
+            error_handler=partial(self.errback, deferred=done))
 
         yield done
 

=== modified file 'ubuntuone/platform/credentials/linux.py'
--- ubuntuone/platform/credentials/linux.py	2011-08-16 13:44:20 +0000
+++ ubuntuone/platform/credentials/linux.py	2011-08-26 17:23:37 +0000
@@ -233,75 +233,16 @@
             reply_handler=reply_handler, error_handler=error_handler)
 
 
-class CredentialsManagementProxy(object):
-    """Proxy that allows to work with the dbus api.
-
-    This proxy has been added to hide the exact details of
-    how the signals are connected. This allows the reuse of
-    the CredentialsManagementTool in diff platforms.
-    """
-
-    def __init__(self):
-        """Create a new instance."""
-        bus = dbus.SessionBus()
-        try:
-            obj = bus.get_object(DBUS_BUS_NAME,
-                                 DBUS_CREDENTIALS_PATH,
-                                 follow_name_owner_changes=True)
-            self.proxy = dbus.Interface(obj, DBUS_CREDENTIALS_IFACE)
-        except:
-            logger.exception('get_creds_proxy:')
-            raise
-
-    def find_credentials(self, *args, **kwargs):
-        """Relay the find_credentials method."""
-        return self.proxy.find_credentials(*args, **kwargs)
-
-    def clear_credentials(self, *args, **kwargs):
-        """Relay the clear_credentials method."""
-        return self.proxy.clear_credentials(*args, **kwargs)
-
-    def store_credentials(self, *args, **kwargs):
-        """Relay the store_credentials method."""
-        return self.proxy.store_credentials(*args, **kwargs)
-
-    def register(self, *args, **kwargs):
-        """Relay the register method."""
-        return self.proxy.register(*args, **kwargs)
-
-    def login(self, *args, **kwargs):
-        """Relay the login method."""
-        return self.proxy.login(*args, **kwargs)
-
-    def login_email_password(self, *args, **kwargs):
-        """Relay the login method."""
-        return self.proxy.login_email_password(*args, **kwargs)
-
-    def register_to_credentials_stored(self, callback):
-        """Register to the CredentialsStored dbus signal."""
-        return self.proxy.connect_to_signal('CredentialsStored', callback)
-
-    def register_to_credentials_cleared(self, callback):
-        """Register to the CredentialsCleared dbus signal."""
-        return self.proxy.connect_to_signal('CredentialsCleared', callback)
-
-    def register_to_credentials_found(self, callback):
-        """Register to the CredentialsFound dbus signal."""
-        return self.proxy.connect_to_signal('CredentialsFound', callback)
-
-    def register_to_credentials_not_found(self, callback):
-        """Register to the CredentialsFound dbus signal."""
-        return self.proxy.connect_to_signal('CredentialsNotFound', callback)
-
-    def register_to_authorization_denied(self, callback):
-        """Register to the AuthorizationDenied dbus signal."""
-        return self.proxy.connect_to_signal('AuthorizationDenied', callback)
-
-    def register_to_credentials_error(self, callback):
-        """Register to the CredentialsError dbus signal."""
-        return self.proxy.connect_to_signal('CredentialsError', callback)
-
-
 def get_creds_proxy():
     """Get the CredentialsManagement proxy."""
-    return CredentialsManagementProxy()
+    bus = dbus.SessionBus()
+    try:
+        obj = bus.get_object(DBUS_BUS_NAME,
+                             DBUS_CREDENTIALS_PATH,
+                             follow_name_owner_changes=True)
+        proxy = dbus.Interface(obj, DBUS_CREDENTIALS_IFACE)
+    except:
+        logger.exception('get_creds_proxy:')
+        raise
+
+    return proxy

=== modified file 'ubuntuone/platform/credentials/windows.py'
--- ubuntuone/platform/credentials/windows.py	2011-08-16 13:44:20 +0000
+++ ubuntuone/platform/credentials/windows.py	2011-08-26 17:23:37 +0000
@@ -30,6 +30,7 @@
 from ubuntuone.platform.credentials import (
     APP_NAME,
     DESCRIPTION,
+    logger,
     NO_OP,
     PING_URL,
     TC_URL,
@@ -44,13 +45,26 @@
         self.proxy = proxy
         self.signal_name = signal_name
         self.callback = callback
-        setattr(self.proxy, signal_name, callback)
+        setattr(self.proxy, signal_name, self)
 
     def __call__(self, *args, **kwargs):
         """Call this instance."""
-        func = getattr(self.proxy, self.signal_name, None)
-        if func is not None:
-            return func(*args, **kwargs)
+        app_name = args[0] if len(args) > 0 else None
+        logger.debug('Handling signal_name: %r, app_name: %r.',
+                     self.signal_name, app_name)
+
+        if app_name != APP_NAME:
+            # This fixed bug #818190: filter signals not related to APP_NAME
+            logger.info('Received %r but app_name %r does not match %r, ' \
+                        'exiting.', self.signal_name, app_name, APP_NAME)
+            return
+
+        if self.callback is not None:
+            # drop the app name, callers do not care about it
+            args = args[1:]
+            logger.debug('Calling %r with args %r and kwargs %r.',
+                         self.callback, args, kwargs)
+            return self.callback(*args, **kwargs)
 
     def remove(self):
         """Remove this signal."""
@@ -61,10 +75,25 @@
 class CredentialsManagement(object):
     """Object that manages Ubuntu One credentials."""
 
+    _SIGNAL_TO_CALLBACK_MAPPING = {
+        'AuthorizationDenied': 'on_authorization_denied_cb',
+        'CredentialsCleared': 'on_credentials_cleared_cb',
+        'CredentialsError': 'on_credentials_error_cb',
+        'CredentialsFound': 'on_credentials_found_cb',
+        'CredentialsNotFound': 'on_credentials_not_found_cb',
+        'CredentialsStored': 'on_credentials_stored_cb',
+    }
+
     def __init__(self, proxy, *args, **kwargs):
         super(CredentialsManagement, self).__init__(*args, **kwargs)
         self.sso_proxy = proxy
 
+    def connect_to_signal(self, signal_name, callback):
+        """Register 'callback' to be called when 'signal_name' is emitted."""
+        cb_name = self._SIGNAL_TO_CALLBACK_MAPPING[signal_name]
+        match = RemovableSignal(self.sso_proxy, cb_name, callback)
+        return match
+
     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, {})
@@ -97,7 +126,8 @@
         d = self.sso_proxy.login(APP_NAME, params)
         d.addCallbacks(lambda _: reply_handler(), error_handler)
 
-    def login_email_password(self, args, reply_handler=NO_OP, error_handler=NO_OP):
+    def login_email_password(self, args,
+                             reply_handler=NO_OP, error_handler=NO_OP):
         """Get credentials if found else login to Ubuntu One."""
         params = {PING_URL_KEY: PING_URL}
         params.update(args)
@@ -116,16 +146,13 @@
 
     def register_to_credentials_found(self, callback):
         """Register to the CredentialsFound dbus signal."""
-        # XXX: we are not filtering by app name (bug #818190)
-        pass_credentials = lambda app_name, creds: callback(creds)
         return RemovableSignal(self.sso_proxy, "on_credentials_found_cb",
-                               pass_credentials)
+                               callback)
 
     def register_to_credentials_not_found(self, callback):
         """Register to the CredentialsFound dbus signal."""
-        pass_empty = lambda *args: callback()
         return RemovableSignal(self.sso_proxy, "on_credentials_not_found_cb",
-                               pass_empty)
+                               callback)
 
     def register_to_authorization_denied(self, callback):
         """Register to the AuthorizationDenied dbus signal."""
@@ -142,6 +169,7 @@
 def get_creds_proxy():
     """Get the CredentialsManagement proxy."""
     client = UbuntuSSOClient()
-    client = yield client.connect()
+    yield client.connect()
     yield client.cred_management.register_to_signals()
-    defer.returnValue(CredentialsManagement(client.cred_management))
+    result = CredentialsManagement(client.cred_management)
+    defer.returnValue(result)