fix: preserve wrapped method metadata in require_authentication#244
Merged
Conversation
…ator `require_authentication` was missing `@functools.wraps(func)` on its inner `wrapper`, so every decorated method (most of `Spond.*`) lost its `__name__`, `__doc__`, `__qualname__`, `__wrapped__`, and real signature. The effect on the published pdoc site was severe: every decorated method rendered as `async def get_xxx(self, *args, **kwargs):` with the decorator's source listing inline, instead of the method's real signature and docstring. The recently-expanded docstrings (#237/#241) existed in source but were never read by `inspect`-based tooling. Fix: - Import `functools`. - Apply `@functools.wraps(func)` to the inner `wrapper`. Regression tests in a new `TestRequireAuthenticationDecorator` class lock in the three properties that need to survive: signature, docstring, and `__name__`. Each would fail without `functools.wraps`.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds @functools.wraps(func) to the require_authentication decorator so wrapped async methods retain their original signature, name, and docstring. This restores correct rendering on the pdoc-generated GitHub Pages site and improves IDE/inspect introspection.
Changes:
- Import
functoolsand apply@functools.wraps(func)to the innerwrapperinrequire_authentication. - Add a new
TestRequireAuthenticationDecoratortest class covering signature, docstring, and__name__preservation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| spond/base.py | Add functools import and @functools.wraps(func) on the require_authentication wrapper. |
| tests/test_spond.py | Add three regression tests asserting the decorator preserves signature, docstring, and name. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
You're right — the function-level docstrings on https://olen.github.io/Spond/spond/spond.html are wrong, but the cause isn't in the docstrings themselves. The `require_authentication` decorator was missing `@functools.wraps(func)`, so every method it decorated lost its metadata:
```python
before
@staticmethod
def require_authentication(func: Callable):
async def wrapper(self, *args, **kwargs):
...
return wrapper
```
Without `functools.wraps`, `Spond.get_profile` was internally just a copy of `wrapper`. `inspect.signature` saw `(self, *args, **kwargs)`. `inspect.getdoc` saw nothing (or the wrapper's docstring leaked through, depending on Python's exact resolution). pdoc reads both, so every decorated method on the live site rendered as:
```
async def get_profile(self, *args, **kwargs):
# source: the wrapper's body, not get_profile's
```
instead of:
```
async def get_profile(self) -> JSONDict:
"""Retrieve the authenticated user's profile.
```
All my docstring work from #237 / #241 landed in source but pdoc never saw it for decorated methods.
The fix
Two lines: `import functools` + `@functools.wraps(func)` on the inner wrapper. After this fix, decorated methods expose their real signatures and docstrings:
```
get_profile(self) -> 'JSONDict'
→ Retrieve the authenticated user's profile.
get_posts(self, group_id: 'str | None' = None, max_posts: 'int' = 20, ...
→ Retrieve posts from group walls.
get_events(self, group_id: 'str | None' = None, subgroup_id: 'str | None' = None, ...
→ Retrieve events visible to the authenticated user.
```
Regression tests
New `TestRequireAuthenticationDecorator` class with three tests that each fail if `functools.wraps` is removed:
Why this only surfaced now
Before #241 expanded the docstrings, all decorated methods had thin one-line docstrings. The wrapper's "leaked" appearance was less obvious because the user-facing docs were already terse. After #241 dropped substantial docstrings on each method, the bug became loud: I had carefully written content for, say, `get_events`, and pdoc kept showing a generic `(self, *args, **kwargs)` shim instead.
Test plan
🤖 Generated with Claude Code