Skip to content

Commit df3ae38

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 a347f39 commit df3ae38

9 files changed

Lines changed: 67 additions & 74 deletions

File tree

Lines changed: 7 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,52 +1,30 @@
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 urls.base_url_override contextvar for the duration of a request
5-
(from the authenticated user) or a notification job (from the recipient), so that every link built via
6-
couchers.urls points back at whatever frontend the developer is testing on. Gated by ENABLE_DEV_APIS, so this is
7-
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+
urls.base_url_override contextvar for the duration of a request (the authenticated user) or a notification job
6+
(the recipient), so that every link built via couchers.urls points back at whatever frontend the developer is
7+
testing on. Gated by ENABLE_DEV_APIS, so this is entirely inert in real prod (no overrides can ever be created
8+
or applied there).
89
"""
910

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

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

17-
from couchers import urls
18-
from couchers.config import config
1916
from couchers.models import BaseUrlOverride
2017
from couchers.utils import now
2118

2219
BASE_URL_OVERRIDE_TTL = timedelta(minutes=15)
2320

2421

2522
def get_active_base_url_override(session: Session, user_id: int) -> str | None:
26-
"""The most recently set, non-expired, non-empty base url override for the user, if any."""
27-
base_url = session.execute(
23+
"""The most recently set, non-expired base url override for the user, if any."""
24+
return session.execute(
2825
select(BaseUrlOverride.base_url)
2926
.where(BaseUrlOverride.user_id == user_id)
3027
.where(BaseUrlOverride.created > now() - BASE_URL_OVERRIDE_TTL)
3128
.order_by(BaseUrlOverride.created.desc(), BaseUrlOverride.id.desc())
3229
.limit(1)
3330
).scalar_one_or_none()
34-
return base_url or None
35-
36-
37-
@contextmanager
38-
def use_base_url_override_for_user(session: Session, user_id: int | None) -> Iterator[None]:
39-
"""Set the base url override contextvar to the user's active override for the duration of the block."""
40-
override = None
41-
if config["ENABLE_DEV_APIS"] and user_id is not None:
42-
override = get_active_base_url_override(session, user_id)
43-
44-
if not override:
45-
yield
46-
return
47-
48-
token = urls.base_url_override.set(override)
49-
try:
50-
yield
51-
finally:
52-
urls.base_url_override.reset(token)

app/backend/src/couchers/context.py

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
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

5-
from couchers import experimentation
8+
from couchers import experimentation, urls
9+
from couchers.base_url_override import get_active_base_url_override
10+
from couchers.config import config
611
from couchers.i18n import LocalizationContext
712

813
if TYPE_CHECKING:
@@ -204,6 +209,27 @@ def _get_growthbook(self) -> GrowthBook:
204209
self._growthbook = experimentation._create_evaluator(self._user_id)
205210
return self._growthbook
206211

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

208234
def make_interactive_context(
209235
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
@@ -133,7 +132,7 @@ def handle_notification(payload: jobs_pb2.HandleNotificationPayload) -> None:
133132
)
134133
)
135134
session.flush()
136-
with use_base_url_override_for_user(session, user.id):
135+
with make_background_user_context(user.id).use_base_url_override(session):
137136
_send_email_notification(session, user, notification)
138137
elif delivery_type == NotificationDeliveryType.digest:
139138
# for digest notifications, add to digest queue
@@ -154,7 +153,7 @@ def handle_notification(payload: jobs_pb2.HandleNotificationPayload) -> None:
154153
)
155154
)
156155
session.flush()
157-
with use_base_url_override_for_user(session, user.id):
156+
with make_background_user_context(user.id).use_base_url_override(session):
158157
_send_push_notification(session, user, notification)
159158

160159

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

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

361+
if not request.base_url:
362+
context.abort(grpc.StatusCode.INVALID_ARGUMENT, "base_url must not be empty")
363+
361364
session.add(BaseUrlOverride(user_id=context.user_id, base_url=request.base_url))
362365
return empty_pb2.Empty()
363366

@@ -377,14 +380,9 @@ def GetBaseUrlOverrides(
377380
.all()
378381
)
379382

