[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 <build@mattermost.com>
This commit is contained in:
Domenico Rizzo 2024-08-02 17:05:21 +02:00 committed by GitHub
parent f7ef0ffbc5
commit bb78de5b7f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -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
}