diff --git a/docs/relations/foreign-key.md b/docs/relations/foreign-key.md index ad37afc3c..23671c230 100644 --- a/docs/relations/foreign-key.md +++ b/docs/relations/foreign-key.md @@ -2,7 +2,7 @@ `ForeignKey(to: Model, *, name: str = None, unique: bool = False, nullable: bool = True, related_name: str = None, virtual: bool = False, onupdate: Union[ReferentialAction, str] = None, -ondelete: Union[ReferentialAction, str] = None, **kwargs: Any)` +ondelete: Union[ReferentialAction, str] = None, foreign_key_name: str = None, **kwargs: Any)` has required parameters `to` that takes target `Model` class. Sqlalchemy column and type are automatically taken from target `Model`. @@ -220,6 +220,28 @@ Set the ForeignKey to its default value; a `server_default` for the ForeignKey m Take `NO ACTION`; NO ACTION and RESTRICT are very much alike. The main difference between NO ACTION and RESTRICT is that with NO ACTION the referential integrity check is done after trying to alter the table. RESTRICT does the check before trying to execute the UPDATE or DELETE statement. Both referential actions act the same if the referential integrity check fails: the UPDATE or DELETE statement will result in an error. +## Overriding the foreign key constraint name + +By default ormar generates the foreign key constraint name as +`fk_{source_table}_{target_table}_{target_pk}_{field_name}`. On databases with a +short identifier length limit (for example MySQL's 64 character limit) the +auto-generated name can be truncated or rejected. Pass `foreign_key_name` to use +a custom name instead: + +```python +class Book(ormar.Model): + ormar_config = base_ormar_config.copy(tablename="books") + + id: int = ormar.Integer(primary_key=True) + author = ormar.ForeignKey( + Author, + foreign_key_name="fk_books_author", + ) +``` + +When used on an abstract base class, each subclass suffixes the name with its +own tablename to avoid constraint name collisions across sibling tables. + ## Relation Setup You have several ways to set-up a relationship connection. diff --git a/docs/relations/many-to-many.md b/docs/relations/many-to-many.md index e3a81945e..5cf713307 100644 --- a/docs/relations/many-to-many.md +++ b/docs/relations/many-to-many.md @@ -221,6 +221,33 @@ class StudentCourse(ormar.Model): provide your own custom Through model you cannot change the names there and you need to use same `through_relation_name` and `through_reverse_relation_name` parameters. +## Overriding foreign key constraint names on the through model + +Auto-generated foreign key constraint names on the through model can be +overridden with: + +* `through_foreign_key_name` - name of the FK constraint on the column that + references the model where `ManyToMany` is declared (the owner side). +* `through_reverse_foreign_key_name` - name of the FK constraint on the column + that references the target model. + +This is primarily useful for databases with short identifier limits (for +example MySQL's 64 character limit) where the auto-generated name would be +truncated. + +```python +class Student(ormar.Model): + ormar_config = base_ormar_config.copy() + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + courses = ormar.ManyToMany( + Course, + through_foreign_key_name="fk_sc_student", + through_reverse_foreign_key_name="fk_sc_course", + ) +``` + ## Through Fields The through field is auto added to the reverse side of the relation. diff --git a/ormar/fields/base.py b/ormar/fields/base.py index 77b5a5934..15bd01950 100644 --- a/ormar/fields/base.py +++ b/ormar/fields/base.py @@ -57,6 +57,12 @@ def __init__(self, **kwargs: Any) -> None: self.through_reverse_relation_name = kwargs.pop( "through_reverse_relation_name", None ) + self.through_foreign_key_name: Optional[str] = kwargs.pop( + "through_foreign_key_name", None + ) + self.through_reverse_foreign_key_name: Optional[str] = kwargs.pop( + "through_reverse_foreign_key_name", None + ) self.skip_reverse: bool = kwargs.pop("skip_reverse", False) self.skip_field: bool = kwargs.pop("skip_field", False) @@ -257,16 +263,20 @@ def construct_constraints(self) -> list: :return: list of sqlalchemy foreign keys - by default one. :rtype: list[sqlalchemy.schema.ForeignKey] """ - constraints = [ - sqlalchemy.ForeignKey( - con.reference, - ondelete=con.ondelete, - onupdate=con.onupdate, - name=f"fk_{self.owner.ormar_config.tablename}_{self.to.ormar_config.tablename}" - f"_{self.to.get_column_alias(self.to.ormar_config.pkname)}_{self.name}", + constraints = [] + for constraint in self.constraints: + owner_table = self.owner.ormar_config.tablename + target_table = self.to.ormar_config.tablename + target_pk = self.to.get_column_alias(self.to.ormar_config.pkname) + default_name = f"fk_{owner_table}_{target_table}_{target_pk}_{self.name}" + constraints.append( + sqlalchemy.ForeignKey( + constraint.reference, + ondelete=constraint.ondelete, + onupdate=constraint.onupdate, + name=constraint.name or default_name, + ) ) - for con in self.constraints - ] return constraints def get_column(self, name: str) -> sqlalchemy.Column: diff --git a/ormar/fields/foreign_key.py b/ormar/fields/foreign_key.py index d5fd6dff7..308f1fab4 100644 --- a/ormar/fields/foreign_key.py +++ b/ormar/fields/foreign_key.py @@ -81,6 +81,7 @@ def populate_fk_params_based_on_to_model( nullable: bool, onupdate: Optional[str] = None, ondelete: Optional[str] = None, + foreign_key_name: Optional[str] = None, ) -> tuple[Any, list, Any, Any]: """ Based on target to model to which relation leads to populates the type of the @@ -96,6 +97,9 @@ def populate_fk_params_based_on_to_model( :param ondelete: parameter passed to sqlalchemy.ForeignKey. How to treat child rows on delete of parent (the one where FK is defined) model. :type ondelete: str + :param foreign_key_name: optional override for the foreign key constraint name + emitted in migrations. Defaults to ``None``, which lets ormar generate a name. + :type foreign_key_name: Optional[str] :return: tuple with target pydantic type, list of fk constraints and target col type :rtype: tuple[Any, list, Any] """ @@ -111,7 +115,10 @@ def populate_fk_params_based_on_to_model( ) constraints = [ ForeignKeyConstraint( - reference=fk_string, ondelete=ondelete, onupdate=onupdate, name=None + reference=fk_string, + ondelete=ondelete, + onupdate=onupdate, + name=foreign_key_name, ) ] column_type = to_field.column_type @@ -203,6 +210,7 @@ def ForeignKey( # type: ignore # noqa CFQ002 virtual: bool = False, onupdate: Union[ReferentialAction, str, None] = None, ondelete: Union[ReferentialAction, str, None] = None, + foreign_key_name: Optional[str] = None, **kwargs: Any, ) -> "T": """ @@ -230,6 +238,10 @@ def ForeignKey( # type: ignore # noqa CFQ002 :param ondelete: parameter passed to sqlalchemy.ForeignKey. How to treat child rows on delete of parent (the one where FK is defined) model. :type ondelete: Union[ReferentialAction, str] + :param foreign_key_name: optional override for the foreign key constraint name + generated in migrations. Useful when the auto-generated name exceeds a database + specific identifier length limit (for example MySQL 64 chars). + :type foreign_key_name: Optional[str] :param kwargs: all other args to be populated by BaseField :type kwargs: Any :return: ormar ForeignKeyField with relation to selected model @@ -270,6 +282,7 @@ def ForeignKey( # type: ignore # noqa CFQ002 nullable=nullable, ondelete=ondelete, onupdate=onupdate, + foreign_key_name=foreign_key_name, ) namespace = dict( @@ -292,6 +305,7 @@ def ForeignKey( # type: ignore # noqa CFQ002 server_default=None, onupdate=onupdate, ondelete=ondelete, + foreign_key_name=foreign_key_name, owner=owner, self_reference=self_reference, is_relation=True, @@ -316,6 +330,7 @@ def __init__(self, **kwargs: Any) -> None: self.to: type["Model"] self.ondelete: str = kwargs.pop("ondelete", None) self.onupdate: str = kwargs.pop("onupdate", None) + self.foreign_key_name: Optional[str] = kwargs.pop("foreign_key_name", None) super().__init__(**kwargs) def get_source_related_name(self) -> str: @@ -447,6 +462,7 @@ def evaluate_forward_ref(self, globalns: Any, localns: Any) -> None: nullable=self.nullable, ondelete=self.ondelete, onupdate=self.onupdate, + foreign_key_name=self.foreign_key_name, ) def _extract_model_from_sequence( diff --git a/ormar/fields/many_to_many.py b/ormar/fields/many_to_many.py index 924e79d76..075149f92 100644 --- a/ormar/fields/many_to_many.py +++ b/ormar/fields/many_to_many.py @@ -128,6 +128,11 @@ def ManyToMany( # type: ignore through_relation_name = kwargs.pop("through_relation_name", None) through_reverse_relation_name = kwargs.pop("through_reverse_relation_name", None) + through_foreign_key_name = kwargs.pop("through_foreign_key_name", None) + through_reverse_foreign_key_name = kwargs.pop( + "through_reverse_foreign_key_name", None + ) + if through is not None and through.__class__ != ForwardRef: forbid_through_relations(cast(type["Model"], through)) @@ -171,6 +176,8 @@ def ManyToMany( # type: ignore skip_field=skip_field, through_relation_name=through_relation_name, through_reverse_relation_name=through_reverse_relation_name, + through_foreign_key_name=through_foreign_key_name, + through_reverse_foreign_key_name=through_reverse_foreign_key_name, ) Field = type("ManyToMany", (ManyToManyField, BaseField), {}) diff --git a/ormar/models/helpers/sqlalchemy.py b/ormar/models/helpers/sqlalchemy.py index 55fc297b1..2d24efa03 100644 --- a/ormar/models/helpers/sqlalchemy.py +++ b/ormar/models/helpers/sqlalchemy.py @@ -44,10 +44,16 @@ def adjust_through_many_to_many_model(model_field: "ManyToManyField") -> None: ) create_and_append_m2m_fk( - model=model_field.to, model_field=model_field, field_name=parent_name + model=model_field.to, + model_field=model_field, + field_name=parent_name, + foreign_key_name=model_field.through_reverse_foreign_key_name, ) create_and_append_m2m_fk( - model=model_field.owner, model_field=model_field, field_name=child_name + model=model_field.owner, + model_field=model_field, + field_name=child_name, + foreign_key_name=model_field.through_foreign_key_name, ) create_pydantic_field(parent_name, model_field.to, model_field) @@ -58,7 +64,10 @@ def adjust_through_many_to_many_model(model_field: "ManyToManyField") -> None: def create_and_append_m2m_fk( - model: type["Model"], model_field: "ManyToManyField", field_name: str + model: type["Model"], + model_field: "ManyToManyField", + field_name: str, + foreign_key_name: Optional[str] = None, ) -> None: """ Registers sqlalchemy Column with sqlalchemy.ForeignKey leading to the model. @@ -72,6 +81,8 @@ def create_and_append_m2m_fk( :type model: Model class :param model_field: field with ManyToMany relation :type model_field: ManyToManyField field + :param foreign_key_name: optional override for the generated FK constraint name. + :type foreign_key_name: Optional[str] """ pk_alias = model.get_column_alias(model.ormar_config.pkname) pk_column = next( @@ -81,6 +92,9 @@ def create_and_append_m2m_fk( raise ormar.ModelDefinitionError( "ManyToMany relation cannot lead to field without pk" ) + through_table = model_field.through.ormar_config.tablename + target_table = model.ormar_config.tablename + default_name = f"fk_{through_table}_{target_table}_{field_name}_{pk_alias}" column = sqlalchemy.Column( field_name, pk_column.type, @@ -88,8 +102,7 @@ def create_and_append_m2m_fk( model.ormar_config.tablename + "." + pk_alias, ondelete="CASCADE", onupdate="CASCADE", - name=f"fk_{model_field.through.ormar_config.tablename}_{model.ormar_config.tablename}" - f"_{field_name}_{pk_alias}", + name=foreign_key_name or default_name, ), ) model_field.through.ormar_config.columns.append(column) diff --git a/ormar/models/metaclass.py b/ormar/models/metaclass.py index e423dceb2..2214c69a2 100644 --- a/ormar/models/metaclass.py +++ b/ormar/models/metaclass.py @@ -1,4 +1,5 @@ import copy +import dataclasses import sys import warnings from pathlib import Path @@ -385,13 +386,24 @@ def copy_data_from_parent_model( # noqa: CCR001 base_class=base_class, # type: ignore ) - elif field.is_relation and field.related_name: + elif field.is_relation and ( + field.related_name or cast(ForeignKeyField, field).foreign_key_name + ): + fk_field = cast(ForeignKeyField, field) Field = type( # type: ignore field.__class__.__name__, (ForeignKeyField, BaseField), {} ) copy_field = Field(**dict(field.__dict__)) - related_name = field.related_name + "_" + table_name - copy_field.related_name = related_name # type: ignore + if fk_field.related_name: + related_name = fk_field.related_name + "_" + table_name + copy_field.related_name = related_name # type: ignore + if fk_field.foreign_key_name: + new_fk_name = f"{fk_field.foreign_key_name}_{table_name}" + copy_field.foreign_key_name = new_fk_name # type: ignore + copy_field.constraints = [ + dataclasses.replace(constraint, name=new_fk_name) + for constraint in fk_field.constraints + ] parent_fields[field_name] = copy_field else: parent_fields[field_name] = field diff --git a/tests/test_inheritance_and_pydantic_generation/test_foreign_key_name_in_inheritance.py b/tests/test_inheritance_and_pydantic_generation/test_foreign_key_name_in_inheritance.py new file mode 100644 index 000000000..6e5a35031 --- /dev/null +++ b/tests/test_inheritance_and_pydantic_generation/test_foreign_key_name_in_inheritance.py @@ -0,0 +1,46 @@ +import ormar +from tests.lifespan import init_tests +from tests.settings import create_config + +base_ormar_config = create_config() + + +class FkInhParent(ormar.Model): + ormar_config = base_ormar_config.copy(tablename="fk_inh_parents") + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + + +class FkInhBase(ormar.Model): + ormar_config = base_ormar_config.copy(abstract=True) + + id: int = ormar.Integer(primary_key=True) + parent = ormar.ForeignKey( + FkInhParent, + related_name="kids", + foreign_key_name="fk_custom_parent", + ) + + +class FkInhChildA(FkInhBase): + ormar_config = base_ormar_config.copy(tablename="fk_inh_child_a") + + +class FkInhChildB(FkInhBase): + ormar_config = base_ormar_config.copy(tablename="fk_inh_child_b") + + +create_test_database = init_tests(base_ormar_config) + + +def _fk_names(table): + return [fk.name for col in table.c for fk in col.foreign_keys] + + +def test_foreign_key_name_is_suffixed_per_subclass_to_avoid_conflicts(): + a_names = _fk_names(FkInhChildA.ormar_config.table) + b_names = _fk_names(FkInhChildB.ormar_config.table) + assert "fk_custom_parent_fk_inh_child_a" in a_names + assert "fk_custom_parent_fk_inh_child_b" in b_names + assert set(a_names).isdisjoint(set(b_names)) diff --git a/tests/test_relations/test_foreign_key_name_override.py b/tests/test_relations/test_foreign_key_name_override.py new file mode 100644 index 000000000..d380d0741 --- /dev/null +++ b/tests/test_relations/test_foreign_key_name_override.py @@ -0,0 +1,113 @@ +from typing import Optional + +import pytest +import sqlalchemy + +import ormar +from tests.lifespan import init_tests +from tests.settings import create_config + +base_ormar_config = create_config() + + +class Author(ormar.Model): + ormar_config = base_ormar_config.copy(tablename="fkn_authors") + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + + +class Book(ormar.Model): + ormar_config = base_ormar_config.copy(tablename="fkn_books") + + id: int = ormar.Integer(primary_key=True) + title: str = ormar.String(max_length=100) + author: Optional[Author] = ormar.ForeignKey( + Author, foreign_key_name="my_custom_fk_books_author" + ) + + +class Tag(ormar.Model): + ormar_config = base_ormar_config.copy(tablename="fkn_tags") + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + + +class BookTag(ormar.Model): + ormar_config = base_ormar_config.copy(tablename="fkn_book_tags") + + id: int = ormar.Integer(primary_key=True) + + +class BookWithTags(ormar.Model): + ormar_config = base_ormar_config.copy(tablename="fkn_books_with_tags") + + id: int = ormar.Integer(primary_key=True) + title: str = ormar.String(max_length=100) + tags = ormar.ManyToMany( + Tag, + through=BookTag, + through_foreign_key_name="my_fk_through_to_book", + through_reverse_foreign_key_name="my_fk_through_to_tag", + ) + + +create_test_database = init_tests(base_ormar_config) + + +def _fk_names_for(table: sqlalchemy.Table) -> list: + names = [] + for col in table.c: + for fk in col.foreign_keys: + names.append(fk.name) + return names + + +def test_foreign_key_name_overrides_generated_constraint_name(): + names = _fk_names_for(Book.ormar_config.table) + assert "my_custom_fk_books_author" in names + assert not any(n.startswith("fk_fkn_books_") for n in names) + + +def test_foreign_key_name_default_is_generated(): + fk_author = Author.ormar_config.model_fields.get("id") + assert ( + fk_author.foreign_key_name is None + if hasattr(fk_author, "foreign_key_name") + else True + ) + + field = Book.ormar_config.model_fields["author"] + assert field.foreign_key_name == "my_custom_fk_books_author" + + +def test_many_to_many_through_foreign_key_name_overrides(): + through_table = BookWithTags.ormar_config.model_fields[ + "tags" + ].through.ormar_config.table + names = _fk_names_for(through_table) + assert "my_fk_through_to_book" in names + assert "my_fk_through_to_tag" in names + + +@pytest.mark.asyncio +async def test_model_with_custom_fk_name_still_works_at_runtime(): + async with base_ormar_config.database: + async with base_ormar_config.database.transaction(force_rollback=True): + author = await Author.objects.create(name="J. Doe") + book = await Book.objects.create(title="Untitled", author=author) + fetched = await Book.objects.select_related("author").get(id=book.id) + assert fetched.author.name == "J. Doe" + + +@pytest.mark.asyncio +async def test_many_to_many_with_custom_through_fk_names_still_works(): + async with base_ormar_config.database: + async with base_ormar_config.database.transaction(force_rollback=True): + tag = await Tag.objects.create(name="scifi") + book = await BookWithTags.objects.create(title="Dune") + await book.tags.add(tag) + fetched = await BookWithTags.objects.select_related("tags").get(id=book.id) + assert len(fetched.tags) == 1 + assert fetched.tags[0].name == "scifi"