Clean up framework deferred-loading logic to use nested on_load blo…

…cks; defer loading development gem `activerecord-explain-analyze` (#931)

.ruby-version

@@ -1 +1 @@
-3.2.1
+3.2.2

Gemfile

@@ -23,7 +23,7 @@ gem 'pg', platforms: [:mri, :mingw, :x64_mingw]
 gem 'rails'
 
 platforms :ruby do
-  gem "activerecord-explain-analyze"
+  gem "activerecord-explain-analyze", require: false
   gem "pry-byebug"
   gem 'rack-mini-profiler'
   gem "rbtrace"

Gemfile.lock

@@ -466,4 +466,4 @@ DEPENDENCIES
   yard-activesupport-concern
 
 BUNDLED WITH
-   2.4.7
+   2.4.12

lib/good_job/dependencies.rb

@@ -6,40 +6,19 @@ module Dependencies
     extend ActiveSupport::Concern
 
     included do
-      # @!attribute [rw] _rails_after_initialize_hook_called
-      #   @!scope class
-      #   Whether Railtie.after_initialize has been called yet (default: +false+).
-      #   This will be set on  but before +Rails.application.initialize?+ is +true+.
-      #   @return [Boolean]
-      mattr_accessor :_rails_after_initialize_hook_called, default: false
-
-      # @!attribute [rw] _active_job_loaded
-      #   @!scope class
-      #   Whether ActiveJob has loaded (default: +false+).
-      #   @return [Boolean]
-      mattr_accessor :_active_job_loaded, default: false
-
-      # @!attribute [rw] _active_record_loaded
-      #   @!scope class
-      #   Whether ActiveRecord has loaded (default: +false+).
-      #   @return [Boolean]
-      mattr_accessor :_active_record_loaded, default: false
+      mattr_accessor :_framework_ready, default: false
     end
 
     class_methods do
-      # Whether GoodJob's  has been initialized as of the calling of +Railtie.after_initialize+.
-      # @return [Boolean]
+      # Whether Rails framework has sufficiently initialized to enable Async execution.
       def async_ready?
-        Rails.application.initialized? || (
-           _rails_after_initialize_hook_called &&
-           _active_job_loaded &&
-           _active_record_loaded
-         )
+        Rails.application.initialized? || _framework_ready
       end
 
-      def start_async_adapters
+      def _start_async_adapters
         return unless async_ready?
 
+        ActiveJob::Base.queue_adapter # Ensure Active Job is initialized
         GoodJob::Adapter.instances
                         .select(&:execute_async?)
                         .reject(&:async_started?)

lib/good_job/engine.rb

@@ -37,26 +37,21 @@ class Engine < ::Rails::Engine
 
     initializer "good_job.start_async" do
       # This hooks into the hookable places during Rails boot, which is unfortunately not Rails.application.initialized?
-      # If an Adapter is initialized during boot, we want to want to start its async executors once the framework dependencies have loaded.
+      # If an Adapter is initialized during boot, we want to want to start async executors once the framework dependencies have loaded.
       # When exactly that happens is out of our control because gems or application code may touch things earlier than expected.
       # For example, as of Rails 6.1, if an ActiveRecord model is touched during boot, that triggers ActiveRecord to load,
       # which touches DestroyAssociationAsyncJob, which loads ActiveJob, which may initialize a GoodJob::Adapter, all of which
       # happens _before_ ActiveRecord finishes loading. GoodJob will deadlock if an async executor is started in the middle of
       # ActiveRecord loading.
-
       config.after_initialize do
         ActiveSupport.on_load(:active_record) do
-          GoodJob._active_record_loaded = true
-          GoodJob.start_async_adapters
-        end
-
-        ActiveSupport.on_load(:active_job) do
-          GoodJob._active_job_loaded = true
-          GoodJob.start_async_adapters
+          ActiveSupport.on_load(:active_job) do
+            GoodJob._framework_ready = true
+            GoodJob._start_async_adapters
+          end
+          GoodJob._start_async_adapters
         end
-
-        GoodJob._rails_after_initialize_hook_called = true
-        GoodJob.start_async_adapters
+        GoodJob._start_async_adapters
       end
     end
   end

spec/integration/server_spec.rb

@@ -16,7 +16,7 @@
       }
 
       ShellOut.command("bundle exec rails s -p #{port} -P #{pidfile}", env: env) do |shell|
