Merge pull request #1423 from bcwaldon/http-refactor-hdlrs

eradicate serverHandler

etcdserver/etcdhttp/client.go

@@ -49,28 +49,46 @@ const (
 
 // NewClientHandler generates a muxed http.Handler with the given parameters to serve etcd client requests.
 func NewClientHandler(server *etcdserver.EtcdServer) http.Handler {
-	sh := &serverHandler{
+	kh := &keysHandler{
+		server:  server,
+		timer:   server,
+		timeout: defaultServerTimeout,
+	}
+
+	sh := &statsHandler{
+		stats: server,
+	}
+
+	amh := &adminMembersHandler{
 		server:      server,
 		clusterInfo: server.Cluster,
-		stats:       server,
-		timer:       server,
-		timeout:     defaultServerTimeout,
 		clock:       clockwork.NewRealClock(),
 	}
+
+	dmh := &deprecatedMachinesHandler{
+		clusterInfo: server.Cluster,
+	}
+
 	mux := http.NewServeMux()
-	mux.HandleFunc(keysPrefix, sh.serveKeys)
-	mux.HandleFunc(keysPrefix+"/", sh.serveKeys)
-	mux.HandleFunc(statsPrefix+"/store", sh.serveStoreStats)
-	mux.HandleFunc(statsPrefix+"/self", sh.serveSelfStats)
-	mux.HandleFunc(statsPrefix+"/leader", sh.serveLeaderStats)
-	mux.HandleFunc(deprecatedMachinesPrefix, sh.serveMachines)
-	mux.HandleFunc(adminMembersPrefix, sh.serveAdminMembers)
-	mux.HandleFunc(versionPrefix, sh.serveVersion)
 	mux.HandleFunc("/", http.NotFound)
+	mux.HandleFunc(versionPrefix, serveVersion)
+	mux.Handle(keysPrefix, kh)
+	mux.Handle(keysPrefix+"/", kh)
+	mux.HandleFunc(statsPrefix+"/store", sh.serveStore)
+	mux.HandleFunc(statsPrefix+"/self", sh.serveSelf)
+	mux.HandleFunc(statsPrefix+"/leader", sh.serveLeader)
+	mux.Handle(adminMembersPrefix, amh)
+	mux.Handle(deprecatedMachinesPrefix, dmh)
 	return mux
 }
 
-func (h serverHandler) serveKeys(w http.ResponseWriter, r *http.Request) {
+type keysHandler struct {
+	server  etcdserver.Server
+	timer   etcdserver.RaftTimer
+	timeout time.Duration
+}
+
+func (h *keysHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if !allowMethod(w, r.Method, "GET", "PUT", "POST", "DELETE") {
 		return
 	}
@@ -106,8 +124,11 @@ func (h serverHandler) serveKeys(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-// serveMachines responds address list in the format '0.0.0.0, 1.1.1.1'.
-func (h serverHandler) serveMachines(w http.ResponseWriter, r *http.Request) {
+type deprecatedMachinesHandler struct {
+	clusterInfo etcdserver.ClusterInfo
+}
+
+func (h *deprecatedMachinesHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if !allowMethod(w, r.Method, "GET", "HEAD") {
 		return
 	}
@@ -115,7 +136,13 @@ func (h serverHandler) serveMachines(w http.ResponseWriter, r *http.Request) {
 	w.Write([]byte(strings.Join(endpoints, ", ")))
 }
 
-func (h serverHandler) serveAdminMembers(w http.ResponseWriter, r *http.Request) {
+type adminMembersHandler struct {
+	server      etcdserver.Server
+	clusterInfo etcdserver.ClusterInfo
+	clock       clockwork.Clock
+}
+
+func (h *adminMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if !allowMethod(w, r.Method, "GET", "POST", "DELETE") {
 		return
 	}
@@ -188,7 +215,11 @@ func (h serverHandler) serveAdminMembers(w http.ResponseWriter, r *http.Request)
 	}
 }
 
-func (h serverHandler) serveStoreStats(w http.ResponseWriter, r *http.Request) {
+type statsHandler struct {
+	stats etcdserver.Stats
+}
+
+func (h *statsHandler) serveStore(w http.ResponseWriter, r *http.Request) {
 	if !allowMethod(w, r.Method, "GET") {
 		return
 	}
@@ -196,7 +227,7 @@ func (h serverHandler) serveStoreStats(w http.ResponseWriter, r *http.Request) {
 	w.Write(h.stats.StoreStats())
 }
 
-func (h serverHandler) serveSelfStats(w http.ResponseWriter, r *http.Request) {
+func (h *statsHandler) serveSelf(w http.ResponseWriter, r *http.Request) {
 	if !allowMethod(w, r.Method, "GET") {
 		return
 	}
@@ -204,7 +235,7 @@ func (h serverHandler) serveSelfStats(w http.ResponseWriter, r *http.Request) {
 	w.Write(h.stats.SelfStats())
 }
 
-func (h serverHandler) serveLeaderStats(w http.ResponseWriter, r *http.Request) {
+func (h *statsHandler) serveLeader(w http.ResponseWriter, r *http.Request) {
 	if !allowMethod(w, r.Method, "GET") {
 		return
 	}
@@ -212,7 +243,7 @@ func (h serverHandler) serveLeaderStats(w http.ResponseWriter, r *http.Request)
 	w.Write(h.stats.LeaderStats())
 }
 
-func (h serverHandler) serveVersion(w http.ResponseWriter, r *http.Request) {
+func serveVersion(w http.ResponseWriter, r *http.Request) {
 	if !allowMethod(w, r.Method, "GET") {
 		return
 	}

etcdserver/etcdhttp/client_test.go

@@ -553,7 +553,7 @@ func TestServeAdminMembers(t *testing.T) {
 	cluster := &fakeCluster{
 		members: map[uint64]*etcdserver.Member{1: &memb1, 2: &memb2},
 	}
-	h := &serverHandler{
+	h := &adminMembersHandler{
 		server:      &serverRecorder{},
 		clock:       clockwork.NewFakeClock(),
 		clusterInfo: cluster,
@@ -588,7 +588,7 @@ func TestServeAdminMembers(t *testing.T) {
 			t.Fatal(err)
 		}
 		rw := httptest.NewRecorder()
-		h.serveAdminMembers(rw, req)
+		h.ServeHTTP(rw, req)
 
 		if rw.Code != tt.wcode {
 			t.Errorf("#%d: code=%d, want %d", i, rw.Code, tt.wcode)
@@ -616,13 +616,13 @@ func TestServeAdminMembersPut(t *testing.T) {
 	}
 	req.Header.Set("Content-Type", "application/json")
 	s := &serverRecorder{}
-	h := &serverHandler{
+	h := &adminMembersHandler{
 		server: s,
 		clock:  clockwork.NewFakeClock(),
 	}
 	rw := httptest.NewRecorder()
 
-	h.serveAdminMembers(rw, req)
+	h.ServeHTTP(rw, req)
 
 	wcode := http.StatusCreated
 	if rw.Code != wcode {
@@ -658,12 +658,12 @@ func TestServeAdminMembersDelete(t *testing.T) {
 		URL:    mustNewURL(t, path.Join(adminMembersPrefix, "BEEF")),
 	}
 	s := &serverRecorder{}
-	h := &serverHandler{
+	h := &adminMembersHandler{
 		server: s,
 	}
 	rw := httptest.NewRecorder()
 
-	h.serveAdminMembers(rw, req)
+	h.ServeHTTP(rw, req)
 
 	wcode := http.StatusNoContent
 	if rw.Code != wcode {
@@ -767,12 +767,12 @@ func TestServeAdminMembersFail(t *testing.T) {
 		},
 	}
 	for i, tt := range tests {
-		h := &serverHandler{
+		h := &adminMembersHandler{
 			server: tt.server,
 			clock:  clockwork.NewFakeClock(),
 		}
 		rw := httptest.NewRecorder()
-		h.serveAdminMembers(rw, tt.req)
+		h.ServeHTTP(rw, tt.req)
 		if rw.Code != tt.wcode {
 			t.Errorf("#%d: code=%d, want %d", i, rw.Code, tt.wcode)
 		}
@@ -884,8 +884,8 @@ func TestServeMachines(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	h := &serverHandler{clusterInfo: cluster}
-	h.serveMachines(writer, req)
+	h := &deprecatedMachinesHandler{clusterInfo: cluster}
+	h.ServeHTTP(writer, req)
 	w := "http://localhost:8080, http://localhost:8081, http://localhost:8082"
 	if g := writer.Body.String(); g != w {
 		t.Errorf("body = %s, want %s", g, w)
@@ -907,11 +907,11 @@ func (ds *dummyStats) UpdateRecvApp(_ uint64, _ int64) {}
 func TestServeSelfStats(t *testing.T) {
 	wb := []byte("some statistics")
 	w := string(wb)
-	sh := &serverHandler{
+	sh := &statsHandler{
 		stats: &dummyStats{data: wb},
 	}
 	rw := httptest.NewRecorder()
-	sh.serveSelfStats(rw, &http.Request{Method: "GET"})
+	sh.serveSelf(rw, &http.Request{Method: "GET"})
 	if rw.Code != http.StatusOK {
 		t.Errorf("code = %d, want %d", rw.Code, http.StatusOK)
 	}
@@ -926,9 +926,9 @@ func TestServeSelfStats(t *testing.T) {
 
 func TestSelfServeStatsBad(t *testing.T) {
 	for _, m := range []string{"PUT", "POST", "DELETE"} {
-		sh := &serverHandler{}
+		sh := &statsHandler{}
 		rw := httptest.NewRecorder()
-		sh.serveSelfStats(
+		sh.serveSelf(
 			rw,
 			&http.Request{
 				Method: m,
@@ -942,9 +942,9 @@ func TestSelfServeStatsBad(t *testing.T) {
 
 func TestLeaderServeStatsBad(t *testing.T) {
 	for _, m := range []string{"PUT", "POST", "DELETE"} {
-		sh := &serverHandler{}
+		sh := &statsHandler{}
 		rw := httptest.NewRecorder()
-		sh.serveLeaderStats(
+		sh.serveLeader(
 			rw,
 			&http.Request{
 				Method: m,
@@ -959,11 +959,11 @@ func TestLeaderServeStatsBad(t *testing.T) {
 func TestServeLeaderStats(t *testing.T) {
 	wb := []byte("some statistics")
 	w := string(wb)
-	sh := &serverHandler{
+	sh := &statsHandler{
 		stats: &dummyStats{data: wb},
 	}
 	rw := httptest.NewRecorder()
-	sh.serveLeaderStats(rw, &http.Request{Method: "GET"})
+	sh.serveLeader(rw, &http.Request{Method: "GET"})
 	if rw.Code != http.StatusOK {
 		t.Errorf("code = %d, want %d", rw.Code, http.StatusOK)
 	}
@@ -979,11 +979,11 @@ func TestServeLeaderStats(t *testing.T) {
 func TestServeStoreStats(t *testing.T) {
 	wb := []byte("some statistics")
 	w := string(wb)
-	sh := &serverHandler{
+	sh := &statsHandler{
 		stats: &dummyStats{data: wb},
 	}
 	rw := httptest.NewRecorder()
-	sh.serveStoreStats(rw, &http.Request{Method: "GET"})
+	sh.serveStore(rw, &http.Request{Method: "GET"})
 	if rw.Code != http.StatusOK {
 		t.Errorf("code = %d, want %d", rw.Code, http.StatusOK)
 	}
@@ -1002,9 +1002,8 @@ func TestServeVersion(t *testing.T) {
 	if err != nil {
 		t.Fatalf("error creating request: %v", err)
 	}
-	h := &serverHandler{}
 	rw := httptest.NewRecorder()
-	h.serveVersion(rw, req)
+	serveVersion(rw, req)
 	if rw.Code != http.StatusOK {
 		t.Errorf("code=%d, want %d", rw.Code, http.StatusOK)
 	}
@@ -1022,9 +1021,8 @@ func TestServeVersionFails(t *testing.T) {
 		if err != nil {
 			t.Fatalf("error creating request: %v", err)
 		}
-		h := &serverHandler{}
 		rw := httptest.NewRecorder()
-		h.serveVersion(rw, req)
+		serveVersion(rw, req)
 		if rw.Code != http.StatusMethodNotAllowed {
 			t.Errorf("method %s: code=%d, want %d", m, rw.Code, http.StatusMethodNotAllowed)
 		}
@@ -1102,12 +1100,12 @@ func TestBadServeKeys(t *testing.T) {
 		},
 	}
 	for i, tt := range testBadCases {
-		h := &serverHandler{
+		h := &keysHandler{
 			timeout: 0, // context times out immediately
 			server:  tt.server,
 		}
 		rw := httptest.NewRecorder()
-		h.serveKeys(rw, tt.req)
+		h.ServeHTTP(rw, tt.req)
 		if rw.Code != tt.wcode {
 			t.Errorf("#%d: got code=%d, want %d", i, rw.Code, tt.wcode)
 		}
@@ -1127,14 +1125,14 @@ func TestServeKeysEvent(t *testing.T) {
 			},
 		},
 	}
-	h := &serverHandler{
+	h := &keysHandler{
 		timeout: time.Hour,
 		server:  server,
 		timer:   &dummyRaftTimer{},
 	}
 	rw := httptest.NewRecorder()
 
-	h.serveKeys(rw, req)
+	h.ServeHTTP(rw, req)
 
 	wcode := http.StatusOK
 	wbody := mustMarshalEvent(
@@ -1165,7 +1163,7 @@ func TestServeKeysWatch(t *testing.T) {
 			Watcher: dw,
 		},
 	}
-	h := &serverHandler{
+	h := &keysHandler{
 		timeout: time.Hour,
 		server:  server,
 		timer:   &dummyRaftTimer{},
@@ -1178,7 +1176,7 @@ func TestServeKeysWatch(t *testing.T) {
 	}()
 	rw := httptest.NewRecorder()
 
-	h.serveKeys(rw, req)
+	h.ServeHTTP(rw, req)
 
 	wcode := http.StatusOK
 	wbody := mustMarshalEvent(

etcdserver/etcdhttp/http.go

@@ -23,9 +23,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/coreos/etcd/Godeps/_workspace/src/github.com/jonboulle/clockwork"
 	etcdErr "github.com/coreos/etcd/error"
-	"github.com/coreos/etcd/etcdserver"
 )
 
 const (
@@ -38,16 +36,6 @@ const (
 
 var errClosed = errors.New("etcdhttp: client closed connection")
 
-// serverHandler provides http.Handlers for etcd client and raft communication.
-type serverHandler struct {
-	timeout     time.Duration
-	server      etcdserver.Server
-	stats       etcdserver.Stats
-	timer       etcdserver.RaftTimer
-	clusterInfo etcdserver.ClusterInfo
-	clock       clockwork.Clock
-}
-
 // writeError logs and writes the given Error to the ResponseWriter
 // If Error is an etcdErr, it is rendered to the ResponseWriter
 // Otherwise, it is assumed to be an InternalServerError

etcdserver/etcdhttp/peer.go

@@ -24,32 +24,41 @@ import (
 	"strconv"
 
 	"github.com/coreos/etcd/Godeps/_workspace/src/code.google.com/p/go.net/context"
-	"github.com/coreos/etcd/Godeps/_workspace/src/github.com/jonboulle/clockwork"
 	"github.com/coreos/etcd/etcdserver"
 	"github.com/coreos/etcd/raft/raftpb"
 )
 
 const (
-	raftPrefix    = "/raft"
-	membersPrefix = "/members"
+	raftPrefix        = "/raft"
+	peerMembersPrefix = "/members"
 )
 
 // NewPeerHandler generates an http.Handler to handle etcd peer (raft) requests.
 func NewPeerHandler(server *etcdserver.EtcdServer) http.Handler {
-	sh := &serverHandler{
-		server:      server,
+	rh := &raftHandler{
 		stats:       server,
+		server:      server,
+		clusterInfo: server.Cluster,
+	}
+
+	mh := &peerMembersHandler{
 		clusterInfo: server.Cluster,
-		clock:       clockwork.NewRealClock(),
 	}
+
 	mux := http.NewServeMux()
-	mux.HandleFunc(raftPrefix, sh.serveRaft)
-	mux.HandleFunc(membersPrefix, sh.serveMembers)
 	mux.HandleFunc("/", http.NotFound)
+	mux.Handle(raftPrefix, rh)
+	mux.Handle(peerMembersPrefix, mh)
 	return mux
 }
 
-func (h serverHandler) serveRaft(w http.ResponseWriter, r *http.Request) {
+type raftHandler struct {
+	stats       etcdserver.Stats
+	server      etcdserver.Server
+	clusterInfo etcdserver.ClusterInfo
+}
+
+func (h *raftHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if !allowMethod(w, r.Method, "POST") {
 		return
 	}
@@ -92,14 +101,18 @@ func (h serverHandler) serveRaft(w http.ResponseWriter, r *http.Request) {
 	w.WriteHeader(http.StatusNoContent)
 }
 
-func (h serverHandler) serveMembers(w http.ResponseWriter, r *http.Request) {
+type peerMembersHandler struct {
+	clusterInfo etcdserver.ClusterInfo
+}
+
+func (h *peerMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if !allowMethod(w, r.Method, "GET") {
 		return
 	}
 	cid := strconv.FormatUint(h.clusterInfo.ID(), 16)
 	w.Header().Set("X-Etcd-Cluster-ID", cid)
 
-	if r.URL.Path != membersPrefix {
+	if r.URL.Path != peerMembersPrefix {
 		http.Error(w, "bad path", http.StatusBadRequest)
 		return
 	}

etcdserver/etcdhttp/peer_test.go

@@ -27,7 +27,6 @@ import (
 	"strconv"
 	"strings"
 	"testing"
-	"time"
 
 	"github.com/coreos/etcd/etcdserver"
 	"github.com/coreos/etcd/raft/raftpb"
@@ -169,13 +168,9 @@ func TestServeRaft(t *testing.T) {
 			t.Fatalf("#%d: could not create request: %#v", i, err)
 		}
 		req.Header.Set("X-Etcd-Cluster-ID", tt.clusterID)
-		h := &serverHandler{
-			timeout:     time.Hour,
-			server:      &errServer{tt.serverErr},
-			clusterInfo: &fakeCluster{id: 0},
-		}
 		rw := httptest.NewRecorder()
-		h.serveRaft(rw, req)
+		h := &raftHandler{stats: nil, server: &errServer{tt.serverErr}, clusterInfo: &fakeCluster{id: 0}}
+		h.ServeHTTP(rw, req)
 		if rw.Code != tt.wcode {
 			t.Errorf("#%d: got code=%d, want %d", i, rw.Code, tt.wcode)
 		}
@@ -201,9 +196,9 @@ func TestServeMembersFails(t *testing.T) {
 		},
 	}
 	for i, tt := range tests {
-		h := &serverHandler{}
 		rw := httptest.NewRecorder()
-		h.serveMembers(rw, &http.Request{Method: tt.method})
+		h := &peerMembersHandler{clusterInfo: nil}
+		h.ServeHTTP(rw, &http.Request{Method: tt.method})
 		if rw.Code != tt.wcode {
 			t.Errorf("#%d: code=%d, want %d", i, rw.Code, tt.wcode)
 		}
@@ -217,11 +212,7 @@ func TestServeMembersGet(t *testing.T) {
 		id:      1,
 		members: map[uint64]*etcdserver.Member{1: &memb1, 2: &memb2},
 	}
-	h := &serverHandler{
-		server:      &serverRecorder{},
-		clusterInfo: cluster,
-	}
-
+	h := &peerMembersHandler{clusterInfo: cluster}
 	msb, err := json.Marshal([]etcdserver.Member{memb1, memb2})
 	if err != nil {
 		t.Fatal(err)
@@ -234,8 +225,8 @@ func TestServeMembersGet(t *testing.T) {
 		wct   string
 		wbody string
 	}{
-		{membersPrefix, http.StatusOK, "application/json", wms},
-		{path.Join(membersPrefix, "bad"), http.StatusBadRequest, "text/plain; charset=utf-8", "bad path\n"},
+		{peerMembersPrefix, http.StatusOK, "application/json", wms},
+		{path.Join(peerMembersPrefix, "bad"), http.StatusBadRequest, "text/plain; charset=utf-8", "bad path\n"},
 	}
 
 	for i, tt := range tests {
@@ -244,7 +235,7 @@ func TestServeMembersGet(t *testing.T) {
 			t.Fatal(err)
 		}
 		rw := httptest.NewRecorder()
-		h.serveMembers(rw, req)
+		h.ServeHTTP(rw, req)
 
 		if rw.Code != tt.wcode {
 			t.Errorf("#%d: code=%d, want %d", i, rw.Code, tt.wcode)