Migrate Delete/Restore/SetDeleteAt methods from ChannelStore to return error interface (#14700)

Automatic Merge

app/channel.go

@@ -643,7 +643,7 @@ func (a *App) RestoreChannel(channel *model.Channel, userId string) (*model.Chan
 	}
 
 	if err := a.Srv().Store.Channel().Restore(channel.Id, model.GetMillis()); err != nil {
-		return nil, err
+		return nil, model.NewAppError("RestoreChannel", "app.channel.restore.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 	channel.DeleteAt = 0
 	a.invalidateCacheForChannel(channel)
@@ -1197,7 +1197,7 @@ func (a *App) DeleteChannel(channel *model.Channel, userId string) *model.AppErr
 	deleteAt := model.GetMillis()
 
 	if err := a.Srv().Store.Channel().Delete(channel.Id, deleteAt); err != nil {
-		return err
+		return model.NewAppError("DeleteChannel", "app.channel.delete.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 	a.invalidateCacheForChannel(channel)
 

i18n/en.json

@@ -2986,6 +2986,10 @@
     "id": "app.channel.create_direct_channel.internal_error",
     "translation": "Unable to save direct channel."
   },
+  {
+    "id": "app.channel.delete.app_error",
+    "translation": "Unable to delete the channel."
+  },
   {
     "id": "app.channel.get.existing.app_error",
     "translation": "Unable to find the existing channel."
@@ -3018,6 +3022,10 @@
     "id": "app.channel.post_update_channel_purpose_message.updated_to",
     "translation": "%s updated the channel purpose to: %s"
   },
+  {
+    "id": "app.channel.restore.app_error",
+    "translation": "Unable to restore the channel."
+  },
   {
     "id": "app.channel.update.bad_id",
     "translation": "Unable to update the channel."
@@ -6098,10 +6106,6 @@
     "id": "store.sql_channel.count_posts_since.app_error",
     "translation": "Unable to count messages since given date."
   },
-  {
-    "id": "store.sql_channel.delete.channel.app_error",
-    "translation": "Unable to delete the channel."
-  },
   {
     "id": "store.sql_channel.get.existing.app_error",
     "translation": "Unable to find the existing channel."
@@ -6346,18 +6350,6 @@
     "id": "store.sql_channel.search_group_channels.app_error",
     "translation": "Unable to get the group channels for the given user and term."
   },
-  {
-    "id": "store.sql_channel.set_delete_at.commit_transaction.app_error",
-    "translation": "Unable to commit transaction."
-  },
-  {
-    "id": "store.sql_channel.set_delete_at.open_transaction.app_error",
-    "translation": "Unable to open transaction."
-  },
-  {
-    "id": "store.sql_channel.set_delete_at.update_public_channel.app_error",
-    "translation": "Unable to update the materialized public channel."
-  },
   {
     "id": "store.sql_channel.update_last_viewed_at.app_error",
     "translation": "Unable to update the last viewed at time."

store/opentracing_layer.go

@@ -612,7 +612,7 @@ func (s *OpenTracingLayerChannelStore) CreateDirectChannel(userId *model.User, o
 	return resultVar0, resultVar1
 }
 
-func (s *OpenTracingLayerChannelStore) Delete(channelId string, time int64) *model.AppError {
+func (s *OpenTracingLayerChannelStore) Delete(channelId string, time int64) error {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.Delete")
 	s.Root.Store.SetContext(newCtx)
@@ -1647,7 +1647,7 @@ func (s *OpenTracingLayerChannelStore) ResetAllChannelSchemes() *model.AppError
 	return resultVar0
 }
 
-func (s *OpenTracingLayerChannelStore) Restore(channelId string, time int64) *model.AppError {
+func (s *OpenTracingLayerChannelStore) Restore(channelId string, time int64) error {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.Restore")
 	s.Root.Store.SetContext(newCtx)
@@ -1845,7 +1845,7 @@ func (s *OpenTracingLayerChannelStore) SearchMore(userId string, teamId string,
 	return resultVar0, resultVar1
 }
 
-func (s *OpenTracingLayerChannelStore) SetDeleteAt(channelId string, deleteAt int64, updateAt int64) *model.AppError {
+func (s *OpenTracingLayerChannelStore) SetDeleteAt(channelId string, deleteAt int64, updateAt int64) error {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.SetDeleteAt")
 	s.Root.Store.SetContext(newCtx)

store/sqlstore/channel_store.go

@@ -770,28 +770,28 @@ func (s SqlChannelStore) get(id string, master bool, allowFromCache bool) (*mode
 }
 
 // Delete records the given deleted timestamp to the channel in question.
-func (s SqlChannelStore) Delete(channelId string, time int64) *model.AppError {
+func (s SqlChannelStore) Delete(channelId string, time int64) error {
 	return s.SetDeleteAt(channelId, time, time)
 }
 
 // Restore reverts a previous deleted timestamp from the channel in question.
-func (s SqlChannelStore) Restore(channelId string, time int64) *model.AppError {
+func (s SqlChannelStore) Restore(channelId string, time int64) error {
 	return s.SetDeleteAt(channelId, 0, time)
 }
 
 // SetDeleteAt records the given deleted and updated timestamp to the channel in question.
-func (s SqlChannelStore) SetDeleteAt(channelId string, deleteAt, updateAt int64) *model.AppError {
+func (s SqlChannelStore) SetDeleteAt(channelId string, deleteAt, updateAt int64) error {
 	defer s.InvalidateChannel(channelId)
 
 	transaction, err := s.GetMaster().Begin()
 	if err != nil {
-		return model.NewAppError("SqlChannelStore.SetDeleteAt", "store.sql_channel.set_delete_at.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return errors.Wrap(err, "SetDeleteAt: begin_transaction")
 	}
 	defer finalizeTransaction(transaction)
 
-	appErr := s.setDeleteAtT(transaction, channelId, deleteAt, updateAt)
+	err = s.setDeleteAtT(transaction, channelId, deleteAt, updateAt)
-	if appErr != nil {
+	if err != nil {
-		return appErr
+		return errors.Wrap(err, "setDeleteAtT")
 	}
 
 	// Additionally propagate the write to the PublicChannels table.
@@ -806,20 +806,20 @@ func (s SqlChannelStore) SetDeleteAt(channelId string, deleteAt, updateAt int64)
 		"DeleteAt":  deleteAt,
 		"ChannelId": channelId,
 	}); err != nil {
-		return model.NewAppError("SqlChannelStore.SetDeleteAt", "store.sql_channel.set_delete_at.update_public_channel.app_error", nil, "channel_id="+channelId+", "+err.Error(), http.StatusInternalServerError)
+		return errors.Wrapf(err, "failed to delete public channels with id=%s", channelId)
 	}
 
 	if err := transaction.Commit(); err != nil {
-		return model.NewAppError("SqlChannelStore.SetDeleteAt", "store.sql_channel.set_delete_at.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return errors.Wrapf(err, "SetDeleteAt: commit_transaction")
 	}
 
 	return nil
 }
 
-func (s SqlChannelStore) setDeleteAtT(transaction *gorp.Transaction, channelId string, deleteAt, updateAt int64) *model.AppError {
+func (s SqlChannelStore) setDeleteAtT(transaction *gorp.Transaction, channelId string, deleteAt, updateAt int64) error {
 	_, err := transaction.Exec("Update Channels SET DeleteAt = :DeleteAt, UpdateAt = :UpdateAt WHERE Id = :ChannelId", map[string]interface{}{"DeleteAt": deleteAt, "UpdateAt": updateAt, "ChannelId": channelId})
 	if err != nil {
-		return model.NewAppError("SqlChannelStore.Delete", "store.sql_channel.delete.channel.app_error", nil, "id="+channelId+", err="+err.Error(), http.StatusInternalServerError)
+		return errors.Wrapf(err, "failed to delete channel with id=%s", channelId)
 	}
 
 	return nil

store/store.go

@@ -138,9 +138,9 @@ type ChannelStore interface {
 	InvalidateChannel(id string)
 	InvalidateChannelByName(teamId, name string)
 	GetFromMaster(id string) (*model.Channel, error)
-	Delete(channelId string, time int64) *model.AppError
+	Delete(channelId string, time int64) error
-	Restore(channelId string, time int64) *model.AppError
+	Restore(channelId string, time int64) error
-	SetDeleteAt(channelId string, deleteAt int64, updateAt int64) *model.AppError
+	SetDeleteAt(channelId string, deleteAt int64, updateAt int64) error
 	PermanentDelete(channelId string) *model.AppError
 	PermanentDeleteByTeam(teamId string) *model.AppError
 	GetByName(team_id string, name string, allowFromCache bool) (*model.Channel, *model.AppError)

store/storetest/channel_store.go

@@ -464,8 +464,8 @@ func testChannelStoreGetChannelsByIds(t *testing.T, ss store.Store) {
 	o3.Type = model.CHANNEL_OPEN
 	_, nErr = ss.Channel().Save(&o3, -1)
 	require.Nil(t, nErr)
-	err = ss.Channel().Delete(o3.Id, 123)
+	nErr = ss.Channel().Delete(o3.Id, 123)
-	require.Nil(t, err)
+	require.Nil(t, nErr)
 	o3.DeleteAt = 123
 	o3.UpdateAt = 123
 
@@ -600,14 +600,14 @@ func testChannelStoreDelete(t *testing.T, ss store.Store) {
 	_, err = ss.Channel().SaveMember(&m2)
 	require.Nil(t, err)
 
-	err = ss.Channel().Delete(o1.Id, model.GetMillis())
+	nErr = ss.Channel().Delete(o1.Id, model.GetMillis())
-	require.Nil(t, err, err)
+	require.Nil(t, nErr, nErr)
 
 	c, _ := ss.Channel().Get(o1.Id, false)
 	require.NotEqual(t, 0, c.DeleteAt, "should have been deleted")
 
-	err = ss.Channel().Delete(o3.Id, model.GetMillis())
+	nErr = ss.Channel().Delete(o3.Id, model.GetMillis())
-	require.Nil(t, err, err)
+	require.Nil(t, nErr, nErr)
 
 	list, err := ss.Channel().GetChannels(o1.TeamId, m1.UserId, false)
 	require.Nil(t, err)
@@ -656,8 +656,8 @@ func testChannelStoreGetByName(t *testing.T, ss store.Store) {
 	result, err = ss.Channel().GetByName(o1.TeamId, "", false)
 	require.NotNil(t, err, "Missing id should have failed")
 
-	err = ss.Channel().Delete(channelID, model.GetMillis())
+	nErr = ss.Channel().Delete(channelID, model.GetMillis())
-	require.Nil(t, err, "channel should have been deleted")
+	require.Nil(t, nErr, "channel should have been deleted")
 
 	result, err = ss.Channel().GetByName(o1.TeamId, o1.Name, false)
 	require.NotNil(t, err, "Deleted channel should not be returned by GetByName()")
@@ -3223,8 +3223,8 @@ func testChannelStoreGetAllChannels(t *testing.T, ss store.Store, s SqlSupplier)
 	require.Nil(t, nErr)
 	c2.DeleteAt = model.GetMillis()
 	c2.UpdateAt = c2.DeleteAt
-	err = ss.Channel().Delete(c2.Id, c2.DeleteAt)
+	nErr = ss.Channel().Delete(c2.Id, c2.DeleteAt)
-	require.Nil(t, err, "channel should have been deleted")
+	require.Nil(t, nErr, "channel should have been deleted")
 
 	c3 := model.Channel{}
 	c3.TeamId = t2.Id
@@ -3408,8 +3408,8 @@ func testChannelStoreGetMoreChannels(t *testing.T, ss store.Store) {
 	_, nErr = ss.Channel().Save(&o7, -1)
 	require.Nil(t, nErr)
 
-	err = ss.Channel().Delete(o7.Id, model.GetMillis())
+	nErr = ss.Channel().Delete(o7.Id, model.GetMillis())
-	require.Nil(t, err, "channel should have been deleted")
+	require.Nil(t, nErr, "channel should have been deleted")
 
 	t.Run("both o3 and o6 listed in more channels", func(t *testing.T) {
 		list, err := ss.Channel().GetMoreChannels(teamId, userId, 0, 100)
@@ -4716,8 +4716,8 @@ func testChannelStoreSearchMore(t *testing.T, ss store.Store) {
 
 	o10.DeleteAt = model.GetMillis()
 	o10.UpdateAt = o10.DeleteAt
-	err = ss.Channel().Delete(o10.Id, o10.DeleteAt)
+	nErr = ss.Channel().Delete(o10.Id, o10.DeleteAt)
-	require.Nil(t, err, "channel should have been deleted")
+	require.Nil(t, nErr, "channel should have been deleted")
 
 	t.Run("three public channels matching 'ChannelA', but already a member of one and one deleted", func(t *testing.T) {
 		channels, err := ss.Channel().SearchMore(m1.UserId, teamId, "ChannelA")
@@ -4911,8 +4911,8 @@ func testChannelStoreSearchInTeam(t *testing.T, ss store.Store) {
 	require.Nil(t, nErr)
 	o13.DeleteAt = model.GetMillis()
 	o13.UpdateAt = o13.DeleteAt
-	err = ss.Channel().Delete(o13.Id, o13.DeleteAt)
+	nErr = ss.Channel().Delete(o13.Id, o13.DeleteAt)
-	require.Nil(t, err, "channel should have been deleted")
+	require.Nil(t, nErr, "channel should have been deleted")
 
 	testCases := []struct {
 		Description     string
@@ -5236,8 +5236,8 @@ func testChannelStoreSearchAllChannels(t *testing.T, ss store.Store) {
 
 	o13.DeleteAt = model.GetMillis()
 	o13.UpdateAt = o13.DeleteAt
-	err = ss.Channel().Delete(o13.Id, o13.DeleteAt)
+	nErr = ss.Channel().Delete(o13.Id, o13.DeleteAt)
-	require.Nil(t, err, "channel should have been deleted")
+	require.Nil(t, nErr, "channel should have been deleted")
 
 	testCases := []struct {
 		Description     string
@@ -5522,14 +5522,14 @@ func testChannelStoreAnalyticsDeletedTypeCount(t *testing.T, ss store.Store) {
 	directStartCount, err = ss.Channel().AnalyticsDeletedTypeCount("", "D")
 	require.Nil(t, err, err)
 
-	err = ss.Channel().Delete(o1.Id, model.GetMillis())
+	nErr = ss.Channel().Delete(o1.Id, model.GetMillis())
-	require.Nil(t, err, "channel should have been deleted")
+	require.Nil(t, nErr, "channel should have been deleted")
-	err = ss.Channel().Delete(o2.Id, model.GetMillis())
+	nErr = ss.Channel().Delete(o2.Id, model.GetMillis())
-	require.Nil(t, err, "channel should have been deleted")
+	require.Nil(t, nErr, "channel should have been deleted")
-	err = ss.Channel().Delete(p3.Id, model.GetMillis())
+	nErr = ss.Channel().Delete(p3.Id, model.GetMillis())
-	require.Nil(t, err, "channel should have been deleted")
+	require.Nil(t, nErr, "channel should have been deleted")
-	err = ss.Channel().Delete(d4.Id, model.GetMillis())
+	nErr = ss.Channel().Delete(d4.Id, model.GetMillis())
-	require.Nil(t, err, "channel should have been deleted")
+	require.Nil(t, nErr, "channel should have been deleted")
 
 	var count int64
 
@@ -6397,8 +6397,8 @@ func testChannelStoreExportAllDirectChannelsDeletedChannel(t *testing.T, ss stor
 	ss.Channel().SaveDirectChannel(&o1, &m1, &m2)
 
 	o1.DeleteAt = 1
-	err = ss.Channel().SetDeleteAt(o1.Id, 1, 1)
+	nErr := ss.Channel().SetDeleteAt(o1.Id, 1, 1)
-	require.Nil(t, err, "channel should have been deleted")
+	require.Nil(t, nErr, "channel should have been deleted")
 
 	d1, err := ss.Channel().GetAllDirectChannelsForExportAfter(10000, strings.Repeat("0", 26))
 	assert.Nil(t, err)

store/storetest/group_store.go

@@ -1664,8 +1664,8 @@ func testChannelMembersToAdd(t *testing.T, ss store.Store) {
 	require.Len(t, channelMembers, 1)
 
 	// No result if Channel deleted
-	err = ss.Channel().Delete(channel.Id, model.GetMillis())
+	nErr = ss.Channel().Delete(channel.Id, model.GetMillis())
-	require.Nil(t, err)
+	require.Nil(t, nErr)
 	channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil)
 	require.Nil(t, err)
 	require.Empty(t, channelMembers)

store/storetest/mocks/ChannelStore.go

@@ -179,16 +179,14 @@ func (_m *ChannelStore) CreateDirectChannel(userId *model.User, otherUserId *mod
 }
 
 // Delete provides a mock function with given fields: channelId, time
-func (_m *ChannelStore) Delete(channelId string, time int64) *model.AppError {
+func (_m *ChannelStore) Delete(channelId string, time int64) error {
 	ret := _m.Called(channelId, time)
 
-	var r0 *model.AppError
+	var r0 error
-	if rf, ok := ret.Get(0).(func(string, int64) *model.AppError); ok {
+	if rf, ok := ret.Get(0).(func(string, int64) error); ok {
 		r0 = rf(channelId, time)
 	} else {
-		if ret.Get(0) != nil {
+		r0 = ret.Error(0)
-			r0 = ret.Get(0).(*model.AppError)
-		}
 	}
 
 	return r0
@@ -1402,16 +1400,14 @@ func (_m *ChannelStore) ResetAllChannelSchemes() *model.AppError {
 }
 
 // Restore provides a mock function with given fields: channelId, time
-func (_m *ChannelStore) Restore(channelId string, time int64) *model.AppError {
+func (_m *ChannelStore) Restore(channelId string, time int64) error {
 	ret := _m.Called(channelId, time)
 
-	var r0 *model.AppError
+	var r0 error
-	if rf, ok := ret.Get(0).(func(string, int64) *model.AppError); ok {
+	if rf, ok := ret.Get(0).(func(string, int64) error); ok {
 		r0 = rf(channelId, time)
 	} else {
-		if ret.Get(0) != nil {
+		r0 = ret.Error(0)
-			r0 = ret.Get(0).(*model.AppError)
-		}
 	}
 
 	return r0
@@ -1671,16 +1667,14 @@ func (_m *ChannelStore) SearchMore(userId string, teamId string, term string) (*
 }
 
 // SetDeleteAt provides a mock function with given fields: channelId, deleteAt, updateAt
-func (_m *ChannelStore) SetDeleteAt(channelId string, deleteAt int64, updateAt int64) *model.AppError {
+func (_m *ChannelStore) SetDeleteAt(channelId string, deleteAt int64, updateAt int64) error {
 	ret := _m.Called(channelId, deleteAt, updateAt)
 
-	var r0 *model.AppError
+	var r0 error
-	if rf, ok := ret.Get(0).(func(string, int64, int64) *model.AppError); ok {
+	if rf, ok := ret.Get(0).(func(string, int64, int64) error); ok {
 		r0 = rf(channelId, deleteAt, updateAt)
 	} else {
-		if ret.Get(0) != nil {
+		r0 = ret.Error(0)
-			r0 = ret.Get(0).(*model.AppError)
-		}
 	}
 
 	return r0

store/storetest/post_store.go

@@ -2686,8 +2686,8 @@ func testPostStoreGetDirectPostParentsForExportAfterDeleted(t *testing.T, ss sto
 	ss.Channel().SaveDirectChannel(&o1, &m1, &m2)
 
 	o1.DeleteAt = 1
-	err = ss.Channel().SetDeleteAt(o1.Id, 1, 1)
+	nErr := ss.Channel().SetDeleteAt(o1.Id, 1, 1)
-	assert.Nil(t, err)
+	assert.Nil(t, nErr)
 
 	p1 := &model.Post{}
 	p1.ChannelId = o1.Id

store/timer_layer.go

@@ -584,7 +584,7 @@ func (s *TimerLayerChannelStore) CreateDirectChannel(userId *model.User, otherUs
 	return resultVar0, resultVar1
 }
 
-func (s *TimerLayerChannelStore) Delete(channelId string, time int64) *model.AppError {
+func (s *TimerLayerChannelStore) Delete(channelId string, time int64) error {
 	start := timemodule.Now()
 
 	resultVar0 := s.ChannelStore.Delete(channelId, time)
@@ -1537,7 +1537,7 @@ func (s *TimerLayerChannelStore) ResetAllChannelSchemes() *model.AppError {
 	return resultVar0
 }
 
-func (s *TimerLayerChannelStore) Restore(channelId string, time int64) *model.AppError {
+func (s *TimerLayerChannelStore) Restore(channelId string, time int64) error {
 	start := timemodule.Now()
 
 	resultVar0 := s.ChannelStore.Restore(channelId, time)
@@ -1713,7 +1713,7 @@ func (s *TimerLayerChannelStore) SearchMore(userId string, teamId string, term s
 	return resultVar0, resultVar1
 }
 
-func (s *TimerLayerChannelStore) SetDeleteAt(channelId string, deleteAt int64, updateAt int64) *model.AppError {
+func (s *TimerLayerChannelStore) SetDeleteAt(channelId string, deleteAt int64, updateAt int64) error {
 	start := timemodule.Now()
 
 	resultVar0 := s.ChannelStore.SetDeleteAt(channelId, deleteAt, updateAt)