From 0395e12d42eb11f2fbda24a96ad16b8c1e4409b2 Mon Sep 17 00:00:00 2001 From: Waket Zheng Date: Mon, 25 Nov 2024 12:06:51 +0800 Subject: [PATCH 1/2] fix: query error when m2m combine with o2o --- CHANGELOG.rst | 6 + tests/contrib/test_pydantic.py | 149 ++++++++++++++++++--- tests/test_relations.py | 20 +++ tests/testmodels.py | 14 ++ tortoise/backends/base/schema_generator.py | 23 +++- tortoise/models.py | 4 +- 6 files changed, 189 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 5463d46aa..9b0a4477e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -9,6 +9,12 @@ Changelog 0.22 ==== +0.22.1 +------ +Fixed +^^^^^ +- Fix unable to use ManyToManyField if OneToOneField passed as Primary Key (#1783) + 0.22.0 ------ Fixed diff --git a/tests/contrib/test_pydantic.py b/tests/contrib/test_pydantic.py index 391b751fa..2276005d3 100644 --- a/tests/contrib/test_pydantic.py +++ b/tests/contrib/test_pydantic.py @@ -68,11 +68,16 @@ def test_event_schema(self): self.Event_Pydantic.model_json_schema(), { "$defs": { - "Address_coqnj7_leaf": { + "Address_e4rhju_leaf": { "additionalProperties": False, "properties": { "city": {"maxLength": 64, "title": "City", "type": "string"}, "street": {"maxLength": 128, "title": "Street", "type": "string"}, + "m2mwitho2opks": { + "items": {"$ref": "#/$defs/M2mWithO2oPk_leajz6_leaf"}, + "title": "M2Mwitho2Opks", + "type": "array", + }, "event_id": { "maximum": 9223372036854775807, "minimum": -9223372036854775808, @@ -80,10 +85,25 @@ def test_event_schema(self): "type": "integer", }, }, - "required": ["city", "street", "event_id"], + "required": ["city", "street", "event_id", "m2mwitho2opks"], "title": "Address", "type": "object", }, + "M2mWithO2oPk_leajz6_leaf": { + "additionalProperties": False, + "properties": { + "id": { + "maximum": 2147483647, + "minimum": -2147483648, + "title": "Id", + "type": "integer", + }, + "name": {"maxLength": 64, "title": "Name", "type": "string"}, + }, + "required": ["id", "name"], + "title": "M2mWithO2oPk", + "type": "object", + }, "Reporter_fgnv33_leaf": { "additionalProperties": False, "description": "Whom is assigned as the reporter", @@ -202,7 +222,7 @@ def test_event_schema(self): }, "address": { "anyOf": [ - {"$ref": "#/$defs/Address_coqnj7_leaf"}, + {"$ref": "#/$defs/Address_e4rhju_leaf"}, {"type": "null"}, ], "nullable": True, @@ -229,7 +249,7 @@ def test_eventlist_schema(self): self.Event_Pydantic_List.model_json_schema(), { "$defs": { - "Event_padfez": { + "Event_mfxmwb": { "additionalProperties": False, "description": "Events on the calendar", "properties": { @@ -282,7 +302,7 @@ def test_eventlist_schema(self): }, "address": { "anyOf": [ - {"$ref": "#/$defs/Address_coqnj7_leaf"}, + {"$ref": "#/$defs/Address_e4rhju_leaf"}, {"type": "null"}, ], "nullable": True, @@ -302,11 +322,16 @@ def test_eventlist_schema(self): "title": "Event", "type": "object", }, - "Address_coqnj7_leaf": { + "Address_e4rhju_leaf": { "additionalProperties": False, "properties": { "city": {"maxLength": 64, "title": "City", "type": "string"}, "street": {"maxLength": 128, "title": "Street", "type": "string"}, + "m2mwitho2opks": { + "items": {"$ref": "#/$defs/M2mWithO2oPk_leajz6_leaf"}, + "title": "M2Mwitho2Opks", + "type": "array", + }, "event_id": { "maximum": 9223372036854775807, "minimum": -9223372036854775808, @@ -314,10 +339,25 @@ def test_eventlist_schema(self): "type": "integer", }, }, - "required": ["city", "street", "event_id"], + "required": ["city", "street", "event_id", "m2mwitho2opks"], "title": "Address", "type": "object", }, + "M2mWithO2oPk_leajz6_leaf": { + "additionalProperties": False, + "properties": { + "id": { + "maximum": 2147483647, + "minimum": -2147483648, + "title": "Id", + "type": "integer", + }, + "name": {"maxLength": 64, "title": "Name", "type": "string"}, + }, + "required": ["id", "name"], + "title": "M2mWithO2oPk", + "type": "object", + }, "Reporter_fgnv33_leaf": { "additionalProperties": False, "description": "Whom is assigned as the reporter", @@ -392,7 +432,7 @@ def test_eventlist_schema(self): }, }, "description": "Events on the calendar", - "items": {"$ref": "#/$defs/Event_padfez"}, + "items": {"$ref": "#/$defs/Event_mfxmwb"}, "title": "Event_list", "type": "array", }, @@ -467,6 +507,21 @@ def test_address_schema(self): "title": "Event", "type": "object", }, + "M2mWithO2oPk_leajz6_leaf": { + "additionalProperties": False, + "properties": { + "id": { + "maximum": 2147483647, + "minimum": -2147483648, + "title": "Id", + "type": "integer", + }, + "name": {"maxLength": 64, "title": "Name", "type": "string"}, + }, + "required": ["id", "name"], + "title": "M2mWithO2oPk", + "type": "object", + }, "Reporter_fgnv33_leaf": { "additionalProperties": False, "description": "Whom is assigned as the reporter", @@ -544,6 +599,11 @@ def test_address_schema(self): "properties": { "city": {"maxLength": 64, "title": "City", "type": "string"}, "street": {"maxLength": 128, "title": "Street", "type": "string"}, + "m2mwitho2opks": { + "items": {"$ref": "#/$defs/M2mWithO2oPk_leajz6_leaf"}, + "title": "M2Mwitho2Opks", + "type": "array", + }, "event": {"$ref": "#/$defs/Event_zvunzw_leaf"}, "event_id": { "maximum": 9223372036854775807, @@ -552,7 +612,7 @@ def test_address_schema(self): "type": "integer", }, }, - "required": ["city", "street", "event", "event_id"], + "required": ["city", "street", "event", "event_id", "m2mwitho2opks"], "title": "Address", "type": "object", }, @@ -563,7 +623,7 @@ def test_tournament_schema(self): self.Tournament_Pydantic.model_json_schema(), { "$defs": { - "Event_jgrv4c_leaf": { + "Event_ln6p2q_leaf": { "additionalProperties": False, "description": "Events on the calendar", "properties": { @@ -612,7 +672,7 @@ def test_tournament_schema(self): }, "address": { "anyOf": [ - {"$ref": "#/$defs/Address_coqnj7_leaf"}, + {"$ref": "#/$defs/Address_e4rhju_leaf"}, {"type": "null"}, ], "nullable": True, @@ -631,11 +691,16 @@ def test_tournament_schema(self): "title": "Event", "type": "object", }, - "Address_coqnj7_leaf": { + "Address_e4rhju_leaf": { "additionalProperties": False, "properties": { "city": {"maxLength": 64, "title": "City", "type": "string"}, "street": {"maxLength": 128, "title": "Street", "type": "string"}, + "m2mwitho2opks": { + "items": {"$ref": "#/$defs/M2mWithO2oPk_leajz6_leaf"}, + "title": "M2Mwitho2Opks", + "type": "array", + }, "event_id": { "maximum": 9223372036854775807, "minimum": -9223372036854775808, @@ -643,10 +708,25 @@ def test_tournament_schema(self): "type": "integer", }, }, - "required": ["city", "street", "event_id"], + "required": ["city", "street", "event_id", "m2mwitho2opks"], "title": "Address", "type": "object", }, + "M2mWithO2oPk_leajz6_leaf": { + "additionalProperties": False, + "properties": { + "id": { + "maximum": 2147483647, + "minimum": -2147483648, + "title": "Id", + "type": "integer", + }, + "name": {"maxLength": 64, "title": "Name", "type": "string"}, + }, + "required": ["id", "name"], + "title": "M2mWithO2oPk", + "type": "object", + }, "Reporter_fgnv33_leaf": { "additionalProperties": False, "description": "Whom is assigned as the reporter", @@ -711,7 +791,7 @@ def test_tournament_schema(self): }, "events": { "description": "What tournaments is a happenin'", - "items": {"$ref": "#/$defs/Event_jgrv4c_leaf"}, + "items": {"$ref": "#/$defs/Event_ln6p2q_leaf"}, "title": "Events", "type": "array", }, @@ -727,7 +807,7 @@ def test_team_schema(self): self.Team_Pydantic.model_json_schema(), { "$defs": { - "Event_n2kadx_leaf": { + "Event_lfs4vy_leaf": { "additionalProperties": False, "description": "Events on the calendar", "properties": { @@ -775,7 +855,7 @@ def test_team_schema(self): }, "address": { "anyOf": [ - {"$ref": "#/$defs/Address_coqnj7_leaf"}, + {"$ref": "#/$defs/Address_e4rhju_leaf"}, {"type": "null"}, ], "nullable": True, @@ -794,11 +874,16 @@ def test_team_schema(self): "title": "Event", "type": "object", }, - "Address_coqnj7_leaf": { + "Address_e4rhju_leaf": { "additionalProperties": False, "properties": { "city": {"maxLength": 64, "title": "City", "type": "string"}, "street": {"maxLength": 128, "title": "Street", "type": "string"}, + "m2mwitho2opks": { + "items": {"$ref": "#/$defs/M2mWithO2oPk_leajz6_leaf"}, + "title": "M2Mwitho2Opks", + "type": "array", + }, "event_id": { "maximum": 9223372036854775807, "minimum": -9223372036854775808, @@ -806,10 +891,25 @@ def test_team_schema(self): "type": "integer", }, }, - "required": ["city", "street", "event_id"], + "required": ["city", "street", "event_id", "m2mwitho2opks"], "title": "Address", "type": "object", }, + "M2mWithO2oPk_leajz6_leaf": { + "additionalProperties": False, + "properties": { + "id": { + "maximum": 2147483647, + "minimum": -2147483648, + "title": "Id", + "type": "integer", + }, + "name": {"maxLength": 64, "title": "Name", "type": "string"}, + }, + "required": ["id", "name"], + "title": "M2mWithO2oPk", + "type": "object", + }, "Reporter_fgnv33_leaf": { "additionalProperties": False, "description": "Whom is assigned as the reporter", @@ -874,7 +974,7 @@ def test_team_schema(self): "title": "Alias", }, "events": { - "items": {"$ref": "#/$defs/Event_n2kadx_leaf"}, + "items": {"$ref": "#/$defs/Event_lfs4vy_leaf"}, "title": "Events", "type": "array", }, @@ -918,6 +1018,7 @@ async def test_eventlist(self): "address": { "event_id": self.address.pk, "city": "Santa Monica", + "m2mwitho2opks": [], "street": "Ocean", }, }, @@ -970,7 +1071,12 @@ async def test_event(self): {"id": self.team1.id, "name": "Onesies", "alias": None}, {"id": self.team2.id, "name": "T-Shirts", "alias": None}, ], - "address": {"event_id": self.address.pk, "city": "Santa Monica", "street": "Ocean"}, + "address": { + "event_id": self.address.pk, + "city": "Santa Monica", + "m2mwitho2opks": [], + "street": "Ocean", + }, }, ) @@ -1004,6 +1110,7 @@ async def test_address(self): "alias": None, }, "event_id": self.address.event_id, + "m2mwitho2opks": [], }, ) @@ -1040,6 +1147,7 @@ async def test_tournament(self): "address": { "event_id": self.address.pk, "city": "Santa Monica", + "m2mwitho2opks": [], "street": "Ocean", }, }, @@ -1093,6 +1201,7 @@ async def test_team(self): "address": { "event_id": self.address.pk, "city": "Santa Monica", + "m2mwitho2opks": [], "street": "Ocean", }, }, diff --git a/tests/test_relations.py b/tests/test_relations.py index ed3d3a8c5..a134df539 100644 --- a/tests/test_relations.py +++ b/tests/test_relations.py @@ -6,6 +6,9 @@ Employee, Event, Extra, + M2mWithO2oPk, + Node, + O2oPkModelWithM2m, Pair, Reporter, Single, @@ -461,3 +464,20 @@ async def test_doublefk_filter_both_values_rel(self): self.assertRegex(query, self.join1_match) self.assertRegex(query, self.join2_match) self.assertEqual(result, [{"name": "middle", "left__name": "one", "right__name": "two"}]) + + async def test_many2many_field_with_o2o_fk(self): + tournament = await Tournament.create(name="t") + event = await Event.create(name="e", tournament=tournament) + address = await Address.create(city="c", street="s", event=event) + obj = await M2mWithO2oPk.create(name="m") + self.assertEqual(await obj.address.all(), []) + await obj.address.add(address) + self.assertEqual(await obj.address.all(), [address]) + + async def test_o2o_fk_model_with_m2m_field(self): + author = await Author.create(name="a") + obj = await O2oPkModelWithM2m.create(author=author) + node = await Node.create(name="n") + self.assertEqual(await obj.nodes.all(), []) + await obj.nodes.add(node) + self.assertEqual(await obj.nodes.all(), [node]) diff --git a/tests/testmodels.py b/tests/testmodels.py index 6386e5e50..541351411 100644 --- a/tests/testmodels.py +++ b/tests/testmodels.py @@ -151,6 +151,20 @@ class Address(Model): ) +class M2mWithO2oPk(Model): + name = fields.CharField(max_length=64) + address: fields.ManyToManyRelation["Address"] = fields.ManyToManyField("models.Address") + + +class O2oPkModelWithM2m(Model): + author: fields.OneToOneRelation[Author] = fields.OneToOneField( + "models.Author", + on_delete=fields.CASCADE, + primary_key=True, + ) + nodes: fields.ManyToManyRelation["Node"] = fields.ManyToManyField("models.Node") + + class Dest_null(Model): name = fields.CharField(max_length=64) diff --git a/tortoise/backends/base/schema_generator.py b/tortoise/backends/base/schema_generator.py index 00ecc5d29..60d7f8295 100644 --- a/tortoise/backends/base/schema_generator.py +++ b/tortoise/backends/base/schema_generator.py @@ -4,12 +4,16 @@ from tortoise.exceptions import ConfigurationError from tortoise.fields import JSONField, TextField, UUIDField +from tortoise.fields.relational import OneToOneFieldInstance from tortoise.indexes import Index if TYPE_CHECKING: # pragma: nocoverage from tortoise.backends.base.client import BaseDBAsyncClient - from tortoise.fields.relational import ForeignKeyFieldInstance # noqa - from tortoise.fields.relational import ManyToManyFieldInstance + from tortoise.fields import Field + from tortoise.fields.relational import ( + ForeignKeyFieldInstance, + ManyToManyFieldInstance, + ) from tortoise.models import Model # pylint: disable=R0201 @@ -186,6 +190,13 @@ def _get_unique_constraint_sql(self, model: "Type[Model]", field_names: List[str fields=", ".join([self.quote(f) for f in field_names]), ) + def _get_pk_field_sql_type(self, pk_field: "Field") -> str: + if isinstance(pk_field, OneToOneFieldInstance): + return self._get_pk_field_sql_type(pk_field.related_model._meta.pk) + if sql_type := pk_field.get_for_dialect(self.DIALECT, "SQL_TYPE"): + return sql_type + raise ConfigurationError(f"Can't get SQL type of {pk_field} for {self.DIALECT}") + def _get_table_sql(self, model: "Type[Model]", safe: bool = True) -> dict: fields_to_create = [] fields_with_index = [] @@ -380,17 +391,17 @@ def _get_table_sql(self, model: "Type[Model]", safe: bool = True) -> dict: ) exists = "IF NOT EXISTS " if safe else "" table_name = field_object.through + backward_type = self._get_pk_field_sql_type(model._meta.pk) + forward_type = self._get_pk_field_sql_type(field_object.related_model._meta.pk) m2m_create_string = self.M2M_TABLE_TEMPLATE.format( exists=exists, table_name=table_name, backward_fk=backward_fk, forward_fk=forward_fk, backward_key=backward_key, - backward_type=model._meta.pk.get_for_dialect(self.DIALECT, "SQL_TYPE"), + backward_type=backward_type, forward_key=forward_key, - forward_type=field_object.related_model._meta.pk.get_for_dialect( - self.DIALECT, "SQL_TYPE" - ), + forward_type=forward_type, extra=self._table_generate_extra(table=field_object.through), comment=( self._table_comment_generator( diff --git a/tortoise/models.py b/tortoise/models.py index ca5c98aa5..141185936 100644 --- a/tortoise/models.py +++ b/tortoise/models.py @@ -614,7 +614,9 @@ def __search_for_field_attributes(base: Type, attrs: dict) -> None: meta.pk_attr = pk_attr meta.pk = fields_map.get(pk_attr) # type: ignore if meta.pk: - meta.db_pk_column = meta.pk.source_field or meta.pk_attr + meta.db_pk_column = meta.pk.source_field or ( + meta.pk_attr + "_id" * isinstance(meta.pk, OneToOneFieldInstance) + ) meta._inited = False if not fields_map: meta.abstract = True From c12376349eef1b60b32bb9f098e3214a8a3e7f11 Mon Sep 17 00:00:00 2001 From: Waket Zheng Date: Mon, 25 Nov 2024 17:02:43 +0800 Subject: [PATCH 2/2] refactor: more readable --- tortoise/models.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tortoise/models.py b/tortoise/models.py index 141185936..7581ff060 100644 --- a/tortoise/models.py +++ b/tortoise/models.py @@ -614,9 +614,12 @@ def __search_for_field_attributes(base: Type, attrs: dict) -> None: meta.pk_attr = pk_attr meta.pk = fields_map.get(pk_attr) # type: ignore if meta.pk: - meta.db_pk_column = meta.pk.source_field or ( - meta.pk_attr + "_id" * isinstance(meta.pk, OneToOneFieldInstance) - ) + if meta.pk.source_field: + meta.db_pk_column = meta.pk.source_field + elif isinstance(meta.pk, OneToOneFieldInstance): + meta.db_pk_column = f"{meta.pk_attr}_id" + else: + meta.db_pk_column = meta.pk_attr meta._inited = False if not fields_map: meta.abstract = True