Now that we dropped support for Ruby 2.7, we no longer
need to check if variables are defined before accessing them
to avoid the undefined variable warning.
Now that we no longer support Ruby 2.7, many `ruby2_keyword` calls
can be eliminated.
The ones that are left could be eliminated but would end up substantially
slower or more compliacated so I left them for now.
When `parallelize` is used, and the ActiveSupport::Testing::ParallelizeExecutor
is being set, we can't call `parallelize_me!` ourselves since the
parallel executor might not be set to run tests in parallel.
In fact, maybe we should stop calling `parallelize_me!` altogether here
and rely on `parallelize` to do the right thing, but this might break
test classes that are using `ActiveSupport::Testing::Isolation` outside
without calling `parallelize`.
This changes the default number of workers when parallelizing tests to
be based on logical core count instead of physical core count. This may
provide a performance boost for parallelized tests.
This also reverts #50545 in favor of using the default worker count.
- tweak opening paragraph to specifically mention that the Redis backing
a Cache should not be the Redis backing Active Job
- add code blocks to Redis::Distributed since it uses the class syntax
- expand on ways to point Cache at a Redis in initialization
- add an rdoc link for #fetch and a code block for `-amount`
ActiveSupport::Deprecation used to `include Singleton` which is why the require is here, but it was removed in 9812641891f1c9ba4c3f8ffb8549ec26fc2668c4 (June 5 2023). Leaving the require seems to have been an oversight.
However, module ActiveJob::Serializers::ObjectSerializer does require Singleton and was relying on AS:Deprecation to require it. This PR just moves the require to the place it's actually used.
- Link to Deprecation::Behavior in configuring guide
The current list of options for `config.active_support.deprecation`
was missing the newly added `:report` option. Instead of adding the
missing option and continuing to keep 4 different lists of the same
options in sync, I opted to replace the list with a link to the
options in the Behavior API docs. This had the additional advantage of
giving more information about all of the options which was not
mentioned in the Configuring guide.
- Use symbols for Behavior options
It felt to me like naming the options did not make it explicit that
those were the symbols to pass to `#behavior=`, but by adding the `:`
that becomes more clear.
- Add some API links
There were a few references to `behavior=`, but we may as well link to
the actual method.
Fix: https://github.com/rails/rails/pull/50136
Using delegate saves on the `method_missing + public_send` combo.
I chose to delegate to all the methods I saw called in Rails own test
suite, but there is likely a handful more candidates for explicit delegation.
But also this rely on a new (private) parameter in `Module#delegate` to
provide the expected delegated type and use that to define delegators
with the extact required signature. This saves on array and hash allocations
caused by splatting.
In some ways it's a continuation of https://github.com/rails/rails/pull/46875
Note that I didn't make the new `as:` parameter public, as I fear it's
a bit too brittle to be used. For the same reason I'm considering reverting
the optimized path behavior on `to: :class` and requiring to explictly pass
`as: self` for that optimized path.
Co-Authored-By: Alexandre Terrasa <alexandre.terrasa@shopify.com>
This is a continuation of https://github.com/rails/rails/pull/46875
The behavior of looking up the class method when `to: :class` is passed
is a bit error prone because it silently degrades.
By passing the expected owner of the delegated method, we can be more
strict, and also generate a delegator in a module rather than having
to do it at inclusion time.
I made this argument private API because we want it in Rails, but
I'm worried it might be a bit too sharp for public API. I can
be convinced otherwise though.
method cause an Internal Server Error due to a TypeError.
In order to display the extraced source of a syntax error, we try
to locate the node id for the backtrace location. The call to
RubyVM::AbstractSyntaxTree.node_id_for_backtrace_location is expecting
a Thread::Backtrace::Location object, but we are passing a
SourceMapLocation instead.
This commit does two things:
1) it addresses the issue by making sure that we are always passing
a Thread::Backtrace::Location instead
2) it allows the development view to show the extracted source fragment
There is a bug in the current implementation of #travel_to:
it remembers a timezone of its argument, and all stubbed methods start
returning results in that remembered timezone. However, the expected
behaviour is to return results in a system timezone.
It can lead to bugs in tests like this one:
https://github.com/faker-ruby/faker/issues/2861
The CI env var was recently [changed][1] to be specific for the
application being tested instead of the framework tests. Because of
this, these lines which should have been true before are now false.
[1]: 1f0262aa2b768b14de5e6ef29d0e547628f570ee
Updates `MemoryStore#write_entry` to pass a `nil` `namespace` to
`exist?`, which expects a _name_ rather than a an already "normalized"
_key_. This fixes a bug where `unless_exist` would overwrite any
existing entry if a `namespace` was used.
Right now if the subprocess exit uncleanly, it straight out bring the
parent down with it because it will fail to parse the (likely empty)
Marshal payload:
```
<internal:marshal>:34:in `load': marshal data too short (ArgumentError)
from 3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:23:in `run'
from 3.3.0+0/gems/minitest-5.20.0/lib/minitest.rb:1094:in `run_one_method'
from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:179:in `block in run'
from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:168:in `with_timestamps'
from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:178:in `run'
from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:229:in `block in run_from_queue'
from 3.3.0+0/gems/ci-queue-0.38.0/lib/ci/queue/redis/worker.rb:55:in `poll'
from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:228:in `run_from_queue'
from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:213:in `__run'
from 3.3.0+0/gems/minitest-5.20.0/lib/minitest.rb:162:in `run'
from 3.3.0+0/gems/minitest-5.20.0/lib/minitest.rb:86:in `block in autorun'
- XXXX::XXXXTest#test_xxxxx - /tmp/bundle/ruby/3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:52:in `write': closed stream (IOError)
from 3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:52:in `puts'
from 3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:52:in `block (2 levels) in run_in_isolation'
from 3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:32:in `fork'
from 3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:32:in `block in run_in_isolation'
from 3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:28:in `pipe'
from 3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:28:in `run_in_isolation'
from 3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:19:in `run'
from 3.3.0+0/gems/minitest-5.20.0/lib/minitest.rb:1094:in `run_one_method'
from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:179:in `block in run'
from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:168:in `with_timestamps'
from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:178:in `run'
from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:229:in `block in run_from_queue'
from 3.3.0+0/gems/ci-queue-0.38.0/lib/ci/queue/redis/worker.rb:55:in `poll'
from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:228:in `run_from_queue'
from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:213:in `__run'
from 3.3.0+0/gems/minitest-5.20.0/lib/minitest.rb:162:in `run'
from 3.3.0+0/gems/minitest-5.20.0/lib/minitest.rb:86:in `block in autorun'
```
This breaks the Minitest contract that `run_one_method` shouldn't raise
ever, and return a `Minitest::Result`.
By properly checking the sub process status, we can turn this crash into
a test failure, allowing the original test process to go on.
It's a common useful pattern for situation where something isn't
supposed to happen, but if it does we can recover from it.
So in such situation you don't want such issue to be hidden
in development or test, as it's likely a bug, but do not want to
fail a request if it happens in production.
In other words, it behaves like `#record` in development and test
environments, and like `raise` in production.
Fix: https://github.com/rails/rails/pull/49638
Fix: https://github.com/rails/rails/pull/49339
Co-Authored-By: Andrew Novoselac <andrew.novoselac@shopify.com>
Co-Authored-By: Dustin Brown <dbrown9@gmail.com>
The return value was not specified before. Now it returns `true` on a
successful write, `nil` if there was an error talking to the cache
backend, and `false` if the write failed for another reason (e.g. the
key already exists and `unless_exist: true` was passed).
Fix: #50047
`Backtrace::Location` instance for code generated with eval always
have their `absolute_path` set to `nil`. So if absolute path is nil
we should fallback to checking `#path`.
Co-Authored-By: fatkodima <fatkodima123@gmail.com>
Previously, the `Cache::Store` instrumentation would call
`normalize_key` when adding a key to the log. However, this resulted in
the logged key not always matching the actual key written/read from the
cache:
```irb
irb(main):004> cache.write("foo", "bar", namespace: "baz")
D, [2023-11-10T12:44:59.286362 #169586] DEBUG -- : Cache write: baz:foo ({:compress=>false, :compress_threshold=>1024, :namespace=>"baz"})
=> true
irb(main):005> cache.delete("foo", namespace: "baz")
D, [2023-11-10T12:45:03.071300 #169586] DEBUG -- : Cache delete: foo
=> true
```
In this example, `#write` would correctly log that the key written to
was `baz:foo` because the `namespace` option would be passed to the
`instrument` method. However, other methods like `#delete` would log
that the `foo` key was deleted because the `namespace` option was _not_
passed to `instrument`.
This commit fixes the issue by making the caller responsible for passing
the correct key to `#instrument`. This allows `normalize_key` to be
removed from the log generation which both prevents the key from being
normalized a second time and removes the need to pass the full options
hash into `#instrument`.
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
On the first call it can be a bit slow because it needs to load
a bunch of data. It's also better to do it as part of boot
so that some of that data is shared via Copy-on-Write
Follow up to previous commit where these procs were allows to be shared
between subclasses. This change allocates the procs as they are first
declared.
Because `#present?` always resolve to `Object#present?`, it's
an extremely polymorphic method, and inline cache hits are low.
In addition, it requires an extra call to `self.blank?` which is
an overhead.
By specializing `present?` on common types, we avoid both of these
slow-downs:
```
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [arm64-darwin22]
Warming up --------------------------------------
present? 198.028k i/100ms
opt_present? 565.521k i/100ms
Calculating -------------------------------------
present? 2.087M (± 8.8%) i/s - 10.297M in 5.028398s
opt_present? 5.584M (± 8.6%) i/s - 27.711M in 5.023852s
Comparison:
present?: 2086621.6 i/s
opt_present?: 5584373.5 i/s - 2.68x faster
```
```
ruby 3.2.2 (2023-03-30 revision e51014f9c0) +YJIT [arm64-darwin22]
Warming up --------------------------------------
present? 819.792k i/100ms
opt_present? 1.047M i/100ms
Calculating -------------------------------------
present? 12.192M (± 8.8%) i/s - 60.665M in 5.050622s
opt_present? 16.540M (± 8.2%) i/s - 82.676M in 5.059029s
Comparison:
present?: 12192047.5 i/s
opt_present?: 16539689.6 i/s - 1.36x faster
```
```ruby
require 'bundler/inline'
gemfile do
source 'https://rubygems.org'
gem 'benchmark-ips'
gem 'activesupport'
end
require 'active_support/all'
class Object
def opt_present?
respond_to?(:empty?) ? !empty? : !!self
end
end
class NilClass
def opt_present?
false
end
end
class FalseClass
def opt_present?
false
end
end
class TrueClass
def opt_present?
true
end
end
class Array
def opt_present?
!empty?
end
end
class Hash
def opt_present?
!empty?
end
end
class Symbol
def opt_present?
!empty?
end
end
class String
def opt_present?
!blank?
end
end
class Numeric # :nodoc:
def opt_present?
true
end
end
class Time # :nodoc:
def opt_present?
true
end
end
array = []
hash = {}
time = Time.now
puts RUBY_DESCRIPTION
Benchmark.ips do |x|
x.report("present?") do
true.present?
false.present?
1.present?
1.0.present?
array.present?
hash.present?
:foo.present?
time.present?
end
x.report("opt_present?") do
true.opt_present?
false.opt_present?
1.opt_present?
1.0.opt_present?
array.opt_present?
hash.opt_present?
:foo.opt_present?
time.opt_present?
end
x.compare!(order: :baseline)
end
```
The `@` prefix is always stripped, so might as well not require it.
For backward compatibility reasons we still handle the prefix for now,
but we eagerly strip it and emit a deprecation.
Prior to this commit, `String#downcase_first` and `String#upcase_first`
would return a frozen string when called on an empty string:
```ruby
# BEFORE
"foo".downcase_first.frozen? # => false
"".downcase_first.frozen? # => true
# AFTER
"foo".downcase_first.frozen? # => false
"".downcase_first.frozen? # => false
```
For better or worse, the Rails guide settled on double quotes
and a large part of the community also use rubocop which enforce
them by default.
So we might as well try to follow that style when providing code
snippets in the documentation or error messages.
Fix: https://github.com/rails/rails/issues/49822
I certainly didn't get them all, but consistency should be significantly
improved.
For SEO purposes, there should only be one `<h1>` per page, and the edge
version of SDoc will provide one using the module name if none are
present in documentation.
The reason of removing dup 5 years ago in 6da99b4e99 was to decrease memory allocations, so, it makes sense to only dup options when we know they will be overwritten.
We had a few cases of tests being skipped accidentally on CI
hence not bein ran for a long time.
Skipping make sense when running the test suite locally, e.g.
you may not have Redis or some other dependency running.
But on CI, a test not being ran should be considered an error.
Previously in https://github.com/rails/rails/pull/43282, which shipped
with Rails 7.0, we added the guarantee that if an exception occurred
within an ActiveSupport::Notificaitons subscriber that the remaining
subscribers would still be run.
This was broken in https://github.com/rails/rails/pull/44469, where the
different types of subscribers were broken into groups by type for
performance. Although we would guard exceptions and allways run all (or
none) of the same group, that didn't work cross-group with different
types of subscriber.
Any classes that don't implement #blank? will fall back to Object's
implementation, which checks whether #empty? is defined before either
using #empty? or implicit truthiness. Since checking whether #empty? is
defined is expensive, some core classes (Array/Hash) alias #blank?
directly to #empty? to skip the respond_to? check.
This commit applies the alias optimization to Symbol. #blank? is called
on Symbols for many Active Record query methods (select, includes,
group, order, joins, etc.) so it seems reasonable to define the #blank?
alias on Symbol as well.
ActiveSupport::Callbacks often ended up with empty Arrays: on callbacks
without a conditional, and on callback sequences which had no before
and/or after callbacks.
Previously, callbacks which were shared with subclasses would not share
between them the procs generated in Filters::Before and Filters::After.
This was also lazily generated so would cause memory growth after an
application is booted (and not sharable between workers via CoW).
This was because we would rebuild the objects used to invoke the
callbacks via CallbackChain#compile, so any difference in the callback
chain would result in all of the callback procs being rebuilt.
This commit changes before and after callbacks (but not around!) to be
shared between all subclasses of where it was defined. This is done by
changing Filters::Before and Filters::After to plain classes which
respond to call instead of generating procs (which wasn't strictly
necessary but was easier to implement, and also results in simpler
objects which use less memory). These objects avoid referencing and tied
to a specific callback sequence and so can be memoized and reused.
This has the most impact on applications with many Controllers, and many
callbacks in the ApplicationController (or similar).
I also took this opportunity to merge together all the different forms
of procs generated (halting, halting_and_conditional, conditional,
simple) into one form with if statements. There isn't any significant
performance benefit from the specialization previously being done.
It used to be stdlib but is being extracted in modern rubies.
Overall its usefulness is dubious. In all cases it is included in
Rails, it's only for the `synchronize` method, but end up exposing
a dozen other useless methods.
In the end just using a Mutex is clearer and simpler.
In some cases we can even get away with a single mutex in a constant.
Data encoded using a non-String purpose and `use_message_serializer_for_metadata == false` was incorrectly decoded,
triggering a "mismatched purpose" error during decode.
Fix this by ensuring that we compare both sides as a String.
e.g.:
```
zzak@mbp16 railties % bin/test test/application/assets_test.rb
Run options: --seed 32028
.............../Users/zzak/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/mail-2.8.1/lib/mail/parsers/content_location_parser.rb:592: warning: assigned but unused variable - disp_type_s
I, [2023-10-15T10:38:33.005925 #97662] INFO -- : [0ca46786-853f-4740-9e71-f644c4893502] Started GET "/assets/demo.js" for 127.0.0.1 at 2023-10-15 10:38:33 +0900
E, [2023-10-15T10:38:33.006641 #97662] ERROR -- : [0ca46786-853f-4740-9e71-f644c4893502]
[0ca46786-853f-4740-9e71-f644c4893502] ActionController::RoutingError (No route matches [GET] "/assets/demo.js"):
[0ca46786-853f-4740-9e71-f644c4893502]
../Users/zzak/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/mail-2.8.1/lib/mail/parsers/content_location_parser.rb:592: warning: assigned but unused variable - disp_type_s
..../Users/zzak/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/mail-2.8.1/lib/mail/parsers/content_location_parser.rb:592: warning: assigned but unused variable - disp_type_s
I, [2023-10-15T10:38:33.973335 #97660] INFO -- : [fa848d82-0d80-4b23-925b-73fa5d0f47d4] Started GET "/posts?debug_assets=true" for 127.0.0.1 at 2023-10-15 10:38:33 +0900
I, [2023-10-15T10:38:33.975021 #97660] INFO -- : [fa848d82-0d80-4b23-925b-73fa5d0f47d4] Processing by PostsController#index as HTML
I, [2023-10-15T10:38:33.975209 #97660] INFO -- : [fa848d82-0d80-4b23-925b-73fa5d0f47d4] Parameters: {"debug_assets"=>"true"}
I, [2023-10-15T10:38:33.977560 #97660] INFO -- : [fa848d82-0d80-4b23-925b-73fa5d0f47d4] Completed 200 OK in 2ms (Views: 1.9ms | ActiveRecord: 0.0ms | Allocations: 1266)
......./Users/zzak/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/mail-2.8.1/lib/mail/parsers/content_location_parser.rb:592: warning: assigned but unused variable - disp_type_s
.
Finished in 4.842481s, 5.9887 runs/s, 23.3351 assertions/s.
29 runs, 113 assertions, 0 failures, 0 errors, 0 skips
```
For example:
https://buildkite.com/rails/rails/builds/94132#0186806f-8a23-4d07-ae8f-3b937f8517ae/1162-1171
See also #47484
Fix: https://github.com/rails/rails/pull/49563
The semantic_logger gems doesn't behave exactly like stdlib logger
in that `SemanticLogger#level` returns a Symbol while stdlib `Logger#level`
returns an Integer.
Because of this we can't simply compare integers, we have to use the
various `#{level}?` methods.
Currently, when a method is called on Rails.logger, the BroadcastLogger will find the loggers that will respond to that method. However, when the method has keyword arguments, they are passed as regular arguments and will throw an ArgumentError. This adds keyword argument support by double splatting hash args.
```
class CustomLogger
def foo(bar:)
true
end
end
Rails.logger.foo(bar: "baz")
```
Expected: `true`
Actual: `wrong number of arguments (given 1, expected 0) (ArgumentError)`
This commit fixes a discrepancy in the behavior of the `#increment` and
`#decrement` methods in `RedisCacheStore` when used with Redis versions less
than 7.0.0. The existing condition `count != amount` prevented setting the
Time-To-Live (TTL) for keys that were equal to the increment/decrement amount
after the `INCRBY`/`DECRBY` operation. This occurs when incrementing a
non-existent key by `1`, for example.
Using Redis pipelining, we minimize the network overhead incurred by checking
for existing TTLs. It decouples the TTL operations from the increment/decrement
operation, allowing the TTL to be set correctly regardless of the resulting
value from the `INCRBY`/`DECRBY`.
New tests have been added to verify the correct behavior of `#increment` and
`#decrement` methods, specifically when the `expires_in` option is not used.
Using a separate cache store instance (`@cache_no_ttl`), these tests ensure that
keys are correctly incremented or decremented and that their TTL remains unset.
Co-authored-by: Benjamin Quorning <benjamin@quorning.net>
Co-authored-by: Jury Razumau <jury.razumau@zendesk.com>
Co-authored-by: Edyta Rozczypała <edyta.rozczypala@zendesk.com>
It looks like #46305 accidentally removed the synchronize block that
would prevent a race conidition where two threads would read the same
value and only a single increment/decrement would take effect as they
would both write the same value.
ForkTracker has been autoloaded since [before][1] the [require][2] was
added.
[1]: 78b9580e5f3208c7048659de24f2220693afb23c
[2]: eba1534939fe1cf005746f12446235bdd52014c1
This also rewords occurrences of "made since the call to `start!` and
the call to `finish!`" to "made between the call to `start!` and the
call to `finish!`", for clarity.
When a `:method:` doc is immediately followed by the `private` keyword,
RDoc will hide that doc as if it were a private method.
To ensure that `ActiveSupport::MessageEncryptors#on_rotation` and
`ActiveSupport::MessageVerifiers#on_rotation` both appear in the
rendered docs, this commit adds a delimiter comment before each
`private` keyword.
The extra indentation on this list causes it to be rendered as code
instead of as an unordered list. But, furthermore, the items in this
list should be setter methods for `ActiveSupport::Cache::WriteOptions`,
not symbols. Since the `ActiveSupport::Cache::WriteOptions` class is
linked in the preceding paragraph, we can simply omit this list.
- This should make it easier for apps or libraries that were
previously relying on the private API.
Also took the opportunity to tweak the doc of the BroadcastLogger
to mention what happens when calling a non-standard method.
Fix#49494
Previously, calling `#to_proc` on `HashWithIndifferentAccess` object used inherited `#to_proc`
method from the `Hash` class, which was not able to access values using indifferent keys.
Fixes#48770.
`ENV` values are strings, so `ENV["RAILS_MAX_THREADS"]` must be parsed
as an int.
Unfortunately, `MessagePack::Factory::Pool::MemberPool` does not expose
a method to check its member count, so the most we can assert is that
roundtripping works as expected.
Fixes#49446.
This reverts commit 54f30488e190eea5e923fe51914051df0e8c33f6.
Reason: THis isn't necessary. `TZInfo::DataSource.get` makes sure
the exception gets translated.
It's possible since Rails 6 (3ea2857943dc294d7809930b4cc5b318b9c39577) to let the framework create Event objects, but the guides and docs weren't updated to lead with this example.
Manually instantiating an Event doesn't record CPU time and allocations, I've seen it more than once that people copy-pasting the example code get confused about these stats returning 0. The tests here show that - just like the apps I've worked on - the old pattern keeps getting copy-pasted.
- An oversight of #48615 is that it changes the `Rails.logger` to be
a broadcast logger after the app is booted. Anything referencing
`Rails.logger` during the boot process will get a simple logger and
ultimately resulting in logs not being broadcasted.
For example `ActionController::Base.logger.info("abc")` would
just output logs in the `development.log` file, not on STDOUT.
----
The only solution I could think of is to create a BroadcastLogger
earlier at boot, and add logger to that broadcast when needed (instead
of modiyfing the `Rails.logger` variable).
- ## Context
While working on https://github.com/rails/rails/pull/44695, I
realised that Broadcasting was still a private API, although it’s
commonly used. Rafael mentioned that making it public would require
some refactor because of the original implementation which was hard
to understand and maintain.
### Changing how broadcasting works:
Broadcasting in a nutshell worked by “transforming” an existing
logger into a broadcasted one.
The logger would then be responsible to log and format its own
messages as well as passing the message along to other logger it
broadcasts to.
The problem with this approach was the following:
- Heavy use of metaprogramming.
- Accessing the loggers in the broadcast wasn’t possible.
Removing a logger from the broadcast either.
- More importantly, modifying the main logger (the one that broadcasts
logs to the others) wasn’t possible and the main source of
misunderstanding.
```ruby
logger = Logger.new(STDOUT)
stderr_logger = Logger.new(STDER))
logger.extend(AS::Logger.broadcast(stderr_logger))
logger.level = DEBUG # This modifies the level on all other loggers
logger.formatter = … # Modified the formatter on all other loggers
```
To keep the contract unchanged on what Rails.logger returns, the new
BroadcastLogger class implement duck typing with all methods
that has the vanilla Ruby Logger class.
It's a simple and boring PORO that keeps an array of all the loggers
that are part of the broadcast and iterate over whenever a log is
sent.
Now, users can access all loggers inside the broadcast and modify
them on the fly. They can also remove any logger from the broadcast
at any time.
```ruby
# Before
stdout_logger = Logger.new(STDOUT)
stderr_logger = Logger.new(STDER)
file_logger = Logger.new(“development.log”)
stdout_logger.extend(AS::Logger.broadcast(stderr_logger))
stdout_logger.extend(AS::Logger.broadcast(file_logger))
# After
broadcast = BroadcastLogger.new(stdout_logger, stderr_logger, file_logger)
```
I also think that now, it should be more clear for users that the
broadcast sole job is to pass everything to the whole loggers in
the broadcast. So there should be no surprise that all loggers in
the broadcast get their level modified when they call
`broadcast.level = DEBUG` .
It’s also easier to wrap your head around more complex setup such
as broadcasting logs to another broadcast:
`broadcast.broadcast_to(stdout_logger, other_broadcast)`
The `concurrent` require was [added][1] previously because a
`Concurrent::Map` was added to hold all of the log levels by `Thread` id.
However, the `Map` was later [removed][2] by storing the log levels in the
`Thread` locals (and later in `IsolatedExecutionState`) instead. The new
implementation additionally removed the `cattr_accessor` (removing the
need to require the "attribute_accessors" core_ext) and also replaced
the [usage][3] of `Fiber` with `Thread` (so the require for `Fiber` is also no
longer necessary).
Since `Concurrent`, `Fiber`, and `cattr_accessor` are no longer used here, we
can remove the requires. Since `Logger` is used here (but was previously
required by `concurrent`), a require was added for it.
[1]: 629efb605728b31ad9644f6f0acaf3760b641a29
[2]: 2379bc5d2a7d9580f270eebfde87d9f94b3da6c9
[3]: 56ec504db6c130d448ffc1d68c9fdd95fdfc1130
Previously, #overlap? would incorrectly return true when one of the
ranges is effectively "empty":
```ruby
(2...2).overlap? 1..2 # => true
(1..2).overlap? 2...2 # => true
```
This is fixed in the Ruby 3.3 implementation of Range#overlap?, so this
commit fixes it for Ruby < 3.3 as well.
The tests added are from the Ruby repository and the implementation is
effectively a Ruby version of the fix in C.
Co-authored-by: Nobuyoshi Nakada <nobu@ruby-lang.org>
Co-authored-by: Shouichi Kamiya <shouichi.kamiya@gmail.com>
This commit uses Ruby 3.3 `Range#overlap?` that has been added to Ruby via https://github.com/ruby/ruby/pull/8242 .
Rails 7.1 renames `Range#overlaps?` to `Range#overlap?` via https://github.com/rails/rails/pull/48565 ,
This commit is not feasible to backport because there is no `Range#overlap?` in Rails 7.0.z
This commit addresses the CI faiilure at https://buildkite.com/rails/rails/builds/99745#018a9ea8-82f0-40a6-90c3-cdaa6dabebab/1092-1095
because without this commit, it shows `warning: method redefined; discarding old overlap?`.
```ruby
$ ruby -v
ruby 3.3.0dev (2023-09-16T05:57:19Z master e9b503f1bb) [x86_64-linux]
$ RAILS_STRICT_WARNING=true bundle exec ruby -w -Itest test/core_ext/range_ext_test.rb
/home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/core_ext/range/overlap.rb:7: warning: method redefined; discarding old overlap?
Running 46 tests in a single process (parallelization threshold is 50)
Run options: --seed 583
\# Running:
..............................................
Finished in 0.011672s, 3940.9670 runs/s, 4883.3722 assertions/s.
46 runs, 57 assertions, 0 failures, 0 errors, 0 skips
```
When `use_message_serializer_for_metadata` is false, the message
format should be enveloped in the same way it was in Rails 7.0
to make sure we don't have inconsistent formats.
Followup: https://github.com/rails/rails/pull/49108
DescendantsTracker need to work whether the `Class#descendants` core
ext is loaded or not. I missed that in the previous PR.
Either we are on a modern Ruby with `Class#subclass`, in which case
`DescendantsTracker#subclass` isn't defined, so we don't need the
filtering module.
Or we're on an old Ruby, and `DescendantsTracker.subclass` already does
the filtering.
`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.
XmlMini is currently being tested against libxml-ruby 4.0.0[^1], but the
doc comment for XmlMini says:
```
To use the much faster libxml parser:
gem 'libxml-ruby', '=0.9.7'
```
This comment seems to be very much out of date and could mislead users.
Fix by removing the version specifier so that the documentation simply
recommends:
```
To use the much faster libxml parser:
gem 'libxml-ruby'
XmlMini.backend = 'LibXML'
```
[^1]: 621bc68548/Gemfile.lock (L310)
When we're editing the contents of encrypted files, we should use the
`Tempfile` class because it creates temporary files with restrictive
permissions. This prevents other users on the same system from reading
the contents of those files while the user is editing them.
[CVE-2023-38037]
Since Rails 6.1 (via c4845aa7791839fcdf723dc77e3df258e7274496), it has
been possible to specify `coder: nil` to allow the store to handle cache
entries directly.
This commit adds documentation and regression tests for the behavior.
In the case that one single test file can't run with more than 1
parallel workers, but the base class has parallelization enabled, we
should still allow the user to set the number of workers to 1.
This adds a cache optimization such that expired and version-mismatched
cache entries can be detected without deserializing their values. This
optimization is enabled when using cache format version >= 7.1 or a
custom serializer.
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
`Rails.cache.delete('key')` is supposed to return `true` if an entry
exists and `false` otherwise. This is how most stores behave.
However, the `RedisCacheStore` would return a `1` when deleting an entry
that does exist and a `0` otherwise.
As `0` is truthy this is unexpected behaviour.
`RedisCacheStore` now returns true if the entry exists and false
otherwise, making it consistent with the other cache stores.
Similarly the `FileCacheStore` now returns `false` instead of `nil` if
the entry doesn't exist.
A test is added to make sure this behaviour applies to all stores.
The documentation for `delete` has been updated to make the behaviour
explicit.
This commit adds support for replacing the compressor used for
serialized cache entries. Custom compressors must respond to `deflate`
and `inflate`. For example:
```ruby
module MyCompressor
def self.deflate(string)
# compression logic...
end
def self.inflate(compressed)
# decompression logic...
end
end
config.cache_store = :redis_cache_store, { compressor: MyCompressor }
```
As part of this work, cache stores now also support a `:serializer`
option. Similar to the `:coder` option, serializers must respond to
`dump` and `load`. However, serializers are only responsible for
serializing a cached value, whereas coders are responsible for
serializing the entire `ActiveSupport::Cache::Entry` instance.
Additionally, the output from serializers can be automatically
compressed, whereas coders are responsible for their own compression.
Specifying a serializer instead of a coder also enables performance
optimizations, including the bare string optimization introduced by cache
format version 7.1.
`ActiveSupport::Cache::UNIVERSAL_OPTIONS` already defines the list of
base options, and it includes option aliases such as `:expire_in` and
`:expired_in`. Thus, using `UNIVERSAL_OPTIONS` allows `RedisCacheStore`
to support these aliases.
Follow-up to #48449.
Since #48449 changed the list of accepted cache format versions back to
just `6.1`, `7.0`, and `7.1`, we can raise a more specific error.
Using [] or the dynamic accessors don't result in the same value because
`[]` is delegated to `config` (the decrypted deserialized YAML), whereas
`[]=` and the dynamic accessors are delegated to `options`, an
ActiveSupport::OrderedOptions instance.
When development tools try to load Rails components, they sometimes end up loading files that will error out since a dependency is missing. In these cases, the tooling can catch the error and change its behaviour.
However, since the warning is printed directly to `$stderr`, the tooling cannot catch and suppress it easily, which ends up causing noise in the output of the tool.
This change makes Rails print these warnings using `Kernel#warn` instead, which can be suppressed by the tooling.
To use a binary-encoded string as a byte buffer, appended strings should
be force-encoded as binary. Otherwise, appending a non-ASCII-only
string will raise `Encoding::CompatibilityError`.
Fixes#48748.
