diff --git a/internal/server/server.go b/internal/server/server.go index 35d2080..3c7014e 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -45,7 +45,6 @@ type Params struct { // It manages routing, middleware, and lifecycle. type Server struct { startupTime time.Time - exitCode int sentryEnabled bool log *slog.Logger ctx context.Context //nolint:containedctx // signal handling pattern @@ -143,7 +142,7 @@ func (srv *Server) enableSentry() { srv.sentryEnabled = true } -func (srv *Server) serve() int { +func (srv *Server) serve() { srv.ctx, srv.cancelFunc = context.WithCancel( context.Background(), ) @@ -168,8 +167,6 @@ func (srv *Server) serve() int { <-srv.ctx.Done() srv.cleanShutdown() - - return srv.exitCode } func (srv *Server) cleanupForExit() { @@ -177,8 +174,6 @@ func (srv *Server) cleanupForExit() { } func (srv *Server) cleanShutdown() { - srv.exitCode = 0 - ctxShutdown, shutdownCancel := context.WithTimeout( context.Background(), shutdownTimeout, ) diff --git a/internal/service/service.go b/internal/service/service.go index 05cdd51..de26722 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -880,23 +880,23 @@ func parseUserModeString( ops := make([]userModeOp, 0, len(modeStr)-1) adding := true - for _, ch := range modeStr { - switch ch { + for _, modeChar := range modeStr { + switch modeChar { case '+': adding = true case '-': adding = false default: - if !isKnownUserModeChar(ch) { + if !isKnownUserModeChar(modeChar) { return nil, unknownFlag } - if ch == 'o' && adding { + if modeChar == 'o' && adding { return nil, unknownFlag } ops = append(ops, userModeOp{ - char: ch, adding: adding, + char: modeChar, adding: adding, }) } } @@ -910,8 +910,8 @@ func parseUserModeString( // isKnownUserModeChar reports whether the character is a // recognized user mode letter. -func isKnownUserModeChar(ch rune) bool { - switch ch { +func isKnownUserModeChar(modeChar rune) bool { + switch modeChar { case 'w', 'o': return true default: diff --git a/internal/service/service_test.go b/internal/service/service_test.go index 5030ebe..2f58e8a 100644 --- a/internal/service/service_test.go +++ b/internal/service/service_test.go @@ -393,6 +393,248 @@ func TestQueryUserMode(t *testing.T) { } } +// applyUserModeCaseState is the subset of session user-mode +// state the rigorous TestApplyUserMode suite asserts on. It +// mirrors the columns (oper, wallops) that the parser is +// permitted to mutate. +type applyUserModeCaseState struct { + oper bool + wallops bool +} + +// applyUserModeCase describes one rigor-suite case for +// Service.ApplyUserMode: the pre-call DB state, the mode +// string input, and the expected post-call observable state +// (mode string on success, IRC error code on rejection, and +// persisted session state either way). +type applyUserModeCase struct { + name string + initialState applyUserModeCaseState + modeStr string + wantModes string + wantErr bool + wantErrCode irc.IRCMessageType + wantState applyUserModeCaseState +} + +// applyUserModeCases returns every case listed in sneak's +// review of PR #96 plus a few adjacent ones. Split across +// helpers by category so each stays under funlen. +func applyUserModeCases() []applyUserModeCase { + cases := applyUserModeHappyPathCases() + cases = append(cases, applyUserModeSignTransitionCases()...) + cases = append(cases, applyUserModeMalformedCases()...) + cases = append(cases, applyUserModeUnknownLetterCases()...) + + return cases +} + +// applyUserModeHappyPathCases covers valid single-char and +// multi-char-without-sign-transition mode operations. +func applyUserModeHappyPathCases() []applyUserModeCase { + return []applyUserModeCase{ + { + name: "+w from empty", + initialState: applyUserModeCaseState{oper: false, wallops: false}, + modeStr: "+w", + wantModes: "+w", + wantErr: false, + wantErrCode: 0, + wantState: applyUserModeCaseState{oper: false, wallops: true}, + }, + { + name: "-w from +w", + initialState: applyUserModeCaseState{oper: false, wallops: true}, + modeStr: "-w", + wantModes: "+", + wantErr: false, + wantErrCode: 0, + wantState: applyUserModeCaseState{oper: false, wallops: false}, + }, + { + name: "-o from +o", + initialState: applyUserModeCaseState{oper: true, wallops: false}, + modeStr: "-o", + wantModes: "+", + wantErr: false, + wantErrCode: 0, + wantState: applyUserModeCaseState{oper: false, wallops: false}, + }, + { + name: "-wo from +ow", + initialState: applyUserModeCaseState{oper: true, wallops: true}, + modeStr: "-wo", + wantModes: "+", + wantErr: false, + wantErrCode: 0, + wantState: applyUserModeCaseState{oper: false, wallops: false}, + }, + } +} + +// applyUserModeSignTransitionCases covers multi-char mode +// strings where '+' and '-' flip partway through. +o is +// never legal via MODE, so strings containing it must be +// rejected atomically. +func applyUserModeSignTransitionCases() []applyUserModeCase { + return []applyUserModeCase{ + { + name: "+w-o from +o", + initialState: applyUserModeCaseState{oper: true, wallops: false}, + modeStr: "+w-o", + wantModes: "+w", + wantErr: false, + wantErrCode: 0, + wantState: applyUserModeCaseState{oper: false, wallops: true}, + }, + { + // +o is rejected before any op applies; wallops + // stays set. + name: "-w+o always rejects +o", + initialState: applyUserModeCaseState{oper: true, wallops: true}, + modeStr: "-w+o", + wantModes: "", + wantErr: true, + wantErrCode: irc.ErrUmodeUnknownFlag, + wantState: applyUserModeCaseState{oper: true, wallops: true}, + }, + { + // Wallops must NOT be cleared; oper must NOT be + // cleared. Rejection is fully atomic. + name: "+o-w+w rejects because of +o", + initialState: applyUserModeCaseState{oper: true, wallops: true}, + modeStr: "+o-w+w", + wantModes: "", + wantErr: true, + wantErrCode: irc.ErrUmodeUnknownFlag, + wantState: applyUserModeCaseState{oper: true, wallops: true}, + }, + } +} + +// applyUserModeMalformedCases covers inputs that lack a +// leading '+' or '-' and inputs that consist of bare signs +// without mode letters. All must be rejected with no side +// effects. +func applyUserModeMalformedCases() []applyUserModeCase { + return []applyUserModeCase{ + { + name: "w no prefix rejects", + initialState: applyUserModeCaseState{oper: false, wallops: false}, + modeStr: "w", + wantModes: "", + wantErr: true, + wantErrCode: irc.ErrUmodeUnknownFlag, + wantState: applyUserModeCaseState{oper: false, wallops: false}, + }, + { + // Prove wallops is NOT cleared — the whole point + // of sneak's review. + name: "xw no prefix rejects (would have been" + + " silently -w before)", + initialState: applyUserModeCaseState{oper: false, wallops: true}, + modeStr: "xw", + wantModes: "", + wantErr: true, + wantErrCode: irc.ErrUmodeUnknownFlag, + wantState: applyUserModeCaseState{oper: false, wallops: true}, + }, + { + name: "empty string rejects", + initialState: applyUserModeCaseState{oper: false, wallops: false}, + modeStr: "", + wantModes: "", + wantErr: true, + wantErrCode: irc.ErrUmodeUnknownFlag, + wantState: applyUserModeCaseState{oper: false, wallops: false}, + }, + { + name: "bare + rejects", + initialState: applyUserModeCaseState{oper: false, wallops: false}, + modeStr: "+", + wantModes: "", + wantErr: true, + wantErrCode: irc.ErrUmodeUnknownFlag, + wantState: applyUserModeCaseState{oper: false, wallops: false}, + }, + { + name: "bare - rejects", + initialState: applyUserModeCaseState{oper: false, wallops: false}, + modeStr: "-", + wantModes: "", + wantErr: true, + wantErrCode: irc.ErrUmodeUnknownFlag, + wantState: applyUserModeCaseState{oper: false, wallops: false}, + }, + { + name: "+-+ rejects (no mode letters)", + initialState: applyUserModeCaseState{oper: false, wallops: false}, + modeStr: "+-+", + wantModes: "", + wantErr: true, + wantErrCode: irc.ErrUmodeUnknownFlag, + wantState: applyUserModeCaseState{oper: false, wallops: false}, + }, + } +} + +// applyUserModeUnknownLetterCases covers well-formed prefix +// strings that contain unknown mode letters. Rejection must +// be atomic: any valid letters before the invalid one must +// not persist. +func applyUserModeUnknownLetterCases() []applyUserModeCase { + return []applyUserModeCase{ + { + name: "-x+y rejects unknown -x", + initialState: applyUserModeCaseState{oper: false, wallops: false}, + modeStr: "-x+y", + wantModes: "", + wantErr: true, + wantErrCode: irc.ErrUmodeUnknownFlag, + wantState: applyUserModeCaseState{oper: false, wallops: false}, + }, + { + name: "+y-x rejects unknown +y", + initialState: applyUserModeCaseState{oper: false, wallops: false}, + modeStr: "+y-x", + wantModes: "", + wantErr: true, + wantErrCode: irc.ErrUmodeUnknownFlag, + wantState: applyUserModeCaseState{oper: false, wallops: false}, + }, + { + name: "+z unknown mode rejects", + initialState: applyUserModeCaseState{oper: false, wallops: false}, + modeStr: "+z", + wantModes: "", + wantErr: true, + wantErrCode: irc.ErrUmodeUnknownFlag, + wantState: applyUserModeCaseState{oper: false, wallops: false}, + }, + { + // Wallops must NOT be set. + name: "+wz rejects whole thing; +w side effect" + + " must NOT persist", + initialState: applyUserModeCaseState{oper: false, wallops: false}, + modeStr: "+wz", + wantModes: "", + wantErr: true, + wantErrCode: irc.ErrUmodeUnknownFlag, + wantState: applyUserModeCaseState{oper: false, wallops: false}, + }, + { + name: "+wo rejects whole thing; +w side effect" + + " must NOT persist", + initialState: applyUserModeCaseState{oper: false, wallops: false}, + modeStr: "+wo", + wantModes: "", + wantErr: true, + wantErrCode: irc.ErrUmodeUnknownFlag, + wantState: applyUserModeCaseState{oper: false, wallops: false}, + }, + } +} + // TestApplyUserMode is the rigorous table-driven suite for // the shared user-mode parser. It covers every case listed // in sneak's review of PR #96 plus a few adjacent ones. @@ -400,253 +642,159 @@ func TestQueryUserMode(t *testing.T) { // persisted session state, to prove that rejected input // leaves no side effects. func TestApplyUserMode(t *testing.T) { - type caseState struct { - oper bool - wallops bool - } - - // Each case starts from initialState, calls - // ApplyUserMode(modeStr), and verifies either the - // returned mode string (wantModes) or an IRCError with - // wantErrCode. After the call, the DB must match - // wantState — proving that rejected input made no - // change. - cases := []struct { - name string - initialState caseState - modeStr string - wantModes string - wantErr bool - wantErrCode irc.IRCMessageType - wantState caseState - }{ - // Happy path: single-char operations. - { - name: "+w from empty", - modeStr: "+w", - wantModes: "+w", - wantState: caseState{wallops: true}, - }, - { - name: "-w from +w", - initialState: caseState{wallops: true}, - modeStr: "-w", - wantModes: "+", - wantState: caseState{}, - }, - { - name: "-o from +o", - initialState: caseState{oper: true}, - modeStr: "-o", - wantModes: "+", - wantState: caseState{}, - }, - // Multi-char without sign transitions. - { - name: "-wo from +ow", - initialState: caseState{oper: true, wallops: true}, - modeStr: "-wo", - wantModes: "+", - wantState: caseState{}, - }, - // Multi-char with sign transitions. - { - name: "+w-o from +o", - initialState: caseState{oper: true}, - modeStr: "+w-o", - wantModes: "+w", - wantState: caseState{wallops: true}, - }, - { - name: "-w+o always rejects +o", - initialState: caseState{wallops: true, oper: true}, - modeStr: "-w+o", - wantErr: true, - wantErrCode: irc.ErrUmodeUnknownFlag, - // +o is rejected before any op applies; wallops - // stays set. - wantState: caseState{wallops: true, oper: true}, - }, - { - name: "+o-w+w rejects because of +o", - initialState: caseState{wallops: true, oper: true}, - modeStr: "+o-w+w", - wantErr: true, - wantErrCode: irc.ErrUmodeUnknownFlag, - // Wallops must NOT be cleared; oper must NOT be - // cleared. Rejection is fully atomic. - wantState: caseState{wallops: true, oper: true}, - }, - { - name: "-x+y rejects unknown -x", - modeStr: "-x+y", - wantErr: true, - wantErrCode: irc.ErrUmodeUnknownFlag, - wantState: caseState{}, - }, - { - name: "+y-x rejects unknown +y", - modeStr: "+y-x", - wantErr: true, - wantErrCode: irc.ErrUmodeUnknownFlag, - wantState: caseState{}, - }, - // Malformed prefix — must not silently treat as '-'. - { - name: "w no prefix rejects", - modeStr: "w", - wantErr: true, - wantErrCode: irc.ErrUmodeUnknownFlag, - wantState: caseState{}, - }, - { - name: "xw no prefix rejects (would have been" + - " silently -w before)", - initialState: caseState{wallops: true}, - modeStr: "xw", - wantErr: true, - wantErrCode: irc.ErrUmodeUnknownFlag, - // Prove wallops is NOT cleared — the whole point - // of sneak's review. - wantState: caseState{wallops: true}, - }, - { - name: "empty string rejects", - modeStr: "", - wantErr: true, - wantErrCode: irc.ErrUmodeUnknownFlag, - wantState: caseState{}, - }, - // Bare signs with no mode letters reject. - { - name: "bare + rejects", - modeStr: "+", - wantErr: true, - wantErrCode: irc.ErrUmodeUnknownFlag, - wantState: caseState{}, - }, - { - name: "bare - rejects", - modeStr: "-", - wantErr: true, - wantErrCode: irc.ErrUmodeUnknownFlag, - wantState: caseState{}, - }, - { - name: "+-+ rejects (no mode letters)", - modeStr: "+-+", - wantErr: true, - wantErrCode: irc.ErrUmodeUnknownFlag, - wantState: caseState{}, - }, - // Unknown mode letters reject whole string. - { - name: "+z unknown mode rejects", - modeStr: "+z", - wantErr: true, - wantErrCode: irc.ErrUmodeUnknownFlag, - wantState: caseState{}, - }, - { - name: "+wz rejects whole thing; +w side effect" + - " must NOT persist", - modeStr: "+wz", - wantErr: true, - wantErrCode: irc.ErrUmodeUnknownFlag, - // Wallops must NOT be set. - wantState: caseState{}, - }, - { - name: "+wo rejects whole thing; +w side effect" + - " must NOT persist", - modeStr: "+wo", - wantErr: true, - wantErrCode: irc.ErrUmodeUnknownFlag, - wantState: caseState{}, - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - env := newTestEnv(t) - ctx := t.Context() - sid := createSession(ctx, t, env.db, "alice") - - if tc.initialState.oper { - if err := env.db.SetSessionOper( - ctx, sid, true, - ); err != nil { - t.Fatalf("init oper: %v", err) - } - } - - if tc.initialState.wallops { - if err := env.db.SetSessionWallops( - ctx, sid, true, - ); err != nil { - t.Fatalf("init wallops: %v", err) - } - } - - result, err := env.svc.ApplyUserMode( - ctx, sid, tc.modeStr, - ) - - if tc.wantErr { - var ircErr *service.IRCError - if !errors.As(err, &ircErr) { - t.Fatalf( - "expected IRCError, got %v", err, - ) - } - - if ircErr.Code != tc.wantErrCode { - t.Errorf( - "code: want %d got %d", - tc.wantErrCode, ircErr.Code, - ) - } - } else { - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - if result != tc.wantModes { - t.Errorf( - "modes: want %q got %q", - tc.wantModes, result, - ) - } - } - - // Whether or not the call errored, the persisted - // state must match wantState — this is the - // atomicity guarantee sneak demanded. - gotOper, err := env.db.IsSessionOper(ctx, sid) - if err != nil { - t.Fatalf("read oper: %v", err) - } - - gotWallops, err := env.db.IsSessionWallops( - ctx, sid, - ) - if err != nil { - t.Fatalf("read wallops: %v", err) - } - - if gotOper != tc.wantState.oper { - t.Errorf( - "oper: want %v got %v", - tc.wantState.oper, gotOper, - ) - } - - if gotWallops != tc.wantState.wallops { - t.Errorf( - "wallops: want %v got %v", - tc.wantState.wallops, gotWallops, - ) - } + for _, testCase := range applyUserModeCases() { + t.Run(testCase.name, func(t *testing.T) { + runApplyUserModeCase(t, testCase) }) } } + +// runApplyUserModeCase executes one applyUserModeCase: seed +// the session state, invoke ApplyUserMode, and verify both +// the returned value and the post-call persisted state. +func runApplyUserModeCase( + t *testing.T, testCase applyUserModeCase, +) { + t.Helper() + + env := newTestEnv(t) + ctx := t.Context() + sid := createSession(ctx, t, env.db, "alice") + + seedApplyUserModeState(ctx, t, env.db, sid, testCase.initialState) + + result, err := env.svc.ApplyUserMode( + ctx, sid, testCase.modeStr, + ) + + verifyApplyUserModeOutcome(t, testCase, result, err) + verifyApplyUserModeState(ctx, t, env.db, sid, testCase.wantState) +} + +// seedApplyUserModeState installs the pre-call session +// state described by initialState. +func seedApplyUserModeState( + ctx context.Context, + t *testing.T, + database *db.Database, + sid int64, + initialState applyUserModeCaseState, +) { + t.Helper() + + if initialState.oper { + if err := database.SetSessionOper( + ctx, sid, true, + ); err != nil { + t.Fatalf("init oper: %v", err) + } + } + + if initialState.wallops { + if err := database.SetSessionWallops( + ctx, sid, true, + ); err != nil { + t.Fatalf("init wallops: %v", err) + } + } +} + +// verifyApplyUserModeOutcome asserts the direct return +// value of ApplyUserMode. It dispatches to the error- or +// success-specific verifier based on wantErr. +func verifyApplyUserModeOutcome( + t *testing.T, + testCase applyUserModeCase, + result string, + err error, +) { + t.Helper() + + if testCase.wantErr { + verifyApplyUserModeError(t, testCase, err) + + return + } + + verifyApplyUserModeSuccess(t, testCase, result, err) +} + +// verifyApplyUserModeError checks that err is a +// *service.IRCError whose code matches wantErrCode. +func verifyApplyUserModeError( + t *testing.T, testCase applyUserModeCase, err error, +) { + t.Helper() + + var ircErr *service.IRCError + if !errors.As(err, &ircErr) { + t.Fatalf("expected IRCError, got %v", err) + } + + if ircErr.Code != testCase.wantErrCode { + t.Errorf( + "code: want %d got %d", + testCase.wantErrCode, ircErr.Code, + ) + } +} + +// verifyApplyUserModeSuccess checks that err is nil and the +// returned mode string matches wantModes. +func verifyApplyUserModeSuccess( + t *testing.T, + testCase applyUserModeCase, + result string, + err error, +) { + t.Helper() + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if result != testCase.wantModes { + t.Errorf( + "modes: want %q got %q", + testCase.wantModes, result, + ) + } +} + +// verifyApplyUserModeState asserts the post-call persisted +// session state. This is the atomicity guarantee sneak +// demanded: whether the call succeeded or was rejected, the +// DB must match wantState exactly. +func verifyApplyUserModeState( + ctx context.Context, + t *testing.T, + database *db.Database, + sid int64, + wantState applyUserModeCaseState, +) { + t.Helper() + + gotOper, err := database.IsSessionOper(ctx, sid) + if err != nil { + t.Fatalf("read oper: %v", err) + } + + gotWallops, err := database.IsSessionWallops(ctx, sid) + if err != nil { + t.Fatalf("read wallops: %v", err) + } + + if gotOper != wantState.oper { + t.Errorf( + "oper: want %v got %v", + wantState.oper, gotOper, + ) + } + + if gotWallops != wantState.wallops { + t.Errorf( + "wallops: want %v got %v", + wantState.wallops, gotWallops, + ) + } +}