From 4071ff7d115d91d8b2606b97b5134c3e1cbc4972 Mon Sep 17 00:00:00 2001 From: collerek Date: Sun, 10 Jan 2021 12:06:49 +0100 Subject: [PATCH] WIP skip relation registration on m2m through instance, simplify registering relations part 2 --- ormar/fields/base.py | 1 - ormar/fields/foreign_key.py | 47 +++++++++++++++-------------- ormar/fields/many_to_many.py | 16 +++++++++- ormar/models/helpers/relations.py | 43 +++++++++++--------------- ormar/models/helpers/sqlalchemy.py | 10 ++++-- ormar/models/metaclass.py | 4 ++- ormar/models/newbasemodel.py | 12 ++++---- ormar/relations/alias_manager.py | 15 ++------- ormar/relations/querysetproxy.py | 15 +++++++-- ormar/relations/relation.py | 8 ++++- ormar/relations/relation_manager.py | 24 ++++----------- ormar/relations/utils.py | 27 ++++------------- tests/test_save_related.py | 12 ++++---- 13 files changed, 114 insertions(+), 120 deletions(-) diff --git a/ormar/fields/base.py b/ormar/fields/base.py index 5c2f4c7..b11a7d7 100644 --- a/ormar/fields/base.py +++ b/ormar/fields/base.py @@ -247,7 +247,6 @@ class BaseField(FieldInfo): value: Any, child: Union["Model", "NewBaseModel"], to_register: bool = True, - relation_name: str = None, ) -> Any: """ Function overwritten for relations, in basic field the value is returned as is. diff --git a/ormar/fields/foreign_key.py b/ormar/fields/foreign_key.py index 1fb77b9..e141e5c 100644 --- a/ormar/fields/foreign_key.py +++ b/ormar/fields/foreign_key.py @@ -213,13 +213,24 @@ class ForeignKeyField(BaseField): ondelete: str onupdate: str + @classmethod + def get_source_related_name(cls) -> str: + """ + Returns name to use for source relation name. + For FK it's the same, differs for m2m fields. + It's either set as `related_name` or by default it's owner model. get_name + 's' + :return: name of the related_name or default related name. + :rtype: str + """ + return cls.get_related_name() + @classmethod def get_related_name(cls) -> str: """ Returns name to use for reverse relation. It's either set as `related_name` or by default it's owner model. get_name + 's' - :return: - :rtype: + :return: name of the related_name or default related name. + :rtype: str """ return cls.related_name or cls.owner.get_name() + "s" @@ -250,7 +261,7 @@ class ForeignKeyField(BaseField): @classmethod def _extract_model_from_sequence( - cls, value: List, child: "Model", to_register: bool, relation_name: str + cls, value: List, child: "Model", to_register: bool, ) -> List["Model"]: """ Takes a list of Models and registers them on parent. @@ -269,17 +280,14 @@ class ForeignKeyField(BaseField): """ return [ cls.expand_relationship( # type: ignore - value=val, - child=child, - to_register=to_register, - relation_name=relation_name, + value=val, child=child, to_register=to_register, ) for val in value ] @classmethod def _register_existing_model( - cls, value: "Model", child: "Model", to_register: bool, relation_name: str + cls, value: "Model", child: "Model", to_register: bool, ) -> "Model": """ Takes already created instance and registers it for parent. @@ -297,12 +305,12 @@ class ForeignKeyField(BaseField): :rtype: Model """ if to_register: - cls.register_relation(model=value, child=child, relation_name=relation_name) + cls.register_relation(model=value, child=child) return value @classmethod def _construct_model_from_dict( - cls, value: dict, child: "Model", to_register: bool, relation_name: str + cls, value: dict, child: "Model", to_register: bool ) -> "Model": """ Takes a dictionary, creates a instance and registers it for parent. @@ -324,12 +332,12 @@ class ForeignKeyField(BaseField): value["__pk_only__"] = True model = cls.to(**value) if to_register: - cls.register_relation(model=model, child=child, relation_name=relation_name) + cls.register_relation(model=model, child=child) return model @classmethod def _construct_model_from_pk( - cls, value: Any, child: "Model", to_register: bool, relation_name: str + cls, value: Any, child: "Model", to_register: bool ) -> "Model": """ Takes a pk value, creates a dummy instance and registers it for parent. @@ -356,13 +364,11 @@ class ForeignKeyField(BaseField): ) model = create_dummy_instance(fk=cls.to, pk=value) if to_register: - cls.register_relation(model=model, child=child, relation_name=relation_name) + cls.register_relation(model=model, child=child) return model @classmethod - def register_relation( - cls, model: "Model", child: "Model", relation_name: str - ) -> None: + def register_relation(cls, model: "Model", child: "Model") -> None: """ Registers relation between parent and child in relation manager. Relation manager is kep on each model (different instance). @@ -376,11 +382,7 @@ class ForeignKeyField(BaseField): :type child: Model class """ model._orm.add( - parent=model, - child=child, - child_name=cls.related_name or child.get_name() + "s", - virtual=cls.virtual, - relation_name=relation_name, + parent=model, child=child, field=cls, ) @classmethod @@ -400,7 +402,6 @@ class ForeignKeyField(BaseField): value: Any, child: Union["Model", "NewBaseModel"], to_register: bool = True, - relation_name: str = None, ) -> Optional[Union["Model", List["Model"]]]: """ For relations the child model is first constructed (if needed), @@ -429,5 +430,5 @@ class ForeignKeyField(BaseField): model = constructors.get( # type: ignore value.__class__.__name__, cls._construct_model_from_pk - )(value, child, to_register, relation_name) + )(value, child, to_register) return model diff --git a/ormar/fields/many_to_many.py b/ormar/fields/many_to_many.py index 1d16750..b9b8049 100644 --- a/ormar/fields/many_to_many.py +++ b/ormar/fields/many_to_many.py @@ -14,7 +14,7 @@ REF_PREFIX = "#/components/schemas/" def populate_m2m_params_based_on_to_model( to: Type["Model"], nullable: bool -) -> Tuple[List, Any]: +) -> Tuple[Any, Any]: """ Based on target to model to which relation leads to populates the type of the pydantic field to use and type of the target column field. @@ -106,6 +106,20 @@ class ManyToManyField(ForeignKeyField, ormar.QuerySetProtocol, ormar.RelationPro Actual class returned from ManyToMany function call and stored in model_fields. """ + @classmethod + def get_source_related_name(cls) -> str: + """ + Returns name to use for source relation name. + For FK it's the same, differs for m2m fields. + It's either set as `related_name` or by default it's field name. + :return: name of the related_name or default related name. + :rtype: str + """ + return ( + cls.through.Meta.model_fields[cls.default_source_field_name()].related_name + or cls.name + ) + @classmethod def default_target_field_name(cls) -> str: """ diff --git a/ormar/models/helpers/relations.py b/ormar/models/helpers/relations.py index 056faab..5acbb52 100644 --- a/ormar/models/helpers/relations.py +++ b/ormar/models/helpers/relations.py @@ -13,7 +13,7 @@ if TYPE_CHECKING: # pragma no cover alias_manager = AliasManager() -def register_relation_on_build(new_model: Type["Model"], field_name: str) -> None: +def register_relation_on_build(field: Type["ForeignKeyField"]) -> None: """ Registers ForeignKey relation in alias_manager to set a table_prefix. Registration include also reverse relation side to be able to join both sides. @@ -22,17 +22,17 @@ def register_relation_on_build(new_model: Type["Model"], field_name: str) -> Non relations between two Models that needs to have different aliases for proper sql joins. - :param new_model: constructed model - :type new_model: Model class - :param field_name: name of the related field - :type field_name: str + :param field: relation field + :type field: ForeignKey class """ - alias_manager.add_relation_type(new_model, field_name) + alias_manager.add_relation_type( + source_model=field.owner, + relation_name=field.name, + reverse_name=field.get_source_related_name(), + ) -def register_many_to_many_relation_on_build( - new_model: Type["Model"], field: Type[ManyToManyField], field_name: str -) -> None: +def register_many_to_many_relation_on_build(field: Type[ManyToManyField]) -> None: """ Registers connection between through model and both sides of the m2m relation. Registration include also reverse relation side to be able to join both sides. @@ -43,21 +43,18 @@ def register_many_to_many_relation_on_build( By default relation name is a model.name.lower(). - :param field_name: name of the relation key - :type field_name: str - :param new_model: model on which m2m field is declared - :type new_model: Model class :param field: relation field :type field: ManyToManyField class """ alias_manager.add_relation_type( - field.through, new_model.get_name(), is_multi=True, reverse_name=field_name + source_model=field.through, + relation_name=field.default_source_field_name(), + reverse_name=field.get_source_related_name(), ) alias_manager.add_relation_type( - field.through, - field.to.get_name(), - is_multi=True, - reverse_name=field.related_name or new_model.get_name() + "s", + source_model=field.through, + relation_name=field.default_target_field_name(), + reverse_name=field.get_related_name(), ) @@ -126,7 +123,7 @@ def register_reverse_model_fields(model_field: Type["ForeignKeyField"]) -> None: def register_relation_in_alias_manager( - new_model: Type["Model"], field: Type[ForeignKeyField], field_name: str + field: Type[ForeignKeyField], field_name: str ) -> None: """ Registers the relation (and reverse relation) in alias manager. @@ -137,8 +134,6 @@ def register_relation_in_alias_manager( m2m - register_many_to_many_relation_on_build fk - register_relation_on_build - :param new_model: model on which relation field is declared - :type new_model: Model class :param field: relation field :type field: ForeignKey or ManyToManyField class :param field_name: name of the relation key @@ -147,13 +142,11 @@ def register_relation_in_alias_manager( if issubclass(field, ManyToManyField): if field.has_unresolved_forward_refs(): return - register_many_to_many_relation_on_build( - new_model=new_model, field=field, field_name=field_name - ) + register_many_to_many_relation_on_build(field=field) elif issubclass(field, ForeignKeyField): if field.has_unresolved_forward_refs(): return - register_relation_on_build(new_model=new_model, field_name=field_name) + register_relation_on_build(field=field) def verify_related_name_dont_duplicate( diff --git a/ormar/models/helpers/sqlalchemy.py b/ormar/models/helpers/sqlalchemy.py index 6c96f7e..3ddf29d 100644 --- a/ormar/models/helpers/sqlalchemy.py +++ b/ormar/models/helpers/sqlalchemy.py @@ -29,10 +29,16 @@ def adjust_through_many_to_many_model(model_field: Type[ManyToManyField]) -> Non child_name = model_field.default_source_field_name() model_field.through.Meta.model_fields[parent_name] = ForeignKey( - model_field.to, real_name=parent_name, ondelete="CASCADE" + model_field.to, + real_name=parent_name, + ondelete="CASCADE", + owner=model_field.owner, ) model_field.through.Meta.model_fields[child_name] = ForeignKey( - model_field.owner, real_name=child_name, ondelete="CASCADE" + model_field.owner, + real_name=child_name, + ondelete="CASCADE", + owner=model_field.owner, ) create_and_append_m2m_fk( diff --git a/ormar/models/metaclass.py b/ormar/models/metaclass.py index f808e60..42be22c 100644 --- a/ormar/models/metaclass.py +++ b/ormar/models/metaclass.py @@ -587,7 +587,9 @@ class ModelMetaclass(pydantic.main.ModelMetaclass): populate_meta_sqlalchemy_table_if_required(new_model.Meta) expand_reverse_relationships(new_model) for field_name, field in new_model.Meta.model_fields.items(): - register_relation_in_alias_manager(new_model, field, field_name) + register_relation_in_alias_manager( + field=field, field_name=field_name + ) if new_model.Meta.pkname not in attrs["__annotations__"]: field_name = new_model.Meta.pkname diff --git a/ormar/models/newbasemodel.py b/ormar/models/newbasemodel.py index 0469d07..ed558e0 100644 --- a/ormar/models/newbasemodel.py +++ b/ormar/models/newbasemodel.py @@ -15,6 +15,7 @@ from typing import ( Type, TypeVar, Union, + cast, ) try: @@ -143,7 +144,7 @@ class NewBaseModel(pydantic.BaseModel, ModelTableProxy, metaclass=ModelMetaclass k: self._convert_json( k, self.Meta.model_fields[k].expand_relationship( - v, self, to_register=False, relation_name=k + v, self, to_register=False, ), "dumps", ) @@ -172,7 +173,7 @@ class NewBaseModel(pydantic.BaseModel, ModelTableProxy, metaclass=ModelMetaclass # register the columns models after initialization for related in self.extract_related_names(): self.Meta.model_fields[related].expand_relationship( - new_kwargs.get(related), self, to_register=True, relation_name=related + new_kwargs.get(related), self, to_register=True, ) def __setattr__(self, name: str, value: Any) -> None: # noqa CCR001 @@ -209,7 +210,7 @@ class NewBaseModel(pydantic.BaseModel, ModelTableProxy, metaclass=ModelMetaclass self.set_save_status(False) elif name in self._orm: model = self.Meta.model_fields[name].expand_relationship( - value=value, child=self, relation_name=name + value=value, child=self ) if isinstance(self.__dict__.get(name), list): # virtual foreign key or many to many @@ -447,13 +448,12 @@ class NewBaseModel(pydantic.BaseModel, ModelTableProxy, metaclass=ModelMetaclass fields_to_check = cls.Meta.model_fields.copy() for field_name, field in fields_to_check.items(): if field.has_unresolved_forward_refs(): + field = cast(Type[ForeignKeyField], field) field.evaluate_forward_ref(globalns=globalns, localns=localns) field.set_self_reference_flag() expand_reverse_relationship(model_field=field) register_relation_in_alias_manager( - cls, # type: ignore - field, - field_name, + field=field, field_name=field_name, ) update_column_definition(model=cls, field=field) populate_meta_sqlalchemy_table_if_required(meta=cls.Meta) diff --git a/ormar/relations/alias_manager.py b/ormar/relations/alias_manager.py index 1e6a6cd..7f7fd7c 100644 --- a/ormar/relations/alias_manager.py +++ b/ormar/relations/alias_manager.py @@ -79,11 +79,7 @@ class AliasManager: return text(f"{name} {alias}_{name}") def add_relation_type( - self, - source_model: Type["Model"], - relation_name: str, - reverse_name: str = None, - is_multi: bool = False, + self, source_model: Type["Model"], relation_name: str, reverse_name: str = None, ) -> None: """ Registers the relations defined in ormar models. @@ -104,21 +100,16 @@ class AliasManager: :type relation_name: str :param reverse_name: name of related_name fo given relation for m2m relations :type reverse_name: Optional[str] - :param is_multi: flag if relation being registered is a through m2m model - :type is_multi: bool :return: none :rtype: None """ parent_key = f"{source_model.get_name()}_{relation_name}" if parent_key not in self._aliases_new: self._aliases_new[parent_key] = get_table_alias() + to_field = source_model.Meta.model_fields[relation_name] child_model = to_field.to - related_name = to_field.related_name - if not related_name: - related_name = reverse_name if is_multi else source_model.get_name() + "s" - - child_key = f"{child_model.get_name()}_{related_name}" + child_key = f"{child_model.get_name()}_{reverse_name}" if child_key not in self._aliases_new: self._aliases_new[child_key] = get_table_alias() diff --git a/ormar/relations/querysetproxy.py b/ormar/relations/querysetproxy.py index 386ac63..841071a 100644 --- a/ormar/relations/querysetproxy.py +++ b/ormar/relations/querysetproxy.py @@ -12,6 +12,7 @@ from typing import ( ) import ormar +from ormar.exceptions import ModelPersistenceError if TYPE_CHECKING: # pragma no cover from ormar.relations import Relation @@ -106,11 +107,19 @@ class QuerysetProxy(ormar.QuerySetProtocol): :param child: child model instance :type child: Model """ - queryset = ormar.QuerySet(model_cls=self.relation.through) + model_cls = self.relation.through owner_column = self._owner.get_name() child_column = child.get_name() - kwargs = {owner_column: self._owner, child_column: child} - await queryset.create(**kwargs) + kwargs = {owner_column: self._owner.pk, child_column: child.pk} + if child.pk is None: + raise ModelPersistenceError( + f"You cannot save {child.get_name()} " + f"model without primary key set! \n" + f"Save the child model first." + ) + expr = model_cls.Meta.table.insert() + expr = expr.values(**kwargs) + await model_cls.Meta.database.execute(expr) async def delete_through_instance(self, child: "T") -> None: """ diff --git a/ormar/relations/relation.py b/ormar/relations/relation.py index d037d4a..9eb8e60 100644 --- a/ormar/relations/relation.py +++ b/ormar/relations/relation.py @@ -63,7 +63,7 @@ class Relation: self._type: RelationType = type_ self._to_remove: Set = set() self.to: Type["T"] = to - self.through: Optional[Type["T"]] = through + self._through: Optional[Type["T"]] = through self.field_name = field_name self.related_models: Optional[Union[RelationProxy, "T"]] = ( RelationProxy(relation=self, type_=type_, field_name=field_name) @@ -71,6 +71,12 @@ class Relation: else None ) + @property + def through(self) -> Type["T"]: + if not self._through: # pragma: no cover + raise RelationshipInstanceError("Relation does not have through model!") + return self._through + def _clean_related(self) -> None: """ Removes dead weakrefs from RelationProxy. diff --git a/ormar/relations/relation_manager.py b/ormar/relations/relation_manager.py index 99d96f1..2e2b733 100644 --- a/ormar/relations/relation_manager.py +++ b/ormar/relations/relation_manager.py @@ -101,13 +101,7 @@ class RelationsManager: return None @staticmethod - def add( - parent: "Model", - child: "Model", - child_name: str, - virtual: bool, - relation_name: str, - ) -> None: + def add(parent: "Model", child: "Model", field: Type["ForeignKeyField"],) -> None: """ Adds relation on both sides -> meaning on both child and parent models. One side of the relation is always weakref proxy to avoid circular refs. @@ -120,25 +114,19 @@ class RelationsManager: :type parent: Model :param child: child model to register :type child: Model - :param child_name: potential child name used if related name is not set - :type child_name: str - :param virtual: - :type virtual: bool - :param relation_name: name of the relation - :type relation_name: str + :param field: field with relation definition + :type field: ForeignKeyField """ - to_field: Type[BaseField] = child.Meta.model_fields[relation_name] - # print('comming', child_name, relation_name) (parent, child, child_name, to_name,) = get_relations_sides_and_names( - to_field, parent, child, child_name, virtual, relation_name + field, parent, child ) - # print('adding', parent.get_name(), child.get_name(), child_name) + # print('adding parent', parent.get_name(), child.get_name(), child_name) parent_relation = parent._orm._get(child_name) if parent_relation: parent_relation.add(child) # type: ignore - # print('adding', child.get_name(), parent.get_name(), child_name) + # print('adding child', child.get_name(), parent.get_name(), to_name) child_relation = child._orm._get(to_name) if child_relation: child_relation.add(parent) diff --git a/ormar/relations/utils.py b/ormar/relations/utils.py index d900bdb..f9315f2 100644 --- a/ormar/relations/utils.py +++ b/ormar/relations/utils.py @@ -1,48 +1,33 @@ from typing import TYPE_CHECKING, Tuple, Type from weakref import proxy -from ormar.fields import BaseField -from ormar.fields.many_to_many import ManyToManyField +from ormar.fields.foreign_key import ForeignKeyField if TYPE_CHECKING: # pragma no cover from ormar import Model def get_relations_sides_and_names( - to_field: Type[BaseField], - parent: "Model", - child: "Model", - child_name: str, - virtual: bool, - relation_name: str, + to_field: Type[ForeignKeyField], parent: "Model", child: "Model", ) -> Tuple["Model", "Model", str, str]: """ Determines the names of child and parent relations names, as well as changes one of the sides of the relation into weakref.proxy to model. :param to_field: field with relation definition - :type to_field: BaseField + :type to_field: ForeignKeyField :param parent: parent model :type parent: Model :param child: child model :type child: Model - :param child_name: name of the child - :type child_name: str - :param virtual: flag if relation is virtual - :type virtual: bool - :param relation_name: - :type relation_name: :return: parent, child, child_name, to_name :rtype: Tuple["Model", "Model", str, str] """ to_name = to_field.name - if issubclass(to_field, ManyToManyField): - child_name = to_field.related_name or child.get_name() + "s" - child = proxy(child) - elif virtual: - child_name, to_name = to_name, child_name or child.get_name() + child_name = to_field.get_related_name() + if to_field.virtual: + child_name, to_name = to_name, child_name child, parent = parent, proxy(child) else: - child_name = child_name or child.get_name() + "s" child = proxy(child) return parent, child, child_name, to_name diff --git a/tests/test_save_related.py b/tests/test_save_related.py index 0696018..fc9f3ff 100644 --- a/tests/test_save_related.py +++ b/tests/test_save_related.py @@ -21,7 +21,7 @@ class CringeLevel(ormar.Model): name: str = ormar.String(max_length=100) -class NickNames(ormar.Model): +class NickName(ormar.Model): class Meta: tablename = "nicks" metadata = metadata @@ -48,7 +48,7 @@ class HQ(ormar.Model): id: int = ormar.Integer(primary_key=True) name: str = ormar.String(max_length=100, nullable=False, name="hq_name") - nicks: List[NickNames] = ormar.ManyToMany(NickNames, through=NicksHq) + nicks: List[NickName] = ormar.ManyToMany(NickName, through=NicksHq) class Company(ormar.Model): @@ -96,8 +96,8 @@ async def test_saving_related_fk_rel(): async def test_saving_many_to_many(): async with database: async with database.transaction(force_rollback=True): - nick1 = await NickNames.objects.create(name="BazingaO", is_lame=False) - nick2 = await NickNames.objects.create(name="Bazinga20", is_lame=True) + nick1 = await NickName.objects.create(name="BazingaO", is_lame=False) + nick2 = await NickName.objects.create(name="Bazinga20", is_lame=True) hq = await HQ.objects.create(name="Main") assert hq.saved @@ -168,10 +168,10 @@ async def test_saving_nested(): async with database.transaction(force_rollback=True): level = await CringeLevel.objects.create(name="High") level2 = await CringeLevel.objects.create(name="Low") - nick1 = await NickNames.objects.create( + nick1 = await NickName.objects.create( name="BazingaO", is_lame=False, level=level ) - nick2 = await NickNames.objects.create( + nick2 = await NickName.objects.create( name="Bazinga20", is_lame=True, level=level2 )