require_dependency has now one single definition, and its implementation
does not depend on Zeitwerk or even applications. It only depends on
having some autoload paths in place.
We can test that in the AS test suite.
Without this fix, the delegation will raise the ArgumentError:
```
% bin/test -w test/per_thread_registry_test.rb
Running 1 tests in a single process (parallelization threshold is 50)
Run options: --seed 23992
# Running:
E
Error:
PerThreadRegistryTest#test_method_missing_with_kwargs:
ArgumentError: wrong number of arguments (given 1, expected 0; required keyword: x)
/Users/kamipo/src/github.com/rails/rails/activesupport/test/per_thread_registry_test.rb:9:in `foo'
/Users/kamipo/src/github.com/rails/rails/activesupport/lib/active_support/per_thread_registry.rb:55:in `foo'
/Users/kamipo/src/github.com/rails/rails/activesupport/lib/active_support/per_thread_registry.rb:57:in `method_missing'
/Users/kamipo/src/github.com/rails/rails/activesupport/test/per_thread_registry_test.rb:13:in `test_method_missing_with_kwargs'
```
Followup: https://github.com/rails/rails/pull/42833
The previous fix wasn't working in practice because the LocalCache
middleware doesn't use `with_local_cache` but directly set a
regular `LocalStore` instance in the registry.
So instead we redecorate it on every access. It may cause some extra
allocations, but since it only happens in 6.1 mode, it's not as
much of a concern.
The test for CurrentAttributes using thread-local variables leaks
an instance in Thread.current.
Usually instances are reset in the test helper but the objects remain:
activesupport/lib/active_support/current_attributes/test_helper.rb:11
In this situation we use clear_all to make sure we don't leave instances
behind when the configuration is changed.
assert_not_equal is an alias for refute_equal that is defined only on
the class ActiveSupport::TestCase. This commit ensures
ActiveSupport::Testing::Assertions#assert_changes doesn't depends on
ActiveSupport::TestCase to work.
Setting `remove_deprecated_time_with_zone_name` didn't work because
the value is checked in a framework initialiser, which runs before application initialiser.
This PR moves the setting inside the `config.after_initialize` block.
I've also added 2 tests for this behaviour.
Fixes#42820
Sometimes it can be very strange or long have to define default values
for config accessors as blocks, specially when config is a simple value
like 1, true, :symbol. This commit adds the ability to specify those
values by just passing a ` default` option when defining the accessor.
It also makes config accessor's interface similar to other Rails
methods like `class_attribute`, which also has the instance_reader,
instance_writer and instance_accessor options.
Since `LocalCache` store needs to call `dup_value!` on write (to avoid
mutating the original value), we end up serializing each cache entry twice.
Once for the local cache, and a second time for the actual backend. So the
write performance is quite bad.
So the idea here is than rather than to store `Entry` instances, the local
cache now instead store whatever payload was sent to the real backend.
This means that we now only serialize the `Entry` once, and if the cache
store was configured with an optimized coder, it will be used for the local
cache too.
Current Rails `main`:
```
fetch in rails 7.0.0.alpha
52.423 (± 1.9%) i/s - 265.000 in 5.058089s
write in rails 7.0.0.alpha
12.412 (± 0.0%) i/s - 62.000 in 5.005204s
```
Current Rails `main` with local cache disabled:
```
fetch in rails 7.0.0.alpha
52.047 (± 3.8%) i/s - 260.000 in 5.000138s
write in rails 7.0.0.alpha
25.513 (± 0.0%) i/s - 128.000 in 5.018942s
```
This branch:
```
fetch in rails 7.0.0.alpha
50.259 (± 4.0%) i/s - 255.000 in 5.085783s
write in rails 7.0.0.alpha
25.805 (± 0.0%) i/s - 130.000 in 5.039486s
```
So essentially, the local cache overhead on write has been eliminated.
Benchmark:
```
require "bundler/inline"
gemfile(true) do
source "https://rubygems.org"
gem "rails", github: "rails/rails", branch: "main"
gem 'benchmark-ips'
gem "mysql2"
gem "pry"
end
require "active_record"
require "logger"
require 'benchmark/ips'
ActiveRecord::Base.establish_connection(adapter: "mysql2", database: 'test', host: 'localhost', user: 'root', password: '')
ActiveRecord::Base.logger = Logger.new(nil)
ActiveRecord::Schema.define do
create_table :users, force: true do |t|
t.string :name
t.integer :phone
end
end
class ApplicationRecord < ActiveRecord::Base
self.abstract_class = true
end
class User < ApplicationRecord
end
1_000.times { |i| User.create(name: "test #{i}") }
cache = ActiveSupport::Cache::FileStore.new(ARGV[0] || '/tmp/rails-cache')
cache.clear
unless ENV['DISABLE_LOCAL_CACHE']
ActiveSupport::Cache::Strategy::LocalCache::LocalCacheRegistry.set_cache_for(
cache.middleware.local_cache_key,
ActiveSupport::Cache::Strategy::LocalCache::LocalStore.new
)
end
h = {}
h = User.last(Integer(ENV.fetch('SIZE', 1000))).each { |u| h[u.id] = u }
puts "== Benchmarking read_entry code and write_entry code in rails #{Rails.version}"
Benchmark.ips do |x|
x.report("fetch in rails #{Rails.version}") {
cache.fetch('key', compress: false) { h }
}
x.report("write in rails #{Rails.version}") {
cache.write("key+#{Time.now}", h, compress: false) { h }
}
end
```
This adds an additional method to configure the parallelization
threshold. Before this, the only way of configuring the threshold was
via an option:
```
config.active_support.test_parallelization_minimum_number_of_tests
```
- Related to https://github.com/rails/rails/pull/42761
- Used `parallelized?` method instead of calling a method `should_parallelize?` to figure out if parallezation is enabled.
- Fixed semantics of the test name corresponding to the change
- Updated test name as per the code review suggestion.
Parallelizing tests has a cost in terms of database setup and fixture
loading. This change makes Rails disable parallelization when the number
of tests is below a configurable threshold.
When running tests in parallel each process gets its own database
instance. On each execution, each process will update each database
schema (if needed) and load all the fixtures. This can be very expensive
for non trivial datasets.
As an example, for HEY, when running a single file with 18 tests,
running tests in parallel in my box adds an overhead of 13 seconds
versus not parallelizing them. Of course parallelizing is totally worthy
when there are many tests to run, but not when running just a few tests.
The threshold is configurable via
config.active_support.test_parallelization_minimum_number_of_tests,
which is 30 50 by default.
This also adds some tracing to know how tests are being executed:
When in parallel:
```
Running 2829 tests in parallel in 8 processes
```
When not in parallel:
```
Running 15 tests in a single process (parallelization threshold is 30)
```
Class variables are confusing and can be slow. In this case we were just
using the class variable as a global to implement Fiber-local variables
on top of. Instead we can use Thread#[] directly to store our Fiber-locals.
Follow up to https://github.com/rails/rails/pull/37313
- Adds regression tests
- Logs a warning in cases where assertions are nested in a way that's likely to be confusing
Date._iso8601 will attempt to parse certain args as ordinal dates eg. '21087' => 2021-03-28. Add ActiveSupport::TimeZone.iso8601 support for these
ordinal values to be consistent with Date._iso8601 and raise ArgumentError where the day of year is invalid.
Previously this function would fail to apply the negative format
correctly if the value being rounded was 0.5 exactly (modulo
precision).
This fixes the logic to use `>= 0.5` instead of `> 0.5`
This is a more-complete version of the fix in #37865 which originally
addressed #37846.
Also see context at #39350 with respect to alternate input formats.
Closes#42577
Same bug as https://github.com/rails/rails/pull/42559, but a very different fix due to https://github.com/rails/rails/pull/42025. To recap, the issue:
- While using the `dalli_store`, you set any value in the Rails cache with no expiry.
- You change to the `mem_cache_store`.
- You upgrade to Rails 7.
- You try to read the same cache key you set in step 1, and it crashes on read.
https://github.com/rails/rails/pull/42025 was backward compatible with entries written using the `mem_cache_store`, but *not* the `dalli_store` which did not use `ActiveSupport::Cache::Entry`. This PR attempts to fix that.
Previously calling ActiveSupport::Inflector::Inflections.clear(:acronyms)
reset the instance variable to an empty Array, which is not the correct
default. The next time an acronym is added a TypeError is thrown.
Because we re-use the message generated by the `Time` instance
it causes `TimeWithZone` to still pretend being a `Time` instance
even with `remove_deprecated_time_with_zone_name = true` which is
confusing.
Manually formats the seconds. This avoids using the %g formatter which can convert large numbers or very small fractions into scientific notation which is not supported by iso8601
Ruby master ships with Psych 4.0.0 which makes `YAML.load`
defaults to safe mode (https://github.com/ruby/psych/pull/487).
However since these YAML files are trustworthy sources
we can parse them with `unsafe_load`.
Reuse class previously defined in the same file to make it easy
to see the change of the output.
Also add the example not to skip callback with if option.
After trying out https://github.com/rails/rails/pull/42123
it appears that the bulk of the deprecations are for cases such as:
```ruby
output << some_helper
```
Where `some_helper` happens to sometime return `nil`.
While a regular String would indeed raise a `TypeError`
on such case, I wonder if it wouldn't be worth to special
case this specific scenario as it is quite harmless and would
drastically reduce the amount of deprecated code.
`codespell` works with a small custom dictionary and seems to find perhaps more spelling mistakes than `misspell` which really only fixes commonly misspelled English words.
Not all spell checkers can check all file types and most spell checkers can't find all the errors.
https://github.com/codespell-project/codespellhttps://pypi.org/project/codespell/
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 new Rails app, the generated credentials file contains an example of some [nested secrets](f95c0b7e96/railties/lib/rails/generators/rails/credentials/credentials_generator.rb (L46)).
Currently you can't access them nicely the way you would top level secrets:
```
2.7.3 :001 > Rails.application.credentials.aws
=> {:access_key_id=>123, :secret_access_key=>345}
2.7.3 :002 > Rails.application.credentials.aws.access_key_id
Traceback (most recent call last):
1: from (irb):2
NoMethodError (undefined method `access_key_id' for {:access_key_id=>123, :secret_access_key=>345}:Hash)
2.7.3 :003 > Rails.application.credentials.secret_key_base
=> "abcd..."
```
It would make secrets easier to use if you could, so this PR adds support for that. `Rails.application.credentials.aws.access_key_id` will now return `123` in the above example.
Active Support's cache have for long been limited because
of its format. It directly serialize its `Entry` object with
`Marshal`, so any internal change might break the format.
The current shortcommings are:
- The minimum entry overhead is quite ridiculous:
`Marshal.dump(ActiveSupport::Cache::Entry.new("")).bytesize # => 107`
- Only the internal `value` is compressed, but unless it's a String, to do so
it first need to be serialized. So we end up with `Marshal.dump(Zlib.deflate(Marshal.dump(value)))`
which is wasteful.
This can cause infinite recursions if you do have a
complex i18n backend that calls `Duration#inspect`.
There is not really any point making `inspect` internationalized
anyway, as most apps will have `:en` as default locale.
* Add `Enumerable#sole`, from `ActiveRecord::FinderMethods#sole`
* distinguish single-item Enumerable and two-item with nil last
Add a test for same.
* add symmetry, against rubocop's wishes
The local cache need to protect against mutations of both
the initial received value, and the value returned.
See:
- https://github.com/rails/rails/pull/36656
- https://github.com/rails/rails/pull/37587
Because of this, the overhead of the local cache end up being
quite significant. On a single read, the value will be deep duped
twice. So unless the value is one of the type benefiting from a
fast path, we'll have to do two `Marshal.load(Marshal.dump(value))`
roundtrips, which for large values can be very expensive.
By using a specialized `LocalEntry` type instead, we can store
the `Marshal` payload rather than the original value. This way
the overhead is reduced to a single `Marshal.dump` on writes
and a single `Marshal.load` on reads.
Running tests in parallel with processes wasn't failing fast when the
option was enabled. The problem was that even when the reporter raised
the `Interrupt` exception, the queue was not emptied, so workers keep
processing jobs as if nothing happened.
This patch basically intercepts the `Interrupt` exception that may
come from the reporter, and tells the server to clear the jobs queue,
so that it can continue the shutdown process as usual. The exception
must then continue its journey so that the backtrace is displayed.
By pushing the checks for TZInfo::Timezone and ActiveSupport::TimeZone
down into the case statement inside the `[]` method we can radically
simplify the logic inside of the `find_timezone!` method.
There's an edge case where passing an invalid argument to `[]` via the
`find_timezone!` method generates a slightly different message for the
`ArgumentError` exception and this is maintained in the unlikely case
someone was relying on the difference.
When utc_to_local_returns_utc_offset_times is false and the time
instance had fractional seconds the new UTC time instance was out
by a factor of 1,000,000 as the Time.utc constructor takes a usec
value and not a fractional second value.
When deep merging a Hash, first a duplicate is created. The duplicate only
does a shallow copy, so deeper level structures are not duplicated but
remain references.
This can lead to unexpected modifications of the original hash when a deeply
nested hash is merged with another hash, and the newly returned hash gets modified.
```
x = { a: { b: "foo" } }
y = { d: { e: "bar" } }
z = x.deep_merge(y)
# z => { a: { b: "foo" }, d: { e: "bar" } }
z[:a][:b] = "baz"
# z => { a: { b: "baz" }, d: { e: "bar" } }
# x => { a: { b: "baz" } }
```
All the complexity of that method was to work around various
problems caused by Ruby's constant lookup semantic as well
as the classic autoloader shortcommings.
Now that Rails require Ruby 2.7 I don't think we need anything
more than just `Object.const_get`.
```ruby
require 'benchmark/ips'
require 'active_support/all'
module Foo
module Bar
module Baz
end
end
end
def patched_constantize(name)
Object.const_get(name)
end
Benchmark.ips do |x|
x.report('orig') { ActiveSupport::Inflector.constantize("Foo::Bar::Baz") }
x.report('patched') { patched_constantize("Foo::Bar::Baz") }
x.compare!
end
```
```
Warming up --------------------------------------
orig 69.668k i/100ms
patched 391.385k i/100ms
Calculating -------------------------------------
orig 705.027k (± 1.9%) i/s - 3.553M in 5.041486s
patched 3.935M (± 1.1%) i/s - 19.961M in 5.072912s
Comparison:
patched: 3935235.5 i/s
orig: 705027.2 i/s - 5.58x (± 0.00) slower
```
The default behavior of the Psych gem for Ruby classes is to call the
`name` method to generate a tag for encoding. However this causes a
stream of deprecation warnings whenever ActiveSupport::TimeWithZone
instances are encoded into YAML. By utilising the load_tags/dump_tags
configuration we can prevent Psych from calling the `name` method and
thereby prevent the triggering of the deprecation warnings.
Rails now delegates autoloading to Zeitwerk, and therefore does not need to test
autoloading itself. Zeitwerk has test coverage, in Rails we only need to test
the integration.
We are gradually trimming AS::Dependencies, and the AS test suite. With the
removal of DependenciesTestHelpers and client code in af27a25, these fixtures
became orphan.
Three of them are left. They are to be autoloaded with Module#autoload because
they raise errors when the file is evaluated. Their current use cases are
already committed.
Sometime it can be useful to set a cache entry expiry
not relative to current time, but as an absolute timestamps,
e.g.:
- If you want to cache an API token that was provided to
you with a precise expiry time.
- If you want to cache something until a precise cutoff
time, e.g. `expires_at: Time.now.at_end_of_hour`
This leaves the `@created_at` variable in a weird state,
but this is to avoid breaking the binary format.
In c00f2d2 the `name` method was overridden to return 'Time' instead of
the real class name 'ActiveSupport::TimeWithZone'. The reasoning for
this is unclear and it can cause confusion for developers assuming that
name is returning the real class name. Since we don't know why this
was added, we're deprecating the method first to give developers a
chance to provide us with feedback and look to fix any issues that arise.
The rewritten test is not super clean with the manual cleanup etc.. If this is a
one-off it's not a big deal. However, if in subsequent rewrites I spot more
occurrences of this pattern, then I'll refactor.
Reverts a change from
2327ebfdc6
which can overwrite `test_order` that may have been manually set in
config. This can cause a situation where the user is depending on a
particular `test_order` but is unknowingly forced into another.
`case format when nil` is very efficient because it end up calling `NilClass === nil`
which pretty much translates to `nil.is_a?(NilClass)`.
On the other hand `format.nil?` benefit from a dedicated op code, so it's quite faster.
In this case `Integer#to_s` is much more often called without any arguments,
so it's worth optimizing for the most common case.
```ruby
class Integer
alias_method :faster_to_s, :to_s
end
require 'active_support/all'
require 'benchmark/ips'
module FasterNumericWithFormat
def faster_to_s(format = nil, options = nil)
if format.nil?
return super()
end
case format
when Integer, String
super(format)
when :phone
ActiveSupport::NumberHelper.number_to_phone(self, options || {})
when :currency
ActiveSupport::NumberHelper.number_to_currency(self, options || {})
when :percentage
ActiveSupport::NumberHelper.number_to_percentage(self, options || {})
when :delimited
ActiveSupport::NumberHelper.number_to_delimited(self, options || {})
when :rounded
ActiveSupport::NumberHelper.number_to_rounded(self, options || {})
when :human
ActiveSupport::NumberHelper.number_to_human(self, options || {})
when :human_size
ActiveSupport::NumberHelper.number_to_human_size(self, options || {})
when Symbol
super()
else
super(format)
end
end
end
Integer.prepend(FasterNumericWithFormat)
Benchmark.ips do |x|
x.report('orig no-arg') { 42.to_s }
x.report('fast no-arg') { 42.faster_to_s }
x.compare!
end
Benchmark.ips do |x|
x.report('orig :human') { 42.to_s(:human) }
x.report('fast :human') { 42.faster_to_s(:human) }
x.compare!
end
```
Ruby 2.7.2
```
Warming up --------------------------------------
orig no-arg 567.569k i/100ms
fast no-arg 692.636k i/100ms
Calculating -------------------------------------
orig no-arg 5.709M (± 1.3%) i/s - 28.946M in 5.070660s
fast no-arg 6.892M (± 0.7%) i/s - 34.632M in 5.024961s
Comparison:
fast no-arg: 6892287.7 i/s
orig no-arg: 5709450.0 i/s - 1.21x (± 0.00) slower
Warming up --------------------------------------
orig :human 575.000 i/100ms
fast :human 619.000 i/100ms
Calculating -------------------------------------
orig :human 6.176k (± 1.6%) i/s - 31.050k in 5.028656s
fast :human 6.179k (± 1.8%) i/s - 30.950k in 5.010372s
Comparison:
fast :human: 6179.1 i/s
orig :human: 6176.3 i/s - same-ish: difference falls within error
```
LogSubscriber overrides start/finish to avoid instrumenting when its
logger is nil. In order to support buffered notification events, as used
by async queries, we need to apply a similar override to
LogSubscriber#publish_event.
Date._iso8601 will return a hash for some values eg. '12936' but this will not contain the expected :mon and :mday keys.
Check for these keys and raise an ArgumentError if they aren't present as this is consistent with other invalid input behaviour.
OrderedHash is deprecated but there are some requires and references
to OrderedHash, which might be confusing.
As described in
https://github.com/rails/rails/issues/22681#issuecomment-166059717
OrderedHash is internal only so references in the docs should be
removed.
When requiring only "active_support/hash_with_indifferent_access",
calling `slice!` method on `HashWithIndifferentAccess` object
causes `NoMethodError`.
This is caused by `slice!` method calls `super` which is defined
in "active_support/core_ext/hash/slice" that' not required by this file.
Adding `require "active_support/core_ext/hash/slice"` to hwia
resolves this issue.
Note: since all tests `require_relative "abstract_unit"` that requires
"active_support/core_ext/hash/slice" eventually, it's pretty hard to
test method behavior without require.
Setting up the parallel workers could be an overhead when running
individual files.
This patch disables that process in case the number of files to run
is less than one.
Results running a sample file:
Before:
```
actionpack $ bin/test test/controller/parameters/accessors_test.rb
Run options: --seed 48261
........................................................................
Finished in 0.211923s, 339.7460 runs/s, 552.0873 assertions/s.
72 runs, 117 assertions, 0 failures, 0 errors, 0 skips
```
After
```
actionpack $ bin/test test/controller/parameters/accessors_test.rb
Run options: --seed 5461
........................................................................
Finished in 0.008411s, 8560.2189 runs/s, 13910.3557 assertions/s.
72 runs, 117 assertions, 0 failures, 0 errors, 0 skips
```
Xavier Noria