From 54a2d8ffc91cf6005724edb6e23fd6bc84374a7e Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Sun, 26 Oct 2014 17:28:47 -0700 Subject: [PATCH] client: move Member models to new types pkg --- client/members.go | 58 +------ client/members_test.go | 129 ---------------- etcdserver/etcdhttp/httptypes/member.go | 72 +++++++++ etcdserver/etcdhttp/httptypes/member_test.go | 151 +++++++++++++++++++ test | 2 +- 5 files changed, 229 insertions(+), 183 deletions(-) create mode 100644 etcdserver/etcdhttp/httptypes/member.go create mode 100644 etcdserver/etcdhttp/httptypes/member_test.go diff --git a/client/members.go b/client/members.go index 6b0e840e1..f2d936df0 100644 --- a/client/members.go +++ b/client/members.go @@ -23,6 +23,8 @@ import ( "net/url" "path" "time" + + "github.com/coreos/etcd/etcdserver/etcdhttp/httptypes" ) var ( @@ -51,64 +53,14 @@ func NewMembersAPI(tr *http.Transport, ep string, to time.Duration) (MembersAPI, } type MembersAPI interface { - List() ([]Member, error) -} - -type Member struct { - ID uint64 - Name string - PeerURLs []url.URL - ClientURLs []url.URL -} - -func (m *Member) UnmarshalJSON(data []byte) (err error) { - rm := struct { - ID uint64 - Name string - PeerURLs []string - ClientURLs []string - }{} - - if err := json.Unmarshal(data, &rm); err != nil { - return err - } - - parseURLs := func(strs []string) ([]url.URL, error) { - urls := make([]url.URL, len(strs)) - for i, s := range strs { - u, err := url.Parse(s) - if err != nil { - return nil, err - } - urls[i] = *u - } - - return urls, nil - } - - if m.PeerURLs, err = parseURLs(rm.PeerURLs); err != nil { - return err - } - - if m.ClientURLs, err = parseURLs(rm.ClientURLs); err != nil { - return err - } - - m.ID = rm.ID - m.Name = rm.Name - - return nil -} - -type membersCollection struct { - Members []Member + List() ([]httptypes.Member, error) } type httpMembersAPI struct { client *httpClient } -func (m *httpMembersAPI) List() ([]Member, error) { +func (m *httpMembersAPI) List() ([]httptypes.Member, error) { httpresp, body, err := m.client.doWithTimeout(&membersAPIActionList{}) if err != nil { return nil, err @@ -123,7 +75,7 @@ func (m *httpMembersAPI) List() ([]Member, error) { return nil, err } - var mCollection membersCollection + var mCollection httptypes.MemberCollection if err = mResponse.unmarshalBody(&mCollection); err != nil { return nil, err } diff --git a/client/members_test.go b/client/members_test.go index acd7d9e44..e061fc004 100644 --- a/client/members_test.go +++ b/client/members_test.go @@ -17,10 +17,8 @@ package client import ( - "encoding/json" "net/http" "net/url" - "reflect" "testing" ) @@ -39,130 +37,3 @@ func TestMembersAPIListAction(t *testing.T) { t.Errorf(err.Error()) } } - -func TestMembersAPIUnmarshalMember(t *testing.T) { - tests := []struct { - body []byte - wantMember Member - wantError bool - }{ - // no URLs, just check ID & Name - { - body: []byte(`{"id": 1, "name": "dungarees"}`), - wantMember: Member{ID: 1, Name: "dungarees", PeerURLs: []url.URL{}, ClientURLs: []url.URL{}}, - }, - - // both client and peer URLs - { - body: []byte(`{"peerURLs": ["http://127.0.0.1:4001"], "clientURLs": ["http://127.0.0.1:4001"]}`), - wantMember: Member{ - PeerURLs: []url.URL{ - {Scheme: "http", Host: "127.0.0.1:4001"}, - }, - ClientURLs: []url.URL{ - {Scheme: "http", Host: "127.0.0.1:4001"}, - }, - }, - }, - - // multiple peer URLs - { - body: []byte(`{"peerURLs": ["http://127.0.0.1:4001", "https://example.com"]}`), - wantMember: Member{ - PeerURLs: []url.URL{ - {Scheme: "http", Host: "127.0.0.1:4001"}, - {Scheme: "https", Host: "example.com"}, - }, - ClientURLs: []url.URL{}, - }, - }, - - // multiple client URLs - { - body: []byte(`{"clientURLs": ["http://127.0.0.1:4001", "https://example.com"]}`), - wantMember: Member{ - PeerURLs: []url.URL{}, - ClientURLs: []url.URL{ - {Scheme: "http", Host: "127.0.0.1:4001"}, - {Scheme: "https", Host: "example.com"}, - }, - }, - }, - - // invalid JSON - { - body: []byte(`{"peerU`), - wantError: true, - }, - - // valid JSON, invalid URL - { - body: []byte(`{"peerURLs": [":"]}`), - wantError: true, - }, - } - - for i, tt := range tests { - got := Member{} - err := json.Unmarshal(tt.body, &got) - if tt.wantError != (err != nil) { - t.Errorf("#%d: want error %t, got %v", i, tt.wantError, err) - continue - } - - if !reflect.DeepEqual(tt.wantMember, got) { - t.Errorf("#%d: incorrect output: want=%#v, got=%#v", i, tt.wantMember, got) - } - } -} - -func TestMembersAPIUnmarshalMembers(t *testing.T) { - body := []byte(`{"members":[{"id":176869799018424574,"peerURLs":["http://127.0.0.1:7003"],"name":"node3","clientURLs":["http://127.0.0.1:4003"]},{"id":297577273835923749,"peerURLs":["http://127.0.0.1:2380","http://127.0.0.1:7001"],"name":"node1","clientURLs":["http://127.0.0.1:2379","http://127.0.0.1:4001"]},{"id":10666918107976480891,"peerURLs":["http://127.0.0.1:7002"],"name":"node2","clientURLs":["http://127.0.0.1:4002"]}]}`) - - want := membersCollection{ - Members: []Member{ - { - ID: 176869799018424574, - Name: "node3", - PeerURLs: []url.URL{ - {Scheme: "http", Host: "127.0.0.1:7003"}, - }, - ClientURLs: []url.URL{ - {Scheme: "http", Host: "127.0.0.1:4003"}, - }, - }, - { - ID: 297577273835923749, - Name: "node1", - PeerURLs: []url.URL{ - {Scheme: "http", Host: "127.0.0.1:2380"}, - {Scheme: "http", Host: "127.0.0.1:7001"}, - }, - ClientURLs: []url.URL{ - {Scheme: "http", Host: "127.0.0.1:2379"}, - {Scheme: "http", Host: "127.0.0.1:4001"}, - }, - }, - { - ID: 10666918107976480891, - Name: "node2", - PeerURLs: []url.URL{ - {Scheme: "http", Host: "127.0.0.1:7002"}, - }, - ClientURLs: []url.URL{ - {Scheme: "http", Host: "127.0.0.1:4002"}, - }, - }, - }, - } - - got := membersCollection{} - err := json.Unmarshal(body, &got) - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - - if !reflect.DeepEqual(want, got) { - t.Errorf("Incorrect output: want=%#v, got=%#v", want, got) - } -} diff --git a/etcdserver/etcdhttp/httptypes/member.go b/etcdserver/etcdhttp/httptypes/member.go new file mode 100644 index 000000000..b035bab45 --- /dev/null +++ b/etcdserver/etcdhttp/httptypes/member.go @@ -0,0 +1,72 @@ +/* + Copyright 2014 CoreOS, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package httptypes + +import ( + "encoding/json" + "net/url" +) + +type Member struct { + ID uint64 + Name string + PeerURLs []url.URL + ClientURLs []url.URL +} + +func (m *Member) UnmarshalJSON(data []byte) (err error) { + rm := struct { + ID uint64 + Name string + PeerURLs []string + ClientURLs []string + }{} + + if err := json.Unmarshal(data, &rm); err != nil { + return err + } + + parseURLs := func(strs []string) ([]url.URL, error) { + urls := make([]url.URL, len(strs)) + for i, s := range strs { + u, err := url.Parse(s) + if err != nil { + return nil, err + } + urls[i] = *u + } + + return urls, nil + } + + if m.PeerURLs, err = parseURLs(rm.PeerURLs); err != nil { + return err + } + + if m.ClientURLs, err = parseURLs(rm.ClientURLs); err != nil { + return err + } + + m.ID = rm.ID + m.Name = rm.Name + + return nil +} + +type MemberCollection struct { + Members []Member +} diff --git a/etcdserver/etcdhttp/httptypes/member_test.go b/etcdserver/etcdhttp/httptypes/member_test.go new file mode 100644 index 000000000..653a797de --- /dev/null +++ b/etcdserver/etcdhttp/httptypes/member_test.go @@ -0,0 +1,151 @@ +/* + Copyright 2014 CoreOS, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package httptypes + +import ( + "encoding/json" + "net/url" + "reflect" + "testing" +) + +func TestMemberUnmarshal(t *testing.T) { + tests := []struct { + body []byte + wantMember Member + wantError bool + }{ + // no URLs, just check ID & Name + { + body: []byte(`{"id": 1, "name": "dungarees"}`), + wantMember: Member{ID: 1, Name: "dungarees", PeerURLs: []url.URL{}, ClientURLs: []url.URL{}}, + }, + + // both client and peer URLs + { + body: []byte(`{"peerURLs": ["http://127.0.0.1:4001"], "clientURLs": ["http://127.0.0.1:4001"]}`), + wantMember: Member{ + PeerURLs: []url.URL{ + {Scheme: "http", Host: "127.0.0.1:4001"}, + }, + ClientURLs: []url.URL{ + {Scheme: "http", Host: "127.0.0.1:4001"}, + }, + }, + }, + + // multiple peer URLs + { + body: []byte(`{"peerURLs": ["http://127.0.0.1:4001", "https://example.com"]}`), + wantMember: Member{ + PeerURLs: []url.URL{ + {Scheme: "http", Host: "127.0.0.1:4001"}, + {Scheme: "https", Host: "example.com"}, + }, + ClientURLs: []url.URL{}, + }, + }, + + // multiple client URLs + { + body: []byte(`{"clientURLs": ["http://127.0.0.1:4001", "https://example.com"]}`), + wantMember: Member{ + PeerURLs: []url.URL{}, + ClientURLs: []url.URL{ + {Scheme: "http", Host: "127.0.0.1:4001"}, + {Scheme: "https", Host: "example.com"}, + }, + }, + }, + + // invalid JSON + { + body: []byte(`{"peerU`), + wantError: true, + }, + + // valid JSON, invalid URL + { + body: []byte(`{"peerURLs": [":"]}`), + wantError: true, + }, + } + + for i, tt := range tests { + got := Member{} + err := json.Unmarshal(tt.body, &got) + if tt.wantError != (err != nil) { + t.Errorf("#%d: want error %t, got %v", i, tt.wantError, err) + continue + } + + if !reflect.DeepEqual(tt.wantMember, got) { + t.Errorf("#%d: incorrect output: want=%#v, got=%#v", i, tt.wantMember, got) + } + } +} + +func TestMemberCollectionUnmarshal(t *testing.T) { + body := []byte(`{"members":[{"id":176869799018424574,"peerURLs":["http://127.0.0.1:7003"],"name":"node3","clientURLs":["http://127.0.0.1:4003"]},{"id":297577273835923749,"peerURLs":["http://127.0.0.1:2380","http://127.0.0.1:7001"],"name":"node1","clientURLs":["http://127.0.0.1:2379","http://127.0.0.1:4001"]},{"id":10666918107976480891,"peerURLs":["http://127.0.0.1:7002"],"name":"node2","clientURLs":["http://127.0.0.1:4002"]}]}`) + + want := MemberCollection{ + Members: []Member{ + { + ID: 176869799018424574, + Name: "node3", + PeerURLs: []url.URL{ + {Scheme: "http", Host: "127.0.0.1:7003"}, + }, + ClientURLs: []url.URL{ + {Scheme: "http", Host: "127.0.0.1:4003"}, + }, + }, + { + ID: 297577273835923749, + Name: "node1", + PeerURLs: []url.URL{ + {Scheme: "http", Host: "127.0.0.1:2380"}, + {Scheme: "http", Host: "127.0.0.1:7001"}, + }, + ClientURLs: []url.URL{ + {Scheme: "http", Host: "127.0.0.1:2379"}, + {Scheme: "http", Host: "127.0.0.1:4001"}, + }, + }, + { + ID: 10666918107976480891, + Name: "node2", + PeerURLs: []url.URL{ + {Scheme: "http", Host: "127.0.0.1:7002"}, + }, + ClientURLs: []url.URL{ + {Scheme: "http", Host: "127.0.0.1:4002"}, + }, + }, + }, + } + + got := MemberCollection{} + err := json.Unmarshal(body, &got) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + if !reflect.DeepEqual(want, got) { + t.Errorf("Incorrect output: want=%#v, got=%#v", want, got) + } +} diff --git a/test b/test index e5b0ca688..6433eca30 100755 --- a/test +++ b/test @@ -15,7 +15,7 @@ COVER=${COVER:-"-cover"} source ./build # Hack: gofmt ./ will recursively check the .git directory. So use *.go for gofmt. -TESTABLE_AND_FORMATTABLE="client discovery etcdctl/command etcdserver etcdserver/etcdhttp etcdserver/etcdserverpb integration pkg/flags pkg/transport proxy raft snap store wait wal" +TESTABLE_AND_FORMATTABLE="client discovery etcdctl/command etcdserver etcdserver/etcdhttp etcdserver/etcdhttp/httptypes etcdserver/etcdserverpb integration pkg/flags pkg/transport proxy raft snap store wait wal" TESTABLE="$TESTABLE_AND_FORMATTABLE ./" FORMATTABLE="$TESTABLE_AND_FORMATTABLE *.go etcdctl/"