Refactor multiple databases support by vjik · Pull Request #371 · yiisoft/db-migration

vjik · 2026-10-15T10:32:39Z

Q	A
Is bugfix?	❌
New feature?	❌
Breaks BC?	❌

codecov · 2026-10-15T10:33:30Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.48%. Comparing base (6f21d40) to head (3474f24).

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #371      +/-   ##
============================================
+ Coverage     97.36%   97.48%   +0.11%     
- Complexity      373      395      +22     
============================================
  Files            25       30       +5     
  Lines          1176     1232      +56     
============================================
+ Hits           1145     1201      +56     
  Misses           31       31

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

🟡 Changes recommended

Registry validation now accepts previously rejected blank database names as migration targets.

1 open finding

Empty registry names bypass validation and create blank database headings · New

What changed in this PR

Refactors multi-database migration support into database configuration, service setup, and command-specific contexts.

Changes:

Replaces database sets with configuration and registry classes.
Moves database-selection policies and context creation into commands.
Updates integration wiring, documentation, and tests.

File	Description
tests/Migration/DatabaseSetRegistryTest.php	Removes old registry tests.
tests/Migration/DatabaseRegistryTest.php	Tests replacement registry and setup creation.
tests/Migration/ConfigTest.php	Updates registry integration assertions.
tests/Migration/BinTest.php	Uses renamed configuration class.
tests/Driver/Sqlite/DatabaseCommandsTest.php	Adapts multi-database command tests.
src/Service/Generate/CreateService.php	Extracts table-prefix default constant.
src/Migrator.php	Extracts migration default constants.
src/DatabaseSetup.php	Bundles configuration and migration services.
src/DatabaseSetRegistry.php	Removes old registry implementation.
src/DatabaseRegistry.php	Adds configuration validation and setup creation.
src/DatabaseConfig.php	Renames configuration class and shares defaults.
src/Command/UpdateCommand.php	Adds update context and selection policy.
src/Command/RedoCommand.php	Adds redo context and selection policy.
src/Command/NewCommand.php	Uses migration context and nullable empty results.
src/Command/MissingDatabaseOptionPolicy.php	Defines database-selection policies.
src/Command/MarkCommand.php	Uses migration context and explicit selection policy.
src/Command/HistoryCommand.php	Uses migration context and nullable empty results.
src/Command/DownCommand.php	Adds rollback context and selection policy.
src/Command/DatabaseCommand.php	Centralizes policy-based dispatch and result handling.
src/Command/CreateCommand.php	Creates generation-specific contexts.
src/Command/Context/UpdateContext.php	Bundles update services.
src/Command/Context/RedoContext.php	Replaces generic context with redo services.
src/Command/Context/MigrationContext.php	Bundles shared migration services.
src/Command/Context/DownContext.php	Bundles rollback services.
src/Command/Context/CreateContext.php	Bundles generation services.
docs/guide/en/usage-with-yii-console.md	Updates configuration class reference.
docs/guide/en/usage-with-symfony.md	Updates services and discovery exclusions.
docs/guide/en/usage-standalone.md	Updates configuration examples.
config/di-console.php	Wires replacement registry and configuration.
CHANGELOG.md	Links the refactor to multi-database support.
bin/yii-db-migration.php	Updates configuration type comment.
bin/yii-db-migration	Instantiates replacement registry.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

🟢 Approval recommended

The refactor preserves command behavior, the renamed types are unreleased, and no blocking issues remain; tests were not executed.

0 open findings

1 resolved since last review

Empty registry names bypass validation and create blank database headings

🧠 Review effort: Balanced

KalimeroMK

Checked out locally: psalm level 1 clean, 247 tests green, and execute() is behaviourally equivalent to master — listing commands have no FAILURE paths left, so null only replaces the empty-set case (and a real FAILURE from a listing command now propagates instead of being swallowed by continue). Dropping the $context->migrator ?? $this->migrator fallbacks and the $this instanceof DownCommand || ... chain is the win here. Comments below are about duplication and where default now lives — nothing blocking.

KalimeroMK · 2026-10-15T11:29:16Z

-        $name = $input->getOption('db');
-        $names = $this->databases?->getNames() ?? [DatabaseSetRegistry::DEFAULT_DATABASE];
-        $multipleDatabases = count($names) > 1;
+        $availableNames = [DatabaseRegistry::DEFAULT_DATABASE, ...($this->databases?->getNames() ?? [])];


default is now known in two places: the registry owns DEFAULT_DATABASE but refuses to resolve it, while the command re-adds it here and special-cases it on line 49. Either move the constant here, or let the registry keep owning the concept (getAllNames() default-first + isDefault()).

The registry only manages additional databases, while default comes from the command's own dependencies. The constant lives in the registry because the registry is what reserves the name and rejects it in the config. Resolving default is the command's job, since only the command has those dependencies. I renamed the command's dependency to $additionalDatabases, so the split is visible at the call site now.

KalimeroMK · 2026-10-15T11:29:16Z

+    /**
+     * Executes the command for a single database.
+     *
+     * @psalm-param TContext $context


Psalm does substitute TContext here — an undefined property on $context is an error at level 1. But it does not check createContext()/createDefaultContext() against @psalm-return TContext: I made UpdateCommand::createContext() return a MigrationContext and psalm stayed green. A mismatch is a runtime Error, caught only by the tests.

Good catch. Psalm doesn't substitute class templates when it checks an overriding return type against @psalm-return TContext. I don't see a clean way around that with annotations. Each command has tests that run it against both the default and a named database, so a mismatch fails there right away.

KalimeroMK · 2026-10-15T11:29:17Z

+        return new MigrationContext($this->migrator, $this->migrationService);
+    }
+
+    protected function createContext(DatabaseSetup $setup): MigrationContext


These two are verbatim identical in MarkCommand and NewCommand — six copies. An intermediate abstract class MigrationCommand extends DatabaseCommand<MigrationContext> would hold them once.

These are one-line constructor calls. A separate abstract level only to hold them makes the hierarchy deeper and hides what each command uses. I'd keep them explicit.

KalimeroMK · 2026-10-15T11:29:17Z

            $io->warning('No migration has been done before.');

-            return Command::FAILURE;
+            return null;


The convention is only half applied: down/redo still signal the same empty result with Command::FAILURE (DownCommand.php:118,131, RedoCommand.php:125,137) and up with SUCCESS. No difference while those are RejectIfMultiple, but it becomes one if that ever relaxes.

Fixed: down and redo now return null for an empty result as well. up returns SUCCESS on purpose, because "nothing to apply" is a normal up-to-date state, the same as on master.

KalimeroMK · 2026-10-15T11:29:17Z

            ->addOption('force-yes', 'y', InputOption::VALUE_NONE, 'Force yes to all questions.');
    }

+    protected function getMissingDatabaseOptionPolicy(InputInterface $input): MissingDatabaseOptionPolicy


This and getMissingDatabaseOptionError() below are verbatim copies of NewCommand's. A MissingDatabaseOptionPolicy::forSourceOverride($input) helper, or a default in the base class, would cover both.

It's only two copies, and I'd rather keep the policy explicit in each command, so you see it without jumping to a helper.

KalimeroMK · 2026-10-15T11:29:17Z

+
+    protected function createContext(DatabaseSetup $setup): DownContext
+    {
+        return new DownContext($setup->migrator, $setup->migrationService, new DownRunner($setup->migrator));


new DownRunner($setup->migrator) also lives in RedoCommand, and new UpdateRunner(...) in two places — construction used to be in one place. Factory methods on DatabaseSetup would keep the laziness and the single site, and let DatabaseSetup::$config go; it exists only so CreateCommand can read db and useTablePrefix.

new DownRunner($migrator) is a trivial one-liner. Keeping DatabaseSetup a plain data object, and leaving each command to build its own services, keeps DatabaseSetup independent of the command-specific services. I'd keep it as is.

KalimeroMK · 2026-10-15T11:29:17Z

+     *
+     * @psalm-return list<string>
+     */
+    public function getNames(): array


