diff options
102 files changed, 1027 insertions, 608 deletions
@@ -5,7 +5,7 @@ gemspec if ENV['AREL'] gem 'arel', path: ENV['AREL'] else - gem 'arel' + gem 'arel', github: 'rails/arel' end gem 'mocha', '>= 0.11.2', :require => false diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index a0d1064094..0050ede806 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -436,10 +436,12 @@ module ActionController values = Hash[$1.split(',').map do |value| value.strip! # remove any spaces between commas and values key, value = value.split(/\=\"?/) # split key=value pairs - value.chomp!('"') # chomp trailing " in value - value.gsub!(/\\\"/, '"') # unescape remaining quotes - [key, value] - end] + if value + value.chomp!('"') # chomp trailing " in value + value.gsub!(/\\\"/, '"') # unescape remaining quotes + [key, value] + end + end.compact] [values.delete("token"), values.with_indifferent_access] end end diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb index 83407846dc..d9c89a74f1 100644 --- a/actionpack/lib/action_controller/metal/responder.rb +++ b/actionpack/lib/action_controller/metal/responder.rb @@ -90,7 +90,7 @@ module ActionController #:nodoc: # # def create # @project = Project.find(params[:project_id]) - # @task = @project.comments.build(params[:task]) + # @task = @project.tasks.build(params[:task]) # flash[:notice] = 'Task was successfully created.' if @task.save # respond_with(@project, @task, :status => 201) # end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 53a4afecb3..ee90bfe63f 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -262,7 +262,7 @@ module ActionDispatch # for root cases, where the latter is the correct one. def self.normalize_path(path) path = Journey::Router::Utils.normalize_path(path) - path.gsub!(%r{/(\(+)/?}, '\1/') unless path =~ %r{^/\(+[^/]+\)$} + path.gsub!(%r{/(\(+)/?}, '\1/') unless path =~ %r{^/\(+[^)]+\)$} path end @@ -430,6 +430,10 @@ module ActionDispatch if options path = options.delete(:at) else + unless Hash === app + raise ArgumentError, "must be called with mount point" + end + options = app app, path = options.find { |k, v| k.respond_to?(:call) } options.delete(app) if app diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 1d6ca0c78d..0bbed6cbea 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -333,7 +333,7 @@ module ActionDispatch end end - MountedHelpers.class_eval <<-RUBY + MountedHelpers.class_eval(<<-RUBY, __FILE__, __LINE__ + 1) def #{name} @#{name} ||= _#{name} end diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index 41fa3a4b95..9de545b3c5 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -127,16 +127,13 @@ module ActionDispatch # with a new RouteSet instance. # # The new instance is yielded to the passed block. Typically the block - # will create some routes using <tt>map.draw { map.connect ... }</tt>: + # will create some routes using <tt>set.draw { match ... }</tt>: # # with_routing do |set| - # set.draw do |map| - # map.connect ':controller/:action/:id' - # assert_equal( - # ['/content/10/show', {}], - # map.generate(:controller => 'content', :id => 10, :action => 'show') - # end + # set.draw do + # resources :users # end + # assert_equal "/users", users_path # end # def with_routing diff --git a/actionpack/lib/action_view/context.rb b/actionpack/lib/action_view/context.rb index 245849d706..ee263df484 100644 --- a/actionpack/lib/action_view/context.rb +++ b/actionpack/lib/action_view/context.rb @@ -26,11 +26,11 @@ module ActionView # Encapsulates the interaction with the view flow so it # returns the correct buffer on +yield+. This is usually - # overwriten by helpers to add more behavior. + # overwritten by helpers to add more behavior. # :api: plugin def _layout_for(name=nil) name ||= :layout view_flow.get(name).html_safe end end -end
\ No newline at end of file +end diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb index eef426703d..72fbbd109a 100644 --- a/actionpack/lib/action_view/helpers/form_options_helper.rb +++ b/actionpack/lib/action_view/helpers/form_options_helper.rb @@ -353,7 +353,7 @@ module ActionView html_attributes[:disabled] = 'disabled' if disabled && option_value_selected?(value, disabled) html_attributes[:value] = value - content_tag(:option, text, html_attributes) + content_tag_string(:option, text, html_attributes) end.join("\n").html_safe end diff --git a/actionpack/lib/action_view/helpers/tags/base.rb b/actionpack/lib/action_view/helpers/tags/base.rb index e077cd5b3c..192f5eebaa 100644 --- a/actionpack/lib/action_view/helpers/tags/base.rb +++ b/actionpack/lib/action_view/helpers/tags/base.rb @@ -137,10 +137,10 @@ module ActionView def add_options(option_tags, options, value = nil) if options[:include_blank] - option_tags = content_tag('option', options[:include_blank].kind_of?(String) ? options[:include_blank] : nil, :value => '') + "\n" + option_tags + option_tags = content_tag_string('option', options[:include_blank].kind_of?(String) ? options[:include_blank] : nil, :value => '') + "\n" + option_tags end if value.blank? && options[:prompt] - option_tags = content_tag('option', prompt_text(options[:prompt]), :value => '') + "\n" + option_tags + option_tags = content_tag_string('option', prompt_text(options[:prompt]), :value => '') + "\n" + option_tags end option_tags end diff --git a/actionpack/lib/action_view/lookup_context.rb b/actionpack/lib/action_view/lookup_context.rb index 00989ec405..47dd932c71 100644 --- a/actionpack/lib/action_view/lookup_context.rb +++ b/actionpack/lib/action_view/lookup_context.rb @@ -96,7 +96,7 @@ module ActionView # Helpers related to template lookup using the lookup context information. module ViewPaths - attr_reader :view_paths + attr_reader :view_paths, :html_fallback_for_js # Whenever setting view paths, makes a copy so we can manipulate then in # instance objects as we wish. @@ -184,7 +184,10 @@ module ActionView def formats=(values) if values values.concat(default_formats) if values.delete "*/*" - values << :html if values == [:js] + if values == [:js] + values << :html + @html_fallback_for_js = true + end end super(values) end diff --git a/actionpack/lib/action_view/renderer/abstract_renderer.rb b/actionpack/lib/action_view/renderer/abstract_renderer.rb index e3d8e9d508..6fb8cbb46c 100644 --- a/actionpack/lib/action_view/renderer/abstract_renderer.rb +++ b/actionpack/lib/action_view/renderer/abstract_renderer.rb @@ -22,5 +22,11 @@ module ActionView def instrument(name, options={}) ActiveSupport::Notifications.instrument("render_#{name}.action_view", options){ yield } end + + def prepend_formats(formats) + formats = Array(formats) + return if formats.empty? || @lookup_context.html_fallback_for_js + @lookup_context.formats = formats | @lookup_context.formats + end end end diff --git a/actionpack/lib/action_view/renderer/partial_renderer.rb b/actionpack/lib/action_view/renderer/partial_renderer.rb index 9100545718..a08a566b35 100644 --- a/actionpack/lib/action_view/renderer/partial_renderer.rb +++ b/actionpack/lib/action_view/renderer/partial_renderer.rb @@ -320,6 +320,8 @@ module ActionView @block = block @details = extract_details(options) + prepend_formats(options[:formats]) + if String === partial @object = options[:object] @path = partial diff --git a/actionpack/lib/action_view/renderer/template_renderer.rb b/actionpack/lib/action_view/renderer/template_renderer.rb index 3c1b11396a..156ad4e547 100644 --- a/actionpack/lib/action_view/renderer/template_renderer.rb +++ b/actionpack/lib/action_view/renderer/template_renderer.rb @@ -8,9 +8,10 @@ module ActionView template = determine_template(options) context = @lookup_context + prepend_formats(template.formats) + unless context.rendered_format - context.formats = template.formats unless template.formats.empty? - context.rendered_format = context.formats.first + context.rendered_format = template.formats.first || formats.last end render_template(template, options[:layout], options[:locals]) diff --git a/actionpack/test/controller/http_token_authentication_test.rb b/actionpack/test/controller/http_token_authentication_test.rb index 3054c1684c..ad4e743be8 100644 --- a/actionpack/test/controller/http_token_authentication_test.rb +++ b/actionpack/test/controller/http_token_authentication_test.rb @@ -79,6 +79,14 @@ class HttpTokenAuthenticationTest < ActionController::TestCase end end + test "authentication request with badly formatted header" do + @request.env['HTTP_AUTHORIZATION'] = "Token foobar" + get :index + + assert_response :unauthorized + assert_equal "HTTP Token: Access denied.\n", @response.body, "Authentication header was not properly parsed" + end + test "authentication request without credential" do get :display diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb index 8070bdec8a..58457b0c28 100644 --- a/actionpack/test/dispatch/mapper_test.rb +++ b/actionpack/test/dispatch/mapper_test.rb @@ -98,6 +98,15 @@ module ActionDispatch mapper.get '/*path', :to => 'pages#show', :format => true assert_equal '/*path.:format', fakeset.conditions.first[:path_info] end + + def test_raising_helpful_error_on_invalid_arguments + fakeset = FakeSet.new + mapper = Mapper.new fakeset + app = lambda { |env| [200, {}, [""]] } + assert_raises ArgumentError do + mapper.mount app + end + end end end end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index fed26d89f8..a42ac60917 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -2679,3 +2679,30 @@ class TestInvalidUrls < ActionDispatch::IntegrationTest end end end + +class TestOptionalRootSegments < ActionDispatch::IntegrationTest + stub_controllers do |routes| + Routes = routes + Routes.draw do + get '/(page/:page)', :to => 'pages#index', :as => :root + end + end + + def app + Routes + end + + include Routes.url_helpers + + def test_optional_root_segments + get '/' + assert_equal 'pages#index', @response.body + assert_equal '/', root_path + + get '/page/1' + assert_equal 'pages#index', @response.body + assert_equal '1', @request.params[:page] + assert_equal '/page/1', root_path('1') + assert_equal '/page/1', root_path(:page => '1') + end +end diff --git a/actionpack/test/fixtures/test/_changing_priority.html.erb b/actionpack/test/fixtures/test/_changing_priority.html.erb new file mode 100644 index 0000000000..3225efc49a --- /dev/null +++ b/actionpack/test/fixtures/test/_changing_priority.html.erb @@ -0,0 +1 @@ +HTML
\ No newline at end of file diff --git a/actionpack/test/fixtures/test/_changing_priority.json.erb b/actionpack/test/fixtures/test/_changing_priority.json.erb new file mode 100644 index 0000000000..7fa41dce66 --- /dev/null +++ b/actionpack/test/fixtures/test/_changing_priority.json.erb @@ -0,0 +1 @@ +JSON
\ No newline at end of file diff --git a/actionpack/test/fixtures/test/_first_json_partial.json.erb b/actionpack/test/fixtures/test/_first_json_partial.json.erb new file mode 100644 index 0000000000..790ee896db --- /dev/null +++ b/actionpack/test/fixtures/test/_first_json_partial.json.erb @@ -0,0 +1 @@ +<%= render :partial => "test/second_json_partial" %>
\ No newline at end of file diff --git a/actionpack/test/fixtures/test/_json_change_priority.json.erb b/actionpack/test/fixtures/test/_json_change_priority.json.erb new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/actionpack/test/fixtures/test/_json_change_priority.json.erb diff --git a/actionpack/test/fixtures/test/_second_json_partial.json.erb b/actionpack/test/fixtures/test/_second_json_partial.json.erb new file mode 100644 index 0000000000..5ebb7f1afd --- /dev/null +++ b/actionpack/test/fixtures/test/_second_json_partial.json.erb @@ -0,0 +1 @@ +Third level
\ No newline at end of file diff --git a/actionpack/test/fixtures/test/change_priorty.html.erb b/actionpack/test/fixtures/test/change_priorty.html.erb new file mode 100644 index 0000000000..5618977d05 --- /dev/null +++ b/actionpack/test/fixtures/test/change_priorty.html.erb @@ -0,0 +1,2 @@ +<%= render :partial => "test/json_change_priority", formats: :json %> +HTML Template, but <%= render :partial => "test/changing_priority" %> partial
\ No newline at end of file diff --git a/actionpack/test/fixtures/test/html_template.html.erb b/actionpack/test/fixtures/test/html_template.html.erb new file mode 100644 index 0000000000..1bbc2b7f09 --- /dev/null +++ b/actionpack/test/fixtures/test/html_template.html.erb @@ -0,0 +1 @@ +<%= render :partial => "test/first_json_partial", formats: :json %>
\ No newline at end of file diff --git a/actionpack/test/template/active_model_helper_test.rb b/actionpack/test/template/active_model_helper_test.rb index 24511df444..86bccdfade 100644 --- a/actionpack/test/template/active_model_helper_test.rb +++ b/actionpack/test/template/active_model_helper_test.rb @@ -41,6 +41,19 @@ class ActiveModelHelperTest < ActionView::TestCase ) end + def test_select_with_errors + assert_dom_equal( + %(<div class="field_with_errors"><select name="post[author_name]" id="post_author_name"><option value="a">a</option>\n<option value="b">b</option></select></div>), + select("post", "author_name", [:a, :b]) + ) + end + + def test_select_with_errors_and_blank_option + expected_dom = %(<div class="field_with_errors"><select name="post[author_name]" id="post_author_name"><option value="">Choose one...</option>\n<option value="a">a</option>\n<option value="b">b</option></select></div>) + assert_dom_equal(expected_dom, select("post", "author_name", [:a, :b], :include_blank => 'Choose one...')) + assert_dom_equal(expected_dom, select("post", "author_name", [:a, :b], :prompt => 'Choose one...')) + end + def test_date_select_with_errors assert_dom_equal( %(<div class="field_with_errors"><select id="post_updated_at_1i" name="post[updated_at(1i)]">\n<option selected="selected" value="2004">2004</option>\n<option value="2005">2005</option>\n</select>\n<input id="post_updated_at_2i" name="post[updated_at(2i)]" type="hidden" value="6" />\n<input id="post_updated_at_3i" name="post[updated_at(3i)]" type="hidden" value="1" />\n</div>), @@ -82,4 +95,5 @@ class ActiveModelHelperTest < ActionView::TestCase ensure ActionView::Base.field_error_proc = old_proc if old_proc end + end diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index 88ed8664c2..3ce1d20bd9 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -54,6 +54,16 @@ module RenderTestCases assert_equal "Hello world", @view.render(:template => "test/one", :formats => [:html]) end + def test_render_partial_implicitly_use_format_of_the_rendered_partial + @view.lookup_context.formats = [:html] + assert_equal "Third level", @view.render(:template => "test/html_template") + end + + def test_render_partial_use_last_prepended_format_for_partials_with_the_same_names + @view.lookup_context.formats = [:html] + assert_equal "\nHTML Template, but JSON partial", @view.render(:template => "test/change_priorty") + end + def test_render_template_with_a_missing_partial_of_another_format @view.lookup_context.formats = [:html] assert_raise ActionView::Template::Error, "Missing partial /missing with {:locale=>[:en], :formats=>[:json], :handlers=>[:erb, :builder]}" do diff --git a/activemodel/lib/active_model/validations/acceptance.rb b/activemodel/lib/active_model/validations/acceptance.rb index 18e71750de..8d5ebf527f 100644 --- a/activemodel/lib/active_model/validations/acceptance.rb +++ b/activemodel/lib/active_model/validations/acceptance.rb @@ -37,9 +37,6 @@ module ActiveModel # Configuration options: # * <tt>:message</tt> - A custom error message (default is: "must be # accepted"). - # * <tt>:on</tt> - Specifies when this validation is active. Runs in all - # validation contexts by default (+nil+), other options are <tt>:create</tt> - # and <tt>:update</tt>. # * <tt>:allow_nil</tt> - Skip validation if attribute is +nil+ (default # is +true+). # * <tt>:accept</tt> - Specifies value that is considered accepted. @@ -47,19 +44,10 @@ module ActiveModel # an HTML checkbox. This should be set to +true+ if you are validating # a database column, since the attribute is typecast from "1" to +true+ # before validation. - # * <tt>:if</tt> - Specifies a method, proc or string to call to determine - # if the validation should occur (e.g. <tt>if: :allow_validation</tt>, - # or <tt>if: Proc.new { |user| user.signup_step > 2 }</tt>). The - # method, proc or string should return or evaluate to a +true+ or - # +false+ value. - # * <tt>:unless</tt> - Specifies a method, proc or string to call to - # determine if the validation should not occur (for example, - # <tt>unless: :skip_validation</tt>, or - # <tt>unless: Proc.new { |user| user.signup_step <= 2 }</tt>). - # The method, proc or string should return or evaluate to a +true+ or - # +false+ value. - # * <tt>:strict</tt> - Specifies whether validation should be strict. - # See <tt>ActiveModel::Validation#validates!</tt> for more information. + # + # There is also a list of default options supported by every validator: + # +:if+, +:unless+, +:on+ and +:strict+. + # See <tt>ActiveModel::Validation#validates</tt> for more information def validates_acceptance_of(*attr_names) validates_with AcceptanceValidator, _merge_attributes(attr_names) end diff --git a/activemodel/lib/active_model/validations/confirmation.rb b/activemodel/lib/active_model/validations/confirmation.rb index d41f401999..baa034eca6 100644 --- a/activemodel/lib/active_model/validations/confirmation.rb +++ b/activemodel/lib/active_model/validations/confirmation.rb @@ -46,22 +46,10 @@ module ActiveModel # Configuration options: # * <tt>:message</tt> - A custom error message (default is: "doesn't match # confirmation"). - # * <tt>:on</tt> - Specifies when this validation is active. Runs in all - # validation contexts by default (+nil+), other options are <tt>:create</tt> - # and <tt>:update</tt>. - # * <tt>:if</tt> - Specifies a method, proc or string to call to determine - # if the validation should occur (e.g. <tt>if: :allow_validation</tt>, - # or <tt>if: Proc.new { |user| user.signup_step > 2 }</tt>). The - # method, proc or string should return or evaluate to a +true+ or - # +false+ value. - # * <tt>:unless</tt> - Specifies a method, proc or string to call to - # determine if the validation should not occur (e.g. - # <tt>unless: :skip_validation</tt>, or - # <tt>unless: Proc.new { |user| user.signup_step <= 2 }</tt>). The - # method, proc or string should return or evaluate to a +true+ or - # +false+ value. - # * <tt>:strict</tt> - Specifies whether validation should be strict. - # See <tt>ActiveModel::Validation#validates!</tt> for more information. + # + # There is also a list of default options supported by every validator: + # +:if+, +:unless+, +:on+ and +:strict+. + # See <tt>ActiveModel::Validation#validates</tt> for more information def validates_confirmation_of(*attr_names) validates_with ConfirmationValidator, _merge_attributes(attr_names) end diff --git a/activemodel/lib/active_model/validations/exclusion.rb b/activemodel/lib/active_model/validations/exclusion.rb index a9a1af039c..3331162bc0 100644 --- a/activemodel/lib/active_model/validations/exclusion.rb +++ b/activemodel/lib/active_model/validations/exclusion.rb @@ -37,20 +37,10 @@ module ActiveModel # attribute is +nil+ (default is +false+). # * <tt>:allow_blank</tt> - If set to true, skips this validation if the # attribute is blank(default is +false+). - # * <tt>:on</tt> - Specifies when this validation is active. Runs in all - # validation contexts by default (+nil+), other options are <tt>:create</tt> - # and <tt>:update</tt>. - # * <tt>:if</tt> - Specifies a method, proc or string to call to determine if - # the validation should occur (e.g. <tt>if: :allow_validation</tt>, or - # <tt>if: Proc.new { |user| user.signup_step > 2 }</tt>). The method, - # proc or string should return or evaluate to a +true+ or +false+ value. - # * <tt>:unless</tt> - Specifies a method, proc or string to call to determine - # if the validation should not occur (e.g. <tt>unless: :skip_validation</tt>, - # or <tt>unless: Proc.new { |user| user.signup_step <= 2 }</tt>). The - # method, proc or string should return or evaluate to a +true+ or - # +false+ value. - # * <tt>:strict</tt> - Specifies whether validation should be strict. - # See <tt>ActiveModel::Validation#validates!</tt> for more information. + # + # There is also a list of default options supported by every validator: + # +:if+, +:unless+, +:on+ and +:strict+. + # See <tt>ActiveModel::Validation#validates</tt> for more information def validates_exclusion_of(*attr_names) validates_with ExclusionValidator, _merge_attributes(attr_names) end diff --git a/activemodel/lib/active_model/validations/format.rb b/activemodel/lib/active_model/validations/format.rb index a2126f8372..80150229a0 100644 --- a/activemodel/lib/active_model/validations/format.rb +++ b/activemodel/lib/active_model/validations/format.rb @@ -101,23 +101,13 @@ module ActiveModel # match will result in a successful validation. This can be provided as # a proc or lambda returning regular expression which will be called at # runtime. - # * <tt>:on</tt> - Specifies when this validation is active. Runs in all - # validation contexts by default (+nil+), other options are <tt>:create</tt> - # and <tt>:update</tt>. - # * <tt>:if</tt> - Specifies a method, proc or string to call to determine - # if the validation should occur (e.g. <tt>if: :allow_validation</tt>, - # or <tt>if: Proc.new { |user| user.signup_step > 2 }</tt>). The method, - # proc or string should return or evaluate to a +true+ or +false+ value. - # * <tt>:unless</tt> - Specifies a method, proc or string to call to determine - # if the validation should not occur (e.g. <tt>unless: :skip_validation</tt>, - # or <tt>unless: Proc.new { |user| user.signup_step <= 2 }</tt>). The - # method, proc or string should return or evaluate to a +true+ or - # +false+ value. - # * <tt>:strict</tt> - Specifies whether validation should be strict. - # See <tt>ActiveModel::Validation#validates!</tt> for more information. # * <tt>:multiline</tt> - Set to true if your regular expression contains # anchors that match the beginning or end of lines as opposed to the # beginning or end of the string. These anchors are <tt>^</tt> and <tt>$</tt>. + # + # There is also a list of default options supported by every validator: + # +:if+, +:unless+, +:on+ and +:strict+. + # See <tt>ActiveModel::Validation#validates</tt> for more information def validates_format_of(*attr_names) validates_with FormatValidator, _merge_attributes(attr_names) end diff --git a/activemodel/lib/active_model/validations/inclusion.rb b/activemodel/lib/active_model/validations/inclusion.rb index e7ad07f040..c995e28d64 100644 --- a/activemodel/lib/active_model/validations/inclusion.rb +++ b/activemodel/lib/active_model/validations/inclusion.rb @@ -36,20 +36,10 @@ module ActiveModel # attribute is +nil+ (default is +false+). # * <tt>:allow_blank</tt> - If set to +true+, skips this validation if the # attribute is blank (default is +false+). - # * <tt>:on</tt> - Specifies when this validation is active. Runs in all - # validation contexts by default (+nil+), other options are <tt>:create</tt> - # and <tt>:update</tt>. - # * <tt>:if</tt> - Specifies a method, proc or string to call to determine if - # the validation should occur (e.g. <tt>if: :allow_validation</tt>, or - # <tt>if: Proc.new { |user| user.signup_step > 2 }</tt>). The method, proc - # or string should return or evaluate to a +true+ or +false+ value. - # * <tt>:unless</tt> - Specifies a method, proc or string to call to determine - # if the validation should not occur (e.g. <tt>unless: :skip_validation</tt>, - # or <tt>unless: Proc.new { |user| user.signup_step <= 2 }</tt>). The - # method, proc or string should return or evaluate to a +true+ or - # +false+ value. - # * <tt>:strict</tt> - Specifies whether validation should be strict. - # See <tt>ActiveModel::Validation#validates!</tt> for more information. + # + # There is also a list of default options supported by every validator: + # +:if+, +:unless+, +:on+ and +:strict+. + # See <tt>ActiveModel::Validation#validates</tt> for more information def validates_inclusion_of(*attr_names) validates_with InclusionValidator, _merge_attributes(attr_names) end diff --git a/activemodel/lib/active_model/validations/length.rb b/activemodel/lib/active_model/validations/length.rb index 3b0be96ac6..aa72ea41c7 100644 --- a/activemodel/lib/active_model/validations/length.rb +++ b/activemodel/lib/active_model/validations/length.rb @@ -96,24 +96,14 @@ module ActiveModel # * <tt>:message</tt> - The error message to use for a <tt>:minimum</tt>, # <tt>:maximum</tt>, or <tt>:is</tt> violation. An alias of the appropriate # <tt>too_long</tt>/<tt>too_short</tt>/<tt>wrong_length</tt> message. - # * <tt>:on</tt> - Specifies when this validation is active. Runs in all - # validation contexts by default (+nil+), other options are <tt>:create</tt> - # and <tt>:update</tt>. - # * <tt>:if</tt> - Specifies a method, proc or string to call to determine if - # the validation should occur (e.g. <tt>if: :allow_validation</tt>, or - # <tt>if: Proc.new { |user| user.signup_step > 2 }</tt>). The method, - # proc or string should return or evaluate to a +true+ or +false+ value. - # * <tt>:unless</tt> - Specifies a method, proc or string to call to determine - # if the validation should not occur (e.g. <tt>unless: :skip_validation</tt>, - # or <tt>unless: Proc.new { |user| user.signup_step <= 2 }</tt>). The - # method, proc or string should return or evaluate to a +true+ or - # +false+ value. # * <tt>:tokenizer</tt> - Specifies how to split up the attribute string. # (e.g. <tt>tokenizer: ->(str) { str.scan(/\w+/) }</tt> to count words # as in above example). Defaults to <tt>->(value) { value.split(//) }</tt> # which counts individual characters. - # * <tt>:strict</tt> - Specifies whether validation should be strict. - # See <tt>ActiveModel::Validation#validates!</tt> for more information. + # + # There is also a list of default options supported by every validator: + # +:if+, +:unless+, +:on+ and +:strict+. + # See <tt>ActiveModel::Validation#validates</tt> for more information def validates_length_of(*attr_names) validates_with LengthValidator, _merge_attributes(attr_names) end diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index 6b3dd6caec..edebca94a8 100644 --- a/activemodel/lib/active_model/validations/numericality.rb +++ b/activemodel/lib/active_model/validations/numericality.rb @@ -90,9 +90,6 @@ module ActiveModel # # Configuration options: # * <tt>:message</tt> - A custom error message (default is: "is not a number"). - # * <tt>:on</tt> - Specifies when this validation is active. Runs in all - # validation contexts by default (+nil+), other options are <tt>:create</tt> - # and <tt>:update</tt>. # * <tt>:only_integer</tt> - Specifies whether the value has to be an # integer, e.g. an integral value (default is +false+). # * <tt>:allow_nil</tt> - Skip validation if attribute is +nil+ (default is @@ -112,17 +109,10 @@ module ActiveModel # supplied value. # * <tt>:odd</tt> - Specifies the value must be an odd number. # * <tt>:even</tt> - Specifies the value must be an even number. - # * <tt>:if</tt> - Specifies a method, proc or string to call to determine - # if the validation should occur (e.g. <tt>if: :allow_validation</tt>, - # or <tt>if: Proc.new { |user| user.signup_step > 2 }</tt>). The method, - # proc or string should return or evaluate to a +true+ or +false+ value. - # * <tt>:unless</tt> - Specifies a method, proc or string to call to determine - # if the validation should not occur (e.g. <tt>unless: :skip_validation</tt>, - # or <tt>unless: Proc.new { |user| user.signup_step <= 2 }</tt>). The - # method, proc or string should return or evaluate to a +true+ or - # +false+ value. - # * <tt>:strict</tt> - Specifies whether validation should be strict. - # See <tt>ActiveModel::Validation#validates!</tt> for more information. + # + # There is also a list of default options supported by every validator: + # +:if+, +:unless+, +:on+ and +:strict+ . + # See <tt>ActiveModel::Validation#validates</tt> for more information # # The following checks can also be supplied with a proc or a symbol which # corresponds to a method: diff --git a/activemodel/lib/active_model/validations/presence.rb b/activemodel/lib/active_model/validations/presence.rb index cb7e9104db..4592a1deb0 100644 --- a/activemodel/lib/active_model/validations/presence.rb +++ b/activemodel/lib/active_model/validations/presence.rb @@ -29,20 +29,10 @@ module ActiveModel # # Configuration options: # * <tt>:message</tt> - A custom error message (default is: "can't be blank"). - # * <tt>:on</tt> - Specifies when this validation is active. Runs in all - # validation contexts by default (+nil+), other options are <tt>:create</tt> - # and <tt>:update</tt>. - # * <tt>:if</tt> - Specifies a method, proc or string to call to determine - # if the validation should occur (e.g. <tt>if: :allow_validation</tt>, - # or <tt>if: Proc.new { |user| user.signup_step > 2 }</tt>). The method, - # proc or string should return or evaluate to a +true+ or +false+ value. - # * <tt>:unless</tt> - Specifies a method, proc or string to call to determine - # if the validation should not occur (e.g. <tt>unless: :skip_validation</tt>, - # or <tt>unless: Proc.new { |user| user.signup_step <= 2 }</tt>). The - # method, proc or string should return or evaluate to a +true+ or - # +false+ value. - # * <tt>:strict</tt> - Specifies whether validation should be strict. - # See <tt>ActiveModel::Validation#validates!</tt> for more information. + # + # There is also a list of default options supported by every validator: + # +:if+, +:unless+, +:on+ and +:strict+. + # See <tt>ActiveModel::Validation#validates</tt> for more information def validates_presence_of(*attr_names) validates_with PresenceValidator, _merge_attributes(attr_names) end diff --git a/activemodel/lib/active_model/validations/validates.rb b/activemodel/lib/active_model/validations/validates.rb index 6c13d2b4a2..ecda9dffcf 100644 --- a/activemodel/lib/active_model/validations/validates.rb +++ b/activemodel/lib/active_model/validations/validates.rb @@ -69,14 +69,31 @@ module ActiveModel # validator's initializer as +options[:in]+ while other types including # regular expressions and strings are passed as +options[:with]+ # + # There is also a list of options that could be used along with validators: + # * <tt>:on</tt> - Specifies when this validation is active. Runs in all + # validation contexts by default (+nil+), other options are <tt>:create</tt> + # and <tt>:update</tt>. + # * <tt>:if</tt> - Specifies a method, proc or string to call to determine + # if the validation should occur (e.g. <tt>if: :allow_validation</tt>, + # or <tt>if: Proc.new { |user| user.signup_step > 2 }</tt>). The method, + # proc or string should return or evaluate to a +true+ or +false+ value. + # * <tt>:unless</tt> - Specifies a method, proc or string to call to determine + # if the validation should not occur (e.g. <tt>unless: :skip_validation</tt>, + # or <tt>unless: Proc.new { |user| user.signup_step <= 2 }</tt>). The + # method, proc or string should return or evaluate to a +true+ or + # +false+ value. + # * <tt>:strict</tt> - Specifies whether validation should be strict. + # See <tt>ActiveModel::Validation#validates!</tt> for more information. + # + # Example: + # + # validates :password, :presence => true, :confirmation => true, :if => :password_required? + # # Finally, the options +:if+, +:unless+, +:on+, +:allow_blank+, +:allow_nil+ and +:strict+ # can be given to one specific validator, as a hash: # # validates :password, :presence => { :if => :password_required? }, :confirmation => true # - # Or to all at the same time: - # - # validates :password, :presence => true, :confirmation => true, :if => :password_required? # def validates(*attributes) defaults = attributes.extract_options!.dup diff --git a/activerecord/RUNNING_UNIT_TESTS b/activerecord/RUNNING_UNIT_TESTS index 2c310e7ac3..bdd8834dcb 100644 --- a/activerecord/RUNNING_UNIT_TESTS +++ b/activerecord/RUNNING_UNIT_TESTS @@ -1,11 +1,10 @@ -== Configure databases +== Setup -Copy test/config.example.yml to test/config.yml and edit as needed. Or just run the tests for -the first time, which will do the copy automatically and use the default (sqlite3). +If you don't have the environment set make sure to read -You can build postgres and mysql databases using the postgresql:build_databases and mysql:build_databases rake tasks. + http://edgeguides.rubyonrails.org/contributing_to_ruby_on_rails.html#testing-active-record. -== Running the tests +== Running the Tests You can run a particular test file from the command line, e.g. @@ -26,7 +25,7 @@ You can run all the tests for a given database via rake: The 'rake test' task will run all the tests for mysql, mysql2, sqlite3 and postgresql. -== Config file +== Custom Config file By default, the config file is expected to be at the path test/config.yml. You can specify a custom location with the ARCONFIG environment variable. diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index a62fce4756..a90c5a2a9a 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1192,8 +1192,8 @@ module ActiveRecord # ORDER BY p.first_name # } # } - def has_many(name, options = {}, &extension) - Builder::HasMany.build(self, name, options, &extension) + def has_many(name, scope = {}, options = nil, &extension) + Builder::HasMany.build(self, name, scope, options, &extension) end # Specifies a one-to-one association with another class. This method should only be used @@ -1308,8 +1308,8 @@ module ActiveRecord # has_one :boss, :readonly => :true # has_one :club, :through => :membership # has_one :primary_address, :through => :addressables, :conditions => ["addressable.primary = ?", true], :source => :addressable - def has_one(name, options = {}) - Builder::HasOne.build(self, name, options) + def has_one(name, scope = {}, options = nil) + Builder::HasOne.build(self, name, scope, options) end # Specifies a one-to-one association with another class. This method should only be used @@ -1431,8 +1431,8 @@ module ActiveRecord # belongs_to :post, :counter_cache => true # belongs_to :company, :touch => true # belongs_to :company, :touch => :employees_last_updated_at - def belongs_to(name, options = {}) - Builder::BelongsTo.build(self, name, options) + def belongs_to(name, scope = {}, options = nil) + Builder::BelongsTo.build(self, name, scope, options) end # Specifies a many-to-many relationship with another class. This associates two classes via an @@ -1605,8 +1605,8 @@ module ActiveRecord # has_and_belongs_to_many :categories, :readonly => true # has_and_belongs_to_many :active_projects, :join_table => 'developers_projects', :delete_sql => # proc { |record| "DELETE FROM developers_projects WHERE active=1 AND developer_id = #{id} AND project_id = #{record.id}" } - def has_and_belongs_to_many(name, options = {}, &extension) - Builder::HasAndBelongsToMany.build(self, name, options, &extension) + def has_and_belongs_to_many(name, scope = {}, options = nil, &extension) + Builder::HasAndBelongsToMany.build(self, name, scope, options, &extension) end end end diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index e75003f261..4b989793da 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -148,6 +148,21 @@ module ActiveRecord end end + # We can't dump @reflection since it contains the scope proc + def marshal_dump + reflection = @reflection + @reflection = nil + + ivars = instance_variables.map { |name| [name, instance_variable_get(name)] } + [reflection.name, ivars] + end + + def marshal_load(data) + reflection_name, ivars = data + ivars.each { |name, val| instance_variable_set(name, val) } + @reflection = @owner.class.reflect_on_association(reflection_name) + end + private def find_target? diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index 89a626693d..1303822868 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -6,7 +6,7 @@ module ActiveRecord attr_reader :association, :alias_tracker delegate :klass, :owner, :reflection, :interpolate, :to => :association - delegate :chain, :conditions, :options, :source_options, :active_record, :to => :reflection + delegate :chain, :scope_chain, :options, :source_options, :active_record, :to => :reflection def initialize(association) @association = association @@ -15,20 +15,7 @@ module ActiveRecord def scope scope = klass.unscoped - - scope.extending!(*Array(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.merge!(options.slice( - :readonly, :references, :order, :limit, :joins, :group, :having, :offset, :select, :uniq)) - - if options[:include] - scope.includes! options[:include] - elsif options[:through] - scope.includes! source_options[:include] - end - + scope.merge! eval_scope(klass, reflection.scope) if reflection.scope add_constraints(scope) end @@ -82,8 +69,6 @@ module ActiveRecord foreign_key = reflection.active_record_primary_key end - conditions = self.conditions[i] - if reflection == chain.last bind_val = bind scope, table.table_name, key.to_s, owner[foreign_key] scope = scope.where(table[key].eq(bind_val)) @@ -93,14 +78,6 @@ module ActiveRecord bind_val = bind scope, table.table_name, reflection.type.to_s, value scope = scope.where(table[reflection.type].eq(bind_val)) end - - conditions.each do |condition| - if options[:through] && condition.is_a?(Hash) - condition = disambiguate_condition(table, condition) - end - - scope = scope.where(interpolate(condition)) - end else constraint = table[key].eq(foreign_table[foreign_key]) @@ -110,13 +87,15 @@ module ActiveRecord end scope = scope.joins(join(foreign_table, constraint)) + end - conditions.each do |condition| - condition = interpolate(condition) - condition = disambiguate_condition(table, condition) unless i == 0 + # Exclude the scope of the association itself, because that + # was already merged in the #scope method. + (scope_chain[i] - [self.reflection.scope]).each do |scope_chain_item| + item = eval_scope(reflection.klass, scope_chain_item) - scope = scope.where(condition) - end + scope.includes! item.includes_values + scope.where_values += item.where_values end end @@ -138,19 +117,11 @@ module ActiveRecord end end - def disambiguate_condition(table, condition) - if condition.is_a?(Hash) - Hash[ - condition.map do |k, v| - if v.is_a?(Hash) - [k, v] - else - [table.table_alias || table.name, { k => v }] - end - end - ] + def eval_scope(klass, scope) + if scope.is_a?(Relation) + scope else - condition + klass.unscoped.instance_exec(owner, &scope) end end end diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index 9a6896dd55..4a78a9c076 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -1,36 +1,61 @@ module ActiveRecord::Associations::Builder class Association #:nodoc: - class_attribute :valid_options - self.valid_options = [:class_name, :foreign_key, :select, :conditions, :include, :extend, :readonly, :validate, :references] + class << self + attr_accessor :valid_options + end - # Set by subclasses - class_attribute :macro + self.valid_options = [:class_name, :foreign_key, :validate] - attr_reader :model, :name, :options, :reflection + attr_reader :model, :name, :scope, :options, :reflection - def self.build(model, name, options) - new(model, name, options).build + def self.build(*args, &block) + new(*args, &block).build end - def initialize(model, name, options) - @model, @name, @options = model, name, options + def initialize(model, name, scope, options) + @model = model + @name = name + + if options + @scope = scope + @options = options + else + @scope = nil + @options = scope + end + + if @scope && @scope.arity == 0 + prev_scope = @scope + @scope = proc { instance_exec(&prev_scope) } + end end def mixin @model.generated_feature_methods end + include Module.new { def build; end } + def build validate_options - reflection = model.create_reflection(self.class.macro, name, options, model) define_accessors - reflection + @reflection = model.create_reflection(macro, name, scope, options, model) + super # provides an extension point + @reflection + end + + def macro + raise NotImplementedError + end + + def valid_options + Association.valid_options end private def validate_options - options.assert_valid_keys(self.class.valid_options) + options.assert_valid_keys(valid_options) end def define_accessors diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 4183c222de..4bef996297 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -2,9 +2,13 @@ require 'active_support/core_ext/object/inclusion' module ActiveRecord::Associations::Builder class BelongsTo < SingularAssociation #:nodoc: - self.macro = :belongs_to + def macro + :belongs_to + end - self.valid_options += [:foreign_type, :polymorphic, :touch] + def valid_options + super + [:foreign_type, :polymorphic, :touch] + end def constructable? !options[:polymorphic] diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb index 768f70b6c9..af81af4ad2 100644 --- a/activerecord/lib/active_record/associations/builder/collection_association.rb +++ b/activerecord/lib/active_record/associations/builder/collection_association.rb @@ -2,19 +2,14 @@ module ActiveRecord::Associations::Builder class CollectionAssociation < Association #:nodoc: CALLBACKS = [:before_add, :after_add, :before_remove, :after_remove] - self.valid_options += [ - :table_name, :order, :group, :having, :limit, :offset, :uniq, :finder_sql, - :counter_sql, :before_add, :after_add, :before_remove, :after_remove - ] - - attr_reader :block_extension - - def self.build(model, name, options, &extension) - new(model, name, options, &extension).build + def valid_options + super + [:table_name, :finder_sql, :counter_sql, :before_add, :after_add, :before_remove, :after_remove] end - def initialize(model, name, options, &extension) - super(model, name, options) + attr_reader :block_extension, :extension_module + + def initialize(*args, &extension) + super(*args) @block_extension = extension end @@ -32,18 +27,24 @@ module ActiveRecord::Associations::Builder private def wrap_block_extension - options[:extend] = Array(options[:extend]) - if block_extension + @extension_module = mod = Module.new(&block_extension) silence_warnings do - model.parent.const_set(extension_module_name, Module.new(&block_extension)) + model.parent.const_set(extension_module_name, mod) + end + + prev_scope = @scope + + if prev_scope + @scope = proc { |owner| instance_exec(owner, &prev_scope).extending(mod) } + else + @scope = proc { extending(mod) } end - options[:extend].push("#{model.parent}::#{extension_module_name}".constantize) end end def extension_module_name - @extension_module_name ||= "#{model.to_s.demodulize}#{name.to_s.camelize}AssociationExtension" + @extension_module_name ||= "#{model.name.demodulize}#{name.to_s.camelize}AssociationExtension" end def define_callback(callback_name) 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 f7656ecd47..7f79ef25f2 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 @@ -1,8 +1,12 @@ module ActiveRecord::Associations::Builder class HasAndBelongsToMany < CollectionAssociation #:nodoc: - self.macro = :has_and_belongs_to_many + def macro + :has_and_belongs_to_many + end - self.valid_options += [:join_table, :association_foreign_key, :delete_sql, :insert_sql] + def valid_options + super + [:join_table, :association_foreign_key, :delete_sql, :insert_sql] + end def build reflection = super diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb index d37d4e9d33..81df1fb135 100644 --- a/activerecord/lib/active_record/associations/builder/has_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_many.rb @@ -2,9 +2,13 @@ require 'active_support/core_ext/object/inclusion' module ActiveRecord::Associations::Builder class HasMany < CollectionAssociation #:nodoc: - self.macro = :has_many + def macro + :has_many + end - self.valid_options += [:primary_key, :dependent, :as, :through, :source, :source_type, :inverse_of] + def valid_options + super + [:primary_key, :dependent, :as, :through, :source, :source_type, :inverse_of] + end def build reflection = super diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb index bc8a212bee..cdb45e8e58 100644 --- a/activerecord/lib/active_record/associations/builder/has_one.rb +++ b/activerecord/lib/active_record/associations/builder/has_one.rb @@ -2,12 +2,15 @@ require 'active_support/core_ext/object/inclusion' module ActiveRecord::Associations::Builder class HasOne < SingularAssociation #:nodoc: - self.macro = :has_one - - self.valid_options += [:order, :as] + def macro + :has_one + end - class_attribute :through_options - self.through_options = [:through, :source, :source_type] + def valid_options + valid = super + [:order, :as] + valid += [:through, :source, :source_type] if options[:through] + valid + end def constructable? !options[:through] @@ -21,12 +24,6 @@ module ActiveRecord::Associations::Builder private - def validate_options - valid_options = self.class.valid_options - valid_options += self.class.through_options if options[:through] - options.assert_valid_keys(valid_options) - end - def configure_dependency if options[:dependent] unless options[:dependent].in?([:destroy, :delete, :nullify, :restrict]) diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb index 436b6c1524..90a4b7c2ef 100644 --- a/activerecord/lib/active_record/associations/builder/singular_association.rb +++ b/activerecord/lib/active_record/associations/builder/singular_association.rb @@ -1,6 +1,8 @@ module ActiveRecord::Associations::Builder class SingularAssociation < Association #:nodoc: - self.valid_options += [:remote, :dependent, :counter_cache, :primary_key, :inverse_of] + def valid_options + super + [:remote, :dependent, :counter_cache, :primary_key, :inverse_of] + end def constructable? true diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 2f6ddfeeb3..55628c81bb 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -183,7 +183,7 @@ module ActiveRecord reflection.klass.count_by_sql(custom_counter_sql) else - if options[:uniq] + if association_scope.uniq_value # This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL. column_name ||= reflection.klass.primary_key count_options[:distinct] = true @@ -246,14 +246,14 @@ module ActiveRecord # +count_records+, which is a method descendants have to provide. def size if !find_target? || loaded? - if options[:uniq] + if association_scope.uniq_value target.uniq.size else target.size end - elsif !loaded? && options[:group] + elsif !loaded? && !association_scope.group_values.empty? load_target.size - elsif !loaded? && !options[:uniq] && target.is_a?(Array) + elsif !loaded? && !association_scope.uniq_value && target.is_a?(Array) unsaved_records = target.select { |r| r.new_record? } unsaved_records.size + count_records else @@ -344,7 +344,7 @@ module ActiveRecord callback(:before_add, record) yield(record) if block_given? - if options[:uniq] && index = @target.index(record) + if association_scope.uniq_value && index = @target.index(record) @target[index] = record else @target << record diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index e631579087..669b7e03c2 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -46,7 +46,7 @@ module ActiveRecord # documented side-effect of the method that may avoid an extra SELECT. @target ||= [] and loaded! if count == 0 - [options[:limit], count].compact.min + [association_scope.limit_value, count].compact.min end def has_cached_counter?(reflection = reflection) 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 0d7d28e458..0d3b4dbab1 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -92,14 +92,21 @@ module ActiveRecord constraint = build_constraint(reflection, table, key, foreign_table, foreign_key) - conditions = self.conditions[i].dup - conditions << { reflection.type => foreign_klass.base_class.name } if reflection.type + scope_chain_items = scope_chain[i] - conditions.each 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) + if reflection.type + scope_chain_items += [ + ActiveRecord::Relation.new(reflection.klass, table) + .where(reflection.type => foreign_klass.base_class.name) + ] + end + + scope_chain_items.each do |item| + unless item.is_a?(Relation) + item = ActiveRecord::Relation.new(reflection.klass, table).instance_exec(self, &item) + end - constraint = constraint.and(condition) + constraint = constraint.and(item.arel.constraints) unless item.arel.constraints.empty? end relation.from(join(table, constraint)) @@ -137,18 +144,8 @@ module ActiveRecord table.table_alias || table.name end - def conditions - @conditions ||= reflection.conditions.reverse - end - - private - - def interpolate(conditions) - if conditions.respond_to?(:to_proc) - instance_eval(&conditions) - else - conditions - end + def scope_chain + @scope_chain ||= reflection.scope_chain.reverse end end diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index 54705e4950..ce5bf15f10 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -42,7 +42,7 @@ module ActiveRecord autoload :HasAndBelongsToMany, 'active_record/associations/preloader/has_and_belongs_to_many' autoload :BelongsTo, 'active_record/associations/preloader/belongs_to' - attr_reader :records, :associations, :options, :model + attr_reader :records, :associations, :preload_scope, :model # Eager loads the named associations for the given Active Record record(s). # @@ -78,15 +78,10 @@ module ActiveRecord # [ :books, :author ] # { :author => :avatar } # [ :books, { :author => :avatar } ] - # - # +options+ contains options that will be passed to ActiveRecord::Base#find - # (which is called under the hood for preloading records). But it is passed - # only one level deep in the +associations+ argument, i.e. it's not passed - # to the child associations when +associations+ is a Hash. - def initialize(records, associations, options = {}) - @records = Array.wrap(records).compact.uniq - @associations = Array.wrap(associations) - @options = options + def initialize(records, associations, preload_scope = nil) + @records = Array.wrap(records).compact.uniq + @associations = Array.wrap(associations) + @preload_scope = preload_scope || Relation.new(nil, nil) end def run @@ -110,7 +105,7 @@ module ActiveRecord def preload_hash(association) association.each do |parent, child| - Preloader.new(records, parent, options).run + Preloader.new(records, parent, preload_scope).run Preloader.new(records.map { |record| record.send(parent) }.flatten, child).run end end @@ -125,7 +120,7 @@ module ActiveRecord def preload_one(association) grouped_records(association).each do |reflection, klasses| klasses.each do |klass, records| - preloader_for(reflection).new(klass, records, reflection, options).run + preloader_for(reflection).new(klass, records, reflection, preload_scope).run end end end diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index b4c3908b10..e7fd72994f 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -2,16 +2,16 @@ module ActiveRecord module Associations class Preloader class Association #:nodoc: - attr_reader :owners, :reflection, :preload_options, :model, :klass - - def initialize(klass, owners, reflection, preload_options) - @klass = klass - @owners = owners - @reflection = reflection - @preload_options = preload_options || {} - @model = owners.first && owners.first.class - @scoped = nil - @owners_by_key = nil + attr_reader :owners, :reflection, :preload_scope, :model, :klass + + def initialize(klass, owners, reflection, preload_scope) + @klass = klass + @owners = owners + @reflection = reflection + @preload_scope = preload_scope + @model = owners.first && owners.first.class + @scoped = nil + @owners_by_key = nil end def run @@ -92,34 +92,29 @@ module ActiveRecord records_by_owner end + def reflection_scope + @reflection_scope ||= reflection.scope ? klass.unscoped.instance_exec(&reflection.scope) : klass.unscoped + end + def build_scope scope = klass.unscoped scope.default_scoped = true - scope = scope.where(interpolate(options[:conditions])) - scope = scope.where(interpolate(preload_options[:conditions])) + values = reflection_scope.values + preload_values = preload_scope.values - scope = scope.select(preload_options[:select] || options[:select] || table[Arel.star]) - scope = scope.includes(preload_options[:include] || options[:include]) + scope.where_values = Array(values[:where]) + Array(preload_values[:where]) + scope.references_values = Array(values[:references]) + Array(preload_values[:references]) + + scope.select! preload_values[:select] || values[:select] || table[Arel.star] + scope.includes! preload_values[:includes] || values[:includes] if options[:as] - scope = scope.where( - klass.table_name => { - reflection.type => model.base_class.sti_name - } - ) + scope.where!(klass.table_name => { reflection.type => model.base_class.sti_name }) end scope end - - def interpolate(conditions) - if conditions.respond_to?(:to_proc) - klass.send(:instance_eval, &conditions) - else - conditions - end - end end end end diff --git a/activerecord/lib/active_record/associations/preloader/collection_association.rb b/activerecord/lib/active_record/associations/preloader/collection_association.rb index c248aeaaf6..e6cd35e7a1 100644 --- a/activerecord/lib/active_record/associations/preloader/collection_association.rb +++ b/activerecord/lib/active_record/associations/preloader/collection_association.rb @@ -6,7 +6,7 @@ module ActiveRecord private def build_scope - super.order(preload_options[:order] || options[:order]) + super.order(preload_scope.values[:order] || reflection_scope.values[:order]) end def preload diff --git a/activerecord/lib/active_record/associations/preloader/has_many_through.rb b/activerecord/lib/active_record/associations/preloader/has_many_through.rb index c6e9ede356..9a662d3f53 100644 --- a/activerecord/lib/active_record/associations/preloader/has_many_through.rb +++ b/activerecord/lib/active_record/associations/preloader/has_many_through.rb @@ -6,7 +6,7 @@ module ActiveRecord def associated_records_by_owner super.each do |owner, records| - records.uniq! if options[:uniq] + records.uniq! if reflection_scope.uniq_value end end end diff --git a/activerecord/lib/active_record/associations/preloader/has_one.rb b/activerecord/lib/active_record/associations/preloader/has_one.rb index 848448bb48..24728e9f01 100644 --- a/activerecord/lib/active_record/associations/preloader/has_one.rb +++ b/activerecord/lib/active_record/associations/preloader/has_one.rb @@ -14,7 +14,7 @@ module ActiveRecord private def build_scope - super.order(preload_options[:order] || options[:order]) + super.order(preload_scope.values[:order] || reflection_scope.values[:order]) 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 ad6374d09a..1c1ba11c44 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -14,10 +14,7 @@ module ActiveRecord def associated_records_by_owner through_records = through_records_by_owner - ActiveRecord::Associations::Preloader.new( - through_records.values.flatten, - source_reflection.name, options - ).run + Preloader.new(through_records.values.flatten, source_reflection.name, reflection_scope).run through_records.each do |owner, records| records.map! { |r| r.send(source_reflection.name) }.flatten! @@ -28,10 +25,7 @@ module ActiveRecord private def through_records_by_owner - ActiveRecord::Associations::Preloader.new( - owners, through_reflection.name, - through_options - ).run + Preloader.new(owners, through_reflection.name, through_scope).run Hash[owners.map do |owner| through_records = Array.wrap(owner.send(through_reflection.name)) @@ -45,21 +39,22 @@ module ActiveRecord end] end - def through_options - through_options = {} + def through_scope + through_scope = through_reflection.klass.unscoped if options[:source_type] - through_options[:conditions] = { reflection.foreign_type => options[:source_type] } + through_scope.where! reflection.foreign_type => options[:source_type] else - if options[:conditions] - through_options[:include] = options[:include] || options[:source] - through_options[:conditions] = options[:conditions] + unless reflection_scope.where_values.empty? + through_scope.includes_values = reflection_scope.values[:includes] || options[:source] + through_scope.where_values = reflection_scope.values[:where] end - through_options[:order] = options[:order] + through_scope.order! reflection_scope.values[:order] + through_scope.references! reflection_scope.values[:references] end - through_options + through_scope end end end diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index d545e7799d..290f57659d 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -127,23 +127,17 @@ module ActiveRecord module AutosaveAssociation extend ActiveSupport::Concern - ASSOCIATION_TYPES = %w{ HasOne HasMany BelongsTo HasAndBelongsToMany } - module AssociationBuilderExtension #:nodoc: - def self.included(base) - base.valid_options << :autosave - end - def build - reflection = super model.send(:add_autosave_association_callbacks, reflection) - reflection + super end end included do - ASSOCIATION_TYPES.each do |type| - Associations::Builder.const_get(type).send(:include, AssociationBuilderExtension) + Associations::Builder::Association.class_eval do + self.valid_options << :autosave + include AssociationBuilderExtension end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index df4a9d5afc..2b0bc3f497 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -318,7 +318,7 @@ module ActiveRecord select_all(sql, 'SCHEMA').map { |table| table.delete('Table_type') sql = "SHOW CREATE TABLE #{quote_table_name(table.to_a.first.last)}" - exec_without_stmt(sql, 'SCHEMA').first['Create Table'] + ";\n\n" + exec_query(sql, 'SCHEMA').first['Create Table'] + ";\n\n" }.join end diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 0b6734b010..3b0353358a 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -215,7 +215,7 @@ module ActiveRecord def select_rows(sql, name = nil) @connection.query_with_result = true - rows = exec_without_stmt(sql, name).rows + rows = exec_query(sql, name).rows @connection.more_results && @connection.next_result # invoking stored procedures with CLIENT_MULTI_RESULTS requires this to tidy up else connection will be dropped rows end @@ -279,31 +279,171 @@ module ActiveRecord end def exec_query(sql, name = 'SQL', binds = []) - log(sql, name, binds) do - exec_stmt(sql, name, binds) do |cols, stmt| - ActiveRecord::Result.new(cols, stmt.to_a) if cols - end + # If the configuration sets prepared_statements:false, binds will + # always be empty, since the bind variables will have been already + # substituted and removed from binds by BindVisitor, so this will + # effectively disable prepared statement usage completely. + if binds.empty? + result_set, affected_rows = exec_without_stmt(sql, name) + else + result_set, affected_rows = exec_stmt(sql, name, binds) end + + yield affected_rows if block_given? + + result_set end def last_inserted_id(result) @connection.insert_id end + class Result < ActiveRecord::Result + def initialize(columns, rows, column_types) + super(columns, rows) + @column_types = column_types + end + end + + module Fields + class Type + def type; end + + def type_cast_for_write(value) + value + end + end + + class Identity < Type + def type_cast(value); value; end + end + + class Integer < Type + def type_cast(value) + return if value.nil? + + value.to_i rescue value ? 1 : 0 + end + end + + class Date < Type + def type; :date; end + + def type_cast(value) + return if value.nil? + + # FIXME: probably we can improve this since we know it is mysql + # specific + ConnectionAdapters::Column.value_to_date value + end + end + + class DateTime < Type + def type; :datetime; end + + def type_cast(value) + return if value.nil? + + # FIXME: probably we can improve this since we know it is mysql + # specific + ConnectionAdapters::Column.string_to_time value + end + end + + class Time < Type + def type; :time; end + + def type_cast(value) + return if value.nil? + + # FIXME: probably we can improve this since we know it is mysql + # specific + ConnectionAdapters::Column.string_to_dummy_time value + end + end + + class Float < Type + def type; :float; end + + def type_cast(value) + return if value.nil? + + value.to_f + end + end + + class Decimal < Type + def type_cast(value) + return if value.nil? + + ConnectionAdapters::Column.value_to_decimal value + end + end + + class Boolean < Type + def type_cast(value) + return if value.nil? + + ConnectionAdapters::Column.value_to_boolean value + end + end + + TYPES = {} + + # Register an MySQL +type_id+ with a typcasting object in + # +type+. + def self.register_type(type_id, type) + TYPES[type_id] = type + end + + def self.alias_type(new, old) + TYPES[new] = TYPES[old] + end + + register_type Mysql::Field::TYPE_TINY, Fields::Boolean.new + register_type Mysql::Field::TYPE_LONG, Fields::Integer.new + alias_type Mysql::Field::TYPE_LONGLONG, Mysql::Field::TYPE_LONG + alias_type Mysql::Field::TYPE_NEWDECIMAL, Mysql::Field::TYPE_LONG + + register_type Mysql::Field::TYPE_VAR_STRING, Fields::Identity.new + register_type Mysql::Field::TYPE_BLOB, Fields::Identity.new + register_type Mysql::Field::TYPE_DATE, Fields::Date.new + register_type Mysql::Field::TYPE_DATETIME, Fields::DateTime.new + register_type Mysql::Field::TYPE_TIME, Fields::Time.new + register_type Mysql::Field::TYPE_FLOAT, Fields::Float.new + + Mysql::Field.constants.grep(/TYPE/).map { |class_name| + Mysql::Field.const_get class_name + }.reject { |const| TYPES.key? const }.each do |const| + register_type const, Fields::Identity.new + end + end + def exec_without_stmt(sql, name = 'SQL') # :nodoc: # Some queries, like SHOW CREATE TABLE don't work through the prepared # statement API. For those queries, we need to use this method. :'( log(sql, name) do result = @connection.query(sql) - cols = [] - rows = [] + affected_rows = @connection.affected_rows if result - cols = result.fetch_fields.map { |field| field.name } - rows = result.to_a + types = {} + result.fetch_fields.each { |field| + if field.decimals > 0 + types[field.name] = Fields::Decimal.new + else + types[field.name] = Fields::TYPES.fetch(field.type) { + Fields::Identity.new + } + end + } + result_set = Result.new(types.keys, result.to_a, types) result.free + else + result_set = ActiveRecord::Result.new([], []) end - ActiveRecord::Result.new(cols, rows) + + [result_set, affected_rows] end end @@ -321,16 +461,18 @@ module ActiveRecord alias :create :insert_sql def exec_delete(sql, name, binds) - log(sql, name, binds) do - exec_stmt(sql, name, binds) do |cols, stmt| - stmt.affected_rows - end + affected_rows = 0 + + exec_query(sql, name, binds) do |n| + affected_rows = n end + + affected_rows end alias :exec_update :exec_delete def begin_db_transaction #:nodoc: - exec_without_stmt "BEGIN" + exec_query "BEGIN" rescue Mysql::Error # Transactions aren't supported end @@ -339,41 +481,44 @@ module ActiveRecord def exec_stmt(sql, name, binds) cache = {} - if binds.empty? - stmt = @connection.prepare(sql) - else - cache = @statements[sql] ||= { - :stmt => @connection.prepare(sql) - } - stmt = cache[:stmt] - end + log(sql, name, binds) do + if binds.empty? + stmt = @connection.prepare(sql) + else + cache = @statements[sql] ||= { + :stmt => @connection.prepare(sql) + } + stmt = cache[:stmt] + end - begin - stmt.execute(*binds.map { |col, val| type_cast(val, col) }) - rescue Mysql::Error => e - # Older versions of MySQL leave the prepared statement in a bad - # place when an error occurs. To support older mysql versions, we - # need to close the statement and delete the statement from the - # cache. - stmt.close - @statements.delete sql - raise e - end + begin + stmt.execute(*binds.map { |col, val| type_cast(val, col) }) + rescue Mysql::Error => e + # Older versions of MySQL leave the prepared statement in a bad + # place when an error occurs. To support older mysql versions, we + # need to close the statement and delete the statement from the + # cache. + stmt.close + @statements.delete sql + raise e + end - cols = nil - if metadata = stmt.result_metadata - cols = cache[:cols] ||= metadata.fetch_fields.map { |field| - field.name - } - end + cols = nil + if metadata = stmt.result_metadata + cols = cache[:cols] ||= metadata.fetch_fields.map { |field| + field.name + } + end - result = yield [cols, stmt] + result_set = ActiveRecord::Result.new(cols, stmt.to_a) if cols + affected_rows = stmt.affected_rows - stmt.result_metadata.free if cols - stmt.free_result - stmt.close if binds.empty? + stmt.result_metadata.free if cols + stmt.free_result + stmt.close if binds.empty? - result + [result_set, affected_rows] + end end def connect diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 7b263fd62d..a57a532ba2 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -1227,12 +1227,19 @@ module ActiveRecord end # Renames a table. + # Also renames a table's primary key sequence if the sequence name matches the + # Active Record default. # # Example: # rename_table('octopuses', 'octopi') def rename_table(name, new_name) clear_cache! execute "ALTER TABLE #{quote_table_name(name)} RENAME TO #{quote_table_name(new_name)}" + pk, seq = pk_and_sequence_for(new_name) + if seq == "#{name}_#{pk}_seq" + new_seq = "#{new_name}_#{pk}_seq" + execute "ALTER TABLE #{quote_table_name(seq)} RENAME TO #{quote_table_name(new_seq)}" + end end # Adds a new column to the named table. diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 90f156456e..803d68187c 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -380,7 +380,7 @@ module ActiveRecord # # So we can avoid the method_missing hit by explicitly defining #to_ary as nil here. # - # See also http://tenderlovemaking.com/2011/06/28/til-its-ok-to-return-nil-from-to_ary/ + # See also http://tenderlovemaking.com/2011/06/28/til-its-ok-to-return-nil-from-to_ary.html def to_ary # :nodoc: nil end diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 4ce42feb74..e0344f3c56 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -86,7 +86,7 @@ module ActiveRecord stmt = relation.where( relation.table[self.class.primary_key].eq(id).and( - relation.table[lock_col].eq(quote_value(previous_lock_value)) + relation.table[lock_col].eq(self.class.quote_value(previous_lock_value)) ) ).arel.compile_update(arel_attributes_with_values_for_update(attribute_names)) diff --git a/activerecord/lib/active_record/model.rb b/activerecord/lib/active_record/model.rb index 0015e3a567..fb3fb643ff 100644 --- a/activerecord/lib/active_record/model.rb +++ b/activerecord/lib/active_record/model.rb @@ -74,9 +74,9 @@ module ActiveRecord include Inheritance include Scoping include Sanitization - include Integration include AttributeAssignment include ActiveModel::Conversion + include Integration include Validations include CounterCache include Locking::Optimistic diff --git a/activerecord/lib/active_record/observer.rb b/activerecord/lib/active_record/observer.rb index fdf17c003c..e940d357da 100644 --- a/activerecord/lib/active_record/observer.rb +++ b/activerecord/lib/active_record/observer.rb @@ -74,6 +74,12 @@ module ActiveRecord # # Observers will not be invoked unless you define these in your application configuration. # + # If you are using Active Record outside Rails, activate the observers explicitly in a configuration or + # environment file: + # + # ActiveRecord::Base.add_observer CommentObserver.instance + # ActiveRecord::Base.add_observer SignupObserver.instance + # # == Loading # # Observers register themselves in the model class they observe, since it is the class that diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 0d9534acd6..3e8d9718f5 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -20,9 +20,9 @@ module ActiveRecord # MacroReflection class has info for the AssociationReflection # class. module ClassMethods - def create_reflection(macro, name, options, active_record) + def create_reflection(macro, name, scope, options, active_record) klass = options[:through] ? ThroughReflection : AssociationReflection - reflection = klass.new(macro, name, options, active_record) + reflection = klass.new(macro, name, scope, options, active_record) self.reflections = self.reflections.merge(name => reflection) reflection @@ -71,6 +71,8 @@ module ActiveRecord # <tt>has_many :clients</tt> returns <tt>:has_many</tt> attr_reader :macro + attr_reader :scope + # Returns the hash of options used for the macro. # # <tt>has_many :clients</tt> returns +{}+ @@ -80,9 +82,10 @@ module ActiveRecord attr_reader :plural_name # :nodoc: - def initialize(macro, name, options, active_record) + def initialize(macro, name, scope, options, active_record) @macro = macro @name = name + @scope = scope @options = options @active_record = active_record @plural_name = active_record.pluralize_table_names ? @@ -142,7 +145,7 @@ module ActiveRecord @klass ||= active_record.send(:compute_type, class_name) end - def initialize(macro, name, options, active_record) + def initialize(*args) super @collection = [:has_many, :has_and_belongs_to_many].include?(macro) end @@ -244,11 +247,10 @@ module ActiveRecord false 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 - [[options[:conditions]].compact] + # An array of arrays of scopes. Each item in the outside array corresponds to a reflection + # in the #chain. + def scope_chain + scope ? [[scope]] : [[]] end alias :source_macro :macro @@ -416,28 +418,25 @@ module ActiveRecord # has_many :tags # end # - # There may be conditions on Person.comment_tags, Article.comment_tags and/or Comment.tags, + # There may be scopes 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.map { |c| c.dup } + # of scopes corresponding to the chain. + def scope_chain + @scope_chain ||= begin + scope_chain = source_reflection.scope_chain.map(&:dup) - # Add to it the conditions from this reflection if necessary. - conditions.first << options[:conditions] if options[:conditions] + # Add to it the scope from this reflection (if any) + scope_chain.first << scope if scope - through_conditions = through_reflection.conditions + through_scope_chain = through_reflection.scope_chain if options[:source_type] - through_conditions.first << { foreign_type => options[:source_type] } + through_scope_chain.first << + through_reflection.klass.where(foreign_type => options[:source_type]) end # Recursively fill out the rest of the array from the through reflection - conditions += through_conditions - - conditions + scope_chain + through_scope_chain end end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index e401ed37b0..1271b74ead 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -51,7 +51,18 @@ module ActiveRecord # # allows you to access the +address+ attribute of the +User+ model without # firing an additional query. This will often result in a - # performance improvement over a simple +join+ + # performance improvement over a simple +join+. + # + # === conditions + # + # If you want to add conditions to your included models you'll have + # to explicitly reference them. For example: + # + # User.includes(:posts).where('posts.name = ?', 'example') + # + # Will throw an error, but this will work: + # + # User.includes(:posts).where('posts.name = ?', 'example').references(:posts) def includes(*args) args.empty? ? self : spawn.includes!(*args) end @@ -63,6 +74,12 @@ module ActiveRecord self end + # Forces eager loading by performing a LEFT OUTER JOIN on +args+: + # + # User.eager_load(:posts) + # => SELECT "users"."id" AS t0_r0, "users"."name" AS t0_r1, ... + # FROM "users" LEFT OUTER JOIN "posts" ON "posts"."user_id" = + # "users"."id" def eager_load(*args) args.blank? ? self : spawn.eager_load!(*args) end @@ -72,6 +89,10 @@ module ActiveRecord self end + # Allows preloading of +args+, in the same way that +includes+ does: + # + # User.preload(:posts) + # => SELECT "posts".* FROM "posts" WHERE "posts"."user_id" IN (1, 2, 3) def preload(*args) args.blank? ? self : spawn.preload!(*args) end @@ -147,7 +168,7 @@ module ActiveRecord # User.group(:name) # => SELECT "users".* FROM "users" GROUP BY name # - # Returns an array with distinct records based on the `group` attribute: + # Returns an array with distinct records based on the +group+ attribute: # # User.select([:id, :name]) # => [#<User id: 1, name: "Oscar">, #<User id: 2, name: "Oscar">, #<User id: 3, name: "Foo"> @@ -211,6 +232,10 @@ module ActiveRecord self end + # Performs a joins on +args+: + # + # User.joins(:posts) + # => SELECT "users".* FROM "users" INNER JOIN "posts" ON "posts"."user_id" = "users"."id" def joins(*args) args.compact.blank? ? self : spawn.joins!(*args) end @@ -334,6 +359,10 @@ module ActiveRecord self end + # Allows to specify a HAVING clause. Note that you can't use HAVING + # without also specifying a GROUP clause. + # + # Order.having('SUM(price) > 30').group('user_id') def having(opts, *rest) opts.blank? ? self : spawn.having!(opts, *rest) end @@ -375,6 +404,8 @@ module ActiveRecord self end + # Specifies locking settings (default to +true+). For more information + # on locking, please see +ActiveRecord::Locking+. def lock(locks = true) spawn.lock!(locks) end @@ -423,6 +454,12 @@ module ActiveRecord scoped.extending(NullRelation) end + # Sets readonly attributes for the returned relation. If value is + # true (default), attempting to update a record will result in an error. + # + # users = User.readonly + # users.first.save + # => ActiveRecord::ReadOnlyRecord: ActiveRecord::ReadOnlyRecord def readonly(value = true) spawn.readonly!(value) end @@ -529,7 +566,7 @@ module ActiveRecord def extending!(*modules, &block) modules << Module.new(&block) if block_given? - self.extending_values = modules.flatten + self.extending_values += modules.flatten extend(*extending_values) if extending_values.any? self @@ -552,7 +589,7 @@ module ActiveRecord end def build_arel - arel = table.from table + arel = Arel::SelectManager.new(table.engine, table) build_joins(arel, joins_values) unless joins_values.empty? diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index 46f6c283e3..ca767cc704 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -148,15 +148,8 @@ module ActiveRecord end # TODO: Deprecate this - def quoted_id #:nodoc: - quote_value(id, column_for_attribute(self.class.primary_key)) - end - - private - - # Quote strings appropriately for SQL statements. - def quote_value(value, column = nil) - self.class.connection.quote(value, column) + def quoted_id + self.class.quote_value(id, column_for_attribute(self.class.primary_key)) end end end diff --git a/activerecord/lib/active_record/schema_migration.rb b/activerecord/lib/active_record/schema_migration.rb index 236ec563d2..ca22154c84 100644 --- a/activerecord/lib/active_record/schema_migration.rb +++ b/activerecord/lib/active_record/schema_migration.rb @@ -7,7 +7,11 @@ module ActiveRecord attr_accessible :version def self.table_name - Base.table_name_prefix + 'schema_migrations' + Base.table_name_suffix + "#{Base.table_name_prefix}schema_migrations#{Base.table_name_suffix}" + end + + def self.index_name + "#{Base.table_name_prefix}unique_schema_migrations#{Base.table_name_suffix}" end def self.create_table @@ -15,14 +19,13 @@ module ActiveRecord connection.create_table(table_name, :id => false) do |t| t.column :version, :string, :null => false end - connection.add_index table_name, :version, :unique => true, - :name => "#{Base.table_name_prefix}unique_schema_migrations#{Base.table_name_suffix}" + connection.add_index table_name, :version, :unique => true, :name => index_name end end def self.drop_table if connection.table_exists?(table_name) - connection.remove_index table_name, :name => "#{Base.table_name_prefix}unique_schema_migrations#{Base.table_name_suffix}" + connection.remove_index table_name, :name => index_name connection.drop_table(table_name) end end diff --git a/activerecord/lib/active_record/session_store.rb b/activerecord/lib/active_record/session_store.rb index 5a256b040b..d2d3106721 100644 --- a/activerecord/lib/active_record/session_store.rb +++ b/activerecord/lib/active_record/session_store.rb @@ -7,7 +7,7 @@ module ActiveRecord # # The default assumes a +sessions+ tables with columns: # +id+ (numeric primary key), - # +session_id+ (text, or longtext if your session data exceeds 65K), and + # +session_id+ (string, usually varchar; maximum length is 255), and # +data+ (text or longtext; careful if your session data exceeds 65KB). # # The +session_id+ column should always be indexed for speedy lookups. @@ -218,7 +218,7 @@ module ActiveRecord # Look up a session by id and unmarshal its data if found. def find_by_session_id(session_id) - if record = connection.select_one("SELECT * FROM #{@@table_name} WHERE #{@@session_id_column}=#{connection.quote(session_id.to_s)}") + if record = connection.select_one("SELECT #{connection.quote_column_name(data_column)} AS data FROM #{@@table_name} WHERE #{connection.quote_column_name(@@session_id_column)}=#{connection.quote(session_id.to_s)}") new(:session_id => session_id, :marshaled_data => record['data']) end end diff --git a/activerecord/lib/active_record/store.rb b/activerecord/lib/active_record/store.rb index d836acf18f..81576e7cd3 100644 --- a/activerecord/lib/active_record/store.rb +++ b/activerecord/lib/active_record/store.rb @@ -57,8 +57,7 @@ module ActiveRecord keys = keys.flatten keys.each do |key| define_method("#{key}=") do |value| - initialize_store_attribute(store_attribute) - attribute = send(store_attribute) + attribute = initialize_store_attribute(store_attribute) if value != attribute[key] attribute[key] = value send :"#{store_attribute}_will_change!" @@ -66,8 +65,7 @@ module ActiveRecord end define_method(key) do - initialize_store_attribute(store_attribute) - send(store_attribute)[key] + initialize_store_attribute(store_attribute)[key] end end @@ -77,16 +75,12 @@ module ActiveRecord private def initialize_store_attribute(store_attribute) - case attribute = send(store_attribute) - when ActiveSupport::HashWithIndifferentAccess - # Already initialized. Do nothing. - when Hash - # Initialized as a Hash. Convert to indifferent access. - send :"#{store_attribute}=", attribute.with_indifferent_access - else - # Uninitialized. Set to an indifferent hash. - send :"#{store_attribute}=", ActiveSupport::HashWithIndifferentAccess.new + attribute = send(store_attribute) + unless attribute.is_a?(HashWithIndifferentAccess) + attribute = IndifferentCoder.as_indifferent_hash(attribute) + send :"#{store_attribute}=", attribute end + attribute end class IndifferentCoder @@ -109,7 +103,7 @@ module ActiveRecord def self.as_indifferent_hash(obj) case obj - when ActiveSupport::HashWithIndifferentAccess + when HashWithIndifferentAccess obj when Hash obj.with_indifferent_access diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index 9e4b588ac2..5a24135f8e 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -26,7 +26,7 @@ module ActiveRecord relation = relation.and(table[finder_class.primary_key.to_sym].not_eq(record.send(:id))) if record.persisted? Array(options[:scope]).each do |scope_item| - scope_value = record.send(scope_item) + scope_value = record.read_attribute(scope_item) reflection = record.class.reflect_on_association(scope_item) if reflection scope_value = record.send(reflection.foreign_key) diff --git a/activerecord/test/cases/adapters/mysql/connection_test.rb b/activerecord/test/cases/adapters/mysql/connection_test.rb index 5e1c52c9ba..c3f82bc63d 100644 --- a/activerecord/test/cases/adapters/mysql/connection_test.rb +++ b/activerecord/test/cases/adapters/mysql/connection_test.rb @@ -70,11 +70,14 @@ class MysqlConnectionTest < ActiveRecord::TestCase assert_equal %w{ id data }, result.columns @connection.exec_query('INSERT INTO ex (id, data) VALUES (1, "foo")') + + # if there are no bind parameters, it will return a string (due to + # the libmysql api) result = @connection.exec_query('SELECT id, data FROM ex') assert_equal 1, result.rows.length assert_equal 2, result.columns.length - assert_equal [[1, 'foo']], result.rows + assert_equal [['1', 'foo']], result.rows end def test_exec_with_binds diff --git a/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb index 475a292f85..ddfe42b375 100644 --- a/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb +++ b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb @@ -26,7 +26,9 @@ module ActiveRecord result = @conn.exec_query('SELECT number FROM ex WHERE number = 10') assert_equal 1, result.rows.length - assert_equal 10, result.rows.last.last + # if there are no bind parameters, it will return a string (due to + # the libmysql api) + assert_equal '10', result.rows.last.last end def test_exec_insert_string diff --git a/activerecord/test/cases/associations/association_scope_test.rb b/activerecord/test/cases/associations/association_scope_test.rb new file mode 100644 index 0000000000..d38648202e --- /dev/null +++ b/activerecord/test/cases/associations/association_scope_test.rb @@ -0,0 +1,15 @@ +require 'cases/helper' +require 'models/post' +require 'models/author' + +module ActiveRecord + module Associations + class AssociationScopeTest < ActiveRecord::TestCase + test 'does not duplicate conditions' do + association_scope = AssociationScope.new(Author.new.association(:welcome_posts)) + wheres = association_scope.scope.where_values.map(&:right) + assert_equal wheres.uniq, wheres + end + end + end +end diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 2c7a240915..2dd8c78eab 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -181,8 +181,8 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase end def test_with_select - assert_equal Company.find(2).firm_with_select.attributes.size, 1 - assert_equal Company.scoped(:includes => :firm_with_select ).find(2).firm_with_select.attributes.size, 1 + assert_equal 1, Company.find(2).firm_with_select.attributes.size + assert_equal 1, Company.scoped(:includes => :firm_with_select ).find(2).firm_with_select.attributes.size end def test_belongs_to_counter diff --git a/activerecord/test/cases/associations/extension_test.rb b/activerecord/test/cases/associations/extension_test.rb index d7c489c2b5..917fe6cf52 100644 --- a/activerecord/test/cases/associations/extension_test.rb +++ b/activerecord/test/cases/associations/extension_test.rb @@ -70,8 +70,8 @@ class AssociationsExtensionsTest < ActiveRecord::TestCase private def extension_name(model) - builder = ActiveRecord::Associations::Builder::HasMany.new(model, :association_name, {}) { } + builder = ActiveRecord::Associations::Builder::HasMany.new(model, :association_name, nil, {}) { } builder.send(:wrap_block_extension) - builder.options[:extend].first.name + builder.extension_module.name end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 3ea6201d60..bd850ce690 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1467,7 +1467,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_defining_has_many_association_with_delete_all_dependency_lazily_evaluates_target_class ActiveRecord::Reflection::AssociationReflection.any_instance.expects(:class_name).never - class_eval <<-EOF + class_eval(<<-EOF, __FILE__, __LINE__ + 1) class DeleteAllModel < ActiveRecord::Base has_many :nonentities, :dependent => :delete_all end @@ -1476,7 +1476,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_defining_has_many_association_with_nullify_dependency_lazily_evaluates_target_class ActiveRecord::Reflection::AssociationReflection.any_instance.expects(:class_name).never - class_eval <<-EOF + class_eval(<<-EOF, __FILE__, __LINE__ + 1) class NullifyModel < ActiveRecord::Base has_many :nonentities, :dependent => :nullify end @@ -1638,4 +1638,11 @@ class HasManyAssociationsTest < ActiveRecord::TestCase post.taggings_with_delete_all.delete_all end end + + test "association using a scope block" do + author = authors(:david) + + assert author.posts.size > 1 + assert_equal author.posts.order('posts.id').limit(1), author.posts_with_scope_block + end end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index fe385feb4a..aa0cdf5dfb 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -816,7 +816,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase end def privatize(method_signature) - @target.class_eval <<-private_method + @target.class_eval(<<-private_method, __FILE__, __LINE__ + 1) private def #{method_signature} "I'm private" diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 8ef3bfef15..b980dc58e3 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -20,22 +20,6 @@ require 'models/company' require 'models/eye' class TestAutosaveAssociationsInGeneral < ActiveRecord::TestCase - def test_autosave_should_be_a_valid_option_for_has_one - assert ActiveRecord::Associations::Builder::HasOne.valid_options.include?(:autosave) - end - - def test_autosave_should_be_a_valid_option_for_belongs_to - assert ActiveRecord::Associations::Builder::BelongsTo.valid_options.include?(:autosave) - end - - def test_autosave_should_be_a_valid_option_for_has_many - assert ActiveRecord::Associations::Builder::HasMany.valid_options.include?(:autosave) - end - - def test_autosave_should_be_a_valid_option_for_has_and_belongs_to_many - assert ActiveRecord::Associations::Builder::HasAndBelongsToMany.valid_options.include?(:autosave) - end - def test_should_not_add_the_same_callbacks_multiple_times_for_has_one assert_no_difference_when_adding_callbacks_twice_for Pirate, :ship end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index e34f505a02..e4ba1c62c9 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1684,6 +1684,12 @@ class BasicsTest < ActiveRecord::TestCase assert_kind_of String, Client.first.to_param end + def test_to_param_returns_id_even_if_not_persisted + client = Client.new + client.id = 1 + assert_equal "1", client.to_param + end + def test_inspect_class assert_equal 'ActiveRecord::Base', ActiveRecord::Base.inspect assert_equal 'LoosePerson(abstract)', LoosePerson.inspect diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index e1c1e449ef..43c034703d 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -433,7 +433,7 @@ class CalculationsTest < ActiveRecord::TestCase Company.create!(:name => "test", :contracts => [Contract.new(:developer_id => 7)]) # TODO: Investigate why PG isn't being typecast - if current_adapter?(:PostgreSQLAdapter) + if current_adapter?(:PostgreSQLAdapter) || current_adapter?(:MysqlAdapter) assert_equal "7", Company.includes(:contracts).maximum(:developer_id) else assert_equal 7, Company.includes(:contracts).maximum(:developer_id) @@ -444,7 +444,7 @@ class CalculationsTest < ActiveRecord::TestCase Company.create!(:name => "test", :contracts => [Contract.new(:developer_id => 7)]) # TODO: Investigate why PG isn't being typecast - if current_adapter?(:PostgreSQLAdapter) + if current_adapter?(:PostgreSQLAdapter) || current_adapter?(:MysqlAdapter) assert_equal "7", Company.includes(:contracts).minimum(:developer_id) else assert_equal 7, Company.includes(:contracts).minimum(:developer_id) diff --git a/activerecord/test/cases/migration/rename_table_test.rb b/activerecord/test/cases/migration/rename_table_test.rb index d5ff2c607f..21901bec3c 100644 --- a/activerecord/test/cases/migration/rename_table_test.rb +++ b/activerecord/test/cases/migration/rename_table_test.rb @@ -14,6 +14,11 @@ module ActiveRecord remove_column 'test_models', :updated_at end + def teardown + rename_table :octopi, :test_models if connection.table_exists? :octopi + super + end + def test_rename_table_for_sqlite_should_work_with_reserved_words renamed = false @@ -26,8 +31,7 @@ module ActiveRecord renamed = true # Using explicit id in insert for compatibility across all databases - con = connection - con.execute "INSERT INTO 'references' (url, created_at, updated_at) VALUES ('http://rubyonrails.com', 0, 0)" + connection.execute "INSERT INTO 'references' (url, created_at, updated_at) VALUES ('http://rubyonrails.com', 0, 0)" assert_equal 'http://rubyonrails.com', connection.select_value("SELECT url FROM 'references' WHERE id=1") ensure return unless renamed @@ -39,16 +43,13 @@ module ActiveRecord rename_table :test_models, :octopi # Using explicit id in insert for compatibility across all databases - con = connection - con.enable_identity_insert("octopi", true) if current_adapter?(:SybaseAdapter) + connection.enable_identity_insert("octopi", true) if current_adapter?(:SybaseAdapter) - con.execute "INSERT INTO octopi (#{con.quote_column_name('id')}, #{con.quote_column_name('url')}) VALUES (1, 'http://www.foreverflying.com/octopus-black7.jpg')" + connection.execute "INSERT INTO octopi (#{connection.quote_column_name('id')}, #{connection.quote_column_name('url')}) VALUES (1, 'http://www.foreverflying.com/octopus-black7.jpg')" - con.enable_identity_insert("octopi", false) if current_adapter?(:SybaseAdapter) + connection.enable_identity_insert("octopi", false) if current_adapter?(:SybaseAdapter) assert_equal 'http://www.foreverflying.com/octopus-black7.jpg', connection.select_value("SELECT url FROM octopi WHERE id=1") - - rename_table :octopi, :test_models end def test_rename_table_with_an_index @@ -57,15 +58,22 @@ module ActiveRecord rename_table :test_models, :octopi # Using explicit id in insert for compatibility across all databases - con = ActiveRecord::Base.connection - con.enable_identity_insert("octopi", true) if current_adapter?(:SybaseAdapter) - con.execute "INSERT INTO octopi (#{con.quote_column_name('id')}, #{con.quote_column_name('url')}) VALUES (1, 'http://www.foreverflying.com/octopus-black7.jpg')" - con.enable_identity_insert("octopi", false) if current_adapter?(:SybaseAdapter) + connection.enable_identity_insert("octopi", true) if current_adapter?(:SybaseAdapter) + connection.execute "INSERT INTO octopi (#{connection.quote_column_name('id')}, #{connection.quote_column_name('url')}) VALUES (1, 'http://www.foreverflying.com/octopus-black7.jpg')" + connection.enable_identity_insert("octopi", false) if current_adapter?(:SybaseAdapter) assert_equal 'http://www.foreverflying.com/octopus-black7.jpg', connection.select_value("SELECT url FROM octopi WHERE id=1") assert connection.indexes(:octopi).first.columns.include?("url") + end + + def test_rename_table_for_postgresql_should_also_rename_default_sequence + skip 'not supported' unless current_adapter?(:PostgreSQLAdapter) + + rename_table :test_models, :octopi + + pk, seq = connection.pk_and_sequence_for('octopi') - rename_table :octopi, :test_models + assert_equal "octopi_#{pk}_seq", seq end end end diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index 0153e74604..83e207a260 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -155,7 +155,7 @@ class QueryCacheTest < ActiveRecord::TestCase # Oracle adapter returns count() as Fixnum or Float if current_adapter?(:OracleAdapter) assert_kind_of Numeric, Task.connection.select_value("SELECT count(*) AS count_all FROM tasks") - elsif current_adapter?(:SQLite3Adapter) || current_adapter?(:Mysql2Adapter) || current_adapter?(:MysqlAdapter) + elsif current_adapter?(:SQLite3Adapter) || current_adapter?(:Mysql2Adapter) # Future versions of the sqlite3 adapter will return numeric assert_instance_of Fixnum, Task.connection.select_value("SELECT count(*) AS count_all FROM tasks") diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index 6631dce08f..67e0c155c8 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -77,7 +77,7 @@ class ReflectionTest < ActiveRecord::TestCase end def test_reflection_klass_for_nested_class_name - reflection = MacroReflection.new(:company, nil, { :class_name => 'MyApplication::Business::Company' }, ActiveRecord::Base) + reflection = MacroReflection.new(:company, nil, nil, { :class_name => 'MyApplication::Business::Company' }, ActiveRecord::Base) assert_nothing_raised do assert_equal MyApplication::Business::Company, reflection.klass end @@ -93,7 +93,7 @@ class ReflectionTest < ActiveRecord::TestCase end def test_has_many_reflection - reflection_for_clients = AssociationReflection.new(:has_many, :clients, { :order => "id", :dependent => :destroy }, Firm) + reflection_for_clients = AssociationReflection.new(:has_many, :clients, nil, { :order => "id", :dependent => :destroy }, Firm) assert_equal reflection_for_clients, Firm.reflect_on_association(:clients) @@ -105,7 +105,7 @@ class ReflectionTest < ActiveRecord::TestCase end def test_has_one_reflection - reflection_for_account = AssociationReflection.new(:has_one, :account, { :foreign_key => "firm_id", :dependent => :destroy }, Firm) + reflection_for_account = AssociationReflection.new(:has_one, :account, nil, { :foreign_key => "firm_id", :dependent => :destroy }, Firm) assert_equal reflection_for_account, Firm.reflect_on_association(:account) assert_equal Account, Firm.reflect_on_association(:account).klass @@ -190,21 +190,25 @@ class ReflectionTest < ActiveRecord::TestCase assert_equal expected, actual end - def test_conditions + def test_scope_chain expected = [ - [{ :tags => { :name => 'Blue' } }], - [{ :taggings => { :comment => 'first' } }], - [{ :posts => { :title => ['misc post by bob', 'misc post by mary'] } }] + [Tagging.reflect_on_association(:tag).scope, Post.reflect_on_association(:first_blue_tags).scope], + [Post.reflect_on_association(:first_taggings).scope], + [Author.reflect_on_association(:misc_posts).scope] ] - actual = Author.reflect_on_association(:misc_post_first_blue_tags).conditions + actual = Author.reflect_on_association(:misc_post_first_blue_tags).scope_chain assert_equal expected, actual expected = [ - [{ :tags => { :name => 'Blue' } }, { :taggings => { :comment => 'first' } }, { :posts => { :title => ['misc post by bob', 'misc post by mary'] } }], + [ + Tagging.reflect_on_association(:blue_tag).scope, + Post.reflect_on_association(:first_blue_tags_2).scope, + Author.reflect_on_association(:misc_post_first_blue_tags_2).scope + ], [], [] ] - actual = Author.reflect_on_association(:misc_post_first_blue_tags_2).conditions + actual = Author.reflect_on_association(:misc_post_first_blue_tags_2).scope_chain assert_equal expected, actual end @@ -230,10 +234,10 @@ class ReflectionTest < ActiveRecord::TestCase end def test_association_primary_key_raises_when_missing_primary_key - reflection = ActiveRecord::Reflection::AssociationReflection.new(:fuu, :edge, {}, Author) + reflection = ActiveRecord::Reflection::AssociationReflection.new(:fuu, :edge, nil, {}, Author) assert_raises(ActiveRecord::UnknownPrimaryKey) { reflection.association_primary_key } - through = ActiveRecord::Reflection::ThroughReflection.new(:fuu, :edge, {}, Author) + through = ActiveRecord::Reflection::ThroughReflection.new(:fuu, :edge, nil, {}, Author) through.stubs(:source_reflection).returns(stub_everything(:options => {}, :class_name => 'Edge')) assert_raises(ActiveRecord::UnknownPrimaryKey) { through.association_primary_key } end @@ -244,7 +248,7 @@ class ReflectionTest < ActiveRecord::TestCase end def test_active_record_primary_key_raises_when_missing_primary_key - reflection = ActiveRecord::Reflection::AssociationReflection.new(:fuu, :author, {}, Edge) + reflection = ActiveRecord::Reflection::AssociationReflection.new(:fuu, :author, nil, {}, Edge) assert_raises(ActiveRecord::UnknownPrimaryKey) { reflection.active_record_primary_key } end @@ -262,32 +266,32 @@ class ReflectionTest < ActiveRecord::TestCase end def test_default_association_validation - assert AssociationReflection.new(:has_many, :clients, {}, Firm).validate? + assert AssociationReflection.new(:has_many, :clients, nil, {}, Firm).validate? - assert !AssociationReflection.new(:has_one, :client, {}, Firm).validate? - assert !AssociationReflection.new(:belongs_to, :client, {}, Firm).validate? - assert !AssociationReflection.new(:has_and_belongs_to_many, :clients, {}, Firm).validate? + assert !AssociationReflection.new(:has_one, :client, nil, {}, Firm).validate? + assert !AssociationReflection.new(:belongs_to, :client, nil, {}, Firm).validate? + assert !AssociationReflection.new(:has_and_belongs_to_many, :clients, nil, {}, Firm).validate? end def test_always_validate_association_if_explicit - assert AssociationReflection.new(:has_one, :client, { :validate => true }, Firm).validate? - assert AssociationReflection.new(:belongs_to, :client, { :validate => true }, Firm).validate? - assert AssociationReflection.new(:has_many, :clients, { :validate => true }, Firm).validate? - assert AssociationReflection.new(:has_and_belongs_to_many, :clients, { :validate => true }, Firm).validate? + assert AssociationReflection.new(:has_one, :client, nil, { :validate => true }, Firm).validate? + assert AssociationReflection.new(:belongs_to, :client, nil, { :validate => true }, Firm).validate? + assert AssociationReflection.new(:has_many, :clients, nil, { :validate => true }, Firm).validate? + assert AssociationReflection.new(:has_and_belongs_to_many, :clients, nil, { :validate => true }, Firm).validate? end def test_validate_association_if_autosave - assert AssociationReflection.new(:has_one, :client, { :autosave => true }, Firm).validate? - assert AssociationReflection.new(:belongs_to, :client, { :autosave => true }, Firm).validate? - assert AssociationReflection.new(:has_many, :clients, { :autosave => true }, Firm).validate? - assert AssociationReflection.new(:has_and_belongs_to_many, :clients, { :autosave => true }, Firm).validate? + assert AssociationReflection.new(:has_one, :client, nil, { :autosave => true }, Firm).validate? + assert AssociationReflection.new(:belongs_to, :client, nil, { :autosave => true }, Firm).validate? + assert AssociationReflection.new(:has_many, :clients, nil, { :autosave => true }, Firm).validate? + assert AssociationReflection.new(:has_and_belongs_to_many, :clients, nil, { :autosave => true }, Firm).validate? end def test_never_validate_association_if_explicit - assert !AssociationReflection.new(:has_one, :client, { :autosave => true, :validate => false }, Firm).validate? - assert !AssociationReflection.new(:belongs_to, :client, { :autosave => true, :validate => false }, Firm).validate? - assert !AssociationReflection.new(:has_many, :clients, { :autosave => true, :validate => false }, Firm).validate? - assert !AssociationReflection.new(:has_and_belongs_to_many, :clients, { :autosave => true, :validate => false }, Firm).validate? + assert !AssociationReflection.new(:has_one, :client, nil, { :autosave => true, :validate => false }, Firm).validate? + assert !AssociationReflection.new(:belongs_to, :client, nil, { :autosave => true, :validate => false }, Firm).validate? + assert !AssociationReflection.new(:has_many, :clients, nil, { :autosave => true, :validate => false }, Firm).validate? + assert !AssociationReflection.new(:has_and_belongs_to_many, :clients, nil, { :autosave => true, :validate => false }, Firm).validate? end def test_foreign_key @@ -295,10 +299,10 @@ class ReflectionTest < ActiveRecord::TestCase assert_equal "category_id", Post.reflect_on_association(:categorizations).foreign_key.to_s end - def test_through_reflection_conditions_do_not_modify_other_reflections - orig_conds = Post.reflect_on_association(:first_blue_tags_2).conditions.inspect - Author.reflect_on_association(:misc_post_first_blue_tags_2).conditions - assert_equal orig_conds, Post.reflect_on_association(:first_blue_tags_2).conditions.inspect + def test_through_reflection_scope_chain_does_not_modify_other_reflections + orig_conds = Post.reflect_on_association(:first_blue_tags_2).scope_chain.inspect + Author.reflect_on_association(:misc_post_first_blue_tags_2).scope_chain + assert_equal orig_conds, Post.reflect_on_association(:first_blue_tags_2).scope_chain.inspect end def test_symbol_for_class_name @@ -309,11 +313,11 @@ class ReflectionTest < ActiveRecord::TestCase category = Struct.new(:table_name, :pluralize_table_names).new('categories', true) product = Struct.new(:table_name, :pluralize_table_names).new('products', true) - reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, {}, product) + reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, nil, {}, product) reflection.stubs(:klass).returns(category) assert_equal 'categories_products', reflection.join_table - reflection = AssociationReflection.new(:has_and_belongs_to_many, :products, {}, category) + reflection = AssociationReflection.new(:has_and_belongs_to_many, :products, nil, {}, category) reflection.stubs(:klass).returns(product) assert_equal 'categories_products', reflection.join_table end @@ -322,11 +326,11 @@ class ReflectionTest < ActiveRecord::TestCase category = Struct.new(:table_name, :pluralize_table_names).new('catalog_categories', true) product = Struct.new(:table_name, :pluralize_table_names).new('catalog_products', true) - reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, {}, product) + reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, nil, {}, product) reflection.stubs(:klass).returns(category) assert_equal 'catalog_categories_products', reflection.join_table - reflection = AssociationReflection.new(:has_and_belongs_to_many, :products, {}, category) + reflection = AssociationReflection.new(:has_and_belongs_to_many, :products, nil, {}, category) reflection.stubs(:klass).returns(product) assert_equal 'catalog_categories_products', reflection.join_table end @@ -335,11 +339,11 @@ class ReflectionTest < ActiveRecord::TestCase category = Struct.new(:table_name, :pluralize_table_names).new('catalog_categories', true) page = Struct.new(:table_name, :pluralize_table_names).new('content_pages', true) - reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, {}, page) + reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, nil, {}, page) reflection.stubs(:klass).returns(category) assert_equal 'catalog_categories_content_pages', reflection.join_table - reflection = AssociationReflection.new(:has_and_belongs_to_many, :pages, {}, category) + reflection = AssociationReflection.new(:has_and_belongs_to_many, :pages, nil, {}, category) reflection.stubs(:klass).returns(page) assert_equal 'catalog_categories_content_pages', reflection.join_table end @@ -348,11 +352,11 @@ class ReflectionTest < ActiveRecord::TestCase category = Struct.new(:table_name, :pluralize_table_names).new('categories', true) product = Struct.new(:table_name, :pluralize_table_names).new('products', true) - reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, { :join_table => 'product_categories' }, product) + reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, nil, { :join_table => 'product_categories' }, product) reflection.stubs(:klass).returns(category) assert_equal 'product_categories', reflection.join_table - reflection = AssociationReflection.new(:has_and_belongs_to_many, :products, { :join_table => 'product_categories' }, category) + reflection = AssociationReflection.new(:has_and_belongs_to_many, :products, nil, { :join_table => 'product_categories' }, category) reflection.stubs(:klass).returns(product) assert_equal 'product_categories', reflection.join_table end diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 89f818a689..034339e413 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -191,11 +191,14 @@ module ActiveRecord end test 'extending!' do - mod = Module.new + mod, mod2 = Module.new, Module.new assert relation.extending!(mod).equal?(relation) - assert [mod], relation.extending_values + assert_equal [mod], relation.extending_values assert relation.is_a?(mod) + + relation.extending!(mod2) + assert_equal [mod, mod2], relation.extending_values end test 'extending! with empty args' do diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 8713b8d5e4..c8da7ddd99 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1332,4 +1332,14 @@ class RelationTest < ActiveRecord::TestCase assert_equal expected, relation.inspect end end + + test 'using a custom table affects the wheres' do + table_alias = Post.arel_table.alias('omg_posts') + + relation = ActiveRecord::Relation.new Post, table_alias + relation.where!(:foo => "bar") + + node = relation.arel.constraints.first.grep(Arel::Attributes::Attribute).first + assert_equal table_alias, node.relation + end end diff --git a/activerecord/test/cases/session_store/sql_bypass_test.rb b/activerecord/test/cases/session_store/sql_bypass_test.rb index 6749d4ce98..b8cf4cf2cc 100644 --- a/activerecord/test/cases/session_store/sql_bypass_test.rb +++ b/activerecord/test/cases/session_store/sql_bypass_test.rb @@ -56,6 +56,20 @@ module ActiveRecord s.destroy assert_nil SqlBypass.find_by_session_id session_id end + + def test_data_column + SqlBypass.drop_table! if exists = Session.table_exists? + old, SqlBypass.data_column = SqlBypass.data_column, 'foo' + SqlBypass.create_table! + + session_id = 20 + SqlBypass.new(:data => 'hello', :session_id => session_id).save + assert_equal 'hello', SqlBypass.find_by_session_id(session_id).data + ensure + SqlBypass.drop_table! + SqlBypass.data_column = old + SqlBypass.create_table! if exists + end end end end diff --git a/activerecord/test/cases/validations/uniqueness_validation_test.rb b/activerecord/test/cases/validations/uniqueness_validation_test.rb index c173ee9a15..ea5e055289 100644 --- a/activerecord/test/cases/validations/uniqueness_validation_test.rb +++ b/activerecord/test/cases/validations/uniqueness_validation_test.rb @@ -22,6 +22,14 @@ end class Thaumaturgist < IneptWizard end +class ReplyTitle; end + +class ReplyWithTitleObject < Reply + validates_uniqueness_of :content, :scope => :title + + def title; ReplyTitle.new; end +end + class UniquenessValidationTest < ActiveRecord::TestCase fixtures :topics, 'warehouse-things', :developers @@ -104,6 +112,14 @@ class UniquenessValidationTest < ActiveRecord::TestCase assert !r2.valid?, "Saving r2 first time" end + def test_validate_uniqueness_with_composed_attribute_scope + r1 = ReplyWithTitleObject.create "title" => "r1", "content" => "hello world" + assert r1.valid?, "Saving r1" + + r2 = ReplyWithTitleObject.create "title" => "r1", "content" => "hello world" + assert !r2.valid?, "Saving r2 first time" + end + def test_validate_uniqueness_with_object_arg Reply.validates_uniqueness_of(:topic) diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index 14444a0092..6017178289 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -26,6 +26,8 @@ class Author < ActiveRecord::Base has_many :comments_with_order_and_conditions, :through => :posts, :source => :comments, :order => 'comments.body', :conditions => "comments.body like 'Thank%'" has_many :comments_with_include, :through => :posts, :source => :comments, :include => :post + has_many :posts_with_scope_block, -> { order('posts.id').limit(1) }, :class_name => "Post" + has_many :first_posts has_many :comments_on_first_posts, :through => :first_posts, :source => :comments, :order => 'posts.id desc, comments.id asc' diff --git a/activesupport/lib/active_support/core_ext/date.rb b/activesupport/lib/active_support/core_ext/date.rb new file mode 100644 index 0000000000..465fedda80 --- /dev/null +++ b/activesupport/lib/active_support/core_ext/date.rb @@ -0,0 +1,5 @@ +require 'active_support/core_ext/date/acts_like' +require 'active_support/core_ext/date/calculations' +require 'active_support/core_ext/date/conversions' +require 'active_support/core_ext/date/zones' + diff --git a/activesupport/lib/active_support/core_ext/date_time.rb b/activesupport/lib/active_support/core_ext/date_time.rb new file mode 100644 index 0000000000..e8a27b9f38 --- /dev/null +++ b/activesupport/lib/active_support/core_ext/date_time.rb @@ -0,0 +1,4 @@ +require 'active_support/core_ext/date_time/acts_like' +require 'active_support/core_ext/date_time/calculations' +require 'active_support/core_ext/date_time/conversions' +require 'active_support/core_ext/date_time/zones' diff --git a/activesupport/lib/active_support/core_ext/time.rb b/activesupport/lib/active_support/core_ext/time.rb new file mode 100644 index 0000000000..32cffe237d --- /dev/null +++ b/activesupport/lib/active_support/core_ext/time.rb @@ -0,0 +1,5 @@ +require 'active_support/core_ext/time/acts_like' +require 'active_support/core_ext/time/calculations' +require 'active_support/core_ext/time/conversions' +require 'active_support/core_ext/time/marshal' +require 'active_support/core_ext/time/zones' diff --git a/activesupport/lib/active_support/core_ext/time/calculations.rb b/activesupport/lib/active_support/core_ext/time/calculations.rb index 0a71fc117c..d0f574f2ba 100644 --- a/activesupport/lib/active_support/core_ext/time/calculations.rb +++ b/activesupport/lib/active_support/core_ext/time/calculations.rb @@ -1,6 +1,7 @@ require 'active_support/duration' require 'active_support/core_ext/time/conversions' require 'active_support/time_with_zone' +require 'active_support/core_ext/time/zones' class Time COMMON_YEAR_DAYS_IN_MONTH = [nil, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31] diff --git a/activesupport/lib/active_support/core_ext/time/zones.rb b/activesupport/lib/active_support/core_ext/time/zones.rb index e48866abe3..37bc3fae24 100644 --- a/activesupport/lib/active_support/core_ext/time/zones.rb +++ b/activesupport/lib/active_support/core_ext/time/zones.rb @@ -1,4 +1,3 @@ -require 'active_support/core_ext/time/calculations' require 'active_support/time_with_zone' class Time @@ -27,11 +26,11 @@ class Time # around_filter :set_time_zone # # def set_time_zone - # old_time_zone = Time.zone - # Time.zone = current_user.time_zone if logged_in? - # yield - # ensure - # Time.zone = old_time_zone + # if logged_in? + # Time.use_zone(current_user.time_zone) { yield } + # else + # yield + # end # end # end def zone=(time_zone) diff --git a/activesupport/test/multibyte_chars_test.rb b/activesupport/test/multibyte_chars_test.rb index a8d69d0ec3..ef289692bc 100644 --- a/activesupport/test/multibyte_chars_test.rb +++ b/activesupport/test/multibyte_chars_test.rb @@ -110,7 +110,7 @@ class MultibyteCharsUTF8BehaviourTest < ActiveSupport::TestCase end %w{capitalize downcase lstrip reverse rstrip swapcase upcase}.each do |method| - class_eval(<<-EOTESTS) + class_eval(<<-EOTESTS, __FILE__, __LINE__ + 1) def test_#{method}_bang_should_return_self_when_modifying_wrapped_string chars = ' él piDió Un bUen café ' assert_equal chars.object_id, chars.send("#{method}!").object_id diff --git a/guides/source/action_mailer_basics.textile b/guides/source/action_mailer_basics.textile index 7c61cc4a8d..54e55d7260 100644 --- a/guides/source/action_mailer_basics.textile +++ b/guides/source/action_mailer_basics.textile @@ -508,8 +508,8 @@ class UserMailerTest < ActionMailer::TestCase # Test the body of the sent email contains what we expect it to assert_equal [user.email], email.to assert_equal "Welcome to My Awesome Site", email.subject - assert_match(/<h1>Welcome to example.com, #{user.name}<\/h1>/, email.encoded) - assert_match(/Welcome to example.com, #{user.name}/, email.encoded) + assert_match "<h1>Welcome to example.com, #{user.name}</h1>", email.body.to_s + assert_match "you have joined to example.com community", email.body.to_s end end </ruby> diff --git a/guides/source/active_record_querying.textile b/guides/source/active_record_querying.textile index 9863d16fda..f71a136b22 100644 --- a/guides/source/active_record_querying.textile +++ b/guides/source/active_record_querying.textile @@ -1043,7 +1043,7 @@ Even though Active Record lets you specify conditions on the eager loaded associ However if you must do this, you may use +where+ as you would normally. <ruby> -Post.includes(:comments).where("comments.visible", true) +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. diff --git a/guides/source/contributing_to_ruby_on_rails.textile b/guides/source/contributing_to_ruby_on_rails.textile index fd5e2b28c0..a8a097d156 100644 --- a/guides/source/contributing_to_ruby_on_rails.textile +++ b/guides/source/contributing_to_ruby_on_rails.textile @@ -34,6 +34,8 @@ h4. What about Feature Requests? Please don't put "feature request" items into GitHub Issues. If there's a new feature that you want to see added to Ruby on Rails, you'll need to write the code yourself - or convince someone else to partner with you to write the code. Later in this guide you'll find detailed instructions for proposing a patch to Ruby on Rails. If you enter a wishlist item in GitHub Issues with no code, you can expect it to be marked "invalid" as soon as it's reviewed. +If you'd like feedback on an idea for a feature before doing the work for make a patch, please send an email to the "rails-core mailing list":https://groups.google.com/forum/?fromgroups#!forum/rubyonrails-core. You might get no response, which means that everyone is indifferent. You might find someone who's also interested in building that feature. You might get a "This won't be accepted." But it's the proper place to discuss new ideas. GitHub Issues are not a particularly good venue for the sometimes long and involved discussions new features require. + h3. Running the Test Suite To move on from submitting bugs to helping resolve existing issues or contributing your own code to Ruby on Rails, you _must_ be able to run its test suite. In this section of the guide you'll learn how to set up the tests on your own computer. diff --git a/guides/source/engines.textile b/guides/source/engines.textile index 86e7254201..53c2845731 100644 --- a/guides/source/engines.textile +++ b/guides/source/engines.textile @@ -347,7 +347,7 @@ The form will be making a +POST+ request to +/posts/:post_id/comments+, which wi <ruby> def create @post = Post.find(params[:post_id]) - @comment = @post.comments.build(params[:comment]) + @comment = @post.comments.create(params[:comment]) flash[:notice] = "Comment has been created!" redirect_to post_path end @@ -563,7 +563,7 @@ end By default, the engine's controllers inherit from <tt>Blorgh::ApplicationController</tt>. So, after making this change they will have access to the main applications +ApplicationController+ as though they were part of the main application. -This change does require that the engine is run from a Rails application that has an +ApplicationController+. +This change does require that the engine is run from a Rails application that has an +ApplicationController+. h4. Configuring an engine @@ -734,12 +734,14 @@ You can also specify these assets as dependencies of other assets using the Asse */ </plain> +INFO. Remember that in order to use languages like Sass or CoffeeScript, you should add the relevant library to your engine's +.gemspec+. + h4. Separate Assets & Precompiling There are some situations where your engine's assets not required by the host application. For example, say that you've created an admin functionality that only exists for your engine. In this case, the host application doesn't need to require +admin.css+ or +admin.js+. Only the gem's admin layout needs these assets. It doesn't make sense for the host app to include +"blorg/admin.css"+ in it's stylesheets. In this situation, you should explicitly define these assets for precompilation. -This tells sprockets to add you engine assets when +rake assets:precompile+ is ran. +This tells sprockets to add you engine assets when +rake assets:precompile+ is ran. You can define assets for precompilation in +engine.rb+ @@ -753,18 +755,40 @@ For more information, read the "Asset Pipeline guide":http://guides.rubyonrails. h4. Other gem dependencies -Gem dependencies inside an engine should be specified inside the +.gemspec+ file that's at the root of the engine. The reason for this is because the engine may be installed as a gem. If dependencies were to be specified inside the +Gemfile+, these would not be recognised by a traditional gem install and so they would not be installed, causing the engine to malfunction. +Gem dependencies inside an engine should be specified inside the +.gemspec+ file +that's at the root of the engine. The reason for this is because the engine may +be installed as a gem. If dependencies were to be specified inside the +Gemfile+, +these would not be recognised by a traditional gem install and so they would not +be installed, causing the engine to malfunction. -To specify a dependency that should be installed with the engine during a traditional +gem install+, specify it inside the +Gem::Specification+ block inside the +.gemspec+ file in the engine: +To specify a dependency that should be installed with the engine during a +traditional +gem install+, specify it inside the +Gem::Specification+ block +inside the +.gemspec+ file in the engine: <ruby> s.add_dependency "moo" </ruby> -To specify a dependency that should only be installed as a development dependency of the application, specify it like this: +To specify a dependency that should only be installed as a development +dependency of the application, specify it like this: <ruby> s.add_development_dependency "moo" </ruby> -Both kinds of dependencies will be installed when +bundle install+ is run inside the application. The development dependencies for the gem will only be used when the tests for the engine are running. +Both kinds of dependencies will be installed when +bundle install+ is run inside +the application. The development dependencies for the gem will only be used when +the tests for the engine are running. + +Note that if you want to immediately require dependencies when the engine is +required, you should require them before engine's initialization. For example: + +<ruby> +require 'other_engine/engine' +require 'yet_another_engine/engine' + +module MyEngine + class Engine < ::Rails::Engine + end +end +</ruby>
\ No newline at end of file diff --git a/guides/source/form_helpers.textile b/guides/source/form_helpers.textile index 1851aceff8..58338ce54b 100644 --- a/guides/source/form_helpers.textile +++ b/guides/source/form_helpers.textile @@ -10,7 +10,7 @@ In this guide you will: * Understand the date and time helpers Rails provides * Learn what makes a file upload form different * Learn some cases of building forms to external resources -* Find out where to look for complex forms +* Find out how to build complex forms endprologue. @@ -816,11 +816,130 @@ Or if you don't want to render an +authenticity_token+ field: h3. Building Complex Forms -Many apps grow beyond simple forms editing a single object. For example when creating a Person you might want to allow the user to (on the same form) create multiple address records (home, work, etc.). When later editing that person the user should be able to add, remove or amend addresses as necessary. While this guide has shown you all the pieces necessary to handle this, Rails does not yet have a standard end-to-end way of accomplishing this, but many have come up with viable approaches. These include: +Many apps grow beyond simple forms editing a single object. For example when creating a Person you might want to allow the user to (on the same form) create multiple address records (home, work, etc.). When later editing that person the user should be able to add, remove or amend addresses as necessary. -* As of Rails 2.3, Rails includes "Nested Attributes":./2_3_release_notes.html#nested-attributes and "Nested Object Forms":./2_3_release_notes.html#nested-object-forms -* Ryan Bates' series of Railscasts on "complex forms":http://railscasts.com/episodes/75 -* Handle Multiple Models in One Form from "Advanced Rails Recipes":http://media.pragprog.com/titles/fr_arr/multiple_models_one_form.pdf -* Eloy Duran's "complex-forms-examples":https://github.com/alloy/complex-form-examples/ application -* Lance Ivy's "nested_assignment":https://github.com/cainlevy/nested_assignment/tree/master plugin and "sample application":https://github.com/cainlevy/complex-form-examples/tree/cainlevy -* James Golick's "attribute_fu":https://github.com/jamesgolick/attribute_fu plugin +h4. Configuring the Model + +Active Record provides model level support via the +accepts_nested_attributes_for+ method: + +<ruby> +class Person < ActiveRecord::Base + has_many :addresses + accepts_nested_attributes_for :addresses + + attr_accessible :name, :addresses_attributes +end + +class Address < ActiveRecord::Base + belongs_to :person + attr_accessible :kind, :street +end +</ruby> + +This creates an +addresses_attributes=+ method on +Person+ that allows you to create, update and (optionally) destroy addresses. When using +attr_accessible+ or +attr_protected+ you must mark +addresses_attributes+ as accessible as well as the other attributes of +Person+ and +Address+ that should be mass assigned. + +h4. Building the Form + +The following form allows a user to create a +Person+ and its associated addresses. + +<erb> +<%= form_for @person do |f| %> + Addresses: + <ul> + <%= f.fields_for :addresses do |addresses_form| %> + <li> + <%= addresses_form.label :kind %> + <%= addresses_form.text_field :kind %> + + <%= addresses_form.label :street %> + <%= addresses_form.text_field :street %> + ... + </li> + <% end %> + </ul> +<% end %> +</erb> + + +When an association accepts nested attributes +fields_for+ renders its block once for every element of the association. In particular, if a person has no addresses it renders nothing. A common pattern is for the controller to build one or more empty children so that at least one set of fields is shown to the user. The example below would result in 3 sets of address fields being rendered on the new person form. + +<ruby> +def new + @person = Person.new + 3.times { @person.addresses.build} +end +</ruby> + ++fields_for+ yields a form builder that names parameters in the format expected the accessor generated by +accepts_nested_attributes_for+. For example when creating a user with 2 addresses, the submitted parameters would look like + +<ruby> +{ + :person => { + :name => 'John Doe', + :addresses_attributes => { + '0' => { + :kind => 'Home', + :street => '221b Baker Street', + }, + '1' => { + :kind => 'Office', + :street => '31 Spooner Street' + } + } + } +} +</ruby> + +The keys of the +:addresses_attributes+ hash are unimportant, they need merely be different for each address. + +If the associated object is already saved, +fields_for+ autogenerates a hidden input with the +id+ of the saved record. You can disable this by passing +:include_id => false+ to +fields_for+. You may wish to do this if the autogenerated input is placed in a location where an input tag is not valid HTML or when using an ORM where children do not have an id. + +h4. The Controller + +You do not need to write any specific controller code to use nested attributes. Create and update records as you would with a simple form. + +h4. Removing Objects + +You can allow users to delete associated objects by passing +allow_destroy => true+ to +accepts_nested_attributes_for+ + +<ruby> +class Person < ActiveRecord::Base + has_many :addresses + accepts_nested_attributes_for :addresses, :allow_destroy => true +end +</ruby> + +If the hash of attributes for an object contains the key +_destroy+ with a value of '1' or 'true' then the object will be destroyed. This form allows users to remove addresses: + +<erb> +<%= form_for @person do |f| %> + Addresses: + <ul> + <%= f.fields_for :addresses do |addresses_form| %> + <li> + <%= check_box :_destroy%> + <%= addresses_form.label :kind %> + <%= addresses_form.text_field :kind %> + ... + </li> + <% end %> + </ul> +<% end %> +</erb> + +h4. Preventing Empty Records + +It is often useful to ignore sets of fields that the user has not filled in. You can control this by passing a +:reject_if+ proc to +accepts_nested_attributes_for+. This proc will be called with each hash of attributes submitted by the form. If the proc returns +false+ then Active Record will not build an associated object for that hash. The example below only tries to build an address if the +kind+ attribute is set. + +<ruby> +class Person < ActiveRecord::Base + has_many :addresses + accepts_nested_attributes_for :addresses, :reject_if => lambda {|attributes| attributes['kind'].blank?} +end +</ruby> + +As a convenience you can instead pass the symbol +:all_blank+ which will create a proc that will reject records where all the attributes are blank excluding any value for +_destroy+. + +h4. Adding Fields on the Fly + +Rather than rendering multiple sets of fields ahead of time you may wish to add them only when a user clicks on an 'Add new child' button. Rails does not provide any builtin support for this. When generating new sets of fields you must ensure the the key of the associated array is unique - the current javascript date (milliseconds after the epoch) is a common choice.
\ No newline at end of file diff --git a/guides/source/getting_started.textile b/guides/source/getting_started.textile index 07419d11b4..b00d9af00c 100644 --- a/guides/source/getting_started.textile +++ b/guides/source/getting_started.textile @@ -1248,6 +1248,7 @@ First, take a look at +comment.rb+: <ruby> class Comment < ActiveRecord::Base belongs_to :post + attr_accessible :body, :commenter end </ruby> diff --git a/guides/source/testing.textile b/guides/source/testing.textile index d35be6a70e..4faf59fad8 100644 --- a/guides/source/testing.textile +++ b/guides/source/testing.textile @@ -1,62 +1,57 @@ h2. A Guide to Testing Rails Applications -This guide covers built-in mechanisms offered by Rails to test your application. By referring to this guide, you will be able to: +This guide covers built-in mechanisms offered by Rails to test your +application. By referring to this guide, you will be able to: * Understand Rails testing terminology -* Write unit, functional and integration tests for your application +* Write unit, functional, and integration tests for your application * Identify other popular testing approaches and plugins -This guide won't teach you to write a Rails application; it assumes basic familiarity with the Rails way of doing things. - endprologue. h3. Why Write Tests for your Rails Applications? -* Rails makes it super easy to write your tests. It starts by producing skeleton test code in the background while you are creating your models and controllers. -* By simply running your Rails tests you can ensure your code adheres to the desired functionality even after some major code refactoring. -* Rails tests can also simulate browser requests and thus you can test your application's response without having to test it through your browser. - -h3. Introduction to Testing - -Testing support was woven into the Rails fabric from the beginning. It wasn't an "oh! let's bolt on support for running tests because they're new and cool" epiphany. Just about every Rails application interacts heavily with a database - and, as a result, your tests will need a database to interact with as well. To write efficient tests, you'll need to understand how to set up this database and populate it with sample data. +Rails makes it super easy to write your tests. It starts by producing skeleton test code while you are creating your models and controllers. -h4. The Three Environments +By simply running your Rails tests you can ensure your code adheres to the desired functionality even after some major code refactoring. -Every Rails application you build has 3 sides: a side for production, a side for development, and a side for testing. +Rails tests can also simulate browser requests and thus you can test your application's response without having to test it through your browser. -One place you'll find this distinction is in the +config/database.yml+ file. This YAML configuration file has 3 different sections defining 3 unique database setups: +h3. Introduction to Testing -* production -* development -* test +Testing support was woven into the Rails fabric from the beginning. It wasn't an "oh! let's bolt on support for running tests because they're new and cool" epiphany. Just about every Rails application interacts heavily with a database and, as a result, your tests will need a database to interact with as well. To write efficient tests, you'll need to understand how to set up this database and populate it with sample data. -This allows you to set up and interact with test data without any danger of your tests altering data from your production environment. +h4. The Test Environment -For example, suppose you need to test your new +delete_this_user_and_every_everything_associated_with_it+ function. Wouldn't you want to run this in an environment where it makes no difference if you destroy data or not? +By default, every Rails application has three environments: development, test, and production. The database for each one of them is configured in +config/database.yml+. -When you do end up destroying your testing database (and it will happen, trust me), you can rebuild it from scratch according to the specs defined in the development database. You can do this by running +rake db:test:prepare+. +A dedicated test database allows you to set up and interact with test data in isolation. Tests can mangle test data with confidence, that won't touch the data in the development or production databases. h4. Rails Sets up for Testing from the Word Go Rails creates a +test+ folder for you as soon as you create a Rails project using +rails new+ _application_name_. If you list the contents of this folder then you shall see: <shell> -$ ls -F test/ +$ ls -F test -fixtures/ functional/ integration/ test_helper.rb unit/ +fixtures/ functional/ integration/ performance/ test_helper.rb unit/ </shell> -The +unit+ folder is meant to hold tests for your models, the +functional+ folder is meant to hold tests for your controllers, and the +integration+ folder is meant to hold tests that involve any number of controllers interacting. Fixtures are a way of organizing test data; they reside in the +fixtures+ folder. The +test_helper.rb+ file holds the default configuration for your tests. +The +unit+ directory is meant to hold tests for your models, the +functional+ directory is meant to hold tests for your controllers, the +integration+ directory is meant to hold tests that involve any number of controllers interacting, and the +performance+ directory is meant for performance tests. + +Fixtures are a way of organizing test data; they reside in the +fixtures+ folder. + +The +test_helper.rb+ file holds the default configuration for your tests. h4. The Low-Down on Fixtures For good tests, you'll need to give some thought to setting up test data. In Rails, you can handle this by defining and customizing fixtures. -h5. What are Fixtures? +h5. What Are Fixtures? -_Fixtures_ is a fancy word for sample data. Fixtures allow you to populate your testing database with predefined data before your tests run. Fixtures are database independent and assume a single format: *YAML*. +_Fixtures_ is a fancy word for sample data. Fixtures allow you to populate your testing database with predefined data before your tests run. Fixtures are database independent written in YAML. There is one file per model. -You'll find fixtures under your +test/fixtures+ directory. When you run +rails generate model+ to create a new model, fixture stubs will be automatically created and placed in this directory. +You'll find fixtures under your +test/fixtures+ directory. When you run +rails generate model+ to create a new model fixture stubs will be automatically created and placed in this directory. h5. YAML @@ -77,35 +72,20 @@ steve: profession: guy with keyboard </yaml> -Each fixture is given a name followed by an indented list of colon-separated key/value pairs. Records are separated by a blank space. You can place comments in a fixture file by using the # character in the first column. +Each fixture is given a name followed by an indented list of colon-separated key/value pairs. Records are typically separated by a blank space. You can place comments in a fixture file by using the # character in the first column. h5. ERB'in It Up -ERB allows you to embed ruby code within templates. YAML fixture format is pre-processed with ERB when you load fixtures. This allows you to use Ruby to help you generate some sample data. - -<erb> -<% earth_size = 20 %> -mercury: - size: <%= earth_size / 50 %> - brightest_on: <%= 113.days.ago.to_s(:db) %> - -venus: - size: <%= earth_size / 2 %> - brightest_on: <%= 67.days.ago.to_s(:db) %> - -mars: - size: <%= earth_size - 69 %> - brightest_on: <%= 13.days.from_now.to_s(:db) %> -</erb> - -Anything encased within the +ERB allows you to embed Ruby code within templates. The YAML fixture format is pre-processed with ERB when Rails loads fixtures. This allows you to use Ruby to help you generate some sample data. For example, the following code generates a thousand users: <erb> -<% %> +<% 1000.times do |n| %> +user_<%= n %>: + username: <%= "user%03d" % n %> + email: <%= "user%03d@example.com" % n %> +<% end %> </erb> -tag is considered Ruby code. When this fixture is loaded, the +size+ attribute of the three records will be set to 20/50, 20/2, and 20-69 respectively. The +brightest_on+ attribute will also be evaluated and formatted by Rails to be compatible with the database. - h5. Fixtures in Action Rails by default automatically loads all fixtures from the +test/fixtures+ folder for your unit and functional test. Loading involves three steps: @@ -377,12 +357,12 @@ There are a bunch of different types of assertions you can use. Here's the compl |_.Assertion |_.Purpose| |+assert( boolean, [msg] )+ |Ensures that the object/expression is true.| -|+assert_equal( obj1, obj2, [msg] )+ |Ensures that +obj1 == obj2+ is true.| -|+assert_not_equal( obj1, obj2, [msg] )+ |Ensures that +obj1 == obj2+ is false.| -|+assert_same( obj1, obj2, [msg] )+ |Ensures that +obj1.equal?(obj2)+ is true.| -|+assert_not_same( obj1, obj2, [msg] )+ |Ensures that +obj1.equal?(obj2)+ is false.| +|+assert_equal( expected, actual, [msg] )+ |Ensures that +expected == actual+ is true.| +|+assert_not_equal( expected, actual, [msg] )+ |Ensures that +expected != actual+ is true.| +|+assert_same( expected, actual, [msg] )+ |Ensures that +expected.equal?(actual)+ is true.| +|+assert_not_same( expected, actual, [msg] )+ |Ensures that +!expected.equal?(actual)+ is true.| |+assert_nil( obj, [msg] )+ |Ensures that +obj.nil?+ is true.| -|+assert_not_nil( obj, [msg] )+ |Ensures that +obj.nil?+ is false.| +|+assert_not_nil( obj, [msg] )+ |Ensures that +!obj.nil?+ is true.| |+assert_match( regexp, string, [msg] )+ |Ensures that a string matches the regular expression.| |+assert_no_match( regexp, string, [msg] )+ |Ensures that a string doesn't match the regular expression.| |+assert_in_delta( expecting, actual, delta, [msg] )+ |Ensures that the numbers +expecting+ and +actual+ are within +delta+ of each other.| @@ -526,7 +506,7 @@ You also have access to three instance variables in your functional tests: h4. Testing Templates and Layouts -If you want to make sure that the response rendered the correct template and layout, you can use the +assert_template+ +If you want to make sure that the response rendered the correct template and layout, you can use the +assert_template+ method: <ruby> diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt index 01f9396403..fee9eb9456 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt @@ -35,7 +35,7 @@ # Do not compress assets. config.assets.compress = false - # Expands the lines which load the assets. + # Debug mode disables concatenation and preprocessing of assets. config.assets.debug = true <%- end -%> diff --git a/railties/test/generators_test.rb b/railties/test/generators_test.rb index ad2f85a5ff..027d8eb9b7 100644 --- a/railties/test/generators_test.rb +++ b/railties/test/generators_test.rb @@ -167,7 +167,7 @@ class GeneratorsTest < Rails::Generators::TestCase def test_developer_options_are_overwriten_by_user_options Rails::Generators.options[:with_options] = { :generate => false } - self.class.class_eval <<-end_eval + self.class.class_eval(<<-end_eval, __FILE__, __LINE__ + 1) class WithOptionsGenerator < Rails::Generators::Base class_option :generate, :default => true end |