Adjust create_proxy_methods() to use kw arguments

Adjusted the means by which classes such as :class:`_orm.scoped_session`
and :class:`_asyncio.AsyncSession` are generated from the base
:class:`_orm.Session` class, such that custom :class:`_orm.Session`
subclasses such as that used by Flask-SQLAlchemy don't need to implement
positional arguments when they call into the superclass method, and can
continue using the same argument styles as in previous releases.

Fixes: #6285
References: https://github.com/pallets/flask-sqlalchemy/issues/953
Change-Id: I8612ab33743625e70eb158efceb0636d783c92a5

diff --git a/doc/build/changelog/unreleased_14/6285.rst b/doc/build/changelog/unreleased_14/6285.rst
new file mode 100644 (file)
index 0000000..7debb5c
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/6285.rst
@@ -0,0 +1,10 @@
+.. change::
+    :tags: bug, orm, regression
+    :tickets: 6285
+
+    Adjusted the means by which classes such as :class:`_orm.scoped_session`
+    and :class:`_asyncio.AsyncSession` are generated from the base
+    :class:`_orm.Session` class, such that custom :class:`_orm.Session`
+    subclasses such as that used by Flask-SQLAlchemy don't need to implement
+    positional arguments when they call into the superclass method, and can
+    continue using the same argument styles as in previous releases.

diff --git a/lib/sqlalchemy/util/langhelpers.py b/lib/sqlalchemy/util/langhelpers.py
index 7796d4106581fa25cd331ad5dd6c4008762c57cb..1308ee7e0619c9f69ae02434890f98e9ca3e5ed0 100644 (file)
--- a/lib/sqlalchemy/util/langhelpers.py
+++ b/lib/sqlalchemy/util/langhelpers.py
@@ -562,6 +562,18 @@ def format_argspec_plus(fn, grouped=True):
         defaulted_vals,
         formatvalue=lambda x: "=" + x,
     )
+
+    if spec[0]:
+        apply_kw_proxied = compat.inspect_formatargspec(
+            name_args[1:],
+            spec[1],
+            spec[2],
+            defaulted_vals,
+            formatvalue=lambda x: "=" + x,
+        )
+    else:
+        apply_kw_proxied = apply_kw
+
     if grouped:
         return dict(
             args=args,
@@ -569,6 +581,7 @@ def format_argspec_plus(fn, grouped=True):
             apply_pos=apply_pos,
             apply_kw=apply_kw,
             apply_pos_proxied=apply_pos_proxied,
+            apply_kw_proxied=apply_kw_proxied,
         )
     else:
         return dict(
@@ -577,6 +590,7 @@ def format_argspec_plus(fn, grouped=True):
             apply_pos=apply_pos[1:-1],
             apply_kw=apply_kw[1:-1],
             apply_pos_proxied=apply_pos_proxied[1:-1],
+            apply_kw_proxied=apply_kw_proxied[1:-1],
         )
 
 
@@ -609,6 +623,7 @@ def format_argspec_init(method, grouped=True):
         apply_pos=args,
         apply_kw=args,
         apply_pos_proxied=proxied,
+        apply_kw_proxied=proxied,
     )
 
 
@@ -638,6 +653,7 @@ def create_proxy_methods(
             metadata = {
                 "name": fn.__name__,
                 "apply_pos_proxied": caller_argspec["apply_pos_proxied"],
+                "apply_kw_proxied": caller_argspec["apply_kw_proxied"],
                 "args": caller_argspec["args"],
                 "self_arg": caller_argspec["self_arg"],
             }
@@ -645,14 +661,14 @@ def create_proxy_methods(
             if clslevel:
                 code = (
                     "def %(name)s(%(args)s):\n"
-                    "    return target_cls.%(name)s(%(apply_pos_proxied)s)"
+                    "    return target_cls.%(name)s(%(apply_kw_proxied)s)"
                     % metadata
                 )
                 env["target_cls"] = target_cls
             else:
                 code = (
                     "def %(name)s(%(args)s):\n"
-                    "    return %(self_arg)s._proxied.%(name)s(%(apply_pos_proxied)s)"  # noqa E501
+                    "    return %(self_arg)s._proxied.%(name)s(%(apply_kw_proxied)s)"  # noqa E501
                     % metadata
                 )
 

diff --git a/test/base/test_utils.py b/test/base/test_utils.py
index b602811ab09a173fdb6eebd44f4c79c1788696f6..849c193eac3f8f7699a86ac587c0d2303bd184b2 100644 (file)
--- a/test/base/test_utils.py
+++ b/test/base/test_utils.py
@@ -2411,6 +2411,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
                 "apply_kw": "()",
                 "apply_pos": "()",
                 "apply_pos_proxied": "()",
+                "apply_kw_proxied": "()",
             },
             True,
         ),
