wintercms / winter

Free, open-source, self-hosted CMS platform based on the Laravel PHP Framework.
https://wintercms.com
MIT License
1.38k stars 195 forks source link

App::make($key, $parameters) cannot resolve driver binding correctly #249

Closed ideepblue closed 3 months ago

ideepblue commented 3 years ago

Description:

I installed a package elfsundae/laravel-hashid via composer require elfsundae/laravel-hashid. And publish the config file via php artisan vendor:publish --tag=hashid. I still use the default config file.

However, when I use tinker to test this hashid package, I got an error:

root@4cc19e2ba78d:/var/www# php artisan tinker
Psy Shell v0.10.8 (PHP 7.3.21 — cli) by Justin Hileman
>>> hashid()
ReflectionException with message 'Class hashid.driver.hashids_integer does not exist'

I have checked the source code of elfsundae/laravel-hashid. In its service provider, it binds the driver into the \Winter\Storm\Foundation\Application::$bindings. The binding key is hashid.driver.hashids_integer (See the screenshot below).

image

Driver binding in \ElfSundae\Laravel\Hashid\HashidServiceProvider::registerServices

    protected function registerServices()
    {
        $this->app->singleton('hashid', function ($app) {
            return new HashidManager($app);
        });
        $this->app->alias('hashid', HashidManager::class);

        foreach ($this->getSingletonDrivers() as $class) {
            $this->app->singleton(
                $key = $this->getBindingKeyForDriver($class),
                function () use ($class) {
                    return new $class;
                }
            );
            $this->app->alias($key, $class);
        }

        foreach ($this->getNonSingletonDrivers() as $class) {
            $this->app->bind($this->getBindingKeyForDriver($class), $class);  // <- driver binding here
        }
    }

When hashid() invoked, the package will resolve driver binding via \ElfSundae\Laravel\Hashid\HashidManager::resolveBinding:

    protected function resolveBinding($key, array $parameters = [])
    {
        if ($this->app->isShared($key)) {
            return $this->app->make($key);
        }

        $makeWith = method_exists($this->app, 'makeWith') ? 'makeWith' : 'make';

        return $this->app->$makeWith($key, $parameters);  // <- call $this->app->makeWith() to resolve
    }

It finally reaches \Winter\Storm\Foundation\Application::make:

    public function make($abstract, array $parameters = [])
    {
        $abstract = $this->getAlias($abstract);

        if (isset($this->deferredServices[$abstract])) {
            $this->loadDeferredProvider($abstract);
        }

        if ($parameters) {
            return $this->make(Maker::class)->make($abstract, $parameters); // <- resolve via \Winter\Storm\Foundation\Maker::make()
        }

        return parent::make($abstract, $parameters);
    }

