Introduce self-rendering filters

[kafkiansky]

Q	A
Bugfix?	❌
Breaks BC?	✔️
New feature?	✔️

Allows to override the error rendering for a specific filter.

Example of usage

The renderer:

<?php
declare(strict_types=1);

namespace App;

use Spiral\Filters\FilterInterface;
use Spiral\Filters\RenderErrors;

/**
 * @template-implements RenderErrors<FilterInterface>
 */
final class SomeErrorsRenderer implements RenderErrors
{
    /**
     * {@inheritdoc}
     */
    public function render(FilterInterface $filter)
    {
        return [
            'success' => false,
            'errors' => $filter->getErrors(),
        ];
    }
}

The filter:

<?php

declare(strict_types=1);

namespace App;

use Spiral\Filters\Filter;
use Spiral\Filters\RenderWith;

#[RenderWith(SomeErrorsRenderer::class)]
final class FilterWithErrorsRenderer extends Filter
{
    public const SCHEMA = [
        'id' => 'query:id'
    ];

    public const VALIDATES = [
        'id' => [
            ['notEmpty', 'err' => '[[ID is not valid.]]']
        ]
    ];
}

[butschster]

Hi, @kafkiansky!

Thank you for the PR! Could you provide an example of using a new feature and some tests?

[kafkiansky]

Hi, @butschster. I wanted to add tests, but I couldn't find where to put them. Perhaps there are already tests for FilterInterceptor?

And the example of use is simple. We globally use FilterInterceptor in our project, which validates all filters before they hit the controller, which is very convenient. However, in some cases we need a specific response for validation errors, not the one used throughout the project: for example, one format is used for internal API, but for external systems we need to return a different format for validation errors. Using the ShouldRenderErrors interface will allow us to influence the format of errors in specific cases.

[butschster]

@kafkiansky It would be great to have something like in this PR https://github.com/spiral/framework/pull/733

[kafkiansky]

@butschster, added tests.

[codecov[bot]]

Codecov Report

Merging #737 (bee5129) into master (69b232a) will increase coverage by 0.24%. The diff coverage is 96.55%.

:exclamation: Current head bee5129 differs from pull request most recent head bdbde08. Consider uploading reports for the commit bdbde08 to get more accurate results

@@             Coverage Diff              @@
##             master     #737      +/-   ##
============================================
+ Coverage     79.89%   80.13%   +0.24%     
- Complexity     6724     6764      +40     
============================================
  Files           759      764       +5     
  Lines         17028    17122      +94     
============================================
+ Hits          13604    13721     +117     
+ Misses         3424     3401      -23     

Impacted Files	Coverage Δ
...rk/Domain/DefaultFilterErrorsRendererInterface.php	85.71% <85.71%> (ø)
src/Filters/src/RenderWith.php	100.00% <100.00%> (ø)
src/Framework/Domain/FilterInterceptor.php	92.68% <100.00%> (+2.20%)	:arrow_up:
...ramework/Domain/FilterWithAttributeInterceptor.php	100.00% <100.00%> (ø)
src/Queue/src/Bootloader/QueueBootloader.php	83.33% <0.00%> (ø)
...dcasting/src/Bootloader/BroadcastingBootloader.php	80.00% <0.00%> (ø)
src/Queue/src/PhpSerializer.php	80.00% <0.00%> (ø)
src/Queue/src/SerializerRegistry.php	100.00% <0.00%> (ø)
src/Queue/src/Config/QueueConfig.php	95.83% <0.00%> (+0.37%)	:arrow_up:
...ternal/Instantiator/NamedArgumentsInstantiator.php	98.61% <0.00%> (+29.16%)	:arrow_up:
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[wolfy-j]

Looks cool!

[kafkiansky]

I think the rendering is responsibility of a Filter. Idea about rendering customization is great but it should be not in a filter.

I think whoever is owned the data and the validation rules for that data, and may know how to output the error information. Otherwise we would have to keep this behaviour separate, which would complicate the creation and maintenance of filters. Moreover, this behaviour is optional.

[wolfy-j]

Maybe just let interceptor to know which renderer to use via attribute on filter?

[kafkiansky]

Maybe just let interceptor to know which renderer to use via attribute on filter?

I don't think such a small feature should require that much boilerplate. In that case you need:

1. A specific interface that be able to render a validation errors:

<?php

declare(strict_types=1);

use Spiral\Filters\Filter;

/**
 * @psalm-template F of Filter
 */
interface RenderErrors
{
     /**
      * @psalm-param F $filter
      *
      * @return mixed
      */
     public function render(Filter $filter);
}

2. An attribute:

<?php

declare(strict_types=1);

#[\Attribute(\Attribute::TARGET_CLASS)]
final class RenderWith
{
    /**
     * @param class-string<RenderErrors> $rendererFqcn
     */
    public function __construct(
          public readonly string $rendererFqcn,
    ) {}
}

3. Some locator that be able to collect the renderable filters to avoid using reflection in runtime each time.

The only advantage of this approach that I have found is that such classes will be autowired:

<?php

declare(strict_types=1);

use Spiral\Filters\Filter;
use Psr\Log\LoggerInterface;

/**
 * @template-implements RenderErrors<Filter>
 */
final class SomeSpecificRenderer implements RenderErrors
{
     public function __construct(
          private readonly LoggerInterface $logger,
     ) {}

     /**
      * {@inheritdoc}
      */
     public function render(Filter $filter)
     {
           $this->logger->info(...);
           
           return [...];
     }
}

I don't think this approach is much cleaner than the one I suggested. But definitely not worse.

I understood your idea correctly?

[kafkiansky]

I updated the PR, but now it has added BC.

[roxblnfk]

Separated interceptor and annotation look better