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

ServiceManager v2-v3 compatibility #95

Merged
merged 3 commits into from
Feb 18, 2016
Merged

ServiceManager v2-v3 compatibility #95

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7756cc1
ServiceManager v2-v3 compatibility
Dec 8, 2015
7796548
Giving --prefer-lowest a try. v2 should test under 5.5, v3 under 5.6
kynx Feb 13, 2016
310470c
Try travis now zend-validator-2.6 is there
kynx Feb 17, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,15 @@ matrix:
- php: 5.5
env:
- EXECUTE_CS_CHECK=true
- php: 5.5
env:
- SERVICE_MANAGER_VERSION="^2.7.5"
Copy link
Member

Choose a reason for hiding this comment

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

I disagree with this. The correct is to test lowest version (composer --prefer-lowest) and newest versions. All versions released in the middle are expected to be compatible and fulfill the API contract.

When we should to test specific versions? When the code HAS an specific logic branch testing if some feature is available i.e. (component_version >= 5.0) ? new API : old API. And this thing should be do with only 1 PHP version and not 5.5. and 5.6 whcih make me the following question?

Why PHP 7 and HHVM are not duplicated?

I think the test matrix proposed in this PR is not necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hum, OK, didn't know about --prefer-lowest (not my preference often ;) I've just been following what I've seen @weierophinney do in PRs for zend-i18n etc.

I noticed @ezimuel has done it slightly differently, with a SERVICE_MANAGER_V2=(true|false). So something like:

install:
  - if [[ $SERVICE_MANAGER_V2 != 'true' ]]; then travis_retry composer install --no-interaction --ignore-platform-reqs ; fi
  - if [[ $SERVICE_MANAGER_V2 == 'true' ]]; then travis_retry composer install --no-interaction --ignore-platform-reqs --prefer-lowest ; fi

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

s/composer install/composer update/ - install doesn't do --prefer-lowest

- php: 5.6
env:
- EXECUTE_TEST_COVERALLS=true
- php: 5.6
env:
- SERVICE_MANAGER_VERSION="^2.7.5"
- php: 7
- php: hhvm
allow_failures:
Expand All @@ -41,6 +47,8 @@ script:
- if [[ $EXECUTE_TEST_COVERALLS == 'true' ]]; then ./vendor/bin/phpunit --coverage-clover clover.xml ; fi
- if [[ $EXECUTE_TEST_COVERALLS != 'true' ]]; then ./vendor/bin/phpunit ; fi
- if [[ $EXECUTE_CS_CHECK == 'true' ]]; then ./vendor/bin/php-cs-fixer fix -v --diff --dry-run ; fi
- if [[ $SERVICE_MANAGER_VERSION != '' ]]; then composer require --no-update "zendframework/zend-servicemanager:$SERVICE_MANAGER_VERSION" ; fi
- if [[ $SERVICE_MANAGER_VERSION == '' ]]; then composer require --no-update "zendframework/zend-servicemanager:^3.0.3" ; fi

after_script:
- if [[ $EXECUTE_TEST_COVERALLS == 'true' ]]; then ./vendor/bin/coveralls ; fi
8 changes: 4 additions & 4 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
},
"require": {
"php": ">=5.5",
"zendframework/zend-filter": "~2.5",
"zendframework/zend-validator": "^2.5.3",
"zendframework/zend-stdlib": "~2.5"
"zendframework/zend-filter": "^2.6.0",
"zendframework/zend-validator": "^2.6.0",
"zendframework/zend-stdlib": "^2.7 || ^3.0"
},
"require-dev": {
"zendframework/zend-servicemanager": "~2.5",
"zendframework/zend-servicemanager": "^2.7.5 || ^3.0.3",
"fabpot/php-cs-fixer": "1.7.*",
"phpunit/PHPUnit": "^4.5"
},
Expand Down
14 changes: 3 additions & 11 deletions src/Factory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

use Traversable;
use Zend\Filter\FilterChain;
use Zend\ServiceManager\ServiceLocatorInterface;
use Zend\ServiceManager\ServiceManager;
use Zend\Stdlib\ArrayUtils;
use Zend\Validator\ValidatorChain;
use Zend\Validator\ValidatorInterface;
Expand Down Expand Up @@ -117,15 +117,7 @@ public function clearDefaultValidatorChain()
public function setInputFilterManager(InputFilterPluginManager $inputFilterManager)
{
$this->inputFilterManager = $inputFilterManager;
$serviceLocator = $this->inputFilterManager->getServiceLocator();
if ($serviceLocator && $serviceLocator instanceof ServiceLocatorInterface) {
if ($serviceLocator->has('ValidatorManager')) {
$this->getDefaultValidatorChain()->setPluginManager($serviceLocator->get('ValidatorManager'));
}
if ($serviceLocator->has('FilterManager')) {
$this->getDefaultFilterChain()->setPluginManager($serviceLocator->get('FilterManager'));
}
}
$inputFilterManager->populateFactoryPluginManagers($this);
return $this;
}

Expand All @@ -135,7 +127,7 @@ public function setInputFilterManager(InputFilterPluginManager $inputFilterManag
public function getInputFilterManager()
{
if (null === $this->inputFilterManager) {
$this->inputFilterManager = new InputFilterPluginManager;
$this->inputFilterManager = new InputFilterPluginManager(new ServiceManager());
}

return $this->inputFilterManager;
Expand Down
66 changes: 50 additions & 16 deletions src/InputFilterAbstractServiceFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@

namespace Zend\InputFilter;

use Interop\Container\ContainerInterface;
use Zend\Filter\FilterPluginManager;
use Zend\ServiceManager\AbstractFactoryInterface;
use Zend\ServiceManager\ServiceLocatorInterface;
use Zend\ServiceManager\ServiceManager;
use Zend\Validator\ValidatorPluginManager;

class InputFilterAbstractServiceFactory implements AbstractFactoryInterface
Expand All @@ -21,22 +23,47 @@ class InputFilterAbstractServiceFactory implements AbstractFactoryInterface
*/
protected $factory;


/**
* @param ServiceLocatorInterface $inputFilters
* @param ContainerInterface $services
* @param string $rName
* @param array $options
* @return InputFilterInterface
*/
public function __invoke(ContainerInterface $services, $rName, array $options = null)
{
// v2 - get parent service manager
if (! method_exists($services, 'configure')) {
$services = $services->getServiceLocator();
}

$allConfig = $services->get('config');
$config = $allConfig['input_filter_specs'][$rName];

$factory = $this->getInputFilterFactory($services);

return $factory->createInputFilter($config);
}

/**
*
* @param ContainerInterface $services
* @param string $cName
* @param string $rName
* @return bool
*/
public function canCreateServiceWithName(ServiceLocatorInterface $inputFilters, $cName, $rName)
public function canCreate(ContainerInterface $services, $rName)
{
$services = $inputFilters->getServiceLocator();
if (! $services instanceof ServiceLocatorInterface
|| ! $services->has('Config')
) {
// v2 - get parent service manager
if (! method_exists($services, 'configure')) {
$services = $services->getServiceLocator();
}

if (! $services->has('config')) {
return false;
}

$config = $services->get('Config');
$config = $services->get('config');
if (!isset($config['input_filter_specs'][$rName])
|| !is_array($config['input_filter_specs'][$rName])
) {
Expand All @@ -46,6 +73,19 @@ public function canCreateServiceWithName(ServiceLocatorInterface $inputFilters,
return true;
}

/**
* Determine if we can create a service with name (v2)
*
* @param ServiceLocatorInterface $serviceLocator
* @param $name
* @param $requestedName
* @return bool
*/
public function canCreateServiceWithName(ServiceLocatorInterface $serviceLocator, $name, $requestedName)
{
return $this->canCreate($serviceLocator, $requestedName);
Copy link
Member

Choose a reason for hiding this comment

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

This method, when invoked, can only happen from v2, so this is where the check for the parent locator should occur (versus in canCreate()). (Similar should occur for createServiceWithName() / __invoke().) I'll make that change on merge.

}

/**
* @param ServiceLocatorInterface $inputFilters
* @param string $cName
Expand All @@ -54,13 +94,7 @@ public function canCreateServiceWithName(ServiceLocatorInterface $inputFilters,
*/
public function createServiceWithName(ServiceLocatorInterface $inputFilters, $cName, $rName)
{
$services = $inputFilters->getServiceLocator();
$allConfig = $services->get('Config');
$config = $allConfig['input_filter_specs'][$rName];

$factory = $this->getInputFilterFactory($services);

return $factory->createInputFilter($config);
return $this($inputFilters, $rName);
}

/**
Expand Down Expand Up @@ -94,7 +128,7 @@ protected function getFilterPluginManager(ServiceLocatorInterface $services)
return $services->get('FilterManager');
}

return new FilterPluginManager();
return new FilterPluginManager(new ServiceManager());
Copy link
Member

Choose a reason for hiding this comment

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

s/new ServiceManager()/$services/ (in both this and the getValidatorPluginManager() method); we already have the instance, so why create a new one? 😄

I'll make that change on merge.

}

/**
Expand All @@ -107,6 +141,6 @@ protected function getValidatorPluginManager(ServiceLocatorInterface $services)
return $services->get('ValidatorManager');
}

return new ValidatorPluginManager();
return new ValidatorPluginManager(new ServiceManager());
}
}
106 changes: 88 additions & 18 deletions src/InputFilterPluginManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@

namespace Zend\InputFilter;

use Interop\Container\ContainerInterface;
use Zend\ServiceManager\AbstractPluginManager;
use Zend\ServiceManager\ConfigInterface;
use Zend\ServiceManager\ServiceLocatorInterface;
use Zend\ServiceManager\Exception\InvalidServiceException;
use Zend\ServiceManager\Factory\InvokableFactory;
use Zend\Stdlib\InitializableInterface;

/**
Expand All @@ -22,55 +23,100 @@
class InputFilterPluginManager extends AbstractPluginManager
{
/**
* Default set of plugins
* Default alias of plugins
*
* @var string[]
*/
protected $invokableClasses = [
protected $aliases = [
'inputfilter' => InputFilter::class,
'inputFilter' => InputFilter::class,
'InputFilter' => InputFilter::class,
'collection' => CollectionInputFilter::class,
'Collection' => CollectionInputFilter::class,
];

/**
* Whether or not to share by default
* Default set of plugins
*
* @var string[]
*/
protected $factories = [
InputFilter::class => InvokableFactory::class,
CollectionInputFilter::class => InvokableFactory::class,
// v2 canonical FQCN
'zendinputfilterinputfilter' => InvokableFactory::class,
'zendinputfiltercollectioninputfilter' => InvokableFactory::class,
];

/**
* Whether or not to share by default (v3)
*
* @var bool
*/
protected $sharedByDefault = false;

/**
* Whether or not to share by default (v2)
*
* @var bool
*/
protected $shareByDefault = false;


/**
* @param ConfigInterface $configuration
* @param ContainerInterface $parentLocator
* @param array $config
*/
public function __construct(ConfigInterface $configuration = null)
public function __construct(ContainerInterface $parentLocator, array $config = [])
Copy link
Member

Choose a reason for hiding this comment

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

First argument must also accept null for full BC with v2; I'll update that when merging. (Discovered this the hard way with zend-view 2.6.0.)

{
parent::__construct($configuration);

parent::__construct($parentLocator, $config);
$this->addInitializer([$this, 'populateFactory']);
}

/**
* Inject this and populate the factory with filter chain and validator chain
*
* @param $inputFilter
* @param mixed $first
* @param mixed $second
*/
public function populateFactory($inputFilter)
public function populateFactory($first, $second)
{
if ($first instanceof ContainerInterface) {
$container = $first;
$inputFilter = $second;
} else {
$container = $second;
$inputFilter = $first;
}
if ($inputFilter instanceof InputFilter) {
$factory = $inputFilter->getFactory();

$factory->setInputFilterManager($this);
}
}

if ($this->serviceLocator instanceof ServiceLocatorInterface) {
$factory->getDefaultFilterChain()->setPluginManager($this->serviceLocator->get('FilterManager'));
$factory->getDefaultValidatorChain()->setPluginManager($this->serviceLocator->get('ValidatorManager'));
}
public function populateFactoryPluginManagers(Factory $factory)
{
if (property_exists($this, 'creationContext')) {
// v3
$container = $this->creationContext;
} else {
// v2
$container = $this->serviceLocator;
}

if ($container && $container->has('FilterManager')) {
$factory->getDefaultFilterChain()->setPluginManager($container->get('FilterManager'));
}
if ($container && $container->has('ValidatorManager')) {
$factory->getDefaultValidatorChain()->setPluginManager($container->get('ValidatorManager'));
}
}

/**
* {@inheritDoc}
* {@inheritDoc} (v3)
*/
public function validatePlugin($plugin)
public function validate($plugin)
{
if ($plugin instanceof InputFilterInterface || $plugin instanceof InputInterface) {
// Hook to perform various initialization, when the inputFilter is not created through the factory
Expand All @@ -82,11 +128,35 @@ public function validatePlugin($plugin)
return;
}

throw new Exception\RuntimeException(sprintf(
throw new InvalidServiceException(sprintf(
'Plugin of type %s is invalid; must implement %s or %s',
(is_object($plugin) ? get_class($plugin) : gettype($plugin)),
InputFilterInterface::class,
InputInterface::class
));
}

/**
* Validate the plugin (v2)
*
* Checks that the filter loaded is either a valid callback or an instance
* of FilterInterface.
*
* @param mixed $plugin
* @return void
* @throws Exception\RuntimeException if invalid
*/
public function validatePlugin($plugin)
{
try {
$this->validate($plugin);
} catch (InvalidServiceException $e) {
Copy link
Member

Choose a reason for hiding this comment

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

This should be Exception\RuntimeException, no? (I'll update on merge)

Copy link
Member

Choose a reason for hiding this comment

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

Never mind, was confusing this with the next line. Nothing to see here, move along...

throw new Exception\RuntimeException($e->getMessage(), $e->getCode(), $e);
}
}

public function shareByDefault()
Copy link
Member

Choose a reason for hiding this comment

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

Why are you adding this?

{
return $this->sharedByDefault;
}
}
Loading