Changes are detected, and migration created, when there is no change #1191
Comments
|
What is the SQL that is generated each time? probably there is a misconfiguration for the custom types you are adding to your app |
|
Tkx for your reply @goetas, and sorry for taking long to reply So, using the example I have above, the doctrine configurations this: <?php
declare(strict_types=1);
use Acme\App\Infrastructure\Persistence\Doctrine\Type\ChannelConfigType;
use Acme\App\Infrastructure\Persistence\Doctrine\Type\ChannelIdType;
use Acme\App\Infrastructure\Persistence\Doctrine\Type\ChannelStateEnumType;
use Acme\App\Infrastructure\Persistence\Doctrine\Type\ChannelTypeEnumType;
use Acme\App\Infrastructure\Persistence\Doctrine\Type\ChannelTypeIdType;
use Acme\App\Infrastructure\Persistence\Doctrine\Type\ClientIdType;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
/** @var string[] $ignoredTables */
$ignoredTables = require __DIR__ . '/doctrine_migrations_ignored_tables.php';
return function (ContainerConfigurator $container) use ($ignoredTables): void {
$container->extension('doctrine', [
'dbal' => [
'url' => '%env(resolve:DATABASE_URL)%',
// 'charset' => 'utf8mb4',
// 'default_table_options' => [
// 'engine' => 'InnoDB',
// 'charset' => 'utf8mb4',
// 'collate' => 'utf8mb4_unicode_ci',
// ],
# the DBAL driverOptions option
// 'options' => [
// 'charset' => 'utf8mb4', // characterset ?
// 'collate' => 'utf8mb4_unicode_ci', // collation ?
// ],
'mapping_types' => [
'enum' => 'string',
],
'types' => [
'channel_id' => ChannelIdType::class,
'channel_config' => ChannelConfigType::class,
'channel_state_enum_type' => ChannelStateEnumType::class,
'channel_type_id' => ChannelTypeIdType::class,
'channel_type_enum' => ChannelTypeEnumType::class,
'client_id' => ClientIdType::class,
],
// Only consider tables with names matching the pattern.
// The pattern is negating (negative lookaround), so it will ignore any table in the list.
'schema_filter' => $ignoredTables === [] ? null : '~^(?!' . implode('|', $ignoredTables) . ')~',
],
'orm' => [
'auto_generate_proxy_classes' => true,
'naming_strategy' => 'doctrine.orm.naming_strategy.underscore_number_aware',
'auto_mapping' => true,
'mappings' => [
'Acme\App\Core\Component' => [
'is_bundle' => false,
'type' => 'php',
'dir' => '%kernel.project_dir%/config/doctrine/mappings/components',
'prefix' => 'Acme\App\Core\Component',
],
],
],
]);
};The entities configurations are these: <?php
declare(strict_types=1);
use Acme\App\Core\Component\Channel\Domain\ChannelStateEnum;
use Acme\App\Core\Component\Channel\Domain\ChannelType;
use Doctrine\ORM\Mapping\Builder\ClassMetadataBuilder;
use Doctrine\ORM\Mapping\ClassMetadata;
/** @var ClassMetadata $metadata */
$builder = new ClassMetadataBuilder($metadata);
$builder->setTable('channel__channel')
->addField(
'id',
'channel_id',
[
'id' => true,
// 'options' => [
// 'charset' => 'utf8mb4',
// 'collation' => 'utf8mb4_unicode_ci',
// ],
]
)
->addField(
'clientId',
'client_id',
[
'nullable' => false,
]
)
->createManyToOne('channelType', ChannelType::class)
->addJoinColumn($columnName = 'channel_type_id', $referencedColumnName = 'id', $nullable = false)
->build()
->addField(
'config',
'channel_config',
[
'nullable' => false,
]
)
->addField(
'stateEnum',
'channel_state_enum_type',
[
'length' => 50,
'nullable' => false,
'default' => (string) ChannelStateEnum::disabled(),
]
);
$metadata->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_NONE);<?php
declare(strict_types=1);
use Doctrine\ORM\Mapping\Builder\ClassMetadataBuilder;
use Doctrine\ORM\Mapping\ClassMetadata;
/** @var ClassMetadata $metadata */
$builder = new ClassMetadataBuilder($metadata);
$builder->setTable('channel__channel_type')
->addField(
'id',
'channel_type_id',
['id' => true]
)
->addField(
'name',
'channel_type_enum',
[
'nullable' => false,
'length' => 100,
]
)
->addField(
'enabled',
'boolean',
[
'nullable' => false,
'default' => 0,
]
);
$metadata->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_NONE);generating the first migration results in this: public function up(Schema $schema): void
{
// this up() migration is auto-generated, please modify it to your needs
$this->addSql('CREATE TABLE channel__channel (
id CHAR(26) NOT NULL COMMENT \'(DC2Type:channel_id)\',
channel_type_id CHAR(26) NOT NULL COMMENT \'(DC2Type:channel_type_id)\',
client_id INT NOT NULL COMMENT \'(DC2Type:client_id)\',
config JSON NOT NULL COMMENT \'(DC2Type:channel_config)\',
state_enum VARCHAR(50) DEFAULT \'DISABLED\' NOT NULL,
INDEX IDX_A1011C93720FB392 (channel_type_id),
PRIMARY KEY(id)
) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = InnoDB');
$this->addSql('CREATE TABLE channel__channel_type (
id CHAR(26) NOT NULL COMMENT \'(DC2Type:channel_type_id)\',
name VARCHAR(100) NOT NULL COMMENT \'(DC2Type:channel_type_enum)\',
enabled TINYINT(1) DEFAULT \'0\' NOT NULL,
PRIMARY KEY(id)
) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = InnoDB');
$this->addSql('CREATE TABLE refresh_tokens (
id INT AUTO_INCREMENT NOT NULL,
refresh_token VARCHAR(128) NOT NULL,
username VARCHAR(255) NOT NULL,
valid DATETIME NOT NULL,
UNIQUE INDEX UNIQ_9BACE7E1C74F2195 (refresh_token),
PRIMARY KEY(id)
) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = InnoDB');
$this->addSql('ALTER TABLE
channel__channel
ADD
CONSTRAINT FK_A1011C93720FB392 FOREIGN KEY (channel_type_id) REFERENCES channel__channel_type (id)');
}Then applying the migration and generating again, results in this: public function up(Schema $schema): void
{
// this up() migration is auto-generated, please modify it to your needs
$this->addSql('ALTER TABLE
channel__channel
CHANGE
id id CHAR(26) NOT NULL COMMENT \'(DC2Type:channel_id)\',
CHANGE
channel_type_id channel_type_id CHAR(26) NOT NULL COMMENT \'(DC2Type:channel_type_id)\',
CHANGE
state_enum state_enum VARCHAR(50) DEFAULT \'DISABLED\' NOT NULL');
$this->addSql('ALTER TABLE
channel__channel_type
CHANGE
id id CHAR(26) NOT NULL COMMENT \'(DC2Type:channel_type_id)\'');
}And applying the migration again and generating again, results in this: public function up(Schema $schema): void
{
// this up() migration is auto-generated, please modify it to your needs
$this->addSql('ALTER TABLE
channel__channel
CHANGE
id id CHAR(26) NOT NULL COMMENT \'(DC2Type:channel_id)\',
CHANGE
channel_type_id channel_type_id CHAR(26) NOT NULL COMMENT \'(DC2Type:channel_type_id)\',
CHANGE
state_enum state_enum VARCHAR(50) DEFAULT \'DISABLED\' NOT NULL');
$this->addSql('ALTER TABLE
channel__channel_type
CHANGE
id id CHAR(26) NOT NULL COMMENT \'(DC2Type:channel_type_id)\'');
}Any ideas? |
|
which platform are you using ? |
|
Might be fixed by #1229 . Please try out 3.4.0 |
|
@stof Sorry for the late reply. What do you mean by platform? @greg0ire Unfortunately I cant install version 3.4: My composer.json is currently this: {
"type": "project",
"license": "proprietary",
"minimum-stability": "stable",
"prefer-stable": true,
"require": {
"php": "^7.2.24",
"ext-ctype": "*",
"ext-iconv": "*",
"ext-intl": "*",
"ext-json": "*",
"ext-openssl": "*",
"composer/package-versions-deprecated": "1.11.99.1",
"doctrine/doctrine-bundle": "^2.4.2",
"doctrine/doctrine-migrations-bundle": "^3.2.1",
"doctrine/migrations": "^3.2.1",
"doctrine/orm": "^2.9.3",
"gesdinet/jwt-refresh-token-bundle": "^0.11.1",
"guzzlehttp/guzzle": "7.3.0",
"lavary/crunz": "^v2.3.1",
"league/flysystem": "^2.2.0",
"league/flysystem-sftp": "^2.1.1",
"league/uri-components": "2.2.0",
"lexik/jwt-authentication-bundle": "^v2.12.6",
"nyholm/psr7": "^1.4.1",
"psr/http-message": "^1.0.1",
"ramsey/uuid": "^4.2.0",
"spatie/url-signer": "1.2.1",
"symfony/console": "^v5.2.14",
"symfony/doctrine-messenger": "^v5.2.12",
"symfony/dotenv": "^v5.2.14",
"symfony/expression-language": "^v5.2.12",
"symfony/flex": "^v1.13.4",
"symfony/framework-bundle": "^v5.2.12",
"symfony/monolog-bundle": "^v3.7.0",
"symfony/proxy-manager-bridge": "^v5.2.12",
"symfony/psr-http-message-bridge": "^v2.1.1",
"symfony/security-bundle": "^v5.2.12",
"symfony/twig-bundle": "^v5.2.12",
"symfony/uid": "^v5.2.11",
"symfony/yaml": "^v5.2.14",
"twig/twig": "^2.12|^3.0"
},
"require-dev": {
"dama/doctrine-test-bundle": "^v6.6.0",
"doctrine/doctrine-fixtures-bundle": "^3.4.0",
"friendsofphp/php-cs-fixer": "^v2.19.1",
"malukenho/mcbumpface": "^1.1.5",
"psalm/plugin-phpunit": "^0.15.2",
"qossmic/deptrac-shim": "0.14.0",
"roave/security-advisories": "dev-latest",
"symfony/browser-kit": "^v5.2.12",
"symfony/phpunit-bridge": "^v5.3.4",
"symfony/stopwatch": "^v5.2.12",
"symfony/var-dumper": "^v5.2.12",
"symfony/web-profiler-bundle": "^v5.2.13",
"vimeo/psalm": "^4.9.2"
},
"config": {
"optimize-autoloader": true,
"preferred-install": {
"*": "dist"
},
"sort-packages": true,
"allow-plugins": {
"composer/package-versions-deprecated": true,
"malukenho/mcbumpface": true,
"symfony/flex": true
}
},
"autoload": {
"psr-4": {
"Acme\\App\\": "src/",
"Acme\\App\\Presentation\\Api\\GraphQl\\Generated\\": "var/cache/graphql/generated",
"Acme\\App\\Build\\": "build/",
"Acme\\PhpExtension\\": "lib/php-extension/src/"
}
},
"autoload-dev": {
"psr-4": {
"Acme\\PhpExtension\\Test\\": "lib/php-extension/tests/",
"Acme\\App\\Test\\": "tests/"
}
},
"replace": {
"symfony/polyfill-ctype": "*",
"symfony/polyfill-iconv": "*",
"symfony/polyfill-php72": "*"
},
"scripts": {
"auto-scripts": {
"cache:clear": "symfony-cmd",
"assets:install %PUBLIC_DIR%": "symfony-cmd"
},
"post-install-cmd": [
"@auto-scripts"
],
"post-update-cmd": [
"@auto-scripts"
]
},
"conflict": {
"symfony/symfony": "*"
},
"extra": {
"symfony": {
"allow-contrib": false,
"require": "5.2.*"
}
}
}Any suggestions? |
|
Yes. Try v3.4.0 of |
|
Yep, it's confirmed, I'm an idiot! 😬 I will try it as soon as I have some time. Tkx for the heads up. |
|
@greg0ire tkx for the suggestion, but it didn't work, the issue is still there. :( |
|
🤔 what version of |
|
We are using doctrine/dbal:2.13.5 Unfortunately we are still on PHP 7.2, and will be so for a few more months :( But I can actually check in the dummy project I created to reproduce this issue, if updating to the latest versions solves the issue. I will do that asap. |
|
As mentioned in the blog post, we do not support v2 for bugfixes anymore. |
|
Np, totally understandable, tkx for the heads up. If updating to the latest version does the trick, I will be very happy with that :) |
|
Just tried it, and it didnt solve the issue :( With a clean DB it generates public function up(Schema $schema): void
{
// this up() migration is auto-generated, please modify it to your needs
$this->addSql('CREATE TABLE channel__channel (
id CHAR(26) NOT NULL COMMENT \'(DC2Type:channel_id)\',
channel_type_id CHAR(26) NOT NULL COMMENT \'(DC2Type:channel_type_id)\',
client_id INT NOT NULL COMMENT \'(DC2Type:client_id)\',
config JSON NOT NULL COMMENT \'(DC2Type:channel_config)\',
state_enum VARCHAR(50) DEFAULT \'DISABLED\' NOT NULL COMMENT \'(DC2Type:channel_state_enum)\',
INDEX IDX_A1011C93720FB392 (channel_type_id),
PRIMARY KEY(id)
) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = InnoDB');
$this->addSql('CREATE TABLE channel__channel_type (
id CHAR(26) NOT NULL COMMENT \'(DC2Type:channel_type_id)\',
name VARCHAR(100) NOT NULL COMMENT \'(DC2Type:channel_type_enum)\',
enabled TINYINT(1) DEFAULT 0 NOT NULL,
PRIMARY KEY(id)
) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = InnoDB');
$this->addSql('CREATE TABLE refresh_tokens (
id INT AUTO_INCREMENT NOT NULL,
refresh_token VARCHAR(128) NOT NULL,
username VARCHAR(255) NOT NULL,
valid DATETIME NOT NULL,
UNIQUE INDEX UNIQ_9BACE7E1C74F2195 (refresh_token),
PRIMARY KEY(id)
) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = InnoDB');
$this->addSql('ALTER TABLE
channel__channel
ADD
CONSTRAINT FK_A1011C93720FB392 FOREIGN KEY (channel_type_id) REFERENCES channel__channel_type (id)');
}I run that migration and generate migrations again and I get this: final class Version20220127115614 extends AbstractMigration
{
public function up(Schema $schema): void
{
// this up() migration is auto-generated, please modify it to your needs
$this->addSql('ALTER TABLE
channel__channel
CHANGE
id id CHAR(26) NOT NULL COMMENT \'(DC2Type:channel_id)\',
CHANGE
channel_type_id channel_type_id CHAR(26) NOT NULL COMMENT \'(DC2Type:channel_type_id)\'');
$this->addSql('ALTER TABLE
channel__channel_type
CHANGE
id id CHAR(26) NOT NULL COMMENT \'(DC2Type:channel_type_id)\'');
}
public function down(Schema $schema): void
{
// this down() migration is auto-generated, please modify it to your needs
$this->addSql('ALTER TABLE
channel__channel
CHANGE
id id CHAR(26) CHARACTER SET utf8mb4 NOT NULL COLLATE `utf8mb4_unicode_ci` COMMENT \'(DC2Type:channel_id)\',
CHANGE
channel_type_id channel_type_id CHAR(26) CHARACTER SET utf8mb4 NOT NULL COLLATE `utf8mb4_unicode_ci` COMMENT \'(DC2Type:channel_type_id)\'');
$this->addSql('ALTER TABLE
channel__channel_type
CHANGE
id id CHAR(26) CHARACTER SET utf8mb4 NOT NULL COLLATE `utf8mb4_unicode_ci` COMMENT \'(DC2Type:channel_type_id)\'');
}
} |
|
Ok… if you debug it, I think you will end up here: migrations/lib/Doctrine/Migrations/Generator/DiffGenerator.php Lines 93 to 105 in e17a946 If you do, and you find that the issue comes from DBAL, try reproducing the issue with the DBAL API (by creating a schema with the DBAL API), and then I would transfer that issue to the DBAL repository. If you can publish your dummy project, that can help too. |
|
My dummy project is here: Tkx for your support in any case, very appreciated. |
|
I don't think you actually have a "Transfer issue" button, but as an admin, I do, so ping me when you think it's time :) |
|
@greg0ire I also debuged and used a break point where you mentioned. Feel free to transfer this issue to the dbal project, if you feel that's the way to go to. I really cant figure out if the issue is on the dbal side or this side. |
|
I will try your dummy project to be sure, and then we'll see |
|
This comment is really interesting: https://github.com/doctrine/dbal/blob/3.3.x/src/Platforms/MySQL/Comparator.php#L15-L17 |
|
I couldn't make the dummy project work: hgraca/migrations-shiet-show#1 |
|
Uff, some owner issues and maybe folders missing. I will solve it this afternoon and test it from scratch. Sorry for wasting your time. |
|
No worries 🙂 |
|
Did some changes and pushed. make docker-create-network # You only need to run this once
make docker-run-testit should work. if you want to enable xdebug, you can do it by uncommenting the first line in |
😛 |
|
TL;DR; I had the same. |
|
Ok, I got it to work! |
|
Here is a zoom on the diff The things that are detected as changed are not the charset or collation, but So it seems related to https://github.com/hgraca/migrations-shiet-show/blob/8d66a6c0578812942ed4daab289e32bebeef9112/src/Infrastructure/Persistence/Doctrine/Type/AbstractUlidType.php#L14-L20 |
|
Oh, it looks like platform-aware comparison is not in use here 🤔 |
|
I think you reported that issue to the correct package, and I think the problem is here: |
|
Oh and also we use |
|
Even when overcoming this with various fixes and hacks, the 2 columns are considered different at the SQL level: |
|
Hold on:
This will make it work properly in the next major (doctrine/dbal#4722) but I'm not sure what to do now. |
|
I was trying to figure out which one comes from PHP and which one comes from MySQL by changing the doctrine type name from channel_id to something else, and both SQL definitions changed… I would expect one to come from the database 🤔 I'm super confused right now. |
|
If it's possible to detect how it is called ( |
|
IIRC, it was possible in PHP 4 but not now. Unlike the accepted answer above, in our case, the method is explicitly marked as |
Maybe we have to? |
|
I'm not sure if replacing a method with the magic ones could be counted as backward-compatible. But it looks like worth exploring. |
|
It could be considered a BC-break for |
|
An optional parameter will be an immediate BC break: https://3v4l.org/PmUnp |
|
I don't feel good about replacing the method with the magic ones and then introducing it back in 4.0. It's one of those cases when the cure is worse than the disease. Instead, we could be either of:
Personally, I'd like to release new versions more often than to put band-aids on the poorly designed ones. |
|
Just when I was going to suggest we use From the consumer standpoint, it would be better than 2. though, I think. So I'd say either 1. or |
|
Or actually, the temporary parameter can be introduced via |
|
Yes, that's what I was proposing… it still involves adding a parameter in this package, then cleaning it up after requiring DBAL 4. BTW, the ORM is also affected by this: https://github.com/doctrine/orm/blob/223b2650c46d97fcfcabfc577e939cef51915e94/lib/Doctrine/ORM/Tools/SchemaTool.php#L945 This is how it makes the upgrade path look like Comparator::compareSchemas($fromSchema, $toSchema); // the past
$comparator->compareSchemas($fromSchema, $toSchema, $comparator); // ugly and static analysis won't shut up about it
$comparator->compareSchemas($fromSchema, $toSchema); // in a distant future |
|
This looks fine to me. Those who will want to adopt the temporary solution shouldn't have a problem to update their static analysis configuration. The distant future may happen as soon as the ORM starts supporting DBAL 4. |
|
For direct consumers yes, for |
|
@hgraca can you please update your test project to |
|
@greg0ire done. |
|
I've tried connecting to the database with a MySQL client. Here is the definition of Now let's try to execute part of the migration: ALTER TABLE channel__channel CHANGE id id CHAR(26) CHARACTER SET utf8mb4 NOT NULL COLLATE `utf8mb4_unicode_ci` COMMENT '(DC2Type:channel_id)';And now let's check the definition again: Let's diff the column definition for - `id` char(26) COLLATE utf8mb4_unicode_ci NOT NULL COMMENT '(DC2Type:channel_id)',
+ `id` char(26) COLLATE utf8mb4_unicode_ci NOT NULL COMMENT '(DC2Type:channel_id)',As you can see, there is no diff at all, for some reason. Next steps:
|
|
For me, I have downgraded doctrine-bundle 2.5.6 used And after I could downgrade dbal to 3.2.0 instead of 3.3.2... Fix my problem for the moment! |
Bug Report
Summary
We generate migrations, and run them. This should put the ORM metadata and the actual DB in sync.
However, if we generate migrations again, it will detect changes and generate some stuff again.
Current behavior
It generates migrations when the DB and metadata are in sync.
If we apply the generated migration and generate migrations again, it generates another migration, equal to the previous one.
This seems to be related to the charset and collation.
How to reproduce
I created a repository to reproduce the issue: https://github.com/hgraca/migrations-shiet-show
It uses docker, so you can just checkout the repository and run the following in the CLI:
Check the last migration file created, in
build/Migration/Version.We can try running that migration and creating a new one, surely that would put everything in sync...
Check the new migration created.
It's exactly the same as the previous one, except for the class name.
Other info
I tried applying the changes (currently commented out) in:
config/doctrine/mappings/components/CM.GlobalTicket.Core.Component.Channel.Domain.Channel.phpconfig/packages/doctrine.phpUnfortunately nothing works.
Expected behavior
After generating a migration and applying it, the DB and mapping should be in sync, therefore generating a new migration should result in no new migrations.
Let me know if I can be of any assistance. I tried debugging this with xdebug but it gets very confusing.
I appreciate any help you can give.
And tkx for all your work on Doctrine & friends.
The text was updated successfully, but these errors were encountered: