Merge pull request #4031 from nichwall/temp_file_ignore_refactor

Refactor ignore file logic

server/Watcher.js

@@ -5,7 +5,7 @@ const Logger = require('./Logger')
 const Task = require('./objects/Task')
 const TaskManager = require('./managers/TaskManager')
 
-const { filePathToPOSIX, isSameOrSubPath, getFileMTimeMs } = require('./utils/fileUtils')
+const { filePathToPOSIX, isSameOrSubPath, getFileMTimeMs, shouldIgnoreFile } = require('./utils/fileUtils')
 
 /**
  * @typedef PendingFileUpdate
@@ -286,15 +286,10 @@ class FolderWatcher extends EventEmitter {
 
     const relPath = path.replace(folderPath, '')
 
-    if (Path.extname(relPath).toLowerCase() === '.part') {
-      Logger.debug(`[Watcher] Ignoring .part file "${relPath}"`)
-      return false
-    }
-
-    // Ignore files/folders starting with "."
-    const hasDotPath = relPath.split('/').find((p) => p.startsWith('.'))
-    if (hasDotPath) {
-      Logger.debug(`[Watcher] Ignoring dot path "${relPath}" | Piece "${hasDotPath}"`)
+    // Check for ignored extensions or directories, such as dotfiles and hidden directories
+    const shouldIgnore = shouldIgnoreFile(relPath)
+    if (shouldIgnore) {
+      Logger.debug(`[Watcher] Ignoring ${shouldIgnore} - "${relPath}"`)
       return false
     }
 

server/utils/fileUtils.js

@@ -131,6 +131,40 @@ async function readTextFile(path) {
 }
 module.exports.readTextFile = readTextFile
 
+/**
+ * Check if file or directory should be ignored. Returns a string of the reason to ignore, or null if not ignored
+ *
+ * @param {string} path
+ * @returns {string}
+ */
+module.exports.shouldIgnoreFile = (path) => {
+  // Check if directory or file name starts with "."
+  if (Path.basename(path).startsWith('.')) {
+    return 'dotfile'
+  }
+  if (path.split('/').find((p) => p.startsWith('.'))) {
+    return 'dotpath'
+  }
+
+  // If these strings exist anywhere in the filename or directory name, ignore. Vendor specific hidden directories
+  const includeAnywhereIgnore = ['@eaDir']
+  const filteredInclude = includeAnywhereIgnore.filter((str) => path.includes(str))
+  if (filteredInclude.length) {
+    return `${filteredInclude[0]} directory`
+  }
+
+  const extensionIgnores = ['.part', '.tmp', '.crdownload', '.download', '.bak', '.old', '.temp', '.tempfile', '.tempfile~']
+
+  // Check extension
+  if (extensionIgnores.includes(Path.extname(path).toLowerCase())) {
+    // Return the extension that is ignored
+    return `${Path.extname(path)} file`
+  }
+
+  // Should not ignore this file or directory
+  return null
+}
+
 /**
  * @typedef FilePathItem
  * @property {string} name - file name e.g. "audiofile.m4b"
@@ -147,7 +181,7 @@ module.exports.readTextFile = readTextFile
  * @param {string} [relPathToReplace]
  * @returns {FilePathItem[]}
  */
