Skip to content

Commit

Permalink
Warn about changing query_constraints: behavior
Browse files Browse the repository at this point in the history
This commit adds a deprecation warning for the `query_constraints:`
association option. This option will change behavior in the future versions
of Rails and applications are encouraged to switch to `foreign_key:` to preserve the
current behavior.
  • Loading branch information
nvasilevski committed Apr 17, 2024
1 parent 6d3fd5b commit c96b4c7
Show file tree
Hide file tree
Showing 16 changed files with 94 additions and 30 deletions.
6 changes: 6 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
* Association option `query_constraints` is deprecated in favor of `foreign_key`.

Applications are advised to switch to `foreign_key:` to maintain current behavior.

*Nikita Vasilevsky*

* Allow `ActiveRecord::Base#pluck` to accept hash values

```ruby
Expand Down
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/fixtures.rb
Expand Up @@ -498,8 +498,8 @@ class FixtureClassNotFound < ActiveRecord::ActiveRecordError # :nodoc:
# # app/models/book_orders.rb
# class BookOrder < ApplicationRecord
# self.primary_key = [:shop_id, :id]
# belongs_to :order, query_constraints: [:shop_id, :order_id]
# belongs_to :book, query_constraints: [:author_id, :book_id]
# belongs_to :order, foreign_key: [:shop_id, :order_id]
# belongs_to :book, foreign_key: [:author_id, :book_id]
# end
#
# <code></code>
Expand Down
7 changes: 7 additions & 0 deletions activerecord/lib/active_record/reflection.rb
Expand Up @@ -516,6 +516,13 @@ def initialize(name, scope, options, active_record)
@foreign_key = nil
@association_foreign_key = nil
@association_primary_key = nil
if options[:query_constraints]
ActiveRecord.deprecator.warn <<~MSG.squish
Setting `query_constraints:` option on `#{active_record}.#{macro} :#{name}` is deprecated.
To maintain current behavior, use the `foreign_key` option instead.
MSG
end

# If the foreign key is an array, set query constraints options and don't use the foreign key
if options[:foreign_key].is_a?(Array)
options[:query_constraints] = options.delete(:foreign_key)
Expand Down
22 changes: 22 additions & 0 deletions activerecord/test/cases/associations_test.rb
Expand Up @@ -461,6 +461,28 @@ def test_nullify_composite_has_many_through_association
assert_empty(blog_post.reload.tags)
assert_not_predicate Sharded::BlogPostTag.where(blog_post_id: blog_post.id, blog_id: blog_post.blog_id), :exists?
end

def test_using_query_constraints_warns_about_changing_behavior
has_many_expected_message = <<~MSG.squish
Setting `query_constraints:` option on `Sharded::BlogPost.has_many :qc_deprecated_comments` is deprecated.
To maintain current behavior, use the `foreign_key` option instead.
MSG

assert_deprecated(has_many_expected_message, ActiveRecord.deprecator) do
Sharded::BlogPost.has_many :qc_deprecated_comments,
class_name: "Sharded::Comment", query_constraints: [:blog_id, :blog_post_id]
end

belongs_to_expected_message = <<~MSG.squish
Setting `query_constraints:` option on `Sharded::Comment.belongs_to :qc_deprecated_blog_post` is deprecated.
To maintain current behavior, use the `foreign_key` option instead.
MSG

assert_deprecated(belongs_to_expected_message, ActiveRecord.deprecator) do
Sharded::Comment.belongs_to :qc_deprecated_blog_post,
class_name: "Sharded::Blog", query_constraints: [:blog_id, :blog_post_id]
end
end
end

class AssociationProxyTest < ActiveRecord::TestCase
Expand Down
29 changes: 29 additions & 0 deletions activerecord/test/cases/reflection_test.rb
Expand Up @@ -628,6 +628,35 @@ def test_belongs_to_reflection_with_query_constraints_infers_correct_foreign_key
assert_equal ["blog_id", "blog_post_id"], blog_post_foreign_key
end

def test_using_query_constraints_warns_about_changing_behavior
has_many_expected_message = <<~MSG.squish
Setting `query_constraints:` option on `Firm.has_many :clients` is deprecated.
To maintain current behavior, use the `foreign_key` option instead.
MSG

assert_deprecated(has_many_expected_message, ActiveRecord.deprecator) do
ActiveRecord::Reflection.create(:has_many, :clients, nil, { query_constraints: [:firm_id, :firm_name] }, Firm)
end

has_one_expected_message = <<~MSG.squish
Setting `query_constraints:` option on `Firm.has_one :account` is deprecated.
To maintain current behavior, use the `foreign_key` option instead.
MSG

assert_deprecated(has_one_expected_message, ActiveRecord.deprecator) do
ActiveRecord::Reflection.create(:has_one, :account, nil, { query_constraints: [:firm_id, :firm_name] }, Firm)
end

belongs_to_expected_message = <<~MSG.squish
Setting `query_constraints:` option on `Firm.belongs_to :client` is deprecated.
To maintain current behavior, use the `foreign_key` option instead.
MSG

