From 76f89047183fc70a077ac37b4168d90539f12e09 Mon Sep 17 00:00:00 2001 From: Peter Smit Date: Fri, 23 Jan 2015 09:54:16 +0200 Subject: [PATCH 1/3] Introducing Collaboration Struct --- models/access.go | 20 ++++++ models/issue.go | 6 +- models/migrations/migrations.go | 98 ++++++++++++++++++++++++++++- models/models.go | 7 ++- models/repo.go | 105 ++++++++++++++++++-------------- routers/api/v1/repo.go | 21 ++++--- routers/repo/issue.go | 26 ++++---- routers/repo/setting.go | 57 ++++------------- routers/user/home.go | 14 +++-- 9 files changed, 230 insertions(+), 124 deletions(-) diff --git a/models/access.go b/models/access.go index 81aa43dc7..65f1d0032 100644 --- a/models/access.go +++ b/models/access.go @@ -78,3 +78,23 @@ func HasAccess(uname, repoName string, mode AccessType) (bool, error) { } return true, nil } + +// GetAccessibleRepositories finds all repositories where a user has access to, +// besides his own. +func (u *User) GetAccessibleRepositories() (map[*Repository]AccessType, error) { + accesses := make([]*Access, 0, 10) + if err := x.Find(&accesses, &Access{UserName: u.LowerName}); err != nil { + return nil, err + } + + repos := make(map[*Repository]AccessType, len(accesses)) + for _, access := range accesses { + repo, err := GetRepositoryByRef(access.RepoName) + if err != nil { + return nil, err + } + repos[repo] = access.Mode + } + + return repos, nil +} diff --git a/models/issue.go b/models/issue.go index c756e4975..8d0525d7c 100644 --- a/models/issue.go +++ b/models/issue.go @@ -282,10 +282,10 @@ type IssueUser struct { } // NewIssueUserPairs adds new issue-user pairs for new issue of repository. -func NewIssueUserPairs(rid, iid, oid, pid, aid int64, repoName string) (err error) { - iu := &IssueUser{IssueId: iid, RepoId: rid} +func NewIssueUserPairs(repo *Repository, iid, oid, pid, aid int64) (err error) { + iu := &IssueUser{IssueId: iid, RepoId: repo.Id} - us, err := GetCollaborators(repoName) + us, err := repo.GetCollaborators() if err != nil { return err } diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 3586e5d0b..43ec4e50b 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -2,6 +2,9 @@ package migrations import ( "errors" + "strconv" + "strings" + "time" "github.com/go-xorm/xorm" ) @@ -16,7 +19,9 @@ type Version struct { // This is a sequence of migrations. Add new migrations to the bottom of the list. // If you want to "retire" a migration, replace it with "expiredMigration" -var migrations = []migration{} +var migrations = []migration{ + accessToCollaboration, +} // Migrate database to current version func Migrate(x *xorm.Engine) error { @@ -29,6 +34,21 @@ func Migrate(x *xorm.Engine) error { if err != nil { return err } else if !has { + needsMigration, err := x.IsTableExist("user") + if err != nil { + return err + } + if needsMigration { + isEmpty, err := x.IsTableEmpty("user") + if err != nil { + return err + } + needsMigration = !isEmpty + } + if !needsMigration { + currentVersion.Version = int64(len(migrations)) + } + if _, err = x.InsertOne(currentVersion); err != nil { return err } @@ -51,3 +71,79 @@ func Migrate(x *xorm.Engine) error { func expiredMigration(x *xorm.Engine) error { return errors.New("You are migrating from a too old gogs version") } + +func mustParseInt64(in []byte) int64 { + i, err := strconv.ParseInt(string(in), 10, 64) + if err != nil { + i = 0 + } + return i +} + +func accessToCollaboration(x *xorm.Engine) error { + type Collaboration struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"UNIQUE(s) INDEX NOT NULL"` + UserID int64 `xorm:"UNIQUE(s) INDEX NOT NULL"` + Created time.Time `xorm:"CREATED"` + } + + x.Sync(new(Collaboration)) + + sql := `SELECT u.id AS uid, a.repo_name AS repo, a.mode AS mode FROM access a JOIN user u ON a.user_name=u.lower_name` + results, err := x.Query(sql) + if err != nil { + return err + } + + for _, result := range results { + userID := mustParseInt64(result["uid"]) + repoRefName := string(result["repo"]) + mode := mustParseInt64(result["mode"]) + + //Collaborators must have write access + if mode < 2 { + continue + } + + parts := strings.SplitN(repoRefName, "/", 2) + ownerName := parts[0] + repoName := parts[1] + + sql = `SELECT u.id as uid, ou.uid as memberid FROM user u LEFT JOIN org_user ou ON ou.org_id=u.id WHERE u.lower_name=?` + results, err := x.Query(sql, ownerName) + if err != nil { + return err + } + if len(results) < 1 { + continue + } + ownerID := mustParseInt64(results[0]["uid"]) + + for _, member := range results { + memberID := mustParseInt64(member["memberid"]) + // We can skip all cases that a user is member of the owning organization + if memberID == userID { + continue + } + } + + sql = `SELECT id FROM repository WHERE owner_id=? AND lower_name=?` + results, err = x.Query(sql, ownerID, repoName) + if err != nil { + return err + } + if len(results) < 1 { + continue + } + + repoID := results[0]["id"] + + sql = `INSERT INTO collaboration (user_id, repo_id) VALUES (?,?)` + _, err = x.Exec(sql, userID, repoID) + if err != nil { + return err + } + } + return nil +} diff --git a/models/models.go b/models/models.go index 55e7bf582..cf4e291cf 100644 --- a/models/models.go +++ b/models/models.go @@ -12,6 +12,7 @@ import ( "strings" _ "github.com/go-sql-driver/mysql" + "github.com/go-xorm/core" "github.com/go-xorm/xorm" _ "github.com/lib/pq" @@ -46,7 +47,7 @@ func init() { new(Issue), new(Comment), new(Attachment), new(IssueUser), new(Label), new(Milestone), new(Mirror), new(Release), new(LoginSource), new(Webhook), new(UpdateTask), new(HookTask), new(Team), new(OrgUser), new(TeamUser), - new(Notice), new(EmailAddress)) + new(Notice), new(EmailAddress), new(Collaboration)) } func LoadModelsConfig() { @@ -100,6 +101,7 @@ func NewTestEngine(x *xorm.Engine) (err error) { return fmt.Errorf("models.init(fail to connect to database): %v", err) } + x.SetMapper(core.GonicMapper{}) return x.Sync(tables...) } @@ -109,6 +111,8 @@ func SetEngine() (err error) { return fmt.Errorf("models.init(fail to connect to database): %v", err) } + x.SetMapper(core.GonicMapper{}) + // WARNING: for serv command, MUST remove the output to os.stdout, // so use log file to instead print to stdout. logPath := path.Join(setting.LogRootPath, "xorm.log") @@ -140,6 +144,7 @@ func NewEngine() (err error) { if err = x.StoreEngine("InnoDB").Sync2(tables...); err != nil { return fmt.Errorf("sync database struct error: %v\n", err) } + return nil } diff --git a/models/repo.go b/models/repo.go index 65689b6a1..663e227ae 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1065,71 +1065,74 @@ func GetRepositoryCount(user *User) (int64, error) { return x.Count(&Repository{OwnerId: user.Id}) } -// GetCollaboratorNames returns a list of user name of repository's collaborators. -func GetCollaboratorNames(repoName string) ([]string, error) { - accesses := make([]*Access, 0, 10) - if err := x.Find(&accesses, &Access{RepoName: strings.ToLower(repoName)}); err != nil { +// GetCollaborators returns the collaborators for a repository +func (r *Repository) GetCollaborators() ([]*User, error) { + collaborations := make([]*Collaboration, 0) + if err := x.Find(&collaborations, &Collaboration{RepoID: r.Id}); err != nil { return nil, err } - names := make([]string, len(accesses)) - for i := range accesses { - names[i] = accesses[i].UserName + users := make([]*User, len(collaborations)) + for i, c := range collaborations { + user, err := GetUserById(c.UserID) + if err != nil { + return nil, err + } + users[i] = user } - return names, nil + return users, nil } -// CollaborativeRepository represents a repository with collaborative information. -type CollaborativeRepository struct { - *Repository - CanPush bool -} +// Add collaborator and accompanying access +func (r *Repository) AddCollaborator(u *User) error { + collaboration := &Collaboration{RepoID: r.Id, UserID: u.Id} -// GetCollaborativeRepos returns a list of repositories that user is collaborator. -func GetCollaborativeRepos(uname string) ([]*CollaborativeRepository, error) { - uname = strings.ToLower(uname) - accesses := make([]*Access, 0, 10) - if err := x.Find(&accesses, &Access{UserName: uname}); err != nil { - return nil, err + has, err := x.Get(collaboration) + if err != nil { + return err + } + if has { + return nil } - repos := make([]*CollaborativeRepository, 0, 10) - for _, access := range accesses { - infos := strings.Split(access.RepoName, "/") - if infos[0] == uname { - continue - } - - u, err := GetUserByName(infos[0]) - if err != nil { - return nil, err - } + if _, err = x.InsertOne(collaboration); err != nil { + return err + } - repo, err := GetRepositoryByName(u.Id, infos[1]) - if err != nil { - return nil, err - } - repo.Owner = u - repos = append(repos, &CollaborativeRepository{repo, access.Mode == WRITABLE}) + if err = r.GetOwner(); err != nil { + return err } - return repos, nil + + return AddAccess(&Access{UserName: u.LowerName, RepoName: path.Join(r.Owner.LowerName, r.LowerName), Mode: WRITABLE}) } -// GetCollaborators returns a list of users of repository's collaborators. -func GetCollaborators(repoName string) (us []*User, err error) { - accesses := make([]*Access, 0, 10) - if err = x.Find(&accesses, &Access{RepoName: strings.ToLower(repoName)}); err != nil { - return nil, err +// Delete collaborator and accompanying access +func (r *Repository) DeleteCollaborator(u *User) error { + collaboration := &Collaboration{RepoID: r.Id, UserID: u.Id} + + if has, err := x.Delete(collaboration); err != nil || has == 0 { + return err } - us = make([]*User, len(accesses)) - for i := range accesses { - us[i], err = GetUserByName(accesses[i].UserName) + if err := r.GetOwner(); err != nil { + return err + } + + needDelete := true + if r.Owner.IsOrganization() { + auth, err := GetHighestAuthorize(r.Owner.Id, u.Id, r.Id, 0) if err != nil { - return nil, err + return err } + if auth > 0 { + needDelete = false + } + } + if needDelete { + return DeleteAccess(&Access{UserName: u.LowerName, RepoName: path.Join(r.Owner.LowerName, r.LowerName), Mode: WRITABLE}) } - return us, nil + + return nil } type SearchOption struct { @@ -1547,3 +1550,11 @@ func ForkRepository(u *User, oldRepo *Repository, name, desc string) (*Repositor return repo, nil } + +// A Collaboration is a relation between an individual and a repository +type Collaboration struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"UNIQUE(s) INDEX NOT NULL"` + UserID int64 `xorm:"UNIQUE(s) INDEX NOT NULL"` + Created time.Time `xorm:"CREATED"` +} diff --git a/routers/api/v1/repo.go b/routers/api/v1/repo.go index fbf9c73ea..469e4808f 100644 --- a/routers/api/v1/repo.go +++ b/routers/api/v1/repo.go @@ -237,28 +237,31 @@ func ListMyRepos(ctx *middleware.Context) { } numOwnRepos := len(ownRepos) - collaRepos, err := models.GetCollaborativeRepos(ctx.User.Name) + accessibleRepos, err := ctx.User.GetAccessibleRepositories() if err != nil { - ctx.JSON(500, &base.ApiJsonErr{"GetCollaborativeRepos: " + err.Error(), base.DOC_URL}) + ctx.JSON(500, &base.ApiJsonErr{"GetAccessibleRepositories: " + err.Error(), base.DOC_URL}) return } - repos := make([]*api.Repository, numOwnRepos+len(collaRepos)) + repos := make([]*api.Repository, numOwnRepos+len(accessibleRepos)) for i := range ownRepos { repos[i] = ToApiRepository(ctx.User, ownRepos[i], api.Permission{true, true, true}) } - for i := range collaRepos { - if err = collaRepos[i].GetOwner(); err != nil { + i := numOwnRepos + + for repo, access := range accessibleRepos { + if err = repo.GetOwner(); err != nil { ctx.JSON(500, &base.ApiJsonErr{"GetOwner: " + err.Error(), base.DOC_URL}) return } - j := i + numOwnRepos - repos[j] = ToApiRepository(collaRepos[i].Owner, collaRepos[i].Repository, api.Permission{false, collaRepos[i].CanPush, true}) + + repos[i] = ToApiRepository(repo.Owner, repo, api.Permission{false, access >= models.WRITABLE, true}) // FIXME: cache result to reduce DB query? - if collaRepos[i].Owner.IsOrganization() && collaRepos[i].Owner.IsOwnedBy(ctx.User.Id) { - repos[j].Permissions.Admin = true + if repo.Owner.IsOrganization() && repo.Owner.IsOwnedBy(ctx.User.Id) { + repos[i].Permissions.Admin = true } + i++ } ctx.JSON(200, &repos) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 999fd0a89..921348dbd 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -174,7 +174,7 @@ func CreateIssue(ctx *middleware.Context) { return } - us, err := models.GetCollaborators(strings.TrimPrefix(ctx.Repo.RepoLink, "/")) + us, err := ctx.Repo.Repository.GetCollaborators() if err != nil { ctx.Handle(500, "issue.CreateIssue(GetCollaborators)", err) return @@ -218,7 +218,7 @@ func CreateIssuePost(ctx *middleware.Context, form auth.CreateIssueForm) { return } - _, err = models.GetCollaborators(strings.TrimPrefix(ctx.Repo.RepoLink, "/")) + _, err = ctx.Repo.Repository.GetCollaborators() if err != nil { send(500, nil, err) return @@ -246,8 +246,8 @@ func CreateIssuePost(ctx *middleware.Context, form auth.CreateIssueForm) { if err := models.NewIssue(issue); err != nil { send(500, nil, err) return - } else if err := models.NewIssueUserPairs(issue.RepoId, issue.Id, ctx.Repo.Owner.Id, - ctx.User.Id, form.AssigneeId, ctx.Repo.Repository.Name); err != nil { + } else if err := models.NewIssueUserPairs(ctx.Repo.Repository, issue.Id, ctx.Repo.Owner.Id, + ctx.User.Id, form.AssigneeId); err != nil { send(500, nil, err) return } @@ -384,7 +384,7 @@ func ViewIssue(ctx *middleware.Context) { } // Get all collaborators. - ctx.Data["Collaborators"], err = models.GetCollaborators(strings.TrimPrefix(ctx.Repo.RepoLink, "/")) + ctx.Data["Collaborators"], err = ctx.Repo.Repository.GetCollaborators() if err != nil { ctx.Handle(500, "issue.CreateIssue(GetCollaborators)", err) return @@ -1122,18 +1122,18 @@ func IssueGetAttachment(ctx *middleware.Context) { // testing route handler for new issue ui page // todo : move to Issue() function -func Issues2(ctx *middleware.Context){ - ctx.HTML(200,"repo/issue2/list") +func Issues2(ctx *middleware.Context) { + ctx.HTML(200, "repo/issue2/list") } -func PullRequest2(ctx *middleware.Context){ - ctx.HTML(200,"repo/pr2/list") +func PullRequest2(ctx *middleware.Context) { + ctx.HTML(200, "repo/pr2/list") } -func Labels2(ctx *middleware.Context){ - ctx.HTML(200,"repo/issue2/labels") +func Labels2(ctx *middleware.Context) { + ctx.HTML(200, "repo/issue2/labels") } -func Milestones2(ctx *middleware.Context){ - ctx.HTML(200,"repo/milestone2/list") +func Milestones2(ctx *middleware.Context) { + ctx.HTML(200, "repo/milestone2/list") } diff --git a/routers/repo/setting.go b/routers/repo/setting.go index 33bf1eab2..a6f50d306 100644 --- a/routers/repo/setting.go +++ b/routers/repo/setting.go @@ -10,7 +10,6 @@ import ( "fmt" "strings" "time" - "path" "github.com/Unknwon/com" @@ -170,22 +169,12 @@ func SettingsCollaboration(ctx *middleware.Context) { ctx.Data["Title"] = ctx.Tr("repo.settings") ctx.Data["PageIsSettingsCollaboration"] = true - repoLink := path.Join(ctx.Repo.Owner.LowerName, ctx.Repo.Repository.LowerName) - if ctx.Req.Method == "POST" { name := strings.ToLower(ctx.Query("collaborator")) if len(name) == 0 || ctx.Repo.Owner.LowerName == name { ctx.Redirect(setting.AppSubUrl + ctx.Req.URL.Path) return } - has, err := models.HasAccess(name, repoLink, models.WRITABLE) - if err != nil { - ctx.Handle(500, "HasAccess", err) - return - } else if has { - ctx.Redirect(setting.AppSubUrl + ctx.Req.URL.Path) - return - } u, err := models.GetUserByName(name) if err != nil { @@ -205,9 +194,8 @@ func SettingsCollaboration(ctx *middleware.Context) { return } - if err = models.AddAccess(&models.Access{UserName: name, RepoName: repoLink, - Mode: models.WRITABLE}); err != nil { - ctx.Handle(500, "AddAccess", err) + if err = ctx.Repo.Repository.AddCollaborator(u); err != nil { + ctx.Handle(500, "AddCollaborator", err) return } @@ -226,50 +214,27 @@ func SettingsCollaboration(ctx *middleware.Context) { // Delete collaborator. remove := strings.ToLower(ctx.Query("remove")) if len(remove) > 0 && remove != ctx.Repo.Owner.LowerName { - needDelete := true - if ctx.User.IsOrganization() { - // Check if user belongs to a team that has access to this repository. - auth, err := models.GetHighestAuthorize(ctx.Repo.Owner.Id, ctx.User.Id, ctx.Repo.Repository.Id, 0) - if err != nil { - ctx.Handle(500, "GetHighestAuthorize", err) - return - } - if auth > 0 { - needDelete = false - } + u, err := models.GetUserByName(remove) + if err != nil { + ctx.Handle(500, "GetUserByName", err) + return } - - if needDelete { - if err := models.DeleteAccess(&models.Access{UserName: remove, RepoName: repoLink}); err != nil { - ctx.Handle(500, "DeleteAccess", err) - return - } + if err := ctx.Repo.Repository.DeleteCollaborator(u); err != nil { + ctx.Handle(500, "DeleteCollaborator", err) + return } ctx.Flash.Success(ctx.Tr("repo.settings.remove_collaborator_success")) ctx.Redirect(ctx.Repo.RepoLink + "/settings/collaboration") return } - names, err := models.GetCollaboratorNames(repoLink) + users, err := ctx.Repo.Repository.GetCollaborators() if err != nil { ctx.Handle(500, "GetCollaborators", err) return } - collaborators := make([]*models.User, 0, len(names)) - for _, name := range names { - u, err := models.GetUserByName(name) - if err != nil { - ctx.Handle(500, "GetUserByName", err) - return - } - // Does not show organization members. - if ctx.Repo.Owner.IsOrganization() && ctx.Repo.Owner.IsOrgMember(u.Id) { - continue - } - collaborators = append(collaborators, u) - } - ctx.Data["Collaborators"] = collaborators + ctx.Data["Collaborators"] = users ctx.HTML(200, COLLABORATION) } diff --git a/routers/user/home.go b/routers/user/home.go index 1aabe0877..5b02154c1 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -49,13 +49,19 @@ func Dashboard(ctx *middleware.Context) { } else { // Normal user. ctxUser = ctx.User - collaborates, err := models.GetCollaborativeRepos(ctxUser.Name) + collaborates, err := ctx.User.GetAccessibleRepositories() if err != nil { - ctx.Handle(500, "GetCollaborativeRepos", err) + ctx.Handle(500, "GetAccessibleRepositories", err) return } - ctx.Data["CollaborateCount"] = len(collaborates) - ctx.Data["CollaborativeRepos"] = collaborates + + repositories := make([]*models.Repository, 0, len(collaborates)) + for repo := range collaborates { + repositories = append(repositories, repo) + } + + ctx.Data["CollaborateCount"] = len(repositories) + ctx.Data["CollaborativeRepos"] = repositories } ctx.Data["ContextUser"] = ctxUser From bef38d9d3f01bf04404aa97d5528ff046da38950 Mon Sep 17 00:00:00 2001 From: Peter Smit Date: Wed, 4 Feb 2015 15:47:40 +0200 Subject: [PATCH 2/3] Fix collaboration migration code --- models/migrations/migrations.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 43ec4e50b..f0ed10b7a 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -90,7 +90,7 @@ func accessToCollaboration(x *xorm.Engine) error { x.Sync(new(Collaboration)) - sql := `SELECT u.id AS uid, a.repo_name AS repo, a.mode AS mode FROM access a JOIN user u ON a.user_name=u.lower_name` + sql := `SELECT u.id AS uid, a.repo_name AS repo, a.mode AS mode, a.created as created FROM access a JOIN user u ON a.user_name=u.lower_name` results, err := x.Query(sql) if err != nil { return err @@ -100,12 +100,14 @@ func accessToCollaboration(x *xorm.Engine) error { userID := mustParseInt64(result["uid"]) repoRefName := string(result["repo"]) mode := mustParseInt64(result["mode"]) + created := result["created"] //Collaborators must have write access if mode < 2 { continue } + // find owner of repository parts := strings.SplitN(repoRefName, "/", 2) ownerName := parts[0] repoName := parts[1] @@ -118,15 +120,24 @@ func accessToCollaboration(x *xorm.Engine) error { if len(results) < 1 { continue } + ownerID := mustParseInt64(results[0]["uid"]) + if ownerID == userID { + continue + } + // test if user is member of owning organization + isMember := false for _, member := range results { memberID := mustParseInt64(member["memberid"]) // We can skip all cases that a user is member of the owning organization if memberID == userID { - continue + isMember = true } } + if isMember { + continue + } sql = `SELECT id FROM repository WHERE owner_id=? AND lower_name=?` results, err = x.Query(sql, ownerID, repoName) @@ -139,8 +150,8 @@ func accessToCollaboration(x *xorm.Engine) error { repoID := results[0]["id"] - sql = `INSERT INTO collaboration (user_id, repo_id) VALUES (?,?)` - _, err = x.Exec(sql, userID, repoID) + sql = `INSERT INTO collaboration (user_id, repo_id, created) VALUES (?,?,?)` + _, err = x.Exec(sql, userID, repoID, created) if err != nil { return err } From fd1df86c44bfbd13b4df0a66840113b0d18695bc Mon Sep 17 00:00:00 2001 From: Peter Smit Date: Wed, 4 Feb 2015 16:08:55 +0200 Subject: [PATCH 3/3] Fix dashboard issue after collaboration migration --- models/access.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/models/access.go b/models/access.go index 65f1d0032..64bb92140 100644 --- a/models/access.go +++ b/models/access.go @@ -93,6 +93,10 @@ func (u *User) GetAccessibleRepositories() (map[*Repository]AccessType, error) { if err != nil { return nil, err } + err = repo.GetOwner() + if err != nil { + return nil, err + } repos[repo] = access.Mode }