From 473e265727c1e47dd83e3f66a5cc92d2f027d951 Mon Sep 17 00:00:00 2001 From: Unknwon Date: Sat, 28 Feb 2015 21:44:09 -0500 Subject: [PATCH] models: done testing on new access system --- gogs.go | 2 +- models/access.go | 98 +++++++++++++++++++++++++++------------------- models/org.go | 57 ++++++++++++++++++--------- models/repo.go | 41 +++++++++++++------ templates/.VERSION | 2 +- 5 files changed, 129 insertions(+), 71 deletions(-) diff --git a/gogs.go b/gogs.go index 930203afa..96c848ec3 100644 --- a/gogs.go +++ b/gogs.go @@ -17,7 +17,7 @@ import ( "github.com/gogits/gogs/modules/setting" ) -const APP_VER = "0.5.15.0224 Beta" +const APP_VER = "0.5.16.0228 Beta" func init() { runtime.GOMAXPROCS(runtime.NumCPU()) diff --git a/models/access.go b/models/access.go index 683282f33..00871f948 100644 --- a/models/access.go +++ b/models/access.go @@ -100,25 +100,58 @@ func maxAccessMode(modes ...AccessMode) AccessMode { return max } -func (repo *Repository) recalculateTeamAccesses(e Engine, mode AccessMode) error { +// FIXME: do corss-comparison so reduce deletions and additions to the minimum? +func (repo *Repository) refreshAccesses(e Engine, accessMap map[int64]AccessMode) (err error) { + minMode := ACCESS_MODE_READ + if !repo.IsPrivate { + minMode = ACCESS_MODE_WRITE + } + + newAccesses := make([]Access, 0, len(accessMap)) + for userID, mode := range accessMap { + if mode < minMode { + continue + } + newAccesses = append(newAccesses, Access{ + UserID: userID, + RepoID: repo.Id, + Mode: mode, + }) + } + // Delete old accesses and insert new ones for repository. + if _, err = e.Delete(&Access{RepoID: repo.Id}); err != nil { + return fmt.Errorf("delete old accesses: %v", err) + } else if _, err = e.Insert(newAccesses); err != nil { + return fmt.Errorf("insert new accesses: %v", err) + } return nil } -func (repo *Repository) recalculateAccesses(e Engine) error { - accessMap := make(map[int64]AccessMode, 20) - - // FIXME: should be able to have read-only access. - // Give all collaborators write access. +// FIXME: should be able to have read-only access. +// Give all collaborators write access. +func (repo *Repository) refreshCollaboratorAccesses(e Engine, accessMap map[int64]AccessMode) error { collaborators, err := repo.getCollaborators(e) if err != nil { - return err + return fmt.Errorf("getCollaborators: %v", err) } for _, c := range collaborators { accessMap[c.Id] = ACCESS_MODE_WRITE } + return nil +} + +// recalculateTeamAccesses recalculates new accesses for teams of an organization +// except the team whose ID is given. It is used to assign a team ID when +// remove repository from that team. +func (repo *Repository) recalculateTeamAccesses(e Engine, ignTeamID int64) (err error) { + accessMap := make(map[int64]AccessMode, 20) + + if err = repo.refreshCollaboratorAccesses(e, accessMap); err != nil { + return fmt.Errorf("refreshCollaboratorAccesses: %v", err) + } - if err := repo.getOwner(e); err != nil { + if err = repo.getOwner(e); err != nil { return err } if repo.Owner.IsOrganization() { @@ -126,47 +159,32 @@ func (repo *Repository) recalculateAccesses(e Engine) error { return err } - for _, team := range repo.Owner.Teams { - if team.IsOwnerTeam() { - team.Authorize = ACCESS_MODE_OWNER + for _, t := range repo.Owner.Teams { + if t.ID == ignTeamID { + continue + } + if t.IsOwnerTeam() { + t.Authorize = ACCESS_MODE_OWNER } - if err = team.getMembers(e); err != nil { - return fmt.Errorf("getMembers '%d': %v", team.ID, err) + if err = t.getMembers(e); err != nil { + return fmt.Errorf("getMembers '%d': %v", t.ID, err) } - for _, u := range team.Members { - accessMap[u.Id] = maxAccessMode(accessMap[u.Id], team.Authorize) + for _, m := range t.Members { + accessMap[m.Id] = maxAccessMode(accessMap[m.Id], t.Authorize) } } } - // FIXME: do corss-comparison so reduce deletions and additions to the minimum? - - minMode := ACCESS_MODE_READ - if !repo.IsPrivate { - minMode = ACCESS_MODE_WRITE - } - - newAccesses := make([]Access, 0, len(accessMap)) - for userID, mode := range accessMap { - if mode < minMode { - continue - } - newAccesses = append(newAccesses, Access{ - UserID: userID, - RepoID: repo.Id, - Mode: mode, - }) - } + return repo.refreshAccesses(e, accessMap) +} - // Delete old accesses and insert new ones for repository. - if _, err = e.Delete(&Access{RepoID: repo.Id}); err != nil { - return fmt.Errorf("delete old accesses: %v", err) - } else if _, err = e.Insert(newAccesses); err != nil { - return fmt.Errorf("insert new accesses: %v", err) +func (repo *Repository) recalculateAccesses(e Engine) error { + accessMap := make(map[int64]AccessMode, 20) + if err := repo.refreshCollaboratorAccesses(e, accessMap); err != nil { + return fmt.Errorf("refreshCollaboratorAccesses: %v", err) } - - return nil + return repo.refreshAccesses(e, accessMap) } // RecalculateAccesses recalculates all accesses for repository. diff --git a/models/org.go b/models/org.go index b47fbced5..2c2b9b635 100644 --- a/models/org.go +++ b/models/org.go @@ -85,6 +85,15 @@ func (org *User) RemoveMember(uid int64) error { return RemoveOrgUser(org.Id, uid) } +func (org *User) removeOrgRepo(e Engine, repoID int64) error { + return removeOrgRepo(e, org.Id, repoID) +} + +// RemoveOrgRepo removes all team-repository relations of organization. +func (org *User) RemoveOrgRepo(repoID int64) error { + return org.removeOrgRepo(x, repoID) +} + // IsOrgEmailUsed returns true if the e-mail has been used in organization account. func IsOrgEmailUsed(email string) (bool, error) { if len(email) == 0 { @@ -364,7 +373,7 @@ func RemoveOrgUser(orgId, uid int64) error { if _, err := sess.Id(ou.ID).Delete(ou); err != nil { sess.Rollback() return err - } else if _, err = sess.Exec("UPDATE `user` SET num_members = num_members - 1 WHERE id = ?", orgId); err != nil { + } else if _, err = sess.Exec("UPDATE `user` SET num_members=num_members-1 WHERE id = ?", orgId); err != nil { sess.Rollback() return err } @@ -496,7 +505,7 @@ func (t *Team) addRepository(e Engine, repo *Repository) (err error) { return fmt.Errorf("update team: %v", err) } - if err = repo.recalculateAccesses(e); err != nil { + if err = repo.recalculateTeamAccesses(e, 0); err != nil { return fmt.Errorf("recalculateAccesses: %v", err) } @@ -532,7 +541,7 @@ func (t *Team) AddRepository(repo *Repository) (err error) { return sess.Commit() } -func (t *Team) removeRepository(e Engine, repo *Repository) (err error) { +func (t *Team) removeRepository(e Engine, repo *Repository, recalculate bool) (err error) { if err = removeTeamRepo(e, t.ID, repo.Id); err != nil { return err } @@ -542,8 +551,11 @@ func (t *Team) removeRepository(e Engine, repo *Repository) (err error) { return err } - if err = repo.recalculateAccesses(e); err != nil { - return err + // Don't need to recalculate when delete a repository from organization. + if recalculate { + if err = repo.recalculateTeamAccesses(e, t.ID); err != nil { + return err + } } if err = t.getMembers(e); err != nil { @@ -582,7 +594,7 @@ func (t *Team) RemoveRepository(repoID int64) error { return err } - if err = t.removeRepository(sess, repo); err != nil { + if err = t.removeRepository(sess, repo, true); err != nil { return err } @@ -623,7 +635,7 @@ func NewTeam(t *Team) error { } // Update organization number of teams. - if _, err = sess.Exec("UPDATE `user` SET num_teams = num_teams + 1 WHERE id = ?", t.OrgID); err != nil { + if _, err = sess.Exec("UPDATE `user` SET num_teams=num_teams+1 WHERE id = ?", t.OrgID); err != nil { sess.Rollback() return err } @@ -683,18 +695,18 @@ func UpdateTeam(t *Team, authChanged bool) (err error) { t.LowerName = strings.ToLower(t.Name) if _, err = sess.Id(t.ID).AllCols().Update(t); err != nil { - return err + return fmt.Errorf("update: %v", err) } // Update access for team members if needed. if authChanged { if err = t.getRepositories(sess); err != nil { - return err + return fmt.Errorf("getRepositories:%v", err) } for _, repo := range t.Repos { - if err = repo.recalculateAccesses(sess); err != nil { - return err + if err = repo.recalculateTeamAccesses(sess, 0); err != nil { + return fmt.Errorf("recalculateTeamAccesses: %v", err) } } } @@ -707,8 +719,6 @@ func UpdateTeam(t *Team, authChanged bool) (err error) { func DeleteTeam(t *Team) error { if err := t.GetRepositories(); err != nil { return err - } else if err = t.GetMembers(); err != nil { - return err } // Get organization. @@ -725,7 +735,7 @@ func DeleteTeam(t *Team) error { // Delete all accesses. for _, repo := range t.Repos { - if err = repo.recalculateAccesses(sess); err != nil { + if err = repo.recalculateTeamAccesses(sess, t.ID); err != nil { return err } } @@ -850,7 +860,6 @@ func AddTeamMember(orgId, teamId, uid int64) error { OrgID: orgId, TeamID: teamId, } - if _, err = sess.Insert(tu); err != nil { return err } else if _, err = sess.Id(t.ID).Update(t); err != nil { @@ -859,7 +868,7 @@ func AddTeamMember(orgId, teamId, uid int64) error { // Give access to team repositories. for _, repo := range t.Repos { - if err = repo.recalculateAccesses(sess); err != nil { + if err = repo.recalculateTeamAccesses(sess, 0); err != nil { return err } } @@ -913,7 +922,6 @@ func removeTeamMember(e Engine, orgId, teamId, uid int64) error { OrgID: orgId, TeamID: teamId, } - if _, err := e.Delete(tu); err != nil { return err } else if _, err = e.Id(t.ID).AllCols().Update(t); err != nil { @@ -922,7 +930,7 @@ func removeTeamMember(e Engine, orgId, teamId, uid int64) error { // Delete access to team repositories. for _, repo := range t.Repos { - if err = repo.recalculateAccesses(e); err != nil { + if err = repo.recalculateTeamAccesses(e, 0); err != nil { return err } } @@ -1007,3 +1015,16 @@ func removeTeamRepo(e Engine, teamID, repoID int64) error { func RemoveTeamRepo(teamID, repoID int64) error { return removeTeamRepo(x, teamID, repoID) } + +func removeOrgRepo(e Engine, orgID, repoID int64) error { + _, err := e.Delete(&TeamRepo{ + OrgID: orgID, + RepoID: repoID, + }) + return err +} + +// RemoveOrgRepo removes all team-repository relations of given organization. +func RemoveOrgRepo(orgID, repoID int64) error { + return removeOrgRepo(x, orgID, repoID) +} diff --git a/models/repo.go b/models/repo.go index 0f7502909..65cd36864 100644 --- a/models/repo.go +++ b/models/repo.go @@ -544,11 +544,6 @@ func CreateRepository(u *User, name, desc, lang, license string, isPrivate, isMi // Give access to all members in owner team. if u.IsOrganization() { - if err = repo.recalculateAccesses(sess); err != nil { - return nil, err - } - - // Update owner team info and count. t, err := u.getOwnerTeam(sess) if err != nil { return nil, fmt.Errorf("getOwnerTeam: %v", err) @@ -556,12 +551,15 @@ func CreateRepository(u *User, name, desc, lang, license string, isPrivate, isMi return nil, fmt.Errorf("addRepository: %v", err) } } else { - if err = watchRepo(sess, u.Id, repo.Id, true); err != nil { - return nil, fmt.Errorf("watchRepo: %v", err) + // Organization called this in addRepository method. + if err = repo.recalculateAccesses(sess); err != nil { + return nil, fmt.Errorf("recalculateAccesses: %v", err) } } - if err = newRepoAction(sess, u, repo); err != nil { + if err = watchRepo(sess, u.Id, repo.Id, true); err != nil { + return nil, fmt.Errorf("watchRepo: %v", err) + } else if err = newRepoAction(sess, u, repo); err != nil { return nil, fmt.Errorf("newRepoAction: %v", err) } @@ -667,6 +665,27 @@ func TransferOwnership(u *User, newOwnerName string, repo *Repository) error { } } + // Remove old team-repository relations. + if owner.IsOrganization() { + if err = owner.getTeams(sess); err != nil { + return fmt.Errorf("getTeams: %v", err) + } + for _, t := range owner.Teams { + if !t.hasRepository(sess, repo.Id) { + continue + } + + t.NumRepos-- + if _, err := sess.Id(t.ID).AllCols().Update(t); err != nil { + return fmt.Errorf("decrease team repository count '%d': %v", t.ID, err) + } + } + + if err = owner.removeOrgRepo(sess, repo.Id); err != nil { + return fmt.Errorf("removeOrgRepo: %v", err) + } + } + if newOwner.IsOrganization() { t, err := newOwner.GetOwnerTeam() if err != nil { @@ -763,7 +782,7 @@ func DeleteRepository(uid, repoID int64, userName string) error { for _, t := range org.Teams { if !t.hasRepository(sess, repoID) { continue - } else if err = t.removeRepository(sess, repo); err != nil { + } else if err = t.removeRepository(sess, repo, false); err != nil { return err } } @@ -771,9 +790,9 @@ func DeleteRepository(uid, repoID int64, userName string) error { if _, err = sess.Delete(&Repository{Id: repoID}); err != nil { return err - } else if _, err := sess.Delete(&Access{RepoID: repo.Id}); err != nil { + } else if _, err = sess.Delete(&Access{RepoID: repo.Id}); err != nil { return err - } else if _, err := sess.Delete(&Action{RepoId: repo.Id}); err != nil { + } else if _, err = sess.Delete(&Action{RepoId: repo.Id}); err != nil { return err } else if _, err = sess.Delete(&Watch{RepoId: repoID}); err != nil { return err diff --git a/templates/.VERSION b/templates/.VERSION index 64dc6626d..0789cc30b 100644 --- a/templates/.VERSION +++ b/templates/.VERSION @@ -1 +1 @@ -0.5.15.0224 Beta \ No newline at end of file +0.5.16.0228 Beta \ No newline at end of file