Skip to content

Commit

Permalink
Remove usage of model.AppError (mattermost#364)
Browse files Browse the repository at this point in the history
* Removed usage of model.AppError
  • Loading branch information
srkgupta authored Apr 17, 2023
1 parent deac043 commit de004a9
Show file tree
Hide file tree
Showing 12 changed files with 134 additions and 122 deletions.
41 changes: 21 additions & 20 deletions server/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func (p *Plugin) withRecovery(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
defer func() {
if x := recover(); x != nil {
p.API.LogWarn("Recovered from a panic",
p.client.Log.Warn("Recovered from a panic",
"url", r.URL.String(),
"error", x,
"stack", string(debug.Stack()))
Expand Down Expand Up @@ -171,7 +171,7 @@ func (p *Plugin) checkAuth(handler http.HandlerFunc, responseType ResponseType)
case ResponseTypePlain:
http.Error(w, "Not authorized", http.StatusUnauthorized)
default:
p.API.LogDebug("Unknown ResponseType detected")
p.client.Log.Debug("Unknown ResponseType detected")
}
return
}
Expand Down Expand Up @@ -204,18 +204,18 @@ func (p *Plugin) writeAPIError(w http.ResponseWriter, err *APIErrorResponse) {
b, _ := json.Marshal(err)
w.WriteHeader(err.StatusCode)
if _, err := w.Write(b); err != nil {
p.API.LogWarn("can't write api error http response", "err", err.Error())
p.client.Log.Warn("can't write api error http response", "err", err.Error())
}
}

func (p *Plugin) writeAPIResponse(w http.ResponseWriter, resp interface{}) {
b, jsonErr := json.Marshal(resp)
if jsonErr != nil {
p.API.LogWarn("Error encoding JSON response", "err", jsonErr.Error())
p.client.Log.Warn("Error encoding JSON response", "err", jsonErr.Error())
p.writeAPIError(w, &APIErrorResponse{ID: "", Message: "Encountered an unexpected error. Please try again.", StatusCode: http.StatusInternalServerError})
}
if _, err := w.Write(b); err != nil {
p.API.LogWarn("can't write response user to http", "err", err.Error())
p.client.Log.Warn("can't write response user to http", "err", err.Error())
p.writeAPIError(w, &APIErrorResponse{ID: "", Message: "Encountered an unexpected error. Please try again.", StatusCode: http.StatusInternalServerError})
}
}
Expand All @@ -231,7 +231,7 @@ func (p *Plugin) connectUserToGitlab(c *Context, w http.ResponseWriter, r *http.

state := fmt.Sprintf("%v_%v", model.NewId()[0:15], userID)

if err := p.API.KVSet(state, []byte(state)); err != nil {
if _, err := p.client.KV.Set(state, []byte(state)); err != nil {
c.Log.WithError(err).Warnf("Can't store state oauth2")
http.Error(w, "can't store state oauth2", http.StatusInternalServerError)
return
Expand Down Expand Up @@ -296,20 +296,21 @@ func (p *Plugin) completeConnectUserToGitlab(c *Context, w http.ResponseWriter,

state := r.URL.Query().Get("state")

storedState, appErr := p.API.KVGet(state)
if appErr != nil {
c.Log.WithError(appErr).Warnf("Can't get state from store")
var storedState []byte
err := p.client.KV.Get(state, &storedState)
if err != nil {
c.Log.WithError(err).Warnf("Can't get state from store")

rErr = errors.Wrap(appErr, "missing stored state")
rErr = errors.Wrap(err, "missing stored state")
http.Error(w, rErr.Error(), http.StatusBadRequest)
return
}

appErr = p.API.KVDelete(state)
if appErr != nil {
c.Log.WithError(appErr).Warnf("Failed to delete state token")
err = p.client.KV.Delete(state)
if err != nil {
c.Log.WithError(err).Warnf("Failed to delete state token")

rErr = errors.Wrap(appErr, "error deleting stored state")
rErr = errors.Wrap(err, "error deleting stored state")
http.Error(w, rErr.Error(), http.StatusBadRequest)
}

Expand Down Expand Up @@ -402,7 +403,7 @@ func (p *Plugin) completeConnectUserToGitlab(c *Context, w http.ResponseWriter,

p.TrackUserEvent("account_connected", userID, nil)

p.API.PublishWebSocketEvent(
p.client.Frontend.PublishWebSocketEvent(
WsEventConnect,
map[string]interface{}{
"connected": true,
Expand Down Expand Up @@ -589,7 +590,7 @@ func (p *Plugin) postToDo(c *UserContext, w http.ResponseWriter, r *http.Request
return
}

if appErr := p.CreateBotDMPost(c.UserID, text, "custom_git_todo"); appErr != nil {
if err := p.CreateBotDMPost(c.UserID, text, "custom_git_todo"); err != nil {
p.writeAPIError(w, &APIErrorResponse{ID: "", Message: "Encountered an error posting the to do items.", StatusCode: http.StatusUnauthorized})
}

Expand Down Expand Up @@ -656,15 +657,15 @@ func (p *Plugin) getChannelSubscriptions(c *UserContext, w http.ResponseWriter,
vars := mux.Vars(r)
channelID := vars["channel_id"]

if !p.API.HasPermissionToChannel(c.UserID, channelID, model.PermissionReadChannel) {
if !p.client.User.HasPermissionToChannel(c.UserID, channelID, model.PermissionReadChannel) {
p.writeAPIError(w, &APIErrorResponse{ID: "", Message: "Not authorized.", StatusCode: http.StatusUnauthorized})
return
}

config := p.getConfiguration()
subscriptions, err := p.GetSubscriptionsByChannel(channelID)
if err != nil {
p.API.LogWarn("unable to get subscriptions by channel", "err", err.Error())
p.client.Log.Warn("unable to get subscriptions by channel", "err", err.Error())
p.writeAPIError(w, &APIErrorResponse{ID: "", Message: "Unable to get subscriptions by channel.", StatusCode: http.StatusInternalServerError})
return
}
Expand All @@ -673,9 +674,9 @@ func (p *Plugin) getChannelSubscriptions(c *UserContext, w http.ResponseWriter,

b, err := json.Marshal(resp)
if err != nil {
p.API.LogWarn("failed to marshal channel subscriptions response", "err", err.Error())
p.client.Log.Warn("failed to marshal channel subscriptions response", "err", err.Error())
p.writeAPIError(w, &APIErrorResponse{ID: "", Message: "Encountered an unexpected error. Please try again.", StatusCode: http.StatusInternalServerError})
} else if _, err := w.Write(b); err != nil {
p.API.LogWarn("can't write api error http response", "err", err.Error())
p.client.Log.Warn("can't write api error http response", "err", err.Error())
}
}
2 changes: 2 additions & 0 deletions server/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"
"time"

pluginapi "github.com/mattermost/mattermost-plugin-api"
"github.com/mattermost/mattermost-server/v6/model"
"github.com/mattermost/mattermost-server/v6/plugin/plugintest"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -51,6 +52,7 @@ func TestGetChannelSubscriptions(t *testing.T) {

mock := &plugintest.API{}
plugin.SetAPI(mock)
plugin.client = pluginapi.NewClient(plugin.API, plugin.Driver)

mock.On("KVGet", "user_id_gitlabtoken").Return(jsonInfo, nil).Once()

Expand Down
10 changes: 5 additions & 5 deletions server/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func (p *Plugin) sendOAuthCompleteEvent(event OAuthCompleteEvent) {
func (p *Plugin) sendMessageToCluster(id string, v interface{}) {
b, err := json.Marshal(v)
if err != nil {
p.API.LogWarn("couldn't get JSON bytes from cluster message",
p.client.Log.Warn("couldn't get JSON bytes from cluster message",
"id", id,
"error", err,
)
Expand All @@ -29,8 +29,8 @@ func (p *Plugin) sendMessageToCluster(id string, v interface{}) {
SendType: model.PluginClusterEventSendTypeReliable,
}

if err := p.API.PublishPluginClusterEvent(event, opts); err != nil {
p.API.LogWarn("error publishing cluster event",
if err := p.client.Cluster.PublishPluginEvent(event, opts); err != nil {
p.client.Log.Warn("error publishing cluster event",
"id", id,
"error", err,
)
Expand All @@ -42,12 +42,12 @@ func (p *Plugin) HandleClusterEvent(ev model.PluginClusterEvent) {
case oauthCompleteEventID:
var event OAuthCompleteEvent
if err := json.Unmarshal(ev.Data, &event); err != nil {
p.API.LogWarn("cannot unmarshal cluster event with OAuth complete event", "error", err)
p.client.Log.Warn("cannot unmarshal cluster event with OAuth complete event", "error", err)
return
}

p.oauthBroker.publishOAuthComplete(event.UserID, event.Err, true)
default:
p.API.LogWarn("unknown cluster event", "id", ev.Id)
p.client.Log.Warn("unknown cluster event", "id", ev.Id)
}
}
37 changes: 18 additions & 19 deletions server/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ const (
)

func (p *Plugin) getCommand(config *configuration) (*model.Command, error) {
iconData, err := command.GetIconData(p.API, "assets/icon.svg")
iconData, err := command.GetIconData(&p.client.System, "assets/icon.svg")
if err != nil {
return nil, errors.Wrap(err, "failed to get icon data")
}
Expand All @@ -114,7 +114,7 @@ func (p *Plugin) postCommandResponse(args *model.CommandArgs, text string) {
RootId: args.RootId,
Message: text,
}
_ = p.API.SendEphemeralPost(args.UserId, post)
p.client.Post.SendEphemeralPost(args.UserId, post)
}

func (p *Plugin) getCommandResponse(args *model.CommandArgs, text string) *model.CommandResponse {
Expand Down Expand Up @@ -168,7 +168,7 @@ func (p *Plugin) ExecuteCommand(c *plugin.Context, args *model.CommandArgs) (*mo
switch {
case err != nil:
text = "Error checking user's permissions"
p.API.LogWarn(text, "error", err.Error())
p.client.Log.Warn(text, "error", err.Error())
case isSysAdmin:
text = "Before using this plugin, you'll need to configure it by running `/gitlab setup`"
default:
Expand All @@ -180,7 +180,7 @@ func (p *Plugin) ExecuteCommand(c *plugin.Context, args *model.CommandArgs) (*mo
}

if action == "connect" {
config := p.API.GetConfig()
config := p.client.Configuration.GetConfig()
if config.ServiceSettings.SiteURL == nil {
return p.getCommandResponse(args, "Encountered an error connecting to GitLab."), nil
}
Expand Down Expand Up @@ -229,7 +229,7 @@ func (p *Plugin) ExecuteCommand(c *plugin.Context, args *model.CommandArgs) (*mo
case "todo":
_, text, err := p.GetToDo(ctx, info)
if err != nil {
p.API.LogWarn("can't get todo in command", "err", err.Error())
p.client.Log.Warn("can't get todo in command", "err", err.Error())
return p.getCommandResponse(args, "Encountered an error getting your to do items."), nil
}
return p.getCommandResponse(args, text), nil
Expand Down Expand Up @@ -259,15 +259,15 @@ func (p *Plugin) ExecuteCommand(c *plugin.Context, args *model.CommandArgs) (*mo
case SettingNotifications:
if value {
if err := p.storeGitlabToUserIDMapping(info.GitlabUsername, info.UserID); err != nil {
p.API.LogWarn("can't store GitLab to user id mapping", "err", err.Error())
p.client.Log.Warn("can't store GitLab to user id mapping", "err", err.Error())
return p.getCommandResponse(args, "Unknown error please retry or ask to an administrator to look at logs"), nil
}
if err := p.storeGitlabIDToUserIDMapping(info.GitlabUsername, info.GitlabUserID); err != nil {
p.API.LogWarn("can't store GitLab to GitLab id mapping", "err", err.Error())
p.client.Log.Warn("can't store GitLab to GitLab id mapping", "err", err.Error())
return p.getCommandResponse(args, "Unknown error please retry or ask to an administrator to look at logs"), nil
}
} else if err := p.deleteGitlabToUserIDMapping(info.GitlabUsername); err != nil {
p.API.LogWarn("can't delete GitLab username in kvstore", "err", err.Error())
p.client.Log.Warn("can't delete GitLab username in kvstore", "err", err.Error())
return p.getCommandResponse(args, "Unknown error please retry or ask to an administrator to look at logs"), nil
}
info.Settings.Notifications = value
Expand All @@ -278,7 +278,7 @@ func (p *Plugin) ExecuteCommand(c *plugin.Context, args *model.CommandArgs) (*mo
}

if err := p.storeGitlabUserInfo(info); err != nil {
p.API.LogWarn("can't store user info after update by command", "err", err.Error())
p.client.Log.Warn("can't store user info after update by command", "err", err.Error())
return p.getCommandResponse(args, "Unknown error please retry or ask to an administrator to look at logs"), nil
}

Expand All @@ -301,7 +301,7 @@ func (p *Plugin) handleSetup(c *plugin.Context, args *model.CommandArgs, paramet
userID := args.UserId
isSysAdmin, err := p.isAuthorizedSysAdmin(userID)
if err != nil {
p.API.LogWarn("Failed to check if user is System Admin", "error", err.Error())
p.client.Log.Warn("Failed to check if user is System Admin", "error", err.Error())

return "Error checking user's permissions"
}
Expand Down Expand Up @@ -385,7 +385,7 @@ func (p *Plugin) webhookCommand(ctx context.Context, parameters []string, info *
return unknownActionMessage
}

siteURL := *p.API.GetConfig().ServiceSettings.SiteURL
siteURL := *p.client.Configuration.GetConfig().ServiceSettings.SiteURL
if siteURL == "" {
return newWebhookEmptySiteURLmessage
}
Expand Down Expand Up @@ -494,7 +494,7 @@ func (p *Plugin) subscriptionDelete(info *gitlab.UserInfo, config *configuration
normalizedPath := normalizePath(fullPath, config.GitlabURL)
deleted, updatedSubscriptions, err := p.Unsubscribe(channelID, normalizedPath)
if err != nil {
p.API.LogWarn("can't unsubscribe channel in command", "err", err.Error())
p.client.Log.Warn("can't unsubscribe channel in command", "err", err.Error())
return "Encountered an error trying to unsubscribe. Please try again.", nil
}

Expand All @@ -512,8 +512,7 @@ func (p *Plugin) subscriptionsListCommand(channelID string) string {
var txt string
subs, err := p.GetSubscriptionsByChannel(channelID)
if err != nil {
txt = err.Error()
return txt
return err.Error()
}
if len(subs) == 0 {
txt = "Currently there are no subscriptions in this channel"
Expand All @@ -538,7 +537,7 @@ func (p *Plugin) subscriptionsAddCommand(ctx context.Context, info *gitlab.UserI
} else if errors.Is(err, gitlab.ErrPrivateResource) {
return "Requested resource is private."
}
p.API.LogWarn(
p.client.Log.Warn(
"unable to resolve subscription namespace and project name",
"err", err.Error(),
)
Expand All @@ -547,7 +546,7 @@ func (p *Plugin) subscriptionsAddCommand(ctx context.Context, info *gitlab.UserI

updatedSubscriptions, subscribeErr := p.Subscribe(info, namespace, project, channelID, features)
if subscribeErr != nil {
p.API.LogWarn(
p.client.Log.Warn(
"failed to subscribe",
"namespace", namespace,
"project", project,
Expand Down Expand Up @@ -670,9 +669,9 @@ func (p *Plugin) pipelineRunCommand(ctx context.Context, namespace, ref, channel
}

func (p *Plugin) isAuthorizedSysAdmin(userID string) (bool, error) {
user, appErr := p.API.GetUser(userID)
if appErr != nil {
return false, appErr
user, err := p.client.User.Get(userID)
if err != nil {
return false, err
}
if !strings.Contains(user.Roles, "system_admin") {
return false, nil
Expand Down
5 changes: 5 additions & 0 deletions server/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"

"github.com/golang/mock/gomock"
pluginapi "github.com/mattermost/mattermost-plugin-api"
"github.com/mattermost/mattermost-server/v6/model"
"github.com/mattermost/mattermost-server/v6/plugin/plugintest"
"github.com/pkg/errors"
Expand Down Expand Up @@ -233,9 +234,12 @@ func getTestPlugin(t *testing.T, mockCtrl *gomock.Controller, hooks []*gitlab.We
var subVal []byte
api.On("KVGet", mock.Anything).Return(subVal, nil)
api.On("KVSet", mock.Anything, mock.Anything).Return(nil)
api.On("KVSetWithOptions", mock.AnythingOfType("string"), mock.Anything, mock.AnythingOfType("model.PluginKVSetOptions")).Return(true, nil)
api.On("PublishWebSocketEvent", mock.Anything, mock.Anything, mock.Anything).Return(nil)

p.SetAPI(api)
p.client = pluginapi.NewClient(api, p.Driver)

return p
}

Expand Down Expand Up @@ -360,6 +364,7 @@ func TestAddWebhookCommand(t *testing.T) {
conf.ServiceSettings.SiteURL = &test.siteURL
api.On("GetConfig", mock.Anything).Return(conf)
p.SetAPI(api)
p.client = pluginapi.NewClient(api, p.Driver)

got := p.webhookCommand(context.Background(), test.parameters, &gitlab.UserInfo{}, true)

Expand Down
11 changes: 8 additions & 3 deletions server/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"reflect"
"strings"

pluginapi "github.com/mattermost/mattermost-plugin-api"
"github.com/mattermost/mattermost-plugin-api/experimental/telemetry"
"github.com/mattermost/mattermost-server/v6/model"
"github.com/pkg/errors"
Expand Down Expand Up @@ -177,16 +178,20 @@ func (p *Plugin) setConfiguration(configuration *configuration, serverConfigurat

// OnConfigurationChange is invoked when configuration changes may have been made.
func (p *Plugin) OnConfigurationChange() error {
if p.client == nil {
p.client = pluginapi.NewClient(p.API, p.Driver)
}

var configuration = new(configuration)

// Load the public configuration fields from the Mattermost server configuration.
if err := p.API.LoadPluginConfiguration(configuration); err != nil {
if err := p.client.Configuration.LoadPluginConfiguration(configuration); err != nil {
return errors.Wrap(err, "failed to load plugin configuration")
}

configuration.sanitize()

serverConfiguration := p.API.GetConfig()
serverConfiguration := p.client.Configuration.GetConfig()

p.setConfiguration(configuration, serverConfiguration)

Expand All @@ -195,7 +200,7 @@ func (p *Plugin) OnConfigurationChange() error {
return errors.Wrap(err, "failed to get command")
}

err = p.API.RegisterCommand(command)
err = p.client.SlashCommand.Register(command)
if err != nil {
return errors.Wrap(err, "failed to register command")
}
Expand Down
Loading

0 comments on commit de004a9

Please sign in to comment.