Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
16 changes: 10 additions & 6 deletions bot/exts/filtering/filtering.py
Original file line number Diff line number Diff line change
Expand Up @@ -1092,17 +1092,21 @@ def _should_run_actions(self, ctx: FilterContext, actions: ActionSettings) -> bo
break
del recent[oldest_key]

key = (
ctx.event.name,
# base_key ignores additional_actions so a events for the same user are caught
# even when antispam skips adding the deletion handler the second time.
base_key = (
getattr(ctx.author, "id", None),
getattr(ctx.channel, "id", None),
json.dumps(to_serializable(actions), sort_keys=True, default=str),
# So that each callable in additional_actions also forms the cache key
)
full_key = base_key + (
tuple(sorted(getattr(a, "__qualname__", repr(a)) for a in ctx.additional_actions)),
)
if key in recent:
if base_key in recent or full_key in recent:
log.info(f"Cache hit, not running actions {ctx.author} (event={ctx.event.name}): {actions}")
return False
recent[key] = now
log.info(f"Cache miss, running actions on {ctx.author} (event={ctx.event.name}): {actions}")
recent[base_key] = now
recent[full_key] = now
return True

def _increment_stats(self, triggered_filters: dict[AtomicList, list[Filter]]) -> None:
Expand Down
50 changes: 48 additions & 2 deletions tests/bot/exts/filtering/test_action_deduplication.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ def _make_actions(send_alert: bool) -> ActionSettings:
return actions

@staticmethod
def _make_context(*, author_id: int = 1, channel_id: int = 10) -> FilterContext:
def _make_context(*, author_id: int = 1, channel_id: int = 10, event: Event = Event.MESSAGE) -> FilterContext:
return FilterContext(
event=Event.MESSAGE,
event=event,
author=MockMember(id=author_id),
channel=MockTextChannel(id=channel_id),
content="hello",
Expand All @@ -42,6 +42,52 @@ def test_different_action_values_are_not_suppressed(self):
self.assertTrue(self.cog._should_run_actions(ctx, first_actions))
self.assertTrue(self.cog._should_run_actions(ctx, second_actions))

def test_same_action_payload_for_different_events_is_suppressed(self):
"""MESSAGE followed by MESSAGE_EDIT for the same user/channel/action should not double-ban."""
msg_ctx = FilterContext(
event=Event.MESSAGE,
author=MockMember(id=1),
channel=MockTextChannel(id=10),
content="hello",
message=None,
)
edit_ctx = FilterContext(
event=Event.MESSAGE_EDIT,
author=MockMember(id=1),
channel=MockTextChannel(id=10),
content="hello",
message=None,
)
actions = self._make_actions(send_alert=True)

self.assertTrue(self.cog._should_run_actions(msg_ctx, actions))
self.assertFalse(self.cog._should_run_actions(edit_ctx, actions))

def test_differing_additional_actions_are_still_suppressed(self):
"""Second event with no additional_actions (antispam already queued) should still be deduped."""

async def fake_handler(ctx: FilterContext) -> None:
pass

first_ctx = self._make_context()
first_ctx.additional_actions.append(fake_handler)

second_ctx = self._make_context() # no additional_actions

actions = self._make_actions(send_alert=True)

self.assertTrue(self.cog._should_run_actions(first_ctx, actions))
self.assertFalse(self.cog._should_run_actions(second_ctx, actions))

def test_same_action_payload_across_channels_is_suppressed(self):
"""Antispam is cross-channel; same user/action in a different channel should not double-ban."""
first_ctx = self._make_context(channel_id=10)
second_ctx = self._make_context(channel_id=20)
actions = self._make_actions(send_alert=True)

self.assertTrue(self.cog._should_run_actions(first_ctx, actions))
self.assertFalse(self.cog._should_run_actions(second_ctx, actions))

def test_same_action_payload_for_different_authors_is_not_suppressed(self):
first_ctx = self._make_context(author_id=1)
second_ctx = self._make_context(author_id=2)
Expand Down