Merge lp:~free.ekanayaka/landscape-client/dont-reuse-connectors into lp:~landscape/landscape-client/trunk

[Free Ekanayaka]

The problem was that the connector object was being reused, meaning multiple connect() calls at the same time. This branch makes the UserManager and UserMonitor use one connector object per connection.

[Jamu Kakar (jkakar)]

[1]

     def _message_dispatch(self, message):

+ user_monitor_connector = RemoteUserMonitorConnector(
+ self.registry.reactor, self.registry.config)

The leading blank line here isn't necessary. Also, would you mind
adding a docstring to this message, please?

Looks good, +1!

review: Approve

[Kevin McDermott (bigkevmcd)]

Looks good to me, +1

Tested and works fine.

review: Approve

[Free Ekanayaka (free.ekanayaka)]

Thanks!

[1] fixed

[lp:~free.ekanayaka/landscape-client/dont-reuse-connectors]

285. By Free Ekanayaka on 2010-08-23

Add docstring (Jamu [1])

=== modified file 'landscape/manager/tests/test_usermanager.py'
--- landscape/manager/tests/test_usermanager.py	2010-04-26 15:19:55 +0000
+++ landscape/manager/tests/test_usermanager.py	2010-08-23 11:50:51 +0000
@@ -1,6 +1,7 @@
 import os
 
 from landscape.lib.persist import Persist
+from landscape.lib.twisted_util import gather_results
 from landscape.manager.plugin import SUCCEEDED, FAILED
 from landscape.monitor.usermonitor import UserMonitor
 from landscape.manager.usermanager import (
@@ -332,6 +333,40 @@
         result.addCallback(handle_callback)
         return result
 
+    def test_many_remove_user_events(self):
+        """
+        The L{UserManager} can handle multiple remove-user events at the same
+        time.
+        """
+        users = [("foo", "x", 1000, 1000, "Foo,,,,", "/home/foo", "/bin/zsh"),
+                 ("bar", "x", 1001, 1001, "Bar,,,,", "/home/bar", "/bin/zsh")]
+        self.setup_environment(users, [], None)
+
+        def handle_callback(ignored):
+            messages = self.broker_service.message_store.get_pending_messages()
+            # Ignore the message created by plugin.run.
+            self.assertMessages([messages[2], messages[1]],
+                                [{"type": "operation-result",
+                                  "status": SUCCEEDED,
+                                  "operation-id": 40, "timestamp": 0,
+                                  "result-text": "remove_user succeeded"},
+                                 {"type": "operation-result",
+                                  "status": SUCCEEDED,
+                                  "operation-id": 39, "timestamp": 0,
+                                  "result-text": "remove_user succeeded"}])
+
+
+        results = []
+        results.append(self.manager.dispatch_message({"username": "foo",
+                                                      "delete-home": True,
+                                                      "type": "remove-user",
+                                                      "operation-id": 39}))
+        results.append(self.manager.dispatch_message({"username": "bar",
+                                                      "delete-home": True,
+                                                      "type": "remove-user",
+                                                      "operation-id": 40}))
+        return gather_results(results).addCallback(handle_callback)
+
     def test_failing_remove_user_event(self):
         """
         When a C{remove-user} event is received, and the user doesn't exist, we

=== modified file 'landscape/manager/usermanager.py'
--- landscape/manager/usermanager.py	2010-05-03 13:08:02 +0000
+++ landscape/manager/usermanager.py	2010-08-23 11:50:51 +0000
@@ -42,8 +42,6 @@
         socket = os.path.join(self.registry.config.sockets_path,
                               self.name + ".sock")
         self._port = self.registry.reactor.listen_unix(socket, factory)
-        self._user_monitor_connector = RemoteUserMonitorConnector(
-            self.registry.reactor, self.registry.config)
 
         for message_type in self._message_types:
             self._registry.register_message(message_type,
@@ -71,15 +69,22 @@
         return locked_users
 
     def _message_dispatch(self, message):
+        """Dispatch the given user-change request to the correct handler.
+
+        @param message: The request we got from the server.
+        """
+        user_monitor_connector = RemoteUserMonitorConnector(
+            self.registry.reactor, self.registry.config)
 
         def detect_changes(user_monitor):
             self._user_monitor = user_monitor
             return user_monitor.detect_changes()
 
-        result = self._user_monitor_connector.connect()
+        result = user_monitor_connector.connect()
         result.addCallback(detect_changes)
         result.addCallback(self._perform_operation, message)
         result.addCallback(self._send_changes, message)
+        result.addCallback(lambda x: user_monitor_connector.disconnect())
         return result
 
     def _perform_operation(self, result, message):
@@ -89,9 +94,7 @@
                                                message)
 
     def _send_changes(self, result, message):
-        result = self._user_monitor.detect_changes(message["operation-id"])
-        result.addCallback(lambda x: self._user_monitor_connector.disconnect())
-        return result
+        return self._user_monitor.detect_changes(message["operation-id"])
 
     def _add_user(self, message):
         """Run an C{add-user} operation."""

=== modified file 'landscape/monitor/usermonitor.py'
--- landscape/monitor/usermonitor.py	2010-05-03 13:08:02 +0000
+++ landscape/monitor/usermonitor.py	2010-08-23 11:50:51 +0000
@@ -37,9 +37,6 @@
         socket = os.path.join(self.registry.config.sockets_path,
                               self.name + ".sock")
         self._port = self.registry.reactor.listen_unix(socket, factory)
-        from landscape.manager.usermanager import RemoteUserManagerConnector
-        self._user_manager_connector = RemoteUserManagerConnector(
-            self.registry.reactor, self.registry.config)
 
     def stop(self):
         """Stop listening for incoming AMP connections."""
@@ -67,6 +64,10 @@
         @param operation_id: When present it will be included in the
             C{operation-id} field.
         """
+        from landscape.manager.usermanager import RemoteUserManagerConnector
+        user_manager_connector = RemoteUserManagerConnector(
+            self.registry.reactor, self.registry.config)
+
         # We'll skip checking the locked users if we're in monitor-only mode.
         if getattr(self.registry.config, "monitor_only", False):
             result = maybeDeferred(self._detect_changes,
@@ -77,10 +78,10 @@
                 return user_manager.get_locked_usernames()
 
             def disconnect(locked_usernames):
-                self._user_manager_connector.disconnect()
+                user_manager_connector.disconnect()
                 return locked_usernames
 
-            result = self._user_manager_connector.connect()
+            result = user_manager_connector.connect()
             result.addCallback(get_locked_usernames)
             result.addCallback(disconnect)
             result.addCallback(self._detect_changes, operation_id)