| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
After #33637 some tests in `activerecord/test/cases/tasks/database_tasks_test.rb`
don't assert anything.
We used to stub `ActiveRecord::Base::configurations` method in those
tests like `ActiveRecord::Base.stub(:configurations, @configurations) {}`.
Since #33637 `ActiveRecord::Base::configurations` is a `ActiveRecord::DatabaseConfigurations`
object(not a Hash object) we can't do so anymore.
`ActiveRecord::DatabaseConfigurations` object builds during `ActiveRecord::Base::configurations=`.
We can replace `ActiveRecord::Base.stub(:configurations, @configurations) {}` to
```
begin
old_configurations = ActiveRecord::Base.configurations
ActiveRecord::Base.configurations = @configurations
# ...
ensure
ActiveRecord::Base.configurations = old_configurations
end
```
Also I fixed tests in `activerecord/test/cases/tasks/legacy_database_tasks_test.rb`
But currently It looks like duplication of
`activerecord/test/cases/tasks/database_tasks_test.rb`.
We should improve those tests or remove them.
I've tried (in `activerecord/test/cases/tasks/legacy_database_tasks_test.rb` file):
```
def with_stubbed_configurations
old_configurations = ActiveRecord::Base.configurations.to_h
ActiveRecord::Base.configurations = @configurations
ActiveRecord::Base.stub(:configurations, ActiveRecord::Base.configurations.to_h) do
yield
end
ensure
ActiveRecord::Base.configurations = old_configurations
end
```
but it causes erros in tests cases.
After discussion we decided to remove
`activerecord/test/cases/tasks/legacy_database_tasks_test.rb`
Related to #33637
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Six Mocha calls prove quite resistant to Minitestification. For example,
if we replace
```
ActiveRecord::Associations::HasManyAssociation
.any_instance
.expects(:reader)
.never
```
with `assert_not_called`, Minitest wisely raises
```
NameError: undefined method `reader' for class `ActiveRecord::Associations::HasManyAssociation'
```
as `:reader` comes from a deeply embedded abstract class,
`ActiveRecord::Associations::CollectionAssociation`.
This patch tackles this difficulty by adding
`ActiveSupport::Testing::MethodCallAsserts#assert_called_on_instance_of`
which injects a stubbed method into `klass`, and verifies the number of
times it is called, similar to `assert_called`. It also adds a convenience
method, `assert_not_called_on_instance_of`, mirroring
`assert_not_called`.
It uses the new method_call_assertions to replace the remaining Mocha
calls in `ActiveRecord` tests.
[utilum + bogdanvlviv + kspath]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Use attr_reader/attr_writer instead of methods
method is 12% slower
Use flat_map over map.flatten(1)
flatten is 66% slower
Use hash[]= instead of hash.merge! with single arguments
merge! is 166% slower
See https://github.com/rails/rails/pull/32337 for more conversation
|
| |
|
|
|
|
|
|
|
|
|
|
| |
Many calls to `Mocha#expects` preceded the introduction of
`ActiveSupport::Testing::MethodCallAssertions` in 53f64c0fb,
and many are simple to replace with `MethodCallAssertions`.
This patch makes all these simple replacements.
Step 5 in #33162
|
|
|
|
| |
Missed these in preparing #33337
|
|
|
|
| |
Should have been removed in #33309.
|
|
|
|
| |
Step 4 in #33162
|
|
|
|
|
|
|
| |
While preparing this I realised that some stubbed returns values
serve no purpose, so this patch drops those as well.
Step 3 in #33162
|
|
|
|
| |
Step 2 in #33162
|
|
|
|
|
|
| |
Step 1 in #33162
[utilum + bogdanvlviv]
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
|
| |
An entry in `ActiveRecord::Base.configurations` can either be a
connection spec ("two-level") or a hash of specs ("three-level").
We were detecting two-level configurations by looking for the `database`
key, but the database can also be specified as part of the `url` key,
which meant we incorrectly treated those configurations as three-level.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
This reverts commit 16f279ebd474626577ced858e3626ac4535a33df, reversing
changes made to 6c6a30a7c357ce1eafa093d77d2b08684fe50887.
The config can be named anything, not just default (although all
generated apps will be named default). We can't just delete configs that
don't have a database because that will break three-tier configs. Oh
well.
|
|
|
|
|
|
|
| |
Because of this default configuration we're constantly checking if the
database exists when looping through configurations. This is unnecessary
and we should just delete it before we need to loop through
configurations.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
|
|
|
| |
`dump_schema_cache` fills `schema_cache` even if the test that modifies
the schema has properly cleared the schema cache.
Fixes #31463.
|
|
|
|
|
|
|
| |
These changes prevent ignoring environments name of which is a `Symbol`
```
config.active_record.protected_environments = ['staging', :production]
```
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
| |
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
|
| |
|
|
|
|
| |
Fix #28905
|
|
|
|
|
| |
Add cases to ensure that environment variables VERBOSE and VERSION have
correct typecast.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
|
| |
since 79887593c18919fed49f441d64236362cb755872, create_all task recreates the connection to AR::Base
which doesn't connect to the in_memory database that is set up for tests
|
|
|
|
|
|
|
|
|
| |
Today `rake db:schema:cache:dump` only supports dumping cache for a
single connection (`ActiveRecord::Base.connection`). This doesn't work
for apps with multiple databases.
This PR makes `DatabaseTasks` to provide an API for dumping schema cache
for any connection.
|
|
|
|
|
|
|
|
| |
Without this patch it's impossible to pass extra flags to
mysqldump/pg_dump when running `rake db:structure:dump` or `load`
The following config variables (`structure_load_flags` and `structure_dump_flags`)
make it better configurable.
|
| |
|
|
|
|
|
|
| |
assert [1, 3].includes?(2) fails with unhelpful "Asserting failed" message
assert_includes [1, 3], 2 fails with "Expected [1, 3] to include 2" which makes it easier to debug and more obvious what went wrong
|
|
|
|
|
|
|
|
| |
Style/SpaceBeforeBlockBraces
Style/SpaceInsideBlockBraces
Style/SpaceInsideHashLiteralBraces
Fix all violations in the repository.
|
| |
|
|
|
|
|
| |
The current code base is not uniform. After some discussion,
we have chosen to go with double quotes by default.
|
|
|
|
|
|
|
|
| |
test_migrate_clears_schema_cache_afterward test.
Disable transactions for this test.
Fixes #24391
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Without clearing the caches afterward, removals done in migrations would
not be reflected in a separate task in the same process. That is, given
a table with a migration to remove a column, the schema cache would
still reflect that a table has that in something such as the
'db:seed' task:
`rake db:migrate db:seed`
(A common thing to do in a script for a project ala `bin/setup`)
vs
`rake db:migrate && rake db:seed`
(Two processes)
The first would not reflect that the column was removed.
The second would (cache reset).
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This reverts a334425caff9b2140d5e99fcfc2eb8c4ab10bdfa.
The main reason is that now the workflow is inconsistent when using
spring.
When using spring `RAILS_ENV` is always set, so only one database is
created.
This means that in development `bin/rake db:create` and `bundle exec
rake db:create` have different results.
It also breaks the `bin/setup` script since `bin/rake db:setup
db:test:prepare` will fail.
|
|\
| |
| | |
Prevent destructive action on production database
|
| |
| |
| |
| | |
Discussion: https://github.com/rails/rails/pull/22967#discussion_r49137035
|
| |
| |
| |
| |
| |
| |
| | |
This PR introduces a key/value type store to Active Record that can be used for storing internal values. It is an alternative implementation to #21237 cc @sgrif @matthewd.
It is possible to run your tests against your production database by accident right now. While infrequently, but as an anecdotal data point, Heroku receives a non-trivial number of requests for a database restore due to this happening. In these cases the loss can be large.
To prevent against running tests against production we can store the "environment" version that was used when migrating the database in a new internal table. Before executing tests we can see if the database is a listed in `protected_environments` and abort. There is a manual escape valve to force this check from happening with environment variable `DISABLE_DATABASE_ENVIRONMENT_CHECK=1`.
|
|/
|
|
| |
Follow up to #22642.
|
|
|
|
| |
database_tasks instead of Migrator
|
| |
|
|
|
|
|
|
|
|
|
|
| |
[Joshua Cody & Yves Senn]
Closes #16757.
Prior to this patch schema loading rake tasks had the potential to leak a
connection to a different database. This had side-effects when rake tasks
operating on the current connection (like `db:seed`) were chained.
|
|
|
|
|
|
| |
This extracts the logic that was embedded in a Rake task into a static
method.
Bonus: the first test for `rake db:migrate`
|
| |
|