First steps towards client interface.

- Move all exported vars to Config struct;
- Plumbing for Config.Me etc;
- Constants and INIT/REGISTER handler from github.com/iopred;

client/connection.go

@@ -3,7 +3,6 @@ package client
 import (
 	"bufio"
 	"crypto/tls"
-	"errors"
 	"fmt"
 	"github.com/fluffle/goirc/state"
 	"github.com/fluffle/golog/logging"
@@ -14,15 +13,6 @@ import (
 
 // An IRC connection is represented by this struct.
 type Conn struct {
-	// Connection related vars people will care about
-	Me        *state.Nick
-	Host      string
-	Network   string
-	Connected bool
-
-	// Deprecated: future work to turn Conn into an interface will break this.
-	// Use the State field to store external state that handlers might need.
-	State interface{}
 
 	// Contains parameters that people can tweak to change client behaviour.
 	cfg *Config
@@ -40,6 +30,7 @@ type Conn struct {
 	io        *bufio.ReadWriter
 	in        chan *Line
 	out       chan string
+	connected bool
 
 	// Control channels to goroutines
 	cSend, cLoop, cPing chan bool
@@ -54,6 +45,9 @@ type Config struct {
 	// Set this to provide the Nick, Ident and Name for the client to use.
 	Me *state.Nick
 
+	// Hostname to connect to and optional connect password.
+	Server, Pass string
+
 	// Are we connecting via SSL? Do we care about certificate validity?
 	SSL       bool
 	SSLConfig *tls.Config
@@ -72,21 +66,19 @@ type Config struct {
 }
 
 func NewConfig(nick string, args ...string) *Config {
-	ident := "goirc"
-	if len(args) > 0 && args[0] != "" {
-		ident = args[0]
-	}
-	name := "Powered by GoIRC"
-	if len(args) > 1 && args[1] != "" {
-		name = args[1]
-	}
 	cfg := &Config{
+		Me:       state.NewNick(nick),
 		PingFreq: 3 * time.Minute,
 		NewNick:  func(s string) string { return s + "_" },
 	}
-	cfg.Me = state.NewNick(nick)
+	cfg.Me.Ident = "goirc"
-	cfg.Me.Ident = ident
+	if len(args) > 0 && args[0] != "" {
-	cfg.Me.Name = name
+		cfg.Me.Ident = args[0]
+	}
+	cfg.Me.Name = "Powered by GoIRC"
+	if len(args) > 1 && args[1] != "" {
+		cfg.Me.Name = args[1]
+	}
 	return cfg
 }
 
@@ -99,10 +91,9 @@ func SimpleClient(nick string, args ...string) (*Conn, error) {
 func Client(cfg *Config) (*Conn, error) {
 	logging.InitFromFlags()
 	if cfg.Me == nil || cfg.Me.Nick == "" || cfg.Me.Ident == "" {
-		return nil, fmt.Errorf("Must provide a valid state.Nick in cfg.Me.")
+		return nil, fmt.Errorf("irc.Client(): Both cfg.Nick and cfg.Ident must be non-empty.")
 	}
 	conn := &Conn{
-		Me:         cfg.Me,
 		cfg:        cfg,
 		in:         make(chan *Line, 32),
 		out:        make(chan string, 32),
@@ -119,17 +110,25 @@ func Client(cfg *Config) (*Conn, error) {
 	return conn, nil
 }
 
+func (conn *Conn) Connected() bool {
+	return conn.connected
+}
+
 func (conn *Conn) Config() *Config {
 	return conn.cfg
 }
 
+func (conn *Conn) StateTracker() state.Tracker {
+	return conn.st
+}
+
 func (conn *Conn) EnableStateTracking() {
 	if conn.st == nil {
-		n := conn.Me
+		n := conn.cfg.Me
 		conn.st = state.NewTracker(n.Nick)
-		conn.Me = conn.st.Me()
+		conn.cfg.Me = conn.st.Me()
-		conn.Me.Ident = n.Ident
+		conn.cfg.Me.Ident = n.Ident
-		conn.Me.Name = n.Name
+		conn.cfg.Me.Name = n.Name
 		conn.addSTHandlers()
 	}
 }
@@ -142,10 +141,6 @@ func (conn *Conn) DisableStateTracking() {
 	}
 }
 
-func (conn *Conn) StateTracker() state.Tracker {
-	return conn.st
-}
-
 // Per-connection state initialisation.
 func (conn *Conn) initialise() {
 	conn.io = nil
@@ -160,43 +155,45 @@ func (conn *Conn) initialise() {
 // on the connection to the IRC server, set Conn.SSL to true before calling
 // Connect(). The port will default to 6697 if ssl is enabled, and 6667
 // otherwise. You can also provide an optional connect password.
-func (conn *Conn) Connect(host string, pass ...string) error {
+func (conn *Conn) ConnectTo(host string, pass ...string) error {
-	if conn.Connected {
+	conn.cfg.Server = host
-		return errors.New(fmt.Sprintf(
+	if len(pass) > 0 {
-			"irc.Connect(): already connected to %s, cannot connect to %s",
+		conn.cfg.Pass = pass[0]
-			conn.Host, host))
 	}
+	return conn.Connect()
+}
 
+func (conn *Conn) Connect() error {
+	if conn.cfg.Server == "" {
+		return fmt.Errorf("irc.Connect(): cfg.Server must be non-empty")
+	}
+	if conn.connected {
+		return fmt.Errorf("irc.Connect(): Cannot connect to %s, already connected.", conn.cfg.Server)
+	}
 	if conn.cfg.SSL {
-		if !hasPort(host) {
+		if !hasPort(conn.cfg.Server) {
-			host += ":6697"
+			conn.cfg.Server += ":6697"
 		}
-		logging.Info("irc.Connect(): Connecting to %s with SSL.", host)
+		logging.Info("irc.Connect(): Connecting to %s with SSL.", conn.cfg.Server)
-		if s, err := tls.Dial("tcp", host, conn.cfg.SSLConfig); err == nil {
+		if s, err := tls.Dial("tcp", conn.cfg.Server, conn.cfg.SSLConfig); err == nil {
 			conn.sock = s
 		} else {
 			return err
 		}
 	} else {
-		if !hasPort(host) {
+		if !hasPort(conn.cfg.Server) {
-			host += ":6667"
+			conn.cfg.Server += ":6667"
 		}
-		logging.Info("irc.Connect(): Connecting to %s without SSL.", host)
+		logging.Info("irc.Connect(): Connecting to %s without SSL.", conn.cfg.Server)
-		if s, err := net.Dial("tcp", host); err == nil {
+		if s, err := net.Dial("tcp", conn.cfg.Server); err == nil {
 			conn.sock = s
 		} else {
 			return err
 		}
 	}
-	conn.Host = host
+	conn.connected = true
-	conn.Connected = true
 	conn.postConnect()
-
+	conn.dispatch(&Line{Cmd: REGISTER})
-	if len(pass) > 0 {
-		conn.Pass(pass[0])
-	}
-	conn.Nick(conn.Me.Nick)
-	conn.User(conn.Me.Ident, conn.Me.Name)
 	return nil
 }
 
@@ -348,12 +345,12 @@ func (conn *Conn) shutdown() {
 func (conn *Conn) String() string {
 	str := "GoIRC Connection\n"
 	str += "----------------\n\n"
-	if conn.Connected {
+	if conn.connected {
-		str += "Connected to " + conn.Host + "\n\n"
+		str += "Connected to " + conn.cfg.Server + "\n\n"
 	} else {
 		str += "Not currently connected!\n\n"
 	}
-	str += conn.Me.String() + "\n"
+	str += conn.cfg.Me.String() + "\n"
 	if conn.st != nil {
 		str += conn.st.String() + "\n"
 	}

client/connection_test.go

@@ -27,7 +27,7 @@ func setUp(t *testing.T, start ...bool) (*Conn, *testState) {
 	c.st = st
 	c.sock = nc
 	c.cfg.Flood = true // Tests can take a while otherwise
-	c.Connected = true
+	c.connected = true
 	if len(start) == 0 {
 		// Hack to allow tests of send, recv, write etc.
 		// NOTE: the value of the boolean doesn't matter.
@@ -69,7 +69,7 @@ func TestEOF(t *testing.T) {
 	<-time.After(time.Millisecond)
 
 	// Verify that the connection no longer thinks it's connected
-	if c.Connected {
+	if c.connected {
 		t.Errorf("Conn still thinks it's connected to the server.")
 	}
 
@@ -85,9 +85,9 @@ func TestClientAndStateTracking(t *testing.T) {
 	c, _ := SimpleClient("test", "test", "Testing IRC")
 
 	// Assert some basic things about the initial state of the Conn struct
-	if c.Me.Nick != "test" || c.Me.Ident != "test" ||
+	if me := c.cfg.Me; me.Nick != "test" || me.Ident != "test" ||
-		c.Me.Name != "Testing IRC" || c.Me.Host != "" {
+		me.Name != "Testing IRC" || me.Host != "" {
-		t.Errorf("Conn.Me not correctly initialised.")
+		t.Errorf("Conn.cfg.Me not correctly initialised.")
 	}
 	// Check that the internal handlers are correctly set up
 	for k, _ := range intHandlers {
@@ -109,20 +109,20 @@ func TestClientAndStateTracking(t *testing.T) {
 	}
 
 	// We're expecting the untracked me to be replaced by a tracked one
-	if c.Me.Nick != "test" || c.Me.Ident != "test" ||
+	if me := c.cfg.Me; me.Nick != "test" || me.Ident != "test" ||
-		c.Me.Name != "Testing IRC" || c.Me.Host != "" {
+		me.Name != "Testing IRC" || me.Host != "" {
 		t.Errorf("Enabling state tracking did not replace Me correctly.")
 	}
-	if c.st == nil || c.Me != c.st.Me() {
+	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.Me
+	me := c.cfg.Me
 	c.st = st
 	st.EXPECT().Wipe()
 	c.DisableStateTracking()
-	if c.st != nil || c.Me != me {
+	if c.st != nil || c.cfg.Me != me {
 		t.Errorf("State tracker not disabled correctly.")
 	}
 

client/handlers.go

@@ -7,8 +7,15 @@ import (
 	"strings"
 )
 
+const (
+	REGISTER = "REGISTER"
+	CONNECTED = "CONNECTED"
+	DISCONNECTED = "DISCONNECTED"
+)
+
 // sets up the internal event handlers to do essential IRC protocol things
 var intHandlers = map[string]HandlerFunc{
+	REGISTER: (*Conn).h_REGISTER,
 	"001":  (*Conn).h_001,
 	"433":  (*Conn).h_433,
 	"CTCP": (*Conn).h_CTCP,
@@ -29,16 +36,25 @@ func (conn *Conn) h_PING(line *Line) {
 	conn.Raw("PONG :" + line.Args[0])
 }
 
-// Handler to trigger a "CONNECTED" event on receipt of numeric 001
+// Handler for initial registration with server once tcp connection is made.
+func (conn *Conn) h_REGISTER(line *Line) {
+	if conn.cfg.Pass != "" {
+		conn.Pass(conn.cfg.Pass)
+	}
+	conn.Nick(conn.cfg.Me.Nick)
+	conn.User(conn.cfg.Me.Ident, conn.cfg.Me.Name)
+}
+
+// Handler to trigger a CONNECTED event on receipt of numeric 001
 func (conn *Conn) h_001(line *Line) {
 	// we're connected!
-	conn.dispatch(&Line{Cmd: "connected"})
+	conn.dispatch(&Line{Cmd: CONNECTED})
 	// and we're being given our hostname (from the server's perspective)
 	t := line.Args[len(line.Args)-1]
 	if idx := strings.LastIndex(t, " "); idx != -1 {
 		t = t[idx+1:]
 		if idx = strings.Index(t, "@"); idx != -1 {
-			conn.Me.Host = t[idx+1:]
+			conn.cfg.Me.Host = t[idx+1:]
 		}
 	}
 }
@@ -59,11 +75,11 @@ func (conn *Conn) h_433(line *Line) {
 	// 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.Me.Nick {
+	if line.Args[1] == conn.cfg.Me.Nick {
 		if conn.st != nil {
-			conn.st.ReNick(conn.Me.Nick, neu)
+			conn.st.ReNick(conn.cfg.Me.Nick, neu)
 		} else {
-			conn.Me.Nick = neu
+			conn.cfg.Me.Nick = neu
 		}
 	}
 }
@@ -79,17 +95,17 @@ func (conn *Conn) h_CTCP(line *Line) {
 
 // Handle updating our own NICK if we're not using the state tracker
 func (conn *Conn) h_NICK(line *Line) {
-	if conn.st == nil && line.Nick == conn.Me.Nick {
+	if conn.st == nil && line.Nick == conn.cfg.Me.Nick {
-		conn.Me.Nick = line.Args[0]
+		conn.cfg.Me.Nick = line.Args[0]
 	}
 }
 
 // Handle PRIVMSGs that trigger Commands
 func (conn *Conn) h_PRIVMSG(line *Line) {
 	txt := line.Args[1]
-	if conn.cfg.CommandStripNick && strings.HasPrefix(txt, conn.Me.Nick) {
+	if conn.cfg.CommandStripNick && strings.HasPrefix(txt, conn.cfg.Me.Nick) {
 		// Look for '^${nick}[:;>,-]? '
-		l := len(conn.Me.Nick)
+		l := len(conn.cfg.Me.Nick)
 		switch txt[l] {
 		case ':', ';', '>', ',', '-':
 			l++

client/handlers_test.go

@@ -19,6 +19,26 @@ func TestPING(t *testing.T) {
 	s.nc.Expect("PONG :1234567890")
 }
 
+// Test the REGISTER handler matches section 3.1 of rfc2812
+func TestREGISTER(t *testing.T) {
+	c, s := setUp(t)
+	defer s.tearDown()
+
+	c.h_REGISTER(&Line{Cmd: REGISTER})
+	s.nc.Expect("NICK test")
+	s.nc.Expect("USER test 12 * :Testing IRC")
+	s.nc.ExpectNothing()
+
+	c.cfg.Pass = "12345"
+	c.cfg.Me.Ident = "idiot"
+	c.cfg.Me.Name = "I've got the same combination on my luggage!"
+	c.h_REGISTER(&Line{Cmd: REGISTER})
+	s.nc.Expect("PASS 12345")
+	s.nc.Expect("NICK test")
+	s.nc.Expect("USER idiot 12 * :I've got the same combination on my luggage!")
+	s.nc.ExpectNothing()
+}
+
 // Test the handler for 001 / RPL_WELCOME
 func Test001(t *testing.T) {
 	c, s := setUp(t)
@@ -39,8 +59,8 @@ func Test001(t *testing.T) {
 	}
 
 	// Check host parsed correctly
-	if c.Me.Host != "somehost.com" {
+	if c.cfg.Me.Host != "somehost.com" {
-		t.Errorf("Host parsing failed, host is '%s'.", c.Me.Host)
+		t.Errorf("Host parsing failed, host is '%s'.", c.cfg.Me.Host)
 	}
 }
 
@@ -49,13 +69,13 @@ func Test433(t *testing.T) {
 	c, s := setUp(t)
 	defer s.tearDown()
 
-	// Call handler with a 433 line, not triggering c.Me.Renick()
+	// Call handler with a 433 line, not triggering c.cfg.Me.Renick()
 	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.Me.Nick != "test" {
+	if c.cfg.Me.Nick != "test" {
-		t.Errorf("ReNick() called unexpectedly, Nick == '%s'.", c.Me.Nick)
+		t.Errorf("ReNick() called unexpectedly, Nick == '%s'.", c.cfg.Me.Nick)
 	}
 
 	// Send a line that will trigger a renick. This happens when our wanted
@@ -66,11 +86,11 @@ func Test433(t *testing.T) {
 	c.h_433(parseLine(":irc.server.org 433 test test :Nickname is already in use."))
 	s.nc.Expect("NICK test_")
 
-	// Counter-intuitively, c.Me.Nick will not change in this case. This is an
+	// Counter-intuitively, c.cfg.Me.Nick will not change in this case. This
-	// artifact of the test set-up, with a mocked out state tracker that
+	// 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.Me.Nick != "test" {
+	if c.cfg.Me.Nick != "test" {
-		t.Errorf("My nick changed from '%s'.", c.Me.Nick)
+		t.Errorf("My nick changed from '%s'.", c.cfg.Me.Nick)
 	}
 
 	// Test the code path that *doesn't* involve state tracking.
@@ -78,8 +98,8 @@ func Test433(t *testing.T) {
 	c.h_433(parseLine(":irc.server.org 433 test test :Nickname is already in use."))
 	s.nc.Expect("NICK test_")
 
-	if c.Me.Nick != "test_" {
+	if c.cfg.Me.Nick != "test_" {
-		t.Errorf("My nick not updated from '%s'.", c.Me.Nick)
+		t.Errorf("My nick not updated from '%s'.", c.cfg.Me.Nick)
 	}
 	c.st = s.st
 }
@@ -96,7 +116,7 @@ func TestNICK(t *testing.T) {
 	c.h_NICK(parseLine(":test!test@somehost.com NICK :test1"))
 
 	// Verify that our Nick has changed
-	if c.Me.Nick != "test1" {
+	if c.cfg.Me.Nick != "test1" {
 		t.Errorf("NICK did not result in changing our nick.")
 	}
 
@@ -104,7 +124,7 @@ func TestNICK(t *testing.T) {
 	c.h_NICK(parseLine(":blah!moo@cows.com NICK :milk"))
 
 	// Verify that our Nick hasn't changed
-	if c.Me.Nick != "test1" {
+	if c.cfg.Me.Nick != "test1" {
 		t.Errorf("NICK did not result in changing our nick.")
 	}
 
@@ -113,7 +133,7 @@ func TestNICK(t *testing.T) {
 	c.h_NICK(parseLine(":test1!test@somehost.com NICK :test2"))
 
 	// Verify that our Nick hasn't changed (should be handled by h_STNICK).
-	if c.Me.Nick != "test1" {
+	if c.cfg.Me.Nick != "test1" {
 		t.Errorf("NICK changed our nick when state tracking enabled.")
 	}
 }
@@ -200,9 +220,9 @@ func TestJOIN(t *testing.T) {
 
 	gomock.InOrder(
 		s.st.EXPECT().GetChannel("#test1").Return(nil),
-		s.st.EXPECT().GetNick("test").Return(c.Me),
+		s.st.EXPECT().GetNick("test").Return(c.cfg.Me),
 		s.st.EXPECT().NewChannel("#test1").Return(chan1),
-		s.st.EXPECT().Associate(chan1, c.Me),
+		s.st.EXPECT().Associate(chan1, c.cfg.Me),
 	)
 
 	// Use #test1 to test expected behaviour
@@ -319,10 +339,10 @@ func TestMODE(t *testing.T) {
 	// Send a nick mode line, returning Me
 	gomock.InOrder(
 		s.st.EXPECT().GetChannel("test").Return(nil),
-		s.st.EXPECT().GetNick("test").Return(c.Me),
+		s.st.EXPECT().GetNick("test").Return(c.cfg.Me),
 	)
 	c.h_MODE(parseLine(":test!test@somehost.com MODE test +i"))
-	if !c.Me.Modes.Invisible {
+	if !c.cfg.Me.Modes.Invisible {
 		t.Errorf("Nick.ParseModes() not called correctly.")
 	}
 
@@ -482,7 +502,7 @@ func Test353(t *testing.T) {
 	nicks := make(map[string]*state.Nick)
 	privs := make(map[string]*state.ChanPrivs)
 
-	nicks["test"] = c.Me
+	nicks["test"] = c.cfg.Me
 	privs["test"] = new(state.ChanPrivs)
 
 	for _, n := range []string{"user1", "user2", "voice", "halfop",
@@ -495,7 +515,7 @@ func Test353(t *testing.T) {
 	s.st.EXPECT().GetChannel("#test1").Return(chan1).Times(2)
 	gomock.InOrder(
 		// "test" is Me, i am known, and already on the channel
-		s.st.EXPECT().GetNick("test").Return(c.Me),
+		s.st.EXPECT().GetNick("test").Return(c.cfg.Me),
 		s.st.EXPECT().IsOn("#test1", "test").Return(privs["test"], true),
 		// user1 is known, but not on the channel, so should be associated
 		s.st.EXPECT().GetNick("user1").Return(nicks["user1"]),

client/state_handlers.go

@@ -50,7 +50,7 @@ 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.Me {
+		if nk != conn.cfg.Me {
 			logging.Warn("irc.JOIN(): JOIN to unknown channel %s received "+
 				"from (non-me) nick %s", line.Args[0], line.Nick)
 			return
@@ -102,7 +102,7 @@ func (conn *Conn) h_MODE(line *Line) {
 		ch.ParseModes(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.Me {
+		if nk != conn.cfg.Me {
 			logging.Warn("irc.MODE(): recieved MODE %s for (non-me) nick %s",
 				line.Args[1], line.Args[0])
 			return