diff options
18 files changed, 125 insertions, 94 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 04a3bf8697..85a83ed7d9 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,27 @@ ## Rails 4.0.0 (unreleased) ## +* Allow setting a symbol as path in scope on routes. This is now allowed: + + scope :api do + resources :users + end + + It is also possible to pass multiple symbols to scope to shorten multiple nested scopes: + + scope :api do + scope :v1 do + resources :users + end + end + + can be rewritten as: + + scope :api, :v1 do + resources :users + end + + *Guillermo Iguaran* + * Fix error when using a non-hash query argument named "params" in `url_for`. Before: diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index d6fe436b68..05cbcf709e 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -644,7 +644,7 @@ module ActionDispatch options = args.extract_options! options = options.dup - options[:path] = args.first if args.first.is_a?(String) + options[:path] = args.flatten.join('/') if args.any? recover = {} options[:constraints] ||= {} diff --git a/actionpack/lib/action_view/renderer/partial_renderer.rb b/actionpack/lib/action_view/renderer/partial_renderer.rb index 36d557e1a3..f5fdf766ad 100644 --- a/actionpack/lib/action_view/renderer/partial_renderer.rb +++ b/actionpack/lib/action_view/renderer/partial_renderer.rb @@ -1,4 +1,3 @@ - module ActionView # = Action View Partials # diff --git a/actionpack/lib/action_view/renderer/renderer.rb b/actionpack/lib/action_view/renderer/renderer.rb index dfef43bc9d..30a0c4be70 100644 --- a/actionpack/lib/action_view/renderer/renderer.rb +++ b/actionpack/lib/action_view/renderer/renderer.rb @@ -33,22 +33,12 @@ module ActionView # Direct accessor to template rendering. def render_template(context, options) #:nodoc: - _template_renderer.render(context, options) + TemplateRenderer.new(@lookup_context).render(context, options) end # Direct access to partial rendering. def render_partial(context, options, &block) #:nodoc: - _partial_renderer.render(context, options, block) - end - - private - - def _template_renderer #:nodoc: - TemplateRenderer.new(@lookup_context) - end - - def _partial_renderer #:nodoc: - PartialRenderer.new(@lookup_context) + PartialRenderer.new(@lookup_context).render(context, options, block) end end end diff --git a/actionpack/lib/action_view/renderer/template_renderer.rb b/actionpack/lib/action_view/renderer/template_renderer.rb index 2a5ea5a711..4d5c5db80c 100644 --- a/actionpack/lib/action_view/renderer/template_renderer.rb +++ b/actionpack/lib/action_view/renderer/template_renderer.rb @@ -41,7 +41,7 @@ module ActionView # Renders the given template. A string representing the layout can be # supplied as well. - def render_template(template, layout_name = nil, locals = {}) #:nodoc: + def render_template(template, layout_name = nil, locals = nil) #:nodoc: view, locals = @view, locals || {} render_with_layout(layout_name, locals) do |layout| diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 34606512dc..0a59d3cf9e 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -370,6 +370,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest scope :path => 'api' do resource :me get '/' => 'mes#index' + scope :v2 do + resource :me, as: 'v2_me' + get '/' => 'mes#index' + end + + scope :v3, :admin do + resource :me, as: 'v3_me' + end end get "(/:username)/followers" => "followers#index" @@ -1467,6 +1475,18 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal 'mes#index', @response.body end + def test_symbol_scope + get '/api/v2/me' + assert_equal 'mes#show', @response.body + assert_equal '/api/v2/me', v2_me_path + + get '/api/v2' + assert_equal 'mes#index', @response.body + + get '/api/v3/admin/me' + assert_equal 'mes#show', @response.body + end + def test_url_generator_for_generic_route get 'whatever/foo/bar' assert_equal 'foo#bar', @response.body diff --git a/activemodel/test/cases/secure_password_test.rb b/activemodel/test/cases/secure_password_test.rb index c7e93370ec..7783bb25d5 100644 --- a/activemodel/test/cases/secure_password_test.rb +++ b/activemodel/test/cases/secure_password_test.rb @@ -5,13 +5,18 @@ require 'models/visitor' require 'models/administrator' class SecurePasswordTest < ActiveModel::TestCase - setup do + ActiveModel::SecurePassword.min_cost = true + @user = User.new @visitor = Visitor.new @oauthed_user = OauthedUser.new end + teardown do + ActiveModel::SecurePassword.min_cost = false + end + test "blank password" do @user.password = @visitor.password = '' assert !@user.valid?(:create), 'user should be invalid' @@ -70,13 +75,16 @@ class SecurePasswordTest < ActiveModel::TestCase end end - test "Password digest cost defaults to bcrypt default cost" do + test "Password digest cost defaults to bcrypt default cost when min_cost is false" do + ActiveModel::SecurePassword.min_cost = false + @user.password = "secret" assert_equal BCrypt::Engine::DEFAULT_COST, @user.password_digest.cost end test "Password digest cost can be set to bcrypt min cost to speed up tests" do ActiveModel::SecurePassword.min_cost = true + @user.password = "secret" assert_equal BCrypt::Engine::MIN_COST, @user.password_digest.cost end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 2b58276461..1a34ed441f 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -5,6 +5,11 @@ *Yves Senn* +* Deprecate calling `Relation#sum` with a block. To perform a calculation over + the array result of the relation, use `to_a.sum(&block)`. + + *Carlos Antonio da Silva* + * Fix postgresql adapter to handle BC timestamps correctly HistoryEvent.create!(:name => "something", :occured_at => Date.new(0) - 5.years) @@ -741,13 +746,6 @@ *Marc-André Lafortune* -* Allow blocks for `count` with `ActiveRecord::Relation`, to work similar as - `Array#count`: - - Person.where("age > 26").count { |person| person.gender == 'female' } - - *Chris Finne & Carlos Antonio da Silva* - * Added support to `CollectionAssociation#delete` for passing `fixnum` or `string` values as record ids. This finds the records responding to the `id` and executes delete on them. diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 651b17920c..d8b6d7a86b 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -234,7 +234,7 @@ module ActiveRecord # others.size | X | X | X # others.length | X | X | X # others.count | X | X | X - # others.sum(args*,&block) | X | X | X + # others.sum(*args) | X | X | X # others.empty? | X | X | X # others.clear | X | X | X # others.delete(other,other,...) | X | X | X diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 862ff201de..1548e68cea 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -161,15 +161,6 @@ module ActiveRecord end end - # Calculate sum using SQL, not Enumerable. - def sum(*args) - if block_given? - scope.sum(*args) { |*block_args| yield(*block_args) } - else - scope.sum(*args) - end - end - # Count all records using SQL. If the +:counter_sql+ or +:finder_sql+ option is set for the # association, it will be used for the query. Otherwise, construct options and pass them with # scope to the target class's +count+. diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 90701938e5..3c03cce838 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -32,21 +32,29 @@ module ActiveRecord protected - # We want to generate the methods via module_eval rather than define_method, - # because define_method is slower on dispatch and uses more memory (because it - # creates a closure). + # We want to generate the methods via module_eval rather than + # define_method, because define_method is slower on dispatch and + # uses more memory (because it creates a closure). # - # But sometimes the database might return columns with characters that are not - # allowed in normal method names (like 'my_column(omg)'. So to work around this - # we first define with the __temp__ identifier, and then use alias method to - # rename it to what we want. - def define_method_attribute(attr_name) + # But sometimes the database might return columns with + # characters that are not allowed in normal method names (like + # 'my_column(omg)'. So to work around this we first define with + # the __temp__ identifier, and then use alias method to rename + # it to what we want. + # + # We are also defining a constant to hold the frozen string of + # the attribute name. Using a constant means that we do not have + # to allocate an object on each call to the attribute method. + # Making it frozen means that it doesn't get duped when used to + # key the @attributes_cache in read_attribute. + def define_method_attribute(name) + safe_name = name.unpack('h*').first generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1 - def __temp__ - read_attribute('#{attr_name}') { |n| missing_attribute(n, caller) } + def __temp__#{safe_name} + read_attribute(AttrNames::ATTR_#{safe_name}) { |n| missing_attribute(n, caller) } end - alias_method '#{attr_name}', :__temp__ - undef_method :__temp__ + alias_method #{name.inspect}, :__temp__#{safe_name} + undef_method :__temp__#{safe_name} STR end diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index fa9097db1f..cd33494cc3 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -9,15 +9,19 @@ module ActiveRecord module ClassMethods protected - def define_method_attribute=(attr_name) - if attr_name =~ ActiveModel::AttributeMethods::NAME_COMPILABLE_REGEXP - generated_attribute_methods.module_eval("def #{attr_name}=(new_value); write_attribute('#{attr_name}', new_value); end", __FILE__, __LINE__) - else - generated_attribute_methods.send(:define_method, "#{attr_name}=") do |new_value| - write_attribute(attr_name, new_value) - end + + # See define_method_attribute in read.rb for an explanation of + # this code. + def define_method_attribute=(name) + safe_name = name.unpack('h*').first + generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1 + def __temp__#{safe_name}=(value) + write_attribute(AttrNames::ATTR_#{safe_name}, value) end - end + alias_method #{(name + '=').inspect}, :__temp__#{safe_name}= + undef_method :__temp__#{safe_name}= + STR + end end # Updates the attribute identified by <tt>attr_name</tt> with the diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 957027c1ee..94c6684700 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -83,7 +83,12 @@ module ActiveRecord @attribute_methods_mutex = Mutex.new # force attribute methods to be higher in inheritance hierarchy than other generated methods - generated_attribute_methods + generated_attribute_methods.const_set(:AttrNames, Module.new { + def self.const_missing(name) + const_set(name, [name.to_s.sub(/ATTR_/, '')].pack('h*').freeze) + end + }) + generated_feature_methods end diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 741f94f777..72b035a023 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -1,3 +1,5 @@ +require 'active_support/deprecation' + module ActiveRecord module Calculations # Count the records. @@ -13,16 +15,9 @@ module ActiveRecord # # Person.count(:age, distinct: true) # # => counts the number of different age values - # - # Person.where("age > 26").count { |person| person.gender == 'female' } - # # => queries people where "age > 26" then count the loaded results filtering by gender def count(column_name = nil, options = {}) - if block_given? - self.to_a.count { |item| yield item } - else - column_name, options = nil, column_name if column_name.is_a?(Hash) - calculate(:count, column_name, options) - end + column_name, options = nil, column_name if column_name.is_a?(Hash) + calculate(:count, column_name, options) end # Calculates the average value on a given column. Returns +nil+ if there's @@ -56,13 +51,13 @@ module ActiveRecord # +calculate+ for examples with options. # # Person.sum('age') # => 4562 - # # => returns the total sum of all people's age - # - # Person.where('age > 100').sum { |person| person.age - 100 } - # # queries people where "age > 100" then perform a sum calculation with the block returns def sum(*args) if block_given? - self.to_a.sum(*args) { |item| yield item } + ActiveSupport::Deprecation.warn( + "Calling #sum with a block is deprecated and will be removed in Rails 4.1. " \ + "If you want to perform sum calculation over the array of elements, use `to_a.sum(&block)`." + ) + self.to_a.sum(*args) {|*block_args| yield(*block_args)} else calculate(:sum, *args) end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 989d08b3e9..2ded97582d 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1666,6 +1666,12 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_deprecated { klass.has_many :foo, :counter_sql => 'lol' } end + test "sum calculation with block for array compatibility is deprecated" do + assert_deprecated do + posts(:welcome).comments.sum { |c| c.id } + end + end + test "has many associations on new records use null relations" do post = Post.new diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 65d28ea028..5cb7eabf0e 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -383,30 +383,16 @@ class CalculationsTest < ActiveRecord::TestCase Company.where(:type => "Firm").from('companies').count(:type) end - def test_count_with_block_acts_as_array - accounts = Account.where('id > 0') - assert_equal Account.count, accounts.count { true } - assert_equal 0, accounts.count { false } - assert_equal Account.where('credit_limit > 50').size, accounts.count { |account| account.credit_limit > 50 } - assert_equal Account.count, Account.count { true } - assert_equal 0, Account.count { false } - end - - def test_sum_with_block_acts_as_array - accounts = Account.where('id > 0') - assert_equal Account.sum(:credit_limit), accounts.sum { |account| account.credit_limit } - assert_equal Account.sum(:credit_limit) + Account.count, accounts.sum{ |account| account.credit_limit + 1 } - assert_equal 0, accounts.sum { |account| 0 } - end - def test_sum_with_from_option assert_equal Account.sum(:credit_limit), Account.from('accounts').sum(:credit_limit) assert_equal Account.where("credit_limit > 50").sum(:credit_limit), Account.where("credit_limit > 50").from('accounts').sum(:credit_limit) end - def test_sum_array_compatibility - assert_equal Account.sum(:credit_limit), Account.sum(&:credit_limit) + def test_sum_array_compatibility_deprecation + assert_deprecated do + assert_equal Account.sum(:credit_limit), Account.sum(&:credit_limit) + end end def test_average_with_from_option diff --git a/activerecord/test/cases/migration/create_join_table_test.rb b/activerecord/test/cases/migration/create_join_table_test.rb index cd1b0e8b47..f262bbaad7 100644 --- a/activerecord/test/cases/migration/create_join_table_test.rb +++ b/activerecord/test/cases/migration/create_join_table_test.rb @@ -35,6 +35,12 @@ module ActiveRecord assert_equal %w(artist_id music_id), connection.columns(:artists_musics).map(&:name).sort end + def test_create_join_table_with_symbol_and_string + connection.create_join_table :artists, 'musics' + + assert_equal %w(artist_id music_id), connection.columns(:artists_musics).map(&:name).sort + end + def test_create_join_table_with_the_proper_order connection.create_join_table :videos, :musics diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index b77da39e29..c155f29973 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -755,11 +755,4 @@ class CopyMigrationsTest < ActiveRecord::TestCase ensure clear end - - def test_create_join_table_with_symbol_and_string - connection.create_join_table :artists, 'musics' - - assert_equal %w(artist_id music_id), connection.columns(:artists_musics).map(&:name).sort - end - end |