From 510563b67fd2a5aba8bb6512d18c13b851fd6949 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 19 Aug 2017 11:17:25 -0700 Subject: [PATCH] Fully test conf handling in httpFactory --- state/remote/http.go | 18 +++++---- state/remote/http_test.go | 80 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 10 deletions(-) diff --git a/state/remote/http.go b/state/remote/http.go index 86f9a4e5b..3d630adb8 100644 --- a/state/remote/http.go +++ b/state/remote/http.go @@ -35,9 +35,9 @@ func httpFactory(conf map[string]string) (Client, error) { } var lockURL *url.URL - lockAddress, ok := conf["lock_address"] - if ok { - lockURL, err := url.Parse(lockAddress) + if lockAddress, ok := conf["lock_address"]; ok { + var err error + lockURL, err = url.Parse(lockAddress) if err != nil { return nil, fmt.Errorf("failed to parse lockAddress URL: %s", err) } @@ -53,9 +53,9 @@ func httpFactory(conf map[string]string) (Client, error) { } var unlockURL *url.URL - unlockAddress, ok := conf["unlock_address"] - if ok { - unlockURL, err := url.Parse(unlockAddress) + if unlockAddress, ok := conf["unlock_address"]; ok { + var err error + unlockURL, err = url.Parse(unlockAddress) if err != nil { return nil, fmt.Errorf("failed to parse unlockAddress URL: %s", err) } @@ -97,9 +97,11 @@ func httpFactory(conf map[string]string) (Client, error) { UnlockURL: unlockURL, UnlockMethod: unlockMethod, - Client: client, Username: conf["username"], Password: conf["password"], + + // accessible only for testing use + Client: client, } return ret, nil @@ -185,7 +187,7 @@ func (c *HTTPClient) Lock(info *state.LockInfo) (string, error) { return "", fmt.Errorf("HTTP remote state endpoint requires auth") case http.StatusForbidden: return "", fmt.Errorf("HTTP remote state endpoint invalid auth") - case http.StatusConflict: + case http.StatusConflict, http.StatusLocked: defer resp.Body.Close() body, err := ioutil.ReadAll(resp.Body) if err != nil { diff --git a/state/remote/http_test.go b/state/remote/http_test.go index bed0c5937..10270bd22 100644 --- a/state/remote/http_test.go +++ b/state/remote/http_test.go @@ -27,11 +27,14 @@ func TestHTTPClient(t *testing.T) { t.Fatalf("err: %s", err) } + // Test basic get/store client := &HTTPClient{URL: url, Client: cleanhttp.DefaultClient()} testClient(t, client) + // Test locking and alternative StoreMethod a := &HTTPClient{ URL: url, + StoreMethod: "PUT", LockURL: url, LockMethod: "LOCK", UnlockURL: url, @@ -40,6 +43,7 @@ func TestHTTPClient(t *testing.T) { } b := &HTTPClient{ URL: url, + StoreMethod: "PUT", LockURL: url, LockMethod: "LOCK", UnlockURL: url, @@ -47,6 +51,78 @@ func TestHTTPClient(t *testing.T) { Client: cleanhttp.DefaultClient(), } TestRemoteLocks(t, a, b) + +} + +func assertError(t *testing.T, err error, expected string) { + if err == nil { + t.Fatalf("Expected empty config to err") + } else if err.Error() != expected { + t.Fatalf("Expected err.Error() to be \"%s\", got \"%s\"", expected, err.Error()) + } +} + +func TestHTTPClientFactory(t *testing.T) { + // missing address + _, err := httpFactory(map[string]string{}) + assertError(t, err, "missing 'address' configuration") + + // defaults + conf := map[string]string{ + "address": "http://127.0.0.1:8888/foo", + } + c, err := httpFactory(conf) + client, _ := c.(*HTTPClient) + if client == nil || err != nil { + t.Fatal("Unexpected failure, address") + } + if client.URL.String() != conf["address"] { + t.Fatalf("Expected address \"%s\", got \"%s\"", conf["address"], client.URL.String()) + } + if client.StoreMethod != "POST" { + t.Fatalf("Expected store_method \"%s\", got \"%s\"", "POST", client.StoreMethod) + } + if client.LockURL != nil || client.LockMethod != "LOCK" { + t.Fatal("Unexpected lock_address or lock_method") + } + if client.UnlockURL != nil || client.UnlockMethod != "UNLOCK" { + t.Fatal("Unexpected unlock_address or unlock_method") + } + if client.Username != "" || client.Password != "" { + t.Fatal("Unexpected username or password") + } + + // custom + conf = map[string]string{ + "address": "http://127.0.0.1:8888/foo", + "store_method": "BLAH", + "lock_address": "http://127.0.0.1:8888/bar", + "lock_method": "BLIP", + "unlock_address": "http://127.0.0.1:8888/baz", + "unlock_method": "BLOOP", + "username": "user", + "password": "pass", + } + c, err = httpFactory(conf) + client, _ = c.(*HTTPClient) + if client == nil || err != nil { + t.Fatal("Unexpected failure, store_method") + } + if client.StoreMethod != "BLAH" { + t.Fatalf("Expected store_method \"%s\", got \"%s\"", "BLAH", client.StoreMethod) + } + if client.LockURL.String() != conf["lock_address"] || client.LockMethod != "BLIP" { + t.Fatalf("Unexpected lock_address \"%s\" vs \"%s\" or lock_method \"%s\" vs \"%s\"", client.LockURL.String(), + conf["lock_address"], client.LockMethod, conf["lock_method"]) + } + if client.UnlockURL.String() != conf["unlock_address"] || client.UnlockMethod != "BLOOP" { + t.Fatalf("Unexpected unlock_address \"%s\" vs \"%s\" or unlock_method \"%s\" vs \"%s\"", client.UnlockURL.String(), + conf["unlock_address"], client.UnlockMethod, conf["unlock_method"]) + } + if client.Username != "user" || client.Password != "pass" { + t.Fatalf("Unexpected username \"%s\" vs \"%s\" or password \"%s\" vs \"%s\"", client.Username, conf["username"], + client.Password, conf["password"]) + } } type testHTTPHandler struct { @@ -58,7 +134,7 @@ func (h *testHTTPHandler) Handle(w http.ResponseWriter, r *http.Request) { switch r.Method { case "GET": w.Write(h.Data) - case "POST": + case "POST", "PUT": buf := new(bytes.Buffer) if _, err := io.Copy(buf, r.Body); err != nil { w.WriteHeader(500) @@ -67,7 +143,7 @@ func (h *testHTTPHandler) Handle(w http.ResponseWriter, r *http.Request) { h.Data = buf.Bytes() case "LOCK": if h.Locked { - w.WriteHeader(409) + w.WriteHeader(423) } else { h.Locked = true }