From 470db4aebf38b7ec87d04a4b3e666f25f003bd87 Mon Sep 17 00:00:00 2001 From: Charles Mita Date: Mon, 13 Jun 2016 16:59:56 +0100 Subject: [PATCH] Change Post responses to go through the process queue first. Fixes out-of-order messages, where an Update (triggered by a method call that changes attributes) can arrive after the method's Return message. --- malcolm/core/block.py | 5 ++++- malcolm/core/method.py | 7 ++++--- malcolm/core/process.py | 9 ++++++++- tests/test_core/test_block.py | 3 ++- tests/test_core/test_method.py | 13 +++++++------ 5 files changed, 25 insertions(+), 12 deletions(-) diff --git a/malcolm/core/block.py b/malcolm/core/block.py index ac8886fee..45033d6ee 100644 --- a/malcolm/core/block.py +++ b/malcolm/core/block.py @@ -1,6 +1,7 @@ from collections import OrderedDict from malcolm.core.monitorable import Monitorable +from malcolm.core.process import BlockRespond class Block(Monitorable): @@ -55,7 +56,9 @@ def handle_request(self, request): self.log_debug("Received request %s", request) if request.type_ == request.POST: method_name = request.endpoint[-1] - self._methods[method_name].handle_request(request) + response = self._methods[method_name].get_response(request) + response = BlockRespond(response, request.response_queue) + self.parent.q.put(response) else: layer = self for next_link in request.endpoint[1:]: diff --git a/malcolm/core/method.py b/malcolm/core/method.py index 9afaa60f0..4f328ae28 100644 --- a/malcolm/core/method.py +++ b/malcolm/core/method.py @@ -3,6 +3,7 @@ from malcolm.core.monitorable import Monitorable from malcolm.core.mapmeta import MapMeta, OPTIONAL, REQUIRED +from malcolm.core.response import Response class Method(Monitorable): @@ -66,7 +67,7 @@ def __call__(self, *args, **kwargs): self.returns.elements[r_name].validate(r_val) return return_val - def handle_request(self, request): + def get_response(self, request): """Call exposed function using request parameters and respond with the result @@ -79,10 +80,10 @@ def handle_request(self, request): except Exception as error: self.log_debug("Error raised %s", error.message) message = "Method %s raised an error: %s" % (self.name, error.message) - request.respond_with_error(message) + return Response.Error(request.id_, request.context, message) else: self.log_debug("Returning result %s", result) - request.respond_with_return(result) + return Response.Return(request.id_, request.context, value=result) def to_dict(self): """Return ordered dictionary representing Method object.""" diff --git a/malcolm/core/process.py b/malcolm/core/process.py index 4e3f71a05..b1d5e6d0e 100644 --- a/malcolm/core/process.py +++ b/malcolm/core/process.py @@ -11,8 +11,10 @@ # Internal update messages BlockNotify = namedtuple("BlockNotify", "name") BlockChanged = namedtuple("BlockChanged", "changes") +BlockRespond = namedtuple("BlockRespond", "response, response_queue") BlockNotify.type_ = "BlockNotify" BlockChanged.type_ = "BlockChanged" +BlockRespond.type_ = "BlockRespond" class Process(Loggable): @@ -35,7 +37,8 @@ def __init__(self, name, sync_factory): Request.GET: self._handle_get, Request.SUBSCRIBE: self._handle_subscribe, BlockNotify.type_: self._handle_block_notify, - BlockChanged.type_: self._handle_block_changed + BlockChanged.type_: self._handle_block_changed, + BlockRespond.type_: self._handle_block_respond } def recv_loop(self): @@ -161,6 +164,10 @@ def _handle_block_changed(self, request): block_changes = self._last_changes.setdefault(path[0], []) block_changes.append([path, value]) + def _handle_block_respond(self, request): + """Push the response to the required queue""" + request.response_queue.put(request.response) + def _handle_subscribe(self, request): """Add a new subscriber and respond with the current sub-structure state""" diff --git a/tests/test_core/test_block.py b/tests/test_core/test_block.py index ac35f5ca5..7c5bcacd8 100644 --- a/tests/test_core/test_block.py +++ b/tests/test_core/test_block.py @@ -98,6 +98,7 @@ class TestHandleRequest(unittest.TestCase): def setUp(self): self.block = Block("TestBlock") + self.block.parent = MagicMock() self.method = MagicMock() self.method.name = "get_things" self.response = MagicMock() @@ -111,7 +112,7 @@ def test_given_request_then_pass_to_correct_method(self): self.block.handle_request(request) - self.method.handle_request.assert_called_once_with(request) + self.method.get_response.assert_called_once_with(request) def test_given_get_then_return_attribute(self): self.block.state = MagicMock() diff --git a/tests/test_core/test_method.py b/tests/test_core/test_method.py index b11b36bd7..7701c1fcc 100644 --- a/tests/test_core/test_method.py +++ b/tests/test_core/test_method.py @@ -10,6 +10,7 @@ from malcolm.core.method import Method, takes, returns from malcolm.core.mapmeta import OPTIONAL, REQUIRED +from malcolm.core.response import Response class TestMethod(unittest.TestCase): @@ -134,9 +135,9 @@ def test_handle_request(self): request = Mock( id=(123, Mock()), type="Post", parameters={"first": 2}, respond_with_return=Mock()) - m.handle_request(request) + response = m.get_response(request) func.assert_called_with({"first": 2}) - request.respond_with_return.assert_called_with({"output": 1}) + self.assertEquals({"output":1}, response.value) def test_handle_request_error(self): func = MagicMock() @@ -147,10 +148,10 @@ def test_handle_request_error(self): m.returns = MagicMock() request = MagicMock() - m.handle_request(request) - - request.respond_with_error.assert_called_once_with( - "Method test_method raised an error: Test error") + response = m.get_response(request) + self.assertEquals(Response.ERROR, response.type_) + self.assertEquals( + "Method test_method raised an error: Test error", response.message) def test_to_dict_serialization(self): func = Mock(return_value={"out": "dummy"})