From 2b1994b98b58d28a6567ce48dbaca346825d4a7b Mon Sep 17 00:00:00 2001 From: Andrew Brookins Date: Wed, 3 Nov 2021 12:37:09 -0700 Subject: [PATCH] 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 | 8 ++ docker-compose.yml | 8 ++ docs/getting_started.md | 10 +- redis_om/checks.py | 28 +++++ redis_om/model/model.py | 46 ++++++++- tests/test_hash_model.py | 4 + tests/test_json_model.py | 6 +- tests/test_oss_redis_features.py | 169 +++++++++++++++++++++++++++++++ 8 files changed, 269 insertions(+), 10 deletions(-) create mode 100644 redis_om/checks.py create mode 100644 tests/test_oss_redis_features.py diff --git a/Makefile b/Makefile index 35d2905..8f3f51f 100644 --- a/Makefile +++ b/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 diff --git a/docker-compose.yml b/docker-compose.yml index 87e406a..f333d22 100644 --- a/docker-compose.yml +++ b/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 \ No newline at end of file diff --git a/docs/getting_started.md b/docs/getting_started.md index 45d05ae..5240403 100644 --- a/docs/getting_started.md +++ b/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 diff --git a/redis_om/checks.py b/redis_om/checks.py new file mode 100644 index 0000000..fde1d87 --- /dev/null +++ b/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 diff --git a/redis_om/model/model.py b/redis_om/model/model.py index 332340d..0effa91 100644 --- a/redis_om/model/model.py +++ b/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._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 @@ 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() diff --git a/tests/test_hash_model.py b/tests/test_hash_model.py index e2a3882..2b4201d 100644 --- a/tests/test_hash_model.py +++ b/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() diff --git a/tests/test_json_model.py b/tests/test_json_model.py index 779da78..166f3c3 100644 --- a/tests/test_json_model.py +++ b/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() diff --git a/tests/test_oss_redis_features.py b/tests/test_oss_redis_features.py new file mode 100644 index 0000000..72eeaf7 --- /dev/null +++ b/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)