Skip to content

Commit c4c3318

Browse files
committed
Remove foot-gun per thread evaluattor fucntion.
1 parent b1137d5 commit c4c3318

3 files changed

Lines changed: 6 additions & 47 deletions

File tree

src/palace/manager/api/admin/controller/patron_auth_services.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
)
2424
from palace.manager.api.authentication.patron_blocking_rules.rule_engine import (
2525
RuleValidationError,
26-
get_evaluator,
26+
make_evaluator,
2727
validate_rule_expression,
2828
)
2929
from palace.manager.integration.goals import Goals
@@ -175,7 +175,7 @@ def process_validate_patron_blocking_rule(self) -> Response | ProblemDetail:
175175
# missing test_identifier, network error, or ILS error response.
176176
live_values = protocol_class.fetch_live_rule_validation_values(settings)
177177

178-
evaluator = get_evaluator()
178+
evaluator = make_evaluator()
179179
try:
180180
validate_rule_expression(rule_expr, live_values, evaluator)
181181
except RuleValidationError as exc:

src/palace/manager/api/authentication/patron_blocking_rules/rule_engine.py

Lines changed: 2 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
from __future__ import annotations
22

33
import re
4-
import threading
54
from collections.abc import Callable, Mapping
65
from dataclasses import dataclass, field
76
from datetime import date
@@ -196,45 +195,11 @@ def age_in_years(
196195
)
197196

198197

199-
_evaluator_local: threading.local = threading.local()
200-
201-
202-
def get_evaluator(
203-
allowed_functions: dict[str, Callable[..., Any]] | None = None,
204-
) -> EvalWithCompoundTypes:
205-
"""Return a per-thread evaluator for rule expression evaluation.
206-
207-
Admin validation and runtime blocking both use this so the evaluator
208-
lifecycle is unified. The evaluator is cached per thread and reused;
209-
:func:`validate_rule_expression` and :func:`evaluate_rule_expression_strict_bool`
210-
clear ``evaluator.names`` after each call so no data leaks between rules.
211-
212-
:param allowed_functions: Override the function whitelist. Pass an empty
213-
dict to disallow all functions. Only the first call per thread uses
214-
this; subsequent calls ignore it.
215-
:returns: A configured :class:`~simpleeval.EvalWithCompoundTypes`.
216-
"""
217-
evaluator = getattr(_evaluator_local, "evaluator", None)
218-
if evaluator is None:
219-
functions = (
220-
allowed_functions
221-
if allowed_functions is not None
222-
else dict(DEFAULT_ALLOWED_FUNCTIONS)
223-
)
224-
evaluator = EvalWithCompoundTypes(functions=functions, names={})
225-
_evaluator_local.evaluator = evaluator
226-
return _evaluator_local.evaluator
227-
228-
229198
def make_evaluator(
230199
allowed_functions: dict[str, Callable[..., Any]] | None = None,
231200
) -> EvalWithCompoundTypes:
232201
"""Create a fresh locked-down :class:`~simpleeval.EvalWithCompoundTypes`.
233202
234-
Prefer :func:`get_evaluator` for production use so admin and runtime share
235-
the same per-thread evaluator. Use this when you need an isolated
236-
evaluator (e.g. tests, or custom function sets).
237-
238203
:param allowed_functions: Override the function whitelist. Pass an empty
239204
dict to disallow all functions.
240205
:returns: A configured :class:`~simpleeval.EvalWithCompoundTypes`.
@@ -286,8 +251,7 @@ def validate_rule_expression(
286251
trial evaluation. For live validation this is the dict returned by
287252
``fetch_live_rule_validation_values``; it contains all raw SIP2
288253
response fields plus the normalised ``fines`` key.
289-
:param evaluator: A locked-down evaluator from :func:`get_evaluator` or
290-
:func:`make_evaluator`.
254+
:param evaluator: A locked-down evaluator from :func:`make_evaluator`.
291255
:raises RuleValidationError: On any validation failure.
292256
"""
293257
if not expr or not expr.strip():
@@ -344,8 +308,7 @@ def evaluate_rule_expression_strict_bool(
344308
345309
:param expr: The raw rule expression string (same as stored).
346310
:param values: Mapping of placeholder key → runtime value.
347-
:param evaluator: A locked-down evaluator from :func:`get_evaluator` or
348-
:func:`make_evaluator`.
311+
:param evaluator: A locked-down evaluator from :func:`make_evaluator`.
349312
:param rule_name: Optional identifier for the rule, included in error messages.
350313
:returns: ``True`` if the patron should be *blocked*, ``False`` otherwise.
351314
:raises RuleEvaluationError: On missing placeholders, parse/eval errors, or

src/palace/manager/integration/patron_auth/patron_blocking.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
from palace.manager.api.authentication.patron_blocking_rules.rule_engine import (
2121
RuleEvaluationError,
2222
evaluate_rule_expression_strict_bool,
23-
get_evaluator,
23+
make_evaluator,
2424
)
2525
from palace.manager.api.problem_details import BLOCKED_BY_POLICY
2626
from palace.manager.util import MoneyUtility
@@ -124,10 +124,6 @@ def check_patron_blocking_rules_with_evaluator(
124124
evaluation continues with the next rule. Only rules that successfully
125125
evaluate to ``True`` cause a block.
126126
127-
Uses a per-thread evaluator from :func:`~palace.manager.api.authentication
128-
.patron_blocking_rules.rule_engine.get_evaluator` so the function is safe
129-
to call from concurrent requests.
130-
131127
:param rules: The list of :class:`PatronBlockingRule` objects for the library.
132128
:param values: Runtime placeholder values; for SIP2 providers this is the
133129
full raw SIP2 response dict (see :func:`build_values_from_sip2_info`).
@@ -136,7 +132,7 @@ def check_patron_blocking_rules_with_evaluator(
136132
(HTTP 403) if the patron should be blocked, or ``None`` if
137133
authentication should proceed normally.
138134
"""
139-
evaluator = get_evaluator()
135+
evaluator = make_evaluator()
140136

141137
for rule in rules:
142138
try:

0 commit comments

Comments
 (0)