Skip to content

Commit adfab2d

Browse files
committed
deprecate Relation#uniq use Relation#distinct instead.
See #9683 for the reasons we switched to `distinct`. Here is the discussion that triggered the actual deprecation #20198. `uniq`, `uniq!` and `uniq_value` are still around. They will be removed in the next minor release after Rails 5.
1 parent b8c31fd commit adfab2d

File tree

12 files changed

+52
-31
lines changed

12 files changed

+52
-31
lines changed

activerecord/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
* Deprecate `Relation#uniq` use `Relation#distinct` instead.
2+
3+
See #9683.
4+
5+
*Yves Senn*
6+
17
* Allow single table inheritance instantiation to work when storing
28
demodulized class names.
39

activerecord/lib/active_record/associations.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,6 @@ def association_instance_set(name, association)
266266
# others.find(*args) | X | X | X
267267
# others.exists? | X | X | X
268268
# others.distinct | X | X | X
269-
# others.uniq | X | X | X
270269
# others.reset | X | X | X
271270
#
272271
# === Overriding generated methods

activerecord/lib/active_record/relation.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class Relation
99
:extending, :unscope]
1010

1111
SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :reordering,
12-
:reverse_order, :distinct, :create_with, :uniq]
12+
:reverse_order, :distinct, :create_with]
1313
CLAUSE_METHODS = [:where, :having, :from]
1414
INVALID_METHODS_FOR_DELETE_ALL = [:limit, :distinct, :offset, :group, :having]
1515

@@ -618,6 +618,7 @@ def joined_includes_values
618618
def uniq_value
619619
distinct_value
620620
end
621+
deprecate uniq_value: :distinct_value
621622

622623
# Compares two relations for equality.
623624
def ==(other)

activerecord/lib/active_record/relation/query_methods.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,7 @@ def rewhere(conditions)
587587
#
588588
# The two relations must be structurally compatible: they must be scoping the same model, and
589589
# they must differ only by +where+ (if no +group+ has been defined) or +having+ (if a +group+ is
590-
# present). Neither relation may have a +limit+, +offset+, or +uniq+ set.
590+
# present). Neither relation may have a +limit+, +offset+, or +distinct+ set.
591591
#
592592
# Post.where("id = 1").or(Post.where("id = 2"))
593593
# # SELECT `posts`.* FROM `posts` WHERE (('id = 1' OR 'id = 2'))
@@ -790,13 +790,15 @@ def distinct(value = true)
790790
spawn.distinct!(value)
791791
end
792792
alias uniq distinct
793+
deprecate uniq: :distinct
793794

794795
# Like #distinct, but modifies relation in place.
795796
def distinct!(value = true) # :nodoc:
796797
self.distinct_value = value
797798
self
798799
end
799800
alias uniq! distinct!
801+
deprecate uniq!: :distinct!
800802

801803
# Used to extend a scope with additional methods, either through
802804
# a module or through a block provided.

activerecord/test/cases/associations/eager_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1167,7 +1167,7 @@ def test_preloading_through_empty_belongs_to
11671167
assert_no_queries { assert client.accounts.empty? }
11681168
end
11691169

1170-
def test_preloading_has_many_through_with_uniq
1170+
def test_preloading_has_many_through_with_distinct
11711171
mary = Author.includes(:unique_categorized_posts).where(:id => authors(:mary).id).first
11721172
assert_equal 1, mary.unique_categorized_posts.length
11731173
assert_equal 1, mary.unique_categorized_post_ids.length

activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ def test_habtm_saving_multiple_relationships
234234
assert_equal developers, new_project.developers
235235
end
236236

237-
def test_habtm_unique_order_preserved
237+
def test_habtm_distinct_order_preserved
238238
assert_equal developers(:poor_jamis, :jamis, :david), projects(:active_record).non_unique_developers
239239
assert_equal developers(:poor_jamis, :jamis, :david), projects(:active_record).developers
240240
end
@@ -339,7 +339,7 @@ def test_creation_respects_hash_condition
339339
assert_equal 'Yet Another Testing Title', another_post.title
340340
end
341341

342-
def test_uniq_after_the_fact
342+
def test_distinct_after_the_fact
343343
dev = developers(:jamis)
344344
dev.projects << projects(:active_record)
345345
dev.projects << projects(:active_record)
@@ -348,13 +348,13 @@ def test_uniq_after_the_fact
348348
assert_equal 1, dev.projects.distinct.size
349349
end
350350

351-
def test_uniq_before_the_fact
351+
def test_distinct_before_the_fact
352352
projects(:active_record).developers << developers(:jamis)
353353
projects(:active_record).developers << developers(:david)
354354
assert_equal 3, projects(:active_record, :reload).developers.size
355355
end
356356

357-
def test_uniq_option_prevents_duplicate_push
357+
def test_distinct_option_prevents_duplicate_push
358358
project = projects(:active_record)
359359
project.developers << developers(:jamis)
360360
project.developers << developers(:david)
@@ -365,7 +365,7 @@ def test_uniq_option_prevents_duplicate_push
365365
assert_equal 3, project.developers.size
366366
end
367367

368-
def test_uniq_when_association_already_loaded
368+
def test_distinct_when_association_already_loaded
369369
project = projects(:active_record)
370370
project.developers << [ developers(:jamis), developers(:david), developers(:jamis), developers(:david) ]
371371
assert_equal 3, Project.includes(:developers).find(project.id).developers.size

activerecord/test/cases/associations/has_many_through_associations_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -960,7 +960,7 @@ def test_create_has_many_through_with_default_scope_on_join_model
960960
assert_equal 1, category.categorizations.where(:special => true).count
961961
end
962962

963-
def test_joining_has_many_through_with_uniq
963+
def test_joining_has_many_through_with_distinct
964964
mary = Author.joins(:unique_categorized_posts).where(:id => authors(:mary).id).first
965965
assert_equal 1, mary.unique_categorized_posts.length
966966
assert_equal 1, mary.unique_categorized_post_ids.length

activerecord/test/cases/associations/join_model_test.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,12 @@ def test_inherited_has_many
3535
assert categories(:sti_test).authors.include?(authors(:mary))
3636
end
3737

38-
def test_has_many_uniq_through_join_model
38+
def test_has_many_distinct_through_join_model
3939
assert_equal 2, authors(:mary).categorized_posts.size
4040
assert_equal 1, authors(:mary).unique_categorized_posts.size
4141
end
4242

43-
def test_has_many_uniq_through_count
43+
def test_has_many_distinct_through_count
4444
author = authors(:mary)
4545
assert !authors(:mary).unique_categorized_posts.loaded?
4646
assert_queries(1) { assert_equal 1, author.unique_categorized_posts.count }
@@ -49,7 +49,7 @@ def test_has_many_uniq_through_count
4949
assert !authors(:mary).unique_categorized_posts.loaded?
5050
end
5151

52-
def test_has_many_uniq_through_find
52+
def test_has_many_distinct_through_find
5353
assert_equal 1, authors(:mary).unique_categorized_posts.to_a.size
5454
end
5555

@@ -625,7 +625,7 @@ def test_has_many_through_has_many_with_sti
625625
assert_equal [comments(:does_it_hurt)], authors(:david).special_post_comments
626626
end
627627

628-
def test_uniq_has_many_through_should_retain_order
628+
def test_distinct_has_many_through_should_retain_order
629629
comment_ids = authors(:david).comments.map(&:id)
630630
assert_equal comment_ids.sort, authors(:david).ordered_uniq_comments.map(&:id)
631631
assert_equal comment_ids.sort.reverse, authors(:david).ordered_uniq_comments_desc.map(&:id)

activerecord/test/cases/base_test.rb

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1411,15 +1411,13 @@ def test_touch_should_raise_error_on_a_new_object
14111411
end
14121412

14131413
def test_uniq_delegates_to_scoped
1414-
scope = stub
1415-
Bird.stubs(:all).returns(mock(:uniq => scope))
1416-
assert_equal scope, Bird.uniq
1414+
assert_deprecated do
1415+
assert_equal Bird.all.distinct, Bird.uniq
1416+
end
14171417
end
14181418

14191419
def test_distinct_delegates_to_scoped
1420-
scope = stub
1421-
Bird.stubs(:all).returns(mock(:distinct => scope))
1422-
assert_equal scope, Bird.distinct
1420+
assert_equal Bird.all.distinct, Bird.distinct
14231421
end
14241422

14251423
def test_table_name_with_2_abstract_subclasses

activerecord/test/cases/calculations_test.rb

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,10 @@ def test_count_with_column_parameter
359359

