Skip to content

[12.x] Allow Container to build Migrator from class name #55501

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Apr 22, 2025
Merged

[12.x] Allow Container to build Migrator from class name #55501

merged 5 commits into from
Apr 22, 2025

Conversation

cosmastech
Copy link
Contributor

Writing integration tests that rely on the migrator (such as testing a single migration) requires me to write:

$this->app->make('migrator')

Because otherwise, there's a binding resolution exception:

Illuminate\Contracts\Container\BindingResolutionException: Target [Illuminate\Database\Migrations\MigrationRepositoryInterface] is not instantiable while building [Illuminate\Database\Migrations\Migrator].

With this change, I will now be able to write tests like this:

$this->app->make(Migrator::class)->run(database_path(''));

@cosmastech cosmastech marked this pull request as draft April 21, 2025 19:04
@cosmastech cosmastech force-pushed the build-migrator-from-container branch from 5c98758 to 36f55c1 Compare April 21, 2025 20:48
@cosmastech cosmastech marked this pull request as ready for review April 21, 2025 21:03
rodrigopedra
rodrigopedra reviewed Apr 22, 2025
View reviewed changes
src/Illuminate/Database/MigrationServiceProvider.php
@@ -82,6 +82,7 @@ protected function registerMigrator()

return new Migrator($repository, $app['db'], $app['files'], $app['events']);
});
$this->app->bind(Migrator::class, fn ($app) => $app['migrator']);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use Container@alias instead of bind.

Besides saving a closure call, as Migrator::class would be an alias to migrator, and migrator is a singleton, it would be automatically considered the same singleton.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe I tried it with a container alias, but because it's a singleton, $app->forget('migrator') still keeps the aliases version in memory.

15d884a

https://github.com/laravel/framework/actions/runs/14579501724/job/40892877095

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is weird, maybe we need to call $this->getAlias($abstract) when forgetting instances, much like is done when forgetting extenders:

framework/src/Illuminate/Container/Container.php

Lines 1555 to 1564 in b6a2af2

/**
* Remove a resolved instance from the instance cache.
*
* @param string $abstract
* @return void
*/
public function forgetInstance($abstract)
{
unset($this->instances[$abstract]);
}

framework/src/Illuminate/Container/Container.php

Lines 1533 to 1542 in b6a2af2

/**
* Remove all of the extender callbacks for a given type.
*
* @param string $abstract
* @return void
*/
public function forgetExtenders($abstract)
{
unset($this->extenders[$this->getAlias($abstract)]);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that makes sense, however, that definitely belongs in a separate PR. Also potentially a breaking-change?

@rodrigopedra
Copy link
Contributor

Most of the aliases to the string bindings are registered on the Illuminate\Foundation\Application@registerCoreContainerAliases method:

framework/src/Illuminate/Foundation/Application.php

Lines 1614 to 1666 in b6a2af2

/**
* Register the core class aliases in the container.
*
* @return void
*/
public function registerCoreContainerAliases()
{
foreach ([
'app' => [self::class, \Illuminate\Contracts\Container\Container::class, \Illuminate\Contracts\Foundation\Application::class, \Psr\Container\ContainerInterface::class],
'auth' => [\Illuminate\Auth\AuthManager::class, \Illuminate\Contracts\Auth\Factory::class],
'auth.driver' => [\Illuminate\Contracts\Auth\Guard::class],
'blade.compiler' => [\Illuminate\View\Compilers\BladeCompiler::class],
'cache' => [\Illuminate\Cache\CacheManager::class, \Illuminate\Contracts\Cache\Factory::class],
'cache.store' => [\Illuminate\Cache\Repository::class, \Illuminate\Contracts\Cache\Repository::class, \Psr\SimpleCache\CacheInterface::class],
'cache.psr6' => [\Symfony\Component\Cache\Adapter\Psr16Adapter::class, \Symfony\Component\Cache\Adapter\AdapterInterface::class, \Psr\Cache\CacheItemPoolInterface::class],
'config' => [\Illuminate\Config\Repository::class, \Illuminate\Contracts\Config\Repository::class],
'cookie' => [\Illuminate\Cookie\CookieJar::class, \Illuminate\Contracts\Cookie\Factory::class, \Illuminate\Contracts\Cookie\QueueingFactory::class],
'db' => [\Illuminate\Database\DatabaseManager::class, \Illuminate\Database\ConnectionResolverInterface::class],
'db.connection' => [\Illuminate\Database\Connection::class, \Illuminate\Database\ConnectionInterface::class],
'db.schema' => [\Illuminate\Database\Schema\Builder::class],
'encrypter' => [\Illuminate\Encryption\Encrypter::class, \Illuminate\Contracts\Encryption\Encrypter::class, \Illuminate\Contracts\Encryption\StringEncrypter::class],
'events' => [\Illuminate\Events\Dispatcher::class, \Illuminate\Contracts\Events\Dispatcher::class],
'files' => [\Illuminate\Filesystem\Filesystem::class],
'filesystem' => [\Illuminate\Filesystem\FilesystemManager::class, \Illuminate\Contracts\Filesystem\Factory::class],
'filesystem.disk' => [\Illuminate\Contracts\Filesystem\Filesystem::class],
'filesystem.cloud' => [\Illuminate\Contracts\Filesystem\Cloud::class],
'hash' => [\Illuminate\Hashing\HashManager::class],
'hash.driver' => [\Illuminate\Contracts\Hashing\Hasher::class],
'translator' => [\Illuminate\Translation\Translator::class, \Illuminate\Contracts\Translation\Translator::class],
'log' => [\Illuminate\Log\LogManager::class, \Psr\Log\LoggerInterface::class],
'mail.manager' => [\Illuminate\Mail\MailManager::class, \Illuminate\Contracts\Mail\Factory::class],
'mailer' => [\Illuminate\Mail\Mailer::class, \Illuminate\Contracts\Mail\Mailer::class, \Illuminate\Contracts\Mail\MailQueue::class],
'auth.password' => [\Illuminate\Auth\Passwords\PasswordBrokerManager::class, \Illuminate\Contracts\Auth\PasswordBrokerFactory::class],
'auth.password.broker' => [\Illuminate\Auth\Passwords\PasswordBroker::class, \Illuminate\Contracts\Auth\PasswordBroker::class],
'queue' => [\Illuminate\Queue\QueueManager::class, \Illuminate\Contracts\Queue\Factory::class, \Illuminate\Contracts\Queue\Monitor::class],
'queue.connection' => [\Illuminate\Contracts\Queue\Queue::class],
'queue.failer' => [\Illuminate\Queue\Failed\FailedJobProviderInterface::class],
'redirect' => [\Illuminate\Routing\Redirector::class],
'redis' => [\Illuminate\Redis\RedisManager::class, \Illuminate\Contracts\Redis\Factory::class],
'redis.connection' => [\Illuminate\Redis\Connections\Connection::class, \Illuminate\Contracts\Redis\Connection::class],
'request' => [\Illuminate\Http\Request::class, \Symfony\Component\HttpFoundation\Request::class],
'router' => [\Illuminate\Routing\Router::class, \Illuminate\Contracts\Routing\Registrar::class, \Illuminate\Contracts\Routing\BindingRegistrar::class],
'session' => [\Illuminate\Session\SessionManager::class],
'session.store' => [\Illuminate\Session\Store::class, \Illuminate\Contracts\Session\Session::class],
'url' => [\Illuminate\Routing\UrlGenerator::class, \Illuminate\Contracts\Routing\UrlGenerator::class],
'validator' => [\Illuminate\Validation\Factory::class, \Illuminate\Contracts\Validation\Factory::class],
'view' => [\Illuminate\View\Factory::class, \Illuminate\Contracts\View\Factory::class],
] as $key => $aliases) {
foreach ($aliases as $alias) {
$this->alias($key, $alias);
}
}
}

Shouldn't this alias also be registered there?

@taylorotwell taylorotwell merged commit 9acd22e into laravel:12.x Apr 22, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rodrigopedra rodrigopedra rodrigopedra left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cosmastech @rodrigopedra @taylorotwell