diff --git a/openlibrary/asgi_app.py b/openlibrary/asgi_app.py index f79de38f0ad..4fd3f8cdcda 100644 --- a/openlibrary/asgi_app.py +++ b/openlibrary/asgi_app.py @@ -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 @@ -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.""" diff --git a/openlibrary/core/experiments.py b/openlibrary/core/experiments.py new file mode 100644 index 00000000000..e6dec9e72ce --- /dev/null +++ b/openlibrary/core/experiments.py @@ -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(): + 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) diff --git a/openlibrary/core/helpers.py b/openlibrary/core/helpers.py index f01ab6fa566..abfb96a89da 100644 --- a/openlibrary/core/helpers.py +++ b/openlibrary/core/helpers.py @@ -105,12 +105,12 @@ def default(self, obj): def json_encode(d, **kw) -> str: - """Calls json.dumps on the given data d. + """Calls json.dumps on the given data d and escapes HTML special characters to prevent XSS. If d is a Nothing object, passes an empty list to json.dumps. Returns the json.dumps results. """ - return json.dumps([] if isinstance(d, Nothing) else d, **kw) + return json.dumps([] if isinstance(d, Nothing) else d, **kw).replace("<", "\\u003c").replace(">", "\\u003e").replace("&", "\\u0026") def safesort(iterable: Iterable, key: Callable | None = None, reverse: bool = False) -> list: diff --git a/openlibrary/fastapi/middleware/experiments.py b/openlibrary/fastapi/middleware/experiments.py new file mode 100644 index 00000000000..ace039f4635 --- /dev/null +++ b/openlibrary/fastapi/middleware/experiments.py @@ -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) diff --git a/openlibrary/plugins/openlibrary/code.py b/openlibrary/plugins/openlibrary/code.py index cdf398523e8..5065d54eba9 100644 --- a/openlibrary/plugins/openlibrary/code.py +++ b/openlibrary/plugins/openlibrary/code.py @@ -84,6 +84,7 @@ def setup_contextvars(handler): ) ) delegate.app.add_processor(processors.PreferenceProcessor()) +delegate.app.add_processor(processors.ExperimentsProcessor()) # 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 @@ -1137,7 +1138,6 @@ def get_supported_languages(): "is_bot": is_bot, "time": time, "input": web.input, - "dumps": json.dumps, } ) diff --git a/openlibrary/plugins/openlibrary/js/experiments.js b/openlibrary/plugins/openlibrary/js/experiments.js new file mode 100644 index 00000000000..3f0fcde1e42 --- /dev/null +++ b/openlibrary/plugins/openlibrary/js/experiments.js @@ -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; +} diff --git a/openlibrary/plugins/openlibrary/js/index.js b/openlibrary/plugins/openlibrary/js/index.js index 2ef67c01142..3a1a37a4480 100644 --- a/openlibrary/plugins/openlibrary/js/index.js +++ b/openlibrary/plugins/openlibrary/js/index.js @@ -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'; diff --git a/openlibrary/plugins/openlibrary/processors.py b/openlibrary/plugins/openlibrary/processors.py index 4ab78f93355..d7d818a471d 100644 --- a/openlibrary/plugins/openlibrary/processors.py +++ b/openlibrary/plugins/openlibrary/processors.py @@ -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 ) @@ -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: + 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 diff --git a/openlibrary/plugins/upstream/utils.py b/openlibrary/plugins/upstream/utils.py index 3fe9e756754..80a8285878c 100644 --- a/openlibrary/plugins/upstream/utils.py +++ b/openlibrary/plugins/upstream/utils.py @@ -301,11 +301,6 @@ def commify_list(items: Iterable[Any]) -> str: return commified_list -@public -def json_encode(d, indent=0) -> str: - return json.dumps(d, indent=indent) - - @public def is_feature_enabled(feature_name: str) -> bool: return features.is_enabled(feature_name) diff --git a/openlibrary/templates/account/readinglog_stats.html b/openlibrary/templates/account/readinglog_stats.html index 0cd29f8e773..dea2efd03e4 100644 --- a/openlibrary/templates/account/readinglog_stats.html +++ b/openlibrary/templates/account/readinglog_stats.html @@ -121,7 +121,7 @@
$:_('Displaying stats about %(works_count)d books. Note all charts show only the top 20 bars. Note reading log stats are private.', works_count=works_count)
-