| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
| |
As part of #19029, in future `skip_before_action`, `skip_after_action` and
`skip_around_action` will raise an ArgumentError if the specified
callback does not exist. `skip_action_callback` calls all three of these
methods and will almost certainly result in an ArgumentError. If anyone
wants to remove all three callbacks then they can still call the three
individual methods. Therefore let's deprecate `skip_action_callback` now
and remove it when #19029 is merged.
|
| |
|
| |
|
|
|
|
| |
ActionDispatch::Request#request_id
|
|\
| |
| | |
Don't use shorthand match on routes with inappropriate symbols
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Shorthand route match is when controller and action are taken literally from path.
E.g.
get '/foo/bar' # => will use 'foo#bar' as endpoint
get '/foo/bar/baz' # => will use 'foo/bar#baz' as endpoint
Not any path with level two or more of nesting can be used as shortcut.
If path contains any characters outside of /[\w-]/ then it can't be
used as such.
This commit ensures that invalid shortcuts aren't used.
':controller/:action/postfix' - is an example of invalid shortcut
that was previosly matched and led to exception:
"ArgumentError - ':controller/:action' is not a supported controller name"
|
|\ \
| | |
| | | |
Allow `method: "all"` as a valid routing test option
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
This allows the test to mirror the production code, since `via: :all` is
a valid option. The behavior in 4.1 did not actually test that it
matched all verbs, but instead defaulted to testing for "GET". This
implementation aims to better handle the intention of passing "all".
What will actually be asserted doesn't quite match up with the generated
route, since it appears to just not create a constraint on the method.
However, I don't think that we can easily test the lack of that
constraint. Testing each of the main 4 HTTP verbs seems to be a
reasonably close approximation, which should be sufficient for our
needs.
Fixes #18511.
|
| | | |
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
This has 2 effects:
1. RoutesProxy is CRAZY faster because it's no longer creating a new
Module each time method_missing is hit.
2. It bypasses an existing bug in ruby that makes `class << obj` unsafe
to be used in threading contexts.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Moves `X-Request-ID`, `action_dispatch.request_id` and
`HTTP_X_REQUEST_ID` strings to constants and freezes them.
We are freezing these strings to reduce the number of allocations in
Rails integration tests. The tests are spending a lot of time in GC and
this reduces the amount of time spent from 12% to 9% (in combination
with Rack PR that also freezes some strings).
Number of allocations before this change: 1030722
Number of allocations after this change: 967722
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
It's unnecessary to call `#clean_path_info`. It doesn't need to be
called on the path with each extension. This reduces allocations to
`Rack::Utils` in integration tests.
Before `#clean_path_info` from `Rack::Utils` (line 622) was number 2
in top 5 allocations:
```
[["rack/lib/rack/utils.rb", 499, :T_STRING], [51034, 4539, 71559, 0, 12, 1791120]]
[["rack/lib/rack/utils.rb", 662, :T_STRING], [33012, 0, 27930, 0, 1, 1226009]]
[["rails/activesupport/lib/active_support/notifications/fanout.rb", 55, :T_DATA], [29998, 0, 25380, 0, 1, 3230600]]
[["rails/activesupport/lib/active_support/subscriber.rb", 99, :T_STRING], [29996, 0, 25378, 0, 2, 1113840]]
[["rails/activesupport/lib/active_support/notifications/instrumenter.rb", 52, :T_HASH], [29994, 147, 27014, 0, 11, 4897784]]
```
After `#clean_path_info` from `Rack::Utils` (line 622) does not appear
in the top 5 highest allocations:
```
[["rack/lib/rack/utils.rb", 499, :T_STRING], [47617, 2414, 68969, 0, 12, 1667360]]
[["rack/lib/rack/body_proxy.rb", 34, :T_ARRAY], [28230, 0, 26060, 0, 1, 1046800]]
[["rails/activesupport/lib/active_support/notifications/fanout.rb", 55, :T_DATA], [28208, 0, 26042, 0, 1, 3034096]]
[["rails/activesupport/lib/active_support/subscriber.rb", 99, :T_STRING], [28204, 0, 26040, 0, 1, 1046080]]
[["rails/activesupport/lib/active_support/callbacks.rb", 165, :T_DATA], [28200, 0, 26046, 0, 2, 3451800]]
```
|
| | |
| | |
| | |
| | |
| | | |
This approach will avoid us to check for NoMethodError when trying to
decode
|
|\ \ \
| | | |
| | | |
| | | | |
Handle non-string authenticity tokens
|
| | | |
| | | |
| | | |
| | | | |
Non-string authenticity tokens raised NoMethodError when decoding the
masked token.
|
| | | | |
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Add http_cache_forever to ActionController, so we can cache results
forever.
Things like static pages are a good candidate for this type of caching.
This cache only controls caching headers, so it is up to the browser to
cache those requests.
|
| | | | |
|
| | | | |
|
| | | | |
|
| | | | |
|
| | | | |
|
| | | |
| | | |
| | | |
| | | |
| | | | |
there's really no benefit here. It's the same number of lines without
the meta programming and is faster
|
| | | | |
|
|/ / /
| | |
| | |
| | | |
now we don't have to call reset! everywhere
|
| | | |
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
We shouldn't cache if it's not absolutely necessary. Removes
route caching and instead skips using the `url_helpers` is the
integration test session doesn't require it. Benchmark ips on
integration and controller index method tests below.
Without any caching or changes to `#url_helpers`:
```
Calculating -------------------------------------
INDEX: Integration Test
71.000 i/100ms
INDEX: Functional Test
99.000 i/100ms
-------------------------------------------------
INDEX: Integration Test
728.878 (± 8.0%) i/s - 3.692k
INDEX: Functional Test
1.015k (± 6.7%) i/s - 5.148k
Comparison:
INDEX: Functional Test: 1015.4 i/s
INDEX: Integration Test: 728.9 i/s - 1.39x slower
```
With caching on `#url_helpers`:
```
Calculating -------------------------------------
INDEX: Integration Test
74.000 i/100ms
INDEX: Functional Test
99.000 i/100ms
-------------------------------------------------
INDEX: Integration Test
752.377 (± 6.9%) i/s - 3.774k
INDEX: Functional Test
1.021k (± 6.7%) i/s - 5.148k
Comparison:
INDEX: Functional Test: 1021.1 i/s
INDEX: Integration Test: 752.4 i/s - 1.36x slower
```
Afer removing the caching and bypassing the `url_helpers` when not
necessary in the session:
```
Calculating -------------------------------------
INDEX: Integration Test
87.000 i/100ms
INDEX: Functional Test
97.000 i/100ms
-------------------------------------------------
INDEX: Integration Test
828.433 (± 6.4%) i/s - 4.176k
INDEX: Functional Test
926.763 (± 7.2%) i/s - 4.656k
Comparison:
INDEX: Functional Test: 926.8 i/s
INDEX: Integration Test: 828.4 i/s - 1.12x slower
```
|
|\ \ \
| | | |
| | | |
| | | | |
ActionController#translate supports symbols
|
| | | | |
|
| | | |
| | | |
| | | |
| | | | |
Made it similar to views helper.
|
| | | | |
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
The methods `fresh_when` and `stale?` from ActionController::ConditionalGet
accept a single record as a short form for a hash. For instance
```ruby
def show
@article = Article.find(params[:id])
fresh_when(@article)
end
```
is just a short form for:
```ruby
def show
@article = Article.find(params[:id])
fresh_when(etag: @article, last_modified: @article.created_at)
end
```
This commit extends `fresh_when` and `stale?` to also accept a collection
of records, so that a short form similar to the one above can be used in
an `index` action. After this commit, the following code:
```ruby
def index
@article = Article.all
fresh_when(etag: @articles, last_modified: @articles.maximum(:created_at))
end
```
can be simply written as:
```ruby
def index
@article = Article.all
fresh_when(@articles)
end
```
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
PR #18772 changed the parameters of `stale?` to use `kwargs`.
[As for this comment](https://github.com/rails/rails/pull/18872/files#r24456288)
the default value for the `etag` parameter should be `record`, not `nil`.
This commit fixes the code and introduces a test that:
- passed before #18872
- fails on the current master (after #18772)
- passes again after setting the default value of `etag` to `record`.
|
| | | | |
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
In match_head_routes, deleted the routes in which request.request_method was empty (matches all HTTP verbs) when responding to a HEAD request. This prevents catch-all routes (such as Racks) from intercepting the HEAD request.
Fixes #18698
|
|\ \ \ \
| | | | |
| | | | | |
Migrating xhr methods to keyword arguments syntax
|
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
in `ActionController::TestCase` and
`ActionDispatch::Integration`
Old syntax:
`xhr :get, :create, params: { id: 1 }`
New syntax example:
`get :create, params: { id: 1 }, xhr: true`
|
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
The use of `# :startdoc:` inside of the class was overriding the
outer-most `# :nodoc:`, causing it to be listed in the documented API.
|
| | | | |
| | | | |
| | | | |
| | | | | |
onwards.
|
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
The commit 3b63780 re-introduced url helper caching but we need to
cache a separate module for Action Mailer without paths.
|
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
`url_helpers` used to be memoized. This was lost in a refactoring and
this PR adds it back. We noticed this while investigating why
integration tests are slower than controller tests.
|
|\ \ \ \ \
| | | | | |
| | | | | | |
Pre-discard flash messages
|
| | | | | | |
|
| | | | | | |
|
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
Inside a controller functional test after the last flash is deleted it
still persists the flash because to_session_value is nil. We should
delete it from the session when the serialized version is nil, same as
the flash middleware.
|
|\ \ \ \ \ \
| | | | | | |
| | | | | | | |
Show proper traces on Windows for the error pages
|
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
This is an issue brought up by @daniel-rikowski in rails/web-console#91.
Citing his PR proposal here:
> Prior to this, backtrace lines were simply split by a single colon.
>
> Unfortunately that is also the drive letter delimiter in Windows paths
> which resulted in a lot of empty source fragments of "C:0". ("C" from
> the drive letter and 0 from "/path/to/rails/file.rb:16".to_i)
>
> Now the trace line is split by the first colon followed by some digits,
> which works for both Windows and Unix path styles.
Now, the PR was sent against web-console, because of the templates copy
issue we used to had. Instead of bothering the contributor to reopen the
issue against upstream Rails itself, I will make sure he gets the credit
by putting his name in [rails-contributors/hard_coded_authors.rb][].
[rails-contributors/hard_coded_authors.rb]: (https://github.com/fxn/rails-contributors/blob/master/app/models/names_manager/hard_coded_authors.rb).
|
|\ \ \ \ \ \ \
| |/ / / / / /
|/| | | | | | |
:scissors: warning from controller renderer test
|
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
rails/actionpack/test/controller/renderer_test.rb:89: warning: possible reference to past scope - defaults
|
|/ / / / / /
| | | | | |
| | | | | |
| | | | | | |
ref: https://github.com/rails/rails/pull/18763#issuecomment-72349769
|