From 8af2a1c25d933c8f419465ef884f6db1167d4777 Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Wed, 21 Jan 2015 21:44:22 +0000
Subject: [PATCH] refactor: make tie breaking part of the Connection interface
 and thus eliminate all knowledge of LocalConnection from the
 LocalPeer.handle* methods

---
 router/connection.go | 12 +++++++++---
 router/local_peer.go | 46 ++++++++++++++++++++++++++------------------
 2 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/router/connection.go b/router/connection.go
index 8ccaaa11..49830b07 100644
--- a/router/connection.go
+++ b/router/connection.go
@@ -15,6 +15,7 @@ import (
 type Connection interface {
 	Local() *Peer
 	Remote() *Peer
+	BreakTie(Connection) ConnectionTieBreak
 	RemoteTCPAddr() string
 	Established() bool
 	Shutdown(error)
@@ -80,6 +81,10 @@ func (conn *RemoteConnection) Remote() *Peer {
 	return conn.remote
 }
 
+func (conn *RemoteConnection) BreakTie(Connection) ConnectionTieBreak {
+	return TieBreakTied
+}
+
 func (conn *RemoteConnection) RemoteTCPAddr() string {
 	return conn.remoteTCPAddr
 }
@@ -124,12 +129,13 @@ func (conn *LocalConnection) Start(acceptNewPeer bool) {
 	go conn.run(queryChan, acceptNewPeer)
 }
 
-func (conn *LocalConnection) BreakTie(dupConn *LocalConnection) ConnectionTieBreak {
+func (conn *LocalConnection) BreakTie(dupConn Connection) ConnectionTieBreak {
+	dupConnLocal := dupConn.(*LocalConnection)
  	// conn.uid is used as the tie breaker here, in the knowledge that
 	// both sides will make the same decision.
-	if conn.uid < dupConn.uid {
+	if conn.uid < dupConnLocal.uid {
 		return TieBreakWon
-	} else if dupConn.uid < conn.uid {
+	} else if dupConnLocal.uid < conn.uid {
 		return TieBreakLost
 	} else {
 		return TieBreakTied
diff --git a/router/local_peer.go b/router/local_peer.go
index f7b707c6..3648038c 100644
--- a/router/local_peer.go
+++ b/router/local_peer.go
@@ -163,19 +163,28 @@ func (peer *LocalPeer) queryLoop(queryChan <-chan *PeerInteraction) {
 		}
 		switch query.code {
 		case PAddConnection:
-			peer.handleAddConnection(query.payload.(*LocalConnection))
+			conn := query.payload.(*LocalConnection)
+			if peer.handleAddConnection(conn) {
+				conn.log("connection added")
+			}
 		case PDeleteConnection:
-			peer.handleDeleteConnection(query.payload.(*LocalConnection))
+			conn := query.payload.(*LocalConnection)
+			if peer.handleDeleteConnection(conn) {
+				conn.log("connection deleted")
+			}
 			query.resultChan <- nil
 		case PConnectionEstablished:
-			peer.handleConnectionEstablished(query.payload.(*LocalConnection))
+			conn := query.payload.(*LocalConnection)
+			if peer.handleConnectionEstablished(conn) {
+				conn.log("connection fully established")
+			}
 		case PSendProtocolMsg:
 			peer.handleSendProtocolMsg(query.payload.(ProtocolMsg))
 		}
 	}
 }
 
-func (peer *LocalPeer) handleAddConnection(conn *LocalConnection) {
+func (peer *LocalPeer) handleAddConnection(conn Connection) bool {
 	if peer.Peer != conn.Local() {
 		log.Fatal("Attempt made to add connection to peer where peer is not the source of connection")
 	}
@@ -187,33 +196,32 @@ func (peer *LocalPeer) handleAddConnection(conn *LocalConnection) {
 	// deliberately non symmetrical
 	if dupConn, found := peer.connections[toName]; found {
 		if dupConn == conn {
-			return
+			return false
 		}
-		dupConnLocal := dupConn.(*LocalConnection)
-		switch conn.BreakTie(dupConnLocal) {
+		switch conn.BreakTie(dupConn) {
 		case TieBreakWon:
 			dupConn.Shutdown(dupErr)
-			peer.handleDeleteConnection(dupConnLocal)
+			peer.handleDeleteConnection(dupConn)
 		case TieBreakLost:
 			conn.Shutdown(dupErr)
-			return
+			return false
 		case TieBreakTied:
 			// oh good grief. Sod it, just kill both of them.
 			conn.Shutdown(dupErr)
 			dupConn.Shutdown(dupErr)
-			peer.handleDeleteConnection(dupConnLocal)
-			return
+			peer.handleDeleteConnection(dupConn)
+			return false
 		}
 	}
 	if err := peer.checkConnectionLimit(); err != nil {
 		conn.Shutdown(err)
-		return
+		return false
 	}
 	peer.addConnection(conn)
-	conn.log("connection added")
+	return true
 }
 
-func (peer *LocalPeer) handleDeleteConnection(conn *LocalConnection) {
+func (peer *LocalPeer) handleDeleteConnection(conn Connection) bool {
 	if peer.Peer != conn.Local() {
 		log.Fatal("Attempt made to delete connection from peer where peer is not the source of connection")
 	}
@@ -222,29 +230,29 @@ func (peer *LocalPeer) handleDeleteConnection(conn *LocalConnection) {
 	}
 	toName := conn.Remote().Name
 	if connFound, found := peer.connections[toName]; !found || connFound != conn {
-		return
+		return false
 	}
 	peer.deleteConnection(conn)
-	conn.log("connection deleted")
 	// Must do garbage collection first to ensure we don't send out an
 	// update with unreachable peers (can cause looping)
 	peer.Router.Peers.GarbageCollect()
 	if conn.Established() {
 		peer.broadcastPeerUpdate()
 	}
+	return true
 }
 
-func (peer *LocalPeer) handleConnectionEstablished(conn *LocalConnection) {
+func (peer *LocalPeer) handleConnectionEstablished(conn Connection) bool {
 	if peer.Peer != conn.Local() {
 		log.Fatal("Peer informed of active connection where peer is not the source of connection")
 	}
 	if dupConn, found := peer.connections[conn.Remote().Name]; !found || conn != dupConn {
 		conn.Shutdown(fmt.Errorf("Cannot set unknown connection active"))
-		return
+		return false
 	}
 	peer.connectionEstablished(conn)
-	conn.log("connection fully established")
 	peer.broadcastPeerUpdate(conn.Remote())
+	return true
 }
 
 func (peer *LocalPeer) handleSendProtocolMsg(m ProtocolMsg) {