Run mutliple Migration sets on same Connection (3.next / Migrator)

[swiffer]

This is a (multiple allowed):

• [x] bug

• [ ] enhancement

• [ ] feature-discussion (RFC)

• CakePHP Version: 4.3.0-RC3

• Migrations plugin version: 3.next

• Database server (MySQL, SQLite, Postgres): Postgres 13.4

• PHP Version: 8.0.8

• Platform / OS: Ubuntu 20.04

What you did

I'm trying to run Migrations for my Application and one Plugin that is used within the application via the new Migrator.

I tried different things in tests/bootstrap.php

$migrator->run();
$migrator->run([['plugin' => 'AuditStash']]);

$migrator->run([['connection' => 'test'], ['plugin' => 'AuditStash', 'connection' => 'test']]);

On execution the verbose logs show:

Reading migrations status for connection "test"...
New migration(s) found.
Dropping Constraints 1 tables for connection test
Dropping 1 tables for connection test
Migrations for connection "test" successfully run.
Truncating 33 tables for connection test
Reading migrations status for connection "test", plugin "AuditStash"...
New migration(s) found.
Dropping Constraints 1 tables for connection test
Dropping 1 tables for connection test
Migrations for connection "test", plugin "AuditStash" successfully run.
Truncating 1 tables for connection test

Expected Behavior

Both Migration sets should run and all tables should be available in the database as for Plugins the phinxlog is prefixed via audit_stash_phinxlog.

Actual Behavior

The migrations of the app itself are not available in the TestSuite once the Migration set for the AuditStash Plugin is added.

The first test is complaining of a missing table which is part of the App Migration Set.

