Sorting migrations breaks listing them

[mamazu]

Bug Report

Q	A
BC Break	no
Version	3.9.7

Summary

In doctrine you can sort the migrations. However enabling this sorting uses a different migration finder. This leads to the fact that if you enable sorting doctrine finds more migrations than if you don't.

Current behavior

If you list migrations in the sulu/sulu project by default it will use the GlobFinder to find migrations (classes) starting with "Version". This works fine. However when using the RecursiveRegexFilter (which gets enabled when switching on the sorting, in the doctrine bundle) then it returns all classes in the Migration directory. In this case this also includes an abstract base class or the migration called "AbstractTagNameToIdMigration" which is an abstract class and can not be instantiated.

How to reproduce

• Create an abstract migration class that can not be instantiated
• Name the class something like "AbstractMigrationVersion" or something not matching the naming scheme
• See it getting picked up

Expected behavior

Make the regex finder also filter down to migrations matching "Version*" like in the glob filter. Or use the same Migration Finder for both and do the sorting afterwards.

More Infos

sulu/sulu#8874

[greg0ire]

I don't understand why we even need 2 implementations of the Finder… @goetas maybe you know?

If RecursiveRegexFinder covers all cases, then we should IMO deprecate GlobFinder in favor of it.

Make the regex finder also filter down to migrations matching "Version*" like in the glob filter.

Wouldn't that be a breaking change?

[mamazu]

I think either way we come down on this issue would be a breaking change. Either we use the RegexFilter and break the normal use case, or we use the GlobFilter everywhere and exclude migrations. But I think making the change would be good either way so that it's a unified handling.

However I don't know how you would determine an order if you allow any file names. Just lexicographically ordering file names?

[greg0ire]

I think either way we come down on this issue would be a breaking change. Either we use the RegexFilter and break the normal use case, or we use the GlobFilter everywhere and exclude migrations. But I think making the change would be good either way so that it's a unified handling.

Maybe we can make that opt-in somehow.

However I don't know how you would determine an order if you allow any file names. Just lexicographically ordering file names?

Yes? That's probably what GlobFinder does, is it not?

[mamazu]

I had a look at the implementation and I think I know what's going on. For "unorganized" migrations it's just a list of classes like "Version20260531.php" But for organized migrations it's "2026/03/Migration.php" which then explains why the regex doesn't filter. Because all migrations in the directory make sense.

However the problem then comes when you're trying to mix and match. For example you have your own migrations under src/Migrations sorted in the folder structure but then a plugin you installed uses the list structure. It falls back to the main directory correctly but then still uses the "all migrations in the directory approach".

I think the "correct" way would be to specify the kind of organization per included migration directory.

[greg0ire]

I think the "correct" way would be to specify the kind of organization per included migration directory.

So you would have the user specify the organization? Shouldn't it be specified by the plugin instead? Maybe that's already what's happening though, I'm not sure how Sulu\Page\Migrations: /app/vendor/sulu/sulu/packages/page/src/Migrations ends up under migrations_paths. Is that configured by the end user or by Sulu?

[mamazu]

This is specified by Sulu, but Sulu assumes that the user is using the glob parser which filters this abstract class out, but the user requires this config and uses the regex parser which doesn't exclude this and it crashes when listing or running the migration since it's unable to instanitate the class.

[greg0ire]

Then I agree with what you previously said. We should allow specifying a per-directory organization.

[mamazu]

Well I would suggest just removing this organization strategy thing all together. I think just making the directory and the match pattern configurable should be enough.

doctrine_migrations:
    migration_paths:
        SomeMigrations: /app/some_migrations # prefill with default "Version*.php" filter.
        Sulu\Content\Migrations: 
             path: /app/vendor/sulu/sulu/packages/content/src/Migrations
             filter: 'Version*.php' # for a list of migrations
        DoctrineMigrations:
             path:  /app/migrations
             filter: '**/*.php' # Replacing the regex finder with a glob that's recursive
            generator: '%Y/%m/MigrationOfDay%d.php' 

And then remove the organize_migrations option all together. This might be a little more configuration but it would give full control over how migrations can be found.

Alternatively, I also like the approach of just the RecursiveRegexFinder and saying all php files in the migration directory would also be fine. As long as it stays consistent, I don't mind either.

[greg0ire]

             filter: '**/*.php' # Replacing the regex finder with a glob that's recursive
            generator: '%Y/%m/MigrationOfDay%d.php' 

Having to configure 2 things that must stay coherent with each other feels a bit wrong 🤔
If we went this way I think we should provide an interface with a method related to finding migrations, and another related to generating a filepath.

[stof]

AFAIK, the organize_migrations does not only impact the finder but also the path where new migrations are generated.

[mamazu]

Nope, the migration finder is dependent on the organization. That's what I was a little confused by.

migrations/src/DependencyFactory.php

Lines 283 to 289 in 28f6365

	public function getMigrationsFinder(): MigrationFinder
	{
	return $this->getDependency(MigrationFinder::class, function (): MigrationFinder {
	$configs = $this->getConfiguration();
	$needsRecursiveFinder = $configs->areMigrationsOrganizedByYear() || $configs->areMigrationsOrganizedByYearAndMonth();
	return $needsRecursiveFinder ? new RecursiveRegexFinder() : new GlobFinder();

But I agree that customizing the generation as well sounds like an overreaction. I don't know how much power we want to give the user and how much of the file name should be fixed. To be honest, I think the best approach is to use all files in the migration directory for both approaches.