From 4270d8a67f00ab2f46867188f073489b5959477c Mon Sep 17 00:00:00 2001
From: Jarek Kowalski <jaak@jkowalski.net>
Date: Sun, 16 Sep 2018 12:48:07 -0700
Subject: [PATCH] policy: refactored retention policy to be applied in
 policy.ApplyRetentionPolicy()

---
 cli/command_snapshot_create.go    | 20 ++------------------
 cli/command_snapshot_expire.go    | 24 +++++++-----------------
 internal/server/source_manager.go |  5 +++++
 policy/expire.go                  | 26 ++++++++++++++++++++++++--
 4 files changed, 38 insertions(+), 37 deletions(-)

diff --git a/cli/command_snapshot_create.go b/cli/command_snapshot_create.go
index c75199203..8dac36385 100644
--- a/cli/command_snapshot_create.go
+++ b/cli/command_snapshot_create.go
@@ -115,24 +115,8 @@ func snapshotSingleSource(ctx context.Context, rep *repo.Repository, u *upload.U
 	printStderr("uploaded snapshot %v (root %v) in %v\n", snapID, manifest.RootObjectID(), time.Since(t0))
 	log.Debugf("Hash Cache: %v", manifest.HashCacheID.String())
 
-	snapshots, err := snapshot.ListSnapshots(ctx, rep, sourceInfo)
-	if err != nil {
-		return err
-	}
-
-	toDelete, err := policy.GetExpiredSnapshots(ctx, rep, snapshots)
-	if err != nil {
-		return err
-	}
-
-	printStderr("Deleting %v expired snapshots of %v...\n", len(toDelete), sourceInfo)
-	for _, it := range toDelete {
-		if err := rep.Manifests.Delete(ctx, it.ID); err != nil {
-			return err
-		}
-	}
-
-	return nil
+	_, err = policy.ApplyRetentionPolicy(ctx, rep, sourceInfo, true)
+	return err
 }
 
 func findPreviousSnapshotManifest(ctx context.Context, rep *repo.Repository, sourceInfo snapshot.SourceInfo) (*snapshot.Manifest, error) {
diff --git a/cli/command_snapshot_expire.go b/cli/command_snapshot_expire.go
index 80b38abad..fdf077a7d 100644
--- a/cli/command_snapshot_expire.go
+++ b/cli/command_snapshot_expire.go
@@ -14,7 +14,7 @@
 
 	snapshotExpireAll    = snapshotExpireCommand.Flag("all", "Expire all snapshots").Bool()
 	snapshotExpirePaths  = snapshotExpireCommand.Arg("path", "Expire snapshots for a given paths only").Strings()
-	snapshotExpireDelete = snapshotExpireCommand.Flag("delete", "Whether to actually delete snapshots").Default("no").String()
+	snapshotExpireDelete = snapshotExpireCommand.Flag("delete", "Whether to actually delete snapshots").Bool()
 )
 
 func getSnapshotSourcesToExpire(ctx context.Context, rep *repo.Repository) ([]snapshot.SourceInfo, error) {
@@ -45,30 +45,20 @@ func runExpireCommand(ctx context.Context, rep *repo.Repository) error {
 	})
 
 	for _, src := range sources {
-		snapshots, err := snapshot.ListSnapshots(ctx, rep, src)
+		deleted, err := policy.ApplyRetentionPolicy(ctx, rep, src, *snapshotExpireDelete)
 		if err != nil {
 			return err
 		}
 
-		toDelete, err := policy.GetExpiredSnapshots(ctx, rep, snapshots)
-		if err != nil {
-			return err
-		}
-
-		if len(toDelete) == 0 {
+		if len(deleted) == 0 {
 			printStderr("Nothing to delete for %v.\n", src)
 			continue
 		}
-		if *snapshotExpireDelete == "yes" {
-			printStderr("Deleting %v snapshots of %v...\n", len(toDelete), src)
-			for _, it := range toDelete {
-				if err := rep.Manifests.Delete(ctx, it.ID); err != nil {
-					return err
-				}
-			}
+		if *snapshotExpireDelete {
+			printStderr("Deleted %v snapshots of %v...\n", len(deleted), src)
 		} else {
-			printStderr("%v snapshot(s) of %v would be deleted. Pass --delete=yes to do it.\n", len(toDelete), src)
-			for _, it := range toDelete {
+			printStderr("%v snapshot(s) of %v would be deleted. Pass --delete to do it.\n", len(deleted), src)
+			for _, it := range deleted {
 				printStderr("  %v\n", it.StartTime.Format(timeFormat))
 			}
 		}
diff --git a/internal/server/source_manager.go b/internal/server/source_manager.go
index a71321b50..0da68d583 100644
--- a/internal/server/source_manager.go
+++ b/internal/server/source_manager.go
@@ -185,6 +185,11 @@ func (s *sourceManager) snapshot(ctx context.Context) {
 		return
 	}
 
+	if _, err := policy.ApplyRetentionPolicy(ctx, s.server.rep, s.src, true); err != nil {
+		log.Errorf("unable to apply retention policy: %v", err)
+		return
+	}
+
 	log.Infof("created snapshot %v", snapshotID)
 	if err := s.server.rep.Flush(ctx); err != nil {
 		log.Errorf("unable to flush: %v", err)
diff --git a/policy/expire.go b/policy/expire.go
index fb533da74..df6e465c8 100644
--- a/policy/expire.go
+++ b/policy/expire.go
@@ -8,8 +8,30 @@
 	"github.com/kopia/kopia/snapshot"
 )
 
-// GetExpiredSnapshots computes the set of snapshot manifests that are not retained according to the policy.
-func GetExpiredSnapshots(ctx context.Context, rep *repo.Repository, snapshots []*snapshot.Manifest) ([]*snapshot.Manifest, error) {
+// ApplyRetentionPolicy applies retention policy to a given source by deleting expired snapshots.
+func ApplyRetentionPolicy(ctx context.Context, rep *repo.Repository, sourceInfo snapshot.SourceInfo, reallyDelete bool) ([]*snapshot.Manifest, error) {
+	snapshots, err := snapshot.ListSnapshots(ctx, rep, sourceInfo)
+	if err != nil {
+		return nil, err
+	}
+
+	toDelete, err := getExpiredSnapshots(ctx, rep, snapshots)
+	if err != nil {
+		return nil, err
+	}
+
+	if reallyDelete {
+		for _, it := range toDelete {
+			if err := rep.Manifests.Delete(ctx, it.ID); err != nil {
+				return toDelete, err
+			}
+		}
+	}
+
+	return toDelete, nil
+}
+
+func getExpiredSnapshots(ctx context.Context, rep *repo.Repository, snapshots []*snapshot.Manifest) ([]*snapshot.Manifest, error) {
 	var toDelete []*snapshot.Manifest
 	for _, snapshotGroup := range snapshot.GroupBySource(snapshots) {
 		td, err := getExpiredSnapshotsForSource(ctx, rep, snapshotGroup)