From eb37139f16f7898cffd61469c1f7e2abf34e3b41 Mon Sep 17 00:00:00 2001 From: Tim Scheuermann Date: Fri, 26 Aug 2022 11:12:59 +0200 Subject: [PATCH] MM-45994 ensure database operations return their errors (#20857) --- store/errors.go | 36 +++++- store/sqlstore/adapters.go | 11 +- store/sqlstore/bot_store.go | 4 +- .../sqlstore/channel_member_history_store.go | 6 +- store/sqlstore/channel_store.go | 112 +++++++++--------- store/sqlstore/channel_store_categories.go | 81 +++++++------ store/sqlstore/cluster_discovery_store.go | 11 +- store/sqlstore/command_webhook_store.go | 4 +- store/sqlstore/emoji_store.go | 4 +- store/sqlstore/group_store.go | 18 ++- store/sqlstore/integrity.go | 7 +- store/sqlstore/oauth_store.go | 4 +- store/sqlstore/post_store.go | 57 +++++---- store/sqlstore/preference_store.go | 6 +- store/sqlstore/product_notices_store.go | 9 +- store/sqlstore/reaction_store.go | 10 +- store/sqlstore/retention_policy_store.go | 17 +-- store/sqlstore/role_store.go | 22 ++-- store/sqlstore/scheme_store.go | 24 ++-- store/sqlstore/shared_channel_store.go | 8 +- store/sqlstore/status_store.go | 20 ++-- store/sqlstore/store.go | 6 +- store/sqlstore/system_store.go | 4 +- store/sqlstore/team_store.go | 13 +- store/sqlstore/thread_store.go | 4 +- store/sqlstore/user_access_token_store.go | 12 +- store/sqlstore/user_store.go | 42 +++++-- store/sqlstore/user_terms_of_service.go | 6 +- store/sqlstore/utils.go | 16 ++- 29 files changed, 326 insertions(+), 248 deletions(-) diff --git a/store/errors.go b/store/errors.go index 82832a1a1b..fcd733aff7 100644 --- a/store/errors.go +++ b/store/errors.go @@ -10,9 +10,10 @@ import ( // ErrInvalidInput indicates an error that has occurred due to an invalid input. type ErrInvalidInput struct { - Entity string // The entity which was sent as the input. - Field string // The field of the entity which was invalid. - Value any // The actual value of the field. + Entity string // The entity which was sent as the input. + Field string // The field of the entity which was invalid. + Value any // The actual value of the field. + wrapped error // The original error } func NewErrInvalidInput(entity, field string, value any) *ErrInvalidInput { @@ -24,9 +25,22 @@ func NewErrInvalidInput(entity, field string, value any) *ErrInvalidInput { } func (e *ErrInvalidInput) Error() string { + if e.wrapped != nil { + return fmt.Sprintf("invalid input: entity: %s field: %s value: %s error: %s", e.Entity, e.Field, e.Value, e.wrapped) + } + return fmt.Sprintf("invalid input: entity: %s field: %s value: %s", e.Entity, e.Field, e.Value) } +func (e *ErrInvalidInput) Wrap(err error) *ErrInvalidInput { + e.wrapped = err + return e +} + +func (e *ErrInvalidInput) Unwrap() error { + return e.wrapped +} + func (e *ErrInvalidInput) InvalidInputInfo() (entity string, field string, value any) { entity = e.Entity field = e.Field @@ -89,6 +103,7 @@ func (e *ErrConflict) IsErrConflict() bool { type ErrNotFound struct { resource string ID string + wrapped error } func NewErrNotFound(resource, id string) *ErrNotFound { @@ -98,8 +113,17 @@ func NewErrNotFound(resource, id string) *ErrNotFound { } } +func (e *ErrNotFound) Wrap(err error) *ErrNotFound { + e.wrapped = err + return e +} + func (e *ErrNotFound) Error() string { - return "resource: " + e.resource + " id: " + e.ID + if e.wrapped != nil { + return fmt.Sprintf("resource: %s id: %s error: %s", e.resource, e.ID, e.wrapped) + } + + return fmt.Sprintf("resource: %s id: %s", e.resource, e.ID) } // IsErrNotFound allows easy type assertion without adding store as a dependency. @@ -142,8 +166,8 @@ type ErrUniqueConstraint struct { // // Examples: // -// store.NewErrUniqueConstraint("DisplayName") // single column constraint -// store.NewErrUniqueConstraint("Name", "Source") // multi-column constraint +// store.NewErrUniqueConstraint("DisplayName") // single column constraint +// store.NewErrUniqueConstraint("Name", "Source") // multi-column constraint func NewErrUniqueConstraint(columns ...string) *ErrUniqueConstraint { return &ErrUniqueConstraint{ Columns: columns, diff --git a/store/sqlstore/adapters.go b/store/sqlstore/adapters.go index 31636a4686..901e8c988f 100644 --- a/store/sqlstore/adapters.go +++ b/store/sqlstore/adapters.go @@ -25,16 +25,17 @@ func (a jsonArray) Value() (driver.Value, error) { if _, err := out.WriteString(strconv.Quote(item)); err != nil { return nil, err } + // Skip the last element. if i < len(a)-1 { - out.WriteByte(',') + if err := out.WriteByte(','); err != nil { + return nil, err + } } } - if err := out.WriteByte(']'); err != nil { - return nil, err - } - return out.Bytes(), nil + err := out.WriteByte(']') + return out.Bytes(), err } type jsonStringVal string diff --git a/store/sqlstore/bot_store.go b/store/sqlstore/bot_store.go index 21e5711aaa..c5d023eb0d 100644 --- a/store/sqlstore/bot_store.go +++ b/store/sqlstore/bot_store.go @@ -144,7 +144,7 @@ func (us SqlBotStore) GetAll(options *model.BotGetOptions) ([]*model.Bot, error) bots := []*model.Bot{} if err := us.GetReplicaX().Select(&bots, sql, args...); err != nil { - return nil, errors.Wrap(err, "select") + return nil, errors.Wrap(err, "error selecting all bots") } return bots, nil @@ -215,7 +215,7 @@ func (us SqlBotStore) Update(bot *model.Bot) (*model.Bot, error) { func (us SqlBotStore) PermanentDelete(botUserId string) error { query := "DELETE FROM Bots WHERE UserId = ?" if _, err := us.GetMasterX().Exec(query, botUserId); err != nil { - return store.NewErrInvalidInput("Bot", "UserId", botUserId) + return store.NewErrInvalidInput("Bot", "UserId", botUserId).Wrap(err) } return nil } diff --git a/store/sqlstore/channel_member_history_store.go b/store/sqlstore/channel_member_history_store.go index 77a2a8cf10..34985dff1d 100644 --- a/store/sqlstore/channel_member_history_store.go +++ b/store/sqlstore/channel_member_history_store.go @@ -192,10 +192,10 @@ func (s SqlChannelMemberHistoryStore) DeleteOrphanedRows(limit int) (deleted int )` result, err := s.GetMasterX().Exec(query, limit) if err != nil { - return + return 0, err } - deleted, err = result.RowsAffected() - return + + return result.RowsAffected() } func (s SqlChannelMemberHistoryStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, error) { diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index dd2023fdb5..51e6fb6f3f 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -402,6 +402,7 @@ func (db allChannelMember) Process() (string, string) { for _, role := range roles { if role == impliedRole { alreadyThere = true + break } } if !alreadyThere { @@ -554,7 +555,7 @@ func (s SqlChannelStore) upsertPublicChannelT(transaction *sqlxTxWrapper, channe } // Save writes the (non-direct) channel channel to the database. -func (s SqlChannelStore) Save(channel *model.Channel, maxChannelsPerTeam int64) (*model.Channel, error) { +func (s SqlChannelStore) Save(channel *model.Channel, maxChannelsPerTeam int64) (_ *model.Channel, err error) { if channel.DeleteAt != 0 { return nil, store.NewErrInvalidInput("Channel", "DeleteAt", channel.DeleteAt) } @@ -568,7 +569,7 @@ func (s SqlChannelStore) Save(channel *model.Channel, maxChannelsPerTeam int64) if err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) newChannel, err = s.saveChannelT(transaction, channel, maxChannelsPerTeam) if err != nil { @@ -619,7 +620,7 @@ func (s SqlChannelStore) CreateDirectChannel(user *model.User, otherUser *model. return s.SaveDirectChannel(channel, cm1, cm2) } -func (s SqlChannelStore) SaveDirectChannel(directChannel *model.Channel, member1 *model.ChannelMember, member2 *model.ChannelMember) (*model.Channel, error) { +func (s SqlChannelStore) SaveDirectChannel(directChannel *model.Channel, member1 *model.ChannelMember, member2 *model.ChannelMember) (_ *model.Channel, err error) { if directChannel.DeleteAt != 0 { return nil, store.NewErrInvalidInput("Channel", "DeleteAt", directChannel.DeleteAt) } @@ -632,7 +633,7 @@ func (s SqlChannelStore) SaveDirectChannel(directChannel *model.Channel, member1 if err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) directChannel.TeamId = "" newChannel, err := s.saveChannelT(transaction, directChannel, 0) @@ -695,12 +696,12 @@ func (s SqlChannelStore) saveChannelT(transaction *sqlxTxWrapper, channel *model } // Update writes the updated channel to the database. -func (s SqlChannelStore) Update(channel *model.Channel) (*model.Channel, error) { +func (s SqlChannelStore) Update(channel *model.Channel) (_ *model.Channel, err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) updatedChannel, err := s.updateChannelT(transaction, channel) if err != nil { @@ -869,14 +870,14 @@ func (s SqlChannelStore) Restore(channelId string, time int64) error { } // SetDeleteAt records the given deleted and updated timestamp to the channel in question. -func (s SqlChannelStore) SetDeleteAt(channelId string, deleteAt, updateAt int64) error { +func (s SqlChannelStore) SetDeleteAt(channelId string, deleteAt, updateAt int64) (err error) { defer s.InvalidateChannel(channelId) transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "SetDeleteAt: begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) err = s.setDeleteAtT(transaction, channelId, deleteAt, updateAt) if err != nil { @@ -915,12 +916,12 @@ func (s SqlChannelStore) setDeleteAtT(transaction *sqlxTxWrapper, channelId stri } // PermanentDeleteByTeam removes all channels for the given team from the database. -func (s SqlChannelStore) PermanentDeleteByTeam(teamId string) error { +func (s SqlChannelStore) PermanentDeleteByTeam(teamId string) (err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "PermanentDeleteByTeam: begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) if err := s.permanentDeleteByTeamtT(transaction, teamId); err != nil { return errors.Wrap(err, "permanentDeleteByTeamtT") @@ -952,12 +953,12 @@ func (s SqlChannelStore) permanentDeleteByTeamtT(transaction *sqlxTxWrapper, tea } // PermanentDelete removes the given channel from the database. -func (s SqlChannelStore) PermanentDelete(channelId string) error { +func (s SqlChannelStore) PermanentDelete(channelId string) (err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "PermanentDelete: begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) if err := s.permanentDeleteT(transaction, channelId); err != nil { return errors.Wrap(err, "permanentDeleteT") @@ -1495,7 +1496,7 @@ func (s SqlChannelStore) GetByNames(teamId string, names []string, allowFromCach if err := s.GetReplicaX().Select(&dbChannels, query, args...); err != nil && err != sql.ErrNoRows { msg := fmt.Sprintf("failed to get channels with names=%v", names) if teamId != "" { - msg += fmt.Sprintf("teamId=%s", teamId) + msg += fmt.Sprintf(" teamId=%s", teamId) } return nil, errors.Wrap(err, msg) } @@ -1556,15 +1557,15 @@ func (s SqlChannelStore) getByName(teamId string, name string, includeDeleted bo return nil, errors.Wrapf(err, "getByName_tosql") } - if err := s.GetReplicaX().Get(&channel, queryStr, args...); err != nil { + if err = s.GetReplicaX().Get(&channel, queryStr, args...); err != nil { if err == sql.ErrNoRows { return nil, store.NewErrNotFound("Channel", fmt.Sprintf("TeamId=%s&Name=%s", teamId, name)) } return nil, errors.Wrapf(err, "failed to find channel with TeamId=%s and Name=%s", teamId, name) } - channelByNameCache.SetWithExpiry(teamId+name, &channel, ChannelCacheDuration) - return &channel, nil + err = channelByNameCache.SetWithExpiry(teamId+name, &channel, ChannelCacheDuration) + return &channel, err } func (s SqlChannelStore) GetDeletedByName(teamId string, name string) (*model.Channel, error) { @@ -1805,7 +1806,7 @@ func (s SqlChannelStore) saveMemberT(member *model.ChannelMember) (*model.Channe return members[0], nil } -func (s SqlChannelStore) UpdateMultipleMembers(members []*model.ChannelMember) ([]*model.ChannelMember, error) { +func (s SqlChannelStore) UpdateMultipleMembers(members []*model.ChannelMember) (_ []*model.ChannelMember, err error) { for _, member := range members { member.PreUpdate() @@ -1815,12 +1816,11 @@ func (s SqlChannelStore) UpdateMultipleMembers(members []*model.ChannelMember) ( } var transaction *sqlxTxWrapper - var err error if transaction, err = s.GetMasterX().Beginx(); err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) updatedMembers := []*model.ChannelMember{} for _, member := range members { @@ -1875,12 +1875,12 @@ func (s SqlChannelStore) UpdateMember(member *model.ChannelMember) (*model.Chann return updatedMembers[0], nil } -func (s SqlChannelStore) UpdateMemberNotifyProps(channelID, userID string, props map[string]string) (*model.ChannelMember, error) { +func (s SqlChannelStore) UpdateMemberNotifyProps(channelID, userID string, props map[string]string) (_ *model.ChannelMember, err error) { tx, err := s.GetMasterX().Beginx() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(tx) + defer finalizeTransactionX(tx, &err) if s.DriverName() == model.DatabaseDriverPostgres { sql, args, err2 := s.getQueryBuilder(). @@ -1891,7 +1891,7 @@ func (s SqlChannelStore) UpdateMemberNotifyProps(channelID, userID string, props "channelid": channelID, }).ToSql() if err2 != nil { - return nil, errors.Wrapf(err, "UpdateMemberNotifyProps_Update_Postgres_ToSql channelID=%s and userID=%s", channelID, userID) + return nil, errors.Wrapf(err2, "UpdateMemberNotifyProps_Update_Postgres_ToSql channelID=%s and userID=%s", channelID, userID) } _, err = tx.Exec(sql, args...) @@ -1914,7 +1914,7 @@ func (s SqlChannelStore) UpdateMemberNotifyProps(channelID, userID string, props "ChannelId": channelID, }).ToSql() if err2 != nil { - return nil, errors.Wrapf(err, "UpdateMemberNotifyProps_Update_MySQL_ToSql channelID=%s and userID=%s", channelID, userID) + return nil, errors.Wrapf(err2, "UpdateMemberNotifyProps_Update_MySQL_ToSql channelID=%s and userID=%s", channelID, userID) } _, err = tx.Exec(sql, args...) @@ -2079,14 +2079,14 @@ func (s SqlChannelStore) GetMemberForPost(postId string, userId string) (*model. return dbMember.ToModel(), nil } -func (s SqlChannelStore) GetAllChannelMembersForUser(userId string, allowFromCache bool, includeDeleted bool) (map[string]string, error) { +func (s SqlChannelStore) GetAllChannelMembersForUser(userId string, allowFromCache bool, includeDeleted bool) (_ map[string]string, err error) { cache_key := userId if includeDeleted { cache_key += "_deleted" } if allowFromCache { - var ids map[string]string - if err := allChannelMembersForUserCache.Get(cache_key, &ids); err == nil { + ids := make(map[string]string) + if err = allChannelMembersForUserCache.Get(cache_key, &ids); err == nil { if s.metrics != nil { s.metrics.IncrementMemCacheHitCounter("All Channel Members for User") } @@ -2127,9 +2127,9 @@ func (s SqlChannelStore) GetAllChannelMembersForUser(userId string, allowFromCac if err != nil { return nil, errors.Wrap(err, "failed to find ChannelMembers, TeamScheme and ChannelScheme data") } + defer deferClose(rows, &err) var data allChannelMembers - defer rows.Close() for rows.Next() { var cm allChannelMember err = rows.Scan( @@ -2549,14 +2549,20 @@ func (s SqlChannelStore) CountPostsAfter(channelId string, timestamp int64, user if userId != "" { query = query.Where(sq.Eq{"UserId": userId}) } - sql, args, _ := query.ToSql() + sql, args, err := query.ToSql() + if err != nil { + return 0, 0, errors.Wrap(err, "CountPostsAfter_ToSql1") + } var unread int64 - err := s.GetReplicaX().Get(&unread, sql, args...) + err = s.GetReplicaX().Get(&unread, sql, args...) if err != nil { return 0, 0, errors.Wrap(err, "failed to count Posts") } - sql2, args2, _ := query.Where(sq.Eq{"RootId": ""}).ToSql() + sql2, args2, err := query.Where(sq.Eq{"RootId": ""}).ToSql() + if err != nil { + return 0, 0, errors.Wrap(err, "CountPostsAfter_ToSql2") + } var unreadRoot int64 err = s.GetReplicaX().Get(&unreadRoot, sql2, args2...) @@ -2766,13 +2772,13 @@ func (s SqlChannelStore) AnalyticsTypeCount(teamId string, channelType model.Cha sql, args, err := query.ToSql() if err != nil { - return int64(0), errors.Wrap(err, "AnalyticsTypeCount_tosql") + return 0, errors.Wrap(err, "AnalyticsTypeCount_ToSql") } var value int64 err = s.GetReplicaX().Get(&value, sql, args...) if err != nil { - return int64(0), errors.Wrap(err, "failed to count Channels") + return 0, errors.Wrap(err, "failed to count Channels") } return value, nil } @@ -2792,7 +2798,7 @@ func (s SqlChannelStore) AnalyticsDeletedTypeCount(teamId string, channelType mo sql, args, err := query.ToSql() if err != nil { - return int64(0), errors.Wrap(err, "AnalyticsDeletedTypeCount_tosql") + return 0, errors.Wrap(err, "AnalyticsDeletedTypeCount_ToSql") } var v int64 @@ -3195,16 +3201,14 @@ func (s SqlChannelStore) SearchArchivedInTeam(teamId string, term string, userId Where(sq.Eq{"UserId": userId})), }) - publicChannels, publicErr := s.performSearch(publicQuery, term) - privateChannels, privateErr := s.performSearch(privateQuery, term) - - outputErr := publicErr - if privateErr != nil { - outputErr = privateErr + publicChannels, err := s.performSearch(publicQuery, term) + if err != nil { + return nil, err } - if outputErr != nil { - return nil, outputErr + privateChannels, err := s.performSearch(privateQuery, term) + if err != nil { + return nil, err } output := publicChannels @@ -3740,14 +3744,13 @@ func (s SqlChannelStore) GetChannelsByScheme(schemeId string, offset int, limit // in batches as a single transaction per batch to ensure consistency but to also minimise execution time to avoid // causing unnecessary table locks. **THIS FUNCTION SHOULD NOT BE USED FOR ANY OTHER PURPOSE.** Executing this function // *after* the new Schemes functionality has been used on an installation will have unintended consequences. -func (s SqlChannelStore) MigrateChannelMembers(fromChannelId string, fromUserId string) (map[string]string, error) { +func (s SqlChannelStore) MigrateChannelMembers(fromChannelId string, fromUserId string) (_ map[string]string, err error) { var transaction *sqlxTxWrapper - var err error if transaction, err = s.GetMasterX().Beginx(); err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) channelMembers := []channelMember{} if err := transaction.Select(&channelMembers, "SELECT * from ChannelMembers WHERE (ChannelId, UserId) > (?, ?) ORDER BY ChannelId, UserId LIMIT 100", fromChannelId, fromUserId); err != nil { @@ -3813,12 +3816,12 @@ func (s SqlChannelStore) MigrateChannelMembers(fromChannelId string, fromUserId return data, nil } -func (s SqlChannelStore) ResetAllChannelSchemes() error { +func (s SqlChannelStore) ResetAllChannelSchemes() (err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) err = s.resetAllChannelSchemesT(transaction) if err != nil { @@ -3840,27 +3843,26 @@ func (s SqlChannelStore) resetAllChannelSchemesT(transaction *sqlxTxWrapper) err return nil } -func (s SqlChannelStore) ClearAllCustomRoleAssignments() error { +func (s SqlChannelStore) ClearAllCustomRoleAssignments() (err error) { builtInRoles := model.MakeDefaultRoles() lastUserId := strings.Repeat("0", 26) lastChannelId := strings.Repeat("0", 26) for { var transaction *sqlxTxWrapper - var err error if transaction, err = s.GetMasterX().Beginx(); err != nil { return errors.Wrap(err, "begin_transaction") } channelMembers := []*channelMember{} - if err := transaction.Select(&channelMembers, "SELECT * from ChannelMembers WHERE (ChannelId, UserId) > (?, ?) ORDER BY ChannelId, UserId LIMIT 1000", lastChannelId, lastUserId); err != nil { - finalizeTransactionX(transaction) + if err = transaction.Select(&channelMembers, "SELECT * from ChannelMembers WHERE (ChannelId, UserId) > (?, ?) ORDER BY ChannelId, UserId LIMIT 1000", lastChannelId, lastUserId); err != nil { + finalizeTransactionX(transaction, &err) return errors.Wrap(err, "failed to find ChannelMembers") } if len(channelMembers) == 0 { - finalizeTransactionX(transaction) + finalizeTransactionX(transaction, &err) break } @@ -3881,15 +3883,15 @@ func (s SqlChannelStore) ClearAllCustomRoleAssignments() error { newRolesString := strings.Join(newRoles, " ") if newRolesString != member.Roles { - if _, err := transaction.Exec("UPDATE ChannelMembers SET Roles = ? WHERE UserId = ? AND ChannelId = ?", newRolesString, member.UserId, member.ChannelId); err != nil { - finalizeTransactionX(transaction) + if _, err = transaction.Exec("UPDATE ChannelMembers SET Roles = ? WHERE UserId = ? AND ChannelId = ?", newRolesString, member.UserId, member.ChannelId); err != nil { + finalizeTransactionX(transaction, &err) return errors.Wrap(err, "failed to update ChannelMembers") } } } - if err := transaction.Commit(); err != nil { - finalizeTransactionX(transaction) + if err = transaction.Commit(); err != nil { + finalizeTransactionX(transaction, &err) return errors.Wrap(err, "commit_transaction") } } diff --git a/store/sqlstore/channel_store_categories.go b/store/sqlstore/channel_store_categories.go index 0d2af6c487..2a1799b01a 100644 --- a/store/sqlstore/channel_store_categories.go +++ b/store/sqlstore/channel_store_categories.go @@ -20,12 +20,12 @@ type dbSelecter interface { Select(i any, query string, args ...any) error } -func (s SqlChannelStore) CreateInitialSidebarCategories(userId string, opts *store.SidebarCategorySearchOpts) (*model.OrderedSidebarCategories, error) { +func (s SqlChannelStore) CreateInitialSidebarCategories(userId string, opts *store.SidebarCategorySearchOpts) (_ *model.OrderedSidebarCategories, err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return nil, errors.Wrap(err, "CreateInitialSidebarCategories: begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) teamsWithExclude, err := s.SqlStore.stores.team.GetTeamsForUser(context.Background(), userId, opts.TeamID, false) if err != nil { @@ -184,11 +184,13 @@ func (s SqlChannelStore) migrateMembershipToSidebar(transaction *sqlxTxWrapper, } for _, favorite := range memberships { - sql, args, _ := s.getQueryBuilder(). + sql, args, err := s.getQueryBuilder(). Insert("SidebarChannels"). Columns("ChannelId", "UserId", "CategoryId", "SortOrder"). Values(favorite.ChannelId, favorite.UserId, favorite.CategoryId, *runningOrder).ToSql() - + if err != nil { + return nil, err + } if _, err := transaction.Exec(sql, args...); err != nil && !IsUniqueConstraintError(err, []string{"UserId", "PRIMARY"}) { return nil, err } @@ -202,7 +204,7 @@ func (s SqlChannelStore) migrateMembershipToSidebar(transaction *sqlxTxWrapper, } func (s SqlChannelStore) migrateFavoritesToSidebarT(transaction *sqlxTxWrapper, userId, teamId, favoritesCategoryId string) error { - favoritesQuery, favoritesParams, _ := s.getQueryBuilder(). + favoritesQuery, favoritesParams, err := s.getQueryBuilder(). Select("Preferences.Name"). From("Preferences"). Join("Channels on Preferences.Name = Channels.Id"). @@ -220,6 +222,9 @@ func (s SqlChannelStore) migrateFavoritesToSidebarT(transaction *sqlxTxWrapper, "Channels.DisplayName", "Channels.Name ASC", ).ToSql() + if err != nil { + return err + } favoriteChannelIds := []string{} if err := transaction.Select(&favoriteChannelIds, favoritesQuery, favoritesParams...); err != nil { @@ -244,13 +249,13 @@ func (s SqlChannelStore) migrateFavoritesToSidebarT(transaction *sqlxTxWrapper, // MigrateFavoritesToSidebarChannels populates the SidebarChannels table by analyzing existing user preferences for favorites // **IMPORTANT** This function should only be called from the migration task and shouldn't be used by itself -func (s SqlChannelStore) MigrateFavoritesToSidebarChannels(lastUserId string, runningOrder int64) (map[string]any, error) { +func (s SqlChannelStore) MigrateFavoritesToSidebarChannels(lastUserId string, runningOrder int64) (_ map[string]any, err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return nil, err } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) sb := s. getQueryBuilder(). @@ -291,13 +296,13 @@ type sidebarCategoryForJoin struct { ChannelId *string } -func (s SqlChannelStore) CreateSidebarCategory(userId, teamId string, newCategory *model.SidebarCategoryWithChannels) (*model.SidebarCategoryWithChannels, error) { +func (s SqlChannelStore) CreateSidebarCategory(userId, teamId string, newCategory *model.SidebarCategoryWithChannels) (_ *model.SidebarCategoryWithChannels, err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) opts := &store.SidebarCategorySearchOpts{ TeamID: teamId, @@ -413,12 +418,12 @@ func (s SqlChannelStore) CreateSidebarCategory(userId, teamId string, newCategor return result, nil } -func (s SqlChannelStore) completePopulatingCategoryChannels(category *model.SidebarCategoryWithChannels) (*model.SidebarCategoryWithChannels, error) { +func (s SqlChannelStore) completePopulatingCategoryChannels(category *model.SidebarCategoryWithChannels) (_ *model.SidebarCategoryWithChannels, err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) result, err := s.completePopulatingCategoryChannelsT(transaction, category) if err != nil { @@ -480,7 +485,7 @@ func (s SqlChannelStore) completePopulatingCategoryChannelsT(db dbSelecter, cate } if err := db.Select(&channels, sql, args...); err != nil { - return nil, store.NewErrNotFound("ChannelMembers", "") + return nil, store.NewErrNotFound("ChannelMembers", "").Wrap(err) } category.Channels = append(channels, category.Channels...) @@ -500,7 +505,7 @@ func (s SqlChannelStore) GetSidebarCategory(categoryId string) (*model.SidebarCa categories := []*sidebarCategoryForJoin{} if err = s.GetReplicaX().Select(&categories, sql, args...); err != nil { - return nil, store.NewErrNotFound("SidebarCategories", categoryId) + return nil, store.NewErrNotFound("SidebarCategories", categoryId).Wrap(err) } if len(categories) == 0 { @@ -547,7 +552,7 @@ func (s SqlChannelStore) getSidebarCategoriesT(db dbSelecter, userId string, opt } if err := db.Select(&categories, sql, args...); err != nil { - return nil, store.NewErrNotFound("SidebarCategories", fmt.Sprintf("userId=%s,teamId=%s", userId, opts.TeamID)) + return nil, store.NewErrNotFound("SidebarCategories", fmt.Sprintf("userId=%s,teamId=%s", userId, opts.TeamID)).Wrap(err) } for _, category := range categories { @@ -608,7 +613,7 @@ func (s SqlChannelStore) GetSidebarCategoryOrder(userId, teamId string) ([]strin } if err := s.GetReplicaX().Select(&ids, sql, args...); err != nil { - return nil, store.NewErrNotFound("SidebarCategories", fmt.Sprintf("userId=%s,teamId=%s", userId, teamId)) + return nil, store.NewErrNotFound("SidebarCategories", fmt.Sprintf("userId=%s,teamId=%s", userId, teamId)).Wrap(err) } return ids, nil @@ -633,13 +638,13 @@ func (s SqlChannelStore) updateSidebarCategoryOrderT(transaction *sqlxTxWrapper, return nil } -func (s SqlChannelStore) UpdateSidebarCategoryOrder(userId, teamId string, categoryOrder []string) error { +func (s SqlChannelStore) UpdateSidebarCategoryOrder(userId, teamId string, categoryOrder []string) (err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) // Ensure no invalid categories are included and that no categories are left out existingOrder, err := s.GetSidebarCategoryOrder(userId, teamId) @@ -676,12 +681,12 @@ func (s SqlChannelStore) UpdateSidebarCategoryOrder(userId, teamId string, categ } //nolint:unparam -func (s SqlChannelStore) UpdateSidebarCategories(userId, teamId string, categories []*model.SidebarCategoryWithChannels) ([]*model.SidebarCategoryWithChannels, []*model.SidebarCategoryWithChannels, error) { +func (s SqlChannelStore) UpdateSidebarCategories(userId, teamId string, categories []*model.SidebarCategoryWithChannels) (updated []*model.SidebarCategoryWithChannels, original []*model.SidebarCategoryWithChannels, err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return nil, nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) updatedCategories := []*model.SidebarCategoryWithChannels{} originalCategories := []*model.SidebarCategoryWithChannels{} @@ -719,14 +724,16 @@ func (s SqlChannelStore) UpdateSidebarCategories(userId, teamId string, categori // The net effect remains the same, but it prevents deadlocks from other transactions // operating on the tables in reverse order. - updateQuery, updateParams, _ := s.getQueryBuilder(). + updateQuery, updateParams, err2 := s.getQueryBuilder(). Update("SidebarCategories"). Set("DisplayName", destCategory.DisplayName). Set("Sorting", destCategory.Sorting). Set("Muted", destCategory.Muted). Set("Collapsed", destCategory.Collapsed). Where(sq.Eq{"Id": destCategory.Id}).ToSql() - + if err2 != nil { + return nil, nil, errors.Wrap(err2, "update_sidebar_categories_tosql1") + } if _, err = transaction.Exec(updateQuery, updateParams...); err != nil { return nil, nil, errors.Wrap(err, "failed to update SidebarCategories") } @@ -746,7 +753,7 @@ func (s SqlChannelStore) UpdateSidebarCategories(userId, teamId string, categori ).ToSql() if err2 != nil { - return nil, nil, errors.Wrap(err2, "update_sidebar_categories_tosql") + return nil, nil, errors.Wrap(err2, "update_sidebar_categories_tosql2") } if _, err = transaction.Exec(query, args...); err != nil { @@ -777,13 +784,16 @@ func (s SqlChannelStore) UpdateSidebarCategories(userId, teamId string, categori // Update the favorites preferences based on channels moving into or out of the Favorites category for compatibility if category.Type == model.SidebarCategoryFavorites { // Remove any old favorites - sql, args, _ := s.getQueryBuilder().Delete("Preferences").Where( + sql, args, err2 := s.getQueryBuilder().Delete("Preferences").Where( sq.Eq{ "UserId": userId, "Name": srcCategory.Channels, "Category": model.PreferenceCategoryFavoriteChannel, }, ).ToSql() + if err2 != nil { + return nil, nil, errors.Wrap(err2, "UpdateSidebarChannels_Tosql_DeletePreferences") + } if _, err = transaction.Exec(sql, args...); err != nil { return nil, nil, errors.Wrap(err, "failed to delete Preferences") @@ -843,12 +853,12 @@ func (s SqlChannelStore) UpdateSidebarCategories(userId, teamId string, categori // UpdateSidebarChannelsByPreferences is called when the Preference table is being updated to keep SidebarCategories in sync // At the moment, it's only handling Favorites and NOT DMs/GMs (those will be handled client side) -func (s SqlChannelStore) UpdateSidebarChannelsByPreferences(preferences model.Preferences) error { +func (s SqlChannelStore) UpdateSidebarChannelsByPreferences(preferences model.Preferences) (err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "UpdateSidebarChannelsByPreferences: begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) for _, preference := range preferences { preference := preference @@ -942,10 +952,13 @@ func (s SqlChannelStore) addChannelToFavoritesCategoryT(transaction *sqlxTxWrapp builder = builder.Where(sq.Eq{"TeamId": channel.TeamId}) } - idsQuery, idsParams, _ := builder.ToSql() + idsQuery, idsParams, err := builder.ToSql() + if err != nil { + return errors.Wrap(err, "addChannelToFavoritesCategoryT_ToSql_Select") + } categoryIds := []string{} - if err := transaction.Select(&categoryIds, idsQuery, idsParams...); err != nil { + if err = transaction.Select(&categoryIds, idsQuery, idsParams...); err != nil { return errors.Wrap(err, "Failed to get Favorites sidebar categories") } @@ -956,7 +969,7 @@ func (s SqlChannelStore) addChannelToFavoritesCategoryT(transaction *sqlxTxWrapp // For each category ID, insert a row into SidebarChannels with the given channel ID and a SortOrder that's less than // all existing SortOrders in the category so that the newly favorited channel comes first - insertQuery, insertParams, _ := s.getQueryBuilder(). + insertQuery, insertParams, err := s.getQueryBuilder(). Insert("SidebarChannels"). Columns( "ChannelId", @@ -976,7 +989,9 @@ func (s SqlChannelStore) addChannelToFavoritesCategoryT(transaction *sqlxTxWrapp "SidebarCategories.Id": categoryIds, }). GroupBy("SidebarCategories.Id")).ToSql() - + if err != nil { + return errors.Wrap(err, "addChannelToFavoritesCategoryT_ToSql_Insert") + } if _, err := transaction.Exec(insertQuery, insertParams...); err != nil { return errors.Wrap(err, "Failed to add sidebar entries for favorited channel") } @@ -986,12 +1001,12 @@ func (s SqlChannelStore) addChannelToFavoritesCategoryT(transaction *sqlxTxWrapp // DeleteSidebarChannelsByPreferences is called when the Preference table is being updated to keep SidebarCategories in sync // At the moment, it's only handling Favorites and NOT DMs/GMs (those will be handled client side) -func (s SqlChannelStore) DeleteSidebarChannelsByPreferences(preferences model.Preferences) error { +func (s SqlChannelStore) DeleteSidebarChannelsByPreferences(preferences model.Preferences) (err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "DeleteSidebarChannelsByPreferences: begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) for _, preference := range preferences { preference := preference @@ -1053,12 +1068,12 @@ func (s SqlChannelStore) ClearSidebarOnTeamLeave(userId, teamId string) error { // DeleteSidebarCategory removes a custom category and moves any channels into it into the Channels and Direct Messages // categories respectively. Assumes that the provided user ID and team ID match the given category ID. -func (s SqlChannelStore) DeleteSidebarCategory(categoryId string) error { +func (s SqlChannelStore) DeleteSidebarCategory(categoryId string) (err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) // Ensure that we're deleting a custom category var category model.SidebarCategory diff --git a/store/sqlstore/cluster_discovery_store.go b/store/sqlstore/cluster_discovery_store.go index 8b9a0fbdab..9a69efeedc 100644 --- a/store/sqlstore/cluster_discovery_store.go +++ b/store/sqlstore/cluster_discovery_store.go @@ -59,10 +59,7 @@ func (s sqlClusterDiscoveryStore) Delete(ClusterDiscovery *model.ClusterDiscover return false, errors.Wrap(err, "failed to count rows affected") } - if count == 0 { - return false, nil - } - return true, nil + return count != 0, nil } func (s sqlClusterDiscoveryStore) Exists(ClusterDiscovery *model.ClusterDiscovery) (bool, error) { @@ -82,10 +79,8 @@ func (s sqlClusterDiscoveryStore) Exists(ClusterDiscovery *model.ClusterDiscover if err := s.GetMasterX().Get(&count, queryString, args...); err != nil { return false, errors.Wrap(err, "failed to count ClusterDiscovery") } - if count == 0 { - return false, nil - } - return true, nil + + return count != 0, nil } func (s sqlClusterDiscoveryStore) GetAll(ClusterDiscoveryType, clusterName string) ([]*model.ClusterDiscovery, error) { diff --git a/store/sqlstore/command_webhook_store.go b/store/sqlstore/command_webhook_store.go index f745b3a9df..3d00beea9f 100644 --- a/store/sqlstore/command_webhook_store.go +++ b/store/sqlstore/command_webhook_store.go @@ -82,8 +82,8 @@ func (s SqlCommandWebhookStore) TryUse(id string, limit int) error { if sqlResult, err := s.GetMasterX().Exec(queryString, args...); err != nil { return errors.Wrapf(err, "tryuse: id=%s limit=%d", id, limit) - } else if rows, _ := sqlResult.RowsAffected(); rows == 0 { - return store.NewErrInvalidInput("CommandWebhook", "id", id) + } else if rows, err := sqlResult.RowsAffected(); rows == 0 { + return store.NewErrInvalidInput("CommandWebhook", "id", id).Wrap(err) } return nil diff --git a/store/sqlstore/emoji_store.go b/store/sqlstore/emoji_store.go index a3bdc5b900..135a6f045e 100644 --- a/store/sqlstore/emoji_store.go +++ b/store/sqlstore/emoji_store.go @@ -99,8 +99,8 @@ func (es SqlEmojiStore) Delete(emoji *model.Emoji, time int64) error { Id = ? AND DeleteAt = 0`, time, time, emoji.Id); err != nil { return errors.Wrap(err, "could not delete emoji") - } else if rows, _ := sqlResult.RowsAffected(); rows == 0 { - return store.NewErrNotFound("Emoji", emoji.Id) + } else if rows, err := sqlResult.RowsAffected(); rows == 0 { + return store.NewErrNotFound("Emoji", emoji.Id).Wrap(err) } return nil diff --git a/store/sqlstore/group_store.go b/store/sqlstore/group_store.go index add231364c..8a560395fa 100644 --- a/store/sqlstore/group_store.go +++ b/store/sqlstore/group_store.go @@ -81,18 +81,18 @@ func (s *SqlGroupStore) Create(group *model.Group) (*model.Group, error) { return group, nil } -func (s *SqlGroupStore) CreateWithUserIds(g *model.GroupWithUserIds) (*model.Group, error) { +func (s *SqlGroupStore) CreateWithUserIds(g *model.GroupWithUserIds) (_ *model.Group, err error) { if g.Id != "" { return nil, store.NewErrInvalidInput("Group", "id", g.Id) } // Check if group values are formatted correctly - if err := g.IsValidForCreate(); err != nil { - return nil, err + if appErr := g.IsValidForCreate(); appErr != nil { + return nil, appErr } // Check Users exist - if err := s.checkUsersExist(g.UserIds); err != nil { + if err = s.checkUsersExist(g.UserIds); err != nil { return nil, err } @@ -118,7 +118,8 @@ func (s *SqlGroupStore) CreateWithUserIds(g *model.GroupWithUserIds) (*model.Gro if err != nil { return nil, err } - defer finalizeTransactionX(txn) + defer finalizeTransactionX(txn, &err) + // Create a new usergroup if _, err = txn.Exec(groupInsertQuery, groupInsertArgs...); err != nil { if IsUniqueConstraintError(err, []string{"Name", "groups_name_key"}) { @@ -1378,12 +1379,7 @@ func (s *SqlGroupStore) GetGroupsAssociatedToChannelsByTeam(teamId string, opts groups := map[string][]*model.GroupWithSchemeAdmin{} for _, tgroup := range tgroups { group := tgroup.groupWithSchemeAdmin.ToModel() - - if val, ok := groups[tgroup.ChannelId]; ok { - groups[tgroup.ChannelId] = append(val, group) - } else { - groups[tgroup.ChannelId] = []*model.GroupWithSchemeAdmin{group} - } + groups[tgroup.ChannelId] = append(groups[tgroup.ChannelId], group) } return groups, nil diff --git a/store/sqlstore/integrity.go b/store/sqlstore/integrity.go index c72e9abfe6..51a262a3e4 100644 --- a/store/sqlstore/integrity.go +++ b/store/sqlstore/integrity.go @@ -52,9 +52,12 @@ func getOrphanedRecords(ss *SqlStore, cfg relationalCheckConfig) ([]model.Orphan main = main.OrderBy("CT." + cfg.parentIdAttr) } - query, args, _ := main.ToSql() + query, args, err := main.ToSql() + if err != nil { + return nil, err + } - err := ss.GetMasterX().Select(&records, query, args...) + err = ss.GetMasterX().Select(&records, query, args...) return records, err } diff --git a/store/sqlstore/oauth_store.go b/store/sqlstore/oauth_store.go index dec6db893e..b7ffbd316e 100644 --- a/store/sqlstore/oauth_store.go +++ b/store/sqlstore/oauth_store.go @@ -124,13 +124,13 @@ func (as SqlOAuthStore) GetAuthorizedApps(userId string, offset, limit int) ([]* return apps, nil } -func (as SqlOAuthStore) DeleteApp(id string) error { +func (as SqlOAuthStore) DeleteApp(id string) (err error) { // wrap in a transaction so that if one fails, everything fails transaction, err := as.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) if err := as.deleteApp(transaction, id); err != nil { return err diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index fc67b29599..233fd3d7ed 100644 --- a/store/sqlstore/post_store.go +++ b/store/sqlstore/post_store.go @@ -209,8 +209,7 @@ func (s *SqlPostStore) SaveMultiple(posts []*model.Post) ([]*model.Post, int, er if err != nil { return posts, -1, errors.Wrap(err, "begin_transaction") } - - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) if _, err = transaction.Exec(query, args...); err != nil { return nil, -1, errors.Wrap(err, "failed to save Post") @@ -388,7 +387,7 @@ func (s *SqlPostStore) Update(newPost *model.Post, oldPost *model.Post) (*model. return newPost, nil } -func (s *SqlPostStore) OverwriteMultiple(posts []*model.Post) ([]*model.Post, int, error) { +func (s *SqlPostStore) OverwriteMultiple(posts []*model.Post) (_ []*model.Post, _ int, err error) { updateAt := model.GetMillis() maxPostSize := s.GetMaxPostSize() for idx, post := range posts { @@ -402,7 +401,7 @@ func (s *SqlPostStore) OverwriteMultiple(posts []*model.Post) ([]*model.Post, in if err != nil { return nil, -1, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(tx) + defer finalizeTransactionX(tx, &err) for idx, post := range posts { if _, err2 := tx.NamedExec(`UPDATE Posts @@ -566,15 +565,18 @@ func (s *SqlPostStore) getPostWithCollapsedThreads(id, userID string, opts model ) var post postWithExtra - postFetchQuery, args, _ := s.getQueryBuilder(). + postFetchQuery, args, err := s.getQueryBuilder(). Select(columns...). From("Posts"). LeftJoin("Threads ON Threads.PostId = Id"). LeftJoin("ThreadMemberships ON ThreadMemberships.PostId = Id AND ThreadMemberships.UserId = ?", userID). Where(sq.Eq{"Posts.DeleteAt": 0}). Where(sq.Eq{"Posts.Id": id}).ToSql() + if err != nil { + return nil, errors.Wrap(err, "getPostWithCollapsedThreads_ToSql2") + } - err := s.GetReplicaX().Get(&post, postFetchQuery, args...) + err = s.GetReplicaX().Get(&post, postFetchQuery, args...) if err != nil { if err == sql.ErrNoRows { return nil, store.NewErrNotFound("Post", id) @@ -641,7 +643,7 @@ func (s *SqlPostStore) getPostWithCollapsedThreads(id, userID string, opts model sql, args, err := query.ToSql() if err != nil { - return nil, errors.Wrap(err, "getPostWithCollapsedThreads_Tosql") + return nil, errors.Wrap(err, "getPostWithCollapsedThreads_Tosql2") } err = s.GetReplicaX().Select(&posts, sql, args...) if err != nil { @@ -847,7 +849,7 @@ func (s *SqlPostStore) GetEtag(channelId string, allowFromCache, collapsedThread if collapsedThreads { q.Where(sq.Eq{"RootId": ""}) } - sql, args, _ := q.ToSql() + sql, args := q.MustSql() var et etagPosts err := s.GetReplicaX().Get(&et, sql, args...) @@ -863,12 +865,12 @@ func (s *SqlPostStore) GetEtag(channelId string, allowFromCache, collapsedThread // Soft deletes a post // and cleans up the thread if it's a comment -func (s *SqlPostStore) Delete(postID string, time int64, deleteByID string) error { +func (s *SqlPostStore) Delete(postID string, time int64, deleteByID string) (err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) id := postIds{} // TODO: change this to later delete thread directly from postID @@ -921,13 +923,13 @@ func (s *SqlPostStore) Delete(postID string, time int64, deleteByID string) erro return nil } -func (s *SqlPostStore) permanentDelete(postId string) error { +func (s *SqlPostStore) permanentDelete(postId string) (err error) { var post model.Post transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) err = transaction.Get(&post, "SELECT * FROM Posts WHERE Id = ?", postId) if err != nil && err != sql.ErrNoRows { @@ -954,13 +956,13 @@ type postIds struct { UserId string } -func (s *SqlPostStore) permanentDeleteAllCommentByUser(userId string) error { +func (s *SqlPostStore) permanentDeleteAllCommentByUser(userId string) (err error) { results := []postIds{} transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) err = transaction.Select(&results, "Select Id, RootId FROM Posts WHERE UserId = ? AND RootId != ''", userId) if err != nil { @@ -1028,12 +1030,12 @@ func (s *SqlPostStore) PermanentDeleteByUser(userId string) error { // Permanent deletes all channel root posts and comments, // deletes all threads and thread memberships // no thread comment cleanup needed, since we are deleting threads and thread memberships -func (s *SqlPostStore) PermanentDeleteByChannel(channelId string) error { +func (s *SqlPostStore) PermanentDeleteByChannel(channelId string) (err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) results := []postIds{} err = transaction.Select(&results, "SELECT Id, RootId, UserId FROM Posts WHERE ChannelId = ?", channelId) @@ -1219,7 +1221,7 @@ func (s *SqlPostStore) getPostsSinceCollapsedThreads(options model.GetPostsSince ) var posts []*postWithExtra - postFetchQuery, args, _ := s.getQueryBuilder(). + postFetchQuery, args, err := s.getQueryBuilder(). Select(columns...). From("Posts"). LeftJoin("Threads ON Threads.PostId = Posts.Id"). @@ -1229,8 +1231,11 @@ func (s *SqlPostStore) getPostsSinceCollapsedThreads(options model.GetPostsSince Where(sq.Gt{"Posts.UpdateAt": options.Time}). Where(sq.Eq{"Posts.RootId": ""}). OrderBy("Posts.CreateAt DESC").ToSql() + if err != nil { + return nil, errors.Wrapf(err, "getPostsSinceCollapsedThreads_ToSql") + } - err := s.GetReplicaX().Select(&posts, postFetchQuery, args...) + err = s.GetReplicaX().Select(&posts, postFetchQuery, args...) if err != nil { return nil, errors.Wrapf(err, "failed to find Posts with channelId=%s", options.ChannelId) } @@ -2661,13 +2666,13 @@ func (s *SqlPostStore) SearchPostsForUser(paramsList []*model.SearchParams, user const lastSearchesLimit = 5 -func (s *SqlPostStore) LogRecentSearch(userID string, searchQuery []byte, createAt int64) error { +func (s *SqlPostStore) LogRecentSearch(userID string, searchQuery []byte, createAt int64) (err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) var lastSearchPointer int var queryStr string @@ -2897,7 +2902,7 @@ func (s *SqlPostStore) updateThreadsFromPosts(transaction *sqlxTxWrapper, posts if len(rootIds) == 0 { return nil } - threadsByRootsSql, threadsByRootsArgs, _ := s.getQueryBuilder(). + threadsByRootsSql, threadsByRootsArgs, err := s.getQueryBuilder(). Select( "Threads.PostId", "Threads.ChannelId", @@ -2909,6 +2914,10 @@ func (s *SqlPostStore) updateThreadsFromPosts(transaction *sqlxTxWrapper, posts From("Threads"). Where(sq.Eq{"Threads.PostId": rootIds}). ToSql() + if err != nil { + return errors.Wrap(err, "updateThreadsFromPosts_ToSql") + } + threadsByRoots := []*model.Thread{} if err := transaction.Select(&threadsByRoots, threadsByRootsSql, threadsByRootsArgs...); err != nil { return err @@ -3133,7 +3142,7 @@ func (s *SqlPostStore) SetPostReminder(reminder *model.PostReminder) error { if err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) sql := `SELECT EXISTS (SELECT 1 FROM Posts WHERE Id=?)` var exist bool @@ -3169,14 +3178,14 @@ func (s *SqlPostStore) SetPostReminder(reminder *model.PostReminder) error { return nil } -func (s *SqlPostStore) GetPostReminders(now int64) ([]*model.PostReminder, error) { +func (s *SqlPostStore) GetPostReminders(now int64) (_ []*model.PostReminder, err error) { reminders := []*model.PostReminder{} transaction, err := s.GetMasterX().Beginx() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) err = transaction.Select(&reminders, `SELECT PostId, UserId FROM PostReminders diff --git a/store/sqlstore/preference_store.go b/store/sqlstore/preference_store.go index beea133fa5..74dffd3d50 100644 --- a/store/sqlstore/preference_store.go +++ b/store/sqlstore/preference_store.go @@ -29,21 +29,21 @@ func (s SqlPreferenceStore) deleteUnusedFeatures() { Where(sq.Eq{"Value": "false"}). Where(sq.Like{"Name": store.FeatureTogglePrefix + "%"}).ToSql() if err != nil { - mlog.Warn(errors.Wrap(err, "could not build sql query to delete unused features!").Error()) + mlog.Warn("Could not build sql query to delete unused features", mlog.Err(err)) } if _, err = s.GetMasterX().Exec(sql, args...); err != nil { mlog.Warn("Failed to delete unused features", mlog.Err(err)) } } -func (s SqlPreferenceStore) Save(preferences model.Preferences) error { +func (s SqlPreferenceStore) Save(preferences model.Preferences) (err error) { // wrap in a transaction so that if one fails, everything fails transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) for _, preference := range preferences { preference := preference if upsertErr := s.saveTx(transaction, &preference); upsertErr != nil { diff --git a/store/sqlstore/product_notices_store.go b/store/sqlstore/product_notices_store.go index 631f541bf4..4bce03d2cd 100644 --- a/store/sqlstore/product_notices_store.go +++ b/store/sqlstore/product_notices_store.go @@ -49,19 +49,22 @@ func (s SqlProductNoticesStore) ClearOldNotices(currentNotices model.ProductNoti return nil } -func (s SqlProductNoticesStore) View(userId string, notices []string) error { +func (s SqlProductNoticesStore) View(userId string, notices []string) (err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) noticeStates := []model.ProductNoticeViewState{} - sql, args, _ := s.getQueryBuilder(). + sql, args, err := s.getQueryBuilder(). Select("*"). From("ProductNoticeViewState"). Where(sq.And{sq.Eq{"UserId": userId}, sq.Eq{"NoticeId": notices}}). ToSql() + if err != nil { + return errors.Wrap(err, "View_ToSql") + } if err := transaction.Select(¬iceStates, sql, args...); err != nil { return errors.Wrapf(err, "failed to get ProductNoticeViewState with userId=%s", userId) } diff --git a/store/sqlstore/reaction_store.go b/store/sqlstore/reaction_store.go index d1a56b6e08..db79b1d405 100644 --- a/store/sqlstore/reaction_store.go +++ b/store/sqlstore/reaction_store.go @@ -21,7 +21,7 @@ func newSqlReactionStore(sqlStore *SqlStore) store.ReactionStore { return &SqlReactionStore{sqlStore} } -func (s *SqlReactionStore) Save(reaction *model.Reaction) (*model.Reaction, error) { +func (s *SqlReactionStore) Save(reaction *model.Reaction) (re *model.Reaction, err error) { reaction.PreSave() if err := reaction.IsValid(); err != nil { return nil, err @@ -30,7 +30,7 @@ func (s *SqlReactionStore) Save(reaction *model.Reaction) (*model.Reaction, erro if err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) if reaction.ChannelId == "" { // get channelId, if not already populated var channelIds []string @@ -58,14 +58,14 @@ func (s *SqlReactionStore) Save(reaction *model.Reaction) (*model.Reaction, erro return reaction, nil } -func (s *SqlReactionStore) Delete(reaction *model.Reaction) (*model.Reaction, error) { +func (s *SqlReactionStore) Delete(reaction *model.Reaction) (re *model.Reaction, err error) { reaction.PreUpdate() transaction, err := s.GetMasterX().Beginx() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) if err := deleteReactionAndUpdatePost(transaction, reaction); err != nil { return nil, errors.Wrap(err, "deleteReactionAndUpdatePost") @@ -189,7 +189,7 @@ func (s *SqlReactionStore) DeleteAllWithEmojiName(emojiName string) error { for _, reaction := range reactions { reaction := reaction - _, err := s.GetMasterX().Exec(UpdatePostHasReactionsOnDeleteQuery, model.GetMillis(), reaction.PostId, reaction.PostId) + _, err := s.GetMasterX().Exec(UpdatePostHasReactionsOnDeleteQuery, now, reaction.PostId, reaction.PostId) if err != nil { mlog.Warn("Unable to update Post.HasReactions while removing reactions", mlog.String("post_id", reaction.PostId), diff --git a/store/sqlstore/retention_policy_store.go b/store/sqlstore/retention_policy_store.go index 118a27691a..34619915d9 100644 --- a/store/sqlstore/retention_policy_store.go +++ b/store/sqlstore/retention_policy_store.go @@ -39,16 +39,16 @@ func executePossiblyEmptyQuery(txn *sqlxTxWrapper, query string, args ...any) (s return txn.Exec(query, args...) } -func (s *SqlRetentionPolicyStore) Save(policy *model.RetentionPolicyWithTeamAndChannelIDs) (*model.RetentionPolicyWithTeamAndChannelCounts, error) { +func (s *SqlRetentionPolicyStore) Save(policy *model.RetentionPolicyWithTeamAndChannelIDs) (_ *model.RetentionPolicyWithTeamAndChannelCounts, err error) { // Strategy: // 1. Insert new policy // 2. Insert new channels into policy // 3. Insert new teams into policy - if err := s.checkTeamsExist(policy.TeamIDs); err != nil { + if err = s.checkTeamsExist(policy.TeamIDs); err != nil { return nil, err } - if err := s.checkChannelsExist(policy.ChannelIDs); err != nil { + if err = s.checkChannelsExist(policy.ChannelIDs); err != nil { return nil, err } @@ -82,7 +82,8 @@ func (s *SqlRetentionPolicyStore) Save(policy *model.RetentionPolicyWithTeamAndC if err != nil { return nil, err } - defer finalizeTransactionX(txn) + defer finalizeTransactionX(txn, &err) + // Create a new policy in RetentionPolicies if _, err = txn.Exec(policyInsertQuery, policyInsertArgs...); err != nil { return nil, err @@ -195,7 +196,7 @@ func (s *SqlRetentionPolicyStore) buildInsertRetentionPoliciesTeamsQuery(policyI return } -func (s *SqlRetentionPolicyStore) Patch(patch *model.RetentionPolicyWithTeamAndChannelIDs) (*model.RetentionPolicyWithTeamAndChannelCounts, error) { +func (s *SqlRetentionPolicyStore) Patch(patch *model.RetentionPolicyWithTeamAndChannelIDs) (_ *model.RetentionPolicyWithTeamAndChannelCounts, err error) { // Strategy: // 1. Update policy attributes // 2. Delete existing channels from policy @@ -204,7 +205,6 @@ func (s *SqlRetentionPolicyStore) Patch(patch *model.RetentionPolicyWithTeamAndC // 5. Insert new teams into policy // 6. Read new policy - var err error if err = s.checkTeamsExist(patch.TeamIDs); err != nil { return nil, err } @@ -277,7 +277,8 @@ func (s *SqlRetentionPolicyStore) Patch(patch *model.RetentionPolicyWithTeamAndC if err != nil { return nil, err } - defer finalizeTransactionX(txn) + defer finalizeTransactionX(txn, &err) + // Update the fields of the policy in RetentionPolicies if _, err = executePossiblyEmptyQuery(txn, policyUpdateQuery, policyUpdateArgs...); err != nil { return nil, err @@ -638,7 +639,7 @@ func (s *SqlRetentionPolicyStore) RemoveTeams(policyId string, teamIds []string) } func subQueryIN(property string, query sq.SelectBuilder) sq.Sqlizer { - queryString, args, _ := query.ToSql() + queryString, args := query.MustSql() subQuery := fmt.Sprintf("%s IN (SELECT * FROM (%s) AS A)", property, queryString) return sq.Expr(subQuery, args...) } diff --git a/store/sqlstore/role_store.go b/store/sqlstore/role_store.go index ae22126d2d..22ee23f1d9 100644 --- a/store/sqlstore/role_store.go +++ b/store/sqlstore/role_store.go @@ -86,24 +86,24 @@ func newSqlRoleStore(sqlStore *SqlStore) store.RoleStore { return &SqlRoleStore{sqlStore} } -func (s *SqlRoleStore) Save(role *model.Role) (*model.Role, error) { +func (s *SqlRoleStore) Save(role *model.Role) (_ *model.Role, err error) { // Check the role is valid before proceeding. if !role.IsValidWithoutId() { return nil, store.NewErrInvalidInput("Role", "", fmt.Sprintf("%v", role)) } if role.Id == "" { - transaction, err := s.GetMasterX().Beginx() - if err != nil { - return nil, errors.Wrap(err, "begin_transaction") + transaction, terr := s.GetMasterX().Beginx() + if terr != nil { + return nil, errors.Wrap(terr, "begin_transaction") } - defer finalizeTransactionX(transaction) - createdRole, err := s.createRole(role, transaction) - if err != nil { - _ = transaction.Rollback() - return nil, errors.Wrap(err, "unable to create Role") - } else if err := transaction.Commit(); err != nil { - return nil, errors.Wrap(err, "commit_transaction") + defer finalizeTransactionX(transaction, &terr) + + createdRole, terr := s.createRole(role, transaction) + if terr != nil { + return nil, errors.Wrap(terr, "unable to create Role") + } else if terr = transaction.Commit(); terr != nil { + return nil, errors.Wrap(terr, "commit_transaction") } return createdRole, nil } diff --git a/store/sqlstore/scheme_store.go b/store/sqlstore/scheme_store.go index 5a3fb69c31..a31ad5a27c 100644 --- a/store/sqlstore/scheme_store.go +++ b/store/sqlstore/scheme_store.go @@ -22,20 +22,20 @@ func newSqlSchemeStore(sqlStore *SqlStore) store.SchemeStore { return &SqlSchemeStore{sqlStore} } -func (s *SqlSchemeStore) Save(scheme *model.Scheme) (*model.Scheme, error) { +func (s *SqlSchemeStore) Save(scheme *model.Scheme) (_ *model.Scheme, err error) { if scheme.Id == "" { - transaction, err := s.GetMasterX().Beginx() - if err != nil { - return nil, errors.Wrap(err, "begin_transaction") + transaction, terr := s.GetMasterX().Beginx() + if terr != nil { + return nil, errors.Wrap(terr, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &terr) - newScheme, err := s.createScheme(scheme, transaction) - if err != nil { - return nil, err + newScheme, terr := s.createScheme(scheme, transaction) + if terr != nil { + return nil, terr } - if err := transaction.Commit(); err != nil { - return nil, errors.Wrap(err, "commit_transaction") + if terr = transaction.Commit(); terr != nil { + return nil, errors.Wrap(terr, "commit_transaction") } return newScheme, nil } @@ -427,7 +427,7 @@ func (s *SqlSchemeStore) CountByScope(scope string) (int64, error) { err := s.GetReplicaX().Get(&count, `SELECT count(*) FROM Schemes WHERE Scope = ? AND DeleteAt = 0`, scope) if err != nil { - return int64(0), errors.Wrap(err, "failed to count Schemes by scope") + return 0, errors.Wrap(err, "failed to count Schemes by scope") } return count, nil } @@ -448,7 +448,7 @@ func (s *SqlSchemeStore) CountWithoutPermission(schemeScope, permissionID string var count int64 err := s.GetReplicaX().Get(&count, query) if err != nil { - return int64(0), errors.Wrap(err, "failed to count Schemes without permission") + return 0, errors.Wrap(err, "failed to count Schemes without permission") } return count, nil } diff --git a/store/sqlstore/shared_channel_store.go b/store/sqlstore/shared_channel_store.go index 379ff2f1f6..551c63544f 100644 --- a/store/sqlstore/shared_channel_store.go +++ b/store/sqlstore/shared_channel_store.go @@ -29,7 +29,7 @@ func newSqlSharedChannelStore(sqlStore *SqlStore) store.SharedChannelStore { } // Save inserts a new shared channel record. -func (s SqlSharedChannelStore) Save(sc *model.SharedChannel) (*model.SharedChannel, error) { +func (s SqlSharedChannelStore) Save(sc *model.SharedChannel) (sh *model.SharedChannel, err error) { sc.PreSave() if err := sc.IsValid(); err != nil { return nil, err @@ -45,7 +45,7 @@ func (s SqlSharedChannelStore) Save(sc *model.SharedChannel) (*model.SharedChann if err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) query, args, err := s.getQueryBuilder().Insert("SharedChannels"). Columns("ChannelId", "TeamId", "Home", "ReadOnly", "ShareName", "ShareDisplayName", "SharePurpose", "ShareHeader", "CreatorId", "CreateAt", "UpdateAt", "RemoteId"). @@ -246,12 +246,12 @@ func (s SqlSharedChannelStore) Update(sc *model.SharedChannel) (*model.SharedCha // Delete deletes a single shared channel plus associated SharedChannelRemotes. // Returns true if shared channel found and deleted, false if not found. -func (s SqlSharedChannelStore) Delete(channelId string) (bool, error) { +func (s SqlSharedChannelStore) Delete(channelId string) (ok bool, err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return false, errors.Wrap(err, "DeleteSharedChannel: begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) squery, args, err := s.getQueryBuilder(). Delete("SharedChannels"). diff --git a/store/sqlstore/status_store.go b/store/sqlstore/status_store.go index 67c7c3de44..30db08d268 100644 --- a/store/sqlstore/status_store.go +++ b/store/sqlstore/status_store.go @@ -137,19 +137,19 @@ func (s SqlStatusStore) updateExpiredStatuses(t *sqlxTxWrapper) ([]*model.Status return statuses, nil } -func (s SqlStatusStore) UpdateExpiredDNDStatuses() ([]*model.Status, error) { +func (s SqlStatusStore) UpdateExpiredDNDStatuses() (_ []*model.Status, err error) { if s.DriverName() == model.DatabaseDriverMysql { - transaction, err := s.GetMasterX().Beginx() - if err != nil { - return nil, errors.Wrap(err, "UpdateExpiredDNDStatuses: begin_transaction") + transaction, terr := s.GetMasterX().Beginx() + if terr != nil { + return nil, errors.Wrap(terr, "UpdateExpiredDNDStatuses: begin_transaction") } - defer finalizeTransactionX(transaction) - statuses, err := s.updateExpiredStatuses(transaction) - if err != nil { - return nil, errors.Wrap(err, "UpdateExpiredDNDStatuses: updateExpiredDNDStatusesT") + defer finalizeTransactionX(transaction, &terr) + statuses, terr := s.updateExpiredStatuses(transaction) + if terr != nil { + return nil, errors.Wrap(terr, "UpdateExpiredDNDStatuses: updateExpiredDNDStatusesT") } - if err := transaction.Commit(); err != nil { - return nil, errors.Wrap(err, "UpdateExpiredDNDStatuses: commit_transaction") + if terr = transaction.Commit(); terr != nil { + return nil, errors.Wrap(terr, "UpdateExpiredDNDStatuses: commit_transaction") } for _, status := range statuses { diff --git a/store/sqlstore/store.go b/store/sqlstore/store.go index f37e27797a..eea25d0ce1 100644 --- a/store/sqlstore/store.go +++ b/store/sqlstore/store.go @@ -1118,15 +1118,15 @@ func (ss *SqlStore) ensureMinimumDBVersion(ver string) (bool, error) { } majorVer, err2 := strconv.Atoi(versions[0]) if err2 != nil { - return false, fmt.Errorf("cannot parse MySQL DB version: %s", err2) + return false, fmt.Errorf("cannot parse MySQL DB version: %w", err2) } minorVer, err2 := strconv.Atoi(versions[1]) if err2 != nil { - return false, fmt.Errorf("cannot parse MySQL DB version: %s", err2) + return false, fmt.Errorf("cannot parse MySQL DB version: %w", err2) } patchVer, err2 := strconv.Atoi(versions[2]) if err2 != nil { - return false, fmt.Errorf("cannot parse MySQL DB version: %s", err2) + return false, fmt.Errorf("cannot parse MySQL DB version: %w", err2) } intVer := majorVer*1000 + minorVer*100 + patchVer if intVer < minimumRequiredMySQLVersion { diff --git a/store/sqlstore/system_store.go b/store/sqlstore/system_store.go index b6a394bb25..37756e0bd3 100644 --- a/store/sqlstore/system_store.go +++ b/store/sqlstore/system_store.go @@ -124,14 +124,14 @@ func (s SqlSystemStore) PermanentDeleteByName(name string) (*model.System, error // InsertIfExists inserts a given system value if it does not already exist. If a value // already exists, it returns the old one, else returns the new one. -func (s SqlSystemStore) InsertIfExists(system *model.System) (*model.System, error) { +func (s SqlSystemStore) InsertIfExists(system *model.System) (_ *model.System, err error) { tx, err := s.GetMasterX().BeginXWithIsolation(&sql.TxOptions{ Isolation: sql.LevelSerializable, }) if err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(tx) + defer finalizeTransactionX(tx, &err) var origSystem model.System if err := tx.Get(&origSystem, `SELECT * FROM Systems diff --git a/store/sqlstore/team_store.go b/store/sqlstore/team_store.go index 10018bd3a8..63da971abb 100644 --- a/store/sqlstore/team_store.go +++ b/store/sqlstore/team_store.go @@ -1282,14 +1282,13 @@ func (s SqlTeamStore) GetTeamsByScheme(schemeId string, offset int, limit int) ( // in batches as a single transaction per batch to ensure consistency but to also minimise execution time to avoid // causing unnecessary table locks. **THIS FUNCTION SHOULD NOT BE USED FOR ANY OTHER PURPOSE.** Executing this function // *after* the new Schemes functionality has been used on an installation will have unintended consequences. -func (s SqlTeamStore) MigrateTeamMembers(fromTeamId string, fromUserId string) (map[string]string, error) { +func (s SqlTeamStore) MigrateTeamMembers(fromTeamId string, fromUserId string) (_ map[string]string, err error) { var transaction *sqlxTxWrapper - var err error if transaction, err = s.GetMasterX().Beginx(); err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) teamMembers := []teamMember{} if err := transaction.Select(&teamMembers, "SELECT * from TeamMembers WHERE (TeamId, UserId) > (?, ?) ORDER BY TeamId, UserId LIMIT 100", fromTeamId, fromUserId); err != nil { @@ -1364,11 +1363,12 @@ func (s SqlTeamStore) ResetAllTeamSchemes() error { func (s SqlTeamStore) ClearCaches() {} // InvalidateAllTeamIdsForUser does not execute anything because the store does not handle the cache. +// //nolint:unparam func (s SqlTeamStore) InvalidateAllTeamIdsForUser(userId string) {} // ClearAllCustomRoleAssignments removes all custom role assignments from TeamMembers. -func (s SqlTeamStore) ClearAllCustomRoleAssignments() error { +func (s SqlTeamStore) ClearAllCustomRoleAssignments() (err error) { builtInRoles := model.MakeDefaultRoles() lastUserId := strings.Repeat("0", 26) @@ -1381,7 +1381,7 @@ func (s SqlTeamStore) ClearAllCustomRoleAssignments() error { if transaction, err = s.GetMasterX().Beginx(); err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) teamMembers := []*teamMember{} if err := transaction.Select(&teamMembers, "SELECT * from TeamMembers WHERE (TeamId, UserId) > (?, ?) ORDER BY TeamId, UserId LIMIT 1000", lastTeamId, lastUserId); err != nil { @@ -1464,6 +1464,7 @@ func (s SqlTeamStore) GetAllForExportAfter(limit int, afterId string) ([]*model. } // GetUserTeamIds get the team ids to which the user belongs to. allowFromCache parameter does not have any effect in this Store +// //nolint:unparam func (s SqlTeamStore) GetUserTeamIds(userId string, allowFromCache bool) ([]string, error) { teamIds := []string{} @@ -1533,7 +1534,7 @@ func (s SqlTeamStore) GetTeamMembersForExport(userId string) ([]*model.TeamMembe return members, nil } -//UserBelongsToTeams returns true if the user denoted by userId is a member of the teams in the teamIds string array. +// UserBelongsToTeams returns true if the user denoted by userId is a member of the teams in the teamIds string array. func (s SqlTeamStore) UserBelongsToTeams(userId string, teamIds []string) (bool, error) { idQuery := sq.Eq{ "UserId": userId, diff --git a/store/sqlstore/thread_store.go b/store/sqlstore/thread_store.go index 04912c5c11..a20b4f9362 100644 --- a/store/sqlstore/thread_store.go +++ b/store/sqlstore/thread_store.go @@ -730,12 +730,12 @@ func (s *SqlThreadStore) DeleteMembershipForUser(userId string, postId string) e // - post creation (mentions handling) // - channel marked unread // - user explicitly following a thread -func (s *SqlThreadStore) MaintainMembership(userId, postId string, opts store.ThreadMembershipOpts) (*model.ThreadMembership, error) { +func (s *SqlThreadStore) MaintainMembership(userId, postId string, opts store.ThreadMembershipOpts) (_ *model.ThreadMembership, err error) { trx, err := s.GetMasterX().Beginx() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(trx) + defer finalizeTransactionX(trx, &err) membership, err := s.getMembershipForUser(trx, userId, postId) now := utils.MillisFromTime(time.Now()) diff --git a/store/sqlstore/user_access_token_store.go b/store/sqlstore/user_access_token_store.go index e6d4f811ed..6a91e29c17 100644 --- a/store/sqlstore/user_access_token_store.go +++ b/store/sqlstore/user_access_token_store.go @@ -41,13 +41,13 @@ func (s SqlUserAccessTokenStore) Save(token *model.UserAccessToken) (*model.User return token, nil } -func (s SqlUserAccessTokenStore) Delete(tokenId string) error { +func (s SqlUserAccessTokenStore) Delete(tokenId string) (err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) if err := s.deleteSessionsAndTokensById(transaction, tokenId); err == nil { if err := transaction.Commit(); err != nil { @@ -85,12 +85,12 @@ func (s SqlUserAccessTokenStore) deleteTokensById(transaction *sqlxTxWrapper, to return nil } -func (s SqlUserAccessTokenStore) DeleteAllForUser(userId string) error { +func (s SqlUserAccessTokenStore) DeleteAllForUser(userId string) (err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) if err := s.deleteSessionsandTokensByUser(transaction, userId); err != nil { return err } @@ -197,12 +197,12 @@ func (s SqlUserAccessTokenStore) UpdateTokenEnable(tokenId string) error { return nil } -func (s SqlUserAccessTokenStore) UpdateTokenDisable(tokenId string) error { +func (s SqlUserAccessTokenStore) UpdateTokenDisable(tokenId string) (err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) if err := s.deleteSessionsAndDisableToken(transaction, tokenId); err != nil { return err diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 6583971640..b5b60c3d57 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -1664,7 +1664,7 @@ func (us SqlUserStore) GetEtagForProfilesNotInTeam(teamId string) string { return fmt.Sprintf("%v.%v", model.CurrentVersion, etag) } -func (us SqlUserStore) ClearAllCustomRoleAssignments() error { +func (us SqlUserStore) ClearAllCustomRoleAssignments() (err error) { builtInRoles := model.MakeDefaultRoles() lastUserId := strings.Repeat("0", 26) @@ -1675,7 +1675,7 @@ func (us SqlUserStore) ClearAllCustomRoleAssignments() error { if transaction, err = us.GetMasterX().Beginx(); err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) users := []*model.User{} if err := transaction.Select(&users, "SELECT * from Users WHERE Id > ? ORDER BY Id LIMIT 1000", lastUserId); err != nil { @@ -1728,7 +1728,7 @@ func (us SqlUserStore) InferSystemInstallDate() (int64, error) { func (us SqlUserStore) GetUsersBatchForIndexing(startTime int64, startFileID string, limit int) ([]*model.UserForIndexing, error) { users := []*model.User{} - usersQuery, args, _ := us.usersQuery. + usersQuery, args, err := us.usersQuery. Where(sq.Or{ sq.Gt{"u.CreateAt": startTime}, sq.And{ @@ -1739,7 +1739,11 @@ func (us SqlUserStore) GetUsersBatchForIndexing(startTime int64, startFileID str OrderBy("u.CreateAt ASC, u.Id ASC"). Limit(uint64(limit)). ToSql() - err := us.GetSearchReplicaX().Select(&users, usersQuery, args...) + if err != nil { + return nil, errors.Wrap(err, "GetUsersBatchForIndexing_ToSql1") + } + + err = us.GetSearchReplicaX().Select(&users, usersQuery, args...) if err != nil { return nil, errors.Wrap(err, "failed to find Users") } @@ -1750,7 +1754,7 @@ func (us SqlUserStore) GetUsersBatchForIndexing(startTime int64, startFileID str } channelMembers := []*model.ChannelMember{} - channelMembersQuery, args, _ := us.getQueryBuilder(). + channelMembersQuery, args, err := us.getQueryBuilder(). Select(` cm.ChannelId, cm.UserId, @@ -1769,17 +1773,25 @@ func (us SqlUserStore) GetUsersBatchForIndexing(startTime int64, startFileID str Join("Channels c ON cm.ChannelId = c.Id"). Where(sq.Eq{"c.Type": model.ChannelTypeOpen, "cm.UserId": userIds}). ToSql() + if err != nil { + return nil, errors.Wrap(err, "GetUsersBatchForIndexing_ToSql2") + } + err = us.GetSearchReplicaX().Select(&channelMembers, channelMembersQuery, args...) if err != nil { return nil, errors.Wrap(err, "failed to find ChannelMembers") } teamMembers := []*model.TeamMember{} - teamMembersQuery, args, _ := us.getQueryBuilder(). - Select("TeamId, UserId, Roles, DeleteAt, CreateAt, (SchemeGuest IS NOT NULL AND SchemeGuest) as SchemeGuest, SchemeUser, SchemeAdmin"). + teamMembersQuery, args, err := us.getQueryBuilder(). + Select("TeamId, UserId, Roles, DeleteAt, (SchemeGuest IS NOT NULL AND SchemeGuest) as SchemeGuest, SchemeUser, SchemeAdmin"). From("TeamMembers"). Where(sq.Eq{"UserId": userIds, "DeleteAt": 0}). ToSql() + if err != nil { + return nil, errors.Wrap(err, "GetUsersBatchForIndexing_ToSql3") + } + err = us.GetSearchReplicaX().Select(&teamMembers, teamMembersQuery, args...) if err != nil { return nil, errors.Wrap(err, "failed to find TeamMembers") @@ -1896,12 +1908,12 @@ func applyViewRestrictionsFilter(query sq.SelectBuilder, restrictions *model.Vie return resultQuery } -func (us SqlUserStore) PromoteGuestToUser(userId string) error { +func (us SqlUserStore) PromoteGuestToUser(userId string) (err error) { transaction, err := us.GetMasterX().Beginx() if err != nil { return errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) user, err := us.Get(context.Background(), userId) if err != nil { @@ -1965,12 +1977,12 @@ func (us SqlUserStore) PromoteGuestToUser(userId string) error { return nil } -func (us SqlUserStore) DemoteUserToGuest(userID string) (*model.User, error) { +func (us SqlUserStore) DemoteUserToGuest(userID string) (_ *model.User, err error) { transaction, err := us.GetMasterX().Beginx() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransactionX(transaction) + defer finalizeTransactionX(transaction, &err) user, err := us.Get(context.Background(), userID) if err != nil { @@ -2070,14 +2082,18 @@ func (us SqlUserStore) AutocompleteUsersInChannel(teamId, channelId, term string // direct and group channels. func (us SqlUserStore) GetKnownUsers(userId string) ([]string, error) { userIds := []string{} - usersQuery, args, _ := us.getQueryBuilder(). + usersQuery, args, err := us.getQueryBuilder(). Select("DISTINCT ocm.UserId"). From("ChannelMembers AS cm"). Join("ChannelMembers AS ocm ON ocm.ChannelId = cm.ChannelId"). Where(sq.NotEq{"ocm.UserId": userId}). Where(sq.Eq{"cm.UserId": userId}). ToSql() - err := us.GetSearchReplicaX().Select(&userIds, usersQuery, args...) + if err != nil { + return nil, errors.Wrap(err, "GetKnownUsers_ToSql") + } + + err = us.GetSearchReplicaX().Select(&userIds, usersQuery, args...) if err != nil { return nil, errors.Wrap(err, "failed to find ChannelMembers") } diff --git a/store/sqlstore/user_terms_of_service.go b/store/sqlstore/user_terms_of_service.go index c99b777cbb..3ff1b7e429 100644 --- a/store/sqlstore/user_terms_of_service.go +++ b/store/sqlstore/user_terms_of_service.go @@ -54,7 +54,11 @@ func (s SqlUserTermsOfServiceStore) Save(userTermsOfService *model.UserTermsOfSe return nil, errors.Wrapf(err, "failed to update UserTermsOfService with userId=%s and termsOfServiceId=%s", userTermsOfService.UserId, userTermsOfService.TermsOfServiceId) } - if updatedRows, _ := result.RowsAffected(); updatedRows == 0 { + updatedRows, err := result.RowsAffected() + if err != nil { + return nil, errors.Wrap(err, "failed to retrieve the number of affected rows for the update of UserTermsOfService") + } + if updatedRows == 0 { query := ` INSERT INTO UserTermsOfService (UserId, TermsOfServiceId, CreateAt) diff --git a/store/sqlstore/utils.go b/store/sqlstore/utils.go index d40694e418..e7fe4958d9 100644 --- a/store/sqlstore/utils.go +++ b/store/sqlstore/utils.go @@ -5,6 +5,7 @@ package sqlstore import ( "database/sql" + "io" "net/url" "strconv" "strings" @@ -12,6 +13,7 @@ import ( "github.com/mattermost/mattermost-server/v6/model" "github.com/mattermost/mattermost-server/v6/shared/mlog" + "github.com/wiggin77/merror" "github.com/go-sql-driver/mysql" ) @@ -50,13 +52,18 @@ func MapStringsToQueryParams(list []string, paramPrefix string) (string, map[str } // finalizeTransactionX ensures a transaction is closed after use, rolling back if not already committed. -func finalizeTransactionX(transaction *sqlxTxWrapper) { +func finalizeTransactionX(transaction *sqlxTxWrapper, perr *error) { // Rollback returns sql.ErrTxDone if the transaction was already closed. if err := transaction.Rollback(); err != nil && err != sql.ErrTxDone { - mlog.Error("Failed to rollback transaction", mlog.Err(err)) + *perr = merror.Append(*perr, err) } } +func deferClose(c io.Closer, perr *error) { + err := c.Close() + *perr = merror.Append(*perr, err) +} + // removeNonAlphaNumericUnquotedTerms removes all unquoted words that only contain // non-alphanumeric chars from given line func removeNonAlphaNumericUnquotedTerms(line, separator string) string { @@ -82,8 +89,9 @@ func containsAlphaNumericChar(s string) bool { } // isQuotedWord return true if the input string is quoted, false otherwise. Ex :- -// "quoted string" - will return true -// unquoted string - will return false +// +// "quoted string" - will return true +// unquoted string - will return false func isQuotedWord(s string) bool { if len(s) < 2 { return false