From 8d5b19bc612b27bad136344752d443d4bffe1cf5 Mon Sep 17 00:00:00 2001 From: Ezequiel Victorero Date: Thu, 15 Dec 2022 10:44:33 -0300 Subject: [PATCH] PublicDashboards: support time range selection on the backend (#60203) --- .../publicdashboards/models/errors.go | 1 + .../publicdashboards/models/models.go | 20 +-- .../publicdashboards/models/models_test.go | 42 +++++-- .../publicdashboards/service/query.go | 4 +- .../publicdashboards/validation/validation.go | 16 ++- .../validation/validation_test.go | 115 ++++++++++++++++++ 6 files changed, 176 insertions(+), 22 deletions(-) diff --git a/pkg/services/publicdashboards/models/errors.go b/pkg/services/publicdashboards/models/errors.go index 51ff7a1def5..0edaebb1610 100644 --- a/pkg/services/publicdashboards/models/errors.go +++ b/pkg/services/publicdashboards/models/errors.go @@ -19,4 +19,5 @@ var ( ErrPublicDashboardHasTemplateVariables = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.hasTemplateVariables", errutil.WithPublicMessage("Public dashboard has template variables")) ErrInvalidInterval = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.invalidInterval", errutil.WithPublicMessage("intervalMS should be greater than 0")) ErrInvalidMaxDataPoints = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.maxDataPoints", errutil.WithPublicMessage("maxDataPoints should be greater than 0")) + ErrInvalidTimeRange = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.invalidTimeRange", errutil.WithPublicMessage("Invalid time range")) ) diff --git a/pkg/services/publicdashboards/models/models.go b/pkg/services/publicdashboards/models/models.go index d5ab42207ee..1de844522d5 100644 --- a/pkg/services/publicdashboards/models/models.go +++ b/pkg/services/publicdashboards/models/models.go @@ -94,24 +94,23 @@ func (ts *TimeSettings) ToDB() ([]byte, error) { return json.Marshal(ts) } -// build time settings object from json on public dashboard. If empty, use -// defaults on the dashboard -func (pd PublicDashboard) BuildTimeSettings(dashboard *models.Dashboard) TimeSettings { +// BuildTimeSettings build time settings object using selected values if enabled and are valid or dashboard default values +func (pd PublicDashboard) BuildTimeSettings(dashboard *models.Dashboard, reqDTO PublicDashboardQueryDTO) TimeSettings { from := dashboard.Data.GetPath("time", "from").MustString() to := dashboard.Data.GetPath("time", "to").MustString() + + if pd.TimeSelectionEnabled { + from = reqDTO.TimeRange.From + to = reqDTO.TimeRange.To + } + timeRange := legacydata.NewDataTimeRange(from, to) // Were using epoch ms because this is used to build a MetricRequest, which is used by query caching, which expected the time range in epoch milliseconds. - ts := TimeSettings{ + return TimeSettings{ From: strconv.FormatInt(timeRange.GetFromAsMsEpoch(), 10), To: strconv.FormatInt(timeRange.GetToAsMsEpoch(), 10), } - - if pd.TimeSettings == nil { - return ts - } - - return ts } // DTO for transforming user input in the api @@ -125,6 +124,7 @@ type SavePublicDashboardDTO struct { type PublicDashboardQueryDTO struct { IntervalMs int64 MaxDataPoints int64 + TimeRange TimeSettings } type AnnotationsQueryDTO struct { diff --git a/pkg/services/publicdashboards/models/models_test.go b/pkg/services/publicdashboards/models/models_test.go index 01be547488e..c7fc470e084 100644 --- a/pkg/services/publicdashboards/models/models_test.go +++ b/pkg/services/publicdashboards/models/models_test.go @@ -1,7 +1,9 @@ package models import ( + "strconv" "testing" + "time" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/models" @@ -15,36 +17,58 @@ func TestPublicDashboardTableName(t *testing.T) { func TestBuildTimeSettings(t *testing.T) { var dashboardData = simplejson.NewFromAny(map[string]interface{}{"time": map[string]interface{}{"from": "2022-09-01T00:00:00.000Z", "to": "2022-09-01T12:00:00.000Z"}}) - fromMs, toMs := internal.GetTimeRangeFromDashboard(t, dashboardData) + defaultFromMs, defaultToMs := internal.GetTimeRangeFromDashboard(t, dashboardData) + + selectionFromMs := strconv.FormatInt(time.Now().UnixMilli(), 10) + selectionToMs := strconv.FormatInt(time.Now().Add(time.Hour).UnixMilli(), 10) + testCases := []struct { name string dashboard *models.Dashboard pubdash *PublicDashboard timeResult TimeSettings + reqDTO PublicDashboardQueryDTO }{ { name: "should use dashboard time if pubdash time empty", dashboard: &models.Dashboard{Data: dashboardData}, - pubdash: &PublicDashboard{}, + pubdash: &PublicDashboard{TimeSelectionEnabled: false}, timeResult: TimeSettings{ - From: fromMs, - To: toMs, + From: defaultFromMs, + To: defaultToMs, }, + reqDTO: PublicDashboardQueryDTO{}, }, { - name: "should use dashboard time even if pubdash time exists", + name: "should use dashboard time when time selection is disabled", dashboard: &models.Dashboard{Data: dashboardData}, - pubdash: &PublicDashboard{TimeSettings: &TimeSettings{From: "now-12", To: "now"}}, + pubdash: &PublicDashboard{TimeSelectionEnabled: false, TimeSettings: &TimeSettings{From: "now-12", To: "now"}}, timeResult: TimeSettings{ - From: fromMs, - To: toMs, + From: defaultFromMs, + To: defaultToMs, + }, + reqDTO: PublicDashboardQueryDTO{}, + }, + { + name: "should use selected values if time selection is enabled", + dashboard: &models.Dashboard{Data: dashboardData}, + pubdash: &PublicDashboard{TimeSelectionEnabled: true, TimeSettings: &TimeSettings{From: "now-12", To: "now"}}, + reqDTO: PublicDashboardQueryDTO{ + TimeRange: TimeSettings{ + From: selectionFromMs, + To: selectionToMs, + }, + }, + timeResult: TimeSettings{ + From: selectionFromMs, + To: selectionToMs, }, }, } for _, test := range testCases { t.Run(test.name, func(t *testing.T) { - assert.Equal(t, test.timeResult, test.pubdash.BuildTimeSettings(test.dashboard)) + assert.Equal(t, test.timeResult, test.pubdash.BuildTimeSettings(test.dashboard, test.reqDTO)) }) } } diff --git a/pkg/services/publicdashboards/service/query.go b/pkg/services/publicdashboards/service/query.go index d5c94b86993..42005beb02d 100644 --- a/pkg/services/publicdashboards/service/query.go +++ b/pkg/services/publicdashboards/service/query.go @@ -99,7 +99,7 @@ func (pd *PublicDashboardServiceImpl) FindAnnotations(ctx context.Context, reqDT // GetMetricRequest returns a metric request for the given panel and query func (pd *PublicDashboardServiceImpl) GetMetricRequest(ctx context.Context, dashboard *dashmodels.Dashboard, publicDashboard *models.PublicDashboard, panelId int64, queryDto models.PublicDashboardQueryDTO) (dtos.MetricRequest, error) { - err := validation.ValidateQueryPublicDashboardRequest(queryDto) + err := validation.ValidateQueryPublicDashboardRequest(queryDto, publicDashboard) if err != nil { return dtos.MetricRequest{}, err } @@ -158,7 +158,7 @@ func (pd *PublicDashboardServiceImpl) buildMetricRequest(ctx context.Context, da return dtos.MetricRequest{}, models.ErrPanelNotFound.Errorf("buildMetricRequest: public dashboard panel not found") } - ts := publicDashboard.BuildTimeSettings(dashboard) + ts := publicDashboard.BuildTimeSettings(dashboard, reqDTO) // determine safe resolution to query data at safeInterval, safeResolution := pd.getSafeIntervalAndMaxDataPoints(reqDTO, ts) diff --git a/pkg/services/publicdashboards/validation/validation.go b/pkg/services/publicdashboards/validation/validation.go index 727e2371580..70c2d25b703 100644 --- a/pkg/services/publicdashboards/validation/validation.go +++ b/pkg/services/publicdashboards/validation/validation.go @@ -3,6 +3,7 @@ package validation import ( "github.com/grafana/grafana/pkg/models" . "github.com/grafana/grafana/pkg/services/publicdashboards/models" + "github.com/grafana/grafana/pkg/tsdb/legacydata" ) func ValidatePublicDashboard(dto *SavePublicDashboardDTO, dashboard *models.Dashboard) error { @@ -19,7 +20,7 @@ func hasTemplateVariables(dashboard *models.Dashboard) bool { return len(templateVariables) > 0 } -func ValidateQueryPublicDashboardRequest(req PublicDashboardQueryDTO) error { +func ValidateQueryPublicDashboardRequest(req PublicDashboardQueryDTO, pd *PublicDashboard) error { if req.IntervalMs < 0 { return ErrInvalidInterval.Errorf("ValidateQueryPublicDashboardRequest: intervalMS should be greater than 0") } @@ -28,5 +29,18 @@ func ValidateQueryPublicDashboardRequest(req PublicDashboardQueryDTO) error { return ErrInvalidMaxDataPoints.Errorf("ValidateQueryPublicDashboardRequest: maxDataPoints should be greater than 0") } + if pd.TimeSelectionEnabled { + timeRange := legacydata.NewDataTimeRange(req.TimeRange.From, req.TimeRange.To) + + _, err := timeRange.ParseFrom() + if err != nil { + return ErrInvalidTimeRange.Errorf("ValidateQueryPublicDashboardRequest: time range from is invalid") + } + _, err = timeRange.ParseTo() + if err != nil { + return ErrInvalidTimeRange.Errorf("ValidateQueryPublicDashboardRequest: time range to is invalid") + } + } + return nil } diff --git a/pkg/services/publicdashboards/validation/validation_test.go b/pkg/services/publicdashboards/validation/validation_test.go index dbae8acc7db..8b606d05a27 100644 --- a/pkg/services/publicdashboards/validation/validation_test.go +++ b/pkg/services/publicdashboards/validation/validation_test.go @@ -42,3 +42,118 @@ func TestValidatePublicDashboard(t *testing.T) { require.NoError(t, err) }) } + +func TestValidateQueryPublicDashboardRequest(t *testing.T) { + type args struct { + req PublicDashboardQueryDTO + pd *PublicDashboard + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "Returns no error when input is valid", + args: args{ + req: PublicDashboardQueryDTO{ + IntervalMs: 1000, + MaxDataPoints: 1000, + TimeRange: TimeSettings{ + From: "now-1h", + To: "now", + }, + }, + pd: &PublicDashboard{ + TimeSelectionEnabled: true, + }, + }, + wantErr: false, + }, + { + name: "Returns no error when input is valid and time selection is disabled", + args: args{ + req: PublicDashboardQueryDTO{ + IntervalMs: 1000, + MaxDataPoints: 1000, + }, + pd: &PublicDashboard{ + TimeSelectionEnabled: false, + }, + }, + wantErr: false, + }, + { + name: "Returns validation error when intervalMs is less than 0", + args: args{ + req: PublicDashboardQueryDTO{ + IntervalMs: -1, + }, + pd: &PublicDashboard{}, + }, + wantErr: true, + }, + { + name: "Returns validation error when maxDataPoints is less than 0", + args: args{ + req: PublicDashboardQueryDTO{ + MaxDataPoints: -1, + }, + pd: &PublicDashboard{}, + }, + wantErr: true, + }, + { + name: "Returns validation error when time range from is invalid", + args: args{ + req: PublicDashboardQueryDTO{ + TimeRange: TimeSettings{ + From: "invalid", + To: "1622560000000", + }, + }, + pd: &PublicDashboard{ + TimeSelectionEnabled: true, + }, + }, + wantErr: true, + }, + { + name: "Returns validation error when time range to is invalid", + args: args{ + req: PublicDashboardQueryDTO{ + TimeRange: TimeSettings{ + From: "1622560000000", + To: "invalid", + }, + }, + pd: &PublicDashboard{ + TimeSelectionEnabled: true, + }, + }, + wantErr: true, + }, + { + name: "Returns validation error when time range from or to is blank", + args: args{ + req: PublicDashboardQueryDTO{ + TimeRange: TimeSettings{ + From: "", + To: "", + }, + }, + pd: &PublicDashboard{ + TimeSelectionEnabled: true, + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := ValidateQueryPublicDashboardRequest(tt.args.req, tt.args.pd); (err != nil) != tt.wantErr { + t.Errorf("ValidateQueryPublicDashboardRequest() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +}