From f8992e0edf43f09707c62750baad1af0581a41fa Mon Sep 17 00:00:00 2001 From: Ben Thomasson Date: Mon, 12 Mar 2018 17:41:15 -0400 Subject: [PATCH] Makes changes suggested by wwitzel3's review --- awx/network_ui/consumers.py | 68 +++++++------------ awx/network_ui/routing.py | 4 +- awx/network_ui_test/consumers.py | 63 ++++++----------- .../migrations/0002_auto_20180306_2243.py | 9 +-- awx/network_ui_test/routing.py | 4 +- awx/network_ui_test/tools/coverage_report.py | 14 +--- 6 files changed, 55 insertions(+), 107 deletions(-) diff --git a/awx/network_ui/consumers.py b/awx/network_ui/consumers.py index bda2e872a3..0cd2f82305 100644 --- a/awx/network_ui/consumers.py +++ b/awx/network_ui/consumers.py @@ -11,28 +11,13 @@ import logging from awx.network_ui.utils import transform_dict -from pprint import pformat import json # Connected to websocket.connect -HISTORY_MESSAGE_IGNORE_TYPES = ['DeviceSelected', - 'DeviceUnSelected', - 'LinkSelected', - 'LinkUnSelected', - 'MouseEvent', - 'MouseWheelEvent', - 'KeyEvent'] - - logger = logging.getLogger("awx.network_ui.consumers") -class NetworkUIException(Exception): - - pass - - def parse_inventory_id(data): inventory_id = data.get('inventory_id', ['null']) try: @@ -43,37 +28,38 @@ def parse_inventory_id(data): inventory_id = None return inventory_id -# Persistence +class Persistence(object): -class _Persistence(object): + def parse_message_text(self, message_text, client_id): + data = json.loads(message_text) + if len(data) == 2: + message_type = data.pop(0) + message_value = data.pop(0) + if isinstance(message_value, list): + logger.error("Message has no sender") + return None, None + if isinstance(message_value, dict) and client_id != message_value.get('sender'): + logger.error("client_id mismatch expected: %s actual %s", client_id, message_value.get('sender')) + return None, None + return message_type, message_value + else: + logger.error("Invalid message text") + return None, None def handle(self, message): topology_id = message.get('topology') assert topology_id is not None, "No topology_id" client_id = message.get('client') assert client_id is not None, "No client_id" - data = json.loads(message['text']) - if isinstance(data[1], list): - logger.error("Message has no sender") + message_type, message_value = self.parse_message_text(message['text'], client_id) + if message_type is None: return - if isinstance(data[1], dict) and client_id != data[1].get('sender'): - logger.error("client_id mismatch expected: %s actual %s", client_id, data[1].get('sender')) - logger.error(pformat(data)) - return - message_type = data[0] - message_value = data[1] handler = self.get_handler(message_type) if handler is not None: try: handler(message_value, topology_id, client_id) - except NetworkUIException, e: - Group("client-%s" % client_id).send({"text": json.dumps(["Error", str(e)])}) - raise - except Exception, e: - Group("client-%s" % client_id).send({"text": json.dumps(["Error", "Server Error"])}) - raise - except BaseException, e: + except Exception: Group("client-%s" % client_id).send({"text": json.dumps(["Error", "Server Error"])}) raise else: @@ -153,6 +139,10 @@ class _Persistence(object): device_map = dict(Device.objects .filter(topology_id=topology_id, id__in=[link['from_device_id'], link['to_device_id']]) .values_list('id', 'pk')) + if link['from_device_id'] not in device_map: + return + if link['to_device_id'] not in device_map: + return Link.objects.filter(id=link['id'], from_device_id=device_map[link['from_device_id']], to_device_id=device_map[link['to_device_id']], @@ -186,21 +176,16 @@ class _Persistence(object): logger.warning("Unsupported message %s", message['msg_type']) -persistence = _Persistence() - - -# UI Channel Events - @channel_session def ws_connect(message): # Accept connection data = urlparse.parse_qs(message.content['query_string']) inventory_id = parse_inventory_id(data) topology_ids = list(TopologyInventory.objects.filter(inventory_id=inventory_id).values_list('topology_id', flat=True)) - topology_id = 0 + topology_id = None if len(topology_ids) > 0: topology_id = topology_ids[0] - if topology_id: + if topology_id is not None: topology = Topology.objects.get(topology_id=topology_id) else: topology = Topology(name="topology", scale=1.0, panX=0, panY=0) @@ -234,8 +219,7 @@ def send_snapshot(channel, topology_id): .filter(device__topology_id=topology_id) .values()): interfaces[i['device_id']].append(i) - devices = list(Device.objects - .filter(topology_id=topology_id).values()) + devices = list(Device.objects.filter(topology_id=topology_id).values()) for device in devices: device['interfaces'] = interfaces[device['device_id']] diff --git a/awx/network_ui/routing.py b/awx/network_ui/routing.py index e0b04d7fe1..dbbb112a7c 100644 --- a/awx/network_ui/routing.py +++ b/awx/network_ui/routing.py @@ -1,10 +1,10 @@ # Copyright (c) 2017 Red Hat, Inc from channels.routing import route -from awx.network_ui.consumers import ws_connect, ws_message, ws_disconnect, persistence +from awx.network_ui.consumers import ws_connect, ws_message, ws_disconnect, Persistence channel_routing = [ route("websocket.connect", ws_connect, path=r"^/network_ui/topology"), route("websocket.receive", ws_message, path=r"^/network_ui/topology"), route("websocket.disconnect", ws_disconnect, path=r"^/network_ui/topology"), - route("persistence", persistence.handle), + route("persistence", Persistence().handle), ] diff --git a/awx/network_ui_test/consumers.py b/awx/network_ui_test/consumers.py index c5b01cb80f..a6d3d6a2cf 100644 --- a/awx/network_ui_test/consumers.py +++ b/awx/network_ui_test/consumers.py @@ -11,28 +11,12 @@ import logging from django.utils.dateparse import parse_datetime -from pprint import pformat - import json # Connected to websocket.connect -HISTORY_MESSAGE_IGNORE_TYPES = ['DeviceSelected', - 'DeviceUnSelected', - 'LinkSelected', - 'LinkUnSelected', - 'MouseEvent', - 'MouseWheelEvent', - 'KeyEvent'] - - logger = logging.getLogger("awx.network_ui_test.consumers") -class NetworkUIException(Exception): - - pass - - def parse_inventory_id(data): inventory_id = data.get('inventory_id', ['null']) try: @@ -43,37 +27,38 @@ def parse_inventory_id(data): inventory_id = None return inventory_id -# TestPersistence +class TestPersistence(object): -class _TestPersistence(object): + def parse_message_text(self, message_text, client_id): + data = json.loads(message_text) + if len(data) == 2: + message_type = data.pop(0) + message_value = data.pop(0) + if isinstance(message_value, list): + logger.error("Message has no sender") + return None, None + if isinstance(message_value, dict) and client_id != message_value.get('sender'): + logger.error("client_id mismatch expected: %s actual %s", client_id, message_value.get('sender')) + return None, None + return message_type, message_value + else: + logger.error("Invalid message text") + return None, None def handle(self, message): topology_id = message.get('topology') assert topology_id is not None, "No topology_id" client_id = message.get('client') assert client_id is not None, "No client_id" - data = json.loads(message['text']) - if isinstance(data[1], list): - logger.error("Message has no sender") + message_type, message_value = self.parse_message_text(message['text'], client_id) + if message_type is None: return - if isinstance(data[1], dict) and client_id != data[1].get('sender'): - logger.error("client_id mismatch expected: %s actual %s", client_id, data[1].get('sender')) - logger.error(pformat(data)) - return - message_type = data[0] - message_value = data[1] handler = self.get_handler(message_type) if handler is not None: try: handler(message_value, topology_id, client_id) - except NetworkUIException, e: - Group("client-%s" % client_id).send({"text": json.dumps(["Error", str(e)])}) - raise - except Exception, e: - Group("client-%s" % client_id).send({"text": json.dumps(["Error", "Server Error"])}) - raise - except BaseException, e: + except Exception: Group("client-%s" % client_id).send({"text": json.dumps(["Error", "Server Error"])}) raise else: @@ -106,7 +91,6 @@ class _TestPersistence(object): commits_since=int(commits_since), commit_hash=commit_hash) - logger.error("TR: %s", test_result) tr = TestResult(id=test_result['id'], result_id=Result.objects.get(name=test_result['result']).pk, test_case_id=TestCase.objects.get(name=test_result['name']).pk, @@ -156,21 +140,16 @@ class _TestPersistence(object): topology_id=topology_id).save() -test_persistence = _TestPersistence() - - -# UI Channel Events - @channel_session def ws_connect(message): # Accept connection data = urlparse.parse_qs(message.content['query_string']) inventory_id = parse_inventory_id(data) topology_ids = list(TopologyInventory.objects.filter(inventory_id=inventory_id).values_list('topology_id', flat=True)) - topology_id = 0 + topology_id = None if len(topology_ids) > 0: topology_id = topology_ids[0] - if topology_id: + if topology_id is not None: topology = Topology.objects.get(topology_id=topology_id) else: topology = Topology(name="topology", scale=1.0, panX=0, panY=0) diff --git a/awx/network_ui_test/migrations/0002_auto_20180306_2243.py b/awx/network_ui_test/migrations/0002_auto_20180306_2243.py index 217ed3d8e4..d1a646dcee 100644 --- a/awx/network_ui_test/migrations/0002_auto_20180306_2243.py +++ b/awx/network_ui_test/migrations/0002_auto_20180306_2243.py @@ -5,13 +5,8 @@ from __future__ import unicode_literals from django.db import migrations -results = ['passed', - 'failed', - 'errored', - 'skipped', - 'aborted', - 'not run', - 'blocked'] +results = ['passed', 'failed', 'errored', 'skipped', + 'aborted', 'not run', 'blocked'] def populate_result_types(apps, schema_editor): diff --git a/awx/network_ui_test/routing.py b/awx/network_ui_test/routing.py index b4ba915891..f2f27f62d2 100644 --- a/awx/network_ui_test/routing.py +++ b/awx/network_ui_test/routing.py @@ -1,12 +1,12 @@ # Copyright (c) 2017 Red Hat, Inc from channels.routing import route -from awx.network_ui_test.consumers import ws_connect, ws_message, ws_disconnect, test_persistence +from awx.network_ui_test.consumers import ws_connect, ws_message, ws_disconnect, TestPersistence channel_routing = [ route("websocket.connect", ws_connect, path=r"^/network_ui/test"), route("websocket.receive", ws_message, path=r"^/network_ui/test"), route("websocket.disconnect", ws_disconnect, path=r"^/network_ui/test"), - route("test_persistence", test_persistence.handle), + route("test_persistence", TestPersistence().handle), ] diff --git a/awx/network_ui_test/tools/coverage_report.py b/awx/network_ui_test/tools/coverage_report.py index 4eaccccee3..0563f71033 100755 --- a/awx/network_ui_test/tools/coverage_report.py +++ b/awx/network_ui_test/tools/coverage_report.py @@ -21,7 +21,6 @@ logger = logging.getLogger('coverage_report') TESTS_API = '/network_ui_test/tests' - def main(args=None): if args is None: args = sys.argv[1:] @@ -33,10 +32,7 @@ def main(args=None): else: logging.basicConfig(level=logging.WARNING) - - print (parsed_args['']) server = parsed_args[''] - tests = requests.get(server + TESTS_API, verify=False).json() for test in tests['tests']: @@ -45,17 +41,11 @@ def main(args=None): with open(test['name'] + "/coverage.json", 'w') as f: f.write(requests.get(server + test['coverage'], verify=False).text) - - #for test in tests['tests']: - # subprocess.Popen('istanbul report html', shell=True, cwd=test['name']).wait() - - + for test in tests['tests']: + subprocess.Popen('istanbul report html', shell=True, cwd=test['name']).wait() subprocess.Popen('istanbul report html', shell=True).wait() - - return 0 if __name__ == '__main__': sys.exit(main(sys.argv[1:])) -