#92, #93: Make RegexCommand handle AnyStr and return more info

tests/adapters/interpreters/command/test_command_interpreter.py

@@ -1,5 +1,5 @@
 from dataclasses import dataclass
-from typing import Callable, Optional, Sequence
+from typing import Callable, Optional, Sequence, Tuple
 
 import pytest
 from mock import AsyncMock, MagicMock, patch
@@ -12,6 +12,8 @@
     "tickit.adapters.interpreters.command.command_interpreter.get_type_hints"
 )
 
+MOCK_PARSE_RETURN = (("arg1", "arg2"), 1, 2, 2)
+
 
 @pytest.fixture
 def command_interpreter():
@@ -28,9 +30,9 @@ def __call__(self, func: Callable) -> Callable:
             setattr(func, "__command__", self)
             return func
 
-        def parse(self, data: bytes) -> Optional[Sequence[str]]:
+        def parse(self, data: bytes) -> Optional[Tuple[Sequence[str], int, int, int]]:
             if data == self.command:
-                return ("arg1", "arg2")
+                return MOCK_PARSE_RETURN
             else:
                 return None
 
@@ -51,7 +53,7 @@ async def test_command_interpreter_handle_calls_func_with_args(
             __command__=MagicMock(
                 Command,
                 interrupt=False,
-                parse=MagicMock(return_value=("arg1", "arg2")),
+                parse=MagicMock(return_value=MOCK_PARSE_RETURN),
             ),
         ),
     )
@@ -74,7 +76,7 @@ async def test_command_interpreter_handle_returns_iterable_reply(
             __command__=MagicMock(
                 Command,
                 interrupt=False,
-                parse=MagicMock(return_value=("arg1", "arg2")),
+                parse=MagicMock(return_value=MOCK_PARSE_RETURN),
             ),
             return_value=reply,
         ),
@@ -97,7 +99,7 @@ async def test_command_interpreter_handle_wraps_non_iterable_reply(
             __command__=MagicMock(
                 Command,
                 interrupt=False,
-                parse=MagicMock(return_value=("arg1", "arg2")),
+                parse=MagicMock(return_value=MOCK_PARSE_RETURN),
             ),
             return_value=reply,
         ),
@@ -125,7 +127,7 @@ async def test_command_interpreter_handle_returns_interupt(
             __command__=MagicMock(
                 Command,
                 interrupt=interrupt,
-                parse=MagicMock(return_value=("arg1", "arg2")),
+                parse=MagicMock(return_value=MOCK_PARSE_RETURN),
             ),
             return_value="DummyReply",
         ),
@@ -174,3 +176,48 @@ async def test_command_interpreter_handle_returns_message_for_no_commands(
         .__aiter__()
         .__anext__()
     )
+
+
+@patch(
+    _GET_TYPE_HINTS,
+    lambda _: {"arg1": str, "arg2": str},
+)
+@pytest.mark.asyncio
+@pytest.mark.parametrize("match_end, expected_num_awaits", [(2, 0), (3, 1)])
+async def test_command_interpreter_matches_commands_against_full_message(
+    command_interpreter: CommandInterpreter, match_end: int, expected_num_awaits: int
+):
+    test_adapter = MagicMock(
+        Adapter,
+        test_method=AsyncMock(
+            __command__=MagicMock(
+                Command,
+                interrupt=False,
+                parse=MagicMock(return_value=(("arg1", "arg2"), 1, match_end, 3)),
+            ),
+        ),
+    )
+    await command_interpreter.handle(test_adapter, b"\x01")
+    assert test_adapter.test_method.await_count == expected_num_awaits
+
+
+@patch(
+    _GET_TYPE_HINTS,
+    lambda _: {"arg1": int, "arg2": float},
+)
+@pytest.mark.asyncio
+async def test_command_interpreter_converts_args_to_types_correctly(
+    command_interpreter: CommandInterpreter,
+):
+    test_adapter = MagicMock(
+        Adapter,
+        test_method=AsyncMock(
+            __command__=MagicMock(
+                Command,
+                interrupt=False,
+                parse=MagicMock(return_value=(("1", "1.0"), 1, 3, 3)),
+            ),
+        ),
+    )
+    await command_interpreter.handle(test_adapter, b"\x01")
+    assert test_adapter.test_method.awaited_with([int(1), float(1.0)])

tests/adapters/interpreters/command/test_regex_command.py

