From f13e1ec7fe8e70336e8851f82b32bbaf44346288 Mon Sep 17 00:00:00 2001 From: "Christoffer G. Thomsen" Date: Wed, 25 Jul 2012 19:36:47 +0300 Subject: [PATCH 01/15] Inserted missing " --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b9d440e..c329aed 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ Synopsis: func(conn *irc.Conn, line *irc.Line) { conn.Join("#channel") }) // And a signal on disconnect quit := make(chan bool) - c.AddHandler("disconnected), + c.AddHandler("disconnected"), func(conn *irc.Conn, line *irc.Line) { quit <- true } // Tell client to connect From 8adfd40223d9a316079ed4bb5d334e53a94c91e7 Mon Sep 17 00:00:00 2001 From: "Christoffer G. Thomsen" Date: Wed, 25 Jul 2012 19:42:00 +0300 Subject: [PATCH 02/15] Fixed another typo --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index c329aed..5f22864 100644 --- a/README.md +++ b/README.md @@ -28,8 +28,8 @@ Synopsis: func(conn *irc.Conn, line *irc.Line) { conn.Join("#channel") }) // And a signal on disconnect quit := make(chan bool) - c.AddHandler("disconnected"), - func(conn *irc.Conn, line *irc.Line) { quit <- true } + c.AddHandler("disconnected", + func(conn *irc.Conn, line *irc.Line) { quit <- true }) // Tell client to connect if err := c.Connect("irc.freenode.net"); err != nil { From aa021c7cacdd901fe64ee9d944b78e0686587a66 Mon Sep 17 00:00:00 2001 From: Alex Bramley Date: Tue, 25 Sep 2012 23:42:37 +0100 Subject: [PATCH 03/15] Fix due to logging api change change. --- client/connection.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/connection.go b/client/connection.go index 55d81fd..9c7676e 100644 --- a/client/connection.go +++ b/client/connection.go @@ -65,7 +65,7 @@ type Conn struct { // that you can add event handlers to it. See AddHandler() for details. func SimpleClient(nick string, args ...string) *Conn { r := event.NewRegistry() - l := logging.DefaultLogger + l := logging.InitFromFlags() ident := "goirc" name := "Powered by GoIRC" From eb92a84e96061ffb82cc81d9530f45b031ef8122 Mon Sep 17 00:00:00 2001 From: StalkR Date: Sun, 6 Jan 2013 19:29:53 +0100 Subject: [PATCH 04/15] state: channel: add Nicks() and NicksStr() accessors for nicks on channel --- state/channel.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/state/channel.go b/state/channel.go index e25c4b9..2efa424 100644 --- a/state/channel.go +++ b/state/channel.go @@ -210,6 +210,24 @@ func (ch *Channel) ParseModes(modes string, modeargs ...string) { } } +// Nicks returns a list of *Nick that are on the channel. +func (ch *Channel) Nicks() []*Nick { + nicks := make([]*Nick, 0, len(ch.lookup)) + for _, nick := range ch.lookup { + nicks = append(nicks, nick) + } + return nicks +} + +// NicksStr returns a list of nick strings that are on the channel. +func (ch *Channel) NicksStr() []string { + nicks := make([]string, 0, len(ch.lookup)) + for _, nick := range ch.lookup { + nicks = append(nicks, nick.Nick) + } + return nicks +} + // Returns a string representing the channel. Looks like: // Channel: e.g. #moo // Topic: e.g. Discussing the merits of cows! From 4962b26ca93d1707c6c15b69f0cd04cc1f417438 Mon Sep 17 00:00:00 2001 From: StalkR Date: Sun, 6 Jan 2013 19:52:11 +0100 Subject: [PATCH 05/15] state: nick: support for Bot mode (+B) --- state/nick.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/state/nick.go b/state/nick.go index 457bec1..d8f103c 100644 --- a/state/nick.go +++ b/state/nick.go @@ -20,13 +20,14 @@ type Nick struct { // This is only really useful for me, as we can't see other people's modes // without IRC operator privileges (and even then only on some IRCd's). type NickMode struct { - // MODE +i, +o, +w, +x, +z - Invisible, Oper, WallOps, HiddenHost, SSL bool + // MODE +B, +i, +o, +w, +x, +z + Bot, Invisible, Oper, WallOps, HiddenHost, SSL bool } // Map *irc.NickMode fields to IRC mode characters and vice versa var StringToNickMode = map[string]string{} var NickModeToString = map[string]string{ + "Bot": "B", "Invisible": "i", "Oper": "o", "WallOps": "w", @@ -94,6 +95,8 @@ func (nk *Nick) ParseModes(modes string) { modeop = true case '-': modeop = false + case 'B': + nk.Modes.Bot = modeop case 'i': nk.Modes.Invisible = modeop case 'o': From 2cc4e94acb1d6d8f7d389981b734463d05a947fa Mon Sep 17 00:00:00 2001 From: StalkR Date: Sun, 6 Jan 2013 20:13:06 +0100 Subject: [PATCH 06/15] state: nick: add Channels/ChannelsStr accessors to get channels a nick is on --- state/nick.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/state/nick.go b/state/nick.go index 457bec1..aad0988 100644 --- a/state/nick.go +++ b/state/nick.go @@ -110,6 +110,24 @@ func (nk *Nick) ParseModes(modes string) { } } +// Channels returns a list of *Channel the nick is on. +func (nk *Nick) Channels() []*Channel { + channels := make([]*Channel, 0, len(nk.lookup)) + for _, channel := range nk.lookup { + channels = append(channels, channel) + } + return channels +} + +// ChannelsStr returns a list of channel strings the nick is on. +func (nk *Nick) ChannelsStr() []string { + channels := make([]string, 0, len(nk.lookup)) + for _, channel := range nk.lookup { + channels = append(channels, channel.Name) + } + return channels +} + // Returns a string representing the nick. Looks like: // Nick: e.g. CowMaster // Hostmask: e.g. moo@cows.org From b39e4717afbd4a48ee2a4066a0cf74764009b93e Mon Sep 17 00:00:00 2001 From: Alex Bramley Date: Sun, 6 Jan 2013 21:01:55 +0000 Subject: [PATCH 07/15] Allow renicking to be customised. (Closes #14) --- client/connection.go | 4 ++++ client/handlers.go | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/client/connection.go b/client/connection.go index 9c7676e..0b3f3ad 100644 --- a/client/connection.go +++ b/client/connection.go @@ -20,6 +20,9 @@ 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 @@ -97,6 +100,7 @@ func Client(nick, ident, name string, SSLConfig: nil, PingFreq: 3 * time.Minute, Flood: false, + NewNick: func(s string) string { return s + "_" }, badness: 0, lastsent: time.Now(), } diff --git a/client/handlers.go b/client/handlers.go index 2624a71..72a72c6 100644 --- a/client/handlers.go +++ b/client/handlers.go @@ -79,7 +79,7 @@ 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 - neu := line.Args[1] + "_" + neu := conn.NewNick(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 From 68005e184580b7a7466983da30445c0d4bbd6905 Mon Sep 17 00:00:00 2001 From: StalkR Date: Thu, 10 Jan 2013 22:22:19 +0100 Subject: [PATCH 08/15] state: parse +r/+Z channel modes --- state/channel.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/state/channel.go b/state/channel.go index 2efa424..c87a00c 100644 --- a/state/channel.go +++ b/state/channel.go @@ -18,6 +18,7 @@ type Channel struct { // A struct representing the modes of an IRC Channel // (the ones we care about, at least). +// http://www.unrealircd.com/files/docs/unreal32docs.html#userchannelmodes type ChanMode struct { // MODE +p, +s, +t, +n, +m Private, Secret, ProtectedTopic, NoExternalMsg, Moderated bool @@ -25,6 +26,9 @@ type ChanMode struct { // MODE +i, +O, +z InviteOnly, OperOnly, SSLOnly bool + // MODE +r, +Z + Registered, AllSSL bool + // MODE +k Key string @@ -49,6 +53,8 @@ var ChanModeToString = map[string]string{ "InviteOnly": "i", "OperOnly": "O", "SSLOnly": "z", + "Registered": "r", + "AllSSL": "Z", "Key": "k", "Limit": "l", } @@ -152,12 +158,16 @@ func (ch *Channel) ParseModes(modes string, modeargs ...string) { ch.Modes.NoExternalMsg = modeop case 'p': ch.Modes.Private = modeop + case 'r': + ch.Modes.Registered = modeop case 's': ch.Modes.Secret = modeop case 't': ch.Modes.ProtectedTopic = modeop case 'z': ch.Modes.SSLOnly = modeop + case 'Z': + ch.Modes.AllSSL = modeop case 'O': ch.Modes.OperOnly = modeop case 'k': From ca46884c720169b1b31f421061f00e008402040b Mon Sep 17 00:00:00 2001 From: Alex Bramley Date: Wed, 23 Jan 2013 22:33:01 +0000 Subject: [PATCH 09/15] Remove embedded logger from state tracker. Hmmmm. --- state/channel.go | 18 +++--- state/channel_test.go | 57 +++-------------- state/nick.go | 10 ++- state/nick_test.go | 47 +++----------- state/tracker.go | 46 +++++++------- state/tracker_test.go | 139 ++++++++++-------------------------------- 6 files changed, 83 insertions(+), 234 deletions(-) diff --git a/state/channel.go b/state/channel.go index c87a00c..5067348 100644 --- a/state/channel.go +++ b/state/channel.go @@ -13,7 +13,6 @@ type Channel struct { Modes *ChanMode lookup map[string]*Nick nicks map[*Nick]*ChanPrivs - l logging.Logger } // A struct representing the modes of an IRC Channel @@ -97,13 +96,12 @@ func init() { * Channel methods for state management \******************************************************************************/ -func NewChannel(name string, l logging.Logger) *Channel { +func NewChannel(name string) *Channel { return &Channel{ Name: name, Modes: new(ChanMode), nicks: make(map[*Nick]*ChanPrivs), lookup: make(map[string]*Nick), - l: l, } } @@ -124,7 +122,7 @@ func (ch *Channel) addNick(nk *Nick, cp *ChanPrivs) { ch.nicks[nk] = cp ch.lookup[nk.Nick] = nk } else { - ch.l.Warn("Channel.addNick(): %s already on %s.", nk.Nick, ch.Name) + logging.Warn("Channel.addNick(): %s already on %s.", nk.Nick, ch.Name) } } @@ -134,7 +132,7 @@ func (ch *Channel) delNick(nk *Nick) { delete(ch.nicks, nk) delete(ch.lookup, nk.Nick) } else { - ch.l.Warn("Channel.delNick(): %s not on %s.", nk.Nick, ch.Name) + logging.Warn("Channel.delNick(): %s not on %s.", nk.Nick, ch.Name) } } @@ -176,7 +174,7 @@ func (ch *Channel) ParseModes(modes string, modeargs ...string) { } else if !modeop { ch.Modes.Key = "" } else { - ch.l.Warn("Channel.ParseModes(): not enough arguments to "+ + logging.Warn("Channel.ParseModes(): not enough arguments to "+ "process MODE %s %s%c", ch.Name, modestr, m) } case 'l': @@ -186,7 +184,7 @@ func (ch *Channel) ParseModes(modes string, modeargs ...string) { } else if !modeop { ch.Modes.Limit = 0 } else { - ch.l.Warn("Channel.ParseModes(): not enough arguments to "+ + logging.Warn("Channel.ParseModes(): not enough arguments to "+ "process MODE %s %s%c", ch.Name, modestr, m) } case 'q', 'a', 'o', 'h', 'v': @@ -207,15 +205,15 @@ func (ch *Channel) ParseModes(modes string, modeargs ...string) { } modeargs = modeargs[1:] } else { - ch.l.Warn("Channel.ParseModes(): untracked nick %s "+ + logging.Warn("Channel.ParseModes(): untracked nick %s "+ "received MODE on channel %s", modeargs[0], ch.Name) } } else { - ch.l.Warn("Channel.ParseModes(): not enough arguments to "+ + logging.Warn("Channel.ParseModes(): not enough arguments to "+ "process MODE %s %s%c", ch.Name, modestr, m) } default: - ch.l.Info("Channel.ParseModes(): unknown mode char %c", m) + logging.Info("Channel.ParseModes(): unknown mode char %c", m) } } } diff --git a/state/channel_test.go b/state/channel_test.go index 48c8124..f714044 100644 --- a/state/channel_test.go +++ b/state/channel_test.go @@ -5,12 +5,9 @@ import ( ) func TestNewChannel(t *testing.T) { - _, s := setUp(t) - defer s.tearDown() + ch := NewChannel("#test1") - ch := NewChannel("#test1", s.log) - - if ch.Name != "#test1" || ch.l != s.log { + if ch.Name != "#test1" { t.Errorf("Channel not created correctly by NewChannel()") } if len(ch.nicks) != 0 || len(ch.lookup) != 0 { @@ -19,11 +16,8 @@ func TestNewChannel(t *testing.T) { } func TestAddNick(t *testing.T) { - _, s := setUp(t) - defer s.tearDown() - - ch := NewChannel("#test1", s.log) - nk := NewNick("test1", s.log) + ch := NewChannel("#test1") + nk := NewNick("test1") cp := new(ChanPrivs) ch.addNick(nk, cp) @@ -37,25 +31,13 @@ func TestAddNick(t *testing.T) { if n, ok := ch.lookup["test1"]; !ok || n != nk { t.Errorf("Nick test1 not properly stored in lookup map.") } - - s.log.EXPECT().Warn("Channel.addNick(): %s already on %s.", - "test1", "#test1") - ch.addNick(nk, cp) } func TestDelNick(t *testing.T) { - _, s := setUp(t) - defer s.tearDown() - - ch := NewChannel("#test1", s.log) - nk := NewNick("test1", s.log) + ch := NewChannel("#test1") + nk := NewNick("test1") cp := new(ChanPrivs) - // Testing the error state first is easier - s.log.EXPECT().Warn("Channel.delNick(): %s not on %s.", - "test1", "#test1") - ch.delNick(nk) - ch.addNick(nk, cp) ch.delNick(nk) if len(ch.nicks) != 0 || len(ch.lookup) != 0 { @@ -70,14 +52,11 @@ func TestDelNick(t *testing.T) { } func TestChannelParseModes(t *testing.T) { - _, s := setUp(t) - defer s.tearDown() - - ch := NewChannel("#test1", s.log) + ch := NewChannel("#test1") md := ch.Modes // Channel modes can adjust channel privs too, so we need a Nick - nk := NewNick("test1", s.log) + nk := NewNick("test1") cp := new(ChanPrivs) ch.addNick(nk, cp) @@ -111,9 +90,7 @@ func TestChannelParseModes(t *testing.T) { t.Errorf("Limit for channel not set correctly") } - // enable limit incorrectly. see nick_test.go for why the byte() cast. - s.log.EXPECT().Warn("Channel.ParseModes(): not enough arguments to "+ - "process MODE %s %s%c", "#test1", "+", byte('l')) + // enable limit incorrectly ch.ParseModes("+l") if md.Limit != 256 { t.Errorf("Bad limit value caused limit to be unset.") @@ -137,8 +114,6 @@ func TestChannelParseModes(t *testing.T) { } // enable key incorrectly - s.log.EXPECT().Warn("Channel.ParseModes(): not enough arguments to "+ - "process MODE %s %s%c", "#test1", "+", byte('k')) ch.ParseModes("+k") if md.Key != "foobar" { t.Errorf("Bad key value caused key to be unset.") @@ -159,18 +134,8 @@ func TestChannelParseModes(t *testing.T) { t.Errorf("Channel privileges not flipped correctly by ParseModes.") } - s.log.EXPECT().Warn("Channel.ParseModes(): untracked nick %s "+ - "received MODE on channel %s", "test2", "#test1") - ch.ParseModes("+v", "test2") - - s.log.EXPECT().Warn("Channel.ParseModes(): not enough arguments to "+ - "process MODE %s %s%c", "#test1", "-", byte('v')) - ch.ParseModes("-v") - // Test a random mix of modes, just to be sure md.Limit = 256 - s.log.EXPECT().Warn("Channel.ParseModes(): not enough arguments to "+ - "process MODE %s %s%c", "#test1", "-", byte('h')) ch.ParseModes("+zpt-qsl+kv-h", "test1", "foobar", "test1") if !md.Private || md.Secret || !md.ProtectedTopic || !md.NoExternalMsg || @@ -184,8 +149,4 @@ func TestChannelParseModes(t *testing.T) { // NOTE: HalfOp not actually unset above thanks to deliberate error. t.Errorf("Channel privileges not flipped correctly by ParseModes (2).") } - - // Finally, check we get an info log for an unrecognised mode character - s.log.EXPECT().Info("Channel.ParseModes(): unknown mode char %c", byte('d')) - ch.ParseModes("+d") } diff --git a/state/nick.go b/state/nick.go index 725e698..96a0b48 100644 --- a/state/nick.go +++ b/state/nick.go @@ -11,7 +11,6 @@ type Nick struct { Modes *NickMode lookup map[string]*Channel chans map[*Channel]*ChanPrivs - l logging.Logger } // A struct representing the modes of an IRC Nick (User Modes) @@ -45,13 +44,12 @@ func init() { * Nick methods for state management \******************************************************************************/ -func NewNick(n string, l logging.Logger) *Nick { +func NewNick(n string) *Nick { return &Nick{ Nick: n, Modes: new(NickMode), chans: make(map[*Channel]*ChanPrivs), lookup: make(map[string]*Channel), - l: l, } } @@ -72,7 +70,7 @@ func (nk *Nick) addChannel(ch *Channel, cp *ChanPrivs) { nk.chans[ch] = cp nk.lookup[ch.Name] = ch } else { - nk.l.Warn("Nick.addChannel(): %s already on %s.", nk.Nick, ch.Name) + logging.Warn("Nick.addChannel(): %s already on %s.", nk.Nick, ch.Name) } } @@ -82,7 +80,7 @@ func (nk *Nick) delChannel(ch *Channel) { delete(nk.chans, ch) delete(nk.lookup, ch.Name) } else { - nk.l.Warn("Nick.delChannel(): %s not on %s.", nk.Nick, ch.Name) + logging.Warn("Nick.delChannel(): %s not on %s.", nk.Nick, ch.Name) } } @@ -108,7 +106,7 @@ func (nk *Nick) ParseModes(modes string) { case 'z': nk.Modes.SSL = modeop default: - nk.l.Info("Nick.ParseModes(): unknown mode char %c", m) + logging.Info("Nick.ParseModes(): unknown mode char %c", m) } } } diff --git a/state/nick_test.go b/state/nick_test.go index 283f908..c01ec1a 100644 --- a/state/nick_test.go +++ b/state/nick_test.go @@ -5,12 +5,9 @@ import ( ) func TestNewNick(t *testing.T) { - _, s := setUp(t) - defer s.tearDown() + nk := NewNick("test1") - nk := NewNick("test1", s.log) - - if nk.Nick != "test1" || nk.l != s.log { + if nk.Nick != "test1" { t.Errorf("Nick not created correctly by NewNick()") } if len(nk.chans) != 0 || len(nk.lookup) != 0 { @@ -19,11 +16,8 @@ func TestNewNick(t *testing.T) { } func TestAddChannel(t *testing.T) { - _, s := setUp(t) - defer s.tearDown() - - nk := NewNick("test1", s.log) - ch := NewChannel("#test1", s.log) + nk := NewNick("test1") + ch := NewChannel("#test1") cp := new(ChanPrivs) nk.addChannel(ch, cp) @@ -37,24 +31,13 @@ func TestAddChannel(t *testing.T) { if c, ok := nk.lookup["#test1"]; !ok || c != ch { t.Errorf("Channel #test1 not properly stored in lookup map.") } - - s.log.EXPECT().Warn("Nick.addChannel(): %s already on %s.", - "test1", "#test1") - nk.addChannel(ch, cp) } func TestDelChannel(t *testing.T) { - _, s := setUp(t) - defer s.tearDown() - - nk := NewNick("test1", s.log) - ch := NewChannel("#test1", s.log) + nk := NewNick("test1") + ch := NewChannel("#test1") cp := new(ChanPrivs) - // Testing the error state first is easier - s.log.EXPECT().Warn("Nick.delChannel(): %s not on %s.", "test1", "#test1") - nk.delChannel(ch) - nk.addChannel(ch, cp) nk.delChannel(ch) if len(nk.chans) != 0 || len(nk.lookup) != 0 { @@ -69,10 +52,7 @@ func TestDelChannel(t *testing.T) { } func TestNickParseModes(t *testing.T) { - _, s := setUp(t) - defer s.tearDown() - - nk := NewNick("test1", s.log) + nk := NewNick("test1") md := nk.Modes // Modes should all be false for a new nick @@ -90,17 +70,4 @@ func TestNickParseModes(t *testing.T) { if !md.Invisible || md.Oper || !md.WallOps || md.HiddenHost || !md.SSL { t.Errorf("Modes not flipped correctly by ParseModes.") } - - // Check that passing an unknown mode char results in an info log - // The cast to byte here is needed to pass; gomock uses reflect.DeepEqual - // to examine argument equality, but 'd' (when not implicitly coerced to a - // uint8 by the type system) is an int, whereas string("+d")[1] is not. - // This type difference (despite the values being nominally the same) - // causes the test to fail with the following confusing error. - // - // no matching expected call: *logging.MockLogger.Info([Nick.ParseModes(): unknown mode char %c [100]]) - // missing call(s) to *logging.MockLogger.Info(is equal to Nick.ParseModes(): unknown mode char %c, is equal to [100]) - - s.log.EXPECT().Info("Nick.ParseModes(): unknown mode char %c", byte('d')) - nk.ParseModes("+d") } diff --git a/state/tracker.go b/state/tracker.go index d168b78..fb29cd2 100644 --- a/state/tracker.go +++ b/state/tracker.go @@ -35,17 +35,13 @@ type stateTracker struct { // We need to keep state on who we are :-) me *Nick - - // For great logging justice. - l logging.Logger } // ... and a constructor to make it ... -func NewTracker(mynick string, l logging.Logger) *stateTracker { +func NewTracker(mynick string) *stateTracker { st := &stateTracker{ chans: make(map[string]*Channel), nicks: make(map[string]*Nick), - l: l, } st.me = st.NewNick(mynick) return st @@ -67,10 +63,10 @@ func (st *stateTracker) Wipe() { // can be properly tracked for state management purposes. func (st *stateTracker) NewNick(n string) *Nick { if _, ok := st.nicks[n]; ok { - st.l.Warn("StateTracker.NewNick(): %s already tracked.", n) + logging.Warn("StateTracker.NewNick(): %s already tracked.", n) return nil } - st.nicks[n] = NewNick(n, st.l) + st.nicks[n] = NewNick(n) return st.nicks[n] } @@ -97,10 +93,10 @@ func (st *stateTracker) ReNick(old, neu string) { ch.lookup[neu] = nk } } else { - st.l.Warn("StateTracker.ReNick(): %s already exists.", neu) + logging.Warn("StateTracker.ReNick(): %s already exists.", neu) } } else { - st.l.Warn("StateTracker.ReNick(): %s not tracked.", old) + logging.Warn("StateTracker.ReNick(): %s not tracked.", old) } } @@ -110,17 +106,17 @@ func (st *stateTracker) DelNick(n string) { if nk != st.me { st.delNick(nk) } else { - st.l.Warn("StateTracker.DelNick(): won't delete myself.") + logging.Warn("StateTracker.DelNick(): won't delete myself.") } } else { - st.l.Warn("StateTracker.DelNick(): %s not tracked.", n) + logging.Warn("StateTracker.DelNick(): %s not tracked.", n) } } func (st *stateTracker) delNick(nk *Nick) { if nk == st.me { // Shouldn't get here => internal state tracking code is fubar. - st.l.Error("StateTracker.DelNick(): TRYING TO DELETE ME :-(") + logging.Error("StateTracker.DelNick(): TRYING TO DELETE ME :-(") return } delete(st.nicks, nk.Nick) @@ -130,7 +126,7 @@ func (st *stateTracker) delNick(nk *Nick) { if len(ch.nicks) == 0 { // Deleting a nick from tracking shouldn't empty any channels as // *we* should be on the channel with them to be tracking them. - st.l.Error("StateTracker.delNick(): deleting nick %s emptied "+ + logging.Error("StateTracker.delNick(): deleting nick %s emptied "+ "channel %s, this shouldn't happen!", nk.Nick, ch.Name) } } @@ -140,10 +136,10 @@ func (st *stateTracker) delNick(nk *Nick) { // can be properly tracked for state management purposes. func (st *stateTracker) NewChannel(c string) *Channel { if _, ok := st.chans[c]; ok { - st.l.Warn("StateTracker.NewChannel(): %s already tracked.", c) + logging.Warn("StateTracker.NewChannel(): %s already tracked.", c) return nil } - st.chans[c] = NewChannel(c, st.l) + st.chans[c] = NewChannel(c) return st.chans[c] } @@ -160,7 +156,7 @@ func (st *stateTracker) DelChannel(c string) { if ch, ok := st.chans[c]; ok { st.delChannel(ch) } else { - st.l.Warn("StateTracker.DelChannel(): %s not tracked.", c) + logging.Warn("StateTracker.DelChannel(): %s not tracked.", c) } } @@ -195,19 +191,21 @@ func (st *stateTracker) IsOn(c, n string) (*ChanPrivs, bool) { // Associates an already known nick with an already known channel. func (st *stateTracker) Associate(ch *Channel, nk *Nick) *ChanPrivs { if ch == nil || nk == nil { - st.l.Error("StateTracker.Associate(): passed nil values :-(") + logging.Error("StateTracker.Associate(): passed nil values :-(") return nil } else if _ch, ok := st.chans[ch.Name]; !ok || ch != _ch { // As we can implicitly delete both nicks and channels from being // tracked by dissociating one from the other, we should verify that // we're not being passed an old Nick or Channel. - st.l.Error("StateTracker.Associate(): channel %s not found in "+ + logging.Error("StateTracker.Associate(): channel %s not found in "+ "(or differs from) internal state.", ch.Name) + return nil } else if _nk, ok := st.nicks[nk.Nick]; !ok || nk != _nk { - st.l.Error("StateTracker.Associate(): nick %s not found in "+ + logging.Error("StateTracker.Associate(): nick %s not found in "+ "(or differs from) internal state.", nk.Nick) + return nil } else if _, ok := nk.IsOn(ch); ok { - st.l.Warn("StateTracker.Associate(): %s already on %s.", + logging.Warn("StateTracker.Associate(): %s already on %s.", nk.Nick, ch.Name) return nil } @@ -222,18 +220,18 @@ func (st *stateTracker) Associate(ch *Channel, nk *Nick) *ChanPrivs { // any common channels with, and channels we're no longer on. func (st *stateTracker) Dissociate(ch *Channel, nk *Nick) { if ch == nil || nk == nil { - st.l.Error("StateTracker.Dissociate(): passed nil values :-(") + logging.Error("StateTracker.Dissociate(): passed nil values :-(") } else if _ch, ok := st.chans[ch.Name]; !ok || ch != _ch { // As we can implicitly delete both nicks and channels from being // tracked by dissociating one from the other, we should verify that // we're not being passed an old Nick or Channel. - st.l.Error("StateTracker.Dissociate(): channel %s not found in "+ + logging.Error("StateTracker.Dissociate(): channel %s not found in "+ "(or differs from) internal state.", ch.Name) } else if _nk, ok := st.nicks[nk.Nick]; !ok || nk != _nk { - st.l.Error("StateTracker.Dissociate(): nick %s not found in "+ + logging.Error("StateTracker.Dissociate(): nick %s not found in "+ "(or differs from) internal state.", nk.Nick) } else if _, ok := nk.IsOn(ch); !ok { - st.l.Warn("StateTracker.Dissociate(): %s not on %s.", + logging.Warn("StateTracker.Dissociate(): %s not on %s.", nk.Nick, ch.Name) } else if nk == st.me { // I'm leaving the channel for some reason, so it won't be tracked. diff --git a/state/tracker_test.go b/state/tracker_test.go index e68273d..e6f3584 100644 --- a/state/tracker_test.go +++ b/state/tracker_test.go @@ -1,33 +1,19 @@ package state import ( - "code.google.com/p/gomock/gomock" "github.com/fluffle/golog/logging" "testing" ) -type testState struct { - ctrl *gomock.Controller - log *logging.MockLogger -} - -func setUp(t *testing.T) (*stateTracker, *testState) { - ctrl := gomock.NewController(t) - log := logging.NewMockLogger(ctrl) - return NewTracker("mynick", log), &testState{ctrl, log} -} - -func (s *testState) tearDown() { - s.ctrl.Finish() +func init() { + // This is probably a dirty hack... + logging.InitFromFlags() + logging.SetLogLevel(logging.LogFatal) } func TestSTNewTracker(t *testing.T) { - st, s := setUp(t) - defer s.tearDown() + st := NewTracker("mynick") - if st.l != s.log { - t.Errorf("State tracker's logger not set correctly.") - } if len(st.nicks) != 1 { t.Errorf("Nick list of new tracker is not 1 (me!).") } @@ -40,28 +26,23 @@ func TestSTNewTracker(t *testing.T) { } func TestSTNewNick(t *testing.T) { - st, s := setUp(t) - defer s.tearDown() - + st := NewTracker("mynick") test1 := st.NewNick("test1") - if test1 == nil || test1.Nick != "test1" || test1.l != s.log { + if test1 == nil || test1.Nick != "test1" { t.Errorf("Nick object created incorrectly by NewNick.") } if n, ok := st.nicks["test1"]; !ok || n != test1 || len(st.nicks) != 2 { t.Errorf("Nick object stored incorrectly by NewNick.") } - s.log.EXPECT().Warn("StateTracker.NewNick(): %s already tracked.", "test1") if fail := st.NewNick("test1"); fail != nil { t.Errorf("Creating duplicate nick did not produce nil return.") } } func TestSTGetNick(t *testing.T) { - st, s := setUp(t) - defer s.tearDown() - + st := NewTracker("mynick") test1 := st.NewNick("test1") if n := st.GetNick("test1"); n != test1 { @@ -76,9 +57,7 @@ func TestSTGetNick(t *testing.T) { } func TestSTReNick(t *testing.T) { - st, s := setUp(t) - defer s.tearDown() - + st := NewTracker("mynick") test1 := st.NewNick("test1") // This channel is here to ensure that its lookup map gets updated @@ -107,8 +86,6 @@ func TestSTReNick(t *testing.T) { } test2 := st.NewNick("test1") - - s.log.EXPECT().Warn("StateTracker.ReNick(): %s already exists.", "test2") st.ReNick("test1", "test2") if n, ok := st.nicks["test2"]; !ok || n != test1 { @@ -120,14 +97,10 @@ func TestSTReNick(t *testing.T) { if len(st.nicks) != 3 { t.Errorf("Nick list changed size during ReNick.") } - - s.log.EXPECT().Warn("StateTracker.ReNick(): %s not tracked.", "test3") - st.ReNick("test3", "test2") } func TestSTDelNick(t *testing.T) { - st, s := setUp(t) - defer s.tearDown() + st := NewTracker("mynick") st.NewNick("test1") st.DelNick("test1") @@ -142,17 +115,13 @@ func TestSTDelNick(t *testing.T) { // Deleting unknown nick shouldn't work, but let's make sure we have a // known nick first to catch any possible accidental removals. nick1 := st.NewNick("test1") - - s.log.EXPECT().Warn("StateTracker.DelNick(): %s not tracked.", "test2") st.DelNick("test2") if len(st.nicks) != 2 { t.Errorf("Deleting unknown nick had unexpected side-effects.") } // Deleting my nick shouldn't work - s.log.EXPECT().Warn("StateTracker.DelNick(): won't delete myself.") st.DelNick("mynick") - if len(st.nicks) != 2 { t.Errorf("Deleting myself had unexpected side-effects.") } @@ -192,8 +161,7 @@ func TestSTDelNick(t *testing.T) { } func TestSTNewChannel(t *testing.T) { - st, s := setUp(t) - defer s.tearDown() + st := NewTracker("mynick") if len(st.chans) != 0 { t.Errorf("Channel list of new tracker is non-zero length.") @@ -201,22 +169,20 @@ func TestSTNewChannel(t *testing.T) { test1 := st.NewChannel("#test1") - if test1 == nil || test1.Name != "#test1" || test1.l != s.log { + if test1 == nil || test1.Name != "#test1" { t.Errorf("Channel object created incorrectly by NewChannel.") } if c, ok := st.chans["#test1"]; !ok || c != test1 || len(st.chans) != 1 { t.Errorf("Channel object stored incorrectly by NewChannel.") } - s.log.EXPECT().Warn("StateTracker.NewChannel(): %s already tracked.", "#test1") if fail := st.NewChannel("#test1"); fail != nil { t.Errorf("Creating duplicate chan did not produce nil return.") } } func TestSTGetChannel(t *testing.T) { - st, s := setUp(t) - defer s.tearDown() + st := NewTracker("mynick") test1 := st.NewChannel("#test1") @@ -232,8 +198,7 @@ func TestSTGetChannel(t *testing.T) { } func TestSTDelChannel(t *testing.T) { - st, s := setUp(t) - defer s.tearDown() + st := NewTracker("mynick") st.NewChannel("#test1") st.DelChannel("#test1") @@ -248,10 +213,7 @@ func TestSTDelChannel(t *testing.T) { // Deleting unknown nick shouldn't work, but let's make sure we have a // known nick first to catch any possible accidental removals. chan1 := st.NewChannel("#test1") - - s.log.EXPECT().Warn("StateTracker.DelChannel(): %s not tracked.", "#test2") st.DelChannel("#test2") - if len(st.chans) != 1 { t.Errorf("DelChannel had unexpected side-effects.") } @@ -306,8 +268,7 @@ func TestSTDelChannel(t *testing.T) { } func TestSTIsOn(t *testing.T) { - st, s := setUp(t) - defer s.tearDown() + st := NewTracker("mynick") nick1 := st.NewNick("test1") chan1 := st.NewChannel("#test1") @@ -322,8 +283,7 @@ func TestSTIsOn(t *testing.T) { } func TestSTAssociate(t *testing.T) { - st, s := setUp(t) - defer s.tearDown() + st := NewTracker("mynick") nick1 := st.NewNick("test1") chan1 := st.NewChannel("#test1") @@ -337,32 +297,25 @@ func TestSTAssociate(t *testing.T) { } // Test error cases - s.log.EXPECT().Error("StateTracker.Associate(): passed nil values :-(") - st.Associate(nil, nick1) - - s.log.EXPECT().Error("StateTracker.Associate(): passed nil values :-(") - st.Associate(chan1, nil) - - s.log.EXPECT().Warn("StateTracker.Associate(): %s already on %s.", - "test1", "#test1") - st.Associate(chan1, nick1) - - // nick2 deliberately created directly here. - nick2 := NewNick("test2", s.log) - s.log.EXPECT().Error("StateTracker.Associate(): nick %s not found in "+ - "(or differs from) internal state.", "test2") - st.Associate(chan1, nick2) - - // chan2 deliberately created directly here. - chan2 := NewChannel("#test2", s.log) - s.log.EXPECT().Error("StateTracker.Associate(): channel %s not found in "+ - "(or differs from) internal state.", "#test2") - st.Associate(chan2, nick1) + if st.Associate(nil, nick1) != nil { + t.Errorf("Associating nil *Channel did not return nil.") + } + if st.Associate(chan1, nil) != nil { + t.Errorf("Associating nil *Nick did not return nil.") + } + if st.Associate(chan1, nick1) != nil { + t.Errorf("Associating already-associated things did not return nil.") + } + if st.Associate(chan1, NewNick("test2")) != nil { + t.Errorf("Associating unknown *Nick did not return nil.") + } + if st.Associate(NewChannel("#test2"), nick1) != nil { + t.Errorf("Associating unknown *Channel did not return nil.") + } } func TestSTDissociate(t *testing.T) { - st, s := setUp(t) - defer s.tearDown() + st := NewTracker("mynick") nick1 := st.NewNick("test1") chan1 := st.NewChannel("#test1") @@ -417,36 +370,10 @@ func TestSTDissociate(t *testing.T) { len(st.me.chans) != 2 || len(nick1.chans) != 0 || len(st.chans) != 2 { t.Errorf("Dissociating a nick from it's last channel went wrong.") } - - // Check error cases - // test1 was deleted above, so "re-track" it for this test. - nick1 = st.NewNick("test1") - s.log.EXPECT().Warn("StateTracker.Dissociate(): %s not on %s.", - "test1", "#test1") - st.Dissociate(chan1, nick1) - - s.log.EXPECT().Error("StateTracker.Dissociate(): passed nil values :-(") - st.Dissociate(chan1, nil) - - s.log.EXPECT().Error("StateTracker.Dissociate(): passed nil values :-(") - st.Dissociate(nil, nick1) - - // nick3 deliberately created directly here. - nick3 := NewNick("test3", s.log) - s.log.EXPECT().Error("StateTracker.Dissociate(): nick %s not found in "+ - "(or differs from) internal state.", "test3") - st.Dissociate(chan1, nick3) - - // chan3 deliberately created directly here. - chan3 := NewChannel("#test3", s.log) - s.log.EXPECT().Error("StateTracker.Dissociate(): channel %s not found in "+ - "(or differs from) internal state.", "#test3") - st.Dissociate(chan3, nick1) } func TestSTWipe(t *testing.T) { - st, s := setUp(t) - defer s.tearDown() + st := NewTracker("mynick") nick1 := st.NewNick("test1") nick2 := st.NewNick("test2") From a038856094b56bfba4c3896edae78a9a0d45e5ce Mon Sep 17 00:00:00 2001 From: Alex Bramley Date: Sat, 16 Feb 2013 00:11:39 +0000 Subject: [PATCH 10/15] Remove embedded logger from client package. --- client/connection.go | 37 +++++++--------- client/connection_test.go | 26 +++-------- client/handlers_test.go | 90 ++++++++++++--------------------------- client/state_handlers.go | 21 ++++----- 4 files changed, 59 insertions(+), 115 deletions(-) diff --git a/client/connection.go b/client/connection.go index 0b3f3ad..112767f 100644 --- a/client/connection.go +++ b/client/connection.go @@ -31,9 +31,6 @@ type Conn struct { 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 ;-) State interface{} @@ -68,7 +65,6 @@ type Conn struct { // that you can add event handlers to it. See AddHandler() for details. func SimpleClient(nick string, args ...string) *Conn { r := event.NewRegistry() - l := logging.InitFromFlags() ident := "goirc" name := "Powered by GoIRC" @@ -78,18 +74,17 @@ func SimpleClient(nick string, args ...string) *Conn { if len(args) > 1 && args[1] != "" { name = args[1] } - return Client(nick, ident, name, r, l) + return Client(nick, ident, name, r) } -func Client(nick, ident, name string, - r event.EventRegistry, l logging.Logger) *Conn { - if r == nil || l == nil { +func Client(nick, ident, name string, r event.EventRegistry) *Conn { + if r == nil { return nil } + logging.InitFromFlags() conn := &Conn{ ER: r, ED: r, - l: l, st: false, in: make(chan *Line, 32), out: make(chan string, 32), @@ -105,7 +100,7 @@ func Client(nick, ident, name string, lastsent: time.Now(), } conn.addIntHandlers() - conn.Me = state.NewNick(nick, l) + conn.Me = state.NewNick(nick) conn.Me.Ident = ident conn.Me.Name = name @@ -116,7 +111,7 @@ func Client(nick, ident, name string, func (conn *Conn) EnableStateTracking() { if !conn.st { n := conn.Me - conn.ST = state.NewTracker(n.Nick, conn.l) + conn.ST = state.NewTracker(n.Nick) conn.Me = conn.ST.Me() conn.Me.Ident = n.Ident conn.Me.Name = n.Name @@ -159,7 +154,7 @@ func (conn *Conn) Connect(host string, pass ...string) error { if !hasPort(host) { host += ":6697" } - conn.l.Info("irc.Connect(): Connecting to %s with SSL.", host) + logging.Info("irc.Connect(): Connecting to %s with SSL.", host) if s, err := tls.Dial("tcp", host, conn.SSLConfig); err == nil { conn.sock = s } else { @@ -169,7 +164,7 @@ func (conn *Conn) Connect(host string, pass ...string) error { if !hasPort(host) { host += ":6667" } - conn.l.Info("irc.Connect(): Connecting to %s without SSL.", host) + logging.Info("irc.Connect(): Connecting to %s without SSL.", host) if s, err := net.Dial("tcp", host); err == nil { conn.sock = s } else { @@ -227,18 +222,18 @@ func (conn *Conn) recv() { for { s, err := conn.io.ReadString('\n') if err != nil { - conn.l.Error("irc.recv(): %s", err.Error()) + logging.Error("irc.recv(): %s", err.Error()) conn.shutdown() return } s = strings.Trim(s, "\r\n") - conn.l.Debug("<- %s", s) + logging.Debug("<- %s", s) if line := parseLine(s); line != nil { line.Time = time.Now() conn.in <- line } else { - conn.l.Warn("irc.recv(): problems parsing line:\n %s", s) + logging.Warn("irc.recv(): problems parsing line:\n %s", s) } } } @@ -276,23 +271,23 @@ func (conn *Conn) write(line string) { if !conn.Flood { if t := conn.rateLimit(len(line)); t != 0 { // sleep for the current line's time value before sending it - conn.l.Debug("irc.rateLimit(): Flood! Sleeping for %.2f secs.", + logging.Debug("irc.rateLimit(): Flood! Sleeping for %.2f secs.", t.Seconds()) <-time.After(t) } } if _, err := conn.io.WriteString(line + "\r\n"); err != nil { - conn.l.Error("irc.send(): %s", err.Error()) + logging.Error("irc.send(): %s", err.Error()) conn.shutdown() return } if err := conn.io.Flush(); err != nil { - conn.l.Error("irc.send(): %s", err.Error()) + logging.Error("irc.send(): %s", err.Error()) conn.shutdown() return } - conn.l.Debug("-> %s", line) + logging.Debug("-> %s", line) } // Implement Hybrid's flood control algorithm to rate-limit outgoing lines. @@ -318,7 +313,7 @@ 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 { - conn.l.Info("irc.shutdown(): Disconnected from server.") + logging.Info("irc.shutdown(): Disconnected from server.") conn.ED.Dispatch("disconnected", conn, &Line{}) conn.Connected = false conn.sock.Close() diff --git a/client/connection_test.go b/client/connection_test.go index caf5812..f6cb1ff 100644 --- a/client/connection_test.go +++ b/client/connection_test.go @@ -13,7 +13,6 @@ import ( type testState struct { ctrl *gomock.Controller - log *logging.MockLogger st *state.MockStateTracker ed *event.MockEventDispatcher nc *mockNetConn @@ -25,13 +24,9 @@ func setUp(t *testing.T, start ...bool) (*Conn, *testState) { st := state.NewMockStateTracker(ctrl) r := event.NewRegistry() ed := event.NewMockEventDispatcher(ctrl) - l := logging.NewMockLogger(ctrl) nc := MockNetConn(t) - c := Client("test", "test", "Testing IRC", r, 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 := Client("test", "test", "Testing IRC", r) + logging.SetLogLevel(logging.LogFatal) c.ED = ed c.ST = st @@ -47,14 +42,12 @@ func setUp(t *testing.T, start ...bool) (*Conn, *testState) { <-time.After(1e6) } - return c, &testState{ctrl, l, st, ed, nc, c} + return c, &testState{ctrl, st, ed, nc, c} } func (s *testState) tearDown() { s.ed.EXPECT().Dispatch("disconnected", s.c, &Line{}) 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) @@ -71,8 +64,6 @@ func TestEOF(t *testing.T) { // Simulate EOF from server s.ed.EXPECT().Dispatch("disconnected", c, &Line{}) 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 @@ -89,7 +80,6 @@ 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) - l := logging.NewMockLogger(ctrl) st := state.NewMockStateTracker(ctrl) for n, _ := range intHandlers { @@ -99,10 +89,10 @@ func TestClientAndStateTracking(t *testing.T) { // handler names are correctly passed to AddHandler. ctrl.RecordCall(r, "AddHandler", gomock.Any(), n) } - c := Client("test", "test", "Testing IRC", r, l) + c := Client("test", "test", "Testing IRC", r) // Assert some basic things about the initial state of the Conn struct - if c.ER != r || c.ED != r || c.l != l || c.st != false || c.ST != nil { + 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 { @@ -246,8 +236,6 @@ func TestRecv(t *testing.T) { // Test that recv does something useful with a line it can't parse // (not that there are many, parseLine is forgiving). - s.log.EXPECT().Warn("irc.recv(): problems parsing line:\n %s", - ":textwithnospaces") s.nc.Send(":textwithnospaces") if l := reader(); l != nil { t.Errorf("Bad line still caused receive on input channel.") @@ -259,8 +247,6 @@ func TestRecv(t *testing.T) { } s.ed.EXPECT().Dispatch("disconnected", c, &Line{}) 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 send and runloop aren't actually running, we need to empty their @@ -449,8 +435,6 @@ func TestWrite(t *testing.T) { s.ed.EXPECT().Dispatch("disconnected", c, &Line{}) s.st.EXPECT().Wipe() - s.log.EXPECT().Info("irc.shutdown(): Disconnected from server.") - s.log.EXPECT().Error("irc.send(): %s", "invalid argument") c.write("she can't pass unit tests") } diff --git a/client/handlers_test.go b/client/handlers_test.go index 1d7e082..1ea08b4 100644 --- a/client/handlers_test.go +++ b/client/handlers_test.go @@ -138,7 +138,7 @@ func TestJOIN(t *testing.T) { defer s.tearDown() // The state tracker should be creating a new channel in this first test - chan1 := state.NewChannel("#test1", s.log) + chan1 := state.NewChannel("#test1") gomock.InOrder( s.st.EXPECT().GetChannel("#test1").Return(nil), @@ -156,7 +156,7 @@ func TestJOIN(t *testing.T) { s.nc.Expect("WHO #test1") // In this second test, we should be creating a new nick - nick1 := state.NewNick("user1", s.log) + nick1 := state.NewNick("user1") gomock.InOrder( s.st.EXPECT().GetChannel("#test1").Return(chan1), @@ -172,7 +172,7 @@ func TestJOIN(t *testing.T) { s.nc.Expect("WHO user1") // In this third test, we'll be pretending we know about the nick already. - nick2 := state.NewNick("user2", c.l) + nick2 := state.NewNick("user2") gomock.InOrder( s.st.EXPECT().GetChannel("#test1").Return(chan1), s.st.EXPECT().GetNick("user2").Return(nick2), @@ -185,13 +185,9 @@ func TestJOIN(t *testing.T) { // 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")) c.h_JOIN(parseLine(":user2!ident2@host2.com JOIN :#test2")) @@ -203,8 +199,8 @@ func TestPART(t *testing.T) { defer s.tearDown() // We need some valid and associated nicks / channels to PART with. - chan1 := state.NewChannel("#test1", s.log) - nick1 := state.NewNick("user1", s.log) + chan1 := state.NewChannel("#test1") + nick1 := state.NewNick("user1") // PART should dissociate a nick from a channel. gomock.InOrder( @@ -222,8 +218,8 @@ func TestKICK(t *testing.T) { defer s.tearDown() // We need some valid and associated nicks / channels to KICK. - chan1 := state.NewChannel("#test1", s.log) - nick1 := state.NewNick("user1", s.log) + chan1 := state.NewChannel("#test1") + nick1 := state.NewNick("user1") // KICK should dissociate a nick from a channel. gomock.InOrder( @@ -249,8 +245,8 @@ func TestMODE(t *testing.T) { c, s := setUp(t) defer s.tearDown() - chan1 := state.NewChannel("#test1", s.log) - nick1 := state.NewNick("user1", s.log) + chan1 := state.NewChannel("#test1") + nick1 := state.NewNick("user1") // Send a channel mode line. Inconveniently, Channel and Nick objects // aren't mockable with gomock as they're not interface types (and I @@ -278,13 +274,9 @@ func TestMODE(t *testing.T) { // 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")) c.h_MODE(parseLine(":user1!ident1@host1.com MODE #test2 +is")) @@ -295,7 +287,7 @@ func TestTOPIC(t *testing.T) { c, s := setUp(t) defer s.tearDown() - chan1 := state.NewChannel("#test1", s.log) + chan1 := state.NewChannel("#test1") // Assert that it has no topic originally if chan1.Topic != "" { @@ -312,11 +304,7 @@ func TestTOPIC(t *testing.T) { } // 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"), - ) + s.st.EXPECT().GetChannel("#test2").Return(nil) c.h_TOPIC(parseLine(":user1!ident1@host1.com TOPIC #test2 :dark side")) } @@ -326,7 +314,7 @@ func Test311(t *testing.T) { defer s.tearDown() // Create user1, who we know little about - nick1 := state.NewNick("user1", s.log) + nick1 := state.NewNick("user1") // Send a 311 reply s.st.EXPECT().GetNick("user1").Return(nick1) @@ -340,11 +328,7 @@ func Test311(t *testing.T) { } // 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"), - ) + s.st.EXPECT().GetNick("user2").Return(nil) c.h_311(parseLine(":irc.server.org 311 test user2 ident2 host2.com * :dongs")) } @@ -354,7 +338,7 @@ func Test324(t *testing.T) { defer s.tearDown() // Create #test1, whose modes we don't know - chan1 := state.NewChannel("#test1", s.log) + chan1 := state.NewChannel("#test1") // Send a 324 reply s.st.EXPECT().GetChannel("#test1").Return(chan1) @@ -363,12 +347,8 @@ func Test324(t *testing.T) { 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"), - ) + // Check error paths -- send 324 for an unknown channel + s.st.EXPECT().GetChannel("#test2").Return(nil) c.h_324(parseLine(":irc.server.org 324 test #test2 +pmt")) } @@ -378,7 +358,7 @@ func Test332(t *testing.T) { defer s.tearDown() // Create #test1, whose topic we don't know - chan1 := state.NewChannel("#test1", s.log) + chan1 := state.NewChannel("#test1") // Assert that it has no topic originally if chan1.Topic != "" { @@ -394,12 +374,8 @@ func Test332(t *testing.T) { t.Errorf("Topic of test channel not set correctly.") } - // Check unknown channel causes an error - gomock.InOrder( - s.st.EXPECT().GetChannel("#test2").Return(nil), - s.log.EXPECT().Warn("irc.332(): received TOPIC value for unknown "+ - "channel %s", "#test2"), - ) + // Check error paths -- send 332 for an unknown channel + s.st.EXPECT().GetChannel("#test2").Return(nil) c.h_332(parseLine(":irc.server.org 332 test #test2 :dark side")) } @@ -409,7 +385,7 @@ func Test352(t *testing.T) { defer s.tearDown() // Create user1, who we know little about - nick1 := state.NewNick("user1", s.log) + nick1 := state.NewNick("user1") // Send a 352 reply s.st.EXPECT().GetNick("user1").Return(nick1) @@ -433,11 +409,7 @@ func Test352(t *testing.T) { } // 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"), - ) + s.st.EXPECT().GetNick("user2").Return(nil) c.h_352(parseLine(":irc.server.org 352 test #test2 ident2 host2.com irc.server.org user2 G :0 fooo")) } @@ -447,7 +419,7 @@ func Test353(t *testing.T) { defer s.tearDown() // Create #test1, whose user list we're mostly unfamiliar with - chan1 := state.NewChannel("#test1", s.log) + chan1 := state.NewChannel("#test1") // Create maps for testing -- this is what the mock ST calls will return nicks := make(map[string]*state.Nick) @@ -458,7 +430,7 @@ func Test353(t *testing.T) { for _, n := range []string{"user1", "user2", "voice", "halfop", "op", "admin", "owner"} { - nicks[n] = state.NewNick(n, s.log) + nicks[n] = state.NewNick(n) privs[n] = new(state.ChanPrivs) } @@ -495,12 +467,8 @@ func Test353(t *testing.T) { t.Errorf("353 handler failed to set correct modes for nicks.") } - // 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"), - ) + // Check error paths -- send 353 for an unknown channel + s.st.EXPECT().GetChannel("#test2").Return(nil) c.h_353(parseLine(":irc.server.org 353 test = #test2 :test ~user3")) } @@ -510,7 +478,7 @@ func Test671(t *testing.T) { defer s.tearDown() // Create user1, who should not be secure - nick1 := state.NewNick("user1", s.log) + nick1 := state.NewNick("user1") if nick1.Modes.SSL { t.Errorf("Test nick user1 is already using SSL?") } @@ -525,10 +493,6 @@ func Test671(t *testing.T) { } // 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"), - ) + s.st.EXPECT().GetNick("user2").Return(nil) c.h_671(parseLine(":irc.server.org 671 test user2 :some ignored text")) } diff --git a/client/state_handlers.go b/client/state_handlers.go index efd8c48..34e87ee 100644 --- a/client/state_handlers.go +++ b/client/state_handlers.go @@ -5,6 +5,7 @@ package client import ( "github.com/fluffle/goevent/event" + "github.com/fluffle/golog/logging" "strings" ) @@ -53,7 +54,7 @@ func (conn *Conn) h_JOIN(line *Line) { // first we've seen of this channel, so should be us joining it // 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 "+ + logging.Warn("irc.JOIN(): JOIN to unknown channel %s received "+ "from (non-me) nick %s", line.Args[0], line.Nick) return } @@ -105,13 +106,13 @@ func (conn *Conn) h_MODE(line *Line) { } else if nk := conn.ST.GetNick(line.Args[0]); nk != nil { // nick mode change, should be us if nk != conn.Me { - conn.l.Warn("irc.MODE(): recieved MODE %s for (non-me) nick %s", + logging.Warn("irc.MODE(): recieved MODE %s for (non-me) nick %s", line.Args[1], line.Args[0]) return } nk.ParseModes(line.Args[1]) } else { - conn.l.Warn("irc.MODE(): not sure what to do with MODE %s", + logging.Warn("irc.MODE(): not sure what to do with MODE %s", strings.Join(line.Args, " ")) } } @@ -121,7 +122,7 @@ func (conn *Conn) h_TOPIC(line *Line) { if ch := conn.ST.GetChannel(line.Args[0]); ch != nil { ch.Topic = line.Args[1] } else { - conn.l.Warn("irc.TOPIC(): topic change on unknown channel %s", + logging.Warn("irc.TOPIC(): topic change on unknown channel %s", line.Args[0]) } } @@ -133,7 +134,7 @@ func (conn *Conn) h_311(line *Line) { nk.Host = line.Args[3] nk.Name = line.Args[5] } else { - conn.l.Warn("irc.311(): received WHOIS info for unknown nick %s", + logging.Warn("irc.311(): received WHOIS info for unknown nick %s", line.Args[1]) } } @@ -143,7 +144,7 @@ func (conn *Conn) h_324(line *Line) { if ch := conn.ST.GetChannel(line.Args[1]); ch != nil { ch.ParseModes(line.Args[2], line.Args[3:]...) } else { - conn.l.Warn("irc.324(): received MODE settings for unknown channel %s", + logging.Warn("irc.324(): received MODE settings for unknown channel %s", line.Args[1]) } } @@ -153,7 +154,7 @@ func (conn *Conn) h_332(line *Line) { if ch := conn.ST.GetChannel(line.Args[1]); ch != nil { ch.Topic = line.Args[2] } else { - conn.l.Warn("irc.332(): received TOPIC value for unknown channel %s", + logging.Warn("irc.332(): received TOPIC value for unknown channel %s", line.Args[1]) } } @@ -175,7 +176,7 @@ func (conn *Conn) h_352(line *Line) { nk.Modes.Invisible = true } } else { - conn.l.Warn("irc.352(): received WHO reply for unknown nick %s", + logging.Warn("irc.352(): received WHO reply for unknown nick %s", line.Args[5]) } } @@ -219,7 +220,7 @@ func (conn *Conn) h_353(line *Line) { } } } else { - conn.l.Warn("irc.353(): received NAMES list for unknown channel %s", + logging.Warn("irc.353(): received NAMES list for unknown channel %s", line.Args[2]) } } @@ -229,7 +230,7 @@ func (conn *Conn) h_671(line *Line) { if nk := conn.ST.GetNick(line.Args[1]); nk != nil { nk.Modes.SSL = true } else { - conn.l.Warn("irc.671(): received WHOIS SSL info for unknown nick %s", + logging.Warn("irc.671(): received WHOIS SSL info for unknown nick %s", line.Args[1]) } } From a6742671280950c09bcc3f1aca27a936c5f4ef8b Mon Sep 17 00:00:00 2001 From: Alex Bramley Date: Sat, 16 Feb 2013 00:17:31 +0000 Subject: [PATCH 11/15] 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 From dffd6b1cb1affbae1bfd49afb92e88652781aabe Mon Sep 17 00:00:00 2001 From: Chris Rhodes Date: Fri, 15 Feb 2013 17:24:54 -0800 Subject: [PATCH 12/15] Use the internal event handling for initial pass/user/nick messages. Added constants for internal events. gofmt'ed all files and updated client.go/documentation. --- README.md | 4 ++-- client.go | 12 +++++++----- client/commands.go | 6 ++++++ client/connection.go | 21 +++++++++++---------- client/connection_test.go | 16 ++++++++-------- client/dispatch.go | 4 +++- client/dispatch_test.go | 20 ++++++++++---------- client/handlers.go | 20 ++++++++++++++++---- client/handlers_test.go | 8 +++----- client/line_test.go | 14 +++++++------- client/state_handlers.go | 24 ++++++++++++------------ 11 files changed, 85 insertions(+), 64 deletions(-) diff --git a/README.md b/README.md index 5f22864..ea53fd9 100644 --- a/README.md +++ b/README.md @@ -24,11 +24,11 @@ Synopsis: // Add handlers to do things here! // e.g. join a channel on connect. - c.AddHandler("connected", + c.HandleFunc(irc.CONNECTED, func(conn *irc.Conn, line *irc.Line) { conn.Join("#channel") }) // And a signal on disconnect quit := make(chan bool) - c.AddHandler("disconnected", + c.HandleFunc(irc.DISCONNECTED, func(conn *irc.Conn, line *irc.Line) { quit <- true }) // Tell client to connect diff --git a/client.go b/client.go index cb8d02f..0ff9d18 100644 --- a/client.go +++ b/client.go @@ -1,11 +1,11 @@ package main import ( - irc "github.com/fluffle/goirc/client" + "bufio" "flag" "fmt" + irc "github.com/fluffle/goirc/client" "os" - "bufio" "strings" ) @@ -16,14 +16,14 @@ func main() { flag.Parse() // create new IRC connection - c := irc.SimpleClient("GoTest", "gotest") + c := irc.Client("GoTest", "gotest") c.EnableStateTracking() - c.AddHandler("connected", + c.HandleFunc(irc.CONNECTED, func(conn *irc.Conn, line *irc.Line) { conn.Join(*channel) }) // Set up a handler to notify of disconnect events. quit := make(chan bool) - c.AddHandler("disconnected", + c.HandleFunc(irc.DISCONNECTED, func(conn *irc.Conn, line *irc.Line) { quit <- true }) // set up a goroutine to read commands from stdin @@ -36,6 +36,8 @@ func main() { if err != nil { // wha?, maybe ctrl-D... close(in) + reallyquit = true + c.Quit("") break } // no point in sending empty lines down the channel diff --git a/client/commands.go b/client/commands.go index 870b6a1..0f58109 100644 --- a/client/commands.go +++ b/client/commands.go @@ -2,6 +2,12 @@ package client import "strings" +const ( + INIT = "init" + CONNECTED = "connected" + DISCONNECTED = "disconnected" +) + // this file contains the various commands you can // send to the server using an Conn connection diff --git a/client/connection.go b/client/connection.go index dce6c43..f66f3c5 100644 --- a/client/connection.go +++ b/client/connection.go @@ -15,9 +15,10 @@ import ( // An IRC connection is represented by this struct. type Conn struct { // Connection Hostname and Nickname - Host string - Me *state.Nick - Network string + Host string + Me *state.Nick + Network string + password string // Handlers and Commands handlers *hSet @@ -60,7 +61,7 @@ type Conn struct { Flood bool // Internal counters for flood protection - badness time.Duration + badness time.Duration lastsent time.Time } @@ -164,13 +165,12 @@ func (conn *Conn) Connect(host string, pass ...string) error { } conn.Host = host conn.Connected = true - conn.postConnect() - if len(pass) > 0 { - conn.Pass(pass[0]) + conn.password = pass[0] + } else { + conn.password = "" } - conn.Nick(conn.Me.Nick) - conn.User(conn.Me.Ident, conn.Me.Name) + conn.postConnect() return nil } @@ -188,6 +188,7 @@ func (conn *Conn) postConnect() { go func() { <-conn.cPing }() } go conn.runLoop() + conn.dispatch(&Line{Cmd: INIT}) } // copied from http.client for great justice @@ -305,7 +306,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.dispatch(&Line{Cmd: "disconnected"}) + 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 6386eb3..caf1389 100644 --- a/client/connection_test.go +++ b/client/connection_test.go @@ -3,8 +3,8 @@ package client import ( "bufio" "code.google.com/p/gomock/gomock" - "github.com/fluffle/golog/logging" "github.com/fluffle/goirc/state" + "github.com/fluffle/golog/logging" "strings" "testing" "time" @@ -57,7 +57,7 @@ func TestEOF(t *testing.T) { // Set up a handler to detect whether disconnected handlers are called dcon := false - c.HandleFunc("disconnected", func (conn *Conn, line *Line) { + c.HandleFunc(DISCONNECTED, func(conn *Conn, line *Line) { dcon = true }) @@ -356,12 +356,12 @@ func TestRunLoop(t *testing.T) { // Set up a handler to detect whether 001 handler is called h001 := false - c.HandleFunc("001", func (conn *Conn, line *Line) { + 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) { + c.HandleFunc("002", func(conn *Conn, line *Line) { h002 = true }) @@ -470,7 +470,7 @@ func TestRateLimit(t *testing.T) { // We'll be needing this later... abs := func(i time.Duration) time.Duration { - if (i < 0) { + if i < 0 { return -i } return i @@ -491,13 +491,13 @@ func TestRateLimit(t *testing.T) { // 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 - 2500*time.Millisecond) > 10 * time.Microsecond { + 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 - 10500*time.Millisecond) > 10 * time.Microsecond { + if l := c.rateLimit(720); l != 8*time.Second || + 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 index f35a10f..dd73865 100644 --- a/client/dispatch.go +++ b/client/dispatch.go @@ -105,7 +105,9 @@ func (hs *hSet) dispatch(conn *Conn, line *Line) { defer hs.RUnlock() ev := strings.ToLower(line.Cmd) list, ok := hs.set[ev] - if !ok { return } + if !ok { + return + } for hn := list.start; hn != nil; hn = hn.next { go hn.Handle(conn, line) } diff --git a/client/dispatch_test.go b/client/dispatch_test.go index 427c69e..84b9e54 100644 --- a/client/dispatch_test.go +++ b/client/dispatch_test.go @@ -83,7 +83,7 @@ func TestHandlerSet(t *testing.T) { if callcount != 0 { t.Errorf("Something incremented call count before we were expecting it.") } - hs.dispatch(nil, &Line{Cmd:"One"}) + hs.dispatch(nil, &Line{Cmd: "One"}) <-time.After(time.Millisecond) if callcount != 4 { t.Errorf("Our handler wasn't called four times :-(") @@ -107,7 +107,7 @@ func TestHandlerSet(t *testing.T) { } // Dispatch should result in 3 additions. - hs.dispatch(nil, &Line{Cmd:"One"}) + hs.dispatch(nil, &Line{Cmd: "One"}) <-time.After(time.Millisecond) if callcount != 7 { t.Errorf("Our handler wasn't called three times :-(") @@ -129,7 +129,7 @@ func TestHandlerSet(t *testing.T) { } // Dispatch should result in 2 additions. - hs.dispatch(nil, &Line{Cmd:"One"}) + hs.dispatch(nil, &Line{Cmd: "One"}) <-time.After(time.Millisecond) if callcount != 9 { t.Errorf("Our handler wasn't called two times :-(") @@ -151,7 +151,7 @@ func TestHandlerSet(t *testing.T) { } // Dispatch should result in 1 addition. - hs.dispatch(nil, &Line{Cmd:"One"}) + hs.dispatch(nil, &Line{Cmd: "One"}) <-time.After(time.Millisecond) if callcount != 10 { t.Errorf("Our handler wasn't called once :-(") @@ -170,7 +170,7 @@ func TestHandlerSet(t *testing.T) { } // Dispatch should result in NO additions. - hs.dispatch(nil, &Line{Cmd:"One"}) + hs.dispatch(nil, &Line{Cmd: "One"}) <-time.After(time.Millisecond) if callcount != 10 { t.Errorf("Our handler was called?") @@ -184,7 +184,7 @@ func TestCommandSet(t *testing.T) { } c := &command{ - fn: func(c *Conn, l *Line) {}, + fn: func(c *Conn, l *Line) {}, help: "wtf?", } @@ -196,7 +196,7 @@ func TestCommandSet(t *testing.T) { 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.") @@ -208,7 +208,7 @@ func TestCommandSet(t *testing.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 { + if c, l := cs.match("one two three"); c.(*cNode) != cn2 || l != 7 { t.Errorf("Didn't match 'one two' when we should have.") } @@ -216,14 +216,14 @@ func TestCommandSet(t *testing.T) { 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 { + 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 { + 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 dcc48f9..9e1c41b 100644 --- a/client/handlers.go +++ b/client/handlers.go @@ -9,8 +9,9 @@ import ( // sets up the internal event handlers to do essential IRC protocol things var intHandlers = map[string]HandlerFunc{ - "001": (*Conn).h_001, - "433": (*Conn).h_433, + INIT: (*Conn).h_init, + "001": (*Conn).h_001, + "433": (*Conn).h_433, "CTCP": (*Conn).h_CTCP, "NICK": (*Conn).h_NICK, "PING": (*Conn).h_PING, @@ -24,6 +25,15 @@ func (conn *Conn) addIntHandlers() { } } +// Password/User/Nick broadcast on connection. +func (conn *Conn) h_init(line *Line) { + if conn.password != "" { + conn.Pass(conn.password) + } + conn.Nick(conn.Me.Nick) + conn.User(conn.Me.Ident, conn.Me.Name) +} + // Basic ping/pong handler func (conn *Conn) h_PING(line *Line) { conn.Raw("PONG :" + line.Args[0]) @@ -32,7 +42,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.dispatch(&Line{Cmd: "connected"}) + 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 { @@ -99,7 +109,9 @@ func (conn *Conn) h_PRIVMSG(line *Line) { } } cmd, l := conn.cmdMatch(txt) - if cmd == nil { return } + if cmd == nil { + return + } if conn.CommandStripPrefix { txt = strings.TrimSpace(txt[l:]) } diff --git a/client/handlers_test.go b/client/handlers_test.go index fcedbb2..1389967 100644 --- a/client/handlers_test.go +++ b/client/handlers_test.go @@ -27,7 +27,7 @@ func Test001(t *testing.T) { l := parseLine(":irc.server.org 001 test :Welcome to IRC test!ident@somehost.com") // Set up a handler to detect whether connected handler is called from 001 hcon := false - c.HandleFunc("connected", func (conn *Conn, line *Line) { + c.HandleFunc(CONNECTED, func(conn *Conn, line *Line) { hcon = true }) @@ -139,11 +139,11 @@ func TestCTCP(t *testing.T) { c.h_CTCP(parseLine(":blah!moo@cows.com PRIVMSG test :\001UNKNOWN ctcp\001")) } -func TestPRIVMSG(t *testing.T){ +func TestPRIVMSG(t *testing.T) { c, s := setUp(t) defer s.tearDown() - f := func (conn *Conn, line *Line) { + f := func(conn *Conn, line *Line) { conn.Privmsg(line.Args[0], line.Args[1]) } c.CommandFunc("prefix", f, "") @@ -188,7 +188,6 @@ func TestPRIVMSG(t *testing.T){ c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :test! prefix bar")) s.nc.ExpectNothing() - } // Test the handler for JOIN messages @@ -317,7 +316,6 @@ func TestMODE(t *testing.T) { t.Errorf("Channel.ParseModes() not called correctly.") } - // Send a nick mode line, returning Me gomock.InOrder( s.st.EXPECT().GetChannel("test").Return(nil), diff --git a/client/line_test.go b/client/line_test.go index 6b9af7e..37e43dd 100644 --- a/client/line_test.go +++ b/client/line_test.go @@ -7,14 +7,14 @@ import ( func TestCopy(t *testing.T) { l1 := &Line{ - Nick: "nick", + Nick: "nick", Ident: "ident", - Host: "host", - Src: "src", - Cmd: "cmd", - Raw: "raw", - Args: []string{"arg", "text"}, - Time: time.Now(), + Host: "host", + Src: "src", + Cmd: "cmd", + Raw: "raw", + Args: []string{"arg", "text"}, + Time: time.Now(), } l2 := l1.Copy() diff --git a/client/state_handlers.go b/client/state_handlers.go index ab2e60a..ffc9211 100644 --- a/client/state_handlers.go +++ b/client/state_handlers.go @@ -9,19 +9,19 @@ import ( ) 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, + "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, + "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() { From 6868c8a0b711fef13848a36c63f6f1a2f1ac09ff Mon Sep 17 00:00:00 2001 From: Chris Rhodes Date: Fri, 15 Feb 2013 18:03:13 -0800 Subject: [PATCH 13/15] Use consistant constants constantly. --- client/commands.go | 69 +++++++++++++++++++++++++++------------- client/commands_test.go | 6 ++++ client/connection.go | 2 +- client/handlers.go | 22 ++++++------- client/line.go | 10 +++--- client/state_handlers.go | 26 +++++++-------- 6 files changed, 83 insertions(+), 52 deletions(-) diff --git a/client/commands.go b/client/commands.go index 0f58109..594cb5b 100644 --- a/client/commands.go +++ b/client/commands.go @@ -6,6 +6,28 @@ const ( INIT = "init" CONNECTED = "connected" DISCONNECTED = "disconnected" + ACTION = "ACTION" + AWAY = "AWAY" + CTCP = "CTCP" + CTCPREPLY = "CTCPREPLY" + INVITE = "INVITE" + JOIN = "JOIN" + KICK = "KICK" + MODE = "MODE" + NICK = "NICK" + NOTICE = "NOTICE" + OPER = "OPER" + PART = "PART" + PASS = "PASS" + PING = "PING" + PONG = "PONG" + PRIVMSG = "PRIVMSG" + QUIT = "QUIT" + TOPIC = "TOPIC" + USER = "USER" + VERSION = "VERSION" + WHO = "WHO" + WHOIS = "WHOIS" ) // this file contains the various commands you can @@ -20,18 +42,18 @@ const ( func (conn *Conn) Raw(rawline string) { conn.out <- rawline } // Pass() sends a PASS command to the server -func (conn *Conn) Pass(password string) { conn.out <- "PASS " + password } +func (conn *Conn) Pass(password string) { conn.out <- PASS + " " + password } // Nick() sends a NICK command to the server -func (conn *Conn) Nick(nick string) { conn.out <- "NICK " + nick } +func (conn *Conn) Nick(nick string) { conn.out <- NICK + " " + nick } // User() sends a USER command to the server func (conn *Conn) User(ident, name string) { - conn.out <- "USER " + ident + " 12 * :" + name + conn.out <- USER + " " + ident + " 12 * :" + name } // Join() sends a JOIN command to the server -func (conn *Conn) Join(channel string) { conn.out <- "JOIN " + channel } +func (conn *Conn) Join(channel string) { conn.out <- JOIN + " " + channel } // Part() sends a PART command to the server with an optional part message func (conn *Conn) Part(channel string, message ...string) { @@ -39,7 +61,7 @@ func (conn *Conn) Part(channel string, message ...string) { if msg != "" { msg = " :" + msg } - conn.out <- "PART " + channel + msg + conn.out <- PART + " " + channel + msg } // Kick() sends a KICK command to remove a nick from a channel @@ -48,7 +70,7 @@ func (conn *Conn) Kick(channel, nick string, message ...string) { if msg != "" { msg = " :" + msg } - conn.out <- "KICK " + channel + " " + nick + msg + conn.out <- KICK + " " + channel + " " + nick + msg } // Quit() sends a QUIT command to the server with an optional quit message @@ -57,20 +79,20 @@ func (conn *Conn) Quit(message ...string) { if msg == "" { msg = "GoBye!" } - conn.out <- "QUIT :" + msg + conn.out <- QUIT + " :" + msg } // Whois() sends a WHOIS command to the server -func (conn *Conn) Whois(nick string) { conn.out <- "WHOIS " + nick } +func (conn *Conn) Whois(nick string) { conn.out <- WHOIS + " " + nick } //Who() sends a WHO command to the server -func (conn *Conn) Who(nick string) { conn.out <- "WHO " + nick } +func (conn *Conn) Who(nick string) { conn.out <- WHO + " " + nick } // Privmsg() sends a PRIVMSG to the target t -func (conn *Conn) Privmsg(t, msg string) { conn.out <- "PRIVMSG " + t + " :" + msg } +func (conn *Conn) Privmsg(t, msg string) { conn.out <- PRIVMSG + " " + t + " :" + msg } // Notice() sends a NOTICE to the target t -func (conn *Conn) Notice(t, msg string) { conn.out <- "NOTICE " + t + " :" + msg } +func (conn *Conn) Notice(t, msg string) { conn.out <- NOTICE + " " + t + " :" + msg } // Ctcp() sends a (generic) CTCP message to the target t // with an optional argument @@ -93,10 +115,10 @@ func (conn *Conn) CtcpReply(t, ctcp string, arg ...string) { } // Version() sends a CTCP "VERSION" to the target t -func (conn *Conn) Version(t string) { conn.Ctcp(t, "VERSION") } +func (conn *Conn) Version(t string) { conn.Ctcp(t, VERSION) } // Action() sends a CTCP "ACTION" to the target t -func (conn *Conn) Action(t, msg string) { conn.Ctcp(t, "ACTION", msg) } +func (conn *Conn) Action(t, msg string) { conn.Ctcp(t, ACTION, msg) } // Topic() sends a TOPIC command to the channel // Topic(channel) retrieves the current channel topic (see "332" handler) @@ -106,7 +128,7 @@ func (conn *Conn) Topic(channel string, topic ...string) { if t != "" { t = " :" + t } - conn.out <- "TOPIC " + channel + t + conn.out <- TOPIC + " " + channel + t } // Mode() sends a MODE command to the server. This one can get complicated if @@ -121,7 +143,7 @@ func (conn *Conn) Mode(t string, modestring ...string) { if mode != "" { mode = " " + mode } - conn.out <- "MODE " + t + mode + conn.out <- MODE + " " + t + mode } // Away() sends an AWAY command to the server @@ -132,15 +154,18 @@ func (conn *Conn) Away(message ...string) { if msg != "" { msg = " :" + msg } - conn.out <- "AWAY" + msg + conn.out <- AWAY + msg } // Invite() sends an INVITE command to the server -func (conn *Conn) Invite(nick, channel string) { - conn.out <- "INVITE " + nick + " " + channel -} +func (conn *Conn) Invite(nick, channel string) { conn.out <- INVITE + " " + nick + " " + channel } // Oper() sends an OPER command to the server -func (conn *Conn) Oper(user, pass string) { - conn.out <- "OPER " + user + " " + pass -} +func (conn *Conn) Oper(user, pass string) { conn.out <- OPER + " " + user + " " + pass } + +// Ping() sends a PING command to the server +// A PONG response is to be expected afterwards +func (conn *Conn) Ping(message string) { conn.out <- PING + " :" + message } + +// Ping() sends a PONG command to the server +func (conn *Conn) Pong(message string) { conn.out <- PONG + " :" + message } diff --git a/client/commands_test.go b/client/commands_test.go index c3bb6ee..05650e3 100644 --- a/client/commands_test.go +++ b/client/commands_test.go @@ -75,4 +75,10 @@ func TestClientCommands(t *testing.T) { c.Oper("user", "pass") s.nc.Expect("OPER user pass") + + c.Ping("woot") + s.nc.Expect("PING :woot") + + c.Pong("pwoot") + s.nc.Expect("PONG :pwoot") } diff --git a/client/connection.go b/client/connection.go index f66f3c5..e1128a2 100644 --- a/client/connection.go +++ b/client/connection.go @@ -236,7 +236,7 @@ func (conn *Conn) ping() { for { select { case <-tick.C: - conn.Raw(fmt.Sprintf("PING :%d", time.Now().UnixNano())) + conn.Ping(fmt.Sprintf("%d", time.Now().UnixNano())) case <-conn.cPing: tick.Stop() return diff --git a/client/handlers.go b/client/handlers.go index 9e1c41b..5d017be 100644 --- a/client/handlers.go +++ b/client/handlers.go @@ -9,12 +9,12 @@ import ( // sets up the internal event handlers to do essential IRC protocol things var intHandlers = map[string]HandlerFunc{ - INIT: (*Conn).h_init, - "001": (*Conn).h_001, - "433": (*Conn).h_433, - "CTCP": (*Conn).h_CTCP, - "NICK": (*Conn).h_NICK, - "PING": (*Conn).h_PING, + INIT: (*Conn).h_init, + "001": (*Conn).h_001, + "433": (*Conn).h_433, + CTCP: (*Conn).h_CTCP, + NICK: (*Conn).h_NICK, + PING: (*Conn).h_PING, } func (conn *Conn) addIntHandlers() { @@ -36,7 +36,7 @@ func (conn *Conn) h_init(line *Line) { // Basic ping/pong handler func (conn *Conn) h_PING(line *Line) { - conn.Raw("PONG :" + line.Args[0]) + conn.Pong(line.Args[0]) } // Handler to trigger a "CONNECTED" event on receipt of numeric 001 @@ -80,10 +80,10 @@ func (conn *Conn) h_433(line *Line) { // Handle VERSION requests and CTCP PING func (conn *Conn) h_CTCP(line *Line) { - if line.Args[0] == "VERSION" { - conn.CtcpReply(line.Nick, "VERSION", "powered by goirc...") - } else if line.Args[0] == "PING" { - conn.CtcpReply(line.Nick, "PING", line.Args[2]) + if line.Args[0] == VERSION { + conn.CtcpReply(line.Nick, VERSION, "powered by goirc...") + } else if line.Args[0] == PING { + conn.CtcpReply(line.Nick, PING, line.Args[2]) } } diff --git a/client/line.go b/client/line.go index 706d373..b29a8fe 100644 --- a/client/line.go +++ b/client/line.go @@ -62,7 +62,7 @@ func parseLine(s string) *Line { // So, I think CTCP and (in particular) CTCP ACTION are better handled as // separate events as opposed to forcing people to have gargantuan // handlers to cope with the possibilities. - if (line.Cmd == "PRIVMSG" || line.Cmd == "NOTICE") && + if (line.Cmd == PRIVMSG || line.Cmd == NOTICE) && len(line.Args[1]) > 2 && strings.HasPrefix(line.Args[1], "\001") && strings.HasSuffix(line.Args[1], "\001") { @@ -72,16 +72,16 @@ func parseLine(s string) *Line { // Replace the line with the unwrapped CTCP line.Args[1] = t[1] } - if c := strings.ToUpper(t[0]); c == "ACTION" && line.Cmd == "PRIVMSG" { + if c := strings.ToUpper(t[0]); c == ACTION && line.Cmd == PRIVMSG { // make a CTCP ACTION it's own event a-la PRIVMSG line.Cmd = c } else { // otherwise, dispatch a generic CTCP/CTCPREPLY event that // contains the type of CTCP in line.Args[0] - if line.Cmd == "PRIVMSG" { - line.Cmd = "CTCP" + if line.Cmd == PRIVMSG { + line.Cmd = CTCP } else { - line.Cmd = "CTCPREPLY" + line.Cmd = CTCPREPLY } line.Args = append([]string{c}, line.Args...) } diff --git a/client/state_handlers.go b/client/state_handlers.go index ffc9211..380f760 100644 --- a/client/state_handlers.go +++ b/client/state_handlers.go @@ -9,19 +9,19 @@ import ( ) 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, + 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() { From bd43f3769bfec4786ee1af6b8591ff651ec00769 Mon Sep 17 00:00:00 2001 From: Chris Rhodes Date: Fri, 15 Feb 2013 18:26:20 -0800 Subject: [PATCH 14/15] Fix tests due to more talky init. --- client/connection_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/client/connection_test.go b/client/connection_test.go index caf1389..e4d09ba 100644 --- a/client/connection_test.go +++ b/client/connection_test.go @@ -33,6 +33,10 @@ func setUp(t *testing.T, start ...bool) (*Conn, *testState) { // Hack to allow tests of send, recv, write etc. // NOTE: the value of the boolean doesn't matter. c.postConnect() + // All connections start with NICK/USER expect these. + nc.Expect("NICK test") + nc.Expect("USER test 12 * :Testing IRC") + // Sleep 1ms to allow background routines to start. <-time.After(1e6) } From 674c2f852eaea7d2db5db2f81b5a3ae8977521e8 Mon Sep 17 00:00:00 2001 From: Chris Rhodes Date: Fri, 15 Feb 2013 18:44:00 -0800 Subject: [PATCH 15/15] Pong Ping, What's the difference? --- client/commands.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/commands.go b/client/commands.go index 594cb5b..f620f61 100644 --- a/client/commands.go +++ b/client/commands.go @@ -167,5 +167,5 @@ func (conn *Conn) Oper(user, pass string) { conn.out <- OPER + " " + user + " " // A PONG response is to be expected afterwards func (conn *Conn) Ping(message string) { conn.out <- PING + " :" + message } -// Ping() sends a PONG command to the server +// Pong() sends a PONG command to the server func (conn *Conn) Pong(message string) { conn.out <- PONG + " :" + message }