dashboard/app: ns/cover?jsonl=1 supports manager and subsystem selection

1. Refactor handleHeatmap.
2. Introduce function options. Build them from http.Request.

5 files changed, 171 insertions, 76 deletions

diff --git a/dashboard/app/coverage.go b/dashboard/app/coverage.go
index e2e0c744f..bbadbeb25 100644
--- a/dashboard/app/coverage.go
+++ b/dashboard/app/coverage.go
@@ -5,12 +5,14 @@ package main
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"html/template"
 	"net/http"
 	"os"
 	"slices"
 	"strconv"
+	"strings"
 
 	"cloud.google.com/go/civil"
 	"github.com/google/syzkaller/pkg/cover"
@@ -56,22 +58,135 @@ type funcStyleBodyJS func(
 	scope *coveragedb.SelectScope, onlyUnique bool, sss, managers []string, dataFilters cover.Format,
 ) (template.CSS, template.HTML, template.HTML, error)
 
-func handleCoverageHeatmap(c context.Context, w http.ResponseWriter, r *http.Request) error {
-	if r.FormValue("jsonl") == "1" {
-		hdr, err := commonHeader(c, r, w, "")
-		if err != nil {
-			return err
+type coverageHeatmapParams struct {
+	manager    string
+	subsystem  string
+	onlyUnique bool
+	periodType string
+	nPeriods   int
+	dateTo     civil.Date
+	cover.Format
+}
+
+const minPeriodsOnThePage = 1
+const maxPeriodsOnThePage = 12
+
+func makeHeatmapParams(ctx context.Context, r *http.Request) (*coverageHeatmapParams, error) {
+	minCoverLinesDrop, err := getIntParam(r, covPageParams[MinCoverLinesDrop], 0)
+	if err != nil {
+		return nil, err
+	}
+	onlyUnique, err := getBoolParam(r, covPageParams[UniqueOnly], false)
+	if err != nil {
+		return nil, err
+	}
+	orderByCoverDrop, err := getBoolParam(r, covPageParams[OrderByCoverDrop], false)
+	if err != nil {
+		return nil, err
+	}
+	periodType, err := getStringParam(r, covPageParams[PeriodType], coveragedb.DayPeriod)
+	if err != nil {
+		return nil, err
+	}
+	if !slices.Contains(coveragedb.AllPeriods, periodType) {
+		return nil, fmt.Errorf("only {%s} are allowed, but received %s instead, %w",
+			strings.Join(coveragedb.AllPeriods, ", "), periodType, ErrClientBadRequest)
+	}
+
+	nPeriods, err := getIntParam(r, covPageParams[PeriodCount], 4)
+	if err != nil || nPeriods > maxPeriodsOnThePage || nPeriods < minPeriodsOnThePage {
+		return nil, fmt.Errorf("periods_count is wrong, expected [%d, %d]: %w",
+			minPeriodsOnThePage, maxPeriodsOnThePage, err)
+	}
+
+	dateTo := civil.DateOf(timeNow(ctx))
+	if customDate := r.FormValue(covPageParams[DateTo]); customDate != "" {
+		if dateTo, err = civil.ParseDate(customDate); err != nil {
+			return nil, fmt.Errorf("civil.ParseDate(%s): %w", customDate, err)
 		}
+	}
+
+	return &coverageHeatmapParams{
+		manager:    r.FormValue(covPageParams[ManagerName]),
+		subsystem:  r.FormValue(covPageParams[SubsystemName]),
+		onlyUnique: onlyUnique,
+		periodType: periodType,
+		nPeriods:   nPeriods,
+		dateTo:     dateTo,
+		Format: cover.Format{
+			DropCoveredLines0:         onlyUnique,
+			OrderByCoveredLinesDrop:   orderByCoverDrop,
+			FilterMinCoveredLinesDrop: minCoverLinesDrop,
+		},
+	}, nil
+}
+
+func getIntParam(r *http.Request, name string, orDefault ...int) (int, error) {
+	if r.FormValue(name) == "" {
+		if len(orDefault) > 0 {
+			return orDefault[0], nil
+		}
+		return 0, errors.New("missing parameter " + name)
+	}
+	res, err := strconv.Atoi(r.FormValue(name))
+	if err != nil {
+		return 0, fmt.Errorf("strconv.Atoi(%s): %w", name, err)
+	}
+	return res, nil
+}
+
+func getBoolParam(r *http.Request, name string, orDefault ...bool) (bool, error) {
+	if r.FormValue(name) == "" {
+		if len(orDefault) > 0 {
+			return orDefault[0], nil
+		}
+		return false, errors.New("missing parameter " + name)
+	}
+	res, err := strconv.ParseBool(r.FormValue(name))
+	if err != nil {
+		return false, fmt.Errorf("strconv.ParseBool(%s): %w", name, err)
+	}
+	return res, nil
+}
+
+func getStringParam(r *http.Request, name string, orDefault ...string) (string, error) {
+	if r.FormValue(name) == "" {
+		if len(orDefault) > 0 {
+			return orDefault[0], nil
+		}
+		return "", errors.New("missing parameter " + name)
+	}
+	return r.FormValue(name), nil
+}
+
+func handleCoverageHeatmap(c context.Context, w http.ResponseWriter, r *http.Request) error {
+	hdr, err := commonHeader(c, r, w, "")
+	if err != nil {
+		return err
+	}
+	params, err := makeHeatmapParams(c, r)
+	if err != nil {
+		return fmt.Errorf("%s: %w", err.Error(), ErrClientBadRequest)
+	}
+	if requestJSONL, _ := getBoolParam(r, "jsonl", false); requestJSONL {
 		ns := hdr.Namespace
 		repo, _ := getNsConfig(c, ns).mainRepoBranch()
 		w.Header().Set("Content-Type", "application/json")
-		return writeExtAPICoverageFor(c, w, ns, repo)
+		return writeExtAPICoverageFor(c, w, ns, repo, params)
 	}
-	return handleHeatmap(c, w, r, cover.DoHeatMapStyleBodyJS)
+	return handleHeatmap(c, w, hdr, params, cover.DoHeatMapStyleBodyJS)
 }
 
 func handleSubsystemsCoverageHeatmap(c context.Context, w http.ResponseWriter, r *http.Request) error {
-	return handleHeatmap(c, w, r, cover.DoSubsystemsHeatMapStyleBodyJS)
+	hdr, err := commonHeader(c, r, w, "")
+	if err != nil {
+		return err
+	}
+	params, err := makeHeatmapParams(c, r)
+	if err != nil {
+		return fmt.Errorf("%s: %w", err.Error(), ErrClientBadRequest)
+	}
+	return handleHeatmap(c, w, hdr, params, cover.DoSubsystemsHeatMapStyleBodyJS)
 }
 
 type covPageParam int
@@ -127,46 +242,14 @@ func coveragePageLink(ns, periodType, dateTo string, minDrop, periodCount int, o
 	return url
 }
 
-func handleHeatmap(c context.Context, w http.ResponseWriter, r *http.Request, f funcStyleBodyJS) error {
-	hdr, err := commonHeader(c, r, w, "")
-	if err != nil {
-		return err
-	}
+func handleHeatmap(c context.Context, w http.ResponseWriter, hdr *uiHeader, p *coverageHeatmapParams,
+	f funcStyleBodyJS) error {
 	nsConfig := getNsConfig(c, hdr.Namespace)
 	if nsConfig.Coverage == nil {
 		return ErrClientNotFound
 	}
-	ss := r.FormValue(covPageParams[SubsystemName])
-	manager := r.FormValue(covPageParams[ManagerName])
-
-	periodType := r.FormValue(covPageParams[PeriodType])
-	if periodType == "" {
-		periodType = coveragedb.DayPeriod
-	}
-	if periodType != coveragedb.DayPeriod &&
-		periodType != coveragedb.MonthPeriod &&
-		periodType != coveragedb.QuarterPeriod {
-		return fmt.Errorf("only 'day', 'month' and 'quarter' are allowed, but received %s instead, %w",
-			periodType, ErrClientBadRequest)
-	}
-
-	periodCount := r.FormValue(covPageParams[PeriodCount])
-	if periodCount == "" {
-		periodCount = "4"
-	}
-	nPeriods, err := strconv.Atoi(periodCount)
-	if err != nil || nPeriods > 12 || nPeriods < 1 {
-		return fmt.Errorf("periods_count is wrong, expected [1, 12]: %w", err)
-	}
-
-	dateTo := civil.DateOf(timeNow(c))
-	if customDate := r.FormValue(covPageParams[DateTo]); customDate != "" {
-		if dateTo, err = civil.ParseDate(customDate); err != nil {
-			return fmt.Errorf("civil.ParseDate(%s): %w", customDate, err)
-		}
-	}
 
-	periods, err := coveragedb.GenNPeriodsTill(nPeriods, dateTo, periodType)
+	periods, err := coveragedb.GenNPeriodsTill(p.nPeriods, p.dateTo, p.periodType)
 	if err != nil {
 		return fmt.Errorf("%s: %w", err.Error(), ErrClientBadRequest)
 	}
@@ -182,29 +265,17 @@ func handleHeatmap(c context.Context, w http.ResponseWriter, r *http.Request, f
 	slices.Sort(managers)
 	slices.Sort(subsystems)
 
-	onlyUnique := r.FormValue(covPageParams[UniqueOnly]) == "1"
-	orderByCoverLinesDrop := r.FormValue(covPageParams[OrderByCoverDrop]) == "1"
-	// Prefixing "0" we don't fail on empty string.
-	minCoverLinesDrop, err := strconv.Atoi("0" + r.FormValue(covPageParams[MinCoverLinesDrop]))
-	if err != nil {
-		return fmt.Errorf(covPageParams[MinCoverLinesDrop] + " should be integer")
-	}
-
 	var style template.CSS
 	var body, js template.HTML
 	if style, body, js, err = f(c, getCoverageDBClient(c),
 		&coveragedb.SelectScope{
 			Ns:        hdr.Namespace,
-			Subsystem: ss,
-			Manager:   manager,
+			Subsystem: p.subsystem,
+			Manager:   p.manager,
 			Periods:   periods,
 		},
-		onlyUnique, subsystems, managers,
-		cover.Format{
-			FilterMinCoveredLinesDrop: minCoverLinesDrop,
-			OrderByCoveredLinesDrop:   orderByCoverLinesDrop,
-			DropCoveredLines0:         onlyUnique,
-		}); err != nil {
+		p.onlyUnique, subsystems, managers, p.Format,
+	); err != nil {
 		return fmt.Errorf("failed to generate heatmap: %w", err)
 	}
 	return serveTemplate(w, "custom_content.html", struct {
@@ -261,7 +332,7 @@ func handleFileCoverage(c context.Context, w http.ResponseWriter, r *http.Reques
 	if err != nil {
 		return fmt.Errorf("coveragedb.MakeTimePeriod: %w", err)
 	}
-	onlyUnique := r.FormValue(covPageParams[UniqueOnly]) == "1"
+	onlyUnique, _ := getBoolParam(r, covPageParams[UniqueOnly], false)
 	mainNsRepo, _ := nsConfig.mainRepoBranch()
 	client := getCoverageDBClient(c)
 	if client == nil {
diff --git a/dashboard/app/public_json_api.go b/dashboard/app/public_json_api.go
index 3b882ea24..5905ec8c6 100644
--- a/dashboard/app/public_json_api.go
+++ b/dashboard/app/public_json_api.go
@@ -178,7 +178,7 @@ func GetJSONDescrFor(page interface{}) ([]byte, error) {
 	return json.MarshalIndent(res, "", "\t")
 }
 
-func writeExtAPICoverageFor(ctx context.Context, w io.Writer, ns, repo string) error {
+func writeExtAPICoverageFor(ctx context.Context, w io.Writer, ns, repo string, p *coverageHeatmapParams) error {
 	// By default, return the previous month coverage. It guarantees the good numbers.
 	//
 	// The alternative is to return the current month.
@@ -187,13 +187,25 @@ func writeExtAPICoverageFor(ctx context.Context, w io.Writer, ns, repo string) e
 	if err != nil {
 		return fmt.Errorf("coveragedb.GenNPeriodsTill: %w", err)
 	}
-	defaultTimePeriod := tps[0]
+
 	covDBClient := getCoverageDBClient(ctx)
-	ff, err := coveragedb.MakeFuncFinder(ctx, covDBClient, ns, defaultTimePeriod)
+	ff, err := coveragedb.MakeFuncFinder(ctx, covDBClient, ns, tps[0])
 	if err != nil {
 		return fmt.Errorf("coveragedb.MakeFuncFinder: %w", err)
 	}
-	covCh, errCh := coveragedb.FilesCoverageStream(ctx, covDBClient, ns, defaultTimePeriod)
+	subsystem := ""
+	manager := ""
+	if p != nil {
+		subsystem = p.subsystem
+		manager = p.manager
+	}
+	covCh, errCh := coveragedb.FilesCoverageStream(ctx, covDBClient,
+		&coveragedb.SelectScope{
+			Ns:        ns,
+			Subsystem: subsystem,
+			Manager:   manager,
+			Periods:   tps,
+		})
 	if err := writeFileCoverage(ctx, w, repo, ff, covCh); err != nil {
 		return fmt.Errorf("populateFileCoverage: %w", err)
 	}
diff --git a/dashboard/app/public_json_api_test.go b/dashboard/app/public_json_api_test.go
index 5a23347db..081bcee23 100644
--- a/dashboard/app/public_json_api_test.go
+++ b/dashboard/app/public_json_api_test.go
@@ -280,7 +280,7 @@ func TestWriteExtAPICoverageFor(t *testing.T) {
 	))
 
 	var buf bytes.Buffer
-	err := writeExtAPICoverageFor(ctx, &buf, "test-ns", "test-repo")
+	err := writeExtAPICoverageFor(ctx, &buf, "test-ns", "test-repo", nil)
 	assert.NoError(t, err)
 	assert.Equal(t, `{
 	"repo": "test-repo",
diff --git a/pkg/coveragedb/coveragedb.go b/pkg/coveragedb/coveragedb.go
index 50fbd0633..6741e72d1 100644
--- a/pkg/coveragedb/coveragedb.go
+++ b/pkg/coveragedb/coveragedb.go
@@ -409,10 +409,10 @@ type SelectScope struct {
 
 // FilesCoverageStream streams information about all the line coverage.
 // It is expensive and better to be used for time insensitive operations.
-func FilesCoverageStream(ctx context.Context, client spannerclient.SpannerClient, ns string, timePeriod TimePeriod,
+func FilesCoverageStream(ctx context.Context, client spannerclient.SpannerClient, scope *SelectScope,
 ) (<-chan *FileCoverageWithLineInfo, <-chan error) {
 	iter := client.Single().Query(ctx,
-		filesCoverageWithDetailsStmt(ns, "", "", timePeriod, true))
+		filesCoverageWithDetailsStmt(scope, true))
 	resCh := make(chan *FileCoverageWithLineInfo)
 	errCh := make(chan error)
 	go func() {
@@ -435,14 +435,24 @@ func FilesCoverageWithDetails(
 	for _, timePeriod := range scope.Periods {
 		needLinesDetails := onlyUnique
 		iterManager := client.Single().Query(ctx,
-			filesCoverageWithDetailsStmt(scope.Ns, scope.Subsystem, scope.Manager, timePeriod, needLinesDetails))
+			filesCoverageWithDetailsStmt(&SelectScope{
+				Ns:        scope.Ns,
+				Subsystem: scope.Subsystem,
+				Manager:   scope.Manager,
+				Periods:   []TimePeriod{timePeriod},
+			}, needLinesDetails))
 		defer iterManager.Stop()
 
 		var err error
 		var periodRes []*FileCoverageWithDetails
 		if onlyUnique {
 			iterAll := client.Single().Query(ctx,
-				filesCoverageWithDetailsStmt(scope.Ns, scope.Subsystem, "", timePeriod, needLinesDetails))
+				filesCoverageWithDetailsStmt(&SelectScope{
+					Ns:        scope.Ns,
+					Subsystem: scope.Subsystem,
+					Manager:   "",
+					Periods:   []TimePeriod{timePeriod},
+				}, needLinesDetails))
 			defer iterAll.Stop()
 			periodRes, err = readCoverageUniq(iterAll, iterManager)
 			if err != nil {
@@ -462,8 +472,8 @@ func FilesCoverageWithDetails(
 	return res, nil
 }
 
-func filesCoverageWithDetailsStmt(ns, subsystem, manager string, timePeriod TimePeriod, withLines bool,
-) spanner.Statement {
+func filesCoverageWithDetailsStmt(scope *SelectScope, withLines bool) spanner.Statement {
+	manager := scope.Manager
 	if manager == "" {
 		manager = "*"
 	}
@@ -481,15 +491,15 @@ from merge_history
 where
   merge_history.namespace=$1 and dateto=$2 and duration=$3 and manager=$4`,
 		Params: map[string]interface{}{
-			"p1": ns,
-			"p2": timePeriod.DateTo,
-			"p3": timePeriod.Days,
+			"p1": scope.Ns,
+			"p2": scope.Periods[0].DateTo,
+			"p3": scope.Periods[0].Days,
 			"p4": manager,
 		},
 	}
-	if subsystem != "" {
+	if scope.Subsystem != "" {
 		stmt.SQL += " and $5=ANY(subsystems)"
-		stmt.Params["p5"] = subsystem
+		stmt.Params["p5"] = scope.Subsystem
 	}
 	stmt.SQL += "\norder by files.filepath"
 	return stmt
diff --git a/pkg/coveragedb/time_period.go b/pkg/coveragedb/time_period.go
index 9af61d1a8..37ec6aa27 100644
--- a/pkg/coveragedb/time_period.go
+++ b/pkg/coveragedb/time_period.go
@@ -41,6 +41,8 @@ const (
 	QuarterPeriod = "quarter"
 )
 
+var AllPeriods = []string{DayPeriod, MonthPeriod, QuarterPeriod}
+
 var errUnknownTimePeriodType = errors.New("unknown time period type")
 
 func MinMaxDays(periodType string) (int, int, error) {