@@ -2422,6 +2423,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
                 "apply_kw": "",
                 "apply_pos": "",
                 "apply_pos_proxied": "",
+                "apply_kw_proxied": "",
             },
             False,
         ),
@@ -2433,6 +2435,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
                 "apply_kw": "(self)",
                 "apply_pos": "(self)",
                 "apply_pos_proxied": "()",
+                "apply_kw_proxied": "()",
             },
             True,
         ),
@@ -2444,6 +2447,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
                 "apply_kw": "self",
                 "apply_pos": "self",
                 "apply_pos_proxied": "",
+                "apply_kw_proxied": "",
             },
             False,
         ),
@@ -2455,6 +2459,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
                 "apply_kw": "(*a)",
                 "apply_pos": "(*a)",
                 "apply_pos_proxied": "(*a)",
+                "apply_kw_proxied": "(*a)",
             },
             True,
         ),
@@ -2466,6 +2471,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
                 "apply_kw": "(**kw)",
                 "apply_pos": "(**kw)",
                 "apply_pos_proxied": "(**kw)",
+                "apply_kw_proxied": "(**kw)",
             },
             True,
         ),
@@ -2477,6 +2483,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
                 "apply_kw": "(*a, **kw)",
                 "apply_pos": "(*a, **kw)",
                 "apply_pos_proxied": "(*a, **kw)",
+                "apply_kw_proxied": "(*a, **kw)",
             },
             True,
         ),
@@ -2488,6 +2495,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
                 "apply_kw": "(a, *b)",
                 "apply_pos": "(a, *b)",
                 "apply_pos_proxied": "(*b)",
+                "apply_kw_proxied": "(*b)",
             },
             True,
         ),
@@ -2499,6 +2507,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
                 "apply_kw": "(a, **b)",
                 "apply_pos": "(a, **b)",
                 "apply_pos_proxied": "(**b)",
+                "apply_kw_proxied": "(**b)",
             },
             True,
         ),
@@ -2510,6 +2519,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
                 "apply_kw": "(a, *b, **c)",
                 "apply_pos": "(a, *b, **c)",
                 "apply_pos_proxied": "(*b, **c)",
+                "apply_kw_proxied": "(*b, **c)",
             },
             True,
         ),
@@ -2521,6 +2531,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
                 "apply_kw": "(a, b=b, **c)",
                 "apply_pos": "(a, b, **c)",
                 "apply_pos_proxied": "(b, **c)",
+                "apply_kw_proxied": "(b=b, **c)",
             },
             True,
         ),
@@ -2532,6 +2543,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
                 "apply_kw": "(a=a, b=b)",
                 "apply_pos": "(a, b)",
                 "apply_pos_proxied": "(b)",
+                "apply_kw_proxied": "(b=b)",
             },
             True,
         ),
@@ -2543,6 +2555,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
                 "apply_kw": "a=a, b=b",
                 "apply_pos": "a, b",
                 "apply_pos_proxied": "b",
+                "apply_kw_proxied": "b=b",
             },
             False,
         ),
@@ -2554,6 +2567,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
                 "apply_pos": "self, a, *, b, c",
                 "apply_kw": "self, a, b=b, c=c",
                 "apply_pos_proxied": "a, *, b, c",
+                "apply_kw_proxied": "a, b=b, c=c",
             },
             False,
             testing.requires.python3,
@@ -2566,6 +2580,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
                 "apply_pos": "self, a, *args, b, c",
                 "apply_kw": "self, a, b=b, c=c, *args",
                 "apply_pos_proxied": "a, *args, b, c",
+                "apply_kw_proxied": "a, b=b, c=c, *args",
             },
             False,
             testing.requires.python3,
@@ -2578,6 +2593,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
                 "apply_pos": "self, a, *, b, c",
                 "apply_kw": "self, a, b=b, c=c",
                 "apply_pos_proxied": "a, *, b, c",
+                "apply_kw_proxied": "a, b=b, c=c",
             },
             False,
             testing.requires.python3,
@@ -2609,6 +2625,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
             "apply_pos": "(self)",
             "apply_kw": "(self)",
             "apply_pos_proxied": "()",
