From dd5d6d15467b66398dd328ff43b863a057899291 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Thu, 3 Jun 2021 09:37:27 -0400 Subject: [PATCH] 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 --- doc/build/changelog/unreleased_14/6285.rst | 10 ++++ lib/sqlalchemy/util/langhelpers.py | 20 +++++++- test/base/test_utils.py | 22 +++++++++ test/orm/test_scoping.py | 54 +++++++++++++++++++++- 4 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 doc/build/changelog/unreleased_14/6285.rst diff --git a/doc/build/changelog/unreleased_14/6285.rst b/doc/build/changelog/unreleased_14/6285.rst new file mode 100644 index 0000000000..7debb5ca1a --- /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 7796d41065..1308ee7e06 100644 --- 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 b602811ab0..849c193eac 100644 --- 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 60ad0285f4..ad4ab55a28 100644 --- 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) -- 2.47.2