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

Provide a standalone plugin manager implementation #87

Merged
merged 8 commits into from Dec 8, 2018

Conversation

Projects
None yet
3 participants
@weierophinney
Member

weierophinney commented Dec 4, 2018

This patch allows zend-hydrator to be used without zend-servicemanager, making that package a truly optional dependency.

First, it adds Zend\Hydrator\HydratorPluginManagerInterface, which extends the PSR-11 ContainerInterface. This is done to ensure that any plugin manager implementation complies with PSR-11 (currently, AbstractPluginManager implementations from zend-servicemanager do not), as well as to allow type-hinting on that interface when a plugin manager is needed. The DelegatingHydratorFactory was updated to test against this interface instead of the HydratorPluginManager.

Next, the patch adds Zend\Hydrator\StandaloneHydratorPluginManager, which provides a minimal plugin manager that supports loading only the shipped hydrator implementations. A factory for creating the plugin manager is also provided via Zend\Hydrator\StandaloneHydratorPluginManagerFactory.

Zend\Hydrator\ConfigProvider was updated to add the StandaloneHydratorPluginManager factory mapping, as well as to change the target of the HydratorManager alias based on the presence of the zend-servicemanager package.

A page on plugin managers was added, and includes an example of creating a custom plugin manager.

Fixes #72

@weierophinney weierophinney added this to the 3.0.0 milestone Dec 4, 2018

weierophinney added a commit to weierophinney/zend-hydrator that referenced this pull request Dec 4, 2018

@weierophinney weierophinney force-pushed the weierophinney:feature/enabled-by-namespace branch from 658dd49 to 8a6336d Dec 4, 2018

weierophinney added a commit to weierophinney/zend-hydrator that referenced this pull request Dec 4, 2018

@weierophinney weierophinney force-pushed the weierophinney:feature/enabled-by-namespace branch from 8a6336d to b0610e7 Dec 5, 2018

weierophinney added a commit to weierophinney/zend-hydrator that referenced this pull request Dec 5, 2018

@weierophinney weierophinney force-pushed the weierophinney:feature/enabled-by-namespace branch from b0610e7 to 70a12f3 Dec 5, 2018

weierophinney added a commit to weierophinney/zend-hydrator that referenced this pull request Dec 5, 2018

@weierophinney

This comment has been minimized.

Member

weierophinney commented Dec 5, 2018

@Ocramius I've updated the patch as requested. The StandaloneHydratorPluginManager now uses (mostly) hard-coded lookups (I'm allowing casing variations for unqualified names). I've also added a section to the plugin managers documentation detailing how you might write a custom version.

@weierophinney weierophinney force-pushed the weierophinney:feature/enabled-by-namespace branch from 70a12f3 to 7e4b612 Dec 5, 2018

weierophinney added a commit to weierophinney/zend-hydrator that referenced this pull request Dec 5, 2018

weierophinney added some commits Dec 3, 2018

Initial standalone Hydrator plugin manager implementation
This patch provides an initial standalone plugin manager implementation,
for users who do not want to use zend-servicemanager, but still want to
use the `DelegatingHydrator`, or to compose the plugin manager in
classes that will retrieve hydrators.

To achieve this, it first adds the interface `Zend\Hydrator\HydratorPluginManagerInterface`,
which extends the PSR-11 `ContainerInterface`; this allows us to
typehint on hydrator plugin managers specifically, as well as ensure
they comply with PSR-11 (which zend-servicemanager v3 variants do not
currently). The `DelegatingHydratorFactory` now tests against that
interface instead of the `HydratorPluginManager` when attempting to
retrieve the hydrator plugin manager to use.

Next, the new `StandaloneHydratorPluginManager` is a non-extensible,
hard-coded container for retrieving the shipped hydrators only. If users
want something that can be configured, they should:

- Use the `HydratorPluginManager`
- Configure their application container to load hydrators
- Write their own implementation

