chore: improve logx gzip (#3332)

core/logx/fs.go

@@ -0,0 +1,40 @@
+package logx
+
+import (
+	"io"
+	"os"
+)
+
+var fileSys realFileSystem
+
+type (
+	fileSystem interface {
+		Close(closer io.Closer) error
+		Copy(writer io.Writer, reader io.Reader) (int64, error)
+		Create(name string) (*os.File, error)
+		Open(name string) (*os.File, error)
+		Remove(name string) error
+	}
+
+	realFileSystem struct{}
+)
+
+func (fs realFileSystem) Close(closer io.Closer) error {
+	return closer.Close()
+}
+
+func (fs realFileSystem) Copy(writer io.Writer, reader io.Reader) (int64, error) {
+	return io.Copy(writer, reader)
+}
+
+func (fs realFileSystem) Create(name string) (*os.File, error) {
+	return os.Create(name)
+}
+
+func (fs realFileSystem) Open(name string) (*os.File, error) {
+	return os.Open(name)
+}
+
+func (fs realFileSystem) Remove(name string) error {
+	return os.Remove(name)
+}

core/logx/rotatelogger.go

@@ -4,7 +4,6 @@ import (
 	"compress/gzip"
 	"errors"
 	"fmt"
-	"io"
 	"log"
 	"os"
 	"path"
@@ -406,7 +405,7 @@ func (l *RotateLogger) write(v []byte) {
 func compressLogFile(file string) {
 	start := time.Now()
 	Infof("compressing log file: %s", file)
-	if err := gzipFile(file); err != nil {
+	if err := gzipFile(file, fileSys); err != nil {
 		Errorf("compress error: %s", err)
 	} else {
 		Infof("compressed log file: %s, took %s", file, time.Since(start))
@@ -421,26 +420,37 @@ func getNowDateInRFC3339Format() string {
 	return time.Now().Format(fileTimeFormat)
 }
 
-func gzipFile(file string) error {
+func gzipFile(file string, fsys fileSystem) (err error) {
-	in, err := os.Open(file)
+	in, err := fsys.Open(file)
 	if err != nil {
 		return err
 	}
+	defer func() {
+		if e := fsys.Close(in); e != nil {
+			Errorf("failed to close file: %s, error: %v", file, e)
+		}
+		if err == nil {
+			// only remove the original file when compression is successful
+			err = fsys.Remove(file)
+		}
+	}()
 
-	out, err := os.Create(fmt.Sprintf("%s%s", file, gzipExt))
+	out, err := fsys.Create(fmt.Sprintf("%s%s", file, gzipExt))
 	if err != nil {
 		return err
 	}
-	defer out.Close()
+	defer func() {
+		e := fsys.Close(out)
+		if err == nil {
+			err = e
+		}
+	}()
 
 	w := gzip.NewWriter(out)
-	if _, err = io.Copy(w, in); err != nil {
+	if _, err = fsys.Copy(w, in); err != nil {
-		return err
+		// failed to copy, no need to close w
-	} else if err = w.Close(); err != nil {
 		return err
 	}
 
-	in.Close()
+	return fsys.Close(w)
-
-	return os.Remove(file)
 }

core/logx/rotatelogger_test.go

@@ -1,9 +1,12 @@
 package logx
 
 import (
+	"errors"
+	"io"
 	"os"
 	"path"
 	"path/filepath"
+	"sync/atomic"
 	"syscall"
 	"testing"
 	"time"
@@ -429,6 +432,70 @@ func TestRotateLoggerWithSizeLimitRotateRuleWrite(t *testing.T) {
 	logger.write([]byte(`baz`))
 }
 
+func TestGzipFile(t *testing.T) {
+	err := errors.New("any error")
+
+	t.Run("gzip file open failed", func(t *testing.T) {
+		fsys := &fakeFileSystem{
+			openFn: func(name string) (*os.File, error) {
+				return nil, err
+			},
+		}
+		assert.ErrorIs(t, err, gzipFile("any", fsys))
+		assert.False(t, fsys.Removed())
+	})
+
+	t.Run("gzip file create failed", func(t *testing.T) {
+		fsys := &fakeFileSystem{
+			createFn: func(name string) (*os.File, error) {
+				return nil, err
+			},
+		}
+		assert.ErrorIs(t, err, gzipFile("any", fsys))
+		assert.False(t, fsys.Removed())
+	})
+
+	t.Run("gzip file copy failed", func(t *testing.T) {
+		fsys := &fakeFileSystem{
+			copyFn: func(writer io.Writer, reader io.Reader) (int64, error) {
+				return 0, err
+			},
+		}
+		assert.ErrorIs(t, err, gzipFile("any", fsys))
+		assert.False(t, fsys.Removed())
+	})
+
+	t.Run("gzip file last close failed", func(t *testing.T) {
+		var called int32
+		fsys := &fakeFileSystem{
+			closeFn: func(closer io.Closer) error {
+				if atomic.AddInt32(&called, 1) > 2 {
+					return err
+				}
+				return nil
+			},
+		}
+		assert.NoError(t, gzipFile("any", fsys))
+		assert.True(t, fsys.Removed())
+	})
+
+	t.Run("gzip file remove failed", func(t *testing.T) {
+		fsys := &fakeFileSystem{
+			removeFn: func(name string) error {
+				return err
+			},
+		}
+		assert.Error(t, err, gzipFile("any", fsys))
+		assert.True(t, fsys.Removed())
+	})
+
+	t.Run("gzip file everything ok", func(t *testing.T) {
+		fsys := &fakeFileSystem{}
+		assert.NoError(t, gzipFile("any", fsys))
+		assert.True(t, fsys.Removed())
+	})
+}
+
 func BenchmarkRotateLogger(b *testing.B) {
 	filename := "./test.log"
 	filename2 := "./test2.log"
@@ -480,3 +547,53 @@ func BenchmarkRotateLogger(b *testing.B) {
 		}
 	})
 }
+
+type fakeFileSystem struct {
+	removed  int32
+	closeFn  func(closer io.Closer) error
+	copyFn   func(writer io.Writer, reader io.Reader) (int64, error)
+	createFn func(name string) (*os.File, error)
+	openFn   func(name string) (*os.File, error)
+	removeFn func(name string) error
+}
+
+func (f *fakeFileSystem) Close(closer io.Closer) error {
+	if f.closeFn != nil {
+		return f.closeFn(closer)
+	}
+	return nil
+}
+
+func (f *fakeFileSystem) Copy(writer io.Writer, reader io.Reader) (int64, error) {
+	if f.copyFn != nil {
+		return f.copyFn(writer, reader)
+	}
+	return 0, nil
+}
+
+func (f *fakeFileSystem) Create(name string) (*os.File, error) {
+	if f.createFn != nil {
+		return f.createFn(name)
+	}
+	return nil, nil
+}
+
+func (f *fakeFileSystem) Open(name string) (*os.File, error) {
+	if f.openFn != nil {
+		return f.openFn(name)
+	}
+	return nil, nil
+}
+
+func (f *fakeFileSystem) Remove(name string) error {
+	atomic.AddInt32(&f.removed, 1)
+
+	if f.removeFn != nil {
+		return f.removeFn(name)
+	}
+	return nil
+}
+
+func (f *fakeFileSystem) Removed() bool {
+	return atomic.LoadInt32(&f.removed) > 0
+}