Skip to content

Commit

Permalink
Fix migrations $connection property (#41161)
Browse files Browse the repository at this point in the history
Despite being documented as allowing `Schema` queries to run on the
given secondary connection, `Schema::connection()` must also be
explicitly called to avoid `up()` and `down()` queries to be run on the
default database connection.

Instead this property is only used by the migrator when wrapping
`DB::transaction()` & `DB::pretend()`. The queries still run on the
default database connection (or the command option --database.)

This change makes queries run using that $connection while not affecting
the migrations repository. i.e., the connection for the `migrations`
history table. If another `Schema::connection()` is called during `up()`
or `down()`, that will be used instead of `$connection`.

$connection also overrides MigrateCommand option '--database'. A
breaking change is required to switch to the opposite behavior.
  • Loading branch information
derekmd authored Feb 22, 2022
1 parent 424bdd4 commit 03e3a80
Show file tree
Hide file tree
Showing 4 changed files with 157 additions and 4 deletions.
29 changes: 25 additions & 4 deletions src/Illuminate/Database/Migrations/Migrator.php
Original file line number Diff line number Diff line change
Expand Up @@ -387,11 +387,11 @@ protected function runMigration($migration, $method)
$migration->getConnection()
);

$callback = function () use ($migration, $method) {
$callback = function () use ($connection, $migration, $method) {
if (method_exists($migration, $method)) {
$this->fireMigrationEvent(new MigrationStarted($migration, $method));

$migration->{$method}();
$this->runMethod($connection, $migration, $method);

$this->fireMigrationEvent(new MigrationEnded($migration, $method));
}
Expand Down Expand Up @@ -447,13 +447,34 @@ protected function getQueries($migration, $method)
$migration->getConnection()
);

return $db->pretend(function () use ($migration, $method) {
return $db->pretend(function () use ($db, $migration, $method) {
if (method_exists($migration, $method)) {
$migration->{$method}();
$this->runMethod($db, $migration, $method);
}
});
}

/**
* Run a migration method on the given connection.
*
* @param \Illuminate\Database\Connection $connection
* @param object $migration
* @param string $method
* @return void
*/
protected function runMethod($connection, $migration, $method)
{
$previousConnection = $this->resolver->getDefaultConnection();

try {
$this->resolver->setDefaultConnection($connection->getName());

$migration->{$method}();
} finally {
$this->resolver->setDefaultConnection($previousConnection);
}
}

/**
* Resolve a migration instance from a file.
*
Expand Down
54 changes: 54 additions & 0 deletions tests/Database/DatabaseMigratorIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ protected function setUp(): void
'database' => ':memory:',
], 'sqlite2');

$db->addConnection([
'driver' => 'sqlite',
'database' => ':memory:',
], 'sqlite3');

$db->setAsGlobal();

$container = new Container;
Expand Down Expand Up @@ -84,6 +89,55 @@ public function testBasicMigrationOfSingleFolder()
$this->assertTrue(Str::contains($ran[1], 'password_resets'));
}

public function testMigrationsDefaultConnectionCanBeChanged()
{
$ran = $this->migrator->usingConnection('sqlite2', function () {
return $this->migrator->run([__DIR__.'/migrations/one'], ['database' => 'sqllite3']);
});

$this->assertFalse($this->db->schema()->hasTable('users'));
$this->assertFalse($this->db->schema()->hasTable('password_resets'));
$this->assertTrue($this->db->schema('sqlite2')->hasTable('users'));
$this->assertTrue($this->db->schema('sqlite2')->hasTable('password_resets'));
$this->assertFalse($this->db->schema('sqlite3')->hasTable('users'));
$this->assertFalse($this->db->schema('sqlite3')->hasTable('password_resets'));

$this->assertTrue(Str::contains($ran[0], 'users'));
$this->assertTrue(Str::contains($ran[1], 'password_resets'));
}

public function testMigrationsCanEachDefineConnection()
{
$ran = $this->migrator->run([__DIR__.'/migrations/connection_configured']);

$this->assertFalse($this->db->schema()->hasTable('failed_jobs'));
$this->assertFalse($this->db->schema()->hasTable('jobs'));
$this->assertFalse($this->db->schema('sqlite2')->hasTable('failed_jobs'));
$this->assertFalse($this->db->schema('sqlite2')->hasTable('jobs'));
$this->assertTrue($this->db->schema('sqlite3')->hasTable('failed_jobs'));
$this->assertTrue($this->db->schema('sqlite3')->hasTable('jobs'));

$this->assertTrue(Str::contains($ran[0], 'failed_jobs'));
$this->assertTrue(Str::contains($ran[1], 'jobs'));
}

public function testMigratorCannotChangeDefinedMigrationConnection()
{
$ran = $this->migrator->usingConnection('sqlite2', function () {
return $this->migrator->run([__DIR__.'/migrations/connection_configured']);
});

$this->assertFalse($this->db->schema()->hasTable('failed_jobs'));
$this->assertFalse($this->db->schema()->hasTable('jobs'));
$this->assertFalse($this->db->schema('sqlite2')->hasTable('failed_jobs'));
$this->assertFalse($this->db->schema('sqlite2')->hasTable('jobs'));
$this->assertTrue($this->db->schema('sqlite3')->hasTable('failed_jobs'));
$this->assertTrue($this->db->schema('sqlite3')->hasTable('jobs'));

$this->assertTrue(Str::contains($ran[0], 'failed_jobs'));
$this->assertTrue(Str::contains($ran[1], 'jobs'));
}

public function testMigrationsCanBeRolledBack()
{
$this->migrator->run([__DIR__.'/migrations/one']);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

return new class extends Migration
{
/**
* The database connection that should be used by the migration.
*
* @var string
*/
protected $connection = 'sqlite3';

/**
* Run the migrations.
*
* @return void
*/
public function up()
{
Schema::create('failed_jobs', function (Blueprint $table) {
$table->id();
$table->text('connection');
$table->text('queue');
$table->longText('payload');
$table->longText('exception');
$table->timestamp('failed_at')->useCurrent();
});
}

/**
* Reverse the migrations.
*
* @return void
*/
public function down()
{
Schema::dropIfExists('failed_jobs');
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

return new class extends Migration
{
/**
* Run the migrations.
*
* @return void
*/
public function up()
{
Schema::connection('sqlite3')->create('jobs', function (Blueprint $table) {
$table->bigIncrements('id');
$table->string('queue')->index();
$table->longText('payload');
$table->unsignedTinyInteger('attempts');
$table->unsignedInteger('reserved_at')->nullable();
$table->unsignedInteger('available_at');
$table->unsignedInteger('created_at');
});
}

/**
* Reverse the migrations.
*
* @return void
*/
public function down()
{
Schema::connection('sqlite3')->dropIfExists('jobs');
}
};

0 comments on commit 03e3a80

Please sign in to comment.