From 8b0eaa9e15f7eea95f269a48033e02c7c1ec04f4 Mon Sep 17 00:00:00 2001
From: Brian Waldon <bcwaldon@gmail.com>
Date: Tue, 28 Oct 2014 10:09:05 -0700
Subject: [PATCH] etcdhttp: separate member create deserialization

---
 etcdserver/etcdhttp/client.go                | 16 +++----
 etcdserver/etcdhttp/httptypes/member.go      | 25 +++++++++++
 etcdserver/etcdhttp/httptypes/member_test.go | 44 ++++++++++++++++++++
 3 files changed, 75 insertions(+), 10 deletions(-)

diff --git a/etcdserver/etcdhttp/client.go b/etcdserver/etcdhttp/client.go
index ce67ac929..7cf255bdd 100644
--- a/etcdserver/etcdhttp/client.go
+++ b/etcdserver/etcdhttp/client.go
@@ -36,7 +36,6 @@ import (
 	"github.com/coreos/etcd/etcdserver/etcdhttp/httptypes"
 	"github.com/coreos/etcd/etcdserver/etcdserverpb"
 	"github.com/coreos/etcd/pkg/strutil"
-	"github.com/coreos/etcd/pkg/types"
 	"github.com/coreos/etcd/store"
 	"github.com/coreos/etcd/version"
 )
@@ -181,24 +180,21 @@ func (h *adminMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 			writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, err.Error()))
 			return
 		}
-		raftAttr := etcdserver.RaftAttributes{}
-		if err := json.Unmarshal(b, &raftAttr); err != nil {
+		req := httptypes.MemberCreateRequest{}
+		if err := json.Unmarshal(b, &req); err != nil {
 			writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, err.Error()))
 			return
 		}
-		validURLs, err := types.NewURLs(raftAttr.PeerURLs)
-		if err != nil {
-			writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, "Bad peer urls"))
-			return
-		}
+
 		now := h.clock.Now()
-		m := etcdserver.NewMember("", validURLs, "", &now)
+		m := etcdserver.NewMember("", req.PeerURLs, "", &now)
 		if err := h.server.AddMember(ctx, *m); err != nil {
 			log.Printf("etcdhttp: error adding node %x: %v", m.ID, err)
 			writeError(w, err)
 			return
 		}
-		log.Printf("etcdhttp: added node %x with peer urls %v", m.ID, raftAttr.PeerURLs)
+		log.Printf("etcdhttp: added node %x with peer urls %v", m.ID, req.PeerURLs)
+
 		w.Header().Set("Content-Type", "application/json")
 		w.WriteHeader(http.StatusCreated)
 		if err := json.NewEncoder(w).Encode(m); err != nil {
diff --git a/etcdserver/etcdhttp/httptypes/member.go b/etcdserver/etcdhttp/httptypes/member.go
index 7d972daf0..06fd0ccb8 100644
--- a/etcdserver/etcdhttp/httptypes/member.go
+++ b/etcdserver/etcdhttp/httptypes/member.go
@@ -18,6 +18,8 @@ package httptypes
 
 import (
 	"encoding/json"
+
+	"github.com/coreos/etcd/pkg/types"
 )
 
 type Member struct {
@@ -27,6 +29,29 @@ type Member struct {
 	ClientURLs []string `json:"clientURLs"`
 }
 
+type MemberCreateRequest struct {
+	PeerURLs types.URLs
+}
+
+func (m *MemberCreateRequest) UnmarshalJSON(data []byte) error {
+	s := struct {
+		PeerURLs []string `json:"peerURLs"`
+	}{}
+
+	err := json.Unmarshal(data, &s)
+	if err != nil {
+		return err
+	}
+
+	urls, err := types.NewURLs(s.PeerURLs)
+	if err != nil {
+		return err
+	}
+
+	m.PeerURLs = urls
+	return nil
+}
+
 type MemberCollection []Member
 
 func (c *MemberCollection) MarshalJSON() ([]byte, error) {
diff --git a/etcdserver/etcdhttp/httptypes/member_test.go b/etcdserver/etcdhttp/httptypes/member_test.go
index 6c0b3cdf5..2b07c28b0 100644
--- a/etcdserver/etcdhttp/httptypes/member_test.go
+++ b/etcdserver/etcdhttp/httptypes/member_test.go
@@ -18,8 +18,11 @@ package httptypes
 
 import (
 	"encoding/json"
+	"net/url"
 	"reflect"
 	"testing"
+
+	"github.com/coreos/etcd/pkg/types"
 )
 
 func TestMemberUnmarshal(t *testing.T) {
@@ -155,3 +158,44 @@ func TestMemberCollectionUnmarshal(t *testing.T) {
 		}
 	}
 }
+
+func TestMemberCreateRequestUnmarshal(t *testing.T) {
+	body := []byte(`{"peerURLs": ["http://127.0.0.1:8081", "https://127.0.0.1:8080"]}`)
+	want := MemberCreateRequest{
+		PeerURLs: types.URLs([]url.URL{
+			url.URL{Scheme: "http", Host: "127.0.0.1:8081"},
+			url.URL{Scheme: "https", Host: "127.0.0.1:8080"},
+		}),
+	}
+
+	var req MemberCreateRequest
+	if err := json.Unmarshal(body, &req); err != nil {
+		t.Fatalf("Unmarshal returned unexpected err=%v", err)
+	}
+
+	if !reflect.DeepEqual(want, req) {
+		t.Fatalf("Failed to unmarshal MemberCreateRequest: want=%#v, got=%#v", want, req)
+	}
+}
+
+func TestMemberCreateRequestUnmarshalFail(t *testing.T) {
+	tests := [][]byte{
+		// invalid JSON
+		[]byte(``),
+		[]byte(`{`),
+
+		// spot-check validation done in types.NewURLs
+		[]byte(`{"peerURLs": "foo"}`),
+		[]byte(`{"peerURLs": ["."]}`),
+		[]byte(`{"peerURLs": []}`),
+		[]byte(`{"peerURLs": ["http://127.0.0.1:4001/foo"]}`),
+		[]byte(`{"peerURLs": ["http://127.0.0.1"]}`),
+	}
+
+	for i, tt := range tests {
+		var req MemberCreateRequest
+		if err := json.Unmarshal(tt, &req); err == nil {
+			t.Errorf("#%d: expected err, got nil", i)
+		}
+	}
+}