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.

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

Entries are not fetched in array parameters of the constructor's factory #488

Closed
jjanvier opened this Issue May 21, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@jjanvier

jjanvier commented May 21, 2017

Hello there :) And thanks for this lib :)

I think I have found a bug, or at least a weirdness. Imagine I'd like to configure a KLogger service.

<?php

use Katzgrau\KLogger\Logger;
use Psr\Log\LoggerInterface;
use Psr\Log\LogLevel;

return [
    'logger.dir' => sys_get_temp_dir(),
    'logger.level' => LogLevel::DEBUG,
    'logger.prefix' => 'a_prefix_'],
    LoggerInterface::class => DI\object(Logger::class)->constructor(
        DI\get('logger.dir'),
        DI\get('logger.level'),
        ['prefix' => DI\get('logger.prefix')]
    ),
];

This configuration does not. The logger prefix is not correctly translated. I get the following error:

[Sun May 21 13:00:07 2017] PHP Catchable fatal error:  Object of class DI\Definition\EntryReference could not be converted to string in /home/jjanvier/psr15_middleware/vendor/katzgrau/klogger/src/Logger.php on line 157
[Sun May 21 13:00:07 2017] PHP Stack trace:
[Sun May 21 13:00:07 2017] PHP   1. {main}() /home/jjanvier/psr15_middleware/examples/src/app.php:0
[Sun May 21 13:00:07 2017] PHP   2. DI\Container->get() /home/jjanvier/psr15_middleware/examples/src/app.php:20
[Sun May 21 13:00:07 2017] PHP   3. DI\Container->resolveDefinition() /home/jjanvier/psr15_middleware/vendor/php-di/php-di/src/DI/Container.php:124
[Sun May 21 13:00:07 2017] PHP   4. DI\Definition\Resolver\ResolverDispatcher->resolve() /home/jjanvier/psr15_middleware/vendor/php-di/php-di/src/DI/Container.php:287
[Sun May 21 13:00:07 2017] PHP   5. DI\Definition\Resolver\ObjectCreator->resolve() /home/jjanvier/psr15_middleware/vendor/php-di/php-di/src/DI/Definition/Resolver/ResolverDispatcher.php:58
[Sun May 21 13:00:07 2017] PHP   6. DI\Definition\Resolver\ObjectCreator->createInstance() /home/jjanvier/psr15_middleware/vendor/php-di/php-di/src/DI/Definition/Resolver/ObjectCreator.php:70
[Sun May 21 13:00:07 2017] PHP   7. ReflectionClass->newInstanceArgs() /home/jjanvier/psr15_middleware/vendor/php-di/php-di/src/DI/Definition/Resolver/ObjectCreator.php:138
[Sun May 21 13:00:07 2017] PHP   8. Katzgrau\KLogger\Logger->__construct() /home/jjanvier/psr15_middleware/vendor/php-di/php-di/src/DI/Definition/Resolver/ObjectCreator.php:138
[Sun May 21 13:00:07 2017] PHP   9. Katzgrau\KLogger\Logger->setLogFilePath() /home/jjanvier/psr15_middleware/vendor/katzgrau/klogger/src/Logger.php:126

A workaround is to configure the service as the following:

<?php

use Katzgrau\KLogger\Logger;
use Psr\Log\LoggerInterface;
use Psr\Log\LogLevel;

return [
    'logger.dir' => sys_get_temp_dir(),
    'logger.level' => LogLevel::DEBUG,
    'logger.options' => ['prefix' => 'yamo_'],
    LoggerInterface::class => DI\object(Logger::class)->constructor(
        DI\get('logger.dir'),
        DI\get('logger.level'),
        DI\get('logger.options')
    ),
];
@jjanvier

This comment has been minimized.

jjanvier commented May 21, 2017

If it's a bug and if you give me some guidelines, I'll try to submit a patch :)

@mnapoli

This comment has been minimized.

Member

mnapoli commented May 25, 2017

Hi, it is indeed not supposed to work. I'll try and look into that for v6 (it won't be an easy patch I think).

@mnapoli mnapoli added this to the 6.0 milestone May 25, 2017

@jjanvier

This comment has been minimized.

jjanvier commented May 25, 2017

ok, thanks for the feedback
good luck so :) 👍

mnapoli added a commit that referenced this issue Jun 4, 2017

Fix #498 and #488
In PHP-DI 5, definitions could be nested in some places (e.g. use a get() in an object definition, etc.). However it did not behave everywhere the same, for example it didn't work for sub-definitions in arrays.

Now in PHP-DI 6 all nested definitions will all be recognized and resolved correctly everywhere. Since #494 (compiled container) performance will not be affected so we can implement a more robust behavior.

mnapoli added a commit that referenced this issue Jun 4, 2017

Fix #498 and #488 Standardize resolution of nested definitions everyw…
…here

In PHP-DI 5, definitions could be nested in some places (e.g. use a get() in an object definition, etc.). However it did not behave everywhere the same, for example it didn't work for sub-definitions in arrays.

Now in PHP-DI 6 all nested definitions will all be recognized and resolved correctly everywhere. Since #494 (compiled container) performance will not be affected so we can implement a more robust behavior.
@mnapoli

This comment has been minimized.

Member

mnapoli commented Jun 4, 2017

This issue was a sub-issue of #498, it has been covered in #499

Thanks!

@mnapoli mnapoli closed this Jun 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment