MM-29067 Add deterministic IDs for default sidebar categories (#16030)

Automatic Merge

api4/channel.go

@@ -40,9 +40,9 @@ func (api *API) InitChannel() {
 	api.BaseRoutes.ChannelCategories.Handle("", api.ApiSessionRequired(updateCategoriesForTeamForUser)).Methods("PUT")
 	api.BaseRoutes.ChannelCategories.Handle("/order", api.ApiSessionRequired(getCategoryOrderForTeamForUser)).Methods("GET")
 	api.BaseRoutes.ChannelCategories.Handle("/order", api.ApiSessionRequired(updateCategoryOrderForTeamForUser)).Methods("PUT")
-	api.BaseRoutes.ChannelCategories.Handle("/{category_id:[A-Za-z0-9]+}", api.ApiSessionRequired(getCategoryForTeamForUser)).Methods("GET")
+	api.BaseRoutes.ChannelCategories.Handle("/{category_id:[A-Za-z0-9_-]+}", api.ApiSessionRequired(getCategoryForTeamForUser)).Methods("GET")
-	api.BaseRoutes.ChannelCategories.Handle("/{category_id:[A-Za-z0-9]+}", api.ApiSessionRequired(updateCategoryForTeamForUser)).Methods("PUT")
+	api.BaseRoutes.ChannelCategories.Handle("/{category_id:[A-Za-z0-9_-]+}", api.ApiSessionRequired(updateCategoryForTeamForUser)).Methods("PUT")
-	api.BaseRoutes.ChannelCategories.Handle("/{category_id:[A-Za-z0-9]+}", api.ApiSessionRequired(deleteCategoryForTeamForUser)).Methods("DELETE")
+	api.BaseRoutes.ChannelCategories.Handle("/{category_id:[A-Za-z0-9_-]+}", api.ApiSessionRequired(deleteCategoryForTeamForUser)).Methods("DELETE")
 
 	api.BaseRoutes.Channel.Handle("", api.ApiSessionRequired(getChannel)).Methods("GET")
 	api.BaseRoutes.Channel.Handle("", api.ApiSessionRequired(updateChannel)).Methods("PUT")

model/channel_sidebar.go

@@ -6,6 +6,7 @@ package model
 import (
 	"encoding/json"
 	"io"
+	"regexp"
 )
 
 type SidebarCategoryType string
@@ -109,3 +110,15 @@ func (o OrderedSidebarCategories) ToJson() []byte {
 		return b
 	}
 }
+
+var categoryIdPattern = regexp.MustCompile("(favorites|channels|direct_messages)_[a-z0-9]{26}_[a-z0-9]{26}")
+
+func IsValidCategoryId(s string) bool {
+	// Category IDs can either be regular IDs
+	if IsValidId(s) {
+		return true
+	}
+
+	// Or default categories can follow the pattern {type}_{userID}_{teamID}
+	return categoryIdPattern.MatchString(s)
+}

model/channel_sidebar_test.go

@@ -0,0 +1,49 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package model
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIsValidCategoryId(t *testing.T) {
+	for _, test := range []struct {
+		Name     string
+		Input    string
+		Expected bool
+	}{
+		{
+			Name:     "should accept a regular ID",
+			Input:    NewId(),
+			Expected: true,
+		},
+		{
+			Name:     "should accept a favorites ID",
+			Input:    fmt.Sprintf("favorites_%s_%s", NewId(), NewId()),
+			Expected: true,
+		},
+		{
+			Name:     "should accept a channels ID",
+			Input:    fmt.Sprintf("channels_%s_%s", NewId(), NewId()),
+			Expected: true,
+		},
+		{
+			Name:     "should accept a direct messages ID",
+			Input:    fmt.Sprintf("direct_messages_%s_%s", NewId(), NewId()),
+			Expected: true,
+		},
+		{
+			Name:     "should reject a garbage ID",
+			Input:    "a garbage ID",
+			Expected: false,
+		},
+	} {
+		t.Run(test.Name, func(t *testing.T) {
+			assert.Equal(t, test.Expected, IsValidCategoryId(test.Input))
+		})
+	}
+}

store/sqlstore/channel_store.go

@@ -391,7 +391,7 @@ func newSqlChannelStore(sqlStore SqlStore, metrics einterfaces.MetricsInterface)
 		tablePublicChannels.ColMap("Purpose").SetMaxSize(250)
 
 		tableSidebarCategories := db.AddTableWithName(model.SidebarCategory{}, "SidebarCategories").SetKeys(false, "Id")
-		tableSidebarCategories.ColMap("Id").SetMaxSize(26)
+		tableSidebarCategories.ColMap("Id").SetMaxSize(128)
 		tableSidebarCategories.ColMap("UserId").SetMaxSize(26)
 		tableSidebarCategories.ColMap("TeamId").SetMaxSize(26)
 		tableSidebarCategories.ColMap("Sorting").SetMaxSize(64)
@@ -401,7 +401,7 @@ func newSqlChannelStore(sqlStore SqlStore, metrics einterfaces.MetricsInterface)
 		tableSidebarChannels := db.AddTableWithName(model.SidebarChannel{}, "SidebarChannels").SetKeys(false, "ChannelId", "UserId", "CategoryId")
 		tableSidebarChannels.ColMap("ChannelId").SetMaxSize(26)
 		tableSidebarChannels.ColMap("UserId").SetMaxSize(26)
-		tableSidebarChannels.ColMap("CategoryId").SetMaxSize(26)
+		tableSidebarChannels.ColMap("CategoryId").SetMaxSize(128)
 	}
 
 	return s

store/sqlstore/channel_store_categories.go

