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]
