Renames `_primary_key_constraints_hash` private method to avoid implying
that it strictly represents a constraint based on the model primary key
Allows columns list used in query constraints to be configurable using
`ActiveRecord::Base.query_constraints` macro
Fixed a bug that caused the alias name of "group by" to be too long and the first half of the name would be the same in both cases if it was cut by max identifier length.
Fix#46285
Co-authored-by: Yusaku ONO <yono@users.noreply.github.com>
When source and target classes have a different set of attributes adapts
attributes such that the extra attributes from target are added.
Fixes#41195
Co-authored-by: SampsonCrowley <sampsonsprojects@gmail.com>
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
[`composed_of`](https://api.rubyonrails.org/classes/ActiveRecord/Aggregations/ClassMethods.html#method-i-composed_of)
is a feature that is not widely used and its API is somewhat confusing,
especially for beginners. It was even deprecated for a while, but 10
years after its deprecation, it's still here.
The Rails documentation includes these examples including mapping:
```ruby
composed_of :temperature, mapping: %w(reading celsius)
composed_of :balance, class_name: "Money", mapping: %w(balance amount)
composed_of :address, mapping: [ %w(address_street street),
%w(address_city city) ]
```
Hashes are accepted kind-of accidentally for the `mapping` option. Using
a hash, instead of an array or array of arrays makes the documentation
more beginner-friendly.
```ruby
composed_of :temperature, mapping: { reading: :celsius }
composed_of :balance, class_name: "Money", mapping: { balance: :amount }
composed_of :address, mapping: { address_street: :street, address_city:
:city }
```
Before Ruby 1.9, looping through a hash didn't have deterministic order,
and the mapping order is important, as that's the same order Rails uses
when initializing the value object. Since Ruby 1.9, this isn't an issue
anymore, so we can change the documentation to use hashes instead.
This commit changes the documentation for `composed_of`, clarifying that
any key-value format (both a `Hash` and an `Array`) are accepted for the
`mapping` option. It also adds tests to ensure hashes are also accepted.
In Psych >= 4.0.0, load defaults to safe_load. This commit
makes the ActiveRecord::Coders::YAMLColum class use Psych safe_load
as the Rails default.
This default is configurable via ActiveRecord.use_yaml_unsafe_load
We conditionally fallback to the correct unsafe load if use_yaml_unsafe_load
is set to true. unsafe_load was introduced in Psych 4.0.0
The list of safe_load permitted classes is configurable via
ActiveRecord.yaml_column_permitted_classes
[CVE-2022-32224]
Fix `touch` to raise an error for readonly columns
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
Co-authored-by: Guillermo Iguaran <guilleiguaran@gmail.com>
The primary_key for the `dl_keyed_join` relation was incorrect. It
expected to use a `joins_key` attribute on the parent record in order
to link to the `dl_keyed_join` record. The parent class does not have a
`joins_key` attribute at all, which means that association was never
setup correctly.
Co-authored-by: Daniel Colson <composerinteralia@github.com>
In our use case - we have a base model that has a default scope that we
want enabled for all queries, ex:
```ruby
class Developer < ApplicationRecord
default_scope -> { where(firm_id: Current.firm_id) }, all_queries:
true
end
```
We're also leveraging a module that will add a default scope to only
find soft-deleted records.
```ruby
module SoftDeletable
extend ActiveSupport::Concern
included do
default_scope { where(deleted_at: nil) }
end
```
Through this, we've found that when using default scopes in combination,
*specifically in the use case where the _non_ all queries scope is
declared first*, that we would get an error when calling `.update`:
```ruby
class Developer < ApplicationRecord
include SoftDeletable
default_scope -> { where(firm_id: Current.firm_id) }, all_queries:
true
```
```ruby
Current.firm_id = 5
developer = Developer.new(name: "Steve")
developer.update(name: "Stephen")
NoMethodError: undefined method `where' for nil:NilClass
```
In digging into the code, this was due to there not being an `else` case
for the `inject` used to combine `default_scopes` together (`inject`
uses the return value of the block as the memoizer).
Without the `else` case, if the block returned `nil`, `nil` was passed
to the evaluation of the next `default_scope`.
This commit adds the `else`, and also makes a minor adjustment in
variable naming (`default_scope` to `combined_scope`) in an effort to
add a little more readability, as we're iterating over an array of
default scopes, but what we're building is _the_ default scope to be
used in the query, etc.
Co-authored-by: Will Cosgrove <will@cosgrove.email>
Instead of pre-validating the configuration, we now check for the
required values when they're used.
Co-authored-by: Alex Ghiculescu <alex@tanda.co>
Co-authored-by: Jorge Manrubia <jorge.manrubia@gmail.com>
Co-authored-by: John Hawthorn <john@hawthorn.email>
This method is intended to replace code like the following, which
returns early when a user with a matching email is not found:
```ruby
User.find_by(email: "...")&.authenticate("...")
```
Such code is vulnerable to timing-based enumeration attacks, wherein an
attacker can determine if a user account with a given email exists.
After confirming that an account exists, the attacker can try passwords
associated with that email address from other leaked databases, in case
the user re-used a password across multiple sites (a common practice).
Additionally, knowing an account email address allows the attacker to
attempt a targeted phishing ("spear phishing") attack.
`authenticate_by` addresses the vulnerability by taking the same amount
of time regardless of whether a user with a matching email is found.
With models that use STI, Fixtures now load based
on the refelction class. This allows to resolve the enums
for each specific class instead of just resolving
those of the base class.
Removes grouping by available_record class names in ActiveRecord::Associations::Preloader::Batch so that records of differing STI classes can be located.
Background
---
I recently noticed we had a number of associations in GitHub that would
benefit from having `inverse_of` set, and so I began adding them. I
ended up adding them to virtually every association with a scope, at
which point I wondered whether Rails might be able to automatically find
these inverses for us.
For GitHub, the changes in this commit end up automatically adding
`inverse_of` to 171 of associations that were missing it.
My understanding is that we do not automatically detect `inverse_of` for
associations with scopes because the scopes could exclude the records we
are trying to inverse from. But I think that should only matter if there
is a scope on the inverse side, not on the association itself.
For example:
Scope on has_many
----
```rb
class Post < ActiveRecord::Base
has_many :comments, -> { visible }
end
class Comment < ActiveRecord::Base
belongs_to :post
scope :visible, -> { where(visible: true) }
scope :hidden, -> { where(visible: false) }
end
post = Post.create!
comment = post.comments.hidden.create!
assert comment.post
```
This code leaves `post.comments` in sort of a weird state, since it
includes a comment that the association would filter out. But that's
true regardless of the changes in this commit.
Regardless of whether the comments association has an inverse,
`comment.post` will return the post. The difference is that when
`inverse_of` is set we use the existing post we have in memory, rather
than loading it again. If there is a downside to having the `inverse_of`
automatically set here I'm not seeing it.
Scope on belongs_to
----
```rb
class Post < ActiveRecord::Base
has_many :comments
scope :visible, -> { where(visible: true) }
scope :hidden, -> { where(visible: false) }
end
class Comment < ActiveRecord::Base
belongs_to :post, -> { visible }
end
post = Post.hidden.create!
comment = post.comments.create!
assert_nil comment.post
```
This example is a different story. We don't want to automatically infer
the inverse here because that would change the behavior of
`comment.post`. It should return `nil`, since it's scoped to visible
posts while this one is hidden.
This behavior was not well tested, so this commit adds a test to
ensure we haven't changed it.
Changes
---
This commit changes `can_find_inverse_of_automatically` to allow us to
automatically detect `inverse_of` when there is a scope on the
association, but not when there is a scope on the potential inverse
association. (`can_find_inverse_of_automatically` gets called first with
the association's reflection, then if it returns true we attempt to find
the inverse reflection, and finally we call the method again with the
inverse reflection to ensure we can really use it.)
Since this is a breaking change—specifically in places where code may
have relied on a missing `inverse_of` causing fresh copies of a record
to be loaded—we've placed it behind the `automatic_scope_inversing` flag
(whose name was inspired by `has_many_inversing`). It is set to true for
new applications via framework defaults.
Testing
---
In addition to the inverse association tests, this commit also adds some
cases to a few tests related to preloading. They are basically
duplicates of existing tests, but with lower query counts.
Testing this change with GitHub, the bulk of the failing tests were
related to lower query counts. There were additionally 3 places (2 in
tests and one in application code) where we relied on missing
`inverse_of` causing fresh copies of a record to be loaded.
There's still one Rails test that wouldn't pass if we ran the whole
suite with `automatic_scope_inversing = true`. It's related to
`TaggedPost`, which changes the polymorphic type from the base class
`Post` to the subclass `TaggedPost`.
```rb
class TaggedPost < Post
has_many :taggings, -> { rewhere(taggable_type: "TaggedPost") }, as: :taggable
end
```
Setting the inverse doesn't work because it ends up changing the type
back to `Post`, something like this:
```rb
post = TaggedPost.new
tagging = post.taggings.new
puts tagging.taggable_type
=> TaggedPost
tagging.taggable = post
puts tagging.taggable_type
=> Post
```
I think this is an acceptable change, given that it's a fairly specific
scenario, and is sort of at odds with the way polymorphic associations
are meant to work (they are meant to contain the base class, not the
subclass). If someone is relying on this specific behavior they can
still either keep `automatic_scope_inversing` set to false, or they can
add `inverse_of: false` to the association.
I haven't found any other cases where having the `inverse_of` would
cause problems like this.
Co-authored-by: Chris Bloom <chrisbloom7@gmail.com>
Sometimes when we are preloading associations on records, we already have
the association objects loaded in-memory. But the Preloader will go to the
database to fetch them anyways because there is no way to tell it about
these loaded objects. This PR adds a new `available_records` argument to
supply the Preloader with a set of in-memory records that it can use to
fill associations without making a database query.
`available_records` is an array of ActiveRecord::Base objects. Mixed
models are supported. Preloader will use these records to try to fill the
requested associations before querying the database. This can save database
queries by reusing in-memory objects.
The optimization is only applied to single associations (i.e. :belongs_to,
:has_one) as we must query the database to do an exhaustive lookup for
collections (i.e. :has_many). It also requires the query to have no scopes
as scopes can filter associations even if they are already in memory.
```ruby
comment = Comment.last
post = Post.find_by(id: comment.post_id)
all_authors = Author.all.to_a
Preloader.new([comment], [:post, :author]).call
Preloader.new([comment], [:post, :author], available_records: [post, all_authors]).call
```
Co-Authored-By: John Hawthorn <john@hawthorn.email>
As of Ruby 2.7 DidYouMean is included as a default gem, so there is no
need to check if DidYouMean is defined in the test suite. We still need
to check if the DidYouMean modules are defined in the actual code, as
someone might run Rails with DidYouMean disabled by using the
`--disable-did_you_mean` flag. This is ussually done for performance
reasons.
This commit also includes some of the changes made by Yuki in:
https://github.com/rails/rails/pull/39555
These changes include replacing Jaro with the more accurate
SpellChecker, and using DidYouMean::Correctable for simplere
corrections.
The DidYouMean::SpellChecker does have a treshold for corrections.
If there is not enough similarity it might not return a suggestion.
To stop the tests from failing some test data had to be changed.
For example, `non_existent` does not meet the treshold for `hello`, but
`ello` does:
DidYouMean::SpellChecker.new(dictionary: %w[hello]).correct('non_existent')
=> []
DidYouMean::SpellChecker.new(dictionary: %w[hello]).correct('ello')
=> ["hello"]
The treshold makes sense for spelling errors. But maybe we should add a
different SpellChecker that helps to get a suggestion even if there is
little overlap. For example for when a model only has 2 attributes
(title and body), it's helpful to get a suggestion for `name`
Co-Authored-By: Yuki Nishijima <yk.nishijima@gmail.com>
When a model has an STI enum type, the type wasn't properly applied when
disable joins was set to true. In this case we need to apply the scope
from the association in `add_constraints` so that `type` is included in
the query. Otherwise in a `has_one :through` with `type` all records
will be returned because we're not filtering on type.
Long term I think this might be in the wrong place and that we want to
do this in a new definition of `target_scope` but that's a future
refactoring that needs to be done.
Instance dependent associations are associations which are defined with
a scope taking an argument which makes the scope they are selected from
dependent on the owner record.
This presented a challenge for preloading and eager_loading, since each
record's association being loaded could require a different scope.
Previously this raised an exception if attempted.
This commit adds the ability to preload instance dependent scopes (eager
loading is still unsupported). This is done by detecting instance
dependent scopes in the preloader and building the scope and
Preloader::Association for each record. Because we now merge similar
scopes later in the preloading process, any records which end up with
the same instance dependent scope will be loaded from a single query.
In the case that there's a different scope for each record, this will
perform N+1 queries. I think it's up to the app developer to consider
this (this is essentially the same as preloading polymorphic scopes).
Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
Co-authored-by: Adam Hess <HParker@github.com>
Co-authored-by: Eileen M. Uchitelle <eileencodes@gmail.com>
Co-authored-by: Dinah Shi <dinahshi@github.com>
If you have a `has_one through` assocation where the `through` has a
`belongs_to` the foreign key won't be `id` for the joins. This change
ensures we use the correct foreign key when querying with disable joins
turned off. Prior to this change the call to `project.lead_developer`
would return `nil` if `disable_joins` was set.
Project has one lead developer through firm. Project also belongs to
firm. So when we call `project.lead_developer` it needs to find the
lead developer through the firm. In these cases we don't want to read
the owners id, we want to read the attribute that will give us the
value of the first chain item's `join_foreign_key`.
Currently Active Record derives the foreign key name for `has_many`
associations based on the Active Record Base `name. Sometimes when using
additional conventions for class names, such as with class name suffixes
and prefixes, it makes sense to be able to further customize the
foreign key logic to account for a different pattern.
Similar to what was done in the
[case of table names](https://github.com/rails/rails/pull/42213),
as Active Record Base extends Active Model Naming, we have already a
`model_name` at the class-level that behaves similarly to `name` while
at the same time giving more flexibility by allowing specifying a custom
name and namespacing.
This commit changes the foreign key computation in Reflections to infer
its value based on the Active Record's `model_name` object instead of
its class `name`. This allows customization of the foreing key pattern
since a distinct instance of `ActiveModel::Name` can be used instead.
For example, to use `post_id` as foreign key for a `PostRecord` class
in a `has_many` association:
```ruby
class PostRecord < ActiveRecord::Base
has_many :comments
class << self
def model_name
ActiveModel::Name.new(self, nil, "Post")
end
end
end
PostRecord.reflect_on_association(:comments).foreign_key
# => "post_id"
```
Instead of using the Active Record Base's class name, the table name is
now inferred based on its model name object. This allows customization
of the table name pattern since a distinct instance of
`ActiveModel::Name` can be used instead.
For example, to map a `PostRecord` class to a `posts` table:
```ruby
class PostRecord < ActiveRecord::Base
class << self
def model_name
ActiveModel::Name.new(self, nil, 'Post')
end
end
end
PostRecord.table_name
# => "posts"
```
This is similar to the `disable_joins` option on `has_many :through`
associations applied to `has_one :through` associations. When
`disable_joins` is set Rails will create 2 or more queries to get
associations instead of generating a join.
```ruby
class Person
belongs_to :dog
has_one :veterinarian, through: :dog, disable_joins: true
end
```
Then instead of generating join SQL, two queries are used for `@person.veterinarian`:
```
SELECT "dogs"."id" FROM "dogs" WHERE "dogs"."person_id" = ? [["person_id", 1]]
SELECT "veterinarians".* FROM "veterinarians" WHERE "veterinarians"."dog_id" = ? [["dog_id", 1]]
```
Co-authored-by: Eileen M. Uchitelle <eileencodes@gmail.com>
Ruby (2.4+) includes a native implementation of `sum` with significant
performance gains. Rails 7.1 will be removing `Enumerable#sum` and
`Array#sum` in favor of Ruby's native implementation.
This commit adds a deprecation warning to calls with non-numeric
arguments without a suitable initial argument as those will be required
once Rails removes this functionality.
Some examples that will now trigger a deprecation warning:
>> %w[foo bar].sum
>> [[1, 2], [3, 4, 5]].sum
To avoid the deprecation warning they should now invoked as follows:
>> %w[foo bar].sum('')
>> [[1, 2], [3, 4, 5]].sum([])
In order to prepare for the deprecation on Rails 7.1, it also
deprecates `[nil].sum == 0`, which in Ruby's native implementation
throws a `TypeError`.
In a multiple database application, associations can't join across
databases. When set, this option tells Rails to make 2 or more queries
rather than using joins for associations.
Set the option on a has many through association:
```ruby
class Dog
has_many :treats, through: :humans, disable_joins: true
has_many :humans
end
```
Then instead of generating join SQL, two queries are used for `@dog.treats`:
```
SELECT "humans"."id" FROM "humans" WHERE "humans"."dog_id" = ? [["dog_id", 1]]
SELECT "treats".* FROM "treats" WHERE "treats"."human_id" IN (?, ?, ?) [["human_id", 1], ["human_id", 2], ["human_id", 3]]
```
This code is extracted from a gem we use internally at GitHub which
means the implementation here is used in production daily and isn't
experimental.
I often get the question "why can't Rails do this automatically" so I
figured I'd include the answer in the commit. Rails can't do this
automatically because associations are lazily loaded. `dog.treats` needs
to load `Dog`, then `Human` and then `Treats`. When `dog.treats` is
called Rails pre-generates the SQL that will be run and puts that
information into a reflection object. Because the SQL parts are pre-generated,
as soon as `dog.treats` is loaded it's too late to skip a join. The join
is already available on the object and that join is what's run to load
`treats` from `dog` through `humans`. I think the only way to avoid setting
an option on the association is to rewrite how and when the SQL is
generated for associations which is a large undertaking. Basically the
way that Active Record associations are designed, it is currently
impossible to have Rails figure out to not join (loading the association
will cause the join to occur, and that join will raise an error if the
models don't live in the same db).
The original implementation was written by me and Aaron. Lee helped port
over tests, and I refactored the extraction to better match Rails style.
Co-authored-by: Lee Quarella <leequarella@gmail.com>
Co-authored-by: Aaron Patterson <aaron@rubyonrails.org>