@@ -53,9 +53,12 @@ func (s SqlChannelStore) createInitialSidebarCategoriesT(transaction *gorp.Trans
 		hasCategoryOfType[existingType] = true
 	}
 
-	if !hasCategoryOfType[model.SidebarCategoryFavorites] {
+	// Use deterministic IDs for default categories to prevent potentially creating multiple copies of a default category
-		favoritesCategoryId := model.NewId()
+	favoritesCategoryId := fmt.Sprintf("%s_%s_%s", model.SidebarCategoryFavorites, userId, teamId)
+	channelsCategoryId := fmt.Sprintf("%s_%s_%s", model.SidebarCategoryChannels, userId, teamId)
+	directMessagesCategoryId := fmt.Sprintf("%s_%s_%s", model.SidebarCategoryDirectMessages, userId, teamId)
 
+	if !hasCategoryOfType[model.SidebarCategoryFavorites] {
 		// Create the SidebarChannels first since there's more opportunity for something to fail here
 		if err := s.migrateFavoritesToSidebarT(transaction, userId, teamId, favoritesCategoryId); err != nil {
 			return errors.Wrap(err, "createInitialSidebarCategoriesT: failed to migrate favorites to sidebar")
@@ -77,7 +80,7 @@ func (s SqlChannelStore) createInitialSidebarCategoriesT(transaction *gorp.Trans
 	if !hasCategoryOfType[model.SidebarCategoryChannels] {
 		if err := transaction.Insert(&model.SidebarCategory{
 			DisplayName: "Channels", // This will be retranslateed by the client into the user's locale
-			Id:          model.NewId(),
+			Id:          channelsCategoryId,
 			UserId:      userId,
 			TeamId:      teamId,
 			Sorting:     model.SidebarCategorySortDefault,
@@ -91,7 +94,7 @@ func (s SqlChannelStore) createInitialSidebarCategoriesT(transaction *gorp.Trans
 	if !hasCategoryOfType[model.SidebarCategoryDirectMessages] {
 		if err := transaction.Insert(&model.SidebarCategory{
 			DisplayName: "Direct Messages", // This will be retranslateed by the client into the user's locale
-			Id:          model.NewId(),
+			Id:          directMessagesCategoryId,
 			UserId:      userId,
 			TeamId:      teamId,
 			Sorting:     model.SidebarCategorySortRecent,

store/sqlstore/upgrade.go

@@ -20,6 +20,7 @@ import (
 
 const (
 	CURRENT_SCHEMA_VERSION   = VERSION_5_28_1
+	VERSION_5_29_0           = "5.29.0"
 	VERSION_5_28_1           = "5.28.1"
 	VERSION_5_28_0           = "5.28.0"
 	VERSION_5_27_0           = "5.27.0"
@@ -190,6 +191,7 @@ func upgradeDatabase(sqlStore SqlStore, currentModelVersionString string) error
 	upgradeDatabaseToVersion527(sqlStore)
 	upgradeDatabaseToVersion528(sqlStore)
 	upgradeDatabaseToVersion5281(sqlStore)
+	upgradeDatabaseToVersion529(sqlStore)
 
 	return nil
 }
@@ -915,3 +917,15 @@ func precheckMigrationToVersion528(sqlStore SqlStore) error {
 
 	return nil
 }
+
+func upgradeDatabaseToVersion529(sqlStore SqlStore) {
+	// if shouldPerformUpgrade(sqlStore, VERSION_5_28_0, VERSION_5_29_0) {
+
+	sqlStore.AlterColumnTypeIfExists("SidebarCategories", "Id", "VARCHAR(128)", "VARCHAR(128)")
+	sqlStore.AlterColumnDefaultIfExists("SidebarCategories", "Id", model.NewString(""), nil)
+	sqlStore.AlterColumnTypeIfExists("SidebarChannels", "CategoryId", "VARCHAR(128)", "VARCHAR(128)")
+	sqlStore.AlterColumnDefaultIfExists("SidebarChannels", "CategoryId", model.NewString(""), nil)
+
+	// 	saveSchemaVersion(sqlStore, VERSION_5_29_0)
+	// }
+}

store/storetest/channel_store_categories.go

@@ -6,6 +6,7 @@ package storetest
 import (
 	"database/sql"
 	"errors"
+	"sync"
 	"testing"
 
 	"github.com/mattermost/mattermost-server/v5/model"
@@ -99,6 +100,29 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) {
 		assert.Equal(t, initialCategories.Categories, res.Categories)
 	})
 
+	t.Run("shouldn't create additional categories when ones already exist even when ran simultaneously", func(t *testing.T) {
+		userId := model.NewId()
+		teamId := model.NewId()
+
+		var wg sync.WaitGroup
+
+		for i := 0; i < 10; i++ {
+			wg.Add(1)
+
+			go func() {
+				defer wg.Done()
+
+				_ = ss.Channel().CreateInitialSidebarCategories(userId, teamId)
+			}()
+		}
+
+		wg.Wait()
+
+		res, err := ss.Channel().GetSidebarCategories(userId, teamId)
+		assert.Nil(t, err)
+		assert.Len(t, res.Categories, 3)
+	})
+
 	t.Run("should populate the Favorites category with regular channels", func(t *testing.T) {
 		userId := model.NewId()
 		teamId := model.NewId()

web/context.go

@@ -301,7 +301,7 @@ func (c *Context) RequireCategoryId() *Context {
 		return c
 	}
 
-	if len(c.Params.CategoryId) != 26 {
+	if !model.IsValidCategoryId(c.Params.CategoryId) {
 		c.SetInvalidUrlParam("category_id")
 	}
 	return c