From 85097043cfc51b71ff9ed555ad0ccbc68d44f95f Mon Sep 17 00:00:00 2001 From: Alex Bramley Date: Sun, 6 Nov 2011 04:56:46 +0000 Subject: [PATCH] Epic final commit for nick/chan/logging/testing refactor. * Brings logging changes to client library. * Brings state tracker to client library. * Rewrites all tests to use mock logger and mock state tracker. * Makes state tracking optional, finally. * Shaves yaks until they are almost completely bald. --- client/commands.go | 2 +- client/commands_test.go | 54 ++-- client/connection.go | 91 +++--- client/connection_test.go | 99 +++--- client/handlers.go | 237 +++++++-------- client/handlers_test.go | 614 +++++++++++++++++--------------------- client/line.go | 5 +- 7 files changed, 525 insertions(+), 577 deletions(-) diff --git a/client/commands.go b/client/commands.go index ea17cca..870b6a1 100644 --- a/client/commands.go +++ b/client/commands.go @@ -9,7 +9,7 @@ import "strings" // the symbol table and add methods/functions on the fly // [ CMD, FMT, FMTARGS ] etc. -// Raw() sends a raw line to the server, should really only be used for +// Raw() sends a raw line to the server, should really only be used for // debugging purposes but may well come in handy. func (conn *Conn) Raw(rawline string) { conn.out <- rawline } diff --git a/client/commands_test.go b/client/commands_test.go index 2e7cd1b..c3bb6ee 100644 --- a/client/commands_test.go +++ b/client/commands_test.go @@ -3,76 +3,76 @@ package client import "testing" func TestClientCommands(t *testing.T) { - m, c := setUp(t) - defer tearDown(m, c) + c, s := setUp(t) + defer s.tearDown() c.Pass("password") - m.Expect("PASS password") + s.nc.Expect("PASS password") c.Nick("test") - m.Expect("NICK test") + s.nc.Expect("NICK test") c.User("test", "Testing IRC") - m.Expect("USER test 12 * :Testing IRC") + s.nc.Expect("USER test 12 * :Testing IRC") c.Raw("JUST a raw :line") - m.Expect("JUST a raw :line") + s.nc.Expect("JUST a raw :line") c.Join("#foo") - m.Expect("JOIN #foo") + s.nc.Expect("JOIN #foo") c.Part("#foo") - m.Expect("PART #foo") + s.nc.Expect("PART #foo") c.Part("#foo", "Screw you guys...") - m.Expect("PART #foo :Screw you guys...") + s.nc.Expect("PART #foo :Screw you guys...") c.Quit() - m.Expect("QUIT :GoBye!") + s.nc.Expect("QUIT :GoBye!") c.Quit("I'm going home.") - m.Expect("QUIT :I'm going home.") + s.nc.Expect("QUIT :I'm going home.") c.Whois("somebody") - m.Expect("WHOIS somebody") + s.nc.Expect("WHOIS somebody") c.Who("*@some.host.com") - m.Expect("WHO *@some.host.com") + s.nc.Expect("WHO *@some.host.com") c.Privmsg("#foo", "bar") - m.Expect("PRIVMSG #foo :bar") + s.nc.Expect("PRIVMSG #foo :bar") c.Notice("somebody", "something") - m.Expect("NOTICE somebody :something") + s.nc.Expect("NOTICE somebody :something") c.Ctcp("somebody", "ping", "123456789") - m.Expect("PRIVMSG somebody :\001PING 123456789\001") + s.nc.Expect("PRIVMSG somebody :\001PING 123456789\001") c.CtcpReply("somebody", "pong", "123456789") - m.Expect("NOTICE somebody :\001PONG 123456789\001") + s.nc.Expect("NOTICE somebody :\001PONG 123456789\001") c.Version("somebody") - m.Expect("PRIVMSG somebody :\001VERSION\001") + s.nc.Expect("PRIVMSG somebody :\001VERSION\001") c.Action("#foo", "pokes somebody") - m.Expect("PRIVMSG #foo :\001ACTION pokes somebody\001") + s.nc.Expect("PRIVMSG #foo :\001ACTION pokes somebody\001") c.Topic("#foo") - m.Expect("TOPIC #foo") + s.nc.Expect("TOPIC #foo") c.Topic("#foo", "la la la") - m.Expect("TOPIC #foo :la la la") + s.nc.Expect("TOPIC #foo :la la la") c.Mode("#foo") - m.Expect("MODE #foo") + s.nc.Expect("MODE #foo") c.Mode("#foo", "+o somebody") - m.Expect("MODE #foo +o somebody") + s.nc.Expect("MODE #foo +o somebody") c.Away() - m.Expect("AWAY") + s.nc.Expect("AWAY") c.Away("Dave's not here, man.") - m.Expect("AWAY :Dave's not here, man.") + s.nc.Expect("AWAY :Dave's not here, man.") c.Invite("somebody", "#foo") - m.Expect("INVITE somebody #foo") + s.nc.Expect("INVITE somebody #foo") c.Oper("user", "pass") - m.Expect("OPER user pass") + s.nc.Expect("OPER user pass") } diff --git a/client/connection.go b/client/connection.go index 22ad09a..ef0a8e7 100644 --- a/client/connection.go +++ b/client/connection.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "github.com/fluffle/goirc/event" "github.com/fluffle/goirc/logging" + "github.com/fluffle/goirc/state" "fmt" "net" "os" @@ -20,15 +21,19 @@ const ( type Conn struct { // Connection Hostname and Nickname Host string - Me *Nick + Me *state.Nick Network string // Event handler registry and dispatcher - Registry event.EventRegistry - Dispatcher event.EventDispatcher + ER event.EventRegistry + ED event.EventDispatcher // State tracker for nicks and channels - Tracker StateTracker + ST state.StateTracker + st bool + + // Logger for debugging/warning/etc output + l logging.Logger // Use the State field to store external state that handlers might need. // Remember ... you might need locking for this ;-) @@ -61,11 +66,20 @@ type Conn struct { // Creates a new IRC connection object, but doesn't connect to anything so // that you can add event handlers to it. See AddHandler() for details. -func New(nick, user, name string) *Conn { - reg := event.NewRegistry() +// Dependency injection is a bitch :-/ +func New(nick, user, name string, st bool, + r event.EventRegistry, l logging.Logger) *Conn { + if r == nil { + r = event.NewRegistry() + } + if l == nil { + l = logging.NewFromFlags() + } conn := &Conn{ - Registry: reg, - Dispatcher: reg, + ER: r, + ED: r, + l: l, + st: st, in: make(chan *Line, 32), out: make(chan string, 32), cSend: make(chan bool), @@ -77,23 +91,27 @@ func New(nick, user, name string) *Conn { badness: 0, lastsent: 0, } + conn.addIntHandlers() + if st { + conn.ST = state.NewTracker(nick, l) + conn.Me = conn.ST.Me() + conn.addSTHandlers() + } else { + conn.Me = state.NewNick(nick, l) + } + conn.Me.Ident = user + conn.Me.Name = name + conn.initialise() - conn.SetupHandlers() - conn.Me = conn.NewNick(nick, user, name, "") return conn } // Per-connection state initialisation. func (conn *Conn) initialise() { - conn.nicks = make(map[string]*Nick) - conn.chans = make(map[string]*Channel) conn.io = nil conn.sock = nil - - // If this is being called because we are reconnecting, conn.Me - // will still have all the old channels referenced -- nuke them! - if conn.Me != nil { - conn.Me = conn.NewNick(conn.Me.Nick, conn.Me.Ident, conn.Me.Name, "") + if conn.st { + conn.ST.Wipe() } } @@ -113,7 +131,7 @@ func (conn *Conn) Connect(host string, pass ...string) os.Error { if !hasPort(host) { host += ":6697" } - logging.Info("irc.Connect(): Connecting to %s with SSL.", host) + conn.l.Info("irc.Connect(): Connecting to %s with SSL.", host) if s, err := tls.Dial("tcp", host, conn.SSLConfig); err == nil { conn.sock = s } else { @@ -123,7 +141,7 @@ func (conn *Conn) Connect(host string, pass ...string) os.Error { if !hasPort(host) { host += ":6667" } - logging.Info("irc.Connect(): Connecting to %s without SSL.", host) + conn.l.Info("irc.Connect(): Connecting to %s without SSL.", host) if s, err := net.Dial("tcp", host); err == nil { conn.sock = s } else { @@ -176,16 +194,18 @@ func (conn *Conn) recv() { for { s, err := conn.io.ReadString('\n') if err != nil { - logging.Error("irc.recv(): %s", err.String()) + conn.l.Error("irc.recv(): %s", err.String()) conn.shutdown() return } s = strings.Trim(s, "\r\n") - logging.Debug("<- %s", s) + conn.l.Debug("<- %s", s) if line := parseLine(s); line != nil { line.Time = time.LocalTime() conn.in <- line + } else { + conn.l.Warn("irc.recv(): problems parsing line:\n %s", s) } } } @@ -195,7 +215,7 @@ func (conn *Conn) runLoop() { for { select { case line := <-conn.in: - conn.Dispatcher.Dispatch(line.Cmd, conn, line) + conn.ED.Dispatch(line.Cmd, conn, line) case <-conn.cLoop: // strobe on control channel, bail out return @@ -211,12 +231,12 @@ func (conn *Conn) write(line string) { } if _, err := conn.io.WriteString(line + "\r\n"); err != nil { - logging.Error("irc.send(): %s", err.String()) + conn.l.Error("irc.send(): %s", err.String()) conn.shutdown() return } conn.io.Flush() - logging.Debug("-> %s", line) + conn.l.Debug("-> %s", line) } // Implement Hybrid's flood control algorithm to rate-limit outgoing lines. @@ -234,9 +254,9 @@ func (conn *Conn) rateLimit(chars int64) { // calculation above, then we're at risk of "Excess Flood". if conn.badness > 10*second && !conn.Flood { // so sleep for the current line's time value before sending it - logging.Debug("irc.rateLimit(): Flood! Sleeping for %.2f secs.", + conn.l.Debug("irc.rateLimit(): Flood! Sleeping for %.2f secs.", float64(linetime)/float64(second)) - time.Sleep(linetime) + <-time.After(linetime) } } @@ -244,8 +264,8 @@ func (conn *Conn) shutdown() { // Guard against double-call of shutdown() if we get an error in send() // as calling sock.Close() will cause recv() to recieve EOF in readstring() if conn.Connected { - logging.Info("irc.shutdown(): Disconnected from server.") - conn.Dispatcher.Dispatch("disconnected", conn, &Line{}) + conn.l.Info("irc.shutdown(): Disconnected from server.") + conn.ED.Dispatch("disconnected", conn, &Line{}) conn.Connected = false conn.sock.Close() conn.cSend <- true @@ -257,7 +277,7 @@ func (conn *Conn) shutdown() { } // Dumps a load of information about the current state of the connection to a -// string for debugging state tracking and other such things. +// string for debugging state tracking and other such things. func (conn *Conn) String() string { str := "GoIRC Connection\n" str += "----------------\n\n" @@ -267,17 +287,8 @@ func (conn *Conn) String() string { str += "Not currently connected!\n\n" } str += conn.Me.String() + "\n" - str += "GoIRC Channels\n" - str += "--------------\n\n" - for _, ch := range conn.chans { - str += ch.String() + "\n" - } - str += "GoIRC NickNames\n" - str += "---------------\n\n" - for _, n := range conn.nicks { - if n != conn.Me { - str += n.String() + "\n" - } + if conn.st { + str += conn.ST.String() + "\n" } return str } diff --git a/client/connection_test.go b/client/connection_test.go index c4b5b9e..2dc09fd 100644 --- a/client/connection_test.go +++ b/client/connection_test.go @@ -2,20 +2,38 @@ package client import ( "github.com/fluffle/goirc/logging" + "github.com/fluffle/goirc/state" + "gomock.googlecode.com/hg/gomock" "strings" "testing" "time" ) -func init() { - // We have Error level logging that is printed on socket shutdown - // which we don't care too much about when testing with a mock conn... - logging.SetLogLevel(logging.LogFatal) +type testState struct { + ctrl *gomock.Controller + log *logging.MockLogger + st *state.MockStateTracker + nc *mockNetConn + c *Conn } -func setUp(t *testing.T) (*mockNetConn, *Conn) { - c := New("test", "test", "Testing IRC") - c.State = t +func setUp(t *testing.T) (*Conn, *testState) { + ctrl := gomock.NewController(t) + st := state.NewMockStateTracker(ctrl) + l := logging.NewMockLogger(ctrl) + nc := MockNetConn(t) + c := New("test", "test", "Testing IRC", false, nil, l) + + // We don't want to have to specify s.log.EXPECT().Debug() for all the + // random crap that gets logged. This mocks it all out nicely. + ctrl.RecordCall(l, "Debug", gomock.Any(), gomock.Any()).AnyTimes() + + c.ST = st + c.st = true + c.sock = nc + c.Flood = true // Tests can take a while otherwise + c.Connected = true + c.postConnect() // Assert some basic things about the initial state of the Conn struct if c.Me.Nick != "test" || @@ -24,40 +42,31 @@ func setUp(t *testing.T) (*mockNetConn, *Conn) { c.Me.Host != "" { t.Errorf("Conn.Me not correctly initialised.") } - if len(c.chans) != 0 { - t.Errorf("Some channels are already known:") - for _, ch := range c.chans { - t.Logf(ch.String()) - } - } - if len(c.nicks) != 1 { - t.Errorf("Other Nicks than ourselves exist:") - for _, n := range c.nicks { - t.Logf(n.String()) - } - } - m := MockNetConn(t) - c.sock = m - c.postConnect() - c.Flood = true // Tests can take a while otherwise - c.Connected = true - return m, c + return c, &testState{ctrl, l, st, nc, c} } -func tearDown(m *mockNetConn, c *Conn) { - c.shutdown() +func (s *testState) tearDown() { + // This can get set to false in some tests + s.c.st = true + s.st.EXPECT().Wipe() + s.log.EXPECT().Error("irc.recv(): %s", "EOF") + s.log.EXPECT().Info("irc.shutdown(): Disconnected from server.") + s.nc.ExpectNothing() + s.c.shutdown() + <-time.After(1e6) + s.ctrl.Finish() } + func TestShutdown(t *testing.T) { - _, c := setUp(t) + c, s := setUp(t) // Setup a mock event dispatcher to test correct triggering of "disconnected" - flag := false - c.Dispatcher = WasEventDispatched("disconnected", &flag) + flag := c.ExpectEvent("disconnected") - // Call shutdown manually - c.shutdown() + // Call shutdown via tearDown + s.tearDown() // Verify that the connection no longer thinks it's connected if c.Connected { @@ -65,7 +74,7 @@ func TestShutdown(t *testing.T) { } // Verify that the "disconnected" event fired correctly - if !flag { + if !*flag { t.Errorf("Calling Close() didn't result in dispatch of disconnected event.") } @@ -76,18 +85,22 @@ func TestShutdown(t *testing.T) { // Practically the same as the above test, but shutdown is called implicitly // by recv() getting an EOF from the mock connection. func TestEOF(t *testing.T) { - m, c := setUp(t) + c, s := setUp(t) + // Since we're not using tearDown() here, manually call Finish() + defer s.ctrl.Finish() // Setup a mock event dispatcher to test correct triggering of "disconnected" - flag := false - c.Dispatcher = WasEventDispatched("disconnected", &flag) + flag := c.ExpectEvent("disconnected") // Simulate EOF from server - m.Close() + s.st.EXPECT().Wipe() + s.log.EXPECT().Info("irc.shutdown(): Disconnected from server.") + s.log.EXPECT().Error("irc.recv(): %s", "EOF") + s.nc.Close() // Since things happen in different internal goroutines, we need to wait // 1 ms should be enough :-) - time.Sleep(1e6) + <-time.After(1e6) // Verify that the connection no longer thinks it's connected if c.Connected { @@ -95,7 +108,7 @@ func TestEOF(t *testing.T) { } // Verify that the "disconnected" event fired correctly - if !flag { + if !*flag { t.Errorf("Calling Close() didn't result in dispatch of disconnected event.") } } @@ -107,10 +120,12 @@ func (d mockDispatcher) Dispatch(name string, ev ...interface{}) { d(name, ev...) } -func WasEventDispatched(name string, flag *bool) mockDispatcher { - return mockDispatcher(func(n string, ev ...interface{}) { +func (conn *Conn) ExpectEvent(name string) *bool { + flag := false + conn.ED = mockDispatcher(func(n string, ev ...interface{}) { if n == strings.ToLower(name) { - *flag = true + flag = true } }) + return &flag } diff --git a/client/handlers.go b/client/handlers.go index 0923f4d..28ea2ad 100644 --- a/client/handlers.go +++ b/client/handlers.go @@ -5,7 +5,6 @@ package client import ( "github.com/fluffle/goirc/event" - "github.com/fluffle/goirc/logging" "strings" ) @@ -18,10 +17,10 @@ type IRCHandler func(*Conn, *Line) // "name" being equivalent to Line.Cmd. Read the RFCs for details on what // replies could come from the server. They'll generally be things like // "PRIVMSG", "JOIN", etc. but all the numeric replies are left as ascii -// strings of digits like "332" (mainly because I really didn't feel like +// strings of digits like "332" (mainly because I really didn't feel like // putting massive constant tables in). func (conn *Conn) AddHandler(name string, f IRCHandler) { - conn.Registry.AddHandler(name, NewHandler(f)) + conn.ER.AddHandler(name, NewHandler(f)) } // Wrap f in an anonymous unboxing function @@ -31,6 +30,15 @@ func NewHandler(f IRCHandler) event.Handler { }) } +// sets up the internal event handlers to do essential IRC protocol things +func (conn *Conn) addIntHandlers() { + conn.AddHandler("001", (*Conn).h_001) + conn.AddHandler("433", (*Conn).h_433) + conn.AddHandler("CTCP", (*Conn).h_CTCP) + conn.AddHandler("NICK", (*Conn).h_NICK) + conn.AddHandler("PING", (*Conn).h_PING) +} + // Basic ping/pong handler func (conn *Conn) h_PING(line *Line) { conn.Raw("PONG :" + line.Args[0]) @@ -39,7 +47,7 @@ func (conn *Conn) h_PING(line *Line) { // Handler to trigger a "CONNECTED" event on receipt of numeric 001 func (conn *Conn) h_001(line *Line) { // we're connected! - conn.Dispatcher.Dispatch("connected", conn, line) + conn.ED.Dispatch("connected", conn, line) // and we're being given our hostname (from the server's perspective) t := line.Args[len(line.Args)-1] if idx := strings.LastIndex(t, " "); idx != -1 { @@ -61,22 +69,17 @@ func (conn *Conn) h_001(line *Line) { // Handler to deal with "433 :Nickname already in use" func (conn *Conn) h_433(line *Line) { // Args[1] is the new nick we were attempting to acquire - conn.Nick(line.Args[1] + "_") + neu := line.Args[1] + "_" + conn.Nick(neu) // if this is happening before we're properly connected (i.e. the nick // we sent in the initial NICK command is in use) we will not receive // a NICK message to confirm our change of nick, so ReNick here... if line.Args[1] == conn.Me.Nick { - conn.Me.ReNick(line.Args[1] + "_") - } -} - -// Handler NICK messages to inform us about nick changes -func (conn *Conn) h_NICK(line *Line) { - // all nicks should be handled the same way, our own included - if n := conn.GetNick(line.Nick); n != nil { - n.ReNick(line.Args[0]) - } else { - logging.Warn("irc.NICK(): unknown nick %s.", line.Nick) + if conn.st { + conn.ST.ReNick(conn.Me.Nick, neu) + } else { + conn.Me.Nick = neu + } } } @@ -89,19 +92,53 @@ func (conn *Conn) h_CTCP(line *Line) { } } +// Handle updating our own NICK if we're not using the state tracker +func (conn *Conn) h_NICK(line *Line) { + if !conn.st && line.Nick == conn.Me.Nick { + conn.Me.Nick = line.Args[0] + } +} + +/******************************************************************************\ + * State tracking handlers below here +\******************************************************************************/ + +func (conn *Conn) addSTHandlers() { + conn.AddHandler("JOIN", (*Conn).h_JOIN) + conn.AddHandler("KICK", (*Conn).h_KICK) + conn.AddHandler("MODE", (*Conn).h_MODE) + conn.AddHandler("NICK", (*Conn).h_STNICK) + conn.AddHandler("PART", (*Conn).h_PART) + conn.AddHandler("QUIT", (*Conn).h_QUIT) + conn.AddHandler("TOPIC", (*Conn).h_TOPIC) + + conn.AddHandler("311", (*Conn).h_311) + conn.AddHandler("324", (*Conn).h_324) + conn.AddHandler("332", (*Conn).h_332) + conn.AddHandler("352", (*Conn).h_352) + conn.AddHandler("353", (*Conn).h_353) + conn.AddHandler("671", (*Conn).h_671) +} + +// Handle NICK messages that need to update the state tracker +func (conn *Conn) h_STNICK(line *Line) { + // all nicks should be handled the same way, our own included + conn.ST.ReNick(line.Nick, line.Args[0]) +} + // Handle JOINs to channels to maintain state func (conn *Conn) h_JOIN(line *Line) { - ch := conn.GetChannel(line.Args[0]) - n := conn.GetNick(line.Nick) + ch := conn.ST.GetChannel(line.Args[0]) + nk := conn.ST.GetNick(line.Nick) if ch == nil { // first we've seen of this channel, so should be us joining it - // NOTE this will also take care of n == nil && ch == nil - if n != conn.Me { - logging.Warn("irc.JOIN(): JOIN to unknown channel %s recieved "+ + // NOTE this will also take care of nk == nil && ch == nil + if nk != conn.Me { + conn.l.Warn("irc.JOIN(): JOIN to unknown channel %s received "+ "from (non-me) nick %s", line.Args[0], line.Nick) return } - ch = conn.NewChannel(line.Args[0]) + ch = conn.ST.NewChannel(line.Args[0]) // since we don't know much about this channel, ask server for info // we get the channel users automatically in 353 and the channel // topic in 332 on join, so we just need to get the modes @@ -110,147 +147,123 @@ func (conn *Conn) h_JOIN(line *Line) { // triggering a WHOIS on every nick from the 353 handler conn.Who(ch.Name) } - if n == nil { + if nk == nil { // this is the first we've seen of this nick - n = conn.NewNick(line.Nick, line.Ident, "", line.Host) + nk = conn.ST.NewNick(line.Nick) + nk.Ident = line.Ident + nk.Host = line.Host // since we don't know much about this nick, ask server for info - conn.Who(n.Nick) + conn.Who(nk.Nick) } // this takes care of both nick and channel linking \o/ - ch.AddNick(n) + conn.ST.Associate(ch, nk) } // Handle PARTs from channels to maintain state func (conn *Conn) h_PART(line *Line) { - ch := conn.GetChannel(line.Args[0]) - n := conn.GetNick(line.Nick) - if ch != nil && n != nil { - if _, ok := ch.Nicks[n]; ok { - ch.DelNick(n) - } else { - logging.Warn("irc.PART(): nick %s is not on channel %s", - line.Nick, line.Args[0]) - } - } else { - logging.Warn("irc.PART(): PART of channel %s by nick %s", - line.Args[0], line.Nick) - } + conn.ST.Dissociate(conn.ST.GetChannel(line.Args[0]), + conn.ST.GetNick(line.Nick)) } // Handle KICKs from channels to maintain state func (conn *Conn) h_KICK(line *Line) { // XXX: this won't handle autorejoining channels on KICK // it's trivial to do this in a seperate handler... - ch := conn.GetChannel(line.Args[0]) - n := conn.GetNick(line.Args[1]) - if ch != nil && n != nil { - if _, ok := ch.Nicks[n]; ok { - ch.DelNick(n) - } else { - logging.Warn("irc.KICK(): nick %s is not on channel %s", - line.Nick, line.Args[0]) - } - } else { - logging.Warn("irc.KICK(): KICK from channel %s of nick %s", - line.Args[0], line.Args[1]) - } + conn.ST.Dissociate(conn.ST.GetChannel(line.Args[0]), + conn.ST.GetNick(line.Args[1])) } // Handle other people's QUITs func (conn *Conn) h_QUIT(line *Line) { - if n := conn.GetNick(line.Nick); n != nil { - n.Delete() - } else { - logging.Warn("irc.QUIT(): QUIT from unknown nick %s", line.Nick) - } + conn.ST.DelNick(line.Nick) } // Handle MODE changes for channels we know about (and our nick personally) func (conn *Conn) h_MODE(line *Line) { - // channel modes first - if ch := conn.GetChannel(line.Args[0]); ch != nil { - conn.ParseChannelModes(ch, line.Args[1], line.Args[2:]) - } else if n := conn.GetNick(line.Args[0]); n != nil { + if ch := conn.ST.GetChannel(line.Args[0]); ch != nil { + // channel modes first + ch.ParseModes(line.Args[1], line.Args[2:]...) + } else if nk := conn.ST.GetNick(line.Args[0]); nk != nil { // nick mode change, should be us - if n != conn.Me { - logging.Warn("irc.MODE(): recieved MODE %s for (non-me) nick %s", - line.Args[0], n.Nick) + if nk != conn.Me { + conn.l.Warn("irc.MODE(): recieved MODE %s for (non-me) nick %s", + line.Args[1], line.Args[0]) return } - conn.ParseNickModes(n, line.Args[1]) + nk.ParseModes(line.Args[1]) } else { - logging.Warn("irc.MODE(): not sure what to do with MODE %s", + conn.l.Warn("irc.MODE(): not sure what to do with MODE %s", strings.Join(line.Args, " ")) } } // Handle TOPIC changes for channels func (conn *Conn) h_TOPIC(line *Line) { - if ch := conn.GetChannel(line.Args[0]); ch != nil { + if ch := conn.ST.GetChannel(line.Args[0]); ch != nil { ch.Topic = line.Args[1] } else { - logging.Warn("irc.TOPIC(): topic change on unknown channel %s", + conn.l.Warn("irc.TOPIC(): topic change on unknown channel %s", line.Args[0]) } } // Handle 311 whois reply func (conn *Conn) h_311(line *Line) { - if n := conn.GetNick(line.Args[1]); n != nil { - n.Ident = line.Args[2] - n.Host = line.Args[3] - n.Name = line.Args[5] + if nk := conn.ST.GetNick(line.Args[1]); nk != nil { + nk.Ident = line.Args[2] + nk.Host = line.Args[3] + nk.Name = line.Args[5] } else { - logging.Warn("irc.311(): received WHOIS info for unknown nick %s", + conn.l.Warn("irc.311(): received WHOIS info for unknown nick %s", line.Args[1]) } } // Handle 324 mode reply func (conn *Conn) h_324(line *Line) { - if ch := conn.GetChannel(line.Args[1]); ch != nil { - conn.ParseChannelModes(ch, line.Args[2], line.Args[3:]) + if ch := conn.ST.GetChannel(line.Args[1]); ch != nil { + ch.ParseModes(line.Args[2], line.Args[3:]...) } else { - logging.Warn("irc.324(): received MODE settings for unknown channel %s", + conn.l.Warn("irc.324(): received MODE settings for unknown channel %s", line.Args[1]) } } // Handle 332 topic reply on join to channel func (conn *Conn) h_332(line *Line) { - if ch := conn.GetChannel(line.Args[1]); ch != nil { + if ch := conn.ST.GetChannel(line.Args[1]); ch != nil { ch.Topic = line.Args[2] } else { - logging.Warn("irc.332(): received TOPIC value for unknown channel %s", + conn.l.Warn("irc.332(): received TOPIC value for unknown channel %s", line.Args[1]) } } // Handle 352 who reply func (conn *Conn) h_352(line *Line) { - if n := conn.GetNick(line.Args[5]); n != nil { - n.Ident = line.Args[2] - n.Host = line.Args[3] + if nk := conn.ST.GetNick(line.Args[5]); nk != nil { + nk.Ident = line.Args[2] + nk.Host = line.Args[3] // XXX: do we care about the actual server the nick is on? // or the hop count to this server? // last arg contains " " a := strings.SplitN(line.Args[len(line.Args)-1], " ", 2) - n.Name = a[1] + nk.Name = a[1] if idx := strings.Index(line.Args[6], "*"); idx != -1 { - n.Modes.Oper = true + nk.Modes.Oper = true } if idx := strings.Index(line.Args[6], "H"); idx != -1 { - n.Modes.Invisible = true + nk.Modes.Invisible = true } } else { - logging.Warn("irc.352(): received WHO reply for unknown nick %s", + conn.l.Warn("irc.352(): received WHO reply for unknown nick %s", line.Args[5]) } } // Handle 353 names reply func (conn *Conn) h_353(line *Line) { - if ch := conn.GetChannel(line.Args[2]); ch != nil { + if ch := conn.ST.GetChannel(line.Args[2]); ch != nil { nicks := strings.Split(line.Args[len(line.Args)-1], " ") for _, nick := range nicks { // UnrealIRCd's coders are lazy and leave a trailing space @@ -262,64 +275,42 @@ func (conn *Conn) h_353(line *Line) { nick = nick[1:] fallthrough default: - n := conn.GetNick(nick) - if n == nil { + nk := conn.ST.GetNick(nick) + if nk == nil { // we don't know this nick yet! - n = conn.NewNick(nick, "", "", "") + nk = conn.ST.NewNick(nick) } - if _, ok := ch.Nicks[n]; !ok { + cp, ok := conn.ST.IsOn(ch.Name, nick) + if !ok { // This nick isn't associated with this channel yet! - ch.AddNick(n) + cp = conn.ST.Associate(ch, nk) } - p := ch.Nicks[n] switch c { case '~': - p.Owner = true + cp.Owner = true case '&': - p.Admin = true + cp.Admin = true case '@': - p.Op = true + cp.Op = true case '%': - p.HalfOp = true + cp.HalfOp = true case '+': - p.Voice = true + cp.Voice = true } } } } else { - logging.Warn("irc.353(): received NAMES list for unknown channel %s", + conn.l.Warn("irc.353(): received NAMES list for unknown channel %s", line.Args[2]) } } // Handle 671 whois reply (nick connected via SSL) func (conn *Conn) h_671(line *Line) { - if n := conn.GetNick(line.Args[1]); n != nil { - n.Modes.SSL = true + if nk := conn.ST.GetNick(line.Args[1]); nk != nil { + nk.Modes.SSL = true } else { - logging.Warn("irc.671(): received WHOIS SSL info for unknown nick %s", + conn.l.Warn("irc.671(): received WHOIS SSL info for unknown nick %s", line.Args[1]) } } - -// sets up the internal event handlers to do useful things with lines -func (conn *Conn) SetupHandlers() { - conn.AddHandler("CTCP", (*Conn).h_CTCP) - conn.AddHandler("JOIN", (*Conn).h_JOIN) - conn.AddHandler("KICK", (*Conn).h_KICK) - conn.AddHandler("MODE", (*Conn).h_MODE) - conn.AddHandler("NICK", (*Conn).h_NICK) - conn.AddHandler("PART", (*Conn).h_PART) - conn.AddHandler("PING", (*Conn).h_PING) - conn.AddHandler("QUIT", (*Conn).h_QUIT) - conn.AddHandler("TOPIC", (*Conn).h_TOPIC) - - conn.AddHandler("001", (*Conn).h_001) - conn.AddHandler("311", (*Conn).h_311) - conn.AddHandler("324", (*Conn).h_324) - conn.AddHandler("332", (*Conn).h_332) - conn.AddHandler("352", (*Conn).h_352) - conn.AddHandler("353", (*Conn).h_353) - conn.AddHandler("433", (*Conn).h_433) - conn.AddHandler("671", (*Conn).h_671) -} diff --git a/client/handlers_test.go b/client/handlers_test.go index 7e806ae..b528d0f 100644 --- a/client/handlers_test.go +++ b/client/handlers_test.go @@ -1,6 +1,8 @@ package client import ( + "github.com/fluffle/goirc/state" + "gomock.googlecode.com/hg/gomock" "testing" ) @@ -9,28 +11,25 @@ import ( // in this file will call their respective handlers synchronously, otherwise // testing becomes more difficult. func TestPING(t *testing.T) { - m, c := setUp(t) - defer tearDown(m, c) - m.Send("PING :1234567890") - m.Expect("PONG :1234567890") + _, s := setUp(t) + defer s.tearDown() + s.nc.Send("PING :1234567890") + s.nc.Expect("PONG :1234567890") } // Test the handler for 001 / RPL_WELCOME func Test001(t *testing.T) { - m, c := setUp(t) - defer tearDown(m, c) + c, s := setUp(t) + defer s.tearDown() // Setup a mock event dispatcher to test correct triggering of "connected" - flag := false - c.Dispatcher = WasEventDispatched("connected", &flag) + flag := c.ExpectEvent("connected") // Call handler with a valid 001 line c.h_001(parseLine(":irc.server.org 001 test :Welcome to IRC test!ident@somehost.com")) - // Should result in no response to server - m.ExpectNothing() // Check that the event was dispatched correctly - if !flag { + if !*flag { t.Errorf("Sending 001 didn't result in dispatch of connected event.") } @@ -42,12 +41,12 @@ func Test001(t *testing.T) { // Test the handler for 433 / ERR_NICKNAMEINUSE func Test433(t *testing.T) { - m, c := setUp(t) - defer tearDown(m, c) + c, s := setUp(t) + defer s.tearDown() // Call handler with a 433 line, not triggering c.Me.Renick() c.h_433(parseLine(":irc.server.org 433 test new :Nickname is already in use.")) - m.Expect("NICK new_") + s.nc.Expect("NICK new_") // In this case, we're expecting the server to send a NICK line if c.Me.Nick != "test" { @@ -58,544 +57,479 @@ func Test433(t *testing.T) { // nick is unavailable during initial negotiation, so we must choose a // different one before the connection can proceed. No NICK line will be // sent by the server to confirm nick change in this case. + s.st.EXPECT().ReNick("test", "test_") c.h_433(parseLine(":irc.server.org 433 test test :Nickname is already in use.")) - m.Expect("NICK test_") + s.nc.Expect("NICK test_") + + // Counter-intuitively, c.Me.Nick will not change in this case. This is an + // artifact of the test set-up, with a mocked out state tracker that + // doesn't actually change any state. Normally, this would be fine :-) + if c.Me.Nick != "test" { + t.Errorf("My nick changed from '%s'.", c.Me.Nick) + } + + // Test the code path that *doesn't* involve state tracking. + c.st = false + c.h_433(parseLine(":irc.server.org 433 test test :Nickname is already in use.")) + s.nc.Expect("NICK test_") if c.Me.Nick != "test_" { - t.Errorf("ReNick() not called, Nick == '%s'.", c.Me.Nick) + t.Errorf("My nick not updated from '%s'.", c.Me.Nick) } } -// Test the handler for NICK messages +// Test the handler for NICK messages when state tracking is disabled func TestNICK(t *testing.T) { - m, c := setUp(t) - defer tearDown(m, c) + c, s := setUp(t) + defer s.tearDown() + + // State tracking is enabled by default in setUp + c.st = false // Call handler with a NICK line changing "our" nick to test1. c.h_NICK(parseLine(":test!test@somehost.com NICK :test1")) - // Should generate no response to server - m.ExpectNothing() // Verify that our Nick has changed if c.Me.Nick != "test1" { t.Errorf("NICK did not result in changing our nick.") } - // Create a "known" nick other than ours - user1 := c.NewNick("user1", "ident1", "name one", "host1.com") - - // Call handler with a NICK line changing user1 to somebody - c.h_NICK(parseLine(":user1!ident1@host1.com NICK :somebody")) - m.ExpectNothing() - - if c.GetNick("user1") != nil { - t.Errorf("Still have a valid Nick for 'user1'.") - } - if n := c.GetNick("somebody"); n != user1 { - t.Errorf("GetNick(somebody) didn't result in correct Nick.") - } - - // Send a NICK line for an unknown nick. + // Send a NICK line for something that isn't us. c.h_NICK(parseLine(":blah!moo@cows.com NICK :milk")) - m.ExpectNothing() + + // Verify that our Nick hasn't changed + if c.Me.Nick != "test1" { + t.Errorf("NICK did not result in changing our nick.") + } + + // Re-enable state tracking and send a line that *should* change nick. + c.st = true + c.h_NICK(parseLine(":test1!test@somehost.com NICK :test2")) + + // Verify that our Nick hasn't changed (should be handled by h_STNICK). + if c.Me.Nick != "test1" { + t.Errorf("NICK changed our nick when state tracking enabled.") + } } // Test the handler for CTCP messages func TestCTCP(t *testing.T) { - m, c := setUp(t) - defer tearDown(m, c) + c, s := setUp(t) + defer s.tearDown() // Call handler with CTCP VERSION c.h_CTCP(parseLine(":blah!moo@cows.com PRIVMSG test :\001VERSION\001")) // Expect a version reply - m.Expect("NOTICE blah :\001VERSION powered by goirc...\001") + s.nc.Expect("NOTICE blah :\001VERSION powered by goirc...\001") // Call handler with CTCP PING c.h_CTCP(parseLine(":blah!moo@cows.com PRIVMSG test :\001PING 1234567890\001")) // Expect a ping reply - m.Expect("NOTICE blah :\001PING 1234567890\001") + s.nc.Expect("NOTICE blah :\001PING 1234567890\001") // Call handler with CTCP UNKNOWN c.h_CTCP(parseLine(":blah!moo@cows.com PRIVMSG test :\001UNKNOWN ctcp\001")) - - // Expect nothing in reply - m.ExpectNothing() } // Test the handler for JOIN messages func TestJOIN(t *testing.T) { - // TODO(fluffle): Without mocking to ensure that the various methods - // h_JOIN uses are called, we must check they do the right thing by - // verifying their expected side-effects instead. Fixing this requires - // significant effort to move Conn to being a mockable interface type - // instead of a concrete struct. I'm not sure how feasible this is :-/ - // - // Soon, we'll find out :-) + c, s := setUp(t) + defer s.tearDown() - m, c := setUp(t) - defer tearDown(m, c) + // The state tracker should be creating a new channel in this first test + chan1 := state.NewChannel("#test1", s.log) + + gomock.InOrder( + s.st.EXPECT().GetChannel("#test1").Return(nil), + s.st.EXPECT().GetNick("test").Return(c.Me), + s.st.EXPECT().NewChannel("#test1").Return(chan1), + s.st.EXPECT().Associate(chan1, c.Me), + ) // Use #test1 to test expected behaviour // Call handler with JOIN by test to #test1 c.h_JOIN(parseLine(":test!test@somehost.com JOIN :#test1")) // Verify that the MODE and WHO commands are sent correctly - m.Expect("MODE #test1") - m.Expect("WHO #test1") + s.nc.Expect("MODE #test1") + s.nc.Expect("WHO #test1") - // Simple verification that NewChannel was called for #test1 - test1 := c.GetChannel("#test1") - if test1 == nil { - t.Errorf("No Channel for #test1 created on JOIN.") - } + // In this second test, we should be creating a new nick + nick1 := state.NewNick("user1", s.log) + + gomock.InOrder( + s.st.EXPECT().GetChannel("#test1").Return(chan1), + s.st.EXPECT().GetNick("user1").Return(nil), + s.st.EXPECT().NewNick("user1").Return(nick1), + s.st.EXPECT().Associate(chan1, nick1), + ) // OK, now #test1 exists, JOIN another user we don't know about c.h_JOIN(parseLine(":user1!ident1@host1.com JOIN :#test1")) // Verify that the WHO command is sent correctly - m.Expect("WHO user1") + s.nc.Expect("WHO user1") - // Simple verification that NewNick was called for user1 - user1 := c.GetNick("user1") - if user1 == nil { - t.Errorf("No Nick for user1 created on JOIN.") - } - - // Now, JOIN a nick we *do* know about. - user2 := c.NewNick("user2", "ident2", "name two", "host2.com") + // In this third test, we'll be pretending we know about the nick already. + nick2 := state.NewNick("user2", c.l) + gomock.InOrder( + s.st.EXPECT().GetChannel("#test1").Return(chan1), + s.st.EXPECT().GetNick("user2").Return(nick2), + s.st.EXPECT().Associate(chan1, nick2), + ) c.h_JOIN(parseLine(":user2!ident2@host2.com JOIN :#test1")) - // We already know about this user and channel, so nothing should be sent - m.ExpectNothing() - - // Simple verification that the state tracking has actually been done - if _, ok := test1.Nicks[user2]; !ok || len(test1.Nicks) != 3 { - t.Errorf("State tracking horked, hopefully other unit tests fail.") - } - - // Test error paths -- unknown channel, unknown nick + // Test error paths + gomock.InOrder( + // unknown channel, unknown nick + s.st.EXPECT().GetChannel("#test2").Return(nil), + s.st.EXPECT().GetNick("blah").Return(nil), + s.log.EXPECT().Warn("irc.JOIN(): JOIN to unknown channel %s "+ + "received from (non-me) nick %s", "#test2", "blah"), + // unknown channel, known nick that isn't Me. + s.st.EXPECT().GetChannel("#test2").Return(nil), + s.st.EXPECT().GetNick("user2").Return(nick2), + s.log.EXPECT().Warn("irc.JOIN(): JOIN to unknown channel %s "+ + "received from (non-me) nick %s", "#test2", "user2"), + ) c.h_JOIN(parseLine(":blah!moo@cows.com JOIN :#test2")) - m.ExpectNothing() - - // unknown channel, known nick that isn't Me. c.h_JOIN(parseLine(":user2!ident2@host2.com JOIN :#test2")) - m.ExpectNothing() } // Test the handler for PART messages func TestPART(t *testing.T) { - m, c := setUp(t) - defer tearDown(m, c) + c, s := setUp(t) + defer s.tearDown() - // Create user1 and add them to #test1 and #test2 - user1 := c.NewNick("user1", "ident1", "name one", "host1.com") - test1 := c.NewChannel("#test1") - test2 := c.NewChannel("#test2") - test1.AddNick(user1) - test2.AddNick(user1) + // We need some valid and associated nicks / channels to PART with. + chan1 := state.NewChannel("#test1", s.log) + nick1 := state.NewNick("user1", s.log) - // Add Me to both channels (not strictly necessary) - test1.AddNick(c.Me) - test2.AddNick(c.Me) - - // Then make them PART + // PART should dissociate a nick from a channel. + gomock.InOrder( + s.st.EXPECT().GetChannel("#test1").Return(chan1), + s.st.EXPECT().GetNick("user1").Return(nick1), + s.st.EXPECT().Dissociate(chan1, nick1), + ) c.h_PART(parseLine(":user1!ident1@host1.com PART #test1 :Bye!")) - - // Expect no output - m.ExpectNothing() - - // Quick check of tracking code - if len(test1.Nicks) != 1 { - t.Errorf("PART failed to remove user1 from #test1.") - } - - // Test error states. - // Part a known user from a known channel they are not on. - c.h_PART(parseLine(":user1!ident1@host1.com PART #test1 :Bye!")) - - // Part an unknown user from a known channel. - c.h_PART(parseLine(":user2!ident2@host2.com PART #test1 :Bye!")) - - // Part a known user from an unknown channel. - c.h_PART(parseLine(":user1!ident1@host1.com PART #test3 :Bye!")) - - // Part an unknown user from an unknown channel. - c.h_PART(parseLine(":user2!ident2@host2.com PART #test3 :Bye!")) } // Test the handler for KICK messages // (this is very similar to the PART message test) func TestKICK(t *testing.T) { - m, c := setUp(t) - defer tearDown(m, c) + c, s := setUp(t) + defer s.tearDown() - // Create user1 and add them to #test1 and #test2 - user1 := c.NewNick("user1", "ident1", "name one", "host1.com") - test1 := c.NewChannel("#test1") - test2 := c.NewChannel("#test2") - test1.AddNick(user1) - test2.AddNick(user1) + // We need some valid and associated nicks / channels to KICK. + chan1 := state.NewChannel("#test1", s.log) + nick1 := state.NewNick("user1", s.log) - // Add Me to both channels (not strictly necessary) - test1.AddNick(c.Me) - test2.AddNick(c.Me) - - // Then kick them! + // KICK should dissociate a nick from a channel. + gomock.InOrder( + s.st.EXPECT().GetChannel("#test1").Return(chan1), + s.st.EXPECT().GetNick("user1").Return(nick1), + s.st.EXPECT().Dissociate(chan1, nick1), + ) c.h_KICK(parseLine(":test!test@somehost.com KICK #test1 user1 :Bye!")) - - // Expect no output - m.ExpectNothing() - - // Quick check of tracking code - if len(test1.Nicks) != 1 { - t.Errorf("PART failed to remove user1 from #test1.") - } - - // Test error states. - // Kick a known user from a known channel they are not on. - c.h_KICK(parseLine(":test!test@somehost.com KICK #test1 user1 :Bye!")) - - // Kick an unknown user from a known channel. - c.h_KICK(parseLine(":test!test@somehost.com KICK #test2 user2 :Bye!")) - - // Kick a known user from an unknown channel. - c.h_KICK(parseLine(":test!test@somehost.com KICK #test3 user1 :Bye!")) - - // Kick an unknown user from an unknown channel. - c.h_KICK(parseLine(":test!test@somehost.com KICK #test4 user2 :Bye!")) } // Test the handler for QUIT messages func TestQUIT(t *testing.T) { - m, c := setUp(t) - defer tearDown(m, c) + c, s := setUp(t) + defer s.tearDown() - // Create user1 and add them to #test1 and #test2 - user1 := c.NewNick("user1", "ident1", "name one", "host1.com") - test1 := c.NewChannel("#test1") - test2 := c.NewChannel("#test2") - test1.AddNick(user1) - test2.AddNick(user1) - - // Add Me to both channels (not strictly necessary) - test1.AddNick(c.Me) - test2.AddNick(c.Me) - - // Have user1 QUIT + // Have user1 QUIT. All possible errors handled by state tracker \o/ + s.st.EXPECT().DelNick("user1") c.h_QUIT(parseLine(":user1!ident1@host1.com QUIT :Bye!")) - - // Expect no output - m.ExpectNothing() - - // Quick check of tracking code - if len(test1.Nicks) != 1 || len(test2.Nicks) != 1 { - t.Errorf("QUIT failed to remove user1 from channels.") - } - - // Ensure user1 is no longer a known nick - if c.GetNick("user1") != nil { - t.Errorf("QUIT failed to remove user1 from state tracking completely.") - } - - // Have user1 QUIT again, expect ERRORS! - c.h_QUIT(parseLine(":user1!ident1@host1.com QUIT :Bye!")) - - // Have a previously unmentioned user quit, expect an error - c.h_QUIT(parseLine(":user2!ident2@host2.com QUIT :Bye!")) } // Test the handler for MODE messages func TestMODE(t *testing.T) { - m, c := setUp(t) - defer tearDown(m, c) + c, s := setUp(t) + defer s.tearDown() - // Create user1 and add them to #test1 - user1 := c.NewNick("user1", "ident1", "name one", "host1.com") - test1 := c.NewChannel("#test1") - test1.AddNick(user1) - test1.AddNick(c.Me) - cm := test1.Modes + chan1 := state.NewChannel("#test1", s.log) + nick1 := state.NewNick("user1", s.log) - // Verify the ChanPrivs exists and modes we're testing aren't set - if cp, ok := user1.Channels[test1]; !ok || c.Me.Channels[test1].Voice || - cp.Op || cm.Key != "" || cm.InviteOnly || cm.Secret { - t.Errorf("Channel privileges in unexpected state before MODE.") + // Send a channel mode line. Inconveniently, Channel and Nick objects + // aren't mockable with gomock as they're not interface types (and I + // don't want them to be, writing accessors for struct fields sucks). + // This makes testing whether ParseModes is called correctly harder. + s.st.EXPECT().GetChannel("#test1").Return(chan1) + c.h_MODE(parseLine(":user1!ident1@host1.com MODE #test1 +sk somekey")) + if !chan1.Modes.Secret || chan1.Modes.Key != "somekey" { + t.Errorf("Channel.ParseModes() not called correctly.") } - // Send a channel mode line - c.h_MODE(parseLine(":user1!ident1@host1.com MODE #test1 +kisvo somekey test user1")) - // Expect no output - m.ExpectNothing() - - // Verify expected state afterwards. - if cp := user1.Channels[test1]; !(cp.Op || c.Me.Channels[test1].Voice || - cm.Key != "somekey" || cm.InviteOnly || cm.Secret) { - t.Errorf("Channel privileges in unexpected state after MODE.") + // Send a nick mode line, returning Me + gomock.InOrder( + s.st.EXPECT().GetChannel("test").Return(nil), + s.st.EXPECT().GetNick("test").Return(c.Me), + ) + c.h_MODE(parseLine(":test!test@somehost.com MODE test +i")) + if !c.Me.Modes.Invisible { + t.Errorf("Nick.ParseModes() not called correctly.") } - // Verify our nick modes are what we expect before test - nm := c.Me.Modes - if nm.Invisible || nm.WallOps || nm.HiddenHost { - t.Errorf("Our nick privileges in unexpected state before MODE.") - } - - // Send a nick mode line - c.h_MODE(parseLine(":test!test@somehost.com MODE test +ix")) - m.ExpectNothing() - - // Verify the two modes we expect to change did so - if !nm.Invisible || nm.WallOps || !nm.HiddenHost { - t.Errorf("Our nick privileges in unexpected state after MODE.") - } - - // Check error paths -- send a valid user mode that's not us + // Check error paths + gomock.InOrder( + // send a valid user mode that's not us + s.st.EXPECT().GetChannel("user1").Return(nil), + s.st.EXPECT().GetNick("user1").Return(nick1), + s.log.EXPECT().Warn("irc.MODE(): recieved MODE %s for (non-me) nick %s", + "+w", "user1"), + // Send a random mode for an unknown channel + s.st.EXPECT().GetChannel("#test2").Return(nil), + s.st.EXPECT().GetNick("#test2").Return(nil), + s.log.EXPECT().Warn("irc.MODE(): not sure what to do with MODE %s", + "#test2 +is"), + ) c.h_MODE(parseLine(":user1!ident1@host1.com MODE user1 +w")) - m.ExpectNothing() - - // Send a random mode for an unknown channel c.h_MODE(parseLine(":user1!ident1@host1.com MODE #test2 +is")) - m.ExpectNothing() } // Test the handler for TOPIC messages func TestTOPIC(t *testing.T) { - m, c := setUp(t) - defer tearDown(m, c) + c, s := setUp(t) + defer s.tearDown() - // Create #test1 so we have a channel to set the topic of - test1 := c.NewChannel("#test1") + chan1 := state.NewChannel("#test1", s.log) // Assert that it has no topic originally - if test1.Topic != "" { + if chan1.Topic != "" { t.Errorf("Test channel already has a topic.") } // Send a TOPIC line + s.st.EXPECT().GetChannel("#test1").Return(chan1) c.h_TOPIC(parseLine(":user1!ident1@host1.com TOPIC #test1 :something something")) - m.ExpectNothing() // Make sure the channel's topic has been changed - if test1.Topic != "something something" { + if chan1.Topic != "something something" { t.Errorf("Topic of test channel not set correctly.") } // Check error paths -- send a topic for an unknown channel + gomock.InOrder( + s.st.EXPECT().GetChannel("#test2").Return(nil), + s.log.EXPECT().Warn("irc.TOPIC(): topic change on unknown channel %s", + "#test2"), + ) c.h_TOPIC(parseLine(":user1!ident1@host1.com TOPIC #test2 :dark side")) - m.ExpectNothing() } // Test the handler for 311 / RPL_WHOISUSER func Test311(t *testing.T) { - m, c := setUp(t) - defer tearDown(m, c) + c, s := setUp(t) + defer s.tearDown() // Create user1, who we know little about - user1 := c.NewNick("user1", "", "", "") + nick1 := state.NewNick("user1", s.log) // Send a 311 reply + s.st.EXPECT().GetNick("user1").Return(nick1) c.h_311(parseLine(":irc.server.org 311 test user1 ident1 host1.com * :name")) - m.ExpectNothing() // Verify we now know more about user1 - if user1.Ident != "ident1" || - user1.Host != "host1.com" || - user1.Name != "name" { + if nick1.Ident != "ident1" || + nick1.Host != "host1.com" || + nick1.Name != "name" { t.Errorf("WHOIS info of user1 not set correctly.") } // Check error paths -- send a 311 for an unknown nick + gomock.InOrder( + s.st.EXPECT().GetNick("user2").Return(nil), + s.log.EXPECT().Warn("irc.311(): received WHOIS info for unknown nick %s", + "user2"), + ) c.h_311(parseLine(":irc.server.org 311 test user2 ident2 host2.com * :dongs")) - m.ExpectNothing() } // Test the handler for 324 / RPL_CHANNELMODEIS func Test324(t *testing.T) { - m, c := setUp(t) - defer tearDown(m, c) + c, s := setUp(t) + defer s.tearDown() // Create #test1, whose modes we don't know - test1 := c.NewChannel("#test1") - cm := test1.Modes - - // Make sure modes are unset first - if cm.Secret || cm.NoExternalMsg || cm.Moderated || cm.Key != "" { - t.Errorf("Channel modes unexpectedly set before 324 reply.") - } + chan1 := state.NewChannel("#test1", s.log) // Send a 324 reply - c.h_324(parseLine(":irc.server.org 324 test #test1 +snk somekey")) - m.ExpectNothing() - - // Make sure the modes we expected to be set were set and vice versa - if !cm.Secret || !cm.NoExternalMsg || cm.Moderated || cm.Key != "somekey" { - t.Errorf("Channel modes unexpectedly set before 324 reply.") + s.st.EXPECT().GetChannel("#test1").Return(chan1) + c.h_324(parseLine(":irc.server.org 324 test #test1 +sk somekey")) + if !chan1.Modes.Secret || chan1.Modes.Key != "somekey" { + t.Errorf("Channel.ParseModes() not called correctly.") } // Check unknown channel causes an error + gomock.InOrder( + s.st.EXPECT().GetChannel("#test2").Return(nil), + s.log.EXPECT().Warn("irc.324(): received MODE settings for unknown "+ + "channel %s", "#test2"), + ) c.h_324(parseLine(":irc.server.org 324 test #test2 +pmt")) - m.ExpectNothing() } // Test the handler for 332 / RPL_TOPIC func Test332(t *testing.T) { - m, c := setUp(t) - defer tearDown(m, c) + c, s := setUp(t) + defer s.tearDown() // Create #test1, whose topic we don't know - test1 := c.NewChannel("#test1") + chan1 := state.NewChannel("#test1", s.log) // Assert that it has no topic originally - if test1.Topic != "" { + if chan1.Topic != "" { t.Errorf("Test channel already has a topic.") } // Send a 332 reply + s.st.EXPECT().GetChannel("#test1").Return(chan1) c.h_332(parseLine(":irc.server.org 332 test #test1 :something something")) - m.ExpectNothing() // Make sure the channel's topic has been changed - if test1.Topic != "something something" { + if chan1.Topic != "something something" { t.Errorf("Topic of test channel not set correctly.") } // Check unknown channel causes an error - c.h_324(parseLine(":irc.server.org 332 test #test2 :dark side")) - m.ExpectNothing() + gomock.InOrder( + s.st.EXPECT().GetChannel("#test2").Return(nil), + s.log.EXPECT().Warn("irc.332(): received TOPIC value for unknown "+ + "channel %s", "#test2"), + ) + c.h_332(parseLine(":irc.server.org 332 test #test2 :dark side")) } // Test the handler for 352 / RPL_WHOREPLY func Test352(t *testing.T) { - m, c := setUp(t) - defer tearDown(m, c) + c, s := setUp(t) + defer s.tearDown() // Create user1, who we know little about - user1 := c.NewNick("user1", "", "", "") + nick1 := state.NewNick("user1", s.log) // Send a 352 reply + s.st.EXPECT().GetNick("user1").Return(nick1) c.h_352(parseLine(":irc.server.org 352 test #test1 ident1 host1.com irc.server.org user1 G :0 name")) - m.ExpectNothing() // Verify we now know more about user1 - if user1.Ident != "ident1" || - user1.Host != "host1.com" || - user1.Name != "name" || - user1.Modes.Invisible || - user1.Modes.Oper { + if nick1.Ident != "ident1" || + nick1.Host != "host1.com" || + nick1.Name != "name" || + nick1.Modes.Invisible || + nick1.Modes.Oper { t.Errorf("WHO info of user1 not set correctly.") } // Check that modes are set correctly from WHOREPLY + s.st.EXPECT().GetNick("user1").Return(nick1) c.h_352(parseLine(":irc.server.org 352 test #test1 ident1 host1.com irc.server.org user1 H* :0 name")) - m.ExpectNothing() - if !user1.Modes.Invisible || !user1.Modes.Oper { + if !nick1.Modes.Invisible || !nick1.Modes.Oper { t.Errorf("WHO modes of user1 not set correctly.") } // Check error paths -- send a 352 for an unknown nick + gomock.InOrder( + s.st.EXPECT().GetNick("user2").Return(nil), + s.log.EXPECT().Warn("irc.352(): received WHO reply for unknown nick %s", + "user2"), + ) c.h_352(parseLine(":irc.server.org 352 test #test2 ident2 host2.com irc.server.org user2 G :0 fooo")) - m.ExpectNothing() } // Test the handler for 353 / RPL_NAMREPLY func Test353(t *testing.T) { - m, c := setUp(t) - defer tearDown(m, c) + c, s := setUp(t) + defer s.tearDown() // Create #test1, whose user list we're mostly unfamiliar with - test1 := c.NewChannel("#test1") - user1 := c.NewNick("user1", "ident1", "name one", "host1.com") - test1.AddNick(user1) - test1.AddNick(c.Me) + chan1 := state.NewChannel("#test1", s.log) - // lazy lazy lazy ;-) - get := func(n string) *ChanPrivs { - if p, ok := test1.Nicks[c.GetNick(n)]; ok { - return p - } - return nil + // Create maps for testing -- this is what the mock ST calls will return + nicks := make(map[string]*state.Nick) + privs := make(map[string]*state.ChanPrivs) + + nicks["test"] = c.Me + privs["test"] = new(state.ChanPrivs) + + for _, n := range []string{"user1", "user2", "voice", "halfop", + "op", "admin", "owner"} { + nicks[n] = state.NewNick(n, s.log) + privs[n] = new(state.ChanPrivs) } - // Verify the lack of nicks - if len(test1.Nicks) != 2 { - t.Errorf("Unexpected number of nicks in test channel before 353.") + // 353 handler is called twice, so GetChannel will be called twice + s.st.EXPECT().GetChannel("#test1").Return(chan1).Times(2) + gomock.InOrder( + // "test" is Me, i am known, and already on the channel + s.st.EXPECT().GetNick("test").Return(c.Me), + s.st.EXPECT().IsOn("#test1", "test").Return(privs["test"], true), + // user1 is known, but not on the channel, so should be associated + s.st.EXPECT().GetNick("user1").Return(nicks["user1"]), + s.st.EXPECT().IsOn("#test1", "user1").Return(nil, false), + s.st.EXPECT().Associate(chan1, nicks["user1"]).Return(privs["user1"]), + ) + for _, n := range []string{"user2", "voice", "halfop", "op", "admin", "owner"} { + gomock.InOrder( + s.st.EXPECT().GetNick(n).Return(nil), + s.st.EXPECT().NewNick(n).Return(nicks[n]), + s.st.EXPECT().IsOn("#test1", n).Return(nil, false), + s.st.EXPECT().Associate(chan1, nicks[n]).Return(privs[n]), + ) } - // Verify that user1 isn't opped yet - if p := get("user1"); p == nil || p.Op { - t.Errorf("Unexpected permissions for user1 before 353.") - } - - // Send a couple of names replies (complete with trailing space), expect no errors + // Send a couple of names replies (complete with trailing space) c.h_353(parseLine(":irc.server.org 353 test = #test1 :test @user1 user2 +voice ")) c.h_353(parseLine(":irc.server.org 353 test = #test1 :%halfop @op &admin ~owner ")) - m.ExpectNothing() - if len(test1.Nicks) != 8 { - t.Errorf("Unexpected number of nicks in test channel after 353.") + if p := privs["user2"]; p.Voice || p.HalfOp || p.Op || p.Admin || p.Owner { + t.Errorf("353 handler incorrectly set modes on nick.") } - // TODO(fluffle): Testing side-effects is starting to get on my tits. - // As a result, this makes some assumptions about the implementation of - // h_353 that may or may not be valid in the future. Hopefully, I will have - // time to rewrite the nick / channel handling soon. - if p := get("user1"); p == nil || !p.Op { - t.Errorf("353 handler failed to op known nick user1.") + if !privs["user1"].Op || !privs["voice"].Voice || !privs["halfop"].HalfOp || + !privs["op"].Op || !privs["admin"].Admin || !privs["owner"].Owner { + t.Errorf("353 handler failed to set correct modes for nicks.") } - if p := get("user2"); p == nil || p.Voice || p.HalfOp || p.Op || p.Admin || p.Owner { - t.Errorf("353 handler set modes on new nick user2.") - } - - if p := get("voice"); p == nil || !p.Voice { - t.Errorf("353 handler failed to parse voice correctly.") - } - - if p := get("halfop"); p == nil || !p.HalfOp { - t.Errorf("353 handler failed to parse halfop correctly.") - } - - if p := get("op"); p == nil || !p.Op { - t.Errorf("353 handler failed to parse op correctly.") - } - - if p := get("admin"); p == nil || !p.Admin { - t.Errorf("353 handler failed to parse admin correctly.") - } - - if p := get("owner"); p == nil || !p.Owner { - t.Errorf("353 handler failed to parse owner correctly.") - } - - // Check unknown channel causes an error - c.h_324(parseLine(":irc.server.org 353 test = #test2 :test ~user3")) - m.ExpectNothing() + // Check unknown channel causes a warning + gomock.InOrder( + s.st.EXPECT().GetChannel("#test2").Return(nil), + s.log.EXPECT().Warn("irc.353(): received NAMES list for unknown "+ + "channel %s", "#test2"), + ) + c.h_353(parseLine(":irc.server.org 353 test = #test2 :test ~user3")) } // Test the handler for 671 (unreal specific) func Test671(t *testing.T) { - m, c := setUp(t) - defer tearDown(m, c) + c, s := setUp(t) + defer s.tearDown() // Create user1, who should not be secure - user1 := c.NewNick("user1", "ident1", "name one", "host1.com") - if user1.Modes.SSL { + nick1 := state.NewNick("user1", s.log) + if nick1.Modes.SSL { t.Errorf("Test nick user1 is already using SSL?") } // Send a 671 reply + s.st.EXPECT().GetNick("user1").Return(nick1) c.h_671(parseLine(":irc.server.org 671 test user1 :some ignored text")) - m.ExpectNothing() // Ensure user1 is now known to be on an SSL connection - if !user1.Modes.SSL { + if !nick1.Modes.SSL { t.Errorf("Test nick user1 not using SSL?") } // Check error paths -- send a 671 for an unknown nick + gomock.InOrder( + s.st.EXPECT().GetNick("user2").Return(nil), + s.log.EXPECT().Warn("irc.671(): received WHOIS SSL info for unknown "+ + "nick %s", "user2"), + ) c.h_671(parseLine(":irc.server.org 671 test user2 :some ignored text")) - m.ExpectNothing() } diff --git a/client/line.go b/client/line.go index 26d21ce..789d3cc 100644 --- a/client/line.go +++ b/client/line.go @@ -1,7 +1,6 @@ package client import ( - "github.com/fluffle/goirc/logging" "strings" "time" ) @@ -34,9 +33,7 @@ func parseLine(s string) *Line { line.Src, s = s[1:idx], s[idx+1:] } else { // pretty sure we shouldn't get here ... - logging.Warn("Parsing of line '%s' didn't go well.", s) - line.Src = s[1:] - return line + return nil } // src can be the hostname of the irc server or a nick!user@host