diff --git a/e2e-tests/cypress/tests/integration/channels/system_console/site_configuration/link_customization_cloud_spec.js b/e2e-tests/cypress/tests/integration/channels/system_console/site_configuration/link_customization_cloud_spec.js index 47012ca7261..88732976cd2 100644 --- a/e2e-tests/cypress/tests/integration/channels/system_console/site_configuration/link_customization_cloud_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/system_console/site_configuration/link_customization_cloud_spec.js @@ -28,7 +28,7 @@ describe('SupportSettings', () => { [ {text: 'Ask the community', link: SupportSettings.ASK_COMMUNITY_LINK}, {text: 'Mattermost user guide', link: SupportSettings.MATTERMOST_USER_GUIDE}, - {text: 'Report a problem', link: SupportSettings.REPORT_A_PROBLEM_LINK}, + {text: 'Report a problem', link: 'mailto:reportaproblem@mattermost.com'}, {text: 'Keyboard shortcuts'}, ].forEach(({text, link}) => { if (link) { diff --git a/e2e-tests/cypress/tests/integration/channels/system_console/ui_and_api/customization_not_cloud_spec.js b/e2e-tests/cypress/tests/integration/channels/system_console/ui_and_api/customization_not_cloud_spec.js index 60e0d1ad6b2..07cad0354b4 100644 --- a/e2e-tests/cypress/tests/integration/channels/system_console/ui_and_api/customization_not_cloud_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/system_console/ui_and_api/customization_not_cloud_spec.js @@ -28,14 +28,14 @@ describe('Customization', () => { }); it('MM-T1214 - Can change Report a Problem Link setting', () => { - // * Verify Report a Problem link label is visible and matches the text - cy.findByTestId('SupportSettings.ReportAProblemLinklabel').scrollIntoView().should('be.visible').and('have.text', 'Report a Problem Link:'); + // # Select 'Custom link' from the Report a Problem dropdown + cy.findByTestId('SupportSettings.ReportAProblemTypedropdown').scrollIntoView().select('Custom link'); - // * Verify Report a Problem link input box has default value. The default value depends on the setup before running the test. - cy.findByTestId('SupportSettings.ReportAProblemLinkinput').should('have.value', origConfig.SupportSettings.ReportAProblemLink); + // * Verify Report a Problem link label is visible and matches the text + cy.findByTestId('SupportSettings.ReportAProblemLinklabel').scrollIntoView().should('be.visible').and('have.text', 'Custom Report a Problem Link:'); // * Verify Report a Problem link help text is visible and matches the text - cy.findByTestId('SupportSettings.ReportAProblemLinkhelp-text').find('span').should('be.visible').and('have.text', 'The URL for the Report a Problem link in the Help Menu. If this field is empty, the link is removed from the Help Menu.'); + cy.findByTestId('SupportSettings.ReportAProblemLinkhelp-text').find('span').should('be.visible').and('have.text', 'Enter the URL that users will be directed to when they choose "Report a Problem".'); // # Enter a problem link const reportAProblemLink = 'https://mattermost.com/pl/report-a-bug'; diff --git a/e2e-tests/cypress/tests/utils/constants.js b/e2e-tests/cypress/tests/utils/constants.js index 321e798b74b..19518fec585 100644 --- a/e2e-tests/cypress/tests/utils/constants.js +++ b/e2e-tests/cypress/tests/utils/constants.js @@ -6,7 +6,6 @@ export const ABOUT_LINK = 'https://mattermost.com/pl/about-mattermost'; export const ASK_COMMUNITY_LINK = 'https://mattermost.com/pl/default-ask-mattermost-community/'; export const HELP_LINK = 'https://mattermost.com/pl/help/'; export const PRIVACY_POLICY_LINK = 'https://mattermost.com/pl/privacy-policy/'; -export const REPORT_A_PROBLEM_LINK = 'https://mattermost.com/pl/report-a-bug'; export const TERMS_OF_SERVICE_LINK = 'https://mattermost.com/pl/terms-of-use/'; export const MATTERMOST_USER_GUIDE = 'https://docs.mattermost.com/guides/use-mattermost.html'; @@ -24,7 +23,6 @@ export const SupportSettings = { ASK_COMMUNITY_LINK, HELP_LINK, PRIVACY_POLICY_LINK, - REPORT_A_PROBLEM_LINK, TERMS_OF_SERVICE_LINK, MATTERMOST_USER_GUIDE, }; diff --git a/server/Makefile b/server/Makefile index 81970e5e986..ea33ed33a93 100644 --- a/server/Makefile +++ b/server/Makefile @@ -158,7 +158,7 @@ TEMPLATES_DIR=templates # Plugins Packages PLUGIN_PACKAGES ?= $(PLUGIN_PACKAGES:) -PLUGIN_PACKAGES += mattermost-plugin-calls-v1.11.4 +PLUGIN_PACKAGES += mattermost-plugin-calls-v1.11.5 PLUGIN_PACKAGES += mattermost-plugin-github-v2.7.1 PLUGIN_PACKAGES += mattermost-plugin-gitlab-v1.12.2 PLUGIN_PACKAGES += mattermost-plugin-jira-v4.7.0 diff --git a/server/channels/store/sqlstore/access_control_policy_store.go b/server/channels/store/sqlstore/access_control_policy_store.go index bfc506bd500..0ae4983afeb 100644 --- a/server/channels/store/sqlstore/access_control_policy_store.go +++ b/server/channels/store/sqlstore/access_control_policy_store.go @@ -186,7 +186,7 @@ func (s *SqlAccessControlPolicyStore) Save(rctx request.CTX, policy *model.Acces return nil, err } - tx, err := s.GetMaster().Beginx() + tx, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "failed to start transaction") } @@ -303,7 +303,7 @@ func (s *SqlAccessControlPolicyStore) Save(rctx request.CTX, policy *model.Acces } func (s *SqlAccessControlPolicyStore) Delete(rctx request.CTX, id string) error { - tx, err := s.GetMaster().Beginx() + tx, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "failed to start transaction") } @@ -360,7 +360,7 @@ func (s *SqlAccessControlPolicyStore) deleteT(_ request.CTX, tx *sqlxTxWrapper, } func (s *SqlAccessControlPolicyStore) SetActiveStatus(rctx request.CTX, id string, active bool) (*model.AccessControlPolicy, error) { - tx, err := s.GetMaster().Beginx() + tx, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "failed to start transaction") } @@ -410,7 +410,7 @@ func (s *SqlAccessControlPolicyStore) SetActiveStatus(rctx request.CTX, id strin } func (s *SqlAccessControlPolicyStore) SetActiveStatusMultiple(rctx request.CTX, list []model.AccessControlPolicyActiveUpdate) ([]*model.AccessControlPolicy, error) { - tx, err := s.GetMaster().Beginx() + tx, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "failed to start transaction") } diff --git a/server/channels/store/sqlstore/attributes_store.go b/server/channels/store/sqlstore/attributes_store.go index 69140069c28..9c6cff78f2b 100644 --- a/server/channels/store/sqlstore/attributes_store.go +++ b/server/channels/store/sqlstore/attributes_store.go @@ -66,10 +66,7 @@ func (s *SqlAttributesStore) GetSubject(rctx request.CTX, ID, groupID string) (* return nil, errors.Wrap(err, "failed to build query for subject") } - row := s.GetReplica().QueryRowxContext(rctx.Context(), q, args...) - if err := row.Err(); err != nil { - return nil, errors.Wrap(err, "failed to get subject") - } + row := s.GetReplica().QueryRowContext(rctx.Context(), q, args...) var subject model.Subject var properties []byte diff --git a/server/channels/store/sqlstore/channel_bookmark_store.go b/server/channels/store/sqlstore/channel_bookmark_store.go index 770060c06dd..f3da639c1ac 100644 --- a/server/channels/store/sqlstore/channel_bookmark_store.go +++ b/server/channels/store/sqlstore/channel_bookmark_store.go @@ -120,7 +120,7 @@ func (s *SqlChannelBookmarkStore) Save(bookmark *model.ChannelBookmark, increase return nil, err } - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, err } @@ -234,7 +234,7 @@ func (s *SqlChannelBookmarkStore) Update(bookmark *model.ChannelBookmark) error func (s *SqlChannelBookmarkStore) UpdateSortOrder(bookmarkId, channelId string, newIndex int64) ([]*model.ChannelBookmarkWithFileInfo, error) { now := model.GetMillis() - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, err } @@ -294,7 +294,7 @@ func (s *SqlChannelBookmarkStore) UpdateSortOrder(bookmarkId, channelId string, func (s *SqlChannelBookmarkStore) Delete(bookmarkId string, deleteFile bool) error { now := model.GetMillis() - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return err } diff --git a/server/channels/store/sqlstore/channel_store.go b/server/channels/store/sqlstore/channel_store.go index abfdc215b0c..4691f73df3f 100644 --- a/server/channels/store/sqlstore/channel_store.go +++ b/server/channels/store/sqlstore/channel_store.go @@ -630,7 +630,7 @@ func (s SqlChannelStore) Save(rctx request.CTX, channel *model.Channel, maxChann } var newChannel *model.Channel - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } @@ -694,7 +694,7 @@ func (s SqlChannelStore) SaveDirectChannel(rctx request.CTX, directChannel *mode return nil, store.NewErrInvalidInput("Channel", "Type", directChannel.Type) } - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } @@ -737,7 +737,7 @@ func (s SqlChannelStore) SaveBoardChannel(rctx request.CTX, channel *model.Chann return nil, nil, store.NewErrInvalidInput("View", "nil", "view is required for board channels") } - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, nil, errors.Wrap(err, "begin_transaction") } @@ -818,7 +818,7 @@ func (s SqlChannelStore) saveChannelT(transaction *sqlxTxWrapper, channel *model // Update writes the updated channel to the database. func (s SqlChannelStore) Update(rctx request.CTX, channel *model.Channel) (_ *model.Channel, err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } @@ -1033,7 +1033,7 @@ func (s SqlChannelStore) Restore(channelId string, time int64) error { func (s SqlChannelStore) SetDeleteAt(channelId string, deleteAt, updateAt int64) (err error) { defer s.InvalidateChannel(channelId) - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "SetDeleteAt: begin_transaction") } @@ -1077,7 +1077,7 @@ 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) (err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "PermanentDeleteByTeam: begin_transaction") } @@ -1114,7 +1114,7 @@ func (s SqlChannelStore) permanentDeleteByTeamtT(transaction *sqlxTxWrapper, tea // PermanentDelete removes the given channel from the database. func (s SqlChannelStore) PermanentDelete(rctx request.CTX, channelId string) (err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "PermanentDelete: begin_transaction") } @@ -1841,7 +1841,7 @@ func (s SqlChannelStore) saveMultipleMembers(members []*model.ChannelMember) (_ defaultTeamRolesByChannel[defaultRoles.Id] = defaultRoles } - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } @@ -1909,7 +1909,7 @@ func (s SqlChannelStore) UpdateMultipleMembers(members []*model.ChannelMember) ( var transaction *sqlxTxWrapper - if transaction, err = s.GetMaster().Beginx(); err != nil { + if transaction, err = s.GetMaster().Begin(); err != nil { return nil, errors.Wrap(err, "begin_transaction") } defer finalizeTransactionX(transaction, &err) @@ -1968,7 +1968,7 @@ func (s SqlChannelStore) UpdateMember(rctx request.CTX, member *model.ChannelMem } func (s SqlChannelStore) UpdateMemberNotifyProps(channelID, userID string, props map[string]string) (_ *model.ChannelMember, err error) { - tx, err := s.GetMaster().Beginx() + tx, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } @@ -2051,7 +2051,7 @@ func (s SqlChannelStore) PatchMultipleMembersNotifyProps(members []*model.Channe Set("LastUpdateAt", model.GetMillis()). Where(whereClause) - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } @@ -2462,7 +2462,7 @@ func (s SqlChannelStore) GetAllChannelMembersForUser(rctx request.CTX, userId st } defer deferClose(rows, &err) - scanner := func(rows *sql.Rows) (string, string, error) { + scanner := func(rows rowScanner) (string, string, error) { var cm allChannelMember err = rows.Scan( &cm.ChannelId, &cm.Roles, &cm.SchemeGuest, &cm.SchemeUser, @@ -2504,7 +2504,7 @@ func (s SqlChannelStore) GetChannelsMemberCount(channelIDs []string) (_ map[stri defaults[channelID] = 0 } - scanner := func(rows *sql.Rows) (string, int64, error) { + scanner := func(rows rowScanner) (string, int64, error) { var channelID string var count int64 err := rows.Scan(&channelID, &count) @@ -3153,7 +3153,7 @@ func (s SqlChannelStore) AnalyticsCountAll(teamId string) (map[model.ChannelType } defer rows.Close() - scanner := func(rows *sql.Rows) (model.ChannelType, int64, error) { + scanner := func(rows rowScanner) (model.ChannelType, int64, error) { var channelType model.ChannelType var count int64 err := rows.Scan(&channelType, &count) @@ -3970,7 +3970,7 @@ func (s SqlChannelStore) GetChannelsByScheme(schemeId string, offset int, limit func (s SqlChannelStore) MigrateChannelMembers(fromChannelId string, fromUserId string) (_ map[string]string, err error) { var transaction *sqlxTxWrapper - if transaction, err = s.GetMaster().Beginx(); err != nil { + if transaction, err = s.GetMaster().Begin(); err != nil { return nil, errors.Wrap(err, "begin_transaction") } defer finalizeTransactionX(transaction, &err) @@ -4064,7 +4064,7 @@ func (s SqlChannelStore) MigrateChannelMembers(fromChannelId string, fromUserId } func (s SqlChannelStore) ResetAllChannelSchemes() (err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } @@ -4098,7 +4098,7 @@ func (s SqlChannelStore) ClearAllCustomRoleAssignments() (err error) { for { var transaction *sqlxTxWrapper - if transaction, err = s.GetMaster().Beginx(); err != nil { + if transaction, err = s.GetMaster().Begin(); err != nil { return errors.Wrap(err, "begin_transaction") } diff --git a/server/channels/store/sqlstore/channel_store_categories.go b/server/channels/store/sqlstore/channel_store_categories.go index 4edbb439955..7cf02d0adad 100644 --- a/server/channels/store/sqlstore/channel_store_categories.go +++ b/server/channels/store/sqlstore/channel_store_categories.go @@ -17,7 +17,7 @@ import ( ) func (s SqlChannelStore) CreateInitialSidebarCategories(rctx request.CTX, userId string, teamID string) (_ *model.OrderedSidebarCategories, err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "CreateInitialSidebarCategories: begin_transaction") } @@ -183,7 +183,7 @@ 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, err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, err } @@ -230,7 +230,7 @@ type sidebarCategoryForJoin struct { } func (s SqlChannelStore) CreateSidebarCategory(userId, teamId string, newCategory *model.SidebarCategoryWithChannels) (_ *model.SidebarCategoryWithChannels, err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } @@ -590,7 +590,7 @@ func (s SqlChannelStore) updateSidebarCategoryOrderT(transaction *sqlxTxWrapper, } func (s SqlChannelStore) UpdateSidebarCategoryOrder(userId, teamId string, categoryOrder []string) (err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } @@ -627,7 +627,7 @@ func (s SqlChannelStore) UpdateSidebarCategoryOrder(userId, teamId string, categ //nolint:unparam func (s SqlChannelStore) UpdateSidebarCategories(userId, teamId string, categories []*model.SidebarCategoryWithChannels) (updated []*model.SidebarCategoryWithChannels, original []*model.SidebarCategoryWithChannels, err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, nil, errors.Wrap(err, "begin_transaction") } @@ -820,7 +820,7 @@ 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) (err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "UpdateSidebarChannelsByPreferences: begin_transaction") } @@ -951,7 +951,7 @@ 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) (err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "DeleteSidebarChannelsByPreferences: begin_transaction") } @@ -1011,7 +1011,7 @@ 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) (err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } diff --git a/server/channels/store/sqlstore/content_flagging_store.go b/server/channels/store/sqlstore/content_flagging_store.go index 7968b7c643d..2c1574e01be 100644 --- a/server/channels/store/sqlstore/content_flagging_store.go +++ b/server/channels/store/sqlstore/content_flagging_store.go @@ -17,7 +17,7 @@ func newContentFlaggingStore(sqlStore *SqlStore) *SqlContentFlaggingStore { } func (s *SqlContentFlaggingStore) SaveReviewerSettings(reviewerSettings model.ReviewerIDsSettings) error { - tx, err := s.GetMaster().Beginx() + tx, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "SqlContentFlaggingStore.SaveReviewerSettings failed to begin transaction") } diff --git a/server/channels/store/sqlstore/group_store.go b/server/channels/store/sqlstore/group_store.go index 6e9e5277882..3f37611c997 100644 --- a/server/channels/store/sqlstore/group_store.go +++ b/server/channels/store/sqlstore/group_store.go @@ -166,7 +166,7 @@ func (s *SqlGroupStore) CreateWithUserIds(g *model.GroupWithUserIds) (_ *model.G return nil, err } - txn, err := s.GetMaster().Beginx() + txn, err := s.GetMaster().Begin() if err != nil { return nil, err } @@ -1952,7 +1952,7 @@ func (s *SqlGroupStore) UpsertMembers(groupID string, userIDs []string) (_ []*mo return members, nil } - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } diff --git a/server/channels/store/sqlstore/job_store.go b/server/channels/store/sqlstore/job_store.go index 8e4471990e6..6b42c46b535 100644 --- a/server/channels/store/sqlstore/job_store.go +++ b/server/channels/store/sqlstore/job_store.go @@ -87,7 +87,7 @@ func (jss SqlJobStore) SaveOnce(job *model.Job) (*model.Job, error) { jsonData = AppendBinaryFlag(jsonData) } - tx, err := jss.GetMaster().BeginXWithIsolation(&sql.TxOptions{ + tx, err := jss.GetMaster().BeginWithIsolation(&sql.TxOptions{ Isolation: sql.LevelSerializable, }) if err != nil { diff --git a/server/channels/store/sqlstore/oauth_store.go b/server/channels/store/sqlstore/oauth_store.go index d4aa2e2f4ae..7caaf6b57c3 100644 --- a/server/channels/store/sqlstore/oauth_store.go +++ b/server/channels/store/sqlstore/oauth_store.go @@ -161,7 +161,7 @@ func (as SqlOAuthStore) GetAuthorizedApps(userId string, offset, limit int) ([]* func (as SqlOAuthStore) DeleteApp(id string) (err error) { // wrap in a transaction so that if one fails, everything fails - transaction, err := as.GetMaster().Beginx() + transaction, err := as.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } diff --git a/server/channels/store/sqlstore/plugin_store.go b/server/channels/store/sqlstore/plugin_store.go index 4fe47217efe..ba7b0eecd3a 100644 --- a/server/channels/store/sqlstore/plugin_store.go +++ b/server/channels/store/sqlstore/plugin_store.go @@ -217,7 +217,7 @@ func (ps SqlPluginStore) Get(pluginId, key string) (*model.PluginKeyValue, error return nil, errors.Wrap(err, "plugin_tosql") } - row := ps.GetReplica().QueryRowx(queryString, args...) + row := ps.GetReplica().QueryRow(queryString, args...) var kv model.PluginKeyValue if err := row.Scan(&kv.PluginId, &kv.Key, &kv.Value, &kv.ExpireAt); err != nil { if err == sql.ErrNoRows { diff --git a/server/channels/store/sqlstore/post_acknowledgements_store.go b/server/channels/store/sqlstore/post_acknowledgements_store.go index 923cc2aa0e8..3375344e73d 100644 --- a/server/channels/store/sqlstore/post_acknowledgements_store.go +++ b/server/channels/store/sqlstore/post_acknowledgements_store.go @@ -51,7 +51,7 @@ func (s *SqlPostAcknowledgementStore) SaveWithModel(acknowledgement *model.PostA acknowledgement.PreSave() - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } @@ -77,7 +77,7 @@ func (s *SqlPostAcknowledgementStore) SaveWithModel(acknowledgement *model.PostA } func (s *SqlPostAcknowledgementStore) Delete(acknowledgement *model.PostAcknowledgement) error { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } @@ -285,7 +285,7 @@ func (s *SqlPostAcknowledgementStore) BatchSave(acknowledgements []*model.PostAc } } - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } @@ -328,7 +328,7 @@ func (s *SqlPostAcknowledgementStore) BatchDelete(acknowledgements []*model.Post return nil } - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } diff --git a/server/channels/store/sqlstore/post_priority_store.go b/server/channels/store/sqlstore/post_priority_store.go index d6f157e7cff..452310c83fb 100644 --- a/server/channels/store/sqlstore/post_priority_store.go +++ b/server/channels/store/sqlstore/post_priority_store.go @@ -68,7 +68,7 @@ func (s *SqlPostPriorityStore) GetForPosts(postIds []string) ([]*model.PostPrior } func (s *SqlPostPriorityStore) Save(priority *model.PostPriority) (*model.PostPriority, error) { - tx, err := s.GetMaster().Beginx() + tx, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } @@ -143,7 +143,7 @@ func (s *SqlPostPriorityStore) Save(priority *model.PostPriority) (*model.PostPr } func (s *SqlPostPriorityStore) Delete(postId string) error { - tx, err := s.GetMaster().Beginx() + tx, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } diff --git a/server/channels/store/sqlstore/post_store.go b/server/channels/store/sqlstore/post_store.go index 8c4a7bd7506..61311fdb1fa 100644 --- a/server/channels/store/sqlstore/post_store.go +++ b/server/channels/store/sqlstore/post_store.go @@ -245,7 +245,7 @@ func (s *SqlPostStore) SaveMultiple(rctx request.CTX, posts []*model.Post) ([]*m } } - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return posts, -1, errors.Wrap(err, "begin_transaction") } @@ -466,7 +466,7 @@ func (s *SqlPostStore) OverwriteMultiple(rctx request.CTX, posts []*model.Post) post.ValidateProps(rctx.Logger()) } - tx, err := s.GetMaster().Beginx() + tx, err := s.GetMaster().Begin() if err != nil { return nil, -1, errors.Wrap(err, "begin_transaction") } @@ -970,7 +970,7 @@ 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(rctx request.CTX, postID string, time int64, deleteByID string) (err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } @@ -1025,7 +1025,7 @@ func (s *SqlPostStore) PermanentDelete(rctx request.CTX, postID string) (err err } func (s *SqlPostStore) permanentDelete(postIds []string) (err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } @@ -1058,7 +1058,7 @@ func (s *SqlPostStore) permanentDelete(postIds []string) (err error) { // - Read Receipts // - Thread replies if post is a root post func (s *SqlPostStore) PermanentDeleteAssociatedData(postIds []string) error { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } @@ -1112,7 +1112,7 @@ type postIds struct { func (s *SqlPostStore) permanentDeleteAllCommentByUser(userId string) (err error) { results := []postIds{} - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } @@ -1200,7 +1200,7 @@ func (s *SqlPostStore) PermanentDeleteByUser(rctx request.CTX, userId string) er // deletes all reactions // no thread comment cleanup needed, since we are deleting threads and thread memberships func (s *SqlPostStore) PermanentDeleteByChannel(rctx request.CTX, channelId string) (err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } @@ -3196,7 +3196,7 @@ func (s *SqlPostStore) updateThreadsFromPosts(transaction *sqlxTxWrapper, posts } func (s *SqlPostStore) SetPostReminder(reminder *model.PostReminder) error { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } @@ -3304,7 +3304,7 @@ func (s *SqlPostStore) RefreshPostStats() error { // it only restores posts deleted by the specified deletedBy user ID, which in case of Content Flagging is // the Content Reviewer bot. func (s *SqlPostStore) RestoreContentFlaggedPost(flaggedPost *model.Post, statusFieldId, contentFlaggingManagedFieldId string) error { - tx, err := s.GetMaster().Beginx() + tx, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } diff --git a/server/channels/store/sqlstore/preference_store.go b/server/channels/store/sqlstore/preference_store.go index 2f0d5dbf071..e923c0175e9 100644 --- a/server/channels/store/sqlstore/preference_store.go +++ b/server/channels/store/sqlstore/preference_store.go @@ -43,7 +43,7 @@ func (s SqlPreferenceStore) deleteUnusedFeatures() { func (s SqlPreferenceStore) Save(preferences model.Preferences) (err error) { // wrap in a transaction so that if one fails, everything fails - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } diff --git a/server/channels/store/sqlstore/product_notices_store.go b/server/channels/store/sqlstore/product_notices_store.go index 80fa6617222..ae94aead339 100644 --- a/server/channels/store/sqlstore/product_notices_store.go +++ b/server/channels/store/sqlstore/product_notices_store.go @@ -60,7 +60,7 @@ func (s SqlProductNoticesStore) ClearOldNotices(currentNotices model.ProductNoti } func (s SqlProductNoticesStore) View(userId string, notices []string) (err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } diff --git a/server/channels/store/sqlstore/property_field_store.go b/server/channels/store/sqlstore/property_field_store.go index 9bd23e0125e..db8d6ad4210 100644 --- a/server/channels/store/sqlstore/property_field_store.go +++ b/server/channels/store/sqlstore/property_field_store.go @@ -231,7 +231,7 @@ func (s *SqlPropertyFieldStore) Update(groupID string, fields []*model.PropertyF return nil, nil } - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "property_field_update_begin_transaction") } diff --git a/server/channels/store/sqlstore/property_value_store.go b/server/channels/store/sqlstore/property_value_store.go index f5a790bb83e..e72d6a7161e 100644 --- a/server/channels/store/sqlstore/property_value_store.go +++ b/server/channels/store/sqlstore/property_value_store.go @@ -62,7 +62,7 @@ func (s *SqlPropertyValueStore) CreateMany(values []*model.PropertyValue) ([]*mo return nil, nil } - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "property_value_create_many_begin_transaction") } @@ -198,7 +198,7 @@ func (s *SqlPropertyValueStore) Update(groupID string, values []*model.PropertyV return nil, nil } - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "property_value_update_begin_transaction") } @@ -264,7 +264,7 @@ func (s *SqlPropertyValueStore) Upsert(values []*model.PropertyValue) (_ []*mode return nil, nil } - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "property_value_upsert_begin_transaction") } diff --git a/server/channels/store/sqlstore/reaction_store.go b/server/channels/store/sqlstore/reaction_store.go index 9b20a70ccbd..cb605832152 100644 --- a/server/channels/store/sqlstore/reaction_store.go +++ b/server/channels/store/sqlstore/reaction_store.go @@ -30,7 +30,7 @@ func (s *SqlReactionStore) Save(reaction *model.Reaction) (re *model.Reaction, e if err := reaction.IsValid(); err != nil { return nil, err } - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } @@ -68,7 +68,7 @@ func (s *SqlReactionStore) Save(reaction *model.Reaction) (re *model.Reaction, e func (s *SqlReactionStore) Delete(reaction *model.Reaction) (re *model.Reaction, err error) { reaction.PreUpdate() - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } @@ -252,7 +252,7 @@ func (s *SqlReactionStore) DeleteAllWithEmojiName(emojiName string) error { } func (s *SqlReactionStore) permanentDeleteReactions(userId string) ([]string, error) { - txn, err := s.GetMaster().Beginx() + txn, err := s.GetMaster().Begin() if err != nil { return nil, err } @@ -289,7 +289,7 @@ func (s SqlReactionStore) PermanentDeleteByUser(userId string) error { return err } - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return err } @@ -312,7 +312,7 @@ func (s SqlReactionStore) PermanentDeleteByUser(userId string) error { } func (s *SqlReactionStore) DeleteOrphanedRowsByIds(r *model.RetentionIdsForDeletion) (int64, error) { - txn, err := s.GetMaster().Beginx() + txn, err := s.GetMaster().Begin() if err != nil { return 0, err } diff --git a/server/channels/store/sqlstore/remote_cluster_store.go b/server/channels/store/sqlstore/remote_cluster_store.go index 2f51b242105..cf8bae08e63 100644 --- a/server/channels/store/sqlstore/remote_cluster_store.go +++ b/server/channels/store/sqlstore/remote_cluster_store.go @@ -121,7 +121,7 @@ func (s sqlRemoteClusterStore) Update(remoteCluster *model.RemoteCluster) (*mode } func (s sqlRemoteClusterStore) Delete(remoteId string) (bool, error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return false, errors.Wrap(err, "DeleteRemoteCluster: begin_transaction") } diff --git a/server/channels/store/sqlstore/retention_policy_store.go b/server/channels/store/sqlstore/retention_policy_store.go index c89aa3e4c25..f3af3af6173 100644 --- a/server/channels/store/sqlstore/retention_policy_store.go +++ b/server/channels/store/sqlstore/retention_policy_store.go @@ -77,7 +77,7 @@ func (s *SqlRetentionPolicyStore) Save(policy *model.RetentionPolicyWithTeamAndC return nil, err } - txn, err := s.GetMaster().Beginx() + txn, err := s.GetMaster().Begin() if err != nil { return nil, err } @@ -272,7 +272,7 @@ func (s *SqlRetentionPolicyStore) Patch(patch *model.RetentionPolicyWithTeamAndC return nil, err } - txn, err := s.GetMaster().Beginx() + txn, err := s.GetMaster().Begin() if err != nil { return nil, err } @@ -807,7 +807,7 @@ func (s *SqlRetentionPolicyStore) GetChannelPoliciesCountForUser(userID string) return count, nil } -func scanRetentionIdsForDeletion(rows *sql.Rows) ([]*model.RetentionIdsForDeletion, error) { +func scanRetentionIdsForDeletion(rows rowScanner) ([]*model.RetentionIdsForDeletion, error) { idsForDeletion := []*model.RetentionIdsForDeletion{} for rows.Next() { var row model.RetentionIdsForDeletion @@ -1014,7 +1014,7 @@ func genericRetentionPoliciesDeletion( } if r.StoreDeletedIds { - txn, err := s.GetMaster().Beginx() + txn, err := s.GetMaster().Begin() if err != nil { return 0, err } @@ -1023,7 +1023,7 @@ func genericRetentionPoliciesDeletion( primaryKeysStr := "(" + strings.Join(r.PrimaryKeys, ",") + ")" query = fmt.Sprintf("DELETE FROM %s WHERE %s IN (%s) RETURNING %s.%s", r.Table, primaryKeysStr, query, r.Table, r.PrimaryKeys[0]) - var rows *sql.Rows + var rows *sqlxRows rows, err = txn.Query(query, args...) if err != nil { return 0, errors.Wrap(err, "failed to delete "+r.Table) diff --git a/server/channels/store/sqlstore/role_store.go b/server/channels/store/sqlstore/role_store.go index 4bf1d9f1677..c188cc1c8e1 100644 --- a/server/channels/store/sqlstore/role_store.go +++ b/server/channels/store/sqlstore/role_store.go @@ -106,7 +106,7 @@ func (s *SqlRoleStore) Save(role *model.Role) (_ *model.Role, err error) { } if role.Id == "" { - transaction, terr := s.GetMaster().Beginx() + transaction, terr := s.GetMaster().Begin() if terr != nil { return nil, errors.Wrap(terr, "begin_transaction") } diff --git a/server/channels/store/sqlstore/scheme_store.go b/server/channels/store/sqlstore/scheme_store.go index c42962fd82c..8a85aaa47f6 100644 --- a/server/channels/store/sqlstore/scheme_store.go +++ b/server/channels/store/sqlstore/scheme_store.go @@ -56,7 +56,7 @@ func newSqlSchemeStore(sqlStore *SqlStore) store.SchemeStore { func (s *SqlSchemeStore) Save(scheme *model.Scheme) (_ *model.Scheme, err error) { if scheme.Id == "" { - transaction, terr := s.GetMaster().Beginx() + transaction, terr := s.GetMaster().Begin() if terr != nil { return nil, errors.Wrap(terr, "begin_transaction") } diff --git a/server/channels/store/sqlstore/shared_channel_store.go b/server/channels/store/sqlstore/shared_channel_store.go index c520f5fc77f..abfc729425e 100644 --- a/server/channels/store/sqlstore/shared_channel_store.go +++ b/server/channels/store/sqlstore/shared_channel_store.go @@ -43,7 +43,7 @@ func (s SqlSharedChannelStore) Save(sc *model.SharedChannel) (sh *model.SharedCh return nil, fmt.Errorf("invalid channel: %w", err) } - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } @@ -273,7 +273,7 @@ func (s SqlSharedChannelStore) Update(sc *model.SharedChannel) (*model.SharedCha // Returns true if shared channel found and deleted, false if not // found. func (s SqlSharedChannelStore) Delete(channelId string) (ok bool, err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return false, errors.Wrap(err, "DeleteSharedChannel: begin_transaction") } diff --git a/server/channels/store/sqlstore/sqlx_wrapper.go b/server/channels/store/sqlstore/sqlx_wrapper.go index 2fdce549169..28fc24dacf0 100644 --- a/server/channels/store/sqlstore/sqlx_wrapper.go +++ b/server/channels/store/sqlstore/sqlx_wrapper.go @@ -46,6 +46,45 @@ type Builder interface { ToSql() (string, []any, error) } +// sqlxRow wraps *sqlx.Row together with the context cancel function for the +// query that produced it. Scan calls cancel immediately after scanning so that +// the timeout context is released as soon as the row has been consumed, rather +// than waiting for the timer to fire. +type sqlxRow struct { + row *sqlx.Row + cancel context.CancelFunc +} + +func (r *sqlxRow) Scan(dest ...any) error { + defer r.cancel() + return r.row.Scan(dest...) +} + +// sqlxRows wraps *sqlx.Rows together with the context cancel function for the +// query that produced it. Close calls cancel so that timeout resources are +// released as soon as the caller is done iterating, rather than waiting for +// the timer to fire. +type sqlxRows struct { + *sqlx.Rows + cancel context.CancelFunc +} + +// Next advances the cursor and cancels the timeout context on EOF so that +// resources are released even when the caller exhausts the rows without an +// explicit Close. +func (r *sqlxRows) Next() bool { + ok := r.Rows.Next() + if !ok { + r.cancel() + } + return ok +} + +func (r *sqlxRows) Close() error { + defer r.cancel() + return r.Rows.Close() +} + // sqlxExecutor exposes sqlx operations. It is used to enable some internal store methods to // accept both transactions (*sqlxTxWrapper) and common db handlers (*sqlxDbWrapper). type sqlxExecutor interface { @@ -55,9 +94,8 @@ type sqlxExecutor interface { Exec(query string, args ...any) (sql.Result, error) ExecBuilder(builder Builder) (sql.Result, error) ExecRaw(query string, args ...any) (sql.Result, error) - NamedQuery(query string, arg any) (*sqlx.Rows, error) - QueryRowX(query string, args ...any) *sqlx.Row - QueryX(query string, args ...any) (*sqlx.Rows, error) + QueryRow(query string, args ...any) *sqlxRow + Query(query string, args ...any) (*sqlxRows, error) Select(dest any, query string, args ...any) error SelectBuilder(dest any, builder Builder) error } @@ -107,7 +145,27 @@ func (w *sqlxDBWrapper) Rebind(query string) string { return w.db.Rebind(query) } -func (w *sqlxDBWrapper) Beginx() (*sqlxTxWrapper, error) { +// noTimeoutKey is the context key that opts a context out of automatic timeout +// injection by ensureQueryTimeout. Use noTimeoutContext() to create such a context. +type noTimeoutKey struct{} + +// ensureQueryTimeout returns ctx unchanged if it already carries a deadline or +// has been explicitly marked as timeout-exempt (via noTimeoutContext), otherwise +// wraps it with the given timeout. Callers that complete synchronously should +// defer the returned cancel. Callers that return a handle to be consumed later +// (e.g. QueryRowContext) may discard it — the timer bounds any resource leak to +// at most timeout duration. +func ensureQueryTimeout(ctx context.Context, timeout time.Duration) (context.Context, context.CancelFunc) { + if _, ok := ctx.Deadline(); ok { + return ctx, func() {} + } + if ctx.Value(noTimeoutKey{}) != nil { + return ctx, func() {} + } + return context.WithTimeout(ctx, timeout) +} + +func (w *sqlxDBWrapper) Begin() (*sqlxTxWrapper, error) { tx, err := w.db.Beginx() if err != nil { return nil, w.checkErr(err) @@ -116,7 +174,7 @@ func (w *sqlxDBWrapper) Beginx() (*sqlxTxWrapper, error) { return newSqlxTxWrapper(tx, w.queryTimeout, w.trace, w), nil } -func (w *sqlxDBWrapper) BeginXWithIsolation(opts *sql.TxOptions) (*sqlxTxWrapper, error) { +func (w *sqlxDBWrapper) BeginWithIsolation(opts *sql.TxOptions) (*sqlxTxWrapper, error) { tx, err := w.db.BeginTxx(context.Background(), opts) if err != nil { return nil, w.checkErr(err) @@ -204,40 +262,38 @@ func (w *sqlxDBWrapper) ExecRaw(query string, args ...any) (sql.Result, error) { return w.checkErrWithResult(w.db.ExecContext(ctx, query, args...)) } -func (w *sqlxDBWrapper) NamedQuery(query string, arg any) (*sqlx.Rows, error) { - query = namedParamRegex.ReplaceAllStringFunc(query, strings.ToLower) - ctx, cancel := context.WithTimeout(context.Background(), w.queryTimeout) - defer cancel() +// QueryRowContext forwards to the underlying *sqlx.DB, adding the wrapper timeout +// if the caller's context carries no deadline. The cancel is released when the +// caller calls Scan on the returned *sqlxRow. +func (w *sqlxDBWrapper) QueryRowContext(ctx context.Context, query string, args ...any) *sqlxRow { + query = w.db.Rebind(query) + ctx, cancel := ensureQueryTimeout(ctx, w.queryTimeout) if w.trace { defer func(then time.Time) { - printArgs(query, time.Since(then), arg) + printArgs(query, time.Since(then), args) }(time.Now()) } - return w.checkErrWithRows(w.db.NamedQueryContext(ctx, query, arg)) + return &sqlxRow{row: w.db.QueryRowxContext(ctx, query, args...), cancel: cancel} } -// QueryRowx forwards to the underlying *sqlx.DB without adding a timeout. -func (w *sqlxDBWrapper) QueryRowx(query string, args ...any) *sqlx.Row { - return w.db.QueryRowxContext(context.Background(), query, args...) -} +func (w *sqlxDBWrapper) QueryRow(query string, args ...any) *sqlxRow { + query = w.db.Rebind(query) + ctx, cancel := context.WithTimeout(context.Background(), w.queryTimeout) -// QueryRowxContext forwards to the underlying *sqlx.DB with the caller-supplied context. -// The caller is responsible for applying an appropriate timeout. -func (w *sqlxDBWrapper) QueryRowxContext(ctx context.Context, query string, args ...any) *sqlx.Row { - return w.db.QueryRowxContext(ctx, query, args...) -} + if w.trace { + defer func(then time.Time) { + printArgs(query, time.Since(then), args) + }(time.Now()) + } -// QueryRow forwards to the underlying *sqlx.DB without adding a timeout. -func (w *sqlxDBWrapper) QueryRow(query string, args ...any) *sql.Row { - return w.db.QueryRow(query, args...) + return &sqlxRow{row: w.db.QueryRowxContext(ctx, query, args...), cancel: cancel} } -func (w *sqlxDBWrapper) QueryRowX(query string, args ...any) *sqlx.Row { +func (w *sqlxDBWrapper) Query(query string, args ...any) (*sqlxRows, error) { query = w.db.Rebind(query) ctx, cancel := context.WithTimeout(context.Background(), w.queryTimeout) - defer cancel() if w.trace { defer func(then time.Time) { @@ -245,12 +301,19 @@ func (w *sqlxDBWrapper) QueryRowX(query string, args ...any) *sqlx.Row { }(time.Now()) } - return w.db.QueryRowxContext(ctx, query, args...) + rows, err := w.db.QueryxContext(ctx, query, args...) + if err != nil { + cancel() + return nil, w.checkErr(err) + } + return &sqlxRows{Rows: rows, cancel: cancel}, nil } -func (w *sqlxDBWrapper) QueryX(query string, args ...any) (*sqlx.Rows, error) { +// ExecContext forwards to the underlying DB, adding the wrapper timeout if the +// caller's context carries no deadline. +func (w *sqlxDBWrapper) ExecContext(ctx context.Context, query string, args ...any) (sql.Result, error) { query = w.db.Rebind(query) - ctx, cancel := context.WithTimeout(context.Background(), w.queryTimeout) + ctx, cancel := ensureQueryTimeout(ctx, w.queryTimeout) defer cancel() if w.trace { @@ -259,29 +322,16 @@ func (w *sqlxDBWrapper) QueryX(query string, args ...any) (*sqlx.Rows, error) { }(time.Now()) } - return w.checkErrWithRows(w.db.QueryxContext(ctx, query, args...)) -} - -// Query forwards to the underlying *sql.DB without adding a timeout. -// Callers that need timeout enforcement should use Select or QueryX instead. -func (w *sqlxDBWrapper) Query(query string, args ...any) (*sql.Rows, error) { - rows, err := w.db.Query(query, args...) - return rows, w.checkErr(err) -} - -// ExecContext forwards to the underlying DB with the caller-supplied context. -// The caller is responsible for applying an appropriate timeout. -func (w *sqlxDBWrapper) ExecContext(ctx context.Context, query string, args ...any) (sql.Result, error) { return w.checkErrWithResult(w.db.ExecContext(ctx, query, args...)) } func (w *sqlxDBWrapper) Select(dest any, query string, args ...any) error { - return w.SelectCtx(context.Background(), dest, query, args...) + return w.SelectContext(context.Background(), dest, query, args...) } -func (w *sqlxDBWrapper) SelectCtx(ctx context.Context, dest any, query string, args ...any) error { +func (w *sqlxDBWrapper) SelectContext(ctx context.Context, dest any, query string, args ...any) error { query = w.db.Rebind(query) - ctx, cancel := context.WithTimeout(ctx, w.queryTimeout) + ctx, cancel := ensureQueryTimeout(ctx, w.queryTimeout) defer cancel() if w.trace { @@ -293,17 +343,25 @@ func (w *sqlxDBWrapper) SelectCtx(ctx context.Context, dest any, query string, a return w.checkErr(w.db.SelectContext(ctx, dest, query, args...)) } -// QueryRowContext forwards to the underlying DB with the caller-supplied context. -// The caller is responsible for applying an appropriate timeout. -func (w *sqlxDBWrapper) QueryRowContext(ctx context.Context, query string, args ...any) *sql.Row { - return w.db.QueryRowContext(ctx, query, args...) -} +// QueryContext forwards to the underlying DB, adding the wrapper timeout if the +// caller's context carries no deadline. The cancel is released when the caller +// calls Close on the returned *sqlxRows (or when Next reaches EOF). +func (w *sqlxDBWrapper) QueryContext(ctx context.Context, query string, args ...any) (*sqlxRows, error) { + query = w.db.Rebind(query) + ctx, cancel := ensureQueryTimeout(ctx, w.queryTimeout) -// QueryContext forwards to the underlying DB with the caller-supplied context. -// The caller is responsible for applying an appropriate timeout. -func (w *sqlxDBWrapper) QueryContext(ctx context.Context, query string, args ...any) (*sql.Rows, error) { - rows, err := w.db.QueryContext(ctx, query, args...) - return rows, w.checkErr(err) + if w.trace { + defer func(then time.Time) { + printArgs(query, time.Since(then), args) + }(time.Now()) + } + + rows, err := w.db.QueryxContext(ctx, query, args...) + if err != nil { + cancel() + return nil, w.checkErr(err) + } + return &sqlxRows{Rows: rows, cancel: cancel}, nil } func (w *sqlxDBWrapper) SelectBuilder(dest any, builder Builder) error { @@ -316,7 +374,7 @@ func (w *sqlxDBWrapper) SelectBuilderCtx(ctx context.Context, dest any, builder return err } - return w.SelectCtx(ctx, dest, query, args...) + return w.SelectContext(ctx, dest, query, args...) } type sqlxTxWrapper struct { @@ -422,52 +480,9 @@ func (w *sqlxTxWrapper) NamedExec(query string, arg any) (sql.Result, error) { return w.dbw.checkErrWithResult(w.tx.NamedExecContext(ctx, query, arg)) } -func (w *sqlxTxWrapper) NamedQuery(query string, arg any) (*sqlx.Rows, error) { - query = namedParamRegex.ReplaceAllStringFunc(query, strings.ToLower) - ctx, cancel := context.WithTimeout(context.Background(), w.queryTimeout) - defer cancel() - - if w.trace { - defer func(then time.Time) { - printArgs(query, time.Since(then), arg) - }(time.Now()) - } - - // There is no tx.NamedQueryContext support in the sqlx API. (https://github.com/jmoiron/sqlx/issues/447) - // So we need to implement this ourselves. - type result struct { - rows *sqlx.Rows - err error - } - - // Need to add a buffer of 1 to prevent goroutine leak. - resChan := make(chan *result, 1) - go func() { - rows, err := w.tx.NamedQuery(query, arg) - resChan <- &result{ - rows: rows, - err: err, - } - }() - - // staticcheck fails to check that res gets re-assigned later. - res := &result{} //nolint:staticcheck - select { - case res = <-resChan: - case <-ctx.Done(): - res = &result{ - rows: nil, - err: ctx.Err(), - } - } - - return res.rows, w.dbw.checkErr(res.err) -} - -func (w *sqlxTxWrapper) QueryRowX(query string, args ...any) *sqlx.Row { +func (w *sqlxTxWrapper) QueryRow(query string, args ...any) *sqlxRow { query = w.tx.Rebind(query) ctx, cancel := context.WithTimeout(context.Background(), w.queryTimeout) - defer cancel() if w.trace { defer func(then time.Time) { @@ -475,13 +490,12 @@ func (w *sqlxTxWrapper) QueryRowX(query string, args ...any) *sqlx.Row { }(time.Now()) } - return w.tx.QueryRowxContext(ctx, query, args...) + return &sqlxRow{row: w.tx.QueryRowxContext(ctx, query, args...), cancel: cancel} } -func (w *sqlxTxWrapper) QueryX(query string, args ...any) (*sqlx.Rows, error) { +func (w *sqlxTxWrapper) Query(query string, args ...any) (*sqlxRows, error) { query = w.tx.Rebind(query) ctx, cancel := context.WithTimeout(context.Background(), w.queryTimeout) - defer cancel() if w.trace { defer func(then time.Time) { @@ -489,7 +503,12 @@ func (w *sqlxTxWrapper) QueryX(query string, args ...any) (*sqlx.Rows, error) { }(time.Now()) } - return w.dbw.checkErrWithRows(w.tx.QueryxContext(ctx, query, args...)) + rows, err := w.tx.QueryxContext(ctx, query, args...) + if err != nil { + cancel() + return nil, w.dbw.checkErr(err) + } + return &sqlxRows{Rows: rows, cancel: cancel}, nil } func (w *sqlxTxWrapper) Select(dest any, query string, args ...any) error { @@ -506,13 +525,6 @@ func (w *sqlxTxWrapper) Select(dest any, query string, args ...any) error { return w.dbw.checkErr(w.tx.SelectContext(ctx, dest, query, args...)) } -// Query forwards to the underlying *sqlx.Tx without adding a timeout. -// Callers that need timeout enforcement should use Select or QueryX instead. -func (w *sqlxTxWrapper) Query(query string, args ...any) (*sql.Rows, error) { - rows, err := w.tx.Query(query, args...) - return rows, w.dbw.checkErr(err) -} - func (w *sqlxTxWrapper) SelectBuilder(dest any, builder Builder) error { query, args, err := builder.ToSql() if err != nil { @@ -546,10 +558,6 @@ func (w *sqlxDBWrapper) checkErrWithResult(res sql.Result, err error) (sql.Resul return res, w.checkErr(err) } -func (w *sqlxDBWrapper) checkErrWithRows(res *sqlx.Rows, err error) (*sqlx.Rows, error) { - return res, w.checkErr(err) -} - func (w *sqlxDBWrapper) checkErr(err error) error { var netError *net.OpError if errors.As(err, &netError) && (!netError.Temporary() && !netError.Timeout()) { diff --git a/server/channels/store/sqlstore/sqlx_wrapper_test.go b/server/channels/store/sqlstore/sqlx_wrapper_test.go index cdfec44f029..74ec277b9ca 100644 --- a/server/channels/store/sqlstore/sqlx_wrapper_test.go +++ b/server/channels/store/sqlstore/sqlx_wrapper_test.go @@ -5,10 +5,14 @@ package sqlstore import ( "context" + "database/sql/driver" + "errors" "strings" "sync" "testing" + "time" + "github.com/lib/pq" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -16,45 +20,29 @@ import ( "github.com/mattermost/mattermost/server/public/shared/mlog" ) -func TestSqlX(t *testing.T) { - t.Run("NamedQuery", func(t *testing.T) { - testDrivers := []string{ - model.DatabaseDriverPostgres, - } - - for _, driver := range testDrivers { - settings, err := makeSqlSettings(driver) - if err != nil { - continue - } - *settings.QueryTimeout = 1 - store := &SqlStore{ - rrCounter: 0, - srCounter: 0, - settings: settings, - logger: mlog.CreateConsoleTestLogger(t), - quitMonitor: make(chan struct{}), - wgMonitor: &sync.WaitGroup{}, - } - - require.NoError(t, store.initConnection()) - - defer store.Close() - - tx, err := store.GetMaster().Beginx() - require.NoError(t, err) - - query := `SELECT pg_sleep(:timeout);` - arg := struct{ Timeout int }{Timeout: 2} - rows, err := tx.NamedQuery(query, arg) - if rows != nil { - defer rows.Close() - } - require.Equal(t, context.DeadlineExceeded, err) - require.NoError(t, tx.Commit()) - } - }) +// requireQueryTimeout asserts that err represents a query-timeout cancellation. +// Three error shapes are possible depending on timing in the pq driver: +// - context.DeadlineExceeded — context fires before the query reaches the server. +// - pq.Error{Code:"57014"} — PostgreSQL cancels the in-flight query and returns +// "canceling statement due to user request" on the original connection. +// - driver.ErrBadConn — pq's watchCancel goroutine sets cn.err on the client side +// before the query loop reads the server's 57014 response; pq short-circuits and +// returns this sentinel rather than the server error. +func requireQueryTimeout(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + var pqErr *pq.Error + switch { + case errors.As(err, &pqErr): + require.Equal(t, "57014", string(pqErr.Code)) + case errors.Is(err, driver.ErrBadConn): + // client-side short-circuit; see comment above + default: + require.ErrorIs(t, err, context.DeadlineExceeded) + } +} +func TestSqlX(t *testing.T) { t.Run("NamedParse", func(t *testing.T) { queries := []struct { in string @@ -88,61 +76,200 @@ func TestSqlX(t *testing.T) { }) } -func TestSqlxSelect(t *testing.T) { - testDrivers := []string{ - model.DatabaseDriverPostgres, +func makeStoreWithTimeout(t *testing.T, driver string, queryTimeout time.Duration) *SqlStore { + t.Helper() + settings, err := makeSqlSettings(driver) + if err != nil { + t.Skip(err) } - for _, driver := range testDrivers { - t.Run(driver, func(t *testing.T) { - settings, err := makeSqlSettings(driver) - if err != nil { - t.Skip(err) - } - *settings.QueryTimeout = 1 - store := &SqlStore{ - rrCounter: 0, - srCounter: 0, - settings: settings, - logger: mlog.CreateConsoleTestLogger(t), - quitMonitor: make(chan struct{}), - wgMonitor: &sync.WaitGroup{}, - } - - require.NoError(t, store.initConnection()) - defer store.Close() - - t.Run("SelectCtx", func(t *testing.T) { - var result []string - err := store.GetMaster().SelectCtx(context.Background(), &result, "SELECT 'test' AS col") - require.NoError(t, err) - require.Equal(t, []string{"test"}, result) - - // Test timeout - ctx, cancel := context.WithTimeout(context.Background(), 1) - defer cancel() - query := "SELECT pg_sleep(2)" - err = store.GetMaster().SelectCtx(ctx, &result, query) - require.Error(t, err) - require.Equal(t, context.DeadlineExceeded, err) - }) - - t.Run("SelectBuilderCtx", func(t *testing.T) { - var result []string - builder := store.getQueryBuilder(). - Select("'test' AS col") - err := store.GetMaster().SelectBuilderCtx(context.Background(), &result, builder) - require.NoError(t, err) - require.Equal(t, []string{"test"}, result) - - // Test timeout - ctx, cancel := context.WithTimeout(context.Background(), 1) - defer cancel() - builder = store.getQueryBuilder(). - Select("pg_sleep(2)") - err = store.GetMaster().SelectBuilderCtx(ctx, &result, builder) - require.Error(t, err) - require.Equal(t, context.DeadlineExceeded, err) - }) - }) + *settings.QueryTimeout = int(queryTimeout.Seconds()) + store := &SqlStore{ + settings: settings, + logger: mlog.CreateConsoleTestLogger(t), + quitMonitor: make(chan struct{}), + wgMonitor: &sync.WaitGroup{}, } + require.NoError(t, store.initConnection()) + t.Cleanup(store.Close) + return store +} + +func TestSqlxQuery(t *testing.T) { + store := makeStoreWithTimeout(t, model.DatabaseDriverPostgres, 1*time.Second) + + t.Run("db/happy", func(t *testing.T) { + rows, err := store.GetMaster().Query("SELECT 1") + require.NoError(t, err) + defer rows.Close() + require.True(t, rows.Next()) + var v int + require.NoError(t, rows.Scan(&v)) + require.Equal(t, 1, v) + }) + + t.Run("db/timeout", func(t *testing.T) { + _, err := store.GetMaster().Query("SELECT pg_sleep(2)") + requireQueryTimeout(t, err) + }) + + t.Run("tx/happy", func(t *testing.T) { + tx, err := store.GetMaster().Begin() + require.NoError(t, err) + defer func() { assert.NoError(t, tx.Rollback()) }() + rows, err := tx.Query("SELECT 1") + require.NoError(t, err) + defer rows.Close() + require.True(t, rows.Next()) + var v int + require.NoError(t, rows.Scan(&v)) + require.Equal(t, 1, v) + }) + + t.Run("tx/timeout", func(t *testing.T) { + tx, err := store.GetMaster().Begin() + require.NoError(t, err) + _, err = tx.Query("SELECT pg_sleep(2)") + require.Error(t, tx.Rollback()) // connection is dead after timeout + requireQueryTimeout(t, err) + }) +} + +func TestSqlxQueryRow(t *testing.T) { + store := makeStoreWithTimeout(t, model.DatabaseDriverPostgres, 1*time.Second) + + t.Run("db/happy", func(t *testing.T) { + var v int + require.NoError(t, store.GetMaster().QueryRow("SELECT 1").Scan(&v)) + require.Equal(t, 1, v) + }) + + t.Run("db/timeout", func(t *testing.T) { + var v int + err := store.GetMaster().QueryRow("SELECT 1 FROM (SELECT pg_sleep(2)) s").Scan(&v) + requireQueryTimeout(t, err) + }) + + t.Run("tx/happy", func(t *testing.T) { + tx, err := store.GetMaster().Begin() + require.NoError(t, err) + defer func() { assert.NoError(t, tx.Rollback()) }() + var v int + require.NoError(t, tx.QueryRow("SELECT 1").Scan(&v)) + require.Equal(t, 1, v) + }) + + t.Run("tx/timeout", func(t *testing.T) { + tx, err := store.GetMaster().Begin() + require.NoError(t, err) + var v int + err = tx.QueryRow("SELECT 1 FROM (SELECT pg_sleep(2)) s").Scan(&v) + require.Error(t, tx.Rollback()) // connection is dead after timeout + requireQueryTimeout(t, err) + }) +} + +func TestEnsureQueryTimeout(t *testing.T) { + t.Run("no deadline adds timeout", func(t *testing.T) { + timeout := 30 * time.Second + ctx, cancel := ensureQueryTimeout(context.Background(), timeout) + defer cancel() + deadline, ok := ctx.Deadline() + require.True(t, ok) + require.WithinDuration(t, time.Now().Add(timeout), deadline, time.Second) + }) + + t.Run("existing deadline is preserved", func(t *testing.T) { + originalDeadline := time.Now().Add(5 * time.Minute) + parent, parentCancel := context.WithDeadline(context.Background(), originalDeadline) + defer parentCancel() + + ctx, cancel := ensureQueryTimeout(parent, 30*time.Second) + defer cancel() + + deadline, ok := ctx.Deadline() + require.True(t, ok) + require.Equal(t, originalDeadline, deadline) + }) + + t.Run("no-op cancel is safe to call", func(t *testing.T) { + parent, parentCancel := context.WithTimeout(context.Background(), time.Minute) + defer parentCancel() + + _, cancel := ensureQueryTimeout(parent, 30*time.Second) + require.NotPanics(t, func() { cancel() }) // calling the no-op cancel must not panic + }) + + t.Run("noTimeoutKey suppresses timeout injection", func(t *testing.T) { + ctx := context.WithValue(context.Background(), noTimeoutKey{}, true) + newCtx, cancel := ensureQueryTimeout(ctx, 30*time.Second) + defer cancel() + _, ok := newCtx.Deadline() + require.False(t, ok) + }) +} + +func TestSqlxSelectContext(t *testing.T) { + store := makeStoreWithTimeout(t, model.DatabaseDriverPostgres, 1*time.Second) + + t.Run("adds timeout when context has none", func(t *testing.T) { + // context.Background() has no deadline — wrapper adds 1s; pg_sleep(2) times out. + var result []string + err := store.GetMaster().SelectContext(context.Background(), &result, "SELECT pg_sleep(2)") + requireQueryTimeout(t, err) + }) + + t.Run("respects caller deadline shorter than wrapper timeout", func(t *testing.T) { + // 1ns context fires before the query reaches the server. + ctx, cancel := context.WithTimeout(context.Background(), 1) + defer cancel() + var result []string + err := store.GetMaster().SelectContext(ctx, &result, "SELECT pg_sleep(2)") + requireQueryTimeout(t, err) + }) + + t.Run("respects caller deadline longer than wrapper timeout", func(t *testing.T) { + // Caller supplies a 5s deadline; wrapper queryTimeout is 1s. + // With "add only if missing" the 5s deadline is used — the 2s sleep completes. + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + var result []int + err := store.GetMaster().SelectContext(ctx, &result, "SELECT 1 FROM (SELECT pg_sleep(2)) s") + require.NoError(t, err) + require.Equal(t, []int{1}, result) + }) +} + +func TestSqlxSelect(t *testing.T) { + store := makeStoreWithTimeout(t, model.DatabaseDriverPostgres, 1*time.Second) + + t.Run("SelectCtx", func(t *testing.T) { + var result []string + err := store.GetMaster().SelectContext(context.Background(), &result, "SELECT 'test' AS col") + require.NoError(t, err) + require.Equal(t, []string{"test"}, result) + + // Test timeout + ctx, cancel := context.WithTimeout(context.Background(), 1) + defer cancel() + query := "SELECT pg_sleep(2)" + err = store.GetMaster().SelectContext(ctx, &result, query) + requireQueryTimeout(t, err) + }) + + t.Run("SelectBuilderCtx", func(t *testing.T) { + var result []string + builder := store.getQueryBuilder(). + Select("'test' AS col") + err := store.GetMaster().SelectBuilderCtx(context.Background(), &result, builder) + require.NoError(t, err) + require.Equal(t, []string{"test"}, result) + + // Test timeout + ctx, cancel := context.WithTimeout(context.Background(), 1) + defer cancel() + builder = store.getQueryBuilder(). + Select("pg_sleep(2)") + err = store.GetMaster().SelectBuilderCtx(ctx, &result, builder) + requireQueryTimeout(t, err) + }) } diff --git a/server/channels/store/sqlstore/status_store.go b/server/channels/store/sqlstore/status_store.go index 06c21453130..a0f167e1f0b 100644 --- a/server/channels/store/sqlstore/status_store.go +++ b/server/channels/store/sqlstore/status_store.go @@ -80,7 +80,7 @@ func (s SqlStatusStore) SaveOrUpdateMany(statuses map[string]*model.Status) (ret statusList = append(statusList, st) } - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } diff --git a/server/channels/store/sqlstore/store.go b/server/channels/store/sqlstore/store.go index fa5da6042c6..d909da831e6 100644 --- a/server/channels/store/sqlstore/store.go +++ b/server/channels/store/sqlstore/store.go @@ -479,9 +479,11 @@ func (ss *SqlStore) analyticsContext() (context.Context, context.CancelFunc) { return context.WithTimeout(context.Background(), time.Duration(*ss.settings.AnalyticsQueryTimeout)*time.Second) } -// noTimeoutContext should only be used with queries that expect no client-side timeout. +// noTimeoutContext returns a context that suppresses automatic timeout injection +// by ensureQueryTimeout. Use only for queries that legitimately must be unbounded, +// such as schema introspection or long-running migrations. func (ss *SqlStore) noTimeoutContext() context.Context { - return context.Background() + return context.WithValue(context.Background(), noTimeoutKey{}, true) } func (ss *SqlStore) monitorReplicas() { diff --git a/server/channels/store/sqlstore/system_store.go b/server/channels/store/sqlstore/system_store.go index 45b23defc57..f3842688993 100644 --- a/server/channels/store/sqlstore/system_store.go +++ b/server/channels/store/sqlstore/system_store.go @@ -114,7 +114,7 @@ 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, err error) { - tx, err := s.GetMaster().BeginXWithIsolation(&sql.TxOptions{ + tx, err := s.GetMaster().BeginWithIsolation(&sql.TxOptions{ Isolation: sql.LevelSerializable, }) if err != nil { diff --git a/server/channels/store/sqlstore/team_store.go b/server/channels/store/sqlstore/team_store.go index 10761edb20e..4d4631d4758 100644 --- a/server/channels/store/sqlstore/team_store.go +++ b/server/channels/store/sqlstore/team_store.go @@ -835,7 +835,7 @@ func (s SqlTeamStore) SaveMultipleMembers(members []*model.TeamMember, maxUsersP } } - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } @@ -1302,7 +1302,7 @@ func (s SqlTeamStore) GetTeamsByScheme(schemeId string, offset int, limit int) ( func (s SqlTeamStore) MigrateTeamMembers(fromTeamId string, fromUserId string) (_ map[string]string, err error) { var transaction *sqlxTxWrapper - if transaction, err = s.GetMaster().Beginx(); err != nil { + if transaction, err = s.GetMaster().Begin(); err != nil { return nil, errors.Wrap(err, "begin_transaction") } defer finalizeTransactionX(transaction, &err) @@ -1398,7 +1398,7 @@ func (s SqlTeamStore) ClearAllCustomRoleAssignments() (err error) { var transaction *sqlxTxWrapper var err error - if transaction, err = s.GetMaster().Beginx(); err != nil { + if transaction, err = s.GetMaster().Begin(); err != nil { return errors.Wrap(err, "begin_transaction") } defer finalizeTransactionX(transaction, &err) diff --git a/server/channels/store/sqlstore/temporary_post_store.go b/server/channels/store/sqlstore/temporary_post_store.go index 32ef1155e9e..10358580b75 100644 --- a/server/channels/store/sqlstore/temporary_post_store.go +++ b/server/channels/store/sqlstore/temporary_post_store.go @@ -54,7 +54,7 @@ func (s *SqlTemporaryPostStore) Save(rctx request.CTX, post *model.TemporaryPost } var tx *sqlxTxWrapper - tx, err = s.GetMaster().Beginx() + tx, err = s.GetMaster().Begin() if err != nil { return nil, err } diff --git a/server/channels/store/sqlstore/thread_store.go b/server/channels/store/sqlstore/thread_store.go index f09bf8b18b1..136c02f2a08 100644 --- a/server/channels/store/sqlstore/thread_store.go +++ b/server/channels/store/sqlstore/thread_store.go @@ -836,7 +836,7 @@ func (s *SqlThreadStore) DeleteMembershipForUser(userId string, postId string) e // - channel marked unread // - user explicitly following a thread func (s *SqlThreadStore) MaintainMembership(userID, postID string, opts store.ThreadMembershipOpts) (_ *model.ThreadMembership, err error) { - trx, err := s.GetMaster().Beginx() + trx, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } @@ -855,7 +855,7 @@ func (s *SqlThreadStore) MaintainMembership(userID, postID string, opts store.Th } func (s *SqlThreadStore) MaintainMultipleFromImport(memberships []*model.ThreadMembership) (_ []*model.ThreadMembership, err error) { - trx, err := s.GetMaster().Beginx() + trx, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } @@ -1081,7 +1081,7 @@ func (s *SqlThreadStore) SaveMultipleMemberships(memberships []*model.ThreadMemb member.LastUpdated = model.GetMillis() } - tx, err := s.GetMaster().Beginx() + tx, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } diff --git a/server/channels/store/sqlstore/user_access_token_store.go b/server/channels/store/sqlstore/user_access_token_store.go index c4abb67e6e7..7a93d389ede 100644 --- a/server/channels/store/sqlstore/user_access_token_store.go +++ b/server/channels/store/sqlstore/user_access_token_store.go @@ -59,7 +59,7 @@ func (s SqlUserAccessTokenStore) Save(token *model.UserAccessToken) (*model.User } func (s SqlUserAccessTokenStore) Delete(tokenId string) (err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } @@ -95,7 +95,7 @@ func (s SqlUserAccessTokenStore) deleteTokensById(transaction *sqlxTxWrapper, to } func (s SqlUserAccessTokenStore) DeleteAllForUser(userId string) (err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } @@ -215,7 +215,7 @@ func (s SqlUserAccessTokenStore) UpdateTokenEnable(tokenId string) error { } func (s SqlUserAccessTokenStore) UpdateTokenDisable(tokenId string) (err error) { - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } diff --git a/server/channels/store/sqlstore/user_store.go b/server/channels/store/sqlstore/user_store.go index 523355432d7..7b11d1220e5 100644 --- a/server/channels/store/sqlstore/user_store.go +++ b/server/channels/store/sqlstore/user_store.go @@ -1884,7 +1884,7 @@ func (us SqlUserStore) ClearAllCustomRoleAssignments() (err error) { var transaction *sqlxTxWrapper var err error - if transaction, err = us.GetMaster().Beginx(); err != nil { + if transaction, err = us.GetMaster().Begin(); err != nil { return errors.Wrap(err, "begin_transaction") } defer finalizeTransactionX(transaction, &err) @@ -2131,7 +2131,7 @@ func applyViewRestrictionsFilter(query sq.SelectBuilder, restrictions *model.Vie } func (us SqlUserStore) PromoteGuestToUser(userId string) (err error) { - transaction, err := us.GetMaster().Beginx() + transaction, err := us.GetMaster().Begin() if err != nil { return errors.Wrap(err, "begin_transaction") } @@ -2200,7 +2200,7 @@ func (us SqlUserStore) PromoteGuestToUser(userId string) (err error) { } func (us SqlUserStore) DemoteUserToGuest(userID string) (_ *model.User, err error) { - transaction, err := us.GetMaster().Beginx() + transaction, err := us.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } diff --git a/server/channels/store/sqlstore/utils.go b/server/channels/store/sqlstore/utils.go index 4ee4a4f4a21..4bac11d2b59 100644 --- a/server/channels/store/sqlstore/utils.go +++ b/server/channels/store/sqlstore/utils.go @@ -193,8 +193,15 @@ func trimInput(input string) string { return input } +// rowScanner is the minimal interface needed to iterate over SQL result rows. +type rowScanner interface { + Next() bool + Scan(dest ...any) error + Err() error +} + // scanRowsIntoMap scans SQL rows into a map, using a provided scanner function to extract key-value pairs -func scanRowsIntoMap[K comparable, V any](rows *sql.Rows, scanner func(rows *sql.Rows) (K, V, error), defaults map[K]V) (map[K]V, error) { +func scanRowsIntoMap[K comparable, V any](rows rowScanner, scanner func(rows rowScanner) (K, V, error), defaults map[K]V) (map[K]V, error) { results := make(map[K]V, len(defaults)) // Initialize with default values if provided diff --git a/server/channels/store/sqlstore/utils_test.go b/server/channels/store/sqlstore/utils_test.go index 4188a5a8843..44e39c47860 100644 --- a/server/channels/store/sqlstore/utils_test.go +++ b/server/channels/store/sqlstore/utils_test.go @@ -4,7 +4,6 @@ package sqlstore import ( - "database/sql" "testing" "github.com/mattermost/mattermost/server/public/shared/request" @@ -214,7 +213,7 @@ func TestScanRowsIntoMap(t *testing.T) { defer rows.Close() // Create scanner function - scanner := func(rows *sql.Rows) (string, int, error) { + scanner := func(rows rowScanner) (string, int, error) { var key string var value int return key, value, rows.Scan(&key, &value) @@ -253,7 +252,7 @@ func TestScanRowsIntoMap(t *testing.T) { defer rows.Close() // Create scanner function - scanner := func(rows *sql.Rows) (string, int, error) { + scanner := func(rows rowScanner) (string, int, error) { var key string var value int return key, value, rows.Scan(&key, &value) @@ -293,7 +292,7 @@ func TestScanRowsIntoMap(t *testing.T) { defer rows.Close() // Create scanner function - scanner := func(rows *sql.Rows) (string, int, error) { + scanner := func(rows rowScanner) (string, int, error) { var key string var value int return key, value, rows.Scan(&key, &value) diff --git a/server/channels/store/sqlstore/view_store.go b/server/channels/store/sqlstore/view_store.go index d343043c86c..5906885389b 100644 --- a/server/channels/store/sqlstore/view_store.go +++ b/server/channels/store/sqlstore/view_store.go @@ -206,7 +206,7 @@ func (s *SqlViewStore) UpdateSortOrder(viewID, channelID string, newIndex int64) } now := model.GetMillis() - transaction, err := s.GetMaster().Beginx() + transaction, err := s.GetMaster().Begin() if err != nil { return nil, errors.Wrap(err, "failed to begin transaction for UpdateSortOrder") } diff --git a/server/channels/store/storetest/channel_store.go b/server/channels/store/storetest/channel_store.go index 6456504685f..a1c6e25bfe3 100644 --- a/server/channels/store/storetest/channel_store.go +++ b/server/channels/store/storetest/channel_store.go @@ -14,7 +14,6 @@ import ( "testing" "time" - "github.com/jmoiron/sqlx" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -37,9 +36,6 @@ type SqlXExecutor interface { NamedExec(query string, arg any) (sql.Result, error) Exec(query string, args ...any) (sql.Result, error) ExecRaw(query string, args ...any) (sql.Result, error) - NamedQuery(query string, arg any) (*sqlx.Rows, error) - QueryRowX(query string, args ...any) *sqlx.Row - QueryX(query string, args ...any) (*sqlx.Rows, error) Select(dest any, query string, args ...any) error } diff --git a/webapp/channels/src/components/admin_console/admin_definition.tsx b/webapp/channels/src/components/admin_console/admin_definition.tsx index 29b930deec1..a62ea3cd26c 100644 --- a/webapp/channels/src/components/admin_console/admin_definition.tsx +++ b/webapp/channels/src/components/admin_console/admin_definition.tsx @@ -219,6 +219,25 @@ const SAML_SETTINGS_CANONICAL_ALGORITHM_C14N11 = 'Canonical1.1'; // - remove_action: An store action to remove the file. // - fileType: A list of extensions separated by ",". E.g. ".jpg,.png,.gif". +const reportAProblemTypeOptions = [ + { + display_name: defineMessage({id: 'admin.support.problemType.defaultLink', defaultMessage: 'Default'}), + value: 'default', + }, + { + display_name: defineMessage({id: 'admin.support.problemType.email', defaultMessage: 'Email address'}), + value: 'email', + }, + { + display_name: defineMessage({id: 'admin.support.problemType.customLink', defaultMessage: 'Custom link'}), + value: 'link', + }, + { + display_name: defineMessage({id: 'admin.support.problemType.hide', defaultMessage: 'Hide link'}), + value: 'hidden', + }, +]; + const adminDefinitionMessages = defineMessages({ data_retention_title: {id: 'admin.data_retention.title', defaultMessage: 'Data Retention Policy'}, ip_filtering_title: {id: 'admin.sidebar.ip_filtering', defaultMessage: 'IP Filtering'}, @@ -2492,57 +2511,32 @@ const AdminDefinition: AdminDefinitionType = { type: 'dropdown', key: 'SupportSettings.ReportAProblemType', label: defineMessage({id: 'admin.support.reportAProblemTypeTitle', defaultMessage: 'Report a Problem:'}), - help_text: defineMessage({id: 'admin.support.reportAProblemTypeDescription', defaultMessage: 'Select how the ‘Report a Problem’ option behaves. Choosing ‘Custom link’ or ‘Email address’ allows you to provide a URL or address in the next field. ‘Hide link’ removes the ‘Report a Problem’ option from the app.'}), - options: [ - { - display_name: defineMessage({id: 'admin.support.problemType.defaultLink', defaultMessage: 'Default link'}), - value: 'default', - }, - { - display_name: defineMessage({id: 'admin.support.problemType.email', defaultMessage: 'Email address'}), - value: 'email', - }, - { - display_name: defineMessage({id: 'admin.support.problemType.customLink', defaultMessage: 'Custom link'}), - value: 'link', - }, - { - display_name: defineMessage({id: 'admin.support.problemType.hide', defaultMessage: 'Hide link'}), - value: 'hidden', - }, - ], - }, - { - type: 'text', - key: 'defaultLicensedReportAProblemLink', - label: defineMessage({id: 'admin.support.reportAProblemDefaultLinkTitle', defaultMessage: 'Default Report a Problem Link:'}), - help_text: defineMessage({id: 'admin.support.reportAProblemDefaultLinkDescription', defaultMessage: 'Users will be directed to this link when they choose ‘Report a Problem’.'}), - default: 'https://mattermost.com/pl/report_a_problem_licensed', - isDisabled: it.all(), + help_text: defineMessage({id: 'admin.support.reportAProblemTypeDescriptionLicensed', defaultMessage: 'By default, selecting "Report a Problem" from the help menu opens a pre-filled email draft to the Mattermost technical support team. You may provide a custom URL or email address for end user support by choosing "Custom link" or "Email address". "Hide link" removes the "Report a Problem" option from the app.'}), + isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.SITE.CUSTOMIZATION)), isHidden: it.any( + it.isFreeEdition, it.configIsTrue('ExperimentalSettings', 'RestrictSystemAdmin'), - it.not(it.stateMatches('SupportSettings.ReportAProblemType', /default/)), - it.not(it.licensed), ), + options: reportAProblemTypeOptions, }, { - type: 'text', - key: 'defaultUnlicensedReportAProblemLink', - label: defineMessage({id: 'admin.support.reportAProblemDefaultLinkTitle', defaultMessage: 'Default Report a Problem Link:'}), - help_text: defineMessage({id: 'admin.support.reportAProblemDefaultLinkDescription', defaultMessage: 'Users will be directed to this link when they choose ‘Report a Problem’.'}), - default: 'https://mattermost.com/pl/report_a_problem_unlicensed', - isDisabled: it.all(), + type: 'dropdown', + key: 'SupportSettings.ReportAProblemType', + label: defineMessage({id: 'admin.support.reportAProblemTypeTitle', defaultMessage: 'Report a Problem:'}), + help_text: defineMessage({id: 'admin.support.reportAProblemTypeDescriptionUnlicensed', defaultMessage: 'By default, selecting "Report a Problem" from the help menu opens the [Mattermost troubleshooting forums](https://mattermost.com/pl/report_a_problem_unlicensed). You may provide a custom URL or email address for end user support by choosing "Custom link" or "Email address". "Hide link" removes the "Report a Problem" option from the app.'}), + help_text_markdown: true, + isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.SITE.CUSTOMIZATION)), isHidden: it.any( + it.not(it.isFreeEdition), it.configIsTrue('ExperimentalSettings', 'RestrictSystemAdmin'), - it.not(it.stateMatches('SupportSettings.ReportAProblemType', /default/)), - it.licensed, ), + options: reportAProblemTypeOptions, }, { type: 'text', key: 'SupportSettings.ReportAProblemLink', label: defineMessage({id: 'admin.support.reportAProblemLinkTitle', defaultMessage: 'Custom Report a Problem Link:'}), - help_text: defineMessage({id: 'admin.support.reportAProblemLinkDescription', defaultMessage: 'Enter the URL that users will be directed to when they choose ‘Report a Problem’.'}), + help_text: defineMessage({id: 'admin.support.reportAProblemLinkDescription', defaultMessage: 'Enter the URL that users will be directed to when they choose "Report a Problem".'}), isDisabled: it.any( it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.SITE.CUSTOMIZATION)), ), @@ -2561,7 +2555,7 @@ const AdminDefinition: AdminDefinitionType = { type: 'text', key: 'SupportSettings.ReportAProblemMail', label: defineMessage({id: 'admin.support.reportAProblemEmailTitle', defaultMessage: 'Report a Problem Email Address:'}), - help_text: defineMessage({id: 'admin.support.reportAProblemEmailDescription', defaultMessage: 'Enter the email address that users will be prompted to send a message to when they choose ‘Report a Problem’.'}), + help_text: defineMessage({id: 'admin.support.reportAProblemEmailDescription', defaultMessage: 'Enter the email address that users will be prompted to send a message to when they choose "Report a Problem".'}), isDisabled: (it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.SITE.CUSTOMIZATION))), isHidden: it.any( it.configIsTrue('ExperimentalSettings', 'RestrictSystemAdmin'), @@ -2578,7 +2572,7 @@ const AdminDefinition: AdminDefinitionType = { type: 'bool', key: 'SupportSettings.AllowDownloadLogs', label: defineMessage({id: 'admin.support.problemAllowDownloadTitle', defaultMessage: 'Allow Mobile App Log Downloads:'}), - help_text: defineMessage({id: 'admin.support.problemAllowDownloadDescription', defaultMessage: 'When enabled, users can download app logs for troubleshooting. If a ‘Report a Problem’ link is shown, logs can be downloaded as part of that flow; if the ‘Report a Problem’ link is hidden, logs remain accessible as a separate option.'}), + help_text: defineMessage({id: 'admin.support.problemAllowDownloadDescription', defaultMessage: 'When enabled, users can download app logs for troubleshooting. If a "Report a Problem" link is shown, logs can be downloaded as part of that flow; if the "Report a Problem" link is hidden, logs remain accessible as a separate option.'}), isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.SITE.CUSTOMIZATION)), }, { diff --git a/webapp/channels/src/components/admin_console/admin_definition_helpers.tsx b/webapp/channels/src/components/admin_console/admin_definition_helpers.tsx index 9617bcbc180..e4c15053bd5 100644 --- a/webapp/channels/src/components/admin_console/admin_definition_helpers.tsx +++ b/webapp/channels/src/components/admin_console/admin_definition_helpers.tsx @@ -48,6 +48,7 @@ export const it = { configContains: (group: keyof Partial, setting: string, word: string) => (config: Partial) => Boolean((config[group] as any)?.[setting]?.includes(word)), enterpriseReady: (config: Partial, state: any, license?: ClientLicense, enterpriseReady?: boolean) => Boolean(enterpriseReady), licensed: (config: Partial, state: any, license?: ClientLicense) => license?.IsLicensed === 'true', + isFreeEdition: (config: Partial, state: any, license?: ClientLicense) => license?.IsLicensed !== 'true' || license?.SkuShortName === LicenseSkus.Entry, cloudLicensed: (config: Partial, state: any, license?: ClientLicense) => Boolean(license?.IsLicensed && isCloudLicense(license)), licensedForFeature: (feature: string) => (config: Partial, state: any, license?: ClientLicense) => Boolean(license?.IsLicensed && license[feature] === 'true'), licensedForSku: (skuName: string) => (config: Partial, state: any, license?: ClientLicense) => Boolean(license?.IsLicensed && license.SkuShortName === skuName), diff --git a/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu.tsx b/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu.tsx index ddda1781a4b..a01733fee7c 100644 --- a/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu.tsx +++ b/webapp/channels/src/components/global_header/left_controls/product_menu/product_menu.tsx @@ -10,7 +10,7 @@ import { ProductsIcon, } from '@mattermost/compass-icons/components'; -import {getLicense} from 'mattermost-redux/selectors/entities/general'; +import {isFreeEdition as isFreeEditionSelector} from 'mattermost-redux/selectors/entities/general'; import {setProductMenuSwitcherOpen} from 'actions/views/product_menu'; import {isSwitcherOpen} from 'selectors/views/product_menu'; @@ -24,7 +24,6 @@ import { import Menu from 'components/widgets/menu/menu'; import MenuWrapper from 'components/widgets/menu/menu_wrapper'; -import {LicenseSkus} from 'utils/constants'; import {useCurrentProductId, useProducts, isChannels} from 'utils/products'; import ProductBranding from './product_branding'; @@ -77,7 +76,7 @@ const ProductMenu = (): JSX.Element => { const switcherOpen = useSelector(isSwitcherOpen); const menuRef = useRef(null); const currentProductID = useCurrentProductId(); - const license = useSelector(getLicense); + const isFreeEdition = useSelector(isFreeEditionSelector); const handleClick = () => dispatch(setProductMenuSwitcherOpen(!switcherOpen)); @@ -114,8 +113,6 @@ const ProductMenu = (): JSX.Element => { ); }); - const isFreeEdition = license.IsLicensed === 'false' || license.SkuShortName === LicenseSkus.Entry; - return (