[MM-56904] Reduce the number of api requests made to fetch user information for GMs on page load (#27149)

* use new endpoint to fetch group members
2024-07-25 15:57:23 -04:00
--- a/server/channels/store/opentracinglayer/opentracinglayer.go
+++ b/server/channels/store/opentracinglayer/opentracinglayer.go
@@ -7378,6 +7378,24 @@ func (s *OpenTracingLayerPreferenceStore) DeleteCategoryAndName(category string,
 	return err
 }

+func (s *OpenTracingLayerPreferenceStore) DeleteInvalidVisibleDmsGms() (int64, error) {
+	origCtx := s.Root.Store.Context()
+	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PreferenceStore.DeleteInvalidVisibleDmsGms")
+	s.Root.Store.SetContext(newCtx)
+	defer func() {
+		s.Root.Store.SetContext(origCtx)
+	}()
+
+	defer span.Finish()
+	result, err := s.PreferenceStore.DeleteInvalidVisibleDmsGms()
+	if err != nil {
+		span.LogFields(spanlog.Error(err))
+		ext.Error.Set(span, true)
+	}
+
+	return result, err
+}
+
 func (s *OpenTracingLayerPreferenceStore) DeleteOrphanedRows(limit int) (int64, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PreferenceStore.DeleteOrphanedRows")
--- a/server/channels/store/retrylayer/retrylayer.go
+++ b/server/channels/store/retrylayer/retrylayer.go
@@ -8384,6 +8384,27 @@ func (s *RetryLayerPreferenceStore) DeleteCategoryAndName(category string, name

 }

+func (s *RetryLayerPreferenceStore) DeleteInvalidVisibleDmsGms() (int64, error) {
+
+	tries := 0
+	for {
+		result, err := s.PreferenceStore.DeleteInvalidVisibleDmsGms()
+		if err == nil {
+			return result, nil
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+		timepkg.Sleep(100 * timepkg.Millisecond)
+	}
+
+}
+
 func (s *RetryLayerPreferenceStore) DeleteOrphanedRows(limit int) (int64, error) {

 	tries := 0
--- a/server/channels/store/sqlstore/preference_store.go
+++ b/server/channels/store/sqlstore/preference_store.go
@@ -315,3 +315,54 @@ func (s SqlPreferenceStore) CleanupFlagsBatch(limit int64) (int64, error) {

 	return rowsAffected, nil
 }
+
+// Delete preference for limit_visible_dms_gms where their value is greater than "40" or less than "1"
+func (s SqlPreferenceStore) DeleteInvalidVisibleDmsGms() (int64, error) {
+	var queryString string
+	var args []interface{}
+	var err error
+
+	// We need to pad the value field with zeros when doing comparison's because the value is stored as a string.
+	// Having them the same length allows Postgres/MySQL to compare them correctly.
+	whereClause := sq.And{
+		sq.Eq{"Category": model.PreferenceCategorySidebarSettings},
+		sq.Eq{"Name": model.PreferenceLimitVisibleDmsGms},
+		sq.Or{
+			sq.Gt{"SUBSTRING(CONCAT('000000000000000', Value), LENGTH(Value) + 1, 15)": "000000000000040"},
+			sq.Lt{"SUBSTRING(CONCAT('000000000000000', Value), LENGTH(Value) + 1, 15)": "000000000000001"},
+		},
+	}
+	if s.DriverName() == "postgres" {
+		subQuery := s.getQueryBuilder().
+			Select("UserId, Category, Name").
+			From("Preferences").
+			Where(whereClause).
+			Limit(100)
+		queryString, args, err = s.getQueryBuilder().
+			Delete("Preferences").
+			Where(sq.Expr("(userid, category, name) IN (?)", subQuery)).
+			ToSql()
+		if err != nil {
+			return int64(0), errors.Wrap(err, "could not build sql query to delete preference")
+		}
+	} else {
+		queryString, args, err = s.getQueryBuilder().
+			Delete("Preferences").
+			Where(whereClause).
+			Limit(100).
+			ToSql()
+		if err != nil {
+			return int64(0), errors.Wrap(err, "could not build sql query to delete preference")
+		}
+	}
+
+	result, err := s.GetMasterX().Exec(queryString, args...)
+	if err != nil {
+		return 0, errors.Wrap(err, "failed to delete Preference")
+	}
+	rowsAffected, err := result.RowsAffected()
+	if err != nil {
+		return 0, errors.Wrap(err, "unable to get rows affected")
+	}
+	return rowsAffected, nil
+}
--- a/server/channels/store/sqlstore/preference_store_test.go
+++ b/server/channels/store/sqlstore/preference_store_test.go
@@ -15,7 +15,7 @@ import (
 )

 func TestPreferenceStore(t *testing.T) {
-	StoreTest(t, storetest.TestPreferenceStore)
+	StoreTestWithSqlStore(t, storetest.TestPreferenceStore)
 }

 func TestDeleteUnusedFeatures(t *testing.T) {
--- a/server/channels/store/store.go
+++ b/server/channels/store/store.go
@@ -652,6 +652,7 @@ type PreferenceStore interface {
 	PermanentDeleteByUser(userID string) error
 	DeleteOrphanedRows(limit int) (deleted int64, err error)
 	CleanupFlagsBatch(limit int64) (int64, error)
+	DeleteInvalidVisibleDmsGms() (int64, error)
 }

 type LicenseStore interface {
--- a/server/channels/store/storetest/mocks/PreferenceStore.go
+++ b/server/channels/store/storetest/mocks/PreferenceStore.go
@@ -96,6 +96,34 @@ func (_m *PreferenceStore) DeleteCategoryAndName(category string, name string) e
 	return r0
 }

+// DeleteInvalidVisibleDmsGms provides a mock function with given fields:
+func (_m *PreferenceStore) DeleteInvalidVisibleDmsGms() (int64, error) {
+	ret := _m.Called()
+
+	if len(ret) == 0 {
+		panic("no return value specified for DeleteInvalidVisibleDmsGms")
+	}
+
+	var r0 int64
+	var r1 error
+	if rf, ok := ret.Get(0).(func() (int64, error)); ok {
+		return rf()
+	}
+	if rf, ok := ret.Get(0).(func() int64); ok {
+		r0 = rf()
+	} else {
+		r0 = ret.Get(0).(int64)
+	}
+
+	if rf, ok := ret.Get(1).(func() error); ok {
+		r1 = rf()
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // DeleteOrphanedRows provides a mock function with given fields: limit
 func (_m *PreferenceStore) DeleteOrphanedRows(limit int) (int64, error) {
 	ret := _m.Called(limit)
--- a/server/channels/store/storetest/preference_store.go
+++ b/server/channels/store/storetest/preference_store.go
@@ -14,7 +14,7 @@ import (
 	"github.com/mattermost/mattermost/server/v8/channels/store"
 )

-func TestPreferenceStore(t *testing.T, rctx request.CTX, ss store.Store) {
+func TestPreferenceStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) {
 	t.Run("PreferenceSave", func(t *testing.T) { testPreferenceSave(t, rctx, ss) })
 	t.Run("PreferenceGet", func(t *testing.T) { testPreferenceGet(t, rctx, ss) })
 	t.Run("PreferenceGetCategory", func(t *testing.T) { testPreferenceGetCategory(t, rctx, ss) })
@@ -24,6 +24,7 @@ func TestPreferenceStore(t *testing.T, rctx request.CTX, ss store.Store) {
 	t.Run("PreferenceDeleteCategory", func(t *testing.T) { testPreferenceDeleteCategory(t, rctx, ss) })
 	t.Run("PreferenceDeleteCategoryAndName", func(t *testing.T) { testPreferenceDeleteCategoryAndName(t, rctx, ss) })
 	t.Run("PreferenceDeleteOrphanedRows", func(t *testing.T) { testPreferenceDeleteOrphanedRows(t, rctx, ss) })
+	t.Run("PreferenceDeleteInvalidVisibleDmsGms", func(t *testing.T) { testDeleteInvalidVisibleDmsGms(t, rctx, ss, s) })
 }

 func testPreferenceSave(t *testing.T, rctx request.CTX, ss store.Store) {
@@ -401,3 +402,78 @@ func testPreferenceDeleteOrphanedRows(t *testing.T, rctx request.CTX, ss store.S
 	_, nErr = ss.Preference().Get(userId, category, preference2.Name)
 	assert.NoError(t, nErr, "newer preference should not have been deleted")
 }
+
+func testDeleteInvalidVisibleDmsGms(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) {
+	userId1 := model.NewId()
+	userId2 := model.NewId()
+	userId3 := model.NewId()
+	userId4 := model.NewId()
+	category := model.PreferenceCategorySidebarSettings
+	name := model.PreferenceLimitVisibleDmsGms
+
+	preferences := model.Preferences{
+		{
+			UserId:   userId1,
+			Category: category,
+			Name:     name,
+			Value:    "10000",
+		},
+		{
+			UserId:   userId2,
+			Category: category,
+			Name:     name,
+			Value:    "40",
+		},
+		{
+			UserId:   userId3,
+			Category: category,
+			Name:     name,
+			Value:    "invalid",
+		},
+		{
+			UserId:   model.NewId(),
+			Category: category,
+			Name:     name,
+			Value:    "-10",
+		},
+		{
+			UserId:   model.NewId(),
+			Category: category,
+			Name:     name,
+			Value:    "0",
+		},
+		{
+			UserId:   model.NewId(),
+			Category: category,
+			Name:     name,
+			Value:    "00000",
+		},
+		{
+			UserId:   userId4,
+			Category: category,
+			Name:     name,
+			Value:    "20",
+		},
+	}
+
+	// Can't insert with Save methods because the values are invalid
+	_, execerr := s.GetMasterX().NamedExec(`
+		INSERT INTO
+		    Preferences(UserId, Category, Name, Value)
+		VALUES
+		    (:UserId, :Category, :Name, :Value);
+	`, preferences)
+	require.NoError(t, execerr)
+
+	count, err := ss.Preference().DeleteInvalidVisibleDmsGms()
+	require.NoError(t, err)
+	assert.Equal(t, int64(5), count)
+
+	preference, err := ss.Preference().Get(userId2, category, name)
+	require.NoError(t, err)
+	require.Equal(t, &preferences[1], preference)
+
+	preference, err = ss.Preference().Get(userId4, category, name)
+	require.NoError(t, err)
+	require.Equal(t, &preferences[6], preference)
+}
--- a/server/channels/store/timerlayer/timerlayer.go
+++ b/server/channels/store/timerlayer/timerlayer.go
@@ -6665,6 +6665,22 @@ func (s *TimerLayerPreferenceStore) DeleteCategoryAndName(category string, name
 	return err
 }

+func (s *TimerLayerPreferenceStore) DeleteInvalidVisibleDmsGms() (int64, error) {
+	start := time.Now()
+
+	result, err := s.PreferenceStore.DeleteInvalidVisibleDmsGms()
+
+	elapsed := float64(time.Since(start)) / float64(time.Second)
+	if s.Root.Metrics != nil {
+		success := "false"
+		if err == nil {
+			success = "true"
+		}
+		s.Root.Metrics.ObserveStoreMethodDuration("PreferenceStore.DeleteInvalidVisibleDmsGms", success, elapsed)
+	}
+	return result, err
+}
+
 func (s *TimerLayerPreferenceStore) DeleteOrphanedRows(limit int) (int64, error) {
 	start := time.Now()