360360
def test_count_with_distinct
361361
assert_equal 4, Account.select(:credit_limit).distinct.count
362-
assert_equal 4, Account.select(:credit_limit).uniq.count
362+
363+
assert_deprecated do
364+
assert_equal 4, Account.select(:credit_limit).uniq.count
365+
end
363366
end
364367

365368
def test_count_with_aliased_attribute
@@ -504,8 +507,8 @@ def test_pluck_type_cast
504507
assert_equal [ topic.written_on ], relation.pluck(:written_on)
505508
end
506509

507-
def test_pluck_and_uniq
508-
assert_equal [50, 53, 55, 60], Account.order(:credit_limit).uniq.pluck(:credit_limit)
510+
def test_pluck_and_distinct
511+
assert_equal [50, 53, 55, 60], Account.order(:credit_limit).distinct.pluck(:credit_limit)
509512
end
510513

511514
def test_pluck_in_relation

activerecord/test/cases/relation/mutation_test.rb

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ def relation
8181
assert_equal [], relation.extending_values
8282
end
8383

84-
(Relation::SINGLE_VALUE_METHODS - [:lock, :reordering, :reverse_order, :create_with]).each do |method|
84+
(Relation::SINGLE_VALUE_METHODS - [:lock, :reordering, :reverse_order, :create_with, :uniq]).each do |method|
8585
test "##{method}!" do
8686
assert relation.public_send("#{method}!", :foo).equal?(relation)
8787
assert_equal :foo, relation.public_send("#{method}_value")
@@ -153,13 +153,22 @@ def relation
153153
test 'distinct!' do
154154
relation.distinct! :foo
155155
assert_equal :foo, relation.distinct_value
156-
assert_equal :foo, relation.uniq_value # deprecated access
156+
157+
assert_deprecated do
158+
assert_equal :foo, relation.uniq_value # deprecated access
159+
end
157160
end
158161

159162
test 'uniq! was replaced by distinct!' do
160-
relation.uniq! :foo
163+
assert_deprecated(/use distinct! instead/) do
164+
relation.uniq! :foo
165+
end
166+
167+
e = assert_deprecated(/use distinct_value instead/) do
168+
assert_equal :foo, relation.uniq_value # deprecated access
169+
end
170+
161171
assert_equal :foo, relation.distinct_value
162-
assert_equal :foo, relation.uniq_value # deprecated access
163172
end
164173
end
165174
end

activerecord/test/cases/relations_test.rb

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -908,7 +908,7 @@ def test_delete_all_loaded
908908

909909
def test_delete_all_with_unpermitted_relation_raises_error
910910
assert_raises(ActiveRecord::ActiveRecordError) { Author.limit(10).delete_all }
911-
assert_raises(ActiveRecord::ActiveRecordError) { Author.uniq.delete_all }
911+
assert_raises(ActiveRecord::ActiveRecordError) { Author.distinct.delete_all }
912912
assert_raises(ActiveRecord::ActiveRecordError) { Author.group(:name).delete_all }
913913
assert_raises(ActiveRecord::ActiveRecordError) { Author.having('SUM(id) < 3').delete_all }
914914
assert_raises(ActiveRecord::ActiveRecordError) { Author.offset(10).delete_all }
@@ -1493,14 +1493,17 @@ def test_distinct
14931493
assert_equal ['Foo', 'Foo'], query.map(&:name)
14941494
assert_sql(/DISTINCT/) do
14951495
assert_equal ['Foo'], query.distinct.map(&:name)
1496-
assert_equal ['Foo'], query.uniq.map(&:name)
1496+
assert_deprecated { assert_equal ['Foo'], query.uniq.map(&:name) }
14971497
end
14981498
assert_sql(/DISTINCT/) do
14991499
assert_equal ['Foo'], query.distinct(true).map(&:name)
1500-
assert_equal ['Foo'], query.uniq(true).map(&:name)
1500+
assert_deprecated { assert_equal ['Foo'], query.uniq(true).map(&:name) }
15011501
end
15021502
assert_equal ['Foo', 'Foo'], query.distinct(true).distinct(false).map(&:name)
1503-
assert_equal ['Foo', 'Foo'], query.uniq(true).uniq(false).map(&:name)
1503+
1504+
assert_deprecated do
1505+
assert_equal ['Foo', 'Foo'], query.uniq(true).uniq(false).map(&:name)
1506+
end
15041507
end
15051508

15061509
def test_doesnt_add_having_values_if_options_are_blank

0 commit comments

Comments
 (0)