Permalink
Browse files

Merge branch 'master' of github.com:lifo/docrails

  • Loading branch information...
2 parents 547407a + 7c68072 commit 2fc32636dc07cd4986e065be2ab3fbded34cbe18 @vijaydev vijaydev committed Mar 26, 2011
Showing with 2,115 additions and 656 deletions.
  1. +3 −0 Gemfile
  2. +1 −1 actionpack/lib/abstract_controller/callbacks.rb
  3. +6 −7 actionpack/lib/action_controller/caching/actions.rb
  4. +1 −1 actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb
  5. +6 −5 actionpack/lib/action_dispatch/routing/mapper.rb
  6. +13 −10 actionpack/lib/action_dispatch/routing/route.rb
  7. +1 −1 actionpack/lib/action_view/helpers/number_helper.rb
  8. +86 −19 actionpack/lib/action_view/template/resolver.rb
  9. +6 −6 actionpack/lib/action_view/testing/resolvers.rb
  10. +7 −0 actionpack/test/action_dispatch/routing/mapper_test.rb
  11. +27 −0 actionpack/test/controller/filters_test.rb
  12. +16 −0 actionpack/test/dispatch/show_exceptions_test.rb
  13. +1 −0 actionpack/test/fixtures/custom_pattern/another.html.erb
  14. +1 −0 actionpack/test/fixtures/custom_pattern/html/another.erb
  15. +1 −0 actionpack/test/fixtures/custom_pattern/html/path.erb
  16. +1 −0 actionpack/test/fixtures/filter_test/implicit_actions/edit.html.erb
  17. +1 −0 actionpack/test/fixtures/filter_test/implicit_actions/show.html.erb
  18. +3 −1 actionpack/test/template/number_helper_test.rb
  19. +2 −2 actionpack/test/template/render_test.rb
  20. +31 −0 actionpack/test/template/resolver_patterns_test.rb
  21. +8 −5 activemodel/lib/active_model/attribute_methods.rb
  22. +2 −2 activemodel/lib/active_model/lint.rb
  23. +56 −3 activemodel/test/cases/attribute_methods_test.rb
  24. +6 −0 activerecord/CHANGELOG
  25. +61 −17 activerecord/lib/active_record/associations.rb
  26. +85 −0 activerecord/lib/active_record/associations/alias_tracker.rb
  27. +6 −38 activerecord/lib/active_record/associations/association.rb
  28. +120 −0 activerecord/lib/active_record/associations/association_scope.rb
  29. +12 −12 activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
  30. +0 −13 activerecord/lib/active_record/associations/collection_association.rb
  31. +0 −22 activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
  32. +0 −2 activerecord/lib/active_record/associations/has_many_association.rb
  33. +6 −0 activerecord/lib/active_record/associations/has_many_through_association.rb
  34. +0 −6 activerecord/lib/active_record/associations/has_one_association.rb
  35. +2 −0 activerecord/lib/active_record/associations/has_one_through_association.rb
  36. +8 −24 activerecord/lib/active_record/associations/join_dependency.rb
  37. +66 −201 activerecord/lib/active_record/associations/join_dependency/join_association.rb
  38. +56 −0 activerecord/lib/active_record/associations/join_helper.rb
  39. +3 −2 activerecord/lib/active_record/associations/preloader/through_association.rb
  40. +16 −94 activerecord/lib/active_record/associations/through_association.rb
  41. +8 −1 activerecord/lib/active_record/attribute_methods/read.rb
  42. +5 −4 activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb
  43. +7 −1 activerecord/lib/active_record/attribute_methods/write.rb
  44. +3 −3 activerecord/lib/active_record/base.rb
  45. +4 −1 activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
  46. +16 −2 activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
  47. +4 −3 activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
  48. +8 −0 activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
  49. +4 −0 activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
  50. +1 −9 activerecord/lib/active_record/persistence.rb
  51. +101 −17 activerecord/lib/active_record/reflection.rb
  52. +14 −3 activerecord/lib/active_record/relation.rb
  53. +12 −0 activerecord/lib/active_record/relation/finder_methods.rb
  54. +1 −1 activerecord/lib/active_record/relation/query_methods.rb
  55. +6 −0 activerecord/lib/active_record/relation/spawn_methods.rb
  56. +36 −0 activerecord/test/cases/adapters/mysql/schema_test.rb
  57. +19 −0 activerecord/test/cases/adapters/sqlite/sqlite_adapter_test.rb
  58. +15 −15 activerecord/test/cases/associations/cascaded_eager_loading_test.rb
  59. +6 −6 activerecord/test/cases/associations/eager_test.rb
  60. +3 −3 activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
  61. +25 −5 activerecord/test/cases/associations/has_many_through_associations_test.rb
  62. +23 −1 activerecord/test/cases/associations/has_one_through_associations_test.rb
  63. +3 −10 activerecord/test/cases/associations/join_model_test.rb
  64. +546 −0 activerecord/test/cases/associations/nested_through_associations_test.rb
  65. +10 −14 activerecord/test/cases/attribute_methods_test.rb
  66. +12 −0 activerecord/test/cases/base_test.rb
  67. +1 −1 activerecord/test/cases/batches_test.rb
  68. +28 −2 activerecord/test/cases/finder_test.rb
  69. +34 −0 activerecord/test/cases/habtm_destroy_order_test.rb
  70. +9 −9 activerecord/test/cases/identity_map_test.rb
  71. +6 −2 activerecord/test/cases/json_serialization_test.rb
  72. +54 −3 activerecord/test/cases/reflection_test.rb
  73. +1 −1 activerecord/test/cases/relation_scoping_test.rb
  74. +28 −20 activerecord/test/cases/relations_test.rb
  75. +6 −0 activerecord/test/fixtures/authors.yml
  76. +2 −0 activerecord/test/fixtures/books.yml
  77. +5 −0 activerecord/test/fixtures/categories.yml
  78. +8 −0 activerecord/test/fixtures/categories_posts.yml
  79. +6 −0 activerecord/test/fixtures/categorizations.yml
  80. +3 −1 activerecord/test/fixtures/clubs.yml
  81. +6 −0 activerecord/test/fixtures/essays.yml
  82. +8 −0 activerecord/test/fixtures/member_details.yml
  83. +3 −0 activerecord/test/fixtures/members.yml
  84. +7 −0 activerecord/test/fixtures/memberships.yml
  85. +1 −0 activerecord/test/fixtures/owners.yml
  86. +28 −0 activerecord/test/fixtures/posts.yml
  87. +14 −0 activerecord/test/fixtures/ratings.yml
  88. +50 −0 activerecord/test/fixtures/taggings.yml
  89. +5 −1 activerecord/test/fixtures/tags.yml
  90. +38 −4 activerecord/test/models/author.rb
  91. +2 −0 activerecord/test/models/book.rb
  92. +2 −0 activerecord/test/models/categorization.rb
  93. +2 −0 activerecord/test/models/category.rb
  94. +1 −0 activerecord/test/models/club.rb
  95. +1 −0 activerecord/test/models/comment.rb
  96. +2 −0 activerecord/test/models/essay.rb
  97. +2 −0 activerecord/test/models/job.rb
  98. +11 −0 activerecord/test/models/member.rb
  99. +2 −0 activerecord/test/models/member_detail.rb
  100. +7 −1 activerecord/test/models/organization.rb
  101. +3 −0 activerecord/test/models/person.rb
  102. +10 −1 activerecord/test/models/post.rb
  103. +4 −0 activerecord/test/models/rating.rb
  104. +2 −0 activerecord/test/models/reference.rb
  105. +2 −0 activerecord/test/models/tagging.rb
  106. +16 −0 activerecord/test/schema/schema.rb
  107. +7 −5 activesupport/lib/active_support/json/backends/yaml.rb
  108. +4 −4 activesupport/test/json/decoding_test.rb
  109. +51 −1 railties/guides/source/active_record_querying.textile
  110. +2 −2 railties/guides/source/active_record_validations_callbacks.textile
  111. +1 −1 railties/guides/source/caching_with_rails.textile
  112. +1 −1 railties/guides/source/getting_started.textile
  113. +1 −1 railties/lib/rails/generators/rails/plugin_new/plugin_new_generator.rb
  114. +8 −0 railties/lib/rails/railtie/configuration.rb
  115. +1 −2 railties/test/generators/shared_generator_tests.rb
View
3 Gemfile
@@ -36,6 +36,9 @@ platforms :mri_19 do
end
platforms :ruby do
+ if ENV["RB_FSEVENT"]
+ gem 'rb-fsevent'
+ end
gem 'json'
gem 'yajl-ruby'
gem "nokogiri", ">= 1.4.4"
View
2 actionpack/lib/abstract_controller/callbacks.rb
@@ -14,7 +14,7 @@ module Callbacks
# Override AbstractController::Base's process_action to run the
# process_action callbacks around the normal behavior.
def process_action(method_name, *args)
- run_callbacks(:process_action, method_name) do
+ run_callbacks(:process_action, action_name) do
super
end
end
View
13 actionpack/lib/action_controller/caching/actions.rb
@@ -56,19 +56,18 @@ module Caching
#
# caches_page :public
#
- # caches_action :index, :if => proc do |c|
- # !c.request.format.json? # cache if is not a JSON request
+ # caches_action :index, :if => proc do
+ # !request.format.json? # cache if is not a JSON request
# end
#
# caches_action :show, :cache_path => { :project => 1 },
# :expires_in => 1.hour
#
- # caches_action :feed, :cache_path => proc do |c|
- # if c.params[:user_id]
- # c.send(:user_list_url,
- # c.params[:user_id], c.params[:id])
+ # caches_action :feed, :cache_path => proc do
+ # if params[:user_id]
+ # user_list_url(params[:user_id, params[:id])
# else
- # c.send(:list_url, c.params[:id])
+ # list_url(params[:id])
# end
# end
# end
View
2 actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb
@@ -1,7 +1,7 @@
<h1>
<%=h @exception.class.to_s %>
<% if @request.parameters['controller'] %>
- in <%=h @request.parameters['controller'].humanize %>Controller<% if @request.parameters['action'] %>#<%=h @request.parameters['action'] %><% end %>
+ in <%=h @request.parameters['controller'].classify.pluralize %>Controller<% if @request.parameters['action'] %>#<%=h @request.parameters['action'] %><% end %>
<% end %>
</h1>
<pre><%=h @exception.message %></pre>
View
11 actionpack/lib/action_dispatch/routing/mapper.rb
@@ -107,7 +107,7 @@ def normalize_path(path)
if @options[:format] == false
@options.delete(:format)
path
- elsif path.include?(":format") || path.end_with?('/')
+ elsif path.include?(":format") || path.end_with?('/') || path.match(/^\/?\*/)
path
else
"#{path}(.:format)"
@@ -195,8 +195,8 @@ def constraints
def request_method_condition
if via = @options[:via]
- via = Array(via).map { |m| m.to_s.dasherize.upcase }
- { :request_method => %r[^#{via.join('|')}$] }
+ list = Array(via).map { |m| m.to_s.dasherize.upcase }
+ { :request_method => list }
else
{ }
end
@@ -372,8 +372,9 @@ def root(options = {})
# # Matches any request starting with 'path'
# match 'path' => 'c#a', :anchor => false
def match(path, options=nil)
- mapping = Mapping.new(@set, @scope, path, options || {}).to_route
- @set.add_route(*mapping)
+ mapping = Mapping.new(@set, @scope, path, options || {})
+ app, conditions, requirements, defaults, as, anchor = mapping.to_route
+ @set.add_route(app, conditions, requirements, defaults, as, anchor)
self
end
View
23 actionpack/lib/action_dispatch/routing/route.rb
@@ -12,6 +12,8 @@ def initialize(set, app, conditions, requirements, defaults, name, anchor)
@defaults = defaults
@name = name
+ # FIXME: we should not be doing this much work in a constructor.
+
@requirements = requirements.merge(defaults)
@requirements.delete(:controller) if @requirements[:controller].is_a?(Regexp)
@requirements.delete_if { |k, v|
@@ -23,21 +25,22 @@ def initialize(set, app, conditions, requirements, defaults, name, anchor)
conditions[:path_info] = ::Rack::Mount::Strexp.compile(path, requirements, SEPARATORS, anchor)
end
- @conditions = Hash[conditions.map { |k,v| [k, Rack::Mount::RegexpWithNamedGroups.new(v)] }]
+ @verbs = conditions[:request_method] || []
+
+ @conditions = conditions.dup
+
+ # Rack-Mount requires that :request_method be a regular expression.
+ # :request_method represents the HTTP verb that matches this route.
+ #
+ # Here we munge values before they get sent on to rack-mount.
+ @conditions[:request_method] = %r[^#{verb}$] unless @verbs.empty?
+ @conditions[:path_info] = Rack::Mount::RegexpWithNamedGroups.new(@conditions[:path_info]) if @conditions[:path_info]
@conditions.delete_if{ |k,v| k != :path_info && !valid_condition?(k) }
@requirements.delete_if{ |k,v| !valid_condition?(k) }
end
def verb
- if method = conditions[:request_method]
- case method
- when Regexp
- source = method.source.upcase
- source =~ /\A\^[-A-Z|]+\$\Z/ ? source[1..-2] : source
- else
- method.to_s.upcase
- end
- end
+ @verbs.join '|'
end
def segment_keys
View
2 actionpack/lib/action_view/helpers/number_helper.rb
@@ -472,7 +472,7 @@ def number_to_human(number, options = {})
end.keys.map{|e_name| inverted_du[e_name] }.sort_by{|e| -e}
number_exponent = number != 0 ? Math.log10(number.abs).floor : 0
- display_exponent = unit_exponents.find{|e| number_exponent >= e }
+ display_exponent = unit_exponents.find{ |e| number_exponent >= e } || 0
number /= 10 ** display_exponent
unit = case units
View
105 actionpack/lib/action_view/template/resolver.rb
@@ -5,6 +5,25 @@
module ActionView
# = Action View Resolver
class Resolver
+ # Keeps all information about view path and builds virtual path.
+ class Path < String
+ attr_reader :name, :prefix, :partial, :virtual
+ alias_method :partial?, :partial
+
+ def initialize(name, prefix, partial)
+ @name, @prefix, @partial = name, prefix, partial
+ rebuild(@name, @prefix, @partial)
+ end
+
+ def rebuild(name, prefix, partial)
+ @virtual = ""
+ @virtual << "#{prefix}/" unless prefix.empty?
+ @virtual << (partial ? "_#{name}" : name)
+
+ self.replace(@virtual)
+ end
+ end
+
cattr_accessor :caching
self.caching = true
@@ -41,10 +60,7 @@ def find_templates(name, prefix, partial, details)
# Helpers that builds a path. Useful for building virtual paths.
def build_path(name, prefix, partial)
- path = ""
- path << "#{prefix}/" unless prefix.empty?
- path << (partial ? "_#{name}" : name)
- path
+ Path.new(name, prefix, partial)
end
# Handles templates caching. If a key is given and caching is on
@@ -97,25 +113,24 @@ def sort_locals(locals) #:nodoc:
end
class PathResolver < Resolver
- EXTENSION_ORDER = [:locale, :formats, :handlers]
+ EXTENSIONS = [:locale, :formats, :handlers]
+ DEFAULT_PATTERN = ":prefix/:action{.:locale,}{.:formats,}{.:handlers,}"
+
+ def initialize(pattern=nil)
+ @pattern = pattern || DEFAULT_PATTERN
+ super()
+ end
private
def find_templates(name, prefix, partial, details)
path = build_path(name, prefix, partial)
- query(path, EXTENSION_ORDER.map { |ext| details[ext] }, details[:formats])
+ extensions = Hash[EXTENSIONS.map { |ext| [ext, details[ext]] }.flatten(0)]
+ query(path, extensions, details[:formats])
end
def query(path, exts, formats)
- query = File.join(@path, path)
-
- query << exts.map { |ext|
- "{#{ext.compact.map { |e| ".#{e}" }.join(',')},}"
- }.join
-
- query.gsub!(/\{\.html,/, "{.html,.text.html,")
- query.gsub!(/\{\.text,/, "{.text,.text.plain,")
-
+ query = build_query(path, exts)
templates = []
sanitizer = Hash.new { |h,k| h[k] = Dir["#{File.dirname(k)}/*"] }
@@ -126,12 +141,28 @@ def query(path, exts, formats)
contents = File.open(p, "rb") {|io| io.read }
templates << Template.new(contents, File.expand_path(p), handler,
- :virtual_path => path, :format => format, :updated_at => mtime(p))
+ :virtual_path => path.virtual, :format => format, :updated_at => mtime(p))
end
templates
end
+ # Helper for building query glob string based on resolver's pattern.
+ def build_query(path, exts)
+ query = @pattern.dup
+ query.gsub!(/\:prefix(\/)?/, path.prefix.empty? ? "" : "#{path.prefix}\\1") # prefix can be empty...
+ query.gsub!(/\:action/, path.partial? ? "_#{path.name}" : path.name)
+
+ exts.each { |ext, variants|
+ query.gsub!(/\:#{ext}/, "{#{variants.compact.uniq.join(',')}}")
+ }
+
+ query.gsub!(/\.{html,/, ".{html,text.html,")
+ query.gsub!(/\.{text,/, ".{text,text.plain,")
+
+ File.expand_path(query, @path)
+ end
+
# Returns the file mtime from the filesystem.
def mtime(p)
File.stat(p).mtime
@@ -149,11 +180,47 @@ def extract_handler_and_format(path, default_formats)
end
end
- # A resolver that loads files from the filesystem.
+ # A resolver that loads files from the filesystem. It allows to set your own
+ # resolving pattern. Such pattern can be a glob string supported by some variables.
+ #
+ # ==== Examples
+ #
+ # Default pattern, loads views the same way as previous versions of rails, eg. when you're
+ # looking for `users/new` it will produce query glob: `users/new{.{en},}{.{html,js},}{.{erb,haml,rjs},}`
+ #
+ # FileSystemResolver.new("/path/to/views", ":prefix/:action{.:locale,}{.:formats,}{.:handlers,}")
+ #
+ # This one allows you to keep files with different formats in seperated subdirectories,
+ # eg. `users/new.html` will be loaded from `users/html/new.erb` or `users/new.html.erb`,
+ # `users/new.js` from `users/js/new.erb` or `users/new.js.erb`, etc.
+ #
+ # FileSystemResolver.new("/path/to/views", ":prefix/{:formats/,}:action{.:locale,}{.:formats,}{.:handlers,}")
+ #
+ # If you don't specify pattern then the default will be used.
+ #
+ # In order to use any of the customized resolvers above in a Rails application, you just need
+ # to configure ActionController::Base.view_paths in an initializer, for example:
+ #
+ # ActionController::Base.view_paths = FileSystemResolver.new(
+ # Rails.root.join("app/views"),
+ # ":prefix{/:locale}/:action{.:formats,}{.:handlers,}"
+ # )
+ #
+ # ==== Pattern format and variables
+ #
+ # Pattern have to be a valid glob string, and it allows you to use the
+ # following variables:
+ #
+ # * <tt>:prefix</tt> - usualy the controller path
+ # * <tt>:action</tt> - name of the action
+ # * <tt>:locale</tt> - possible locale versions
+ # * <tt>:formats</tt> - possible request formats (for example html, json, xml...)
+ # * <tt>:handlers</tt> - possible handlers (for example erb, haml, builder...)
+ #
class FileSystemResolver < PathResolver
- def initialize(path)
+ def initialize(path, pattern=nil)
raise ArgumentError, "path already is a Resolver class" if path.is_a?(Resolver)
- super()
+ super(pattern)
@path = File.expand_path(path)
end
View
12 actionpack/lib/action_view/testing/resolvers.rb
@@ -8,8 +8,8 @@ module ActionView #:nodoc:
class FixtureResolver < PathResolver
attr_reader :hash
- def initialize(hash = {})
- super()
+ def initialize(hash = {}, pattern=nil)
+ super(pattern)
@hash = hash
end
@@ -21,8 +21,8 @@ def to_s
def query(path, exts, formats)
query = ""
- exts.each do |ext|
- query << '(' << ext.map {|e| e && Regexp.escape(".#{e}") }.join('|') << '|)'
+ EXTENSIONS.each do |ext|
+ query << '(' << exts[ext].map {|e| e && Regexp.escape(".#{e}") }.join('|') << '|)'
end
query = /^(#{Regexp.escape(path)})#{query}$/
@@ -32,9 +32,9 @@ def query(path, exts, formats)
next unless _path =~ query
handler, format = extract_handler_and_format(_path, formats)
templates << Template.new(source, _path, handler,
- :virtual_path => $1, :format => format, :updated_at => updated_at)
+ :virtual_path => path.virtual, :format => format, :updated_at => updated_at)
end
-
+
templates.sort_by {|t| -t.identifier.match(/^#{query}$/).captures.reject(&:blank?).size }
end
end
View
7 actionpack/test/action_dispatch/routing/mapper_test.rb
@@ -46,6 +46,13 @@ def test_map_more_slashes
mapper.match '/one/two/', :to => 'posts#index', :as => :main
assert_equal '/one/two(.:format)', fakeset.conditions.first[:path_info]
end
+
+ def test_map_wildcard
+ fakeset = FakeSet.new
+ mapper = Mapper.new fakeset
+ mapper.match '/*path', :to => 'pages#show', :as => :page
+ assert_equal '/*path', fakeset.conditions.first[:path_info]
+ end
end
end
end
View
27 actionpack/test/controller/filters_test.rb
@@ -505,6 +505,21 @@ def show
end
end
+ class ImplicitActionsController < ActionController::Base
+ before_filter :find_only, :only => :edit
+ before_filter :find_except, :except => :edit
+
+ private
+
+ def find_only
+ @only = 'Only'
+ end
+
+ def find_except
+ @except = 'Except'
+ end
+ end
+
def test_sweeper_should_not_block_rendering
response = test_process(SweeperTestController)
assert_equal 'hello world', response.body
@@ -783,6 +798,18 @@ def test_a_rescuing_around_filter
assert_equal("I rescued this: #<FilterTest::ErrorToRescue: Something made the bad noise.>", response.body)
end
+ def test_filters_obey_only_and_except_for_implicit_actions
+ test_process(ImplicitActionsController, 'show')
+ assert_equal 'Except', assigns(:except)
+ assert_nil assigns(:only)
+ assert_equal 'show', response.body
+
+ test_process(ImplicitActionsController, 'edit')
+ assert_equal 'Only', assigns(:only)
+ assert_nil assigns(:except)
+ assert_equal 'edit', response.body
+ end
+
private
def test_process(controller, action = "show")
@controller = controller.is_a?(Class) ? controller.new : controller
View
16 actionpack/test/dispatch/show_exceptions_test.rb
@@ -7,6 +7,8 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest
case req.path
when "/not_found"
raise ActionController::UnknownAction
+ when "/runtime_error"
+ raise RuntimeError
when "/method_not_allowed"
raise ActionController::MethodNotAllowed
when "/not_implemented"
@@ -121,4 +123,18 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest
assert_response 404
assert_match(/AbstractController::ActionNotFound/, body)
end
+
+ test "show the controller name in the diagnostics template when controller name is present" do
+ @app = ProductionApp
+ get("/runtime_error", {}, {
+ 'action_dispatch.show_exceptions' => true,
+ 'action_dispatch.request.parameters' => {
+ 'action' => 'show',
+ 'id' => 'unknown',
+ 'controller' => 'featured_tiles'
+ }
+ })
+ assert_response 500
+ assert_match(/RuntimeError\n in FeaturedTilesController/, body)
+ end
end
View
1 actionpack/test/fixtures/custom_pattern/another.html.erb
@@ -0,0 +1 @@
+Hello custom patterns!
View
1 actionpack/test/fixtures/custom_pattern/html/another.erb
@@ -0,0 +1 @@
+Another template!
View
1 actionpack/test/fixtures/custom_pattern/html/path.erb
@@ -0,0 +1 @@
+Hello custom patterns!
View
1 actionpack/test/fixtures/filter_test/implicit_actions/edit.html.erb
@@ -0,0 +1 @@
+edit
View
1 actionpack/test/fixtures/filter_test/implicit_actions/show.html.erb
@@ -0,0 +1 @@
+show
View
4 actionpack/test/template/number_helper_test.rb
@@ -195,7 +195,9 @@ def test_number_to_human_size_with_custom_delimiter_and_separator
def test_number_to_human
assert_equal '-123', number_to_human(-123)
- assert_equal '0', number_to_human(0)
+ assert_equal '-0.5', number_to_human(-0.5)
+ assert_equal '0', number_to_human(0)
+ assert_equal '0.5', number_to_human(0.5)
assert_equal '123', number_to_human(123)
assert_equal '1.23 Thousand', number_to_human(1234)
assert_equal '12.3 Thousand', number_to_human(12345)
View
4 actionpack/test/template/render_test.rb
@@ -381,7 +381,7 @@ def test_render_utf8_template_with_default_external_encoding
end
def test_render_utf8_template_with_incompatible_external_encoding
- with_external_encoding Encoding::SJIS do
+ with_external_encoding Encoding::SHIFT_JIS do
begin
result = @view.render(:file => "test/utf8.html.erb", :layouts => "layouts/yield")
flunk 'Should have raised incompatible encoding error'
@@ -392,7 +392,7 @@ def test_render_utf8_template_with_incompatible_external_encoding
end
def test_render_utf8_template_with_partial_with_incompatible_encoding
- with_external_encoding Encoding::SJIS do
+ with_external_encoding Encoding::SHIFT_JIS do
begin
result = @view.render(:file => "test/utf8_magic_with_bare_partial.html.erb", :layouts => "layouts/yield")
flunk 'Should have raised incompatible encoding error'
View
31 actionpack/test/template/resolver_patterns_test.rb
@@ -0,0 +1,31 @@
+require 'abstract_unit'
+
+class ResolverPatternsTest < ActiveSupport::TestCase
+ def setup
+ path = File.expand_path("../../fixtures/", __FILE__)
+ pattern = ":prefix/{:formats/,}:action{.:formats,}{.:handlers,}"
+ @resolver = ActionView::FileSystemResolver.new(path, pattern)
+ end
+
+ def test_should_return_empty_list_for_unknown_path
+ templates = @resolver.find_all("unknown", "custom_pattern", false, {:locale => [], :formats => [:html], :handlers => [:erb]})
+ assert_equal [], templates, "expected an empty list of templates"
+ end
+
+ def test_should_return_template_for_declared_path
+ templates = @resolver.find_all("path", "custom_pattern", false, {:locale => [], :formats => [:html], :handlers => [:erb]})
+ assert_equal 1, templates.size, "expected one template"
+ assert_equal "Hello custom patterns!", templates.first.source
+ assert_equal "custom_pattern/path", templates.first.virtual_path
+ assert_equal [:html], templates.first.formats
+ end
+
+ def test_should_return_all_templates_when_ambigous_pattern
+ templates = @resolver.find_all("another", "custom_pattern", false, {:locale => [], :formats => [:html], :handlers => [:erb]})
+ assert_equal 2, templates.size, "expected two templates"
+ assert_equal "Another template!", templates[0].source
+ assert_equal "custom_pattern/another", templates[0].virtual_path
+ assert_equal "Hello custom patterns!", templates[1].source
+ assert_equal "custom_pattern/another", templates[1].virtual_path
+ end
+end
View
13 activemodel/lib/active_model/attribute_methods.rb
@@ -106,11 +106,14 @@ def define_attr_method(name, value=nil, &block)
if block_given?
sing.send :define_method, name, &block
else
- # use eval instead of a block to work around a memory leak in dev
- # mode in fcgi
- sing.class_eval <<-eorb, __FILE__, __LINE__ + 1
- def #{name}; #{value.nil? ? 'nil' : value.to_s.inspect}; end
- eorb
+ if name =~ /^[a-zA-Z_]\w*[!?=]?$/
+ sing.class_eval <<-eorb, __FILE__, __LINE__ + 1
+ def #{name}; #{value.nil? ? 'nil' : value.to_s.inspect}; end
+ eorb
+ else
+ value = value.to_s if value
+ sing.send(:define_method, name) { value }
+ end
end
end
View
4 activemodel/lib/active_model/lint.rb
@@ -23,7 +23,7 @@ module Tests
def test_to_key
assert model.respond_to?(:to_key), "The model should respond to to_key"
def model.persisted?() false end
- assert model.to_key.nil?
+ assert model.to_key.nil?, "to_key should return nil when `persisted?` returns false"
end
# == Responds to <tt>to_param</tt>
@@ -40,7 +40,7 @@ def test_to_param
assert model.respond_to?(:to_param), "The model should respond to to_param"
def model.to_key() [1] end
def model.persisted?() false end
- assert model.to_param.nil?
+ assert model.to_param.nil?, "to_param should return nil when `persisted?` returns false"
end
# == Responds to <tt>valid?</tt>
View
59 activemodel/test/cases/attribute_methods_test.rb
@@ -5,6 +5,12 @@ class ModelWithAttributes
attribute_method_suffix ''
+ class << self
+ define_method(:bar) do
+ 'original bar'
+ end
+ end
+
def attributes
{ :foo => 'value of foo' }
end
@@ -36,6 +42,27 @@ def attribute(name)
end
end
+class ModelWithWeirdNamesAttributes
+ include ActiveModel::AttributeMethods
+
+ attribute_method_suffix ''
+
+ class << self
+ define_method(:'c?d') do
+ 'original c?d'
+ end
+ end
+
+ def attributes
+ { :'a?b' => 'value of a?b' }
+ end
+
+private
+ def attribute(name)
+ attributes[name.to_sym]
+ end
+end
+
class AttributeMethodsTest < ActiveModel::TestCase
test 'unrelated classes should not share attribute method matchers' do
assert_not_equal ModelWithAttributes.send(:attribute_method_matchers),
@@ -49,6 +76,14 @@ class AttributeMethodsTest < ActiveModel::TestCase
assert_equal "value of foo", ModelWithAttributes.new.foo
end
+ test '#define_attribute_method generates attribute method with invalid identifier characters' do
+ ModelWithWeirdNamesAttributes.define_attribute_method(:'a?b')
+ ModelWithWeirdNamesAttributes.define_attribute_method(:'a?b')
+
+ assert_respond_to ModelWithWeirdNamesAttributes.new, :'a?b'
+ assert_equal "value of a?b", ModelWithWeirdNamesAttributes.new.send('a?b')
+ end
+
test '#define_attribute_methods generates attribute methods' do
ModelWithAttributes.define_attribute_methods([:foo])
@@ -58,15 +93,33 @@ class AttributeMethodsTest < ActiveModel::TestCase
test '#define_attribute_methods generates attribute methods with spaces in their names' do
ModelWithAttributesWithSpaces.define_attribute_methods([:'foo bar'])
-
+
assert_respond_to ModelWithAttributesWithSpaces.new, :'foo bar'
assert_equal "value of foo bar", ModelWithAttributesWithSpaces.new.send(:'foo bar')
end
-
+
+ test '#define_attr_method generates attribute method' do
+ ModelWithAttributes.define_attr_method(:bar, 'bar')
+
+ assert_respond_to ModelWithAttributes, :bar
+ assert_equal "original bar", ModelWithAttributes.original_bar
+ assert_equal "bar", ModelWithAttributes.bar
+ ModelWithAttributes.define_attr_method(:bar)
+ assert !ModelWithAttributes.bar
+ end
+
+ test '#define_attr_method generates attribute method with invalid identifier characters' do
+ ModelWithWeirdNamesAttributes.define_attr_method(:'c?d', 'c?d')
+
+ assert_respond_to ModelWithWeirdNamesAttributes, :'c?d'
+ assert_equal "original c?d", ModelWithWeirdNamesAttributes.send('original_c?d')
+ assert_equal "c?d", ModelWithWeirdNamesAttributes.send('c?d')
+ end
+
test '#alias_attribute works with attributes with spaces in their names' do
ModelWithAttributesWithSpaces.define_attribute_methods([:'foo bar'])
ModelWithAttributesWithSpaces.alias_attribute(:'foo_bar', :'foo bar')
-
+
assert_equal "value of foo bar", ModelWithAttributesWithSpaces.new.foo_bar
end
View
6 activerecord/CHANGELOG
@@ -1,5 +1,11 @@
*Rails 3.1.0 (unreleased)*
+* Associations with a :through option can now use *any* association as the
+ through or source association, including other associations which have a
+ :through option and has_and_belongs_to_many associations
+
+ [Jon Leighton]
+
* The configuration for the current database connection is now accessible via
ActiveRecord::Base.connection_config. [fxn]
View
78 activerecord/lib/active_record/associations.rb
@@ -52,14 +52,6 @@ def initialize(reflection)
end
end
- class HasManyThroughSourceAssociationMacroError < ActiveRecordError #:nodoc:
- def initialize(reflection)
- through_reflection = reflection.through_reflection
- source_reflection = reflection.source_reflection
- super("Invalid source reflection macro :#{source_reflection.macro}#{" :through" if source_reflection.options[:through]} for has_many #{reflection.name.inspect}, :through => #{through_reflection.name.inspect}. Use :source to specify the source reflection.")
- end
- end
-
class HasManyThroughCantAssociateThroughHasOneOrManyReflection < ActiveRecordError #:nodoc:
def initialize(owner, reflection)
super("Cannot modify association '#{owner.class.name}##{reflection.name}' because the source reflection class '#{reflection.source_reflection.class_name}' is associated to '#{reflection.through_reflection.class_name}' via :#{reflection.source_reflection.macro}.")
@@ -78,6 +70,12 @@ def initialize(owner, reflection)
end
end
+ class HasManyThroughNestedAssociationsAreReadonly < ActiveRecordError #:nodoc
+ def initialize(owner, reflection)
+ super("Cannot modify association '#{owner.class.name}##{reflection.name}' because it goes through more than one other association.")
+ end
+ end
+
class HasAndBelongsToManyAssociationWithPrimaryKeyError < ActiveRecordError #:nodoc:
def initialize(reflection)
super("Primary key is not allowed in a has_and_belongs_to_many join table (#{reflection.options[:join_table]}).")
@@ -142,8 +140,11 @@ module Builder #:nodoc:
autoload :HasAndBelongsToMany, 'active_record/associations/builder/has_and_belongs_to_many'
end
- autoload :Preloader, 'active_record/associations/preloader'
- autoload :JoinDependency, 'active_record/associations/join_dependency'
+ autoload :Preloader, 'active_record/associations/preloader'
+ autoload :JoinDependency, 'active_record/associations/join_dependency'
+ autoload :AssociationScope, 'active_record/associations/association_scope'
+ autoload :AliasTracker, 'active_record/associations/alias_tracker'
+ autoload :JoinHelper, 'active_record/associations/join_helper'
# Clears out the association cache.
def clear_association_cache #:nodoc:
@@ -548,6 +549,49 @@ def association_instance_set(name, association)
# belongs_to :tag, :inverse_of => :taggings
# end
#
+ # === Nested Associations
+ #
+ # You can actually specify *any* association with the <tt>:through</tt> option, including an
+ # association which has a <tt>:through</tt> option itself. For example:
+ #
+ # class Author < ActiveRecord::Base
+ # has_many :posts
+ # has_many :comments, :through => :posts
+ # has_many :commenters, :through => :comments
+ # end
+ #
+ # class Post < ActiveRecord::Base
+ # has_many :comments
+ # end
+ #
+ # class Comment < ActiveRecord::Base
+ # belongs_to :commenter
+ # end
+ #
+ # @author = Author.first
+ # @author.commenters # => People who commented on posts written by the author
+ #
+ # An equivalent way of setting up this association this would be:
+ #
+ # class Author < ActiveRecord::Base
+ # has_many :posts
+ # has_many :commenters, :through => :posts
+ # end
+ #
+ # class Post < ActiveRecord::Base
+ # has_many :comments
+ # has_many :commenters, :through => :comments
+ # end
+ #
+ # class Comment < ActiveRecord::Base
+ # belongs_to :commenter
+ # end
+ #
+ # When using nested association, you will not be able to modify the association because there
+ # is not enough information to know what modification to make. For example, if you tried to
+ # add a <tt>Commenter</tt> in the example above, there would be no way to tell how to set up the
+ # intermediate <tt>Post</tt> and <tt>Comment</tt> objects.
+ #
# === Polymorphic Associations
#
# Polymorphic associations on models are not restricted on what types of models they
@@ -1068,10 +1112,10 @@ module ClassMethods
# [:as]
# Specifies a polymorphic interface (See <tt>belongs_to</tt>).
# [:through]
- # Specifies a join model through which to perform the query. Options for <tt>:class_name</tt>,
+ # Specifies an association through which to perform the query. This can be any other type
+ # of association, including other <tt>:through</tt> associations. Options for <tt>:class_name</tt>,
# <tt>:primary_key</tt> and <tt>:foreign_key</tt> are ignored, as the association uses the
- # source reflection. You can only use a <tt>:through</tt> query through a <tt>belongs_to</tt>,
- # <tt>has_one</tt> or <tt>has_many</tt> association on the join model.
+ # source reflection.
#
# If the association on the join model is a +belongs_to+, the collection can be modified
# and the records on the <tt>:through</tt> model will be automatically created and removed
@@ -1198,10 +1242,10 @@ def has_many(name, options = {}, &extension)
# you want to do a join but not include the joined columns. Do not forget to include the
# primary and foreign keys, otherwise it will raise an error.
# [:through]
- # Specifies a Join Model through which to perform the query. Options for <tt>:class_name</tt>
- # and <tt>:foreign_key</tt> are ignored, as the association uses the source reflection. You
- # can only use a <tt>:through</tt> query through a <tt>has_one</tt> or <tt>belongs_to</tt>
- # association on the join model.
+ # Specifies a Join Model through which to perform the query. Options for <tt>:class_name</tt>,
+ # <tt>:primary_key</tt>, and <tt>:foreign_key</tt> are ignored, as the association uses the
+ # source reflection. You can only use a <tt>:through</tt> query through a <tt>has_one</tt>
+ # or <tt>belongs_to</tt> association on the join model.
# [:source]
# Specifies the source association name used by <tt>has_one :through</tt> queries.
# Only use it if the name cannot be inferred from the association.
View
85 activerecord/lib/active_record/associations/alias_tracker.rb
@@ -0,0 +1,85 @@
+require 'active_support/core_ext/string/conversions'
+
+module ActiveRecord
+ module Associations
+ # Keeps track of table aliases for ActiveRecord::Associations::ClassMethods::JoinDependency and
+ # ActiveRecord::Associations::ThroughAssociationScope
+ class AliasTracker # :nodoc:
+ attr_reader :aliases, :table_joins
+
+ # table_joins is an array of arel joins which might conflict with the aliases we assign here
+ def initialize(table_joins = [])
+ @aliases = Hash.new
+ @table_joins = table_joins
+ end
+
+ def aliased_table_for(table_name, aliased_name = nil)
+ table_alias = aliased_name_for(table_name, aliased_name)
+
+ if table_alias == table_name
+ Arel::Table.new(table_name)
+ else
+ Arel::Table.new(table_name).alias(table_alias)
+ end
+ end
+
+ def aliased_name_for(table_name, aliased_name = nil)
+ aliased_name ||= table_name
+
+ initialize_count_for(table_name) if aliases[table_name].nil?
+
+ if aliases[table_name].zero?
+ # If it's zero, we can have our table_name
+ aliases[table_name] = 1
+ table_name
+ else
+ # Otherwise, we need to use an alias
+ aliased_name = connection.table_alias_for(aliased_name)
+
+ initialize_count_for(aliased_name) if aliases[aliased_name].nil?
+
+ # Update the count
+ aliases[aliased_name] += 1
+
+ if aliases[aliased_name] > 1
+ "#{truncate(aliased_name)}_#{aliases[aliased_name]}"
+ else
+ aliased_name
+ end
+ end
+ end
+
+ def pluralize(table_name)
+ ActiveRecord::Base.pluralize_table_names ? table_name.to_s.pluralize : table_name
+ end
+
+ private
+
+ def initialize_count_for(name)
+ aliases[name] = 0
+
+ unless Arel::Table === table_joins
+ # quoted_name should be downcased as some database adapters (Oracle) return quoted name in uppercase
+ quoted_name = connection.quote_table_name(name).downcase
+
+ aliases[name] += table_joins.map { |join|
+ # Table names + table aliases
+ join.left.downcase.scan(
+ /join(?:\s+\w+)?\s+(\S+\s+)?#{quoted_name}\son/
+ ).size
+ }.sum
+ end
+
+ aliases[name]
+ end
+
+ def truncate(name)
+ name[0..connection.table_alias_length-3]
+ end
+
+ def connection
+ ActiveRecord::Base.connection
+ end
+ end
+ end
+end
View
44 activerecord/lib/active_record/associations/association.rb
@@ -93,23 +93,9 @@ def scoped
# by scope.scoping { ... } or with_scope { ... } etc, which affects the scope which
# actually gets built.
def construct_scope
- @association_scope = association_scope if klass
- end
-
- def association_scope
- scope = klass.unscoped
- scope = scope.create_with(creation_attributes)
- scope = scope.apply_finder_options(options.slice(:readonly, :include))
- scope = scope.where(interpolate(options[:conditions]))
- if select = select_value
- scope = scope.select(select)
+ if klass
+ @association_scope = AssociationScope.new(self).scope
end
- scope = scope.extending(*Array.wrap(options[:extend]))
- scope.where(construct_owner_conditions)
- end
-
- def aliased_table
- klass.arel_table
end
# Set the inverse association, if possible
@@ -174,42 +160,24 @@ def interpolate(sql, record = nil)
end
end
- def select_value
- options[:select]
- end
-
- # Implemented by (some) subclasses
def creation_attributes
- { }
- end
-
- # Returns a hash linking the owner to the association represented by the reflection
- def construct_owner_attributes(reflection = reflection)
attributes = {}
- if reflection.macro == :belongs_to
- attributes[reflection.association_primary_key] = owner[reflection.foreign_key]
- else
+
+ if [:has_one, :has_many].include?(reflection.macro) && !options[:through]
attributes[reflection.foreign_key] = owner[reflection.active_record_primary_key]
if reflection.options[:as]
attributes[reflection.type] = owner.class.base_class.name
end
end
- attributes
- end
- # Builds an array of arel nodes from the owner attributes hash
- def construct_owner_conditions(table = aliased_table, reflection = reflection)
- conditions = construct_owner_attributes(reflection).map do |attr, value|
- table[attr].eq(value)
- end
- table.create_and(conditions)
+ attributes
end
# Sets the owner attributes on the given record
def set_owner_attributes(record)
if owner.persisted?
- construct_owner_attributes.each { |key, value| record[key] = value }
+ creation_attributes.each { |key, value| record[key] = value }
end
end
View
120 activerecord/lib/active_record/associations/association_scope.rb
@@ -0,0 +1,120 @@
+module ActiveRecord
+ module Associations
+ class AssociationScope #:nodoc:
+ include JoinHelper
+
+ attr_reader :association, :alias_tracker
+
+ delegate :klass, :owner, :reflection, :interpolate, :to => :association
+ delegate :chain, :conditions, :options, :source_options, :active_record, :to => :reflection
+
+ def initialize(association)
+ @association = association
+ @alias_tracker = AliasTracker.new
+ end
+
+ def scope
+ scope = klass.unscoped
+ scope = scope.extending(*Array.wrap(options[:extend]))
+
+ # It's okay to just apply all these like this. The options will only be present if the
+ # association supports that option; this is enforced by the association builder.
+ scope = scope.apply_finder_options(options.slice(
+ :readonly, :include, :order, :limit, :joins, :group, :having, :offset))
+
+ if options[:through] && !options[:include]
+ scope = scope.includes(source_options[:include])
+ end
+
+ if select = select_value
+ scope = scope.select(select)
+ end
+
+ add_constraints(scope)
+ end
+
+ private
+
+ def select_value
+ select_value = options[:select]
+
+ if reflection.collection?
+ select_value ||= options[:uniq] && "DISTINCT #{reflection.quoted_table_name}.*"
+ end
+
+ if reflection.macro == :has_and_belongs_to_many
+ select_value ||= reflection.klass.arel_table[Arel.star]
+ end
+
+ select_value
+ end
+
+ def add_constraints(scope)
+ tables = construct_tables
+
+ chain.each_with_index do |reflection, i|
+ table, foreign_table = tables.shift, tables.first
+
+ if reflection.source_macro == :has_and_belongs_to_many
+ join_table = tables.shift
+
+ scope = scope.joins(join(
+ join_table,
+ table[reflection.active_record_primary_key].
+ eq(join_table[reflection.association_foreign_key])
+ ))
+
+ table, foreign_table = join_table, tables.first
+ end
+
+ if reflection.source_macro == :belongs_to
+ key = reflection.association_primary_key
+ foreign_key = reflection.foreign_key
+ else
+ key = reflection.foreign_key
+ foreign_key = reflection.active_record_primary_key
+ end
+
+ if reflection == chain.last
+ scope = scope.where(table[key].eq(owner[foreign_key]))
+
+ conditions[i].each do |condition|
+ if options[:through] && condition.is_a?(Hash)
+ condition = { table.name => condition }
+ end
+
+ scope = scope.where(interpolate(condition))
+ end
+ else
+ constraint = table[key].eq(foreign_table[foreign_key])
+ join = join(foreign_table, constraint)
+
+ scope = scope.joins(join)
+
+ unless conditions[i].empty?
+ scope = scope.where(sanitize(conditions[i], table))
+ end
+ end
+ end
+
+ scope
+ end
+
+ def alias_suffix
+ reflection.name
+ end
+
+ def table_name_for(reflection)
+ if reflection == self.reflection
+ # If this is a polymorphic belongs_to, we want to get the klass from the
+ # association because it depends on the polymorphic_type attribute of
+ # the owner
+ klass.table_name
+ else
+ reflection.table_name
+ end
+ end
+
+ end
+ end
+end
View
24 activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
@@ -7,24 +7,24 @@ class HasAndBelongsToMany < CollectionAssociation #:nodoc:
def build
reflection = super
check_validity(reflection)
- redefine_destroy
+ define_after_destroy_method
reflection
end
private
- def redefine_destroy
- # Don't use a before_destroy callback since users' before_destroy
- # callbacks will be executed after the association is wiped out.
+ def define_after_destroy_method
name = self.name
- model.send(:include, Module.new {
- class_eval <<-RUBY, __FILE__, __LINE__ + 1
- def destroy # def destroy
- super # super
- #{name}.clear # posts.clear
- end # end
- RUBY
- })
+ model.send(:class_eval, <<-eoruby, __FILE__, __LINE__ + 1)
+ def #{after_destroy_method_name}
+ association(#{name.to_sym.inspect}).delete_all
+ end
+ eoruby
+ model.after_destroy after_destroy_method_name
+ end
+
+ def after_destroy_method_name
+ "has_and_belongs_to_many_after_destroy_for_#{name}"
end
# TODO: These checks should probably be moved into the Reflection, and we should not be
View
13 activerecord/lib/active_record/associations/collection_association.rb
@@ -331,11 +331,6 @@ def cached_scope(method, args)
@scopes_cache[method][args] ||= scoped.readonly(nil).send(method, *args)
end
- def association_scope
- options = reflection.options.slice(:order, :limit, :joins, :group, :having, :offset)
- super.apply_finder_options(options)
- end
-
def load_target
if find_target?
targets = []
@@ -373,14 +368,6 @@ def add_to_target(record)
private
- def select_value
- super || uniq_select_value
- end
-
- def uniq_select_value
- options[:uniq] && "DISTINCT #{reflection.quoted_table_name}.*"
- end
-
def custom_counter_sql
if options[:counter_sql]
interpolate(options[:counter_sql])
View
22 activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -26,10 +26,6 @@ def insert_record(record, validate = true)
record
end
- def association_scope
- super.joins(construct_joins)
- end
-
private
def count_records
@@ -48,24 +44,6 @@ def delete_records(records, method)
end
end
- def construct_joins
- right = join_table
- left = reflection.klass.arel_table
-
- condition = left[reflection.klass.primary_key].eq(
- right[reflection.association_foreign_key])
-
- right.create_join(right, right.create_on(condition))
- end
-
- def construct_owner_conditions
- super(join_table)
- end
-
- def select_value
- super || reflection.klass.arel_table[Arel.star]
- end
-
def invertible_for?(record)
false
end
View
2 activerecord/lib/active_record/associations/has_many_association.rb
@@ -94,8 +94,6 @@ def delete_records(records, method)
end
end
end
-
- alias creation_attributes construct_owner_attributes
end
end
end
View
6 activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -34,7 +34,9 @@ def concat(*records)
end
def insert_record(record, validate = true)
+ ensure_not_nested
return if record.new_record? && !record.save(:validate => validate)
+
through_record(record).save!
update_counter(1)
record
@@ -59,6 +61,8 @@ def through_record(record)
end
def build_record(attributes)
+ ensure_not_nested
+
record = super(attributes)
inverse = source_reflection.inverse_of
@@ -93,6 +97,8 @@ def update_through_counter?(method)
end
def delete_records(records, method)
+ ensure_not_nested
+
through = owner.association(through_reflection.name)
scope = through.scoped.where(construct_join_attributes(*records))
View
6 activerecord/lib/active_record/associations/has_one_association.rb
@@ -39,14 +39,8 @@ def delete(method = options[:dependent])
end
end
- def association_scope
- super.order(options[:order])
- end
-
private
- alias creation_attributes construct_owner_attributes
-
# The reason that the save param for replace is false, if for create (not just build),
# is because the setting of the foreign keys is actually handled by the scoping when
# the record is instantiated, and so they are set straight away and do not need to be
View
2 activerecord/lib/active_record/associations/has_one_through_association.rb
@@ -12,6 +12,8 @@ def replace(record)
private
def create_through_record(record)
+ ensure_not_nested
+
through_proxy = owner.association(through_reflection.name)
through_record = through_proxy.send(:load_target)
View
32 activerecord/lib/active_record/associations/join_dependency.rb
@@ -5,18 +5,16 @@ class JoinDependency # :nodoc:
autoload :JoinBase, 'active_record/associations/join_dependency/join_base'
autoload :JoinAssociation, 'active_record/associations/join_dependency/join_association'
- attr_reader :join_parts, :reflections, :table_aliases, :active_record
+ attr_reader :join_parts, :reflections, :alias_tracker, :active_record
def initialize(base, associations, joins)
- @active_record = base
- @table_joins = joins
- @join_parts = [JoinBase.new(base)]
- @associations = {}
- @reflections = []
- @table_aliases = Hash.new do |h,name|
- h[name] = count_aliases_from_table_joins(name.downcase)
- end
- @table_aliases[base.table_name] = 1
+ @active_record = base
+ @table_joins = joins
+ @join_parts = [JoinBase.new(base)]
+ @associations = {}
+ @reflections = []
+ @alias_tracker = AliasTracker.new(joins)
+ @alias_tracker.aliased_name_for(base.table_name) # Updates the count for base.table_name to 1
build(associations)
end
@@ -45,20 +43,6 @@ def columns
}.flatten
end
- def count_aliases_from_table_joins(name)
- return 0 if Arel::Table === @table_joins
-
- # quoted_name should be downcased as some database adapters (Oracle) return quoted name in uppercase
- quoted_name = active_record.connection.quote_table_name(name).downcase
-
- @table_joins.map { |join|
- # Table names + table aliases
- join.left.downcase.scan(
- /join(?:\s+\w+)?\s+(\S+\s+)?#{quoted_name}\son/
- ).size
- }.sum
- end
-
def instantiate(rows)
primary_key = join_base.aliased_primary_key
parents = {}
View
267 activerecord/lib/active_record/associations/join_dependency/join_association.rb
@@ -2,6 +2,8 @@ module ActiveRecord
module Associations
class JoinDependency # :nodoc:
class JoinAssociation < JoinPart # :nodoc:
+ include JoinHelper
+
# The reflection of the association represented
attr_reader :reflection
@@ -18,10 +20,15 @@ class JoinAssociation < JoinPart # :nodoc:
attr_accessor :join_type
# These implement abstract methods from the superclass
- attr_reader :aliased_prefix, :aliased_table_name
+ attr_reader :aliased_prefix
+
+ attr_reader :tables
- delegate :options, :through_reflection, :source_reflection, :to => :reflection
+ delegate :options, :through_reflection, :source_reflection, :chain, :to => :reflection
delegate :table, :table_name, :to => :parent, :prefix => :parent
+ delegate :alias_tracker, :to => :join_dependency
+
+ alias :alias_suffix :parent_table_name
def initialize(reflection, join_dependency, parent = nil)
reflection.check_validity!
@@ -37,14 +44,7 @@ def initialize(reflection, join_dependency, parent = nil)
@parent = parent
@join_type = Arel::InnerJoin
@aliased_prefix = "t#{ join_dependency.join_parts.size }"
-
- # This must be done eagerly upon initialisation because the alias which is produced
- # depends on the state of the join dependency, but we want it to work the same way
- # every time.
- allocate_aliases
- @table = Arel::Table.new(
- table_name, :as => aliased_table_name, :engine => arel_engine
- )
+ @tables = construct_tables.reverse
end
def ==(other)
@@ -60,219 +60,84 @@ def find_parent_in(other_join_dependency)
end
def join_to(relation)
- send("join_#{reflection.macro}_to", relation)
- end
-
- def join_relation(joining_relation)
- self.join_type = Arel::OuterJoin
- joining_relation.joins(self)
- end
-
- attr_reader :table
- # More semantic name given we are talking about associations
- alias_method :target_table, :table
-
- protected
-
- def aliased_table_name_for(name, suffix = nil)
- aliases = @join_dependency.table_aliases
-
- if aliases[name] != 0 # We need an alias
- connection = active_record.connection
-
- name = connection.table_alias_for "#{pluralize(reflection.name)}_#{parent_table_name}#{suffix}"
- aliases[name] += 1
- name = name[0, connection.table_alias_length-3] + "_#{aliases[name]}" if aliases[name] > 1
- else
- aliases[name] += 1
- end
-
- name
- end
+ tables = @tables.dup
+ foreign_table = parent_table
+
+ # The chain starts with the target table, but we want to end with it here (makes
+ # more sense in this context), so we reverse
+ chain.reverse.each_with_index do |reflection, i|
+ table = tables.shift
+
+ case reflection.source_macro
+ when :belongs_to
+ key = reflection.association_primary_key
+ foreign_key = reflection.foreign_key
+ when :has_and_belongs_to_many
+ # Join the join table first...
+ relation.from(join(
+ table,
+ table[reflection.foreign_key].
+ eq(foreign_table[reflection.active_record_primary_key])
+ ))
+
+ foreign_table, table = table, tables.shift
+
+ key = reflection.association_primary_key
+ foreign_key = reflection.association_foreign_key
+ else
+ key = reflection.foreign_key
+ foreign_key = reflection.active_record_primary_key
+ end
- def pluralize(table_name)
- ActiveRecord::Base.pluralize_table_names ? table_name.to_s.pluralize : table_name
- end
+ constraint = table[key].eq(foreign_table[foreign_key])
- private
+ if reflection.klass.finder_needs_type_condition?
+ constraint = table.create_and([
+ constraint,
+ reflection.klass.send(:type_condition, table)
+ ])
+ end
- def allocate_aliases
- @aliased_table_name = aliased_table_name_for(table_name)
+ relation.from(join(table, constraint))
- if reflection.macro == :has_and_belongs_to_many
- @aliased_join_table_name = aliased_table_name_for(reflection.options[:join_table], "_join")
- elsif [:has_many, :has_one].include?(reflection.macro) && reflection.options[:through]
- @aliased_join_table_name = aliased_table_name_for(reflection.through_reflection.klass.table_name, "_join")
- end
- end
+ unless conditions[i].empty?
+ relation.where(sanitize(conditions[i], table))
+ end
- def process_conditions(conditions, table_name)
- if conditions.respond_to?(:to_proc)
- conditions = instance_eval(&conditions)
+ # The current table in this iteration becomes the foreign table in the next
+ foreign_table = table
end
- Arel.sql(sanitize_sql(conditions, table_name))
+ relation
end
- def sanitize_sql(condition, table_name)
- active_record.send(:sanitize_sql, condition, table_name)
+ def join_relation(joining_relation)
+ self.join_type = Arel::OuterJoin
+ joining_relation.joins(self)
end
- def join_target_table(relation, condition)
- conditions = [condition]
-
- # If the target table is an STI model then we must be sure to only include records of
- # its type and its sub-types.
- unless active_record.descends_from_active_record?
- sti_column = target_table[active_record.inheritance_column]
- subclasses = active_record.descendants
- sti_condition = sti_column.eq(active_record.sti_name)
-
- conditions << subclasses.inject(sti_condition) { |attr,subclass|
- attr.or(sti_column.eq(subclass.sti_name))
- }
- end
-
- # If the reflection has conditions, add them
- if options[:conditions]
- conditions << process_conditions(options[:conditions], aliased_table_name)
- end
-
- ands = relation.create_and(conditions)
-
- join = relation.create_join(
- target_table,
- relation.create_on(ands),
- join_type)
-
- relation.from join
+ def table
+ tables.last
end
- def join_has_and_belongs_to_many_to(relation)
- join_table = Arel::Table.new(
- options[:join_table]
- ).alias(@aliased_join_table_name)
-
- fk = options[:foreign_key] || reflection.active_record.to_s.foreign_key
- klass_fk = options[:association_foreign_key] || reflection.klass.to_s.foreign_key
-
- relation = relation.join(join_table, join_type)
- relation = relation.on(
- join_table[fk].
- eq(parent_table[reflection.active_record.primary_key])
- )
-
- join_target_table(
- relation,
- target_table[reflection.klass.primary_key].
- eq(join_table[klass_fk])
- )
+ def aliased_table_name
+ table.table_alias || table.name
end
- def join_has_many_to(relation)
- if reflection.options[:through]
- join_has_many_through_to(relation)
- elsif reflection.options[:as]
- join_has_many_polymorphic_to(relation)
- else
- foreign_key = options[:foreign_key] || reflection.active_record.name.foreign_key
- primary_key = options[:primary_key] || parent.primary_key
-
- join_target_table(
- relation,
- target_table[foreign_key].
- eq(parent_table[primary_key])
- )
- end
+ def conditions
+ @conditions ||= reflection.conditions.reverse
end
- alias :join_has_one_to :join_has_many_to
-
- def join_has_many_through_to(relation)
- join_table = Arel::Table.new(
- through_reflection.klass.table_name
- ).alias @aliased_join_table_name
- jt_conditions = []
- first_key = second_key = nil
+ private
- if through_reflection.macro == :belongs_to
- jt_primary_key = through_reflection.foreign_key
- jt_foreign_key = through_reflection.association_primary_key
+ def interpolate(conditions)
+ if conditions.respond_to?(:to_proc)
+ instance_eval(&conditions)
else
- jt_primary_key = through_reflection.active_record_primary_key
- jt_foreign_key = through_reflection.foreign_key
-
- if through_reflection.options[:as] # has_many :through against a polymorphic join
- jt_conditions <<
- join_table["#{through_reflection.options[:as]}_type"].
- eq(parent.active_record.base_class.name)
- end
+ conditions
end
-
- case source_reflection.macro
- when :has_many
- second_key = options[:foreign_key] || primary_key
-
- if source_reflection.options[:as]
- first_key = "#{source_reflection.options[:as]}_id"
- else
- first_key = through_reflection.klass.base_class.to_s.foreign_key
- end
-
- unless through_reflection.klass.descends_from_active_record?
- jt_conditions <<
- join_table[through_reflection.active_record.inheritance_column].
- eq(through_reflection.klass.sti_name)
- end
- when :belongs_to
- first_key = primary_key
-
- if reflection.options[:source_type]
- second_key = source_reflection.association_foreign_key
-
- jt_conditions <<
- join_table[reflection.source_reflection.foreign_type].
- eq(reflection.options[:source_type])
- else
- second_key = source_reflection.foreign_key
- end
- end
-
- jt_conditions <<
- parent_table[jt_primary_key].
- eq(join_table[jt_foreign_key])
-
- if through_reflection.options[:conditions]
- jt_conditions << process_conditions(through_reflection.options[:conditions], aliased_table_name)
- end
-
- relation = relation.join(join_table, join_type).on(*jt_conditions)
-
- join_target_table(
- relation,
- target_table[first_key].eq(join_table[second_key])
- )
end
- def join_has_many_polymorphic_to(relation)
- join_target_table(
- relation,
- target_table["#{reflection.options[:as]}_id"].
- eq(parent_table[parent.primary_key]).and(
- target_table["#{reflection.options[:as]}_type"].
- eq(parent.active_record.base_class.name))
- )
- end
-
- def join_belongs_to_to(relation)
- foreign_key = options[:foreign_key] || reflection.foreign_key
- primary_key = options[:primary_key] || reflection.klass.primary_key
-
- join_target_table(
- relation,
- target_table[primary_key].eq(parent_table[foreign_key])
- )
- end
end
end
end
View
56 activerecord/lib/active_record/associations/join_helper.rb
@@ -0,0 +1,56 @@
+module ActiveRecord
+ module Associations
+ # Helper class module which gets mixed into JoinDependency::JoinAssociation and AssociationScope
+ module JoinHelper #:nodoc:
+
+ def join_type
+ Arel::InnerJoin
+ end
+
+ private
+
+ def construct_tables
+ tables = []
+ chain.each do |reflection|
+ tables << alias_tracker.aliased_table_for(
+ table_name_for(reflection),
+ table_alias_for(reflection, reflection != self.reflection)
+ )
+
+ if reflection.source_macro == :has_and_belongs_to_many
+ tables << alias_tracker.aliased_table_for(
+ (reflection.source_reflection || reflection).options[:join_table],
+ table_alias_for(reflection, true)
+ )
+ end
+ end
+ tables
+ end
+
+ def table_name_for(reflection)
+ reflection.table_name
+ end
+
+ def table_alias_for(reflection, join = false)
+ name = alias_tracker.pluralize(reflection.name)
+ name << "_#{alias_suffix}"
+ name << "_join" if join
+ name
+ end
+
+ def join(table, constraint)
+ table.create_join(table, table.create_on(constraint), join_type)
+ end
+
+ def sanitize(conditions, table)
+ conditions = conditions.map do |condition|
+ condition = active_record.send(:sanitize_sql, interpolate(condition), table.table_alias || table.name)
+ condition = Arel.sql(condition) unless condition.is_a?(Arel::Node)
+ condition
+ end
+
+ conditions.length == 1 ? conditions.first : Arel::Nodes::And.new(conditions)
+ end
+ end
+ end
+end
View
5 activerecord/lib/active_record/associations/preloader/through_association.rb
@@ -19,8 +19,9 @@ def associated_records_by_owner
source_reflection.name, options
).run
- through_records.each do |owner, owner_through_records|
- owner_through_records.map! { |r| r.send(source_reflection.name) }.flatten!
+ through_records.each do |owner, records|
+ records.map! { |r| r.send(source_reflection.name) }.flatten!
+ records.compact!
end
end
View
110 activerecord/lib/active_record/associations/through_association.rb
@@ -3,79 +3,24 @@ module ActiveRecord
module Associations
module ThroughAssociation #:nodoc:
- delegate :source_options, :through_options, :source_reflection, :through_reflection, :to => :reflection
+ delegate :source_reflection, :through_reflection, :chain, :to => :reflection
protected
+ # We merge in these scopes for two reasons:
+ #
+ # 1. To get the default_scope conditions for any of the other reflections in the chain
+ # 2. To get the type conditions for any STI models in the chain
def target_scope
- super.merge(through_reflection.klass.scoped)
- end
-
- def association_scope
- scope = super.joins(construct_joins)
- scope = add_conditions(scope)
- unless options[:include]
- scope = scope.includes(source_options[:include])
+ scope = super
+ chain[1..-1].each do |reflection|
+ scope = scope.merge(reflection.klass.scoped)
end
scope
end
private
- # This scope affects the creation of the associated records (not the join records). At the
- # moment we only support creating on a :through association when the source reflection is a
- # belongs_to. Thus it's not necessary to set a foreign key on the associated record(s), so
- # this scope has can legitimately be empty.
- def creation_attributes
- { }
- end
-
- def aliased_through_table
- name = through_reflection.table_name
-
- reflection.table_name == name ?
- through_reflection.klass.arel_table.alias(name + "_join") :
- through_reflection.klass.arel_table
- end
-
- def construct_owner_conditions
- super(aliased_through_table, through_reflection)
- end
-
- def construct_joins
- right = aliased_through_table
- left = reflection.klass.arel_table
-
- conditions = []
-
- if source_reflection.macro == :belongs_to
- reflection_primary_key = source_reflection.association_primary_key
- source_primary_key = source_reflection.foreign_key
-
- if options[:source_type]
- column = source_reflection.foreign_type
- conditions <<
- right[column].eq(options[:source_type])
- end
- else
- reflection_primary_key = source_reflection.foreign_key
- source_primary_key = source_reflection.active_record_primary_key
-
- if source_options[:as]
- column = "#{source_options[:as]}_type"
- conditions <<
- left[column].eq(through_reflection.klass.name)
- end
- end
-
- conditions <<
- left[reflection_primary_key].eq(right[source_primary_key])
-
- right.create_join(
- right,
- right.create_on(right.create_and(conditions)))
- end
-
# Construct attributes for :through pointing to owner and associate. This is used by the
# methods which create and delete records on the association.
#
@@ -112,37 +57,8 @@ def construct_join_attributes(*records)
end
end
- # The reason that we are operating directly on the scope here (rather than passing
- # back some arel conditions to be added to the scope) is because scope.where([x, y])
- # has a different meaning to scope.where(x).where(y) - the first version might
- # perform some substitution if x is a string.
- def add_conditions(scope)
- unless through_reflection.klass.descends_from_active_record?
- scope = scope.where(through_reflection.klass.send(:type_condition))
- end
-
- scope = scope.where(interpolate(source_options[:conditions]))
- scope.where(through_conditions)
- end
-
- # If there is a hash of conditions then we make sure the keys are scoped to the
- # through table name if left ambiguous.
- def through_conditions
- conditions = interpolate(through_options[:conditions])
-
- if conditions.is_a?(Hash)
- Hash[conditions.map { |key, value|
- unless value.is_a?(Hash) || key.to_s.include?('.')
- key = aliased_through_table.name + '.' + key.to_s
- end
-
- [key, value]
- }]
- else
- conditions
- end
- end
-
+ # Note: this does not capture all cases, for example it would be crazy to try to
+ # properly support stale-checking for nested associations.
def stale_state
if through_reflection.macro == :belongs_to
owner[through_reflection.foreign_key].to_s
@@ -153,6 +69,12 @@ def foreign_key_present?
through_reflection.macro == :belongs_to &&
!owner[through_reflection.foreign_key].nil?
end
+
+ def ensure_not_nested
+ if reflection.nested?
+ raise HasManyThroughNestedAssociationsAreReadonly.new(owner, reflection)
+ end
+ end
end
end
end
View
9 activerecord/lib/active_record/attribute_methods/read.rb
@@ -70,7 +70,14 @@ def define_read_method(symbol, attr