Fix user tracking for users outside channels

Previously, if an unknown user sent a PRIVMSG directly to the
connection, it would become tracked. When a user with the same name then
joined a tracked channel, they would be treated as if they were the
first user. That is not necessarily the case.

Also, channels the connection is no longer in would stay alive in its
list of channels, even though nothing is actually known about them. This
would also keep user objects alive, since they were still seen as being
in that channel.

This commit fixes the issues and adds some tests around user tracking.

birch.test.asd

@@ -9,12 +9,14 @@
                (:file "ctcp")
                (:file "parse")
                (:file "commands")
-               (:file "events"))
+               (:file "events")
+               (:file "connection"))
   :perform (test-op :after (op component)
              (let ((results (mapcar (intern #.(string :run-test-package) :prove)
                                     (list :birch.test/ctcp
                                           :birch.test/parse
                                           :birch.test/commands
-                                          :birch.test/events))))
+                                          :birch.test/events
+                                          :birch.test/connection))))
                (unless (every #'identity results)
                  (error "Tests failed.")))))

src/connection.lisp

@@ -140,10 +140,22 @@ reconnect")
   (let ((found (find nick (users connection)
                      :test #'string=
                      :key #'nick)))
-    (when (and found user host)
-      (setf (user found) user
-            (host found) host))
-    found))
+    (cond ((eq connection found) found)
+          ((and found (null (channels found)))
+           ;; If we've seen a user before but they aren't in any of the
+           ;; channels we know of, we don't actually know if it's the same
+           ;; person. Thus any existing USER object is irrelevant.
+           ;;
+           ;; A better way to handle this would be to never put those users
+           ;; into the list, but due to the way the tracking is set up that's
+           ;; not known at the time of creation.
+           (prog1 nil
+             (removef (users connection) found)))
+          ((and found (channels found))
+           (prog1 found
+             (when (and user host)
+               (setf (user found) user
+                     (host found) host)))))))
 
 (defun valid-channel-name-p (name)
   (member (char name 0) '(#\& #\# #\+ #\!)))
@@ -187,8 +199,12 @@ updated."
 (defun remove-user (user &optional channel)
   (cond
     (channel (removef (channels user) channel)
-             (removef (users channel) user))
+             (removef (users channel) user)
+             (when (eq user (connection user))
+               (dolist (other-user (users channel))
+                 (remove-user other-user channel))))
     (t (dolist (channel (channels user))
          (remove-user user channel))))
-  (when (not (channels user))
+  (when (and (not (channels user))
+             (not (eq user (connection user))))
     (removef (users (connection user)) user)))

test/connection.lisp

@@ -0,0 +1,66 @@
+(defpackage :birch.test/connection
+  (:use :cl :birch.test/util :prove)
+  (:import-from :birch/connection
+                #:make-channel
+                #:make-user
+                #:users
+                #:channels
+                #:add-user
+                #:remove-user))
+(in-package :birch.test/connection)
+
+(diag "User tracking tests")
+(plan 1)
+
+(deftest "Users joining and leaving a channel"
+  (plan 19)
+  (with-test-connection (connection stream)
+    (declare (ignore stream))
+    ;; A user outside of a channel can not be tracked.
+    (isnt (make-user connection "WiZ")
+          (make-user connection "WiZ"))
+    (let ((channel (make-channel connection "#test"))
+          (user (make-user connection "WiZ")))
+      ;; A user with whom we're in the same channel can be tracked.
+      (add-user connection channel)
+      (add-user user channel)
+      (is (make-user connection "WiZ")
+          user)
+      ;; Both users are now in 1 channel, as far as we know.
+      (is (length (channels user)) 1)
+      (is (length (channels connection)) 1)
+      ;; We're now tracking ourselves and USER.
+      (is (length (users connection)) 2)
+
+      ;; When the user leaves the channel again, we can no longer track them.
+      (remove-user user channel)
+      (is (length (users connection)) 1)
+      (isnt (make-user connection "WiZ")
+            user)
+      (is (length (channels user)) 0)
+      (is (length (channels connection)) 1)
+      ;; This is a tricky one. Because "untracked" users are still in the list
+      ;; of users, the new "WiZ" will be in the list of users.
+      (is (length (users connection)) 2)
+
+      ;; When a new user joins the channel we can track them fine.
+      (let ((user2 (make-user connection "WiZ")))
+        (add-user user2 channel)
+        (is (make-user connection "WiZ")
+            user2)
+        (is (length (channels user2)) 1)
+        (is (length (channels connection)) 1)
+        (is (length (users connection)) 2))
+
+      ;; If we now leave the channel, we no longer have a clue who they might
+      ;; be.
+      (remove-user connection channel)
+      (is (length (users connection)) 1)
+      (isnt (make-user connection "WiZ")
+            user)
+      (is (length (channels user)) 0)
+      (is (length (channels connection)) 0)
+      ;; Again, the untracked "WiZ" is now in the list of users.
+      (is (length (users connection)) 2)
+      ;; After we've left the channel, we no longer know who's in it.
+      (is (length (users channel)) 0))))