-        wait_until(max: 30) do
+        wait_until(max: 30, increments_of: 0.5) do
           expect(shell.output).to include(/Listening on/)
           # In development, GoodJob starts up before Puma redirects logs to stdout
 
@@ -42,7 +42,7 @@
         "GOOD_JOB_ENABLE_CRON" => "true",
       }
       ShellOut.command("bundle exec rails s -p #{port} -P #{pidfile}", env: env) do |shell|
-        wait_until(max: 30) do
+        wait_until(max: 30, increments_of: 0.5) do
           expect(shell.output).to include(/Listening on/)
           expect(shell.output).to include(/GoodJob [0-9.]+ started scheduler/)
           expect(shell.output).to include(/GoodJob started cron/)
@@ -56,7 +56,7 @@
         "GOOD_JOB_EXECUTION_MODE" => "async",
       }
       ShellOut.command('bundle exec rails db:version', env: env) do |shell|
-        wait_until(max: 30) do
+        wait_until(max: 30, increments_of: 0.5) do
           expect(shell.output).to include(/Current version/)
         end
         expect(shell.output).not_to include(/GoodJob [0-9.]+ started scheduler/)

spec/test_app/config/application.rb

@@ -17,6 +17,8 @@ class Application < Rails::Application
     # the framework and any gems in your application.
     #
 
+    config.active_job.queue_adapter = :good_job
+
     # config.middleware.insert_before Rack::Sendfile, ActionDispatch::DebugLocks
     config.log_level = :debug
 

spec/test_app/config/environments/test.rb

@@ -7,6 +7,8 @@
   config.cache_classes = true
   config.eager_load = true
 
+  config.active_job.queue_adapter = :test
+
   # Raises error for missing translations.
   if Gem::Version.new(Rails.version) < Gem::Version.new('6.1')
     config.action_view.raise_on_missing_translations = true

spec/test_app/config/initializers/activerecord_explain_analyze.rb

@@ -1,18 +1,25 @@
 # frozen_string_literal: true
-return unless defined? ActiveRecordExplainAnalyze
 
-module ActiveRecordExplainAnalyze
-  module Relation
-    alias original_explain explain
+ActiveSupport.on_load(:active_record) do
+  begin
+    require 'activerecord-explain-analyze'
+  rescue LoadError
+    next # ignore
+  end
+
+  module ActiveRecordExplainAnalyze
+    module Relation
+      alias original_explain explain
 
-    def explain(analyze: false, format: :text, indexscan: false)
-      if indexscan
-        ActiveRecord::Base.connection.execute('SET enable_seqscan = OFF')
-        result = original_explain(analyze: analyze, format: format)
-        ActiveRecord::Base.connection.execute('SET enable_seqscan = ON')
-        result
-      else
-        original_explain(analyze: analyze, format: format)
+      def explain(analyze: false, format: :text, indexscan: false)
+        if indexscan
+          ActiveRecord::Base.connection.execute('SET enable_seqscan = OFF')
+          result = original_explain(analyze: analyze, format: format)
+          ActiveRecord::Base.connection.execute('SET enable_seqscan = ON')
+          result
+        else
+          original_explain(analyze: analyze, format: format)
+        end
       end
     end
   end

spec/test_app/config/initializers/good_job.rb

@@ -13,7 +13,6 @@
 
 case Rails.env
 when 'development'
-  ActiveJob::Base.queue_adapter = :good_job
   GoodJob.on_thread_error = -> (error) { Rails.logger.warn("#{error}\n#{error.backtrace}") }
 
   Rails.application.configure do
@@ -44,8 +43,6 @@
 when 'test'
   # test
 when 'demo'
-  ActiveJob::Base.queue_adapter = :good_job
-
   Rails.application.configure do
     config.good_job.execution_mode = :async
     config.good_job.poll_interval = 30
@@ -81,5 +78,7 @@
     }
   end
 when 'production'
-  ActiveJob::Base.queue_adapter = :good_job
+  # production
+else
+  raise "Unconfigured environment for GoodJob"
 end