From 3143ddfa6dd020c024d642bce7567417cfe92754 Mon Sep 17 00:00:00 2001
From: Nicolas Simonds <nisimond@cisco.com>
Date: Thu, 14 Mar 2024 00:37:47 -0700
Subject: [PATCH] refactor: use the File methods for Stat, Chmod

Use the already-open file descriptors for reading/writing metadata,
rather than the long-form VFS versions.

Add/use FailFS fixtures as needed.

Refactor `repository.CopySources` to be a bit simpler
---
 internal/mocks/vfs/failfs.go            | 20 ++++++++++++++++++
 internal/repository/copy.go             |  6 +++---
 internal/repository/copy_public_test.go |  6 +++---
 internal/repository/repository.go       | 27 +++++++++++--------------
 4 files changed, 38 insertions(+), 21 deletions(-)

diff --git a/internal/mocks/vfs/failfs.go b/internal/mocks/vfs/failfs.go
index 855cf6e..b806c59 100644
--- a/internal/mocks/vfs/failfs.go
+++ b/internal/mocks/vfs/failfs.go
@@ -383,6 +383,14 @@ func (f *FailFile) Chdir() error {
 }
 
 func (f *FailFile) Chmod(mode fs.FileMode) error {
+	if failFn, ok := f.vfs.failFn["file.Chmod"]; ok {
+		results := f.vfs.callFailFn(failFn, mode)
+		var err error
+		if !results[0].IsNil() {
+			err, _ = results[0].Interface().(error)
+		}
+		return err
+	}
 	return f.baseFile.Chmod(mode)
 }
 
@@ -433,6 +441,18 @@ func (f *FailFile) Seek(offset int64, whence int) (ret int64, err error) {
 }
 
 func (f *FailFile) Stat() (fs.FileInfo, error) {
+	if failFn, ok := f.vfs.failFn["file.Stat"]; ok {
+		results := f.vfs.callFailFn(failFn)
+		var fileInfo fs.FileInfo
+		var err error
+		if !results[0].IsNil() {
+			fileInfo, _ = results[0].Interface().(fs.FileInfo)
+		}
+		if !results[1].IsNil() {
+			err, _ = results[1].Interface().(error)
+		}
+		return fileInfo, err
+	}
 	return f.baseFile.Stat()
 }
 
diff --git a/internal/repository/copy.go b/internal/repository/copy.go
index c281218..42128de 100644
--- a/internal/repository/copy.go
+++ b/internal/repository/copy.go
@@ -63,7 +63,7 @@ func (r *Copy) CopyFile(
 	}
 	defer func() { _ = in.Close() }()
 
-	si, err := r.appFs.Stat(src)
+	si, err := in.Stat()
 	if err != nil {
 		return err
 	}
@@ -77,7 +77,7 @@ func (r *Copy) CopyFile(
 	}
 	defer func() { _ = out.Close() }()
 
-	err = r.appFs.Chmod(dst, 0o600)
+	err = out.Chmod(0o600)
 	if err != nil {
 		return err
 	}
@@ -93,7 +93,7 @@ func (r *Copy) CopyFile(
 	}
 
 	// All done; make the permissions match
-	err = r.appFs.Chmod(dst, si.Mode())
+	err = out.Chmod(si.Mode())
 	if err != nil {
 		return err
 	}
diff --git a/internal/repository/copy_public_test.go b/internal/repository/copy_public_test.go
index 630e7db..7ec3fad 100644
--- a/internal/repository/copy_public_test.go
+++ b/internal/repository/copy_public_test.go
@@ -102,7 +102,7 @@ func (suite *CopyPublicTestSuite) TestCopyFileErrorStat() {
 	suite.appFs = failfs.New(
 		suite.appFs,
 		map[string]interface{}{
-			"Stat": func(string) (fs.FileInfo, error) { return nil, errors.New("FailFS!") },
+			"file.Stat": func() (fs.FileInfo, error) { return nil, errors.New("FailFS!") },
 		},
 	)
 	assertFile := suite.appFs.Join(suite.dstDir, "1.txt")
@@ -143,7 +143,7 @@ func (suite *CopyPublicTestSuite) TestCopyFileErrorSettingDestfilePerms() {
 	suite.appFs = failfs.New(
 		suite.appFs,
 		map[string]interface{}{
-			"Chmod": func(string, fs.FileMode) error { return errors.New("FailFS!") },
+			"file.Chmod": func(fs.FileMode) error { return errors.New("FailFS!") },
 		},
 	)
 	assertFile := suite.appFs.Join(suite.dstDir, "1.txt")
@@ -212,7 +212,7 @@ func (suite *CopyPublicTestSuite) TestCopyFileErrorFinalizingDestfilePerms() {
 	suite.appFs = failfs.New(
 		suite.appFs,
 		map[string]interface{}{
-			"Chmod": func(name string, mode fs.FileMode) error {
+			"file.Chmod": func(mode fs.FileMode) error {
 				if mode == 0o600 {
 					return nil
 				}
diff --git a/internal/repository/repository.go b/internal/repository/repository.go
index 3115bd0..1fd8b00 100644
--- a/internal/repository/repository.go
+++ b/internal/repository/repository.go
@@ -100,15 +100,23 @@ func (r *Repository) CopySources(
 		}
 
 		for _, src := range globbedSrc {
-			// The source is a file
-			if info, err := r.appFs.Stat(src); err == nil && info.Mode().IsRegular() {
-				// ... and the dst is declared a directory
+			// The source is a directory
+			if info, err := r.appFs.Stat(src); err == nil && info.IsDir() {
+				// ... and dst dir exists
+				if info, err := r.appFs.Stat(source.DstDir); err == nil && info.IsDir() {
+					if err := r.appFs.RemoveAll(source.DstDir); err != nil {
+						return err
+					}
+				}
+				if err := r.copyManager.CopyDir(src, source.DstDir); err != nil {
+					return err
+				}
+			} else {
 				if source.DstFile != "" {
 					if err := r.copyManager.CopyFile(src, source.DstFile); err != nil {
 						return err
 					}
 				} else if source.DstDir != "" {
-					// ... and create the dst directory
 					if err := r.appFs.MkdirAll(source.DstDir, 0o755); err != nil {
 						return fmt.Errorf("unable to create dest dir: %s", err)
 					}
@@ -118,17 +126,6 @@ func (r *Repository) CopySources(
 						return err
 					}
 				}
-				// The source is a directory
-			} else if info, err := r.appFs.Stat(src); err == nil && info.Mode().IsDir() {
-				// ... and dst dir exists
-				if info, err := r.appFs.Stat(source.DstDir); err == nil && info.Mode().IsDir() {
-					if err := r.appFs.RemoveAll(source.DstDir); err != nil {
-						return err
-					}
-				}
-				if err := r.copyManager.CopyDir(src, source.DstDir); err != nil {
-					return err
-				}
 			}
 		}
 	}