From d8ac09b302663fc5b8759579585a506112db3c7c Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Tue, 7 Jan 2020 00:08:24 +0300 Subject: [PATCH] Gh 10166 (#13021) * model/config: add required server config field to manifest * plugin/helpers: add check required server configuration method * plugin/helpers: code polish * model/manifest: add documentation * Update plugin/helpers_config.go Co-Authored-By: Ben Schumacher * plugin/helpers_config: remove stagnant line * plugin/helpers: update to v5 * plugin/helpers_config: add license Co-authored-by: Ben Schumacher Co-authored-by: Jesse Hallam Co-authored-by: mattermod --- model/manifest.go | 5 +++ plugin/helpers.go | 6 +++ plugin/helpers_config.go | 32 ++++++++++++++ plugin/helpers_config_test.go | 82 +++++++++++++++++++++++++++++++++++ plugin/plugintest/helpers.go | 21 +++++++++ 5 files changed, 146 insertions(+) create mode 100644 plugin/helpers_config.go create mode 100644 plugin/helpers_config_test.go diff --git a/model/manifest.go b/model/manifest.go index 8d4038f47a..43966e057a 100644 --- a/model/manifest.go +++ b/model/manifest.go @@ -168,6 +168,11 @@ type Manifest struct { // Plugins can store any kind of data in Props to allow other plugins to use it. Props map[string]interface{} `json:"props,omitempty" yaml:"props,omitempty"` + + // RequiredConfig defines any required server configuration fields for the plugin to function properly. + // + // Use the plugin helpers CheckRequiredServerConfiguration method to enforce this. + RequiredConfig *Config `json:"required_configuration,omitempty" yaml:"required_configuration,omitempty"` } type ManifestServer struct { diff --git a/plugin/helpers.go b/plugin/helpers.go index a871cf4584..c3cf910733 100644 --- a/plugin/helpers.go +++ b/plugin/helpers.go @@ -58,6 +58,12 @@ type Helpers interface { // Minimum server version: 5.6 KVSetWithExpiryJSON(key string, value interface{}, expireInSeconds int64) error + // CheckRequiredServerConfiguration checks if the server is configured according to + // plugin requirements. + // + // Minimum server version: 5.2 + CheckRequiredServerConfiguration(req *model.Config) (bool, error) + // ShouldProcessMessage returns if the message should be processed by a message hook. // // Use this method to avoid processing unnecessary messages in a MessageHasBeenPosted diff --git a/plugin/helpers_config.go b/plugin/helpers_config.go new file mode 100644 index 0000000000..613b03473d --- /dev/null +++ b/plugin/helpers_config.go @@ -0,0 +1,32 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package plugin + +import ( + "github.com/pkg/errors" + + "github.com/mattermost/mattermost-server/v5/model" + "github.com/mattermost/mattermost-server/v5/utils" +) + +// CheckRequiredServerConfiguration implements Helpers.CheckRequiredServerConfiguration +func (p *HelpersImpl) CheckRequiredServerConfiguration(req *model.Config) (bool, error) { + if req == nil { + return true, nil + } + + cfg := p.API.GetConfig() + + mc, err := utils.Merge(req, cfg, nil) + if err != nil { + return false, errors.Wrap(err, "could not merge configurations") + } + + mergedCfg := mc.(model.Config) + if mergedCfg.ToJson() != cfg.ToJson() { + return false, nil + } + + return true, nil +} diff --git a/plugin/helpers_config_test.go b/plugin/helpers_config_test.go new file mode 100644 index 0000000000..82838241a2 --- /dev/null +++ b/plugin/helpers_config_test.go @@ -0,0 +1,82 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package plugin_test + +import ( + "testing" + + "github.com/mattermost/mattermost-server/v5/model" + "github.com/mattermost/mattermost-server/v5/plugin" + "github.com/mattermost/mattermost-server/v5/plugin/plugintest" + "github.com/stretchr/testify/assert" +) + +func TestCheckRequiredServerConfiguration(t *testing.T) { + for name, test := range map[string]struct { + SetupAPI func(*plugintest.API) *plugintest.API + Input *model.Config + ShouldReturn bool + ShouldError bool + }{ + "no required config therefore it should be compatible": { + SetupAPI: func(api *plugintest.API) *plugintest.API { + return api + }, + Input: nil, + ShouldReturn: true, + ShouldError: false, + }, + "same configurations": { + SetupAPI: func(api *plugintest.API) *plugintest.API { + api.On("GetConfig").Return(&model.Config{ + ServiceSettings: model.ServiceSettings{ + EnableCommands: model.NewBool(true), + }, + }) + + return api + }, + Input: &model.Config{ + ServiceSettings: model.ServiceSettings{ + EnableCommands: model.NewBool(true), + }, + }, + ShouldReturn: true, + ShouldError: false, + }, + "different configurations": { + SetupAPI: func(api *plugintest.API) *plugintest.API { + api.On("GetConfig").Return(&model.Config{}) + + return api + }, + Input: &model.Config{ + ServiceSettings: model.ServiceSettings{ + EnableCommands: model.NewBool(true), + }, + }, + ShouldReturn: false, + ShouldError: false, + }, + } { + t.Run(name, func(t *testing.T) { + api := test.SetupAPI(&plugintest.API{}) + defer api.AssertExpectations(t) + + p := &plugin.HelpersImpl{} + p.API = api + + ok, err := p.CheckRequiredServerConfiguration(test.Input) + + if !ok { + assert.False(t, ok) + } + if test.ShouldError { + assert.NotNil(t, err) + } else { + assert.Nil(t, err) + } + }) + } +} diff --git a/plugin/plugintest/helpers.go b/plugin/plugintest/helpers.go index e79ad4a379..f14602eb51 100644 --- a/plugin/plugintest/helpers.go +++ b/plugin/plugintest/helpers.go @@ -15,6 +15,27 @@ type Helpers struct { mock.Mock } +// CheckRequiredServerConfiguration provides a mock function with given fields: req +func (_m *Helpers) CheckRequiredServerConfiguration(req *model.Config) (bool, error) { + ret := _m.Called(req) + + var r0 bool + if rf, ok := ret.Get(0).(func(*model.Config) bool); ok { + r0 = rf(req) + } else { + r0 = ret.Get(0).(bool) + } + + var r1 error + if rf, ok := ret.Get(1).(func(*model.Config) error); ok { + r1 = rf(req) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // EnsureBot provides a mock function with given fields: bot, options func (_m *Helpers) EnsureBot(bot *model.Bot, options ...plugin.EnsureBotOption) (string, error) { _va := make([]interface{}, len(options))