+            "apply_kw_proxied": "()",
         }
         wrapper_spec = {
             "args": "(self, *args, **kwargs)",
@@ -2616,12 +2633,14 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
             "apply_pos": "(self, *args, **kwargs)",
             "apply_kw": "(self, *args, **kwargs)",
             "apply_pos_proxied": "(*args, **kwargs)",
+            "apply_kw_proxied": "(*args, **kwargs)",
         }
         custom_spec = {
             "args": "(slef, a=123)",
             "self_arg": "slef",  # yes, slef
             "apply_pos": "(slef, a)",
             "apply_pos_proxied": "(a)",
+            "apply_kw_proxied": "(a=a)",
             "apply_kw": "(slef, a=a)",
         }
 
@@ -2636,6 +2655,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
             "apply_pos": "self",
             "apply_kw": "self",
             "apply_pos_proxied": "",
+            "apply_kw_proxied": "",
         }
         wrapper_spec = {
             "args": "self, *args, **kwargs",
@@ -2643,6 +2663,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
             "apply_pos": "self, *args, **kwargs",
             "apply_kw": "self, *args, **kwargs",
             "apply_pos_proxied": "*args, **kwargs",
+            "apply_kw_proxied": "*args, **kwargs",
         }
         custom_spec = {
             "args": "slef, a=123",
@@ -2650,6 +2671,7 @@ class TestFormatArgspec(_Py3KFixtures, fixtures.TestBase):
             "apply_pos": "slef, a",
             "apply_kw": "slef, a=a",
             "apply_pos_proxied": "a",
+            "apply_kw_proxied": "a=a",
         }
 
         self._test_init(False, object_spec, wrapper_spec, custom_spec)

diff --git a/test/orm/test_scoping.py b/test/orm/test_scoping.py
index 60ad0285f444bac6d6e5c0976379d2873605cf7d..ad4ab55a284b07d3a38f9c9097a26bf4e97ecda2 100644 (file)
--- a/test/orm/test_scoping.py
+++ b/test/orm/test_scoping.py
@@ -7,9 +7,12 @@ from sqlalchemy.orm import mapper
 from sqlalchemy.orm import query
 from sqlalchemy.orm import relationship
 from sqlalchemy.orm import scoped_session
+from sqlalchemy.orm import Session
+from sqlalchemy.orm import sessionmaker
 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 mock
 from sqlalchemy.testing.mock import Mock
 from sqlalchemy.testing.schema import Column
@@ -145,10 +148,15 @@ class ScopedSessionTest(fixtures.MappedTest):
         eq_(
             mock_session.mock_calls,
             [
-                mock.call.add("add", True),
+                mock.call.add("add", _warn=True),
                 mock.call.delete("delete"),
                 mock.call.get(
-                    "Cls", 5, mock.ANY, mock.ANY, mock.ANY, mock.ANY
+                    "Cls",
+                    5,
+                    options=None,
+                    populate_existing=False,
+                    with_for_update=None,
+                    identity_token=None,
                 ),
             ],
         )
@@ -159,3 +167,45 @@ class ScopedSessionTest(fixtures.MappedTest):
             sess.object_session("foo")
 
         eq_(mock_object_session.mock_calls, [mock.call("foo")])
+
+    @testing.combinations(
+        ("style1", testing.requires.python3),
+        ("style2", testing.requires.python3),
+        "style3",
+        "style4",
+    )
+    def test_get_bind_custom_session_subclass(self, style):
+        """test #6285"""
+
+        class MySession(Session):
+            if style == "style1":
+
+                def get_bind(self, mapper=None, **kwargs):
+                    return super().get_bind(mapper=mapper, **kwargs)
+
+            elif style == "style2":
+                # this was the workaround for #6285, ensure it continues
+                # working as well
+                def get_bind(self, mapper=None, *args, **kwargs):
+                    return super().get_bind(mapper, *args, **kwargs)
+
+            elif style == "style3":
+                # py2k style
+                def get_bind(self, mapper=None, *args, **kwargs):
+                    return super(MySession, self).get_bind(
+                        mapper, *args, **kwargs
+                    )
+
+            elif style == "style4":
+                # py2k style
+                def get_bind(self, mapper=None, **kwargs):
+                    return super(MySession, self).get_bind(
+                        mapper=mapper, **kwargs
+                    )
+
+        s1 = MySession(testing.db)
+        is_(s1.get_bind(), testing.db)
+
+        ss = scoped_session(sessionmaker(testing.db, class_=MySession))
+
+        is_(ss.get_bind(), testing.db)