clean bugfix (#95)

* clean bugfix

* minor edit

* minor edit on testcases

* minor edit

* add comment & parameter name for driver interface

* .

storage/driver/driver.go

@@ -4,18 +4,19 @@ import (
 	"io"
 )
 
+// https://github.com/distribution/distribution/blob/v2.8.2/registry/storage/driver/storagedriver.go#L41
 type Driver interface {
 	Name() string
-	GetContent(string) ([]byte, error)
-	PutContent(string, []byte) error
-	Reader(string) (io.ReadCloser, error)
-	Writer(string) (FileWriter, error)
-	Stat(string) (FileInfo, error)
-	List(string) ([]string, error)
-	Move(string, string) error
-	Delete(string) error
-	Walk(string) ([]FileInfo, error)
-	WalkDir(string) ([]string, error)
-	Mkdir(string) error
+	GetContent(path string) ([]byte, error)
+	PutContent(path string, content []byte) error
+	Reader(path string) (io.ReadCloser, error)
+	Writer(path string) (FileWriter, error)
+	Stat(path string) (FileInfo, error)
+	List(path string) ([]string, error)
+	Move(sourcePath string, destPath string) error
+	Delete(path string) error
+	Walk(path string) ([]FileInfo, error)
+	WalkDir(path string) ([]string, error)
+	Mkdir(path string) error
 	RootDirectory() string
 }

storage/driver/error.go

@@ -8,5 +8,5 @@ type PathNotFoundError struct {
 }
 
 func (err PathNotFoundError) Error() string {
-	return fmt.Sprintf("Path not found: %s", err.Path)
+	return fmt.Sprintf("Path not found: %s, err: %s", err.Path, err.Err)
 }

storage/driver/error_test.go

@@ -1,13 +1,39 @@
 package driver
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 )
 
 func TestError(t *testing.T) {
-	err := PathNotFoundError{Path: "/tmp"}
-	got := err.Error()
-	assert.Equal(t, "Path not found: /tmp", got)
+	testCases := []struct {
+		path      string
+		err       error
+		wantError string
+	}{
+		{
+			"", nil,
+			"Path not found: , err: %!s(<nil>)",
+		},
+		{
+			"", errors.New("hello"),
+			"Path not found: , err: hello",
+		},
+		{
+			"/tmp", nil,
+			"Path not found: /tmp, err: %!s(<nil>)",
+		},
+		{
+			"/tmp", errors.New("hello"),
+			"Path not found: /tmp, err: hello",
+		},
+	}
+	for _, tc := range testCases {
+		t.Run("", func(t *testing.T) {
+			err := PathNotFoundError{Path: tc.path, Err: tc.err}
+			assert.EqualError(t, err, tc.wantError)
+		})
+	}
 }

storage/driver/filesystem/driver.go

