Commit Graph

1559 Commits

Author SHA1 Message Date
Rafael Mendonça França
667b150c20
Merge pull request #37404 from joshmn/respect_the_force
Fix collision check when using a generator and using the force option
2020-01-06 21:32:43 -03:00
Haroon Ahmed
db1ae8cbb4 remove reference to global rails command and replace with bin/rails 2019-12-27 19:32:37 +00:00
Ryuta Kamizono
5324f2cb09 Revert "Merge pull request #37215 from utilum/avoid_test_flunking_on_warning"
This reverts commit ed78e96408f3f83e779a71c65b86aeb1cfc5616e, reversing
changes made to eca6c273fe2729b9634907562c2717cf86443b6b.
2019-12-25 17:13:09 +09:00
yuuji.yaginuma
c8b88dd4df Use the single line editor in console test
It's a bit difficult to deal properly with IRB's multi line and color,
and I think we don't need to check it out in `rails console` tests.
2019-12-24 20:47:21 +09:00
Brian Buchalter
53e9438ef2 Ignore test env in DatabaseTasks when DATABASE_URL is present
Fixes https://github.com/rails/rails/issues/28827.

The steps to reproduce are as follows:

git clone git@github.com:bbuchalter/rails-issue-28827.git
cd rails-issue-28827
bundle install
bin/rails db:create

Observe that we create two databases when invoking db:create: development and test. Now observe what happens when we invoke our drop command while using DATABASE_URL.

DATABASE_URL=sqlite3://$(pwd)/db/database_url.sqlite3 bin/rails db:create

As expected, the development environment now uses the DATABASE_URL. What is unexpected is that the test environment does not.

It's unclear what the expected behavior should be in this case, but the cause of it is this: 9f2c74eda0/activerecord/lib/active_record/tasks/database_tasks.rb (L494)

Because of each_local_configuration, there seems to be no way invoke these database rake on only the development environment to ensure DATABASE_URL is respected.

