From 40f71f93aad1f4f651a7a920f9641da6cb0163d2 Mon Sep 17 00:00:00 2001 From: ivcosla Date: Thu, 16 May 2019 21:17:52 +0200 Subject: [PATCH] bug seems fixed --- pkg/messaging-discovery/client/client.go | 23 ++++++++--------- .../client/client_mock_test.go | 8 ++++++ pkg/messaging-discovery/client/entry.go | 24 ++++++++++++++++++ .../client/http_message.go | 11 ++++++-- pkg/messaging-discovery/client/testing.go | 25 ++++++++++++++++++- 5 files changed, 75 insertions(+), 16 deletions(-) diff --git a/pkg/messaging-discovery/client/client.go b/pkg/messaging-discovery/client/client.go index 6da9cae16..f70068bec 100644 --- a/pkg/messaging-discovery/client/client.go +++ b/pkg/messaging-discovery/client/client.go @@ -5,7 +5,6 @@ import ( "bytes" "context" "encoding/json" - "errors" "fmt" "io/ioutil" "net/http" @@ -65,7 +64,7 @@ func (c *httpClient) Entry(ctx context.Context, publicKey cipher.PubKey) (*Entry return nil, err } - return nil, errors.New(message.String()) + return nil, errFromString(message.String()) } err = json.NewDecoder(resp.Body).Decode(&entry) @@ -110,8 +109,7 @@ func (c *httpClient) SetEntry(ctx context.Context, e *Entry) error { if err != nil { return err } - - return errors.New(httpResponse.String()) + return errFromString(httpResponse.Message) } return nil } @@ -130,11 +128,11 @@ func (c *httpClient) UpdateEntry(ctx context.Context, sk cipher.SecKey, e *Entry for { err = c.SetEntry(ctx, e) - if err != nil && err != ErrValidationWrongSequence { - e.Sequence-- - return err - } - if err == ErrValidationWrongSequence { + if err != nil { + if err != ErrValidationWrongSequence { + e.Sequence-- + return err + } rE, entryErr := c.Entry(ctx, e.Static) if entryErr != nil { return err @@ -148,10 +146,9 @@ func (c *httpClient) UpdateEntry(ctx context.Context, sk cipher.SecKey, e *Entry if err != nil { return err } + continue } - if err == nil { - return nil - } + return nil } } @@ -181,7 +178,7 @@ func (c *httpClient) AvailableServers(ctx context.Context) ([]*Entry, error) { return nil, err } - return nil, errors.New(message.String()) + return nil, errFromString(message.String()) } err = json.NewDecoder(resp.Body).Decode(&entries) diff --git a/pkg/messaging-discovery/client/client_mock_test.go b/pkg/messaging-discovery/client/client_mock_test.go index f407b8600..23a0ee428 100644 --- a/pkg/messaging-discovery/client/client_mock_test.go +++ b/pkg/messaging-discovery/client/client_mock_test.go @@ -287,6 +287,14 @@ func TestNewMockUpdateEntriesEndpoint(t *testing.T) { e.Server.Address = "different one" }, }, + { + name: "udpate retries on wrong sequence", + responseShouldError: false, + secretKey: sk, + entryPreHook: func(entry *client.Entry) { + entry.Sequence = 3 + }, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/pkg/messaging-discovery/client/entry.go b/pkg/messaging-discovery/client/entry.go index 73bd79a0b..660e2c3cd 100644 --- a/pkg/messaging-discovery/client/entry.go +++ b/pkg/messaging-discovery/client/entry.go @@ -28,8 +28,32 @@ var ( ErrValidationNoClientOrServer = NewEntryValidationError("entry has neither client or server field") ErrValidationWrongSequence = NewEntryValidationError("sequence field of new entry is not sequence of old entry + 1") ErrValidationWrongTime = NewEntryValidationError("previous entry timestamp is not set before current entry timestamp") + + errReverseMap = map[string]error{ + ErrKeyNotFound.Error(): ErrKeyNotFound, + ErrUnexpected.Error(): ErrUnexpected, + ErrUnauthorized.Error(): ErrUnauthorized, + ErrBadInput.Error(): ErrBadInput, + ErrValidationNonZeroSequence.Error(): ErrValidationNonZeroSequence, + ErrValidationNilEphemerals.Error(): ErrValidationNilEphemerals, + ErrValidationNilKeys.Error(): ErrValidationNilKeys, + ErrValidationNonNilEphemerals.Error(): ErrValidationNonNilEphemerals, + ErrValidationNoSignature.Error(): ErrValidationNoSignature, + ErrValidationNoVersion.Error(): ErrValidationNoVersion, + ErrValidationNoClientOrServer.Error(): ErrValidationNoClientOrServer, + ErrValidationWrongSequence.Error(): ErrValidationWrongSequence, + ErrValidationWrongTime.Error(): ErrValidationWrongTime, + } ) +func errFromString(s string) error { + err, ok := errReverseMap[s] + if !ok { + return ErrUnexpected + } + return err +} + // EntryValidationError represents transient error caused by invalid // data in Entry type EntryValidationError struct { diff --git a/pkg/messaging-discovery/client/http_message.go b/pkg/messaging-discovery/client/http_message.go index 6d50604a8..f5adc68be 100644 --- a/pkg/messaging-discovery/client/http_message.go +++ b/pkg/messaging-discovery/client/http_message.go @@ -1,14 +1,15 @@ package client import ( + "errors" "fmt" "net/http" ) // Exposed Http messages var ( - MsgEntrySet = HTTPMessage{Code: http.StatusOK, Message: "wrote a new entry"} - MsgEntryUpdated = HTTPMessage{Code: http.StatusOK, Message: "wrote new entry iteration"} + MsgEntrySet = HTTPMessage{Code: http.StatusOK, Message: "wrote a new entry"} + MsgEntryUpdated = HTTPMessage{Code: http.StatusOK, Message: "wrote new entry iteration"} ) // HTTPMessage represents a message to be returned as an http response @@ -20,3 +21,9 @@ type HTTPMessage struct { func (h HTTPMessage) String() string { return fmt.Sprintf("status code: %d. message: %s", h.Code, h.Message) } + +// ToError returns an error representing the httpMessage for error comparisons, this is preferred for this type instead +// of implementing error +func (h HTTPMessage) ToError() error { + return errors.New(h.String()) +} diff --git a/pkg/messaging-discovery/client/testing.go b/pkg/messaging-discovery/client/testing.go index 8fd77ade4..c0483d995 100644 --- a/pkg/messaging-discovery/client/testing.go +++ b/pkg/messaging-discovery/client/testing.go @@ -91,7 +91,30 @@ func (m *mockClient) UpdateEntry(ctx context.Context, sk cipher.SecKey, e *Entry return err } - return m.SetEntry(ctx, e) + for { + err = m.SetEntry(ctx, e) + if err != nil { + if err != ErrValidationWrongSequence { + e.Sequence-- + return err + } + rE, entryErr := m.Entry(ctx, e.Static) + if entryErr != nil { + return err + } + if rE.Timestamp > e.Timestamp { // If there is a more up to date entry drop update + e.Sequence = rE.Sequence + return nil + } + e.Sequence = rE.Sequence + 1 + err := e.Sign(sk) + if err != nil { + return err + } + continue + } + return nil + } } // AvailableServers returns all the servers that the APIClient mock has