The great state tracker privatisation 3/3: tracker.

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)
+	ReNick(old, neu string) *Nick
-	DelNick(nick string)
+	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
+	Associate(channel, nick string) *ChanPrivs
-	Dissociate(channel *Channel, nick *Nick)
+	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),
+		chans: make(map[string]*channel),
-		nicks: make(map[string]*Nick),
+		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)
+	st.nicks[n] = newNick(n)
-	return st.nicks[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) {
+func (st *stateTracker) ReNick(old, neu string) *Nick {
-	if nk, ok := st.nicks[old]; ok {
+	st.mu.Lock()
-		if _, ok := st.nicks[neu]; !ok {
+	defer st.mu.Unlock()
-			nk.Nick = neu
+	nk, ok := st.nicks[old]
-			delete(st.nicks, old)
+	if !ok {
-			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 {
 		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.
+// Removes a nick from being tracked.
-func (st *stateTracker) DelNick(n string) {
+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 {
+		if nk == st.me {
-			st.delNick(nk)
-		} else {
 			logging.Warn("Tracker.DelNick(): won't delete myself.")
+			return nil
 		}
-	} else {
+		st.delNick(nk)
-		logging.Warn("Tracker.DelNick(): %s not tracked.", n)
+		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)
+	st.chans[c] = newChannel(c)
-	return st.chans[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 {
+		return ch.Channel()
-		logging.Warn("Tracker.DelChannel(): %s not tracked.", c)
 	}
+	logging.Warn("Tracker.DelChannel(): %s not tracked.", c)
+	return nil
 }
 
-func (st *stateTracker) delChannel(ch *Channel) {
+func (st *stateTracker) delChannel(ch *channel) {
-	delete(st.chans, ch.Name)
+	// 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)
+	st.mu.Lock()
-	ch := st.GetChannel(c)
+	defer st.mu.Unlock()
-	if nk != nil && ch != nil {
+	nk, nok := st.nicks[n]
-		return nk.IsOn(ch)
+	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 {
+func (st *stateTracker) Associate(c, n string) *ChanPrivs {
-	if ch == nil || nk == nil {
+	st.mu.Lock()
-		logging.Error("Tracker.Associate(): passed nil values :-(")
+	defer st.mu.Unlock()
-		return nil
+	nk, nok := st.nicks[n]
-	} else if _ch, ok := st.chans[ch.Name]; !ok || ch != _ch {
+	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) {
+func (st *stateTracker) Dissociate(c, n string) {
-	if ch == nil || nk == nil {
+	st.mu.Lock()
-		logging.Error("Tracker.Dissociate(): passed nil values :-(")
+	defer st.mu.Unlock()
-	} else if _ch, ok := st.chans[ch.Name]; !ok || ch != _ch {
+	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)
+			"internal state.", c)
-	} else if _nk, ok := st.nicks[nk.Nick]; !ok || nk != _nk {
+	} else if !nok {
 		logging.Error("Tracker.Dissociate(): nick %s not found in "+
-			"(or differs from) internal state.", nk.Nick)
+			"internal state.", n)
-	} else if _, ok := nk.IsOn(ch); !ok {
+	} 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 {

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.NewChannel("#chan1")
-	st.Associate(chan1, test1)
+	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" {
+	if test1.Nick != "test1" {
-		t.Errorf("Nick test1 not changed correctly.")
+		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.NewNick("test1")
-	st.ReNick("test1", "test2")
+	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")
+	add := st.NewNick("test1")
-	st.DelNick("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.NewNick("test1")
-	st.DelNick("test2")
+	fail := st.DelNick("test2")
-	if len(st.nicks) != 2 {
+	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")
+	fail = st.DelNick("mynick")
-	if len(st.nicks) != 2 {
+	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("#test1", "mynick")
-	st.Associate(chan1, nick1)
+	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 ||
+	if len(c1.nicks) != 2 || len(st.nicks) != 2 ||
-		len(st.me.chans) != 1 || len(nick1.chans) != 1 || len(st.chans) != 1 {
+		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(c1.nicks) != 1 || len(st.nicks) != 1 ||
-	if len(chan1.nicks) != 1 || len(st.chans) != 1 ||
+		len(st.me.chans) != 1 || len(n1.chans) != 0 || len(st.chans) != 1 {
-		len(st.me.chans) != 1 || len(nick1.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")
+	add := st.NewChannel("#test1")
-	st.DelChannel("#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
+	// Deleting unknown channel shouldn't work, but let's make sure we have a
-	// known nick first to catch any possible accidental removals.
+	// known channel first to catch any possible accidental removals.
-	chan1 := st.NewChannel("#test1")
+	st.NewChannel("#test1")
-	st.DelChannel("#test2")
+	fail := st.DelChannel("#test2")
-	if len(st.chans) != 1 {
+	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")
+	st.NewChannel("#test2")
-	nick1 := st.NewNick("test1")
+	st.NewNick("test1")
 
 	// Associate both "my" nick and test1 with the channels
-	st.Associate(chan1, st.me)
+	st.Associate("#test1", "mynick")
-	st.Associate(chan1, nick1)
+	st.Associate("#test1", "test1")
-	st.Associate(chan2, st.me)
+	st.Associate("#test2", "mynick")
-	st.Associate(chan2, nick1)
+	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 ||
+	if len(c1.nicks) != 2 || len(c2.nicks) != 2 || len(st.nicks) != 2 ||
-		len(st.me.chans) != 2 || len(nick1.chans) != 2 || len(st.chans) != 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 ||
+	if len(c1.nicks) != 0 || len(c2.nicks) != 2 || len(st.nicks) != 2 ||
-		len(st.me.chans) != 1 || len(nick1.chans) != 1 || len(st.chans) != 1 {
+		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 ||
+	if len(c1.nicks) != 0 || len(c2.nicks) != 0 || len(st.nicks) != 1 ||
-		len(st.me.chans) != 0 || len(nick1.chans) != 0 || len(st.chans) != 0 {
+		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")
+	st.NewNick("test1")
-	chan1 := st.NewChannel("#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)
+	st.Associate("#test1", "test1")
-	if priv, ok := st.IsOn("#test1", "test1"); !ok || priv != cp {
+	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")
+	st.NewNick("test1")
-	chan1 := st.NewChannel("#test1")
+	st.NewChannel("#test1")
 
-	cp := st.Associate(chan1, nick1)
+	// We need to check out the manipulation of the internals.
-	if priv, ok := nick1.chans[chan1]; !ok || cp != priv {
+	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 {
+	if st.Associate("", "test1") != nil {
-		t.Errorf("Associating nil *Channel did not return nil.")
+		t.Errorf("Associating unknown channel did not return nil.")
 	}
-	if st.Associate(chan1, nil) != nil {
+	if st.Associate("#test1", "") != nil {
-		t.Errorf("Associating nil *Nick did not return 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")
+	st.NewNick("test1")
-	chan1 := st.NewChannel("#test1")
+	st.NewChannel("#test1")
-	chan2 := st.NewChannel("#test2")
+	st.NewChannel("#test2")
 
 	// Associate both "my" nick and test1 with the channels
-	st.Associate(chan1, st.me)
+	st.Associate("#test1", "mynick")
-	st.Associate(chan1, nick1)
+	st.Associate("#test1", "test1")
-	st.Associate(chan2, st.me)
+	st.Associate("#test2", "mynick")
-	st.Associate(chan2, nick1)
+	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 ||
+	if len(c1.nicks) != 2 || len(c2.nicks) != 2 || len(st.nicks) != 2 ||
-		len(st.me.chans) != 2 || len(nick1.chans) != 2 || len(st.chans) != 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 ||
+	if len(c1.nicks) != 2 || len(c2.nicks) != 0 || len(st.nicks) != 2 ||
-		len(st.me.chans) != 1 || len(nick1.chans) != 1 || len(st.chans) != 1 {
+		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.NewChannel("#test2")
-	st.Associate(chan2, st.me)
+	st.Associate("#test2", "mynick")
-	st.Associate(chan2, nick1)
+	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 ||
+	if len(c1.nicks) != 2 || len(c2.nicks) != 2 || len(st.nicks) != 2 ||
-		len(st.me.chans) != 2 || len(nick1.chans) != 2 || len(st.chans) != 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 ||
+	if len(c1.nicks) != 1 || len(c2.nicks) != 2 || len(st.nicks) != 2 ||
-		len(st.me.chans) != 2 || len(nick1.chans) != 1 || len(st.chans) != 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 ||
+	if len(c1.nicks) != 1 || len(c2.nicks) != 1 || len(st.nicks) != 1 ||
-		len(st.me.chans) != 2 || len(nick1.chans) != 0 || len(st.chans) != 2 {
+		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")
+	st.NewNick("test1")
-	nick2 := st.NewNick("test2")
+	st.NewNick("test2")
-	nick3 := st.NewNick("test3")
+	st.NewNick("test3")
-
+	st.NewChannel("#test1")
-	chan1 := st.NewChannel("#test1")
+	st.NewChannel("#test2")
-	chan2 := st.NewChannel("#test2")
+	st.NewChannel("#test3")
-	chan3 := st.NewChannel("#test3")
 
 	// Some associations
-	st.Associate(chan1, st.me)
+	st.Associate("#test1", "mynick")
-	st.Associate(chan2, st.me)
+	st.Associate("#test2", "mynick")
-	st.Associate(chan3, st.me)
+	st.Associate("#test3", "mynick")
 
-	st.Associate(chan1, nick1)
+	st.Associate("#test1", "test1")
-	st.Associate(chan2, nick2)
+	st.Associate("#test2", "test2")
-	st.Associate(chan3, nick3)
+	st.Associate("#test3", "test3")
 
-	st.Associate(chan1, nick2)
+	st.Associate("#test1", "test2")
-	st.Associate(chan2, nick3)
+	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 {