dashboard: refactor bisection job querying

This change is necessary for further development.

Query all bisection jobs for a bug and then explicitly pick the most
suitable one for display on the bug page / sending a notification.

Don't take any unfinished and manually invalidated jobs. Later we'll also
skip all cross tree bisections to only leave the result worth displaying
to the user.

As a side effect of the changes, display cause/fix bisection history if
there's been more than 1 attempt.

3 files changed, 177 insertions, 138 deletions

diff --git a/dashboard/app/jobs.go b/dashboard/app/jobs.go
index bb9e98b88..5e97099ab 100644
--- a/dashboard/app/jobs.go
+++ b/dashboard/app/jobs.go
@@ -1554,3 +1554,105 @@ func uniqueBugs(inBugs []*Bug, inKeys []*db.Key) ([]*Bug, []*db.Key) {
 	}
 	return bugs, keys
 }
+
+type bugJobs struct {
+	list []*bugJob
+}
+
+type bugJob struct {
+	bug      *Bug
+	job      *Job
+	key      *db.Key
+	crash    *Crash
+	crashKey *db.Key
+	build    *Build
+}
+
+func queryBugJobs(c context.Context, bug *Bug, jobType JobType) (*bugJobs, error) {
+	// Just in case.
+	const limitJobs = 25
+	var jobs []*Job
+	jobKeys, err := db.NewQuery("Job").
+		Ancestor(bug.key(c)).
+		Filter("Type=", jobType).
+		Order("-Finished").
+		Limit(limitJobs).
+		GetAll(c, &jobs)
+	if err != nil {
+		return nil, fmt.Errorf("failed to fetch bug jobs: %w", err)
+	}
+	bugKey := bug.key(c)
+	ret := &bugJobs{}
+	for i := range jobs {
+		job := jobs[i]
+		var crashKey *db.Key
+		if job.CrashID != 0 {
+			crashKey = db.NewKey(c, "Crash", "", job.CrashID, bugKey)
+		}
+		ret.list = append(ret.list, &bugJob{
+			bug:      bug,
+			job:      job,
+			key:      jobKeys[i],
+			crashKey: crashKey,
+		})
+	}
+	return ret, nil
+}
+
+func queryBestBisection(c context.Context, bug *Bug, jobType JobType) (*bugJob, error) {
+	jobs, err := queryBugJobs(c, bug, jobType)
+	if err != nil {
+		return nil, err
+	}
+	return jobs.bestBisection(), nil
+}
+
+// Find the most representative bisection result.
+func (b *bugJobs) bestBisection() *bugJob {
+	// Let's take the most recent finished one.
+	for _, j := range b.list {
+		if !j.job.IsFinished() {
+			continue
+		}
+		if j.job.InvalidatedBy != "" {
+			continue
+		}
+		return j
+	}
+	return nil
+}
+
+func (b *bugJobs) all() []*bugJob {
+	return b.list
+}
+
+func (j *bugJob) load(c context.Context) error {
+	err := j.loadCrash(c)
+	if err != nil {
+		return fmt.Errorf("failed to load crash: %w", err)
+	}
+	return j.loadBuild(c)
+}
+
+func (j *bugJob) loadCrash(c context.Context) error {
+	if j.crash != nil {
+		return nil
+	}
+	j.crash = new(Crash)
+	return db.Get(c, j.crashKey, j.crash)
+}
+
+func (j *bugJob) loadBuild(c context.Context) error {
+	if j.build != nil {
+		return nil
+	}
+	err := j.loadCrash(c)
+	if err != nil {
+		return fmt.Errorf("failed to load crash: %w", err)
+	}
+	j.build, err = loadBuild(c, j.bug.Namespace, j.crash.BuildID)
+	if err != nil {
+		return err
+	}
+	return nil
+}
diff --git a/dashboard/app/main.go b/dashboard/app/main.go
index 80a730164..d7655fd47 100644
--- a/dashboard/app/main.go
+++ b/dashboard/app/main.go
@@ -792,16 +792,24 @@ func handleBug(c context.Context, w http.ResponseWriter, r *http.Request) error
 			Value: similar,
 		})
 	}
