Class methods hydrator skips getters with optional parameters #4496

Merged
merged 5 commits into from May 24, 2013

2 participants

@Ocramius
Zend Framework member

Currently, Zend\Stdlib\Hydrator\ClassMethods skips getters like getFoo($bar = null) as they have >0 parameters.

This hotfix allows to call those methods as long as all parameters are optional.

@Ocramius
Zend Framework member

Travis failed because of some filesystem tests

@Ocramius Ocramius referenced this pull request in doctrine/DoctrineModule May 17, 2013
Merged

Add isser to extract by value #250

@Ocramius Ocramius and 1 other commented on an outdated diff May 17, 2013
...d/Stdlib/Hydrator/Filter/OptionalParametersFilter.php
+use InvalidArgumentException;
+use ReflectionException;
+use ReflectionMethod;
+use ReflectionParameter;
+
+/**
+ * Filter that includes methods which have no parameters or only optional parameters
+ */
+class OptionalParametersFilter implements FilterInterface
+{
+ /**
+ * {@inheritDoc}
+ */
+ public function filter($property)
+ {
+ try {
@Ocramius
Zend Framework member

In theory, this result could be cached in a private static property since the result depends only on reflection.

@weierophinney
Zend Framework member

So... do you want me to wait to merge until you've done that improvement as well?

@Ocramius
Zend Framework member

Will do it tomorrow then - otherwise fine? :)

@weierophinney
Zend Framework member

Overall, looks good! I think caching the value makes sense, though, and would like to see that in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on the diff May 24, 2013
library/Zend/Stdlib/Hydrator/ClassMethods.php
@@ -30,6 +30,11 @@ class ClassMethods extends AbstractHydrator implements HydratorOptionsInterface
protected $underscoreSeparatedKeys = true;
/**
+ * @var \Zend\Stdlib\Hydrator\Filter\FilterInterface
+ */
+ private $callableMethodFilter;
@weierophinney
Zend Framework member

Why private?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on the diff May 24, 2013
...d/Stdlib/Hydrator/Filter/OptionalParametersFilter.php
+use ReflectionMethod;
+use ReflectionParameter;
+
+/**
+ * Filter that includes methods which have no parameters or only optional parameters
+ */
+class OptionalParametersFilter implements FilterInterface
+{
+ /**
+ * Map of methods already analyzed
+ * by {@see \Zend\Stdlib\Hydrator\Filter\OptionalParametersFilter::filter()},
+ * cached for performance reasons
+ *
+ * @var bool[]
+ */
+ private static $propertiesCache = array();
@weierophinney
Zend Framework member

why private?

@Ocramius
Zend Framework member

As discussed on IRC, these properties only provide caching functionality, and aren't thought as extension points

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney merged commit 41f4b98 into zendframework:master May 24, 2013

1 check passed

Details default The Travis CI build passed
@weierophinney weierophinney added a commit that referenced this pull request May 24, 2013
@weierophinney weierophinney Merge branch 'hotfix/4496' into develop
Forward port #4496
672b83e
@weierophinney weierophinney added a commit that referenced this pull request May 24, 2013
@weierophinney weierophinney Merge branch 'hotfix/4496'
Close #4496
5758f25
@ghost Unknown pushed a commit that referenced this pull request Jul 14, 2013
@weierophinney weierophinney Merge branch 'hotfix/4496'
Close #4496
efcd99d
@weierophinney weierophinney added a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge pull request zendframework/zendframework#4496 from Ocramius/hot…
…fix/class-methods-hydrator-optional-parameters

Class methods hydrator skips getters with optional parameters
cf0246f
@weierophinney weierophinney added a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'hotfix/4496' fdeb27b
@weierophinney weierophinney added a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'hotfix/4496' into develop 8e2ba99
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment