bitexpert / disco Goto Github PK
View Code? Open in Web Editor NEWPSR-11 compatible Dependency Injection Container for PHP.
License: Apache License 2.0
PSR-11 compatible Dependency Injection Container for PHP.
License: Apache License 2.0
Port the bitExpert internal Phing infrastructure to disco to make sure all our packages follow the same conventions.
Pass GeneratorStrategy to proxyManagerConfiguration only when defined.
There is a problem with installing the latest (v0.1.2) version of bitexpert/disco via composer.
$> composer require bitexpert/disco
Using version ^0.1.2 for bitexpert/disco ./composer.json has been created
Loading composer repositories with package information
Updating dependencies (including require-dev)
Your requirements could not be resolved to an installable set of packages.
Problem 1
Potential causes:
Read https://getcomposer.org/doc/articles/troubleshooting.md for further common problems.
Installation failed, deleting ./composer.json.
Hey,
I'd like to suggest another way to provide application wide configuration and a second source for parameter injecting.
https://framework.zend.com/manual/2.4/en/modules/zend.config.introduction.html
Zend config encapsulates config into an object and provides getter, setter and methods to make config values readonly.
This could be pretty useful instead of using arrays. For example, if a config is not properly set an exception will be thrown. Error handling using a config structure like this could be improved.
No need to use zend/config, but for me it's pretty nice and something similar or even zend/config could be helpful.
It also can parse config files as yaml, xml, json and so on, which is nice.
For example using zend/config get you could pass a second parameter as default.
I'm not sure how Parameters and Parameter annotations work at the moment and I didn't dig in, but would it work in general to make use of it?
If so and the idea sounds not bad I could try to implement it.
Regards
Document the new "protected methods" behaviour, see #10.
Add BeanFactoryBeanPostProcessor which passes a beanFactory instance to classes implementing the BeanFactoryAware interface. Made sure the implementation is added to the list of the default post processors.
Add a few more checks for the @bean annotated methods, e.g. the methods need to be publicly accessible, otherwise the methods cannot be called from the AnnotationBeanFactory. The generator should throw some exceptions when an "error" in the configuration was found.
Move mikey179/vfsStream
to the require-dev section of composer.json.
Given you have bean method configuration that looks like this:
/** @Bean({"alias" = "\My\Namespace\Service\ProductService"}) */
public function productService() : \My\Namespace\Service\ProductService
{
return new \My\Namespace\Service\DefaultProductService();
}
The configured alias matches the return type definition - in this case either the name of an interface or a parent class. Since the alias is a hard-coded string it won't get updated during the usual refactorings (moving files to a different namespace and such) which is not what we want.
It would be nice to provide a sepcial keyword to let Disco automatically determine the return type declaration during the cached configuration generation process and automatically use the return type declaration as an alias, e.g.
/** @Bean({"alias" = "type"}) */
public function productService() : \My\Namespace\Service\ProductService
{
return new \My\Namespace\Service\DefaultProductService();
}
That way I could access the bean like this:
$beanFactory->get('\My\Namespace\Service\ProductService');
There are a bunch of @expectedException*
annotations left in the unit tests which should be replaced by the corresponding self::expectException*()
method calls due to being "a new best practice" as well as for consistency reasons.
Sitepoint once published a DI container performance comparison. Since I never benchmarked Disco it would be interesting to figure out what the performance really is about. The results could then be added to the README as well.
When a required parameter is not defined the exception thrown should show the parameter name in the exception message allow easier debugging.
As discussed with @Ocramius today it would make sense for Disco to be able to support autowiring for bean instances to reduce the amount of configuration code needed and to be able to get rid of traits for structuring the configuration code.
The following configuration:
/** @Bean */
protected function database() : Database
{
return new Database('mysql://user:secret@localhost/mydb');
}
/** @Bean */
public function productService() : ProductService
{
return new ProductService($this->database());
}
could then be simplified like this:
/** @Bean */
protected function database() : Database
{
return new Database('mysql://user:secret@localhost/mydb');
}
/** @Bean */
abstract public function productService() : ProductService;
At a later stage we could move the methods into separate interfaces - to be able to get rid of the abstract
keyword and ultimately get rid of the trait approach.
A few problems need to be solved:
Check if there`s a good way to implement the container-interop "delegate lookup feature" in Disco as described here.
Maybe by using the __call() method we could redirect calls to the delegate container when the dependency is not found locally.
Hey Stephan,
In my container benchmark, Disco seems to extremely underperform in tests where autoloading and startup times are also counted (https://rawgit.com/kocsismate/php-di-container-benchmarks/master/var/benchmark.html)
Could you please tell me why it happens? Once, I saw a compiled Disco container in my "/tmp" directory and I have a feeling that it is generated run-time. Can you confirm it? If this is the case, why it doesn't happen build-time (as a separate step)? I saw in the readme that Disco's startup time can be slow but if my benchmark is correct, unfortunately this is way much than slow.
Change visibility of wrapBeanAsLazy helper method from public to protected as the method is just used internally in the generated configuration class.
Given that PHP 7 is now a stable release we should remove it from the allow_failures configuration for the Travis build.
To avoid naming collisions make use of UniqueIdentifierGenerator::getIdentifier to generate unique method names for the following methods:
Since these helper methods are accessible from the outside or "magic" methods needed by PHP we cannot generate unique methods names:
PhpStorm supports a so-called Meta file helping with autocompletion for container calls. Add a (Phing) task to generate such a file for the given container configuration to enable autocomplete support for the return values of the BeanFactory::get() calls.
Disco needs a logo which can be used in the documentation as well as for marketing purposes.
Optimize the README.md file to make it easier for beginners to get started. In addition I would love to see the following changes:
Since a new version of proxy-manager was released it's time for an upgrade.
See the SensioLabs Insight report for Disco.
Exposing proxymanager configuration to be able to customize the inner workings of the proxymanager, e.g. where to store the generated files. Currently the system temp dir will be used which can turn into a problem when multiple projects have configuration classes with the same name.
To make sure that there are no security issues open for the dependencies used by Disco rely on the SensioLabs Security Advisories Checker. Since we use Phing for the build it makes the most sense to add the bitexpert/phing-securitychecker package and include it in the build.xml file.
Include Producer to improve the release process of Disco.
If strict_types=1 you can set types like string in function signature.
Seems like these types are removed while compilation resulting in following error:
PHP Warning: Declaration of ProxyManagerGeneratedProxy__PM__\src\company\application\Config\Generateda2fc343f86910958b2805ad36ffa91cc::application($sessionSecret = NULL) should be compatible with src\company\application\Config::application(string $sessionSecret = NULL) in /vagrant/api/cache/di/ProxyManagerGeneratedProxy__PM__srccompanyapplicationConfigGenerateda2fc343f86910958b2805ad36ffa91cc.php on line 314
Due to the changes #40 it is no longer possible to return primitive types. As pointed out by @settermjd it might make sense to return primitives sometimes.
This requires a few changes:
It make sense to wait for the ProxyManager 2 / PHP 7.x migration (see #46) before tackling this problem as with the built-in return typehint support it might be easier to handle the type detection.
We should make use of PHIVE to install dev toolings like phpstan, phpdocumentor, php-coveralls.
Currently the @Parameter "required" option
is casted to boolean via (bool)
internally, while the @Bean "lazy" option
is casted via a dedicated method called parseBooleanValue
.
This leads to the inconvenience that you only may use "required"="0" since 0 as string gets casted correctly to false while "false" will result in true.
Solution suggestion:
Use an AnnotationAttributeParser
which contains the method parseBooleanValue
as static method and use it in the Bean annotation and Parameter Annotation
Hint: For testing you will have to clear the cache files after each configuration change to get the correct results.
Currently, Disco has a code coverage of ~95%, but most of its coverage happened by hardcoded instance creations over bitExpert\Disco\AnnotationBeanFactory
and bitExpert\Disco\AnnotationBeanFactoryUnitTest
. In case of possible refactorings, does it make sense to add coverage by testing every class' methods separated in custom unit tests?
I started to take a look into the uncovered code and asked myself if there may be unreachable code by writing tests.
For example: https://github.com/bitExpert/disco/blob/v0.8.0/src/bitExpert/Disco/BeanFactoryConfiguration.php#L146
if (!spl_autoload_register($this->proxyAutoloader, false)) {
throw new RuntimeException(
sprintf('Cannot register autoloader "%s"', get_class($this->proxyAutoloader))
);
}
spl_autoload_register
can return false in cases of an error, like using a function name, that doesn't exist, in form of a string, but I didn't find a way to make this fail with a callable autoloader class using __invoke()
. I haven't try the same for spl_autoload_unregister
yet, but I expect the same like in spl_autoload_register
.
Update Composer dependencies.
Given we don't use complex logic in our build.xml, we could easily migrate to composer scripts and drop "just another" dev dependency from the project.
We'd simply need to manually install security-checker as the tool is not yet installable via PHIVE.
While experimenting with Disco and Expressive I realized that currently Disco is not able to work with Expressive for one reason: Expressive uses pseudo class names to query service instances from the service container. Since Disco tries to call a method with the same name on the config class this currently does not work because "" is not a valid character to be used for a method name.
To solve the issue we need a mapper in between to translate the invalid characters into valid ones.
AnnotationBeanFactory::has() returns true for "internal dependencies" (methods in config class marked as protected) since method_exists() simply checks if the method does exists not if the method can actually be called from the current context. This is a problem when AnnotationBeanFactory::get() is called since call_user_func() is not able to call the method as intended.
To fix the problem substitute the method_exists() call with is_callable(). This seems to be a side-effect of #10.
For consistency reasons I would be nice to have the declare(strict_types=1);
setting in the generated configuration file.
Currently the ConfigurationGenerator is working with a fixed set of annotations which is ok for now but in the future I`d like to see a way to add custom annotation handlers which should be able to generate methods (and maybe more?) in the generated proxy class so that frameworks could add own annotations (e.g. an annotation to configure the routing or security settings).
In the process of upgrading to ProxyManager 2 do:
Add an PSR-3 logger instance via bitexpert/slf4psrlog to trace what`s going on during the cached configuration building phase. This should help debugging when things "go wrong".
With PHP 8 and having attributes support, configuring beans can be even more simpler. I don't expect to have a complete picture of what's needed, since I have only experience with discolight where I did this proposal (sort of). Anyway here's my take:
There are some options to walk this path:
I can't think of a good reason to go for option 2. Adding it to 0.x allows for a gradual upgrade path allowing to deprecate features which can eventually be removed in 1.x.
Adding this feature can be done by:
While new Attribute classes allows you to start with a clean slate, current Annotation classes can quite easily be adapted in order to be used as Attribute classes as well.
As option 1 is in my opinion the most valuable approach I'll go a bit further on that:
Example of how that can be achieved in case of the Alias class:
/**
* @Annotation
* @Target({"ANNOTATION"})
* @Attributes({
* @Attribute("name", type = "string"),
* @Attribute("type", type = "bool"),
* })
*/
#[\Attribute(\Attribute::TARGET_METHOD | \Attribute::IS_REPEATABLE)]
final class Alias
{
/**
* @var string
*/
private $name;
/**
* @var bool
*/
private $type;
/**
* Creates a new {@link \bitExpert\Disco\Annotations\Bean\Alias}.
*
* @param array $attributes
* @throws AnnotationException
*/
public function __construct(array $attributes = [], ?string $name = null, bool $type = false)
{
// When configured by annotations the $attributes['value'] is not empty, so
// in that case override the $name and $type variables from the $attributes['value'] array.
if (!empty($attributes['value'])) {
$attributes = $attributes['value'];
// Assign local variables the value from $attributes meanwhile making sure the keys exist
// with at least the default value as defined in the constructor.
['name' => $name, 'type' => $type] = $attributes + ['name' => $name, 'type' => $type];
}
if (!is_bool($type)) {
$type = AnnotationAttributeParser::parseBooleanValue($type);
}
if ($type && $name) {
throw new AnnotationException('Type alias should not have a name!');
}
if (!$type && !$name) {
throw new AnnotationException('Alias should either be a named alias or a type alias!');
}
$this->type = $type;
$this->name = $name;
}
public function getName(): ?string
{
return $this->name;
}
public function isTypeAlias(): bool
{
return $this->type;
}
}
class Config
{
/**
* Using named arguments
*/
#[Bean]
#[Alias(type: true)]
#[Alias(name: 'my.service.id')]
public function myService(): Service
{}
/**
* Using ordered arguments
*/
#[Bean]
#[Alias([], null, true)]
#[Alias([], 'my.service.id')]
public function myOtherService(): OtherService
{}
}
As you can see named arguments allow for a clean usage of existing Annotation classes.
What about nested attributes?
In PHP's current implementation of attributes nesting is not supported, so explicitly configuring needs to be done on the same level, or instances of nested attributes/annotations should be able to be configured with scalars and instantiated in the "parent" attribute instance.
class Config {
#[Bean(aliases: [
['type' => true],
['name' => 'my.service.id'],
])]
public function myService(): Service
{}
}
Alias
Allow configuration of
Bean
Allow configuration of
BeanPostProcessor
Allow configuration of
Configuration
Just an identifier attribute
Parameter
The ordering of parameters is important since the method is called with the parameters in the order they are configured. Nesting the parameters inside the Bean annotation provides a natural way of respecting the ordering of the arguments in the methods signature.
But requiring attributes to be listed in a specific order feels a bit like a smell to me.
class Config {
#[Bean(parameters: [
['name' => 'config.key1', 'default' => 'default value'],
['name' => 'config.key2', 'default' => 'other default value'],
])]
public function myService($key1, $key2): Service
{}
// Correct ordering of the parameter attributes required, NOT IDEAL
#[Bean]
#[Parameter(name: 'config.key1', default => 'default value')]
#[Parameter(name: 'config.key2', default => 'other default value')]
public function myOtherService($key1, $key2): OtherService
{}
}
Possible solution: When using the Parameter attribute, require a config key with the name of the argument so it can be used for calling the method with named arguments. The nicest would be something like this:
$parameters = [
'config' => [
'key1' => 'value of key1',
'key2' => 'value of key2',
],
];
class Config {
#[Bean]
#[Parameter(name: 'arg2', key: 'config.key2', default => 'other default value')]
#[Parameter(name: 'arg1', key: 'config.key1', default => 'default value')]
public function myService($arg1, $arg2): Service
{
// $arg1 = 'value of key1;
// $arg2 = 'value of key2;
}
}
Unfortunately name is already in use as the name of the parameter key, but i.e. argname can perhaps be a good alternative.
The doctrine AnnotationReader can be swapped out for a FallbackAttributeReader. The FallbackAttributeReader would prioritize attributes over Annotations.
I would not recommend mixing annotations and attributes on the same level.
Possible guidelines:
As disco is already hosted on github, wouldn't it be a great idea to serve the disco-documentation (that is currently only available via a local server) via github pages?
AFAIK that would require enabling the github-pages for the repo and setting the source to the docs
-folder. Everything else should then work out from scratch. Probably linking the different pages needs to be taken care of.
Alternative would be to use the existing bookdown-setup to create the docs and deploy them to github pages…
Make sure that the current version of Disco can be installed on PHP 7.2. Also check for side-effects with the latest version of ProxyManager which is focused PHP 7.2 only.
Extend ConfigurationGenerator to also deal with protected methods. Protected methods could be used as "internal" dependencies which cannot retrieved externally by calling the get() method on the BeanFactory but can be used as dependencies for other instances. For example I do not want to expose my database connection object to the public but use it as an internal dependency.
Currently is this already possible but all annotations get ignored as no wrapper methods get generated in the proxy class.
Add a configuration setting to enable the production mode for ProxyManager to gain a massive performance improvement.
Since the Container-Interop ContainerInterface does not use type hinting but the get() and has() method have declared $id as string param type, AnnotationBeanFactory should check the given id for being a non-empty string and throw an exception else. Otherwise at least the AliasContainer will throw a confusing exception concerning the AliasContainer because hasAlias has defined a string type hint in its signature.
BeanPostProcessor cannot depend on method with parameter since the bean post processors gets instantiated before the parameters are set. To fix it set the parameters first before instantiating the bean post processors.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.