Framework: [5.7] Sqlite & dropForeign

[okdewit]

• Laravel Version: 5.7.1
• PHP Version: 7.2
• Database Driver & Version: MySQL 8, Sqlite

Description:

When using Laravel, using in-memory Sqlite for tests while using MySQL/Postgres for dev/prod requests is quite common.

Laravel 5.7 contains an undocumented change which throws an exception when trying to drop foreign keys in Sqlite: https://github.com/laravel/framework/pull/24052

When a testsuite runs migrations, it produces the message:
SQLite doesn't support dropping foreign keys (you would need to re-create the table).

While this is technically correct, this strict behavior also removes the possibility to silently ignore foreign keys in Sqlite and use the same migrations with multiple drivers.

Could the checks in https://github.com/laravel/framework/pull/24052 be reverted, or be made configurable?

If not, I think it should at least be added to the 5.7 upgrade docs to set expectations. Rewriting hundreds of migrations is quite impactful.

[laurencei]

Just use the new migrate:refresh in your test suite? i.e. why are you trying to drop tables, just drop the database, it would be faster?

[okdewit]

@laurencei

We have been using the builtin refreshDatabase trait on functional tests, which calls Illuminate\Foundation\Testing::refreshInMemoryDatabase() in every setUp.

This is the way to do it according to the 5.7 documentation.

I am not even dropping tables yet, just migrating upwards.

It happens quite often that a table is removed, renamed after creation, hence there are plenty of dropForeign() calls in the up() methods.

It seems messy to rewrite all existing migrations to add database driver checks: if (DB::getDriverName() !== 'sqlite') $table->dropForeign('...'), so I was hoping the warning could be handled more gracefully.

[laurencei]

The best/easiest option would probably be you just extend Illuminate/Database/Schema/Blueprint.php and change the warning to suit your needs...

[okdewit]

@laurencei That is indeed a reasonable option. I understand there are conflicting interests here, if you are deliberately trying to work with foreign keys on a production SQLite database, you would be glad to be informed by an exception.

I do think this should be added to the 5.7 upgrade guide for clarity though, so I have created a PR on the docs repository.

[staudenmeir]

Can you close the issue?

[chris-rs]

You can also just catch that exception instead of extending Illuminate/Database/Schema/Blueprint.php

try {
   parent::tearDown();
}catch(\BadMethodCallException $e) {
   // do something, or nothing: just ignore
}

[okdewit]

@chris-rs That was my first thought as well, but in many scenarios you have dropForeign in your "up" chain as well. Real world databases are rarely pretty, and often need to change/remove tables from old experiments.

Catching exceptions silently suppresses the error and continues within your custom tearDown, but it also causes your database to stop in the middle of a migration, with unexpected test results.

[okdewit]

@chris-rs Overriding classes turned out to be quite hard for migrations as well, so I opted to just make a macro for now:

Blueprint::macro('dropForeignSilently', function($index): Fluent {
    if (DB::getDriverName() === 'sqlite') return new Fluent();
    return self::dropForeign($index);
});

... And just do a full search-replace on the migrations directory.

This seems to work fine for to restore Laravel 5.6 behavior for now.

I might later expand it with some logic which truly drops the foreign key by fully copying & swapping out the underlying SQLite tables.

[CasperLaiTW]

I found a way to replace sqlite connection, don't update any migration.

Hopefully can help.

abstract class TestCase extends BaseTestCase
{
    public function __construct(?string $name = null, array $data = [], string $dataName = '')
    {
        parent::__construct($name, $data, $dataName);
        $this->hotfixSqlite();
    }
    /**
     *
     */
    public function hotfixSqlite()
    {
        \Illuminate\Database\Connection::resolverFor('sqlite', function ($connection, $database, $prefix, $config) {
            return new class($connection, $database, $prefix, $config) extends SQLiteConnection {
                public function getSchemaBuilder()
                {
                    if ($this->schemaGrammar === null) {
                        $this->useDefaultSchemaGrammar();
                    }
                    return new class($this) extends SQLiteBuilder {
                        protected function createBlueprint($table, Closure $callback = null)
                        {
                            return new class($table, $callback) extends Blueprint {
                                public function dropForeign($index)
                                {
                                    return new Fluent();
                                }
                            };
                        }
                    };
                }
            };
        });
    }
}

[romanopablo]

I found a way to replace sqlite connection, don't update any migration.

Hopefully can help.

abstract class TestCase extends BaseTestCase
{
    public function __construct(?string $name = null, array $data = [], string $dataName = '')
    {
        parent::__construct($name, $data, $dataName);
        $this->hotfixSqlite();
    }
    /**
     *
     */
    public function hotfixSqlite()
    {
        \Illuminate\Database\Connection::resolverFor('sqlite', function ($connection, $database, $prefix, $config) {
            return new class($connection, $database, $prefix, $config) extends SQLiteConnection {
                public function getSchemaBuilder()
                {
                    if ($this->schemaGrammar === null) {
                        $this->useDefaultSchemaGrammar();
                    }
                    return new class($this) extends SQLiteBuilder {
                        protected function createBlueprint($table, Closure $callback = null)
                        {
                            return new class($table, $callback) extends Blueprint {
                                public function dropForeign($index)
                                {
                                    return new Fluent();
                                }
                            };
                        }
                    };
                }
            };
        });
    }
}

Adding a little detail, remember to inject the classes needed on top:

use \Illuminate\Database\SQLiteConnection;  
use \Illuminate\Database\Schema\{SQLiteBuilder, Blueprint}; 
use \Illuminate\Support\Fluent; 

[OzanKurt]

Reopen?

[JJCLane]

Note when using PHPUnit ^9 you need to register the connection before calling parent::setUp() like so:

    protected function setUp(): void
    {
        $this->hotfixSqlite();
        parent::setUp();
    }

This confused me for a while so hopefully it helps someone!

[HenkPoley]

So full version for your base TestCase class:

    public function __construct(?string $name = null, array $data = [], string $dataName = '')
    {
        $this->hotfixSqlite();
        parent::__construct($name, $data, $dataName);
    }

    /**
     * Fix for: BadMethodCallException : SQLite doesn't support dropping foreign keys (you would need to re-create the table).
     */
    public function hotfixSqlite()
    {
        \Illuminate\Database\Connection::resolverFor('sqlite', function ($connection, $database, $prefix, $config) {
            return new class($connection, $database, $prefix, $config) extends \Illuminate\Database\SQLiteConnection {
                public function getSchemaBuilder()
                {
                    if ($this->schemaGrammar === null) {
                        $this->useDefaultSchemaGrammar();
                    }
                    return new class($this) extends \Illuminate\Database\Schema\SQLiteBuilder {
                        protected function createBlueprint($table, \Closure $callback = null)
                        {
                            return new class($table, $callback) extends \Illuminate\Database\Schema\Blueprint {
                                public function dropForeign($index)
                                {
                                    return new \Illuminate\Support\Fluent();
                                }
                            };
                        }
                    };
                }
            };
        });
    }

[Younesi]

I got this error when tried the solution.
ErrorException: Declaration of class@anonymous::createBlueprint($table, ?Tests\Closure $callback = NULL) should be compatible with Illuminate\Database\Schema\Builder::createBlueprint($table, ?Closure $callback = NULL)

[HenkPoley]

Maybe change createBlueprint($table, Closure $callback = null) to createBlueprint($table, \Closure $callback = null) ?

E.g. backslash ('\') before 'Closure'.