diff --git a/History.rst b/History.rst index 45a59da..ce1176b 100644 --- a/History.rst +++ b/History.rst @@ -1,6 +1,38 @@ History ======= +Draft v2.0.0 / Unreleased +------------------------- + + * **Breaking change**: Remove ``Response::body``'s ``binary`` parameter and enforce a keyword argument for ``chunked`` + + * The ``binary`` parameter is no longer needed since responses are now always byte-encoded in all cases (see below). + * A keyword argument is now enforced for ``chunked`` to ensure unnamed arguments meant for the removed ``binary`` parameter cannot be confused as ``chunked`` arguments. + + * Only return byte-encoded response bodies, matching the bahviour of all supported client libraries + + * This is possible because for all supported client libraries, pook mocks the actual response sent to the + client library over the wire, which will, in all cases, be bytes. Client libraries that support reading + response content as a string or other formats will continue to work as expected, because they'll always + be handling bytes from pook. + * This causes no change to application code for users of pook, except for users of the standard library ``urllib``, + for which this also fixed a bug where non-bytes bodies were returned by pook in certain cases. This is impossible + in real application code. If you rely on pook to mock ``urllib`` responses and have code that handles non-bytes response + bodies, that code can be safely deleted (provided the only reason it was there was pook in the first place). + + * **Breaking change**: Remove ``Mock::body``'s ``binary`` parameter. + + * This parameter was already unused due to a bug in the code (it was not passed through to the ``BodyMatcher``), + so this will not cause any changes to tests that relied on it: it didn't do anything to begin with. + * The breaking change is simply the removal of the unused parameter, which should be deleted from tests using pook. + * Pook's code has also been updated to remove all cases where non-bytes objects were being handled. Instead, the body + of any interecepted request will always be stored as bytes, and only decoded when necessary for individual downstream + matchers (JSON, XML). + + * Correct documentation strings for ``XMLMatcher`` and ``JSONMatcher`` to no longer suggest they can handle regex matchers + + * These classes never implemented regex matching + v1.4.3 / 2024-02-23 ------------------- diff --git a/src/pook/assertion.py b/src/pook/assertion.py index 152f1e6..d288702 100644 --- a/src/pook/assertion.py +++ b/src/pook/assertion.py @@ -50,6 +50,9 @@ def matches(x, y, regex_expr=False): # Parse regex expression, if needed x = strip_regex(x) if regex_expr and isregex_expr(x) else x + if isinstance(getattr(x, "pattern", None), str) and hasattr(y, "decode"): + y = y.decode("utf-8", "backslashescape") + # Assert regular expression via unittest matchers return test_case().assertRegex(y, x) or True diff --git a/src/pook/interceptors/aiohttp.py b/src/pook/interceptors/aiohttp.py index 446c57f..21ec716 100644 --- a/src/pook/interceptors/aiohttp.py +++ b/src/pook/interceptors/aiohttp.py @@ -115,11 +115,7 @@ async def _on_request( _res._headers = multidict.CIMultiDictProxy(multidict.CIMultiDict(headers)) if res._body: - _res.content = SimpleContent( - res._body - if res._binary - else res._body.encode("utf-8", errors="replace"), - ) + _res.content = SimpleContent(res._body) else: # Define `_content` attribute with an empty string to # force do not read from stream (which won't exists) diff --git a/src/pook/interceptors/http.py b/src/pook/interceptors/http.py index 6645705..36de1dd 100644 --- a/src/pook/interceptors/http.py +++ b/src/pook/interceptors/http.py @@ -92,7 +92,7 @@ def getresponse(): # Path reader def read(): - return res._body or "" + return res._body or b"" mockres.read = read diff --git a/src/pook/interceptors/urllib3.py b/src/pook/interceptors/urllib3.py index a9ca23d..aa68aea 100644 --- a/src/pook/interceptors/urllib3.py +++ b/src/pook/interceptors/urllib3.py @@ -152,9 +152,6 @@ def _on_request( if is_chunked_response(headers): body_chunks = body if isinstance(body, list) else [body] - body_chunks = [ - chunk if res._binary else chunk.encode() for chunk in body_chunks - ] body = ClientHTTPResponse(MockSock) body.fp = FakeChunkedResponseBody(body_chunks) diff --git a/src/pook/matchers/body.py b/src/pook/matchers/body.py index 657f581..50a5eff 100644 --- a/src/pook/matchers/body.py +++ b/src/pook/matchers/body.py @@ -7,16 +7,6 @@ class BodyMatcher(BaseMatcher): regular expression based matching. """ - def __init__(self, *args, **kwargs): - self.binary = kwargs.pop("binary", False) - super().__init__(*args, **kwargs) - @BaseMatcher.matcher def match(self, req): - expectation = self.expectation - - # Decode bytes input - if isinstance(expectation, bytes) and not self.binary: - expectation = expectation.decode("utf-8") - return self.compare(self.expectation, req.body) diff --git a/src/pook/matchers/json.py b/src/pook/matchers/json.py index c360b41..98e411d 100644 --- a/src/pook/matchers/json.py +++ b/src/pook/matchers/json.py @@ -1,11 +1,26 @@ import json from .base import BaseMatcher +from ..assertion import equal class JSONMatcher(BaseMatcher): """ - JSONMatcher implements a JSON body matcher supporting strict structure - and regular expression based comparisons. + Match JSON documents of equivalent value. + + JSON documents are matched on the structured data in the document, + rather than on the strict organisation of the document. + + The following two JSON snippets are treated as identical by this matcher: + + {"a": "one", "b": ["two"]} + + ... is considered idential to ... + + {"b": ["two"], "a": "one"} + + In other words, the order does not matter in comparison. + + Use ``BodyMatcher`` to strictly match the exact textual structure. """ def __init__(self, data): @@ -16,15 +31,7 @@ def __init__(self, data): @BaseMatcher.matcher def match(self, req): - body = req.body - - if isinstance(body, str): - try: - body = json.loads(body) - except Exception: - return False - x = json.dumps(self.expectation, sort_keys=True, indent=4) - y = json.dumps(body, sort_keys=True, indent=4) + y = json.dumps(req.json, sort_keys=True, indent=4) - return self.compare(x, y) + return equal(x, y) diff --git a/src/pook/matchers/json_schema.py b/src/pook/matchers/json_schema.py index d33bf7c..d1749e7 100644 --- a/src/pook/matchers/json_schema.py +++ b/src/pook/matchers/json_schema.py @@ -17,19 +17,13 @@ def __init__(self, schema): @BaseMatcher.matcher def match(self, req): - body = req.body + req_json = req.json - if isinstance(body, str): - try: - body = json.loads(body) - except Exception: - return False - - if not body: + if not req_json: return False try: - validate(body, self.expectation) + validate(req_json, self.expectation) except Exception: return False diff --git a/src/pook/matchers/xml.py b/src/pook/matchers/xml.py index 65abbdd..707f4f6 100644 --- a/src/pook/matchers/xml.py +++ b/src/pook/matchers/xml.py @@ -1,12 +1,29 @@ import json import xmltodict from .base import BaseMatcher +from ..assertion import equal class XMLMatcher(BaseMatcher): """ - XMLMatcher implements a XML body matcher supporting both strict structure - comparison and regular expression. + Match XML documents of equivalent structural value. + + XML documents are matched on the structured data in the document, + rather than on the strict organisation of the document. + + The following two XML snippets are treated as identical by this matcher: + + + two + + ... is considered idential to ... + + two + + + In other words, the order does not matter in comparison. + + Use ``BodyMatcher`` to strictly match the exact textual structure. """ def __init__(self, data): @@ -19,13 +36,13 @@ def compare(self, data): x = json.dumps(xmltodict.parse(data), sort_keys=True) y = json.dumps(self.expectation, sort_keys=True) - return x == y + return equal(x, y) @BaseMatcher.matcher def match(self, req): - data = req.body + xml = req.xml - if not isinstance(data, str): + if not isinstance(xml, str): return False - return self.compare(data) + return self.compare(xml) diff --git a/src/pook/mock.py b/src/pook/mock.py index ae606d0..bfba6eb 100644 --- a/src/pook/mock.py +++ b/src/pook/mock.py @@ -396,21 +396,23 @@ def params(self, params): self.add_matcher(matcher("QueryMatcher", params)) return self - def body(self, body, binary=False): + def body(self, body): """ Defines the body data to match. - ``body`` argument can be a ``str``, ``binary`` or a regular expression. + ``body`` argument can be a ``str``, ``bytes`` or a regular expression. Arguments: - body (str|binary|regex): body data to match. - binary (bool): prevent decoding body as text when True. + body (str|bytes|regex): body data to match. Returns: self: current Mock instance. """ + if hasattr(body, "encode"): + body = body.encode("utf-8", "backslashreplace") + self._request.body = body - self.add_matcher(matcher("BodyMatcher", body, binary=False)) + self.add_matcher(matcher("BodyMatcher", body)) return self def json(self, json): @@ -468,9 +470,8 @@ def file(self, path): Returns: self: current Mock instance. """ - with open(path, "r") as f: - self.body(str(f.read())) - return self + with open(path, "rb") as f: + return self.body(f.read()) def add_matcher(self, matcher): """ diff --git a/src/pook/request.py b/src/pook/request.py index e4f7279..0fe0178 100644 --- a/src/pook/request.py +++ b/src/pook/request.py @@ -18,7 +18,7 @@ class Request(object): headers (dict): HTTP headers to match. query (dict): URL query params to match. Complementely to URL defined query params. - body (str|regex): request body payload to match. + body (bytes|regex): request body payload to match. json (str|dict|list): JSON payload body structure to match. xml (str): XML payload data structure to match. @@ -28,13 +28,13 @@ class Request(object): headers (dict): HTTP headers to match. query (dict): URL query params to match. Complementely to URL defined query params. - body (str|regex): request body payload to match. + body (bytes|regex): request body payload to match. json (str|dict|list): JSON payload body structure to match. xml (str): XML payload data structure to match. """ # Store keys - keys = ("method", "headers", "body", "url", "query") + keys = ("method", "headers", "body", "url", "query", "xml", "json") def __init__(self, method="GET", **kw): self._url = None @@ -111,32 +111,29 @@ def body(self): @body.setter def body(self, body): - if hasattr(body, "decode"): - try: - body = body.decode("utf-8", "strict") - except Exception: - pass + if hasattr(body, "encode"): + body = body.encode("utf-8", "backslashreplace") self._body = body @property def json(self): - return _json.loads(self._body) + return _json.loads(self.body.decode("utf-8")) @json.setter def json(self, data): if isinstance(data, str): - self._body = data + self.body = data else: - self._body = _json.dumps(data) + self.body = _json.dumps(data) @property def xml(self): - return self._body + return self.body.decode("utf-8") @xml.setter def xml(self, data): - self._body = data + self.body = data def copy(self): """ diff --git a/src/pook/response.py b/src/pook/response.py index 9effd56..398439d 100644 --- a/src/pook/response.py +++ b/src/pook/response.py @@ -41,7 +41,6 @@ def __init__(self, **kw): self._status = 200 self._mock = None self._body = None - self._binary = None self._headers = HTTPHeaderDict() # Trigger response method based on input arguments @@ -131,8 +130,7 @@ def type(self, name): Returns: self: ``pook.Response`` current instance. """ - self.content(name) - return self + return self.content(name) def content(self, name): """ @@ -158,23 +156,25 @@ def content(self, name): self._headers["Content-Type"] = TYPES.get(name, name) return self - def body(self, body, binary=False, chunked=False): + def body(self, body, *, chunked=False): """ Defines response body data. Arguments: body (str|bytes|list): response body to use. - binary (bool): prevent decoding the body as text when True. chunked (bool): return a chunked response. Returns: self: ``pook.Response`` current instance. """ - if isinstance(body, bytes) and not binary: - body = body.decode("utf-8") + if hasattr(body, "encode"): + body = body.encode("utf-8", "backslashreplace") + elif isinstance(body, list): + for i, chunk in enumerate(body): + if hasattr(chunk, "encode"): + body[i] = chunk.encode("utf-8", "backslashreplace") self._body = body - self._binary = binary if chunked: self.header("Transfer-Encoding", "chunked") @@ -193,8 +193,8 @@ def json(self, data): self._headers["Content-Type"] = "application/json" if not isinstance(data, str): data = json.dumps(data, indent=4) - self._body = data - return self + + return self.body(data) def xml(self, xml): """ @@ -208,8 +208,7 @@ def xml(self, xml): Returns: self: ``pook.Response`` current instance. """ - self.body(xml) - return self + return self.body(xml) def file(self, path): """ @@ -221,9 +220,8 @@ def file(self, path): Returns: self: ``pook.Response`` current instance. """ - with open(path, "r") as f: - self._body = str(f.read()) - return self + with open(path, "rb") as f: + return self.body(f.read()) @property def mock(self): diff --git a/tests/unit/fixtures/__init__.py b/tests/unit/fixtures/__init__.py new file mode 100644 index 0000000..e53322b --- /dev/null +++ b/tests/unit/fixtures/__init__.py @@ -0,0 +1,5 @@ +from pathlib import Path + + +BINARY_FILE_PATH = Path(__file__).parent / "nothing.bin" +BINARY_FILE = BINARY_FILE_PATH.read_bytes() diff --git a/tests/unit/interceptors/aiohttp_test.py b/tests/unit/interceptors/aiohttp_test.py index a29009a..b43c489 100644 --- a/tests/unit/interceptors/aiohttp_test.py +++ b/tests/unit/interceptors/aiohttp_test.py @@ -1,11 +1,10 @@ -from pathlib import Path - import pytest import aiohttp import pook from tests.unit.interceptors.base import StandardTests +from tests.unit.fixtures import BINARY_FILE pytestmark = [pytest.mark.pook] @@ -18,10 +17,7 @@ async def amake_request(self, method, url): async with aiohttp.ClientSession(loop=self.loop) as session: req = await session.request(method=method, url=url) content = await req.read() - return req.status, content.decode("utf-8") - - -binary_file = (Path(__file__).parents[1] / "fixtures" / "nothing.bin").read_bytes() + return req.status, content def _pook_url(URL): @@ -55,7 +51,7 @@ async def test_await_request(URL): @pytest.mark.asyncio async def test_binary_body(URL): - pook.get(URL).reply(200).body(binary_file, binary=True) + pook.get(URL).reply(200).body(BINARY_FILE) async with aiohttp.ClientSession() as session: req = await session.get(URL) - assert await req.read() == binary_file + assert await req.read() == BINARY_FILE diff --git a/tests/unit/interceptors/base.py b/tests/unit/interceptors/base.py index ee5effb..3d0cfb4 100644 --- a/tests/unit/interceptors/base.py +++ b/tests/unit/interceptors/base.py @@ -9,12 +9,12 @@ class StandardTests: is_async: bool = False - async def amake_request(self, method: str, url: str) -> Tuple[int, Optional[str]]: + async def amake_request(self, method: str, url: str) -> Tuple[int, Optional[bytes]]: raise NotImplementedError( "Sub-classes for async transports must implement `amake_request`" ) - def make_request(self, method: str, url: str) -> Tuple[int, Optional[str]]: + def make_request(self, method: str, url: str) -> Tuple[int, Optional[bytes]]: if self.is_async: return self.loop.run_until_complete(self.amake_request(method, url)) @@ -37,7 +37,7 @@ def test_activate_deactivate(self, httpbin): status, body = self.make_request("GET", url) assert status == 200 - assert body == "hello from pook" + assert body == b"hello from pook" pook.disable() diff --git a/tests/unit/interceptors/httpx_test.py b/tests/unit/interceptors/httpx_test.py index c9acc45..ac8c695 100644 --- a/tests/unit/interceptors/httpx_test.py +++ b/tests/unit/interceptors/httpx_test.py @@ -17,14 +17,14 @@ async def amake_request(self, method, url): async with httpx.AsyncClient() as client: response = await client.request(method=method, url=url) content = await response.aread() - return response.status_code, content.decode("utf-8") + return response.status_code, content class TestStandardSyncHttpx(StandardTests): def make_request(self, method, url): response = httpx.request(method=method, url=url) content = response.read() - return response.status_code, content.decode("utf-8") + return response.status_code, content @pytest.fixture @@ -41,7 +41,7 @@ def test_sync(URL): async def test_async(URL): - pook.get(URL).times(1).reply(200).body(b"async_body", binary=True).mock + pook.get(URL).times(1).reply(200).body(b"async_body").mock async with httpx.AsyncClient() as client: response = await client.get(URL) diff --git a/tests/unit/interceptors/urllib3_test.py b/tests/unit/interceptors/urllib3_test.py index 78ead1a..78357cf 100644 --- a/tests/unit/interceptors/urllib3_test.py +++ b/tests/unit/interceptors/urllib3_test.py @@ -2,19 +2,16 @@ import pook import pytest -from pathlib import Path from tests.unit.interceptors.base import StandardTests - - -binary_file = (Path(__file__).parents[1] / "fixtures" / "nothing.bin").read_bytes() +from tests.unit.fixtures import BINARY_FILE class TestStandardUrllib3(StandardTests): def make_request(self, method, url): http = urllib3.PoolManager() response = http.request(method, url) - return response.status, response.read().decode("utf-8") + return response.status, response.read() @pytest.fixture @@ -32,20 +29,19 @@ def assert_chunked_response(URL, input_data, expected): assert r.status == 204 chunks = list(r.read_chunked()) - chunks = [c.decode() if isinstance(c, bytes) else c for c in chunks] assert chunks == expected def test_chunked_response_list(URL): - assert_chunked_response(URL, ["a", "b", "c"], ["a", "b", "c"]) + assert_chunked_response(URL, ["a", "b", "c"], [b"a", b"b", b"c"]) def test_chunked_response_str(URL): - assert_chunked_response(URL, "text", ["text"]) + assert_chunked_response(URL, "text", [b"text"]) def test_chunked_response_byte(URL): - assert_chunked_response(URL, b"byteman", ["byteman"]) + assert_chunked_response(URL, b"byteman", [b"byteman"]) def test_chunked_response_empty(URL): @@ -53,7 +49,7 @@ def test_chunked_response_empty(URL): def test_chunked_response_contains_newline(URL): - assert_chunked_response(URL, "newline\r\n", ["newline\r\n"]) + assert_chunked_response(URL, "newline\r\n", [b"newline\r\n"]) def test_activate_disable(): @@ -68,22 +64,22 @@ def test_activate_disable(): @pook.on def test_binary_body(URL): - (pook.get(URL).reply(200).body(binary_file, binary=True)) + (pook.get(URL).reply(200).body(BINARY_FILE)) http = urllib3.PoolManager() r = http.request("GET", URL) - assert r.read() == binary_file + assert r.read() == BINARY_FILE @pook.on def test_binary_body_chunked(URL): - (pook.get(URL).reply(200).body(binary_file, binary=True, chunked=True)) + (pook.get(URL).reply(200).body(BINARY_FILE, chunked=True)) http = urllib3.PoolManager() r = http.request("GET", URL) - assert list(r.read_chunked()) == [binary_file] + assert list(r.read_chunked()) == [BINARY_FILE] @pytest.mark.pook diff --git a/tests/unit/interceptors/urllib_test.py b/tests/unit/interceptors/urllib_test.py index 901b4c3..055ad0a 100644 --- a/tests/unit/interceptors/urllib_test.py +++ b/tests/unit/interceptors/urllib_test.py @@ -25,7 +25,7 @@ def test_urllib_ssl(): pook.get("https://example.com").reply(200).body("Hello from pook") res = urlopen("https://example.com") - assert res.read() == "Hello from pook" + assert res.read() == b"Hello from pook" @pytest.mark.pook @@ -33,4 +33,4 @@ def test_urllib_clear(): pook.get("http://example.com").reply(200).body("Hello from pook") res = urlopen("http://example.com") - assert res.read() == "Hello from pook" + assert res.read() == b"Hello from pook" diff --git a/tests/unit/matchers/body_test.py b/tests/unit/matchers/body_test.py deleted file mode 100644 index e69de29..0000000 diff --git a/tests/unit/matchers/helpers_test.py b/tests/unit/matchers/helpers_test.py deleted file mode 100644 index e69de29..0000000 diff --git a/tests/unit/matchers/json_schema_test.py b/tests/unit/matchers/json_schema_test.py deleted file mode 100644 index e69de29..0000000 diff --git a/tests/unit/matchers/json_test.py b/tests/unit/matchers/json_test.py deleted file mode 100644 index e69de29..0000000 diff --git a/tests/unit/matchers/method_test.py b/tests/unit/matchers/method_test.py deleted file mode 100644 index e69de29..0000000 diff --git a/tests/unit/matchers/path_test.py b/tests/unit/matchers/path_test.py deleted file mode 100644 index e69de29..0000000 diff --git a/tests/unit/mock_test.py b/tests/unit/mock_test.py index 856a5e7..fb702ae 100644 --- a/tests/unit/mock_test.py +++ b/tests/unit/mock_test.py @@ -1,5 +1,8 @@ import pytest import json +import itertools +from textwrap import dedent +import re import pook from pook.mock import Mock @@ -7,6 +10,8 @@ from pook.exceptions import PookNoMatches from urllib.request import urlopen +from tests.unit.fixtures import BINARY_FILE, BINARY_FILE_PATH + @pytest.fixture def mock(): @@ -148,10 +153,125 @@ def test_times_integrated(httpbin): pook.get(url).times(2).reply(200).body("hello from pook") res = urlopen(url) - assert res.read() == "hello from pook" + assert res.read() == b"hello from pook" res = urlopen(url) - assert res.read() == "hello from pook" + assert res.read() == b"hello from pook" with pytest.raises(PookNoMatches, match="Mock matches request but is expired."): urlopen(url) + + +def test_file_matches(httpbin, mock): + mock.file(BINARY_FILE_PATH) + + req = Request( + url=httpbin.url, + body=BINARY_FILE, + ) + + assert mock.match(req) == (True, []) + + +def test_file_not_matches(httpbin, mock): + mock.file(BINARY_FILE_PATH) + + req = Request( + url=httpbin.url, + body=b"not the binary file you're looking for!", + ) + + matches, errors = mock.match(req) + assert not matches + assert len(errors) == 1 + assert errors[0].startswith("BodyMatcher") + + +@pytest.mark.parametrize( + "mock_body, request_body", + # Both sides will always match, regardless of str/bytes + list(itertools.product((b"hello", "hello"), (b"hello", "hello"))), +) +def test_body_matches(httpbin, mock, mock_body, request_body): + mock.body(mock_body) + req = Request(url=httpbin.url, body=request_body) + + assert mock.match(req) == (True, []) + + +@pytest.mark.parametrize( + "mock_body, request_body", + # Neither left nor right side will ever match due to differing contents + list(itertools.product((b"bytes", "str"), (b"hello bytes", "hello str"))), +) +def test_body_not_matches(httpbin, mock, mock_body, request_body): + mock.body(mock_body) + req = Request(url=httpbin.url, body=request_body) + + matches, errors = mock.match(req) + assert not matches + assert len(errors) == 1 + assert errors[0].startswith("BodyMatcher: ") + + +def test_body_matches_string_regex(httpbin, mock): + mock.body(re.compile(r"hello, me!")) + req = Request( + url=httpbin.url, + body="This is a big sentence... hello, me! wow, another part", + ) + + assert mock.match(req) == (True, []) + + +def test_body_matches_bytes_regex(httpbin, mock): + mock.body(re.compile(rb"hello, me!")) + req = Request( + url=httpbin.url, + body="This is a big sentence... hello, me! wow, another part", + ) + + assert mock.match(req) == (True, []) + + +def test_xml_matches(httpbin, mock): + xml = dedent( + """ + + + + A pook test for XML! + + """ + ).strip() + mock.xml(xml) + + req = Request( + url=httpbin.url, + xml=xml, + ) + + assert mock.match(req) == (True, []) + + +def test_xml_not_matches(httpbin, mock): + xml = dedent( + """ + + + + A pook test for XML! + + """ + ).strip() + mock.xml(xml) + + req = Request( + url=httpbin.url, + xml=xml.replace("A pook test for XML!", "Not this one!"), + ) + + matches, errors = mock.match(req) + assert not matches + assert len(errors) == 1 + assert errors[0].startswith("XMLMatcher:")