The patch also adds a factory for the `StandaloneHydratorPluginManager`;
it simply instantiates and returns it.

Finally, `HydratorPluginManagerFactory` now raises an exception if
zend-servicemanager is not detected, and guides users to use the
`StandaloneHydratorPluginManager` or to install zend-servicemanager,
based on their configuration needs.
Register the StandaloneHydratorPluginManager service with the container
Registers the StandaloneHydratorPluginManager via factory configuration.

If zend-servicemanager is installed, it aliases the `HydratorManager`
service to the `HydratorPluginManager`, but otherwise uses the
`StandaloneHydratorPluginManager`.
Documents hydrator plugin managers
Documents what they are, why they exist, and how to configure each. It
also provides an example of a custom hydrator plugin manager.

@weierophinney weierophinney force-pushed the weierophinney:feature/enabled-by-namespace branch from 7e4b612 to 4f9a017 Dec 5, 2018

Show resolved Hide resolved docs/book/v3/plugin-managers.md
use Zend\Hydrator\HydratorPluginManagerInterface;
use Zend\Hydrator\StandaloneHydratorPluginManager;
class CustomHydratorPluginManager implements

This comment has been minimized.

@Ocramius

Ocramius Dec 6, 2018

Member

I think this section goes too deep in detail: describing what interface is needed should be sufficient, no?

This comment has been minimized.

@weierophinney

weierophinney Dec 6, 2018

Member

It may be too much detail for you or me, but having a concrete example to look at is really useful for junior and medior developers. I want to demonstrate a complete solution - even if it's something they would need to alter for their specific needs.

This comment has been minimized.

@Ocramius

Ocramius Dec 6, 2018

Member

This is code sitting in the docs, hidden in a block in a very edge case area: I'd simplify it to the bone to prevent that.

Show resolved Hide resolved docs/book/v3/plugin-managers.md Outdated
Show resolved Hide resolved src/HydratorPluginManagerInterface.php
Show resolved Hide resolved src/StandaloneHydratorPluginManager.php Outdated
Show resolved Hide resolved src/StandaloneHydratorPluginManager.php Outdated
Show resolved Hide resolved src/StandaloneHydratorPluginManager.php Outdated
Show resolved Hide resolved src/StandaloneHydratorPluginManager.php Outdated
Show resolved Hide resolved src/StandaloneHydratorPluginManagerFactory.php

weierophinney added some commits Dec 6, 2018

Make documentation changes based on feedback from @Ocramius
- Split code block into one for each discrete class and/or configuration; list location of file for each.
- Remove `ContainerInterface` from list of what `CustomHydratorPluginManager` implements; it is implied by `HydratorPluginManagerInterface`.
Adds `@method` annotation for `get()` to `HydratorPluginManagerInterf…
…ace`

- Indicates that the `get()` method should return a hydrator implementation.
Remove `$invokableFactory` property from StandaloneHydratorPluginManager
- Not needed, as assigned in constructor, and never again referenced.
Remove redundancies
- useless variable assignment in `resolveName()`
- basically useless docblock for `resolveName()` (as implementation can
  be easily understood looking at its five lines of code)
class StandaloneHydratorPluginManagerTest extends TestCase
{
public function setUp()

This comment has been minimized.

@malukenho

malukenho Dec 7, 2018

Member

IMHO we should keep the visibility from the parent class, so it could be protected instead.

This comment has been minimized.

@weierophinney

weierophinney Dec 7, 2018

Member

We use public visibility pretty consistently everywhere for the setUp and tearDown methods. I'm not going to change it here.

@Ocramius Ocramius self-assigned this Dec 8, 2018

@Ocramius

🚢

@Ocramius Ocramius merged commit f074aa3 into zendframework:develop Dec 8, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@weierophinney weierophinney deleted the weierophinney:feature/enabled-by-namespace branch Dec 10, 2018

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