From 6c757b11e0125892ca215cc82667daebbe787626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Biarda?= <1135380+michalbiarda@users.noreply.github.com> Date: Wed, 13 May 2026 17:31:38 +0200 Subject: [PATCH 1/2] fix(database): use SchemaRegistry in DiffCommand and MigrateCommand for extender merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Single-pass buildEntitySchema loop called schemaBuilder->build() per entity independently, so extender columns (Table(extends:)) were never merged into the parent table schema. Commands saw extender columns as missing from the entity definition — reporting existing ones as destructive drops and never generating ADD COLUMN for new ones. Replaced the loop with SchemaRegistry::registerEntities(), which already implements the correct two-pass merge. Removed the now-unused EntityMetadataFactory and SchemaBuilder constructor dependencies from both commands; updated test helpers accordingly. Co-Authored-By: Claude Sonnet 4.6 --- packages/database/src/Command/DiffCommand.php | 17 +++++------------ .../database/src/Command/MigrateCommand.php | 17 +++++------------ packages/database/tests/Command/Helpers.php | 4 ++-- .../tests/Command/MigrateCommandTest.php | 7 +++---- 4 files changed, 15 insertions(+), 30 deletions(-) diff --git a/packages/database/src/Command/DiffCommand.php b/packages/database/src/Command/DiffCommand.php index fcbee57f..0996a3f9 100644 --- a/packages/database/src/Command/DiffCommand.php +++ b/packages/database/src/Command/DiffCommand.php @@ -13,10 +13,9 @@ use Marko\Database\Diff\SchemaDiff; use Marko\Database\Diff\TableDiff; use Marko\Database\Entity\EntityDiscovery; -use Marko\Database\Entity\EntityMetadataFactory; -use Marko\Database\Entity\SchemaBuilder; use Marko\Database\Exceptions\EntityException; use Marko\Database\Introspection\IntrospectorInterface; +use Marko\Database\Schema\SchemaRegistry; use Marko\Database\Schema\Table; /** @noinspection PhpUnused */ @@ -26,8 +25,7 @@ public function __construct( private EntityDiscovery $discovery, private IntrospectorInterface $introspector, - private EntityMetadataFactory $metadataFactory, - private SchemaBuilder $schemaBuilder, + private SchemaRegistry $schemaRegistry, private DiffCalculator $diffCalculator, private ProjectPaths $paths, ) {} @@ -77,15 +75,10 @@ public function execute( private function buildEntitySchema( array $entityClasses, ): array { - $schema = []; - - foreach ($entityClasses as $entityClass) { - $metadata = $this->metadataFactory->parse($entityClass); - $table = $this->schemaBuilder->build($metadata); - $schema[$table->name] = $table; - } + $this->schemaRegistry->clear(); + $this->schemaRegistry->registerEntities($entityClasses); - return $schema; + return $this->schemaRegistry->getTables(); } /** diff --git a/packages/database/src/Command/MigrateCommand.php b/packages/database/src/Command/MigrateCommand.php index d5bad656..0e47e678 100644 --- a/packages/database/src/Command/MigrateCommand.php +++ b/packages/database/src/Command/MigrateCommand.php @@ -13,14 +13,13 @@ use Marko\Database\Diff\SchemaDiff; use Marko\Database\Diff\SqlGeneratorInterface; use Marko\Database\Entity\EntityDiscovery; -use Marko\Database\Entity\EntityMetadataFactory; -use Marko\Database\Entity\SchemaBuilder; use Marko\Database\Exceptions\EntityException; use Marko\Database\Exceptions\MigrationException; use Marko\Database\Introspection\IntrospectorInterface; use Marko\Database\Migration\DataMigrator; use Marko\Database\Migration\MigrationGenerator; use Marko\Database\Migration\Migrator; +use Marko\Database\Schema\SchemaRegistry; use Marko\Database\Schema\Table; /** @noinspection PhpUnused */ @@ -33,8 +32,7 @@ public function __construct( private MigrationGenerator $migrationGenerator, private EntityDiscovery $entityDiscovery, private IntrospectorInterface $introspector, - private EntityMetadataFactory $metadataFactory, - private SchemaBuilder $schemaBuilder, + private SchemaRegistry $schemaRegistry, private DiffCalculator $diffCalculator, private SqlGeneratorInterface $sqlGenerator, private ProjectPaths $paths, @@ -251,15 +249,10 @@ private function calculateDiff(): SchemaDiff private function buildEntitySchema( array $entityClasses, ): array { - $schema = []; - - foreach ($entityClasses as $entityClass) { - $metadata = $this->metadataFactory->parse($entityClass); - $table = $this->schemaBuilder->build($metadata); - $schema[$table->name] = $table; - } + $this->schemaRegistry->clear(); + $this->schemaRegistry->registerEntities($entityClasses); - return $schema; + return $this->schemaRegistry->getTables(); } /** diff --git a/packages/database/tests/Command/Helpers.php b/packages/database/tests/Command/Helpers.php index 4e750fe0..5b6e9190 100644 --- a/packages/database/tests/Command/Helpers.php +++ b/packages/database/tests/Command/Helpers.php @@ -14,6 +14,7 @@ use Marko\Database\Entity\EntityDiscovery; use Marko\Database\Entity\EntityMetadataFactory; use Marko\Database\Entity\SchemaBuilder; +use Marko\Database\Schema\SchemaRegistry; use Marko\Database\Introspection\IntrospectorInterface; use Marko\Database\Schema\Table; @@ -229,8 +230,7 @@ public static function createDiffCommand( return new DiffCommand( discovery: self::createStubEntityDiscovery(), introspector: self::createStubIntrospector($tables), - metadataFactory: new EntityMetadataFactory(), - schemaBuilder: new SchemaBuilder(), + schemaRegistry: new SchemaRegistry(new EntityMetadataFactory(), new SchemaBuilder()), diffCalculator: $diffCalculator ?? new DiffCalculator(), paths: new ProjectPaths('/test'), ); diff --git a/packages/database/tests/Command/MigrateCommandTest.php b/packages/database/tests/Command/MigrateCommandTest.php index 34b9c4de..2847f127 100644 --- a/packages/database/tests/Command/MigrateCommandTest.php +++ b/packages/database/tests/Command/MigrateCommandTest.php @@ -12,6 +12,7 @@ use Marko\Database\Diff\SqlGeneratorInterface; use Marko\Database\Entity\EntityMetadataFactory; use Marko\Database\Entity\SchemaBuilder; +use Marko\Database\Schema\SchemaRegistry; use Marko\Database\Exceptions\MigrationException; use Marko\Database\Migration\DataMigrator; use Marko\Database\Migration\MigrationGenerator; @@ -304,8 +305,7 @@ function createMigrateCommand( migrationGenerator: $generator ?? createMigrationGeneratorStub(), entityDiscovery: Helpers::createStubEntityDiscovery(), introspector: Helpers::createStubIntrospector(), - metadataFactory: new EntityMetadataFactory(), - schemaBuilder: new SchemaBuilder(), + schemaRegistry: new SchemaRegistry(new EntityMetadataFactory(), new SchemaBuilder()), diffCalculator: createMigrateDiffCalculator($diff ?? new SchemaDiff()), sqlGenerator: $sqlGenerator ?? createMigrateSqlGenerator(), paths: new ProjectPaths('/test'), @@ -684,8 +684,7 @@ function executeMigrateCommand( migrationGenerator: $generator, entityDiscovery: Helpers::createStubEntityDiscovery(), introspector: $introspector, - metadataFactory: new EntityMetadataFactory(), - schemaBuilder: new SchemaBuilder(), + schemaRegistry: new SchemaRegistry(new EntityMetadataFactory(), new SchemaBuilder()), diffCalculator: new DiffCalculator(), sqlGenerator: createMigrateSqlGenerator(), paths: new ProjectPaths('/test'), From d1a8bb71f6ac0ca7bbf7271babb0c43cf01266ac Mon Sep 17 00:00:00 2001 From: Mark Shust Date: Sun, 24 May 2026 11:18:29 -0400 Subject: [PATCH 2/2] test(database): add regression tests locking in extender merge for db commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DiffCommandTest builds an extender entity (BasicExtenderEntity extends ExtenderParentEntity), seeds the introspector with the schema produced by SchemaRegistry, and asserts "No changes detected" — pre-fix this would report "Drop column: extra" as destructive. MigrateCommandTest wires a capturing DiffCalculator and asserts the entitySchema passed to it contains both `id` (parent) and `extra` (extender), proving the merge happened before the diff was computed. Co-Authored-By: Michał Biarda <1135380+michalbiarda@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) --- .../tests/Command/DiffCommandTest.php | 31 +++++++++++ packages/database/tests/Command/Helpers.php | 6 ++- .../tests/Command/MigrateCommandTest.php | 54 ++++++++++++++++++- 3 files changed, 88 insertions(+), 3 deletions(-) diff --git a/packages/database/tests/Command/DiffCommandTest.php b/packages/database/tests/Command/DiffCommandTest.php index a0348fcd..6afc868c 100644 --- a/packages/database/tests/Command/DiffCommandTest.php +++ b/packages/database/tests/Command/DiffCommandTest.php @@ -8,11 +8,16 @@ use Marko\Database\Diff\DiffCalculator; use Marko\Database\Diff\SchemaDiff; use Marko\Database\Diff\TableDiff; +use Marko\Database\Entity\EntityMetadataFactory; +use Marko\Database\Entity\SchemaBuilder; use Marko\Database\Schema\Column; use Marko\Database\Schema\Index; use Marko\Database\Schema\IndexType; +use Marko\Database\Schema\SchemaRegistry; use Marko\Database\Schema\Table; use Marko\Database\Tests\Command\Helpers; +use Marko\Database\Tests\Entity\Fixtures\ExtenderFactory\BasicExtenderEntity; +use Marko\Database\Tests\Entity\Fixtures\ExtenderFactory\ExtenderParentEntity; it('registers as db:diff command via #[Command] attribute', function (): void { $reflection = new ReflectionClass(DiffCommand::class); @@ -349,3 +354,29 @@ public function calculate( expect($exitCode2)->toBe(1); }); + +it('merges extender columns into parent table schema (regression for #66)', function (): void { + // Build the expected merged schema via the real SchemaRegistry so the + // introspector stub mirrors exactly what DiffCommand will compute. If + // DiffCommand bypasses the extender merge, the entity-side schema will + // be missing the extender's `extra` column and DiffCalculator will + // report it as a destructive drop. + $registry = new SchemaRegistry( + new EntityMetadataFactory(), + new SchemaBuilder(), + ); + $registry->registerEntities([ExtenderParentEntity::class, BasicExtenderEntity::class]); + $mergedTables = $registry->getTables(); + + expect($mergedTables['users']->columns)->toHaveCount(2); + + $command = Helpers::createDiffCommand( + tables: $mergedTables, + entities: [ExtenderParentEntity::class, BasicExtenderEntity::class], + ); + + ['output' => $output, 'exitCode' => $exitCode] = Helpers::executeDiffCommand($command); + + expect($output)->toContain('No changes detected') + ->and($exitCode)->toBe(0); +}); diff --git a/packages/database/tests/Command/Helpers.php b/packages/database/tests/Command/Helpers.php index 5b6e9190..c2f1bad1 100644 --- a/packages/database/tests/Command/Helpers.php +++ b/packages/database/tests/Command/Helpers.php @@ -14,8 +14,8 @@ use Marko\Database\Entity\EntityDiscovery; use Marko\Database\Entity\EntityMetadataFactory; use Marko\Database\Entity\SchemaBuilder; -use Marko\Database\Schema\SchemaRegistry; use Marko\Database\Introspection\IntrospectorInterface; +use Marko\Database\Schema\SchemaRegistry; use Marko\Database\Schema\Table; /** @@ -222,13 +222,15 @@ public function lastInsertId(): int * Helper to create a DiffCommand with standard dependencies. * * @param array $tables Tables for introspector + * @param array $entities Entity classes for discovery */ public static function createDiffCommand( ?DiffCalculator $diffCalculator = null, array $tables = [], + array $entities = [], ): DiffCommand { return new DiffCommand( - discovery: self::createStubEntityDiscovery(), + discovery: self::createStubEntityDiscovery($entities), introspector: self::createStubIntrospector($tables), schemaRegistry: new SchemaRegistry(new EntityMetadataFactory(), new SchemaBuilder()), diffCalculator: $diffCalculator ?? new DiffCalculator(), diff --git a/packages/database/tests/Command/MigrateCommandTest.php b/packages/database/tests/Command/MigrateCommandTest.php index 2847f127..f75e607f 100644 --- a/packages/database/tests/Command/MigrateCommandTest.php +++ b/packages/database/tests/Command/MigrateCommandTest.php @@ -12,7 +12,6 @@ use Marko\Database\Diff\SqlGeneratorInterface; use Marko\Database\Entity\EntityMetadataFactory; use Marko\Database\Entity\SchemaBuilder; -use Marko\Database\Schema\SchemaRegistry; use Marko\Database\Exceptions\MigrationException; use Marko\Database\Migration\DataMigrator; use Marko\Database\Migration\MigrationGenerator; @@ -20,8 +19,11 @@ use Marko\Database\Schema\Column; use Marko\Database\Schema\ForeignKey; use Marko\Database\Schema\Index; +use Marko\Database\Schema\SchemaRegistry; use Marko\Database\Schema\Table; use Marko\Database\Tests\Command\Helpers; +use Marko\Database\Tests\Entity\Fixtures\ExtenderFactory\BasicExtenderEntity; +use Marko\Database\Tests\Entity\Fixtures\ExtenderFactory\ExtenderParentEntity; /** * Create a stub Migrator for testing. @@ -697,3 +699,53 @@ function executeMigrateCommand( expect($output)->toContain('Nothing to migrate') ->and($generator->generateCalled)->toBeFalse(); }); + +it('merges extender columns into parent table schema before computing diff (regression for #66)', function (): void { + // Capture the entitySchema passed to DiffCalculator so we can prove the + // extender's `extra` column was merged into the parent `users` table + // before the diff was computed. Pre-fix the command bypassed + // SchemaRegistry's extender merge and the extender column would be + // absent from $entitySchema['users']. + /** @var array|null $captured */ + $captured = null; + + $capturingCalculator = new class ($captured) extends DiffCalculator + { + public function __construct( + /** @noinspection PhpPropertyOnlyWrittenInspection - Reference property captures schema for assertion */ + private ?array &$captured, + ) {} + + public function calculate( + array $entitySchema, + array $databaseSchema, + ): SchemaDiff { + $this->captured = $entitySchema; + + return new SchemaDiff(); + } + }; + + $command = new MigrateCommand( + migrator: createMigratorStub(), + dataMigrator: createDataMigratorStub(), + migrationGenerator: createMigrationGeneratorStub(), + entityDiscovery: Helpers::createStubEntityDiscovery([ExtenderParentEntity::class, BasicExtenderEntity::class]), + introspector: Helpers::createStubIntrospector(), + schemaRegistry: new SchemaRegistry(new EntityMetadataFactory(), new SchemaBuilder()), + diffCalculator: $capturingCalculator, + sqlGenerator: createMigrateSqlGenerator(), + paths: new ProjectPaths('/test'), + isProduction: false, + ); + + executeMigrateCommand($command); + + expect($captured)->toHaveKey('users') + ->and($captured['users']->columns)->toHaveCount(2); + + $columnNames = array_map(fn (Column $c): string => $c->name, $captured['users']->columns); + + expect($columnNames)->toContain('id') + ->and($columnNames)->toContain('extra'); +});