Created cleanup functionality for syncthing (#6884)

* Add clean up for Simple File Versioning pt.1

created test

* Add clean up for Simple File Versioning pt.2

Passing the test

* stuck on how javascript communicates with backend

* Add trash clean up for Simple File Versioning

Add trash clean up functionality of to allow the user to delete backups
after specified amount of days.

* Fixed html and js style

* Refactored cleanup test cases

Refactored cleanup test cases to one file and deleted duplicated code.

* Added copyright to test file

* Refactor folder cleanout to utility function

* change utility function to package private

* refactored utility function; fixed build errors

* Updated copyright year.

* refactor test and logging

* refactor html and js

* revert style change in html

* reverted changes in html and some js

* checkout origin head version edit...html

* checkout upstream master and correct file

gui/default/syncthing/core/syncthingController.js

@@ -663,7 +663,7 @@ angular.module('syncthing.core')
 
 
         function setDefaultTheme() {
-            if (!document.getElementById("fallback-theme-css")){
+            if (!document.getElementById("fallback-theme-css")) {
 
                 // check if no support for prefers-color-scheme
                 var colorSchemeNotSupported = typeof window.matchMedia === "undefined" || window.matchMedia('(prefers-color-scheme: dark)').media === 'not all';
@@ -1733,7 +1733,7 @@ angular.module('syncthing.core')
             });
             $scope.currentFolder.unrelatedDevices = $scope.devices.filter(function (n) {
                 return n.deviceID !== $scope.myID
-                    && ! $scope.currentFolder.selectedDevices[n.deviceID]
+                    && !$scope.currentFolder.selectedDevices[n.deviceID]
             });
             if ($scope.currentFolder.versioning && $scope.currentFolder.versioning.type === "trashcan") {
                 $scope.currentFolder.trashcanFileVersioning = true;
@@ -1744,6 +1744,8 @@ angular.module('syncthing.core')
                 $scope.currentFolder.simpleFileVersioning = true;
                 $scope.currentFolder.fileVersioningSelector = "simple";
                 $scope.currentFolder.simpleKeep = +$scope.currentFolder.versioning.params.keep;
+                $scope.currentFolder.versioningCleanupIntervalS = +$scope.currentFolder.versioning.cleanupIntervalS;
+                $scope.currentFolder.trashcanClean = +$scope.currentFolder.versioning.params.cleanoutDays;
             } else if ($scope.currentFolder.versioning && $scope.currentFolder.versioning.type === "staggered") {
                 $scope.currentFolder.staggeredFileVersioning = true;
                 $scope.currentFolder.fileVersioningSelector = "staggered";
@@ -1878,7 +1880,8 @@ angular.module('syncthing.core')
                 folderCfg.versioning = {
                     'type': 'simple',
                     'params': {
-                        'keep': '' + folderCfg.simpleKeep
+                        'keep': '' + folderCfg.simpleKeep,
+                        'cleanoutDays': '' + folderCfg.trashcanClean
                     },
                     'cleanupIntervalS': folderCfg.versioningCleanupIntervalS
                 };

gui/default/syncthing/folder/editFolderModalView.html

@@ -96,7 +96,7 @@
               <option value="external" translate>External File Versioning</option>
             </select>
           </div>
-          <div class="form-group" ng-if="currentFolder.fileVersioningSelector=='trashcan'" ng-class="{'has-error': folderEditor.trashcanClean.$invalid && folderEditor.trashcanClean.$dirty}">
+          <div class="form-group" ng-if="currentFolder.fileVersioningSelector=='trashcan' || currentFolder.fileVersioningSelector=='simple'" ng-class="{'has-error': folderEditor.trashcanClean.$invalid && folderEditor.trashcanClean.$dirty}">
             <p translate class="help-block">Files are moved to .stversions directory when replaced or deleted by Syncthing.</p>
             <label translate for="trashcanClean">Clean out after</label>
             <div class="input-group">
@@ -144,7 +144,7 @@
               <span translate ng-if="folderEditor.externalCommand.$error.required && folderEditor.externalCommand.$dirty">The path cannot be blank.</span>
             </p>
           </div>
-          <div class="form-group" ng-if="currentFolder.fileVersioningSelector == 'staggered' || currentFolder.fileVersioningSelector == 'trashcan'" ng-class="{'has-error': folderEditor.versioningCleanupIntervalS.$invalid && folderEditor.versioningCleanupIntervalS.$dirty}">
+          <div class="form-group" ng-if="currentFolder.fileVersioningSelector == 'none'" ng-class="{'has-error': folderEditor.versioningCleanupIntervalS.$invalid && folderEditor.versioningCleanupIntervalS.$dirty}">
             <label translate for="versioningCleanupIntervalS">Cleanup Interval</label>
             <div class="input-group">
               <input name="versioningCleanupIntervalS" id="versioningCleanupIntervalS" class="form-control text-right" type="number" ng-model="currentFolder.versioningCleanupIntervalS" required="" min="0" max="31536000" aria-required="true" />

lib/versioner/simple.go

@@ -22,6 +22,7 @@ func init() {
 
 type simple struct {
 	keep            int
+	cleanoutDays    int
 	folderFs        fs.Filesystem
 	versionsFs      fs.Filesystem
 	copyRangeMethod fs.CopyRangeMethod
@@ -29,12 +30,16 @@ type simple struct {
 
 func newSimple(cfg config.FolderConfiguration) Versioner {
 	var keep, err = strconv.Atoi(cfg.Versioning.Params["keep"])
+	cleanoutDays, _ := strconv.Atoi(cfg.Versioning.Params["cleanoutDays"])
+	// On error we default to 0, "do not clean out the trash can"
+
 	if err != nil {
 		keep = 5 // A reasonable default
 	}
 
 	s := simple{
 		keep:            keep,
+		cleanoutDays:    cleanoutDays,
 		folderFs:        cfg.Filesystem(),
 		versionsFs:      versionerFsFromFolderCfg(cfg),
 		copyRangeMethod: cfg.CopyRangeMethod,
@@ -75,6 +80,6 @@ func (v simple) Restore(filepath string, versionTime time.Time) error {
 	return restoreFile(v.copyRangeMethod, v.versionsFs, v.folderFs, filepath, versionTime, TagFilename)
 }
 
-func (v simple) Clean(_ context.Context) error {
+func (v simple) Clean(ctx context.Context) error {
-	return nil
+	return cleanByDay(ctx, v.versionsFs, v.cleanoutDays)
 }

lib/versioner/simple_test.go

@@ -7,13 +7,14 @@
 package versioner
 
 import (
-	"github.com/syncthing/syncthing/lib/config"
 	"io/ioutil"
 	"math"
 	"path/filepath"
 	"testing"
 	"time"
 
+	"github.com/syncthing/syncthing/lib/config"
+
 	"github.com/syncthing/syncthing/lib/fs"
 )
 

lib/versioner/trashcan.go

@@ -56,51 +56,7 @@ func (t *trashcan) String() string {
 }
 
 func (t *trashcan) Clean(ctx context.Context) error {
-	if t.cleanoutDays <= 0 {
+	return cleanByDay(ctx, t.versionsFs, t.cleanoutDays)
-		// no cleanout requested
-		return nil
-	}
-
-	if _, err := t.versionsFs.Lstat("."); fs.IsNotExist(err) {
-		return nil
-	}
-
-	cutoff := time.Now().Add(time.Duration(-24*t.cleanoutDays) * time.Hour)
-	dirTracker := make(emptyDirTracker)
-
-	walkFn := func(path string, info fs.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
-		select {
-		case <-ctx.Done():
-			return ctx.Err()
-		default:
-		}
-
-		if info.IsDir() && !info.IsSymlink() {
-			dirTracker.addDir(path)
-			return nil
-		}
-
-		if info.ModTime().Before(cutoff) {
-			// The file is too old; remove it.
-			err = t.versionsFs.Remove(path)
-		} else {
-			// Keep this file, and remember it so we don't unnecessarily try
-			// to remove this directory.
-			dirTracker.addFile(path)
-		}
-		return err
-	}
-
-	if err := t.versionsFs.Walk(".", walkFn); err != nil {
-		return err
-	}
-
-	dirTracker.deleteEmptyDirs(t.versionsFs)
-
-	return nil
 }
 
 func (t *trashcan) GetVersions() (map[string][]FileVersion, error) {

lib/versioner/trashcan_test.go

@@ -7,10 +7,7 @@
 package versioner
 
 import (
-	"context"
 	"io/ioutil"
-	"os"
-	"path/filepath"
 	"testing"
 	"time"
 
@@ -18,76 +15,6 @@ import (
 	"github.com/syncthing/syncthing/lib/fs"
 )
 
-func TestTrashcanCleanout(t *testing.T) {
-	// Verify that files older than the cutoff are removed, that files newer
-	// than the cutoff are *not* removed, and that empty directories are
-	// removed (best effort).
-
-	var testcases = []struct {
-		file         string
-		shouldRemove bool
-	}{
-		{"testdata/.stversions/file1", false},
-		{"testdata/.stversions/file2", true},
-		{"testdata/.stversions/keep1/file1", false},
-		{"testdata/.stversions/keep1/file2", false},
-		{"testdata/.stversions/keep2/file1", false},
-		{"testdata/.stversions/keep2/file2", true},
-		{"testdata/.stversions/keep3/keepsubdir/file1", false},
-		{"testdata/.stversions/remove/file1", true},
-		{"testdata/.stversions/remove/file2", true},
-		{"testdata/.stversions/remove/removesubdir/file1", true},
-	}
-
-	os.RemoveAll("testdata")
-	defer os.RemoveAll("testdata")
-
-	oldTime := time.Now().Add(-8 * 24 * time.Hour)
-	for _, tc := range testcases {
-		os.MkdirAll(filepath.Dir(tc.file), 0777)
-		if err := ioutil.WriteFile(tc.file, []byte("data"), 0644); err != nil {
-			t.Fatal(err)
-		}
-		if tc.shouldRemove {
-			if err := os.Chtimes(tc.file, oldTime, oldTime); err != nil {
-				t.Fatal(err)
-			}
-		}
-	}
-
-	cfg := config.FolderConfiguration{
-		FilesystemType: fs.FilesystemTypeBasic,
-		Path:           "testdata",
-		Versioning: config.VersioningConfiguration{
-			Params: map[string]string{
-				"cleanoutDays": "7",
-			},
-		},
-	}
-
-	versioner := newTrashcan(cfg).(*trashcan)
-	if err := versioner.Clean(context.Background()); err != nil {
-		t.Fatal(err)
-	}
-
-	for _, tc := range testcases {
-		_, err := os.Lstat(tc.file)
-		if tc.shouldRemove && !os.IsNotExist(err) {
-			t.Error(tc.file, "should have been removed")
-		} else if !tc.shouldRemove && err != nil {
-			t.Error(tc.file, "should not have been removed")
-		}
-	}
-
-	if _, err := os.Lstat("testdata/.stversions/keep3"); os.IsNotExist(err) {
-		t.Error("directory with non empty subdirs should not be removed")
-	}
-
-	if _, err := os.Lstat("testdata/.stversions/remove"); !os.IsNotExist(err) {
-		t.Error("empty directory should have been removed")
-	}
-}
-
 func TestTrashcanArchiveRestoreSwitcharoo(t *testing.T) {
 	// This tests that trashcan versioner restoration correctly archives existing file, because trashcan versioner
 	// files are untagged, archiving existing file to replace with a restored version technically should collide in

lib/versioner/util.go

@@ -7,6 +7,7 @@
 package versioner
 
 import (
+	"context"
 	"path/filepath"
 	"regexp"
 	"sort"
@@ -293,3 +294,51 @@ func findAllVersions(fs fs.Filesystem, filePath string) []string {
 
 	return versions
 }
+
+func cleanByDay(ctx context.Context, versionsFs fs.Filesystem, cleanoutDays int) error {
+	if cleanoutDays <= 0 {
+		return nil
+	}
+
+	if _, err := versionsFs.Lstat("."); fs.IsNotExist(err) {
+		return nil
+	}
+
+	cutoff := time.Now().Add(time.Duration(-24*cleanoutDays) * time.Hour)
+	dirTracker := make(emptyDirTracker)
+
+	walkFn := func(path string, info fs.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		select {
+		case <-ctx.Done():
+			return ctx.Err()
+		default:
+		}
+
+		if info.IsDir() && !info.IsSymlink() {
+			dirTracker.addDir(path)
+			return nil
+		}
+
+		if info.ModTime().Before(cutoff) {
+			// The file is too old; remove it.
+			err = versionsFs.Remove(path)
+		} else {
+			// Keep this file, and remember it so we don't unnecessarily try
+			// to remove this directory.
+			dirTracker.addFile(path)
+		}
+		return err
+	}
+
+	if err := versionsFs.Walk(".", walkFn); err != nil {
+		return err
+	}
+
+	dirTracker.deleteEmptyDirs(versionsFs)
+
+	return nil
+}

lib/versioner/versioner_test.go

@@ -0,0 +1,91 @@
+// Copyright (C) 2020 The Syncthing Authors.
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at https://mozilla.org/MPL/2.0/.
+
+package versioner
+
+import (
+	"context"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/syncthing/syncthing/lib/config"
+	"github.com/syncthing/syncthing/lib/fs"
+)
+
+func TestVersionerCleanOut(t *testing.T) {
+	cfg := config.FolderConfiguration{
+		FilesystemType: fs.FilesystemTypeBasic,
+		Path:           "testdata",
+		Versioning: config.VersioningConfiguration{
+			Params: map[string]string{
+				"cleanoutDays": "7",
+			},
+		},
+	}
+
+	testCasesVersioner := map[string]Versioner{
+		"simple":   newSimple(cfg),
+		"trashcan": newTrashcan(cfg),
+	}
+
+	var testcases = map[string]bool{
+		"testdata/.stversions/file1":                     false,
+		"testdata/.stversions/file2":                     true,
+		"testdata/.stversions/keep1/file1":               false,
+		"testdata/.stversions/keep1/file2":               false,
+		"testdata/.stversions/keep2/file1":               false,
+		"testdata/.stversions/keep2/file2":               true,
+		"testdata/.stversions/keep3/keepsubdir/file1":    false,
+		"testdata/.stversions/remove/file1":              true,
+		"testdata/.stversions/remove/file2":              true,
+		"testdata/.stversions/remove/removesubdir/file1": true,
+	}
+
+	for versionerType, versioner := range testCasesVersioner {
+		t.Run(fmt.Sprintf("%v versioner trashcan clean up", versionerType), func(t *testing.T) {
+			os.RemoveAll("testdata")
+			defer os.RemoveAll("testdata")
+
+			oldTime := time.Now().Add(-8 * 24 * time.Hour)
+			for file, shouldRemove := range testcases {
+				os.MkdirAll(filepath.Dir(file), 0777)
+				if err := ioutil.WriteFile(file, []byte("data"), 0644); err != nil {
+					t.Fatal(err)
+				}
+				if shouldRemove {
+					if err := os.Chtimes(file, oldTime, oldTime); err != nil {
+						t.Fatal(err)
+					}
+				}
+			}
+
+			if err := versioner.Clean(context.Background()); err != nil {
+				t.Fatal(err)
+			}
+
+			for file, shouldRemove := range testcases {
+				_, err := os.Lstat(file)
+				if shouldRemove && !os.IsNotExist(err) {
+					t.Error(file, "should have been removed")
+				} else if !shouldRemove && err != nil {
+					t.Error(file, "should not have been removed")
+				}
+			}
+
+			if _, err := os.Lstat("testdata/.stversions/keep3"); os.IsNotExist(err) {
+				t.Error("directory with non empty subdirs should not be removed")
+			}
+
+			if _, err := os.Lstat("testdata/.stversions/remove"); !os.IsNotExist(err) {
+				t.Error("empty directory should have been removed")
+			}
+		})
+	}
+}