From f90ec02cc883a9897547b36560880c5dae51e927 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20L=C3=B3pez?= Date: Wed, 9 Dec 2020 17:26:18 -0600 Subject: [PATCH 01/43] draft --- agave/models/__init__.py | 20 ++++++++++++++++++-- agave/models/base.py | 12 ++++++------ agave/models/mongo.py | 10 ++++++++++ agave/models/redis.py | 10 ++++++++++ 4 files changed, 44 insertions(+), 8 deletions(-) create mode 100644 agave/models/mongo.py create mode 100644 agave/models/redis.py diff --git a/agave/models/__init__.py b/agave/models/__init__.py index 5f4f48a7..0cfc39ac 100644 --- a/agave/models/__init__.py +++ b/agave/models/__init__.py @@ -1,3 +1,19 @@ -__all__ = ['BaseModel'] +__all__ = [] -from .base import BaseModel + +try: + import mongoengine +except ImportError: + ... +else: + from .mongo import MongoModel + __all__.append('MongoModel') + + +try: + import rom +except ImportError: + ... +else: + from .redis import RedisModel + __all__.append('RedisModel') diff --git a/agave/models/base.py b/agave/models/base.py index 4a370ba3..000c233c 100644 --- a/agave/models/base.py +++ b/agave/models/base.py @@ -1,6 +1,6 @@ -from typing import ClassVar, Dict +from typing import Callable, ClassVar -from ..lib.mongoengine.model_helpers import mongo_to_dict +from cuenca_validations.typing import DictStrAny class BaseModel: @@ -10,14 +10,14 @@ class BaseModel: def __init__(self, *args, **values): return super().__init__(*args, **values) - def to_dict(self) -> Dict: + def _dict(self, dict_func: Callable) -> DictStrAny: private_fields = [f for f in dir(self) if f.startswith('_')] excluded = self._excluded + private_fields - mongo_dict: dict = mongo_to_dict(self, excluded) + d: dict = dict_func(self, excluded) for field in self._hidden: - mongo_dict[field] = '********' - return mongo_dict + d[field] = '********' + return d def __repr__(self) -> str: return str(self.to_dict()) # pragma: no cover diff --git a/agave/models/mongo.py b/agave/models/mongo.py new file mode 100644 index 00000000..c73fd85e --- /dev/null +++ b/agave/models/mongo.py @@ -0,0 +1,10 @@ +from cuenca_validations.typing import DictStrAny + +from .base import BaseModel +from ..lib.mongoengine.model_helpers import mongo_to_dict + + +class MongoModel(BaseModel): + + def dict(self) -> DictStrAny: + return self._dict(mongo_to_dict) diff --git a/agave/models/redis.py b/agave/models/redis.py new file mode 100644 index 00000000..ce92f057 --- /dev/null +++ b/agave/models/redis.py @@ -0,0 +1,10 @@ +from cuenca_validations.typing import DictStrAny +from cuenca_validations.validators import sanitize_dict + +from .base import BaseModel + + +class RedisModel(BaseModel): + + def dict(self) -> DictStrAny: + return self._dict(sanitize_dict) From e67aadc95ed5d45c5d5e9d002863c14cf38e1bd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20L=C3=B3pez?= Date: Wed, 9 Dec 2020 18:28:13 -0600 Subject: [PATCH 02/43] filterer --- agave/blueprints/rest_api.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index ef07eb21..fa879e24 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -178,27 +178,24 @@ def query(): # Set user_id request as query param if self.user_id_filter_required(): query_params.user_id = self.current_user_id - filters = cls.get_query_filter(query_params) + filterer = cls.get_query_filter(query_params) if query_params.count: - return _count(filters) - return _all(query_params, filters) + return _count(filterer) + return _all(query_params, filterer) - def _count(filters: Q): - count = cls.model.objects.filter(filters).count() + def _count(query: QueryParams, filterer): + count = filterer(query) return dict(count=count) - def _all(query: QueryParams, filters: Q): + def _all(query: QueryParams, filterer: Callable): if query.limit: limit = min(query.limit, query.page_size) query.limit = max(0, query.limit - limit) # type: ignore else: limit = query.page_size - items = ( - cls.model.objects.order_by("-created_at") - .filter(filters) - .limit(limit) - ) - item_dicts = [i.to_dict() for i in items] + items = filterer(query, limit) + + item_dicts = [i.dict() for i in items] has_more: Optional[bool] = None if wants_more := query.limit is None or query.limit > 0: From 85d2c25771dd808ae01bdf8f8b9901611265d227 Mon Sep 17 00:00:00 2001 From: Glen Date: Thu, 10 Dec 2020 18:20:08 -0600 Subject: [PATCH 03/43] rest-api-sin-codigo-mongo --- agave/blueprints/rest_api.py | 25 +++---- agave/filters.py | 70 +++++++++++++++++++- agave/models/__init__.py | 2 + agave/models/base.py | 2 +- agave/models/helpers.py | 40 +++++++++++ agave/models/mongo.py | 6 +- agave/models/redis.py | 9 ++- examples/chalicelib/models/accounts.py | 6 +- examples/chalicelib/models/accounts_redis.py | 23 +++++++ examples/chalicelib/models/transactions.py | 4 +- examples/chalicelib/resources/accounts.py | 11 +-- requirements-test.txt | 1 + requirements.txt | 2 + tests/blueprint/test_blueprint.py | 10 ++- tests/conftest.py | 30 ++++++++- 15 files changed, 208 insertions(+), 33 deletions(-) create mode 100644 examples/chalicelib/models/accounts_redis.py diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index fa879e24..566b478e 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -1,13 +1,15 @@ -from typing import Optional, Type +from typing import Callable, Optional, Type from urllib.parse import urlencode from chalice import Blueprint, NotFoundError, Response from cuenca_validations.types import QueryParams -from mongoengine import DoesNotExist, Q +from mongoengine import Q from pydantic import BaseModel, ValidationError from .decorators import copy_attributes +from ..filters import get_by_id, get_by_id_and_user + class RestApiBlueprint(Blueprint): def get(self, path: str, **kwargs): @@ -112,10 +114,10 @@ def update(id: str): params = self.current_request.json_body or dict() try: data = cls.update_validator(**params) - model = cls.model.objects.get(id=id) + model = get_by_id(cls, id=id) except ValidationError as e: return Response(e.json(), status_code=400) - except DoesNotExist: + except Exception: raise NotFoundError('Not valid id') else: return cls.update(model, data) @@ -141,13 +143,12 @@ def retrieve(id: str): # retrieve method return cls.retrieve(id) # pragma: no cover try: - id_query = Q(id=id) + data = get_by_id(cls, id=id) if self.user_id_filter_required(): - id_query = id_query & Q(user_id=self.current_user_id) - data = cls.model.objects.get(id_query) - except DoesNotExist: + data = get_by_id_and_user(cls, id=id) + except Exception: raise NotFoundError('Not valid id') - return data.to_dict() + return data.dict() @self.get(path) @copy_attributes(cls) @@ -178,10 +179,10 @@ def query(): # Set user_id request as query param if self.user_id_filter_required(): query_params.user_id = self.current_user_id - filterer = cls.get_query_filter(query_params) + filters = cls.get_query_filter(query_params) if query_params.count: - return _count(filterer) - return _all(query_params, filterer) + return _count(filters) + return _all(query_params, filters) def _count(query: QueryParams, filterer): count = filterer(query) diff --git a/agave/filters.py b/agave/filters.py index b44978ed..b4950c74 100644 --- a/agave/filters.py +++ b/agave/filters.py @@ -1,5 +1,8 @@ +import datetime as dt +from typing import Any, Dict + from cuenca_validations.types import QueryParams -from mongoengine import Q +from mongoengine import DoesNotExist, Q def generic_query(query: QueryParams) -> Q: @@ -20,3 +23,68 @@ def generic_query(query: QueryParams) -> Q: if 'count' in fields: del fields['count'] return filters & Q(**fields) + + +def get_by_id(cls, id: str): + try: + id_obj = cls.model.objects.get(id=id) + except Exception as exc: + if isinstance(exc, DoesNotExist): + raise + id_obj = cls.model.get_by(id=id) + if not id_obj: + raise + return id_obj + + +def get_by_id_and_user(cls, id: str): + try: + id_query = Q(id=id) + id_query = id_query & Q(user_id=cls.current_user_id) + id_obj = cls.model.objects.get(id_query) + except Exception as exc: + if isinstance(exc, DoesNotExist): + raise + id_obj = cls.model.query.filter( + id=id, + user=cls.current_user.o_id, # It can only query the numeric index + ).first() + if not id_obj: + raise + return id_obj + + +def generic_query_redis(query: QueryParams, **kwargs) -> Dict[str, Any]: + filters: Dict[str, Any] = dict() + if query.created_before or query.created_after: + # Restamos o sumamos un microsegundo porque la comparación + # aquí es inclusiva + created_at_lt = ( + query.created_before.replace(tzinfo=None) + + dt.timedelta(microseconds=-1) + if query.created_before + else None + ) + created_at_gt = ( + query.created_after.replace(tzinfo=None) + + dt.timedelta(microseconds=1) + if query.created_after + else None + ) + filters['created_at'] = (created_at_gt, created_at_lt) + exclude_fields = { + 'created_before', + 'created_after', + 'active', + 'limit', + 'page_size', + 'key', + } + fields = query.dict(exclude=exclude_fields) + fields = {**fields, **kwargs} + if 'count' in fields: + del fields['count'] + if len(filters) == 0: + filters = fields + return filters + return filters diff --git a/agave/models/__init__.py b/agave/models/__init__.py index 0cfc39ac..037796cf 100644 --- a/agave/models/__init__.py +++ b/agave/models/__init__.py @@ -7,6 +7,7 @@ ... else: from .mongo import MongoModel + __all__.append('MongoModel') @@ -16,4 +17,5 @@ ... else: from .redis import RedisModel + __all__.append('RedisModel') diff --git a/agave/models/base.py b/agave/models/base.py index 000c233c..3bd31414 100644 --- a/agave/models/base.py +++ b/agave/models/base.py @@ -20,4 +20,4 @@ def _dict(self, dict_func: Callable) -> DictStrAny: return d def __repr__(self) -> str: - return str(self.to_dict()) # pragma: no cover + return str(self.dict()) # pragma: no cover diff --git a/agave/models/helpers.py b/agave/models/helpers.py index 55212984..13ca5145 100644 --- a/agave/models/helpers.py +++ b/agave/models/helpers.py @@ -1,5 +1,9 @@ +import datetime as dt import uuid from base64 import urlsafe_b64encode +from typing import Any + +from rom import Column, Model, PrimaryKey def uuid_field(prefix: str = ''): @@ -7,3 +11,39 @@ def base64_uuid_func() -> str: return prefix + urlsafe_b64encode(uuid.uuid4().bytes).decode()[:-2] return base64_uuid_func + + +class String(Column): + """ + No utilizo la clase String de rom porque todo lo maneja en bytes + codificado en latin-1. + """ + + _allowed = str + + def _to_redis(self, value): + return value.encode('utf-8') + + def _from_redis(self, value): + return value.decode('utf-8') + + +def sanitize_item(item: Any) -> Any: + if isinstance(item, dt.date): + rv = item.isoformat() + elif hasattr(item, 'dict'): + rv = item.dict() + else: + rv = item + return rv + + +def redis_to_dit(obj, exclude_fields: list = None) -> dict: + o_id = PrimaryKey() + excluded = ['o_id'] + response = { + key: sanitize_item(value) + for key, value in obj._data.items() + if key not in excluded + } + return response diff --git a/agave/models/mongo.py b/agave/models/mongo.py index c73fd85e..6cbf540b 100644 --- a/agave/models/mongo.py +++ b/agave/models/mongo.py @@ -1,10 +1,12 @@ from cuenca_validations.typing import DictStrAny +from mongoengine import Document -from .base import BaseModel from ..lib.mongoengine.model_helpers import mongo_to_dict +from .base import BaseModel -class MongoModel(BaseModel): +class MongoModel(BaseModel, Document): + meta = {'allow_inheritance': True} def dict(self) -> DictStrAny: return self._dict(mongo_to_dict) diff --git a/agave/models/redis.py b/agave/models/redis.py index ce92f057..777147cb 100644 --- a/agave/models/redis.py +++ b/agave/models/redis.py @@ -1,10 +1,13 @@ from cuenca_validations.typing import DictStrAny -from cuenca_validations.validators import sanitize_dict +from rom import Model, PrimaryKey + +from agave.models.helpers import redis_to_dit from .base import BaseModel -class RedisModel(BaseModel): +class RedisModel(BaseModel, Model): + o_id = PrimaryKey() # Para que podamos usar `id` en los modelos def dict(self) -> DictStrAny: - return self._dict(sanitize_dict) + return self._dict(redis_to_dit) diff --git a/examples/chalicelib/models/accounts.py b/examples/chalicelib/models/accounts.py index fd6617c2..097be929 100644 --- a/examples/chalicelib/models/accounts.py +++ b/examples/chalicelib/models/accounts.py @@ -1,10 +1,10 @@ -from mongoengine import DateTimeField, Document, StringField +from mongoengine import DateTimeField, StringField -from agave.models import BaseModel +from agave.models.mongo import MongoModel from agave.models.helpers import uuid_field -class Account(BaseModel, Document): +class Account(MongoModel): id = StringField(primary_key=True, default=uuid_field('AC')) name = StringField(required=True) user_id = StringField(required=True) diff --git a/examples/chalicelib/models/accounts_redis.py b/examples/chalicelib/models/accounts_redis.py new file mode 100644 index 00000000..dca88e79 --- /dev/null +++ b/examples/chalicelib/models/accounts_redis.py @@ -0,0 +1,23 @@ +import datetime as dt + +from rom import DateTime, util + +from agave.models.redis import RedisModel +from agave.models.helpers import uuid_field, String + +DEFAULT_MISSING_DATE = dt.datetime.utcfromtimestamp(0) + + +class AccountRedis(RedisModel): + id = String( + default=uuid_field('US'), + required=True, + unique=True, + index=True, + keygen=util.IDENTITY, + ) + name = String(required=True, index=True, keygen=util.IDENTITY) + user_id = String(required=True, index=True, keygen=util.IDENTITY) + secret = String() + created_at = DateTime(default=dt.datetime.utcnow, index=True) + deactivated_at = DateTime(default=DEFAULT_MISSING_DATE, index=True) diff --git a/examples/chalicelib/models/transactions.py b/examples/chalicelib/models/transactions.py index dcde86d4..ffe246e1 100644 --- a/examples/chalicelib/models/transactions.py +++ b/examples/chalicelib/models/transactions.py @@ -1,10 +1,10 @@ from mongoengine import Document, FloatField, StringField -from agave.models import BaseModel +from agave.models.mongo import MongoModel from agave.models.helpers import uuid_field -class Transaction(BaseModel, Document): +class Transaction(MongoModel): id = StringField(primary_key=True, default=uuid_field('TR')) user_id = StringField(required=True) amount = FloatField(required=True) diff --git a/examples/chalicelib/resources/accounts.py b/examples/chalicelib/resources/accounts.py index 37c4b2f0..41445323 100644 --- a/examples/chalicelib/resources/accounts.py +++ b/examples/chalicelib/resources/accounts.py @@ -3,9 +3,10 @@ from chalice import NotFoundError, Response from mongoengine import DoesNotExist -from agave.filters import generic_query +from agave.filters import generic_query, generic_query_redis -from ..models import Account as AccountModel +# from ..models import Account as AccountModel +from ..models.accounts_redis import AccountRedis as AccountModel from ..validators import AccountQuery, AccountRequest, AccountUpdateRequest from .base import app @@ -15,7 +16,7 @@ class Account: model = AccountModel query_validator = AccountQuery update_validator = AccountUpdateRequest - get_query_filter = generic_query + get_query_filter = generic_query_redis @staticmethod @app.validate(AccountRequest) @@ -25,7 +26,7 @@ def create(request: AccountRequest) -> Response: user_id=app.current_user_id, ) account.save() - return Response(account.to_dict(), status_code=201) + return Response(account.dict(), status_code=201) @staticmethod def update( @@ -33,7 +34,7 @@ def update( ) -> Response: account.name = request.name account.save() - return Response(account.to_dict(), status_code=200) + return Response(account.dict(), status_code=200) @staticmethod def delete(id: str) -> Response: diff --git a/requirements-test.txt b/requirements-test.txt index c0523696..ca33b452 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -8,3 +8,4 @@ mypy==0.790 pytest-chalice==0.0.* mongomock==3.21.* mock==4.0.3 +redislite==5.0. diff --git a/requirements.txt b/requirements.txt index 9bdbff20..e6d9ac9f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,3 +3,5 @@ chalice==1.21.6 mongoengine==0.22.1 cuenca-validations==0.6.8 dnspython==2.0.0 +rom==1.0.0 + diff --git a/tests/blueprint/test_blueprint.py b/tests/blueprint/test_blueprint.py index 4324cf7e..260d292f 100644 --- a/tests/blueprint/test_blueprint.py +++ b/tests/blueprint/test_blueprint.py @@ -4,7 +4,8 @@ from chalice.test import Client from mock import MagicMock, patch -from examples.chalicelib.models import Account +# from examples.chalicelib.models import Account +from examples.chalicelib.models.accounts_redis import AccountRedis as Account USER_ID_FILTER_REQUIRED = ( 'examples.chalicelib.blueprints.authed.' @@ -30,7 +31,7 @@ def test_create_resource_bad_request(client: Client) -> None: def test_retrieve_resource(client: Client, account: Account) -> None: resp = client.http.get(f'/accounts/{account.id}') assert resp.status_code == 200 - assert resp.json_body == account.to_dict() + assert resp.json_body == account.dict() @patch(USER_ID_FILTER_REQUIRED, MagicMock(return_value=True)) @@ -68,7 +69,10 @@ def test_update_resource(client: Client, account: Account) -> None: f'/accounts/{account.id}', json=dict(name='Maria Felix'), ) - account.reload() + if isinstance(account, Account): + account.reload() + else: + account.update() assert resp.json_body['name'] == 'Maria Felix' assert account.name == 'Maria Felix' assert resp.status_code == 200 diff --git a/tests/conftest.py b/tests/conftest.py index 9081cf9b..bc42ca14 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,13 +2,41 @@ from typing import Generator, List import pytest +import rom +from _pytest.monkeypatch import MonkeyPatch from chalice.test import Client +from redislite import Redis -from examples.chalicelib.models import Account +from examples.chalicelib.models.accounts_redis import AccountRedis as Account +# from examples.chalicelib.models import Account from .helpers import accept_json +@pytest.fixture(scope='session') +def monkeypatchsession(request): + mpatch = MonkeyPatch() + yield mpatch + mpatch.undo() + + +@pytest.fixture(autouse=True) +def setup_redis(monkeypatchsession) -> Generator[None, None, None]: + # Usa un fake redis para no utilizar un servidor de Redis + redis_connection = Redis('/tmp/redis.db') + monkeypatchsession.setattr( + rom.util, 'get_connection', lambda: redis_connection + ) + yield + + +@pytest.fixture(autouse=True) +def flush_redis() -> Generator[None, None, None]: + yield + redis_connection = Redis('/tmp/redis.db') + redis_connection.flushall() + + @pytest.fixture() def client() -> Generator[Client, None, None]: from examples import app From dfe42ab28fb1df49eec4c3d0796ffac8cf36adde Mon Sep 17 00:00:00 2001 From: Glen Date: Fri, 11 Dec 2020 18:57:54 -0600 Subject: [PATCH 04/43] required-changes: implementar funciones en rest-api, aun faltan mas cambios por hacer --- agave/blueprints/rest_api.py | 22 ++-- agave/filters.py | 107 ++++++++++-------- agave/models/__init__.py | 1 - agave/models/helpers.py | 3 +- examples/chalicelib/resources/accounts.py | 7 +- examples/chalicelib/resources/transactions.py | 4 +- tests/blueprint/test_blueprint.py | 8 +- tests/conftest.py | 3 +- 8 files changed, 79 insertions(+), 76 deletions(-) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index 566b478e..a552fb4c 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -1,14 +1,13 @@ -from typing import Callable, Optional, Type +from typing import Any, Optional, Type from urllib.parse import urlencode from chalice import Blueprint, NotFoundError, Response from cuenca_validations.types import QueryParams -from mongoengine import Q from pydantic import BaseModel, ValidationError from .decorators import copy_attributes -from ..filters import get_by_id, get_by_id_and_user +from ..filters import filter_count, filter_limit, get class RestApiBlueprint(Blueprint): @@ -114,7 +113,7 @@ def update(id: str): params = self.current_request.json_body or dict() try: data = cls.update_validator(**params) - model = get_by_id(cls, id=id) + model = get(cls, id=id) except ValidationError as e: return Response(e.json(), status_code=400) except Exception: @@ -143,9 +142,9 @@ def retrieve(id: str): # retrieve method return cls.retrieve(id) # pragma: no cover try: - data = get_by_id(cls, id=id) + data = get(cls, id=id) if self.user_id_filter_required(): - data = get_by_id_and_user(cls, id=id) + data = get(cls, id=id, user_id=self.current_user_id) except Exception: raise NotFoundError('Not valid id') return data.dict() @@ -184,24 +183,23 @@ def query(): return _count(filters) return _all(query_params, filters) - def _count(query: QueryParams, filterer): - count = filterer(query) + def _count(filters: Any): + count = filter_count(cls, filters) return dict(count=count) - def _all(query: QueryParams, filterer: Callable): + def _all(query: QueryParams, filters: Any): if query.limit: limit = min(query.limit, query.page_size) query.limit = max(0, query.limit - limit) # type: ignore else: limit = query.page_size - items = filterer(query, limit) - + items, items_limit = filter_limit(cls, filters, limit) item_dicts = [i.dict() for i in items] has_more: Optional[bool] = None if wants_more := query.limit is None or query.limit > 0: # only perform this query if it's necessary - has_more = items.limit(limit + 1).count() > limit + has_more = items_limit next_page_uri: Optional[str] = None if wants_more and has_more: diff --git a/agave/filters.py b/agave/filters.py index b4950c74..a75edf3a 100644 --- a/agave/filters.py +++ b/agave/filters.py @@ -1,16 +1,11 @@ import datetime as dt -from typing import Any, Dict +from typing import Any, Dict, Optional, Tuple from cuenca_validations.types import QueryParams from mongoengine import DoesNotExist, Q -def generic_query(query: QueryParams) -> Q: - filters = Q() - if query.created_before: - filters &= Q(created_at__lt=query.created_before) - if query.created_after: - filters &= Q(created_at__gt=query.created_after) +def exclude_fields(query: QueryParams) -> Dict[str, Any]: exclude_fields = { 'created_before', 'created_after', @@ -22,39 +17,20 @@ def generic_query(query: QueryParams) -> Q: fields = query.dict(exclude=exclude_fields) if 'count' in fields: del fields['count'] - return filters & Q(**fields) - - -def get_by_id(cls, id: str): - try: - id_obj = cls.model.objects.get(id=id) - except Exception as exc: - if isinstance(exc, DoesNotExist): - raise - id_obj = cls.model.get_by(id=id) - if not id_obj: - raise - return id_obj + return fields -def get_by_id_and_user(cls, id: str): - try: - id_query = Q(id=id) - id_query = id_query & Q(user_id=cls.current_user_id) - id_obj = cls.model.objects.get(id_query) - except Exception as exc: - if isinstance(exc, DoesNotExist): - raise - id_obj = cls.model.query.filter( - id=id, - user=cls.current_user.o_id, # It can only query the numeric index - ).first() - if not id_obj: - raise - return id_obj +def generic_mongo_query(query: QueryParams) -> Q: + filters = Q() + if query.created_before: + filters &= Q(created_at__lt=query.created_before) + if query.created_after: + filters &= Q(created_at__gt=query.created_after) + fields = exclude_fields(query) + return filters & Q(**fields) -def generic_query_redis(query: QueryParams, **kwargs) -> Dict[str, Any]: +def generic_redis_query(query: QueryParams, **kwargs) -> Dict[str, Any]: filters: Dict[str, Any] = dict() if query.created_before or query.created_after: # Restamos o sumamos un microsegundo porque la comparación @@ -72,19 +48,56 @@ def generic_query_redis(query: QueryParams, **kwargs) -> Dict[str, Any]: else None ) filters['created_at'] = (created_at_gt, created_at_lt) - exclude_fields = { - 'created_before', - 'created_after', - 'active', - 'limit', - 'page_size', - 'key', - } - fields = query.dict(exclude=exclude_fields) + fields = exclude_fields(query) fields = {**fields, **kwargs} - if 'count' in fields: - del fields['count'] if len(filters) == 0: filters = fields return filters return filters + + +def get(cls, id: str, user_id: Optional[str] = None): + try: + id_query = Q(id=id) + if user_id: + id_query = id_query & Q(user_id=user_id) + id_obj = cls.model.objects.get(id_query) + except DoesNotExist: + raise + except Exception: + if user_id: + id_obj = cls.model.query.filter( + id=id, + user_id=user_id, + ).first() + else: + id_obj = cls.model.get_by(id=id) + if not id_obj: + raise + return id_obj + + +def filter_count(cls, filters: Any) -> Dict[str, Any]: + try: + count = cls.model.objects.filter(filters).count() + except Exception: + count = cls.model.query.filter(**filters).count() + return count + + +def filter_limit(cls, filters: Any, limit: int) -> Tuple[any, bool]: + try: + items = ( + cls.model.objects.order_by("-created_at") + .filter(filters) + .limit(limit) + ) + has_more = items.limit(limit + 1).count() > limit + except Exception: + items = ( + cls.model.query.filter(**filters) + .order_by('-created_at') + .limit(0, limit) + ) + has_more = items.limit(0, limit + 1).count() > limit + return items, has_more diff --git a/agave/models/__init__.py b/agave/models/__init__.py index 037796cf..f14e5040 100644 --- a/agave/models/__init__.py +++ b/agave/models/__init__.py @@ -1,6 +1,5 @@ __all__ = [] - try: import mongoengine except ImportError: diff --git a/agave/models/helpers.py b/agave/models/helpers.py index 13ca5145..12d760cd 100644 --- a/agave/models/helpers.py +++ b/agave/models/helpers.py @@ -3,7 +3,7 @@ from base64 import urlsafe_b64encode from typing import Any -from rom import Column, Model, PrimaryKey +from rom import Column def uuid_field(prefix: str = ''): @@ -39,7 +39,6 @@ def sanitize_item(item: Any) -> Any: def redis_to_dit(obj, exclude_fields: list = None) -> dict: - o_id = PrimaryKey() excluded = ['o_id'] response = { key: sanitize_item(value) diff --git a/examples/chalicelib/resources/accounts.py b/examples/chalicelib/resources/accounts.py index 41445323..53005f44 100644 --- a/examples/chalicelib/resources/accounts.py +++ b/examples/chalicelib/resources/accounts.py @@ -3,10 +3,9 @@ from chalice import NotFoundError, Response from mongoengine import DoesNotExist -from agave.filters import generic_query, generic_query_redis +from agave.filters import generic_mongo_query -# from ..models import Account as AccountModel -from ..models.accounts_redis import AccountRedis as AccountModel +from ..models import Account as AccountModel from ..validators import AccountQuery, AccountRequest, AccountUpdateRequest from .base import app @@ -16,7 +15,7 @@ class Account: model = AccountModel query_validator = AccountQuery update_validator = AccountUpdateRequest - get_query_filter = generic_query_redis + get_query_filter = generic_mongo_query @staticmethod @app.validate(AccountRequest) diff --git a/examples/chalicelib/resources/transactions.py b/examples/chalicelib/resources/transactions.py index f940510f..b7ee0309 100644 --- a/examples/chalicelib/resources/transactions.py +++ b/examples/chalicelib/resources/transactions.py @@ -1,4 +1,4 @@ -from agave.filters import generic_query +from agave.filters import generic_mongo_query from ..models.transactions import Transaction as TransactionModel from ..validators import TransactionQuery @@ -9,4 +9,4 @@ class Transaction: model = TransactionModel query_validator = TransactionQuery - get_query_filter = generic_query + get_query_filter = generic_mongo_query diff --git a/tests/blueprint/test_blueprint.py b/tests/blueprint/test_blueprint.py index 260d292f..42a00045 100644 --- a/tests/blueprint/test_blueprint.py +++ b/tests/blueprint/test_blueprint.py @@ -4,8 +4,7 @@ from chalice.test import Client from mock import MagicMock, patch -# from examples.chalicelib.models import Account -from examples.chalicelib.models.accounts_redis import AccountRedis as Account +from examples.chalicelib.models import Account USER_ID_FILTER_REQUIRED = ( 'examples.chalicelib.blueprints.authed.' @@ -69,10 +68,7 @@ def test_update_resource(client: Client, account: Account) -> None: f'/accounts/{account.id}', json=dict(name='Maria Felix'), ) - if isinstance(account, Account): - account.reload() - else: - account.update() + account.reload() assert resp.json_body['name'] == 'Maria Felix' assert account.name == 'Maria Felix' assert resp.status_code == 200 diff --git a/tests/conftest.py b/tests/conftest.py index bc42ca14..ce519513 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,9 +7,8 @@ from chalice.test import Client from redislite import Redis -from examples.chalicelib.models.accounts_redis import AccountRedis as Account +from examples.chalicelib.models import Account -# from examples.chalicelib.models import Account from .helpers import accept_json From 450fea06ba823c489fb390a06c8e7d76d113e4dd Mon Sep 17 00:00:00 2001 From: Glen Date: Tue, 15 Dec 2020 16:55:24 -0600 Subject: [PATCH 05/43] fix: change time assignation logic --- agave/blueprints/rest_api.py | 9 +++-- agave/models/helpers.py | 39 -------------------- agave/models/redis.py | 30 +++++++++++++-- examples/chalicelib/models/accounts_redis.py | 4 +- examples/chalicelib/resources/accounts.py | 3 +- tests/blueprint/test_blueprint.py | 1 + tests/conftest.py | 2 + 7 files changed, 40 insertions(+), 48 deletions(-) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index a552fb4c..ba6df0a3 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -194,7 +194,7 @@ def _all(query: QueryParams, filters: Any): else: limit = query.page_size items, items_limit = filter_limit(cls, filters, limit) - item_dicts = [i.dict() for i in items] + items = list(items) has_more: Optional[bool] = None if wants_more := query.limit is None or query.limit > 0: @@ -203,13 +203,16 @@ def _all(query: QueryParams, filters: Any): next_page_uri: Optional[str] = None if wants_more and has_more: - query.created_before = item_dicts[-1]['created_at'] + query.created_before = items[-1].created_at.isoformat() path = self.current_request.context['resourcePath'] params = query.dict() if self.user_id_filter_required(): params.pop('user_id') next_page_uri = f'{path}?{urlencode(params)}' - return dict(items=item_dicts, next_page_uri=next_page_uri) + return dict( + items=[i.dict() for i in items], + next_page_uri=next_page_uri, + ) return cls diff --git a/agave/models/helpers.py b/agave/models/helpers.py index 12d760cd..55212984 100644 --- a/agave/models/helpers.py +++ b/agave/models/helpers.py @@ -1,9 +1,5 @@ -import datetime as dt import uuid from base64 import urlsafe_b64encode -from typing import Any - -from rom import Column def uuid_field(prefix: str = ''): @@ -11,38 +7,3 @@ def base64_uuid_func() -> str: return prefix + urlsafe_b64encode(uuid.uuid4().bytes).decode()[:-2] return base64_uuid_func - - -class String(Column): - """ - No utilizo la clase String de rom porque todo lo maneja en bytes - codificado en latin-1. - """ - - _allowed = str - - def _to_redis(self, value): - return value.encode('utf-8') - - def _from_redis(self, value): - return value.decode('utf-8') - - -def sanitize_item(item: Any) -> Any: - if isinstance(item, dt.date): - rv = item.isoformat() - elif hasattr(item, 'dict'): - rv = item.dict() - else: - rv = item - return rv - - -def redis_to_dit(obj, exclude_fields: list = None) -> dict: - excluded = ['o_id'] - response = { - key: sanitize_item(value) - for key, value in obj._data.items() - if key not in excluded - } - return response diff --git a/agave/models/redis.py b/agave/models/redis.py index 777147cb..ba54de64 100644 --- a/agave/models/redis.py +++ b/agave/models/redis.py @@ -1,11 +1,35 @@ from cuenca_validations.typing import DictStrAny -from rom import Model, PrimaryKey - -from agave.models.helpers import redis_to_dit +from cuenca_validations.validators import sanitize_item +from rom import Column, Model, PrimaryKey from .base import BaseModel +class String(Column): + """ + No utilizo la clase String de rom porque todo lo maneja en bytes + codificado en latin-1. + """ + + _allowed = str + + def _to_redis(self, value): + return value.encode('utf-8') + + def _from_redis(self, value): + return value.decode('utf-8') + + +def redis_to_dit(obj, exclude_fields: list = None) -> dict: + excluded = ['o_id'] + response = { + key: sanitize_item(value) + for key, value in obj._data.items() + if key not in excluded + } + return response + + class RedisModel(BaseModel, Model): o_id = PrimaryKey() # Para que podamos usar `id` en los modelos diff --git a/examples/chalicelib/models/accounts_redis.py b/examples/chalicelib/models/accounts_redis.py index dca88e79..a2220244 100644 --- a/examples/chalicelib/models/accounts_redis.py +++ b/examples/chalicelib/models/accounts_redis.py @@ -2,8 +2,8 @@ from rom import DateTime, util -from agave.models.redis import RedisModel -from agave.models.helpers import uuid_field, String +from agave.models.redis import RedisModel, String +from agave.models.helpers import uuid_field DEFAULT_MISSING_DATE = dt.datetime.utcfromtimestamp(0) diff --git a/examples/chalicelib/resources/accounts.py b/examples/chalicelib/resources/accounts.py index 53005f44..147e7069 100644 --- a/examples/chalicelib/resources/accounts.py +++ b/examples/chalicelib/resources/accounts.py @@ -3,9 +3,10 @@ from chalice import NotFoundError, Response from mongoengine import DoesNotExist -from agave.filters import generic_mongo_query +from agave.filters import generic_mongo_query, generic_redis_query from ..models import Account as AccountModel +# from ..models.accounts_redis import AccountRedis as AccountModel from ..validators import AccountQuery, AccountRequest, AccountUpdateRequest from .base import app diff --git a/tests/blueprint/test_blueprint.py b/tests/blueprint/test_blueprint.py index 42a00045..a6cefa9a 100644 --- a/tests/blueprint/test_blueprint.py +++ b/tests/blueprint/test_blueprint.py @@ -6,6 +6,7 @@ from examples.chalicelib.models import Account + USER_ID_FILTER_REQUIRED = ( 'examples.chalicelib.blueprints.authed.' 'AuthedBlueprint.user_id_filter_required' diff --git a/tests/conftest.py b/tests/conftest.py index ce519513..966eba9b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,6 +11,8 @@ from .helpers import accept_json +# from examples.chalicelib.models.accounts_redis import AccountRedis as Account + @pytest.fixture(scope='session') def monkeypatchsession(request): From 8978c197cb181ba52dc05f831d2069e9a9cc5389 Mon Sep 17 00:00:00 2001 From: Glen Date: Tue, 15 Dec 2020 17:21:36 -0600 Subject: [PATCH 06/43] format --- agave/blueprints/rest_api.py | 2 +- agave/models/__init__.py | 8 ++++---- examples/chalicelib/resources/accounts.py | 2 +- tests/blueprint/test_blueprint.py | 2 +- tests/blueprint/test_filters.py | 6 +++--- tests/models/test_base.py | 6 +++--- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index ba6df0a3..2e6f28a9 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -210,7 +210,7 @@ def _all(query: QueryParams, filters: Any): params.pop('user_id') next_page_uri = f'{path}?{urlencode(params)}' return dict( - items=[i.dict() for i in items], + items=[i.dict() for i in items], # type: ignore next_page_uri=next_page_uri, ) diff --git a/agave/models/__init__.py b/agave/models/__init__.py index f14e5040..295c436b 100644 --- a/agave/models/__init__.py +++ b/agave/models/__init__.py @@ -1,20 +1,20 @@ __all__ = [] try: - import mongoengine + import mongoengine # noqa except ImportError: ... else: - from .mongo import MongoModel + from .mongo import MongoModel # noqa __all__.append('MongoModel') try: - import rom + import rom # noqa except ImportError: ... else: - from .redis import RedisModel + from .redis import RedisModel # noqa __all__.append('RedisModel') diff --git a/examples/chalicelib/resources/accounts.py b/examples/chalicelib/resources/accounts.py index 147e7069..7685327b 100644 --- a/examples/chalicelib/resources/accounts.py +++ b/examples/chalicelib/resources/accounts.py @@ -45,4 +45,4 @@ def delete(id: str) -> Response: account.deactivated_at = dt.datetime.utcnow().replace(microsecond=0) account.save() - return Response(account.to_dict(), status_code=200) + return Response(account.dict(), status_code=200) diff --git a/tests/blueprint/test_blueprint.py b/tests/blueprint/test_blueprint.py index a6cefa9a..1b708fee 100644 --- a/tests/blueprint/test_blueprint.py +++ b/tests/blueprint/test_blueprint.py @@ -18,7 +18,7 @@ def test_create_resource(client: Client) -> None: resp = client.http.post('/accounts', json=data) model = Account.objects.get(id=resp.json_body['id']) assert resp.status_code == 201 - assert model.to_dict() == resp.json_body + assert model.dict() == resp.json_body model.delete() diff --git a/tests/blueprint/test_filters.py b/tests/blueprint/test_filters.py index b9a58849..d57f2e14 100644 --- a/tests/blueprint/test_filters.py +++ b/tests/blueprint/test_filters.py @@ -2,18 +2,18 @@ from cuenca_validations.types import QueryParams -from agave.filters import generic_query +from agave.filters import generic_mongo_query def test_generic_query_before(): params = QueryParams(created_before=dt.datetime.utcnow().isoformat()) - query = generic_query(params) + query = generic_mongo_query(params) assert "created_at__lt" in repr(query) assert "user" not in repr(query) def test_generic_query_after(): params = QueryParams(created_after=dt.datetime.utcnow().isoformat()) - query = generic_query(params) + query = generic_mongo_query(params) assert "created_at__gt" in repr(query) assert "user" not in repr(query) diff --git a/tests/models/test_base.py b/tests/models/test_base.py index d86f6815..5314848c 100644 --- a/tests/models/test_base.py +++ b/tests/models/test_base.py @@ -1,9 +1,9 @@ from mongoengine import Document, StringField -from agave.models import BaseModel +from agave.models.mongo import MongoModel -class TestModel(BaseModel, Document): +class TestModel(MongoModel, Document): id = StringField() secret_field = StringField() __test__ = False @@ -12,6 +12,6 @@ class TestModel(BaseModel, Document): def test_hide_field(): model = TestModel(id='12345', secret_field='secret') - model_dict = model.to_dict() + model_dict = model.dict() assert model_dict['secret_field'] == '********' assert model_dict['id'] == '12345' From 1accf3e4a55f9d888d6525339345e13aa279c206 Mon Sep 17 00:00:00 2001 From: Glen Date: Wed, 16 Dec 2020 13:54:12 -0600 Subject: [PATCH 07/43] fix: add functions to models --- agave/blueprints/rest_api.py | 16 ++++++----- agave/filters.py | 51 ++---------------------------------- agave/models/mongo.py | 27 ++++++++++++++++++- agave/models/redis.py | 29 ++++++++++++++++++++ 4 files changed, 66 insertions(+), 57 deletions(-) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index 2e6f28a9..f4fb3cd0 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -7,8 +7,6 @@ from .decorators import copy_attributes -from ..filters import filter_count, filter_limit, get - class RestApiBlueprint(Blueprint): def get(self, path: str, **kwargs): @@ -113,7 +111,7 @@ def update(id: str): params = self.current_request.json_body or dict() try: data = cls.update_validator(**params) - model = get(cls, id=id) + model = cls.model.get_id(cls, id=id) except ValidationError as e: return Response(e.json(), status_code=400) except Exception: @@ -142,9 +140,11 @@ def retrieve(id: str): # retrieve method return cls.retrieve(id) # pragma: no cover try: - data = get(cls, id=id) + data = cls.model.get_id(cls, id=id) if self.user_id_filter_required(): - data = get(cls, id=id, user_id=self.current_user_id) + data = cls.model.get_id( + cls, id=id, user_id=self.current_user_id + ) except Exception: raise NotFoundError('Not valid id') return data.dict() @@ -184,7 +184,7 @@ def query(): return _all(query_params, filters) def _count(filters: Any): - count = filter_count(cls, filters) + count = cls.model.filter_count(cls, filters) return dict(count=count) def _all(query: QueryParams, filters: Any): @@ -193,7 +193,9 @@ def _all(query: QueryParams, filters: Any): query.limit = max(0, query.limit - limit) # type: ignore else: limit = query.page_size - items, items_limit = filter_limit(cls, filters, limit) + items, items_limit = cls.model.filter_limit( + cls, filters, limit + ) items = list(items) has_more: Optional[bool] = None diff --git a/agave/filters.py b/agave/filters.py index a75edf3a..f17ecec3 100644 --- a/agave/filters.py +++ b/agave/filters.py @@ -1,8 +1,8 @@ import datetime as dt -from typing import Any, Dict, Optional, Tuple +from typing import Any, Dict from cuenca_validations.types import QueryParams -from mongoengine import DoesNotExist, Q +from mongoengine import Q def exclude_fields(query: QueryParams) -> Dict[str, Any]: @@ -54,50 +54,3 @@ def generic_redis_query(query: QueryParams, **kwargs) -> Dict[str, Any]: filters = fields return filters return filters - - -def get(cls, id: str, user_id: Optional[str] = None): - try: - id_query = Q(id=id) - if user_id: - id_query = id_query & Q(user_id=user_id) - id_obj = cls.model.objects.get(id_query) - except DoesNotExist: - raise - except Exception: - if user_id: - id_obj = cls.model.query.filter( - id=id, - user_id=user_id, - ).first() - else: - id_obj = cls.model.get_by(id=id) - if not id_obj: - raise - return id_obj - - -def filter_count(cls, filters: Any) -> Dict[str, Any]: - try: - count = cls.model.objects.filter(filters).count() - except Exception: - count = cls.model.query.filter(**filters).count() - return count - - -def filter_limit(cls, filters: Any, limit: int) -> Tuple[any, bool]: - try: - items = ( - cls.model.objects.order_by("-created_at") - .filter(filters) - .limit(limit) - ) - has_more = items.limit(limit + 1).count() > limit - except Exception: - items = ( - cls.model.query.filter(**filters) - .order_by('-created_at') - .limit(0, limit) - ) - has_more = items.limit(0, limit + 1).count() > limit - return items, has_more diff --git a/agave/models/mongo.py b/agave/models/mongo.py index 6cbf540b..68eb8c7e 100644 --- a/agave/models/mongo.py +++ b/agave/models/mongo.py @@ -1,5 +1,7 @@ +from typing import Any, Dict, Optional, Tuple + from cuenca_validations.typing import DictStrAny -from mongoengine import Document +from mongoengine import Document, DoesNotExist, Q from ..lib.mongoengine.model_helpers import mongo_to_dict from .base import BaseModel @@ -10,3 +12,26 @@ class MongoModel(BaseModel, Document): def dict(self) -> DictStrAny: return self._dict(mongo_to_dict) + + def get_id(self, id: str, user_id: Optional[str] = None): + try: + id_query = Q(id=id) + if user_id: + id_query = id_query & Q(user_id=user_id) + id_obj = self.model.objects.get(id_query) + except DoesNotExist: + raise + return id_obj + + def filter_count(self, filters: Any) -> Dict[str, Any]: + count = self.model.objects.filter(filters).count() + return count + + def filter_limit(self, filters: Any, limit: int) -> Tuple[any, bool]: + items = ( + self.model.objects.order_by("-created_at") + .filter(filters) + .limit(limit) + ) + has_more = items.limit(limit + 1).count() > limit + return items, has_more diff --git a/agave/models/redis.py b/agave/models/redis.py index ba54de64..6f09ed3d 100644 --- a/agave/models/redis.py +++ b/agave/models/redis.py @@ -1,3 +1,5 @@ +from typing import Any, Dict, Optional, Tuple + from cuenca_validations.typing import DictStrAny from cuenca_validations.validators import sanitize_item from rom import Column, Model, PrimaryKey @@ -31,7 +33,34 @@ def redis_to_dit(obj, exclude_fields: list = None) -> dict: class RedisModel(BaseModel, Model): + meta = {'allow_inheritance': True} o_id = PrimaryKey() # Para que podamos usar `id` en los modelos def dict(self) -> DictStrAny: return self._dict(redis_to_dit) + + def get_id(self, id: str, user_id: Optional[str] = None): + + if user_id: + id_obj = self.model.query.filter( + id=id, + user_id=user_id, + ).first() + else: + id_obj = self.model.get_by(id=id) + if not id_obj: + raise + return id_obj + + def filter_count(self, filters: Any) -> Dict[str, Any]: + count = self.model.query.filter(**filters).count() + return count + + def filter_limit(self, filters: Any, limit: int) -> Tuple[any, bool]: + items = ( + self.model.query.filter(**filters) + .order_by('-created_at') + .limit(0, limit) + ) + has_more = items.limit(0, limit + 1).count() > limit + return items, has_more From 4fed7763f5d7dc5d01635a23a4040238ec165e1e Mon Sep 17 00:00:00 2001 From: Glen Date: Wed, 16 Dec 2020 16:26:38 -0600 Subject: [PATCH 08/43] fix: se modifican nombres de funciones, se agregan nuevas formas de obtener el get en los test --- agave/blueprints/rest_api.py | 8 ++++---- agave/models/mongo.py | 4 ++-- agave/models/redis.py | 4 ++-- examples/chalicelib/resources/accounts.py | 11 ++++++----- tests/blueprint/test_blueprint.py | 14 +++++++++++--- tests/conftest.py | 2 -- 6 files changed, 25 insertions(+), 18 deletions(-) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index f4fb3cd0..da0990b0 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -111,7 +111,7 @@ def update(id: str): params = self.current_request.json_body or dict() try: data = cls.update_validator(**params) - model = cls.model.get_id(cls, id=id) + model = cls.model.retrieve(cls, id=id) except ValidationError as e: return Response(e.json(), status_code=400) except Exception: @@ -140,9 +140,9 @@ def retrieve(id: str): # retrieve method return cls.retrieve(id) # pragma: no cover try: - data = cls.model.get_id(cls, id=id) + data = cls.model.retrieve(cls, id=id) if self.user_id_filter_required(): - data = cls.model.get_id( + data = cls.model.retrieve( cls, id=id, user_id=self.current_user_id ) except Exception: @@ -184,7 +184,7 @@ def query(): return _all(query_params, filters) def _count(filters: Any): - count = cls.model.filter_count(cls, filters) + count = cls.model.count(cls, filters) return dict(count=count) def _all(query: QueryParams, filters: Any): diff --git a/agave/models/mongo.py b/agave/models/mongo.py index 68eb8c7e..2d03d3f0 100644 --- a/agave/models/mongo.py +++ b/agave/models/mongo.py @@ -13,7 +13,7 @@ class MongoModel(BaseModel, Document): def dict(self) -> DictStrAny: return self._dict(mongo_to_dict) - def get_id(self, id: str, user_id: Optional[str] = None): + def retrieve(self, id: str, user_id: Optional[str] = None): try: id_query = Q(id=id) if user_id: @@ -23,7 +23,7 @@ def get_id(self, id: str, user_id: Optional[str] = None): raise return id_obj - def filter_count(self, filters: Any) -> Dict[str, Any]: + def count(self, filters: Any) -> Dict[str, Any]: count = self.model.objects.filter(filters).count() return count diff --git a/agave/models/redis.py b/agave/models/redis.py index 6f09ed3d..5eac6280 100644 --- a/agave/models/redis.py +++ b/agave/models/redis.py @@ -39,7 +39,7 @@ class RedisModel(BaseModel, Model): def dict(self) -> DictStrAny: return self._dict(redis_to_dit) - def get_id(self, id: str, user_id: Optional[str] = None): + def retrieve(self, id: str, user_id: Optional[str] = None): if user_id: id_obj = self.model.query.filter( @@ -52,7 +52,7 @@ def get_id(self, id: str, user_id: Optional[str] = None): raise return id_obj - def filter_count(self, filters: Any) -> Dict[str, Any]: + def count(self, filters: Any) -> Dict[str, Any]: count = self.model.query.filter(**filters).count() return count diff --git a/examples/chalicelib/resources/accounts.py b/examples/chalicelib/resources/accounts.py index 7685327b..65b9cc03 100644 --- a/examples/chalicelib/resources/accounts.py +++ b/examples/chalicelib/resources/accounts.py @@ -1,12 +1,10 @@ import datetime as dt - from chalice import NotFoundError, Response from mongoengine import DoesNotExist -from agave.filters import generic_mongo_query, generic_redis_query +from agave.filters import generic_mongo_query from ..models import Account as AccountModel -# from ..models.accounts_redis import AccountRedis as AccountModel from ..validators import AccountQuery, AccountRequest, AccountUpdateRequest from .base import app @@ -38,11 +36,14 @@ def update( @staticmethod def delete(id: str) -> Response: + account = None try: - account = AccountModel.objects.get(id=id) + account = AccountModel.retrieve(Account, id=id) except DoesNotExist: raise NotFoundError('Not valid id') - + except Exception: + if not account: + raise NotFoundError('Not valid id') account.deactivated_at = dt.datetime.utcnow().replace(microsecond=0) account.save() return Response(account.dict(), status_code=200) diff --git a/tests/blueprint/test_blueprint.py b/tests/blueprint/test_blueprint.py index 1b708fee..9e817fbf 100644 --- a/tests/blueprint/test_blueprint.py +++ b/tests/blueprint/test_blueprint.py @@ -6,6 +6,8 @@ from examples.chalicelib.models import Account +from examples.chalicelib.models.accounts_redis import AccountRedis +from examples.chalicelib.resources.accounts import Account as AccountResource USER_ID_FILTER_REQUIRED = ( 'examples.chalicelib.blueprints.authed.' @@ -16,7 +18,7 @@ def test_create_resource(client: Client) -> None: data = dict(name='Doroteo Arango') resp = client.http.post('/accounts', json=data) - model = Account.objects.get(id=resp.json_body['id']) + model = Account.retrieve(AccountResource, id=resp.json_body['id']) assert resp.status_code == 201 assert model.dict() == resp.json_body model.delete() @@ -69,7 +71,10 @@ def test_update_resource(client: Client, account: Account) -> None: f'/accounts/{account.id}', json=dict(name='Maria Felix'), ) - account.reload() + if issubclass(Account, AccountRedis): + account.update() + else: + account.reload() assert resp.json_body['name'] == 'Maria Felix' assert account.name == 'Maria Felix' assert resp.status_code == 200 @@ -77,7 +82,10 @@ def test_update_resource(client: Client, account: Account) -> None: def test_delete_resource(client: Client, account: Account) -> None: resp = client.http.delete(f'/accounts/{account.id}') - account.reload() + if issubclass(Account, AccountRedis): + account.update() + else: + account.reload() assert resp.status_code == 200 assert resp.json_body['deactivated_at'] is not None assert account.deactivated_at is not None diff --git a/tests/conftest.py b/tests/conftest.py index 966eba9b..ce519513 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,8 +11,6 @@ from .helpers import accept_json -# from examples.chalicelib.models.accounts_redis import AccountRedis as Account - @pytest.fixture(scope='session') def monkeypatchsession(request): From 1d4991601e5f6d2fef01322e61c6d5e62bfadd1f Mon Sep 17 00:00:00 2001 From: Glen Date: Wed, 16 Dec 2020 16:44:39 -0600 Subject: [PATCH 09/43] fix: se agrega exc y se elimina Exception --- agave/blueprints/rest_api.py | 5 +++-- agave/exc.py | 2 ++ agave/models/mongo.py | 3 ++- agave/models/redis.py | 3 ++- 4 files changed, 9 insertions(+), 4 deletions(-) create mode 100644 agave/exc.py diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index da0990b0..a02b2f1e 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -4,6 +4,7 @@ from chalice import Blueprint, NotFoundError, Response from cuenca_validations.types import QueryParams from pydantic import BaseModel, ValidationError +from ..exc import ModelDoesNotExist from .decorators import copy_attributes @@ -114,7 +115,7 @@ def update(id: str): model = cls.model.retrieve(cls, id=id) except ValidationError as e: return Response(e.json(), status_code=400) - except Exception: + except ModelDoesNotExist: raise NotFoundError('Not valid id') else: return cls.update(model, data) @@ -145,7 +146,7 @@ def retrieve(id: str): data = cls.model.retrieve( cls, id=id, user_id=self.current_user_id ) - except Exception: + except ModelDoesNotExist: raise NotFoundError('Not valid id') return data.dict() diff --git a/agave/exc.py b/agave/exc.py new file mode 100644 index 00000000..423ed534 --- /dev/null +++ b/agave/exc.py @@ -0,0 +1,2 @@ +class ModelDoesNotExist(Exception): + '''object does not exist''' diff --git a/agave/models/mongo.py b/agave/models/mongo.py index 2d03d3f0..680d6aef 100644 --- a/agave/models/mongo.py +++ b/agave/models/mongo.py @@ -5,6 +5,7 @@ from ..lib.mongoengine.model_helpers import mongo_to_dict from .base import BaseModel +from ..exc import ModelDoesNotExist class MongoModel(BaseModel, Document): @@ -20,7 +21,7 @@ def retrieve(self, id: str, user_id: Optional[str] = None): id_query = id_query & Q(user_id=user_id) id_obj = self.model.objects.get(id_query) except DoesNotExist: - raise + raise ModelDoesNotExist return id_obj def count(self, filters: Any) -> Dict[str, Any]: diff --git a/agave/models/redis.py b/agave/models/redis.py index 5eac6280..2d5524fc 100644 --- a/agave/models/redis.py +++ b/agave/models/redis.py @@ -5,6 +5,7 @@ from rom import Column, Model, PrimaryKey from .base import BaseModel +from ..exc import ModelDoesNotExist class String(Column): @@ -49,7 +50,7 @@ def retrieve(self, id: str, user_id: Optional[str] = None): else: id_obj = self.model.get_by(id=id) if not id_obj: - raise + raise ModelDoesNotExist return id_obj def count(self, filters: Any) -> Dict[str, Any]: From 43f72031848a7bfc42594415c0c8cacc7183f858 Mon Sep 17 00:00:00 2001 From: Glen Date: Wed, 16 Dec 2020 16:47:14 -0600 Subject: [PATCH 10/43] fix: format --- agave/blueprints/rest_api.py | 1 + agave/models/mongo.py | 2 +- agave/models/redis.py | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index a02b2f1e..c6e53c4c 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -4,6 +4,7 @@ from chalice import Blueprint, NotFoundError, Response from cuenca_validations.types import QueryParams from pydantic import BaseModel, ValidationError + from ..exc import ModelDoesNotExist from .decorators import copy_attributes diff --git a/agave/models/mongo.py b/agave/models/mongo.py index 680d6aef..9e519eb0 100644 --- a/agave/models/mongo.py +++ b/agave/models/mongo.py @@ -3,9 +3,9 @@ from cuenca_validations.typing import DictStrAny from mongoengine import Document, DoesNotExist, Q +from ..exc import ModelDoesNotExist from ..lib.mongoengine.model_helpers import mongo_to_dict from .base import BaseModel -from ..exc import ModelDoesNotExist class MongoModel(BaseModel, Document): diff --git a/agave/models/redis.py b/agave/models/redis.py index 2d5524fc..57e9d3aa 100644 --- a/agave/models/redis.py +++ b/agave/models/redis.py @@ -4,8 +4,8 @@ from cuenca_validations.validators import sanitize_item from rom import Column, Model, PrimaryKey -from .base import BaseModel from ..exc import ModelDoesNotExist +from .base import BaseModel class String(Column): From b3b593b0dc89ec5169d23195eaecf860d513f480 Mon Sep 17 00:00:00 2001 From: Glen Date: Wed, 16 Dec 2020 17:20:11 -0600 Subject: [PATCH 11/43] fix-lint --- agave/models/base.py | 2 +- agave/models/mongo.py | 2 +- agave/models/redis.py | 2 +- examples/chalicelib/resources/accounts.py | 2 +- tests/blueprint/test_blueprint.py | 4 +++- 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/agave/models/base.py b/agave/models/base.py index 3bd31414..f6e676c4 100644 --- a/agave/models/base.py +++ b/agave/models/base.py @@ -20,4 +20,4 @@ def _dict(self, dict_func: Callable) -> DictStrAny: return d def __repr__(self) -> str: - return str(self.dict()) # pragma: no cover + return str(self.dict()) # type: ignore # pragma: no cover diff --git a/agave/models/mongo.py b/agave/models/mongo.py index 9e519eb0..8805e196 100644 --- a/agave/models/mongo.py +++ b/agave/models/mongo.py @@ -28,7 +28,7 @@ def count(self, filters: Any) -> Dict[str, Any]: count = self.model.objects.filter(filters).count() return count - def filter_limit(self, filters: Any, limit: int) -> Tuple[any, bool]: + def filter_limit(self, filters: Any, limit: int) -> Tuple[Any, bool]: items = ( self.model.objects.order_by("-created_at") .filter(filters) diff --git a/agave/models/redis.py b/agave/models/redis.py index 57e9d3aa..bc255696 100644 --- a/agave/models/redis.py +++ b/agave/models/redis.py @@ -57,7 +57,7 @@ def count(self, filters: Any) -> Dict[str, Any]: count = self.model.query.filter(**filters).count() return count - def filter_limit(self, filters: Any, limit: int) -> Tuple[any, bool]: + def filter_limit(self, filters: Any, limit: int) -> Tuple[Any, bool]: items = ( self.model.query.filter(**filters) .order_by('-created_at') diff --git a/examples/chalicelib/resources/accounts.py b/examples/chalicelib/resources/accounts.py index 65b9cc03..348c3b51 100644 --- a/examples/chalicelib/resources/accounts.py +++ b/examples/chalicelib/resources/accounts.py @@ -38,7 +38,7 @@ def update( def delete(id: str) -> Response: account = None try: - account = AccountModel.retrieve(Account, id=id) + account = AccountModel.retrieve(Account, id=id) # type: ignore except DoesNotExist: raise NotFoundError('Not valid id') except Exception: diff --git a/tests/blueprint/test_blueprint.py b/tests/blueprint/test_blueprint.py index 9e817fbf..57ac23aa 100644 --- a/tests/blueprint/test_blueprint.py +++ b/tests/blueprint/test_blueprint.py @@ -18,7 +18,9 @@ def test_create_resource(client: Client) -> None: data = dict(name='Doroteo Arango') resp = client.http.post('/accounts', json=data) - model = Account.retrieve(AccountResource, id=resp.json_body['id']) + model = Account.retrieve( + AccountResource, id=resp.json_body['id'] # type: ignore + ) assert resp.status_code == 201 assert model.dict() == resp.json_body model.delete() From 487b00d3d5b2ed32e632434e12335274b769e7b1 Mon Sep 17 00:00:00 2001 From: Glen Date: Thu, 17 Dec 2020 13:56:41 -0600 Subject: [PATCH 12/43] fix: change-required --- agave/blueprints/rest_api.py | 18 ++++++++--------- agave/exc.py | 2 +- agave/models/mongo.py | 21 ++++++++++---------- agave/models/redis.py | 24 +++++++++++------------ examples/chalicelib/resources/accounts.py | 7 ++++--- tests/blueprint/test_blueprint.py | 5 +---- 6 files changed, 37 insertions(+), 40 deletions(-) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index c6e53c4c..933da8fb 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -5,7 +5,7 @@ from cuenca_validations.types import QueryParams from pydantic import BaseModel, ValidationError -from ..exc import ModelDoesNotExist +from ..exc import ObjectDoesNotExist from .decorators import copy_attributes @@ -113,10 +113,10 @@ def update(id: str): params = self.current_request.json_body or dict() try: data = cls.update_validator(**params) - model = cls.model.retrieve(cls, id=id) + model = cls.model.retrieve(id=id) except ValidationError as e: return Response(e.json(), status_code=400) - except ModelDoesNotExist: + except ObjectDoesNotExist: raise NotFoundError('Not valid id') else: return cls.update(model, data) @@ -142,12 +142,12 @@ def retrieve(id: str): # retrieve method return cls.retrieve(id) # pragma: no cover try: - data = cls.model.retrieve(cls, id=id) + data = cls.model.retrieve(id=id) if self.user_id_filter_required(): data = cls.model.retrieve( - cls, id=id, user_id=self.current_user_id + id=id, user_id=self.current_user_id ) - except ModelDoesNotExist: + except ObjectDoesNotExist: raise NotFoundError('Not valid id') return data.dict() @@ -186,7 +186,7 @@ def query(): return _all(query_params, filters) def _count(filters: Any): - count = cls.model.count(cls, filters) + count = cls.model.count(filters) return dict(count=count) def _all(query: QueryParams, filters: Any): @@ -195,9 +195,7 @@ def _all(query: QueryParams, filters: Any): query.limit = max(0, query.limit - limit) # type: ignore else: limit = query.page_size - items, items_limit = cls.model.filter_limit( - cls, filters, limit - ) + items, items_limit = cls.model.filter_limit(filters, limit) items = list(items) has_more: Optional[bool] = None diff --git a/agave/exc.py b/agave/exc.py index 423ed534..328652be 100644 --- a/agave/exc.py +++ b/agave/exc.py @@ -1,2 +1,2 @@ -class ModelDoesNotExist(Exception): +class ObjectDoesNotExist(Exception): '''object does not exist''' diff --git a/agave/models/mongo.py b/agave/models/mongo.py index 8805e196..716321e7 100644 --- a/agave/models/mongo.py +++ b/agave/models/mongo.py @@ -3,7 +3,7 @@ from cuenca_validations.typing import DictStrAny from mongoengine import Document, DoesNotExist, Q -from ..exc import ModelDoesNotExist +from ..exc import ObjectDoesNotExist from ..lib.mongoengine.model_helpers import mongo_to_dict from .base import BaseModel @@ -14,25 +14,26 @@ class MongoModel(BaseModel, Document): def dict(self) -> DictStrAny: return self._dict(mongo_to_dict) - def retrieve(self, id: str, user_id: Optional[str] = None): + @classmethod + def retrieve(cls, id: str, user_id: Optional[str] = None): try: id_query = Q(id=id) if user_id: id_query = id_query & Q(user_id=user_id) - id_obj = self.model.objects.get(id_query) + id_obj = cls.objects.get(id_query) except DoesNotExist: - raise ModelDoesNotExist + raise ObjectDoesNotExist return id_obj - def count(self, filters: Any) -> Dict[str, Any]: - count = self.model.objects.filter(filters).count() + @classmethod + def count(cls, filters: Any) -> Dict[str, Any]: + count = cls.objects.filter(filters).count() return count - def filter_limit(self, filters: Any, limit: int) -> Tuple[Any, bool]: + @classmethod + def filter_limit(cls, filters: Any, limit: int) -> Tuple[Any, bool]: items = ( - self.model.objects.order_by("-created_at") - .filter(filters) - .limit(limit) + cls.objects.order_by("-created_at").filter(filters).limit(limit) ) has_more = items.limit(limit + 1).count() > limit return items, has_more diff --git a/agave/models/redis.py b/agave/models/redis.py index bc255696..9c087ad6 100644 --- a/agave/models/redis.py +++ b/agave/models/redis.py @@ -4,7 +4,7 @@ from cuenca_validations.validators import sanitize_item from rom import Column, Model, PrimaryKey -from ..exc import ModelDoesNotExist +from ..exc import ObjectDoesNotExist from .base import BaseModel @@ -40,28 +40,28 @@ class RedisModel(BaseModel, Model): def dict(self) -> DictStrAny: return self._dict(redis_to_dit) - def retrieve(self, id: str, user_id: Optional[str] = None): - + @classmethod + def retrieve(cls, id: str, user_id: Optional[str] = None): if user_id: - id_obj = self.model.query.filter( + id_obj = cls.query.filter( id=id, user_id=user_id, ).first() else: - id_obj = self.model.get_by(id=id) + id_obj = cls.get_by(id=id) if not id_obj: - raise ModelDoesNotExist + raise ObjectDoesNotExist return id_obj - def count(self, filters: Any) -> Dict[str, Any]: - count = self.model.query.filter(**filters).count() + @classmethod + def count(cls, filters: Any) -> Dict[str, Any]: + count = cls.query.filter(**filters).count() return count - def filter_limit(self, filters: Any, limit: int) -> Tuple[Any, bool]: + @classmethod + def filter_limit(cls, filters: Any, limit: int) -> Tuple[Any, bool]: items = ( - self.model.query.filter(**filters) - .order_by('-created_at') - .limit(0, limit) + cls.query.filter(**filters).order_by('-created_at').limit(0, limit) ) has_more = items.limit(0, limit + 1).count() > limit return items, has_more diff --git a/examples/chalicelib/resources/accounts.py b/examples/chalicelib/resources/accounts.py index 348c3b51..884a955d 100644 --- a/examples/chalicelib/resources/accounts.py +++ b/examples/chalicelib/resources/accounts.py @@ -2,11 +2,12 @@ from chalice import NotFoundError, Response from mongoengine import DoesNotExist -from agave.filters import generic_mongo_query +from agave.filters import generic_mongo_query, generic_redis_query from ..models import Account as AccountModel from ..validators import AccountQuery, AccountRequest, AccountUpdateRequest from .base import app +from agave.exc import ObjectDoesNotExist @app.resource('/accounts') @@ -38,10 +39,10 @@ def update( def delete(id: str) -> Response: account = None try: - account = AccountModel.retrieve(Account, id=id) # type: ignore + account = AccountModel.retrieve(id=id) # type: ignore except DoesNotExist: raise NotFoundError('Not valid id') - except Exception: + except ObjectDoesNotExist: if not account: raise NotFoundError('Not valid id') account.deactivated_at = dt.datetime.utcnow().replace(microsecond=0) diff --git a/tests/blueprint/test_blueprint.py b/tests/blueprint/test_blueprint.py index 57ac23aa..6d17d37c 100644 --- a/tests/blueprint/test_blueprint.py +++ b/tests/blueprint/test_blueprint.py @@ -7,7 +7,6 @@ from examples.chalicelib.models import Account from examples.chalicelib.models.accounts_redis import AccountRedis -from examples.chalicelib.resources.accounts import Account as AccountResource USER_ID_FILTER_REQUIRED = ( 'examples.chalicelib.blueprints.authed.' @@ -18,9 +17,7 @@ def test_create_resource(client: Client) -> None: data = dict(name='Doroteo Arango') resp = client.http.post('/accounts', json=data) - model = Account.retrieve( - AccountResource, id=resp.json_body['id'] # type: ignore - ) + model = Account.retrieve(id=resp.json_body['id']) # type: ignore assert resp.status_code == 201 assert model.dict() == resp.json_body model.delete() From 72b966656c6fce51c14bc5c460bb3c26ab5b3217 Mon Sep 17 00:00:00 2001 From: Glen Date: Thu, 17 Dec 2020 14:04:30 -0600 Subject: [PATCH 13/43] fix: return int --- agave/models/mongo.py | 2 +- agave/models/redis.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/agave/models/mongo.py b/agave/models/mongo.py index 716321e7..b0a95708 100644 --- a/agave/models/mongo.py +++ b/agave/models/mongo.py @@ -26,7 +26,7 @@ def retrieve(cls, id: str, user_id: Optional[str] = None): return id_obj @classmethod - def count(cls, filters: Any) -> Dict[str, Any]: + def count(cls, filters: Any) -> int: count = cls.objects.filter(filters).count() return count diff --git a/agave/models/redis.py b/agave/models/redis.py index 9c087ad6..391249f6 100644 --- a/agave/models/redis.py +++ b/agave/models/redis.py @@ -1,4 +1,4 @@ -from typing import Any, Dict, Optional, Tuple +from typing import Any, Optional, Tuple from cuenca_validations.typing import DictStrAny from cuenca_validations.validators import sanitize_item @@ -54,7 +54,7 @@ def retrieve(cls, id: str, user_id: Optional[str] = None): return id_obj @classmethod - def count(cls, filters: Any) -> Dict[str, Any]: + def count(cls, filters: Any) -> int: count = cls.query.filter(**filters).count() return count From 48fd0afe562176668db10ea521a9a22a49c8d23d Mon Sep 17 00:00:00 2001 From: Glen Date: Mon, 21 Dec 2020 11:29:56 -0600 Subject: [PATCH 14/43] list --- agave/blueprints/rest_api.py | 1 - agave/models/mongo.py | 1 + agave/models/redis.py | 5 +++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index 933da8fb..61884d91 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -196,7 +196,6 @@ def _all(query: QueryParams, filters: Any): else: limit = query.page_size items, items_limit = cls.model.filter_limit(filters, limit) - items = list(items) has_more: Optional[bool] = None if wants_more := query.limit is None or query.limit > 0: diff --git a/agave/models/mongo.py b/agave/models/mongo.py index b0a95708..21cb9a82 100644 --- a/agave/models/mongo.py +++ b/agave/models/mongo.py @@ -36,4 +36,5 @@ def filter_limit(cls, filters: Any, limit: int) -> Tuple[Any, bool]: cls.objects.order_by("-created_at").filter(filters).limit(limit) ) has_more = items.limit(limit + 1).count() > limit + items = list(items) return items, has_more diff --git a/agave/models/redis.py b/agave/models/redis.py index 391249f6..08d0479c 100644 --- a/agave/models/redis.py +++ b/agave/models/redis.py @@ -1,4 +1,4 @@ -from typing import Any, Optional, Tuple +from typing import Any, List, Optional, Tuple from cuenca_validations.typing import DictStrAny from cuenca_validations.validators import sanitize_item @@ -59,9 +59,10 @@ def count(cls, filters: Any) -> int: return count @classmethod - def filter_limit(cls, filters: Any, limit: int) -> Tuple[Any, bool]: + def filter_limit(cls, filters: Any, limit: int) -> Tuple[List, bool]: items = ( cls.query.filter(**filters).order_by('-created_at').limit(0, limit) ) has_more = items.limit(0, limit + 1).count() > limit + items = list(items) return items, has_more From 95e47c8076a186ee1313e13f56f92eff51532faa Mon Sep 17 00:00:00 2001 From: Glen Date: Mon, 21 Dec 2020 11:31:18 -0600 Subject: [PATCH 15/43] remove-import --- agave/models/mongo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agave/models/mongo.py b/agave/models/mongo.py index 21cb9a82..12b333c6 100644 --- a/agave/models/mongo.py +++ b/agave/models/mongo.py @@ -1,4 +1,4 @@ -from typing import Any, Dict, Optional, Tuple +from typing import Any, Optional, Tuple from cuenca_validations.typing import DictStrAny from mongoengine import Document, DoesNotExist, Q From ef2849e1a3daff709560e06b9bb79394ca844b85 Mon Sep 17 00:00:00 2001 From: Glen Date: Mon, 21 Dec 2020 12:16:01 -0600 Subject: [PATCH 16/43] fixes --- agave/blueprints/rest_api.py | 7 ++-- agave/filters.py | 38 ------------------- agave/models/__init__.py | 1 + agave/models/base.py | 6 +-- agave/models/mongo/__init__.py | 3 ++ agave/models/mongo/filters.py | 14 +++++++ .../models/{mongo.py => mongo/mongo_model.py} | 6 +-- agave/models/redis/__init__.py | 3 ++ agave/models/redis/filters.py | 32 ++++++++++++++++ .../models/{redis.py => redis/redis_model.py} | 4 +- examples/chalicelib/resources/accounts.py | 4 +- examples/chalicelib/resources/transactions.py | 3 +- tests/blueprint/test_filters.py | 2 +- tests/conftest.py | 4 +- 14 files changed, 70 insertions(+), 57 deletions(-) create mode 100644 agave/models/mongo/__init__.py create mode 100644 agave/models/mongo/filters.py rename agave/models/{mongo.py => mongo/mongo_model.py} (88%) create mode 100644 agave/models/redis/__init__.py create mode 100644 agave/models/redis/filters.py rename agave/models/{redis.py => redis/redis_model.py} (95%) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index 61884d91..3dedc58d 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -142,11 +142,10 @@ def retrieve(id: str): # retrieve method return cls.retrieve(id) # pragma: no cover try: - data = cls.model.retrieve(id=id) + user_id: Optional[bool] = None if self.user_id_filter_required(): - data = cls.model.retrieve( - id=id, user_id=self.current_user_id - ) + user_id = self.current_user_id + data = cls.model.retrieve(id, user_id) except ObjectDoesNotExist: raise NotFoundError('Not valid id') return data.dict() diff --git a/agave/filters.py b/agave/filters.py index f17ecec3..721aeb13 100644 --- a/agave/filters.py +++ b/agave/filters.py @@ -1,8 +1,6 @@ -import datetime as dt from typing import Any, Dict from cuenca_validations.types import QueryParams -from mongoengine import Q def exclude_fields(query: QueryParams) -> Dict[str, Any]: @@ -18,39 +16,3 @@ def exclude_fields(query: QueryParams) -> Dict[str, Any]: if 'count' in fields: del fields['count'] return fields - - -def generic_mongo_query(query: QueryParams) -> Q: - filters = Q() - if query.created_before: - filters &= Q(created_at__lt=query.created_before) - if query.created_after: - filters &= Q(created_at__gt=query.created_after) - fields = exclude_fields(query) - return filters & Q(**fields) - - -def generic_redis_query(query: QueryParams, **kwargs) -> Dict[str, Any]: - filters: Dict[str, Any] = dict() - if query.created_before or query.created_after: - # Restamos o sumamos un microsegundo porque la comparación - # aquí es inclusiva - created_at_lt = ( - query.created_before.replace(tzinfo=None) - + dt.timedelta(microseconds=-1) - if query.created_before - else None - ) - created_at_gt = ( - query.created_after.replace(tzinfo=None) - + dt.timedelta(microseconds=1) - if query.created_after - else None - ) - filters['created_at'] = (created_at_gt, created_at_lt) - fields = exclude_fields(query) - fields = {**fields, **kwargs} - if len(filters) == 0: - filters = fields - return filters - return filters diff --git a/agave/models/__init__.py b/agave/models/__init__.py index 295c436b..df73ff5f 100644 --- a/agave/models/__init__.py +++ b/agave/models/__init__.py @@ -15,6 +15,7 @@ except ImportError: ... else: + from .redis import RedisModel # noqa __all__.append('RedisModel') diff --git a/agave/models/base.py b/agave/models/base.py index f6e676c4..1a1f645d 100644 --- a/agave/models/base.py +++ b/agave/models/base.py @@ -13,11 +13,11 @@ def __init__(self, *args, **values): def _dict(self, dict_func: Callable) -> DictStrAny: private_fields = [f for f in dir(self) if f.startswith('_')] excluded = self._excluded + private_fields - d: dict = dict_func(self, excluded) + response = dict_func(self, excluded) for field in self._hidden: - d[field] = '********' - return d + response[field] = '********' + return response def __repr__(self) -> str: return str(self.dict()) # type: ignore # pragma: no cover diff --git a/agave/models/mongo/__init__.py b/agave/models/mongo/__init__.py new file mode 100644 index 00000000..866d39ce --- /dev/null +++ b/agave/models/mongo/__init__.py @@ -0,0 +1,3 @@ +__all__ = ['MongoModel'] + +from .mongo_model import MongoModel diff --git a/agave/models/mongo/filters.py b/agave/models/mongo/filters.py new file mode 100644 index 00000000..8b4db6f9 --- /dev/null +++ b/agave/models/mongo/filters.py @@ -0,0 +1,14 @@ +from cuenca_validations.types import QueryParams +from mongoengine import Q + +from agave.filters import exclude_fields + + +def generic_mongo_query(query: QueryParams) -> Q: + filters = Q() + if query.created_before: + filters &= Q(created_at__lt=query.created_before) + if query.created_after: + filters &= Q(created_at__gt=query.created_after) + fields = exclude_fields(query) + return filters & Q(**fields) diff --git a/agave/models/mongo.py b/agave/models/mongo/mongo_model.py similarity index 88% rename from agave/models/mongo.py rename to agave/models/mongo/mongo_model.py index 12b333c6..95595686 100644 --- a/agave/models/mongo.py +++ b/agave/models/mongo/mongo_model.py @@ -3,9 +3,9 @@ from cuenca_validations.typing import DictStrAny from mongoengine import Document, DoesNotExist, Q -from ..exc import ObjectDoesNotExist -from ..lib.mongoengine.model_helpers import mongo_to_dict -from .base import BaseModel +from agave.exc import ObjectDoesNotExist +from agave.lib.mongoengine.model_helpers import mongo_to_dict +from agave.models.base import BaseModel class MongoModel(BaseModel, Document): diff --git a/agave/models/redis/__init__.py b/agave/models/redis/__init__.py new file mode 100644 index 00000000..b5317760 --- /dev/null +++ b/agave/models/redis/__init__.py @@ -0,0 +1,3 @@ +__all__ = ['RedisModel', 'String'] + +from .redis_model import RedisModel, String diff --git a/agave/models/redis/filters.py b/agave/models/redis/filters.py new file mode 100644 index 00000000..ae12d930 --- /dev/null +++ b/agave/models/redis/filters.py @@ -0,0 +1,32 @@ +import datetime as dt +from typing import Any, Dict + +from cuenca_validations.types import QueryParams + +from agave.filters import exclude_fields + + +def generic_redis_query(query: QueryParams, **kwargs) -> Dict[str, Any]: + filters: Dict[str, Any] = dict() + if query.created_before or query.created_after: + # Restamos o sumamos un microsegundo porque la comparación + # aquí es inclusiva + created_at_lt = ( + query.created_before.replace(tzinfo=None) + + dt.timedelta(microseconds=-1) + if query.created_before + else None + ) + created_at_gt = ( + query.created_after.replace(tzinfo=None) + + dt.timedelta(microseconds=1) + if query.created_after + else None + ) + filters['created_at'] = (created_at_gt, created_at_lt) + fields = exclude_fields(query) + fields = {**fields, **kwargs} + if len(filters) == 0: + filters = fields + return filters + return filters diff --git a/agave/models/redis.py b/agave/models/redis/redis_model.py similarity index 95% rename from agave/models/redis.py rename to agave/models/redis/redis_model.py index 08d0479c..59f7dc79 100644 --- a/agave/models/redis.py +++ b/agave/models/redis/redis_model.py @@ -4,8 +4,8 @@ from cuenca_validations.validators import sanitize_item from rom import Column, Model, PrimaryKey -from ..exc import ObjectDoesNotExist -from .base import BaseModel +from agave.exc import ObjectDoesNotExist +from agave.models.base import BaseModel class String(Column): diff --git a/examples/chalicelib/resources/accounts.py b/examples/chalicelib/resources/accounts.py index 884a955d..043abbc7 100644 --- a/examples/chalicelib/resources/accounts.py +++ b/examples/chalicelib/resources/accounts.py @@ -2,8 +2,8 @@ from chalice import NotFoundError, Response from mongoengine import DoesNotExist -from agave.filters import generic_mongo_query, generic_redis_query - +from agave.models.mongo.filters import generic_mongo_query +from agave.models.redis.filters import generic_redis_query from ..models import Account as AccountModel from ..validators import AccountQuery, AccountRequest, AccountUpdateRequest from .base import app diff --git a/examples/chalicelib/resources/transactions.py b/examples/chalicelib/resources/transactions.py index b7ee0309..19dc080e 100644 --- a/examples/chalicelib/resources/transactions.py +++ b/examples/chalicelib/resources/transactions.py @@ -1,5 +1,4 @@ -from agave.filters import generic_mongo_query - +from agave.models.mongo.filters import generic_mongo_query from ..models.transactions import Transaction as TransactionModel from ..validators import TransactionQuery from .base import app diff --git a/tests/blueprint/test_filters.py b/tests/blueprint/test_filters.py index d57f2e14..31621b5a 100644 --- a/tests/blueprint/test_filters.py +++ b/tests/blueprint/test_filters.py @@ -2,7 +2,7 @@ from cuenca_validations.types import QueryParams -from agave.filters import generic_mongo_query +from agave.models.mongo.filters import generic_mongo_query def test_generic_query_before(): diff --git a/tests/conftest.py b/tests/conftest.py index ce519513..5aa66cf5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,10 +7,10 @@ from chalice.test import Client from redislite import Redis -from examples.chalicelib.models import Account - from .helpers import accept_json +from examples.chalicelib.models import Account + @pytest.fixture(scope='session') def monkeypatchsession(request): From 86f8a1248815607ad75c195facb4390648237519 Mon Sep 17 00:00:00 2001 From: Glen Date: Mon, 21 Dec 2020 12:18:16 -0600 Subject: [PATCH 17/43] format --- tests/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5aa66cf5..ce519513 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,10 +7,10 @@ from chalice.test import Client from redislite import Redis -from .helpers import accept_json - from examples.chalicelib.models import Account +from .helpers import accept_json + @pytest.fixture(scope='session') def monkeypatchsession(request): From c9aa577afd0cad62c6905a9ad24a85bc2aa60cbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20L=C3=B3pez?= Date: Mon, 21 Dec 2020 12:27:46 -0600 Subject: [PATCH 18/43] format --- agave/filters.py | 4 +- agave/models/mongo/filters.py | 2 +- agave/models/redis/filters.py | 2 +- agave/models/redis/redis_model.py | 4 +- tests/blueprint/test_blueprint.py | 10 ++--- tests/conftest.py | 16 ++++++- tests/models/conftest.py | 69 +++++++++++++++++++++++++++++++ 7 files changed, 92 insertions(+), 15 deletions(-) create mode 100644 tests/models/conftest.py diff --git a/agave/filters.py b/agave/filters.py index 721aeb13..803f59c8 100644 --- a/agave/filters.py +++ b/agave/filters.py @@ -4,7 +4,7 @@ def exclude_fields(query: QueryParams) -> Dict[str, Any]: - exclude_fields = { + excluded_fields = { 'created_before', 'created_after', 'active', @@ -12,7 +12,7 @@ def exclude_fields(query: QueryParams) -> Dict[str, Any]: 'page_size', 'key', } - fields = query.dict(exclude=exclude_fields) + fields = query.dict(exclude=excluded_fields) if 'count' in fields: del fields['count'] return fields diff --git a/agave/models/mongo/filters.py b/agave/models/mongo/filters.py index 8b4db6f9..bbbce455 100644 --- a/agave/models/mongo/filters.py +++ b/agave/models/mongo/filters.py @@ -1,7 +1,7 @@ from cuenca_validations.types import QueryParams from mongoengine import Q -from agave.filters import exclude_fields +from ..filters import exclude_fields def generic_mongo_query(query: QueryParams) -> Q: diff --git a/agave/models/redis/filters.py b/agave/models/redis/filters.py index ae12d930..11b91f1c 100644 --- a/agave/models/redis/filters.py +++ b/agave/models/redis/filters.py @@ -3,7 +3,7 @@ from cuenca_validations.types import QueryParams -from agave.filters import exclude_fields +from ..filters import exclude_fields def generic_redis_query(query: QueryParams, **kwargs) -> Dict[str, Any]: diff --git a/agave/models/redis/redis_model.py b/agave/models/redis/redis_model.py index 59f7dc79..a8330f04 100644 --- a/agave/models/redis/redis_model.py +++ b/agave/models/redis/redis_model.py @@ -1,4 +1,4 @@ -from typing import Any, List, Optional, Tuple +from typing import Any, Dict, Optional, Tuple from cuenca_validations.typing import DictStrAny from cuenca_validations.validators import sanitize_item @@ -59,7 +59,7 @@ def count(cls, filters: Any) -> int: return count @classmethod - def filter_limit(cls, filters: Any, limit: int) -> Tuple[List, bool]: + def filter_limit(cls, filters: Dict, limit: int) -> Tuple[Any, bool]: items = ( cls.query.filter(**filters).order_by('-created_at').limit(0, limit) ) diff --git a/tests/blueprint/test_blueprint.py b/tests/blueprint/test_blueprint.py index 6d17d37c..42b4612c 100644 --- a/tests/blueprint/test_blueprint.py +++ b/tests/blueprint/test_blueprint.py @@ -124,21 +124,17 @@ def test_query_all_resource(client: Client) -> None: @pytest.mark.usefixtures('accounts') @patch(USER_ID_FILTER_REQUIRED, MagicMock(return_value=True)) -def test_query_user_id_filter_required(client: Client) -> None: +def test_query_user_id_filter_required(client: Client, user_id: str) -> None: query_params = dict(page_size=2) resp = client.http.get(f'/accounts?{urlencode(query_params)}') assert resp.status_code == 200 assert len(resp.json_body['items']) == 2 - assert all( - item['user_id'] == 'US123456789' for item in resp.json_body['items'] - ) + assert all(item['user_id'] == user_id for item in resp.json_body['items']) resp = client.http.get(resp.json_body['next_page_uri']) assert resp.status_code == 200 assert len(resp.json_body['items']) == 1 - assert all( - item['user_id'] == 'US123456789' for item in resp.json_body['items'] - ) + assert all(item['user_id'] == user_id for item in resp.json_body['items']) def test_query_resource_with_invalid_params(client: Client) -> None: diff --git a/tests/conftest.py b/tests/conftest.py index ce519513..0a90f646 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,6 +12,16 @@ from .helpers import accept_json +@pytest.fixture +def user_id() -> str: + return 'US123456789' + + +@pytest.fixture +def another_user_id() -> str: + return 'US987654321' + + @pytest.fixture(scope='session') def monkeypatchsession(request): mpatch = MonkeyPatch() @@ -55,7 +65,9 @@ def client() -> Generator[Client, None, None]: @pytest.fixture -def accounts() -> Generator[List[Account], None, None]: +def accounts( + user_id: str, another_user_id: str +) -> Generator[List[Account], None, None]: user_id = 'US123456789' accs = [ Account( @@ -75,7 +87,7 @@ def accounts() -> Generator[List[Account], None, None]: ), Account( name='Remedios Varo', - user_id='US987654321', + user_id=another_user_id, created_at=dt.datetime(2020, 4, 1), ), ] diff --git a/tests/models/conftest.py b/tests/models/conftest.py new file mode 100644 index 00000000..37e7ec79 --- /dev/null +++ b/tests/models/conftest.py @@ -0,0 +1,69 @@ +import datetime as dt +from typing import Generator, List, Type, Union + +import pytest + +from agave.models import mongo, redis +from examples.chalicelib.models import mongo_models, redis_models + +DbModel = Union[mongo.MongoModel, redis.RedisModel] + + +def pytest_generate_tests(metafunc): + if "db_model" in metafunc.fixturenames: + metafunc.parametrize( + 'db_model', + [mongo_models.Account, redis_models.Account], + indirect=['db_model'], + ) + + +@pytest.fixture +def db_model(request) -> Type[DbModel]: + return request.param + + +@pytest.fixture +def accounts( + db_model: Type[DbModel], user_id: str, another_user_id: str +) -> Generator[List[DbModel], None, None]: + accs = [ + db_model( + name='Frida Kahlo', + user_id=user_id, + created_at=dt.datetime(2020, 1, 1), + ), + db_model( + name='Sor Juana Inés', + user_id=user_id, + created_at=dt.datetime(2020, 2, 1), + ), + db_model( + name='Leona Vicario', + user_id=user_id, + created_at=dt.datetime(2020, 3, 1), + ), + db_model( + name='Remedios Varo', + user_id=another_user_id, + created_at=dt.datetime(2020, 4, 1), + ), + ] + + for acc in accs: + acc.save() + yield accs + for acc in accs: + acc.delete() + + +@pytest.fixture +def account(accounts: List[DbModel]) -> Generator[DbModel, None, None]: + yield accounts[0] + + +@pytest.fixture +def another_account( + accounts: List[DbModel], +) -> Generator[DbModel, None, None]: + yield accounts[-1] From e6f4d0171c6a10d427948f3024b1439994fc48ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20L=C3=B3pez?= Date: Mon, 21 Dec 2020 12:44:42 -0600 Subject: [PATCH 19/43] test --- agave/models/mongo/filters.py | 2 +- agave/models/redis/filters.py | 2 +- examples/chalicelib/models/__init__.py | 4 ---- .../models/{accounts.py => mongo_models.py} | 8 +++++++- .../models/{accounts_redis.py => redis_models.py} | 0 examples/chalicelib/models/transactions.py | 10 ---------- examples/chalicelib/resources/accounts.py | 3 +-- examples/chalicelib/resources/transactions.py | 2 +- tests/blueprint/test_blueprint.py | 14 ++++---------- tests/conftest.py | 2 +- 10 files changed, 16 insertions(+), 31 deletions(-) rename examples/chalicelib/models/{accounts.py => mongo_models.py} (57%) rename examples/chalicelib/models/{accounts_redis.py => redis_models.py} (100%) diff --git a/agave/models/mongo/filters.py b/agave/models/mongo/filters.py index bbbce455..418f5354 100644 --- a/agave/models/mongo/filters.py +++ b/agave/models/mongo/filters.py @@ -1,7 +1,7 @@ from cuenca_validations.types import QueryParams from mongoengine import Q -from ..filters import exclude_fields +from ...filters import exclude_fields def generic_mongo_query(query: QueryParams) -> Q: diff --git a/agave/models/redis/filters.py b/agave/models/redis/filters.py index 11b91f1c..d834b4bd 100644 --- a/agave/models/redis/filters.py +++ b/agave/models/redis/filters.py @@ -3,7 +3,7 @@ from cuenca_validations.types import QueryParams -from ..filters import exclude_fields +from ...filters import exclude_fields def generic_redis_query(query: QueryParams, **kwargs) -> Dict[str, Any]: diff --git a/examples/chalicelib/models/__init__.py b/examples/chalicelib/models/__init__.py index a3a1c293..e69de29b 100644 --- a/examples/chalicelib/models/__init__.py +++ b/examples/chalicelib/models/__init__.py @@ -1,4 +0,0 @@ -__all__ = ['Account', 'Transaction'] - -from .accounts import Account -from .transactions import Transaction diff --git a/examples/chalicelib/models/accounts.py b/examples/chalicelib/models/mongo_models.py similarity index 57% rename from examples/chalicelib/models/accounts.py rename to examples/chalicelib/models/mongo_models.py index 097be929..10c820e6 100644 --- a/examples/chalicelib/models/accounts.py +++ b/examples/chalicelib/models/mongo_models.py @@ -1,4 +1,4 @@ -from mongoengine import DateTimeField, StringField +from mongoengine import DateTimeField, FloatField, StringField from agave.models.mongo import MongoModel from agave.models.helpers import uuid_field @@ -10,3 +10,9 @@ class Account(MongoModel): user_id = StringField(required=True) created_at = DateTimeField() deactivated_at = DateTimeField() + + +class Transaction(MongoModel): + id = StringField(primary_key=True, default=uuid_field('TR')) + user_id = StringField(required=True) + amount = FloatField(required=True) diff --git a/examples/chalicelib/models/accounts_redis.py b/examples/chalicelib/models/redis_models.py similarity index 100% rename from examples/chalicelib/models/accounts_redis.py rename to examples/chalicelib/models/redis_models.py diff --git a/examples/chalicelib/models/transactions.py b/examples/chalicelib/models/transactions.py index ffe246e1..e69de29b 100644 --- a/examples/chalicelib/models/transactions.py +++ b/examples/chalicelib/models/transactions.py @@ -1,10 +0,0 @@ -from mongoengine import Document, FloatField, StringField - -from agave.models.mongo import MongoModel -from agave.models.helpers import uuid_field - - -class Transaction(MongoModel): - id = StringField(primary_key=True, default=uuid_field('TR')) - user_id = StringField(required=True) - amount = FloatField(required=True) diff --git a/examples/chalicelib/resources/accounts.py b/examples/chalicelib/resources/accounts.py index 043abbc7..676897c9 100644 --- a/examples/chalicelib/resources/accounts.py +++ b/examples/chalicelib/resources/accounts.py @@ -3,8 +3,7 @@ from mongoengine import DoesNotExist from agave.models.mongo.filters import generic_mongo_query -from agave.models.redis.filters import generic_redis_query -from ..models import Account as AccountModel +from ..models.mongo_models import Account as AccountModel from ..validators import AccountQuery, AccountRequest, AccountUpdateRequest from .base import app from agave.exc import ObjectDoesNotExist diff --git a/examples/chalicelib/resources/transactions.py b/examples/chalicelib/resources/transactions.py index 19dc080e..0db4fe24 100644 --- a/examples/chalicelib/resources/transactions.py +++ b/examples/chalicelib/resources/transactions.py @@ -1,5 +1,5 @@ from agave.models.mongo.filters import generic_mongo_query -from ..models.transactions import Transaction as TransactionModel +from ..models.mongo_models import Transaction as TransactionModel from ..validators import TransactionQuery from .base import app diff --git a/tests/blueprint/test_blueprint.py b/tests/blueprint/test_blueprint.py index 42b4612c..8bbc199e 100644 --- a/tests/blueprint/test_blueprint.py +++ b/tests/blueprint/test_blueprint.py @@ -4,9 +4,8 @@ from chalice.test import Client from mock import MagicMock, patch -from examples.chalicelib.models import Account +from examples.chalicelib.models.mongo_models import Account -from examples.chalicelib.models.accounts_redis import AccountRedis USER_ID_FILTER_REQUIRED = ( 'examples.chalicelib.blueprints.authed.' @@ -70,10 +69,8 @@ def test_update_resource(client: Client, account: Account) -> None: f'/accounts/{account.id}', json=dict(name='Maria Felix'), ) - if issubclass(Account, AccountRedis): - account.update() - else: - account.reload() + + account.reload() assert resp.json_body['name'] == 'Maria Felix' assert account.name == 'Maria Felix' assert resp.status_code == 200 @@ -81,10 +78,7 @@ def test_update_resource(client: Client, account: Account) -> None: def test_delete_resource(client: Client, account: Account) -> None: resp = client.http.delete(f'/accounts/{account.id}') - if issubclass(Account, AccountRedis): - account.update() - else: - account.reload() + account.reload() assert resp.status_code == 200 assert resp.json_body['deactivated_at'] is not None assert account.deactivated_at is not None diff --git a/tests/conftest.py b/tests/conftest.py index 0a90f646..56730e1f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,7 +7,7 @@ from chalice.test import Client from redislite import Redis -from examples.chalicelib.models import Account +from examples.chalicelib.models.mongo_models import Account from .helpers import accept_json From 9b9bb022f005e118e64413b630e780d4a9d1676b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20L=C3=B3pez?= Date: Mon, 21 Dec 2020 13:03:55 -0600 Subject: [PATCH 20/43] lint --- agave/models/__init__.py | 3 ++- examples/chalicelib/models/redis_models.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/agave/models/__init__.py b/agave/models/__init__.py index df73ff5f..bb9c4d1e 100644 --- a/agave/models/__init__.py +++ b/agave/models/__init__.py @@ -6,8 +6,9 @@ ... else: from .mongo import MongoModel # noqa + from .mongo.filters import generic_mongo_query # noqa - __all__.append('MongoModel') + __all__.extend(['MongoModel', 'generic_mongo_query']) try: diff --git a/examples/chalicelib/models/redis_models.py b/examples/chalicelib/models/redis_models.py index a2220244..750cbdf8 100644 --- a/examples/chalicelib/models/redis_models.py +++ b/examples/chalicelib/models/redis_models.py @@ -8,7 +8,7 @@ DEFAULT_MISSING_DATE = dt.datetime.utcfromtimestamp(0) -class AccountRedis(RedisModel): +class Account(RedisModel): id = String( default=uuid_field('US'), required=True, From 5433433022f0e074304916b42c17090357757bc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20L=C3=B3pez?= Date: Mon, 21 Dec 2020 13:08:22 -0600 Subject: [PATCH 21/43] lint --- agave/models/mongo/mongo_model.py | 6 +++--- agave/models/redis/redis_model.py | 13 +++++-------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/agave/models/mongo/mongo_model.py b/agave/models/mongo/mongo_model.py index 95595686..0eb06831 100644 --- a/agave/models/mongo/mongo_model.py +++ b/agave/models/mongo/mongo_model.py @@ -17,10 +17,10 @@ def dict(self) -> DictStrAny: @classmethod def retrieve(cls, id: str, user_id: Optional[str] = None): try: - id_query = Q(id=id) + query = Q(id=id) if user_id: - id_query = id_query & Q(user_id=user_id) - id_obj = cls.objects.get(id_query) + query = query & Q(user_id=user_id) + id_obj = cls.objects.get(query) except DoesNotExist: raise ObjectDoesNotExist return id_obj diff --git a/agave/models/redis/redis_model.py b/agave/models/redis/redis_model.py index a8330f04..18726386 100644 --- a/agave/models/redis/redis_model.py +++ b/agave/models/redis/redis_model.py @@ -42,16 +42,13 @@ def dict(self) -> DictStrAny: @classmethod def retrieve(cls, id: str, user_id: Optional[str] = None): + params = dict(id=id) if user_id: - id_obj = cls.query.filter( - id=id, - user_id=user_id, - ).first() - else: - id_obj = cls.get_by(id=id) - if not id_obj: + params['user_id'] = user_id + obj = cls.query.filter(**params).first() + if not obj: raise ObjectDoesNotExist - return id_obj + return obj @classmethod def count(cls, filters: Any) -> int: From c685d8519916ab11a2eb8e3a2eaa9be1da5af682 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20L=C3=B3pez?= Date: Mon, 21 Dec 2020 13:11:02 -0600 Subject: [PATCH 22/43] missing test --- tests/models/test_models.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 tests/models/test_models.py diff --git a/tests/models/test_models.py b/tests/models/test_models.py new file mode 100644 index 00000000..6a3f6568 --- /dev/null +++ b/tests/models/test_models.py @@ -0,0 +1,33 @@ +from typing import Type, Union + +import pytest + +from agave.exc import ObjectDoesNotExist +from agave.models import mongo, redis + +DbModel = Union[mongo.MongoModel, redis.RedisModel] + + +def test_retrieve(db_model: Type[DbModel], account: DbModel) -> None: + obj = db_model.retrieve(account.id) + assert obj.id == account.id + + +def test_retrieve_not_found(db_model: Type[DbModel]) -> None: + with pytest.raises(ObjectDoesNotExist): + db_model.retrieve('unknown-id') + + +def test_retrieve_with_user_id_filter( + db_model: Type[DbModel], account: DbModel, user_id: str +) -> None: + obj = db_model.retrieve(account.id, user_id) + assert obj.id == account.id + assert obj.user_id == user_id + + +def test_retrieve_not_found_with_user_id_filter( + db_model: Type[DbModel], +) -> None: + with pytest.raises(ObjectDoesNotExist): + db_model.retrieve('unknown-id', 'unknown-user-id') From aebbfb82bad9c27786aa5b998952ea9cf8a0288b Mon Sep 17 00:00:00 2001 From: Glen Date: Mon, 21 Dec 2020 18:54:30 -0600 Subject: [PATCH 23/43] test: add-more-test --- agave/blueprints/rest_api.py | 5 +-- agave/models/mongo/mongo_model.py | 11 +++--- agave/models/redis/redis_model.py | 11 +++--- tests/blueprint/test_filters.py | 8 +++++ tests/models/conftest.py | 11 ++++++ tests/models/test_base.py | 19 +++++++--- tests/models/test_models.py | 58 +++++++++++++++++++++++++++++++ 7 files changed, 108 insertions(+), 15 deletions(-) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index 3dedc58d..3922a50b 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -194,12 +194,13 @@ def _all(query: QueryParams, filters: Any): query.limit = max(0, query.limit - limit) # type: ignore else: limit = query.page_size - items, items_limit = cls.model.filter_limit(filters, limit) + items_model = cls.model.filter_limit(filters, limit) + items = list(items_model) has_more: Optional[bool] = None if wants_more := query.limit is None or query.limit > 0: # only perform this query if it's necessary - has_more = items_limit + has_more = cls.model.has_more(items_model, limit) next_page_uri: Optional[str] = None if wants_more and has_more: diff --git a/agave/models/mongo/mongo_model.py b/agave/models/mongo/mongo_model.py index 0eb06831..c770f9a3 100644 --- a/agave/models/mongo/mongo_model.py +++ b/agave/models/mongo/mongo_model.py @@ -1,4 +1,4 @@ -from typing import Any, Optional, Tuple +from typing import Any, Optional from cuenca_validations.typing import DictStrAny from mongoengine import Document, DoesNotExist, Q @@ -31,10 +31,13 @@ def count(cls, filters: Any) -> int: return count @classmethod - def filter_limit(cls, filters: Any, limit: int) -> Tuple[Any, bool]: + def filter_limit(cls, filters: Any, limit: int): items = ( cls.objects.order_by("-created_at").filter(filters).limit(limit) ) + return items + + @classmethod + def has_more(cls, items: Any, limit: int): has_more = items.limit(limit + 1).count() > limit - items = list(items) - return items, has_more + return has_more diff --git a/agave/models/redis/redis_model.py b/agave/models/redis/redis_model.py index 18726386..7b54529f 100644 --- a/agave/models/redis/redis_model.py +++ b/agave/models/redis/redis_model.py @@ -1,4 +1,4 @@ -from typing import Any, Dict, Optional, Tuple +from typing import Any, Dict, Optional from cuenca_validations.typing import DictStrAny from cuenca_validations.validators import sanitize_item @@ -56,10 +56,13 @@ def count(cls, filters: Any) -> int: return count @classmethod - def filter_limit(cls, filters: Dict, limit: int) -> Tuple[Any, bool]: + def filter_limit(cls, filters: Dict, limit: int): items = ( cls.query.filter(**filters).order_by('-created_at').limit(0, limit) ) + return items + + @classmethod + def has_more(cls, items: Any, limit: int): has_more = items.limit(0, limit + 1).count() > limit - items = list(items) - return items, has_more + return has_more diff --git a/tests/blueprint/test_filters.py b/tests/blueprint/test_filters.py index 31621b5a..bb57aef4 100644 --- a/tests/blueprint/test_filters.py +++ b/tests/blueprint/test_filters.py @@ -3,6 +3,7 @@ from cuenca_validations.types import QueryParams from agave.models.mongo.filters import generic_mongo_query +from agave.models.redis.filters import generic_redis_query def test_generic_query_before(): @@ -17,3 +18,10 @@ def test_generic_query_after(): query = generic_mongo_query(params) assert "created_at__gt" in repr(query) assert "user" not in repr(query) + + +def test_generic_query_before_redis(): + params = QueryParams(created_before=dt.datetime.utcnow().isoformat()) + query = generic_redis_query(params) + assert "created_at" in repr(query) + assert "user" not in repr(query) diff --git a/tests/models/conftest.py b/tests/models/conftest.py index 37e7ec79..a1e045d7 100644 --- a/tests/models/conftest.py +++ b/tests/models/conftest.py @@ -6,7 +6,10 @@ from agave.models import mongo, redis from examples.chalicelib.models import mongo_models, redis_models +from ..models.test_base import TestModel, TestModelRedis + DbModel = Union[mongo.MongoModel, redis.RedisModel] +ModelBase = Union[TestModel, TestModelRedis] def pytest_generate_tests(metafunc): @@ -23,6 +26,14 @@ def db_model(request) -> Type[DbModel]: return request.param +@pytest.fixture +def model_base(request) -> Type[ModelBase]: + if request.param == 'mongo': + return TestModel + else: + return TestModelRedis + + @pytest.fixture def accounts( db_model: Type[DbModel], user_id: str, another_user_id: str diff --git a/tests/models/test_base.py b/tests/models/test_base.py index 5314848c..50c3978a 100644 --- a/tests/models/test_base.py +++ b/tests/models/test_base.py @@ -1,6 +1,9 @@ from mongoengine import Document, StringField +from rom import util +from agave.models.helpers import uuid_field from agave.models.mongo import MongoModel +from agave.models.redis import RedisModel, String class TestModel(MongoModel, Document): @@ -10,8 +13,14 @@ class TestModel(MongoModel, Document): _hidden = ['secret_field'] -def test_hide_field(): - model = TestModel(id='12345', secret_field='secret') - model_dict = model.dict() - assert model_dict['secret_field'] == '********' - assert model_dict['id'] == '12345' +class TestModelRedis(RedisModel): + id = String( + default=uuid_field('US'), + required=True, + unique=True, + index=True, + keygen=util.IDENTITY, + ) + secret_field = String(required=True, index=True, keygen=util.IDENTITY) + + _hidden = ['secret_field'] diff --git a/tests/models/test_models.py b/tests/models/test_models.py index 6a3f6568..1c956ef4 100644 --- a/tests/models/test_models.py +++ b/tests/models/test_models.py @@ -4,8 +4,14 @@ from agave.exc import ObjectDoesNotExist from agave.models import mongo, redis +from agave.models.mongo.filters import generic_mongo_query +from agave.models.redis.filters import generic_redis_query +from examples.chalicelib.validators import AccountQuery + +from ..models.test_base import TestModel, TestModelRedis DbModel = Union[mongo.MongoModel, redis.RedisModel] +ModelBase = Union[TestModel, TestModelRedis] def test_retrieve(db_model: Type[DbModel], account: DbModel) -> None: @@ -31,3 +37,55 @@ def test_retrieve_not_found_with_user_id_filter( ) -> None: with pytest.raises(ObjectDoesNotExist): db_model.retrieve('unknown-id', 'unknown-user-id') + + +@pytest.mark.usefixtures('accounts') +def test_query_count(db_model: Type[DbModel]): + params = dict(count=1, name='Frida Kahlo') + query_params = AccountQuery(**params) + if issubclass(db_model, mongo.MongoModel): + filters = generic_mongo_query(query_params) + else: + filters = generic_redis_query(query_params) + count = db_model.count(filters) + assert count == 1 + + +@pytest.mark.usefixtures('accounts') +def test_query_all_with_limit(db_model: Type[DbModel]): + limit = 2 + params = dict(limit=limit) + query_params = AccountQuery(**params) + if issubclass(db_model, mongo.MongoModel): + filters = generic_mongo_query(query_params) + else: + filters = generic_redis_query(query_params) + items = db_model.filter_limit(filters, limit) + items = list(items) + assert len(items) == 2 + + +@pytest.mark.usefixtures('accounts') +def test_query_all_resource(db_model: Type[DbModel]): + params = dict(page_size=2) + limit = 2 + query_params = AccountQuery(**params) + if issubclass(db_model, mongo.MongoModel): + filters = generic_mongo_query(query_params) + else: + filters = generic_redis_query(query_params) + items = db_model.filter_limit(filters, limit) + has_more = db_model.has_more(items, limit) + assert has_more is True + + +@pytest.mark.parametrize( + 'model_base', + ['mongo', 'redis'], + indirect=True, +) +def test_hide_field_redis(model_base: Type[ModelBase]): + model = model_base(id='12345', secret_field='secret') + model_dict = model.dict() + assert model_dict['secret_field'] == '********' + assert model_dict['id'] == '12345' From b81f1a7e975d7b2909427fda52ace889b6091192 Mon Sep 17 00:00:00 2001 From: Glen Date: Mon, 21 Dec 2020 18:58:13 -0600 Subject: [PATCH 24/43] init-models-missing-lines --- agave/models/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agave/models/__init__.py b/agave/models/__init__.py index bb9c4d1e..56ad998f 100644 --- a/agave/models/__init__.py +++ b/agave/models/__init__.py @@ -2,7 +2,7 @@ try: import mongoengine # noqa -except ImportError: +except ImportError: # pragma: no cover ... else: from .mongo import MongoModel # noqa @@ -13,7 +13,7 @@ try: import rom # noqa -except ImportError: +except ImportError: # pragma: no cover ... else: From 7c93282580df1511507e5738838677b891c904ba Mon Sep 17 00:00:00 2001 From: Glen Date: Mon, 21 Dec 2020 18:59:32 -0600 Subject: [PATCH 25/43] rename-test --- tests/blueprint/test_filters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/blueprint/test_filters.py b/tests/blueprint/test_filters.py index bb57aef4..14100a6e 100644 --- a/tests/blueprint/test_filters.py +++ b/tests/blueprint/test_filters.py @@ -20,7 +20,7 @@ def test_generic_query_after(): assert "user" not in repr(query) -def test_generic_query_before_redis(): +def test_generic_query_redis(): params = QueryParams(created_before=dt.datetime.utcnow().isoformat()) query = generic_redis_query(params) assert "created_at" in repr(query) From 8b13889bc8cde721593fb47d8104416eedd69466 Mon Sep 17 00:00:00 2001 From: Glen Date: Tue, 22 Dec 2020 12:34:37 -0600 Subject: [PATCH 26/43] rebase-main --- agave/blueprints/rest_api.py | 1 - requirements-test.txt | 3 +-- tests/blueprint/test_blueprint.py | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index 3922a50b..5c59b0b9 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -6,7 +6,6 @@ from pydantic import BaseModel, ValidationError from ..exc import ObjectDoesNotExist - from .decorators import copy_attributes diff --git a/requirements-test.txt b/requirements-test.txt index ca33b452..7c7877db 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -7,5 +7,4 @@ flake8==3.8.* mypy==0.790 pytest-chalice==0.0.* mongomock==3.21.* -mock==4.0.3 -redislite==5.0. +mock==4.0.3 \ No newline at end of file diff --git a/tests/blueprint/test_blueprint.py b/tests/blueprint/test_blueprint.py index 8bbc199e..c15fa7e8 100644 --- a/tests/blueprint/test_blueprint.py +++ b/tests/blueprint/test_blueprint.py @@ -6,7 +6,6 @@ from examples.chalicelib.models.mongo_models import Account - USER_ID_FILTER_REQUIRED = ( 'examples.chalicelib.blueprints.authed.' 'AuthedBlueprint.user_id_filter_required' From fbc7c388fcfc6c941c222d6506ac662ed5c182fa Mon Sep 17 00:00:00 2001 From: Glen Date: Tue, 22 Dec 2020 12:39:20 -0600 Subject: [PATCH 27/43] add-requirements-test --- requirements-test.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/requirements-test.txt b/requirements-test.txt index 7c7877db..1df8f17e 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -7,4 +7,5 @@ flake8==3.8.* mypy==0.790 pytest-chalice==0.0.* mongomock==3.21.* -mock==4.0.3 \ No newline at end of file +mock==4.0.3 +redislite==5.0.* From 3869cdc9dad61e8a5785f0b25bf5caa04d88d85c Mon Sep 17 00:00:00 2001 From: Glen Date: Tue, 22 Dec 2020 12:42:10 -0600 Subject: [PATCH 28/43] fix: add-filter-import --- agave/models/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/agave/models/__init__.py b/agave/models/__init__.py index 56ad998f..93a3e24e 100644 --- a/agave/models/__init__.py +++ b/agave/models/__init__.py @@ -18,5 +18,6 @@ else: from .redis import RedisModel # noqa + from .redis.filters import generic_redis_query # noqa - __all__.append('RedisModel') + __all__.extend(['RedisModel', 'generic_redis_query']) From 3a706b8285987fe0e625711c13289ff6fdf32347 Mon Sep 17 00:00:00 2001 From: Glen Date: Tue, 22 Dec 2020 14:31:37 -0600 Subject: [PATCH 29/43] more-changes-required --- examples/chalicelib/models/mongo_models.py | 3 +++ examples/chalicelib/models/redis_models.py | 4 +++- tests/models/conftest.py | 11 --------- tests/models/test_base.py | 26 ---------------------- tests/models/test_models.py | 14 ++++-------- 5 files changed, 10 insertions(+), 48 deletions(-) delete mode 100644 tests/models/test_base.py diff --git a/examples/chalicelib/models/mongo_models.py b/examples/chalicelib/models/mongo_models.py index 10c820e6..fb6e13a3 100644 --- a/examples/chalicelib/models/mongo_models.py +++ b/examples/chalicelib/models/mongo_models.py @@ -10,6 +10,9 @@ class Account(MongoModel): user_id = StringField(required=True) created_at = DateTimeField() deactivated_at = DateTimeField() + secret_field = StringField() + __test__ = False + _hidden = ['secret_field'] class Transaction(MongoModel): diff --git a/examples/chalicelib/models/redis_models.py b/examples/chalicelib/models/redis_models.py index 750cbdf8..9aecbf9c 100644 --- a/examples/chalicelib/models/redis_models.py +++ b/examples/chalicelib/models/redis_models.py @@ -18,6 +18,8 @@ class Account(RedisModel): ) name = String(required=True, index=True, keygen=util.IDENTITY) user_id = String(required=True, index=True, keygen=util.IDENTITY) - secret = String() created_at = DateTime(default=dt.datetime.utcnow, index=True) deactivated_at = DateTime(default=DEFAULT_MISSING_DATE, index=True) + secret_field = String(index=True, keygen=util.IDENTITY) + __test__ = False + _hidden = ['secret_field'] diff --git a/tests/models/conftest.py b/tests/models/conftest.py index a1e045d7..37e7ec79 100644 --- a/tests/models/conftest.py +++ b/tests/models/conftest.py @@ -6,10 +6,7 @@ from agave.models import mongo, redis from examples.chalicelib.models import mongo_models, redis_models -from ..models.test_base import TestModel, TestModelRedis - DbModel = Union[mongo.MongoModel, redis.RedisModel] -ModelBase = Union[TestModel, TestModelRedis] def pytest_generate_tests(metafunc): @@ -26,14 +23,6 @@ def db_model(request) -> Type[DbModel]: return request.param -@pytest.fixture -def model_base(request) -> Type[ModelBase]: - if request.param == 'mongo': - return TestModel - else: - return TestModelRedis - - @pytest.fixture def accounts( db_model: Type[DbModel], user_id: str, another_user_id: str diff --git a/tests/models/test_base.py b/tests/models/test_base.py deleted file mode 100644 index 50c3978a..00000000 --- a/tests/models/test_base.py +++ /dev/null @@ -1,26 +0,0 @@ -from mongoengine import Document, StringField -from rom import util - -from agave.models.helpers import uuid_field -from agave.models.mongo import MongoModel -from agave.models.redis import RedisModel, String - - -class TestModel(MongoModel, Document): - id = StringField() - secret_field = StringField() - __test__ = False - _hidden = ['secret_field'] - - -class TestModelRedis(RedisModel): - id = String( - default=uuid_field('US'), - required=True, - unique=True, - index=True, - keygen=util.IDENTITY, - ) - secret_field = String(required=True, index=True, keygen=util.IDENTITY) - - _hidden = ['secret_field'] diff --git a/tests/models/test_models.py b/tests/models/test_models.py index 1c956ef4..cf5821d3 100644 --- a/tests/models/test_models.py +++ b/tests/models/test_models.py @@ -8,10 +8,7 @@ from agave.models.redis.filters import generic_redis_query from examples.chalicelib.validators import AccountQuery -from ..models.test_base import TestModel, TestModelRedis - DbModel = Union[mongo.MongoModel, redis.RedisModel] -ModelBase = Union[TestModel, TestModelRedis] def test_retrieve(db_model: Type[DbModel], account: DbModel) -> None: @@ -79,13 +76,10 @@ def test_query_all_resource(db_model: Type[DbModel]): assert has_more is True -@pytest.mark.parametrize( - 'model_base', - ['mongo', 'redis'], - indirect=True, -) -def test_hide_field_redis(model_base: Type[ModelBase]): - model = model_base(id='12345', secret_field='secret') +def test_hide_field_redis(db_model: Type[DbModel]): + model = db_model( + id='12345', secret_field='secret', name='frida', user_id='w72638' + ) model_dict = model.dict() assert model_dict['secret_field'] == '********' assert model_dict['id'] == '12345' From 21034559d3793f1fb8562f754fb682bd992c063c Mon Sep 17 00:00:00 2001 From: Glen Date: Tue, 22 Dec 2020 17:37:55 -0600 Subject: [PATCH 30/43] fix: syntax, type, exc, filters --- agave/blueprints/rest_api.py | 10 +++++----- agave/exc.py | 2 +- agave/models/mongo/mongo_model.py | 16 ++++++++-------- agave/models/redis/filters.py | 2 +- agave/models/redis/redis_model.py | 8 ++++---- examples/chalicelib/resources/accounts.py | 4 ++-- tests/models/test_models.py | 14 +++++++------- 7 files changed, 28 insertions(+), 28 deletions(-) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index 5c59b0b9..f66b5715 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -5,7 +5,7 @@ from cuenca_validations.types import QueryParams from pydantic import BaseModel, ValidationError -from ..exc import ObjectDoesNotExist +from ..exc import DoesNotExist from .decorators import copy_attributes @@ -115,7 +115,7 @@ def update(id: str): model = cls.model.retrieve(id=id) except ValidationError as e: return Response(e.json(), status_code=400) - except ObjectDoesNotExist: + except DoesNotExist: raise NotFoundError('Not valid id') else: return cls.update(model, data) @@ -144,8 +144,8 @@ def retrieve(id: str): user_id: Optional[bool] = None if self.user_id_filter_required(): user_id = self.current_user_id - data = cls.model.retrieve(id, user_id) - except ObjectDoesNotExist: + data = cls.model.retrieve(id, user_id=user_id) + except DoesNotExist: raise NotFoundError('Not valid id') return data.dict() @@ -193,7 +193,7 @@ def _all(query: QueryParams, filters: Any): query.limit = max(0, query.limit - limit) # type: ignore else: limit = query.page_size - items_model = cls.model.filter_limit(filters, limit) + items_model = cls.model.all(filters, limit=limit) items = list(items_model) has_more: Optional[bool] = None diff --git a/agave/exc.py b/agave/exc.py index 328652be..5cdf3691 100644 --- a/agave/exc.py +++ b/agave/exc.py @@ -1,2 +1,2 @@ -class ObjectDoesNotExist(Exception): +class DoesNotExist(Exception): '''object does not exist''' diff --git a/agave/models/mongo/mongo_model.py b/agave/models/mongo/mongo_model.py index c770f9a3..e49ced58 100644 --- a/agave/models/mongo/mongo_model.py +++ b/agave/models/mongo/mongo_model.py @@ -3,7 +3,7 @@ from cuenca_validations.typing import DictStrAny from mongoengine import Document, DoesNotExist, Q -from agave.exc import ObjectDoesNotExist +from agave.exc import DoesNotExist as AgaveDoesNotExist from agave.lib.mongoengine.model_helpers import mongo_to_dict from agave.models.base import BaseModel @@ -15,23 +15,23 @@ def dict(self) -> DictStrAny: return self._dict(mongo_to_dict) @classmethod - def retrieve(cls, id: str, user_id: Optional[str] = None): + def retrieve(cls, id: str, *, user_id: Optional[str] = None): + query = Q(id=id) + if user_id: + query = query & Q(user_id=user_id) try: - query = Q(id=id) - if user_id: - query = query & Q(user_id=user_id) id_obj = cls.objects.get(query) except DoesNotExist: - raise ObjectDoesNotExist + raise AgaveDoesNotExist return id_obj @classmethod - def count(cls, filters: Any) -> int: + def count(cls, filters) -> int: count = cls.objects.filter(filters).count() return count @classmethod - def filter_limit(cls, filters: Any, limit: int): + def all(cls, filters, *, limit: int): items = ( cls.objects.order_by("-created_at").filter(filters).limit(limit) ) diff --git a/agave/models/redis/filters.py b/agave/models/redis/filters.py index d834b4bd..a3aa30da 100644 --- a/agave/models/redis/filters.py +++ b/agave/models/redis/filters.py @@ -26,7 +26,7 @@ def generic_redis_query(query: QueryParams, **kwargs) -> Dict[str, Any]: filters['created_at'] = (created_at_gt, created_at_lt) fields = exclude_fields(query) fields = {**fields, **kwargs} - if len(filters) == 0: + if not filters: filters = fields return filters return filters diff --git a/agave/models/redis/redis_model.py b/agave/models/redis/redis_model.py index 7b54529f..1564dce4 100644 --- a/agave/models/redis/redis_model.py +++ b/agave/models/redis/redis_model.py @@ -4,7 +4,7 @@ from cuenca_validations.validators import sanitize_item from rom import Column, Model, PrimaryKey -from agave.exc import ObjectDoesNotExist +from agave.exc import DoesNotExist from agave.models.base import BaseModel @@ -41,13 +41,13 @@ def dict(self) -> DictStrAny: return self._dict(redis_to_dit) @classmethod - def retrieve(cls, id: str, user_id: Optional[str] = None): + def retrieve(cls, id: str, *, user_id: Optional[str] = None): params = dict(id=id) if user_id: params['user_id'] = user_id obj = cls.query.filter(**params).first() if not obj: - raise ObjectDoesNotExist + raise DoesNotExist return obj @classmethod @@ -56,7 +56,7 @@ def count(cls, filters: Any) -> int: return count @classmethod - def filter_limit(cls, filters: Dict, limit: int): + def all(cls, filters: Dict, *, limit: int): items = ( cls.query.filter(**filters).order_by('-created_at').limit(0, limit) ) diff --git a/examples/chalicelib/resources/accounts.py b/examples/chalicelib/resources/accounts.py index 676897c9..972769ae 100644 --- a/examples/chalicelib/resources/accounts.py +++ b/examples/chalicelib/resources/accounts.py @@ -6,7 +6,7 @@ from ..models.mongo_models import Account as AccountModel from ..validators import AccountQuery, AccountRequest, AccountUpdateRequest from .base import app -from agave.exc import ObjectDoesNotExist +from agave.exc import DoesNotExist @app.resource('/accounts') @@ -41,7 +41,7 @@ def delete(id: str) -> Response: account = AccountModel.retrieve(id=id) # type: ignore except DoesNotExist: raise NotFoundError('Not valid id') - except ObjectDoesNotExist: + except Exception: if not account: raise NotFoundError('Not valid id') account.deactivated_at = dt.datetime.utcnow().replace(microsecond=0) diff --git a/tests/models/test_models.py b/tests/models/test_models.py index cf5821d3..99fd087c 100644 --- a/tests/models/test_models.py +++ b/tests/models/test_models.py @@ -2,7 +2,7 @@ import pytest -from agave.exc import ObjectDoesNotExist +from agave.exc import DoesNotExist from agave.models import mongo, redis from agave.models.mongo.filters import generic_mongo_query from agave.models.redis.filters import generic_redis_query @@ -17,14 +17,14 @@ def test_retrieve(db_model: Type[DbModel], account: DbModel) -> None: def test_retrieve_not_found(db_model: Type[DbModel]) -> None: - with pytest.raises(ObjectDoesNotExist): + with pytest.raises(DoesNotExist): db_model.retrieve('unknown-id') def test_retrieve_with_user_id_filter( db_model: Type[DbModel], account: DbModel, user_id: str ) -> None: - obj = db_model.retrieve(account.id, user_id) + obj = db_model.retrieve(account.id, user_id=user_id) assert obj.id == account.id assert obj.user_id == user_id @@ -32,8 +32,8 @@ def test_retrieve_with_user_id_filter( def test_retrieve_not_found_with_user_id_filter( db_model: Type[DbModel], ) -> None: - with pytest.raises(ObjectDoesNotExist): - db_model.retrieve('unknown-id', 'unknown-user-id') + with pytest.raises(DoesNotExist): + db_model.retrieve('unknown-id', user_id='unknown-user-id') @pytest.mark.usefixtures('accounts') @@ -57,7 +57,7 @@ def test_query_all_with_limit(db_model: Type[DbModel]): filters = generic_mongo_query(query_params) else: filters = generic_redis_query(query_params) - items = db_model.filter_limit(filters, limit) + items = db_model.all(filters, limit=limit) items = list(items) assert len(items) == 2 @@ -71,7 +71,7 @@ def test_query_all_resource(db_model: Type[DbModel]): filters = generic_mongo_query(query_params) else: filters = generic_redis_query(query_params) - items = db_model.filter_limit(filters, limit) + items = db_model.all(filters, limit=limit) has_more = db_model.has_more(items, limit) assert has_more is True From a5e981ee3fab137033061b35cb56c4ebdf34fdb6 Mon Sep 17 00:00:00 2001 From: Glen Date: Tue, 22 Dec 2020 17:43:13 -0600 Subject: [PATCH 31/43] fix: add typehint --- agave/models/mongo/mongo_model.py | 6 +++--- agave/models/redis/redis_model.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/agave/models/mongo/mongo_model.py b/agave/models/mongo/mongo_model.py index e49ced58..32dff929 100644 --- a/agave/models/mongo/mongo_model.py +++ b/agave/models/mongo/mongo_model.py @@ -26,18 +26,18 @@ def retrieve(cls, id: str, *, user_id: Optional[str] = None): return id_obj @classmethod - def count(cls, filters) -> int: + def count(cls, filters: Q) -> int: count = cls.objects.filter(filters).count() return count @classmethod - def all(cls, filters, *, limit: int): + def all(cls, filters: Q, *, limit: int): items = ( cls.objects.order_by("-created_at").filter(filters).limit(limit) ) return items @classmethod - def has_more(cls, items: Any, limit: int): + def has_more(cls, items: Q, limit: int): has_more = items.limit(limit + 1).count() > limit return has_more diff --git a/agave/models/redis/redis_model.py b/agave/models/redis/redis_model.py index 1564dce4..5d419f3c 100644 --- a/agave/models/redis/redis_model.py +++ b/agave/models/redis/redis_model.py @@ -51,7 +51,7 @@ def retrieve(cls, id: str, *, user_id: Optional[str] = None): return obj @classmethod - def count(cls, filters: Any) -> int: + def count(cls, filters: Dict) -> int: count = cls.query.filter(**filters).count() return count @@ -63,6 +63,6 @@ def all(cls, filters: Dict, *, limit: int): return items @classmethod - def has_more(cls, items: Any, limit: int): + def has_more(cls, items, limit: int): has_more = items.limit(0, limit + 1).count() > limit return has_more From e3c02ac8c6289a46096e57f4cd3f65a30a7facbd Mon Sep 17 00:00:00 2001 From: Glen Date: Tue, 22 Dec 2020 17:45:05 -0600 Subject: [PATCH 32/43] fix: format --- agave/models/mongo/mongo_model.py | 2 +- agave/models/redis/redis_model.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/agave/models/mongo/mongo_model.py b/agave/models/mongo/mongo_model.py index 32dff929..8c2fdff1 100644 --- a/agave/models/mongo/mongo_model.py +++ b/agave/models/mongo/mongo_model.py @@ -1,4 +1,4 @@ -from typing import Any, Optional +from typing import Optional from cuenca_validations.typing import DictStrAny from mongoengine import Document, DoesNotExist, Q diff --git a/agave/models/redis/redis_model.py b/agave/models/redis/redis_model.py index 5d419f3c..4911cf8d 100644 --- a/agave/models/redis/redis_model.py +++ b/agave/models/redis/redis_model.py @@ -1,4 +1,4 @@ -from typing import Any, Dict, Optional +from typing import Dict, Optional from cuenca_validations.typing import DictStrAny from cuenca_validations.validators import sanitize_item From 513012765cc05d61f1c33719b0c855c96bb5f634 Mon Sep 17 00:00:00 2001 From: Glen Date: Tue, 22 Dec 2020 18:05:27 -0600 Subject: [PATCH 33/43] fix: return statements --- agave/models/redis/filters.py | 1 - 1 file changed, 1 deletion(-) diff --git a/agave/models/redis/filters.py b/agave/models/redis/filters.py index a3aa30da..6ee96ac7 100644 --- a/agave/models/redis/filters.py +++ b/agave/models/redis/filters.py @@ -28,5 +28,4 @@ def generic_redis_query(query: QueryParams, **kwargs) -> Dict[str, Any]: fields = {**fields, **kwargs} if not filters: filters = fields - return filters return filters From 8457644604d661b50a5e5936d08f2491e39e162e Mon Sep 17 00:00:00 2001 From: Glen Date: Tue, 22 Dec 2020 19:12:29 -0600 Subject: [PATCH 34/43] fix: return items, has_more --- agave/blueprints/rest_api.py | 6 ++---- agave/models/mongo/mongo_model.py | 11 ++++------- agave/models/redis/redis_model.py | 11 ++++------- tests/models/test_models.py | 4 +--- 4 files changed, 11 insertions(+), 21 deletions(-) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index f66b5715..679fac7c 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -193,13 +193,11 @@ def _all(query: QueryParams, filters: Any): query.limit = max(0, query.limit - limit) # type: ignore else: limit = query.page_size - items_model = cls.model.all(filters, limit=limit) - items = list(items_model) + items, has_more = cls.model.all(filters, limit=limit) - has_more: Optional[bool] = None if wants_more := query.limit is None or query.limit > 0: # only perform this query if it's necessary - has_more = cls.model.has_more(items_model, limit) + has_more = has_more next_page_uri: Optional[str] = None if wants_more and has_more: diff --git a/agave/models/mongo/mongo_model.py b/agave/models/mongo/mongo_model.py index 8c2fdff1..4ce09744 100644 --- a/agave/models/mongo/mongo_model.py +++ b/agave/models/mongo/mongo_model.py @@ -1,4 +1,4 @@ -from typing import Optional +from typing import Optional, Tuple from cuenca_validations.typing import DictStrAny from mongoengine import Document, DoesNotExist, Q @@ -31,13 +31,10 @@ def count(cls, filters: Q) -> int: return count @classmethod - def all(cls, filters: Q, *, limit: int): + def all(cls, filters: Q, *, limit: int) -> Tuple[list, bool]: items = ( cls.objects.order_by("-created_at").filter(filters).limit(limit) ) - return items - - @classmethod - def has_more(cls, items: Q, limit: int): has_more = items.limit(limit + 1).count() > limit - return has_more + items = list(items) + return items, has_more diff --git a/agave/models/redis/redis_model.py b/agave/models/redis/redis_model.py index 4911cf8d..5fad85ec 100644 --- a/agave/models/redis/redis_model.py +++ b/agave/models/redis/redis_model.py @@ -1,4 +1,4 @@ -from typing import Dict, Optional +from typing import Dict, Optional, Tuple from cuenca_validations.typing import DictStrAny from cuenca_validations.validators import sanitize_item @@ -56,13 +56,10 @@ def count(cls, filters: Dict) -> int: return count @classmethod - def all(cls, filters: Dict, *, limit: int): + def all(cls, filters: Dict, *, limit: int) -> Tuple[list, bool]: items = ( cls.query.filter(**filters).order_by('-created_at').limit(0, limit) ) - return items - - @classmethod - def has_more(cls, items, limit: int): has_more = items.limit(0, limit + 1).count() > limit - return has_more + items = list(items) + return items, has_more diff --git a/tests/models/test_models.py b/tests/models/test_models.py index 99fd087c..65127cc3 100644 --- a/tests/models/test_models.py +++ b/tests/models/test_models.py @@ -58,7 +58,6 @@ def test_query_all_with_limit(db_model: Type[DbModel]): else: filters = generic_redis_query(query_params) items = db_model.all(filters, limit=limit) - items = list(items) assert len(items) == 2 @@ -71,8 +70,7 @@ def test_query_all_resource(db_model: Type[DbModel]): filters = generic_mongo_query(query_params) else: filters = generic_redis_query(query_params) - items = db_model.all(filters, limit=limit) - has_more = db_model.has_more(items, limit) + items, has_more = db_model.all(filters, limit=limit) assert has_more is True From 0d412e6034a29a9f72540e69153deb80e0b03a58 Mon Sep 17 00:00:00 2001 From: Glen Date: Tue, 22 Dec 2020 19:18:53 -0600 Subject: [PATCH 35/43] fix: constant-excluded --- agave/models/redis/redis_model.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/agave/models/redis/redis_model.py b/agave/models/redis/redis_model.py index 5fad85ec..d7e54cee 100644 --- a/agave/models/redis/redis_model.py +++ b/agave/models/redis/redis_model.py @@ -7,6 +7,8 @@ from agave.exc import DoesNotExist from agave.models.base import BaseModel +EXCLUDED = ['o_id'] + class String(Column): """ @@ -24,11 +26,10 @@ def _from_redis(self, value): def redis_to_dit(obj, exclude_fields: list = None) -> dict: - excluded = ['o_id'] response = { key: sanitize_item(value) for key, value in obj._data.items() - if key not in excluded + if key not in EXCLUDED } return response From 724abd4789ec64d5053f2fb8be9d61b064f1ee4c Mon Sep 17 00:00:00 2001 From: Glen Date: Tue, 22 Dec 2020 19:26:41 -0600 Subject: [PATCH 36/43] remove-typehint-Any --- agave/blueprints/rest_api.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index 679fac7c..2f126a14 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -1,4 +1,4 @@ -from typing import Any, Optional, Type +from typing import Optional, Type from urllib.parse import urlencode from chalice import Blueprint, NotFoundError, Response @@ -183,11 +183,11 @@ def query(): return _count(filters) return _all(query_params, filters) - def _count(filters: Any): + def _count(filters): count = cls.model.count(filters) return dict(count=count) - def _all(query: QueryParams, filters: Any): + def _all(query: QueryParams, filters): if query.limit: limit = min(query.limit, query.page_size) query.limit = max(0, query.limit - limit) # type: ignore From 37f7e6eb0c7a21e9980e36759fac0c24b032b097 Mon Sep 17 00:00:00 2001 From: Glen Date: Tue, 22 Dec 2020 19:38:34 -0600 Subject: [PATCH 37/43] fix: remove more than one line in a try block --- agave/blueprints/rest_api.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index 2f126a14..7888da7c 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -112,9 +112,10 @@ def update(id: str): params = self.current_request.json_body or dict() try: data = cls.update_validator(**params) - model = cls.model.retrieve(id=id) except ValidationError as e: return Response(e.json(), status_code=400) + try: + model = cls.model.retrieve(id=id) except DoesNotExist: raise NotFoundError('Not valid id') else: @@ -136,14 +137,14 @@ def retrieve(id: str): The most of times this implementation is enough and is not necessary define a custom "retrieve" method """ + user_id: Optional[bool] = None + if self.user_id_filter_required(): + user_id = self.current_user_id if hasattr(cls, 'retrieve'): # at the moment, there are no resources with a custom # retrieve method return cls.retrieve(id) # pragma: no cover try: - user_id: Optional[bool] = None - if self.user_id_filter_required(): - user_id = self.current_user_id data = cls.model.retrieve(id, user_id=user_id) except DoesNotExist: raise NotFoundError('Not valid id') From 51996aecbb20f4426a84657952c76768120c44b3 Mon Sep 17 00:00:00 2001 From: Glen Date: Wed, 23 Dec 2020 16:17:34 -0600 Subject: [PATCH 38/43] type-hint-list --- agave/models/redis/redis_model.py | 4 ++-- requirements.txt | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/agave/models/redis/redis_model.py b/agave/models/redis/redis_model.py index d7e54cee..75832a99 100644 --- a/agave/models/redis/redis_model.py +++ b/agave/models/redis/redis_model.py @@ -1,4 +1,4 @@ -from typing import Dict, Optional, Tuple +from typing import Any, Dict, List, Optional, Tuple from cuenca_validations.typing import DictStrAny from cuenca_validations.validators import sanitize_item @@ -25,7 +25,7 @@ def _from_redis(self, value): return value.decode('utf-8') -def redis_to_dit(obj, exclude_fields: list = None) -> dict: +def redis_to_dit(obj, exclude_fields: List[Dict[str, Any]]) -> dict: response = { key: sanitize_item(value) for key, value in obj._data.items() diff --git a/requirements.txt b/requirements.txt index e6d9ac9f..9f21026b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,4 +4,3 @@ mongoengine==0.22.1 cuenca-validations==0.6.8 dnspython==2.0.0 rom==1.0.0 - From 54383c980cd143caa472e1f38ef4596a96aa76c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20L=C3=B3pez?= Date: Wed, 23 Dec 2020 18:08:01 -0600 Subject: [PATCH 39/43] tests review --- agave/blueprints/rest_api.py | 10 +- agave/models/mongo/mongo_model.py | 35 ++++--- agave/models/redis/filters.py | 6 +- agave/models/redis/redis_model.py | 26 +++-- examples/chalicelib/models/mongo_models.py | 2 +- examples/chalicelib/models/redis_models.py | 2 +- tests/models/conftest.py | 10 -- tests/models/test_models.py | 109 ++++++++++++++------- 8 files changed, 121 insertions(+), 79 deletions(-) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index 7888da7c..1275888e 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -194,13 +194,13 @@ def _all(query: QueryParams, filters): query.limit = max(0, query.limit - limit) # type: ignore else: limit = query.page_size - items, has_more = cls.model.all(filters, limit=limit) - if wants_more := query.limit is None or query.limit > 0: - # only perform this query if it's necessary - has_more = has_more + wants_more = query.limit is None or query.limit > 0 + items, has_more = cls.model.all( + filters, limit=limit, wants_more=wants_more + ) - next_page_uri: Optional[str] = None + next_page_uri = None if wants_more and has_more: query.created_before = items[-1].created_at.isoformat() path = self.current_request.context['resourcePath'] diff --git a/agave/models/mongo/mongo_model.py b/agave/models/mongo/mongo_model.py index 4ce09744..7c5122c3 100644 --- a/agave/models/mongo/mongo_model.py +++ b/agave/models/mongo/mongo_model.py @@ -1,9 +1,10 @@ -from typing import Optional, Tuple +from typing import List, Optional, Tuple +import mongoengine as mongo from cuenca_validations.typing import DictStrAny -from mongoengine import Document, DoesNotExist, Q +from mongoengine import Document, Q -from agave.exc import DoesNotExist as AgaveDoesNotExist +from agave import exc from agave.lib.mongoengine.model_helpers import mongo_to_dict from agave.models.base import BaseModel @@ -15,26 +16,32 @@ def dict(self) -> DictStrAny: return self._dict(mongo_to_dict) @classmethod - def retrieve(cls, id: str, *, user_id: Optional[str] = None): + def retrieve( + cls, id: str, *, user_id: Optional[str] = None + ) -> 'MongoModel': query = Q(id=id) if user_id: query = query & Q(user_id=user_id) try: - id_obj = cls.objects.get(query) - except DoesNotExist: - raise AgaveDoesNotExist - return id_obj + obj = cls.objects.get(query) + except mongo.DoesNotExist: + raise exc.DoesNotExist + return obj @classmethod def count(cls, filters: Q) -> int: - count = cls.objects.filter(filters).count() - return count + return cls.objects.filter(filters).count() @classmethod - def all(cls, filters: Q, *, limit: int) -> Tuple[list, bool]: + def all( + cls, filters: Q, *, limit: int, wants_more: bool + ) -> Tuple[List['MongoModel'], bool]: items = ( cls.objects.order_by("-created_at").filter(filters).limit(limit) ) - has_more = items.limit(limit + 1).count() > limit - items = list(items) - return items, has_more + + has_more = False + if wants_more: + has_more = items.limit(limit + 1).count() > limit + + return list(items), has_more diff --git a/agave/models/redis/filters.py b/agave/models/redis/filters.py index 6ee96ac7..75dc4370 100644 --- a/agave/models/redis/filters.py +++ b/agave/models/redis/filters.py @@ -1,13 +1,13 @@ import datetime as dt -from typing import Any, Dict from cuenca_validations.types import QueryParams +from cuenca_validations.typing import DictStrAny from ...filters import exclude_fields -def generic_redis_query(query: QueryParams, **kwargs) -> Dict[str, Any]: - filters: Dict[str, Any] = dict() +def generic_redis_query(query: QueryParams, **kwargs) -> DictStrAny: + filters = dict() if query.created_before or query.created_after: # Restamos o sumamos un microsegundo porque la comparación # aquí es inclusiva diff --git a/agave/models/redis/redis_model.py b/agave/models/redis/redis_model.py index 75832a99..565c6ccb 100644 --- a/agave/models/redis/redis_model.py +++ b/agave/models/redis/redis_model.py @@ -1,4 +1,4 @@ -from typing import Any, Dict, List, Optional, Tuple +from typing import Dict, List, Optional, Tuple from cuenca_validations.typing import DictStrAny from cuenca_validations.validators import sanitize_item @@ -25,7 +25,7 @@ def _from_redis(self, value): return value.decode('utf-8') -def redis_to_dit(obj, exclude_fields: List[Dict[str, Any]]) -> dict: +def redis_to_dict(obj) -> DictStrAny: response = { key: sanitize_item(value) for key, value in obj._data.items() @@ -39,10 +39,12 @@ class RedisModel(BaseModel, Model): o_id = PrimaryKey() # Para que podamos usar `id` en los modelos def dict(self) -> DictStrAny: - return self._dict(redis_to_dit) + return self._dict(redis_to_dict) @classmethod - def retrieve(cls, id: str, *, user_id: Optional[str] = None): + def retrieve( + cls, id: str, *, user_id: Optional[str] = None + ) -> 'RedisModel': params = dict(id=id) if user_id: params['user_id'] = user_id @@ -53,14 +55,18 @@ def retrieve(cls, id: str, *, user_id: Optional[str] = None): @classmethod def count(cls, filters: Dict) -> int: - count = cls.query.filter(**filters).count() - return count + return cls.query.filter(**filters).count() @classmethod - def all(cls, filters: Dict, *, limit: int) -> Tuple[list, bool]: + def all( + cls, filters: Dict, *, limit: int, wants_more: bool + ) -> Tuple[List['RedisModel'], bool]: items = ( cls.query.filter(**filters).order_by('-created_at').limit(0, limit) ) - has_more = items.limit(0, limit + 1).count() > limit - items = list(items) - return items, has_more + + has_more = False + if wants_more: + has_more = items.limit(0, limit + 1).count() > limit + + return list(items), has_more diff --git a/examples/chalicelib/models/mongo_models.py b/examples/chalicelib/models/mongo_models.py index fb6e13a3..b726027a 100644 --- a/examples/chalicelib/models/mongo_models.py +++ b/examples/chalicelib/models/mongo_models.py @@ -11,7 +11,7 @@ class Account(MongoModel): created_at = DateTimeField() deactivated_at = DateTimeField() secret_field = StringField() - __test__ = False + _hidden = ['secret_field'] diff --git a/examples/chalicelib/models/redis_models.py b/examples/chalicelib/models/redis_models.py index 9aecbf9c..a8d15eb5 100644 --- a/examples/chalicelib/models/redis_models.py +++ b/examples/chalicelib/models/redis_models.py @@ -21,5 +21,5 @@ class Account(RedisModel): created_at = DateTime(default=dt.datetime.utcnow, index=True) deactivated_at = DateTime(default=DEFAULT_MISSING_DATE, index=True) secret_field = String(index=True, keygen=util.IDENTITY) - __test__ = False + _hidden = ['secret_field'] diff --git a/tests/models/conftest.py b/tests/models/conftest.py index 37e7ec79..ad225024 100644 --- a/tests/models/conftest.py +++ b/tests/models/conftest.py @@ -4,20 +4,10 @@ import pytest from agave.models import mongo, redis -from examples.chalicelib.models import mongo_models, redis_models DbModel = Union[mongo.MongoModel, redis.RedisModel] -def pytest_generate_tests(metafunc): - if "db_model" in metafunc.fixturenames: - metafunc.parametrize( - 'db_model', - [mongo_models.Account, redis_models.Account], - indirect=['db_model'], - ) - - @pytest.fixture def db_model(request) -> Type[DbModel]: return request.param diff --git a/tests/models/test_models.py b/tests/models/test_models.py index 65127cc3..9bfb4c27 100644 --- a/tests/models/test_models.py +++ b/tests/models/test_models.py @@ -1,26 +1,33 @@ -from typing import Type, Union +from typing import Callable, List, Type, Union import pytest from agave.exc import DoesNotExist from agave.models import mongo, redis -from agave.models.mongo.filters import generic_mongo_query -from agave.models.redis.filters import generic_redis_query +from examples.chalicelib.models import mongo_models, redis_models from examples.chalicelib.validators import AccountQuery DbModel = Union[mongo.MongoModel, redis.RedisModel] +models = [mongo_models.Account, redis_models.Account] +generic_query_funcs = [ + mongo.filters.generic_mongo_query, + redis.filters.generic_redis_query, +] +@pytest.mark.parametrize('db_model', models, indirect=['db_model']) def test_retrieve(db_model: Type[DbModel], account: DbModel) -> None: obj = db_model.retrieve(account.id) assert obj.id == account.id +@pytest.mark.parametrize('db_model', models, indirect=['db_model']) def test_retrieve_not_found(db_model: Type[DbModel]) -> None: with pytest.raises(DoesNotExist): db_model.retrieve('unknown-id') +@pytest.mark.parametrize('db_model', models, indirect=['db_model']) def test_retrieve_with_user_id_filter( db_model: Type[DbModel], account: DbModel, user_id: str ) -> None: @@ -29,52 +36,84 @@ def test_retrieve_with_user_id_filter( assert obj.user_id == user_id +@pytest.mark.parametrize('db_model', models, indirect=['db_model']) def test_retrieve_not_found_with_user_id_filter( - db_model: Type[DbModel], + db_model: Type[DbModel], account: DbModel, another_user_id ) -> None: with pytest.raises(DoesNotExist): - db_model.retrieve('unknown-id', user_id='unknown-user-id') + db_model.retrieve(account.id, user_id=another_user_id) -@pytest.mark.usefixtures('accounts') -def test_query_count(db_model: Type[DbModel]): - params = dict(count=1, name='Frida Kahlo') - query_params = AccountQuery(**params) - if issubclass(db_model, mongo.MongoModel): - filters = generic_mongo_query(query_params) - else: - filters = generic_redis_query(query_params) - count = db_model.count(filters) - assert count == 1 +@pytest.mark.parametrize( + 'db_model,generic_query_func', + zip(models, generic_query_funcs), + indirect=['db_model'], +) +def test_query_count( + db_model: Type[DbModel], + generic_query_func: Callable, + accounts: List[DbModel], + user_id: str, +) -> None: + query_params = AccountQuery(count=1, name='Frida Kahlo') + assert db_model.count(generic_query_func(query_params)) == 1 + query_params = AccountQuery(count=1) + assert db_model.count(generic_query_func(query_params)) == len(accounts) + query_params = AccountQuery(count=1, user_id=user_id) + assert db_model.count(generic_query_func(query_params)) == len( + [acc for acc in accounts if acc.user_id == user_id] + ) + + +@pytest.mark.parametrize( + 'db_model,generic_query_func', + zip(models, generic_query_funcs), + indirect=['db_model'], +) @pytest.mark.usefixtures('accounts') -def test_query_all_with_limit(db_model: Type[DbModel]): +def test_query_all_with_limit( + db_model: Type[DbModel], generic_query_func: Callable +) -> None: limit = 2 - params = dict(limit=limit) - query_params = AccountQuery(**params) - if issubclass(db_model, mongo.MongoModel): - filters = generic_mongo_query(query_params) - else: - filters = generic_redis_query(query_params) - items = db_model.all(filters, limit=limit) - assert len(items) == 2 + query_params = AccountQuery(limit=limit) + items, has_more = db_model.all( + generic_query_func(query_params), limit=limit, wants_more=False + ) + assert not has_more + assert len(items) == limit +@pytest.mark.parametrize( + 'db_model,generic_query_func', + zip(models, generic_query_funcs), + indirect=['db_model'], +) @pytest.mark.usefixtures('accounts') -def test_query_all_resource(db_model: Type[DbModel]): - params = dict(page_size=2) - limit = 2 - query_params = AccountQuery(**params) - if issubclass(db_model, mongo.MongoModel): - filters = generic_mongo_query(query_params) - else: - filters = generic_redis_query(query_params) - items, has_more = db_model.all(filters, limit=limit) - assert has_more is True +def test_query_all_resource( + db_model: Type[DbModel], generic_query_func: Callable +) -> None: + limit = 3 + query_params = AccountQuery(page_size=limit) + items, has_more = db_model.all( + generic_query_func(query_params), limit=limit, wants_more=True + ) + assert has_more + assert len(items) == limit + + query_params = AccountQuery( + page_size=limit, created_before=items[-1].created_at + ) + items, has_more = db_model.all( + generic_query_func(query_params), limit=limit, wants_more=True + ) + assert not has_more + assert len(items) == 1 -def test_hide_field_redis(db_model: Type[DbModel]): +@pytest.mark.parametrize('db_model', models, indirect=['db_model']) +def test_hide_field_redis(db_model: Type[DbModel]) -> None: model = db_model( id='12345', secret_field='secret', name='frida', user_id='w72638' ) From ed5b88e75b67cf0910fb0bbbc15b1d20c8c2fa92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20L=C3=B3pez?= Date: Wed, 23 Dec 2020 19:55:50 -0600 Subject: [PATCH 40/43] tests --- agave/models/redis/redis_model.py | 5 +++-- examples/chalicelib/models/redis_models.py | 1 + tests/models/test_models.py | 20 ++++++++++++++++---- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/agave/models/redis/redis_model.py b/agave/models/redis/redis_model.py index 565c6ccb..6cfd0f7c 100644 --- a/agave/models/redis/redis_model.py +++ b/agave/models/redis/redis_model.py @@ -25,11 +25,12 @@ def _from_redis(self, value): return value.decode('utf-8') -def redis_to_dict(obj) -> DictStrAny: +def redis_to_dict(obj, exclude_fields: List[str]) -> DictStrAny: + excluded = EXCLUDED + exclude_fields response = { key: sanitize_item(value) for key, value in obj._data.items() - if key not in EXCLUDED + if key not in excluded } return response diff --git a/examples/chalicelib/models/redis_models.py b/examples/chalicelib/models/redis_models.py index a8d15eb5..7019d03b 100644 --- a/examples/chalicelib/models/redis_models.py +++ b/examples/chalicelib/models/redis_models.py @@ -23,3 +23,4 @@ class Account(RedisModel): secret_field = String(index=True, keygen=util.IDENTITY) _hidden = ['secret_field'] + _excluded = ['deactivated_at'] diff --git a/tests/models/test_models.py b/tests/models/test_models.py index 9bfb4c27..b20dd643 100644 --- a/tests/models/test_models.py +++ b/tests/models/test_models.py @@ -1,3 +1,4 @@ +import datetime as dt from typing import Callable, List, Type, Union import pytest @@ -113,10 +114,21 @@ def test_query_all_resource( @pytest.mark.parametrize('db_model', models, indirect=['db_model']) -def test_hide_field_redis(db_model: Type[DbModel]) -> None: +def test_to_dict(db_model: Type[DbModel]) -> None: + now = dt.datetime.utcnow() + expected = dict( + id='12345', + name='frida', + user_id='w72638', + secret_field='********', + ) model = db_model( - id='12345', secret_field='secret', name='frida', user_id='w72638' + id='12345', + name='frida', + user_id='w72638', + secret_field='secret', + created_at=now, ) + model.save() model_dict = model.dict() - assert model_dict['secret_field'] == '********' - assert model_dict['id'] == '12345' + assert all(model_dict[key] == val for key, val in expected.items()) From 48a5c6ff19d5b9eb987054f02051ab7b1cac2390 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20L=C3=B3pez?= Date: Wed, 23 Dec 2020 20:07:01 -0600 Subject: [PATCH 41/43] minor fixes --- agave/blueprints/rest_api.py | 9 +++++---- agave/models/base.py | 6 +++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/agave/blueprints/rest_api.py b/agave/blueprints/rest_api.py index 1275888e..0c1838ed 100644 --- a/agave/blueprints/rest_api.py +++ b/agave/blueprints/rest_api.py @@ -1,4 +1,4 @@ -from typing import Optional, Type +from typing import Type from urllib.parse import urlencode from chalice import Blueprint, NotFoundError, Response @@ -137,13 +137,14 @@ def retrieve(id: str): The most of times this implementation is enough and is not necessary define a custom "retrieve" method """ - user_id: Optional[bool] = None - if self.user_id_filter_required(): - user_id = self.current_user_id if hasattr(cls, 'retrieve'): # at the moment, there are no resources with a custom # retrieve method return cls.retrieve(id) # pragma: no cover + + user_id = None + if self.user_id_filter_required(): + user_id = self.current_user_id try: data = cls.model.retrieve(id, user_id=user_id) except DoesNotExist: diff --git a/agave/models/base.py b/agave/models/base.py index 1a1f645d..98d58801 100644 --- a/agave/models/base.py +++ b/agave/models/base.py @@ -13,11 +13,11 @@ def __init__(self, *args, **values): def _dict(self, dict_func: Callable) -> DictStrAny: private_fields = [f for f in dir(self) if f.startswith('_')] excluded = self._excluded + private_fields - response = dict_func(self, excluded) + model_dict = dict_func(self, excluded) for field in self._hidden: - response[field] = '********' - return response + model_dict[field] = '********' + return model_dict def __repr__(self) -> str: return str(self.dict()) # type: ignore # pragma: no cover From 5d00beae2f4aa82a2a67a02f3c4107b47db7121a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20L=C3=B3pez?= Date: Wed, 23 Dec 2020 20:11:51 -0600 Subject: [PATCH 42/43] extras_requires --- agave/version.py | 2 +- setup.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/agave/version.py b/agave/version.py index eead3198..2458914b 100644 --- a/agave/version.py +++ b/agave/version.py @@ -1 +1 @@ -__version__ = '0.0.5' +__version__ = '0.0.6.dev0' diff --git a/setup.py b/setup.py index 9fb90bf2..23efbd0f 100644 --- a/setup.py +++ b/setup.py @@ -26,10 +26,13 @@ 'chalice>=1.16.0,<1.21.7', 'cuenca-validations>=0.4,<0.7', 'blinker>=1.4,<1.5', - 'mongoengine>=0.20.0,<0.23.0', 'dnspython>=2.0.0,<2.1.0', 'dataclasses>=0.6;python_version<"3.7"', ], + extras_require=[ + 'mongoengine>=0.20.0,<0.23.0', + 'rom<=1.0.0' + ], classifiers=[ 'Programming Language :: Python :: 3.8', 'License :: OSI Approved :: MIT License', From 03f0eed9352065cea158e98fab1acaf5b5afefb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20L=C3=B3pez?= Date: Wed, 23 Dec 2020 20:22:52 -0600 Subject: [PATCH 43/43] fix extras_requires --- setup.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/setup.py b/setup.py index 23efbd0f..df933924 100644 --- a/setup.py +++ b/setup.py @@ -29,10 +29,10 @@ 'dnspython>=2.0.0,<2.1.0', 'dataclasses>=0.6;python_version<"3.7"', ], - extras_require=[ - 'mongoengine>=0.20.0,<0.23.0', - 'rom<=1.0.0' - ], + extras_require={ + 'mongoengine': 'mongoengine>=0.20.0,<0.23.0', + 'rom': 'rom>=1.0.0,<1.1.0', + }, classifiers=[ 'Programming Language :: Python :: 3.8', 'License :: OSI Approved :: MIT License',