Runner per location (#250)

* Create relation between location and rescan_runner

* Edit method to use instance's location

* Create method to schedule all runners

* tmp rollback

* Migrate and annotate

* Fix wrong method name

* Simplify schedule_all and make run private

* Rework Controller and routes

* Don't schedule new job while running

* Update model tests

* Rename controller to RescansController

* Add tests for new controller

* Fix routes

* Fix policy

* Update way we test schedule methods

* Dont' set finished_at, since this is done in SQL

* Remove unused method

* Update rake task

* Explicitly test that RescanRunner is create in callback

* Include id in serializer

* Update seeds

* Update test

* Make count explicit

* Fix test

Co-authored-by: Charlotte Van Petegem <[email protected]>

app/controllers/rescans_controller.rb

@@ -0,0 +1,31 @@
+class RescansController < ApplicationController
+  before_action :set_rescan, only: %i[show start]
+
+  def index
+    authorize RescanRunner
+    @rescans = policy_scope(RescanRunner).paginate(page: params[:page], per_page: params[:per_page])
+    add_pagination_headers(@rescans)
+    render json: @rescans
+  end
+
+  def show
+    render json: @rescan
+  end
+
+  def start
+    @rescan.schedule
+    render json: @rescan
+  end
+
+  def start_all
+    authorize RescanRunner
+    RescanRunner.schedule_all
+  end
+
+  private
+
+  def set_rescan
+    @rescan = RescanRunner.find(params[:id])
+    authorize @rescan
+  end
+end

app/models/location.rb

@@ -8,6 +8,9 @@
 
 class Location < ApplicationRecord
   has_many :audio_files, dependent: :destroy
+  has_one :rescan_runner, dependent: :destroy
+
+  after_create :create_runner
 
   validates :path, presence: true, uniqueness: true
   validate :cant_be_parent_or_subdir_of_other_location
@@ -24,4 +27,9 @@ def cant_be_parent_or_subdir_of_other_location
       errors.add(:path, 'path-is-parent') if l.expanded_path.fnmatch?(File.join(expanded_path, '**'))
     end
   end
+
+  def create_runner
+    runner = RescanRunner.create(location: self)
+    runner.schedule
+  end
 end

app/models/rescan_runner.rb

@@ -8,17 +8,24 @@
 #  processed    :integer          default(0), not null
 #  running      :boolean          default(FALSE), not null
 #  warning_text :text
+#  location_id  :bigint           not null
 #
 
 class RescanRunner < ApplicationRecord
-  def self.instance
-    RescanRunner.first || RescanRunner.create
-  end
+  belongs_to :location
 
   def schedule
+    return if running?
+
     delay(queue: :rescans).run
   end
 
+  def self.schedule_all
+    find_each(&:schedule)
+  end
+
+  private
+
   def run
     # rubocop:disable Rails/SkipsModelValidations
     # RescanRunner doesn't have validations, and we need to use update_all to use it's atomicity
@@ -30,21 +37,14 @@ def run
     reload
 
     begin
-      unless Location.count.positive?
-        update(error_text: 'No locations defined')
-        return
-      end
-
       unless Codec.count.positive?
         update(error_text: 'No codecs defined')
         return
       end
 
-      Location.all.find_each do |l|
-        process_all_files(l, l.path)
-      end
+      process_all_files(location.path)
 
-      AudioFile.find_each do |af|
+      location.audio_files.find_each do |af|
         update(warning_text: "#{warning_text}File #{af.full_path} doesn't exist anymore.\n") unless af.check_self
       end
     rescue StandardError => e
@@ -55,23 +55,21 @@ def run
     end
   end
 
-  private
-
-  def process_all_files(location, path)
+  def process_all_files(path)
     unless File.directory?(path)
       update(error_text: "#{error_text}#{path} (in #{location.path} is not a directory\n")
       return
     end
 
     Dir.each_child(path) do |child|
       if File.directory?(File.join(path, child))
-        process_all_files(location, File.join(path, child))
+        process_all_files(File.join(path, child))
       else
         Codec.all.find_each do |c|
           next unless File.extname(child)[1..]&.downcase == c.extension.downcase.to_s
 
           begin
-            process_file(location, c, File.join(path, child))
+            process_file(c, File.join(path, child))
             update(processed: processed + 1)
           rescue StandardError => e
             backtrace = Rails.env.production? ? e.backtrace.first(5).join("\n") : e.backtrace.join("\n")
@@ -82,7 +80,7 @@ def process_all_files(location, path)
     end
   end
 
-  def process_file(location, codec, path)
+  def process_file(codec, path)
     relative_path = Pathname.new(path).relative_path_from(Pathname.new(location.path))
 
     return if AudioFile.exists?(location: location, filename: relative_path.to_s)

app/policies/rescan_runner_policy.rb

@@ -1,9 +1,23 @@
 class RescanRunnerPolicy < ApplicationPolicy
+  class Scope < Scope
+    def resolve
+      scope.all if user&.moderator?
+    end
+  end
+
+  def index?
+    user&.moderator?
+  end
+
   def show?
     user&.moderator?
   end
 
   def start?
     user&.moderator?
   end
+
+  def start_all?
+    user&.moderator?
+  end
 end

app/serializers/rescan_runner_serializer.rb

@@ -8,7 +8,8 @@
 #  processed    :integer          default(0), not null
 #  running      :boolean          default(FALSE), not null
 #  warning_text :text
+#  location_id  :bigint           not null
 #
 class RescanRunnerSerializer < ActiveModel::Serializer
-  attributes :error_text, :warning_text, :processed, :running, :finished_at
+  attributes :id, :error_text, :warning_text, :processed, :running, :finished_at, :location_id
 end

config/routes.rb

@@ -65,7 +65,8 @@
 #                                 POST   /api/locations(.:format)                                                                          locations#create
 #                        location GET    /api/locations/:id(.:format)                                                                      locations#show
 #                                 DELETE /api/locations/:id(.:format)                                                                      locations#destroy
-#                           plays POST   /api/plays(.:format)                                                                              plays#create
+#                           plays GET    /api/plays(.:format)                                                                              plays#index
+#                                 POST   /api/plays(.:format)                                                                              plays#create
 #            destroy_empty_tracks POST   /api/tracks/destroy_empty(.:format)                                                               tracks#destroy_empty
 #                     audio_track GET    /api/tracks/:id/audio(.:format)                                                                   tracks#audio
 #                     merge_track POST   /api/tracks/:id/merge(.:format)                                                                   tracks#merge
@@ -81,8 +82,10 @@
 #                                 PATCH  /api/users/:id(.:format)                                                                          users#update
 #                                 PUT    /api/users/:id(.:format)                                                                          users#update
 #                                 DELETE /api/users/:id(.:format)                                                                          users#destroy
-#                          rescan GET    /api/rescan(.:format)                                                                             rescan#show
-#                                 POST   /api/rescan(.:format)                                                                             rescan#start
+#                         rescans GET    /api/rescans(.:format)                                                                            rescans#index
+#                          rescan GET    /api/rescans/:id(.:format)                                                                        rescans#show
+#                                 POST   /api/rescans/:id(.:format)                                                                        rescans#start
+#                                 POST   /api/rescans(.:format)                                                                            rescans#start_all
 #              rails_service_blob GET    /rails/active_storage/blobs/redirect/:signed_id/*filename(.:format)                               active_storage/blobs/redirect#show
 #        rails_service_blob_proxy GET    /rails/active_storage/blobs/proxy/:signed_id/*filename(.:format)                                  active_storage/blobs/proxy#show
 #                                 GET    /rails/active_storage/blobs/:signed_id/*filename(.:format)                                        active_storage/blobs/redirect#show
@@ -144,8 +147,8 @@
       end
     end
     resources :users
-
-    get '/rescan' => 'rescan#show'
-    post '/rescan' => 'rescan#start'
+    resources :rescans, only: %i[index show]
+    post 'rescans/:id', to: 'rescans#start'
+    post 'rescans', to: 'rescans#start_all'
   end
 end

db/migrate/20210905090539_add_location_to_rescan_runner.rb

@@ -0,0 +1,11 @@
+class AddLocationToRescanRunner < ActiveRecord::Migration[6.1]
+  def change
+    RescanRunner.destroy_all
+
+    add_reference :rescan_runners, :location, null: false, foreign_key: true
+
+    Location.find_each do |l|
+      RescanRunner.create(finished_at: Date.new, location: l)
+    end
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2021_01_04_190247) do
+ActiveRecord::Schema.define(version: 2021_09_05_090539) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -208,6 +208,8 @@
     t.integer "processed", default: 0, null: false
     t.boolean "running", default: false, null: false
     t.datetime "finished_at", default: -> { "CURRENT_TIMESTAMP" }, null: false
+    t.bigint "location_id", null: false
+    t.index ["location_id"], name: "index_rescan_runners_on_location_id"
   end
 
   create_table "track_artists", force: :cascade do |t|
@@ -276,6 +278,7 @@
   add_foreign_key "images", "image_types"
   add_foreign_key "plays", "tracks"
   add_foreign_key "plays", "users"
+  add_foreign_key "rescan_runners", "locations"
   add_foreign_key "track_artists", "artists"
   add_foreign_key "track_artists", "tracks"
   add_foreign_key "tracks", "albums"

db/seeds.rb

@@ -21,10 +21,10 @@
 
 if File.directory? File.expand_path('~/music')
   Location.create(path: File.expand_path('~/music'))
-  RescanRunner.create.run
 elsif File.directory? File.expand_path('~/Music')
   Location.create(path: File.expand_path('~/Music'))
-  RescanRunner.create.run
 end
 
+RescanRunner.schedule_all
+
 Delayed::Job.enqueue(TranscodeCacheCleanJob.new, cron: '0 4 * * *')

lib/tasks/rescan.rake

@@ -1,5 +1,5 @@
 namespace :rescan do
   task start: :environment do
-    RescanRunner.instance.schedule
+    RescanRunner.schedule_all
   end
 end

test/controllers/rescans_controller_test.rb

