A pasted album is taken through the real irc_loop, not only on paper (#897)

docs/UPDATES-PUBLIC.md

@@ -4,6 +4,7 @@
 
 - **Added (mIRC console): the @DCCore window's button now lights up like a channel's.** It turns the message colour (red, by default) when there is new activity - a request, a send, a search - and the highlight colour on a failed transfer, so failures stand out. The five-minute status line and joins, parts and bans do not light it, as they would not in a channel. Needs mIRC 7 or later; update `dccore.mrc` and reload it.
 - **Fixed: pasting a batch of requests could mute you and then ban you for an hour, and the mute notice wrongly said your queue had been cleared.** Asking for a file counted toward the flood protection exactly like a search, so pasting about a dozen rows from the list - one album, the ordinary way these lists are used - could get the eleventh line muted and the twelfth banned for an hour, on any client or connection that sends a paste quickly. Asking for files is no longer rate-limited at all: the limit is your queue, as it always should have been (100 files each by default), and past it you are told once rather than once per line. Searches and the other commands are unchanged. The mute notice was also wrong in a way that caused the ban - what it drops is the bot's pending replies to you, never your queued files - so it now says that other commands are ignored for the next 30 seconds and that your queued files are safe.
+- **Fixed: pasting a batch of requests could mute you and then ban you for an hour, and the mute notice wrongly said your queue had been cleared.** Asking for a file counted toward the flood protection exactly like a search, so pasting about a dozen rows from the list - one album, the ordinary way these lists are used - could get the eleventh line muted and the twelfth banned for an hour, on any client or connection that sends a paste quickly. Asking for files is no longer rate-limited at all: the limit is your queue, as it always should have been (100 files each by default), and past it you are told once rather than once per line. Searches and the other commands are unchanged. The mute notice was also wrong in a way that caused the ban - what it drops is the bot's pending replies to you, never your queued files - so it now says that other commands are ignored for the next 30 seconds and that your queued files are safe. The Anti-flood setting that used to count them is now labelled *Max commands per window*, and its help says file requests are not counted.
 - **Fixed: pasting several lines from the list at once could be answered "Busy looking up other files - try again in a moment".** Requesting a file meant searching the library for it every single time - even for a file just sent, and even for the file next to it in the same album - and only two of those searches could run at once, so most of a pasted batch was refused outright (and "try again in a moment" is the one thing that risks tripping the flood protection). The bot now remembers where it found a file and which folders it has been finding them in, so a batch from one album costs one search instead of nine, and a request that arrives while the library is busy waits its turn instead of being turned away. What it remembers is dropped on `!rehash`, so changing your library folders takes effect for requests straight away rather than a few minutes later.
 - **A download that fails because your client never answered now says so, to the person downloading.** They used to be told only "transfer did not complete", which names nothing they can act on - while the two causes are both on their side: a DCC prompt nobody accepted in time, or a client set to ignore that kind of file (a user who could take a `.jpg` but never a `.nfo` is the usual shape). The notice now says *your client never accepted it*, and what to try.
 

docs/UPDATES.md

@@ -76,6 +76,16 @@ path, deliberately, so it asserts instead that every dispatched message is meter
 exemption, that the exemption covers the file requests, and that it covers nothing else - a second unmetered
 path added later still fails there.
 
+Completed afterwards. #896: what the operator is told `MAX_REQUESTS` counts still said file requests - its
+`defaults.py` comment, the dashboard help in three languages, the *Max requests per window* label (now *Max
+commands per window*: "requests" read as exactly the thing no longer counted), FUTURE.md's anti-flood line and
+the sample. #897: nothing executed the gate's own `if` - with `not is_file_request` taken back out of it the
+whole suite stayed green (6414 OK), since the tests above read the exemption expression and the wording rather
+than run a paste. `tests/test_a_pasted_album_is_taken_through_the_real_loop.py` (5) drives `irc_loop()` with
+one - thirty file rows and fifteen `!rar` rows all dispatched with no mute or ban, searches still muted then
+banned, rows during a search-earned mute served without escalating it, a ban still refusing them - and three of
+the five fail with the exemption removed.
+
 ### 📋 A batch of requests pasted from the list is served, not refused (#886)
 
 Reported live, from the same evening as #879/#884: a user pasted nine request lines in about seven seconds -

tests/test_a_pasted_album_is_taken_through_the_real_loop.py

@@ -0,0 +1,112 @@
+"""A pasted album is taken through the real irc_loop, not only on paper (#888).
+
+#894 exempted file requests from the flood gate and tested the pieces: the
+exemption expression lifted out of irc.py, the mute notice's wording, the
+file queue left untouched, and the queue-full notice said once. What nothing
+executed was the gate's own `if` - `is_bot_command and not is_file_request
+and security.is_flooding(user)`. With `not is_file_request` taken back out of
+that line the whole suite stayed green (6414 OK, checked on main), so the
+behaviour the operator asked for - "bot should just add them to queue one by
+one as he requests" - had no test that would notice it going.
+
+These drive irc_loop() against a scripted server, the #789 harness: a paste
+of rows arrives as channel lines, the loop's threads are recorded, and the
+mute and ban tables are read afterwards.
+"""
+
+import os
+import sys
+import time
+import unittest
+
+REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+if REPO_ROOT not in sys.path:
+    sys.path.insert(0, REPO_ROOT)
+
+import dcc  # noqa: E402
+import defaults as config  # noqa: E402
+import irc  # noqa: E402
+
+from tests import test_the_bots_own_nick_follows_the_server as own  # noqa: E402
+from tests.test_what_a_user_typed_reaches_the_log_printable import _Records  # noqa: E402
+
+CHANNEL = "#somechannel"
+BOT = "SomeBot"
+
+
+class APasteInTheChannel(own.DrivesPastRegistration):
+
+    def setUp(self):
+        super().setUp()
+        self.set_config(MAX_REQUESTS=10, REQUEST_WINDOW=5, MUTE_TIME=30,
+                        FLOOD_BAN_SECONDS=3600)
+        for table in (config.muted_until, config.banned_users, config.user_requests):
+            table.clear()
+            self.addCleanup(table.clear)
+        _Records.started = []
+        irc.threading.Thread = _Records
+
+    def line(self, nick, text):
+        return ":%s!~u@%s.example PRIVMSG %s :%s" % (nick, nick.lower(), CHANNEL, text)
+
+    def paste(self, nick, rows):
+        self.oserve.queued[:] = []
+        self.run_registration(own.NOTICE_AUTH, own.welcome(BOT),
+                              *[self.line(nick, row) for row in rows])
+
+    def requests_started(self, nick):
+        return [args for target, args in _Records.started
+                if target is dcc.handle_download_request and args[1] == nick]
+
+    def warnings_to(self, nick):
+        return [m for _u, m, *_ in self.oserve.queued
+                if m.startswith("NOTICE %s :" % nick) and "moving too fast" in m]
+
+    def test_thirty_request_rows_are_all_taken_and_nobody_is_punished(self):
+        rows = ["!%s Some Band - Track %02d.flac" % (BOT, n) for n in range(1, 31)]
+
+        self.paste("mara", rows)
+
+        self.assertEqual(len(self.requests_started("mara")), 30,
+                         "a pasted row was dropped by the flood gate")
+        self.assertNotIn("mara", config.muted_until)
+        self.assertNotIn("mara", config.banned_users)
+        self.assertEqual(self.warnings_to("mara"), [])
+
+    def test_folder_requests_are_taken_the_same_way(self):
+        rows = ["!%s !rar Some Band\\Album %02d\\" % (BOT, n) for n in range(1, 16)]
+
+        self.paste("mara", rows)
+
+        self.assertEqual(len(self.requests_started("mara")), 15)
+        self.assertNotIn("mara", config.banned_users)
+
+    def test_searches_are_still_metered(self):
+        """Everything that is not a file request meets the gate as before."""
+        self.paste("tobi", ["@find some band %d" % n for n in range(12)])
+
+        self.assertIn("tobi", config.banned_users,
+                      "an 11th search mutes, a 12th during the mute bans - as before")
+
+    def test_requests_during_a_mute_are_taken_and_never_escalate_it(self):
+        """Muted for flooding searches, then pasting rows: the rows are
+        served and the mute does not become a ban because of them."""
+        rows = ["@find x %d" % n for n in range(11)]
+        rows += ["!%s Some Band - Track %02d.flac" % (BOT, n) for n in range(1, 6)]
+
+        self.paste("lin", rows)
+
+        self.assertIn("lin", config.muted_until, "the 11th search muted them")
+        self.assertNotIn("lin", config.banned_users, "a request line escalated the mute")
+        self.assertEqual(len(self.requests_started("lin")), 5)
+
+    def test_a_ban_still_refuses_requests(self):
+        config.banned_users["ivo"] = time.time() + 3600
+
+        self.paste("ivo", ["!%s Some Band - Track 01.flac" % BOT])
+
+        self.assertEqual(self.requests_started("ivo"), [])
+
+
+if __name__ == "__main__":
+    unittest.main()