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.
In `bootstrap.rb` we set the `Rails.logger.level` to `config.log_level`. But at this point, we may have already set up a `BroadcastLogger` with multiple broadcasts that have different levels. So, calling `level=` on the `BroadcastLogger` will overwrite the level of the individual broadcasts. So instead, let's only set the `Rails.logger.level` if the logger is not a `BroadcastLogger`.
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.
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
Right now we are using both to test the Rails applications we generate
and to test Rails itself. Let's keep CI for the app and BUILDKITE to
the framework.
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).
