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>
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
```
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.
`with_test.rb` should define a class named `WithTest`. Instead, it
redefines `BlankTest`. This seems to be a copy/paste typo.
This commit renames `BlankTest` to `WithTest` to follow convention, and
to avoid any potential conflicts of 2 test files defining the same
class.
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.
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>
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.
It's possible since Rails 6 (3ea2857943) 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)`
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>
Sander Verdonschot