From 84a83a1e0f4d2dbd7aa28766acd1e0049cc14094 Mon Sep 17 00:00:00 2001 From: Arya Khochare <91268931+Aryakoste@users.noreply.github.com> Date: Wed, 22 Jan 2025 21:29:41 +0530 Subject: [PATCH] [MM-62142] Avoid SELECT * in status_store.go (#29610) * SELECT * to query builder * execBuilder, manual column name and one more select * remaining instance --------- Co-authored-by: Mattermost Build --- .../channels/store/sqlstore/status_store.go | 64 +++++++++++++------ 1 file changed, 43 insertions(+), 21 deletions(-) diff --git a/server/channels/store/sqlstore/status_store.go b/server/channels/store/sqlstore/status_store.go index 40f55773cc..d41cd3f156 100644 --- a/server/channels/store/sqlstore/status_store.go +++ b/server/channels/store/sqlstore/status_store.go @@ -17,10 +17,20 @@ import ( type SqlStatusStore struct { *SqlStore + + statusSelectQuery sq.SelectBuilder } func newSqlStatusStore(sqlStore *SqlStore) store.StatusStore { - return &SqlStatusStore{sqlStore} + s := SqlStatusStore{ + SqlStore: sqlStore, + } + + s.statusSelectQuery = s.getQueryBuilder(). + Select("UserId", "Status", quoteColumnName(s.DriverName(), "Manual"), "LastActivityAt", "DNDEndTime", "PrevStatus"). + From("Status") + + return &s } func (s SqlStatusStore) SaveOrUpdate(st *model.Status) error { @@ -50,9 +60,10 @@ func (s SqlStatusStore) SaveOrUpdate(st *model.Status) error { } func (s SqlStatusStore) Get(userId string) (*model.Status, error) { - var status model.Status + query := s.statusSelectQuery.Where(sq.Eq{"UserId": userId}) - if err := s.GetReplica().Get(&status, "SELECT * FROM Status WHERE UserId = ?", userId); err != nil { + var status model.Status + if err := s.GetReplica().GetBuilder(&status, query); err != nil { if err == sql.ErrNoRows { return nil, store.NewErrNotFound("Status", fmt.Sprintf("userId=%s", userId)) } @@ -62,10 +73,7 @@ func (s SqlStatusStore) Get(userId string) (*model.Status, error) { } func (s SqlStatusStore) GetByIds(userIds []string) ([]*model.Status, error) { - query := s.getQueryBuilder(). - Select(fmt.Sprintf("UserId, Status, %s, LastActivityAt", quoteColumnName(s.DriverName(), "Manual"))). - From("Status"). - Where(sq.Eq{"UserId": userIds}) + query := s.statusSelectQuery.Where(sq.Eq{"UserId": userIds}) queryString, args, err := query.ToSql() if err != nil { return nil, errors.Wrap(err, "status_tosql") @@ -78,7 +86,7 @@ func (s SqlStatusStore) GetByIds(userIds []string) ([]*model.Status, error) { defer rows.Close() for rows.Next() { var status model.Status - if err = rows.Scan(&status.UserId, &status.Status, &status.Manual, &status.LastActivityAt); err != nil { + if err = rows.Scan(&status.UserId, &status.Status, &status.Manual, &status.LastActivityAt, &status.DNDEndTime, &status.PrevStatus); err != nil { return nil, errors.Wrap(err, "unable to scan from rows") } statuses = append(statuses, &status) @@ -94,20 +102,19 @@ func (s SqlStatusStore) GetByIds(userIds []string) ([]*model.Status, error) { func (s SqlStatusStore) updateExpiredStatuses(t *sqlxTxWrapper) ([]*model.Status, error) { statuses := []*model.Status{} currUnixTime := time.Now().UTC().Unix() - selectQuery, selectParams, err := s.getQueryBuilder(). - Select("*"). - From("Status"). - Where( - sq.And{ - sq.Eq{"Status": model.StatusDnd}, - sq.Gt{"DNDEndTime": 0}, - sq.LtOrEq{"DNDEndTime": currUnixTime}, - }, - ).ToSql() + selectQuery := s.statusSelectQuery.Where( + sq.And{ + sq.Eq{"Status": model.StatusDnd}, + sq.Gt{"DNDEndTime": 0}, + sq.LtOrEq{"DNDEndTime": currUnixTime}, + }, + ) + + selectQueryString, selectParams, err := selectQuery.ToSql() if err != nil { return nil, errors.Wrap(err, "status_tosql") } - err = t.Select(&statuses, selectQuery, selectParams...) + err = t.Select(&statuses, selectQueryString, selectParams...) if err != nil { return nil, errors.Wrap(err, "updateExpiredStatusesT: failed to get expired dnd statuses") } @@ -212,8 +219,18 @@ func (s SqlStatusStore) ResetAll() error { func (s SqlStatusStore) GetTotalActiveUsersCount() (int64, error) { time := model.GetMillis() - (1000 * 60 * 60 * 24) + query := s.getQueryBuilder(). + Select("COUNT(UserId)"). + From("Status"). + Where(sq.Gt{"LastActivityAt": time}) + + queryString, args, err := query.ToSql() + if err != nil { + return 0, errors.Wrap(err, "status_tosql") + } + var count int64 - err := s.GetReplica().Get(&count, "SELECT COUNT(UserId) FROM Status WHERE LastActivityAt > ?", time) + err = s.GetReplica().Get(&count, queryString, args...) if err != nil { return count, errors.Wrap(err, "failed to count active users") } @@ -221,7 +238,12 @@ func (s SqlStatusStore) GetTotalActiveUsersCount() (int64, error) { } func (s SqlStatusStore) UpdateLastActivityAt(userId string, lastActivityAt int64) error { - if _, err := s.GetMaster().Exec("UPDATE Status SET LastActivityAt = ? WHERE UserId = ?", lastActivityAt, userId); err != nil { + builder := s.getQueryBuilder(). + Update("Status"). + Set("LastActivityAt", lastActivityAt). + Where(sq.Eq{"UserId": userId}) + + if _, err := s.GetMaster().ExecBuilder(builder); err != nil { return errors.Wrapf(err, "failed to update last activity for userId=%s", userId) }