@@ -159,14 +159,15 @@ func (d *driver) Move(sourcePath, targetPath string) error {
 }
 
 func (d *driver) Delete(subpath string) error {
-	if len(strings.Split(subpath, string(os.PathSeparator))) < 2 {
-		return fmt.Errorf("deleting 0-1 depth directory is not allowed")
-	}
 	fullpath := d.fullPath(subpath)
-	_, err := os.Stat(fullpath)
+	fileInfo, err := os.Stat(fullpath)
 	if err != nil {
 		return storagedriver.PathNotFoundError{Path: subpath, Err: fmt.Errorf("stat err: %w", err)}
 	}
+	depth := len(strings.Split(subpath, string(os.PathSeparator)))
+	if depth < 2 && fileInfo.IsDir() {
+		return fmt.Errorf("deleting 0-1 depth directory is not allowed")
+	}
 	err = os.RemoveAll(fullpath)
 	if err != nil {
 		return fmt.Errorf("removeAll err: %w", err)

storage/driver/filesystem/driver_test.go

@@ -230,7 +230,7 @@ func TestStat(t *testing.T) {
 		{
 			"hello",
 			"",
-			"Path not found: hello",
+			"Path not found: hello, err: stat err: stat tmp/storage_driver_filesystem_driver_test/hello: no such file or directory",
 		},
 		{
 			"node",
@@ -250,7 +250,7 @@ func TestStat(t *testing.T) {
 		{
 			"pod/namespace01/hello.log",
 			"",
-			"Path not found: pod/namespace01/hello.log",
+			"Path not found: pod/namespace01/hello.log, err: stat err: stat tmp/storage_driver_filesystem_driver_test/pod/namespace01/hello.log: no such file or directory",
 		},
 		{
 			"pod/namespace01/2009-11-10_21.log",
@@ -291,7 +291,7 @@ func TestList(t *testing.T) {
 		{
 			"hello",
 			nil,
-			"Path not found: hello",
+			"Path not found: hello, err: open err: open tmp/storage_driver_filesystem_driver_test/hello: no such file or directory",
 		},
 		{
 			"pod",
@@ -330,31 +330,31 @@ func TestMove(t *testing.T) {
 		},
 		{
 			"hello", "",
-			"Path not found: hello",
+			"Path not found: hello, err: stat err: stat tmp/storage_driver_filesystem_driver_test/hello: no such file or directory",
 		},
 		{
 			"", "hello",
 			"rename tmp/storage_driver_filesystem_driver_test tmp/storage_driver_filesystem_driver_test/hello: invalid argument",
 		},
 		{
 			"hello", "hello",
-			"Path not found: hello",
+			"Path not found: hello, err: stat err: stat tmp/storage_driver_filesystem_driver_test/hello: no such file or directory",
 		},
 		{
 			"pod/namespace01/hello.log", "pod/namespace01/hello.log",
-			"Path not found: pod/namespace01/hello.log",
+			"Path not found: pod/namespace01/hello.log, err: stat err: stat tmp/storage_driver_filesystem_driver_test/pod/namespace01/hello.log: no such file or directory",
 		},
 		{
 			"pod/namespace01/2009-11-10_21.log", "pod/namespace01/2009-11-10_00.log", // move
 			"",
 		},
 		{
 			"pod/namespace01/2009-11-10_21.log", "pod/namespace01/2009-11-10_00.log", // duplicate
-			"Path not found: pod/namespace01/2009-11-10_21.log",
+			"Path not found: pod/namespace01/2009-11-10_21.log, err: stat err: stat tmp/storage_driver_filesystem_driver_test/pod/namespace01/2009-11-10_21.log: no such file or directory",
 		},
 	}
-	for i, tc := range testCases {
-		t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) {
+	for _, tc := range testCases {
+		t.Run("", func(t *testing.T) {
 			err := driver1.Move(tc.a, tc.b)
 			if tc.wantError == "" {
 				assert.NoError(t, err)
@@ -389,7 +389,7 @@ func TestDelete(t *testing.T) {
 		},
 		{
 			"pod/namespace01/2009-11-10_21.log", // duplicate
-			"Path not found: pod/namespace01/2009-11-10_21.log",
+			"Path not found: pod/namespace01/2009-11-10_21.log, err: stat err: stat tmp/storage_driver_filesystem_driver_test/pod/namespace01/2009-11-10_21.log: no such file or directory",
 		},
 	}
 	for i, tc := range testCases {

storage/fileservice/clean.go

@@ -1,7 +1,7 @@
 package fileservice
 
 import (
-	"fmt"
+	"strings"
 
 	"github.com/kuoss/common/logger"
 )
@@ -12,20 +12,22 @@ func (s *FileService) Clean() {
 }
 
 func (s *FileService) removeFilesWithPrefix(prefix string) {
-	files, err := s.driver.Walk(fmt.Sprintf("%s/%s.*", s.config.LogDataPath(), prefix))
+	files, err := s.driver.List("")
 	if err != nil {
-		logger.Warnf("glob err: %s, prefix: %s", err.Error(), prefix)
+		logger.Warnf("list err: %s, prefix: %s", err.Error(), prefix)
 		return
 	}
 	if len(files) < 1 {
 		return
 	}
-	logger.Warnf("cleansing files prefix: %s", prefix)
+	logger.Warnf("cleanning files prefix: %s", prefix)
 	for _, file := range files {
-		logger.Infof("remove file: %s", file)
-		err := s.driver.Delete(file.Fullpath())
-		if err != nil {
-			logger.Warnf("remove err: %s, file: %s", err.Error(), file)
+		if strings.HasPrefix(file, prefix) {
+			logger.Infof("remove file: %s", file)
+			err := s.driver.Delete(file)
+			if err != nil {
+				logger.Warnf("remove err: %s, file: %s", err.Error(), file)
+			}
 		}
 	}
 }

storage/fileservice/clean_test.go

@@ -1 +1,45 @@
 package fileservice
+
+import (
+	"os"
+	"testing"
+
+	"github.com/kuoss/lethe/config"
+	"github.com/kuoss/lethe/util/testutil"
+	"github.com/stretchr/testify/require"
+)
+
+var (
+	fileService_clean *FileService
+	logDataPath_clean string = "tmp/storage_fileservice_clean_test"
+)
+
+func init() {
+	testutil.ChdirRoot()
+	testutil.ResetLogData()
+
+	cfg, err := config.New("test")
+	if err != nil {
+		panic(err)
+	}
+	cfg.SetLogDataPath(logDataPath_clean)
+	fileService_clean, err = New(cfg)
+	if err != nil {
+		panic(err)
+	}
+}
+
+func TestClean(t *testing.T) {
+	// setup
+	var err error
+	err = os.WriteFile("tmp/storage_fileservice_clean_test/kube.1", []byte("hello"), 0644)
+	require.NoError(t, err)
+	err = os.WriteFile("tmp/storage_fileservice_clean_test/host.1", []byte("hello"), 0644)
+	require.NoError(t, err)
+
+	require.FileExists(t, "tmp/storage_fileservice_clean_test/kube.1")
+	require.FileExists(t, "tmp/storage_fileservice_clean_test/host.1")
+	fileService_clean.Clean()
+	require.NoFileExists(t, "tmp/storage_fileservice_clean_test/kube.1")
+	require.NoFileExists(t, "tmp/storage_fileservice_clean_test/host.1")
+}

storage/fileservice/list_test.go

@@ -36,16 +36,16 @@ func TestDirSize(t *testing.T) {
 		wantError string
 	}{
 		{"", 0, ""},
-		{"hello", 0, "Path not found: hello"},
+		{"hello", 0, "Path not found: hello, err: open err: open tmp/init/hello: no such file or directory"},
 		{"node", 0, ""},
 		{"pod", 0, ""},
 		{"node/node01", 1234, ""},
 		{"node/node02", 1116, ""},
 		{"pod/namespace01", 2620, ""},
 		{"pod/namespace02", 1137, ""},
 	}
-	for i, tc := range testCases {
-		t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) {
+	for _, tc := range testCases {
+		t.Run("", func(t *testing.T) {
 			got, err := fileService.dirSize(tc.path)
 			if tc.wantError == "" {
 				assert.NoError(t, err)
@@ -76,7 +76,7 @@ func TestList(t *testing.T) {
 		{
 			"hello",
 			nil,
-			"list err: Path not found: hello",
+			"list err: Path not found: hello, err: open err: open tmp/init/hello: no such file or directory",
 		},
 		{
 			"node",
@@ -99,8 +99,8 @@ func TestList(t *testing.T) {
 			"list err: readdirnames err: readdirent tmp/init/pod/namespace01/2029-11-10_23.log: not a directory",
 		},
 	}
-	for i, tc := range testCases {
-		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+	for _, tc := range testCases {
+		t.Run("", func(t *testing.T) {
 			got, err := fileService.List(tc.subpath)
 			if tc.wantError == "" {
 				assert.NoError(t, err)