laminas/laminas-hydrator

Redundant OptionalParameterFilter::filter calls

pine3ree opened this issue · 2 comments

In line:

if (! $filter->filter($methodFqn, $isAnonymous ? $object : null)

$filter may be either the hydrator's composite filter (which (AND)includes an OptionalParameterFilter) or an ad-hoc composite filter for the current FilterProviderInterface object instance, as provided by the initCompoisteFilter call.

This latter ad-hoc filter does not (AND)compose an optional-parameter filter, so the $this->callableMethodFilter->filter call in line:

|| ! $this->callableMethodFilter->filter($methodFqn, $isAnonymous ? $object : null)

is necessary.

At the same time, this is a duplicate check for the internal composite filter case, which already composes it (Just var_dump($property) in OptionalParameterFilter::filter() to see the consecutive duplicate calls).

Is there a reason for it, or am I just missing some very obvious point? :-)
Couldn't we just add an optional-parameter-filter (even the same shard instance) in the AND filters array of the FilterProviderInterface object in line:

[new Filter\MethodMatchFilter('getFilter')]

?
like...

    private function initCompositeFilter(object $object) : Filter\FilterComposite
    {
        if ($object instanceof Filter\FilterProviderInterface) {
            return new Filter\FilterComposite(
                [$object->getFilter()],
                [
                     new Filter\MethodMatchFilter('getFilter'),
                     new Filter\OptionalParametersFilter() // ...and dropping the extra call. 
                     // We could even use 1 shared instance of this filter for the insternal composite-filter and all the FilterProviderInterface objects
                ]
            );
        }

        return $this->getCompositeFilter();
    }

kind regards

...or simply just drop

        $compositeFilter->addFilter(
            'parameter',
            new Filter\OptionalParametersFilter(),
            Filter\FilterComposite::CONDITION_AND
        );

keeping the extra call. In this case the unatached filter is already shared.

The fix was released with version 4.2.0.