Skip to content
Open
Show file tree
Hide file tree
Changes from 9 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
105 changes: 105 additions & 0 deletions openlibrary/core/experiments.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
import hashlib
from typing import Final, Literal, TypedDict


class Audience:
"""Constants for experiment audience targeting."""

ALL: Final = "all"
LOGGED_IN: Final = "logged_in"
LOGGED_OUT: Final = "logged_out"


class ExperimentConfig(TypedDict, total=False):
"""Type schema for autocomplete and static analysis."""

variants: dict[str, int]
audience: Literal["all", "logged_in", "logged_out"]


ACTIVE_EXPERIMENTS: dict[str, ExperimentConfig] = {
"Sample_Experiment_1": {
"variants": {"control": 30, "a": 35, "b": 35},
"audience": Audience.ALL,
},
"Sample_Experiment_2": {
"variants": {"control": 50, "treatment": 50},
"audience": Audience.LOGGED_OUT,
},
"Sample_Experiment_3": {
"variants": {"control": 50, "treatment": 50},
"audience": Audience.LOGGED_IN,
},
}


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

config = ACTIVE_EXPERIMENTS[experiment_name]
audience = config.get("audience", Audience.ALL)

if audience == Audience.LOGGED_IN and not is_logged_in:
return "control"
if audience == Audience.LOGGED_OUT and is_logged_in:
return "control"

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 config.get("variants", {}).items():
Comment thread
Sadashii marked this conversation as resolved.
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, handling optional overrides."""
overrides = overrides or {}
results = {}

for exp_name, config in ACTIVE_EXPERIMENTS.items():
override_val = overrides.get(f"experiment_{exp_name}")

if override_val in config.get("variants", {}):
results[exp_name] = override_val
else:
results[exp_name] = get_variant(exp_name, user_identifier, is_logged_in)

return results


def evaluate_experiments_for_request(
session_value: str | None,
forwarded_for: str | None,
client_ip: str | None,
query_params: dict[str, str],
) -> dict[str, str]:
"""Resolves active experiments for a request based on session, IP, and query params."""
from openlibrary.accounts.model import verify_session_cookie

is_authenticated = False
user_id = None

if session_value and session_value.startswith("/people/") and verify_session_cookie(session_value):
is_authenticated = True
user_id = session_value.split(",")[0]

if not user_id:
if forwarded_for:
user_id = forwarded_for.split(",")[0].strip()
else:
user_id = client_ip or "127.0.0.1"

query_overrides = {k: v for k, v in query_params.items() if k.startswith("experiment_")}

return get_user_experiments(user_id, overrides=query_overrides, is_logged_in=is_authenticated)
37 changes: 37 additions & 0 deletions openlibrary/fastapi/middleware/experiments.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
from __future__ import annotations

from typing import TYPE_CHECKING

from fastapi import Request

from infogami import config
from openlibrary.core.experiments import evaluate_experiments_for_request

if TYPE_CHECKING:
from starlette.types import ASGIApp, Receive, Scope, Send


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_value = request.cookies.get(config.get("login_cookie_name", "session"))
forwarded_for = request.headers.get("X-Forwarded-For")
client_ip = request.client.host if request.client else "127.0.0.1"
query_params = dict(request.query_params)

request.state.experiments = evaluate_experiments_for_request(
session_value=session_value,
forwarded_for=forwarded_for,
client_ip=client_ip,
query_params=query_params,
)

await self.app(scope, receive, send)
16 changes: 13 additions & 3 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 Expand Up @@ -1081,6 +1086,11 @@ def is_recognized_bot():
return req_context.get().is_recognized_bot


def htmlsafe_dumps(obj, **kwargs) -> str:
"""Escapes HTML special characters in JSON to prevent XSS in template script tags."""
return json.dumps(obj, **kwargs).replace("<", "\\u003c").replace(">", "\\u003e").replace("&", "\\u0026")

Comment thread
Sadashii marked this conversation as resolved.
Outdated

def setup_template_globals():
# must be imported here, otherwise silently messes up infogami's import execution
# order, resulting in random errors like the the /account/login.json endpoint
Expand Down Expand Up @@ -1137,7 +1147,7 @@ def get_supported_languages():
"is_bot": is_bot,
"time": time,
"input": web.input,
"dumps": json.dumps,
"dumps": htmlsafe_dumps,
}
)

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
30 changes: 30 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 evaluate_experiments_for_request
from openlibrary.core.processors import (
ReadableUrlProcessor, # noqa: F401 side effects may be needed
)
Expand Down Expand Up @@ -156,6 +158,34 @@ 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):
session_value = web.cookies().get(config.get("login_cookie_name", "session"))
forwarded_for = web.ctx.env.get("HTTP_X_FORWARDED_FOR")
client_ip = web.ctx.ip

try:
query_params = dict(web.input(_method="GET"))
except AttributeError, KeyError, ValueError:
Comment thread
Sadashii marked this conversation as resolved.
query_params = {}

experiments = evaluate_experiments_for_request(
session_value=session_value,
forwarded_for=forwarded_for,
client_ip=client_ip,
query_params=query_params,
)

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

return handler()


if __name__ == "__main__":
import doctest

Expand Down
2 changes: 1 addition & 1 deletion openlibrary/plugins/upstream/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ def commify_list(items: Iterable[Any]) -> str:

@public
def json_encode(d, indent=0) -> str:
return json.dumps(d, indent=indent)
return json.dumps(d, indent=indent).replace("<", "\\u003c").replace(">", "\\u003e").replace("&", "\\u0026")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems kind of hacky.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure of simpler ways, do you have any recommendations?



@public
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
Loading