From bb78de5b7f83edac38528a2fcfbedcf92f263d3d Mon Sep 17 00:00:00 2001 From: Domenico Rizzo Date: Fri, 2 Aug 2024 17:05:21 +0200 Subject: [PATCH] [MM-50087] Refactored channel saving logic in case of duplicate (#27542) * Refactored channel saving logic The code for saving channels has been refactored handling different database drivers. The query string is now dynamically generated based on the driver in use. Additionally, error handling has been improved to account for cases where no rows are affected by the insert operation. Because a conflict should be detected due to the complex flow under the call of the function. * Refactor variable name and error handling in channel store The variable 'q' has been renamed to 'insert' for better readability. Error handling after executing the insert statement has been improved by checking for errors immediately after execution, rather than waiting until rows affected are checked. This provides a more immediate response to potential issues during the insert operation. --------- Co-authored-by: Mattermost Build --- .../channels/store/sqlstore/channel_store.go | 39 ++++++++++++++----- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/server/channels/store/sqlstore/channel_store.go b/server/channels/store/sqlstore/channel_store.go index 7d53ef21cfc..fc87d3005b8 100644 --- a/server/channels/store/sqlstore/channel_store.go +++ b/server/channels/store/sqlstore/channel_store.go @@ -672,19 +672,40 @@ func (s SqlChannelStore) saveChannelT(transaction *sqlxTxWrapper, channel *model } } - if _, err := transaction.NamedExec(`INSERT INTO Channels + var insert string + if s.DriverName() == model.DatabaseDriverMysql { + insert = `INSERT IGNORE INTO Channels (Id, CreateAt, UpdateAt, DeleteAt, TeamId, Type, DisplayName, Name, Header, Purpose, LastPostAt, TotalMsgCount, ExtraUpdateAt, CreatorId, SchemeId, GroupConstrained, Shared, TotalMsgCountRoot, LastRootPostAt) VALUES - (:Id, :CreateAt, :UpdateAt, :DeleteAt, :TeamId, :Type, :DisplayName, :Name, :Header, :Purpose, :LastPostAt, :TotalMsgCount, :ExtraUpdateAt, :CreatorId, :SchemeId, :GroupConstrained, :Shared, :TotalMsgCountRoot, :LastRootPostAt)`, channel); err != nil { - if IsUniqueConstraintError(err, []string{"Name", "channels_name_teamid_key"}) { - dupChannel := model.Channel{} - if serr := s.GetMasterX().Get(&dupChannel, "SELECT * FROM Channels WHERE TeamId = ? AND Name = ?", channel.TeamId, channel.Name); serr != nil { - return nil, errors.Wrapf(serr, "error while retrieving existing channel %s", channel.Name) // do not return this as a *store.ErrConflict as it would be treated as a recoverable error - } - return &dupChannel, store.NewErrConflict("Channel", err, "id="+channel.Id) - } + (:Id, :CreateAt, :UpdateAt, :DeleteAt, :TeamId, :Type, :DisplayName, :Name, :Header, :Purpose, :LastPostAt, :TotalMsgCount, :ExtraUpdateAt, :CreatorId, :SchemeId, :GroupConstrained, :Shared, :TotalMsgCountRoot, :LastRootPostAt)` + } else { + insert = `INSERT INTO Channels + (Id, CreateAt, UpdateAt, DeleteAt, TeamId, Type, DisplayName, Name, Header, Purpose, LastPostAt, TotalMsgCount, ExtraUpdateAt, CreatorId, SchemeId, GroupConstrained, Shared, TotalMsgCountRoot, LastRootPostAt) + VALUES + (:Id, :CreateAt, :UpdateAt, :DeleteAt, :TeamId, :Type, :DisplayName, :Name, :Header, :Purpose, :LastPostAt, :TotalMsgCount, :ExtraUpdateAt, :CreatorId, :SchemeId, :GroupConstrained, :Shared, :TotalMsgCountRoot, :LastRootPostAt) + ON CONFLICT (TeamId, Name) DO NOTHING` + } + + insertResult, err := transaction.NamedExec(insert, channel) + + if err != nil { return nil, errors.Wrapf(err, "save_channel: id=%s", channel.Id) } + + rowAffected, err := insertResult.RowsAffected() + + if err != nil { + return nil, errors.Wrapf(err, "save_channel: id=%s", channel.Id) + } + + if rowAffected == 0 { + dupChannel := model.Channel{} + if serr := s.GetMasterX().Get(&dupChannel, "SELECT * FROM Channels WHERE TeamId = ? AND Name = ?", channel.TeamId, channel.Name); serr != nil { + return nil, errors.Wrapf(serr, "error while retrieving existing channel %s", channel.Name) // do not return this as a *store.ErrConflict as it would be treated as a recoverable error + } + return &dupChannel, store.NewErrConflict("Channel", err, "id="+channel.Id) + } + return channel, nil }