Skip to content

Commit

Permalink
fix(MigrateStartingHandler): Do not load when migrations have run sin…
Browse files Browse the repository at this point in the history
…ce that would wipe development data.
  • Loading branch information
Paul Rogers committed Apr 5, 2019
1 parent c1adf39 commit fd7ef29
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 2 deletions.
26 changes: 25 additions & 1 deletion src/Handlers/MigrateStartingHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ class MigrateStartingHandler

public function handle(CommandStarting $event)
{
// CONSIDER: Never implicitly loading on `migrate` to simplify code
// (dropping this whole class) and to avoid confusion.

if (
'migrate' === $event->command
// Avoid knowingly starting migrate which will fail.
Expand All @@ -60,7 +63,7 @@ public function handle(CommandStarting $event)
// No point in implicitly loading when it's not present.
&& file_exists(database_path() . MigrateDumpCommand::SCHEMA_SQL_PATH_SUFFIX)
) {
// Must pass along options or else it'll use wrong DB or have
// Must pass along options or it may use wrong DB or have
// inconsistent output.
$options = self::inputToArtisanOptions($event->input);
$database = $options['--database'] ?? env('DB_CONNECTION');
Expand All @@ -70,6 +73,27 @@ public function handle(CommandStarting $event)
return;
}

// Only implicitly load when DB has *not* migrated any since load
// would wipe existing data.
$has_migrated_any = false;
// Try-catch instead of information_schema since not all have one.
try {
$has_migrated_any = ! is_null(
\DB::connection($database)->table('migrations')->value('id')
);
} catch (\PDOException $e) {
// No op. when table does not exist.
if (
! in_array($e->getCode(), ['42P01', '42S02'], true)
&& ! preg_match("/\bdoes ?n[o']t exist\b/iu", $e->getMessage())
) {
throw $e;
}
}
if ($has_migrated_any) {
return;
}

// CONSIDER: Defaulting to --no-drop when not explicitly specified
// with environment variable, for extra safety.
// CONSIDER: Explicitly passing output class (since underlying
Expand Down
22 changes: 21 additions & 1 deletion tests/MigrateHookTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ public function test_handle()
// TODO: Fix inclusion of `, ['--quiet' => true]` here breaking test.
$result = \Artisan::call('migrate:dump');
$this->assertEquals(0, $result);
\Schema::dropAllTables();
// Implicitly load when no migrations.
\DB::table('migrations')->delete();

// Test that dump file is used.
$output = new BufferedOutput();
Expand All @@ -26,4 +27,23 @@ public function test_handle()
$this->assertContains('Loaded schema', $output_string);
$this->assertContains('Dumped schema', $output_string);
}

public function test_handle_doesNotLoadWhenDbHasMigrated()
{
// Make the dump file.
$this->createTestTablesWithoutMigrate();
// TODO: Fix inclusion of `, ['--quiet' => true]` here breaking test.
$result = \Artisan::call('migrate:dump');
$this->assertEquals(0, $result);

// Test that dump file is used.
$output = new BufferedOutput();
$result = \Artisan::call('migrate', [], $output);
$this->assertEquals(0, $result);

$output_string = $output->fetch();
$this->assertNotContains('Loaded schema', $output_string);

$this->assertEquals(1, \DB::table('test_ms')->count());
}
}
1 change: 1 addition & 0 deletions tests/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ protected function createTestTablesWithoutMigrate() : void
\Schema::dropAllTables();
\Schema::dropAllViews();
\Schema::create('migrations', function (\Illuminate\Database\Schema\Blueprint $table) {
$table->increments('id');
$table->string('migration', 255);
$table->integer('batch');
});
Expand Down

0 comments on commit fd7ef29

Please sign in to comment.