IBX-7987: Node filter for extraction text

[adamwojs]

Question	Answer
JIRA issue	IBX-7987
Bug/Improvement	yes
New feature	yes
Target version	4.6+
BC breaks	no
Tests pass	yes
Doc needed	yes

Background

See JIRA issue.

Node filtering

This PR introduced extension point allowing to exclude nodes from text extraction:

namespace Ibexa\Contracts\FieldTypeRichText\RichText\TextExtractor;

interface NodeFilterInterface
{
    /**
     * Return false to preserve the node, true to remove it.
     */
    public function filter(DOMNode $node): bool;
}

\Ibexa\Contracts\FieldTypeRichText\RichText\TextExtractor\NodeFilterInterface implementations should be tagged via ibexa.field_type.richtext.text_extractor.node_filter

Common use case

\Ibexa\Contracts\FieldTypeRichText\RichText\TextExtractor\NodeFilterFactoryInterface ¹ covers the common use case: excluding node with given path, e.g. /foo/bar/baz.

interface NodeFilterFactoryInterface
{
    public function createPathFilter(string ...$path): NodeFilterInterface;
}

Usage example:

services:
    # ...
    app.field_type.richtext.text_extractor.node_filter.custom:
        class: Ibexa\Contracts\FieldTypeRichText\RichText\TextExtractor\NodeFilterInterface
        factory: ['@Ibexa\Contracts\FieldTypeRichText\RichText\TextExtractor\NodeFilterFactoryInterface', 'createPathFilter']
        arguments: ['foo', 'bar', 'baz']
        tags:
            - { name: ibexa.field_type.richtext.text_extractor.node_filter }

¹ The Factory pattern prevents the exposure of implementation details within contracts

Nodes excluded out of the box

The following nodes are excluded by default (xpath syntax):

• //eztemplate/ezconfig

Notes for QA

Identify other potential elements which should be excluded from full text index

TODO:

• Implement feature / fix a bug.
• Implement tests.
• Fix new code according to Coding Standards ($ composer fix-cs).
• Ask for Code Review.

[konradoboza]

Interesting approach and convenient usage. LGTM.

[alongosz]

IMHO the result of false/true combined with filter might be a bit misleading. My first thought was that it should work the opposite way. Maybe changing it to filterOut would be more clear?

[konradoboza]

I personalny stand with the original naming.

[Steveb-p]

I have to agree with @alongosz here.

Most methods (ArrayCollection::filter) and functions (array_filter) work in the opposite way:

• when true, an entry is preserved
• when false, an entry is removed

Therefore, I'd suggest reversing the logic to comply with generally established PHP practice.

[Steveb-p]

LGTM. Before giving approval, would love to see a test that includes both FullTextExtractor and filters that you're using, as we're solving a specific need - to filter out nodes when working with FullTextExtractor, right?

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[Steveb-p]

I have to agree with @alongosz here.

Most methods (ArrayCollection::filter) and functions (array_filter) work in the opposite way:

• when true, an entry is preserved
• when false, an entry is removed

Therefore, I'd suggest reversing the logic to comply with generally established PHP practice.