From 016287ab64b502e410487896398402f3bd25a5aa Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 9 Mar 2022 00:16:59 +0100 Subject: [PATCH 1/4] xorm builder --- models/repo.go | 56 +++++++++++++++++++++++--------------------------- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/models/repo.go b/models/repo.go index 53199bcca355f..e812eb0bff17d 100644 --- a/models/repo.go +++ b/models/repo.go @@ -218,50 +218,46 @@ func getReviewers(ctx context.Context, repo *repo_model.Repository, doerID, post return nil, err } - var users []*user_model.User - e := db.GetEngine(ctx) + cond := builder.NewCond() + users := make([]*user_model.User, 0, 8) if repo.IsPrivate || repo.Owner.Visibility == api.VisibleTypePrivate { // This a private repository: // Anyone who can read the repository is a requestable reviewer - if err := e. - SQL("SELECT * FROM `user` WHERE id in ("+ - "SELECT user_id FROM `access` WHERE repo_id = ? AND mode >= ? AND user_id != ?"+ // private org repos - ") ORDER BY name", - repo.ID, perm.AccessModeRead, - posterID). - Find(&users); err != nil { - return nil, err - } + + cond = cond.And(builder.In("`user`.id", + builder.Select("user_id").From("access").Where( + builder.Eq{"repo_id": repo.ID}. + And(builder.Neq{"user_id": posterID}). + And(builder.Gte{"mode": perm.AccessModeRead}), + ), + )) if repo.Owner.Type == user_model.UserTypeIndividual && repo.Owner.ID != posterID { // as private *user* repos don't generate an entry in the `access` table, // the owner of a private repo needs to be explicitly added. - users = append(users, repo.Owner) + cond = cond.Or(builder.Eq{"id": repo.Owner.ID}) } - return users, nil + return users, db.GetEngine(ctx).Where(cond).OrderBy("name").Find(&users) } // This is a "public" repository: // Any user that has read access, is a watcher or organization member can be requested to review - if err := e. - SQL("SELECT * FROM `user` WHERE id IN ( "+ - "SELECT user_id FROM `access` WHERE repo_id = ? AND mode >= ? "+ - "UNION "+ - "SELECT user_id FROM `watch` WHERE repo_id = ? AND mode IN (?, ?) "+ - "UNION "+ - "SELECT uid AS user_id FROM `org_user` WHERE org_id = ? "+ - ") AND id != ? ORDER BY name", - repo.ID, perm.AccessModeRead, - repo.ID, repo_model.WatchModeNormal, repo_model.WatchModeAuto, - repo.OwnerID, - posterID). - Find(&users); err != nil { - return nil, err - } - - return users, nil + cond = cond.And(builder.And(builder.In("`user`.id", + builder.Select("user_id").From("access"). + Where(builder.Eq{"repo_id": repo.ID}. + And(builder.Gte{"mode": perm.AccessModeRead})), + ).Or(builder.In("`user`.id", + builder.Select("user_id").From("watch"). + Where(builder.Eq{"repo_id": repo.ID}. + And(builder.In("mode", repo_model.WatchModeNormal, repo_model.WatchModeAuto))), + ).Or(builder.In("`user`.id", + builder.Select("uid").From("org_user"). + Where(builder.Eq{"org_id": repo.OwnerID}), + ))))).And(builder.Neq{"`user`.id": posterID}) + + return users, db.GetEngine(ctx).Where(cond).OrderBy("name").Find(&users) } // GetReviewers get all users can be requested to review: From c0e210723a959097b3dffd598c3a9decc7b939d5 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 9 Mar 2022 00:20:59 +0100 Subject: [PATCH 2/4] dedup code --- models/repo.go | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/models/repo.go b/models/repo.go index e812eb0bff17d..c885b482aa084 100644 --- a/models/repo.go +++ b/models/repo.go @@ -219,7 +219,6 @@ func getReviewers(ctx context.Context, repo *repo_model.Repository, doerID, post } cond := builder.NewCond() - users := make([]*user_model.User, 0, 8) if repo.IsPrivate || repo.Owner.Visibility == api.VisibleTypePrivate { // This a private repository: @@ -239,24 +238,24 @@ func getReviewers(ctx context.Context, repo *repo_model.Repository, doerID, post cond = cond.Or(builder.Eq{"id": repo.Owner.ID}) } - return users, db.GetEngine(ctx).Where(cond).OrderBy("name").Find(&users) + } else { + // This is a "public" repository: + // Any user that has read access, is a watcher or organization member can be requested to review + cond = cond.And(builder.And(builder.In("`user`.id", + builder.Select("user_id").From("access"). + Where(builder.Eq{"repo_id": repo.ID}. + And(builder.Gte{"mode": perm.AccessModeRead})), + ).Or(builder.In("`user`.id", + builder.Select("user_id").From("watch"). + Where(builder.Eq{"repo_id": repo.ID}. + And(builder.In("mode", repo_model.WatchModeNormal, repo_model.WatchModeAuto))), + ).Or(builder.In("`user`.id", + builder.Select("uid").From("org_user"). + Where(builder.Eq{"org_id": repo.OwnerID}), + ))))).And(builder.Neq{"`user`.id": posterID}) } - // This is a "public" repository: - // Any user that has read access, is a watcher or organization member can be requested to review - cond = cond.And(builder.And(builder.In("`user`.id", - builder.Select("user_id").From("access"). - Where(builder.Eq{"repo_id": repo.ID}. - And(builder.Gte{"mode": perm.AccessModeRead})), - ).Or(builder.In("`user`.id", - builder.Select("user_id").From("watch"). - Where(builder.Eq{"repo_id": repo.ID}. - And(builder.In("mode", repo_model.WatchModeNormal, repo_model.WatchModeAuto))), - ).Or(builder.In("`user`.id", - builder.Select("uid").From("org_user"). - Where(builder.Eq{"org_id": repo.OwnerID}), - ))))).And(builder.Neq{"`user`.id": posterID}) - + users := make([]*user_model.User, 0, 8) return users, db.GetEngine(ctx).Where(cond).OrderBy("name").Find(&users) } From eb0f1f10ce0aefe71da7366f109e95e6656ae4b1 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 9 Mar 2022 00:25:24 +0100 Subject: [PATCH 3/4] more dedup --- models/repo.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/models/repo.go b/models/repo.go index c885b482aa084..1bcf7eabf2421 100644 --- a/models/repo.go +++ b/models/repo.go @@ -218,7 +218,7 @@ func getReviewers(ctx context.Context, repo *repo_model.Repository, doerID, post return nil, err } - cond := builder.NewCond() + cond := builder.And(builder.Neq{"`user`.id": posterID}) if repo.IsPrivate || repo.Owner.Visibility == api.VisibleTypePrivate { // This a private repository: @@ -227,7 +227,6 @@ func getReviewers(ctx context.Context, repo *repo_model.Repository, doerID, post cond = cond.And(builder.In("`user`.id", builder.Select("user_id").From("access").Where( builder.Eq{"repo_id": repo.ID}. - And(builder.Neq{"user_id": posterID}). And(builder.Gte{"mode": perm.AccessModeRead}), ), )) @@ -252,7 +251,7 @@ func getReviewers(ctx context.Context, repo *repo_model.Repository, doerID, post ).Or(builder.In("`user`.id", builder.Select("uid").From("org_user"). Where(builder.Eq{"org_id": repo.OwnerID}), - ))))).And(builder.Neq{"`user`.id": posterID}) + ))))) } users := make([]*user_model.User, 0, 8) From ac6c38488bf4760e7d7e2e0123a5ef84e472443e Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 10 Mar 2022 17:17:41 +0100 Subject: [PATCH 4/4] more explicite id Co-authored-by: zeripath --- models/repo.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/repo.go b/models/repo.go index 1bcf7eabf2421..d20e5f81d31be 100644 --- a/models/repo.go +++ b/models/repo.go @@ -234,7 +234,7 @@ func getReviewers(ctx context.Context, repo *repo_model.Repository, doerID, post if repo.Owner.Type == user_model.UserTypeIndividual && repo.Owner.ID != posterID { // as private *user* repos don't generate an entry in the `access` table, // the owner of a private repo needs to be explicitly added. - cond = cond.Or(builder.Eq{"id": repo.Owner.ID}) + cond = cond.Or(builder.Eq{"`user`.id": repo.Owner.ID}) } } else {