Mirroristas/etcd
2
0
Fork 0
mirror of https://github.com/etcd-io/etcd.git synced 2024-09-27 06:25:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #6308 from gyuho/manual2

Browse Source 
client: do not send previous node data (optional)
This commit is contained in:
Gyu-Ho Lee 2016-08-30 13:33:22 -07:00 committed by GitHub
commit 48941cea95
7 changed files with 241 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
Documentation/v2/api.md
View File

@ -559,6 +559,25 @@ Let's create a key-value pair first: `foo=one`.
curl http://127.0.0.1:2379/v2/keys/foo -XPUT -d value=one
```
```json
{
"action":"set",
"node":{
"key":"/foo",
"value":"one",
"modifiedIndex":4,
"createdIndex":4
}
}
```
Specifying `noValueOnSuccess` option skips returning the node as value.
```sh
curl http://127.0.0.1:2379/v2/keys/foo?noValueOnSuccess=true -XPUT -d value=one
# {"action":"set"}
```
Now let's try some invalid `CompareAndSwap` commands.
Trying to set this existing key with `prevExist=false` fails as expected:

3
client/keys.generated.go
View File

@ -8,10 +8,11 @@ package client
import (
"errors"
"fmt"
codec1978 "github.com/ugorji/go/codec"
"reflect"
"runtime"
time "time"
codec1978 "github.com/ugorji/go/codec"
)
const (

27
client/keys.go
View File

@ -191,6 +191,10 @@ type SetOptions struct {
// Dir specifies whether or not this Node should be created as a directory.
Dir bool
// NoValueOnSuccess specifies whether the response contains the current value of the Node.
// If set, the response will only contain the current value when the request fails.
NoValueOnSuccess bool
}
type GetOptions struct {
@ -335,6 +339,7 @@ func (k *httpKeysAPI) Set(ctx context.Context, key, val string, opts *SetOptions
act.TTL = opts.TTL
act.Refresh = opts.Refresh
act.Dir = opts.Dir
act.NoValueOnSuccess = opts.NoValueOnSuccess
}
doCtx := ctx
@ -523,15 +528,16 @@ func (w *waitAction) HTTPRequest(ep url.URL) *http.Request {
}
type setAction struct {
Prefix string
Key string
Value string
PrevValue string
PrevIndex uint64
PrevExist PrevExistType
TTL time.Duration
Refresh bool
Dir bool
Prefix string
Key string
Value string
PrevValue string
PrevIndex uint64
PrevExist PrevExistType
TTL time.Duration
Refresh bool
Dir bool
NoValueOnSuccess bool
}
func (a *setAction) HTTPRequest(ep url.URL) *http.Request {
@ -565,6 +571,9 @@ func (a *setAction) HTTPRequest(ep url.URL) *http.Request {
if a.Refresh {
form.Add("refresh", "true")
}
if a.NoValueOnSuccess {
params.Set("noValueOnSuccess", strconv.FormatBool(a.NoValueOnSuccess))
}
u.RawQuery = params.Encode()
body := strings.NewReader(form.Encode())

9
client/keys_test.go
View File

@ -407,6 +407,15 @@ func TestSetAction(t *testing.T) {
wantURL: "http://example.com/foo?dir=true",
wantBody: "",
},
// NoValueOnSuccess is set
{
act: setAction{
Key: "foo",
NoValueOnSuccess: true,
},
wantURL: "http://example.com/foo?noValueOnSuccess=true",
wantBody: "value=",
},
}
for i, tt := range tests {

58
etcdserver/api/v2http/client.go
View File

@ -153,7 +153,7 @@ func (h *keysHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
defer cancel()
clock := clockwork.NewRealClock()
startTime := clock.Now()
rr, err := parseKeyRequest(r, clock)
rr, noValueOnSuccess, err := parseKeyRequest(r, clock)
if err != nil {
writeKeyError(w, err)
return
@ -175,7 +175,7 @@ func (h *keysHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}
switch {
case resp.Event != nil:
if err := writeKeyEvent(w, resp.Event, h.timer); err != nil {
if err := writeKeyEvent(w, resp.Event, noValueOnSuccess, h.timer); err != nil {
// Should never be reached
plog.Errorf("error writing event (%v)", err)
}
@ -449,19 +449,20 @@ func logHandleFunc(w http.ResponseWriter, r *http.Request) {
// parseKeyRequest converts a received http.Request on keysPrefix to
// a server Request, performing validation of supplied fields as appropriate.
// If any validation fails, an empty Request and non-nil error is returned.
func parseKeyRequest(r *http.Request, clock clockwork.Clock) (etcdserverpb.Request, error) {
func parseKeyRequest(r *http.Request, clock clockwork.Clock) (etcdserverpb.Request, bool, error) {
noValueOnSuccess := false
emptyReq := etcdserverpb.Request{}
err := r.ParseForm()
if err != nil {
return emptyReq, etcdErr.NewRequestError(
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodeInvalidForm,
err.Error(),
)
}
if !strings.HasPrefix(r.URL.Path, keysPrefix) {
return emptyReq, etcdErr.NewRequestError(
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodeInvalidForm,
"incorrect key prefix",
)
@ -470,13 +471,13 @@ func parseKeyRequest(r *http.Request, clock clockwork.Clock) (etcdserverpb.Reque
var pIdx, wIdx uint64
if pIdx, err = getUint64(r.Form, "prevIndex"); err != nil {
return emptyReq, etcdErr.NewRequestError(
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodeIndexNaN,
`invalid value for "prevIndex"`,
)
}
if wIdx, err = getUint64(r.Form, "waitIndex"); err != nil {
return emptyReq, etcdErr.NewRequestError(
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodeIndexNaN,
`invalid value for "waitIndex"`,
)
@ -484,45 +485,45 @@ func parseKeyRequest(r *http.Request, clock clockwork.Clock) (etcdserverpb.Reque
var rec, sort, wait, dir, quorum, stream bool
if rec, err = getBool(r.Form, "recursive"); err != nil {
return emptyReq, etcdErr.NewRequestError(
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodeInvalidField,
`invalid value for "recursive"`,
)
}
if sort, err = getBool(r.Form, "sorted"); err != nil {
return emptyReq, etcdErr.NewRequestError(
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodeInvalidField,
`invalid value for "sorted"`,
)
}
if wait, err = getBool(r.Form, "wait"); err != nil {
return emptyReq, etcdErr.NewRequestError(
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodeInvalidField,
`invalid value for "wait"`,
)
}
// TODO(jonboulle): define what parameters dir is/isn't compatible with?
if dir, err = getBool(r.Form, "dir"); err != nil {
return emptyReq, etcdErr.NewRequestError(
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodeInvalidField,
`invalid value for "dir"`,
)
}
if quorum, err = getBool(r.Form, "quorum"); err != nil {
return emptyReq, etcdErr.NewRequestError(
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodeInvalidField,
`invalid value for "quorum"`,
)
}
if stream, err = getBool(r.Form, "stream"); err != nil {
return emptyReq, etcdErr.NewRequestError(
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodeInvalidField,
`invalid value for "stream"`,
)
}
if wait && r.Method != "GET" {
return emptyReq, etcdErr.NewRequestError(
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodeInvalidField,
`"wait" can only be used with GET requests`,
)
@ -530,19 +531,26 @@ func parseKeyRequest(r *http.Request, clock clockwork.Clock) (etcdserverpb.Reque
pV := r.FormValue("prevValue")
if _, ok := r.Form["prevValue"]; ok && pV == "" {
return emptyReq, etcdErr.NewRequestError(
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodePrevValueRequired,
`"prevValue" cannot be empty`,
)
}
if noValueOnSuccess, err = getBool(r.Form, "noValueOnSuccess"); err != nil {
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodeInvalidField,
`invalid value for "noValueOnSuccess"`,
)
}
// TTL is nullable, so leave it null if not specified
// or an empty string
var ttl *uint64
if len(r.FormValue("ttl")) > 0 {
i, err := getUint64(r.Form, "ttl")
if err != nil {
return emptyReq, etcdErr.NewRequestError(
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodeTTLNaN,
`invalid value for "ttl"`,
)
@ -555,7 +563,7 @@ func parseKeyRequest(r *http.Request, clock clockwork.Clock) (etcdserverpb.Reque
if _, ok := r.Form["prevExist"]; ok {
bv, err := getBool(r.Form, "prevExist")
if err != nil {
return emptyReq, etcdErr.NewRequestError(
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodeInvalidField,
"invalid value for prevExist",
)
@ -568,7 +576,7 @@ func parseKeyRequest(r *http.Request, clock clockwork.Clock) (etcdserverpb.Reque
if _, ok := r.Form["refresh"]; ok {
bv, err := getBool(r.Form, "refresh")
if err != nil {
return emptyReq, etcdErr.NewRequestError(
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodeInvalidField,
"invalid value for refresh",
)
@ -577,13 +585,13 @@ func parseKeyRequest(r *http.Request, clock clockwork.Clock) (etcdserverpb.Reque
if refresh != nil && *refresh {
val := r.FormValue("value")
if _, ok := r.Form["value"]; ok && val != "" {
return emptyReq, etcdErr.NewRequestError(
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodeRefreshValue,
`A value was provided on a refresh`,
)
}
if ttl == nil {
return emptyReq, etcdErr.NewRequestError(
return emptyReq, false, etcdErr.NewRequestError(
etcdErr.EcodeRefreshTTLRequired,
`No TTL value set`,
)
@ -621,13 +629,13 @@ func parseKeyRequest(r *http.Request, clock clockwork.Clock) (etcdserverpb.Reque
rr.Expiration = clock.Now().Add(expr).UnixNano()
}
return rr, nil
return rr, noValueOnSuccess, nil
}
// writeKeyEvent trims the prefix of key path in a single Event under
// StoreKeysPrefix, serializes it and writes the resulting JSON to the given
// ResponseWriter, along with the appropriate headers.
func writeKeyEvent(w http.ResponseWriter, ev *store.Event, rt etcdserver.RaftTimer) error {
func writeKeyEvent(w http.ResponseWriter, ev *store.Event, noValueOnSuccess bool, rt etcdserver.RaftTimer) error {
if ev == nil {
return errors.New("cannot write empty Event!")
}
@ -641,6 +649,12 @@ func writeKeyEvent(w http.ResponseWriter, ev *store.Event, rt etcdserver.RaftTim
}
ev = trimEventPrefix(ev, etcdserver.StoreKeysPrefix)
if noValueOnSuccess &&
(ev.Action == store.Set || ev.Action == store.CompareAndSwap ||
ev.Action == store.Create || ev.Action == store.Update) {
ev.Node = nil
ev.PrevNode = nil
}
return json.NewEncoder(w).Encode(ev)
}

138
etcdserver/api/v2http/client_test.go
View File

@ -196,7 +196,7 @@ func TestBadRefreshRequest(t *testing.T) {
},
}
for i, tt := range tests {
got, err := parseKeyRequest(tt.in, clockwork.NewFakeClock())
got, _, err := parseKeyRequest(tt.in, clockwork.NewFakeClock())
if err == nil {
t.Errorf("#%d: unexpected nil error!", i)
continue
@ -360,7 +360,7 @@ func TestBadParseRequest(t *testing.T) {
},
}
for i, tt := range tests {
got, err := parseKeyRequest(tt.in, clockwork.NewFakeClock())
got, _, err := parseKeyRequest(tt.in, clockwork.NewFakeClock())
if err == nil {
t.Errorf("#%d: unexpected nil error!", i)
continue
@ -384,8 +384,9 @@ func TestGoodParseRequest(t *testing.T) {
fc := clockwork.NewFakeClock()
fc.Advance(1111)
tests := []struct {
in *http.Request
w etcdserverpb.Request
in *http.Request
w etcdserverpb.Request
noValue bool
}{
{
// good prefix, all other values default
@ -394,6 +395,7 @@ func TestGoodParseRequest(t *testing.T) {
Method: "GET",
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
},
false,
},
{
// value specified
@ -407,6 +409,7 @@ func TestGoodParseRequest(t *testing.T) {
Val: "some_value",
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
},
false,
},
{
// prevIndex specified
@ -420,6 +423,7 @@ func TestGoodParseRequest(t *testing.T) {
PrevIndex: 98765,
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
},
false,
},
{
// recursive specified
@ -433,6 +437,7 @@ func TestGoodParseRequest(t *testing.T) {
Recursive: true,
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
},
false,
},
{
// sorted specified
@ -446,6 +451,7 @@ func TestGoodParseRequest(t *testing.T) {
Sorted: true,
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
},
false,
},
{
// quorum specified
@ -459,6 +465,7 @@ func TestGoodParseRequest(t *testing.T) {
Quorum: true,
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
},
false,
},
{
// wait specified
@ -468,6 +475,7 @@ func TestGoodParseRequest(t *testing.T) {
Wait: true,
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
},
false,
},
{
// empty TTL specified
@ -477,6 +485,7 @@ func TestGoodParseRequest(t *testing.T) {
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
Expiration: 0,
},
false,
},
{
// non-empty TTL specified
@ -486,6 +495,7 @@ func TestGoodParseRequest(t *testing.T) {
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
Expiration: fc.Now().Add(5678 * time.Second).UnixNano(),
},
false,
},
{
// zero TTL specified
@ -495,6 +505,7 @@ func TestGoodParseRequest(t *testing.T) {
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
Expiration: fc.Now().UnixNano(),
},
false,
},
{
// dir specified
@ -504,6 +515,7 @@ func TestGoodParseRequest(t *testing.T) {
Dir: true,
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
},
false,
},
{
// dir specified negatively
@ -513,6 +525,7 @@ func TestGoodParseRequest(t *testing.T) {
Dir: false,
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
},
false,
},
{
// prevExist should be non-null if specified
@ -526,6 +539,7 @@ func TestGoodParseRequest(t *testing.T) {
PrevExist: boolp(true),
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
},
false,
},
{
// prevExist should be non-null if specified
@ -539,6 +553,7 @@ func TestGoodParseRequest(t *testing.T) {
PrevExist: boolp(false),
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
},
false,
},
// mix various fields
{
@ -558,6 +573,7 @@ func TestGoodParseRequest(t *testing.T) {
Val: "some value",
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
},
false,
},
// query parameters should be used if given
{
@ -571,6 +587,7 @@ func TestGoodParseRequest(t *testing.T) {
PrevValue: "woof",
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
},
false,
},
// but form values should take precedence over query parameters
{
@ -586,14 +603,33 @@ func TestGoodParseRequest(t *testing.T) {
PrevValue: "miaow",
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
},
false,
},
{
// noValueOnSuccess specified
mustNewForm(
t,
"foo",
url.Values{"noValueOnSuccess": []string{"true"}},
),
etcdserverpb.Request{
Method: "PUT",
Path: path.Join(etcdserver.StoreKeysPrefix, "/foo"),
},
true,
},
}
for i, tt := range tests {
got, err := parseKeyRequest(tt.in, fc)
got, noValueOnSuccess, err := parseKeyRequest(tt.in, fc)
if err != nil {
t.Errorf("#%d: err = %v, want %v", i, err, nil)
}
if noValueOnSuccess != tt.noValue {
t.Errorf("#%d: noValue=%t, want %t", i, noValueOnSuccess, tt.noValue)
}
if !reflect.DeepEqual(got, tt.w) {
t.Errorf("#%d: request=%#v, want %#v", i, got, tt.w)
}
@ -1112,7 +1148,7 @@ func TestServeMembersFail(t *testing.T) {
func TestWriteEvent(t *testing.T) {
// nil event should not panic
rec := httptest.NewRecorder()
writeKeyEvent(rec, nil, dummyRaftTimer{})
writeKeyEvent(rec, nil, false, dummyRaftTimer{})
h := rec.Header()
if len(h) > 0 {
t.Fatalf("unexpected non-empty headers: %#v", h)
@ -1123,8 +1159,9 @@ func TestWriteEvent(t *testing.T) {
}
tests := []struct {
ev *store.Event
idx string
ev *store.Event
noValue bool
idx string
// TODO(jonboulle): check body as well as just status code
code int
err error
@ -1136,6 +1173,7 @@ func TestWriteEvent(t *testing.T) {
Node: &store.NodeExtern{},
PrevNode: &store.NodeExtern{},
},
false,
"0",
http.StatusOK,
nil,
@ -1147,6 +1185,7 @@ func TestWriteEvent(t *testing.T) {
Node: &store.NodeExtern{},
PrevNode: &store.NodeExtern{},
},
false,
"0",
http.StatusCreated,
nil,
@ -1155,7 +1194,7 @@ func TestWriteEvent(t *testing.T) {
for i, tt := range tests {
rw := httptest.NewRecorder()
writeKeyEvent(rw, tt.ev, dummyRaftTimer{})
writeKeyEvent(rw, tt.ev, tt.noValue, dummyRaftTimer{})
if gct := rw.Header().Get("Content-Type"); gct != "application/json" {
t.Errorf("case %d: bad Content-Type: got %q, want application/json", i, gct)
}
@ -1550,45 +1589,76 @@ func TestServeKeysGood(t *testing.T) {
}
func TestServeKeysEvent(t *testing.T) {
req := mustNewRequest(t, "foo")
server := &resServer{
etcdserver.Response{
Event: &store.Event{
tests := []struct {
req *http.Request
rsp etcdserver.Response
wcode int
event *store.Event
}{
{
mustNewRequest(t, "foo"),
etcdserver.Response{
Event: &store.Event{
Action: store.Get,
Node: &store.NodeExtern{},
},
},
http.StatusOK,
&store.Event{
Action: store.Get,
Node: &store.NodeExtern{},
},
},
{
mustNewForm(
t,
"foo",
url.Values{"noValueOnSuccess": []string{"true"}},
),
etcdserver.Response{
Event: &store.Event{
Action: store.CompareAndSwap,
Node: &store.NodeExtern{},
},
},
http.StatusOK,
&store.Event{
Action: store.CompareAndSwap,
Node: nil,
},
},
}
server := &resServer{}
h := &keysHandler{
timeout: time.Hour,
server: server,
cluster: &fakeCluster{id: 1},
timer: &dummyRaftTimer{},
}
rw := httptest.NewRecorder()
h.ServeHTTP(rw, req)
for _, tt := range tests {
server.res = tt.rsp
rw := httptest.NewRecorder()
h.ServeHTTP(rw, tt.req)
wcode := http.StatusOK
wbody := mustMarshalEvent(
t,
&store.Event{
Action: store.Get,
Node: &store.NodeExtern{},
},
)
wbody := mustMarshalEvent(
t,
tt.event,
)
if rw.Code != wcode {
t.Errorf("got code=%d, want %d", rw.Code, wcode)
}
gcid := rw.Header().Get("X-Etcd-Cluster-ID")
wcid := h.cluster.ID().String()
if gcid != wcid {
t.Errorf("cid = %s, want %s", gcid, wcid)
}
g := rw.Body.String()
if g != wbody {
t.Errorf("got body=%#v, want %#v", g, wbody)
if rw.Code != tt.wcode {
t.Errorf("got code=%d, want %d", rw.Code, tt.wcode)
}
gcid := rw.Header().Get("X-Etcd-Cluster-ID")
wcid := h.cluster.ID().String()
if gcid != wcid {
t.Errorf("cid = %s, want %s", gcid, wcid)
}
g := rw.Body.String()
if g != wbody {
t.Errorf("got body=%#v, want %#v", g, wbody)
}
}
}

53
integration/v2_http_kv_test.go
View File

@ -45,6 +45,9 @@ func TestV2Set(t *testing.T) {
tc := NewTestClient()
v := url.Values{}
v.Set("value", "bar")
vAndNoValue := url.Values{}
vAndNoValue.Set("value", "bar")
vAndNoValue.Set("noValueOnSuccess", "true")
tests := []struct {
relativeURL string
@ -70,6 +73,12 @@ func TestV2Set(t *testing.T) {
http.StatusCreated,
`{"action":"set","node":{"key":"/fooempty","value":"","modifiedIndex":6,"createdIndex":6}}`,
},
{
"/v2/keys/foo/novalue",
vAndNoValue,
http.StatusCreated,
`{"action":"set"}`,
},
}
for i, tt := range tests {
@ -183,6 +192,31 @@ func TestV2CreateUpdate(t *testing.T) {
"cause": "/nonexist",
},
},
// create with no value on success
{
"/v2/keys/create/novalue",
url.Values(map[string][]string{"value": {"XXX"}, "prevExist": {"false"}, "noValueOnSuccess": {"true"}}),
http.StatusCreated,
map[string]interface{}{},
},
// update with no value on success
{
"/v2/keys/create/novalue",
url.Values(map[string][]string{"value": {"XXX"}, "prevExist": {"true"}, "noValueOnSuccess": {"true"}}),
http.StatusOK,
map[string]interface{}{},
},
// created key failed with no value on success
{
"/v2/keys/create/foo",
url.Values(map[string][]string{"value": {"XXX"}, "prevExist": {"false"}, "noValueOnSuccess": {"true"}}),
http.StatusPreconditionFailed,
map[string]interface{}{
"errorCode": float64(105),
"message": "Key already exists",
"cause": "/create/foo",
},
},
}
for i, tt := range tests {
@ -312,6 +346,25 @@ func TestV2CAS(t *testing.T) {
"cause": "[bad_value != ZZZ]",
},
},
{
"/v2/keys/cas/foo",
url.Values(map[string][]string{"value": {"YYY"}, "prevIndex": {"6"}, "noValueOnSuccess": {"true"}}),
http.StatusOK,
map[string]interface{}{
"action": "compareAndSwap",
},
},
{
"/v2/keys/cas/foo",
url.Values(map[string][]string{"value": {"YYY"}, "prevIndex": {"10"}, "noValueOnSuccess": {"true"}}),
http.StatusPreconditionFailed,
map[string]interface{}{
"errorCode": float64(101),
"message": "Compare failed",
"cause": "[10 != 7]",
"index": float64(7),
},
},
}
for i, tt := range tests {