Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Added app/[models|controllers|helpers] to the load path for plugins t…

…hat has an app directory (go engines ;)) [DHH]
  • Loading branch information...
commit 63d8f56774dcb1ea601928c3eb6c119d359fae10 1 parent 133c349
@dhh dhh authored
View
2  railties/CHANGELOG
@@ -1,5 +1,7 @@
*2.3.0 [Edge]*
+* Added app/[models|controllers|helpers] to the load path for plugins that has an app directory (go engines ;)) [DHH]
+
* Add config.preload_frameworks to load all frameworks at startup. Default to false so Rails autoloads itself as it's used. Turn this on for Passenger and JRuby. Also turned on by config.threadsafe! [Jeremy Kemper]
* Add a rake task to generate dispatchers : rake rails:generate_dispatchers [Pratik]
View
25 railties/lib/rails/plugin.rb
@@ -28,13 +28,17 @@ def initialize(directory)
end
def valid?
- File.directory?(directory) && (has_lib_directory? || has_init_file?)
+ File.directory?(directory) && (has_app_directory? || has_lib_directory? || has_init_file?)
end
# Returns a list of paths this plugin wishes to make available in <tt>$LOAD_PATH</tt>.
def load_paths
report_nonexistant_or_empty_plugin! unless valid?
- has_lib_directory? ? [lib_path] : []
+
+ returning [] do |load_paths|
+ load_paths << lib_path if has_lib_directory?
+ load_paths << app_paths if has_app_directory?
+ end.flatten
end
# Evaluates a plugin's init.rb file.
@@ -68,7 +72,16 @@ def load_about_information
def report_nonexistant_or_empty_plugin!
raise LoadError, "Can not find the plugin named: #{name}"
- end
+ end
+
+
+ def app_paths
+ [
+ File.join(directory, 'app', 'models'),
+ File.join(directory, 'app', 'controllers'),
+ File.join(directory, 'app', 'helpers')
+ ]
+ end
def lib_path
File.join(directory, 'lib')
@@ -86,6 +99,11 @@ def init_path
File.file?(gem_init_path) ? gem_init_path : classic_init_path
end
+
+ def has_app_directory?
+ File.directory?(File.join(directory, 'app'))
+ end
+
def has_lib_directory?
File.directory?(lib_path)
end
@@ -94,6 +112,7 @@ def has_init_file?
File.file?(init_path)
end
+
def evaluate_init_rb(initializer)
if has_init_file?
silence_warnings do
View
10 railties/lib/rails/plugin/loader.rb
@@ -33,6 +33,7 @@ def load_plugins
plugin.load(initializer)
register_plugin_as_loaded(plugin)
end
+
ensure_all_registered_plugins_are_loaded!
end
@@ -45,12 +46,15 @@ def add_plugin_load_paths
plugins.each do |plugin|
plugin.load_paths.each do |path|
$LOAD_PATH.insert(application_lib_index + 1, path)
- ActiveSupport::Dependencies.load_paths << path
+
+ ActiveSupport::Dependencies.load_paths << path
+
unless Rails.configuration.reload_plugins?
ActiveSupport::Dependencies.load_once_paths << path
end
end
end
+
$LOAD_PATH.uniq!
end
@@ -59,9 +63,9 @@ def add_plugin_load_paths
# The locate_plugins method uses each class in config.plugin_locators to
# find the set of all plugins available to this Rails application.
def locate_plugins
- configuration.plugin_locators.map { |locator|
+ configuration.plugin_locators.map do |locator|
locator.new(initializer).plugins
- }.flatten
+ end.flatten
# TODO: sorting based on config.plugins
end
View
8 railties/test/initializer_test.rb
@@ -209,7 +209,7 @@ def test_only_the_specified_plugins_are_located_in_the_order_listed
def test_all_plugins_are_loaded_when_registered_plugin_list_is_untouched
failure_tip = "It's likely someone has added a new plugin fixture without updating this list"
load_plugins!
- assert_plugins [:a, :acts_as_chunky_bacon, :gemlike, :plugin_with_no_lib_dir, :stubby], @initializer.loaded_plugins, failure_tip
+ assert_plugins [:a, :acts_as_chunky_bacon, :engine, :gemlike, :plugin_with_no_lib_dir, :stubby], @initializer.loaded_plugins, failure_tip
end
def test_all_plugins_loaded_when_all_is_used
@@ -217,7 +217,7 @@ def test_all_plugins_loaded_when_all_is_used
only_load_the_following_plugins! plugin_names
load_plugins!
failure_tip = "It's likely someone has added a new plugin fixture without updating this list"
- assert_plugins [:stubby, :acts_as_chunky_bacon, :a, :gemlike, :plugin_with_no_lib_dir], @initializer.loaded_plugins, failure_tip
+ assert_plugins [:stubby, :acts_as_chunky_bacon, :a, :engine, :gemlike, :plugin_with_no_lib_dir], @initializer.loaded_plugins, failure_tip
end
def test_all_plugins_loaded_after_all
@@ -225,7 +225,7 @@ def test_all_plugins_loaded_after_all
only_load_the_following_plugins! plugin_names
load_plugins!
failure_tip = "It's likely someone has added a new plugin fixture without updating this list"
- assert_plugins [:stubby, :a, :gemlike, :plugin_with_no_lib_dir, :acts_as_chunky_bacon], @initializer.loaded_plugins, failure_tip
+ assert_plugins [:stubby, :a, :engine, :gemlike, :plugin_with_no_lib_dir, :acts_as_chunky_bacon], @initializer.loaded_plugins, failure_tip
end
def test_plugin_names_may_be_strings
@@ -299,7 +299,7 @@ def test_config_defaults_and_settings_should_be_added_to_i18n_defaults
File.expand_path("./test/../../activesupport/lib/active_support/locale/en.yml"),
File.expand_path("./test/../../actionpack/lib/action_view/locale/en.yml"),
"my/test/locale.yml",
- "my/other/locale.yml" ], I18n.load_path
+ "my/other/locale.yml" ], I18n.load_path.collect { |path| path =~ /^\./ ? File.expand_path(path) : path }
end
def test_setting_another_default_locale
View
25 railties/test/plugin_loader_test.rb
@@ -48,16 +48,16 @@ def test_should_return_empty_array_if_configuration_plugins_is_empty
end
def test_should_find_all_availble_plugins_and_return_as_all_plugins
- assert_plugins [:stubby, :plugin_with_no_lib_dir, :gemlike, :acts_as_chunky_bacon, :a], @loader.all_plugins.reverse, @failure_tip
+ assert_plugins [ :engine, :stubby, :plugin_with_no_lib_dir, :gemlike, :acts_as_chunky_bacon, :a], @loader.all_plugins.reverse, @failure_tip
end
def test_should_return_all_plugins_as_plugins_when_registered_plugin_list_is_untouched
- assert_plugins [:a, :acts_as_chunky_bacon, :gemlike, :plugin_with_no_lib_dir, :stubby], @loader.plugins, @failure_tip
+ assert_plugins [:a, :acts_as_chunky_bacon, :engine, :gemlike, :plugin_with_no_lib_dir, :stubby], @loader.plugins, @failure_tip
end
def test_should_return_all_plugins_as_plugins_when_registered_plugin_list_is_nil
@configuration.plugins = nil
- assert_plugins [:a, :acts_as_chunky_bacon, :gemlike, :plugin_with_no_lib_dir, :stubby], @loader.plugins, @failure_tip
+ assert_plugins [:a, :acts_as_chunky_bacon, :engine, :gemlike, :plugin_with_no_lib_dir, :stubby], @loader.plugins, @failure_tip
end
def test_should_return_specific_plugins_named_in_config_plugins_array_if_set
@@ -74,17 +74,17 @@ def test_should_respect_the_order_of_plugins_given_in_configuration
def test_should_load_all_plugins_in_natural_order_when_all_is_used
only_load_the_following_plugins! [:all]
- assert_plugins [:a, :acts_as_chunky_bacon, :gemlike, :plugin_with_no_lib_dir, :stubby], @loader.plugins, @failure_tip
+ assert_plugins [:a, :acts_as_chunky_bacon, :engine, :gemlike, :plugin_with_no_lib_dir, :stubby], @loader.plugins, @failure_tip
end
def test_should_load_specified_plugins_in_order_and_then_all_remaining_plugins_when_all_is_used
only_load_the_following_plugins! [:stubby, :acts_as_chunky_bacon, :all]
- assert_plugins [:stubby, :acts_as_chunky_bacon, :a, :gemlike, :plugin_with_no_lib_dir], @loader.plugins, @failure_tip
+ assert_plugins [:stubby, :acts_as_chunky_bacon, :a, :engine, :gemlike, :plugin_with_no_lib_dir], @loader.plugins, @failure_tip
end
def test_should_be_able_to_specify_loading_of_plugins_loaded_after_all
only_load_the_following_plugins! [:stubby, :all, :acts_as_chunky_bacon]
- assert_plugins [:stubby, :a, :gemlike, :plugin_with_no_lib_dir, :acts_as_chunky_bacon], @loader.plugins, @failure_tip
+ assert_plugins [:stubby, :a, :engine, :gemlike, :plugin_with_no_lib_dir, :acts_as_chunky_bacon], @loader.plugins, @failure_tip
end
def test_should_accept_plugin_names_given_as_strings
@@ -112,6 +112,19 @@ def test_should_add_plugin_load_paths_to_Dependencies_load_paths
assert ActiveSupport::Dependencies.load_paths.include?(File.join(plugin_fixture_path('default/acts/acts_as_chunky_bacon'), 'lib'))
end
+
+ def test_should_add_engine_load_paths_to_Dependencies_load_paths
+ only_load_the_following_plugins! [:engine]
+
+ @loader.add_plugin_load_paths
+
+ %w( models controllers helpers ).each do |app_part|
+ assert ActiveSupport::Dependencies.load_paths.include?(
+ File.join(plugin_fixture_path('engines/engine'), 'app', app_part)
+ ), "Couldn't find #{app_part} in load path"
+ end
+ end
+
def test_should_add_plugin_load_paths_to_Dependencies_load_once_paths
only_load_the_following_plugins! [:stubby, :acts_as_chunky_bacon]

