internal/db/queries.go

@@ -2415,6 +2415,71 @@ func (database *Database) SetChannelUserLimit(
 	return nil
 }
 
+// SetSessionUserModes applies a set of user-mode flag
+// changes to a session inside a single transaction, so a
+// multi-mode change such as "+w-o" is all-or-nothing. A nil
+// pointer means the caller did not mention that mode and
+// the stored value must be left untouched.
+func (database *Database) SetSessionUserModes(
+	ctx context.Context,
+	sessionID int64,
+	wallops *bool,
+	oper *bool,
+) error {
+	if wallops == nil && oper == nil {
+		return nil
+	}
+
+	transaction, err := database.conn.BeginTx(ctx, nil)
+	if err != nil {
+		return fmt.Errorf("begin tx: %w", err)
+	}
+
+	if wallops != nil {
+		if _, err := transaction.ExecContext(
+			ctx,
+			`UPDATE sessions SET is_wallops = ? WHERE id = ?`,
+			boolToInt(*wallops), sessionID,
+		); err != nil {
+			_ = transaction.Rollback()
+
+			return fmt.Errorf(
+				"set session wallops: %w", err,
+			)
+		}
+	}
+
+	if oper != nil {
+		if _, err := transaction.ExecContext(
+			ctx,
+			`UPDATE sessions SET is_oper = ? WHERE id = ?`,
+			boolToInt(*oper), sessionID,
+		); err != nil {
+			_ = transaction.Rollback()
+
+			return fmt.Errorf("set session oper: %w", err)
+		}
+	}
+
+	if err := transaction.Commit(); err != nil {
+		_ = transaction.Rollback()
+
+		return fmt.Errorf("commit user modes: %w", err)
+	}
+
+	return nil
+}
+
+// boolToInt renders a Go bool as the 0/1 integer used for
+// boolean columns in the SQLite schema.
+func boolToInt(value bool) int {
+	if value {
+		return 1
+	}
+
+	return 0
+}
+
 // SetSessionWallops sets the wallops (+w) flag on a
 // session.
 func (database *Database) SetSessionWallops(

internal/service/service.go

@@ -956,8 +956,11 @@ type userModeOp struct {
 // rejects malformed input (empty string, no leading sign,
 // bare sign with no mode letters, unknown mode letters,
 // +o which must be set via OPER) with an IRCError. On
-// failure, no persistent change is made. On success, the
+// failure, no persistent change is made: parsing happens
-// resulting mode string is returned.
+// before any write, and the writes themselves run inside a
+// single database transaction that is rolled back whole if
+// any statement fails. On success, the resulting mode
+// string is returned.
 func (s *Service) ApplyUserMode(
 	ctx context.Context,
 	sessionID int64,
@@ -968,12 +971,19 @@ func (s *Service) ApplyUserMode(
 		return "", err
 	}
 
-	for _, op := range ops {
+	wallops, oper, err := collapseUserModeOps(ops)
-		if err := s.applySingleUserMode(
+	if err != nil {
-			ctx, sessionID, op.char, op.adding,
-		); err != nil {
 		return "", err
 	}
+
+	if err := s.db.SetSessionUserModes(
+		ctx, sessionID, wallops, oper,
+	); err != nil {
+		s.log.Error(
+			"apply user modes failed", "error", err,
+		)
+
+		return "", fmt.Errorf("apply user modes: %w", err)
 	}
 
 	return s.QueryUserMode(ctx, sessionID)
@@ -1048,56 +1058,43 @@ func isKnownUserModeChar(modeChar rune) bool {
 	}
 }
 
-// applySingleUserMode applies one already-validated user
+// collapseUserModeOps reduces an already-parsed operation
-// mode character to the session. parseUserModeString must
+// list to the final desired value of each user mode flag.
-// have validated the character and sign before this runs;
+// A nil result for a flag means the mode string never
-// the default branch here is defence-in-depth only.
+// mentioned it, so it must be left untouched. Later
-func (s *Service) applySingleUserMode(
+// operations win over earlier ones for the same letter
-	ctx context.Context,
+// (e.g. "+w-w" ends with wallops off), which matches the
-	sessionID int64,
+// left-to-right semantics of applying each op in turn.
-	modeChar rune,
+// parseUserModeString must have validated every character
-	adding bool,
+// and sign before this runs; the default branch here is
-) error {
+// defence-in-depth only.
-	switch modeChar {
+func collapseUserModeOps(
+	ops []userModeOp,
+) (wallops, oper *bool, err error) {
+	unknownFlag := &IRCError{
+		Code:    irc.ErrUmodeUnknownFlag,
+		Params:  nil,
+		Message: "Unknown MODE flag",
+	}
+
+	for _, op := range ops {
+		switch op.char {
 		case 'w':
-		err := s.db.SetSessionWallops(
+			val := op.adding
-			ctx, sessionID, adding,
+			wallops = &val
-		)
-		if err != nil {
-			s.log.Error(
-				"set wallops mode failed", "error", err,
-			)
-
-			return fmt.Errorf("set wallops: %w", err)
-		}
 		case 'o':
-		if adding {
+			if op.adding {
-			return &IRCError{
+				return nil, nil, unknownFlag
-				Code:    irc.ErrUmodeUnknownFlag,
-				Params:  nil,
-				Message: "Unknown MODE flag",
-			}
 			}
 
-		err := s.db.SetSessionOper(
+			val := false
-			ctx, sessionID, false,
+			oper = &val
-		)
-		if err != nil {
-			s.log.Error(
-				"clear oper mode failed", "error", err,
-			)
-
-			return fmt.Errorf("clear oper: %w", err)
-		}
 		default:
-		return &IRCError{
+			return nil, nil, unknownFlag
-			Code:    irc.ErrUmodeUnknownFlag,
-			Params:  nil,
-			Message: "Unknown MODE flag",
 		}
 	}
 
-	return nil
+	return wallops, oper, nil
 }
 
 // broadcastNickChange notifies channel peers of a nick