diff --git a/docs/releases.md b/docs/releases.md index d874436..6db4463 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -9,15 +9,21 @@ * `exclude: Union[Set, Dict, None]` -> set/dict of relations to exclude from save, those relation won't be saved even with `follow=True` and `save_all=True`. To exclude nested relations pass a nested dictionary like: `exclude={"child":{"sub_child": {"exclude_sub_child_realtion"}}}`. The allowed values follow the `fields/exclude_fields` (from `QuerySet`) methods schema so when in doubt you can refer to docs in queries -> selecting subset of fields -> fields. -* `dict()` method previously included only directly related models or nested models if they were not nullable and not virtual, +* `Model.dict()` method previously included only directly related models or nested models if they were not nullable and not virtual, now all related models not previously visited without loops are included in `dict()`. This should be not breaking as just more data will be dumped to dict, but it should not be missing. - +* `QuerySet.delete(each=False, **kwargs)` previously required that you either pass a `filter` (by `**kwargs` or as a separate `filter()` call) or set `each=True` now also accepts + `exclude()` calls that generates NOT filter. So either `each=True` needs to be set to delete whole table or at least one of `filter/exclude` clauses. +* Same thing applies to `QuerySet.update(each=False, **kwargs)` which also previously required that you either pass a `filter` (by `**kwargs` or as a separate `filter()` call) or set `each=True` now also accepts + `exclude()` calls that generates NOT filter. So either `each=True` needs to be set to update whole table or at least one of `filter/exclude` clauses. +* Same thing applies to `QuerysetProxy.update(each=False, **kwargs)` which also previously required that you either pass a `filter` (by `**kwargs` or as a separate `filter()` call) or set `each=True` now also accepts + `exclude()` calls that generates NOT filter. So either `each=True` needs to be set to update whole table or at least one of `filter/exclude` clauses. ## Fixes * Fix improper relation field resolution in `QuerysetProxy` if fk column has different database alias. * Fix hitting recursion error with very complicated models structure with loops when calling `dict()`. +* Fix bug when two non-relation fields were merged (appended) in query result when they were not relation fields (i.e. JSON) ## Other diff --git a/ormar/models/mixins/merge_mixin.py b/ormar/models/mixins/merge_mixin.py index 32d7288..e7234e4 100644 --- a/ormar/models/mixins/merge_mixin.py +++ b/ormar/models/mixins/merge_mixin.py @@ -60,20 +60,23 @@ class MergeModelMixin: :return: current Model instance with data merged from previous one. :rtype: Model """ - for field in one.Meta.model_fields.keys(): - current_field = getattr(one, field) - if isinstance(current_field, list) and not isinstance( - current_field, ormar.Model - ): - setattr(other, field, current_field + getattr(other, field)) - elif ( - isinstance(current_field, ormar.Model) - and current_field.pk == getattr(other, field).pk - ): - setattr( - other, - field, - cls.merge_two_instances(current_field, getattr(other, field)), - ) + for field_name, field in one.Meta.model_fields.items(): + current_field = getattr(one, field_name) + if field.is_relation: + if isinstance(current_field, list): + setattr( + other, field_name, current_field + getattr(other, field_name) + ) + elif ( + isinstance(current_field, ormar.Model) + and current_field.pk == getattr(other, field_name).pk + ): + setattr( + other, + field_name, + cls.merge_two_instances( + current_field, getattr(other, field_name) + ), + ) other.set_save_status(True) return other diff --git a/ormar/models/model.py b/ormar/models/model.py index 4eb0c29..4a2e81f 100644 --- a/ormar/models/model.py +++ b/ormar/models/model.py @@ -150,15 +150,16 @@ class Model(ModelRow): for related in self.extract_related_names(): if relation_map and related in relation_map: value = getattr(self, related) - update_count = await self._update_and_follow( - value=value, - follow=follow, - save_all=save_all, - relation_map=self._skip_ellipsis( # type: ignore - relation_map, related, default_return={} - ), - update_count=update_count, - ) + if value: + update_count = await self._update_and_follow( + value=value, + follow=follow, + save_all=save_all, + relation_map=self._skip_ellipsis( # type: ignore + relation_map, related, default_return={} + ), + update_count=update_count, + ) return update_count @staticmethod diff --git a/ormar/queryset/queryset.py b/ormar/queryset/queryset.py index e05b108..c325a20 100644 --- a/ormar/queryset/queryset.py +++ b/ormar/queryset/queryset.py @@ -650,7 +650,7 @@ class QuerySet(Generic[T]): :return: number of updated rows :rtype: int """ - if not each and not self.filter_clauses: + if not each and not (self.filter_clauses or self.exclude_clauses): raise QueryDefinitionError( "You cannot update without filtering the queryset first. " "If you want to update all rows use update(each=True, **kwargs)" @@ -666,6 +666,9 @@ class QuerySet(Generic[T]): expr = FilterQuery(filter_clauses=self.filter_clauses).apply( self.table.update().values(**updates) ) + expr = FilterQuery(filter_clauses=self.exclude_clauses, exclude=True).apply( + expr + ) return await self.database.execute(expr) async def delete(self, each: bool = False, **kwargs: Any) -> int: @@ -684,7 +687,7 @@ class QuerySet(Generic[T]): """ if kwargs: return await self.filter(**kwargs).delete() - if not each and not self.filter_clauses: + if not each and not (self.filter_clauses or self.exclude_clauses): raise QueryDefinitionError( "You cannot delete without filtering the queryset first. " "If you want to delete all rows use delete(each=True)" @@ -692,6 +695,9 @@ class QuerySet(Generic[T]): expr = FilterQuery(filter_clauses=self.filter_clauses).apply( self.table.delete() ) + expr = FilterQuery(filter_clauses=self.exclude_clauses, exclude=True).apply( + expr + ) return await self.database.execute(expr) def paginate(self, page: int, page_size: int = 20) -> "QuerySet[T]": diff --git a/ormar/relations/querysetproxy.py b/ormar/relations/querysetproxy.py index 7874b06..953b43d 100644 --- a/ormar/relations/querysetproxy.py +++ b/ormar/relations/querysetproxy.py @@ -389,7 +389,11 @@ class QuerysetProxy(Generic[T]): :rtype: int """ # queryset proxy always have one filter for pk of parent model - if not each and len(self.queryset.filter_clauses) == 1: + if ( + not each + and (len(self.queryset.filter_clauses) + len(self.queryset.exclude_clauses)) + == 1 + ): raise QueryDefinitionError( "You cannot update without filtering the queryset first. " "If you want to update all rows use update(each=True, **kwargs)" diff --git a/tests/test_queries/test_non_relation_fields_not_merged.py b/tests/test_queries/test_non_relation_fields_not_merged.py new file mode 100644 index 0000000..38cda87 --- /dev/null +++ b/tests/test_queries/test_non_relation_fields_not_merged.py @@ -0,0 +1,53 @@ +from typing import Dict, List, Optional + +import databases +import pytest +import sqlalchemy + +import ormar +from tests.settings import DATABASE_URL + +database = databases.Database(DATABASE_URL) +metadata = sqlalchemy.MetaData() + + +class BaseMeta(ormar.ModelMeta): + metadata = metadata + database = database + + +class Chart(ormar.Model): + class Meta(BaseMeta): + tablename = "authors" + + id: int = ormar.Integer(primary_key=True) + datasets = ormar.JSON() + + +class Config(ormar.Model): + class Meta(BaseMeta): + tablename = "books" + + id: int = ormar.Integer(primary_key=True) + chart: Optional[Chart] = ormar.ForeignKey(Chart) + + +@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_list_field_that_is_not_relation_is_not_merged(): + async with database: + chart = await Chart.objects.create(datasets=[{"test": "ok"}]) + await Config.objects.create(chart=chart) + await Config.objects.create(chart=chart) + + chart2 = await Chart.objects.select_related("configs").get() + assert len(chart2.datasets) == 1 + assert chart2.datasets == [{"test": "ok"}]