Be less fiscal regarding validators functions

Adjusted the check in the mapper for a callable object that is used as a
``@validates`` validator function or a ``@reconstructor`` reconstruction
function, to check for "callable" more liberally such as to accommodate
objects based on fundamental attributes like ``__func__`` and
``__call___``, rather than testing for ``MethodType`` / ``FunctionType``,
allowing things like cython functions to work properly. Pull request
courtesy Miłosz Stypiński.

Fixes: #6538
Closes: #6539
Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/6539
Pull-request-sha: ed1d7fe5c9386bab0416ff32095afc777c26b6ca

Change-Id: I8350558bc9a9ba58f43e48e12ce25a0b30e4d767

diff --git a/doc/build/changelog/unreleased_14/6538.rst b/doc/build/changelog/unreleased_14/6538.rst
new file mode 100644 (file)
index 0000000..f50b592
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/6538.rst
@@ -0,0 +1,11 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 6538
+
+    Adjusted the check in the mapper for a callable object that is used as a
+    ``@validates`` validator function or a ``@reconstructor`` reconstruction
+    function, to check for "callable" more liberally such as to accommodate
+    objects based on fundamental attributes like ``__func__`` and
+    ``__call___``, rather than testing for ``MethodType`` / ``FunctionType``,
+    allowing things like cython functions to work properly. Pull request
+    courtesy Miłosz Stypiński.

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 378c6527856404137cb2cde165afa8c24d5335ee..bab2eb6b9aee79dff70c0f917301df78c1c62fa2 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -19,7 +19,6 @@ from __future__ import absolute_import
 from collections import deque
 from itertools import chain
 import sys
-import types
 import weakref
 
 from . import attributes
@@ -1260,9 +1259,9 @@ class Mapper(
         for key, method in util.iterate_attributes(self.class_):
             if key == "__init__" and hasattr(method, "_sa_original_init"):
                 method = method._sa_original_init
-                if isinstance(method, types.MethodType):
+                if hasattr(method, "__func__"):
                     method = method.__func__
-            if isinstance(method, types.FunctionType):
+            if callable(method):
                 if hasattr(method, "__sa_reconstructor__"):
                     self._reconstructor = method
                     event.listen(manager, "load", _event_on_load, raw=True)

diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py
index b9164111b7ea1f1944fc6b8ff1cbbda363026f52..d8da3a1f634c80c1eceb71202ef6ab060d8271bd 100644 (file)
--- a/test/orm/test_mapper.py
+++ b/test/orm/test_mapper.py
@@ -1928,6 +1928,65 @@ class MapperTest(_fixtures.FixtureTest, AssertsCompiledSQL):
         fixture_session().query(User).first()
         eq_(recon, ["go"])
 
+    def test_reconstructor_init_callable_object(self):
+        """test #6538"""
+
+        users = self.tables.users
+
+        recon = []
+
+        class recon_obj(object):
+            def __call__(self, obj):
+                recon.append("go")
+
+        class User(object):
+            __init__ = reconstructor(recon_obj())
+
+        self.mapper(User, users)
+
+        User()
+        eq_(recon, ["go"])
+
+        recon[:] = []
+        fixture_session().query(User).first()
+        eq_(recon, ["go"])
+
+    def test_reconstructor_init_simulate_cython(self):
+        """test #6538
+
+        This test is pretty contrived in order to ensure we aren't using
+        ``isinstance(obj, MethodType)`` within the mapper.
+
+        """
+
+        users = self.tables.users
+
+        recon = []
+
+        class recon_obj(object):
+            def __call__(self, obj):
+                recon.append("go")
+
+            __sa_reconstructor__ = True
+
+        class recon_meth(object):
+            __func__ = recon_obj()
+
+            def __call__(self, *arg, **kw):
+                return self.__func__.__call__(*arg, **kw)
+
+        class User(object):
+            __init__ = recon_meth()
+
+        self.mapper(User, users)
+
+        User()
+        eq_(recon, ["go"])
+
+        recon[:] = []
+        fixture_session().query(User).first()
+        eq_(recon, ["go"])
+
     def test_reconstructor_init_inheritance(self):
         users = self.tables.users
 

diff --git a/test/orm/test_validators.py b/test/orm/test_validators.py
index 887ff7754a3e6e8aa702b980ed413c32b201aef9..0254aa24d7c12e4dbf64bdbbd15dc0e219e1e1ee 100644 (file)
--- a/test/orm/test_validators.py
+++ b/test/orm/test_validators.py
@@ -246,6 +246,24 @@ class ValidatorTest(_fixtures.FixtureTest):
             },
         )
 
+    def test_validator_as_callable_object(self):
+        """test #6538"""
+        users = self.tables.users
+        canary = Mock()
+
+        class SomeValidator(object):
+            def __call__(self, obj, key, name):
+                canary(key, name)
+                ne_(name, "fred")
+                return name + " modified"
+
+        class User(fixtures.ComparableEntity):
+            sv = validates("name")(SomeValidator())
+
+        mapper(User, users)
+        u1 = User(name="ed")
+        eq_(u1.name, "ed modified")
+
     def test_validator_multi_warning(self):
         users = self.tables.users