[MI-3591] Add transactions in the functions to avoid race conditions (#…

…344)

* [MI-3591] Add transactions in the functions to avoid race conditions

* [MI-3591] Fix test cases and lint errors

* [MI-3591] Update test cases

* [MI-3591] Refactore test cases

* [MI-3591] Add checks for nil in client

* [MI-3591] Add more checks for nil in client

* [MI-3591] Update log messages

* [MI-3591] Fix lint

* [MI-3591] Remove extra condition

server/command.go

@@ -207,15 +207,35 @@ func (p *Plugin) executeLinkCommand(args *model.CommandArgs, parameters []string
 		return p.cmdError(args.UserId, args.ChannelId, "Unable to create new link.")
 	}
 
-	err = p.store.SaveChannelSubscription(storemodels.ChannelSubscription{
+	tx, err := p.store.BeginTx()
+	if err != nil {
+		p.API.LogError("Unable to begin the database transaction", "error", err.Error())
+		return p.cmdError(args.UserId, args.ChannelId, "Something went wrong")
+	}
+
+	var txErr error
+	defer func() {
+		if txErr != nil {
+			if err := p.store.RollbackTx(tx); err != nil {
+				p.API.LogError("Unable to rollback database transaction", "error", err.Error())
+			}
+		}
+	}()
+
+	if txErr = p.store.SaveChannelSubscription(storemodels.ChannelSubscription{
 		SubscriptionID: channelsSubscription.ID,
 		TeamID:         channelLink.MSTeamsTeam,
 		ChannelID:      channelLink.MSTeamsChannel,
 		ExpiresOn:      channelsSubscription.ExpiresOn,
 		Secret:         p.getConfiguration().WebhookSecret,
-	})
-	if err != nil {
-		return p.cmdError(args.UserId, args.ChannelId, "Unable to save the subscription in the monitoring system: "+err.Error())
+	}, tx); txErr != nil {
+		p.API.LogWarn("Unable to save the subscription in the DB", "error", txErr.Error())
+		return p.cmdError(args.UserId, args.ChannelId, "Error occurred while saving the subscription")
+	}
+
+	if err := p.store.CommitTx(tx); err != nil {
+		p.API.LogError("Unable to commit database transaction", "error", err.Error())
+		return p.cmdError(args.UserId, args.ChannelId, "Something went wrong")
 	}
 
 	p.sendBotEphemeralPost(args.UserId, args.ChannelId, "The MS Teams channel is now linked to this Mattermost channel.")

server/handlers/handlers.go

@@ -264,8 +264,7 @@ func (ah *ActivityHandler) handleCreatedActivity(activityIds msteams.ActivityIds
 
 	ah.updateLastReceivedChangeDate(msg.LastUpdateAt)
 	if newPost != nil && newPost.Id != "" && msg.ID != "" {
-		err = ah.plugin.GetStore().LinkPosts(storemodels.PostInfo{MattermostID: newPost.Id, MSTeamsChannel: msg.ChatID + msg.ChannelID, MSTeamsID: msg.ID, MSTeamsLastUpdateAt: msg.LastUpdateAt})
-		if err != nil {
+		if err := ah.plugin.GetStore().LinkPosts(storemodels.PostInfo{MattermostID: newPost.Id, MSTeamsChannel: fmt.Sprintf(msg.ChatID + msg.ChannelID), MSTeamsID: msg.ID, MSTeamsLastUpdateAt: msg.LastUpdateAt}, nil); err != nil {
 			ah.plugin.GetAPI().LogWarn("Error updating the MSTeams/Mattermost post link metadata", "error", err)
 		}
 	}

server/handlers/handlers_test.go

@@ -1,6 +1,7 @@
 package handlers
 
 import (
+	"database/sql"
 	"errors"
 	"fmt"
 	"testing"
@@ -374,7 +375,7 @@ func TestHandleCreatedActivity(t *testing.T) {
 					MattermostID:   testutils.GetID(),
 					MSTeamsID:      testutils.GetMessageID(),
 					MSTeamsChannel: testutils.GetMSTeamsChannelID(),
-				}).Return(errors.New("unable to update the post")).Times(1)
+				}, (*sql.Tx)(nil)).Return(errors.New("unable to update the post")).Times(1)
 			},
 		},
 		{
@@ -430,7 +431,7 @@ func TestHandleCreatedActivity(t *testing.T) {
 					MattermostID:   testutils.GetID(),
 					MSTeamsID:      testutils.GetMessageID(),
 					MSTeamsChannel: testutils.GetMSTeamsChannelID(),
-				}).Return(nil).Times(1)
+				}, (*sql.Tx)(nil)).Return(nil).Times(1)
 			},
 		},
 		{
@@ -477,7 +478,7 @@ func TestHandleCreatedActivity(t *testing.T) {
 					MattermostID:   testutils.GetID(),
 					MSTeamsID:      testutils.GetMessageID(),
 					MSTeamsChannel: testutils.GetChannelID(),
-				}).Return(nil).Times(1)
+				}, (*sql.Tx)(nil)).Return(nil).Times(1)
 			},
 		},
 	} {