Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Added filter possibility #3359

Merged
merged 14 commits into from Jan 14, 2013

Conversation

Projects
None yet
3 participants
Contributor

iwalz commented Jan 6, 2013

I often came around with the ClassMethods hydrator, that there are parts in the hydration that shouldn't be there. Or related to #3352 that the method definition does not match.
Those few lines will give you the possibility, to easily ignore the "has" methods in your hydration e.g:

$hydrator = new ClassMethods(false);
$hydrator->removeFilter('has');
$hydrator->extract($myObject);

Or you can easily add custom filters:

$hydrator = new ClassMethods(false);
$hydrator->addFilter('serviceLocator',
    function($property) {
        list($class, $method) = explode('::', $property);
        // do not hydrate getServiceLocator
        if ($method === 'getServiceLocator') {
            return false;
        }
        return true;
    }, FilterComposite::CONDITION_AND
);
$hydrator->extract($myObject);

Heavily tested code may have more getters and setters than necessary, but if you want to transform your objects to the userland, you may want to strip some getters and setters.

Internally, there are 2 arrays, where the composition is working on. The "OR", where only 1 method from that collection has to return true and on the other hand, the "AND" condition, where all filters has to return true to get hydrated. With this implementation, #3352 can be easily implemented as another filter. If you want me to do so, close #3352 and give me a hint here. What do you think about the idea/use-case/implementation? If you like the idea behind, I can make it work the abstract-hydrator too.

Owner

weierophinney commented Jan 7, 2013

I asked @iwalz on IRC for more details on this, specifically how this differs from Zend\Stdlib\Hydrator\Strategy. Here is the conversation we had:

14:42:58 < waly> weierophinney: the problem is, the strategy can be used to manipulate stuff afterwards
14:43:18 < waly> weierophinney: that one will not even add it to the internal stuff that can be manipulated by the filter
14:43:26 < weierophinney> waly: what would be awesome is if you could update the description to show what you were trying to do with strategy, and either couldn't (or using it opened up your objects to incorrect manipulation), and then show how this solves that shortcoming.
14:43:28 < waly> s/filter/strategy/
14:44:34 < waly> weierophinney: the problem was this match:
14:44:37 < waly> if (!preg_match('/^(get|has|is)[A-Z]\w*/', $method))
14:44:58 < waly> weierophinney: if that matches, even if you don't want to use it, it's getting extracted
14:45:51 < weierophinney> waly: aha, I see.
14:46:03 < weierophinney> so for things like composed event managers, input filters, etc, that raises a problem.
14:46:15 < waly> weierophinney: same with something like hasFoo($bar). It will get executed without a parameter and break (there is another pr, linked there)
14:47:11 < waly> weierophinney: If you decide to merge, I'll add a "NoneParameterFilter" to fix this. But if not, the other one simply fix that issue

@weierophinney weierophinney commented on an outdated diff Jan 7, 2013

library/Zend/Stdlib/Hydrator/ClassMethods.php
@@ -10,7 +10,11 @@
namespace Zend\Stdlib\Hydrator;
-use Zend\Stdlib\Exception;
+use Zend\Stdlib\Exception,
+ Zend\Stdlib\Hydrator\Filter\FilterComposite,
+ Zend\Stdlib\Hydrator\Filter\IsFilter,
+ Zend\Stdlib\Hydrator\Filter\GetFilter,
+ Zend\Stdlib\Hydrator\Filter\HasFilter;
@weierophinney

weierophinney Jan 7, 2013

Owner

To follow PSR-1/2, we need one use per import (i.e., do not use the "comma" notation with imports).

@weierophinney weierophinney commented on an outdated diff Jan 7, 2013

