diff --git a/cmd/server/command_menu_test.go b/cmd/server/command_menu_test.go index c1189e2..8c4f816 100644 --- a/cmd/server/command_menu_test.go +++ b/cmd/server/command_menu_test.go @@ -75,12 +75,14 @@ func TestCommandDiscovery_AllPublicCommandsHaveSafeMetadata(t *testing.T) { "addsticker": "[emoji...]", "alias": "", "aliases": "", + "blacklist": "[text...]", "blacklist_add": "[text...]", "blacklist_del": "", "blacklist_rules": "", "blacklist_check": "", "whitelist_add": "[text...]", "whitelist_del": "", + "whitelist_rnd": "", "unalias": "", "insert": "", "stats": "[users | user | cmd ]", diff --git a/cmd/server/main_test.go b/cmd/server/main_test.go index 59a7d37..3b40077 100644 --- a/cmd/server/main_test.go +++ b/cmd/server/main_test.go @@ -139,15 +139,15 @@ func TestFactoriesRegistersBlacklistCommands(t *testing.T) { t.Fatalf("Build blacklist: %v", err) } for _, name := range []string{ - "blacklist_add", "blacklist_del", "blacklist_rules", "blacklist_check", - "whitelist_add", "whitelist_del", + "blacklist", "blacklist_add", "blacklist_del", "blacklist_rules", + "blacklist_check", "whitelist_add", "whitelist_del", "whitelist_rnd", } { if _, ok := reg.AllCommands[name]; !ok { t.Fatalf("missing command %s", name) } } - if got := len(reg.AllCommands); got != 6 { - t.Fatalf("blacklist registered %d commands, want 6", got) + if got := len(reg.AllCommands); got != 8 { + t.Fatalf("blacklist registered %d commands, want 8", got) } } diff --git a/docs/blacklist.md b/docs/blacklist.md index 4f21c2e..568a1f9 100644 --- a/docs/blacklist.md +++ b/docs/blacklist.md @@ -5,15 +5,24 @@ exceptions, then ask whether a given text is blocked. | Command | Parameters | What it does | |---|---|---| +| `/blacklist` | `[text...]` | Bare, lists both lists; with text, judges it | | `/blacklist_add` | `[text...]` | Adds text to the blacklist, or the message you replied to | | `/blacklist_del` | `` | Removes text from the blacklist | | `/whitelist_add` | `[text...]` | Adds an exception, or the message you replied to | | `/whitelist_del` | `` | Removes an exception | | `/blacklist_rules` | — | Lists both lists in one message | | `/blacklist_check` | `` | Judges a text against both lists | +| `/whitelist_rnd` | — | Returns one whitelist entry at random | All are public and single-shot. +`/blacklist` is the short form of the two read commands: on its own it does what +`/blacklist_rules` does, and given text it does what `/blacklist_check` does. Nothing is +lost by using it — the long names remain for when the intent should be explicit. + +`/whitelist_rnd` returns a single whitelist entry chosen at random, and says so plainly when +the whitelist is empty rather than answering with nothing. + ## The bot does not police the chat This is the first thing to know, because the module's name promises something it diff --git a/internal/modules/blacklist/blacklist.go b/internal/modules/blacklist/blacklist.go index d20cad7..5e4c1b9 100644 --- a/internal/modules/blacklist/blacklist.go +++ b/internal/modules/blacklist/blacklist.go @@ -45,47 +45,62 @@ func New(deps modules.Deps) modules.Module { s := &state{store: storage.Typed[Entry](deps.Store)} return modules.Module{ Commands: []modules.Command{ + { + // The whole module behind one short name: bare it lists, with + // an argument it checks. + Name: "blacklist", + Visibility: modules.VisibilityPublic, + Description: "Check a text, or list both", + Parameters: "[text...]", + Handler: s.handleShort, + }, { Name: "blacklist_add", Visibility: modules.VisibilityPublic, - Description: "Blacklist a text, or a message you reply to", + Description: "Blacklist a text or a reply", Parameters: "[text...]", Handler: s.handleAdd(listBlack), }, { Name: "blacklist_del", Visibility: modules.VisibilityPublic, - Description: "Remove a text from the blacklist", + Description: "Remove from the blacklist", Parameters: "", Handler: s.handleDel(listBlack), }, { Name: "whitelist_add", Visibility: modules.VisibilityPublic, - Description: "Whitelist a text, or a message you reply to", + Description: "Whitelist a text or a reply", Parameters: "[text...]", Handler: s.handleAdd(listWhite), }, { Name: "whitelist_del", Visibility: modules.VisibilityPublic, - Description: "Remove a text from the whitelist", + Description: "Remove from the whitelist", Parameters: "", Handler: s.handleDel(listWhite), }, { Name: "blacklist_rules", Visibility: modules.VisibilityPublic, - Description: "List both lists for this topic", + Description: "List both lists here", Handler: s.handleRules, }, { Name: "blacklist_check", Visibility: modules.VisibilityPublic, - Description: "Check if a text is blacklisted here", + Description: "Check if a text is blacklisted", Parameters: "", Handler: s.handleCheck, }, + { + Name: "whitelist_rnd", + Visibility: modules.VisibilityPublic, + Description: "Random whitelist entry", + Handler: s.handleWhitelistRandom, + }, }, } } diff --git a/internal/modules/blacklist/handlers.go b/internal/modules/blacklist/handlers.go index a001c69..f82770f 100644 --- a/internal/modules/blacklist/handlers.go +++ b/internal/modules/blacklist/handlers.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "html" + "math/rand/v2" "sort" "strings" "time" @@ -291,9 +292,6 @@ func (s *state) entriesFor(ctx context.Context, chatID int64, threadID int, list } // handleCheck judges a text against this thread's rules. -// -// Unlike the mutation commands this applies no length cap: judging a long -// message is the point, and nothing here becomes a storage key. func (s *state) handleCheck(ctx context.Context, b *bot.Bot, update *models.Update) error { ctx, cancel := context.WithTimeout(ctx, handlerTimeout) defer cancel() @@ -307,7 +305,36 @@ func (s *state) handleCheck(ctx context.Context, b *bot.Bot, update *models.Upda if !ok { return chathelper.Reply(ctx, b, msg, "Usage: /blacklist_check ") } + return s.checkText(ctx, b, msg, normText) +} +// handleShort is /blacklist: the whole module behind one short name. Bare it +// lists both lists, with an argument it judges that text. +// +// The two behaviours are the two questions someone actually has — "what is set +// up here?" and "is this blocked?" — and neither needs its own long name once +// the argument distinguishes them. +func (s *state) handleShort(ctx context.Context, b *bot.Bot, update *models.Update) error { + ctx, cancel := context.WithTimeout(ctx, handlerTimeout) + defer cancel() + + msg := update.Message + if msg == nil { + return nil + } + + normText, ok := Normalize(chathelper.ArgAfterCommand(msg.Text)) + if !ok { + return s.showRules(ctx, b, msg) + } + return s.checkText(ctx, b, msg, normText) +} + +// checkText answers the verdict for already-normalized text. +// +// Unlike the mutation commands this applies no length cap: judging a long +// message is the point, and nothing here becomes a storage key. +func (s *state) checkText(ctx context.Context, b *bot.Bot, msg *models.Message, normText string) error { chatID, threadID := threadOf(msg) black, err := s.entriesFor(ctx, chatID, threadID, listBlack) if err != nil { @@ -350,7 +377,11 @@ func (s *state) handleRules(ctx context.Context, b *bot.Bot, update *models.Upda if msg == nil { return nil } + return s.showRules(ctx, b, msg) +} +// showRules renders both lists for the message's thread. +func (s *state) showRules(ctx context.Context, b *bot.Bot, msg *models.Message) error { chatID, threadID := threadOf(msg) blackPrefix := scopePrefix(chatID, threadID, listBlack) whitePrefix := scopePrefix(chatID, threadID, listWhite) @@ -413,3 +444,35 @@ func renderSection(sb *strings.Builder, list, prefix string, docs []storage.Doc[ sb.WriteString(line) } } + +// handleWhitelistRandom picks one whitelist entry at random. +func (s *state) handleWhitelistRandom(ctx context.Context, b *bot.Bot, update *models.Update) error { + ctx, cancel := context.WithTimeout(ctx, handlerTimeout) + defer cancel() + + msg := update.Message + if msg == nil { + return nil + } + + chatID, threadID := threadOf(msg) + prefix := scopePrefix(chatID, threadID, listWhite) + docs, err := s.store.Scan(ctx, prefix) + if err != nil { + log.Error("blacklist_rnd_scan", "list", listWhite, "err", err) + return chathelper.Reply(ctx, b, msg, genericFailure) + } + if len(docs) == 0 { + return chathelper.Reply(ctx, b, msg, + "This topic's whitelist is empty. Add something with /whitelist_add first.") + } + + pick := docs[rand.IntN(len(docs))] + // The record carries what the adder typed; the key carries only the + // normalized form, which is the fallback if a record ever lacks text. + text := pick.Val.Text + if text == "" { + text = decodeKeyText(strings.TrimPrefix(pick.ID, prefix)) + } + return chathelper.ReplyHTML(ctx, b, msg, ""+html.EscapeString(text)+"") +} diff --git a/internal/modules/blacklist/handlers_test.go b/internal/modules/blacklist/handlers_test.go index 373e8cd..66759c4 100644 --- a/internal/modules/blacklist/handlers_test.go +++ b/internal/modules/blacklist/handlers_test.go @@ -459,3 +459,95 @@ func TestAdd_ReplyStaysWithinOneMessage(t *testing.T) { t.Fatalf("add reply = %q; want a trim notice", last) } } + +// /blacklist is the whole module behind one name: bare it lists, with an +// argument it checks. +func TestShortCommand_ListsWhenBareAndChecksWithText(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add cat")) + + bare := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist")) + rules := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_rules")) + if bare != rules { + t.Fatalf("/blacklist = %q; want the same as /blacklist_rules = %q", bare, rules) + } + + hit := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist cat")) + check := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_check cat")) + if hit != check { + t.Fatalf("/blacklist cat = %q; want the same as /blacklist_check cat = %q", hit, check) + } + if !strings.Contains(hit, "🚫") { + t.Fatalf("/blacklist cat = %q; want blocked", hit) + } +} + +// Whitespace alone is not an argument, so it still lists rather than checking. +func TestShortCommand_BlankArgumentStillLists(t *testing.T) { + rb := installBlacklist(t) + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist ")); !strings.Contains(got, "Blacklist") { + t.Fatalf("/blacklist with blank argument = %q; want the listing", got) + } +} + +func TestWhitelistRandom_RefusesAnEmptyList(t *testing.T) { + rb := installBlacklist(t) + // A populated blacklist must not make the whitelist look non-empty. + send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add cat")) + + got := send(t, rb, testutil.NewPrivateMessage(7, "/whitelist_rnd")) + if !strings.Contains(got, "empty") { + t.Fatalf("/whitelist_rnd = %q; want an empty-list message", got) + } +} + +func TestWhitelistRandom_PicksFromTheWhitelistOnly(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add nope")) + entries := []string{"alpha", "beta", "gamma"} + for _, e := range entries { + send(t, rb, testutil.NewPrivateMessage(7, "/whitelist_add "+e)) + } + + seen := map[string]bool{} + for range 30 { + got := send(t, rb, testutil.NewPrivateMessage(7, "/whitelist_rnd")) + if strings.Contains(got, "nope") { + t.Fatalf("/whitelist_rnd = %q; picked from the blacklist", got) + } + match := "" + for _, e := range entries { + if strings.Contains(got, ""+e+"") { + match = e + } + } + if match == "" { + t.Fatalf("/whitelist_rnd = %q; want one of %v", got, entries) + } + seen[match] = true + } + // Three entries over thirty draws: landing on one every time would mean the + // pick is not random at all. + if len(seen) < 2 { + t.Fatalf("30 draws returned only %v", seen) + } +} + +func TestWhitelistRandom_IsPerTopic(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, inTopic(-100, 11, "/whitelist_add alpha")) + + if got := send(t, rb, inTopic(-100, 12, "/whitelist_rnd")); !strings.Contains(got, "empty") { + t.Fatalf("/whitelist_rnd in a sibling topic = %q; want an empty-list message", got) + } +} + +func TestWhitelistRandom_EscapesUserText(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, testutil.NewPrivateMessage(7, "/whitelist_add bold")) + + got := send(t, rb, testutil.NewPrivateMessage(7, "/whitelist_rnd")) + if strings.Contains(got, "bold") || !strings.Contains(got, "<b>bold</b>") { + t.Fatalf("/whitelist_rnd = %q; want the markup escaped", got) + } +}