Rails Ruby on Rails → 3.0.2

Comments and changes to this ticket

• You flagged this item as spam.
  

  Paul Rosania January 27th, 2010 @ 06:30 AM

  Patch 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.

• You flagged this item as spam.
  

  Paul Rosania January 27th, 2010 @ 01:41 PM

  • 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 January 28th, 2010 @ 04:14 PM

  • 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]>.
  

• You flagged this item as spam.
  

  Paul Rosania January 28th, 2010 @ 04:25 PM

  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.

• You flagged this item as spam.
  

  Paul Rosania January 28th, 2010 @ 09:56 PM

  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.4 KB
• 

  Anuj Dutta January 29th, 2010 @ 12:59 AM

  • 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 '
  
  

• You flagged this item as spam.
  

  Paul Rosania January 29th, 2010 @ 02:29 AM

  Okay. Here is one that definitely clears all Railties tests. Sorry for the misfire!

  • initializer_tsort_v3.diff 5.9 KB
• 

  Anuj Dutta January 29th, 2010 @ 03:16 AM

  +1 Patch applies cleanly and all railties test pass.

• 

  José Valim February 23rd, 2010 @ 09:40 PM

  • 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.

• You flagged this item as spam.
  

  Dan Kubb (dkubb) March 16th, 2010 @ 10:42 PM

  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).

• 

  Jeremy Kemper October 15th, 2010 @ 11:01 PM

  • Milestone set to “3.0.2”
  • Importance changed from “” to “Medium”

  [bulk edit]