diff --git a/CHANGELOG.md b/CHANGELOG.md index fe8038d..6ebc7ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## v0.2.1 + +- Fixed maximum recursion depth exceeded when calling the original method of a + patched class calls `super()`. + ## v0.2.0 - Added compatibility for Indico v3.3. diff --git a/pyproject.toml b/pyproject.toml index 0c35bc7..424d1b9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ prefer-active-python = true [tool.poetry] name = "indico-patcher" -version = "0.2.0" +version = "0.2.1" description = "The Swiss Army knife to customize Indico" repository = "https://github.com/unconventionaldotdev/indico-patcher" readme = "README.md" diff --git a/src/indico_patcher/types.py b/src/indico_patcher/types.py index 9910c54..ed67994 100644 --- a/src/indico_patcher/types.py +++ b/src/indico_patcher/types.py @@ -7,6 +7,7 @@ from enum import EnumMeta from types import FunctionType from typing import Any +from typing import TypedDict from typing import Union from sqlalchemy.ext.hybrid import hybrid_property @@ -24,3 +25,15 @@ class PatchedClass: __patches__: list[type] __unpatched__: dict[str, dict[str, Any]] + + +# Dictionary of property descriptor functions +class PropertyDescriptors(TypedDict, total=False): + fget: FunctionType | None + fset: FunctionType | None + fdel: FunctionType | None + + +# Dictionary of hybrid_property descriptor functions +class HybridPropertyDescriptors(PropertyDescriptors, total=False): + expr: FunctionType | None diff --git a/src/indico_patcher/util.py b/src/indico_patcher/util.py index c8baf59..df057c6 100644 --- a/src/indico_patcher/util.py +++ b/src/indico_patcher/util.py @@ -4,19 +4,25 @@ from __future__ import annotations import sys -from collections.abc import Callable from functools import partial from types import FrameType from types import FunctionType from types import MappingProxyType from typing import Any +from typing import cast from sqlalchemy.ext.hybrid import hybrid_property +from .types import HybridPropertyDescriptors from .types import PatchedClass +from .types import PropertyDescriptors from .types import methodlike from .types import propertylike +# TODO: Add `fset` and `fdel` descriptors once SuperProxy supports them +SUPER_ENABLED_DESCRIPTORS = {"fget"} +SUPPORTED_DESCRIPTORS = {"fget", "fset", "fdel", "expr"} + class SuperProxy: """A proxy for super that allows calling the original class' methods.""" @@ -53,10 +59,10 @@ def __getattribute__(self_, name: str) -> Any: return partial(method, obj) if classmethod := self.orig_class.__unpatched__["classmethods"].get(name): - return classmethod + return partial(classmethod.__func__, self.orig_class) if staticmethod := self.orig_class.__unpatched__["staticmethods"].get(name): - return staticmethod + return partial(staticmethod) # Fallback to the original class' member return getattr(obj, name) if obj else getattr(self.orig_class, name) @@ -102,10 +108,9 @@ def patch_member(orig_class: PatchedClass, member_name: str, member: Any) -> Non # TODO: Patch relationship # TODO: Patch deferred columns if isinstance(member, property): - # TODO: Add `fset` and `fdel` descriptors once SuperProxy supports them - _patch_propertylike(orig_class, member_name, member, "properties", ("fget",)) + _patch_propertylike(orig_class, member_name, member, "properties", ("fget", "fset", "fdel")) elif isinstance(member, hybrid_property): - _patch_propertylike(orig_class, member_name, member, "hybrid_properties", ("fget", "expression")) + _patch_propertylike(orig_class, member_name, member, "hybrid_properties", ("fget", "fset", "fdel", "expr")) elif isinstance(member, FunctionType): _patch_methodlike(orig_class, member_name, member, "methods") elif isinstance(member, classmethod): @@ -140,16 +145,20 @@ def _patch_propertylike(orig_class: PatchedClass, prop_name: str, prop: property """ if category not in {"properties", "hybrid_properties"}: raise ValueError(f"Unsupported category '{category}'") - if unsupported_fnames := set(fnames) - {"fget", "expression"}: + if unsupported_fnames := set(fnames) - SUPPORTED_DESCRIPTORS: raise ValueError(f"Unsupported descriptor method '{list(unsupported_fnames)[0]}'") # Keep a reference to the original property-like member _store_unpatched(orig_class, prop_name, category) + # Inject super() in the property descriptor methods + # TODO: Figure out how to avoid casting + funcs: PropertyDescriptors | HybridPropertyDescriptors = cast(PropertyDescriptors | HybridPropertyDescriptors, { + fname: _inject_super_proxy(getattr(prop, fname), orig_class) if fname in SUPER_ENABLED_DESCRIPTORS else + getattr(prop, fname) + for fname in fnames + }) + new_prop = property(**funcs) if isinstance(prop, property) else hybrid_property(**funcs) # Replace the original property-like member - setattr(orig_class, prop_name, prop) - # Override super() in the property descriptor methods - for fname in fnames: - if func := getattr(prop, fname, None): - func.__globals__["super"] = SuperProxy(orig_class) + setattr(orig_class, prop_name, new_prop) def _patch_methodlike(orig_class: PatchedClass, method_name: str, method: methodlike, category: str) -> None: @@ -158,19 +167,20 @@ def _patch_methodlike(orig_class: PatchedClass, method_name: str, method: method :param orig_class: The class to patch :param method_name: The name of the method-like member to patch in the class :param method: The method-like object to replace the original member with - :param category: The category of unpached members to store the original member in + :param category: The category of unpatched members to store the original member in """ if category not in {"methods", "classmethods", "staticmethods"}: raise ValueError(f"Unsupported category '{category}'") - # Keep a reference to the original method + # Keep a reference to the original method-like member _store_unpatched(orig_class, method_name, category) + # Override super() in the method globals + # XXX: Type is casted and type checking is disabled because mypy infers the wrong types + # for __func__ in classmethods (https://github.com/python/mypy/issues/3482) + func = method if isinstance(method, FunctionType) else cast(FunctionType, method.__func__) + new_func = _inject_super_proxy(func, orig_class) + new_method = classmethod(new_func) if isinstance(method, classmethod) else new_func # Replace the original method - setattr(orig_class, method_name, method) - # Override super() in the method - # XXX: Type is declared explicitly and type checking is disabled because mypy - # infers wrong types for classmethods and staticmethods (https://github.com/python/mypy/issues/3482) - func: Callable = method if isinstance(method, FunctionType) else method.__func__ - func.__globals__["super"] = SuperProxy(orig_class) + setattr(orig_class, method_name, new_method) def _store_unpatched(orig_class: PatchedClass, member_name: str, category: str) -> None: @@ -179,6 +189,19 @@ def _store_unpatched(orig_class: PatchedClass, member_name: str, category: str) :param orig_class: The class to store the reference in :param member_name: The name of the member to store the reference for """ - if hasattr(orig_class, member_name): + # None can be a valid value for the member, so we need to check if the member is in the class dict + if member_name in orig_class.__dict__: # TODO: Log warning if member is already patched - orig_class.__unpatched__[category][member_name] = getattr(orig_class, member_name) + member = orig_class.__dict__[member_name] + orig_class.__unpatched__[category][member_name] = member + + +def _inject_super_proxy(func: FunctionType, orig_class: PatchedClass) -> FunctionType: + """Return a new function from which super() will call SuperProxy(). + + :param func: The function that will get SuperProxy injected + :param orig_class: The original class that will be passed to SuperProxy + """ + globals = func.__globals__.copy() + globals["super"] = SuperProxy(orig_class) + return FunctionType(func.__code__, globals, func.__name__, func.__defaults__, func.__closure__) diff --git a/tests/test_util.py b/tests/test_util.py index 803c15f..2870261 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -8,7 +8,9 @@ from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy.sql.elements import ClauseElement +from indico_patcher.util import SUPER_ENABLED_DESCRIPTORS from indico_patcher.util import SuperProxy +from indico_patcher.util import _inject_super_proxy from indico_patcher.util import _patch_attr from indico_patcher.util import _patch_methodlike from indico_patcher.util import _patch_propertylike @@ -45,6 +47,50 @@ def meth(self): return Fool +@pytest.fixture +def _Fool(): + class _Fool: + @property + def prop(self): + pass + + @prop.setter + def prop(self, value): + pass + + @prop.deleter + def prop(self): + pass + + @hybrid_property + def hprop(self): + pass + + @hprop.setter + def hprop(self, value): + pass + + @hprop.deleter + def hprop(self): + pass + + @hprop.expression + def hprop(cls): + return ClauseElement() + + @staticmethod + def smeth(): + pass + + @classmethod + def cmeth(cls): + pass + + def meth(self): + pass + + return _Fool + # -- super proxy --------------------------------------------------------------- def test_superproxy_repr(Fool): @@ -107,17 +153,19 @@ def test_patch_member_for_attribute(Fool): @mock.patch("indico_patcher.util._patch_propertylike") def test_patch_member_for_propertylike(_patch_propertylike, Fool): + prop = Fool.__dict__["prop"] patch_member(Fool, "prop", Fool.prop) - _patch_propertylike.assert_called_with(Fool, "prop", Fool.prop, "properties", ("fget",)) + _patch_propertylike.assert_called_with(Fool, "prop", prop, "properties", ("fget", "fset", "fdel")) hprop = Fool.__dict__["hprop"] patch_member(Fool, "hprop", hprop) - _patch_propertylike.assert_called_with(Fool, "hprop", hprop, "hybrid_properties", ("fget", "expression")) + _patch_propertylike.assert_called_with(Fool, "hprop", hprop, "hybrid_properties", ("fget", "fset", "fdel", "expr")) @mock.patch("indico_patcher.util._patch_methodlike") def test_patch_member_for_methodlike(_patch_methodlike, Fool): + meth = Fool.__dict__["meth"] patch_member(Fool, "meth", Fool.meth) - _patch_methodlike.assert_called_with(Fool, "meth", Fool.meth, "methods") + _patch_methodlike.assert_called_with(Fool, "meth", meth, "methods") smeth = Fool.__dict__["smeth"] patch_member(Fool, "smeth", smeth) _patch_methodlike.assert_called_with(Fool, "smeth", smeth, "staticmethods") @@ -145,43 +193,46 @@ def test_patch_propertylike_for_invalid_values(Fool): _patch_propertylike(Fool, None, None, "properties", ("foo",)) -def test_patch_propertylike_for_property(Fool): - fnames = ("fget",) - orig_prop = Fool.prop - - class _Fool: - @property - def prop(self): - pass - - _patch_propertylike(Fool, "prop", _Fool.prop, "properties", fnames) - assert Fool.prop == _Fool.prop - assert Fool.__unpatched__["properties"]["prop"] == orig_prop +@mock.patch("indico_patcher.util._store_unpatched") +@mock.patch("indico_patcher.util._inject_super_proxy") +def test_patch_propertylike_for_property(_inject_super_proxy, _store_unpatched, Fool, _Fool): + mock_func = mock.Mock() + _inject_super_proxy.return_value = mock_func + fnames = ("fget", "fset", "fdel") + prop = _Fool.__dict__["prop"] + _patch_propertylike(Fool, "prop", prop, "properties", fnames) + new_prop = Fool.__dict__["prop"] + _store_unpatched.assert_called_with(Fool, "prop", "properties") + expected_calls = [] for fname in fnames: - func = getattr(Fool.prop, fname) - assert isinstance(func.__globals__["super"], SuperProxy) - - -def test_patch_propertylike_for_hybrid_property(Fool): - fnames = ("fget", "expression") - orig_hprop = Fool.hprop - - class _Fool: - @hybrid_property - def hprop(self): - pass - - @hprop.expression - def hprop(cls): - return ClauseElement() - - new_hprop = _Fool.__dict__["hprop"] - _patch_propertylike(Fool, "hprop", new_hprop, "hybrid_properties", fnames) - assert Fool.__dict__["hprop"] == new_hprop - assert Fool.__unpatched__["hybrid_properties"]["hprop"] == orig_hprop + func = getattr(prop, fname) + if fname in SUPER_ENABLED_DESCRIPTORS: + expected_calls.append(mock.call(func, Fool)) + assert getattr(new_prop, fname) == mock_func + else: + assert getattr(new_prop, fname) == func + _inject_super_proxy.assert_has_calls(expected_calls) + + +@mock.patch("indico_patcher.util._store_unpatched") +@mock.patch("indico_patcher.util._inject_super_proxy") +def test_patch_propertylike_for_hybrid_property(_inject_super_proxy, _store_unpatched, Fool, _Fool): + mock_func = mock.Mock() + _inject_super_proxy.return_value = mock_func + fnames = ("fget", "fset", "fdel", "expr") + hprop = _Fool.__dict__["hprop"] + _patch_propertylike(Fool, "hprop", hprop, "hybrid_properties", fnames) + new_hprop = Fool.__dict__["hprop"] + _store_unpatched.assert_called_with(Fool, "hprop", "hybrid_properties") + expected_calls = [] for fname in fnames: - func = getattr(new_hprop, fname) - assert isinstance(func.__globals__["super"], SuperProxy) + func = getattr(hprop, fname) + if fname in SUPER_ENABLED_DESCRIPTORS: + expected_calls.append(mock.call(func, Fool)) + assert getattr(new_hprop, fname) == mock_func + else: + assert getattr(new_hprop, fname) == func + _inject_super_proxy.assert_has_calls(expected_calls) # -- method-like --------------------------------------------------------------- @@ -191,49 +242,73 @@ def test_patch_methodlike_for_invalid_values(Fool): _patch_methodlike(Fool, None, None, "properties") -def test_patch_methodlike_for_method(Fool): - orig_meth = Fool.meth +@mock.patch("indico_patcher.util._store_unpatched") +@mock.patch("indico_patcher.util._inject_super_proxy") +def test_patch_methodlike_for_method(_inject_super_proxy, _store_unpatched, Fool, _Fool): + mock_func = mock.Mock() + _inject_super_proxy.return_value = mock_func + meth = _Fool.__dict__["meth"] + _patch_methodlike(Fool, "meth", meth, "methods") + _store_unpatched.assert_called_with(Fool, "meth", "methods") + _inject_super_proxy.assert_called_with(meth, Fool) + assert Fool.meth == mock_func - class _Fool: - def meth(self): - pass - _patch_methodlike(Fool, "meth", _Fool.meth, "methods") - assert Fool.meth == _Fool.meth - assert Fool.__unpatched__["methods"]["meth"] == orig_meth - assert isinstance(Fool.meth.__globals__["super"], SuperProxy) +@mock.patch("indico_patcher.util._store_unpatched") +@mock.patch("indico_patcher.util._inject_super_proxy") +def test_patch_methodlike_for_classmethod(_inject_super_proxy, _store_unpatched, Fool, _Fool): + mock_func = mock.Mock() + _inject_super_proxy.return_value = mock_func + cmeth = _Fool.__dict__["cmeth"] + _patch_methodlike(Fool, "cmeth", cmeth, "classmethods") + _store_unpatched.assert_called_with(Fool, "cmeth", "classmethods") + _inject_super_proxy.assert_called_with(cmeth.__func__, Fool) + assert Fool.cmeth.__func__ == mock_func -def test_patch_methodlike_for_classmethod(Fool): - orig_cmeth = Fool.cmeth - class _Fool: - @classmethod - def cmeth(cls): - pass +@mock.patch("indico_patcher.util._store_unpatched") +@mock.patch("indico_patcher.util._inject_super_proxy") +def test_patch_methodlike_for_staticmethod(_inject_super_proxy, _store_unpatched, Fool, _Fool): + mock_func = mock.Mock() + _inject_super_proxy.return_value = mock_func + smeth = _Fool.__dict__["smeth"] + _patch_methodlike(Fool, "smeth", smeth, "staticmethods") + _store_unpatched.assert_called_with(Fool, "smeth", "staticmethods") + _inject_super_proxy.assert_called_with(smeth.__func__, Fool) + assert Fool.smeth == mock_func - _patch_methodlike(Fool, "cmeth", _Fool.cmeth, "classmethods") - assert Fool.cmeth == _Fool.cmeth - assert Fool.__unpatched__["classmethods"]["cmeth"] == orig_cmeth - assert isinstance(Fool.cmeth.__globals__["super"], SuperProxy) +# -- store unpatched member ---------------------------------------------------- -def test_patch_methodlike_for_staticmethod(Fool): - orig_smeth = Fool.smeth +@pytest.mark.parametrize(("member_name", "category"), [ + ("attr", "attributes"), + ("prop", "properties"), + ("hprop", "hybrid_properties"), + ("meth", "methods"), + ("cmeth", "classmethods"), + ("smeth", "staticmethods"), +]) +def test_store_unpatched_member(member_name, category, Fool): + _store_unpatched(Fool, member_name, category) + assert Fool.__unpatched__[category][member_name] == Fool.__dict__[member_name] - class _Fool: - @staticmethod - def smeth(): - pass - _patch_methodlike(Fool, "smeth", _Fool.smeth, "staticmethods") - assert Fool.smeth == _Fool.smeth - assert Fool.__unpatched__["staticmethods"]["smeth"] == orig_smeth - assert isinstance(Fool.smeth.__globals__["super"], SuperProxy) +# -- inject super proxy -------------------------------------------------------- +def test_inject_super_proxy(Fool): + orig_meth = Fool.meth -# -- store unpatched member ---------------------------------------------------- + class _Fool: + def meth(self): + pass -def test_store_unpatched_member(Fool): - _store_unpatched(Fool, "attr", "attributes") - assert Fool.__unpatched__["attributes"]["attr"] == Fool.attr + new_func = _inject_super_proxy(_Fool.meth, Fool) + super_proxy = new_func.__globals__["super"] + assert "super" not in orig_meth.__globals__ + assert isinstance(super_proxy, SuperProxy) + assert super_proxy.orig_class == Fool + assert new_func.__code__ == _Fool.meth.__code__ + assert new_func.__name__ == _Fool.meth.__name__ + assert new_func.__defaults__ == _Fool.meth.__defaults__ + assert new_func.__closure__ == _Fool.meth.__closure__