diff --git a/scanner/metadata/taglib.go b/scanner/metadata/taglib.go index e2d154638..8465225d5 100644 --- a/scanner/metadata/taglib.go +++ b/scanner/metadata/taglib.go @@ -51,7 +51,6 @@ func (e *taglibExtractor) extractMetadata(filePath string) (*taglibMetadata, err md.tags, err = taglib.Read(filePath) if err != nil { log.Warn("Error reading metadata from file. Skipping", "filePath", filePath, err) - return nil, errors.New("error reading tags") } md.hasPicture = hasEmbeddedImage(filePath) return md, nil diff --git a/scanner/walk_dir_tree.go b/scanner/walk_dir_tree.go index aa7fee464..11ba3c2d9 100644 --- a/scanner/walk_dir_tree.go +++ b/scanner/walk_dir_tree.go @@ -2,9 +2,10 @@ package scanner import ( "context" - "io/ioutil" + "io/fs" "os" "path/filepath" + "sort" "strings" "time" @@ -24,6 +25,26 @@ type ( walkResults = chan dirStats ) +func fullReadDir(name string) ([]os.DirEntry, error) { + f, err := os.Open(name) + if err != nil { + return nil, err + } + defer f.Close() + + var allDirs []os.DirEntry + for { + dirs, err := f.ReadDir(-1) + allDirs = append(allDirs, dirs...) + if err == nil { + break + } + log.Warn("Skipping DirEntry", err) + } + sort.Slice(allDirs, func(i, j int) bool { return allDirs[i].Name() < allDirs[j].Name() }) + return allDirs, nil +} + func walkDirTree(ctx context.Context, rootFolder string, results walkResults) error { err := walkFolder(ctx, rootFolder, rootFolder, results) if err != nil { @@ -49,84 +70,94 @@ func walkFolder(ctx context.Context, rootPath string, currentFolder string, resu log.Trace(ctx, "Found directory", "dir", dir, "audioCount", stats.AudioFilesCount, "hasImages", stats.HasImages, "hasPlaylist", stats.HasPlaylist) stats.Path = dir - results <- stats + results <- *stats return nil } -func loadDir(ctx context.Context, dirPath string) (children []string, stats dirStats, err error) { +func loadDir(ctx context.Context, dirPath string) ([]string, *dirStats, error) { + var children []string + stats := &dirStats{} + dirInfo, err := os.Stat(dirPath) if err != nil { log.Error(ctx, "Error stating dir", "path", dirPath, err) - return + return nil, nil, err } stats.ModTime = dirInfo.ModTime() - files, err := ioutil.ReadDir(dirPath) + dirEntries, err := fullReadDir(dirPath) if err != nil { - log.Error(ctx, "Error reading dir", "path", dirPath, err) - return + log.Error(ctx, "Error in ReadDir", "path", dirPath, err) + return children, stats, err } - for _, f := range files { - isDir, err := isDirOrSymlinkToDir(dirPath, f) + for _, entry := range dirEntries { + isDir, err := isDirOrSymlinkToDir(dirPath, entry) // Skip invalid symlinks if err != nil { - log.Error(ctx, "Invalid symlink", "dir", dirPath) + log.Error(ctx, "Invalid symlink", "dir", filepath.Join(dirPath, entry.Name()), err) continue } - if isDir && !isDirIgnored(dirPath, f) && isDirReadable(dirPath, f) { - children = append(children, filepath.Join(dirPath, f.Name())) + if isDir && !isDirIgnored(dirPath, entry) && isDirReadable(dirPath, entry) { + children = append(children, filepath.Join(dirPath, entry.Name())) } else { - if f.ModTime().After(stats.ModTime) { - stats.ModTime = f.ModTime() + fileInfo, err := entry.Info() + if err != nil { + log.Error(ctx, "Error getting fileInfo", "name", entry.Name(), err) + return children, stats, err } - if utils.IsAudioFile(f.Name()) { + if fileInfo.ModTime().After(stats.ModTime) { + stats.ModTime = fileInfo.ModTime() + } + if utils.IsAudioFile(entry.Name()) { stats.AudioFilesCount++ } else { - stats.HasPlaylist = stats.HasPlaylist || utils.IsPlaylist(f.Name()) - stats.HasImages = stats.HasImages || utils.IsImageFile(f.Name()) + stats.HasPlaylist = stats.HasPlaylist || utils.IsPlaylist(entry.Name()) + stats.HasImages = stats.HasImages || utils.IsImageFile(entry.Name()) } } } - return + return children, stats, nil } -// isDirOrSymlinkToDir returns true if and only if the dirInfo represents a file -// system directory, or a symbolic link to a directory. Note that if the dirInfo +// isDirOrSymlinkToDir returns true if and only if the dirEnt represents a file +// system directory, or a symbolic link to a directory. Note that if the dirEnt // is not a directory but is a symbolic link, this method will resolve by // sending a request to the operating system to follow the symbolic link. -// Copied from github.com/karrick/godirwalk -func isDirOrSymlinkToDir(baseDir string, dirInfo os.FileInfo) (bool, error) { - if dirInfo.IsDir() { +// originally copied from github.com/karrick/godirwalk, modified to use dirEntry for +// efficiency for go 1.16 and beyond +func isDirOrSymlinkToDir(baseDir string, dirEnt fs.DirEntry) (bool, error) { + if dirEnt.IsDir() { return true, nil } - if dirInfo.Mode()&os.ModeSymlink == 0 { + if dirEnt.Type()&os.ModeSymlink == 0 { return false, nil } // Does this symlink point to a directory? - dirInfo, err := os.Stat(filepath.Join(baseDir, dirInfo.Name())) + fileInfo, err := os.Stat(filepath.Join(baseDir, dirEnt.Name())) if err != nil { return false, err } - return dirInfo.IsDir(), nil + return fileInfo.IsDir(), nil } -// isDirIgnored returns true if the directory represented by dirInfo contains an +// isDirIgnored returns true if the directory represented by dirEnt contains an // `ignore` file (named after consts.SkipScanFile) -func isDirIgnored(baseDir string, dirInfo os.FileInfo) bool { - if strings.HasPrefix(dirInfo.Name(), ".") { +func isDirIgnored(baseDir string, dirEnt fs.DirEntry) bool { + // allows Album folders for albums which eg start with ellipses + if strings.HasPrefix(dirEnt.Name(), ".") && !strings.HasPrefix(dirEnt.Name(), "..") { return true } - _, err := os.Stat(filepath.Join(baseDir, dirInfo.Name(), consts.SkipScanFile)) + _, err := os.Stat(filepath.Join(baseDir, dirEnt.Name(), consts.SkipScanFile)) return err == nil } -// isDirReadable returns true if the directory represented by dirInfo is readable -func isDirReadable(baseDir string, dirInfo os.FileInfo) bool { - path := filepath.Join(baseDir, dirInfo.Name()) +// isDirReadable returns true if the directory represented by dirEnt is readable +func isDirReadable(baseDir string, dirEnt fs.DirEntry) bool { + path := filepath.Join(baseDir, dirEnt.Name()) res, err := utils.IsDirReadable(path) if !res { - log.Debug("Warning: Skipping unreadable directory", "path", path, err) + log.Warn("Skipping unreadable directory", "path", path, err) } return res } diff --git a/scanner/walk_dir_tree_test.go b/scanner/walk_dir_tree_test.go index 47c58d0ed..6c8464bfb 100644 --- a/scanner/walk_dir_tree_test.go +++ b/scanner/walk_dir_tree_test.go @@ -10,7 +10,8 @@ import ( . "github.com/onsi/gomega/gstruct" ) -var _ = Describe("load_tree", func() { +var _ = Describe("walk_dir_tree", func() { + baseDir := filepath.Join("tests", "fixtures") Describe("walkDirTree", func() { It("reads all info correctly", func() { @@ -18,7 +19,7 @@ var _ = Describe("load_tree", func() { results := make(walkResults, 5000) var err error go func() { - err = walkDirTree(context.TODO(), "tests/fixtures", results) + err = walkDirTree(context.TODO(), baseDir, results) }() for { @@ -30,49 +31,61 @@ var _ = Describe("load_tree", func() { } Expect(err).To(BeNil()) - Expect(collected["tests/fixtures"]).To(MatchFields(IgnoreExtras, Fields{ + Expect(collected[baseDir]).To(MatchFields(IgnoreExtras, Fields{ "HasImages": BeTrue(), "HasPlaylist": BeFalse(), "AudioFilesCount": BeNumerically("==", 4), })) - Expect(collected["tests/fixtures/playlists"].HasPlaylist).To(BeTrue()) - Expect(collected).To(HaveKey("tests/fixtures/symlink2dir")) - Expect(collected).To(HaveKey("tests/fixtures/empty_folder")) + Expect(collected[filepath.Join(baseDir, "playlists")].HasPlaylist).To(BeTrue()) + Expect(collected).To(HaveKey(filepath.Join(baseDir, "symlink2dir"))) + Expect(collected).To(HaveKey(filepath.Join(baseDir, "empty_folder"))) }) }) Describe("isDirOrSymlinkToDir", func() { It("returns true for normal dirs", func() { - dir, _ := os.Stat("tests/fixtures") - Expect(isDirOrSymlinkToDir("tests", dir)).To(BeTrue()) + dirEntry, _ := getDirEntry("tests", "fixtures") + Expect(isDirOrSymlinkToDir(baseDir, dirEntry)).To(BeTrue()) }) It("returns true for symlinks to dirs", func() { - dir, _ := os.Stat("tests/fixtures/symlink2dir") - Expect(isDirOrSymlinkToDir("tests/fixtures", dir)).To(BeTrue()) + dirEntry, _ := getDirEntry(baseDir, "symlink2dir") + Expect(isDirOrSymlinkToDir(baseDir, dirEntry)).To(BeTrue()) }) It("returns false for files", func() { - dir, _ := os.Stat("tests/fixtures/test.mp3") - Expect(isDirOrSymlinkToDir("tests/fixtures", dir)).To(BeFalse()) + dirEntry, _ := getDirEntry(baseDir, "test.mp3") + Expect(isDirOrSymlinkToDir(baseDir, dirEntry)).To(BeFalse()) }) It("returns false for symlinks to files", func() { - dir, _ := os.Stat("tests/fixtures/symlink") - Expect(isDirOrSymlinkToDir("tests/fixtures", dir)).To(BeFalse()) + dirEntry, _ := getDirEntry(baseDir, "symlink") + Expect(isDirOrSymlinkToDir(baseDir, dirEntry)).To(BeFalse()) }) }) - Describe("isDirIgnored", func() { - baseDir := filepath.Join("tests", "fixtures") It("returns false for normal dirs", func() { - dir, _ := os.Stat(filepath.Join(baseDir, "empty_folder")) - Expect(isDirIgnored(baseDir, dir)).To(BeFalse()) + dirEntry, _ := getDirEntry(baseDir, "empty_folder") + Expect(isDirIgnored(baseDir, dirEntry)).To(BeFalse()) }) It("returns true when folder contains .ndignore file", func() { - dir, _ := os.Stat(filepath.Join(baseDir, "ignored_folder")) - Expect(isDirIgnored(baseDir, dir)).To(BeTrue()) + dirEntry, _ := getDirEntry(baseDir, "ignored_folder") + Expect(isDirIgnored(baseDir, dirEntry)).To(BeTrue()) }) It("returns true when folder name starts with a `.`", func() { - dir, _ := os.Stat(filepath.Join(baseDir, ".hidden_folder")) - Expect(isDirIgnored(baseDir, dir)).To(BeTrue()) + dirEntry, _ := getDirEntry(baseDir, ".hidden_folder") + Expect(isDirIgnored(baseDir, dirEntry)).To(BeTrue()) + }) + It("returns false when folder name starts with ellipses", func() { + dirEntry, _ := getDirEntry(baseDir, "...unhidden_folder") + Expect(isDirIgnored(baseDir, dirEntry)).To(BeFalse()) }) }) }) + +func getDirEntry(baseDir, name string) (os.DirEntry, error) { + dirEntries, _ := os.ReadDir(baseDir) + for _, entry := range dirEntries { + if entry.Name() == name { + return entry, nil + } + } + return nil, os.ErrNotExist +} diff --git a/tests/fixtures/...unhidden_folder/.gitkeep b/tests/fixtures/...unhidden_folder/.gitkeep new file mode 100644 index 000000000..e69de29bb