From aba24b4d91e6493946f578d9ee457f861f1091ad Mon Sep 17 00:00:00 2001 From: Maksim Novikov Date: Sat, 20 Nov 2021 13:18:17 +0100 Subject: [PATCH 1/2] Fix buffer is closed error when using PythonParser class Fixes: #1212 --- CHANGES/1212.bugfix | 1 + CONTRIBUTORS.txt | 1 + aioredis/connection.py | 2 +- tests/conftest.py | 37 +++++++++++++++++++++++++++++++++---- tests/test_connection.py | 12 +++++------- 5 files changed, 41 insertions(+), 12 deletions(-) create mode 100644 CHANGES/1212.bugfix diff --git a/CHANGES/1212.bugfix b/CHANGES/1212.bugfix new file mode 100644 index 000000000..750068ed2 --- /dev/null +++ b/CHANGES/1212.bugfix @@ -0,0 +1 @@ +Fix buffer is closed error when using PythonParser class diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index ffa53e370..144be833d 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -38,6 +38,7 @@ Jeff Moser Joongi Kim Kyrylo Dehtyarenko Leonid Shvechikov +Maksim Novikov Manuel Miranda Marek Szapiel Marijn Giesen diff --git a/aioredis/connection.py b/aioredis/connection.py index 55585ed13..a368d2d8c 100644 --- a/aioredis/connection.py +++ b/aioredis/connection.py @@ -373,7 +373,7 @@ def __init__(self, socket_read_size: int): def on_connect(self, connection: "Connection"): """Called when the stream connects""" self._stream = connection._reader - if self._buffer is None or self._stream is None: + if self._stream is None: raise RedisError("Buffer is closed.") self._buffer = SocketBuffer( diff --git a/tests/conftest.py b/tests/conftest.py index 4ef9da6c2..9d8520545 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,7 +8,12 @@ import aioredis from aioredis.client import Monitor -from aioredis.connection import parse_url +from aioredis.connection import ( + HIREDIS_AVAILABLE, + HiredisParser, + PythonParser, + parse_url, +) from .compat import mock @@ -125,16 +130,40 @@ def skip_unless_arch_bits(arch_bits): ) -@pytest.fixture(params=[True, False], ids=["single", "pool"]) +@pytest.fixture( + params=[ + (True, PythonParser), + (False, PythonParser), + pytest.param( + (True, HiredisParser), + marks=pytest.mark.skipif( + not HIREDIS_AVAILABLE, reason="hiredis is not installed" + ), + ), + pytest.param( + (False, HiredisParser), + marks=pytest.mark.skipif( + not HIREDIS_AVAILABLE, reason="hiredis is not installed" + ), + ), + ], + ids=[ + "single-python-parser", + "pool-python-parser", + "single-hiredis", + "pool-hiredis", + ], +) def create_redis(request, event_loop): """Wrapper around aioredis.create_redis.""" - single_connection = request.param + single_connection, parser_cls = request.param async def f(url: str = request.config.getoption("--redis-url"), **kwargs): single = kwargs.pop("single_connection_client", False) or single_connection + parser_class = kwargs.pop("parser_class", None) or parser_cls url_options = parse_url(url) url_options.update(kwargs) - pool = aioredis.ConnectionPool(**url_options) + pool = aioredis.ConnectionPool(parser_class=parser_class, **url_options) client: aioredis.Redis = aioredis.Redis(connection_pool=pool) if single: client = client.client() diff --git a/tests/test_connection.py b/tests/test_connection.py index df5da7ad1..b71847c4c 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -4,17 +4,15 @@ import pytest -from aioredis.connection import UnixDomainSocketConnection +from aioredis.connection import PythonParser, UnixDomainSocketConnection from aioredis.exceptions import InvalidResponse -from aioredis.utils import HIREDIS_AVAILABLE -if TYPE_CHECKING: - from aioredis.connection import PythonParser - -@pytest.mark.skipif(HIREDIS_AVAILABLE, reason="PythonParser only") @pytest.mark.asyncio -async def test_invalid_response(r): +@pytest.mark.parametrize("create_redis", [(True, PythonParser)], indirect=True) +async def test_invalid_response(create_redis): + r = await create_redis() + raw = b"x" parser: "PythonParser" = r.connection._parser with mock.patch.object(parser._buffer, "readline", return_value=raw): From b00926de961709221f73750fa16ac18a15a8bf63 Mon Sep 17 00:00:00 2001 From: Maksim Novikov Date: Sat, 20 Nov 2021 21:05:46 +0100 Subject: [PATCH 2/2] Fix test_invalid_response on older python versions without AsyncMock --- tests/test_connection.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/test_connection.py b/tests/test_connection.py index b71847c4c..399f3aed7 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1,12 +1,13 @@ import asyncio from typing import TYPE_CHECKING -from unittest import mock import pytest from aioredis.connection import PythonParser, UnixDomainSocketConnection from aioredis.exceptions import InvalidResponse +from .compat import mock + @pytest.mark.asyncio @pytest.mark.parametrize("create_redis", [(True, PythonParser)], indirect=True) @@ -14,8 +15,10 @@ async def test_invalid_response(create_redis): r = await create_redis() raw = b"x" + readline_mock = mock.AsyncMock(return_value=raw) + parser: "PythonParser" = r.connection._parser - with mock.patch.object(parser._buffer, "readline", return_value=raw): + with mock.patch.object(parser._buffer, "readline", readline_mock): with pytest.raises(InvalidResponse) as cm: await parser.read_response() assert str(cm.value) == "Protocol Error: %r" % raw