From bffe946388532115d5f480ecaf9296d303043a6e Mon Sep 17 00:00:00 2001
From: Alex Bramley <a.bramley@gmail.com>
Date: Wed, 31 Dec 2014 13:17:11 +0000
Subject: [PATCH 1/7] The great state tracker privatisation 1/3: channels.

---
 state/channel.go      | 163 +++++++++++++++++++++++++-----------------
 state/channel_test.go |  66 +++++++++++------
 2 files changed, 141 insertions(+), 88 deletions(-)

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).")

From 4dd8bc72d5eeee7a323f5da9d3b8eb343db694ac Mon Sep 17 00:00:00 2001
From: Alex Bramley <a.bramley@gmail.com>
Date: Wed, 31 Dec 2014 13:17:28 +0000
Subject: [PATCH 2/7] The great state tracker privatisation 2/3: nicks.

---
 state/nick.go      | 128 ++++++++++++++++++++++++++-------------------
 state/nick_test.go |  39 +++++++++-----
 2 files changed, 101 insertions(+), 66 deletions(-)

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.")
 	}

From f3c49069c059d6a10dcd015e5b4b3c32fa547a9d Mon Sep 17 00:00:00 2001
From: Alex Bramley <a.bramley@gmail.com>
Date: Wed, 31 Dec 2014 13:17:46 +0000
Subject: [PATCH 3/7] The great state tracker privatisation 3/3: tracker.

---
 state/tracker.go      | 254 +++++++++++++++++++---------
 state/tracker_test.go | 381 +++++++++++++++++++++++++++++-------------
 2 files changed, 442 insertions(+), 193 deletions(-)

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..165b5aa 100644
--- a/state/tracker_test.go
+++ b/state/tracker_test.go
@@ -2,6 +2,12 @@ package state
 
 import "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 +17,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 +29,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 +61,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 +116,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 +125,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 +237,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 +254,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 +268,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 +277,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 +316,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 +331,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 +343,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 +405,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 {

From 36e4aeb603cd3aaa6e77a4f220e6bf1fad5caccf Mon Sep 17 00:00:00 2001
From: Alex Bramley <a.bramley@gmail.com>
Date: Wed, 31 Dec 2014 17:31:50 +0000
Subject: [PATCH 4/7] An attempt to trigger data races in the state tracker.

---
 state/tracker_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/state/tracker_test.go b/state/tracker_test.go
index 9ae2c19..78521dc 100644
--- a/state/tracker_test.go
+++ b/state/tracker_test.go
@@ -1,6 +1,9 @@
 package state
 
-import "testing"
+import (
+	"sync"
+	"testing"
+)
 
 func TestSTNewTracker(t *testing.T) {
 	st := NewTracker("mynick")
@@ -413,3 +416,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, st.me)
+			wg.Done()
+		}(string(i))
+		go func(s string) {
+			n := st.GetNick("nick-" + s)
+			c := st.GetChannel("#chan-" + s)
+			st.Associate(c, n)
+			wg.Done()
+		}(string(i))
+	}
+	wg.Wait()
+
+	wg = sync.WaitGroup{}
+	race := func(ns, cs string) {
+		wg.Add(5)
+		go func() {
+			st.Associate(st.GetChannel("#chan-"+cs), st.GetNick("nick-"+ns))
+			wg.Done()
+		}()
+		go func() {
+			st.GetNick("nick-"+ns).Channels()
+			wg.Done()
+		}()
+		go func() {
+			st.GetChannel("#chan-"+cs).Nicks()
+			wg.Done()
+		}()
+		go func() {
+			st.Dissociate(st.GetChannel("#chan-"+cs), st.GetNick("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()
+}

From ed6eb6115fe0dd7f49815011f30183911fffd587 Mon Sep 17 00:00:00 2001
From: Alex Bramley <a.bramley@gmail.com>
Date: Wed, 31 Dec 2014 17:37:08 +0000
Subject: [PATCH 5/7] Race test changes.

---
 state/tracker_test.go | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/state/tracker_test.go b/state/tracker_test.go
index 5e27f29..6330098 100644
--- a/state/tracker_test.go
+++ b/state/tracker_test.go
@@ -573,13 +573,13 @@ func TestSTRaces(t *testing.T) {
 		go func(s string) {
 			st.NewNick("nick-" + s)
 			c := st.NewChannel("#chan-" + s)
-			st.Associate(c, st.me)
+			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, n)
+			st.Associate(c.Name, n.Nick)
 			wg.Done()
 		}(string(i))
 	}
@@ -589,19 +589,19 @@ func TestSTRaces(t *testing.T) {
 	race := func(ns, cs string) {
 		wg.Add(5)
 		go func() {
-			st.Associate(st.GetChannel("#chan-"+cs), st.GetNick("nick-"+ns))
+			st.Associate("#chan-"+cs, "nick-"+ns)
 			wg.Done()
 		}()
 		go func() {
-			st.GetNick("nick-"+ns).Channels()
+			st.GetNick("nick-"+ns)
 			wg.Done()
 		}()
 		go func() {
-			st.GetChannel("#chan-"+cs).Nicks()
+			st.GetChannel("#chan-"+cs)
 			wg.Done()
 		}()
 		go func() {
-			st.Dissociate(st.GetChannel("#chan-"+cs), st.GetNick("nick-"+ns))
+			st.Dissociate("#chan-"+cs, "nick-"+ns)
 			wg.Done()
 		}()
 		go func() {

From d11d209dcd295f82459454f2b59d5c01b6aca2b9 Mon Sep 17 00:00:00 2001
From: Alex Bramley <a.bramley@gmail.com>
Date: Wed, 31 Dec 2014 17:50:03 +0000
Subject: [PATCH 6/7] Generate new tracker mock.

---
 state/mock_tracker.go | 67 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 59 insertions(+), 8 deletions(-)

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)
 }
 

From 0216e0406e8be3494ff2fdd9be55a34dfbf0a2ec Mon Sep 17 00:00:00 2001
From: Alex Bramley <a.bramley@gmail.com>
Date: Fri, 2 Jan 2015 12:58:50 +0000
Subject: [PATCH 7/7] Update client for new state tracking code.

---
 client/connection.go      |  14 +-
 client/connection_test.go |  13 +-
 client/handlers.go        |  12 +-
 client/handlers_test.go   | 264 ++++++++++++++------------------------
 client/state_handlers.go  |  75 +++++------
 5 files changed, 158 insertions(+), 220 deletions(-)

diff --git a/client/connection.go b/client/connection.go
index d94f736..13c1db9 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 "<hop count> <real name>"
 	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])