From 7beb7de516142a0cd2a8731232f1bc76e52e67f5 Mon Sep 17 00:00:00 2001 From: Thierry Carrez Date: Thu, 26 Nov 2020 17:56:18 +0100 Subject: [PATCH] Refactor processing of track and admin commands Separate the processing of track commands and admin commands into separate files, and normalize handling of messages. Adjust test cases so that they match the new normalized messages. Change-Id: I26e3683bbf3b7daab68407cd9928ee3d7f5c50dc --- ptgbot/admincommands.py | 86 ++++++++++ ptgbot/bot.py | 246 ++++----------------------- ptgbot/tests/test_message_process.py | 15 +- ptgbot/trackcommands.py | 117 +++++++++++++ 4 files changed, 248 insertions(+), 216 deletions(-) create mode 100644 ptgbot/admincommands.py create mode 100644 ptgbot/trackcommands.py diff --git a/ptgbot/admincommands.py b/ptgbot/admincommands.py new file mode 100644 index 0000000..6da7fca --- /dev/null +++ b/ptgbot/admincommands.py @@ -0,0 +1,86 @@ +# Copyright 2011, 2013, 2020 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +def process_admin_command(db, command, params): + + if command == 'emptydb': + db.empty() + + elif command == 'fetchdb': + if len(params) < 1: + return "Missing URL to fetch (~fetch URL)" + url = params[0] + try: + db.import_json(url) + return "Loaded DB from %s" % url + except Exception as e: + return "Error loading DB: %s" % e + + elif command == 'newday': + db.new_day_cleanup() + + elif command == 'motd': + if len(params) < 1: + return "Missing subcommand (~motd add|del|clean|reorder ...)" + + if params[0] == "add": + if len(params) < 3: + return "Missing parameters (~motd add LEVEL MSG)" + if params[1] not in ['info', 'success', 'warning', 'danger']: + return ("Incorrect message level '%s' (should be info, " + "success, warning or danger)" % params[1]) + db.motd_add(params[1], str.join(' ', params[2:])) + + elif params[0] == "del": + if len(params) < 2: + return "Missing message number (~motd del NUM)" + if not db.motd_has(params[1]): + return "Incorrect message number %s" % params[1] + db.motd_del(params[1]) + + elif params[0] == "clean": + if len(params) > 1: + return "'~motd clean' does not take parameters" + db.motd_clean() + + elif params[0] == "reorder": + if len(params) < 2: + return "Missing params (~motd reorder X Y...)" + order = [] + for num in params[1:]: + if not db.motd_has(num): + return "Incorrect message number %s" % num + order.append(num) + db.motd_reorder(order) + + else: + return "Unknown motd subcommand %s" % params[0] + + elif command == 'requirevoice': + db.require_voice() + + elif command == 'alloweveryone': + db.allow_everyone() + + elif command == 'list': + return 'Available tracks: ' + str.join(' ', db.list_tracks()) + + elif command in ('clean', 'add', 'del'): + if len(params) < 1: + return "This command takes one or more arguments" + getattr(db, command + '_tracks')(params[1:]) + + else: + return "Unknown command '%s'" % command diff --git a/ptgbot/bot.py b/ptgbot/bot.py index 7d7c731..51584a0 100644 --- a/ptgbot/bot.py +++ b/ptgbot/bot.py @@ -21,12 +21,13 @@ from ib3.connection import SSL import irc.bot import json import logging.config -import re import os import time import textwrap import ptgbot.db +from ptgbot.admincommands import process_admin_command +from ptgbot.trackcommands import process_track_command from ptgbot.usercommands import process_user_command @@ -130,6 +131,35 @@ class PTGBot(SASL, SSL, irc.bot.SingleServerIRCBot): return (self.channels[chan].is_voiced(nick) or self.channels[chan].is_oper(nick)) + def handle_public_command(self, chan, nick, args): + words = args.split() + cmd = words[0].lower() + + if len(cmd) > 1 and cmd[1:] == 'help': + return "See PTGbot documentation at: " + DOC_URL + + if cmd.startswith('+'): + return process_user_command(self.data, nick, cmd[1:], words[1:]) + + if cmd.startswith('#'): + + if cmd in ['#in', '#out', '#seen', '#subscribe', '#unsubscribe']: + return process_user_command(self.data, nick, + cmd[1:], words[1:]) + else: + if (self.data.is_voice_required() and + not self.is_voiced(nick, chan)): + return "Need voice to issue commands" + track = words[0][1:].lower() + return process_track_command(self.data, self.send, track, + words[1:]) + + if cmd.startswith('~'): + if not self.is_chanop(nick, chan): + return "Need op for admin commands" + directive = words[0][1:].lower() + return process_admin_command(self.data, directive, words[1:]) + @make_safe def on_pubmsg(self, c, e): if not self.identify_msg_cap: @@ -137,217 +167,13 @@ class PTGBot(SASL, SSL, irc.bot.SingleServerIRCBot): "cap not enabled") return nick = e.source.split('!')[0] - msg = e.arguments[0][1:] + args = e.arguments[0][1:] chan = e.target - if msg.startswith('+'): - words = msg.split() - cmd = words[0].lower()[1:] - ret = process_user_command(self.data, nick, cmd, words[1:]) - if ret: - self.send(chan, "%s: %s" % (nick, ret)) - - if msg.startswith('#'): - words = msg.split() - cmd = words[0].lower() - - if cmd in ['#in', '#out', '#seen', '#subscribe', '#unsubscribe']: - cmd = cmd[1:] - ret = process_user_command(self.data, nick, cmd, words[1:]) - if ret: - self.send(chan, "%s: %s" % (nick, ret)) - return - - if (self.data.is_voice_required() and - not self.is_voiced(nick, chan)): - self.send(chan, "%s: Need voice to issue commands" % (nick,)) - return - - if cmd == '#help': - self.usage(chan) - return - - if ((len(words) < 2) or - (len(words) == 2 and words[1].lower() != 'clean')): - self.send(chan, "%s: Incorrect arguments" % (nick,)) - self.usage(chan) - return - - track = words[0][1:].lower() - if not self.data.is_track_valid(track): - self.send(chan, "%s: unknown track '%s'" % (nick, track)) - self.send_track_list(chan) - return - - adverb = words[1].lower() - params = str.join(' ', words[2:]) - if adverb == 'now': - self.data.add_now(track, params) - self.notify(track, adverb, params) - elif adverb == 'next': - self.data.add_next(track, params) - self.notify(track, adverb, params) - elif adverb == 'clean': - self.data.clean_tracks([track]) - elif adverb == 'etherpad': - self.data.add_etherpad(track, params) - elif adverb == 'url': - self.data.add_url(track, params) - elif adverb == 'color': - self.data.add_color(track, params) - elif adverb == 'location': - self.data.add_location(track, params) - elif adverb == 'book': - room, sep, timeslot = params.partition('-') - if self.data.is_slot_valid_and_empty(room, timeslot): - self.data.book(track, room, timeslot) - self.send(chan, "%s: Room %s is now booked on %s for %s" % - (nick, room, timeslot, track)) - else: - self.send(chan, "%s: slot '%s' is invalid (or booked)" % - (nick, params)) - elif adverb == 'unbook': - room, sep, timeslot = params.partition('-') - if self.data.is_slot_booked_for_track(track, room, timeslot): - self.data.unbook(room, timeslot) - self.send(chan, "%s: Room %s (previously booked for %s) " - "is now free on %s" % - (nick, room, track, timeslot)) - else: - self.send(chan, "%s: slot '%s' is invalid " - "(or not booked for %s)" % - (nick, params, track)) - else: - self.send(chan, "%s: unknown directive '%s'. " - "Did you mean: %s now %s... ?" % - (nick, adverb, track, adverb)) - return - if adverb in ['now', 'next']: - if not self.data.get_track_room(track): - self.send(chan, "%s: message added, but please note that " - "track '%s' does not appear to have a room " - "scheduled today." % (nick, track)) - - if msg.startswith('~'): - if not self.is_chanop(nick, chan): - self.send(chan, "%s: Need op for admin commands" % (nick,)) - return - words = msg.split() - command = words[0][1:].lower() - if command == 'emptydb': - self.data.empty() - elif command == 'fetchdb': - url = words[1] - self.send(chan, "Loading DB from %s ..." % url) - try: - self.data.import_json(url) - self.send(chan, "Done.") - except Exception as e: - self.send(chan, "Error loading DB: %s" % e) - elif command == 'newday': - self.data.new_day_cleanup() - - elif command == 'motd': - if len(words) < 2: - self.send( - chan, - "Missing subcommand (~motd add|del|clean|reorder ...)" - ) - return - if words[1] == "add": - if len(words) < 4: - self.send( - chan, - "Missing parameters (~motd add LEVEL MSG)" - ) - return - if words[2] not in [ - 'info', 'success', 'warning', 'danger' - ]: - self.send( - chan, - "Incorrect message level '%s' (should be info, " - "success, warning or danger)" % words[2] - ) - return - self.data.motd_add(words[2], str.join(' ', words[3:])) - elif words[1] == "del": - if len(words) < 3: - self.send( - chan, - "Missing message number (~motd del NUM)" - ) - return - if not self.data.motd_has(words[2]): - self.send( - chan, - "Incorrect message number %s" % words[2] - ) - return - self.data.motd_del(words[2]) - elif words[1] == "clean": - if len(words) > 2: - self.send( - chan, - "'~motd clean' does not take parameters" - ) - return - self.data.motd_clean() - elif words[1] == "reorder": - if len(words) < 3: - self.send( - chan, - "Missing params (~motd reorder X Y...)" - ) - return - order = [] - for num in words[2:]: - if not self.data.motd_has(num): - self.send( - chan, - "Incorrect message number %s" % num - ) - return - order.append(num) - self.data.motd_reorder(order) - else: - self.send(chan, "Unknown motd subcommand %s" % words[1]) - - elif command == 'requirevoice': - self.data.require_voice() - elif command == 'alloweveryone': - self.data.allow_everyone() - elif command == 'list': - self.send_track_list(chan) - elif command in ('clean', 'add', 'del'): - if len(words) < 2: - self.send(chan, "this command takes one or more arguments") - return - getattr(self.data, command + '_tracks')(words[1:]) - else: - self.send(chan, "Unknown command '%s'" % command) - return - - def notify(self, track, adverb, params): - location = self.data.get_location(track) - track = '#' + track - trackloc = track - if location is not None: - trackloc = "%s (%s)" % (track, location) - - for nick, regexp in self.data.get_subscriptions().items(): - if regexp is None: - # Person did #unsubscribe, so skip - continue - event_text = " ".join([track, adverb, params]) - if re.search(regexp, event_text, re.IGNORECASE): - message = "%s in %s: %s" % (adverb, trackloc, params) - # Note: there is no guarantee that nick will be online - # at this point. However if not, the bot will receive - # a 401 :No such nick/channel message which it will - # ignore due to the lack of a nosuchnick handler. - # Fortunately this is the behaviour we want. - self.send(nick, message) + msg = self.handle_public_command(chan, nick, args) + if msg: + self.send(chan, ("%s: " % nick) + msg) + return def send(self, channel, msg): # 400 chars is an estimate of a safe line length (which can vary) diff --git a/ptgbot/tests/test_message_process.py b/ptgbot/tests/test_message_process.py index e09eb6f..fc4c363 100644 --- a/ptgbot/tests/test_message_process.py +++ b/ptgbot/tests/test_message_process.py @@ -60,7 +60,10 @@ class TestProcessMessage(testtools.TestCase): self.bot, 'send', ) as mock_send: self.bot.on_pubmsg('', msg) - mock_send.assert_called_with('#channel', "See doc at: " + DOC_URL) + mock_send.assert_called_with( + '#channel', + "johndoe: See PTGbot documentation at: " + DOC_URL, + ) def test_invalidtrack(self): msg = Event('', @@ -74,7 +77,7 @@ class TestProcessMessage(testtools.TestCase): self.bot.on_pubmsg('', msg) mock_send.assert_any_call( '#channel', - "johndoe: unknown track 'svift'" + "johndoe: Unknown track 'svift'" ) def test_now(self): @@ -241,7 +244,7 @@ class TestProcessMessage(testtools.TestCase): self.bot.on_pubmsg('', msg) mock_send.assert_called_with( '#channel', - "johndoe: slot '%s' is invalid (or booked)" % slot + "johndoe: Slot '%s' is invalid (or booked)" % slot ) mock_send.reset_mock() @@ -259,7 +262,7 @@ class TestProcessMessage(testtools.TestCase): self.bot.on_pubmsg('', msg) mock_send.assert_called_with( '#channel', - "johndoe: slot '%s' is invalid " + "johndoe: Slot '%s' is invalid " "(or not booked for swift)" % slot ) mock_send.reset_mock() @@ -492,7 +495,7 @@ class TestProcessMessage(testtools.TestCase): '~motd clean 2': "'~motd clean' does not take parameters", '~motd reorder': "Missing params (~motd reorder X Y...)", '~motd reorder 999': "Incorrect message number 999", - '~add': "this command takes one or more arguments", + '~add': "This command takes one or more arguments", } self.bot.is_chanop = mock.MagicMock(return_value=True) original_db_data = copy.deepcopy(self.db.data) @@ -507,7 +510,7 @@ class TestProcessMessage(testtools.TestCase): self.bot.on_pubmsg('', msg) mock_send.assert_called_with( '#channel', - response + "johndoe: " + response ) self.assertEqual(self.db.data, original_db_data) mock_send.reset_mock() diff --git a/ptgbot/trackcommands.py b/ptgbot/trackcommands.py new file mode 100644 index 0000000..dead6a0 --- /dev/null +++ b/ptgbot/trackcommands.py @@ -0,0 +1,117 @@ +# Copyright 2011, 2013, 2020 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import re + + +def notify(db, botsend, track, adverb, sentence): + location = db.get_location(track) + track = '#' + track + trackloc = track + if location is not None: + trackloc = "%s (%s)" % (track, location) + + for nick, regexp in db.get_subscriptions().items(): + if regexp is None: + # Person did #unsubscribe, so skip + continue + event_text = " ".join([track, adverb, sentence]) + if re.search(regexp, event_text, re.IGNORECASE): + message = "%s in %s: %s" % (adverb, trackloc, sentence) + # Note: there is no guarantee that nick will be online + # at this point. However if not, the bot will receive + # a 401 :No such nick/channel message which it will + # ignore due to the lack of a nosuchnick handler. + # Fortunately this is the behaviour we want. + botsend(nick, message) + + +def not_scheduled_today(db, track): + if not db.get_track_room(track): + return ("Message added, but please note that track '%s' does not " + "appear to have a room scheduled today." % track) + + +def process_track_command(db, botsend, track, params): + if not db.is_track_valid(track): + return "Unknown track '%s'" % track + + if len(params) < 1: + return "Missing track command (#TRACK [now|next|clean...] ...)" + + adverb = params[0].lower() + sentence = str.join(' ', params[1:]) + + if adverb == 'now': + if len(params) < 2: + return "Missing sentence (#TRACK now ...)" + db.add_now(track, sentence) + notify(db, botsend, track, adverb, sentence) + return not_scheduled_today(db, track) + + elif adverb == 'next': + if len(params) < 2: + return "Missing sentence (#TRACK next ...)" + db.add_next(track, sentence) + notify(db, botsend, track, adverb, sentence) + return not_scheduled_today(db, track) + + elif adverb == 'clean': + if len(params) > 1: + return "'#TRACK clean' does not take any parameter" + db.clean_tracks([track]) + + elif adverb == 'etherpad': + if len(params) != 2: + return "'#TRACK etherpad' takes a single URL parameter" + db.add_etherpad(track, params[1]) + + elif adverb == 'url': + if len(params) != 2: + return "'#TRACK url' takes a single URL parameter" + db.add_url(track, params[1]) + + elif adverb == 'color': + if len(params) != 2: + return "'#TRACK color' takes a single colorcode parameter" + db.add_color(track, params[1]) + + elif adverb == 'location': + db.add_location(track, sentence) + + elif adverb == 'book': + if len(params) != 2: + return "'#TRACK book' takes a single slotname parameter" + room, sep, tslot = params[1].partition('-') + if db.is_slot_valid_and_empty(room, tslot): + db.book(track, room, tslot) + return "Room %s is now booked on %s for %s" % (room, tslot, track) + else: + return "Slot '%s' is invalid (or booked)" % params[1] + + elif adverb == 'unbook': + if len(params) != 2: + return "'#TRACK unbook' takes a single slotname parameter" + room, sep, tslot = params[1].partition('-') + if db.is_slot_booked_for_track(track, room, tslot): + db.unbook(room, tslot) + return ("Room %s (previously booked for %s) is now free on %s" % + (room, track, tslot)) + else: + return ("Slot '%s' is invalid (or not booked for %s)" % + (params[1], track)) + + else: + return ("Unknown command '%s'. Did you mean: %s now %s... ?" % + (adverb, track, adverb))