More code changes, thanks Robert

2 files changed, 24 insertions, 20 deletions

diff --git a/app/workers/repository_check_worker.rb b/app/workers/repository_check_worker.rb
index bdacdd4c6b0..d7ead91f94e 100644
--- a/app/workers/repository_check_worker.rb
+++ b/app/workers/repository_check_worker.rb
@@ -25,9 +25,11 @@ class RepositoryCheckWorker
 
   private
 
-  # In an ideal world we would use Project.where(...).find_each.
-  # Unfortunately, calling 'find_each' drops the 'where', so we must build
-  # an array of IDs instead.
+  # Project.find_each does not support WHERE clauses and
+  # Project.find_in_batches does not support ordering. So we just build an
+  # array of ID's. This is OK because we do it only once an hour, because
+  # getting ID's from Postgres is not terribly slow, and because no user
+  # has to sit and wait for this query to finish.
   def project_ids
     limit = 10_000
     never_checked_projects = Project.where('last_repository_check_at IS NULL').limit(limit).
@@ -40,17 +42,20 @@ class RepositoryCheckWorker
   def try_obtain_lease(id)
     # Use a 24-hour timeout because on servers/projects where 'git fsck' is
     # super slow we definitely do not want to run it twice in parallel.
-    lease = Gitlab::ExclusiveLease.new(
+    Gitlab::ExclusiveLease.new(
       "project_repository_check:#{id}",
       timeout: 24.hours
-    )
-    lease.try_obtain
+    ).try_obtain
   end
 
   def current_settings
     # No caching of the settings! If we cache them and an admin disables
     # this feature, an active RepositoryCheckWorker would keep going for up
     # to 1 hour after the feature was disabled.
-    ApplicationSetting.current || Gitlab::CurrentSettings.fake_application_settings
+    if Rails.env.test?
+      Gitlab::CurrentSettings.fake_application_settings
+    else
+      ApplicationSetting.current
+    end
   end
 end
diff --git a/app/workers/single_repository_check_worker.rb b/app/workers/single_repository_check_worker.rb
index d9eed9bd708..6257f382d86 100644
--- a/app/workers/single_repository_check_worker.rb
+++ b/app/workers/single_repository_check_worker.rb
@@ -5,30 +5,29 @@ class SingleRepositoryCheckWorker
 
   def perform(project_id)
     project = Project.find(project_id)
-    update(project, success: check(project))
+    project.update_columns(
+      last_repository_check_failed: !check(project),
+      last_repository_check_at: Time.now,
+    )
   end
 
   private
 
   def check(project)
-    [project.repository.path_to_repo, project.wiki.wiki.path].all? do |path|
-      git_fsck(path)
+    [project.repository, project.wiki.repository].all? do |repository|
+      git_fsck(repository.path_to_repo)
     end
   end
 
   def git_fsck(path)
     cmd = %W(nice git --git-dir=#{path} fsck)
     output, status = Gitlab::Popen.popen(cmd)
-    return true if status.zero?
-
-    Gitlab::RepositoryCheckLogger.error("command failed: #{cmd.join(' ')}\n#{output}")
-    false
-  end
 
-  def update(project, success:)
-    project.update_columns(
-      last_repository_check_failed: !success,
-      last_repository_check_at: Time.now,
-    )
+    if status.zero?
+      true
+    else
+      Gitlab::RepositoryCheckLogger.error("command failed: #{cmd.join(' ')}\n#{output}")
+      false
+    end
   end
 end