From ef255e5119cd585e1f3bb36428651b9d7dd9704f Mon Sep 17 00:00:00 2001 From: Greg Bell Date: Sat, 29 Oct 2011 10:29:11 -0700 Subject: [PATCH 1/7] Only reload Active Admin if active admin files have changed This is a HUGE performance increase in development mode. Before this Active Admin was reloading on every single request to the servers (assets included). Now we only reload the Active Admin files if something in the AA load paths has changed. --- features/support/env.rb | 12 +++++++++--- lib/active_admin/application.rb | 2 +- lib/active_admin/reloader.rb | 9 +++++++-- spec/unit/reloader_spec.rb | 16 ++++++++++++---- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/features/support/env.rb b/features/support/env.rb index 55074cd03e2..f82a8736b73 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -9,18 +9,24 @@ require File.expand_path('../../../spec/support/detect_rails_version', __FILE__) ENV["RAILS"] = detect_rails_version +ENV["RAILS_ENV"] ||= "cucumber" +ENV['RAILS_ROOT'] = File.expand_path("../../../spec/rails/rails-#{ENV["RAILS"]}", __FILE__) + + require 'rubygems' require "bundler" Bundler.setup -ENV["RAILS_ENV"] ||= "cucumber" -ENV['RAILS_ROOT'] = File.expand_path("../../../spec/rails/rails-#{ENV["RAILS"]}", __FILE__) - # Create the test app if it doesn't exists unless File.exists?(ENV['RAILS_ROOT']) system 'rake setup' end +# Ensure the Active Admin load path is happy +require 'rails' +require 'active_admin' +ActiveAdmin.application.load_paths = [ENV['RAILS_ROOT'] + "/app/admin"] + require ENV['RAILS_ROOT'] + '/config/environment' # Setup autoloading of ActiveAdmin and the load path diff --git a/lib/active_admin/application.rb b/lib/active_admin/application.rb index 5c99af99cc2..82f287b2f76 100644 --- a/lib/active_admin/application.rb +++ b/lib/active_admin/application.rb @@ -208,7 +208,7 @@ def remove_active_admin_load_paths_from_rails_autoload_and_eager_load end def attach_reloader - ActiveAdmin::Reloader.new(Rails.version).attach! + ActiveAdmin::Reloader.new(self, Rails.version).attach! end diff --git a/lib/active_admin/reloader.rb b/lib/active_admin/reloader.rb index f24e3725751..f0e3ea6df2a 100644 --- a/lib/active_admin/reloader.rb +++ b/lib/active_admin/reloader.rb @@ -6,16 +6,21 @@ class Reloader # @param [String] rails_version # The version of Rails we're using. We use this to switch between # the correcr Rails reloader class. - def initialize(rails_version) + def initialize(app, rails_version) + @app = app @rails_version = rails_version.to_s end # Attach to Rails and perform the reload on each request. def attach! - reloader_class.to_prepare do + file_update_checker = ActiveSupport::FileUpdateChecker.new(@app.load_paths) do ActiveAdmin.application.unload! Rails.application.reload_routes! end + + reloader_class.to_prepare do + file_update_checker.execute_if_updated + end end def reloader_class diff --git a/spec/unit/reloader_spec.rb b/spec/unit/reloader_spec.rb index 65b3efd235a..7975bdaf713 100644 --- a/spec/unit/reloader_spec.rb +++ b/spec/unit/reloader_spec.rb @@ -4,25 +4,33 @@ begin ActionDispatch::Reloader rescue - module ActionDispatch; module Reloader; end; end + module ActionDispatch; module Reloader; def self.to_prepare; end; end; end end begin ActionDispatch::Callbacks rescue - module ActionDispatch; module Callbacks; end; end + module ActionDispatch; module Callbacks; def self.to_prepare; end; end; end end describe ActiveAdmin::Reloader do + let(:mock_app){ mock(:load_paths => []) } + it "should use ActionDispatch::Reloader if rails 3.1" do - reloader = ActiveAdmin::Reloader.new '3.1.0' + reloader = ActiveAdmin::Reloader.new mock_app, '3.1.0' reloader.reloader_class.should == ActionDispatch::Reloader end it "should use ActionDispatch::Callbacks if rails 3.0" do - reloader = ActiveAdmin::Reloader.new '3.0.0' + reloader = ActiveAdmin::Reloader.new mock_app, '3.0.0' reloader.reloader_class.should == ActionDispatch::Callbacks end + + it "should initialize a new file update checker" do + ActiveSupport::FileUpdateChecker.should_receive(:new).with(mock_app.load_paths).and_return(mock(:execute_if_updated => true)) + ActiveAdmin::Reloader.new(mock_app, '3.1.0').attach! + end + end From c8fa4a642f2233ffbcfa3c0bcd37c9ea66be513f Mon Sep 17 00:00:00 2001 From: Greg Bell Date: Sat, 29 Oct 2011 10:43:54 -0700 Subject: [PATCH 2/7] Bumping version to 0.3.3 --- CHANGELOG.md | 14 ++++++++++++++ lib/active_admin/version.rb | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 115687fabd2..481f7f21186 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,20 @@ Nothing yet +## 0.3.3 + +1 commit by 1 author + +### Enhancements + +* Only reload Active Admin when files in the load paths have changed. This is a + major speed increase in development mode. Also helps with memory consumption + because we aren't reloading Active admin all the time. + +### Contributors + +* Greg Bell + ## 0.3.2 45 commits by 15 contributors diff --git a/lib/active_admin/version.rb b/lib/active_admin/version.rb index 574b4fec531..f3962140702 100644 --- a/lib/active_admin/version.rb +++ b/lib/active_admin/version.rb @@ -1,3 +1,3 @@ module ActiveAdmin - VERSION = '0.3.2' + VERSION = '0.3.3' end From 6b9be9e29f840b485accce2e3e4c1fe5fda67e99 Mon Sep 17 00:00:00 2001 From: Greg Bell Date: Tue, 8 Nov 2011 07:30:36 -0800 Subject: [PATCH 3/7] Fix for active admin reloading issues in #707 --- lib/active_admin/application.rb | 2 +- lib/active_admin/reloader.rb | 49 +++++++++++++++++++------ spec/unit/reloader_spec.rb | 64 +++++++++++++++++++++++++++------ 3 files changed, 94 insertions(+), 21 deletions(-) diff --git a/lib/active_admin/application.rb b/lib/active_admin/application.rb index 82f287b2f76..7cee1925aa8 100644 --- a/lib/active_admin/application.rb +++ b/lib/active_admin/application.rb @@ -208,7 +208,7 @@ def remove_active_admin_load_paths_from_rails_autoload_and_eager_load end def attach_reloader - ActiveAdmin::Reloader.new(self, Rails.version).attach! + ActiveAdmin::Reloader.new(Rails.application, self, Rails.version).attach! end diff --git a/lib/active_admin/reloader.rb b/lib/active_admin/reloader.rb index f0e3ea6df2a..97c1a3787ad 100644 --- a/lib/active_admin/reloader.rb +++ b/lib/active_admin/reloader.rb @@ -1,28 +1,57 @@ module ActiveAdmin + + class FileUpdateChecker < ::ActiveSupport::FileUpdateChecker + + # Over-ride the default #updated_at to support the deletion of files + def updated_at + paths.map { |path| File.mtime(path) rescue Time.now }.max + end + + end + # Deals with reloading Active Admin on each request in # development and once in production. class Reloader - # @param [String] rails_version - # The version of Rails we're using. We use this to switch between - # the correcr Rails reloader class. - def initialize(app, rails_version) - @app = app + attr_reader :active_admin_app, + :rails_app, + :file_update_checker + + # @param [ActiveAdmin::Application] app + # @param [String] rails_version The version of Rails we're using. + # We use this to switch between the correcrt Rails reloader class. + def initialize(rails_app, active_admin_app, rails_version) + @rails_app = rails_app + @active_admin_app = active_admin_app @rails_version = rails_version.to_s + @file_update_checker = FileUpdateChecker.new(watched_paths) do + reload! + end + end + + def reload! + active_admin_app.unload! + rails_app.reload_routes! + file_update_checker.paths.clear + watched_paths.each{|path| file_update_checker.paths << path } end # Attach to Rails and perform the reload on each request. def attach! - file_update_checker = ActiveSupport::FileUpdateChecker.new(@app.load_paths) do - ActiveAdmin.application.unload! - Rails.application.reload_routes! - end + # Bring the checker into local scope for the ruby block + checker = file_update_checker reloader_class.to_prepare do - file_update_checker.execute_if_updated + checker.execute_if_updated end end + def watched_paths + paths = active_admin_app.load_paths + active_admin_app.load_paths.each{|path| paths += Dir[File.join(path, "**", "*.rb")]} + paths + end + def reloader_class if @rails_version[0..2] == '3.1' ActionDispatch::Reloader diff --git a/spec/unit/reloader_spec.rb b/spec/unit/reloader_spec.rb index 7975bdaf713..c8abd35cee1 100644 --- a/spec/unit/reloader_spec.rb +++ b/spec/unit/reloader_spec.rb @@ -16,21 +16,65 @@ module ActionDispatch; module Callbacks; def self.to_prepare; end; end; end describe ActiveAdmin::Reloader do - let(:mock_app){ mock(:load_paths => []) } + let(:rails_app){ mock(:reload_routes! => true)} + let(:mock_app){ mock(:load_paths => ["app/admin"], :unload! => true)} + let(:reloader){ ActiveAdmin::Reloader.new(rails_app, mock_app, "3.1.0")} - it "should use ActionDispatch::Reloader if rails 3.1" do - reloader = ActiveAdmin::Reloader.new mock_app, '3.1.0' - reloader.reloader_class.should == ActionDispatch::Reloader + it "should initialize a new file update checker" do + ActiveSupport::FileUpdateChecker.should_receive(:new).with(mock_app.load_paths).and_return(mock(:execute_if_updated => true)) + ActiveAdmin::Reloader.new(rails_app, mock_app, '3.1.0').attach! end - it "should use ActionDispatch::Callbacks if rails 3.0" do - reloader = ActiveAdmin::Reloader.new mock_app, '3.0.0' - reloader.reloader_class.should == ActionDispatch::Callbacks + describe "#reloader_class" do + it "should use ActionDispatch::Reloader if rails 3.1" do + reloader = ActiveAdmin::Reloader.new rails_app, mock_app, '3.1.0' + reloader.reloader_class.should == ActionDispatch::Reloader + end + + it "should use ActionDispatch::Callbacks if rails 3.0" do + reloader = ActiveAdmin::Reloader.new rails_app, mock_app, '3.0.0' + reloader.reloader_class.should == ActionDispatch::Callbacks + end end - it "should initialize a new file update checker" do - ActiveSupport::FileUpdateChecker.should_receive(:new).with(mock_app.load_paths).and_return(mock(:execute_if_updated => true)) - ActiveAdmin::Reloader.new(mock_app, '3.1.0').attach! + describe "#reload!" do + + it "should unload the active admin app" do + mock_app.should_receive(:unload!) + reloader.reload! + end + + it "should reload the rails app routes" do + rails_app.should_receive(:reload_routes!) + reloader.reload! + end + + it 'should reset the files within the file_update_checker' do + reloader.file_update_checker.paths.should_receive(:clear) + reloader.file_update_checker.paths.should_receive(:<<).with("app/admin") + reloader.reload! + end + + end + + describe "#watched_paths" do + let(:mock_app){ ActiveAdmin::Application.new } + let(:admin_path){ File.join(Rails.root, "app", "admin") } + + before do + mock_app.load_paths = [admin_path] + end + + it "should return the load path directories" do + reloader.watched_paths.should include(admin_path) + end + + it "should include all files in the directory" do + root = Rails.root + "/app/admin" + reloader.watched_paths.should include(*Dir["#{admin_path}/**/*.rb"]) + end + end + end From 97425ca81e7237d461546ea0600c568afcdec053 Mon Sep 17 00:00:00 2001 From: Greg Bell Date: Tue, 8 Nov 2011 10:24:00 -0800 Subject: [PATCH 4/7] Ensure that SASS doesn't reload on every request. Thanks to @dhiemstra! View the details at https://gist.github.com/1348501 --- lib/active_admin/sass/css_loader.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_admin/sass/css_loader.rb b/lib/active_admin/sass/css_loader.rb index 2f427db9020..ac78be3d9cc 100644 --- a/lib/active_admin/sass/css_loader.rb +++ b/lib/active_admin/sass/css_loader.rb @@ -9,7 +9,7 @@ class Sass::Importers::Filesystem # We want to ensure that all *.css.scss files are loaded as scss files def extensions_with_css - extensions_without_css.merge('css.scss' => :scss) + extensions_without_css.merge('{css.,}scss' => :scss) end alias_method_chain :extensions, :css From e98e892da9c9219e6eba46b5dfe9ac0af6315d29 Mon Sep 17 00:00:00 2001 From: Greg Bell Date: Tue, 8 Nov 2011 11:00:28 -0800 Subject: [PATCH 5/7] Added to CHANGELOG and bumped to 0.3.4 --- CHANGELOG.md | 16 ++++++++++++++++ lib/active_admin/version.rb | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 481f7f21186..148e569137c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,22 @@ Nothing yet +## 0.3.4 + +2 commits by 2 authors + +### Bug Fixes + +* Fix reloading issues across operating systems. +* Fix issue where SASS was recompiling on every request. This can seriously + decrease the load time of applications when running in development mode. + Thanks @dhiemstra for tracking this one down! + +### Contributors + +* Danny Hiemstra +* Greg Bell + ## 0.3.3 1 commit by 1 author diff --git a/lib/active_admin/version.rb b/lib/active_admin/version.rb index f3962140702..4d07919bf8d 100644 --- a/lib/active_admin/version.rb +++ b/lib/active_admin/version.rb @@ -1,3 +1,3 @@ module ActiveAdmin - VERSION = '0.3.3' + VERSION = '0.3.4' end From 70a1a73700dd11db0566c687b007da11661c05c0 Mon Sep 17 00:00:00 2001 From: Alexey Date: Mon, 12 Dec 2011 00:20:56 +0200 Subject: [PATCH 6/7] Fix typo --- .gitignore | 1 + lib/active_admin/resource/action_items.rb | 6 +++--- lib/active_admin/views/index_as_table.rb | 25 ++++++++++++++++++++--- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index eee7ffa0c8a..165cce23774 100644 --- a/.gitignore +++ b/.gitignore @@ -33,3 +33,4 @@ test-rails* public .rvmrc .rspec +.idea/**/* \ No newline at end of file diff --git a/lib/active_admin/resource/action_items.rb b/lib/active_admin/resource/action_items.rb index 58c8531a734..78b181e75ef 100644 --- a/lib/active_admin/resource/action_items.rb +++ b/lib/active_admin/resource/action_items.rb @@ -49,14 +49,14 @@ def add_default_action_items # New Link on all actions except :new and :show add_action_item :except => [:new, :show] do if controller.action_methods.include?('new') - link_to(I18n.t('active_admin.new_model', :model => active_admin_config.resource_name), new_resource_path) + link_to(I18n.t('active_admin.new_model', :model => active_admin_config.resource_name), new_resource_path, :class => "active_admin new_action") end end # Edit link on show add_action_item :only => :show do if controller.action_methods.include?('edit') - link_to(I18n.t('active_admin.edit_model', :model => active_admin_config.resource_name), edit_resource_path(resource)) + link_to(I18n.t('active_admin.edit_model', :model => active_admin_config.resource_name), edit_resource_path(resource), :class => "active_admin edit_action") end end @@ -64,7 +64,7 @@ def add_default_action_items add_action_item :only => :show do if controller.action_methods.include?("destroy") link_to(I18n.t('active_admin.delete_model', :model => active_admin_config.resource_name), - resource_path(resource), + resource_path(resource), :class => "active_admin delete_action", :method => :delete, :confirm => I18n.t('active_admin.delete_confirmation')) end end diff --git a/lib/active_admin/views/index_as_table.rb b/lib/active_admin/views/index_as_table.rb index 9762f3355f9..8c1fc3c8165 100644 --- a/lib/active_admin/views/index_as_table.rb +++ b/lib/active_admin/views/index_as_table.rb @@ -121,14 +121,33 @@ def id_column end # Adds links to View, Edit and Delete + # + # To include or exclude specific actions, use the :only and :except options. + # Insert additional markup before or after the default actions by using the :before and :after optinos. If the + # :before or :after options is a Proc, it will be called, with the current resource applied as first argument. + # Sample Usage: default_actions :except => [:delete, :edit] def default_actions(options = {}) options = { - :name => "" + :name => "", + :except => [], + :only => nil, + :before => "", + :after => "" }.merge(options) + + # :except takes precedence over :only. + display = options[:only] || [:view, :edit, :delete] + display.delete_if do |item| options[:except].include?(item) end + + # puts controller.action_methods + column options[:name] do |resource| links = link_to I18n.t('active_admin.view'), resource_path(resource), :class => "member_link view_link" - links += link_to I18n.t('active_admin.edit'), edit_resource_path(resource), :class => "member_link edit_link" - links += link_to I18n.t('active_admin.delete'), resource_path(resource), :method => :delete, :confirm => I18n.t('active_admin.delete_confirmation'), :class => "member_link delete_link" + links += options[:before].is_a?(Proc) ? options[:before].call(resource) : options[:before] + links += link_to I18n.t('active_admin.view'), resource_path(resource), :class => "member_link view_link" if display.include?(:view) + links += link_to I18n.t('active_admin.edit'), edit_resource_path(resource), :class => "member_link edit_link" if display.include?(:edit) && controller.action_methods.include?("edit") + links += link_to I18n.t('active_admin.delete'), resource_path(resource), :method => :delete, :confirm => I18n.t('active_admin.delete_confirmation'), :class => "member_link delete_link" if display.include?(:delete) && controller.action_methods.include?("destroy") + links += options[:after].is_a?(Proc) ? options[:after].call(resource) : options[:after] links end end From c466893af35dc7d130486c4160a85557b2a2bf78 Mon Sep 17 00:00:00 2001 From: Alexey Date: Wed, 14 Dec 2011 00:42:52 +0200 Subject: [PATCH 7/7] Added can? verification to render or not actions New/Edit/Destroy --- lib/active_admin/resource/action_items.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/active_admin/resource/action_items.rb b/lib/active_admin/resource/action_items.rb index 78b181e75ef..4e8c292d8b8 100644 --- a/lib/active_admin/resource/action_items.rb +++ b/lib/active_admin/resource/action_items.rb @@ -48,21 +48,21 @@ def clear_action_items! def add_default_action_items # New Link on all actions except :new and :show add_action_item :except => [:new, :show] do - if controller.action_methods.include?('new') + if controller.action_methods.include?('new') && can?(:create, active_admin_config.resource_name.constantize) link_to(I18n.t('active_admin.new_model', :model => active_admin_config.resource_name), new_resource_path, :class => "active_admin new_action") end end # Edit link on show add_action_item :only => :show do - if controller.action_methods.include?('edit') + if controller.action_methods.include?('edit') && can?(:update, active_admin_config.resource_name.constantize) link_to(I18n.t('active_admin.edit_model', :model => active_admin_config.resource_name), edit_resource_path(resource), :class => "active_admin edit_action") end end # Destroy link on show add_action_item :only => :show do - if controller.action_methods.include?("destroy") + if controller.action_methods.include?("destroy") && can?(:destroy, active_admin_config.resource_name.constantize) link_to(I18n.t('active_admin.delete_model', :model => active_admin_config.resource_name), resource_path(resource), :class => "active_admin delete_action", :method => :delete, :confirm => I18n.t('active_admin.delete_confirmation'))