From 4ad843a6a5258fca5c22054557f922305612bd92 Mon Sep 17 00:00:00 2001 From: collerek Date: Tue, 23 Mar 2021 17:36:20 +0100 Subject: [PATCH] fix quoting in order_by, add get_or_none --- README.md | 1 + docs/index.md | 1 + docs/queries/filter-and-sort.md | 12 +++ docs/queries/index.md | 4 + docs/queries/read.md | 15 ++++ docs/releases.md | 11 +++ ormar/__init__.py | 2 +- ormar/queryset/actions/order_action.py | 8 +- ormar/queryset/queryset.py | 20 +++++ ormar/relations/querysetproxy.py | 24 +++++ tests/test_encrypted_columns.py | 2 + tests/test_many_to_many.py | 15 ++++ tests/test_models.py | 2 + tests/test_reserved_sql_keywords_escaped.py | 99 +++++++++++++++++++++ 14 files changed, 214 insertions(+), 2 deletions(-) create mode 100644 tests/test_reserved_sql_keywords_escaped.py diff --git a/README.md b/README.md index 22db942..4d2e6ea 100644 --- a/README.md +++ b/README.md @@ -429,6 +429,7 @@ metadata.drop_all(engine) * `create(**kwargs): -> Model` * `get(**kwargs): -> Model` +* `get_or_none(**kwargs): -> Optional[Model]` * `get_or_create(**kwargs) -> Model` * `first(): -> Model` * `update(each: bool = False, **kwargs) -> int` diff --git a/docs/index.md b/docs/index.md index 22db942..4d2e6ea 100644 --- a/docs/index.md +++ b/docs/index.md @@ -429,6 +429,7 @@ metadata.drop_all(engine) * `create(**kwargs): -> Model` * `get(**kwargs): -> Model` +* `get_or_none(**kwargs): -> Optional[Model]` * `get_or_create(**kwargs) -> Model` * `first(): -> Model` * `update(each: bool = False, **kwargs) -> int` diff --git a/docs/queries/filter-and-sort.md b/docs/queries/filter-and-sort.md index 227cf00..ab2625b 100644 --- a/docs/queries/filter-and-sort.md +++ b/docs/queries/filter-and-sort.md @@ -5,6 +5,7 @@ You can use following methods to filter the data (sql where clause). * `filter(**kwargs) -> QuerySet` * `exclude(**kwargs) -> QuerySet` * `get(**kwargs) -> Model` +* `get_or_none(**kwargs) -> Optional[Model]` * `get_or_create(**kwargs) -> Model` * `all(**kwargs) -> List[Optional[Model]]` @@ -13,6 +14,7 @@ You can use following methods to filter the data (sql where clause). * `QuerysetProxy.filter(**kwargs)` method * `QuerysetProxy.exclude(**kwargs)` method * `QuerysetProxy.get(**kwargs)` method + * `QuerysetProxy.get_or_none(**kwargs)` method * `QuerysetProxy.get_or_create(**kwargs)` method * `QuerysetProxy.all(**kwargs)` method @@ -397,6 +399,11 @@ When any kwargs are passed it's a shortcut equivalent to calling `filter(**kwarg To read more about `get` go to [read/get](../read/#get) +## get_or_none + +Exact equivalent of get described above but instead of raising the exception returns `None` if no db record matching the criteria is found. + + ## get_or_create `get_or_create(**kwargs) -> Model` @@ -461,6 +468,11 @@ objects from other side of the relation. !!!tip To read more about `QuerysetProxy` visit [querysetproxy][querysetproxy] section +#### get_or_none + +Exact equivalent of get described above but instead of raising the exception returns `None` if no db record matching the criteria is found. + + #### get_or_create Works exactly the same as [get_or_create](./#get_or_create) function above but allows diff --git a/docs/queries/index.md b/docs/queries/index.md index cd74990..3fb94af 100644 --- a/docs/queries/index.md +++ b/docs/queries/index.md @@ -46,6 +46,7 @@ To read more about any specific section or function please refer to the details ### [Read data from database](./read.md) * `get(**kwargs) -> Model` +* `get_or_none(**kwargs) -> Optional[Model]` * `get_or_create(**kwargs) -> Model` * `first() -> Model` * `all(**kwargs) -> List[Optional[Model]]` @@ -57,6 +58,7 @@ To read more about any specific section or function please refer to the details * `QuerysetProxy` * `QuerysetProxy.get(**kwargs)` method + * `QuerysetProxy.get_or_none(**kwargs)` method * `QuerysetProxy.get_or_create(**kwargs)` method * `QuerysetProxy.first()` method * `QuerysetProxy.all(**kwargs)` method @@ -122,6 +124,7 @@ To read more about any specific section or function please refer to the details * `exclude(**kwargs) -> QuerySet` * `order_by(columns:Union[List, str]) -> QuerySet` * `get(**kwargs) -> Model` +* `get_or_none(**kwargs) -> Optional[Model]` * `get_or_create(**kwargs) -> Model` * `all(**kwargs) -> List[Optional[Model]]` @@ -131,6 +134,7 @@ To read more about any specific section or function please refer to the details * `QuerysetProxy.exclude(**kwargs)` method * `QuerysetProxy.order_by(columns:Union[List, str])` method * `QuerysetProxy.get(**kwargs)` method + * `QuerysetProxy.get_or_none(**kwargs)` method * `QuerysetProxy.get_or_create(**kwargs)` method * `QuerysetProxy.all(**kwargs)` method diff --git a/docs/queries/read.md b/docs/queries/read.md index f973507..40b84bb 100644 --- a/docs/queries/read.md +++ b/docs/queries/read.md @@ -55,6 +55,13 @@ track == track2 If there are multiple rows meeting the criteria the `MultipleMatches` exception is raised. +## get_or_none + +`get_or_none(**kwargs) -> Model` + +Exact equivalent of get described above but instead of raising the exception returns `None` if no db record matching the criteria is found. + + ## get_or_create `get_or_create(**kwargs) -> Model` @@ -190,6 +197,14 @@ objects from other side of the relation. !!!tip To read more about `QuerysetProxy` visit [querysetproxy][querysetproxy] section +### get_or_none + +Exact equivalent of get described above but instead of raising the exception returns `None` if no db record matching the criteria is found. + +!!!tip + To read more about `QuerysetProxy` visit [querysetproxy][querysetproxy] section + + ### get_or_create Works exactly the same as [get_or_create](./#get_or_create) function above but allows diff --git a/docs/releases.md b/docs/releases.md index 9eaf097..7f2b475 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -1,3 +1,14 @@ +# 0.10.1 + +## Features + +* add `get_or_none(**kwargs)` method to `QuerySet` and `QuerysetProxy`. It is exact equivalent of `get(**kwargs)` but instead of raising `ormar.NoMatch` exception if there is no db record matching the criteria, `get_or_none` simply returns `None`. + +## Fixes + +* Fix dialect dependent quoting of column and table names in order_by clauses not working + properly in postgres. + # 0.10.0 ## Breaking diff --git a/ormar/__init__.py b/ormar/__init__.py index 67962ac..2e8ae3f 100644 --- a/ormar/__init__.py +++ b/ormar/__init__.py @@ -75,7 +75,7 @@ class UndefinedType: # pragma no cover Undefined = UndefinedType() -__version__ = "0.10.0" +__version__ = "0.10.1" __all__ = [ "Integer", "BigInteger", diff --git a/ormar/queryset/actions/order_action.py b/ormar/queryset/actions/order_action.py index 9d5f248..0927df5 100644 --- a/ormar/queryset/actions/order_action.py +++ b/ormar/queryset/actions/order_action.py @@ -69,7 +69,13 @@ class OrderAction(QueryAction): :rtype: sqlalchemy.sql.elements.TextClause """ prefix = f"{self.table_prefix}_" if self.table_prefix else "" - return text(f"{prefix}{self.table}" f".{self.field_alias} {self.direction}") + table_name = self.table.name + field_name = self.field_alias + if not prefix: + dialect = self.target_model.Meta.database._backend._dialect + table_name = dialect.identifier_preparer.quote(table_name) + field_name = dialect.identifier_preparer.quote(field_name) + return text(f"{prefix}{table_name}" f".{field_name} {self.direction}") def _split_value_into_parts(self, order_str: str) -> None: if order_str.startswith("-"): diff --git a/ormar/queryset/queryset.py b/ormar/queryset/queryset.py index 420a880..e05b108 100644 --- a/ormar/queryset/queryset.py +++ b/ormar/queryset/queryset.py @@ -778,6 +778,26 @@ class QuerySet(Generic[T]): self.check_single_result_rows_count(processed_rows) return processed_rows[0] # type: ignore + async def get_or_none(self, **kwargs: Any) -> Optional["T"]: + """ + Get's the first row from the db meeting the criteria set by kwargs. + + If no criteria set it will return the last row in db sorted by pk. + + Passing a criteria is actually calling filter(**kwargs) method described below. + + If not match is found None will be returned. + + :param kwargs: fields names and proper value types + :type kwargs: Any + :return: returned model + :rtype: Model + """ + try: + return await self.get(**kwargs) + except ormar.NoMatch: + return None + async def get(self, **kwargs: Any) -> "T": """ Get's the first row from the db meeting the criteria set by kwargs. diff --git a/ormar/relations/querysetproxy.py b/ormar/relations/querysetproxy.py index ba39415..7874b06 100644 --- a/ormar/relations/querysetproxy.py +++ b/ormar/relations/querysetproxy.py @@ -279,6 +279,30 @@ class QuerysetProxy(Generic[T]): self._register_related(first) return first + async def get_or_none(self, **kwargs: Any) -> Optional["T"]: + """ + Get's the first row from the db meeting the criteria set by kwargs. + + If no criteria set it will return the last row in db sorted by pk. + + Passing a criteria is actually calling filter(**kwargs) method described below. + + If not match is found None will be returned. + + :param kwargs: fields names and proper value types + :type kwargs: Any + :return: returned model + :rtype: Model + """ + try: + get = await self.queryset.get(**kwargs) + except ormar.NoMatch: + return None + + self._clean_items_on_load() + self._register_related(get) + return get + async def get(self, **kwargs: Any) -> "T": """ Get's the first row from the db meeting the criteria set by kwargs. diff --git a/tests/test_encrypted_columns.py b/tests/test_encrypted_columns.py index 1f0d7d7..2cfe0e8 100644 --- a/tests/test_encrypted_columns.py +++ b/tests/test_encrypted_columns.py @@ -230,6 +230,8 @@ async def test_fernet_filters_nomatch(): with pytest.raises(NoMatch): await Filter.objects.get(name="test1") + assert await Filter.objects.get_or_none(name="test1") is None + @pytest.mark.asyncio async def test_hash_filters_works(): diff --git a/tests/test_many_to_many.py b/tests/test_many_to_many.py index 3989df8..21a00ef 100644 --- a/tests/test_many_to_many.py +++ b/tests/test_many_to_many.py @@ -83,6 +83,18 @@ async def test_not_saved_raises_error(cleanup): await post.categories.add(news) +@pytest.mark.asyncio +async def test_not_existing_raises_error(cleanup): + async with database: + guido = await Author(first_name="Guido", last_name="Van Rossum").save() + post = await Post.objects.create(title="Hello, M2M", author=guido) + + with pytest.raises(NoMatch): + await post.categories.get() + + assert await post.categories.get_or_none() is None + + @pytest.mark.asyncio async def test_assigning_related_objects(cleanup): async with database: @@ -95,6 +107,9 @@ async def test_assigning_related_objects(cleanup): # or from the other end: await news.posts.add(post) + assert await post.categories.get_or_none(name="no exist") is None + assert await post.categories.get_or_none(name="News") == news + # Creating columns object from instance: await post.categories.create(name="Tips") assert len(post.categories) == 2 diff --git a/tests/test_models.py b/tests/test_models.py index b15ec84..3459768 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -220,6 +220,8 @@ async def test_model_get(): with pytest.raises(ormar.NoMatch): await User.objects.get() + assert await User.objects.get_or_none() is None + user = await User.objects.create(name="Tom") lookup = await User.objects.get() assert lookup == user diff --git a/tests/test_reserved_sql_keywords_escaped.py b/tests/test_reserved_sql_keywords_escaped.py new file mode 100644 index 0000000..b881b3b --- /dev/null +++ b/tests/test_reserved_sql_keywords_escaped.py @@ -0,0 +1,99 @@ +import databases +import pytest +import sqlalchemy + +import ormar + +from tests.settings import DATABASE_URL + +database = databases.Database(DATABASE_URL, force_rollback=True) +metadata = sqlalchemy.MetaData() + + +class BaseMeta(ormar.ModelMeta): + metadata = metadata + database = database + + +class User(ormar.Model): + class Meta(BaseMeta): + tablename = "user" + + id: int = ormar.Integer(primary_key=True, autoincrement=True, nullable=False) + user: str = ormar.String( + unique=True, index=True, nullable=False, max_length=255 + ) # ID of the user on auth0 + first: str = ormar.String(nullable=False, max_length=255) + last: str = ormar.String(nullable=False, max_length=255) + email: str = ormar.String(unique=True, index=True, nullable=False, max_length=255) + display_name: str = ormar.String( + unique=True, index=True, nullable=False, max_length=255 + ) + pic_url: str = ormar.Text(nullable=True) + + +class Task(ormar.Model): + class Meta(BaseMeta): + tablename = "task" + + id: int = ormar.Integer(primary_key=True, autoincrement=True, nullable=False) + from_: str = ormar.String(name="from", nullable=True, max_length=200) + user = ormar.ForeignKey(User) + + +@pytest.fixture(autouse=True, scope="module") +def create_test_database(): + engine = sqlalchemy.create_engine(DATABASE_URL) + metadata.drop_all(engine) + metadata.create_all(engine) + yield + metadata.drop_all(engine) + + +@pytest.mark.asyncio +async def test_single_model_quotes(): + async with database: + await User.objects.create( + user="test", + first="first", + last="last", + email="email@com.com", + display_name="first last", + ) + + user = await User.objects.order_by("user").get(first="first") + assert user.last == "last" + assert user.email == "email@com.com" + + +@pytest.mark.asyncio +async def test_two_model_quotes(): + async with database: + user = await User.objects.create( + user="test", + first="first", + last="last", + email="email@com.com", + display_name="first last", + ) + + await Task(user=user, from_="aa").save() + await Task(user=user, from_="bb").save() + + task = ( + await Task.objects.select_related("user") + .order_by("user__user") + .get(from_="aa") + ) + assert task.user.last == "last" + assert task.user.email == "email@com.com" + + tasks = await Task.objects.select_related("user").order_by("-from").all() + assert len(tasks) == 2 + assert tasks[0].user.last == "last" + assert tasks[0].user.email == "email@com.com" + assert tasks[0].from_ == "bb" + + assert tasks[1].user.last == "last" + assert tasks[1].user.email == "email@com.com" + assert tasks[1].from_ == "aa"