@@ -0,0 +1,60 @@
+require 'test_helper'
+
+class RescansControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    @runner = create(:rescan_runner)
+    @moderator = create(:moderator)
+    @user = create(:user)
+    sign_in_as(@user)
+  end
+
+  test 'should get not index for user' do
+    get rescans_url
+    assert_response :forbidden
+  end
+
+  test 'should get index for moderator' do
+    sign_in_as(@moderator)
+    get rescans_url
+    assert_response :success
+  end
+
+  test 'should get not show for user' do
+    get rescan_url(@runner)
+    assert_response :forbidden
+  end
+
+  test 'should get show for moderator' do
+    sign_in_as(@moderator)
+    get rescan_url(@runner)
+    assert_response :success
+  end
+
+  test 'should not start rescan for user' do
+    post rescan_url(@runner)
+    assert_response :forbidden
+  end
+
+  test 'should start rescan' do
+    sign_in_as(@moderator)
+    prev = @runner.finished_at
+    post rescan_url(@runner)
+    assert_response :success
+    @runner.reload
+    assert_not_equal prev, @runner.finished_at
+  end
+
+  test 'should not start all rescans for user' do
+    post rescans_url
+    assert_response :forbidden
+  end
+
+  test 'should start all rescans' do
+    sign_in_as(@moderator)
+    prev = @runner.finished_at
+    post rescans_url
+    assert_response :success
+    @runner.reload
+    assert_not_equal prev, @runner.finished_at
+  end
+end