MM-24132: Migrate AppError from SaveDirectChannel/channel_store.go (#14318)

* MM-24135: Migrate AppError from SaveChannel/channel_store.go

This is the first POC of migration of store app errors to plain error.

We create a few basic error types in the store package and use
them to return the errors from store methods. In the app layer,
we inspect the error and re-create the exact app errors. This lets
us preserve the same error content, but yet move to plain errors.

Since this is a gradual migration, this means that the error inspection
code will be duplicated across the app layer whenever a store method
is invoked. But all of that should go away once we start propagating
the errors higher up the hierarchy.

There have been a significant amount of changes in the storetest and searchtest
layer, primarily because we have to rename the err variable now that it is of
a different type.

* Addressed review comments

* MM-24132: Migrate AppError from SaveDirectChannel/channel_store.go

This PR migrates 2 new methods SaveDirectChannel and CreateDirectChannel
to return error instead of AppError.

We also need to handle the error internally in SaveMultipleMember for now
until that is migrated too.

* Fix layers

Co-authored-by: mattermod <mattermod@users.noreply.github.com>
Этот коммит содержится в:
Agniva De Sarker
2020-05-14 23:41:05 +05:30
коммит произвёл GitHub
родитель 624980ff54
Коммит 090743de86
11 изменённых файлов: 121 добавлений и 152 удалений

Просмотреть файл

@@ -534,7 +534,7 @@ func (s SqlChannelStore) Save(channel *model.Channel, maxChannelsPerTeam int64)
return newChannel, nil
}
func (s SqlChannelStore) CreateDirectChannel(user *model.User, otherUser *model.User) (*model.Channel, *model.AppError) {
func (s SqlChannelStore) CreateDirectChannel(user *model.User, otherUser *model.User) (*model.Channel, error) {
channel := new(model.Channel)
channel.DisplayName = ""
@@ -559,74 +559,42 @@ func (s SqlChannelStore) CreateDirectChannel(user *model.User, otherUser *model.
return s.SaveDirectChannel(channel, cm1, cm2)
}
func (s SqlChannelStore) SaveDirectChannel(directchannel *model.Channel, member1 *model.ChannelMember, member2 *model.ChannelMember) (*model.Channel, *model.AppError) {
func (s SqlChannelStore) SaveDirectChannel(directchannel *model.Channel, member1 *model.ChannelMember, member2 *model.ChannelMember) (*model.Channel, error) {
if directchannel.DeleteAt != 0 {
return nil, model.NewAppError("SqlChannelStore.Save", "store.sql_channel.save.archived_channel.app_error", nil, "", http.StatusBadRequest)
return nil, store.NewErrInvalidInput("Channel", "DeleteAt", directchannel.DeleteAt)
}
if directchannel.Type != model.CHANNEL_DIRECT {
return nil, model.NewAppError("SqlChannelStore.SaveDirectChannel", "store.sql_channel.save_direct_channel.not_direct.app_error", nil, "", http.StatusBadRequest)
return nil, store.NewErrInvalidInput("Channel", "Type", directchannel.Type)
}
transaction, err := s.GetMaster().Begin()
if err != nil {
return nil, model.NewAppError("SqlChannelStore.SaveDirectChannel", "store.sql_channel.save_direct_channel.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrap(err, "begin_transaction")
}
defer finalizeTransaction(transaction)
directchannel.TeamId = ""
newChannel, err := s.saveChannelT(transaction, directchannel, 0)
if err != nil {
// TODO: This will go away once SaveDirectChannel returns error
var invErr *store.ErrInvalidInput
var cErr *store.ErrConflict
var ltErr *store.ErrLimitExceeded
var appErr *model.AppError
if errors.As(err, &invErr) {
if invErr.Entity == "Channel" && invErr.Field == "DeleteAt" {
return newChannel, model.NewAppError("CreateChannel", "store.sql_channel.save.archived_channel.app_error", nil, "", http.StatusBadRequest)
}
if invErr.Entity == "Channel" && invErr.Field == "Type" {
return newChannel, model.NewAppError("CreateChannel", "store.sql_channel.save.direct_channel.app_error", nil, "", http.StatusBadRequest)
}
if invErr.Entity == "Channel" && invErr.Field == "Id" {
return newChannel, model.NewAppError("SqlChannelStore.Save", "store.sql_channel.save_channel.existing.app_error", nil, "id="+invErr.Value.(string), http.StatusBadRequest)
}
}
if errors.As(err, &cErr) {
if cErr.Resource == "Channel" {
return newChannel, model.NewAppError("CreateChannel", store.CHANNEL_EXISTS_ERROR, nil, cErr.Error(), http.StatusBadRequest)
}
}
if errors.As(err, &ltErr) {
if ltErr.What == "channels_per_team" {
return newChannel, model.NewAppError("CreateChannel", "store.sql_channel.save_channel.limit.app_error", nil, ltErr.Error(), http.StatusBadRequest)
}
}
if errors.As(err, &appErr) {
return nil, appErr
} else {
return nil, model.NewAppError("SqlChannelStore.SaveDirectChannel", "store.sql_channel.save_direct_channel.internal_error", nil, err.Error(), http.StatusInternalServerError)
}
return newChannel, err
}
// Members need new channel ID
member1.ChannelId = newChannel.Id
member2.ChannelId = newChannel.Id
var memberSaveErr *model.AppError
if member1.UserId != member2.UserId {
_, memberSaveErr = s.saveMultipleMembersT(transaction, []*model.ChannelMember{member1, member2})
_, err = s.saveMultipleMembersT(transaction, []*model.ChannelMember{member1, member2})
} else {
_, memberSaveErr = s.saveMemberT(transaction, member2)
_, err = s.saveMemberT(transaction, member2)
}
if memberSaveErr != nil {
return nil, model.NewAppError("SqlChannelStore.SaveDirectChannel", "store.sql_channel.save_direct_channel.add_members.app_error", nil, memberSaveErr.Error(), http.StatusInternalServerError)
if err != nil {
return nil, err
}
if err := transaction.Commit(); err != nil {
return nil, model.NewAppError("SqlChannelStore.SaveDirectChannel", "store.sql_channel.save_direct_channel.commit.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrap(err, "commit_transaction")
}
return newChannel, nil
@@ -639,7 +607,7 @@ func (s SqlChannelStore) saveChannelT(transaction *gorp.Transaction, channel *mo
}
channel.PreSave()
if err := channel.IsValid(); err != nil { // TODO: this needs to return plain error
if err := channel.IsValid(); err != nil { // TODO: this needs to return plain error in v6.
return nil, err // we just pass through the error as-is for now.
}
@@ -1345,9 +1313,21 @@ func (s SqlChannelStore) SaveMultipleMembers(members []*model.ChannelMember) ([]
}
defer finalizeTransaction(transaction)
newMembers, appErr := s.saveMultipleMembersT(transaction, members)
if appErr != nil {
return nil, appErr
newMembers, err := s.saveMultipleMembersT(transaction, members)
if err != nil { // TODO: this will go away once SaveMultipleMembers is migrated too.
var cErr *store.ErrConflict
var appErr *model.AppError
switch {
case errors.As(err, &cErr):
switch cErr.Resource {
case "ChannelMembers":
return nil, model.NewAppError("CreateChannel", "store.sql_channel.save_member.exists.app_error", nil, cErr.Error(), http.StatusBadRequest)
}
case errors.As(err, &appErr): // in case we haven't converted to plain error.
return nil, appErr
default: // last fallback in case it doesn't map to an existing app error.
return nil, model.NewAppError("CreateDirectChannel", "app.channel.create_direct_channel.internal_error", nil, err.Error(), http.StatusInternalServerError)
}
}
if err := transaction.Commit(); err != nil {
@@ -1365,7 +1345,7 @@ func (s SqlChannelStore) SaveMember(member *model.ChannelMember) (*model.Channel
return newMembers[0], nil
}
func (s SqlChannelStore) saveMultipleMembersT(transaction *gorp.Transaction, members []*model.ChannelMember) ([]*model.ChannelMember, *model.AppError) {
func (s SqlChannelStore) saveMultipleMembersT(transaction *gorp.Transaction, members []*model.ChannelMember) ([]*model.ChannelMember, error) {
newChannelMembers := map[string]int{}
users := map[string]bool{}
for _, member := range members {
@@ -1377,7 +1357,7 @@ func (s SqlChannelStore) saveMultipleMembersT(transaction *gorp.Transaction, mem
users[member.UserId] = true
member.PreSave()
if err := member.IsValid(); err != nil {
if err := member.IsValid(); err != nil { // TODO: this needs to return plain error in v6.
return nil, err
}
}
@@ -1407,7 +1387,7 @@ func (s SqlChannelStore) saveMultipleMembersT(transaction *gorp.Transaction, mem
channelRolesSql, channelRolesArgs, err := channelRolesQuery.ToSql()
if err != nil {
return nil, model.NewAppError("SqlChannelStore.SaveMultipleMembers", "store.sql_channel.save_multimple_members.channel_roles.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrap(err, "channel_roles_tosql")
}
var defaultChannelsRoles []struct {
@@ -1418,7 +1398,7 @@ func (s SqlChannelStore) saveMultipleMembersT(transaction *gorp.Transaction, mem
}
_, err = s.GetMaster().Select(&defaultChannelsRoles, channelRolesSql, channelRolesArgs...)
if err != nil {
return nil, model.NewAppError("SqlChannelStore.SaveMultipleMembers", "store.sql_channel.save_multimple_members.channel_roles_query.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrap(err, "default_channel_roles_select")
}
for _, defaultRoles := range defaultChannelsRoles {
@@ -1446,7 +1426,7 @@ func (s SqlChannelStore) saveMultipleMembersT(transaction *gorp.Transaction, mem
teamRolesSql, teamRolesArgs, err := teamRolesQuery.ToSql()
if err != nil {
return nil, model.NewAppError("SqlChannelStore.SaveMultipleMembers", "store.sql_channel.save_multimple_members.team_roles.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrap(err, "team_roles_tosql")
}
var defaultTeamsRoles []struct {
@@ -1457,7 +1437,7 @@ func (s SqlChannelStore) saveMultipleMembersT(transaction *gorp.Transaction, mem
}
_, err = s.GetMaster().Select(&defaultTeamsRoles, teamRolesSql, teamRolesArgs...)
if err != nil {
return nil, model.NewAppError("SqlChannelStore.SaveMultipleMembers", "store.sql_channel.save_multimple_members.team_roles_query.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrap(err, "default_team_roles_select")
}
for _, defaultRoles := range defaultTeamsRoles {
@@ -1471,14 +1451,14 @@ func (s SqlChannelStore) saveMultipleMembersT(transaction *gorp.Transaction, mem
sql, args, err := query.ToSql()
if err != nil {
return nil, model.NewAppError("SqlTeamStore.SaveMember", "store.sql_channel.save_member.save.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrap(err, "channel_members_tosql")
}
if _, err := s.GetMaster().Exec(sql, args...); err != nil {
if IsUniqueConstraintError(err, []string{"ChannelId", "channelmembers_pkey", "PRIMARY"}) {
return nil, model.NewAppError("SqlTeamStore.SaveMember", "store.sql_channel.save_member.exists.app_error", nil, err.Error(), http.StatusBadRequest)
return nil, store.NewErrConflict("ChannelMembers", err, "")
}
return nil, model.NewAppError("SqlTeamStore.SaveMember", "store.sql_channel.save_member.save.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrap(err, "channel_members_save")
}
newMembers := []*model.ChannelMember{}
@@ -1506,7 +1486,7 @@ func (s SqlChannelStore) saveMultipleMembersT(transaction *gorp.Transaction, mem
return newMembers, nil
}
func (s SqlChannelStore) saveMemberT(transaction *gorp.Transaction, member *model.ChannelMember) (*model.ChannelMember, *model.AppError) {
func (s SqlChannelStore) saveMemberT(transaction *gorp.Transaction, member *model.ChannelMember) (*model.ChannelMember, error) {
members, err := s.saveMultipleMembersT(transaction, []*model.ChannelMember{member})
if err != nil {
return nil, err