diff options
137 files changed, 2315 insertions, 788 deletions
@@ -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" @@ -85,6 +85,7 @@ RDoc::Task.new do |rdoc| rdoc.rdoc_files.include('actionmailer/README.rdoc') rdoc.rdoc_files.include('actionmailer/CHANGELOG') rdoc.rdoc_files.include('actionmailer/lib/action_mailer/base.rb') + rdoc.rdoc_files.include('actionmailer/lib/action_mailer/mail_helper.rb') rdoc.rdoc_files.exclude('actionmailer/lib/action_mailer/vendor/*') rdoc.rdoc_files.include('activesupport/README.rdoc') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 5ab92c8cfc..cc19402243 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,11 @@ *Rails 3.1.0 (unreleased)* +* Allow FormHelper#form_for to specify the :method as a direct option instead of through the :html hash [DHH] + + form_for(@post, remote: true, method: :delete) instead of form_for(@post, remote: true, html: { method: :delete }) + +* Make JavaScriptHelper#j() an alias for JavaScriptHelper#escape_javascript() -- note this then supersedes the Object#j() method that the JSON gem adds within templates using the JavaScriptHelper [DHH] + * Sensitive query string parameters (specified in config.filter_parameters) will now be filtered out from the request paths in the log file. [Prem Sichanugrist, fxn] * URL parameters which return false for to_param now appear in the query string (previously they were removed) [Andrew White] diff --git a/actionpack/lib/abstract_controller/callbacks.rb b/actionpack/lib/abstract_controller/callbacks.rb index 95992c2698..1943ca4436 100644 --- a/actionpack/lib/abstract_controller/callbacks.rb +++ b/actionpack/lib/abstract_controller/callbacks.rb @@ -14,7 +14,7 @@ module AbstractController # 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 diff --git a/actionpack/lib/abstract_controller/view_paths.rb b/actionpack/lib/abstract_controller/view_paths.rb index 6544c8949a..cea0f5ad1e 100644 --- a/actionpack/lib/abstract_controller/view_paths.rb +++ b/actionpack/lib/abstract_controller/view_paths.rb @@ -36,7 +36,7 @@ module AbstractController # ==== Parameters # * <tt>path</tt> - If a String is provided, it gets converted into # the default view path. You may also provide a custom view path - # (see ActionView::ViewPathSet for more information) + # (see ActionView::PathSet for more information) def append_view_path(path) self.view_paths = view_paths.dup + Array(path) end @@ -46,7 +46,7 @@ module AbstractController # ==== Parameters # * <tt>path</tt> - If a String is provided, it gets converted into # the default view path. You may also provide a custom view path - # (see ActionView::ViewPathSet for more information) + # (see ActionView::PathSet for more information) def prepend_view_path(path) self.view_paths = Array(path) + view_paths.dup end @@ -59,8 +59,8 @@ module AbstractController # Set the view paths. # # ==== Parameters - # * <tt>paths</tt> - If a ViewPathSet is provided, use that; - # otherwise, process the parameter into a ViewPathSet. + # * <tt>paths</tt> - If a PathSet is provided, use that; + # otherwise, process the parameter into a PathSet. def view_paths=(paths) self._view_paths = ActionView::Base.process_view_paths(paths) self._view_paths.freeze diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb index 50d8ca9484..2099fd069a 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb +++ b/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> diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index f67708722b..14c424f24b 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -107,7 +107,7 @@ module ActionDispatch 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 @@ module ActionDispatch 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 @@ -364,9 +364,17 @@ module ActionDispatch # match 'path' => 'c#a', :defaults => { :format => 'jpg' } # # See <tt>Scoping#defaults</tt> for its scope equivalent. + # + # [:anchor] + # Boolean to anchor a #match pattern. Default is true. When set to + # false, the pattern matches any request prefixed with the given path. + # + # # 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 diff --git a/actionpack/lib/action_dispatch/routing/route.rb b/actionpack/lib/action_dispatch/routing/route.rb index eae9d4ea6d..a049510182 100644 --- a/actionpack/lib/action_dispatch/routing/route.rb +++ b/actionpack/lib/action_dispatch/routing/route.rb @@ -12,6 +12,8 @@ module ActionDispatch @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 @@ module ActionDispatch 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 diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 48abf119f1..9025d9e24c 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -185,7 +185,7 @@ module ActionView # # is equivalent to something like: # - # <%= form_for @post, :as => :post, :url => post_path(@post), :html => { :method => :put, :class => "edit_post", :id => "edit_post_45" } do |f| %> + # <%= form_for @post, :as => :post, :url => post_path(@post), :method => :put, :html => { :class => "edit_post", :id => "edit_post_45" } do |f| %> # ... # <% end %> # @@ -236,6 +236,16 @@ module ActionView # Where <tt>@document = Document.find(params[:id])</tt> and # <tt>@comment = Comment.new</tt>. # + # === Setting the method + # + # You can force the form to use the full array of HTTP verbs by setting + # + # :method => (:get|:post|:put|:delete) + # + # in the options hash. If the verb is not GET or POST, which are natively supported by HTML forms, the + # form will be set to POST and a hidden input called _method will carry the intended verb for the server + # to interpret. + # # === Unobtrusive JavaScript # # Specifying: @@ -298,7 +308,7 @@ module ActionView # # In this case, if you use this: # - # <%= render :partial => f %> + # <%= render f %> # # The rendered template is <tt>people/_labelling_form</tt> and the local # variable referencing the form builder is called @@ -350,6 +360,7 @@ module ActionView end options[:html][:remote] = options.delete(:remote) + options[:html][:method] = options.delete(:method) if options.has_key?(:method) options[:html][:authenticity_token] = options.delete(:authenticity_token) builder = options[:parent_builder] = instantiate_builder(object_name, object, options, &proc) diff --git a/actionpack/lib/action_view/helpers/javascript_helper.rb b/actionpack/lib/action_view/helpers/javascript_helper.rb index cd3a3eac80..6eda23a9a3 100644 --- a/actionpack/lib/action_view/helpers/javascript_helper.rb +++ b/actionpack/lib/action_view/helpers/javascript_helper.rb @@ -47,6 +47,9 @@ module ActionView "'" => "\\'" } # Escape carrier returns and single and double quotes for JavaScript segments. + # Also available through the alias j(). This is particularly helpful in JavaScript responses, like: + # + # $('some_element').replaceWith('<%=j render 'some/element_template' %>'); def escape_javascript(javascript) if javascript javascript.gsub(/(\\|<\/|\r\n|[\n\r"'])/) { JS_ESCAPE_MAP[$1] } @@ -55,6 +58,8 @@ module ActionView end end + alias_method :escape_javascript, :j + # Returns a JavaScript tag with the +content+ inside. Example: # javascript_tag "alert('All is good')" # diff --git a/actionpack/lib/action_view/helpers/number_helper.rb b/actionpack/lib/action_view/helpers/number_helper.rb index 4e44843c4b..05a9c5b4f1 100644 --- a/actionpack/lib/action_view/helpers/number_helper.rb +++ b/actionpack/lib/action_view/helpers/number_helper.rb @@ -472,7 +472,7 @@ module ActionView 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 diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index 589b2a1a76..6c1063592f 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -5,6 +5,25 @@ require "action_view/template" 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 @@ module ActionView # 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 @@ module ActionView 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 @@ module ActionView 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 @@ module ActionView 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 diff --git a/actionpack/lib/action_view/testing/resolvers.rb b/actionpack/lib/action_view/testing/resolvers.rb index 5c5cab7c7d..773dfcbb1d 100644 --- a/actionpack/lib/action_view/testing/resolvers.rb +++ b/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 @@ module ActionView #:nodoc: 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 @@ module ActionView #:nodoc: 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 diff --git a/actionpack/test/action_dispatch/routing/mapper_test.rb b/actionpack/test/action_dispatch/routing/mapper_test.rb index 9966234f1b..e21b271907 100644 --- a/actionpack/test/action_dispatch/routing/mapper_test.rb +++ b/actionpack/test/action_dispatch/routing/mapper_test.rb @@ -46,6 +46,13 @@ module ActionDispatch 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 diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index 330fa276d0..9e44e8e088 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -505,6 +505,21 @@ class FilterTest < ActionController::TestCase 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 @@ class FilterTest < ActionController::TestCase 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 diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index 2a478c214f..e453dd11ce 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/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 diff --git a/actionpack/test/fixtures/custom_pattern/another.html.erb b/actionpack/test/fixtures/custom_pattern/another.html.erb new file mode 100644 index 0000000000..6d7f3bafbb --- /dev/null +++ b/actionpack/test/fixtures/custom_pattern/another.html.erb @@ -0,0 +1 @@ +Hello custom patterns!
\ No newline at end of file diff --git a/actionpack/test/fixtures/custom_pattern/html/another.erb b/actionpack/test/fixtures/custom_pattern/html/another.erb new file mode 100644 index 0000000000..dbd7e96ab6 --- /dev/null +++ b/actionpack/test/fixtures/custom_pattern/html/another.erb @@ -0,0 +1 @@ +Another template!
\ No newline at end of file diff --git a/actionpack/test/fixtures/custom_pattern/html/path.erb b/actionpack/test/fixtures/custom_pattern/html/path.erb new file mode 100644 index 0000000000..6d7f3bafbb --- /dev/null +++ b/actionpack/test/fixtures/custom_pattern/html/path.erb @@ -0,0 +1 @@ +Hello custom patterns!
\ No newline at end of file diff --git a/actionpack/test/fixtures/filter_test/implicit_actions/edit.html.erb b/actionpack/test/fixtures/filter_test/implicit_actions/edit.html.erb new file mode 100644 index 0000000000..8491ab9f80 --- /dev/null +++ b/actionpack/test/fixtures/filter_test/implicit_actions/edit.html.erb @@ -0,0 +1 @@ +edit
\ No newline at end of file diff --git a/actionpack/test/fixtures/filter_test/implicit_actions/show.html.erb b/actionpack/test/fixtures/filter_test/implicit_actions/show.html.erb new file mode 100644 index 0000000000..0a89cecf05 --- /dev/null +++ b/actionpack/test/fixtures/filter_test/implicit_actions/show.html.erb @@ -0,0 +1 @@ +show
\ No newline at end of file diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index 359b078466..ff183d097d 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -715,14 +715,31 @@ class FormHelperTest < ActionView::TestCase assert_dom_equal expected, output_buffer end + def test_form_for_with_method_as_part_of_html_options + form_for(@post, :url => '/', :html => { :id => 'create-post', :method => :delete }) do |f| + concat f.text_field(:title) + concat f.text_area(:body) + concat f.check_box(:secret) + end + + expected = whole_form("/", "create-post", "edit_post", "delete") do + "<input name='post[title]' size='30' type='text' id='post_title' value='Hello World' />" + + "<textarea name='post[body]' id='post_body' rows='20' cols='40'>Back to the hill and over it again!</textarea>" + + "<input name='post[secret]' type='hidden' value='0' />" + + "<input name='post[secret]' checked='checked' type='checkbox' id='post_secret' value='1' />" + end + + assert_dom_equal expected, output_buffer + end + def test_form_for_with_method - form_for(@post, :url => '/', :html => { :id => 'create-post', :method => :put }) do |f| + form_for(@post, :url => '/', :method => :delete, :html => { :id => 'create-post' }) do |f| concat f.text_field(:title) concat f.text_area(:body) concat f.check_box(:secret) end - expected = whole_form("/", "create-post", "edit_post", "put") do + expected = whole_form("/", "create-post", "edit_post", "delete") do "<input name='post[title]' size='30' type='text' id='post_title' value='Hello World' />" + "<textarea name='post[body]' id='post_body' rows='20' cols='40'>Back to the hill and over it again!</textarea>" + "<input name='post[secret]' type='hidden' value='0' />" + diff --git a/actionpack/test/template/number_helper_test.rb b/actionpack/test/template/number_helper_test.rb index c1b4bab903..c8d50ebf75 100644 --- a/actionpack/test/template/number_helper_test.rb +++ b/actionpack/test/template/number_helper_test.rb @@ -195,7 +195,9 @@ class NumberHelperTest < ActionView::TestCase 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) diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index 034fb6c210..dd86bfed04 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -381,7 +381,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase 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 @@ class LazyViewRenderTest < ActiveSupport::TestCase 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' diff --git a/actionpack/test/template/resolver_patterns_test.rb b/actionpack/test/template/resolver_patterns_test.rb new file mode 100644 index 0000000000..97b1bad055 --- /dev/null +++ b/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 diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index 2a99450a3d..be55581c66 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -106,11 +106,14 @@ module ActiveModel 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 diff --git a/activemodel/lib/active_model/lint.rb b/activemodel/lib/active_model/lint.rb index 957d1b9d70..b71ef4b22e 100644 --- a/activemodel/lib/active_model/lint.rb +++ b/activemodel/lib/active_model/lint.rb @@ -23,7 +23,7 @@ module ActiveModel 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 @@ module ActiveModel 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> diff --git a/activemodel/test/cases/attribute_methods_test.rb b/activemodel/test/cases/attribute_methods_test.rb index b001adb35a..022c6716bd 100644 --- a/activemodel/test/cases/attribute_methods_test.rb +++ b/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 @@ private 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 diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 8b4e9a34cb..6be46349fb 100644 --- a/activerecord/CHANGELOG +++ b/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] diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index e91cbd7f33..08fb6bf3c4 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -52,14 +52,6 @@ module ActiveRecord 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 @@ module ActiveRecord 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 ActiveRecord 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 @@ module ActiveRecord # 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 ActiveRecord # [: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 @@ module ActiveRecord # 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. diff --git a/activerecord/lib/active_record/associations/alias_tracker.rb b/activerecord/lib/active_record/associations/alias_tracker.rb new file mode 100644 index 0000000000..634dee2289 --- /dev/null +++ b/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 diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 25b4b9d90d..27c446b12c 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -93,23 +93,9 @@ module ActiveRecord # 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 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb new file mode 100644 index 0000000000..ab102b2b8f --- /dev/null +++ b/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 diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index e40b32826a..4b48757da7 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -7,24 +7,24 @@ module ActiveRecord::Associations::Builder 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 diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index f3761bd2c7..9f4fc44cc6 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -331,11 +331,6 @@ module ActiveRecord @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 @@ module ActiveRecord 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]) diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index 028630977d..217213808b 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -26,10 +26,6 @@ module ActiveRecord record end - def association_scope - super.joins(construct_joins) - end - private def count_records @@ -48,24 +44,6 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index cebf3e477a..78c5c4b870 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -94,8 +94,6 @@ module ActiveRecord end end end - - alias creation_attributes construct_owner_attributes end end end diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index acac68fda5..9d2b29685b 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -34,7 +34,9 @@ module ActiveRecord 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 @@ module ActiveRecord end def build_record(attributes) + ensure_not_nested + record = super(attributes) inverse = source_reflection.inverse_of @@ -93,6 +97,8 @@ module ActiveRecord end def delete_records(records, method) + ensure_not_nested + through = owner.association(through_reflection.name) scope = through.scoped.where(construct_join_attributes(*records)) diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index e13f97125f..dfcb116392 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -34,19 +34,14 @@ module ActiveRecord when :destroy target.destroy when :nullify - target.update_attribute(reflection.foreign_key, nil) + target.send("#{reflection.foreign_key}=", nil) + target.save(:validations => false) end 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 diff --git a/activerecord/lib/active_record/associations/has_one_through_association.rb b/activerecord/lib/active_record/associations/has_one_through_association.rb index d76d729303..fdf8ae1453 100644 --- a/activerecord/lib/active_record/associations/has_one_through_association.rb +++ b/activerecord/lib/active_record/associations/has_one_through_association.rb @@ -12,6 +12,8 @@ module ActiveRecord private def create_through_record(record) + ensure_not_nested + through_proxy = owner.association(through_reflection.name) through_record = through_proxy.send(:load_target) diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index c7c3cf521c..504f25271c 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -5,18 +5,16 @@ module ActiveRecord 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 @@ module ActiveRecord }.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 = {} diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index ebe39c35fe..4121a5b378 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/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 @@ module ActiveRecord 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 @@ module ActiveRecord @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 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/associations/join_helper.rb b/activerecord/lib/active_record/associations/join_helper.rb new file mode 100644 index 0000000000..eae546e76e --- /dev/null +++ b/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 diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index d630fc4c63..ad6374d09a 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -19,8 +19,9 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index e1d60ccb17..e6ab628719 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/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 @@ module ActiveRecord 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 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index ab86d8bad1..69d5cd83f1 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -70,7 +70,14 @@ module ActiveRecord if cache_attribute?(attr_name) access_code = "@attributes_cache['#{attr_name}'] ||= (#{access_code})" end - generated_attribute_methods.module_eval("def _#{symbol}; #{access_code}; end; alias #{symbol} _#{symbol}", __FILE__, __LINE__) + if symbol =~ /^[a-zA-Z_]\w*[!?=]?$/ + generated_attribute_methods.module_eval("def _#{symbol}; #{access_code}; end; alias #{symbol} _#{symbol}", __FILE__, __LINE__) + else + generated_attribute_methods.module_eval do + define_method("_#{symbol}") { eval(access_code) } + alias_method(symbol, "_#{symbol}") + end + end end end diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index 76218d2a73..6aac96df6f 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -21,9 +21,9 @@ module ActiveRecord def define_method_attribute(attr_name) if create_time_zone_conversion_attribute?(attr_name, columns_hash[attr_name]) method_body, line = <<-EOV, __LINE__ + 1 - def _#{attr_name}(reload = false) + def _#{attr_name} cached = @attributes_cache['#{attr_name}'] - return cached if cached && !reload + return cached if cached time = _read_attribute('#{attr_name}') @attributes_cache['#{attr_name}'] = time.acts_like?(:time) ? time.in_time_zone : time end @@ -41,12 +41,13 @@ module ActiveRecord if create_time_zone_conversion_attribute?(attr_name, columns_hash[attr_name]) method_body, line = <<-EOV, __LINE__ + 1 def #{attr_name}=(original_time) - time = original_time.dup unless original_time.nil? + time = original_time unless time.acts_like?(:time) time = time.is_a?(String) ? Time.zone.parse(time) : time.to_time rescue time end time = time.in_time_zone rescue nil if time - write_attribute(:#{attr_name}, (time || original_time)) + write_attribute(:#{attr_name}, original_time) + @attributes_cache["#{attr_name}"] = time end EOV generated_attribute_methods.module_eval(method_body, __FILE__, line) diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index 6a593a7e0e..3c4dab304e 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -10,7 +10,13 @@ module ActiveRecord module ClassMethods protected def define_method_attribute=(attr_name) - generated_attribute_methods.module_eval("def #{attr_name}=(new_value); write_attribute('#{attr_name}', new_value); end", __FILE__, __LINE__) + if attr_name =~ /^[a-zA-Z_]\w*[!?=]?$/ + generated_attribute_methods.module_eval("def #{attr_name}=(new_value); write_attribute('#{attr_name}', new_value); end", __FILE__, __LINE__) + else + generated_attribute_methods.send(:define_method, "#{attr_name}=") do |new_value| + write_attribute(attr_name, new_value) + end + end end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index a5e1c91f47..b778b0c0f0 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -973,8 +973,8 @@ module ActiveRecord #:nodoc: relation end - def type_condition - sti_column = arel_table[inheritance_column.to_sym] + def type_condition(table = arel_table) + sti_column = table[inheritance_column.to_sym] sti_names = ([self] + descendants).map { |model| model.sti_name } sti_column.in(sti_names) @@ -995,7 +995,7 @@ module ActiveRecord #:nodoc: if parent < ActiveRecord::Base && !parent.abstract_class? contained = parent.table_name contained = contained.singularize if parent.pluralize_table_names - contained << '_' + contained += '_' end "#{full_table_name_prefix}#{contained}#{undecorated_table_name(name)}#{table_name_suffix}" else diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 5c1ce173c8..a3082b8f01 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -237,7 +237,6 @@ module ActiveRecord # add_limit_offset!('SELECT * FROM suppliers', {:limit => 10, :offset => 50}) # generates # SELECT * FROM suppliers LIMIT 10 OFFSET 50 - def add_limit_offset!(sql, options) if limit = options[:limit] sql << " LIMIT #{sanitize_limit(limit)}" @@ -272,6 +271,10 @@ module ActiveRecord execute "INSERT INTO #{quote_table_name(table_name)} (#{key_list.join(', ')}) VALUES (#{value_list.join(', ')})", 'Fixture Insert' end + def null_insert_value + Arel.sql 'DEFAULT' + end + def empty_insert_statement_value "VALUES(DEFAULT)" end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 3ec7dd02a4..8bae50885f 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -279,12 +279,11 @@ module ActiveRecord raise NotImplementedError, "change_column is not implemented" end - # Sets a new default value for a column. If you want to set the default - # value to +NULL+, you are out of luck. You need to - # DatabaseStatements#execute the appropriate SQL statement yourself. + # Sets a new default value for a column. # ===== Examples # change_column_default(:suppliers, :qualification, 'new') # change_column_default(:accounts, :authorized, 1) + # change_column_default(:users, :email, nil) def change_column_default(table_name, column_name, default) raise NotImplementedError, "change_column_default is not implemented" end diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 368c5b2023..e1186209d3 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -504,14 +504,28 @@ module ActiveRecord show_variable 'collation_database' end - def tables(name = nil) #:nodoc: + def tables(name = nil, database = nil) #:nodoc: tables = [] - result = execute("SHOW TABLES", name) + result = execute(["SHOW TABLES", database].compact.join(' IN '), name) result.each { |field| tables << field[0] } result.free tables end + def table_exists?(name) + return true if super + + name = name.to_s + schema, table = name.split('.', 2) + + unless table # A table was provided without a schema + table = schema + schema = nil + end + + tables(nil, schema).include? table + end + def drop_table(table_name, options = {}) super(table_name, options) end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 576450bc3a..5a830a50fb 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -453,7 +453,7 @@ module ActiveRecord # If a pk is given, fallback to default sequence name. # Don't fetch last insert id for a table without a pk. if pk && sequence_name ||= default_sequence_name(table, pk) - last_insert_id(table, sequence_name) + last_insert_id(sequence_name) end end end @@ -1038,8 +1038,9 @@ module ActiveRecord end # Returns the current ID of a table's sequence. - def last_insert_id(table, sequence_name) #:nodoc: - Integer(select_value("SELECT currval('#{sequence_name}')")) + def last_insert_id(sequence_name) #:nodoc: + r = exec_query("SELECT currval($1)", 'SQL', [[nil, sequence_name]]) + Integer(r.rows.first.first) end # Executes a SELECT query and returns the results, performing any data type diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index c2cd9e8d5e..c3a7b039ff 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -34,6 +34,14 @@ module ActiveRecord module ConnectionAdapters #:nodoc: class SQLite3Adapter < SQLiteAdapter # :nodoc: + def quote(value, column = nil) + if value.kind_of?(String) && column && column.type == :binary && column.class.respond_to?(:string_to_binary) + s = column.class.string_to_binary(value).unpack("H*")[0] + "x'#{s}'" + else + super + end + end # Returns the current database encoding format as a string, eg: 'UTF-8' def encoding diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 9ee6b88ab6..ae61d6ce94 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -336,6 +336,10 @@ module ActiveRecord alter_table(table_name, :rename => {column_name.to_s => new_column_name.to_s}) end + def null_insert_value + Arel.sql 'NULL' + end + def empty_insert_statement_value "VALUES(NULL)" end diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 6b2b1ebafe..9a31675782 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -23,7 +23,7 @@ module ActiveRecord # p2.first_name = "should fail" # p2.save # Raises a ActiveRecord::StaleObjectError # - # Optimistic locking will also check for stale data when objects are destroyed. Example: + # Optimistic locking will also check for stale data when objects are destroyed. Example: # # p1 = Person.find(1) # p2 = Person.find(1) diff --git a/activerecord/lib/active_record/locking/pessimistic.rb b/activerecord/lib/active_record/locking/pessimistic.rb index 557b277d6b..862cf8f72a 100644 --- a/activerecord/lib/active_record/locking/pessimistic.rb +++ b/activerecord/lib/active_record/locking/pessimistic.rb @@ -9,9 +9,8 @@ module ActiveRecord # Account.find(1, :lock => true) # # Pass <tt>:lock => 'some locking clause'</tt> to give a database-specific locking clause - # of your own such as 'LOCK IN SHARE MODE' or 'FOR UPDATE NOWAIT'. + # of your own such as 'LOCK IN SHARE MODE' or 'FOR UPDATE NOWAIT'. Example: # - # Example: # Account.transaction do # # select * from accounts where name = 'shugo' limit 1 for update # shugo = Account.where("name = 'shugo'").lock(true).first @@ -24,6 +23,7 @@ module ActiveRecord # # You can also use ActiveRecord::Base#lock! method to lock one record by id. # This may be better if you don't need to lock every row. Example: + # # Account.transaction do # # select * from accounts where ... # accounts = Account.where(...).all @@ -44,7 +44,7 @@ module ActiveRecord module Pessimistic # Obtain a row lock on this record. Reloads the record to obtain the requested # lock. Pass an SQL locking clause to append the end of the SELECT statement - # or pass true for "FOR UPDATE" (the default, an exclusive row lock). Returns + # or pass true for "FOR UPDATE" (the default, an exclusive row lock). Returns # the locked record. def lock!(lock = true) reload(:lock => lock) if persisted? diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index df7b22080c..3377a5934b 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -104,19 +104,17 @@ module ActiveRecord became end - # Updates a single attribute and saves the record. - # This is especially useful for boolean flags on existing records. Also note that + # Updates a single attribute of an object, without calling save. # # * Validation is skipped. - # * Callbacks are invoked. - # * updated_at/updated_on column is updated if that column is available. - # * Updates all the attributes that are dirty in this object. + # * Callbacks are skipped. + # * updated_at/updated_on column is not updated in any case. # - def update_attribute(name, value) + def update_column(name, value) name = name.to_s raise ActiveRecordError, "#{name} is marked as readonly" if self.class.readonly_attributes.include?(name) send("#{name}=", value) - save(:validate => false) + self.class.update_all({ name => value }, self.class.primary_key => id) end # Updates the attributes of the model from the passed-in hash and saves the @@ -156,7 +154,7 @@ module ActiveRecord # Saving is not subjected to validation checks. Returns +true+ if the # record could be saved. def increment!(attribute, by = 1) - increment(attribute, by).update_attribute(attribute, self[attribute]) + increment(attribute, by).update_column(attribute, self[attribute]) end # Initializes +attribute+ to zero if +nil+ and subtracts the value passed as +by+ (default is 1). @@ -173,7 +171,7 @@ module ActiveRecord # Saving is not subjected to validation checks. Returns +true+ if the # record could be saved. def decrement!(attribute, by = 1) - decrement(attribute, by).update_attribute(attribute, self[attribute]) + decrement(attribute, by).update_column(attribute, self[attribute]) end # Assigns to +attribute+ the boolean opposite of <tt>attribute?</tt>. So @@ -190,7 +188,7 @@ module ActiveRecord # Saving is not subjected to validation checks. Returns +true+ if the # record could be saved. def toggle!(attribute) - toggle(attribute).update_attribute(attribute, self[attribute]) + toggle(attribute).update_column(attribute, self[attribute]) end # Reloads the attributes of this object from the database. @@ -270,17 +268,9 @@ module ActiveRecord # Creates a record with values matching those of the instance attributes # and returns its id. def create - if id.nil? && connection.prefetch_primary_key?(self.class.table_name) - self.id = connection.next_sequence_value(self.class.sequence_name) - end - attributes_values = arel_attributes_values(!id.nil?) - new_id = if attributes_values.empty? - self.class.unscoped.insert connection.empty_insert_statement_value - else - self.class.unscoped.insert attributes_values - end + new_id = self.class.unscoped.insert attributes_values self.id ||= new_id diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 5de08953f9..e801bc4afa 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -262,16 +262,30 @@ module ActiveRecord end def through_reflection - false - end - - def through_reflection_foreign_key + nil end def source_reflection nil end + # A chain of reflections from this one back to the owner. For more see the explanation in + # ThroughReflection. + def chain + [self] + end + + # An array of arrays of conditions. Each item in the outside array corresponds to a reflection + # in the #chain. The inside arrays are simply conditions (and each condition may itself be + # a hash, array, arel predicate, etc...) + def conditions + conditions = [options[:conditions]].compact + conditions << { type => active_record.base_class.name } if options[:as] + [conditions] + end + + alias :source_macro :macro + def has_inverse? @options[:inverse_of] end @@ -363,7 +377,7 @@ module ActiveRecord # Holds all the meta-data about a :through association as it was specified # in the Active Record class. class ThroughReflection < AssociationReflection #:nodoc: - delegate :association_primary_key, :foreign_type, :to => :source_reflection + delegate :foreign_key, :foreign_type, :association_foreign_key, :active_record_primary_key, :to => :source_reflection # Gets the source of the through reflection. It checks both a singularized # and pluralized form for <tt>:belongs_to</tt> or <tt>:has_many</tt>. @@ -392,6 +406,88 @@ module ActiveRecord @through_reflection ||= active_record.reflect_on_association(options[:through]) end + # Returns an array of reflections which are involved in this association. Each item in the + # array corresponds to a table which will be part of the query for this association. + # + # The chain is built by recursively calling #chain on the source reflection and the through + # reflection. The base case for the recursion is a normal association, which just returns + # [self] as its #chain. + def chain + @chain ||= begin + chain = source_reflection.chain + through_reflection.chain + chain[0] = self # Use self so we don't lose the information from :source_type + chain + end + end + + # Consider the following example: + # + # class Person + # has_many :articles + # has_many :comment_tags, :through => :articles + # end + # + # class Article + # has_many :comments + # has_many :comment_tags, :through => :comments, :source => :tags + # end + # + # class Comment + # has_many :tags + # end + # + # There may be conditions on Person.comment_tags, Article.comment_tags and/or Comment.tags, + # but only Comment.tags will be represented in the #chain. So this method creates an array + # of conditions corresponding to the chain. Each item in the #conditions array corresponds + # to an item in the #chain, and is itself an array of conditions from an arbitrary number + # of relevant reflections, plus any :source_type or polymorphic :as constraints. + def conditions + @conditions ||= begin + conditions = source_reflection.conditions + + # Add to it the conditions from this reflection if necessary. + conditions.first << options[:conditions] if options[:conditions] + + through_conditions = through_reflection.conditions + + if options[:source_type] + through_conditions.first << { foreign_type => options[:source_type] } + end + + # Recursively fill out the rest of the array from the through reflection + conditions += through_conditions + + # And return + conditions + end + end + + # The macro used by the source association + def source_macro + source_reflection.source_macro + end + + # A through association is nested iff there would be more than one join table + def nested? + chain.length > 2 || through_reflection.macro == :has_and_belongs_to_many + end + + # We want to use the klass from this reflection, rather than just delegate straight to + # the source_reflection, because the source_reflection may be polymorphic. We still + # need to respect the source_reflection's :primary_key option, though. + def association_primary_key + @association_primary_key ||= begin + # Get the "actual" source reflection if the immediate source reflection has a + # source reflection itself + source_reflection = self.source_reflection + while source_reflection.source_reflection + source_reflection = source_reflection.source_reflection + end + + source_reflection.options[:primary_key] || klass.primary_key + end + end + # Gets an array of possible <tt>:through</tt> source reflection names: # # [:singularized, :pluralized] @@ -429,10 +525,6 @@ module ActiveRecord raise HasManyThroughAssociationPolymorphicSourceError.new(active_record.name, self, source_reflection) end - unless [:belongs_to, :has_many, :has_one].include?(source_reflection.macro) && source_reflection.options[:through].nil? - raise HasManyThroughSourceAssociationMacroError.new(self) - end - if macro == :has_one && through_reflection.collection? raise HasOneThroughCantAssociateThroughCollection.new(active_record.name, self, through_reflection) end @@ -440,14 +532,6 @@ module ActiveRecord check_validity_of_inverse! end - def through_reflection_primary_key - through_reflection.belongs_to? ? through_reflection.klass.primary_key : through_reflection.foreign_key - end - - def through_reflection_foreign_key - through_reflection.foreign_key if through_reflection.belongs_to? - end - private def derive_class_name # get the class_name of the belongs_to association of the through reflection diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 371403f510..8e545f9cad 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -30,15 +30,26 @@ module ActiveRecord end def insert(values) - im = arel.compile_insert values - im.into @table - primary_key_value = nil if primary_key && Hash === values primary_key_value = values[values.keys.find { |k| k.name == primary_key }] + + if !primary_key_value && connection.prefetch_primary_key?(klass.table_name) + primary_key_value = connection.next_sequence_value(klass.sequence_name) + values[klass.arel_table[klass.primary_key]] = primary_key_value + end + end + + im = arel.create_insert + im.into @table + + if values.empty? # empty insert + im.values = im.create_values [connection.null_insert_value], [] + else + im.insert values end @klass.connection.insert( diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 426000fde1..563843f3cc 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -123,6 +123,12 @@ module ActiveRecord end end + # Same as +first+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record + # is found. Note that <tt>first!</tt> accepts no arguments. + def first! + first or raise RecordNotFound + end + # A convenience wrapper for <tt>find(:last, *args)</tt>. You can pass in all the # same arguments to this method as you can to <tt>find(:last)</tt>. def last(*args) @@ -137,6 +143,12 @@ module ActiveRecord end end + # Same as +last+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record + # is found. Note that <tt>last!</tt> accepts no arguments. + def last! + last or raise RecordNotFound + end + # A convenience wrapper for <tt>find(:all, *args)</tt>. You can pass in all the # same arguments to this method as you can to <tt>find(:all)</tt>. def all(*args) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index cd1d7108b3..9470e7c6c5 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -261,7 +261,7 @@ module ActiveRecord ) join_nodes.each do |join| - join_dependency.table_aliases[join.left.name.downcase] = 1 + join_dependency.alias_tracker.aliased_name_for(join.left.name.downcase) end join_dependency.graft(*stashed_association_joins) diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 4150e36a9a..128e0fbd86 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -79,6 +79,9 @@ module ActiveRecord result.send(:"#{method}_value=", send(:"#{method}_value")) end + # Apply scope extension modules + result.send(:apply_modules, extensions) + result end @@ -100,6 +103,9 @@ module ActiveRecord result.send(:"#{method}_value=", send(:"#{method}_value")) end + # Apply scope extension modules + result.send(:apply_modules, extensions) + result end diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index a96796f9ff..9cd6c26322 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -173,10 +173,17 @@ module ActiveRecord # This technique is also known as optimistic concurrency control: # http://en.wikipedia.org/wiki/Optimistic_concurrency_control # - # Active Record currently provides no way to distinguish unique - # index constraint errors from other types of database errors, so you - # will have to parse the (database-specific) exception message to detect - # such a case. + # The bundled ActiveRecord::ConnectionAdapters distinguish unique index + # constraint errors from other types of database errors by throwing an + # ActiveRecord::RecordNotUnique exception. + # For other adapters you will have to parse the (database-specific) exception + # message to detect such a case. + # The following bundled adapters throw the ActiveRecord::RecordNotUnique exception: + # * ActiveRecord::ConnectionAdapters::MysqlAdapter + # * ActiveRecord::ConnectionAdapters::Mysql2Adapter + # * ActiveRecord::ConnectionAdapters::SQLiteAdapter + # * ActiveRecord::ConnectionAdapters::SQLite3Adapter + # * ActiveRecord::ConnectionAdapters::PostgreSQLAdapter # def validates_uniqueness_of(*attr_names) validates_with UniquenessValidator, _merge_attributes(attr_names) diff --git a/activerecord/test/cases/adapters/mysql/schema_test.rb b/activerecord/test/cases/adapters/mysql/schema_test.rb new file mode 100644 index 0000000000..c6c1d1dad5 --- /dev/null +++ b/activerecord/test/cases/adapters/mysql/schema_test.rb @@ -0,0 +1,36 @@ +require "cases/helper" +require 'models/post' +require 'models/comment' + +module ActiveRecord + module ConnectionAdapters + class MysqlSchemaTest < ActiveRecord::TestCase + fixtures :posts + + def setup + @connection = ActiveRecord::Base.connection + db = Post.connection_pool.spec.config[:database] + table = Post.table_name + @db_name = db + + @omgpost = Class.new(Post) do + set_table_name "#{db}.#{table}" + def self.name; 'Post'; end + end + end + + def test_schema + assert @omgpost.find(:first) + end + + def test_table_exists? + name = @omgpost.table_name + assert @connection.table_exists?(name), "#{name} table should exist" + end + + def test_table_exists_wrong_schema + assert(!@connection.table_exists?("#{@db_name}.zomg"), "table should not exist") + end + end if current_adapter?(:MysqlAdapter) + end +end diff --git a/activerecord/test/cases/adapters/sqlite/sqlite_adapter_test.rb b/activerecord/test/cases/adapters/sqlite/sqlite_adapter_test.rb index ce0b2f5f5b..d1fc470907 100644 --- a/activerecord/test/cases/adapters/sqlite/sqlite_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite/sqlite_adapter_test.rb @@ -1,8 +1,13 @@ +# encoding: utf-8 require "cases/helper" +require 'models/binary' module ActiveRecord module ConnectionAdapters class SQLiteAdapterTest < ActiveRecord::TestCase + class DualEncoding < ActiveRecord::Base + end + def setup @ctx = Base.sqlite3_connection :database => ':memory:', :adapter => 'sqlite3', @@ -15,6 +20,20 @@ module ActiveRecord eosql end + def test_quote_binary_column_escapes_it + DualEncoding.connection.execute(<<-eosql) + CREATE TABLE dual_encodings ( + id integer PRIMARY KEY AUTOINCREMENT, + name string, + data binary + ) + eosql + str = "\x80".force_encoding("ASCII-8BIT") + binary = DualEncoding.new :name => 'いただきます!', :data => str + binary.save! + assert_equal str, binary.data + end + def test_execute @ctx.execute "INSERT INTO items (number) VALUES (10)" records = @ctx.execute "SELECT * FROM items" diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb index 0742e311d9..39e8a7960a 100644 --- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb +++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb @@ -15,17 +15,17 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase def test_eager_association_loading_with_cascaded_two_levels authors = Author.find(:all, :include=>{:posts=>:comments}, :order=>"authors.id") - assert_equal 2, authors.size + assert_equal 3, authors.size assert_equal 5, authors[0].posts.size - assert_equal 1, authors[1].posts.size + assert_equal 3, authors[1].posts.size assert_equal 10, authors[0].posts.collect{|post| post.comments.size }.inject(0){|sum,i| sum+i} end def test_eager_association_loading_with_cascaded_two_levels_and_one_level authors = Author.find(:all, :include=>[{:posts=>:comments}, :categorizations], :order=>"authors.id") - assert_equal 2, authors.size + assert_equal 3, authors.size assert_equal 5, authors[0].posts.size - assert_equal 1, authors[1].posts.size + assert_equal 3, authors[1].posts.size assert_equal 10, authors[0].posts.collect{|post| post.comments.size }.inject(0){|sum,i| sum+i} assert_equal 1, authors[0].categorizations.size assert_equal 2, authors[1].categorizations.size @@ -51,24 +51,24 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase categories = Category.joins(:categorizations).includes([{:posts=>:comments}, :authors]) assert_nothing_raised do - assert_equal 2, categories.count - assert_equal 2, categories.all.uniq.size # Must uniq since instantiating with inner joins will get dupes + assert_equal 3, categories.count + assert_equal 3, categories.all.uniq.size # Must uniq since instantiating with inner joins will get dupes end end def test_cascaded_eager_association_loading_with_duplicated_includes categories = Category.includes(:categorizations).includes(:categorizations => :author).where("categorizations.id is not null") assert_nothing_raised do - assert_equal 2, categories.count - assert_equal 2, categories.all.size + assert_equal 3, categories.count + assert_equal 3, categories.all.size end end def test_cascaded_eager_association_loading_with_twice_includes_edge_cases categories = Category.includes(:categorizations => :author).includes(:categorizations => :post).where("posts.id is not null") assert_nothing_raised do - assert_equal 2, categories.count - assert_equal 2, categories.all.size + assert_equal 3, categories.count + assert_equal 3, categories.all.size end end @@ -81,15 +81,15 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase def test_eager_association_loading_with_cascaded_two_levels_with_two_has_many_associations authors = Author.find(:all, :include=>{:posts=>[:comments, :categorizations]}, :order=>"authors.id") - assert_equal 2, authors.size + assert_equal 3, authors.size assert_equal 5, authors[0].posts.size - assert_equal 1, authors[1].posts.size + assert_equal 3, authors[1].posts.size assert_equal 10, authors[0].posts.collect{|post| post.comments.size }.inject(0){|sum,i| sum+i} end def test_eager_association_loading_with_cascaded_two_levels_and_self_table_reference authors = Author.find(:all, :include=>{:posts=>[:comments, :author]}, :order=>"authors.id") - assert_equal 2, authors.size + assert_equal 3, authors.size assert_equal 5, authors[0].posts.size assert_equal authors(:david).name, authors[0].name assert_equal [authors(:david).name], authors[0].posts.collect{|post| post.author.name}.uniq @@ -157,9 +157,9 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase def test_eager_association_loading_where_first_level_returns_nil authors = Author.find(:all, :include => {:post_about_thinking => :comments}, :order => 'authors.id DESC') - assert_equal [authors(:mary), authors(:david)], authors + assert_equal [authors(:bob), authors(:mary), authors(:david)], authors assert_no_queries do - authors[1].post_about_thinking.comments.first + authors[2].post_about_thinking.comments.first end end end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 48406a9b98..40c82f2fb8 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -68,8 +68,8 @@ class EagerAssociationTest < ActiveRecord::TestCase def test_with_ordering list = Post.find(:all, :include => :comments, :order => "posts.id DESC") - [:eager_other, :sti_habtm, :sti_post_and_comments, :sti_comments, - :authorless, :thinking, :welcome + [:other_by_mary, :other_by_bob, :misc_by_mary, :misc_by_bob, :eager_other, + :sti_habtm, :sti_post_and_comments, :sti_comments, :authorless, :thinking, :welcome ].each_with_index do |post, index| assert_equal posts(post), list[index] end @@ -97,25 +97,25 @@ class EagerAssociationTest < ActiveRecord::TestCase def test_preloading_has_many_in_multiple_queries_with_more_ids_than_database_can_handle Post.connection.expects(:in_clause_length).at_least_once.returns(5) posts = Post.find(:all, :include=>:comments) - assert_equal 7, posts.size + assert_equal 11, posts.size end def test_preloading_has_many_in_one_queries_when_database_has_no_limit_on_ids_it_can_handle Post.connection.expects(:in_clause_length).at_least_once.returns(nil) posts = Post.find(:all, :include=>:comments) - assert_equal 7, posts.size + assert_equal 11, posts.size end def test_preloading_habtm_in_multiple_queries_with_more_ids_than_database_can_handle Post.connection.expects(:in_clause_length).at_least_once.returns(5) posts = Post.find(:all, :include=>:categories) - assert_equal 7, posts.size + assert_equal 11, posts.size end def test_preloading_habtm_in_one_queries_when_database_has_no_limit_on_ids_it_can_handle Post.connection.expects(:in_clause_length).at_least_once.returns(nil) posts = Post.find(:all, :include=>:categories) - assert_equal 7, posts.size + assert_equal 11, posts.size end def test_load_associated_records_in_one_query_when_adapter_has_no_limit diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index dc382c3007..f4d14853d3 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -604,7 +604,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase project = SpecialProject.create("name" => "Special Project") assert developer.save developer.projects << project - developer.update_attribute("name", "Bruza") + developer.update_column("name", "Bruza") assert_equal 1, Developer.connection.select_value(<<-end_sql).to_i SELECT count(*) FROM developers_projects WHERE project_id = #{project.id} @@ -642,12 +642,12 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase def test_find_grouped all_posts_from_category1 = Post.find(:all, :conditions => "category_id = 1", :joins => :categories) grouped_posts_of_category1 = Post.find(:all, :conditions => "category_id = 1", :group => "author_id", :select => 'count(posts.id) as posts_count', :joins => :categories) - assert_equal 4, all_posts_from_category1.size - assert_equal 1, grouped_posts_of_category1.size + assert_equal 5, all_posts_from_category1.size + assert_equal 2, grouped_posts_of_category1.size end def test_find_scoped_grouped - assert_equal 4, categories(:general).posts_grouped_by_title.size + assert_equal 5, categories(:general).posts_grouped_by_title.size assert_equal 1, categories(:technology).posts_grouped_by_title.size end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index ad774eb9ce..16d4877fe8 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -639,7 +639,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_deleting_updates_counter_cache_with_dependent_delete_all post = posts(:welcome) - post.update_attribute(:taggings_with_delete_all_count, post.taggings_count) + post.update_column(:taggings_with_delete_all_count, post.taggings_count) assert_difference "post.reload.taggings_with_delete_all_count", -1 do post.taggings_with_delete_all.delete(post.taggings_with_delete_all.first) @@ -648,7 +648,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_deleting_updates_counter_cache_with_dependent_destroy post = posts(:welcome) - post.update_attribute(:taggings_with_destroy_count, post.taggings_count) + post.update_column(:taggings_with_destroy_count, post.taggings_count) assert_difference "post.reload.taggings_with_destroy_count", -1 do post.taggings_with_destroy.delete(post.taggings_with_destroy.first) @@ -787,7 +787,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase firm = Firm.find(:first) # break the vanilla firm_id foreign key assert_equal 2, firm.clients.count - firm.clients.first.update_attribute(:firm_id, nil) + firm.clients.first.update_column(:firm_id, nil) assert_equal 1, firm.clients(true).count assert_equal 1, firm.clients_using_primary_key_with_delete_all.count old_record = firm.clients_using_primary_key_with_delete_all.first diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index efdecd4b09..1efe3420a0 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -17,9 +17,10 @@ require 'models/developer' require 'models/subscriber' require 'models/book' require 'models/subscription' -require 'models/categorization' -require 'models/category' require 'models/essay' +require 'models/category' +require 'models/owner' +require 'models/categorization' require 'models/member' require 'models/membership' require 'models/club' @@ -27,7 +28,7 @@ require 'models/club' class HasManyThroughAssociationsTest < ActiveRecord::TestCase fixtures :posts, :readers, :people, :comments, :authors, :categories, :taggings, :tags, :owners, :pets, :toys, :jobs, :references, :companies, :members, :author_addresses, - :subscribers, :books, :subscriptions, :developers, :categorizations + :subscribers, :books, :subscriptions, :developers, :categorizations, :essays # Dummies to force column loads so query counts are clean. def setup @@ -285,7 +286,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase def test_update_counter_caches_on_delete_with_dependent_destroy post = posts(:welcome) tag = post.tags.create!(:name => 'doomed') - post.update_attribute(:tags_with_destroy_count, post.tags.count) + post.update_column(:tags_with_destroy_count, post.tags.count) assert_difference ['post.reload.taggings_count', 'post.reload.tags_with_destroy_count'], -1 do posts(:welcome).tags_with_destroy.delete(tag) @@ -295,7 +296,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase def test_update_counter_caches_on_delete_with_dependent_nullify post = posts(:welcome) tag = post.tags.create!(:name => 'doomed') - post.update_attribute(:tags_with_nullify_count, post.tags.count) + post.update_column(:tags_with_nullify_count, post.tags.count) assert_no_difference 'post.reload.taggings_count' do assert_difference 'post.reload.tags_with_nullify_count', -1 do @@ -656,6 +657,25 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert author.comments.include?(comment) end + def test_has_many_through_polymorphic_with_primary_key_option + assert_equal [categories(:general)], authors(:david).essay_categories + + authors = Author.joins(:essay_categories).where('categories.id' => categories(:general).id) + assert_equal authors(:david), authors.first + + assert_equal [owners(:blackbeard)], authors(:david).essay_owners + + authors = Author.joins(:essay_owners).where("owners.name = 'blackbeard'") + assert_equal authors(:david), authors.first + end + + def test_has_many_through_with_primary_key_option + assert_equal [categories(:general)], authors(:david).essay_categories_2 + + authors = Author.joins(:essay_categories_2).where('categories.id' => categories(:general).id) + assert_equal authors(:david), authors.first + end + def test_size_of_through_association_should_increase_correctly_when_has_many_association_is_added post = posts(:thinking) readers = post.readers.size @@ -679,10 +699,10 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end def test_joining_has_many_through_belongs_to - posts = Post.joins(:author_categorizations). + posts = Post.joins(:author_categorizations).order('posts.id'). where('categorizations.id' => categorizations(:mary_thinking_sti).id) - assert_equal [posts(:eager_other)], posts + assert_equal [posts(:eager_other), posts(:misc_by_mary), posts(:other_by_mary)], posts end def test_select_chosen_fields_only diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index bfc5ddc747..968025ade8 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -9,13 +9,16 @@ require 'models/member_detail' require 'models/minivan' require 'models/dashboard' require 'models/speedometer' +require 'models/category' require 'models/author' +require 'models/essay' +require 'models/owner' require 'models/post' require 'models/comment' class HasOneThroughAssociationsTest < ActiveRecord::TestCase fixtures :member_types, :members, :clubs, :memberships, :sponsors, :organizations, :minivans, - :dashboards, :speedometers, :authors, :posts, :comments + :dashboards, :speedometers, :authors, :posts, :comments, :categories, :essays, :owners def setup @member = members(:groucho) @@ -87,12 +90,12 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase def test_has_one_through_with_conditions_eager_loading # conditions on the through table assert_equal clubs(:moustache_club), Member.find(@member.id, :include => :favourite_club).favourite_club - memberships(:membership_of_favourite_club).update_attribute(:favourite, false) + memberships(:membership_of_favourite_club).update_column(:favourite, false) assert_equal nil, Member.find(@member.id, :include => :favourite_club).reload.favourite_club # conditions on the source table assert_equal clubs(:moustache_club), Member.find(@member.id, :include => :hairy_club).hairy_club - clubs(:moustache_club).update_attribute(:name, "Association of Clean-Shaven Persons") + clubs(:moustache_club).update_column(:name, "Association of Clean-Shaven Persons") assert_equal nil, Member.find(@member.id, :include => :hairy_club).reload.hairy_club end @@ -242,6 +245,25 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase end end + def test_has_one_through_polymorphic_with_primary_key_option + assert_equal categories(:general), authors(:david).essay_category + + authors = Author.joins(:essay_category).where('categories.id' => categories(:general).id) + assert_equal authors(:david), authors.first + + assert_equal owners(:blackbeard), authors(:david).essay_owner + + authors = Author.joins(:essay_owner).where("owners.name = 'blackbeard'") + assert_equal authors(:david), authors.first + end + + def test_has_one_through_with_primary_key_option + assert_equal categories(:general), authors(:david).essay_category_2 + + authors = Author.joins(:essay_category_2).where('categories.id' => categories(:general).id) + assert_equal authors(:david), authors.first + end + def test_has_one_through_with_default_scope_on_join_model assert_equal posts(:welcome).comments.order('id').first, authors(:david).comment_on_first_post end diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 19303fef9f..5a7b139030 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -2,6 +2,7 @@ require "cases/helper" require 'models/tag' require 'models/tagging' require 'models/post' +require 'models/rating' require 'models/item' require 'models/comment' require 'models/author' @@ -160,7 +161,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_delete_polymorphic_has_many_with_delete_all assert_equal 1, posts(:welcome).taggings.count - posts(:welcome).taggings.first.update_attribute :taggable_type, 'PostWithHasManyDeleteAll' + posts(:welcome).taggings.first.update_column :taggable_type, 'PostWithHasManyDeleteAll' post = find_post_with_dependency(1, :has_many, :taggings, :delete_all) old_count = Tagging.count @@ -171,7 +172,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_delete_polymorphic_has_many_with_destroy assert_equal 1, posts(:welcome).taggings.count - posts(:welcome).taggings.first.update_attribute :taggable_type, 'PostWithHasManyDestroy' + posts(:welcome).taggings.first.update_column :taggable_type, 'PostWithHasManyDestroy' post = find_post_with_dependency(1, :has_many, :taggings, :destroy) old_count = Tagging.count @@ -182,7 +183,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_delete_polymorphic_has_many_with_nullify assert_equal 1, posts(:welcome).taggings.count - posts(:welcome).taggings.first.update_attribute :taggable_type, 'PostWithHasManyNullify' + posts(:welcome).taggings.first.update_column :taggable_type, 'PostWithHasManyNullify' post = find_post_with_dependency(1, :has_many, :taggings, :nullify) old_count = Tagging.count @@ -193,7 +194,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_delete_polymorphic_has_one_with_destroy assert posts(:welcome).tagging - posts(:welcome).tagging.update_attribute :taggable_type, 'PostWithHasOneDestroy' + posts(:welcome).tagging.update_column :taggable_type, 'PostWithHasOneDestroy' post = find_post_with_dependency(1, :has_one, :tagging, :destroy) old_count = Tagging.count @@ -204,7 +205,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_delete_polymorphic_has_one_with_nullify assert posts(:welcome).tagging - posts(:welcome).tagging.update_attribute :taggable_type, 'PostWithHasOneNullify' + posts(:welcome).tagging.update_column :taggable_type, 'PostWithHasOneNullify' post = find_post_with_dependency(1, :has_one, :tagging, :nullify) old_count = Tagging.count @@ -288,7 +289,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase end def test_has_many_going_through_join_model_with_custom_foreign_key - assert_equal [], posts(:thinking).authors + assert_equal [authors(:bob)], posts(:thinking).authors assert_equal [authors(:mary)], posts(:authorless).authors end @@ -305,7 +306,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase end def test_has_many_through_with_custom_primary_key_on_has_many_source - assert_equal [authors(:david)], posts(:thinking).authors_using_custom_pk + assert_equal [authors(:david), authors(:bob)], posts(:thinking).authors_using_custom_pk.order('authors.id') end def test_both_scoped_and_explicit_joins_should_be_respected @@ -399,14 +400,6 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase end end - def test_has_many_through_has_many_through - assert_raise(ActiveRecord::HasManyThroughSourceAssociationMacroError) { authors(:david).tags } - end - - def test_has_many_through_habtm - assert_raise(ActiveRecord::HasManyThroughSourceAssociationMacroError) { authors(:david).post_categories } - end - def test_eager_load_has_many_through_has_many author = Author.find :first, :conditions => ['name = ?', 'David'], :include => :comments, :order => 'comments.id' SpecialComment.new; VerySpecialComment.new @@ -714,7 +707,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase # create dynamic Post models to allow different dependency options def find_post_with_dependency(post_id, association, association_name, dependency) class_name = "PostWith#{association.to_s.classify}#{dependency.to_s.classify}" - Post.find(post_id).update_attribute :type, class_name + Post.find(post_id).update_column :type, class_name klass = Object.const_set(class_name, Class.new(ActiveRecord::Base)) klass.set_table_name 'posts' klass.send(association, association_name, :as => :taggable, :dependent => dependency) diff --git a/activerecord/test/cases/associations/nested_through_associations_test.rb b/activerecord/test/cases/associations/nested_through_associations_test.rb new file mode 100644 index 0000000000..dd450a2a8e --- /dev/null +++ b/activerecord/test/cases/associations/nested_through_associations_test.rb @@ -0,0 +1,546 @@ +require "cases/helper" +require 'models/author' +require 'models/post' +require 'models/person' +require 'models/reference' +require 'models/job' +require 'models/reader' +require 'models/comment' +require 'models/tag' +require 'models/tagging' +require 'models/subscriber' +require 'models/book' +require 'models/subscription' +require 'models/rating' +require 'models/member' +require 'models/member_detail' +require 'models/member_type' +require 'models/sponsor' +require 'models/club' +require 'models/organization' +require 'models/category' +require 'models/categorization' +require 'models/membership' +require 'models/essay' + +class NestedThroughAssociationsTest < ActiveRecord::TestCase + fixtures :authors, :books, :posts, :subscriptions, :subscribers, :tags, :taggings, + :people, :readers, :references, :jobs, :ratings, :comments, :members, :member_details, + :member_types, :sponsors, :clubs, :organizations, :categories, :categories_posts, + :categorizations, :memberships, :essays + + # Through associations can either use the has_many or has_one macros. + # + # has_many + # - Source reflection can be has_many, has_one, belongs_to or has_and_belongs_to_many + # - Through reflection can be has_many, has_one, belongs_to or has_and_belongs_to_many + # + # has_one + # - Source reflection can be has_one or belongs_to + # - Through reflection can be has_one or belongs_to + # + # Additionally, the source reflection and/or through reflection may be subject to + # polymorphism and/or STI. + # + # When testing these, we need to make sure it works via loading the association directly, or + # joining the association, or including the association. We also need to ensure that associations + # are readonly where relevant. + + # has_many through + # Source: has_many through + # Through: has_many + def test_has_many_through_has_many_with_has_many_through_source_reflection + general = tags(:general) + assert_equal [general, general], authors(:david).tags + end + + def test_has_many_through_has_many_with_has_many_through_source_reflection_preload + authors = assert_queries(5) { Author.includes(:tags).to_a } + general = tags(:general) + + assert_no_queries do + assert_equal [general, general], authors.first.tags + end + end + + def test_has_many_through_has_many_with_has_many_through_source_reflection_preload_via_joins + assert_includes_and_joins_equal( + Author.where('tags.id' => tags(:general).id), + [authors(:david)], :tags + ) + + # This ensures that the polymorphism of taggings is being observed correctly + authors = Author.joins(:tags).where('taggings.taggable_type' => 'FakeModel') + assert authors.empty? + end + + # has_many through + # Source: has_many + # Through: has_many through + def test_has_many_through_has_many_through_with_has_many_source_reflection + luke, david = subscribers(:first), subscribers(:second) + assert_equal [luke, david, david], authors(:david).subscribers.order('subscribers.nick') + end + + def test_has_many_through_has_many_through_with_has_many_source_reflection_preload + luke, david = subscribers(:first), subscribers(:second) + authors = assert_queries(4) { Author.includes(:subscribers).to_a } + assert_no_queries do + assert_equal [luke, david, david], authors.first.subscribers.sort_by(&:nick) + end + end + + def test_has_many_through_has_many_through_with_has_many_source_reflection_preload_via_joins + # All authors with subscribers where one of the subscribers' nick is 'alterself' + assert_includes_and_joins_equal( + Author.where('subscribers.nick' => 'alterself'), + [authors(:david)], :subscribers + ) + end + + # has_many through + # Source: has_one through + # Through: has_one + def test_has_many_through_has_one_with_has_one_through_source_reflection + assert_equal [member_types(:founding)], members(:groucho).nested_member_types + end + + def test_has_many_through_has_one_with_has_one_through_source_reflection_preload + members = assert_queries(4) { Member.includes(:nested_member_types).to_a } + founding = member_types(:founding) + assert_no_queries do + assert_equal [founding], members.first.nested_member_types + end + end + + def test_has_many_through_has_one_with_has_one_through_source_reflection_preload_via_joins + assert_includes_and_joins_equal( + Member.where('member_types.id' => member_types(:founding).id), + [members(:groucho)], :nested_member_types + ) + end + + # has_many through + # Source: has_one + # Through: has_one through + def test_has_many_through_has_one_through_with_has_one_source_reflection + assert_equal [sponsors(:moustache_club_sponsor_for_groucho)], members(:groucho).nested_sponsors + end + + def test_has_many_through_has_one_through_with_has_one_source_reflection_preload + members = assert_queries(4) { Member.includes(:nested_sponsors).to_a } + mustache = sponsors(:moustache_club_sponsor_for_groucho) + assert_no_queries do + assert_equal [mustache], members.first.nested_sponsors + end + end + + def test_has_many_through_has_one_through_with_has_one_source_reflection_preload_via_joins + assert_includes_and_joins_equal( + Member.where('sponsors.id' => sponsors(:moustache_club_sponsor_for_groucho).id), + [members(:groucho)], :nested_sponsors + ) + end + + # has_many through + # Source: has_many through + # Through: has_one + def test_has_many_through_has_one_with_has_many_through_source_reflection + groucho_details, other_details = member_details(:groucho), member_details(:some_other_guy) + + assert_equal [groucho_details, other_details], + members(:groucho).organization_member_details.order('member_details.id') + end + + def test_has_many_through_has_one_with_has_many_through_source_reflection_preload + members = assert_queries(4) { Member.includes(:organization_member_details).to_a.sort_by(&:id) } + groucho_details, other_details = member_details(:groucho), member_details(:some_other_guy) + + assert_no_queries do + assert_equal [groucho_details, other_details], members.first.organization_member_details.sort_by(&:id) + end + end + + def test_has_many_through_has_one_with_has_many_through_source_reflection_preload_via_joins + assert_includes_and_joins_equal( + Member.where('member_details.id' => member_details(:groucho).id).order('member_details.id'), + [members(:groucho), members(:some_other_guy)], :organization_member_details + ) + + members = Member.joins(:organization_member_details). + where('member_details.id' => 9) + assert members.empty? + end + + # has_many through + # Source: has_many + # Through: has_one through + def test_has_many_through_has_one_through_with_has_many_source_reflection + groucho_details, other_details = member_details(:groucho), member_details(:some_other_guy) + + assert_equal [groucho_details, other_details], + members(:groucho).organization_member_details_2.order('member_details.id') + end + + def test_has_many_through_has_one_through_with_has_many_source_reflection_preload + members = assert_queries(4) { Member.includes(:organization_member_details_2).to_a.sort_by(&:id) } + groucho_details, other_details = member_details(:groucho), member_details(:some_other_guy) + + assert_no_queries do + assert_equal [groucho_details, other_details], members.first.organization_member_details_2.sort_by(&:id) + end + end + + def test_has_many_through_has_one_through_with_has_many_source_reflection_preload_via_joins + assert_includes_and_joins_equal( + Member.where('member_details.id' => member_details(:groucho).id).order('member_details.id'), + [members(:groucho), members(:some_other_guy)], :organization_member_details_2 + ) + + members = Member.joins(:organization_member_details_2). + where('member_details.id' => 9) + assert members.empty? + end + + # has_many through + # Source: has_and_belongs_to_many + # Through: has_many + def test_has_many_through_has_many_with_has_and_belongs_to_many_source_reflection + general, cooking = categories(:general), categories(:cooking) + + assert_equal [general, cooking], authors(:bob).post_categories.order('categories.id') + end + + def test_has_many_through_has_many_with_has_and_belongs_to_many_source_reflection_preload + authors = assert_queries(3) { Author.includes(:post_categories).to_a.sort_by(&:id) } + general, cooking = categories(:general), categories(:cooking) + + assert_no_queries do + assert_equal [general, cooking], authors[2].post_categories.sort_by(&:id) + end + end + + def test_has_many_through_has_many_with_has_and_belongs_to_many_source_reflection_preload_via_joins + assert_includes_and_joins_equal( + Author.where('categories.id' => categories(:cooking).id), + [authors(:bob)], :post_categories + ) + end + + # has_many through + # Source: has_many + # Through: has_and_belongs_to_many + def test_has_many_through_has_and_belongs_to_many_with_has_many_source_reflection + greetings, more = comments(:greetings), comments(:more_greetings) + + assert_equal [greetings, more], categories(:technology).post_comments.order('comments.id') + end + + def test_has_many_through_has_and_belongs_to_many_with_has_many_source_reflection_preload + categories = assert_queries(3) { Category.includes(:post_comments).to_a.sort_by(&:id) } + greetings, more = comments(:greetings), comments(:more_greetings) + + assert_no_queries do + assert_equal [greetings, more], categories[1].post_comments.sort_by(&:id) + end + end + + def test_has_many_through_has_and_belongs_to_many_with_has_many_source_reflection_preload_via_joins + assert_includes_and_joins_equal( + Category.where('comments.id' => comments(:more_greetings).id).order('comments.id'), + [categories(:general), categories(:technology)], :post_comments + ) + end + + # has_many through + # Source: has_many through a habtm + # Through: has_many through + def test_has_many_through_has_many_with_has_many_through_habtm_source_reflection + greetings, more = comments(:greetings), comments(:more_greetings) + + assert_equal [greetings, more], authors(:bob).category_post_comments.order('comments.id') + end + + def test_has_many_through_has_many_with_has_many_through_habtm_source_reflection_preload + authors = assert_queries(5) { Author.includes(:category_post_comments).to_a.sort_by(&:id) } + greetings, more = comments(:greetings), comments(:more_greetings) + + assert_no_queries do + assert_equal [greetings, more], authors[2].category_post_comments.sort_by(&:id) + end + end + + def test_has_many_through_has_many_with_has_many_through_habtm_source_reflection_preload_via_joins + assert_includes_and_joins_equal( + Author.where('comments.id' => comments(:does_it_hurt).id).order('authors.id'), + [authors(:david), authors(:mary)], :category_post_comments + ) + end + + # has_many through + # Source: belongs_to + # Through: has_many through + def test_has_many_through_has_many_through_with_belongs_to_source_reflection + assert_equal [tags(:general), tags(:general)], authors(:david).tagging_tags + end + + def test_has_many_through_has_many_through_with_belongs_to_source_reflection_preload + authors = assert_queries(5) { Author.includes(:tagging_tags).to_a } + general = tags(:general) + + assert_no_queries do + assert_equal [general, general], authors.first.tagging_tags + end + end + + def test_has_many_through_has_many_through_with_belongs_to_source_reflection_preload_via_joins + assert_includes_and_joins_equal( + Author.where('tags.id' => tags(:general).id), + [authors(:david)], :tagging_tags + ) + end + + # has_many through + # Source: has_many through + # Through: belongs_to + def test_has_many_through_belongs_to_with_has_many_through_source_reflection + welcome_general, thinking_general = taggings(:welcome_general), taggings(:thinking_general) + + assert_equal [welcome_general, thinking_general], + categorizations(:david_welcome_general).post_taggings.order('taggings.id') + end + + def test_has_many_through_belongs_to_with_has_many_through_source_reflection_preload + categorizations = assert_queries(4) { Categorization.includes(:post_taggings).to_a.sort_by(&:id) } + welcome_general, thinking_general = taggings(:welcome_general), taggings(:thinking_general) + + assert_no_queries do + assert_equal [welcome_general, thinking_general], categorizations.first.post_taggings.sort_by(&:id) + end + end + + def test_has_many_through_belongs_to_with_has_many_through_source_reflection_preload_via_joins + assert_includes_and_joins_equal( + Categorization.where('taggings.id' => taggings(:welcome_general).id).order('taggings.id'), + [categorizations(:david_welcome_general)], :post_taggings + ) + end + + # has_one through + # Source: has_one through + # Through: has_one + def test_has_one_through_has_one_with_has_one_through_source_reflection + assert_equal member_types(:founding), members(:groucho).nested_member_type + end + + def test_has_one_through_has_one_with_has_one_through_source_reflection_preload + members = assert_queries(4) { Member.includes(:nested_member_type).to_a.sort_by(&:id) } + founding = member_types(:founding) + + assert_no_queries do + assert_equal founding, members.first.nested_member_type + end + end + + def test_has_one_through_has_one_with_has_one_through_source_reflection_preload_via_joins + assert_includes_and_joins_equal( + Member.where('member_types.id' => member_types(:founding).id), + [members(:groucho)], :nested_member_type + ) + end + + # has_one through + # Source: belongs_to + # Through: has_one through + def test_has_one_through_has_one_through_with_belongs_to_source_reflection + assert_equal categories(:general), members(:groucho).club_category + end + + def test_has_one_through_has_one_through_with_belongs_to_source_reflection_preload + members = assert_queries(4) { Member.includes(:club_category).to_a.sort_by(&:id) } + general = categories(:general) + + assert_no_queries do + assert_equal general, members.first.club_category + end + end + + def test_has_one_through_has_one_through_with_belongs_to_source_reflection_preload_via_joins + assert_includes_and_joins_equal( + Member.where('categories.id' => categories(:technology).id), + [members(:blarpy_winkup)], :club_category + ) + end + + def test_distinct_has_many_through_a_has_many_through_association_on_source_reflection + author = authors(:david) + assert_equal [tags(:general)], author.distinct_tags + end + + def test_distinct_has_many_through_a_has_many_through_association_on_through_reflection + author = authors(:david) + assert_equal [subscribers(:first), subscribers(:second)], + author.distinct_subscribers.order('subscribers.nick') + end + + def test_nested_has_many_through_with_a_table_referenced_multiple_times + author = authors(:bob) + assert_equal [posts(:misc_by_bob), posts(:misc_by_mary), posts(:other_by_bob), posts(:other_by_mary)], + author.similar_posts.sort_by(&:id) + + # Mary and Bob both have posts in misc, but they are the only ones. + authors = Author.joins(:similar_posts).where('posts.id' => posts(:misc_by_bob).id) + assert_equal [authors(:mary), authors(:bob)], authors.uniq.sort_by(&:id) + + # Check the polymorphism of taggings is being observed correctly (in both joins) + authors = Author.joins(:similar_posts).where('taggings.taggable_type' => 'FakeModel') + assert authors.empty? + authors = Author.joins(:similar_posts).where('taggings_authors_join.taggable_type' => 'FakeModel') + assert authors.empty? + end + + def test_has_many_through_with_foreign_key_option_on_through_reflection + assert_equal [posts(:welcome), posts(:authorless)], people(:david).agents_posts.order('posts.id') + assert_equal [authors(:david)], references(:david_unicyclist).agents_posts_authors + + references = Reference.joins(:agents_posts_authors).where('authors.id' => authors(:david).id) + assert_equal [references(:david_unicyclist)], references + end + + def test_has_many_through_with_foreign_key_option_on_source_reflection + assert_equal [people(:michael), people(:susan)], jobs(:unicyclist).agents.order('people.id') + + jobs = Job.joins(:agents) + assert_equal [jobs(:unicyclist), jobs(:unicyclist)], jobs + end + + def test_has_many_through_with_sti_on_through_reflection + ratings = posts(:sti_comments).special_comments_ratings.sort_by(&:id) + assert_equal [ratings(:special_comment_rating), ratings(:sub_special_comment_rating)], ratings + + # Ensure STI is respected in the join + scope = Post.joins(:special_comments_ratings).where(:id => posts(:sti_comments).id) + assert scope.where("comments.type" => "Comment").empty? + assert !scope.where("comments.type" => "SpecialComment").empty? + assert !scope.where("comments.type" => "SubSpecialComment").empty? + end + + def test_has_many_through_with_sti_on_nested_through_reflection + taggings = posts(:sti_comments).special_comments_ratings_taggings + assert_equal [taggings(:special_comment_rating)], taggings + + scope = Post.joins(:special_comments_ratings_taggings).where(:id => posts(:sti_comments).id) + assert scope.where("comments.type" => "Comment").empty? + assert !scope.where("comments.type" => "SpecialComment").empty? + end + + def test_nested_has_many_through_writers_should_raise_error + david = authors(:david) + subscriber = subscribers(:first) + + assert_raises(ActiveRecord::HasManyThroughNestedAssociationsAreReadonly) do + david.subscribers = [subscriber] + end + + assert_raises(ActiveRecord::HasManyThroughNestedAssociationsAreReadonly) do + david.subscriber_ids = [subscriber.id] + end + + assert_raises(ActiveRecord::HasManyThroughNestedAssociationsAreReadonly) do + david.subscribers << subscriber + end + + assert_raises(ActiveRecord::HasManyThroughNestedAssociationsAreReadonly) do + david.subscribers.delete(subscriber) + end + + assert_raises(ActiveRecord::HasManyThroughNestedAssociationsAreReadonly) do + david.subscribers.clear + end + + assert_raises(ActiveRecord::HasManyThroughNestedAssociationsAreReadonly) do + david.subscribers.build + end + + assert_raises(ActiveRecord::HasManyThroughNestedAssociationsAreReadonly) do + david.subscribers.create + end + end + + def test_nested_has_one_through_writers_should_raise_error + groucho = members(:groucho) + founding = member_types(:founding) + + assert_raises(ActiveRecord::HasManyThroughNestedAssociationsAreReadonly) do + groucho.nested_member_type = founding + end + end + + def test_nested_has_many_through_with_conditions_on_through_associations + assert_equal [tags(:blue)], authors(:bob).misc_post_first_blue_tags + end + + def test_nested_has_many_through_with_conditions_on_through_associations_preload + assert Author.where('tags.id' => 100).joins(:misc_post_first_blue_tags).empty? + + authors = assert_queries(3) { Author.includes(:misc_post_first_blue_tags).to_a.sort_by(&:id) } + blue = tags(:blue) + + assert_no_queries do + assert_equal [blue], authors[2].misc_post_first_blue_tags + end + end + + def test_nested_has_many_through_with_conditions_on_through_associations_preload_via_joins + # Pointless condition to force single-query loading + assert_includes_and_joins_equal( + Author.where('tags.id = tags.id'), + [authors(:bob)], :misc_post_first_blue_tags + ) + end + + def test_nested_has_many_through_with_conditions_on_source_associations + assert_equal [tags(:blue)], authors(:bob).misc_post_first_blue_tags_2 + end + + def test_nested_has_many_through_with_conditions_on_source_associations_preload + authors = assert_queries(4) { Author.includes(:misc_post_first_blue_tags_2).to_a.sort_by(&:id) } + blue = tags(:blue) + + assert_no_queries do + assert_equal [blue], authors[2].misc_post_first_blue_tags_2 + end + end + + def test_nested_has_many_through_with_conditions_on_source_associations_preload_via_joins + # Pointless condition to force single-query loading + assert_includes_and_joins_equal( + Author.where('tags.id = tags.id'), + [authors(:bob)], :misc_post_first_blue_tags_2 + ) + end + + def test_nested_has_many_through_with_foreign_key_option_on_the_source_reflection_through_reflection + assert_equal [categories(:general)], organizations(:nsa).author_essay_categories + + organizations = Organization.joins(:author_essay_categories). + where('categories.id' => categories(:general).id) + assert_equal [organizations(:nsa)], organizations + + assert_equal categories(:general), organizations(:nsa).author_owned_essay_category + + organizations = Organization.joins(:author_owned_essay_category). + where('categories.id' => categories(:general).id) + assert_equal [organizations(:nsa)], organizations + end + + private + + def assert_includes_and_joins_equal(query, expected, association) + actual = assert_queries(1) { query.joins(association).to_a.uniq } + assert_equal expected, actual + + actual = assert_queries(1) { query.includes(association).to_a.uniq } + assert_equal expected, actual + end +end diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 47b8e48582..04f628a398 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -66,7 +66,7 @@ class AssociationsTest < ActiveRecord::TestCase ship = Ship.create!(:name => "The good ship Dollypop") part = ship.parts.create!(:name => "Mast") part.mark_for_destruction - ShipPart.find(part.id).update_attribute(:name, 'Deck') + ShipPart.find(part.id).update_column(:name, 'Deck') ship.parts.send(:load_target) assert_equal 'Deck', ship.parts[0].name end @@ -170,7 +170,7 @@ class AssociationProxyTest < ActiveRecord::TestCase david = developers(:david) assert !david.projects.loaded? - david.update_attribute(:created_at, Time.now) + david.update_column(:created_at, Time.now) assert !david.projects.loaded? end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index dfacf58da8..84f75cc803 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -118,22 +118,39 @@ class AttributeMethodsTest < ActiveRecord::TestCase end def test_read_attributes_before_type_cast_on_datetime - developer = Developer.find(:first) - if current_adapter?(:Mysql2Adapter, :OracleAdapter) - # Mysql2 and Oracle adapters keep the value in Time instance - assert_equal developer.created_at.to_s(:db), developer.attributes_before_type_cast["created_at"].to_s(:db) - else - assert_equal developer.created_at.to_s(:db), developer.attributes_before_type_cast["created_at"].to_s + in_time_zone "Pacific Time (US & Canada)" do + record = @target.new + + record.written_on = "345643456" + assert_equal "345643456", record.written_on_before_type_cast + assert_equal nil, record.written_on + + record.written_on = "2009-10-11 12:13:14" + assert_equal "2009-10-11 12:13:14", record.written_on_before_type_cast + assert_equal Time.zone.parse("2009-10-11 12:13:14"), record.written_on + assert_equal ActiveSupport::TimeZone["Pacific Time (US & Canada)"], record.written_on.time_zone end + end + + def test_read_attributes_after_type_cast_on_datetime + tz = "Pacific Time (US & Canada)" + + in_time_zone tz do + record = @target.new + + date_string = "2011-03-24" + time = Time.zone.parse date_string - developer.created_at = "345643456" + record.written_on = date_string + assert_equal date_string, record.written_on_before_type_cast + assert_equal time, record.written_on + assert_equal ActiveSupport::TimeZone[tz], record.written_on.time_zone - assert_equal developer.created_at_before_type_cast, "345643456" - assert_equal developer.created_at, nil + record.save + record.reload - developer.created_at = "2010-03-21 21:23:32" - assert_equal developer.created_at_before_type_cast, "2010-03-21 21:23:32" - assert_equal developer.created_at, Time.parse("2010-03-21 21:23:32") + assert_equal time, record.written_on + end end def test_hash_content diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index d03fc68a11..aeb0b28bab 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -45,6 +45,8 @@ class ReadonlyTitlePost < Post attr_readonly :title end +class Weird < ActiveRecord::Base; end + class Boolean < ActiveRecord::Base; end class BasicsTest < ActiveRecord::TestCase @@ -477,6 +479,16 @@ class BasicsTest < ActiveRecord::TestCase assert_equal "changed", post.body end + def test_non_valid_identifier_column_name + weird = Weird.create('a$b' => 'value') + weird.reload + assert_equal 'value', weird.send('a$b') + + weird.update_column('a$b', 'value2') + weird.reload + assert_equal 'value2', weird.send('a$b') + end + def test_multiparameter_attributes_on_date attributes = { "last_read(1i)" => "2004", "last_read(2i)" => "6", "last_read(3i)" => "24" } topic = Topic.find(1) diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index 6f65fb96d1..dc0e0da4c5 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -25,7 +25,7 @@ class EachTest < ActiveRecord::TestCase end def test_each_should_execute_if_id_is_in_select - assert_queries(4) do + assert_queries(6) do Post.find_each(:select => "id, title, type", :batch_size => 2) do |post| assert_kind_of Post, post end diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index caf07a7357..c97f1ae634 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -311,8 +311,8 @@ class CalculationsTest < ActiveRecord::TestCase def test_should_count_scoped_select_with_options Account.update_all("credit_limit = NULL") - Account.last.update_attribute('credit_limit', 49) - Account.first.update_attribute('credit_limit', 51) + Account.last.update_column('credit_limit', 49) + Account.first.update_column('credit_limit', 51) assert_equal 1, Account.scoped(:select => "credit_limit").count(:conditions => ['credit_limit >= 50']) end diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index a6738fb654..b75482a956 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -413,7 +413,7 @@ class DirtyTest < ActiveRecord::TestCase with_partial_updates(Topic) do Topic.create!(:author_name => 'Bill', :content => {:a => "a"}) topic = Topic.select('id, author_name').first - topic.update_attribute :author_name, 'John' + topic.update_column :author_name, 'John' topic = Topic.first assert_not_nil topic.content end @@ -487,7 +487,8 @@ class DirtyTest < ActiveRecord::TestCase assert !pirate.previous_changes.key?('created_on') pirate = Pirate.find_by_catchphrase("Ahoy!") - pirate.update_attribute(:catchphrase, "Ninjas suck!") + pirate.catchphrase = "Ninjas suck!" + pirate.save(:validations => false) assert_equal 2, pirate.previous_changes.size assert_equal ["Ahoy!", "Ninjas suck!"], pirate.previous_changes['catchphrase'] diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index c35590b84b..543981b4a0 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -124,11 +124,13 @@ class FinderTest < ActiveRecord::TestCase def test_find_all_with_limit_and_offset_and_multiple_order_clauses first_three_posts = Post.find :all, :order => 'author_id, id', :limit => 3, :offset => 0 second_three_posts = Post.find :all, :order => ' author_id,id ', :limit => 3, :offset => 3 - last_posts = Post.find :all, :order => ' author_id, id ', :limit => 3, :offset => 6 + third_three_posts = Post.find :all, :order => ' author_id, id ', :limit => 3, :offset => 6 + last_posts = Post.find :all, :order => ' author_id, id ', :limit => 3, :offset => 9 assert_equal [[0,3],[1,1],[1,2]], first_three_posts.map { |p| [p.author_id, p.id] } assert_equal [[1,4],[1,5],[1,6]], second_three_posts.map { |p| [p.author_id, p.id] } - assert_equal [[2,7]], last_posts.map { |p| [p.author_id, p.id] } + assert_equal [[2,7],[2,9],[2,11]], third_three_posts.map { |p| [p.author_id, p.id] } + assert_equal [[3,8],[3,10]], last_posts.map { |p| [p.author_id, p.id] } end @@ -189,6 +191,30 @@ class FinderTest < ActiveRecord::TestCase assert_nil Topic.where("title = 'The Second Topic of the day!'").first end + def test_first_bang_present + assert_nothing_raised do + assert_equal topics(:second), Topic.where("title = 'The Second Topic of the day'").first! + end + end + + def test_first_bang_missing + assert_raises ActiveRecord::RecordNotFound do + Topic.where("title = 'This title does not exist'").first! + end + end + + def test_last_bang_present + assert_nothing_raised do + assert_equal topics(:second), Topic.where("title = 'The Second Topic of the day'").last! + end + end + + def test_last_bang_missing + assert_raises ActiveRecord::RecordNotFound do + Topic.where("title = 'This title does not exist'").last! + end + end + def test_unexisting_record_exception_handling assert_raise(ActiveRecord::RecordNotFound) { Topic.find(1).parent diff --git a/activerecord/test/cases/habtm_destroy_order_test.rb b/activerecord/test/cases/habtm_destroy_order_test.rb index 15598392e2..f2b91d977e 100644 --- a/activerecord/test/cases/habtm_destroy_order_test.rb +++ b/activerecord/test/cases/habtm_destroy_order_test.rb @@ -13,5 +13,39 @@ class HabtmDestroyOrderTest < ActiveRecord::TestCase sicp.destroy end end + assert !sicp.destroyed? + end + + test "not destroying a student with lessons leaves student<=>lesson association intact" do + # test a normal before_destroy doesn't destroy the habtm joins + begin + sicp = Lesson.new(:name => "SICP") + ben = Student.new(:name => "Ben Bitdiddle") + # add a before destroy to student + Student.class_eval do + before_destroy do + raise ActiveRecord::Rollback unless lessons.empty? + end + end + ben.lessons << sicp + ben.save! + ben.destroy + assert !ben.reload.lessons.empty? + ensure + # get rid of it so Student is still like it was + Student.reset_callbacks(:destroy) + end + end + + test "not destroying a lesson with students leaves student<=>lesson association intact" do + # test a more aggressive before_destroy doesn't destroy the habtm joins and still throws the exception + sicp = Lesson.new(:name => "SICP") + ben = Student.new(:name => "Ben Bitdiddle") + sicp.students << ben + sicp.save! + assert_raises LessonError do + sicp.destroy + end + assert !sicp.reload.students.empty? end end diff --git a/activerecord/test/cases/identity_map_test.rb b/activerecord/test/cases/identity_map_test.rb index 399d6c26df..89f7b92d09 100644 --- a/activerecord/test/cases/identity_map_test.rb +++ b/activerecord/test/cases/identity_map_test.rb @@ -207,31 +207,31 @@ class IdentityMapTest < ActiveRecord::TestCase def test_find_with_preloaded_associations assert_queries(2) do - posts = Post.preload(:comments) + posts = Post.preload(:comments).order('posts.id') assert posts.first.comments.first end # With IM we'll retrieve post object from previous query, it'll have comments # already preloaded from first call assert_queries(1) do - posts = Post.preload(:comments).to_a + posts = Post.preload(:comments).order('posts.id') assert posts.first.comments.first end assert_queries(2) do - posts = Post.preload(:author) + posts = Post.preload(:author).order('posts.id') assert posts.first.author end # With IM we'll retrieve post object from previous query, it'll have comments # already preloaded from first call assert_queries(1) do - posts = Post.preload(:author).to_a + posts = Post.preload(:author).order('posts.id') assert posts.first.author end assert_queries(1) do - posts = Post.preload(:author, :comments).to_a + posts = Post.preload(:author, :comments).order('posts.id') assert posts.first.author assert posts.first.comments.first end @@ -239,22 +239,22 @@ class IdentityMapTest < ActiveRecord::TestCase def test_find_with_included_associations assert_queries(2) do - posts = Post.includes(:comments) + posts = Post.includes(:comments).order('posts.id') assert posts.first.comments.first end assert_queries(1) do - posts = Post.scoped.includes(:comments) + posts = Post.scoped.includes(:comments).order('posts.id') assert posts.first.comments.first end assert_queries(2) do - posts = Post.includes(:author) + posts = Post.includes(:author).order('posts.id') assert posts.first.author end assert_queries(1) do - posts = Post.includes(:author, :comments).to_a + posts = Post.includes(:author, :comments).order('posts.id') assert posts.first.author assert posts.first.comments.first end diff --git a/activerecord/test/cases/json_serialization_test.rb b/activerecord/test/cases/json_serialization_test.rb index 5da7f9e1b9..8664d63e8f 100644 --- a/activerecord/test/cases/json_serialization_test.rb +++ b/activerecord/test/cases/json_serialization_test.rb @@ -181,7 +181,11 @@ class DatabaseConnectedJsonEncodingTest < ActiveRecord::TestCase def test_should_allow_except_option_for_list_of_authors ActiveRecord::Base.include_root_in_json = false authors = [@david, @mary] - assert_equal %([{"id":1},{"id":2}]), ActiveSupport::JSON.encode(authors, :except => [:name, :author_address_id, :author_address_extra_id]) + encoded = ActiveSupport::JSON.encode(authors, :except => [ + :name, :author_address_id, :author_address_extra_id, + :organization_id, :owned_essay_id + ]) + assert_equal %([{"id":1},{"id":2}]), encoded ensure ActiveRecord::Base.include_root_in_json = true end @@ -196,7 +200,7 @@ class DatabaseConnectedJsonEncodingTest < ActiveRecord::TestCase ) ['"name":"David"', '"posts":[', '{"id":1}', '{"id":2}', '{"id":4}', - '{"id":5}', '{"id":6}', '"name":"Mary"', '"posts":[{"id":7}]'].each do |fragment| + '{"id":5}', '{"id":6}', '"name":"Mary"', '"posts":[', '{"id":7}', '{"id":9}'].each do |fragment| assert json.include?(fragment), json end end diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 8ca9d626d1..68d861c9c2 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -327,66 +327,62 @@ class PersistencesTest < ActiveRecord::TestCase assert_raise(ActiveSupport::FrozenObjectError) { client.name = "something else" } end - def test_update_attribute - assert !Topic.find(1).approved? - Topic.find(1).update_attribute("approved", true) - assert Topic.find(1).approved? + def test_update_column + topic = Topic.find(1) + topic.update_column("approved", true) + assert topic.approved? + topic.reload + assert topic.approved? - Topic.find(1).update_attribute(:approved, false) - assert !Topic.find(1).approved? + topic.update_column(:approved, false) + assert !topic.approved? + topic.reload + assert !topic.approved? end - def test_update_attribute_for_readonly_attribute + def test_update_column_with_model_having_primary_key_other_than_id minivan = Minivan.find('m1') - assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_attribute(:color, 'black') } - end - - # This test is correct, but it is hard to fix it since - # update_attribute trigger simply call save! that triggers - # all callbacks. - # def test_update_attribute_with_one_changed_and_one_updated - # t = Topic.order('id').limit(1).first - # title, author_name = t.title, t.author_name - # t.author_name = 'John' - # t.update_attribute(:title, 'super_title') - # assert_equal 'John', t.author_name - # assert_equal 'super_title', t.title - # assert t.changed?, "topic should have changed" - # assert t.author_name_changed?, "author_name should have changed" - # assert !t.title_changed?, "title should not have changed" - # assert_nil t.title_change, 'title change should be nil' - # assert_equal ['author_name'], t.changed - # - # t.reload - # assert_equal 'David', t.author_name - # assert_equal 'super_title', t.title - # end - - def test_update_attribute_with_one_updated - t = Topic.first - title = t.title - t.update_attribute(:title, 'super_title') - assert_equal 'super_title', t.title - assert !t.changed?, "topic should not have changed" - assert !t.title_changed?, "title should not have changed" - assert_nil t.title_change, 'title change should be nil' + new_name = 'sebavan' - t.reload - assert_equal 'super_title', t.title + minivan.update_column(:name, new_name) + assert_equal new_name, minivan.name end - def test_update_attribute_for_updated_at_on + def test_update_column_for_readonly_attribute + minivan = Minivan.find('m1') + prev_color = minivan.color + assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_column(:color, 'black') } + assert_equal prev_color, minivan.color + end + + def test_update_column_should_not_modify_updated_at developer = Developer.find(1) prev_month = Time.now.prev_month - developer.update_attribute(:updated_at, prev_month) + developer.update_column(:updated_at, prev_month) assert_equal prev_month, developer.updated_at - developer.update_attribute(:salary, 80001) - assert_not_equal prev_month, developer.updated_at + developer.update_column(:salary, 80001) + assert_equal prev_month, developer.updated_at developer.reload - assert_not_equal prev_month, developer.updated_at + assert_equal prev_month, developer.updated_at + end + + def test_update_column_with_one_changed_and_one_updated + t = Topic.order('id').limit(1).first + title, author_name = t.title, t.author_name + t.author_name = 'John' + t.update_column(:title, 'super_title') + assert_equal 'John', t.author_name + assert_equal 'super_title', t.title + assert t.changed?, "topic should have changed" + assert t.author_name_changed?, "author_name should have changed" + assert t.title_changed?, "title should have changed" + + t.reload + assert_equal author_name, t.author_name + assert_equal 'super_title', t.title end def test_update_attributes diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index eb580928ba..97d9669483 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -7,9 +7,16 @@ require 'models/subscriber' require 'models/ship' require 'models/pirate' require 'models/price_estimate' -require 'models/tagging' +require 'models/essay' require 'models/author' +require 'models/organization' require 'models/post' +require 'models/tagging' +require 'models/category' +require 'models/book' +require 'models/subscriber' +require 'models/subscription' +require 'models/tag' require 'models/sponsor' class ReflectionTest < ActiveRecord::TestCase @@ -195,10 +202,54 @@ class ReflectionTest < ActiveRecord::TestCase assert_kind_of ThroughReflection, Subscriber.reflect_on_association(:books) end + def test_chain + expected = [ + Organization.reflect_on_association(:author_essay_categories), + Author.reflect_on_association(:essays), + Organization.reflect_on_association(:authors) + ] + actual = Organization.reflect_on_association(:author_essay_categories).chain + + assert_equal expected, actual + end + + def test_conditions + expected = [ + [{ :tags => { :name => 'Blue' } }], + [{ :taggings => { :comment => 'first' } }, { "taggable_type" => "Post" }], + [{ :posts => { :title => ['misc post by bob', 'misc post by mary'] } }] + ] + actual = Author.reflect_on_association(:misc_post_first_blue_tags).conditions + assert_equal expected, actual + + expected = [ + [{ :tags => { :name => 'Blue' } }, { :taggings => { :comment => 'first' } }, { :posts => { :title => ['misc post by bob', 'misc post by mary'] } }], + [{ "taggable_type" => "Post" }], + [] + ] + actual = Author.reflect_on_association(:misc_post_first_blue_tags_2).conditions + assert_equal expected, actual + end + + def test_nested? + assert !Author.reflect_on_association(:comments).nested? + assert Author.reflect_on_association(:tags).nested? + + # Only goes :through once, but the through_reflection is a has_and_belongs_to_many, so this is + # a nested through association + assert Category.reflect_on_association(:post_comments).nested? + end + def test_association_primary_key - assert_equal "id", Author.reflect_on_association(:posts).association_primary_key.to_s + # Normal association + assert_equal "id", Author.reflect_on_association(:posts).association_primary_key.to_s assert_equal "name", Author.reflect_on_association(:essay).association_primary_key.to_s - assert_equal "id", Tagging.reflect_on_association(:taggable).association_primary_key.to_s + assert_equal "id", Tagging.reflect_on_association(:taggable).association_primary_key.to_s + + # Through association (uses the :primary_key option from the source reflection) + assert_equal "nick", Author.reflect_on_association(:subscribers).association_primary_key.to_s + assert_equal "name", Author.reflect_on_association(:essay_category).association_primary_key.to_s + assert_equal "custom_primary_key", Author.reflect_on_association(:tags_with_primary_key).association_primary_key.to_s # nested end def test_active_record_primary_key diff --git a/activerecord/test/cases/relation_scoping_test.rb b/activerecord/test/cases/relation_scoping_test.rb index cda2850b02..7369aaea1d 100644 --- a/activerecord/test/cases/relation_scoping_test.rb +++ b/activerecord/test/cases/relation_scoping_test.rb @@ -335,7 +335,7 @@ class DefaultScopingTest < ActiveRecord::TestCase end records = klass.all - assert_equal 1, records.length + assert_equal 3, records.length assert_equal 2, records.first.author_id end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 00b7c26080..4c5c871251 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -165,7 +165,7 @@ class RelationTest < ActiveRecord::TestCase def test_finding_with_complex_order tags = Tag.includes(:taggings).order("REPLACE('abc', taggings.taggable_type, taggings.taggable_type)").to_a - assert_equal 2, tags.length + assert_equal 3, tags.length end def test_finding_with_order_limit_and_offset @@ -281,27 +281,27 @@ class RelationTest < ActiveRecord::TestCase def test_find_with_preloaded_associations assert_queries(2) do - posts = Post.preload(:comments) + posts = Post.preload(:comments).order('posts.id') assert posts.first.comments.first end assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) do - posts = Post.preload(:comments).to_a + posts = Post.preload(:comments).order('posts.id') assert posts.first.comments.first end assert_queries(2) do - posts = Post.preload(:author) + posts = Post.preload(:author).order('posts.id') assert posts.first.author end assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) do - posts = Post.preload(:author).to_a + posts = Post.preload(:author).order('posts.id') assert posts.first.author end assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 3) do - posts = Post.preload(:author, :comments).to_a + posts = Post.preload(:author, :comments).order('posts.id') assert posts.first.author assert posts.first.comments.first end @@ -309,22 +309,22 @@ class RelationTest < ActiveRecord::TestCase def test_find_with_included_associations assert_queries(2) do - posts = Post.includes(:comments) + posts = Post.includes(:comments).order('posts.id') assert posts.first.comments.first end assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) do - posts = Post.scoped.includes(:comments) + posts = Post.scoped.includes(:comments).order('posts.id') assert posts.first.comments.first end assert_queries(2) do - posts = Post.includes(:author) + posts = Post.includes(:author).order('posts.id') assert posts.first.author end assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 3) do - posts = Post.includes(:author, :comments).to_a + posts = Post.includes(:author, :comments).order('posts.id') assert posts.first.author assert posts.first.comments.first end @@ -538,7 +538,7 @@ class RelationTest < ActiveRecord::TestCase def test_last authors = Author.scoped - assert_equal authors(:mary), authors.last + assert_equal authors(:bob), authors.last end def test_destroy_all @@ -618,22 +618,22 @@ class RelationTest < ActiveRecord::TestCase def test_count posts = Post.scoped - assert_equal 7, posts.count - assert_equal 7, posts.count(:all) - assert_equal 7, posts.count(:id) + assert_equal 11, posts.count + assert_equal 11, posts.count(:all) + assert_equal 11, posts.count(:id) assert_equal 1, posts.where('comments_count > 1').count - assert_equal 5, posts.where(:comments_count => 0).count + assert_equal 9, posts.where(:comments_count => 0).count end def test_count_with_distinct posts = Post.scoped assert_equal 3, posts.count(:comments_count, :distinct => true) - assert_equal 7, posts.count(:comments_count, :distinct => false) + assert_equal 11, posts.count(:comments_count, :distinct => false) assert_equal 3, posts.select(:comments_count).count(:distinct => true) - assert_equal 7, posts.select(:comments_count).count(:distinct => false) + assert_equal 11, posts.select(:comments_count).count(:distinct => false) end def test_count_explicit_columns @@ -643,7 +643,7 @@ class RelationTest < ActiveRecord::TestCase assert_equal [0], posts.select('comments_count').where('id is not null').group('id').order('id').count.values.uniq assert_equal 0, posts.where('id is not null').select('comments_count').count - assert_equal 7, posts.select('comments_count').count('id') + assert_equal 11, posts.select('comments_count').count('id') assert_equal 0, posts.select('comments_count').count assert_equal 0, posts.count(:comments_count) assert_equal 0, posts.count('comments_count') @@ -658,12 +658,12 @@ class RelationTest < ActiveRecord::TestCase def test_size posts = Post.scoped - assert_queries(1) { assert_equal 7, posts.size } + assert_queries(1) { assert_equal 11, posts.size } assert ! posts.loaded? best_posts = posts.where(:comments_count => 0) best_posts.to_a # force load - assert_no_queries { assert_equal 5, best_posts.size } + assert_no_queries { assert_equal 9, best_posts.size } end def test_count_complex_chained_relations @@ -797,6 +797,10 @@ class RelationTest < ActiveRecord::TestCase assert_equal Post.all, all_posts.all end + def test_extensions_with_except + assert_equal 2, Topic.named_extension.order(:author_name).except(:order).two + end + def test_only relation = Post.where(:author_id => 1).order('id ASC').limit(1) assert_equal [posts(:welcome)], relation.all @@ -808,6 +812,10 @@ class RelationTest < ActiveRecord::TestCase assert_equal Post.limit(1).all.first, all_posts.first end + def test_extensions_with_only + assert_equal 2, Topic.named_extension.order(:author_name).only(:order).two + end + def test_anonymous_extension relation = Post.where(:author_id => 1).order('id ASC').extending do def author diff --git a/activerecord/test/cases/timestamp_test.rb b/activerecord/test/cases/timestamp_test.rb index 1c21f0f3b6..08bbd14d38 100644 --- a/activerecord/test/cases/timestamp_test.rb +++ b/activerecord/test/cases/timestamp_test.rb @@ -113,7 +113,8 @@ class TimestampTest < ActiveRecord::TestCase pet = Pet.first owner = pet.owner - owner.update_attribute(:happy_at, 3.days.ago) + owner.happy_at = 3.days.ago + owner.save previously_owner_updated_at = owner.updated_at pet.name = "I'm a parrot" @@ -131,8 +132,9 @@ class TimestampTest < ActiveRecord::TestCase toy = Toy.first pet = toy.pet owner = pet.owner + time = 3.days.ago - owner.update_attribute(:updated_at, (time = 3.days.ago)) + owner.update_column(:updated_at, time) toy.touch owner.reload diff --git a/activerecord/test/fixtures/authors.yml b/activerecord/test/fixtures/authors.yml index de2ec7d38b..832236a486 100644 --- a/activerecord/test/fixtures/authors.yml +++ b/activerecord/test/fixtures/authors.yml @@ -3,7 +3,13 @@ david: name: David author_address_id: 1 author_address_extra_id: 2 + organization_id: No Such Agency + owned_essay_id: A Modest Proposal mary: id: 2 name: Mary + +bob: + id: 3 + name: Bob diff --git a/activerecord/test/fixtures/books.yml b/activerecord/test/fixtures/books.yml index 473663ff5b..fb48645456 100644 --- a/activerecord/test/fixtures/books.yml +++ b/activerecord/test/fixtures/books.yml @@ -1,7 +1,9 @@ awdr: + author_id: 1 id: 1 name: "Agile Web Development with Rails" rfr: + author_id: 1 id: 2 name: "Ruby for Rails" diff --git a/activerecord/test/fixtures/categories.yml b/activerecord/test/fixtures/categories.yml index b0770a093d..3e75e733a6 100644 --- a/activerecord/test/fixtures/categories.yml +++ b/activerecord/test/fixtures/categories.yml @@ -12,3 +12,8 @@ sti_test: id: 3 name: Special category type: SpecialCategory + +cooking: + id: 4 + name: Cooking + type: Category diff --git a/activerecord/test/fixtures/categories_posts.yml b/activerecord/test/fixtures/categories_posts.yml index 9b67ab4fa4..c6f0d885f5 100644 --- a/activerecord/test/fixtures/categories_posts.yml +++ b/activerecord/test/fixtures/categories_posts.yml @@ -21,3 +21,11 @@ sti_test_sti_habtm: general_hello: category_id: 1 post_id: 4 + +general_misc_by_bob: + category_id: 1 + post_id: 8 + +cooking_misc_by_bob: + category_id: 4 + post_id: 8 diff --git a/activerecord/test/fixtures/categorizations.yml b/activerecord/test/fixtures/categorizations.yml index c5b6fc9a51..62e5bd111a 100644 --- a/activerecord/test/fixtures/categorizations.yml +++ b/activerecord/test/fixtures/categorizations.yml @@ -15,3 +15,9 @@ mary_thinking_general: author_id: 2 post_id: 2 category_id: 1 + +bob_misc_by_bob_technology: + id: 4 + author_id: 3 + post_id: 8 + category_id: 2 diff --git a/activerecord/test/fixtures/clubs.yml b/activerecord/test/fixtures/clubs.yml index 1986d28229..82e439e8e5 100644 --- a/activerecord/test/fixtures/clubs.yml +++ b/activerecord/test/fixtures/clubs.yml @@ -1,6 +1,8 @@ boring_club: name: Banana appreciation society + category_id: 1 moustache_club: name: Moustache and Eyebrow Fancier Club crazy_club: - name: Skull and bones
\ No newline at end of file + name: Skull and bones + category_id: 2 diff --git a/activerecord/test/fixtures/essays.yml b/activerecord/test/fixtures/essays.yml new file mode 100644 index 0000000000..9d15d82359 --- /dev/null +++ b/activerecord/test/fixtures/essays.yml @@ -0,0 +1,6 @@ +david_modest_proposal: + name: A Modest Proposal + writer_type: Author + writer_id: David + category_id: General + author_id: David diff --git a/activerecord/test/fixtures/member_details.yml b/activerecord/test/fixtures/member_details.yml new file mode 100644 index 0000000000..e1fe695a9b --- /dev/null +++ b/activerecord/test/fixtures/member_details.yml @@ -0,0 +1,8 @@ +groucho: + id: 1 + member_id: 1 + organization: nsa +some_other_guy: + id: 2 + member_id: 2 + organization: nsa diff --git a/activerecord/test/fixtures/members.yml b/activerecord/test/fixtures/members.yml index 824840b7e5..f3bbf0dac6 100644 --- a/activerecord/test/fixtures/members.yml +++ b/activerecord/test/fixtures/members.yml @@ -6,3 +6,6 @@ some_other_guy: id: 2 name: Englebert Humperdink member_type_id: 2 +blarpy_winkup: + id: 3 + name: Blarpy Winkup diff --git a/activerecord/test/fixtures/memberships.yml b/activerecord/test/fixtures/memberships.yml index eed8b22af8..60eb641054 100644 --- a/activerecord/test/fixtures/memberships.yml +++ b/activerecord/test/fixtures/memberships.yml @@ -18,3 +18,10 @@ other_guys_membership: member_id: 2 favourite: false type: CurrentMembership + +blarpy_winkup_crazy_club: + joined_on: <%= 4.weeks.ago.to_s(:db) %> + club: crazy_club + member_id: 3 + favourite: false + type: CurrentMembership diff --git a/activerecord/test/fixtures/owners.yml b/activerecord/test/fixtures/owners.yml index d5493a84b7..2d21ce433c 100644 --- a/activerecord/test/fixtures/owners.yml +++ b/activerecord/test/fixtures/owners.yml @@ -1,6 +1,7 @@ blackbeard: owner_id: 1 name: blackbeard + essay_id: A Modest Proposal ashley: owner_id: 2 diff --git a/activerecord/test/fixtures/posts.yml b/activerecord/test/fixtures/posts.yml index 07069a064f..7298096025 100644 --- a/activerecord/test/fixtures/posts.yml +++ b/activerecord/test/fixtures/posts.yml @@ -52,3 +52,31 @@ eager_other: title: eager loading with OR'd conditions body: hello type: Post + +misc_by_bob: + id: 8 + author_id: 3 + title: misc post by bob + body: hello + type: Post + +misc_by_mary: + id: 9 + author_id: 2 + title: misc post by mary + body: hello + type: Post + +other_by_bob: + id: 10 + author_id: 3 + title: other post by bob + body: hello + type: Post + +other_by_mary: + id: 11 + author_id: 2 + title: other post by mary + body: hello + type: Post diff --git a/activerecord/test/fixtures/ratings.yml b/activerecord/test/fixtures/ratings.yml new file mode 100644 index 0000000000..34e208efa3 --- /dev/null +++ b/activerecord/test/fixtures/ratings.yml @@ -0,0 +1,14 @@ +normal_comment_rating: + id: 1 + comment_id: 8 + value: 1 + +special_comment_rating: + id: 2 + comment_id: 6 + value: 1 + +sub_special_comment_rating: + id: 3 + comment_id: 12 + value: 1 diff --git a/activerecord/test/fixtures/taggings.yml b/activerecord/test/fixtures/taggings.yml index 3db6a4c079..d339c12b25 100644 --- a/activerecord/test/fixtures/taggings.yml +++ b/activerecord/test/fixtures/taggings.yml @@ -26,3 +26,53 @@ godfather: orphaned: id: 5 tag_id: 1 + +misc_post_by_bob: + id: 6 + tag_id: 2 + taggable_id: 8 + taggable_type: Post + +misc_post_by_mary: + id: 7 + tag_id: 2 + taggable_id: 9 + taggable_type: Post + +misc_by_bob_blue_first: + id: 8 + tag_id: 3 + taggable_id: 8 + taggable_type: Post + comment: first + +misc_by_bob_blue_second: + id: 9 + tag_id: 3 + taggable_id: 8 + taggable_type: Post + comment: second + +other_by_bob_blue: + id: 10 + tag_id: 3 + taggable_id: 10 + taggable_type: Post + comment: first + +other_by_mary_blue: + id: 11 + tag_id: 3 + taggable_id: 11 + taggable_type: Post + comment: first + +special_comment_rating: + id: 12 + taggable_id: 2 + taggable_type: Rating + +normal_comment_rating: + id: 13 + taggable_id: 1 + taggable_type: Rating diff --git a/activerecord/test/fixtures/tags.yml b/activerecord/test/fixtures/tags.yml index 7610fd38b9..d4b7c9a4d5 100644 --- a/activerecord/test/fixtures/tags.yml +++ b/activerecord/test/fixtures/tags.yml @@ -4,4 +4,8 @@ general: misc: id: 2 - name: Misc
\ No newline at end of file + name: Misc + +blue: + id: 3 + name: Blue diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index 83a6f5d926..e0cbc44265 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -94,16 +94,50 @@ class Author < ActiveRecord::Base has_many :author_favorites has_many :favorite_authors, :through => :author_favorites, :order => 'name' - has_many :tagging, :through => :posts # through polymorphic has_one - has_many :taggings, :through => :posts, :source => :taggings # through polymorphic has_many - has_many :tags, :through => :posts # through has_many :through + has_many :tagging, :through => :posts + has_many :taggings, :through => :posts + has_many :tags, :through => :posts + has_many :similar_posts, :through => :tags, :source => :tagged_posts, :uniq => true + has_many :distinct_tags, :through => :posts, :source => :tags, :select => "DISTINCT tags.*", :order => "tags.name" has_many :post_categories, :through => :posts, :source => :categories + has_many :tagging_tags, :through => :taggings, :source => :tag + has_many :tags_with_primary_key, :through => :posts + + has_many :books + has_many :subscriptions, :through => :books + has_many :subscribers, :through => :subscriptions, :order => "subscribers.nick" # through has_many :through (on through reflection) + has_many :distinct_subscribers, :through => :subscriptions, :source => :subscriber, :select => "DISTINCT subscribers.*", :order => "subscribers.nick" has_one :essay, :primary_key => :name, :as => :writer + has_one :essay_category, :through => :essay, :source => :category + has_one :essay_owner, :through => :essay, :source => :owner + + has_one :essay_2, :primary_key => :name, :class_name => 'Essay', :foreign_key => :author_id + has_one :essay_category_2, :through => :essay_2, :source => :category + + has_many :essays, :primary_key => :name, :as => :writer + has_many :essay_categories, :through => :essays, :source => :category + has_many :essay_owners, :through => :essays, :source => :owner + + has_many :essays_2, :primary_key => :name, :class_name => 'Essay', :foreign_key => :author_id + has_many :essay_categories_2, :through => :essays_2, :source => :category + + belongs_to :owned_essay, :primary_key => :name, :class_name => 'Essay' + has_one :owned_essay_category, :through => :owned_essay, :source => :category - belongs_to :author_address, :dependent => :destroy + belongs_to :author_address, :dependent => :destroy belongs_to :author_address_extra, :dependent => :delete, :class_name => "AuthorAddress" + has_many :post_categories, :through => :posts, :source => :categories + has_many :category_post_comments, :through => :categories, :source => :post_comments + + has_many :misc_posts, :class_name => 'Post', + :conditions => { :posts => { :title => ['misc post by bob', 'misc post by mary'] } } + has_many :misc_post_first_blue_tags, :through => :misc_posts, :source => :first_blue_tags + + has_many :misc_post_first_blue_tags_2, :through => :posts, :source => :first_blue_tags_2, + :conditions => { :posts => { :title => ['misc post by bob', 'misc post by mary'] } } + scope :relation_include_posts, includes(:posts) scope :relation_include_tags, includes(:tags) diff --git a/activerecord/test/models/book.rb b/activerecord/test/models/book.rb index 1e030b4f59..d27d0af77c 100644 --- a/activerecord/test/models/book.rb +++ b/activerecord/test/models/book.rb @@ -1,4 +1,6 @@ class Book < ActiveRecord::Base + has_many :authors + has_many :citations, :foreign_key => 'book1_id' has_many :references, :through => :citations, :source => :reference_of, :uniq => true diff --git a/activerecord/test/models/categorization.rb b/activerecord/test/models/categorization.rb index 45f50e4af3..09489b8ea4 100644 --- a/activerecord/test/models/categorization.rb +++ b/activerecord/test/models/categorization.rb @@ -4,6 +4,8 @@ class Categorization < ActiveRecord::Base belongs_to :named_category, :class_name => 'Category', :foreign_key => :named_category_name, :primary_key => :name belongs_to :author + has_many :post_taggings, :through => :author, :source => :taggings + belongs_to :author_using_custom_pk, :class_name => 'Author', :foreign_key => :author_id, :primary_key => :author_address_extra_id has_many :authors_using_custom_pk, :class_name => 'Author', :foreign_key => :id, :primary_key => :category_id end diff --git a/activerecord/test/models/category.rb b/activerecord/test/models/category.rb index 8f37433ec6..02b85fd38a 100644 --- a/activerecord/test/models/category.rb +++ b/activerecord/test/models/category.rb @@ -22,6 +22,8 @@ class Category < ActiveRecord::Base end has_many :categorizations + has_many :post_comments, :through => :posts, :source => :comments + has_many :authors, :through => :categorizations has_many :authors_with_select, :through => :categorizations, :source => :author, :select => 'authors.*, categorizations.post_id' diff --git a/activerecord/test/models/club.rb b/activerecord/test/models/club.rb index c432a6ace8..24a65b0f2f 100644 --- a/activerecord/test/models/club.rb +++ b/activerecord/test/models/club.rb @@ -5,6 +5,7 @@ class Club < ActiveRecord::Base has_many :current_memberships has_one :sponsor has_one :sponsored_member, :through => :sponsor, :source => :sponsorable, :source_type => "Member" + belongs_to :category private diff --git a/activerecord/test/models/comment.rb b/activerecord/test/models/comment.rb index ff533717cc..2a4c37089a 100644 --- a/activerecord/test/models/comment.rb +++ b/activerecord/test/models/comment.rb @@ -8,6 +8,7 @@ class Comment < ActiveRecord::Base :conditions => { "posts.author_id" => 1 } belongs_to :post, :counter_cache => true + has_many :ratings def self.what_are_you 'a comment...' diff --git a/activerecord/test/models/essay.rb b/activerecord/test/models/essay.rb index 6c28f5e49b..ec4b982b5b 100644 --- a/activerecord/test/models/essay.rb +++ b/activerecord/test/models/essay.rb @@ -1,3 +1,5 @@ class Essay < ActiveRecord::Base belongs_to :writer, :primary_key => :name, :polymorphic => true + belongs_to :category, :primary_key => :name + has_one :owner, :primary_key => :name end diff --git a/activerecord/test/models/job.rb b/activerecord/test/models/job.rb index 3333a02e27..f7b0e787b1 100644 --- a/activerecord/test/models/job.rb +++ b/activerecord/test/models/job.rb @@ -2,4 +2,6 @@ class Job < ActiveRecord::Base has_many :references has_many :people, :through => :references belongs_to :ideal_reference, :class_name => 'Reference' + + has_many :agents, :through => :people end diff --git a/activerecord/test/models/member.rb b/activerecord/test/models/member.rb index e6e78f9e45..991e0e051f 100644 --- a/activerecord/test/models/member.rb +++ b/activerecord/test/models/member.rb @@ -11,6 +11,17 @@ class Member < ActiveRecord::Base has_one :organization, :through => :member_detail belongs_to :member_type + has_many :nested_member_types, :through => :member_detail, :source => :member_type + has_one :nested_member_type, :through => :member_detail, :source => :member_type + + has_many :nested_sponsors, :through => :sponsor_club, :source => :sponsor + has_one :nested_sponsor, :through => :sponsor_club, :source => :sponsor + + has_many :organization_member_details, :through => :member_detail + has_many :organization_member_details_2, :through => :organization, :source => :member_details + + has_one :club_category, :through => :club, :source => :category + has_many :current_memberships has_one :club_through_many, :through => :current_memberships, :source => :club diff --git a/activerecord/test/models/member_detail.rb b/activerecord/test/models/member_detail.rb index 94f59e5794..fe619f8732 100644 --- a/activerecord/test/models/member_detail.rb +++ b/activerecord/test/models/member_detail.rb @@ -2,4 +2,6 @@ class MemberDetail < ActiveRecord::Base belongs_to :member belongs_to :organization has_one :member_type, :through => :member + + has_many :organization_member_details, :through => :organization, :source => :member_details end diff --git a/activerecord/test/models/organization.rb b/activerecord/test/models/organization.rb index 1da342a0bd..4a4111833f 100644 --- a/activerecord/test/models/organization.rb +++ b/activerecord/test/models/organization.rb @@ -2,5 +2,11 @@ class Organization < ActiveRecord::Base has_many :member_details has_many :members, :through => :member_details + has_many :authors, :primary_key => :name + has_many :author_essay_categories, :through => :authors, :source => :essay_categories + + has_one :author, :primary_key => :name + has_one :author_owned_essay_category, :through => :author, :source => :owned_essay_category + scope :clubs, { :from => 'clubs' } -end
\ No newline at end of file +end diff --git a/activerecord/test/models/person.rb b/activerecord/test/models/person.rb index cc3a4f5f9d..ad59d12672 100644 --- a/activerecord/test/models/person.rb +++ b/activerecord/test/models/person.rb @@ -21,6 +21,9 @@ class Person < ActiveRecord::Base has_many :agents_of_agents, :through => :agents, :source => :agents belongs_to :number1_fan, :class_name => 'Person' + has_many :agents_posts, :through => :agents, :source => :posts + has_many :agents_posts_authors, :through => :agents_posts, :source => :author + scope :males, :conditions => { :gender => 'M' } scope :females, :conditions => { :gender => 'F' } end diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index a342aaf60b..82894a3d57 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -49,6 +49,9 @@ class Post < ActiveRecord::Base has_many :special_comments has_many :nonexistant_comments, :class_name => 'Comment', :conditions => 'comments.id < 0' + has_many :special_comments_ratings, :through => :special_comments, :source => :ratings + has_many :special_comments_ratings_taggings, :through => :special_comments_ratings, :source => :taggings + has_and_belongs_to_many :categories has_and_belongs_to_many :special_categories, :join_table => "categories_posts", :association_foreign_key => 'category_id' @@ -70,11 +73,17 @@ class Post < ActiveRecord::Base has_many :tags_with_destroy, :through => :taggings, :source => :tag, :dependent => :destroy has_many :tags_with_nullify, :through => :taggings, :source => :tag, :dependent => :nullify - has_many :misc_tags, :through => :taggings, :source => :tag, :conditions => "tags.name = 'Misc'" + has_many :misc_tags, :through => :taggings, :source => :tag, :conditions => { :tags => { :name => 'Misc' } } has_many :funky_tags, :through => :taggings, :source => :tag has_many :super_tags, :through => :taggings + has_many :tags_with_primary_key, :through => :taggings, :source => :tag_with_primary_key has_one :tagging, :as => :taggable + has_many :first_taggings, :as => :taggable, :class_name => 'Tagging', :conditions => { :taggings => { :comment => 'first' } } + has_many :first_blue_tags, :through => :first_taggings, :source => :tag, :conditions => { :tags => { :name => 'Blue' } } + + has_many :first_blue_tags_2, :through => :taggings, :source => :blue_tag, :conditions => { :taggings => { :comment => 'first' } } + has_many :invalid_taggings, :as => :taggable, :class_name => "Tagging", :conditions => 'taggings.id < 0' has_many :invalid_tags, :through => :invalid_taggings, :source => :tag diff --git a/activerecord/test/models/rating.rb b/activerecord/test/models/rating.rb new file mode 100644 index 0000000000..25a52c4ad7 --- /dev/null +++ b/activerecord/test/models/rating.rb @@ -0,0 +1,4 @@ +class Rating < ActiveRecord::Base + belongs_to :comment + has_many :taggings, :as => :taggable +end diff --git a/activerecord/test/models/reference.rb b/activerecord/test/models/reference.rb index 06c4f79ef3..e33a0f2acc 100644 --- a/activerecord/test/models/reference.rb +++ b/activerecord/test/models/reference.rb @@ -2,6 +2,8 @@ class Reference < ActiveRecord::Base belongs_to :person belongs_to :job + has_many :agents_posts_authors, :through => :person + class << self attr_accessor :make_comments end diff --git a/activerecord/test/models/tagging.rb b/activerecord/test/models/tagging.rb index 231d2b5890..ef323df158 100644 --- a/activerecord/test/models/tagging.rb +++ b/activerecord/test/models/tagging.rb @@ -6,6 +6,8 @@ class Tagging < ActiveRecord::Base belongs_to :tag, :include => :tagging belongs_to :super_tag, :class_name => 'Tag', :foreign_key => 'super_tag_id' belongs_to :invalid_tag, :class_name => 'Tag', :foreign_key => 'tag_id' + belongs_to :blue_tag, :class_name => 'Tag', :foreign_key => :tag_id, :conditions => { :tags => { :name => 'Blue' } } + belongs_to :tag_with_primary_key, :class_name => 'Tag', :foreign_key => :tag_id, :primary_key => :custom_primary_key belongs_to :interpolated_tag, :class_name => 'Tag', :foreign_key => :tag_id, :conditions => proc { "1 = #{1}" } belongs_to :taggable, :polymorphic => true, :counter_cache => true has_many :things, :through => :taggable diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 0b3865fc78..362475de36 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -49,6 +49,8 @@ ActiveRecord::Schema.define do t.string :name, :null => false t.integer :author_address_id t.integer :author_address_extra_id + t.string :organization_id + t.string :owned_essay_id end create_table :author_addresses, :force => true do |t| @@ -75,6 +77,7 @@ ActiveRecord::Schema.define do end create_table :books, :force => true do |t| + t.integer :author_id t.column :name, :string end @@ -123,6 +126,7 @@ ActiveRecord::Schema.define do create_table :clubs, :force => true do |t| t.string :name + t.integer :category_id end create_table :collections, :force => true do |t| @@ -216,6 +220,8 @@ ActiveRecord::Schema.define do t.string :name t.string :writer_id t.string :writer_type + t.string :category_id + t.string :author_id end create_table :events, :force => true do |t| @@ -393,6 +399,7 @@ ActiveRecord::Schema.define do t.string :name t.column :updated_at, :datetime t.column :happy_at, :datetime + t.string :essay_id end create_table :paint_colors, :force => true do |t| @@ -482,6 +489,11 @@ ActiveRecord::Schema.define do t.string :type end + create_table :ratings, :force => true do |t| + t.integer :comment_id + t.integer :value + end + create_table :readers, :force => true do |t| t.integer :post_id, :null => false t.integer :person_id, :null => false @@ -553,6 +565,7 @@ ActiveRecord::Schema.define do t.column :super_tag_id, :integer t.column :taggable_type, :string t.column :taggable_id, :integer + t.string :comment end create_table :tasks, :force => true do |t| @@ -677,6 +690,9 @@ ActiveRecord::Schema.define do t.integer :molecule_id t.string :name end + create_table :weirds, :force => true do |t| + t.string 'a$b' + end except 'SQLite' do # fk_test_has_fk should be before fk_test_has_pk diff --git a/activesupport/lib/active_support/json/backends/yaml.rb b/activesupport/lib/active_support/json/backends/yaml.rb index 077eda548a..e25e29d36b 100644 --- a/activesupport/lib/active_support/json/backends/yaml.rb +++ b/activesupport/lib/active_support/json/backends/yaml.rb @@ -29,7 +29,7 @@ module ActiveSupport def convert_json_to_yaml(json) #:nodoc: require 'strscan' unless defined? ::StringScanner scanner, quoting, marks, pos, times = ::StringScanner.new(json), false, [], nil, [] - while scanner.scan_until(/(\\['"]|['":,\\]|\\.)/) + while scanner.scan_until(/(\\['"]|['":,\\]|\\.|[\]])/) case char = scanner[1] when '"', "'" if !quoting @@ -43,7 +43,7 @@ module ActiveSupport end quoting = false end - when ":","," + when ":",",", "]" marks << scanner.pos - 1 unless quoting when "\\" scanner.skip(/\\/) @@ -70,9 +70,11 @@ module ActiveSupport left_pos.each_with_index do |left, i| scanner.pos = left.succ chunk = scanner.peek(right_pos[i] - scanner.pos + 1) - # overwrite the quotes found around the dates with spaces - while times.size > 0 && times[0] <= right_pos[i] - chunk.insert(times.shift - scanner.pos - 1, '! ') + if ActiveSupport.parse_json_times + # overwrite the quotes found around the dates with spaces + while times.size > 0 && times[0] <= right_pos[i] + chunk.insert(times.shift - scanner.pos - 1, '! ') + end end chunk.gsub!(/\\([\\\/]|u[[:xdigit:]]{4})/) do ustr = $1 diff --git a/activesupport/test/json/decoding_test.rb b/activesupport/test/json/decoding_test.rb index 24d9f88c09..88cf97de7e 100644 --- a/activesupport/test/json/decoding_test.rb +++ b/activesupport/test/json/decoding_test.rb @@ -17,6 +17,8 @@ class TestJSONDecoding < ActiveSupport::TestCase %({"matzue": "松江", "asakusa": "浅草"}) => {"matzue" => "松江", "asakusa" => "浅草"}, %({"a": "2007-01-01"}) => {'a' => Date.new(2007, 1, 1)}, %({"a": "2007-01-01 01:12:34 Z"}) => {'a' => Time.utc(2007, 1, 1, 1, 12, 34)}, + %(["2007-01-01 01:12:34 Z"]) => [Time.utc(2007, 1, 1, 1, 12, 34)], + %(["2007-01-01 01:12:34 Z", "2007-01-01 01:12:35 Z"]) => [Time.utc(2007, 1, 1, 1, 12, 34), Time.utc(2007, 1, 1, 1, 12, 35)], # no time zone %({"a": "2007-01-01 01:12:34"}) => {'a' => "2007-01-01 01:12:34"}, # invalid date @@ -72,13 +74,11 @@ class TestJSONDecoding < ActiveSupport::TestCase end end end - end - if backends.include?("JSONGem") - test "json decodes time json with time parsing disabled" do + test "json decodes time json with time parsing disabled with the #{backend} backend" do ActiveSupport.parse_json_times = false expected = {"a" => "2007-01-01 01:12:34 Z"} - ActiveSupport::JSON.with_backend "JSONGem" do + ActiveSupport::JSON.with_backend backend do assert_equal expected, ActiveSupport::JSON.decode(%({"a": "2007-01-01 01:12:34 Z"})) end end diff --git a/railties/guides/source/3_0_release_notes.textile b/railties/guides/source/3_0_release_notes.textile index 001f458fd9..f75b245ed8 100644 --- a/railties/guides/source/3_0_release_notes.textile +++ b/railties/guides/source/3_0_release_notes.textile @@ -59,12 +59,12 @@ The +config.gem+ method is gone and has been replaced by using +bundler+ and a + h4. Upgrade Process -To help with the upgrade process, a plugin named "Rails Upgrade":http://github.com/rails/rails_upgrade has been created to automate part of it. +To help with the upgrade process, a plugin named "Rails Upgrade":http://github.com/jm/rails_upgrade has been created to automate part of it. Simply install the plugin, then run +rake rails:upgrade:check+ to check your app for pieces that need to be updated (with links to information on how to update them). It also offers a task to generate a +Gemfile+ based on your current +config.gem+ calls and a task to generate a new routes file from your current one. To get the plugin, simply run the following: <shell> -$ ruby script/plugin install git://github.com/rails/rails_upgrade.git +$ ruby script/plugin install git://github.com/jm/rails_upgrade.git </shell> You can see an example of how that works at "Rails Upgrade is now an Official Plugin":http://omgbloglol.com/post/364624593/rails-upgrade-is-now-an-official-plugin diff --git a/railties/guides/source/active_record_querying.textile b/railties/guides/source/active_record_querying.textile index ed3968e226..2c5d9e67e3 100644 --- a/railties/guides/source/active_record_querying.textile +++ b/railties/guides/source/active_record_querying.textile @@ -76,7 +76,7 @@ Primary operation of <tt>Model.find(options)</tt> can be summarized as: h4. Retrieving a Single Object -Active Record lets you retrieve a single object using three different ways. +Active Record lets you retrieve a single object using five different ways. h5. Using a Primary Key @@ -130,6 +130,40 @@ SELECT * FROM clients ORDER BY clients.id DESC LIMIT 1 <tt>Model.last</tt> returns +nil+ if no matching record is found. No exception will be raised. +h5. +first!+ + +<tt>Model.first!</tt> finds the first record. For example: + +<ruby> +client = Client.first! +=> #<Client id: 1, first_name: "Lifo"> +</ruby> + +SQL equivalent of the above is: + +<sql> +SELECT * FROM clients LIMIT 1 +</sql> + +<tt>Model.first!</tt> raises +RecordNotFound+ if no matching record is found. + +h5. +last!+ + +<tt>Model.last!</tt> finds the last record. For example: + +<ruby> +client = Client.last! +=> #<Client id: 221, first_name: "Russel"> +</ruby> + +SQL equivalent of the above is: + +<sql> +SELECT * FROM clients ORDER BY clients.id DESC LIMIT 1 +</sql> + +<tt>Model.last!</tt> raises +RecordNotFound+ if no matching record is found. + h4. Retrieving Multiple Objects h5. Using Multiple Primary Keys @@ -747,6 +781,22 @@ h4. Specifying Conditions on Eager Loaded Associations Even though Active Record lets you specify conditions on the eager loaded associations just like +joins+, the recommended way is to use "joins":#joining-tables instead. +However if you must do this, you may use +where+ as you would normally. + +<ruby> +Post.includes(:comments).where("comments.visible", true) +</ruby> + +This would generate a query which contains a +LEFT OUTER JOIN+ whereas the +joins+ method would generate one using the +INNER JOIN+ function instead. + +<ruby> + SELECT "posts"."id" AS t0_r0, ... "comments"."updated_at" AS t1_r5 FROM "posts" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" WHERE (comments.visible) +</ruby> + +If there was no +where+ condition, this would generate the normal set of two queries. + +If, in the case of this +includes+ query, there were no comments for any posts, all the posts would still be loaded. By using +joins+ (an INNER JOIN), the join conditions *must* match, otherwise no records will be returned. + h3. Scopes Scoping allows you to specify commonly-used ARel queries which can be referenced as method calls on the association objects or models. With these scopes, you can use every method previously covered such as +where+, +joins+ and +includes+. All scope methods will return an +ActiveRecord::Relation+ object which will allow for further methods (such as other scopes) to be called on it. diff --git a/railties/guides/source/active_record_validations_callbacks.textile b/railties/guides/source/active_record_validations_callbacks.textile index 58a184a48e..e5349d546c 100644 --- a/railties/guides/source/active_record_validations_callbacks.textile +++ b/railties/guides/source/active_record_validations_callbacks.textile @@ -461,11 +461,11 @@ The block receives the model, the attribute's name and the attribute's value. Yo h3. Common Validation Options -There are some common options that all the validation helpers can use. Here they are, except for the +:if+ and +:unless+ options, which are discussed later in "Conditional Validation":#conditional-validation. +These are common validation options: h4. +:allow_nil+ -The +:allow_nil+ option skips the validation when the value being validated is +nil+. Using +:allow_nil+ with +validates_presence_of+ allows for +nil+, but any other +blank?+ value will still be rejected. +The +:allow_nil+ option skips the validation when the value being validated is +nil+. <ruby> class Coffee < ActiveRecord::Base @@ -474,6 +474,8 @@ class Coffee < ActiveRecord::Base end </ruby> +TIP: +:allow_nil+ is ignored by the presence validator. + h4. +:allow_blank+ The +:allow_blank+ option is similar to the +:allow_nil+ option. This option will let validation pass if the attribute's value is +blank?+, like +nil+ or an empty string for example. @@ -487,6 +489,8 @@ Topic.create("title" => "").valid? # => true Topic.create("title" => nil).valid? # => true </ruby> +TIP: +:allow_blank+ is ignored by the presence validator. + h4. +:message+ As you've already seen, the +:message+ option lets you specify the message that will be added to the +errors+ collection when validation fails. When this option is not used, Active Record will use the respective default error message for each validation helper. @@ -742,7 +746,7 @@ Rails maintains an official plugin that provides helpers to display the error me h4. Installing as a plugin <shell> -$ rails plugin install git://github.com/rails/dynamic_form.git +$ rails plugin install git://github.com/joelmoss/dynamic_form.git </shell> h4 Installing as a Gem diff --git a/railties/guides/source/caching_with_rails.textile b/railties/guides/source/caching_with_rails.textile index 1b5ec40d16..297ba2d661 100644 --- a/railties/guides/source/caching_with_rails.textile +++ b/railties/guides/source/caching_with_rails.textile @@ -65,7 +65,7 @@ end If you want a more complicated expiration scheme, you can use cache sweepers to expire cached objects when things change. This is covered in the section on Sweepers. -Note: Page caching ignores all parameters. For example +/products?page=1+ will be written out to the filesystem as +products.html+ with no reference to the +page+ parameter. Thus, if someone requests +/products?page=2+ later, they will get the cached first page. Be careful when page caching GET parameters in the URL! +NOTE: Page caching ignores all parameters. For example +/products?page=1+ will be written out to the filesystem as +products.html+ with no reference to the +page+ parameter. Thus, if someone requests +/products?page=2+ later, they will get the cached first page. Be careful when page caching GET parameters in the URL! INFO: Page caching runs in an after filter. Thus, invalid requests won't generate spurious cache entries as long as you halt them. Typically, a redirection in some before filter that checks request preconditions does the job. diff --git a/railties/guides/source/contributing_to_ruby_on_rails.textile b/railties/guides/source/contributing_to_ruby_on_rails.textile index 4ba01a2a8f..1fcc4fd7e3 100644 --- a/railties/guides/source/contributing_to_ruby_on_rails.textile +++ b/railties/guides/source/contributing_to_ruby_on_rails.textile @@ -364,19 +364,6 @@ Please make sure the patch does not introduce whitespace errors: $ git apply --whitespace=error-all mynew_patch.diff </shell> -You can check your patches by applying your patch to an different dedicated branch: - -<shell> -$ git checkout -b testing_branch -$ git apply --check my_new_patch.diff -</shell> - -You can make sure your patches don't add any whitespace by applying it yourself using the --whitespace=error-all option. Make sure you are on your dedicated test branche and: - -<shell> -$ git apply --whitespace=error-all mynew_patch.diff -</shell> - h4. Create a Lighthouse Ticket @@ -384,7 +371,7 @@ Now create a ticket with your patch. Go to the "new ticket":http://rails.lightho h4. Get Some Feedback -Now you need to get other people to look at your patch, just as you've looked at other people's patches. You can use the rubyonrails-core mailing list or the #rails-contrib channel on IRC freenode for this. You might also try just talking to Rails developers that you know. +Now you need to get other people to look at your patch, just as you've looked at other people's patches. You can use the "rubyonrails-core mailing list":http://groups.google.com/group/rubyonrails-core/ or the #rails-contrib channel on IRC freenode for this. You might also try just talking to Rails developers that you know. h4. Iterate as Necessary diff --git a/railties/guides/source/debugging_rails_applications.textile b/railties/guides/source/debugging_rails_applications.textile index 67e17056e5..045b8823ca 100644 --- a/railties/guides/source/debugging_rails_applications.textile +++ b/railties/guides/source/debugging_rails_applications.textile @@ -686,7 +686,7 @@ There are some Rails plugins to help you to find errors and debug your applicati * "Query Trace":https://github.com/ntalbott/query_trace/tree/master: Adds query origin tracing to your logs. * "Query Stats":https://github.com/dan-manges/query_stats/tree/master: A Rails plugin to track database queries. * "Query Reviewer":http://code.google.com/p/query-reviewer/: This rails plugin not only runs "EXPLAIN" before each of your select queries in development, but provides a small DIV in the rendered output of each page with the summary of warnings for each query that it analyzed. -* "Exception Notifier":https://github.com/rails/exception_notification/tree/master: Provides a mailer object and a default set of templates for sending email notifications when errors occur in a Rails application. +* "Exception Notifier":https://github.com/smartinez87/exception_notification/tree/master: Provides a mailer object and a default set of templates for sending email notifications when errors occur in a Rails application. * "Exception Logger":https://github.com/defunkt/exception_logger/tree/master: Logs your Rails exceptions in the database and provides a funky web interface to manage them. h3. References diff --git a/railties/guides/source/form_helpers.textile b/railties/guides/source/form_helpers.textile index 0cfc005f80..1f21c27ae6 100644 --- a/railties/guides/source/form_helpers.textile +++ b/railties/guides/source/form_helpers.textile @@ -475,7 +475,7 @@ To leverage time zone support in Rails, you have to ask your users what time zon There is also +time_zone_options_for_select+ helper for a more manual (therefore more customizable) way of doing this. Read the API documentation to learn about the possible arguments for these two methods. -Rails _used_ to have a +country_select+ helper for choosing countries, but this has been extracted to the "country_select plugin":https://github.com/rails/country_select/tree/master. When using this, be aware that the exclusion or inclusion of certain names from the list can be somewhat controversial (and was the reason this functionality was extracted from Rails). +Rails _used_ to have a +country_select+ helper for choosing countries, but this has been extracted to the "country_select plugin":https://github.com/chrislerum/country_select. When using this, be aware that the exclusion or inclusion of certain names from the list can be somewhat controversial (and was the reason this functionality was extracted from Rails). h3. Using Date and Time Form Helpers diff --git a/railties/guides/source/getting_started.textile b/railties/guides/source/getting_started.textile index e94bdc97b0..0661549644 100644 --- a/railties/guides/source/getting_started.textile +++ b/railties/guides/source/getting_started.textile @@ -149,7 +149,7 @@ Usually run this as the root user: # gem install rails </shell> -TIP. If you're working on Windows, you should be aware that the vast majority of Rails development is done in Unix environments. While Ruby and Rails themselves install easily using for example "Ruby Installer":http://rubyinstaller.org/, the supporting ecosystem often assumes you are able to build C-based rubygems and work in a command window. If at all possible, we suggest that you install a Linux virtual machine and use that for Rails development, instead of using Windows. +TIP. If you're working on Windows, you can quickly install Ruby and Rails with "Rails Installer":http://railsinstaller.org. h4. Creating the Blog Application diff --git a/railties/guides/source/plugins.textile b/railties/guides/source/plugins.textile index daca50ee9e..2d9821e627 100644 --- a/railties/guides/source/plugins.textile +++ b/railties/guides/source/plugins.textile @@ -45,9 +45,9 @@ as a gem. This tutorial will begin to bridge that gap by demonstrating how to c "Enginex gem":http://www.github.com/josevalim/enginex. <shell> - gem install enginex - enginex --help - enginex yaffle +$ gem install enginex +$ enginex --help +$ enginex yaffle </shell> This command will create a new directory named "yaffle" within the current directory. @@ -401,7 +401,9 @@ h3. Publishing your Gem Gem plugins in progress can be easily be shared from any Git repository. To share the Yaffle gem with others, simply commit the code to a Git repository (like Github) and add a line to the Gemfile of the any application: -gem 'yaffle', :git => 'git://github.com/yaffle_watcher/yaffle.git' +<ruby> +gem 'yaffle', :git => 'git://github.com/yaffle_watcher/yaffle.git' +</ruby> After running +bundle install+, your gem functionality will be available to the application. @@ -450,8 +452,6 @@ Once your comments are good to go, navigate to your plugin directory and run: $ rake rdoc </shell> -!!!!!!!!!!!!!! Make sure these still make sense. Add any references that you see fit. !!!!!!!!!!!!! - h4. References * "Developing a RubyGem using Bundler":https://github.com/radar/guides/blob/master/gem-development.md @@ -462,6 +462,7 @@ h4. References h3. Changelog +* March 10, 2011: Minor formatting tweaks. * February 13, 2011: Get guide in synch with Rails 3.0.3. Remove information not compatible with Rails 3. Send reader elsewhere for information that is covered elsewhere. * April 4, 2010: Fixed document to validate XHTML 1.0 Strict. "Jaime Iniesta":http://jaimeiniesta.com diff --git a/railties/guides/source/testing.textile b/railties/guides/source/testing.textile index a75cedbdc1..4ebdb3edf6 100644 --- a/railties/guides/source/testing.textile +++ b/railties/guides/source/testing.textile @@ -500,6 +500,8 @@ If you're familiar with the HTTP protocol, you'll know that +get+ is a type of r All of request types are methods that you can use, however, you'll probably end up using the first two more often than the others. +NOTE: Functional tests do not verify whether the specified request type should be accepted by the action. Request types in this context exist to make your tests more descriptive. + h4. The Four Hashes of the Apocalypse After a request has been made by using one of the 5 methods (+get+, +post+, etc.) and processed, you will have 4 Hash objects ready for use: diff --git a/railties/lib/rails/generators/rails/plugin_new/plugin_new_generator.rb b/railties/lib/rails/generators/rails/plugin_new/plugin_new_generator.rb index 5b4687ce64..3cf8410d1e 100644 --- a/railties/lib/rails/generators/rails/plugin_new/plugin_new_generator.rb +++ b/railties/lib/rails/generators/rails/plugin_new/plugin_new_generator.rb @@ -62,7 +62,7 @@ task :default => :test end def generate_test_dummy(force = false) - opts = (options || {}).slice(:skip_active_record, :skip_javascript, :database, :javascript) + opts = (options || {}).slice(:skip_active_record, :skip_javascript, :database, :javascript, :quiet, :pretend, :force, :skip) opts[:force] = force invoke Rails::Generators::AppGenerator, @@ -145,6 +145,7 @@ task :default => :test def initialize(*args) raise Error, "Options should be given after the plugin name. For details run: rails plugin --help" if args[0].blank? + @dummy_path = nil super end diff --git a/railties/lib/rails/railtie/configuration.rb b/railties/lib/rails/railtie/configuration.rb index e4368866a1..2c7b5bc048 100644 --- a/railties/lib/rails/railtie/configuration.rb +++ b/railties/lib/rails/railtie/configuration.rb @@ -31,26 +31,34 @@ module Rails app_generators(&block) end + # First configurable block to run. Called before any initializers are run. def before_configuration(&block) ActiveSupport.on_load(:before_configuration, :yield => true, &block) end + # Third configurable block to run. Does not run if config.cache_classes + # set to false. def before_eager_load(&block) ActiveSupport.on_load(:before_eager_load, :yield => true, &block) end + # Second configurable block to run. Called before frameworks initialize. def before_initialize(&block) ActiveSupport.on_load(:before_initialize, :yield => true, &block) end + # Last configurable block to run. Called after frameworks initialize. def after_initialize(&block) ActiveSupport.on_load(:after_initialize, :yield => true, &block) end + # Array of callbacks defined by #to_prepare. def to_prepare_blocks @@to_prepare_blocks ||= [] end + # Defines generic callbacks to run before #after_initialize. Useful for + # Rails::Railtie subclasses. def to_prepare(&blk) to_prepare_blocks << blk if blk end diff --git a/railties/test/generators/plugin_new_generator_test.rb b/railties/test/generators/plugin_new_generator_test.rb index 2a8b337144..3c11c8dbaf 100644 --- a/railties/test/generators/plugin_new_generator_test.rb +++ b/railties/test/generators/plugin_new_generator_test.rb @@ -55,7 +55,7 @@ class PluginNewGeneratorTest < Rails::Generators::TestCase def test_ensure_that_plugin_options_are_not_passed_to_app_generator FileUtils.cd(Rails.root) - assert_no_match /It works from file!.*It works_from_file/, run_generator([destination_root, "-m", "lib/template.rb"]) + assert_no_match(/It works from file!.*It works_from_file/, run_generator([destination_root, "-m", "lib/template.rb"])) end def test_ensure_that_test_dummy_can_be_generated_from_a_template @@ -85,7 +85,7 @@ class PluginNewGeneratorTest < Rails::Generators::TestCase def test_ensure_that_skip_active_record_option_is_passed_to_app_generator run_generator [destination_root, "--skip_active_record"] assert_no_file "test/dummy/config/database.yml" - assert_no_match /ActiveRecord/, File.read(File.join(destination_root, "test/test_helper.rb")) + assert_no_match(/ActiveRecord/, File.read(File.join(destination_root, "test/test_helper.rb"))) end def test_ensure_that_database_option_is_passed_to_app_generator @@ -134,21 +134,21 @@ class PluginNewGeneratorTest < Rails::Generators::TestCase def test_template_from_dir_pwd FileUtils.cd(Rails.root) - assert_match /It works from file!/, run_generator([destination_root, "-m", "lib/template.rb"]) + assert_match(/It works from file!/, run_generator([destination_root, "-m", "lib/template.rb"])) end def test_ensure_that_tests_works run_generator FileUtils.cd destination_root `bundle install` - assert_match /1 tests, 1 assertions, 0 failures, 0 errors/, `bundle exec rake test` + assert_match(/1 tests, 1 assertions, 0 failures, 0 errors/, `bundle exec rake test`) end def test_ensure_that_tests_works_in_full_mode run_generator [destination_root, "--full", "--skip_active_record"] FileUtils.cd destination_root `bundle install` - assert_match /2 tests, 2 assertions, 0 failures, 0 errors/, `bundle exec rake test` + assert_match(/2 tests, 2 assertions, 0 failures, 0 errors/, `bundle exec rake test`) end def test_creating_engine_in_full_mode @@ -159,7 +159,7 @@ class PluginNewGeneratorTest < Rails::Generators::TestCase end def test_being_quiet_while_creating_dummy_application - assert_no_match /create\s+config\/application.rb/, run_generator + assert_no_match(/create\s+config\/application.rb/, run_generator) end def test_create_mountable_application_with_mountable_option diff --git a/railties/test/generators/shared_generator_tests.rb b/railties/test/generators/shared_generator_tests.rb index 9e6169721b..c9c5d2fad2 100644 --- a/railties/test/generators/shared_generator_tests.rb +++ b/railties/test/generators/shared_generator_tests.rb @@ -26,13 +26,12 @@ module SharedGeneratorTests def test_plugin_new_generate_pretend run_generator ["testapp", "--pretend"] - - default_files.each{ |path| assert_no_file path } + default_files.each{ |path| assert_no_file File.join("testapp",path) } end def test_invalid_database_option_raises_an_error content = capture(:stderr){ run_generator([destination_root, "-d", "unknown"]) } - assert_match /Invalid value for \-\-database option/, content + assert_match(/Invalid value for \-\-database option/, content) end def test_test_unit_is_skipped_if_required @@ -42,21 +41,21 @@ module SharedGeneratorTests def test_options_before_application_name_raises_an_error content = capture(:stderr){ run_generator(["--pretend", destination_root]) } - assert_match /Options should be given after the \w+ name. For details run: rails( plugin)? --help\n/, content + assert_match(/Options should be given after the \w+ name. For details run: rails( plugin)? --help\n/, content) end def test_name_collision_raises_an_error reserved_words = %w[application destroy plugin runner test] reserved_words.each do |reserved| content = capture(:stderr){ run_generator [File.join(destination_root, reserved)] } - assert_match /Invalid \w+ name #{reserved}. Please give a name which does not match one of the reserved rails words.\n/, content + assert_match(/Invalid \w+ name #{reserved}. Please give a name which does not match one of the reserved rails words.\n/, content) end end def test_name_raises_an_error_if_name_already_used_constant %w{ String Hash Class Module Set Symbol }.each do |ruby_class| content = capture(:stderr){ run_generator [File.join(destination_root, ruby_class)] } - assert_match /Invalid \w+ name #{ruby_class}, constant #{ruby_class} is already in use. Please choose another \w+ name.\n/, content + assert_match(/Invalid \w+ name #{ruby_class}, constant #{ruby_class} is already in use. Please choose another \w+ name.\n/, content) end end @@ -72,8 +71,8 @@ module SharedGeneratorTests def test_template_raises_an_error_with_invalid_path content = capture(:stderr){ run_generator([destination_root, "-m", "non/existant/path"]) } - assert_match /The template \[.*\] could not be loaded/, content - assert_match /non\/existant\/path/, content + assert_match(/The template \[.*\] could not be loaded/, content) + assert_match(/non\/existant\/path/, content) end def test_template_is_executed_when_supplied @@ -82,7 +81,7 @@ module SharedGeneratorTests template.instance_eval "def read; self; end" # Make the string respond to read generator([destination_root], :template => path).expects(:open).with(path, 'Accept' => 'application/x-thor-template').returns(template) - assert_match /It works!/, silence(:stdout){ generator.invoke_all } + assert_match(/It works!/, silence(:stdout){ generator.invoke_all }) end def test_dev_option @@ -100,8 +99,8 @@ module SharedGeneratorTests def test_template_raises_an_error_with_invalid_path content = capture(:stderr){ run_generator([destination_root, "-m", "non/existant/path"]) } - assert_match /The template \[.*\] could not be loaded/, content - assert_match /non\/existant\/path/, content + assert_match(/The template \[.*\] could not be loaded/, content) + assert_match(/non\/existant\/path/, content) end def test_template_is_executed_when_supplied @@ -110,7 +109,7 @@ module SharedGeneratorTests template.instance_eval "def read; self; end" # Make the string respond to read generator([destination_root], :template => path).expects(:open).with(path, 'Accept' => 'application/x-thor-template').returns(template) - assert_match /It works!/, silence(:stdout){ generator.invoke_all } + assert_match(/It works!/, silence(:stdout){ generator.invoke_all }) end def test_template_is_executed_when_supplied_an_https_path @@ -119,7 +118,7 @@ module SharedGeneratorTests template.instance_eval "def read; self; end" # Make the string respond to read generator([destination_root], :template => path).expects(:open).with(path, 'Accept' => 'application/x-thor-template').returns(template) - assert_match /It works!/, silence(:stdout){ generator.invoke_all } + assert_match(/It works!/, silence(:stdout){ generator.invoke_all }) end def test_dev_option @@ -191,6 +190,6 @@ module SharedCustomGeneratorTests generator([destination_root], :builder => path).expects(:open).with(path, 'Accept' => 'application/x-thor-template').returns(template) capture(:stdout) { generator.invoke_all } - default_files.each{ |path| assert_no_file path } + default_files.each{ |path| assert_no_file(path) } end end |