From dac5b0b7f8249630fde6b4b71e79e7f0113733e8 Mon Sep 17 00:00:00 2001 From: Fedor Korotkiy Date: Sat, 7 Mar 2020 22:14:49 +0300 Subject: [PATCH] Add retryupdate task --- go.mod | 3 + go.sum | 8 + retryupdate/README.md | 30 ++++ retryupdate/kvapi/api.go | 36 +++++ retryupdate/kvapi/errors.go | 56 +++++++ retryupdate/mock_test.go | 64 ++++++++ retryupdate/update.go | 9 ++ retryupdate/update_test.go | 303 ++++++++++++++++++++++++++++++++++++ 8 files changed, 509 insertions(+) create mode 100644 retryupdate/README.md create mode 100644 retryupdate/kvapi/api.go create mode 100644 retryupdate/kvapi/errors.go create mode 100644 retryupdate/mock_test.go create mode 100644 retryupdate/update.go create mode 100644 retryupdate/update_test.go diff --git a/go.mod b/go.mod index 51a03cc..6b09e7a 100644 --- a/go.mod +++ b/go.mod @@ -4,11 +4,14 @@ go 1.13 require ( github.com/go-resty/resty/v2 v2.1.0 + github.com/gofrs/uuid v3.2.0+incompatible + github.com/golang/mock v1.4.1 github.com/spf13/cobra v0.0.5 github.com/stretchr/testify v1.4.0 go.uber.org/goleak v1.0.0 golang.org/x/net v0.0.0-20190628185345-da137c7871d7 golang.org/x/perf v0.0.0-20191209155426-36b577b0eb03 golang.org/x/tools v0.0.0-20200125223703-d33eef8e6825 + golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898 gopkg.in/yaml.v2 v2.2.8 ) diff --git a/go.sum b/go.sum index 8127b7f..c220d13 100644 --- a/go.sum +++ b/go.sum @@ -15,6 +15,10 @@ github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMo github.com/go-resty/resty/v2 v2.1.0 h1:Z6IefCpUMfnvItVJaJXWv/pMiiD11So35QgwEELsldE= github.com/go-resty/resty/v2 v2.1.0/go.mod h1:dZGr0i9PLlaaTD4H/hoZIDjQ+r6xq8mgbRzHZf7f2J8= github.com/go-sql-driver/mysql v1.4.1/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w= +github.com/gofrs/uuid v3.2.0+incompatible h1:y12jRkkFxsd7GpqdSZ+/KCs/fJbqpEXSGd4+jfEaewE= +github.com/gofrs/uuid v3.2.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM= +github.com/golang/mock v1.4.1 h1:ocYkMQY5RrXTYgXl7ICpV0IXwlEQGwKIsery4gyXa1U= +github.com/golang/mock v1.4.1/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/gonum/blas v0.0.0-20181208220705-f22b278b28ac/go.mod h1:P32wAyui1PQ58Oce/KYkOqQv8cVw1zAapXOl+dRFGbc= github.com/gonum/floats v0.0.0-20181209220543-c233463c7e82/go.mod h1:PxC8OnwL11+aosOB5+iEPoV3picfs8tUpkVd0pDo+Kg= @@ -77,8 +81,10 @@ golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sys v0.0.0-20181205085412-a5c9d58dba9a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= +golang.org/x/tools v0.0.0-20190425150028-36563e24a262/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= golang.org/x/tools v0.0.0-20191108193012-7d206e10da11/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200125223703-d33eef8e6825 h1:aNQeSIHKi0RWpKA5NO0CqyLjx6Beh5l0LLUEnndEjz0= golang.org/x/tools v0.0.0-20200125223703-d33eef8e6825/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= @@ -96,3 +102,5 @@ gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= +rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= diff --git a/retryupdate/README.md b/retryupdate/README.md new file mode 100644 index 0000000..4b8327b --- /dev/null +++ b/retryupdate/README.md @@ -0,0 +1,30 @@ +# retryupdate + +В этой задаче, вам нужно познакомиться с паттернами обработки ошибок в golang. + +Вам дан клиент вымышленного сервиса. Сервис хранит отображение `key -> (value, version)`, +где `key` и `value` - это обычные строки. А `version` - уникальный идентификатор, который сгенерировал +клиент. + +Вам доступны два метода - `Get` и `Set`. Поведение методов описано в пакете `kvapi`. + +Методы могут завершаться с различными ошибками. Ошибки описаны в том же пакете. + +Вам нужно реализовать функцию `UpdateValue`, которая принимает ключ и функцию +для "обновления" значения. Функция должна обновить значение по ключу, правильно обрабатывая +ошибки, возникающие в процессе работы. + + - Если ключ не нашёлся, нужно передать в `updateFn` значение `nil`. + - Если произошла ошибка доступа, то нужно сразу выходить. + - Если произошла ошибка внутри updateFn, то нужно сразу выходить. + - Если случился конфликт при записи, и значение изменилось с момента последнего чтения, + то нужно прочитать новое значение и попробовать заново. + - Любые другие ошибки из `api` нужно "ретраить" бесконечно. + +Функция должна минимизировать число запросов к API. + +Для генерации нового `uuid` на клиенте, используйте вызов `uuid.Must(uuid.NewV4())`. + +Большую цепочку `if`-ов удобнее писать через `switch`. + +Для выхода из вложенных циклов вам помогут [loop labels](https://golang.org/doc/effective_go.html#switch). diff --git a/retryupdate/kvapi/api.go b/retryupdate/kvapi/api.go new file mode 100644 index 0000000..93c8c1c --- /dev/null +++ b/retryupdate/kvapi/api.go @@ -0,0 +1,36 @@ +// +build !change + +package kvapi + +import "github.com/gofrs/uuid" + +type ( + Client interface { + // Get the value of key. + Get(req *GetRequest) (*GetResponse, error) + + // Set key to hold given value. + Set(rsp *SetRequest) (*SetResponse, error) + } + + GetRequest struct { + Key string + } + + GetResponse struct { + Value string + Version uuid.UUID + } + + SetRequest struct { + Key, Value string + + // OldVersion field is checked before updating value. + // + // When updating old value, OldVersion must specify uuid of currently stored value. + // When creating new value, OldVersion must hold zero value of UUID. + OldVersion, NewVersion uuid.UUID + } + + SetResponse struct{} +) diff --git a/retryupdate/kvapi/errors.go b/retryupdate/kvapi/errors.go new file mode 100644 index 0000000..a1a4928 --- /dev/null +++ b/retryupdate/kvapi/errors.go @@ -0,0 +1,56 @@ +// +build !change + +package kvapi + +import ( + "fmt" + + "github.com/gofrs/uuid" +) + +var ( + _ error = (*ApiError)(nil) + _ error = (*ConflictError)(nil) + _ error = (*AuthError)(nil) + _ error = (*NotFoundError)(nil) +) + +type ( + ApiError struct { + Method string + + Err error + } + + ConflictError struct { + ProvidedVersion, ExpectedVersion uuid.UUID + } + + AuthError struct { + Msg string + } + + NotFoundError struct { + Key string + } +) + +func (a *ApiError) Error() string { + return fmt.Sprintf("api: %q error: %v", a.Method, a.Err) +} + +func (a *ApiError) Unwrap() error { + return a.Err +} + +func (a *ConflictError) Error() string { + return fmt.Sprintf("api: conflict: expected_version=%d, provided_version=%d", a.ExpectedVersion, a.ProvidedVersion) +} + +func (a *AuthError) Error() string { + return fmt.Sprintf("api: auth: %s", a.Msg) +} + +func (a *NotFoundError) Error() string { + return fmt.Sprintf("api: key %q is not found", a.Key) +} diff --git a/retryupdate/mock_test.go b/retryupdate/mock_test.go new file mode 100644 index 0000000..ef9b119 --- /dev/null +++ b/retryupdate/mock_test.go @@ -0,0 +1,64 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: gitlab.com/slon/shad-go/retryupdate/kvapi (interfaces: Client) + +// Package retryupdate_test is a generated GoMock package. +package retryupdate_test + +import ( + gomock "github.com/golang/mock/gomock" + kvapi "gitlab.com/slon/shad-go/retryupdate/kvapi" + reflect "reflect" +) + +// MockClient is a mock of Client interface +type MockClient struct { + ctrl *gomock.Controller + recorder *MockClientMockRecorder +} + +// MockClientMockRecorder is the mock recorder for MockClient +type MockClientMockRecorder struct { + mock *MockClient +} + +// NewMockClient creates a new mock instance +func NewMockClient(ctrl *gomock.Controller) *MockClient { + mock := &MockClient{ctrl: ctrl} + mock.recorder = &MockClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockClient) EXPECT() *MockClientMockRecorder { + return m.recorder +} + +// Get mocks base method +func (m *MockClient) Get(arg0 *kvapi.GetRequest) (*kvapi.GetResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Get", arg0) + ret0, _ := ret[0].(*kvapi.GetResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Get indicates an expected call of Get +func (mr *MockClientMockRecorder) Get(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockClient)(nil).Get), arg0) +} + +// Set mocks base method +func (m *MockClient) Set(arg0 *kvapi.SetRequest) (*kvapi.SetResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Set", arg0) + ret0, _ := ret[0].(*kvapi.SetResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Set indicates an expected call of Set +func (mr *MockClientMockRecorder) Set(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Set", reflect.TypeOf((*MockClient)(nil).Set), arg0) +} diff --git a/retryupdate/update.go b/retryupdate/update.go new file mode 100644 index 0000000..651b6cc --- /dev/null +++ b/retryupdate/update.go @@ -0,0 +1,9 @@ +// +build !solution + +package retryupdate + +import "gitlab.com/slon/shad-go/retryupdate/kvapi" + +func UpdateValue(c kvapi.Client, key string, updateFn func(oldValue *string) (newValue string, err error)) error { + panic("implement me") +} diff --git a/retryupdate/update_test.go b/retryupdate/update_test.go new file mode 100644 index 0000000..172c0c0 --- /dev/null +++ b/retryupdate/update_test.go @@ -0,0 +1,303 @@ +package retryupdate_test + +//go:generate mockgen -destination mock_test.go -package retryupdate_test gitlab.com/slon/shad-go/retryupdate/kvapi Client + +import ( + "errors" + "fmt" + "testing" + + "github.com/gofrs/uuid" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" + + "gitlab.com/slon/shad-go/retryupdate" + "gitlab.com/slon/shad-go/retryupdate/kvapi" +) + +var ( + K0 = "K0" + V0 = "V0" + V1 = "V1" + V2 = "V2" + V3 = "V3" + + UUID0 = uuid.Must(uuid.NewV4()) + UUID1 = uuid.Must(uuid.NewV4()) + UUID2 = uuid.Must(uuid.NewV4()) + + errUpdate = errors.New("update error") + + errGetAuth = &kvapi.ApiError{Method: "get", Err: &kvapi.AuthError{Msg: "token expired"}} + errSetAuth = &kvapi.ApiError{Method: "set", Err: &kvapi.AuthError{Msg: "token expired"}} + + errGetNoKey = &kvapi.ApiError{Method: "get", Err: &kvapi.NotFoundError{Key: K0}} + errSetNoKey = &kvapi.ApiError{Method: "set", Err: &kvapi.NotFoundError{Key: K0}} + + errGetTemporary = &kvapi.ApiError{Method: "get", Err: errors.New("unavailable")} + errSetTemporary = &kvapi.ApiError{Method: "set", Err: errors.New("unavailable")} +) + +type setMatcher struct { + kvapi.SetRequest + + save *uuid.UUID +} + +func (m setMatcher) Matches(x interface{}) bool { + if arg, ok := x.(*kvapi.SetRequest); ok { + if m.save != nil { + *m.save = arg.NewVersion + } + + return arg.Key == m.Key && arg.Value == m.Value && arg.OldVersion == m.OldVersion + } + + return false +} + +func (m setMatcher) String() string { + return fmt.Sprintf("%v", m.SetRequest) +} + +func SetRequest(k, v string, oldVersion uuid.UUID, saveUUID ...*uuid.UUID) gomock.Matcher { + m := setMatcher{ + SetRequest: kvapi.SetRequest{ + Key: k, + Value: v, + OldVersion: oldVersion, + }, + } + + if len(saveUUID) > 1 { + panic("error") + } + + if len(saveUUID) == 1 { + m.save = saveUUID[0] + } + + return m +} + +func updateFn(oldValue *string) (string, error) { + switch { + case oldValue == nil: + return V0, nil + case *oldValue == V0: + return V1, nil + case *oldValue == V1: + return V2, nil + case *oldValue == V2: + return V3, nil + default: + return "", errUpdate + } +} + +func TestSimpleUpdate(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + c := NewMockClient(ctrl) + gomock.InOrder( + c.EXPECT(). + Get(&kvapi.GetRequest{Key: K0}). + Return(&kvapi.GetResponse{Value: V0, Version: UUID0}, nil), + + c.EXPECT(). + Set(SetRequest(K0, V1, UUID0)). + Return(&kvapi.SetResponse{}, nil), + ) + + require.NoError(t, retryupdate.UpdateValue(c, K0, updateFn)) +} + +func TestUpdateFnError(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + c := NewMockClient(ctrl) + gomock.InOrder( + c.EXPECT(). + Get(&kvapi.GetRequest{Key: K0}). + Return(&kvapi.GetResponse{Value: V3, Version: UUID0}, nil), + ) + + require.Equal(t, errUpdate, retryupdate.UpdateValue(c, K0, updateFn)) +} +func TestCreateKey(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + c := NewMockClient(ctrl) + gomock.InOrder( + c.EXPECT(). + Get(&kvapi.GetRequest{Key: K0}). + Return(nil, errGetNoKey), + + c.EXPECT(). + Set(SetRequest(K0, V0, uuid.UUID{})). + Return(&kvapi.SetResponse{}, nil), + ) + + require.NoError(t, retryupdate.UpdateValue(c, K0, updateFn)) +} + +func TestKeyVanished(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + c := NewMockClient(ctrl) + gomock.InOrder( + c.EXPECT(). + Get(&kvapi.GetRequest{Key: K0}). + Return(&kvapi.GetResponse{Value: V2, Version: UUID0}, nil), + + c.EXPECT(). + Set(SetRequest(K0, V3, UUID0)). + Return(nil, errSetNoKey), + + c.EXPECT(). + Set(SetRequest(K0, V0, uuid.UUID{})). + Return(&kvapi.SetResponse{}, nil), + ) + + require.NoError(t, retryupdate.UpdateValue(c, K0, updateFn)) +} + +func TestFailOnAuthErrorInGet(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + c := NewMockClient(ctrl) + gomock.InOrder( + c.EXPECT(). + Get(&kvapi.GetRequest{Key: K0}). + Return(nil, errGetAuth), + ) + + require.Equal(t, errGetAuth, retryupdate.UpdateValue(c, K0, updateFn)) +} + +func TestFailOnAuthErrorInSet(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + c := NewMockClient(ctrl) + gomock.InOrder( + c.EXPECT(). + Get(&kvapi.GetRequest{Key: K0}). + Return(nil, errGetNoKey), + + c.EXPECT(). + Set(SetRequest(K0, V0, uuid.UUID{})). + Return(nil, errSetAuth), + ) + + require.Equal(t, errSetAuth, retryupdate.UpdateValue(c, K0, updateFn)) +} + +func TestRetryGetError(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + c := NewMockClient(ctrl) + gomock.InOrder( + c.EXPECT(). + Get(&kvapi.GetRequest{Key: K0}). + Return(nil, errGetTemporary), + + c.EXPECT(). + Get(&kvapi.GetRequest{Key: K0}). + Return(nil, errGetTemporary), + + c.EXPECT(). + Get(&kvapi.GetRequest{Key: K0}). + Return(&kvapi.GetResponse{Value: V0, Version: UUID0}, nil), + + c.EXPECT(). + Set(SetRequest(K0, V1, UUID0)). + Return(&kvapi.SetResponse{}, nil), + ) + + require.NoError(t, retryupdate.UpdateValue(c, K0, updateFn)) +} + +func TestRetrySetError(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + c := NewMockClient(ctrl) + gomock.InOrder( + c.EXPECT(). + Get(&kvapi.GetRequest{Key: K0}). + Return(&kvapi.GetResponse{Value: V0, Version: UUID0}, nil), + + c.EXPECT(). + Set(SetRequest(K0, V1, UUID0)). + Return(nil, errSetTemporary), + + c.EXPECT(). + Set(SetRequest(K0, V1, UUID0)). + Return(&kvapi.SetResponse{}, nil), + ) + + require.NoError(t, retryupdate.UpdateValue(c, K0, updateFn)) +} + +func TestRetrySetConflict(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + c := NewMockClient(ctrl) + gomock.InOrder( + c.EXPECT(). + Get(&kvapi.GetRequest{Key: K0}). + Return(&kvapi.GetResponse{Value: V0, Version: UUID0}, nil), + + c.EXPECT(). + Set(SetRequest(K0, V1, UUID0)). + Return(nil, errSetTemporary), + + c.EXPECT(). + Set(SetRequest(K0, V1, UUID0)). + Return(nil, &kvapi.ApiError{Method: "set", Err: &kvapi.ConflictError{ExpectedVersion: UUID1, ProvidedVersion: UUID0}}), + + c.EXPECT(). + Get(&kvapi.GetRequest{Key: K0}). + Return(&kvapi.GetResponse{Value: V2, Version: UUID1}, nil), + + c.EXPECT(). + Set(SetRequest(K0, V3, UUID1)). + Return(&kvapi.SetResponse{}, nil), + ) + + require.NoError(t, retryupdate.UpdateValue(c, K0, updateFn)) +} + +func TestRetrySetFalseConflict(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + conflictErr := &kvapi.ConflictError{ProvidedVersion: UUID0} + + c := NewMockClient(ctrl) + gomock.InOrder( + c.EXPECT(). + Get(&kvapi.GetRequest{Key: K0}). + Return(&kvapi.GetResponse{Value: V0, Version: UUID0}, nil), + + // first Set updates key state, but returns an error. + c.EXPECT(). + Set(SetRequest(K0, V1, UUID0, &conflictErr.ExpectedVersion)). + Return(nil, errSetTemporary), + + // second Set returns conflict with ExpectedVersion == OldVersion from previous request. + c.EXPECT(). + Set(SetRequest(K0, V1, UUID0)). + Return(nil, &kvapi.ApiError{Method: "set", Err: conflictErr}), + ) + + require.NoError(t, retryupdate.UpdateValue(c, K0, updateFn)) +}