From caa30c9bc48341f604bfce4459a23ee428871d1d Mon Sep 17 00:00:00 2001 From: Tim Scheuermann Date: Thu, 10 Mar 2022 20:57:09 +0100 Subject: [PATCH] MM-19336 Migrated GetMembersByIds and GetMembersByChannelIds to Squirrel (#19728) * Ported GetMembersByIds and GetMembersByChannelIds to Squirrel * Added checks for empty ID lists * Updated GetAllChannelMembersById to use Squirrel * Added a method to get the query placeholder directly * Migrated UpdateMultipleMembers to use a Squirrel query * Migrated UpdateMultipleMembers to Squirrel * Initialize a prepared query builder for channelMembersForTeamWithSchemeSelectQuery * Migrated GetMembersForUser to Squirrel * Slight improvement for constructMySQLJSONArgs * Migrated UpdateMemberNotifyProps to Squirrel * Migrated GetMembers to Squirrel * Migrated GetMember to Squirrel * Avoid shadowing err * Don't set query builder on copy of SqlChannelStore * Fixed typo in error message * Fixed missing elipsis * Shorter SQL generation for GetAllChannelMembersById * Don't unnecessarily copy the reference * Use a function to generate the ChannelMember map * Avoid shadowing err * Don't use IN for matching multiple possible values * Initialize the members lists * Remove check for empty channelIDs list * Fixed test and removed check for empty list * Use err2 rather than eerr Co-authored-by: Tim Scheuermann Co-authored-by: Mattermod --- store/sqlstore/channel_store.go | 286 ++++++++++++++++++--------- store/sqlstore/channel_store_test.go | 31 ++- store/sqlstore/store.go | 9 +- store/sqlstore/utils.go | 5 +- store/storetest/channel_store.go | 5 +- 5 files changed, 215 insertions(+), 121 deletions(-) diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 08456c4912..326782ffce 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -35,6 +35,9 @@ const ( type SqlChannelStore struct { *SqlStore metrics einterfaces.MetricsInterface + + // prepared query builders for use in multiple methods + channelMembersForTeamWithSchemeSelectQuery sq.SelectBuilder } type channelMember struct { @@ -53,21 +56,21 @@ type channelMember struct { MsgCountRoot int64 } -func NewChannelMemberFromModel(cm *model.ChannelMember) *channelMember { - return &channelMember{ - ChannelId: cm.ChannelId, - UserId: cm.UserId, - Roles: cm.ExplicitRoles, - LastViewedAt: cm.LastViewedAt, - MsgCount: cm.MsgCount, - MentionCount: cm.MentionCount, - MentionCountRoot: cm.MentionCountRoot, - MsgCountRoot: cm.MsgCountRoot, - NotifyProps: cm.NotifyProps, - LastUpdateAt: cm.LastUpdateAt, - SchemeGuest: sql.NullBool{Valid: true, Bool: cm.SchemeGuest}, - SchemeUser: sql.NullBool{Valid: true, Bool: cm.SchemeUser}, - SchemeAdmin: sql.NullBool{Valid: true, Bool: cm.SchemeAdmin}, +func NewMapFromChannelMemberModel(cm *model.ChannelMember) map[string]interface{} { + return map[string]interface{}{ + "ChannelId": cm.ChannelId, + "UserId": cm.UserId, + "Roles": cm.ExplicitRoles, + "LastViewedAt": cm.LastViewedAt, + "MsgCount": cm.MsgCount, + "MentionCount": cm.MentionCount, + "MentionCountRoot": cm.MentionCountRoot, + "MsgCountRoot": cm.MsgCountRoot, + "NotifyProps": cm.NotifyProps, + "LastUpdateAt": cm.LastUpdateAt, + "SchemeGuest": sql.NullBool{Valid: true, Bool: cm.SchemeGuest}, + "SchemeUser": sql.NullBool{Valid: true, Bool: cm.SchemeUser}, + "SchemeAdmin": sql.NullBool{Valid: true, Bool: cm.SchemeAdmin}, } } @@ -454,10 +457,32 @@ func (s SqlChannelStore) ClearCaches() { } func newSqlChannelStore(sqlStore *SqlStore, metrics einterfaces.MetricsInterface) store.ChannelStore { - return &SqlChannelStore{ + s := &SqlChannelStore{ SqlStore: sqlStore, metrics: metrics, } + + s.initializeQueries() + + return s +} + +func (s *SqlChannelStore) initializeQueries() { + s.channelMembersForTeamWithSchemeSelectQuery = s.getQueryBuilder(). + Select( + "ChannelMembers.*", + "TeamScheme.DefaultChannelGuestRole TeamSchemeDefaultGuestRole", + "TeamScheme.DefaultChannelUserRole TeamSchemeDefaultUserRole", + "TeamScheme.DefaultChannelAdminRole TeamSchemeDefaultAdminRole", + "ChannelScheme.DefaultChannelGuestRole ChannelSchemeDefaultGuestRole", + "ChannelScheme.DefaultChannelUserRole ChannelSchemeDefaultUserRole", + "ChannelScheme.DefaultChannelAdminRole ChannelSchemeDefaultAdminRole", + ). + From("ChannelMembers"). + InnerJoin("Channels ON ChannelMembers.ChannelId = Channels.Id"). + LeftJoin("Schemes ChannelScheme ON Channels.SchemeId = ChannelScheme.Id"). + LeftJoin("Teams ON Channels.TeamId = Teams.Id"). + LeftJoin("Schemes TeamScheme ON Teams.SchemeId = TeamScheme.Id") } func (s SqlChannelStore) upsertPublicChannelT(transaction *sqlxTxWrapper, channel *model.Channel) error { @@ -1136,15 +1161,22 @@ func (s SqlChannelStore) GetChannelsByUser(userId string, includeDeleted bool, l } func (s SqlChannelStore) GetAllChannelMembersById(channelID string) ([]string, error) { + sql, args, err := s.channelMembersForTeamWithSchemeSelectQuery.Where(sq.Eq{ + "ChannelId": channelID, + }).ToSql() + if err != nil { + return nil, errors.Wrap(err, "GetAllChannelMembersById_ToSql") + } + dbMembers := channelMemberWithSchemeRolesList{} - err := s.GetReplicaX().Select(&dbMembers, channelMembersForTeamWithSchemeSelectQuery+"WHERE ChannelId = ?", channelID) + err = s.GetReplicaX().Select(&dbMembers, sql, args...) if err != nil { return nil, errors.Wrapf(err, "failed to get ChannelMembers with channelID=%s", channelID) } - res := make([]string, 0, len(dbMembers)) - for _, member := range dbMembers.ToModel() { - res = append(res, member.UserId) + res := make([]string, len(dbMembers)) + for i, member := range dbMembers.ToModel() { + res[i] = member.UserId } return res, nil @@ -1558,27 +1590,6 @@ func (s SqlChannelStore) GetDeleted(teamId string, offset int, limit int, userId return channels, nil } -var channelMembersForTeamWithSchemeSelectQuery = ` - SELECT - ChannelMembers.*, - TeamScheme.DefaultChannelGuestRole TeamSchemeDefaultGuestRole, - TeamScheme.DefaultChannelUserRole TeamSchemeDefaultUserRole, - TeamScheme.DefaultChannelAdminRole TeamSchemeDefaultAdminRole, - ChannelScheme.DefaultChannelGuestRole ChannelSchemeDefaultGuestRole, - ChannelScheme.DefaultChannelUserRole ChannelSchemeDefaultUserRole, - ChannelScheme.DefaultChannelAdminRole ChannelSchemeDefaultAdminRole - FROM - ChannelMembers - INNER JOIN - Channels ON ChannelMembers.ChannelId = Channels.Id - LEFT JOIN - Schemes ChannelScheme ON Channels.SchemeId = ChannelScheme.Id - LEFT JOIN - Teams ON Channels.TeamId = Teams.Id - LEFT JOIN - Schemes TeamScheme ON Teams.SchemeId = TeamScheme.Id -` - var channelMembersWithSchemeSelectQuery = ` SELECT ChannelMembers.*, @@ -1792,25 +1803,35 @@ func (s SqlChannelStore) UpdateMultipleMembers(members []*model.ChannelMember) ( updatedMembers := []*model.ChannelMember{} for _, member := range members { - if _, err := transaction.NamedExec(`UPDATE ChannelMembers - SET Roles=:Roles, - LastViewedAt=:LastViewedAt, - MsgCount=:MsgCount, - MentionCount=:MentionCount, - NotifyProps=:NotifyProps, - LastUpdateAt=:LastUpdateAt, - SchemeUser=:SchemeUser, - SchemeAdmin=:SchemeAdmin, - SchemeGuest=:SchemeGuest, - MentionCountRoot=:MentionCountRoot, - MsgCountRoot=:MsgCountRoot - WHERE ChannelId=:ChannelId AND UserId=:UserId`, NewChannelMemberFromModel(member)); err != nil { + update := s.getQueryBuilder(). + Update("ChannelMembers"). + SetMap(NewMapFromChannelMemberModel(member)). + Where(sq.Eq{ + "ChannelId": member.ChannelId, + "UserId": member.UserId, + }) + + sqlUpdate, args, err := update.ToSql() + if err != nil { + return nil, errors.Wrapf(err, "UpdateMultipleMembers_Update_ToSql ChannelID=%s UserID=%s", member.ChannelId, member.UserId) + } + + if _, err = transaction.Exec(sqlUpdate, args...); err != nil { return nil, errors.Wrap(err, "failed to update ChannelMember") } + sqlSelect, args, err := s.channelMembersForTeamWithSchemeSelectQuery. + Where(sq.Eq{ + "ChannelMembers.ChannelId": member.ChannelId, + "ChannelMembers.UserId": member.UserId, + }).ToSql() + if err != nil { + return nil, errors.Wrapf(err, "UpdateMultipleMembers_Select_ToSql ChannelID=%s UserID=%s", member.ChannelId, member.UserId) + } + // TODO: Get this out of the transaction when is possible var dbMember channelMemberWithSchemeRoles - if err := transaction.Get(&dbMember, channelMembersForTeamWithSchemeSelectQuery+"WHERE ChannelMembers.ChannelId = ? AND ChannelMembers.UserId = ?", member.ChannelId, member.UserId); err != nil { + if err := transaction.Get(&dbMember, sqlSelect, args...); err != nil { if err == sql.ErrNoRows { return nil, store.NewErrNotFound("ChannelMember", fmt.Sprintf("channelId=%s, userId=%s", member.ChannelId, member.UserId)) } @@ -1841,31 +1862,57 @@ func (s SqlChannelStore) UpdateMemberNotifyProps(channelID, userID string, props defer finalizeTransactionX(tx) if s.DriverName() == model.DatabaseDriverPostgres { - _, err = tx.Exec(`UPDATE channelmembers - SET notifyprops = notifyprops || ?::jsonb - WHERE userid=? AND channelid=?`, model.MapToJSON(props), userID, channelID) - } else { + sql, args, err2 := s.getQueryBuilder(). + Update("channelmembers"). + Set("notifyprops", sq.Expr("notifyprops || ?::jsonb", model.MapToJSON(props))). + Where(sq.Eq{ + "userid": userID, + "channelid": channelID, + }).ToSql() + if err2 != nil { + return nil, errors.Wrapf(err, "UpdateMemberNotifyProps_Update_Postgres_ToSql channelID=%s and userID=%s", channelID, userID) + } + + _, err = tx.Exec(sql, args...) + } else if len(props) > 0 { // It's difficult to construct a SQL query for MySQL // to handle a case of empty map. So we just ignore it. - if len(props) > 0 { - // unpack the keys and values to pass to MySQL. - args, argString := constructMySQLJSONArgs(props) - args = append(args, userID, channelID) - // Example: UPDATE ChannelMembers - // SET NotifyProps = JSON_SET(NotifyProps, '$.mark_unread', '"yes"' [, ...]) - // WHERE ... - _, err = tx.Exec(`UPDATE ChannelMembers - SET NotifyProps = JSON_SET(NotifyProps, `+argString+`) - WHERE UserId=? AND ChannelId=?`, args...) + // unpack the keys and values to pass to MySQL. + jsonArgs, jsonSQL := constructMySQLJSONArgs(props) + jsonExpr := sq.Expr(fmt.Sprintf("JSON_SET(NotifyProps, %s)", jsonSQL), jsonArgs...) + + // Example: UPDATE ChannelMembers + // SET NotifyProps = JSON_SET(NotifyProps, '$.mark_unread', '"yes"' [, ...]) + // WHERE ... + sql, args, err2 := s.getQueryBuilder(). + Update("ChannelMembers"). + Set("NotifyProps", jsonExpr). + Where(sq.Eq{ + "UserId": userID, + "ChannelId": channelID, + }).ToSql() + if err2 != nil { + return nil, errors.Wrapf(err, "UpdateMemberNotifyProps_Update_MySQL_ToSql channelID=%s and userID=%s", channelID, userID) } + + _, err = tx.Exec(sql, args...) } if err != nil { return nil, errors.Wrapf(err, "failed to update ChannelMember with channelID=%s and userID=%s", channelID, userID) } + selectSQL, args, err := s.channelMembersForTeamWithSchemeSelectQuery. + Where(sq.Eq{ + "ChannelMembers.ChannelId": channelID, + "ChannelMembers.UserId": userID, + }).ToSql() + if err != nil { + return nil, errors.Wrapf(err, "UpdateMemberNotifyProps_Select_ToSql channelID=%s and userID=%s", channelID, userID) + } + var dbMember channelMemberWithSchemeRoles - if err2 := tx.Get(&dbMember, channelMembersForTeamWithSchemeSelectQuery+"WHERE ChannelMembers.ChannelId = ? AND ChannelMembers.UserId = ?", channelID, userID); err2 != nil { + if err2 := tx.Get(&dbMember, selectSQL, args...); err2 != nil { if err2 == sql.ErrNoRows { return nil, store.NewErrNotFound("ChannelMember", fmt.Sprintf("channelId=%s, userId=%s", channelID, userID)) } @@ -1879,11 +1926,22 @@ func (s SqlChannelStore) UpdateMemberNotifyProps(channelID, userID string, props return dbMember.ToModel(), err } -func (s SqlChannelStore) GetMembers(channelId string, offset, limit int) (model.ChannelMembers, error) { - dbMembers := channelMemberWithSchemeRolesList{} - err := s.GetReplicaX().Select(&dbMembers, channelMembersForTeamWithSchemeSelectQuery+"WHERE ChannelId = ? LIMIT ? OFFSET ?", channelId, limit, offset) +func (s SqlChannelStore) GetMembers(channelID string, offset, limit int) (model.ChannelMembers, error) { + sql, args, err := s.channelMembersForTeamWithSchemeSelectQuery. + Where(sq.Eq{ + "ChannelId": channelID, + }). + Limit(uint64(limit)). + Offset(uint64(offset)). + ToSql() if err != nil { - return nil, errors.Wrapf(err, "failed to get ChannelMembers with channelId=%s", channelId) + return nil, errors.Wrapf(err, "GetMember_ToSql ChannelID=%s", channelID) + } + + dbMembers := channelMemberWithSchemeRolesList{} + err = s.GetReplicaX().Select(&dbMembers, sql, args...) + if err != nil { + return nil, errors.Wrapf(err, "failed to get ChannelMembers with channelId=%s", channelID) } return dbMembers.ToModel(), nil @@ -1908,14 +1966,23 @@ func (s SqlChannelStore) GetChannelMembersTimezones(channelId string) ([]model.S return dbMembersTimezone, nil } -func (s SqlChannelStore) GetMember(ctx context.Context, channelId string, userId string) (*model.ChannelMember, error) { +func (s SqlChannelStore) GetMember(ctx context.Context, channelID string, userID string) (*model.ChannelMember, error) { + selectSQL, args, err := s.channelMembersForTeamWithSchemeSelectQuery. + Where(sq.Eq{ + "ChannelMembers.ChannelId": channelID, + "ChannelMembers.UserId": userID, + }).ToSql() + if err != nil { + return nil, errors.Wrapf(err, "GetMember_ToSql ChannelID=%s UserID=%s", channelID, userID) + } + var dbMember channelMemberWithSchemeRoles - if err := s.DBXFromContext(ctx).Get(&dbMember, channelMembersForTeamWithSchemeSelectQuery+"WHERE ChannelMembers.ChannelId = ? AND ChannelMembers.UserId = ?", channelId, userId); err != nil { + if err := s.DBXFromContext(ctx).Get(&dbMember, selectSQL, args...); err != nil { if err == sql.ErrNoRows { - return nil, store.NewErrNotFound("ChannelMember", fmt.Sprintf("channelId=%s, userId=%s", channelId, userId)) + return nil, store.NewErrNotFound("ChannelMember", fmt.Sprintf("channelId=%s, userId=%s", channelID, userID)) } - return nil, errors.Wrapf(err, "failed to get ChannelMember with channelId=%s and userId=%s", channelId, userId) + return nil, errors.Wrapf(err, "failed to get ChannelMember with channelId=%s and userId=%s", channelID, userID) } return dbMember.ToModel(), nil @@ -2695,11 +2762,24 @@ func (s SqlChannelStore) AnalyticsDeletedTypeCount(teamId string, channelType mo return v, nil } -func (s SqlChannelStore) GetMembersForUser(teamId string, userId string) (model.ChannelMembers, error) { - dbMembers := channelMemberWithSchemeRolesList{} - err := s.GetReplicaX().Select(&dbMembers, channelMembersForTeamWithSchemeSelectQuery+"WHERE ChannelMembers.UserId = ? AND (Teams.Id = ? OR Teams.Id = '' OR Teams.Id IS NULL)", userId, teamId) +func (s SqlChannelStore) GetMembersForUser(teamID string, userID string) (model.ChannelMembers, error) { + sql, args, err := s.channelMembersForTeamWithSchemeSelectQuery. + Where(sq.And{ + sq.Eq{"ChannelMembers.UserId": userID}, + sq.Or{ + sq.Eq{"Teams.Id": teamID}, + sq.Eq{"Teams.Id": ""}, + sq.Eq{"Teams.Id": nil}, + }, + }).ToSql() if err != nil { - return nil, errors.Wrapf(err, "failed to find ChannelMembers data with teamId=%s and userId=%s", teamId, userId) + return nil, errors.Wrapf(err, "GetMembersForUser_ToSql teamID=%s userID=%s", teamID, userID) + } + + dbMembers := channelMemberWithSchemeRolesList{} + err = s.GetReplicaX().Select(&dbMembers, sql, args...) + if err != nil { + return nil, errors.Wrapf(err, "failed to find ChannelMembers data with teamId=%s and userId=%s", teamID, userID) } return dbMembers.ToModel(), nil @@ -3545,29 +3625,43 @@ func (s SqlChannelStore) SearchGroupChannels(userId, term string) (model.Channel return groupChannels, nil } -// TODO: rewrite in squirrel (https://github.com/mattermost/mattermost-server/issues/19336) -func (s SqlChannelStore) GetMembersByIds(channelId string, userIds []string) (model.ChannelMembers, error) { - var dbMembers channelMemberWithSchemeRolesList +func (s SqlChannelStore) GetMembersByIds(channelID string, userIDs []string) (model.ChannelMembers, error) { + query := s.channelMembersForTeamWithSchemeSelectQuery.Where( + sq.Eq{ + "ChannelMembers.ChannelId": channelID, + "ChannelMembers.UserId": userIDs, + }, + ) - keys, props := MapStringsToQueryParams(userIds, "User") - props["ChannelId"] = channelId + sql, args, err := query.ToSql() + if err != nil { + return nil, errors.Wrap(err, "GetMembersByIds_ToSql") + } - if _, err := s.GetReplica().Select(&dbMembers, channelMembersForTeamWithSchemeSelectQuery+"WHERE ChannelMembers.ChannelId = :ChannelId AND ChannelMembers.UserId IN "+keys, props); err != nil { - return nil, errors.Wrapf(err, "failed to find ChannelMembers with channelId=%s and userId in %v", channelId, userIds) + dbMembers := channelMemberWithSchemeRolesList{} + if err := s.GetReplicaX().Select(&dbMembers, sql, args...); err != nil { + return nil, errors.Wrapf(err, "failed to find ChannelMembers with channelId=%s and userId in %v", channelID, userIDs) } return dbMembers.ToModel(), nil } -// TODO: rewrite in squirrel (https://github.com/mattermost/mattermost-server/issues/19336) -func (s SqlChannelStore) GetMembersByChannelIds(channelIds []string, userId string) (model.ChannelMembers, error) { - var dbMembers channelMemberWithSchemeRolesList +func (s SqlChannelStore) GetMembersByChannelIds(channelIDs []string, userID string) (model.ChannelMembers, error) { + query := s.channelMembersForTeamWithSchemeSelectQuery.Where( + sq.Eq{ + "ChannelMembers.ChannelId": channelIDs, + "ChannelMembers.UserId": userID, + }, + ) - keys, props := MapStringsToQueryParams(channelIds, "Channel") - props["UserId"] = userId + sql, args, err := query.ToSql() + if err != nil { + return nil, errors.Wrap(err, "GetMembersByChannelIds_ToSql") + } - if _, err := s.GetReplica().Select(&dbMembers, channelMembersForTeamWithSchemeSelectQuery+"WHERE ChannelMembers.UserId = :UserId AND ChannelMembers.ChannelId IN "+keys, props); err != nil { - return nil, errors.Wrapf(err, "failed to find ChannelMembers with userId=%s and channelId in %v", userId, channelIds) + dbMembers := channelMemberWithSchemeRolesList{} + if err := s.GetReplicaX().Select(&dbMembers, sql, args...); err != nil { + return nil, errors.Wrapf(err, "failed to find ChannelMembers with userId=%s and channelId in %v", userID, channelIDs) } return dbMembers.ToModel(), nil diff --git a/store/sqlstore/channel_store_test.go b/store/sqlstore/channel_store_test.go index ee69b0f662..3f39b4143f 100644 --- a/store/sqlstore/channel_store_test.go +++ b/store/sqlstore/channel_store_test.go @@ -41,12 +41,12 @@ func TestChannelSearchQuerySQLInjection(t *testing.T) { } func TestChannelStoreInternalDataTypes(t *testing.T) { - t.Run("NewChannelMemberFromModel", func(t *testing.T) { testNewChannelMemberFromModel(t) }) + t.Run("NewMapFromChannelMemberModel", func(t *testing.T) { testNewMapFromChannelMemberModel(t) }) t.Run("ChannelMemberWithSchemeRolesToModel", func(t *testing.T) { testChannelMemberWithSchemeRolesToModel(t) }) t.Run("AllChannelMemberProcess", func(t *testing.T) { testAllChannelMemberProcess(t) }) } -func testNewChannelMemberFromModel(t *testing.T) { +func testNewMapFromChannelMemberModel(t *testing.T) { m := model.ChannelMember{ ChannelId: model.NewId(), UserId: model.NewId(), @@ -62,23 +62,20 @@ func testNewChannelMemberFromModel(t *testing.T) { ExplicitRoles: "custom_role", } - db := NewChannelMemberFromModel(&m) + db := NewMapFromChannelMemberModel(&m) - assert.Equal(t, m.ChannelId, db.ChannelId) - assert.Equal(t, m.UserId, db.UserId) - assert.Equal(t, m.LastViewedAt, db.LastViewedAt) - assert.Equal(t, m.MsgCount, db.MsgCount) - assert.Equal(t, m.MentionCount, db.MentionCount) + assert.Equal(t, m.ChannelId, db["ChannelId"]) + assert.Equal(t, m.UserId, db["UserId"]) + assert.Equal(t, m.LastViewedAt, db["LastViewedAt"]) + assert.Equal(t, m.MsgCount, db["MsgCount"]) + assert.Equal(t, m.MentionCount, db["MentionCount"]) assert.Equal(t, int64(0), m.MentionCountRoot) - assert.Equal(t, m.NotifyProps, db.NotifyProps) - assert.Equal(t, m.LastUpdateAt, db.LastUpdateAt) - assert.Equal(t, true, db.SchemeGuest.Valid) - assert.Equal(t, true, db.SchemeUser.Valid) - assert.Equal(t, true, db.SchemeAdmin.Valid) - assert.Equal(t, m.SchemeGuest, db.SchemeGuest.Bool) - assert.Equal(t, m.SchemeUser, db.SchemeUser.Bool) - assert.Equal(t, m.SchemeAdmin, db.SchemeAdmin.Bool) - assert.Equal(t, m.ExplicitRoles, db.Roles) + assert.Equal(t, m.NotifyProps, db["NotifyProps"]) + assert.Equal(t, m.LastUpdateAt, db["LastUpdateAt"]) + assert.Equal(t, sql.NullBool{Bool: false, Valid: true}, db["SchemeGuest"]) + assert.Equal(t, sql.NullBool{Bool: true, Valid: true}, db["SchemeUser"]) + assert.Equal(t, sql.NullBool{Bool: true, Valid: true}, db["SchemeAdmin"]) + assert.Equal(t, m.ExplicitRoles, db["Roles"]) } func testChannelMemberWithSchemeRolesToModel(t *testing.T) { diff --git a/store/sqlstore/store.go b/store/sqlstore/store.go index cc43626017..5bff4e872e 100644 --- a/store/sqlstore/store.go +++ b/store/sqlstore/store.go @@ -961,11 +961,14 @@ func (ss *SqlStore) DropAllTables() { } func (ss *SqlStore) getQueryBuilder() sq.StatementBuilderType { - builder := sq.StatementBuilder.PlaceholderFormat(sq.Question) + return sq.StatementBuilder.PlaceholderFormat(ss.getQueryPlaceholder()) +} + +func (ss *SqlStore) getQueryPlaceholder() sq.PlaceholderFormat { if ss.DriverName() == model.DatabaseDriverPostgres { - builder = builder.PlaceholderFormat(sq.Dollar) + return sq.Dollar } - return builder + return sq.Question } // getSubQueryBuilder is necessary to generate the SQL query and args to pass to sub-queries because squirrel does not support WHERE clause in sub-queries. diff --git a/store/sqlstore/utils.go b/store/sqlstore/utils.go index e3f57513d3..32ed9a07ae 100644 --- a/store/sqlstore/utils.go +++ b/store/sqlstore/utils.go @@ -113,14 +113,13 @@ func constructMySQLJSONArgs(props map[string]string) ([]interface{}, string) { // Unpack the keys and values to pass to MySQL. args := make([]interface{}, 0, len(props)) for k, v := range props { - args = append(args, "$."+k) - args = append(args, v) + args = append(args, "$."+k, v) } // We calculate the number of ? to set in the query string. argString := strings.Repeat("?, ", len(props)*2) // Strip off the trailing comma. - argString = strings.TrimSuffix(strings.TrimSpace(argString), ",") + argString = strings.TrimSuffix(argString, ", ") return args, argString } diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index 1449ce739d..3e7343af75 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -6321,8 +6321,9 @@ func testChannelStoreGetMembersByIds(t *testing.T, ss store.Store) { require.NoError(t, nErr, nErr) require.Len(t, members, 2, "return wrong number of results") - _, nErr = ss.Channel().GetMembersByIds(m1.ChannelId, []string{}) - require.Error(t, nErr, "empty user ids - should have failed") + members, nErr = ss.Channel().GetMembersByIds(m1.ChannelId, []string{}) + require.NoError(t, nErr) + require.Len(t, members, 0) } func testChannelStoreGetMembersByChannelIds(t *testing.T, ss store.Store) {