Merge lp:~gingerchris/endroid/assorted_bug_fixes into lp:~gingerchris/endroid/remote_changes

[ChrisD]

SMS period bugfix, pounce and trains bugfixes

 - Fix bug in SMS period reset (wrong number of args)
 - Pounce:
  - remove pounce message cmd (merge with punce)
  - add checking to see if user is known (all the way down the APIs) or empty string
  - always messages the pounce setter when the target comes online (even when message specified)
  - Add pounce setting username to the pounce message
 - Trains:
  - Fix help
  - Fix bug with DB entry removal and failure to extract results
  - Add better error if time no understood
  - Add better error if HTTP request fails
  - Support 4 digit 24 hour times and error impossible hours
 - Minor changes to: wiki pages, pluginmanager

[Phil Connell (pconnell)]

review: Approve

[ChrisD (gingerchris)]

review: Approve

=== modified file 'doc/wiki/GettingStarted'
--- doc/wiki/GettingStarted	2013-09-11 14:24:24 +0000
+++ doc/wiki/GettingStarted	2014-09-16 16:39:09 +0000
@@ -7,11 +7,11 @@
 Installing EnDroid is straightforward. Installation packages are provided (currently only for Ubuntu) [[https://launchpad.net/endroid/+download|on Launchpad]]. It is also possible to run it directly
 by checking out the code using `bzr branch lp:endroid`.
 
-  Note: !EnDroid requires python 2.7 or later to run, and depends on Twisted, Wokkel, PyTZ and python-dateutil.
+  Note: !EnDroid requires python 2.7 or later to run, and depends on Twisted, Wokkel, PyTZ, treq, jinja2 and python-dateutil.
 
 In order to run EnDroid needs to load certain settings from its config file. An example one is at `/usr/share/doc/endroid/examples/endroid.conf` if !EnDroid has been installed or in the 
 source repository at `etc/endroid.conf`. The settings that must be added to the config file are details of an XMPP account for !EnDroid to use (free accounts can be created easily e.g. from 
-[[http://comm.unicate.me/|comm.unicate.me]]). 
+[[http://www.jabber.at/|jabber.at]]). 
 
  Note: '''Do not use your own account''' as !EnDroid will cut down its contact list to users listed in the config file.
 
@@ -21,12 +21,12 @@
 = Running EnDroid =
 
  * If you installed !EnDroid, you should be able to run it by typing `endroid` into the console.
- * If you pulled the source, run !EnDroid from within the `src` directory with `./endroid.sh`
+ * If you pulled the source, run !EnDroid using `src/endroid.sh`
  * See `-h` for full list of command line arguments, some key ones are:
-   * `<config-file>` - specify an alternative config file.
+   * `[-c <config-file>]` - specify an alternative config file.
    * `[-l|--level] <log-level>` - specify the verbosity of the console log (lower is more verbose)
    * `[-L|--logfile] <log-file>` - redirect the console logging to a file.
-   * `[-t|--logtraffic]` -  if `-t` is present, `twisted` will log all the raw message xml to the file log.
+   * `[-t|--logtraffic]` - log all the raw message xml to the file log.
 
 See the [[../Debugging|debugging page]] for more details on logging and `Manhole`.
 
 
=== modified file 'doc/wiki/Reference'
--- doc/wiki/Reference	2014-09-02 10:51:47 +0000
+++ doc/wiki/Reference	2014-09-16 16:39:09 +0000
@@ -233,7 +233,7 @@
 For examples of using the context-aware response callbacks, see the Remote
 and Timer plugins.
 
-=== Context awareness === 
+=== Context awareness ===
 This is implemented by allowing plugins to attach callbacks to 
 MessageHandler.send_chat() for 'when user next replies' and 'if no reply
 received'. There is a config file option for the latter.
 
=== modified file 'src/endroid/pluginmanager.py'
--- src/endroid/pluginmanager.py	2014-08-11 15:30:01 +0000
+++ src/endroid/pluginmanager.py	2014-09-16 16:39:09 +0000
@@ -305,8 +305,8 @@
             __import__(modname)
         except ImportError as i:
             logging.error(i)
-            logging.error("**Could Not Import Plugin \"" + modname
-                          + "\". Check That It Exists In Your PYTHONPATH.")
+            logging.error("**Could not import plugin \"" + modname
+                          + "\". Check that it exists in your PYTHONPATH.")
             return
         except Exception as e:
             logging.error(e)
 
=== modified file 'src/endroid/plugins/periodicpinger.py'
--- src/endroid/plugins/periodicpinger.py	2014-09-10 13:13:58 +0000
+++ src/endroid/plugins/periodicpinger.py	2014-09-16 16:39:09 +0000
@@ -62,7 +62,7 @@
         """Pings a user on each server Endroid knows about."""
 
         users_to_ping = set(self._get_one_from_each())
-        logging.debug('PeriodicPinger: Pinging {}...'.format(users_to_ping))
+        self.log.debug('PeriodicPinger: Pinging {}...'.format(users_to_ping))
         for user in users_to_ping:
             self.usermanagement.ping(user)
 
 
=== modified file 'src/endroid/plugins/pounce.py'
--- src/endroid/plugins/pounce.py	2014-08-14 10:01:44 +0000
+++ src/endroid/plugins/pounce.py	2014-09-16 16:39:09 +0000
@@ -12,59 +12,61 @@
     name = "pounce"
     help = ("Pounces on a user by sending them a message immediately when they"
             " come online. \n"
-            "Use 'pounce message <user> <message>', \n"
-            "or 'stalk <user>' (to be notified when a user goes offline), \n"
-            "or 'pounce <user>' to be told when the user comes online.")
+            "Use 'pounce <user> [<message>]' to be told when user comes online "
+            "and optionally send them message when they do, \n"
+            "or 'stalk <user>' to be notified when a user goes offline.")
 
     
-    @command(helphint='{user}')
+    @command(helphint='{user} [{message}]')
     def pounce(self, msg, args):
-        subject = args.strip()  # strip spaces from the outside if necessary
-        if args[:8] == 'message ':
-            # we don't want to handle this, as it's a 'pounce message' call
-            msg.unhandled()
-            return
-        if self.rosters.is_online(subject):
-            msg.reply(subject + ' is already online.')
-            return
-
-        message = subject + ' came online.'
-        cb = lambda: self.messages.send_chat(msg.sender, message)
-
-        self.rosters.register_presence_callback(user=subject, callback=cb,
-                                               available=True)
-        msg.reply("I'll let you know when {} comes online.".format(subject))
-
-    @command(helphint="{user} {message}")
-    def pounce_message(self, msg, args):
-        args_split = args.split(' ')
-        tojid = args_split[0]
-
-        if args_split[1:]:
-            message = msg.sender + ' pounced! ' + ' '.join(args_split[1:])
+        reply = None
+        args = args.strip()
+        if args:
+            arg_tokens = args.split(' ') 
+            target = arg_tokens.pop(0)
+            message = " ".join(arg_tokens)
+            if message:
+                message = "{} says: {}".format(msg.sender, message)
         else:
-            msg.reply('You need to specify both a user and a message for me to '
-                      'deliver when I pounce.')
-            return
-
-        if self.rosters.is_online(tojid):
-            self.messages.send_chat(tojid, message, msg.sender)
-            msg.reply("The user was already online, so I just told them direct.")
-            return
-
-        cb = lambda: self.messages.send_chat(tojid, message, msg.sender)
-
-        self.rosters.register_presence_callback(user=tojid, callback=cb,
-                                                       available=True)
-
-        msg.reply("Lying in wait for {} with message '{}'".format(tojid, message))
-
+            reply = "Doesn't look like you've told me who to pounce"
+
+        if not reply and target not in self.rosters.users:
+            reply = "I don't know the user {}.".format(target)
+
+        if not reply and self.rosters.is_online(target):
+            if message:
+                # Send them the message now
+                self.messages.send_chat(target, message, msg.sender)
+                reply = target + " was already online, so I just told them direct."
+            else:
+                reply = target + " is already online."
+
+        if not reply:
+            if message:
+                # When the target comes online send them message and the
+                # requestor the pounce_message
+                pounce_message = target + ' came online - message sent'
+                reply = ("I'll let you know when {} comes online and send "
+                         "them the message.".format(target))
+            else:
+                pounce_message = target + ' came online.'
+                reply = "I'll let you know when {} comes online.".format(target)
+
+            def cb():
+                self.messages.send_chat(msg.sender, pounce_message)
+                if message:
+                    self.messages.send_chat(target, message)
+
+            self.rosters.register_presence_callback(user=target, callback=cb,
+                                                    available=True)
+
+        msg.reply(reply)
 
     @command(helphint="{user}")
     def stalk(self, msg, args):
         cb = lambda: self.messages.send_chat(msg.sender,
                                              'User {} is offline'.format(args))
         self.rosters.register_presence_callback(user=args, callback=cb,
-                                               unavailable=True)
+                                                unavailable=True)
 
         msg.reply("You will be notified when user {} goes offline.".format(args))
 
=== modified file 'src/endroid/plugins/sms/__init__.py'
--- src/endroid/plugins/sms/__init__.py	2014-09-12 12:22:44 +0000
+++ src/endroid/plugins/sms/__init__.py	2014-09-16 16:39:09 +0000
@@ -123,10 +123,8 @@
 
         # logging file stuff
         self.log = logging.getLogger(__name__)
-        try:
-            logfile = os.path.expanduser(self.vars['logfile'])
-        except KeyError:
-            logfile = os.path.expanduser('~/.endroid/sms.log')
+        logfile = os.path.expanduser(self.vars.get('logfile', '~/.endroid/sms.log'))
+        self.log.info("Logging SMS activity to {}".format(logfile))
         handler = logging.FileHandler(logfile)
         formatter = logging.Formatter('%(asctime)s %(levelname)s %(message)s')
         handler.setFormatter(formatter)
@@ -245,7 +243,7 @@
 
         return users_blocked
 
-    def _sms_reset_number_sent(self):
+    def _sms_reset_number_sent(self, unused_params):
         """Resets the number of texts sent in the database."""
 
         self.db.empty_table(DB_LIMIT_TABLE)
 
=== modified file 'src/endroid/plugins/trains.py'
--- src/endroid/plugins/trains.py	2014-08-12 10:28:57 +0000
+++ src/endroid/plugins/trains.py	2014-09-16 16:39:09 +0000
@@ -22,9 +22,6 @@
 
 class TrainTimes(CommandPlugin):
     name = "traintimes"
-    help_topics = {
-        "": "When do trains leave?",
-    }
 
     def endroid_init(self):
         if not self.database.table_exists(STATION_TABLE):
@@ -32,6 +29,9 @@
         if not self.database.table_exists(HOME_TABLE):
             self.database.create_table(HOME_TABLE, ("jid", "station"))
 
+    def help(self):
+        return "When do trains leave?"
+
     def _station_update(self, msg, args, table, jid, display):
         if not args:
             rows = self.database.fetch(table, ("station",),
@@ -43,7 +43,7 @@
                 msg.reply_to_sender("You don't have a {} station set."
                                     .format(display))
             return
-        self.database.delete(table, {"jid": msg.sender})
+        self.database.delete(table, {"jid" : jid})
         if args != "delete":
             self.database.insert(table, {"jid": jid, "station": args})
             msg.reply_to_sender("Your new {} station is: {}"
@@ -54,11 +54,15 @@
 
     @command(helphint="{<station name>|delete}")
     def nearest_station(self, msg, args):
+        # Use msg.sender_full (with resource) because a person might
+        # be in different locations with different resources
         self._station_update(msg, args, STATION_TABLE, msg.sender_full,
                              "nearest")
 
     @command(helphint="{<station name>|delete}")
     def home_station(self, msg, args):
+        # Use msg.sender (no resource) because a person probably
+        # lives in the same place regardless of XMPP resource
         self._station_update(msg, args, HOME_TABLE, msg.sender, "home")
 
     @command(helphint="from <stn> to <stn> [[arriving|leaving] at <time>]",
@@ -70,7 +74,10 @@
             return
 
         def extract_results(data):
-            results = RESULTRE.findall(ULRE.search(data).group(1))
+            result = None
+            m = ULRE.search(data)
+            if m:
+                results = RESULTRE.findall(m.group(1))
             if results:
                 msg.reply(u"Trains from {} to {}: {}"
                           .format(src, dst,
@@ -78,8 +85,12 @@
             else:
                 msg.reply("Either your request is malformed, or there are no matching trains")
 
-        home, src, dst, typ, when, time, _,_,_ = match.groups()
-        time = self._canonical_time(time) if time is not None else time
+        home, src, dst, typ, when, raw_time, _,_,_ = match.groups()
+        time = self._canonical_time(raw_time) if raw_time is not None else None
+        if raw_time and not time:
+            msg.reply("I can't understand the time you've entered can you try another way?")
+            return
+
         if dst == "home" or home == "home":
             rows = self.database.fetch(HOME_TABLE, ("station",),
                                        {"jid": msg.sender})
@@ -104,17 +115,22 @@
                 "a" if typ == "arriving" else "",
                 ("/" + when.replace(" ", "-")) if when else "")
         getPage("http://www.traintimes.org.uk" + urllib.quote(url)
-                ).addCallbacks(extract_results, msg.unhandled)
+                ).addCallbacks(extract_results, lambda x: msg.reply("Bad train request"))
 
     @staticmethod
     def _canonical_time(time):
         match = TIMERE.match(time.strip())
         assert match, "We've already checked this - how can it fail?!"
         hour, minute, half = match.groups()
+        if len(hour) == 4 and not minute and not half:
+            # This is just a 4 digit number, treat as if its a time in 24h
+            minute = hour[2:]
+            hour = hour[:2]
         hour, minute = map(lambda n: int(n) if n else 0, (hour, minute))
+        if hour > 24:
+            return None
+          
         if half and half == 'pm':
             if hour <= 12:
                 hour += 12
-        if not minute:
-            minute = 0
         return "{}:{:02}".format(hour, minute)
 
=== modified file 'src/endroid/usermanagement.py'
--- src/endroid/usermanagement.py	2014-08-22 12:28:28 +0000
+++ src/endroid/usermanagement.py	2014-09-16 16:39:09 +0000
@@ -480,9 +480,11 @@
         :param unavailable: set True to have callback fire when user goes offline
         """
         if not (unavailable or available):
-        # let us know that our callback will never fire
             raise ValueError('Callback specified neither available nor unavailable')
 
+        if user not in self.users():
+            raise ValueError('Invalid user {}'.format(user))
+
         if available:
             self._callbacks_when_available[user].append(callback)
         if unavailable:
@@ -905,6 +907,9 @@
         :param available: set True to have callback fire when user comes online
         :param unavailable: set True to have callback fire when user goes offline
         """
+
+        if user not in self.users:
+            raise ValueError('Invalid user {}'.format(user))
         self._usermanagement._register_presence_callback(user, callback,
                                                          available=available,
                                                          unavailable=unavailable)