WIP: Use intermediate table to compute user permissions

contrib/fixtures/fixture_generation.go

@@ -22,7 +22,7 @@ var (
 		name string
 	}{
 		{
-			models.GetYamlFixturesAccess, "access",
+			models.GetYamlFixturesUserRepoUnit, "user_repo_unit",
 		},
 	}
 	fixturesDir string

integrations/user_repo_unit_test.go

@@ -0,0 +1,20 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package integrations
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models"
+)
+
+// IMPORTANT: THIS FILE IS ONLY A BUILDING BLOCK TO HELP TEST THE FEATURE
+// DURING DEVELOPMENT. IT'S NOT INTENDED TO GO LIKE THIS IN THE FINAL
+// VERSION OF THE PR.
+
+func TestUserRepoUnit(t *testing.T) {
+
+	models.UserRepoUnitTest(t)
+}

models/access.go

@@ -61,10 +61,10 @@ func ParseAccessMode(permission string) AccessMode {
 	}
 }
 
-// Access represents the highest access level of a user to the repository. The only access type
-// that is not in this table is the real owner of a repository. In case of an organization
-// repository, the members of the owners team are in this table.
+// Access struct is deprecated
 type Access struct {
+	// FIXME: GAP: Remove Access from database
+
 	ID     int64 `xorm:"pk autoincr"`
 	UserID int64 `xorm:"UNIQUE(s)"`
 	RepoID int64 `xorm:"UNIQUE(s)"`
@@ -100,31 +100,30 @@ func accessLevel(e Engine, user *User, repo *Repository) (AccessMode, error) {
 	return a.Mode, nil
 }
 
-type repoAccess struct {
-	Access     `xorm:"extends"`
-	Repository `xorm:"extends"`
-}
-
-func (repoAccess) TableName() string {
-	return "access"
-}
-
-// GetRepositoryAccesses finds all repositories with their access mode where a user has access but does not own.
+// GetRepositoryAccesses finds all repositories with their access mode
+// where a user has any kind of **explicit** access but does not directly own.
 func (user *User) GetRepositoryAccesses() (map[*Repository]AccessMode, error) {
+	// A query that retrieves all repositories with their max
+	// access mode can be made, but unfortunately Xorm doesn't seem to
+	// support such complex queries (one that gets us both the repository
+	// and the mode from a sub-query).
+	// So, we first query for the list of repositories; later we will retrieve
+	// the best set of permissions for each one and relate each other.
 	rows, err := x.
-		Join("INNER", "repository", "repository.id = access.repo_id").
-		Where("access.user_id = ?", user.ID).
+		Where(accessibleRepositoryConditionExplicit(user, true)).
 		And("repository.owner_id <> ?", user.ID).
-		Rows(new(repoAccess))
+		Rows(new(Repository))
 	if err != nil {
 		return nil, err
 	}
 	defer rows.Close()
 
 	var repos = make(map[*Repository]AccessMode, 10)
+	var reposByID = make(map[int64]*Repository, 10)
 	var ownerCache = make(map[int64]*User, 10)
+
 	for rows.Next() {
-		var repo repoAccess
+		var repo Repository
 		err = rows.Scan(&repo)
 		if err != nil {
 			return nil, err
@@ -137,27 +136,69 @@ func (user *User) GetRepositoryAccesses() (map[*Repository]AccessMode, error) {
 			}
 			ownerCache[repo.OwnerID] = repo.Owner
 		}
+		// Temporary nil permission until we find out the correct one
+		repos[&repo] = AccessModeNone
+		reposByID[repo.ID] = &repo
+	}
+
+	rows.Close()
+
+	type bestAccessMode struct {
+		RepoID int64
+		Mode   AccessMode
+	}
+
+	rows, err = x.Table("user_repo_unit").
+		Join("INNER", "repository", "repository.id = user_repo_unit.repo_id").
+		Where(accessibleRepositoryCondition(user)).
+		And("repository.owner_id <> ?", user.ID).
+		Select("repo_id, max(mode) as mode").
+		GroupBy("repo_id").
+		Rows(new(bestAccessMode))
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
 
-		repos[&repo.Repository] = repo.Access.Mode
+	for rows.Next() {
+		var bestMode bestAccessMode
+		err = rows.Scan(&bestMode)
+		if err != nil {
+			return nil, err
+		}
+		if repo, ok := reposByID[bestMode.RepoID]; ok {
+			repos[repo] = bestMode.Mode
+		}
 	}
+
+	// Final pass, delete any repos from the map
+	// that have not been updated (e.g. might have lose
+	// access between the first and second queries)
+	for _, repo := range reposByID {
+		if repos[repo] == AccessModeNone {
+			delete(repos, repo)
+		}
+	}
+
 	return repos, nil
 }
 
 // GetAccessibleRepositories finds repositories which the user has access but does not own.
 // If limit is smaller than 1 means returns all found results.
 func (user *User) GetAccessibleRepositories(limit int) (repos []*Repository, _ error) {
-	sess := x.
-		Where("owner_id !=? ", user.ID).
+	// FIXME: GAP: Test this query
+	sess := x.Table(&Repository{}).
+		Where(accessibleRepositoryConditionExplicit(user, true)).
+		And("repository.owner_id <> ?", user.ID).
 		Desc("updated_unix")
 	if limit > 0 {
 		sess.Limit(limit)
 		repos = make([]*Repository, 0, limit)
 	} else {
 		repos = make([]*Repository, 0, 10)
 	}
-	return repos, sess.
-		Join("INNER", "access", "access.user_id = ? AND access.repo_id = repository.id", user.ID).
-		Find(&repos)
+
+	return repos, sess.Find(&repos)
 }
 
 func maxAccessMode(modes ...AccessMode) AccessMode {
@@ -170,167 +211,29 @@ func maxAccessMode(modes ...AccessMode) AccessMode {
 	return max
 }
 
-type userAccess struct {
-	User *User
-	Mode AccessMode
-}
-
-// updateUserAccess updates an access map so that user has at least mode
-func updateUserAccess(accessMap map[int64]*userAccess, user *User, mode AccessMode) {
-	if ua, ok := accessMap[user.ID]; ok {
-		ua.Mode = maxAccessMode(ua.Mode, mode)
-	} else {
-		accessMap[user.ID] = &userAccess{User: user, Mode: mode}
-	}
-}
-
-// FIXME: do cross-comparison so reduce deletions and additions to the minimum?
-func (repo *Repository) refreshAccesses(e Engine, accessMap map[int64]*userAccess) (err error) {
-	minMode := AccessModeRead
-	if !repo.IsPrivate {
-		minMode = AccessModeWrite
-	}
-
-	newAccesses := make([]Access, 0, len(accessMap))
-	for userID, ua := range accessMap {
-		if ua.Mode < minMode && !ua.User.IsRestricted {
-			continue
-		}
-
-		newAccesses = append(newAccesses, Access{
-			UserID: userID,
-			RepoID: repo.ID,
-			Mode:   ua.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
-}
-
-// refreshCollaboratorAccesses retrieves repository collaborations with their access modes.
-func (repo *Repository) refreshCollaboratorAccesses(e Engine, accessMap map[int64]*userAccess) error {
-	collaborators, err := repo.getCollaborators(e, ListOptions{})
-	if err != nil {
-		return fmt.Errorf("getCollaborations: %v", err)
-	}
-	for _, c := range collaborators {
-		updateUserAccess(accessMap, c.User, c.Collaboration.Mode)
-	}
-	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]*userAccess, 20)
-
-	if err = repo.getOwner(e); err != nil {
-		return err
-	} else if !repo.Owner.IsOrganization() {
-		return fmt.Errorf("owner is not an organization: %d", repo.OwnerID)
-	}
-
-	if err = repo.refreshCollaboratorAccesses(e, accessMap); err != nil {
-		return fmt.Errorf("refreshCollaboratorAccesses: %v", err)
-	}
-
-	if err = repo.Owner.getTeams(e); err != nil {
-		return err
-	}
-
-	for _, t := range repo.Owner.Teams {
-		if t.ID == ignTeamID {
-			continue
-		}
-
-		// Owner team gets owner access, and skip for teams that do not
-		// have relations with repository.
-		if t.IsOwnerTeam() {
-			t.Authorize = AccessModeOwner
-		} else if !t.hasRepository(e, repo.ID) {
-			continue
-		}
-
-		if err = t.getMembers(e); err != nil {
-			return fmt.Errorf("getMembers '%d': %v", t.ID, err)
-		}
-		for _, m := range t.Members {
-			updateUserAccess(accessMap, m, t.Authorize)
-		}
-	}
-
-	return repo.refreshAccesses(e, accessMap)
-}
-
-// recalculateUserAccess recalculates new access for a single user
-// Usable if we know access only affected one user
+// recalculateUserAccess recalculates repository access for a specific user
 func (repo *Repository) recalculateUserAccess(e Engine, uid int64) (err error) {
-	minMode := AccessModeRead
-	if !repo.IsPrivate {
-		minMode = AccessModeWrite
-	}
-
-	accessMode := AccessModeNone
-	collaborator, err := repo.getCollaboration(e, uid)
-	if err != nil {
-		return err
-	} else if collaborator != nil {
-		accessMode = collaborator.Mode
-	}
-
-	if err = repo.getOwner(e); err != nil {
-		return err
-	} else if repo.Owner.IsOrganization() {
-		var teams []Team
-		if err := e.Join("INNER", "team_repo", "team_repo.team_id = team.id").
-			Join("INNER", "team_user", "team_user.team_id = team.id").
-			Where("team.org_id = ?", repo.OwnerID).
-			And("team_repo.repo_id=?", repo.ID).
-			And("team_user.uid=?", uid).
-			Find(&teams); err != nil {
-			return err
-		}
-
-		for _, t := range teams {
-			if t.IsOwnerTeam() {
-				t.Authorize = AccessModeOwner
-			}
-
-			accessMode = maxAccessMode(accessMode, t.Authorize)
-		}
-	}
-
-	// Delete old user accesses and insert new one for repository.
-	if _, err = e.Delete(&Access{RepoID: repo.ID, UserID: uid}); err != nil {
-		return fmt.Errorf("delete old user accesses: %v", err)
-	} else if accessMode >= minMode {
-		if _, err = e.Insert(&Access{RepoID: repo.ID, UserID: uid, Mode: accessMode}); err != nil {
-			return fmt.Errorf("insert new user accesses: %v", err)
-		}
-	}
-	return nil
+	return RebuildUserIDRepoUnits(e, uid, repo)
 }
 
+// recalculateAccesses recalculates repository access for all users
 func (repo *Repository) recalculateAccesses(e Engine) error {
-	if repo.Owner.IsOrganization() {
-		return repo.recalculateTeamAccesses(e, 0)
-	}
-
-	accessMap := make(map[int64]*userAccess, 20)
-	if err := repo.refreshCollaboratorAccesses(e, accessMap); err != nil {
-		return fmt.Errorf("refreshCollaboratorAccesses: %v", err)
-	}
-	return repo.refreshAccesses(e, accessMap)
+	return RebuildRepoUnits(e, repo, -1)
 }
 
-// RecalculateAccesses recalculates all accesses for repository.
+// RecalculateAccesses recalculates repository access for all users
 func (repo *Repository) RecalculateAccesses() error {
 	return repo.recalculateAccesses(x)
 }
+
+// addTeamAccesses adds accesses for a team on the repository.
+func (repo *Repository) addTeamAccesses(e Engine, team *Team) error {
+	return AddTeamRepoUnits(e, team, repo)
+}
+
+// addTeamAccesses adds accesses for a team on the repository.
+func (repo *Repository) removeTeamAccesses(e Engine, teamID int64) error {
+	// There's no RemoveTeamRepoUnits() function due to how UserRepoUnit
+	// is built. We need to recalculate all accesses to this repository.
+	return RebuildRepoUnits(e, repo, teamID)
+}