Currently `ActiveModel::Model` is defined as the minimum API to talk
with Action Pack and Action View.
Its name suggests it can be included to create Active Record type
models, but for creating models it's probably too minimal. For example
it's very common to include ActiveModel::Attributes as well.
By moving `ActiveModel::Model`'s implementation to a new
`ActiveModel::API` we keep a definition of the minimum API to talk with
Action Pack and Action View.
For `ActiveModel::Model` we only need to include `ActiveModel::API`.
This will allow adding more funcationality to `ActiveModel::Model` while
keeping backwards compatibility.
Co-authored-by: Nathaniel Watts <1141717+thewatts@users.noreply.github.com>
for ActiveRecord objects, it will force the entire attributes hash to
be constructed, which may include expensive deserialization.
ActiveModel already has a simple attribute_names method defined which
returns attributes.keys, exactly like the code we're replacing.
ActiveRecord overrides that method, and returns the names of the
attributes _without_ having to deserialize any values.
this change can save a lot of CPU if you have a serialized column
on a model that you often load from the DB because it's not worth
the effort to customize the SELECT on every relation, but
also rarely expose in any JSON serializations (i.e. have a default
except option for it).
* Without the change the new test fails like this:
Failure:
ActiveModel::TypeTest#test_registering_a_new_type [test/cases/type_test.rb:21]:
Expected: #<struct args={}>
Actual: #<struct args=nil>
* (*args, **kwargs)-delegation is not correct on Ruby 2.7 unless the
target always accepts keyword arguments (not the case for `Struct.new(:args).new`).
See https://eregon.me/blog/2021/02/13/correct-delegation-in-ruby-2-27-3.html
* Without the change the new test fails like this:
Failure:
ActiveModel::Type::RegistryTest#test_a_class_can_be_registered_for_a_symbol [test/cases/type/registry_test.rb:16]:
Expected: [{}, {}]
Actual: [nil, nil]
* (*args, **kwargs)-delegation is not correct on Ruby 2.7 unless the
target always accepts keyword arguments (not the case for `Array.new`).
See https://eregon.me/blog/2021/02/13/correct-delegation-in-ruby-2-27-3.html
Somehow it isn't caused on CI, but it consistently causes on locally.
```
% bin/test -w
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:85: warning: method redefined; discarding old validates_each
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:85: warning: previous definition of validates_each was here
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:89: warning: already initialized constant ActiveModel::Validations::ClassMethods::VALID_OPTIONS_FOR_VALIDATE
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:89: warning: previous definition of VALID_OPTIONS_FOR_VALIDATE was here
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:152: warning: method redefined; discarding old validate
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:152: warning: previous definition of validate was here
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:192: warning: method redefined; discarding old validators
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:192: warning: previous definition of validators was here
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:234: warning: method redefined; discarding old clear_validators!
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:234: warning: previous definition of clear_validators! was here
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:254: warning: method redefined; discarding old validators_on
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:254: warning: previous definition of validators_on was here
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:270: warning: method redefined; discarding old attribute_method?
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:270: warning: previous definition of attribute_method? was here
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:275: warning: method redefined; discarding old inherited
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:275: warning: previous definition of inherited was here
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:283: warning: method redefined; discarding old initialize_dup
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:283: warning: previous definition of initialize_dup was here
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:301: warning: method redefined; discarding old errors
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:301: warning: previous definition of errors was here
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:373: warning: method redefined; discarding old invalid?
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:373: warning: previous definition of invalid? was here
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:382: warning: method redefined; discarding old validate!
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:382: warning: previous definition of validate! was here
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:405: warning: method redefined; discarding old run_validations!
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:405: warning: previous definition of run_validations! was here
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:410: warning: method redefined; discarding old raise_validation_error
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:410: warning: previous definition of raise_validation_error was here
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:426: warning: method redefined; discarding old model
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:428: warning: method redefined; discarding old initialize
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:428: warning: previous definition of initialize was here
Traceback (most recent call last):
23: from bin/test:5:in `<main>'
22: from bin/test:5:in `require_relative'
21: from /Users/kamipo/src/github.com/rails/rails/tools/test.rb:18:in `<top (required)>'
20: from /Users/kamipo/src/github.com/rails/rails/railties/lib/rails/test_unit/runner.rb:40:in `run'
19: from /Users/kamipo/src/github.com/rails/rails/railties/lib/rails/test_unit/runner.rb:52:in `load_tests'
18: from /Users/kamipo/src/github.com/rails/rails/railties/lib/rails/test_unit/runner.rb:52:in `each'
17: from /Users/kamipo/src/github.com/rails/rails/railties/lib/rails/test_unit/runner.rb:52:in `block in load_tests'
16: from /Users/kamipo/src/github.com/rails/rails/railties/lib/rails/test_unit/runner.rb:52:in `require'
15: from /Users/kamipo/src/github.com/rails/rails/activemodel/test/cases/attributes_dirty_test.rb:5:in `<top (required)>'
14: from /Users/kamipo/src/github.com/rails/rails/activemodel/test/cases/attributes_dirty_test.rb:6:in `<class:AttributesDirtyTest>'
13: from /Users/kamipo/src/github.com/rails/rails/activemodel/test/cases/attributes_dirty_test.rb:7:in `<class:DirtyModel>'
12: from /Users/kamipo/src/github.com/rails/rails/activemodel/test/cases/attributes_dirty_test.rb:7:in `require'
11: from /Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/model.rb:3:in `<top (required)>'
10: from /Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/model.rb:59:in `<module:ActiveModel>'
9: from /Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/model.rb:62:in `<module:Model>'
8: from /Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/model.rb:62:in `require'
7: from /Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:436:in `<top (required)>'
6: from /Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:436:in `each'
5: from /Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:436:in `block in <top (required)>'
4: from /Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations.rb:436:in `require'
3: from /Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations/numericality.rb:5:in `<top (required)>'
2: from /Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations/numericality.rb:6:in `<module:ActiveModel>'
1: from /Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations/numericality.rb:7:in `<module:Validations>'
/Users/kamipo/src/github.com/rails/rails/activemodel/lib/active_model/validations/numericality.rb:8:in `<class:NumericalityValidator>': uninitialized constant ActiveModel::Validations::NumericalityValidator::Comparability (NameError)
```
This saves some array allocations from avoiding `*args`, as well
as makes the Method object `arity` and `parameters` correct.
e.g. before this patch, ArgumentError would be confusing:
```ruby
>> model.name_was(1)
ArgumentError: wrong number of arguments (given 2, expected 1)
```
ActiveRecord::Type::Registry doesn't need to inherit from
ActiveModel::Type::Registry, and it makes both classes more simple.
Co-authored-by: Adrianna Chang <adrianna.chang@shopify.com>
Since Ruby 2.7 `self.some_private_method` works fine.
So now that Ruby 2.7 is the minimal supported version,
`define_proxy_call` can always prepend `self.`
I used `type.cast(value)` to emulate unchecked serialized value in
`unboundable?`, since `RangeError` was raised only for the integer type,
so the emulation works enough for the integer type.
But since #41516, Enum types are also not always serializable, so
`type.cast(value)` may also be called for the enum types.
I've delegated `type.cast(value)` to the subtype if an unknown label is
passed to work the emulation even on Enum types in 3b6461b. But it is
strange to delegate to the subtype for the emulation only if an unknown
label is passed.
Instead of using `type.cast(value)` for the emulation, extend
`serializable?` to get unchecked serialized value if the value is not
serializable.
Since a `BindParam` object always has an attribute object as the value
in the Active Record usage, so `_insert_record`/`_update_record` could
be passed attribute set instead of wrapping casted value by a query
attribute.
RDoc Markup does not support backticks the way Markdown does to mark up
inline code. Additionally, `<tt>` must be used to mark up inline code
that includes spaces or certain punctuation characters (e.g. quotes).
These methods have changed in Ruby 2.5 to be more akin to grep:
https://bugs.ruby-lang.org/issues/11286
Using classes seems to be faster (and a bit more expressive) than iterating over
the collection items:
```
Warming up --------------------------------------
#all? with class 504.000 i/100ms
#all? with proc 189.000 i/100ms
Calculating -------------------------------------
#all? with class 4.960k (± 1.6%) i/s - 25.200k in 5.082049s
#all? with proc 1.874k (± 2.8%) i/s - 9.450k in 5.047866s
Comparison:
#all? with class: 4959.9 i/s
#all? with proc: 1873.8 i/s - 2.65x (± 0.00) slower
```
Benchmark script:
```ruby
require "minitest/autorun"
require "benchmark/ips"
class BugTest < Minitest::Test
def test_enumerators_with_classes
arr = (1..10000).to_a << nil
assert_equal arr.all?(Integer), arr.all? { |v| v.is_a?(Integer) }
Benchmark.ips do |x|
x.report("#all? with class") do
arr.all?(Integer)
end
x.report("#all? with proc") do
arr.all? { |v| v.is_a?(Integer) }
end
x.compare!
end
end
end
```
We allow for compare validations in NumericalityValidator, but these
only work on numbers. There are various comparisons people may want
to validate, from dates to strings, to custom comparisons.
```
validates_comparison_of :end_date, greater_than: :start_date
```
Refactor NumericalityValidator to share module Comparison with ComparabilityValidator
* Move creating the option_value into a reusable module
* Separate COMPARE_CHECKS which support compare functions and accept values
* Move odd/even checks to NUMBER_CHECKS as they can only be run on numbers
f72f743 introduces truncate(scale) in the Numericality validator.
This behaviour conflicts with AR decimal type conversion,
which uses round(scale) instead.
Changes the Numericality validator in order to use
round(scale) for consistency.
The ability has lost due to reverted #39321 in #41049.
We should allow updating with dirty locking value to work the documented
usage, but if casted value has no difference (i.e. regarded as no dirty),
identify the object by the original (uninitialized default) value.
In some cases, the framework was mutating the :if option of callbacks.
Since #38323, those options are frozen, so the framework could raise
exception when trying to mutate those options if they were being resued
with method like `with_options`.
It is a regression for 4cc438a1df.
`NumericalityValidator` basically takes the value before typecasting,
but `allow_nil` should work for the typecasted value for the
compatibility.
Fixes#40750.
This reverts commit d93a5d385e.
Revert "Revert "Remove unused internal methods in ActiveModel::Attributes""
This reverts commit 2d7967204e.
Reason: read_attribute was added in 6.1 as a performance optimization
and it is not needed anymore and write_attribute only existed to make
possible to call something that is not `attribute=` with send. We don't
need those methods internally and since they were never part of the
public API we can remove them.
There are validation cases in which the human_attribute_name depends on
other fields of the base class.
For instance, an Address model that depends on the selected country to
localize the attribute name to be shown in error messages. E.g. the
:address1 and :address2 attributes can be displayed as very different
strings depending on whether the address is in the US or in Japan.
The model global `read_attribute_for_validation` is not fit for some
validators (specifically for numericality validator).
To allow per-validator customization for attribute value, it extracts
`read_attribute_for_validation` in `EachValidator`.
This makes `datetime.serialize` about 10% faster.
```ruby
type = ActiveRecord::Type.lookup(:datetime)
time = Time.now.utc
Benchmark.ips do |x|
x.report("type.serialize(time)") do
type.serialize(time)
type.serialize(time)
type.serialize(time)
type.serialize(time)
end
end
```
Before:
```
Warming up --------------------------------------
type.serialize(time) 12.899k i/100ms
Calculating -------------------------------------
type.serialize(time) 131.293k (± 1.6%) i/s - 657.849k in 5.011870s
```
After:
```
Warming up --------------------------------------
type.serialize(time) 14.603k i/100ms
Calculating -------------------------------------
type.serialize(time) 145.941k (± 1.1%) i/s - 730.150k in 5.003639s
```
Follow up to c07dff7227.
Actually it is not the cop's fault, but we mistakenly use `^`, `$`, and
`\Z` in much places, the cop doesn't correct those conservatively.
I've checked all those usage and replaced all safe ones.
Instantiating attributes hash from raw database values is one of the
slower part of attributes.
Why that is necessary is to detect mutations. In other words, that isn't
necessary until mutations are happened.
`LazyAttributeHash` which was introduced at 0f29c21 is to instantiate
attribute lazily until first accessing the attribute (i.e.
`Model.find(1)` isn't slow yet, but `Model.find(1).attr_name` is still
slow).
This introduces `LazyAttributeSet` to instantiate attribute more lazily,
it doesn't instantiate attribute until first assigning/dirty checking
the attribute (i.e. `Model.find(1).attr_name` is no longer slow).
It makes attributes access about 35% faster for readonly (non-mutation)
usage.
https://gist.github.com/kamipo/4002c96a02859d8fe6503e26d7be4ad8
Before:
```
IPS
Warming up --------------------------------------
attribute access 1.000 i/100ms
Calculating -------------------------------------
attribute access 3.444 (± 0.0%) i/s - 18.000 in 5.259030s
MEMORY
Calculating -------------------------------------
attribute access 38.902M memsize ( 0.000 retained)
350.044k objects ( 0.000 retained)
15.000 strings ( 0.000 retained)
```
After (with `immutable_strings_by_default = true`):
```
IPS
Warming up --------------------------------------
attribute access 1.000 i/100ms
Calculating -------------------------------------
attribute access 4.652 (±21.5%) i/s - 23.000 in 5.034853s
MEMORY
Calculating -------------------------------------
attribute access 27.782M memsize ( 0.000 retained)
170.044k objects ( 0.000 retained)
15.000 strings ( 0.000 retained)
```
And benchmark with this branch for immutable string type:
```ruby
ActiveRecord::Schema.define do
create_table :users, force: true do |t|
t.string :name
t.string :fast_name
end
end
class User < ActiveRecord::Base
attribute :fast_name, :immutable_string
end
user = User.new
Benchmark.ips do |x|
x.report("user.name") do
user.name = "foo"
user.name_changed?
end
x.report("user.fast_name") do
user.fast_name = "foo"
user.fast_name_changed?
end
end
```
```
Warming up --------------------------------------
user.name 34.811k i/100ms
user.fast_name 39.505k i/100ms
Calculating -------------------------------------
user.name 343.864k (± 3.6%) i/s - 1.741M in 5.068576s
user.fast_name 384.033k (± 2.7%) i/s - 1.936M in 5.044425s
```
In Rails 4.2 we introduced mutation detection, to remove the need to
call `attribute_will_change!` after modifying a field. One side effect
of that change was that we needed to enforce that the
`_before_type_cast` form of a value is a different object than the post
type cast value, if the value is mutable. That contract is really only
relevant for strings, but it meant we needed to dup them.
In Rails 5 we added the `ImmutableString` type, to allow people to opt
out of this duping in places where the memory usage was causing
problems, and they don't need to mutate that field.
This takes that a step further, and adds a class-level setting to
specify whether strings should be frozen by default or not. The default
value of this setting is `false` (strings are mutable), and I do not
plan on changing that.
While I think that immutable strings by default would be a reasonable
default for new applications, I do not think that we would be able to
document the value of this setting in a place that people will be
looking when they can't figure out why some string is frozen.
Realistically, the number of applications where this setting is relevant
is fairly small, so I don't think it would make sense to ever enable it
by default.
Delegating to just one line method is to not be worth it.
Avoiding the delegation makes `read_attribute` about 15% faster.
```ruby
ActiveRecord::Schema.define do
create_table :users, force: true do |t|
t.string :name
end
end
class User < ActiveRecord::Base
def fast_read_attribute(attr_name, &block)
name = attr_name.to_s
name = self.class.attribute_aliases[name] || name
name = @primary_key if name == "id" && @primary_key
@attributes.fetch_value(name, &block)
end
end
user = User.create!(name: "user name")
Benchmark.ips do |x|
x.report("read_attribute('id')") { user.read_attribute('id') }
x.report("read_attribute('name')") { user.read_attribute('name') }
x.report("fast_read_attribute('id')") { user.fast_read_attribute('id') }
x.report("fast_read_attribute('name')") { user.fast_read_attribute('name') }
end
```
```
Warming up --------------------------------------
read_attribute('id') 165.744k i/100ms
read_attribute('name')
162.229k i/100ms
fast_read_attribute('id')
192.543k i/100ms
fast_read_attribute('name')
191.209k i/100ms
Calculating -------------------------------------
read_attribute('id') 1.648M (± 1.7%) i/s - 8.287M in 5.030170s
read_attribute('name')
1.636M (± 3.9%) i/s - 8.274M in 5.065356s
fast_read_attribute('id')
1.918M (± 1.8%) i/s - 9.627M in 5.021271s
fast_read_attribute('name')
1.928M (± 0.9%) i/s - 9.752M in 5.058820s
```
Redundant `to_s` has a few overhead. Especially private methods are not
intend to be passed user input directly so it should be passed always
string.
Removing redundant `to_s` makes attribute methods about 10% faster.
```ruby
ActiveRecord::Schema.define do
create_table :users, force: true do |t|
end
end
class User < ActiveRecord::Base
def fast_read_attribute(attr_name, &block)
@attributes.fetch_value(attr_name, &block)
end
end
user = User.create!
Benchmark.ips do |x|
x.report("user._read_attribute('id')") { user._read_attribute("id") }
x.report("user.fast_read_attribute('id')") { user.fast_read_attribute("id") }
end
```
```
Warming up --------------------------------------
user._read_attribute('id')
272.151k i/100ms
user.fast_read_attribute('id')
283.518k i/100ms
Calculating -------------------------------------
user._read_attribute('id')
2.699M (± 1.3%) i/s - 13.608M in 5.042846s
user.fast_read_attribute('id')
2.988M (± 1.2%) i/s - 15.026M in 5.029056s
```
For now, `increment` with aliased attribute does work, but `increment!`
with aliased attribute does not work, due to `clear_attribute_change` is
not aware of attribute aliases.
We sometimes partially updates specific attributes in dirties, at that
time it relies on `clear_attribute_change` to clear partially updated
attribute dirties. If `clear_attribute_change` is not attribute method
unlike others, we need to resolve attribute aliases manually only for
`clear_attribute_change`, it is a little inconvinient for me.
From another point of view, we have `restore_attributes`,
`restore_attribute!`, `clear_attribute_changes`, and
`clear_attribute_change`. Despite almost similar features
`restore_attribute!` is an attribute method but `clear_attribute_change`
is not.
Given the above, I'd like to promote `clear_attribute_change` as
attribute methods to fix issues caused by the inconsisteny.
This reverts 8538dfdc08, which broke the
activemodel-serializers-xml gem.
We can still get most of the benefit by applying the optimisation from
7b39197742 to empty hashes as well as nil.
This has the additional benefit of retaining the optimisation when the
user passes an empty options hash.
Since we're checking `serializable?` in the new `HomogeneousIn`
`serialize` will no longer raise an exception. We implemented
`unchecked_serialize` to avoid raising in these cases, but with some of
our refactoring we no longer need it.
I discovered this while trying to fix a query in our application that
was not properly serializing binary columns. I discovered that in at
least 2 of our active model types we were not calling the correct
serialization. Since `serialize` wasn't aliased to `unchecked_serialize`
in `ActiveModel::Type::Binary` and `ActiveModel::Type::Boolean` (I
didn't check others but pretty sure all the AM Types are broken) the SQL
was being treated as a `String` and not the correct type.
This caused Rails to incorrectly query by string values. This is
problematic for columns storing binary data like our emoji columns at
GitHub. The test added here is an example of how the Binary type was
broken previously. The SQL should be using the hex values, not the
string value of "🥦" or other emoji.
We still have the problem `unchecked_serialize` was supposed to fix -
that `serialize` shouldn't validate data, just convert it. We'll be
fixing that in a followup PR so for now we should use `serialize` so we
know all the values are going through the right serialization for their
SQL.
Since #31827, marshalling attributes hash format is changed to improve
performance because materializing lazy attribute hash is too expensive.
In that time, we had kept an ability to load from legacy attributes
format, since that performance improvement is backported to 5-1-stable
and 5-0-stable.
Now all supported versions will dump attributes as new format, the
backward compatibity should no longer be needed.
Rails has a monkey patch on `range.cover?` that is slower than Ruby's
`range.cover?`. We don't need Range support in this case because the SQL
creates a `BETWEEN` not an `IN` statement.
A coworker at GitHub found a few months back that if we used
`santitize_sql` over `where` when we knew the values going into `where`
it was a lot faster than `where`.
This PR adds a new Arel node type called `HomogenousIn` that will be
used when Rails knows the values are all homogenous and can therefore
pick a faster codepath. This new codepath skips some of the required
processing by `where` to make `wheres` with homogenous arrays faster
without requiring the application author to know when to use which query
type.
Using our benchmark code:
```ruby
ids = (1..1000).each.map do |n|
Post.create!.id
end
Benchmark.ips do |x|
x.report("where with ids") do
Post.where(id: ids).to_a
end
x.report("where with sanitize") do
Post.where(ActiveRecord::Base.sanitize_sql(["id IN (?)", ids])).to_a
end
x.compare!
end
```
Before this PR comparing where with a list of IDs to santitize sql:
```
Warming up --------------------------------------
where with ids 11.000 i/100ms
where with sanitize 17.000 i/100ms
Calculating -------------------------------------
where with ids 115.733 (± 4.3%) i/s - 583.000 in 5.045828s
where with sanitize 174.231 (± 4.0%) i/s - 884.000 in 5.081495s
Comparison:
where with sanitize: 174.2 i/s
where with ids: 115.7 i/s - 1.51x slower
```
After this PR comparing where with a list of IDs to santitize sql:
```
Warming up --------------------------------------
where with ids 16.000 i/100ms
where with sanitize 19.000 i/100ms
Calculating -------------------------------------
where with ids 158.293 (± 6.3%) i/s - 800.000 in 5.072208s
where with sanitize 169.141 (± 3.5%) i/s - 855.000 in 5.060878s
Comparison:
where with sanitize: 169.1 i/s
where with ids: 158.3 i/s - same-ish: difference falls within error
```
Co-authored-by: Aaron Patterson <aaron.patterson@gmail.com>
This deprecation is useless since the result is still an Error object
and there is no way to fix the code to remove the deprecation.
Let's just accept as a breaking change.
This method replaces the `keys` method on `errors` as a way to get the error attribute names from the errors object without treating `errors` like a hash.
This message more clearly communicates how to access the attribute and message keys that you would expect to get when using the previous API.
before you might iterate over errors like,
errors.each do |attribute, message|
# My error code here
end
This message helps the user find the methods on error that match the previous API.
Mark private constants
Display alternative for deprecation removal warning
Annotate Error's attributes
More emphasis on adding an error instead of message
Rewrite scaffold template using new errors API
Set first and last with behavior change deprecation
Update more doc and example
Add inspect for easier debugging
LazilyDefineAttributes only needs to stay in the model's ancestors long
enough to define accessors for acceptance attributes, after that it is
not needed anymore. To optimize this, we eagerly define attributes as
soon as either +method_missing+ or +respond_to_missing?+ is called, then
immediately remove those methods from the module so that future calls
will incur no performance penalty.
I've also removed the call to +define_attribute_methods+ here, which
is no longer be necessary to avoid an infinite loop.
non-database-backed attribute.
Writing to database-backed attributes after freezing an object would
raise FrozenError, but wouldn't raise FrozenError for user-defined
attributes.
Fixes#37208
This PR fixes a `FrozenError` when attempting to clear or delete
`ActiveModel::Errors` through the deprecated array methods. In
particular the error happens in the following situations:
# calling `clear` on the deprecated array
errors[:some_attribute].clear
# calling `delete` on the deprecated messages hash
errors.messages.delete(:some_attribute)
Following the recent introduction of `ActiveModel::Error`, this PR adds
deprecation warnings for those two messages.
When updating our app for 9def05385f, I
found several incorrectly configured validations that looked like this:
validates :name, uniqueness: true, case_sensitive: false
The intent is clearly for `case_sensitive: false` to be passed as an
option to the uniqueness validator, but instead it's being passed as
its own separate validation. Because the value `false` disables the
validation, the validator class isn't loaded and the failure is silent.
The validator should always be loaded, even if it's disabled, to ensure
it exists and avoid configuration errors like the one described above.
Symbols serialize to strings when persisted:
model.some_string = :foo
model.save! # "foo" is persisted
This PR updates the immutable string class to serialize symbols to strings to mirror this behavior as ActiveModel::Attribute calls this serialize method to determine the return value for changed_in_place? Prior to this change this code would report that "something" had changed:
comment = Comment.create! # (has a string "something" field)
comment.update_column :something, :anything # persists "anything" to the "something" field of the comments table
comment.something # or comment.attributes
comment.something_change # will be ["anything", "anything"], note these are `to` and `from` values, but are identical
After this PR the comment.something_change will return nil for this situation.
Fixes#36463
In most cases it works now without explicit require because it's accidentally required through
active_support/core_ext/date_and_time/calculations.rb where we still call `try`,
but that would stop working if we changed the Calculations implementation and remove the require call there.
- `AM::Error#to_h` was kind of broken before and would return in the
hash values a single error message.
```ruby
person = Person.new
person.errors.add(:name, "cannot be blank")
person.errors.add(:name, "too long")
puts person.errors.to_h # {name: 'too long'}
```
Since an attribute can have different errors, the previous behavior
didn't make much sense.
Now, `ActiveModel::Errors#to_hash` correctly returns an array of
error messages containing all the errors for an attribute.
However, one can easily be surprised by this change, so let's
deprecated it first.
- In ef4d3215b1 I made a change to
pass `AM::Error` object in case the arity of the block passed to
`each` accepted less than 2 arguments.
This is causing one issue for `to_h` as it expects the argument
passed to the block to be an Array (and were are passing it an
instance of `AM::Error`).
There is no real reason to use `to_h` anymore since `to_hash` exists
Deprecating `to_h` inf favor of `to_hash`
Co-Authored-By: Rafael França <rafael@franca.dev>
- `AM::Errors#each` is implemented for the `Enumerator` module and
get called indirectly by a bunch of method in the ruby land
(map, first, select ...)
These methods have a `-1` arity as they are written in C and they
wrongly trigger a deprecation warning.
This commit fixes that and correctectly return a `AM::Error` object
when `each` is called with a negative arity.
- Since `ActiveModel::Error` can now be inherited by
`ActiveModel::NestedError`, when the latter generates a
`full_message`, the `i18n_customize_full_message` accessor set in
the parent class is not set.
This commit fixes that by using a `class_attribute` instead.
- One regression introduced by the "AM errors as object" features is
about the `full_messages` method.
It's currently impossible to call that method if the `base` object
passed in the constructor of `AM::Errors` doesn't respond to the
`errors` method.
That's because `full_messages` now makes a weird back and forth trip
`AM::Errors#full_messages` -> `AM::Error#full_message` -> `AM::Errors#full_message`
Since `full_message` (singular) isn't needed by AM::Errors, I moved
it to the `AM::Error` (singular) class. This way we don't need to
grab the `AM::Errors` object from the base.
- `AM::Errors#delete` currently returns an empty array when trying
to delete an error that doesn't exist.
This behaviour is surprising and I think it would be better
to no return a truthy value but instead return nil like
`Hash#delete` does.
- When a ActiveRecord record get saved and validated as part of a
collection association, the errors attribute are changed to reflect
the children names. You end up with an error attribute that will
look like this:
`author.errors # {:'books.title' => [:blank]}`
2fe20cb55c/activerecord/lib/active_record/autosave_association.rb (L331-L340)
We then can't check if the `books.title` errors was added using
`ActiveModel::Errors#added?` because it tries to generate a message
to make the match and end up calling the "books.title" method
on the Author.
```
author.errors.added?(:'books.title', :blank) => NoMethodError: undefined method `books.title'
```
This patch modify the behaviour of `strict_match?` to not generate
a message to make the comparison but instead make a strict
comparison with the `options` from the error.
- In 86620cc3aa, a change was made
on how we remove error duplication on a record for autosave
association
This fix has one caveat where validation having a `if` / `unless`
options passed as a proc would be considered different.
Example:
```ruby
class Book < ApplicationRecord
has_one :author
validates :title, presence: true, if -> { true }
validates :title, presence: true, if -> { true }
end
Book.new.valid? # false
Book.errors.full_messages # ["title can't be blank", "title can't be blank"]
```
While this example might sound strange, I think it's better to
ignore `AM::Validations` options (if, unless ...) when making the
comparison.
Currently `type.serialize` and `connection.{quote|type_cast}` for a time
object always does `time.getutc` call regardless of whether it is
already utc time object or not, that duplicated proccess
(`connection.type_cast(type.serialize(time))`) allocates extra/useless
time objects for each type casting.
This avoids that redundant `time.getutc` call if it is already utc time
object. In the case of a model has timestamps (`created_at` and
`updated_at`), it avoids 6,000 time objects allocation for 1,000 times
`model.save`.
```ruby
ObjectSpace::AllocationTracer.setup(%i{path line type})
pp ObjectSpace::AllocationTracer.trace {
1_000.times { User.create }
}.select { |k, _| k[0].end_with?("quoting.rb", "time_value.rb") }
```
Before (c104bfe424):
```
{["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
203,
:T_ARRAY]=>[1004, 0, 778, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
220,
:T_STRING]=>[2, 0, 2, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
209,
:T_ARRAY]=>[8, 0, 8, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
57,
:T_ARRAY]=>[4, 0, 4, 1, 1, 0],
["~/rails/activemodel/lib/active_model/type/helpers/time_value.rb",
17,
:T_DATA]=>[4000, 0, 3096, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
120,
:T_DATA]=>[2000, 0, 1548, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
126,
:T_STRING]=>[4000, 0, 3096, 0, 1, 0]}
```
After (this change):
```
{["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
203,
:T_ARRAY]=>[1004, 0, 823, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
220,
:T_STRING]=>[2, 0, 2, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
209,
:T_ARRAY]=>[8, 0, 8, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
57,
:T_ARRAY]=>[4, 0, 4, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
126,
:T_STRING]=>[2000, 0, 1638, 0, 1, 0]}
```
This code takes the "plain" matcher with no prefix and suffix and puts
it at the end of the matchers array such that it is de-prioritized among
all matchers. The comment explaining this code, originally from commimt
8b8b7143ef dated in 2011, is in a context where detection from matchers
happened immediately. In that situation, the plain matcher would indeed
always match first and no others would ever be used.
However, the current code does not immediately detect one match but
rather maps matchers to matches for the method_name. Detection only
happens for matches whose attribute name is valid.
In this context, there is no need to bump the plain matcher to the end
of the array, since it will only be selected if the attribute name it
catpures matches a valid attribute name.
Currently, if `:datetime` type has a precision, type casting always does
round off subseconds regardless of whether necessary or not, it is a bit
slow.
Since #34970, `t.timestamps` with `precision: 6` by default, so
`pluck(:created_at)` in newly created app will always be affected by the
round off.
This avoids the round off if possible, it makes `pluck(:created_at)`
about 25% faster.
https://gist.github.com/kamipo/e029539f2632aee9f5e711fe66fc8842
Before (0a87d7c9dd with postgresql adapter):
```
Warming up --------------------------------------
users.pluck(:id) 344.000 i/100ms
users.pluck(:name) 336.000 i/100ms
users.pluck(:created_at) 206.000 i/100ms
Calculating -------------------------------------
users.pluck(:id) 3.620k (± 8.5%) i/s - 18.232k in 5.077316s
users.pluck(:name) 3.579k (± 9.4%) i/s - 17.808k in 5.020230s
users.pluck(:created_at) 2.069k (± 8.0%) i/s - 10.300k in 5.019284s
```
Before (0a87d7c9dd with mysql2 adapter):
```
Warming up --------------------------------------
users.pluck(:id) 245.000 i/100ms
users.pluck(:name) 240.000 i/100ms
users.pluck(:created_at) 180.000 i/100ms
Calculating -------------------------------------
users.pluck(:id) 2.548k (± 9.4%) i/s - 12.740k in 5.066574s
users.pluck(:name) 2.513k (± 8.0%) i/s - 12.480k in 5.011260s
users.pluck(:created_at) 1.771k (±11.2%) i/s - 8.820k in 5.084473s
```
After (this change with postgresql adapter):
```
Warming up --------------------------------------
users.pluck(:id) 348.000 i/100ms
users.pluck(:name) 357.000 i/100ms
users.pluck(:created_at) 254.000 i/100ms
Calculating -------------------------------------
users.pluck(:id) 3.628k (± 8.2%) i/s - 18.096k in 5.024748s
users.pluck(:name) 3.624k (±12.4%) i/s - 17.850k in 5.020959s
users.pluck(:created_at) 2.567k (± 7.0%) i/s - 12.954k in 5.081153s
```
After (this change with mysql2 adapter):
```
Warming up --------------------------------------
users.pluck(:id) 268.000 i/100ms
users.pluck(:name) 265.000 i/100ms
users.pluck(:created_at) 207.000 i/100ms
Calculating -------------------------------------
users.pluck(:id) 2.586k (±10.9%) i/s - 12.864k in 5.050546s
users.pluck(:name) 2.543k (±10.2%) i/s - 12.720k in 5.067726s
users.pluck(:created_at) 2.263k (±14.5%) i/s - 10.971k in 5.004039s
```
* Change the deprecation for Enumerating ActiveModel::Errors to Rails 6.1 instead of 6.0
* Changed the deprecation message for ActiveModel::Errors methods: slice, values, keys and to_xml
The current comment here is from 2011 and its original context has
changed completely. The plain matcher will not "match every time"
anymore since the code now filters all matchers, and only selects those
for which the captured attribute is valid.
To avoid confusion, I updated the comment. For more discussion, see:
https://github.com/rails/rails/pull/36005
Most of the time, these methods are called from actual methods defined
from columns in the schema, not from method_missing, so the current
wording is misleading.
The target of matchers is used in two contexts: to define attribute
methods which dispatch to handlers like attribute_was, and to match
method calls in method_missing and dispatch to those same handler
methods.
Only in the latter context does the term "method_missing_target" make
any sense; in the former context it is just confusing. "target" is not
ideal as a term but at least it avoids this confusion.
Currently, although using both dirty tracking (ivar backed and
attributes backed) on one model is not supported (doesn't fully work at
least), both dirty tracking are being performed, that is very slow.
As long as attributes backed dirty tracking is used, ivar backed dirty
tracking should not need to be performed.
I've refactored to extract new `ForcedMutationTracker` which only tracks
`force_change` to be performed for ivar backed dirty tracking, that
makes dirty tracking on Active Record 2x ~ 30x faster.
https://gist.github.com/kamipo/971dfe0891f0fe1ec7db8ab31f016435
Before:
```
Warming up --------------------------------------
changed? 4.467k i/100ms
changed 5.134k i/100ms
changes 3.023k i/100ms
changed_attributes 4.358k i/100ms
title_change 3.185k i/100ms
title_was 3.381k i/100ms
Calculating -------------------------------------
changed? 42.197k (±28.5%) i/s - 187.614k in 5.050446s
changed 50.481k (±16.0%) i/s - 246.432k in 5.045759s
changes 30.799k (± 7.2%) i/s - 154.173k in 5.030765s
changed_attributes 51.530k (±14.2%) i/s - 252.764k in 5.041106s
title_change 44.667k (± 9.0%) i/s - 222.950k in 5.040646s
title_was 44.635k (±16.6%) i/s - 216.384k in 5.051098s
```
After:
```
Warming up --------------------------------------
changed? 24.130k i/100ms
changed 13.503k i/100ms
changes 6.511k i/100ms
changed_attributes 9.226k i/100ms
title_change 48.221k i/100ms
title_was 96.060k i/100ms
Calculating -------------------------------------
changed? 245.478k (±16.1%) i/s - 1.182M in 5.015837s
changed 157.641k (± 4.9%) i/s - 796.677k in 5.066734s
changes 70.633k (± 5.7%) i/s - 358.105k in 5.086553s
changed_attributes 95.155k (±13.6%) i/s - 470.526k in 5.082841s
title_change 566.481k (± 3.5%) i/s - 2.845M in 5.028852s
title_was 1.487M (± 3.9%) i/s - 7.493M in 5.046774s
```
Markus Doits