From 485ea6f14f38cfa5da4fa27865f62fcc7691ffb4 Mon Sep 17 00:00:00 2001 From: Unknwon Date: Tue, 10 Feb 2015 23:44:16 -0500 Subject: [PATCH] models: make code change for session issue with SQLite3 #739 --- models/models.go | 3 ++ models/org.go | 94 ++++++++++++++++++++++++-------------------- models/repo.go | 8 ++-- routers/org/teams.go | 7 +--- 4 files changed, 60 insertions(+), 52 deletions(-) diff --git a/models/models.go b/models/models.go index df030e51bb..3eeeabda3f 100644 --- a/models/models.go +++ b/models/models.go @@ -23,7 +23,10 @@ import ( type Engine interface { Delete(interface{}) (int64, error) Exec(string, ...interface{}) (sql.Result, error) + Get(interface{}) (bool, error) Insert(...interface{}) (int64, error) + Id(interface{}) *xorm.Session + Where(string, ...interface{}) *xorm.Session } var ( diff --git a/models/org.go b/models/org.go index 5431a111c3..3d37a37d69 100644 --- a/models/org.go +++ b/models/org.go @@ -12,7 +12,6 @@ import ( "strings" "github.com/Unknwon/com" - "github.com/go-xorm/xorm" "github.com/gogits/gogs/modules/base" ) @@ -391,7 +390,7 @@ func RemoveOrgUser(orgId, uid int64) error { return err } for _, t := range ts { - if err = removeTeamMemberWithSess(org.Id, t.Id, u.Id, sess); err != nil { + if err = removeTeamMember(sess, org.Id, t.Id, u.Id); err != nil { return err } } @@ -486,18 +485,18 @@ func (t *Team) RemoveMember(uid int64) error { } // addAccessWithAuthorize inserts or updates access with given mode. -func addAccessWithAuthorize(sess *xorm.Session, access *Access, mode AccessType) error { - has, err := x.Get(access) +func addAccessWithAuthorize(e Engine, access *Access, mode AccessType) error { + has, err := e.Get(access) if err != nil { return fmt.Errorf("fail to get access: %v", err) } access.Mode = mode if has { - if _, err = sess.Id(access.Id).Update(access); err != nil { + if _, err = e.Id(access.Id).Update(access); err != nil { return fmt.Errorf("fail to update access: %v", err) } } else { - if _, err = sess.Insert(access); err != nil { + if _, err = e.Insert(access); err != nil { return fmt.Errorf("fail to insert access: %v", err) } } @@ -536,7 +535,7 @@ func (t *Team) AddRepository(repo *Repository) (err error) { mode := AuthorizeToAccessType(t.Authorize) for _, u := range t.Members { - auth, err := GetHighestAuthorize(t.OrgId, u.Id, repo.Id, t.Id) + auth, err := getHighestAuthorize(sess, t.OrgId, u.Id, repo.Id, t.Id) if err != nil { sess.Rollback() return err @@ -552,7 +551,7 @@ func (t *Team) AddRepository(repo *Repository) (err error) { return err } } - if err = WatchRepo(u.Id, repo.Id, true); err != nil { + if err = watchRepo(sess, u.Id, repo.Id, true); err != nil { sess.Rollback() return err } @@ -593,7 +592,7 @@ func (t *Team) RemoveRepository(repoId int64) error { // Remove access to team members. for _, u := range t.Members { - auth, err := GetHighestAuthorize(t.OrgId, u.Id, repo.Id, t.Id) + auth, err := getHighestAuthorize(sess, t.OrgId, u.Id, repo.Id, t.Id) if err != nil { sess.Rollback() return err @@ -607,7 +606,7 @@ func (t *Team) RemoveRepository(repoId int64) error { if _, err = sess.Delete(access); err != nil { sess.Rollback() return fmt.Errorf("fail to delete access: %v", err) - } else if err = WatchRepo(u.Id, repo.Id, false); err != nil { + } else if err = watchRepo(sess, u.Id, repo.Id, false); err != nil { sess.Rollback() return err } @@ -678,10 +677,9 @@ func GetTeam(orgId int64, name string) (*Team, error) { return t, nil } -// GetTeamById returns team by given ID. -func GetTeamById(teamId int64) (*Team, error) { +func getTeamById(e Engine, teamId int64) (*Team, error) { t := new(Team) - has, err := x.Id(teamId).Get(t) + has, err := e.Id(teamId).Get(t) if err != nil { return nil, err } else if !has { @@ -690,9 +688,13 @@ func GetTeamById(teamId int64) (*Team, error) { return t, nil } -// GetHighestAuthorize returns highest repository authorize level for given user and team. -func GetHighestAuthorize(orgId, uid, repoId, teamId int64) (AuthorizeType, error) { - ts, err := GetUserTeams(orgId, uid) +// GetTeamById returns team by given ID. +func GetTeamById(teamId int64) (*Team, error) { + return getTeamById(x, teamId) +} + +func getHighestAuthorize(e Engine, orgId, uid, repoId, teamId int64) (AuthorizeType, error) { + ts, err := getUserTeams(e, orgId, uid) if err != nil { return 0, err } @@ -714,6 +716,11 @@ func GetHighestAuthorize(orgId, uid, repoId, teamId int64) (AuthorizeType, error return auth, nil } +// GetHighestAuthorize returns highest repository authorize level for given user and team. +func GetHighestAuthorize(orgId, uid, repoId, teamId int64) (AuthorizeType, error) { + return getHighestAuthorize(x, orgId, uid, repoId, teamId) +} + // UpdateTeam updates information of team. func UpdateTeam(t *Team, authChanged bool) (err error) { if !IsLegalName(t.Name) { @@ -866,10 +873,14 @@ type TeamUser struct { TeamId int64 } +func isTeamMember(e Engine, orgId, teamId, uid int64) bool { + has, _ := e.Where("uid=?", uid).And("org_id=?", orgId).And("team_id=?", teamId).Get(new(TeamUser)) + return has +} + // IsTeamMember returns true if given user is a member of team. func IsTeamMember(orgId, teamId, uid int64) bool { - has, _ := x.Where("uid=?", uid).And("org_id=?", orgId).And("team_id=?", teamId).Get(new(TeamUser)) - return has + return isTeamMember(x, orgId, teamId, uid) } // GetTeamMembers returns all members in given team of organization. @@ -879,17 +890,16 @@ func GetTeamMembers(orgId, teamId int64) ([]*User, error) { return us, err } -// GetUserTeams returns all teams that user belongs to in given organization. -func GetUserTeams(orgId, uid int64) ([]*Team, error) { +func getUserTeams(e Engine, orgId, uid int64) ([]*Team, error) { tus := make([]*TeamUser, 0, 5) - if err := x.Where("uid=?", uid).And("org_id=?", orgId).Find(&tus); err != nil { + if err := e.Where("uid=?", uid).And("org_id=?", orgId).Find(&tus); err != nil { return nil, err } ts := make([]*Team, len(tus)) for i, tu := range tus { t := new(Team) - has, err := x.Id(tu.TeamId).Get(t) + has, err := e.Id(tu.TeamId).Get(t) if err != nil { return nil, err } else if !has { @@ -900,6 +910,11 @@ func GetUserTeams(orgId, uid int64) ([]*Team, error) { return ts, nil } +// GetUserTeams returns all teams that user belongs to in given organization. +func GetUserTeams(orgId, uid int64) ([]*Team, error) { + return getUserTeams(x, orgId, uid) +} + // AddTeamMember adds new member to given team of given organization. func AddTeamMember(orgId, teamId, uid int64) error { if IsTeamMember(orgId, teamId, uid) { @@ -956,7 +971,7 @@ func AddTeamMember(orgId, teamId, uid int64) error { // Give access to team repositories. mode := AuthorizeToAccessType(t.Authorize) for _, repo := range t.Repos { - auth, err := GetHighestAuthorize(t.OrgId, u.Id, repo.Id, teamId) + auth, err := getHighestAuthorize(sess, t.OrgId, u.Id, repo.Id, teamId) if err != nil { sess.Rollback() return err @@ -993,13 +1008,13 @@ func AddTeamMember(orgId, teamId, uid int64) error { return sess.Commit() } -func removeTeamMemberWithSess(orgId, teamId, uid int64, sess *xorm.Session) error { - if !IsTeamMember(orgId, teamId, uid) { +func removeTeamMember(e Engine, orgId, teamId, uid int64) error { + if !isTeamMember(e, orgId, teamId, uid) { return nil } // Get team and its repositories. - t, err := GetTeamById(teamId) + t, err := getTeamById(e, teamId) if err != nil { return err } @@ -1033,19 +1048,16 @@ func removeTeamMemberWithSess(orgId, teamId, uid int64, sess *xorm.Session) erro TeamId: teamId, } - if _, err := sess.Delete(tu); err != nil { - sess.Rollback() + if _, err := e.Delete(tu); err != nil { return err - } else if _, err = sess.Id(t.Id).AllCols().Update(t); err != nil { - sess.Rollback() + } else if _, err = e.Id(t.Id).AllCols().Update(t); err != nil { return err } // Delete access to team repositories. for _, repo := range t.Repos { - auth, err := GetHighestAuthorize(t.OrgId, u.Id, repo.Id, teamId) + auth, err := getHighestAuthorize(e, t.OrgId, u.Id, repo.Id, teamId) if err != nil { - sess.Rollback() return err } @@ -1055,17 +1067,14 @@ func removeTeamMemberWithSess(orgId, teamId, uid int64, sess *xorm.Session) erro } // Delete access if this is the last team user belongs to. if auth == 0 { - if _, err = sess.Delete(access); err != nil { - sess.Rollback() + if _, err = e.Delete(access); err != nil { return fmt.Errorf("fail to delete access: %v", err) - } else if err = WatchRepo(u.Id, repo.Id, false); err != nil { - sess.Rollback() + } else if err = watchRepo(e, u.Id, repo.Id, false); err != nil { return err } } else if auth < t.Authorize { // Downgrade authorize level. - if err = addAccessWithAuthorize(sess, access, AuthorizeToAccessType(auth)); err != nil { - sess.Rollback() + if err = addAccessWithAuthorize(e, access, AuthorizeToAccessType(auth)); err != nil { return err } } @@ -1073,17 +1082,15 @@ func removeTeamMemberWithSess(orgId, teamId, uid int64, sess *xorm.Session) erro // This must exist. ou := new(OrgUser) - _, err = sess.Where("uid=?", uid).And("org_id=?", org.Id).Get(ou) + _, err = e.Where("uid=?", uid).And("org_id=?", org.Id).Get(ou) if err != nil { - sess.Rollback() return err } ou.NumTeams-- if t.IsOwnerTeam() { ou.IsOwner = false } - if _, err = sess.Id(ou.Id).AllCols().Update(ou); err != nil { - sess.Rollback() + if _, err = e.Id(ou.Id).AllCols().Update(ou); err != nil { return err } return nil @@ -1096,7 +1103,8 @@ func RemoveTeamMember(orgId, teamId, uid int64) error { if err := sess.Begin(); err != nil { return err } - if err := removeTeamMemberWithSess(orgId, teamId, uid, sess); err != nil { + if err := removeTeamMember(sess, orgId, teamId, uid); err != nil { + sess.Rollback() return err } return sess.Commit() diff --git a/models/repo.go b/models/repo.go index 15aadbba66..cdb838a1fb 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1296,7 +1296,7 @@ func IsWatching(uid, repoId int64) bool { return has } -func watchRepoWithEngine(e Engine, uid, repoId int64, watch bool) (err error) { +func watchRepo(e Engine, uid, repoId int64, watch bool) (err error) { if watch { if IsWatching(uid, repoId) { return nil @@ -1319,7 +1319,7 @@ func watchRepoWithEngine(e Engine, uid, repoId int64, watch bool) (err error) { // Watch or unwatch repository. func WatchRepo(uid, repoId int64, watch bool) (err error) { - return watchRepoWithEngine(x, uid, repoId, watch) + return watchRepo(x, uid, repoId, watch) } // GetWatchers returns all watchers of given repository. @@ -1507,14 +1507,14 @@ func ForkRepository(u *User, oldRepo *Repository, name, desc string) (*Repositor log.Error(4, "GetMembers: %v", err) } else { for _, u := range t.Members { - if err = watchRepoWithEngine(sess, u.Id, repo.Id, true); err != nil { + if err = watchRepo(sess, u.Id, repo.Id, true); err != nil { log.Error(4, "WatchRepo2: %v", err) } } } } } else { - if err = watchRepoWithEngine(sess, u.Id, repo.Id, true); err != nil { + if err = watchRepo(sess, u.Id, repo.Id, true); err != nil { log.Error(4, "WatchRepo3: %v", err) } } diff --git a/routers/org/teams.go b/routers/org/teams.go index 77a7b6e13c..9dd9b8e229 100644 --- a/routers/org/teams.go +++ b/routers/org/teams.go @@ -138,11 +138,8 @@ func TeamsRepoAction(ctx *middleware.Context) { } if err != nil { - log.Error(3, "Action(%s): %v", ctx.Params(":action"), err) - ctx.JSON(200, map[string]interface{}{ - "ok": false, - "err": err.Error(), - }) + log.Error(3, "Action(%s): '%s' %v", ctx.Params(":action"), ctx.Org.Team.Name, err) + ctx.Handle(500, "TeamsRepoAction", err) return } ctx.Redirect(ctx.Org.OrgLink + "/teams/" + ctx.Org.Team.LowerName + "/repositories")