Piotr Kliczewski has uploaded a new change for review.
Change subject: tests: StompAdapter test suite ......................................................................
tests: StompAdapter test suite
Change-Id: Ic22a8ec43413431ff57ecf66da62e065c534bac7 Signed-off-by: pkliczewski piotr.kliczewski@gmail.com --- M lib/yajsonrpc/stompreactor.py M tests/Makefile.am A tests/stompAdapterTests.py 3 files changed, 343 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/42/43342/1
diff --git a/lib/yajsonrpc/stompreactor.py b/lib/yajsonrpc/stompreactor.py index e3b71bf..467a588 100644 --- a/lib/yajsonrpc/stompreactor.py +++ b/lib/yajsonrpc/stompreactor.py @@ -187,9 +187,8 @@ frame.body) return else: - try: - subs = self._sub_dests[destination] - except KeyError: + subs = self._sub_dests[destination] + if len(subs) == 0: self._send_error("Subscription not available", dispatcher.connection) return diff --git a/tests/Makefile.am b/tests/Makefile.am index 7ae8912..eff79f5 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -94,6 +94,7 @@ sourceroutingTests.py \ sslhelper.py \ sslTests.py \ + stompAdapterTests.py \ storageMailboxTests.py \ storageMonitorTests.py \ storageServerTests.py \ diff --git a/tests/stompAdapterTests.py b/tests/stompAdapterTests.py new file mode 100644 index 0000000..a0be47f --- /dev/null +++ b/tests/stompAdapterTests.py @@ -0,0 +1,340 @@ +# +# Copyright 2015 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +# +# Refer to the README and COPYING files for full details of the license +# +from collections import defaultdict + +from testlib import VdsmTestCase as TestCaseBase +from yajsonrpc import JsonRpcRequest +from yajsonrpc.betterAsyncore import Reactor +from yajsonrpc.stomp import \ + Command, \ + Frame, \ + Headers, \ + LEGACY_SUBSCRIPTION_ID_REQUEST +from yajsonrpc.stompreactor import StompAdapterImpl + + +class TestConnection(object): + + def __init__(self, client): + self._client = client + + def send_raw(self, msg): + self._client.queue_frame(msg) + + def handleMessage(self, data): + self._client.queue_frame(data) + + +class TestDispatcher(object): + + def __init__(self, client): + self._client = client + + def setHeartBeat(self, outgoing, incoming=0): + pass + + @property + def connection(self): + return TestConnection(self._client) + + +class TestSubscription(object): + + def __init__(self, destination, id): + self._destination = destination + self._id = id + + def set_client(self, client): + self._client = TestConnection(client) + + @property + def destination(self): + return self._destination + + @property + def id(self): + return self._id + + @property + def client(self): + return self._client + + +class ConnectFrameTest(TestCaseBase): + + def test_connect(self): + frame = Frame(Command.CONNECT, + {Headers.ACCEPT_VERSION: '1.2', + Headers.HEARTEBEAT: '0,8000'}) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.CONNECTED) + self.assertEquals(resp_frame.headers['version'], '1.2') + self.assertEquals(resp_frame.headers[Headers.HEARTEBEAT], '8000,0') + + def test_min_heartbeat(self): + frame = Frame(Command.CONNECT, + {Headers.ACCEPT_VERSION: '1.2', + Headers.HEARTEBEAT: '0,500'}) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.CONNECTED) + self.assertEquals(resp_frame.headers['version'], '1.2') + self.assertEquals(resp_frame.headers[Headers.HEARTEBEAT], '1000,0') + + def test_unsuported_version(self): + frame = Frame(Command.CONNECT, + {Headers.ACCEPT_VERSION: '1.0'}) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.ERROR) + self.assertEquals(resp_frame.body, 'Version unsupported') + + def test_no_heartbeat(self): + frame = Frame(Command.CONNECT, + {Headers.ACCEPT_VERSION: '1.2'}) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.CONNECTED) + self.assertEquals(resp_frame.headers['version'], '1.2') + self.assertEquals(resp_frame.headers[Headers.HEARTEBEAT], '0,0') + + def test_no_headers(self): + frame = Frame(Command.CONNECT) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.ERROR) + self.assertEquals(resp_frame.body, 'Version unsupported') + + +class SubscriptionFrameTest(TestCaseBase): + + def test_subscribe(self): + frame = Frame(Command.SUBSCRIBE, + {Headers.DESTINATION: 'jms.queue.events', + 'ack': 'auto', + 'id': 'ad052acb-a934-4e10-8ec3-00c7417ef8d1'}) + + destinations = defaultdict(list) + + adapter = StompAdapterImpl(Reactor(), destinations, {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + subscription = destinations['jms.queue.events'][0] + self.assertEquals(subscription.id, + 'ad052acb-a934-4e10-8ec3-00c7417ef8d1') + self.assertEquals(subscription.destination, + 'jms.queue.events') + + def test_no_destination(self): + frame = Frame(Command.SUBSCRIBE, + {'ack': 'auto', + 'id': 'ad052acb-a934-4e10-8ec3-00c7417ef8d1'}) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.ERROR) + self.assertEquals(resp_frame.body, + 'Missing destination or subscription id header') + + def test_no_id(self): + frame = Frame(Command.SUBSCRIBE, + {'ack': 'auto', + Headers.DESTINATION: 'jms.queue.events'}) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.ERROR) + self.assertEquals(resp_frame.body, + 'Missing destination or subscription id header') + + +class UnsubscribeFrameTest(TestCaseBase): + + def test_unsubscribe(self): + frame = Frame(Command.UNSUBSCRIBE, + {'id': 'ad052acb-a934-4e10-8ec3-00c7417ef8d1'}) + + subscription = TestSubscription('jms.queue.events', + 'ad052acb-a934-4e10-8ec3-00c7417ef8d1') + + destinations = defaultdict(list) + destinations['jms.queue.events'].append(subscription) + + adapter = StompAdapterImpl(Reactor(), destinations, {}) + adapter._sub_ids['ad052acb-a934-4e10-8ec3-00c7417ef8d1'] = subscription + adapter.handle_frame(TestDispatcher(adapter), frame) + + self.assertTrue(len(adapter._sub_ids) == 0) + self.assertTrue(len(destinations) == 0) + + def test_multipe_subscriptions(self): + frame = Frame(Command.UNSUBSCRIBE, + {'id': 'ad052acb-a934-4e10-8ec3-00c7417ef8d1'}) + + subscription = TestSubscription('jms.queue.events', + 'ad052acb-a934-4e10-8ec3-00c7417ef8d1') + subscription2 = TestSubscription('jms.queue.events', + 'e8a93a6-d886-4cfa-97b9-2d54209053ff') + + destinations = defaultdict(list) + destinations['jms.queue.events'].append(subscription) + destinations['jms.queue.events'].append(subscription2) + + adapter = StompAdapterImpl(Reactor(), destinations, {}) + adapter._sub_ids['ad052acb-a934-4e10-8ec3-00c7417ef8d1'] = subscription + adapter.handle_frame(TestDispatcher(adapter), frame) + + self.assertTrue(len(adapter._sub_ids) == 0) + self.assertTrue(len(destinations) == 1) + self.assertEquals(destinations['jms.queue.events'], [subscription2]) + + def test_no_id(self): + frame = Frame(Command.UNSUBSCRIBE) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.ERROR) + self.assertEquals(resp_frame.body, 'Missing id header') + + +class SendFrameTest(TestCaseBase): + + def test_send(self): + frame = Frame(command=Command.SEND, + headers={Headers.DESTINATION: 'jms.topic.vdsm_requests', + Headers.REPLY_TO: 'jms.topic.vdsm_responses', + Headers.CONTENT_LENGTH: '103'}, + body=('{"jsonrpc":"2.0","method":"Host.getAllVmStats",' + '"params":{},"id":"e8a936a6-d886-4cfa-97b9-2d54209' + '053ff"}' + ) + ) + + ids = {} + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), ids) + adapter.handle_frame(TestDispatcher(adapter), frame) + + data = adapter.pop_message() + self.assertIsNot(data, None) + request = JsonRpcRequest.decode(data) + self.assertEquals(request.method, 'Host.getAllVmStats') + self.assertTrue(len(ids) == 1) + + def test_send_legacy(self): + dest = LEGACY_SUBSCRIPTION_ID_REQUEST + frame = Frame(command=Command.SEND, + headers={Headers.DESTINATION: dest, + Headers.REPLY_TO: 'jms.topic.vdsm_responses', + Headers.CONTENT_LENGTH: '103'}, + body=('{"jsonrpc":"2.0","method":"Host.getAllVmStats",' + '"params":{},"id":"e8a936a6-d886-4cfa-97b9-2d54209' + '053ff"}' + ) + ) + + ids = {} + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), ids) + adapter.handle_frame(TestDispatcher(adapter), frame) + + data = adapter.pop_message() + self.assertIsNot(data, None) + request = JsonRpcRequest.decode(data) + self.assertEquals(request.method, 'Host.getAllVmStats') + self.assertTrue(len(ids) == 1) + + def test_send_no_destination(self): + frame = Frame(Command.SEND, + {Headers.DESTINATION: 'jms.topic.unknown'}) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.ERROR) + self.assertEquals(resp_frame.body, 'Subscription not available') + + def test_send_batch(self): + body = ('[{"jsonrpc":"2.0","method":"Host.getAllVmStats","params":{},' + '"id":"e8a936a6-d886-4cfa-97b9-2d54209053ff"},' + '{"jsonrpc":"2.0","method":"Host.getAllVmStats","params":{},' + '"id":"1202b274-5a06-4671-8b13-1d2715429668"}]') + + frame = Frame(command=Command.SEND, + headers={Headers.DESTINATION: 'jms.topic.vdsm_requests', + Headers.REPLY_TO: 'jms.topic.vdsm_responses', + Headers.CONTENT_LENGTH: '209'}, + body=body + ) + + ids = {} + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), ids) + adapter.handle_frame(TestDispatcher(adapter), frame) + + data = adapter.pop_message() + self.assertIsNot(data, None) + self.assertTrue(len(ids) == 2) + + def test_send_broker(self): + frame = Frame(command=Command.SEND, + headers={Headers.DESTINATION: 'jms.topic.destination', + Headers.CONTENT_LENGTH: '103'}, + body=('{"jsonrpc":"2.0","method":"Host.getAllVmStats",' + '"params":{},"id":"e8a936a6-d886-4cfa-97b9-2d54209' + '053ff"}' + ) + ) + + subscription = TestSubscription('jms.topic.destination', + 'e8a936a6-d886-4cfa-97b9-2d54209053ff') + + destinations = defaultdict(list) + destinations['jms.topic.destination'].append(subscription) + + adapter = StompAdapterImpl(Reactor(), destinations, {}) + subscription.set_client(adapter) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.MESSAGE)
automation@ovirt.org has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 1: Verified+1
Verified by running locally. This is one of few unit tests that we want to have for stomp/jsonrpc.
automation@ovirt.org has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 2:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 3:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 3: Verified+1
Rebase only, no code changes. Copying verification flag from previous patch set.
Yaniv Bronhaim has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 3:
(1 comment)
https://gerrit.ovirt.org/#/c/43342/3/lib/yajsonrpc/stompreactor.py File lib/yajsonrpc/stompreactor.py:
Line 189: else: Line 190: subs = self._sub_dests[destination] Line 191: if len(subs) == 0: Line 192: self._send_error("Subscription not available", Line 193: dispatcher.connection) you still can get KeyError :/ why is it related to the tests? Line 194: return Line 195: Line 196: for subscription in subs: Line 197: headers = utils.picklecopy(frame.headers)
Piotr Kliczewski has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 3:
(1 comment)
https://gerrit.ovirt.org/#/c/43342/3/lib/yajsonrpc/stompreactor.py File lib/yajsonrpc/stompreactor.py:
Line 189: else: Line 190: subs = self._sub_dests[destination] Line 191: if len(subs) == 0: Line 192: self._send_error("Subscription not available", Line 193: dispatcher.connection)
you still can get KeyError :/ why is it related to the tests?
The KeyError is not raised here because we use defaultdict. Line 194: return Line 195: Line 196: for subscription in subs: Line 197: headers = utils.picklecopy(frame.headers)
automation@ovirt.org has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 4:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 4: Verified+1
Verified by running local build.
automation@ovirt.org has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 5:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 6:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 6: Verified+1
Verified by running vdsm build
Yaniv Bronhaim has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 6: Code-Review+1
Yaniv Bronhaim has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 6:
can you add link to the commit message for the patch that introduced the stompAdapter so it will be easier to review if you tested all points?
Piotr Kliczewski has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 6:
I think it was added in several patches so it wont be easy to give single url.
automation@ovirt.org has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 7:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 7:
Verified by building locally. Rebased only.
Piotr Kliczewski has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 7: Verified+1
Yaniv Bronhaim has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 7: Code-Review+1
Dan Kenigsberg has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 7: Code-Review+2
Dan Kenigsberg has submitted this change and it was merged.
Change subject: tests: StompAdapter test suite ......................................................................
tests: StompAdapter test suite
Change-Id: Ic22a8ec43413431ff57ecf66da62e065c534bac7 Signed-off-by: pkliczewski piotr.kliczewski@gmail.com Reviewed-on: https://gerrit.ovirt.org/43342 Continuous-Integration: Jenkins CI Reviewed-by: Yaniv Bronhaim Reviewed-by: Dan Kenigsberg danken@redhat.com --- M tests/Makefile.am A tests/stompAdapterTests.py 2 files changed, 341 insertions(+), 0 deletions(-)
Approvals: Yaniv Bronhaim: Looks good to me, but someone else must approve Piotr Kliczewski: Verified Jenkins CI: Passed CI tests Dan Kenigsberg: Looks good to me, approved
automation@ovirt.org has posted comments on this change.
Change subject: tests: StompAdapter test suite ......................................................................
Patch Set 8:
* Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found.
vdsm-patches@lists.fedorahosted.org