Meaning changed from "all, default first" to "additional only". Safe BC-wise since nothing shipped, but the name no longer says what it returns — getAdditionalNames() would.

The whole registry is about additional databases (see the class docblock), so getNames() there already means "names of the databases in this registry". Adding "additional" to one method would be inconsistent with createSetup(). Instead, I renamed the command's dependency to $additionalDatabases, so the call site now reads $this->additionalDatabases?->getNames().

KalimeroMK · 2026-10-15T11:29:17Z

+    public function createSetup(string $name): DatabaseSetup
+    {
+        $config = $this->configs[$name]
+            ?? throw new InvalidArgumentException(sprintf('Unknown additional database: %s.', $name));


Unknown additional database: default. reads wrong for a name that is a valid --db value, just not registry-managed.

createSetup() is @internal, and the command never calls it for default, so users can't see this message. For a direct call it's accurate: default is not an additional database.

KalimeroMK · 2026-10-15T11:29:17Z


-Each set is an array of `Yiisoft\Db\Migration\DatabaseSet` constructor arguments, where `db` is the ID of a connection
-service. Define these connections in the DI container, for example, in `config/common/db.php`:
+Each set is an array of `Yiisoft\Db\Migration\DatabaseConfig` constructor arguments, where `db` is the ID of


The docs still say "set" while the API no longer does — also line 99 here, multiple-databases.md:4-6, README.md:56 and usage-with-symfony.md:99. Worth finishing the rename.

Fixed, thanks.

# Conflicts: # tests/Driver/Sqlite/DatabaseCommandsTest.php

KalimeroMK

Looks good — thanks for the fixes and the explanations.

* Removed `MissingDatabaseOptionPolicy` * Fix * Rename `getAllDatabaseNames()` to `getAvailableDatabaseNames()`

vjik added 7 commits October 15, 2026 12:40

Refactor

16f2232

Refactor

73c9b55

Refactor

ae5a250

Refactor

75d365a

Refactor

ddeac16

Refactor

8076764

Refactor

f996b62

vjik requested a balanced review from Copilot October 15, 2026 10:34

Copilot started reviewing on behalf of vjik October 15, 2026 10:35

View session

Copilot AI reviewed Oct 15, 2026

View reviewed changes

Comment thread src/DatabaseRegistry.php

fix

d3c96ff

vjik requested a balanced review from Copilot October 15, 2026 11:02

Copilot started reviewing on behalf of vjik October 15, 2026 11:04

View session

Copilot AI reviewed Oct 15, 2026

View reviewed changes

vjik requested a review from a team October 15, 2026 11:10

vjik added the status:code review The pull request needs review. label Oct 15, 2026

KalimeroMK reviewed Oct 15, 2026

View reviewed changes

vjik added 2 commits October 15, 2026 18:57

Merge remote-tracking branch 'origin/master' into refactor

348e954

# Conflicts: # tests/Driver/Sqlite/DatabaseCommandsTest.php

improve

38e96e2

vjik requested review from a team and KalimeroMK October 15, 2026 16:16

KalimeroMK approved these changes Oct 15, 2026

View reviewed changes

Tigrov mentioned this pull request Oct 16, 2026

Removed MissingDatabaseOptionPolicy #372

Merged

Removed MissingDatabaseOptionPolicy (#372)

3474f24

* Removed `MissingDatabaseOptionPolicy` * Fix * Rename `getAllDatabaseNames()` to `getAvailableDatabaseNames()`

Tigrov approved these changes Oct 16, 2026

View reviewed changes

vjik merged commit 51b0a94 into master Oct 16, 2026
58 checks passed

vjik deleted the refactor branch October 16, 2026 12:41

Conversation

vjik commented Oct 15, 2026

Uh oh!

codecov Bot commented Oct 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

KalimeroMK left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KalimeroMK left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Oct 15, 2026 •

edited

Loading