MM-11292: clean up plugins GoDoc (#9109)

* clean up plugins GoDoc:

- eliminate plugin.NewBlankContext() as unnecessary
- export ValidIdRegex as a string vs. the less readable var
- add/update various documentation strings
- hide everything by default, except where used by client plugins or the mattermost-server. The exception to this rule are the `*(Args|Returns)` structs which must be public for go-plugin, but are now prefixed with `Z_` with a warning not to use.
- include a top-level example to get plugin authors started

This is not a breaking change for existing plugins compiled against
plugins-v2.

* remove commented out ServeHTTPResponseWriter

* update examples to match developer docs

* add missing plugin/doc.go license header

app/plugin_commands.go

@@ -105,7 +105,7 @@ func (a *App) ExecutePluginCommand(args *model.CommandArgs) (*model.Command, *mo
 			if err != nil {
 				return pc.Command, nil, model.NewAppError("ExecutePluginCommand", "model.plugin_command.error.app_error", nil, "err="+err.Error(), http.StatusInternalServerError)
 			}
-			response, appErr := pluginHooks.ExecuteCommand(plugin.NewBlankContext(), args)
+			response, appErr := pluginHooks.ExecuteCommand(&plugin.Context{}, args)
 			return pc.Command, response, appErr
 		}
 	}

app/plugin_install.go

@@ -52,7 +52,7 @@ func (a *App) installPlugin(pluginFile io.Reader) (*model.Manifest, *model.AppEr
 	}
 
 	if !plugin.IsValidId(manifest.Id) {
-		return nil, model.NewAppError("installPlugin", "app.plugin.invalid_id.app_error", map[string]interface{}{"Min": plugin.MinIdLength, "Max": plugin.MaxIdLength, "Regex": plugin.ValidId.String()}, "", http.StatusBadRequest)
+		return nil, model.NewAppError("installPlugin", "app.plugin.invalid_id.app_error", map[string]interface{}{"Min": plugin.MinIdLength, "Max": plugin.MaxIdLength, "Regex": plugin.ValidIdRegex}, "", http.StatusBadRequest)
 	}
 
 	bundles, err := a.Plugins.Available()

app/plugin_requests.go

@@ -72,5 +72,5 @@ func (a *App) servePluginRequest(w http.ResponseWriter, r *http.Request, handler
 	r.URL.RawQuery = newQuery.Encode()
 	r.URL.Path = strings.TrimPrefix(r.URL.Path, "/plugins/"+params["plugin_id"])
 
-	handler(plugin.NewBlankContext(), w, r)
+	handler(&plugin.Context{}, w, r)
 }

plugin/api.go

