Disable features without required Redis modules

Some features, like querying and embedded models, require
either the RediSearch or RedisJSON modules running in Redis.
Without these modules, using these features would result
in inscrutable errors.

We now disable some tests if the Redis module required for the
test is not found in the Redis instance the tests are using,
and raise errors or log messages if the same is true during
execution of HashModel and JsonModel.

Author: Andrew Brookins

Makefile

@@ -56,6 +56,14 @@ format: $(INSTALL_STAMP)
 test: $(INSTALL_STAMP)
 	$(POETRY) run pytest -n auto -s -vv ./tests/ --cov-report term-missing --cov $(NAME)
 
+.PHONY: test_oss
+test_oss: $(INSTALL_STAMP)
+	# Specifically tests against a local OSS Redis instance via
+	# docker-compose.yml. Do not use this for CI testing, where we should
+	# instead have a matrix of Docker images.
+	REDIS_OM_URL="redis://localhost:6381" $(POETRY) run pytest -n auto -s -vv ./tests/ --cov-report term-missing --cov $(NAME)
+
+
 .PHONY: shell
 shell: $(INSTALL_STAMP)
 	$(POETRY) shell

docker-compose.yml

@@ -9,3 +9,11 @@ services:
       - "6380:6379"
     volumes:
       - ./data:/data
+
+  oss_redis:
+      image: "redis:latest"
+      restart: always
+      ports:
+        - "6381:6379"
+      volumes:
+        - ./oss_data:/oss_data

docs/getting_started.md

@@ -112,13 +112,13 @@ We're almost ready to create a Redis OM model! But first, we need to make sure t
 
 By default, Redis OM tries to connect to Redis on your localhost at port 6379. Most local install methods will result in Redis running at this location, in which case you don't need to do anything special.
 
-However, if you configured Redis to run on a different port, or if you're using a remote Redis server, you'll need to set the `REDIS_URL` environment variable.
+However, if you configured Redis to run on a different port, or if you're using a remote Redis server, you'll need to set the `REDIS_OM_URL` environment variable.
 
-The `REDIS_URL` environment variable follows the redis-py URL format:
+The `REDIS_OM_URL` environment variable follows the redis-py URL format:
 
     redis://[[username]:[password]]@localhost:6379/[database number]
 
-The default connection is equivalent to the following `REDIS_URL` environment variable:
+The default connection is equivalent to the following `REDIS_OM_URL` environment variable:
 
     redis://@localhost:6379
 
