diff options
-rw-r--r-- | app/plugin_api_test.go | 40 | ||||
-rw-r--r-- | plugin/client.go | 22 | ||||
-rw-r--r-- | plugin/client_rpc.go | 13 | ||||
-rw-r--r-- | plugin/example_hello_world_test.go | 2 | ||||
-rw-r--r-- | plugin/example_help_test.go | 77 | ||||
-rw-r--r-- | plugin/hooks.go | 4 |
6 files changed, 105 insertions, 53 deletions
diff --git a/app/plugin_api_test.go b/app/plugin_api_test.go index 618805bb6..12701e87e 100644 --- a/app/plugin_api_test.go +++ b/app/plugin_api_test.go @@ -76,16 +76,28 @@ func TestPluginAPILoadPluginConfiguration(t *testing.T) { "fmt" ) - type MyPlugin struct { - plugin.MattermostPlugin - + type configuration struct { MyStringSetting string MyIntSetting int MyBoolSetting bool } + type MyPlugin struct { + plugin.MattermostPlugin + + configuration configuration + } + + func (p *MyPlugin) OnConfigurationChange() error { + if err := p.API.LoadPluginConfiguration(&p.configuration); err != nil { + return err + } + + return nil + } + func (p *MyPlugin) MessageWillBePosted(c *plugin.Context, post *model.Post) (*model.Post, string) { - return nil, fmt.Sprintf("%v%v%v", p.MyStringSetting, p.MyIntSetting, p.MyBoolSetting) + return nil, fmt.Sprintf("%v%v%v", p.configuration.MyStringSetting, p.configuration.MyIntSetting, p.configuration.MyBoolSetting) } func main() { @@ -135,16 +147,28 @@ func TestPluginAPILoadPluginConfigurationDefaults(t *testing.T) { "fmt" ) - type MyPlugin struct { - plugin.MattermostPlugin - + type configuration struct { MyStringSetting string MyIntSetting int MyBoolSetting bool } + type MyPlugin struct { + plugin.MattermostPlugin + + configuration configuration + } + + func (p *MyPlugin) OnConfigurationChange() error { + if err := p.API.LoadPluginConfiguration(&p.configuration); err != nil { + return err + } + + return nil + } + func (p *MyPlugin) MessageWillBePosted(c *plugin.Context, post *model.Post) (*model.Post, string) { - return nil, fmt.Sprintf("%v%v%v", p.MyStringSetting, p.MyIntSetting, p.MyBoolSetting) + return nil, fmt.Sprintf("%v%v%v", p.configuration.MyStringSetting, p.configuration.MyIntSetting, p.configuration.MyBoolSetting) } func main() { diff --git a/plugin/client.go b/plugin/client.go index 457a16cf4..63cedfbce 100644 --- a/plugin/client.go +++ b/plugin/client.go @@ -13,12 +13,10 @@ import ( func ClientMain(pluginImplementation interface{}) { if impl, ok := pluginImplementation.(interface { SetAPI(api API) - SetSelfRef(ref interface{}) }); !ok { panic("Plugin implementation given must embed plugin.MattermostPlugin") } else { impl.SetAPI(nil) - impl.SetSelfRef(pluginImplementation) } pluginMap := map[string]plugin.Plugin{ @@ -34,8 +32,6 @@ func ClientMain(pluginImplementation interface{}) { type MattermostPlugin struct { // API exposes the plugin api, and becomes available just prior to the OnActive hook. API API - - selfRef interface{} // This is so we can unmarshal into our parent } // SetAPI persists the given API interface to the plugin. It is invoked just prior to the @@ -43,21 +39,3 @@ type MattermostPlugin struct { func (p *MattermostPlugin) SetAPI(api API) { p.API = api } - -// SetSelfRef is called by ClientMain to maintain a pointer to the plugin interface originally -// registered. This allows for the default implementation of OnConfigurationChange. -func (p *MattermostPlugin) SetSelfRef(ref interface{}) { - p.selfRef = ref -} - -// OnConfigurationChange provides a default implementation of this hook event that unmarshals the -// plugin configuration directly onto the plugin struct. -// -// Feel free to implement your own version of OnConfigurationChange if you need more advanced -// configuration handling. -func (p *MattermostPlugin) OnConfigurationChange() error { - if p.selfRef != nil { - return p.API.LoadPluginConfiguration(p.selfRef) - } - return nil -} diff --git a/plugin/client_rpc.go b/plugin/client_rpc.go index 72bd41f68..2e85466d7 100644 --- a/plugin/client_rpc.go +++ b/plugin/client_rpc.go @@ -195,11 +195,16 @@ func (s *hooksRPCServer) OnActivate(args *Z_OnActivateArgs, returns *Z_OnActivat if mmplugin, ok := s.impl.(interface { SetAPI(api API) - OnConfigurationChange() error - }); !ok { - } else { + }); ok { mmplugin.SetAPI(s.apiRPCClient) - mmplugin.OnConfigurationChange() + } + + if mmplugin, ok := s.impl.(interface { + OnConfigurationChange() error + }); ok { + if err := mmplugin.OnConfigurationChange(); err != nil { + fmt.Fprintf(os.Stderr, "[ERROR] call to OnConfigurationChange failed, error: %v", err.Error()) + } } // Capture output of standard logger because go-plugin diff --git a/plugin/example_hello_world_test.go b/plugin/example_hello_world_test.go index 0da171eb7..26f25c91a 100644 --- a/plugin/example_hello_world_test.go +++ b/plugin/example_hello_world_test.go @@ -12,7 +12,7 @@ type HelloWorldPlugin struct { } func (p *HelloWorldPlugin) ServeHTTP(c *plugin.Context, w http.ResponseWriter, r *http.Request) { - fmt.Fprintf(w, "Hello, world!") + fmt.Fprint(w, "Hello, world!") } // This example demonstrates a plugin that handles HTTP requests which respond by greeting the diff --git a/plugin/example_help_test.go b/plugin/example_help_test.go index 175b6588e..e4aa9dec3 100644 --- a/plugin/example_help_test.go +++ b/plugin/example_help_test.go @@ -2,48 +2,91 @@ package plugin_test import ( "strings" + "sync" + + "github.com/pkg/errors" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/plugin" ) -type HelpPlugin struct { - plugin.MattermostPlugin - +// configuration represents the configuration for this plugin as exposed via the Mattermost +// server configuration. +type configuration struct { TeamName string ChannelName string + // channelId is resolved when the public configuration fields above change channelId string } +type HelpPlugin struct { + plugin.MattermostPlugin + + // configurationLock synchronizes access to the configuration. + configurationLock sync.RWMutex + + // configuration is the active plugin configuration. Consult getConfiguration and + // setConfiguration for usage. + configuration *configuration +} + +// getConfiguration retrieves the active configuration under lock, making it safe to use +// concurrently. The active configuration may change underneath the client of this method, but +// the struct returned by this API call is considered immutable. +func (p *HelpPlugin) getConfiguration() *configuration { + p.configurationLock.RLock() + defer p.configurationLock.RUnlock() + + if p.configuration == nil { + return &configuration{} + } + + return p.configuration +} + +// setConfiguration replaces the active configuration under lock. +// +// Do not call setConfiguration while holding the configurationLock, as sync.Mutex is not +// reentrant. +func (p *HelpPlugin) setConfiguration(configuration *configuration) { + // Replace the active configuration under lock. + p.configurationLock.Lock() + defer p.configurationLock.Unlock() + p.configuration = configuration +} + +// OnConfigurationChange updates the active configuration for this plugin under lock. func (p *HelpPlugin) OnConfigurationChange() error { - // Reuse the default implementation of OnConfigurationChange to automatically load the - // required TeamName and ChannelName. - if err := p.MattermostPlugin.OnConfigurationChange(); err != nil { - p.API.LogError(err.Error()) - return nil + var configuration = new(configuration) + + // Load the public configuration fields from the Mattermost server configuration. + if err := p.API.LoadPluginConfiguration(configuration); err != nil { + return errors.Wrap(err, "failed to load plugin configuration") } - team, err := p.API.GetTeamByName(p.TeamName) + team, err := p.API.GetTeamByName(configuration.TeamName) if err != nil { - p.API.LogError("failed to find team", "team_name", p.TeamName) - return nil + return errors.Wrapf(err, "failed to find team %s", configuration.TeamName) } - channel, err := p.API.GetChannelByName(p.ChannelName, team.Id, false) + channel, err := p.API.GetChannelByName(configuration.ChannelName, team.Id, false) if err != nil { - p.API.LogError("failed to find channel", "channel_name", p.ChannelName) - return nil + return errors.Wrapf(err, "failed to find channel %s", configuration.ChannelName) } - p.channelId = channel.Id + configuration.channelId = channel.Id + + p.setConfiguration(configuration) return nil } func (p *HelpPlugin) MessageHasBeenPosted(c *plugin.Context, post *model.Post) { + configuration := p.getConfiguration() + // Ignore posts not in the configured channel - if post.ChannelId != p.channelId { + if post.ChannelId != configuration.channelId { return } @@ -58,7 +101,7 @@ func (p *HelpPlugin) MessageHasBeenPosted(c *plugin.Context, post *model.Post) { } p.API.SendEphemeralPost(post.UserId, &model.Post{ - ChannelId: p.channelId, + ChannelId: configuration.channelId, Message: "You asked for help? Checkout https://about.mattermost.com/help/", Props: map[string]interface{}{ "sent_by_plugin": true, diff --git a/plugin/hooks.go b/plugin/hooks.go index 4af177c0d..b5cff6f6b 100644 --- a/plugin/hooks.go +++ b/plugin/hooks.go @@ -55,7 +55,9 @@ type Hooks interface { // will stop receiving hooks just prior to this method being called. OnDeactivate() error - // OnConfigurationChange is invoked when configuration changes may have been made. + // OnConfigurationChange is invoked when configuration changes may have been made. Any + // returned error is logged, but does not stop the plugin. You must be prepared to handle + // a configuration failure gracefully. OnConfigurationChange() error // ServeHTTP allows the plugin to implement the http.Handler interface. Requests destined for |