MM-62158: group store no SELECT * (Part 1) (#30276)

* improved test coverage

* initial pass on removing SELECT * from group store
Этот коммит содержится в:
Jesse Hallam
2025-05-01 09:39:05 -03:00
коммит произвёл GitHub
родитель 80c58a9742
Коммит e1f47e22e7
2 изменённых файлов: 450 добавлений и 154 удалений

Просмотреть файл

@@ -49,10 +49,67 @@ type groupChannelJoin struct {
type SqlGroupStore struct {
*SqlStore
userGroupsSelectQuery sq.SelectBuilder
groupMembersSelectQuery sq.SelectBuilder
groupMemberUsersSelectQuery sq.SelectBuilder
groupTeamsSelectQuery sq.SelectBuilder
groupChannelsSelectQuery sq.SelectBuilder
}
func newSqlGroupStore(sqlStore *SqlStore) store.GroupStore {
return &SqlGroupStore{SqlStore: sqlStore}
s := &SqlGroupStore{SqlStore: sqlStore}
s.userGroupsSelectQuery = s.getQueryBuilder().
Select(
"UserGroups.Id",
"UserGroups.Name",
"UserGroups.DisplayName",
"UserGroups.Description",
"UserGroups.Source",
"UserGroups.RemoteId",
"UserGroups.CreateAt",
"UserGroups.UpdateAt",
"UserGroups.DeleteAt",
"UserGroups.AllowReference",
).
From("UserGroups")
s.groupMembersSelectQuery = s.getQueryBuilder().
Select(
"GroupMembers.GroupId",
"GroupMembers.UserId",
"GroupMembers.CreateAt",
"GroupMembers.DeleteAt",
).From("GroupMembers")
s.groupMemberUsersSelectQuery = s.getQueryBuilder().
Select(getUsersColumns()...).
From("GroupMembers").
Join("Users ON Users.Id = GroupMembers.UserId")
s.groupTeamsSelectQuery = s.getQueryBuilder().
Select(
"GroupTeams.GroupId",
"GroupTeams.TeamId",
"GroupTeams.AutoAdd",
"GroupTeams.SchemeAdmin",
"GroupTeams.CreateAt",
"GroupTeams.UpdateAt",
"GroupTeams.DeleteAt",
).From("GroupTeams")
s.groupChannelsSelectQuery = s.getQueryBuilder().
Select(
"GroupChannels.GroupId",
"GroupChannels.ChannelId",
"GroupChannels.AutoAdd",
"GroupChannels.SchemeAdmin",
"GroupChannels.CreateAt",
"GroupChannels.UpdateAt",
"GroupChannels.DeleteAt",
).From("GroupChannels")
return s
}
func (s *SqlGroupStore) Create(group *model.Group) (*model.Group, error) {
@@ -132,14 +189,9 @@ func (s *SqlGroupStore) CreateWithUserIds(g *model.GroupWithUserIds) (_ *model.G
return nil, err
}
// Get the new Group along with the member count
groupGroupQuery := `
SELECT
UserGroups.*,
A.Count AS MemberCount
FROM
UserGroups
INNER JOIN (
groupGroupQuery := s.userGroupsSelectQuery.
Column("A.Count AS MemberCount").
InnerJoin(`(
SELECT
UserGroups.Id,
COUNT(GroupMembers.UserId) AS Count
@@ -154,12 +206,12 @@ func (s *SqlGroupStore) CreateWithUserIds(g *model.GroupWithUserIds) (_ *model.G
UserGroups.DisplayName,
UserGroups.Id
LIMIT
? OFFSET ?
) AS A ON UserGroups.Id = A.Id
ORDER BY
UserGroups.CreateAt DESC`
1 OFFSET 0
) AS A ON UserGroups.Id = A.Id`, g.Id).
OrderBy("UserGroups.CreateAt DESC")
var newGroup group
if err = txn.Get(&newGroup, groupGroupQuery, g.Id, 1, 0); err != nil {
if err = txn.GetBuilder(&newGroup, groupGroupQuery); err != nil {
return nil, err
}
if err = txn.Commit(); err != nil {
@@ -172,16 +224,13 @@ func (s *SqlGroupStore) checkUsersExist(userIDs []string) error {
if len(userIDs) == 0 {
return nil
}
usersSelectQuery, usersSelectArgs, err := s.getQueryBuilder().
usersSelectQuery := s.getQueryBuilder().
Select("Id").
From("Users").
Where(sq.Eq{"Id": userIDs, "DeleteAt": 0}).
ToSql()
if err != nil {
return err
}
Where(sq.Eq{"Id": userIDs, "DeleteAt": 0})
var rows []string
err = s.GetReplica().Select(&rows, usersSelectQuery, usersSelectArgs...)
err := s.GetReplica().SelectBuilder(&rows, usersSelectQuery)
if err != nil {
return err
}
@@ -215,9 +264,7 @@ func (s *SqlGroupStore) buildInsertGroupUsersQuery(groupId string, userIds []str
func (s *SqlGroupStore) Get(groupId string) (*model.Group, error) {
var group model.Group
builder := s.getQueryBuilder().
Select("*").
From("UserGroups").
builder := s.userGroupsSelectQuery.
Where(sq.Eq{"Id": groupId})
if err := s.GetReplica().GetBuilder(&group, builder); err != nil {
@@ -232,16 +279,14 @@ func (s *SqlGroupStore) Get(groupId string) (*model.Group, error) {
func (s *SqlGroupStore) GetByName(name string, opts model.GroupSearchOpts) (*model.Group, error) {
var group model.Group
query := s.getQueryBuilder().Select("*").From("UserGroups").Where(sq.Eq{"Name": name})
query := s.userGroupsSelectQuery.
Where(sq.Eq{"Name": name})
if opts.FilterAllowReference {
query = query.Where("AllowReference = true")
}
queryString, args, err := query.ToSql()
if err != nil {
return nil, errors.Wrap(err, "get_by_name_tosql")
}
if err := s.GetReplica().Get(&group, queryString, args...); err != nil {
if err := s.GetReplica().GetBuilder(&group, query); err != nil {
if err == sql.ErrNoRows {
return nil, store.NewErrNotFound("Group", fmt.Sprintf("name=%s", name))
}
@@ -253,12 +298,8 @@ func (s *SqlGroupStore) GetByName(name string, opts model.GroupSearchOpts) (*mod
func (s *SqlGroupStore) GetByIDs(groupIDs []string) ([]*model.Group, error) {
groups := []*model.Group{}
query := s.getQueryBuilder().Select("*").From("UserGroups").Where(sq.Eq{"Id": groupIDs})
queryString, args, err := query.ToSql()
if err != nil {
return nil, errors.Wrap(err, "get_by_ids_tosql")
}
if err := s.GetReplica().Select(&groups, queryString, args...); err != nil {
query := s.userGroupsSelectQuery.Where(sq.Eq{"Id": groupIDs})
if err := s.GetReplica().SelectBuilder(&groups, query); err != nil {
return nil, errors.Wrap(err, "failed to find Groups by ids")
}
return groups, nil
@@ -266,9 +307,7 @@ func (s *SqlGroupStore) GetByIDs(groupIDs []string) ([]*model.Group, error) {
func (s *SqlGroupStore) GetByRemoteID(remoteID string, groupSource model.GroupSource) (*model.Group, error) {
var group model.Group
builder := s.getQueryBuilder().
Select("*").
From("UserGroups").
builder := s.userGroupsSelectQuery.
Where(sq.Eq{
"RemoteId": remoteID,
"Source": groupSource,
@@ -286,9 +325,7 @@ func (s *SqlGroupStore) GetByRemoteID(remoteID string, groupSource model.GroupSo
func (s *SqlGroupStore) GetAllBySource(groupSource model.GroupSource) ([]*model.Group, error) {
groups := []*model.Group{}
builder := s.getQueryBuilder().
Select("*").
From("UserGroups").
builder := s.userGroupsSelectQuery.
Where(sq.Eq{
"DeleteAt": 0,
"Source": groupSource,
@@ -304,13 +341,11 @@ func (s *SqlGroupStore) GetAllBySource(groupSource model.GroupSource) ([]*model.
func (s *SqlGroupStore) GetByUser(userID string, opts model.GroupSearchOpts) ([]*model.Group, error) {
groups := []*model.Group{}
builder := s.getQueryBuilder().
Select("UserGroups.*").
From("GroupMembers").
Join("UserGroups ON UserGroups.Id = GroupMembers.GroupId").
builder := s.userGroupsSelectQuery.
Join("GroupMembers ON GroupMembers.GroupId = UserGroups.Id").
Where(sq.Eq{
"GroupMembers.DeleteAt": 0,
"UserId": userID,
"GroupMembers.UserId": userID,
})
if opts.FilterAllowReference {
@@ -326,10 +361,7 @@ func (s *SqlGroupStore) GetByUser(userID string, opts model.GroupSearchOpts) ([]
func (s *SqlGroupStore) Update(group *model.Group) (*model.Group, error) {
var retrievedGroup model.Group
builder := s.getQueryBuilder().
Select("*").
From("UserGroups").
Where(sq.Eq{"Id": group.Id})
builder := s.userGroupsSelectQuery.Where(sq.Eq{"Id": group.Id})
if err := s.GetReplica().GetBuilder(&retrievedGroup, builder); err != nil {
if err == sql.ErrNoRows {
@@ -371,9 +403,7 @@ func (s *SqlGroupStore) Update(group *model.Group) (*model.Group, error) {
func (s *SqlGroupStore) Delete(groupID string) (*model.Group, error) {
var group model.Group
builder := s.getQueryBuilder().
Select("*").
From("UserGroups").
builder := s.userGroupsSelectQuery.
Where(sq.Eq{
"Id": groupID,
"DeleteAt": 0,
@@ -400,9 +430,7 @@ func (s *SqlGroupStore) Delete(groupID string) (*model.Group, error) {
func (s *SqlGroupStore) Restore(groupID string) (*model.Group, error) {
var group model.Group
builder := s.getQueryBuilder().
Select("*").
From("UserGroups").
builder := s.userGroupsSelectQuery.
Where(sq.And{
sq.Eq{"Id": groupID},
sq.NotEq{"DeleteAt": 0},
@@ -427,12 +455,11 @@ func (s *SqlGroupStore) Restore(groupID string) (*model.Group, error) {
}
func (s *SqlGroupStore) GetMember(groupID, userID string) (*model.GroupMember, error) {
builder := s.getQueryBuilder().
Select("*").
From("GroupMembers").
builder := s.groupMembersSelectQuery.
Where(sq.Eq{"UserId": userID}).
Where(sq.Eq{"GroupId": groupID}).
Where(sq.Eq{"DeleteAt": 0})
var groupMember model.GroupMember
if err := s.GetReplica().GetBuilder(&groupMember, builder); err != nil {
return nil, errors.Wrap(err, "GetMember")
@@ -443,14 +470,11 @@ func (s *SqlGroupStore) GetMember(groupID, userID string) (*model.GroupMember, e
func (s *SqlGroupStore) GetMemberUsers(groupID string) ([]*model.User, error) {
groupMembers := []*model.User{}
builder := s.getQueryBuilder().
Select("Users.*").
From("GroupMembers").
Join("Users ON Users.Id = GroupMembers.UserId").
builder := s.groupMemberUsersSelectQuery.
Where(sq.Eq{
"GroupMembers.DeleteAt": 0,
"Users.DeleteAt": 0,
"GroupId": groupID,
"GroupMembers.GroupId": groupID,
})
if err := s.GetReplica().SelectBuilder(&groupMembers, builder); err != nil {
@@ -467,23 +491,16 @@ func (s *SqlGroupStore) GetMemberUsersPage(groupID string, page int, perPage int
func (s *SqlGroupStore) GetMemberUsersSortedPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions, teammateNameDisplay string) ([]*model.User, error) {
groupMembers := []*model.User{}
userQuery := s.getQueryBuilder().
Select(`Users.*`).
From("GroupMembers").
Join("Users ON Users.Id = GroupMembers.UserId").
userQuery := s.groupMemberUsersSelectQuery.
Where(sq.Eq{"GroupMembers.DeleteAt": 0}).
Where(sq.Eq{"Users.DeleteAt": 0}).
Where(sq.Eq{"GroupId": groupID})
Where(sq.Eq{"GroupMembers.GroupId": groupID})
userQuery = applyViewRestrictionsFilter(userQuery, viewRestrictions, true)
queryString, args, err := userQuery.ToSql()
if err != nil {
return nil, errors.Wrap(err, "")
}
orderQuery := s.getQueryBuilder().
Select("Users.*").
From("(" + queryString + ") AS Users")
Select(getUsersColumns()...).
FromSelect(userQuery, "Users")
if teammateNameDisplay == model.ShowNicknameFullName {
orderQuery = orderQuery.OrderBy(`
@@ -510,12 +527,7 @@ func (s *SqlGroupStore) GetMemberUsersSortedPage(groupID string, page int, perPa
Limit(uint64(perPage)).
Offset(uint64(page * perPage))
queryString, _, err = orderQuery.ToSql()
if err != nil {
return nil, errors.Wrap(err, "")
}
if err := s.GetReplica().Select(&groupMembers, queryString, args...); err != nil {
if err := s.GetReplica().SelectBuilder(&groupMembers, orderQuery); err != nil {
return nil, errors.Wrapf(err, "failed to find member Users for Group with id=%s", groupID)
}
@@ -525,9 +537,7 @@ func (s *SqlGroupStore) GetMemberUsersSortedPage(groupID string, page int, perPa
func (s *SqlGroupStore) GetNonMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) {
groupMembers := []*model.User{}
builder := s.getQueryBuilder().
Select("*").
From("UserGroups").
builder := s.userGroupsSelectQuery.
Where(sq.Eq{"Id": groupID})
if err := s.GetReplica().GetBuilder(&model.Group{}, builder); err != nil {
@@ -535,7 +545,7 @@ func (s *SqlGroupStore) GetNonMemberUsersPage(groupID string, page int, perPage
}
builder = s.getQueryBuilder().
Select("Users.*").
Select(getUsersColumns()...).
From("Users").
LeftJoin("GroupMembers ON (GroupMembers.UserId = Users.Id AND GroupMembers.GroupId = ?)", groupID).
Where(sq.Eq{"Users.DeleteAt": 0}).
@@ -568,14 +578,8 @@ func (s *SqlGroupStore) GetMemberCountWithRestrictions(groupID string, viewRestr
query = applyViewRestrictionsFilter(query, viewRestrictions, false)
queryString, args, err := query.ToSql()
if err != nil {
return int64(0), errors.Wrap(err, "")
}
var count int64
err = s.GetReplica().Get(&count, queryString, args...)
if err != nil {
if err := s.GetReplica().GetBuilder(&count, query); err != nil {
return int64(0), errors.Wrapf(err, "failed to count member Users for Group with id=%s", groupID)
}
@@ -647,22 +651,22 @@ func (s *SqlGroupStore) GetMemberUsersNotInChannel(groupID string, channelID str
}
func (s *SqlGroupStore) UpsertMember(groupID string, userID string) (*model.GroupMember, error) {
members, query, args, err := s.buildUpsertMembersQuery(groupID, []string{userID})
members, query, err := s.buildUpsertMembersQuery(groupID, []string{userID})
if err != nil {
return nil, err
}
if _, err = s.GetMaster().Exec(query, args...); err != nil {
if _, err = s.GetMaster().ExecBuilder(query); err != nil {
return nil, errors.Wrap(err, "failed to save GroupMember")
}
return members[0], nil
}
func (s *SqlGroupStore) DeleteMember(groupID string, userID string) (*model.GroupMember, error) {
members, query, args, err := s.buildDeleteMembersQuery(groupID, []string{userID})
members, query, err := s.buildDeleteMembersQuery(groupID, []string{userID})
if err != nil {
return nil, err
}
if _, err = s.GetMaster().Exec(query, args...); err != nil {
if _, err = s.GetMaster().ExecBuilder(query); err != nil {
return nil, errors.Wrapf(err, "failed to update GroupMember with groupId=%s and userId=%s", groupID, userID)
}
@@ -742,11 +746,17 @@ func (s *SqlGroupStore) getGroupSyncable(groupID string, syncableID string, sync
switch syncableType {
case model.GroupSyncableTypeTeam:
var team groupTeam
err = s.GetReplica().Get(&team, `SELECT * FROM GroupTeams WHERE GroupId=? AND TeamId=?`, groupID, syncableID)
err = s.GetReplica().GetBuilder(&team, s.groupTeamsSelectQuery.Where(sq.Eq{
"GroupTeams.GroupId": groupID,
"GroupTeams.TeamId": syncableID,
}))
result = &team
case model.GroupSyncableTypeChannel:
var ch groupChannel
err = s.GetReplica().Get(&ch, `SELECT * FROM GroupChannels WHERE GroupId=? AND ChannelId=?`, groupID, syncableID)
err = s.GetReplica().GetBuilder(&ch, s.groupChannelsSelectQuery.Where(sq.Eq{
"GroupChannels.GroupId": groupID,
"GroupChannels.ChannelId": syncableID,
}))
result = &ch
}
@@ -790,19 +800,16 @@ func (s *SqlGroupStore) GetAllGroupSyncablesByGroupId(groupID string, syncableTy
switch syncableType {
case model.GroupSyncableTypeTeam:
sqlQuery := `
SELECT
GroupTeams.*,
Teams.DisplayName AS TeamDisplayName,
Teams.Type AS TeamType
FROM
GroupTeams
JOIN Teams ON Teams.Id = GroupTeams.TeamId
WHERE
GroupId = ? AND GroupTeams.DeleteAt = 0`
query := s.groupTeamsSelectQuery.
Columns("Teams.DisplayName AS TeamDisplayName", "Teams.Type AS TeamType").
Join("Teams ON Teams.Id = GroupTeams.TeamId").
Where(sq.Eq{
"GroupTeams.GroupId": groupID,
"GroupTeams.DeleteAt": 0,
})
results := []*groupTeamJoin{}
err := s.GetReplica().Select(&results, sqlQuery, groupID)
err := s.GetReplica().SelectBuilder(&results, query)
if err != nil {
return nil, errors.Wrapf(err, "failed to find GroupTeams with groupId=%s", groupID)
}
@@ -822,23 +829,22 @@ func (s *SqlGroupStore) GetAllGroupSyncablesByGroupId(groupID string, syncableTy
groupSyncables = append(groupSyncables, groupSyncable)
}
case model.GroupSyncableTypeChannel:
sqlQuery := `
SELECT
GroupChannels.*,
Channels.DisplayName AS ChannelDisplayName,
Teams.DisplayName AS TeamDisplayName,
Channels.Type As ChannelType,
Teams.Type As TeamType,
Teams.Id AS TeamId
FROM
GroupChannels
JOIN Channels ON Channels.Id = GroupChannels.ChannelId
JOIN Teams ON Teams.Id = Channels.TeamId
WHERE
GroupId = ? AND GroupChannels.DeleteAt = 0`
query := s.groupChannelsSelectQuery.
Columns(
"Channels.DisplayName AS ChannelDisplayName",
"Teams.DisplayName AS TeamDisplayName",
"Channels.Type As ChannelType",
"Teams.Type As TeamType",
"Teams.Id AS TeamId",
).Join("Channels ON Channels.Id = GroupChannels.ChannelId").
Join("Teams ON Teams.Id = Channels.TeamId").
Where(sq.Eq{
"GroupChannels.GroupId": groupID,
"GroupChannels.DeleteAt": 0,
})
results := []*groupChannelJoin{}
err := s.GetReplica().Select(&results, sqlQuery, groupID)
err := s.GetReplica().SelectBuilder(&results, query)
if err != nil {
return nil, errors.Wrapf(err, "failed to find GroupChannels with groupId=%s", groupID)
}
@@ -1902,22 +1908,22 @@ func (s *SqlGroupStore) countTableWithSelectAndWhere(selectStr, tableName string
}
func (s *SqlGroupStore) UpsertMembers(groupID string, userIDs []string) ([]*model.GroupMember, error) {
members, query, args, err := s.buildUpsertMembersQuery(groupID, userIDs)
members, query, err := s.buildUpsertMembersQuery(groupID, userIDs)
if err != nil {
return nil, err
}
if _, err = s.GetMaster().Exec(query, args...); err != nil {
if _, err = s.GetMaster().ExecBuilder(query); err != nil {
return nil, errors.Wrap(err, "failed to save GroupMember")
}
return members, err
}
func (s *SqlGroupStore) buildUpsertMembersQuery(groupID string, userIDs []string) (members []*model.GroupMember, query string, args []any, err error) {
func (s *SqlGroupStore) buildUpsertMembersQuery(groupID string, userIDs []string) (members []*model.GroupMember, builder sq.InsertBuilder, err error) {
var retrievedGroup model.Group
// Check Group exists
if err = s.GetReplica().Get(&retrievedGroup, "SELECT * FROM UserGroups WHERE Id = ?", groupID); err != nil {
if err = s.GetReplica().GetBuilder(&retrievedGroup, s.userGroupsSelectQuery.Where(sq.Eq{"UserGroups.Id": groupID})); err != nil {
err = errors.Wrapf(err, "failed to get UserGroup with groupId=%s", groupID)
return
}
@@ -1927,7 +1933,7 @@ func (s *SqlGroupStore) buildUpsertMembersQuery(groupID string, userIDs []string
return
}
builder := s.getQueryBuilder().
builder = s.getQueryBuilder().
Insert("GroupMembers").
Columns("GroupId", "UserId", "CreateAt", "DeleteAt")
@@ -1950,38 +1956,30 @@ func (s *SqlGroupStore) buildUpsertMembersQuery(groupID string, userIDs []string
builder = builder.SuffixExpr(sq.Expr("ON CONFLICT (groupid, userid) DO UPDATE SET CreateAt = ?, DeleteAt = ?", createAt, 0))
}
query, args, err = builder.ToSql()
return
}
func (s *SqlGroupStore) DeleteMembers(groupID string, userIDs []string) ([]*model.GroupMember, error) {
members, query, args, err := s.buildDeleteMembersQuery(groupID, userIDs)
members, query, err := s.buildDeleteMembersQuery(groupID, userIDs)
if err != nil {
return nil, err
}
if _, err = s.GetMaster().Exec(query, args...); err != nil {
if _, err = s.GetMaster().ExecBuilder(query); err != nil {
return nil, errors.Wrap(err, "failed to delete GroupMembers")
}
return members, err
}
func (s *SqlGroupStore) buildDeleteMembersQuery(groupID string, userIDs []string) (members []*model.GroupMember, query string, args []any, err error) {
membersSelectQuery, membersSelectArgs, err := s.getQueryBuilder().
Select("*").
From("GroupMembers").
func (s *SqlGroupStore) buildDeleteMembersQuery(groupID string, userIDs []string) (members []*model.GroupMember, builder sq.UpdateBuilder, err error) {
membersSelectQuery := s.groupMembersSelectQuery.
Where(sq.And{
sq.Eq{"GroupId": groupID},
sq.Eq{"UserId": userIDs},
sq.Eq{"DeleteAt": 0},
}).
ToSql()
if err != nil {
return
}
sq.Eq{"GroupMembers.GroupId": groupID},
sq.Eq{"GroupMembers.UserId": userIDs},
sq.Eq{"GroupMembers.DeleteAt": 0},
})
err = s.GetReplica().Select(&members, membersSelectQuery, membersSelectArgs...)
if err != nil {
if err = s.GetReplica().SelectBuilder(&members, membersSelectQuery); err != nil {
return
}
if len(members) != len(userIDs) {
@@ -2003,7 +2001,7 @@ func (s *SqlGroupStore) buildDeleteMembersQuery(groupID string, userIDs []string
member.DeleteAt = deleteAt
}
builder := s.getQueryBuilder().
builder = s.getQueryBuilder().
Update("GroupMembers").
Set("DeleteAt", deleteAt).
Where(sq.And{
@@ -2011,6 +2009,5 @@ func (s *SqlGroupStore) buildDeleteMembersQuery(groupID string, userIDs []string
sq.Eq{"UserId": userIDs},
})
query, args, err = builder.ToSql()
return
}