[PATCH] remove hook that reloads routes after adding custom inflections

+1, tests pass. Very useful, I remember seeing this happen to me a few times but never thought of fixing it.

Updated for purely cosmetic reasons:

Inflector.module_eval do
  def inflections_with_route_reloading(&block)
    returning(inflections_without_route_reloading(&block)) {
      ActionController::Routing::Routes.reload! if block_given?
    }
  end

  alias_method_chain :inflections, :route_reloading
end

+1, this works great for me. My uncountable resource routes finally work like they should without hacks. Tests pass.

I got a hunk patching, so here's a new patch for trunk as of [7816].

updated against [7816]

(In [7849]) Make sure that custom inflections are picked up by map.resources by triggering a routing reload when new inflections are defined. Closes #9815 [mislav, kampers]

This patch makes it difficult to add custom routes in a plugin because it alters order in which routing is initialized with respect to plugins.

In a typical situation rails will load plugins and then setup routes. This makes it possible for things like Jamis Buck's routing tricks http://weblog.jamisbuck.org/2006/10/26/monkey-patching-rails-extending-routes-2 plugin.

Let's say we have 2 plugins - PluginA and PluginZ - and the standard inflector initializer. They will be loaded by default in this order:

• PluginA
• PluginZ
• Inflector
• Routes#reload

Let's say PluginA defines a custom inflection like so:

Inflector.inflections do |inflect|
  inflect.uncountable 'something'
end

PluginZ defines a custom route mapper, like so:

module Routing
  module MapperExtensions
    def my_custom_route
      @set.add_route("/my_route", {:controller => "my_controller"})
    end
  end
end
ActionController::Routing::RouteSet::Mapper.send :include, Routing::MapperExtensions

Before this patch rails would:

• call PluginA's inflector
• mixes in PluginZ's route mapper
• install the routes

So all is well. Inflections get called before routes. Routes get called after the custom module is mixed in. However, after this patch, rails:

• calls PluginA's inflector
• loads the routes via load! without mixing in PluginZ's mapper
• mixes in PluginZ's route mapper

Note that this is not a problem with standard inflections because they load after the plugin mixins have been processed.

When the person tries to run the app with map.my_custom_route in routes.rb it fails. #6829 seems like it was fixed by moving environment configuration to the initializers directory. However, the failure is mysterious and this patch doesn't seem to actually fix anything anymore.

For the moment, if this happens to anyone else you can set the routing plugin before the plugin with the inflector so it's mixed in before the routes are called if the routing plugin doesn't depend on anything in the inflector plugin.

As a permanent fix I recommend that the core team:

• Roll back this patch
• Close #6829 as invalid

I'm attaching a patch that removes the offending code and the test for the offending code.

Removes the changes made in the previous patch

I just closed #6829, this ticket that this originally referenced and verified that it is no longer an issue.

I just bumped into this exact issue and anybody using Mephisto with plugins that add custom routes is also likely to experience it.

What's happening is that the in-built inflections end up loading the routes earlier than designed to in Initializer. Eventually when initialize_routing is called in Initializer it sees that routes haven't changed so they don't get reloaded with routes from plugins.

Personally, I think the best solution would be to add a comment to initializers/inflections.rb something like this:

# Uncomment this if your custom inflections are used your routes
# ActionController::Routing::Routes.reload!

Otherwise patching Inflections to automatically reload routes IMO violates the principle of least surprise.

In addition to violating the principle of least surprise, it is completely unnecessary for inflections to call reload! now. Initializers fixed the problem, but the original patch (inflector-route-reloading.2.diff) wasn't rolled back.

So rather than add a comment about inflections, I think we should just remove the problem (which I did in remove_inflector_route_reloading.diff above).

I'm running into this issue with Rails Engines -- custom route mapper being loaded halfway through plugin loading. Luckily the call to Inflector isn't really used (it's in ActiveMerchant, so I can just not call it), but it seems like this is a terrible idea. Also, the original patch seemed to be in totally the wrong place, making it pretty difficult to fix without modifying the rails code.

Maybe this should be made into a new bug, and tracked there?

As far as I'm concerned the bug is fixed and this is a great place to track it. My patch (http://dev.rubyonrails.org/attachment/ticket/9815/remove_inflector_route_reloading.diff) removes the offending code, and everything goes back to normal.

If anyone on core sees this, please give a +1. It's a simple patch, includes updates to the tests.

This functionality was added for a reason so removing it without an alternative fix isn't a good option.

Replying to nzkoz:

This functionality was added for a reason so removing it without an alternative fix isn't a good option.

It seems to me that this patch never should have been accepted anyway -- it breaks the load order of plugins. Evidently the tests that passed weren't the right set of tests. The alternative seems to be to manually reload the routes after doing inflections; there is no equivalent alternative to make plugins work properly as the code stands currently. The only way to make the plugin/route load order correct again is to rollback this change.

Replying to nzkoz:

This functionality was added for a reason so removing it without an alternative fix isn't a good option.

Here's the history of what happened:

* at one point you could not use custom inflections in routes * mislav submitted a patch that allowed routes to pick up your custom inflections, but it introduced a few subtle and very difficult to track down bugs * rails core added config/initializers and changed the load order _which fixed the original problem_ * I added a patch that rolled back mislav's code, and removed his tests

So nzkoz - the code was added for a reason, and that reason is no longer necessary. An unrelated update to rails core fixed the issue that mislav's patch attempted to fix.

http://dev.rubyonrails.org/attachment/ticket/9815/remove_inflector_route_reloading.diff roles mislav's changes back, and makes everything work again.

To be clear, mislav's patch only serves to create bugs at this point. It is totally unnecessary. My patch (attached above) fixes this.

Please read my previous comments for a detailed description of why mislav's patch is unnecessary now, and how removing it does not remove any functionality - but does remove bugs.

(sorry for the formatting on that last one - the history is:)

• at one point you could not use custom inflections in routes
• mislav submitted a patch that allowed routes to pick up your custom inflections, but it introduced a few subtle and very difficult to track down bugs
• rails core added config/initializers and changed the load order _which fixed the original problem_
• I added a patch that rolled back mislav's code, and removed his tests

Thanks for clearing that up, I don't see the patch removing any tests though?

See the section title "actionpack/test/controller/routing_test.rb" on http://dev.rubyonrails.org/attachment/ticket/9815/remove_inflector_route_reloading.diff

The patch removes "test_adding_inflections_forces_reload" (removed code is in red).

+1

+1 for removing the hook.

The new load order in Initializer really obsoletes this. When I really need to alter the routes based on custom inflections after the Initializer has finished, I can always manually call ActionController::Routing::Routes.reload!

This appears to have been resolved in git.