diff --git a/README.md b/README.md index 5ef3834..39b33b0 100644 --- a/README.md +++ b/README.md @@ -116,8 +116,8 @@ The main types are: - `Image` and `Video` embed `Object`. The file extension decides which one a file becomes. -A mounted filesystem is a DCF store when its root holds a `DCIM` or `M4ROOT` -directory. `dcf` never mounts a filesystem. +A mounted filesystem is a DCF store when its root holds a `DCIM` or +`PRIVATE/M4ROOT` directory. `dcf` never mounts a filesystem. ## TODO @@ -126,13 +126,9 @@ directory. `dcf` never mounts a filesystem. tar of a memory card that uses them, and I'll see what I can do. - Parse the DCF directory and file names, rather than listing every media file on the filesystem. -- Look for `PRIVATE/M4ROOT` rather than `M4ROOT` when detecting a DCF store. -- Return exactly the number of stores asked for. `GetDCFStores` returns one too - few when asked for two or more. - Read stores through `fs.FS`, with a `context.Context` to cancel a walk. - Return a plain slice from `GetDCFStores`, not a pointer to one. -- Test the walk of a store and the detection of mount points. They only run - against real mounted media today. +- Test the walk of a store. It only runs against real mounted media today. - Copy files off a card, and verify each copy against its SHA256 hash. ## License diff --git a/pkg/dcf/helpers.go b/pkg/dcf/helpers.go index db80710..5725697 100644 --- a/pkg/dcf/helpers.go +++ b/pkg/dcf/helpers.go @@ -28,45 +28,51 @@ func findAllMountPoints() ([]string, error) { } // findDCFMountPoints returns a list of strings of the paths of mountpoints -// that contain a DCIM or M4ROOT directory, which is how we detect DCF stores. -// Its only argument is an integer of how many mountpoints to return. If the -// argument is 0, all mountpoints are returned. +// that hold a DCF store. Its only argument is an integer of how many mountpoints +// to return. If the argument is 0, all mountpoints are returned. func findDCFMountPoints(requestedCount int) ([]string, error) { - filteredMountpoints := []string{} - mountpoints, err := findAllMountPoints() if err != nil { return nil, err } + return filterDCFMountPoints(mountpoints, requestedCount), nil +} + +// filterDCFMountPoints returns the first requestedCount of the mountpoints that +// hold a DCF store. If requestedCount is 0, it returns all of them. +func filterDCFMountPoints(mountpoints []string, requestedCount int) []string { + filteredMountpoints := []string{} + for _, mountpoint := range mountpoints { - dcimPath := filepath.Join(mountpoint, "DCIM") - m4rootPath := filepath.Join(mountpoint, "M4ROOT") - shouldKeep := false - - _, err = os.Stat(dcimPath) - if err == nil { - shouldKeep = true - - slog.Debug(fmt.Sprintf("Found DCIM directory at %s\n", dcimPath)) + if !isDCFMountPoint(mountpoint) { + continue } - _, err = os.Stat(m4rootPath) - if err == nil { - shouldKeep = true + slog.Debug(fmt.Sprintf("Keeping mountpoint %s\n", mountpoint)) - slog.Debug(fmt.Sprintf("Found M4ROOT directory at %s\n", m4rootPath)) - } - - if shouldKeep { - slog.Debug(fmt.Sprintf("Keeping mountpoint %s\n", mountpoint)) - - filteredMountpoints = append(filteredMountpoints, mountpoint) - if (requestedCount > 0) && (len(filteredMountpoints)+1 >= requestedCount) { - break - } + filteredMountpoints = append(filteredMountpoints, mountpoint) + if (requestedCount > 0) && (len(filteredMountpoints) >= requestedCount) { + break } } - return filteredMountpoints, nil + return filteredMountpoints +} + +// isDCFMountPoint reports whether the root of mountpoint holds a DCIM or +// PRIVATE/M4ROOT directory, which is how we detect DCF stores. +func isDCFMountPoint(mountpoint string) bool { + for _, directory := range []string{"DCIM", "PRIVATE/M4ROOT"} { + path := filepath.Join(mountpoint, directory) + + info, err := os.Stat(path) + if err == nil && info.IsDir() { + slog.Debug(fmt.Sprintf("Found %s directory at %s\n", directory, path)) + + return true + } + } + + return false } diff --git a/pkg/dcf/helpers_internal_test.go b/pkg/dcf/helpers_internal_test.go new file mode 100644 index 0000000..1d6bef9 --- /dev/null +++ b/pkg/dcf/helpers_internal_test.go @@ -0,0 +1,85 @@ +package dcf + +import ( + "os" + "path/filepath" + "slices" + "testing" +) + +// newMountPoint returns a temporary directory holding the given directories. +func newMountPoint(t *testing.T, directories ...string) string { + t.Helper() + + root := t.TempDir() + + for _, directory := range directories { + err := os.MkdirAll(filepath.Join(root, directory), 0o750) + if err != nil { + t.Fatalf("MkdirAll: %v", err) + } + } + + return root +} + +func TestIsDCFMountPoint(t *testing.T) { + t.Parallel() + + cases := map[string]bool{ + "DCIM": true, + "PRIVATE/M4ROOT": true, + "M4ROOT": false, + "PRIVATE": false, + } + + for directory, want := range cases { + got := isDCFMountPoint(newMountPoint(t, directory)) + if got != want { + t.Errorf("isDCFMountPoint with only %q = %v, want %v", + directory, got, want) + } + } +} + +func TestIsDCFMountPointIgnoresPlainFile(t *testing.T) { + t.Parallel() + + root := t.TempDir() + + err := os.WriteFile(filepath.Join(root, "DCIM"), nil, 0o600) + if err != nil { + t.Fatalf("WriteFile: %v", err) + } + + if isDCFMountPoint(root) { + t.Fatal("isDCFMountPoint counted a plain file named DCIM as a directory") + } +} + +func TestFilterDCFMountPointsReturnsRequestedCount(t *testing.T) { + t.Parallel() + + first := newMountPoint(t, "DCIM") + second := newMountPoint(t, "PRIVATE/M4ROOT") + third := newMountPoint(t, "DCIM") + + // This one holds no DCF store, so it must not use up the requested count. + mountpoints := []string{newMountPoint(t), first, second, third} + + cases := map[int][]string{ + 0: {first, second, third}, + 1: {first}, + 2: {first, second}, + 3: {first, second, third}, + 4: {first, second, third}, + } + + for requestedCount, want := range cases { + got := filterDCFMountPoints(mountpoints, requestedCount) + if !slices.Equal(got, want) { + t.Errorf("filterDCFMountPoints(%d) = %v, want %v", + requestedCount, got, want) + } + } +}