diff options
author | Aaron Patterson <aaron.patterson@gmail.com> | 2013-05-21 11:02:15 -0700 |
---|---|---|
committer | Aaron Patterson <aaron.patterson@gmail.com> | 2013-05-21 11:02:15 -0700 |
commit | 68a97e81187358ae8a4d59499d9afc2f09d35d24 (patch) | |
tree | 93b9cd36021c746545de70d2f658b446fa71d910 /activerecord | |
parent | eb2bb7d56f1eefb96a99321b18ed7b4b7ef1d4d6 (diff) | |
parent | ac70ec64138765ddd2a7c5e0130243e6df98cf2d (diff) | |
download | rails-68a97e81187358ae8a4d59499d9afc2f09d35d24.tar.gz rails-68a97e81187358ae8a4d59499d9afc2f09d35d24.tar.bz2 rails-68a97e81187358ae8a4d59499d9afc2f09d35d24.zip |
Merge branch 'master' into experiment
* master: (49 commits)
avoid creating a set if no where values are removed
remove bind values for where clauses that were removed
push partitioning up so bind elimination can get the removed wheres
push partion logic down and initialization logic up
the rake task `db:test:prepare` needs to load the configuration
In batches test @total was assigned but not used. Use it in tests instead of Post.count
partition the where values so we can access the removed ones
eliminate some conditionals
change method name to reflect what it actually does.
save the where values in variables so we don't need to look them up all the time
pass where values to the helper function rather than rely on internal state
Spelling correction in Upgrading Guide
Add has_named_route? to the mapper API
No need CHANGELOG entry for a test fix
Fix wrong `case_sensitive` in uniqueness validity test
Fix typo in test name and documentation
Missing ending ``` at 14.2 Merging of scopes
copy edits[ci skip]
Revert "Corrected documentation and added some more for the classify method in inflectors"
Revert "Changed the CHANGELOG for active_support and improved the doc for inflector method classify"
...
Diffstat (limited to 'activerecord')
20 files changed, 115 insertions, 48 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 858c33c4e5..19d2195bca 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,4 +1,4 @@ -* Also support extentions in PostgreSQL 9.1. This feature has been supported since 9.1. +* Also support extensions in PostgreSQL 9.1. This feature has been supported since 9.1. *kennyj* diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index 5c37f42794..6a3658f328 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -1,3 +1,15 @@ +# This is the parent Association class which defines the variables +# used by all associations. +# +# The hierarchy is defined as follows: +# Association +# - SingularAssociation +# - BelongsToAssociation +# - HasOneAssociation +# - CollectionAssociation +# - HasManyAssociation +# - HasAndBelongsToManyAssociation + module ActiveRecord::Associations::Builder class Association #:nodoc: class << self @@ -58,6 +70,13 @@ module ActiveRecord::Associations::Builder def validate_options options.assert_valid_keys(valid_options) end + + # Defines the setter and getter methods for the association + # class Post < ActiveRecord::Base + # has_many :comments + # end + # + # Post.first.comments and Post.first.comments= methods are defined by this method... def define_accessors define_readers diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb index fdead16761..9c6690b721 100644 --- a/activerecord/lib/active_record/associations/builder/collection_association.rb +++ b/activerecord/lib/active_record/associations/builder/collection_association.rb @@ -1,3 +1,5 @@ +# This class is inherited by the has_many and has_many_and_belongs_to_many association classes + require 'active_record/associations' module ActiveRecord::Associations::Builder @@ -66,6 +68,8 @@ module ActiveRecord::Associations::Builder model.send("#{full_callback_name}=", Array(options[callback_name.to_sym])) end + # Defines the setter and getter methods for the collection_singular_ids. + def define_readers super diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb index f06426a09d..96ccbeb8a3 100644 --- a/activerecord/lib/active_record/associations/builder/singular_association.rb +++ b/activerecord/lib/active_record/associations/builder/singular_association.rb @@ -1,3 +1,5 @@ +# This class is inherited by the has_one and belongs_to association classes + module ActiveRecord::Associations::Builder class SingularAssociation < Association #:nodoc: def valid_options @@ -13,6 +15,8 @@ module ActiveRecord::Associations::Builder define_constructors if constructable? end + # Defines the (build|create)_association methods for belongs_to or has_one association + def define_constructors mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 def build_#{name}(*args, &block) diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 2a00ac1386..efd7ecb97c 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -237,11 +237,11 @@ module ActiveRecord end end - # Destroy +records+ and remove them from this association calling - # +before_remove+ and +after_remove+ callbacks. + # Deletes the +records+ and removes them from this association calling + # +before_remove+ , +after_remove+ , +before_destroy+ and +after_destroy+ callbacks. # - # Note that this method will _always_ remove records from the database - # ignoring the +:dependent+ option. + # Note that this method removes records from the database ignoring the + # +:dependent+ option. def destroy(*records) records = find(records) if records.any? { |record| record.kind_of?(Fixnum) || record.kind_of?(String) } delete_or_destroy(records, :destroy) diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 71b64de5ea..e82c195335 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -422,9 +422,9 @@ module ActiveRecord @association.delete_all end - # Deletes the records of the collection directly from the database. - # This will _always_ remove the records ignoring the +:dependent+ - # option. + # Deletes the records of the collection directly from the database + # ignoring the +:dependent+ option. It invokes +before_remove+, + # +after_remove+ , +before_destroy+ and +after_destroy+ callbacks. # # class Person < ActiveRecord::Base # has_many :pets diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index f78ccb01aa..3d85898c41 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -14,7 +14,7 @@ module ActiveRecord # 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 # this method from. If you call <tt>Product.find_by_sql</tt> then the results will be returned in - # a Product object with the attributes you specified in the SQL query. + # a +Product+ object with the attributes you specified in the SQL query. # # If you call a complicated SQL query which spans multiple tables the columns specified by the # SELECT will be attributes of the model, whether or not they are columns of the corresponding @@ -29,9 +29,10 @@ module ActiveRecord # Post.find_by_sql "SELECT p.title, c.author FROM posts p, comments c WHERE p.id = c.post_id" # # => [#<Post:0x36bff9c @attributes={"title"=>"Ruby Meetup", "first_name"=>"Quentin"}>, ...] # - # # You can use the same string replacement techniques as you can with ActiveRecord#find + # You can use the same string replacement techniques as you can with <tt>ActiveRecord::QueryMethods#where</tt>: + # # Post.find_by_sql ["SELECT title FROM posts WHERE author = ? AND created > ?", author_id, start_date] - # # => [#<Post:0x36bff9c @attributes={"title"=>"The Cheap Man Buys Twice"}>, ...] + # Post.find_by_sql ["SELECT body FROM comments WHERE author = :user_id OR approved_by = :user_id", { :user_id => user_id }] def find_by_sql(sql, binds = []) result_set = connection.select_all(sanitize_sql(sql), "#{name} Load", binds) column_types = {} diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 434af3c5f8..f69e9b2217 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -360,7 +360,7 @@ db_namespace = namespace :db do end # desc 'Check for pending migrations and load the test schema' - task :prepare do + task :prepare => :load_config do unless ActiveRecord::Base.configurations.blank? db_namespace['test:load'].invoke end diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index eea43aff0e..c114ea0c0d 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -99,8 +99,15 @@ module ActiveRecord end def merge_multi_values - relation.where_values = merged_wheres - relation.bind_values = merged_binds + lhs_wheres = relation.where_values + rhs_wheres = values[:where] || [] + lhs_binds = relation.bind_values + rhs_binds = values[:bind] || [] + + removed, kept = partition_overwrites(lhs_wheres, rhs_wheres) + + relation.where_values = kept + rhs_wheres + relation.bind_values = filter_binds(lhs_binds, removed) + rhs_binds if values[:reordering] # override any order specified in the original relation @@ -123,33 +130,27 @@ module ActiveRecord end end - def merged_binds - if values[:bind] - (relation.bind_values + values[:bind]).uniq(&:first) - else - relation.bind_values - end - end - - def merged_wheres - values[:where] ||= [] + def filter_binds(lhs_binds, removed_wheres) + return lhs_binds if removed_wheres.empty? - if values[:where].empty? || relation.where_values.empty? - relation.where_values + values[:where] - else - sanitized_wheres + values[:where] - end + set = Set.new removed_wheres.map { |x| x.left.name } + lhs_binds.dup.delete_if { |col,_| set.include? col.name } end # Remove equalities from the existing relation with a LHS which is # present in the relation being merged in. - def sanitized_wheres - seen = Set.new - values[:where].each do |w| - seen << w.left if w.respond_to?(:operator) && w.operator == :== + # returns [things_to_remove, things_to_keep] + def partition_overwrites(lhs_wheres, rhs_wheres) + if lhs_wheres.empty? || rhs_wheres.empty? + return [[], lhs_wheres] + end + + nodes = rhs_wheres.find_all do |w| + w.respond_to?(:operator) && w.operator == :== end + seen = Set.new(nodes) { |node| node.left } - relation.where_values.reject do |w| + lhs_wheres.partition do |w| w.respond_to?(:operator) && w.operator == :== && seen.include?(w.left) end end diff --git a/activerecord/lib/rails/generators/active_record/model/model_generator.rb b/activerecord/lib/rails/generators/active_record/model/model_generator.rb index 40e134e626..7e8d68ce69 100644 --- a/activerecord/lib/rails/generators/active_record/model/model_generator.rb +++ b/activerecord/lib/rails/generators/active_record/model/model_generator.rb @@ -12,6 +12,9 @@ module ActiveRecord class_option :parent, :type => :string, :desc => "The parent class for the generated model" class_option :indexes, :type => :boolean, :default => true, :desc => "Add indexes for references and belongs_to columns" + + # creates the migration file for the model. + def create_migration_file return unless options[:migration] && options[:parent].nil? attributes.each { |a| a.attr_options.delete(:index) if a.reference? && !a.has_index? } if options[:indexes] == false @@ -39,6 +42,7 @@ module ActiveRecord protected + # Used by the migration template to determine the parent name of the model def parent_class_name options[:parent] || "ActiveRecord::Base" end diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index 83aedfb61a..1c2d5cdaf2 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -30,7 +30,7 @@ module ActiveRecord assert @conn.valid_type?(column.type) end - # sqlite databses should be able to support any type and not + # sqlite databases should be able to support any type and not # just the ones mentioned in the native_database_types. # Therefore test_invalid column should always return true # even if the type is not valid. diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 0f2d22a4a2..c3b728296e 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -217,7 +217,7 @@ class AssociationProxyTest < ActiveRecord::TestCase assert_equal post.body, "More cool stuff!" end - def test_reload_returns_assocition + def test_reload_returns_association david = developers(:david) assert_nothing_raised do assert_equal david.projects, david.projects.reload.reload diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index d20ccaa5ca..15d536daf5 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1236,7 +1236,7 @@ class BasicsTest < ActiveRecord::TestCase def test_inspect_instance topic = topics(:first) - assert_equal %(#<Topic id: 1, title: "The First Topic", author_name: "David", author_email_address: "david@loudthinking.com", written_on: "#{topic.written_on.to_s(:db)}", bonus_time: "#{topic.bonus_time.to_s(:db)}", last_read: "#{topic.last_read.to_s(:db)}", content: "Have a nice day", important: nil, approved: false, replies_count: 1, parent_id: nil, parent_title: nil, type: nil, group: nil, created_at: "#{topic.created_at.to_s(:db)}", updated_at: "#{topic.updated_at.to_s(:db)}">), topic.inspect + assert_equal %(#<Topic id: 1, title: "The First Topic", author_name: "David", author_email_address: "david@loudthinking.com", written_on: "#{topic.written_on.to_s(:db)}", bonus_time: "#{topic.bonus_time.to_s(:db)}", last_read: "#{topic.last_read.to_s(:db)}", content: "Have a nice day", important: nil, approved: false, replies_count: 1, unique_replies_count: 0, parent_id: nil, parent_title: nil, type: nil, group: nil, created_at: "#{topic.created_at.to_s(:db)}", updated_at: "#{topic.updated_at.to_s(:db)}">), topic.inspect end def test_inspect_new_instance diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index ba6b0b1362..e09fa95756 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -12,7 +12,7 @@ class EachTest < ActiveRecord::TestCase end def test_each_should_execute_one_query_per_batch - assert_queries(Post.count + 1) do + assert_queries(@total + 1) do Post.find_each(:batch_size => 1) do |post| assert_kind_of Post, post end @@ -51,7 +51,7 @@ class EachTest < ActiveRecord::TestCase end def test_find_in_batches_should_return_batches - assert_queries(Post.count + 1) do + assert_queries(@total + 1) do Post.find_in_batches(:batch_size => 1) do |batch| assert_kind_of Array, batch assert_kind_of Post, batch.first @@ -60,7 +60,7 @@ class EachTest < ActiveRecord::TestCase end def test_find_in_batches_should_start_from_the_start_option - assert_queries(Post.count) do + assert_queries(@total) do Post.find_in_batches(:batch_size => 1, :start => 2) do |batch| assert_kind_of Array, batch assert_kind_of Post, batch.first @@ -69,14 +69,12 @@ class EachTest < ActiveRecord::TestCase end def test_find_in_batches_shouldnt_execute_query_unless_needed - post_count = Post.count - assert_queries(2) do - Post.find_in_batches(:batch_size => post_count) {|batch| assert_kind_of Array, batch } + Post.find_in_batches(:batch_size => @total) {|batch| assert_kind_of Array, batch } end assert_queries(1) do - Post.find_in_batches(:batch_size => post_count + 1) {|batch| assert_kind_of Array, batch } + Post.find_in_batches(:batch_size => @total + 1) {|batch| assert_kind_of Array, batch } end end diff --git a/activerecord/test/cases/counter_cache_test.rb b/activerecord/test/cases/counter_cache_test.rb index ac093251a5..61f9d4cdae 100644 --- a/activerecord/test/cases/counter_cache_test.rb +++ b/activerecord/test/cases/counter_cache_test.rb @@ -51,6 +51,18 @@ class CounterCacheTest < ActiveRecord::TestCase end end + test 'reset multiple association counters' do + Topic.increment_counter(:replies_count, @topic.id) + assert_difference '@topic.reload.replies_count', -1 do + Topic.reset_counters(@topic.id, :replies, :unique_replies) + end + + Topic.increment_counter(:unique_replies_count, @topic.id) + assert_difference '@topic.reload.unique_replies_count', -1 do + Topic.reset_counters(@topic.id, :replies, :unique_replies) + end + end + test "reset counters with string argument" do Topic.increment_counter('replies_count', @topic.id) diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index a9d46f4fba..b5314bc9be 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -35,18 +35,18 @@ class ReflectionTest < ActiveRecord::TestCase def test_read_attribute_names assert_equal( - %w( id title author_name author_email_address bonus_time written_on last_read content important group approved replies_count parent_id parent_title type created_at updated_at ).sort, + %w( id title author_name author_email_address bonus_time written_on last_read content important group approved replies_count unique_replies_count parent_id parent_title type created_at updated_at ).sort, @first.attribute_names.sort ) end def test_columns - assert_equal 17, Topic.columns.length + assert_equal 18, Topic.columns.length end def test_columns_are_returned_in_the_order_they_were_declared column_names = Topic.columns.map { |column| column.name } - assert_equal %w(id title author_name author_email_address written_on bonus_time last_read content important approved replies_count parent_id parent_title type group created_at updated_at), column_names + assert_equal %w(id title author_name author_email_address written_on bonus_time last_read content important approved replies_count unique_replies_count parent_id parent_title type group created_at updated_at), column_names end def test_content_columns diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index cf6af4e8f4..b64ff13d29 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1546,4 +1546,26 @@ class RelationTest < ActiveRecord::TestCase assert merged.to_sql.include?("wtf") assert merged.to_sql.include?("bbq") end + + def test_merging_removes_rhs_bind_parameters + left = Post.where(id: Arel::Nodes::BindParam.new('?')) + column = Post.columns_hash['id'] + left.bind_values += [[column, 20]] + right = Post.where(id: 10) + + merged = left.merge(right) + assert_equal [], merged.bind_values + end + + def test_merging_keeps_lhs_bind_parameters + column = Post.columns_hash['id'] + binds = [[column, 20]] + + right = Post.where(id: Arel::Nodes::BindParam.new('?')) + right.bind_values += binds + left = Post.where(id: 10) + + merged = left.merge(right) + assert_equal binds, merged.bind_values + end end diff --git a/activerecord/test/cases/validations/uniqueness_validation_test.rb b/activerecord/test/cases/validations/uniqueness_validation_test.rb index 57457359b1..2b33f01783 100644 --- a/activerecord/test/cases/validations/uniqueness_validation_test.rb +++ b/activerecord/test/cases/validations/uniqueness_validation_test.rb @@ -268,7 +268,7 @@ class UniquenessValidationTest < ActiveRecord::TestCase end def test_validate_case_sensitive_uniqueness_with_attribute_passed_as_integer - Topic.validates_uniqueness_of(:title, :case_sensitve => true) + Topic.validates_uniqueness_of(:title, :case_sensitive => true) Topic.create!('title' => 101) t2 = Topic.new('title' => 101) diff --git a/activerecord/test/models/reply.rb b/activerecord/test/models/reply.rb index c88262580e..3e82e55d89 100644 --- a/activerecord/test/models/reply.rb +++ b/activerecord/test/models/reply.rb @@ -7,6 +7,7 @@ class Reply < Topic end class UniqueReply < Reply + belongs_to :topic, :foreign_key => 'parent_id', :counter_cache => true validates_uniqueness_of :content, :scope => 'parent_id' end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 8beb58f3fc..188a3f0164 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -675,6 +675,7 @@ ActiveRecord::Schema.define do end t.boolean :approved, :default => true t.integer :replies_count, :default => 0 + t.integer :unique_replies_count, :default => 0 t.integer :parent_id t.string :parent_title t.string :type |