Merge pull request #1352 from bf4/railties

Fix generators (@dgynn); load Railtie only with Rails, ensures caching configured

CHANGELOG.md

@@ -58,6 +58,7 @@ Features:
 
 Fixes:
 
+- [#1352](https://github.com/rails-api/active_model_serializers/pull/1352) Fix generators; Isolate Rails-specifc code in Railties (@dgynn, @bf4)
 - [#1384](https://github.com/rails-api/active_model_serializers/pull/1384)Fix database state leaking across tests (@bf4)
 - [#1297](https://github.com/rails-api/active_model_serializers/pull/1297) Fix `fields` option to restrict relationships as well (@beauby)
 - [#1239](https://github.com/rails-api/active_model_serializers/pull/1239) Fix duplicates in JSON API compound documents (@beauby)

Rakefile

@@ -44,7 +44,32 @@ Rake::TestTask.new do |t|
   t.verbose = true
 end
 
-task default: [:test, :rubocop]
+desc 'Run isolated tests'
+task isolated: ['test:isolated:railtie']
+namespace :test do
+  namespace :isolated do
+    desc 'Run isolated tests for Railtie'
+    task :railtie do
+      dir = File.dirname(__FILE__)
+      file = "#{dir}/test/active_model_serializers/railtie_test_isolated.rb"
+
+      # https://github.com/rails/rails/blob/3d590add45/railties/lib/rails/generators/app_base.rb#L345-L363
+      _bundle_command = Gem.bin_path('bundler', 'bundle')
+      require 'bundler'
+      Bundler.with_clean_env do
+        command = "-w -I#{dir}/lib -I#{dir}/test #{file}"
+        full_command = %("#{Gem.ruby}" #{command})
+        system(full_command) or fail 'Failures' # rubocop:disable Style/AndOr
+      end
+    end
+  end
+end
+
+if ENV['RAILS_VERSION'].to_s > '4.0' && RUBY_ENGINE == 'ruby'
+  task default: [:isolated, :test, :rubocop]
+else
+  task default: [:test, :rubocop]
+end
 
 desc 'CI test task'
 task :ci => [:default]

docs/general/configuration_options.md

@@ -24,4 +24,4 @@ preferably inside an initializer.
 
 ## Hooks
 
-To run a hook when ActiveModelSerializers is loaded, use `ActiveSupport.on_load(:active_model_serializers) do end`
+To run a hook when ActiveModelSerializers is loaded, use `ActiveSupport.on_load(:action_controller) do end`

lib/active_model_serializers.rb

@@ -1,33 +1,23 @@
 require 'active_model'
 require 'active_support'
-require 'action_controller'
-require 'action_controller/railtie'
+require 'active_support/core_ext/object/with_options'
 module ActiveModelSerializers
-  mattr_accessor(:logger) { ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new(STDOUT)) }
-
-  def self.config
-    ActiveModel::Serializer.config
-  end
-
   extend ActiveSupport::Autoload
   autoload :Model
   autoload :Callbacks
   autoload :Deserialization
   autoload :Logging
   autoload :Test
-end
 
-require 'active_model/serializer'
-require 'active_model/serializable_resource'
-require 'active_model/serializer/version'
+  class << self; attr_accessor :logger; end
+  self.logger = ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new(STDOUT))
 
-require 'action_controller/serialization'
-ActiveSupport.on_load(:action_controller) do
-  ActiveSupport.run_load_hooks(:active_model_serializers, ActiveModelSerializers)
-  include ::ActionController::Serialization
-  ActionDispatch::Reloader.to_prepare do
-    ActiveModel::Serializer.serializers_cache.clear
+  def self.config
+    ActiveModel::Serializer.config
   end
-end
 
-require 'active_model/serializer/railtie'
+  require 'active_model/serializer/version'
+  require 'active_model/serializer'
+  require 'active_model/serializable_resource'
+  require 'active_model_serializers/railtie' if defined?(::Rails)
+end

lib/active_model_serializers/railtie.rb

@@ -0,0 +1,36 @@
+require 'rails/railtie'
+require 'action_controller'
+require 'action_controller/railtie'
+require 'action_controller/serialization'
+
+module ActiveModelSerializers
+  class Railtie < Rails::Railtie
+    config.to_prepare do
+      ActiveModel::Serializer.serializers_cache.clear
+    end
+
+    initializer 'active_model_serializers.action_controller' do
+      ActiveSupport.on_load(:action_controller) do
+        include(::ActionController::Serialization)
+      end
+    end
+
+    # This hook is run after the action_controller railtie has set the configuration
+    # based on the *environment* configuration and before any config/initializers are run
+    # and also before eager_loading (if enabled).
+    initializer 'active_model_serializers.set_configs', :after => 'action_controller.set_configs' do
+      ActiveModelSerializers.logger = Rails.configuration.action_controller.logger
+      ActiveModelSerializers.config.cache_store     = Rails.configuration.action_controller.cache_store
+      ActiveModelSerializers.config.perform_caching = Rails.configuration.action_controller.perform_caching
+    end
+
+    generators do
+      require 'generators/rails/resource_override'
+    end
+
+    if Rails.env.test?
+      ActionController::TestCase.send(:include, ActiveModelSerializers::Test::Schema)
+      ActionController::TestCase.send(:include, ActiveModelSerializers::Test::Serializer)
+    end
+  end
+end

lib/generators/serializer/resource_override.rb → lib/generators/rails/resource_override.rb

@@ -4,9 +4,7 @@
 module Rails
   module Generators
     class ResourceGenerator
-      def add_serializer
-        invoke 'serializer'
-      end
+      hook_for :serializer, default: true, boolean: true
     end
   end
 end

lib/generators/serializer/serializer_generator.rb → lib/generators/rails/serializer_generator.rb

@@ -15,11 +15,11 @@ def create_serializer_file
       private
 
       def attributes_names
-        [:id] + attributes.select { |attr| !attr.reference? }.map { |a| a.name.to_sym }
+        [:id] + attributes.reject(&:reference?).map! { |a| a.name.to_sym }
       end
 
       def association_names
-        attributes.select { |attr| attr.reference? }.map { |a| a.name.to_sym }
+        attributes.select(&:reference?).map! { |a| a.name.to_sym }
       end
 
       def parent_class_name
@@ -34,4 +34,3 @@ def parent_class_name
     end
   end
 end
-

test/active_model_serializers/railtie_test_isolated.rb

@@ -0,0 +1,57 @@
+# Execute this test in isolation
+require 'support/isolated_unit'
+
+class RailtieTest < ActiveSupport::TestCase
+  include ActiveSupport::Testing::Isolation
+
+  class WithRails < RailtieTest
+    setup do
+      require 'rails'
+      require 'active_model_serializers'
+      make_basic_app
+    end
+
+    test 'mixes ActionController::Serialization into ActionController::Base' do
+      assert ActionController.const_defined?(:Serialization),
+        "ActionController::Serialization should be defined, but isn't"
+      assert ::ActionController::Base.included_modules.include?(::ActionController::Serialization),
+        "ActionController::Serialization should be included in ActionController::Base, but isn't"
+    end
+
+    test 'sets the ActiveModelSerializers.logger to Rails.logger' do
+      refute_nil Rails.logger
+      refute_nil ActiveModelSerializers.logger
+      assert_equal Rails.logger, ActiveModelSerializers.logger
+    end
+
+    test 'it is configured for caching' do
+      assert_equal ActionController::Base.cache_store, ActiveModelSerializers.config.cache_store
+      assert_equal Rails.configuration.action_controller.perform_caching, ActiveModelSerializers.config.perform_caching
+    end
+  end
+
+  class WithoutRails < RailtieTest
+    setup do
+      require 'active_model_serializers'
+      make_basic_app
+    end
+
+    test 'does not mix ActionController::Serialization into ActionController::Base' do
+      refute ActionController.const_defined?(:Serialization),
+        'ActionController::Serialization should not be defined, but is'
+    end
+
+    test 'has its own logger at ActiveModelSerializers.logger' do
+      refute_nil Rails.logger
+      refute_nil ActiveModelSerializers.logger
+      refute_equal Rails.logger, ActiveModelSerializers.logger
+    end
+
+    test 'it is not configured for caching' do
+      refute_nil ActionController::Base.cache_store
+      assert_nil ActiveModelSerializers.config.cache_store
+      refute Rails.configuration.action_controller.perform_caching
+      refute ActiveModelSerializers.config.perform_caching
+    end
+  end
+end

test/generators/scaffold_controller_generator_test.rb

@@ -1,4 +1,5 @@
 require 'test_helper'
+require 'generators/rails/resource_override'
 
 class ResourceGeneratorTest < Rails::Generators::TestCase
   destination File.expand_path('../../../tmp/generators', __FILE__)

test/generators/serializer_generator_test.rb

@@ -1,5 +1,6 @@
 require 'test_helper'
-require 'generators/serializer/serializer_generator'
+require 'generators/rails/resource_override'
+require 'generators/rails/serializer_generator'
 
 class SerializerGeneratorTest < Rails::Generators::TestCase
   destination File.expand_path('../../../tmp/generators', __FILE__)

test/support/isolated_unit.rb

@@ -0,0 +1,77 @@
+# https://github.com/rails/rails/blob/v5.0.0.beta1/railties/test/isolation/abstract_unit.rb
+
+# Usage Example:
+#
+# require 'support/isolated_unit'
+#
+# class RailtieTest < ActiveSupport::TestCase
+#   include ActiveSupport::Testing::Isolation
+#
+#   class WithRailsDefinedOnLoad < RailtieTest
+#     setup do
+#       require 'rails'
+#       require 'active_model_serializers'
+#       make_basic_app
+#     end
+#
+#     # some tests
+#   end
+#
+#   class WithoutRailsDefinedOnLoad < RailtieTest
+#     setup do
+#       require 'active_model_serializers'
+#       make_basic_app
+#     end
+#
+#     # some tests
+#   end
+# end
+#
+# Note:
+# It is important to keep this file as light as possible
+# the goal for tests that require this is to test booting up
+# rails from an empty state, so anything added here could
+# hide potential failures
+#
+# It is also good to know what is the bare minimum to get
+# Rails booted up.
+require 'bundler/setup' unless defined?(Bundler)
+require 'active_support'
+require 'active_support/core_ext/string/access'
+
+# These files do not require any others and are needed
+# to run the tests
+require 'active_support/testing/autorun'
+require 'active_support/testing/isolation'
+
+module TestHelpers
+  module Generation
+    # Make a very basic app, without creating the whole directory structure.
+    # Is faster and simpler than generating a Rails app in a temp directory
+    def make_basic_app
+      require 'rails'
+      require 'action_controller/railtie'
+
+      @app = Class.new(Rails::Application) do
+        config.eager_load = false
+        config.session_store :cookie_store, key: '_myapp_session'
+        config.active_support.deprecation = :log
+        config.active_support.test_order = :parallel
+        ActiveSupport::TestCase.respond_to?(:test_order=) && ActiveSupport::TestCase.test_order = :parallel
+        config.root = File.dirname(__FILE__)
+        config.log_level = :info
+        # Set a fake logger to avoid creating the log directory automatically
+        fake_logger = Logger.new(nil)
+        config.logger = fake_logger
+      end
+      @app.respond_to?(:secrets) && @app.secrets.secret_key_base = '3b7cd727ee24e8444053437c36cc66c4'
+
+      yield @app if block_given?
+      @app.initialize!
+    end
+  end
+end
+
+class ActiveSupport::TestCase
+  include TestHelpers::Generation
+end

test/test_helper.rb

@@ -17,6 +17,7 @@
 require 'action_controller/test_case'
 require 'action_controller/railtie'
 require 'active_support/json'
+require 'active_model_serializers'
 require 'fileutils'
 FileUtils.mkdir_p(File.expand_path('../../tmp/cache', __FILE__))
 
@@ -42,9 +43,6 @@
 require 'minitest/reporters'
 Minitest::Reporters.use!
 
-require 'active_model_serializers'
-require 'active_model/serializer/railtie'
-
 require 'support/stream_capture'
 
 require 'support/rails_app'
@@ -59,7 +57,7 @@
 
 require 'fixtures/poro'
 
-ActiveSupport.on_load(:active_model_serializers) do
+ActiveSupport.on_load(:action_controller) do
   $action_controller_logger = ActiveModelSerializers.logger
   ActiveModelSerializers.logger = Logger.new(IO::NULL)
 end