Merge pull request rails#54142 from rails/rm-fix-precompile-missconfigured-active-storage

Don't load ActiveStorage::Blob when validating the service

Author: rafaelfranca

activestorage/app/models/active_storage/blob.rb

@@ -151,22 +151,6 @@ def compose(blobs, key: nil, filename:, content_type: nil, metadata: nil)
         combined_blob.save!
       end
     end
-
-    def validate_service_configuration(service_name, model_class, association_name) # :nodoc:
-      if service_name
-        services.fetch(service_name) do
-          raise ArgumentError, "Cannot configure service #{service_name.inspect} for #{model_class}##{association_name}"
-        end
-      else
-        validate_global_service_configuration
-      end
-    end
-
-    def validate_global_service_configuration # :nodoc:
-      if connected? && table_exists? && Rails.configuration.active_storage.service.nil?
-        raise RuntimeError, "Missing Active Storage service name. Specify Active Storage service name for config.active_storage.service in config/environments/#{Rails.env}.rb"
-      end
-    end
   end
 
   include Analyzable

activestorage/lib/active_storage/attached/changes/create_one.rb

@@ -121,7 +121,7 @@ def attachment_service_name
         service_name = record.attachment_reflections[name].options[:service_name]
         if service_name.is_a?(Proc)
           service_name = service_name.call(record)
-          ActiveStorage::Blob.validate_service_configuration(service_name, record.class, name)
+          Attached::Model.validate_service_configuration(service_name, record.class, name)
         end
         service_name
       end

activestorage/lib/active_storage/attached/model.rb

@@ -106,7 +106,7 @@ module Attached::Model
       # <tt>active_storage_attachments.record_type</tt> polymorphic type column of
       # the corresponding rows.
       def has_one_attached(name, dependent: :purge_later, service: nil, strict_loading: false)
-        ActiveStorage::Blob.validate_service_configuration(service, self, name) unless service.is_a?(Proc)
+        Attached::Model.validate_service_configuration(service, self, name) unless service.is_a?(Proc)
 
         generated_association_methods.class_eval <<-CODE, __FILE__, __LINE__ + 1
           # frozen_string_literal: true
@@ -208,7 +208,7 @@ def #{name}=(attachable)
       # <tt>active_storage_attachments.record_type</tt> polymorphic type column of
       # the corresponding rows.
       def has_many_attached(name, dependent: :purge_later, service: nil, strict_loading: false)
-        ActiveStorage::Blob.validate_service_configuration(service, self, name) unless service.is_a?(Proc)
+        Attached::Model.validate_service_configuration(service, self, name) unless service.is_a?(Proc)
 
         generated_association_methods.class_eval <<-CODE, __FILE__, __LINE__ + 1
           # frozen_string_literal: true
@@ -259,6 +259,25 @@ def #{name}=(attachables)
       end
     end
 
+    class << self
+      def validate_service_configuration(service_name, model_class, association_name) # :nodoc:
+        if service_name
+          ActiveStorage::Blob.services.fetch(service_name) do
+            raise ArgumentError, "Cannot configure service #{service_name.inspect} for #{model_class}##{association_name}"
+          end
+        else
+          validate_global_service_configuration(model_class)
+        end
+      end
+
+      private
+        def validate_global_service_configuration(model_class)
+          if model_class.connected? && ActiveStorage::Blob.table_exists? && Rails.configuration.active_storage.service.nil?
+            raise RuntimeError, "Missing Active Storage service name. Specify Active Storage service name for config.active_storage.service in config/environments/#{Rails.env}.rb"
+          end
+        end
+    end
+
     def attachment_changes # :nodoc:
       @attachment_changes ||= {}
     end

railties/test/application/active_storage/validating_service_test.rb

@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require "isolation/abstract_unit"
+require "rack/test"
+require "rails-dom-testing"
+
+module ApplicationTests
+  class ValidatingServiceTest < ActiveSupport::TestCase
+    include ActiveSupport::Testing::Isolation
+    include Rack::Test::Methods
+    include Rails::Dom::Testing::Assertions
+
+    self.file_fixture_path = "#{RAILS_FRAMEWORK_ROOT}/activestorage/test/fixtures/files"
+
+    def setup
+      build_app
+    end
+
+    def teardown
+      teardown_app
+    end
+
+    def test_boot_application_with_model_using_active_storage_should_not_load_active_storage_blob
+      rails "active_storage:install"
+
+      rails "generate", "model", "user", "name:string", "avatar:attachment"
+      rails "db:migrate"
+
+      app_file "config/routes.rb", <<~RUBY
+        Rails.application.routes.draw do
+          _ = User
+          resources :users, only: [:show, :create]
+          Rails.configuration.ok_to_proceed = true
+        end
+      RUBY
+
+      app_file "config/initializers/active_storage.rb", <<~RUBY
+        Rails.configuration.ok_to_proceed = false
+
+        ActiveSupport.on_load(:active_storage_blob) do
+          raise "ActiveStorage::Blob was loaded" unless Rails.configuration.ok_to_proceed
+        end
+      RUBY
+
+      assert_nothing_raised do
+        with_env "RAILS_ENV" => "production" do
+          rails ["environment", "--trace"]
+        end
+      end
+    end
+  end
+end

railties/test/application/assets_test.rb

@@ -26,13 +26,6 @@ def precompile!(env = nil)
       end
     end
 
-    def with_env(env)
-      env.each { |k, v| ENV[k.to_s] = v }
-      yield
-    ensure
-      env.each_key { |k| ENV.delete k.to_s }
-    end
-
     def clean_assets!
       quietly do
         rails ["assets:clobber"]

railties/test/application/sprockets_assets_test.rb

@@ -56,13 +56,6 @@ def run_app_update
       end
     end
 
-    def with_env(env)
-      env.each { |k, v| ENV[k.to_s] = v }
-      yield
-    ensure
-      env.each_key { |k| ENV.delete k.to_s }
-    end
-
     def clean_assets!
       quietly do
         rails ["assets:clobber"]

railties/test/isolation/abstract_unit.rb

@@ -595,6 +595,14 @@ class ActiveSupport::TestCase
   include TestHelpers::Reload
   include ActiveSupport::Testing::Stream
   include ActiveSupport::Testing::MethodCallAssertions
+
+  private
+    def with_env(env)
+      env.each { |k, v| ENV[k.to_s] = v }
+      yield
+    ensure
+      env.each_key { |k| ENV.delete k.to_s }
+    end
 end
 
 # Create a scope and build a fixture rails app