Most of the support here is in implementing how to correctly substitute
multiple values in place of one, for composite caes. In composite cases,
it's not sufficient to hash a label into a single integer value.
Instead, we build an API that accepts a single label, and a list of
columns that we'd like to map to. The algorithm used internally is very
similar to #identify, with an additional bit shift and modulo to cycle
the hash and ensure it doesn't exceed a max.
This fixes `ids_writer` so that it can handle a composite primary key.
Using a CPK model associated with a non-CPK model was working correctly
(which I added a test for). Using a CPK model associated with another
CPK model was not working correctly. It now takes it into account to
write the correct ids.
While working on `ids_reader` I found that `pluck` is not working for
CPK because it's passing an array of attributes and that's not supported
by `disallow_raw_sql!`. I chose to call `flatten` in `pluck` but not
conditionally because this seems like it could be a problem elsewhere as
well. This fixes pluck by CPK overall and fixes a test in the
calculations test file.
Given a model associated with a composite primary key by `id` attribute,
for example:
```ruby
Order.primary_key = [:shop_id, :id]
OrderAgreement.primary_key = :id
Order.has_many :order_agreements, primary_key: :id
```
Accessing the association should perform queries using
the `id` attribute value and not the `id` as Order's composite primary key.
```ruby
order = Order.last # => #<Order id: 1, shop_id: 2>
order.order_agreements.to_a
```
In 15369fd we introduced the ability to infer the foreign_key for a model using `inverse_of`.
In some association configurations, such as when there is a `has_one` that is the inverse of another `has_one` association, this inference causes infinite recursion.
This addresses that by adding a param to `Reflection#foreign_key` to indicate whether to infer from `inverse_of`, and in `#derive_foreign_key` we explicitly disable this behavior when calling `#foreign_key` on the inverse association.
* Infer `foerign_key` when `inverse_of` is present
Automatically infer `foreign_key` on `has_one` and `has_many` associations when `inverse_of` is present.
When inverse of is present, rails has all the info it needs to figure out what the foreign_key on the associated model should be. I can't imagine this breaking anything
* Update test models to remove redundant foreign_keys
* add changelog entry
* fix changelog grammar
Co-authored-by: Rafael Mendonça França <rafael@franca.dev>
I discovered this while working on https://github.com/rails/rails/pull/47800
The bug is quite subtle.
If you call `Model.all.delete(id)`, `all` is an `ActiveRecord::Relation`
which does not respond to `delete`, so it delegates to `Model.delete`
and generate that method in the `GeneratedRelationMethods` module.
The problem however, is that `CollectionProxy` does define `delete`,
so after that initial call, the `Model::ActiveRecord_CollectionProxy`
subclass now has its `delete` method overridden, and now delegate
to the model.
Here I chose to keep that method generation caching, but I'm honestly
not convinced it's really needed. I question how much of a hotspot
these methods are, and we're busting method caches and generating
a lot of code to save on a minor `method_missing` call.
I think we should just remove that caching.
Fix: https://github.com/rails/rails/issues/47704
Superseed: https://github.com/rails/rails/pull/47722
While the instance variable ordering bug will be fixed in Ruby 3.2.2,
it's not great that we're depending on such brittle implementation detail.
Additionally, Marshalling Active Record instances is currently very inefficient,
the payload include lots of redundant data that shouldn't make it into the cache.
In this new format the serialized payload only contains basic Ruby core or stdlib objects,
reducing the risk of changes in the internal representation of Rails classes.
The codepaths related to destroying associations asynchronously now
consider when query constraints are present. In most cases, this means
interpreting the primary key as an array of columns, and identifying
associated records by a tuple of these columns, where previously this
would've been a single ID. In each of the callsites, we use branching.
This is done to maintain backwards compatibility and ensure the
signature of the destroy job remains stable: it has consumers outside of
Rails.
Given a model with a composite primary key, the `query_constraints_list`
should equal the `primary_key` value to enable capabilities managed by
`query_constraints_list` such as `destroy`, `update`, `delete` and others.
Given a model with a composite primary key:
```ruby
class Order < ActiveRecord::Base
self.primary_key = [:shop_id, :id]
end
```
`ActiveRecord::Base#id` method will return an array of values for every
column of the primary key.
```ruby
order = Order.create!(shop_id: 1, id: 2)
order.id # => [1, 2]
```
The `id` column is accessible through the `read_attribute` method:
```ruby
order.read_attribute(:id) # => 2
```
`where_values_hash` method signature accepts `table_name` which is not
always the same as the association name.
So passing `through_association.reflection.name.to_s` as `table_name`
in `through_scope_attributes` wasn't accurate for every case.
This commit fixes the issue by passing the `table_name` taken from
`through_association.reflection.klass.table_name` instead.
YAML has quite a bit of footguns, as such it's desirable
to be able to substitute it for something else or even
simply to force users to define a serializer explictly for
every serialized columns.
Given an association like:
```ruby
BlogPost.has_many :blog_post_tags,
query_constraints: [:blog_id, :blog_post_id]
BlogPost.has_many :tags, through: :blog_post_tags
```
The `tags` association records can be queried
and the resulting JOIN query will include both `blog_id` and
`blog_post_id` in the ON clause.
This PR refactors the query constraints feature to be more contained and
have less implicit behavior. Eventually we'll want `primary_key` to flow
through query constraints and return `["id"]` when we have a single
column and internally Rails will use an array for all `primary_key` and
`foreign_key` calls, falling back to query constraints. At the moment
however, this is a large change in Rails and one that could break
current expected behavior. In order to implenent the feature and not
break compatibility I think we should walk back the feature a little
bit. The changes are:
* Only return `query_constraints_list` if `query_constraints` was set by
the model, otherwise return nil.
* Re-implement `primary_key` calls where we only have a single ID
* Update the `foreign_key` option on associations to use a
`query_constraints` option instead so we don't need to check
`is_a?(Array)`.
These changes will ensure that the changes are contained to
`query_constraints` rather than having to make decisions about whether
we want to treat `primary_key` as an array. For now we will call
everything `query_constraints` which makes it easy to see the line when
we want an array vs single column. Later we can change the meaning of
`primary_key` if necessary.
Follow-up to #46605.
For a scenario like:
```ruby
class Cat < ActiveRecord::Base
has_many :lives
end
class Life
end
```
this commit changes the error from:
> Rails couldn't find a valid model for the lives association. Use the
> :class_name option on the association declaration to tell Rails which
> model to use.
to:
> The Life model class for the Cat#lives association is not an
> ActiveRecord::Base subclass.
Additionally, for a scenario like:
```ruby
class Cat < ActiveRecord::Base
has_many :lives
end
class Live < ActiveRecord::Base
end
```
this commit changes the error from:
> Rails couldn't find a valid model for the lives association. Use the
> :class_name option on the association declaration to tell Rails which
> model to use.
to:
> Missing model class Life for the Cat#lives association. You can
> specify a different model class with the :class_name option.
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>
