public-dashboards: refactor query method (#54119)

This PR refactors the GetPublicDashboard method so we don't need to make extra database calls. Also adds general documentation

pkg/services/publicdashboards/api/api.go

@@ -54,6 +54,7 @@ func ProvideApi(
 	return api
 }
 
+//Registers Endpoints on Grafana Router
 func (api *Api) RegisterAPIEndpoints() {
 	auth := accesscontrol.Middleware(api.AccessControl)
 	reqSignedIn := middleware.ReqSignedIn
@@ -70,20 +71,20 @@ func (api *Api) RegisterAPIEndpoints() {
 	api.RouteRegister.Post("/api/dashboards/uid/:uid/public-config", auth(reqSignedIn, accesscontrol.EvalPermission(dashboards.ActionDashboardsWrite)), routing.Wrap(api.SavePublicDashboardConfig))
 }
 
-// gets public dashboard
+// Gets public dashboard
+// GET /api/public/dashboards/:accessToken
 func (api *Api) GetPublicDashboard(c *models.ReqContext) response.Response {
 	accessToken := web.Params(c.Req)[":accessToken"]
 
-	dash, err := api.PublicDashboardService.GetPublicDashboard(c.Req.Context(), accessToken)
+	pubdash, dash, err := api.PublicDashboardService.GetPublicDashboard(
+		c.Req.Context(),
+		web.Params(c.Req)[":accessToken"],
+	)
+
 	if err != nil {
 		return handleDashboardErr(http.StatusInternalServerError, "Failed to get public dashboard", err)
 	}
 
-	pubDash, err := api.PublicDashboardService.GetPublicDashboardConfig(c.Req.Context(), dash.OrgId, dash.Uid)
-	if err != nil {
-		return handleDashboardErr(http.StatusInternalServerError, "Failed to get public dashboard config", err)
-	}
-
 	meta := dtos.DashboardMeta{
 		Slug:                       dash.Slug,
 		Type:                       models.DashTypeDB,
@@ -98,7 +99,7 @@ func (api *Api) GetPublicDashboard(c *models.ReqContext) response.Response {
 		IsFolder:                   false,
 		FolderId:                   dash.FolderId,
 		PublicDashboardAccessToken: accessToken,
-		PublicDashboardUID:         pubDash.Uid,
+		PublicDashboardUID:         pubdash.Uid,
 	}
 
 	dto := dtos.DashboardFullWithMeta{Meta: meta, Dashboard: dash.Data}
@@ -106,7 +107,8 @@ func (api *Api) GetPublicDashboard(c *models.ReqContext) response.Response {
 	return response.JSON(http.StatusOK, dto)
 }
 
-// gets public dashboard configuration for dashboard
+// Gets public dashboard configuration for dashboard
+// GET /api/dashboards/uid/:uid/public-config
 func (api *Api) GetPublicDashboardConfig(c *models.ReqContext) response.Response {
 	pdc, err := api.PublicDashboardService.GetPublicDashboardConfig(c.Req.Context(), c.OrgID, web.Params(c.Req)[":uid"])
 	if err != nil {
@@ -115,7 +117,8 @@ func (api *Api) GetPublicDashboardConfig(c *models.ReqContext) response.Response
 	return response.JSON(http.StatusOK, pdc)
 }
 
-// sets public dashboard configuration for dashboard
+// Sets public dashboard configuration for dashboard
+// POST /api/dashboards/uid/:uid/public-config
 func (api *Api) SavePublicDashboardConfig(c *models.ReqContext) response.Response {
 	pubdash := &PublicDashboard{}
 	if err := web.Bind(c.Req, pubdash); err != nil {
@@ -149,32 +152,30 @@ func (api *Api) QueryPublicDashboard(c *models.ReqContext) response.Response {
 		return response.Error(http.StatusBadRequest, "invalid panel ID", err)
 	}
 
-	dashboard, err := api.PublicDashboardService.GetPublicDashboard(c.Req.Context(), web.Params(c.Req)[":accessToken"])
+	// Get the dashboard
+	pubdash, dashboard, err := api.PublicDashboardService.GetPublicDashboard(c.Req.Context(), web.Params(c.Req)[":accessToken"])
 	if err != nil {
 		return response.Error(http.StatusInternalServerError, "could not fetch dashboard", err)
 	}
 
-	publicDashboard, err := api.PublicDashboardService.GetPublicDashboardConfig(c.Req.Context(), dashboard.OrgId, dashboard.Uid)
+	// Build the request data objecct
-	if err != nil {
-		return response.Error(http.StatusInternalServerError, "could not fetch public dashboard", err)
-	}
-
 	reqDTO, err := api.PublicDashboardService.BuildPublicDashboardMetricRequest(
 		c.Req.Context(),
 		dashboard,
-		publicDashboard,
+		pubdash,
 		panelId,
 	)
 	if err != nil {
 		return handleDashboardErr(http.StatusInternalServerError, "Failed to get queries for public dashboard", err)
 	}
 
+	// Build anonymous user for the request
 	anonymousUser, err := api.PublicDashboardService.BuildAnonymousUser(c.Req.Context(), dashboard)
-
 	if err != nil {
 		return response.Error(http.StatusInternalServerError, "could not create anonymous user", err)
 	}
 
+	// Make the request
 	resp, err := api.QueryDataService.QueryDataMultipleSources(c.Req.Context(), anonymousUser, c.SkipCache, reqDTO, true)
 
 	if err != nil {

pkg/services/publicdashboards/api/api_test.go

@@ -43,10 +43,7 @@ func TestAPIGetPublicDashboard(t *testing.T) {
 		qs := buildQueryDataService(t, nil, nil, nil)
 		service := publicdashboards.NewFakePublicDashboardService(t)
 		service.On("GetPublicDashboard", mock.Anything, mock.AnythingOfType("string")).
-			Return(&models.Dashboard{}, nil).Maybe()
+			Return(&PublicDashboard{}, &models.Dashboard{}, nil).Maybe()
-		service.On("GetPublicDashboardConfig", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).
-			Return(&PublicDashboard{}, nil).Maybe()
-
 		testServer := setupTestServer(t, cfg, qs, featuremgmt.WithFeatures(), service, nil)
 
 		response := callAPI(testServer, http.MethodGet, "/api/public/dashboards", nil, t)
@@ -67,29 +64,29 @@ func TestAPIGetPublicDashboard(t *testing.T) {
 	accessToken := fmt.Sprintf("%x", token)
 
 	testCases := []struct {
-		Name                  string
+		Name                 string
-		AccessToken           string
+		AccessToken          string
-		ExpectedHttpResponse  int
+		ExpectedHttpResponse int
-		PublicDashboardResult *models.Dashboard
+		DashboardResult      *models.Dashboard
-		PublicDashboardErr    error
+		Err                  error
 	}{
 		{
 			Name:                 "It gets a public dashboard",
 			AccessToken:          accessToken,
 			ExpectedHttpResponse: http.StatusOK,
-			PublicDashboardResult: &models.Dashboard{
+			DashboardResult: &models.Dashboard{
 				Data: simplejson.NewFromAny(map[string]interface{}{
 					"Uid": DashboardUid,
 				}),
 			},
-			PublicDashboardErr: nil,
+			Err: nil,
 		},
 		{
-			Name:                  "It should return 404 if no public dashboard",
+			Name:                 "It should return 404 if no public dashboard",
-			AccessToken:           accessToken,
+			AccessToken:          accessToken,
-			ExpectedHttpResponse:  http.StatusNotFound,
+			ExpectedHttpResponse: http.StatusNotFound,
-			PublicDashboardResult: nil,
+			DashboardResult:      nil,
-			PublicDashboardErr:    ErrPublicDashboardNotFound,
+			Err:                  ErrPublicDashboardNotFound,
 		},
 	}
 
@@ -97,9 +94,7 @@ func TestAPIGetPublicDashboard(t *testing.T) {
 		t.Run(test.Name, func(t *testing.T) {
 			service := publicdashboards.NewFakePublicDashboardService(t)
 			service.On("GetPublicDashboard", mock.Anything, mock.AnythingOfType("string")).
-				Return(test.PublicDashboardResult, test.PublicDashboardErr).Maybe()
+				Return(&PublicDashboard{}, test.DashboardResult, test.Err).Maybe()
-			service.On("GetPublicDashboardConfig", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).
-				Return(&PublicDashboard{}, nil).Maybe()
 
 			cfg := setting.NewCfg()
 			cfg.RBACEnabled = false
@@ -121,7 +116,7 @@ func TestAPIGetPublicDashboard(t *testing.T) {
 
 			assert.Equal(t, test.ExpectedHttpResponse, response.Code)
 
-			if test.PublicDashboardErr == nil {
+			if test.Err == nil {
 				var dashResp dtos.DashboardFullWithMeta
 				err := json.Unmarshal(response.Body.Bytes(), &dashResp)
 				require.NoError(t, err)
@@ -136,7 +131,7 @@ func TestAPIGetPublicDashboard(t *testing.T) {
 				}
 				err := json.Unmarshal(response.Body.Bytes(), &errResp)
 				require.NoError(t, err)
-				assert.Equal(t, test.PublicDashboardErr.Error(), errResp.Error)
+				assert.Equal(t, test.Err.Error(), errResp.Error)
 			}
 		})
 	}
@@ -349,8 +344,7 @@ func TestAPIQueryPublicDashboard(t *testing.T) {
 	t.Run("Returns query data when feature toggle is enabled", func(t *testing.T) {
 		server, fakeDashboardService := setup(true)
 
-		fakeDashboardService.On("GetPublicDashboard", mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil)
+		fakeDashboardService.On("GetPublicDashboard", mock.Anything, mock.Anything).Return(&PublicDashboard{}, &models.Dashboard{}, nil)
-		fakeDashboardService.On("GetPublicDashboardConfig", mock.Anything, mock.Anything, mock.Anything).Return(&PublicDashboard{}, nil)
 		fakeDashboardService.On("BuildAnonymousUser", mock.Anything, mock.Anything, mock.Anything).Return(&user.SignedInUser{}, nil)
 		fakeDashboardService.On("BuildPublicDashboardMetricRequest", mock.Anything, mock.Anything, mock.Anything, int64(2)).Return(dtos.MetricRequest{
 			Queries: []*simplejson.Json{
@@ -400,8 +394,7 @@ func TestAPIQueryPublicDashboard(t *testing.T) {
 	t.Run("Status code is 500 when the query fails", func(t *testing.T) {
 		server, fakeDashboardService := setup(true)
 
-		fakeDashboardService.On("GetPublicDashboard", mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil)
+		fakeDashboardService.On("GetPublicDashboard", mock.Anything, mock.Anything).Return(&PublicDashboard{}, &models.Dashboard{}, nil)
-		fakeDashboardService.On("GetPublicDashboardConfig", mock.Anything, mock.Anything, mock.Anything).Return(&PublicDashboard{}, nil)
 		fakeDashboardService.On("BuildAnonymousUser", mock.Anything, mock.Anything, mock.Anything).Return(&user.SignedInUser{}, nil)
 		fakeDashboardService.On("BuildPublicDashboardMetricRequest", mock.Anything, mock.Anything, mock.Anything, int64(2)).Return(dtos.MetricRequest{
 			Queries: []*simplejson.Json{
@@ -430,8 +423,7 @@ func TestAPIQueryPublicDashboard(t *testing.T) {
 	t.Run("Status code is 200 when a panel has queries from multiple datasources", func(t *testing.T) {
 		server, fakeDashboardService := setup(true)
 
-		fakeDashboardService.On("GetPublicDashboard", mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil)
+		fakeDashboardService.On("GetPublicDashboard", mock.Anything, mock.Anything).Return(&PublicDashboard{}, &models.Dashboard{}, nil)
-		fakeDashboardService.On("GetPublicDashboardConfig", mock.Anything, mock.Anything, mock.Anything).Return(&PublicDashboard{}, nil)
 		fakeDashboardService.On("BuildAnonymousUser", mock.Anything, mock.Anything, mock.Anything).Return(&user.SignedInUser{}, nil)
 		fakeDashboardService.On("BuildPublicDashboardMetricRequest", mock.Anything, mock.Anything, mock.Anything, int64(2)).Return(dtos.MetricRequest{
 			Queries: []*simplejson.Json{

pkg/services/publicdashboards/public_dashboard_service_mock.go

@@ -9,11 +9,12 @@ import (
 	mock "github.com/stretchr/testify/mock"
 
 	models "github.com/grafana/grafana/pkg/models"
-	"github.com/grafana/grafana/pkg/services/user"
 
 	publicdashboardsmodels "github.com/grafana/grafana/pkg/services/publicdashboards/models"
 
 	testing "testing"
+
+	user "github.com/grafana/grafana/pkg/services/user"
 )
 
 // FakePublicDashboardService is an autogenerated mock type for the Service type
@@ -110,26 +111,35 @@ func (_m *FakePublicDashboardService) GetDashboard(ctx context.Context, dashboar
 }
 
 // GetPublicDashboard provides a mock function with given fields: ctx, accessToken
-func (_m *FakePublicDashboardService) GetPublicDashboard(ctx context.Context, accessToken string) (*models.Dashboard, error) {
+func (_m *FakePublicDashboardService) GetPublicDashboard(ctx context.Context, accessToken string) (*publicdashboardsmodels.PublicDashboard, *models.Dashboard, error) {
 	ret := _m.Called(ctx, accessToken)
 
-	var r0 *models.Dashboard
+	var r0 *publicdashboardsmodels.PublicDashboard
-	if rf, ok := ret.Get(0).(func(context.Context, string) *models.Dashboard); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, string) *publicdashboardsmodels.PublicDashboard); ok {
 		r0 = rf(ctx, accessToken)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(*models.Dashboard)
+			r0 = ret.Get(0).(*publicdashboardsmodels.PublicDashboard)
 		}
 	}
 
-	var r1 error
+	var r1 *models.Dashboard
-	if rf, ok := ret.Get(1).(func(context.Context, string) error); ok {
+	if rf, ok := ret.Get(1).(func(context.Context, string) *models.Dashboard); ok {
 		r1 = rf(ctx, accessToken)
 	} else {
-		r1 = ret.Error(1)
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*models.Dashboard)
+		}
 	}
 
-	return r0, r1
+	var r2 error
+	if rf, ok := ret.Get(2).(func(context.Context, string) error); ok {
+		r2 = rf(ctx, accessToken)
+	} else {
+		r2 = ret.Error(2)
+	}
+
+	return r0, r1, r2
 }
 
 // GetPublicDashboardConfig provides a mock function with given fields: ctx, orgId, dashboardUid

pkg/services/publicdashboards/publicdashboard.go

@@ -14,7 +14,7 @@ import (
 //go:generate mockery --name Service --structname FakePublicDashboardService --inpackage --filename public_dashboard_service_mock.go
 type Service interface {
 	BuildAnonymousUser(ctx context.Context, dashboard *models.Dashboard) (*user.SignedInUser, error)
-	GetPublicDashboard(ctx context.Context, accessToken string) (*models.Dashboard, error)
+	GetPublicDashboard(ctx context.Context, accessToken string) (*PublicDashboard, *models.Dashboard, error)
 	GetDashboard(ctx context.Context, dashboardUid string) (*models.Dashboard, error)
 	GetPublicDashboardConfig(ctx context.Context, orgId int64, dashboardUid string) (*PublicDashboard, error)
 	SavePublicDashboardConfig(ctx context.Context, dto *SavePublicDashboardConfigDTO) (*PublicDashboard, error)

pkg/services/publicdashboards/service/service.go

@@ -56,26 +56,22 @@ func (pd *PublicDashboardServiceImpl) GetDashboard(ctx context.Context, dashboar
 }
 
 // Gets public dashboard via access token
-func (pd *PublicDashboardServiceImpl) GetPublicDashboard(ctx context.Context, accessToken string) (*models.Dashboard, error) {
+func (pd *PublicDashboardServiceImpl) GetPublicDashboard(ctx context.Context, accessToken string) (*PublicDashboard, *models.Dashboard, error) {
-	pubdash, d, err := pd.store.GetPublicDashboard(ctx, accessToken)
+	pubdash, dash, err := pd.store.GetPublicDashboard(ctx, accessToken)
 
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
-	if pubdash == nil || d == nil {
+	if pubdash == nil || dash == nil {
-		return nil, ErrPublicDashboardNotFound
+		return nil, nil, ErrPublicDashboardNotFound
 	}
 
 	if !pubdash.IsEnabled {
-		return nil, ErrPublicDashboardNotFound
+		return nil, nil, ErrPublicDashboardNotFound
 	}
 
-	ts := pubdash.BuildTimeSettings(d)
+	return pubdash, dash, nil
-	d.Data.SetPath([]string{"time", "from"}, ts.From)
-	d.Data.SetPath([]string{"time", "to"}, ts.To)
-
-	return d, nil
 }
 
 // GetPublicDashboardConfig is a helper method to retrieve the public dashboard configuration for a given dashboard from the database

pkg/services/publicdashboards/service/service_test.go

@@ -25,7 +25,6 @@ import (
 var timeSettings, _ = simplejson.NewJson([]byte(`{"from": "now-12", "to": "now"}`))
 var defaultPubdashTimeSettings, _ = simplejson.NewJson([]byte(`{}`))
 var dashboardData = simplejson.NewFromAny(map[string]interface{}{"time": map[string]interface{}{"from": "now-8", "to": "now"}})
-var mergedDashboardData = simplejson.NewFromAny(map[string]interface{}{"time": map[string]interface{}{"from": "now-12", "to": "now"}})
 
 func TestLogPrefix(t *testing.T) {
 	assert.Equal(t, LogPrefix, "publicdashboards.service")
@@ -49,29 +48,18 @@ func TestGetPublicDashboard(t *testing.T) {
 			Name:        "returns a dashboard",
 			AccessToken: "abc123",
 			StoreResp: &storeResp{
-				pd:  &PublicDashboard{IsEnabled: true},
+				pd:  &PublicDashboard{AccessToken: "abcdToken", IsEnabled: true},
 				d:   &models.Dashboard{Uid: "mydashboard", Data: dashboardData},
 				err: nil,
 			},
 			ErrResp:  nil,
 			DashResp: &models.Dashboard{Uid: "mydashboard", Data: dashboardData},
 		},
-		{
-			Name:        "puts pubdash time settings into dashboard",
-			AccessToken: "abc123",
-			StoreResp: &storeResp{
-				pd:  &PublicDashboard{IsEnabled: true, TimeSettings: timeSettings},
-				d:   &models.Dashboard{Data: dashboardData},
-				err: nil,
-			},
-			ErrResp:  nil,
-			DashResp: &models.Dashboard{Data: mergedDashboardData},
-		},
 		{
 			Name:        "returns ErrPublicDashboardNotFound when isEnabled is false",
 			AccessToken: "abc123",
 			StoreResp: &storeResp{
-				pd:  &PublicDashboard{IsEnabled: false},
+				pd:  &PublicDashboard{AccessToken: "abcdToken", IsEnabled: false},
 				d:   &models.Dashboard{Uid: "mydashboard"},
 				err: nil,
 			},
@@ -105,17 +93,18 @@ func TestGetPublicDashboard(t *testing.T) {
 			fakeStore.On("GetPublicDashboard", mock.Anything, mock.Anything).
 				Return(test.StoreResp.pd, test.StoreResp.d, test.StoreResp.err)
 
-			dashboard, err := service.GetPublicDashboard(context.Background(), test.AccessToken)
+			pdc, dash, err := service.GetPublicDashboard(context.Background(), test.AccessToken)
 			if test.ErrResp != nil {
 				assert.Error(t, test.ErrResp, err)
 			} else {
 				require.NoError(t, err)
 			}
 
-			assert.Equal(t, test.DashResp, dashboard)
+			assert.Equal(t, test.DashResp, dash)
 
 			if test.DashResp != nil {
-				assert.NotNil(t, dashboard.CreatedBy)
+				assert.NotNil(t, dash.CreatedBy)
+				assert.Equal(t, test.StoreResp.pd, pdc)
 			}
 		})
 	}