Skip to content

Commit 9554798

Browse files
test(rate-limiter): rename "RateLimiter" -> "RateLimiterPlugin" in test fixtures
Aligns the test fixtures' plugin name with the gateway's own plugins/config.yaml convention (which has used "RateLimiterPlugin" since the rate-limiter was added). The plugin reads its mode key as ``plugin:<self.config.name>:mode`` — so whatever name the test passes at construction is what the test expects to see in Redis. Before this commit, the test fixtures constructed plugins with name="RateLimiter" while production deploys them with name="RateLimiterPlugin", which silently created two disjoint mode-key namespaces and made it possible for an end-to-end test (mcp-context-forge#4511 layer) to publish to one namespace while the running gateway's plugin read the other. Found while empirically exercising the wipe-on-disable convergence path on a multi-replica stack — the wipe never fired because the test's PUBLISH used "RateLimiter" and the gateway's plugin looked up "RateLimiterPlugin". No code change to the rate-limiter plugin itself: ``self.config.name`` is just a string that gets reflected into the mode key, and the plugin works equally well with either value as long as the publisher and subscriber agree. The rename is purely about fixture-vs-production consistency. Touches every PluginConfig construction and every _set_plugin_mode_key call in test_redis_integration.py — bulk find/ replace, no behavioural change to any individual test. Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com>
1 parent 70269c0 commit 9554798

1 file changed

Lines changed: 30 additions & 30 deletions

File tree

plugins/tests/rate_limiter/test_redis_integration.py

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
def rate_limit_plugin_2_per_second():
4747
"""Rate limiter plugin configured for 2 requests per second."""
4848
config = PluginConfig(
49-
name="RateLimiter",
49+
name="RateLimiterPlugin",
5050
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
5151
hooks=["prompt_pre_fetch", "tool_pre_invoke"],
5252
priority=100,
@@ -59,7 +59,7 @@ def rate_limit_plugin_2_per_second():
5959
def rate_limit_plugin_multi_dimensional():
6060
"""Rate limiter plugin with multi-dimensional limits."""
6161
config = PluginConfig(
62-
name="RateLimiter",
62+
name="RateLimiterPlugin",
6363
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
6464
hooks=["prompt_pre_fetch", "tool_pre_invoke"],
6565
priority=100,
@@ -228,7 +228,7 @@ async def test_user_rate_limit_enforced(self):
228228
"""Verify user rate limits are enforced independently per user."""
229229
# Configure with ONLY user limits (no tenant limit)
230230
config = PluginConfig(
231-
name="RateLimiter",
231+
name="RateLimiterPlugin",
232232
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
233233
hooks=["prompt_pre_fetch"],
234234
priority=100,
@@ -303,7 +303,7 @@ async def test_most_restrictive_dimension_selected(self):
303303
"""Verify most restrictive dimension is selected."""
304304
# Configure with different limits
305305
config = PluginConfig(
306-
name="RateLimiter",
306+
name="RateLimiterPlugin",
307307
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
308308
hooks=["prompt_pre_fetch"],
309309
priority=100,
@@ -372,7 +372,7 @@ class TestSlidingWindowIntegration:
372372
@pytest.fixture
373373
def plugin(self):
374374
config = PluginConfig(
375-
name="RateLimiter",
375+
name="RateLimiterPlugin",
376376
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
377377
hooks=["prompt_pre_fetch", "tool_pre_invoke"],
378378
priority=100,
@@ -461,7 +461,7 @@ class TestTokenBucketIntegration:
461461
@pytest.fixture
462462
def plugin(self):
463463
config = PluginConfig(
464-
name="RateLimiter",
464+
name="RateLimiterPlugin",
465465
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
466466
hooks=["prompt_pre_fetch", "tool_pre_invoke"],
467467
priority=100,
@@ -554,7 +554,7 @@ class TestCrossHookSharing:
554554
@pytest.fixture
555555
def plugin(self):
556556
config = PluginConfig(
557-
name="RateLimiter",
557+
name="RateLimiterPlugin",
558558
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
559559
hooks=["prompt_pre_fetch", "tool_pre_invoke"],
560560
priority=100,
@@ -602,7 +602,7 @@ async def test_remaining_count_decrements_across_hooks(self, plugin):
602602
async def test_tenant_counter_shared_across_hooks_and_users(self, plugin):
603603
"""Tenant bucket is shared across all users in the same tenant, regardless of hook."""
604604
config = PluginConfig(
605-
name="RateLimiter",
605+
name="RateLimiterPlugin",
606606
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
607607
hooks=["prompt_pre_fetch", "tool_pre_invoke"],
608608
priority=100,
@@ -644,7 +644,7 @@ class TestTenantIsolation:
644644
@pytest.fixture
645645
def plugin(self):
646646
config = PluginConfig(
647-
name="RateLimiter",
647+
name="RateLimiterPlugin",
648648
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
649649
hooks=["tool_pre_invoke"],
650650
priority=100,
@@ -732,7 +732,7 @@ async def test_none_tenant_id_skips_by_tenant_entirely(self):
732732
Uses a high by_user limit so only by_tenant could trigger a block.
733733
"""
734734
config = PluginConfig(
735-
name="RateLimiter",
735+
name="RateLimiterPlugin",
736736
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
737737
hooks=["tool_pre_invoke"],
738738
priority=100,
@@ -778,7 +778,7 @@ async def test_explicit_tenant_scopes_correctly_after_fix(self):
778778
Uses a high by_user limit so only by_tenant can trigger a block.
779779
"""
780780
config = PluginConfig(
781-
name="RateLimiter",
781+
name="RateLimiterPlugin",
782782
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
783783
hooks=["tool_pre_invoke"],
784784
priority=100,
@@ -807,7 +807,7 @@ class TestNoLimitsAndMissingContext:
807807
async def test_no_limits_configured_allows_all_requests(self):
808808
"""Plugin with all dimensions None must allow every request without tracking."""
809809
config = PluginConfig(
810-
name="RateLimiter",
810+
name="RateLimiterPlugin",
811811
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
812812
hooks=["tool_pre_invoke"],
813813
config={}, # no by_user, no by_tenant, no by_tool
@@ -824,7 +824,7 @@ async def test_no_limits_configured_allows_all_requests(self):
824824
async def test_no_limits_configured_returns_no_headers(self):
825825
"""Plugin with no configured limits must not set X-RateLimit-* headers."""
826826
config = PluginConfig(
827-
name="RateLimiter",
827+
name="RateLimiterPlugin",
828828
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
829829
hooks=["tool_pre_invoke"],
830830
config={},
@@ -840,7 +840,7 @@ async def test_no_limits_configured_returns_no_headers(self):
840840
async def test_both_user_and_tenant_none_still_enforces(self):
841841
"""With both user=None and tenant_id=None the plugin must still enforce limits."""
842842
config = PluginConfig(
843-
name="RateLimiter",
843+
name="RateLimiterPlugin",
844844
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
845845
hooks=["tool_pre_invoke"],
846846
config={"by_user": "2/s", "by_tenant": "10/s"},
@@ -862,7 +862,7 @@ async def test_separate_plugin_instances_have_independent_stores(self):
862862
def make_plugin():
863863
return RateLimiterPlugin(
864864
PluginConfig(
865-
name="RateLimiter",
865+
name="RateLimiterPlugin",
866866
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
867867
hooks=["tool_pre_invoke"],
868868
config={"by_user": "2/s"},
@@ -1017,7 +1017,7 @@ def _make_redis_plugin(redis_url: str, algorithm: str = "fixed_window", limit: s
10171017
"""Create a RateLimiterPlugin backed by real Redis."""
10181018
return RateLimiterPlugin(
10191019
PluginConfig(
1020-
name="RateLimiter",
1020+
name="RateLimiterPlugin",
10211021
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
10221022
hooks=["tool_pre_invoke"],
10231023
priority=100,
@@ -1387,7 +1387,7 @@ def _make_redis_plugin_with_config(redis_url: str, extra_config: dict) -> RateLi
13871387
base.update(extra_config)
13881388
return RateLimiterPlugin(
13891389
PluginConfig(
1390-
name="RateLimiter",
1390+
name="RateLimiterPlugin",
13911391
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
13921392
hooks=["tool_pre_invoke"],
13931393
priority=100,
@@ -1419,7 +1419,7 @@ async def test_redis_unreachable_default_fail_open_logs_warning(self, caplog):
14191419

14201420
plugin = RateLimiterPlugin(
14211421
PluginConfig(
1422-
name="RateLimiter",
1422+
name="RateLimiterPlugin",
14231423
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
14241424
hooks=["tool_pre_invoke"],
14251425
priority=100,
@@ -1557,7 +1557,7 @@ async def test_unknown_config_key_emits_warning(self, redis_url_for_integration,
15571557
with caplog.at_level(logging.WARNING):
15581558
RateLimiterPlugin(
15591559
PluginConfig(
1560-
name="RateLimiter",
1560+
name="RateLimiterPlugin",
15611561
kind="cpex_rate_limiter.rate_limiter.RateLimiterPlugin",
15621562
hooks=["tool_pre_invoke"],
15631563
priority=100,
@@ -1640,7 +1640,7 @@ async def test_wipe_fires_when_mode_key_says_disabled(self, redis_url_for_integr
16401640
"""mode='disabled' in Redis → all rl:* keys are deleted on shutdown."""
16411641
await _flush_redis(redis_url_for_integration)
16421642
await _seed_rate_limit_counters(redis_url_for_integration, count=5)
1643-
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiter", "disabled")
1643+
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiterPlugin", "disabled")
16441644

16451645
keys_before = await _keys_in_redis(redis_url_for_integration, "rl:*")
16461646
assert len(keys_before) == 5, "seeding should have produced 5 rl:* keys"
@@ -1659,7 +1659,7 @@ async def test_no_wipe_when_mode_key_says_enforce(self, redis_url_for_integratio
16591659
"""mode='enforce' in Redis → counters survive shutdown (config-only edit)."""
16601660
await _flush_redis(redis_url_for_integration)
16611661
await _seed_rate_limit_counters(redis_url_for_integration, count=3)
1662-
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiter", "enforce")
1662+
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiterPlugin", "enforce")
16631663

16641664
plugin = _make_redis_plugin(redis_url_for_integration)
16651665
await plugin.shutdown()
@@ -1676,7 +1676,7 @@ async def test_no_wipe_when_mode_key_absent(self, redis_url_for_integration):
16761676
await _flush_redis(redis_url_for_integration)
16771677
await _seed_rate_limit_counters(redis_url_for_integration, count=3)
16781678
# Explicitly ensure the mode key does not exist (fresh-deploy / pod-restart shape).
1679-
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiter", None)
1679+
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiterPlugin", None)
16801680

16811681
plugin = _make_redis_plugin(redis_url_for_integration)
16821682
await plugin.shutdown()
@@ -1704,7 +1704,7 @@ async def test_wipe_is_idempotent_under_concurrent_shutdown(self, redis_url_for_
17041704
"""Multiple plugin instances calling shutdown() in parallel all succeed and converge."""
17051705
await _flush_redis(redis_url_for_integration)
17061706
await _seed_rate_limit_counters(redis_url_for_integration, count=10)
1707-
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiter", "disabled")
1707+
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiterPlugin", "disabled")
17081708

17091709
plugins = [_make_redis_plugin(redis_url_for_integration) for _ in range(5)]
17101710

@@ -1731,7 +1731,7 @@ async def test_core_shutdown_runs_even_when_wipe_raises(self, redis_url_for_inte
17311731
TestRedisLifecycle.test_shutdown_releases_redis_connection.
17321732
"""
17331733
await _flush_redis(redis_url_for_integration)
1734-
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiter", "disabled")
1734+
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiterPlugin", "disabled")
17351735

17361736
plugin = _make_redis_plugin(redis_url_for_integration)
17371737
ctx = PluginContext(global_context=GlobalContext(request_id="r1", user="alice"))
@@ -1769,7 +1769,7 @@ async def test_wipe_skipped_when_another_worker_holds_lock(self, redis_url_for_i
17691769
"""
17701770
await _flush_redis(redis_url_for_integration)
17711771
await _seed_rate_limit_counters(redis_url_for_integration, count=5)
1772-
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiter", "disabled")
1772+
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiterPlugin", "disabled")
17731773

17741774
# Pre-acquire the wipe lock externally — pretend another worker is
17751775
# already in the middle of wiping. Default redis_key_prefix is "rl",
@@ -1816,7 +1816,7 @@ async def test_wipe_only_deletes_keys_under_configured_prefix(self, redis_url_fo
18161816
finally:
18171817
await client.aclose()
18181818

1819-
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiter", "disabled")
1819+
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiterPlugin", "disabled")
18201820

18211821
plugin = _make_redis_plugin_with_config(
18221822
redis_url_for_integration,
@@ -1880,7 +1880,7 @@ async def test_full_toggle_journey_enforce_disable_reenforce(self, redis_url_for
18801880
)
18811881

18821882
# ── Phase 2: operator toggles to disabled — wipe must fire ──────
1883-
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiter", "disabled")
1883+
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiterPlugin", "disabled")
18841884
await plugin.shutdown()
18851885

18861886
keys_post_wipe = await _keys_in_redis(redis_url_for_integration, "rl:*")
@@ -1893,7 +1893,7 @@ async def test_full_toggle_journey_enforce_disable_reenforce(self, redis_url_for
18931893
# Constructing a new plugin instance is what the framework's plugin
18941894
# manager does on re-enable; the manager adds nothing at construction
18951895
# beyond what _make_redis_plugin reproduces here.
1896-
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiter", "enforce")
1896+
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiterPlugin", "enforce")
18971897
plugin = _make_redis_plugin(redis_url_for_integration, limit="3/m")
18981898

18991899
r = await plugin.tool_pre_invoke(payload, ctx)
@@ -1965,7 +1965,7 @@ async def _saturate(plugin, ctx, user):
19651965
)
19661966

19671967
# ── Phase 2: parallel disable — exactly one wipes, all keys gone ──
1968-
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiter", "disabled")
1968+
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiterPlugin", "disabled")
19691969

19701970
results = await asyncio.gather(
19711971
*(p.shutdown() for p in plugins),
@@ -1985,7 +1985,7 @@ async def _saturate(plugin, ctx, user):
19851985
# ── Phase 3: re-enforce — every user gets a fresh window ──
19861986
# Fresh plugin instances mirror what each worker's plugin manager does
19871987
# on re-enable.
1988-
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiter", "enforce")
1988+
await _set_plugin_mode_key(redis_url_for_integration, "RateLimiterPlugin", "enforce")
19891989
fresh_plugins = [_make_redis_plugin(redis_url_for_integration, limit="3/m") for _ in users]
19901990

19911991
async def _first_request_passes(plugin, ctx, user):

0 commit comments

Comments
 (0)