380-
# The active override is the most recent row within the TTL, and only if it's non-empty (an empty
381-
# base_url is an explicit clear). This mirrors base_url_override.get_active_base_url_override.
383+
# The active override is the most recent row within the TTL. Mirrors get_active_base_url_override.
382384
cutoff = now() - BASE_URL_OVERRIDE_TTL
383-
active_id = None
384-
for override in overrides:
385-
if override.created > cutoff:
386-
active_id = override.id if override.base_url else None
387-
break
385+
active_id = next((o.id for o in overrides if o.created > cutoff), None)
388386

389387
return notifications_pb2.GetBaseUrlOverridesRes(
390388
overrides=[

app/backend/src/tests/test_notifications.py

Lines changed: 14 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,10 @@
1414
from couchers.base_url_override import (
1515
BASE_URL_OVERRIDE_TTL,
1616
get_active_base_url_override,
17-
use_base_url_override_for_user,
1817
)
1918
from couchers.config import config
2019
from couchers.constants import DATETIME_INFINITY
21-
from couchers.context import make_background_user_context
20+
from couchers.context import make_background_user_context, make_logged_out_context
2221
from couchers.crypto import b64decode
2322
from couchers.db import session_scope
2423
from couchers.i18n import LocalizationContext
@@ -1940,40 +1939,34 @@ def test_get_active_base_url_override(db):
19401939
session.flush()
19411940
assert get_active_base_url_override(session, user.id) == "https://b.example"
19421941

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

19591950

1960-
def test_use_base_url_override_for_user(db):
1951+
def test_context_use_base_url_override(db):
19611952
user, _ = generate_user()
19621953
with session_scope() as session:
19631954
session.add(BaseUrlOverride(user_id=user.id, base_url="https://preview.example.org"))
19641955
session.flush()
19651956

1957+
context = make_background_user_context(user.id)
1958+
19661959
# inert unless dev APIs are enabled
1967-
with use_base_url_override_for_user(session, user.id):
1960+
with context.use_base_url_override(session):
19681961
assert urls.base_url() == config["BASE_URL"]
19691962

19701963
with patch.dict(config, {"ENABLE_DEV_APIS": True}):
1971-
with use_base_url_override_for_user(session, user.id):
1964+
with context.use_base_url_override(session):
19721965
assert urls.base_url() == "https://preview.example.org"
19731966
# restored on exit
19741967
assert urls.base_url() == config["BASE_URL"]
1975-
# a missing user is a no-op
1976-
with use_base_url_override_for_user(session, None):
1968+
# a logged-out context is a no-op
1969+
with make_logged_out_context(LocalizationContext.en_utc()).use_base_url_override(session):
19771970
assert urls.base_url() == config["BASE_URL"]
19781971

19791972

@@ -1991,17 +1984,13 @@ def test_SetBaseUrlOverride_and_GetBaseUrlOverrides(db):
19911984
assert res.overrides[1].active is False
19921985

19931986

1994-
def test_SetBaseUrlOverride_clear(db):
1987+
def test_SetBaseUrlOverride_rejects_empty(db):
19951988
user, token = generate_user()
19961989
with patch.dict(config, {"ENABLE_DEV_APIS": True}):
19971990
with notifications_session(token) as notifications:
1998-
notifications.SetBaseUrlOverride(notifications_pb2.SetBaseUrlOverrideReq(base_url="https://a.example"))
1999-
notifications.SetBaseUrlOverride(notifications_pb2.SetBaseUrlOverrideReq(base_url=""))
2000-
res = notifications.GetBaseUrlOverrides(empty_pb2.Empty())
2001-
2002-
# clearing records a row but nothing is active
2003-
assert [o.base_url for o in res.overrides] == ["", "https://a.example"]
2004-
assert all(not o.active for o in res.overrides)
1991+
with pytest.raises(grpc.RpcError) as e:
1992+
notifications.SetBaseUrlOverride(notifications_pb2.SetBaseUrlOverrideReq(base_url=""))
1993+
assert e.value.code() == grpc.StatusCode.INVALID_ARGUMENT
20051994

20061995

20071996
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)