From 88738b4db651b826629ace8fad6b96f676de6784 Mon Sep 17 00:00:00 2001 From: gaoflow Date: Thu, 4 Jun 2026 18:25:12 +0200 Subject: [PATCH] Prevent generated RPC methods from being shadowed by ServiceStub params A generated RPC method is a class-level method on the ServiceStub subclass. When its snake_cased name collides with a base __init__ parameter (e.g. an RPC named Metadata -> metadata), the base class stored that parameter as an instance attribute, which takes precedence over the class method on lookup. Calling the RPC then raised TypeError ('dict'/'NoneType' object is not callable). Store channel/timeout/deadline/metadata under private names and expose them through read-only properties. A subclass RPC method now overrides the base property in the MRO, while non-colliding access still returns the parameter. Fixes #224. --- .../betterproto2/grpclib/grpclib_client.py | 43 ++++++++++++----- .../grpc/test_service_stub_attributes.py | 46 +++++++++++++++++++ 2 files changed, 78 insertions(+), 11 deletions(-) create mode 100644 betterproto2/tests/grpc/test_service_stub_attributes.py diff --git a/betterproto2/src/betterproto2/grpclib/grpclib_client.py b/betterproto2/src/betterproto2/grpclib/grpclib_client.py index d8c66575..ad16cade 100644 --- a/betterproto2/src/betterproto2/grpclib/grpclib_client.py +++ b/betterproto2/src/betterproto2/grpclib/grpclib_client.py @@ -30,10 +30,31 @@ def __init__( deadline: Optional["Deadline"] = None, metadata: MetadataLike | None = None, ) -> None: - self.channel = channel - self.timeout = timeout - self.deadline = deadline - self.metadata = metadata + # These are stored under private names and exposed through read-only + # properties so a generated RPC method whose name collides with one of + # them (e.g. an RPC named ``Metadata``) is not shadowed. A subclass + # method overrides the base property in the MRO, whereas an instance + # attribute would take precedence over the method. + self._channel = channel + self._timeout = timeout + self._deadline = deadline + self._metadata = metadata + + @property + def channel(self) -> "Channel": + return self._channel + + @property + def timeout(self) -> float | None: + return self._timeout + + @property + def deadline(self) -> Optional["Deadline"]: + return self._deadline + + @property + def metadata(self) -> MetadataLike | None: + return self._metadata def __resolve_request_kwargs( self, @@ -42,9 +63,9 @@ def __resolve_request_kwargs( metadata: MetadataLike | None, ): return { - "timeout": self.timeout if timeout is None else timeout, - "deadline": self.deadline if deadline is None else deadline, - "metadata": self.metadata if metadata is None else metadata, + "timeout": self._timeout if timeout is None else timeout, + "deadline": self._deadline if deadline is None else deadline, + "metadata": self._metadata if metadata is None else metadata, } async def _unary_unary( @@ -58,7 +79,7 @@ async def _unary_unary( metadata: MetadataLike | None = None, ) -> "T": """Make a unary request and return the response.""" - async with self.channel.request( + async with self._channel.request( route, grpclib.const.Cardinality.UNARY_UNARY, type(request), @@ -81,7 +102,7 @@ async def _unary_stream( metadata: MetadataLike | None = None, ) -> AsyncIterator["T"]: """Make a unary request and return the stream response iterator.""" - async with self.channel.request( + async with self._channel.request( route, grpclib.const.Cardinality.UNARY_STREAM, type(request), @@ -104,7 +125,7 @@ async def _stream_unary( metadata: MetadataLike | None = None, ) -> "T": """Make a stream request and return the response.""" - async with self.channel.request( + async with self._channel.request( route, grpclib.const.Cardinality.STREAM_UNARY, request_type, @@ -132,7 +153,7 @@ async def _stream_stream( Make a stream request and return an AsyncIterator to iterate over response messages. """ - async with self.channel.request( + async with self._channel.request( route, grpclib.const.Cardinality.STREAM_STREAM, request_type, diff --git a/betterproto2/tests/grpc/test_service_stub_attributes.py b/betterproto2/tests/grpc/test_service_stub_attributes.py new file mode 100644 index 00000000..bcdcbc53 --- /dev/null +++ b/betterproto2/tests/grpc/test_service_stub_attributes.py @@ -0,0 +1,46 @@ +"""Regression tests for ServiceStub attribute handling. + +A generated RPC method is a class-level method on the ``ServiceStub`` subclass. +If its snake_cased name happens to collide with one of the base constructor +parameters (``channel``/``timeout``/``deadline``/``metadata`` — e.g. an RPC +literally named ``Metadata``), storing those parameters as plain instance +attributes would shadow the generated method, because an instance attribute +takes precedence over a class method during attribute lookup. The base class +keeps them under private names and exposes read-only properties instead, so a +subclass method overrides the property in the MRO. See issue #224. +""" + +import pytest + +from tests.mocks import MockChannel +from tests.util import requires_grpclib # noqa: F401 + + +@pytest.mark.asyncio +async def test_rpc_method_not_shadowed_by_constructor_param(requires_grpclib): + from betterproto2.grpclib import ServiceStub + + class _StubWithCollidingRpc(ServiceStub): + # Mimics a generated RPC named ``Metadata`` (snake_case ``metadata``), + # which collides with the ``metadata`` constructor parameter. + async def metadata(self): + return "rpc-result" + + stub = _StubWithCollidingRpc(MockChannel(), metadata={"authorization": "token"}) + + assert callable(stub.metadata), "RPC method must not be shadowed by the parameter" + assert await stub.metadata() == "rpc-result" + + +@pytest.mark.asyncio +async def test_constructor_params_exposed_as_properties(requires_grpclib): + from betterproto2.grpclib import ServiceStub + + channel = MockChannel() + metadata = {"authorization": "token"} + stub = ServiceStub(channel, timeout=12.5, metadata=metadata) + + assert stub.channel is channel + assert stub.timeout == 12.5 + assert stub.deadline is None + assert stub.metadata == metadata