| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| | |
|
| |\
| |
| | |
Raise ArgumentError if an unrecognised callback is skipped
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
At present, if you skip a callback that hasn't been defined,
activesupport callbacks silently does nothing. However, it's easy to
mistype the name of a callback and mistakenly think that it's being
skipped, when it is not.
This problem even exists in the current test suite.
CallbacksTest::SkipCallbacksTest#test_skip_person attempts to skip
callbacks that were never set up.
This PR changes `skip_callback` to raise an `ArgumentError` if the
specified callback cannot be found.
|
| |\ \
| |/
|/| |
Fix AS::Callbacks raising an error when `:run` callback is defined.
|
| | |
| |
| |
| | |
This reverts commit 796cab45561fce268aa74e6587cdb9cae3bb243e.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
When Active Record calls `set_callback` inside `after_commit`,
[these lines of code](https://github.com/rails/rails/blob/master/activerecord/lib/active_record/transactions.rb#L276)
pass an **array** of methods as the `:if` condition:
```ruby
options[:if] = Array(options[:if])
options[:if] << "transaction_include_any_action?(#{fire_on})"
```
That made me realize that anyone could pass an **array** of `:if` and `:unless`
conditions to `set_callback`, since Active Support transforms these conditions
into an array anyways in [these lines of code](https://github.com/rails/rails/blob/master/activesupport/lib/active_support/callbacks.rb#L365):
```ruby
@if = Array(options[:if])
@unless = Array(options[:unless])
```
Long story short, this commit updates the documentation of the `set_callback`
method to explain that arrays are also accepted.
It also replaces +false+ and +true+ with false and true, since any _falsey_ or
_truthy_ value will work.
[ci skip]
|
| |/ |
|
| |
|
|
|
|
| |
`CallbackSequence#call` can only ever take one argument. Using `*args`
here produces unnecessary array allocations. Since it only ever takes
one argument we should use `arg` instead of `*args`.
|
| |\ |
|
| | | |
|
| | |
| |
| |
| | |
ActiveSupport::Callbacks::Filters::Around
|
| | |
| |
| |
| |
| | |
Also remove the default value since it will be always passed and
Array(nil) returns an empty array
|
| |/
|
|
| |
Renamed it to indicate what it actually does.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This stems from [a comment](rails#17227 (comment)) by @dhh.
In summary:
* New Rails 5.0 apps will not accept `return false` as a way to halt callback chains, and will not display a deprecation warning.
* Existing apps ported to Rails 5.0 will still accept `return false` as a way to halt callback chains, albeit with a deprecation warning.
For this purpose, this commit introduces a Rails configuration option:
```ruby
config.active_support.halt_callback_chains_on_return_false
```
For new Rails 5.0 apps, this option will be set to `false` by a new initializer
`config/initializers/callback_terminator.rb`:
```ruby
Rails.application.config.active_support.halt_callback_chains_on_return_false = false
```
For existing apps ported to Rails 5.0, the initializers above will not exist.
Even running `rake rails:update` will not create this initializer.
Since the default value of `halt_callback_chains_on_return_false` is set to
`true`, these apps will still accept `return true` as a way to halt callback
chains, displaying a deprecation warning.
Developers will be able to switch to the new behavior (and stop the warning)
by manually adding the line above to their `config/application.rb`.
A gist with the suggested release notes to add to Rails 5.0 after this
commit is available at https://gist.github.com/claudiob/614c59409fb7d11f2931
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
After this commit, returning `false` in a callback will display a deprecation
warning to make developers aware of the fact that they need to explicitly
`throw(:abort)` if their intention is to halt a callback chain.
This commit also patches two internal uses of AS::Callbacks (inside
ActiveRecord and ActionDispatch) which sometimes return `false` but whose
returned value is not meaningful for the purpose of execution.
In both cases, the returned value is set to `true`, which does not affect the
execution of the callbacks but prevents unrequested deprecation warnings from
showing up.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
This commit changes arguments and default value of CallbackChain's :terminator
option.
After this commit, Chains of callbacks defined **without** an explicit
`:terminator` option will be halted as soon as a `before_` callback throws
`:abort`.
Chains of callbacks defined **with** a `:terminator` option will maintain their
existing behavior of halting as soon as a `before_` callback matches the
terminator's expectation. For instance, ActiveModel's callbacks will still
halt the chain when a `before_` callback returns `false`.
|
| | |
|
| |
|
|
|
|
|
|
|
|
| |
`define_callbacks` from `ActiveSupport::Callbacks` accepts the
`:skip_after_callbacks_if_terminated` option since #4866 but the option
is not tested anywhere.
This commit adds tests and fixes documentation for the option, making it clear
that halting a callback chain only stops following `before_` and `around_`
callbacks by default.
|
| |
|
|
| |
This will avoid naming clash with user defined methods
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Inspired by @tenderlove's work in
c363fff29f060e6a2effe1e4bb2c4dd4cd805d6e, this reduces the number of
strings allocated when running callbacks for ActiveRecord instances. I
measured that using this script:
```
require 'objspace'
require 'active_record'
require 'allocation_tracer'
ActiveRecord::Base.establish_connection adapter: "sqlite3",
database: ":memory:"
ActiveRecord::Base.connection.instance_eval do
create_table(:articles) { |t| t.string :name }
end
class Article < ActiveRecord::Base; end
a = Article.create name: "foo"
a = Article.find a.id
N = 10
result = ObjectSpace::AllocationTracer.trace do
N.times { Article.find a.id }
end
result.sort.each do |k,v|
p k => v
end
puts "total: #{result.values.map(&:first).inject(:+)}"
```
When I run this against master and this branch I get this output:
```
pete@balloon:~/projects/rails/activerecord$ git checkout master
M Gemfile
Switched to branch 'master'
pete@balloon:~/projects/rails/activerecord$ bundle exec ruby benchmark_allocation_with_callback_send.rb > allocations_before
pete@balloon:~/projects/rails/activerecord$ git checkout remove-dynamic-send-on-built-in-callbacks
M Gemfile
Switched to branch 'remove-dynamic-send-on-built-in-callbacks'
pete@balloon:~/projects/rails/activerecord$ bundle exec ruby benchmark_allocation_with_callback_send.rb > allocations_after
pete@balloon:~/projects/rails/activerecord$ diff allocations_before allocations_after
39d38
<
{["/home/pete/projects/rails/activesupport/lib/active_support/callbacks.rb",
81]=>[40, 0, 0, 0, 0, 0]}
42c41
< total: 630
---
> total: 590
```
In addition to this, there are two micro-optimizations present:
* Using `block.call if block` vs `yield if block_given?` when the block was being captured already.
```
pete@balloon:~/projects$ cat benchmark_block_call_vs_yield.rb
require 'benchmark/ips'
def block_capture_with_yield &block
yield if block_given?
end
def block_capture_with_call &block
block.call if block
end
def no_block_capture
yield if block_given?
end
Benchmark.ips do |b|
b.report("block_capture_with_yield") { block_capture_with_yield }
b.report("block_capture_with_call") { block_capture_with_call }
b.report("no_block_capture") { no_block_capture }
end
pete@balloon:~/projects$ ruby benchmark_block_call_vs_yield.rb
Calculating -------------------------------------
block_capture_with_yield
124979 i/100ms
block_capture_with_call
138340 i/100ms
no_block_capture 136827 i/100ms
-------------------------------------------------
block_capture_with_yield
5703108.9 (±2.4%) i/s - 28495212 in 4.999368s
block_capture_with_call
6840730.5 (±3.6%) i/s - 34169980 in 5.002649s
no_block_capture 5821141.4 (±2.8%) i/s - 29144151 in 5.010580s
```
* Defining and calling methods instead of using send.
```
pete@balloon:~/projects$ cat benchmark_method_call_vs_send.rb
require 'benchmark/ips'
class Foo
def tacos
nil
end
end
my_foo = Foo.new
Benchmark.ips do |b|
b.report('send') { my_foo.send('tacos') }
b.report('call') { my_foo.tacos }
end
pete@balloon:~/projects$ ruby benchmark_method_call_vs_send.rb
Calculating -------------------------------------
send 97736 i/100ms
call 151142 i/100ms
-------------------------------------------------
send 2683730.3 (±2.8%) i/s - 13487568 in 5.029763s
call 8005963.9 (±2.7%) i/s - 40052630 in 5.006604s
```
The result of this is making typical ActiveRecord operations slightly faster:
https://gist.github.com/phiggins/e46e51dcc7edb45b5f98
|
| |
|
|
|
|
| |
A minor version breakage due to a rewrite of the callbacks code now
requires an explicit block to be passed to #set_callback. This amends
the documentation.
|
| |
|
| |
This has changed since around 2b1500d6
|
| | |
|
| |
|
| |
In some cases run_callbacks will return nil. I'm attempting to update the documentation for the method to clarify.
|
| |
|
|
|
| |
This deprecation was released in 4.1.0 and can be removed for 4.2.0,
deprecation message / handling is no longer necessary.
|
| |
|
|
| |
Use if/else instead of unless/else so conditional reads better.
|
| |\
| |
| | |
Active support callback's before/after/around filters are not correctly making their singleton methods private
|
| | | |
|
| | | |
|
| |\ \
| | |
| | | |
typos rectified lifecycle => life cycle
|
| | | | |
|
| |/ / |
|
| |/ |
|
| |
|
|
|
|
|
|
| |
This reverts commit d108672dada7ba97d3b3b56f0c6001cea621061e.
Conflicts:
activesupport/CHANGELOG.md
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In #11195 I noticed a trailing comma in the docs, but I decided to
further clean it up.
What I have done:
* Clean up the trailing comma in the docs and some extra whitespace
lines.
* Used `Array#extract` options to factor the repetitive pattern of
`args.last.is_a(Hash) ? ...`
* Renamed the local var `config` to `options` in `define_callbacks`, as
`options` seems to be the de facto name for the options objects.
* Renamed the local var `l` to `line` in `define_callback` (maybe it
meant `lambda` in the context) as single `l` may look like `1` in some
fonts.
|
| |
|
|
|
|
|
| |
_ Rename the define_callbacks params to `names`
- in order to match the naming conventions for `get_callbacks` and `set_callbacks` at https://github.com/rails/rails/blob/master/activesupport/lib/active_support/callbacks.rb#L736-743
- `define_callbacks` just register names(events), not define the real callback functions.
- Rename the `reset_callbacks` params
|
| |
|
|
| |
The change is commited at ba552764344bc0a3c25b8576ec11f127ceaa16da
|
| | |
|
| |
|
|
|
| |
- eval'ed to eval'd in accordance with https://github.com/rails/rails/pull/10889
- tried to improve statement about compiling Procs into methods using define_method
|
| | |
|
| |
|
|
|
|
| |
the"
This reverts commit 55975c71ec9c2c18b67020484959ff5c69d4d3fb.
|
| |
|
|
| |
class level
|
| | |
|
| | |
|
| | |
|
| | |
|
