From 8dc7c5762f94fee37f4005694161b4f8060a5913 Mon Sep 17 00:00:00 2001 From: Rodrigo Villablanca Date: Thu, 18 Jun 2020 00:26:35 -0400 Subject: [PATCH] Migrate GetDeletedByName from ChannelStore to return error interface (#14709) Automatic Merge --- app/slackimport.go | 2 +- i18n/en.json | 8 -------- store/opentracing_layer.go | 2 +- store/sqlstore/channel_store.go | 6 +++--- store/store.go | 2 +- store/storetest/channel_store.go | 8 ++++---- store/storetest/mocks/ChannelStore.go | 10 ++++------ store/timer_layer.go | 2 +- 8 files changed, 15 insertions(+), 25 deletions(-) diff --git a/app/slackimport.go b/app/slackimport.go index 2903a5b34e..eed160cba8 100644 --- a/app/slackimport.go +++ b/app/slackimport.go @@ -536,7 +536,7 @@ func (a *App) SlackAddChannels(teamId string, slackchannels []SlackChannel, post if mChannel, err = a.Srv().Store.Channel().GetByName(teamId, sChannel.Name, true); err == nil { // The channel already exists as an active channel. Merge with the existing one. importerLog.WriteString(utils.T("api.slackimport.slack_add_channels.merge", map[string]interface{}{"DisplayName": newChannel.DisplayName})) - } else if _, err := a.Srv().Store.Channel().GetDeletedByName(teamId, sChannel.Name); err == nil { + } else if _, nErr := a.Srv().Store.Channel().GetDeletedByName(teamId, sChannel.Name); nErr == nil { // The channel already exists but has been deleted. Generate a random string for the handle instead. newChannel.Name = model.NewId() newChannel = SlackSanitiseChannelProperties(newChannel) diff --git a/i18n/en.json b/i18n/en.json index c8c87aaa3d..91c1f8e898 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -6198,14 +6198,6 @@ "id": "store.sql_channel.get_channels_by_ids.not_found.app_error", "translation": "No channel found." }, - { - "id": "store.sql_channel.get_deleted_by_name.existing.app_error", - "translation": "Unable to find the existing deleted channel." - }, - { - "id": "store.sql_channel.get_deleted_by_name.missing.app_error", - "translation": "No deleted channel exists with that name." - }, { "id": "store.sql_channel.get_for_post.app_error", "translation": "Unable to get the channel for the given post." diff --git a/store/opentracing_layer.go b/store/opentracing_layer.go index b97bce0e9a..523dc7fd4b 100644 --- a/store/opentracing_layer.go +++ b/store/opentracing_layer.go @@ -990,7 +990,7 @@ func (s *OpenTracingLayerChannelStore) GetDeleted(team_id string, offset int, li return resultVar0, resultVar1 } -func (s *OpenTracingLayerChannelStore) GetDeletedByName(team_id string, name string) (*model.Channel, *model.AppError) { +func (s *OpenTracingLayerChannelStore) GetDeletedByName(team_id string, name string) (*model.Channel, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.GetDeletedByName") s.Root.Store.SetContext(newCtx) diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index f56ca004e1..c5646cee3e 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -1256,14 +1256,14 @@ func (s SqlChannelStore) getByName(teamId string, name string, includeDeleted bo return &channel, nil } -func (s SqlChannelStore) GetDeletedByName(teamId string, name string) (*model.Channel, *model.AppError) { +func (s SqlChannelStore) GetDeletedByName(teamId string, name string) (*model.Channel, error) { channel := model.Channel{} if err := s.GetReplica().SelectOne(&channel, "SELECT * FROM Channels WHERE (TeamId = :TeamId OR TeamId = '') AND Name = :Name AND DeleteAt != 0", map[string]interface{}{"TeamId": teamId, "Name": name}); err != nil { if err == sql.ErrNoRows { - return nil, model.NewAppError("SqlChannelStore.GetDeletedByName", "store.sql_channel.get_deleted_by_name.missing.app_error", nil, "teamId="+teamId+", "+"name="+name+", "+err.Error(), http.StatusNotFound) + return nil, store.NewErrNotFound("Channel", fmt.Sprintf("name=%s", name)) } - return nil, model.NewAppError("SqlChannelStore.GetDeletedByName", "store.sql_channel.get_deleted_by_name.existing.app_error", nil, "teamId="+teamId+", "+"name="+name+", "+err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to get channel by teamId=%s and name=%s", teamId, name) } return &channel, nil diff --git a/store/store.go b/store/store.go index ed60ac35cf..cc5d1747f8 100644 --- a/store/store.go +++ b/store/store.go @@ -146,7 +146,7 @@ type ChannelStore interface { GetByName(team_id string, name string, allowFromCache bool) (*model.Channel, error) GetByNames(team_id string, names []string, allowFromCache bool) ([]*model.Channel, error) GetByNameIncludeDeleted(team_id string, name string, allowFromCache bool) (*model.Channel, error) - GetDeletedByName(team_id string, name string) (*model.Channel, *model.AppError) + GetDeletedByName(team_id string, name string) (*model.Channel, error) GetDeleted(team_id string, offset int, limit int, userId string) (*model.ChannelList, error) GetChannels(teamId string, userId string, includeDeleted bool) (*model.ChannelList, error) GetAllChannels(page, perPage int, opts ChannelSearchOpts) (*model.ChannelListWithTeamData, error) diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index 69c1d46697..c7f4227a54 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -734,12 +734,12 @@ func testChannelStoreGetDeletedByName(t *testing.T, ss store.Store) { o1.DeleteAt = now o1.UpdateAt = now - r1, err := ss.Channel().GetDeletedByName(o1.TeamId, o1.Name) - require.Nil(t, err) + r1, nErr := ss.Channel().GetDeletedByName(o1.TeamId, o1.Name) + require.Nil(t, nErr) require.Equal(t, o1, r1) - _, err = ss.Channel().GetDeletedByName(o1.TeamId, "") - require.NotNil(t, err, "missing id should have failed") + _, nErr = ss.Channel().GetDeletedByName(o1.TeamId, "") + require.NotNil(t, nErr, "missing id should have failed") } func testChannelStoreGetDeleted(t *testing.T, ss store.Store) { diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index 9725d1dd8a..a5d3a9397e 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -675,7 +675,7 @@ func (_m *ChannelStore) GetDeleted(team_id string, offset int, limit int, userId } // GetDeletedByName provides a mock function with given fields: team_id, name -func (_m *ChannelStore) GetDeletedByName(team_id string, name string) (*model.Channel, *model.AppError) { +func (_m *ChannelStore) GetDeletedByName(team_id string, name string) (*model.Channel, error) { ret := _m.Called(team_id, name) var r0 *model.Channel @@ -687,13 +687,11 @@ func (_m *ChannelStore) GetDeletedByName(team_id string, name string) (*model.Ch } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string, string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string, string) error); ok { r1 = rf(team_id, name) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 diff --git a/store/timer_layer.go b/store/timer_layer.go index df6aa6121f..5f234c5f00 100644 --- a/store/timer_layer.go +++ b/store/timer_layer.go @@ -920,7 +920,7 @@ func (s *TimerLayerChannelStore) GetDeleted(team_id string, offset int, limit in return resultVar0, resultVar1 } -func (s *TimerLayerChannelStore) GetDeletedByName(team_id string, name string) (*model.Channel, *model.AppError) { +func (s *TimerLayerChannelStore) GetDeletedByName(team_id string, name string) (*model.Channel, error) { start := timemodule.Now() resultVar0, resultVar1 := s.ChannelStore.GetDeletedByName(team_id, name)