diff --git a/api4/scheme_test.go b/api4/scheme_test.go index 8d23dd6b9a..0aef98673a 100644 --- a/api4/scheme_test.go +++ b/api4/scheme_test.go @@ -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) diff --git a/app/channel.go b/app/channel.go index 6b27b20ab7..3f439a649f 100644 --- a/app/channel.go +++ b/app/channel.go @@ -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) diff --git a/app/import_functions.go b/app/import_functions.go index cecdf67e33..2eb066b905 100644 --- a/app/import_functions.go +++ b/app/import_functions.go @@ -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) } } diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 94f32e8839..54f964d172 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -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) { diff --git a/store/store.go b/store/store.go index b7148ee8ff..de7e9416dd 100644 --- a/store/store.go +++ b/store/store.go @@ -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) diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index c114d62e87..60a9c1d49b 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -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) diff --git a/store/storetest/group_supplier.go b/store/storetest/group_supplier.go index 52e3af3c0d..537c444b10 100644 --- a/store/storetest/group_supplier.go +++ b/store/storetest/group_supplier.go @@ -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) diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index 826ad813ad..3819fb5465 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -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 diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index 6047fa88bd..ddc678f6d0 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -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)