guard msteamsAppClient (#440)

We had a mutex guarding msteamsAppClient, but it wasn't being used consistently. Fix that, and change it to a `RWMutex` to minimize the critical sections.
mattermost · Dec 14, 2023 · 38f3f62 · 38f3f62
1 parent f3bdb58
commit 38f3f62
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 10 deletions.
diff --git a/server/api.go b/server/api.go
@@ -87,7 +87,7 @@ func (a *API) getAvatar(w http.ResponseWriter, r *http.Request) {
 	photo, appErr := a.store.GetAvatarCache(userID)
 	if appErr != nil || len(photo) == 0 {
 		var err error
-		photo, err = a.p.msteamsAppClient.GetUserAvatar(userID)
+		photo, err = a.p.GetClientForApp().GetUserAvatar(userID)
 		if err != nil {
 			a.p.API.LogError("Unable to get user avatar", "msteamsUserID", userID, "error", err.Error())
 			http.Error(w, "avatar not found", http.StatusNotFound)

diff --git a/server/command.go b/server/command.go
@@ -198,7 +198,7 @@ func (p *Plugin) executeLinkCommand(args *model.CommandArgs, parameters []string
 	}
 
 	p.sendBotEphemeralPost(args.UserId, args.ChannelId, commandWaitingMessage)
-	channelsSubscription, err := p.msteamsAppClient.SubscribeToChannel(channelLink.MSTeamsTeam, channelLink.MSTeamsChannel, p.GetURL()+"/", p.getConfiguration().WebhookSecret, p.getBase64Certificate())
+	channelsSubscription, err := p.GetClientForApp().SubscribeToChannel(channelLink.MSTeamsTeam, channelLink.MSTeamsChannel, p.GetURL()+"/", p.getConfiguration().WebhookSecret, p.getBase64Certificate())
 	if err != nil {
 		p.API.LogDebug("Unable to subscribe to the channel", "channelID", channelLink.MattermostChannelID, "error", err.Error())
 		return p.cmdError(args.UserId, args.ChannelId, "Unable to subscribe to the channel")
@@ -284,7 +284,7 @@ func (p *Plugin) executeUnlinkCommand(args *model.CommandArgs) (*model.CommandRe
 		return &model.CommandResponse{}, nil
 	}
 
-	if err = p.msteamsAppClient.DeleteSubscription(subscription.SubscriptionID); err != nil {
+	if err = p.GetClientForApp().DeleteSubscription(subscription.SubscriptionID); err != nil {
 		p.API.LogDebug("Unable to delete the subscription on MS Teams", "subscriptionID", subscription.SubscriptionID, "error", err.Error())
 	}
 
@@ -297,12 +297,12 @@ func (p *Plugin) executeShowCommand(args *model.CommandArgs) (*model.CommandResp
 		return p.cmdError(args.UserId, args.ChannelId, "Link doesn't exist.")
 	}
 
-	msteamsTeam, err := p.msteamsAppClient.GetTeam(link.MSTeamsTeam)
+	msteamsTeam, err := p.GetClientForApp().GetTeam(link.MSTeamsTeam)
 	if err != nil {
 		return p.cmdError(args.UserId, args.ChannelId, "Unable to get the MS Teams team information.")
 	}
 
-	msteamsChannel, err := p.msteamsAppClient.GetChannelInTeam(link.MSTeamsTeam, link.MSTeamsChannel)
+	msteamsChannel, err := p.GetClientForApp().GetChannelInTeam(link.MSTeamsTeam, link.MSTeamsChannel)
 	if err != nil {
 		return p.cmdError(args.UserId, args.ChannelId, "Unable to get the MS Teams channel information.")
 	}
@@ -404,7 +404,7 @@ func (p *Plugin) GetMSTeamsTeamDetails(msTeamsTeamIDsVsNames map[string]string)
 
 	teamsQuery := msTeamsFilterQuery.String()
 	teamsQuery = strings.TrimSuffix(teamsQuery, ", ") + ")"
-	msTeamsTeams, err := p.msteamsAppClient.GetTeams(teamsQuery)
+	msTeamsTeams, err := p.GetClientForApp().GetTeams(teamsQuery)
 	if err != nil {
 		p.API.LogDebug("Unable to get the MS Teams teams information", "Error", err.Error())
 		return true
@@ -420,7 +420,7 @@ func (p *Plugin) GetMSTeamsTeamDetails(msTeamsTeamIDsVsNames map[string]string)
 func (p *Plugin) GetMSTeamsChannelDetailsForAllTeams(msTeamsTeamIDsVsChannelsQuery, msTeamsChannelIDsVsNames map[string]string) bool {
 	errorsFound := false
 	for teamID, channelsQuery := range msTeamsTeamIDsVsChannelsQuery {
-		channels, err := p.msteamsAppClient.GetChannelsInTeam(teamID, channelsQuery+")")
+		channels, err := p.GetClientForApp().GetChannelsInTeam(teamID, channelsQuery+")")
 		if err != nil {
 			p.API.LogDebug("Unable to get the MS Teams channel information for the team", "TeamID", teamID, "Error", err.Error())
 			errorsFound = true

diff --git a/server/plugin.go b/server/plugin.go
@@ -60,7 +60,7 @@ type Plugin struct {
 	// setConfiguration for usage.
 	configuration *configuration
 
-	msteamsAppClientMutex sync.Mutex
+	msteamsAppClientMutex sync.RWMutex
 	msteamsAppClient      msteams.Client
 
 	stopSubscriptions func()
@@ -124,6 +124,9 @@ func (p *Plugin) GetBotUserID() string {
 }
 
 func (p *Plugin) GetClientForApp() msteams.Client {
+	p.msteamsAppClientMutex.RLock()
+	defer p.msteamsAppClientMutex.RUnlock()
+
 	return p.msteamsAppClient
 }
 
@@ -200,7 +203,7 @@ func (p *Plugin) start(isRestart bool) {
 		return
 	}
 
-	p.monitor = monitor.New(p.msteamsAppClient, p.store, p.API, p.GetMetrics(), p.GetURL()+"/", p.getConfiguration().WebhookSecret, p.getConfiguration().EvaluationAPI, p.getBase64Certificate())
+	p.monitor = monitor.New(p.GetClientForApp(), p.store, p.API, p.GetMetrics(), p.GetURL()+"/", p.getConfiguration().WebhookSecret, p.getConfiguration().EvaluationAPI, p.getBase64Certificate())
 	if err = p.monitor.Start(); err != nil {
 		p.API.LogError("Unable to start the monitoring system", "error", err.Error())
 	}
@@ -465,7 +468,7 @@ func (p *Plugin) stopSyncUsersJob() {
 }
 
 func (p *Plugin) syncUsers() {
-	msUsers, err := p.msteamsAppClient.ListUsers()
+	msUsers, err := p.GetClientForApp().ListUsers()
 	if err != nil {
 		p.API.LogError("Unable to list MS Teams users during sync user job", "error", err.Error())
 		return