From e4ae6698e52e56a1cefdf5c4097b29b8306f21e2 Mon Sep 17 00:00:00 2001 From: "M.Shibuya" Date: Sun, 17 Oct 2021 13:50:31 +0900 Subject: [PATCH] Reload configuration on change in development mode Fixes #2726, Fixes #3025 --- .../initializers/active_record_extensions.rb | 4 +- lib/rails_admin.rb | 2 +- lib/rails_admin/abstract_model.rb | 2 +- lib/rails_admin/adapters/mongoid/extension.rb | 4 +- lib/rails_admin/config.rb | 35 ++++- lib/rails_admin/config/lazy_model.rb | 68 ---------- lib/rails_admin/engine.rb | 20 ++- spec/rails_admin/abstract_model_spec.rb | 12 ++ spec/rails_admin/config/has_fields_spec.rb | 20 +++ spec/rails_admin/config/lazy_model_spec.rb | 45 ------- spec/rails_admin/config_spec.rb | 125 ++++++++++-------- spec/rails_admin/engine_spec.rb | 30 +++++ 12 files changed, 190 insertions(+), 177 deletions(-) delete mode 100644 lib/rails_admin/config/lazy_model.rb delete mode 100644 spec/rails_admin/config/lazy_model_spec.rb create mode 100644 spec/rails_admin/engine_spec.rb diff --git a/config/initializers/active_record_extensions.rb b/config/initializers/active_record_extensions.rb index f2638805ee..8302025059 100644 --- a/config/initializers/active_record_extensions.rb +++ b/config/initializers/active_record_extensions.rb @@ -2,7 +2,9 @@ module ActiveRecord class Base def self.rails_admin(&block) - RailsAdmin.config(self, &block) + RailsAdmin.config do |config| + config.model(self, &block) + end end def rails_admin_default_object_label_method diff --git a/lib/rails_admin.rb b/lib/rails_admin.rb index 3e96b2e85c..bc08f44614 100644 --- a/lib/rails_admin.rb +++ b/lib/rails_admin.rb @@ -27,7 +27,7 @@ def self.config(entity = nil, &block) if entity RailsAdmin::Config.model(entity, &block) elsif block_given? - block.call(RailsAdmin::Config) + RailsAdmin::Config.apply(&block) else RailsAdmin::Config end diff --git a/lib/rails_admin/abstract_model.rb b/lib/rails_admin/abstract_model.rb index 61c7cdf062..33adb28f04 100644 --- a/lib/rails_admin/abstract_model.rb +++ b/lib/rails_admin/abstract_model.rb @@ -19,7 +19,7 @@ def all(adapter = nil) def new(m) m = m.constantize unless m.is_a?(Class) (am = old_new(m)).model && am.adapter ? am : nil - rescue LoadError, NameError + rescue *([LoadError, NameError] + (defined?(ActiveRecord) ? ['ActiveRecord::NoDatabaseError'.constantize] : [])) puts "[RailsAdmin] Could not load model #{m}, assuming model is non existing. (#{$ERROR_INFO})" unless Rails.env.test? nil end diff --git a/lib/rails_admin/adapters/mongoid/extension.rb b/lib/rails_admin/adapters/mongoid/extension.rb index 361489cbf6..d87c92c9d5 100644 --- a/lib/rails_admin/adapters/mongoid/extension.rb +++ b/lib/rails_admin/adapters/mongoid/extension.rb @@ -9,7 +9,9 @@ module Extension self.nested_attributes_options = {} class << self def rails_admin(&block) - RailsAdmin.config(self, &block) + RailsAdmin.config do |config| + config.model(self, &block) + end end alias_method :accepts_nested_attributes_for_without_rails_admin, :accepts_nested_attributes_for diff --git a/lib/rails_admin/config.rb b/lib/rails_admin/config.rb index 0ced253039..53efe0e222 100644 --- a/lib/rails_admin/config.rb +++ b/lib/rails_admin/config.rb @@ -1,4 +1,4 @@ -require 'rails_admin/config/lazy_model' +require 'rails_admin/config/model' require 'rails_admin/config/sections/list' require 'active_support/core_ext/module/attribute_accessors' @@ -20,6 +20,10 @@ module Config DEFAULT_CURRENT_USER = proc {} + # Variables to track initialization process + @initialized = false + @deferred_blocks = [] + class << self # Application title, can be an array of two elements attr_accessor :main_app_name @@ -82,6 +86,22 @@ class << self attr_accessor :navigation_static_links attr_accessor :navigation_static_label + # Finish initialization by executing deferred configuration blocks + def initialize! + @deferred_blocks.each { |block| block.call(self) } + @deferred_blocks.clear + @initialized = true + end + + # Evaluate the given block either immediately or lazily, based on initialization status. + def apply(&block) + if @initialized + block.call(self) + else + @deferred_blocks << block + end + end + # Setup authentication to be run as a before filter # This is run inside the controller instance so you can setup any authentication you need to # @@ -235,8 +255,8 @@ def model(entity, &block) end end - @registry[key] ||= RailsAdmin::Config::LazyModel.new(entity) - @registry[key].add_deferred_block(&block) if block + @registry[key] ||= RailsAdmin::Config::Model.new(entity) + @registry[key].instance_eval(&block) if block && @registry[key].abstract_model @registry[key] end @@ -311,10 +331,17 @@ def reset_all_models @registry = {} end + # Perform reset, then load RailsAdmin initializer again + def reload! + @initialized = false + reset + load RailsAdmin::Engine.config.initializer_path + initialize! + end + # Get all models that are configured as visible sorted by their weight and label. # # @see RailsAdmin::Config::Hideable - def visible_models(bindings) visible_models_with_bindings(bindings).sort do |a, b| if (weight_order = a.weight <=> b.weight) == 0 diff --git a/lib/rails_admin/config/lazy_model.rb b/lib/rails_admin/config/lazy_model.rb deleted file mode 100644 index 63bcfd3cdd..0000000000 --- a/lib/rails_admin/config/lazy_model.rb +++ /dev/null @@ -1,68 +0,0 @@ -require 'rails_admin/config/model' - -module RailsAdmin - module Config - class LazyModel < BasicObject - def initialize(entity, &block) - @entity = entity - @deferred_blocks = [*block] - @existing_blocks = [] - end - - def add_deferred_block(&block) - @deferred_blocks << block - end - - def target - @model ||= ::RailsAdmin::Config::Model.new(@entity) - # When evaluating multiple configuration blocks, the order of - # execution is important. As one would expect (in my opinion), - # options defined within a resource should take precedence over - # more general options defined in an initializer. This way, - # general settings for a number of resources could be specified - # in the initializer, while models could override these settings - # later, if required. - # - # CAVEAT: It cannot be guaranteed that blocks defined in an initializer - # will be loaded (and adde to @deferred_blocks) first. For instance, if - # the initializer references a model class before defining - # a RailsAdmin configuration block, the configuration from the - # resource will get added to @deferred_blocks first: - # - # # app/models/some_model.rb - # class SomeModel - # rails_admin do - # : - # end - # end - # - # # config/initializers/rails_admin.rb - # model = 'SomeModel'.constantize # blocks from SomeModel get loaded - # model.config model do # blocks from initializer gets loaded - # : - # end - # - # Thus, sort all blocks to excute for a resource by Proc.source_path, - # to guarantee that blocks from 'config/initializers' evaluate before - # blocks defined within a model class. - unless @deferred_blocks.empty? - @existing_blocks += @deferred_blocks - @existing_blocks. - partition { |block| block.source_location.first =~ %r{config\/initializers} }. - flatten. - each { |block| @model.instance_eval(&block) } - @deferred_blocks = [] - end - @model - end - - def method_missing(method_name, *args, &block) - target.send(method_name, *args, &block) - end - - def respond_to?(method_name, include_private = false) - super || target.respond_to?(method_name, include_private) - end - end - end -end diff --git a/lib/rails_admin/engine.rb b/lib/rails_admin/engine.rb index 8b3344e0f7..9b2cb2fd6e 100644 --- a/lib/rails_admin/engine.rb +++ b/lib/rails_admin/engine.rb @@ -24,11 +24,23 @@ class Engine < Rails::Engine app.config.middleware.use Rack::Pjax end - initializer 'RailsAdmin reload config in development' do - if Rails.application.config.cache_classes + initializer 'RailsAdmin reload config in development' do |app| + config.initializer_path = app.root.join('config/initializers/rails_admin.rb') + + unless Rails.application.config.cache_classes ActiveSupport::Reloader.before_class_unload do - RailsAdmin::Config.reset_all_models + RailsAdmin::Config.reload! + end + + reloader = app.config.file_watcher.new([config.initializer_path], []) do + # Do nothing, ActiveSupport::Reloader will trigger class_unload! anyway end + + app.reloaders << reloader + app.reloader.to_run do + reloader.execute_if_updated { require_unload_lock! } + end + reloader.execute end end @@ -57,6 +69,8 @@ class Engine < Rails::Engine to config/application.rb. EOM end + + RailsAdmin::Config.initialize! end end end diff --git a/spec/rails_admin/abstract_model_spec.rb b/spec/rails_admin/abstract_model_spec.rb index e4048b9f86..93ceb8e90f 100644 --- a/spec/rails_admin/abstract_model_spec.rb +++ b/spec/rails_admin/abstract_model_spec.rb @@ -11,6 +11,18 @@ end end + describe '.new' do + context 'on ActiveRecord::NoDatabaseError', active_record: true do + before do + expect(WithoutTable).to receive(:table_exists?).and_raise(ActiveRecord::NoDatabaseError) + end + + it 'does not raise error and returns nil' do + expect(RailsAdmin::AbstractModel.new('WithoutTable')).to eq nil + end + end + end + describe '#to_s' do it 'returns model\'s name' do expect(RailsAdmin::AbstractModel.new(Cms::BasicPage).to_s).to eq Cms::BasicPage.to_s diff --git a/spec/rails_admin/config/has_fields_spec.rb b/spec/rails_admin/config/has_fields_spec.rb index 613398fbec..65c3d42a2c 100644 --- a/spec/rails_admin/config/has_fields_spec.rb +++ b/spec/rails_admin/config/has_fields_spec.rb @@ -51,4 +51,24 @@ expect(RailsAdmin.config(Team).fields.map(&:name)).to eql(original_fields_order) end + + describe '#_fields' do + let(:config) { RailsAdmin.config(Team) } + before do + RailsAdmin.config(Team) do + field :id + field :wins, :boolean + end + end + + it "does not cause FrozenError by changing exiting field's tye" do + # Reference the fields for readonly + config.edit.send(:_fields, true) + + RailsAdmin.config(Team) do + field :wins, :integer + end + expect(config.fields.map(&:name)).to match_array %i(id wins) + end + end end diff --git a/spec/rails_admin/config/lazy_model_spec.rb b/spec/rails_admin/config/lazy_model_spec.rb deleted file mode 100644 index 4fed5b2101..0000000000 --- a/spec/rails_admin/config/lazy_model_spec.rb +++ /dev/null @@ -1,45 +0,0 @@ -require 'spec_helper' - -RSpec.describe RailsAdmin::Config::LazyModel do - subject { RailsAdmin::Config::LazyModel.new(:Team, &block) } - let(:block) { proc { register_instance_option('parameter') } } # an arbitrary instance method we can spy on - - describe '#store' do - it "doesn't evaluate the block immediately" do - expect_any_instance_of(RailsAdmin::Config::Model).not_to receive(:register_instance_option) - subject - end - - it 'evaluates block when reading' do - expect_any_instance_of(RailsAdmin::Config::Model).to receive(:register_instance_option).with('parameter') - subject.groups # an arbitrary instance method on RailsAdmin::Config::Model to wake up lazy_model - end - - it 'evaluates config block only once' do - expect_any_instance_of(RailsAdmin::Config::Model).to receive(:register_instance_option).once.with('parameter') - - subject.groups - subject.groups - end - end - - context 'when a method is defined in Kernel' do - before do - Kernel.module_eval do - def weight - 42 - end - end - end - - after do - Kernel.module_eval do - undef weight - end - end - - it 'proxies calls for the method to @object' do - expect(subject.weight).to eq 0 - end - end -end diff --git a/spec/rails_admin/config_spec.rb b/spec/rails_admin/config_spec.rb index 6f35181427..4294f62e07 100644 --- a/spec/rails_admin/config_spec.rb +++ b/spec/rails_admin/config_spec.rb @@ -296,6 +296,7 @@ class RecursivelyEmbedsMany end end end + context 'when model expanded' do before do described_class.model(Team) do @@ -306,6 +307,7 @@ class RecursivelyEmbedsMany expect(fields.map(&:name)).to match_array %i(players fans) end end + context 'when expand redefine behavior' do before do described_class.model Team do @@ -316,14 +318,12 @@ class RecursivelyEmbedsMany expect(fields.find { |f| f.name == :players }.visible).to be true end end - context 'when model expanded in config' do - let(:block) { proc { field :players } } - before do - allow(block).to receive(:source_location).and_return(['config/initializers/rails_admin.rb']) - described_class.model(Team, &block) - end - it 'executes first' do - expect(fields.find { |f| f.name == :players }.visible).to be false + + context 'when model has no table yet', active_record: true do + it 'does not try to apply the configuration block' do + described_class.model(WithoutTable) do + include_all_fields + end end end end @@ -347,67 +347,86 @@ class RecursivelyEmbedsMany end end - describe "field types code reloading" do - before { Rails.application.config.cache_classes = false } - after { Rails.application.config.cache_classes = true } - - let(:config) { described_class.model(Team) } - let(:fields) { described_class.model(Team).edit.fields } - - let(:team_config) do - proc do - field :id - field :wins, :boolean + describe ".apply" do + subject { RailsAdmin::Config.apply(&block) } + let(:block) do + proc do |config| + config.model Team do + register_instance_option('parameter') # an arbitrary instance method we can spy on + end end end - - let(:team_config2) do - proc do - field :wins, :toggle - end + before { RailsAdmin::Config.instance_variable_set(:@initialized, false) } + after do + RailsAdmin::Config.instance_variable_set(:@initialized, true) + RailsAdmin::Config.instance_variable_set(:@deferred_blocks, []) end - let(:team_config3) do - proc do - field :wins - end + it "doesn't evaluate the block immediately" do + expect_any_instance_of(RailsAdmin::Config::Model).not_to receive(:register_instance_option) + subject end - it "allows code reloading" do - Team.send(:rails_admin, &team_config) + it 'evaluates block when initialize! is finished' do + expect_any_instance_of(RailsAdmin::Config::Model).to receive(:register_instance_option).with('parameter') + subject + RailsAdmin::Config.initialize! + end - # This simulates the way RailsAdmin really does it - config.edit.send(:_fields, true) + it 'evaluates config block only once' do + expect_any_instance_of(RailsAdmin::Config::Model).to receive(:register_instance_option).once.with('parameter') + subject + RailsAdmin::Config.initialize! + RailsAdmin::Config.initialize! + end - module RailsAdmin - module Config - module Fields - module Types - class Toggle < RailsAdmin::Config::Fields::Base - RailsAdmin::Config::Fields::Types.register(self) - end - end + context 'with a non-existent class' do + let(:block) do + proc do |config| + config.model UnknownClass do + field :name end end end - Team.send(:rails_admin, &team_config2) - expect(fields.map(&:name)).to match_array %i(id wins) + + it "doesn't break immediately" do + subject + end + end + end + + describe '.reload!' do + before do + RailsAdmin.config Player do + field :name + end + RailsAdmin.config Team do + field :color, :hidden + end end - it "updates model config when reloading code for rails 5" do - Team.send(:rails_admin, &team_config) + it 'clears current configuration' do + RailsAdmin::Config.reload! + expect(RailsAdmin::Config.model(Player).fields.map(&:name)).to include :number + end - # this simulates rails code reloading - RailsAdmin::Engine.initializers.select do |i| - i.name == "RailsAdmin reload config in development" - end.first.block.call - Rails.application.executor.wrap do - ActiveSupport::Reloader.new.tap(&:class_unload!).complete! + it 're-applies the configuration from the initializer' do + RailsAdmin::Config.reload! + expect(RailsAdmin::Config.model(Team).fields.find { |f| f.name == :color }.type).to eq :color + end + + it "applies the initializer's configuration first, then models' configurations" do + # simulate the situation that Team model is loaded in the middle of processing RailsAdmin initializer + allow_any_instance_of(RailsAdmin::Config::Model).to receive(:include_all_fields).and_wrap_original do |method| + Team.rails_admin do + field :color, :integer + end + method.call end - Team.send(:rails_admin, &team_config3) - expect(fields.map(&:name)).to match_array %i(wins) - end if defined?(ActiveSupport::Reloader) + RailsAdmin::Config.reload! + expect(RailsAdmin::Config.model(Team).fields.find { |f| f.name == :color }.type).to eq :integer + end end end diff --git a/spec/rails_admin/engine_spec.rb b/spec/rails_admin/engine_spec.rb new file mode 100644 index 0000000000..4c1c40d2d1 --- /dev/null +++ b/spec/rails_admin/engine_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +RSpec.describe RailsAdmin::Engine do + context 'on class unload' do + let(:fields) { RailsAdmin.config(Player).edit.fields } + before do + Rails.application.config.cache_classes = false + RailsAdmin.config(Player) do + field :name + field :number + end + end + after { Rails.application.config.cache_classes = true } + + it "triggers RailsAdmin config to be reloaded" do + # this simulates rails code reloading + RailsAdmin::Engine.initializers.select do |i| + i.name == "RailsAdmin reload config in development" + end.first.block.call(Rails.application) + Rails.application.executor.wrap do + ActiveSupport::Reloader.new.tap(&:class_unload!).complete! + end + + RailsAdmin.config(Player) do + field :number + end + expect(fields.map(&:name)).to match_array %i(number) + end + end +end