From 6f2eae56caae92b962fc6f2af859fa3e311b3ed0 Mon Sep 17 00:00:00 2001 From: Cezary Statkiewicz Date: Tue, 16 Aug 2022 20:23:29 +0200 Subject: [PATCH] #44 client code updated * mllp_client code updated to react on timeouts from socket, use of argparse instead of obsolete optparse * tests updated to follow new features and convetions --- hl7/client.py | 119 +++++++++++++++++++-------------- hl7/exceptions.py | 6 ++ tests/test_client.py | 154 +++++++++++++++++++++++++++---------------- 3 files changed, 172 insertions(+), 107 deletions(-) diff --git a/hl7/client.py b/hl7/client.py index a6805a9..1a0893e 100644 --- a/hl7/client.py +++ b/hl7/client.py @@ -1,12 +1,13 @@ import io +import logging import os.path import socket import sys import time -from optparse import OptionParser -import logging +from argparse import ArgumentParser import hl7 +from hl7.exceptions import CLIException SB = b"\x0b" # , vertical tab EB = b"\x1c" # , file separator @@ -18,6 +19,7 @@ log = logging.getLogger(__name__) + class MLLPException(Exception): pass @@ -39,20 +41,22 @@ class MLLPClient(object): with MLLPClient(host, port) as client: client.send_message('MSH|...') - MLLPClient takes an optional ``encoding`` parameter, defaults to UTF-8, - for encoding unicode messages [#]_. + MLLPClient takes optional parameters: - `timeout` in seconds will be used to wait for full response from the server. + * ``encoding``, defaults to UTF-8, for encoding unicode messages [#]_. + * ``timeout`` in seconds, timeout for socket operations [_t]_. + * ``deadline`` in seconds, will be used by the client to determine how long it should wait for full response. .. [#] http://wiki.hl7.org/index.php?title=Character_Set_used_in_v2_messages + .. [_t] https://docs.python.org/3/library/socket.html#socket.socket.settimeout """ def __init__(self, host, port, encoding="utf-8", timeout=10, deadline=3): self.socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) self.socket.connect((host, port)) self.encoding = encoding - self.timeout = timeout # seconds - self.deadline = deadline # seconds + self.timeout = timeout # seconds for socket timeout + self.deadline = deadline # seconds for max time client will wait for a response def __enter__(self): return self @@ -91,24 +95,27 @@ def send(self, data): wrapped in an MLLP container). Blocks until the server returns. """ # upload the data - log.debug(f'sending {(data,)}') + log.debug(f"sending {(data,)}") self.socket.send(data) # wait for the ACK/NACK self.socket.settimeout(self.timeout) - buff = b'' + buff = b"" # the whole message should be received within this deadline - deadline = (time.time() + self.deadline) + deadline = time.time() + self.deadline # This will read data until deadline is reached. # some HL7 counterparts may send responses in chunks, so we should wait some # and read from the socket until we probably receive full message. # timeout/deadline are configurable on client creation. while deadline > time.time(): - data = self.socket.recv(RECV_BUFFER) + try: + data = self.socket.recv(RECV_BUFFER) + except TimeoutError: + data = None if not data: continue buff += data - log.debug(f'received {(buff,)}') + log.debug(f"received {(buff,)}") return buff @@ -143,7 +150,7 @@ def read_stream(stream): if not data: break if isinstance(data, str): - data = data.encode('utf-8') + data = data.encode("utf-8") # usually should be broken up by EB, but I have seen FF separating # messages messages = (_buffer + data).split(EB if FF not in data else FF) @@ -190,95 +197,109 @@ def read_loose(stream): yield START_BLOCK + m -def mllp_send(): +def mllp_send(in_args=None): """Command line tool to send messages to an MLLP server""" # set up the command line options - script_name = os.path.basename(sys.argv[0]) - parser = OptionParser(usage=script_name + " [options] ") - parser.add_option( + in_args = in_args or sys.argv + script_name = os.path.basename(in_args[0]) + parser = ArgumentParser(usage=script_name + " [options] ") + parser.add_argument("host", action="store", nargs=1, help="Host to connect to") + parser.add_argument( "--version", action="store_true", dest="version", - default=False, help="print current version and exit", ) - parser.add_option( + parser.add_argument( "-p", "--port", action="store", - type="int", + type=int, + required=False, dest="port", default=6661, help="port to connect to", ) - parser.add_option( + parser.add_argument( "-f", "--file", + required=False, dest="filename", + default=None, help="read from FILE instead of stdin", metavar="FILE", ) - parser.add_option( + parser.add_argument( "-q", "--quiet", - action="store_true", + action="store_false", dest="verbose", - default=True, + default=True, help="do not print status messages to stdout", ) - parser.add_option( + parser.add_argument( "--loose", action="store_true", dest="loose", - default=False, help=( "allow file to be a HL7-like object (\\r\\n instead " "of \\r). Requires that messages start with " '"MSH|^~\\&|". Requires --file option (no stdin)' ), ) + parser.add_argument( + "--timeout", + action="store", + dest="timeout", + required=False, + default=10, + type=float, + help="number of seconds for socket operations to timeout", + ) + parser.add_argument( + "--deadline", + action="store", + required=False, + dest="deadline", + default=3, + type=float, + help="number of seconds for the client to receive full response", + ) - (options, args) = parser.parse_args() - - if options.version: + args = parser.parse_args(in_args[1:]) + if args.version: import hl7 - stdout(hl7.__version__) return - if len(args) == 1: - host = args[0] - else: - # server not present - parser.print_usage() - stderr().write("server required\n") - sys.exit(1) - return # for testing when sys.exit mocked + host = args.host[0] - if options.filename is not None: + if args.filename is not None: # Previously set stream to the open() handle, but then we did not # close the open file handle. This new approach consumes the entire # file into memory before starting to process, which is not required # or ideal, since we can handle a stream - with open(options.filename, "rb") as f: + with open(args.filename, "rb") as f: stream = io.BytesIO(f.read()) else: - if options.loose: + if args.loose: stderr().write("--loose requires --file\n") - sys.exit(1) - return # for testing when sys.exit mocked + raise CLIException(1) stream = stdin() - with MLLPClient(host, options.port) as client: - message_stream = ( - read_stream(stream) if not options.loose else read_loose(stream) - ) + with MLLPClient( + host, args.port, deadline=args.deadline, timeout=args.timeout + ) as client: + message_stream = read_stream(stream) if not args.loose else read_loose(stream) for message in message_stream: result = client.send_message(message) - if options.verbose: + if args.verbose: stdout(result) if __name__ == "__main__": - mllp_send() + try: + mllp_send(sys.argv) + except CLIException as err: + sys.exit(err.exit_code) diff --git a/hl7/exceptions.py b/hl7/exceptions.py index 8ea37e3..9f90c56 100644 --- a/hl7/exceptions.py +++ b/hl7/exceptions.py @@ -16,3 +16,9 @@ class MalformedFileException(HL7Exception): class ParseException(HL7Exception): pass + + +class CLIException(HL7Exception): + """ An exception to propagate expected exit code from cli script""" + def __init__(self, exit_code): + self.exit_code = exit_code \ No newline at end of file diff --git a/tests/test_client.py b/tests/test_client.py index 15b53aa..aa93677 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1,14 +1,37 @@ import os import socket -from optparse import Values +import typing +import logging from shutil import rmtree from tempfile import mkdtemp from unittest import TestCase -from unittest.mock import Mock, patch +from unittest.mock import patch import hl7 from hl7 import __version__ as hl7_version -from hl7.client import CR, EB, SB, MLLPClient, MLLPException, mllp_send +from hl7.client import CR, EB, MLLPClient, MLLPException, SB, mllp_send +from hl7.exceptions import CLIException + + +log = logging.getLogger(__name__) + +def return_values_list(*values) -> typing.Iterable[typing.Any]: + """ + Generates an iterator, which will return each of values and None after values is depleted. + + This helper is used to mock socket.recv() behavior, when no data from the opposite endpoint may result + in returning None, but still having the socket active. + :param values: + :return: + """ + + def _iter(*_values): + for v in _values: + yield v + while True: + yield None + + return _iter(*values) class MLLPClientTest(TestCase): @@ -17,7 +40,7 @@ def setUp(self): self.socket_patch = patch("hl7.client.socket.socket") self.mock_socket = self.socket_patch.start() - self.client = MLLPClient("localhost", 6666) + self.client = MLLPClient("localhost", 6666, deadline=0.0001) def tearDown(self): # unpatch socket @@ -32,37 +55,39 @@ def test_close(self): self.client.socket.close.assert_called_once_with() def test_send(self): - self.client.socket.recv.return_value = "thanks" + # socket.recv returns bytes: https://docs.python.org/3/library/socket.html#socket.socket.recv + # > Receive data from the socket. The return value is a bytes object representing the data received. + self.client.socket.recv.side_effect = return_values_list(b"thanks") result = self.client.send("foobar\n") - self.assertEqual(result, "thanks") + self.assertEqual(result, b"thanks") self.client.socket.send.assert_called_once_with("foobar\n") - self.client.socket.recv.assert_called_once_with(4096) + self.client.socket.recv.assert_any_call(4096) def test_send_message_unicode(self): - self.client.socket.recv.return_value = "thanks" + self.client.socket.recv.side_effect = return_values_list(b"thanks") result = self.client.send_message("foobar") - self.assertEqual(result, "thanks") + self.assertEqual(result, b"thanks") self.client.socket.send.assert_called_once_with(b"\x0bfoobar\x1c\x0d") def test_send_message_bytestring(self): - self.client.socket.recv.return_value = "thanks" + self.client.socket.recv.side_effect = return_values_list(b"thanks") result = self.client.send_message(b"foobar") - self.assertEqual(result, "thanks") + self.assertEqual(result, b"thanks") self.client.socket.send.assert_called_once_with(b"\x0bfoobar\x1c\x0d") def test_send_message_hl7_message(self): - self.client.socket.recv.return_value = "thanks" + self.client.socket.recv.side_effect = return_values_list(b"thanks") message = hl7.parse(r"MSH|^~\&|GHH LAB|ELAB") result = self.client.send_message(message) - self.assertEqual(result, "thanks") + self.assertEqual(result, b"thanks") self.client.socket.send.assert_called_once_with( b"\x0bMSH|^~\\&|GHH LAB|ELAB\r\x1c\x0d" @@ -89,7 +114,7 @@ def setUp(self): # patch to avoid touching sys and socket self.socket_patch = patch("hl7.client.socket.socket") self.mock_socket = self.socket_patch.start() - self.mock_socket().recv.return_value = "thanks" + self.mock_socket().recv.side_effect = return_values_list(b"thanks") self.stdout_patch = patch("hl7.client.stdout") self.mock_stdout = self.stdout_patch.start() @@ -100,33 +125,32 @@ def setUp(self): self.stderr_patch = patch("hl7.client.stderr") self.mock_stderr = self.stderr_patch.start() - self.exit_patch = patch("hl7.client.sys.exit") + self.exit_patch = patch("argparse.ArgumentParser.exit") + self.exit_patch.side_effect = CLIException(2) self.mock_exit = self.exit_patch.start() # we need a temporary directory self.dir = mkdtemp() self.write(SB + b"foobar" + EB + CR) - self.option_values = Values( - { - "port": 6661, - "filename": os.path.join(self.dir, "test.hl7"), - "verbose": True, - "loose": False, - "version": False, - } - ) - - self.options_patch = patch("hl7.client.OptionParser") - option_parser = self.options_patch.start() - self.mock_options = Mock() - option_parser.return_value = self.mock_options - self.mock_options.parse_args.return_value = (self.option_values, ["localhost"]) + self.option_values = [ + __name__, + "--file", + os.path.join(self.dir, "test.hl7"), + "--port", + "6661", + "--deadline", + "0.0001", + "localhost", + ] + + def _mllp_send(self, args: typing.Optional[typing.List] = None): + log.debug('calling mllp_send with args: ', args or self.option_values) + return mllp_send(args or self.option_values) def tearDown(self): # unpatch self.socket_patch.stop() - self.options_patch.stop() self.stdout_patch.stop() self.stdin_patch.stop() self.stderr_patch.stop() @@ -140,18 +164,18 @@ def write(self, content, path="test.hl7"): f.write(content) def test_send(self): - mllp_send() + self._mllp_send() self.mock_socket().connect.assert_called_once_with(("localhost", 6661)) self.mock_socket().send.assert_called_once_with(SB + b"foobar" + EB + CR) - self.mock_stdout.assert_called_once_with("thanks") + self.mock_stdout.assert_called_once_with(b"thanks") self.assertFalse(self.mock_exit.called) def test_send_multiple(self): - self.mock_socket().recv.return_value = "thanks" + self.mock_socket().recv.side_effect = return_values_list(b"thanks") self.write(SB + b"foobar" + EB + CR + SB + b"hello" + EB + CR) - mllp_send() + self._mllp_send() self.assertEqual( self.mock_socket().send.call_args_list[0][0][0], SB + b"foobar" + EB + CR @@ -163,80 +187,92 @@ def test_send_multiple(self): def test_leftover_buffer(self): self.write(SB + b"foobar" + EB + CR + SB + b"stuff") - self.assertRaises(MLLPException, mllp_send) + self.assertRaises(MLLPException, mllp_send, self.option_values) self.mock_socket().send.assert_called_once_with(SB + b"foobar" + EB + CR) def test_quiet(self): - self.option_values.verbose = False + options = self.option_values.copy() + options.append('--quiet') - mllp_send() + self._mllp_send(options) self.mock_socket().send.assert_called_once_with(SB + b"foobar" + EB + CR) - self.assertFalse(self.mock_stdout.called) + self.assertFalse(self.mock_stdout.called, self.mock_stdout.call_args) def test_port(self): - self.option_values.port = 7890 + # replace default port with some exotic value + options = self.option_values[:4] + ['7890'] + self.option_values[5:] - mllp_send() + self._mllp_send(options) self.mock_socket().connect.assert_called_once_with(("localhost", 7890)) def test_stdin(self): - self.option_values.filename = None + # no filename, just stdin + options = self.option_values.copy() + options = options[:1] + options[3:] + self.mock_stdin.return_value = FakeStream() - mllp_send() + self._mllp_send(options) self.mock_socket().send.assert_called_once_with(SB + b"hello" + EB + CR) def test_loose_no_stdin(self): - self.option_values.loose = True - self.option_values.filename = None + options = self.option_values.copy() + options.append('--loose') + # cut out file path + options = options[:1] + options[3:] self.mock_stdin.return_value = FakeStream() - mllp_send() + self.assertRaises(CLIException, self._mllp_send, options) self.assertFalse(self.mock_socket().send.called) self.mock_stderr().write.assert_called_with("--loose requires --file\n") - self.mock_exit.assert_called_with(1) def test_loose_windows_newline(self): - self.option_values.loose = True + options = self.option_values.copy() + options.append('--loose') + self.write(SB + b"MSH|^~\\&|foo\r\nbar\r\n" + EB + CR) - mllp_send() + self._mllp_send(options) self.mock_socket().send.assert_called_once_with( SB + b"MSH|^~\\&|foo\rbar" + EB + CR ) def test_loose_unix_newline(self): - self.option_values.loose = True + options = self.option_values.copy() + options.append('--loose') + self.write(SB + b"MSH|^~\\&|foo\nbar\n" + EB + CR) - mllp_send() + self._mllp_send(options) self.mock_socket().send.assert_called_once_with( SB + b"MSH|^~\\&|foo\rbar" + EB + CR ) def test_loose_no_mllp_characters(self): - self.option_values.loose = True + options = self.option_values.copy() + options.append('--loose') self.write(b"MSH|^~\\&|foo\r\nbar\r\n") - mllp_send() + self._mllp_send(options) self.mock_socket().send.assert_called_once_with( SB + b"MSH|^~\\&|foo\rbar" + EB + CR ) def test_loose_send_mutliple(self): - self.option_values.loose = True - self.mock_socket().recv.return_value = "thanks" + options = self.option_values.copy() + options.append('--loose') + self.mock_socket().recv.side_effect = return_values_list(b"thanks") self.write(b"MSH|^~\\&|1\r\nOBX|1\r\nMSH|^~\\&|2\r\nOBX|2\r\n") - mllp_send() + self._mllp_send(options) self.assertEqual( self.mock_socket().send.call_args_list[0][0][0], @@ -248,9 +284,11 @@ def test_loose_send_mutliple(self): ) def test_version(self): - self.option_values.version = True - mllp_send() + options = self.option_values + options.append('--version') + + self._mllp_send(options) self.assertFalse(self.mock_socket().connect.called) self.mock_stdout.assert_called_once_with(str(hl7_version))