Broadcast a session's QUIT once when its clients leave together (closes #154)

Logout deleted the client, counted the session's clients and ran
BroadcastQuit as separate statements, and BroadcastQuit read the peers,
parted, deleted empty channels and deleted the session separately too.
Two logouts or QUITs of one session at once both sent its QUIT, and a
JOIN landing between a part and the channel's deletion got into a +k
channel without the key.

db.QuitSession now does all of that in one transaction, parting each
channel as PartChannelAndDeleteIfEmpty does, and reports whether it
deleted the session; only that caller sends the QUIT. db.LogoutClient
deletes the client and counts the rest in the same transaction.

Model: opus-5-5

internal/service/service.go

@@ -826,61 +826,42 @@ func (s *Service) ChangeNick(
 	return nil
 }
 
-// BroadcastQuit broadcasts a QUIT to all channel peers,
-// parts all channels, and deletes the session. Uses the
-// FanOut pattern: one message row fanned out to all unique
-// peer sessions.
+// BroadcastQuit parts the session from all its channels,
+// deletes it, and sends a QUIT to the users who shared a
+// channel with it. Of several calls for one session at
+// once, only the one that deletes it sends the QUIT.
 func (s *Service) BroadcastQuit(
 	ctx context.Context,
 	sessionID int64,
 	nick, reason string,
 ) {
-	channels, err := s.db.GetSessionChannels(
-		ctx, sessionID,
-	)
-	if err != nil {
+	peerIDs, deleted, err := s.db.QuitSession(ctx, sessionID)
+	if err != nil || !deleted {
 		return
 	}
 
-	notified := make(map[int64]bool)
+	s.sendQuit(ctx, nick, reason, peerIDs)
+}
 
-	for _, ch := range channels {
-		memberIDs, memErr := s.db.GetChannelMemberIDs(
-			ctx, ch.ID,
-		)
-		if memErr != nil {
-			continue
-		}
-
-		for _, mid := range memberIDs {
-			if mid == sessionID || notified[mid] {
-				continue
-			}
-
-			notified[mid] = true
-		}
+// Logout deletes a client of the session and, if that was
+// the session's last client, does what BroadcastQuit does.
+func (s *Service) Logout(
+	ctx context.Context,
+	sessionID, clientID int64,
+	nick string,
+) error {
+	peerIDs, deleted, err := s.db.LogoutClient(
+		ctx, sessionID, clientID,
+	)
+	if err != nil {
+		return fmt.Errorf("logout: %w", err)
 	}
 
-	if len(notified) > 0 {
-		recipients := make([]int64, 0, len(notified))
-		for sid := range notified {
-			recipients = append(recipients, sid)
-		}
-
-		body, _ := json.Marshal([]string{reason}) //nolint:errchkjson
-
-		_, _, _ = s.FanOut(
-			ctx, irc.CmdQuit, nick, "", 0, 0,
-			nil, body, nil, recipients,
-		)
+	if deleted {
+		s.sendQuit(ctx, nick, "Connection closed", peerIDs)
 	}
 
-	for _, ch := range channels {
-		s.db.PartChannel(ctx, ch.ID, sessionID) //nolint:errcheck,gosec
-		s.db.DeleteChannelIfEmpty(ctx, ch.ID)   //nolint:errcheck,gosec
-	}
-
-	s.db.DeleteSession(ctx, sessionID) //nolint:errcheck,gosec
+	return nil
 }
 
 // SetAway sets or clears the away message. Returns true
@@ -1362,6 +1343,26 @@ func (s *Service) broadcastNickChange(
 	}
 }
 
+// sendQuit sends the QUIT of nick, with reason, to the
+// sessions peerIDs, as one message row fanned out to all of
+// them.
+func (s *Service) sendQuit(
+	ctx context.Context,
+	nick, reason string,
+	peerIDs []int64,
+) {
+	if len(peerIDs) == 0 {
+		return
+	}
+
+	body, _ := json.Marshal([]string{reason}) //nolint:errchkjson
+
+	_, _, _ = s.FanOut( //nolint:dogsled // fire-and-forget broadcast
+		ctx, irc.CmdQuit, nick, "", 0, 0,
+		nil, body, nil, peerIDs,
+	)
+}
+
 // joinError returns the IRC error for a JOIN of channel
 // that db.CreateOrJoinChannel refused with err, or err
 // wrapped if it is not a refusal.

internal/service/service_test.go

@@ -495,6 +495,78 @@ func TestJoinConcurrentWithLastPart(t *testing.T) {
 	}
 }
 
+// TestJoinConcurrentWithLastQuit checks that a user who
+// sends JOIN without the key to a +k channel, again after
+// each ERR_BADCHANNELKEY, while the channel's only member
+// quits, gets in only once the channel is deleted, into a
+// new channel of that name with no key.
+func TestJoinConcurrentWithLastQuit(t *testing.T) {
+	env := newTestEnv(t)
+	ctx := t.Context()
+
+	for trial := range concurrentJoinTrials {
+		channel := fmt.Sprintf("#keyed%d", trial)
+		leaverNick := fmt.Sprintf("leaver%d", trial)
+		leaverSID := createSession(ctx, t, env.db, leaverNick)
+		joinerNick := fmt.Sprintf("joiner%d", trial)
+		joinerSID := createSession(ctx, t, env.db, joinerNick)
+
+		created, err := env.svc.JoinChannel(
+			ctx, leaverSID, leaverNick, channel, "",
+		)
+		if err != nil {
+			t.Fatalf("join %s: %v", leaverNick, err)
+		}
+
+		err = env.db.SetChannelKey(ctx, created.ChannelID, "secret")
+		if err != nil {
+			t.Fatalf("set +k: %v", err)
+		}
+
+		var joined *service.JoinResult
+
+		errs := runConcurrently(
+			func() error {
+				env.svc.BroadcastQuit(ctx, leaverSID, leaverNick, "")
+
+				return nil
+			},
+			// The database has one connection, so retried
+			// JOINs take turns with the QUIT's statements
+			// and land between any two that are not in one
+			// transaction. A JOIN after the QUIT gets in, so
+			// the loop ends.
+			func() error {
+				for {
+					var (
+						joinErr error
+						ircErr  *service.IRCError
+					)
+
+					joined, joinErr = env.svc.JoinChannel(
+						ctx, joinerSID, joinerNick, channel, "",
+					)
+					if !errors.As(joinErr, &ircErr) ||
+						ircErr.Code != irc.ErrBadChannelKey {
+						return joinErr
+					}
+				}
+			},
+		)
+		if errs[1] != nil {
+			t.Fatalf("trial %d: JOIN: %v", trial, errs[1])
+		}
+
+		key, _ := env.db.GetChannelKey(ctx, joined.ChannelID)
+		if key != "" {
+			t.Errorf(
+				"trial %d: %s got into +k %s without the key",
+				trial, joinerNick, channel,
+			)
+		}
+	}
+}
+
 // TestJoinUsesUpInvite checks that the JOIN an invite let
 // into a +i channel uses the invite up, so that a JOIN
 // after leaving gets ERR_INVITEONLYCHAN.