assert_deprecated(belongs_to_expected_message, ActiveRecord.deprecator) do
ActiveRecord::Reflection.create(:belongs_to, :client, nil, { query_constraints: [:firm_id, :firm_name] }, Firm)
end
end

private
def assert_reflection(klass, association, options)
assert reflection = klass.reflect_on_association(association)
Expand Down
12 changes: 6 additions & 6 deletions activerecord/test/models/cpk/book.rb
Expand Up @@ -5,11 +5,11 @@ class Book < ActiveRecord::Base
attr_accessor :fail_destroy

self.table_name = :cpk_books
belongs_to :order, autosave: true, query_constraints: [:shop_id, :order_id], counter_cache: true
belongs_to :order_explicit_fk_pk, class_name: "Cpk::Order", query_constraints: [:shop_id, :order_id], primary_key: [:shop_id, :id]
belongs_to :order, autosave: true, foreign_key: [:shop_id, :order_id], counter_cache: true
belongs_to :order_explicit_fk_pk, class_name: "Cpk::Order", foreign_key: [:shop_id, :order_id], primary_key: [:shop_id, :id]
belongs_to :author, class_name: "Cpk::Author"

has_many :chapters, query_constraints: [:author_id, :book_id]
has_many :chapters, foreign_key: [:author_id, :book_id]

before_destroy :prevent_destroy_if_set

Expand All @@ -27,17 +27,17 @@ class BrokenBook < Book
end

class BrokenBookWithNonCpkOrder < Book
belongs_to :order, class_name: "Cpk::NonCpkOrder", query_constraints: [:shop_id, :order_id]
belongs_to :order, class_name: "Cpk::NonCpkOrder", foreign_key: [:shop_id, :order_id]
end

class NonCpkBook < Book
self.primary_key = :id

belongs_to :non_cpk_order, query_constraints: [:order_id]
belongs_to :non_cpk_order, foreign_key: [:order_id]
end

class NullifiedBook < Book
has_one :chapter, query_constraints: [:author_id, :book_id], dependent: :nullify
has_one :chapter, foreign_key: [:author_id, :book_id], dependent: :nullify
end

class BookWithOrderAgreements < Book
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/models/cpk/chapter.rb
Expand Up @@ -7,6 +7,6 @@ class Chapter < ActiveRecord::Base
# to be shared between different databases
self.primary_key = [:author_id, :id]

belongs_to :book, query_constraints: [:author_id, :book_id]
belongs_to :book, foreign_key: [:author_id, :book_id]
end
end
4 changes: 2 additions & 2 deletions activerecord/test/models/cpk/comment.rb
Expand Up @@ -3,7 +3,7 @@
module Cpk
class Comment < ActiveRecord::Base
self.table_name = :cpk_comments
belongs_to :commentable, class_name: "Cpk::Post", query_constraints: %i[commentable_title commentable_author], polymorphic: true
belongs_to :post, class_name: "Cpk::Post", query_constraints: %i[commentable_title commentable_author]
belongs_to :commentable, class_name: "Cpk::Post", foreign_key: %i[commentable_title commentable_author], polymorphic: true
belongs_to :post, class_name: "Cpk::Post", foreign_key: %i[commentable_title commentable_author]
end
end
10 changes: 5 additions & 5 deletions activerecord/test/models/cpk/order.rb
Expand Up @@ -10,8 +10,8 @@ class Order < ActiveRecord::Base
alias_attribute :id_value, :id

has_many :order_agreements
has_many :books, query_constraints: [:shop_id, :order_id]
has_one :book, query_constraints: [:shop_id, :order_id]
has_many :books, foreign_key: [:shop_id, :order_id]
has_one :book, foreign_key: [:shop_id, :order_id]
has_many :order_tags
has_many :tags, through: :order_tags
end
Expand All @@ -26,8 +26,8 @@ class BrokenOrder < Order
class OrderWithSpecialPrimaryKey < Order
self.primary_key = [:shop_id, :status]

has_many :books, query_constraints: [:shop_id, :status]
has_one :book, query_constraints: [:shop_id, :status]
has_many :books, foreign_key: [:shop_id, :status]
has_one :book, foreign_key: [:shop_id, :status]
end

class BrokenOrderWithNonCpkBooks < Order
Expand All @@ -46,7 +46,7 @@ class OrderWithPrimaryKeyAssociatedBook < Order
end

class OrderWithNullifiedBook < Order
has_one :book, query_constraints: [:shop_id, :order_id], dependent: :nullify
has_one :book, foreign_key: [:shop_id, :order_id], dependent: :nullify
end

class OrderWithSingularBookChapters < Order
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/models/cpk/post.rb
Expand Up @@ -3,6 +3,6 @@
module Cpk
class Post < ActiveRecord::Base
self.table_name = :cpk_posts
has_many :comments, class_name: "Cpk::Comment", query_constraints: %i[commentable_title commentable_author], as: :commentable
has_many :comments, class_name: "Cpk::Comment", foreign_key: %i[commentable_title commentable_author], as: :commentable
end
end
2 changes: 1 addition & 1 deletion activerecord/test/models/cpk/review.rb
Expand Up @@ -4,6 +4,6 @@ module Cpk
class Review < ActiveRecord::Base
self.table_name = :cpk_reviews

