Skip to content

Commit 1d4f64f

Browse files
aapelivclaude
andcommitted
Backend: resolve BASE_URL override through context, drop clear semantics
Address review feedback on the per-user BASE_URL override: - Resolution moves onto CouchersContext.use_base_url_override (a contextmanager that sets the urls contextvar), replacing the free-floating use_base_url_override_for_user(session, user_id) helper. The interceptor and notification job now go through context, matching how the rest of the codebase carries per-operation user state. - An override is always non-empty: SetBaseUrlOverride rejects an empty base_url and the empty-string-clears-the-override semantics is gone (it expires via TTL). - Add a composite index (user_id, created) serving the active-override lookup, replacing the redundant single-column user_id index. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent d25a1b0 commit 1d4f64f

9 files changed

Lines changed: 62 additions & 72 deletions

File tree

Lines changed: 7 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,19 @@
11
"""
22
Dev/testing override of BASE_URL, see couchers.models.rest.BaseUrlOverride.
33
4-
The active override for a user is set on the base_url_override contextvar for the duration of a request (from the
5-
authenticated user) or a notification job (from the recipient), and CouchersContext.base_url reads it, so that
6-
every link built via couchers.urls points back at whatever frontend the developer is testing on. Gated by
7-
ENABLE_DEV_APIS, so this is entirely inert in real prod (no overrides can ever be created or applied there).
4+
The active override for a user is applied via CouchersContext.use_base_url_override, which sets the
5+
base_url_override contextvar for the duration of a request (the authenticated user) or a notification job (the
6+
recipient); CouchersContext.base_url reads it, so that every link built via couchers.urls points back at whatever
7+
frontend the developer is testing on. Gated by ENABLE_DEV_APIS, so this is entirely inert in real prod (no
8+
overrides can ever be created or applied there).
89
"""
910

10-
from collections.abc import Iterator
11-
from contextlib import contextmanager
1211
from contextvars import ContextVar
1312
from datetime import timedelta
1413

1514
from sqlalchemy import select
1615
from sqlalchemy.orm import Session
1716

18-
from couchers.config import config
1917
from couchers.models import BaseUrlOverride
2018
from couchers.utils import now
2119

@@ -27,30 +25,11 @@
2725

2826

2927
def get_active_base_url_override(session: Session, user_id: int) -> str | None:
30-
"""The most recently set, non-expired, non-empty base url override for the user, if any."""
31-
base_url = session.execute(
28+
"""The most recently set, non-expired base url override for the user, if any."""
29+
return session.execute(
3230
select(BaseUrlOverride.base_url)
3331
.where(BaseUrlOverride.user_id == user_id)
3432
.where(BaseUrlOverride.created > now() - BASE_URL_OVERRIDE_TTL)
3533
.order_by(BaseUrlOverride.created.desc(), BaseUrlOverride.id.desc())
3634
.limit(1)
3735
).scalar_one_or_none()
38-
return base_url or None
39-
40-
41-
@contextmanager
42-
def use_base_url_override_for_user(session: Session, user_id: int | None) -> Iterator[None]:
43-
"""Set the base url override contextvar to the user's active override for the duration of the block."""
44-
override = None
45-
if config["ENABLE_DEV_APIS"] and user_id is not None:
46-
override = get_active_base_url_override(session, user_id)
47-
48-
if not override:
49-
yield
50-
return
51-
52-
token = base_url_override.set(override)
53-
try:
54-
yield
55-
finally:
56-
base_url_override.reset(token)

app/backend/src/couchers/context.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
1+
from collections.abc import Iterator
2+
from contextlib import contextmanager
13
from typing import TYPE_CHECKING, NoReturn, cast
24

35
import grpc
6+
from sqlalchemy.orm import Session
47

58
from couchers import experimentation
6-
from couchers.base_url_override import base_url_override
9+
from couchers.base_url_override import base_url_override, get_active_base_url_override
710
from couchers.config import config
811
from couchers.i18n import LocalizationContext
912

@@ -212,6 +215,27 @@ def _get_growthbook(self) -> GrowthBook:
212215
self._growthbook = experimentation._create_evaluator(self._user_id)
213216
return self._growthbook
214217

218+
@contextmanager
219+
def use_base_url_override(self, session: Session) -> Iterator[None]:
220+
"""
221+
Point links built via couchers.urls at this context's user's active base url override (if any) for the
222+
duration of the block. Gated by ENABLE_DEV_APIS, so it's a no-op in real prod. See
223+
couchers.base_url_override.
224+
"""
225+
override = None
226+
if config["ENABLE_DEV_APIS"] and self._user_id is not None:
227+
override = get_active_base_url_override(session, self._user_id)
228+
229+
if not override:
230+
yield
231+
return
232+
233+
token = base_url_override.set(override)
234+
try:
235+
yield
236+
finally:
237+
base_url_override.reset(token)
238+
215239

216240
def make_interactive_context(
217241
grpc_context: grpc.ServicerContext,

app/backend/src/couchers/interceptors.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
from sqlalchemy import Function, literal_column, select
2020
from sqlalchemy.sql import and_, func
2121

22-
from couchers.base_url_override import use_base_url_override_for_user
2322
from couchers.constants import (
2423
CALL_CANCELLED_ERROR_MESSAGE,
2524
COOKIES_AND_AUTH_HEADER_ERROR_MESSAGE,
@@ -319,7 +318,7 @@ def function_without_couchers_stuff(req: Message, grpc_context: grpc.ServicerCon
319318

320319
with session_scope() as session:
321320
try:
322-
with use_base_url_override_for_user(session, auth_info.user_id if auth_info else None):
321+
with couchers_context.use_base_url_override(session):
323322
_res = prev_function(req, couchers_context, session) # type: ignore[call-arg, arg-type]
324323
res = cast(Message, _res)
325324
finished = perf_counter_ns()

app/backend/src/couchers/migrations/versions/0158_add_base_url_overrides.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ def upgrade() -> None:
2626
sa.ForeignKeyConstraint(["user_id"], ["users.id"], name=op.f("fk_base_url_overrides_user_id_users")),
2727
sa.PrimaryKeyConstraint("id", name=op.f("pk_base_url_overrides")),
2828
)
29-
op.create_index(op.f("ix_base_url_overrides_user_id"), "base_url_overrides", ["user_id"], unique=False)
29+
op.create_index("ix_base_url_overrides_active", "base_url_overrides", ["user_id", "created"], unique=False)
3030

3131

3232
def downgrade() -> None:
33-
op.drop_index(op.f("ix_base_url_overrides_user_id"), table_name="base_url_overrides")
33+
op.drop_index("ix_base_url_overrides_active", table_name="base_url_overrides")
3434
op.drop_table("base_url_overrides")

app/backend/src/couchers/models/rest.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -626,10 +626,14 @@ class BaseUrlOverride(Base, kw_only=True):
626626
id: Mapped[int] = mapped_column(BigInteger, primary_key=True, init=False)
627627
created: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now(), init=False)
628628

629-
user_id: Mapped[int] = mapped_column(ForeignKey("users.id"), index=True)
629+
user_id: Mapped[int] = mapped_column(ForeignKey("users.id"))
630630

631-
# the base url to use, e.g. "https://my-preview.vercel.app". Empty string clears the override (records that
632-
# the user explicitly went back to the configured BASE_URL).
631+
# the base url to use, e.g. "https://my-preview.vercel.app"
633632
base_url: Mapped[str] = mapped_column(String)
634633

635634
user: Mapped[User] = relationship(init=False)
635+
636+
__table_args__ = (
637+
# serves the active-override lookup: filter by user_id, range on created, ordered by created desc
638+
Index("ix_base_url_overrides_active", user_id, created),
639+
)

app/backend/src/couchers/notifications/background.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
from sqlalchemy.orm import Session
77
from sqlalchemy.sql import exists, func
88

9-
from couchers.base_url_override import use_base_url_override_for_user
109
from couchers.config import config
1110
from couchers.context import make_background_user_context
1211
from couchers.db import session_scope
@@ -141,7 +140,7 @@ def handle_notification(payload: jobs_pb2.HandleNotificationPayload) -> None:
141140
)
142141
)
143142
session.flush()
144-
with use_base_url_override_for_user(session, user.id):
143+
with make_background_user_context(user.id).use_base_url_override(session):
145144
_send_email_notification(session, user, notification)
146145
elif delivery_type == NotificationDeliveryType.digest:
147146
# for digest notifications, add to digest queue
@@ -162,7 +161,7 @@ def handle_notification(payload: jobs_pb2.HandleNotificationPayload) -> None:
162161
)
163162
)
164163
session.flush()
165-
with use_base_url_override_for_user(session, user.id):
164+
with make_background_user_context(user.id).use_base_url_override(session):
166165
_send_push_notification(session, user, notification)
167166

168167

app/backend/src/couchers/servicers/notifications.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,9 @@ def SetBaseUrlOverride(
366366
if not config["ENABLE_DEV_APIS"]:
367367
context.abort_with_error_code(grpc.StatusCode.UNAVAILABLE, "dev_apis_disabled")
368368

369+
if not request.base_url:
370+
context.abort(grpc.StatusCode.INVALID_ARGUMENT, "base_url must not be empty")
371+
369372
session.add(BaseUrlOverride(user_id=context.user_id, base_url=request.base_url))
370373
return empty_pb2.Empty()
371374

@@ -385,14 +388,9 @@ def GetBaseUrlOverrides(
385388
.all()
386389
)
387390

388-
# The active override is the most recent row within the TTL, and only if it's non-empty (an empty
389-
# base_url is an explicit clear). This mirrors base_url_override.get_active_base_url_override.
391+
# The active override is the most recent row within the TTL. Mirrors get_active_base_url_override.
390392
cutoff = now() - BASE_URL_OVERRIDE_TTL
391-
active_id = None
392-
for override in overrides:
393-
if override.created > cutoff:
394-
active_id = override.id if override.base_url else None
395-
break
393+
active_id = next((o.id for o in overrides if o.created > cutoff), None)
396394

397395
return notifications_pb2.GetBaseUrlOverridesRes(
398396
overrides=[

app/backend/src/tests/test_notifications.py

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
BASE_URL_OVERRIDE_TTL,
1616
base_url_override,
1717
get_active_base_url_override,
18-
use_base_url_override_for_user,
1918
)
2019
from couchers.config import config
2120
from couchers.constants import DATETIME_INFINITY
@@ -1942,24 +1941,16 @@ def test_get_active_base_url_override(db):
19421941
session.flush()
19431942
assert get_active_base_url_override(session, user.id) == "https://b.example"
19441943

1945-
# an empty base_url is an explicit clear
1946-
session.add(BaseUrlOverride(user_id=user.id, base_url=""))
1947-
session.flush()
1948-
assert get_active_base_url_override(session, user.id) is None
1949-
1950-
# an expired override is ignored
1951-
expired = BaseUrlOverride(user_id=user.id, base_url="https://c.example")
1952-
session.add(expired)
1953-
session.flush()
1944+
# expired overrides are ignored
19541945
session.execute(
19551946
update(BaseUrlOverride)
1956-
.where(BaseUrlOverride.id == expired.id)
1947+
.where(BaseUrlOverride.user_id == user.id)
19571948
.values(created=now() - BASE_URL_OVERRIDE_TTL - timedelta(minutes=1))
19581949
)
19591950
assert get_active_base_url_override(session, user.id) is None
19601951

19611952

1962-
def test_use_base_url_override_for_user(db):
1953+
def test_context_use_base_url_override(db):
19631954
user, _ = generate_user()
19641955
with session_scope() as session:
19651956
session.add(BaseUrlOverride(user_id=user.id, base_url="https://preview.example.org"))
@@ -1968,16 +1959,16 @@ def test_use_base_url_override_for_user(db):
19681959
context = make_background_user_context(user.id)
19691960

19701961
# inert unless dev APIs are enabled
1971-
with use_base_url_override_for_user(session, user.id):
1962+
with context.use_base_url_override(session):
19721963
assert context.base_url == config["BASE_URL"]
19731964

19741965
with patch.dict(config, {"ENABLE_DEV_APIS": True}):
1975-
with use_base_url_override_for_user(session, user.id):
1966+
with context.use_base_url_override(session):
19761967
assert context.base_url == "https://preview.example.org"
19771968
# restored on exit
19781969
assert context.base_url == config["BASE_URL"]
1979-
# a missing user is a no-op
1980-
with use_base_url_override_for_user(session, None):
1970+
# a logged-out context is a no-op
1971+
with make_logged_out_context(LocalizationContext.en_utc()).use_base_url_override(session):
19811972
assert context.base_url == config["BASE_URL"]
19821973

19831974

@@ -1995,17 +1986,13 @@ def test_SetBaseUrlOverride_and_GetBaseUrlOverrides(db):
19951986
assert res.overrides[1].active is False
19961987

19971988

1998-
def test_SetBaseUrlOverride_clear(db):
1989+
def test_SetBaseUrlOverride_rejects_empty(db):
19991990
user, token = generate_user()
20001991
with patch.dict(config, {"ENABLE_DEV_APIS": True}):
20011992
with notifications_session(token) as notifications:
2002-
notifications.SetBaseUrlOverride(notifications_pb2.SetBaseUrlOverrideReq(base_url="https://a.example"))
2003-
notifications.SetBaseUrlOverride(notifications_pb2.SetBaseUrlOverrideReq(base_url=""))
2004-
res = notifications.GetBaseUrlOverrides(empty_pb2.Empty())
2005-
2006-
# clearing records a row but nothing is active
2007-
assert [o.base_url for o in res.overrides] == ["", "https://a.example"]
2008-
assert all(not o.active for o in res.overrides)
1993+
with pytest.raises(grpc.RpcError) as e:
1994+
notifications.SetBaseUrlOverride(notifications_pb2.SetBaseUrlOverrideReq(base_url=""))
1995+
assert e.value.code() == grpc.StatusCode.INVALID_ARGUMENT
20091996

20101997

20111998
def test_SetBaseUrlOverride_dev_apis_disabled(db):

app/proto/notifications.proto

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,8 @@ message DebugRedeliverPushNotificationReq {
185185
}
186186

187187
message SetBaseUrlOverrideReq {
188-
// the base url to point generated links at, e.g. "https://my-preview.vercel.app".
189-
// An empty string clears the override (links go back to the configured BASE_URL).
188+
// the base url to point generated links at, e.g. "https://my-preview.vercel.app". Must be non-empty; the
189+
// override applies for ~15 min, after which links go back to the configured BASE_URL.
190190
string base_url = 1;
191191
}
192192

0 commit comments

Comments
 (0)