Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

A few changes were done in this commit:

* Added :autoload to engines path API and redefine usage to be in sync with 6f83a50;
* Do not autoload code in *lib* for applications (now you need to explicitly require them). This makes an application behave closer to an engine (code in lib is still autoloaded for plugins);
* Always autoload code in app/ for engines and plugins. This makes engines behave closer to an application and should allow us to get rid of the unloadable hack required when controllers inside engines inherit from ApplicationController;
  • Loading branch information...
commit 9b19a6f16cebf4257d2f0b839f6cc8ff5db5c47b 1 parent 4329f81
@josevalim josevalim authored
View
7 railties/lib/rails/application.rb
@@ -28,7 +28,7 @@ module Rails
# Besides providing the same configuration as Rails::Engine and Rails::Railtie,
# the application object has several specific configurations, for example
# "allow_concurrency", "cache_classes", "consider_all_requests_local", "filter_parameters",
- # "logger", "reload_engines", "reload_plugins" and so forth.
+ # "logger", "reload_plugins" and so forth.
#
# Check Rails::Application::Configuration to see them all.
#
@@ -217,10 +217,5 @@ def initialize_tasks
def initialize_generators
require "rails/generators"
end
-
- # Application is always reloadable when config.cache_classes is false.
- def reloadable?(app)
- true
- end
end
end
View
4 railties/lib/rails/application/configuration.rb
@@ -10,7 +10,7 @@ class Configuration < ::Rails::Engine::Configuration
attr_accessor :allow_concurrency, :cache_classes, :cache_store,
:encoding, :consider_all_requests_local, :dependency_loading,
:filter_parameters, :log_level, :logger, :middleware,
- :plugins, :preload_frameworks, :reload_engines, :reload_plugins,
+ :plugins, :preload_frameworks, :reload_plugins,
:secret_token, :serve_static_assets, :session_options,
:time_zone, :whiny_nils
@@ -59,7 +59,7 @@ def paths
if File.exists?("#{root}/test/mocks/#{Rails.env}")
ActiveSupport::Deprecation.warn "\"Rails.root/test/mocks/#{Rails.env}\" won't be added " <<
"automatically to load paths anymore in future releases"
- paths.mocks_path "test/mocks", :load_path => true, :glob => Rails.env
+ paths.mocks_path "test/mocks", :autoload => true, :glob => Rails.env
end
paths
View
21 railties/lib/rails/engine.rb
@@ -142,7 +142,7 @@ def eager_load!
# Add configured load paths to ruby load paths and remove duplicates.
initializer :set_load_path, :before => :bootstrap_hook do
- config.autoload_paths.reverse_each do |path|
+ _all_load_paths.reverse_each do |path|
$LOAD_PATH.unshift(path) if File.directory?(path)
end
$LOAD_PATH.uniq!
@@ -154,16 +154,12 @@ def eager_load!
# This needs to be an initializer, since it needs to run once
# per engine and get the engine as a block parameter
initializer :set_autoload_paths, :before => :bootstrap_hook do |app|
- ActiveSupport::Dependencies.autoload_paths.unshift(*config.autoload_paths)
-
- if reloadable?(app)
- ActiveSupport::Dependencies.autoload_once_paths.unshift(*config.autoload_once_paths)
- else
- ActiveSupport::Dependencies.autoload_once_paths.unshift(*config.autoload_paths)
- end
+ ActiveSupport::Dependencies.autoload_paths.unshift(*_all_autoload_paths)
+ ActiveSupport::Dependencies.autoload_once_paths.unshift(*config.autoload_once_paths)
# Freeze so future modifications will fail rather than do nothing mysteriously
config.autoload_paths.freeze
+ config.eager_load_paths.freeze
config.autoload_once_paths.freeze
end
@@ -195,7 +191,6 @@ def eager_load!
ActiveSupport.on_load(:action_controller) do
prepend_view_path(views)
end
-
ActiveSupport.on_load(:action_mailer) do
prepend_view_path(views)
end
@@ -214,8 +209,12 @@ def eager_load!
protected
- def reloadable?(app)
- app.config.reload_engines
+ def _all_autoload_paths
+ @_all_autoload_paths ||= (config.autoload_paths + config.eager_load_paths + config.autoload_once_paths).uniq
+ end
+
+ def _all_load_paths
+ @_all_load_paths ||= (config.paths.load_paths + _all_autoload_paths).uniq
end
end
end
View
4 railties/lib/rails/engine/configuration.rb
@@ -42,11 +42,11 @@ def eager_load_paths
end
def autoload_once_paths
- @autoload_once_paths ||= paths.load_once
+ @autoload_once_paths ||= paths.autoload_once
end
def autoload_paths
- @autoload_paths ||= paths.load_paths
+ @autoload_paths ||= paths.autoload_paths
end
end
end
View
51 railties/lib/rails/paths.rb
@@ -25,9 +25,7 @@ class Root
def initialize(path)
raise if path.is_a?(Array)
-
@children = {}
-
@path = path
@root = self
@all_paths = []
@@ -38,14 +36,18 @@ def all_paths
@all_paths
end
- def load_once
- filter_by(:load_once?)
+ def autoload_once
+ filter_by(:autoload_once?)
end
def eager_load
filter_by(:eager_load?)
end
+ def autoload_paths
+ filter_by(:autoload?)
+ end
+
def load_paths
filter_by(:load_path?)
end
@@ -61,15 +63,17 @@ def push(*)
protected
def filter_by(constraint)
- all_paths.map do |path|
+ all = []
+ all_paths.each do |path|
if path.send(constraint)
paths = path.paths
paths -= path.children.values.map { |p| p.send(constraint) ? [] : p.paths }.flatten
- paths
- else
- []
+ all.concat(paths)
end
- end.flatten.uniq.select { |p| File.exists?(p) }
+ end
+ all.uniq!
+ all.reject! { |p| !File.exists?(p) }
+ all
end
end
@@ -80,15 +84,16 @@ class Path
attr_accessor :glob
def initialize(root, *paths)
- @options = paths.last.is_a?(::Hash) ? paths.pop : {}
+ options = paths.last.is_a?(::Hash) ? paths.pop : {}
@children = {}
@root = root
@paths = paths.flatten
- @glob = @options.delete(:glob)
+ @glob = options[:glob]
- @load_once = @options[:load_once]
- @eager_load = @options[:eager_load]
- @load_path = @options[:load_path] || @eager_load || @load_once
+ autoload_once! if options[:autoload_once]
+ eager_load! if options[:eager_load]
+ autoload! if options[:autoload]
+ load_path! if options[:load_path]
@root.all_paths << self
end
@@ -111,24 +116,30 @@ def concat(paths)
@paths.concat paths
end
- def load_once!
- @load_once = true
- @load_path = true
+ def autoload_once!
+ @autoload_once = true
end
- def load_once?
- @load_once
+ def autoload_once?
+ @autoload_once
end
def eager_load!
@eager_load = true
- @load_path = true
end
def eager_load?
@eager_load
end
+ def autoload!
+ @autoload = true
+ end
+
+ def autoload?
+ @autoload
+ end
+
def load_path!
@load_path = true
end
View
16 railties/lib/rails/plugin.rb
@@ -61,6 +61,16 @@ def config
@config ||= Engine::Configuration.new
end
+ initializer :handle_lib_autoload, :before => :set_load_path do |app|
+ paths = if app.config.reload_plugins
+ config.autoload_paths
+ else
+ config.autoload_once_paths
+ end
+
+ paths.concat config.paths.lib.to_a
+ end
+
initializer :load_init_rb, :before => :load_config_initializers do |app|
files = %w(rails/init.rb init.rb).map { |path| File.expand_path path, root }
if initrb = files.find { |path| File.file? path }
@@ -77,11 +87,5 @@ def config
raise "\"#{name}\" is a Railtie/Engine and cannot be installed as plugin"
end
end
-
- protected
-
- def reloadable?(app)
- app.config.reload_plugins
- end
end
end
View
56 railties/test/paths_test.rb
@@ -120,36 +120,36 @@ def setup
test "it is possible to add a path that should be loaded only once" do
@root.app = "/app"
- @root.app.load_once!
- assert @root.app.load_once?
- assert @root.load_once.include?(@root.app.paths.first)
+ @root.app.autoload_once!
+ assert @root.app.autoload_once?
+ assert @root.autoload_once.include?(@root.app.paths.first)
end
test "it is possible to add a path without assignment and specify it should be loaded only once" do
- @root.app "/app", :load_once => true
- assert @root.app.load_once?
- assert @root.load_once.include?("/app")
+ @root.app "/app", :autoload_once => true
+ assert @root.app.autoload_once?
+ assert @root.autoload_once.include?("/app")
end
test "it is possible to add multiple paths without assignment and specify it should be loaded only once" do
- @root.app "/app", "/app2", :load_once => true
- assert @root.app.load_once?
- assert @root.load_once.include?("/app")
- assert @root.load_once.include?("/app2")
+ @root.app "/app", "/app2", :autoload_once => true
+ assert @root.app.autoload_once?
+ assert @root.autoload_once.include?("/app")
+ assert @root.autoload_once.include?("/app2")
end
- test "making a path load_once more than once only includes it once in @root.load_once" do
+ test "making a path autoload_once more than once only includes it once in @root.load_once" do
@root.app = "/app"
- @root.app.load_once!
- @root.app.load_once!
- assert_equal 1, @root.load_once.select {|p| p == @root.app.paths.first }.size
+ @root.app.autoload_once!
+ @root.app.autoload_once!
+ assert_equal 1, @root.autoload_once.select {|p| p == @root.app.paths.first }.size
end
- test "paths added to a load_once path should be added to the load_once collection" do
+ test "paths added to a load_once path should be added to the autoload_once collection" do
@root.app = "/app"
- @root.app.load_once!
+ @root.app.autoload_once!
@root.app << "/app2"
- assert_equal 2, @root.load_once.size
+ assert_equal 2, @root.autoload_once.size
end
test "it is possible to mark a path as eager" do
@@ -173,11 +173,11 @@ def setup
end
test "it is possible to create a path without assignment and mark it both as eager and load once" do
- @root.app "/app", :eager_load => true, :load_once => true
+ @root.app "/app", :eager_load => true, :autoload_once => true
assert @root.app.eager_load?
- assert @root.app.load_once?
+ assert @root.app.autoload_once?
assert @root.eager_load.include?("/app")
- assert @root.load_once.include?("/app")
+ assert @root.autoload_once.include?("/app")
end
test "making a path eager more than once only includes it once in @root.eager_paths" do
@@ -218,16 +218,16 @@ def setup
assert_equal ["/app"], @root.load_paths
end
- test "adding a path to the eager paths also adds it to the load path" do
+ test "a path can be marked as autoload path" do
@root.app = "app"
- @root.app.eager_load!
- assert_equal ["/foo/bar/app"], @root.load_paths
+ @root.app.autoload!
+ @root.app.models = "app/models"
+ assert_equal ["/foo/bar/app"], @root.autoload_paths
end
- test "adding a path to the load once paths also adds it to the load path" do
- @root.app = "app"
- @root.app.load_once!
- assert_equal ["/foo/bar/app"], @root.load_paths
+ test "a path can be marked as autoload on creation" do
+ @root.app "/app", :autoload => true
+ assert @root.app.autoload?
+ assert_equal ["/app"], @root.autoload_paths
end
-
end
View
4 railties/test/railties/engine_test.rb
@@ -20,10 +20,6 @@ class Engine < ::Rails::Engine
end
end
- def reload_config
- :reload_engines
- end
-
test "Rails::Engine itself does not respond to config" do
boot_rails
assert !Rails::Engine.respond_to?(:config)
View
30 railties/test/railties/plugin_test.rb
@@ -15,10 +15,6 @@ def setup
end
end
- def reload_config
- :reload_plugins
- end
-
test "Rails::Plugin itself does not respond to config" do
boot_rails
assert !Rails::Plugin.respond_to?(:config)
@@ -37,6 +33,32 @@ class Foo < Rails::Plugin; end
assert_equal "Bukkits", Bukkits.name
end
+ test "plugin gets added to dependency list" do
+ boot_rails
+ assert_equal "Another", Another.name
+ end
+
+ test "plugin constants get reloaded if config.reload_plugins is set to true" do
+ add_to_config <<-RUBY
+ config.reload_plugins = true
+ RUBY
+
+ boot_rails
+
+ assert_equal "Another", Another.name
+ ActiveSupport::Dependencies.clear
+ @plugin.delete("lib/another.rb")
+ assert_raises(NameError) { Another }
+ end
+
+ test "plugin constants are not reloaded by default" do
+ boot_rails
+ assert_equal "Another", Another.name
+ ActiveSupport::Dependencies.clear
+ @plugin.delete("lib/another.rb")
+ assert_nothing_raised { Another }
+ end
+
test "it loads the plugin's init.rb file" do
boot_rails
assert_equal "loaded", BUKKITS
View
45 railties/test/railties/shared_tests.rb
@@ -10,51 +10,25 @@ def app
@app ||= Rails.application
end
- def test_plugin_puts_its_lib_directory_on_load_path
+ def test_puts_its_lib_directory_on_load_path
boot_rails
require "another"
assert_equal "Another", Another.name
end
- def test_plugin_paths_get_added_to_as_dependency_list
- boot_rails
- assert_equal "Another", Another.name
- end
-
- def test_plugins_constants_are_not_reloaded_by_default
- boot_rails
- assert_equal "Another", Another.name
- ActiveSupport::Dependencies.clear
- @plugin.delete("lib/another.rb")
- assert_nothing_raised { Another }
- end
-
- def test_plugin_constants_get_reloaded_if_config_reload_plugins
- add_to_config <<-RUBY
- config.#{reload_config} = true
- RUBY
-
- boot_rails
-
- assert_equal "Another", Another.name
- ActiveSupport::Dependencies.clear
- @plugin.delete("lib/another.rb")
- assert_raises(NameError) { Another }
- end
-
- def test_plugin_puts_its_models_directory_on_load_path
+ def test_puts_its_models_directory_on_autoload_path
@plugin.write "app/models/my_bukkit.rb", "class MyBukkit ; end"
boot_rails
assert_nothing_raised { MyBukkit }
end
- def test_plugin_puts_its_controllers_directory_on_the_load_path
+ def test_puts_its_controllers_directory_on_autoload_path
@plugin.write "app/controllers/bukkit_controller.rb", "class BukkitController ; end"
boot_rails
assert_nothing_raised { BukkitController }
end
- def test_plugin_adds_its_views_to_view_paths
+ def test_adds_its_views_to_view_paths
@plugin.write "app/controllers/bukkit_controller.rb", <<-RUBY
class BukkitController < ActionController::Base
def index
@@ -72,7 +46,7 @@ def index
assert_equal "Hello bukkits\n", response[2].body
end
- def test_plugin_adds_its_views_to_view_paths_with_lower_proriority
+ def test_adds_its_views_to_view_paths_with_lower_proriority_than_app_ones
@plugin.write "app/controllers/bukkit_controller.rb", <<-RUBY
class BukkitController < ActionController::Base
def index
@@ -91,7 +65,7 @@ def index
assert_equal "Hi bukkits\n", response[2].body
end
- def test_plugin_adds_helpers_to_controller_views
+ def test_adds_helpers_to_controller_views
@plugin.write "app/controllers/bukkit_controller.rb", <<-RUBY
class BukkitController < ActionController::Base
def index
@@ -116,11 +90,10 @@ def bukkits
assert_equal "Hello bukkits\n", response[2].body
end
- def test_plugin_eager_load_any_path_under_app
+ def test_autoload_any_path_under_app
@plugin.write "app/anything/foo.rb", <<-RUBY
module Foo; end
RUBY
-
boot_rails
assert Foo
end
@@ -269,7 +242,7 @@ def index
assert_equal "Rendered from namespace", last_response.body
end
- def test_plugin_initializers
+ def test_initializers
$plugin_initializer = false
@plugin.write "config/initializers/foo.rb", <<-RUBY
$plugin_initializer = true
@@ -279,7 +252,7 @@ def test_plugin_initializers
assert $plugin_initializer
end
- def test_plugin_midleware_referenced_in_configuration
+ def test_midleware_referenced_in_configuration
@plugin.write "lib/bukkits.rb", <<-RUBY
class Bukkits
def initialize(app)

9 comments on commit 9b19a6f

@fxn
Owner

Just for the record, if you still prefer to have stuff in lib autoloaded just add lib to config.autoload_paths in config/application.rb. Follow the example in the comment there for exact notation.

@jamesotron

This commit removed config.load_paths. Perhaps it should have a deprecation warning?

@jeremy
Owner

James, indeed: check out http://github.com/rails/rails/commit/4a745ca67054f840a67b438277de6cf7ad6f4f15 for the deprecation on 2-3-stable

@jamesotron

Right but it's present on 3.0.0.beta4 and gone in edge without a deprecation warning.

@jeremy
Owner

Gotcha, deprecation between the prereleases. Patches welcome ;)

@jpartogi

Agree with james. This change surprised me as well.

@rohit

There is already a ticket open for adding the deprecation warning, so if you make a patch add it there https://rails.lighthouseapp.com/projects/8994/tickets/5001-configload_paths-gone-but-no-deprecation-warning :)

@jamesotron

@rohit I know, I opened it :)

@jamesotron

@rohit I know, I opened it :)

Please sign in to comment.
Something went wrong with that request. Please try again.