From c6d955f4c141841aad4ac88459702f5bb338b0b2 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Wed, 21 Jan 2015 12:22:16 -0800 Subject: [PATCH] client: drive Create with setAction; drop TTL --- client/keys.go | 51 ++++++++------ client/keys_test.go | 130 +++++++++++++++++++++++++++++------- discovery/discovery.go | 2 +- discovery/discovery_test.go | 8 +-- integration/cluster_test.go | 6 +- 5 files changed, 145 insertions(+), 52 deletions(-) diff --git a/client/keys.go b/client/keys.go index f8bff9dd5..7537f3340 100644 --- a/client/keys.go +++ b/client/keys.go @@ -23,11 +23,18 @@ import ( "path" "strconv" "strings" - "time" "github.com/coreos/etcd/Godeps/_workspace/src/golang.org/x/net/context" ) +type PrevExistType string + +const ( + PrevIgnore = PrevExistType("") + PrevExist = PrevExistType("true") + PrevNoExist = PrevExistType("false") +) + var ( DefaultV2KeysPrefix = "/v2/keys" ) @@ -54,13 +61,17 @@ func NewDiscoveryKeysAPI(c HTTPClient) KeysAPI { } type KeysAPI interface { - Create(ctx context.Context, key, value string, ttl time.Duration) (*Response, error) + Create(ctx context.Context, key, value string) (*Response, error) Get(ctx context.Context, key string) (*Response, error) Watch(key string, idx uint64) Watcher RecursiveWatch(key string, idx uint64) Watcher } +type SetOptions struct { + PrevExist PrevExistType +} + type Watcher interface { Next(context.Context) (*Response, error) } @@ -90,18 +101,17 @@ type httpKeysAPI struct { prefix string } -func (k *httpKeysAPI) Create(ctx context.Context, key, val string, ttl time.Duration) (*Response, error) { - create := &createAction{ +func (k *httpKeysAPI) Create(ctx context.Context, key, val string) (*Response, error) { + act := &setAction{ Prefix: k.prefix, Key: key, Value: val, - } - if ttl >= 0 { - uttl := uint64(ttl.Seconds()) - create.TTL = &uttl + Options: SetOptions{ + PrevExist: PrevNoExist, + }, } - resp, body, err := k.client.Do(ctx, create) + resp, body, err := k.client.Do(ctx, act) if err != nil { return nil, err } @@ -215,25 +225,24 @@ func (w *waitAction) HTTPRequest(ep url.URL) *http.Request { return req } -type createAction struct { - Prefix string - Key string - Value string - TTL *uint64 +type setAction struct { + Prefix string + Key string + Value string + Options SetOptions } -func (c *createAction) HTTPRequest(ep url.URL) *http.Request { - u := v2KeysURL(ep, c.Prefix, c.Key) +func (a *setAction) HTTPRequest(ep url.URL) *http.Request { + u := v2KeysURL(ep, a.Prefix, a.Key) params := u.Query() - params.Set("prevExist", "false") + if a.Options.PrevExist != PrevIgnore { + params.Set("prevExist", string(a.Options.PrevExist)) + } u.RawQuery = params.Encode() form := url.Values{} - form.Add("value", c.Value) - if c.TTL != nil { - form.Add("ttl", strconv.FormatUint(*c.TTL, 10)) - } + form.Add("value", a.Value) body := strings.NewReader(form.Encode()) req, _ := http.NewRequest("PUT", u.String(), body) diff --git a/client/keys_test.go b/client/keys_test.go index ea0eaedd6..ceb75c872 100644 --- a/client/keys_test.go +++ b/client/keys_test.go @@ -176,45 +176,129 @@ func TestWaitAction(t *testing.T) { } } -func TestCreateAction(t *testing.T) { - ep := url.URL{Scheme: "http", Host: "example.com/v2/keys"} - wantURL := &url.URL{ - Scheme: "http", - Host: "example.com", - Path: "/v2/keys/foo/bar", - RawQuery: "prevExist=false", - } +func TestSetAction(t *testing.T) { wantHeader := http.Header(map[string][]string{ "Content-Type": []string{"application/x-www-form-urlencoded"}, }) - ttl12 := uint64(12) tests := []struct { - value string - ttl *uint64 + act setAction + wantURL string wantBody string }{ + // default prefix { - value: "baz", + act: setAction{ + Prefix: DefaultV2KeysPrefix, + Key: "foo", + }, + wantURL: "http://example.com/v2/keys/foo", + wantBody: "value=", + }, + + // non-default prefix + { + act: setAction{ + Prefix: "/pfx", + Key: "foo", + }, + wantURL: "http://example.com/pfx/foo", + wantBody: "value=", + }, + + // no prefix + { + act: setAction{ + Key: "foo", + }, + wantURL: "http://example.com/foo", + wantBody: "value=", + }, + + // Key with path separators + { + act: setAction{ + Prefix: DefaultV2KeysPrefix, + Key: "foo/bar/baz", + }, + wantURL: "http://example.com/v2/keys/foo/bar/baz", + wantBody: "value=", + }, + + // Key with leading slash, Prefix with trailing slash + { + act: setAction{ + Prefix: "/foo/", + Key: "/bar", + }, + wantURL: "http://example.com/foo/bar", + wantBody: "value=", + }, + + // Key with trailing slash + { + act: setAction{ + Key: "/foo/", + }, + wantURL: "http://example.com/foo", + wantBody: "value=", + }, + + // Value is set + { + act: setAction{ + Key: "foo", + Value: "baz", + }, + wantURL: "http://example.com/foo", wantBody: "value=baz", }, + + // PrevExist set, but still ignored { - value: "baz", - ttl: &ttl12, - wantBody: "ttl=12&value=baz", + act: setAction{ + Key: "foo", + Options: SetOptions{ + PrevExist: PrevIgnore, + }, + }, + wantURL: "http://example.com/foo", + wantBody: "value=", + }, + + // PrevExist set to true + { + act: setAction{ + Key: "foo", + Options: SetOptions{ + PrevExist: PrevExist, + }, + }, + wantURL: "http://example.com/foo?prevExist=true", + wantBody: "value=", + }, + + // PrevExist set to false + { + act: setAction{ + Key: "foo", + Options: SetOptions{ + PrevExist: PrevNoExist, + }, + }, + wantURL: "http://example.com/foo?prevExist=false", + wantBody: "value=", }, } for i, tt := range tests { - f := createAction{ - Key: "/foo/bar", - Value: tt.value, - TTL: tt.ttl, - } - got := *f.HTTPRequest(ep) - - err := assertResponse(got, wantURL, wantHeader, []byte(tt.wantBody)) + u, err := url.Parse(tt.wantURL) if err != nil { + t.Errorf("#%d: unable to use wantURL fixture: %v", i, err) + } + + got := tt.act.HTTPRequest(url.URL{Scheme: "http", Host: "example.com"}) + if err := assertResponse(*got, u, wantHeader, []byte(tt.wantBody)); err != nil { t.Errorf("#%d: %v", i, err) } } diff --git a/discovery/discovery.go b/discovery/discovery.go index 7bd728e79..fb2fc0ed4 100644 --- a/discovery/discovery.go +++ b/discovery/discovery.go @@ -176,7 +176,7 @@ func (d *discovery) getCluster() (string, error) { func (d *discovery) createSelf(contents string) error { ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) - resp, err := d.c.Create(ctx, d.selfKey(), contents, -1) + resp, err := d.c.Create(ctx, d.selfKey(), contents) cancel() if err != nil { if err == client.ErrKeyExists { diff --git a/discovery/discovery_test.go b/discovery/discovery_test.go index 92e870e2a..01a916fca 100644 --- a/discovery/discovery_test.go +++ b/discovery/discovery_test.go @@ -411,7 +411,7 @@ type clientWithResp struct { w client.Watcher } -func (c *clientWithResp) Create(ctx context.Context, key string, value string, ttl time.Duration) (*client.Response, error) { +func (c *clientWithResp) Create(ctx context.Context, key string, value string) (*client.Response, error) { if len(c.rs) == 0 { return &client.Response{}, nil } @@ -442,7 +442,7 @@ type clientWithErr struct { w client.Watcher } -func (c *clientWithErr) Create(ctx context.Context, key string, value string, ttl time.Duration) (*client.Response, error) { +func (c *clientWithErr) Create(ctx context.Context, key string, value string) (*client.Response, error) { return &client.Response{}, c.err } @@ -486,12 +486,12 @@ type clientWithRetry struct { failTimes int } -func (c *clientWithRetry) Create(ctx context.Context, key string, value string, ttl time.Duration) (*client.Response, error) { +func (c *clientWithRetry) Create(ctx context.Context, key string, value string) (*client.Response, error) { if c.failCount < c.failTimes { c.failCount++ return nil, client.ErrTimeout } - return c.clientWithResp.Create(ctx, key, value, ttl) + return c.clientWithResp.Create(ctx, key, value) } func (c *clientWithRetry) Get(ctx context.Context, key string) (*client.Response, error) { diff --git a/integration/cluster_test.go b/integration/cluster_test.go index b7d7c7715..c2166dff6 100644 --- a/integration/cluster_test.go +++ b/integration/cluster_test.go @@ -85,7 +85,7 @@ func testClusterUsingDiscovery(t *testing.T, size int) { dcc := mustNewHTTPClient(t, dc.URLs()) dkapi := client.NewKeysAPI(dcc) ctx, cancel := context.WithTimeout(context.Background(), requestTimeout) - if _, err := dkapi.Create(ctx, "/_config/size", fmt.Sprintf("%d", size), -1); err != nil { + if _, err := dkapi.Create(ctx, "/_config/size", fmt.Sprintf("%d", size)); err != nil { t.Fatal(err) } cancel() @@ -135,7 +135,7 @@ func TestForceNewCluster(t *testing.T) { cc := mustNewHTTPClient(t, []string{c.Members[0].URL()}) kapi := client.NewKeysAPI(cc) ctx, cancel := context.WithTimeout(context.Background(), requestTimeout) - resp, err := kapi.Create(ctx, "/foo", "bar", -1) + resp, err := kapi.Create(ctx, "/foo", "bar") if err != nil { t.Fatalf("unexpected create error: %v", err) } @@ -178,7 +178,7 @@ func clusterMustProgress(t *testing.T, membs []*member) { kapi := client.NewKeysAPI(cc) ctx, cancel := context.WithTimeout(context.Background(), requestTimeout) key := fmt.Sprintf("foo%d", rand.Int()) - resp, err := kapi.Create(ctx, "/"+key, "bar", -1) + resp, err := kapi.Create(ctx, "/"+key, "bar") if err != nil { t.Fatalf("create on %s error: %v", membs[0].URL(), err) }