diff options
Diffstat (limited to 'activerecord')
22 files changed, 191 insertions, 55 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index aa84dacf07..a661a44f1f 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,11 @@ ## Rails 4.0.0 (unreleased) ## +* Added default order to `first` to assure consistent results among + diferent database engines. Introduced `take` as a replacement to + the old behavior of `first`. + + *Marcelo Silveira* + * Added an :index option to automatically create indexes for references and belongs_to statements in migrations. @@ -580,19 +586,6 @@ a URI that specifies the connection configuration. For example: ActiveRecord::Base.establish_connection 'postgres://localhost/foo' -* Active Record's dynamic finder will now raise the error if you passing in less number of arguments than what you call in method signature. - - So if you were doing this and expecting the second argument to be nil: - - User.find_by_username_and_group("sikachu") - - You'll now get `ArgumentError: wrong number of arguments (1 for 2).` You'll then have to do this: - - User.find_by_username_and_group("sikachu", nil) - - *Prem Sichanugrist* - - ## Rails 3.1.0 (August 30, 2011) ## * Add a proxy_association method to association proxies, which can be called by association diff --git a/activerecord/Rakefile b/activerecord/Rakefile index 4090293b56..7feb0b75a0 100755..100644 --- a/activerecord/Rakefile +++ b/activerecord/Rakefile @@ -1,4 +1,3 @@ -#!/usr/bin/env rake require 'rake/testtask' require 'rake/packagetask' require 'rubygems/package_task' diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index ad029d1101..50d16b16a9 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -81,7 +81,7 @@ module ActiveRecord def method_missing(method, *args, &block) match = DynamicFinderMatch.match(method) if match && match.instantiator? - send(:find_or_instantiator_by_attributes, match, match.attribute_names, *args) do |r| + scoped.send(:find_or_instantiator_by_attributes, match, match.attribute_names, *args) do |r| proxy_association.send :set_owner_attributes, r proxy_association.send :add_to_target, r yield(r) if block_given? @@ -101,7 +101,7 @@ module ActiveRecord end else - scoped.readonly(nil).send(method, *args, &block) + scoped.readonly(nil).public_send(method, *args, &block) end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index fc3906d395..189985b671 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -304,21 +304,22 @@ module ActiveRecord #:nodoc: # (or a bad spelling of an existing one). # * AssociationTypeMismatch - The object assigned to the association wasn't of the type # specified in the association definition. - # * SerializationTypeMismatch - The serialized object wasn't of the class specified as the second parameter. + # * AttributeAssignmentError - An error occurred while doing a mass assignment through the + # <tt>attributes=</tt> method. + # You can inspect the +attribute+ property of the exception object to determine which attribute + # triggered the error. # * ConnectionNotEstablished - No connection has been established. Use <tt>establish_connection</tt> # before querying. - # * RecordNotFound - No record responded to the +find+ method. Either the row with the given ID doesn't exist - # or the row didn't meet the additional restrictions. Some +find+ calls do not raise this exception to signal - # nothing was found, please check its documentation for further details. - # * StatementInvalid - The database server rejected the SQL statement. The precise error is added in the message. # * MultiparameterAssignmentErrors - Collection of errors that occurred during a mass assignment using the # <tt>attributes=</tt> method. The +errors+ property of this exception contains an array of # AttributeAssignmentError # objects that should be inspected to determine which attributes triggered the errors. - # * AttributeAssignmentError - An error occurred while doing a mass assignment through the - # <tt>attributes=</tt> method. - # You can inspect the +attribute+ property of the exception object to determine which attribute - # triggered the error. + # * RecordInvalid - raised by save! and create! when the record is invalid. + # * RecordNotFound - No record responded to the +find+ method. Either the row with the given ID doesn't exist + # or the row didn't meet the additional restrictions. Some +find+ calls do not raise this exception to signal + # nothing was found, please check its documentation for further details. + # * SerializationTypeMismatch - The serialized object wasn't of the class specified as the second parameter. + # * StatementInvalid - The database server rejected the SQL statement. The precise error is added in the message. # # *Note*: The attributes listed are class-level attributes (accessible from both the class and instance level). # So it's possible to assign a logger to the class through <tt>Base.logger=</tt> which will then be used by all diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 3546873550..f0b6ae2b7d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -408,7 +408,7 @@ module ActiveRecord # t.remove(:qualification) # t.remove(:qualification, :experience) def remove(*column_names) - @base.remove_column(@table_name, column_names) + @base.remove_column(@table_name, *column_names) end # Removes the given index from the table. diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 30a4f9aa35..e7a4f061fd 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -618,8 +618,6 @@ module ActiveRecord end def columns_for_remove(table_name, *column_names) - column_names = column_names.flatten - raise ArgumentError.new("You must specify at least one column name. Example: remove_column(:people, :first_name)") if column_names.blank? column_names.map {|column_name| quote_column_name(column_name) } end diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb index b7e1513422..9af8e46120 100644 --- a/activerecord/lib/active_record/connection_adapters/column.rb +++ b/activerecord/lib/active_record/connection_adapters/column.rb @@ -95,7 +95,7 @@ module ActiveRecord case type when :string, :text then value - when :integer then value.to_i rescue value ? 1 : 0 + when :integer then value.to_i when :float then value.to_f when :decimal then klass.value_to_decimal(value) when :datetime, :timestamp then klass.string_to_time(value) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 273c165084..68cf495025 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -1233,7 +1233,10 @@ module ActiveRecord # Construct a clean list of column names from the ORDER BY clause, removing # any ASC/DESC modifiers - order_columns = orders.collect { |s| s.gsub(/\s+(ASC|DESC)\s*(NULLS\s+(FIRST|LAST)\s*)?/i, '') } + order_columns = orders.collect do |s| + s = s.to_sql unless s.is_a?(String) + s.gsub(/\s+(ASC|DESC)\s*(NULLS\s+(FIRST|LAST)\s*)?/i, '') + end order_columns.delete_if { |c| c.blank? } order_columns = order_columns.zip((0...order_columns.size).to_a).map { |s,i| "#{s} AS alias_#{i}" } diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 83f75e3505..44e407a561 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -446,7 +446,7 @@ module ActiveRecord def remove_column(table_name, *column_names) #:nodoc: raise ArgumentError.new("You must specify at least one column name. Example: remove_column(:people, :first_name)") if column_names.empty? - column_names.flatten.each do |column_name| + column_names.each do |column_name| alter_table(table_name) do |definition| definition.columns.delete(definition[column_name]) end diff --git a/activerecord/lib/active_record/explain.rb b/activerecord/lib/active_record/explain.rb index 01cacf6153..313fdb3487 100644 --- a/activerecord/lib/active_record/explain.rb +++ b/activerecord/lib/active_record/explain.rb @@ -70,7 +70,7 @@ module ActiveRecord # the threshold is set to 0. # # As the name of the method suggests this only applies to automatic - # EXPLAINs, manual calls to +ActiveRecord::Relation#explain+ run. + # EXPLAINs, manual calls to <tt>ActiveRecord::Relation#explain</tt> run. def silence_auto_explain current = Thread.current original, current[:available_queries_for_explain] = current[:available_queries_for_explain], false diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index 95565b503a..4d8283bcff 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -3,7 +3,7 @@ require 'active_support/deprecation' module ActiveRecord module Querying - delegate :find, :first, :first!, :last, :last!, :all, :exists?, :any?, :many?, :to => :scoped + delegate :find, :take, :take!, :first, :first!, :last, :last!, :all, :exists?, :any?, :many?, :to => :scoped delegate :first_or_create, :first_or_create!, :first_or_initialize, :to => :scoped delegate :find_by, :find_by!, :to => :scoped delegate :destroy, :destroy_all, :delete, :delete_all, :update, :update_all, :to => :scoped @@ -11,7 +11,7 @@ module ActiveRecord delegate :select, :group, :order, :except, :reorder, :limit, :offset, :joins, :where, :preload, :eager_load, :includes, :from, :lock, :readonly, :having, :create_with, :uniq, :references, :none, :to => :scoped - delegate :count, :average, :minimum, :maximum, :sum, :calculate, :pluck, :to => :scoped + delegate :count, :average, :minimum, :maximum, :sum, :calculate, :pluck, :ids, :to => :scoped # Executes a custom SQL query against your database and returns all the results. The results will # be returned as an array with columns requested encapsulated as attributes of the model you call diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 333d31d8a3..779e052e3c 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -390,6 +390,8 @@ module ActiveRecord # If you need to destroy dependent associations or call your <tt>before_*</tt> or # +after_destroy+ callbacks, use the +destroy_all+ method instead. def delete_all(conditions = nil) + raise ActiveRecordError.new("delete_all doesn't support limit scope") if self.limit_value + if conditions where(conditions).delete_all else diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index db894c8aa9..f388b75c05 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -139,6 +139,16 @@ module ActiveRecord end end + # Pluck all the ID's for the relation using the table's primary key + # + # Examples: + # + # Person.ids # SELECT people.id FROM people + # Person.joins(:companies).ids # SELECT people.id FROM PEOPLE INNER JOIN companies ON companies.person_id = people.id + def ids + pluck primary_key + end + private def perform_calculation(operation, column_name, options = {}) diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 416b55f5c5..3c9c9c4e84 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -60,6 +60,28 @@ module ActiveRecord where(*args).first! end + # Gives a record (or N records if a parameter is supplied) without any implied + # order. The order will depend on the database implementation. + # If an order is supplied it will be respected. + # + # Examples: + # + # Person.take # returns an object fetched by SELECT * FROM people + # Person.take(5) # returns 5 objects fetched by SELECT * FROM people LIMIT 5 + # Person.where(["name LIKE '%?'", name]).take + def take(limit = nil) + limit ? limit(limit).to_a : find_take + end + + # Same as +take+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record + # is found. Note that <tt>take!</tt> accepts no arguments. + def take! + take or raise RecordNotFound + end + + # Find the first record (or first N records if a parameter is supplied). + # If no order is defined it will order by primary key. + # # Examples: # # Person.first # returns the first object fetched by SELECT * FROM people @@ -67,7 +89,15 @@ module ActiveRecord # Person.where(["user_name = :u", { :u => user_name }]).first # Person.order("created_on DESC").offset(5).first def first(limit = nil) - limit ? limit(limit).to_a : find_first + if limit + if order_values.empty? && primary_key + order(arel_table[primary_key].asc).limit(limit).to_a + else + limit(limit).to_a + end + else + find_first + end end # Same as +first+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record @@ -76,6 +106,9 @@ module ActiveRecord first or raise RecordNotFound end + # Find the last record (or last N records if a parameter is supplied). + # If no order is defined it will order by primary key. + # # Examples: # # Person.last # returns the last object fetched by SELECT * FROM people @@ -83,8 +116,8 @@ module ActiveRecord # Person.order("created_on DESC").offset(5).last def last(limit = nil) if limit - if order_values.empty? - order("#{primary_key} DESC").limit(limit).reverse + if order_values.empty? && primary_key + order(arel_table[primary_key].desc).limit(limit).reverse else to_a.last(limit) end @@ -217,8 +250,11 @@ module ActiveRecord if match.bang? && result.blank? raise RecordNotFound, "Couldn't find #{@klass.name} with #{conditions.to_a.collect {|p| p.join(' = ')}.join(', ')}" else - yield(result) if block_given? - result + if block_given? && result + yield(result) + else + result + end end end @@ -312,11 +348,24 @@ module ActiveRecord end end + def find_take + if loaded? + @records.take(1).first + else + @take ||= limit(1).to_a.first + end + end + def find_first if loaded? @records.first else - @first ||= limit(1).to_a[0] + @first ||= + if order_values.empty? && primary_key + order(arel_table[primary_key].asc).limit(1).to_a.first + else + limit(1).to_a.first + end end end @@ -328,7 +377,7 @@ module ActiveRecord if offset_value || limit_value to_a.last else - reverse_order.limit(1).to_a[0] + reverse_order.limit(1).to_a.first end 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 a98e5b115c..f74fe42dc2 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1703,4 +1703,9 @@ class HasManyAssociationsTest < ActiveRecord::TestCase ensure ActiveRecord::Base.dependent_restrict_raises = option_before end + + def test_collection_association_with_private_kernel_method + firm = companies(:first_firm) + assert_equal [accounts(:signals37)], firm.accounts.open + end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 8a4ce5e963..35e0045e4c 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -125,20 +125,13 @@ class BasicsTest < ActiveRecord::TestCase unless current_adapter?(:PostgreSQLAdapter,:OracleAdapter,:SQLServerAdapter) def test_limit_with_comma - assert_nothing_raised do - Topic.limit("1,2").all - end + assert Topic.limit("1,2").all end end def test_limit_without_comma - assert_nothing_raised do - assert_equal 1, Topic.limit("1").all.length - end - - assert_nothing_raised do - assert_equal 1, Topic.limit(1).all.length - end + assert_equal 1, Topic.limit("1").all.length + assert_equal 1, Topic.limit(1).all.length end def test_invalid_limit @@ -175,6 +168,24 @@ class BasicsTest < ActiveRecord::TestCase assert Topic.table_exists? end + def test_finder_block + t = Topic.first + found = nil + Topic.find_by_id(t.id) { |f| found = f } + assert_equal t, found + end + + def test_finder_block_nothing_found + bad_id = Topic.maximum(:id) + 1 + assert_nil Topic.find_by_id(bad_id) { |f| raise } + end + + def test_find_returns_block_value + t = Topic.first + x = Topic.find_by_id(t.id) { |f| "hi mom!" } + assert_equal "hi mom!", x + end + def test_preserving_date_objects if current_adapter?(:SybaseAdapter) # Sybase ctlib does not (yet?) support the date type; use datetime instead. diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 2e566ff873..e096585f62 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -465,4 +465,8 @@ class CalculationsTest < ActiveRecord::TestCase Company.create!(:name => "test", :contracts => [Contract.new(:developer_id => 7)]) assert_equal [7], Company.joins(:contracts).pluck(:developer_id) end + + def test_plucks_with_ids + assert_equal Company.all.map(&:id).sort, Company.ids.sort + end end diff --git a/activerecord/test/cases/column_test.rb b/activerecord/test/cases/column_test.rb index ccc57cb876..4fcf8a33a4 100644 --- a/activerecord/test/cases/column_test.rb +++ b/activerecord/test/cases/column_test.rb @@ -24,6 +24,30 @@ module ActiveRecord assert !column.type_cast('off') assert !column.type_cast('OFF') end + + def test_type_cast_integer + column = Column.new("field", nil, "integer") + assert_equal 1, column.type_cast(1) + assert_equal 1, column.type_cast('1') + assert_equal 1, column.type_cast('1ignore') + assert_equal 0, column.type_cast('bad1') + assert_equal 0, column.type_cast('bad') + assert_equal 1, column.type_cast(1.7) + assert_nil column.type_cast(nil) + end + + def test_type_cast_non_integer_to_integer + column = Column.new("field", nil, "integer") + assert_raises(NoMethodError) do + column.type_cast([]) + end + assert_raises(NoMethodError) do + column.type_cast(true) + end + assert_raises(NoMethodError) do + column.type_cast(false) + end + end end end end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 29469c42ed..54801bd101 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -143,6 +143,26 @@ class FinderTest < ActiveRecord::TestCase assert_equal [Account], accounts.collect(&:class).uniq end + def test_take + assert_equal topics(:first), Topic.take + end + + def test_take_failing + assert_nil Topic.where("title = 'This title does not exist'").take + end + + def test_take_bang_present + assert_nothing_raised do + assert_equal topics(:second), Topic.where("title = 'The Second Topic of the day'").take! + end + end + + def test_take_bang_missing + assert_raises ActiveRecord::RecordNotFound do + Topic.where("title = 'This title does not exist'").take! + end + end + def test_first assert_equal topics(:second).title, Topic.where("title = 'The Second Topic of the day'").first.title end @@ -163,6 +183,12 @@ class FinderTest < ActiveRecord::TestCase end end + def test_first_have_primary_key_order_by_default + expected = topics(:first) + expected.touch # PostgreSQL changes the default order if no order clause is used + assert_equal expected, Topic.first + end + def test_model_class_responds_to_first_bang assert Topic.first! Topic.delete_all @@ -191,7 +217,8 @@ class FinderTest < ActiveRecord::TestCase end end - def test_first_and_last_with_integer_should_use_sql_limit + def test_take_and_first_and_last_with_integer_should_use_sql_limit + assert_sql(/LIMIT 3|ROWNUM <= 3/) { Topic.take(3).entries } assert_sql(/LIMIT 2|ROWNUM <= 2/) { Topic.first(2).entries } assert_sql(/LIMIT 5|ROWNUM <= 5/) { Topic.last(5).entries } end @@ -212,7 +239,8 @@ class FinderTest < ActiveRecord::TestCase assert_no_match(/LIMIT/, query.first) end - def test_first_and_last_with_integer_should_return_an_array + def test_take_and_first_and_last_with_integer_should_return_an_array + assert_kind_of Array, Topic.take(5) assert_kind_of Array, Topic.first(5) assert_kind_of Array, Topic.last(5) end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index e71b84a3a5..f788690b0d 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -588,14 +588,14 @@ class ChangeTableMigrationsTest < ActiveRecord::TestCase def test_remove_drops_single_column with_change_table do |t| - @connection.expects(:remove_column).with(:delete_me, [:bar]) + @connection.expects(:remove_column).with(:delete_me, :bar) t.remove :bar end end def test_remove_drops_multiple_columns with_change_table do |t| - @connection.expects(:remove_column).with(:delete_me, [:bar, :baz]) + @connection.expects(:remove_column).with(:delete_me, :bar, :baz) t.remove :bar, :baz end end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 4b8b57bac2..3ef357e297 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -688,6 +688,10 @@ class RelationTest < ActiveRecord::TestCase assert davids.loaded? end + def test_delete_all_limit_error + assert_raises(ActiveRecord::ActiveRecordError) { Author.limit(10).delete_all } + end + def test_select_argument_error assert_raises(ArgumentError) { Developer.select } end diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index fbdfaa2c29..7b993d5a2c 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -198,6 +198,11 @@ class Account < ActiveRecord::Base @destroyed_account_ids ||= Hash.new { |h,k| h[k] = [] } end + # Test private kernel method through collection proxy using has_many. + def self.open + where('firm_name = ?', '37signals') + end + before_destroy do |account| if account.firm Account.destroyed_account_ids[account.firm.id] << account.id |