Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Inject logger instance via constructor #34

Merged
merged 12 commits into from
Jan 26, 2018
10 changes: 8 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ All notable changes to this project will be documented in this file, in reverse

### Added

- [#33](https://github.com/zendframework/zend-di/pull/32) add ability to pass
`Psr\Log\LoggerInterface` to `Zend\Di\CodeGenerator\InjectorGenerator`'s constructor


Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted: Double space willl be removed -> single space.

- [#32](https://github.com/zendframework/zend-di/pull/32) adds the implementation of
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Ocramius I'm a bit unsure about leaving this in the changelog. Should it be removed as for readability (Same goes for the Removed and Fixed section). Since the changes from PR #32 were never released.

Copy link
Member

Choose a reason for hiding this comment

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

Note is fine as long as you aggregate it with the changes in this patch. What matters is that the reader of the release notes understands what happened.

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 guess it would be more confusing then, since it may lead people to the assumption that 3.1 introduces the Aware implementation which is actually not the case. I moved the reference to PR #32 to the Removed and Fixed entries to state clearly that this is not the case.

`Psr\Log\LoggerAwareInterface` to `Zend\Di\CodeGenerator\InjectorGenerator`

Expand All @@ -27,11 +31,13 @@ All notable changes to this project will be documented in this file, in reverse

### Removed

- Nothing.
- [#33](https://github.com/zendframework/zend-di/pull/32) removes the implementation of
`Psr\Log\LoggerAwareInterface` from `Zend\Di\CodeGenerator\InjectorGenerator`

### Fixed

- Nothing.
- [#33](https://github.com/zendframework/zend-di/pull/32) fixes discouraged use of
`Psr\Log\LoggerAware*`

## 3.0.1 - TBD

Expand Down
3 changes: 0 additions & 3 deletions src/CodeGenerator/GeneratorTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

namespace Zend\Di\CodeGenerator;

use Psr\Log\LoggerAwareTrait;
use Zend\Di\Exception\GenerateCodeException;
use Zend\Di\Exception\LogicException;

Expand All @@ -16,8 +15,6 @@
*/
trait GeneratorTrait
{
use LoggerAwareTrait;

/**
* @var int
*/
Expand Down
15 changes: 12 additions & 3 deletions src/CodeGenerator/InjectorGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
namespace Zend\Di\CodeGenerator;

use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerInterface;
use Psr\Log\NullLogger;
use Throwable;
use Zend\Code\Generator\ClassGenerator;
Expand All @@ -25,7 +26,7 @@
* type, if available. This factory will contained pre-resolved dependencies
* from the provided configuration, definition and resolver instances.
*/
class InjectorGenerator implements LoggerAwareInterface
class InjectorGenerator
{
use GeneratorTrait;

Expand Down Expand Up @@ -60,25 +61,33 @@ class InjectorGenerator implements LoggerAwareInterface
*/
private $autoloadGenerator;

/**
* @var LoggerInterface
*/
private $logger;

/**
* Constructs the compiler instance
*
* @param ConfigInterface $config The configuration to compile from
* @param DependencyResolverInterface $resolver The resolver to utilize
* @param string $namespace Namespace to use for generated class; defaults
* to Zend\Di\Generated.
* @param LoggerInterface $logger An optional logger instance to log failures
Copy link
Member

Choose a reason for hiding this comment

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

Missing null type in php doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, on $namespace as well ...

* and processed classes.
*/
public function __construct(
ConfigInterface $config,
DependencyResolverInterface $resolver,
?string $namespace = null
?string $namespace = null,
Copy link
Member

Choose a reason for hiding this comment

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

Just an FYI: you don't need to use a nullable type here when providing a default null value. (Something @webimpress demonstrated to me recently.)

LoggerInterface $logger = null
) {
$this->config = $config;
$this->resolver = $resolver;
$this->namespace = $namespace ? : 'Zend\Di\Generated';
$this->factoryGenerator = new FactoryGenerator($config, $resolver, $this->namespace . '\Factory');
$this->autoloadGenerator = new AutoloadGenerator($this->namespace);
$this->logger = new NullLogger();
$this->logger = $logger ?? new NullLogger();
}

/**
Expand Down
6 changes: 2 additions & 4 deletions test/CodeGenerator/InjectorGeneratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,10 @@ public function testGeneratorLogsDebugForEachClass()
{
$config = new Config();
$resolver = new DependencyResolver(new RuntimeDefinition(), $config);
$generator = new InjectorGenerator($config, $resolver);
$logger = $this->prophesize(LoggerInterface::class);

$generator = new InjectorGenerator($config, $resolver, null, $logger->reveal());
Copy link
Member

Choose a reason for hiding this comment

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

Did you also change the constructor, or was it already accepting an optional logger?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the test, only. Just commited the code changes ;-)

$generator->setOutputDirectory($this->dir);
$generator->setLogger($logger->reveal());
$generator->generate([
TestAsset\B::class
]);
Expand All @@ -90,11 +89,10 @@ public function testGeneratorLogsErrorWhenFactoryGenerationFailed()
{
$config = new Config();
$resolver = new DependencyResolver(new RuntimeDefinition(), $config);
$generator = new InjectorGenerator($config, $resolver);
$logger = $this->prophesize(LoggerInterface::class);
$generator = new InjectorGenerator($config, $resolver, null, $logger->reveal());

$generator->setOutputDirectory($this->dir);
$generator->setLogger($logger->reveal());
$generator->generate([
'Bad.And.Undefined.ClassName'
]);
Expand Down