fix merging non relation list fields

This commit is contained in:
collerek
2021-03-31 17:07:42 +02:00
parent 4dc11b0de4
commit 749975d665
6 changed files with 102 additions and 29 deletions

View File

@ -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`. * `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 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. 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 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. 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 ## Fixes
* Fix improper relation field resolution in `QuerysetProxy` if fk column has different database alias. * 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 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 ## Other

View File

@ -60,20 +60,23 @@ class MergeModelMixin:
:return: current Model instance with data merged from previous one. :return: current Model instance with data merged from previous one.
:rtype: Model :rtype: Model
""" """
for field in one.Meta.model_fields.keys(): for field_name, field in one.Meta.model_fields.items():
current_field = getattr(one, field) current_field = getattr(one, field_name)
if isinstance(current_field, list) and not isinstance( if field.is_relation:
current_field, ormar.Model if isinstance(current_field, list):
): setattr(
setattr(other, field, current_field + getattr(other, field)) other, field_name, current_field + getattr(other, field_name)
)
elif ( elif (
isinstance(current_field, ormar.Model) isinstance(current_field, ormar.Model)
and current_field.pk == getattr(other, field).pk and current_field.pk == getattr(other, field_name).pk
): ):
setattr( setattr(
other, other,
field, field_name,
cls.merge_two_instances(current_field, getattr(other, field)), cls.merge_two_instances(
current_field, getattr(other, field_name)
),
) )
other.set_save_status(True) other.set_save_status(True)
return other return other

View File

@ -150,6 +150,7 @@ class Model(ModelRow):
for related in self.extract_related_names(): for related in self.extract_related_names():
if relation_map and related in relation_map: if relation_map and related in relation_map:
value = getattr(self, related) value = getattr(self, related)
if value:
update_count = await self._update_and_follow( update_count = await self._update_and_follow(
value=value, value=value,
follow=follow, follow=follow,

View File

@ -650,7 +650,7 @@ class QuerySet(Generic[T]):
:return: number of updated rows :return: number of updated rows
:rtype: int :rtype: int
""" """
if not each and not self.filter_clauses: if not each and not (self.filter_clauses or self.exclude_clauses):
raise QueryDefinitionError( raise QueryDefinitionError(
"You cannot update without filtering the queryset first. " "You cannot update without filtering the queryset first. "
"If you want to update all rows use update(each=True, **kwargs)" "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( expr = FilterQuery(filter_clauses=self.filter_clauses).apply(
self.table.update().values(**updates) self.table.update().values(**updates)
) )
expr = FilterQuery(filter_clauses=self.exclude_clauses, exclude=True).apply(
expr
)
return await self.database.execute(expr) return await self.database.execute(expr)
async def delete(self, each: bool = False, **kwargs: Any) -> int: async def delete(self, each: bool = False, **kwargs: Any) -> int:
@ -684,7 +687,7 @@ class QuerySet(Generic[T]):
""" """
if kwargs: if kwargs:
return await self.filter(**kwargs).delete() 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( raise QueryDefinitionError(
"You cannot delete without filtering the queryset first. " "You cannot delete without filtering the queryset first. "
"If you want to delete all rows use delete(each=True)" "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( expr = FilterQuery(filter_clauses=self.filter_clauses).apply(
self.table.delete() self.table.delete()
) )
expr = FilterQuery(filter_clauses=self.exclude_clauses, exclude=True).apply(
expr
)
return await self.database.execute(expr) return await self.database.execute(expr)
def paginate(self, page: int, page_size: int = 20) -> "QuerySet[T]": def paginate(self, page: int, page_size: int = 20) -> "QuerySet[T]":

View File

@ -389,7 +389,11 @@ class QuerysetProxy(Generic[T]):
:rtype: int :rtype: int
""" """
# queryset proxy always have one filter for pk of parent model # 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( raise QueryDefinitionError(
"You cannot update without filtering the queryset first. " "You cannot update without filtering the queryset first. "
"If you want to update all rows use update(each=True, **kwargs)" "If you want to update all rows use update(each=True, **kwargs)"

View File

@ -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"}]