belongs_to :book, class_name: "Cpk::Book", query_constraints: [:author_id, :number]
belongs_to :book, class_name: "Cpk::Book", foreign_key: [:author_id, :number]
end
end
4 changes: 2 additions & 2 deletions activerecord/test/models/sharded/blog_post_destroy_async.rb
Expand Up @@ -6,9 +6,9 @@ class BlogPostDestroyAsync < ActiveRecord::Base
query_constraints :blog_id, :id

belongs_to :blog
has_many :comments, dependent: :destroy_async, query_constraints: [:blog_id, :blog_post_id], class_name: "Sharded::CommentDestroyAsync"
has_many :comments, dependent: :destroy_async, foreign_key: [:blog_id, :blog_post_id], class_name: "Sharded::CommentDestroyAsync"

has_many :blog_post_tags, query_constraints: [:blog_id, :blog_post_id], class_name: "Sharded::BlogPostTag"
has_many :blog_post_tags, foreign_key: [:blog_id, :blog_post_id], class_name: "Sharded::BlogPostTag"
has_many :tags, through: :blog_post_tags, dependent: :destroy_async, class_name: "Sharded::Tag"
end
end
Expand Up @@ -6,6 +6,6 @@ class BlogPostWithRevision < ActiveRecord::Base
self.table_name = :sharded_blog_posts
query_constraints :blog_id, :revision, :id

has_many :comments, primary_key: [:blog_id, :id], query_constraints: [:blog_id, :blog_post_id]
has_many :comments, primary_key: [:blog_id, :id], foreign_key: [:blog_id, :blog_post_id]
end
end
2 changes: 1 addition & 1 deletion activerecord/test/models/sharded/comment_destroy_async.rb
Expand Up @@ -5,7 +5,7 @@ class CommentDestroyAsync < ActiveRecord::Base
self.table_name = :sharded_comments
query_constraints :blog_id, :id

belongs_to :blog_post, dependent: :destroy_async, query_constraints: [:blog_id, :blog_post_id], class_name: "Sharded::BlogPostDestroyAsync"
belongs_to :blog_post, dependent: :destroy_async, foreign_key: [:blog_id, :blog_post_id], class_name: "Sharded::BlogPostDestroyAsync"
belongs_to :blog_post_by_id, class_name: "Sharded::BlogPostDestroyAsync", foreign_key: :blog_post_id
belongs_to :blog
end
Expand Down
10 changes: 5 additions & 5 deletions guides/source/active_record_composite_primary_keys.md
Expand Up @@ -158,19 +158,19 @@ SELECT * FROM orders WHERE id = 2

This only works if the model's composite primary key contains the `:id` column,
_and_ the column is unique for all records. In order to use the full composite
primary key in associations, set the `query_constraints` option on the
primary key in associations, set the `foreign_key:` option on the
association. This option specifies a composite foreign key on the association,
meaning that all columns in the foreign key will be used to query the
associated record(s). For example:

```ruby
class Author < ApplicationRecord
self.primary_key = [:first_name, :last_name]
has_many :books, query_constraints: [:first_name, :last_name]
has_many :books, foreign_key: [:first_name, :last_name]
end

class Book < ApplicationRecord
belongs_to :author, query_constraints: [:author_first_name, :author_last_name]
belongs_to :author, foreign_key: [:author_first_name, :author_last_name]
end
```

Expand Down Expand Up @@ -285,8 +285,8 @@ you must use the `composite_identify` method:
```ruby
class BookOrder < ApplicationRecord
self.primary_key = [:shop_id, :id]
belongs_to :order, query_constraints: [:shop_id, :order_id]
belongs_to :book, query_constraints: [:author_id, :book_id]
belongs_to :order, foreign_key: [:shop_id, :order_id]
belongs_to :book, foreign_key: [:author_id, :book_id]
end
```

Expand Down
6 changes: 3 additions & 3 deletions guides/source/association_basics.md
Expand Up @@ -579,18 +579,18 @@ SELECT * FROM orders WHERE id = 2
```

This only works if the model's composite primary key contains the `:id` column, _and_ the column is unique for
all records. In order to use the full composite primary key in associations, set the `query_constraints` option on
all records. In order to use the full composite primary key in associations, set the `foreign_key:` option on
the association. This option specifies a composite foreign key on the association: all columns in the foreign key will
be used when querying the associated record(s). For example:

```ruby
class Author < ApplicationRecord
self.primary_key = [:first_name, :last_name]
has_many :books, query_constraints: [:first_name, :last_name]
has_many :books, foreign_key: [:first_name, :last_name]
end

class Book < ApplicationRecord
belongs_to :author, query_constraints: [:author_first_name, :author_last_name]
belongs_to :author, foreign_key: [:author_first_name, :author_last_name]
end
```

Expand Down

0 comments on commit c96b4c7

Please sign in to comment.