Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions openlibrary/asgi_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from uvicorn.middleware.proxy_headers import ProxyHeadersMiddleware

import infogami
from openlibrary.fastapi.middleware.experiments import ABTestingMiddleware
from openlibrary.utils.request_context import set_context_from_fastapi
from openlibrary.utils.sentry import Sentry, init_sentry

Expand Down Expand Up @@ -179,6 +180,8 @@ def create_app() -> FastAPI | None:
# Needed for the staging nginx proxy
app.add_middleware(ProxyHeadersMiddleware, trusted_hosts="*")

app.add_middleware(ABTestingMiddleware)

@app.middleware("http")
async def add_fastapi_header(request: Request, call_next):
"""Middleware to add a header indicating the response came from FastAPI."""
Expand Down
53 changes: 53 additions & 0 deletions openlibrary/core/experiments.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import hashlib
from typing import Any

ACTIVE_EXPERIMENTS: dict[str, dict[str, Any]] = {
"AB_Testing": {
"variants": {
"control": 30,
"a": 35,
"b": 35,
},
"authorized_only": False,
}
}


def get_variant(experiment_name: str, user_identifier: str | None, is_logged_in: bool = False) -> str:
"""Deterministically assigns a user to a variant based on configured weights."""
if not user_identifier or experiment_name not in ACTIVE_EXPERIMENTS:
return "control"

exp_config = ACTIVE_EXPERIMENTS[experiment_name]
if exp_config.get("authorized_only") and not is_logged_in:
return "control"

variants = exp_config.get("variants", {})

hash_key = f"{experiment_name}-{user_identifier}".encode()
bucket = int(hashlib.md5(hash_key).hexdigest()[:8], 16) % 100

cumulative_weight = 0
for variant, weight in variants.items():
cumulative_weight += weight
if bucket < cumulative_weight:
return variant

return "control"


def get_user_experiments(
user_identifier: str | None,
overrides: dict[str, str] | None = None,
is_logged_in: bool = False,
) -> dict[str, str]:
"""Evaluates all active experiments for a user, applying optional overrides."""
experiments = {}
for exp, exp_config in ACTIVE_EXPERIMENTS.items():
variants = exp_config.get("variants", {})
override_key = f"experiment_{exp}"
if overrides and override_key in overrides and overrides[override_key] in variants:
experiments[exp] = overrides[override_key]
else:
experiments[exp] = get_variant(exp, user_identifier, is_logged_in=is_logged_in)
return experiments
29 changes: 29 additions & 0 deletions openlibrary/fastapi/middleware/experiments.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
from fastapi import Request
from starlette.types import ASGIApp, Receive, Scope, Send

from infogami import config
from openlibrary.core.experiments import get_user_experiments
from openlibrary.fastapi.auth import authenticate_user_from_cookie


class ABTestingMiddleware:
def __init__(self, app: ASGIApp) -> None:
self.app = app

async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
if scope["type"] != "http":
await self.app(scope, receive, send)
return

request = Request(scope, receive=receive)
session_cookie_name = config.get("login_cookie_name", "session")
session_value = request.cookies.get(session_cookie_name)
user = authenticate_user_from_cookie(session_value)
is_logged_in = user is not None

user_id = session_value or (request.client.host if request.client else "127.0.0.1")

query_overrides = {k: v for k, v in request.query_params.items() if k.startswith("experiment_")}
request.state.experiments = get_user_experiments(user_id, overrides=query_overrides, is_logged_in=is_logged_in) if user_id else {}

await self.app(scope, receive, send)
9 changes: 7 additions & 2 deletions openlibrary/plugins/openlibrary/code.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,14 @@ def setup_contextvars(handler):
delegate.app.add_processor(processors.PreferenceProcessor())
# Refer to https://github.com/internetarchive/openlibrary/pull/10005 to force patron's to login
# delegate.app.add_processor(processors.RequireLogoutProcessor())
# IMPORTANT: setup_contextvars must run AFTER other processors but BEFORE the handler
# It's added here (not as a loadhook) so it runs in the same request context

# IMPORTANT: setup_contextvars must run AFTER other processors but BEFORE the handler.
# The only exception is ExperimentsProcessor, which needs to run after setup_contextvars
# so that request ContextVars (like `site` and `req_context`) are fully initialized,
# allowing `get_current_user()` to function correctly.
# It's added here (not as a loadhook) so it runs in the same request context.
Comment thread
Sadashii marked this conversation as resolved.
Outdated
delegate.app.add_processor(setup_contextvars)
delegate.app.add_processor(processors.ExperimentsProcessor())

Comment thread
Sadashii marked this conversation as resolved.
Outdated
try:
from infogami.plugins.api import code as api
Expand Down
10 changes: 10 additions & 0 deletions openlibrary/plugins/openlibrary/js/experiments.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export function getExperiment(experimentName) {
if (typeof window === 'undefined') {
return 'control';
}
return window.OL_EXPERIMENTS?.[experimentName] || 'control';
}

if (typeof window !== 'undefined') {
window.getExperiment = getExperiment;
}
1 change: 1 addition & 0 deletions openlibrary/plugins/openlibrary/js/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { exposeGlobally } from './jsdef';
import initAnalytics from './ol.analytics';
import init from './ol.js';
import initServiceWorker from './service-worker-init.js';
import './experiments.js';
import '../../../../static/css/js-all.css';
// polyfill Promise support for IE11
import Promise from 'promise-polyfill';
Expand Down
24 changes: 24 additions & 0 deletions openlibrary/plugins/openlibrary/processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
import web

from infogami import config
from infogami.utils.context import context
from openlibrary.accounts import get_current_user
from openlibrary.core import helpers as h
from openlibrary.core.experiments import get_user_experiments
from openlibrary.core.processors import (
ReadableUrlProcessor, # noqa: F401 side effects may be needed
)
Expand Down Expand Up @@ -156,6 +158,28 @@ def __call__(self, handler):
return handler()


class ExperimentsProcessor:
"""Processor to evaluate A/B testing feature flags and inject them
into the global web.ctx so they are available to macros and templates.
"""

def __call__(self, handler):
user = get_current_user()
user_id = user.key if user else (web.cookies().get(config.get("login_cookie_name", "session")) or web.ctx.ip)
Comment thread
Sadashii marked this conversation as resolved.
Outdated

try:
overrides = {k: v for k, v in web.input(_method="GET").items() if k.startswith("experiment_")}
except (AttributeError, KeyError, ValueError):
overrides = {}

experiments = get_user_experiments(user_id, overrides=overrides, is_logged_in=user is not None)

web.ctx["experiments"] = experiments
context["experiments"] = experiments

return handler()


if __name__ == "__main__":
import doctest

Expand Down
5 changes: 5 additions & 0 deletions openlibrary/templates/site/head.html
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@
}
</style>
</noscript>

<script>
window.OL_EXPERIMENTS = $:dumps(ctx.get("experiments", {}));
</script>
Comment thread
Sadashii marked this conversation as resolved.

<script>
$if not is_bot() and not ctx.get('disable_analytics'):
// Dimension 1 (visit-scoped): "Days Since Registration" — ID must match Matomo admin config (issue #12144)
Expand Down
94 changes: 94 additions & 0 deletions openlibrary/tests/core/test_experiments.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
from openlibrary.core.experiments import ACTIVE_EXPERIMENTS, get_user_experiments, get_variant


def test_get_variant_fallback():
# Empty or None user identifier should fall back to control
assert get_variant("AB_Testing", "") == "control"
assert get_variant("AB_Testing", None) == "control"

# Non-existent experiment should fall back to control
assert get_variant("Non_Existent", "user123") == "control"


def test_get_variant_distribution():
# Evaluate distribution across a reasonable number of users
variants_seen = set()
for i in range(100):
variant = get_variant("AB_Testing", f"user_{i}")
assert variant in ACTIVE_EXPERIMENTS["AB_Testing"]["variants"]
variants_seen.add(variant)

# Ensure all variants are allocated at least once (probabilistically guaranteed for N=100)
assert len(variants_seen) > 1
assert "control" in variants_seen
assert "a" in variants_seen
assert "b" in variants_seen


def test_get_user_experiments():
experiments = get_user_experiments("user_456")
assert "AB_Testing" in experiments
assert experiments["AB_Testing"] in ACTIVE_EXPERIMENTS["AB_Testing"]["variants"]


def test_get_user_experiments_overrides():
# Valid override should change the variant
overrides = {"experiment_AB_Testing": "b"}
experiments = get_user_experiments("user_456", overrides=overrides)
assert experiments["AB_Testing"] == "b"

# Invalid override group/variant should be ignored
overrides = {"experiment_AB_Testing": "invalid_group"}
experiments = get_user_experiments("user_456", overrides=overrides)
assert experiments["AB_Testing"] != "invalid_group"
assert experiments["AB_Testing"] in ACTIVE_EXPERIMENTS["AB_Testing"]["variants"]

# Invalid override key/name format should be ignored
overrides = {"AB_Testing": "b"}
experiments = get_user_experiments("user_456", overrides=overrides)
assert experiments["AB_Testing"] in ACTIVE_EXPERIMENTS["AB_Testing"]["variants"]


def test_authorized_only_experiment():
ACTIVE_EXPERIMENTS["Auth_Only_Testing"] = {
"variants": {
"control": 25,
"treatment": 75,
},
"authorized_only": True,
}
try:
# If not logged in, should always fall back to control
for i in range(100):
assert get_variant("Auth_Only_Testing", f"user_{i}", is_logged_in=False) == "control"

# If logged in, should distribute into variants
variants_seen = set()
for i in range(100):
variant = get_variant("Auth_Only_Testing", f"user_{i}", is_logged_in=True)
assert variant in ["control", "treatment"]
variants_seen.add(variant)

assert len(variants_seen) > 1
finally:
ACTIVE_EXPERIMENTS.pop("Auth_Only_Testing", None)


def test_get_user_experiments_authorized_only():
ACTIVE_EXPERIMENTS["Auth_Only_Testing"] = {
"variants": {
"control": 25,
"treatment": 75,
},
"authorized_only": True,
}
try:
# If not logged in, should get control
experiments = get_user_experiments("user_456", is_logged_in=False)
assert experiments["Auth_Only_Testing"] == "control"

# If logged in, should evaluate variant
experiments_logged_in = get_user_experiments("user_456", is_logged_in=True)
assert experiments_logged_in["Auth_Only_Testing"] in ["control", "treatment"]
finally:
ACTIVE_EXPERIMENTS.pop("Auth_Only_Testing", None)