+	causeBisections, err := queryBugJobs(c, bug, JobBisectCause)
+	if err != nil {
+		return fmt.Errorf("failed to load cause bisections: %w", err)
+	}
 	var bisectCause *uiJob
 	if bug.BisectCause > BisectPending {
-		bisectCause, err = getUIJob(c, bug, JobBisectCause)
+		bisectCause, err = causeBisections.uiBestBisection(c)
 		if err != nil {
 			return err
 		}
 	}
+	fixBisections, err := queryBugJobs(c, bug, JobBisectFix)
+	if err != nil {
+		return fmt.Errorf("failed to load cause bisections: %w", err)
+	}
 	var bisectFix *uiJob
 	if bug.BisectFix > BisectPending {
-		bisectFix, err = getUIJob(c, bug, JobBisectFix)
+		bisectFix, err = fixBisections.uiBestBisection(c)
 		if err != nil {
 			return err
 		}
@@ -840,25 +848,25 @@ func handleBug(c context.Context, w http.ResponseWriter, r *http.Request) error
 	// - no fix bisections have been performed on the bug
 	// - fix bisection was performed but resulted in a crash on HEAD
 	// - there have been infrastructure problems during the job execution
-	if bug.BisectFix == BisectNot {
-		fixBisections, err := loadBisectionsForBug(c, bug, JobBisectFix)
+	if len(fixBisections.all()) > 1 || len(fixBisections.all()) > 0 && bisectFix == nil {
+		uiList, err := fixBisections.uiAll(c)
 		if err != nil {
 			return err
 		}
-		if len(fixBisections) != 0 {
+		if len(uiList) != 0 {
 			data.Sections = append(data.Sections, makeCollapsibleBugJobs(
-				"Fix bisection attempts", fixBisections))
+				"Fix bisection attempts", uiList))
 		}
 	}
 	// Similarly, a cause bisection can be repeated if there were infrastructure problems.
-	if bug.BisectCause == BisectNot {
-		causeBisections, err := loadBisectionsForBug(c, bug, JobBisectCause)
+	if len(causeBisections.all()) > 1 || len(causeBisections.all()) > 0 && bisectCause == nil {
+		uiList, err := causeBisections.uiAll(c)
 		if err != nil {
 			return err
 		}
-		if len(causeBisections) != 0 {
+		if len(uiList) != 0 {
 			data.Sections = append(data.Sections, makeCollapsibleBugJobs(
-				"Cause bisection attempts", causeBisections))
+				"Cause bisection attempts", uiList))
 		}
 	}
 	if isJSONRequested(r) {
@@ -1063,18 +1071,6 @@ func findBugByID(c context.Context, r *http.Request) (*Bug, error) {
 	return nil, fmt.Errorf("mandatory parameter id/extid is missing")
 }
 
-func getUIJob(c context.Context, bug *Bug, jobType JobType) (*uiJob, error) {
-	job, crash, jobKey, _, err := loadBisectJob(c, bug, jobType)
-	if err != nil {
-		return nil, err
-	}
-	build, err := loadBuild(c, bug.Namespace, crash.BuildID)
-	if err != nil {
-		return nil, err
-	}
-	return makeUIJob(c, job, jobKey, bug, crash, build), nil
-}
-
 func handleSubsystemsList(c context.Context, w http.ResponseWriter, r *http.Request) error {
 	hdr, err := commonHeader(c, r, w, "")
 	if err != nil {
@@ -1683,31 +1679,6 @@ func linkifyReport(report []byte, repo, commit string) template.HTML {
 
 var sourceFileRe = regexp.MustCompile("( |\t|\n)([a-zA-Z0-9/_.-]+\\.(?:h|c|cc|cpp|s|S|go|rs)):([0-9]+)( |!|\\)|\t|\n)")
 
-func loadBisectionsForBug(c context.Context, bug *Bug, jobType JobType) ([]*uiJob, error) {
-	bugKey := bug.key(c)
-	jobs, jobKeys, err := queryJobsForBug(c, bugKey, jobType)
-	if err != nil {
-		return nil, err
-	}
-	var results []*uiJob
-	for i, job := range jobs {
-		crash, err := queryCrashForJob(c, job, bugKey)
-		if err != nil {
-			return nil, err
-		}
-		if crash == nil {
-			continue
-		}
-		build, err := loadBuild(c, bug.Namespace, job.BuildID)
-		if err != nil {
-			return nil, err
-		}
-
-		results = append(results, makeUIJob(c, job, jobKeys[i], bug, crash, build))
-	}
-	return results, nil
-}
-
 func makeUICrash(c context.Context, crash *Crash, build *Build) *uiCrash {
 	uiAssets := []*uiAsset{}
 	for _, asset := range createAssetList(build, crash) {
@@ -2106,6 +2077,34 @@ func fetchErrorLogs(c context.Context) ([]byte, error) {
 	return buf.Bytes(), nil
 }
 
+func (j *bugJob) ui(c context.Context) (*uiJob, error) {
+	err := j.load(c)
+	if err != nil {
+		return nil, err
+	}
+	return makeUIJob(c, j.job, j.key, j.bug, j.crash, j.build), nil
+}
+
+func (b *bugJobs) uiAll(c context.Context) ([]*uiJob, error) {
+	var ret []*uiJob
+	for _, j := range b.all() {
+		obj, err := j.ui(c)
+		if err != nil {
+			return nil, err
+		}
+		ret = append(ret, obj)
+	}
+	return ret, nil
+}
+
+func (b *bugJobs) uiBestBisection(c context.Context) (*uiJob, error) {
+	j := b.bestBisection()
+	if j == nil {
+		return nil, nil
+	}
+	return j.ui(c)
+}
+
 // bugExtLink should be preferred to bugLink since it provides a URL that's more consistent with
 // links from email addresses.
 func bugExtLink(bug *Bug) string {
diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go
index e29d2da16..0db180afb 100644
--- a/dashboard/app/reporting.go
+++ b/dashboard/app/reporting.go
@@ -473,19 +473,25 @@ func createBugReport(c context.Context, bug *Bug, crash *Crash, crashKey *db.Key
 	var job *Job
 	if bug.BisectCause == BisectYes || bug.BisectCause == BisectInconclusive || bug.BisectCause == BisectHorizont {
 		// If we have bisection results, report the crash/repro used for bisection.
-		job1, crash1, _, crashKey1, err := loadBisectJob(c, bug, JobBisectCause)
+		causeBisect, err := queryBestBisection(c, bug, JobBisectCause)
 		if err != nil {
 			return nil, err
 		}
+		if causeBisect != nil {
+			err := causeBisect.loadCrash(c)
+			if err != nil {
+				return nil, err
+			}
+		}
 		// If we didn't check whether the bisect is unreliable, even though it would not be
 		// reported anyway, we could still eventually Cc people from those commits later
 		// (e.g. when we did bisected with a syz repro and then notified about a C repro).
-		if !job1.isUnreliableBisect() {
-			job = job1
-			if crash1.ReproC != 0 || crash.ReproC == 0 {
+		if causeBisect != nil && !causeBisect.job.isUnreliableBisect() {
+			job = causeBisect.job
+			if causeBisect.crash.ReproC != 0 || crash.ReproC == 0 {
 				// Don't override the crash in this case,
 				// otherwise we will always think that we haven't reported the C repro.
-				crash, crashKey = crash1, crashKey1
+				crash, crashKey = causeBisect.crash, causeBisect.crashKey
 			}
 		}
 	}
@@ -665,35 +671,6 @@ func fillBugReport(c context.Context, rep *dashapi.BugReport, bug *Bug, bugRepor
 	return nil
 }
 
-func loadBisectJob(c context.Context, bug *Bug, jobType JobType) (*Job, *Crash, *db.Key, *db.Key, error) {
-	bugKey := bug.key(c)
-	var jobs []*Job
-	keys, err := db.NewQuery("Job").
-		Ancestor(bugKey).
-		Filter("Type=", jobType).
-		Filter("Finished>", time.Time{}).
-		Order("-Finished").
-		Limit(1).
-		GetAll(c, &jobs)
-	if err != nil {
-		return nil, nil, nil, nil, fmt.Errorf("failed to query jobs: %v", err)
-	}
-	if len(jobs) == 0 {
-		jobStr := map[JobType]string{
-			JobBisectCause: "bisect cause",
-			JobBisectFix:   "bisect fix",
-		}
-		return nil, nil, nil, nil, fmt.Errorf("can't find %s job for bug", jobStr[jobType])
-	}
-	job := jobs[0]
-	crash := new(Crash)
-	crashKey := db.NewKey(c, "Crash", "", job.CrashID, bugKey)
-	if err := db.Get(c, crashKey, crash); err != nil {
-		return nil, nil, nil, nil, fmt.Errorf("failed to get crash: %v", err)
-	}
-	return job, crash, keys[0], crashKey, nil
-}
-
 func managersToRepos(c context.Context, ns string, managers []string) []string {
 	var repos []string
 	dedup := make(map[string]bool)
@@ -714,6 +691,21 @@ func managersToRepos(c context.Context, ns string, managers []string) []string {
 	return repos
 }
 
+func queryCrashesForBug(c context.Context, bugKey *db.Key, limit int) (
+	[]*Crash, []*db.Key, error) {
+	var crashes []*Crash
+	keys, err := db.NewQuery("Crash").
+		Ancestor(bugKey).
+		Order("-ReportLen").
+		Order("-Time").
+		Limit(limit).
+		GetAll(c, &crashes)
+	if err != nil {
+		return nil, nil, fmt.Errorf("failed to fetch crashes: %w", err)
+	}
+	return crashes, keys, nil
+}
+
 func loadAllBugs(c context.Context, filter func(*db.Query) *db.Query) ([]*Bug, []*db.Key, error) {
 	var bugs []*Bug
 	var keys []*db.Key
@@ -1281,61 +1273,6 @@ func (bug *Bug) updateReportings(cfg *Config, now time.Time) error {
 	return nil
 }
 
-func queryCrashesForBug(c context.Context, bugKey *db.Key, limit int) (
-	[]*Crash, []*db.Key, error) {
-	var crashes []*Crash
-	keys, err := db.NewQuery("Crash").
-		Ancestor(bugKey).
-		Order("-ReportLen").
-		Order("-Time").
-		Limit(limit).
-		GetAll(c, &crashes)
-	if err != nil {
-		return nil, nil, fmt.Errorf("failed to fetch crashes: %v", err)
-	}
-	return crashes, keys, nil
-}
-
-func queryJobsForBug(c context.Context, bugKey *db.Key, jobType JobType) (
-	[]*Job, []*db.Key, error) {
-	var jobs []*Job
-	keys, err := db.NewQuery("Job").
-		Ancestor(bugKey).
-		Filter("Type=", jobType).
-		Filter("Finished>", time.Time{}).
-		Order("-Finished").
-		GetAll(c, &jobs)
-	if err != nil {
-		return nil, nil, fmt.Errorf("failed to fetch fix bisections: %v", err)
-	}
-	return jobs, keys, nil
-}
-
-func queryCrashForJob(c context.Context, job *Job, bugKey *db.Key) (*Crash, error) {
-	// If there was no crash corresponding to the Job, return.
-	if job.CrashTitle == "" {
-		return nil, nil
-	}
-	// First, fetch the crash who's repro was used to start the bisection
-	// job.
-	crash := new(Crash)
-	crashKey := db.NewKey(c, "Crash", "", job.CrashID, bugKey)
-	if err := db.Get(c, crashKey, crash); err != nil {
-		return nil, err
-	}
-	// Now, create a crash object with the crash details from the job.
-	ret := &Crash{
-		Manager:   crash.Manager,
-		Time:      job.Finished,
-		Log:       job.Log,
-		Report:    job.CrashReport,
-		ReproOpts: crash.ReproOpts,
-		ReproSyz:  crash.ReproSyz,
-		ReproC:    crash.ReproC,
-	}
-	return ret, nil
-}
-
 func findCrashForBug(c context.Context, bug *Bug) (*Crash, *db.Key, error) {
 	bugKey := bug.key(c)
 	crashes, keys, err := queryCrashesForBug(c, bugKey, 1)
@@ -1467,10 +1404,11 @@ func loadFullBugInfo(c context.Context, bug *Bug, bugKey *db.Key,
 
 func prepareBisectionReport(c context.Context, bug *Bug, jobType JobType,
 	reporting *Reporting) (*dashapi.BugReport, error) {
-	job, _, jobKey, _, err := loadBisectJob(c, bug, jobType)
-	if err != nil {
+	bisectionJob, err := queryBestBisection(c, bug, jobType)
+	if bisectionJob == nil || err != nil {
 		return nil, err
 	}
+	job, jobKey := bisectionJob.job, bisectionJob.key
 	if job.Reporting != "" {
 		ret, err := createBugReportForJob(c, job, jobKey, reporting.Config)
 		if err != nil {