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

TIP-1145: Add parent filter #10205

Merged
merged 2 commits into from Jun 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions CHANGELOG-3.2.md
Expand Up @@ -2,11 +2,11 @@

## Bug fixes

- PIM-8270: Update export jobs after a change on a channel category


- PIM-8270: Update export jobs after a change on a channel category

## BC Breaks

- Rename `Akeneo\Pim\Enrichment\Bundle\Elasticsearch\Filter\Field\AncestorFilter` to `Akeneo\Pim\Enrichment\Bundle\Elasticsearch\Filter\Field\AncestorIdFilter`



Expand Down
2 changes: 1 addition & 1 deletion UPGRADE.md
Expand Up @@ -176,7 +176,7 @@ Before updating the dependencies and migrating your data, please deactivate all
Several classes and services have been moved or renamed. The following commands help to migrate references to them:

```bash
TODO: ADD THE SEDs
find ./src/ -type f -print0 | xargs -0 sed -i 's/Akeneo\\Pim\\Enrichment\\Bundle\\Elasticsearch\\Filter\\Field\\AncestorFilter/Akeneo\\Pim\\Enrichment\\Bundle\\Elasticsearch\\Filter\\Field\\AncestorIdFilter/g'
```

2. Adapt your custom codes to handle this breaking changes we introduced:
Expand Down
@@ -0,0 +1,62 @@
<?php

declare(strict_types=1);

namespace Akeneo\Pim\Enrichment\Bundle\Elasticsearch\Filter\Field;

use Akeneo\Pim\Enrichment\Component\Product\Query\Filter\FieldFilterHelper;
use Akeneo\Pim\Enrichment\Component\Product\Repository\ProductModelRepositoryInterface;
use Akeneo\Tool\Component\StorageUtils\Repository\IdentifiableObjectRepositoryInterface;

/**
* This class filters data with the ancestor code, i.e. the parent code. We cannot use ParentFilter as it this
* field is unfortunately not in product index. It is only in product and product model index.
* This is temporary and is used into external API product list, and will be removed after TIP-1150.
*
* @see src/Akeneo/Pim/Enrichment/Component/Product/Connector/UseCase/ApplyProductSearchQueryParametersToPQB.php
*
* @author Pierre Allard <pierre.allard@akeneo.com>
* @copyright 2019 Akeneo SAS (http://www.akeneo.com)
* @license http://opensource.org/licenses/osl-3.0.php Open Software License (OSL 3.0)
*/
class AncestorCodeFilter extends AbstractFieldFilter
{
private const ANCESTOR_CODES_ES_FIELD = 'ancestors.codes';

/** @var IdentifiableObjectRepositoryInterface */
private $productModelRepository;

/**
* @param ProductModelRepositoryInterface $productModelRepository
* @param array $supportedFields
* @param array $supportedOperators
*/
public function __construct(
ProductModelRepositoryInterface $productModelRepository,
array $supportedFields,
array $supportedOperators
) {
$this->productModelRepository = $productModelRepository;
$this->supportedFields = $supportedFields;
$this->supportedOperators = $supportedOperators;
}

/**
* {@inheritdoc}
*/
public function addFieldFilter($field, $operator, $value, $locale = null, $channel = null, $options = []): void
{
if (null === $this->searchQueryBuilder) {
throw new \LogicException('The search query builder is not initialized in the filter.');
}

FieldFilterHelper::checkIdentifier($field, $value, static::class);
$clause = [
'terms' => [
self::ANCESTOR_CODES_ES_FIELD => [$value],
],
];

$this->searchQueryBuilder->addFilter($clause);
}
}
Expand Up @@ -31,7 +31,7 @@
* @copyright 2017 Akeneo SAS (http://www.akeneo.com)
* @license http://opensource.org/licenses/osl-3.0.php Open Software License (OSL 3.0)
*/
class AncestorFilter extends AbstractFieldFilter
class AncestorIdFilter extends AbstractFieldFilter
{
private const ANCESTOR_ID_ES_FIELD = 'ancestors.ids';

Expand Down Expand Up @@ -63,7 +63,7 @@ public function addFieldFilter($field, $operator, $values, $locale = null, $chan
}

if (!$this->supportsOperator($operator)) {
throw InvalidOperatorException::notSupported($operator, AncestorFilter::class);
throw InvalidOperatorException::notSupported($operator, AncestorIdFilter::class);
}

$this->checkValues($values);
Expand Down
Expand Up @@ -8,6 +8,7 @@
use Akeneo\Pim\Enrichment\Component\Product\Query\Filter\FieldFilterInterface;
use Akeneo\Pim\Enrichment\Component\Product\Query\Filter\Operators;
use Akeneo\Pim\Enrichment\Component\Product\Repository\ProductModelRepositoryInterface;
use Akeneo\Tool\Component\StorageUtils\Exception\InvalidPropertyTypeException;

/**
* Parent filter for an Elasticsearch query
Expand Down Expand Up @@ -38,19 +39,18 @@ public function __construct(

/**
* {@inheritdoc}
*
* @throws ObjectNotFoundException
*/
public function addFieldFilter($field, $operator, $value, $locale = null, $channel = null, $options = [])
{
if (null === $this->searchQueryBuilder) {
throw new \LogicException('The search query builder is not initialized in the filter.');
}

if ($operator === Operators::IN_LIST) {
$this->checkValue($field, $value);
}

switch ($operator) {
case Operators::IN_LIST:
$this->checkValue($field, $value);
$clause = [
'terms' => [
$field => $value,
Expand Down Expand Up @@ -90,6 +90,7 @@ public function addFieldFilter($field, $operator, $value, $locale = null, $chann
* @param mixed $values
*
* @throws ObjectNotFoundException
* @throws InvalidPropertyTypeException
*/
protected function checkValue($field, $values)
{
Expand Down
Expand Up @@ -38,7 +38,7 @@ parameters:
pim_catalog.query.elasticsearch.filter.metric.class: Akeneo\Pim\Enrichment\Bundle\Elasticsearch\Filter\Attribute\MetricFilter
pim_catalog.query.elasticsearch.filter.media.class: Akeneo\Pim\Enrichment\Bundle\Elasticsearch\Filter\Attribute\MediaFilter
pim_catalog.query.elasticsearch.filter.boolean.class: Akeneo\Pim\Enrichment\Bundle\Elasticsearch\Filter\Attribute\BooleanFilter
pim_catalog.query.elasticsearch.filter.ancestor.class: Akeneo\Pim\Enrichment\Bundle\Elasticsearch\Filter\Field\AncestorFilter
pim_catalog.query.elasticsearch.filter.ancestor_id.class: Akeneo\Pim\Enrichment\Bundle\Elasticsearch\Filter\Field\AncestorIdFilter
pim_catalog.query.elasticsearch.filter.self_and_ancestor.class: Akeneo\Pim\Enrichment\Bundle\Elasticsearch\Filter\Field\SelfAndAncestorFilter

pim_catalog.query.elasticsearch.sorter.base_field.class: Akeneo\Pim\Enrichment\Bundle\Elasticsearch\Sorter\Field\BaseFieldSorter
Expand Down Expand Up @@ -423,14 +423,23 @@ services:
- { name: 'pim_catalog.elasticsearch.query.product_and_product_model_filter', priority: 30 }

pim_catalog.query.elasticsearch.filter.ancestor:
class: '%pim_catalog.query.elasticsearch.filter.ancestor.class%'
class: '%pim_catalog.query.elasticsearch.filter.ancestor_id.class%'
arguments:
- '@pim_catalog.repository.product_model'
- ['ancestor.id']
- ['IN', 'NOT IN']
tags:
- { name: 'pim_catalog.elasticsearch.query.product_and_product_model_filter', priority: 30 }

pim_catalog.query.elasticsearch.filter.ancestor_code:
class: 'Akeneo\Pim\Enrichment\Bundle\Elasticsearch\Filter\Field\AncestorCodeFilter'
arguments:
- '@pim_catalog.repository.product_model'
- ['ancestor.code']
- ['=']
tags:
- { name: 'pim_catalog.elasticsearch.query.product_filter', priority: 30 }

pim_catalog.query.elasticsearch.filter.self_and_ancestor:
class: '%pim_catalog.query.elasticsearch.filter.self_and_ancestor.class%'
arguments:
Expand Down
Expand Up @@ -85,7 +85,19 @@ public function apply(
}
}

$pqb->addFilter($propertyCode, $filter['operator'], $value, $context);
/**
* Today, external API uses the "product_index" to search the products. This index indexes parent data
* with help of 'ancestors.codes' and 'ancestors.ids'.
* To avoid a big refactoring (i.e. use the product_and_product_model_index, TIP-1150), we consider to change the
* parent filter to a dedicated one `AncestorCodeFilter`.
*
* @see src/Akeneo/Pim/Enrichment/Bundle/Elasticsearch/Filter/Field/AncestorCodeFilter.php
*/
if ($propertyCode === 'parent') {
$pqb->addfilter('ancestor.code', $filter['operator'], $value, $context);
} else {
$pqb->addFilter($propertyCode, $filter['operator'], $value, $context);
}
}
}
}
Expand Down
Expand Up @@ -4,6 +4,7 @@

namespace Akeneo\Pim\Enrichment\Component\Product\Connector\UseCase\Validator;

use Akeneo\Pim\Enrichment\Component\Product\Query\Filter\Operators;
use Akeneo\Tool\Component\Api\Exception\InvalidQueryException;
use Akeneo\Tool\Component\StorageUtils\Repository\IdentifiableObjectRepositoryInterface;

Expand All @@ -25,6 +26,7 @@ final class ValidateProperties
'updated',
'enabled',
'groups',
'parent',
];

/** @var IdentifiableObjectRepositoryInterface */
Expand All @@ -43,10 +45,11 @@ public function validate(array $search): void
{
foreach ($search as $propertyCode => $filters) {
foreach ($filters as $filter) {
if (
!in_array($propertyCode, self::$productFields) &&
null === $this->attributeRepository->findOneByIdentifier($propertyCode)
) {
if (!(
pierallard marked this conversation as resolved.
Show resolved Hide resolved
$this->isProductField($propertyCode) ||
$this->isExistingAttribute($propertyCode) ||
$this->hasWrongOperatorForParentProperty($propertyCode, $filter['operator'])
)) {
throw new InvalidQueryException(
sprintf(
'Filter on property "%s" is not supported or does not support operator "%s"',
Expand All @@ -58,4 +61,19 @@ public function validate(array $search): void
}
}
}

private function isProductField(string $propertyCode): bool
{
return in_array($propertyCode, self::$productFields);
}

private function isExistingAttribute(string $propertyCode): bool
{
return null !== $this->attributeRepository->findOneByIdentifier($propertyCode);
}

private function hasWrongOperatorForParentProperty(string $propertyCode, string $operator): bool
{
return ($propertyCode === 'parent' && $operator !== Operators::EQUALS);
}
}