diff --git a/client/connection.go b/client/connection.go index 6f004f5..c577b51 100644 --- a/client/connection.go +++ b/client/connection.go @@ -90,7 +90,7 @@ type Config struct { func NewConfig(nick string, args ...string) *Config { cfg := &Config{ - Me: state.NewNick(nick), + Me: &state.Nick{Nick: nick}, PingFreq: 3 * time.Minute, NewNick: func(s string) string { return s + "_" }, Recover: (*Conn).LogPanic, // in dispatch.go @@ -122,7 +122,7 @@ func Client(cfg *Config) *Conn { cfg = NewConfig("__idiot__") } if cfg.Me == nil || cfg.Me.Nick == "" || cfg.Me.Ident == "" { - cfg.Me = state.NewNick("__idiot__") + cfg.Me = &state.Nick{Nick: "__idiot__"} cfg.Me.Ident = "goirc" cfg.Me.Name = "Powered by GoIRC" } @@ -169,6 +169,11 @@ func (conn *Conn) Config() *Config { } func (conn *Conn) Me() *state.Nick { + conn.mu.RLock() + defer conn.mu.RUnlock() + if conn.st != nil { + conn.cfg.Me = conn.st.Me() + } return conn.cfg.Me } @@ -177,6 +182,8 @@ func (conn *Conn) StateTracker() state.Tracker { } func (conn *Conn) EnableStateTracking() { + conn.mu.Lock() + defer conn.mu.Unlock() if conn.st == nil { n := conn.cfg.Me conn.st = state.NewTracker(n.Nick) @@ -188,7 +195,10 @@ func (conn *Conn) EnableStateTracking() { } func (conn *Conn) DisableStateTracking() { + conn.mu.Lock() + defer conn.mu.Unlock() if conn.st != nil { + conn.cfg.Me = conn.st.Me() conn.delSTHandlers() conn.st.Wipe() conn.st = nil diff --git a/client/connection_test.go b/client/connection_test.go index aaa11f7..116f06f 100644 --- a/client/connection_test.go +++ b/client/connection_test.go @@ -128,20 +128,23 @@ func TestClientAndStateTracking(t *testing.T) { } // We're expecting the untracked me to be replaced by a tracked one + if c.st == nil { + t.Errorf("State tracker not enabled correctly.") + } if me := c.cfg.Me; me.Nick != "test" || me.Ident != "test" || me.Name != "Testing IRC" || me.Host != "" { t.Errorf("Enabling state tracking did not replace Me correctly.") } - if c.st == nil || c.cfg.Me != c.st.Me() { - t.Errorf("State tracker not enabled correctly.") - } // Now, shim in the mock state tracker and test disabling state tracking me := c.cfg.Me c.st = st - st.EXPECT().Wipe() + gomock.InOrder( + st.EXPECT().Me().Return(me), + st.EXPECT().Wipe(), + ) c.DisableStateTracking() - if c.st != nil || c.cfg.Me != me { + if c.st != nil || !c.cfg.Me.Equals(me) { t.Errorf("State tracker not disabled correctly.") } diff --git a/client/handlers.go b/client/handlers.go index 13d1e2d..884a9bd 100644 --- a/client/handlers.go +++ b/client/handlers.go @@ -49,7 +49,12 @@ func (conn *Conn) h_001(line *Line) { if idx := strings.LastIndex(t, " "); idx != -1 { t = t[idx+1:] if idx = strings.Index(t, "@"); idx != -1 { - conn.cfg.Me.Host = t[idx+1:] + if conn.st != nil { + me := conn.Me() + conn.st.NickInfo(me.Nick, me.Ident, t[idx+1:], me.Name) + } else { + conn.cfg.Me.Host = t[idx+1:] + } } } } @@ -65,14 +70,15 @@ 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 + me := conn.Me() neu := conn.cfg.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 // a NICK message to confirm our change of nick, so ReNick here... - if line.Args[1] == conn.cfg.Me.Nick { + if line.Args[1] == me.Nick { if conn.st != nil { - conn.st.ReNick(conn.cfg.Me.Nick, neu) + conn.cfg.Me = conn.st.ReNick(me.Nick, neu) } else { conn.cfg.Me.Nick = neu } diff --git a/client/handlers_test.go b/client/handlers_test.go index 6d436bd..a601563 100644 --- a/client/handlers_test.go +++ b/client/handlers_test.go @@ -50,6 +50,11 @@ func Test001(t *testing.T) { hcon = true }) + // Test state tracking first. + gomock.InOrder( + s.st.EXPECT().Me().Return(c.cfg.Me), + s.st.EXPECT().NickInfo("test", "test", "somehost.com", "Testing IRC"), + ) // Call handler with a valid 001 line c.h_001(l) <-time.After(time.Millisecond) @@ -57,10 +62,14 @@ func Test001(t *testing.T) { t.Errorf("001 handler did not dispatch connected event.") } + // Now without state tracking. + c.st = nil + c.h_001(l) // Check host parsed correctly if c.cfg.Me.Host != "somehost.com" { t.Errorf("Host parsing failed, host is '%s'.", c.cfg.Me.Host) } + c.st = s.st } // Test the handler for 433 / ERR_NICKNAMEINUSE @@ -69,29 +78,21 @@ func Test433(t *testing.T) { defer s.tearDown() // Call handler with a 433 line, not triggering c.cfg.Me.Renick() + s.st.EXPECT().Me().Return(c.cfg.Me) c.h_433(ParseLine(":irc.server.org 433 test new :Nickname is already in use.")) s.nc.Expect("NICK new_") - // In this case, we're expecting the server to send a NICK line - if c.cfg.Me.Nick != "test" { - t.Errorf("ReNick() called unexpectedly, Nick == '%s'.", c.cfg.Me.Nick) - } - // Send a line that will trigger a renick. This happens when our wanted // nick is unavailable during initial negotiation, so we must choose a // different one before the connection can proceed. No NICK line will be // sent by the server to confirm nick change in this case. - s.st.EXPECT().ReNick("test", "test_") + gomock.InOrder( + s.st.EXPECT().Me().Return(c.cfg.Me), + s.st.EXPECT().ReNick("test", "test_").Return(c.cfg.Me), + ) c.h_433(ParseLine(":irc.server.org 433 test test :Nickname is already in use.")) s.nc.Expect("NICK test_") - // Counter-intuitively, c.cfg.Me.Nick will not change in this case. This - // is an artifact of the test set-up, with a mocked out state tracker that - // doesn't actually change any state. Normally, this would be fine :-) - if c.cfg.Me.Nick != "test" { - t.Errorf("My nick changed from '%s'.", c.cfg.Me.Nick) - } - // Test the code path that *doesn't* involve state tracking. c.st = nil c.h_433(ParseLine(":irc.server.org 433 test test :Nickname is already in use.")) @@ -164,13 +165,14 @@ 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") + chan1 := &state.Channel{Name: "#test1"} gomock.InOrder( s.st.EXPECT().GetChannel("#test1").Return(nil), s.st.EXPECT().GetNick("test").Return(c.cfg.Me), + s.st.EXPECT().Me().Return(c.cfg.Me), s.st.EXPECT().NewChannel("#test1").Return(chan1), - s.st.EXPECT().Associate(chan1, c.cfg.Me), + s.st.EXPECT().Associate("#test1", "test"), ) // Use #test1 to test expected behaviour @@ -182,13 +184,14 @@ 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") + nick1 := &state.Nick{Nick: "user1"} gomock.InOrder( s.st.EXPECT().GetChannel("#test1").Return(chan1), s.st.EXPECT().GetNick("user1").Return(nil), s.st.EXPECT().NewNick("user1").Return(nick1), - s.st.EXPECT().Associate(chan1, nick1), + s.st.EXPECT().NickInfo("user1", "ident1", "host1.com", "").Return(nick1), + s.st.EXPECT().Associate("#test1", "user1"), ) // OK, now #test1 exists, JOIN another user we don't know about @@ -198,11 +201,11 @@ 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") + nick2 := &state.Nick{Nick: "user2"} gomock.InOrder( s.st.EXPECT().GetChannel("#test1").Return(chan1), s.st.EXPECT().GetNick("user2").Return(nick2), - s.st.EXPECT().Associate(chan1, nick2), + s.st.EXPECT().Associate("#test1", "user2"), ) c.h_JOIN(ParseLine(":user2!ident2@host2.com JOIN :#test1")) @@ -211,9 +214,11 @@ func TestJOIN(t *testing.T) { // unknown channel, unknown nick s.st.EXPECT().GetChannel("#test2").Return(nil), s.st.EXPECT().GetNick("blah").Return(nil), + s.st.EXPECT().Me().Return(c.cfg.Me), // unknown channel, known nick that isn't Me. s.st.EXPECT().GetChannel("#test2").Return(nil), s.st.EXPECT().GetNick("user2").Return(nick2), + s.st.EXPECT().Me().Return(c.cfg.Me), ) c.h_JOIN(ParseLine(":blah!moo@cows.com JOIN :#test2")) c.h_JOIN(ParseLine(":user2!ident2@host2.com JOIN :#test2")) @@ -224,16 +229,8 @@ func TestPART(t *testing.T) { c, s := setUp(t) defer s.tearDown() - // We need some valid and associated nicks / channels to PART with. - chan1 := state.NewChannel("#test1") - nick1 := state.NewNick("user1") - // PART should dissociate a nick from a channel. - gomock.InOrder( - s.st.EXPECT().GetChannel("#test1").Return(chan1), - s.st.EXPECT().GetNick("user1").Return(nick1), - s.st.EXPECT().Dissociate(chan1, nick1), - ) + s.st.EXPECT().Dissociate("#test1", "user1") c.h_PART(ParseLine(":user1!ident1@host1.com PART #test1 :Bye!")) } @@ -243,16 +240,8 @@ func TestKICK(t *testing.T) { c, s := setUp(t) defer s.tearDown() - // We need some valid and associated nicks / channels to KICK. - chan1 := state.NewChannel("#test1") - nick1 := state.NewNick("user1") - // KICK should dissociate a nick from a channel. - gomock.InOrder( - s.st.EXPECT().GetChannel("#test1").Return(chan1), - s.st.EXPECT().GetNick("user1").Return(nick1), - s.st.EXPECT().Dissociate(chan1, nick1), - ) + s.st.EXPECT().Dissociate("#test1", "user1") c.h_KICK(ParseLine(":test!test@somehost.com KICK #test1 user1 :Bye!")) } @@ -271,34 +260,28 @@ func TestMODE(t *testing.T) { c, s := setUp(t) defer s.tearDown() - 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 - // don't want them to be, writing accessors for struct fields sucks). - // This makes testing whether ParseModes is called correctly harder. - s.st.EXPECT().GetChannel("#test1").Return(chan1) + // Channel modes + gomock.InOrder( + s.st.EXPECT().GetChannel("#test1").Return(&state.Channel{Name: "#test1"}), + s.st.EXPECT().ChannelModes("#test1", "+sk", "somekey"), + ) c.h_MODE(ParseLine(":user1!ident1@host1.com MODE #test1 +sk somekey")) - if !chan1.Modes.Secret || chan1.Modes.Key != "somekey" { - t.Errorf("Channel.ParseModes() not called correctly.") - } - // Send a nick mode line, returning Me + // Nick modes for Me. gomock.InOrder( s.st.EXPECT().GetChannel("test").Return(nil), s.st.EXPECT().GetNick("test").Return(c.cfg.Me), + s.st.EXPECT().Me().Return(c.cfg.Me), + s.st.EXPECT().NickModes("test", "+i"), ) c.h_MODE(ParseLine(":test!test@somehost.com MODE test +i")) - if !c.cfg.Me.Modes.Invisible { - t.Errorf("Nick.ParseModes() not called correctly.") - } // Check error paths gomock.InOrder( // send a valid user mode that's not us s.st.EXPECT().GetChannel("user1").Return(nil), - s.st.EXPECT().GetNick("user1").Return(nick1), + s.st.EXPECT().GetNick("user1").Return(&state.Nick{Nick: "user1"}), + s.st.EXPECT().Me().Return(c.cfg.Me), // Send a random mode for an unknown channel s.st.EXPECT().GetChannel("#test2").Return(nil), s.st.EXPECT().GetNick("#test2").Return(nil), @@ -312,22 +295,13 @@ func TestTOPIC(t *testing.T) { c, s := setUp(t) defer s.tearDown() - chan1 := state.NewChannel("#test1") - - // Assert that it has no topic originally - if chan1.Topic != "" { - t.Errorf("Test channel already has a topic.") - } - - // Send a TOPIC line - s.st.EXPECT().GetChannel("#test1").Return(chan1) + // Ensure TOPIC reply calls Topic + gomock.InOrder( + s.st.EXPECT().GetChannel("#test1").Return(&state.Channel{Name: "#test1"}), + s.st.EXPECT().Topic("#test1", "something something"), + ) c.h_TOPIC(ParseLine(":user1!ident1@host1.com TOPIC #test1 :something something")) - // Make sure the channel's topic has been changed - if chan1.Topic != "something something" { - t.Errorf("Topic of test channel not set correctly.") - } - // Check error paths -- send a topic for an unknown channel s.st.EXPECT().GetChannel("#test2").Return(nil) c.h_TOPIC(ParseLine(":user1!ident1@host1.com TOPIC #test2 :dark side")) @@ -338,20 +312,14 @@ func Test311(t *testing.T) { c, s := setUp(t) defer s.tearDown() - // Create user1, who we know little about - nick1 := state.NewNick("user1") - - // Send a 311 reply - s.st.EXPECT().GetNick("user1").Return(nick1) + // Ensure 311 reply calls NickInfo + gomock.InOrder( + s.st.EXPECT().GetNick("user1").Return(&state.Nick{Nick: "user1"}), + s.st.EXPECT().Me().Return(c.cfg.Me), + s.st.EXPECT().NickInfo("user1", "ident1", "host1.com", "name"), + ) c.h_311(ParseLine(":irc.server.org 311 test user1 ident1 host1.com * :name")) - // Verify we now know more about user1 - if nick1.Ident != "ident1" || - nick1.Host != "host1.com" || - nick1.Name != "name" { - t.Errorf("WHOIS info of user1 not set correctly.") - } - // Check error paths -- send a 311 for an unknown nick s.st.EXPECT().GetNick("user2").Return(nil) c.h_311(ParseLine(":irc.server.org 311 test user2 ident2 host2.com * :dongs")) @@ -362,15 +330,12 @@ func Test324(t *testing.T) { c, s := setUp(t) defer s.tearDown() - // Create #test1, whose modes we don't know - chan1 := state.NewChannel("#test1") - - // Send a 324 reply - s.st.EXPECT().GetChannel("#test1").Return(chan1) + // Ensure 324 reply calls ChannelModes + gomock.InOrder( + s.st.EXPECT().GetChannel("#test1").Return(&state.Channel{Name: "#test1"}), + s.st.EXPECT().ChannelModes("#test1", "+sk", "somekey"), + ) c.h_324(ParseLine(":irc.server.org 324 test #test1 +sk somekey")) - if !chan1.Modes.Secret || chan1.Modes.Key != "somekey" { - t.Errorf("Channel.ParseModes() not called correctly.") - } // Check error paths -- send 324 for an unknown channel s.st.EXPECT().GetChannel("#test2").Return(nil) @@ -382,23 +347,13 @@ func Test332(t *testing.T) { c, s := setUp(t) defer s.tearDown() - // Create #test1, whose topic we don't know - chan1 := state.NewChannel("#test1") - - // Assert that it has no topic originally - if chan1.Topic != "" { - t.Errorf("Test channel already has a topic.") - } - - // Send a 332 reply - s.st.EXPECT().GetChannel("#test1").Return(chan1) + // Ensure 332 reply calls Topic + gomock.InOrder( + s.st.EXPECT().GetChannel("#test1").Return(&state.Channel{Name: "#test1"}), + s.st.EXPECT().Topic("#test1", "something something"), + ) c.h_332(ParseLine(":irc.server.org 332 test #test1 :something something")) - // Make sure the channel's topic has been changed - if chan1.Topic != "something something" { - t.Errorf("Topic of test channel not set correctly.") - } - // 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,30 +364,24 @@ func Test352(t *testing.T) { c, s := setUp(t) defer s.tearDown() - // Create user1, who we know little about - nick1 := state.NewNick("user1") - - // Send a 352 reply - s.st.EXPECT().GetNick("user1").Return(nick1) + // Ensure 352 reply calls NickInfo and NickModes + gomock.InOrder( + s.st.EXPECT().GetNick("user1").Return(&state.Nick{Nick: "user1"}), + s.st.EXPECT().Me().Return(c.cfg.Me), + s.st.EXPECT().NickInfo("user1", "ident1", "host1.com", "name"), + ) c.h_352(ParseLine(":irc.server.org 352 test #test1 ident1 host1.com irc.server.org user1 G :0 name")) - // Verify we now know more about user1 - if nick1.Ident != "ident1" || - nick1.Host != "host1.com" || - nick1.Name != "name" || - nick1.Modes.Invisible || - nick1.Modes.Oper { - t.Errorf("WHO info of user1 not set correctly.") - } - // Check that modes are set correctly from WHOREPLY - s.st.EXPECT().GetNick("user1").Return(nick1) + gomock.InOrder( + s.st.EXPECT().GetNick("user1").Return(&state.Nick{Nick: "user1"}), + s.st.EXPECT().Me().Return(c.cfg.Me), + s.st.EXPECT().NickInfo("user1", "ident1", "host1.com", "name"), + s.st.EXPECT().NickModes("user1", "+o"), + s.st.EXPECT().NickModes("user1", "+i"), + ) c.h_352(ParseLine(":irc.server.org 352 test #test1 ident1 host1.com irc.server.org user1 H* :0 name")) - if !nick1.Modes.Invisible || !nick1.Modes.Oper { - t.Errorf("WHO modes of user1 not set correctly.") - } - // Check error paths -- send a 352 for an unknown nick 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")) @@ -443,55 +392,42 @@ func Test353(t *testing.T) { c, s := setUp(t) defer s.tearDown() - // Create #test1, whose user list we're mostly unfamiliar with - chan1 := state.NewChannel("#test1") - - // Create maps for testing -- this is what the mock ST calls will return - nicks := make(map[string]*state.Nick) - privs := make(map[string]*state.ChanPrivs) - - nicks["test"] = c.cfg.Me - privs["test"] = new(state.ChanPrivs) - - for _, n := range []string{"user1", "user2", "voice", "halfop", - "op", "admin", "owner"} { - nicks[n] = state.NewNick(n) - privs[n] = new(state.ChanPrivs) - } - // 353 handler is called twice, so GetChannel will be called twice - s.st.EXPECT().GetChannel("#test1").Return(chan1).Times(2) + s.st.EXPECT().GetChannel("#test1").Return(&state.Channel{Name: "#test1"}).Times(2) gomock.InOrder( // "test" is Me, i am known, and already on the channel s.st.EXPECT().GetNick("test").Return(c.cfg.Me), - s.st.EXPECT().IsOn("#test1", "test").Return(privs["test"], true), + s.st.EXPECT().IsOn("#test1", "test").Return(&state.ChanPrivs{}, true), // user1 is known, but not on the channel, so should be associated - s.st.EXPECT().GetNick("user1").Return(nicks["user1"]), + s.st.EXPECT().GetNick("user1").Return(&state.Nick{Nick: "user1"}), s.st.EXPECT().IsOn("#test1", "user1").Return(nil, false), - s.st.EXPECT().Associate(chan1, nicks["user1"]).Return(privs["user1"]), + s.st.EXPECT().Associate("#test1", "user1").Return(&state.ChanPrivs{}), + s.st.EXPECT().ChannelModes("#test1", "+o", "user1"), ) - for _, n := range []string{"user2", "voice", "halfop", "op", "admin", "owner"} { - gomock.InOrder( + for n, m := range map[string]string{ + "user2": "", + "voice": "+v", + "halfop": "+h", + "op": "+o", + "admin": "+a", + "owner": "+q", + } { + calls := []*gomock.Call{ s.st.EXPECT().GetNick(n).Return(nil), - s.st.EXPECT().NewNick(n).Return(nicks[n]), + s.st.EXPECT().NewNick(n).Return(&state.Nick{Nick: n}), s.st.EXPECT().IsOn("#test1", n).Return(nil, false), - s.st.EXPECT().Associate(chan1, nicks[n]).Return(privs[n]), - ) + s.st.EXPECT().Associate("#test1", n).Return(&state.ChanPrivs{}), + } + if m != "" { + calls = append(calls, s.st.EXPECT().ChannelModes("#test1", m, n)) + } + gomock.InOrder(calls...) } // Send a couple of names replies (complete with trailing space) c.h_353(ParseLine(":irc.server.org 353 test = #test1 :test @user1 user2 +voice ")) c.h_353(ParseLine(":irc.server.org 353 test = #test1 :%halfop @op &admin ~owner ")) - if p := privs["user2"]; p.Voice || p.HalfOp || p.Op || p.Admin || p.Owner { - t.Errorf("353 handler incorrectly set modes on nick.") - } - - if !privs["user1"].Op || !privs["voice"].Voice || !privs["halfop"].HalfOp || - !privs["op"].Op || !privs["admin"].Admin || !privs["owner"].Owner { - t.Errorf("353 handler failed to set correct modes for nicks.") - } - // 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")) @@ -502,21 +438,13 @@ func Test671(t *testing.T) { c, s := setUp(t) defer s.tearDown() - // Create user1, who should not be secure - nick1 := state.NewNick("user1") - if nick1.Modes.SSL { - t.Errorf("Test nick user1 is already using SSL?") - } - - // Send a 671 reply - s.st.EXPECT().GetNick("user1").Return(nick1) + // Ensure 671 reply calls NickModes + gomock.InOrder( + s.st.EXPECT().GetNick("user1").Return(&state.Nick{Nick: "user1"}), + s.st.EXPECT().NickModes("user1", "+z"), + ) c.h_671(ParseLine(":irc.server.org 671 test user1 :some ignored text")) - // Ensure user1 is now known to be on an SSL connection - if !nick1.Modes.SSL { - t.Errorf("Test nick user1 not using SSL?") - } - // Check error paths -- send a 671 for an unknown nick 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 5bff1cf..3c62957 100644 --- a/client/state_handlers.go +++ b/client/state_handlers.go @@ -51,44 +51,41 @@ func (conn *Conn) h_JOIN(line *Line) { if ch == nil { // 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.cfg.Me { + if !conn.Me().Equals(nk) { logging.Warn("irc.JOIN(): JOIN to unknown channel %s received "+ "from (non-me) nick %s", line.Args[0], line.Nick) return } - ch = conn.st.NewChannel(line.Args[0]) + conn.st.NewChannel(line.Args[0]) // since we don't know much about this channel, ask server for info // we get the channel users automatically in 353 and the channel // topic in 332 on join, so we just need to get the modes - conn.Mode(ch.Name) + conn.Mode(line.Args[0]) // sending a WHO for the channel is MUCH more efficient than // triggering a WHOIS on every nick from the 353 handler - conn.Who(ch.Name) + conn.Who(line.Args[0]) } if nk == nil { // this is the first we've seen of this nick - nk = conn.st.NewNick(line.Nick) - nk.Ident = line.Ident - nk.Host = line.Host + conn.st.NewNick(line.Nick) + conn.st.NickInfo(line.Nick, line.Ident, line.Host, "") // since we don't know much about this nick, ask server for info - conn.Who(nk.Nick) + conn.Who(line.Nick) } // this takes care of both nick and channel linking \o/ - conn.st.Associate(ch, nk) + conn.st.Associate(line.Args[0], line.Nick) } // Handle PARTs from channels to maintain state func (conn *Conn) h_PART(line *Line) { - conn.st.Dissociate(conn.st.GetChannel(line.Args[0]), - conn.st.GetNick(line.Nick)) + conn.st.Dissociate(line.Args[0], line.Nick) } // Handle KICKs from channels to maintain state func (conn *Conn) h_KICK(line *Line) { // XXX: this won't handle autorejoining channels on KICK // it's trivial to do this in a seperate handler... - conn.st.Dissociate(conn.st.GetChannel(line.Args[0]), - conn.st.GetNick(line.Args[1])) + conn.st.Dissociate(line.Args[0], line.Args[1]) } // Handle other people's QUITs @@ -100,15 +97,15 @@ func (conn *Conn) h_QUIT(line *Line) { func (conn *Conn) h_MODE(line *Line) { if ch := conn.st.GetChannel(line.Args[0]); ch != nil { // channel modes first - ch.ParseModes(line.Args[1], line.Args[2:]...) + conn.st.ChannelModes(line.Args[0], line.Args[1], line.Args[2:]...) } else if nk := conn.st.GetNick(line.Args[0]); nk != nil { // nick mode change, should be us - if nk != conn.cfg.Me { + if !conn.Me().Equals(nk) { logging.Warn("irc.MODE(): recieved MODE %s for (non-me) nick %s", line.Args[1], line.Args[0]) return } - nk.ParseModes(line.Args[1]) + conn.st.NickModes(line.Args[0], line.Args[1]) } else { logging.Warn("irc.MODE(): not sure what to do with MODE %s", strings.Join(line.Args, " ")) @@ -118,7 +115,7 @@ func (conn *Conn) h_MODE(line *Line) { // Handle TOPIC changes for channels func (conn *Conn) h_TOPIC(line *Line) { if ch := conn.st.GetChannel(line.Args[0]); ch != nil { - ch.Topic = line.Args[1] + conn.st.Topic(line.Args[0], line.Args[1]) } else { logging.Warn("irc.TOPIC(): topic change on unknown channel %s", line.Args[0]) @@ -127,10 +124,8 @@ func (conn *Conn) h_TOPIC(line *Line) { // Handle 311 whois reply func (conn *Conn) h_311(line *Line) { - if nk := conn.st.GetNick(line.Args[1]); nk != nil && nk != conn.Me() { - nk.Ident = line.Args[2] - nk.Host = line.Args[3] - nk.Name = line.Args[5] + if nk := conn.st.GetNick(line.Args[1]); nk != nil && !conn.Me().Equals(nk) { + conn.st.NickInfo(line.Args[1], line.Args[2], line.Args[3], line.Args[5]) } else { logging.Warn("irc.311(): received WHOIS info for unknown nick %s", line.Args[1]) @@ -140,7 +135,7 @@ func (conn *Conn) h_311(line *Line) { // Handle 324 mode reply 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:]...) + conn.st.ChannelModes(line.Args[1], line.Args[2], line.Args[3:]...) } else { logging.Warn("irc.324(): received MODE settings for unknown channel %s", line.Args[1]) @@ -150,7 +145,7 @@ func (conn *Conn) h_324(line *Line) { // Handle 332 topic reply on join to channel func (conn *Conn) h_332(line *Line) { if ch := conn.st.GetChannel(line.Args[1]); ch != nil { - ch.Topic = line.Args[2] + conn.st.Topic(line.Args[1], line.Args[2]) } else { logging.Warn("irc.332(): received TOPIC value for unknown channel %s", line.Args[1]) @@ -165,24 +160,22 @@ func (conn *Conn) h_352(line *Line) { line.Args[5]) return } - if nk == conn.Me() { + if conn.Me().Equals(nk) { return } - nk.Ident = line.Args[2] - nk.Host = line.Args[3] // XXX: do we care about the actual server the nick is on? // or the hop count to this server? // last arg contains " " a := strings.SplitN(line.Args[len(line.Args)-1], " ", 2) - nk.Name = a[1] + conn.st.NickInfo(nk.Nick, line.Args[2], line.Args[3], a[1]) if idx := strings.Index(line.Args[6], "*"); idx != -1 { - nk.Modes.Oper = true + conn.st.NickModes(nk.Nick, "+o") } if idx := strings.Index(line.Args[6], "B"); idx != -1 { - nk.Modes.Bot = true + conn.st.NickModes(nk.Nick, "+B") } if idx := strings.Index(line.Args[6], "H"); idx != -1 { - nk.Modes.Invisible = true + conn.st.NickModes(nk.Nick, "+i") } } @@ -200,27 +193,25 @@ func (conn *Conn) h_353(line *Line) { nick = nick[1:] fallthrough default: - nk := conn.st.GetNick(nick) - if nk == nil { + if conn.st.GetNick(nick) == nil { // we don't know this nick yet! - nk = conn.st.NewNick(nick) + conn.st.NewNick(nick) } - cp, ok := conn.st.IsOn(ch.Name, nick) - if !ok { + if _, ok := conn.st.IsOn(ch.Name, nick); !ok { // This nick isn't associated with this channel yet! - cp = conn.st.Associate(ch, nk) + conn.st.Associate(ch.Name, nick) } switch c { case '~': - cp.Owner = true + conn.st.ChannelModes(ch.Name, "+q", nick) case '&': - cp.Admin = true + conn.st.ChannelModes(ch.Name, "+a", nick) case '@': - cp.Op = true + conn.st.ChannelModes(ch.Name, "+o", nick) case '%': - cp.HalfOp = true + conn.st.ChannelModes(ch.Name, "+h", nick) case '+': - cp.Voice = true + conn.st.ChannelModes(ch.Name, "+v", nick) } } } @@ -233,7 +224,7 @@ func (conn *Conn) h_353(line *Line) { // Handle 671 whois reply (nick connected via SSL) func (conn *Conn) h_671(line *Line) { if nk := conn.st.GetNick(line.Args[1]); nk != nil { - nk.Modes.SSL = true + conn.st.NickModes(nk.Nick, "+z") } else { logging.Warn("irc.671(): received WHOIS SSL info for unknown nick %s", line.Args[1]) diff --git a/state/channel.go b/state/channel.go index 9bc1e5f..35379d2 100644 --- a/state/channel.go +++ b/state/channel.go @@ -3,18 +3,24 @@ package state import ( "github.com/fluffle/goirc/logging" - "fmt" "reflect" - "sort" "strconv" ) -// A struct representing an IRC channel +// A Channel is returned from the state tracker and contains +// a copy of the channel state at a particular time. type Channel struct { Name, Topic string Modes *ChanMode - lookup map[string]*Nick - nicks map[*Nick]*ChanPrivs + Nicks map[string]*ChanPrivs +} + +// Internal bookkeeping struct for channels. +type channel struct { + name, topic string + modes *ChanMode + lookup map[string]*nick + nicks map[*nick]*ChanPrivs } // A struct representing the modes of an IRC Channel @@ -98,48 +104,57 @@ func init() { * Channel methods for state management \******************************************************************************/ -func NewChannel(name string) *Channel { - return &Channel{ - Name: name, - Modes: new(ChanMode), - nicks: make(map[*Nick]*ChanPrivs), - lookup: make(map[string]*Nick), +func newChannel(name string) *channel { + return &channel{ + name: name, + modes: new(ChanMode), + nicks: make(map[*nick]*ChanPrivs), + lookup: make(map[string]*nick), } } -// Returns true if the Nick is associated with the Channel -func (ch *Channel) IsOn(nk *Nick) (*ChanPrivs, bool) { - cp, ok := ch.nicks[nk] - return cp, ok +// Returns a copy of the internal tracker channel state at this time. +// Relies on tracker-level locking for concurrent access. +func (ch *channel) Channel() *Channel { + c := &Channel{ + Name: ch.name, + Topic: ch.topic, + Modes: ch.modes.Copy(), + Nicks: make(map[string]*ChanPrivs), + } + for n, cp := range ch.nicks { + c.Nicks[n.nick] = cp.Copy() + } + return c } -func (ch *Channel) IsOnStr(n string) (*Nick, bool) { - nk, ok := ch.lookup[n] - return nk, ok +func (ch *channel) isOn(nk *nick) (*ChanPrivs, bool) { + cp, ok := ch.nicks[nk] + return cp.Copy(), ok } // Associates a Nick with a Channel -func (ch *Channel) addNick(nk *Nick, cp *ChanPrivs) { +func (ch *channel) addNick(nk *nick, cp *ChanPrivs) { if _, ok := ch.nicks[nk]; !ok { ch.nicks[nk] = cp - ch.lookup[nk.Nick] = nk + ch.lookup[nk.nick] = nk } else { - logging.Warn("Channel.addNick(): %s already on %s.", nk.Nick, ch.Name) + logging.Warn("Channel.addNick(): %s already on %s.", nk.nick, ch.name) } } // Disassociates a Nick from a Channel. -func (ch *Channel) delNick(nk *Nick) { +func (ch *channel) delNick(nk *nick) { if _, ok := ch.nicks[nk]; ok { delete(ch.nicks, nk) - delete(ch.lookup, nk.Nick) + delete(ch.lookup, nk.nick) } else { - logging.Warn("Channel.delNick(): %s not on %s.", nk.Nick, ch.Name) + logging.Warn("Channel.delNick(): %s not on %s.", nk.nick, ch.name) } } // Parses mode strings for a channel. -func (ch *Channel) ParseModes(modes string, modeargs ...string) { +func (ch *channel) parseModes(modes string, modeargs ...string) { var modeop bool // true => add mode, false => remove mode var modestr string for i := 0; i < len(modes); i++ { @@ -151,43 +166,43 @@ func (ch *Channel) ParseModes(modes string, modeargs ...string) { modeop = false modestr = string(m) case 'i': - ch.Modes.InviteOnly = modeop + ch.modes.InviteOnly = modeop case 'm': - ch.Modes.Moderated = modeop + ch.modes.Moderated = modeop case 'n': - ch.Modes.NoExternalMsg = modeop + ch.modes.NoExternalMsg = modeop case 'p': - ch.Modes.Private = modeop + ch.modes.Private = modeop case 'r': - ch.Modes.Registered = modeop + ch.modes.Registered = modeop case 's': - ch.Modes.Secret = modeop + ch.modes.Secret = modeop case 't': - ch.Modes.ProtectedTopic = modeop + ch.modes.ProtectedTopic = modeop case 'z': - ch.Modes.SSLOnly = modeop + ch.modes.SSLOnly = modeop case 'Z': - ch.Modes.AllSSL = modeop + ch.modes.AllSSL = modeop case 'O': - ch.Modes.OperOnly = modeop + ch.modes.OperOnly = modeop case 'k': if modeop && len(modeargs) != 0 { - ch.Modes.Key, modeargs = modeargs[0], modeargs[1:] + ch.modes.Key, modeargs = modeargs[0], modeargs[1:] } else if !modeop { - ch.Modes.Key = "" + ch.modes.Key = "" } else { logging.Warn("Channel.ParseModes(): not enough arguments to "+ - "process MODE %s %s%c", ch.Name, modestr, m) + "process MODE %s %s%c", ch.name, modestr, m) } case 'l': if modeop && len(modeargs) != 0 { - ch.Modes.Limit, _ = strconv.Atoi(modeargs[0]) + ch.modes.Limit, _ = strconv.Atoi(modeargs[0]) modeargs = modeargs[1:] } else if !modeop { - ch.Modes.Limit = 0 + ch.modes.Limit = 0 } else { logging.Warn("Channel.ParseModes(): not enough arguments to "+ - "process MODE %s %s%c", ch.Name, modestr, m) + "process MODE %s %s%c", ch.name, modestr, m) } case 'q', 'a', 'o', 'h', 'v': if len(modeargs) != 0 { @@ -208,11 +223,11 @@ func (ch *Channel) ParseModes(modes string, modeargs ...string) { modeargs = modeargs[1:] } else { logging.Warn("Channel.ParseModes(): untracked nick %s "+ - "received MODE on channel %s", modeargs[0], ch.Name) + "received MODE on channel %s", modeargs[0], ch.name) } } else { logging.Warn("Channel.ParseModes(): not enough arguments to "+ - "process MODE %s %s%c", ch.Name, modestr, m) + "process MODE %s %s%c", ch.name, modestr, m) } default: logging.Info("Channel.ParseModes(): unknown mode char %c", m) @@ -220,29 +235,39 @@ func (ch *Channel) ParseModes(modes string, modeargs ...string) { } } -type byNick []*Nick - -func (b byNick) Len() int { return len(b) } -func (b byNick) Less(i, j int) bool { return b[i].Nick < b[j].Nick } -func (b byNick) Swap(i, j int) { b[i], b[j] = b[j], b[i] } - -// Nicks returns a list of *Nick that are on the channel, sorted by nick. -func (ch *Channel) Nicks() []*Nick { - nicks := make([]*Nick, 0, len(ch.lookup)) - for _, nick := range ch.lookup { - nicks = append(nicks, nick) - } - sort.Sort(byNick(nicks)) - return nicks +// Returns true if the Nick is associated with the Channel +func (ch *Channel) IsOn(nk string) (*ChanPrivs, bool) { + cp, ok := ch.Nicks[nk] + return cp, ok } -// NicksStr returns a list of nick strings that are on the channel, sorted by nick. -func (ch *Channel) NicksStr() []string { - var nicks []string - for _, nick := range ch.Nicks() { - nicks = append(nicks, nick.Nick) - } - return nicks +// Test Channel equality. +func (ch *Channel) Equals(other *Channel) bool { + return reflect.DeepEqual(ch, other) +} + +// Duplicates a ChanMode struct. +func (cm *ChanMode) Copy() *ChanMode { + if cm == nil { return nil } + c := *cm + return &c +} + +// Test ChanMode equality. +func (cm *ChanMode) Equals(other *ChanMode) bool { + return reflect.DeepEqual(cm, other) +} + +// Duplicates a ChanPrivs struct. +func (cp *ChanPrivs) Copy() *ChanPrivs { + if cp == nil { return nil } + c := *cp + return &c +} + +// Test ChanPrivs equality. +func (cp *ChanPrivs) Equals(other *ChanPrivs) bool { + return reflect.DeepEqual(cp, other) } // Returns a string representing the channel. Looks like: @@ -257,12 +282,16 @@ func (ch *Channel) String() string { str += "Topic: " + ch.Topic + "\n\t" str += "Modes: " + ch.Modes.String() + "\n\t" str += "Nicks: \n" - for nk, cp := range ch.nicks { - str += "\t\t" + nk.Nick + ": " + cp.String() + "\n" + for nk, cp := range ch.Nicks { + str += "\t\t" + nk + ": " + cp.String() + "\n" } return str } +func (ch *channel) String() string { + return ch.Channel().String() +} + // Returns a string representing the channel modes. Looks like: // +npk key func (cm *ChanMode) String() string { @@ -284,7 +313,7 @@ func (cm *ChanMode) String() string { case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: if f.Int() != 0 { str += ChanModeToString[t.Field(i).Name] - a = append(a, fmt.Sprintf("%d", f.Int())) + a = append(a, strconv.FormatInt(f.Int(), 10)) } } } diff --git a/state/channel_test.go b/state/channel_test.go index f714044..d5c804d 100644 --- a/state/channel_test.go +++ b/state/channel_test.go @@ -1,23 +1,35 @@ package state -import ( - "testing" -) +import "testing" + +func compareChannel(t *testing.T, ch *channel) { + c := ch.Channel() + if c.Name != ch.name || c.Topic != ch.topic || + !c.Modes.Equals(ch.modes) || len(c.Nicks) != len(ch.nicks) { + t.Errorf("Channel not duped correctly from internal state.") + } + for nk, cp := range ch.nicks { + if other, ok := c.Nicks[nk.nick]; !ok || !cp.Equals(other) { + t.Errorf("Nick not duped correctly from internal state.") + } + } +} func TestNewChannel(t *testing.T) { - ch := NewChannel("#test1") + ch := newChannel("#test1") - if ch.Name != "#test1" { + if ch.name != "#test1" { t.Errorf("Channel not created correctly by NewChannel()") } if len(ch.nicks) != 0 || len(ch.lookup) != 0 { t.Errorf("Channel maps contain data after NewChannel()") } + compareChannel(t, ch) } func TestAddNick(t *testing.T) { - ch := NewChannel("#test1") - nk := NewNick("test1") + ch := newChannel("#test1") + nk := newNick("test1") cp := new(ChanPrivs) ch.addNick(nk, cp) @@ -31,11 +43,12 @@ func TestAddNick(t *testing.T) { if n, ok := ch.lookup["test1"]; !ok || n != nk { t.Errorf("Nick test1 not properly stored in lookup map.") } + compareChannel(t, ch) } func TestDelNick(t *testing.T) { - ch := NewChannel("#test1") - nk := NewNick("test1") + ch := newChannel("#test1") + nk := newNick("test1") cp := new(ChanPrivs) ch.addNick(nk, cp) @@ -49,18 +62,20 @@ func TestDelNick(t *testing.T) { if n, ok := ch.lookup["#test1"]; ok || n != nil { t.Errorf("Nick test1 not properly removed from lookup map.") } + compareChannel(t, ch) } func TestChannelParseModes(t *testing.T) { - ch := NewChannel("#test1") - md := ch.Modes + ch := newChannel("#test1") + md := ch.modes // Channel modes can adjust channel privs too, so we need a Nick - nk := NewNick("test1") + nk := newNick("test1") cp := new(ChanPrivs) ch.addNick(nk, cp) // Test bools first. + compareChannel(t, ch) if md.Private || md.Secret || md.ProtectedTopic || md.NoExternalMsg || md.Moderated || md.InviteOnly || md.OperOnly || md.SSLOnly { t.Errorf("Modes for new channel set to true.") @@ -72,8 +87,9 @@ func TestChannelParseModes(t *testing.T) { md.InviteOnly = true // Flip some MOAR bits. - ch.ParseModes("+s-p+tm-i") + ch.parseModes("+s-p+tm-i") + compareChannel(t, ch) if md.Private || !md.Secret || !md.ProtectedTopic || !md.NoExternalMsg || !md.Moderated || md.InviteOnly || md.OperOnly || md.SSLOnly { t.Errorf("Modes not flipped correctly by ParseModes.") @@ -85,19 +101,22 @@ func TestChannelParseModes(t *testing.T) { } // enable limit correctly - ch.ParseModes("+l", "256") + ch.parseModes("+l", "256") + compareChannel(t, ch) if md.Limit != 256 { t.Errorf("Limit for channel not set correctly") } // enable limit incorrectly - ch.ParseModes("+l") + ch.parseModes("+l") + compareChannel(t, ch) if md.Limit != 256 { t.Errorf("Bad limit value caused limit to be unset.") } // disable limit correctly - ch.ParseModes("-l") + ch.parseModes("-l") + compareChannel(t, ch) if md.Limit != 0 { t.Errorf("Limit for channel not unset correctly") } @@ -108,19 +127,22 @@ func TestChannelParseModes(t *testing.T) { } // enable key correctly - ch.ParseModes("+k", "foobar") + ch.parseModes("+k", "foobar") + compareChannel(t, ch) if md.Key != "foobar" { t.Errorf("Key for channel not set correctly") } // enable key incorrectly - ch.ParseModes("+k") + ch.parseModes("+k") + compareChannel(t, ch) if md.Key != "foobar" { t.Errorf("Bad key value caused key to be unset.") } // disable key correctly - ch.ParseModes("-k") + ch.parseModes("-k") + compareChannel(t, ch) if md.Key != "" { t.Errorf("Key for channel not unset correctly") } @@ -128,16 +150,18 @@ func TestChannelParseModes(t *testing.T) { // Test chan privs parsing. cp.Op = true cp.HalfOp = true - ch.ParseModes("+aq-o", "test1", "test1", "test1") + ch.parseModes("+aq-o", "test1", "test1", "test1") + compareChannel(t, ch) if !cp.Owner || !cp.Admin || cp.Op || !cp.HalfOp || cp.Voice { t.Errorf("Channel privileges not flipped correctly by ParseModes.") } // Test a random mix of modes, just to be sure md.Limit = 256 - ch.ParseModes("+zpt-qsl+kv-h", "test1", "foobar", "test1") + ch.parseModes("+zpt-qsl+kv-h", "test1", "foobar", "test1") + compareChannel(t, ch) if !md.Private || md.Secret || !md.ProtectedTopic || !md.NoExternalMsg || !md.Moderated || md.InviteOnly || md.OperOnly || !md.SSLOnly { t.Errorf("Modes not flipped correctly by ParseModes (2).") diff --git a/state/mock_tracker.go b/state/mock_tracker.go index 310b5b7..1902230 100644 --- a/state/mock_tracker.go +++ b/state/mock_tracker.go @@ -48,22 +48,46 @@ func (_mr *_MockTrackerRecorder) GetNick(arg0 interface{}) *gomock.Call { return _mr.mock.ctrl.RecordCall(_mr.mock, "GetNick", arg0) } -func (_m *MockTracker) ReNick(old string, neu string) { - _m.ctrl.Call(_m, "ReNick", old, neu) +func (_m *MockTracker) ReNick(old string, neu string) *Nick { + ret := _m.ctrl.Call(_m, "ReNick", old, neu) + ret0, _ := ret[0].(*Nick) + return ret0 } func (_mr *_MockTrackerRecorder) ReNick(arg0, arg1 interface{}) *gomock.Call { return _mr.mock.ctrl.RecordCall(_mr.mock, "ReNick", arg0, arg1) } -func (_m *MockTracker) DelNick(nick string) { - _m.ctrl.Call(_m, "DelNick", nick) +func (_m *MockTracker) DelNick(nick string) *Nick { + ret := _m.ctrl.Call(_m, "DelNick", nick) + ret0, _ := ret[0].(*Nick) + return ret0 } func (_mr *_MockTrackerRecorder) DelNick(arg0 interface{}) *gomock.Call { return _mr.mock.ctrl.RecordCall(_mr.mock, "DelNick", arg0) } +func (_m *MockTracker) NickInfo(nick string, ident string, host string, name string) *Nick { + ret := _m.ctrl.Call(_m, "NickInfo", nick, ident, host, name) + ret0, _ := ret[0].(*Nick) + return ret0 +} + +func (_mr *_MockTrackerRecorder) NickInfo(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { + return _mr.mock.ctrl.RecordCall(_mr.mock, "NickInfo", arg0, arg1, arg2, arg3) +} + +func (_m *MockTracker) NickModes(nick string, modestr string) *Nick { + ret := _m.ctrl.Call(_m, "NickModes", nick, modestr) + ret0, _ := ret[0].(*Nick) + return ret0 +} + +func (_mr *_MockTrackerRecorder) NickModes(arg0, arg1 interface{}) *gomock.Call { + return _mr.mock.ctrl.RecordCall(_mr.mock, "NickModes", arg0, arg1) +} + func (_m *MockTracker) NewChannel(channel string) *Channel { ret := _m.ctrl.Call(_m, "NewChannel", channel) ret0, _ := ret[0].(*Channel) @@ -84,14 +108,41 @@ func (_mr *_MockTrackerRecorder) GetChannel(arg0 interface{}) *gomock.Call { return _mr.mock.ctrl.RecordCall(_mr.mock, "GetChannel", arg0) } -func (_m *MockTracker) DelChannel(channel string) { - _m.ctrl.Call(_m, "DelChannel", channel) +func (_m *MockTracker) DelChannel(channel string) *Channel { + ret := _m.ctrl.Call(_m, "DelChannel", channel) + ret0, _ := ret[0].(*Channel) + return ret0 } func (_mr *_MockTrackerRecorder) DelChannel(arg0 interface{}) *gomock.Call { return _mr.mock.ctrl.RecordCall(_mr.mock, "DelChannel", arg0) } +func (_m *MockTracker) Topic(channel string, topic string) *Channel { + ret := _m.ctrl.Call(_m, "Topic", channel, topic) + ret0, _ := ret[0].(*Channel) + return ret0 +} + +func (_mr *_MockTrackerRecorder) Topic(arg0, arg1 interface{}) *gomock.Call { + return _mr.mock.ctrl.RecordCall(_mr.mock, "Topic", arg0, arg1) +} + +func (_m *MockTracker) ChannelModes(channel string, modestr string, modeargs ...string) *Channel { + _s := []interface{}{channel, modestr} + for _, _x := range modeargs { + _s = append(_s, _x) + } + ret := _m.ctrl.Call(_m, "ChannelModes", _s...) + ret0, _ := ret[0].(*Channel) + return ret0 +} + +func (_mr *_MockTrackerRecorder) ChannelModes(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + _s := append([]interface{}{arg0, arg1}, arg2...) + return _mr.mock.ctrl.RecordCall(_mr.mock, "ChannelModes", _s...) +} + func (_m *MockTracker) Me() *Nick { ret := _m.ctrl.Call(_m, "Me") ret0, _ := ret[0].(*Nick) @@ -113,7 +164,7 @@ func (_mr *_MockTrackerRecorder) IsOn(arg0, arg1 interface{}) *gomock.Call { return _mr.mock.ctrl.RecordCall(_mr.mock, "IsOn", arg0, arg1) } -func (_m *MockTracker) Associate(channel *Channel, nick *Nick) *ChanPrivs { +func (_m *MockTracker) Associate(channel string, nick string) *ChanPrivs { ret := _m.ctrl.Call(_m, "Associate", channel, nick) ret0, _ := ret[0].(*ChanPrivs) return ret0 @@ -123,7 +174,7 @@ func (_mr *_MockTrackerRecorder) Associate(arg0, arg1 interface{}) *gomock.Call return _mr.mock.ctrl.RecordCall(_mr.mock, "Associate", arg0, arg1) } -func (_m *MockTracker) Dissociate(channel *Channel, nick *Nick) { +func (_m *MockTracker) Dissociate(channel string, nick string) { _m.ctrl.Call(_m, "Dissociate", channel, nick) } diff --git a/state/nick.go b/state/nick.go index 0683fb9..7704b0b 100644 --- a/state/nick.go +++ b/state/nick.go @@ -4,15 +4,22 @@ import ( "github.com/fluffle/goirc/logging" "reflect" - "sort" ) -// A struct representing an IRC nick +// A Nick is returned from the state tracker and contains +// a copy of the nick state at a particular time. type Nick struct { Nick, Ident, Host, Name string Modes *NickMode - lookup map[string]*Channel - chans map[*Channel]*ChanPrivs + Channels map[string]*ChanPrivs +} + +// Internal bookkeeping struct for nicks. +type nick struct { + nick, ident, host, name string + modes *NickMode + lookup map[string]*channel + chans map[*channel]*ChanPrivs } // A struct representing the modes of an IRC Nick (User Modes) @@ -43,51 +50,62 @@ func init() { } /******************************************************************************\ - * Nick methods for state management + * nick methods for state management \******************************************************************************/ -func NewNick(n string) *Nick { - return &Nick{ - Nick: n, - Modes: new(NickMode), - chans: make(map[*Channel]*ChanPrivs), - lookup: make(map[string]*Channel), +func newNick(n string) *nick { + return &nick{ + nick: n, + modes: new(NickMode), + chans: make(map[*channel]*ChanPrivs), + lookup: make(map[string]*channel), } } -// Returns true if the Nick is associated with the Channel. -func (nk *Nick) IsOn(ch *Channel) (*ChanPrivs, bool) { - cp, ok := nk.chans[ch] - return cp, ok +// Returns a copy of the internal tracker nick state at this time. +// Relies on tracker-level locking for concurrent access. +func (nk *nick) Nick() *Nick { + n := &Nick{ + Nick: nk.nick, + Ident: nk.ident, + Host: nk.host, + Name: nk.name, + Modes: nk.modes.Copy(), + Channels: make(map[string]*ChanPrivs), + } + for c, cp := range nk.chans { + n.Channels[c.name] = cp.Copy() + } + return n } -func (nk *Nick) IsOnStr(c string) (*Channel, bool) { - ch, ok := nk.lookup[c] - return ch, ok +func (nk *nick) isOn(ch *channel) (*ChanPrivs, bool) { + cp, ok := nk.chans[ch] + return cp.Copy(), ok } // Associates a Channel with a Nick. -func (nk *Nick) addChannel(ch *Channel, cp *ChanPrivs) { +func (nk *nick) addChannel(ch *channel, cp *ChanPrivs) { if _, ok := nk.chans[ch]; !ok { nk.chans[ch] = cp - nk.lookup[ch.Name] = ch + nk.lookup[ch.name] = ch } else { - logging.Warn("Nick.addChannel(): %s already on %s.", nk.Nick, ch.Name) + logging.Warn("Nick.addChannel(): %s already on %s.", nk.nick, ch.name) } } // Disassociates a Channel from a Nick. -func (nk *Nick) delChannel(ch *Channel) { +func (nk *nick) delChannel(ch *channel) { if _, ok := nk.chans[ch]; ok { delete(nk.chans, ch) - delete(nk.lookup, ch.Name) + delete(nk.lookup, ch.name) } else { - logging.Warn("Nick.delChannel(): %s not on %s.", nk.Nick, ch.Name) + logging.Warn("Nick.delChannel(): %s not on %s.", nk.nick, ch.name) } } // Parse mode strings for a Nick. -func (nk *Nick) ParseModes(modes string) { +func (nk *nick) parseModes(modes string) { var modeop bool // true => add mode, false => remove mode for i := 0; i < len(modes); i++ { switch m := modes[i]; m { @@ -96,46 +114,44 @@ func (nk *Nick) ParseModes(modes string) { case '-': modeop = false case 'B': - nk.Modes.Bot = modeop + nk.modes.Bot = modeop case 'i': - nk.Modes.Invisible = modeop + nk.modes.Invisible = modeop case 'o': - nk.Modes.Oper = modeop + nk.modes.Oper = modeop case 'w': - nk.Modes.WallOps = modeop + nk.modes.WallOps = modeop case 'x': - nk.Modes.HiddenHost = modeop + nk.modes.HiddenHost = modeop case 'z': - nk.Modes.SSL = modeop + nk.modes.SSL = modeop default: logging.Info("Nick.ParseModes(): unknown mode char %c", m) } } } -type byName []*Channel - -func (b byName) Len() int { return len(b) } -func (b byName) Less(i, j int) bool { return b[i].Name < b[j].Name } -func (b byName) Swap(i, j int) { b[i], b[j] = b[j], b[i] } - -// Channels returns a list of *Channel the nick is on, sorted by name. -func (nk *Nick) Channels() []*Channel { - channels := make([]*Channel, 0, len(nk.lookup)) - for _, channel := range nk.lookup { - channels = append(channels, channel) - } - sort.Sort(byName(channels)) - return channels +// Returns true if the Nick is associated with the Channel. +func (nk *Nick) IsOn(ch string) (*ChanPrivs, bool) { + cp, ok := nk.Channels[ch] + return cp, ok } -// ChannelsStr returns a list of channel strings the nick is on, sorted by name. -func (nk *Nick) ChannelsStr() []string { - var names []string - for _, channel := range nk.Channels() { - names = append(names, channel.Name) - } - return names +// Tests Nick equality. +func (nk *Nick) Equals(other *Nick) bool { + return reflect.DeepEqual(nk, other) +} + +// Duplicates a NickMode struct. +func (nm *NickMode) Copy() *NickMode { + if nm == nil { return nil } + n := *nm + return &n +} + +// Tests NickMode equality. +func (nm *NickMode) Equals(other *NickMode) bool { + return reflect.DeepEqual(nm, other) } // Returns a string representing the nick. Looks like: @@ -152,12 +168,16 @@ func (nk *Nick) String() string { str += "Real Name: " + nk.Name + "\n\t" str += "Modes: " + nk.Modes.String() + "\n\t" str += "Channels: \n" - for ch, cp := range nk.chans { - str += "\t\t" + ch.Name + ": " + cp.String() + "\n" + for ch, cp := range nk.Channels { + str += "\t\t" + ch + ": " + cp.String() + "\n" } return str } +func (nk *nick) String() string { + return nk.Nick().String() +} + // Returns a string representing the nick modes. Looks like: // +iwx func (nm *NickMode) String() string { diff --git a/state/nick_test.go b/state/nick_test.go index c01ec1a..1344400 100644 --- a/state/nick_test.go +++ b/state/nick_test.go @@ -1,23 +1,35 @@ package state -import ( - "testing" -) +import "testing" + +func compareNick(t *testing.T, nk *nick) { + n := nk.Nick() + if n.Nick != nk.nick || n.Ident != nk.ident || n.Host != nk.host || n.Name != nk.name || + !n.Modes.Equals(nk.modes) || len(n.Channels) != len(nk.chans) { + t.Errorf("Nick not duped correctly from internal state.") + } + for ch, cp := range nk.chans { + if other, ok := n.Channels[ch.name]; !ok || !cp.Equals(other) { + t.Errorf("Channel not duped correctly from internal state.") + } + } +} func TestNewNick(t *testing.T) { - nk := NewNick("test1") + nk := newNick("test1") - if nk.Nick != "test1" { + if nk.nick != "test1" { t.Errorf("Nick not created correctly by NewNick()") } if len(nk.chans) != 0 || len(nk.lookup) != 0 { t.Errorf("Nick maps contain data after NewNick()") } + compareNick(t, nk) } func TestAddChannel(t *testing.T) { - nk := NewNick("test1") - ch := NewChannel("#test1") + nk := newNick("test1") + ch := newChannel("#test1") cp := new(ChanPrivs) nk.addChannel(ch, cp) @@ -31,11 +43,12 @@ func TestAddChannel(t *testing.T) { if c, ok := nk.lookup["#test1"]; !ok || c != ch { t.Errorf("Channel #test1 not properly stored in lookup map.") } + compareNick(t, nk) } func TestDelChannel(t *testing.T) { - nk := NewNick("test1") - ch := NewChannel("#test1") + nk := newNick("test1") + ch := newChannel("#test1") cp := new(ChanPrivs) nk.addChannel(ch, cp) @@ -49,11 +62,12 @@ func TestDelChannel(t *testing.T) { if c, ok := nk.lookup["#test1"]; ok || c != nil { t.Errorf("Channel #test1 not properly removed from lookup map.") } + compareNick(t, nk) } func TestNickParseModes(t *testing.T) { - nk := NewNick("test1") - md := nk.Modes + nk := newNick("test1") + md := nk.modes // Modes should all be false for a new nick if md.Invisible || md.Oper || md.WallOps || md.HiddenHost || md.SSL { @@ -65,8 +79,9 @@ func TestNickParseModes(t *testing.T) { md.HiddenHost = true // Parse a mode line that flips one true to false and two false to true - nk.ParseModes("+z-x+w") + nk.parseModes("+z-x+w") + compareNick(t, nk) if !md.Invisible || md.Oper || !md.WallOps || md.HiddenHost || !md.SSL { t.Errorf("Modes not flipped correctly by ParseModes.") } diff --git a/state/tracker.go b/state/tracker.go index b666dfd..686fedc 100644 --- a/state/tracker.go +++ b/state/tracker.go @@ -2,6 +2,8 @@ package state import ( "github.com/fluffle/goirc/logging" + + "sync" ) // The state manager interface @@ -9,18 +11,22 @@ type Tracker interface { // Nick methods NewNick(nick string) *Nick GetNick(nick string) *Nick - ReNick(old, neu string) - DelNick(nick string) + ReNick(old, neu string) *Nick + DelNick(nick string) *Nick + NickInfo(nick, ident, host, name string) *Nick + NickModes(nick, modestr string) *Nick // Channel methods NewChannel(channel string) *Channel GetChannel(channel string) *Channel - DelChannel(channel string) + DelChannel(channel string) *Channel + Topic(channel, topic string) *Channel + ChannelModes(channel, modestr string, modeargs ...string) *Channel // Information about ME! Me() *Nick // And the tracking operations IsOn(channel, nick string) (*ChanPrivs, bool) - Associate(channel *Channel, nick *Nick) *ChanPrivs - Dissociate(channel *Channel, nick *Nick) + Associate(channel, nick string) *ChanPrivs + Dissociate(channel, nick string) Wipe() // The state tracker can output a debugging string String() string @@ -29,26 +35,34 @@ type Tracker interface { // ... and a struct to implement it ... type stateTracker struct { // Map of channels we're on - chans map[string]*Channel + chans map[string]*channel // Map of nicks we know about - nicks map[string]*Nick + nicks map[string]*nick // We need to keep state on who we are :-) - me *Nick + me *nick + + // And we need to protect against data races *cough*. + mu sync.Mutex } +var _ Tracker = (*stateTracker)(nil) + // ... and a constructor to make it ... func NewTracker(mynick string) *stateTracker { st := &stateTracker{ - chans: make(map[string]*Channel), - nicks: make(map[string]*Nick), + chans: make(map[string]*channel), + nicks: make(map[string]*nick), } - st.me = st.NewNick(mynick) + st.me = newNick(mynick) + st.nicks[mynick] = st.me return st } // ... and a method to wipe the state clean. func (st *stateTracker) Wipe() { + st.mu.Lock() + defer st.mu.Unlock() // Deleting all the channels implicitly deletes every nick but me. for _, ch := range st.chans { st.delChannel(ch) @@ -59,67 +73,84 @@ func (st *stateTracker) Wipe() { * tracker methods to create/look up nicks/channels \******************************************************************************/ -// Creates a new Nick, initialises it, and stores it so it +// Creates a new nick, initialises it, and stores it so it // can be properly tracked for state management purposes. func (st *stateTracker) NewNick(n string) *Nick { + if n == "" { + logging.Warn("Tracker.NewNick(): Not tracking empty nick.") + return nil + } + st.mu.Lock() + defer st.mu.Unlock() if _, ok := st.nicks[n]; ok { logging.Warn("Tracker.NewNick(): %s already tracked.", n) return nil } - st.nicks[n] = NewNick(n) - return st.nicks[n] + st.nicks[n] = newNick(n) + return st.nicks[n].Nick() } -// Returns a Nick for the nick n, if we're tracking it. +// Returns a nick for the nick n, if we're tracking it. func (st *stateTracker) GetNick(n string) *Nick { + st.mu.Lock() + defer st.mu.Unlock() if nk, ok := st.nicks[n]; ok { - return nk + return nk.Nick() } return nil } -// Signals to the tracker that a Nick should be tracked +// Signals to the tracker that a nick should be tracked // under a "neu" nick rather than the old one. -func (st *stateTracker) ReNick(old, neu string) { - if nk, ok := st.nicks[old]; ok { - if _, ok := st.nicks[neu]; !ok { - nk.Nick = neu - delete(st.nicks, old) - st.nicks[neu] = nk - for ch, _ := range nk.chans { - // We also need to update the lookup maps of all the channels - // the nick is on, to keep things in sync. - delete(ch.lookup, old) - ch.lookup[neu] = nk - } - } else { - logging.Warn("Tracker.ReNick(): %s already exists.", neu) - } - } else { +func (st *stateTracker) ReNick(old, neu string) *Nick { + st.mu.Lock() + defer st.mu.Unlock() + nk, ok := st.nicks[old] + if !ok { logging.Warn("Tracker.ReNick(): %s not tracked.", old) + return nil } + if _, ok := st.nicks[neu]; ok { + logging.Warn("Tracker.ReNick(): %s already exists.", neu) + return nil + } + + nk.nick = neu + delete(st.nicks, old) + st.nicks[neu] = nk + for ch, _ := range nk.chans { + // We also need to update the lookup maps of all the channels + // the nick is on, to keep things in sync. + delete(ch.lookup, old) + ch.lookup[neu] = nk + } + return nk.Nick() } -// Removes a Nick from being tracked. -func (st *stateTracker) DelNick(n string) { +// Removes a nick from being tracked. +func (st *stateTracker) DelNick(n string) *Nick { + st.mu.Lock() + defer st.mu.Unlock() if nk, ok := st.nicks[n]; ok { - if nk != st.me { - st.delNick(nk) - } else { + if nk == st.me { logging.Warn("Tracker.DelNick(): won't delete myself.") + return nil } - } else { - logging.Warn("Tracker.DelNick(): %s not tracked.", n) + st.delNick(nk) + return nk.Nick() } + logging.Warn("Tracker.DelNick(): %s not tracked.", n) + return nil } -func (st *stateTracker) delNick(nk *Nick) { +func (st *stateTracker) delNick(nk *nick) { + // st.mu lock held by DelNick, DelChannel or Wipe if nk == st.me { // Shouldn't get here => internal state tracking code is fubar. logging.Error("Tracker.DelNick(): TRYING TO DELETE ME :-(") return } - delete(st.nicks, nk.Nick) + delete(st.nicks, nk.nick) for ch, _ := range nk.chans { nk.delChannel(ch) ch.delNick(nk) @@ -127,41 +158,79 @@ func (st *stateTracker) delNick(nk *Nick) { // Deleting a nick from tracking shouldn't empty any channels as // *we* should be on the channel with them to be tracking them. logging.Error("Tracker.delNick(): deleting nick %s emptied "+ - "channel %s, this shouldn't happen!", nk.Nick, ch.Name) + "channel %s, this shouldn't happen!", nk.nick, ch.name) } } } +// Sets ident, host and "real" name for the nick. +func (st *stateTracker) NickInfo(n, ident, host, name string) *Nick { + st.mu.Lock() + defer st.mu.Unlock() + nk, ok := st.nicks[n] + if !ok { + return nil + } + nk.ident = ident + nk.host = host + nk.name = name + return nk.Nick() +} + +// Sets user modes for the nick. +func (st *stateTracker) NickModes(n, modes string) *Nick { + st.mu.Lock() + defer st.mu.Unlock() + nk, ok := st.nicks[n] + if !ok { + return nil + } + nk.parseModes(modes) + return nk.Nick() +} + // Creates a new Channel, initialises it, and stores it so it // can be properly tracked for state management purposes. func (st *stateTracker) NewChannel(c string) *Channel { + if c == "" { + logging.Warn("Tracker.NewChannel(): Not tracking empty channel.") + return nil + } + st.mu.Lock() + defer st.mu.Unlock() if _, ok := st.chans[c]; ok { logging.Warn("Tracker.NewChannel(): %s already tracked.", c) return nil } - st.chans[c] = NewChannel(c) - return st.chans[c] + st.chans[c] = newChannel(c) + return st.chans[c].Channel() } // Returns a Channel for the channel c, if we're tracking it. func (st *stateTracker) GetChannel(c string) *Channel { + st.mu.Lock() + defer st.mu.Unlock() if ch, ok := st.chans[c]; ok { - return ch + return ch.Channel() } return nil } // Removes a Channel from being tracked. -func (st *stateTracker) DelChannel(c string) { +func (st *stateTracker) DelChannel(c string) *Channel { + st.mu.Lock() + defer st.mu.Unlock() if ch, ok := st.chans[c]; ok { st.delChannel(ch) - } else { - logging.Warn("Tracker.DelChannel(): %s not tracked.", c) + return ch.Channel() } + logging.Warn("Tracker.DelChannel(): %s not tracked.", c) + return nil } -func (st *stateTracker) delChannel(ch *Channel) { - delete(st.chans, ch.Name) +func (st *stateTracker) delChannel(ch *channel) { + // st.mu lock held by DelChannel or Wipe + delete(st.chans, ch.name) for nk, _ := range ch.nicks { ch.delNick(nk) nk.delChannel(ch) @@ -172,67 +241,98 @@ func (st *stateTracker) delChannel(ch *Channel) { } } +// Sets the topic of a channel. +func (st *stateTracker) Topic(c, topic string) *Channel { + st.mu.Lock() + defer st.mu.Unlock() + ch, ok := st.chans[c] + if !ok { + return nil + } + ch.topic = topic + return ch.Channel() +} + +// Sets modes for a channel, including privileges like +o. +func (st *stateTracker) ChannelModes(c, modes string, args ...string) *Channel { + st.mu.Lock() + defer st.mu.Unlock() + ch, ok := st.chans[c] + if !ok { + return nil + } + ch.parseModes(modes, args...) + return ch.Channel() +} + // Returns the Nick the state tracker thinks is Me. func (st *stateTracker) Me() *Nick { - return st.me + return st.me.Nick() } // Returns true if both the channel c and the nick n are tracked // and the nick is associated with the channel. func (st *stateTracker) IsOn(c, n string) (*ChanPrivs, bool) { - nk := st.GetNick(n) - ch := st.GetChannel(c) - if nk != nil && ch != nil { - return nk.IsOn(ch) + st.mu.Lock() + defer st.mu.Unlock() + nk, nok := st.nicks[n] + ch, cok := st.chans[c] + if nok && cok { + return nk.isOn(ch) } return nil, false } // Associates an already known nick with an already known channel. -func (st *stateTracker) Associate(ch *Channel, nk *Nick) *ChanPrivs { - if ch == nil || nk == nil { - logging.Error("Tracker.Associate(): passed nil values :-(") - return nil - } else if _ch, ok := st.chans[ch.Name]; !ok || ch != _ch { +func (st *stateTracker) Associate(c, n string) *ChanPrivs { + st.mu.Lock() + defer st.mu.Unlock() + nk, nok := st.nicks[n] + ch, cok := st.chans[c] + + if !cok { // 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. logging.Error("Tracker.Associate(): channel %s not found in "+ - "(or differs from) internal state.", ch.Name) + "internal state.", c) return nil - } else if _nk, ok := st.nicks[nk.Nick]; !ok || nk != _nk { + } else if !nok { logging.Error("Tracker.Associate(): nick %s not found in "+ - "(or differs from) internal state.", nk.Nick) + "internal state.", n) return nil - } else if _, ok := nk.IsOn(ch); ok { + } else if _, ok := nk.isOn(ch); ok { logging.Warn("Tracker.Associate(): %s already on %s.", - nk.Nick, ch.Name) + nk, ch) return nil } cp := new(ChanPrivs) ch.addNick(nk, cp) nk.addChannel(ch, cp) - return cp + return cp.Copy() } // Dissociates an already known nick from an already known channel. // Does some tidying up to stop tracking nicks we're no longer on // any common channels with, and channels we're no longer on. -func (st *stateTracker) Dissociate(ch *Channel, nk *Nick) { - if ch == nil || nk == nil { - logging.Error("Tracker.Dissociate(): passed nil values :-(") - } else if _ch, ok := st.chans[ch.Name]; !ok || ch != _ch { +func (st *stateTracker) Dissociate(c, n string) { + st.mu.Lock() + defer st.mu.Unlock() + nk, nok := st.nicks[n] + ch, cok := st.chans[c] + + if !cok { // 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. logging.Error("Tracker.Dissociate(): channel %s not found in "+ - "(or differs from) internal state.", ch.Name) - } else if _nk, ok := st.nicks[nk.Nick]; !ok || nk != _nk { + "internal state.", c) + } else if !nok { logging.Error("Tracker.Dissociate(): nick %s not found in "+ - "(or differs from) internal state.", nk.Nick) - } else if _, ok := nk.IsOn(ch); !ok { + "internal state.", n) + } else if _, ok := nk.isOn(ch); !ok { logging.Warn("Tracker.Dissociate(): %s not on %s.", - nk.Nick, ch.Name) + nk.nick, ch.name) } else if nk == st.me { // I'm leaving the channel for some reason, so it won't be tracked. st.delChannel(ch) @@ -248,6 +348,8 @@ func (st *stateTracker) Dissociate(ch *Channel, nk *Nick) { } func (st *stateTracker) String() string { + st.mu.Lock() + defer st.mu.Unlock() str := "GoIRC Channels\n" str += "--------------\n\n" for _, ch := range st.chans { diff --git a/state/tracker_test.go b/state/tracker_test.go index 9ae2c19..6330098 100644 --- a/state/tracker_test.go +++ b/state/tracker_test.go @@ -1,6 +1,15 @@ package state -import "testing" +import ( + "sync" + "testing" +) + +// There is some awkwardness in these tests. Items retrieved directly from the +// state trackers internal maps are private and only have private, +// uncaptialised members. Items retrieved from state tracker public interface +// methods are public and only have public, capitalised members. Comparisons of +// the two are done on the basis of nick or channel name. func TestSTNewTracker(t *testing.T) { st := NewTracker("mynick") @@ -11,7 +20,7 @@ func TestSTNewTracker(t *testing.T) { if len(st.chans) != 0 { t.Errorf("Channel list of new tracker is not empty.") } - if nk, ok := st.nicks["mynick"]; !ok || nk.Nick != "mynick" || nk != st.me { + if nk, ok := st.nicks["mynick"]; !ok || nk.nick != "mynick" || nk != st.me { t.Errorf("My nick not stored correctly in tracker.") } } @@ -23,20 +32,23 @@ func TestSTNewNick(t *testing.T) { 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 { + if n, ok := st.nicks["test1"]; !ok || !test1.Equals(n.Nick()) || len(st.nicks) != 2 { t.Errorf("Nick object stored incorrectly by NewNick.") } if fail := st.NewNick("test1"); fail != nil { t.Errorf("Creating duplicate nick did not produce nil return.") } + if fail := st.NewNick(""); fail != nil { + t.Errorf("Creating empty nick did not produce nil return.") + } } func TestSTGetNick(t *testing.T) { st := NewTracker("mynick") test1 := st.NewNick("test1") - if n := st.GetNick("test1"); n != test1 { + if n := st.GetNick("test1"); !test1.Equals(n) { t.Errorf("Incorrect nick returned by GetNick.") } if n := st.GetNick("test2"); n != nil { @@ -52,39 +64,53 @@ func TestSTReNick(t *testing.T) { test1 := st.NewNick("test1") // This channel is here to ensure that its lookup map gets updated - chan1 := st.NewChannel("#chan1") - st.Associate(chan1, test1) + st.NewChannel("#chan1") + st.Associate("#chan1", "test1") - st.ReNick("test1", "test2") + // We need to check out the manipulation of the internals. + n1 := st.nicks["test1"] + c1 := st.chans["#chan1"] + + test2 := st.ReNick("test1", "test2") if _, ok := st.nicks["test1"]; ok { t.Errorf("Nick test1 still exists after ReNick.") } - if n, ok := st.nicks["test2"]; !ok || n != test1 { + if n, ok := st.nicks["test2"]; !ok || n != n1 { t.Errorf("Nick test2 doesn't exist after ReNick.") } - if _, ok := chan1.lookup["test1"]; ok { + if _, ok := c1.lookup["test1"]; ok { t.Errorf("Channel #chan1 still knows about test1 after ReNick.") } - if n, ok := chan1.lookup["test2"]; !ok || n != test1 { + if n, ok := c1.lookup["test2"]; !ok || n != n1 { t.Errorf("Channel #chan1 doesn't know about test2 after ReNick.") } - if test1.Nick != "test2" { - t.Errorf("Nick test1 not changed correctly.") + if test1.Nick != "test1" { + t.Errorf("Nick test1 changed unexpectedly.") + } + if !test2.Equals(n1.Nick()) { + t.Errorf("Nick test2 did not change.") } if len(st.nicks) != 2 { t.Errorf("Nick list changed size during ReNick.") } + if len(c1.lookup) != 1 { + t.Errorf("Channel lookup list changed size during ReNick.") + } - test2 := st.NewNick("test1") - st.ReNick("test1", "test2") + st.NewNick("test1") + n2 := st.nicks["test1"] + fail := st.ReNick("test1", "test2") - if n, ok := st.nicks["test2"]; !ok || n != test1 { + if n, ok := st.nicks["test2"]; !ok || n != n1 { t.Errorf("Nick test2 overwritten/deleted by ReNick.") } - if n, ok := st.nicks["test1"]; !ok || n != test2 { + if n, ok := st.nicks["test1"]; !ok || n != n2 { t.Errorf("Nick test1 overwritten/deleted by ReNick.") } + if fail != nil { + t.Errorf("ReNick returned Nick on failure.") + } if len(st.nicks) != 3 { t.Errorf("Nick list changed size during ReNick.") } @@ -93,8 +119,8 @@ func TestSTReNick(t *testing.T) { func TestSTDelNick(t *testing.T) { st := NewTracker("mynick") - st.NewNick("test1") - st.DelNick("test1") + add := st.NewNick("test1") + del := st.DelNick("test1") if _, ok := st.nicks["test1"]; ok { t.Errorf("Nick test1 still exists after DelNick.") @@ -102,55 +128,106 @@ func TestSTDelNick(t *testing.T) { if len(st.nicks) != 1 { t.Errorf("Nick list still contains nicks after DelNick.") } + if !add.Equals(del) { + t.Errorf("DelNick returned different nick.") + } // 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") - st.DelNick("test2") - if len(st.nicks) != 2 { + st.NewNick("test1") + fail := st.DelNick("test2") + if fail != nil || len(st.nicks) != 2 { t.Errorf("Deleting unknown nick had unexpected side-effects.") } // Deleting my nick shouldn't work - st.DelNick("mynick") - if len(st.nicks) != 2 { + fail = st.DelNick("mynick") + if fail != nil || len(st.nicks) != 2 { t.Errorf("Deleting myself had unexpected side-effects.") } // Test that deletion correctly dissociates nick from channels. // NOTE: the two error states in delNick (as opposed to DelNick) // are not tested for here, as they will only arise from programming - // errors in other methods. The mock logger should catch these. + // errors in other methods. // Create a new channel for testing purposes. - chan1 := st.NewChannel("#test1") + st.NewChannel("#test1") // Associate both "my" nick and test1 with the channel - st.Associate(chan1, st.me) - st.Associate(chan1, nick1) + st.Associate("#test1", "mynick") + st.Associate("#test1", "test1") + + // We need to check out the manipulation of the internals. + n1 := st.nicks["test1"] + c1 := st.chans["#test1"] // Test we have the expected starting state (at least vaguely) - if len(chan1.nicks) != 2 || len(st.nicks) != 2 || - len(st.me.chans) != 1 || len(nick1.chans) != 1 || len(st.chans) != 1 { + if len(c1.nicks) != 2 || len(st.nicks) != 2 || + len(st.me.chans) != 1 || len(n1.chans) != 1 || len(st.chans) != 1 { t.Errorf("Bad initial state for test DelNick() channel dissociation.") } + // Actual deletion tested above... st.DelNick("test1") - // Actual deletion tested above... - if len(chan1.nicks) != 1 || len(st.chans) != 1 || - len(st.me.chans) != 1 || len(nick1.chans) != 0 || len(st.chans) != 1 { + if len(c1.nicks) != 1 || len(st.nicks) != 1 || + len(st.me.chans) != 1 || len(n1.chans) != 0 || len(st.chans) != 1 { t.Errorf("Deleting nick didn't dissociate correctly from channels.") } - if _, ok := chan1.nicks[nick1]; ok { + if _, ok := c1.nicks[n1]; ok { t.Errorf("Nick not removed from channel's nick map.") } - if _, ok := chan1.lookup["test1"]; ok { + if _, ok := c1.lookup["test1"]; ok { t.Errorf("Nick not removed from channel's lookup map.") } } +func TestSTNickInfo(t *testing.T) { + st := NewTracker("mynick") + test1 := st.NewNick("test1") + test2 := st.NickInfo("test1", "foo", "bar", "baz") + test3 := st.GetNick("test1") + + if test1.Equals(test2) { + t.Errorf("NickInfo did not return modified nick.") + } + if !test3.Equals(test2) { + t.Errorf("Getting nick after NickInfo returned different nick.") + } + test1.Ident, test1.Host, test1.Name = "foo", "bar", "baz" + if !test1.Equals(test2) { + t.Errorf("NickInfo did not set nick info correctly.") + } + + if fail := st.NickInfo("test2", "foo", "bar", "baz"); fail != nil { + t.Errorf("NickInfo for nonexistent nick did not return nil.") + } +} + +func TestSTNickModes(t *testing.T) { + st := NewTracker("mynick") + test1 := st.NewNick("test1") + test2 := st.NickModes("test1", "+iB") + test3 := st.GetNick("test1") + + if test1.Equals(test2) { + t.Errorf("NickModes did not return modified nick.") + } + if !test3.Equals(test2) { + t.Errorf("Getting nick after NickModes returned different nick.") + } + test1.Modes.Invisible, test1.Modes.Bot = true, true + if !test1.Equals(test2) { + t.Errorf("NickModes did not set nick modes correctly.") + } + + if fail := st.NickModes("test2", "whatevs"); fail != nil { + t.Errorf("NickModes for nonexistent nick did not return nil.") + } +} + func TestSTNewChannel(t *testing.T) { st := NewTracker("mynick") @@ -163,13 +240,16 @@ func TestSTNewChannel(t *testing.T) { 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 { + if c, ok := st.chans["#test1"]; !ok || !test1.Equals(c.Channel()) || len(st.chans) != 1 { t.Errorf("Channel object stored incorrectly by NewChannel.") } if fail := st.NewChannel("#test1"); fail != nil { t.Errorf("Creating duplicate chan did not produce nil return.") } + if fail := st.NewChannel(""); fail != nil { + t.Errorf("Creating empty chan did not produce nil return.") + } } func TestSTGetChannel(t *testing.T) { @@ -177,7 +257,7 @@ func TestSTGetChannel(t *testing.T) { test1 := st.NewChannel("#test1") - if c := st.GetChannel("#test1"); c != test1 { + if c := st.GetChannel("#test1"); !test1.Equals(c) { t.Errorf("Incorrect Channel returned by GetChannel.") } if c := st.GetChannel("#test2"); c != nil { @@ -191,8 +271,8 @@ func TestSTGetChannel(t *testing.T) { func TestSTDelChannel(t *testing.T) { st := NewTracker("mynick") - st.NewChannel("#test1") - st.DelChannel("#test1") + add := st.NewChannel("#test1") + del := st.DelChannel("#test1") if _, ok := st.chans["#test1"]; ok { t.Errorf("Channel test1 still exists after DelChannel.") @@ -200,30 +280,38 @@ func TestSTDelChannel(t *testing.T) { if len(st.chans) != 0 { t.Errorf("Channel list still contains chans after DelChannel.") } + if !add.Equals(del) { + t.Errorf("DelChannel returned different channel.") + } - // 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") - st.DelChannel("#test2") - if len(st.chans) != 1 { + // Deleting unknown channel shouldn't work, but let's make sure we have a + // known channel first to catch any possible accidental removals. + st.NewChannel("#test1") + fail := st.DelChannel("#test2") + if fail != nil || len(st.chans) != 1 { t.Errorf("DelChannel had unexpected side-effects.") } // Test that deletion correctly dissociates channel from tracked nicks. // In order to test this thoroughly we need two channels (so that delNick() // is not called internally in delChannel() when len(nick1.chans) == 0. - chan2 := st.NewChannel("#test2") - nick1 := st.NewNick("test1") + st.NewChannel("#test2") + st.NewNick("test1") // Associate both "my" nick and test1 with the channels - st.Associate(chan1, st.me) - st.Associate(chan1, nick1) - st.Associate(chan2, st.me) - st.Associate(chan2, nick1) + st.Associate("#test1", "mynick") + st.Associate("#test1", "test1") + st.Associate("#test2", "mynick") + st.Associate("#test2", "test1") + + // We need to check out the manipulation of the internals. + n1 := st.nicks["test1"] + c1 := st.chans["#test1"] + c2 := st.chans["#test2"] // Test we have the expected starting state (at least vaguely) - if len(chan1.nicks) != 2 || len(chan2.nicks) != 2 || len(st.nicks) != 2 || - len(st.me.chans) != 2 || len(nick1.chans) != 2 || len(st.chans) != 2 { + if len(c1.nicks) != 2 || len(c2.nicks) != 2 || len(st.nicks) != 2 || + len(st.me.chans) != 2 || len(n1.chans) != 2 || len(st.chans) != 2 { t.Errorf("Bad initial state for test DelChannel() nick dissociation.") } @@ -231,14 +319,14 @@ func TestSTDelChannel(t *testing.T) { // Test intermediate state. We're still on #test2 with test1, so test1 // shouldn't be deleted from state tracking itself just yet. - if len(chan1.nicks) != 0 || len(chan2.nicks) != 2 || len(st.nicks) != 2 || - len(st.me.chans) != 1 || len(nick1.chans) != 1 || len(st.chans) != 1 { + if len(c1.nicks) != 0 || len(c2.nicks) != 2 || len(st.nicks) != 2 || + len(st.me.chans) != 1 || len(n1.chans) != 1 || len(st.chans) != 1 { t.Errorf("Deleting channel didn't dissociate correctly from nicks.") } - if _, ok := nick1.chans[chan1]; ok { + if _, ok := n1.chans[c1]; ok { t.Errorf("Channel not removed from nick's chans map.") } - if _, ok := nick1.lookup["#test1"]; ok { + if _, ok := n1.lookup["#test1"]; ok { t.Errorf("Channel not removed from nick's lookup map.") } @@ -246,8 +334,8 @@ func TestSTDelChannel(t *testing.T) { // Test final state. Deleting #test2 means that we're no longer on any // common channels with test1, and thus it should be removed from tracking. - if len(chan1.nicks) != 0 || len(chan2.nicks) != 0 || len(st.nicks) != 1 || - len(st.me.chans) != 0 || len(nick1.chans) != 0 || len(st.chans) != 0 { + if len(c1.nicks) != 0 || len(c2.nicks) != 0 || len(st.nicks) != 1 || + len(st.me.chans) != 0 || len(n1.chans) != 0 || len(st.chans) != 0 { t.Errorf("Deleting last channel didn't dissociate correctly from nicks.") } if _, ok := st.nicks["test1"]; ok { @@ -258,17 +346,61 @@ func TestSTDelChannel(t *testing.T) { } } +func TestSTTopic(t *testing.T) { + st := NewTracker("mynick") + test1 := st.NewChannel("#test1") + test2 := st.Topic("#test1", "foo bar") + test3 := st.GetChannel("#test1") + + if test1.Equals(test2) { + t.Errorf("Topic did not return modified channel.") + } + if !test3.Equals(test2) { + t.Errorf("Getting channel after Topic returned different channel.") + } + test1.Topic = "foo bar" + if !test1.Equals(test2) { + t.Errorf("Topic did not set channel topic correctly.") + } + + if fail := st.Topic("#test2", "foo baz"); fail != nil { + t.Errorf("Topic for nonexistent channel did not return nil.") + } +} + +func TestSTChannelModes(t *testing.T) { + st := NewTracker("mynick") + test1 := st.NewChannel("#test1") + test2 := st.ChannelModes("#test1", "+sk", "foo") + test3 := st.GetChannel("#test1") + + if test1.Equals(test2) { + t.Errorf("ChannelModes did not return modified channel.") + } + if !test3.Equals(test2) { + t.Errorf("Getting channel after ChannelModes returned different channel.") + } + test1.Modes.Secret, test1.Modes.Key = true, "foo" + if !test1.Equals(test2) { + t.Errorf("ChannelModes did not set channel modes correctly.") + } + + if fail := st.ChannelModes("test2", "whatevs"); fail != nil { + t.Errorf("ChannelModes for nonexistent channel did not return nil.") + } +} + func TestSTIsOn(t *testing.T) { st := NewTracker("mynick") - nick1 := st.NewNick("test1") - chan1 := st.NewChannel("#test1") + st.NewNick("test1") + st.NewChannel("#test1") if priv, ok := st.IsOn("#test1", "test1"); ok || priv != nil { t.Errorf("test1 is not on #test1 (yet)") } - cp := st.Associate(chan1, nick1) - if priv, ok := st.IsOn("#test1", "test1"); !ok || priv != cp { + st.Associate("#test1", "test1") + if priv, ok := st.IsOn("#test1", "test1"); !ok || priv == nil { t.Errorf("test1 is on #test1 (now)") } } @@ -276,117 +408,135 @@ func TestSTIsOn(t *testing.T) { func TestSTAssociate(t *testing.T) { st := NewTracker("mynick") - nick1 := st.NewNick("test1") - chan1 := st.NewChannel("#test1") + st.NewNick("test1") + st.NewChannel("#test1") - cp := st.Associate(chan1, nick1) - if priv, ok := nick1.chans[chan1]; !ok || cp != priv { + // We need to check out the manipulation of the internals. + n1 := st.nicks["test1"] + c1 := st.chans["#test1"] + + st.Associate("#test1", "test1") + npriv, nok := n1.chans[c1] + cpriv, cok := c1.nicks[n1] + if !nok || !cok || npriv != cpriv { t.Errorf("#test1 was not associated with test1.") } - if priv, ok := chan1.nicks[nick1]; !ok || cp != priv { - t.Errorf("test1 was not associated with #test1.") - } // Test error cases - if st.Associate(nil, nick1) != nil { - t.Errorf("Associating nil *Channel did not return nil.") + if st.Associate("", "test1") != nil { + t.Errorf("Associating unknown channel did not return nil.") } - if st.Associate(chan1, nil) != nil { - t.Errorf("Associating nil *Nick did not return nil.") + if st.Associate("#test1", "") != nil { + t.Errorf("Associating unknown nick did not return nil.") } - if st.Associate(chan1, nick1) != nil { + if st.Associate("#test1", "test1") != 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 := NewTracker("mynick") - nick1 := st.NewNick("test1") - chan1 := st.NewChannel("#test1") - chan2 := st.NewChannel("#test2") + st.NewNick("test1") + st.NewChannel("#test1") + st.NewChannel("#test2") // Associate both "my" nick and test1 with the channels - st.Associate(chan1, st.me) - st.Associate(chan1, nick1) - st.Associate(chan2, st.me) - st.Associate(chan2, nick1) + st.Associate("#test1", "mynick") + st.Associate("#test1", "test1") + st.Associate("#test2", "mynick") + st.Associate("#test2", "test1") + + // We need to check out the manipulation of the internals. + n1 := st.nicks["test1"] + c1 := st.chans["#test1"] + c2 := st.chans["#test2"] // Check the initial state looks mostly like we expect it to. - if len(chan1.nicks) != 2 || len(chan2.nicks) != 2 || len(st.nicks) != 2 || - len(st.me.chans) != 2 || len(nick1.chans) != 2 || len(st.chans) != 2 { + if len(c1.nicks) != 2 || len(c2.nicks) != 2 || len(st.nicks) != 2 || + len(st.me.chans) != 2 || len(n1.chans) != 2 || len(st.chans) != 2 { t.Errorf("Initial state for dissociation tests looks odd.") } // First, test the case of me leaving #test2 - st.Dissociate(chan2, st.me) + st.Dissociate("#test2", "mynick") // This should have resulted in the complete deletion of the channel. - if len(chan1.nicks) != 2 || len(chan2.nicks) != 0 || len(st.nicks) != 2 || - len(st.me.chans) != 1 || len(nick1.chans) != 1 || len(st.chans) != 1 { + if len(c1.nicks) != 2 || len(c2.nicks) != 0 || len(st.nicks) != 2 || + len(st.me.chans) != 1 || len(n1.chans) != 1 || len(st.chans) != 1 { t.Errorf("Dissociating myself from channel didn't delete it correctly.") } + if st.GetChannel("#test2") != nil { + t.Errorf("Able to get channel after dissociating myself.") + } // Reassociating myself and test1 to #test2 shouldn't cause any errors. - chan2 = st.NewChannel("#test2") - st.Associate(chan2, st.me) - st.Associate(chan2, nick1) + st.NewChannel("#test2") + st.Associate("#test2", "mynick") + st.Associate("#test2", "test1") + + // c2 is out of date with the complete deletion of the channel + c2 = st.chans["#test2"] // Check state once moar. - if len(chan1.nicks) != 2 || len(chan2.nicks) != 2 || len(st.nicks) != 2 || - len(st.me.chans) != 2 || len(nick1.chans) != 2 || len(st.chans) != 2 { + if len(c1.nicks) != 2 || len(c2.nicks) != 2 || len(st.nicks) != 2 || + len(st.me.chans) != 2 || len(n1.chans) != 2 || len(st.chans) != 2 { t.Errorf("Reassociating to channel has produced unexpected state.") } // Now, lets dissociate test1 from #test1 then #test2. // This first one should only result in a change in associations. - st.Dissociate(chan1, nick1) + st.Dissociate("#test1", "test1") - if len(chan1.nicks) != 1 || len(chan2.nicks) != 2 || len(st.nicks) != 2 || - len(st.me.chans) != 2 || len(nick1.chans) != 1 || len(st.chans) != 2 { + if len(c1.nicks) != 1 || len(c2.nicks) != 2 || len(st.nicks) != 2 || + len(st.me.chans) != 2 || len(n1.chans) != 1 || len(st.chans) != 2 { t.Errorf("Dissociating a nick from one channel went wrong.") } // This second one should also delete test1 // as it's no longer on any common channels with us - st.Dissociate(chan2, nick1) + st.Dissociate("#test2", "test1") - if len(chan1.nicks) != 1 || len(chan2.nicks) != 1 || len(st.nicks) != 1 || - len(st.me.chans) != 2 || len(nick1.chans) != 0 || len(st.chans) != 2 { + if len(c1.nicks) != 1 || len(c2.nicks) != 1 || len(st.nicks) != 1 || + len(st.me.chans) != 2 || len(n1.chans) != 0 || len(st.chans) != 2 { t.Errorf("Dissociating a nick from it's last channel went wrong.") } + if st.GetNick("test1") != nil { + t.Errorf("Able to get nick after dissociating from all channels.") + } } func TestSTWipe(t *testing.T) { st := NewTracker("mynick") - nick1 := st.NewNick("test1") - nick2 := st.NewNick("test2") - nick3 := st.NewNick("test3") - - chan1 := st.NewChannel("#test1") - chan2 := st.NewChannel("#test2") - chan3 := st.NewChannel("#test3") + st.NewNick("test1") + st.NewNick("test2") + st.NewNick("test3") + st.NewChannel("#test1") + st.NewChannel("#test2") + st.NewChannel("#test3") // Some associations - st.Associate(chan1, st.me) - st.Associate(chan2, st.me) - st.Associate(chan3, st.me) + st.Associate("#test1", "mynick") + st.Associate("#test2", "mynick") + st.Associate("#test3", "mynick") - st.Associate(chan1, nick1) - st.Associate(chan2, nick2) - st.Associate(chan3, nick3) + st.Associate("#test1", "test1") + st.Associate("#test2", "test2") + st.Associate("#test3", "test3") - st.Associate(chan1, nick2) - st.Associate(chan2, nick3) + st.Associate("#test1", "test2") + st.Associate("#test2", "test3") - st.Associate(chan1, nick3) + st.Associate("#test1", "test3") + + // We need to check out the manipulation of the internals. + nick1 := st.nicks["test1"] + nick2 := st.nicks["test2"] + nick3 := st.nicks["test3"] + chan1 := st.chans["#test1"] + chan2 := st.chans["#test2"] + chan3 := st.chans["#test3"] // Check the state we have at this point is what we would expect. if len(st.nicks) != 4 || len(st.chans) != 3 || len(st.me.chans) != 3 { @@ -413,3 +563,56 @@ func TestSTWipe(t *testing.T) { t.Errorf("Nick chan lists wrong length after wipe.") } } + +func TestSTRaces(t *testing.T) { + st := NewTracker("mynick") + wg := sync.WaitGroup{} + + for i := 'a'; i < 'g'; i++ { + wg.Add(2) + go func(s string) { + st.NewNick("nick-" + s) + c := st.NewChannel("#chan-" + s) + st.Associate(c.Name, "mynick") + wg.Done() + }(string(i)) + go func(s string) { + n := st.GetNick("nick-" + s) + c := st.GetChannel("#chan-" + s) + st.Associate(c.Name, n.Nick) + wg.Done() + }(string(i)) + } + wg.Wait() + + wg = sync.WaitGroup{} + race := func(ns, cs string) { + wg.Add(5) + go func() { + st.Associate("#chan-"+cs, "nick-"+ns) + wg.Done() + }() + go func() { + st.GetNick("nick-"+ns) + wg.Done() + }() + go func() { + st.GetChannel("#chan-"+cs) + wg.Done() + }() + go func() { + st.Dissociate("#chan-"+cs, "nick-"+ns) + wg.Done() + }() + go func() { + st.ReNick("nick-"+ns, "nick2-"+ns) + wg.Done() + }() + } + for n := 'a'; n < 'g'; n++ { + for c := 'a'; c < 'g'; c++ { + race(string(n), string(c)) + } + } + wg.Wait() +}