Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MM-50985] Remove usage of model.AppError #648

Merged
merged 8 commits into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
github.com/Masterminds/sprig/v3 v3.2.2
github.com/google/go-github/v41 v41.0.0
github.com/gorilla/mux v1.8.0
github.com/mattermost/mattermost-plugin-api v0.1.1
github.com/mattermost/mattermost-plugin-api v0.1.2
// mmgoget: github.com/mattermost/mattermost-server/[email protected] is replaced by -> github.com/mattermost/mattermost-server/v6@21aec2741b
github.com/mattermost/mattermost-server/v6 v6.0.0-20221109191448-21aec2741bfe
github.com/microcosm-cc/bluemonday v1.0.19
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,8 @@ github.com/mattermost/ldap v0.0.0-20201202150706-ee0e6284187d h1:/RJ/UV7M5c7L2TQ
github.com/mattermost/ldap v0.0.0-20201202150706-ee0e6284187d/go.mod h1:HLbgMEI5K131jpxGazJ97AxfPDt31osq36YS1oxFQPQ=
github.com/mattermost/logr/v2 v2.0.15 h1:+WNbGcsc3dBao65eXlceB6dTILNJRIrvubnsTl3zBew=
github.com/mattermost/logr/v2 v2.0.15/go.mod h1:mpPp935r5dIkFDo2y9Q87cQWhFR/4xXpNh0k/y8Hmwg=
github.com/mattermost/mattermost-plugin-api v0.1.1 h1:bNnPbWCLWZpT/k2kjUxNnzCfUggU8WKs2ddz7hNjg1U=
github.com/mattermost/mattermost-plugin-api v0.1.1/go.mod h1:9yZhtg0bBj3kqSTjXnjYBMZoTsWbe3ajdFMdl9/Jz34=
github.com/mattermost/mattermost-plugin-api v0.1.2 h1:poMLvI3oEULpKqb9pfxWSthFpqKnZsB/eFk4u30tzEY=
github.com/mattermost/mattermost-plugin-api v0.1.2/go.mod h1:EON5x6XoqubANT9zYLuL+m42SijzpK/dDOuu76PGbSc=
github.com/mattermost/mattermost-server/v6 v6.0.0-20221109191448-21aec2741bfe h1:9P759/e7xB3IQYo+6Ju9qsj3YUxpnPRXl2x+XFFsRIM=
github.com/mattermost/mattermost-server/v6 v6.0.0-20221109191448-21aec2741bfe/go.mod h1:Eu6nct2XZRPlHHW4MfZtjli5bY7G9QomKcor/lm+0Sw=
github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc=
Expand Down
81 changes: 36 additions & 45 deletions server/plugin/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/pkg/errors"
"golang.org/x/oauth2"

pluginapi "github.com/mattermost/mattermost-plugin-api"
"github.com/mattermost/mattermost-plugin-api/experimental/bot/logger"
"github.com/mattermost/mattermost-plugin-api/experimental/flow"
"github.com/mattermost/mattermost-server/v6/model"
Expand Down Expand Up @@ -86,13 +87,13 @@ const (
func (p *Plugin) writeJSON(w http.ResponseWriter, v interface{}) {
b, err := json.Marshal(v)
if err != nil {
p.API.LogWarn("Failed to marshal JSON response", "error", err.Error())
p.client.Log.Warn("Failed to marshal JSON response", "error", err.Error())
w.WriteHeader(http.StatusInternalServerError)
return
}
_, err = w.Write(b)
if err != nil {
p.API.LogWarn("Failed to write JSON response", "error", err.Error())
p.client.Log.Warn("Failed to write JSON response", "error", err.Error())
w.WriteHeader(http.StatusInternalServerError)
return
}
Expand All @@ -101,7 +102,7 @@ func (p *Plugin) writeJSON(w http.ResponseWriter, v interface{}) {
func (p *Plugin) writeAPIError(w http.ResponseWriter, apiErr *APIErrorResponse) {
b, err := json.Marshal(apiErr)
if err != nil {
p.API.LogWarn("Failed to marshal API error", "error", err.Error())
p.client.Log.Warn("Failed to marshal API error", "error", err.Error())
w.WriteHeader(http.StatusInternalServerError)
return
}
Expand All @@ -110,7 +111,7 @@ func (p *Plugin) writeAPIError(w http.ResponseWriter, apiErr *APIErrorResponse)

_, err = w.Write(b)
if err != nil {
p.API.LogWarn("Failed to write JSON response", "error", err.Error())
p.client.Log.Warn("Failed to write JSON response", "error", err.Error())
w.WriteHeader(http.StatusInternalServerError)
return
}
Expand Down Expand Up @@ -158,7 +159,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 @@ -192,7 +193,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 @@ -286,14 +287,8 @@ func (p *Plugin) connectUserToGitHub(c *Context, w http.ResponseWriter, r *http.
PrivateAllowed: privateAllowed,
}

stateBytes, err := json.Marshal(state)
_, err := p.client.KV.Set(githubOauthKey+state.Token, state, pluginapi.SetExpiry(TokenTTL))
if err != nil {
http.Error(w, "json marshal failed", http.StatusInternalServerError)
return
}

appErr := p.API.KVSetWithExpiry(githubOauthKey+state.Token, stateBytes, TokenTTL)
if appErr != nil {
http.Error(w, "error setting stored state", http.StatusBadRequest)
return
}
Expand Down Expand Up @@ -347,27 +342,21 @@ func (p *Plugin) completeConnectUserToGitHub(c *Context, w http.ResponseWriter,

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

storedState, appErr := p.API.KVGet(githubOauthKey + stateToken)
if appErr != nil {
c.Log.Warnf("Failed to get state token", "error", appErr.Error())

rErr = errors.Wrap(appErr, "missing stored state")
http.Error(w, rErr.Error(), http.StatusBadRequest)
return
}
var state OAuthState
err := p.client.KV.Get(githubOauthKey+stateToken, &state)
if err != nil {
c.Log.Warnf("Failed to get state token", "error", err.Error())

if err := json.Unmarshal(storedState, &state); err != nil {
rErr = errors.Wrap(err, "json unmarshal failed")
http.Error(w, err.Error(), http.StatusBadRequest)
rErr = errors.Wrap(err, "missing stored state")
http.Error(w, rErr.Error(), http.StatusBadRequest)
return
}

appErr = p.API.KVDelete(githubOauthKey + stateToken)
if appErr != nil {
c.Log.WithError(appErr).Warnf("Failed to delete state token")
err = p.client.KV.Delete(githubOauthKey + stateToken)
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)
return
}
Expand Down Expand Up @@ -480,7 +469,7 @@ func (p *Plugin) completeConnectUserToGitHub(c *Context, w http.ResponseWriter,

config := p.getConfiguration()

p.API.PublishWebSocketEvent(
p.client.Frontend.PublishWebSocketEvent(
wsEventConnect,
map[string]interface{}{
"connected": true,
Expand Down Expand Up @@ -618,7 +607,8 @@ func (p *Plugin) getConnected(c *Context, w http.ResponseWriter, r *http.Request

privateRepoStoreKey := info.UserID + githubPrivateRepoKey
if config.EnablePrivateRepo && !info.AllowedPrivateRepos {
val, err := p.API.KVGet(privateRepoStoreKey)
var val []byte
err := p.client.KV.Get(privateRepoStoreKey, &val)
if err != nil {
c.Log.WithError(err).Warnf("Unable to get private repo key value")
return
Expand All @@ -632,7 +622,7 @@ func (p *Plugin) getConnected(c *Context, w http.ResponseWriter, r *http.Request
} else {
p.CreateBotDMPost(info.UserID, fmt.Sprintf(message, "`/github connect private`."), "")
}
err := p.API.KVSet(privateRepoStoreKey, []byte("1"))
_, err := p.client.KV.Set(privateRepoStoreKey, []byte("1"))
if err != nil {
c.Log.WithError(err).Warnf("Unable to set private repo key value")
}
Expand Down Expand Up @@ -849,7 +839,7 @@ func (p *Plugin) searchIssues(c *UserContext, w http.ResponseWriter, r *http.Req
}

func (p *Plugin) getPermaLink(postID string) string {
siteURL := *p.API.GetConfig().ServiceSettings.SiteURL
siteURL := *p.client.Configuration.GetConfig().ServiceSettings.SiteURL

return fmt.Sprintf("%v/_redirect/pl/%v", siteURL, postID)
}
Expand Down Expand Up @@ -916,8 +906,8 @@ func (p *Plugin) createIssueComment(c *UserContext, w http.ResponseWriter, r *ht

githubClient := p.githubConnectUser(c.Context.Ctx, c.GHInfo)

post, appErr := p.API.GetPost(req.PostID)
if appErr != nil {
post, err := p.client.Post.GetPost(req.PostID)
if err != nil {
p.writeAPIError(w, &APIErrorResponse{ID: "", Message: "failed to load post " + req.PostID, StatusCode: http.StatusInternalServerError})
return
}
Expand Down Expand Up @@ -965,8 +955,8 @@ func (p *Plugin) createIssueComment(c *UserContext, w http.ResponseWriter, r *ht
UserId: c.UserID,
}

_, appErr = p.API.CreatePost(reply)
if appErr != nil {
err = p.client.Post.CreatePost(reply)
if err != nil {
p.writeAPIError(w, &APIErrorResponse{ID: "", Message: "failed to create notification post " + req.PostID, StatusCode: http.StatusInternalServerError})
return
}
Expand Down Expand Up @@ -1272,6 +1262,7 @@ func (p *Plugin) createIssue(c *UserContext, w http.ResponseWriter, r *http.Requ

// get data for the issue from the request body and fill IssueRequest object
issue := &IssueRequest{}

if err := json.NewDecoder(r.Body).Decode(&issue); err != nil {
c.Log.WithError(err).Warnf("Error decoding JSON body")
p.writeAPIError(w, &APIErrorResponse{ID: "", Message: "Please provide a JSON object.", StatusCode: http.StatusBadRequest})
Expand All @@ -1297,9 +1288,9 @@ func (p *Plugin) createIssue(c *UserContext, w http.ResponseWriter, r *http.Requ
var post *model.Post
permalink := ""
if issue.PostID != "" {
var appErr *model.AppError
post, appErr = p.API.GetPost(issue.PostID)
if appErr != nil {
var err error
post, err = p.client.Post.GetPost(issue.PostID)
if err != nil {
p.writeAPIError(w, &APIErrorResponse{ID: "", Message: "failed to load post " + issue.PostID, StatusCode: http.StatusInternalServerError})
return
}
Expand Down Expand Up @@ -1337,8 +1328,8 @@ func (p *Plugin) createIssue(c *UserContext, w http.ResponseWriter, r *http.Requ
}
*ghIssue.Body = ghIssue.GetBody() + mmMessage

currentUser, appErr := p.API.GetUser(c.UserID)
if appErr != nil {
currentUser, err := p.client.User.Get(c.UserID)
if err != nil {
p.writeAPIError(w, &APIErrorResponse{ID: "", Message: "failed to load current user", StatusCode: http.StatusInternalServerError})
return
}
Expand Down Expand Up @@ -1387,12 +1378,12 @@ func (p *Plugin) createIssue(c *UserContext, w http.ResponseWriter, r *http.Requ
}

if post != nil {
_, appErr = p.API.CreatePost(reply)
err = p.client.Post.CreatePost(reply)
} else {
p.API.SendEphemeralPost(c.UserID, reply)
p.client.Post.SendEphemeralPost(c.UserID, reply)
}
if appErr != nil {
c.Log.WithError(appErr).Warnf("failed to create notification post")
if err != nil {
c.Log.WithError(err).Warnf("failed to create notification post")
p.writeAPIError(w, &APIErrorResponse{ID: "", Message: "failed to create notification post, postID: " + issue.PostID + ", channelID: " + channelID, StatusCode: http.StatusInternalServerError})
return
}
Expand Down
2 changes: 2 additions & 0 deletions server/plugin/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/http/httptest"
"testing"

pluginapi "github.com/mattermost/mattermost-plugin-api"
"github.com/mattermost/mattermost-server/v6/plugin"
"github.com/mattermost/mattermost-server/v6/plugin/plugintest"
"github.com/stretchr/testify/mock"
Expand Down Expand Up @@ -36,6 +37,7 @@ func TestWithRecovery(t *testing.T) {
"error", "bad handler",
"stack", mock.Anything)
p.SetAPI(api)
p.client = pluginapi.NewClient(p.API, p.Driver)

ph := panicHandler{}
handler := p.withRecovery(ph)
Expand Down
12 changes: 6 additions & 6 deletions server/plugin/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,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 @@ -35,8 +35,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 @@ -48,20 +48,20 @@ func (p *Plugin) HandleClusterEvent(ev model.PluginClusterEvent) {
case webHookPingEventID:
var event github.PingEvent
if err := json.Unmarshal(ev.Data, &event); err != nil {
p.API.LogWarn("cannot unmarshal cluster event with GitHub ping event", "error", err)
p.client.Log.Warn("cannot unmarshal cluster event with GitHub ping event", "error", err)
return
}

p.webhookBroker.publishPing(&event, true)
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)
}
}
Loading