24 comments on commit 63d8f56

@svenfuchs

wohoooooo :)

@lazyatom

Nice ;)

@zargony

Great. I wonder if/how an engine plugin can add routes.

@Virtuaffinity

Good to see this making it into core, nice stuff!

@tekin

hallelujah!

@DefV

@zargony 16:28 :: DHH:: It gets better shortly ;) 16:28 :: DHH:: almost have config/routes.rb working for plugins too

@yaroslav

yay!

@maxlapshin

Cool!!! Waiting for Django-like apps!

@andrewroth

I’m happy to see this change, but I am curious — what made you change your mind?

@matthewrudy

need to find a good way to extend routing from a plugin.

@lazyatom

@matthewrudy: it’s being taken care of.

@dpickett

how would migrations work for models like this? they’d probably still have to be written through a generator, right?

@boblmartens

If I knew exactly what this did, I’d be excited.

@ioquatix

DHH: How does this integrate into GEM plugins? GEMs have a directory called “rails” so shouldn’t “app” and so on be in that directory to keep things in the correct namespace? How about rake tasks? You might want to check out my fork of engines which solves these problems and others: http://github.com/ioquatix/engines/tree/master

@stephankaag

How about views in a plugin?

@Roman2K

+1 for <plugin-root>/rails/app instead of <plugin-root>/app (following ioquatix’s comment above).

