Skip to content
Browse files

Merge pull request #13757 from terracatta/master

Ensure #second (and others) acts like #first AR finder
  • Loading branch information...
2 parents 5a09f13 + cafe31a commit bc625080308e4853ae3036f2ad74fe3826e463ef @rafaelfranca rafaelfranca committed Jan 20, 2014
View
31 activerecord/CHANGELOG.md
@@ -1,3 +1,34 @@
+* Ensure `second` through `fifth` methods act like the `first` finder.
+
+ The famous ordinal Array instance methods defined in ActiveSupport
+ (`first`, `second`, `third`, `fourth`, and `fifth`) are now available as
+ full-fledged finders in ActiveRecord. The biggest benefit of this is ordering
+ of the records returned now defaults to the table's primary key in ascending order.
+
+ Example:
+
+ User.all.second
+
+ # Before
+ # => 'SELECT "users".* FROM "users"'
+
+ # After
+ # => SELECT "users".* FROM "users" ORDER BY "users"."id" ASC LIMIT 1 OFFSET 1'
+
+ User.offset(3).second
+
+ # Before
+ # => 'SELECT "users".* FROM "users" LIMIT -1 OFFSET 3' # sqlite3 gem
+ # => 'SELECT "users".* FROM "users" OFFSET 3' # pg gem
+ # => 'SELECT `users`.* FROM `users` LIMIT 18446744073709551615 OFFSET 3' # mysql2 gem
+
+ # After
+ # => SELECT "users".* FROM "users" ORDER BY "users"."id" ASC LIMIT 1 OFFSET 4'
+
+ Fixes #13743.
+
+ *Jason Meller*
+
* ActiveRecord states are now correctly restored after a rollback for
models that did not define any transactional callbacks (i.e.
`after_commit`, `after_rollback` or `after_create`).
View
1 activerecord/lib/active_record/querying.rb
@@ -1,6 +1,7 @@
module ActiveRecord
module Querying
delegate :find, :take, :take!, :first, :first!, :last, :last!, :exists?, :any?, :many?, to: :all
+ delegate :second, :second!, :third, :third!, :fourth, :fourth!, :fifth, :fifth!, :forty_two, :forty_two!, to: :all
delegate :first_or_create, :first_or_create!, :first_or_initialize, to: :all
delegate :find_or_create_by, :find_or_create_by!, :find_or_initialize_by, to: :all
delegate :find_by, :find_by!, to: :all
View
4 activerecord/lib/active_record/relation.rb
@@ -20,10 +20,11 @@ class Relation
alias :model :klass
alias :loaded? :loaded
- def initialize(klass, table, values = {})
+ def initialize(klass, table, values = {}, offsets = {})
@klass = klass
@table = table
@values = values
+ @offsets = offsets
@loaded = false
end
@@ -498,6 +499,7 @@ def reset
@first = @last = @to_sql = @order_clause = @scope_for_create = @arel = @loaded = nil
@should_eager_load = @join_dependency = nil
@records = []
+ @offsets = {}
self
end
View
94 activerecord/lib/active_record/relation/finder_methods.rb
@@ -127,9 +127,9 @@ def take!
#
def first(limit = nil)
if limit
- find_first_with_limit(limit)
+ find_nth_with_limit(offset_value, limit)
else
- find_first
+ find_nth(offset_value)
end
end
@@ -172,6 +172,86 @@ def last!
last or raise RecordNotFound
end
+ # Find the second record.
+ # If no order is defined it will order by primary key.
+ #
+ # Person.second # returns the second object fetched by SELECT * FROM people
+ # Person.offset(3).second # returns the second object from OFFSET 3 (which is OFFSET 4)
+ # Person.where(["user_name = :u", { u: user_name }]).second
+ def second
+ find_nth(offset_value ? offset_value + 1 : 1)
+ end
+
+ # Same as +second+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record
+ # is found.
+ def second!
+ second or raise RecordNotFound
+ end
+
+ # Find the third record.
+ # If no order is defined it will order by primary key.
+ #
+ # Person.third # returns the third object fetched by SELECT * FROM people
+ # Person.offset(3).third # returns the third object from OFFSET 3 (which is OFFSET 5)
+ # Person.where(["user_name = :u", { u: user_name }]).third
+ def third
+ find_nth(offset_value ? offset_value + 2 : 2)
+ end
+
+ # Same as +third+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record
+ # is found.
+ def third!
+ third or raise RecordNotFound
+ end
+
+ # Find the fourth record.
+ # If no order is defined it will order by primary key.
+ #
+ # Person.fourth # returns the fourth object fetched by SELECT * FROM people
+ # Person.offset(3).fourth # returns the fourth object from OFFSET 3 (which is OFFSET 6)
+ # Person.where(["user_name = :u", { u: user_name }]).fourth
+ def fourth
+ find_nth(offset_value ? offset_value + 3 : 3)
+ end
+
+ # Same as +fourth+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record
+ # is found.
+ def fourth!
+ fourth or raise RecordNotFound
+ end
+
+ # Find the fifth record.
+ # If no order is defined it will order by primary key.
+ #
+ # Person.fifth # returns the fifth object fetched by SELECT * FROM people
+ # Person.offset(3).fifth # returns the fifth object from OFFSET 3 (which is OFFSET 7)
+ # Person.where(["user_name = :u", { u: user_name }]).fifth
+ def fifth
+ find_nth(offset_value ? offset_value + 4 : 4)
+ end
+
+ # Same as +fifth+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record
+ # is found.
+ def fifth!
+ fifth or raise RecordNotFound
+ end
+
+ # Find the forty-second record. Also known as accessing "the reddit".
+ # If no order is defined it will order by primary key.
+ #
+ # Person.forty_two # returns the forty-second object fetched by SELECT * FROM people
+ # Person.offset(3).forty_two # returns the fifth object from OFFSET 3 (which is OFFSET 44)
+ # Person.where(["user_name = :u", { u: user_name }]).forty_two
+ def forty_two
+ find_nth(offset_value ? offset_value + 41 : 41)
+ end
+
+ # Same as +forty_two+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record
+ # is found.
+ def forty_two!
+ forty_two or raise RecordNotFound
+ end
+
# Returns +true+ if a record exists in the table that matches the +id+ or
# conditions given, or +false+ otherwise. The argument can take six forms:
#
@@ -364,19 +444,19 @@ def find_take
end
end
- def find_first
+ def find_nth(offset)
if loaded?
@records.first
else
- @first ||= find_first_with_limit(1).first
+ @offsets[offset] ||= find_nth_with_limit(offset, 1).first
end
end
- def find_first_with_limit(limit)
+ def find_nth_with_limit(offset, limit)
if order_values.empty? && primary_key
- order(arel_table[primary_key].asc).limit(limit).to_a
+ order(arel_table[primary_key].asc).limit(limit).offset(offset).to_a
else
- limit(limit).to_a
+ limit(limit).offset(offset).to_a
end
end
View
2 activerecord/test/cases/base_test.rb
@@ -321,7 +321,7 @@ def test_create_after_initialize_with_array_param
def test_load
topics = Topic.all.merge!(:order => 'id').to_a
- assert_equal(4, topics.size)
+ assert_equal(5, topics.size)
assert_equal(topics(:first).title, topics.first.title)
end
View
18 activerecord/test/cases/calculations_test.rb
@@ -466,14 +466,14 @@ def test_from_option_with_table_different_than_class
def test_distinct_is_honored_when_used_with_count_operation_after_group
# Count the number of authors for approved topics
approved_topics_count = Topic.group(:approved).count(:author_name)[true]
- assert_equal approved_topics_count, 3
+ assert_equal approved_topics_count, 4
# Count the number of distinct authors for approved Topics
distinct_authors_for_approved_count = Topic.group(:approved).distinct.count(:author_name)[true]
- assert_equal distinct_authors_for_approved_count, 2
+ assert_equal distinct_authors_for_approved_count, 3
end
def test_pluck
- assert_equal [1,2,3,4], Topic.order(:id).pluck(:id)
+ assert_equal [1,2,3,4,5], Topic.order(:id).pluck(:id)
end
def test_pluck_without_column_names
@@ -509,7 +509,7 @@ def test_pluck_with_serialization
end
def test_pluck_with_qualified_column_name
- assert_equal [1,2,3,4], Topic.order(:id).pluck("topics.id")
+ assert_equal [1,2,3,4,5], Topic.order(:id).pluck("topics.id")
end
def test_pluck_auto_table_name_prefix
@@ -557,11 +557,13 @@ def test_pluck_not_auto_table_name_prefix_if_column_included
def test_pluck_multiple_columns
assert_equal [
[1, "The First Topic"], [2, "The Second Topic of the day"],
- [3, "The Third Topic of the day"], [4, "The Fourth Topic of the day"]
+ [3, "The Third Topic of the day"], [4, "The Fourth Topic of the day"],
+ [5, "The Fifth Topic of the day"]
], Topic.order(:id).pluck(:id, :title)
assert_equal [
[1, "The First Topic", "David"], [2, "The Second Topic of the day", "Mary"],
- [3, "The Third Topic of the day", "Carl"], [4, "The Fourth Topic of the day", "Carl"]
+ [3, "The Third Topic of the day", "Carl"], [4, "The Fourth Topic of the day", "Carl"],
+ [5, "The Fifth Topic of the day", "Jason"]
], Topic.order(:id).pluck(:id, :title, :author_name)
end
@@ -587,7 +589,7 @@ def test_pluck_with_reserved_words
def test_pluck_replaces_select_clause
taks_relation = Topic.select(:approved, :id).order(:id)
- assert_equal [1,2,3,4], taks_relation.pluck(:id)
- assert_equal [false, true, true, true], taks_relation.pluck(:approved)
+ assert_equal [1,2,3,4,5], taks_relation.pluck(:id)
+ assert_equal [false, true, true, true, true], taks_relation.pluck(:approved)
end
end
View
90 activerecord/test/cases/finder_test.rb
@@ -254,6 +254,94 @@ def test_model_class_responds_to_first_bang
end
end
+ def test_second
+ assert_equal topics(:second).title, Topic.second.title
+ end
+
+ def test_second_with_offset
+ assert_equal topics(:fifth), Topic.offset(3).second
+ end
+
+ def test_second_have_primary_key_order_by_default
+ expected = topics(:second)
+ expected.touch # PostgreSQL changes the default order if no order clause is used
+ assert_equal expected, Topic.second
+ end
+
+ def test_model_class_responds_to_second_bang
+ assert Topic.second!
+ Topic.delete_all
+ assert_raises ActiveRecord::RecordNotFound do
+ Topic.second!
+ end
+ end
+
+ def test_third
+ assert_equal topics(:third).title, Topic.third.title
+ end
+
+ def test_third_with_offset
+ assert_equal topics(:fifth), Topic.offset(2).third
+ end
+
+ def test_third_have_primary_key_order_by_default
+ expected = topics(:third)
+ expected.touch # PostgreSQL changes the default order if no order clause is used
+ assert_equal expected, Topic.third
+ end
+
+ def test_model_class_responds_to_third_bang
+ assert Topic.third!
+ Topic.delete_all
+ assert_raises ActiveRecord::RecordNotFound do
+ Topic.third!
+ end
+ end
+
+ def test_fourth
+ assert_equal topics(:fourth).title, Topic.fourth.title
+ end
+
+ def test_fourth_with_offset
+ assert_equal topics(:fifth), Topic.offset(1).fourth
+ end
+
+ def test_fourth_have_primary_key_order_by_default
+ expected = topics(:fourth)
+ expected.touch # PostgreSQL changes the default order if no order clause is used
+ assert_equal expected, Topic.fourth
+ end
+
+ def test_model_class_responds_to_fourth_bang
+ assert Topic.fourth!
+ Topic.delete_all
+ assert_raises ActiveRecord::RecordNotFound do
+ Topic.fourth!
+ end
+ end
+
+ def test_fifth
+ assert_equal topics(:fifth).title, Topic.fifth.title
+ end
+
+ def test_fifth_with_offset
+ assert_equal topics(:fifth), Topic.offset(0).fifth
+ end
+
+ def test_fifth_have_primary_key_order_by_default
+ expected = topics(:fifth)
+ expected.touch # PostgreSQL changes the default order if no order clause is used
+ assert_equal expected, Topic.fifth
+ end
+
+ def test_model_class_responds_to_fifth_bang
+ assert Topic.fifth!
+ Topic.delete_all
+ assert_raises ActiveRecord::RecordNotFound do
+ Topic.fifth!
+ end
+ end
+
def test_last_bang_present
assert_nothing_raised do
assert_equal topics(:second), Topic.where("title = 'The Second Topic of the day'").last!
@@ -267,7 +355,7 @@ def test_last_bang_missing
end
def test_model_class_responds_to_last_bang
- assert_equal topics(:fourth), Topic.last!
+ assert_equal topics(:fifth), Topic.last!
assert_raises ActiveRecord::RecordNotFound do
Topic.delete_all
Topic.last!
View
24 activerecord/test/cases/relations_test.rb
@@ -65,7 +65,7 @@ def test_multivalue_where
def test_scoped
topics = Topic.all
assert_kind_of ActiveRecord::Relation, topics
- assert_equal 4, topics.size
+ assert_equal 5, topics.size
end
def test_to_json
@@ -86,14 +86,14 @@ def test_to_xml
def test_scoped_all
topics = Topic.all.to_a
assert_kind_of Array, topics
- assert_no_queries { assert_equal 4, topics.size }
+ assert_no_queries { assert_equal 5, topics.size }
end
def test_loaded_all
topics = Topic.all
assert_queries(1) do
- 2.times { assert_equal 4, topics.to_a.size }
+ 2.times { assert_equal 5, topics.to_a.size }
end
assert topics.loaded?
@@ -164,27 +164,27 @@ def test_finding_with_conditions
def test_finding_with_order
topics = Topic.order('id')
- assert_equal 4, topics.to_a.size
+ assert_equal 5, topics.to_a.size
assert_equal topics(:first).title, topics.first.title
end
def test_finding_with_arel_order
topics = Topic.order(Topic.arel_table[:id].asc)
- assert_equal 4, topics.to_a.size
+ assert_equal 5, topics.to_a.size
assert_equal topics(:first).title, topics.first.title
end
def test_finding_with_assoc_order
topics = Topic.order(:id => :desc)
- assert_equal 4, topics.to_a.size
- assert_equal topics(:fourth).title, topics.first.title
+ assert_equal 5, topics.to_a.size
+ assert_equal topics(:fifth).title, topics.first.title
end
def test_finding_with_reverted_assoc_order
topics = Topic.order(:id => :asc).reverse_order
- assert_equal 4, topics.to_a.size
- assert_equal topics(:fourth).title, topics.first.title
+ assert_equal 5, topics.to_a.size
+ assert_equal topics(:fifth).title, topics.first.title
end
def test_order_with_hash_and_symbol_generates_the_same_sql
@@ -197,19 +197,19 @@ def test_raising_exception_on_invalid_hash_params
def test_finding_last_with_arel_order
topics = Topic.order(Topic.arel_table[:id].asc)
- assert_equal topics(:fourth).title, topics.last.title
+ assert_equal topics(:fifth).title, topics.last.title
end
def test_finding_with_order_concatenated
topics = Topic.order('author_name').order('title')
- assert_equal 4, topics.to_a.size
+ assert_equal 5, topics.to_a.size
assert_equal topics(:fourth).title, topics.first.title
end
def test_finding_with_reorder
topics = Topic.order('author_name').order('title').reorder('id').to_a
topics_titles = topics.map{ |t| t.title }
- assert_equal ['The First Topic', 'The Second Topic of the day', 'The Third Topic of the day', 'The Fourth Topic of the day'], topics_titles
+ assert_equal ['The First Topic', 'The Second Topic of the day', 'The Third Topic of the day', 'The Fourth Topic of the day', 'The Fifth Topic of the day'], topics_titles
end
def test_finding_with_order_and_take
View
8 activerecord/test/cases/scoping/named_scoping_test.rb
@@ -344,13 +344,13 @@ def test_chaining_combines_conditions_when_searching
end
def test_scopes_batch_finders
- assert_equal 3, Topic.approved.count
+ assert_equal 4, Topic.approved.count
- assert_queries(4) do
+ assert_queries(5) do
Topic.approved.find_each(:batch_size => 1) {|t| assert t.approved? }
end
- assert_queries(2) do
+ assert_queries(3) do
Topic.approved.find_in_batches(:batch_size => 2) do |group|
group.each {|t| assert t.approved? }
end
@@ -366,7 +366,7 @@ def test_table_names_for_chaining_scopes_with_and_without_table_name_included
def test_scopes_on_relations
# Topic.replied
approved_topics = Topic.all.approved.order('id DESC')
- assert_equal topics(:fourth), approved_topics.first
+ assert_equal topics(:fifth), approved_topics.first
replied_approved_topics = approved_topics.replied
assert_equal topics(:third), replied_approved_topics.first
View
7 activerecord/test/fixtures/topics.yml
@@ -40,3 +40,10 @@ fourth:
type: Reply
parent_id: 3
+fifth:
+ id: 5
+ title: The Fifth Topic of the day
+ author_name: Jason
+ written_on: 2013-07-13t12:11:00.0099+01:00
+ content: Omakase
+ approved: true

0 comments on commit bc62508

Please sign in to comment.
Something went wrong with that request. Please try again.