diff --git a/plugin/api.go b/plugin/api.go index 5c901dcc01..f9e29c7058 100644 --- a/plugin/api.go +++ b/plugin/api.go @@ -456,37 +456,36 @@ type API interface { // KV Store Section - // KVSet will store a key-value pair, unique per plugin. + // KVSet stores a key-value pair, unique per plugin. // Provided helper functions and internal plugin code will use the prefix `mmi_` before keys. Do not use this prefix. KVSet(key string, value []byte) *model.AppError - // KVCompareAndSet will update a key-value pair, - // unique per plugin, to the given new value if the current value == the old value. + // KVCompareAndSet updates a key-value pair, unique per plugin, but only if the current value matches the given oldValue. // Inserts a new key if oldValue == nil. // Returns (false, err) if DB error occurred - // Returns (false, nil) if current value != old value or key already exists when inserting - // Returns (true, nil) if current value == old value or new key is inserted + // Returns (false, nil) if current value != oldValue or key already exists when inserting + // Returns (true, nil) if current value == oldValue or new key is inserted // // Minimum server version: 5.12 KVCompareAndSet(key string, oldValue, newValue []byte) (bool, *model.AppError) - // KVSet will store a key-value pair, unique per plugin with an expiry time + // KVSet stores a key-value pair with an expiry time, unique per plugin. // // Minimum server version: 5.6 KVSetWithExpiry(key string, value []byte, expireInSeconds int64) *model.AppError - // KVGet will retrieve a value based on the key. Returns nil for non-existent keys. + // KVGet retrieves a value based on the key, unique per plugin. Returns nil for non-existent keys. KVGet(key string) ([]byte, *model.AppError) - // KVDelete will remove a key-value pair. Returns nil for non-existent keys. + // KVDelete removes a key-value pair, unique per plugin. Returns nil for non-existent keys. KVDelete(key string) *model.AppError - // KVDeleteAll will remove all key-value pairs for a plugin. + // KVDeleteAll removes all key-value pairs for a plugin. // // Minimum server version: 5.6 KVDeleteAll() *model.AppError - // KVList will list all keys for a plugin. + // KVList lists all keys for a plugin. // // Minimum server version: 5.6 KVList(page, perPage int) ([]string, *model.AppError) diff --git a/plugin/helpers.go b/plugin/helpers.go index a0f77e4972..ed230cd548 100644 --- a/plugin/helpers.go +++ b/plugin/helpers.go @@ -6,25 +6,28 @@ package plugin import "github.com/mattermost/mattermost-server/model" type Helpers interface { - // EnsureBot either returns an existing bot user or creates a bot user with - // the specifications of the passed bot. - // Returns the id of the bot created or existing. + // EnsureBot either returns an existing bot user matching the given bot, or creates a bot user from the given bot. + // Returns the id of the resulting bot. EnsureBot(bot *model.Bot) (string, error) - // KVGetJSON retrievs a value based on the key. - KVGetJSON(key string, value interface{}) error - - // KVSetJSON stores a key-value pair. + // KVSetJSON stores a key-value pair, unique per plugin, marshalling the given value as a JSON string. KVSetJSON(key string, value interface{}) error - // KVCompareAndSetJSON updates a key-value pair if the current - // value is equal to oldValue. - // Returns (false, err) if DB/marshal/unmarshal error occurred - // Returns (false, nil) if current value != old value - // Returns (true, nil) if current value == old value or new key is inserted + // KVCompareAndSetJSON updates a key-value pair, unique per plugin, but only if the current value matches the given oldValue after marshalling as a JSON string. + // Inserts a new key if oldValue == nil. + // Returns (false, err) if DB error occurred + // Returns (false, nil) if current value != oldValue or key already exists when inserting + // Returns (true, nil) if current value == oldValue or new key is inserted + // + // Minimum server version: 5.12 KVCompareAndSetJSON(key string, oldValue interface{}, newValue interface{}) (bool, error) - // KVSetWithExpiryJSON stores a key-value pair with an expiry time. + // KVGetJSON retrieves a value based on the key, unique per plugin, unmarshalling the previously set JSON string into the given value. Returns true if the key exists. + KVGetJSON(key string, value interface{}) (bool, error) + + // KVSetWithExpiryJSON stores a key-value pair with an expiry time, unique per plugin, marshalling the given value as a JSON string. + // + // Minimum server version: 5.6 KVSetWithExpiryJSON(key string, value interface{}, expireInSeconds int64) error } diff --git a/plugin/helpers_bots.go b/plugin/helpers_bots.go index 4dac270dca..664a954d5d 100644 --- a/plugin/helpers_bots.go +++ b/plugin/helpers_bots.go @@ -4,7 +4,6 @@ package plugin import ( - "encoding/json" "time" "github.com/mattermost/mattermost-server/model" @@ -65,42 +64,3 @@ func (p *HelpersImpl) EnsureBot(bot *model.Bot) (retBotId string, retErr error) return createdBot.UserId, nil } - -func (p *HelpersImpl) KVGetJSON(key string, value interface{}) error { - data, err := p.API.KVGet(key) - if err != nil { - return err - } - - return json.Unmarshal(data, value) -} - -func (p *HelpersImpl) KVSetJSON(key string, value interface{}) error { - data, err := json.Marshal(value) - if err != nil { - return err - } - return p.API.KVSet(key, data) -} - -func (p *HelpersImpl) KVCompareAndSetJSON(key string, oldValue interface{}, newValue interface{}) (bool, error) { - oldData, err := json.Marshal(oldValue) - if err != nil { - return false, errors.Wrap(err, "unable to marshal old value") - } - - newData, err := json.Marshal(newValue) - if err != nil { - return false, errors.Wrap(err, "unable to marshal new value") - } - - return p.API.KVCompareAndSet(key, oldData, newData) -} - -func (p *HelpersImpl) KVSetWithExpiryJSON(key string, value interface{}, expireInSeconds int64) error { - data, err := json.Marshal(value) - if err != nil { - return err - } - return p.API.KVSetWithExpiry(key, data, expireInSeconds) -} diff --git a/plugin/helpers_bots_test.go b/plugin/helpers_bots_test.go index cbd1b2405b..776c32a10b 100644 --- a/plugin/helpers_bots_test.go +++ b/plugin/helpers_bots_test.go @@ -153,179 +153,3 @@ func TestEnsureBot(t *testing.T) { }) }) } - -func TestKVGetJSON(t *testing.T) { - setupAPI := func() *plugintest.API { - return &plugintest.API{} - } - - t.Run("KVGet error", func(t *testing.T) { - p := &plugin.HelpersImpl{} - - api := setupAPI() - api.On("KVGet", plugin.BOT_USER_KEY).Return(nil, &model.AppError{}) - p.API = api - - var dat map[string]interface{} - - err := p.KVGetJSON(plugin.BOT_USER_KEY, dat) - - api.AssertExpectations(t) - assert.NotNil(t, err) - assert.Nil(t, dat) - }) - - t.Run("Malformed JSON", func(t *testing.T) { - key := "test-key" - - p := &plugin.HelpersImpl{} - - api := setupAPI() - api.On("KVGet", key).Return([]byte(`{{:}"val-a": 10}`), nil) - p.API = api - - var dat map[string]interface{} - - err := p.KVGetJSON(key, &dat) - - api.AssertExpectations(t) - assert.NotNil(t, err) - assert.Nil(t, dat) - }) - - t.Run("Valid parameters passed (happy-path)", func(t *testing.T) { - key := "test-key" - - p := &plugin.HelpersImpl{} - - api := setupAPI() - api.On("KVGet", key).Return([]byte(`{"val-a": 10}`), nil) - p.API = api - - var dat map[string]interface{} - - err := p.KVGetJSON(key, &dat) - - api.AssertExpectations(t) - assert.Nil(t, err) - assert.Equal(t, map[string]interface{}{ - "val-a": float64(10), - }, dat) - }) -} - -func TestKVSetJSON(t *testing.T) { - key := "test-key" - - setupAPI := func() *plugintest.API { - return &plugintest.API{} - } - - t.Run("JSON Marshal error", func(t *testing.T) { - api := setupAPI() - api.AssertNotCalled(t, "KVSet") - - p := &plugin.HelpersImpl{API: api} - - err := p.KVSetJSON(key, func() { return }) - - api.AssertExpectations(t) - assert.NotNil(t, err) - }) - - t.Run("Valid parameters passed (Happy-path)", func(t *testing.T) { - api := setupAPI() - api.On("KVSet", key, []byte(`{"val-a":10}`)).Return(nil) - - p := &plugin.HelpersImpl{API: api} - - err := p.KVSetJSON(key, map[string]interface{}{ - "val-a": float64(10), - }) - - api.AssertExpectations(t) - assert.Nil(t, err) - }) -} - -func TestKVCompareAndSetJSON(t *testing.T) { - key := "test-key" - setupAPI := func() *plugintest.API { - return &plugintest.API{} - } - - t.Run("old value JSON marshal error", func(t *testing.T) { - api := setupAPI() - api.AssertNotCalled(t, "KVCompareAndSet") - p := &plugin.HelpersImpl{API: api} - - ok, err := p.KVCompareAndSetJSON(key, func() { return }, map[string]interface{}{}) - - api.AssertExpectations(t) - assert.Equal(t, false, ok) - assert.NotNil(t, err) - }) - - t.Run("new value JSON marshal error", func(t *testing.T) { - api := setupAPI() - api.AssertNotCalled(t, "KVCompareAndSet") - - p := &plugin.HelpersImpl{API: api} - - ok, err := p.KVCompareAndSetJSON(key, map[string]interface{}{}, func() { return }) - - api.AssertExpectations(t) - assert.Equal(t, false, ok) - assert.NotNil(t, err) - }) - - t.Run("Valid parameters passed (happy-path)", func(t *testing.T) { - api := setupAPI() - api.On("KVCompareAndSet", key, []byte(`{"val-a":10}`), []byte(`{"val-b":20}`)).Return(false, nil) - p := &plugin.HelpersImpl{API: api} - - ok, err := p.KVCompareAndSetJSON(key, map[string]interface{}{ - "val-a": 10, - }, map[string]interface{}{ - "val-b": 20, - }) - - api.AssertExpectations(t) - assert.Equal(t, false, ok) - assert.Nil(t, err) - }) -} - -func TestKVSetWithExpiryJSON(t *testing.T) { - key := "test-key" - - setupAPI := func() *plugintest.API { - return &plugintest.API{} - } - - t.Run("JSON Marshal error", func(t *testing.T) { - api := setupAPI() - api.AssertNotCalled(t, "KVSetWithExpiry") - - p := &plugin.HelpersImpl{API: api} - - err := p.KVSetWithExpiryJSON(key, func() { return }, 100) - - api.AssertExpectations(t) - assert.NotNil(t, err) - }) - - t.Run("valid parameters passed (happy-path)", func(t *testing.T) { - api := setupAPI() - api.On("KVSetWithExpiry", key, []byte(`{"val-a":10}`), int64(100)).Return(nil) - - p := &plugin.HelpersImpl{API: api} - - err := p.KVSetWithExpiryJSON(key, map[string]interface{}{ - "val-a": float64(10), - }, 100) - - api.AssertExpectations(t) - assert.Nil(t, err) - }) -} diff --git a/plugin/helpers_kv.go b/plugin/helpers_kv.go new file mode 100644 index 0000000000..d230698b1c --- /dev/null +++ b/plugin/helpers_kv.go @@ -0,0 +1,70 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package plugin + +import ( + "encoding/json" + + "github.com/pkg/errors" +) + +// KVGetJSON is a wrapper around KVGet to simplify reading a JSON object from the key value store. +func (p *HelpersImpl) KVGetJSON(key string, value interface{}) (bool, error) { + data, appErr := p.API.KVGet(key) + if appErr != nil { + return false, appErr + } + if data == nil { + return false, nil + } + + err := json.Unmarshal(data, value) + if err != nil { + return false, err + } + + return true, nil +} + +// KVSetJSON is a wrapper around KVSet to simplify writing a JSON object to the key value store. +func (p *HelpersImpl) KVSetJSON(key string, value interface{}) error { + data, err := json.Marshal(value) + if err != nil { + return err + } + + return p.API.KVSet(key, data) +} + +// KVCompareAndSetJSON is a wrapper around KVCompareAndSet to simplify atomically writing a JSON object to the key value store. +func (p *HelpersImpl) KVCompareAndSetJSON(key string, oldValue interface{}, newValue interface{}) (bool, error) { + var oldData, newData []byte + var err error + + if oldValue != nil { + oldData, err = json.Marshal(oldValue) + if err != nil { + return false, errors.Wrap(err, "unable to marshal old value") + } + } + + if newValue != nil { + newData, err = json.Marshal(newValue) + if err != nil { + return false, errors.Wrap(err, "unable to marshal new value") + } + } + + return p.API.KVCompareAndSet(key, oldData, newData) +} + +// KVSetWithExpiryJSON is a wrapper around KVSetWithExpiry to simplify atomically writing a JSON object with expiry to the key value store. +func (p *HelpersImpl) KVSetWithExpiryJSON(key string, value interface{}, expireInSeconds int64) error { + data, err := json.Marshal(value) + if err != nil { + return err + } + + return p.API.KVSetWithExpiry(key, data, expireInSeconds) +} diff --git a/plugin/helpers_kv_test.go b/plugin/helpers_kv_test.go new file mode 100644 index 0000000000..bc313ba05a --- /dev/null +++ b/plugin/helpers_kv_test.go @@ -0,0 +1,204 @@ +package plugin_test + +import ( + "testing" + + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/plugin" + "github.com/mattermost/mattermost-server/plugin/plugintest" + "github.com/stretchr/testify/assert" +) + +func TestKVGetJSON(t *testing.T) { + t.Run("KVGet error", func(t *testing.T) { + p := &plugin.HelpersImpl{} + + api := &plugintest.API{} + api.On("KVGet", "test-key").Return(nil, &model.AppError{}) + p.API = api + + var dat map[string]interface{} + + ok, err := p.KVGetJSON("test-key", dat) + api.AssertExpectations(t) + assert.False(t, ok) + assert.NotNil(t, err) + assert.Nil(t, dat) + }) + + t.Run("unknown key", func(t *testing.T) { + p := &plugin.HelpersImpl{} + + api := &plugintest.API{} + api.On("KVGet", "test-key").Return(nil, nil) + p.API = api + + var dat map[string]interface{} + + ok, err := p.KVGetJSON("test-key", dat) + api.AssertExpectations(t) + assert.False(t, ok) + assert.Nil(t, err) + assert.Nil(t, dat) + }) + + t.Run("malformed JSON", func(t *testing.T) { + p := &plugin.HelpersImpl{} + + api := &plugintest.API{} + api.On("KVGet", "test-key").Return([]byte(`{{:}"val-a": 10}`), nil) + p.API = api + + var dat map[string]interface{} + + ok, err := p.KVGetJSON("test-key", &dat) + api.AssertExpectations(t) + assert.False(t, ok) + assert.NotNil(t, err) + assert.Nil(t, dat) + }) + + t.Run("wellformed JSON", func(t *testing.T) { + p := &plugin.HelpersImpl{} + + api := &plugintest.API{} + api.On("KVGet", "test-key").Return([]byte(`{"val-a": 10}`), nil) + p.API = api + + var dat map[string]interface{} + + ok, err := p.KVGetJSON("test-key", &dat) + assert.True(t, ok) + api.AssertExpectations(t) + assert.Nil(t, err) + assert.Equal(t, map[string]interface{}{ + "val-a": float64(10), + }, dat) + }) +} + +func TestKVSetJSON(t *testing.T) { + t.Run("JSON marshal error", func(t *testing.T) { + api := &plugintest.API{} + api.AssertNotCalled(t, "KVSet") + + p := &plugin.HelpersImpl{API: api} + + err := p.KVSetJSON("test-key", func() { return }) + api.AssertExpectations(t) + assert.NotNil(t, err) + }) + + t.Run("marshallable struct", func(t *testing.T) { + api := &plugintest.API{} + api.On("KVSet", "test-key", []byte(`{"val-a":10}`)).Return(nil) + + p := &plugin.HelpersImpl{API: api} + + err := p.KVSetJSON("test-key", map[string]interface{}{ + "val-a": float64(10), + }) + + api.AssertExpectations(t) + assert.Nil(t, err) + }) +} + +func TestKVCompareAndSetJSON(t *testing.T) { + t.Run("old value JSON marshal error", func(t *testing.T) { + api := &plugintest.API{} + api.AssertNotCalled(t, "KVCompareAndSet") + p := &plugin.HelpersImpl{API: api} + + ok, err := p.KVCompareAndSetJSON("test-key", func() { return }, map[string]interface{}{}) + + api.AssertExpectations(t) + assert.Equal(t, false, ok) + assert.NotNil(t, err) + }) + + t.Run("new value JSON marshal error", func(t *testing.T) { + api := &plugintest.API{} + api.AssertNotCalled(t, "KVCompareAndSet") + + p := &plugin.HelpersImpl{API: api} + + ok, err := p.KVCompareAndSetJSON("test-key", map[string]interface{}{}, func() { return }) + + api.AssertExpectations(t) + assert.False(t, ok) + assert.NotNil(t, err) + }) + + t.Run("old value nil", func(t *testing.T) { + api := &plugintest.API{} + api.On("KVCompareAndSet", "test-key", []byte(nil), []byte(`{"val-b":20}`)).Return(true, nil) + p := &plugin.HelpersImpl{API: api} + + ok, err := p.KVCompareAndSetJSON("test-key", nil, map[string]interface{}{ + "val-b": 20, + }) + + api.AssertExpectations(t) + assert.True(t, ok) + assert.Nil(t, err) + }) + + t.Run("old value non-nil", func(t *testing.T) { + api := &plugintest.API{} + api.On("KVCompareAndSet", "test-key", []byte(`{"val-a":10}`), []byte(`{"val-b":20}`)).Return(true, nil) + p := &plugin.HelpersImpl{API: api} + + ok, err := p.KVCompareAndSetJSON("test-key", map[string]interface{}{ + "val-a": 10, + }, map[string]interface{}{ + "val-b": 20, + }) + + api.AssertExpectations(t) + assert.True(t, ok) + assert.Nil(t, err) + }) + + t.Run("new value nil", func(t *testing.T) { + api := &plugintest.API{} + api.On("KVCompareAndSet", "test-key", []byte(`{"val-a":10}`), []byte(nil)).Return(true, nil) + p := &plugin.HelpersImpl{API: api} + + ok, err := p.KVCompareAndSetJSON("test-key", map[string]interface{}{ + "val-a": 10, + }, nil) + + api.AssertExpectations(t) + assert.True(t, ok) + assert.Nil(t, err) + }) +} + +func TestKVSetWithExpiryJSON(t *testing.T) { + t.Run("JSON marshal error", func(t *testing.T) { + api := &plugintest.API{} + api.AssertNotCalled(t, "KVSetWithExpiry") + + p := &plugin.HelpersImpl{API: api} + + err := p.KVSetWithExpiryJSON("test-key", func() { return }, 100) + + api.AssertExpectations(t) + assert.NotNil(t, err) + }) + + t.Run("wellformed JSON", func(t *testing.T) { + api := &plugintest.API{} + api.On("KVSetWithExpiry", "test-key", []byte(`{"val-a":10}`), int64(100)).Return(nil) + + p := &plugin.HelpersImpl{API: api} + + err := p.KVSetWithExpiryJSON("test-key", map[string]interface{}{ + "val-a": float64(10), + }, 100) + + api.AssertExpectations(t) + assert.Nil(t, err) + }) +} diff --git a/plugin/plugintest/helpers.go b/plugin/plugintest/helpers.go index 17fcd75970..bf474c8471 100644 --- a/plugin/plugintest/helpers.go +++ b/plugin/plugintest/helpers.go @@ -55,17 +55,24 @@ func (_m *Helpers) KVCompareAndSetJSON(key string, oldValue interface{}, newValu } // KVGetJSON provides a mock function with given fields: key, value -func (_m *Helpers) KVGetJSON(key string, value interface{}) error { +func (_m *Helpers) KVGetJSON(key string, value interface{}) (bool, error) { ret := _m.Called(key, value) - var r0 error - if rf, ok := ret.Get(0).(func(string, interface{}) error); ok { + var r0 bool + if rf, ok := ret.Get(0).(func(string, interface{}) bool); ok { r0 = rf(key, value) } else { - r0 = ret.Error(0) + r0 = ret.Get(0).(bool) } - return r0 + var r1 error + if rf, ok := ret.Get(1).(func(string, interface{}) error); ok { + r1 = rf(key, value) + } else { + r1 = ret.Error(1) + } + + return r0, r1 } // KVSetJSON provides a mock function with given fields: key, value