mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-10-05 08:11:11 +01:00
Make IRC less spammy and require a bot name for conversatons (#1065)
First step towards fixing the friction created by shelfmark in #997
This commit is contained in:
@@ -1293,7 +1293,7 @@ Delay between requests in seconds to avoid rate limiting (0-10).
|
||||
| `IRC_USE_TLS` | Enable TLS/SSL encryption for the IRC connection. Disable for servers that don't support TLS. | boolean | `true` |
|
||||
| `IRC_CHANNEL` | Channel name without the # prefix | string | _none_ |
|
||||
| `IRC_NICK` | Your IRC nickname (required). Must be unique on the IRC network. | string | _none_ |
|
||||
| `IRC_SEARCH_BOT` | The search bot to query for results | string | _none_ |
|
||||
| `IRC_SEARCH_BOT` | The search bot to address queries to (required). | string | _none_ |
|
||||
| `IRC_CACHE_TTL` | How long to keep cached search results before they expire. | string (choice) | `2592000` |
|
||||
|
||||
<details>
|
||||
@@ -1351,10 +1351,11 @@ Your IRC nickname (required). Must be unique on the IRC network.
|
||||
|
||||
**Search bot**
|
||||
|
||||
The search bot to query for results
|
||||
The search bot to address queries to (required). Searches are sent as "@<bot> <query>". Without it, queries would be posted unaddressed to the channel.
|
||||
|
||||
- **Type:** string
|
||||
- **Default:** _none_
|
||||
- **Required:** Yes
|
||||
|
||||
#### `IRC_CACHE_TTL`
|
||||
|
||||
|
||||
@@ -88,7 +88,11 @@ def irc_settings() -> list[SettingsField]:
|
||||
key="IRC_SEARCH_BOT",
|
||||
label="Search bot",
|
||||
placeholder="e.g. search",
|
||||
description="The search bot to query for results",
|
||||
description=(
|
||||
"The search bot to address queries to (required). Searches are sent as "
|
||||
'"@<bot> <query>".'
|
||||
),
|
||||
required=True,
|
||||
env_supported=True,
|
||||
),
|
||||
HeadingField(
|
||||
|
||||
@@ -88,6 +88,13 @@ def _emit_status(message: str, phase: str = "searching") -> None:
|
||||
MIN_SEARCH_INTERVAL = 15.0
|
||||
_last_search_time: float = 0
|
||||
|
||||
# Per-query cooldown: never re-post an identical search to the channel within this
|
||||
# window, even when a user hits "Refresh" or retries a book that returned nothing.
|
||||
# This stops retry loops from flooding the channel with the same message over and over.
|
||||
# One day: results don't change minute-to-minute, so there's no reason to re-ask sooner.
|
||||
QUERY_COOLDOWN_SECONDS = 24 * 60 * 60 # 1 day
|
||||
_recent_query_times: dict[str, float] = {}
|
||||
|
||||
|
||||
def _enforce_rate_limit() -> None:
|
||||
"""Ensure minimum time between searches."""
|
||||
@@ -102,6 +109,31 @@ def _enforce_rate_limit() -> None:
|
||||
_last_search_time = time.time()
|
||||
|
||||
|
||||
def _query_cooldown_key(channel: str, query: str) -> str:
|
||||
"""Build a normalized key identifying a search posted to a channel."""
|
||||
return f"{channel.casefold()}:{query.casefold().strip()}"
|
||||
|
||||
|
||||
def _query_on_cooldown(key: str) -> bool:
|
||||
"""Return True if an identical query was posted to the channel recently."""
|
||||
last = _recent_query_times.get(key)
|
||||
if last is None:
|
||||
return False
|
||||
return (time.time() - last) < QUERY_COOLDOWN_SECONDS
|
||||
|
||||
|
||||
def _record_query_sent(key: str) -> None:
|
||||
"""Record that a query was just posted to the channel (for cooldown)."""
|
||||
now = time.time()
|
||||
_recent_query_times[key] = now
|
||||
# Opportunistically prune stale entries so the dict can't grow unbounded.
|
||||
stale = [
|
||||
k for k, sent_at in _recent_query_times.items() if now - sent_at > QUERY_COOLDOWN_SECONDS
|
||||
]
|
||||
for k in stale:
|
||||
_recent_query_times.pop(k, None)
|
||||
|
||||
|
||||
@register_source("irc")
|
||||
class IRCReleaseSource(ReleaseSource):
|
||||
"""Search IRC channels for ebook and audiobook releases."""
|
||||
@@ -117,11 +149,16 @@ class IRCReleaseSource(ReleaseSource):
|
||||
self._online_servers: set[str] | None = None
|
||||
|
||||
def is_available(self) -> bool:
|
||||
"""Check if IRC is configured (server, channel, and nick are set)."""
|
||||
"""Check if IRC is configured (server, channel, nick, and search bot are set).
|
||||
|
||||
The search bot is required: without it we would post bare queries straight
|
||||
to the channel, which reads as spam and gets the nick banned.
|
||||
"""
|
||||
server = _config_text("IRC_SERVER")
|
||||
channel = _config_text("IRC_CHANNEL")
|
||||
nick = _config_text("IRC_NICK")
|
||||
return bool(server and channel and nick)
|
||||
search_bot = _config_text("IRC_SEARCH_BOT")
|
||||
return bool(server and channel and nick and search_bot)
|
||||
|
||||
def get_column_config(self) -> ReleaseColumnConfig:
|
||||
"""Configure UI columns for IRC results."""
|
||||
@@ -193,11 +230,6 @@ class IRCReleaseSource(ReleaseSource):
|
||||
logger.warning("No search query could be built")
|
||||
return []
|
||||
|
||||
logger.info("IRC search: %s", query)
|
||||
|
||||
# Enforce rate limit
|
||||
_enforce_rate_limit()
|
||||
|
||||
# Get IRC settings
|
||||
server = _config_text("IRC_SERVER")
|
||||
port = _config_port("IRC_PORT", 6697)
|
||||
@@ -206,6 +238,34 @@ class IRCReleaseSource(ReleaseSource):
|
||||
nick = _config_text("IRC_NICK")
|
||||
search_bot = _config_text("IRC_SEARCH_BOT")
|
||||
|
||||
# Never post an unaddressed query to the channel. A bare book title looks like
|
||||
# spam to everyone else in the channel and gets the nick banned. Searches must
|
||||
# be addressed to a search bot ("@<bot> <query>").
|
||||
if not search_bot:
|
||||
logger.warning(
|
||||
"IRC search bot not configured; refusing to post unaddressed query to channel"
|
||||
)
|
||||
_emit_status("IRC search bot not configured", phase="error")
|
||||
return []
|
||||
|
||||
# Don't re-post an identical query to the channel within the cooldown window,
|
||||
# even on refresh. This is what stops a frustrated user (or a retry loop) from
|
||||
# spamming the same title over and over. Fall back to whatever is cached.
|
||||
cooldown_key = _query_cooldown_key(channel, query)
|
||||
if _query_on_cooldown(cooldown_key):
|
||||
logger.info("IRC query on cooldown, not re-posting to channel: %s", query)
|
||||
_emit_status("Search sent recently — showing latest results", phase="complete")
|
||||
cached = get_cached_results(book.provider, book.provider_id, content_type=content_type)
|
||||
if cached:
|
||||
self._online_servers = set(cached.get("online_servers", []))
|
||||
return cached["releases"]
|
||||
return []
|
||||
|
||||
logger.info("IRC search: %s", query)
|
||||
|
||||
# Enforce rate limit
|
||||
_enforce_rate_limit()
|
||||
|
||||
client = None
|
||||
try:
|
||||
# Get or reuse IRC connection
|
||||
@@ -221,9 +281,10 @@ class IRCReleaseSource(ReleaseSource):
|
||||
# Capture online servers (elevated users in channel)
|
||||
self._online_servers = client.online_servers
|
||||
|
||||
# Send search request
|
||||
search_msg = f"@{search_bot} {query}" if search_bot else query
|
||||
# Send search request (always addressed to the search bot, never bare)
|
||||
search_msg = f"@{search_bot} {query}"
|
||||
client.send_message(f"#{channel}", search_msg)
|
||||
_record_query_sent(cooldown_key)
|
||||
|
||||
# Wait for results DCC - this is the long wait
|
||||
_emit_status(f"Connected to #{channel} - Waiting for results...", phase="searching")
|
||||
|
||||
+102
-1
@@ -84,11 +84,27 @@ def test_search_no_dcc_offer_releases_connection_and_caches_empty_result(monkeyp
|
||||
self.channel = channel
|
||||
self.message = message
|
||||
|
||||
def wait_for_dcc(self, *, timeout: float, result_type: bool) -> None:
|
||||
def wait_for_dcc(
|
||||
self, *, timeout: float, result_type: bool, expected_senders: object = None
|
||||
) -> None:
|
||||
return None
|
||||
|
||||
client = FakeClient()
|
||||
|
||||
# A search bot is required; make config report one so the channel send path runs.
|
||||
monkeypatch.setattr(
|
||||
irc_source,
|
||||
"_config_text",
|
||||
lambda key: {
|
||||
"IRC_SERVER": "irc.example.net",
|
||||
"IRC_CHANNEL": "ebooks",
|
||||
"IRC_NICK": "tester",
|
||||
"IRC_SEARCH_BOT": "search",
|
||||
}.get(key, ""),
|
||||
)
|
||||
# Ensure no leftover cooldown entry from a previous test blocks the send.
|
||||
irc_source._recent_query_times.clear()
|
||||
|
||||
monkeypatch.setattr(source, "is_available", lambda: True)
|
||||
monkeypatch.setattr(irc_source, "_enforce_rate_limit", lambda: None)
|
||||
monkeypatch.setattr(irc_source, "_emit_status", lambda *_args, **_kwargs: None)
|
||||
@@ -137,3 +153,88 @@ def test_search_no_dcc_offer_releases_connection_and_caches_empty_result(monkeyp
|
||||
"online_servers": ["AudioBot"],
|
||||
}
|
||||
]
|
||||
|
||||
|
||||
def test_search_without_search_bot_never_posts_to_channel(monkeypatch):
|
||||
"""A bare (unaddressed) query must never reach the channel; refuse to connect."""
|
||||
import shelfmark.release_sources.irc.source as irc_source
|
||||
|
||||
source = IRCReleaseSource()
|
||||
|
||||
# Force is_available True so we exercise the in-search guard (defense in depth).
|
||||
monkeypatch.setattr(source, "is_available", lambda: True)
|
||||
monkeypatch.setattr(irc_source, "_emit_status", lambda *_args, **_kwargs: None)
|
||||
monkeypatch.setattr(irc_source, "_enforce_rate_limit", lambda: None)
|
||||
monkeypatch.setattr(
|
||||
"shelfmark.release_sources.irc.cache.get_cached_results",
|
||||
lambda provider, provider_id, *, content_type: None,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
irc_source,
|
||||
"_config_text",
|
||||
lambda key: {
|
||||
"IRC_SERVER": "irc.example.net",
|
||||
"IRC_CHANNEL": "ebooks",
|
||||
"IRC_NICK": "tester",
|
||||
"IRC_SEARCH_BOT": "", # not configured
|
||||
}.get(key, ""),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"shelfmark.release_sources.irc.connection_manager.connection_manager.get_connection",
|
||||
lambda **_kwargs: (_ for _ in ()).throw(
|
||||
AssertionError("must not connect/post without a search bot")
|
||||
),
|
||||
)
|
||||
|
||||
book = BookMetadata(provider="hardcover", provider_id="nobot", title="No Bot")
|
||||
plan = SimpleNamespace(primary_query="No Bot")
|
||||
|
||||
assert source.search(book, plan) == []
|
||||
|
||||
|
||||
def test_search_on_cooldown_returns_cache_without_reposting(monkeypatch):
|
||||
"""An identical query within the cooldown window must not be re-posted to the channel."""
|
||||
import shelfmark.release_sources.irc.source as irc_source
|
||||
|
||||
source = IRCReleaseSource()
|
||||
cached_release = Release(source="irc", source_id="cached-line", title="Cached Result")
|
||||
|
||||
monkeypatch.setattr(source, "is_available", lambda: True)
|
||||
monkeypatch.setattr(irc_source, "_emit_status", lambda *_args, **_kwargs: None)
|
||||
monkeypatch.setattr(irc_source, "_enforce_rate_limit", lambda: None)
|
||||
monkeypatch.setattr(
|
||||
irc_source,
|
||||
"_config_text",
|
||||
lambda key: {
|
||||
"IRC_SERVER": "irc.example.net",
|
||||
"IRC_CHANNEL": "ebooks",
|
||||
"IRC_NICK": "tester",
|
||||
"IRC_SEARCH_BOT": "search",
|
||||
}.get(key, ""),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"shelfmark.release_sources.irc.cache.get_cached_results",
|
||||
lambda provider, provider_id, *, content_type: {
|
||||
"releases": [cached_release],
|
||||
"online_servers": ["AudioBot"],
|
||||
},
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"shelfmark.release_sources.irc.connection_manager.connection_manager.get_connection",
|
||||
lambda **_kwargs: (_ for _ in ()).throw(
|
||||
AssertionError("cooldown should skip IRC connection")
|
||||
),
|
||||
)
|
||||
|
||||
# Pretend the same query was just posted to the channel.
|
||||
irc_source._recent_query_times.clear()
|
||||
irc_source._record_query_sent(irc_source._query_cooldown_key("ebooks", "Cooldown Book"))
|
||||
|
||||
book = BookMetadata(provider="hardcover", provider_id="cd", title="Cooldown Book")
|
||||
plan = SimpleNamespace(primary_query="Cooldown Book")
|
||||
|
||||
# expand_search=True bypasses the normal top-level cache, forcing the cooldown path.
|
||||
releases = source.search(book, plan, expand_search=True)
|
||||
|
||||
assert releases == [cached_release]
|
||||
assert source._online_servers == {"AudioBot"}
|
||||
|
||||
Reference in New Issue
Block a user