From: Daniele Varrazzo Date: Wed, 10 Sep 2025 10:07:46 +0000 (+0200) Subject: fix: don't raise a warning using a builtin for JSON dumps/loads X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=21eb5ca44a9d20eaee88ece5d947deb1813b8189;p=thirdparty%2Fpsycopg.git fix: don't raise a warning using a builtin for JSON dumps/loads These types are stable so will not leak. However they have no code/closure dunder attr so they will raise a warning. Fix #1165. --- diff --git a/docs/news.rst b/docs/news.rst index db427d9ef..563939a07 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -7,6 +7,16 @@ ``psycopg`` release notes ========================= +Future releases +--------------- + +Psycopg 3.2.11 (unreleased) +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +- Don't raise warning, and don't leak resources, if a builtin function is used + as JSON dumper/loader function (:ticket:`#1165`). + + Current release --------------- diff --git a/psycopg/psycopg/types/json.py b/psycopg/psycopg/types/json.py index 83ce719fa..c99a4a00d 100644 --- a/psycopg/psycopg/types/json.py +++ b/psycopg/psycopg/types/json.py @@ -8,7 +8,7 @@ from __future__ import annotations import json import logging -from types import CodeType # noqa[F401] +from types import BuiltinFunctionType, CodeType # noqa[F401] from typing import Any, Callable from warnings import warn from threading import Lock @@ -22,7 +22,7 @@ from .._compat import TypeAlias JsonDumpsFunction: TypeAlias = Callable[[Any], "str | bytes"] JsonLoadsFunction: TypeAlias = Callable[["str | bytes"], Any] -_AdapterKey: TypeAlias = "tuple[type, CodeType]" +_AdapterKey: TypeAlias = "tuple[type, CodeType | BuiltinFunctionType | type]" logger = logging.getLogger("psycopg") @@ -155,8 +155,12 @@ def _get_adapter_key(t: type, f: Callable[..., Any]) -> _AdapterKey | None: the same if a lambda if defined in a function, so we can use it as a more stable hash key. """ + # A builtin is stable and has no cache so it's a good candidate as hash key. + if isinstance(f, (BuiltinFunctionType, type)): + return (t, f) + # Check if there's an unexpected Python implementation that doesn't define - # these dunder attributes. If thta's the case, raise a warning, which will + # these dunder attributes. If that's the case, raise a warning, which will # crash our test suite and/or hopefully will be detected by the user. try: f.__code__ diff --git a/tests/types/test_json.py b/tests/types/test_json.py index 2c670da88..48a012cb0 100644 --- a/tests/types/test_json.py +++ b/tests/types/test_json.py @@ -256,6 +256,33 @@ def test_dump_leak_with_local_functions(dsn, binary, pgtype, caplog): assert caplog.records +@pytest.mark.parametrize("binary", [True, False]) +@pytest.mark.parametrize("pgtype", ["json", "jsonb"]) +@pytest.mark.parametrize("dumps", [str, len]) +def test_dumper_warning_builtin(dsn, binary, pgtype, dumps, caplog, recwarn): + caplog.set_level(logging.WARNING, logger="psycopg") + recwarn.clear() + + # Note: private implementation, it might change + from psycopg.types.json import _dumpers_cache + + # A function with no closure is cached on the code, so lambdas are not + # different items. + + with psycopg.connect(dsn) as conn1: + set_json_dumps(dumps, conn1) + assert not recwarn + assert (size1 := len(_dumpers_cache)) + + with psycopg.connect(dsn) as conn2: + set_json_dumps(dumps, conn2) + size2 = len(_dumpers_cache) + + assert size1 == size2 + assert not caplog.records + assert not recwarn + + @pytest.mark.parametrize("binary", [True, False]) @pytest.mark.parametrize("pgtype", ["json", "jsonb"]) def test_load_leak_with_local_functions(dsn, binary, pgtype, caplog): @@ -298,6 +325,33 @@ def test_load_leak_with_local_functions(dsn, binary, pgtype, caplog): assert caplog.records +@pytest.mark.parametrize("binary", [True, False]) +@pytest.mark.parametrize("pgtype", ["json", "jsonb"]) +@pytest.mark.parametrize("loads", [str, len]) +def test_loader_warning_builtin(dsn, binary, pgtype, loads, caplog, recwarn): + caplog.set_level(logging.WARNING, logger="psycopg") + recwarn.clear() + + # Note: private implementation, it might change + from psycopg.types.json import _loaders_cache + + # A function with no closure is cached on the code, so lambdas are not + # different items. + + with psycopg.connect(dsn) as conn1: + set_json_loads(loads, conn1) + assert not recwarn + assert (size1 := len(_loaders_cache)) + + with psycopg.connect(dsn) as conn2: + set_json_loads(loads, conn2) + size2 = len(_loaders_cache) + + assert size1 == size2 + assert not caplog.records + assert not recwarn + + def my_dumps(obj): obj = deepcopy(obj) obj["baz"] = "qux"