From 627d2b9d6ce87fdaca2641e98c41256a1f5112a4 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma <43992469+shadowshot-x@users.noreply.github.com> Date: Mon, 15 Nov 2021 22:10:13 +0530 Subject: [PATCH] [MM-39628] Migrate from gorp to sqlx in store/sqlstore/scheme_store.go (#18895) Automatic Merge --- store/sqlstore/role_store.go | 12 ++-- store/sqlstore/scheme_store.go | 106 +++++++++++++++++++++++---------- 2 files changed, 80 insertions(+), 38 deletions(-) diff --git a/store/sqlstore/role_store.go b/store/sqlstore/role_store.go index 5e8d120ef8..e1902a35ed 100644 --- a/store/sqlstore/role_store.go +++ b/store/sqlstore/role_store.go @@ -10,7 +10,6 @@ import ( "strings" sq "github.com/Masterminds/squirrel" - "github.com/mattermost/gorp" "github.com/pkg/errors" "github.com/mattermost/mattermost-server/v6/model" @@ -104,11 +103,11 @@ func (s *SqlRoleStore) Save(role *model.Role) (*model.Role, error) { } if role.Id == "" { - transaction, err := s.GetMaster().Begin() + transaction, err := s.GetMasterX().Beginx() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransaction(transaction) + defer finalizeTransactionX(transaction) createdRole, err := s.createRole(role, transaction) if err != nil { _ = transaction.Rollback() @@ -130,7 +129,7 @@ func (s *SqlRoleStore) Save(role *model.Role) (*model.Role, error) { return dbRole.ToModel(), nil } -func (s *SqlRoleStore) createRole(role *model.Role, transaction *gorp.Transaction) (*model.Role, error) { +func (s *SqlRoleStore) createRole(role *model.Role, transaction *sqlxTxWrapper) (*model.Role, error) { // Check the role is valid before proceeding. if !role.IsValidWithoutId() { return nil, store.NewErrInvalidInput("Role", "", fmt.Sprintf("%v", role)) @@ -142,7 +141,10 @@ func (s *SqlRoleStore) createRole(role *model.Role, transaction *gorp.Transactio dbRole.CreateAt = model.GetMillis() dbRole.UpdateAt = dbRole.CreateAt - if err := transaction.Insert(dbRole); err != nil { + if _, err := transaction.NamedExec(`INSERT INTO Roles + (Id, Name, DisplayName, Description, Permissions, CreateAt, UpdateAt, DeleteAt, SchemeManaged, BuiltIn) + VALUES + (:Id, :Name, :DisplayName, :Description, :Permissions, :CreateAt, :UpdateAt, :DeleteAt, :SchemeManaged, :BuiltIn)`, dbRole); err != nil { return nil, errors.Wrap(err, "failed to save Role") } diff --git a/store/sqlstore/scheme_store.go b/store/sqlstore/scheme_store.go index 07f7c9773a..2c025e8c81 100644 --- a/store/sqlstore/scheme_store.go +++ b/store/sqlstore/scheme_store.go @@ -6,9 +6,8 @@ package sqlstore import ( "database/sql" "fmt" - "strings" - "github.com/mattermost/gorp" + sq "github.com/Masterminds/squirrel" "github.com/pkg/errors" "github.com/mattermost/mattermost-server/v6/model" @@ -48,11 +47,11 @@ func (s SqlSchemeStore) createIndexesIfNotExists() { func (s *SqlSchemeStore) Save(scheme *model.Scheme) (*model.Scheme, error) { if scheme.Id == "" { - transaction, err := s.GetMaster().Begin() + transaction, err := s.GetMasterX().Beginx() if err != nil { return nil, errors.Wrap(err, "begin_transaction") } - defer finalizeTransaction(transaction) + defer finalizeTransactionX(transaction) newScheme, err := s.createScheme(scheme, transaction) if err != nil { @@ -70,10 +69,20 @@ func (s *SqlSchemeStore) Save(scheme *model.Scheme) (*model.Scheme, error) { scheme.UpdateAt = model.GetMillis() - rowsChanged, err := s.GetMaster().Update(scheme) + res, err := s.GetMasterX().NamedExec(`UPDATE Schemes + SET UpdateAt=:UpdateAt, CreateAt=:CreateAt, DeleteAt=:DeleteAt, Name=:Name, DisplayName=:DisplayName, Description=:Description, Scope=:Scope, + DefaultTeamAdminRole=:DefaultTeamAdminRole, DefaultTeamUserRole=:DefaultTeamUserRole, DefaultTeamGuestRole=:DefaultTeamGuestRole, + DefaultChannelAdminRole=:DefaultChannelAdminRole, DefaultChannelUserRole=:DefaultChannelUserRole, DefaultChannelGuestRole=:DefaultChannelGuestRole + WHERE Id=:Id`, scheme) + if err != nil { return nil, errors.Wrap(err, "failed to update Scheme") } + + rowsChanged, err := res.RowsAffected() + if err != nil { + return nil, errors.Wrap(err, "error while getting rows_affected") + } if rowsChanged != 1 { return nil, errors.New("no record to update") } @@ -81,7 +90,7 @@ func (s *SqlSchemeStore) Save(scheme *model.Scheme) (*model.Scheme, error) { return scheme, nil } -func (s *SqlSchemeStore) createScheme(scheme *model.Scheme, transaction *gorp.Transaction) (*model.Scheme, error) { +func (s *SqlSchemeStore) createScheme(scheme *model.Scheme, transaction *sqlxTxWrapper) (*model.Scheme, error) { // Fetch the default system scheme roles to populate default permissions. defaultRoleNames := []string{model.TeamAdminRoleId, model.TeamUserRoleId, model.TeamGuestRoleId, model.ChannelAdminRoleId, model.ChannelUserRoleId, model.ChannelGuestRoleId} defaultRoles := make(map[string]*model.Role) @@ -224,7 +233,10 @@ func (s *SqlSchemeStore) createScheme(scheme *model.Scheme, transaction *gorp.Tr return nil, store.NewErrInvalidInput("Scheme", "", fmt.Sprintf("%v", scheme)) } - if err := transaction.Insert(scheme); err != nil { + if _, err := transaction.NamedExec(`INSERT INTO Schemes + (Id, Name, DisplayName, Description, Scope, DefaultTeamAdminRole, DefaultTeamUserRole, DefaultTeamGuestRole, DefaultChannelAdminRole, DefaultChannelUserRole, DefaultChannelGuestRole, CreateAt, UpdateAt, DeleteAt) + VALUES + (:Id, :Name, :DisplayName, :Description, :Scope, :DefaultTeamAdminRole, :DefaultTeamUserRole, :DefaultTeamGuestRole, :DefaultChannelAdminRole, :DefaultChannelUserRole, :DefaultChannelGuestRole, :CreateAt, :UpdateAt, :DeleteAt)`, scheme); err != nil { return nil, errors.Wrap(err, "failed to save Scheme") } @@ -243,7 +255,7 @@ func filterModerated(permissions []string) []string { func (s *SqlSchemeStore) Get(schemeId string) (*model.Scheme, error) { var scheme model.Scheme - if err := s.GetReplica().SelectOne(&scheme, "SELECT * from Schemes WHERE Id = :Id", map[string]interface{}{"Id": schemeId}); err != nil { + if err := s.GetReplicaX().Get(&scheme, "SELECT * from Schemes WHERE Id = ?", schemeId); err != nil { if err == sql.ErrNoRows { return nil, store.NewErrNotFound("Scheme", fmt.Sprintf("schemeId=%s", schemeId)) } @@ -256,7 +268,7 @@ func (s *SqlSchemeStore) Get(schemeId string) (*model.Scheme, error) { func (s *SqlSchemeStore) GetByName(schemeName string) (*model.Scheme, error) { var scheme model.Scheme - if err := s.GetReplica().SelectOne(&scheme, "SELECT * from Schemes WHERE Name = :Name", map[string]interface{}{"Name": schemeName}); err != nil { + if err := s.GetReplicaX().Get(&scheme, "SELECT * from Schemes WHERE Name = ?", schemeName); err != nil { if err == sql.ErrNoRows { return nil, store.NewErrNotFound("Scheme", fmt.Sprintf("schemeName=%s", schemeName)) } @@ -268,8 +280,8 @@ func (s *SqlSchemeStore) GetByName(schemeName string) (*model.Scheme, error) { func (s *SqlSchemeStore) Delete(schemeId string) (*model.Scheme, error) { // Get the scheme - var scheme model.Scheme - if err := s.GetMaster().SelectOne(&scheme, "SELECT * from Schemes WHERE Id = :Id", map[string]interface{}{"Id": schemeId}); err != nil { + scheme := model.Scheme{} + if err := s.GetMasterX().Get(&scheme, `SELECT * from Schemes WHERE Id = ?`, schemeId); err != nil { if err == sql.ErrNoRows { return nil, store.NewErrNotFound("Scheme", fmt.Sprintf("schemeId=%s", schemeId)) } @@ -278,13 +290,13 @@ func (s *SqlSchemeStore) Delete(schemeId string) (*model.Scheme, error) { // Update any teams or channels using this scheme to the default scheme. if scheme.Scope == model.SchemeScopeTeam { - if _, err := s.GetMaster().Exec("UPDATE Teams SET SchemeId = '' WHERE SchemeId = :SchemeId", map[string]interface{}{"SchemeId": schemeId}); err != nil { + if _, err := s.GetMasterX().Exec(`UPDATE Teams SET SchemeId = '' WHERE SchemeId = ?`, schemeId); err != nil { return nil, errors.Wrapf(err, "failed to update Teams with schemeId=%s", schemeId) } s.Team().ClearCaches() } else if scheme.Scope == model.SchemeScopeChannel { - if _, err := s.GetMaster().Exec("UPDATE Channels SET SchemeId = '' WHERE SchemeId = :SchemeId", map[string]interface{}{"SchemeId": schemeId}); err != nil { + if _, err := s.GetMasterX().Exec(`UPDATE Channels SET SchemeId = '' WHERE SchemeId = ?`, schemeId); err != nil { return nil, errors.Wrapf(err, "failed to update Channels with schemeId=%s", schemeId) } } @@ -298,30 +310,42 @@ func (s *SqlSchemeStore) Delete(schemeId string) (*model.Scheme, error) { roleNames = append(roleNames, scheme.DefaultTeamGuestRole, scheme.DefaultTeamUserRole, scheme.DefaultTeamAdminRole) } - var inQueryList []string - queryArgs := make(map[string]interface{}) - for i, roleId := range roleNames { - inQueryList = append(inQueryList, fmt.Sprintf(":RoleName%v", i)) - queryArgs[fmt.Sprintf("RoleName%v", i)] = roleId - } - inQuery := strings.Join(inQueryList, ", ") - time := model.GetMillis() - queryArgs["UpdateAt"] = time - queryArgs["DeleteAt"] = time - if _, err := s.GetMaster().Exec("UPDATE Roles SET UpdateAt = :UpdateAt, DeleteAt = :DeleteAt WHERE Name IN ("+inQuery+")", queryArgs); err != nil { - return nil, errors.Wrapf(err, "failed to update Roles with name in (%s)", inQuery) + updateQuery, args, err := s.getQueryBuilder(). + Update("Roles"). + Where(sq.Eq{"Name": roleNames}). + Set("UpdateAt", time). + Set("DeleteAt", time). + ToSql() + + if err != nil { + return nil, errors.Wrap(err, "status_tosql") + } + + if _, err = s.GetMasterX().Exec(updateQuery, args...); err != nil { + return nil, errors.Wrapf(err, "failed to update Roles with name in (%s)", roleNames) } // Delete the scheme itself. scheme.UpdateAt = time scheme.DeleteAt = time - rowsChanged, err := s.GetMaster().Update(&scheme) + res, err := s.GetMasterX().NamedExec(`UPDATE Schemes + SET UpdateAt=:UpdateAt, DeleteAt=:DeleteAt, CreateAt=:CreateAt, Name=:Name, DisplayName=:DisplayName, Description=:Description, Scope=:Scope, + DefaultTeamAdminRole=:DefaultTeamAdminRole, DefaultTeamUserRole=:DefaultTeamUserRole, DefaultTeamGuestRole=:DefaultTeamGuestRole, + DefaultChannelAdminRole=:DefaultChannelAdminRole, DefaultChannelUserRole=:DefaultChannelUserRole, DefaultChannelGuestRole=:DefaultChannelGuestRole + WHERE Id=:Id`, &scheme) + if err != nil { return nil, errors.Wrapf(err, "failed to update Scheme with schemeId=%s", schemeId) } + + rowsChanged, err := res.RowsAffected() + + if err != nil { + return nil, errors.Wrapf(err, "failed to get RowsAffected while updating scheme with schemeId=%s", schemeId) + } if rowsChanged != 1 { return nil, errors.New("no record to update") } @@ -329,14 +353,26 @@ func (s *SqlSchemeStore) Delete(schemeId string) (*model.Scheme, error) { } func (s *SqlSchemeStore) GetAllPage(scope string, offset int, limit int) ([]*model.Scheme, error) { - var schemes []*model.Scheme + schemes := []*model.Scheme{} + + query := s.getQueryBuilder(). + Select("*"). + From("Schemes"). + Where(sq.Eq{"DeleteAt": 0}). + OrderBy("CreateAt DESC"). + Limit(uint64(limit)). + Offset(uint64(offset)) - scopeClause := "" if scope != "" { - scopeClause = " AND Scope=:Scope " + query = query.Where(sq.Eq{"Scope": scope}) } - if _, err := s.GetReplica().Select(&schemes, "SELECT * from Schemes WHERE DeleteAt = 0 "+scopeClause+" ORDER BY CreateAt DESC LIMIT :Limit OFFSET :Offset", map[string]interface{}{"Limit": limit, "Offset": offset, "Scope": scope}); err != nil { + queryString, args, err := query.ToSql() + if err != nil { + return nil, errors.Wrap(err, "status_tosql") + } + + if err := s.GetReplicaX().Select(&schemes, queryString, args...); err != nil { return nil, errors.Wrapf(err, "failed to get Schemes") } @@ -344,7 +380,7 @@ func (s *SqlSchemeStore) GetAllPage(scope string, offset int, limit int) ([]*mod } func (s *SqlSchemeStore) PermanentDeleteAll() error { - if _, err := s.GetMaster().Exec("DELETE from Schemes"); err != nil { + if _, err := s.GetMasterX().Exec("DELETE from Schemes"); err != nil { return errors.Wrap(err, "failed to delete Schemes") } @@ -352,7 +388,9 @@ func (s *SqlSchemeStore) PermanentDeleteAll() error { } func (s *SqlSchemeStore) CountByScope(scope string) (int64, error) { - count, err := s.GetReplica().SelectInt("SELECT count(*) FROM Schemes WHERE Scope = :Scope AND DeleteAt = 0", map[string]interface{}{"Scope": scope}) + var count int64 + err := s.GetReplicaX().Get(&count, `SELECT count(*) FROM Schemes WHERE Scope = ? AND DeleteAt = 0`, scope) + if err != nil { return int64(0), errors.Wrap(err, "failed to count Schemes by scope") } @@ -371,7 +409,9 @@ func (s *SqlSchemeStore) CountWithoutPermission(schemeScope, permissionID string Schemes.Scope = '%s' AND Roles.Permissions NOT LIKE '%%%s%%' `, joinCol, schemeScope, permissionID) - count, err := s.GetReplica().SelectInt(query) + + var count int64 + err := s.GetReplicaX().Get(&count, query) if err != nil { return int64(0), errors.Wrap(err, "failed to count Schemes without permission") }