-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Add various plugin manager #4319
Add various plugin manager #4319
Conversation
👍 |
* @var array | ||
*/ | ||
protected $invokableClasses = array( | ||
'arrayserializable' => 'Zend\Stdlib\Hydrator\ArraySerializable', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bakura10 maybe use aliases instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ocramius Why? Other plugin managers use invokables as shortname/classname pairs...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Furthremore all plugin managers are configured to autoadd any invokables classes.
@bakura10 It looks good. A few comments:
|
I'm thinking about that but do we really need those listeners and co ? Can't the factories simply read the config ? Like we do in modules. (I'm going to make the changes) |
* @link http://github.com/zendframework/zf2 for the canonical source repository | ||
* @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com) | ||
* @license http://framework.zend.com/license/new-bsd New BSD License | ||
* @package Zend_Form |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove @package
Why |
$factory = $element->getFactory(); | ||
$factory->getDefaultFilterChain()->setPluginManager($this->serviceLocator->get('FilterManager')); | ||
$factory->getDefaultValidatorChain()->setPluginManager($this->serviceLocator->get('ValidatorManager')); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should probably check that $this->serviceLocator
is indeed an instance before calling get()
on it. You could do it in the initial conditional:
if ($element instanceof InputFilter && $this->serviceLocator instanceof ServiceLocatorInterface) {
/* ... */
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it always the case ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's always a remote possibility that somebody will instantiate it themselves. :)
Done. |
What about the |
@danizord See https://github.com/zendframework/zf2/pull/4319/files#L2R38 -- this is already covered, as the default InputFilter class already composes a factory; those lines there will inject the validator and filter chains on creation. :) |
* Plugin manager implementation for input filters. | ||
*/ | ||
class InputFilterPluginManager extends AbstractPluginManager | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just realized this one should be marked sharedByDefault == false as well (as input filters are rarely if ever shared). I'll do that on merge.
Add various plugin manager
- Intention changed mid-PR, but tests were not updated; demonstrate that Factory composes a default filter chain and validator chain by default.
…rator-plugin-manager Add various plugin manager
…rator-plugin-manager Add various plugin manager
…rator-plugin-manager Add various plugin manager
- Intention changed mid-PR, but tests were not updated; demonstrate that Factory composes a default filter chain and validator chain by default.
Hi,
This PR adds two plugin managers: one for hydrators and one for input filters. It also fixes a bug for the Serializer adapter (I introduced another way than adding all the listener in the ModuleManagerFactory but in fact my approach did not work at all, so until we find a cleanest way, I've just added another new listenenr in the ModuleManagerFactory).
Those plugin managers would be really useful for one module I'm working on, and this continue to make the framework more uniform with same logic.
I'd like this to be merged for 2.2 if that's possible =).
(PS: for ZF 3, we'd like to find a better solution, I think that adding so much listeners will lower performance, at some point...)