Merge lp:~mikemc/ubuntuone-control-panel/fix-credentials-race into lp:ubuntuone-control-panel

[Mike McCracken]

- Fix race in backend get_credentials call. (LP: #1042860)

- Fix race in backend get_credentials call. (LP: #1042860)

[Manuel de la Peña (mandel)]

I wonder why we did not see this before..

review: Approve

[Roberto Alsina (ralsina)]

review: Approve

=== modified file 'ubuntuone/controlpanel/backend.py'
--- ubuntuone/controlpanel/backend.py	2012-08-27 15:11:37 +0000
+++ ubuntuone/controlpanel/backend.py	2012-08-30 00:24:17 +0000
@@ -23,7 +23,9 @@
 from collections import defaultdict
 from functools import wraps
 
-from twisted.internet.defer import inlineCallbacks, returnValue
+from twisted.internet.defer import (inlineCallbacks,
+                                    returnValue,
+                                    DeferredLock)
 from ubuntuone.platform.credentials import CredentialsManagementTool
 
 from ubuntuone.controlpanel import sd_client, replication_client
@@ -124,6 +126,8 @@
         UPLOAD_KEY: -1,  # no limit
     }
 
+    credentials_lock = DeferredLock()
+
     def __init__(self, shutdown_func=None):
         """Initialize the web_client."""
         self._status_changed_handler = None
@@ -323,8 +327,12 @@
     @inlineCallbacks
     def get_credentials(self):
         """Find credentials."""
-        if not self._credentials:
-            self._credentials = yield self.login_client.find_credentials()
+        yield self.credentials_lock.acquire()
+        try:
+            if not self._credentials:
+                self._credentials = yield self.login_client.find_credentials()
+        finally:
+            self.credentials_lock.release()
         returnValue(self._credentials)
 
     @inlineCallbacks

=== modified file 'ubuntuone/controlpanel/tests/test_backend.py'
--- ubuntuone/controlpanel/tests/test_backend.py	2012-08-27 15:00:01 +0000
+++ ubuntuone/controlpanel/tests/test_backend.py	2012-08-30 00:24:17 +0000
@@ -524,6 +524,32 @@
         self.assertEqual(self.be.login_client._called['find_credentials'], 1)
 
     @inlineCallbacks
+    def test_credentials_are_cached_slow(self):
+        """The credentials are still cached when find_credentials is slow."""
+        dq = defer.DeferredQueue()
+
+        @inlineCallbacks
+        def delayed_find_creds():
+            """Function with externally controlled return time."""
+            fake_creds = yield dq.get()
+            defer.returnValue(fake_creds)
+
+        self.patch(self.be.login_client, 'find_credentials',
+                   delayed_find_creds)
+
+        # Ensure that the second call to get_credentials occurs before
+        # the first one gets a result from find_credentials by leaving
+        # the queue empty until both are called:
+
+        d1 = self.be.get_credentials()
+        d2 = self.be.get_credentials()
+        dq.put("Blizzard")
+        dq.put("Dilly Bar")
+        creds1 = yield d1
+        creds2 = yield d2
+        self.assertEqual(creds1, creds2)
+
+    @inlineCallbacks
     def test_clear_credentials(self):
         """The credentials can be cleared."""
         # ensure we have creds