Make an IRC listener connection follow its session's nick (closes #184)

When the session's nick was changed from another client, such as the
web client over the HTTP API, the IRC listener connection kept the old
nick and sent its messages, joins and later NICK changes under it. Now,
whenever the relay passes on a NICK line, the connection first takes
the nick its session holds in the database. It does not go by the
line's old nick, as the connection may have taken that nick after
another user gave it up. As the relay goroutine can now change the
nick, the command handlers read it under the connection's lock,
through currentNick, as the relay already did.

Model: opus-5-5

internal/db/queries.go

@@ -264,6 +264,25 @@ func (database *Database) GetSessionByNick(
 	return sessionID, nil
 }
 
+// GetSessionNick returns the nick a session holds.
+func (database *Database) GetSessionNick(
+	ctx context.Context,
+	sessionID int64,
+) (string, error) {
+	var nick string
+
+	err := database.conn.QueryRowContext(
+		ctx,
+		"SELECT nick FROM sessions WHERE id = ?",
+		sessionID,
+	).Scan(&nick)
+	if err != nil {
+		return "", fmt.Errorf("get session nick: %w", err)
+	}
+
+	return nick, nil
+}
+
 // SessionHostInfo holds the username, hostname, and IP
 // for a session.
 type SessionHostInfo struct {

internal/ircserver/commands.go

@@ -81,7 +81,7 @@ func (c *Conn) handleNick(
 	}
 
 	oldMask := c.hostmask()
-	oldNick := c.nick
+	oldNick := c.currentNick()
 
 	err := c.svc.ChangeNick(
 		ctx, c.sessionID, c.clientID, oldNick, newNick,
@@ -129,7 +129,7 @@ func (c *Conn) handlePrivmsg(
 
 	if strings.HasPrefix(target, "#") {
 		_, err := c.svc.SendChannelMessage(
-			ctx, c.sessionID, c.clientID, c.nick,
+			ctx, c.sessionID, c.clientID, c.currentNick(),
 			msg.Command, target, body, nil,
 		)
 		if err != nil {
@@ -137,7 +137,7 @@ func (c *Conn) handlePrivmsg(
 		}
 	} else {
 		result, err := c.svc.SendDirectMessage(
-			ctx, c.sessionID, c.clientID, c.nick,
+			ctx, c.sessionID, c.clientID, c.currentNick(),
 			msg.Command, target, body, nil,
 		)
 		if err != nil {
@@ -199,7 +199,7 @@ func (c *Conn) joinChannel(
 	channel, key string,
 ) {
 	result, err := c.svc.JoinChannel(
-		ctx, c.sessionID, c.clientID, c.nick, channel, key,
+		ctx, c.sessionID, c.clientID, c.currentNick(), channel, key,
 	)
 	if err != nil {
 		c.sendIRCError(err)
@@ -337,7 +337,7 @@ func (c *Conn) partChannel(
 	channel, reason string,
 ) {
 	err := c.svc.PartChannel(
-		ctx, c.sessionID, c.clientID, c.nick, channel, reason,
+		ctx, c.sessionID, c.clientID, c.currentNick(), channel, reason,
 	)
 	if err != nil {
 		c.sendIRCError(err)
@@ -401,7 +401,7 @@ func (c *Conn) handleTopic(
 	newTopic := msg.Params[1]
 
 	err := c.svc.SetTopic(
-		ctx, c.sessionID, c.clientID, c.nick, channel, newTopic,
+		ctx, c.sessionID, c.clientID, c.currentNick(), channel, newTopic,
 	)
 	if err != nil {
 		c.sendIRCError(err)
@@ -708,7 +708,7 @@ func (c *Conn) applyChannelModes(
 		c.send(FormatMessage(c.hostmask(), "MODE", params...))
 
 		c.svc.BroadcastMode(
-			ctx, c.clientID, c.nick, channel, chID, modeReply,
+			ctx, c.clientID, c.currentNick(), channel, chID, modeReply,
 		)
 	}
 }
@@ -1185,7 +1185,7 @@ func (c *Conn) handleKick(
 	}
 
 	err := c.svc.KickUser(
-		ctx, c.sessionID, c.clientID, c.nick,
+		ctx, c.sessionID, c.clientID, c.currentNick(),
 		channel, targetNick, reason,
 	)
 	if err != nil {
@@ -1225,7 +1225,7 @@ func (c *Conn) handlePassPostReg(
 		return
 	}
 
-	c.sendFromServer("NOTICE", c.nick,
+	c.sendFromServer("NOTICE", c.currentNick(),
 		"Password set. You can reconnect using "+
 			"PASS <password> with your nick.",
 	)
@@ -1249,7 +1249,7 @@ func (c *Conn) handleInvite(
 	channel := msg.Params[1]
 
 	targetID, err := c.svc.InviteUser(
-		ctx, c.sessionID, c.nick, targetNick, channel,
+		ctx, c.sessionID, c.currentNick(), targetNick, channel,
 	)
 	if err != nil {
 		c.sendIRCError(err)
@@ -1266,7 +1266,7 @@ func (c *Conn) handleInvite(
 	body, _ := json.Marshal([]string{channel}) //nolint:errchkjson
 
 	_, _, _ = c.svc.FanOut( //nolint:dogsled // fire-and-forget broadcast
-		ctx, "INVITE", c.nick, targetNick, targetID, 0,
+		ctx, "INVITE", c.currentNick(), targetNick, targetID, 0,
 		nil, body, nil, []int64{targetID}, 0,
 	)
 }

internal/ircserver/conn.go

@@ -345,7 +345,7 @@ func (c *Conn) sendNumeric(
 	code irc.IRCMessageType,
 	params ...string,
 ) {
-	nick := c.nick
+	nick := c.currentNick()
 	if nick == "" {
 		nick = "*"
 	}
@@ -369,9 +369,11 @@ func (c *Conn) sendFromServer(
 // hostmask returns the client's full hostmask
 // (nick!user@host).
 func (c *Conn) hostmask() string {
+	nick := c.currentNick()
+
 	user := c.username
 	if user == "" {
-		user = c.nick
+		user = nick
 	}
 
 	host := c.hostname
@@ -379,7 +381,7 @@ func (c *Conn) hostmask() string {
 		host = c.remoteIP
 	}
 
-	return c.nick + "!" + user + "@" + host
+	return nick + "!" + user + "@" + host
 }
 
 // handleMessage dispatches a parsed IRC message using
@@ -657,7 +659,7 @@ func (c *Conn) checkPasswordLength(pw string) bool {
 		return true
 	}
 
-	c.sendFromServer("NOTICE", c.nick,
+	c.sendFromServer("NOTICE", c.currentNick(),
 		"Password must be 8 to 72 bytes",
 	)
 

internal/ircserver/relay.go

@@ -92,7 +92,7 @@ func (c *Conn) drainQueue(ctx context.Context) bool {
 //
 //nolint:cyclop // dispatch table
 func (c *Conn) deliverIRCMessage(
-	_ context.Context,
+	ctx context.Context,
 	msg *db.IRCMessage,
 ) {
 	command := msg.Command
@@ -120,7 +120,7 @@ func (c *Conn) deliverIRCMessage(
 	case command == irc.CmdPart:
 		c.deliverPart(msg, text)
 	case command == irc.CmdNick:
-		c.deliverNickChange(msg, text)
+		c.deliverNickChange(ctx, msg, text)
 	case command == irc.CmdQuit:
 		c.deliverQuitMsg(msg, text)
 	case command == irc.CmdTopic:
@@ -230,11 +230,27 @@ func (c *Conn) deliverPart(msg *db.IRCMessage, text string) {
 	}
 }
 
-// deliverNickChange sends a NICK change notification.
+// deliverNickChange sends a NICK change notification. The
+// change may be its own session's, made from another client,
+// so the connection first takes the nick its session holds
+// now. It does not go by the line's old nick: the connection
+// may have taken that nick after another user gave it up. It
+// reads the nick under c.mu, so that a NICK sent on the
+// connection meanwhile is not undone.
 func (c *Conn) deliverNickChange(
+	ctx context.Context,
 	msg *db.IRCMessage,
 	newNick string,
 ) {
+	c.mu.Lock()
+
+	nick, err := c.database.GetSessionNick(ctx, c.sessionID)
+	if err == nil {
+		c.nick = nick
+	}
+
+	c.mu.Unlock()
+
 	prefix := msg.From + "!" + msg.From + "@*"
 
 	c.send(FormatMessage(prefix, "NICK", newNick))

internal/ircserver/relay_test.go

@@ -511,13 +511,10 @@ func TestRelayOwnSessionOnce(t *testing.T) {
 	}
 }
 
-// ownSessionCopies connects alice to the IRC listener and has
-// her join #chan, which bob is on too, then adds an HTTP API
-// client to her session with the password her connection set,
-// as POST /api/v1/login does. It makes the command line from
-// her connection, or with api from the HTTP API client when api
-// is not nil, and returns how many copies of it her connection
-// and her HTTP API client got.
+// ownSessionCopies sets up alice with aliceWithHTTPClient. It
+// makes the command line from her connection, or with api from
+// the HTTP API client when api is not nil, and returns how many
+// copies of it her connection and her HTTP API client got.
 func ownSessionCopies(
 	t *testing.T,
 	line string,
@@ -525,27 +522,9 @@ func ownSessionCopies(
 ) (int, int) {
 	t.Helper()
 
-	const password = "alicepass"
-
 	ctx := t.Context()
 	env := newTestEnv(t)
-
-	alice := env.dial(t)
-	alice.send("PASS " + password)
-	alice.register("alice")
-	// Registration stores the password before the JOIN runs.
-	alice.joinAndDrain("#chan")
-
-	bob := env.dial(t)
-	bob.register("bob")
-	bob.joinAndDrain("#chan")
-
-	sessionID, httpClientID, _, err := env.database.LoginUser(
-		ctx, "alice", password, "127.0.0.1", "localhost",
-	)
-	if err != nil {
-		t.Fatalf("log in as alice: %v", err)
-	}
+	alice, _, sessionID, httpClientID := aliceWithHTTPClient(t, env)
 
 	var lines []string
 
@@ -555,7 +534,7 @@ func ownSessionCopies(
 		// Her echo comes before the reply to her next command.
 		lines = alice.sendAndExpect("PING :done", "PONG")
 	} else {
-		err = api(ctx, env.srv.Service(), sessionID)
+		err := api(ctx, env.srv.Service(), sessionID)
 		if err != nil {
 			t.Fatalf("%s over the HTTP API: %v", line, err)
 		}
@@ -601,3 +580,118 @@ func ownSessionCopies(
 
 	return ircCopies, httpCopies
 }
+
+// aliceWithHTTPClient connects alice to the IRC listener and
+// has her join #chan, which bob is on too, then adds an HTTP API
+// client to her session with the password her connection set,
+// as POST /api/v1/login does. It returns her connection, bob's
+// connection, her session and her HTTP API client.
+func aliceWithHTTPClient(
+	t *testing.T, env *testEnv,
+) (*testClient, *testClient, int64, int64) {
+	t.Helper()
+
+	const password = "alicepass"
+
+	alice := env.dial(t)
+	alice.send("PASS " + password)
+	alice.register("alice")
+	// Registration stores the password before the JOIN runs.
+	alice.joinAndDrain("#chan")
+
+	bob := env.dial(t)
+	bob.register("bob")
+	bob.joinAndDrain("#chan")
+
+	sessionID, httpClientID, _, err := env.database.LoginUser(
+		t.Context(), "alice", password, "127.0.0.1", "localhost",
+	)
+	if err != nil {
+		t.Fatalf("log in as alice: %v", err)
+	}
+
+	return alice, bob, sessionID, httpClientID
+}
+
+// TestRelayNickChangeFromHTTPAPI checks that once alice's nick
+// is changed from her HTTP API client, her IRC listener
+// connection uses the new nick: what she then sends from it
+// reaches bob under the new nick.
+func TestRelayNickChangeFromHTTPAPI(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+	alice, bob, sessionID, _ := aliceWithHTTPClient(t, env)
+
+	bob.joinAndDrain("#other")
+
+	// As the HTTP API does.
+	err := env.srv.Service().ChangeNick(
+		t.Context(), sessionID, 0, "alice", "alice2",
+	)
+	if err != nil {
+		t.Fatalf("change nick over the HTTP API: %v", err)
+	}
+
+	// Her connection has taken the new nick once it has sent her
+	// the change.
+	const change = ":alice!alice@* NICK alice2"
+
+	alice.readUntil(func(l string) bool { return l == change })
+	bob.readUntil(func(l string) bool { return l == change })
+
+	for _, step := range []struct {
+		line, want string
+	}{
+		{"PRIVMSG #chan :hi there", ":alice2!alice2@* PRIVMSG #chan :hi there"},
+		{"JOIN #other", ":alice2!alice2@* JOIN #other"},
+		{"NICK alice3", ":alice2!alice2@* NICK alice3"},
+	} {
+		alice.send(step.line)
+
+		got := bob.readLine()
+		if got != step.want {
+			t.Errorf("%s: got %q, want %q", step.line, got, step.want)
+		}
+	}
+}
+
+// TestRelayNickChangeKeepsTakenNick checks that an IRC
+// listener connection which takes a nick another user has
+// given up keeps it when that user's change reaches it
+// afterwards: what it then sends goes out under the nick its
+// session holds.
+func TestRelayNickChangeKeepsTakenNick(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	bob := env.dial(t)
+	bob.register("bob")
+	bob.sendAndExpect("NICK bob2", " NICK bob2")
+
+	alice := env.dial(t)
+	alice.register("alice")
+	alice.sendAndExpect("NICK bob", " NICK bob")
+	alice.joinAndDrain("#chan")
+
+	bob.joinAndDrain("#chan")
+
+	// Bob's change reaches alice only now, as it does when she
+	// takes his old nick before her connection has passed the
+	// change on. She holds bob now, so it is queued to bob.
+	env.queueMessage(t, "bob", "NICK", "bob", "", []string{"bob2"})
+
+	alice.readUntil(func(l string) bool {
+		return l == ":bob!bob@* NICK bob2"
+	})
+
+	alice.send("PRIVMSG #chan :hi there")
+
+	const want = ":bob!bob@* PRIVMSG #chan :hi there"
+
+	got := bob.readLine()
+	if got != want {
+		t.Errorf("got %q, want %q", got, want)
+	}
+}