library/Zend/Stdlib/Hydrator/Filter/FilterComposite.php
+ */
+namespace Zend\Stdlib\Hydrator\Filter;
+
+use Zend\Stdlib\Exception\InvalidArgumentException;
+
+/**
+ * @category Zend
+ * @package Zend_Stdlib
+ * @subpackage Hydrator
+ */
+class FilterComposite implements FilterInterface
+{
+ /**
+ * @var \ArrayObject
+ */
+ protected $orFilter;
@weierophinney

weierophinney Jan 7, 2013

Owner

Please add an empty line after each property declaration.

@weierophinney weierophinney commented on an outdated diff Jan 7, 2013

library/Zend/Stdlib/Hydrator/Filter/FilterComposite.php
+ * Constant to add with "and" conditition
+ */
+ const CONDITION_AND = 2;
+
+ /**
+ * Define default Filter
+ *
+ * @throws InvalidArgumentException
+ */
+ public function __construct($orFilter = array(), $andFilter = array())
+ {
+ array_walk($orFilter,
+ function($value, $key) {
+ if(
+ !is_callable($value) &&
+ !$value instanceof FilterInterface
@weierophinney

weierophinney Jan 7, 2013

Owner

Move the "&&" to the next line (should prefix the line, not suffix, per ZF CS).

@weierophinney weierophinney commented on an outdated diff Jan 7, 2013

library/Zend/Stdlib/Hydrator/Filter/FilterComposite.php
+ if(
+ !is_callable($value) &&
+ !$value instanceof FilterInterface
+ ) {
+ throw new InvalidArgumentException(
+ 'The value of ' . $key . ' should be either a callable or ' .
+ 'an instance of Zend\Stdlib\Hydrator\Filter\FilterInterface'
+ );
+ }
+ }
+ );
+
+ array_walk($andFilter,
+ function($value, $key) {
+ if(
+ !is_callable($value) &&
@weierophinney

weierophinney Jan 7, 2013

Owner

Same comment as line 50

@weierophinney weierophinney commented on an outdated diff Jan 7, 2013

library/Zend/Stdlib/Hydrator/Filter/FilterComposite.php
+
+ array_walk($andFilter,
+ function($value, $key) {
+ if(
+ !is_callable($value) &&
+ !$value instanceof FilterInterface
+ ) {
+ throw new InvalidArgumentException(
+ 'The value of ' . $key . ' should be either a callable or ' .
+ 'an instance of Zend\Stdlib\Hydrator\Filter\FilterInterface'
+ );
+ }
+ }
+ );
+
+ $this->orFilter = new \ArrayObject($orFilter);
@weierophinney

weierophinney Jan 7, 2013

Owner

Import ArrayObject

@weierophinney weierophinney commented on an outdated diff Jan 7, 2013

library/Zend/Stdlib/Hydrator/Filter/FilterComposite.php
+ * return false;
+ * }
+ * return true;
+ * }, FilterComposite::CONDITION_AND
+ * );
+ * </code>
+ *
+ * @param string $name
+ * @param callable|FilterInterface $filter
+ * @param int $condition Can be either FilterComposite::CONDITION_OR or FilterComposite::CONDITION_AND
+ * @throws InvalidArgumentException
+ */
+ public function addFilter($name, $filter, $condition = self::CONDITION_OR)
+ {
+ if (
+ is_callable($filter) ||
@weierophinney

weierophinney Jan 7, 2013

Owner

Put condition onto next line

@weierophinney weierophinney commented on an outdated diff Jan 7, 2013

library/Zend/Stdlib/Hydrator/Filter/FilterComposite.php
+ * @param callable|FilterInterface $filter
+ * @param int $condition Can be either FilterComposite::CONDITION_OR or FilterComposite::CONDITION_AND
+ * @throws InvalidArgumentException
+ */
+ public function addFilter($name, $filter, $condition = self::CONDITION_OR)
+ {
+ if (
+ is_callable($filter) ||
+ $filter instanceof FilterInterface
+ ) {
+ if ($condition === self::CONDITION_OR) {
+ $this->orFilter[$name] = $filter;
+ } elseif ($condition === self::CONDITION_AND) {
+ $this->andFilter[$name] = $filter;
+ }
+ } else {
@weierophinney

weierophinney Jan 7, 2013

Owner

Reverse the conditional, so that the method body doesn't happen inside a conditional:

if (!is_callable($filter) && !$filter instanceof FilterInterface) {
    /* throw new ... */
}

if ($condition === self::CONDITION_OR) {
    $this->orFilter[$name] = $filter;
    return $this;
}
if ($condition === self::CONDITION_AND) {
    $this->andFilter[$name] = $filter;
    return $this;
}
return $this;

@weierophinney weierophinney commented on an outdated diff Jan 7, 2013

library/Zend/Stdlib/Hydrator/Filter/FilterComposite.php
+
+ if( isset($this->andFilter[$name])) {
+ unset($this->andFilter[$name]);
+ }
+ }
+
+ /**
+ * Check if $name has a filter registered
+ *
+ * @param $name string Identifier for the filter
+ * @return bool
+ */
+ public function hasFilter($name)
+ {
+ return
+ isset($this->orFilter[$name]) || isset($this->andFilter[$name]);
@weierophinney

weierophinney Jan 7, 2013

Owner

Join these lines.

@weierophinney weierophinney commented on an outdated diff Jan 7, 2013

library/Zend/Stdlib/Hydrator/Filter/FilterComposite.php
+ count($this->orFilter) === 0 &&
+ count($this->andFilter) === 0
+ ) {
+ return true;
+ }
+
+ // Check if 1 from the or filters return true
+ $returnValue = false;
+ foreach($this->orFilter as $filter) {
+ if (is_callable($filter)) {
+ if( $filter($property) === true)
+ {
+ $returnValue = true;
+ break;
+ }
+ } else {
@weierophinney

weierophinney Jan 7, 2013

Owner

Add a continue before the else, and then you can do away with the else statement.

@weierophinney weierophinney commented on an outdated diff Jan 7, 2013

library/Zend/Stdlib/Hydrator/Filter/FilterComposite.php
+ }
+ } else {
+ if ( $filter->filter($property) === true) {
+ $returnValue = true;
+ break;
+ }
+ }
+ }
+
+ // Check if all of the and condition return true
+ foreach($this->andFilter as $filter) {
+ if (is_callable($filter)) {
+ if( $filter($property) === false) {
+ return false;
+ }
+ } else {
@weierophinney

weierophinney Jan 7, 2013

Owner

Add a continue inside the conditional, and then you can do away with the else statement.

@weierophinney weierophinney commented on an outdated diff Jan 7, 2013

tests/ZendTest/Stdlib/HydratorTest.php
+ $this->assertTrue(isset($datas['isFoo']));
+ $this->assertEquals($datas['isFoo'], true);
+ $this->assertTrue(isset($datas['isBar']));
+ $this->assertEquals($datas['isBar'], true);
+ $this->assertTrue(isset($datas['hasFoo']));
+ $this->assertEquals($datas['hasFoo'], true);
+ $this->assertTrue(isset($datas['hasBar']));
+ $this->assertEquals($datas['hasBar'], true);
+
+ $hydrator->removeFilter('has');
+ $datas = $hydrator->extract($this->classMethodsCamelCase);
+ $this->assertTrue(isset($datas['hasFoo'])); //method is getHasFoo
+ $this->assertFalse(isset($datas['hasBar'])); //method is hasBar
+ }
+
+ public function testHydratorClassMethodsWithCostumFilter()
@weierophinney

weierophinney Jan 7, 2013

Owner

Small typo: s/Costum/Custom/ :)

Owner

weierophinney commented Jan 7, 2013

@iwalz Functionality looks good -- I'd love to see you incorporate the filter functionality into the AbstractHydrator, and use it to resolve #3352 as well.

Thanks!

@ghost ghost assigned weierophinney Jan 7, 2013

Contributor

iwalz commented Jan 9, 2013

Should be done @weierophinney
I'll update the docs these days, if you want to, I can add a section about the strategies too (haven't seen one on rtd).

Contributor

iwalz commented Jan 11, 2013

With the latest commit, you can tell your own objects how to get hydrated to the userland, providing a filterinterface.
A MethodMatchFilter has been added too.

Can you think for any use-case on hydrate()? That's all about extract() only

@marc-mabe marc-mabe commented on an outdated diff Jan 12, 2013

library/Zend/Stdlib/Hydrator/ArraySerializable.php
});
+ #var_dump($data);
@marc-mabe

marc-mabe Jan 12, 2013

Member

Please remove the commented-out var_dump

@iwalz iwalz referenced this pull request in zendframework/zf2-documentation Jan 12, 2013

Merged

Added hydrator filter doc #595

Contributor

iwalz commented Jan 13, 2013

The failed test on ZendTest/Cache seem not to be linked to the changes I made. The suite passed locally

@weierophinney weierophinney merged commit 4052748 into zendframework:develop Jan 14, 2013

1 check failed

default The Travis build failed
Details

weierophinney added a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015

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