Skip to content

Expose flask.g.user from requires_session, drop redundant User.find_by_id calls #527

@mircealungu

Description

@mircealungu

Background

The @requires_session decorator in zeeguu/api/utils/route_wrappers.py already fetches the authenticated User object as part of its work — see the user = User.find_by_id(user_id) call at route_wrappers.py:87. It uses user for update_last_seen_if_needed, the email-verification check, etc., and then drops the reference.

Every authenticated endpoint that needs the user then fetches it a second time with user = User.find_by_id(flask.g.user_id). There are 20+ such call sites today (article.py, bookmark.py, exercise_sessions.py, browsing_sessions.py, the new _update_streak helper, and many more).

Proposal

In route_wrappers.py, after the existing user = User.find_by_id(user_id) call, add:

```python
flask.g.user = user
```

Then sweep through endpoints and replace:

```python
user = User.find_by_id(flask.g.user_id)

... use user.something

```

with:

```python
flask.g.user.something
```

Why it's worth doing

  • Removes ~20 duplicate User.find_by_id calls across the codebase. Even though SQLAlchemy's identity map usually makes the second call a cache hit, the code still has to be read.
  • Fewer lines, less noise. Each of those endpoints currently has a 1-2 line preamble that adds no information; the body becomes one line shorter and more direct.
  • Better mental model for new contributors. `flask.g.user_id` and `flask.g.user` paired together make it obvious that auth runs before the endpoint and that both are guaranteed available. Today, a reader has to know that `User.find_by_id` uses `.one()` (which raises) and that `@requires_session` already validated the id — knowledge that isn't visible at the call site.
  • Eliminates dead defensive guards. Several endpoints have `if user else None`-style checks that are unreachable because `@requires_session` would have already 401'd. With `flask.g.user` always present, these guards become obviously unnecessary.

Why a separate PR (not part of #525)

This came up during the review of #525 (audio-streak attribution fix). I deliberately kept it out of that PR because:

  1. Blast radius. Touching `route_wrappers.py` affects every authenticated endpoint and every test that exercises auth — much riskier than the localized streak fix.
  2. Reviewability. Mixing a bug fix with a 20-file structural refactor doubles the reviewer's mental load and hides the actual fix.
  3. Independent value. The refactor would be valuable even if the streak bug never existed.
  4. Risk isolation. If the `flask.g.user` change has a subtle issue (e.g., teardown ordering, anonymous-user edge cases), you want to revert it without reverting the streak fix.

Implementation notes / things to check

  • Does `flask.g` reset per-request as expected? (Should be yes — `flask.g` is request-scoped — but worth verifying with a quick test.)
  • The anonymous-user code path: `route_wrappers.py` has logic that handles anonymous sessions; make sure `flask.g.user` is set to a sensible value (or explicitly `None` plus a docstring) for those.
  • Edge case at `route_wrappers.py:89` — `if user:` — there's currently a guard, suggesting `User.find_by_id` might return None in some path. Worth confirming whether that's actually reachable or just paranoia, before relying on `flask.g.user` being non-None unconditionally.
  • The sweep is mechanical but should be done with grep + manual review, not blanket regex, since some endpoints fetch users other than the current one.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions