Run JOIN, PART and KICK as one transaction each (closes #153)

JoinChannel made the channel, counted its members, ran the join
checks and inserted the member as separate statements, so two
requests could each decide on a count the other was about to change:
+l was exceeded, two first joiners both became operator, and a JOIN
racing the last PART failed its foreign key and answered 500.

db.CreateOrJoinChannel now does all of that, and uses up the invite,
in one transaction; the join checks moved into it, and the service
turns its refusals into the same IRC errors as before. PART and KICK
remove the member and delete an empty channel in one transaction
(db.PartChannelAndDeleteIfEmpty).

Model: opus-5-5

README.md

@@ -2677,6 +2677,9 @@ IRC_LISTEN_ADDR: ""
   An optional `PASS` before registration sets the session password (minimum 8
   characters). Registering creates a session without a hashcash stamp; closing
   the connection ends the session, as `QUIT` does.
+- **PING**: a connection that sends nothing for 90 seconds is sent a `PING`, and
+  is closed if it sends nothing in the 30 seconds after that; any line, such as
+  the client's `PONG`, starts both waits again.
 - **CAP negotiation**: `CAP LS` gets an empty capability list and `CAP END` is
   ignored, for compatibility with modern clients.
 - **Channel prefixes**: Channels must start with `#`. `JOIN` prepends it if

internal/db/queries.go

@@ -462,46 +462,207 @@ func (database *Database) JoinChannel(
 	return nil
 }
 
-// JoinChannelAsOperator adds a session to a channel with
-// operator status. Used when a user creates a new channel.
-func (database *Database) JoinChannelAsOperator(
+// Errors that CreateOrJoinChannel refuses a join with.
+var (
+	ErrBannedFromChannel = errors.New("banned from channel")
+	ErrInviteOnlyChannel = errors.New("channel is invite-only")
+	ErrBadChannelKey     = errors.New("wrong channel key")
+	ErrChannelIsFull     = errors.New("channel is full")
+)
+
+// CreateOrJoinChannel adds a session to the channel called
+// name, creating the channel if it does not exist. The
+// session is made operator if the channel has no members;
+// otherwise checkJoinRestrictions may refuse it. A join
+// uses up the session's invite to the channel. It returns
+// the channel ID and whether the session was made operator.
+// All of it is one transaction, and the database has one
+// connection, so every other statement waits until the
+// join is decided.
+func (database *Database) CreateOrJoinChannel(
 	ctx context.Context,
-	channelID, sessionID int64,
-) error {
-	_, err := database.conn.ExecContext(ctx,
+	name string,
+	sessionID int64,
+	suppliedKey string,
+) (int64, bool, error) {
+	transaction, err := database.conn.BeginTx(ctx, nil)
+	if err != nil {
+		return 0, false, fmt.Errorf("begin join: %w", err)
+	}
+
+	defer func() { _ = transaction.Rollback() }()
+
+	now := time.Now()
+
+	_, err = transaction.ExecContext(ctx,
+		`INSERT OR IGNORE INTO channels
+		 (name, created_at, updated_at)
+		 VALUES (?, ?, ?)`,
+		name, now, now)
+	if err != nil {
+		return 0, false, fmt.Errorf("create channel: %w", err)
+	}
+
+	var channelID, memberCount int64
+
+	err = transaction.QueryRowContext(ctx,
+		`SELECT id,
+		   (SELECT COUNT(*) FROM channel_members
+		    WHERE channel_id = channels.id)
+		 FROM channels WHERE name = ?`,
+		name,
+	).Scan(&channelID, &memberCount)
+	if err != nil {
+		return 0, false, fmt.Errorf("get channel: %w", err)
+	}
+
+	isOperator := memberCount == 0
+	if !isOperator {
+		err = checkJoinRestrictions(
+			ctx, transaction, channelID, sessionID,
+			suppliedKey, memberCount,
+		)
+		if err != nil {
+			return 0, false, err
+		}
+	}
+
+	_, err = transaction.ExecContext(ctx,
 		`INSERT OR IGNORE INTO channel_members
 		 (channel_id, session_id, is_operator, joined_at)
-		 VALUES (?, ?, 1, ?)`,
-		channelID, sessionID, time.Now())
+		 VALUES (?, ?, ?, ?)`,
+		channelID, sessionID, isOperator, now)
 	if err != nil {
-		return fmt.Errorf(
-			"join channel as operator: %w", err,
-		)
+		return 0, false, fmt.Errorf("join channel: %w", err)
+	}
+
+	_, err = transaction.ExecContext(ctx,
+		`DELETE FROM channel_invites
+		 WHERE channel_id = ? AND session_id = ?`,
+		channelID, sessionID)
+	if err != nil {
+		return 0, false, fmt.Errorf("clear invite: %w", err)
+	}
+
+	err = transaction.Commit()
+	if err != nil {
+		return 0, false, fmt.Errorf("commit join: %w", err)
+	}
+
+	return channelID, isOperator, nil
+}
+
+// checkJoinRestrictions returns the error that refuses a
+// session's join of a channel with memberCount members:
+// ErrBannedFromChannel when a ban matches its hostmask,
+// ErrInviteOnlyChannel when the channel is +i and the
+// session has no invite, ErrBadChannelKey when suppliedKey
+// is not the +k key, or ErrChannelIsFull when the channel
+// has its +l number of members. It returns nil when none
+// applies.
+func checkJoinRestrictions(
+	ctx context.Context,
+	transaction *sql.Tx,
+	channelID, sessionID int64,
+	suppliedKey string,
+	memberCount int64,
+) error {
+	banned, err := isSessionBannedInTx(
+		ctx, transaction, channelID, sessionID,
+	)
+	if err != nil {
+		return err
+	}
+
+	if banned {
+		return ErrBannedFromChannel
+	}
+
+	var (
+		inviteOnly, invited bool
+		key                 string
+		userLimit           int64
+	)
+
+	err = transaction.QueryRowContext(ctx,
+		`SELECT is_invite_only, channel_key, user_limit,
+		   EXISTS (SELECT 1 FROM channel_invites
+		           WHERE channel_id = channels.id
+		             AND session_id = ?)
+		 FROM channels WHERE id = ?`,
+		sessionID, channelID,
+	).Scan(&inviteOnly, &key, &userLimit, &invited)
+	if err != nil {
+		return fmt.Errorf("get channel modes: %w", err)
+	}
+
+	switch {
+	case inviteOnly && !invited:
+		return ErrInviteOnlyChannel
+	case key != "" && suppliedKey != key:
+		return ErrBadChannelKey
+	case userLimit > 0 && memberCount >= userLimit:
+		return ErrChannelIsFull
 	}
 
 	return nil
 }
 
-// CountChannelMembers returns the number of members in
-// a channel.
-func (database *Database) CountChannelMembers(
+// isSessionBannedInTx is IsSessionBanned, reading through
+// transaction.
+func isSessionBannedInTx(
 	ctx context.Context,
-	channelID int64,
-) (int64, error) {
-	var count int64
+	transaction *sql.Tx,
+	channelID, sessionID int64,
+) (bool, error) {
+	var nick, username, hostname string
 
-	err := database.conn.QueryRowContext(ctx,
-		`SELECT COUNT(*) FROM channel_members
-		 WHERE channel_id = ?`,
-		channelID,
-	).Scan(&count)
+	err := transaction.QueryRowContext(ctx,
+		`SELECT nick, username, hostname
+		 FROM sessions WHERE id = ?`,
+		sessionID,
+	).Scan(&nick, &username, &hostname)
 	if err != nil {
-		return 0, fmt.Errorf(
-			"count channel members: %w", err,
+		return false, fmt.Errorf(
+			"get session hostmask: %w", err,
 		)
 	}
 
-	return count, nil
+	hostmask := FormatHostmask(nick, username, hostname)
+
+	rows, err := transaction.QueryContext(ctx,
+		`SELECT mask FROM channel_bans
+		 WHERE channel_id = ?`,
+		channelID)
+	if err != nil {
+		return false, fmt.Errorf("list channel bans: %w", err)
+	}
+
+	defer func() { _ = rows.Close() }()
+
+	for rows.Next() {
+		var mask string
+
+		err = rows.Scan(&mask)
+		if err != nil {
+			return false, fmt.Errorf(
+				"scan channel ban: %w", err,
+			)
+		}
+
+		if MatchBanMask(mask, hostmask) {
+			return true, nil
+		}
+	}
+
+	err = rows.Err()
+	if err != nil {
+		return false, fmt.Errorf(
+			"iterate channel bans: %w", err,
+		)
+	}
+
+	return false, nil
 }
 
 // IsChannelOperator checks if a session has operator
@@ -729,6 +890,49 @@ func (database *Database) DeleteChannelIfEmpty(
 	return nil
 }
 
+// PartChannelAndDeleteIfEmpty removes a session from a
+// channel and deletes the channel if that left it with no
+// members, in one transaction, so that no JOIN lands
+// between the two.
+func (database *Database) PartChannelAndDeleteIfEmpty(
+	ctx context.Context,
+	channelID, sessionID int64,
+) error {
+	transaction, err := database.conn.BeginTx(ctx, nil)
+	if err != nil {
+		return fmt.Errorf("begin part: %w", err)
+	}
+
+	defer func() { _ = transaction.Rollback() }()
+
+	_, err = transaction.ExecContext(ctx,
+		`DELETE FROM channel_members
+		 WHERE channel_id = ? AND session_id = ?`,
+		channelID, sessionID)
+	if err != nil {
+		return fmt.Errorf("part channel: %w", err)
+	}
+
+	_, err = transaction.ExecContext(ctx,
+		`DELETE FROM channels WHERE id = ?
+		 AND NOT EXISTS
+		 (SELECT 1 FROM channel_members
+		  WHERE channel_id = ?)`,
+		channelID, channelID)
+	if err != nil {
+		return fmt.Errorf(
+			"delete channel if empty: %w", err,
+		)
+	}
+
+	err = transaction.Commit()
+	if err != nil {
+		return fmt.Errorf("commit part: %w", err)
+	}
+
+	return nil
+}
+
 // scanChannels scans rows into a ChannelInfo slice.
 func scanChannels(
 	rows *sql.Rows,

internal/ircserver/conn.go

@@ -20,7 +20,6 @@ import (
 
 const (
 	maxLineLen      = 512
-	readTimeout     = 5 * time.Minute
 	writeTimeout    = 30 * time.Second
 	killWriteWindow = 2 * time.Second
 	dnsTimeout      = 3 * time.Second
@@ -214,17 +213,31 @@ func resolveHost(ctx context.Context, addr string) string {
 
 // serve is the main loop for a single IRC client connection.
 // Cancelling ctx when it returns stops the relay goroutine.
-func (c *Conn) serve(ctx context.Context) {
+// When nothing has been read for interval the client is sent
+// a PING, and when nothing is read within deadline after that
+// the connection is closed.
+func (c *Conn) serve(
+	ctx context.Context,
+	interval, deadline time.Duration,
+) {
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 	defer c.cleanup(ctx)
 
+	ping := time.AfterFunc(interval, func() {
+		c.send("PING :" + c.serverSfx)
+	})
+	defer ping.Stop()
+
 	scanner := bufio.NewScanner(c.conn)
 	scanner.Buffer(make([]byte, maxLineLen), maxLineLen)
 
 	for {
+		// Every line read starts both waits again.
+		ping.Reset(interval)
+
 		_ = c.conn.SetReadDeadline(
-			time.Now().Add(readTimeout),
+			time.Now().Add(interval + deadline),
 		)
 
 		if !scanner.Scan() {
@@ -366,7 +379,7 @@ func (c *Conn) handleMessage(
 	msg *Message,
 ) {
 	// Before registration, only NICK, USER, PASS, PING,
-	// QUIT, and CAP are accepted.
+	// PONG, QUIT, and CAP are accepted.
 	if !c.registered {
 		c.handlePreRegistration(ctx, msg)
 
@@ -452,6 +465,10 @@ func (c *Conn) handlePreRegistration(
 	case irc.CmdPing:
 		c.handlePing(msg)
 
+		return
+	case irc.CmdPong:
+		// The answer to serve's PING, which unregistered
+		// clients are sent too.
 		return
 	case irc.CmdQuit:
 		c.handleQuit(msg)

internal/ircserver/export_test.go

@@ -13,13 +13,16 @@ import (
 	"sneak.berlin/go/neoirc/internal/service"
 )
 
-// NewTestServer creates a Server suitable for testing.
+// NewTestServer creates a Server suitable for testing, whose
+// connections use interval and deadline in place of
+// pingInterval and pongDeadline.
 // The caller must call Stop() when finished.
 func NewTestServer(
 	log *slog.Logger,
 	cfg *config.Config,
 	database *db.Database,
 	brk *broker.Broker,
+	interval, deadline time.Duration,
 ) *Server {
 	globs := &globals.Globals{
 		Appname:   "neoirc",
@@ -38,6 +41,9 @@ func NewTestServer(
 		brk:      brk,
 		svc:      svc,
 		conns:    make(map[*Conn]struct{}),
+
+		pingInterval: interval,
+		pongDeadline: deadline,
 	}
 }
 
@@ -56,6 +62,13 @@ func (s *Server) Listener() net.Listener {
 	return s.listener
 }
 
+// PingTimes returns how long the server's connections wait
+// with nothing read before they are sent a PING, and after it
+// before they are closed.
+func (s *Server) PingTimes() (time.Duration, time.Duration) {
+	return s.pingInterval, s.pongDeadline
+}
+
 // KillWriteWindow is how long Disconnect allows each write
 // to a killed victim.
 const KillWriteWindow = killWriteWindow
@@ -64,6 +77,14 @@ const KillWriteWindow = killWriteWindow
 // output queue when no broker notification arrives.
 const PollInterval = pollInterval
 
+// PingInterval is how long the server waits with nothing read
+// from a connection before it sends a PING.
+const PingInterval = pingInterval
+
+// PongDeadline is how long the server waits after the PING
+// for a line before it closes the connection.
+const PongDeadline = pongDeadline
+
 // NewTestConn returns a Conn for nick on tcpConn, without a
 // server, database or service behind it.
 func NewTestConn(

internal/ircserver/server.go

@@ -6,6 +6,7 @@ import (
 	"log/slog"
 	"net"
 	"sync"
+	"time"
 
 	"go.uber.org/fx"
 	"sneak.berlin/go/neoirc/internal/broker"
@@ -38,6 +39,11 @@ type Server struct {
 	mu       sync.Mutex
 	conns    map[*Conn]struct{}
 	cancel   context.CancelFunc
+
+	// pingInterval and pongDeadline are the constants of the
+	// same names, except in tests, which shorten them.
+	pingInterval time.Duration
+	pongDeadline time.Duration
 }
 
 // New creates a new IRC Server and registers its lifecycle
@@ -57,6 +63,9 @@ func New(
 		listener: nil,
 		cancel:   nil,
 		mu:       sync.Mutex{},
+
+		pingInterval: pingInterval,
+		pongDeadline: pongDeadline,
 	}
 
 	listenAddr := params.Config.IRCListenAddr
@@ -153,7 +162,7 @@ func (s *Server) acceptLoop(ctx context.Context) {
 				s.mu.Unlock()
 			}()
 
-			client.serve(ctx)
+			client.serve(ctx, s.pingInterval, s.pongDeadline)
 		}()
 	}
 }

internal/ircserver/server_test.go

@@ -13,10 +13,13 @@ import (
 	"testing"
 	"time"
 
+	"go.uber.org/fx/fxtest"
 	"sneak.berlin/go/neoirc/internal/broker"
 	"sneak.berlin/go/neoirc/internal/config"
 	"sneak.berlin/go/neoirc/internal/db"
+	"sneak.berlin/go/neoirc/internal/globals"
 	"sneak.berlin/go/neoirc/internal/ircserver"
+	"sneak.berlin/go/neoirc/internal/logger"
 
 	_ "modernc.org/sqlite"
 )
@@ -70,6 +73,22 @@ func newTestEnvWithConfig(
 ) *testEnv {
 	t.Helper()
 
+	return newTestEnvWithPingTimes(
+		t, cfg, ircserver.PingInterval, ircserver.PongDeadline,
+	)
+}
+
+// newTestEnvWithPingTimes is newTestEnvWithConfig, except
+// that the server sends a connection a PING after interval
+// with nothing read, and closes it when nothing is read
+// within deadline after that.
+func newTestEnvWithPingTimes(
+	t *testing.T,
+	cfg *config.Config,
+	interval, deadline time.Duration,
+) *testEnv {
+	t.Helper()
+
 	// A random name, so that no other test environment, not
 	// even an earlier run of the same test, can share it.
 	dsn := fmt.Sprintf(
@@ -126,7 +145,9 @@ func newTestEnvWithConfig(
 		&slog.HandlerOptions{Level: slog.LevelError}, //nolint:exhaustruct
 	))
 
-	srv := ircserver.NewTestServer(log, cfg, database, brk)
+	srv := ircserver.NewTestServer(
+		log, cfg, database, brk, interval, deadline,
+	)
 
 	err = srv.Start(addr)
 	if err != nil {
@@ -174,6 +195,34 @@ func (env *testEnv) dial(t *testing.T) *testClient {
 	}
 }
 
+// The ping interval and PONG deadline for
+// newTestEnvWithShortPings. They differ, so that a test can
+// tell them apart; the PONG deadline is how long a test
+// client has to answer a PING.
+const (
+	testPingInterval = time.Second
+	testPongDeadline = 300 * time.Millisecond
+)
+
+// newTestEnvWithShortPings starts an IRC server that uses
+// testPingInterval and testPongDeadline in place of
+// pingInterval and pongDeadline.
+func newTestEnvWithShortPings(t *testing.T) *testEnv {
+	t.Helper()
+
+	return newTestEnvWithPingTimes(
+		t,
+		&config.Config{ServerName: testServerName},
+		testPingInterval, testPongDeadline,
+	)
+}
+
+// isServerPing reports whether line is the PING the test
+// server sends a quiet client.
+func isServerPing(line string) bool {
+	return line == "PING :"+testServerName
+}
+
 // testClient wraps a raw TCP connection with helpers.
 type testClient struct {
 	t       *testing.T
@@ -272,6 +321,24 @@ func (tc *testClient) readUntilClosed() []string {
 	return lines
 }
 
+// readServerPing reads until the PING the test server sends a
+// quiet client, and fails the test if it came sooner than
+// testPingInterval after lastSent, a time just before the
+// client sent its last line.
+func (tc *testClient) readServerPing(lastSent time.Time) {
+	tc.t.Helper()
+
+	tc.readUntil(isServerPing)
+
+	waited := time.Since(lastSent)
+	if waited < testPingInterval {
+		tc.t.Errorf(
+			"PING came %v after the client's last line, want at least %v",
+			waited, testPingInterval,
+		)
+	}
+}
+
 // assertContains checks that at least one line matches the
 // given substring.
 func assertContains(
@@ -388,6 +455,129 @@ func TestPingPong(t *testing.T) {
 	assertContains(t, lines, "PONG", "PONG response")
 }
 
+// TestQuietClientAnsweringPingStaysConnected checks that a
+// registered client that sends nothing is sent a PING once
+// the ping interval has passed since its last line, and stays
+// connected as long as it answers each one, whether with a
+// PONG or with another line.
+func TestQuietClientAnsweringPingStaysConnected(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name   string
+		answer string
+	}{
+		{name: "PONG", answer: "PONG :" + testServerName},
+		{name: "PING of its own", answer: "PING :answer"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			env := newTestEnvWithShortPings(t)
+			client := env.dial(t)
+
+			lastSent := time.Now()
+
+			client.register("answers")
+
+			// The second PING comes later than a client that
+			// answers none stays connected.
+			for range 2 {
+				client.readServerPing(lastSent)
+
+				lastSent = time.Now()
+
+				client.send(tt.answer)
+			}
+
+			client.sendAndExpect("PING :still here", "still here")
+		})
+	}
+}
+
+// TestQuietClientNotAnsweringPingIsClosed checks that a
+// registered client that does not answer the PING is
+// disconnected once the PONG deadline has passed after it.
+func TestQuietClientNotAnsweringPingIsClosed(t *testing.T) {
+	t.Parallel()
+
+	// latest leaves room for a loaded host, but is sooner than
+	// a read deadline of two ping intervals would close it.
+	const (
+		earliest = testPingInterval + testPongDeadline
+		latest   = 2 * testPingInterval
+	)
+
+	env := newTestEnvWithShortPings(t)
+	client := env.dial(t)
+
+	lastSent := time.Now()
+
+	client.register("silent")
+	client.readServerPing(lastSent)
+	client.readUntilClosed()
+
+	waited := time.Since(lastSent)
+	if waited < earliest || waited >= latest {
+		t.Errorf(
+			"closed %v after the client's last line, want %v to %v",
+			waited, earliest, latest,
+		)
+	}
+}
+
+// TestUnregisteredClientAnswersPing checks that a PONG before
+// registration, the answer to the PING unregistered clients
+// are sent too, gets no ERR_NOTREGISTERED.
+func TestUnregisteredClientAnswersPing(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnvWithShortPings(t)
+	client := env.dial(t)
+
+	client.readUntil(isServerPing)
+	client.send("PONG :" + testServerName)
+
+	lines := client.sendAndExpect("PING :after", "PONG")
+	assertNotContains(t, lines, " 451 ", "ERR_NOTREGISTERED")
+}
+
+// TestNewPingTimes checks that a server made by New sends a
+// PING after 90 seconds with nothing read, and closes the
+// connection 30 seconds after that, as README.md states.
+func TestNewPingTimes(t *testing.T) {
+	t.Parallel()
+
+	const (
+		wantInterval = 90 * time.Second
+		wantDeadline = 30 * time.Second
+	)
+
+	log, err := logger.New(nil, logger.Params{
+		Globals: &globals.Globals{},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// With no IRC_LISTEN_ADDR, New starts no listener and
+	// uses nothing else from its params.
+	srv := ircserver.New(fxtest.NewLifecycle(t), ircserver.Params{
+		Logger: log,
+		Config: &config.Config{},
+	})
+
+	interval, deadline := srv.PingTimes()
+	if interval != wantInterval || deadline != wantDeadline {
+		t.Errorf(
+			"PING after %v and close %v later, want %v and %v",
+			interval, deadline, wantInterval, wantDeadline,
+		)
+	}
+}
+
 // TestIdleCleanupKeepsActiveUser checks that a user who
 // sends commands is kept by the idle cleanup, and a user who
 // sends nothing is not.

internal/server/export_test.go

@@ -0,0 +1,12 @@
+package server
+
+import "github.com/go-chi/chi/v5"
+
+// NewTestSPAServer creates a Server whose router serves only
+// the embedded web client.
+func NewTestSPAServer() *Server {
+	srv := &Server{router: chi.NewRouter()}
+	srv.setupSPA()
+
+	return srv
+}

internal/server/routes.go

@@ -3,6 +3,7 @@ package server
 import (
 	"io/fs"
 	"net/http"
+	"strings"
 	"time"
 
 	"sneak.berlin/go/neoirc/web"
@@ -137,8 +138,10 @@ func (srv *Server) setupSPA() {
 			return
 		}
 
+		// The path is empty, not "/", for a request line such
+		// as "GET http://host HTTP/1.1".
 		fileData, readErr := readFS.ReadFile(
-			request.URL.Path[1:],
+			strings.TrimPrefix(request.URL.Path, "/"),
 		)
 		if readErr != nil || len(fileData) == 0 {
 			indexHTML, indexErr := readFS.ReadFile(

internal/server/routes_test.go

@@ -0,0 +1,41 @@
+package server_test
+
+import (
+	"io/fs"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"sneak.berlin/go/neoirc/internal/server"
+	"sneak.berlin/go/neoirc/web"
+)
+
+// An absolute-form request line with no path, such as
+// "GET http://example.com HTTP/1.1", leaves URL.Path empty.
+func TestSPAServesIndexForEmptyPath(t *testing.T) {
+	t.Parallel()
+
+	request := httptest.NewRequestWithContext(
+		t.Context(), http.MethodGet, "http://example.com", nil,
+	)
+	if request.URL.Path != "" {
+		t.Fatalf("URL.Path is %q, want it empty", request.URL.Path)
+	}
+
+	recorder := httptest.NewRecorder()
+
+	server.NewTestSPAServer().ServeHTTP(recorder, request)
+
+	if recorder.Code != http.StatusOK {
+		t.Fatalf("status %d, want %d", recorder.Code, http.StatusOK)
+	}
+
+	indexHTML, err := fs.ReadFile(web.Dist, "dist/index.html")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if recorder.Body.String() != string(indexHTML) {
+		t.Errorf("body is not the index page")
+	}
+}

internal/service/service.go

@@ -536,41 +536,13 @@ func (s *Service) JoinChannel(
 		}
 	}
 
-	chID, err := s.db.GetOrCreateChannel(ctx, channel)
-	if err != nil {
-		return nil, fmt.Errorf("get/create channel: %w", err)
-	}
-
-	memberCount, countErr := s.db.CountChannelMembers(
-		ctx, chID,
+	chID, isCreator, err := s.db.CreateOrJoinChannel(
+		ctx, channel, sessionID, suppliedKey,
 	)
-	isCreator := countErr == nil && memberCount == 0
-
-	if !isCreator {
-		joinErr := checkJoinRestrictions(
-			ctx, s.db, chID, sessionID,
-			channel, suppliedKey, memberCount,
-		)
-		if joinErr != nil {
-			return nil, joinErr
-		}
-	}
-
-	if isCreator {
-		err = s.db.JoinChannelAsOperator(
-			ctx, chID, sessionID,
-		)
-	} else {
-		err = s.db.JoinChannel(ctx, chID, sessionID)
-	}
-
 	if err != nil {
-		return nil, fmt.Errorf("join channel: %w", err)
+		return nil, joinError(channel, err)
 	}
 
-	// Clear invite after successful join.
-	_ = s.db.ClearChannelInvite(ctx, chID, sessionID)
-
 	memberIDs, _ := s.db.GetChannelMemberIDs(ctx, chID)
 	body, _ := json.Marshal([]string{channel}) //nolint:errchkjson
 
@@ -622,8 +594,7 @@ func (s *Service) PartChannel(
 		nil, body, nil, recipients,
 	)
 
-	s.db.PartChannel(ctx, chID, sessionID) //nolint:errcheck,gosec
-	s.db.DeleteChannelIfEmpty(ctx, chID)   //nolint:errcheck,gosec
+	s.db.PartChannelAndDeleteIfEmpty(ctx, chID, sessionID) //nolint:errcheck,gosec
 
 	return nil
 }
@@ -748,8 +719,7 @@ func (s *Service) KickUser(
 		params, body, nil, memberIDs,
 	)
 
-	s.db.PartChannel(ctx, chID, targetSID) //nolint:errcheck,gosec
-	s.db.DeleteChannelIfEmpty(ctx, chID)   //nolint:errcheck,gosec
+	s.db.PartChannelAndDeleteIfEmpty(ctx, chID, targetSID) //nolint:errcheck,gosec
 
 	return nil
 }
@@ -1392,54 +1362,30 @@ func (s *Service) broadcastNickChange(
 	}
 }
 
-// checkJoinRestrictions validates Tier 2 join conditions:
-// bans, invite-only, channel key, and user limit.
-func checkJoinRestrictions(
-	ctx context.Context,
-	database *db.Database,
-	chID, sessionID int64,
-	channel, suppliedKey string,
-	memberCount int64,
-) error {
-	isBanned, banErr := database.IsSessionBanned(
-		ctx, chID, sessionID,
-	)
-	if banErr == nil && isBanned {
+// 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.
+func joinError(channel string, err error) error {
+	switch {
+	case errors.Is(err, db.ErrBannedFromChannel):
 		return &IRCError{
 			Code:    irc.ErrBannedFromChan,
 			Params:  []string{channel},
 			Message: "Cannot join channel (+b)",
 		}
-	}
-
-	isInviteOnly, ioErr := database.IsChannelInviteOnly(
-		ctx, chID,
-	)
-	if ioErr == nil && isInviteOnly {
-		hasInvite, invErr := database.HasChannelInvite(
-			ctx, chID, sessionID,
-		)
-		if invErr != nil || !hasInvite {
-			return &IRCError{
-				Code:    irc.ErrInviteOnlyChan,
-				Params:  []string{channel},
-				Message: "Cannot join channel (+i)",
-			}
+	case errors.Is(err, db.ErrInviteOnlyChannel):
+		return &IRCError{
+			Code:    irc.ErrInviteOnlyChan,
+			Params:  []string{channel},
+			Message: "Cannot join channel (+i)",
 		}
-	}
-
-	key, keyErr := database.GetChannelKey(ctx, chID)
-	if keyErr == nil && key != "" && suppliedKey != key {
+	case errors.Is(err, db.ErrBadChannelKey):
 		return &IRCError{
 			Code:    irc.ErrBadChannelKey,
 			Params:  []string{channel},
 			Message: "Cannot join channel (+k)",
 		}
-	}
-
-	limit, limErr := database.GetChannelUserLimit(ctx, chID)
-	if limErr == nil && limit > 0 &&
-		memberCount >= int64(limit) {
+	case errors.Is(err, db.ErrChannelIsFull):
 		return &IRCError{
 			Code:    irc.ErrChannelIsFull,
 			Params:  []string{channel},
@@ -1447,7 +1393,7 @@ func checkJoinRestrictions(
 		}
 	}
 
-	return nil
+	return fmt.Errorf("join channel: %w", err)
 }
 
 // extractHashcashFromMeta parses the meta JSON and

internal/service/service_test.go

@@ -13,6 +13,7 @@ import (
 	"os"
 	"slices"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -255,6 +256,402 @@ func TestPartChannel(t *testing.T) {
 	}
 }
 
+// concurrentJoinTrials is how many times each test of
+// concurrent JOINs repeats its race. Before a JOIN ran as
+// one transaction, ten trials lost each race every time.
+const concurrentJoinTrials = 10
+
+// runConcurrently runs each of funcs in its own goroutine,
+// releasing them all at once, and returns their errors in
+// the order of funcs.
+func runConcurrently(funcs ...func() error) []error {
+	start := make(chan struct{})
+	errs := make([]error, len(funcs))
+
+	var wg sync.WaitGroup
+
+	for i, run := range funcs {
+		wg.Add(1)
+
+		go func() {
+			defer wg.Done()
+
+			<-start
+
+			errs[i] = run()
+		}()
+	}
+
+	close(start)
+	wg.Wait()
+
+	return errs
+}
+
+// joinConcurrently creates a session for each nick and has
+// them all JOIN channel at once. It returns the result and
+// error of each JOIN, in the order of nicks.
+func joinConcurrently(
+	ctx context.Context,
+	t *testing.T,
+	env *testEnv,
+	channel string,
+	nicks ...string,
+) ([]*service.JoinResult, []error) {
+	t.Helper()
+
+	results := make([]*service.JoinResult, len(nicks))
+	joins := make([]func() error, len(nicks))
+
+	for i, nick := range nicks {
+		sessionID := createSession(ctx, t, env.db, nick)
+
+		joins[i] = func() error {
+			result, err := env.svc.JoinChannel(
+				ctx, sessionID, nick, channel, "",
+			)
+			results[i] = result
+
+			return err
+		}
+	}
+
+	return results, runConcurrently(joins...)
+}
+
+// TestConcurrentJoinsRespectUserLimit checks that of two
+// concurrent JOINs of a +l 2 channel with one member, one
+// gets in and the other gets ERR_CHANNELISFULL.
+func TestConcurrentJoinsRespectUserLimit(t *testing.T) {
+	const userLimit = 2
+
+	env := newTestEnv(t)
+	ctx := t.Context()
+
+	for trial := range concurrentJoinTrials {
+		channel := fmt.Sprintf("#full%d", trial)
+		opNick := fmt.Sprintf("op%d", trial)
+		opSID := createSession(ctx, t, env.db, opNick)
+
+		created, err := env.svc.JoinChannel(
+			ctx, opSID, opNick, channel, "",
+		)
+		if err != nil {
+			t.Fatalf("join %s: %v", opNick, err)
+		}
+
+		err = env.db.SetChannelUserLimit(
+			ctx, created.ChannelID, userLimit,
+		)
+		if err != nil {
+			t.Fatalf("set +l: %v", err)
+		}
+
+		_, errs := joinConcurrently(
+			ctx, t, env, channel,
+			fmt.Sprintf("a%d", trial), fmt.Sprintf("b%d", trial),
+		)
+
+		refused := 0
+
+		for _, joinErr := range errs {
+			var ircErr *service.IRCError
+
+			switch {
+			case joinErr == nil:
+			case errors.As(joinErr, &ircErr) &&
+				ircErr.Code == irc.ErrChannelIsFull:
+				refused++
+			default:
+				t.Errorf("trial %d: JOIN: %v", trial, joinErr)
+			}
+		}
+
+		members, err := env.db.ChannelMembers(ctx, created.ChannelID)
+		if err != nil {
+			t.Fatalf("channel members: %v", err)
+		}
+
+		if len(members) != userLimit || refused != 1 {
+			t.Errorf(
+				"trial %d: %d members and %d refused, want %d and 1",
+				trial, len(members), refused, userLimit,
+			)
+		}
+	}
+}
+
+// TestConcurrentFirstJoinsMakeOneOperator checks that of
+// two concurrent JOINs that create a channel, exactly one
+// makes its session the operator.
+func TestConcurrentFirstJoinsMakeOneOperator(t *testing.T) {
+	env := newTestEnv(t)
+	ctx := t.Context()
+
+	for trial := range concurrentJoinTrials {
+		results, errs := joinConcurrently(
+			ctx, t, env, fmt.Sprintf("#new%d", trial),
+			fmt.Sprintf("first%d", trial),
+			fmt.Sprintf("second%d", trial),
+		)
+
+		creators := 0
+
+		for i, err := range errs {
+			if err != nil {
+				t.Fatalf("trial %d: JOIN: %v", trial, err)
+			}
+
+			if results[i].IsCreator {
+				creators++
+			}
+		}
+
+		members, err := env.db.ChannelMembers(
+			ctx, results[0].ChannelID,
+		)
+		if err != nil {
+			t.Fatalf("channel members: %v", err)
+		}
+
+		operators := 0
+
+		for _, member := range members {
+			if member.IsOperator {
+				operators++
+			}
+		}
+
+		if creators != 1 || operators != 1 {
+			t.Errorf(
+				"trial %d: %d creators and %d operators, want 1 each",
+				trial, creators, operators,
+			)
+		}
+	}
+}
+
+// TestJoinConcurrentWithLastPart checks that a JOIN
+// concurrent with the PART of a channel's only member
+// either joins a channel that still exists or gets
+// ERR_NOSUCHCHANNEL, and never fails otherwise.
+func TestJoinConcurrentWithLastPart(t *testing.T) {
+	env := newTestEnv(t)
+	ctx := t.Context()
+
+	for trial := range concurrentJoinTrials {
+		channel := fmt.Sprintf("#last%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)
+
+		_, err := env.svc.JoinChannel(
+			ctx, leaverSID, leaverNick, channel, "",
+		)
+		if err != nil {
+			t.Fatalf("join %s: %v", leaverNick, err)
+		}
+
+		errs := runConcurrently(
+			func() error {
+				return env.svc.PartChannel(
+					ctx, leaverSID, leaverNick, channel, "",
+				)
+			},
+			func() error {
+				_, joinErr := env.svc.JoinChannel(
+					ctx, joinerSID, joinerNick, channel, "",
+				)
+
+				return joinErr
+			},
+		)
+		if errs[0] != nil {
+			t.Fatalf("trial %d: PART: %v", trial, errs[0])
+		}
+
+		var ircErr *service.IRCError
+
+		switch {
+		case errs[1] == nil:
+			chID, lookupErr := env.db.GetChannelByName(ctx, channel)
+			isMember, _ := env.db.IsChannelMember(ctx, chID, joinerSID)
+
+			if lookupErr != nil || !isMember {
+				t.Errorf(
+					"trial %d: JOIN succeeded, but %s is not on %s",
+					trial, joinerNick, channel,
+				)
+			}
+		case errors.As(errs[1], &ircErr) &&
+			ircErr.Code == irc.ErrNoSuchChannel:
+		default:
+			t.Errorf(
+				"trial %d: JOIN got %v, want success or 403",
+				trial, errs[1],
+			)
+		}
+	}
+}
+
+// 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.
+func TestJoinUsesUpInvite(t *testing.T) {
+	const channel = "#club"
+
+	env := newTestEnv(t)
+	ctx := t.Context()
+
+	opSID := createSession(ctx, t, env.db, "op")
+	guestSID := createSession(ctx, t, env.db, "guest")
+
+	created, err := env.svc.JoinChannel(ctx, opSID, "op", channel, "")
+	if err != nil {
+		t.Fatalf("join op: %v", err)
+	}
+
+	err = env.db.SetChannelInviteOnly(ctx, created.ChannelID, true)
+	if err != nil {
+		t.Fatalf("set +i: %v", err)
+	}
+
+	_, err = env.svc.InviteUser(ctx, opSID, "op", "guest", channel)
+	if err != nil {
+		t.Fatalf("invite guest: %v", err)
+	}
+
+	_, err = env.svc.JoinChannel(ctx, guestSID, "guest", channel, "")
+	if err != nil {
+		t.Fatalf("invited join: %v", err)
+	}
+
+	err = env.svc.PartChannel(ctx, guestSID, "guest", channel, "")
+	if err != nil {
+		t.Fatalf("part guest: %v", err)
+	}
+
+	_, err = env.svc.JoinChannel(ctx, guestSID, "guest", channel, "")
+
+	var ircErr *service.IRCError
+	if !errors.As(err, &ircErr) || ircErr.Code != irc.ErrInviteOnlyChan {
+		t.Fatalf("second join: want ERR_INVITEONLYCHAN, got %v", err)
+	}
+}
+
+// TestJoinRefusedByHostmaskBan checks that a ban naming a
+// session's nick, username and hostname refuses its JOIN
+// with ERR_BANNEDFROMCHAN.
+func TestJoinRefusedByHostmaskBan(t *testing.T) {
+	const channel = "#club"
+
+	env := newTestEnv(t)
+	ctx := t.Context()
+
+	opSID := createSession(ctx, t, env.db, "op")
+
+	created, err := env.svc.JoinChannel(ctx, opSID, "op", channel, "")
+	if err != nil {
+		t.Fatalf("join op: %v", err)
+	}
+
+	err = env.db.AddChannelBan(
+		ctx, created.ChannelID, "guest!user@host.example", "op",
+	)
+	if err != nil {
+		t.Fatalf("add ban: %v", err)
+	}
+
+	guestSID, _, _, err := env.db.CreateSession(
+		ctx, "guest", "user", "host.example", "127.0.0.1",
+	)
+	if err != nil {
+		t.Fatalf("create session guest: %v", err)
+	}
+
+	_, err = env.svc.JoinChannel(ctx, guestSID, "guest", channel, "")
+
+	var ircErr *service.IRCError
+	if !errors.As(err, &ircErr) || ircErr.Code != irc.ErrBannedFromChan {
+		t.Fatalf("want ERR_BANNEDFROMCHAN, got %v", err)
+	}
+}
+
+// TestPartDeletesChannelOnlyWhenEmpty checks that a PART
+// keeps a channel other members are still on and deletes
+// the channel when its last member parts.
+func TestPartDeletesChannelOnlyWhenEmpty(t *testing.T) {
+	const channel = "#room"
+
+	env := newTestEnv(t)
+	ctx := t.Context()
+
+	aliceSID := createSession(ctx, t, env.db, "alice")
+	bobSID := createSession(ctx, t, env.db, "bob")
+
+	_, err := env.svc.JoinChannel(ctx, aliceSID, "alice", channel, "")
+	if err != nil {
+		t.Fatalf("join alice: %v", err)
+	}
+
+	_, err = env.svc.JoinChannel(ctx, bobSID, "bob", channel, "")
+	if err != nil {
+		t.Fatalf("join bob: %v", err)
+	}
+
+	err = env.svc.PartChannel(ctx, aliceSID, "alice", channel, "")
+	if err != nil {
+		t.Fatalf("part alice: %v", err)
+	}
+
+	chID, err := env.db.GetChannelByName(ctx, channel)
+	if err != nil {
+		t.Fatalf("channel gone while bob is on it: %v", err)
+	}
+
+	isMember, _ := env.db.IsChannelMember(ctx, chID, bobSID)
+	if !isMember {
+		t.Fatal("bob is not on the channel after alice parted")
+	}
+
+	err = env.svc.PartChannel(ctx, bobSID, "bob", channel, "")
+	if err != nil {
+		t.Fatalf("part bob: %v", err)
+	}
+
+	_, err = env.db.GetChannelByName(ctx, channel)
+	if err == nil {
+		t.Fatal("channel still exists after its last member parted")
+	}
+}
+
+// TestKickOfLastMemberDeletesChannel checks that a KICK
+// that leaves a channel with no members deletes it.
+func TestKickOfLastMemberDeletesChannel(t *testing.T) {
+	const channel = "#room"
+
+	env := newTestEnv(t)
+	ctx := t.Context()
+
+	sid := createSession(ctx, t, env.db, "alice")
+
+	_, err := env.svc.JoinChannel(ctx, sid, "alice", channel, "")
+	if err != nil {
+		t.Fatalf("join alice: %v", err)
+	}
+
+	err = env.svc.KickUser(ctx, sid, "alice", channel, "alice", "")
+	if err != nil {
+		t.Fatalf("kick alice: %v", err)
+	}
+
+	_, err = env.db.GetChannelByName(ctx, channel)
+	if err == nil {
+		t.Fatal("channel still exists after its last member was kicked")
+	}
+}
+
 func TestSendChannelMessage(t *testing.T) {
 	env := newTestEnv(t)
 	ctx := t.Context()