Merge pull request #1072 from ecosyste-ms/remove-eager-load-dependencies-workers

Stop eager loading dependencies in dependency parsing workers

Author: andrew

app/sidekiq/parse_dependencies_worker.rb

@@ -3,6 +3,6 @@ class ParseDependenciesWorker
   sidekiq_options queue: 'dependencies', lock: :until_executed, lock_expiration: 1.day.to_i
 
   def perform(repository_id)
-    Repository.includes(manifests: :dependencies).find_by_id(repository_id).try(:parse_dependencies)
+    Repository.includes(:manifests).find_by_id(repository_id).try(:parse_dependencies)
   end
 end

app/sidekiq/parse_tag_dependencies_worker.rb

@@ -3,6 +3,6 @@ class ParseTagDependenciesWorker
   sidekiq_options queue: 'dependencies', lock: :until_executed, lock_expiration: 1.day.to_i
 
   def perform(tag_id)
-    Tag.includes(manifests: :dependencies).find_by_id(tag_id).try(:parse_dependencies)
+    Tag.includes(:manifests).find_by_id(tag_id).try(:parse_dependencies)
   end
 end

test/workers/parse_dependencies_worker_test.rb

@@ -0,0 +1,46 @@
+require 'test_helper'
+
+class ParseDependenciesWorkerTest < ActiveSupport::TestCase
+  context '#perform' do
+    should 'call parse_dependencies on repository' do
+      host = create(:host)
+      repository = create(:repository, host: host)
+
+      Repository.any_instance.expects(:parse_dependencies).once
+
+      worker = ParseDependenciesWorker.new
+      worker.perform(repository.id)
+    end
+
+    should 'handle non-existent repository gracefully' do
+      worker = ParseDependenciesWorker.new
+
+      assert_nothing_raised do
+        worker.perform(999999)
+      end
+    end
+
+    should 'eager load manifests but not dependencies' do
+      host = create(:host)
+      repository = create(:repository, host: host)
+      manifest = create(:manifest, repository: repository)
+      create(:dependency, repository: repository, manifest: manifest)
+
+      Repository.any_instance.stubs(:parse_dependencies)
+
+      queries = []
+      callback = lambda { |_name, _start, _finish, _id, payload|
+        queries << payload[:sql] if payload[:sql].present?
+      }
+
+      ActiveSupport::Notifications.subscribed(callback, "sql.active_record") do
+        ParseDependenciesWorker.new.perform(repository.id)
+      end
+
+      manifest_queries = queries.select { |q| q.include?('manifests') }
+      dependency_queries = queries.select { |q| q.include?('dependencies') }
+      assert manifest_queries.any?, "Should eager load manifests"
+      assert_empty dependency_queries, "Should not eager load dependencies"
+    end
+  end
+end

test/workers/parse_tag_dependencies_worker_test.rb

@@ -0,0 +1,47 @@
+require 'test_helper'
+
+class ParseTagDependenciesWorkerTest < ActiveSupport::TestCase
+  context '#perform' do
+    should 'call parse_dependencies on tag' do
+      host = create(:host)
+      repository = create(:repository, host: host)
+      tag = create(:tag, repository: repository)
+
+      Tag.any_instance.expects(:parse_dependencies).once
+
+      worker = ParseTagDependenciesWorker.new
+      worker.perform(tag.id)
+    end
+
+    should 'handle non-existent tag gracefully' do
+      worker = ParseTagDependenciesWorker.new
+
+      assert_nothing_raised do
+        worker.perform(999999)
+      end
+    end
+
+    should 'eager load manifests but not dependencies' do
+      host = create(:host)
+      repository = create(:repository, host: host)
+      tag = create(:tag, repository: repository)
+      create(:manifest, tag: tag, repository: nil)
+
+      Tag.any_instance.stubs(:parse_dependencies)
+
+      queries = []
+      callback = lambda { |_name, _start, _finish, _id, payload|
+        queries << payload[:sql] if payload[:sql].present?
+      }
+
+      ActiveSupport::Notifications.subscribed(callback, "sql.active_record") do
+        ParseTagDependenciesWorker.new.perform(tag.id)
+      end
+
+      manifest_queries = queries.select { |q| q.include?('manifests') }
+      dependency_queries = queries.select { |q| q.include?('dependencies') }
+      assert manifest_queries.any?, "Should eager load manifests"
+      assert_empty dependency_queries, "Should not eager load dependencies"
+    end
+  end
+end