@@ -50,4 +50,74 @@ def test_regex_command_parse_unmatched_returns_none(
 def test_regex_command_parse_match_returns_args(
     regex_command: RegexCommand, message: bytes, expected: Tuple[object]
 ):
-    assert expected == regex_command.parse(message)
+    result = regex_command.parse(message)
+    assert result is not None
+    args, _, _, _ = result
+    assert expected == args
+
+
+@pytest.mark.parametrize(
+    ["regex", "interrupt", "format", "message", "expected"],
+    [
+        (r"(Test)Message", False, None, r"TestMessage", ("Test",)),
+        (b"\\x01(.)", False, None, b"\x01\x02", (b"\x02",)),
+    ],
+)
+def test_regex_command_parse_can_take_anystr(
+    regex_command: RegexCommand, message: AnyStr, expected
+):
+    result = regex_command.parse(message)
+    assert result is not None
+    args, _, _, _ = result
+    assert expected == args
+
+
+@pytest.mark.parametrize(
+    ["regex", "interrupt", "format", "message", "expected_end_of_match"],
+    [
+        (r"Test", False, None, r"TestMessage", 4),
+        (r"Test", False, "utf-32", "TestMessage".encode("utf-32"), 20),
+    ],
+)
+def test_parse_gives_right_end(
+    regex_command: RegexCommand, message: AnyStr, expected_end_of_match
+):
+    result = regex_command.parse(message)
+    assert result is not None
+    _, _, end, _ = result
+    assert end == expected_end_of_match
+
+
+@pytest.mark.parametrize(
+    ["regex", "interrupt", "format", "message", "expected_end_of_match"],
+    [
+        (r"Test", False, None, r"\nTest\r\n", 6),
+        (r"Test", False, "utf-8", b"\nTest\r\n", 4),
+    ],
+)
+def test_parse_strips_correctly_when_formatting(
+    regex_command: RegexCommand, message: AnyStr, expected_end_of_match
+):
+    result = regex_command.parse(message)
+    assert result is not None
+    _, _, end, _ = result
+    assert end == expected_end_of_match
+
+
+@pytest.mark.parametrize(
+    ["regex", "interrupt", "format", "message", "full_match_expected"],
+    [
+        (r"Test", False, None, r"\nTest\r\n", False),
+        (r"Test", False, "utf-8", b"\nTest\r\n", True),
+        (rb"Test", False, "utf-8", b"\nTest\r\n", False),
+        (rb"Test", False, None, b"\nTest\r\n", False),
+    ],
+)
+def test_parse_gives_correct_whole_matches(
+    regex_command: RegexCommand, message: AnyStr, full_match_expected
+):
+    result = regex_command.parse(message)
+    assert result is not None
+    _, _, end, message_length = result
+    full_match = end == message_length
+    assert full_match == full_match_expected

tickit/adapters/interpreters/command/command_interpreter.py

@@ -1,6 +1,14 @@
 from abc import abstractmethod
 from inspect import getmembers
-from typing import AnyStr, AsyncIterable, Optional, Sequence, Tuple, get_type_hints
+from typing import (
+    AnyStr,
+    AsyncIterable,
+    Optional,
+    Sequence,
+    Tuple,
+    cast,
+    get_type_hints,
+)
 
 from tickit.core.adapter import Adapter, Interpreter
 from tickit.utils.compat.typing_compat import Protocol, runtime_checkable
@@ -12,16 +20,17 @@ class Command(Protocol):
 
     #: A flag which indicates whether calling of the method should trigger an interrupt
     interrupt: bool
+    # format: Optional[str] = None
 
     @abstractmethod
-    def parse(self, data: bytes) -> Optional[Sequence[AnyStr]]:
+    def parse(self, data: AnyStr) -> Optional[Tuple[Sequence[AnyStr], int, int, int]]:
         """An abstract method which parses a message and extracts arguments.
 
         An abstract method which parses a message and produces arguments if a match is
         found, otherwise None is returned.
 
         Args:
-            data (bytes): The message to be parsed.
+            data (AnyStr): The message to be parsed.
 
         Returns:
             Optional[Sequence[AnyStr]]:
@@ -84,16 +93,33 @@ async def handle(
                 indicating whether an interrupt should be raised by the adapter.
         """
         for _, method in getmembers(adapter):
-            command = getattr(method, "__command__", None)
+            message = message
+            command = cast(Command, getattr(method, "__command__", None))
             if command is None:
                 continue
-            args = command.parse(message)
-            if args is None:
+            parse_result = command.parse(message)
+            if parse_result is None:
+                continue
+            (
+                match_groups,
+                _,
+                match_end,
+                message_end,
+            ) = parse_result
+            if match_end != message_end:
+                # We want the whole (formatted) message to match a command
                 continue
             args = (
                 argtype(arg)
-                for arg, argtype in zip(args, get_type_hints(method).values())
+                for arg, argtype in zip(match_groups, get_type_hints(method).values())
             )
+            # can we use signature instead to more cleverly do conversions? i.e. if
+            # type hints aren't available just pass on with no conversion? Otherwise
+            # we can get unhelpful error messages.
+            # Also, should we deal with str/bytes conversions seperately?
+            # Create a mapping function that does conversion rather than argtype(arg)?
+            # It would take args and inspect.Parameters as parameters?
+            # Is type hints enough? - doesn't know about non-hinted vars
             resp = await method(*args)
             if not isinstance(resp, AsyncIterable):
                 resp = CommandInterpreter._wrap(resp)

tickit/adapters/interpreters/command/regex_command.py

@@ -1,6 +1,6 @@
 import re
 from dataclasses import dataclass
-from typing import AnyStr, Callable, Generic, Optional, Sequence
+from typing import AnyStr, Callable, Generic, Optional, Sequence, Tuple, cast
 
 
 @dataclass(frozen=True)
@@ -36,24 +36,78 @@ def __call__(self, func: Callable) -> Callable:
         setattr(func, "__command__", self)
         return func
 
-    def parse(self, data: bytes) -> Optional[Sequence[AnyStr]]:
+    # def parse(self, data: AnyStr) -> Optional[Sequence[AnyStr]]:
+    #     """Performs message decoding and regex matching to match and extract args.
+
+    #     A method which performs message decoding accoridng to the command formatting
+    #     string, checks for a full regular expression match and returns a sequence of
+    #     function arguments if a match is found, otherwise the method returns None.
+
+    #     Args:
+    #         data (bytes): The message data to be parsed.
+
+    #     Returns:
+    #         Optional[Sequence[AnyStr]]:
+    #             If a full match is found a sequence of function arguments is returned,
+    #             otherwise the method returns None.
+    #     """
+    #     if isinstance(data, bytes):
+    #         message = cast(bytes, data)
+    #         message_formatted = (
+    #             message.decode(self.format, "ignore").strip() if self.format else data
+    #         )
+    #     else:
+    #         message_formatted = data
+    #     if isinstance(message_formatted, type(self.regex)):
+    #         match = re.fullmatch(self.regex, message_formatted)
+    #         if match:
+    #             return match.groups()
+    #     return None
+
+    def parse(self, data: AnyStr) -> Optional[Tuple[Sequence[AnyStr], int, int, int]]:
         """Performs message decoding and regex matching to match and extract arguments.
 
         A method which performs message decoding accoridng to the command formatting
-        string, checks for a full regular expression match and returns a sequence of
-        function arguments if a match is found, otherwise the method returns None.
+        string, checks for a regular expression match using re.search. If a match is
+        found, it returns a sequence of function arguments together with some integers
+        giving information about the match, otherwise the method returns None.
 
         Args:
             data (bytes): The message data to be parsed.
 
         Returns:
-            Optional[Sequence[AnyStr]]:
-                If a full match is found a sequence of function arguments is returned,
-                otherwise the method returns None.
+            Optional[Tuple[Sequence[AnyStr], int, int, int]]:
+                If a match is found a sequence of function arguments is returned,
+                together with three integers corresponding to the start and end indices
+                of the match with respect to the (foramtted) data as well as the length
+                of the (formatted) data; otherwise the method returns None.
         """
-        message = data.decode(self.format, "ignore").strip() if self.format else data
-        if isinstance(message, type(self.regex)):
-            match = re.fullmatch(self.regex, message)
-            if match:
-                return match.groups()
+        to_format = (
+            self.format and isinstance(data, bytes) and isinstance(self.regex, str)
+        )
+        if to_format:  # isinstance(data, bytes) and isinstance(self.regex, str):
+            # We only want to format if matching a string pattern against bytes data.
+            # Formatting consists of encoding the pattern and stripping the data of
+            # ascii whitespace.
+            regex_formatted = cast(str, self.regex).encode(
+                cast(str, self.format), "ignore"
+            )
+            regex_formatted = regex_formatted
+            message = data.strip()
+        else:
+            regex_formatted = cast(bytes, self.regex)
+            message = data
+        if isinstance(message, type(regex_formatted)):
+            match = re.search(regex_formatted, message)
+            if match is None:
+                return None
+            match_groups = match.groups()
+            match_start = match.start()
+            match_end = match.end()
+            formatted_message_length = len(message)
+            if to_format:
+                match_groups = tuple(
+                    (group.decode(cast(str, self.format)) for group in match_groups)
+                )
+            return match_groups, match_start, match_end, formatted_message_length
         return None