Cake\Core\Exception\CakeException: Cannot describe schema for table `companies` for fixture `App\Test\Fixture\CompaniesFixture`: the table does not exist.```

[markstory]

Both Migration sets should run and all tables should be available in the database as for Plugins the phinxlog is prefixed via audit_stash_phinxlog.

How do you have that prefix configured?

[swiffer]

The prefix does not need to be configured, it's automatically

running this in tests/bootstrap.php

$migrator = new Migrator(['outputLevel' => ConsoleIo::VERBOSE]);
$migrator->run([['connection' => 'test'], ['plugin' => 'AuditStash', 'connection' => 'test']]);

for Sqlite results in this (and tests failing because app tables are missing):

Reading migrations status for connection "test"...
New migration(s) found.
Reading migrations status for connection "test", plugin "AuditStash"...
New migration(s) found.
Dropping Constraints 1 tables for connection test
Dropping 1 tables for connection test
Migrations for connection "test" successfully run.
Migrations for connection "test", plugin "AuditStash" successfully run.
Truncating 1 tables for connection test

and for PostgreSQL results in this (and tests passing):

Reading migrations status for connection "test"...
New migration(s) found.
Reading migrations status for connection "test", plugin "AuditStash"...
New migration(s) found.
Dropping Constraints 2 tables for connection test
Dropping 2 tables for connection test
Migrations for connection "test" successfully run.
Migrations for connection "test", plugin "AuditStash" successfully run.
Truncating 34 tables for connection test

[markstory]

Ok. I will see if I can reproduce this.

[markstory]

I wasn't able to reproduce the problem you're having. I also get different output from Migrator. Instead of what you're seeing, I get:

Reading migrations status for connection "test"...
New migration(s) found.
Reading migrations status for connection "test", plugin "Cms"...
New migration(s) found.
Dropping Constraints 2 tables for connection test
Dropping 2 tables for connection test
Migrations for connection "test" successfully run.
Migrations for connection "test", plugin "Cms" successfully run.
Truncating 2 tables for connection test

I've pushed up a branch of the code I was using. I got the expected output of (cms_phinxlog, contact, phinxlog, tasks) with SQLite and Postgres 13. What am I missing?

[swiffer]

ah! it is caused by running sqlite tests in memory

app/local.php

        'test' => [
            'host' => null,
            //'port' => 'non_standard_port_number',
            'username' => '',
            'password' => '',
            'database' => ':memory:',
            //'schema' => 'myapp',
            'url' => env('DATABASE_TEST_URL', null),
        ],

results in:

########## DEBUG ##########
[
  (int) 0 => 'cms_phinxlog',
  (int) 1 => 'contacts'
]
###########################

PHP 8.0.11NTS Visual C++ 2019 x64 - it was working for 4.2.9 - is it because of nts (non thread safe) and something new now running in parallel?

[markstory]

Migrations creates a different connection to the database, so you likely have two different databases in play. I don't know how we fix that.

[swiffer]

I see - running tests in memory via sqlite locally while against prod env. database in ci/cd only was a great developer experience as it was superfast (10 seconds (inmemory) vs 1+ minutes (on disk)). any chance for improvements here ?

[swiffer]

just want to leave this here as it seems that sqlite supports sharing the same in memory database between multiple connections - it's just not implemented in php at the moment:

https://www.sqlite.org/inmemorydb.html https://bugs.php.net/bug.php?id=76868

[othercorey]

@swiffer How does the latest 3.next work for you?

[swiffer]

@othercorey with the latest changes setting the test database to run in memory is no longer working for me. everything runs fine when using a sqlite file as database but tests run ~70 seconds instead of ~10. maybe it's better to try fixing this in php core but i never did that.

[othercorey]

I tried changing the Sqlite connection to enable shared cache, but it just creates a file with cache=shared in it.

[othercorey]

According to sqlite docs, URI filenames are disabled by default so I don't know if this support would actually help.

In order to maintain full backwards compatibility for legacy applications, the URI filename capability is disabled by default.

https://www.sqlite.org/uri.html

[othercorey]

Following on this more, I found that php does enable URI filenames when open_basedir is disabled.

There was an open php bug regarding shared cache with sqlite pdo. https://bugs.php.net/bug.php?id=76868

However, after planning on add empty filename and possibly URI support, I added some tests to the php testsuite and found they are supported.

https://github.com/php/php-src/pull/7662

I did not test the behavior of shared cache, but if this is available, you could possibly use it. However, migrator will still run separately.

[swiffer]

Thanks @othercorey - just checking the comment in the bug report - how is intended to be specified in the cakephp connection ?

As of PHP 8.1.0, the following is supported having the same effect:

sqlite:file::memory:?cache=shared

However, that doesn't work, if open_basedir is set. Would that be a problem for you?

[othercorey]

I don't know that it can be. The filename includes a ? in this case.

[othercorey]

We could potentially detect specific query parameters and append them to the filename. Would this help in the migrator situation?

@markstory

[markstory]

However, that doesn't work, if open_basedir is set. Would that be a problem for you?

I'm not worried about CI environments that can't have open_basedir set correctly.

We could potentially detect specific query parameters and append them to the filename. Would this help in the migrator situation?

I think so. Migrator uses Phinx's connection which we generate from the cake one.

[swiffer]

@othercorey - i tried this with cakephp/[email protected] and a dsn configured in app_local.php like this

        'url' => env('DATABASE_TEST_URL', 'sqlite:///:memory:?cache=shared'),

sadly within the testsuite CakePHP still complaints that it can't describe tables inserted via the Migrator

[othercorey]

It's very possible that sphinx doesn't support it or they are still executed in a different process.

In fact, since we had to customize the filename, I'm pretty sure sphinx needs the same thing added if it parsed the URI manually.

[swiffer]

After doing the following changes it is working, should i open pull requests on both projects for this / are additional changes needed besides these:

https://github.com/cakephp/phinx/blob/master/src/Phinx/Db/Adapter/SQLiteAdapter.php#L156

            $options = $this->getOptions();

            if (PHP_VERSION_ID < 80100 && (!empty($options['mode']) || !empty($options['cache']))) {
                throw new RuntimeException('SQLite URI support requires PHP 8.1.');
            } elseif ((!empty($options['mode']) || !empty($options['cache'])) && !empty($options['memory'])) {
                throw new RuntimeException('Memory must not be set when cache or mode are.');
            } elseif (PHP_VERSION_ID >= 80100 && (!empty($options['mode']) || !empty($options['cache']))) {
                $params = [];
                if (!empty($options['cache'])) {
                    $params[] = 'cache=' . $options['cache'];
                }
                if (!empty($options['mode'])) {
                    $params[] = 'mode=' . $options['mode'];
                }
                $dsn = 'sqlite:file:' . ($options['name'] ?? '') . '?' . implode('&', $params);
            } else {
                // use a memory database if the option was specified
                if (!empty($options['memory']) || $options['name'] === static::MEMORY) {
                    $dsn = 'sqlite:' . static::MEMORY;
                } else {
                    $dsn = 'sqlite:' . $options['name'] . $this->suffix;
                }
            }

            $driverOptions = [];

https://github.com/cakephp/migrations/blob/3.x/src/ConfigurationTrait.php#L151

        if ($adapterName === 'sqlite') {
            if (!empty($connectionConfig['cache'])) {
                $config['environments']['default']['cache'] = $connectionConfig['cache'];
            }
            if (!empty($connectionConfig['mode'])) {
                $config['environments']['default']['mode'] = $connectionConfig['mode'];
            }
        }

[othercorey]

You'll definitely have to get the changes into phinx.

[swiffer]

ok - opened:

https://github.com/cakephp/phinx/pull/2053/files https://github.com/cakephp/phinx/issues/2052

[othercorey]

@swiffer Did you get anywhere trying to configure phinx with the shared cache?

[swiffer]

Yes, for ne it's working fine with the PR applied to the phinx Branch and Code mentioned Here in the comments