MM-31182: Reuse the created bot user when creating a DM channel (#16894)

* MM-31182: Reuse the created bot user when creating a DM channel

When creating a bot, we also create a DM channel between the owner
and the bot. So we would hit a replica lag issue when trying to get the
user from the replica instance immediately after creating it.

This falls into the category of read-after-write within a single request.
We fix this by passing the already created bot user during channel creation
itself, thereby getting correctness and saving a DB query.

While here, we also make some other cosmetic improvements:

- Change userId to userID
- Rename nErr to err

https://mattermost.atlassian.net/browse/MM-31182

```release-notes
Fix a bug where creation of a bot would fail due to replica lag.
```

* use correct appError name

app/bot.go

@@ -62,7 +62,11 @@ func (a *App) CreateBot(bot *model.Bot) (*model.Bot, *model.AppError) {
 	} else if ownerUser != nil {
 		// Send a message to the bot's creator to inform them that the bot needs to be added
 		// to a team and channel after it's created
-		channel, err := a.GetOrCreateDirectChannel(savedBot.UserId, bot.OwnerId)
+		botOwner, err := a.GetUser(bot.OwnerId)
+		if err != nil {
+			return nil, err
+		}
+		channel, err := a.getOrCreateDirectChannelWithUser(user, botOwner)
 		if err != nil {
 			return nil, err
 		}

app/bot_test.go

@@ -76,8 +76,11 @@ func TestCreateBot(t *testing.T) {
 		assert.Equal(t, "a bot", bot.Description)
 		assert.Equal(t, th.BasicUser.Id, bot.OwnerId)
 
+		user, err := th.App.GetUser(bot.UserId)
+		require.Nil(t, err)
+
 		// Check that a post was created to add bot to team and channels
-		channel, err := th.App.GetOrCreateDirectChannel(bot.UserId, th.BasicUser.Id)
+		channel, err := th.App.getOrCreateDirectChannelWithUser(user, th.BasicUser)
 		require.Nil(t, err)
 		posts, err := th.App.GetPosts(channel.Id, 0, 1)
 		require.Nil(t, err)

app/channel.go

@@ -339,7 +339,34 @@ func (a *App) GetOrCreateDirectChannel(userID, otherUserID string) (*model.Chann
 	}
 
 	a.WaitForChannelMembership(channel.Id, userID)
+	a.handleCreationEvent(userID, otherUserID, channel)
+	return channel, nil
+}
 
+func (a *App) getOrCreateDirectChannelWithUser(user, otherUser *model.User) (*model.Channel, *model.AppError) {
+	channel, nErr := a.getDirectChannel(user.Id, otherUser.Id)
+	if nErr != nil {
+		return nil, nErr
+	}
+
+	if channel != nil {
+		return channel, nil
+	}
+
+	channel, err := a.createDirectChannelWithUser(user, otherUser)
+	if err != nil {
+		if err.Id == store.ChannelExistsError {
+			return channel, nil
+		}
+		return nil, err
+	}
+
+	a.WaitForChannelMembership(channel.Id, user.Id)
+	a.handleCreationEvent(user.Id, otherUser.Id, channel)
+	return channel, nil
+}
+
+func (a *App) handleCreationEvent(userID, otherUserID string, channel *model.Channel) {
 	a.InvalidateCacheForUser(userID)
 	a.InvalidateCacheForUser(otherUserID)
 
@@ -356,11 +383,9 @@ func (a *App) GetOrCreateDirectChannel(userID, otherUserID string) (*model.Chann
 	message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_DIRECT_ADDED, "", channel.Id, "", nil)
 	message.Add("teammate_id", otherUserID)
 	a.Publish(message)
-
-	return channel, nil
 }
 
-func (a *App) createDirectChannel(userID string, otherUserID string) (*model.Channel, *model.AppError) {
+func (a *App) createDirectChannel(userID, otherUserID string) (*model.Channel, *model.AppError) {
 	users, err := a.Srv().Store.User().GetMany([]string{userID, otherUserID})
 	if err != nil {
 		return nil, model.NewAppError("CreateDirectChannel", "api.channel.create_direct_channel.invalid_user.app_error", nil, err.Error(), http.StatusBadRequest)
@@ -390,7 +415,10 @@ func (a *App) createDirectChannel(userID string, otherUserID string) (*model.Cha
 		user = users[1]
 		otherUser = users[0]
 	}
+	return a.createDirectChannelWithUser(user, otherUser)
+}
 
+func (a *App) createDirectChannelWithUser(user, otherUser *model.User) (*model.Channel, *model.AppError) {
 	channel, nErr := a.Srv().Store.Channel().CreateDirectChannel(user, otherUser)
 	if nErr != nil {
 		var invErr *store.ErrInvalidInput
@@ -401,34 +429,34 @@ func (a *App) createDirectChannel(userID string, otherUserID string) (*model.Cha
 		case errors.As(nErr, &invErr):
 			switch {
 			case invErr.Entity == "Channel" && invErr.Field == "DeleteAt":
-				return nil, model.NewAppError("CreateChannel", "store.sql_channel.save.archived_channel.app_error", nil, "", http.StatusBadRequest)
+				return nil, model.NewAppError("createDirectChannelWithUser", "store.sql_channel.save.archived_channel.app_error", nil, "", http.StatusBadRequest)
 			case invErr.Entity == "Channel" && invErr.Field == "Type":
-				return nil, model.NewAppError("CreateChannel", "store.sql_channel.save_direct_channel.not_direct.app_error", nil, "", http.StatusBadRequest)
+				return nil, model.NewAppError("createDirectChannelWithUser", "store.sql_channel.save_direct_channel.not_direct.app_error", nil, "", http.StatusBadRequest)
 			case invErr.Entity == "Channel" && invErr.Field == "Id":
 				return nil, model.NewAppError("SqlChannelStore.Save", "store.sql_channel.save_channel.existing.app_error", nil, "id="+invErr.Value.(string), http.StatusBadRequest)
 			}
 		case errors.As(nErr, &cErr):
 			switch cErr.Resource {
 			case "Channel":
-				return channel, model.NewAppError("CreateChannel", store.ChannelExistsError, nil, cErr.Error(), http.StatusBadRequest)
+				return channel, model.NewAppError("createDirectChannelWithUser", store.ChannelExistsError, nil, cErr.Error(), http.StatusBadRequest)
 			case "ChannelMembers":
-				return nil, model.NewAppError("CreateChannel", "app.channel.save_member.exists.app_error", nil, cErr.Error(), http.StatusBadRequest)
+				return nil, model.NewAppError("createDirectChannelWithUser", "app.channel.save_member.exists.app_error", nil, cErr.Error(), http.StatusBadRequest)
 			}
 		case errors.As(nErr, &ltErr):
-			return nil, model.NewAppError("CreateChannel", "store.sql_channel.save_channel.limit.app_error", nil, ltErr.Error(), http.StatusBadRequest)
+			return nil, model.NewAppError("createDirectChannelWithUser", "store.sql_channel.save_channel.limit.app_error", nil, ltErr.Error(), http.StatusBadRequest)
 		case errors.As(nErr, &appErr): // in case we haven't converted to plain error.
 			return nil, appErr
 		default: // last fallback in case it doesn't map to an existing app error.
-			return nil, model.NewAppError("CreateDirectChannel", "app.channel.create_direct_channel.internal_error", nil, nErr.Error(), http.StatusInternalServerError)
+			return nil, model.NewAppError("createDirectChannelWithUser", "app.channel.create_direct_channel.internal_error", nil, nErr.Error(), http.StatusInternalServerError)
 		}
 	}
 
-	if err := a.Srv().Store.ChannelMemberHistory().LogJoinEvent(userID, channel.Id, model.GetMillis()); err != nil {
-		return nil, model.NewAppError("CreateDirectChannel", "app.channel_member_history.log_join_event.internal_error", nil, err.Error(), http.StatusInternalServerError)
+	if err := a.Srv().Store.ChannelMemberHistory().LogJoinEvent(user.Id, channel.Id, model.GetMillis()); err != nil {
+		return nil, model.NewAppError("createDirectChannelWithUser", "app.channel_member_history.log_join_event.internal_error", nil, err.Error(), http.StatusInternalServerError)
 	}
-	if userID != otherUserID {
-		if err := a.Srv().Store.ChannelMemberHistory().LogJoinEvent(otherUserID, channel.Id, model.GetMillis()); err != nil {
-			return nil, model.NewAppError("CreateDirectChannel", "app.channel_member_history.log_join_event.internal_error", nil, err.Error(), http.StatusInternalServerError)
+	if user.Id != otherUser.Id {
+		if err := a.Srv().Store.ChannelMemberHistory().LogJoinEvent(otherUser.Id, channel.Id, model.GetMillis()); err != nil {
+			return nil, model.NewAppError("createDirectChannelWithUser", "app.channel_member_history.log_join_event.internal_error", nil, err.Error(), http.StatusInternalServerError)
 		}
 	}