[MM-15356] Migrate "Channel.Update" to Sync by default (#10815)

This commit is contained in:
Shota Gvinepadze
2019-05-15 01:02:04 +04:00
committed by Hanzei
parent 3bb11d747d
commit b561d7900c
9 changed files with 66 additions and 69 deletions

View File

@@ -410,9 +410,8 @@ func TestGetChannelsForScheme(t *testing.T) {
assert.Zero(t, len(l2))
channel1.SchemeId = &scheme1.Id
result2 := <-th.App.Srv.Store.Channel().Update(channel1)
assert.Nil(t, result2.Err)
channel1 = result2.Data.(*model.Channel)
channel1, err := th.App.Srv.Store.Channel().Update(channel1)
assert.Nil(t, err)
l3, r3 := th.SystemAdminClient.GetChannelsForScheme(scheme1.Id, 0, 100)
CheckNoError(t, r3)

View File

@@ -496,9 +496,9 @@ func (a *App) GetGroupChannel(userIds []string) (*model.Channel, *model.AppError
}
func (a *App) UpdateChannel(channel *model.Channel) (*model.Channel, *model.AppError) {
result := <-a.Srv.Store.Channel().Update(channel)
if result.Err != nil {
return nil, result.Err
_, err := a.Srv.Store.Channel().Update(channel)
if err != nil {
return nil, err
}
a.InvalidateCacheForChannel(channel)
@@ -1957,8 +1957,8 @@ func (a *App) MoveChannel(team *model.Team, channel *model.Channel, user *model.
}
channel.TeamId = team.Id
if result := <-a.Srv.Store.Channel().Update(channel); result.Err != nil {
return result.Err
if _, err := a.Srv.Store.Channel().Update(channel); err != nil {
return err
}
a.postChannelMoveMessage(user, channel, previousTeam)

View File

@@ -1136,8 +1136,8 @@ func (a *App) ImportDirectChannel(data *DirectChannelImportData, dryRun bool) *m
if data.Header != nil {
channel.Header = *data.Header
if result := <-a.Srv.Store.Channel().Update(channel); result.Err != nil {
return model.NewAppError("BulkImport", "app.import.import_direct_channel.update_header_failed.error", nil, result.Err.Error(), http.StatusBadRequest)
if _, apperr := a.Srv.Store.Channel().Update(channel); apperr != nil {
return model.NewAppError("BulkImport", "app.import.import_direct_channel.update_header_failed.error", nil, apperr.Error(), http.StatusBadRequest)
}
}

View File

@@ -614,46 +614,38 @@ func (s SqlChannelStore) saveChannelT(transaction *gorp.Transaction, channel *mo
}
// Update writes the updated channel to the database.
func (s SqlChannelStore) Update(channel *model.Channel) store.StoreChannel {
return store.Do(func(result *store.StoreResult) {
transaction, err := s.GetMaster().Begin()
if err != nil {
result.Err = model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError)
return
}
defer finalizeTransaction(transaction)
func (s SqlChannelStore) Update(channel *model.Channel) (*model.Channel, *model.AppError) {
transaction, err := s.GetMaster().Begin()
if err != nil {
return nil, model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError)
}
defer finalizeTransaction(transaction)
*result = s.updateChannelT(transaction, channel)
if result.Err != nil {
return
}
updatedChannel, apperr := s.updateChannelT(transaction, channel)
if apperr != nil {
return nil, apperr
}
// Additionally propagate the write to the PublicChannels table.
if err := s.upsertPublicChannelT(transaction, result.Data.(*model.Channel)); err != nil {
result.Err = model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.upsert_public_channel.app_error", nil, err.Error(), http.StatusInternalServerError)
return
}
if err := transaction.Commit(); err != nil {
result.Err = model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError)
return
}
})
// Additionally propagate the write to the PublicChannels table.
if err := s.upsertPublicChannelT(transaction, updatedChannel); err != nil {
return nil, model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.upsert_public_channel.app_error", nil, err.Error(), http.StatusInternalServerError)
}
if err := transaction.Commit(); err != nil {
return nil, model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError)
}
return updatedChannel, nil
}
func (s SqlChannelStore) updateChannelT(transaction *gorp.Transaction, channel *model.Channel) store.StoreResult {
result := store.StoreResult{}
func (s SqlChannelStore) updateChannelT(transaction *gorp.Transaction, channel *model.Channel) (*model.Channel, *model.AppError) {
channel.PreUpdate()
if channel.DeleteAt != 0 {
result.Err = model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.archived_channel.app_error", nil, "", http.StatusBadRequest)
return result
return nil, model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.archived_channel.app_error", nil, "", http.StatusBadRequest)
}
if result.Err = channel.IsValid(); result.Err != nil {
return result
if err := channel.IsValid(); err != nil {
return nil, err
}
count, err := transaction.Update(channel)
@@ -662,24 +654,18 @@ func (s SqlChannelStore) updateChannelT(transaction *gorp.Transaction, channel *
dupChannel := model.Channel{}
s.GetReplica().SelectOne(&dupChannel, "SELECT * FROM Channels WHERE TeamId = :TeamId AND Name= :Name AND DeleteAt > 0", map[string]interface{}{"TeamId": channel.TeamId, "Name": channel.Name})
if dupChannel.DeleteAt > 0 {
result.Err = model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.previously.app_error", nil, "id="+channel.Id+", "+err.Error(), http.StatusBadRequest)
return result
return nil, model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.previously.app_error", nil, "id="+channel.Id+", "+err.Error(), http.StatusBadRequest)
}
result.Err = model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.exists.app_error", nil, "id="+channel.Id+", "+err.Error(), http.StatusBadRequest)
return result
return nil, model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.exists.app_error", nil, "id="+channel.Id+", "+err.Error(), http.StatusBadRequest)
}
result.Err = model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.updating.app_error", nil, "id="+channel.Id+", "+err.Error(), http.StatusInternalServerError)
return result
return nil, model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.updating.app_error", nil, "id="+channel.Id+", "+err.Error(), http.StatusInternalServerError)
}
if count != 1 {
result.Err = model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.app_error", nil, "id="+channel.Id, http.StatusInternalServerError)
return result
return nil, model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.app_error", nil, "id="+channel.Id, http.StatusInternalServerError)
}
result.Data = channel
return result
return channel, nil
}
func (s SqlChannelStore) GetChannelUnread(channelId, userId string) (*model.ChannelUnread, *model.AppError) {

View File

@@ -132,7 +132,7 @@ type ChannelStore interface {
Save(channel *model.Channel, maxChannelsPerTeam int64) StoreChannel
CreateDirectChannel(userId string, otherUserId string) StoreChannel
SaveDirectChannel(channel *model.Channel, member1 *model.ChannelMember, member2 *model.ChannelMember) StoreChannel
Update(channel *model.Channel) StoreChannel
Update(channel *model.Channel) (*model.Channel, *model.AppError)
Get(id string, allowFromCache bool) (*model.Channel, *model.AppError)
InvalidateChannel(id string)
InvalidateChannelByName(teamId, name string)

View File

@@ -250,28 +250,28 @@ func testChannelStoreUpdate(t *testing.T, ss store.Store) {
time.Sleep(100 * time.Millisecond)
if err := (<-ss.Channel().Update(&o1)).Err; err != nil {
if _, err := ss.Channel().Update(&o1); err != nil {
t.Fatal(err)
}
o1.DeleteAt = 100
if err := (<-ss.Channel().Update(&o1)).Err; err == nil {
if _, err := ss.Channel().Update(&o1); err == nil {
t.Fatal("Update should have failed because channel is archived")
}
o1.DeleteAt = 0
o1.Id = "missing"
if err := (<-ss.Channel().Update(&o1)).Err; err == nil {
if _, err := ss.Channel().Update(&o1); err == nil {
t.Fatal("Update should have failed because of missing key")
}
o1.Id = model.NewId()
if err := (<-ss.Channel().Update(&o1)).Err; err == nil {
if _, err := ss.Channel().Update(&o1); err == nil {
t.Fatal("Update should have faile because id change")
}
o2.Name = o1.Name
if err := (<-ss.Channel().Update(&o2)).Err; err == nil {
if _, err := ss.Channel().Update(&o2); err == nil {
t.Fatal("Update should have failed because of existing name")
}
}
@@ -2993,7 +2993,8 @@ func testMaterializedPublicChannels(t *testing.T, ss store.Store, s SqlSupplier)
})
o2.Type = model.CHANNEL_PRIVATE
require.Nil(t, (<-ss.Channel().Update(&o2)).Err)
_, apperr := ss.Channel().Update(&o2)
require.Nil(t, apperr)
t.Run("o2 no longer listed since now private", func(t *testing.T) {
result := <-ss.Channel().SearchInTeam(teamId, "", true)
@@ -3002,7 +3003,8 @@ func testMaterializedPublicChannels(t *testing.T, ss store.Store, s SqlSupplier)
})
o2.Type = model.CHANNEL_OPEN
require.Nil(t, (<-ss.Channel().Update(&o2)).Err)
_, apperr = ss.Channel().Update(&o2)
require.Nil(t, apperr)
t.Run("o2 listed once again since now public", func(t *testing.T) {
result := <-ss.Channel().SearchInTeam(teamId, "", true)
@@ -3087,7 +3089,8 @@ func testMaterializedPublicChannels(t *testing.T, ss store.Store, s SqlSupplier)
require.Nil(t, err)
o4.DisplayName += " - Modified"
require.Nil(t, (<-ss.Channel().Update(&o4)).Err)
_, apperr = ss.Channel().Update(&o4)
require.Nil(t, apperr)
t.Run("verify o4 UPDATE converted to INSERT", func(t *testing.T) {
result := <-ss.Channel().SearchInTeam(teamId, "", true)

View File

@@ -1177,8 +1177,8 @@ func testPendingAutoAddChannelMembers(t *testing.T, ss store.Store) {
// reset state of channel and verify
channel.DeleteAt = 0
res = <-ss.Channel().Update(channel)
require.Nil(t, res.Err)
_, err := ss.Channel().Update(channel)
require.Nil(t, err)
res = <-ss.Group().ChannelMembersToAdd(0)
require.Nil(t, res.Err)
require.Len(t, res.Data, 1)

View File

@@ -1050,19 +1050,28 @@ func (_m *ChannelStore) SetDeleteAt(channelId string, deleteAt int64, updateAt i
}
// Update provides a mock function with given fields: channel
func (_m *ChannelStore) Update(channel *model.Channel) store.StoreChannel {
func (_m *ChannelStore) Update(channel *model.Channel) (*model.Channel, *model.AppError) {
ret := _m.Called(channel)
var r0 store.StoreChannel
if rf, ok := ret.Get(0).(func(*model.Channel) store.StoreChannel); ok {
var r0 *model.Channel
if rf, ok := ret.Get(0).(func(*model.Channel) *model.Channel); ok {
r0 = rf(channel)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(store.StoreChannel)
r0 = ret.Get(0).(*model.Channel)
}
}
return r0
var r1 *model.AppError
if rf, ok := ret.Get(1).(func(*model.Channel) *model.AppError); ok {
r1 = rf(channel)
} else {
if ret.Get(1) != nil {
r1 = ret.Get(1).(*model.AppError)
}
}
return r0, r1
}
// UpdateLastViewedAt provides a mock function with given fields: channelIds, userId

View File

@@ -3654,8 +3654,8 @@ func testUserStoreGetChannelGroupUsers(t *testing.T, ss store.Store) {
// update team to be group-constrained
channel.GroupConstrained = model.NewBool(true)
res = <-ss.Channel().Update(channel)
require.Nil(t, res.Err)
_, err := ss.Channel().Update(channel)
require.Nil(t, err)
// still returns user (being group-constrained has no effect)
requireNUsers(1)