Skip to content

Commit fd7ef29

Browse files
author
Paul Rogers
committed
fix(MigrateStartingHandler): Do not load when migrations have run since that would wipe development data.
1 parent c1adf39 commit fd7ef29

File tree

3 files changed

+47
-2
lines changed

3 files changed

+47
-2
lines changed

src/Handlers/MigrateStartingHandler.php

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ class MigrateStartingHandler
4747

4848
public function handle(CommandStarting $event)
4949
{
50+
// CONSIDER: Never implicitly loading on `migrate` to simplify code
51+
// (dropping this whole class) and to avoid confusion.
52+
5053
if (
5154
'migrate' === $event->command
5255
// Avoid knowingly starting migrate which will fail.
@@ -60,7 +63,7 @@ public function handle(CommandStarting $event)
6063
// No point in implicitly loading when it's not present.
6164
&& file_exists(database_path() . MigrateDumpCommand::SCHEMA_SQL_PATH_SUFFIX)
6265
) {
63-
// Must pass along options or else it'll use wrong DB or have
66+
// Must pass along options or it may use wrong DB or have
6467
// inconsistent output.
6568
$options = self::inputToArtisanOptions($event->input);
6669
$database = $options['--database'] ?? env('DB_CONNECTION');
@@ -70,6 +73,27 @@ public function handle(CommandStarting $event)
7073
return;
7174
}
7275

76+
// Only implicitly load when DB has *not* migrated any since load
77+
// would wipe existing data.
78+
$has_migrated_any = false;
79+
// Try-catch instead of information_schema since not all have one.
80+
try {
81+
$has_migrated_any = ! is_null(
82+
\DB::connection($database)->table('migrations')->value('id')
83+
);
84+
} catch (\PDOException $e) {
85+
// No op. when table does not exist.
86+
if (
87+
! in_array($e->getCode(), ['42P01', '42S02'], true)
88+
&& ! preg_match("/\bdoes ?n[o']t exist\b/iu", $e->getMessage())
89+
) {
90+
throw $e;
91+
}
92+
}
93+
if ($has_migrated_any) {
94+
return;
95+
}
96+
7397
// CONSIDER: Defaulting to --no-drop when not explicitly specified
7498
// with environment variable, for extra safety.
7599
// CONSIDER: Explicitly passing output class (since underlying

tests/MigrateHookTest.php

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ public function test_handle()
1515
// TODO: Fix inclusion of `, ['--quiet' => true]` here breaking test.
1616
$result = \Artisan::call('migrate:dump');
1717
$this->assertEquals(0, $result);
18-
\Schema::dropAllTables();
18+
// Implicitly load when no migrations.
19+
\DB::table('migrations')->delete();
1920

2021
// Test that dump file is used.
2122
$output = new BufferedOutput();
@@ -26,4 +27,23 @@ public function test_handle()
2627
$this->assertContains('Loaded schema', $output_string);
2728
$this->assertContains('Dumped schema', $output_string);
2829
}
30+
31+
public function test_handle_doesNotLoadWhenDbHasMigrated()
32+
{
33+
// Make the dump file.
34+
$this->createTestTablesWithoutMigrate();
35+
// TODO: Fix inclusion of `, ['--quiet' => true]` here breaking test.
36+
$result = \Artisan::call('migrate:dump');
37+
$this->assertEquals(0, $result);
38+
39+
// Test that dump file is used.
40+
$output = new BufferedOutput();
41+
$result = \Artisan::call('migrate', [], $output);
42+
$this->assertEquals(0, $result);
43+
44+
$output_string = $output->fetch();
45+
$this->assertNotContains('Loaded schema', $output_string);
46+
47+
$this->assertEquals(1, \DB::table('test_ms')->count());
48+
}
2949
}

tests/TestCase.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ protected function createTestTablesWithoutMigrate() : void
4949
\Schema::dropAllTables();
5050
\Schema::dropAllViews();
5151
\Schema::create('migrations', function (\Illuminate\Database\Schema\Blueprint $table) {
52+
$table->increments('id');
5253
$table->string('migration', 255);
5354
$table->integer('batch');
5455
});

0 commit comments

Comments
 (0)