Applying review comments.

Author: petyaslavova

redis/commands/core.py

@@ -3,6 +3,7 @@
 import datetime
 import hashlib
 import warnings
+from enum import Enum
 from typing import (
     TYPE_CHECKING,
     Any,
@@ -4907,6 +4908,16 @@ def pfmerge(self, dest: KeyT, *sources: KeyT) -> ResponseT:
 AsyncHyperlogCommands = HyperlogCommands
 
 
+class HashDataPersistOptions(Enum):
+    # set the value for each provided key to each
+    # provided value only if all do not already exist.
+    FNX = "FNX"
+
+    # set the value for each provided key to each
+    # provided value only if all already exist.
+    FXX = "FXX"
+
+
 class HashCommands(CommandsProtocol):
     """
     Redis commands for Hash data type.
@@ -4949,7 +4960,9 @@ def hgetall(self, name: str) -> Union[Awaitable[dict], dict]:
 
     def hgetdel(
         self, name: str, *keys: str
-    ) -> Union[Awaitable[Optional[str]], Optional[str]]:
+    ) -> Union[
+        Awaitable[Optional[List[Union[str, bytes]]]], Optional[List[Union[str, bytes]]]
+    ]:
         """
         Return the value of ``key`` within the hash ``name`` and
         delete the field in the hash.
@@ -4967,15 +4980,18 @@ def hgetdel(
     def hgetex(
         self,
         name: KeyT,
-        *keys: str,
+        key: Optional[str] = None,
+        keys: Optional[list] = None,
         ex: Optional[ExpiryT] = None,
         px: Optional[ExpiryT] = None,
         exat: Optional[AbsExpiryT] = None,
         pxat: Optional[AbsExpiryT] = None,
         persist: bool = False,
-    ) -> Union[Awaitable[Optional[str]], Optional[str]]:
+    ) -> Union[
+        Awaitable[Optional[List[Union[str, bytes]]]], Optional[List[Union[str, bytes]]]
+    ]:
         """
-        Return the values of ``keys`` within the hash ``name``
+        Return the values of ``key`` and ``keys`` within the hash ``name``
         and optionally set their expiration.
 
         ``ex`` sets an expire flag on ``kyes`` for ``ex`` seconds.
@@ -4993,8 +5009,7 @@ def hgetex(
         Available since Redis 8.0
         For more information see https://redis.io/commands/hgetex
         """
-
-        if len(keys) == 0:
+        if key is None and not keys:
             raise DataError("'hgetex' should have at least one key provided")
 
         opset = {ex, px, exat, pxat}
@@ -5003,14 +5018,24 @@ def hgetex(
                 "``ex``, ``px``, ``exat``, ``pxat``, "
                 "and ``persist`` are mutually exclusive."
             )
+        keys_to_request = []
+        if key is not None:
+            keys_to_request.append(key)
+        if keys:
+            keys_to_request.extend(keys)
 
         exp_options: list[EncodableT] = extract_expire_flags(ex, px, exat, pxat)
 
         if persist:
             exp_options.append("PERSIST")
 
         return self.execute_command(
-            "HGETEX", name, *exp_options, "FIELDS", len(keys), *keys
+            "HGETEX",
+            name,
+            *exp_options,
+            "FIELDS",
+            len(keys_to_request),
+            *keys_to_request,
         )
 
     def hincrby(
@@ -5093,8 +5118,7 @@ def hsetex(
         px: Optional[ExpiryT] = None,
         exat: Optional[AbsExpiryT] = None,
         pxat: Optional[AbsExpiryT] = None,
-        fnx: bool = False,
-        fxx: bool = False,
+        data_persist_option: Optional[HashDataPersistOptions] = None,
         keepttl: bool = False,
     ) -> Union[Awaitable[int], int]:
         """
@@ -5116,11 +5140,12 @@ def hsetex(
         ``pxat`` sets an expire flag on ``keys`` for ``ex`` milliseconds,
             specified in unix time.
 
-        ``fnx`` if set to True, set the value for each provided key to each
-            provided value only if all do not already exist.
-
-        ``fxx`` if set to True, set the value for each provided key to each
-            provided value only if all already exist.
+        ``data_persist_option`` can be set to ``FNX`` or ``FXX`` to control the
+            behavior of the command.
+            ``FNX`` will set the value for each provided key to each
+                provided value only if all do not already exist.
+            ``FXX`` will set the value for each provided key to each
+                provided value only if all already exist.
 
         ``keepttl`` if True, retain the time to live associated with the keys.
 
@@ -5145,14 +5170,10 @@ def hsetex(
                 "and ``keepttl`` are mutually exclusive."
             )
 
-        if fnx and fxx:
-            raise DataError("``fnx`` and ``fxx`` are mutually exclusive.")
-
         exp_options: list[EncodableT] = extract_expire_flags(ex, px, exat, pxat)
-        if fnx:
-            exp_options.append("FNX")
-        if fxx:
-            exp_options.append("FXX")
+        if data_persist_option:
+            exp_options.append(data_persist_option.value)
+
         if keepttl:
             exp_options.append("KEEPTTL")
 

tests/test_asyncio/test_hash.py

@@ -5,6 +5,7 @@
 import pytest
 
 from redis import exceptions
+from redis.commands.core import HashDataPersistOptions
 from tests.conftest import skip_if_server_version_lt
 from tests.test_asyncio.test_utils import redis_server_time
 
@@ -327,7 +328,9 @@ async def test_hgetex_no_expiration(r):
         "b", "foo", "bar", mapping={"1": 1, "2": 2, "3": "three", "4": b"four"}
     )
 
-    assert await r.hgetex("b", "foo", "1", "4") == [b"bar", b"1", b"four"]
+    assert await r.hgetex("b", keys=["foo", "1", "4"]) == [b"bar", b"1", b"four"]
+    assert await r.hgetex("b", "foo", keys=["1", "4"]) == [b"bar", b"1", b"four"]
+    assert await r.hgetex("b", "foo") == [b"bar"]
     assert await r.httl("b", "foo", "1", "4") == [-1, -1, -1]
 
 
@@ -338,66 +341,76 @@ async def test_hgetex_expiration_configs(r):
         "test:hash", "foo", "bar", mapping={"1": 1, "3": "three", "4": b"four"}
     )
 
+    test_keys = ["foo", "1", "4"]
     # test get with multiple fields with expiration set through 'ex'
-    assert await r.hgetex("test:hash", "foo", "1", "4", ex=10) == [
+    assert await r.hgetex("test:hash", keys=test_keys, ex=10) == [
         b"bar",
         b"1",
         b"four",
     ]
-    assert await r.httl("test:hash", "foo", "1", "4") == [10, 10, 10]
+    ttls = await r.httl("test:hash", *test_keys)
+    for ttl in ttls:
+        assert pytest.approx(ttl) == 10
 
     # test get with multiple fields removing expiration settings with 'persist'
-    assert await r.hgetex("test:hash", "foo", "1", "4", persist=True) == [
+    assert await r.hgetex("test:hash", "foo", keys=["1", "4"], persist=True) == [
         b"bar",
         b"1",
         b"four",
     ]
-    assert await r.httl("test:hash", "foo", "1", "4") == [-1, -1, -1]
+    assert await r.httl("test:hash", *test_keys) == [-1, -1, -1]
 
     # test get with multiple fields with expiration set through 'px'
-    assert await r.hgetex("test:hash", "foo", "1", "4", px=6000) == [
+    assert await r.hgetex("test:hash", keys=test_keys, px=6000) == [
         b"bar",
         b"1",
         b"four",
     ]
-    assert await r.httl("test:hash", "foo", "1", "4") == [6, 6, 6]
+    ttls = await r.httl("test:hash", *test_keys)
+    for ttl in ttls:
+        assert pytest.approx(ttl) == 6
 
     # test get single field with expiration set through 'pxat'
     expire_at = await redis_server_time(r) + timedelta(minutes=1)
     assert await r.hgetex("test:hash", "foo", pxat=expire_at) == [b"bar"]
     assert (await r.httl("test:hash", "foo"))[0] <= 61
 
+    # test get single field with expiration set through 'exat'
+    expire_at = await redis_server_time(r) + timedelta(seconds=10)
+    assert await r.hgetex("test:hash", "foo", exat=expire_at) == [b"bar"]
+    assert (await r.httl("test:hash", "foo"))[0] <= 10
+
 
 @skip_if_server_version_lt("7.9.0")
-async def test_hgetex_validate_expired_foields_removed(r):
+async def test_hgetex_validate_expired_fields_removed(r):
     await r.delete("test:hash")
     await r.hset(
         "test:hash", "foo", "bar", mapping={"1": 1, "3": "three", "4": b"four"}
     )
 
     # test get multiple fields with expiration set
     # validate that expired fields are removed
-    assert await r.hgetex("test:hash", "foo", "1", "3", ex=1) == [
+    assert await r.hgetex("test:hash", keys=["foo", "1", "3"], ex=1) == [
         b"bar",
         b"1",
         b"three",
     ]
     await asyncio.sleep(1.1)
-    assert await r.hgetex("test:hash", "foo", "1", "3") == [None, None, None]
+    assert await r.hgetex("test:hash", "foo", keys=["1", "3"]) == [None, None, None]
     assert await r.httl("test:hash", "foo", "1", "3") == [-2, -2, -2]
     assert await r.hgetex("test:hash", "4") == [b"four"]
 
 
 @skip_if_server_version_lt("7.9.0")
 async def test_hgetex_invalid_inputs(r):
     with pytest.raises(exceptions.DataError):
-        await r.hgetex("b", "foo", "1", "3", ex=10, persist=True)
+        await r.hgetex("b", "foo", ex=10, persist=True)
 
     with pytest.raises(exceptions.DataError):
-        await r.hgetex("b", "foo", "1", "3", ex=10.0, persist=True)
+        await r.hgetex("b", "foo", ex=10.0, persist=True)
 
     with pytest.raises(exceptions.DataError):
-        await r.hgetex("b", "foo", "1", "3", ex=10, px=6000)
+        await r.hgetex("b", "foo", ex=10, px=6000)
 
     with pytest.raises(exceptions.DataError):
         await r.hgetex("b", ex=10)
@@ -430,14 +443,12 @@ async def test_hsetex_expiration_ex_and_keepttl(r):
         )
         == 1
     )
-    assert await r.httl("test:hash", "foo", "1", "2", "i1", "i2") == [
-        10,
-        10,
-        10,
-        10,
-        10,
-    ]
-    assert await r.hgetex("test:hash", "foo", "1", "2", "i1", "i2") == [
+    test_keys = ["foo", "1", "2", "i1", "i2"]
+    ttls = await r.httl("test:hash", *test_keys)
+    for ttl in ttls:
+        assert pytest.approx(ttl) == 10
+
+    assert await r.hgetex("test:hash", keys=test_keys) == [
         b"bar",
         b"1",
         b"2",
@@ -447,7 +458,7 @@ async def test_hsetex_expiration_ex_and_keepttl(r):
     await asyncio.sleep(1.1)
     # validate keepttl
     assert await r.hsetex("test:hash", "foo", "bar1", keepttl=True) == 1
-    assert (await r.httl("test:hash", "foo"))[0] < 10
+    assert 0 < (await r.httl("test:hash", "foo"))[0] < 10
 
 
 @skip_if_server_version_lt("7.9.0")
@@ -459,8 +470,12 @@ async def test_hsetex_expiration_px(r):
         await r.hsetex("test:hash", "foo", "bar", mapping={"1": 1, "2": "2"}, px=60000)
         == 1
     )
-    assert await r.httl("test:hash", "foo", "1", "2") == [60, 60, 60]
-    assert await r.hgetex("test:hash", "foo", "1", "2") == [b"bar", b"1", b"2"]
+    test_keys = ["foo", "1", "2"]
+    ttls = await r.httl("test:hash", *test_keys)
+    for ttl in ttls:
+        assert pytest.approx(ttl) == 60
+
+    assert await r.hgetex("test:hash", keys=test_keys) == [b"bar", b"1", b"2"]
 
 
 @skip_if_server_version_lt("7.9.0")
@@ -474,30 +489,35 @@ async def test_hsetex_expiration_pxat_and_fnx(r):
     expire_at = await redis_server_time(r) + timedelta(minutes=1)
     assert (
         await r.hsetex(
-            "test:hash", "foo", "bar1", mapping={"new": "ok"}, pxat=expire_at, fnx=True
+            "test:hash",
+            "foo",
+            "bar1",
+            mapping={"new": "ok"},
+            pxat=expire_at,
+            data_persist_option=HashDataPersistOptions.FNX,
         )
         == 0
     )
     ttls = await r.httl("test:hash", "foo", "new")
     assert ttls[0] <= 30
     assert ttls[1] == -2
 
-    assert await r.hgetex("test:hash", "foo", "1", "new") == [b"bar", b"1", None]
+    assert await r.hgetex("test:hash", keys=["foo", "1", "new"]) == [b"bar", b"1", None]
     assert (
         await r.hsetex(
             "test:hash",
             "foo_new",
             "bar1",
             mapping={"new": "ok"},
             pxat=expire_at,
-            fnx=True,
+            data_persist_option=HashDataPersistOptions.FNX,
         )
         == 1
     )
     ttls = await r.httl("test:hash", "foo", "new")
     for ttl in ttls:
         assert ttl <= 61
-    assert await r.hgetex("test:hash", "foo", "foo_new", "new") == [
+    assert await r.hgetex("test:hash", keys=["foo", "foo_new", "new"]) == [
         b"bar",
         b"bar1",
         b"ok",
@@ -520,27 +540,27 @@ async def test_hsetex_expiration_exat_and_fxx(r):
             "bar1",
             mapping={"new": "ok"},
             exat=expire_at,
-            fxx=True,
+            data_persist_option=HashDataPersistOptions.FXX,
         )
         == 0
     )
     ttls = await r.httl("test:hash", "foo", "new")
     assert 10 < ttls[0] <= 30
     assert ttls[1] == -2
 
-    assert await r.hgetex("test:hash", "foo", "1", "new") == [b"bar", b"1", None]
+    assert await r.hgetex("test:hash", keys=["foo", "1", "new"]) == [b"bar", b"1", None]
     assert (
         await r.hsetex(
             "test:hash",
             "foo",
             "bar1",
             mapping={"1": "new_value"},
             exat=expire_at,
-            fxx=True,
+            data_persist_option=HashDataPersistOptions.FXX,
         )
         == 1
     )
-    assert await r.hgetex("test:hash", "foo", "1") == [b"bar1", b"new_value"]
+    assert await r.hgetex("test:hash", keys=["foo", "1"]) == [b"bar1", b"new_value"]
 
 
 @skip_if_server_version_lt("7.9.0")
@@ -556,6 +576,3 @@ async def test_hsetex_invalid_inputs(r):
 
     with pytest.raises(exceptions.DataError):
         await r.hsetex("b", "foo", "bar", ex=10, keepttl=True)
-
-    with pytest.raises(exceptions.DataError):
-        await r.hsetex("b", "foo", "bar", ex=10, fxx=True, fnx=True)