snapshots: fixes cleanup of old snapshots

Snapshot cleanup did not work due to time.Now syntax error. Added test
for it as well to catch any future errors.

Added error and debug logging so that it is possible to see any errors in the future.

Removed an unused configuration value and deprecated the remove expired snapshots
setting.

pkg/services/sqlstore/dashboard_snapshot.go

@@ -16,20 +16,23 @@ func init() {
 	bus.AddHandler("sql", DeleteExpiredSnapshots)
 }
 
+// DeleteExpiredSnapshots removes snapshots with old expiry dates.
+// SnapShotRemoveExpired is deprecated and should be removed in the future.
+// Snapshot expiry is decided by the user when they share the snapshot.
 func DeleteExpiredSnapshots(cmd *m.DeleteExpiredSnapshotsCommand) error {
 	return inTransaction(func(sess *DBSession) error {
-		var expiredCount int64 = 0
-
-		if setting.SnapShotRemoveExpired {
-			deleteExpiredSql := "DELETE FROM dashboard_snapshot WHERE expires < ?"
-			expiredResponse, err := x.Exec(deleteExpiredSql, time.Now)
-			if err != nil {
-				return err
-			}
-			expiredCount, _ = expiredResponse.RowsAffected()
+		if !setting.SnapShotRemoveExpired {
+			sqlog.Warn("[Deprecated] The snapshot_remove_expired setting is outdated. Please remove from your config.")
+			return nil
 		}
 
-		sqlog.Debug("Deleted old/expired snaphots", "expired", expiredCount)
+		deleteExpiredSql := "DELETE FROM dashboard_snapshot WHERE expires < ?"
+		expiredResponse, err := sess.Exec(deleteExpiredSql, time.Now())
+		if err != nil {
+			return err
+		}
+		cmd.DeletedRows, _ = expiredResponse.RowsAffected()
+
 		return nil
 	})
 }

pkg/services/sqlstore/dashboard_snapshot_test.go

@@ -2,11 +2,14 @@ package sqlstore
 
 import (
 	"testing"
+	"time"
 
+	"github.com/go-xorm/xorm"
 	. "github.com/smartystreets/goconvey/convey"
 
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	m "github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/setting"
 )
 
 func TestDashboardSnapshotDBAccess(t *testing.T) {
@@ -105,3 +108,65 @@ func TestDashboardSnapshotDBAccess(t *testing.T) {
 		})
 	})
 }
+
+func TestDeleteExpiredSnapshots(t *testing.T) {
+	Convey("Testing dashboard snapshots clean up", t, func() {
+		x := InitTestDB(t)
+
+		setting.SnapShotRemoveExpired = true
+
+		notExpiredsnapshot := createTestSnapshot(x, "key1", 1000)
+		createTestSnapshot(x, "key2", -1000)
+		createTestSnapshot(x, "key3", -1000)
+
+		Convey("Clean up old dashboard snapshots", func() {
+			err := DeleteExpiredSnapshots(&m.DeleteExpiredSnapshotsCommand{})
+			So(err, ShouldBeNil)
+
+			query := m.GetDashboardSnapshotsQuery{
+				OrgId:        1,
+				SignedInUser: &m.SignedInUser{OrgRole: m.ROLE_ADMIN},
+			}
+			err = SearchDashboardSnapshots(&query)
+			So(err, ShouldBeNil)
+
+			So(len(query.Result), ShouldEqual, 1)
+			So(query.Result[0].Key, ShouldEqual, notExpiredsnapshot.Key)
+		})
+
+		Convey("Don't delete anything if there are no expired snapshots", func() {
+			err := DeleteExpiredSnapshots(&m.DeleteExpiredSnapshotsCommand{})
+			So(err, ShouldBeNil)
+
+			query := m.GetDashboardSnapshotsQuery{
+				OrgId:        1,
+				SignedInUser: &m.SignedInUser{OrgRole: m.ROLE_ADMIN},
+			}
+			SearchDashboardSnapshots(&query)
+
+			So(len(query.Result), ShouldEqual, 1)
+		})
+	})
+}
+
+func createTestSnapshot(x *xorm.Engine, key string, expires int64) *m.DashboardSnapshot {
+	cmd := m.CreateDashboardSnapshotCommand{
+		Key:       key,
+		DeleteKey: "delete" + key,
+		Dashboard: simplejson.NewFromAny(map[string]interface{}{
+			"hello": "mupp",
+		}),
+		UserId:  1000,
+		OrgId:   1,
+		Expires: expires,
+	}
+	err := CreateDashboardSnapshot(&cmd)
+	So(err, ShouldBeNil)
+
+	// Set expiry date manually - to be able to create expired snapshots
+	expireDate := time.Now().Add(time.Second * time.Duration(expires))
+	_, err = x.Exec("update dashboard_snapshot set expires = ? where "+dialect.Quote("key")+" = ?", expireDate, key)
+	So(err, ShouldBeNil)
+
+	return cmd.Result
+}

pkg/services/sqlstore/dashboard_version.go

@@ -69,7 +69,6 @@ func GetDashboardVersions(query *m.GetDashboardVersionsQuery) error {
 
 func DeleteExpiredVersions(cmd *m.DeleteExpiredVersionsCommand) error {
 	return inTransaction(func(sess *DBSession) error {
-		expiredCount := int64(0)
 		versions := []DashboardVersionExp{}
 		versionsToKeep := setting.DashboardVersionsToKeep
 
@@ -98,8 +97,7 @@ func DeleteExpiredVersions(cmd *m.DeleteExpiredVersionsCommand) error {
 			if err != nil {
 				return err
 			}
-			expiredCount, _ = expiredResponse.RowsAffected()
-			sqlog.Debug("Deleted old/expired dashboard versions", "expired", expiredCount)
+			cmd.DeletedRows, _ = expiredResponse.RowsAffected()
 		}
 
 		return nil