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.

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 pydantic.utils import Representation
 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 @@ class FindQuery:
         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 @@ class FindQuery:
 
         self._expression = None
         self._query: Optional[str] = None
-        self._pagination: list[str] = []
+        self._pagination: List[str] = []
-        self._model_cache: list[RedisModel] = []
+        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 @@ class ModelMeta(ModelMetaclass):
             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 @@ class RedisModel(BaseModel, abc.ABC, metaclass=ModelMeta):
     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 @@ class HashModel(RedisModel, abc.ABC):
         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 @@ class HashModel(RedisModel, abc.ABC):
 
 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 @@ from unittest import mock
 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 @@ from unittest import mock
 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()

tests/test_oss_redis_features.py

@@ -0,0 +1,169 @@
+import abc
+import datetime
+import decimal
+from collections import namedtuple
+from typing import Optional
+from unittest import mock
+
+import pytest
+from pydantic import ValidationError
+
+from redis_om.model import Field, HashModel
+from redis_om.model.migrations.migrator import Migrator
+from redis_om.model.model import NotFoundError, QueryNotSupportedError, RedisModelError
+
+
+today = datetime.date.today()
+
+
+@pytest.fixture
+def m(key_prefix):
+    class BaseHashModel(HashModel, abc.ABC):
+        class Meta:
+            global_key_prefix = key_prefix
+
+    class Order(BaseHashModel):
+        total: decimal.Decimal
+        currency: str
+        created_on: datetime.datetime
+
+    class Member(BaseHashModel):
+        first_name: str
+        last_name: str
+        email: str
+        join_date: datetime.date
+        age: int
+
+        class Meta:
+            model_key_prefix = "member"
+            primary_key_pattern = ""
+
+    Migrator().run()
+
+    return namedtuple("Models", ["BaseHashModel", "Order", "Member"])(
+        BaseHashModel, Order, Member
+    )
+
+
+@pytest.fixture
+def members(m):
+    member1 = m.Member(
+        first_name="Andrew",
+        last_name="Brookins",
+        email="a@example.com",
+        age=38,
+        join_date=today,
+    )
+
+    member2 = m.Member(
+        first_name="Kim",
+        last_name="Brookins",
+        email="k@example.com",
+        age=34,
+        join_date=today,
+    )
+
+    member3 = m.Member(
+        first_name="Andrew",
+        last_name="Smith",
+        email="as@example.com",
+        age=100,
+        join_date=today,
+    )
+    member1.save()
+    member2.save()
+    member3.save()
+
+    yield member1, member2, member3
+
+
+def test_validates_required_fields(m):
+    # Raises ValidationError: last_name is required
+    with pytest.raises(ValidationError):
+        m.Member(first_name="Andrew", zipcode="97086", join_date=today)
+
+
+def test_validates_field(m):
+    # Raises ValidationError: join_date is not a date
+    with pytest.raises(ValidationError):
+        m.Member(first_name="Andrew", last_name="Brookins", join_date="yesterday")
+
+
+# Passes validation
+def test_validation_passes(m):
+    member = m.Member(
+        first_name="Andrew",
+        last_name="Brookins",
+        email="a@example.com",
+        join_date=today,
+        age=38,
+    )
+    assert member.first_name == "Andrew"
+
+
+def test_saves_model_and_creates_pk(m):
+    member = m.Member(
+        first_name="Andrew",
+        last_name="Brookins",
+        email="a@example.com",
+        join_date=today,
+        age=38,
+    )
+    # Save a model instance to Redis
+    member.save()
+
+    member2 = m.Member.get(member.pk)
+    assert member2 == member
+
+
+def test_raises_error_with_embedded_models(m):
+    class Address(m.BaseHashModel):
+        address_line_1: str
+        address_line_2: Optional[str]
+        city: str
+        country: str
+        postal_code: str
+
+    with pytest.raises(RedisModelError):
+
+        class InvalidMember(m.BaseHashModel):
+            address: Address
+
+
+@pytest.mark.skip("Not implemented yet")
+def test_saves_many(m):
+    members = [
+        m.Member(
+            first_name="Andrew",
+            last_name="Brookins",
+            email="a@example.com",
+            join_date=today,
+        ),
+        m.Member(
+            first_name="Kim",
+            last_name="Brookins",
+            email="k@example.com",
+            join_date=today,
+        ),
+    ]
+    m.Member.add(members)
+
+
+@pytest.mark.skip("Not ready yet")
+def test_updates_a_model(members, m):
+    member1, member2, member3 = members
+
+    # Or, with an implicit save:
+    member1.update(last_name="Smith")
+    assert m.Member.find(m.Member.pk == member1.pk).first() == member1
+
+    # Or, affecting multiple model instances with an implicit save:
+    m.Member.find(m.Member.last_name == "Brookins").update(last_name="Smith")
+    results = m.Member.find(m.Member.last_name == "Smith")
+    assert results == members
+
+
+def test_not_found(m):
+    with pytest.raises(NotFoundError):
+        # This ID does not exist.
+        m.Member.get(1000)