diff --git a/redisvl/index/index.py b/redisvl/index/index.py index 77fc6009..e473a977 100644 --- a/redisvl/index/index.py +++ b/redisvl/index/index.py @@ -1394,12 +1394,19 @@ def drop_keys( Cluster, keys are unlinked individually so a chunk that spans hash slots does not raise ``CROSSSLOT``. + NOTE: Cluster users must only call this method with keys that share a + single hash tag, matching ``drop_documents``. + Args: keys (Union[str, List[str]]): The document ID or IDs to remove from the index. batch_size (int): Number of keys to unlink per round-trip. Returns: int: Count of records deleted from Redis. + + Raises: + ValueError: On Redis Cluster, if the keys do not all share a hash + tag (keeps behavior aligned with ``drop_documents``). """ if not isinstance(keys, list): return self._redis_client.unlink(keys) # type: ignore @@ -1407,6 +1414,14 @@ def drop_keys( if not keys: return 0 + # Match drop_documents: require a shared hash tag before multi-key + # unlink work on cluster so keys= and ids= paths fail the same way. + if isinstance(self._redis_client, RedisCluster): + if not _keys_share_hash_tag(keys): + raise ValueError( + "All keys must share a hash tag when using Redis Cluster." + ) + total = 0 for i in range(0, len(keys), batch_size): total += self._unlink_batch(keys[i : i + batch_size]) @@ -2801,12 +2816,19 @@ async def drop_keys( Cluster, keys are unlinked individually so a chunk that spans hash slots does not raise ``CROSSSLOT``. + NOTE: Cluster users must only call this method with keys that share a + single hash tag, matching ``drop_documents``. + Args: keys (Union[str, List[str]]): The document ID or IDs to remove from the index. batch_size (int): Number of keys to unlink per round-trip. Returns: int: Count of records deleted from Redis. + + Raises: + ValueError: On Redis Cluster, if the keys do not all share a hash + tag (keeps behavior aligned with ``drop_documents``). """ client = await self._get_client() @@ -2816,6 +2838,14 @@ async def drop_keys( if not keys: return 0 + # Match drop_documents: require a shared hash tag before multi-key + # unlink work on cluster so keys= and ids= paths fail the same way. + if isinstance(client, AsyncRedisCluster): + if not _keys_share_hash_tag(keys): + raise ValueError( + "All keys must share a hash tag when using Redis Cluster." + ) + total = 0 for i in range(0, len(keys), batch_size): total += await self._unlink_batch(keys[i : i + batch_size]) diff --git a/tests/unit/test_error_handling.py b/tests/unit/test_error_handling.py index 4f10e6a8..b7d61c07 100644 --- a/tests/unit/test_error_handling.py +++ b/tests/unit/test_error_handling.py @@ -256,7 +256,7 @@ def test_router_config_numeric_error(self): class TestClusterCompatibilityValidation: - """Test cluster compatibility validation for drop_documents.""" + """Test cluster compatibility validation for drop_documents and drop_keys.""" @patch("redisvl.redis.connection.RedisConnectionFactory.validate_sync_redis") def test_drop_documents_cluster_validation_success(self, mock_validate): @@ -334,6 +334,77 @@ def test_drop_documents_non_cluster_no_validation(self, mock_validate): assert result == 2 mock_client.delete.assert_called_once() + @patch("redisvl.redis.connection.RedisConnectionFactory.validate_sync_redis") + def test_drop_keys_cluster_validation_success(self, mock_validate): + """Test that keys with the same hash tag work in cluster.""" + from redisvl.index import SearchIndex + from redisvl.schema import IndexSchema + + schema = Mock(spec=IndexSchema) + schema.index = Mock() + schema.index.prefix = "test" + schema.index.key_separator = ":" + schema.index.storage_type = StorageType.HASH + + mock_cluster_client = Mock(spec=RedisCluster) + mock_cluster_client.unlink.return_value = 1 + + index = SearchIndex(schema=schema, redis_client=mock_cluster_client) + + keys = ["prefix:{user123}:key1", "prefix:{user123}:key2"] + + result = index.drop_keys(keys) + assert result == 2 + assert mock_cluster_client.unlink.call_count == 2 + + @patch("redisvl.redis.connection.RedisConnectionFactory.validate_sync_redis") + def test_drop_keys_cluster_validation_failure(self, mock_validate): + """Test that keys with different hash tags fail in cluster.""" + from redisvl.index import SearchIndex + from redisvl.schema import IndexSchema + + schema = Mock(spec=IndexSchema) + schema.index = Mock() + schema.index.prefix = "test" + schema.index.key_separator = ":" + schema.index.storage_type = StorageType.HASH + + mock_cluster_client = Mock(spec=RedisCluster) + + index = SearchIndex(schema=schema, redis_client=mock_cluster_client) + + keys = ["prefix:{user123}:key1", "prefix:{user456}:key2"] + + with pytest.raises(ValueError) as exc_info: + index.drop_keys(keys) + + error_msg = str(exc_info.value) + assert "All keys must share a hash tag when using Redis Cluster" in error_msg + mock_cluster_client.unlink.assert_not_called() + + @patch("redisvl.redis.connection.RedisConnectionFactory.validate_sync_redis") + def test_drop_keys_non_cluster_no_validation(self, mock_validate): + """Test that non-cluster clients skip hash tag validation for drop_keys.""" + from redisvl.index import SearchIndex + from redisvl.schema import IndexSchema + + schema = Mock(spec=IndexSchema) + schema.index = Mock() + schema.index.prefix = "test" + schema.index.key_separator = ":" + schema.index.storage_type = StorageType.HASH + + mock_client = Mock(spec=Redis) + mock_client.unlink.return_value = 2 + + index = SearchIndex(schema=schema, redis_client=mock_client) + + keys = ["prefix:{user123}:key1", "prefix:{user456}:key2"] + + result = index.drop_keys(keys) + assert result == 2 + mock_client.unlink.assert_called_once() + class TestAsyncErrorHandling: """Test error handling in async methods.""" @@ -426,6 +497,36 @@ async def test_async_drop_documents_cluster_validation(self, mock_validate): error_msg = str(exc_info.value) assert "All keys must share a hash tag when using Redis Cluster" in error_msg + @patch("redisvl.redis.connection.RedisConnectionFactory.validate_async_redis") + @pytest.mark.asyncio + async def test_async_drop_keys_cluster_validation(self, mock_validate): + """Test async drop_keys cluster validation matches drop_documents.""" + from unittest.mock import AsyncMock + + from redisvl.index import AsyncSearchIndex + from redisvl.schema import IndexSchema + + schema = Mock(spec=IndexSchema) + schema.index = Mock() + schema.index.prefix = "test" + schema.index.key_separator = ":" + schema.index.storage_type = StorageType.HASH + + mock_cluster_client = Mock() + mock_cluster_client.__class__ = AsyncRedisCluster + mock_cluster_client.unlink = AsyncMock(return_value=1) + + index = AsyncSearchIndex(schema=schema, redis_client=mock_cluster_client) + + keys = ["prefix:{user123}:key1", "prefix:{user456}:key2"] + + with pytest.raises(ValueError) as exc_info: + await index.drop_keys(keys) + + error_msg = str(exc_info.value) + assert "All keys must share a hash tag when using Redis Cluster" in error_msg + mock_cluster_client.unlink.assert_not_called() + class TestClusterOperationsErrorHandling: """Test error handling for cluster operations."""