@@ -133,11 +133,11 @@ For more details about how to connect to Redis with Redis OM, see the [connectio
 
 ### Redis Cluster Support
 
-Redis OM supports connecting to Redis Cluster, but this preview release does not support doing so with the `REDIS_URL` environment variable. However, you can connect by manually creating a connection object.
+Redis OM supports connecting to Redis Cluster, but this preview release does not support doing so with the `REDIS_OM_URL` environment variable. However, you can connect by manually creating a connection object.
 
 See the [connections documentation](connections.md) for examples of connecting to Redis Cluster.
 
-Support for connecting to Redis Cluster via `REDIS_URL` will be added in a future release.
+Support for connecting to Redis Cluster via `REDIS_OM_URL` will be added in a future release.
 
 ## Defining a Model
 

redis_om/checks.py

@@ -0,0 +1,28 @@
+from functools import lru_cache
+from typing import List
+
+from redis_om.connections import get_redis_connection
+
+
+@lru_cache(maxsize=None)
+def get_modules(conn) -> List[str]:
+    modules = conn.execute_command("module", "list")
+    return [m[1] for m in modules]
+
+
+@lru_cache(maxsize=None)
+def has_redis_json(conn=None):
+    if conn is None:
+        conn = get_redis_connection()
+    names = get_modules(conn)
+    return b"ReJSON" in names or "ReJSON" in names
+
+
+@lru_cache(maxsize=None)
+def has_redisearch(conn=None):
+    if conn is None:
+        conn = get_redis_connection()
+    if has_redis_json(conn):
+        return True
+    names = get_modules(conn)
+    return b"search" in names or "search" in names

redis_om/model/model.py

@@ -37,6 +37,7 @@
 from redis.client import Pipeline
 from ulid import ULID
 
+from ..checks import has_redis_json, has_redisearch
 from ..connections import get_redis_connection
 from .encoders import jsonable_encoder
 from .render_tree import render_tree
@@ -121,6 +122,20 @@ def validate_model_fields(model: Type["RedisModel"], field_values: Dict[str, Any
             )
 
 
+def decode_redis_value(
+    obj: Union[List[bytes], Dict[bytes, bytes], bytes], encoding: str
+) -> Union[List[str], Dict[str, str], str]:
+    """Decode a binary-encoded Redis hash into the specified encoding."""
+    if isinstance(obj, list):
+        return [v.decode(encoding) for v in obj]
+    if isinstance(obj, dict):
+        return {
+            key.decode(encoding): value.decode(encoding) for key, value in obj.items()
+        }
+    elif isinstance(obj, bytes):
+        return obj.decode(encoding)
+
+
 class ExpressionProtocol(Protocol):
     op: Operators
     left: ExpressionOrModelField
@@ -317,6 +332,11 @@ def __init__(
         page_size: int = DEFAULT_PAGE_SIZE,
         sort_fields: Optional[List[str]] = None,
     ):
+        if not has_redisearch(model.db()):
+            raise RedisModelError("Your Redis instance does not have either the RediSearch module "
+                                  "or RedisJSON module installed. Querying requires that your Redis "
+                                  "instance has one of these modules installed.")
+
         self.expressions = expressions
         self.model = model
         self.offset = offset
@@ -330,8 +350,8 @@ def __init__(
 
         self._expression = None
         self._query: Optional[str] = None
-        self._pagination: list[str] = []
-        self._model_cache: list[RedisModel] = []
+        self._pagination: List[str] = []
+        self._model_cache: List[RedisModel] = []
 
     def dict(self) -> Dict[str, Any]:
         return dict(
@@ -919,6 +939,7 @@ class MetaProtocol(Protocol):
     index_name: str
     abstract: bool
     embedded: bool
+    encoding: str
 
 
 @dataclasses.dataclass
@@ -938,6 +959,7 @@ class DefaultMeta:
     index_name: Optional[str] = None
     abstract: Optional[bool] = False
     embedded: Optional[bool] = False
+    encoding: Optional[str] = "utf-8"
 
 
 class ModelMeta(ModelMetaclass):
@@ -1007,6 +1029,8 @@ def __new__(cls, name, bases, attrs, **kwargs):  # noqa C901
             new_class._meta.database = getattr(
                 base_meta, "database", get_redis_connection()
             )
+        if not getattr(new_class._meta, "encoding", None):
+            new_class._meta.encoding = getattr(base_meta, "encoding")
         if not getattr(new_class._meta, "primary_key_creator_cls", None):
             new_class._meta.primary_key_creator_cls = getattr(
                 base_meta, "primary_key_creator_cls", UlidPrimaryKey
@@ -1059,7 +1083,7 @@ def update(self, **field_values):
     def save(self, pipeline: Optional[Pipeline] = None) -> "RedisModel":
         raise NotImplementedError
 
-    @validator("pk", always=True)
+    @validator("pk", always=True, allow_reuse=True)
     def validate_pk(cls, v):
         if not v:
             v = cls._meta.primary_key_creator_cls().create_pk()
@@ -1205,7 +1229,18 @@ def get(cls, pk: Any) -> "HashModel":
         document = cls.db().hgetall(cls.make_primary_key(pk))
         if not document:
             raise NotFoundError
-        return cls.parse_obj(document)
+        try:
+            result = cls.parse_obj(document)
+        except TypeError as e:
+            log.warning(
+                f'Could not parse Redis response. Error was: "{e}". Probably, the '
+                "connection is not set to decode responses from bytes. "
+                "Attempting to decode response using the encoding set on "
+                f"model class ({cls.__class__}. Encoding: {cls.Meta.encoding}."
+            )
+            document = decode_redis_value(document, cls.Meta.encoding)
+            result = cls.parse_obj(document)
+        return result
 
     @classmethod
     @no_type_check
@@ -1316,6 +1351,9 @@ def schema_for_type(cls, name, typ: Any, field_info: PydanticFieldInfo):
 
 class JsonModel(RedisModel, abc.ABC):
     def __init_subclass__(cls, **kwargs):
+        if not has_redis_json(cls.db()):
+            log.error("Your Redis instance does not have the RedisJson module "
+                      "loaded. JsonModel depends on RedisJson.")
         # Generate the RediSearch schema once to validate fields.
         cls.redisearch_schema()
 

tests/test_hash_model.py

@@ -8,11 +8,15 @@
 import pytest
 from pydantic import ValidationError
 
+from redis_om.checks import has_redisearch
 from redis_om.model import Field, HashModel
 from redis_om.model.migrations.migrator import Migrator
 from redis_om.model.model import NotFoundError, QueryNotSupportedError, RedisModelError
 
 
+if not has_redisearch():
+    pytestmark = pytest.mark.skip
+
 today = datetime.date.today()
 
 

tests/test_json_model.py

@@ -8,11 +8,15 @@
 import pytest
 from pydantic import ValidationError
 
+from redis_om.checks import has_redis_json
 from redis_om.model import EmbeddedJsonModel, Field, JsonModel
 from redis_om.model.migrations.migrator import Migrator
 from redis_om.model.model import NotFoundError, QueryNotSupportedError, RedisModelError
 
 
+if not has_redis_json():
+    pytestmark = pytest.mark.skip
+
 today = datetime.date.today()
 
 
@@ -477,7 +481,7 @@ def test_numeric_queries(members, m):
     actual = m.Member.find(m.Member.age == 34).all()
     assert actual == [member2]
 
-    actual = m.Member.find(m.Member.age > 34).all()
+    actual = m.Member.find(m.Member.age > 34).sort_by("age").all()
     assert actual == [member1, member3]
 
     actual = m.Member.find(m.Member.age < 35).all()