Because the driver is binding into the Winter\Storm\Foundation\Application, not Winter\Storm\Foundation\Maker.

    public function make($abstract, $parameters = [])
    {
        return $this->build(
            $this->getBinding($abstract),  // <- first parameter is "hashid.driver.hashids_integer", not Driver class closure.
            $parameters
        );

Then, the actual \Winter\Storm\Foundation\Maker::build function reaches $reflector = new ReflectionClass('');, exception throwed.

    protected function build($concrete, $parameters)
    {
        if ($concrete instanceof Closure) {
            return $concrete($this->container, $parameters);
        }

        $reflector = new ReflectionClass($concrete); // $concrete = 'hashid.driver.hashids_integer', exception throwed.

        if (!$reflector->isInstantiable()) {
            throw new BindingResolutionException("Target [$concrete] is not instantiable.");
        }

        $constructor = $reflector->getConstructor();

        if (is_null($constructor)) {
            return new $concrete;
        }
        ...

Steps To Reproduce:

  1. composer require elfsundae/laravel-hashid
  2. php artisan vendor:publish --tag=hashid
  3. php artisan tinker then type hashid()
bennothommo commented 3 years ago

@ideepblue by default, Winter CMS does not auto-discover Laravel packages, to allow plugins to control whether Laravel packages are loaded or not depending on whether the plugin is installed or not.

You can enable package autodiscovery by changing the loadDiscoveredPackages value in config/app.php to true. Alternatively, if you know that you want this package to be always enabled for your project, you can add its ServiceProvider class into the providers array in the same file.

ideepblue commented 3 years ago

@bennothommo I have already enabled the loadDiscoveredPackages to true. I can confirm this package's ServiceProvider has been called when booting up the Winter CMS. That's why I have attached a screenshot showing the driver has been bind into the container protected variable $bindings.

Please have a look at the screenshot below.

image

The error only happens when the Laravel package tries to resolve its driver via $this->app->makeWith('<driver-key-name>', $parameters).

bennothommo commented 3 years ago

@ideepblue My apologies - you are right. We will look into this a bit further.

ideepblue commented 3 years ago

Thanks @bennothommo , let me know if you need more information from me.

github-actions[bot] commented 3 years ago

This issue will be closed and archived in 3 days, as there has been no activity in the last 60 days. If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue. If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

ideepblue commented 3 years ago

My temporary solution, binding the driver to the Winter\Storm\Foundation\Maker:

use ElfSundae\Laravel\Hashid\Base62Driver;
use ElfSundae\Laravel\Hashid\Base62IntegerDriver;
use ElfSundae\Laravel\Hashid\HashidsDriver;
use ElfSundae\Laravel\Hashid\HashidsIntegerDriver;
use ElfSundae\Laravel\Hashid\HashidsHexDriver;
use ElfSundae\Laravel\Hashid\HashidsStringDriver;
use ElfSundae\Laravel\Hashid\OptimusDriver;
use Winter\Storm\Foundation\Maker;

class Plugin extends PluginBase
{
    public function boot()
    {
        // hashid
        $this->solveHashidDriverIssue();
    }

    private function solveHashidDriverIssue()
    {
        /** @var Maker $maker */
        $drivers = [
            Base62Driver::class,
            Base62IntegerDriver::class,
            HashidsDriver::class,
            HashidsHexDriver::class,
            HashidsIntegerDriver::class,
            HashidsStringDriver::class,
            OptimusDriver::class,
        ];

        // Ref: https://github.com/wintercms/winter/issues/249
        $maker = $this->app->make(Maker::class);
        foreach ($drivers as $concrete) {
            $abstract = 'hashid.driver.'
                . Str::snake(preg_replace('#Driver$#', '', class_basename($concrete)));
            $closure = function ($container, $parameters = []) use ($abstract, $concrete) {
                return new $concrete(array_get($parameters, 'config', []));
            };
            $maker->bind($abstract, $closure);
        }
    }
}
github-actions[bot] commented 3 years ago

This issue will be closed and archived in 3 days, as there has been no activity in the last 60 days. If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue. If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

github-actions[bot] commented 2 years ago

This issue will be closed and archived in 3 days, as there has been no activity in the last 60 days. If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue. If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

github-actions[bot] commented 2 years ago

This issue will be closed and archived in 3 days, as there has been no activity in the last 60 days. If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue. If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

github-actions[bot] commented 2 years ago

This issue will be closed and archived in 3 days, as there has been no activity in the last 60 days. If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue. If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

github-actions[bot] commented 2 years ago

This issue will be closed and archived in 3 days, as there has been no activity in this issue for the last 6 months. If this issue is still relevant or you would like to see it actioned, please respond within 3 days. If this issue is critical for your business, please reach out to us at wintercms@luketowers.ca.

github-actions[bot] commented 1 year ago

This issue will be closed and archived in 3 days, as there has been no activity in this issue for the last 6 months. If this issue is still relevant or you would like to see it actioned, please respond within 3 days. If this issue is critical for your business, please reach out to us at wintercms@luketowers.ca.

github-actions[bot] commented 1 year ago

This issue will be closed and archived in 3 days, as there has been no activity in this issue for the last 6 months. If this issue is still relevant or you would like to see it actioned, please respond within 3 days. If this issue is critical for your business, please reach out to us at wintercms@luketowers.ca.

github-actions[bot] commented 3 months ago

This issue will be closed and archived in 3 days, as there has been no activity in this issue for the last 6 months. If this issue is still relevant or you would like to see it actioned, please respond within 3 days. If this issue is critical for your business, please reach out to us at wintercms@luketowers.ca.