diff options
54 files changed, 550 insertions, 224 deletions
diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 7ba8319e4c..9bd2e622ad 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -479,7 +479,6 @@ module ActionController @request.session["flash"].sweep @controller.request = @request - @controller.params.merge!(parameters) build_request_uri(action, parameters) @controller.class.class_eval { include Testing } @controller.recycle! diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 796e0dbc45..4ac13695bf 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -154,7 +154,7 @@ module ActionDispatch # (case-insensitive). All major JavaScript libraries send this header with # every Ajax request. def xml_http_request? - @env['HTTP_X_REQUESTED_WITH'] =~ /XMLHttpRequest/i + /XMLHttpRequest/i === @env['HTTP_X_REQUESTED_WITH'] end alias :xhr? :xml_http_request? diff --git a/actionpack/lib/action_dispatch/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb index c758110367..9098f4e170 100644 --- a/actionpack/lib/action_dispatch/middleware/ssl.rb +++ b/actionpack/lib/action_dispatch/middleware/ssl.rb @@ -13,14 +13,11 @@ module ActionDispatch @hsts = {} if @hsts == true @hsts = self.class.default_hsts_options.merge(@hsts) if @hsts - @exclude = options[:exclude] @host = options[:host] @port = options[:port] end def call(env) - return @app.call(env) if exclude?(env) - request = Request.new(env) if request.ssl? @@ -34,10 +31,6 @@ module ActionDispatch end private - def exclude?(env) - @exclude && @exclude.call(env) - end - def redirect_to_https(request) url = URI(request.url) url.scheme = "https" @@ -65,7 +58,7 @@ module ActionDispatch cookies = cookies.split("\n") headers['Set-Cookie'] = cookies.map { |cookie| - if cookie !~ /; secure(;|$)/ + if cookie !~ /;\s+secure(;|$)/ "#{cookie}; secure" else cookie diff --git a/actionpack/lib/action_view/helpers/date_helper.rb b/actionpack/lib/action_view/helpers/date_helper.rb index ee9f0da9b8..2bfc6371f5 100644 --- a/actionpack/lib/action_view/helpers/date_helper.rb +++ b/actionpack/lib/action_view/helpers/date_helper.rb @@ -626,7 +626,7 @@ module ActionView # <time datetime="2010-11-04" pubdate="pubdate">November 04, 2010</time> # # <%= time_tag Time.now do %> - # <span>Right now</span> + # <span>Right now</span> # <% end %> # # => <time datetime="2010-11-04T17:55:45+01:00"><span>Right now</span></time> # @@ -674,11 +674,7 @@ module ActionView @options[:discard_minute] ||= true if @options[:discard_hour] @options[:discard_second] ||= true unless @options[:include_seconds] && !@options[:discard_minute] - # If the day is hidden and the month is visible, the day should be set to the 1st so all month choices are - # valid (otherwise it could be 31 and February wouldn't be a valid date) - if @datetime && @options[:discard_day] && !@options[:discard_month] - @datetime = @datetime.change(:day => 1) - end + set_day_if_discarded if @options[:tag] && @options[:ignore_date] select_time @@ -701,11 +697,7 @@ module ActionView @options[:discard_month] ||= true unless order.include?(:month) @options[:discard_day] ||= true if @options[:discard_month] || !order.include?(:day) - # If the day is hidden and the month is visible, the day should be set to the 1st so all month choices are - # valid (otherwise it could be 31 and February wouldn't be a valid date) - if @datetime && @options[:discard_day] && !@options[:discard_month] - @datetime = @datetime.change(:day => 1) - end + set_day_if_discarded [:day, :month, :year].each { |o| order.unshift(o) unless order.include?(o) } @@ -807,6 +799,14 @@ module ActionView end end + # If the day is hidden, the day should be set to the 1st so all month and year choices are + # valid. Otherwise, February 31st or February 29th, 2011 can be selected, which are invalid. + def set_day_if_discarded + if @datetime && @options[:discard_day] + @datetime = @datetime.change(:day => 1) + end + end + # Returns translated month names, but also ensures that a custom month # name array has a leading nil element. def month_names diff --git a/actionpack/test/dispatch/ssl_test.rb b/actionpack/test/dispatch/ssl_test.rb index b1463f31cf..6f075a9074 100644 --- a/actionpack/test/dispatch/ssl_test.rb +++ b/actionpack/test/dispatch/ssl_test.rb @@ -31,12 +31,6 @@ class SSLTest < ActionDispatch::IntegrationTest response.headers['Location'] end - def test_exclude_from_redirect - self.app = ActionDispatch::SSL.new(default_app, :exclude => lambda { |env| true }) - get "http://example.org/" - assert_response :success - end - def test_hsts_header_by_default get "https://example.org/" assert_equal "max-age=31536000", @@ -90,6 +84,34 @@ class SSLTest < ActionDispatch::IntegrationTest response.headers['Set-Cookie'].split("\n") end + def test_flag_cookies_as_secure_with_more_spaces_before + self.app = ActionDispatch::SSL.new(lambda { |env| + headers = { + 'Content-Type' => "text/html", + 'Set-Cookie' => "problem=def; path=/; HttpOnly; secure" + } + [200, headers, ["OK"]] + }) + + get "https://example.org/" + assert_equal ["problem=def; path=/; HttpOnly; secure"], + response.headers['Set-Cookie'].split("\n") + end + + def test_flag_cookies_as_secure_with_more_spaces_after + self.app = ActionDispatch::SSL.new(lambda { |env| + headers = { + 'Content-Type' => "text/html", + 'Set-Cookie' => "problem=def; path=/; secure; HttpOnly" + } + [200, headers, ["OK"]] + }) + + get "https://example.org/" + assert_equal ["problem=def; path=/; secure; HttpOnly"], + response.headers['Set-Cookie'].split("\n") + end + def test_no_cookies self.app = ActionDispatch::SSL.new(lambda { |env| [200, {'Content-Type' => "text/html"}, ["OK"]] diff --git a/actionpack/test/fixtures/reply.rb b/actionpack/test/fixtures/reply.rb index 19cba93673..0d3b0a7c98 100644 --- a/actionpack/test/fixtures/reply.rb +++ b/actionpack/test/fixtures/reply.rb @@ -1,5 +1,5 @@ class Reply < ActiveRecord::Base - scope :base + scope :base, -> { scoped } belongs_to :topic, :include => [:replies] belongs_to :developer diff --git a/actionpack/test/template/active_model_helper_test.rb b/actionpack/test/template/active_model_helper_test.rb index 18468ee91a..24511df444 100644 --- a/actionpack/test/template/active_model_helper_test.rb +++ b/actionpack/test/template/active_model_helper_test.rb @@ -43,14 +43,14 @@ class ActiveModelHelperTest < ActionView::TestCase 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="15" />\n</div>), + %(<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>), date_select("post", "updated_at", :discard_month => true, :discard_day => true, :start_year => 2004, :end_year => 2005) ) end def test_datetime_select_with_errors assert_dom_equal( - %(<div class="field_with_errors"><input id="post_updated_at_1i" name="post[updated_at(1i)]" type="hidden" value="2004" />\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="15" />\n<select id="post_updated_at_4i" name="post[updated_at(4i)]">\n<option selected="selected" value="00">00</option>\n<option value="01">01</option>\n<option value="02">02</option>\n<option value="03">03</option>\n<option value="04">04</option>\n<option value="05">05</option>\n<option value="06">06</option>\n<option value="07">07</option>\n<option value="08">08</option>\n<option value="09">09</option>\n<option value="10">10</option>\n<option value="11">11</option>\n<option value="12">12</option>\n<option value="13">13</option>\n<option value="14">14</option>\n<option value="15">15</option>\n<option value="16">16</option>\n<option value="17">17</option>\n<option value="18">18</option>\n<option value="19">19</option>\n<option value="20">20</option>\n<option value="21">21</option>\n<option value="22">22</option>\n<option value="23">23</option>\n</select>\n : <select id="post_updated_at_5i" name="post[updated_at(5i)]">\n<option selected="selected" value="00">00</option>\n</select>\n</div>), + %(<div class="field_with_errors"><input id="post_updated_at_1i" name="post[updated_at(1i)]" type="hidden" value="2004" />\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<select id="post_updated_at_4i" name="post[updated_at(4i)]">\n<option selected="selected" value="00">00</option>\n<option value="01">01</option>\n<option value="02">02</option>\n<option value="03">03</option>\n<option value="04">04</option>\n<option value="05">05</option>\n<option value="06">06</option>\n<option value="07">07</option>\n<option value="08">08</option>\n<option value="09">09</option>\n<option value="10">10</option>\n<option value="11">11</option>\n<option value="12">12</option>\n<option value="13">13</option>\n<option value="14">14</option>\n<option value="15">15</option>\n<option value="16">16</option>\n<option value="17">17</option>\n<option value="18">18</option>\n<option value="19">19</option>\n<option value="20">20</option>\n<option value="21">21</option>\n<option value="22">22</option>\n<option value="23">23</option>\n</select>\n : <select id="post_updated_at_5i" name="post[updated_at(5i)]">\n<option selected="selected" value="00">00</option>\n</select>\n</div>), datetime_select("post", "updated_at", :discard_year => true, :discard_month => true, :discard_day => true, :minute_step => 60) ) end diff --git a/actionpack/test/template/date_helper_test.rb b/actionpack/test/template/date_helper_test.rb index 1d934b153f..f622097b57 100644 --- a/actionpack/test/template/date_helper_test.rb +++ b/actionpack/test/template/date_helper_test.rb @@ -711,7 +711,7 @@ class DateHelperTest < ActionView::TestCase # Since the order is incomplete nothing will be shown expected = %(<input id="date_first_year" name="date[first][year]" type="hidden" value="2003" />\n) expected << %(<input id="date_first_month" name="date[first][month]" type="hidden" value="8" />\n) - expected << %(<input id="date_first_day" name="date[first][day]" type="hidden" value="16" />\n) + expected << %(<input id="date_first_day" name="date[first][day]" type="hidden" value="1" />\n) assert_dom_equal expected, select_date(Time.mktime(2003, 8, 16), :start_year => 2003, :end_year => 2005, :prefix => "date[first]", :order => [:day]) end @@ -943,7 +943,7 @@ class DateHelperTest < ActionView::TestCase expected << "</select>\n" expected << %(<input type="hidden" id="date_first_month" name="date[first][month]" value="8" />\n) - expected << %(<input type="hidden" id="date_first_day" name="date[first][day]" value="16" />\n) + expected << %(<input type="hidden" id="date_first_day" name="date[first][day]" value="1" />\n) assert_dom_equal expected, select_date(Time.mktime(2003, 8, 16), { :date_separator => " / ", :discard_month => true, :discard_day => true, :start_year => 2003, :end_year => 2005, :prefix => "date[first]"}) end @@ -1396,6 +1396,20 @@ class DateHelperTest < ActionView::TestCase assert_dom_equal expected, date_select("post", "written_on", :order => [ :month, :year ]) end + def test_date_select_without_day_and_month + @post = Post.new + @post.written_on = Date.new(2004, 2, 29) + + expected = "<input type=\"hidden\" id=\"post_written_on_2i\" name=\"post[written_on(2i)]\" value=\"2\" />\n" + expected << "<input type=\"hidden\" id=\"post_written_on_3i\" name=\"post[written_on(3i)]\" value=\"1\" />\n" + + expected << %{<select id="post_written_on_1i" name="post[written_on(1i)]">\n} + expected << %{<option value="1999">1999</option>\n<option value="2000">2000</option>\n<option value="2001">2001</option>\n<option value="2002">2002</option>\n<option value="2003">2003</option>\n<option value="2004" selected="selected">2004</option>\n<option value="2005">2005</option>\n<option value="2006">2006</option>\n<option value="2007">2007</option>\n<option value="2008">2008</option>\n<option value="2009">2009</option>\n} + expected << "</select>\n" + + assert_dom_equal expected, date_select("post", "written_on", :order => [ :year ]) + end + def test_date_select_without_day_with_separator @post = Post.new @post.written_on = Date.new(2004, 6, 15) @@ -2427,7 +2441,7 @@ class DateHelperTest < ActionView::TestCase 1999.upto(2009) { |i| expected << %(<option value="#{i}"#{' selected="selected"' if i == 2004}>#{i}</option>\n) } expected << "</select>\n" expected << %{<input type="hidden" id="post_updated_at_2i" name="post[updated_at(2i)]" value="6" />\n} - expected << %{<input type="hidden" id="post_updated_at_3i" name="post[updated_at(3i)]" value="15" />\n} + expected << %{<input type="hidden" id="post_updated_at_3i" name="post[updated_at(3i)]" value="1" />\n} expected << " — " @@ -2448,7 +2462,7 @@ class DateHelperTest < ActionView::TestCase expected = %{<input type="hidden" id="post_updated_at_1i" name="post[updated_at(1i)]" value="2004" />\n} expected << %{<input type="hidden" id="post_updated_at_2i" name="post[updated_at(2i)]" value="6" />\n} - expected << %{<input type="hidden" id="post_updated_at_3i" name="post[updated_at(3i)]" value="15" />\n} + expected << %{<input type="hidden" id="post_updated_at_3i" name="post[updated_at(3i)]" value="1" />\n} expected << %{<select id="post_updated_at_4i" name="post[updated_at(4i)]">\n} 0.upto(23) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 15}>#{sprintf("%02d", i)}</option>\n) } @@ -2467,7 +2481,7 @@ class DateHelperTest < ActionView::TestCase expected = %{<input type="hidden" id="post_updated_at_1i" disabled="disabled" name="post[updated_at(1i)]" value="2004" />\n} expected << %{<input type="hidden" id="post_updated_at_2i" disabled="disabled" name="post[updated_at(2i)]" value="6" />\n} - expected << %{<input type="hidden" id="post_updated_at_3i" disabled="disabled" name="post[updated_at(3i)]" value="15" />\n} + expected << %{<input type="hidden" id="post_updated_at_3i" disabled="disabled" name="post[updated_at(3i)]" value="1" />\n} expected << %{<select id="post_updated_at_4i" disabled="disabled" name="post[updated_at(4i)]">\n} 0.upto(23) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 15}>#{sprintf("%02d", i)}</option>\n) } diff --git a/activemodel/lib/active_model/naming.rb b/activemodel/lib/active_model/naming.rb index fd0bc4e8e9..b22d9539b0 100644 --- a/activemodel/lib/active_model/naming.rb +++ b/activemodel/lib/active_model/naming.rb @@ -6,13 +6,11 @@ require 'active_support/core_ext/object/blank' module ActiveModel class Name < String - attr_reader :singular, :plural, :element, :collection, :partial_path, + attr_reader :singular, :plural, :element, :collection, :singular_route_key, :route_key, :param_key, :i18n_key alias_method :cache_key, :collection - deprecate :partial_path => "ActiveModel::Name#partial_path is deprecated. Call #to_partial_path on model instances directly instead." - def initialize(klass, namespace = nil, name = nil) name ||= klass.name @@ -27,7 +25,6 @@ module ActiveModel @element = ActiveSupport::Inflector.underscore(ActiveSupport::Inflector.demodulize(self)).freeze @human = ActiveSupport::Inflector.humanize(@element).freeze @collection = ActiveSupport::Inflector.tableize(self).freeze - @partial_path = "#{@collection}/#{@element}".freeze @param_key = (namespace ? _singularize(@unnamespaced) : @singular).freeze @i18n_key = self.underscore.to_sym diff --git a/activemodel/test/cases/naming_test.rb b/activemodel/test/cases/naming_test.rb index 1e14d83bcb..49d8706ac2 100644 --- a/activemodel/test/cases/naming_test.rb +++ b/activemodel/test/cases/naming_test.rb @@ -25,12 +25,6 @@ class NamingTest < ActiveModel::TestCase assert_equal 'post/track_backs', @model_name.collection end - def test_partial_path - assert_deprecated(/#partial_path.*#to_partial_path/) do - assert_equal 'post/track_backs/track_back', @model_name.partial_path - end - end - def test_human assert_equal 'Track back', @model_name.human end @@ -61,12 +55,6 @@ class NamingWithNamespacedModelInIsolatedNamespaceTest < ActiveModel::TestCase assert_equal 'blog/posts', @model_name.collection end - def test_partial_path - assert_deprecated(/#partial_path.*#to_partial_path/) do - assert_equal 'blog/posts/post', @model_name.partial_path - end - end - def test_human assert_equal 'Post', @model_name.human end @@ -105,12 +93,6 @@ class NamingWithNamespacedModelInSharedNamespaceTest < ActiveModel::TestCase assert_equal 'blog/posts', @model_name.collection end - def test_partial_path - assert_deprecated(/#partial_path.*#to_partial_path/) do - assert_equal 'blog/posts/post', @model_name.partial_path - end - end - def test_human assert_equal 'Post', @model_name.human end @@ -149,12 +131,6 @@ class NamingWithSuppliedModelNameTest < ActiveModel::TestCase assert_equal 'articles', @model_name.collection end - def test_partial_path - assert_deprecated(/#partial_path.*#to_partial_path/) do - assert_equal 'articles/article', @model_name.partial_path - end - end - def test_human assert_equal 'Article', @model_name.human end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 5a289a5aac..46031e7c13 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,36 @@ ## Rails 4.0.0 (unreleased) ## +* Deprecate eager-evaluated scopes. + + Don't use this: + + scope :red, where(color: 'red') + default_scope where(color: 'red') + + Use this: + + scope :red, -> { where(color: 'red') } + default_scope { where(color: 'red') } + + The former has numerous issues. It is a common newbie gotcha to do + the following: + + scope :recent, where(published_at: Time.now - 2.weeks) + + Or a more subtle variant: + + scope :recent, -> { where(published_at: Time.now - 2.weeks) } + scope :recent_red, recent.where(color: 'red') + + Eager scopes are also very complex to implement within Active + Record, and there are still bugs. For example, the following does + not do what you expect: + + scope :remove_conditions, except(:where) + where(...).remove_conditions # => still has conditions + + *Jon Leighton* + * Remove IdentityMap IdentityMap has never graduated to be an "enabled-by-default" feature, due diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 5c42e8f719..9796b0a321 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -4,16 +4,12 @@ require 'zlib' require 'active_support/dependencies' require 'active_support/core_ext/object/blank' require 'active_record/fixtures/file' +require 'active_record/errors' -if defined? ActiveRecord +module ActiveRecord class FixtureClassNotFound < ActiveRecord::ActiveRecordError #:nodoc: end -else - class FixtureClassNotFound < StandardError #:nodoc: - end -end -module ActiveRecord # \Fixtures are a way of organizing data that you want to test against; in short, sample data. # # They are stored in YAML files, one file per model, which are placed in the directory diff --git a/activerecord/lib/active_record/scoping/default.rb b/activerecord/lib/active_record/scoping/default.rb index 5f05d146f2..b0609a8c08 100644 --- a/activerecord/lib/active_record/scoping/default.rb +++ b/activerecord/lib/active_record/scoping/default.rb @@ -1,4 +1,5 @@ require 'active_support/concern' +require 'active_support/deprecation' module ActiveRecord module Scoping @@ -51,7 +52,7 @@ module ActiveRecord # the model. # # class Article < ActiveRecord::Base - # default_scope where(:published => true) + # default_scope { where(:published => true) } # end # # Article.all # => SELECT * FROM articles WHERE published = true @@ -62,12 +63,6 @@ module ActiveRecord # Article.new.published # => true # Article.create.published # => true # - # You can also use <tt>default_scope</tt> with a block, in order to have it lazily evaluated: - # - # class Article < ActiveRecord::Base - # default_scope { where(:published_at => Time.now - 1.week) } - # end - # # (You can also pass any object which responds to <tt>call</tt> to the <tt>default_scope</tt> # macro, and it will be called when building the default scope.) # @@ -75,8 +70,8 @@ module ActiveRecord # be merged together: # # class Article < ActiveRecord::Base - # default_scope where(:published => true) - # default_scope where(:rating => 'G') + # default_scope { where(:published => true) } + # default_scope { where(:rating => 'G') } # end # # Article.all # => SELECT * FROM articles WHERE published = true AND rating = 'G' @@ -94,6 +89,16 @@ module ActiveRecord # end def default_scope(scope = {}) scope = Proc.new if block_given? + + if scope.is_a?(Relation) || !scope.respond_to?(:call) + ActiveSupport::Deprecation.warn( + "Calling #default_scope without a block is deprecated. For example instead " \ + "of `default_scope where(color: 'red')`, please use " \ + "`default_scope { where(color: 'red') }`. (Alternatively you can just redefine " \ + "self.default_scope.)" + ) + end + self.default_scopes = default_scopes + [scope] end @@ -106,7 +111,7 @@ module ActiveRecord if scope.is_a?(Hash) default_scope.apply_finder_options(scope) elsif !scope.is_a?(Relation) && scope.respond_to?(:call) - default_scope.merge(scope.call) + default_scope.merge(unscoped { scope.call }) else default_scope.merge(scope) end diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb index 0edc3f1dcc..077e2d067e 100644 --- a/activerecord/lib/active_record/scoping/named.rb +++ b/activerecord/lib/active_record/scoping/named.rb @@ -3,6 +3,7 @@ require 'active_support/core_ext/hash/except' require 'active_support/core_ext/kernel/singleton_class' require 'active_support/core_ext/object/blank' require 'active_support/core_ext/class/attribute' +require 'active_support/deprecation' module ActiveRecord # = Active Record Named \Scopes @@ -171,30 +172,30 @@ module ActiveRecord # Article.published.featured.latest_article # Article.featured.titles - def scope(name, scope_options = {}) - name = name.to_sym - valid_scope_name?(name) - extension = Module.new(&Proc.new) if block_given? + def scope(name, body = {}, &block) + extension = Module.new(&block) if block - scope_proc = lambda do |*args| - options = scope_options.respond_to?(:call) ? unscoped { scope_options.call(*args) } : scope_options + # Check body.is_a?(Relation) to prevent the relation actually being + # loaded by respond_to? + if body.is_a?(Relation) || !body.respond_to?(:call) + ActiveSupport::Deprecation.warn( + "Using #scope without passing a callable object is deprecated. For " \ + "example `scope :red, where(color: 'red')` should be changed to " \ + "`scope :red, -> { where(color: 'red') }`. There are numerous gotchas " \ + "in the former usage and it makes the implementation more complicated " \ + "and buggy. (If you prefer, you can just define a class method named " \ + "`self.red`.)" + ) + end + + singleton_class.send(:define_method, name) do |*args| + options = body.respond_to?(:call) ? unscoped { body.call(*args) } : body options = scoped.apply_finder_options(options) if options.is_a?(Hash) relation = scoped.merge(options) extension ? relation.extending(extension) : relation end - - singleton_class.send(:redefine_method, name, &scope_proc) - end - - protected - - def valid_scope_name?(name) - if respond_to?(name, true) - logger.warn "Creating scope :#{name}. " \ - "Overwriting existing method #{self.name}.#{name}." - end end end end diff --git a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb index d084a00ed7..9ca63a209e 100644 --- a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb +++ b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb @@ -5,7 +5,7 @@ class <%= migration_class_name %> < ActiveRecord::Migration add_column :<%= table_name %>, :<%= attribute.name %>, :<%= attribute.type %><%= attribute.inject_options %> <%- if attribute.has_index? -%> add_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %> - <%- end %> + <%- end -%> <%- end -%> end <%- else -%> @@ -24,6 +24,9 @@ class <%= migration_class_name %> < ActiveRecord::Migration <% attributes.reverse.each do |attribute| -%> <%- if migration_action -%> <%= migration_action == 'add' ? 'remove' : 'add' %>_column :<%= table_name %>, :<%= attribute.name %><% if migration_action == 'remove' %>, :<%= attribute.type %><%= attribute.inject_options %><% end %> + <%- if attribute.has_index? && migration_action == 'remove' -%> + add_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %> + <%- end -%> <%- end -%> <%- end -%> end diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index 562b370c97..e660b2ca35 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -514,7 +514,7 @@ class InvalidTableNameFixturesTest < ActiveRecord::TestCase self.use_transactional_fixtures = false def test_raises_error - assert_raise FixtureClassNotFound do + assert_raise ActiveRecord::FixtureClassNotFound do funny_jokes(:a_joke) end end diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index e17ba76437..0d3c0b20a4 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -123,16 +123,6 @@ class NamedScopeTest < ActiveRecord::TestCase assert objects.all?(&:approved?), 'all objects should be approved' end - def test_extensions - assert_equal 1, Topic.anonymous_extension.one - assert_equal 2, Topic.named_extension.two - end - - def test_multiple_extensions - assert_equal 2, Topic.multiple_extensions.extension_two - assert_equal 1, Topic.multiple_extensions.extension_one - end - def test_has_many_associations_have_access_to_scopes assert_not_equal Post.containing_the_letter_a, authors(:david).posts assert !Post.containing_the_letter_a.empty? @@ -393,25 +383,6 @@ class NamedScopeTest < ActiveRecord::TestCase end end - def test_scopes_with_reserved_names - class << Topic - def public_method; end - public :public_method - - def protected_method; end - protected :protected_method - - def private_method; end - private :private_method - end - - [:public_method, :protected_method, :private_method].each do |reserved_method| - assert Topic.respond_to?(reserved_method, true) - ActiveRecord::Base.logger.expects(:warn) - Topic.scope(reserved_method) - end - end - def test_scopes_on_relations # Topic.replied approved_topics = Topic.scoped.approved.order('id DESC') @@ -483,6 +454,41 @@ class NamedScopeTest < ActiveRecord::TestCase require "models/without_table" end end + + def test_eager_scopes_are_deprecated + klass = Class.new(ActiveRecord::Base) + klass.table_name = 'posts' + + assert_deprecated do + klass.scope :welcome, { :conditions => { :id => posts(:welcome).id } } + end + assert_equal [posts(:welcome).title], klass.welcome.map(&:title) + + assert_deprecated do + klass.scope :welcome_2, klass.where(:id => posts(:welcome).id) + end + assert_equal [posts(:welcome).title], klass.welcome_2.map(&:title) + end + + def test_eager_default_scope_hashes_are_deprecated + klass = Class.new(ActiveRecord::Base) + klass.table_name = 'posts' + + assert_deprecated do + klass.send(:default_scope, :conditions => { :id => posts(:welcome).id }) + end + assert_equal [posts(:welcome).title], klass.all.map(&:title) + end + + def test_eager_default_scope_relations_are_deprecated + klass = Class.new(ActiveRecord::Base) + klass.table_name = 'posts' + + assert_deprecated do + klass.send(:default_scope, klass.where(:id => posts(:welcome).id)) + end + assert_equal [posts(:welcome).title], klass.all.map(&:title) + end end class DynamicScopeMatchTest < ActiveRecord::TestCase diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index d50e11d6c9..14444a0092 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -140,8 +140,8 @@ class Author < ActiveRecord::Base has_many :posts_with_default_include, :class_name => 'PostWithDefaultInclude' has_many :comments_on_posts_with_default_include, :through => :posts_with_default_include, :source => :comments - scope :relation_include_posts, includes(:posts) - scope :relation_include_tags, includes(:tags) + scope :relation_include_posts, -> { includes(:posts) } + scope :relation_include_tags, -> { includes(:tags) } attr_accessor :post_log after_initialize :set_post_log diff --git a/activerecord/test/models/bulb.rb b/activerecord/test/models/bulb.rb index 888afc7604..640e57555d 100644 --- a/activerecord/test/models/bulb.rb +++ b/activerecord/test/models/bulb.rb @@ -1,5 +1,5 @@ class Bulb < ActiveRecord::Base - default_scope where(:name => 'defaulty') + default_scope { where(:name => 'defaulty') } belongs_to :car attr_protected :car_id, :frickinawesome diff --git a/activerecord/test/models/car.rb b/activerecord/test/models/car.rb index 6ff1329d8e..42ac81690f 100644 --- a/activerecord/test/models/car.rb +++ b/activerecord/test/models/car.rb @@ -11,18 +11,18 @@ class Car < ActiveRecord::Base has_many :engines, :dependent => :destroy has_many :wheels, :as => :wheelable, :dependent => :destroy - scope :incl_tyres, includes(:tyres) - scope :incl_engines, includes(:engines) + scope :incl_tyres, -> { includes(:tyres) } + scope :incl_engines, -> { includes(:engines) } - scope :order_using_new_style, order('name asc') - scope :order_using_old_style, :order => 'name asc' + scope :order_using_new_style, -> { order('name asc') } + scope :order_using_old_style, -> { { :order => 'name asc' } } end class CoolCar < Car - default_scope :order => 'name desc' + default_scope { order('name desc') } end class FastCar < Car - default_scope :order => 'name desc' + default_scope { order('name desc') } end diff --git a/activerecord/test/models/categorization.rb b/activerecord/test/models/categorization.rb index 4bd980e606..6588531de6 100644 --- a/activerecord/test/models/categorization.rb +++ b/activerecord/test/models/categorization.rb @@ -12,7 +12,7 @@ end class SpecialCategorization < ActiveRecord::Base self.table_name = 'categorizations' - default_scope where(:special => true) + default_scope { where(:special => true) } belongs_to :author belongs_to :category diff --git a/activerecord/test/models/category.rb b/activerecord/test/models/category.rb index 02b85fd38a..ab3139680c 100644 --- a/activerecord/test/models/category.rb +++ b/activerecord/test/models/category.rb @@ -27,7 +27,7 @@ class Category < ActiveRecord::Base has_many :authors, :through => :categorizations has_many :authors_with_select, :through => :categorizations, :source => :author, :select => 'authors.*, categorizations.post_id' - scope :general, :conditions => { :name => 'General' } + scope :general, -> { where(:name => 'General') } end class SpecialCategory < Category diff --git a/activerecord/test/models/comment.rb b/activerecord/test/models/comment.rb index 88b139d931..00f5a74070 100644 --- a/activerecord/test/models/comment.rb +++ b/activerecord/test/models/comment.rb @@ -1,12 +1,10 @@ class Comment < ActiveRecord::Base scope :limit_by, lambda {|l| limit(l) } - scope :containing_the_letter_e, :conditions => "comments.body LIKE '%e%'" - scope :not_again, where("comments.body NOT LIKE '%again%'") - scope :for_first_post, :conditions => { :post_id => 1 } - scope :for_first_author, - :joins => :post, - :conditions => { "posts.author_id" => 1 } - scope :created + scope :containing_the_letter_e, -> { where("comments.body LIKE '%e%'") } + scope :not_again, -> { where("comments.body NOT LIKE '%again%'") } + scope :for_first_post, -> { where(:post_id => 1) } + scope :for_first_author, -> { joins(:post).where("posts.author_id" => 1) } + scope :created, -> { scoped } belongs_to :post, :counter_cache => true has_many :ratings @@ -25,7 +23,7 @@ class Comment < ActiveRecord::Base def self.all_as_method all end - scope :all_as_scope, {} + scope :all_as_scope, -> { scoped } end class SpecialComment < Comment diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index 4dc9fff9fd..4cc4947e3b 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -45,7 +45,7 @@ class Developer < ActiveRecord::Base has_many :audit_logs - scope :jamises, :conditions => {:name => 'Jamis'} + scope :jamises, -> { where(:name => 'Jamis') } validates_inclusion_of :salary, :in => 50000..200000 validates_length_of :name, :within => 3..20 @@ -88,20 +88,20 @@ end class DeveloperWithSelect < ActiveRecord::Base self.table_name = 'developers' - default_scope select('name') + default_scope { select('name') } end class DeveloperWithIncludes < ActiveRecord::Base self.table_name = 'developers' has_many :audit_logs, :foreign_key => :developer_id - default_scope includes(:audit_logs) + default_scope { includes(:audit_logs) } end class DeveloperOrderedBySalary < ActiveRecord::Base self.table_name = 'developers' - default_scope :order => 'salary DESC' + default_scope { order('salary DESC') } - scope :by_name, order('name DESC') + scope :by_name, -> { order('name DESC') } def self.all_ordered_by_name with_scope(:find => { :order => 'name DESC' }) do @@ -112,7 +112,7 @@ end class DeveloperCalledDavid < ActiveRecord::Base self.table_name = 'developers' - default_scope where("name = 'David'") + default_scope { where("name = 'David'") } end class LazyLambdaDeveloperCalledDavid < ActiveRecord::Base @@ -140,7 +140,7 @@ end class ClassMethodReferencingScopeDeveloperCalledDavid < ActiveRecord::Base self.table_name = 'developers' - scope :david, where(:name => 'David') + scope :david, -> { where(:name => 'David') } def self.default_scope david @@ -149,40 +149,40 @@ end class LazyBlockReferencingScopeDeveloperCalledDavid < ActiveRecord::Base self.table_name = 'developers' - scope :david, where(:name => 'David') + scope :david, -> { where(:name => 'David') } default_scope { david } end class DeveloperCalledJamis < ActiveRecord::Base self.table_name = 'developers' - default_scope where(:name => 'Jamis') - scope :poor, where('salary < 150000') + default_scope { where(:name => 'Jamis') } + scope :poor, -> { where('salary < 150000') } end class PoorDeveloperCalledJamis < ActiveRecord::Base self.table_name = 'developers' - default_scope where(:name => 'Jamis', :salary => 50000) + default_scope -> { where(:name => 'Jamis', :salary => 50000) } end class InheritedPoorDeveloperCalledJamis < DeveloperCalledJamis self.table_name = 'developers' - default_scope where(:salary => 50000) + default_scope -> { where(:salary => 50000) } end class MultiplePoorDeveloperCalledJamis < ActiveRecord::Base self.table_name = 'developers' - default_scope where(:name => 'Jamis') - default_scope where(:salary => 50000) + default_scope -> { where(:name => 'Jamis') } + default_scope -> { where(:salary => 50000) } end module SalaryDefaultScope extend ActiveSupport::Concern - included { default_scope where(:salary => 50000) } + included { default_scope { where(:salary => 50000) } } end class ModuleIncludedPoorDeveloperCalledJamis < DeveloperCalledJamis @@ -195,7 +195,7 @@ class EagerDeveloperWithDefaultScope < ActiveRecord::Base self.table_name = 'developers' has_and_belongs_to_many :projects, :foreign_key => 'developer_id', :join_table => 'developers_projects', :order => 'projects.id' - default_scope includes(:projects) + default_scope { includes(:projects) } end class EagerDeveloperWithClassMethodDefaultScope < ActiveRecord::Base diff --git a/activerecord/test/models/organization.rb b/activerecord/test/models/organization.rb index 4a4111833f..72e7bade68 100644 --- a/activerecord/test/models/organization.rb +++ b/activerecord/test/models/organization.rb @@ -8,5 +8,5 @@ class Organization < ActiveRecord::Base has_one :author, :primary_key => :name has_one :author_owned_essay_category, :through => :author, :source => :owned_essay_category - scope :clubs, { :from => 'clubs' } + scope :clubs, -> { from('clubs') } end diff --git a/activerecord/test/models/person.rb b/activerecord/test/models/person.rb index 84bc901b5e..b7d5dabc4f 100644 --- a/activerecord/test/models/person.rb +++ b/activerecord/test/models/person.rb @@ -27,8 +27,8 @@ class Person < ActiveRecord::Base has_many :agents_posts, :through => :agents, :source => :posts has_many :agents_posts_authors, :through => :agents_posts, :source => :author - scope :males, :conditions => { :gender => 'M' } - scope :females, :conditions => { :gender => 'F' } + scope :males, -> { where(:gender => 'M') } + scope :females, -> { where(:gender => 'F') } end class PersonWithDependentDestroyJobs < ActiveRecord::Base diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 0fc22ac6a3..5002ab9ff8 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -5,8 +5,8 @@ class Post < ActiveRecord::Base end end - scope :containing_the_letter_a, where("body LIKE '%a%'") - scope :ranked_by_comments, order("comments_count DESC") + scope :containing_the_letter_a, -> { where("body LIKE '%a%'") } + scope :ranked_by_comments, -> { order("comments_count DESC") } scope :limit_by, lambda {|l| limit(l) } scope :with_authors_at_address, lambda { |address| { @@ -30,8 +30,8 @@ class Post < ActiveRecord::Base has_one :first_comment, :class_name => 'Comment', :order => 'id ASC' has_one :last_comment, :class_name => 'Comment', :order => 'id desc' - scope :with_special_comments, :joins => :comments, :conditions => {:comments => {:type => 'SpecialComment'} } - scope :with_very_special_comments, joins(:comments).where(:comments => {:type => 'VerySpecialComment'}) + scope :with_special_comments, -> { joins(:comments).where(:comments => {:type => 'SpecialComment'}) } + scope :with_very_special_comments, -> { joins(:comments).where(:comments => {:type => 'VerySpecialComment'}) } scope :with_post, lambda {|post_id| { :joins => :comments, :conditions => {:comments => {:post_id => post_id} } } } @@ -171,7 +171,7 @@ end class FirstPost < ActiveRecord::Base self.table_name = 'posts' - default_scope where(:id => 1) + default_scope { where(:id => 1) } has_many :comments, :foreign_key => :post_id has_one :comment, :foreign_key => :post_id @@ -179,16 +179,16 @@ end class PostWithDefaultInclude < ActiveRecord::Base self.table_name = 'posts' - default_scope includes(:comments) + default_scope { includes(:comments) } has_many :comments, :foreign_key => :post_id end class PostWithDefaultScope < ActiveRecord::Base self.table_name = 'posts' - default_scope :order => :title + default_scope { order(:title) } end class SpecialPostWithDefaultScope < ActiveRecord::Base self.table_name = 'posts' - default_scope where(:id => [1, 5,6]) + default_scope { where(:id => [1, 5,6]) } end diff --git a/activerecord/test/models/project.rb b/activerecord/test/models/project.rb index efe1ce67da..32ce164995 100644 --- a/activerecord/test/models/project.rb +++ b/activerecord/test/models/project.rb @@ -32,7 +32,7 @@ class Project < ActiveRecord::Base def self.all_as_method all end - scope :all_as_scope, {} + scope :all_as_scope, -> { scoped } end class SpecialProject < Project diff --git a/activerecord/test/models/reference.rb b/activerecord/test/models/reference.rb index c5af0b5d5f..561b431766 100644 --- a/activerecord/test/models/reference.rb +++ b/activerecord/test/models/reference.rb @@ -19,5 +19,5 @@ end class BadReference < ActiveRecord::Base self.table_name = 'references' - default_scope where(:favourite => false) + default_scope { where(:favourite => false) } end diff --git a/activerecord/test/models/reply.rb b/activerecord/test/models/reply.rb index 6adfe0ae3c..53bc95e5f2 100644 --- a/activerecord/test/models/reply.rb +++ b/activerecord/test/models/reply.rb @@ -1,7 +1,7 @@ require 'models/topic' class Reply < Topic - scope :base + scope :base, -> { scoped } belongs_to :topic, :foreign_key => "parent_id", :counter_cache => true belongs_to :topic_with_primary_key, :class_name => "Topic", :primary_key => "title", :foreign_key => "parent_title", :counter_cache => "replies_count" diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb index 8bcb9df8a8..785839be75 100644 --- a/activerecord/test/models/topic.rb +++ b/activerecord/test/models/topic.rb @@ -1,21 +1,24 @@ class Topic < ActiveRecord::Base - scope :base + scope :base, -> { scoped } scope :written_before, lambda { |time| if time { :conditions => ['written_on < ?', time] } end } - scope :approved, :conditions => {:approved => true} - scope :rejected, :conditions => {:approved => false} + scope :approved, -> { where(:approved => true) } + scope :rejected, -> { where(:approved => false) } scope :scope_with_lambda, lambda { scoped } - scope :by_lifo, :conditions => {:author_name => 'lifo'} + scope :by_lifo, -> { where(:author_name => 'lifo') } - scope :approved_as_hash_condition, :conditions => {:topics => {:approved => true}} - scope 'approved_as_string', :conditions => {:approved => true} - scope :replied, :conditions => ['replies_count > 0'] - scope :anonymous_extension do + ActiveSupport::Deprecation.silence do + scope :approved_as_hash_condition, :conditions => {:topics => {:approved => true}} + scope :replied, :conditions => ['replies_count > 0'] + end + + scope 'approved_as_string', -> { where(:approved => true) } + scope :anonymous_extension, -> { scoped } do def one 1 end @@ -42,8 +45,8 @@ class Topic < ActiveRecord::Base 2 end end - scope :named_extension, :extend => NamedExtension - scope :multiple_extensions, :extend => [MultipleExtensionTwo, MultipleExtensionOne] + scope :named_extension, -> { { :extend => NamedExtension } } + scope :multiple_extensions, -> { { :extend => [MultipleExtensionTwo, MultipleExtensionOne] } } has_many :replies, :dependent => :destroy, :foreign_key => "parent_id" has_many :replies_with_primary_key, :class_name => "Reply", :dependent => :destroy, :primary_key => "title", :foreign_key => "parent_title" diff --git a/activerecord/test/models/toy.rb b/activerecord/test/models/toy.rb index 0377e50011..ddc7048a56 100644 --- a/activerecord/test/models/toy.rb +++ b/activerecord/test/models/toy.rb @@ -2,5 +2,5 @@ class Toy < ActiveRecord::Base self.primary_key = :toy_id belongs_to :pet - scope :with_pet, joins(:pet) + scope :with_pet, -> { joins(:pet) } end diff --git a/activerecord/test/models/without_table.rb b/activerecord/test/models/without_table.rb index 184ab1649e..50c824e4ac 100644 --- a/activerecord/test/models/without_table.rb +++ b/activerecord/test/models/without_table.rb @@ -1,3 +1,3 @@ class WithoutTable < ActiveRecord::Base - default_scope where(:published => true) + default_scope -> { where(:published => true) } end diff --git a/activesupport/lib/active_support/core_ext/hash/slice.rb b/activesupport/lib/active_support/core_ext/hash/slice.rb index fbc6e538d5..45181f0e16 100644 --- a/activesupport/lib/active_support/core_ext/hash/slice.rb +++ b/activesupport/lib/active_support/core_ext/hash/slice.rb @@ -13,7 +13,7 @@ class Hash # valid_keys = [:mass, :velocity, :time] # search(options.slice(*valid_keys)) def slice(*keys) - keys.map! { |key| convert_key(key) } if respond_to?(:convert_key) + keys.map! { |key| convert_key(key) } if respond_to?(:convert_key, true) keys.each_with_object(self.class.new) { |k, hash| hash[k] = self[k] if has_key?(k) } end @@ -21,7 +21,7 @@ class Hash # Returns a hash contained the removed key/value pairs # {:a => 1, :b => 2, :c => 3, :d => 4}.slice!(:a, :b) # => {:c => 3, :d => 4} def slice!(*keys) - keys.map! { |key| convert_key(key) } if respond_to?(:convert_key) + keys.map! { |key| convert_key(key) } if respond_to?(:convert_key, true) omit = slice(*self.keys - keys) hash = slice(*keys) replace(hash) diff --git a/activesupport/lib/active_support/core_ext/object/duplicable.rb b/activesupport/lib/active_support/core_ext/object/duplicable.rb index 9d044eba71..9d1630bb7c 100644 --- a/activesupport/lib/active_support/core_ext/object/duplicable.rb +++ b/activesupport/lib/active_support/core_ext/object/duplicable.rb @@ -104,3 +104,16 @@ class Module false end end + +require 'bigdecimal' +class BigDecimal + begin + BigDecimal.new('4.56').dup + + def duplicable? + true + end + rescue TypeError + # can't dup, so use superclass implementation + end +end diff --git a/activesupport/lib/active_support/core_ext/proc.rb b/activesupport/lib/active_support/core_ext/proc.rb index 94bb5fb0cb..cd63740940 100644 --- a/activesupport/lib/active_support/core_ext/proc.rb +++ b/activesupport/lib/active_support/core_ext/proc.rb @@ -1,7 +1,10 @@ require "active_support/core_ext/kernel/singleton_class" +require "active_support/deprecation" class Proc #:nodoc: def bind(object) + ActiveSupport::Deprecation.warn 'Proc#bind is deprecated and will be removed in future versions', caller + block, time = self, Time.now object.class_eval do method_name = "__bind_#{time.to_i}_#{time.usec}" diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index e4a13870d7..49aa012268 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -5,7 +5,7 @@ module ActiveSupport # people can write <tt>params[:key]</tt> instead of <tt>params['key']</tt> # and they get the same value for both keys. class HashWithIndifferentAccess < Hash - + # Always returns true, so that <tt>Array#extract_options!</tt> finds members of this class. def extractable_options? true diff --git a/activesupport/lib/active_support/notifications.rb b/activesupport/lib/active_support/notifications.rb index 13f675c654..8cf7bdafda 100644 --- a/activesupport/lib/active_support/notifications.rb +++ b/activesupport/lib/active_support/notifications.rb @@ -1,3 +1,6 @@ +require 'active_support/notifications/instrumenter' +require 'active_support/notifications/fanout' + module ActiveSupport # = Notifications # @@ -105,10 +108,6 @@ module ActiveSupport # to log subscribers in a thread. You can use any queue implementation you want. # module Notifications - autoload :Instrumenter, 'active_support/notifications/instrumenter' - autoload :Event, 'active_support/notifications/instrumenter' - autoload :Fanout, 'active_support/notifications/fanout' - @instrumenters = Hash.new { |h,k| h[k] = notifier.listening?(k) } class << self diff --git a/activesupport/lib/active_support/notifications/fanout.rb b/activesupport/lib/active_support/notifications/fanout.rb index 4c1b7b2784..17c99089c1 100644 --- a/activesupport/lib/active_support/notifications/fanout.rb +++ b/activesupport/lib/active_support/notifications/fanout.rb @@ -9,7 +9,7 @@ module ActiveSupport end def subscribe(pattern = nil, block = Proc.new) - subscriber = Subscriber.new(pattern, block) + subscriber = Subscribers.new pattern, block @subscribers << subscriber @listeners_for.clear subscriber @@ -20,6 +20,14 @@ module ActiveSupport @listeners_for.clear end + def start(name, id, payload) + listeners_for(name).each { |s| s.start(name, id, payload) } + end + + def finish(name, id, payload) + listeners_for(name).each { |s| s.finish(name, id, payload) } + end + def publish(name, *args) listeners_for(name).each { |s| s.publish(name, *args) } end @@ -36,23 +44,89 @@ module ActiveSupport def wait end - class Subscriber #:nodoc: - def initialize(pattern, delegate) - @pattern = pattern - @delegate = delegate + module Subscribers # :nodoc: + def self.new(pattern, listener) + if listener.respond_to?(:call) + subscriber = Timed.new pattern, listener + else + subscriber = Evented.new pattern, listener + end + + unless pattern + AllMessages.new(subscriber) + else + subscriber + end end - def publish(message, *args) - @delegate.call(message, *args) + class Evented #:nodoc: + def initialize(pattern, delegate) + @pattern = pattern + @delegate = delegate + end + + def start(name, id, payload) + @delegate.start name, id, payload + end + + def finish(name, id, payload) + @delegate.finish name, id, payload + end + + def subscribed_to?(name) + @pattern === name.to_s + end + + def matches?(subscriber_or_name) + self === subscriber_or_name || + @pattern && @pattern === subscriber_or_name + end end - def subscribed_to?(name) - !@pattern || @pattern === name.to_s + class Timed < Evented + def initialize(pattern, delegate) + @timestack = Hash.new { |h,id| + h[id] = Hash.new { |ids,name| ids[name] = [] } + } + super + end + + def publish(name, *args) + @delegate.call name, *args + end + + def start(name, id, payload) + @timestack[id][name].push Time.now + end + + def finish(name, id, payload) + started = @timestack[id][name].pop + @delegate.call(name, started, Time.now, id, payload) + end end - def matches?(subscriber_or_name) - self === subscriber_or_name || - @pattern && @pattern === subscriber_or_name + class AllMessages # :nodoc: + def initialize(delegate) + @delegate = delegate + end + + def start(name, id, payload) + @delegate.start name, id, payload + end + + def finish(name, id, payload) + @delegate.finish name, id, payload + end + + def publish(name, *args) + @delegate.publish name, *args + end + + def subscribed_to?(name) + true + end + + alias :matches? :=== end end end diff --git a/activesupport/lib/active_support/notifications/instrumenter.rb b/activesupport/lib/active_support/notifications/instrumenter.rb index 547df5c731..58e292c658 100644 --- a/activesupport/lib/active_support/notifications/instrumenter.rb +++ b/activesupport/lib/active_support/notifications/instrumenter.rb @@ -1,5 +1,6 @@ module ActiveSupport module Notifications + # Instrumentors are stored in a thread local. class Instrumenter attr_reader :id @@ -12,15 +13,14 @@ module ActiveSupport # and publish it. Notice that events get sent even if an error occurs # in the passed-in block def instrument(name, payload={}) - started = Time.now - + @notifier.start(name, @id, payload) begin yield rescue Exception => e payload[:exception] = [e.class.name, e.message] raise e ensure - @notifier.publish(name, started, Time.now, @id, payload) + @notifier.finish(name, @id, payload) end end diff --git a/activesupport/lib/active_support/rescuable.rb b/activesupport/lib/active_support/rescuable.rb index 0f4a06468a..85e84bc203 100644 --- a/activesupport/lib/active_support/rescuable.rb +++ b/activesupport/lib/active_support/rescuable.rb @@ -108,7 +108,11 @@ module ActiveSupport when Symbol method(rescuer) when Proc - rescuer.bind(self) + if rescuer.arity == 0 + Proc.new { instance_exec(&rescuer) } + else + Proc.new { |_exception| instance_exec(_exception, &rescuer) } + end end end end diff --git a/activesupport/test/core_ext/duplicable_test.rb b/activesupport/test/core_ext/duplicable_test.rb index 3e54266051..1105353e45 100644 --- a/activesupport/test/core_ext/duplicable_test.rb +++ b/activesupport/test/core_ext/duplicable_test.rb @@ -4,17 +4,25 @@ require 'active_support/core_ext/object/duplicable' require 'active_support/core_ext/numeric/time' class DuplicableTest < ActiveSupport::TestCase - RAISE_DUP = [nil, false, true, :symbol, 1, 2.3, BigDecimal.new('4.56'), 5.seconds] + RAISE_DUP = [nil, false, true, :symbol, 1, 2.3, 5.seconds] YES = ['1', Object.new, /foo/, [], {}, Time.now] NO = [Class.new, Module.new] + begin + bd = BigDecimal.new('4.56') + YES << bd.dup + rescue TypeError + RAISE_DUP << bd + end + + def test_duplicable (RAISE_DUP + NO).each do |v| assert !v.duplicable? end YES.each do |v| - assert v.duplicable? + assert v.duplicable?, "#{v.class} should be duplicable" end (YES + NO).each do |v| diff --git a/activesupport/test/core_ext/proc_test.rb b/activesupport/test/core_ext/proc_test.rb index 690bfd3bf8..c4d5592196 100644 --- a/activesupport/test/core_ext/proc_test.rb +++ b/activesupport/test/core_ext/proc_test.rb @@ -3,10 +3,12 @@ require 'active_support/core_ext/proc' class ProcTests < ActiveSupport::TestCase def test_bind_returns_method_with_changed_self - block = Proc.new { self } - assert_equal self, block.call - bound_block = block.bind("hello") - assert_not_equal block, bound_block - assert_equal "hello", bound_block.call + assert_deprecated do + block = Proc.new { self } + assert_equal self, block.call + bound_block = block.bind("hello") + assert_not_equal block, bound_block + assert_equal "hello", bound_block.call + end end end diff --git a/activesupport/test/notifications/evented_notification_test.rb b/activesupport/test/notifications/evented_notification_test.rb new file mode 100644 index 0000000000..f77a0eb3fa --- /dev/null +++ b/activesupport/test/notifications/evented_notification_test.rb @@ -0,0 +1,67 @@ +require 'abstract_unit' + +module ActiveSupport + module Notifications + class EventedTest < ActiveSupport::TestCase + class Listener + attr_reader :events + + def initialize + @events = [] + end + + def start(name, id, payload) + @events << [:start, name, id, payload] + end + + def finish(name, id, payload) + @events << [:finish, name, id, payload] + end + end + + def test_evented_listener + notifier = Fanout.new + listener = Listener.new + notifier.subscribe 'hi', listener + notifier.start 'hi', 1, {} + notifier.start 'hi', 2, {} + notifier.finish 'hi', 2, {} + notifier.finish 'hi', 1, {} + + assert_equal 4, listener.events.length + assert_equal [ + [:start, 'hi', 1, {}], + [:start, 'hi', 2, {}], + [:finish, 'hi', 2, {}], + [:finish, 'hi', 1, {}], + ], listener.events + end + + def test_evented_listener_no_events + notifier = Fanout.new + listener = Listener.new + notifier.subscribe 'hi', listener + notifier.start 'world', 1, {} + assert_equal 0, listener.events.length + end + + def test_listen_to_everything + notifier = Fanout.new + listener = Listener.new + notifier.subscribe nil, listener + notifier.start 'hello', 1, {} + notifier.start 'world', 1, {} + notifier.finish 'world', 1, {} + notifier.finish 'hello', 1, {} + + assert_equal 4, listener.events.length + assert_equal [ + [:start, 'hello', 1, {}], + [:start, 'world', 1, {}], + [:finish, 'world', 1, {}], + [:finish, 'hello', 1, {}], + ], listener.events + end + end + end +end diff --git a/activesupport/test/ordered_options_test.rb b/activesupport/test/ordered_options_test.rb index 0eee991e20..3526c7a366 100644 --- a/activesupport/test/ordered_options_test.rb +++ b/activesupport/test/ordered_options_test.rb @@ -1,4 +1,5 @@ require 'abstract_unit' +require 'active_support/ordered_options' class OrderedOptionsTest < ActiveSupport::TestCase def test_usage diff --git a/guides/source/ruby_on_rails_guides_guidelines.textile b/guides/source/ruby_on_rails_guides_guidelines.textile index f7d1ed8f65..f3e934d38c 100644 --- a/guides/source/ruby_on_rails_guides_guidelines.textile +++ b/guides/source/ruby_on_rails_guides_guidelines.textile @@ -44,7 +44,7 @@ h3. HTML Guides h4. Generation -To generate all the guides, just +cd+ into the +guides+ directory and execute: +To generate all the guides, just +cd+ into the *+guides+* directory and execute: <plain> bundle exec rake generate_guides diff --git a/railties/lib/rails/commands/console.rb b/railties/lib/rails/commands/console.rb index 86376ac7e6..d7c9e820dc 100644 --- a/railties/lib/rails/commands/console.rb +++ b/railties/lib/rails/commands/console.rb @@ -24,6 +24,9 @@ module Rails OptionParser.new do |opt| opt.banner = "Usage: console [environment] [options]" opt.on('-s', '--sandbox', 'Rollback database modifications on exit.') { |v| options[:sandbox] = v } + opt.on("-e", "--environment=name", String, + "Specifies the environment to run this console under (test/development/production).", + "Default: development") { |v| options[:environment] = v.strip } opt.on("--debugger", 'Enable ruby-debugging for the console.') { |v| options[:debugger] = v } opt.parse!(arguments) end @@ -36,6 +39,14 @@ module Rails options[:sandbox] end + def environment? + options[:environment] + end + + def set_environment! + Rails.env = options[:environment] + end + def debugger? options[:debugger] end @@ -45,6 +56,8 @@ module Rails require_debugger if debugger? + set_environment! if environment? + if sandbox? puts "Loading #{Rails.env} environment in sandbox (Rails #{Rails.version})" puts "Any modifications you make will be rolled back on exit" diff --git a/railties/lib/rails/commands/server.rb b/railties/lib/rails/commands/server.rb index a608693ca4..721a47a974 100644 --- a/railties/lib/rails/commands/server.rb +++ b/railties/lib/rails/commands/server.rb @@ -32,6 +32,11 @@ module Rails opt_parser.parse! args + # Handle's environment like RAILS_ENV=production passed in directly + if index = args.index {|arg| arg.include?("RAILS_ENV")} + options[:environment] ||= args.delete_at(index).split('=').last + end + options[:server] = args.shift options end diff --git a/railties/lib/rails/paths.rb b/railties/lib/rails/paths.rb index b37421c09c..4ac3d88dc2 100644 --- a/railties/lib/rails/paths.rb +++ b/railties/lib/rails/paths.rb @@ -157,7 +157,25 @@ module Rails path = File.expand_path(p, @root.path) if @glob - result.concat Dir[File.join(path, @glob)].sort + if File.directory? path + result.concat expand_dir(path, @glob) + else + # FIXME: I think we can remove this branch, but I'm not sure. + # Say the filesystem has this file: + # + # /tmp/foobar + # + # and someone adds this path: + # + # /tmp/foo + # + # with a glob of "*", then this function will return + # + # /tmp/foobar + # + # We need to figure out if that is desired behavior. + result.concat expand_file(path, @glob) + end else result << path end @@ -177,6 +195,17 @@ module Rails end alias to_a expanded + + private + def expand_file(path, glob) + Dir[File.join(path, glob)].sort + end + + def expand_dir(path, glob) + Dir.chdir(path) do + Dir.glob(@glob).map { |file| File.join path, file }.sort + end + end end end end diff --git a/railties/lib/rails/source_annotation_extractor.rb b/railties/lib/rails/source_annotation_extractor.rb index 9bfc2b16ab..4cd60fdc39 100644 --- a/railties/lib/rails/source_annotation_extractor.rb +++ b/railties/lib/rails/source_annotation_extractor.rb @@ -22,7 +22,7 @@ class SourceAnnotationExtractor # If +options+ has a flag <tt>:tag</tt> the tag is shown as in the example above. # Otherwise the string contains just line and text. def to_s(options={}) - s = "[#{line.to_s.rjust(options[:indent])}]" + s = "[#{line.to_s.rjust(options[:indent])}] " s << "[#{tag}] " if options[:tag] s << text end diff --git a/railties/test/application/rake/notes_test.rb b/railties/test/application/rake/notes_test.rb index b66433f64d..04abf9e3a1 100644 --- a/railties/test/application/rake/notes_test.rb +++ b/railties/test/application/rake/notes_test.rb @@ -28,7 +28,7 @@ module ApplicationTests Dir.chdir(app_path) do output = `bundle exec rake notes` - lines = output.scan(/\[([0-9\s]+)\]/).flatten + lines = output.scan(/\[([0-9\s]+)\](\s)/) assert_match /note in erb/, output assert_match /note in haml/, output @@ -38,8 +38,9 @@ module ApplicationTests assert_equal 5, lines.size - lines.each do |line_number| - assert_equal 4, line_number.size + lines.each do |line| + assert_equal 4, line[0].size + assert_equal ' ', line[1] end end diff --git a/railties/test/commands/console_test.rb b/railties/test/commands/console_test.rb index 01847ae58c..9aa1d68675 100644 --- a/railties/test/commands/console_test.rb +++ b/railties/test/commands/console_test.rb @@ -55,6 +55,25 @@ class Rails::ConsoleTest < ActiveSupport::TestCase assert_match /Loading \w+ environment in sandbox \(Rails/, output end + def test_console_with_environment + app.expects(:sandbox=).with(nil) + FakeConsole.expects(:start) + + start ["-e production"] + + assert_match /production/, output + end + + def test_console_with_rails_environment + app.expects(:sandbox=).with(nil) + FakeConsole.expects(:start) + + start ["RAILS_ENV=production"] + + assert_match /production/, output + end + + def test_console_defaults_to_IRB config = mock("config", :console => nil) app = mock("app", :config => config) diff --git a/railties/test/commands/server_test.rb b/railties/test/commands/server_test.rb new file mode 100644 index 0000000000..8039aec873 --- /dev/null +++ b/railties/test/commands/server_test.rb @@ -0,0 +1,26 @@ +require 'abstract_unit' +require 'rails/commands/server' + +class Rails::ServerTest < ActiveSupport::TestCase + + def test_environment_with_server_option + args = ["thin", "RAILS_ENV=production"] + options = Rails::Server::Options.new.parse!(args) + assert_equal 'production', options[:environment] + assert_equal 'thin', options[:server] + end + + def test_environment_without_server_option + args = ["RAILS_ENV=production"] + options = Rails::Server::Options.new.parse!(args) + assert_equal 'production', options[:environment] + assert_nil options[:server] + end + + def test_server_option_without_environment + args = ["thin"] + options = Rails::Server::Options.new.parse!(args) + assert_nil options[:environment] + assert_equal 'thin', options[:server] + end +end diff --git a/railties/test/generators/migration_generator_test.rb b/railties/test/generators/migration_generator_test.rb index 4e08e5dae1..fd84164340 100644 --- a/railties/test/generators/migration_generator_test.rb +++ b/railties/test/generators/migration_generator_test.rb @@ -41,6 +41,24 @@ class MigrationGeneratorTest < Rails::Generators::TestCase end end + def test_remove_migration_with_indexed_attribute + migration = "remove_title_body_from_posts" + run_generator [migration, "title:string:index", "body:text"] + + assert_migration "db/migrate/#{migration}.rb" do |content| + assert_method :up, content do |up| + assert_match(/remove_column :posts, :title/, up) + assert_match(/remove_column :posts, :body/, up) + end + + assert_method :down, content do |down| + assert_match(/add_column :posts, :title, :string/, down) + assert_match(/add_column :posts, :body, :text/, down) + assert_match(/add_index :posts, :title/, down) + end + end + end + def test_remove_migration_with_attributes migration = "remove_title_body_from_posts" run_generator [migration, "title:string", "body:text"] |