@@ -11,7 +11,8 @@ import (
 // The API can be used to retrieve data or perform actions on behalf of the plugin. Most methods
 // have direct counterparts in the REST API and very similar behavior.
 //
-// Plugins can obtain access to the API by implementing the OnActivate hook.
+// Plugins obtain access to the API by embedding MattermostPlugin and accessing the API member
+// directly.
 type API interface {
 	// LoadPluginConfiguration loads the plugin's configuration. dest should be a pointer to a
 	// struct that the configuration JSON can be unmarshalled to.
@@ -178,7 +179,7 @@ type API interface {
 	LogWarn(msg string, keyValuePairs ...interface{})
 }
 
-var Handshake = plugin.HandshakeConfig{
+var handshake = plugin.HandshakeConfig{
 	ProtocolVersion:  1,
 	MagicCookieKey:   "MATTERMOST_PLUGIN",
 	MagicCookieValue: "Securely message teams, anywhere.",

plugin/client.go

@@ -7,8 +7,9 @@ import (
 	"github.com/hashicorp/go-plugin"
 )
 
-// Starts the serving of a Mattermost plugin over rpc or gRPC
+// Starts the serving of a Mattermost plugin over net/rpc. gRPC is not yet supported.
-// Call this when your plugin is ready to start
+//
+// Call this when your plugin is ready to start.
 func ClientMain(pluginImplementation interface{}) {
 	if impl, ok := pluginImplementation.(interface {
 		SetAPI(api API)
@@ -21,28 +22,39 @@ func ClientMain(pluginImplementation interface{}) {
 	}
 
 	pluginMap := map[string]plugin.Plugin{
-		"hooks": &HooksPlugin{hooks: pluginImplementation},
+		"hooks": &hooksPlugin{hooks: pluginImplementation},
 	}
 
 	plugin.Serve(&plugin.ServeConfig{
-		HandshakeConfig: Handshake,
+		HandshakeConfig: handshake,
 		Plugins:         pluginMap,
 	})
 }
 
 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
+// OnActivate hook, exposing the API for use by the plugin.
 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)

plugin/client_rpc.go

@@ -22,9 +22,9 @@ import (
 	"github.com/mattermost/mattermost-server/model"
 )
 
-var HookNameToId map[string]int = make(map[string]int)
+var hookNameToId map[string]int = make(map[string]int)
 
-type HooksRPCClient struct {
+type hooksRPCClient struct {
 	client      *rpc.Client
 	log         *mlog.Logger
 	muxBroker   *plugin.MuxBroker
@@ -32,33 +32,33 @@ type HooksRPCClient struct {
 	implemented [TotalHooksId]bool
 }
 
-type HooksRPCServer struct {
+type hooksRPCServer struct {
 	impl         interface{}
 	muxBroker    *plugin.MuxBroker
-	apiRPCClient *APIRPCClient
+	apiRPCClient *apiRPCClient
 }
 
 // Implements hashicorp/go-plugin/plugin.Plugin interface to connect the hooks of a plugin
-type HooksPlugin struct {
+type hooksPlugin struct {
 	hooks   interface{}
 	apiImpl API
 	log     *mlog.Logger
 }
 
-func (p *HooksPlugin) Server(b *plugin.MuxBroker) (interface{}, error) {
+func (p *hooksPlugin) Server(b *plugin.MuxBroker) (interface{}, error) {
-	return &HooksRPCServer{impl: p.hooks, muxBroker: b}, nil
+	return &hooksRPCServer{impl: p.hooks, muxBroker: b}, nil
 }
 
-func (p *HooksPlugin) Client(b *plugin.MuxBroker, client *rpc.Client) (interface{}, error) {
+func (p *hooksPlugin) Client(b *plugin.MuxBroker, client *rpc.Client) (interface{}, error) {
-	return &HooksRPCClient{client: client, log: p.log, muxBroker: b, apiImpl: p.apiImpl}, nil
+	return &hooksRPCClient{client: client, log: p.log, muxBroker: b, apiImpl: p.apiImpl}, nil
 }
 
-type APIRPCClient struct {
+type apiRPCClient struct {
 	client *rpc.Client
 	log    *mlog.Logger
 }
 
-type APIRPCServer struct {
+type apiRPCServer struct {
 	impl API
 }
 
@@ -72,17 +72,17 @@ func init() {
 // These enforce compile time checks to make sure types implement the interface
 // If you are getting an error here, you probably need to run `make pluginapi` to
 // autogenerate RPC glue code
-var _ plugin.Plugin = &HooksPlugin{}
+var _ plugin.Plugin = &hooksPlugin{}
-var _ Hooks = &HooksRPCClient{}
+var _ Hooks = &hooksRPCClient{}
 
 //
 // Below are specal cases for hooks or APIs that can not be auto generated
 //
 
-func (g *HooksRPCClient) Implemented() (impl []string, err error) {
+func (g *hooksRPCClient) Implemented() (impl []string, err error) {
 	err = g.client.Call("Plugin.Implemented", struct{}{}, &impl)
 	for _, hookName := range impl {
-		if hookId, ok := HookNameToId[hookName]; ok {
+		if hookId, ok := hookNameToId[hookName]; ok {
 			g.implemented[hookId] = true
 		}
 	}
@@ -90,7 +90,7 @@ func (g *HooksRPCClient) Implemented() (impl []string, err error) {
 }
 
 // Implemented replies with the names of the hooks that are implemented.
-func (s *HooksRPCServer) Implemented(args struct{}, reply *[]string) error {
+func (s *hooksRPCServer) Implemented(args struct{}, reply *[]string) error {
 	ifaceType := reflect.TypeOf((*Hooks)(nil)).Elem()
 	implType := reflect.TypeOf(s.impl)
 	selfType := reflect.TypeOf(s)
@@ -130,24 +130,24 @@ func (s *HooksRPCServer) Implemented(args struct{}, reply *[]string) error {
 	return nil
 }
 
-type OnActivateArgs struct {
+type Z_OnActivateArgs struct {
 	APIMuxId uint32
 }
 
-type OnActivateReturns struct {
+type Z_OnActivateReturns struct {
 	A error
 }
 
-func (g *HooksRPCClient) OnActivate() error {
+func (g *hooksRPCClient) OnActivate() error {
 	muxId := g.muxBroker.NextId()
-	go g.muxBroker.AcceptAndServe(muxId, &APIRPCServer{
+	go g.muxBroker.AcceptAndServe(muxId, &apiRPCServer{
 		impl: g.apiImpl,
 	})
 
-	_args := &OnActivateArgs{
+	_args := &Z_OnActivateArgs{
 		APIMuxId: muxId,
 	}
-	_returns := &OnActivateReturns{}
+	_returns := &Z_OnActivateReturns{}
 
 	if err := g.client.Call("Plugin.OnActivate", _args, _returns); err != nil {
 		g.log.Error("RPC call to OnActivate plugin failed.", mlog.Err(err))
@@ -155,13 +155,13 @@ func (g *HooksRPCClient) OnActivate() error {
 	return _returns.A
 }
 
-func (s *HooksRPCServer) OnActivate(args *OnActivateArgs, returns *OnActivateReturns) error {
+func (s *hooksRPCServer) OnActivate(args *Z_OnActivateArgs, returns *Z_OnActivateReturns) error {
 	connection, err := s.muxBroker.Dial(args.APIMuxId)
 	if err != nil {
 		return err
 	}
 
-	s.apiRPCClient = &APIRPCClient{
+	s.apiRPCClient = &apiRPCClient{
 		client: rpc.NewClient(connection),
 	}
 
@@ -186,23 +186,23 @@ func (s *HooksRPCServer) OnActivate(args *OnActivateArgs, returns *OnActivateRet
 	return nil
 }
 
-type LoadPluginConfigurationArgs struct {
+type Z_LoadPluginConfigurationArgsArgs struct {
 }
 
-type LoadPluginConfigurationReturns struct {
+type Z_LoadPluginConfigurationArgsReturns struct {
 	A []byte
 }
 
-func (g *APIRPCClient) LoadPluginConfiguration(dest interface{}) error {
+func (g *apiRPCClient) LoadPluginConfiguration(dest interface{}) error {
-	_args := &LoadPluginConfigurationArgs{}
+	_args := &Z_LoadPluginConfigurationArgsArgs{}
-	_returns := &LoadPluginConfigurationReturns{}
+	_returns := &Z_LoadPluginConfigurationArgsReturns{}
 	if err := g.client.Call("Plugin.LoadPluginConfiguration", _args, _returns); err != nil {
 		g.log.Error("RPC call to LoadPluginConfiguration API failed.", mlog.Err(err))
 	}
 	return json.Unmarshal(_returns.A, dest)
 }
 
-func (s *APIRPCServer) LoadPluginConfiguration(args *LoadPluginConfigurationArgs, returns *LoadPluginConfigurationReturns) error {
+func (s *apiRPCServer) LoadPluginConfiguration(args *Z_LoadPluginConfigurationArgsArgs, returns *Z_LoadPluginConfigurationArgsReturns) error {
 	var config interface{}
 	if hook, ok := s.impl.(interface {
 		LoadPluginConfiguration(dest interface{}) error
@@ -220,17 +220,17 @@ func (s *APIRPCServer) LoadPluginConfiguration(args *LoadPluginConfigurationArgs
 }
 
 func init() {
-	HookNameToId["ServeHTTP"] = ServeHTTPId
+	hookNameToId["ServeHTTP"] = ServeHTTPId
 }
 
-type ServeHTTPArgs struct {
+type Z_ServeHTTPArgs struct {
 	ResponseWriterStream uint32
 	Request              *http.Request
 	Context              *Context
 	RequestBodyStream    uint32
 }
 
-func (g *HooksRPCClient) ServeHTTP(c *Context, w http.ResponseWriter, r *http.Request) {
+func (g *hooksRPCClient) ServeHTTP(c *Context, w http.ResponseWriter, r *http.Request) {
 	if !g.implemented[ServeHTTPId] {
 		http.NotFound(w, r)
 		return
@@ -247,7 +247,7 @@ func (g *HooksRPCClient) ServeHTTP(c *Context, w http.ResponseWriter, r *http.Re
 		defer connection.Close()
 
 		rpcServer := rpc.NewServer()
-		if err := rpcServer.RegisterName("Plugin", &HTTPResponseWriterRPCServer{w: w}); err != nil {
+		if err := rpcServer.RegisterName("Plugin", &httpResponseWriterRPCServer{w: w}); err != nil {
 			g.log.Error("Plugin failed to ServeHTTP, coulden't register RPC name", mlog.Err(err))
 			http.Error(w, "500 internal server error", http.StatusInternalServerError)
 			return
@@ -266,7 +266,7 @@ func (g *HooksRPCClient) ServeHTTP(c *Context, w http.ResponseWriter, r *http.Re
 				return
 			}
 			defer bodyConnection.Close()
-			ServeIOReader(r.Body, bodyConnection)
+			serveIOReader(r.Body, bodyConnection)
 		}()
 	}
 
@@ -282,7 +282,7 @@ func (g *HooksRPCClient) ServeHTTP(c *Context, w http.ResponseWriter, r *http.Re
 		RequestURI: r.RequestURI,
 	}
 
-	if err := g.client.Call("Plugin.ServeHTTP", ServeHTTPArgs{
+	if err := g.client.Call("Plugin.ServeHTTP", Z_ServeHTTPArgs{
 		Context:              c,
 		ResponseWriterStream: serveHTTPStreamId,
 		Request:              forwardedRequest,
@@ -294,13 +294,13 @@ func (g *HooksRPCClient) ServeHTTP(c *Context, w http.ResponseWriter, r *http.Re
 	return
 }
 
-func (s *HooksRPCServer) ServeHTTP(args *ServeHTTPArgs, returns *struct{}) error {
+func (s *hooksRPCServer) ServeHTTP(args *Z_ServeHTTPArgs, returns *struct{}) error {
 	connection, err := s.muxBroker.Dial(args.ResponseWriterStream)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "[ERROR] Can't connect to remote response writer stream, error: %v", err.Error())
 		return err
 	}
-	w := ConnectHTTPResponseWriter(connection)
+	w := connectHTTPResponseWriter(connection)
 	defer w.Close()
 
 	r := args.Request
@@ -310,7 +310,7 @@ func (s *HooksRPCServer) ServeHTTP(args *ServeHTTPArgs, returns *struct{}) error
 			fmt.Fprintf(os.Stderr, "[ERROR] Can't connect to remote request body stream, error: %v", err.Error())
 			return err
 		}
-		r.Body = ConnectIOReader(connection)
+		r.Body = connectIOReader(connection)
 	} else {
 		r.Body = ioutil.NopCloser(&bytes.Buffer{})
 	}

plugin/context.go

@@ -3,9 +3,8 @@
 
 package plugin
 
+// Context passes through metadata about the request or hook event.
+//
+// It is currently a placeholder while the implementation details are sorted out.
 type Context struct {
 }
-
-func NewBlankContext() *Context {
-	return &Context{}
-}

plugin/doc.go

@@ -0,0 +1,9 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+// The plugin package is used by Mattermost server plugins written in go. It also enables the
+// Mattermost server to manage and interact with the running plugin environment.
+//
+// Note that this package exports a large number of types prefixed with Z_. These are public only
+// to allow their use with Hashicorp's go-plugin (and net/rpc). Do not use these directly.
+package plugin

plugin/environment.go

@@ -14,31 +14,37 @@ import (
 	"github.com/pkg/errors"
 )
 
-type APIImplCreatorFunc func(*model.Manifest) API
+type apiImplCreatorFunc func(*model.Manifest) API
-type SupervisorCreatorFunc func(*model.BundleInfo, *mlog.Logger, API) (*Supervisor, error)
+type supervisorCreatorFunc func(*model.BundleInfo, *mlog.Logger, API) (*supervisor, error)
 
-// Hooks will be the hooks API for the plugin
+// multiPluginHookRunnerFunc is a callback function to invoke as part of RunMultiPluginHook.
-// Return value should be true if we should continue calling more plugins
+//
-type MultliPluginHookRunnerFunc func(hooks Hooks) bool
+// Return false to stop the hook from iterating to subsequent plugins.
+type multiPluginHookRunnerFunc func(hooks Hooks) bool
 
-type ActivePlugin struct {
+type activePlugin struct {
 	BundleInfo *model.BundleInfo
 	State      int
-	Supervisor *Supervisor
+
+	supervisor *supervisor
 }
 
+// Environment represents the execution environment of active plugins.
+//
+// It is meant for use by the Mattermost server to manipulate, interact with and report on the set
+// of active plugins.
 type Environment struct {
-	activePlugins   map[string]ActivePlugin
+	activePlugins   map[string]activePlugin
 	mutex           sync.RWMutex
 	logger          *mlog.Logger
-	newAPIImpl      APIImplCreatorFunc
+	newAPIImpl      apiImplCreatorFunc
 	pluginDir       string
 	webappPluginDir string
 }
 
-func NewEnvironment(newAPIImpl APIImplCreatorFunc, pluginDir string, webappPluginDir string, logger *mlog.Logger) (*Environment, error) {
+func NewEnvironment(newAPIImpl apiImplCreatorFunc, pluginDir string, webappPluginDir string, logger *mlog.Logger) (*Environment, error) {
 	return &Environment{
-		activePlugins:   make(map[string]ActivePlugin),
+		activePlugins:   make(map[string]activePlugin),
 		logger:          logger,
 		newAPIImpl:      newAPIImpl,
 		pluginDir:       pluginDir,
@@ -53,7 +59,7 @@ func NewEnvironment(newAPIImpl APIImplCreatorFunc, pluginDir string, webappPlugi
 // parsed).
 //
 // Plugins are found non-recursively and paths beginning with a dot are always ignored.
-func ScanSearchPath(path string) ([]*model.BundleInfo, error) {
+func scanSearchPath(path string) ([]*model.BundleInfo, error) {
 	files, err := ioutil.ReadDir(path)
 	if err != nil {
 		return nil, err
@@ -72,7 +78,7 @@ func ScanSearchPath(path string) ([]*model.BundleInfo, error) {
 
 // Returns a list of all plugins within the environment.
 func (env *Environment) Available() ([]*model.BundleInfo, error) {
-	return ScanSearchPath(env.pluginDir)
+	return scanSearchPath(env.pluginDir)
 }
 
 // Returns a list of all currently active plugins within the environment.
@@ -88,12 +94,13 @@ func (env *Environment) Active() []*model.BundleInfo {
 	return activePlugins
 }
 
+// IsActive returns true if the plugin with the given id is active.
 func (env *Environment) IsActive(id string) bool {
 	_, ok := env.activePlugins[id]
 	return ok
 }
 
-// Returns a list of plugin statuses reprensenting the state of every plugin
+// Statuses returns a list of plugin statuses representing the state of every plugin
 func (env *Environment) Statuses() (model.PluginStatuses, error) {
 	env.mutex.RLock()
 	defer env.mutex.RUnlock()
@@ -130,6 +137,7 @@ func (env *Environment) Statuses() (model.PluginStatuses, error) {
 	return pluginStatuses, nil
 }
 
+// Activate activates the plugin with the given id.
 func (env *Environment) Activate(id string) (reterr error) {
 	env.mutex.Lock()
 	defer env.mutex.Unlock()
@@ -156,7 +164,7 @@ func (env *Environment) Activate(id string) (reterr error) {
 		return fmt.Errorf("plugin not found: %v", id)
 	}
 
-	activePlugin := ActivePlugin{BundleInfo: pluginInfo}
+	activePlugin := activePlugin{BundleInfo: pluginInfo}
 	defer func() {
 		if reterr == nil {
 			activePlugin.State = model.PluginStateRunning
@@ -185,11 +193,11 @@ func (env *Environment) Activate(id string) (reterr error) {
 	}
 
 	if pluginInfo.Manifest.Backend != nil {
-		supervisor, err := NewSupervisor(pluginInfo, env.logger, env.newAPIImpl(pluginInfo.Manifest))
+		supervisor, err := newSupervisor(pluginInfo, env.logger, env.newAPIImpl(pluginInfo.Manifest))
 		if err != nil {
 			return errors.Wrapf(err, "unable to start plugin: %v", id)
 		}
-		activePlugin.Supervisor = supervisor
+		activePlugin.supervisor = supervisor
 	}
 
 	return nil
@@ -204,56 +212,59 @@ func (env *Environment) Deactivate(id string) {
 		return
 	} else {
 		delete(env.activePlugins, id)
-		if activePlugin.Supervisor != nil {
+		if activePlugin.supervisor != nil {
-			if err := activePlugin.Supervisor.Hooks().OnDeactivate(); err != nil {
+			if err := activePlugin.supervisor.Hooks().OnDeactivate(); err != nil {
 				env.logger.Error("Plugin OnDeactivate() error", mlog.String("plugin_id", activePlugin.BundleInfo.Manifest.Id), mlog.Err(err))
 			}
-			activePlugin.Supervisor.Shutdown()
+			activePlugin.supervisor.Shutdown()
 		}
 	}
 }
 
-// Deactivates all plugins and gracefully shuts down the environment.
+// Shutdown deactivates all plugins and gracefully shuts down the environment.
 func (env *Environment) Shutdown() {
 	env.mutex.Lock()
 	defer env.mutex.Unlock()
 
 	for _, activePlugin := range env.activePlugins {
-		if activePlugin.Supervisor != nil {
+		if activePlugin.supervisor != nil {
-			if err := activePlugin.Supervisor.Hooks().OnDeactivate(); err != nil {
+			if err := activePlugin.supervisor.Hooks().OnDeactivate(); err != nil {
 				env.logger.Error("Plugin OnDeactivate() error", mlog.String("plugin_id", activePlugin.BundleInfo.Manifest.Id), mlog.Err(err))
 			}
-			activePlugin.Supervisor.Shutdown()
+			activePlugin.supervisor.Shutdown()
 		}
 	}
-	env.activePlugins = make(map[string]ActivePlugin)
+	env.activePlugins = make(map[string]activePlugin)
 	return
 }
 
-// Returns the hooks API for the plugin ID specified
+// HooksForPlugin returns the hooks API for the plugin with the given id.
-// You should probably use RunMultiPluginHook instead.
+//
+// Consider using RunMultiPluginHook instead.
 func (env *Environment) HooksForPlugin(id string) (Hooks, error) {
 	env.mutex.RLock()
 	defer env.mutex.RUnlock()
 
-	if plug, ok := env.activePlugins[id]; ok && plug.Supervisor != nil {
+	if plug, ok := env.activePlugins[id]; ok && plug.supervisor != nil {
-		return plug.Supervisor.Hooks(), nil
+		return plug.supervisor.Hooks(), nil
 	}
 
 	return nil, fmt.Errorf("plugin not found: %v", id)
 }
 
-// Calls hookRunnerFunc with the hooks for each active plugin that implments the given HookId
+// RunMultiPluginHook invokes hookRunnerFunc for each plugin that implements the given hookId.
-// If hookRunnerFunc returns false, then iteration will not continue.
+//
-func (env *Environment) RunMultiPluginHook(hookRunnerFunc MultliPluginHookRunnerFunc, mustImplement int) {
+// If hookRunnerFunc returns false, iteration will not continue. The iteration order among active
+// plugins is not specified.
+func (env *Environment) RunMultiPluginHook(hookRunnerFunc multiPluginHookRunnerFunc, hookId int) {
 	env.mutex.RLock()
 	defer env.mutex.RUnlock()
 
 	for _, activePlugin := range env.activePlugins {
-		if activePlugin.Supervisor == nil || !activePlugin.Supervisor.Implements(mustImplement) {
+		if activePlugin.supervisor == nil || !activePlugin.supervisor.Implements(hookId) {
 			continue
 		}
-		if !hookRunnerFunc(activePlugin.Supervisor.Hooks()) {
+		if !hookRunnerFunc(activePlugin.supervisor.Hooks()) {
 			break
 		}
 	}

plugin/example_hello_world_test.go

@@ -0,0 +1,20 @@
+package plugin_test
+
+import (
+	"fmt"
+	"net/http"
+
+	"github.com/mattermost/mattermost-server/plugin"
+)
+
+type HelloWorldPlugin struct{}
+
+func (p *HelloWorldPlugin) ServeHTTP(c *plugin.Context, w http.ResponseWriter, r *http.Request) {
+	fmt.Fprintf(w, "Hello, world!")
+}
+
+// This example demonstrates a plugin that handles HTTP requests which respond by greeting the
+// world.
+func Example_helloWorld() {
+	plugin.ClientMain(&HelloWorldPlugin{})
+}

plugin/example_help_test.go

@@ -0,0 +1,71 @@
+package plugin_test
+
+import (
+	"strings"
+
+	"github.com/mattermost/mattermost-server/model"
+	"github.com/mattermost/mattermost-server/plugin"
+)
+
+type HelpPlugin struct {
+	plugin.MattermostPlugin
+
+	TeamName    string
+	ChannelName string
+
+	channelId string
+}
+
+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
+	}
+
+	team, err := p.API.GetTeamByName(p.TeamName)
+	if err != nil {
+		p.API.LogError("failed to find team", "team_name", p.TeamName)
+		return nil
+	}
+
+	channel, err := p.API.GetChannelByName(p.ChannelName, team.Id)
+	if err != nil {
+		p.API.LogError("failed to find channel", "channel_name", p.ChannelName)
+		return nil
+	}
+
+	p.channelId = channel.Id
+
+	return nil
+}
+
+func (p *HelpPlugin) MessageHasBeenPosted(c *plugin.Context, post *model.Post) {
+	// Ignore posts not in the configured channel
+	if post.ChannelId != p.channelId {
+		return
+	}
+
+	// Ignore posts this plugin made.
+	if sentByPlugin, _ := post.Props["sent_by_plugin"].(bool); sentByPlugin {
+		return
+	}
+
+	// Ignore posts without a plea for help.
+	if !strings.Contains(post.Message, "help") {
+		return
+	}
+
+	p.API.SendEphemeralPost(post.UserId, &model.Post{
+		ChannelId: p.channelId,
+		Message:   "You asked for help? Checkout https://about.mattermost.com/help/",
+		Props: map[string]interface{}{
+			"sent_by_plugin": true,
+		},
+	})
+}
+
+func Example_helpPlugin() {
+	plugin.ClientMain(&HelpPlugin{})
+}

plugin/hclog_adapter.go

@@ -12,12 +12,12 @@ import (
 	"github.com/mattermost/mattermost-server/mlog"
 )
 
-type HclogAdapter struct {
+type hclogAdapter struct {
 	wrappedLogger *mlog.Logger
 	extrasKey     string
 }
 
-func (h *HclogAdapter) Trace(msg string, args ...interface{}) {
+func (h *hclogAdapter) Trace(msg string, args ...interface{}) {
 	extras := strings.TrimSpace(fmt.Sprint(args...))
 	if extras != "" {
 		h.wrappedLogger.Debug(msg, mlog.String(h.extrasKey, extras))
@@ -26,7 +26,7 @@ func (h *HclogAdapter) Trace(msg string, args ...interface{}) {
 	}
 }
 
-func (h *HclogAdapter) Debug(msg string, args ...interface{}) {
+func (h *hclogAdapter) Debug(msg string, args ...interface{}) {
 	extras := strings.TrimSpace(fmt.Sprint(args...))
 	if extras != "" {
 		h.wrappedLogger.Debug(msg, mlog.String(h.extrasKey, extras))
@@ -35,7 +35,7 @@ func (h *HclogAdapter) Debug(msg string, args ...interface{}) {
 	}
 }
 
-func (h *HclogAdapter) Info(msg string, args ...interface{}) {
+func (h *hclogAdapter) Info(msg string, args ...interface{}) {
 	extras := strings.TrimSpace(fmt.Sprint(args...))
 	if extras != "" {
 		h.wrappedLogger.Info(msg, mlog.String(h.extrasKey, extras))
@@ -44,7 +44,7 @@ func (h *HclogAdapter) Info(msg string, args ...interface{}) {
 	}
 }
 
-func (h *HclogAdapter) Warn(msg string, args ...interface{}) {
+func (h *hclogAdapter) Warn(msg string, args ...interface{}) {
 	extras := strings.TrimSpace(fmt.Sprint(args...))
 	if extras != "" {
 		h.wrappedLogger.Warn(msg, mlog.String(h.extrasKey, extras))
@@ -53,7 +53,7 @@ func (h *HclogAdapter) Warn(msg string, args ...interface{}) {
 	}
 }
 
-func (h *HclogAdapter) Error(msg string, args ...interface{}) {
+func (h *hclogAdapter) Error(msg string, args ...interface{}) {
 	extras := strings.TrimSpace(fmt.Sprint(args...))
 	if extras != "" {
 		h.wrappedLogger.Error(msg, mlog.String(h.extrasKey, extras))
@@ -62,38 +62,38 @@ func (h *HclogAdapter) Error(msg string, args ...interface{}) {
 	}
 }
 
-func (h *HclogAdapter) IsTrace() bool {
+func (h *hclogAdapter) IsTrace() bool {
 	return false
 }
 
-func (h *HclogAdapter) IsDebug() bool {
+func (h *hclogAdapter) IsDebug() bool {
 	return true
 }
 
-func (h *HclogAdapter) IsInfo() bool {
+func (h *hclogAdapter) IsInfo() bool {
 	return true
 }
 
-func (h *HclogAdapter) IsWarn() bool {
+func (h *hclogAdapter) IsWarn() bool {
 	return true
 }
 
-func (h *HclogAdapter) IsError() bool {
+func (h *hclogAdapter) IsError() bool {
 	return true
 }
 
-func (h *HclogAdapter) With(args ...interface{}) hclog.Logger {
+func (h *hclogAdapter) With(args ...interface{}) hclog.Logger {
 	return h
 }
 
-func (h *HclogAdapter) Named(name string) hclog.Logger {
+func (h *hclogAdapter) Named(name string) hclog.Logger {
 	return h
 }
 
-func (h *HclogAdapter) ResetNamed(name string) hclog.Logger {
+func (h *hclogAdapter) ResetNamed(name string) hclog.Logger {
 	return h
 }
 
-func (h *HclogAdapter) StandardLogger(opts *hclog.StandardLoggerOptions) *log.Logger {
+func (h *hclogAdapter) StandardLogger(opts *hclog.StandardLoggerOptions) *log.Logger {
 	return h.wrappedLogger.StdLog()
 }

plugin/hooks.go

@@ -9,8 +9,10 @@ import (
 	"github.com/mattermost/mattermost-server/model"
 )
 
-// These assignments are part of the wire protocol. You can add more, but should not change existing
+// These assignments are part of the wire protocol used to trigger hook events in plugins.
-// assignments. Follow the naming convention of <HookName>Id as the autogenerated glue code depends on that.
+//
+// Feel free to add more, but do not change existing assignments. Follow the naming convention of
+// <HookName>Id as the autogenerated glue code depends on that.
 const (
 	OnActivateId            = 0
 	OnDeactivateId          = 1
@@ -29,15 +31,16 @@ const (
 	TotalHooksId            = iota
 )
 
-// Methods from the Hooks interface can be used by a plugin to respond to events. Methods are likely
+// Hooks describes the methods a plugin may implement to automatically receive the corresponding
-// to be added over time, and plugins are not expected to implement all of them. Instead, plugins
+// event.
-// are expected to implement a subset of them and pass an instance to plugin/rpcplugin.Main, which
+//
-// will take over execution of the process and add default behaviors for missing hooks.
+// A plugin only need implement the hooks it cares about. The MattermostPlugin provides some
+// default implementations for convenience but may be overridden.
 type Hooks interface {
 	// OnActivate is invoked when the plugin is activated.
 	OnActivate() error
 
-	// Implemented returns a list of hooks that are implmented by the plugin.
+	// Implemented returns a list of hooks that are implemented by the plugin.
 	// Plugins do not need to provide an implementation. Any given will be ignored.
 	Implemented() ([]string, error)
 

plugin/http.go

@@ -9,26 +9,26 @@ import (
 	"net/rpc"
 )
 
-type HTTPResponseWriterRPCServer struct {
+type httpResponseWriterRPCServer struct {
 	w http.ResponseWriter
 }
 
-func (w *HTTPResponseWriterRPCServer) Header(args struct{}, reply *http.Header) error {
+func (w *httpResponseWriterRPCServer) Header(args struct{}, reply *http.Header) error {
 	*reply = w.w.Header()
 	return nil
 }
 
-func (w *HTTPResponseWriterRPCServer) Write(args []byte, reply *struct{}) error {
+func (w *httpResponseWriterRPCServer) Write(args []byte, reply *struct{}) error {
 	_, err := w.w.Write(args)
 	return err
 }
 
-func (w *HTTPResponseWriterRPCServer) WriteHeader(args int, reply *struct{}) error {
+func (w *httpResponseWriterRPCServer) WriteHeader(args int, reply *struct{}) error {
 	w.w.WriteHeader(args)
 	return nil
 }
 
-func (w *HTTPResponseWriterRPCServer) SyncHeader(args http.Header, reply *struct{}) error {
+func (w *httpResponseWriterRPCServer) SyncHeader(args http.Header, reply *struct{}) error {
 	dest := w.w.Header()
 	for k := range dest {
 		if _, ok := args[k]; !ok {
@@ -41,29 +41,21 @@ func (w *HTTPResponseWriterRPCServer) SyncHeader(args http.Header, reply *struct
 	return nil
 }
 
-func ServeHTTPResponseWriter(w http.ResponseWriter, conn io.ReadWriteCloser) {
+type httpResponseWriterRPCClient struct {
-	server := rpc.NewServer()
-	server.Register(&HTTPResponseWriterRPCServer{
-		w: w,
-	})
-	server.ServeConn(conn)
-}
-
-type HTTPResponseWriterRPCClient struct {
 	client *rpc.Client
 	header http.Header
 }
 
-var _ http.ResponseWriter = (*HTTPResponseWriterRPCClient)(nil)
+var _ http.ResponseWriter = (*httpResponseWriterRPCClient)(nil)
 
-func (w *HTTPResponseWriterRPCClient) Header() http.Header {
+func (w *httpResponseWriterRPCClient) Header() http.Header {
 	if w.header == nil {
 		w.client.Call("Plugin.Header", struct{}{}, &w.header)
 	}
 	return w.header
 }
 
-func (w *HTTPResponseWriterRPCClient) Write(b []byte) (int, error) {
+func (w *httpResponseWriterRPCClient) Write(b []byte) (int, error) {
 	if err := w.client.Call("Plugin.SyncHeader", w.header, nil); err != nil {
 		return 0, err
 	}
@@ -73,19 +65,19 @@ func (w *HTTPResponseWriterRPCClient) Write(b []byte) (int, error) {
 	return len(b), nil
 }
 
-func (w *HTTPResponseWriterRPCClient) WriteHeader(statusCode int) {
+func (w *httpResponseWriterRPCClient) WriteHeader(statusCode int) {
 	if err := w.client.Call("Plugin.SyncHeader", w.header, nil); err != nil {
 		return
 	}
 	w.client.Call("Plugin.WriteHeader", statusCode, nil)
 }
 
-func (h *HTTPResponseWriterRPCClient) Close() error {
+func (h *httpResponseWriterRPCClient) Close() error {
 	return h.client.Close()
 }
 
-func ConnectHTTPResponseWriter(conn io.ReadWriteCloser) *HTTPResponseWriterRPCClient {
+func connectHTTPResponseWriter(conn io.ReadWriteCloser) *httpResponseWriterRPCClient {
-	return &HTTPResponseWriterRPCClient{
+	return &httpResponseWriterRPCClient{
 		client: rpc.NewClient(conn),
 	}
 }

plugin/interface_generator/main.go

@@ -210,20 +210,20 @@ package plugin
 {{range .HooksMethods}}
 
 func init() {
-	HookNameToId["{{.Name}}"] = {{.Name}}Id
+	hookNameToId["{{.Name}}"] = {{.Name}}Id
 }
 
-type {{.Name}}Args struct {
+type {{.Name | obscure}}Args struct {
 	{{structStyle .Params}}
 }
 
-type {{.Name}}Returns struct {
+type {{.Name | obscure}}Returns struct {
 	{{structStyle .Return}}
 }
 
-func (g *HooksRPCClient) {{.Name}}{{funcStyle .Params}} {{funcStyle .Return}} {
+func (g *hooksRPCClient) {{.Name}}{{funcStyle .Params}} {{funcStyle .Return}} {
-	_args := &{{.Name}}Args{ {{valuesOnly .Params}} }
+	_args := &{{.Name | obscure}}Args{ {{valuesOnly .Params}} }
-	_returns := &{{.Name}}Returns{}
+	_returns := &{{.Name | obscure}}Returns{}
 	if g.implemented[{{.Name}}Id] {
 		if err := g.client.Call("Plugin.{{.Name}}", _args, _returns); err != nil {
 			g.log.Error("RPC call {{.Name}} to plugin failed.", mlog.Err(err))
@@ -232,7 +232,7 @@ func (g *HooksRPCClient) {{.Name}}{{funcStyle .Params}} {{funcStyle .Return}} {
 	return {{destruct "_returns." .Return}}
 }
 
-func (s *HooksRPCServer) {{.Name}}(args *{{.Name}}Args, returns *{{.Name}}Returns) error {
+func (s *hooksRPCServer) {{.Name}}(args *{{.Name | obscure}}Args, returns *{{.Name | obscure}}Returns) error {
 	if hook, ok := s.impl.(interface {
 		{{.Name}}{{funcStyle .Params}} {{funcStyle .Return}}
 	}); ok {
@@ -246,24 +246,24 @@ func (s *HooksRPCServer) {{.Name}}(args *{{.Name}}Args, returns *{{.Name}}Return
 
 {{range .APIMethods}}
 
-type {{.Name}}Args struct {
+type {{.Name | obscure}}Args struct {
 	{{structStyle .Params}}
 }
 
-type {{.Name}}Returns struct {
+type {{.Name | obscure}}Returns struct {
 	{{structStyle .Return}}
 }
 
-func (g *APIRPCClient) {{.Name}}{{funcStyle .Params}} {{funcStyle .Return}} {
+func (g *apiRPCClient) {{.Name}}{{funcStyle .Params}} {{funcStyle .Return}} {
-	_args := &{{.Name}}Args{ {{valuesOnly .Params}} }
+	_args := &{{.Name | obscure}}Args{ {{valuesOnly .Params}} }
-	_returns := &{{.Name}}Returns{}
+	_returns := &{{.Name | obscure}}Returns{}
 	if err := g.client.Call("Plugin.{{.Name}}", _args, _returns); err != nil {
 		g.log.Error("RPC call to {{.Name}} API failed.", mlog.Err(err))
 	}
 	return {{destruct "_returns." .Return}}
 }
 
-func (s *APIRPCServer) {{.Name}}(args *{{.Name}}Args, returns *{{.Name}}Returns) error {
+func (s *apiRPCServer) {{.Name}}(args *{{.Name | obscure}}Args, returns *{{.Name | obscure}}Returns) error {
 	if hook, ok := s.impl.(interface {
 		{{.Name}}{{funcStyle .Params}} {{funcStyle .Return}}
 	}); ok {
@@ -295,6 +295,9 @@ func generateGlue(info *PluginInterfaceInfo) {
 		"destruct": func(structPrefix string, fields *ast.FieldList) string {
 			return FieldListDestruct(structPrefix, fields, info.FileSet)
 		},
+		"obscure": func(name string) string {
+			return "Z_" + name
+		},
 	}
 
 	hooksTemplate, err := template.New("hooks").Funcs(templateFunctions).Parse(hooksTemplate)

plugin/io_rpc.go

@@ -22,15 +22,11 @@ func (rwc *rwc) Close() (err error) {
 	return
 }
 
-func NewReadWriteCloser(r io.ReadCloser, w io.WriteCloser) io.ReadWriteCloser {
+type remoteIOReader struct {
-	return &rwc{r, w}
-}
-
-type RemoteIOReader struct {
 	conn io.ReadWriteCloser
 }
 
-func (r *RemoteIOReader) Read(b []byte) (int, error) {
+func (r *remoteIOReader) Read(b []byte) (int, error) {
 	var buf [10]byte
 	n := binary.PutVarint(buf[:], int64(len(b)))
 	if _, err := r.conn.Write(buf[:n]); err != nil {
@@ -39,15 +35,15 @@ func (r *RemoteIOReader) Read(b []byte) (int, error) {
 	return r.conn.Read(b)
 }
 
-func (r *RemoteIOReader) Close() error {
+func (r *remoteIOReader) Close() error {
 	return r.conn.Close()
 }
 
-func ConnectIOReader(conn io.ReadWriteCloser) io.ReadCloser {
+func connectIOReader(conn io.ReadWriteCloser) io.ReadCloser {
-	return &RemoteIOReader{conn}
+	return &remoteIOReader{conn}
 }
 
-func ServeIOReader(r io.Reader, conn io.ReadWriteCloser) {
+func serveIOReader(r io.Reader, conn io.ReadWriteCloser) {
 	cr := bufio.NewReader(conn)
 	defer conn.Close()
 	buf := make([]byte, 32*1024)

plugin/supervisor.go

@@ -15,25 +15,25 @@ import (
 	"github.com/mattermost/mattermost-server/model"
 )
 
-type Supervisor struct {
+type supervisor struct {
 	pluginId    string
 	client      *plugin.Client
 	hooks       Hooks
 	implemented [TotalHooksId]bool
 }
 
-func NewSupervisor(pluginInfo *model.BundleInfo, parentLogger *mlog.Logger, apiImpl API) (*Supervisor, error) {
+func newSupervisor(pluginInfo *model.BundleInfo, parentLogger *mlog.Logger, apiImpl API) (*supervisor, error) {
-	supervisor := Supervisor{}
+	supervisor := supervisor{}
 
 	wrappedLogger := pluginInfo.WrapLogger(parentLogger)
 
-	hclogAdaptedLogger := &HclogAdapter{
+	hclogAdaptedLogger := &hclogAdapter{
 		wrappedLogger: wrappedLogger.WithCallerSkip(1),
 		extrasKey:     "wrapped_extras",
 	}
 
 	pluginMap := map[string]plugin.Plugin{
-		"hooks": &HooksPlugin{
+		"hooks": &hooksPlugin{
 			log:     wrappedLogger,
 			apiImpl: apiImpl,
 		},
@@ -46,7 +46,7 @@ func NewSupervisor(pluginInfo *model.BundleInfo, parentLogger *mlog.Logger, apiI
 	executable = filepath.Join(pluginInfo.Path, executable)
 
 	supervisor.client = plugin.NewClient(&plugin.ClientConfig{
-		HandshakeConfig: Handshake,
+		HandshakeConfig: handshake,
 		Plugins:         pluginMap,
 		Cmd:             exec.Command(executable),
 		SyncStdout:      wrappedLogger.With(mlog.String("source", "plugin_stdout")).StdLogWriter(),
@@ -71,7 +71,7 @@ func NewSupervisor(pluginInfo *model.BundleInfo, parentLogger *mlog.Logger, apiI
 		return nil, err
 	} else {
 		for _, hookName := range impl {
-			if hookId, ok := HookNameToId[hookName]; ok {
+			if hookId, ok := hookNameToId[hookName]; ok {
 				supervisor.implemented[hookId] = true
 			}
 		}
@@ -85,14 +85,14 @@ func NewSupervisor(pluginInfo *model.BundleInfo, parentLogger *mlog.Logger, apiI
 	return &supervisor, nil
 }
 
-func (sup *Supervisor) Shutdown() {
+func (sup *supervisor) Shutdown() {
 	sup.client.Kill()
 }
 
-func (sup *Supervisor) Hooks() Hooks {
+func (sup *supervisor) Hooks() Hooks {
 	return sup.hooks
 }
 
-func (sup *Supervisor) Implements(hookId int) bool {
+func (sup *supervisor) Implements(hookId int) bool {
 	return sup.implemented[hookId]
 }

plugin/supervisor_test.go

@@ -73,7 +73,7 @@ func testSupervisor(t *testing.T) {
 		ConsoleLevel:  "error",
 		EnableFile:    false,
 	})
-	supervisor, err := NewSupervisor(bundle, log, &api)
+	supervisor, err := newSupervisor(bundle, log, &api)
 	require.NoError(t, err)
 	supervisor.Shutdown()
 }
@@ -92,7 +92,7 @@ func testSupervisor_InvalidExecutablePath(t *testing.T) {
 		ConsoleLevel:  "error",
 		EnableFile:    false,
 	})
-	supervisor, err := NewSupervisor(bundle, log, nil)
+	supervisor, err := newSupervisor(bundle, log, nil)
 	assert.Nil(t, supervisor)
 	assert.Error(t, err)
 }
@@ -111,7 +111,7 @@ func testSupervisor_NonExistentExecutablePath(t *testing.T) {
 		ConsoleLevel:  "error",
 		EnableFile:    false,
 	})
-	supervisor, err := NewSupervisor(bundle, log, nil)
+	supervisor, err := newSupervisor(bundle, log, nil)
 	require.Error(t, err)
 	require.Nil(t, supervisor)
 }
@@ -141,7 +141,7 @@ func testSupervisor_StartTimeout(t *testing.T) {
 		ConsoleLevel:  "error",
 		EnableFile:    false,
 	})
-	supervisor, err := NewSupervisor(bundle, log, nil)
+	supervisor, err := newSupervisor(bundle, log, nil)
 	require.Error(t, err)
 	require.Nil(t, supervisor)
 }

plugin/valid.go

@@ -11,14 +11,21 @@ import (
 const (
 	MinIdLength  = 3
 	MaxIdLength  = 190
+	ValidIdRegex = `^[a-zA-Z0-9-_\.]+$`
 )
 
-var ValidId *regexp.Regexp
+// ValidId constrains the set of valid plugin identifiers:
+//  ^[a-zA-Z0-9-_\.]+
+var validId *regexp.Regexp
 
 func init() {
-	ValidId = regexp.MustCompile(`^[a-zA-Z0-9-_\.]+$`)
+	validId = regexp.MustCompile(ValidIdRegex)
 }
 
+// IsValidId verifies that the plugin id has a minimum length of 3, maximum length of 190, and
+// contains only alphanumeric characters, dashes, underscores and periods.
+//
+// These constraints are necessary since the plugin id is used as part of a filesystem path.
 func IsValidId(id string) bool {
 	if utf8.RuneCountInString(id) < MinIdLength {
 		return false
@@ -28,5 +35,5 @@ func IsValidId(id string) bool {
 		return false
 	}
 
-	return ValidId.MatchString(id)
+	return validId.MatchString(id)
 }