From 23685de59b5b19d1937c7914a01879724577acf6 Mon Sep 17 00:00:00 2001
From: Umputun <umputun@gmail.com>
Date: Fri, 29 Dec 2017 19:35:23 -0600
Subject: [PATCH] isolate all backup-related to AutoBackup

---
 app/main.go                   |  7 +++++-
 app/migrator/migrator.go      | 42 +++++++++++++++++++++--------------
 app/migrator/migrator_test.go |  7 +++---
 3 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/app/main.go b/app/main.go
index 6cb4ce37..4029ffff 100644
--- a/app/main.go
+++ b/app/main.go
@@ -114,7 +114,12 @@ func main() {
 	}
 
 	for _, siteID := range opts.Sites {
-		go migrator.AutoBackup(&exporter, opts.BackupLocation, siteID, opts.MaxBackupFiles)
+		go migrator.AutoBackup{
+			Exporter:       &exporter,
+			BackupLocation: opts.BackupLocation,
+			SiteID:         siteID,
+			KeepMax:        opts.MaxBackupFiles,
+		}.Do()
 	}
 
 	srv.Run()
diff --git a/app/migrator/migrator.go b/app/migrator/migrator.go
index c478cdd9..1f0edeab 100644
--- a/app/migrator/migrator.go
+++ b/app/migrator/migrator.go
@@ -61,31 +61,39 @@ func ImportComments(p ImportParams) error {
 	return importer.Import(fh, p.SiteID)
 }
 
-// AutoBackup runs daily export to local files, keeps up to keepMax backups for given siteID
-func AutoBackup(exporter Exporter, backupLocation string, siteID string, keepMax int) {
+// AutoBackup struct handles daily backups params for siteID
+type AutoBackup struct {
+	Exporter       Exporter
+	BackupLocation string
+	SiteID         string
+	KeepMax        int
+}
+
+// Do runs daily export to local files, keeps up to keepMax backups for given siteID
+func (ab AutoBackup) Do() {
 	log.Print("[INFO] activate auto-backup")
 	tick := time.NewTicker(24 * time.Hour)
 	for range tick.C {
-		_, err := makeBackup(exporter, backupLocation, siteID)
+		_, err := ab.makeBackup()
 		if err != nil {
-			log.Printf("[WARN] auto-backup for %s failed, %s", siteID, err)
+			log.Printf("[WARN] auto-backup for %s failed, %s", ab.SiteID, err)
 			continue
 		}
-		removeOldBackupFiles(backupLocation, siteID, keepMax)
+		ab.removeOldBackupFiles()
 	}
 }
 
-func makeBackup(exporter Exporter, backupLocation string, siteID string) (string, error) {
-	log.Printf("[DEBUG] make backup for %s", siteID)
-	backupFile := fmt.Sprintf("%s/backup-%s-%s.gz", backupLocation, siteID, time.Now().Format("20060102"))
+func (ab AutoBackup) makeBackup() (string, error) {
+	log.Printf("[DEBUG] make backup for %s", ab.SiteID)
+	backupFile := fmt.Sprintf("%s/backup-%s-%s.gz", ab.BackupLocation, ab.SiteID, time.Now().Format("20060102"))
 	fh, err := os.Create(backupFile)
 	if err != nil {
 		return "", errors.Wrapf(err, "can't create backup file %s", backupFile)
 	}
 	gz := gzip.NewWriter(fh)
 
-	if err = exporter.Export(gz, siteID); err != nil {
-		return "", errors.Wrapf(err, "export failed for %s", siteID)
+	if err = ab.Exporter.Export(gz, ab.SiteID); err != nil {
+		return "", errors.Wrapf(err, "export failed for %s", ab.SiteID)
 	}
 	if err = gz.Close(); err != nil {
 		return "", errors.Wrapf(err, "can't close gz for %s", backupFile)
@@ -97,23 +105,23 @@ func makeBackup(exporter Exporter, backupLocation string, siteID string) (string
 	return backupFile, nil
 }
 
-func removeOldBackupFiles(backupLocation string, siteID string, keepMax int) {
-	files, err := ioutil.ReadDir(backupLocation)
+func (ab AutoBackup) removeOldBackupFiles() {
+	files, err := ioutil.ReadDir(ab.BackupLocation)
 	if err != nil {
-		log.Printf("[WARN] can't read files in backup directory %s, %s", backupLocation, err)
+		log.Printf("[WARN] can't read files in backup directory %s, %s", ab.BackupLocation, err)
 		return
 	}
 	backFiles := []os.FileInfo{}
 	for _, file := range files {
-		if strings.HasPrefix(file.Name(), "backup-"+siteID) {
+		if strings.HasPrefix(file.Name(), "backup-"+ab.SiteID) {
 			backFiles = append(backFiles, file)
 		}
 	}
 	sort.Slice(backFiles, func(i int, j int) bool { return backFiles[i].Name() < backFiles[j].Name() })
 
-	if len(backFiles) > keepMax {
-		for i := 0; i < len(backFiles)-keepMax; i++ {
-			fpath := backupLocation + "/" + backFiles[i].Name()
+	if len(backFiles) > ab.KeepMax {
+		for i := 0; i < len(backFiles)-ab.KeepMax; i++ {
+			fpath := ab.BackupLocation + "/" + backFiles[i].Name()
 			if e := os.Remove(fpath); e != nil {
 				log.Printf("[WARN] can't delete %s, %s", fpath, err)
 				continue
diff --git a/app/migrator/migrator_test.go b/app/migrator/migrator_test.go
index 3ab19664..ad1e24ec 100644
--- a/app/migrator/migrator_test.go
+++ b/app/migrator/migrator_test.go
@@ -21,8 +21,8 @@ func TestMigrator_RemoveOldBackupFiles(t *testing.T) {
 		err := ioutil.WriteFile(fname, []byte("blah"), 0600)
 		assert.Nil(t, err)
 	}
-
-	removeOldBackupFiles(loc, "site1", 3)
+	bk := AutoBackup{BackupLocation: loc, SiteID: "site1", KeepMax: 3}
+	bk.removeOldBackupFiles()
 	ff, err := ioutil.ReadDir(loc)
 	assert.Nil(t, err)
 	assert.Equal(t, 3, len(ff), "should keep 3 files only")
@@ -36,7 +36,8 @@ func TestMigrator_MakeBackup(t *testing.T) {
 	defer os.RemoveAll(loc)
 	os.MkdirAll(loc, 0700)
 
-	fname, err := makeBackup(&mockExporter{}, loc, "site1")
+	bk := AutoBackup{BackupLocation: loc, SiteID: "site1", KeepMax: 3, Exporter: &mockExporter{}}
+	fname, err := bk.makeBackup()
 	assert.NoError(t, err)
 	expFile := fmt.Sprintf("/tmp/remark-backups.test/backup-site1-%s.gz", time.Now().Format("20060102"))
 	assert.Equal(t, expFile, fname)