Refactor and fix the file watcher

app/Helpers/FSWatchRecord.php

@@ -23,6 +23,8 @@ class FSWatchRecord
      */
     protected $eventFlags;
 
+    protected $watchedEvents = ['Created', 'Removed', 'Renamed', 'Updated'];
+
     /**
      * Construct an FSWatchRecord object for a record string.
      *
@@ -36,6 +38,23 @@ class FSWatchRecord
         $this->path = implode(' ', $parts);
     }
 
+    /**
+     * Determine if the event is valid to Koel.
+     * We only watch Created, Removed, Renamed, and Updated events.
+     *
+     * @return boolean
+     */
+    public function isValidEvent()
+    {
+        foreach ($this->watchedEvents as $e) {
+            if (in_array($e, $this->eventFlags)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     /**
      * Determine if the file/directory is deleted from the system.
      * We can't rely on fswatch, since the event is OS-dependent.
@@ -49,16 +68,6 @@ class FSWatchRecord
         return !file_exists($this->path);
     }
 
-    /**
-     * Determine if the object is renamed.
-     *
-     * @return boolean
-     */
-    public function isRenamed()
-    {
-        return in_array('Renamed', $this->eventFlags);
-    }
-
     /**
      * Determine if the changed object is a file.
      *
@@ -66,7 +75,7 @@ class FSWatchRecord
      */
     public function isFile()
     {
-        return in_array('IsFile', $this->eventFlags);
+        return is_file($this->path);
     }
 
     /**
@@ -76,7 +85,7 @@ class FSWatchRecord
      */
     public function isDir()
     {
-        return in_array('IsDir', $this->eventFlags);
+        return is_dir($this->path);
     }
 
     /**

app/Services/Media.php

@@ -151,54 +151,41 @@ class Media
             $record = new FSWatchRecord($record);
         }
 
+        if (!$record->isValidEvent()) {
+            return;
+        }
+
         $path = $record->getPath();
 
-        if ($record->isFile()) {
-            // If the file has been deleted...
         if ($record->isDeleted()) {
-                // ...and it has a record in our database, remove it.
+            // Since the item has been deleted, we can't tell if it was a file or a directory.
+            // So here we're assuming it to be a file first, and directory second.
             if ($song = Song::byPath($path)) {
                 $song->delete();
-
                 Log::info("Deleted $path");
+            } else {
+                Song::inDirectory($path)->delete();
+                Log::info("Deleted all song(s) under $path");
+            }
 
             event(new LibraryChanged());
-                }
-            }
-            // Otherwise, it's a new or changed file. Try to sync it in.
-            // File format etc. will be handled by the syncFile method.
-            else {
-                Log::info("Syncing file $path");
-                Log::info($this->syncFile($path) instanceof Song ? "Synchronized $path" : "Invalid file $path");
-            }
 
             return;
         }
 
-        if ($record->isDir()) {
+        // Now if the added/modified item is a file, we simply sync it into the database.
-            if ($record->isDeleted()) {
+        if ($record->isFile()) {
-                // A whole directory is removed.
+            Log::info("Syncing file $path");
-                // We remove all songs in it.
+            Log::info($this->syncFile($path) instanceof Song ? "Synchronized $path" : "Invalid file $path");
-                Song::inDirectory($path)->delete();
+        }
-
+        // But if it's a whole directory, we traverse through it and sync all children.
-                Log::info("Deleted all song(s) under $path");
+        else {
-
-                event(new LibraryChanged());
-            } elseif ($record->isRenamed()) {
             foreach ($this->gatherFiles($path) as $file) {
                 $this->syncFile($file);
             }
 
             Log::info("Synced all song(s) under $path");
-            } else {
-                // "New directory" fswatch event actually comes with individual "new file" events,
-                // which should already be handled by our logic above.
         }
-
-            return;
-        }
-
-        // The changed item is a symlink maybe. But we're not doing anything with it.
     }
 
     /**

tests/MediaTest.php

@@ -73,6 +73,7 @@ class MediaTest extends TestCase
             'isDeleted' => false,
             'getPath' => $path,
             'isFile' => true,
+            'isValidEvent' => true,
         ], ["$path IsFile"]);
 
         (new Media())->syncFSWatchRecord($record);
@@ -91,6 +92,7 @@ class MediaTest extends TestCase
             'isDeleted' => true,
             'getPath' => $song->path,
             'isFile' => true,
+            'isValidEvent' => true,
         ], ["{$song->path} IsFile"]);
 
         (new Media())->syncFSWatchRecord($record);
@@ -111,6 +113,7 @@ class MediaTest extends TestCase
             'getPath' => $path,
             'isFile' => false,
             'isDir' => true,
+            'isValidEvent' => true,
         ], ["$path IsDir"]);
 
         $media->syncFSWatchRecord($record);