From 86290685aee0d39c512e69e1714dff73b6d39963 Mon Sep 17 00:00:00 2001 From: Rodrigo Villablanca Date: Tue, 4 Aug 2020 10:37:21 -0400 Subject: [PATCH] RoleStore migration (#15017) Automatic Merge --- app/import_functions_test.go | 120 ++++++++++----------- app/migrations.go | 4 +- app/permissions.go | 2 +- app/permissions_migrations.go | 11 +- app/role.go | 93 ++++++++++++---- i18n/en.json | 72 +++++-------- store/localcachelayer/role_layer.go | 12 +-- store/opentracinglayer/opentracinglayer.go | 20 ++-- store/sqlstore/role_store.go | 87 +++++++-------- store/store.go | 20 ++-- store/storetest/mocks/RoleStore.go | 100 +++++++---------- store/storetest/scheme_store.go | 2 +- store/timerlayer/timerlayer.go | 20 ++-- 13 files changed, 289 insertions(+), 274 deletions(-) diff --git a/app/import_functions_test.go b/app/import_functions_test.go index 39885a43d8..3f8a7c7f8a 100644 --- a/app/import_functions_test.go +++ b/app/import_functions_test.go @@ -98,43 +98,43 @@ func TestImportImportScheme(t *testing.T) { assert.Equal(t, *data.Description, scheme.Description) assert.Equal(t, *data.Scope, scheme.Scope) - role, err := th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamAdminRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr := th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamAdminRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultTeamAdminRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamUserRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamUserRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultTeamUserRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamGuestRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamGuestRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultTeamGuestRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelAdminRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelAdminRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultChannelAdminRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelUserRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelUserRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultChannelUserRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelGuestRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelGuestRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultChannelGuestRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) @@ -155,43 +155,43 @@ func TestImportImportScheme(t *testing.T) { assert.Equal(t, *data.Description, scheme.Description) assert.Equal(t, *data.Scope, scheme.Scope) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamAdminRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamAdminRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultTeamAdminRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamUserRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamUserRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultTeamUserRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamGuestRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamGuestRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultTeamGuestRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelAdminRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelAdminRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultChannelAdminRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelUserRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelUserRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultChannelUserRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelGuestRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelGuestRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultChannelGuestRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) @@ -285,43 +285,43 @@ func TestImportImportSchemeWithoutGuestRoles(t *testing.T) { assert.Equal(t, *data.Description, scheme.Description) assert.Equal(t, *data.Scope, scheme.Scope) - role, err := th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamAdminRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr := th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamAdminRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultTeamAdminRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamUserRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamUserRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultTeamUserRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamGuestRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamGuestRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultTeamGuestRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelAdminRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelAdminRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultChannelAdminRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelUserRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelUserRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultChannelUserRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelGuestRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelGuestRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultChannelGuestRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) @@ -342,43 +342,43 @@ func TestImportImportSchemeWithoutGuestRoles(t *testing.T) { assert.Equal(t, *data.Description, scheme.Description) assert.Equal(t, *data.Scope, scheme.Scope) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamAdminRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamAdminRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultTeamAdminRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamUserRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamUserRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultTeamUserRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamGuestRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamGuestRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultTeamGuestRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelAdminRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelAdminRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultChannelAdminRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelUserRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelUserRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultChannelUserRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) assert.True(t, role.SchemeManaged) - role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelGuestRole) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelGuestRole) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.DefaultChannelGuestRole.DisplayName, role.DisplayName) assert.False(t, role.BuiltIn) @@ -412,8 +412,8 @@ func TestImportImportRole(t *testing.T) { err := th.App.importRole(&data, true, false) require.NotNil(t, err, "Should have failed to import.") - _, err = th.App.Srv().Store.Role().GetByName(rid1) - require.NotNil(t, err, "Should have failed to import.") + _, nErr := th.App.Srv().Store.Role().GetByName(rid1) + require.NotNil(t, nErr, "Should have failed to import.") // Try importing the valid role in dryRun mode. data.DisplayName = ptrStr("display name") @@ -421,8 +421,8 @@ func TestImportImportRole(t *testing.T) { err = th.App.importRole(&data, true, false) require.Nil(t, err, "Should have succeeded.") - _, err = th.App.Srv().Store.Role().GetByName(rid1) - require.NotNil(t, err, "Role should not have imported as we are in dry run mode.") + _, nErr = th.App.Srv().Store.Role().GetByName(rid1) + require.NotNil(t, nErr, "Role should not have imported as we are in dry run mode.") // Try importing an invalid role. data.DisplayName = nil @@ -430,8 +430,8 @@ func TestImportImportRole(t *testing.T) { err = th.App.importRole(&data, false, false) require.NotNil(t, err, "Should have failed to import.") - _, err = th.App.Srv().Store.Role().GetByName(rid1) - require.NotNil(t, err, "Role should not have imported.") + _, nErr = th.App.Srv().Store.Role().GetByName(rid1) + require.NotNil(t, nErr, "Role should not have imported.") // Try importing a valid role with all params set. data.DisplayName = ptrStr("display name") @@ -441,8 +441,8 @@ func TestImportImportRole(t *testing.T) { err = th.App.importRole(&data, false, false) require.Nil(t, err, "Should have succeeded.") - role, err := th.App.Srv().Store.Role().GetByName(rid1) - require.Nil(t, err, "Should have found the imported role.") + role, nErr := th.App.Srv().Store.Role().GetByName(rid1) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.Name, role.Name) assert.Equal(t, *data.DisplayName, role.DisplayName) @@ -459,8 +459,8 @@ func TestImportImportRole(t *testing.T) { err = th.App.importRole(&data, false, true) require.Nil(t, err, "Should have succeeded. %v", err) - role, err = th.App.Srv().Store.Role().GetByName(rid1) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(rid1) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data.Name, role.Name) assert.Equal(t, *data.DisplayName, role.DisplayName) @@ -478,8 +478,8 @@ func TestImportImportRole(t *testing.T) { err = th.App.importRole(&data2, false, false) require.Nil(t, err, "Should have succeeded.") - role, err = th.App.Srv().Store.Role().GetByName(rid1) - require.Nil(t, err, "Should have found the imported role.") + role, nErr = th.App.Srv().Store.Role().GetByName(rid1) + require.Nil(t, nErr, "Should have found the imported role.") assert.Equal(t, *data2.Name, role.Name) assert.Equal(t, *data2.DisplayName, role.DisplayName) diff --git a/app/migrations.go b/app/migrations.go index 7f1abc6061..501a0241ef 100644 --- a/app/migrations.go +++ b/app/migrations.go @@ -122,8 +122,8 @@ func (a *App) DoEmojisPermissionsMigration() { if role != nil { role.Permissions = append(role.Permissions, model.PERMISSION_CREATE_EMOJIS.Id, model.PERMISSION_DELETE_EMOJIS.Id) - if _, err = a.Srv().Store.Role().Save(role); err != nil { - mlog.Critical("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(err)) + if _, nErr := a.Srv().Store.Role().Save(role); nErr != nil { + mlog.Critical("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(nErr)) return } } diff --git a/app/permissions.go b/app/permissions.go index 59c65c91a0..d0281c6bef 100644 --- a/app/permissions.go +++ b/app/permissions.go @@ -50,7 +50,7 @@ func (a *App) ResetPermissionsSystem() *model.AppError { // Purge all roles from the database. if err := a.Srv().Store.Role().PermanentDeleteAll(); err != nil { - return err + return model.NewAppError("ResetPermissionsSystem", "app.role.permanent_delete_all.app_error", nil, err.Error(), http.StatusInternalServerError) } // Remove the "System" table entry that marks the advanced permissions migration as done. diff --git a/app/permissions_migrations.go b/app/permissions_migrations.go index 2ed2c5b120..99b76edbd2 100644 --- a/app/permissions_migrations.go +++ b/app/permissions_migrations.go @@ -4,9 +4,12 @@ package app import ( + "errors" + "net/http" "strings" "github.com/mattermost/mattermost-server/v5/model" + "github.com/mattermost/mattermost-server/v5/store" ) type permissionTransformation struct { @@ -162,7 +165,13 @@ func (a *App) doPermissionsMigration(key string, migrationMap permissionsMap) *m for _, role := range roles { role.Permissions = applyPermissionsMap(role, roleMap, migrationMap) if _, err := a.Srv().Store.Role().Save(role); err != nil { - return err + var invErr *store.ErrInvalidInput + switch { + case errors.As(err, &invErr): + return model.NewAppError("doPermissionsMigration", "app.role.save.invalid_role.app_error", nil, invErr.Error(), http.StatusBadRequest) + default: + return model.NewAppError("doPermissionsMigration", "app.role.save.insert.app_error", nil, err.Error(), http.StatusInternalServerError) + } } } diff --git a/app/role.go b/app/role.go index 2de152be77..2492cce9f8 100644 --- a/app/role.go +++ b/app/role.go @@ -4,29 +4,53 @@ package app import ( + "errors" "net/http" "reflect" "strings" "github.com/mattermost/mattermost-server/v5/model" + "github.com/mattermost/mattermost-server/v5/store" "github.com/mattermost/mattermost-server/v5/utils" ) func (a *App) GetRole(id string) (*model.Role, *model.AppError) { - return a.Srv().Store.Role().Get(id) + role, err := a.Srv().Store.Role().Get(id) + if err != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(err, &nfErr): + return nil, model.NewAppError("GetRole", "app.role.get.app_error", nil, nfErr.Error(), http.StatusNotFound) + default: + return nil, model.NewAppError("GetRole", "app.role.get.app_error", nil, err.Error(), http.StatusInternalServerError) + } + } + + return role, nil } func (a *App) GetAllRoles() ([]*model.Role, *model.AppError) { - return a.Srv().Store.Role().GetAll() + roles, err := a.Srv().Store.Role().GetAll() + if err != nil { + return nil, model.NewAppError("GetAllRoles", "app.role.get_all.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + return roles, nil } func (s *Server) GetRoleByName(name string) (*model.Role, *model.AppError) { - role, err := s.Store.Role().GetByName(name) - if err != nil { - return nil, err + role, nErr := s.Store.Role().GetByName(name) + if nErr != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(nErr, &nfErr): + return nil, model.NewAppError("GetRoleByName", "app.role.get_by_name.app_error", nil, nfErr.Error(), http.StatusNotFound) + default: + return nil, model.NewAppError("GetRoleByName", "app.role.get_by_name.app_error", nil, nErr.Error(), http.StatusInternalServerError) + } } - err = s.mergeChannelHigherScopedPermissions([]*model.Role{role}) + err := s.mergeChannelHigherScopedPermissions([]*model.Role{role}) if err != nil { return nil, err } @@ -39,12 +63,12 @@ func (a *App) GetRoleByName(name string) (*model.Role, *model.AppError) { } func (a *App) GetRolesByNames(names []string) ([]*model.Role, *model.AppError) { - roles, err := a.Srv().Store.Role().GetByNames(names) - if err != nil { - return nil, err + roles, nErr := a.Srv().Store.Role().GetByNames(names) + if nErr != nil { + return nil, model.NewAppError("GetRolesByNames", "app.role.get_by_names.app_error", nil, nErr.Error(), http.StatusInternalServerError) } - err = a.mergeChannelHigherScopedPermissions(roles) + err := a.mergeChannelHigherScopedPermissions(roles) if err != nil { return nil, err } @@ -69,7 +93,7 @@ func (s *Server) mergeChannelHigherScopedPermissions(roles []*model.Role) *model higherScopedPermissionsMap, err := s.Store.Role().ChannelHigherScopedPermissions(higherScopeNamesToQuery) if err != nil { - return err + return model.NewAppError("mergeChannelHigherScopedPermissions", "app.role.get_by_names.app_error", nil, err.Error(), http.StatusInternalServerError) } for _, role := range roles { @@ -112,14 +136,31 @@ func (a *App) CreateRole(role *model.Role) (*model.Role, *model.AppError) { role.BuiltIn = false role.SchemeManaged = false - return a.Srv().Store.Role().Save(role) + var err error + role, err = a.Srv().Store.Role().Save(role) + if err != nil { + var invErr *store.ErrInvalidInput + switch { + case errors.As(err, &invErr): + return nil, model.NewAppError("CreateRole", "app.role.save.invalid_role.app_error", nil, invErr.Error(), http.StatusBadRequest) + default: + return nil, model.NewAppError("CreateRole", "app.role.save.insert.app_error", nil, err.Error(), http.StatusInternalServerError) + } + } + return role, nil } func (a *App) UpdateRole(role *model.Role) (*model.Role, *model.AppError) { savedRole, err := a.Srv().Store.Role().Save(role) if err != nil { - return nil, err + var invErr *store.ErrInvalidInput + switch { + case errors.As(err, &invErr): + return nil, model.NewAppError("UpdateRole", "app.role.save.invalid_role.app_error", nil, invErr.Error(), http.StatusBadRequest) + default: + return nil, model.NewAppError("UpdateRole", "app.role.save.insert.app_error", nil, err.Error(), http.StatusInternalServerError) + } } builtInChannelRoles := []string{ @@ -138,23 +179,33 @@ func (a *App) UpdateRole(role *model.Role) (*model.Role, *model.AppError) { if utils.StringInSlice(savedRole.Name, builtInChannelRoles) { roleRetrievalFunc = func() ([]*model.Role, *model.AppError) { - return a.Srv().Store.Role().AllChannelSchemeRoles() + roles, nErr := a.Srv().Store.Role().AllChannelSchemeRoles() + if nErr != nil { + return nil, model.NewAppError("UpdateRole", "app.role.get.app_error", nil, nErr.Error(), http.StatusInternalServerError) + } + + return roles, nil } } else { roleRetrievalFunc = func() ([]*model.Role, *model.AppError) { - return a.Srv().Store.Role().ChannelRolesUnderTeamRole(savedRole.Name) + roles, nErr := a.Srv().Store.Role().ChannelRolesUnderTeamRole(savedRole.Name) + if nErr != nil { + return nil, model.NewAppError("UpdateRole", "app.role.get.app_error", nil, nErr.Error(), http.StatusInternalServerError) + } + + return roles, nil } } - impactedRoles, err := roleRetrievalFunc() - if err != nil { - return nil, err + impactedRoles, appErr := roleRetrievalFunc() + if appErr != nil { + return nil, appErr } impactedRoles = append(impactedRoles, role) - err = a.mergeChannelHigherScopedPermissions(impactedRoles) - if err != nil { - return nil, err + appErr = a.mergeChannelHigherScopedPermissions(impactedRoles) + if appErr != nil { + return nil, appErr } for _, ir := range impactedRoles { diff --git a/i18n/en.json b/i18n/en.json index 110ee4365d..74b0a4bad6 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -4298,6 +4298,34 @@ "id": "app.role.check_roles_exist.role_not_found", "translation": "The provided role does not exist" }, + { + "id": "app.role.get.app_error", + "translation": "Unable to get role." + }, + { + "id": "app.role.get_all.app_error", + "translation": "Unable to get all the roles." + }, + { + "id": "app.role.get_by_name.app_error", + "translation": "Unable to get role." + }, + { + "id": "app.role.get_by_names.app_error", + "translation": "Unable to get roles." + }, + { + "id": "app.role.permanent_delete_all.app_error", + "translation": "We could not permanently delete all the roles." + }, + { + "id": "app.role.save.insert.app_error", + "translation": "Unable to save new role." + }, + { + "id": "app.role.save.invalid_role.app_error", + "translation": "The role was not valid." + }, { "id": "app.save_config.app_error", "translation": "An error occurred saving the configuration." @@ -7258,50 +7286,6 @@ "id": "store.sql_post.update.app_error", "translation": "Unable to update the Post." }, - { - "id": "store.sql_role.delete.update.app_error", - "translation": "Unable to delete the role." - }, - { - "id": "store.sql_role.get.app_error", - "translation": "Unable to get role." - }, - { - "id": "store.sql_role.get_all.app_error", - "translation": "Unable to get all the roles." - }, - { - "id": "store.sql_role.get_by_name.app_error", - "translation": "Unable to get role." - }, - { - "id": "store.sql_role.get_by_names.app_error", - "translation": "Unable to get roles." - }, - { - "id": "store.sql_role.permanent_delete_all.app_error", - "translation": "We could not permanently delete all the roles." - }, - { - "id": "store.sql_role.save.insert.app_error", - "translation": "Unable to save new role." - }, - { - "id": "store.sql_role.save.invalid_role.app_error", - "translation": "The role was not valid." - }, - { - "id": "store.sql_role.save.open_transaction.app_error", - "translation": "Failed to open the transaction to save the role." - }, - { - "id": "store.sql_role.save.update.app_error", - "translation": "Unable to update role." - }, - { - "id": "store.sql_role.save_role.commit_transaction.app_error", - "translation": "Failed to commit the transaction to save the role." - }, { "id": "store.sql_status.get.app_error", "translation": "Encountered an error retrieving the status." diff --git a/store/localcachelayer/role_layer.go b/store/localcachelayer/role_layer.go index f19d59a95e..2c624d1a8d 100644 --- a/store/localcachelayer/role_layer.go +++ b/store/localcachelayer/role_layer.go @@ -32,7 +32,7 @@ func (s *LocalCacheRoleStore) handleClusterInvalidateRolePermissions(msg *model. } } -func (s LocalCacheRoleStore) Save(role *model.Role) (*model.Role, *model.AppError) { +func (s LocalCacheRoleStore) Save(role *model.Role) (*model.Role, error) { if len(role.Name) != 0 { defer s.rootStore.doInvalidateCacheCluster(s.rootStore.roleCache, role.Name) defer s.rootStore.doClearCacheCluster(s.rootStore.rolePermissionsCache) @@ -40,7 +40,7 @@ func (s LocalCacheRoleStore) Save(role *model.Role) (*model.Role, *model.AppErro return s.RoleStore.Save(role) } -func (s LocalCacheRoleStore) GetByName(name string) (*model.Role, *model.AppError) { +func (s LocalCacheRoleStore) GetByName(name string) (*model.Role, error) { var role *model.Role if err := s.rootStore.doStandardReadCache(s.rootStore.roleCache, name, &role); err == nil { return role, nil @@ -54,7 +54,7 @@ func (s LocalCacheRoleStore) GetByName(name string) (*model.Role, *model.AppErro return role, nil } -func (s LocalCacheRoleStore) GetByNames(names []string) ([]*model.Role, *model.AppError) { +func (s LocalCacheRoleStore) GetByNames(names []string) ([]*model.Role, error) { var foundRoles []*model.Role var rolesToQuery []string @@ -76,7 +76,7 @@ func (s LocalCacheRoleStore) GetByNames(names []string) ([]*model.Role, *model.A return append(foundRoles, roles...), nil } -func (s LocalCacheRoleStore) Delete(roleId string) (*model.Role, *model.AppError) { +func (s LocalCacheRoleStore) Delete(roleId string) (*model.Role, error) { role, err := s.RoleStore.Delete(roleId) if err == nil { @@ -86,7 +86,7 @@ func (s LocalCacheRoleStore) Delete(roleId string) (*model.Role, *model.AppError return role, err } -func (s LocalCacheRoleStore) PermanentDeleteAll() *model.AppError { +func (s LocalCacheRoleStore) PermanentDeleteAll() error { defer s.rootStore.roleCache.Purge() defer s.rootStore.doClearCacheCluster(s.rootStore.roleCache) defer s.rootStore.doClearCacheCluster(s.rootStore.rolePermissionsCache) @@ -94,7 +94,7 @@ func (s LocalCacheRoleStore) PermanentDeleteAll() *model.AppError { return s.RoleStore.PermanentDeleteAll() } -func (s LocalCacheRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, *model.AppError) { +func (s LocalCacheRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, error) { sort.Strings(roleNames) cacheKey := strings.Join(roleNames, "/") var rolePermissionsMap map[string]*model.RolePermissions diff --git a/store/opentracinglayer/opentracinglayer.go b/store/opentracinglayer/opentracinglayer.go index 704619a6ad..989247483e 100644 --- a/store/opentracinglayer/opentracinglayer.go +++ b/store/opentracinglayer/opentracinglayer.go @@ -5617,7 +5617,7 @@ func (s *OpenTracingLayerReactionStore) Save(reaction *model.Reaction) (*model.R return resultVar0, resultVar1 } -func (s *OpenTracingLayerRoleStore) AllChannelSchemeRoles() ([]*model.Role, *model.AppError) { +func (s *OpenTracingLayerRoleStore) AllChannelSchemeRoles() ([]*model.Role, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.AllChannelSchemeRoles") s.Root.Store.SetContext(newCtx) @@ -5635,7 +5635,7 @@ func (s *OpenTracingLayerRoleStore) AllChannelSchemeRoles() ([]*model.Role, *mod return resultVar0, resultVar1 } -func (s *OpenTracingLayerRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, *model.AppError) { +func (s *OpenTracingLayerRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.ChannelHigherScopedPermissions") s.Root.Store.SetContext(newCtx) @@ -5653,7 +5653,7 @@ func (s *OpenTracingLayerRoleStore) ChannelHigherScopedPermissions(roleNames []s return resultVar0, resultVar1 } -func (s *OpenTracingLayerRoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, *model.AppError) { +func (s *OpenTracingLayerRoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.ChannelRolesUnderTeamRole") s.Root.Store.SetContext(newCtx) @@ -5671,7 +5671,7 @@ func (s *OpenTracingLayerRoleStore) ChannelRolesUnderTeamRole(roleName string) ( return resultVar0, resultVar1 } -func (s *OpenTracingLayerRoleStore) Delete(roleId string) (*model.Role, *model.AppError) { +func (s *OpenTracingLayerRoleStore) Delete(roleId string) (*model.Role, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.Delete") s.Root.Store.SetContext(newCtx) @@ -5689,7 +5689,7 @@ func (s *OpenTracingLayerRoleStore) Delete(roleId string) (*model.Role, *model.A return resultVar0, resultVar1 } -func (s *OpenTracingLayerRoleStore) Get(roleId string) (*model.Role, *model.AppError) { +func (s *OpenTracingLayerRoleStore) Get(roleId string) (*model.Role, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.Get") s.Root.Store.SetContext(newCtx) @@ -5707,7 +5707,7 @@ func (s *OpenTracingLayerRoleStore) Get(roleId string) (*model.Role, *model.AppE return resultVar0, resultVar1 } -func (s *OpenTracingLayerRoleStore) GetAll() ([]*model.Role, *model.AppError) { +func (s *OpenTracingLayerRoleStore) GetAll() ([]*model.Role, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.GetAll") s.Root.Store.SetContext(newCtx) @@ -5725,7 +5725,7 @@ func (s *OpenTracingLayerRoleStore) GetAll() ([]*model.Role, *model.AppError) { return resultVar0, resultVar1 } -func (s *OpenTracingLayerRoleStore) GetByName(name string) (*model.Role, *model.AppError) { +func (s *OpenTracingLayerRoleStore) GetByName(name string) (*model.Role, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.GetByName") s.Root.Store.SetContext(newCtx) @@ -5743,7 +5743,7 @@ func (s *OpenTracingLayerRoleStore) GetByName(name string) (*model.Role, *model. return resultVar0, resultVar1 } -func (s *OpenTracingLayerRoleStore) GetByNames(names []string) ([]*model.Role, *model.AppError) { +func (s *OpenTracingLayerRoleStore) GetByNames(names []string) ([]*model.Role, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.GetByNames") s.Root.Store.SetContext(newCtx) @@ -5761,7 +5761,7 @@ func (s *OpenTracingLayerRoleStore) GetByNames(names []string) ([]*model.Role, * return resultVar0, resultVar1 } -func (s *OpenTracingLayerRoleStore) PermanentDeleteAll() *model.AppError { +func (s *OpenTracingLayerRoleStore) PermanentDeleteAll() error { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.PermanentDeleteAll") s.Root.Store.SetContext(newCtx) @@ -5779,7 +5779,7 @@ func (s *OpenTracingLayerRoleStore) PermanentDeleteAll() *model.AppError { return resultVar0 } -func (s *OpenTracingLayerRoleStore) Save(role *model.Role) (*model.Role, *model.AppError) { +func (s *OpenTracingLayerRoleStore) Save(role *model.Role) (*model.Role, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.Save") s.Root.Store.SetContext(newCtx) diff --git a/store/sqlstore/role_store.go b/store/sqlstore/role_store.go index e863445694..1ca396fb9b 100644 --- a/store/sqlstore/role_store.go +++ b/store/sqlstore/role_store.go @@ -6,7 +6,6 @@ package sqlstore import ( "database/sql" "fmt" - "net/http" "strings" sq "github.com/Masterminds/squirrel" @@ -97,24 +96,24 @@ func newSqlRoleStore(sqlStore SqlStore) store.RoleStore { return s } -func (s *SqlRoleStore) Save(role *model.Role) (*model.Role, *model.AppError) { +func (s *SqlRoleStore) Save(role *model.Role) (*model.Role, error) { // Check the role is valid before proceeding. if !role.IsValidWithoutId() { - return nil, model.NewAppError("SqlRoleStore.Save", "store.sql_role.save.invalid_role.app_error", nil, "", http.StatusBadRequest) + return nil, store.NewErrInvalidInput("Role", "", fmt.Sprintf("%v", role)) } if len(role.Id) == 0 { transaction, err := s.GetMaster().Begin() if err != nil { - return nil, model.NewAppError("SqlRoleStore.RoleSave", "store.sql_role.save.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "begin_transaction") } defer finalizeTransaction(transaction) createdRole, err := s.createRole(role, transaction) if err != nil { - transaction.Rollback() - return nil, model.NewAppError("SqlRoleStore.RoleSave", "store.sql_role.save.insert.app_error", nil, err.Error(), http.StatusInternalServerError) + _ = transaction.Rollback() + return nil, errors.Wrap(err, "unable to create Role") } else if err := transaction.Commit(); err != nil { - return nil, model.NewAppError("SqlRoleStore.RoleSave", "store.sql_role.save_role.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "commit_transaction") } return createdRole, nil } @@ -122,9 +121,9 @@ func (s *SqlRoleStore) Save(role *model.Role) (*model.Role, *model.AppError) { dbRole := NewRoleFromModel(role) dbRole.UpdateAt = model.GetMillis() if rowsChanged, err := s.GetMaster().Update(dbRole); err != nil { - return nil, model.NewAppError("SqlRoleStore.Save", "store.sql_role.save.update.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "failed to update Role") } else if rowsChanged != 1 { - return nil, model.NewAppError("SqlRoleStore.Save", "store.sql_role.save.update.app_error", nil, "no record to update", http.StatusInternalServerError) + return nil, fmt.Errorf("invalid number of updated rows, expected 1 but got %d", rowsChanged) } return dbRole.ToModel(), nil @@ -149,27 +148,24 @@ func (s *SqlRoleStore) createRole(role *model.Role, transaction *gorp.Transactio return dbRole.ToModel(), nil } -func (s *SqlRoleStore) Get(roleId string) (*model.Role, *model.AppError) { +func (s *SqlRoleStore) Get(roleId string) (*model.Role, error) { var dbRole Role if err := s.GetReplica().SelectOne(&dbRole, "SELECT * from Roles WHERE Id = :Id", map[string]interface{}{"Id": roleId}); err != nil { if err == sql.ErrNoRows { - return nil, model.NewAppError("SqlRoleStore.Get", "store.sql_role.get.app_error", nil, "Id="+roleId+", "+err.Error(), http.StatusNotFound) + return nil, store.NewErrNotFound("Role", roleId) } - return nil, model.NewAppError("SqlRoleStore.Get", "store.sql_role.get.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "failed to get Role") } return dbRole.ToModel(), nil } -func (s *SqlRoleStore) GetAll() ([]*model.Role, *model.AppError) { +func (s *SqlRoleStore) GetAll() ([]*model.Role, error) { var dbRoles []Role if _, err := s.GetReplica().Select(&dbRoles, "SELECT * from Roles", map[string]interface{}{}); err != nil { - if err == sql.ErrNoRows { - return nil, model.NewAppError("SqlRoleStore.GetAll", "store.sql_role.get_all.app_error", nil, err.Error(), http.StatusNotFound) - } - return nil, model.NewAppError("SqlRoleStore.GetAll", "store.sql_role.get_all.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "failed to find Roles") } var roles []*model.Role @@ -179,40 +175,36 @@ func (s *SqlRoleStore) GetAll() ([]*model.Role, *model.AppError) { return roles, nil } -func (s *SqlRoleStore) GetByName(name string) (*model.Role, *model.AppError) { +func (s *SqlRoleStore) GetByName(name string) (*model.Role, error) { var dbRole Role if err := s.GetReplica().SelectOne(&dbRole, "SELECT * from Roles WHERE Name = :Name", map[string]interface{}{"Name": name}); err != nil { if err == sql.ErrNoRows { - return nil, model.NewAppError("SqlRoleStore.GetByName", "store.sql_role.get_by_name.app_error", nil, "name="+name+",err="+err.Error(), http.StatusNotFound) + return nil, store.NewErrNotFound("Role", fmt.Sprintf("name=%s", name)) } - return nil, model.NewAppError("SqlRoleStore.GetByName", "store.sql_role.get_by_name.app_error", nil, "name="+name+",err="+err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to find Roles with name=%s", name) } return dbRole.ToModel(), nil } -func (s *SqlRoleStore) GetByNames(names []string) ([]*model.Role, *model.AppError) { +func (s *SqlRoleStore) GetByNames(names []string) ([]*model.Role, error) { if len(names) == 0 { return []*model.Role{}, nil } - failure := func(e error) ([]*model.Role, *model.AppError) { - return nil, model.NewAppError("SqlRoleStore.GetByNames", "store.sql_role.get_by_names.app_error", nil, e.Error(), http.StatusInternalServerError) - } - query := s.getQueryBuilder(). Select("Id, Name, DisplayName, Description, CreateAt, UpdateAt, DeleteAt, Permissions, SchemeManaged, BuiltIn"). From("Roles"). Where(sq.Eq{"Name": names}) queryString, args, err := query.ToSql() if err != nil { - return failure(err) + return nil, errors.Wrap(err, "role_tosql") } rows, err := s.GetReplica().Db.Query(queryString, args...) if err != nil { - return failure(err) + return nil, errors.Wrap(err, "failed to find Roles") } var roles []*model.Role @@ -224,26 +216,25 @@ func (s *SqlRoleStore) GetByNames(names []string) ([]*model.Role, *model.AppErro &role.CreateAt, &role.UpdateAt, &role.DeleteAt, &role.Permissions, &role.SchemeManaged, &role.BuiltIn) if err != nil { - return failure(err) + return nil, errors.Wrap(err, "failed to scan values") } roles = append(roles, role.ToModel()) } - err = rows.Err() - if err != nil { - return failure(err) + if err = rows.Err(); err != nil { + return nil, errors.Wrap(err, "unable to iterate over rows") } return roles, nil } -func (s *SqlRoleStore) Delete(roleId string) (*model.Role, *model.AppError) { +func (s *SqlRoleStore) Delete(roleId string) (*model.Role, error) { // Get the role. var role *Role if err := s.GetReplica().SelectOne(&role, "SELECT * from Roles WHERE Id = :Id", map[string]interface{}{"Id": roleId}); err != nil { if err == sql.ErrNoRows { - return nil, model.NewAppError("SqlRoleStore.Delete", "store.sql_role.get.app_error", nil, "Id="+roleId+", "+err.Error(), http.StatusNotFound) + return nil, store.NewErrNotFound("Role", roleId) } - return nil, model.NewAppError("SqlRoleStore.Delete", "store.sql_role.get.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to get Role with id=%s", roleId) } time := model.GetMillis() @@ -251,16 +242,16 @@ func (s *SqlRoleStore) Delete(roleId string) (*model.Role, *model.AppError) { role.UpdateAt = time if rowsChanged, err := s.GetMaster().Update(role); err != nil { - return nil, model.NewAppError("SqlRoleStore.Delete", "store.sql_role.delete.update.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "failed to update Role") } else if rowsChanged != 1 { - return nil, model.NewAppError("SqlRoleStore.Delete", "store.sql_role.delete.update.app_error", nil, "no record to update", http.StatusInternalServerError) + return nil, errors.Wrapf(err, "invalid number of updated rows, expected 1 but got %d", rowsChanged) } return role.ToModel(), nil } -func (s *SqlRoleStore) PermanentDeleteAll() *model.AppError { +func (s *SqlRoleStore) PermanentDeleteAll() error { if _, err := s.GetMaster().Exec("DELETE FROM Roles"); err != nil { - return model.NewAppError("SqlRoleStore.PermanentDeleteAll", "store.sql_role.permanent_delete_all.app_error", nil, err.Error(), http.StatusInternalServerError) + return errors.Wrap(err, "failed to delete Roles") } return nil @@ -340,12 +331,12 @@ func (s *SqlRoleStore) channelHigherScopedPermissionsQuery(roleNames []string) s ) } -func (s *SqlRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, *model.AppError) { - sql := s.channelHigherScopedPermissionsQuery(roleNames) +func (s *SqlRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, error) { + query := s.channelHigherScopedPermissionsQuery(roleNames) var rolesPermissions []*channelRolesPermissions - if _, err := s.GetReplica().Select(&rolesPermissions, sql); err != nil { - return nil, model.NewAppError("SqlRoleStore.HigherScopedPermissions", "store.sql_role.get_by_names.app_error", nil, err.Error(), http.StatusInternalServerError) + if _, err := s.GetReplica().Select(&rolesPermissions, query); err != nil { + return nil, errors.Wrap(err, "failed to find RolePermissions") } roleNameHigherScopedPermissions := map[string]*model.RolePermissions{} @@ -359,7 +350,7 @@ func (s *SqlRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[s return roleNameHigherScopedPermissions, nil } -func (s *SqlRoleStore) AllChannelSchemeRoles() ([]*model.Role, *model.AppError) { +func (s *SqlRoleStore) AllChannelSchemeRoles() ([]*model.Role, error) { query := s.getQueryBuilder(). Select("Roles.*"). From("Schemes"). @@ -370,12 +361,12 @@ func (s *SqlRoleStore) AllChannelSchemeRoles() ([]*model.Role, *model.AppError) queryString, args, err := query.ToSql() if err != nil { - return nil, model.NewAppError("SqlRoleStore.AllChannelSchemeManagedRoles", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "role_tosql") } var dbRoles []*Role if _, err = s.GetReplica().Select(&dbRoles, queryString, args...); err != nil { - return nil, model.NewAppError("SqlRoleStore.AllChannelSchemeManagedRoles", "store.sql_role.get.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "failed to find Roles") } var roles []*model.Role @@ -387,7 +378,7 @@ func (s *SqlRoleStore) AllChannelSchemeRoles() ([]*model.Role, *model.AppError) } // ChannelRolesUnderTeamRole finds all of the channel-scheme roles under the team of the given team-scheme role. -func (s *SqlRoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, *model.AppError) { +func (s *SqlRoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, error) { query := s.getQueryBuilder(). Select("ChannelSchemeRoles.*"). From("Roles AS HigherScopedRoles"). @@ -407,12 +398,12 @@ func (s *SqlRoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role queryString, args, err := query.ToSql() if err != nil { - return nil, model.NewAppError("SqlRoleStore.ChannelRolesUnderTeamRole", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "role_tosql") } var dbRoles []*Role if _, err = s.GetReplica().Select(&dbRoles, queryString, args...); err != nil { - return nil, model.NewAppError("SqlRoleStore.ChannelRolesUnderTeamRole", "store.sql_role.get.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "failed to find Roles") } var roles []*model.Role diff --git a/store/store.go b/store/store.go index 4a72a67a1d..9de2e1328c 100644 --- a/store/store.go +++ b/store/store.go @@ -603,24 +603,24 @@ type PluginStore interface { } type RoleStore interface { - Save(role *model.Role) (*model.Role, *model.AppError) - Get(roleId string) (*model.Role, *model.AppError) - GetAll() ([]*model.Role, *model.AppError) - GetByName(name string) (*model.Role, *model.AppError) - GetByNames(names []string) ([]*model.Role, *model.AppError) - Delete(roleId string) (*model.Role, *model.AppError) - PermanentDeleteAll() *model.AppError + Save(role *model.Role) (*model.Role, error) + Get(roleId string) (*model.Role, error) + GetAll() ([]*model.Role, error) + GetByName(name string) (*model.Role, error) + GetByNames(names []string) ([]*model.Role, error) + Delete(roleId string) (*model.Role, error) + PermanentDeleteAll() error // HigherScopedPermissions retrieves the higher-scoped permissions of a list of role names. The higher-scope // (either team scheme or system scheme) is determined based on whether the team has a scheme or not. - ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, *model.AppError) + ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, error) // AllChannelSchemeRoles returns all of the roles associated to channel schemes. - AllChannelSchemeRoles() ([]*model.Role, *model.AppError) + AllChannelSchemeRoles() ([]*model.Role, error) // ChannelRolesUnderTeamRole returns all of the non-deleted roles that are affected by updates to the // given role. - ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, *model.AppError) + ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, error) } type SchemeStore interface { diff --git a/store/storetest/mocks/RoleStore.go b/store/storetest/mocks/RoleStore.go index a8809e33e3..a21e2b48bf 100644 --- a/store/storetest/mocks/RoleStore.go +++ b/store/storetest/mocks/RoleStore.go @@ -15,7 +15,7 @@ type RoleStore struct { } // AllChannelSchemeRoles provides a mock function with given fields: -func (_m *RoleStore) AllChannelSchemeRoles() ([]*model.Role, *model.AppError) { +func (_m *RoleStore) AllChannelSchemeRoles() ([]*model.Role, error) { ret := _m.Called() var r0 []*model.Role @@ -27,20 +27,18 @@ func (_m *RoleStore) AllChannelSchemeRoles() ([]*model.Role, *model.AppError) { } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func() *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func() error); ok { r1 = rf() } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // ChannelHigherScopedPermissions provides a mock function with given fields: roleNames -func (_m *RoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, *model.AppError) { +func (_m *RoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, error) { ret := _m.Called(roleNames) var r0 map[string]*model.RolePermissions @@ -52,20 +50,18 @@ func (_m *RoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[str } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func([]string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func([]string) error); ok { r1 = rf(roleNames) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // ChannelRolesUnderTeamRole provides a mock function with given fields: roleName -func (_m *RoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, *model.AppError) { +func (_m *RoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, error) { ret := _m.Called(roleName) var r0 []*model.Role @@ -77,20 +73,18 @@ func (_m *RoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { r1 = rf(roleName) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // Delete provides a mock function with given fields: roleId -func (_m *RoleStore) Delete(roleId string) (*model.Role, *model.AppError) { +func (_m *RoleStore) Delete(roleId string) (*model.Role, error) { ret := _m.Called(roleId) var r0 *model.Role @@ -102,20 +96,18 @@ func (_m *RoleStore) Delete(roleId string) (*model.Role, *model.AppError) { } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { r1 = rf(roleId) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // Get provides a mock function with given fields: roleId -func (_m *RoleStore) Get(roleId string) (*model.Role, *model.AppError) { +func (_m *RoleStore) Get(roleId string) (*model.Role, error) { ret := _m.Called(roleId) var r0 *model.Role @@ -127,20 +119,18 @@ func (_m *RoleStore) Get(roleId string) (*model.Role, *model.AppError) { } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { r1 = rf(roleId) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // GetAll provides a mock function with given fields: -func (_m *RoleStore) GetAll() ([]*model.Role, *model.AppError) { +func (_m *RoleStore) GetAll() ([]*model.Role, error) { ret := _m.Called() var r0 []*model.Role @@ -152,20 +142,18 @@ func (_m *RoleStore) GetAll() ([]*model.Role, *model.AppError) { } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func() *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func() error); ok { r1 = rf() } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // GetByName provides a mock function with given fields: name -func (_m *RoleStore) GetByName(name string) (*model.Role, *model.AppError) { +func (_m *RoleStore) GetByName(name string) (*model.Role, error) { ret := _m.Called(name) var r0 *model.Role @@ -177,20 +165,18 @@ func (_m *RoleStore) GetByName(name string) (*model.Role, *model.AppError) { } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { r1 = rf(name) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // GetByNames provides a mock function with given fields: names -func (_m *RoleStore) GetByNames(names []string) ([]*model.Role, *model.AppError) { +func (_m *RoleStore) GetByNames(names []string) ([]*model.Role, error) { ret := _m.Called(names) var r0 []*model.Role @@ -202,36 +188,32 @@ func (_m *RoleStore) GetByNames(names []string) ([]*model.Role, *model.AppError) } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func([]string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func([]string) error); ok { r1 = rf(names) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // PermanentDeleteAll provides a mock function with given fields: -func (_m *RoleStore) PermanentDeleteAll() *model.AppError { +func (_m *RoleStore) PermanentDeleteAll() error { ret := _m.Called() - var r0 *model.AppError - if rf, ok := ret.Get(0).(func() *model.AppError); ok { + var r0 error + if rf, ok := ret.Get(0).(func() error); ok { r0 = rf() } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.AppError) - } + r0 = ret.Error(0) } return r0 } // Save provides a mock function with given fields: role -func (_m *RoleStore) Save(role *model.Role) (*model.Role, *model.AppError) { +func (_m *RoleStore) Save(role *model.Role) (*model.Role, error) { ret := _m.Called(role) var r0 *model.Role @@ -243,13 +225,11 @@ func (_m *RoleStore) Save(role *model.Role) (*model.Role, *model.AppError) { } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(*model.Role) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(*model.Role) error); ok { r1 = rf(role) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 diff --git a/store/storetest/scheme_store.go b/store/storetest/scheme_store.go index 41cd53043e..0c03b4bf66 100644 --- a/store/storetest/scheme_store.go +++ b/store/storetest/scheme_store.go @@ -537,7 +537,7 @@ func testCountWithoutPermission(t *testing.T, ss store.Store) { } getRoles := func(scheme *model.Scheme) (channelUser, channelGuest *model.Role) { - var err *model.AppError + var err error channelUser, err = ss.Role().GetByName(scheme.DefaultChannelUserRole) require.Nil(t, err) require.NotNil(t, channelUser) diff --git a/store/timerlayer/timerlayer.go b/store/timerlayer/timerlayer.go index a9cccd53c1..4a70a9bd78 100644 --- a/store/timerlayer/timerlayer.go +++ b/store/timerlayer/timerlayer.go @@ -5085,7 +5085,7 @@ func (s *TimerLayerReactionStore) Save(reaction *model.Reaction) (*model.Reactio return resultVar0, resultVar1 } -func (s *TimerLayerRoleStore) AllChannelSchemeRoles() ([]*model.Role, *model.AppError) { +func (s *TimerLayerRoleStore) AllChannelSchemeRoles() ([]*model.Role, error) { start := timemodule.Now() resultVar0, resultVar1 := s.RoleStore.AllChannelSchemeRoles() @@ -5101,7 +5101,7 @@ func (s *TimerLayerRoleStore) AllChannelSchemeRoles() ([]*model.Role, *model.App return resultVar0, resultVar1 } -func (s *TimerLayerRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, *model.AppError) { +func (s *TimerLayerRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, error) { start := timemodule.Now() resultVar0, resultVar1 := s.RoleStore.ChannelHigherScopedPermissions(roleNames) @@ -5117,7 +5117,7 @@ func (s *TimerLayerRoleStore) ChannelHigherScopedPermissions(roleNames []string) return resultVar0, resultVar1 } -func (s *TimerLayerRoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, *model.AppError) { +func (s *TimerLayerRoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, error) { start := timemodule.Now() resultVar0, resultVar1 := s.RoleStore.ChannelRolesUnderTeamRole(roleName) @@ -5133,7 +5133,7 @@ func (s *TimerLayerRoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*mod return resultVar0, resultVar1 } -func (s *TimerLayerRoleStore) Delete(roleId string) (*model.Role, *model.AppError) { +func (s *TimerLayerRoleStore) Delete(roleId string) (*model.Role, error) { start := timemodule.Now() resultVar0, resultVar1 := s.RoleStore.Delete(roleId) @@ -5149,7 +5149,7 @@ func (s *TimerLayerRoleStore) Delete(roleId string) (*model.Role, *model.AppErro return resultVar0, resultVar1 } -func (s *TimerLayerRoleStore) Get(roleId string) (*model.Role, *model.AppError) { +func (s *TimerLayerRoleStore) Get(roleId string) (*model.Role, error) { start := timemodule.Now() resultVar0, resultVar1 := s.RoleStore.Get(roleId) @@ -5165,7 +5165,7 @@ func (s *TimerLayerRoleStore) Get(roleId string) (*model.Role, *model.AppError) return resultVar0, resultVar1 } -func (s *TimerLayerRoleStore) GetAll() ([]*model.Role, *model.AppError) { +func (s *TimerLayerRoleStore) GetAll() ([]*model.Role, error) { start := timemodule.Now() resultVar0, resultVar1 := s.RoleStore.GetAll() @@ -5181,7 +5181,7 @@ func (s *TimerLayerRoleStore) GetAll() ([]*model.Role, *model.AppError) { return resultVar0, resultVar1 } -func (s *TimerLayerRoleStore) GetByName(name string) (*model.Role, *model.AppError) { +func (s *TimerLayerRoleStore) GetByName(name string) (*model.Role, error) { start := timemodule.Now() resultVar0, resultVar1 := s.RoleStore.GetByName(name) @@ -5197,7 +5197,7 @@ func (s *TimerLayerRoleStore) GetByName(name string) (*model.Role, *model.AppErr return resultVar0, resultVar1 } -func (s *TimerLayerRoleStore) GetByNames(names []string) ([]*model.Role, *model.AppError) { +func (s *TimerLayerRoleStore) GetByNames(names []string) ([]*model.Role, error) { start := timemodule.Now() resultVar0, resultVar1 := s.RoleStore.GetByNames(names) @@ -5213,7 +5213,7 @@ func (s *TimerLayerRoleStore) GetByNames(names []string) ([]*model.Role, *model. return resultVar0, resultVar1 } -func (s *TimerLayerRoleStore) PermanentDeleteAll() *model.AppError { +func (s *TimerLayerRoleStore) PermanentDeleteAll() error { start := timemodule.Now() resultVar0 := s.RoleStore.PermanentDeleteAll() @@ -5229,7 +5229,7 @@ func (s *TimerLayerRoleStore) PermanentDeleteAll() *model.AppError { return resultVar0 } -func (s *TimerLayerRoleStore) Save(role *model.Role) (*model.Role, *model.AppError) { +func (s *TimerLayerRoleStore) Save(role *model.Role) (*model.Role, error) { start := timemodule.Now() resultVar0, resultVar1 := s.RoleStore.Save(role)