Skip to content

Commit 73edac9

Browse files
lunnylunny
committed
Fix milestones too many SQL variables bug
1 parent bf847b9 commit 73edac9

File tree

3 files changed

+116
-101
lines changed

3 files changed

+116
-101
lines changed

‎models/issue_milestone.go

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -525,10 +525,12 @@ func DeleteMilestoneByRepoID(repoID, id int64) error {
525525
return sess.Commit()
526526
}
527527

528-
// CountMilestonesByRepoIDs map from repoIDs to number of milestones matching the options`
529-
func CountMilestonesByRepoIDs(repoIDs []int64, isClosed bool) (map[int64]int64, error) {
528+
// CountMilestones map from repo conditions to number of milestones matching the options`
529+
func CountMilestones(repoCond builder.Cond, isClosed bool) (map[int64]int64, error) {
530530
sess := x.Where("is_closed = ?", isClosed)
531-
sess.In("repo_id", repoIDs)
531+
if repoCond.IsValid() {
532+
sess.In("repo_id", builder.Select("id").From("repository").Where(repoCond))
533+
}
532534

533535
countsSlice := make([]*struct {
534536
RepoID int64
@@ -548,11 +550,21 @@ func CountMilestonesByRepoIDs(repoIDs []int64, isClosed bool) (map[int64]int64,
548550
return countMap, nil
549551
}
550552

551-
// GetMilestonesByRepoIDs returns a list of milestones of given repositories and status.
552-
func GetMilestonesByRepoIDs(repoIDs []int64, page int, isClosed bool, sortType string) (MilestoneList, error) {
553+
// CountMilestonesByRepoIDs map from repoIDs to number of milestones matching the options`
554+
func CountMilestonesByRepoIDs(repoIDs []int64, isClosed bool) (map[int64]int64, error) {
555+
return CountMilestones(
556+
builder.In("repo_id", repoIDs),
557+
isClosed,
558+
)
559+
}
560+
561+
// SearchMilestones search milestones
562+
func SearchMilestones(repoCond builder.Cond, page int, isClosed bool, sortType string) (MilestoneList, error) {
553563
miles := make([]*Milestone, 0, setting.UI.IssuePagingNum)
554564
sess := x.Where("is_closed = ?", isClosed)
555-
sess.In("repo_id", repoIDs)
565+
if repoCond.IsValid() {
566+
sess.In("repo_id", builder.Select("id").From("repository").Where(repoCond))
567+
}
556568
if page > 0 {
557569
sess = sess.Limit(setting.UI.IssuePagingNum, (page-1)*setting.UI.IssuePagingNum)
558570
}
@@ -574,25 +586,45 @@ func GetMilestonesByRepoIDs(repoIDs []int64, page int, isClosed bool, sortType s
574586
return miles, sess.Find(&miles)
575587
}
576588

589+
// GetMilestonesByRepoIDs returns a list of milestones of given repositories and status.
590+
func GetMilestonesByRepoIDs(repoIDs []int64, page int, isClosed bool, sortType string) (MilestoneList, error) {
591+
return SearchMilestones(
592+
builder.In("repo_id", repoIDs),
593+
page,
594+
isClosed,
595+
sortType,
596+
)
597+
}
598+
577599
// MilestonesStats represents milestone statistic information.
578600
type MilestonesStats struct {
579601
OpenCount, ClosedCount int64
580602
}
581603

604+
// Total returns the total counts of milestones
605+
func (m MilestonesStats) Total() int64 {
606+
return m.OpenCount + m.ClosedCount
607+
}
608+
582609
// GetMilestonesStats returns milestone statistic information for dashboard by given conditions.
583-
func GetMilestonesStats(userRepoIDs []int64) (*MilestonesStats, error) {
610+
func GetMilestonesStats(repoCond builder.Cond) (*MilestonesStats, error) {
584611
var err error
585612
stats := &MilestonesStats{}
586613

587-
stats.OpenCount, err = x.Where("is_closed = ?", false).
588-
And(builder.In("repo_id", userRepoIDs)).
589-
Count(new(Milestone))
614+
sess := x.Where("is_closed = ?", false)
615+
if repoCond.IsValid() {
616+
sess.And(builder.In("repo_id", builder.Select("id").From("repository").Where(repoCond)))
617+
}
618+
stats.OpenCount, err = sess.Count(new(Milestone))
590619
if err != nil {
591620
return nil, err
592621
}
593-
stats.ClosedCount, err = x.Where("is_closed = ?", true).
594-
And(builder.In("repo_id", userRepoIDs)).
595-
Count(new(Milestone))
622+
623+
sess = x.Where("is_closed = ?", true)
624+
if repoCond.IsValid() {
625+
sess.And(builder.In("repo_id", builder.Select("id").From("repository").Where(repoCond)))
626+
}
627+
stats.ClosedCount, err = sess.Count(new(Milestone))
596628
if err != nil {
597629
return nil, err
598630
}

‎models/repo_list.go

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,11 @@ func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond {
301301
// it returns results in given range and number of total results.
302302
func SearchRepository(opts *SearchRepoOptions) (RepositoryList, int64, error) {
303303
cond := SearchRepositoryCondition(opts)
304+
return SearchRepositoryByCondition(opts, cond, true)
305+
}
304306

307+
// SearchRepositoryByCondition search repositories by condition
308+
func SearchRepositoryByCondition(opts *SearchRepoOptions, cond builder.Cond, loadAttributes bool) (RepositoryList, int64, error) {
305309
if opts.Page <= 0 {
306310
opts.Page = 1
307311
}
@@ -326,16 +330,18 @@ func SearchRepository(opts *SearchRepoOptions) (RepositoryList, int64, error) {
326330
}
327331

328332
repos := make(RepositoryList, 0, opts.PageSize)
329-
if err = sess.
330-
Where(cond).
331-
OrderBy(opts.OrderBy.String()).
332-
Limit(opts.PageSize, (opts.Page-1)*opts.PageSize).
333-
Find(&repos); err != nil {
333+
sess.Where(cond).OrderBy(opts.OrderBy.String())
334+
if opts.PageSize > 0 {
335+
sess.Limit(opts.PageSize, (opts.Page-1)*opts.PageSize)
336+
}
337+
if err = sess.Find(&repos); err != nil {
334338
return nil, 0, fmt.Errorf("Repo: %v", err)
335339
}
336340

337-
if err = repos.loadAttributes(sess); err != nil {
338-
return nil, 0, fmt.Errorf("LoadAttributes: %v", err)
341+
if loadAttributes {
342+
if err = repos.loadAttributes(sess); err != nil {
343+
return nil, 0, fmt.Errorf("LoadAttributes: %v", err)
344+
}
339345
}
340346

341347
return repos, count, nil

‎routers/user/home.go

Lines changed: 58 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import (
2525

2626
"github.com/keybase/go-crypto/openpgp"
2727
"github.com/keybase/go-crypto/openpgp/armor"
28-
"github.com/unknwon/com"
28+
"xorm.io/builder"
2929
)
3030

3131
const (
@@ -173,135 +173,112 @@ func Milestones(ctx *context.Context) {
173173
return
174174
}
175175

176-
sortType := ctx.Query("sort")
177-
page := ctx.QueryInt("page")
176+
var (
177+
repoOpts = models.SearchRepoOptions{
178+
Actor: ctxUser,
179+
OwnerID: ctxUser.ID,
180+
Private: true,
181+
AllPublic: false, // Include also all public repositories of users and public organisations
182+
AllLimited: false, // Include also all public repositories of limited organisations
183+
}
184+
185+
userRepoCond = models.SearchRepositoryCondition(&repoOpts) // all repo condition user could visit
186+
repoCond = userRepoCond
187+
repoIDs []int64
188+
189+
reposQuery = ctx.Query("repos")
190+
isShowClosed = ctx.Query("state") == "closed"
191+
sortType = ctx.Query("sort")
192+
page = ctx.QueryInt("page")
193+
)
194+
178195
if page <= 1 {
179196
page = 1
180197
}
181198

182-
reposQuery := ctx.Query("repos")
183-
isShowClosed := ctx.Query("state") == "closed"
184-
185-
// Get repositories.
186-
var err error
187-
var userRepoIDs []int64
188-
if ctxUser.IsOrganization() {
189-
env, err := ctxUser.AccessibleReposEnv(ctx.User.ID)
190-
if err != nil {
191-
ctx.ServerError("AccessibleReposEnv", err)
192-
return
193-
}
194-
userRepoIDs, err = env.RepoIDs(1, ctxUser.NumRepos)
195-
if err != nil {
196-
ctx.ServerError("env.RepoIDs", err)
197-
return
198-
}
199-
userRepoIDs, err = models.FilterOutRepoIdsWithoutUnitAccess(ctx.User, userRepoIDs, models.UnitTypeIssues, models.UnitTypePullRequests)
200-
if err != nil {
201-
ctx.ServerError("FilterOutRepoIdsWithoutUnitAccess", err)
202-
return
203-
}
204-
} else {
205-
userRepoIDs, err = ctxUser.GetAccessRepoIDs(models.UnitTypeIssues, models.UnitTypePullRequests)
206-
if err != nil {
207-
ctx.ServerError("ctxUser.GetAccessRepoIDs", err)
208-
return
209-
}
210-
}
211-
if len(userRepoIDs) == 0 {
212-
userRepoIDs = []int64{-1}
213-
}
214-
215-
var repoIDs []int64
216199
if len(reposQuery) != 0 {
217200
if issueReposQueryPattern.MatchString(reposQuery) {
218201
// remove "[" and "]" from string
219202
reposQuery = reposQuery[1 : len(reposQuery)-1]
220203
//for each ID (delimiter ",") add to int to repoIDs
221-
reposSet := false
204+
222205
for _, rID := range strings.Split(reposQuery, ",") {
223206
// Ensure nonempty string entries
224207
if rID != "" && rID != "0" {
225-
reposSet = true
226208
rIDint64, err := strconv.ParseInt(rID, 10, 64)
227209
// If the repo id specified by query is not parseable or not accessible by user, just ignore it.
228-
if err == nil && com.IsSliceContainsInt64(userRepoIDs, rIDint64) {
210+
if err == nil {
229211
repoIDs = append(repoIDs, rIDint64)
230212
}
231213
}
232214
}
233-
if reposSet && len(repoIDs) == 0 {
234-
// force an empty result
235-
repoIDs = []int64{-1}
215+
if len(repoIDs) > 0 {
216+
repoCond = repoCond.And(builder.In("id", repoIDs))
236217
}
237218
} else {
238219
log.Warn("issueReposQueryPattern not match with query")
239220
}
240221
}
241222

242-
if len(repoIDs) == 0 {
243-
repoIDs = userRepoIDs
244-
}
245-
246-
counts, err := models.CountMilestonesByRepoIDs(userRepoIDs, isShowClosed)
223+
counts, err := models.CountMilestones(userRepoCond, isShowClosed)
247224
if err != nil {
248225
ctx.ServerError("CountMilestonesByRepoIDs", err)
249226
return
250227
}
251228

252-
milestones, err := models.GetMilestonesByRepoIDs(repoIDs, page, isShowClosed, sortType)
229+
milestones, err := models.SearchMilestones(repoCond, page, isShowClosed, sortType)
253230
if err != nil {
254231
ctx.ServerError("GetMilestonesByRepoIDs", err)
255232
return
256233
}
257234

258-
showReposMap := make(map[int64]*models.Repository, len(counts))
259-
for rID := range counts {
260-
if rID == -1 {
261-
break
262-
}
263-
repo, err := models.GetRepositoryByID(rID)
264-
if err != nil {
265-
if models.IsErrRepoNotExist(err) {
266-
ctx.NotFound("GetRepositoryByID", err)
267-
return
268-
} else if err != nil {
269-
ctx.ServerError("GetRepositoryByID", fmt.Errorf("[%d]%v", rID, err))
270-
return
271-
}
272-
}
273-
showReposMap[rID] = repo
235+
showRepos, _, err := models.SearchRepositoryByCondition(&repoOpts, userRepoCond, false)
236+
if err != nil {
237+
ctx.ServerError("SearchRepositoryByCondition", err)
238+
return
274239
}
275240

276-
showRepos := models.RepositoryListOfMap(showReposMap)
277241
sort.Sort(showRepos)
278-
if err = showRepos.LoadAttributes(); err != nil {
279-
ctx.ServerError("LoadAttributes", err)
280-
return
281-
}
282242

283-
for _, m := range milestones {
284-
m.Repo = showReposMap[m.RepoID]
285-
m.RenderedContent = string(markdown.Render([]byte(m.Content), m.Repo.Link(), m.Repo.ComposeMetas()))
286-
if m.Repo.IsTimetrackerEnabled() {
287-
err := m.LoadTotalTrackedTime()
243+
for i := 0; i < len(milestones); {
244+
for _, repo := range showRepos {
245+
if milestones[i].RepoID == repo.ID {
246+
milestones[i].Repo = repo
247+
break
248+
}
249+
}
250+
if milestones[i].Repo == nil {
251+
log.Warn("Cannot find milestone %d 's repository %d", milestones[i].ID, milestones[i].RepoID)
252+
milestones = append(milestones[:i], milestones[i+1:]...)
253+
continue
254+
}
255+
256+
milestones[i].RenderedContent = string(markdown.Render([]byte(milestones[i].Content), milestones[i].Repo.Link(), milestones[i].Repo.ComposeMetas()))
257+
if milestones[i].Repo.IsTimetrackerEnabled() {
258+
err := milestones[i].LoadTotalTrackedTime()
288259
if err != nil {
289260
ctx.ServerError("LoadTotalTrackedTime", err)
290261
return
291262
}
292263
}
264+
i++
293265
}
294266

295-
milestoneStats, err := models.GetMilestonesStats(repoIDs)
267+
milestoneStats, err := models.GetMilestonesStats(repoCond)
296268
if err != nil {
297269
ctx.ServerError("GetMilestoneStats", err)
298270
return
299271
}
300272

301-
totalMilestoneStats, err := models.GetMilestonesStats(userRepoIDs)
302-
if err != nil {
303-
ctx.ServerError("GetMilestoneStats", err)
304-
return
273+
var totalMilestoneStats *models.MilestonesStats
274+
if len(repoIDs) == 0 {
275+
totalMilestoneStats = milestoneStats
276+
} else {
277+
totalMilestoneStats, err = models.GetMilestonesStats(userRepoCond)
278+
if err != nil {
279+
ctx.ServerError("GetMilestoneStats", err)
280+
return
281+
}
305282
}
306283

307284
var pagerCount int
@@ -320,7 +297,7 @@ func Milestones(ctx *context.Context) {
320297
ctx.Data["Counts"] = counts
321298
ctx.Data["MilestoneStats"] = milestoneStats
322299
ctx.Data["SortType"] = sortType
323-
if len(repoIDs) != len(userRepoIDs) {
300+
if milestoneStats.Total() != totalMilestoneStats.Total() {
324301
ctx.Data["RepoIDs"] = repoIDs
325302
}
326303
ctx.Data["IsShowClosed"] = isShowClosed

0 commit comments

Comments
 (0)