-async function recurseFiles(path, relPathToReplace = null) {
+module.exports.recurseFiles = async (path, relPathToReplace = null) => {
   path = filePathToPOSIX(path)
   if (!path.endsWith('/')) path = path + '/'
 
@@ -197,14 +231,10 @@ async function recurseFiles(path, relPathToReplace = null) {
         return false
       }
 
-      if (item.extension === '.part') {
-        Logger.debug(`[fileUtils] Ignoring .part file "${relpath}"`)
-        return false
-      }
-
-      // Ignore any file if a directory or the filename starts with "."
-      if (relpath.split('/').find((p) => p.startsWith('.'))) {
-        Logger.debug(`[fileUtils] Ignoring path has . "${relpath}"`)
+      // Check for ignored extensions or directories
+      const shouldIgnore = this.shouldIgnoreFile(relpath)
+      if (shouldIgnore) {
+        Logger.debug(`[fileUtils] Ignoring ${shouldIgnore} - "${relpath}"`)
         return false
       }
 
@@ -235,7 +265,6 @@ async function recurseFiles(path, relPathToReplace = null) {
 
   return list
 }
-module.exports.recurseFiles = recurseFiles
 
 /**
  *

test/server/utils/fileUtils.test.js

@@ -0,0 +1,127 @@
+const chai = require('chai')
+const expect = chai.expect
+const sinon = require('sinon')
+const fileUtils = require('../../../server/utils/fileUtils')
+const fs = require('fs')
+const Logger = require('../../../server/Logger')
+
+describe('fileUtils', () => {
+  it('shouldIgnoreFile', () => {
+    global.isWin = process.platform === 'win32'
+
+    const testCases = [
+      { path: 'test.txt', expected: null },
+      { path: 'folder/test.mp3', expected: null },
+      { path: 'normal/path/file.m4b', expected: null },
+      { path: 'test.txt.part', expected: '.part file' },
+      { path: 'test.txt.tmp', expected: '.tmp file' },
+      { path: 'test.txt.crdownload', expected: '.crdownload file' },
+      { path: 'test.txt.download', expected: '.download file' },
+      { path: 'test.txt.bak', expected: '.bak file' },
+      { path: 'test.txt.old', expected: '.old file' },
+      { path: 'test.txt.temp', expected: '.temp file' },
+      { path: 'test.txt.tempfile', expected: '.tempfile file' },
+      { path: 'test.txt.tempfile~', expected: '.tempfile~ file' },
+      { path: '.gitignore', expected: 'dotfile' },
+      { path: 'folder/.hidden', expected: 'dotfile' },
+      { path: '.git/config', expected: 'dotpath' },
+      { path: 'path/.hidden/file.txt', expected: 'dotpath' },
+      { path: '@eaDir', expected: '@eaDir directory' },
+      { path: 'folder/@eaDir', expected: '@eaDir directory' },
+      { path: 'path/@eaDir/file.txt', expected: '@eaDir directory' },
+      { path: '.hidden/test.tmp', expected: 'dotpath' },
+      { path: '@eaDir/test.part', expected: '@eaDir directory' }
+    ]
+
+    testCases.forEach(({ path, expected }) => {
+      const result = fileUtils.shouldIgnoreFile(path)
+      expect(result).to.equal(expected)
+    })
+  })
+
+  describe('recurseFiles', () => {
+    let readdirStub, realpathStub, statStub
+
+    beforeEach(() => {
+      global.isWin = process.platform === 'win32'
+
+      // Mock file structure with normalized paths
+      const mockDirContents = new Map([
+        ['/test', ['file1.mp3', 'subfolder', 'ignoreme', 'temp.mp3.tmp']],
+        ['/test/subfolder', ['file2.m4b']],
+        ['/test/ignoreme', ['.ignore', 'ignored.mp3']]
+      ])
+
+      const mockStats = new Map([
+        ['/test/file1.mp3', { isDirectory: () => false, size: 1024, mtimeMs: Date.now(), ino: '1' }],
+        ['/test/subfolder', { isDirectory: () => true, size: 0, mtimeMs: Date.now(), ino: '2' }],
+        ['/test/subfolder/file2.m4b', { isDirectory: () => false, size: 1024, mtimeMs: Date.now(), ino: '3' }],
+        ['/test/ignoreme', { isDirectory: () => true, size: 0, mtimeMs: Date.now(), ino: '4' }],
+        ['/test/ignoreme/.ignore', { isDirectory: () => false, size: 0, mtimeMs: Date.now(), ino: '5' }],
+        ['/test/ignoreme/ignored.mp3', { isDirectory: () => false, size: 1024, mtimeMs: Date.now(), ino: '6' }],
+        ['/test/temp.mp3.tmp', { isDirectory: () => false, size: 1024, mtimeMs: Date.now(), ino: '7' }]
+      ])
+
+      // Stub fs.readdir
+      readdirStub = sinon.stub(fs, 'readdir')
+      readdirStub.callsFake((path, callback) => {
+        const contents = mockDirContents.get(path)
+        if (contents) {
+          callback(null, contents)
+        } else {
+          callback(new Error(`ENOENT: no such file or directory, scandir '${path}'`))
+        }
+      })
+
+      // Stub fs.realpath
+      realpathStub = sinon.stub(fs, 'realpath')
+      realpathStub.callsFake((path, callback) => {
+        // Return normalized path
+        callback(null, fileUtils.filePathToPOSIX(path).replace(/\/$/, ''))
+      })
+
+      // Stub fs.stat
+      statStub = sinon.stub(fs, 'stat')
+      statStub.callsFake((path, callback) => {
+        const normalizedPath = fileUtils.filePathToPOSIX(path).replace(/\/$/, '')
+        const stats = mockStats.get(normalizedPath)
+        if (stats) {
+          callback(null, stats)
+        } else {
+          callback(new Error(`ENOENT: no such file or directory, stat '${normalizedPath}'`))
+        }
+      })
+
+      // Stub Logger
+      sinon.stub(Logger, 'debug')
+    })
+
+    afterEach(() => {
+      sinon.restore()
+    })
+
+    it('should return filtered file list', async () => {
+      const files = await fileUtils.recurseFiles('/test')
+      expect(files).to.be.an('array')
+      expect(files).to.have.lengthOf(2)
+
+      expect(files[0]).to.deep.equal({
+        name: 'file1.mp3',
+        path: 'file1.mp3',
+        reldirpath: '',
+        fullpath: '/test/file1.mp3',
+        extension: '.mp3',
+        deep: 0
+      })
+
+      expect(files[1]).to.deep.equal({
+        name: 'file2.m4b',
+        path: 'subfolder/file2.m4b',
+        reldirpath: 'subfolder',
+        fullpath: '/test/subfolder/file2.m4b',
+        extension: '.m4b',
+        deep: 1
+      })
+    })
+  })
+})