The smallest scope of change I can think to make would be to conditionalize this behavior so it does not get applied when DATABASE_URL is present.
2019-12-20 14:22:31 -08:00
Rafael França
e36d4a0381
Merge pull request #38026 from Edouard-chin/ec-av-base-loadorder
Don't require "action_view/base" in action pack:
2019-12-19 13:40:56 -03:00
Edouard CHIN
88ee52f9d9 Don't require "action_view/base" in action pack:
- ### Problem

  ActionPack requires "action_view/base" at boot time, this
  causes a variety of issue that I described in detail in #38024.

  There is no real reason to require av/base in the
  ActionDispatch::Debugexceptions class.

  ### Solution

  Like any other components (such as ActiveRecord, ActiveJob...),
  ActionView::Base shouldn't be loaded at boot time.

  Here are the two main changes needed for this:

  1) Actionview has a special initializer that needs to run
     before the app is fully booted (adding a executor needs to be done
     before application is done booting)
  63ec70e700/actionview/lib/action_view/railtie.rb (L81-L84)

     That initializer used a lazy load hooks but we can't do that anymore
     because Action::Base view won't be triggered during booting process.
     When it will get triggered, (presumably on the first request),
     it's too late to add an executor.

  ------------------------------------------------

  2) Compare to other components, ActionView doesn't use `Base` for
     configuration flag. A lot of flags ares instead set on modules
     (FormHelper, FormTagHelper).
     The problem is that those module depends on AV::Base to be
     loaded, as otherwise configuration set by the user aren't applied.
     (Since the lazy load hooks hasn't been triggered)
     63ec70e700/actionview/lib/action_view/railtie.rb (L66-L69)

     We shouldn't wait for AB::Base to be loaded in order to set these
     configuration. However, we need to do it inside an
     `after_initialize` block in order to let application
     set it to the value they want.

  Closes #28538

  Co-authored-by: betesh <iybetesh@gmail.com>"
2019-12-19 17:28:24 +01:00
eileencodes
3dffacd16e Fix remaining connection_config calls
We missed these in rails/rails#38005 because deprecation warnings are
silently swallowed by these tests.

Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
2019-12-18 09:12:27 -05:00
yuuji.yaginuma
1555fcf32c Load an application before use
Without this, `Rails.application` returns `nil`.
Ref: https://buildkite.com/rails/rails/builds/65683#e86ec58e-53a5-41be-8aeb-11c5705ee580/1042-1053
2019-12-18 18:01:02 +09:00
Ryuta Kamizono
72af0bbc3d Fix typos 2019-12-18 16:47:18 +09:00
Rafael Mendonça França
ed5f2c6192
Merge pull request #28209 from tjoyal/railties/add-config-rake_eager_load
[Railties] Add config rake_eager_load
2019-12-17 22:10:14 -03:00
Kasper Timm Hansen
5c3b6c73c4
Add helper method to slim down config_for test set up 2019-12-17 02:02:27 +01:00
Jean Boussier
21c7199c0f
Allow non-hash values in config files
Fix: https://github.com/rails/rails/issues/37800
2019-12-17 01:46:15 +01:00
Kasper Timm Hansen
2da5439b85
Fix test name and test 6.1 default 2019-12-15 02:43:35 +01:00
Cédric Fabianski
7ccaa125ba
Add SameSite protection to every written cookie
Enabling `SameSite` cookie protection is an addition to CSRF protection,
where cookies won't be sent by browsers in cross-site POST requests when set to `:lax`.

`:strict` disables cookies being sent in cross-site GET or POST requests.

Passing `:none` disables this protection and is the same as previous versions albeit a `; SameSite=None` is appended to the cookie.

See upgrade instructions in config/initializers/new_framework_defaults_6_1.rb.

More info [here](https://tools.ietf.org/html/draft-west-first-party-cookies-07)

_NB: Technically already possible as Rack supports SameSite protection, this is to ensure it's applied to all cookies_
2019-12-15 01:37:24 +01:00
Kasper Timm Hansen
5ded839cfb
Strip default_ prefix from retry_jitter config to match conventions 2019-12-13 01:42:58 +01:00
Cliff Pruitt
e2cdffce3d Add config option for ActiveJob::Base.default_retry_jitter 2019-12-10 12:11:46 -05:00
Edouard CHIN
bbfab0b33a Don't run AJ after_enqueue / after_perform when chain is halted:
- ### Problem

  ```ruby
    MyJob < ApplicationJob
      before_enqueue { throw(:abort) }
      after_enqueue { # enters here }
    end
  ```
  I find AJ behaviour on after_enqueue and after_perform callbacks
  weird as they get run even when the callback chain is halted.
  It's counter intuitive to run the after_enqueue callbacks even
  though the job wasn't event enqueued.

  ### Solution

  In Rails 6.2, I propose to make the new behaviour the default
  and stop running after callbacks when the chain is halted.
  For application that wants this behaviour now or in 6.1
  they can do so by adding the `config.active_job.skip_after_callbacks_if_terminated = true`
  in their configuration file.
2019-12-09 17:17:23 +01:00
kirikiriyamama
4d858b3f2a Merge shared configuration deeply 2019-12-08 22:18:08 +09:00
Peter Zhu
fbb83d78c3 Use DiskController for both public and private files 2019-12-06 16:02:16 -05:00
Edouard CHIN
33bf253282 Bring back feature that allows loading external route iles:
= This feature existed back in 2012 5e7d6bba79
  but got reverted with the incentive that there was a better approach.
  After discussions, we agreed that it's a useful feature for apps
  that have a really large set of routes.

  Co-authored-by: Yehuda Katz <wycats@gmail.com>
2019-12-06 14:20:12 +01:00
Alan Tan
767cec041d Revert "Use app.config.file_watcher for watcher in RoutesReloader"
This reverts commit 28e44f472d1cd6853726f85eeb7623e5901c4d37.

A limitation of Listen is that it currently only supports watching directories.
Therefore, watching `config/routes.rb` will end up watching the entire `config` directory
if we use the evented file watcher. This causes problems especially if symlinks are present
in the `config` directory.
2019-12-06 14:58:07 +08:00
Ryuta Kamizono
3b0c3124b1
Merge pull request #37853 from tgxworld/use_proper_file_watcher
Use `app.config.file_watcher` for watcher in `RoutesReloader`
2019-12-06 09:59:14 +09:00
Étienne Barrié
e5e9c558a3 Remove deprecated non-symbol access to nested config_for hashes 2019-12-04 09:55:56 -05:00
Alan Tan
28e44f472d Use app.config.file_watcher for watcher in RoutesReloader 2019-12-03 10:25:36 +08:00
Ryuta Kamizono
d558febe32 Auto-correct rubocop offences 2019-11-24 09:54:47 +09:00
John Hawthorn
75afb43a25 Use rails() instead of system() 2019-11-14 12:31:56 -08:00
Xavier Noria
43863bfc57 let environments configure load-related paths
Co-authored-by: Allen Hsu <allen.hsusp@gmail.com>
2019-11-06 20:40:39 +01:00
eileencodes
042b6fec87 Appease rubocop
Rubocop wants double quoted strings, not single. I missed this when I
merged #37601
2019-11-01 15:43:43 -04:00
Edouard CHIN
c9e44d1a06 Reestablish connection to previous database after migrating:
- The migrate task iterates and establish a connection over each db
  resulting in the last one to be used by subsequent rake tasks.
  We should reestablish a connection to the connection that was
  established before the migrate tasks was run
- Fix #37578
2019-10-30 19:31:22 +01:00
Gannon McGibbon
74201c3885 Make has_many inversing opt-in
Make has_many inversing support available through an opt-in config
variable. This behaviour is likely to break existing applications, but
it is correct behaviour.
2019-10-11 15:55:46 -04:00
Peter Zhu
feab7031b5 Permanent URLs for public storage blobs
Services can be configured in `config/storage.yml` with a new key
`public: true | false` to indicate whether a service holds public
blobs or private blobs. Public services will always return a
permanent URL.

Deprecates `Blob#service_url` in favor of `Blob#url`.
2019-10-11 15:14:43 -04:00
John Hawthorn
72b1ae6a5a Sprockets uses debug. not self. now 2019-10-10 14:43:03 -07:00
John Hawthorn
441917629e Link .js from manifest.js in assets_test
We no longer link JS by default, we need to modify manifest.js for that
now.
2019-10-10 14:43:03 -07:00
John Hawthorn
8d037fd3dd Remove a javascript from test 2019-10-10 14:43:03 -07:00
John Hawthorn
e63695535e Use a stylesheet instead of a javascript in test
We no longer link all js by default, so we should do this test with a
css instead (we don't care about that specifics of the dir just that its
in the manifest and in this dir).
2019-10-10 14:43:03 -07:00
John Hawthorn
5a4d4c7492 Maybe we understand index.js better now? 2019-10-10 14:43:03 -07:00
John Hawthorn
added06d3f Add a few more assertions 2019-10-10 14:43:03 -07:00
John Hawthorn
4cb93a39e2 Fix Sprockets::DoubleLinkError in test 2019-10-09 21:48:37 -07:00
Josh Brody
1dd2b73cb2 Generators skip collision check if force option is passed. 2019-10-08 19:07:38 -05:00
Kasper Timm Hansen
ed78e96408
Merge pull request #37215 from utilum/avoid_test_flunking_on_warning
Avoid flunking tests on warnings in output
2019-10-07 02:22:09 +02:00
Masaki Hara
89f5789aad Delay ActionDispatch::Response configuration to load-time
It fixes the problem in propagating return_only_media_type_on_content_type
and fixes the corresponding test being ineffective.

The mentioned test addes the following line:
...config.action_dispatch.return_only_media_type_on_content_type = true
to the config and checks if it takes effect. However, in this scenario,
the value is already true before this line.
Moreover, the users are supposed to flip this from true to false in real
situations.

This commit flips the config in the test, making it to fail as
expected. The next commit will fix the failure.

In order for return_only_media_type_on_content_type to appropriately
take effect on ActionDispatch::Response, we want to know when
ActionDispatch::Response is loaded.
As load hooks for ActionDispatch would be too broad, the appropriate
registry is for ActionDispatch::Response itself.

Looking into other examples, a hook name is a full class name in
snake case with `_base` suffix omitted, if any. Therefore, in this case,
:action_dispatch_response seems appropriate.
2019-10-01 17:42:05 +09:00
utilum
e0b8c918a0 Avoid flunking tests on warning in output 2019-09-28 12:52:31 +02:00
John Crepezzi
4e66e2dffc Update test to avoid Puma output format change
We are seeing some test failures for this test in #37291. It looks like
what's going on is that Puma has changed the output for this command
between 4.1 and 4.2

Previously:

```
...
* Environment: development
* Listening on tcp://localhost:3000
...

```

Now:

```
...
* Environment: development
* Listening on tcp://127.0.0.1:3000
* Listening on tcp://[::1]:3000
...

```

So to get around this, instead of checking the binding address, just
check for the presence of 'Listening' generally like we do on server
start.

Co-authored-by: eileencodes <eileencodes@gmail.com>
2019-09-25 13:11:58 -04:00
Jean Boussier
48c716bbfa Instantiate ConnectionPool with a DatabaseConfig rather than a ConnectionSpecification 2019-09-24 15:12:22 +02:00
John Crepezzi
b8b2d40659 Make all reads on configuration_hash use methods
Convert all uses of `db_config.configuration_hash[*]` to use methods
defined on an implementation of `DatabaseConfigurations::DatabaseConfig`.

Since we want to get away from accessing properties directly on the
underlying configuration hash, we'll move here to accessing those values
via the implementations on `DatabaseConfig` (or more specifically,
`HashConfig`).

There are still codepaths that are passing around `configuration_hash`,
and follow-on PRs will address those with the goal of using
configuration objects everywhere up until the point we pass a resolved
hash over to the underlying client.

Co-authored-by: eileencodes <eileencodes@gmail.com>
2019-09-23 16:46:05 -04:00
Eileen M. Uchitelle
9b6433bb82
Merge pull request #37199 from seejohnrun/reduce-surface-area-of-connection-specification
Reduce surface area of ConnectionSpecification
2019-09-16 09:35:22 -04:00
Akira Matsuda
9bce8c3c02 form_authenticity_token takes keyword arguments 2019-09-15 03:05:52 +09:00
eileencodes
b8fc0150d6 Reduce surface area of ConnectionSpecification
Eventually we'd like to get rid of this class altogether but for now
this PR reduces the surface area by removing methods from the class and
moving classes out into their own files.

* `adapter_method` was moved into database configurations
* `initialize_dup` was removed because it was only used in tests
* Resolver is now it's own class under connection adapters
* ConnectionUrlResolver, only used by the configurations, is in a class
under DatabaseConfigurations

Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
2019-09-13 22:05:02 -04:00
eileencodes
ce9b197cc9 Use symbols everywhere for database configurations
Previously in some places we used symbol keys, and in some places we used
string keys. That made it pretty confusing to figure out in a particular
place what type of configuration object you were working with.

Now internally, all configuration hashes are keyed by symbols and
converted to such on the way in.

A few exceptions:

- `DatabaseConfigurations#to_h` still returns strings for backward compatibility
- Same for `legacy_hash`
- `default_hash` previously could return strings, but the associated
  comment mentions it returns symbol-key `Hash` and now it always does

Because this is a change in behavior, a few method renames have happened:

- `DatabaseConfig#config` is now `DatabaseConfig#configuration_hash` and returns a symbol-key `Hash`
- `ConnectionSpecification#config` is now `ConnectionSpecification#underlying_configuration_hash` and returns the `Hash` of the underlying `DatabaseConfig`
- `DatabaseConfig#config` was added back, returns `String`-keys for backward compatibility, and is deprecated in favor of the new `configuration_hash`

Co-authored-by: eileencodes <eileencodes@gmail.com>
2019-09-13 08:53:22 -04:00