Lock des sites dans les tâches de fond (#1949)

* sync with git jobs use execute instead of perform

* lock

* no skip

* fix

* 30s

* schema

* fix

* fix

app/errors/communication/website/lock_error.rb

+class Communication::Website::LockError < StandardError
+end
\ No newline at end of file

app/jobs/communication/website/base_job.rb

@@ -9,15 +9,27 @@ class Communication::Website::BaseJob < ApplicationJob
 
   # Retry the job after 1 minute if it is interrupted, to prevent queue from being blocked
   retry_on GoodJob::InterruptError, wait: 1.minute, attempts: Float::INFINITY
+  # Retry the job after 30 seconds when the website was locked.
+  retry_on Communication::Website::LockError, wait: 30.seconds, attempts: Float::INFINITY
 
-  attr_accessor :website_id
+  attr_accessor :website_id, :options
 
-  def perform(website_id)
+  def perform(website_id, options = {})
     @website_id = website_id
+    @options = options
     # Website might be deleted in between
     return unless website.present?
-    # TODO manage lock / unlock
-    execute
+    # Raise if website is locked to retry later
+    raise Communication::Website::LockError.new("Interrupted because of website lock.") if website.locked_for_background_jobs?
+    # We lock the website to prevent race conditions
+    website.lock_for_background_jobs!
+    begin
+      # We execute the job
+      execute
+    ensure
+      # We make sure to unlock the website to allow the other jobs to run
+      website.unlock_for_background_jobs!
+    end
   end
 
   protected

app/jobs/communication/website/direct_object/sync_with_git_job.rb

 class Communication::Website::DirectObject::SyncWithGitJob < Communication::Website::BaseJob
-  def perform(direct_object)
-    direct_object.sync_with_git_safely
+  def execute
+    direct_object = options.fetch(:direct_object)
+    direct_object.sync_with_git_safely if direct_object.present?
   end
 end
\ No newline at end of file

app/jobs/communication/website/indirect_object/sync_with_git_job.rb

 class Communication::Website::IndirectObject::SyncWithGitJob < Communication::Website::BaseJob
-  def perform(website, indirect_object)
-    website.sync_indirect_object_with_git(indirect_object)
+  def execute
+    indirect_object = options.fetch(:indirect_object)
+    website.sync_indirect_object_with_git(indirect_object) if indirect_object.present?
   end
 end
\ No newline at end of file

app/models/concerns/as_indirect_object.rb

@@ -74,7 +74,7 @@ module AsIndirectObject
       direct_source.website.connect self, direct_source
     end
     websites.each do |website|
-      Communication::Website::IndirectObject::SyncWithGitJob.perform_later(website, self)
+      Communication::Website::IndirectObject::SyncWithGitJob.perform_later(website.id, indirect_object: self)
     end
   end
 

app/models/concerns/with_git.rb

@@ -24,7 +24,7 @@ module WithGit
   end
 
   def sync_with_git
-    Communication::Website::DirectObject::SyncWithGitJob.perform_later(self)
+    Communication::Website::DirectObject::SyncWithGitJob.perform_later(website.id, direct_object: self)
   end
 
   def sync_with_git_safely

app/models/research/hal.rb

@@ -27,15 +27,12 @@ module Research::Hal
 
   def self.clear_queue!
     ids = []
-    GoodJob::Job.find_each do |job|
-      job_class = job.job_class
+    indirect_object_sync_job_class = "Communication::Website::IndirectObject::SyncWithGitJob"
+    GoodJob::Job.queued.where(job_class: indirect_object_sync_job_class).find_each do |job|
       job_arguments = job.serialized_params["arguments"]
-      indirect_object_gid = job_arguments.last["_aj_globalid"].to_s
-
-      if job_class == "Communication::Website::IndirectObject::SyncWithGitJob" &&
-         indirect_object_gid.include?("Research::Publication")
-        ids << job.id
-      end
+      job_options_argument = job_arguments.last
+      indirect_object_gid = job_options_argument.dig("indirect_object", "_aj_globalid").to_s
+      ids << job.id if indirect_object_gid.include?("Research::Publication")
     end
     GoodJob::Job.where(id: ids).destroy_all
   end

test/models/communication/website/dependency_test.rb

@@ -50,7 +50,7 @@ class Communication::Website::DependencyTest < ActiveSupport::TestCase
     # - une tâche de nettoyage des git files (dépendances du bloc supprimé)
     GoodJob::Job.destroy_all
 
-    assert_enqueued_with(job: Communication::Website::DirectObject::SyncWithGitJob, args: [page]) do
+    assert_enqueued_with(job: Communication::Website::DirectObject::SyncWithGitJob, args: [page.communication_website_id, direct_object: page]) do
       assert_enqueued_with(job: Communication::Website::CleanJob) do
         block.destroy
       end