Skip to content

Instantly share code, notes, and snippets.

@AquaGeek
Created May 14, 2011 02:17
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save AquaGeek/971642 to your computer and use it in GitHub Desktop.
Save AquaGeek/971642 to your computer and use it in GitHub Desktop.
Rails Lighthouse ticket #3165
From ccc8aee0aeac0213cf15abd67ed5dab577899007 Mon Sep 17 00:00:00 2001
From: Rob Olson <rob@thinkingdigitally.com>
Date: Fri, 23 Oct 2009 19:20:48 -0700
Subject: [PATCH 1/2] Add option to skip record initialization for sql queries.
This fixes an exception caused by queries generated by #exists?
when there is an after_initialize which accesses an attribute
on the record.
---
activerecord/lib/active_record/base.rb | 19 ++++++++++---------
activerecord/test/cases/finder_test.rb | 4 ++++
activerecord/test/models/entrant.rb | 4 ++++
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 2ec2f73..13cd762 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -657,8 +657,8 @@ module ActiveRecord #:nodoc:
# # You can use the same string replacement techniques as you can with ActiveRecord#find
# Post.find_by_sql ["SELECT title FROM posts WHERE author = ? AND created > ?", author_id, start_date]
# > [#<Post:0x36bff9c @attributes={"first_name"=>"The Cheap Man Buys Twice"}>, ...]
- def find_by_sql(sql)
- connection.select_all(sanitize_sql(sql), "#{name} Load").collect! { |record| instantiate(record) }
+ def find_by_sql(sql, skip_instantiation = false)
+ connection.select_all(sanitize_sql(sql), "#{name} Load").collect! { |record| skip_instantiation ? record : instantiate(record) }
end
# Returns true if a record exists in the table that matches the +id+ or
@@ -687,9 +687,10 @@ module ActiveRecord #:nodoc:
# Person.exists?(['name LIKE ?', "%#{query}%"])
# Person.exists?
def exists?(id_or_conditions = {})
- find_initial(
+ find_initial({
:select => "#{quoted_table_name}.#{primary_key}",
- :conditions => expand_id_conditions(id_or_conditions)) ? true : false
+ :conditions => expand_id_conditions(id_or_conditions)},
+ :skip_instantiation) ? true : false
end
# Creates an object (or multiple objects) and saves it to the database, if validations pass.
@@ -1500,9 +1501,9 @@ module ActiveRecord #:nodoc:
end
private
- def find_initial(options)
+ def find_initial(options, skip_instantiation = false)
options.update(:limit => 1)
- find_every(options).first
+ find_every(options, skip_instantiation).first
end
def find_last(options)
@@ -1539,14 +1540,14 @@ module ActiveRecord #:nodoc:
}.join(',')
end
- def find_every(options)
+ def find_every(options, skip_instantiation = false)
include_associations = merge_includes(scope(:find, :include), options[:include])
if include_associations.any? && references_eager_loaded_tables?(options)
records = find_with_associations(options)
else
- records = find_by_sql(construct_finder_sql(options))
- if include_associations.any?
+ records = find_by_sql(construct_finder_sql(options), skip_instantiation)
+ if include_associations.any? && !skip_instantiation
preload_associations(records, include_associations)
end
end
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index f1bede9..6e9f759 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -125,6 +125,10 @@ class FinderTest < ActiveRecord::TestCase
end
end
+ def test_exists_on_model_with_after_initialize_method_should_not_blow_up
+ assert Entrant.exists?
+ end
+
def test_find_by_array_of_one_id
assert_kind_of(Array, Topic.find([ 1 ]))
assert_equal(1, Topic.find([ 1 ]).length)
diff --git a/activerecord/test/models/entrant.rb b/activerecord/test/models/entrant.rb
index 4682ce4..dfec275 100644
--- a/activerecord/test/models/entrant.rb
+++ b/activerecord/test/models/entrant.rb
@@ -1,3 +1,7 @@
class Entrant < ActiveRecord::Base
belongs_to :course
+
+ after_initialize do
+ self.name ||= ""
+ end
end
--
1.6.3.3
From cdc43f185161bda7abf2385957be9bb2cf1e4ea2 Mon Sep 17 00:00:00 2001
From: Rob Olson <rob@thinkingdigitally.com>
Date: Mon, 9 Nov 2009 19:31:53 -0800
Subject: [PATCH 2/2] updated test_exists_on_model_with_after_initialize_method_should_not_blow_up to fail properly
---
activerecord/test/cases/finder_test.rb | 2 +-
activerecord/test/models/entrant.rb | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index 6e9f759..fd43065 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -126,7 +126,7 @@ class FinderTest < ActiveRecord::TestCase
end
def test_exists_on_model_with_after_initialize_method_should_not_blow_up
- assert Entrant.exists?
+ assert_nothing_raised { assert Entrant.exists? }
end
def test_find_by_array_of_one_id
diff --git a/activerecord/test/models/entrant.rb b/activerecord/test/models/entrant.rb
index dfec275..1d96617 100644
--- a/activerecord/test/models/entrant.rb
+++ b/activerecord/test/models/entrant.rb
@@ -1,7 +1,7 @@
class Entrant < ActiveRecord::Base
belongs_to :course
- after_initialize do
+ def after_initialize
self.name ||= ""
end
end
--
1.6.3.3
Index: activerecord/lib/active_record/base.rb
===================================================================
--- a/rails/activerecord/lib/active_record/base.rb
+++ b/rails/activerecord/lib/active_record/base.rb
@@ -689,5 +689,4 @@
def exists?(id_or_conditions = {})
find_initial(
- :select => "#{quoted_table_name}.#{primary_key}",
:conditions => expand_id_conditions(id_or_conditions)) ? true : false
end
From 81904d95af9f6c7922d54ab43ed4147433172e86 Mon Sep 17 00:00:00 2001
From: Rob Olson <rob@thinkingdigitally.com>
Date: Fri, 23 Oct 2009 19:20:48 -0700
Subject: [PATCH] Add option to skip record initialization for sql queries.
This fixes an exception caused by queries generated by #exists?
when there is an after_initialize which accesses an attribute
on the record.
---
activerecord/lib/active_record/base.rb | 19 ++++++++++---------
activerecord/test/cases/finder_test.rb | 4 ++++
activerecord/test/models/entrant.rb | 4 ++++
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 4e60904..831f0bb 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -707,8 +707,8 @@ module ActiveRecord #:nodoc:
# # You can use the same string replacement techniques as you can with ActiveRecord#find
# Post.find_by_sql ["SELECT title FROM posts WHERE author = ? AND created > ?", author_id, start_date]
# > [#<Post:0x36bff9c @attributes={"first_name"=>"The Cheap Man Buys Twice"}>, ...]
- def find_by_sql(sql)
- connection.select_all(sanitize_sql(sql), "#{name} Load").collect! { |record| instantiate(record) }
+ def find_by_sql(sql, skip_instantiation = false)
+ connection.select_all(sanitize_sql(sql), "#{name} Load").collect! { |record| skip_instantiation ? record : instantiate(record) }
end
# Returns true if a record exists in the table that matches the +id+ or
@@ -737,9 +737,10 @@ module ActiveRecord #:nodoc:
# Person.exists?(['name LIKE ?', "%#{query}%"])
# Person.exists?
def exists?(id_or_conditions = {})
- find_initial(
+ find_initial({
:select => "#{quoted_table_name}.#{primary_key}",
- :conditions => expand_id_conditions(id_or_conditions)) ? true : false
+ :conditions => expand_id_conditions(id_or_conditions)},
+ :skip_instantiation) ? true : false
end
# Creates an object (or multiple objects) and saves it to the database, if validations pass.
@@ -1502,9 +1503,9 @@ module ActiveRecord #:nodoc:
end
private
- def find_initial(options)
+ def find_initial(options, skip_instantiation = false)
options.update(:limit => 1)
- find_every(options).first
+ find_every(options, skip_instantiation).first
end
def find_last(options)
@@ -1541,14 +1542,14 @@ module ActiveRecord #:nodoc:
}.join(',')
end
- def find_every(options)
+ def find_every(options, skip_instantiation = false)
include_associations = merge_includes(scope(:find, :include), options[:include])
if include_associations.any? && references_eager_loaded_tables?(options)
records = find_with_associations(options)
else
- records = find_by_sql(construct_finder_sql(options))
- if include_associations.any?
+ records = find_by_sql(construct_finder_sql(options), skip_instantiation)
+ if include_associations.any? && !skip_instantiation
preload_associations(records, include_associations)
end
end
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index 3de0779..3fdab03 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -125,6 +125,10 @@ class FinderTest < ActiveRecord::TestCase
end
end
+ def test_exists_on_model_with_after_initialize_method_should_not_blow_up
+ assert Entrant.exists?
+ end
+
def test_find_by_array_of_one_id
assert_kind_of(Array, Topic.find([ 1 ]))
assert_equal(1, Topic.find([ 1 ]).length)
diff --git a/activerecord/test/models/entrant.rb b/activerecord/test/models/entrant.rb
index 4682ce4..dfec275 100644
--- a/activerecord/test/models/entrant.rb
+++ b/activerecord/test/models/entrant.rb
@@ -1,3 +1,7 @@
class Entrant < ActiveRecord::Base
belongs_to :course
+
+ after_initialize do
+ self.name ||= ""
+ end
end
--
1.6.3.3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment