Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possibility to use tags on filters #19

Open
RozbehSharahi opened this issue Apr 1, 2019 · 6 comments
Open

Possibility to use tags on filters #19

RozbehSharahi opened this issue Apr 1, 2019 · 6 comments

Comments

@RozbehSharahi
Copy link

Hello,

i would actually write this on https://github.com/ezsystems/ezplatform-solr-search-engine but unfortunately the issue board is turned off there and the creating the pull-request for that is maybe to early.

It seems though that this problem also exists with your extra bundle.

When trying to exclude filters from facets in order to have f.i. multi-select facets i realized that it is not possible with the current implementation of ezplatform-solr-bundle.

Normally you would do that by tagging your filters and excluding them on the facet, see:
http://yonik.com/multi-select-faceting/

Unfortunately the solr bundle is just sending the whole query as one long query string:
fq={!lucene}bla:123 AND (bar:123) AND (foo:333)
which makes tagging impossible as far as I know, because this could only be achieved like follwoing:

fq[]={!tag=bla_filter}bla:123
fq[]={!tag=foo_filter}foo:333
fq[]={!tag=bar_filter}bar:123

The idea of having an agnostic data-layer where you can use solr as if it was a SQL Database conflicts with this since queries are nested as you would like to:
AND (OR (AND bar=1)))

I am not an expert on solr or maybe there is another way to exclude filters on facets which i am not aware of.

Do you have experience with that? Or is this to be considered a real issue?

Kind regards
Robert (Rozbeh Chiryai Sharahi)

@RozbehSharahi
Copy link
Author

I think that creating an extra property on the query object beside Query::filters with the name Query::facetFilters that will automatically tag those filters could be a solution for instance. This way we can also make sure that our facet filters are not inside other ANDs and ORs that were applied before.

@pspanja
Copy link
Member

pspanja commented Apr 2, 2019

Hi @RozbehSharahi, thanks for the suggestion!

I'm not aware there's an alternative way to tag and exclude. Technically, we could go with an extra property, but also it would not be complicated to take top level filter criteria that is combined with logical AND and pass it through multiple fq parameters.

I think the main challenge here is to come up with PHP API that would be simple to use, and also compatible with the current semantics. Thinking a little bit about it, one other approach would be to go for a method on the Query object that would work with spl_object_hash, like:

public function tagFilter(Criterion $criteria, string $tag): void;

$query->tagFilter($criteria, $tag);

If you could suggest/mock an API, I think it would be a good way to get this started.

FYI @alongosz, @andrerom

@andrerom
Copy link

andrerom commented Apr 2, 2019

/cc @adamwojs

@RozbehSharahi
Copy link
Author

@pspanja Hello,

i agree that using this should be as simple as possible. Taking the top level filter is not that easy, because the solr bundle will add a couple of other conditions to the query which results into following:

filters = LogicalAnd: [
  0 => SomeQuery from the solr bundle regarind (locations and user ...)
  1 => SomeOtherQuery,
  2 => LogicalAnd [
    // Here are the condition that were actually defined from the user/developer
    0 => ...
    1 => ...
  ]
]

This means the conditions are actually not the top level criteria and we never know how many criteria were added before by the framework. It sound very fragile to me.

What I did on the current project im working on is to add a new field called facetFilters on the query object Query::setFacetFilters(array $facetFilters)

And instead of directly setting a list of criterion i set a new class that i created called FacetFilter

FacetFilter consists of two properties: tag and criterion.

And of course i had to extend NativeQueryConverter and add the FacetFilter class.

This is the code:
EzSystems\EzPlatformSolrSearchEngine\Query\Common\QueryConverter\NativeQueryConverter

public function convert(Query $query)
    {
        $params = array(
            'defType' => 'edismax',
            'q' => '{!lucene}'.$this->criterionVisitor->visit($query->query),
            'fq' => '{!lucene}'.$this->criterionVisitor->visit($query->filter),
            'sort' => $this->getSortClauses($query->sortClauses),
            'start' => $query->offset,
            'rows' => $query->limit,
            'fl' => '*,score,[shard]',
            'wt' => 'json',
        );

        $facetParams = $this->getFacetParams($query->facetBuilders);
        if (!empty($facetParams)) {
            $params['facet'] = 'true';
            $params['facet.sort'] = 'count';
            $params = array_merge($facetParams, $params);
        }

        // This is very i apply the facetFilters
        $params = $this->applyFacetFilters($query, $params);

        return $params;
    }

    /**
     * @param Query $query
     * @param array $params
     *
     * @return array
     */
    protected function applyFacetFilters(Query $query, array $params): array
    {
        if (!$query instanceof SolrQuery) {
            return $params;
        }

        $filterQueryIndex = 1;

        // First transform fq to array, since we want to send multiple fqs
        $params['fq'] = [
            ($filterQueryIndex++) => $params['fq'],
        ];

        // now add facet filters
        $facetFilterQueries = array_map(function (FacetFilter $facetFilter) {
            $tag = $facetFilter->getTag();
            $solrCriterion = $this->criterionVisitor->visit($facetFilter->getCriterion());

            return '{!tag='.$tag.'}'.$solrCriterion;
        }, $query->getFacetFilters());
        foreach ($facetFilterQueries as $facetFilterQuery) {
            $params['fq'][$filterQueryIndex++] = $facetFilterQuery;
        }

        return $params;
    }

// FacetFilter class:

<?php

namespace AppBundle\Solr\API\Repository\Values\Content;

use eZ\Publish\API\Repository\Values\Content\Query\Criterion;
use eZ\Publish\API\Repository\Values\ValueObject;

class FacetFilter extends ValueObject
{
    /**
     * @var string
     */
    protected $tag = 'no-tag';

    /**
     * @var Criterion
     */
    protected $criterion;

    /**
     * @return string
     */
    public function getTag(): string
    {
        return $this->tag;
    }

    /**
     * @param string $tag
     *
     * @return FacetFilter
     */
    public function setTag(string $tag): self
    {
        $this->tag = $tag;

        return $this;
    }

    /**
     * @return Criterion
     */
    public function getCriterion(): ?Criterion
    {
        return $this->criterion;
    }

    /**
     * @param Criterion $criterion
     *
     * @return FacetFilter
     */
    public function setCriterion(?Criterion $criterion): self
    {
        $this->criterion = $criterion;

        return $this;
    }
}

And finally this is how you use it:

$query = new Query();
$query->setFacetFilters(
  (new FacetFilter)->setTag('My custom tag')->setCriterion(new Criterion\Field(...))
);

This is what i made to make it fast work for our project. In fact though if i would create a PR on ezpublish-kernel I would suggest a different method, that is cleaner in my opinion. I split that in a separated comment since this is not an easy topic.

@RozbehSharahi
Copy link
Author

Plan of attack

Constraints:

  • The way this is implemented should not break the idea of having an agnostic/generic database access layer that can be connected to SQL or Solr
  • It should be easy to use and document.

The idea of tagging is only interesting to bind a facet to a filter.

In order to keep this in sync that facet should actually contain a corresponding filter/criterion.

Code wise this means:

  1. add a new property to $constraint to FacetBuilder:
abstract class FacetBuilder {
  ...
  /**
   * This is the right Criterion we want to use in order to handle the facet entry
   * @var Criterion
   */
  $criterion = null;
  
  /**
   * This is basically which filters are selected on this facetBuilder
   * @var array
   */
  $values = [];
}

So in order to use facetBuilders we do now following:

$query->facetBuilders= new FacetBuilder\CustomField([
  // The criterion to use
  "criterion" => new Criterion\CustomField('bla', 'eq', 'This value will be overriden by FacetBuilder values'),

  // The values that were selected in the UI
  "values" => [
    0 => 'Robert',
    1 => 'Peter'
  ],

  // whatever else belongs to this facet builder type
  ...
]);

This way the facetBuilder is directly related to it's corresponding criterion and we can build stuff like:

$query->facetBuilders= new FacetBuilder\CustomField([
  // The criterion to use
  "criterion" => new Criterion\FieldLike(...),
  // The values that were selected in the UI
  "values" => [
    0 => 'a',
    1 => 'b'
  ],
  // whatever else belongs to this facet builder type
  ...
]);

Or range facets, ...

In my opinion this is very extendible. Will not need many changes on ez core classes. The user/developer doesn't even need to define a tag since as you mentioned we could use spl_object_hash but since we are in the scope of a facetBuilder we can simply use the name, which is much easier to debug when debugging the solr queries.

The good thing is: FacetBuilders are AFAIK not available on EZ-Legacy-SQL-Driver which would keep this still agnostic. In fact it would even be possible to implement SQL facets as well and the pattern should be perfectly fine for that.

Action Items:

  • Add properties $criterion and $values to FacetBuilder on ezpublish/kernel
  • Adapt Native Query Builder to add the facetBuilder queries if "values" are not empty.
  • Add FacetBuilder\Field.php to ezpublish-kernel
  • Update documentation of facet builder

@RozbehSharahi
Copy link
Author

Ok i just saw that there is even already a $filter on FacetBuilder :) Maybe this can be achieved as well without adding the $values property actually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants