| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
| |
Currently we sometimes find a redundant begin block in code review
(e.g. https://github.com/rails/rails/pull/33604#discussion_r209784205).
I'd like to enable `Style/RedundantBegin` cop to avoid that, since
rescue/else/ensure are allowed inside do/end blocks in Ruby 2.5
(https://bugs.ruby-lang.org/issues/12906), so we'd probably meets with
that situation than before.
|
|
|
|
| |
This argument was added in fa5a028ed9f, and #34137 but hasn't been used.
|
|
|
|
|
| |
Adds support for multiple databases to `rails db:schema:cache:dump`
and `rails db:schema:cache:clear`.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Before this change this test was passing even if we revert #31135. The
reason for that is that `app 'development'` will load the environment in
the test process and it is happening before db_create_and_drop is
called.
This was not asserting that the environment was loaded in the db:create
task itself.
To test it we enhance the db:create task with a block that writes to a
tmp file the value of the config. If the environment is loaded before
that task enhancement runs the content of the file will have "true"
insteand of "false".
|
|
|
|
|
|
|
|
|
| |
The rake tasks which became deprecate now does not load the environment.
Therefore, even if the application specifies the behavior of deprecating,
the message is output to stderr ignoring the specification.
It seems that this is not the expected behavior.
We should respect the setting even in the rake tasks.
|
|
|
|
|
|
|
|
|
|
|
| |
Changes the `configs_for` method from using traditional arguments to
using kwargs. This is so I can add the `include_replicas` kwarg without
having to always include `env_name` and `spec_name` in the method call.
`include_replicas` defaults to false because everywhere internally in
Rails we don't want replicas. `configs_for` is for iterating over
configurations to create / run rake tasks, so we really don't ever need
replicas in that case.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
While the three-tier config makes it easier to define databases for
multiple database applications, it quickly became clear to offer full
support for multiple databases we need to change the way the connections
hash was handled.
A three-tier config means that when Rails needed to choose a default
configuration (in the case a user doesn't ask for a specific
configuration) it wasn't clear to Rails which the default was. I
[bandaid fixed this so the rake tasks could work](#32271) but that fix
wasn't correct because it actually doubled up the configuration hashes.
Instead of attemping to manipulate the hashes @tenderlove and I decided
that it made more sense if we converted the hashes to objects so we can
easily ask those object questions. In a three tier config like this:
```
development:
primary:
database: "my_primary_db"
animals:
database; "my_animals_db"
```
We end up with an object like this:
```
@configurations=[
#<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fd1acbded10
@env_name="development",@spec_name="primary",
@config={"adapter"=>"sqlite3", "database"=>"db/development.sqlite3"}>,
#<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fd1acbdea90
@env_name="development",@spec_name="animals",
@config={"adapter"=>"sqlite3", "database"=>"db/development.sqlite3"}>
]>
```
The configurations setter takes the database configuration set by your
application and turns them into an
`ActiveRecord::DatabaseConfigurations` object that has one getter -
`@configurations` which is an array of all the database objects.
The configurations getter returns this object by default since it acts
like a hash in most of the cases we need. For example if you need to
access the default `development` database we can simply request it as we
did before:
```
ActiveRecord::Base.configurations["development"]
```
This will return primary development database configuration hash:
```
{ "database" => "my_primary_db" }
```
Internally all of Active Record has been converted to use the new
objects. I've built this to be backwards compatible but allow for
accessing the hash if needed for a deprecation period. To get the
original hash instead of the object you can either add `to_h` on the
configurations call or pass `legacy: true` to `configurations.
```
ActiveRecord::Base.configurations.to_h
=> { "development => { "database" => "my_primary_db" } }
ActiveRecord::Base.configurations(legacy: true)
=> { "development => { "database" => "my_primary_db" } }
```
The new configurations object allows us to iterate over the Active
Record configurations without losing the known environment or
specification name for that configuration. You can also select all the
configs for an env or env and spec. With this we can always ask
any object what environment it belongs to:
```
db_configs = ActiveRecord::Base.configurations.configurations_for("development")
=> #<ActiveRecord::DatabaseConfigurations:0x00007fd1acbdf800
@configurations=[
#<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fd1acbded10
@env_name="development",@spec_name="primary",
@config={"adapter"=>"sqlite3", "database"=>"db/development.sqlite3"}>,
#<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fd1acbdea90
@env_name="development",@spec_name="animals",
@config={"adapter"=>"sqlite3", "database"=>"db/development.sqlite3"}>
]>
db_config.env_name
=> "development"
db_config.spec_name
=> "primary"
db_config.config
=> { "adapter"=>"sqlite3", "database"=>"db/development.sqlite3" }
```
The configurations object is more flexible than the configurations hash
and will allow us to build on top of the connection management in order
to add support for primary/replica connections, sharding, and
constructing queries for associations that live in multiple databases.
|
|\
| |
| | |
Prevent leaking of user's DB credentials on `rails db:create` failure
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Issue #27852 reports that when `rails db:create` fails, it causes
leaking of user's DB credentials to $stderr.
We print a DB's configuration hash in order to help users more quickly
to figure out what could be wrong with his configuration.
This commit changes message from
"Couldn't create database for #{configuration.inspect}" to
"Couldn't create '#{configuration['database']}' database. Please check your configuration.".
There are two PRs that fixing it #27878, #27879, but they need a bit more work.
I decided help to finish this and added Author of those PRs credit in this commit.
Since it is a security issue, I think we should backport it to
`5-2-stable`, and `5-1-stable`.
Guided by https://edgeguides.rubyonrails.org/maintenance_policy.html#security-issues
Fixes #27852
Closes #27879
Related to #27878
[Alexander Marrs & bogdanvlviv]
|
| |
| |
| |
| | |
I removed the argument so I should remove the conditional too.
|
| |
| |
| |
| | |
The test that used this was updated and it's no longer needed.
|
|/
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We originally did the whole `load_database_yaml` thing because this test
wasn't cooperating and we needed to finish the namespaced rake tasks for
multiple databases.
However, it turns out that YAML can't eval ERB if you don't tell it it's
ERB so you get Pysch parse errors if you're using multi-line ERB or
ERB with conditionals. It's a hot mess.
After trying a few things and thinking it over we decided that it wasn't
worth bandaiding over, the test needed to be improved. The test was
added in #31135 to test that the env is loaded in these tasks. But it
was blowing up because we were trying to read a database name out of the
configuration - however that's not the purpose of this change. We want
to read environment files in the rake tasks, but not in the config
file.
In this PR we changed the test to test what the PR was actually fixing.
We've also deleted the `load_database_yaml` because it caused more
problems than it was worth. This should fix the issues described in
https://github.com/rails/rails/pull/32274#issuecomment-384161057. We
also had these problems at GitHub.
Co-authored-by: alimi <aibrahim2k2@gmail.com>
|
|
|
|
|
|
|
| |
`rake routes` was a public task. Therefore, I think that we should deprecate
it before deleting it.
Related to #32121.
|
|
|
|
|
|
| |
* Invoke Rails::Command within the rake task
* Adds test for calling initializers with 'bin/rake'
* Adds deprecation warning to the rake task
|
|
|
|
|
| |
* Call the Rails::Command::DevCommand in the rake task for dev:cache
* Add deprecation for using `bin/rake` in favor of `bin/rails`
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
After 1996fbe2a3e46ff5698bfa3812afb7f42cdfa899 `rails notes`
isn't the same as `rake notes`.
Since that, we should test `rake routes` instead of `rails notes` in
`railties/test/application/rake/notes_test.rb` file.
So I changed all occurrences of `rails routes` to `rake routes` in that file,
and added assertions about deprecation warning of using `rake notes`.
It will help to figure out that we should remove
`railties/test/application/rake/notes_test.rb` entirely in favour of
`railties/test/commands/notes_test.rb` that was added
in 1996fbe2a3e46ff5698bfa3812afb7f42cdfa899.
|
|
|
|
| |
Also, added a test that a deprecated message will be output.
|
|
|
|
| |
Follow up of #32605.
|
|
|
|
|
| |
This autocorrects the violations after adding a custom cop in
3305c78dcd.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Before:
```
$ ruby -v
ruby 2.5.1p57 (2018-03-29 revision 63029) [x86_64-linux]
$ ruby -w -Itest -Ilib -I../activesupport/lib -I../actionpack/lib -I../actionview/lib -I../activemodel/lib test/application/rake/multi_dbs_test.rb
Run options: --seed 28744
F
Failure:
ApplicationTests::RakeTests::RakeMultiDbsTest#test_db:migrate_and_db:structure:dump_and_db:structure:load_works_on_all_databases [test/application/rake/multi_dbs_test.rb:70]:
Expected /CREATE TABLE \"books\"/ to match "CREATE TABLE IF NOT EXISTS \"schema_migrations\" (\"version\" varchar NOT NULL PRIMARY KEY);\nCREATE TABLE IF NOT EXISTS \"ar_internal_metadata\" (\"key\" varchar NOT NULL PRIMARY KEY, \"value\" varchar, \"created_at\" datetime NOT NULL, \"updated_at\" datetime NOT NULL);\nCREATE TABLE IF NOT EXISTS \"books\" (\"id\" integer PRIMARY KEY AUTOINCREMENT NOT NULL, \"title\" varchar, \"created_at\" datetime NOT NULL, \"updated_at\" datetime NOT NULL);\nCREATE TABLE sqlite_sequence(name,seq);\nINSERT INTO \"schema_migrations\" (version) VALUES\n('20180416201805');\n\n\n".
```
|
|
|
|
|
|
|
|
|
| |
I realized I wasn't really testing some of the new rake tasks added so I
built out this new test that uses a multi-db database.yml and allows us
to run create/drop/migrate/schema:dump/schema:load and those that are
namespaced like create:animals. This will make our testing more robust
so we can catch problems quicker and set a good place to add future
tests as these features evolve.
|
|
|
|
| |
This class should be under Rails module as it belongs to Rails.
|
|
|
|
|
|
|
|
|
|
|
|
| |
down is only called with a block from the rake tasks where it passes a
`SCOPE`. Technically this was tested but since we don't run all the
migrations we're not actually testing the down works with a `SCOPE`. To
ensure we're testing both we can run `db:migrate` again to migrate users
and then run `down` with a scope to test that only the bukkits migration
is reverted.
Updates test to prevent having to fix regressions like we did in
4d4db4c.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Rails has some support for multiple databases but it can be hard to
handle migrations with those. The easiest way to implement multiple
databases is to contain migrations into their own folder ("db/migrate"
for the primary db and "db/seconddb_migrate" for the second db). Without
this you would need to write code that allowed you to switch connections
in migrations. I can tell you from experience that is not a fun way to
implement multiple databases.
This refactoring is a pre-requisite for implementing other features
related to parallel testing and improved handling for multiple
databases.
The refactoring here moves the class methods from the `Migrator` class
into it's own new class `MigrationContext`. The goal was to move the
`migrations_paths` method off of the `Migrator` class and onto the
connection. This allows users to do the following in their
`database.yml`:
```
development:
adapter: mysql2
username: root
password:
development_seconddb:
adapter: mysql2
username: root
password:
migrations_paths: "db/second_db_migrate"
```
Migrations for the `seconddb` can now be store in the
`db/second_db_migrate` directory. Migrations for the primary database
are stored in `db/migrate`".
The refactoring here drastically reduces the internal API for migrations
since we don't need to pass `migrations_paths` around to every single
method. Additionally this change does not require any Rails applications
to make changes unless they want to use the new public API. All of the
class methods from the `Migrator` class were `nodoc`'d except for the
`migrations_paths` and `migrations_path` getter/setters respectively.
|
|
|
|
| |
Follow up of #31432.
|
|
|
|
|
|
|
| |
This is necessary in order to make the processing dependent on
`Migrator.current_version` work even without database.
Context: https://github.com/rails/rails/pull/31135#issuecomment-348404326
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently the environment is not loaded in some db tasks.
Therefore, if use encrypted secrets values in `database.yml`,
`read_encrypted_secrets` will not be true, so the value can not be
used correctly.
To fix this, added `environment` as dependency of `load_config`.
It also removes explicit `environment` dependencies that are no longer
needed.
Fixes #30717
|
|
|
|
|
| |
Since isolation application is generated with the `--skip-gemfile`
option, so `active_storage:install` is not executed.
|
|
|
|
|
|
| |
Ensure that `bin/rails db:migrate` with specified `VERSION` reverts
all migrations only if `VERSION` is `0`.
Raise error if target migration doesn't exist.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Omit `rails activestorage:install` for jdbcmysql, jdbc and shebang tests
AppGeneratorTest#test_config_jdbcmysql_database
rails aborted!
LoadError: Could not load 'active_record/connection_adapters/mysql_adapter'.
Make sure that the adapter in config/database.yml is valid.
If you use an adapter other than 'mysql2', 'postgresql' or 'sqlite3' add
the necessary adapter gem to the Gemfile.
(compressed)
bin/rails:4:in `<main>'
Tasks: TOP => activestorage:install => environment
(See full trace by running task with --trace)
AppGeneratorTest#test_config_jdbc_database
rails aborted!
LoadError: Could not load 'active_record/connection_adapters/jdbc_adapter'.
Make sure that the adapter in config/database.yml is valid.
If you use an adapter other than 'mysql2', 'postgresql' or 'sqlite3' add
the necessary adapter gem to the Gemfile.
(compressed)
bin/rails:4:in `<main>'
Tasks: TOP => activestorage:install => environment
(See full trace by running task with --trace)
AppGeneratorTest#test_shebang_is_added_to_rails_file
/home/ubuntu/.rbenv/versions/2.4.1/bin/ruby: no Ruby script found in input (LoadError)
Prevent PendingMigrationError in tests
* Run `bin/rails db:migrate RAILS_ENV=test` in test_cases before start tests to prevent PendingMigrationError
* FileUtils.rm_r("db/migrate")
* --skip-active-storage
Fix failed tests in `railties/test/railties/engine_test.rb`
Related to #30111
Imporve `SharedGeneratorTests#test_default_frameworks_are_required_when_others_are_removed`
- Explicitly skip active_storage
- Ensure that skipped frameworks are commented
- Ensure that default frameworks are not commented
Fix error `Errno::ENOSPC: No space left on device - sendfile`
Since `rails new` runs `rails active_storage:install`
that boots an app.
Since adding Bootsnap 0312a5c67e35b960e33677b5358c539f1047e4e1
during booting an app, it creates the cache:
264K tmp/cache/bootsnap-load-path-cache
27M tmp/cache/bootsnap-compile-cache
* teardown_app must remove app
|
|
|
|
|
|
|
|
|
|
|
|
| |
database connection
When load schema from `structure.sql`, database connection isn't
established. `ActiveRecord::Tasks::DatabaseTasks.load_schema` has to
establish database connection since it executes
```
ActiveRecord::InternalMetadata.create_table
ActiveRecord::InternalMetadata[:environment] = environment
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
ar_internal_metadata's data for a test database.
Before:
```
$ RAILS_ENV=test rails dbconsole
> SELECT * FROM ar_internal_metadata;
key|value|created_at|updated_at
environment|development|2017-09-11 23:14:10.815679|2017-09-11 23:14:10.815679
```
After:
```
$ RAILS_ENV=test rails dbconsole
> SELECT * FROM ar_internal_metadata;
key|value|created_at|updated_at
environment|test|2017-09-11 23:14:10.815679|2017-09-11 23:14:10.815679
```
Fixes #26731.
|
| |
|
|
|
|
|
| |
While this avoids shell argument parsing, we still pass through
everything in our stack.
|
|
|
|
|
|
|
|
|
|
| |
Originally, it hard-coded pid file path. It can not be removed when customizing
pid file path.
But rake task can not get pid file path. Therefore, do not remove file in rake
task, makes it possible to judge whether it is restart from the argument of the
command and removes the file in server command.
Fixes #29306
|
| |
|
| |
|
|
|
|
|
|
|
|
| |
This fixes the following warning:
```
railties/test/application/rake/dbs_test.rb:265: warning: ambiguous first argument; put parentheses or a space even after `/' operator
```
|
|
|
|
|
|
| |
This was missed when the frozen string literal pragma was added to this
file because the string is only modified when running in the context of
a full Rails app, which wasn't covered by the test suite.
|
| |
|
|
|
|
|
| |
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
|
|\
| |
| |
| | |
Enforce frozen string in Rubocop
|
| | |
|
|/
|
|
|
|
|
|
|
| |
If system test fails, it creates screenshot under `tmp/screenshots`.
https://github.com/rails/rails/blob/34fe2a4fc778d18b7fe6bdf3629c1481bee789b9/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb#L45
But currently, screenshot files is not cleared by `tmp:clear` task.
This patch make clears screenshot files with `tmp:clear` task as well
as other tmp files.
|
|
|
|
| |
Fix #28905
|
|
|
|
|
| |
Raise error on the movement of migrations
when the current migration does not exist.
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Add stronger assertions to rake migration tasks to make sure the user is providing a numeric VERSION
An empty string was getting converted to version = 0. This would in turn pass the presence check.
Address linting warning
Add test for rake task and refactor code to meet expectations
In particular passing VERSION=0 should not raise an error.
Addressed Comments for PR #28485. Trimmed empty lines + change of wording for error message
Adjust test for change of wording in error message
Change condition to follow rails idioms
|