Permalink
Browse files

Rewrite some of Populate, add a bunch of tests, update client

  • Loading branch information...
xthexder committed Sep 7, 2016
1 parent af07d9e commit 2526adc590a5f1e80d37dce7e39a68045eee250c
Showing with 219 additions and 109 deletions.
  1. +33 −31 api.go
  2. +141 −42 api_test.go
  3. +18 −32 client/client.go
  4. +1 −1 proxy.go
  5. +23 −0 proxy_collection.go
  6. +2 −2 proxy_test.go
  7. +1 −1 toxics/toxic_test.go
View
64 api.go
@@ -132,16 +132,19 @@ func (server *ApiServer) ProxyCreate(response http.ResponseWriter, request *http
}
func (server *ApiServer) Populate(response http.ResponseWriter, request *http.Request) {
- input := []Proxy{}
+ input := []struct {
+ Proxy
+ Enabled *bool `json:"enabled"` // Overrides Proxy field to make field nullable
+ }{}
err := json.NewDecoder(request.Body).Decode(&input)
if apiError(response, joinError(err, ErrBadRequestBody)) {
return
}
- proxiesCreated := []*Proxy{}
-
- for _, p := range input {
+ // Check for valid input before creating any proxies
+ t := true
+ for i, p := range input {
if len(p.Name) < 1 {
apiError(response, joinError(fmt.Errorf("name"), ErrMissingField))
return
@@ -150,42 +153,49 @@ func (server *ApiServer) Populate(response http.ResponseWriter, request *http.Re
apiError(response, joinError(fmt.Errorf("upstream"), ErrMissingField))
return
}
+ if p.Enabled == nil {
+ input[i].Enabled = &t
+ }
+ }
- // TODO(jpittis) Defaulting to enabled is not standard but I dont see another way.
- p.Enabled = true
+ proxies := make([]proxyToxics, 0, len(input))
+ responseCode := http.StatusCreated
+ var apiErr *ApiError
+ for _, p := range input {
proxy := NewProxy()
proxy.Name = p.Name
proxy.Listen = p.Listen
proxy.Upstream = p.Upstream
- // TODO(jpittis) Tried to make this transactional. Kinda a mess of error handling code.
- err = server.Collection.Add(proxy, p.Enabled)
+ err = server.Collection.AddOrReplace(proxy, *p.Enabled)
if err != nil {
- for _, p := range proxiesCreated {
- err := server.Collection.Remove(p.Name)
- if apiError(response, err) {
- return
- }
- }
- if apiError(response, joinError(err, ErrBadRequestBody)) {
- return
+ var ok bool
+ apiErr, ok = err.(*ApiError)
+ if !ok {
+ logrus.Warn("Error did not include status code: ", err)
+ apiErr = &ApiError{err.Error(), http.StatusInternalServerError}
}
+ responseCode = apiErr.StatusCode
+ break
}
- proxiesCreated = append(proxiesCreated, proxy)
+ proxies = append(proxies, proxyWithToxics(proxy))
}
- data, err := json.Marshal(proxiesWithToxics(proxiesCreated))
+ data, err := json.Marshal(struct {
+ *ApiError `json:",omitempty"`
+ Proxies []proxyToxics `json:"proxies"`
+ }{apiErr, proxies})
if apiError(response, err) {
return
}
response.Header().Set("Content-Type", "application/json")
- response.WriteHeader(http.StatusCreated)
+ response.WriteHeader(responseCode)
_, err = response.Write(data)
if err != nil {
- logrus.Warn("ProxyCreate: Failed to write response to client", err)
+ logrus.Warn("Populate: Failed to write response to client", err)
}
}
@@ -382,7 +392,7 @@ func (server *ApiServer) Version(response http.ResponseWriter, request *http.Req
}
type ApiError struct {
- Message string `json:"title"`
+ Message string `json:"error"`
StatusCode int `json:"status"`
}
@@ -415,7 +425,7 @@ var (
func apiError(resp http.ResponseWriter, err error) bool {
obj, ok := err.(*ApiError)
if !ok && err != nil {
- logrus.Warn("Error did not include status code:", err)
+ logrus.Warn("Error did not include status code: ", err)
obj = &ApiError{err.Error(), http.StatusInternalServerError}
}
@@ -425,7 +435,7 @@ func apiError(resp http.ResponseWriter, err error) bool {
data, err2 := json.Marshal(obj)
if err2 != nil {
- logrus.Warn("Error json encoding error (╯°□°)╯︵ ┻━┻", err2)
+ logrus.Warn("Error json encoding error (╯°□°)╯︵ ┻━┻ ", err2)
}
resp.Header().Set("Content-Type", "application/json")
http.Error(resp, string(data), obj.StatusCode)
@@ -443,11 +453,3 @@ func proxyWithToxics(proxy *Proxy) (result proxyToxics) {
result.Toxics = proxy.Toxics.GetToxicArray()
return
}
-
-func proxiesWithToxics(proxies []*Proxy) []proxyToxics {
- result := make([]proxyToxics, len(proxies))
- for i := range proxies {
- result[i] = proxyWithToxics(proxies[i])
- }
- return result
-}
View
@@ -1,9 +1,9 @@
package toxiproxy_test
import (
+ "bytes"
"io/ioutil"
"net/http"
- "strconv"
"testing"
"time"
@@ -74,51 +74,186 @@ func TestCreateProxyBlankUpstream(t *testing.T) {
func TestPopulateProxy(t *testing.T) {
WithServer(t, func(addr string) {
- testProxies, err := client.Populate2([]tclient.Proxy{
+ testProxies, err := client.Populate([]tclient.Proxy{
{
Name: "one",
Listen: "localhost:7070",
Upstream: "localhost:7171",
+ Enabled: true,
},
{
Name: "two",
Listen: "localhost:7373",
Upstream: "localhost:7474",
+ Enabled: true,
},
})
+ if err != nil {
+ t.Fatal("Unable to populate:", err)
+ } else if len(testProxies) != 2 {
+ t.Fatalf("Wrong number of proxies returned: %d != 2", len(testProxies))
+ } else if testProxies[0].Name != "one" || testProxies[1].Name != "two" {
+ t.Fatalf("Wrong proxy names returned: %s, %s", testProxies[0].Name, testProxies[1].Name)
+ }
+
for _, p := range testProxies {
AssertProxyUp(t, p.Listen, true)
}
+ })
+}
+
+func TestPopulateDefaultEnabled(t *testing.T) {
+ WithServer(t, func(addr string) {
+ request := []byte(`[{"name": "test", "listen": "localhost:7070", "upstream": "localhost:7171"}]`)
+
+ resp, err := http.Post(addr+"/populate", "application/json", bytes.NewReader(request))
+ if err != nil {
+ t.Fatal("Failed to send POST to /populate:", err)
+ }
+
+ if resp.StatusCode != http.StatusCreated {
+ message, _ := ioutil.ReadAll(resp.Body)
+ t.Fatalf("Failed to populate proxy list: HTTP %s\n%s", resp.Status, string(message))
+ }
+
+ proxies, err := client.Proxies()
+ if err != nil {
+ t.Fatal(err)
+ } else if len(proxies) != 1 {
+ t.Fatalf("Wrong number of proxies created: %d != 1", len(proxies))
+ } else if _, ok := proxies["test"]; !ok {
+ t.Fatalf("Wrong proxy name returned")
+ }
+
+ for _, p := range proxies {
+ AssertProxyUp(t, p.Listen, true)
+ }
+ })
+}
+
+func TestPopulateExistingProxy(t *testing.T) {
+ WithServer(t, func(addr string) {
+ testProxy, err := client.CreateProxy("one", "localhost:7070", "localhost:7171")
+ if err != nil {
+ t.Fatal("Unable to create proxy:", err)
+ }
+ _, err = client.CreateProxy("two", "localhost:7373", "localhost:7474")
+ if err != nil {
+ t.Fatal("Unable to create proxy:", err)
+ }
+
+ // Create a toxic so we can make sure the proxy wasn't replaced
+ _, err = testProxy.AddToxic("", "latency", "downstream", 1, nil)
+ if err != nil {
+ t.Fatal("Unable to create toxic:", err)
+ }
+
+ testProxies, err := client.Populate([]tclient.Proxy{
+ {
+ Name: "one",
+ Listen: "127.0.0.1:7070", // TODO(xthexder): Will replace existing proxy if not resolved ip...
+ Upstream: "localhost:7171",
+ Enabled: true,
+ },
+ {
+ Name: "two",
+ Listen: "127.0.0.1:7575",
+ Upstream: "localhost:7676",
+ Enabled: true,
+ },
+ })
if err != nil {
t.Fatal("Unable to populate:", err)
+ } else if len(testProxies) != 2 {
+ t.Fatalf("Wrong number of proxies returned: %d != 2", len(testProxies))
+ } else if testProxies[0].Name != "one" || testProxies[1].Name != "two" {
+ t.Fatalf("Wrong proxy names returned: %s, %s", testProxies[0].Name, testProxies[1].Name)
+ } else if testProxies[0].Listen != "127.0.0.1:7070" || testProxies[1].Listen != "127.0.0.1:7575" {
+ t.Fatalf("Wrong proxy listen addresses returned: %s, %s", testProxies[0].Listen, testProxies[1].Listen)
+ }
+
+ toxics, err := testProxy.Toxics()
+ if err != nil {
+ t.Fatal("Unable to get toxics:", err)
+ }
+ if len(toxics) != 1 || toxics[0].Type != "latency" {
+ t.Fatalf("Populate did not preseve existing proxy. (%d toxics)", len(toxics))
+ }
+
+ for _, p := range testProxies {
+ AssertProxyUp(t, p.Listen, true)
}
})
}
-func TestPopulateProxyWithBadDataShouldBeTransactional(t *testing.T) {
+func TestPopulateWithBadName(t *testing.T) {
WithServer(t, func(addr string) {
- _, err := client.Populate2([]tclient.Proxy{
+ testProxies, err := client.Populate([]tclient.Proxy{
{
Name: "one",
Listen: "localhost:7070",
Upstream: "localhost:7171",
+ Enabled: true,
+ },
+ {
+ Name: "",
+ Listen: "",
+ Enabled: true,
+ },
+ })
+
+ if err == nil {
+ t.Fatal("Expected Populate to fail.")
+ } else if err.Error() != "Populate: HTTP 400: missing required field: name" {
+ t.Fatal("Expected different error during populate:", err)
+ } else if len(testProxies) != 0 {
+ t.Fatalf("Wrong number of proxies returned: %d != 0", len(testProxies))
+ }
+
+ proxies, err := client.Proxies()
+ if err != nil {
+ t.Fatal(err)
+ } else if len(proxies) != 0 {
+ t.Fatalf("Expected no proxies to be created: %d != 0", len(proxies))
+ }
+ })
+}
+
+func TestPopulateProxyWithBadDataShouldReturnError(t *testing.T) {
+ WithServer(t, func(addr string) {
+ testProxies, err := client.Populate([]tclient.Proxy{
+ {
+ Name: "one",
+ Listen: "localhost:7070",
+ Upstream: "localhost:7171",
+ Enabled: true,
},
{
Name: "two",
Listen: "local373",
Upstream: "localhost:7474",
+ Enabled: true,
},
{
Name: "three",
Listen: "localhost:7575",
Upstream: "localhost:7676",
+ Enabled: true,
},
})
if err == nil {
t.Fatal("Expected Populate to fail.")
+ } else if len(testProxies) != 1 {
+ t.Fatalf("Wrong number of proxies returned: %d != %d", len(testProxies), 1)
+ } else if testProxies[0].Name != "one" {
+ t.Fatalf("Wrong proxy name returned: %s != one", testProxies[0].Name)
+ }
+
+ for _, p := range testProxies {
+ AssertProxyUp(t, p.Listen, true)
}
proxies, err := client.Proxies()
@@ -127,8 +262,8 @@ func TestPopulateProxyWithBadDataShouldBeTransactional(t *testing.T) {
}
for _, p := range proxies {
- if p.Name == "one" || p.Name == "two" || p.Name == "three" {
- t.Fatalf("Proxy %s was still exists. Populate was not transactional.")
+ if p.Name == "two" || p.Name == "three" {
+ t.Fatalf("Proxy %s exists, populate did not fail correctly.")
}
}
})
@@ -762,42 +897,6 @@ func TestInvalidStream(t *testing.T) {
})
}
-func BenchmarkPopulateProxyList(b *testing.B) {
- WithServer(nil, func(addr string) {
- proxyList := make([]tclient.Proxy, b.N)
- for i := 0; i < len(proxyList); i++ {
- proxyList[i] = tclient.Proxy{
- Name: "test" + strconv.Itoa(i),
- Listen: "127.0.0.1:0",
- Upstream: "localhost:7171",
- }
- }
- _, err := client.Populate(proxyList)
-
- if err != nil {
- b.Fatal("Unable to populate:", err)
- }
- })
-}
-
-func BenchmarkPopulateProxyList2(b *testing.B) {
- WithServer(nil, func(addr string) {
- proxyList := make([]tclient.Proxy, b.N)
- for i := 0; i < len(proxyList); i++ {
- proxyList[i] = tclient.Proxy{
- Name: "test" + strconv.Itoa(i),
- Listen: "127.0.0.1:0",
- Upstream: "localhost:7171",
- }
- }
- _, err := client.Populate2(proxyList)
-
- if err != nil {
- b.Fatal("Unable to populate:", err)
- }
- })
-}
-
func AssertToxicExists(t *testing.T, toxics tclient.Toxics, name, typeName, stream string, exists bool) *tclient.Toxic {
var toxic *tclient.Toxic
var actualType, actualStream string
Oops, something went wrong.

0 comments on commit 2526adc

Please sign in to comment.