Skip to content

Commit 1c5a58f

Browse files
tarasmadantarasmadan
committed
dashboard/app: ns/cover?jsonl=1 supports manager and subsystem selection
1. Refactor handleHeatmap. 2. Introduce function options. Build them from http.Request.
1 parent fd5e6e6 commit 1c5a58f

File tree

4 files changed

+165
-76
lines changed

4 files changed

+165
-76
lines changed

‎dashboard/app/coverage.go‎

Lines changed: 127 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package main
55

66
import (
77
"context"
8+
"errors"
89
"fmt"
910
"html/template"
1011
"net/http"
@@ -56,22 +57,132 @@ type funcStyleBodyJS func(
5657
scope *coveragedb.SelectScope, onlyUnique bool, sss, managers []string, dataFilters cover.Format,
5758
) (template.CSS, template.HTML, template.HTML, error)
5859

59-
func handleCoverageHeatmap(c context.Context, w http.ResponseWriter, r *http.Request) error {
60-
if r.FormValue("jsonl") == "1" {
61-
hdr, err := commonHeader(c, r, w, "")
62-
if err != nil {
63-
return err
60+
type coverageHeatmapParams struct {
61+
manager string
62+
subsystem string
63+
onlyUnique bool
64+
periodType string
65+
nPeriods int
66+
dateTo civil.Date
67+
cover.Format
68+
}
69+
70+
const minPeriodsOnThePage = 1
71+
const maxPeriodsOnThePage = 12
72+
73+
func makeHeatmapParams(ctx context.Context, r *http.Request) (*coverageHeatmapParams, error) {
74+
minCoverLinesDrop, err := getIntParam(r, covPageParams[MinCoverLinesDrop], 0)
75+
if err != nil {
76+
return nil, fmt.Errorf("min-cover-lines-drop is wrong: %w", err)
77+
}
78+
79+
onlyUnique, err := getBoolParam(r, covPageParams[UniqueOnly], false)
80+
if err != nil {
81+
return nil, fmt.Errorf("unique-only is wrong: %w", err)
82+
}
83+
84+
periodType, err := getStringParam(r, covPageParams[PeriodType], coveragedb.DayPeriod)
85+
if err != nil {
86+
return nil, fmt.Errorf("period is wrong: %w", err)
87+
}
88+
if !slices.Contains([]string{coveragedb.DayPeriod, coveragedb.MonthPeriod, coveragedb.QuarterPeriod}, periodType) {
89+
return nil, fmt.Errorf("only 'day', 'month' and 'quarter' are allowed, but received %s instead, %w",
90+
periodType, ErrClientBadRequest)
91+
}
92+
93+
nPeriods, err := getIntParam(r, covPageParams[PeriodCount], 4)
94+
if err != nil || nPeriods > maxPeriodsOnThePage || nPeriods < minPeriodsOnThePage {
95+
return nil, fmt.Errorf("periods_count is wrong, expected [1, 12]: %w", err)
96+
}
97+
98+
dateTo := civil.DateOf(timeNow(ctx))
99+
if customDate := r.FormValue(covPageParams[DateTo]); customDate != "" {
100+
if dateTo, err = civil.ParseDate(customDate); err != nil {
101+
return nil, fmt.Errorf("civil.ParseDate(%s): %w", customDate, err)
102+
}
103+
}
104+
105+
return &coverageHeatmapParams{
106+
manager: r.FormValue(covPageParams[ManagerName]),
107+
subsystem: r.FormValue(covPageParams[SubsystemName]),
108+
onlyUnique: onlyUnique,
109+
periodType: periodType,
110+
nPeriods: nPeriods,
111+
dateTo: dateTo,
112+
Format: cover.Format{
113+
DropCoveredLines0: onlyUnique,
114+
OrderByCoveredLinesDrop: r.FormValue(covPageParams[OrderByCoverDrop]) == "1",
115+
FilterMinCoveredLinesDrop: minCoverLinesDrop,
116+
},
117+
}, nil
118+
}
119+
120+
func getIntParam(r *http.Request, name string, orDefault ...int) (int, error) {
121+
if r.FormValue(name) == "" {
122+
if len(orDefault) > 0 {
123+
return orDefault[0], nil
64124
}
125+
return 0, errors.New("missing parameter " + name)
126+
}
127+
res, err := strconv.Atoi(r.FormValue(name))
128+
if err != nil {
129+
return 0, fmt.Errorf("strconv.Atoi: %w", err)
130+
}
131+
return res, nil
132+
}
133+
134+
func getBoolParam(r *http.Request, name string, orDefault ...bool) (bool, error) {
135+
if r.FormValue(name) == "" {
136+
if len(orDefault) > 0 {
137+
return orDefault[0], nil
138+
}
139+
return false, errors.New("missing parameter " + name)
140+
}
141+
res, err := strconv.ParseBool(r.FormValue(name))
142+
if err != nil {
143+
return false, fmt.Errorf("strconv.ParseBool: %w", err)
144+
}
145+
return res, nil
146+
}
147+
148+
func getStringParam(r *http.Request, name string, orDefault ...string) (string, error) {
149+
if r.FormValue(name) == "" {
150+
if len(orDefault) > 0 {
151+
return orDefault[0], nil
152+
}
153+
return "", errors.New("missing parameter " + name)
154+
}
155+
return r.FormValue(name), nil
156+
}
157+
158+
func handleCoverageHeatmap(c context.Context, w http.ResponseWriter, r *http.Request) error {
159+
hdr, err := commonHeader(c, r, w, "")
160+
if err != nil {
161+
return err
162+
}
163+
params, err := makeHeatmapParams(c, r)
164+
if err != nil {
165+
return fmt.Errorf("%s: %w", err.Error(), ErrClientBadRequest)
166+
}
167+
if askJSONL, _ := getBoolParam(r, "jsonl", false); askJSONL {
65168
ns := hdr.Namespace
66169
repo, _ := getNsConfig(c, ns).mainRepoBranch()
67170
w.Header().Set("Content-Type", "application/json")
68-
return writeExtAPICoverageFor(c, w, ns, repo)
171+
return writeExtAPICoverageFor(c, w, ns, repo, params)
69172
}
70-
return handleHeatmap(c, w, r, cover.DoHeatMapStyleBodyJS)
173+
return handleHeatmap(c, w, hdr, params, cover.DoHeatMapStyleBodyJS)
71174
}
72175

73176
func handleSubsystemsCoverageHeatmap(c context.Context, w http.ResponseWriter, r *http.Request) error {
74-
return handleHeatmap(c, w, r, cover.DoSubsystemsHeatMapStyleBodyJS)
177+
hdr, err := commonHeader(c, r, w, "")
178+
if err != nil {
179+
return err
180+
}
181+
params, err := makeHeatmapParams(c, r)
182+
if err != nil {
183+
return fmt.Errorf("%s: %w", err.Error(), ErrClientBadRequest)
184+
}
185+
return handleHeatmap(c, w, hdr, params, cover.DoSubsystemsHeatMapStyleBodyJS)
75186
}
76187

77188
type covPageParam int
@@ -127,46 +238,14 @@ func coveragePageLink(ns, periodType, dateTo string, minDrop, periodCount int, o
127238
return url
128239
}
129240

130-
func handleHeatmap(c context.Context, w http.ResponseWriter, r *http.Request, f funcStyleBodyJS) error {
131-
hdr, err := commonHeader(c, r, w, "")
132-
if err != nil {
133-
return err
134-
}
241+
func handleHeatmap(c context.Context, w http.ResponseWriter, hdr *uiHeader, p *coverageHeatmapParams,
242+
f funcStyleBodyJS) error {
135243
nsConfig := getNsConfig(c, hdr.Namespace)
136244
if nsConfig.Coverage == nil {
137245
return ErrClientNotFound
138246
}
139-
ss := r.FormValue(covPageParams[SubsystemName])
140-
manager := r.FormValue(covPageParams[ManagerName])
141247

142-
periodType := r.FormValue(covPageParams[PeriodType])
143-
if periodType == "" {
144-
periodType = coveragedb.DayPeriod
145-
}
146-
if periodType != coveragedb.DayPeriod &&
147-
periodType != coveragedb.MonthPeriod &&
148-
periodType != coveragedb.QuarterPeriod {
149-
return fmt.Errorf("only 'day', 'month' and 'quarter' are allowed, but received %s instead, %w",
150-
periodType, ErrClientBadRequest)
151-
}
152-
153-
periodCount := r.FormValue(covPageParams[PeriodCount])
154-
if periodCount == "" {
155-
periodCount = "4"
156-
}
157-
nPeriods, err := strconv.Atoi(periodCount)
158-
if err != nil || nPeriods > 12 || nPeriods < 1 {
159-
return fmt.Errorf("periods_count is wrong, expected [1, 12]: %w", err)
160-
}
161-
162-
dateTo := civil.DateOf(timeNow(c))
163-
if customDate := r.FormValue(covPageParams[DateTo]); customDate != "" {
164-
if dateTo, err = civil.ParseDate(customDate); err != nil {
165-
return fmt.Errorf("civil.ParseDate(%s): %w", customDate, err)
166-
}
167-
}
168-
169-
periods, err := coveragedb.GenNPeriodsTill(nPeriods, dateTo, periodType)
248+
periods, err := coveragedb.GenNPeriodsTill(p.nPeriods, p.dateTo, p.periodType)
170249
if err != nil {
171250
return fmt.Errorf("%s: %w", err.Error(), ErrClientBadRequest)
172251
}
@@ -182,29 +261,17 @@ func handleHeatmap(c context.Context, w http.ResponseWriter, r *http.Request, f
182261
slices.Sort(managers)
183262
slices.Sort(subsystems)
184263

185-
onlyUnique := r.FormValue(covPageParams[UniqueOnly]) == "1"
186-
orderByCoverLinesDrop := r.FormValue(covPageParams[OrderByCoverDrop]) == "1"
187-
// Prefixing "0" we don't fail on empty string.
188-
minCoverLinesDrop, err := strconv.Atoi("0" + r.FormValue(covPageParams[MinCoverLinesDrop]))
189-
if err != nil {
190-
return fmt.Errorf(covPageParams[MinCoverLinesDrop] + " should be integer")
191-
}
192-
193264
var style template.CSS
194265
var body, js template.HTML
195266
if style, body, js, err = f(c, getCoverageDBClient(c),
196267
&coveragedb.SelectScope{
197268
Ns: hdr.Namespace,
198-
Subsystem: ss,
199-
Manager: manager,
269+
Subsystem: p.subsystem,
270+
Manager: p.manager,
200271
Periods: periods,
201272
},
202-
onlyUnique, subsystems, managers,
203-
cover.Format{
204-
FilterMinCoveredLinesDrop: minCoverLinesDrop,
205-
OrderByCoveredLinesDrop: orderByCoverLinesDrop,
206-
DropCoveredLines0: onlyUnique,
207-
}); err != nil {
273+
p.onlyUnique, subsystems, managers, p.Format,
274+
); err != nil {
208275
return fmt.Errorf("failed to generate heatmap: %w", err)
209276
}
210277
return serveTemplate(w, "custom_content.html", struct {
@@ -261,7 +328,7 @@ func handleFileCoverage(c context.Context, w http.ResponseWriter, r *http.Reques
261328
if err != nil {
262329
return fmt.Errorf("coveragedb.MakeTimePeriod: %w", err)
263330
}
264-
onlyUnique := r.FormValue(covPageParams[UniqueOnly]) == "1"
331+
onlyUnique, _ := getBoolParam(r, covPageParams[UniqueOnly], false)
265332
mainNsRepo, _ := nsConfig.mainRepoBranch()
266333
client := getCoverageDBClient(c)
267334
if client == nil {

‎dashboard/app/public_json_api.go‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ func GetJSONDescrFor(page interface{}) ([]byte, error) {
178178
return json.MarshalIndent(res, "", "\t")
179179
}
180180

181-
func writeExtAPICoverageFor(ctx context.Context, w io.Writer, ns, repo string) error {
181+
func writeExtAPICoverageFor(ctx context.Context, w io.Writer, ns, repo string, p *coverageHeatmapParams) error {
182182
// By default, return the previous month coverage. It guarantees the good numbers.
183183
//
184184
// The alternative is to return the current month.
@@ -187,13 +187,25 @@ func writeExtAPICoverageFor(ctx context.Context, w io.Writer, ns, repo string) e
187187
if err != nil {
188188
return fmt.Errorf("coveragedb.GenNPeriodsTill: %w", err)
189189
}
190-
defaultTimePeriod := tps[0]
190+
191191
covDBClient := getCoverageDBClient(ctx)
192-
ff, err := coveragedb.MakeFuncFinder(ctx, covDBClient, ns, defaultTimePeriod)
192+
ff, err := coveragedb.MakeFuncFinder(ctx, covDBClient, ns, tps[0])
193193
if err != nil {
194194
return fmt.Errorf("coveragedb.MakeFuncFinder: %w", err)
195195
}
196-
covCh, errCh := coveragedb.FilesCoverageStream(ctx, covDBClient, ns, defaultTimePeriod)
196+
subsystem := ""
197+
manager := ""
198+
if p != nil {
199+
subsystem = p.subsystem
200+
manager = p.manager
201+
}
202+
covCh, errCh := coveragedb.FilesCoverageStream(ctx, covDBClient,
203+
&coveragedb.SelectScope{
204+
Ns: ns,
205+
Subsystem: subsystem,
206+
Manager: manager,
207+
Periods: tps,
208+
})
197209
if err := writeFileCoverage(ctx, w, repo, ff, covCh); err != nil {
198210
return fmt.Errorf("populateFileCoverage: %w", err)
199211
}

‎dashboard/app/public_json_api_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ func TestWriteExtAPICoverageFor(t *testing.T) {
280280
))
281281

282282
var buf bytes.Buffer
283-
err := writeExtAPICoverageFor(ctx, &buf, "test-ns", "test-repo")
283+
err := writeExtAPICoverageFor(ctx, &buf, "test-ns", "test-repo", nil)
284284
assert.NoError(t, err)
285285
assert.Equal(t, `{
286286
"repo": "test-repo",

‎pkg/coveragedb/coveragedb.go‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -409,10 +409,10 @@ type SelectScope struct {
409409

410410
// FilesCoverageStream streams information about all the line coverage.
411411
// It is expensive and better to be used for time insensitive operations.
412-
func FilesCoverageStream(ctx context.Context, client spannerclient.SpannerClient, ns string, timePeriod TimePeriod,
412+
func FilesCoverageStream(ctx context.Context, client spannerclient.SpannerClient, scope *SelectScope,
413413
) (<-chan *FileCoverageWithLineInfo, <-chan error) {
414414
iter := client.Single().Query(ctx,
415-
filesCoverageWithDetailsStmt(ns, "", "", timePeriod, true))
415+
filesCoverageWithDetailsStmt(scope, true))
416416
resCh := make(chan *FileCoverageWithLineInfo)
417417
errCh := make(chan error)
418418
go func() {
@@ -435,14 +435,24 @@ func FilesCoverageWithDetails(
435435
for _, timePeriod := range scope.Periods {
436436
needLinesDetails := onlyUnique
437437
iterManager := client.Single().Query(ctx,
438-
filesCoverageWithDetailsStmt(scope.Ns, scope.Subsystem, scope.Manager, timePeriod, needLinesDetails))
438+
filesCoverageWithDetailsStmt(&SelectScope{
439+
Ns: scope.Ns,
440+
Subsystem: scope.Subsystem,
441+
Manager: scope.Manager,
442+
Periods: []TimePeriod{timePeriod},
443+
}, needLinesDetails))
439444
defer iterManager.Stop()
440445

441446
var err error
442447
var periodRes []*FileCoverageWithDetails
443448
if onlyUnique {
444449
iterAll := client.Single().Query(ctx,
445-
filesCoverageWithDetailsStmt(scope.Ns, scope.Subsystem, "", timePeriod, needLinesDetails))
450+
filesCoverageWithDetailsStmt(&SelectScope{
451+
Ns: scope.Ns,
452+
Subsystem: scope.Subsystem,
453+
Manager: "",
454+
Periods: []TimePeriod{timePeriod},
455+
}, needLinesDetails))
446456
defer iterAll.Stop()
447457
periodRes, err = readCoverageUniq(iterAll, iterManager)
448458
if err != nil {
@@ -462,8 +472,8 @@ func FilesCoverageWithDetails(
462472
return res, nil
463473
}
464474

465-
func filesCoverageWithDetailsStmt(ns, subsystem, manager string, timePeriod TimePeriod, withLines bool,
466-
) spanner.Statement {
475+
func filesCoverageWithDetailsStmt(scope *SelectScope, withLines bool) spanner.Statement {
476+
manager := scope.Manager
467477
if manager == "" {
468478
manager = "*"
469479
}
@@ -481,15 +491,15 @@ from merge_history
481491
where
482492
merge_history.namespace=$1 and dateto=$2 and duration=$3 and manager=$4`,
483493
Params: map[string]interface{}{
484-
"p1": ns,
485-
"p2": timePeriod.DateTo,
486-
"p3": timePeriod.Days,
494+
"p1": scope.Ns,
495+
"p2": scope.Periods[0].DateTo,
496+
"p3": scope.Periods[0].Days,
487497
"p4": manager,
488498
},
489499
}
490-
if subsystem != "" {
500+
if scope.Subsystem != "" {
491501
stmt.SQL += " and $5=ANY(subsystems)"
492-
stmt.Params["p5"] = subsystem
502+
stmt.Params["p5"] = scope.Subsystem
493503
}
494504
stmt.SQL += "\norder by files.filepath"
495505
return stmt

0 commit comments

Comments
 (0)