From d5dc02b5e88324bdbd274a5008a1d6b7a2f6f9d7 Mon Sep 17 00:00:00 2001 From: ozzyaaron Date: Tue, 29 Mar 2011 11:22:16 +0800 Subject: Added back the Callback debugging section by interrogating the _*_callbacks method --- activerecord/lib/active_record/callbacks.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index 86d58df99b..a175bf003c 100644 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -214,6 +214,24 @@ module ActiveRecord # needs to be aware of it because an ordinary +save+ will raise such exception # instead of quietly returning +false+. # + # == Debugging callbacks + # + # The callback chain is accessible via the _*_callbacks method on an object. ActiveModel Callbacks support + # :before, :after and :around as values for the kind property. The kind property + # defines what part of the chain the callback runs in. + # + # To find all callbacks in the before_save callback chain: + # + # Topic._save_callbacks.select { |cb| cb.kind.eql?(:before) } + # + # Returns an array of callback objects that form the before_save chain. + # + # To further check if the before_save chain contains a proc defined as rest_when_dead use the filter property of the callback object: + # + # Topic._save_callbacks.select { |cb| cb.kind.eql?(:before) }.collect(&:filter).include?(:rest_when_dead) + # + # Returns true or false depending on whether the proc is contained in the before_save callback chain on a Topic model. + # module Callbacks extend ActiveSupport::Concern -- cgit v1.2.3 From 45d5d6b2683be263ae9c977324633972f318b814 Mon Sep 17 00:00:00 2001 From: Eadz Date: Sat, 19 Mar 2011 00:00:50 -0700 Subject: Documented undocumented feature: Class methods on your model are automatically available on scopes --- activerecord/lib/active_record/named_scope.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index d291632260..a445f68790 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -99,6 +99,28 @@ module ActiveRecord # # Article.published.new.published # => true # Article.published.create.published # => true + # + # Class methods on your model are automatically available + # on scopes + # + # class Article < ActiveRecord::Base + # scope :pubished, where(:published => true) + # scope :featured, where(:featured => true) + # + # def self.latest_article + # order('published_at desc').first + # end + # + # def self.titles + # map{|article| article.title} + # end + # + # end + # + # Example usage: + # Article.published.featured.latest_article + # Article.featured.titles + def scope(name, scope_options = {}) name = name.to_sym valid_scope_name?(name) -- cgit v1.2.3 From 6a1715111e16e07a30bd61eaecf059fd90732e59 Mon Sep 17 00:00:00 2001 From: Eadz Date: Sat, 19 Mar 2011 00:02:53 -0700 Subject: add space to conform with style --- activerecord/lib/active_record/named_scope.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index a445f68790..9c0652c3a0 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -112,7 +112,7 @@ module ActiveRecord # end # # def self.titles - # map{|article| article.title} + # map {|article| article.title} # end # # end -- cgit v1.2.3 From 7a34ab7d60756856b79d2f8ef33ac843a78b70ad Mon Sep 17 00:00:00 2001 From: Ryan Bigg Date: Thu, 31 Mar 2011 06:46:02 +1100 Subject: Fix typo in named_scope documentation --- activerecord/lib/active_record/named_scope.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 9c0652c3a0..603a0c169a 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -104,7 +104,7 @@ module ActiveRecord # on scopes # # class Article < ActiveRecord::Base - # scope :pubished, where(:published => true) + # scope :published, where(:published => true) # scope :featured, where(:featured => true) # # def self.latest_article -- cgit v1.2.3 From 04d5decfd3c8f899df462bfc7f1ccb9770542a97 Mon Sep 17 00:00:00 2001 From: Ryan Bigg Date: Thu, 31 Mar 2011 06:47:01 +1100 Subject: Cleanup of named_scope documentation --- activerecord/lib/active_record/named_scope.rb | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 603a0c169a..8eb87f7b7a 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -101,24 +101,25 @@ module ActiveRecord # Article.published.create.published # => true # # Class methods on your model are automatically available - # on scopes - # + # on scopes. Assuming the following setup: + # # class Article < ActiveRecord::Base # scope :published, where(:published => true) # scope :featured, where(:featured => true) - # + # # def self.latest_article - # order('published_at desc').first + # order('published_at desc').first # end - # + # # def self.titles # map {|article| article.title} # end # # end - # - # Example usage: - # Article.published.featured.latest_article + # + # We are able to call the methods like this: + # + # Article.published.featured.latest_article # Article.featured.titles def scope(name, scope_options = {}) -- cgit v1.2.3 From cf07da0929bbeaaeb68cbafbb600727b3bda470e Mon Sep 17 00:00:00 2001 From: Ryan Bigg Date: Thu, 31 Mar 2011 06:59:48 +1100 Subject: Symbol to proc is preferred over longer form of map --- activerecord/lib/active_record/named_scope.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 8eb87f7b7a..d5fff65303 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -112,7 +112,7 @@ module ActiveRecord # end # # def self.titles - # map {|article| article.title} + # map(&:title) # end # # end -- cgit v1.2.3 From bd3cdeea354ebff97b0d5102a0857ce85eedcfa4 Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Sun, 3 Apr 2011 12:47:51 +0900 Subject: s/ERb/ERB/g The author of ERB sais, his eRuby implementation was originally named "ERb/ERbLight" and then renamed to "ERB" when started bundled as a Ruby standard lib. http://www2a.biglobe.ne.jp/~seki/ruby/erb.html --- activerecord/lib/active_record/fixtures.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index d523c643ba..0939ec2626 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -173,10 +173,10 @@ class FixturesFileNotFound < StandardError; end # traversed in the database to create the fixture hash and/or instance variables. This is expensive for # large sets of fixtured data. # -# = Dynamic fixtures with ERb +# = Dynamic fixtures with ERB # # Some times you don't care about the content of the fixtures as much as you care about the volume. In these cases, you can -# mix ERb in with your YAML or CSV fixtures to create a bunch of fixtures for load testing, like: +# mix ERB in with your YAML or CSV fixtures to create a bunch of fixtures for load testing, like: # # <% for i in 1..1000 %> # fix_<%= i %>: @@ -186,7 +186,7 @@ class FixturesFileNotFound < StandardError; end # # This will create 1000 very simple YAML fixtures. # -# Using ERb, you can also inject dynamic values into your fixtures with inserts like <%= Date.today.strftime("%Y-%m-%d") %>. +# Using ERB, you can also inject dynamic values into your fixtures with inserts like <%= Date.today.strftime("%Y-%m-%d") %>. # This is however a feature to be used with some caution. The point of fixtures are that they're # stable units of predictable sample data. If you feel that you need to inject dynamic values, then # perhaps you should reexamine whether your application is properly testable. Hence, dynamic values -- cgit v1.2.3 From b35617235d43bdb32016a623044e7f4005879969 Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Sun, 3 Apr 2011 20:09:00 -0300 Subject: Use IM when trying to load records using ID. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- activerecord/lib/active_record/relation.rb | 3 +++ activerecord/lib/active_record/relation/finder_methods.rb | 9 +++++++++ activerecord/lib/active_record/transactions.rb | 1 + activerecord/test/cases/base_test.rb | 10 +++++----- activerecord/test/cases/identity_map_test.rb | 13 ++++++++++--- activerecord/test/cases/query_cache_test.rb | 2 +- 6 files changed, 29 insertions(+), 9 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 896daf516e..75269c85d9 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -194,6 +194,7 @@ module ActiveRecord # # The same idea applies to limit and order # Book.where('title LIKE ?', '%Rails%').order(:created_at).limit(5).update_all(:author => 'David') def update_all(updates, conditions = nil, options = {}) + IdentityMap.repository[symbolized_base_class].clear if IdentityMap.enabled? if conditions || options.present? where(conditions).apply_finder_options(options.slice(:limit, :order)).update_all(updates) else @@ -322,6 +323,7 @@ module ActiveRecord # If you need to destroy dependent associations or call your before_* or # +after_destroy+ callbacks, use the +destroy_all+ method instead. def delete_all(conditions = nil) + IdentityMap.repository[symbolized_base_class] = {} if IdentityMap.enabled? if conditions where(conditions).delete_all else @@ -353,6 +355,7 @@ module ActiveRecord # # Delete multiple rows # Todo.delete([2,3,4]) def delete(id_or_array) + IdentityMap.remove_by_id(self.symbolized_base_class, id_or_array) if IdentityMap.enabled? where(primary_key => id_or_array).delete_all end diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 8fa315bdf3..7fe6fe0ed0 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -309,6 +309,15 @@ module ActiveRecord def find_one(id) id = id.id if ActiveRecord::Base === id + if IdentityMap.enabled? && where_values.blank? && + limit_value.blank? && order_values.blank? && + includes_values.blank? && preload_values.blank? && + readonly_value.nil? && joins_values.blank? && + !@klass.locking_enabled? && + record = IdentityMap.get(@klass, id) + return record + end + column = columns_hash[primary_key] substitute = connection.substitute_for(column, @bind_values) diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 60d4c256c4..d363f36108 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -270,6 +270,7 @@ module ActiveRecord def rolledback!(force_restore_state = false) #:nodoc: run_callbacks :rollback ensure + IdentityMap.remove(self) if IdentityMap.enabled? restore_transaction_record_state(force_restore_state) end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index aeb0b28bab..b3facf50b8 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -172,7 +172,7 @@ class BasicsTest < ActiveRecord::TestCase with_active_record_default_timezone :utc do time = Time.local(2000) topic = Topic.create('written_on' => time) - saved_time = Topic.find(topic.id).written_on + saved_time = Topic.find(topic.id).reload.written_on assert_equal time, saved_time assert_equal [0, 0, 0, 1, 1, 2000, 6, 1, false, "EST"], time.to_a assert_equal [0, 0, 5, 1, 1, 2000, 6, 1, false, "UTC"], saved_time.to_a @@ -186,7 +186,7 @@ class BasicsTest < ActiveRecord::TestCase Time.use_zone 'Central Time (US & Canada)' do time = Time.zone.local(2000) topic = Topic.create('written_on' => time) - saved_time = Topic.find(topic.id).written_on + saved_time = Topic.find(topic.id).reload.written_on assert_equal time, saved_time assert_equal [0, 0, 0, 1, 1, 2000, 6, 1, false, "CST"], time.to_a assert_equal [0, 0, 6, 1, 1, 2000, 6, 1, false, "UTC"], saved_time.to_a @@ -199,7 +199,7 @@ class BasicsTest < ActiveRecord::TestCase with_env_tz 'America/New_York' do time = Time.utc(2000) topic = Topic.create('written_on' => time) - saved_time = Topic.find(topic.id).written_on + saved_time = Topic.find(topic.id).reload.written_on assert_equal time, saved_time assert_equal [0, 0, 0, 1, 1, 2000, 6, 1, false, "UTC"], time.to_a assert_equal [0, 0, 19, 31, 12, 1999, 5, 365, false, "EST"], saved_time.to_a @@ -212,7 +212,7 @@ class BasicsTest < ActiveRecord::TestCase Time.use_zone 'Central Time (US & Canada)' do time = Time.zone.local(2000) topic = Topic.create('written_on' => time) - saved_time = Topic.find(topic.id).written_on + saved_time = Topic.find(topic.id).reload.written_on assert_equal time, saved_time assert_equal [0, 0, 0, 1, 1, 2000, 6, 1, false, "CST"], time.to_a assert_equal [0, 0, 1, 1, 1, 2000, 6, 1, false, "EST"], saved_time.to_a @@ -1048,7 +1048,7 @@ class BasicsTest < ActiveRecord::TestCase topic = Topic.new(:content => myobj) assert topic.save Topic.serialize(:content, Hash) - assert_raise(ActiveRecord::SerializationTypeMismatch) { Topic.find(topic.id).content } + assert_raise(ActiveRecord::SerializationTypeMismatch) { Topic.find(topic.id).reload.content } ensure Topic.serialize(:content) end diff --git a/activerecord/test/cases/identity_map_test.rb b/activerecord/test/cases/identity_map_test.rb index 89f7b92d09..9cf3db7f87 100644 --- a/activerecord/test/cases/identity_map_test.rb +++ b/activerecord/test/cases/identity_map_test.rb @@ -90,6 +90,13 @@ class IdentityMapTest < ActiveRecord::TestCase ) end + def test_queries_are_not_executed_when_finding_by_id + Post.find 1 + assert_no_queries do + Post.find 1 + end + end + ############################################################################## # Tests checking if IM is functioning properly on more advanced finds # # and associations # @@ -144,7 +151,7 @@ class IdentityMapTest < ActiveRecord::TestCase s = Subscriber.find('swistak') - assert_equal({'name' => ["Raczkowski Marcin", "Swistak Sreberkowiec"]}, swistak.changes) + assert_equal({"name"=>["Marcin Raczkowski", "Swistak Sreberkowiec"]}, swistak.changes) assert_equal("Swistak Sreberkowiec", swistak.name) end @@ -159,8 +166,8 @@ class IdentityMapTest < ActiveRecord::TestCase s = Subscriber.find('swistak') assert_equal("Swistak Sreberkowiec", swistak.name) - assert_equal({}, swistak.changes) - assert !swistak.name_changed? + assert_equal({"name"=>["Marcin Raczkowski", "Swistak Sreberkowiec"]}, swistak.changes) + assert swistak.name_changed? end def test_has_many_associations diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index 53aefc7b58..287f7e255b 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -13,7 +13,7 @@ class QueryCacheTest < ActiveRecord::TestCase end def test_find_queries - assert_queries(2) { Task.find(1); Task.find(1) } + assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) { Task.find(1); Task.find(1) } end def test_find_queries_with_cache -- cgit v1.2.3 From 454ec93ff79af4c0ddb31d01827021b549d12413 Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Sun, 3 Apr 2011 20:19:52 -0300 Subject: Add log message when loading records from Identity Map. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- activerecord/lib/active_record/identity_map.rb | 9 ++++++++- activerecord/test/cases/identity_map_test.rb | 9 +++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/identity_map.rb b/activerecord/lib/active_record/identity_map.rb index d18b2b0a54..95a8e5cff7 100644 --- a/activerecord/lib/active_record/identity_map.rb +++ b/activerecord/lib/active_record/identity_map.rb @@ -50,7 +50,14 @@ module ActiveRecord def get(klass, primary_key) obj = repository[klass.symbolized_base_class][primary_key] - obj.is_a?(klass) ? obj : nil + if obj.is_a?(klass) + if ActiveRecord::Base.logger + ActiveRecord::Base.logger.debug "#{klass} with ID = #{primary_key} loaded from Identity Map" + end + obj + else + nil + end end def add(record) diff --git a/activerecord/test/cases/identity_map_test.rb b/activerecord/test/cases/identity_map_test.rb index 9cf3db7f87..199e59657d 100644 --- a/activerecord/test/cases/identity_map_test.rb +++ b/activerecord/test/cases/identity_map_test.rb @@ -388,6 +388,15 @@ class IdentityMapTest < ActiveRecord::TestCase assert_not_nil post.title end + def test_log + log = StringIO.new + ActiveRecord::Base.logger = Logger.new(log) + ActiveRecord::Base.logger.level = Logger::DEBUG + Post.find 1 + Post.find 1 + assert_match(/Post with ID = 1 loaded from Identity Map/, log.string) + end + # Currently AR is not allowing changing primary key (see Persistence#update) # So we ignore it. If this changes, this test needs to be uncommented. # def test_updating_of_pkey -- cgit v1.2.3 From a9b4b5da7c216e4464eeb9dbd0a39ea258d64325 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Tue, 5 Apr 2011 11:01:04 +0200 Subject: Destroying records via nested attributes works independent of reject_if: - When a :_destroy truthiness is provided in the attributes hash, the record should get destroyed regardless of the result of the proc or method supplied to :reject_if. (If :allow_destroy is true) [#6006 state:committed] Signed-off-by: Santiago Pastorino --- activerecord/lib/active_record/nested_attributes.rb | 1 + activerecord/test/cases/nested_attributes_test.rb | 8 ++++++++ 2 files changed, 9 insertions(+) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 522c0cfc9f..c111a968dc 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -452,6 +452,7 @@ module ActiveRecord end def call_reject_if(association_name, attributes) + return false if has_destroy_flag?(attributes) case callback = self.nested_attributes_options[association_name][:reject_if] when Symbol method(callback).arity == 0 ? send(callback) : send(callback, attributes) diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index c57ab7ed28..6568eb1d18 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -131,6 +131,14 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase assert_equal 'photography', interest.reload.topic end + def test_destroy_works_independent_of_reject_if + Man.accepts_nested_attributes_for :interests, :reject_if => proc {|attributes| true }, :allow_destroy => true + man = Man.create(:name => "Jon") + interest = man.interests.create(:topic => 'the ladies') + man.update_attributes({:interests_attributes => { :_destroy => "1", :id => interest.id } }) + assert man.reload.interests.empty? + end + def test_has_many_association_updating_a_single_record Man.accepts_nested_attributes_for(:interests) man = Man.create(:name => 'John') -- cgit v1.2.3 From 18dde7bf4ffffc93c9b32df65c168df29a898fbb Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Tue, 5 Apr 2011 17:01:54 -0300 Subject: Disable IdentityMap in log tests, it's not important and when running tests rake task it logs more messages in the tested buffer. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- activerecord/test/cases/log_subscriber_test.rb | 3 +++ 1 file changed, 3 insertions(+) (limited to 'activerecord') diff --git a/activerecord/test/cases/log_subscriber_test.rb b/activerecord/test/cases/log_subscriber_test.rb index 8ebde933b4..5f55299065 100644 --- a/activerecord/test/cases/log_subscriber_test.rb +++ b/activerecord/test/cases/log_subscriber_test.rb @@ -8,6 +8,8 @@ class LogSubscriberTest < ActiveRecord::TestCase def setup @old_logger = ActiveRecord::Base.logger + @using_identity_map = ActiveRecord::IdentityMap.enabled? + ActiveRecord::IdentityMap.enabled = false super ActiveRecord::LogSubscriber.attach_to(:active_record) end @@ -16,6 +18,7 @@ class LogSubscriberTest < ActiveRecord::TestCase super ActiveRecord::LogSubscriber.log_subscribers.pop ActiveRecord::Base.logger = @old_logger + ActiveRecord::IdentityMap.enabled = @using_identity_map end def set_logger(logger) -- cgit v1.2.3 From 508c679f11adfd14ac35077917510c8fe81c73c0 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 8 Apr 2011 11:22:21 -0700 Subject: moving sqlite_adapter_tests to sqlite3_adapter_test so that the tests are actually run. :bomb: --- .../cases/adapters/sqlite/sqlite_adapter_test.rb | 229 --------------------- .../cases/adapters/sqlite3/sqlite3_adapter_test.rb | 215 +++++++++++++++++++ 2 files changed, 215 insertions(+), 229 deletions(-) delete mode 100644 activerecord/test/cases/adapters/sqlite/sqlite_adapter_test.rb (limited to 'activerecord') diff --git a/activerecord/test/cases/adapters/sqlite/sqlite_adapter_test.rb b/activerecord/test/cases/adapters/sqlite/sqlite_adapter_test.rb deleted file mode 100644 index d1fc470907..0000000000 --- a/activerecord/test/cases/adapters/sqlite/sqlite_adapter_test.rb +++ /dev/null @@ -1,229 +0,0 @@ -# encoding: utf-8 -require "cases/helper" -require 'models/binary' - -module ActiveRecord - module ConnectionAdapters - class SQLiteAdapterTest < ActiveRecord::TestCase - class DualEncoding < ActiveRecord::Base - end - - def setup - @ctx = Base.sqlite3_connection :database => ':memory:', - :adapter => 'sqlite3', - :timeout => nil - @ctx.execute <<-eosql - CREATE TABLE items ( - id integer PRIMARY KEY AUTOINCREMENT, - number integer - ) - eosql - end - - def test_quote_binary_column_escapes_it - DualEncoding.connection.execute(<<-eosql) - CREATE TABLE dual_encodings ( - id integer PRIMARY KEY AUTOINCREMENT, - name string, - data binary - ) - eosql - str = "\x80".force_encoding("ASCII-8BIT") - binary = DualEncoding.new :name => 'いただきます！', :data => str - binary.save! - assert_equal str, binary.data - end - - def test_execute - @ctx.execute "INSERT INTO items (number) VALUES (10)" - records = @ctx.execute "SELECT * FROM items" - assert_equal 1, records.length - - record = records.first - assert_equal 10, record['number'] - assert_equal 1, record['id'] - end - - def test_quote_string - assert_equal "''", @ctx.quote_string("'") - end - - def test_insert_sql - 2.times do |i| - rv = @ctx.insert_sql "INSERT INTO items (number) VALUES (#{i})" - assert_equal(i + 1, rv) - end - - records = @ctx.execute "SELECT * FROM items" - assert_equal 2, records.length - end - - def test_insert_sql_logged - sql = "INSERT INTO items (number) VALUES (10)" - name = "foo" - - assert_logged([[sql, name]]) do - @ctx.insert_sql sql, name - end - end - - def test_insert_id_value_returned - sql = "INSERT INTO items (number) VALUES (10)" - idval = 'vuvuzela' - id = @ctx.insert_sql sql, nil, nil, idval - assert_equal idval, id - end - - def test_select_rows - 2.times do |i| - @ctx.create "INSERT INTO items (number) VALUES (#{i})" - end - rows = @ctx.select_rows 'select number, id from items' - assert_equal [[0, 1], [1, 2]], rows - end - - def test_select_rows_logged - sql = "select * from items" - name = "foo" - - assert_logged([[sql, name]]) do - @ctx.select_rows sql, name - end - end - - def test_transaction - count_sql = 'select count(*) from items' - - @ctx.begin_db_transaction - @ctx.create "INSERT INTO items (number) VALUES (10)" - - assert_equal 1, @ctx.select_rows(count_sql).first.first - @ctx.rollback_db_transaction - assert_equal 0, @ctx.select_rows(count_sql).first.first - end - - def test_tables - assert_equal %w{ items }, @ctx.tables - - @ctx.execute <<-eosql - CREATE TABLE people ( - id integer PRIMARY KEY AUTOINCREMENT, - number integer - ) - eosql - assert_equal %w{ items people }.sort, @ctx.tables.sort - end - - def test_tables_logs_name - name = "hello" - assert_logged [[name]] do - @ctx.tables(name) - assert_not_nil @ctx.logged.first.shift - end - end - - def test_columns - columns = @ctx.columns('items').sort_by { |x| x.name } - assert_equal 2, columns.length - assert_equal %w{ id number }.sort, columns.map { |x| x.name } - assert_equal [nil, nil], columns.map { |x| x.default } - assert_equal [true, true], columns.map { |x| x.null } - end - - def test_columns_with_default - @ctx.execute <<-eosql - CREATE TABLE columns_with_default ( - id integer PRIMARY KEY AUTOINCREMENT, - number integer default 10 - ) - eosql - column = @ctx.columns('columns_with_default').find { |x| - x.name == 'number' - } - assert_equal 10, column.default - end - - def test_columns_with_not_null - @ctx.execute <<-eosql - CREATE TABLE columns_with_default ( - id integer PRIMARY KEY AUTOINCREMENT, - number integer not null - ) - eosql - column = @ctx.columns('columns_with_default').find { |x| - x.name == 'number' - } - assert !column.null, "column should not be null" - end - - def test_indexes_logs - intercept_logs_on @ctx - assert_difference('@ctx.logged.length') do - @ctx.indexes('items') - end - assert_match(/items/, @ctx.logged.last.first) - end - - def test_no_indexes - assert_equal [], @ctx.indexes('items') - end - - def test_index - @ctx.add_index 'items', 'id', :unique => true, :name => 'fun' - index = @ctx.indexes('items').find { |idx| idx.name == 'fun' } - - assert_equal 'items', index.table - assert index.unique, 'index is unique' - assert_equal ['id'], index.columns - end - - def test_non_unique_index - @ctx.add_index 'items', 'id', :name => 'fun' - index = @ctx.indexes('items').find { |idx| idx.name == 'fun' } - assert !index.unique, 'index is not unique' - end - - def test_compound_index - @ctx.add_index 'items', %w{ id number }, :name => 'fun' - index = @ctx.indexes('items').find { |idx| idx.name == 'fun' } - assert_equal %w{ id number }.sort, index.columns.sort - end - - def test_primary_key - assert_equal 'id', @ctx.primary_key('items') - - @ctx.execute <<-eosql - CREATE TABLE foos ( - internet integer PRIMARY KEY AUTOINCREMENT, - number integer not null - ) - eosql - assert_equal 'internet', @ctx.primary_key('foos') - end - - def test_no_primary_key - @ctx.execute 'CREATE TABLE failboat (number integer not null)' - assert_nil @ctx.primary_key('failboat') - end - - private - - def assert_logged logs - intercept_logs_on @ctx - yield - assert_equal logs, @ctx.logged - end - - def intercept_logs_on ctx - @ctx.extend(Module.new { - attr_accessor :logged - def log sql, name - @logged << [sql, name] - yield - end - }) - @ctx.logged = [] - end - end - end -end diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index b8abdface4..a2ed3302aa 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -1,12 +1,22 @@ +# encoding: utf-8 require "cases/helper" module ActiveRecord module ConnectionAdapters class SQLite3AdapterTest < ActiveRecord::TestCase + class DualEncoding < ActiveRecord::Base + end + def setup @conn = Base.sqlite3_connection :database => ':memory:', :adapter => 'sqlite3', :timeout => 100 + @conn.execute <<-eosql + CREATE TABLE items ( + id integer PRIMARY KEY AUTOINCREMENT, + number integer + ) + eosql end def test_primary_key_returns_nil_for_no_pk @@ -102,6 +112,211 @@ module ActiveRecord assert_equal [[1, 'foo']], result.rows end + + def test_quote_binary_column_escapes_it + DualEncoding.connection.execute(<<-eosql) + CREATE TABLE dual_encodings ( + id integer PRIMARY KEY AUTOINCREMENT, + name string, + data binary + ) + eosql + str = "\x80".force_encoding("ASCII-8BIT") + binary = DualEncoding.new :name => 'いただきます！', :data => str + binary.save! + assert_equal str, binary.data + end + + def test_execute + @conn.execute "INSERT INTO items (number) VALUES (10)" + records = @conn.execute "SELECT * FROM items" + assert_equal 1, records.length + + record = records.first + assert_equal 10, record['number'] + assert_equal 1, record['id'] + end + + def test_quote_string + assert_equal "''", @conn.quote_string("'") + end + + def test_insert_sql + 2.times do |i| + rv = @conn.insert_sql "INSERT INTO items (number) VALUES (#{i})" + assert_equal(i + 1, rv) + end + + records = @conn.execute "SELECT * FROM items" + assert_equal 2, records.length + end + + def test_insert_sql_logged + sql = "INSERT INTO items (number) VALUES (10)" + name = "foo" + + assert_logged([[sql, name, []]]) do + @conn.insert_sql sql, name + end + end + + def test_insert_id_value_returned + sql = "INSERT INTO items (number) VALUES (10)" + idval = 'vuvuzela' + id = @conn.insert_sql sql, nil, nil, idval + assert_equal idval, id + end + + def test_select_rows + 2.times do |i| + @conn.create "INSERT INTO items (number) VALUES (#{i})" + end + rows = @conn.select_rows 'select number, id from items' + assert_equal [[0, 1], [1, 2]], rows + end + + def test_select_rows_logged + sql = "select * from items" + name = "foo" + + assert_logged([[sql, name, []]]) do + @conn.select_rows sql, name + end + end + + def test_transaction + count_sql = 'select count(*) from items' + + @conn.begin_db_transaction + @conn.create "INSERT INTO items (number) VALUES (10)" + + assert_equal 1, @conn.select_rows(count_sql).first.first + @conn.rollback_db_transaction + assert_equal 0, @conn.select_rows(count_sql).first.first + end + + def test_tables + assert_equal %w{ items }, @conn.tables + + @conn.execute <<-eosql + CREATE TABLE people ( + id integer PRIMARY KEY AUTOINCREMENT, + number integer + ) + eosql + assert_equal %w{ items people }.sort, @conn.tables.sort + end + + def test_tables_logs_name + name = "hello" + assert_logged [[name, []]] do + @conn.tables(name) + assert_not_nil @conn.logged.first.shift + end + end + + def test_columns + columns = @conn.columns('items').sort_by { |x| x.name } + assert_equal 2, columns.length + assert_equal %w{ id number }.sort, columns.map { |x| x.name } + assert_equal [nil, nil], columns.map { |x| x.default } + assert_equal [true, true], columns.map { |x| x.null } + end + + def test_columns_with_default + @conn.execute <<-eosql + CREATE TABLE columns_with_default ( + id integer PRIMARY KEY AUTOINCREMENT, + number integer default 10 + ) + eosql + column = @conn.columns('columns_with_default').find { |x| + x.name == 'number' + } + assert_equal 10, column.default + end + + def test_columns_with_not_null + @conn.execute <<-eosql + CREATE TABLE columns_with_default ( + id integer PRIMARY KEY AUTOINCREMENT, + number integer not null + ) + eosql + column = @conn.columns('columns_with_default').find { |x| + x.name == 'number' + } + assert !column.null, "column should not be null" + end + + def test_indexes_logs + intercept_logs_on @conn + assert_difference('@conn.logged.length') do + @conn.indexes('items') + end + assert_match(/items/, @conn.logged.last.first) + end + + def test_no_indexes + assert_equal [], @conn.indexes('items') + end + + def test_index + @conn.add_index 'items', 'id', :unique => true, :name => 'fun' + index = @conn.indexes('items').find { |idx| idx.name == 'fun' } + + assert_equal 'items', index.table + assert index.unique, 'index is unique' + assert_equal ['id'], index.columns + end + + def test_non_unique_index + @conn.add_index 'items', 'id', :name => 'fun' + index = @conn.indexes('items').find { |idx| idx.name == 'fun' } + assert !index.unique, 'index is not unique' + end + + def test_compound_index + @conn.add_index 'items', %w{ id number }, :name => 'fun' + index = @conn.indexes('items').find { |idx| idx.name == 'fun' } + assert_equal %w{ id number }.sort, index.columns.sort + end + + def test_primary_key + assert_equal 'id', @conn.primary_key('items') + + @conn.execute <<-eosql + CREATE TABLE foos ( + internet integer PRIMARY KEY AUTOINCREMENT, + number integer not null + ) + eosql + assert_equal 'internet', @conn.primary_key('foos') + end + + def test_no_primary_key + @conn.execute 'CREATE TABLE failboat (number integer not null)' + assert_nil @conn.primary_key('failboat') + end + + private + + def assert_logged logs + intercept_logs_on @conn + yield + assert_equal logs, @conn.logged + end + + def intercept_logs_on ctx + @conn.extend(Module.new { + attr_accessor :logged + def log sql, name, binds = [] + @logged << [sql, name, binds] + yield + end + }) + @conn.logged = [] + end end end end -- cgit v1.2.3 From 62b2755f7a4ca12491a7899348066695a6a26917 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Apr 2011 13:03:49 +0200 Subject: Remove dead branch code that appeared back in a merge. --- activerecord/lib/active_record/nested_attributes.rb | 6 ------ 1 file changed, 6 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index c111a968dc..08b27b6a8e 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -403,12 +403,6 @@ module ActiveRecord unless reject_new_record?(association_name, attributes) association.build(attributes.except(*UNASSIGNABLE_KEYS)) end - elsif existing_records.count == 0 #Existing record but not yet associated - existing_record = self.class.reflect_on_association(association_name).klass.find(attributes['id']) - if !call_reject_if(association_name, attributes) - association.send(:add_record_to_target_with_callbacks, existing_record) if !association.loaded? - assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) - end elsif existing_record = existing_records.detect { |record| record.id.to_s == attributes['id'].to_s } unless association.loaded? || call_reject_if(association_name, attributes) # Make sure we are operating on the actual object which is in the association's -- cgit v1.2.3 From a9f3c9da01d721963d05949604ead909aaabbf36 Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Mon, 11 Apr 2011 00:52:42 +0800 Subject: Using Object#in? and Object#either? in various places There're a lot of places in Rails source code which make a lot of sense to switching to Object#in? or Object#either? instead of using [].include?. --- activerecord/lib/active_record/associations/association.rb | 3 ++- activerecord/lib/active_record/associations/builder/belongs_to.rb | 4 +++- activerecord/lib/active_record/associations/builder/has_many.rb | 4 +++- activerecord/lib/active_record/associations/builder/has_one.rb | 4 +++- activerecord/lib/active_record/associations/has_one_association.rb | 4 +++- .../lib/active_record/attribute_methods/time_zone_conversion.rb | 3 ++- activerecord/lib/active_record/railties/databases.rake | 4 +++- activerecord/lib/active_record/reflection.rb | 3 ++- .../active_record/session_migration/session_migration_generator.rb | 3 ++- activerecord/test/cases/associations/join_model_test.rb | 5 +++-- activerecord/test/cases/attribute_methods/read_test.rb | 5 +++-- activerecord/test/cases/attribute_methods_test.rb | 3 ++- activerecord/test/cases/defaults_test.rb | 3 ++- 13 files changed, 33 insertions(+), 15 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 27c446b12c..b250783d9e 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -1,4 +1,5 @@ require 'active_support/core_ext/array/wrap' +require 'active_support/core_ext/object/inclusion' module ActiveRecord module Associations @@ -163,7 +164,7 @@ module ActiveRecord def creation_attributes attributes = {} - if [:has_one, :has_many].include?(reflection.macro) && !options[:through] + if reflection.macro.either?(:has_one, :has_many) && !options[:through] attributes[reflection.foreign_key] = owner[reflection.active_record_primary_key] if reflection.options[:as] diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 964e7fddc8..9ea37da78f 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/object/inclusion' + module ActiveRecord::Associations::Builder class BelongsTo < SingularAssociation #:nodoc: self.macro = :belongs_to @@ -65,7 +67,7 @@ module ActiveRecord::Associations::Builder def configure_dependency if options[:dependent] - unless [:destroy, :delete].include?(options[:dependent]) + unless options[:dependent].either?(:destroy, :delete) raise ArgumentError, "The :dependent option expects either :destroy or :delete (#{options[:dependent].inspect})" end diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb index 77bb66228d..3b489d9647 100644 --- a/activerecord/lib/active_record/associations/builder/has_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_many.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/object/inclusion' + module ActiveRecord::Associations::Builder class HasMany < CollectionAssociation #:nodoc: self.macro = :has_many @@ -14,7 +16,7 @@ module ActiveRecord::Associations::Builder def configure_dependency if options[:dependent] - unless [:destroy, :delete_all, :nullify, :restrict].include?(options[:dependent]) + unless options[:dependent].either?(:destroy, :delete_all, :nullify, :restrict) raise ArgumentError, "The :dependent option expects either :destroy, :delete_all, " \ ":nullify or :restrict (#{options[:dependent].inspect})" end diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb index 07ba5d088e..7efe2ec74d 100644 --- a/activerecord/lib/active_record/associations/builder/has_one.rb +++ b/activerecord/lib/active_record/associations/builder/has_one.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/object/inclusion' + module ActiveRecord::Associations::Builder class HasOne < SingularAssociation #:nodoc: self.macro = :has_one @@ -27,7 +29,7 @@ module ActiveRecord::Associations::Builder def configure_dependency if options[:dependent] - unless [:destroy, :delete, :nullify, :restrict].include?(options[:dependent]) + unless options[:dependent].either?(:destroy, :delete, :nullify, :restrict) raise ArgumentError, "The :dependent option expects either :destroy, :delete, " \ ":nullify or :restrict (#{options[:dependent].inspect})" end diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 1d2e8667e4..82f5696ad8 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/object/inclusion' + module ActiveRecord # = Active Record Belongs To Has One Association module Associations @@ -50,7 +52,7 @@ module ActiveRecord end def remove_target!(method) - if [:delete, :destroy].include?(method) + if method.either?(:delete, :destroy) target.send(method) else nullify_owner_attributes(target) diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index 6aac96df6f..032a3135f0 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -1,4 +1,5 @@ require 'active_support/core_ext/class/attribute' +require 'active_support/core_ext/object/inclusion' module ActiveRecord module AttributeMethods @@ -58,7 +59,7 @@ module ActiveRecord private def create_time_zone_conversion_attribute?(name, column) - time_zone_aware_attributes && !self.skip_time_zone_conversion_for_attributes.include?(name.to_sym) && [:datetime, :timestamp].include?(column.type) + time_zone_aware_attributes && !self.skip_time_zone_conversion_for_attributes.include?(name.to_sym) && column.type.either?(:datetime, :timestamp) end end end diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index ff36814684..a1b0270d1b 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -1,3 +1,5 @@ +require 'active_support/core_ext/object/inclusion' + db_namespace = namespace :db do task :load_config => :rails_env do require 'active_record' @@ -135,7 +137,7 @@ db_namespace = namespace :db do end def local_database?(config, &block) - if %w( 127.0.0.1 localhost ).include?(config['host']) || config['host'].blank? + if config['host'].either?("127.0.0.1", "localhost") || config['host'].blank? yield else $stderr.puts "This task only modifies local databases. #{config['database']} is on a remote host." diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index e801bc4afa..82561e86cf 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -1,5 +1,6 @@ require 'active_support/core_ext/class/attribute' require 'active_support/core_ext/module/deprecation' +require 'active_support/core_ext/object/inclusion' module ActiveRecord # = Active Record Reflection @@ -163,7 +164,7 @@ module ActiveRecord def initialize(macro, name, options, active_record) super - @collection = [:has_many, :has_and_belongs_to_many].include?(macro) + @collection = macro.either?(:has_many, :has_and_belongs_to_many) end # Returns a new, unsaved instance of the associated class. +options+ will diff --git a/activerecord/lib/rails/generators/active_record/session_migration/session_migration_generator.rb b/activerecord/lib/rails/generators/active_record/session_migration/session_migration_generator.rb index afcda2a98a..e803ee5764 100644 --- a/activerecord/lib/rails/generators/active_record/session_migration/session_migration_generator.rb +++ b/activerecord/lib/rails/generators/active_record/session_migration/session_migration_generator.rb @@ -1,4 +1,5 @@ require 'rails/generators/active_record' +require 'active_support/core_ext/object/inclusion' module ActiveRecord module Generators @@ -13,7 +14,7 @@ module ActiveRecord def session_table_name current_table_name = ActiveRecord::SessionStore::Session.table_name - if ["sessions", "session"].include?(current_table_name) + if current_table_name.either?("sessions", "session") current_table_name = (ActiveRecord::Base.pluralize_table_names ? 'session'.pluralize : 'session') end current_table_name diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 5a7b139030..afc830cae9 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -1,4 +1,5 @@ require "cases/helper" +require 'active_support/core_ext/object/inclusion' require 'models/tag' require 'models/tagging' require 'models/post' @@ -453,7 +454,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase assert saved_post.tags.include?(new_tag) assert new_tag.persisted? - assert saved_post.reload.tags(true).include?(new_tag) + assert new_tag.in?(saved_post.reload.tags(true)) new_post = Post.new(:title => "Association replacmenet works!", :body => "You best believe it.") @@ -466,7 +467,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase new_post.save! assert new_post.persisted? - assert new_post.reload.tags(true).include?(saved_tag) + assert saved_tag.in?(new_post.reload.tags(true)) assert !posts(:thinking).tags.build.persisted? assert !posts(:thinking).tags.new.persisted? diff --git a/activerecord/test/cases/attribute_methods/read_test.rb b/activerecord/test/cases/attribute_methods/read_test.rb index d0a9028264..3641031d12 100644 --- a/activerecord/test/cases/attribute_methods/read_test.rb +++ b/activerecord/test/cases/attribute_methods/read_test.rb @@ -1,4 +1,5 @@ require "cases/helper" +require 'active_support/core_ext/object/inclusion' module ActiveRecord module AttributeMethods @@ -41,13 +42,13 @@ module ActiveRecord instance = @klass.new @klass.column_names.each do |name| - assert ! instance.methods.map(&:to_s).include?(name) + assert !name.in?(instance.methods.map(&:to_s)) end @klass.define_attribute_methods @klass.column_names.each do |name| - assert(instance.methods.map(&:to_s).include?(name), "#{name} is not defined") + assert name.in?(instance.methods.map(&:to_s)), "#{name} is not defined" end end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 84f75cc803..aaf2435cd9 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -1,4 +1,5 @@ require "cases/helper" +require 'active_support/core_ext/object/inclusion' require 'models/minimalistic' require 'models/developer' require 'models/auto_id' @@ -638,7 +639,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase end def time_related_columns_on_topic - Topic.columns.select { |c| [:time, :date, :datetime, :timestamp].include?(c.type) } + Topic.columns.select { |c| c.type.either?(:time, :date, :datetime, :timestamp) } end def serialized_columns_on_topic diff --git a/activerecord/test/cases/defaults_test.rb b/activerecord/test/cases/defaults_test.rb index deaf5252db..57709f0c71 100644 --- a/activerecord/test/cases/defaults_test.rb +++ b/activerecord/test/cases/defaults_test.rb @@ -1,4 +1,5 @@ require "cases/helper" +require 'active_support/core_ext/object/inclusion' require 'models/default' require 'models/entrant' @@ -94,7 +95,7 @@ if current_adapter?(:MysqlAdapter) or current_adapter?(:Mysql2Adapter) assert_equal 0, klass.columns_hash['zero'].default assert !klass.columns_hash['zero'].null # 0 in MySQL 4, nil in 5. - assert [0, nil].include?(klass.columns_hash['omit'].default) + assert klass.columns_hash['omit'].default.either?(0, nil) assert !klass.columns_hash['omit'].null assert_raise(ActiveRecord::StatementInvalid) { klass.create! } -- cgit v1.2.3 From a30b440debe87722c0c072555db7945181ceffde Mon Sep 17 00:00:00 2001 From: Brian Cardarella Date: Sat, 19 Mar 2011 13:28:36 -0400 Subject: Refactored uniqueness validator to use Arel instead of hardcoded SQL --- .../lib/active_record/validations/uniqueness.rb | 37 +++++++--------------- 1 file changed, 11 insertions(+), 26 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index 9cd6c26322..37381d063d 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -14,6 +14,7 @@ module ActiveRecord def validate_each(record, attribute, value) finder_class = find_finder_class_for(record) + table = finder_class.arel_table coder = record.class.serialized_attributes[attribute.to_s] @@ -21,21 +22,15 @@ module ActiveRecord value = coder.dump value end - sql, params = mount_sql_and_params(finder_class, record.class.quoted_table_name, attribute, value) - - relation = finder_class.unscoped.where(sql, *params) + relation = build_relation(finder_class, table, attribute, value) + relation = relation.and(table[finder_class.primary_key.to_sym].not_eq(record.send(:id))) if record.persisted? Array.wrap(options[:scope]).each do |scope_item| scope_value = record.send(scope_item) - relation = relation.where(scope_item => scope_value) - end - - if record.persisted? - # TODO : This should be in Arel - relation = relation.where("#{record.class.quoted_table_name}.#{record.class.primary_key} <> ?", record.send(:id)) + relation = relation.and(table[scope_item].eq(scope_value)) end - if relation.exists? + if finder_class.unscoped.where(relation).exists? record.errors.add(attribute, :taken, options.except(:case_sensitive, :scope).merge(:value => value)) end end @@ -57,27 +52,17 @@ module ActiveRecord class_hierarchy.detect { |klass| !klass.abstract_class? } end - def mount_sql_and_params(klass, table_name, attribute, value) #:nodoc: + def build_relation(klass, table, attribute, value) #:nodoc: column = klass.columns_hash[attribute.to_s] + value = column.limit ? value.to_s.mb_chars[0, column.limit] : value.to_s if column.text? - operator = if value.nil? - "IS ?" - elsif column.text? - value = column.limit ? value.to_s.mb_chars[0, column.limit] : value.to_s - "#{klass.connection.case_sensitive_equality_operator} ?" - else - "= ?" - end - - sql_attribute = "#{table_name}.#{klass.connection.quote_column_name(attribute)}" - - if value.nil? || (options[:case_sensitive] || !column.text?) - sql = "#{sql_attribute} #{operator}" + if !options[:case_sensitive] && column.text? + relation = table[attribute].matches(value) else - sql = "LOWER(#{sql_attribute}) = LOWER(?)" + relation = table[attribute].eq(value) end - [sql, [value]] + relation end end -- cgit v1.2.3 From 35dba50be9f9641ba757e6253b6e39f4b92dd37f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 10 Apr 2011 22:30:27 -0700 Subject: community support for pg < 8.2 has ended, so we can drop support for those versions --- .../connection_adapters/postgresql_adapter.rb | 74 ++++------------------ 1 file changed, 12 insertions(+), 62 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 5a830a50fb..acd4c5cc4a 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -229,6 +229,11 @@ module ActiveRecord @statements = {} connect + + if postgresql_version < 80200 + raise "Your version of PostgreSQL (#{postgresql_version}) is too old, please upgrade!" + end + @local_tz = execute('SHOW TIME ZONE').first["TimeZone"] end @@ -299,7 +304,7 @@ module ActiveRecord end def supports_insert_with_returning? - postgresql_version >= 80200 + true end def supports_ddl_transactions? @@ -310,10 +315,9 @@ module ActiveRecord true end - # Returns the configured supported identifier length supported by PostgreSQL, - # or report the default of 63 on PostgreSQL 7.x. + # Returns the configured supported identifier length supported by PostgreSQL def table_alias_length - @table_alias_length ||= (postgresql_version >= 80000 ? query('SHOW max_identifier_length')[0][0].to_i : 63) + @table_alias_length ||= query('SHOW max_identifier_length')[0][0].to_i end # QUOTING ================================================== @@ -404,7 +408,7 @@ module ActiveRecord # REFERENTIAL INTEGRITY ==================================== def supports_disable_referential_integrity?() #:nodoc: - postgresql_version >= 80100 + true end def disable_referential_integrity #:nodoc: @@ -632,15 +636,7 @@ module ActiveRecord # Example: # drop_database 'matt_development' def drop_database(name) #:nodoc: - if postgresql_version >= 80200 - execute "DROP DATABASE IF EXISTS #{quote_table_name(name)}" - else - begin - execute "DROP DATABASE #{quote_table_name(name)}" - rescue ActiveRecord::StatementInvalid - @logger.warn "#{name} database doesn't exist." if @logger - end - end + execute "DROP DATABASE IF EXISTS #{quote_table_name(name)}" end # Returns the list of all tables in the schema search path or a specified schema. @@ -803,28 +799,6 @@ module ActiveRecord AND dep.refobjid = '#{quote_table_name(table)}'::regclass end_sql - if result.nil? or result.empty? - # If that fails, try parsing the primary key's default value. - # Support the 7.x and 8.0 nextval('foo'::text) as well as - # the 8.1+ nextval('foo'::regclass). - result = query(<<-end_sql, 'PK and custom sequence')[0] - SELECT attr.attname, - CASE - WHEN split_part(def.adsrc, '''', 2) ~ '.' THEN - substr(split_part(def.adsrc, '''', 2), - strpos(split_part(def.adsrc, '''', 2), '.')+1) - ELSE split_part(def.adsrc, '''', 2) - END - FROM pg_class t - JOIN pg_attribute attr ON (t.oid = attrelid) - JOIN pg_attrdef def ON (adrelid = attrelid AND adnum = attnum) - JOIN pg_constraint cons ON (conrelid = adrelid AND adnum = conkey[1]) - WHERE t.oid = '#{quote_table_name(table)}'::regclass - AND cons.contype = 'p' - AND def.adsrc ~* 'nextval' - end_sql - end - # [primary_key, sequence] [result.first, result.last] rescue @@ -848,38 +822,14 @@ module ActiveRecord add_column_sql = "ALTER TABLE #{quote_table_name(table_name)} ADD COLUMN #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}" add_column_options!(add_column_sql, options) - begin - execute add_column_sql - rescue ActiveRecord::StatementInvalid => e - raise e if postgresql_version > 80000 - - execute("ALTER TABLE #{quote_table_name(table_name)} ADD COLUMN #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}") - change_column_default(table_name, column_name, options[:default]) if options_include_default?(options) - change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) - end + execute add_column_sql end # Changes the column of a table. def change_column(table_name, column_name, type, options = {}) quoted_table_name = quote_table_name(table_name) - begin - execute "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quote_column_name(column_name)} TYPE #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}" - rescue ActiveRecord::StatementInvalid => e - raise e if postgresql_version > 80000 - # This is PostgreSQL 7.x, so we have to use a more arcane way of doing it. - begin - begin_db_transaction - tmp_column_name = "#{column_name}_ar_tmp" - add_column(table_name, tmp_column_name, type, options) - execute "UPDATE #{quoted_table_name} SET #{quote_column_name(tmp_column_name)} = CAST(#{quote_column_name(column_name)} AS #{type_to_sql(type, options[:limit], options[:precision], options[:scale])})" - remove_column(table_name, column_name) - rename_column(table_name, tmp_column_name, column_name) - commit_db_transaction - rescue - rollback_db_transaction - end - end + execute "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quote_column_name(column_name)} TYPE #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}" change_column_default(table_name, column_name, options[:default]) if options_include_default?(options) change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) -- cgit v1.2.3 From 743b6631dfc5b91b711f09eb13099cdf9dd89c1b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 10 Apr 2011 22:34:27 -0700 Subject: adding pg support notes to the changelog --- activerecord/CHANGELOG | 2 ++ 1 file changed, 2 insertions(+) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index e536d2b408..88c2562619 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *Rails 3.1.0 (unreleased)* +* PostgreSQL adapter only supports PostgreSQL version 8.2 and higher. + * ConnectionManagement middleware is changed to clean up the connection pool after the rack body has been flushed. -- cgit v1.2.3 From 1f3d3eb49d16b62250c24e3374cc36de99b397b8 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 10 Apr 2011 18:02:57 -0700 Subject: pg should define insert_sql so that query cache actually works for inserts --- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index acd4c5cc4a..7546afcc18 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -431,7 +431,7 @@ module ActiveRecord end # Executes an INSERT query and returns the new record's ID - def insert(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) + def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) # Extract the table from the insert sql. Yuck. table = sql.split(" ", 4)[2].gsub('"', '') @@ -440,7 +440,6 @@ module ActiveRecord pk, sequence_name = *pk_and_sequence_for(table) unless pk if pk id = select_value("#{sql} RETURNING #{quote_column_name(pk)}") - clear_query_cache return id end end -- cgit v1.2.3 From 35b2715456999662cc34390e258962738aaa8dc7 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 10 Apr 2011 23:53:02 -0700 Subject: only support pg >= 8.2, so no need to check --- .../active_record/connection_adapters/postgresql_adapter.rb | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 7546afcc18..15ced08499 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -435,13 +435,10 @@ module ActiveRecord # Extract the table from the insert sql. Yuck. table = sql.split(" ", 4)[2].gsub('"', '') - # Try an insert with 'returning id' if available (PG >= 8.2) - if supports_insert_with_returning? - pk, sequence_name = *pk_and_sequence_for(table) unless pk - if pk - id = select_value("#{sql} RETURNING #{quote_column_name(pk)}") - return id - end + pk, sequence_name = *pk_and_sequence_for(table) unless pk + if pk + id = select_value("#{sql} RETURNING #{quote_column_name(pk)}") + return id end # Otherwise, insert then grab last_insert_id. -- cgit v1.2.3 From c4fc3963003f53cb963435ea3733bd3e8164c803 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 11 Apr 2011 10:02:39 -0700 Subject: adding a case_sensitive_modifier for forcing comparisons to be case sensitive --- .../lib/active_record/connection_adapters/abstract_adapter.rb | 4 ++++ activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb | 5 +++++ activerecord/lib/active_record/connection_adapters/mysql_adapter.rb | 5 +++++ activerecord/lib/active_record/validations/uniqueness.rb | 1 + 4 files changed, 15 insertions(+) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 0f44baa2fe..b0e6136e12 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -203,6 +203,10 @@ module ActiveRecord def release_savepoint end + def case_sensitive_modifier(node) + node + end + def current_savepoint_name "active_record_#{open_transactions}" end diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 7bad511c64..7e237db357 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -528,6 +528,11 @@ module ActiveRecord def case_sensitive_equality_operator "= BINARY" end + deprecate :case_sensitive_equality_operator + + def case_sensitive_modifier(node) + Arel::Nodes::Bin.new(node) + end def limited_update_conditions(where_sql, quoted_table_name, quoted_primary_key) where_sql diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index e1186209d3..ceee2ab73a 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -655,6 +655,11 @@ module ActiveRecord def case_sensitive_equality_operator "= BINARY" end + deprecate :case_sensitive_equality_operator + + def case_sensitive_modifier(node) + Arel::Nodes::Bin.new(node) + end def limited_update_conditions(where_sql, quoted_table_name, quoted_primary_key) where_sql diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index 37381d063d..d1225a9ed9 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -59,6 +59,7 @@ module ActiveRecord if !options[:case_sensitive] && column.text? relation = table[attribute].matches(value) else + value = klass.connection.case_sensitive_modifier(value) relation = table[attribute].eq(value) end -- cgit v1.2.3 From 32dbf00d99c28b0ce143ceb5f94abcd93019eb61 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 8 Apr 2011 14:50:42 -0700 Subject: adding exec_insert to sqlite3 --- .../lib/active_record/connection_adapters/sqlite_adapter.rb | 4 ++++ .../test/cases/adapters/sqlite3/sqlite3_adapter_test.rb | 12 ++++++++++++ 2 files changed, 16 insertions(+) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 32229a8410..5c4d76be98 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -173,6 +173,10 @@ module ActiveRecord end end + def exec_insert(sql, name, binds) + exec_query(sql, name, binds) + end + def execute(sql, name = nil) #:nodoc: log(sql, name) { @connection.execute(sql) } end diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index a2ed3302aa..600dc581f2 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -19,6 +19,18 @@ module ActiveRecord eosql end + def test_exec_insert + column = @conn.columns('items').find { |col| col.name == 'number' } + vals = [[column, 10]] + @conn.exec_insert('insert into items (number) VALUES (?)', 'SQL', vals) + + result = @conn.exec_query( + 'select number from items where number = ?', 'SQL', vals) + + assert_equal 1, result.rows.length + assert_equal 10, result.rows.first.first + end + def test_primary_key_returns_nil_for_no_pk @conn.exec_query('create table ex(id int, data string)') assert_nil @conn.primary_key('ex') -- cgit v1.2.3 From f9d3f018669bc96b107cfa802d2fa9b0b5555717 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 8 Apr 2011 16:44:54 -0700 Subject: properly name schema queries for logging --- activerecord/lib/active_record/connection_adapters/mysql_adapter.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index ceee2ab73a..33add72283 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -506,7 +506,7 @@ module ActiveRecord def tables(name = nil, database = nil) #:nodoc: tables = [] - result = execute(["SHOW TABLES", database].compact.join(' IN '), name) + result = execute(["SHOW TABLES", database].compact.join(' IN '), 'SCHEMA') result.each { |field| tables << field[0] } result.free tables @@ -551,7 +551,7 @@ module ActiveRecord def columns(table_name, name = nil)#:nodoc: sql = "SHOW FIELDS FROM #{quote_table_name(table_name)}" columns = [] - result = execute(sql) + result = execute(sql, 'SCHEMA') result.each { |field| columns << MysqlColumn.new(field[0], field[4], field[1], field[2] == "YES") } result.free columns @@ -638,7 +638,7 @@ module ActiveRecord # Returns a table's primary key and belonging sequence. def pk_and_sequence_for(table) #:nodoc: keys = [] - result = execute("describe #{quote_table_name(table)}") + result = execute("describe #{quote_table_name(table)}", 'SCHEMA') result.each_hash do |h| keys << h["Field"]if h["Key"] == "PRI" end -- cgit v1.2.3 From 0845b5062d118197f2d1531dfbb9dd9ddd90bef2 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 8 Apr 2011 17:37:10 -0700 Subject: adding client_encoding method for discovering the encoding set for this client, testing exec_insert on a string --- .../connection_adapters/mysql_adapter.rb | 62 ++++++++++++++++++++++ 1 file changed, 62 insertions(+) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 33add72283..d88075fdea 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -196,6 +196,7 @@ module ActiveRecord @connection_options, @config = connection_options, config @quoted_column_names, @quoted_table_names = {}, {} @statements = {} + @client_encoding = nil connect end @@ -330,6 +331,63 @@ module ActiveRecord @statements.clear end + if "<3".respond_to?(:encode) + # Taken from here: + # https://github.com/tmtm/ruby-mysql/blob/master/lib/mysql/charset.rb + # Author: TOMITA Masahiro + ENCODINGS = { + "armscii8" => nil, + "ascii" => Encoding::US_ASCII, + "big5" => Encoding::Big5, + "binary" => Encoding::ASCII_8BIT, + "cp1250" => Encoding::Windows_1250, + "cp1251" => Encoding::Windows_1251, + "cp1256" => Encoding::Windows_1256, + "cp1257" => Encoding::Windows_1257, + "cp850" => Encoding::CP850, + "cp852" => Encoding::CP852, + "cp866" => Encoding::IBM866, + "cp932" => Encoding::Windows_31J, + "dec8" => nil, + "eucjpms" => Encoding::EucJP_ms, + "euckr" => Encoding::EUC_KR, + "gb2312" => Encoding::EUC_CN, + "gbk" => Encoding::GBK, + "geostd8" => nil, + "greek" => Encoding::ISO_8859_7, + "hebrew" => Encoding::ISO_8859_8, + "hp8" => nil, + "keybcs2" => nil, + "koi8r" => Encoding::KOI8_R, + "koi8u" => Encoding::KOI8_U, + "latin1" => Encoding::ISO_8859_1, + "latin2" => Encoding::ISO_8859_2, + "latin5" => Encoding::ISO_8859_9, + "latin7" => Encoding::ISO_8859_13, + "macce" => Encoding::MacCentEuro, + "macroman" => Encoding::MacRoman, + "sjis" => Encoding::SHIFT_JIS, + "swe7" => nil, + "tis620" => Encoding::TIS_620, + "ucs2" => Encoding::UTF_16BE, + "ujis" => Encoding::EucJP_ms, + "utf8" => Encoding::UTF_8, + "utf8mb4" => Encoding::UTF_8, + } + else + ENCODINGS = Hash.new { |h,k| h[k] = k } + end + + # Get the client encoding for this database + def client_encoding + return @client_encoding if @client_encoding + + result = exec_query( + "SHOW VARIABLES WHERE Variable_name = 'character_set_client'", + 'SCHEMA') + @client_encoding = ENCODINGS[result.rows.last.last] + end + def exec_query(sql, name = 'SQL', binds = []) log(sql, name, binds) do result = nil @@ -363,6 +421,10 @@ module ActiveRecord end end + def exec_insert(sql, name, binds) + exec_query(sql, name, binds) + end + def exec_without_stmt(sql, name = 'SQL') # :nodoc: # Some queries, like SHOW CREATE TABLE don't work through the prepared # statement API. For those queries, we need to use this method. :'( -- cgit v1.2.3 From 3d96e6217bb117ae50d8cb06bc27e3582f5ba850 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 8 Apr 2011 17:37:27 -0700 Subject: adding mysql adapter test case --- .../cases/adapters/mysql/mysql_adapter_test.rb | 69 ++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb (limited to 'activerecord') diff --git a/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb new file mode 100644 index 0000000000..0d8eb55388 --- /dev/null +++ b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb @@ -0,0 +1,69 @@ +# encoding: utf-8 + +require "cases/helper" + +module ActiveRecord + module ConnectionAdapters + class MysqlAdapterTest < ActiveRecord::TestCase + def setup + @conn = ActiveRecord::Base.connection + @conn.exec_query('drop table if exists ex') + @conn.exec_query(<<-eosql) + CREATE TABLE `ex` ( + `id` int(11) DEFAULT NULL auto_increment PRIMARY KEY, + `number` integer, + `data` varchar(255)) + eosql + end + + def test_client_encoding + if "<3".respond_to?(:encoding) + assert_equal Encoding::UTF_8, @conn.client_encoding + else + assert_equal 'utf8', @conn.client_encoding + end + end + + def test_exec_insert_number + insert(@conn, 'number' => 10) + + result = @conn.exec_query('SELECT number FROM ex WHERE number = 10') + + assert_equal 1, result.rows.length + assert_equal 10, result.rows.last.last + end + + def test_exec_insert_string + str = 'いただきます！' + insert(@conn, 'number' => 10, 'data' => str) + + result = @conn.exec_query('SELECT number, data FROM ex WHERE number = 10') + + value = result.rows.last.last + + if "<3".respond_to?(:encoding) + # FIXME: this should probably be inside the mysql AR adapter? + value.force_encoding(@conn.client_encoding) + + # The strings in this file are utf-8, so transcode to utf-8 + value.encode!(Encoding::UTF_8) + end + + assert_equal str, value + end + + private + def insert(ctx, data) + binds = data.map { |name, value| + [@conn.columns('ex').find { |x| x.name == name }, value] + } + columns = binds.map(&:first).map(&:name) + + sql = "INSERT INTO ex (#{columns.join(", ")}) + VALUES (#{(['?'] * columns.length).join(', ')})" + + ctx.exec_insert(sql, 'SQL', binds) + end + end + end +end -- cgit v1.2.3 From 90a371496a7a44b74879011d4e316ffaa6fcf299 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 8 Apr 2011 17:45:18 -0700 Subject: properly name schema queries for the logger --- .../connection_adapters/postgresql_adapter.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 15ced08499..08329b33ed 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -234,7 +234,7 @@ module ActiveRecord raise "Your version of PostgreSQL (#{postgresql_version}) is too old, please upgrade!" end - @local_tz = execute('SHOW TIME ZONE').first["TimeZone"] + @local_tz = execute('SHOW TIME ZONE', 'SCHEMA').first["TimeZone"] end def clear_cache! @@ -298,7 +298,7 @@ module ActiveRecord # Enable standard-conforming strings if available. def set_standard_conforming_strings old, self.client_min_messages = client_min_messages, 'panic' - execute('SET standard_conforming_strings = on') rescue nil + execute('SET standard_conforming_strings = on', 'SCHEMA') rescue nil ensure self.client_min_messages = old end @@ -637,7 +637,7 @@ module ActiveRecord # Returns the list of all tables in the schema search path or a specified schema. def tables(name = nil) - query(<<-SQL, name).map { |row| row[0] } + query(<<-SQL, 'SCHEMA').map { |row| row[0] } SELECT tablename FROM pg_tables WHERE schemaname = ANY (current_schemas(false)) @@ -658,7 +658,7 @@ module ActiveRecord schema = nil end - query(<<-SQL).first[0].to_i > 0 + query(<<-SQL, 'SCHEMA').first[0].to_i > 0 SELECT COUNT(*) FROM pg_tables WHERE tablename = '#{table.gsub(/(^"|"$)/,'')}' @@ -740,12 +740,12 @@ module ActiveRecord # Returns the current client message level. def client_min_messages - query('SHOW client_min_messages')[0][0] + query('SHOW client_min_messages', 'SCHEMA')[0][0] end # Set the client message level. def client_min_messages=(level) - execute("SET client_min_messages TO '#{level}'") + execute("SET client_min_messages TO '#{level}'", 'SCHEMA') end # Returns the sequence name for a table's primary key or some other specified key. @@ -778,7 +778,7 @@ module ActiveRecord def pk_and_sequence_for(table) #:nodoc: # First try looking for a sequence with a dependency on the # given table's primary key. - result = exec_query(<<-end_sql, 'PK and serial sequence').rows.first + result = exec_query(<<-end_sql, 'SCHEMA').rows.first SELECT attr.attname, seq.relname FROM pg_class seq, pg_attribute attr, @@ -977,9 +977,9 @@ module ActiveRecord # If using Active Record's time zone support configure the connection to return # TIMESTAMP WITH ZONE types in UTC. if ActiveRecord::Base.default_timezone == :utc - execute("SET time zone 'UTC'") + execute("SET time zone 'UTC'", 'SCHEMA') elsif @local_tz - execute("SET time zone '#{@local_tz}'") + execute("SET time zone '#{@local_tz}'", 'SCHEMA') end end @@ -1022,7 +1022,7 @@ module ActiveRecord # - format_type includes the column size constraint, e.g. varchar(50) # - ::regclass is a function that gives the id for a table name def column_definitions(table_name) #:nodoc: - exec_query(<<-end_sql).rows + exec_query(<<-end_sql, 'SCHEMA').rows SELECT a.attname, format_type(a.atttypid, a.atttypmod), d.adsrc, a.attnotnull FROM pg_attribute a LEFT JOIN pg_attrdef d ON a.attrelid = d.adrelid AND a.attnum = d.adnum -- cgit v1.2.3 From b1ba04b32be25596d957a301553dd03c97588e14 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 8 Apr 2011 17:58:00 -0700 Subject: fixing variable name in mysql test --- activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb index 0d8eb55388..146b77a95c 100644 --- a/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb +++ b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb @@ -55,7 +55,7 @@ module ActiveRecord private def insert(ctx, data) binds = data.map { |name, value| - [@conn.columns('ex').find { |x| x.name == name }, value] + [ctx.columns('ex').find { |x| x.name == name }, value] } columns = binds.map(&:first).map(&:name) -- cgit v1.2.3 From 58259bbf29b55571da7708607463e61a9b69565d Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 8 Apr 2011 18:19:24 -0700 Subject: adding exec_insert for postgresql --- .../connection_adapters/postgresql_adapter.rb | 4 +++ .../adapters/postgresql/postgresql_adapter_test.rb | 38 +++++++++++++++++++++- .../cases/adapters/sqlite3/sqlite3_adapter_test.rb | 2 ++ 3 files changed, 43 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 08329b33ed..a0be143c8e 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -554,6 +554,10 @@ module ActiveRecord end end + def exec_insert(sql, name, binds) + exec_query(sql, name, binds) + end + # Executes an UPDATE query and returns the number of affected tuples. def update_sql(sql, name = nil) super.cmd_tuples diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index b0a4a4e39d..861e3344b2 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -1,3 +1,4 @@ +# encoding: utf-8 require "cases/helper" module ActiveRecord @@ -6,7 +7,27 @@ module ActiveRecord def setup @connection = ActiveRecord::Base.connection @connection.exec_query('drop table if exists ex') - @connection.exec_query('create table ex(id serial primary key, data character varying(255))') + @connection.exec_query('create table ex(id serial primary key, number integer, data character varying(255))') + end + + def test_exec_insert_number + insert(@connection, 'number' => 10) + + result = @connection.exec_query('SELECT number FROM ex WHERE number = 10') + + assert_equal 1, result.rows.length + assert_equal "10", result.rows.last.last + end + + def test_exec_insert_string + str = 'いただきます！' + insert(@connection, 'number' => 10, 'data' => str) + + result = @connection.exec_query('SELECT number, data FROM ex WHERE number = 10') + + value = result.rows.last.last + + assert_equal str, value end def test_table_alias_length @@ -63,6 +84,21 @@ module ActiveRecord bind = @connection.substitute_for(nil, [nil]) assert_equal Arel.sql('$2'), bind end + + private + def insert(ctx, data) + binds = data.map { |name, value| + [ctx.columns('ex').find { |x| x.name == name }, value] + } + columns = binds.map(&:first).map(&:name) + + bind_subs = columns.length.times.map { |x| "$#{x + 1}" } + + sql = "INSERT INTO ex (#{columns.join(", ")}) + VALUES (#{bind_subs.join(', ')})" + + ctx.exec_insert(sql, 'SQL', binds) + end end end end diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index 600dc581f2..0e2f468908 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -126,6 +126,8 @@ module ActiveRecord end def test_quote_binary_column_escapes_it + return unless "<3".respond_to?(:encode) + DualEncoding.connection.execute(<<-eosql) CREATE TABLE dual_encodings ( id integer PRIMARY KEY AUTOINCREMENT, -- cgit v1.2.3 From 8a11799a474d6cc1dc599e8823ce5bbb012944e7 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 10 Apr 2011 18:12:42 -0700 Subject: make sqlite insert_sql more consistent with other adapters --- activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 5c4d76be98..8ef286b473 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -192,7 +192,8 @@ module ActiveRecord end def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) #:nodoc: - super || @connection.last_insert_row_id + super + id_value || @connection.last_insert_row_id end alias :create :insert_sql -- cgit v1.2.3 From 269cd1b3c55c6ceb69c167ad97c357b56f49cb4b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 10 Apr 2011 19:02:06 -0700 Subject: implement exec_query on mysql2 adapter --- .../connection_adapters/mysql2_adapter.rb | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 7e237db357..1f7e527eeb 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -592,8 +592,26 @@ module ActiveRecord # Returns an array of record hashes with the column names as keys and # column values as values. - def select(sql, name = nil) - execute(sql, name).each(:as => :hash) + def select(sql, name = nil, binds = []) + exec_query(sql, name, binds).to_a + end + + def exec_query(sql, name = 'SQL', binds = []) + @connection.query_options[:database_timezone] = ActiveRecord::Base.default_timezone + + log(sql, name, binds) do + begin + result = @connection.query(sql) + rescue ActiveRecord::StatementInvalid => exception + if exception.message.split(":").first =~ /Packets out of order/ + raise ActiveRecord::StatementInvalid, "'Packets out of order' error was received from the database. Please update your mysql bindings (gem install mysql) and read http://dev.mysql.com/doc/mysql/en/password-hashing.html for more information. If you're on Windows, use the Instant Rails installer to get the updated mysql bindings." + else + raise + end + end + + ActiveRecord::Result.new(result.fields, result.to_a) + end end def supports_views? -- cgit v1.2.3 From 4c30304e159b2a8fafb5b3c5c8b6d996e490e657 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 10 Apr 2011 19:08:20 -0700 Subject: updating the docco for ActiveRecord::Result --- activerecord/lib/active_record/result.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/result.rb b/activerecord/lib/active_record/result.rb index 0465b21e88..243012f88c 100644 --- a/activerecord/lib/active_record/result.rb +++ b/activerecord/lib/active_record/result.rb @@ -1,9 +1,9 @@ module ActiveRecord ### - # This class encapsulates a Result returned from calling +exec+ on any + # This class encapsulates a Result returned from calling +exec_query+ on any # database connection adapter. For example: # - # x = ActiveRecord::Base.connection.exec('SELECT * FROM foo') + # x = ActiveRecord::Base.connection.exec_query('SELECT * FROM foo') # x # => # class Result include Enumerable -- cgit v1.2.3 From 302b6f3f73731c8daa108f9c40d7cf41463898d8 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 11 Apr 2011 10:31:13 -0700 Subject: pg does not know the insert_id in advance, so super will never return true --- .../connection_adapters/postgresql_adapter.rb | 23 ++++++++++------------ 1 file changed, 10 insertions(+), 13 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index a0be143c8e..a90e9a4b71 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -441,20 +441,17 @@ module ActiveRecord return id end - # Otherwise, insert then grab last_insert_id. - if insert_id = super - insert_id - else - # If neither pk nor sequence name is given, look them up. - unless pk || sequence_name - pk, sequence_name = *pk_and_sequence_for(table) - end + super - # If a pk is given, fallback to default sequence name. - # Don't fetch last insert id for a table without a pk. - if pk && sequence_name ||= default_sequence_name(table, pk) - last_insert_id(sequence_name) - end + # If neither pk nor sequence name is given, look them up. + unless pk || sequence_name + pk, sequence_name = *pk_and_sequence_for(table) + end + + # If a pk is given, fallback to default sequence name. + # Don't fetch last insert id for a table without a pk. + if pk && sequence_name ||= default_sequence_name(table, pk) + last_insert_id(sequence_name) end end alias :create :insert -- cgit v1.2.3 From cbb65de1a60f1263cab05627ab99c9f5e246cd5c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 11 Apr 2011 10:37:23 -0700 Subject: always look up pk and sequence unless both are provided --- .../active_record/connection_adapters/postgresql_adapter.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index a90e9a4b71..617d131c40 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -435,7 +435,11 @@ module ActiveRecord # Extract the table from the insert sql. Yuck. table = sql.split(" ", 4)[2].gsub('"', '') - pk, sequence_name = *pk_and_sequence_for(table) unless pk + # If neither pk nor sequence name is given, look them up. + unless pk || sequence_name + pk, sequence_name = *pk_and_sequence_for(table) + end + if pk id = select_value("#{sql} RETURNING #{quote_column_name(pk)}") return id @@ -443,11 +447,6 @@ module ActiveRecord super - # If neither pk nor sequence name is given, look them up. - unless pk || sequence_name - pk, sequence_name = *pk_and_sequence_for(table) - end - # If a pk is given, fallback to default sequence name. # Don't fetch last insert id for a table without a pk. if pk && sequence_name ||= default_sequence_name(table, pk) -- cgit v1.2.3 From 9ba94c8fc4e7d5577e87aca07f92c9f51f08bdba Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 11 Apr 2011 10:54:08 -0700 Subject: we know the table and pk, so we can calculate a default sequence name --- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 617d131c40..392eadb9d5 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -447,11 +447,12 @@ module ActiveRecord super + return unless pk + # If a pk is given, fallback to default sequence name. # Don't fetch last insert id for a table without a pk. - if pk && sequence_name ||= default_sequence_name(table, pk) - last_insert_id(sequence_name) - end + sequence_name ||= "#{table}_#{pk}_seq" + last_insert_id(sequence_name) end alias :create :insert -- cgit v1.2.3 From 5df072d64bf24213c1eb99b3762b4fd597e28903 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 11 Apr 2011 13:10:20 -0700 Subject: last insert id can never be called, so remove that code path --- .../connection_adapters/postgresql_adapter.rb | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 392eadb9d5..c179be828e 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -441,18 +441,10 @@ module ActiveRecord end if pk - id = select_value("#{sql} RETURNING #{quote_column_name(pk)}") - return id + select_value("#{sql} RETURNING #{quote_column_name(pk)}") + else + super end - - super - - return unless pk - - # If a pk is given, fallback to default sequence name. - # Don't fetch last insert id for a table without a pk. - sequence_name ||= "#{table}_#{pk}_seq" - last_insert_id(sequence_name) end alias :create :insert -- cgit v1.2.3 From a9e8554b46a29573094eec139b16778e29e5c37a Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 11 Apr 2011 13:57:00 -0700 Subject: use prepared statements for primary key queries --- .../connection_adapters/postgresql_adapter.rb | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index c179be828e..468f8d75d7 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -796,8 +796,21 @@ module ActiveRecord # Returns just a table's primary key def primary_key(table) - pk_and_sequence = pk_and_sequence_for(table) - pk_and_sequence && pk_and_sequence.first + row = exec_query(<<-end_sql, 'SCHEMA', [[nil, table]]).rows.first + SELECT DISTINCT(attr.attname) + FROM pg_attribute attr, + pg_depend dep, + pg_namespace name, + pg_constraint cons + WHERE attr.attrelid = dep.refobjid + AND attr.attnum = dep.refobjsubid + AND attr.attrelid = cons.conrelid + AND attr.attnum = cons.conkey[1] + AND cons.contype = 'p' + AND dep.refobjid = $1::regclass + end_sql + + row && row.first end # Renames a table. -- cgit v1.2.3 From f6c0c8ff6101fed24dd176fb0cce0383807c502d Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 11 Apr 2011 14:08:27 -0700 Subject: only use the primary_key method, refactor schema and table name parsing --- .../connection_adapters/postgresql_adapter.rb | 28 ++++++++++++---------- 1 file changed, 15 insertions(+), 13 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 468f8d75d7..d76b03e80a 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -433,12 +433,9 @@ module ActiveRecord # Executes an INSERT query and returns the new record's ID def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) # Extract the table from the insert sql. Yuck. - table = sql.split(" ", 4)[2].gsub('"', '') + _, table = extract_schema_and_table(sql.split(" ", 4)[2]) - # If neither pk nor sequence name is given, look them up. - unless pk || sequence_name - pk, sequence_name = *pk_and_sequence_for(table) - end + pk ||= primary_key(table) if pk select_value("#{sql} RETURNING #{quote_column_name(pk)}") @@ -638,7 +635,18 @@ module ActiveRecord end def table_exists?(name) - name = name.to_s + schema, table = extract_schema_and_table(name.to_s) + + query(<<-SQL, 'SCHEMA').first[0].to_i > 0 + SELECT COUNT(*) + FROM pg_tables + WHERE tablename = '#{table.gsub(/(^"|"$)/,'')}' + #{schema ? "AND schemaname = '#{schema}'" : ''} + SQL + end + + # Extracts the table and schema name from +name+ + def extract_schema_and_table(name) schema, table = name.split('.', 2) unless table # A table was provided without a schema @@ -650,13 +658,7 @@ module ActiveRecord table = name schema = nil end - - query(<<-SQL, 'SCHEMA').first[0].to_i > 0 - SELECT COUNT(*) - FROM pg_tables - WHERE tablename = '#{table.gsub(/(^"|"$)/,'')}' - #{schema ? "AND schemaname = '#{schema}'" : ''} - SQL + [schema, table] end # Returns the list of all indexes for a table. -- cgit v1.2.3 From 75dc9fbac76a2da78b8d21e1ede16fea38d16564 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 11 Apr 2011 14:12:31 -0700 Subject: cache table exists queries in prepared statement cache --- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index d76b03e80a..015d5eb646 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -637,11 +637,14 @@ module ActiveRecord def table_exists?(name) schema, table = extract_schema_and_table(name.to_s) - query(<<-SQL, 'SCHEMA').first[0].to_i > 0 + binds = [[nil, table.gsub(/(^"|"$)/,'')]] + binds << [nil, schema] if schema + + exec_query(<<-SQL, 'SCHEMA', binds).rows.first[0].to_i > 0 SELECT COUNT(*) FROM pg_tables - WHERE tablename = '#{table.gsub(/(^"|"$)/,'')}' - #{schema ? "AND schemaname = '#{schema}'" : ''} + WHERE tablename = $1 + #{schema ? "AND schemaname = $2" : ''} SQL end -- cgit v1.2.3 From 622f23b604546784aeab3cca5a47f0d6b281cf91 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 11 Apr 2011 14:38:42 -0700 Subject: wrap the pg_get_serial_sequence function and reuse it for the default sequence name --- .../connection_adapters/postgresql_adapter.rb | 12 +++++++++-- .../adapters/postgresql/postgresql_adapter_test.rb | 25 ++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 015d5eb646..2283a5bb68 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -748,8 +748,16 @@ module ActiveRecord # Returns the sequence name for a table's primary key or some other specified key. def default_sequence_name(table_name, pk = nil) #:nodoc: - default_pk, default_seq = pk_and_sequence_for(table_name) - default_seq || "#{table_name}_#{pk || default_pk || 'id'}_seq" + serial_sequence(table_name, pk || 'id').split('.').last + rescue ActiveRecord::StatementInvalid + "#{table_name}_#{pk || 'id'}_seq" + end + + def serial_sequence(table, column) + result = exec_query(<<-eosql, 'SCHEMA', [[nil, table], [nil, column]]) + SELECT pg_get_serial_sequence($1, $2) + eosql + result.rows.first.first end # Resets the sequence of a table's primary key to the maximum value. diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index 861e3344b2..2d412a6e2a 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -10,6 +10,31 @@ module ActiveRecord @connection.exec_query('create table ex(id serial primary key, number integer, data character varying(255))') end + def test_serial_sequence + assert_equal 'public.accounts_id_seq', + @connection.serial_sequence('accounts', 'id') + + assert_raises(ActiveRecord::StatementInvalid) do + @connection.serial_sequence('zomg', 'id') + end + end + + def test_default_sequence_name + assert_equal 'accounts_id_seq', + @connection.default_sequence_name('accounts', 'id') + + assert_equal 'accounts_id_seq', + @connection.default_sequence_name('accounts') + end + + def test_default_sequence_name_bad_table + assert_equal 'zomg_id_seq', + @connection.default_sequence_name('zomg', 'id') + + assert_equal 'zomg_id_seq', + @connection.default_sequence_name('zomg') + end + def test_exec_insert_number insert(@connection, 'number' => 10) -- cgit v1.2.3 From 5918b868b24ff384365d436367611aea577f3723 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 11 Apr 2011 15:19:10 -0700 Subject: remove so many nested if statements --- .../connection_adapters/postgresql_adapter.rb | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 2283a5bb68..1c2cc7d5fa 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -764,19 +764,21 @@ module ActiveRecord def reset_pk_sequence!(table, pk = nil, sequence = nil) #:nodoc: unless pk and sequence default_pk, default_sequence = pk_and_sequence_for(table) + pk ||= default_pk sequence ||= default_sequence end - if pk - if sequence - quoted_sequence = quote_column_name(sequence) - select_value <<-end_sql, 'Reset sequence' - SELECT setval('#{quoted_sequence}', (SELECT COALESCE(MAX(#{quote_column_name pk})+(SELECT increment_by FROM #{quoted_sequence}), (SELECT min_value FROM #{quoted_sequence})) FROM #{quote_table_name(table)}), false) - end_sql - else - @logger.warn "#{table} has primary key #{pk} with no default sequence" if @logger - end + if @logger && pk && !sequence + @logger.warn "#{table} has primary key #{pk} with no default sequence" + end + + if pk && sequence + quoted_sequence = quote_column_name(sequence) + + select_value <<-end_sql, 'Reset sequence' + SELECT setval('#{quoted_sequence}', (SELECT COALESCE(MAX(#{quote_column_name pk})+(SELECT increment_by FROM #{quoted_sequence}), (SELECT min_value FROM #{quoted_sequence})) FROM #{quote_table_name(table)}), false) + end_sql end end -- cgit v1.2.3 From d1575ae1b9658c91145d6a46ec2a144a5a089207 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 12 Apr 2011 00:23:07 +0200 Subject: Change Object#either? to Object#among? -- thanks to @jamesarosen for the suggestion! --- activerecord/lib/active_record/associations/association.rb | 2 +- activerecord/lib/active_record/associations/builder/belongs_to.rb | 2 +- activerecord/lib/active_record/associations/builder/has_many.rb | 2 +- activerecord/lib/active_record/associations/builder/has_one.rb | 2 +- activerecord/lib/active_record/associations/has_one_association.rb | 2 +- .../lib/active_record/attribute_methods/time_zone_conversion.rb | 2 +- activerecord/lib/active_record/railties/databases.rake | 2 +- activerecord/lib/active_record/reflection.rb | 2 +- .../active_record/session_migration/session_migration_generator.rb | 2 +- activerecord/test/cases/attribute_methods_test.rb | 2 +- activerecord/test/cases/defaults_test.rb | 2 +- 11 files changed, 11 insertions(+), 11 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index b250783d9e..66f6477ec5 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -164,7 +164,7 @@ module ActiveRecord def creation_attributes attributes = {} - if reflection.macro.either?(:has_one, :has_many) && !options[:through] + if reflection.macro.among?(:has_one, :has_many) && !options[:through] attributes[reflection.foreign_key] = owner[reflection.active_record_primary_key] if reflection.options[:as] diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 9ea37da78f..763b12f708 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -67,7 +67,7 @@ module ActiveRecord::Associations::Builder def configure_dependency if options[:dependent] - unless options[:dependent].either?(:destroy, :delete) + unless options[:dependent].among?(:destroy, :delete) raise ArgumentError, "The :dependent option expects either :destroy or :delete (#{options[:dependent].inspect})" end diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb index 3b489d9647..67ce339380 100644 --- a/activerecord/lib/active_record/associations/builder/has_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_many.rb @@ -16,7 +16,7 @@ module ActiveRecord::Associations::Builder def configure_dependency if options[:dependent] - unless options[:dependent].either?(:destroy, :delete_all, :nullify, :restrict) + unless options[:dependent].among?(:destroy, :delete_all, :nullify, :restrict) raise ArgumentError, "The :dependent option expects either :destroy, :delete_all, " \ ":nullify or :restrict (#{options[:dependent].inspect})" end diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb index 7efe2ec74d..18d7117bb7 100644 --- a/activerecord/lib/active_record/associations/builder/has_one.rb +++ b/activerecord/lib/active_record/associations/builder/has_one.rb @@ -29,7 +29,7 @@ module ActiveRecord::Associations::Builder def configure_dependency if options[:dependent] - unless options[:dependent].either?(:destroy, :delete, :nullify, :restrict) + unless options[:dependent].among?(:destroy, :delete, :nullify, :restrict) raise ArgumentError, "The :dependent option expects either :destroy, :delete, " \ ":nullify or :restrict (#{options[:dependent].inspect})" end diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 82f5696ad8..aaa8b97389 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -52,7 +52,7 @@ module ActiveRecord end def remove_target!(method) - if method.either?(:delete, :destroy) + if method.among?(:delete, :destroy) target.send(method) else nullify_owner_attributes(target) diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index 032a3135f0..67b0e77a25 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -59,7 +59,7 @@ module ActiveRecord private def create_time_zone_conversion_attribute?(name, column) - time_zone_aware_attributes && !self.skip_time_zone_conversion_for_attributes.include?(name.to_sym) && column.type.either?(:datetime, :timestamp) + time_zone_aware_attributes && !self.skip_time_zone_conversion_for_attributes.include?(name.to_sym) && column.type.among?(:datetime, :timestamp) end end end diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index a1b0270d1b..944a29a3e6 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -137,7 +137,7 @@ db_namespace = namespace :db do end def local_database?(config, &block) - if config['host'].either?("127.0.0.1", "localhost") || config['host'].blank? + if config['host'].among?("127.0.0.1", "localhost") || config['host'].blank? yield else $stderr.puts "This task only modifies local databases. #{config['database']} is on a remote host." diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 82561e86cf..eac6a5dcad 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -164,7 +164,7 @@ module ActiveRecord def initialize(macro, name, options, active_record) super - @collection = macro.either?(:has_many, :has_and_belongs_to_many) + @collection = macro.among?(:has_many, :has_and_belongs_to_many) end # Returns a new, unsaved instance of the associated class. +options+ will diff --git a/activerecord/lib/rails/generators/active_record/session_migration/session_migration_generator.rb b/activerecord/lib/rails/generators/active_record/session_migration/session_migration_generator.rb index e803ee5764..d80c8ba996 100644 --- a/activerecord/lib/rails/generators/active_record/session_migration/session_migration_generator.rb +++ b/activerecord/lib/rails/generators/active_record/session_migration/session_migration_generator.rb @@ -14,7 +14,7 @@ module ActiveRecord def session_table_name current_table_name = ActiveRecord::SessionStore::Session.table_name - if current_table_name.either?("sessions", "session") + if current_table_name.among?("sessions", "session") current_table_name = (ActiveRecord::Base.pluralize_table_names ? 'session'.pluralize : 'session') end current_table_name diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index aaf2435cd9..a3c5c14758 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -639,7 +639,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase end def time_related_columns_on_topic - Topic.columns.select { |c| c.type.either?(:time, :date, :datetime, :timestamp) } + Topic.columns.select { |c| c.type.among?(:time, :date, :datetime, :timestamp) } end def serialized_columns_on_topic diff --git a/activerecord/test/cases/defaults_test.rb b/activerecord/test/cases/defaults_test.rb index 57709f0c71..42b14bd25c 100644 --- a/activerecord/test/cases/defaults_test.rb +++ b/activerecord/test/cases/defaults_test.rb @@ -95,7 +95,7 @@ if current_adapter?(:MysqlAdapter) or current_adapter?(:Mysql2Adapter) assert_equal 0, klass.columns_hash['zero'].default assert !klass.columns_hash['zero'].null # 0 in MySQL 4, nil in 5. - assert klass.columns_hash['omit'].default.either?(0, nil) + assert klass.columns_hash['omit'].default.among?(0, nil) assert !klass.columns_hash['omit'].null assert_raise(ActiveRecord::StatementInvalid) { klass.create! } -- cgit v1.2.3 From b53ffb35e085362bb2690577ae123a1f4872c12a Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 11 Apr 2011 15:49:37 -0700 Subject: stop using deprecated methods in arel --- activerecord/lib/active_record/relation/finder_methods.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 7fe6fe0ed0..673e47942b 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -352,8 +352,8 @@ module ActiveRecord if result.size == expected_size result else - conditions = arel.wheres.map { |x| x.value }.join(', ') - conditions = " [WHERE #{conditions}]" if conditions.present? + conditions = arel.where_sql + conditions = " [#{conditions}]" if conditions error = "Couldn't find all #{@klass.name.pluralize} with IDs " error << "(#{ids.join(", ")})#{conditions} (found #{result.size} results, but was looking for #{expected_size})" -- cgit v1.2.3 From fc9a04b6a69d6821a6f1601e3e0dd518300fa138 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Mon, 28 Mar 2011 22:36:29 +0100 Subject: Removing the scope-caching which happens on association proxies, because the query is already cached by the query cacher. For formalised proof see http://www.youtube.com/watch?v=wDefXLb-FDs --- .../associations/collection_association.rb | 12 ----------- .../active_record/associations/collection_proxy.rb | 2 -- activerecord/test/cases/named_scope_test.rb | 25 +++++++++++++--------- 3 files changed, 15 insertions(+), 24 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 9f4fc44cc6..33a184d48d 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -21,14 +21,7 @@ module ActiveRecord attr_reader :proxy def initialize(owner, reflection) - # When scopes are created via method_missing on the proxy, they are stored so that - # any records fetched from the database are kept around for future use. - @scopes_cache = Hash.new do |hash, method| - hash[method] = { } - end - super - @proxy = CollectionProxy.new(self) end @@ -74,7 +67,6 @@ module ActiveRecord def reset @loaded = false @target = [] - @scopes_cache.clear end def select(select = nil) @@ -327,10 +319,6 @@ module ActiveRecord end end - def cached_scope(method, args) - @scopes_cache[method][args] ||= scoped.readonly(nil).send(method, *args) - end - def load_target if find_target? targets = [] diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index cf77d770c9..388173c1fb 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -82,8 +82,6 @@ module ActiveRecord end end - elsif @association.klass.scopes[method] - @association.cached_scope(method, args) else scoped.readonly(nil).send(method, *args, &block) end diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index 9b20ea08de..2b3e1900e1 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -440,26 +440,31 @@ class NamedScopeTest < ActiveRecord::TestCase end end + # Note: these next two are kinda odd because they are essentially just testing that the + # query cache works as it should, but they are here for legacy reasons as they was previously + # a separate cache on association proxies, and these show that that is not necessary. def test_scopes_are_cached_on_associations post = posts(:welcome) - assert_equal post.comments.containing_the_letter_e.object_id, post.comments.containing_the_letter_e.object_id - - post.comments.containing_the_letter_e.all # force load - assert_no_queries { post.comments.containing_the_letter_e.all } + Post.cache do + assert_queries(1) { post.comments.containing_the_letter_e.all } + assert_no_queries { post.comments.containing_the_letter_e.all } + end end def test_scopes_with_arguments_are_cached_on_associations post = posts(:welcome) - one = post.comments.limit_by(1).all - assert_equal 1, one.size + Post.cache do + one = assert_queries(1) { post.comments.limit_by(1).all } + assert_equal 1, one.size - two = post.comments.limit_by(2).all - assert_equal 2, two.size + two = assert_queries(1) { post.comments.limit_by(2).all } + assert_equal 2, two.size - assert_no_queries { post.comments.limit_by(1).all } - assert_no_queries { post.comments.limit_by(2).all } + assert_no_queries { post.comments.limit_by(1).all } + assert_no_queries { post.comments.limit_by(2).all } + end end def test_scopes_are_reset_on_association_reload -- cgit v1.2.3 From 5740d4ec0c16d68b82f440e74fd8b74ae3fe48e6 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Mon, 4 Apr 2011 00:07:45 +0100 Subject: Deprecated support for passing hashes and relations to default_scope, in favour of defining a 'default_scope' class method in the model. See the CHANGELOG for more details. --- activerecord/CHANGELOG | 21 ++ activerecord/lib/active_record/base.rb | 144 ++++++++----- activerecord/lib/active_record/named_scope.rb | 2 +- activerecord/lib/active_record/relation.rb | 7 +- .../associations/has_many_associations_test.rb | 14 +- .../associations/has_one_associations_test.rb | 14 +- activerecord/test/cases/base_test.rb | 12 +- activerecord/test/cases/method_scoping_test.rb | 12 +- activerecord/test/cases/relation_scoping_test.rb | 231 +++++++++++++++------ activerecord/test/models/bulb.rb | 13 +- activerecord/test/models/car.rb | 9 +- activerecord/test/models/categorization.rb | 4 +- activerecord/test/models/developer.rb | 69 +++++- activerecord/test/models/pirate.rb | 2 +- activerecord/test/models/post.rb | 15 +- activerecord/test/models/reference.rb | 7 +- activerecord/test/models/without_table.rb | 6 +- 17 files changed, 410 insertions(+), 172 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 88c2562619..64be577624 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,26 @@ *Rails 3.1.0 (unreleased)* +* Deprecated support for passing hashes and relations to 'default_scope'. Please create a class + method for your scope instead. For example, change this: + + class Post < ActiveRecord::Base + default_scope where(:published => true) + end + + To this: + + class Post < ActiveRecord::Base + def self.default_scope + where(:published => true) + end + end + + Rationale: It will make the implementation simpler because we can simply use inheritance to + handle inheritance scenarios, rather than trying to make up our own rules about what should + happen when you call default_scope multiple times or in subclasses. + + [Jon Leighton] + * PostgreSQL adapter only supports PostgreSQL version 8.2 and higher. * ConnectionManagement middleware is changed to clean up the connection pool diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index fe81c7dc2f..e0fb94c96e 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -425,8 +425,8 @@ module ActiveRecord #:nodoc: self.store_full_sti_class = true # Stores the default scope for the class - class_attribute :default_scoping, :instance_writer => false - self.default_scoping = [] + class_attribute :default_scopes, :instance_writer => false + self.default_scopes = [] # Returns a hash of all the attributes that have been specified for serialization as # keys and their class restriction as values. @@ -870,7 +870,9 @@ module ActiveRecord #:nodoc: # Returns a scope for this class without taking into account the default_scope. # # class Post < ActiveRecord::Base - # default_scope :published => true + # def self.default_scope + # where :published => true + # end # end # # Post.all # Fires "SELECT * FROM posts WHERE published = true" @@ -892,13 +894,8 @@ module ActiveRecord #:nodoc: block_given? ? relation.scoping { yield } : relation end - def scoped_methods #:nodoc: - key = :"#{self}_scoped_methods" - Thread.current[key] = Thread.current[key].presence || self.default_scoping.dup - end - def before_remove_const #:nodoc: - reset_scoped_methods + self.current_scope = nil end # Specifies how the record is loaded by +Marshal+. @@ -1020,7 +1017,7 @@ module ActiveRecord #:nodoc: super unless all_attributes_exists?(attribute_names) if match.finder? options = arguments.extract_options! - relation = options.any? ? construct_finder_arel(options, current_scoped_methods) : scoped + relation = options.any? ? scoped(options) : scoped relation.send :find_by_attributes, match, attribute_names, *arguments elsif match.instantiator? scoped.send :find_or_instantiator_by_attributes, match, attribute_names, *arguments, &block @@ -1109,43 +1106,48 @@ module ActiveRecord #:nodoc: # end # # *Note*: the +:find+ scope also has effect on update and deletion methods, like +update_all+ and +delete_all+. - def with_scope(method_scoping = {}, action = :merge, &block) - method_scoping = method_scoping.method_scoping if method_scoping.respond_to?(:method_scoping) + def with_scope(scope = {}, action = :merge, &block) + # If another Active Record class has been passed in, get its current scope + scope = scope.current_scope if !scope.is_a?(Relation) && scope.respond_to?(:current_scope) + + # Cannot use self.current_scope, because that could trigger build_default_scope, which + # could in turn result in with_scope being called and hence an infinite loop. + previous_scope = Thread.current[:"#{self}_current_scope"] - if method_scoping.is_a?(Hash) + if scope.is_a?(Hash) # Dup first and second level of hash (method and params). - method_scoping = method_scoping.dup - method_scoping.each do |method, params| - method_scoping[method] = params.dup unless params == true + scope = scope.dup + scope.each do |method, params| + scope[method] = params.dup unless params == true end - method_scoping.assert_valid_keys([ :find, :create ]) - relation = construct_finder_arel(method_scoping[:find] || {}) + scope.assert_valid_keys([ :find, :create ]) + relation = construct_finder_arel(scope[:find] || {}) - if current_scoped_methods && current_scoped_methods.create_with_value && method_scoping[:create] + if previous_scope && previous_scope.create_with_value && scope[:create] scope_for_create = if action == :merge - current_scoped_methods.create_with_value.merge(method_scoping[:create]) + previous_scope.create_with_value.merge(scope[:create]) else - method_scoping[:create] + scope[:create] end relation = relation.create_with(scope_for_create) else - scope_for_create = method_scoping[:create] - scope_for_create ||= current_scoped_methods.create_with_value if current_scoped_methods + scope_for_create = scope[:create] + scope_for_create ||= previous_scope.create_with_value if previous_scope relation = relation.create_with(scope_for_create) if scope_for_create end - method_scoping = relation + scope = relation end - method_scoping = current_scoped_methods.merge(method_scoping) if current_scoped_methods && action == :merge + scope = previous_scope.merge(scope) if previous_scope && action == :merge - self.scoped_methods << method_scoping + self.current_scope = scope begin yield ensure - self.scoped_methods.pop + self.current_scope = previous_scope end end @@ -1168,39 +1170,82 @@ MSG with_scope(method_scoping, :overwrite, &block) end - # Sets the default options for the model. The format of the - # options argument is the same as in find. + def current_scope #:nodoc: + Thread.current[:"#{self}_current_scope"] ||= build_default_scope + end + + def current_scope=(scope) #:nodoc: + Thread.current[:"#{self}_current_scope"] = scope + end + + # Implement this method in your model to set a default scope for all operations on + # the model. # # class Person < ActiveRecord::Base - # default_scope order('last_name, first_name') + # def self.default_scope + # order('last_name, first_name') + # end # end # - # default_scope is also applied while creating/building a record. It is not + # Person.all # => SELECT * FROM people ORDER BY last_name, first_name + # + # The default_scope is also applied while creating/building a record. It is not # applied while updating a record. # # class Article < ActiveRecord::Base - # default_scope where(:published => true) + # def self.default_scope + # where(:published => true) + # end # end # # Article.new.published # => true # Article.create.published # => true - def default_scope(options = {}) - reset_scoped_methods - default_scoping = self.default_scoping.dup - self.default_scoping = default_scoping << construct_finder_arel(options, default_scoping.pop) - end + # + # === Deprecation warning + # + # There is an alternative syntax as follows: + # + # class Person < ActiveRecord::Base + # default_scope order('last_name, first_name') + # end + # + # This is now deprecated and will be removed in Rails 3.2. + def default_scope(scope = {}) + ActiveSupport::Deprecation.warn <<-WARN +Passing a hash or scope to default_scope is deprecated and will be removed in Rails 3.2. You should create a class method for your scope instead. For example, change this: - def current_scoped_methods #:nodoc: - method = scoped_methods.last - if method.respond_to?(:call) - relation.scoping { method.call } - else - method - end +class Post < ActiveRecord::Base + default_scope where(:published => true) +end + +To this: + +class Post < ActiveRecord::Base + def self.default_scope + where(:published => true) + end +end +WARN + + # Reset the current scope as it may contain scopes based on a now-invalid default scope + self.current_scope = nil + self.default_scopes = default_scopes.dup << scope end - def reset_scoped_methods #:nodoc: - Thread.current[:"#{self}_scoped_methods"] = nil + def build_default_scope #:nodoc: + if method(:default_scope).owner != Base.singleton_class + # Exclusively scope to just the relation, to avoid infinite recursion where the + # default scope tries to use the default scope tries to use the default scope... + relation.scoping { default_scope } + elsif default_scopes.any? + default_scopes.inject(relation) do |default_scope, scope| + if scope.is_a?(Hash) + default_scope.apply_finder_options(scope) + else + default_scope.merge(scope) + end + end + end end # Returns the class type of the record using the current module as a prefix. So descendants of @@ -1916,11 +1961,8 @@ MSG end def populate_with_current_scope_attributes - if scope = self.class.send(:current_scoped_methods) - create_with = scope.scope_for_create - create_with.each { |att,value| - respond_to?("#{att}=") && send("#{att}=", value) - } + self.class.scoped.scope_for_create.each do |att,value| + respond_to?("#{att}=") && send("#{att}=", value) end end diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index d5fff65303..a398476dd6 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -35,7 +35,7 @@ module ActiveRecord if options scoped.apply_finder_options(options) else - current_scoped_methods ? relation.merge(current_scoped_methods) : relation.clone + (current_scope || relation).clone end end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 75269c85d9..645ad0fce1 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -154,12 +154,7 @@ module ActiveRecord # Please check unscoped if you want to remove all previous scopes (including # the default_scope) during the execution of a block. def scoping - @klass.scoped_methods << self - begin - yield - ensure - @klass.scoped_methods.pop - end + @klass.send(:with_scope, self, :overwrite) { yield } end # Updates all records with details given if they match a set of conditions supplied, limits and order can diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 16d4877fe8..007f11b535 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -70,16 +70,16 @@ class HasManyAssociationsTest < ActiveRecord::TestCase # would be convenient), because this would cause that scope to be applied to any callbacks etc. def test_build_and_create_should_not_happen_within_scope car = cars(:honda) - original_scoped_methods = Bulb.scoped_methods + scoped_count = car.foo_bulbs.scoped.where_values.count - bulb = car.bulbs.build - assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize + bulb = car.foo_bulbs.build + assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count - bulb = car.bulbs.create - assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize + bulb = car.foo_bulbs.create + assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count - bulb = car.bulbs.create! - assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize + bulb = car.foo_bulbs.create! + assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count end def test_no_sql_should_be_fired_if_association_already_loaded diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index c1dad5e246..f3c96ccbe6 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -165,16 +165,16 @@ class HasOneAssociationsTest < ActiveRecord::TestCase def test_build_and_create_should_not_happen_within_scope pirate = pirates(:blackbeard) - original_scoped_methods = Bulb.scoped_methods.dup + scoped_count = pirate.association(:foo_bulb).scoped.where_values.count - bulb = pirate.build_bulb - assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize + bulb = pirate.build_foo_bulb + assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count - bulb = pirate.create_bulb - assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize + bulb = pirate.create_foo_bulb + assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count - bulb = pirate.create_bulb! - assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize + bulb = pirate.create_foo_bulb! + assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count end def test_create_association diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index b3facf50b8..73887f9b49 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1630,14 +1630,18 @@ class BasicsTest < ActiveRecord::TestCase Object.const_set :UnloadablePost, Class.new(ActiveRecord::Base) UnloadablePost.table_name = 'posts' UnloadablePost.class_eval do - default_scope order('posts.comments_count ASC') + class << self + def default_scope + order('posts.comments_count ASC') + end + end end - UnloadablePost.scoped_methods # make Thread.current[:UnloadablePost_scoped_methods] not nil + UnloadablePost.scoped # make Thread.current[:UnloadablePost_scoped_methods] not nil UnloadablePost.unloadable - assert_not_nil Thread.current[:UnloadablePost_scoped_methods] + assert_not_nil Thread.current[:UnloadablePost_current_scope] ActiveSupport::Dependencies.remove_unloadable_constants! - assert_nil Thread.current[:UnloadablePost_scoped_methods] + assert_nil Thread.current[:UnloadablePost_current_scope] ensure Object.class_eval{ remove_const :UnloadablePost } if defined?(UnloadablePost) end diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index 7e8383da9e..7f0f007a70 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -249,22 +249,21 @@ class MethodScopingTest < ActiveRecord::TestCase end def test_scoped_with_duck_typing - scoping = Struct.new(:method_scoping).new(:find => { :conditions => ["name = ?", 'David'] }) + scoping = Struct.new(:current_scope).new(:find => { :conditions => ["name = ?", 'David'] }) Developer.send(:with_scope, scoping) do assert_equal %w(David), Developer.find(:all).map { |d| d.name } end end def test_ensure_that_method_scoping_is_correctly_restored - scoped_methods = Developer.instance_eval('current_scoped_methods') - begin Developer.send(:with_scope, :find => { :conditions => "name = 'Jamis'" }) do raise "an exception" end rescue end - assert_equal scoped_methods, Developer.instance_eval('current_scoped_methods') + + assert !Developer.scoped.where_values.include?("name = 'Jamis'") end end @@ -509,14 +508,15 @@ class NestedScopingTest < ActiveRecord::TestCase def test_ensure_that_method_scoping_is_correctly_restored Developer.send(:with_scope, :find => { :conditions => "name = 'David'" }) do - scoped_methods = Developer.instance_eval('current_scoped_methods') begin Developer.send(:with_scope, :find => { :conditions => "name = 'Maiha'" }) do raise "an exception" end rescue end - assert_equal scoped_methods, Developer.instance_eval('current_scoped_methods') + + assert Developer.scoped.where_values.include?("name = 'David'") + assert !Developer.scoped.where_values.include?("name = 'Maiha'") end end diff --git a/activerecord/test/cases/relation_scoping_test.rb b/activerecord/test/cases/relation_scoping_test.rb index 30a783d5a2..a91072b94b 100644 --- a/activerecord/test/cases/relation_scoping_test.rb +++ b/activerecord/test/cases/relation_scoping_test.rb @@ -132,8 +132,6 @@ class RelationScopingTest < ActiveRecord::TestCase end def test_ensure_that_method_scoping_is_correctly_restored - scoped_methods = Developer.send(:current_scoped_methods) - begin Developer.where("name = 'Jamis'").scoping do raise "an exception" @@ -141,7 +139,7 @@ class RelationScopingTest < ActiveRecord::TestCase rescue end - assert_equal scoped_methods, Developer.send(:current_scoped_methods) + assert !Developer.scoped.where_values.include?("name = 'Jamis'") end end @@ -310,72 +308,173 @@ class DefaultScopingTest < ActiveRecord::TestCase assert_equal expected, received end - def test_default_scope_with_lambda - expected = Post.find_all_by_author_id(2) - PostForAuthor.selected_author = 2 - received = PostForAuthor.all - assert_equal expected, received - expected = Post.find_all_by_author_id(1) - PostForAuthor.selected_author = 1 - received = PostForAuthor.all + def test_default_scope_is_unscoped_on_find + assert_equal 1, DeveloperCalledDavid.count + assert_equal 11, DeveloperCalledDavid.unscoped.count + end + + def test_default_scope_is_unscoped_on_create + assert_nil DeveloperCalledJamis.unscoped.create!.name + end + + def test_default_scope_with_conditions_string + assert_equal Developer.find_all_by_name('David').map(&:id).sort, DeveloperCalledDavid.find(:all).map(&:id).sort + assert_equal nil, DeveloperCalledDavid.create!.name + end + + def test_default_scope_with_conditions_hash + assert_equal Developer.find_all_by_name('Jamis').map(&:id).sort, DeveloperCalledJamis.find(:all).map(&:id).sort + assert_equal 'Jamis', DeveloperCalledJamis.create!.name + end + + def test_default_scoping_with_threads + 2.times do + Thread.new { assert_equal ['salary DESC'], DeveloperOrderedBySalary.scoped.order_values }.join + end + end + + def test_default_scope_with_inheritance + wheres = InheritedPoorDeveloperCalledJamis.scoped.where_values_hash + assert_equal "Jamis", wheres[:name] + assert_equal 50000, wheres[:salary] + end + + def test_method_scope + expected = Developer.find(:all, :order => 'salary DESC, name DESC').collect { |dev| dev.salary } + received = DeveloperOrderedBySalary.all_ordered_by_name.collect { |dev| dev.salary } assert_equal expected, received end - def test_default_scope_with_thing_that_responds_to_call - klass = Class.new(ActiveRecord::Base) do - self.table_name = 'posts' + def test_nested_scope + expected = Developer.find(:all, :order => 'salary DESC, name DESC').collect { |dev| dev.salary } + received = DeveloperOrderedBySalary.send(:with_scope, :find => { :order => 'name DESC'}) do + DeveloperOrderedBySalary.find(:all).collect { |dev| dev.salary } end + assert_equal expected, received + end - klass.class_eval do - default_scope Class.new(Struct.new(:klass)) { - def call - klass.where(:author_id => 2) - end - }.new(self) + def test_scope_overwrites_default + expected = Developer.find(:all, :order => 'salary DESC, name DESC').collect { |dev| dev.name } + received = DeveloperOrderedBySalary.by_name.find(:all).collect { |dev| dev.name } + assert_equal expected, received + end + + def test_reorder_overrides_default_scope_order + expected = Developer.order('name DESC').collect { |dev| dev.name } + received = DeveloperOrderedBySalary.reorder('name DESC').collect { |dev| dev.name } + assert_equal expected, received + end + + def test_nested_exclusive_scope + expected = Developer.find(:all, :limit => 100).collect { |dev| dev.salary } + received = DeveloperOrderedBySalary.send(:with_exclusive_scope, :find => { :limit => 100 }) do + DeveloperOrderedBySalary.find(:all).collect { |dev| dev.salary } end + assert_equal expected, received + end + + def test_order_in_default_scope_should_prevail + expected = Developer.find(:all, :order => 'salary desc').collect { |dev| dev.salary } + received = DeveloperOrderedBySalary.find(:all, :order => 'salary').collect { |dev| dev.salary } + assert_equal expected, received + end - records = klass.all - assert_equal 3, records.length - assert_equal 2, records.first.author_id + def test_create_attribute_overwrites_default_scoping + assert_equal 'David', PoorDeveloperCalledJamis.create!(:name => 'David').name + assert_equal 200000, PoorDeveloperCalledJamis.create!(:name => 'David', :salary => 200000).salary + end + + def test_create_attribute_overwrites_default_values + assert_equal nil, PoorDeveloperCalledJamis.create!(:salary => nil).salary + assert_equal 50000, PoorDeveloperCalledJamis.create!(:name => 'David').salary + end + + def test_default_scope_attribute + jamis = PoorDeveloperCalledJamis.new(:name => 'David') + assert_equal 50000, jamis.salary + end + + def test_where_attribute + aaron = PoorDeveloperCalledJamis.where(:salary => 20).new(:name => 'Aaron') + assert_equal 20, aaron.salary + assert_equal 'Aaron', aaron.name + end + + def test_where_attribute_merge + aaron = PoorDeveloperCalledJamis.where(:name => 'foo').new(:name => 'Aaron') + assert_equal 'Aaron', aaron.name + end + + def test_scope_composed_by_limit_and_then_offset_is_equal_to_scope_composed_by_offset_and_then_limit + posts_limit_offset = Post.limit(3).offset(2) + posts_offset_limit = Post.offset(2).limit(3) + assert_equal posts_limit_offset, posts_offset_limit + end + + def test_create_with_merge + aaron = PoorDeveloperCalledJamis.create_with(:name => 'foo', :salary => 20).merge( + PoorDeveloperCalledJamis.create_with(:name => 'Aaron')).new + assert_equal 20, aaron.salary + assert_equal 'Aaron', aaron.name + + aaron = PoorDeveloperCalledJamis.create_with(:name => 'foo', :salary => 20). + create_with(:name => 'Aaron').new + assert_equal 20, aaron.salary + assert_equal 'Aaron', aaron.name + end + + def test_create_with_reset + jamis = PoorDeveloperCalledJamis.create_with(:name => 'Aaron').create_with(nil).new + assert_equal 'Jamis', jamis.name + end +end + +class DeprecatedDefaultScopingTest < ActiveRecord::TestCase + fixtures :developers, :posts + + def test_default_scope + expected = Developer.find(:all, :order => 'salary DESC').collect { |dev| dev.salary } + received = DeprecatedDeveloperOrderedBySalary.find(:all).collect { |dev| dev.salary } + assert_equal expected, received end def test_default_scope_is_unscoped_on_find - assert_equal 1, DeveloperCalledDavid.count - assert_equal 11, DeveloperCalledDavid.unscoped.count + assert_equal 1, DeprecatedDeveloperCalledDavid.count + assert_equal 11, DeprecatedDeveloperCalledDavid.unscoped.count end def test_default_scope_is_unscoped_on_create - assert_nil DeveloperCalledJamis.unscoped.create!.name + assert_nil DeprecatedDeveloperCalledJamis.unscoped.create!.name end def test_default_scope_with_conditions_string - assert_equal Developer.find_all_by_name('David').map(&:id).sort, DeveloperCalledDavid.find(:all).map(&:id).sort - assert_equal nil, DeveloperCalledDavid.create!.name + assert_equal Developer.find_all_by_name('David').map(&:id).sort, DeprecatedDeveloperCalledDavid.find(:all).map(&:id).sort + assert_equal nil, DeprecatedDeveloperCalledDavid.create!.name end def test_default_scope_with_conditions_hash - assert_equal Developer.find_all_by_name('Jamis').map(&:id).sort, DeveloperCalledJamis.find(:all).map(&:id).sort - assert_equal 'Jamis', DeveloperCalledJamis.create!.name + assert_equal Developer.find_all_by_name('Jamis').map(&:id).sort, DeprecatedDeveloperCalledJamis.find(:all).map(&:id).sort + assert_equal 'Jamis', DeprecatedDeveloperCalledJamis.create!.name end def test_default_scoping_with_threads 2.times do - Thread.new { assert_equal ['salary DESC'], DeveloperOrderedBySalary.scoped.order_values }.join + Thread.new { assert_equal ['salary DESC'], DeprecatedDeveloperOrderedBySalary.scoped.order_values }.join end end def test_default_scoping_with_inheritance # Inherit a class having a default scope and define a new default scope - klass = Class.new(DeveloperOrderedBySalary) - klass.send :default_scope, :limit => 1 + klass = Class.new(DeprecatedDeveloperOrderedBySalary) + ActiveSupport::Deprecation.silence { klass.send :default_scope, :limit => 1 } # Scopes added on children should append to parent scope assert_equal 1, klass.scoped.limit_value assert_equal ['salary DESC'], klass.scoped.order_values # Parent should still have the original scope - assert_nil DeveloperOrderedBySalary.scoped.limit_value - assert_equal ['salary DESC'], DeveloperOrderedBySalary.scoped.order_values + assert_nil DeprecatedDeveloperOrderedBySalary.scoped.limit_value + assert_equal ['salary DESC'], DeprecatedDeveloperOrderedBySalary.scoped.order_values end def test_default_scope_called_twice_merges_conditions @@ -385,8 +484,10 @@ class DefaultScopingTest < ActiveRecord::TestCase Developer.create!(:name => "Brian", :salary => 100000) klass = Class.new(Developer) - klass.__send__ :default_scope, :conditions => { :name => "David" } - klass.__send__ :default_scope, :conditions => { :salary => 100000 } + ActiveSupport::Deprecation.silence do + klass.__send__ :default_scope, :conditions => { :name => "David" } + klass.__send__ :default_scope, :conditions => { :salary => 100000 } + end assert_equal 1, klass.count assert_equal "David", klass.first.name assert_equal 100000, klass.first.salary @@ -399,9 +500,11 @@ class DefaultScopingTest < ActiveRecord::TestCase Developer.create!(:name => "Brian", :salary => 100000) klass = Class.new(Developer) - klass.class_eval do - default_scope where("name = 'David'") - default_scope where("salary = 100000") + ActiveSupport::Deprecation.silence do + klass.class_eval do + default_scope where("name = 'David'") + default_scope where("salary = 100000") + end end assert_equal 1, klass.count @@ -411,96 +514,90 @@ class DefaultScopingTest < ActiveRecord::TestCase def test_method_scope expected = Developer.find(:all, :order => 'salary DESC, name DESC').collect { |dev| dev.salary } - received = DeveloperOrderedBySalary.all_ordered_by_name.collect { |dev| dev.salary } + received = DeprecatedDeveloperOrderedBySalary.all_ordered_by_name.collect { |dev| dev.salary } assert_equal expected, received end def test_nested_scope expected = Developer.find(:all, :order => 'salary DESC, name DESC').collect { |dev| dev.salary } - received = DeveloperOrderedBySalary.send(:with_scope, :find => { :order => 'name DESC'}) do - DeveloperOrderedBySalary.find(:all).collect { |dev| dev.salary } + received = DeprecatedDeveloperOrderedBySalary.send(:with_scope, :find => { :order => 'name DESC'}) do + DeprecatedDeveloperOrderedBySalary.find(:all).collect { |dev| dev.salary } end assert_equal expected, received end def test_scope_overwrites_default expected = Developer.find(:all, :order => 'salary DESC, name DESC').collect { |dev| dev.name } - received = DeveloperOrderedBySalary.by_name.find(:all).collect { |dev| dev.name } + received = DeprecatedDeveloperOrderedBySalary.by_name.find(:all).collect { |dev| dev.name } assert_equal expected, received end def test_reorder_overrides_default_scope_order expected = Developer.order('name DESC').collect { |dev| dev.name } - received = DeveloperOrderedBySalary.reorder('name DESC').collect { |dev| dev.name } + received = DeprecatedDeveloperOrderedBySalary.reorder('name DESC').collect { |dev| dev.name } assert_equal expected, received end def test_nested_exclusive_scope expected = Developer.find(:all, :limit => 100).collect { |dev| dev.salary } - received = DeveloperOrderedBySalary.send(:with_exclusive_scope, :find => { :limit => 100 }) do - DeveloperOrderedBySalary.find(:all).collect { |dev| dev.salary } + received = DeprecatedDeveloperOrderedBySalary.send(:with_exclusive_scope, :find => { :limit => 100 }) do + DeprecatedDeveloperOrderedBySalary.find(:all).collect { |dev| dev.salary } end assert_equal expected, received end def test_order_in_default_scope_should_prevail expected = Developer.find(:all, :order => 'salary desc').collect { |dev| dev.salary } - received = DeveloperOrderedBySalary.find(:all, :order => 'salary').collect { |dev| dev.salary } + received = DeprecatedDeveloperOrderedBySalary.find(:all, :order => 'salary').collect { |dev| dev.salary } assert_equal expected, received end def test_default_scope_using_relation - posts = PostWithComment.scoped - assert_equal 2, posts.count + posts = DeprecatedPostWithComment.scoped + assert_equal 2, posts.to_a.length assert_equal posts(:thinking), posts.first end def test_create_attribute_overwrites_default_scoping - assert_equal 'David', PoorDeveloperCalledJamis.create!(:name => 'David').name - assert_equal 200000, PoorDeveloperCalledJamis.create!(:name => 'David', :salary => 200000).salary + assert_equal 'David', DeprecatedPoorDeveloperCalledJamis.create!(:name => 'David').name + assert_equal 200000, DeprecatedPoorDeveloperCalledJamis.create!(:name => 'David', :salary => 200000).salary end def test_create_attribute_overwrites_default_values - assert_equal nil, PoorDeveloperCalledJamis.create!(:salary => nil).salary - assert_equal 50000, PoorDeveloperCalledJamis.create!(:name => 'David').salary + assert_equal nil, DeprecatedPoorDeveloperCalledJamis.create!(:salary => nil).salary + assert_equal 50000, DeprecatedPoorDeveloperCalledJamis.create!(:name => 'David').salary end def test_default_scope_attribute - jamis = PoorDeveloperCalledJamis.new(:name => 'David') + jamis = DeprecatedPoorDeveloperCalledJamis.new(:name => 'David') assert_equal 50000, jamis.salary end def test_where_attribute - aaron = PoorDeveloperCalledJamis.where(:salary => 20).new(:name => 'Aaron') + aaron = DeprecatedPoorDeveloperCalledJamis.where(:salary => 20).new(:name => 'Aaron') assert_equal 20, aaron.salary assert_equal 'Aaron', aaron.name end def test_where_attribute_merge - aaron = PoorDeveloperCalledJamis.where(:name => 'foo').new(:name => 'Aaron') + aaron = DeprecatedPoorDeveloperCalledJamis.where(:name => 'foo').new(:name => 'Aaron') assert_equal 'Aaron', aaron.name end - def test_scope_composed_by_limit_and_then_offset_is_equal_to_scope_composed_by_offset_and_then_limit - posts_limit_offset = Post.limit(3).offset(2) - posts_offset_limit = Post.offset(2).limit(3) - assert_equal posts_limit_offset, posts_offset_limit - end - def test_create_with_merge - aaron = PoorDeveloperCalledJamis.create_with(:name => 'foo', :salary => 20).merge( - PoorDeveloperCalledJamis.create_with(:name => 'Aaron')).new + aaron = DeprecatedPoorDeveloperCalledJamis.create_with(:name => 'foo', :salary => 20).merge( + DeprecatedPoorDeveloperCalledJamis.create_with(:name => 'Aaron')).new assert_equal 20, aaron.salary assert_equal 'Aaron', aaron.name - aaron = PoorDeveloperCalledJamis.create_with(:name => 'foo', :salary => 20). + aaron = DeprecatedPoorDeveloperCalledJamis.create_with(:name => 'foo', :salary => 20). create_with(:name => 'Aaron').new assert_equal 20, aaron.salary assert_equal 'Aaron', aaron.name end def test_create_with_reset - jamis = PoorDeveloperCalledJamis.create_with(:name => 'Aaron').create_with(nil).new + jamis = DeprecatedPoorDeveloperCalledJamis.create_with(:name => 'Aaron').create_with(nil).new assert_equal 'Jamis', jamis.name end end diff --git a/activerecord/test/models/bulb.rb b/activerecord/test/models/bulb.rb index 7178bb0d00..89ee5416bf 100644 --- a/activerecord/test/models/bulb.rb +++ b/activerecord/test/models/bulb.rb @@ -1,14 +1,15 @@ class Bulb < ActiveRecord::Base - - default_scope :conditions => {:name => 'defaulty' } + def self.default_scope + where :name => 'defaulty' + end belongs_to :car - attr_reader :scoped_methods_after_initialize + attr_reader :scope_after_initialize - after_initialize :record_scoped_methods_after_initialize - def record_scoped_methods_after_initialize - @scoped_methods_after_initialize = self.class.scoped_methods.dup + after_initialize :record_scope_after_initialize + def record_scope_after_initialize + @scope_after_initialize = self.class.scoped end end diff --git a/activerecord/test/models/car.rb b/activerecord/test/models/car.rb index e7db3d3423..a978debb58 100644 --- a/activerecord/test/models/car.rb +++ b/activerecord/test/models/car.rb @@ -1,6 +1,7 @@ class Car < ActiveRecord::Base has_many :bulbs + has_many :foo_bulbs, :class_name => "Bulb", :conditions => { :name => 'foo' } has_many :tyres has_many :engines has_many :wheels, :as => :wheelable @@ -14,9 +15,13 @@ class Car < ActiveRecord::Base end class CoolCar < Car - default_scope :order => 'name desc' + def self.default_scope + order 'name desc' + end end class FastCar < Car - default_scope order('name desc') + def self.default_scope + order 'name desc' + end end diff --git a/activerecord/test/models/categorization.rb b/activerecord/test/models/categorization.rb index 09489b8ea4..39441e8610 100644 --- a/activerecord/test/models/categorization.rb +++ b/activerecord/test/models/categorization.rb @@ -13,7 +13,9 @@ end class SpecialCategorization < ActiveRecord::Base self.table_name = 'categorizations' - default_scope where(:special => true) + def self.default_scope + where(:special => true) + end belongs_to :author belongs_to :category diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index 32d060cf09..28b31caf7b 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -86,7 +86,11 @@ end class DeveloperOrderedBySalary < ActiveRecord::Base self.table_name = 'developers' - default_scope :order => 'salary DESC' + + def self.default_scope + order('salary DESC') + end + scope :by_name, order('name DESC') def self.all_ordered_by_name @@ -98,15 +102,72 @@ end class DeveloperCalledDavid < ActiveRecord::Base self.table_name = 'developers' - default_scope :conditions => "name = 'David'" + + def self.default_scope + where "name = 'David'" + end end class DeveloperCalledJamis < ActiveRecord::Base self.table_name = 'developers' - default_scope :conditions => { :name => 'Jamis' } + + def self.default_scope + where :name => 'Jamis' + end +end + +class AbstractDeveloperCalledJamis < ActiveRecord::Base + self.abstract_class = true + + def self.default_scope + where :name => 'Jamis' + end end class PoorDeveloperCalledJamis < ActiveRecord::Base self.table_name = 'developers' - default_scope :conditions => { :name => 'Jamis', :salary => 50000 } + + def self.default_scope + where :name => 'Jamis', :salary => 50000 + end +end + +class InheritedPoorDeveloperCalledJamis < DeveloperCalledJamis + self.table_name = 'developers' + + def self.default_scope + super.where :salary => 50000 + end +end + +ActiveSupport::Deprecation.silence do + class DeprecatedDeveloperOrderedBySalary < ActiveRecord::Base + self.table_name = 'developers' + default_scope :order => 'salary DESC' + + def self.by_name + order('name DESC') + end + + def self.all_ordered_by_name + with_scope(:find => { :order => 'name DESC' }) do + find(:all) + end + end + end + + class DeprecatedDeveloperCalledDavid < ActiveRecord::Base + self.table_name = 'developers' + default_scope :conditions => "name = 'David'" + end + + class DeprecatedDeveloperCalledJamis < ActiveRecord::Base + self.table_name = 'developers' + default_scope :conditions => { :name => 'Jamis' } + end + + class DeprecatedPoorDeveloperCalledJamis < ActiveRecord::Base + self.table_name = 'developers' + default_scope :conditions => { :name => 'Jamis', :salary => 50000 } + end end diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb index 0d3f62bb33..5e0f5323e6 100644 --- a/activerecord/test/models/pirate.rb +++ b/activerecord/test/models/pirate.rb @@ -34,7 +34,7 @@ class Pirate < ActiveRecord::Base :after_remove => proc {|p,b| p.ship_log << "after_removing_proc_bird_#{b.id}"} has_many :birds_with_reject_all_blank, :class_name => "Bird" - has_one :bulb, :foreign_key => :car_id + has_one :foo_bulb, :foreign_key => :car_id, :class_name => "Bulb", :conditions => { :name => 'foo' } accepts_nested_attributes_for :parrots, :birds, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 82894a3d57..632f83e5d4 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -142,20 +142,25 @@ class SubStiPost < StiPost self.table_name = Post.table_name end -class PostWithComment < ActiveRecord::Base - self.table_name = 'posts' - default_scope where("posts.comments_count > 0").order("posts.comments_count ASC") +ActiveSupport::Deprecation.silence do + class DeprecatedPostWithComment < ActiveRecord::Base + self.table_name = 'posts' + default_scope where("posts.comments_count > 0").order("posts.comments_count ASC") + end end class PostForAuthor < ActiveRecord::Base self.table_name = 'posts' cattr_accessor :selected_author - default_scope lambda { where(:author_id => PostForAuthor.selected_author) } end class FirstPost < ActiveRecord::Base self.table_name = 'posts' - default_scope where(:id => 1) + + def self.default_scope + where(:id => 1) + end + has_many :comments, :foreign_key => :post_id has_one :comment, :foreign_key => :post_id end diff --git a/activerecord/test/models/reference.rb b/activerecord/test/models/reference.rb index e33a0f2acc..76c0a1a32e 100644 --- a/activerecord/test/models/reference.rb +++ b/activerecord/test/models/reference.rb @@ -18,6 +18,9 @@ class Reference < ActiveRecord::Base end class BadReference < ActiveRecord::Base - self.table_name ='references' - default_scope :conditions => {:favourite => false } + self.table_name = 'references' + + def self.default_scope + where :favourite => false + end end diff --git a/activerecord/test/models/without_table.rb b/activerecord/test/models/without_table.rb index 87f80911e1..1a63d6ceb6 100644 --- a/activerecord/test/models/without_table.rb +++ b/activerecord/test/models/without_table.rb @@ -1,3 +1,5 @@ class WithoutTable < ActiveRecord::Base - default_scope where(:published => true) -end \ No newline at end of file + def self.default_scope + where(:published => true) + end +end -- cgit v1.2.3 From 8572ae6671c6ec7c2524f327cee82215896e5648 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Fri, 8 Apr 2011 20:56:33 +0100 Subject: Evaluate default scopes at the last possible moment in order to avoid problems with default scopes getting included into other scopes and then being unable to remove the default part via unscoped. --- activerecord/CHANGELOG | 11 +++++++++++ activerecord/lib/active_record/base.rb | 13 +++++-------- activerecord/lib/active_record/named_scope.rb | 8 +++++++- activerecord/lib/active_record/relation.rb | 17 ++++++++++++++--- .../lib/active_record/relation/query_methods.rb | 14 ++++++++++---- .../lib/active_record/relation/spawn_methods.rb | 4 ++++ activerecord/test/cases/base_test.rb | 12 ++---------- activerecord/test/cases/relation_scoping_test.rb | 17 +++++++++++------ activerecord/test/cases/relation_test.rb | 2 +- activerecord/test/models/developer.rb | 2 ++ 10 files changed, 67 insertions(+), 33 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 64be577624..6b3d408720 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,16 @@ *Rails 3.1.0 (unreleased)* +* Default scopes are now evaluated at the latest possible moment, to avoid problems where + scopes would be created which would implicitly contain the default scope, which would then + be impossible to get rid of via Model.unscoped. + + Note that this means that if you are inspecting the internal structure of an + ActiveRecord::Relation, it will *not* contain the default scope, though the resulting + query will do. You can get a relation containing the default scope by calling + ActiveRecord#with_default_scope, though this is not part of the public API. + + [Jon Leighton] + * Deprecated support for passing hashes and relations to 'default_scope'. Please create a class method for your scope instead. For example, change this: diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index e0fb94c96e..08a41e2d8b 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1110,9 +1110,7 @@ module ActiveRecord #:nodoc: # If another Active Record class has been passed in, get its current scope scope = scope.current_scope if !scope.is_a?(Relation) && scope.respond_to?(:current_scope) - # Cannot use self.current_scope, because that could trigger build_default_scope, which - # could in turn result in with_scope being called and hence an infinite loop. - previous_scope = Thread.current[:"#{self}_current_scope"] + previous_scope = self.current_scope if scope.is_a?(Hash) # Dup first and second level of hash (method and params). @@ -1123,6 +1121,7 @@ module ActiveRecord #:nodoc: scope.assert_valid_keys([ :find, :create ]) relation = construct_finder_arel(scope[:find] || {}) + relation.default_scoped = true unless action == :overwrite if previous_scope && previous_scope.create_with_value && scope[:create] scope_for_create = if action == :merge @@ -1171,7 +1170,7 @@ MSG end def current_scope #:nodoc: - Thread.current[:"#{self}_current_scope"] ||= build_default_scope + Thread.current[:"#{self}_current_scope"] end def current_scope=(scope) #:nodoc: @@ -1227,15 +1226,13 @@ class Post < ActiveRecord::Base end WARN - # Reset the current scope as it may contain scopes based on a now-invalid default scope - self.current_scope = nil self.default_scopes = default_scopes.dup << scope end def build_default_scope #:nodoc: if method(:default_scope).owner != Base.singleton_class - # Exclusively scope to just the relation, to avoid infinite recursion where the - # default scope tries to use the default scope tries to use the default scope... + # Use relation.scoping to ensure we ignore whatever the current value of + # self.current_scope may be. relation.scoping { default_scope } elsif default_scopes.any? default_scopes.inject(relation) do |default_scope, scope| diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index a398476dd6..68e3018e22 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -35,7 +35,13 @@ module ActiveRecord if options scoped.apply_finder_options(options) else - (current_scope || relation).clone + if current_scope + current_scope.clone + else + scope = relation.clone + scope.default_scoped = true + scope + end end end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 645ad0fce1..4aa2516cb2 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -6,7 +6,7 @@ module ActiveRecord JoinOperation = Struct.new(:relation, :join_class, :on) ASSOCIATION_METHODS = [:includes, :eager_load, :preload] MULTI_VALUE_METHODS = [:select, :group, :order, :joins, :where, :having, :bind] - SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :create_with, :from] + SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :create_with, :from, :reorder] include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches @@ -15,14 +15,16 @@ module ActiveRecord delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key, :to => :klass attr_reader :table, :klass, :loaded - attr_accessor :extensions + attr_accessor :extensions, :default_scoped alias :loaded? :loaded + alias :default_scoped? :default_scoped def initialize(klass, table) @klass, @table = klass, table @implicit_readonly = nil @loaded = false + @default_scoped = false SINGLE_VALUE_METHODS.each {|v| instance_variable_set(:"@#{v}_value", nil)} (ASSOCIATION_METHODS + MULTI_VALUE_METHODS).each {|v| instance_variable_set(:"@#{v}_values", [])} @@ -372,7 +374,7 @@ module ActiveRecord end def where_values_hash - equalities = @where_values.grep(Arel::Nodes::Equality).find_all { |node| + equalities = with_default_scope.where_values.grep(Arel::Nodes::Equality).find_all { |node| node.left.relation.name == table_name } @@ -400,6 +402,15 @@ module ActiveRecord to_a.inspect end + def with_default_scope #:nodoc: + if default_scoped? + default_scope = @klass.send(:build_default_scope) + default_scope ? default_scope.merge(self) : self + else + self + end + end + protected def method_missing(method, *args, &block) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 02b7056989..94aa999715 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -8,7 +8,8 @@ module ActiveRecord attr_accessor :includes_values, :eager_load_values, :preload_values, :select_values, :group_values, :order_values, :joins_values, :where_values, :having_values, :bind_values, - :limit_value, :offset_value, :lock_value, :readonly_value, :create_with_value, :from_value + :limit_value, :offset_value, :lock_value, :readonly_value, :create_with_value, + :from_value, :reorder_value def includes(*args) args.reject! {|a| a.blank? } @@ -63,7 +64,11 @@ module ActiveRecord end def reorder(*args) - except(:order).order(args) + return self if args.blank? + + relation = clone + relation.reorder_value = args.flatten + relation end def joins(*args) @@ -163,7 +168,7 @@ module ActiveRecord end def arel - @arel ||= build_arel + @arel ||= with_default_scope.build_arel end def build_arel @@ -180,7 +185,8 @@ module ActiveRecord arel.group(*@group_values.uniq.reject{|g| g.blank?}) unless @group_values.empty? - arel.order(*@order_values.uniq.reject{|o| o.blank?}) unless @order_values.empty? + order = @reorder_value ? @reorder_value : @order_values + arel.order(*order.uniq.reject{|o| o.blank?}) unless order.empty? build_select(arel, @select_values.uniq) diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 128e0fbd86..69706b5ead 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -8,6 +8,8 @@ module ActiveRecord merged_relation = clone + r = r.with_default_scope if r.default_scoped? && r.klass != klass + Relation::ASSOCIATION_METHODS.each do |method| value = r.send(:"#{method}_values") @@ -70,6 +72,7 @@ module ActiveRecord # def except(*skips) result = self.class.new(@klass, table) + result.default_scoped = default_scoped ((Relation::ASSOCIATION_METHODS + Relation::MULTI_VALUE_METHODS) - skips).each do |method| result.send(:"#{method}_values=", send(:"#{method}_values")) @@ -94,6 +97,7 @@ module ActiveRecord # def only(*onlies) result = self.class.new(@klass, table) + result.default_scoped = default_scoped ((Relation::ASSOCIATION_METHODS + Relation::MULTI_VALUE_METHODS) & onlies).each do |method| result.send(:"#{method}_values=", send(:"#{method}_values")) diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 73887f9b49..e57c5b3b87 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1626,17 +1626,9 @@ class BasicsTest < ActiveRecord::TestCase assert_not_equal c1, c2 end - def test_default_scope_is_reset + def test_current_scope_is_reset Object.const_set :UnloadablePost, Class.new(ActiveRecord::Base) - UnloadablePost.table_name = 'posts' - UnloadablePost.class_eval do - class << self - def default_scope - order('posts.comments_count ASC') - end - end - end - UnloadablePost.scoped # make Thread.current[:UnloadablePost_scoped_methods] not nil + UnloadablePost.send(:current_scope=, UnloadablePost.scoped) UnloadablePost.unloadable assert_not_nil Thread.current[:UnloadablePost_current_scope] diff --git a/activerecord/test/cases/relation_scoping_test.rb b/activerecord/test/cases/relation_scoping_test.rb index a91072b94b..5079aec9ba 100644 --- a/activerecord/test/cases/relation_scoping_test.rb +++ b/activerecord/test/cases/relation_scoping_test.rb @@ -329,7 +329,7 @@ class DefaultScopingTest < ActiveRecord::TestCase def test_default_scoping_with_threads 2.times do - Thread.new { assert_equal ['salary DESC'], DeveloperOrderedBySalary.scoped.order_values }.join + Thread.new { assert DeveloperOrderedBySalary.scoped.to_sql.include?('salary DESC') }.join end end @@ -427,6 +427,13 @@ class DefaultScopingTest < ActiveRecord::TestCase jamis = PoorDeveloperCalledJamis.create_with(:name => 'Aaron').create_with(nil).new assert_equal 'Jamis', jamis.name end + + def test_unscoped_with_named_scope_should_not_have_default_scope + assert_equal [DeveloperCalledJamis.find(developers(:poor_jamis).id)], DeveloperCalledJamis.poor + + assert DeveloperCalledJamis.unscoped.poor.include?(developers(:david).becomes(DeveloperCalledJamis)) + assert_equal 10, DeveloperCalledJamis.unscoped.poor.length + end end class DeprecatedDefaultScopingTest < ActiveRecord::TestCase @@ -459,7 +466,7 @@ class DeprecatedDefaultScopingTest < ActiveRecord::TestCase def test_default_scoping_with_threads 2.times do - Thread.new { assert_equal ['salary DESC'], DeprecatedDeveloperOrderedBySalary.scoped.order_values }.join + Thread.new { assert DeprecatedDeveloperOrderedBySalary.scoped.to_sql.include?('salary DESC') }.join end end @@ -469,12 +476,10 @@ class DeprecatedDefaultScopingTest < ActiveRecord::TestCase ActiveSupport::Deprecation.silence { klass.send :default_scope, :limit => 1 } # Scopes added on children should append to parent scope - assert_equal 1, klass.scoped.limit_value - assert_equal ['salary DESC'], klass.scoped.order_values + assert_equal [developers(:jamis).id], klass.all.map(&:id) # Parent should still have the original scope - assert_nil DeprecatedDeveloperOrderedBySalary.scoped.limit_value - assert_equal ['salary DESC'], DeprecatedDeveloperOrderedBySalary.scoped.order_values + assert_equal Developer.order('salary DESC').map(&:id), DeprecatedDeveloperOrderedBySalary.all.map(&:id) end def test_default_scope_called_twice_merges_conditions diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 7bdbd773b6..6874bd18f8 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -20,7 +20,7 @@ module ActiveRecord end def test_single_values - assert_equal [:limit, :offset, :lock, :readonly, :create_with, :from].map(&:to_s).sort, + assert_equal [:limit, :offset, :lock, :readonly, :create_with, :from, :reorder].map(&:to_s).sort, Relation::SINGLE_VALUE_METHODS.map(&:to_s).sort end diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index 28b31caf7b..10385ba899 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -114,6 +114,8 @@ class DeveloperCalledJamis < ActiveRecord::Base def self.default_scope where :name => 'Jamis' end + + scope :poor, where('salary < 150000') end class AbstractDeveloperCalledJamis < ActiveRecord::Base -- cgit v1.2.3 From 788bd30859f3f21184defd240c3d32f179515225 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Fri, 8 Apr 2011 23:53:25 +0100 Subject: ActiveRecord::Base.scopes hash is not needed --- activerecord/lib/active_record/named_scope.rb | 20 ++++---------------- activerecord/lib/active_record/relation.rb | 2 -- activerecord/test/cases/named_scope_test.rb | 11 ----------- 3 files changed, 4 insertions(+), 29 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 68e3018e22..60840e6958 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -9,11 +9,6 @@ module ActiveRecord module NamedScope extend ActiveSupport::Concern - included do - class_attribute :scopes - self.scopes = {} - end - module ClassMethods # Returns an anonymous \scope. # @@ -135,27 +130,20 @@ module ActiveRecord scope_proc = lambda do |*args| options = scope_options.respond_to?(:call) ? scope_options.call(*args) : scope_options + options = scoped.apply_finder_options(options) if options.is_a?(Hash) - relation = if options.is_a?(Hash) - scoped.apply_finder_options(options) - elsif options - scoped.merge(options) - else - scoped - end + relation = scoped.merge(options) extension ? relation.extending(extension) : relation end - self.scopes = self.scopes.merge name => scope_proc - - singleton_class.send(:redefine_method, name, &scopes[name]) + singleton_class.send(:redefine_method, name, &scope_proc) end protected def valid_scope_name?(name) - if !scopes[name] && respond_to?(name, true) + if respond_to?(name, true) logger.warn "Creating scope :#{name}. " \ "Overwriting existing method #{self.name}.#{name}." end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 4aa2516cb2..490360ccb5 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -416,8 +416,6 @@ module ActiveRecord def method_missing(method, *args, &block) if Array.method_defined?(method) to_a.send(method, *args, &block) - elsif @klass.scopes[method] - merge(@klass.send(method, *args, &block)) elsif @klass.respond_to?(method) scoping { @klass.send(method, *args, &block) } elsif arel.respond_to?(method) diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index 2b3e1900e1..8fd1fc2577 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -58,17 +58,6 @@ class NamedScopeTest < ActiveRecord::TestCase assert Topic.approved.respond_to?(:tables_in_string, true) end - def test_subclasses_inherit_scopes - assert Topic.scopes.include?(:base) - - assert Reply.scopes.include?(:base) - assert_equal Reply.find(:all), Reply.base - end - - def test_classes_dont_inherit_subclasses_scopes - assert !ActiveRecord::Base.scopes.include?(:base) - end - def test_scopes_with_options_limit_finds_to_those_matching_the_criteria_specified assert !Topic.find(:all, :conditions => {:approved => true}).empty? -- cgit v1.2.3 From f0e198bfa1e3f9689e0cde1d194a44027fc90b3c Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Fri, 8 Apr 2011 23:54:54 +0100 Subject: Deprecate defining scopes with a callable (lambda, proc, etc) via the scope class method. Just define a class method yourself instead. --- activerecord/CHANGELOG | 18 ++++++++ activerecord/lib/active_record/named_scope.rb | 62 ++++++++++++++++++++++++++- activerecord/test/cases/named_scope_test.rb | 6 +++ activerecord/test/models/comment.rb | 5 ++- activerecord/test/models/post.rb | 22 ++++++---- activerecord/test/models/topic.rb | 26 ++++++----- 6 files changed, 116 insertions(+), 23 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 6b3d408720..93eb42a52c 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,23 @@ *Rails 3.1.0 (unreleased)* +* Passing a proc (or other object that responds to #call) to scope is deprecated. If you need your + scope to be lazily evaluated, or takes parameters, please define it as a normal class method + instead. For example, change this: + + class Post < ActiveRecord::Base + scope :unpublished, lambda { where('published_at > ?', Time.now) } + end + + To this: + + class Post < ActiveRecord::Base + def self.unpublished + where('published_at > ?', Time.now) + end + end + + [Jon Leighton] + * Default scopes are now evaluated at the latest possible moment, to avoid problems where scopes would be created which would implicitly contain the default scope, which would then be impossible to get rid of via Model.unscoped. diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 60840e6958..f1df04950b 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -51,6 +51,14 @@ module ActiveRecord # The above calls to scope define class methods Shirt.red and Shirt.dry_clean_only. Shirt.red, # in effect, represents the query Shirt.where(:color => 'red'). # + # Note that this is simply 'syntactic sugar' for defining an actual class method: + # + # class Shirt < ActiveRecord::Base + # def self.red + # where(:color => 'red') + # end + # end + # # Unlike Shirt.find(...), however, the object returned by Shirt.red is not an Array; it # resembles the association object constructed by a has_many declaration. For instance, # you can invoke Shirt.red.first, Shirt.red.count, Shirt.red.where(:size => 'small'). @@ -74,14 +82,34 @@ module ActiveRecord # then elton.shirts.red.dry_clean_only will return all of Elton's red, dry clean # only shirts. # - # Named \scopes can also be procedural: + # If you need to pass parameters to a scope, define it as a normal method: # # class Shirt < ActiveRecord::Base - # scope :colored, lambda {|color| where(:color => color) } + # def self.colored(color) + # where(:color => color) + # end # end # # In this example, Shirt.colored('puce') finds all puce shirts. # + # Note that scopes defined with \scope will be evaluated when they are defined, rather than + # when they are used. For example, the following would be incorrect: + # + # class Post < ActiveRecord::Base + # scope :recent, where('published_at >= ?', Time.now - 1.week) + # end + # + # The example above would be 'frozen' to the Time.now value when the Post + # class was defined, and so the resultant SQL query would always be the same. The correct + # way to do this would be via a class method, which will re-evaluate the scope each time + # it is called: + # + # class Post < ActiveRecord::Base + # def self.recent + # where('published_at >= ?', Time.now - 1.week) + # end + # end + # # Named \scopes can also have extensions, just as with has_many declarations: # # class Shirt < ActiveRecord::Base @@ -92,6 +120,18 @@ module ActiveRecord # end # end # + # The above could also be written as a class method like so: + # + # class Shirt < ActiveRecord::Base + # def self.red + # where(:color => 'red').extending do + # def dom_id + # 'red_shirts' + # end + # end + # end + # end + # # Scopes can also be used while creating/building a record. # # class Article < ActiveRecord::Base @@ -128,6 +168,24 @@ module ActiveRecord valid_scope_name?(name) extension = Module.new(&Proc.new) if block_given? + if !scope_options.is_a?(Relation) && scope_options.respond_to?(:call) + ActiveSupport::Deprecation.warn <<-WARN +Passing a proc (or other object that responds to #call) to scope is deprecated. If you need your scope to be lazily evaluated, or takes parameters, please define it as a normal class method instead. For example, change this: + +class Post < ActiveRecord::Base + scope :unpublished, lambda { where('published_at > ?', Time.now) } +end + +To this: + +class Post < ActiveRecord::Base + def self.unpublished + where('published_at > ?', Time.now) + end +end + WARN + end + scope_proc = lambda do |*args| options = scope_options.respond_to?(:call) ? scope_options.call(*args) : scope_options options = scoped.apply_finder_options(options) if options.is_a?(Hash) diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index 8fd1fc2577..2880fdc651 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -471,6 +471,12 @@ class NamedScopeTest < ActiveRecord::TestCase require "models/without_table" end end + + def test_scopes_with_callables_are_deprecated + assert_deprecated do + Post.scope :WE_SO_EXCITED, lambda { |partyingpartyingpartying, yeah| fun!.fun!.fun! } + end + end end class DynamicScopeMatchTest < ActiveRecord::TestCase diff --git a/activerecord/test/models/comment.rb b/activerecord/test/models/comment.rb index 2a4c37089a..3bd7db7834 100644 --- a/activerecord/test/models/comment.rb +++ b/activerecord/test/models/comment.rb @@ -1,5 +1,8 @@ class Comment < ActiveRecord::Base - scope :limit_by, lambda {|l| limit(l) } + def self.limit_by(l) + limit(l) + end + 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 } diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 632f83e5d4..7055380d85 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -7,12 +7,15 @@ class Post < ActiveRecord::Base 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| { - :conditions => [ 'authors.author_address_id = ?', address.id ], - :joins => 'JOIN authors ON authors.id = posts.author_id' - } - } + + def self.limit_by(l) + limit(l) + end + + def self.with_authors_at_address(address) + where('authors.author_address_id = ?', address.id) + .joins('JOIN authors ON authors.id = posts.author_id') + end belongs_to :author do def greeting @@ -27,9 +30,10 @@ class Post < ActiveRecord::Base scope :with_special_comments, :joins => :comments, :conditions => {: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} } } - } + + def self.with_post(post_id) + joins(:comments).where(:comments => { :post_id => post_id }) + end has_many :comments do def find_most_recent diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb index 6440dbe8ab..60e750e6c4 100644 --- a/activerecord/test/models/topic.rb +++ b/activerecord/test/models/topic.rb @@ -1,10 +1,20 @@ class Topic < ActiveRecord::Base scope :base - scope :written_before, lambda { |time| - if time - { :conditions => ['written_on < ?', time] } - end - } + + ActiveSupport::Deprecation.silence do + scope :written_before, lambda { |time| + if time + { :conditions => ['written_on < ?', time] } + end + } + + scope :with_object, Class.new(Struct.new(:klass)) { + def call + klass.where(:approved => true) + end + }.new(self) + end + scope :approved, :conditions => {:approved => true} scope :rejected, :conditions => {:approved => false} @@ -19,12 +29,6 @@ class Topic < ActiveRecord::Base end end - scope :with_object, Class.new(Struct.new(:klass)) { - def call - klass.where(:approved => true) - end - }.new(self) - module NamedExtension def two 2 -- cgit v1.2.3 From 1b5b53da5e4beb24dc7d41a1e9e5d72b82586985 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 12 Apr 2011 20:29:35 -0700 Subject: common @jonleighton :bomb: --- activerecord/test/models/post.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 7055380d85..a91c10276b 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -13,8 +13,7 @@ class Post < ActiveRecord::Base end def self.with_authors_at_address(address) - where('authors.author_address_id = ?', address.id) - .joins('JOIN authors ON authors.id = posts.author_id') + where('authors.author_address_id = ?', address.id).joins('JOIN authors ON authors.id = posts.author_id') end belongs_to :author do -- cgit v1.2.3 From 733bfa63f5d8d3b963202b6d3e9f00b4db070b91 Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Wed, 13 Apr 2011 00:04:40 +0800 Subject: Remove `#among?` from Active Support After a long list of discussion about the performance problem from using varargs and the reason that we can't find a great pair for it, it would be best to remove support for it for now. It will come back if we can find a good pair for it. For now, Bon Voyage, `#among?`. --- activerecord/lib/active_record/associations/association.rb | 2 +- activerecord/lib/active_record/associations/builder/belongs_to.rb | 2 +- activerecord/lib/active_record/associations/builder/has_many.rb | 2 +- activerecord/lib/active_record/associations/builder/has_one.rb | 2 +- activerecord/lib/active_record/associations/has_one_association.rb | 2 +- .../lib/active_record/attribute_methods/time_zone_conversion.rb | 2 +- activerecord/lib/active_record/railties/databases.rake | 2 +- activerecord/lib/active_record/reflection.rb | 2 +- .../active_record/session_migration/session_migration_generator.rb | 2 +- activerecord/test/cases/attribute_methods_test.rb | 2 +- activerecord/test/cases/defaults_test.rb | 2 +- 11 files changed, 11 insertions(+), 11 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 66f6477ec5..687b668634 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -164,7 +164,7 @@ module ActiveRecord def creation_attributes attributes = {} - if reflection.macro.among?(:has_one, :has_many) && !options[:through] + if reflection.macro.in?([:has_one, :has_many]) && !options[:through] attributes[reflection.foreign_key] = owner[reflection.active_record_primary_key] if reflection.options[:as] diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 763b12f708..f6d26840c2 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -67,7 +67,7 @@ module ActiveRecord::Associations::Builder def configure_dependency if options[:dependent] - unless options[:dependent].among?(:destroy, :delete) + unless options[:dependent].in?([:destroy, :delete]) raise ArgumentError, "The :dependent option expects either :destroy or :delete (#{options[:dependent].inspect})" end diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb index 67ce339380..ecbc70888f 100644 --- a/activerecord/lib/active_record/associations/builder/has_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_many.rb @@ -16,7 +16,7 @@ module ActiveRecord::Associations::Builder def configure_dependency if options[:dependent] - unless options[:dependent].among?(:destroy, :delete_all, :nullify, :restrict) + unless options[:dependent].in?([:destroy, :delete_all, :nullify, :restrict]) raise ArgumentError, "The :dependent option expects either :destroy, :delete_all, " \ ":nullify or :restrict (#{options[:dependent].inspect})" end diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb index 18d7117bb7..88c0d3e90f 100644 --- a/activerecord/lib/active_record/associations/builder/has_one.rb +++ b/activerecord/lib/active_record/associations/builder/has_one.rb @@ -29,7 +29,7 @@ module ActiveRecord::Associations::Builder def configure_dependency if options[:dependent] - unless options[:dependent].among?(:destroy, :delete, :nullify, :restrict) + unless options[:dependent].in?([:destroy, :delete, :nullify, :restrict]) raise ArgumentError, "The :dependent option expects either :destroy, :delete, " \ ":nullify or :restrict (#{options[:dependent].inspect})" end diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index aaa8b97389..7134dc85c8 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -52,7 +52,7 @@ module ActiveRecord end def remove_target!(method) - if method.among?(:delete, :destroy) + if method.in?([:delete, :destroy]) target.send(method) else nullify_owner_attributes(target) diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index 67b0e77a25..62a3cfa9a5 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -59,7 +59,7 @@ module ActiveRecord private def create_time_zone_conversion_attribute?(name, column) - time_zone_aware_attributes && !self.skip_time_zone_conversion_for_attributes.include?(name.to_sym) && column.type.among?(:datetime, :timestamp) + time_zone_aware_attributes && !self.skip_time_zone_conversion_for_attributes.include?(name.to_sym) && column.type.in?([:datetime, :timestamp]) end end end diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 944a29a3e6..6b3c38cb58 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -137,7 +137,7 @@ db_namespace = namespace :db do end def local_database?(config, &block) - if config['host'].among?("127.0.0.1", "localhost") || config['host'].blank? + if config['host'].in?(["127.0.0.1", "localhost"]) || config['host'].blank? yield else $stderr.puts "This task only modifies local databases. #{config['database']} is on a remote host." diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index eac6a5dcad..bcba85d7a4 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -164,7 +164,7 @@ module ActiveRecord def initialize(macro, name, options, active_record) super - @collection = macro.among?(:has_many, :has_and_belongs_to_many) + @collection = macro.in?([:has_many, :has_and_belongs_to_many]) end # Returns a new, unsaved instance of the associated class. +options+ will diff --git a/activerecord/lib/rails/generators/active_record/session_migration/session_migration_generator.rb b/activerecord/lib/rails/generators/active_record/session_migration/session_migration_generator.rb index d80c8ba996..90923f6e74 100644 --- a/activerecord/lib/rails/generators/active_record/session_migration/session_migration_generator.rb +++ b/activerecord/lib/rails/generators/active_record/session_migration/session_migration_generator.rb @@ -14,7 +14,7 @@ module ActiveRecord def session_table_name current_table_name = ActiveRecord::SessionStore::Session.table_name - if current_table_name.among?("sessions", "session") + if current_table_name.in?(["sessions", "session"]) current_table_name = (ActiveRecord::Base.pluralize_table_names ? 'session'.pluralize : 'session') end current_table_name diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index a3c5c14758..b898c003bd 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -639,7 +639,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase end def time_related_columns_on_topic - Topic.columns.select { |c| c.type.among?(:time, :date, :datetime, :timestamp) } + Topic.columns.select { |c| c.type.in?([:time, :date, :datetime, :timestamp]) } end def serialized_columns_on_topic diff --git a/activerecord/test/cases/defaults_test.rb b/activerecord/test/cases/defaults_test.rb index 42b14bd25c..b3a281d960 100644 --- a/activerecord/test/cases/defaults_test.rb +++ b/activerecord/test/cases/defaults_test.rb @@ -95,7 +95,7 @@ if current_adapter?(:MysqlAdapter) or current_adapter?(:Mysql2Adapter) assert_equal 0, klass.columns_hash['zero'].default assert !klass.columns_hash['zero'].null # 0 in MySQL 4, nil in 5. - assert klass.columns_hash['omit'].default.among?(0, nil) + assert klass.columns_hash['omit'].default.in?([0, nil]) assert !klass.columns_hash['omit'].null assert_raise(ActiveRecord::StatementInvalid) { klass.create! } -- cgit v1.2.3 From eebb19c954d64760c232cda10263200f32fcf036 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Apr 2011 09:42:31 -0700 Subject: use index based substitution for bind parameters --- .../lib/active_record/connection_adapters/abstract_adapter.rb | 2 +- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 4 ++-- activerecord/lib/active_record/relation/finder_methods.rb | 2 +- activerecord/test/cases/adapters/mysql/connection_test.rb | 2 +- .../test/cases/adapters/postgresql/postgresql_adapter_test.rb | 6 +++--- activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb | 2 +- activerecord/test/cases/bind_parameter_test.rb | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index b0e6136e12..d24cce0a3c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -105,7 +105,7 @@ module ActiveRecord # Returns a bind substitution value given a +column+ and list of current # +binds+ - def substitute_for(column, binds) + def substitute_at(column, index) Arel.sql '?' end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 1c2cc7d5fa..05f0e5ebe1 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -511,8 +511,8 @@ module ActiveRecord end end - def substitute_for(column, current_values) - Arel.sql("$#{current_values.length + 1}") + def substitute_at(column, index) + Arel.sql("$#{index + 1}") end def exec_query(sql, name = 'SQL', binds = []) diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 673e47942b..aae257a0e7 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -320,7 +320,7 @@ module ActiveRecord column = columns_hash[primary_key] - substitute = connection.substitute_for(column, @bind_values) + substitute = connection.substitute_at(column, @bind_values.length) relation = where(table[primary_key].eq(substitute)) relation.bind_values = [[column, id]] record = relation.first diff --git a/activerecord/test/cases/adapters/mysql/connection_test.rb b/activerecord/test/cases/adapters/mysql/connection_test.rb index eb3f8143e7..eee771ecff 100644 --- a/activerecord/test/cases/adapters/mysql/connection_test.rb +++ b/activerecord/test/cases/adapters/mysql/connection_test.rb @@ -44,7 +44,7 @@ class MysqlConnectionTest < ActiveRecord::TestCase end def test_bind_value_substitute - bind_param = @connection.substitute_for('foo', []) + bind_param = @connection.substitute_at('foo', 0) assert_equal Arel.sql('?'), bind_param end diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index 2d412a6e2a..7c49236854 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -102,11 +102,11 @@ module ActiveRecord assert_equal [['1', 'foo']], result.rows end - def test_substitute_for - bind = @connection.substitute_for(nil, []) + def test_substitute_at + bind = @connection.substitute_at(nil, 0) assert_equal Arel.sql('$1'), bind - bind = @connection.substitute_for(nil, [nil]) + bind = @connection.substitute_at(nil, 1) assert_equal Arel.sql('$2'), bind end diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index 0e2f468908..6ff04e3eb3 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -80,7 +80,7 @@ module ActiveRecord end def test_bind_value_substitute - bind_param = @conn.substitute_for('foo', []) + bind_param = @conn.substitute_at('foo', 0) assert_equal Arel.sql('?'), bind_param end diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb index 19383bb06b..3652255c38 100644 --- a/activerecord/test/cases/bind_parameter_test.rb +++ b/activerecord/test/cases/bind_parameter_test.rb @@ -33,7 +33,7 @@ module ActiveRecord # FIXME: use skip with minitest return unless @connection.supports_statement_cache? - sub = @connection.substitute_for(@pk, []) + sub = @connection.substitute_at(@pk, 0) binds = [[@pk, 1]] sql = "select * from topics where id = #{sub}" -- cgit v1.2.3 From 1f4dae9daa8d5be44a676f59681c013e8f501e8f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Apr 2011 11:41:34 -0700 Subject: do not depend on to_yaml being called, but rather depend on YAML being dumped --- .../active_record/connection_adapters/abstract/quoting.rb | 2 +- activerecord/test/cases/quoting_test.rb | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb index 7489e88eef..325665dd87 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb @@ -35,7 +35,7 @@ module ActiveRecord when Date, Time then "'#{quoted_date(value)}'" when Symbol then "'#{quote_string(value.to_s)}'" else - "'#{quote_string(value.to_yaml)}'" + "'#{quote_string(YAML.dump(value))}'" end end diff --git a/activerecord/test/cases/quoting_test.rb b/activerecord/test/cases/quoting_test.rb index b87fb51d97..80ee74e41e 100644 --- a/activerecord/test/cases/quoting_test.rb +++ b/activerecord/test/cases/quoting_test.rb @@ -154,15 +154,16 @@ module ActiveRecord end def test_crazy_object - crazy = Class.new { def to_yaml; 'lol' end }.new - assert_equal "'lol'", @quoter.quote(crazy, nil) - assert_equal "'lol'", @quoter.quote(crazy, Object.new) + crazy = Class.new.new + expected = "'#{YAML.dump(crazy)}'" + assert_equal expected, @quoter.quote(crazy, nil) + assert_equal expected, @quoter.quote(crazy, Object.new) end def test_crazy_object_calls_quote_string - crazy = Class.new { def to_yaml; 'lo\l' end }.new - assert_equal "'lo\\\\l'", @quoter.quote(crazy, nil) - assert_equal "'lo\\\\l'", @quoter.quote(crazy, Object.new) + crazy = Class.new { def initialize; @lol = 'lo\l' end }.new + assert_match "lo\\\\l", @quoter.quote(crazy, nil) + assert_match "lo\\\\l", @quoter.quote(crazy, Object.new) end def test_quote_string_no_column -- cgit v1.2.3 From 93641ed6c8c684f6b4db02b6c8a22fa9bc7f0eaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stian=20Gryt=C3=B8yr?= Date: Thu, 14 Apr 2011 16:31:08 +0200 Subject: Fixes performance issue introduced in 3.0.6 (issue #6695) --- activerecord/lib/active_record/attribute_methods/read.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 69d5cd83f1..43f736c89c 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -84,9 +84,11 @@ module ActiveRecord # Returns the value of the attribute identified by attr_name after it has been typecast (for example, # "2004-12-12" in a data column is cast to a date object, like Date.new(2004, 12, 12)). def read_attribute(attr_name) - send "_#{attr_name}" - rescue NoMethodError - _read_attribute attr_name + if respond_to? "_#{attr_name}" + send "_#{attr_name}" + else + _read_attribute attr_name + end end def _read_attribute(attr_name) -- cgit v1.2.3 From 6b6ecbefad648f39b507dbb59c9d22ff9031f7a8 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Wed, 13 Apr 2011 00:15:24 +0100 Subject: Extract the constraint-building for joins in JoinAssociation into a separate method to make it easy to change/override (requested by Ernie Miller so that MetaWhere can add to it easily) --- .../join_dependency/join_association.rb | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index 4121a5b378..0a666598ed 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -89,14 +89,7 @@ module ActiveRecord foreign_key = reflection.active_record_primary_key end - constraint = table[key].eq(foreign_table[foreign_key]) - - if reflection.klass.finder_needs_type_condition? - constraint = table.create_and([ - constraint, - reflection.klass.send(:type_condition, table) - ]) - end + constraint = build_constraint(reflection, table, key, foreign_table, foreign_key) relation.from(join(table, constraint)) @@ -111,6 +104,19 @@ module ActiveRecord relation end + def build_constraint(reflection, table, key, foreign_table, foreign_key) + constraint = table[key].eq(foreign_table[foreign_key]) + + if reflection.klass.finder_needs_type_condition? + constraint = table.create_and([ + constraint, + reflection.klass.send(:type_condition, table) + ]) + end + + constraint + end + def join_relation(joining_relation) self.join_type = Arel::OuterJoin joining_relation.joins(self) -- cgit v1.2.3 From 4893170da20eee28c016408a0f72f1996343a048 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Apr 2011 14:03:35 -0700 Subject: adding a type cast method for prepared statements --- .../connection_adapters/abstract/quoting.rb | 36 +++++++++ .../connection_adapters/sqlite_adapter.rb | 2 +- .../test/cases/adapters/sqlite3/quoting_test.rb | 93 ++++++++++++++++++++++ 3 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 activerecord/test/cases/adapters/sqlite3/quoting_test.rb (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb index 325665dd87..5634e53e80 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb @@ -39,6 +39,42 @@ module ActiveRecord end end + # Cast a +value+ to a type that the database understands. For example, + # SQLite does not understand dates, so this method will convert a Date + # to a String. + def type_cast(value, column) + return value.id if value.respond_to?(:quoted_id) + + case value + when String, ActiveSupport::Multibyte::Chars + value = value.to_s + return value unless column + + case column.type + when :binary then value + when :integer then value.to_i + when :float then value.to_f + else + value + end + + when true, false + if column && column.type == :integer + value ? 1 : 0 + else + value ? 't' : 'f' + end + # BigDecimals need to be put in a non-normalized form and quoted. + when nil then nil + when BigDecimal then value.to_f + when Numeric then value + when Date, Time then quoted_date(value) + when Symbol then value.to_s + else + YAML.dump(value) + end + end + # Quotes a string, escaping any ' (single quote) and \ (backslash) # characters. def quote_string(s) diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 8ef286b473..14c4fd689b 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -165,7 +165,7 @@ module ActiveRecord cols = cache[:cols] ||= stmt.columns stmt.reset! stmt.bind_params binds.map { |col, val| - col ? col.type_cast(val) : val + type_cast(val, col) } end diff --git a/activerecord/test/cases/adapters/sqlite3/quoting_test.rb b/activerecord/test/cases/adapters/sqlite3/quoting_test.rb new file mode 100644 index 0000000000..e0152e7ccf --- /dev/null +++ b/activerecord/test/cases/adapters/sqlite3/quoting_test.rb @@ -0,0 +1,93 @@ +require "cases/helper" +require 'bigdecimal' +require 'yaml' + +module ActiveRecord + module ConnectionAdapters + class SQLiteAdapter + class QuotingTest < ActiveRecord::TestCase + def setup + @conn = Base.sqlite3_connection :database => ':memory:', + :adapter => 'sqlite3', + :timeout => 100 + end + + def test_type_cast_symbol + assert_equal 'foo', @conn.type_cast(:foo, nil) + end + + def test_type_cast_date + date = Date.today + expected = @conn.quoted_date(date) + assert_equal expected, @conn.type_cast(date, nil) + end + + def test_type_cast_time + time = Time.now + expected = @conn.quoted_date(time) + assert_equal expected, @conn.type_cast(time, nil) + end + + def test_type_cast_numeric + assert_equal 10, @conn.type_cast(10, nil) + assert_equal 2.2, @conn.type_cast(2.2, nil) + end + + def test_type_cast_nil + assert_equal nil, @conn.type_cast(nil, nil) + end + + def test_type_cast_true + c = Column.new(nil, 1, 'int') + assert_equal 't', @conn.type_cast(true, nil) + assert_equal 1, @conn.type_cast(true, c) + end + + def test_type_cast_false + c = Column.new(nil, 1, 'int') + assert_equal 'f', @conn.type_cast(false, nil) + assert_equal 0, @conn.type_cast(false, c) + end + + def test_type_cast_string + assert_equal '10', @conn.type_cast('10', nil) + + c = Column.new(nil, 1, 'int') + assert_equal 10, @conn.type_cast('10', c) + + c = Column.new(nil, 1, 'float') + assert_equal 10.1, @conn.type_cast('10.1', c) + + c = Column.new(nil, 1, 'binary') + assert_equal '10.1', @conn.type_cast('10.1', c) + + c = Column.new(nil, 1, 'date') + assert_equal '10.1', @conn.type_cast('10.1', c) + end + + def test_type_cast_bigdecimal + bd = BigDecimal.new '10.0' + assert_equal bd.to_f, @conn.type_cast(bd, nil) + end + + def test_type_cast_unknown + obj = Class.new.new + assert_equal YAML.dump(obj), @conn.type_cast(obj, nil) + end + + def test_quoted_id + quoted_id_obj = Class.new { + def quoted_id + "'zomg'" + end + + def id + 10 + end + }.new + assert_equal 10, @conn.type_cast(quoted_id_obj, nil) + end + end + end + end +end -- cgit v1.2.3 From 8571facea3b51717b3c57c50b2deae5dbf997c6e Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Apr 2011 10:41:12 -0700 Subject: insert statements are prepared, but values are not escaped properly --- .../abstract/database_statements.rb | 22 ++++++++++++++-- .../connection_adapters/mysql_adapter.rb | 4 +++ .../connection_adapters/postgresql_adapter.rb | 12 +++++++++ .../connection_adapters/sqlite_adapter.rb | 4 +++ activerecord/lib/active_record/relation.rb | 30 +++++++++++++++++----- 5 files changed, 64 insertions(+), 8 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index a3082b8f01..6d9b5c7b32 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -56,8 +56,17 @@ module ActiveRecord end # Returns the last auto-generated ID from the affected table. - def insert(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) - insert_sql(sql, name, pk, id_value, sequence_name) + # + # +id_value+ will be returned unless the value is nil, in + # which case the database will attempt to calculate the last inserted + # id and return that value. + # + # If the next id was calculated in advance (as in Oracle), it should be + # passed in as +id_value+. + def insert(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil, binds = []) + sql, binds = sql_for_insert(sql, pk, id_value, sequence_name, binds) + value = exec_insert(sql, name, binds) + id_value || last_inserted_id(value) end # Executes the update statement and returns the number of rows affected. @@ -364,6 +373,15 @@ module ActiveRecord end end end + + def sql_for_insert(sql, pk, id_value, sequence_name, binds) + [sql, binds] + end + + def last_inserted_id(result) + row = result.rows.first + row && row.first + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index d88075fdea..f461162dde 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -425,6 +425,10 @@ module ActiveRecord exec_query(sql, name, binds) end + def last_inserted_id(result) + @connection.insert_id + end + def exec_without_stmt(sql, name = 'SQL') # :nodoc: # Some queries, like SHOW CREATE TABLE don't work through the prepared # statement API. For those queries, we need to use this method. :'( diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 05f0e5ebe1..0884968363 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -544,6 +544,18 @@ module ActiveRecord exec_query(sql, name, binds) end + def sql_for_insert(sql, pk, id_value, sequence_name, binds) + unless pk + _, table = extract_schema_and_table(sql.split(" ", 4)[2]) + + pk = primary_key(table) + end + + sql = "#{sql} RETURNING #{quote_column_name(pk)}" if pk + + [sql, binds] + end + # Executes an UPDATE query and returns the number of affected tuples. def update_sql(sql, name = nil) super.cmd_tuples diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 14c4fd689b..9e7f874f4b 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -177,6 +177,10 @@ module ActiveRecord exec_query(sql, name, binds) end + def last_inserted_id(result) + @connection.last_insert_row_id + end + def execute(sql, name = nil) #:nodoc: log(sql, name) { @connection.execute(sql) } end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 490360ccb5..2f9970dec1 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -48,17 +48,35 @@ module ActiveRecord im = arel.create_insert im.into @table + conn = @klass.connection + if values.empty? # empty insert im.values = im.create_values [connection.null_insert_value], [] + @klass.connection.insert( + im.to_sql, + 'SQL', + primary_key, + primary_key_value) else - im.insert values + substitutes = values.to_a + binds = substitutes.map do |arel_attr, value| + [@klass.columns_hash[arel_attr.name], value] + end + substitutes.each_with_index do |tuple, i| + tuple[1] = conn.substitute_at(tuple.first, i) + end + + im.insert substitutes + + conn.insert( + im.to_sql, + 'SQL', + primary_key, + primary_key_value, + nil, + binds) end - @klass.connection.insert( - im.to_sql, - 'SQL', - primary_key, - primary_key_value) end def new(*args, &block) -- cgit v1.2.3 From 27f8c57f5f88304212a00cb8e32f8227755a7265 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Apr 2011 15:34:08 -0700 Subject: inserting big decimals as strings works consistently among dbs, so use string form --- activerecord/lib/active_record/connection_adapters/abstract/quoting.rb | 2 +- activerecord/test/cases/adapters/sqlite3/quoting_test.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb index 5634e53e80..3de850ec9e 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb @@ -66,7 +66,7 @@ module ActiveRecord end # BigDecimals need to be put in a non-normalized form and quoted. when nil then nil - when BigDecimal then value.to_f + when BigDecimal then value.to_s('F') when Numeric then value when Date, Time then quoted_date(value) when Symbol then value.to_s diff --git a/activerecord/test/cases/adapters/sqlite3/quoting_test.rb b/activerecord/test/cases/adapters/sqlite3/quoting_test.rb index e0152e7ccf..0d9db92447 100644 --- a/activerecord/test/cases/adapters/sqlite3/quoting_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/quoting_test.rb @@ -67,7 +67,7 @@ module ActiveRecord def test_type_cast_bigdecimal bd = BigDecimal.new '10.0' - assert_equal bd.to_f, @conn.type_cast(bd, nil) + assert_equal bd.to_s('F'), @conn.type_cast(bd, nil) end def test_type_cast_unknown -- cgit v1.2.3 From a0d4c8d1bf2198608e2e319ff833560f88d20855 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Apr 2011 15:35:53 -0700 Subject: using the database adapter to typecast before executing prepared statement --- .../connection_adapters/mysql_adapter.rb | 2 +- .../connection_adapters/postgresql_adapter.rb | 14 +++++++++++++- activerecord/test/cases/binary_test.rb | 19 +++++++++++++++++++ activerecord/test/schema/schema.rb | 1 + 4 files changed, 34 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index f461162dde..085357678c 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -403,7 +403,7 @@ module ActiveRecord end stmt.execute(*binds.map { |col, val| - col ? col.type_cast(val) : val + type_cast(val, col) }) if metadata = stmt.result_metadata cols = cache[:cols] ||= metadata.fetch_fields.map { |field| diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 0884968363..e74ec84e81 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -360,6 +360,18 @@ module ActiveRecord end end + def type_cast(value, column) + return super unless column + + case value + when String + return super unless 'bytea' == column.sql_type + escape_bytea(value) + else + super + end + end + # Quotes strings for use in SQL input. def quote_string(s) #:nodoc: @connection.escape(s) @@ -530,7 +542,7 @@ module ActiveRecord # Clear the queue @connection.get_last_result @connection.send_query_prepared(key, binds.map { |col, val| - col ? col.type_cast(val) : val + type_cast(val, col) }) @connection.block result = @connection.get_last_result diff --git a/activerecord/test/cases/binary_test.rb b/activerecord/test/cases/binary_test.rb index 8545ba97cc..06c14cb108 100644 --- a/activerecord/test/cases/binary_test.rb +++ b/activerecord/test/cases/binary_test.rb @@ -1,3 +1,4 @@ +# encoding: utf-8 require "cases/helper" # Without using prepared statements, it makes no sense to test @@ -9,6 +10,24 @@ unless current_adapter?(:SybaseAdapter, :DB2Adapter, :FirebirdAdapter) class BinaryTest < ActiveRecord::TestCase FIXTURES = %w(flowers.jpg example.log) + def test_mixed_encoding + str = "\x80" + str.force_encoding('ASCII-8BIT') if str.respond_to?(:force_encoding) + + binary = Binary.new :name => 'いただきます！', :data => str + binary.save! + binary.reload + assert_equal str, binary.data + + name = binary.name + + # Mysql adapter doesn't properly encode things, so we have to do it + if current_adapter?(:MysqlAdapter) + name.force_encoding('UTF-8') if name.respond_to?(:force_encoding) + end + assert_equal 'いただきます！', name + end + def test_load_save Binary.delete_all diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 362475de36..ceadb05644 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -67,6 +67,7 @@ ActiveRecord::Schema.define do end create_table :binaries, :force => true do |t| + t.string :name t.binary :data end -- cgit v1.2.3 From a22ceaeefa454a0dc7ae8277dda86e4caff5f99c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Apr 2011 16:05:30 -0700 Subject: mysql type cast should return integers when typecasting true / false --- .../connection_adapters/mysql_adapter.rb | 6 +++++ .../test/cases/adapters/mysql/quoting_test.rb | 26 ++++++++++++++++++++++ .../test/cases/adapters/postgresql/quoting_test.rb | 25 +++++++++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 activerecord/test/cases/adapters/mysql/quoting_test.rb create mode 100644 activerecord/test/cases/adapters/postgresql/quoting_test.rb (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 085357678c..c2e75acb9a 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -244,6 +244,12 @@ module ActiveRecord end end + def type_cast(value, column) + return super unless value == true || value == false + + value ? 1 : 0 + end + def quote_column_name(name) #:nodoc: @quoted_column_names[name] ||= "`#{name}`" end diff --git a/activerecord/test/cases/adapters/mysql/quoting_test.rb b/activerecord/test/cases/adapters/mysql/quoting_test.rb new file mode 100644 index 0000000000..9673e2bb46 --- /dev/null +++ b/activerecord/test/cases/adapters/mysql/quoting_test.rb @@ -0,0 +1,26 @@ +require "cases/helper" + +module ActiveRecord + module ConnectionAdapters + class MysqlAdapter + class QuotingTest < ActiveRecord::TestCase + def setup + @conn = ActiveRecord::Base.connection + end + + def test_type_cast_true + c = Column.new(nil, 1, 'boolean') + assert_equal 1, @conn.type_cast(true, nil) + assert_equal 1, @conn.type_cast(true, c) + end + + def test_type_cast_false + c = Column.new(nil, 1, 'boolean') + assert_equal 0, @conn.type_cast(false, nil) + assert_equal 0, @conn.type_cast(false, c) + end + end + end + end +end + diff --git a/activerecord/test/cases/adapters/postgresql/quoting_test.rb b/activerecord/test/cases/adapters/postgresql/quoting_test.rb new file mode 100644 index 0000000000..172055f15c --- /dev/null +++ b/activerecord/test/cases/adapters/postgresql/quoting_test.rb @@ -0,0 +1,25 @@ +require "cases/helper" + +module ActiveRecord + module ConnectionAdapters + class PostgreSQLAdapter + class QuotingTest < ActiveRecord::TestCase + def setup + @conn = ActiveRecord::Base.connection + end + + def test_type_cast_true + c = Column.new(nil, 1, 'boolean') + assert_equal 't', @conn.type_cast(true, nil) + assert_equal 't', @conn.type_cast(true, c) + end + + def test_type_cast_false + c = Column.new(nil, 1, 'boolean') + assert_equal 'f', @conn.type_cast(false, nil) + assert_equal 'f', @conn.type_cast(false, c) + end + end + end + end +end -- cgit v1.2.3 From 0268eac96366da46eabf7530131415867b9c09e1 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Apr 2011 16:28:23 -0700 Subject: mimic prepared statements in the exec_insert for mysql2 --- .../lib/active_record/connection_adapters/mysql2_adapter.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 1f7e527eeb..7ac72acd58 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -282,6 +282,17 @@ module ActiveRecord end alias :create :insert_sql + def exec_insert(sql, name, binds) + binds = binds.dup + + # Pretend to support bind parameters + execute sql.gsub('?') { quote(*binds.shift.reverse) }, name + end + + def last_inserted_id(result) + @connection.last_id + end + def update_sql(sql, name = nil) super @connection.affected_rows -- cgit v1.2.3 From 12ae92216bb17b6585f3ab4e9010bacd49a5ba9c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Apr 2011 18:07:22 -0700 Subject: refactoring inserts to use the same method on the connection --- activerecord/lib/active_record/relation.rb | 37 +++++++++++++----------------- 1 file changed, 16 insertions(+), 21 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 2f9970dec1..9fe806425e 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -50,33 +50,28 @@ module ActiveRecord conn = @klass.connection + substitutes = values.to_a + binds = substitutes.map do |arel_attr, value| + [@klass.columns_hash[arel_attr.name], value] + end + + substitutes.each_with_index do |tuple, i| + tuple[1] = conn.substitute_at(tuple.first, i) + end + if values.empty? # empty insert im.values = im.create_values [connection.null_insert_value], [] - @klass.connection.insert( - im.to_sql, - 'SQL', - primary_key, - primary_key_value) else - substitutes = values.to_a - binds = substitutes.map do |arel_attr, value| - [@klass.columns_hash[arel_attr.name], value] - end - substitutes.each_with_index do |tuple, i| - tuple[1] = conn.substitute_at(tuple.first, i) - end - im.insert substitutes - - conn.insert( - im.to_sql, - 'SQL', - primary_key, - primary_key_value, - nil, - binds) end + conn.insert( + im.to_sql, + 'SQL', + primary_key, + primary_key_value, + nil, + binds) end def new(*args, &block) -- cgit v1.2.3 From 9951af02891f889cbc8de818adb6b50ceb31e7bf Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 14 Apr 2011 14:26:57 -0700 Subject: sort insert columns for better cache hits --- activerecord/lib/active_record/relation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 9fe806425e..359f9d8a66 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -50,7 +50,7 @@ module ActiveRecord conn = @klass.connection - substitutes = values.to_a + substitutes = values.sort_by { |arel_attr,_| arel_attr.name } binds = substitutes.map do |arel_attr, value| [@klass.columns_hash[arel_attr.name], value] end -- cgit v1.2.3 From bbe0a507f287c20ab4ae8a244fbfc810665deda5 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Fri, 15 Apr 2011 01:44:27 +0100 Subject: Remove unnecessary code from define_read_method and add assertion to make sure the underscored version is actually generated --- activerecord/lib/active_record/attribute_methods/read.rb | 11 ++++------- activerecord/test/cases/attribute_methods_test.rb | 1 + 2 files changed, 5 insertions(+), 7 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 43f736c89c..dd3f4699f7 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -70,13 +70,10 @@ module ActiveRecord if cache_attribute?(attr_name) access_code = "@attributes_cache['#{attr_name}'] ||= (#{access_code})" end - if symbol =~ /^[a-zA-Z_]\w*[!?=]?$/ - generated_attribute_methods.module_eval("def _#{symbol}; #{access_code}; end; alias #{symbol} _#{symbol}", __FILE__, __LINE__) - else - generated_attribute_methods.module_eval do - define_method("_#{symbol}") { eval(access_code) } - alias_method(symbol, "_#{symbol}") - end + + generated_attribute_methods.module_eval do + define_method("_#{attr_name}") { eval(access_code) } + alias_method(attr_name, "_#{attr_name}") end end end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index b898c003bd..899166d129 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -77,6 +77,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase def test_respond_to? topic = Topic.find(1) assert_respond_to topic, "title" + assert_respond_to topic, "_title" assert_respond_to topic, "title?" assert_respond_to topic, "title=" assert_respond_to topic, :title -- cgit v1.2.3 From e68a83c9ecdc5b9395e0dde687cafdf8330f9de0 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Fri, 15 Apr 2011 02:19:11 +0100 Subject: Refactor test to avoid hackery --- activerecord/test/cases/associations/join_model_test.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index afc830cae9..49a1c117bc 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -516,10 +516,10 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase end def test_has_many_through_collection_size_uses_counter_cache_if_it_exists - author = authors(:david) - author.stubs(:_read_attribute).with('comments_count').returns(100) - assert_equal 100, author.comments.size - assert !author.comments.loaded? + c = categories(:general) + c.categorizations_count = 100 + assert_equal 100, c.categorizations.size + assert !c.categorizations.loaded? end def test_adding_junk_to_has_many_through_should_raise_type_mismatch -- cgit v1.2.3 From e01dfb27fc5573db9070dce788f2e5ee62094722 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Fri, 15 Apr 2011 13:09:12 +0100 Subject: Undo performances regressions I introduced in bbe0a507f287c20ab4ae8a244fbfc810665deda5 and add test for an edge case. Add comments to explain the intent of the code. --- .../lib/active_record/attribute_methods/read.rb | 28 ++++++++++++++++++---- activerecord/test/cases/attribute_methods_test.rb | 11 +++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index dd3f4699f7..cd6c4002d2 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -43,7 +43,7 @@ module ActiveRecord end if attr_name == primary_key && attr_name != "id" - define_read_method(:id, attr_name, columns_hash[attr_name]) + define_read_method('id', attr_name, columns_hash[attr_name]) end end @@ -59,7 +59,9 @@ module ActiveRecord end # Define an attribute reader method. Cope with nil column. - def define_read_method(symbol, attr_name, column) + # method_name is the same as attr_name except when a non-standard primary key is used, + # we still define #id as an accessor for the key + def define_read_method(method_name, attr_name, column) cast_code = column.type_cast_code('v') access_code = "(v=@attributes['#{attr_name}']) && #{cast_code}" @@ -71,9 +73,25 @@ module ActiveRecord access_code = "@attributes_cache['#{attr_name}'] ||= (#{access_code})" end - generated_attribute_methods.module_eval do - define_method("_#{attr_name}") { eval(access_code) } - alias_method(attr_name, "_#{attr_name}") + # Where possible, generate the method by evalling a string, as this will result in + # faster accesses because it avoids the block eval and then string eval incurred + # by the second branch. + # + # The second, slower, branch is necessary to support instances where the database + # returns columns with extra stuff in (like 'my_column(omg)'). + if method_name =~ /^[a-zA-Z_]\w*[!?=]?$/ + generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + def _#{method_name} + #{access_code} + end + + alias #{method_name} _#{method_name} + STR + else + generated_attribute_methods.module_eval do + define_method("_#{method_name}") { eval(access_code) } + alias_method(method_name, "_#{method_name}") + end end end end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 899166d129..5074ae50ab 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -10,6 +10,7 @@ require 'models/company' require 'models/category' require 'models/reply' require 'models/contact' +require 'models/keyboard' class AttributeMethodsTest < ActiveRecord::TestCase fixtures :topics, :developers, :companies, :computers @@ -89,6 +90,16 @@ class AttributeMethodsTest < ActiveRecord::TestCase assert !topic.respond_to?(:nothingness) end + def test_respond_to_with_custom_primary_key + keyboard = Keyboard.create + assert_not_nil keyboard.key_number + assert_equal keyboard.key_number, keyboard.id + assert keyboard.respond_to?('key_number') + assert keyboard.respond_to?('_key_number') + assert keyboard.respond_to?('id') + assert keyboard.respond_to?('_id') + end + # Syck calls respond_to? before actually calling initialize def test_respond_to_with_allocated_object topic = Topic.allocate -- cgit v1.2.3 From 65469a6e5e8cd2418c99c3862dd33feed69536bd Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Fri, 15 Apr 2011 13:27:08 +0100 Subject: Return nil from read_attribute(:foo) if 'foo' is not present in the @attributes hash, but the _foo method has been defined. This brings the behaviour into line with the 3-0-stable branch and the master branch before 93641ed6c8c684f6b4db02b6c8a22fa9bc7f0eaf (there were previously no assertions about this which is why the change slipped through). Note that actually calling the 'foo' method will still raise an error if the attribute is not present. --- activerecord/lib/active_record/attribute_methods/read.rb | 2 +- activerecord/test/cases/finder_test.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index cd6c4002d2..a248eb3a7b 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -100,7 +100,7 @@ module ActiveRecord # "2004-12-12" in a data column is cast to a date object, like Date.new(2004, 12, 12)). def read_attribute(attr_name) if respond_to? "_#{attr_name}" - send "_#{attr_name}" + send "_#{attr_name}" if @attributes.has_key?(attr_name.to_s) else _read_attribute attr_name end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 3c242667eb..655437318f 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -247,9 +247,10 @@ class FinderTest < ActiveRecord::TestCase def test_find_only_some_columns topic = Topic.find(1, :select => "author_name") assert_raise(ActiveModel::MissingAttributeError) {topic.title} + assert_nil topic.read_attribute("title") assert_equal "David", topic.author_name assert !topic.attribute_present?("title") - #assert !topic.respond_to?("title") + assert !topic.attribute_present?(:title) assert topic.attribute_present?("author_name") assert_respond_to topic, "author_name" end -- cgit v1.2.3