From 869bfc45acbc0b69aa7eb8537cc891c2d8660897 Mon Sep 17 00:00:00 2001 From: Ivan Golikov Date: Wed, 8 Jan 2025 21:44:41 +0100 Subject: [PATCH 1/4] Added support for all Redis versions (>=1.0.0) Previously support was provided for Redis>=6.2.0 --- pssecret_server/utils.py | 26 ++++++++++++++++++++++++ pyproject.toml | 3 +++ tests/integration/test_utils.py | 36 +++++++++++++++++++++++++++++++-- tests/unit/test_utils.py | 22 +++++++++++++++++++- 4 files changed, 84 insertions(+), 3 deletions(-) diff --git a/pssecret_server/utils.py b/pssecret_server/utils.py index e57b710..0fe0b23 100644 --- a/pssecret_server/utils.py +++ b/pssecret_server/utils.py @@ -1,7 +1,10 @@ +from functools import lru_cache from uuid import uuid4 from cryptography.fernet import Fernet from redis.asyncio import Redis +from redis.exceptions import ResponseError +from redis.typing import ResponseT from pssecret_server.models import Secret @@ -30,3 +33,26 @@ async def save_secret(data: Secret, redis: Redis) -> str: await redis.setex(new_key, 60 * 60 * 24, data.data) return new_key + + +@lru_cache +async def _is_getdel_available(redis: Redis) -> bool: + """GETDEL is not available in Redis prior to version 6.2""" + try: + await redis.getdel("test:getdel:availability") + except ResponseError: + return False + + return True + + +async def getdel(redis: Redis, key: str) -> ResponseT: + result: ResponseT + + if await _is_getdel_available(redis): + result = await redis.getdel(key) + else: + result = await redis.getset(key, "") + await redis.delete(key) + + return result diff --git a/pyproject.toml b/pyproject.toml index 3e746fb..8dc6c71 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -53,3 +53,6 @@ reportUnusedCallResult = "none" [tool.pytest.ini_options] asyncio_mode = "auto" + +[tool.isort] +profile = "black" diff --git a/tests/integration/test_utils.py b/tests/integration/test_utils.py index bef19c4..0d9290d 100644 --- a/tests/integration/test_utils.py +++ b/tests/integration/test_utils.py @@ -1,8 +1,8 @@ -from unittest.mock import patch +from unittest.mock import AsyncMock, Mock, patch from redis.asyncio import Redis -from pssecret_server.utils import get_new_key, save_secret +from pssecret_server.utils import get_new_key, getdel, save_secret from ..factories import SecretFactory @@ -33,3 +33,35 @@ async def test_save_secret_data(redis_server: Redis) -> None: assert redis_data is not None assert redis_data.decode() == secret.data + + +@patch("pssecret_server.utils._is_getdel_available", side_effect=AsyncMock()) +async def test_getdel_when_available( + is_getdel_available: Mock, redis_server: Redis +) -> None: + is_getdel_available.side_effect.return_value = True + + test_value = "test_data" + test_key = "test_key" + await redis_server.set(test_key, test_value) + + result = await getdel(redis_server, test_key) + + assert result.decode() == test_value + assert not await redis_server.exists(test_key) + + +@patch("pssecret_server.utils._is_getdel_available", side_effect=AsyncMock()) +async def test_getdel_when_not_available( + is_getdel_available: Mock, redis_server: Redis +) -> None: + is_getdel_available.side_effect.return_value = False + + test_value = "test_data" + test_key = "test_key" + await redis_server.set(test_key, test_value) + + result = await getdel(redis_server, test_key) + + assert result.decode() == test_value + assert not await redis_server.exists(test_key) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index de2984b..321bb40 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -1,7 +1,10 @@ +from unittest.mock import AsyncMock + import pytest from cryptography.fernet import Fernet, InvalidToken +from redis.exceptions import ResponseError -from pssecret_server.utils import decrypt_secret, encrypt_secret +from pssecret_server.utils import _is_getdel_available, decrypt_secret, encrypt_secret from ..factories import SecretFactory @@ -29,3 +32,20 @@ def test_secret_is_not_decryptable_by_random_key(fernet: Fernet): with pytest.raises(InvalidToken): decrypt_secret(encrypted_secret.data.encode(), random_fernet) + + +async def test_is_getdel_available_when_supported(): + redis = AsyncMock() + + result = await _is_getdel_available(redis) + + assert result is True + + +async def test_is_getdel_available_when_not_supported(): + redis = AsyncMock() + redis.getdel.side_effect = ResponseError + + result = await _is_getdel_available(redis) + + assert result is False -- 2.43.0 From fcefa41956b96addf55a60cdcb97f6110076f816 Mon Sep 17 00:00:00 2001 From: Ivan Golikov Date: Wed, 8 Jan 2025 22:12:21 +0100 Subject: [PATCH 2/4] Combining test cases --- tests/integration/test_utils.py | 30 +++++++++--------------------- tests/unit/test_utils.py | 19 ++++++++----------- 2 files changed, 17 insertions(+), 32 deletions(-) diff --git a/tests/integration/test_utils.py b/tests/integration/test_utils.py index 0d9290d..03ae701 100644 --- a/tests/integration/test_utils.py +++ b/tests/integration/test_utils.py @@ -1,5 +1,6 @@ -from unittest.mock import AsyncMock, Mock, patch +from unittest.mock import AsyncMock, patch +import pytest from redis.asyncio import Redis from pssecret_server.utils import get_new_key, getdel, save_secret @@ -35,27 +36,14 @@ async def test_save_secret_data(redis_server: Redis) -> None: assert redis_data.decode() == secret.data -@patch("pssecret_server.utils._is_getdel_available", side_effect=AsyncMock()) -async def test_getdel_when_available( - is_getdel_available: Mock, redis_server: Redis +@pytest.mark.parametrize("getdel_available", [True, False]) +@patch("pssecret_server.utils._is_getdel_available", new_callable=AsyncMock) +async def test_getdel( + mock_is_getdel_available: AsyncMock, + getdel_available: bool, + redis_server: Redis, ) -> None: - is_getdel_available.side_effect.return_value = True - - test_value = "test_data" - test_key = "test_key" - await redis_server.set(test_key, test_value) - - result = await getdel(redis_server, test_key) - - assert result.decode() == test_value - assert not await redis_server.exists(test_key) - - -@patch("pssecret_server.utils._is_getdel_available", side_effect=AsyncMock()) -async def test_getdel_when_not_available( - is_getdel_available: Mock, redis_server: Redis -) -> None: - is_getdel_available.side_effect.return_value = False + mock_is_getdel_available.return_value = getdel_available test_value = "test_data" test_key = "test_key" diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 321bb40..0fa6100 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -34,18 +34,15 @@ def test_secret_is_not_decryptable_by_random_key(fernet: Fernet): decrypt_secret(encrypted_secret.data.encode(), random_fernet) -async def test_is_getdel_available_when_supported(): +@pytest.mark.parametrize( + ("getdel_effect", "expected_result"), [(None, True), (ResponseError, False)] +) +async def test_is_getdel_available( + getdel_effect: ResponseError | None, expected_result: bool +): redis = AsyncMock() + redis.getdel.side_effect = getdel_effect # pyright: ignore[reportAny] result = await _is_getdel_available(redis) - assert result is True - - -async def test_is_getdel_available_when_not_supported(): - redis = AsyncMock() - redis.getdel.side_effect = ResponseError - - result = await _is_getdel_available(redis) - - assert result is False + assert result is expected_result -- 2.43.0 From c67826047d924e67ab36c7465f9fe59e5bc4b35d Mon Sep 17 00:00:00 2001 From: Ivan Golikov Date: Wed, 8 Jan 2025 22:19:08 +0100 Subject: [PATCH 3/4] Better docstrings --- pssecret_server/utils.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pssecret_server/utils.py b/pssecret_server/utils.py index 0fe0b23..ea10ef5 100644 --- a/pssecret_server/utils.py +++ b/pssecret_server/utils.py @@ -37,7 +37,10 @@ async def save_secret(data: Secret, redis: Redis) -> str: @lru_cache async def _is_getdel_available(redis: Redis) -> bool: - """GETDEL is not available in Redis prior to version 6.2""" + """Checks the availability of GETDEL command on the Redis server instance + + GETDEL is not available in Redis prior to version 6.2 + """ try: await redis.getdel("test:getdel:availability") except ResponseError: @@ -47,6 +50,12 @@ async def _is_getdel_available(redis: Redis) -> bool: async def getdel(redis: Redis, key: str) -> ResponseT: + """Gets the value of key and deletes the key + + Depending on the capabilities of Redis server this function + will either call GETDEL command, either first call GETSET with empty string + and DEL right after that. + """ result: ResponseT if await _is_getdel_available(redis): -- 2.43.0 From 0b9ba669a2ccbc23c1250a80acfe8313af9d1d26 Mon Sep 17 00:00:00 2001 From: Ivan Golikov Date: Wed, 8 Jan 2025 22:21:16 +0100 Subject: [PATCH 4/4] pyright ignore --- tests/integration/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_utils.py b/tests/integration/test_utils.py index 03ae701..c7d54b0 100644 --- a/tests/integration/test_utils.py +++ b/tests/integration/test_utils.py @@ -51,5 +51,5 @@ async def test_getdel( result = await getdel(redis_server, test_key) - assert result.decode() == test_value + assert result.decode() == test_value # pyright: ignore[reportAttributeAccessIssue] assert not await redis_server.exists(test_key) -- 2.43.0