👔 up: update rotate writer async clean logic, fix test bugs

.github/workflows/go.yml

@@ -59,7 +59,7 @@ jobs:
 
     - name: Run unit tests
       # run: go test -v -cover ./...
-      run: go test -v -coverprofile="profile.cov" ./...
+      run: go test -coverprofile="profile.cov" ./...
 
     - name: Send coverage
       uses: shogo82148/actions-goveralls@v1

handler/handler_test.go

@@ -149,6 +149,7 @@ func TestLockWrapper_Lock(t *testing.T) {
 func TestNewSysLogHandler(t *testing.T) {
 	if sysutil.IsWin() {
 		t.Skip("skip test on windows")
+		return
 	}
 
 	h, err := handler.NewSysLogHandler(syslog.LOG_INFO, "slog")

rotatefile/cleanup.go

@@ -91,7 +91,8 @@ func NewCConfig() *CConfig {
 	}
 }
 
-// FilesClear multi files by time. TODO
+// FilesClear multi files by time.
+//
 // use for rotate and clear other program produce log files
 type FilesClear struct {
 	// mu sync.Mutex
@@ -226,7 +227,7 @@ func (r *FilesClear) cleanByPattern(filePattern string) (err error) {
 			return err
 		}
 
-		// not handle subdir
+		// not handle subdir TODO: support subdir
 		if stat.IsDir() {
 			return nil
 		}

rotatefile/cleanup_test.go

@@ -15,6 +15,13 @@ import (
 )
 
 func TestFilesClear_Clean(t *testing.T) {
+	// make files for clean
+	makeNum := 5
+	makeWaitCleanFiles("file_clean.log", makeNum)
+	_, err := fsutil.PutContents("testdata/subdir/some.txt", "test data")
+	assert.NoErr(t, err)
+
+	// create clear
 	fc := rotatefile.NewFilesClear()
 	fc.WithConfig(rotatefile.NewCConfig())
 	fc.WithConfigFn(func(c *rotatefile.CConfig) {
@@ -28,12 +35,6 @@ func TestFilesClear_Clean(t *testing.T) {
 	assert.Eq(t, uint(1), cfg.BackupNum)
 	dump.P(cfg)
 
-	// make files for clean
-	makeNum := 5
-	makeWaitCleanFiles("file_clean.log", makeNum)
-	_, err := fsutil.PutContents("testdata/subdir/some.txt", "test data")
-	assert.NoErr(t, err)
-
 	// do clean
 	assert.NoErr(t, fc.Clean())
 

rotatefile/config.go

@@ -188,7 +188,7 @@ type Config struct {
 	// default see DefaultFilenameFn
 	RenameFunc func(filePath string, rotateNum uint) string
 
-	// TimeClock for rotate
+	// TimeClock for rotate file by time.
 	TimeClock Clocker
 }
 

rotatefile/util.go

@@ -41,56 +41,6 @@ func compressFile(srcPath, dstPath string) error {
 	return zw.Close()
 }
 
-type filterFunc func(fPath string, fi os.FileInfo) bool
-type handleFunc func(fPath string, fi os.FileInfo) error
-
-// from the go pkg: path/filepath.glob()
-// TODO replace use fsutil.FindInDir()
-func findFilesInDir(dir string, handleFn handleFunc, filters ...filterFunc) (err error) {
-	fi, err := os.Stat(dir)
-	if err != nil {
-		return // ignore I/O error
-	}
-	if !fi.IsDir() {
-		return // ignore I/O error
-	}
-
-	d, err := os.Open(dir)
-	if err != nil {
-		return // ignore I/O error
-	}
-	defer d.Close()
-
-	// names, _ := d.Readdirnames(-1)
-	// sort.Strings(names)
-
-	stats, _ := d.Readdir(-1)
-	for _, fi := range stats {
-		baseName := fi.Name()
-		filePath := dir + baseName
-
-		// call filters
-		if len(filters) > 0 {
-			var filtered = false
-			for _, filter := range filters {
-				if !filter(filePath, fi) {
-					filtered = true
-					break
-				}
-			}
-
-			if filtered {
-				continue
-			}
-		}
-
-		if err := handleFn(filePath, fi); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
 // TODO replace to fsutil.FileInfo
 type fileInfo struct {
 	fs.FileInfo

rotatefile/writer.go

@@ -2,9 +2,9 @@ package rotatefile
 
 import (
 	"fmt"
+	"io/fs"
 	"os"
 	"path"
-	"path/filepath"
 	"sort"
 	"strings"
 	"sync"
@@ -24,11 +24,14 @@ type Writer struct {
 	// current opened logfile
 	file *os.File
 	path string
-	// file dir path for the Config.Filepath
+	// logfile dir path for the Config.Filepath
 	fileDir string
-	// file max backup time. equals Config.BackupTime * time.Hour
+
+	// logfile max backup time. equals Config.BackupTime * time.Hour
 	backupDur time.Duration
 	// oldFiles []string
+	cleanCh chan struct{}
+	stopCh  chan struct{}
 
 	// context use for rotating file by size
 	written   uint64 // written size
@@ -101,10 +104,15 @@ func (d *Writer) Sync() error {
 // Close the writer.
 // will sync data to disk, then close the file handle
 func (d *Writer) Close() error {
-	err := d.file.Sync()
-	if err != nil {
+	if err := d.file.Sync(); err != nil {
 		return err
 	}
+
+	// stop the async clean backups
+	if d.stopCh != nil {
+		close(d.stopCh)
+		d.stopCh = nil
+	}
 	return d.file.Close()
 }
 
@@ -140,12 +148,10 @@ func (d *Writer) Write(p []byte) (n int, err error) {
 	return
 }
 
-// Rotate the file by config.
-func (d *Writer) Rotate() (err error) {
-	return d.doRotate()
-}
+// Rotate the file by config and async clean backups
+func (d *Writer) Rotate() error { return d.doRotate() }
 
-// Rotate the file by config.
+// do rotate the logfile by config and async clean backups
 func (d *Writer) doRotate() (err error) {
 	// do rotate file by size
 	if d.cfg.MaxSize > 0 && d.written >= d.cfg.MaxSize {
@@ -160,12 +166,12 @@ func (d *Writer) doRotate() (err error) {
 		err = d.rotatingByTime()
 	}
 
-	// clean backup files
-	d.asyncCleanBackups()
+	// async clean backup files
+	d.asyncClean()
 	return
 }
 
-// TIP: only called on d.checkInterval > 0
+// TIP: should only call on d.checkInterval > 0
 func (d *Writer) rotatingByTime() error {
 	now := d.cfg.TimeClock.Now()
 	if d.nextRotatingAt > now.Unix() {
@@ -232,14 +238,6 @@ func (d *Writer) rotatingFile(bakFile string, rename bool) error {
 	return nil
 }
 
-// ReopenFile the log file
-func (d *Writer) ReopenFile() error {
-	if d.file != nil {
-		d.file.Close()
-	}
-	return d.openFile(d.path)
-}
-
 // open the log file. and set the d.file, d.path
 func (d *Writer) openFile(logfile string) error {
 	file, err := fsutil.OpenFile(logfile, DefaultFileFlags, d.cfg.FilePerm)
@@ -258,15 +256,37 @@ func (d *Writer) openFile(logfile string) error {
 // ---------------------------------------------------------------------------
 //
 
-// async clean old files by config
-func (d *Writer) asyncCleanBackups() {
+// async clean old files by config. should be in lock.
+func (d *Writer) asyncClean() {
 	if d.cfg.BackupNum == 0 && d.cfg.BackupTime == 0 {
 		return
 	}
 
-	// TODO pref: only start once
+	// if already running, send a signal
+	if d.cleanCh != nil {
+		select {
+		case d.cleanCh <- struct{}{}:
+		// case <-d.stopCh:
+		// 	return // stop clean
+		default: // skip on blocking
+		}
+		return
+	}
+
+	// init clean channel
+	d.cleanCh = make(chan struct{}, 1)
+	d.stopCh = make(chan struct{})
+
+	// start a goroutine to clean backups
 	go func() {
-		printErrln("rotatefile: clean backup files error:", d.Clean())
+		// consume the signal until stop
+		select {
+		case <-d.cleanCh:
+			printErrln("rotatefile: clean backup files error:", d.Clean())
+		case <-d.stopCh:
+			d.cleanCh = nil
+			return // stop clean
+		}
 	}()
 }
 
@@ -276,25 +296,28 @@ func (d *Writer) Clean() (err error) {
 		return errorx.Err("clean: backupNum and backupTime are both 0")
 	}
 
-	// oldFiles: old xx.log.xx files, no gz file
+	// oldFiles: xx.log.yy files, no gz file
 	var oldFiles, gzFiles []fileInfo
 	fileDir, fileName := path.Split(d.cfg.Filepath)
 
 	// find and clean old files
-	err = findFilesInDir(fileDir, func(fPath string, fi os.FileInfo) error {
-		if strings.HasSuffix(fi.Name(), compressSuffix) {
+	err = fsutil.FindInDir(fileDir, func(fPath string, ent fs.DirEntry) error {
+		fi, err := ent.Info()
+		if err != nil {
+			return err
+		}
+
+		if strings.HasSuffix(ent.Name(), compressSuffix) {
 			gzFiles = append(gzFiles, newFileInfo(fPath, fi))
 		} else {
 			oldFiles = append(oldFiles, newFileInfo(fPath, fi))
 		}
-
 		return nil
 	}, d.buildFilterFns(fileName)...)
 
 	gzNum := len(gzFiles)
 	oldNum := len(oldFiles)
-	maxNum := int(d.cfg.BackupNum)
-	remNum := gzNum + oldNum - maxNum
+	remNum := gzNum + oldNum - int(d.cfg.BackupNum)
 
 	if remNum > 0 {
 		// remove old gz files
@@ -347,25 +370,30 @@ func (d *Writer) Clean() (err error) {
 	return
 }
 
-func (d *Writer) buildFilterFns(fileName string) []filterFunc {
-	filterFns := []filterFunc{
+func (d *Writer) buildFilterFns(fileName string) []fsutil.FilterFunc {
+	filterFns := []fsutil.FilterFunc{
+		fsutil.OnlyFindFile,
 		// filter by name. should match like error.log.*
 		// eg: error.log.xx, error.log.xx.gz
-		func(fPath string, fi os.FileInfo) bool {
-			ok, err := filepath.Match(fileName+".*", fi.Name())
+		func(fPath string, ent fs.DirEntry) bool {
+			ok, err := path.Match(fileName+".*", ent.Name())
 			if err != nil {
 				printErrln("rotatefile: match old file error:", err)
 				return false // skip, not handle
 			}
-
 			return ok
 		},
 	}
 
 	// filter by mod-time, clear expired files
 	if d.cfg.BackupTime > 0 {
 		cutTime := d.cfg.TimeClock.Now().Add(-d.backupDur)
-		filterFns = append(filterFns, func(fPath string, fi os.FileInfo) bool {
+		filterFns = append(filterFns, func(fPath string, ent fs.DirEntry) bool {
+			fi, err := ent.Info()
+			if err != nil {
+				return false // skip, not handle
+			}
+
 			// collect un-expired
 			if fi.ModTime().After(cutTime) {
 				return true

rotatefile/writer_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/gookit/goutil"
+	"github.com/gookit/goutil/dump"
 	"github.com/gookit/goutil/fsutil"
 	"github.com/gookit/goutil/mathutil"
 	"github.com/gookit/goutil/testutil/assert"
@@ -86,9 +87,7 @@ func TestWriter_Clean(t *testing.T) {
 	logfile := "testdata/writer_clean.log"
 
 	c := rotatefile.NewConfig(logfile)
-	c.MaxSize = 128
-	c.BackupNum = 0
-	c.BackupTime = 0
+	c.MaxSize = 128 // will rotate by size
 
 	wr, err := c.Create()
 	assert.NoErr(t, err)
@@ -99,15 +98,24 @@ func TestWriter_Clean(t *testing.T) {
 	}
 
 	assert.True(t, fsutil.IsFile(logfile))
-
 	_, err = wr.WriteString("hi\n")
 	assert.NoErr(t, err)
 
-	assert.Err(t, wr.Clean())
-
-	// test clean and backup
-	c.BackupNum = 2
-	c.Compress = true
-	err = wr.Clean()
-	assert.NoErr(t, err)
+	files := fsutil.Glob("testdata/writer_clean.log.*")
+	dump.P(files)
+
+	// test clean error
+	t.Run("clean error", func(t *testing.T) {
+		c.BackupNum = 0
+		c.BackupTime = 0
+		assert.Err(t, wr.Clean())
+	})
+
+	// test clean and compress backup
+	t.Run("clean and compress", func(t *testing.T) {
+		c.BackupNum = 2
+		c.Compress = true
+		err = wr.Clean()
+		assert.NoErr(t, err)
+	})
 }