Fix system.multicall() broken by faster start/stop patch

In past versions, startProcess() and stopProcess() would always return
a callback.  50d18577af28b6b053c0d446c959b42b5300a0f8 changed this
so they may return either a callback or a bool, but the code that handles
system.multicall() was not updated.  If multicall was used with stopProcess()
followed by startProcess(), it would try to start the process before it had
finished stopping.  This broke the restart process link on the web interface.

supervisor/tests/test_rpcinterfaces.py

@@ -2121,44 +2121,34 @@ class SystemNamespaceXMLRPCInterfaceTests(TestBase):
 
     def test_multicall_simplevals(self):
         interface = self._makeOne()
-        callback = interface.multicall([
+        results = interface.multicall([
             {'methodName':'system.methodHelp', 'params':['system.methodHelp']},
             {'methodName':'system.listMethods', 'params':[]},
             ])
-        from supervisor import http
-        result = http.NOT_DONE_YET
-        while result is http.NOT_DONE_YET:
-            result = callback()
-        self.assertEqual(result[0], interface.methodHelp('system.methodHelp'))
-        self.assertEqual(result[1], interface.listMethods())
+        self.assertEqual(results[0], interface.methodHelp('system.methodHelp'))
+        self.assertEqual(results[1], interface.listMethods())
 
     def test_multicall_recursion_guard(self):
         from supervisor import xmlrpc
         interface = self._makeOne()
-        callback = interface.multicall([
+        results = interface.multicall([
             {'methodName': 'system.multicall', 'params': []},
         ])
 
-        from supervisor import http
-        result = http.NOT_DONE_YET
-        while result is http.NOT_DONE_YET:
-            result = callback()
-
-        code = xmlrpc.Faults.INCORRECT_PARAMETERS
-        desc = xmlrpc.getFaultDescription(code)
-        recursion_fault = {'faultCode': code, 'faultString': desc}
-
-        self.assertEqual(result, [recursion_fault])
+        e = xmlrpc.RPCError(xmlrpc.Faults.INCORRECT_PARAMETERS,
+                'Recursive system.multicall forbidden')
+        recursion_fault = {'faultCode': e.code, 'faultString': e.text}
+        self.assertEqual(results, [recursion_fault])
 
     def test_multicall_nested_callback(self):
+        from supervisor import http
         interface = self._makeOne()
         callback = interface.multicall([
             {'methodName':'supervisor.stopAllProcesses'}])
-        from supervisor import http
-        result = http.NOT_DONE_YET
-        while result is http.NOT_DONE_YET:
-            result = callback()
-        self.assertEqual(result[0], [])
+        results = http.NOT_DONE_YET
+        while results is http.NOT_DONE_YET:
+            results = callback()
+        self.assertEqual(results[0], [])
 
     def test_methodHelp(self):
         from supervisor import xmlrpc

supervisor/tests/test_xmlrpc.py

@@ -480,6 +480,253 @@ class TestDeferredXMLRPCResponse(unittest.TestCase):
         self.assertEqual(src, 'XML-RPC response callback error')
         self.assertTrue("Traceback" in msg)
 
+class TestSystemNamespaceRPCInterface(unittest.TestCase):
+    def _makeOne(self, namespaces=()):
+        from supervisor.xmlrpc import SystemNamespaceRPCInterface
+        return SystemNamespaceRPCInterface(namespaces)
+
+    def test_listMethods_gardenpath(self):
+        inst = self._makeOne()
+        result = inst.listMethods()
+        self.assertEqual(
+            result,
+            ['system.listMethods',
+             'system.methodHelp',
+             'system.methodSignature',
+             'system.multicall',
+             ]
+            )
+
+    def test_listMethods_omits_underscore_attrs(self):
+        class DummyNamespace(object):
+            def foo(self): pass
+            def _bar(self): pass
+        ns1 = DummyNamespace()
+        inst = self._makeOne([('ns1', ns1)])
+        result = inst.listMethods()
+        self.assertEqual(
+            result,
+            ['ns1.foo',
+             'system.listMethods',
+             'system.methodHelp',
+             'system.methodSignature',
+             'system.multicall'
+             ]
+            )
+
+    def test_methodHelp_known_method(self):
+        inst = self._makeOne()
+        result = inst.methodHelp('system.listMethods')
+        self.assertTrue('array' in result)
+
+    def test_methodHelp_unknown_method(self):
+        from supervisor.xmlrpc import RPCError
+        inst = self._makeOne()
+        self.assertRaises(RPCError, inst.methodHelp, 'wont.be.found')
+
+    def test_methodSignature_known_method(self):
+        inst = self._makeOne()
+        result = inst.methodSignature('system.methodSignature')
+        self.assertEqual(result, ['array', 'string'])
+
+    def test_methodSignature_unknown_method(self):
+        from supervisor.xmlrpc import RPCError
+        inst = self._makeOne()
+        self.assertRaises(RPCError, inst.methodSignature, 'wont.be.found')
+
+    def test_methodSignature_with_bad_sig(self):
+        from supervisor.xmlrpc import RPCError
+        class DummyNamespace(object):
+            def foo(self):
+                """ @param string name The thing"""
+        ns1 = DummyNamespace()
+        inst = self._makeOne([('ns1', ns1)])
+        self.assertRaises(RPCError, inst.methodSignature, 'ns1.foo')
+
+    def test_multicall_faults_for_recursion(self):
+        from supervisor.xmlrpc import Faults
+        inst = self._makeOne()
+        calls = [{'methodName':'system.multicall'}]
+        results = inst.multicall(calls)
+        self.assertEqual(
+            results,
+            [{'faultCode': Faults.INCORRECT_PARAMETERS,
+              'faultString': ('INCORRECT_PARAMETERS: Recursive '
+                              'system.multicall forbidden')}]
+            )
+
+    def test_multicall_faults_for_missing_methodName(self):
+        from supervisor.xmlrpc import Faults
+        inst = self._makeOne()
+        calls = [{}]
+        results = inst.multicall(calls)
+        self.assertEqual(
+            results,
+            [{'faultCode': Faults.INCORRECT_PARAMETERS,
+              'faultString': 'INCORRECT_PARAMETERS: No methodName'}]
+            )
+
+    def test_multicall_faults_for_methodName_bad_namespace(self):
+        from supervisor.xmlrpc import Faults
+        inst = self._makeOne()
+        calls = [{'methodName': 'bad.stopProcess'}]
+        results = inst.multicall(calls)
+        self.assertEqual(
+            results,
+            [{'faultCode': Faults.UNKNOWN_METHOD,
+              'faultString': 'UNKNOWN_METHOD'}]
+            )
+
+    def test_multicall_faults_for_methodName_good_ns_bad_method(self):
+        from supervisor.xmlrpc import Faults
+        class DummyNamespace(object):
+            pass
+        ns1 = DummyNamespace()
+        inst = self._makeOne([('ns1', ns1)])
+        calls = [{'methodName': 'ns1.bad'}]
+        results = inst.multicall(calls)
+        self.assertEqual(
+            results,
+            [{'faultCode': Faults.UNKNOWN_METHOD,
+              'faultString': 'UNKNOWN_METHOD'}]
+            )
+
+    def test_multicall_returns_empty_results_for_empty_calls(self):
+        inst = self._makeOne()
+        calls = []
+        results = inst.multicall(calls)
+        self.assertEqual(results, [])
+
+    def test_multicall_performs_noncallback_functions_serially(self):
+        class DummyNamespace(object):
+            def say(self, name):
+                """ @param string name Process name"""
+                return name
+        ns1 = DummyNamespace()
+        inst = self._makeOne([('ns1', ns1)])
+        calls = [
+            {'methodName': 'ns1.say', 'params': ['Alvin']},
+            {'methodName': 'ns1.say', 'params': ['Simon']},
+            {'methodName': 'ns1.say', 'params': ['Theodore']}
+        ]
+        results = inst.multicall(calls)
+        self.assertEqual(results, ['Alvin', 'Simon', 'Theodore'])
+
+    def test_multicall_catches_noncallback_exceptions(self):
+        import errno
+        from supervisor.xmlrpc import RPCError, Faults
+        class DummyNamespace(object):
+            def bad_name(self):
+                raise RPCError(Faults.BAD_NAME, 'foo')
+            def os_error(self):
+                raise OSError(errno.ENOENT)
+        ns1 = DummyNamespace()
+        inst = self._makeOne([('ns1', ns1)])
+        calls = [{'methodName': 'ns1.bad_name'}, {'methodName': 'ns1.os_error'}]
+        results = inst.multicall(calls)
+
+        bad_name = {'faultCode': Faults.BAD_NAME,
+                    'faultString': 'BAD_NAME: foo'}
+        os_error = {'faultCode': Faults.FAILED,
+                    'faultString': "FAILED: %s:2" % OSError}
+        self.assertEqual(results, [bad_name, os_error])
+
+    def test_multicall_catches_callback_exceptions(self):
+        import errno
+        from supervisor.xmlrpc import RPCError, Faults
+        from supervisor.http import NOT_DONE_YET
+        class DummyNamespace(object):
+            def bad_name(self):
+                def inner():
+                    raise RPCError(Faults.BAD_NAME, 'foo')
+                return inner
+            def os_error(self):
+                def inner():
+                    raise OSError(errno.ENOENT)
+                return inner
+        ns1 = DummyNamespace()
+        inst = self._makeOne([('ns1', ns1)])
+        calls = [{'methodName': 'ns1.bad_name'}, {'methodName': 'ns1.os_error'}]
+        callback = inst.multicall(calls)
+        results = NOT_DONE_YET
+        while results is NOT_DONE_YET:
+            results = callback()
+
+        bad_name = {'faultCode': Faults.BAD_NAME,
+                    'faultString': 'BAD_NAME: foo'}
+        os_error = {'faultCode': Faults.FAILED,
+                    'faultString': "FAILED: %s:2" % OSError}
+        self.assertEqual(results, [bad_name, os_error])
+
+    def test_multicall_performs_callback_functions_serially(self):
+        from supervisor.http import NOT_DONE_YET
+        class DummyNamespace(object):
+            def __init__(self):
+                self.stop_results = [NOT_DONE_YET, NOT_DONE_YET,
+                    NOT_DONE_YET, 'stop result']
+                self.start_results = ['start result']
+            def stopProcess(self, name):
+                def inner():
+                    result = self.stop_results.pop(0)
+                    if result is not NOT_DONE_YET:
+                        self.stopped = True
+                    return result
+                return inner
+            def startProcess(self, name):
+                def inner():
+                    if not self.stopped:
+                        raise Exception("This should not raise")
+                    return self.start_results.pop(0)
+                return inner
+        ns1 = DummyNamespace()
+        inst = self._makeOne([('ns1', ns1)])
+        calls = [{'methodName': 'ns1.stopProcess',
+                  'params': {'name': 'foo'}},
+                 {'methodName': 'ns1.startProcess',
+                  'params': {'name': 'foo'}}]
+        callback = inst.multicall(calls)
+        results = NOT_DONE_YET
+        while results is NOT_DONE_YET:
+            results = callback()
+        self.assertEqual(results, ['stop result', 'start result'])
+
+class Test_gettags(unittest.TestCase):
+    def _callFUT(self, comment):
+        from supervisor.xmlrpc import gettags
+        return gettags(comment)
+
+    def test_one_atpart(self):
+        lines = '@foo'
+        result = self._callFUT(lines)
+        self.assertEqual(
+            result,
+            [(0, None, None, None, ''), (0, 'foo', '', '', '')]
+            )
+
+    def test_two_atparts(self):
+        lines = '@foo array'
+        result = self._callFUT(lines)
+        self.assertEqual(
+            result,
+            [(0, None, None, None, ''), (0, 'foo', 'array', '', '')]
+            )
+
+    def test_three_atparts(self):
+        lines = '@foo array name'
+        result = self._callFUT(lines)
+        self.assertEqual(
+            result,
+            [(0, None, None, None, ''), (0, 'foo', 'array', 'name', '')]
+            )
+
+    def test_four_atparts(self):
+        lines = '@foo array name text'
+        result = self._callFUT(lines)
+        self.assertEqual(
+            result,
+            [(0, None, None, None, ''), (0, 'foo', 'array', 'name', 'text')]
+            )
+
 class DummyResponse:
     def __init__(self, status=200, body='', reason='reason'):
         self.status = status

supervisor/web.py

@@ -405,20 +405,27 @@ class StatusView(MeldView):
                         return stopdone
 
                 elif action == 'restart':
-                    callback = rpcinterface.system.multicall(
+                    results_or_callback = rpcinterface.system.multicall(
                         [ {'methodName':'supervisor.stopProcess',
                            'params': [namespec]},
                           {'methodName':'supervisor.startProcess',
                            'params': [namespec]},
                           ]
                         )
-                    def restartprocess():
-                        result = callback()
-                        if result is NOT_DONE_YET:
-                            return NOT_DONE_YET
-                        return 'Process %s restarted' % namespec
-                    restartprocess.delay = 0.05
-                    return restartprocess
+                    if callable(results_or_callback):
+                        callback = results_or_callback
+                        def restartprocess():
+                            results = callback()
+                            if results is NOT_DONE_YET:
+                                return NOT_DONE_YET
+                            return 'Process %s restarted' % namespec
+                        restartprocess.delay = 0.05
+                        return restartprocess
+                    else:
+                        def restartdone():
+                            return 'Process %s restarted' % namespec
+                        restartdone.delay = 0.05
+                        return restartdone
 
                 elif action == 'clearlog':
                     try:

supervisor/xmlrpc.py

@@ -239,7 +239,7 @@ class SystemNamespaceRPCInterface:
         """Process an array of calls, and return an array of
         results. Calls should be structs of the form {'methodName':
         string, 'params': array}. Each result will either be a
-        single-item array containg the result value, or a struct of
+        single-item array containing the result value, or a struct of
         the form {'faultCode': int, 'faultString': string}. This is
         useful when you need to make lots of small calls without lots
         of round trips.
@@ -247,63 +247,81 @@ class SystemNamespaceRPCInterface:
         @param array calls  An array of call requests
         @return array result  An array of results
         """
-        producers = []
+        remaining_calls = calls[:] # [{'methodName':x, 'params':x}, ...]
+        callbacks = [] # always empty or 1 callback function only
+        results = [] # results of completed calls
 
-        for call in calls:
-            try:
-                name = call['methodName']
-                params = call.get('params', [])
-                if name == 'system.multicall':
-                    # Recursive system.multicall forbidden
-                    raise RPCError(Faults.INCORRECT_PARAMETERS)
-                root = AttrDict(self.namespaces)
-                value = traverse(root, name, params)
-            except RPCError, inst:
-                value = {'faultCode': inst.code,
-                         'faultString': inst.text}
-            except:
-                errmsg = "%s:%s" % (sys.exc_type, sys.exc_value)
-                value = {'faultCode': 1, 'faultString': errmsg}
-            producers.append(value)
-
-        results = []
-
-        def multiproduce():
-            """ Run through all the producers in order """
-            if not producers:
-                return []
-
-            callback = producers.pop(0)
-
-            if isinstance(callback, types.FunctionType):
-                try:
-                    value = callback()
-                except RPCError, inst:
-                    value = {'faultCode':inst.code, 'faultString':inst.text}
+        # args are only to fool scoping and are never passed by caller
+        def multi(remaining_calls=remaining_calls,
+                  callbacks=callbacks,
+                  results=results):
 
-                if value is NOT_DONE_YET:
-                    # push it back in the front of the queue because we
-                    # need to finish the calls in requested order
-                    producers.insert(0, callback)
-                    return NOT_DONE_YET
-            else:
-                value = callback
+            # if waiting on a callback, call it, then remove it if it's done
+            if callbacks:
+                try:
+                    value = callbacks[0]()
+                except RPCError, exc:
+                    value = {'faultCode': exc.code,
+                             'faultString': exc.text}
+                except:
+                    info = sys.exc_info()
+                    errmsg = "%s:%s" % (info[0], info[1])
+                    value = {'faultCode': Faults.FAILED,
+                             'faultString': 'FAILED: ' + errmsg}
+                if value is not NOT_DONE_YET:
+                    callbacks.pop(0)
+                    results.append(value)
+
+            # if we don't have a callback now, pop calls and call them in
+            # order until one returns a callback.
+            while (not callbacks) and remaining_calls:
+                call = remaining_calls.pop(0)
+                name = call.get('methodName', None)
+                params = call.get('params', [])
 
-            results.append(value)
+                try:
+                    if name is None:
+                        raise RPCError(Faults.INCORRECT_PARAMETERS,
+                            'No methodName')
+                    if name == 'system.multicall':
+                        raise RPCError(Faults.INCORRECT_PARAMETERS,
+                            'Recursive system.multicall forbidden')
+                    # make the call, may return a callback or not
+                    root = AttrDict(self.namespaces)
+                    value = traverse(root, name, params)
+                except RPCError, exc:
+                    value = {'faultCode': exc.code,
+                             'faultString': exc.text}
+                except:
+                    info = sys.exc_info()
+                    errmsg = "%s:%s" % (info[0], info[1])
+                    value = {'faultCode': Faults.FAILED,
+                             'faultString': 'FAILED: ' + errmsg}
+
+                if isinstance(value, types.FunctionType):
+                    callbacks.append(value)
+                else:
+                    results.append(value)
 
-            if producers:
-                # only finish when all producers are finished
+            # we are done when there's no callback and no more calls queued
+            if callbacks or remaining_calls:
                 return NOT_DONE_YET
-
-            return results
-
-        multiproduce.delay = .05
-        return multiproduce
+            else:
+                return results
+        multi.delay = 0.05
+
+        # optimization: multi() is called here instead of just returning
+        # multi in case all calls complete and we can return with no delay.
+        value = multi()
+        if value is NOT_DONE_YET:
+            return multi
+        else:
+            return value
 
 class AttrDict(dict):
     # hack to make a dict's getattr equivalent to its getitem
     def __getattr__(self, name):
-        return self[name]
+        return self.get(name)
 
 class RootRPCInterface:
     def __init__(self, subinterfaces):