From 91c71da2ddb39039d1e0567b769ffd4b8096d588 Mon Sep 17 00:00:00 2001 From: Alex Bramley Date: Sat, 16 Feb 2013 00:17:31 +0000 Subject: [PATCH] Re-work Handlers for IRC events; add Commands. --- client/connection.go | 65 +++++------ client/connection_test.go | 137 +++++++++++++--------- client/dispatch.go | 228 ++++++++++++++++++++++++++++++++++++ client/dispatch_test.go | 229 +++++++++++++++++++++++++++++++++++++ client/handlers.go | 77 +++++++------ client/handlers_test.go | 71 +++++++++++- client/mocknetconn_test.go | 4 +- client/state_handlers.go | 39 +++---- 8 files changed, 692 insertions(+), 158 deletions(-) create mode 100644 client/dispatch.go create mode 100644 client/dispatch_test.go diff --git a/client/connection.go b/client/connection.go index 112767f..dce6c43 100644 --- a/client/connection.go +++ b/client/connection.go @@ -5,7 +5,6 @@ import ( "crypto/tls" "errors" "fmt" - "github.com/fluffle/goevent/event" "github.com/fluffle/goirc/state" "github.com/fluffle/golog/logging" "net" @@ -20,16 +19,14 @@ type Conn struct { Me *state.Nick Network string - // Replaceable function to customise the 433 handler's new nick - NewNick func(string) string - - // Event handler registry and dispatcher - ER event.EventRegistry - ED event.EventDispatcher + // Handlers and Commands + handlers *hSet + commands *cSet // State tracker for nicks and channels - ST state.StateTracker - st bool + ST state.StateTracker + st bool + stRemovers []Remover // Use the State field to store external state that handlers might need. // Remember ... you might need locking for this ;-) @@ -50,9 +47,15 @@ type Conn struct { SSL bool SSLConfig *tls.Config + // Replaceable function to customise the 433 handler's new nick + NewNick func(string) string + // Client->server ping frequency, in seconds. Defaults to 3m. PingFreq time.Duration + // Controls what is stripped from line.Args[1] for Commands + CommandStripNick, CommandStripPrefix bool + // Set this to true to disable flood protection and false to re-enable Flood bool @@ -62,9 +65,9 @@ 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 SimpleClient(nick string, args ...string) *Conn { - r := event.NewRegistry() +// that you can add event handlers to it. See AddHandler() for details +func Client(nick string, args ...string) *Conn { + logging.InitFromFlags() ident := "goirc" name := "Powered by GoIRC" @@ -74,30 +77,18 @@ func SimpleClient(nick string, args ...string) *Conn { if len(args) > 1 && args[1] != "" { name = args[1] } - return Client(nick, ident, name, r) -} - -func Client(nick, ident, name string, r event.EventRegistry) *Conn { - if r == nil { - return nil - } - logging.InitFromFlags() conn := &Conn{ - ER: r, - ED: r, - st: false, - in: make(chan *Line, 32), - out: make(chan string, 32), - cSend: make(chan bool), - cLoop: make(chan bool), - cPing: make(chan bool), - SSL: false, - SSLConfig: nil, - PingFreq: 3 * time.Minute, - Flood: false, - NewNick: func(s string) string { return s + "_" }, - badness: 0, - lastsent: time.Now(), + in: make(chan *Line, 32), + out: make(chan string, 32), + cSend: make(chan bool), + cLoop: make(chan bool), + cPing: make(chan bool), + handlers: handlerSet(), + commands: commandSet(), + stRemovers: make([]Remover, 0, len(stHandlers)), + PingFreq: 3 * time.Minute, + NewNick: func(s string) string { return s + "_" }, + lastsent: time.Now(), } conn.addIntHandlers() conn.Me = state.NewNick(nick) @@ -257,7 +248,7 @@ func (conn *Conn) runLoop() { for { select { case line := <-conn.in: - conn.ED.Dispatch(line.Cmd, conn, line) + conn.dispatch(line) case <-conn.cLoop: // strobe on control channel, bail out return @@ -314,7 +305,7 @@ func (conn *Conn) shutdown() { // as calling sock.Close() will cause recv() to recieve EOF in readstring() if conn.Connected { logging.Info("irc.shutdown(): Disconnected from server.") - conn.ED.Dispatch("disconnected", conn, &Line{}) + conn.dispatch(&Line{Cmd: "disconnected"}) conn.Connected = false conn.sock.Close() conn.cSend <- true diff --git a/client/connection_test.go b/client/connection_test.go index f6cb1ff..6386eb3 100644 --- a/client/connection_test.go +++ b/client/connection_test.go @@ -3,7 +3,6 @@ package client import ( "bufio" "code.google.com/p/gomock/gomock" - "github.com/fluffle/goevent/event" "github.com/fluffle/golog/logging" "github.com/fluffle/goirc/state" "strings" @@ -14,7 +13,6 @@ import ( type testState struct { ctrl *gomock.Controller st *state.MockStateTracker - ed *event.MockEventDispatcher nc *mockNetConn c *Conn } @@ -22,13 +20,10 @@ type testState struct { func setUp(t *testing.T, start ...bool) (*Conn, *testState) { ctrl := gomock.NewController(t) st := state.NewMockStateTracker(ctrl) - r := event.NewRegistry() - ed := event.NewMockEventDispatcher(ctrl) nc := MockNetConn(t) - c := Client("test", "test", "Testing IRC", r) + c := Client("test", "test", "Testing IRC") logging.SetLogLevel(logging.LogFatal) - c.ED = ed c.ST = st c.st = true c.sock = nc @@ -42,15 +37,14 @@ func setUp(t *testing.T, start ...bool) (*Conn, *testState) { <-time.After(1e6) } - return c, &testState{ctrl, st, ed, nc, c} + return c, &testState{ctrl, st, nc, c} } func (s *testState) tearDown() { - s.ed.EXPECT().Dispatch("disconnected", s.c, &Line{}) s.st.EXPECT().Wipe() s.nc.ExpectNothing() s.c.shutdown() - <-time.After(1e6) + <-time.After(time.Millisecond) s.ctrl.Finish() } @@ -61,56 +55,61 @@ func TestEOF(t *testing.T) { // Since we're not using tearDown() here, manually call Finish() defer s.ctrl.Finish() + // Set up a handler to detect whether disconnected handlers are called + dcon := false + c.HandleFunc("disconnected", func (conn *Conn, line *Line) { + dcon = true + }) + // Simulate EOF from server - s.ed.EXPECT().Dispatch("disconnected", c, &Line{}) s.st.EXPECT().Wipe() s.nc.Close() // Since things happen in different internal goroutines, we need to wait // 1 ms should be enough :-) - <-time.After(1e6) + <-time.After(time.Millisecond) // Verify that the connection no longer thinks it's connected if c.Connected { t.Errorf("Conn still thinks it's connected to the server.") } + + // Verify that disconnected handler was called + if !dcon { + t.Errorf("Conn did not call disconnected handlers.") + } } func TestClientAndStateTracking(t *testing.T) { - // This doesn't use setUp() as we want to pass in a mock EventRegistry. ctrl := gomock.NewController(t) - r := event.NewMockEventRegistry(ctrl) st := state.NewMockStateTracker(ctrl) - - for n, _ := range intHandlers { - // We can't use EXPECT() here as comparisons of functions are - // no longer valid in Go, which causes reflect.DeepEqual to bail. - // Instead, ignore the function arg and just ensure that all the - // handler names are correctly passed to AddHandler. - ctrl.RecordCall(r, "AddHandler", gomock.Any(), n) - } - c := Client("test", "test", "Testing IRC", r) + c := Client("test", "test", "Testing IRC") // Assert some basic things about the initial state of the Conn struct - if c.ER != r || c.ED != r || c.st != false || c.ST != nil { - t.Errorf("Conn not correctly initialised with external deps.") - } - if c.in == nil || c.out == nil || c.cSend == nil || c.cLoop == nil { - t.Errorf("Conn control channels not correctly initialised.") - } if c.Me.Nick != "test" || c.Me.Ident != "test" || c.Me.Name != "Testing IRC" || c.Me.Host != "" { t.Errorf("Conn.Me not correctly initialised.") } - - // OK, while we're here with a mock event registry... - for n, _ := range stHandlers { - // See above. - ctrl.RecordCall(r, "AddHandler", gomock.Any(), n) + // Check that the internal handlers are correctly set up + for k, _ := range intHandlers { + if _, ok := c.handlers.set[strings.ToLower(k)]; !ok { + t.Errorf("Missing internal handler for '%s'.", k) + } } - c.EnableStateTracking() - // We're expecting the untracked me to be replaced by a tracked one. + // Now enable the state tracking code and check its handlers + c.EnableStateTracking() + for k, _ := range stHandlers { + if _, ok := c.handlers.set[strings.ToLower(k)]; !ok { + t.Errorf("Missing state handler for '%s'.", k) + } + } + if len(c.stRemovers) != len(stHandlers) { + t.Errorf("Incorrect number of Removers (%d != %d) when adding state handlers.", + len(c.stRemovers), len(stHandlers)) + } + + // We're expecting the untracked me to be replaced by a tracked one if c.Me.Nick != "test" || c.Me.Ident != "test" || c.Me.Name != "Testing IRC" || c.Me.Host != "" { t.Errorf("Enabling state tracking did not replace Me correctly.") @@ -119,18 +118,25 @@ func TestClientAndStateTracking(t *testing.T) { t.Errorf("State tracker not enabled correctly.") } - // Now, shim in the mock state tracker and test disabling state tracking. + // Now, shim in the mock state tracker and test disabling state tracking me := c.Me c.ST = st st.EXPECT().Wipe() - for n, _ := range stHandlers { - // See above. - ctrl.RecordCall(r, "DelHandler", gomock.Any(), n) - } c.DisableStateTracking() if c.st || c.ST != nil || c.Me != me { t.Errorf("State tracker not disabled correctly.") } + + // Finally, check state tracking handlers were all removed correctly + for k, _ := range stHandlers { + if _, ok := c.handlers.set[strings.ToLower(k)]; ok && k != "NICK" { + // A bit leaky, because intHandlers adds a NICK handler. + t.Errorf("State handler for '%s' not removed correctly.", k) + } + } + if len(c.stRemovers) != 0 { + t.Errorf("stRemovers not zeroed correctly when removing state handlers.") + } ctrl.Finish() } @@ -202,7 +208,7 @@ func TestRecv(t *testing.T) { // reader is a helper to do a "non-blocking" read of c.in reader := func() *Line { select { - case <-time.After(1e6): + case <-time.After(time.Millisecond): case l := <-c.in: return l } @@ -221,7 +227,7 @@ func TestRecv(t *testing.T) { // Strangely, recv() needs some time to start up, but *only* when this test // is run standalone with: client/_test/_testmain --test.run TestRecv - <-time.After(1e6) + <-time.After(time.Millisecond) // Now, this should mean that we'll receive our parsed line on c.in if l := reader(); l == nil || l.Cmd != "001" { @@ -245,7 +251,6 @@ func TestRecv(t *testing.T) { if exited { t.Errorf("Exited before socket close.") } - s.ed.EXPECT().Dispatch("disconnected", c, &Line{}) s.st.EXPECT().Wipe() s.nc.Close() @@ -255,7 +260,7 @@ func TestRecv(t *testing.T) { <-c.cLoop <-c.cPing // Give things time to shake themselves out... - <-time.After(1e6) + <-time.After(time.Millisecond) if !exited { t.Errorf("Didn't exit on socket close.") } @@ -296,7 +301,7 @@ func TestPing(t *testing.T) { exited = true }() - // The first ping should be after a second, + // The first ping should be after 50ms, // so we don't expect anything now on c.in if s := reader(); s != "" { t.Errorf("Line output directly after ping started.") @@ -349,15 +354,25 @@ func TestRunLoop(t *testing.T) { bufio.NewReader(c.sock), bufio.NewWriter(c.sock)) - // NOTE: here we assert that no Dispatch event has been called yet by - // calling s.ctrl.Finish(). There doesn't appear to be any harm in this. + // Set up a handler to detect whether 001 handler is called + h001 := false + c.HandleFunc("001", func (conn *Conn, line *Line) { + h001 = true + }) + // Set up a handler to detect whether 002 handler is called + h002 := false + c.HandleFunc("002", func (conn *Conn, line *Line) { + h002 = true + }) + l1 := parseLine(":irc.server.org 001 test :First test line.") c.in <- l1 - s.ctrl.Finish() + if h001 { + t.Errorf("001 handler called before runLoop started.") + } // We want to test that the a goroutine calling runLoop will exit correctly. // Now, we can expect the call to Dispatch to take place as runLoop starts. - s.ed.EXPECT().Dispatch("001", c, l1) exited := false go func() { c.runLoop() @@ -365,15 +380,20 @@ func TestRunLoop(t *testing.T) { }() // Here, the opposite seemed to take place, with TestRunLoop failing when // run as part of the suite but passing when run on it's own. - <-time.After(1e6) + <-time.After(time.Millisecond) + if !h001 { + t.Errorf("001 handler not called after runLoop started.") + } // Send another line, just to be sure :-) l2 := parseLine(":irc.server.org 002 test :Second test line.") - s.ed.EXPECT().Dispatch("002", c, l2) c.in <- l2 // It appears some sleeping is needed after all of these to ensure channel // sends occur before the close signal is sent below... - <-time.After(1e6) + <-time.After(time.Millisecond) + if !h002 { + t.Errorf("002 handler not called while runLoop started.") + } // Now, use the control channel to exit send and kill the goroutine. if exited { @@ -381,13 +401,17 @@ func TestRunLoop(t *testing.T) { } c.cLoop <- true // Allow propagation time... - <-time.After(1e6) + <-time.After(time.Millisecond) if !exited { t.Errorf("Didn't exit after signal.") } // Sending more on c.in shouldn't dispatch any further events + h001 = false c.in <- l1 + if h001 { + t.Errorf("001 handler called after runLoop ended.") + } } func TestWrite(t *testing.T) { @@ -432,8 +456,6 @@ func TestWrite(t *testing.T) { <-c.cPing }() s.nc.Close() - - s.ed.EXPECT().Dispatch("disconnected", c, &Line{}) s.st.EXPECT().Wipe() c.write("she can't pass unit tests") } @@ -468,13 +490,14 @@ func TestRateLimit(t *testing.T) { // characters as the line length means we should be increasing badness by // 2.5 seconds minus the delta between the two ratelimit calls. This should // be minimal but it's guaranteed that it won't be zero. Use 10us as a fuzz. - if l := c.rateLimit(60); l != 0 || abs(c.badness - 25*1e8) > 10 * time.Microsecond { + if l := c.rateLimit(60); l != 0 || + abs(c.badness - 2500*time.Millisecond) > 10 * time.Microsecond { t.Errorf("Rate limit calculating badness incorrectly.") } // At this point, we can tip over the badness scale, with a bit of help. // 720 chars => +8 seconds of badness => 10.5 seconds => ratelimit if l := c.rateLimit(720); l != 8 * time.Second || - abs(c.badness - 105*1e8) > 10 * time.Microsecond { + abs(c.badness - 10500*time.Millisecond) > 10 * time.Microsecond { t.Errorf("Rate limit failed to return correct limiting values.") t.Errorf("l=%d, badness=%d", l, c.badness) } diff --git a/client/dispatch.go b/client/dispatch.go new file mode 100644 index 0000000..f35a10f --- /dev/null +++ b/client/dispatch.go @@ -0,0 +1,228 @@ +package client + +import ( + "github.com/fluffle/golog/logging" + "strings" + "sync" +) + +// An IRC handler looks like this: +type Handler interface { + Handle(*Conn, *Line) +} + +// And when they've been added to the client they are removable. +type Remover interface { + Remove() +} + +type HandlerFunc func(*Conn, *Line) + +func (hf HandlerFunc) Handle(conn *Conn, line *Line) { + hf(conn, line) +} + +type hList struct { + start, end *hNode +} + +type hNode struct { + next, prev *hNode + set *hSet + event string + handler Handler +} + +func (hn *hNode) Handle(conn *Conn, line *Line) { + hn.handler.Handle(conn, line) +} + +func (hn *hNode) Remove() { + hn.set.remove(hn) +} + +type hSet struct { + set map[string]*hList + sync.RWMutex +} + +func handlerSet() *hSet { + return &hSet{set: make(map[string]*hList)} +} + +func (hs *hSet) add(ev string, h Handler) Remover { + hs.Lock() + defer hs.Unlock() + ev = strings.ToLower(ev) + l, ok := hs.set[ev] + if !ok { + l = &hList{} + } + hn := &hNode{ + set: hs, + event: ev, + handler: h, + } + if !ok { + l.start = hn + } else { + hn.prev = l.end + l.end.next = hn + } + l.end = hn + hs.set[ev] = l + return hn +} + +func (hs *hSet) remove(hn *hNode) { + hs.Lock() + defer hs.Unlock() + l, ok := hs.set[hn.event] + if !ok { + logging.Error("Removing node for unknown event '%s'", hn.event) + return + } + if hn.next == nil { + l.end = hn.prev + } else { + hn.next.prev = hn.prev + } + if hn.prev == nil { + l.start = hn.next + } else { + hn.prev.next = hn.next + } + hn.next = nil + hn.prev = nil + hn.set = nil + if l.start == nil || l.end == nil { + delete(hs.set, hn.event) + } +} + +func (hs *hSet) dispatch(conn *Conn, line *Line) { + hs.RLock() + defer hs.RUnlock() + ev := strings.ToLower(line.Cmd) + list, ok := hs.set[ev] + if !ok { return } + for hn := list.start; hn != nil; hn = hn.next { + go hn.Handle(conn, line) + } +} + +// An IRC command looks like this: +type Command interface { + Execute(*Conn, *Line) + Help() string +} + +type command struct { + fn HandlerFunc + help string +} + +func (c *command) Execute(conn *Conn, line *Line) { + c.fn(conn, line) +} + +func (c *command) Help() string { + return c.help +} + +type cNode struct { + cmd Command + set *cSet + prefix string +} + +func (cn *cNode) Execute(conn *Conn, line *Line) { + cn.cmd.Execute(conn, line) +} + +func (cn *cNode) Help() string { + return cn.cmd.Help() +} + +func (cn *cNode) Remove() { + cn.set.remove(cn) +} + +type cSet struct { + set map[string]*cNode + sync.RWMutex +} + +func commandSet() *cSet { + return &cSet{set: make(map[string]*cNode)} +} + +func (cs *cSet) add(pf string, c Command) Remover { + cs.Lock() + defer cs.Unlock() + pf = strings.ToLower(pf) + if _, ok := cs.set[pf]; ok { + logging.Error("Command prefix '%s' already registered.", pf) + return nil + } + cn := &cNode{ + cmd: c, + set: cs, + prefix: pf, + } + cs.set[pf] = cn + return cn +} + +func (cs *cSet) remove(cn *cNode) { + cs.Lock() + defer cs.Unlock() + delete(cs.set, cn.prefix) + cn.set = nil +} + +func (cs *cSet) match(txt string) (final Command, prefixlen int) { + cs.RLock() + defer cs.RUnlock() + txt = strings.ToLower(txt) + for prefix, cmd := range cs.set { + if !strings.HasPrefix(txt, prefix) { + continue + } + if final == nil || len(prefix) > prefixlen { + prefixlen = len(prefix) + final = cmd + } + } + return +} + +// Handlers are triggered on incoming Lines from the server, with the handler +// "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 +// putting massive constant tables in). +func (conn *Conn) Handle(name string, h Handler) Remover { + return conn.handlers.add(name, h) +} + +func (conn *Conn) HandleFunc(name string, hf HandlerFunc) Remover { + return conn.Handle(name, hf) +} + +func (conn *Conn) Command(prefix string, c Command) Remover { + return conn.commands.add(prefix, c) +} + +func (conn *Conn) CommandFunc(prefix string, hf HandlerFunc, help string) Remover { + return conn.Command(prefix, &command{hf, help}) +} + +func (conn *Conn) dispatch(line *Line) { + conn.handlers.dispatch(conn, line) +} + +func (conn *Conn) cmdMatch(txt string) (Command, int) { + return conn.commands.match(txt) +} diff --git a/client/dispatch_test.go b/client/dispatch_test.go new file mode 100644 index 0000000..427c69e --- /dev/null +++ b/client/dispatch_test.go @@ -0,0 +1,229 @@ +package client + +import ( + "testing" + "time" +) + +func TestHandlerSet(t *testing.T) { + hs := handlerSet() + if len(hs.set) != 0 { + t.Errorf("New set contains things!") + } + + callcount := 0 + f := func(c *Conn, l *Line) { + callcount++ + } + + // Add one + hn1 := hs.add("ONE", HandlerFunc(f)).(*hNode) + hl, ok := hs.set["one"] + if len(hs.set) != 1 || !ok { + t.Errorf("Set doesn't contain 'one' list after add().") + } + if hn1.set != hs || hn1.event != "one" || hn1.prev != nil || hn1.next != nil { + t.Errorf("First node for 'one' not created correctly") + } + if hl.start != hn1 || hl.end != hn1 { + t.Errorf("Node not added to empty 'one' list correctly.") + } + + // Add another one... + hn2 := hs.add("one", HandlerFunc(f)).(*hNode) + if len(hs.set) != 1 { + t.Errorf("Set contains more than 'one' list after add().") + } + if hn2.set != hs || hn2.event != "one" { + t.Errorf("Second node for 'one' not created correctly") + } + if hn1.prev != nil || hn1.next != hn2 || hn2.prev != hn1 || hn2.next != nil { + t.Errorf("Nodes for 'one' not linked correctly.") + } + if hl.start != hn1 || hl.end != hn2 { + t.Errorf("Node not appended to 'one' list correctly.") + } + + // Add a third one! + hn3 := hs.add("one", HandlerFunc(f)).(*hNode) + if len(hs.set) != 1 { + t.Errorf("Set contains more than 'one' list after add().") + } + if hn3.set != hs || hn3.event != "one" { + t.Errorf("Third node for 'one' not created correctly") + } + if hn1.prev != nil || hn1.next != hn2 || + hn2.prev != hn1 || hn2.next != hn3 || + hn3.prev != hn2 || hn3.next != nil { + t.Errorf("Nodes for 'one' not linked correctly.") + } + if hl.start != hn1 || hl.end != hn3 { + t.Errorf("Node not appended to 'one' list correctly.") + } + + // And finally a fourth one! + hn4 := hs.add("one", HandlerFunc(f)).(*hNode) + if len(hs.set) != 1 { + t.Errorf("Set contains more than 'one' list after add().") + } + if hn4.set != hs || hn4.event != "one" { + t.Errorf("Fourth node for 'one' not created correctly.") + } + if hn1.prev != nil || hn1.next != hn2 || + hn2.prev != hn1 || hn2.next != hn3 || + hn3.prev != hn2 || hn3.next != hn4 || + hn4.prev != hn3 || hn4.next != nil { + t.Errorf("Nodes for 'one' not linked correctly.") + } + if hl.start != hn1 || hl.end != hn4 { + t.Errorf("Node not appended to 'one' list correctly.") + } + + // Dispatch should result in 4 additions. + if callcount != 0 { + t.Errorf("Something incremented call count before we were expecting it.") + } + hs.dispatch(nil, &Line{Cmd:"One"}) + <-time.After(time.Millisecond) + if callcount != 4 { + t.Errorf("Our handler wasn't called four times :-(") + } + + // Remove node 3. + hn3.Remove() + if len(hs.set) != 1 { + t.Errorf("Set list count changed after remove().") + } + if hn3.set != nil || hn3.prev != nil || hn3.next != nil { + t.Errorf("Third node for 'one' not removed correctly.") + } + if hn1.prev != nil || hn1.next != hn2 || + hn2.prev != hn1 || hn2.next != hn4 || + hn4.prev != hn2 || hn4.next != nil { + t.Errorf("Third node for 'one' not unlinked correctly.") + } + if hl.start != hn1 || hl.end != hn4 { + t.Errorf("Third node for 'one' changed list pointers.") + } + + // Dispatch should result in 3 additions. + hs.dispatch(nil, &Line{Cmd:"One"}) + <-time.After(time.Millisecond) + if callcount != 7 { + t.Errorf("Our handler wasn't called three times :-(") + } + + // Remove node 1. + hs.remove(hn1) + if len(hs.set) != 1 { + t.Errorf("Set list count changed after remove().") + } + if hn1.set != nil || hn1.prev != nil || hn1.next != nil { + t.Errorf("First node for 'one' not removed correctly.") + } + if hn2.prev != nil || hn2.next != hn4 || hn4.prev != hn2 || hn4.next != nil { + t.Errorf("First node for 'one' not unlinked correctly.") + } + if hl.start != hn2 || hl.end != hn4 { + t.Errorf("First node for 'one' didn't change list pointers.") + } + + // Dispatch should result in 2 additions. + hs.dispatch(nil, &Line{Cmd:"One"}) + <-time.After(time.Millisecond) + if callcount != 9 { + t.Errorf("Our handler wasn't called two times :-(") + } + + // Remove node 4. + hn4.Remove() + if len(hs.set) != 1 { + t.Errorf("Set list count changed after remove().") + } + if hn4.set != nil || hn4.prev != nil || hn4.next != nil { + t.Errorf("Fourth node for 'one' not removed correctly.") + } + if hn2.prev != nil || hn2.next != nil { + t.Errorf("Fourth node for 'one' not unlinked correctly.") + } + if hl.start != hn2 || hl.end != hn2 { + t.Errorf("Fourth node for 'one' didn't change list pointers.") + } + + // Dispatch should result in 1 addition. + hs.dispatch(nil, &Line{Cmd:"One"}) + <-time.After(time.Millisecond) + if callcount != 10 { + t.Errorf("Our handler wasn't called once :-(") + } + + // Remove node 2. + hs.remove(hn2) + if len(hs.set) != 0 { + t.Errorf("Removing last node in 'one' didn't remove list.") + } + if hn2.set != nil || hn2.prev != nil || hn2.next != nil { + t.Errorf("Second node for 'one' not removed correctly.") + } + if hl.start != nil || hl.end != nil { + t.Errorf("Second node for 'one' didn't change list pointers.") + } + + // Dispatch should result in NO additions. + hs.dispatch(nil, &Line{Cmd:"One"}) + <-time.After(time.Millisecond) + if callcount != 10 { + t.Errorf("Our handler was called?") + } +} + +func TestCommandSet(t *testing.T) { + cs := commandSet() + if len(cs.set) != 0 { + t.Errorf("New set contains things!") + } + + c := &command{ + fn: func(c *Conn, l *Line) {}, + help: "wtf?", + } + + cn1 := cs.add("ONE", c).(*cNode) + if _, ok := cs.set["one"]; !ok || cn1.set != cs || cn1.prefix != "one" { + t.Errorf("Command 'one' not added to set correctly.") + } + + if fail := cs.add("one", c); fail != nil { + t.Errorf("Adding a second 'one' command did not fail as expected.") + } + + cn2 := cs.add("One Two", c).(*cNode) + if _, ok := cs.set["one two"]; !ok || cn2.set != cs || cn2.prefix != "one two" { + t.Errorf("Command 'one two' not added to set correctly.") + } + + if c, l := cs.match("foo"); c != nil || l != 0 { + t.Errorf("Matched 'foo' when we shouldn't.") + } + if c, l := cs.match("one"); c.(*cNode) != cn1 || l != 3 { + t.Errorf("Didn't match 'one' when we should have.") + } + if c, l := cs.match ("one two three"); c.(*cNode) != cn2 || l != 7 { + t.Errorf("Didn't match 'one two' when we should have.") + } + + cs.remove(cn2) + if _, ok := cs.set["one two"]; ok || cn2.set != nil { + t.Errorf("Command 'one two' not removed correctly.") + } + if c, l := cs.match ("one two three"); c.(*cNode) != cn1 || l != 3 { + t.Errorf("Didn't match 'one' when we should have.") + } + cn1.Remove() + if _, ok := cs.set["one"]; ok || cn1.set != nil { + t.Errorf("Command 'one' not removed correctly.") + } + if c, l := cs.match ("one two three"); c != nil || l != 0 { + t.Errorf("Matched 'one' when we shouldn't have.") + } +} diff --git a/client/handlers.go b/client/handlers.go index 72a72c6..dcc48f9 100644 --- a/client/handlers.go +++ b/client/handlers.go @@ -4,48 +4,23 @@ package client // to manage tracking an irc connection etc. import ( - "github.com/fluffle/goevent/event" "strings" ) -// An IRC handler looks like this: -type IRCHandler func(*Conn, *Line) - -// AddHandler() adds an event handler for a specific IRC command. -// -// Handlers are triggered on incoming Lines from the server, with the handler -// "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 -// putting massive constant tables in). -func (conn *Conn) AddHandler(name string, f IRCHandler) event.Handler { - h := NewHandler(f) - conn.ER.AddHandler(h, name) - return h -} - -// Wrap f in an anonymous unboxing function -func NewHandler(f IRCHandler) event.Handler { - return event.NewHandler(func(ev ...interface{}) { - f(ev[0].(*Conn), ev[1].(*Line)) - }) -} - // sets up the internal event handlers to do essential IRC protocol things -var intHandlers map[string]event.Handler -func init() { - intHandlers = make(map[string]event.Handler) - intHandlers["001"] = NewHandler((*Conn).h_001) - intHandlers["433"] = NewHandler((*Conn).h_433) - intHandlers["CTCP"] = NewHandler((*Conn).h_CTCP) - intHandlers["NICK"] = NewHandler((*Conn).h_NICK) - intHandlers["PING"] = NewHandler((*Conn).h_PING) +var intHandlers = map[string]HandlerFunc{ + "001": (*Conn).h_001, + "433": (*Conn).h_433, + "CTCP": (*Conn).h_CTCP, + "NICK": (*Conn).h_NICK, + "PING": (*Conn).h_PING, } func (conn *Conn) addIntHandlers() { for n, h := range intHandlers { - conn.ER.AddHandler(h, n) + // internal handlers are essential for the IRC client + // to function, so we don't save their Removers here + conn.Handle(n, h) } } @@ -57,7 +32,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.ED.Dispatch("connected", conn, line) + conn.dispatch(&Line{Cmd: "connected"}) // 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 { @@ -108,3 +83,35 @@ func (conn *Conn) h_NICK(line *Line) { conn.Me.Nick = line.Args[0] } } + +// Handle PRIVMSGs that trigger Commands +func (conn *Conn) h_PRIVMSG(line *Line) { + txt := line.Args[1] + if conn.CommandStripNick && strings.HasPrefix(txt, conn.Me.Nick) { + // Look for '^${nick}[:;>,-]? ' + l := len(conn.Me.Nick) + switch txt[l] { + case ':', ';', '>', ',', '-': + l++ + } + if txt[l] == ' ' { + txt = strings.TrimSpace(txt[l:]) + } + } + cmd, l := conn.cmdMatch(txt) + if cmd == nil { return } + if conn.CommandStripPrefix { + txt = strings.TrimSpace(txt[l:]) + } + if txt != line.Args[1] { + line = line.Copy() + line.Args[1] = txt + } + cmd.Execute(conn, line) +} + +func (conn *Conn) c_HELP(line *Line) { + if cmd, _ := conn.cmdMatch(line.Args[1]); cmd != nil { + conn.Privmsg(line.Args[0], cmd.Help()) + } +} diff --git a/client/handlers_test.go b/client/handlers_test.go index 1ea08b4..fcedbb2 100644 --- a/client/handlers_test.go +++ b/client/handlers_test.go @@ -2,8 +2,10 @@ package client import ( "code.google.com/p/gomock/gomock" + "fmt" "github.com/fluffle/goirc/state" "testing" + "time" ) // This test performs a simple end-to-end verification of correct line parsing @@ -11,14 +13,10 @@ import ( // in this file will call their respective handlers synchronously, otherwise // testing becomes more difficult. func TestPING(t *testing.T) { - c, s := setUp(t) + _, s := setUp(t) defer s.tearDown() - // As this is a real end-to-end test, we need a real end-to-end dispatcher. - c.ED = c.ER s.nc.Send("PING :1234567890") s.nc.Expect("PONG :1234567890") - // Return mock dispatcher to it's rightful place afterwards for tearDown. - c.ED = s.ed } // Test the handler for 001 / RPL_WELCOME @@ -27,9 +25,18 @@ func Test001(t *testing.T) { defer s.tearDown() l := parseLine(":irc.server.org 001 test :Welcome to IRC test!ident@somehost.com") - s.ed.EXPECT().Dispatch("connected", c, l) + // Set up a handler to detect whether connected handler is called from 001 + hcon := false + c.HandleFunc("connected", func (conn *Conn, line *Line) { + hcon = true + }) + // Call handler with a valid 001 line c.h_001(l) + <-time.After(time.Millisecond) + if !hcon { + t.Errorf("001 handler did not dispatch connected event.") + } // Check host parsed correctly if c.Me.Host != "somehost.com" { @@ -132,6 +139,58 @@ func TestCTCP(t *testing.T) { c.h_CTCP(parseLine(":blah!moo@cows.com PRIVMSG test :\001UNKNOWN ctcp\001")) } +func TestPRIVMSG(t *testing.T){ + c, s := setUp(t) + defer s.tearDown() + + f := func (conn *Conn, line *Line) { + conn.Privmsg(line.Args[0], line.Args[1]) + } + c.CommandFunc("prefix", f, "") + + // CommandStripNick and CommandStripPrefix are both false to begin + c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :prefix bar")) + s.nc.Expect("PRIVMSG #foo :prefix bar") + // If we're not stripping off the nick, the prefix won't match. + // This might be considered a bug, but then the library currently has a + // poor understanding of the concept of "being addressed". + c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :test: prefix bar")) + s.nc.ExpectNothing() + + c.CommandStripNick = true + c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :prefix bar")) + s.nc.Expect("PRIVMSG #foo :prefix bar") + c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :test: prefix bar")) + s.nc.Expect("PRIVMSG #foo :prefix bar") + + c.CommandStripPrefix = true + c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :prefix bar")) + s.nc.Expect("PRIVMSG #foo :bar") + c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :test: prefix bar")) + s.nc.Expect("PRIVMSG #foo :bar") + + c.CommandStripNick = false + c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :prefix bar")) + s.nc.Expect("PRIVMSG #foo :bar") + c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :test: prefix bar")) + s.nc.ExpectNothing() + + // Check the various nick addressing notations that are supported. + c.CommandStripNick = true + for _, addr := range []string{":", ";", ",", ">", "-", ""} { + c.h_PRIVMSG(parseLine(fmt.Sprintf( + ":blah!moo@cows.com PRIVMSG #foo :test%s prefix bar", addr))) + s.nc.Expect("PRIVMSG #foo :bar") + c.h_PRIVMSG(parseLine(fmt.Sprintf( + ":blah!moo@cows.com PRIVMSG #foo :test%sprefix bar", addr))) + s.nc.ExpectNothing() + } + c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :test! prefix bar")) + s.nc.ExpectNothing() + + +} + // Test the handler for JOIN messages func TestJOIN(t *testing.T) { c, s := setUp(t) diff --git a/client/mocknetconn_test.go b/client/mocknetconn_test.go index 76835f9..1159268 100644 --- a/client/mocknetconn_test.go +++ b/client/mocknetconn_test.go @@ -74,7 +74,7 @@ func (m *mockNetConn) Send(s string) { func (m *mockNetConn) Expect(e string) { select { - case <-time.After(1e6): + case <-time.After(time.Millisecond): m.Errorf("Mock connection did not receive expected output.\n\t"+ "Expected: '%s', got nothing.", e) case s := <-m.Out: @@ -88,7 +88,7 @@ func (m *mockNetConn) Expect(e string) { func (m *mockNetConn) ExpectNothing() { select { - case <-time.After(1e6): + case <-time.After(time.Millisecond): case s := <-m.Out: s = strings.Trim(s, "\r\n") m.Errorf("Mock connection received unexpected output.\n\t"+ diff --git a/client/state_handlers.go b/client/state_handlers.go index 34e87ee..ab2e60a 100644 --- a/client/state_handlers.go +++ b/client/state_handlers.go @@ -4,40 +4,37 @@ package client // to manage tracking state for an IRC connection import ( - "github.com/fluffle/goevent/event" "github.com/fluffle/golog/logging" "strings" ) -var stHandlers map[string]event.Handler - -func init() { - stHandlers = make(map[string]event.Handler) - stHandlers["JOIN"] = NewHandler((*Conn).h_JOIN) - stHandlers["KICK"] = NewHandler((*Conn).h_KICK) - stHandlers["MODE"] = NewHandler((*Conn).h_MODE) - stHandlers["NICK"] = NewHandler((*Conn).h_STNICK) - stHandlers["PART"] = NewHandler((*Conn).h_PART) - stHandlers["QUIT"] = NewHandler((*Conn).h_QUIT) - stHandlers["TOPIC"] = NewHandler((*Conn).h_TOPIC) - stHandlers["311"] = NewHandler((*Conn).h_311) - stHandlers["324"] = NewHandler((*Conn).h_324) - stHandlers["332"] = NewHandler((*Conn).h_332) - stHandlers["352"] = NewHandler((*Conn).h_352) - stHandlers["353"] = NewHandler((*Conn).h_353) - stHandlers["671"] = NewHandler((*Conn).h_671) +var stHandlers = map[string]HandlerFunc{ + "JOIN": (*Conn).h_JOIN, + "KICK": (*Conn).h_KICK, + "MODE": (*Conn).h_MODE, + "NICK": (*Conn).h_STNICK, + "PART": (*Conn).h_PART, + "QUIT": (*Conn).h_QUIT, + "TOPIC": (*Conn).h_TOPIC, + "311": (*Conn).h_311, + "324": (*Conn).h_324, + "332": (*Conn).h_332, + "352": (*Conn).h_352, + "353": (*Conn).h_353, + "671": (*Conn).h_671, } func (conn *Conn) addSTHandlers() { for n, h := range stHandlers { - conn.ER.AddHandler(h, n) + conn.stRemovers = append(conn.stRemovers, conn.Handle(n, h)) } } func (conn *Conn) delSTHandlers() { - for n, h := range stHandlers { - conn.ER.DelHandler(h, n) + for _, h := range conn.stRemovers { + h.Remove() } + conn.stRemovers = conn.stRemovers[:0] } // Handle NICK messages that need to update the state tracker