From dba885ab1149eb85b8664ab42829243b57734bbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mari=C3=A1n=20Skrip?= Date: Wed, 7 Aug 2024 14:37:21 +0200 Subject: [PATCH 1/3] Fix instance of related object added to session on validation Previously, when validating instance when a session was open and the model instance had a related object a new instance of this related object was created and added to the session. --- sqlmodel/_compat.py | 2 +- tests/test_validation.py | 73 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/sqlmodel/_compat.py b/sqlmodel/_compat.py index 4018d1bb39..4f64d661ec 100644 --- a/sqlmodel/_compat.py +++ b/sqlmodel/_compat.py @@ -335,7 +335,7 @@ def sqlmodel_validate( for key in new_obj.__sqlmodel_relationships__: value = getattr(use_obj, key, Undefined) if value is not Undefined: - setattr(new_obj, key, value) + new_obj.__dict__[key] = value return new_obj def sqlmodel_init(*, self: "SQLModel", data: Dict[str, Any]) -> None: diff --git a/tests/test_validation.py b/tests/test_validation.py index 3265922070..e74179302c 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -1,8 +1,9 @@ -from typing import Optional +from typing import List, Optional import pytest from pydantic.error_wrappers import ValidationError -from sqlmodel import SQLModel +from sqlmodel import Session, SQLModel, create_engine +from sqlmodel.main import Field, Relationship from .conftest import needs_pydanticv1, needs_pydanticv2 @@ -63,3 +64,71 @@ def reject_none(cls, v): with pytest.raises(ValidationError): Hero.model_validate({"name": None, "age": 25}) + + +@needs_pydanticv1 +def test_validation_related_object_not_in_session_pydantic_v1(clear_sqlmodel): + class Team(SQLModel, table=True): + id: Optional[int] = Field(default=None, primary_key=True) + name: str + heroes: List["Hero"] = Relationship(back_populates="team") + + class Hero(SQLModel, table=True): + id: Optional[int] = Field(default=None, primary_key=True) + name: str + + team_id: Optional[int] = Field(default=None, foreign_key="team.id") + team: Optional[Team] = Relationship(back_populates="heroes") + + engine = create_engine("sqlite://") + SQLModel.metadata.create_all(engine) + team = Team(name="team") + hero = Hero(name="hero", team=team) + with Session(engine) as session: + session.add(team) + session.add(hero) + session.commit() + + with Session(engine) as session: + hero = session.get(Hero, 1) + assert not session.dirty + assert not session.new + + Hero.validate(hero) + + assert not session.dirty + assert not session.new + + +@needs_pydanticv2 +def test_validation_related_object_not_in_session_pydantic_v2(clear_sqlmodel): + class Team(SQLModel, table=True): + id: Optional[int] = Field(default=None, primary_key=True) + name: str + heroes: List["Hero"] = Relationship(back_populates="team") + + class Hero(SQLModel, table=True): + id: Optional[int] = Field(default=None, primary_key=True) + name: str + + team_id: Optional[int] = Field(default=None, foreign_key="team.id") + team: Optional[Team] = Relationship(back_populates="heroes") + + engine = create_engine("sqlite://") + SQLModel.metadata.create_all(engine) + team = Team(name="team") + hero = Hero(name="hero", team=team) + with Session(engine) as session: + session.add(team) + session.add(hero) + session.commit() + + with Session(engine) as session: + hero = session.get(Hero, 1) + assert not session.dirty + assert not session.new + + Hero.model_validate(hero) + + assert not session.dirty + assert not session.new From ad51506fb0429c2838e53fcb777d4a15b24287e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mari=C3=A1n=20Skrip?= Date: Wed, 7 Aug 2024 16:05:54 +0200 Subject: [PATCH 2/3] Use better assertion in test --- tests/test_validation.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/test_validation.py b/tests/test_validation.py index e74179302c..b930554670 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -125,10 +125,8 @@ class Hero(SQLModel, table=True): with Session(engine) as session: hero = session.get(Hero, 1) - assert not session.dirty - assert not session.new + assert session._is_clean() Hero.model_validate(hero) - assert not session.dirty - assert not session.new + assert session._is_clean() From 29ee43de4ed44ed380a25dedb668be30e845c5d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mari=C3=A1n=20Skrip?= Date: Thu, 8 Aug 2024 09:46:53 +0200 Subject: [PATCH 3/3] Add stronger assertion that the related object is the same --- tests/test_validation.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/test_validation.py b/tests/test_validation.py index b930554670..92f57c5d5e 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -91,13 +91,14 @@ class Hero(SQLModel, table=True): with Session(engine) as session: hero = session.get(Hero, 1) - assert not session.dirty - assert not session.new + assert session._is_clean() - Hero.validate(hero) + new_hero = Hero.validate(hero) - assert not session.dirty - assert not session.new + assert session._is_clean() + # The new hero is a different instance, but the team is the same + assert id(new_hero) != id(hero) + assert id(new_hero.team) == id(hero.team) @needs_pydanticv2 @@ -127,6 +128,9 @@ class Hero(SQLModel, table=True): hero = session.get(Hero, 1) assert session._is_clean() - Hero.model_validate(hero) + new_hero = Hero.model_validate(hero) assert session._is_clean() + # The new hero is a different instance, but the team is the same + assert id(new_hero) != id(hero) + assert id(new_hero.team) == id(hero.team)