Merge pull request #358 from blackcandy-org/speedup-media-sync

Speed up media sync on rescan

app/models/media.rb

@@ -47,12 +47,17 @@ def syncing=(is_syncing)
     private
 
     def add_files(file_paths)
-      file_paths.map do |file_path|
+      file_md5_hashes = file_paths.map { |file_path| MediaFile.get_md5_hash(file_path, with_mtime: true) }
+      existing_songs = Song.where(md5_hash: file_md5_hashes)
+
+      added_song_hashes = (file_paths - existing_songs.pluck(:file_path)).map do |file_path|
         file_info = MediaFile.file_info(file_path)
         file_info[:md5_hash] if attach(file_info)
       rescue
         next
       end.compact
+
+      added_song_hashes + existing_songs.pluck(:md5_hash)
     end
 
     def remove_files(file_paths)

app/models/media_file.rb

@@ -33,6 +33,8 @@ def file_info(file_path)
     end
 
     def get_md5_hash(file_path, with_mtime: false)
+      return unless File.exist?(file_path)
+
       string = "#{file_path}#{File.mtime(file_path) if with_mtime}"
       Digest::MD5.base64digest(string)
     end

test/models/media_test.rb

@@ -50,7 +50,7 @@ class MediaTest < ActiveSupport::TestCase
   end
 
   test "should change associations when modify album info on file" do
-    MediaFile.stub(:file_info, media_file_info_stub(file_fixture("artist1_album2.mp3"), album_name: "album1")) do
+    stub_file_metadata(file_fixture("artist1_album2.mp3"), album_name: "album1") do
       Media.sync_all
 
       album1_songs_ids = Song.where(name: %w[flac_sample m4a_sample mp3_sample]).ids.sort
@@ -61,10 +61,7 @@ class MediaTest < ActiveSupport::TestCase
   end
 
   test "should change associations when modify artist info on file" do
-    MediaFile.stub(
-      :file_info,
-      media_file_info_stub(file_fixture("artist1_album2.mp3"), artist_name: "artist2", albumartist_name: "artist2")
-    ) do
+    stub_file_metadata(file_fixture("artist1_album2.mp3"), artist_name: "artist2", albumartist_name: "artist2") do
       Media.sync_all
 
       artist2_songs_ids = Song.where(
@@ -77,7 +74,7 @@ class MediaTest < ActiveSupport::TestCase
   end
 
   test "should change song attribute when modify song info on file" do
-    MediaFile.stub(:file_info, media_file_info_stub(file_fixture("artist1_album2.mp3"), tracknum: 2)) do
+    stub_file_metadata(file_fixture("artist1_album2.mp3"), tracknum: 2) do
       assert_changes -> { Song.find_by(name: "mp3_sample").tracknum }, from: 1, to: 2 do
         Media.sync_all
       end
@@ -177,7 +174,7 @@ class MediaTest < ActiveSupport::TestCase
   end
 
   test "should change associations when selectively modified album info on file" do
-    MediaFile.stub(:file_info, media_file_info_stub(file_fixture("artist1_album2.mp3"), album_name: "album1")) do
+    stub_file_metadata(file_fixture("artist1_album2.mp3"), album_name: "album1") do
       Media.sync(:modified, [file_fixture("artist1_album2.mp3")])
 
       album1_songs_ids = Song.where(name: %w[flac_sample m4a_sample mp3_sample]).ids.sort
@@ -188,10 +185,7 @@ class MediaTest < ActiveSupport::TestCase
   end
 
   test "should change associations when selectively modified artist info on file" do
-    MediaFile.stub(
-      :file_info,
-      media_file_info_stub(file_fixture("artist1_album2.mp3"), artist_name: "artist2", albumartist_name: "artist2")
-    ) do
+    stub_file_metadata(file_fixture("artist1_album2.mp3"), artist_name: "artist2", albumartist_name: "artist2") do
       Media.sync(:modified, [file_fixture("artist1_album2.mp3")])
 
       artist2_songs_ids = Song.where(
@@ -204,7 +198,7 @@ class MediaTest < ActiveSupport::TestCase
   end
 
   test "should change song attribute when selectively modified song info on file" do
-    MediaFile.stub(:file_info, media_file_info_stub(file_fixture("artist1_album2.mp3"), tracknum: 2)) do
+    stub_file_metadata(file_fixture("artist1_album2.mp3"), tracknum: 2) do
       assert_changes -> { Song.find_by(name: "mp3_sample").tracknum }, from: 1, to: 2 do
         Media.sync(:modified, [file_fixture("artist1_album2.mp3")])
       end

test/test_helper.rb

@@ -36,6 +36,26 @@
 MediaListener.config do |config|
   config.service_name = "media_listener_service_test"
 end
+class MediaFileMock < MediaFile
+  class << self
+    alias_method :real_file_info, :file_info
+  end
+
+  def initialize(file_path, attributes = {})
+    @file_path = file_path
+    @attributes = attributes
+    @mtime = File.mtime(file_path)
+  end
+
+  def mtime(path)
+    (path.to_s == @file_path.to_s) ? Time.now : @mtime
+  end
+
+  def file_info(path)
+    file_info = self.class.real_file_info(path)
+    (path.to_s == @file_path.to_s) ? file_info.merge(@attributes) : file_info
+  end
+end
 
 class ActiveSupport::TestCase
   include Turbo::Broadcastable::TestHelper
@@ -100,15 +120,13 @@ def binary_data(file_path)
     File.read(file_path).force_encoding("BINARY").strip
   end
 
-  def media_file_info_stub(file_path, attributes = {})
-    proc do |media_file_path|
-      file_info = MediaFile.send(:get_tag_info, media_file_path).merge(
-        file_path: media_file_path.to_s,
-        file_path_hash: MediaFile.get_md5_hash(media_file_path),
-        md5_hash: MediaFile.get_md5_hash(media_file_path, with_mtime: true)
-      )
+  def stub_file_metadata(file_path, attributes = {})
+    media_file_mock = MediaFileMock.new(file_path, attributes)
 
-      (media_file_path.to_s == file_path.to_s) ? file_info.merge(**attributes, md5_hash: "new_md5_hash") : file_info
+    File.stub(:mtime, media_file_mock.method(:mtime)) do
+      MediaFile.stub(:file_info, media_file_mock.method(:file_info)) do
+        yield
+      end
     end
   end