diff --git a/CHANGELOG.md b/CHANGELOG.md index 80cd0c683..d7fa21492 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/Rakefile b/Rakefile index 58a49598e..4106d987d 100644 --- a/Rakefile +++ b/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] diff --git a/docs/general/configuration_options.md b/docs/general/configuration_options.md index 61cdb58aa..f6ca86353 100644 --- a/docs/general/configuration_options.md +++ b/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` diff --git a/lib/active_model/serializer/railtie.rb b/lib/active_model/serializer/railtie.rb deleted file mode 100644 index e2f992b60..000000000 --- a/lib/active_model/serializer/railtie.rb +++ /dev/null @@ -1,28 +0,0 @@ -require 'rails/railtie' - -module ActiveModel - class Railtie < Rails::Railtie - initializer 'active_model_serializers.logger' do - ActiveSupport.on_load(:active_model_serializers) do - self.logger = ActionController::Base.logger - end - end - - initializer 'active_model_serializers.caching' do - ActiveSupport.on_load(:action_controller) do - ActiveModelSerializers.config.cache_store = ActionController::Base.cache_store - ActiveModelSerializers.config.perform_caching = Rails.configuration.action_controller.perform_caching - end - end - - initializer 'generators' do |app| - app.load_generators - require 'generators/serializer/resource_override' - end - - if Rails.env.test? - ActionController::TestCase.send(:include, ActiveModelSerializers::Test::Schema) - ActionController::TestCase.send(:include, ActiveModelSerializers::Test::Serializer) - end - end -end diff --git a/lib/active_model_serializers.rb b/lib/active_model_serializers.rb index af216516e..d92823b5a 100644 --- a/lib/active_model_serializers.rb +++ b/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 diff --git a/lib/active_model_serializers/railtie.rb b/lib/active_model_serializers/railtie.rb new file mode 100644 index 000000000..4b98ec8b2 --- /dev/null +++ b/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 diff --git a/lib/generators/serializer/USAGE b/lib/generators/rails/USAGE similarity index 100% rename from lib/generators/serializer/USAGE rename to lib/generators/rails/USAGE diff --git a/lib/generators/serializer/resource_override.rb b/lib/generators/rails/resource_override.rb similarity index 72% rename from lib/generators/serializer/resource_override.rb rename to lib/generators/rails/resource_override.rb index 6da61166d..ebcba8df3 100644 --- a/lib/generators/serializer/resource_override.rb +++ b/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 diff --git a/lib/generators/serializer/serializer_generator.rb b/lib/generators/rails/serializer_generator.rb similarity index 84% rename from lib/generators/serializer/serializer_generator.rb rename to lib/generators/rails/serializer_generator.rb index 7a65fe773..c564a7c98 100644 --- a/lib/generators/serializer/serializer_generator.rb +++ b/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 - diff --git a/lib/generators/serializer/templates/serializer.rb.erb b/lib/generators/rails/templates/serializer.rb.erb similarity index 100% rename from lib/generators/serializer/templates/serializer.rb.erb rename to lib/generators/rails/templates/serializer.rb.erb diff --git a/test/active_model_serializers/railtie_test_isolated.rb b/test/active_model_serializers/railtie_test_isolated.rb new file mode 100644 index 000000000..2e2818ed6 --- /dev/null +++ b/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 diff --git a/test/generators/scaffold_controller_generator_test.rb b/test/generators/scaffold_controller_generator_test.rb index aabe6122f..183bb4f6f 100644 --- a/test/generators/scaffold_controller_generator_test.rb +++ b/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__) diff --git a/test/generators/serializer_generator_test.rb b/test/generators/serializer_generator_test.rb index 5395dff66..562b93380 100644 --- a/test/generators/serializer_generator_test.rb +++ b/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__) diff --git a/test/support/isolated_unit.rb b/test/support/isolated_unit.rb new file mode 100644 index 000000000..50362239d --- /dev/null +++ b/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 diff --git a/test/test_helper.rb b/test/test_helper.rb index 809f3ca43..59c0d3f41 100644 --- a/test/test_helper.rb +++ b/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