Dashboards: move validation to service (#98769)

pkg/services/dashboards/database/database.go

@@ -82,6 +82,8 @@ func (d *dashboardStore) emitEntityEvent() bool {
 	return d.features != nil && d.features.IsEnabledGlobally(featuremgmt.FlagPanelTitleSearch)
 }
 
+// TODO: once the folder service removes usage of this function, remove it here. The dashboard service now implements this
+// on the service level for dashboards.
 func (d *dashboardStore) ValidateDashboardBeforeSave(ctx context.Context, dashboard *dashboards.Dashboard, overwrite bool) (bool, error) {
 	ctx, span := tracer.Start(ctx, "dashboards.database.ValidateDashboardBeforesave")
 	defer span.End()

pkg/services/dashboards/service/dashboard_service.go

@@ -208,7 +208,7 @@ func (dr *DashboardServiceImpl) BuildSaveDashboardCommand(ctx context.Context, d
 		dash.FolderUID = folder.UID
 	}
 
-	isParentFolderChanged, err := dr.dashboardStore.ValidateDashboardBeforeSave(ctx, dash, dto.Overwrite)
+	isParentFolderChanged, err := dr.ValidateDashboardBeforeSave(ctx, dash, dto.Overwrite)
 	if err != nil {
 		return nil, err
 	}
@@ -290,6 +290,78 @@ func (dr *DashboardServiceImpl) BuildSaveDashboardCommand(ctx context.Context, d
 	return cmd, nil
 }
 
+func (dr *DashboardServiceImpl) ValidateDashboardBeforeSave(ctx context.Context, dashboard *dashboards.Dashboard, overwrite bool) (bool, error) {
+	ctx, span := tracer.Start(ctx, "dashboards.service.ValidateDashboardBeforesave")
+	defer span.End()
+
+	isParentFolderChanged := false
+
+	var existingById *dashboards.Dashboard
+	var err error
+	if dashboard.ID > 0 {
+		// if ID is set and the dashboard is not found, ErrDashboardNotFound will be returned
+		existingById, err = dr.GetDashboard(ctx, &dashboards.GetDashboardQuery{OrgID: dashboard.OrgID, ID: dashboard.ID})
+		if err != nil {
+			return false, err
+		}
+
+		if dashboard.UID == "" {
+			dashboard.SetUID(existingById.UID)
+		}
+	}
+	dashWithIdExists := (existingById != nil)
+
+	var existingByUid *dashboards.Dashboard
+	if dashboard.UID != "" {
+		existingByUid, err = dr.GetDashboard(ctx, &dashboards.GetDashboardQuery{OrgID: dashboard.OrgID, UID: dashboard.UID})
+		if err != nil && !errors.Is(err, dashboards.ErrDashboardNotFound) {
+			return false, err
+		}
+	}
+	dashWithUidExists := (existingByUid != nil)
+
+	if !dashWithIdExists && !dashWithUidExists {
+		return false, nil
+	}
+
+	if dashWithIdExists && dashWithUidExists && existingById.ID != existingByUid.ID {
+		return false, dashboards.ErrDashboardWithSameUIDExists
+	}
+
+	existing := existingById
+
+	if !dashWithIdExists && dashWithUidExists {
+		dashboard.SetID(existingByUid.ID)
+		dashboard.SetUID(existingByUid.UID)
+		existing = existingByUid
+	}
+
+	if (existing.IsFolder && !dashboard.IsFolder) ||
+		(!existing.IsFolder && dashboard.IsFolder) {
+		return isParentFolderChanged, dashboards.ErrDashboardTypeMismatch
+	}
+
+	if !dashboard.IsFolder && dashboard.FolderUID != existing.FolderUID {
+		isParentFolderChanged = true
+	}
+
+	// check for is someone else has written in between
+	if dashboard.Version != existing.Version {
+		if overwrite {
+			dashboard.SetVersion(existing.Version)
+		} else {
+			return isParentFolderChanged, dashboards.ErrDashboardVersionMismatch
+		}
+	}
+
+	// do not allow plugin dashboard updates without overwrite flag
+	if existing.PluginID != "" && !overwrite {
+		return isParentFolderChanged, dashboards.UpdatePluginDashboardError{PluginId: existing.PluginID}
+	}
+
+	return isParentFolderChanged, nil
+}
+
 func (dr *DashboardServiceImpl) DeleteOrphanedProvisionedDashboards(ctx context.Context, cmd *dashboards.DeleteOrphanedProvisionedDashboardsCommand) error {
 	// TODO: once we can search in unistore by id, go through k8s cli too
 	return dr.dashboardStore.DeleteOrphanedProvisionedDashboards(ctx, cmd)

pkg/services/dashboards/service/dashboard_service_test.go

@@ -88,7 +88,7 @@ func TestDashboardService(t *testing.T) {
 					dto.User = &user.SignedInUser{}
 
 					if tc.Error == nil {
-						fakeStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.Anything, mock.AnythingOfType("bool")).Return(true, nil).Once()
+						fakeStore.On("GetDashboard", mock.Anything, mock.Anything).Return(&dashboards.Dashboard{}, nil).Once()
 					}
 					_, err := service.BuildSaveDashboardCommand(context.Background(), dto, false)
 					require.Equal(t, err, tc.Error)
@@ -106,7 +106,7 @@ func TestDashboardService(t *testing.T) {
 			})
 
 			t.Run("Should return validation error if dashboard is provisioned", func(t *testing.T) {
-				fakeStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.Anything, mock.AnythingOfType("bool")).Return(true, nil).Once()
+				fakeStore.On("GetDashboard", mock.Anything, mock.Anything).Return(&dashboards.Dashboard{}, nil).Once()
 				fakeStore.On("GetProvisionedDataByDashboardID", mock.Anything, mock.AnythingOfType("int64")).Return(&dashboards.DashboardProvisioning{}, nil).Once()
 
 				dto.Dashboard = dashboards.NewDashboard("Dash")
@@ -117,7 +117,7 @@ func TestDashboardService(t *testing.T) {
 			})
 
 			t.Run("Should not return validation error if dashboard is provisioned but UI updates allowed", func(t *testing.T) {
-				fakeStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.Anything, mock.AnythingOfType("bool")).Return(true, nil).Once()
+				fakeStore.On("GetDashboard", mock.Anything, mock.Anything).Return(&dashboards.Dashboard{}, nil).Once()
 				fakeStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(&dashboards.Dashboard{Data: simplejson.New()}, nil).Once()
 
 				dto.Dashboard = dashboards.NewDashboard("Dash")
@@ -132,7 +132,6 @@ func TestDashboardService(t *testing.T) {
 			dto := &dashboards.SaveDashboardDTO{}
 
 			t.Run("Should not return validation error if dashboard is provisioned", func(t *testing.T) {
-				fakeStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.Anything, mock.AnythingOfType("bool")).Return(true, nil).Once()
 				fakeStore.On("SaveProvisionedDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand"), mock.AnythingOfType("*dashboards.DashboardProvisioning")).Return(&dashboards.Dashboard{Data: simplejson.New()}, nil).Once()
 
 				dto.Dashboard = dashboards.NewDashboard("Dash")
@@ -143,7 +142,6 @@ func TestDashboardService(t *testing.T) {
 			})
 
 			t.Run("Should override invalid refresh interval if dashboard is provisioned", func(t *testing.T) {
-				fakeStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.Anything, mock.AnythingOfType("bool")).Return(true, nil).Once()
 				fakeStore.On("SaveProvisionedDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand"), mock.AnythingOfType("*dashboards.DashboardProvisioning")).Return(&dashboards.Dashboard{Data: simplejson.New()}, nil).Once()
 
 				oldRefreshInterval := service.cfg.MinRefreshInterval
@@ -164,7 +162,6 @@ func TestDashboardService(t *testing.T) {
 			dto := &dashboards.SaveDashboardDTO{}
 
 			t.Run("Should return validation error if dashboard is provisioned", func(t *testing.T) {
-				fakeStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.Anything, mock.AnythingOfType("bool")).Return(true, nil).Once()
 				fakeStore.On("GetProvisionedDataByDashboardID", mock.Anything, mock.AnythingOfType("int64")).Return(&dashboards.DashboardProvisioning{}, nil).Once()
 
 				dto.Dashboard = dashboards.NewDashboard("Dash")
@@ -594,7 +591,7 @@ func TestSaveDashboard(t *testing.T) {
 	t.Run("Should fallback to dashboard store if Kubernetes feature flags are not enabled", func(t *testing.T) {
 		service.features = featuremgmt.WithFeatures()
 		fakeStore.On("GetProvisionedDataByDashboardID", mock.Anything, mock.Anything).Return(nil, nil)
-		fakeStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.Anything, mock.AnythingOfType("bool")).Return(true, nil)
+		fakeStore.On("GetDashboard", mock.Anything, mock.Anything).Return(&dashboards.Dashboard{}, nil)
 		fakeStore.On("SaveDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&dashboards.Dashboard{}, nil)
 		dashboard, err := service.SaveDashboard(context.Background(), query, false)
 		require.NoError(t, err)
@@ -616,7 +613,7 @@ func TestSaveDashboard(t *testing.T) {
 	t.Run("Should use Kubernetes create if feature flags are enabled and dashboard doesn't exist", func(t *testing.T) {
 		ctx, k8sClientMock, k8sResourceMock := setupK8sDashboardTests(service)
 		k8sClientMock.On("getClient", mock.Anything, int64(1)).Return(k8sResourceMock, true)
-		k8sResourceMock.On("Get", mock.Anything, query.Dashboard.UID, mock.Anything, mock.Anything).Return(nil, nil)
+		k8sResourceMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, nil)
 		k8sResourceMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil)
 
 		dashboard, err := service.SaveDashboard(ctx, query, false)
@@ -627,13 +624,24 @@ func TestSaveDashboard(t *testing.T) {
 	t.Run("Should use Kubernetes update if feature flags are enabled and dashboard exists", func(t *testing.T) {
 		ctx, k8sClientMock, k8sResourceMock := setupK8sDashboardTests(service)
 		k8sClientMock.On("getClient", mock.Anything, int64(1)).Return(k8sResourceMock, true)
-		k8sResourceMock.On("Get", mock.Anything, query.Dashboard.UID, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil)
+		k8sResourceMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil)
 		k8sResourceMock.On("Update", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil)
 
 		dashboard, err := service.SaveDashboard(ctx, query, false)
 		require.NoError(t, err)
 		require.NotNil(t, dashboard)
 	})
+
+	t.Run("Should return an error if uid is invalid", func(t *testing.T) {
+		ctx, k8sClientMock, k8sResourceMock := setupK8sDashboardTests(service)
+		k8sClientMock.On("getClient", mock.Anything, int64(1)).Return(k8sResourceMock, true)
+		k8sResourceMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, nil)
+		k8sResourceMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil)
+
+		query.Dashboard.UID = "invalid/uid"
+		_, err := service.SaveDashboard(ctx, query, false)
+		require.Error(t, err)
+	})
 }
 
 func TestDeleteDashboard(t *testing.T) {