From 0f8721fa52e335ab2abeb548c8914b99a8c5e1fd Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 12 May 2017 10:53:54 -0400 Subject: [PATCH] Cascade mappers in terms of the instance's mapper Fixed a (extremely old) bug in cascade_mappers where the first cascade we do is against the "self" mapper, and not the one that actually corresponds to the state given. These are different in the case where we start with a relationship to a class, and the instance is of a subclass, which itself can have relationships that aren't on the base mapper. A pretty severe bug that somehow has avoided the radar since the beginning. Change-Id: I512956b9757b07e06f3ca1ccb507a33fb10bed31 Fixes: #3986 --- doc/build/changelog/changelog_11.rst | 10 ++ lib/sqlalchemy/orm/mapper.py | 14 ++- test/orm/test_cascade.py | 139 +++++++++++++++++++++++++++ 3 files changed, 159 insertions(+), 4 deletions(-) diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst index c591b483f7..523b66f82e 100644 --- a/doc/build/changelog/changelog_11.rst +++ b/doc/build/changelog/changelog_11.rst @@ -21,6 +21,16 @@ .. changelog:: :version: 1.1.10 + .. change:: 3986 + :tags: bug, orm + :versions: 1.2.0b1 + :tickets: 3986 + + Fixed bug where a cascade such as "delete-orphan" (but others as well) + would fail to locate an object linked to a relationship that itself + is local to a subclass in an inheritance relationship, thus causing + the operation to not take place. + .. change:: 3975 :tags: bug, oracle :versions: 1.2.0b1 diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 3fdba44825..61a9290c4c 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -2689,7 +2689,9 @@ class Mapper(InspectionAttr): visited_states = set() prp, mpp = object(), object() - visitables = deque([(deque(self._props.values()), prp, + assert state.mapper.isa(self) + + visitables = deque([(deque(state.mapper._props.values()), prp, state, state.dict)]) while visitables: @@ -2712,9 +2714,13 @@ class Mapper(InspectionAttr): corresponding_dict = iterator.popleft() yield instance, instance_mapper, \ corresponding_state, corresponding_dict - visitables.append((deque(instance_mapper._props.values()), - prp, corresponding_state, - corresponding_dict)) + visitables.append( + ( + deque(instance_mapper._props.values()), + prp, corresponding_state, + corresponding_dict + ) + ) @_memoized_configured_property def _compiled_cache(self): diff --git a/test/orm/test_cascade.py b/test/orm/test_cascade.py index f660321c24..55467db4b9 100644 --- a/test/orm/test_cascade.py +++ b/test/orm/test_cascade.py @@ -3067,3 +3067,142 @@ class PartialFlushTest(fixtures.MappedTest): assert p1 in sess.new assert c1 not in sess.new assert c2 in sess.new + + +class SubclassCascadeTest(fixtures.DeclarativeMappedTest): + @classmethod + def setup_classes(cls): + Base = cls.DeclarativeBasic + + class Company(Base): + __tablename__ = 'company' + id = Column(Integer, primary_key=True) + name = Column(String(50)) + employees = relationship("Employee", cascade="all, delete-orphan") + + class Employee(Base): + __tablename__ = 'employee' + id = Column(Integer, primary_key=True) + name = Column(String(50)) + type = Column(String(50)) + company_id = Column(ForeignKey('company.id')) + + __mapper_args__ = { + 'polymorphic_identity': 'employee', + 'polymorphic_on': type + } + + class Engineer(Employee): + __tablename__ = 'engineer' + id = Column(Integer, ForeignKey('employee.id'), primary_key=True) + engineer_name = Column(String(30)) + languages = relationship("Language", cascade="all, delete-orphan") + + __mapper_args__ = { + 'polymorphic_identity': 'engineer', + } + + class MavenBuild(Base): + __tablename__ = 'maven_build' + id = Column(Integer, primary_key=True) + java_language_id = Column( + ForeignKey('java_language.id'), nullable=False) + + class Manager(Employee): + __tablename__ = 'manager' + id = Column(Integer, ForeignKey('employee.id'), primary_key=True) + manager_name = Column(String(30)) + + __mapper_args__ = { + 'polymorphic_identity': 'manager', + } + + class Language(Base): + __tablename__ = 'language' + id = Column(Integer, primary_key=True) + engineer_id = Column(ForeignKey('engineer.id'), nullable=False) + name = Column(String(50)) + type = Column(String(50)) + + __mapper_args__ = { + "polymorphic_on": type, + "polymorphic_identity": "language" + } + + class JavaLanguage(Language): + __tablename__ = 'java_language' + id = Column(ForeignKey('language.id'), primary_key=True) + maven_builds = relationship("MavenBuild", + cascade="all, delete-orphan") + + __mapper_args__ = { + "polymorphic_identity": "java_language" + } + + def test_cascade_iterator_polymorphic(self): + Company, Employee, Engineer, Language, JavaLanguage, MavenBuild = \ + self.classes( + 'Company', 'Employee', 'Engineer', 'Language', 'JavaLanguage', + 'MavenBuild' + ) + + obj = Company( + employees=[ + Engineer( + languages=[ + JavaLanguage( + name="java", + maven_builds=[MavenBuild()] + ) + ], + + ) + ] + ) + eng = obj.employees[0] + lang = eng.languages[0] + maven_build = lang.maven_builds[0] + + from sqlalchemy import inspect + state = inspect(obj) + it = inspect(Company).cascade_iterator("save-update", state) + eq_( + set([rec[0] for rec in it]), + set([eng, maven_build, lang]) + ) + + state = inspect(eng) + it = inspect(Employee).cascade_iterator("save-update", state) + eq_( + set([rec[0] for rec in it]), + set([maven_build, lang]) + ) + + def test_delete_orphan_round_trip(self): + Company, Employee, Engineer, Language, JavaLanguage, \ + MavenBuild = self.classes( + 'Company', 'Employee', 'Engineer', 'Language', 'JavaLanguage', + 'MavenBuild' + ) + + obj = Company( + employees=[ + Engineer( + languages=[ + JavaLanguage( + name="java", + maven_builds=[MavenBuild()] + ) + ], + + ) + ] + ) + s = Session() + s.add(obj) + s.commit() + + obj.employees = [] + s.commit() + + eq_(s.query(Language).count(), 0) \ No newline at end of file -- 2.39.5