Follow-up to #47420.
Whereas the original behavior (`on: :create`) is invoked only once
before a record is persisted, the new behavior (`on: :initialize`) is
invoked not only new record but also persisted records.
It should be invoked only once for new record consistently.
This was accidentally left in when this change was [submitted][1] to
main. This was already addressed in the backport commit but this
addresses it for main.
[1]: 57a9e25f8e
Follow-up to [#47420][]
With the changes made in [#47420][], `has_secure_token` declarations can
be configured to execute in an `after_initialize` callback. This commit
proposed a new Rails 7.1 default: generate all `has_secure_token` values
when their corresponding models are initialized.
To preserve pre-7.1 behavior, applications can set
`config.active_record.generate_secure_token_on = :create`.
By default, generate the value when the model is initialized:
```ruby
class User < ApplicationRecord
has_secure_token
end
record = User.new
record.token # => "fwZcXX6SkJBJRogzMdciS7wf"
```
With `config.active_record.generate_secure_token_on = :create`, generate
the value when the model is created:
```ruby
# config/application.rb
config.active_record.generate_secure_token_on = :create
# app/models/user.rb
class User < ApplicationRecord
has_secure_token on: :create
end
record = User.new
record.token # => nil
record.save!
record.token # => "fwZcXX6SkJBJRogzMdciS7wf"
```
[#47420]: https://github.com/rails/rails/pull/47420
Co-authored-by: Hartley McGuire <skipkayhil@gmail.com>
There were a few 6.1 migration compatibility fixes in [previous][1]
[commits][2]. Most importantly, those commits reorganized some of the
compatibility tests to ensure that the tests would run against every
Migration version. To continue the effort of improving test coverage for
Migration compatibility, this commit converts tests for create_table and
change_column setting the correct precision on datetime columns.
While the create_table tests all pass, the change_column test did not
pass for 7.0 versioned Migrations on sqlite. This was due to the sqlite
adapter not using new_column_definition to set the options on the new
column (new_column_definition is where precision: 6 gets set if no
precision is specified). This happens because columns can't be modified
in place in sqlite and instead the whole table must be recreated and the
data copied. Before this commit, change_column would use the options
of the existing column as a base and merge in the exact options (and
type) passed to change_column.
This commit changes the change_column method to replace the existing
column without using the existing options. This ensures that precision:
6 is set consistently across adapters when change_column is used to
create a datetime column.
[1]: c2f838e80c
[2]: 9b07b2d6ca
`8a5cf4cf4415ae1cdad7feecfb27149c151b0b10` made changes to `to_key` in order
to support composite identifiers. This commit adds CHANGELOG entries for those.
I noticed this test was throwing a warning for `alias_attribute
:id_value, :id` defined by Rails. Since this test messes with primary
keys I decided to fix it by making custom classes for each of the
tests that need to change the PK.
While debugging these tests I noticed the deprecation wasn't formatted
correctly so I fixed that while in there.
Active Record already has the ability to guess the foreign key. With
query constraints we can use the information from the owner class and
that foreign key to infer what the query constraints should be for an
association. This change improves the ergonomics of the query
constraints feature by making it so that you only have to define the
query constraints on the top level model and skip defining it on the
association. There are a few caveats to this as of this PR:
- If the query constraints on the parent are greater than 2, Rails can't
derive the association constraints
- If the query constraints on the parent don't include the primary key,
Rails can't derive the association constraints.
- If the association has an explicit `foreign_key` or
`query_constraints` option set, Active Record won't infer the key.
- I have not implemented support for CPK yet as it's more complex and
maybe not possible since we don't know which key to split on and use
for the foreign association.
Prior to this change you would need to do the following to use query
constraints:
```ruby
class Post
query_constraints :blog_id, :id
has_many :comments, query_constraints: [:blog_id, :blog_post_id]
end
class Comment
query_constraints :blog_id, :id
belongs_to :blog, query_constraints: [:blog_id, :blog_post_id]
end
```
After this change, the associations don't require you to set the query
constraints, Active Record will generate `[:blog_id, :blog_post_id]`
foreign key itself.
```ruby
class Post
query_constraints :blog_id, :id
has_many :comments
end
class Comment
query_constraints :blog_id, :id
belongs_to :blog
end
```
`Kernel#caller` has linear performance based on how deep the
stack is. While this is a development only feature, it can end
up being quite slow.
Ruby 3.2 introduced `Thread.each_caller_location`, which lazily
yield `Backtrace::Location` objects.
Ref: https://bugs.ruby-lang.org/issues/16663
This is perfect for this use case as we are searching for the
closest frame that matches a pattern, saving us from collecting
the entire backtrace.
We ran into a few cases at GitHub where we were using alias_attribute
incorrectly and the new behavior either didn't warn or raised an unclear
deprecation warning. This attempts to add clarity to the deprecation reason
when you try to alias something that isn't actually an attribute.
Previously, trying to alias a generated attribute method, such as `attribute_in_database`, would
still hit `define_proxy_call`, because we were only checking the owner of the target method.
In the future, we should probably raise if you try to use alias_attribute for a non-attribute.
Note that we don't raise the warning for abstract classes, because the attribute may be implemented
by a child class. We could potentially figure out a way to raise in these cases as well, but this
hopefully is good enough for now.
Finally, I also updated the way we're setting `local_alias_attributes` when `alias_attribute` is
first called. This was causing problems since we now use `alias_attribute` early in the
`load_schema!` call for some models: https://buildkite.com/rails/rails/builds/98910
This backfills a changelog entry for PR #48876. This is potentially something Rails users
should be aware of as in very specific situations it can be a change in behavior
This commit deprecates `read_attribute(:id)` returning the primary key
if the model's primary key is not the id column. Starting in Rails 7.2,
`read_attribute(:id)` will always return the value of the id column.
This commit also changes `read_attribute(:id)` for composite primary
key models to return the value of the id column, not the composite
primary key.
In `1818beb3a3ea5fdb498095d4885f8a7e512f24ca` Rails changed the target
where alias attribute methods are defined. It lead to
`undefine_attribute_methods` to clean alias attribute methods along with
the attribute methods. It was an intended behavior change but it wasn't
properly documented and tested. This commit clarifies the new behavior
in the Active Model changelog along with covering the behavior with tests.
When an abstract class inherited from an Active Record model
defines an alias attribute Rails should expect the original methods
of the aliased attribute to be defined in the parent class and avoid
raising deprecation warning.
This is similar to a [previous commit][1] which ensures that versioned
migrations always call `super` in `compatible_table_definition`. In this
case, these methods are being pulled up to `Current` so that all
subclasses will use a `TableDefinition` class and future developers do
not have to remember to add all of these methods to new versioned
classes when a new one is created.
[1]: 16f8bd7944
While working on #48969, I found that some of the Compatibility test
cases were not working correctly. The tests removed in this commit were
never running the `change_table` migration and so were not actually
testing that `change_table` works correctly. The issue is that the two
migrations created in these tests both have `nil` versions, and so the
Migrator only runs the first one.
This commit refactors the tests so that its easier to test the behavior
of each Migration class version (and I think the rest of the tests
should be updated to use this strategy as well). Additionally, since the
tests are fixed it exposed that `t.change` in a `change_table` is not
behaving as expected so that is fixed as well.
This commit adds support for composite identifiers in `to_key`.
Rails 7.1 adds support for composite primary key which means that
composite primary key models' `#id` method returns an `Array` and
`to_key` needs to avoid double-wrapping the value.
This allows access to the raw id column value on records for which an
id column exists but is not the primary key. This is common amongst
models with composite primary keys.
rails/rails@a88f47d fixed an issue where subclasses were regenerating aliased attribute methods defined on parent classes. Part of the solution was to call #generate_alias_attributes recursively on a class's superclass to generate all the alias attributes in the inheritance hierarchy. However, the implementation relies on #base_class to determine if we should call #generate_alias_attributes on the superclass. Since all models that inherit from abstract classes are base classes, this means that #generate_alias_attributes will never be called on abstract classes, meaning no method will be generated for any alias attributes defined on them.
To fix this issue, we should always call #generate_alias_attributes on the superclass unless the superclass is ActiveRecord::Base.
I'm working on an enhancement to query constraints that will require me
to know when we have query constraints but not a composite key.
Currently if you have a composite key it will be included in the query
constraints list. There's not a way to differentiate between the two
which means that we're forced into setting the query constraints on the
associations for the primary key.
This change adds a `has_query_constraints?` method so we can check the
class for query constraints. The options still work as well but we can
be sure we're always picking up the query constraints when they're
present.
We originally implemented this project to use OR queries instead of IN
because it seemed like there was no way to do that without a row
constructor. While working on implementing this feature in our vitess
gem I noticed that we can actually get the queries we wanted, and the
new code is more performant than generating an OR.
SQL Before:
```sql
SELECT "sharded_comments".*
FROM "sharded_comments"
WHERE ("sharded_comments"."blog_id" = 969142904
AND ("sharded_comments"."blog_post_id" = 357271355
OR "sharded_comments"."blog_post_id" = 756811794)
OR "sharded_comments"."blog_id" = 308674288
AND "sharded_comments"."blog_post_id" = 1055755181)
```
SQL After:
```sql
SELECT "sharded_comments".*
FROM "sharded_comments"
WHERE "sharded_comments"."blog_id"
IN (969142904, 308674288)
AND "sharded_comments"."blog_post_id"
IN (357271355, 756811794, 1055755181)
```
Using one of the tests that utilizes this code path, I benchmarked the
queries. The new implementation is faster:
Before:
```
Warming up --------------------------------------
queries 147.000 i/100ms
Calculating -------------------------------------
queries 1.486k (± 3.2%) i/s - 7.497k in 5.050742s
```
After:
```
Warming up --------------------------------------
queries 179.000 i/100ms
Calculating -------------------------------------
queries 1.747k (± 4.5%) i/s - 8.771k in 5.031424s
```
We can probably improve this more but I think this query is more in line
with what we want and expect while also being more performant (without
having to build a new row constructor in arel).
Because we're using a class_attribute to track all of the attribute_aliases,
each subclass was regenerating the parent class methods, causing some unexpected
deprecation messages.
Instead, we can use a class instance variable to track the attributes aliased locally
in that particular subclass. We then walk up the chain and re-define the attribute methods
if they haven't been defined yet.
Pluses cannot be used to create code blocks when the content includes a
space.
Found using a regular expression:
```bash
$ rg '#\s[^+]*\+[^+]*\s[^+]*\S\+'
```
This will make easier to make sure the service is running when the
development container is started.
Before after the first time the container was created, if it was
restarted you needed to run the boot.sh script manually to start the
service.
If the user defined a translation to a nested error on base we should
look it up in the same way we do for the other attributes.
If no translation is set, we fallback to the name of the association.
Fixes#48884.
* `supports_json?` always return false on mariadb so the test needs to
stub.
* to_a(as: :hash) is not supported on trilogy. And we should be dealing
with ActiveRecord::Result objects anyway.
The result of the SQL is not the same on all databases, so writing a
test that would pass in all cases would mean conditionals here.
Since we already testing if the SQL results of a As node is correct in
the visitor tests, we only need to check if the As node attributes are
correct here.
If you have already loaded the relation when using in_batches, you
would reload each batch from the database. Since we already have the
records available, we can avoid these queries and return the records
as-if the query was run.
Pulled from #48413
"Fix autosave associations with validations added on `:base` of the associated objects"
Co-authored-by: fatkodima <fatkodima123@gmail.com>
Despite the inconvenience of double-backslashing, the backslash was chosen
because it's a very common char to use in escaping across multiple programming
languages. A developer, without looking at documentation, may intuitively try
to use it to achieve the desired results in this scenario.
Fixes#37779
`define_proxy_call` accepts `call_args` as an argument which impacts
method body generation but it doesn't use `call_args` in the `namespace`
generation which leads to the same cached method to be reused even if
`call_args` differ.
It has never been an issue since `define_proxy_call` was only used to
generate Active Record attribute methods and we were always passing
the same `call_args` per method name.
However, since https://github.com/rails/rails/pull/48533 we are using
`define_proxy_call` to generate alias attribute methods where `call_args`
differ for the same method name which leads to the same cached method
being reused in wrong places.
This commit fixes the issue by making sure `call_args` are being
considered when generating the `namespace` for the method.
AbstractAdapter#schema_version uses migration_context.current_version by
default, but some adapters might redefine this method to use a custom
schema version number. This fix ensures that generated schema cache
files use this method and that it is also used when validating a file's
version.
This PR fixes a bug introduced in https://github.com/rails/rails/pull/48761
which leads to a `NoMethodError` when a non Active Record object passed
to `include?` or `member?` since only Active Record objects
respond to `composite_primary_key?`.
Using `alias_attribute` to alias an association is not an indented
use case. This commit removes `alias_attribute` calls to alias an
association along with the relevant tests. However, it doesn't mean
that the commit brakes any current behaviors.
Followup: https://github.com/rails/rails/pull/48743
After careful consideration, unless users have a schema cache dump loaded
and `check_schema_cache_dump_version = false`, we have no choice but
to arbitrate between resiliency and performance.
If we define attribute methods during boot, we allow them to be shared
between forked workers, and prevent the first requests to be slower.
However, by doing so we'd trigger a connection to the datase, which
if it's unresponsive could lead to workers not being able to restart
triggering a cascading failure.
Previously Rails used to be in some sort of middle-ground where
it would define attribute methods during boot if for some reason
it was already connected to the database at this point.
But this is now deemed undesirable, as an application initializer
inadvertantly establishing the database connection woudl lead to
a readically different behavior.
`include` and `member?` delegate to `exists?` with the record's primary
key to determine if an unloaded relation contains a given record. If
the primary key is composite, `exists?` belives we are passing
where-style conditions and fails.
This commit fixes the issue by turning the record's composite primary key
into a hash of column / value pairs that `exists?` can accept.
The callback when the value is generated. When called with `on:
:initialize`, the value is generated in an `after_initialize` callback,
otherwise the value will be used in a `before_` callback. It will
default to `:create`.
This commit changes bodies of methods generated by `alias_attribute`
along with generating these methods lazily.
Previously the body of the `alias_attribute :new_title, :title` was
`def new_title; title; end`. This commit changes it to
`def new_title; attribute("title"); end`.
This allows for `alias_attribute` to be used to alias attributes named
with a reserved names like `id`:
```ruby
class Topic < ActiveRecord::Base
self.primary_key = :title
alias_attribute :id_value, :id
end
Topic.new.id_value # => 1
```
Ryuta Kamizono