This refactoring moves the controller class name that was on the route
set to the request. The purpose of this refactoring is for changes we
need to move controller tests to integration tests, mainly being able to
access the controller on the request instead of having to go through
the router.
[Eileen M. Uchitelle & Aaron Patterson]
We should build the routes using the user facing API which is `Mapper`.
This frees up the library internals to change as we see fit. IOW we
shouldn't be testing internals.
the same value that is extracted from the options hash earlier is
returned, so we don't need to pass it in in the first place. The caller
already has the data, so stop passing it around.
We're going to try pulling this up further, and check `via` validity
sooner. This way we don't have to do a bunch of processing on `options`
hashes only to find out that the route is incorrect
we want to try to pull this logic up to where the user actually passed
in "controller" so that it's close to the related call. That way when
we're down the stack, we don't need to wonder "why are we doing this?"
The tests and methods were hard to read with `options[:options]` all
over the place. This refactoring makes the code easier to understand.
The change came out of work for moving the underlying code of controller
tests to integraiton tests.
Using the string version of the class reference is now deprecated when
referencing middleware. This should be written as a class not as a string.
Deprecation warning that this change fixes:
```
DEPRECATION WARNING: Passing strings or symbols to the middleware
builder is deprecated, please change
them to actual class references. For example:
"ActionDispatch::ShowExceptions" => ActionDispatch::ShowExceptions
```
We were doing extra work that could be pushed off to Integration test
and SharedRoutes. Creating an extra module isn't necessary when those
are created by their respective classes.
We should do the hard work outside the constructor. Also fix the tests
to not directly construct middleware objects, but to go through the
stack object.
ExceptionWrapper only cares about the backtrace cleaner, so lets just
pass the cleaner to the wrapper. It does not need to know that env
exists or what key the backtrace cleaner is stored in
This clears the transaction record state when the transaction finishes
with a `:committed` status.
Considering the following example where `name` is a required attribute.
Before we had `new_record?` returning `true` for a persisted record:
```ruby
author = Author.create! name: 'foo'
author.name = nil
author.save # => false
author.new_record? # => true
```
When executing an `ActionController::Parameters#fetch` with a block
that raises a `KeyError` the raised `KeyError` will be rescued and
converted to an `ActionController::ParameterMissing` exception,
covering up the original exception.
[Jonas Schubert Erlandsson & Roque Pinel]
This will silence deprecation warnings.
Most of the test can be changed from `render :text` to render `:plain`
or `render :body` right away. However, there are some tests that needed
to be fixed by hand as they actually assert the default Content-Type
returned from `render :body`.
We've started on discouraging the usage of `render :text` in #12374.
This is a follow-up commit to make sure that we print out the
deprecation warning.
This is another take at #14384 as we decided to wait until `master` is
targeting Rails 5.0. This commit is implementation-complete, as it
guarantees that all the public methods on the hash-inherited Parameters
are still working (based on test case). We can decide to follow-up later
if we want to remove some methods out from Parameters.
We shouldn't depend on specific methods imlemented in the TestResponse
subclass because the response could actually be a real response object.
In the future, we should either push the aliased predicate methods in
TestResponse up to the real response object, or remove them
The concurrent-ruby gem is a toolset containing many concurrency
utilities. Many of these utilities include runtime-specific
optimizations when possible. Rather than clutter the Rails codebase with
concurrency utilities separate from the core task, such tools can be
superseded by similar tools in the more specialized gem. This commit
replaces `ActiveSupport::Concurrency::Latch` with
`Concurrent::CountDownLatch`, which is functionally equivalent.
We should convert request parameters to a query string, then let the
request object parse that query string. This should give us results
that are more similar to the real-world
Instead of trying to manually clear out a request object, lets just
allocate a new one. The rack ENV is reused and cleaned (still), but the
request object is not.
Variants are typically set in the controller based on some attribute of
the request that the browser sent. We should make our tests more in
line with reality by doing the same and not mutating the request object.
Using `assert_predicate` and `assert_match` instead of just `assert` is
preferrable because better error messages are output.
In the case of `assert response.cookies.empty?` the error message was
`Failed assertion, no message given.` but now with `assert_predicate` it
will be `Expected {"user_name"=>"david"} to be empty?.`
For `assert_match(/user_name=david/,
response.headers["Set-Cookie"])` as well, the message returned was
unhelpful - `Failed assertion, no message given.` but now will tell what
was expected and what was returned with `Expected /user_name=david/ to
match "user_name=nope; path=/".`
Add the possibility to only filter parameters based on
their full path instead of relying on the immediate key.
config.filter_parameters += ['credit_card.code']
{ 'credit_card' => { 'code' => '[FILTERED]' },
'source' => { 'code' => '<%= puts 5 %>' } }
In 0de4a23 the behavior when there is a missing template was changed to
not raise an error, but instead head :no_content. This is a breaking
change and some gems rely on this happening.
To allow gems and other code to work around this, allow
`default_render` to take a block which, if provided, will
execute the contents of that block instead of doing the `head :no_content`.
This reverts commit 0b3397872582f2cf1bc6960960a6393f477c55e6, reversing
changes made to 56d52e3749180e6c1dcf7166adbad967470aa78b.
As pointed out on the PR, this will hide development mistakes too, which
is not ideal.
People should be free to mutate the header object, but not to set a new
header object. That header object may be specific to the webserver, and
we need to hide it's internals.
`ActionDispatch::SSL` changes headers to `Hash`.
So some headers will be broken if there are some middlewares
on ActionDispatch::SSL and if it uses `Rack::Utils::HeaderHash`.
Now ActionDispatch::Static can accept HTTP headers so that developers
will have control of returning arbitrary headers like
'Access-Control-Allow-Origin' when a response is delivered. They can
be configured through `#config.public_file_server.headers`:
config.public_file_server.headers = {
"Cache-Control" => "public, max-age=60",
"Access-Control-Allow-Origin" => "http://rubyonrails.org"
}
Also deprecate `config.static_cache_control` in favor of
`config.public_file_server.headers`.
ParamsWrapper was initially removed from API controllers according to
the following discusision:
https://github.com/rails-api/rails-api/issues/33
However, we're including it again so Rails API devs can decide
whether to enable or disable it.
Set `config.static_index` to serve a static directory index file not
named `index`. For example, to serve `main.html` instead of `index.html`
for directory requests, set `config.static_index` to `"main"`.
When 7e504927 was merged setting `Encoding.default_internal` and
`Encoding.default_external` would throw a warning when the ActionPack
tests were run.
Example warning: `actionpack/test/dispatch/static_test.rb:12: warning:
setting Encoding.default_external`
This patch silences the warnings as other similar tests do for setting
default_internal and default_external.
After merging #19377 ActionPack tests were missing a require for
`ActiveSupport::LogSubscriber::TestHelper` and change didn't take
into account that logger could be nil. Added the require and only log to
info if logger exists.
This wasn't caught earlier because these tests only run after a merge.
The initial attempt was to remove the method at all in
4926aa68c9.
The method overrides Rack's `#form_data?`
6f8808d420/lib/rack/request.rb (L172-L184).
Which may have some incorrect implementation actually. `type.nil?` isn't possible I suppose. I'll check.
The current implementation of ActionController::Parameters.const_missing
returns `ActionController::Parameters.always_permitted_parameters` even
if its `super` returns a constant without raising error. This prevents its
subclass in a autoloading module/class from taking advantage of
autoloading constants.
class SomeParameters < ActionController::Parameters
def do_something
DefinedSomewhere.do_something
end
end
In the code above, `DefinedSomewhere` is to be autoloaded with
`Module.const_missing` but `ActionController::Parameters.const_missing`
returns `always_permitted_parameters` instead of the autoloaded
constant.
This pull request fixes the issue respecting `const_missing`'s `super`.
Previously, an empty X_FORWARDED_HOST header would cause
Actiondispatch::Http:URL.raw_host_with_port to return nil, causing
Actiondispatch::Http:URL.host to raise a NoMethodError.
Fixes the issue described in #18764 - prevents Rack middleware from
swallowing up HEAD requests that should have been matched by a
higher-precedence `get` route, but still allows Rack middleware to
respond to HEAD requests.
Preserving RACK_ENV behavior.
This reverts commit 7bdc7635b885e473f6a577264fd8efad1c02174f, reversing
changes made to 45786be516e13d55a1fca9a4abaddd5781209103.
ActionDispatch::IntegrationTest HTTP request methods will accept only
certain kwargs in the future. This test caused a deprecation warning
when running ActionPack tests. Added `params` and `headers` to fix.
As of the upgrade to Rack 1.5, request.session_options[:id] is no
longer populated. Reflect this change in the tests by using
request.session.id instead.
Related change in Rack:
https://github.com/rack/rack/commit/83a270d6
This fixes the reasons 4cf3b8a, 303567e, and fa63448 needed to be
reverted in 7142059. The revert has been reverted and this fixes
the issues caused previously.
If we call `super` first we will end up nuking the session settings in the
application tests that do `setup do` - so any session login or cookie
settings will not be persisted thoughout the test sessions.
Calling `super` last prevents `@integration_session` from getting nuked
and set to nil if it's already set.
Test added to prevent regression of this behavior in the future.
Since the `ForkingExecutor` class seems to be pretty slow on Rubinius
due to DRb (c.f. http://git.io/xIVg), let's avoid running tests with
it on this platform.
Also, the `parallelize_me!` call make the suite to output a bunch of
errors due to rubinius/rubinius#2934 since there are thread-safety
problems with autoloading.
In f6e293ec54f02f83cdb37502bea117f66f87bcae we avoided a segfault in the
tests, however I think we should try to avoid the crash, as it may
happen in user code as well.
Here is what I distiled the bug down to:
```ruby
# Rails case - works on 2.0, 2.1; crashes on 2.2
require 'action_dispatch'
ActionDispatch::Response.new(200, "Content-Type" => "text/xml")
# General case - works on 2.0, 2.1; crashes on 2.2
def foo(optional = {}, default_argument: nil)
end
foo('quux' => 'bar')
```
Introduced in f6e293e ActionPack tests began sefaulting. I found that it
was the kwargs and the test causing the seg fault was missing the new
default_headers argument.
Haven't diagnosed yet. No similarly failing tests in Rails to work from.
cc @tenderlove, @eileencodes
Revert "there is always an integration session, so remove the check"
Revert "lazily create the integration session"
Revert "use before_setup to set up test instance variables"
This reverts commits 4cf3b8ac47f109fa83a6f66eb97d6cb0eace0d05, 303567e554de26822f3107be55c471d6477a745f, and fa63448420d3385dbd043aca22dba973b45b8bb2.
