refactor: factor common code to check whether file exists

Part of ooni/probe#2401

cmd/ooniprobe/internal/autorun/autorun_darwin.go

@@ -11,7 +11,7 @@ import (
 	"text/template"
 
 	"github.com/apex/log"
-	"github.com/ooni/probe-cli/v3/cmd/ooniprobe/internal/utils"
+	"github.com/ooni/probe-cli/v3/internal/fsx"
 	"github.com/ooni/probe-cli/v3/internal/shellx"
 	"golang.org/x/sys/execabs"
 	"golang.org/x/sys/unix"
@@ -90,7 +90,7 @@ func (managerDarwin) LogStream() error {
 
 func (managerDarwin) mustNotHavePlist() error {
 	log.Infof("exec: test -f %s && already_registered()", plistPath)
-	if utils.FileExists(plistPath) {
+	if fsx.RegularFileExists(plistPath) {
 		// This is not atomic. Do we need atomicity here?
 		return errors.New("autorun: service already registered")
 	}

cmd/ooniprobe/internal/utils/paths.go

@@ -6,6 +6,7 @@ import (
 	"path/filepath"
 
 	"github.com/ooni/probe-cli/v3/cmd/ooniprobe/internal/utils/homedir"
+	"github.com/ooni/probe-cli/v3/internal/fsx"
 )
 
 // RequiredDirs returns the required ooni home directories
@@ -44,13 +45,6 @@ func DBDir(home string, name string) string {
 	return filepath.Join(home, "db", fmt.Sprintf("%s.sqlite3", name))
 }
 
-// FileExists returns true if the specified path exists and is a
-// regular file.
-func FileExists(path string) bool {
-	stat, err := os.Stat(path)
-	return err == nil && stat.Mode().IsRegular()
-}
-
 // GetOONIHome returns the path to the OONI Home
 func GetOONIHome() (string, error) {
 	if ooniHome := os.Getenv("OONI_HOME"); ooniHome != "" {
@@ -74,5 +68,5 @@ func DidLegacyInformedConsent() bool {
 	}
 
 	path := filepath.Join(filepath.Join(home, ".ooni"), "initialized")
-	return FileExists(path)
+	return fsx.RegularFileExists(path)
 }

internal/cmd/miniooni/consent.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"path"
 
+	"github.com/ooni/probe-cli/v3/internal/fsx"
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
 )
 
@@ -18,7 +19,7 @@ func acquireUserConsent(miniooniDir string, currentOptions *Options) {
 	err := maybeWriteConsentFile(currentOptions.Yes, consentFile)
 	runtimex.PanicOnError(err, "cannot write informed consent file")
 	runtimex.Assert(
-		regularFileExists(consentFile),
+		fsx.RegularFileExists(consentFile),
 		riskOfRunningOONI,
 	)
 }

internal/cmd/miniooni/utils.go

@@ -14,12 +14,6 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
 )
 
-// regularFileExists returns true if the given filepath exists and is a regular file
-func regularFileExists(filepath string) bool {
-	stat, err := os.Stat(filepath)
-	return err == nil && stat.Mode().IsRegular()
-}
-
 // splitPair takes in input a string in the form KEY=VALUE and splits it. This
 // function returns an error if it cannot find the = character to split the string.
 func splitPair(s string) (string, string, error) {

internal/cmd/oonireport/oonireport.go

@@ -13,6 +13,7 @@ import (
 	"github.com/apex/log"
 
 	"github.com/ooni/probe-cli/v3/internal/engine"
+	"github.com/ooni/probe-cli/v3/internal/fsx"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/probeservices"
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
@@ -52,11 +53,6 @@ func fatalIfFalse(cond bool, msg string) {
 	}
 }
 
-func canOpen(filepath string) bool {
-	stat, err := os.Stat(filepath)
-	return err == nil && stat.Mode().IsRegular()
-}
-
 func readLines(path string) []string {
 	// open measurement file
 	file, err := os.Open(path)
@@ -127,7 +123,7 @@ func submitAll(ctx context.Context, lines []string, subm *probeservices.Submitte
 func mainWithArgs(args []string) {
 	fatalIfFalse(len(args) == 2, "Usage: ./oonireport upload <file>")
 	fatalIfFalse(args[0] == "upload", "Unsupported operation")
-	fatalIfFalse(canOpen(args[1]), "Cannot open measurement file")
+	fatalIfFalse(fsx.RegularFileExists(args[1]), "Cannot open measurement file")
 
 	path = args[1]
 	lines := readLines(path)

internal/cmd/oonireport/oonireport_test.go

@@ -5,13 +5,6 @@ import (
 	"testing"
 )
 
-func TestCanOpen(t *testing.T) {
-	ok := canOpen("testdata/testmeasurement.json")
-	if !ok {
-		t.Fatal("unexpected error")
-	}
-}
-
 func TestReadLines(t *testing.T) {
 	lines := readLines("testdata/testmeasurement.json")
 	if lines == nil {

internal/fsx/fsx.go

@@ -29,7 +29,7 @@ func openWithFS(fs fs.FS, pathname string) (fs.File, error) {
 		file.Close()
 		return nil, err
 	}
-	if info.IsDir() {
+	if !isRegular(info) {
 		file.Close()
 		return nil, &os.PathError{
 			Op:   "openFile",
@@ -47,3 +47,17 @@ type filesystem struct{}
 func (filesystem) Open(pathname string) (fs.File, error) {
 	return os.Open(pathname)
 }
+
+func isRegular(info fs.FileInfo) bool {
+	return info.Mode().IsRegular()
+}
+
+// RegularFileExists returns whether the given [filename]
+// exists and is a regular file.
+func RegularFileExists(filename string) bool {
+	finfo, err := os.Stat(filename)
+	if err != nil {
+		return false
+	}
+	return isRegular(finfo)
+}

internal/fsx/fsx_test.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"io/fs"
 	"os"
+	"path/filepath"
 	"sync/atomic"
 	"syscall"
 	"testing"
@@ -81,3 +82,28 @@ func TestOpeningExistingFileShouldWork(t *testing.T) {
 	}
 	defer file.Close()
 }
+
+func TestRegularFileExists(t *testing.T) {
+	t.Run("for existing file", func(t *testing.T) {
+		path := filepath.Join("testdata", "testfile.txt")
+		exists := RegularFileExists(path)
+		if !exists {
+			t.Fatal("should exist")
+		}
+	})
+
+	t.Run("for existing directory", func(t *testing.T) {
+		exists := RegularFileExists("testdata")
+		if exists {
+			t.Fatal("should not exist")
+		}
+	})
+
+	t.Run("for nonexisting file", func(t *testing.T) {
+		path := filepath.Join("testdata", "nonexistent")
+		exists := RegularFileExists(path)
+		if exists {
+			t.Fatal("should not exist")
+		}
+	})
+}