@matthewrudy

you’re on top of everything @lazyatom.

@oboxodo

+1 on engines-like gems support, still supporting rails plugins, but discouraging them.

@m3talsmith

-1 for /rails/app instead of /app +1 for bringing engines back

I love having all of the above and the gem dependency options open. I love where rails is headed. Keep it up core.

@ioquatix

m3talsmith: can you clarify your -1 comment? Since everything else for rails is in a subdirectory, why not put app there as well. Its much cleaner overall.

@m3talsmith

@ioquatix:

I’m more concerned about keeping a pure directory structure in the vendor/plugins/your_plugin_or_engine_name area. When I made the minus comment I hadn’t was in a rush on my way out the door. I overlooked the GEM’s part of the comment. Given that I take my -1 back because it does make sense to keep it in the same namespace. So ….

+1 for /rails/app for the GEMS directory -2 for m3talsmith firing off a comment on the run.

@railsdog

Will this be like extensions in Radiant or Spree? Both of these projects already support views, controllers, routes, migrations, etc. in your extensions.

@xtoddx

Is there an easy way to run migrations from plugins? Maybe being able to set the schema migrations table name from within the migrations would be enough. I’ve been doing it with monkeypatching and a custom rake task so far.

@dhh
Owner

xtoddx, we’re working on getting a good story for both migrations and files in public in before this is released.

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