From 090743de86e16010284af2e703e1b10b28f42b5c Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Thu, 14 May 2020 23:41:05 +0530 Subject: [PATCH] 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 --- app/channel.go | 38 +++++++++-- i18n/en.json | 40 ++---------- store/opentracing_layer.go | 4 +- store/searchlayer/channel_layer.go | 2 +- store/sqlstore/channel_store.go | 94 +++++++++++---------------- store/store.go | 4 +- store/storetest/channel_store.go | 55 ++++++++-------- store/storetest/compliance_store.go | 8 +-- store/storetest/mocks/ChannelStore.go | 20 +++--- store/storetest/user_store.go | 4 +- store/timer_layer.go | 4 +- 11 files changed, 121 insertions(+), 152 deletions(-) diff --git a/app/channel.go b/app/channel.go index eb4332a475..984b8af511 100644 --- a/app/channel.go +++ b/app/channel.go @@ -336,20 +336,44 @@ func (a *App) createDirectChannel(userId string, otherUserId string) (*model.Cha } otherUser := result.Data.(*model.User) - channel, err := a.Srv().Store.Channel().CreateDirectChannel(user, otherUser) - if err != nil { - if err.Id == store.CHANNEL_EXISTS_ERROR { - return channel, err + channel, nErr := a.Srv().Store.Channel().CreateDirectChannel(user, otherUser) + if nErr != nil { + var invErr *store.ErrInvalidInput + var cErr *store.ErrConflict + var ltErr *store.ErrLimitExceeded + var appErr *model.AppError + switch { + case errors.As(nErr, &invErr): + switch { + case invErr.Entity == "Channel" && invErr.Field == "DeleteAt": + return nil, model.NewAppError("CreateChannel", "store.sql_channel.save.archived_channel.app_error", nil, "", http.StatusBadRequest) + case invErr.Entity == "Channel" && invErr.Field == "Type": + return nil, model.NewAppError("CreateChannel", "store.sql_channel.save_direct_channel.not_direct.app_error", nil, "", http.StatusBadRequest) + case invErr.Entity == "Channel" && invErr.Field == "Id": + return nil, model.NewAppError("SqlChannelStore.Save", "store.sql_channel.save_channel.existing.app_error", nil, "id="+invErr.Value.(string), http.StatusBadRequest) + } + case errors.As(nErr, &cErr): + switch cErr.Resource { + case "Channel": + return channel, model.NewAppError("CreateChannel", store.CHANNEL_EXISTS_ERROR, nil, cErr.Error(), http.StatusBadRequest) + case "ChannelMembers": + return nil, model.NewAppError("CreateChannel", "store.sql_channel.save_member.exists.app_error", nil, cErr.Error(), http.StatusBadRequest) + } + case errors.As(nErr, <Err): + return nil, model.NewAppError("CreateChannel", "store.sql_channel.save_channel.limit.app_error", nil, ltErr.Error(), http.StatusBadRequest) + case errors.As(nErr, &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, nErr.Error(), http.StatusInternalServerError) } - return nil, err } - if err = a.Srv().Store.ChannelMemberHistory().LogJoinEvent(userId, channel.Id, model.GetMillis()); err != nil { + if err := a.Srv().Store.ChannelMemberHistory().LogJoinEvent(userId, channel.Id, model.GetMillis()); err != nil { mlog.Error("Failed to update ChannelMemberHistory table", mlog.Err(err)) return nil, err } if userId != otherUserId { - if err = a.Srv().Store.ChannelMemberHistory().LogJoinEvent(otherUserId, channel.Id, model.GetMillis()); err != nil { + if err := a.Srv().Store.ChannelMemberHistory().LogJoinEvent(otherUserId, channel.Id, model.GetMillis()); err != nil { mlog.Error("Failed to update ChannelMemberHistory table", mlog.Err(err)) return nil, err } diff --git a/i18n/en.json b/i18n/en.json index ba1abd6825..3c8b425ab3 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -2938,6 +2938,10 @@ "id": "app.channel.create_channel.no_team_id.app_error", "translation": "Must specify the team ID to create a channel." }, + { + "id": "app.channel.create_direct_channel.internal_error", + "translation": "Unable to save direct channel." + }, { "id": "app.channel.move_channel.members_do_not_match.error", "translation": "Unable to move a channel unless all its members are already members of the destination team." @@ -6062,26 +6066,10 @@ "id": "store.sql_channel.save_channel.limit.app_error", "translation": "You've reached the limit of the number of allowed channels." }, - { - "id": "store.sql_channel.save_direct_channel.add_members.app_error", - "translation": "Unable to add direct channel members." - }, - { - "id": "store.sql_channel.save_direct_channel.commit.app_error", - "translation": "Unable to commit transaction." - }, - { - "id": "store.sql_channel.save_direct_channel.internal_error", - "translation": "Unable to save direct channel." - }, { "id": "store.sql_channel.save_direct_channel.not_direct.app_error", "translation": "Not a direct channel attempted to be created with SaveDirectChannel." }, - { - "id": "store.sql_channel.save_direct_channel.open_transaction.app_error", - "translation": "Unable to open transaction." - }, { "id": "store.sql_channel.save_member.commit_transaction.app_error", "translation": "Unable to commit transaction." @@ -6094,26 +6082,6 @@ "id": "store.sql_channel.save_member.open_transaction.app_error", "translation": "Unable to open transaction." }, - { - "id": "store.sql_channel.save_member.save.app_error", - "translation": "Unable to save the channel member." - }, - { - "id": "store.sql_channel.save_multimple_members.channel_roles.app_error", - "translation": "Unable to build query to get the channel roles." - }, - { - "id": "store.sql_channel.save_multimple_members.channel_roles_query.app_error", - "translation": "Error fetching the channel roles." - }, - { - "id": "store.sql_channel.save_multimple_members.team_roles.app_error", - "translation": "Unable to build query to get the team roles." - }, - { - "id": "store.sql_channel.save_multimple_members.team_roles_query.app_error", - "translation": "Error fetching the team roles." - }, { "id": "store.sql_channel.search.app_error", "translation": "We encountered an error searching channels." diff --git a/store/opentracing_layer.go b/store/opentracing_layer.go index 6a8e0ba636..a6de601b57 100644 --- a/store/opentracing_layer.go +++ b/store/opentracing_layer.go @@ -594,7 +594,7 @@ func (s *OpenTracingLayerChannelStore) CountPostsAfter(channelId string, timesta return resultVar0, resultVar1 } -func (s *OpenTracingLayerChannelStore) CreateDirectChannel(userId *model.User, otherUserId *model.User) (*model.Channel, *model.AppError) { +func (s *OpenTracingLayerChannelStore) CreateDirectChannel(userId *model.User, otherUserId *model.User) (*model.Channel, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.CreateDirectChannel") s.Root.Store.SetContext(newCtx) @@ -1683,7 +1683,7 @@ func (s *OpenTracingLayerChannelStore) Save(channel *model.Channel, maxChannelsP return resultVar0, resultVar1 } -func (s *OpenTracingLayerChannelStore) SaveDirectChannel(channel *model.Channel, member1 *model.ChannelMember, member2 *model.ChannelMember) (*model.Channel, *model.AppError) { +func (s *OpenTracingLayerChannelStore) SaveDirectChannel(channel *model.Channel, member1 *model.ChannelMember, member2 *model.ChannelMember) (*model.Channel, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.SaveDirectChannel") s.Root.Store.SetContext(newCtx) diff --git a/store/searchlayer/channel_layer.go b/store/searchlayer/channel_layer.go index 4c4b67f0a2..bf1c2b924f 100644 --- a/store/searchlayer/channel_layer.go +++ b/store/searchlayer/channel_layer.go @@ -97,7 +97,7 @@ func (c *SearchChannelStore) RemoveMember(channelId, userIdToRemove string) *mod return err } -func (c *SearchChannelStore) CreateDirectChannel(user *model.User, otherUser *model.User) (*model.Channel, *model.AppError) { +func (c *SearchChannelStore) CreateDirectChannel(user *model.User, otherUser *model.User) (*model.Channel, error) { channel, err := c.ChannelStore.CreateDirectChannel(user, otherUser) if err == nil { c.rootStore.indexUserFromID(user.Id) diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 0bc6e86210..65518b235e 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -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, <Err) { - 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 diff --git a/store/store.go b/store/store.go index 5fe1ab85b4..b35f7fa2b3 100644 --- a/store/store.go +++ b/store/store.go @@ -128,8 +128,8 @@ type TeamStore interface { type ChannelStore interface { Save(channel *model.Channel, maxChannelsPerTeam int64) (*model.Channel, error) - CreateDirectChannel(userId *model.User, otherUserId *model.User) (*model.Channel, *model.AppError) - SaveDirectChannel(channel *model.Channel, member1 *model.ChannelMember, member2 *model.ChannelMember) (*model.Channel, *model.AppError) + CreateDirectChannel(userId *model.User, otherUserId *model.User) (*model.Channel, error) + SaveDirectChannel(channel *model.Channel, member1 *model.ChannelMember, member2 *model.ChannelMember) (*model.Channel, error) Update(channel *model.Channel) (*model.Channel, *model.AppError) Get(id string, allowFromCache bool) (*model.Channel, *model.AppError) InvalidateChannel(id string) diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index 938c7ca372..a8ab1cf5fb 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -185,15 +185,15 @@ func testChannelStoreSaveDirectChannel(t *testing.T, ss store.Store, s SqlSuppli m2.UserId = u2.Id m2.NotifyProps = model.GetDefaultChannelNotifyProps() - _, err = ss.Channel().SaveDirectChannel(&o1, &m1, &m2) - require.Nil(t, err, "couldn't save direct channel", err) + _, nErr := ss.Channel().SaveDirectChannel(&o1, &m1, &m2) + require.Nil(t, nErr, "couldn't save direct channel", nErr) members, err := ss.Channel().GetMembers(o1.Id, 0, 100) require.Nil(t, err) require.Len(t, *members, 2, "should have saved 2 members") - _, err = ss.Channel().SaveDirectChannel(&o1, &m1, &m2) - require.NotNil(t, err, "shoudn't be a able to update from save") + _, nErr = ss.Channel().SaveDirectChannel(&o1, &m1, &m2) + require.NotNil(t, nErr, "shoudn't be a able to update from save") // Attempt to save a direct channel that already exists o1a := model.Channel{ @@ -203,25 +203,26 @@ func testChannelStoreSaveDirectChannel(t *testing.T, ss store.Store, s SqlSuppli Type: o1.Type, } - returnedChannel, err := ss.Channel().SaveDirectChannel(&o1a, &m1, &m2) - require.NotNil(t, err, "should've failed to save a duplicate direct channel") - require.Equal(t, store.CHANNEL_EXISTS_ERROR, err.Id, "should've returned CHANNEL_EXISTS_ERROR") + returnedChannel, nErr := ss.Channel().SaveDirectChannel(&o1a, &m1, &m2) + require.NotNil(t, nErr, "should've failed to save a duplicate direct channel") + var cErr *store.ErrConflict + require.Truef(t, errors.As(nErr, &cErr), "should've returned CHANNEL_EXISTS_ERROR") require.Equal(t, o1.Id, returnedChannel.Id, "should've failed to save a duplicate direct channel") // Attempt to save a non-direct channel o1.Id = "" o1.Name = "zz" + model.NewId() + "b" o1.Type = model.CHANNEL_OPEN - _, err = ss.Channel().SaveDirectChannel(&o1, &m1, &m2) - require.NotNil(t, err, "Should not be able to save non-direct channel") + _, nErr = ss.Channel().SaveDirectChannel(&o1, &m1, &m2) + require.NotNil(t, nErr, "Should not be able to save non-direct channel") // Save yourself Direct Message o1.Id = "" o1.DisplayName = "Myself" o1.Name = "zz" + model.NewId() + "b" o1.Type = model.CHANNEL_DIRECT - _, err = ss.Channel().SaveDirectChannel(&o1, &m1, &m1) - require.Nil(t, err, "couldn't save direct channel", err) + _, nErr = ss.Channel().SaveDirectChannel(&o1, &m1, &m1) + require.Nil(t, nErr, "couldn't save direct channel", nErr) members, err = ss.Channel().GetMembers(o1.Id, 0, 100) require.Nil(t, err) @@ -248,8 +249,8 @@ func testChannelStoreCreateDirectChannel(t *testing.T, ss store.Store) { _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: model.NewId(), UserId: u2.Id}, -1) require.Nil(t, err) - c1, err := ss.Channel().CreateDirectChannel(u1, u2) - require.Nil(t, err, "couldn't create direct channel", err) + c1, nErr := ss.Channel().CreateDirectChannel(u1, u2) + require.Nil(t, nErr, "couldn't create direct channel", nErr) defer func() { ss.Channel().PermanentDeleteMembersByChannel(c1.Id) ss.Channel().PermanentDelete(c1.Id) @@ -402,8 +403,8 @@ func testChannelStoreGet(t *testing.T, ss store.Store, s SqlSupplier) { m2.UserId = u2.Id m2.NotifyProps = model.GetDefaultChannelNotifyProps() - _, err = ss.Channel().SaveDirectChannel(&o2, &m1, &m2) - require.Nil(t, err) + _, nErr = ss.Channel().SaveDirectChannel(&o2, &m1, &m2) + require.Nil(t, nErr) c2, err := ss.Channel().Get(o2.Id, false) require.Nil(t, err, err) @@ -478,8 +479,8 @@ func testChannelStoreGetChannelsByIds(t *testing.T, ss store.Store) { m2.UserId = u2.Id m2.NotifyProps = model.GetDefaultChannelNotifyProps() - _, err = ss.Channel().SaveDirectChannel(&o2, &m1, &m2) - require.Nil(t, err) + _, nErr = ss.Channel().SaveDirectChannel(&o2, &m1, &m2) + require.Nil(t, nErr) t.Run("Get 2 existing channels", func(t *testing.T) { r1, err := ss.Channel().GetChannelsByIds([]string{o1.Id, o2.Id}, false) @@ -3235,8 +3236,8 @@ func testChannelStoreGetAllChannels(t *testing.T, ss store.Store, s SqlSupplier) u1 := model.User{Id: model.NewId()} u2 := model.User{Id: model.NewId()} - _, err = ss.Channel().CreateDirectChannel(&u1, &u2) - require.Nil(t, err) + _, nErr = ss.Channel().CreateDirectChannel(&u1, &u2) + require.Nil(t, nErr) userIds := []string{model.NewId(), model.NewId(), model.NewId()} @@ -3711,13 +3712,13 @@ func testChannelStoreGetMembersForUser(t *testing.T, ss store.Store) { u2 := model.User{Id: model.NewId()} u3 := model.User{Id: model.NewId()} u4 := model.User{Id: model.NewId()} - _, err = ss.Channel().CreateDirectChannel(&u1, &user) - require.Nil(t, err) - _, err = ss.Channel().CreateDirectChannel(&u2, &user) - require.Nil(t, err) + _, nErr = ss.Channel().CreateDirectChannel(&u1, &user) + require.Nil(t, nErr) + _, nErr = ss.Channel().CreateDirectChannel(&u2, &user) + require.Nil(t, nErr) // other user direct message - _, err = ss.Channel().CreateDirectChannel(&u3, &u4) - require.Nil(t, err) + _, nErr = ss.Channel().CreateDirectChannel(&u3, &u4) + require.Nil(t, nErr) var members *model.ChannelMembers members, err = ss.Channel().GetMembersForUser(o1.TeamId, m1.UserId) @@ -5502,8 +5503,8 @@ func testChannelStoreAnalyticsDeletedTypeCount(t *testing.T, ss store.Store) { _, err = ss.User().Save(u2) require.Nil(t, err) - d4, err := ss.Channel().CreateDirectChannel(u1, u2) - require.Nil(t, err) + d4, nErr := ss.Channel().CreateDirectChannel(u1, u2) + require.Nil(t, nErr) defer func() { ss.Channel().PermanentDeleteMembersByChannel(d4.Id) ss.Channel().PermanentDelete(d4.Id) diff --git a/store/storetest/compliance_store.go b/store/storetest/compliance_store.go index 8de08ff8d6..ccc278e96d 100644 --- a/store/storetest/compliance_store.go +++ b/store/storetest/compliance_store.go @@ -239,8 +239,8 @@ func testComplianceExportDirectMessages(t *testing.T, ss store.Store) { c1, nErr := ss.Channel().Save(c1, -1) require.Nil(t, nErr) - cDM, err := ss.Channel().CreateDirectChannel(u1, u2) - require.Nil(t, err) + cDM, nErr := ss.Channel().CreateDirectChannel(u1, u2) + require.Nil(t, nErr) o1 := &model.Post{} o1.ChannelId = c1.Id o1.UserId = u1.Id @@ -546,8 +546,8 @@ func testMessageExportDirectMessageChannel(t *testing.T, ss store.Store) { require.Nil(t, err) // as well as a DM channel between those users - directMessageChannel, err := ss.Channel().CreateDirectChannel(user1, user2) - require.Nil(t, err) + directMessageChannel, nErr := ss.Channel().CreateDirectChannel(user1, user2) + require.Nil(t, nErr) // user1 also sends a DM to user2 post := &model.Post{ diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index 04a4eaa754..40675cc5c4 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -156,7 +156,7 @@ func (_m *ChannelStore) CountPostsAfter(channelId string, timestamp int64, userI } // CreateDirectChannel provides a mock function with given fields: userId, otherUserId -func (_m *ChannelStore) CreateDirectChannel(userId *model.User, otherUserId *model.User) (*model.Channel, *model.AppError) { +func (_m *ChannelStore) CreateDirectChannel(userId *model.User, otherUserId *model.User) (*model.Channel, error) { ret := _m.Called(userId, otherUserId) var r0 *model.Channel @@ -168,13 +168,11 @@ func (_m *ChannelStore) CreateDirectChannel(userId *model.User, otherUserId *mod } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(*model.User, *model.User) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(*model.User, *model.User) error); ok { r1 = rf(userId, otherUserId) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 @@ -1447,7 +1445,7 @@ func (_m *ChannelStore) Save(channel *model.Channel, maxChannelsPerTeam int64) ( } // SaveDirectChannel provides a mock function with given fields: channel, member1, member2 -func (_m *ChannelStore) SaveDirectChannel(channel *model.Channel, member1 *model.ChannelMember, member2 *model.ChannelMember) (*model.Channel, *model.AppError) { +func (_m *ChannelStore) SaveDirectChannel(channel *model.Channel, member1 *model.ChannelMember, member2 *model.ChannelMember) (*model.Channel, error) { ret := _m.Called(channel, member1, member2) var r0 *model.Channel @@ -1459,13 +1457,11 @@ func (_m *ChannelStore) SaveDirectChannel(channel *model.Channel, member1 *model } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(*model.Channel, *model.ChannelMember, *model.ChannelMember) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(*model.Channel, *model.ChannelMember, *model.ChannelMember) error); ok { r1 = rf(channel, member1, member2) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index c857fa5834..1e37a4e0ef 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -1971,8 +1971,8 @@ func testUserUnreadCount(t *testing.T, ss store.Store) { m1.ChannelId = c2.Id m2.ChannelId = c2.Id - _, err = ss.Channel().SaveDirectChannel(&c2, &m1, &m2) - require.Nil(t, err, "couldn't save direct channel") + _, nErr = ss.Channel().SaveDirectChannel(&c2, &m1, &m2) + require.Nil(t, nErr, "couldn't save direct channel") p1 := model.Post{} p1.ChannelId = c1.Id diff --git a/store/timer_layer.go b/store/timer_layer.go index 57790b201d..f4f1773170 100644 --- a/store/timer_layer.go +++ b/store/timer_layer.go @@ -568,7 +568,7 @@ func (s *TimerLayerChannelStore) CountPostsAfter(channelId string, timestamp int return resultVar0, resultVar1 } -func (s *TimerLayerChannelStore) CreateDirectChannel(userId *model.User, otherUserId *model.User) (*model.Channel, *model.AppError) { +func (s *TimerLayerChannelStore) CreateDirectChannel(userId *model.User, otherUserId *model.User) (*model.Channel, error) { start := timemodule.Now() resultVar0, resultVar1 := s.ChannelStore.CreateDirectChannel(userId, otherUserId) @@ -1569,7 +1569,7 @@ func (s *TimerLayerChannelStore) Save(channel *model.Channel, maxChannelsPerTeam return resultVar0, resultVar1 } -func (s *TimerLayerChannelStore) SaveDirectChannel(channel *model.Channel, member1 *model.ChannelMember, member2 *model.ChannelMember) (*model.Channel, *model.AppError) { +func (s *TimerLayerChannelStore) SaveDirectChannel(channel *model.Channel, member1 *model.ChannelMember, member2 *model.ChannelMember) (*model.Channel, error) { start := timemodule.Now() resultVar0, resultVar1 := s.ChannelStore.SaveDirectChannel(channel, member1, member2)