This project is archived and is in readonly mode.
[PATCH] Rails 3.0.pre initializers do not always bind in the proper order
-
Paul RosaniaPatch attached for discussion.
I have modified the dependency resolution algorithm to use topographical sort (leveraging tsort from the Ruby standard library).
Following the principle of least surprise, and to satisfy pre-existing tests, initializer declarations now implicitly create an :after dependency on the prior initializer in the class (if there is one, and no explicit :after is stated).
However, because of the nature of the dependency graph created during boot (DAG), it is not guaranteed that an initializer with an :after (s/after/before) will run immediately after the specified initializer dependency. Therefore, it is necessary in these rare cases to be explicit in stating both a :before and :after condition to make sure execution happens as desired. I have updated the tests to reflect this situation.
-
Paul Rosania
- Title changed from Rails 3.0.pre initializers do not always bind in the proper order to [PATCH] Rails 3.0.pre initializers do not always bind in the proper order
-
Jeremy Kemper- State changed from new to open
- Assigned user set to Yehuda Katz (wycats)
- Milestone cleared.
Good thinking, Paul! I get a test failure though:
1) Failure: test_database_middleware_initializes_when_session_store_is_active_record(ApplicationTests::FrameworlsTest) [test/application/initializers/frameworks_test.rb:71]: <[ActiveRecord::ConnectionAdapters::ConnectionManagement, ActiveRecord::QueryCache, ActiveRecord::SessionStore]> expected but was <[ActiveRecord::SessionStore, ActiveRecord::ConnectionAdapters::ConnectionManagement, ActiveRecord::QueryCache]>.
-
Paul Rosania
Hmm. This is a side effect of the sort. Any test checking whether initializers loaded or not will need to be agnostic to the exact order. I will take a look and revise the test accordingly.
-
Paul Rosania
Updated patch attached.
The new initializer method highlights an implicit :before dependency in the AR initializers. I made the dependency explicit, which resolves the outstanding test case.
I should note that while the tsort method is much more robust, it necessarily increases the chances that implicit dependencies will break. Primarily, this impacts how one thinks about what a :before dependencies means.
For example, under the proposed system, :after dependencies guarantee that the referenced initializer has been run. They do not guarantee that your initializer will run immediately afterward; in fact it could run at any point onward. A :before dependency is required if you want to box execution very specifically.
-
initializer_tsort_v2.diff
5.39 KB
-
-
Anuj Dutta- Tag cleared.
Hey Paul,
Got this Error:
test_plugin_init_is_ran_before_application_ones(RailtiesTest::PluginTest): RuntimeError: no $foo /Users/andhapp/Projects/git-repository/andhapp-rails/railties/tmp/app/config/initializers/foo.rb:1:in `' /Users/andhapp/Projects/git-repository/andhapp-rails/activesupport/lib/active_support/dependencies.rb:156:in `load' /Users/andhapp/Projects/git-repository/andhapp-rails/activesupport/lib/active_support/dependencies.rb:156:in `block in load_with_new_constant_marking' /Users/andhapp/Projects/git-repository/andhapp-rails/activesupport/lib/active_support/dependencies.rb:537:in `new_constants_in' /Users/andhapp/Projects/git-repository/andhapp-rails/activesupport/lib/active_support/dependencies.rb:156:in `load_with_new_constant_marking' /Users/andhapp/Projects/git-repository/andhapp-rails/railties/lib/rails/engine.rb:107:in `block (2 levels) in ' /Users/andhapp/Projects/git-repository/andhapp-rails/railties/lib/rails/engine.rb:106:in `each' /Users/andhapp/Projects/git-repository/andhapp-rails/railties/lib/rails/engine.rb:106:in `block in ' /Users/andhapp/Projects/git-repository/andhapp-rails/railties/lib/rails/initializable.rb:25:in `instance_exec' /Users/andhapp/Projects/git-repository/andhapp-rails/railties/lib/rails/initializable.rb:25:in `run' /Users/andhapp/Projects/git-repository/andhapp-rails/railties/lib/rails/initializable.rb:55:in `block in run_initializers' /Users/andhapp/Projects/git-repository/andhapp-rails/railties/lib/rails/initializable.rb:54:in `each' /Users/andhapp/Projects/git-repository/andhapp-rails/railties/lib/rails/initializable.rb:54:in `run_initializers' /Users/andhapp/Projects/git-repository/andhapp-rails/railties/lib/rails/application.rb:71:in `initialize!' /Users/andhapp/Projects/git-repository/andhapp-rails/railties/lib/rails/application.rb:41:in `method_missing' /Users/andhapp/Projects/git-repository/andhapp-rails/railties/tmp/app/config/environment.rb:5:in `' /Users/andhapp/Projects/git-repository/andhapp-rails/railties/test/railties/shared_tests.rb:6:in `require' /Users/andhapp/Projects/git-repository/andhapp-rails/railties/test/railties/shared_tests.rb:6:in `boot_rails' test/railties/plugin_test.rb:60:in `block in ' -
Paul Rosania
Okay. Here is one that definitely clears all Railties tests. Sorry for the misfire!
-
initializer_tsort_v3.diff
5.94 KB
-
-
Anuj Dutta
+1 Patch applies cleanly and all railties test pass.
-
José Valim- State changed from open to resolved
- Assigned user changed from Yehuda Katz (wycats) to José Valim
I guess this one was already applied. Please tell me if I'm wrong.
-
Dan Kubb (dkubb)FWIW, I've found it possible to do a stable topological sort using tsort by yielding to the nodes in the order they were added. So if two nodes are equivalent, the original order will be preserved.
I had track what order things were inserted in and sort by this order in tsort_each_node and tsort_each_child, but otherwise it was relatively simple change.
Of course the extra sort is slightly less efficient than a pure tsort, I think that it matches most people's mental model of how dependencies should be resolved (i.e. things given equivalent weight in the sort would be processed in the order they were added).
-