From 5c32f924130d9ad7e5f01885f70dd1de065d5a9c Mon Sep 17 00:00:00 2001 From: piperRyan <35411044+piperRyan@users.noreply.github.com> Date: Wed, 19 Jun 2019 02:27:09 -0600 Subject: [PATCH] [MM-16330] Migrate "Channel.GetForPost" to Sync by default (#11199) --- app/authorization.go | 6 ++--- app/integration_action.go | 7 +++++- app/post.go | 14 ++++++------ store/sqlstore/channel_store.go | 33 ++++++++++++--------------- store/store.go | 2 +- store/storetest/channel_store.go | 6 ++--- store/storetest/mocks/ChannelStore.go | 19 +++++++++++---- 7 files changed, 48 insertions(+), 39 deletions(-) diff --git a/app/authorization.go b/app/authorization.go index 3ed3e5cbf7..49372a5a3f 100644 --- a/app/authorization.go +++ b/app/authorization.go @@ -75,8 +75,7 @@ func (a *App) SessionHasPermissionToChannelByPost(session model.Session, postId } } - if result := <-a.Srv.Store.Channel().GetForPost(postId); result.Err == nil { - channel := result.Data.(*model.Channel) + if channel, err := a.Srv.Store.Channel().GetForPost(postId); err == nil { if channel.TeamId != "" { return a.SessionHasPermissionToTeam(session, channel.TeamId, permission) } @@ -175,8 +174,7 @@ func (a *App) HasPermissionToChannelByPost(askingUserId string, postId string, p } } - if result := <-a.Srv.Store.Channel().GetForPost(postId); result.Err == nil { - channel := result.Data.(*model.Channel) + if channel, err := a.Srv.Store.Channel().GetForPost(postId); err == nil { return a.HasPermissionToTeam(askingUserId, channel.TeamId, permission) } diff --git a/app/integration_action.go b/app/integration_action.go index 0706b789c5..ad5473ef8a 100644 --- a/app/integration_action.go +++ b/app/integration_action.go @@ -69,7 +69,12 @@ func (a *App) DoPostActionWithCookie(postId, actionId, userId, selectedOption st close(pchan) }() - cchan := a.Srv.Store.Channel().GetForPost(postId) + cchan := make(chan store.StoreResult, 1) + go func() { + channel, err := a.Srv.Store.Channel().GetForPost(postId) + cchan <- store.StoreResult{Data: channel, Err: err} + close(cchan) + }() result := <-pchan if result.Err != nil { diff --git a/app/post.go b/app/post.go index 90f851c7fb..2afc2472d0 100644 --- a/app/post.go +++ b/app/post.go @@ -341,11 +341,11 @@ func (a *App) FillInPostProps(post *model.Post, channel *model.Channel) *model.A if len(channelMentions) > 0 { if channel == nil { - result := <-a.Srv.Store.Channel().GetForPost(post.Id) - if result.Err != nil { - return model.NewAppError("FillInPostProps", "api.context.invalid_param.app_error", map[string]interface{}{"Name": "post.channel_id"}, result.Err.Error(), http.StatusBadRequest) + postChannel, err := a.Srv.Store.Channel().GetForPost(post.Id) + if err != nil { + return model.NewAppError("FillInPostProps", "api.context.invalid_param.app_error", map[string]interface{}{"Name": "post.channel_id"}, err.Error(), http.StatusBadRequest) } - channel = result.Data.(*model.Channel) + channel = postChannel } mentionedChannels, err := a.GetChannelsByNames(channelMentions, channel.TeamId) @@ -552,12 +552,12 @@ func (a *App) UpdatePost(post *model.Post, safeUpdate bool) (*model.Post, *model if a.IsESIndexingEnabled() { a.Srv.Go(func() { - rchannel := <-a.Srv.Store.Channel().GetForPost(rpost.Id) - if rchannel.Err != nil { + channel, chanErr := a.Srv.Store.Channel().GetForPost(rpost.Id) + if chanErr != nil { mlog.Error(fmt.Sprintf("Couldn't get channel %v for post %v for Elasticsearch indexing.", rpost.ChannelId, rpost.Id)) return } - if err := a.Elasticsearch.IndexPost(rpost, rchannel.Data.(*model.Channel).TeamId); err != nil { + if err := a.Elasticsearch.IndexPost(rpost, channel.TeamId); err != nil { mlog.Error("Encountered error indexing post", mlog.String("post_id", post.Id), mlog.Err(err)) } }) diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 40b9d29289..4a29dd945d 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -1857,25 +1857,22 @@ func (s SqlChannelStore) GetChannelsByIds(channelIds []string) ([]*model.Channel return channels, nil } -func (s SqlChannelStore) GetForPost(postId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - channel := &model.Channel{} - if err := s.GetReplica().SelectOne( - channel, - `SELECT - Channels.* - FROM - Channels, - Posts - WHERE - Channels.Id = Posts.ChannelId - AND Posts.Id = :PostId`, map[string]interface{}{"PostId": postId}); err != nil { - result.Err = model.NewAppError("SqlChannelStore.GetForPost", "store.sql_channel.get_for_post.app_error", nil, "postId="+postId+", err="+err.Error(), http.StatusInternalServerError) - return - } +func (s SqlChannelStore) GetForPost(postId string) (*model.Channel, *model.AppError) { + channel := &model.Channel{} + if err := s.GetReplica().SelectOne( + channel, + `SELECT + Channels.* + FROM + Channels, + Posts + WHERE + Channels.Id = Posts.ChannelId + AND Posts.Id = :PostId`, map[string]interface{}{"PostId": postId}); err != nil { + return nil, model.NewAppError("SqlChannelStore.GetForPost", "store.sql_channel.get_for_post.app_error", nil, "postId="+postId+", err="+err.Error(), http.StatusInternalServerError) - result.Data = channel - }) + } + return channel, nil } func (s SqlChannelStore) AnalyticsTypeCount(teamId string, channelType string) (int64, *model.AppError) { diff --git a/store/store.go b/store/store.go index cea0c088cb..a03bc6128d 100644 --- a/store/store.go +++ b/store/store.go @@ -156,7 +156,7 @@ type ChannelStore interface { GetTeamChannels(teamId string) (*model.ChannelList, *model.AppError) GetAll(teamId string) StoreChannel GetChannelsByIds(channelIds []string) ([]*model.Channel, *model.AppError) - GetForPost(postId string) StoreChannel + GetForPost(postId string) (*model.Channel, *model.AppError) SaveMember(member *model.ChannelMember) StoreChannel UpdateMember(member *model.ChannelMember) StoreChannel GetMembers(channelId string, offset, limit int) StoreChannel diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index c99fa27a97..fe8b77b7d3 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -528,9 +528,9 @@ func testChannelStoreGetForPost(t *testing.T, ss store.Store) { }) require.Nil(t, err) - if r1 := <-ss.Channel().GetForPost(p1.Id); r1.Err != nil { - t.Fatal(r1.Err) - } else if r1.Data.(*model.Channel).Id != o1.Id { + if channel, chanErr := ss.Channel().GetForPost(p1.Id); chanErr != nil { + t.Fatal(chanErr) + } else if channel.Id != o1.Id { t.Fatal("incorrect channel returned") } } diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index de9424f61f..8fd18117ab 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -566,19 +566,28 @@ func (_m *ChannelStore) GetDeletedByName(team_id string, name string) store.Stor } // GetForPost provides a mock function with given fields: postId -func (_m *ChannelStore) GetForPost(postId string) store.StoreChannel { +func (_m *ChannelStore) GetForPost(postId string) (*model.Channel, *model.AppError) { ret := _m.Called(postId) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 *model.Channel + if rf, ok := ret.Get(0).(func(string) *model.Channel); ok { r0 = rf(postId) } 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(string) *model.AppError); ok { + r1 = rf(postId) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetFromMaster provides a mock function with given fields: id