Ensure extended instrumentation is fully disposed

Fixed regression in the ``sqlalchemy.ext.instrumentation`` extension that
prevented instrumentation disposal from working completely. This fix
includes both a 1.4 regression fix as well as a fix for a related issue
that existed in 1.3 also.   As part of this change, the
:class:`sqlalchemy.ext.instrumentation.InstrumentationManager` class now
has a new method ``unregister()``, which replaces the previous method
``dispose()``, which was not called as of version 1.4.

Fixes: #6390
Change-Id: I1b7a0f190b56d31b93b5ba11f67dc9f73889958b

diff --git a/doc/build/changelog/unreleased_14/6390.rst b/doc/build/changelog/unreleased_14/6390.rst
new file mode 100644 (file)
index 0000000..6c38abb
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/6390.rst
@@ -0,0 +1,12 @@
+.. change::
+    :tags: bug, regression, ext
+    :tickets: 6390
+
+    Fixed regression in the ``sqlalchemy.ext.instrumentation`` extension that
+    prevented instrumentation disposal from working completely. This fix
+    includes both a 1.4 regression fix as well as a fix for a related issue
+    that existed in 1.3 also.   As part of this change, the
+    :class:`sqlalchemy.ext.instrumentation.InstrumentationManager` class now
+    has a new method ``unregister()``, which replaces the previous method
+    ``dispose()``, which was not called as of version 1.4.
+

diff --git a/lib/sqlalchemy/ext/instrumentation.py b/lib/sqlalchemy/ext/instrumentation.py
index 300d0d81cfa4199ce6c85a856a2c665cc55b5dea..54f3e64c5d519dc91b2cbd118188e401bc2c3bb4 100644 (file)
--- a/lib/sqlalchemy/ext/instrumentation.py
+++ b/lib/sqlalchemy/ext/instrumentation.py
@@ -155,11 +155,11 @@ class ExtendedInstrumentationRegistry(InstrumentationFactory):
         return factories
 
     def unregister(self, class_):
+        super(ExtendedInstrumentationRegistry, self).unregister(class_)
         if class_ in self._manager_finders:
             del self._manager_finders[class_]
             del self._state_finders[class_]
             del self._dict_finders[class_]
-        super(ExtendedInstrumentationRegistry, self).unregister(class_)
 
     def manager_of_class(self, cls):
         if cls is None:
@@ -220,7 +220,7 @@ class InstrumentationManager(object):
     def manage(self, class_, manager):
         setattr(class_, "_default_class_manager", manager)
 
-    def dispose(self, class_, manager):
+    def unregister(self, class_, manager):
         delattr(class_, "_default_class_manager")
 
     def manager_getter(self, class_):
@@ -282,8 +282,8 @@ class _ClassInstrumentationAdapter(ClassManager):
     def manage(self):
         self._adapted.manage(self.class_, self)
 
-    def dispose(self):
-        self._adapted.dispose(self.class_)
+    def unregister(self):
+        self._adapted.unregister(self.class_, self)
 
     def manager_getter(self):
         return self._adapted.manager_getter(self.class_)

diff --git a/test/ext/test_extendedattr.py b/test/ext/test_extendedattr.py
index f3eceb0dca5f2ef6d8512fba2480eeb0989103c8..c762754bc58f4e885bfc4bdb26c04d74fe86d650 100644 (file)
--- a/test/ext/test_extendedattr.py
+++ b/test/ext/test_extendedattr.py
@@ -1,5 +1,8 @@
 import sqlalchemy as sa
+from sqlalchemy import Column
 from sqlalchemy import event
+from sqlalchemy import Integer
+from sqlalchemy import Table
 from sqlalchemy import util
 from sqlalchemy.ext import instrumentation
 from sqlalchemy.orm import attributes
@@ -16,6 +19,8 @@ from sqlalchemy.testing import assert_raises
 from sqlalchemy.testing import assert_raises_message
 from sqlalchemy.testing import eq_
 from sqlalchemy.testing import fixtures
+from sqlalchemy.testing import is_
+from sqlalchemy.testing import is_not
 from sqlalchemy.testing import ne_
 from sqlalchemy.testing.util import decorator
 
@@ -87,6 +92,48 @@ class MyListLike(list):
 MyBaseClass, MyClass = None, None
 
 
+class DisposeTest(_ExtBase, fixtures.TestBase):
+    def test_unregister(self, registry):
+        class MyClassState(instrumentation.InstrumentationManager):
+            def manage(self, class_, manager):
+                setattr(class_, "xyz", manager)
+
+            def unregister(self, class_, manager):
+                delattr(class_, "xyz")
+
+            def manager_getter(self, class_):
+                def get(cls):
+                    return cls.xyz
+
+                return get
+
+        class MyClass(object):
+            __sa_instrumentation_manager__ = MyClassState
+
+        assert attributes.manager_of_class(MyClass) is None
+
+        t = Table(
+            "my_table",
+            registry.metadata,
+            Column("id", Integer, primary_key=True),
+        )
+
+        registry.map_imperatively(MyClass, t)
+
+        manager = attributes.manager_of_class(MyClass)
+        is_not(manager, None)
+        is_(manager, MyClass.xyz)
+
+        registry.configure()
+
+        registry.dispose()
+
+        manager = attributes.manager_of_class(MyClass)
+        is_(manager, None)
+
+        assert not hasattr(MyClass, "xyz")
+
+
 class UserDefinedExtensionTest(_ExtBase, fixtures.ORMTest):
     @classmethod
     def setup_test_class(cls):