From 26295386d896d01937a4ec21436581aac86a3e9a Mon Sep 17 00:00:00 2001 From: thomascorthals Date: Tue, 13 Jun 2023 09:45:42 +0200 Subject: [PATCH] Set excludes and terms fluently on facet fields (#1070) * Set excludes and terms fluently on facet fields * Additional test coverage --------- Co-authored-by: Markus Kalkbrenner --- CHANGELOG.md | 4 + .../facetset-component/facet-field.md | 7 +- .../facetset-component/facetset-component.md | 8 +- examples/2.1.5.1.1.1-facet-field-filters.php | 13 ++- examples/2.1.5.1.1.2-facet-field-excludes.php | 2 +- examples/2.1.5.1.4.3-facet-range-excludes.php | 2 +- src/Component/Facet/AbstractFacet.php | 86 +++++++++++++++- src/Component/Facet/AbstractRange.php | 10 +- src/Component/Facet/FacetInterface.php | 56 ++++++++++- src/Component/Facet/Field.php | 84 ++++++++++++++++ .../Facet/FieldValueParametersInterface.php | 8 +- .../Facet/FieldValueParametersTrait.php | 16 +-- src/Component/Facet/JsonFacetTrait.php | 36 ++++--- src/Component/Facet/MultiQuery.php | 98 ++++++++++++++++--- src/Component/Facet/Pivot.php | 12 ++- src/Component/Facet/Range.php | 8 ++ src/Component/FacetSetInterface.php | 20 ++-- src/Component/RequestBuilder/FacetSet.php | 4 +- src/Core/Query/AbstractRequestBuilder.php | 9 +- src/Core/Query/Helper.php | 16 ++- .../Query/LocalParameters/LocalParameter.php | 26 +++++ .../Query/LocalParameters/LocalParameters.php | 16 ++- .../LocalParameters/LocalParametersTrait.php | 10 +- tests/Component/Facet/FacetTest.php | 48 +++++++-- tests/Component/Facet/FieldTest.php | 64 +++++++++++- tests/Component/Facet/MultiQueryTest.php | 96 +++++++++++++++++- tests/Component/Facet/RangeTest.php | 29 +++++- .../Component/RequestBuilder/FacetSetTest.php | 94 +++++++++++++++++- tests/Core/Query/HelperTest.php | 28 ++++++ .../LocalParameters/LocalParameterTest.php | 6 ++ .../LocalParameters/LocalParametersTest.php | 93 +++++++++++++++--- tests/Core/Query/RequestBuilderTest.php | 16 +++ 32 files changed, 914 insertions(+), 111 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cef1e203d..acbb038e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,10 +11,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Solarium\Core\Query\Result\QueryType::getStatus() and getQueryTime(), inherited by all Solarium\QueryType Results - Solarium\QueryType\CoreAdmin\Result\Result::getInitFailureResults() - Solarium\QueryType\Ping\Result::getPingStatus() and getZkConnected() +- Fluent interface methods for adding/removing excludes in Solarium\Component\Facet\AbstractFacet +- Fluent interface methods for adding/removing terms in Solarium\Component\Facet\Field ### Fixed - JSON serialization of arrays with non-consecutive indices in multivalue fields - PHP 8.2 deprecations +- Handling of escaped literal commas in local parameters for faceting ### Changed - Update queries use the JSON request format by default @@ -27,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Solarium\QueryType\Server\Collections\Result\CreateResult::getStatus(), use getCreateStatus() instead - Solarium\QueryType\Server\Collections\Result\DeleteResult::getStatus(), use getDeleteStatus() instead - Solarium\QueryType\Server\Collections\Result\ReloadResult::getStatus(), use getReloadStatus() instead +- LocalParameters::removeTerms(), use removeTerm() instead ## [6.2.8] diff --git a/docs/queries/select-query/building-a-select-query/components/facetset-component/facet-field.md b/docs/queries/select-query/building-a-select-query/components/facetset-component/facet-field.md index df17a5895..baefe24d2 100644 --- a/docs/queries/select-query/building-a-select-query/components/facetset-component/facet-field.md +++ b/docs/queries/select-query/building-a-select-query/components/facetset-component/facet-field.md @@ -7,9 +7,10 @@ The options below can be set as query option values, but also by using the set/g Only the facet-type specific options are listed. See [FacetSet component](facetset-component.md) for the options shared by all facet types. -| Name | Type | Default value | Description | -|-------|--------|---------------|--------------------------------| -| field | string | id | The index field for the facet. | +| Name | Type | Default value | Description | +|-------------|--------|---------------|-----------------------------------------------------------------------------------------------------| +| field | string | id | The index field for the facet. | +| local_terms | string | null | Limit field facet to specified terms. Specify a comma separated list. Use `\,` for a literal comma. | || Example diff --git a/docs/queries/select-query/building-a-select-query/components/facetset-component/facetset-component.md b/docs/queries/select-query/building-a-select-query/components/facetset-component/facetset-component.md index 9e9ac8fa6..14592a339 100644 --- a/docs/queries/select-query/building-a-select-query/components/facetset-component/facetset-component.md +++ b/docs/queries/select-query/building-a-select-query/components/facetset-component/facetset-component.md @@ -32,10 +32,10 @@ Standard facet options All facet types available in the facetset extend a base class that offers a standard set of options. The following options are available for ALL facet types: -| Name | Type | Default value | Description | -|----------|--------|---------------|-------------------------------------------------------------| -| key | string | null | Key to identify the facet (mandatory) | -| excludes | string | null | Add one or multiple filterquery tags to exclude for a facet | +| Name | Type | Default value | Description | +|---------------|--------|---------------|--------------------------------------------------------------| +| local_key | string | null | Key to identify the facet (mandatory). | +| local_exclude | string | null | Add one or multiple filterquery tags to exclude for a facet. | || Pivot facet options diff --git a/examples/2.1.5.1.1.1-facet-field-filters.php b/examples/2.1.5.1.1.1-facet-field-filters.php index 01ac8ce62..61f43c8b5 100644 --- a/examples/2.1.5.1.1.1-facet-field-filters.php +++ b/examples/2.1.5.1.1.1-facet-field-filters.php @@ -29,12 +29,16 @@ $facetSet->createFacetField('electronicsAndMore')->setField('cat')->setMatches('electronics.+'); // setExcludeTerms takes a comma separated list of terms to exclude from the facet -// escape the comma for a literal match e.g. 'yes\, this facet with be excluded' +// escape the comma for a literal match e.g. 'yes\, this term will be excluded' $facetSet->createFacetField('electronicsExclude')->setField('cat')->setExcludeTerms('electronics,music'); // all three restriction types can also be used on the facetset as a whole and will affect all (non json) facets // e.g. $facetSet->setExcludeTerms('search'); +// setTerms takes a comma separated list of terms to exclude from the facet +// escape the comma for a literal match e.g. 'yes\, this term will be included' +$facetSet->createFacetField('electronicsTerms')->setField('cat')->setTerms('electronics,music'); + // this executes the query and returns the result $resultset = $client->select($query); @@ -76,4 +80,11 @@ echo $value . ' [' . $count . ']
'; } +// display facet counts +echo '
Facet counts for field "cat"; terms limited to "electronics" and "music":
'; +$facet = $resultset->getFacetSet()->getFacet('electronicsTerms'); +foreach ($facet as $value => $count) { + echo $value . ' [' . $count . ']
'; +} + htmlFooter(); diff --git a/examples/2.1.5.1.1.2-facet-field-excludes.php b/examples/2.1.5.1.1.2-facet-field-excludes.php index 20085117c..02d61914f 100644 --- a/examples/2.1.5.1.1.2-facet-field-excludes.php +++ b/examples/2.1.5.1.1.2-facet-field-excludes.php @@ -22,7 +22,7 @@ $facetSet->createFacetField('category')->setField('cat'); // addExcludes will exclude filters by tag -$facetSet->createFacetField('unfiltered')->setField('cat')->getLocalParameters()->addExcludes(['electronics']); +$facetSet->createFacetField('unfiltered')->setField('cat')->addExcludes(['electronics']); // this executes the query and returns the result $resultset = $client->select($query); diff --git a/examples/2.1.5.1.4.3-facet-range-excludes.php b/examples/2.1.5.1.4.3-facet-range-excludes.php index f587a0e62..fe36ea8b2 100644 --- a/examples/2.1.5.1.4.3-facet-range-excludes.php +++ b/examples/2.1.5.1.4.3-facet-range-excludes.php @@ -32,7 +32,7 @@ $facet->setStart(1); $facet->setGap(100); $facet->setEnd(1000); -$facet->getLocalParameters()->addExcludes(['budget']); +$facet->addExcludes(['budget']); // this executes the query and returns the result $resultset = $client->select($query); diff --git a/src/Component/Facet/AbstractFacet.php b/src/Component/Facet/AbstractFacet.php index 639652458..86f472207 100644 --- a/src/Component/Facet/AbstractFacet.php +++ b/src/Component/Facet/AbstractFacet.php @@ -47,10 +47,94 @@ public function getKey(): ?string * * @return self Provides fluent interface */ - public function setKey(string $key): FacetInterface + public function setKey(string $key): self { $this->getLocalParameters()->setKey($key); return $this; } + + /** + * Add an exclude tag. + * + * @param string $exclude + * + * @return self Provides fluent interface + */ + public function addExclude(string $exclude) + { + $this->getLocalParameters()->setExclude($exclude); + + return $this; + } + + /** + * Add multiple exclude tags. + * + * @param array|string $excludes array or string with comma separated exclude tags + * + * @return self Provides fluent interface + */ + public function addExcludes($excludes) + { + if (\is_string($excludes)) { + $excludes = preg_split('/(?getLocalParameters()->addExcludes($excludes); + + return $this; + } + + /** + * Set the list of exclude tags. + * + * This overwrites any existing exclude tags. + * + * @param array|string $excludes + * + * @return self Provides fluent interface + */ + public function setExcludes($excludes) + { + $this->clearExcludes()->addExcludes($excludes); + + return $this; + } + + /** + * Remove a single exclude tag. + * + * @param string $exclude + * + * @return self Provides fluent interface + */ + public function removeExclude(string $exclude) + { + $this->getLocalParameters()->removeExclude($exclude); + + return $this; + } + + /** + * Remove all exclude tags. + * + * @return self Provides fluent interface + */ + public function clearExcludes() + { + $this->getLocalParameters()->clearExcludes(); + + return $this; + } + + /** + * Get the list of exclude tags. + * + * @return array + */ + public function getExcludes(): array + { + return $this->getLocalParameters()->getExcludes(); + } } diff --git a/src/Component/Facet/AbstractRange.php b/src/Component/Facet/AbstractRange.php index 508dd8702..e5b18a8e7 100644 --- a/src/Component/Facet/AbstractRange.php +++ b/src/Component/Facet/AbstractRange.php @@ -9,8 +9,6 @@ namespace Solarium\Component\Facet; -use Solarium\Core\Configurable; - /** * Facet range. * @@ -272,11 +270,13 @@ public function getInclude(): array /** * @param \Solarium\Component\Facet\Pivot|array $pivot * - * @return \Solarium\Core\Configurable + * @return self Provides fluent interface */ - public function setPivot($pivot): Configurable + public function setPivot($pivot): self { - return $this->setOption('pivot', $pivot); + $this->setOption('pivot', $pivot); + + return $this; } /** diff --git a/src/Component/Facet/FacetInterface.php b/src/Component/Facet/FacetInterface.php index f8cb0e964..b4dd14963 100644 --- a/src/Component/Facet/FacetInterface.php +++ b/src/Component/Facet/FacetInterface.php @@ -37,7 +37,59 @@ public function getKey(): ?string; * * @param string $key * - * @return self + * @return self Provides fluent interface */ - public function setKey(string $key): self; + public function setKey(string $key); + + /** + * Add an exclude tag. + * + * @param string $exclude + * + * @return self Provides fluent interface + */ + public function addExclude(string $exclude); + + /** + * Add multiple exclude tags. + * + * @param array|string $excludes array or string with comma separated exclude tags + * + * @return self Provides fluent interface + */ + public function addExcludes($excludes); + + /** + * Set the list of exclude tags. + * + * This overwrites any existing exclude tags. + * + * @param array|string $excludes + * + * @return self Provides fluent interface + */ + public function setExcludes($excludes); + + /** + * Remove a single exclude tag. + * + * @param string $exclude + * + * @return self Provides fluent interface + */ + public function removeExclude(string $exclude); + + /** + * Remove all exclude tags. + * + * @return self Provides fluent interface + */ + public function clearExcludes(); + + /** + * Get the list of exclude tags. + * + * @return array + */ + public function getExcludes(); } diff --git a/src/Component/Facet/Field.php b/src/Component/Facet/Field.php index 9f2df10d2..8a1928530 100644 --- a/src/Component/Facet/Field.php +++ b/src/Component/Facet/Field.php @@ -62,4 +62,88 @@ public function getField(): ?string { return $this->getOption('field'); } + + /** + * Add a term. + * + * @param string $term + * + * @return self Provides fluent interface + */ + public function addTerm(string $term): self + { + $this->getLocalParameters()->setTerm($term); + + return $this; + } + + /** + * Add multiple terms. + * + * @param array|string $terms array or string with comma separated terms + * + * @return self Provides fluent interface + */ + public function addTerms($terms): self + { + if (\is_string($terms)) { + $terms = preg_split('/(?getLocalParameters()->addTerms($terms); + + return $this; + } + + /** + * Set the list of terms. + * + * This overwrites any existing terms. + * + * @param array|string $terms + * + * @return self Provides fluent interface + */ + public function setTerms($terms): self + { + $this->clearTerms()->addTerms($terms); + + return $this; + } + + /** + * Remove a single term. + * + * @param string $term + * + * @return self Provides fluent interface + */ + public function removeTerm(string $term): self + { + $this->getLocalParameters()->removeTerm($term); + + return $this; + } + + /** + * Remove all terms. + * + * @return self Provides fluent interface + */ + public function clearTerms(): self + { + $this->getLocalParameters()->clearTerms(); + + return $this; + } + + /** + * Get the list of terms. + * + * @return array + */ + public function getTerms(): array + { + return $this->getLocalParameters()->getTerms(); + } } diff --git a/src/Component/Facet/FieldValueParametersInterface.php b/src/Component/Facet/FieldValueParametersInterface.php index 3b0ebc260..4687f9a39 100644 --- a/src/Component/Facet/FieldValueParametersInterface.php +++ b/src/Component/Facet/FieldValueParametersInterface.php @@ -217,7 +217,7 @@ public function getMethod(): ?string; * * @return self Provides fluent interface */ - public function setEnumCacheMinimumDocumentFrequency($frequency); + public function setEnumCacheMinimumDocumentFrequency(int $frequency); /** * Get the minimum document frequency for which the filterCache should be used. @@ -229,7 +229,7 @@ public function getEnumCacheMinimumDocumentFrequency(): ?int; /** * Set to true to cap facet counts by 1. * - * @param int $exists + * @param bool $exists * * @return self Provides fluent interface */ @@ -267,7 +267,7 @@ public function getExcludeTerms(): ?string; * * @return self Provides fluent interface */ - public function setOverrequestCount($count); + public function setOverrequestCount(int $count); /** * Get the facet overrequest count. @@ -283,7 +283,7 @@ public function getOverrequestCount(): ?int; * * @return self Provides fluent interface */ - public function setOverrequestRatio($ratio); + public function setOverrequestRatio(float $ratio); /** * Get the facet overrequest ratio. diff --git a/src/Component/Facet/FieldValueParametersTrait.php b/src/Component/Facet/FieldValueParametersTrait.php index d270e105b..9ac04b167 100644 --- a/src/Component/Facet/FieldValueParametersTrait.php +++ b/src/Component/Facet/FieldValueParametersTrait.php @@ -267,7 +267,7 @@ public function getMethod(): ?string * * @return self Provides fluent interface */ - public function setEnumCacheMinimumDocumentFrequency($frequency): self + public function setEnumCacheMinimumDocumentFrequency(int $frequency): self { $this->setOption('enum.cache.minDf', $frequency); @@ -287,7 +287,7 @@ public function getEnumCacheMinimumDocumentFrequency(): ?int /** * Set to true to cap facet counts by 1. * - * @param int $exists + * @param bool $exists * * @return self Provides fluent interface */ @@ -341,9 +341,11 @@ public function getExcludeTerms(): ?string * * @return self Provides fluent interface */ - public function setOverrequestCount($count): self + public function setOverrequestCount(int $count): self { - return $this->setOption('overrequest.count', $count); + $this->setOption('overrequest.count', $count); + + return $this; } /** @@ -363,9 +365,11 @@ public function getOverrequestCount(): ?int * * @return self Provides fluent interface */ - public function setOverrequestRatio($ratio): self + public function setOverrequestRatio(float $ratio): self { - return $this->setOption('overrequest.ratio', $ratio); + $this->setOption('overrequest.ratio', $ratio); + + return $this; } /** diff --git a/src/Component/Facet/JsonFacetTrait.php b/src/Component/Facet/JsonFacetTrait.php index a594cc063..f7083ec2b 100644 --- a/src/Component/Facet/JsonFacetTrait.php +++ b/src/Component/Facet/JsonFacetTrait.php @@ -59,9 +59,9 @@ public function getDomainFilter() * @param string $query * @param array $bind Bind values for placeholders in the query string * - * @return \Solarium\Component\FacetSetInterface + * @return self Provides fluent interface */ - public function setDomainFilterQuery(string $query, array $bind = null): FacetSetInterface + public function setDomainFilterQuery(string $query, array $bind = null): self { if (null !== $bind) { $helper = new Helper(); @@ -70,14 +70,18 @@ public function setDomainFilterQuery(string $query, array $bind = null): FacetSe $filter = $this->getDomainFilter(); if (!$filter || \is_string($filter)) { - return $this->setOption('domain', ['filter' => $query]); + $this->setOption('domain', ['filter' => $query]); + + return $this; } foreach ($filter as &$paramOrQuery) { if (\is_string($paramOrQuery)) { $paramOrQuery = $query; - return $this->setOption('domain', ['filter' => $filter]); + $this->setOption('domain', ['filter' => $filter]); + + return $this; } } unset($paramOrQuery); @@ -85,7 +89,9 @@ public function setDomainFilterQuery(string $query, array $bind = null): FacetSe /* @noinspection UnsupportedStringOffsetOperationsInspection */ $filter[] = $query; - return $this->setOption('domain', ['filter' => $filter]); + $this->setOption('domain', ['filter' => $filter]); + + return $this; } /** @@ -95,15 +101,19 @@ public function setDomainFilterQuery(string $query, array $bind = null): FacetSe * * @return self Provides fluent interface */ - public function addDomainFilterParameter(string $param): FacetSetInterface + public function addDomainFilterParameter(string $param): self { $filter = $this->getDomainFilter(); if (!$filter) { - return $this->setOption('domain', ['filter' => ['param' => $param]]); + $this->setOption('domain', ['filter' => ['param' => $param]]); + + return $this; } if (\is_string($filter) || 1 === \count($filter)) { - return $this->setOption('domain', ['filter' => [$filter, ['param' => $param]]]); + $this->setOption('domain', ['filter' => [$filter, ['param' => $param]]]); + + return $this; } foreach ($filter as &$paramOrQuery) { @@ -116,7 +126,9 @@ public function addDomainFilterParameter(string $param): FacetSetInterface /* @noinspection UnsupportedStringOffsetOperationsInspection */ $filter[] = ['param' => $param]; - return $this->setOption('domain', ['filter' => $filter]); + $this->setOption('domain', ['filter' => $filter]); + + return $this; } /** @@ -155,7 +167,7 @@ public function serialize() * * @return self Provides fluent interface */ - public function addFacet($facet): FacetSetInterface + public function addFacet($facet): self { if ($facet instanceof JsonFacetInterface) { $this->facetSetAddFacet($facet); @@ -176,7 +188,7 @@ public function addFacet($facet): FacetSetInterface * * @return self Provides fluent interface */ - public function removeFacet($facet): FacetSetInterface + public function removeFacet($facet): self { $this->facetSetRemoveFacet($facet); $this->serialize(); @@ -189,7 +201,7 @@ public function removeFacet($facet): FacetSetInterface * * @return self Provides fluent interface */ - public function clearFacets(): FacetSetInterface + public function clearFacets(): self { $this->facetSetClearFacets(); $this->serialize(); diff --git a/src/Component/Facet/MultiQuery.php b/src/Component/Facet/MultiQuery.php index 2ad3667e2..f333a2fb3 100644 --- a/src/Component/Facet/MultiQuery.php +++ b/src/Component/Facet/MultiQuery.php @@ -203,7 +203,7 @@ public function setQueries(array $facetQueries): self /** * Add an exclude tag. * - * Excludes added to the MultiQuery facet a shared by all underlying + * Excludes added to the MultiQuery facet are shared by all underlying * FacetQueries, so they must be forwarded to any existing instances. * * If you don't want to share an exclude use the addExclude method of a @@ -211,14 +211,12 @@ public function setQueries(array $facetQueries): self * * @param string $exclude * - * @throws OutOfBoundsException - * * @return self Provides fluent interface */ - public function addExclude(string $exclude): AbstractFacet + public function addExclude(string $exclude): self { foreach ($this->facetQueries as $facetQuery) { - $facetQuery->getLocalParameters()->setExclude($exclude); + $facetQuery->addExclude($exclude); } $this->getLocalParameters()->setExclude($exclude); @@ -226,10 +224,66 @@ public function addExclude(string $exclude): AbstractFacet return $this; } + /** + * Add exclude tags. + * + * Excludes added to the MultiQuery facet are shared by all underlying + * FacetQueries, so they must be forwarded to any existing instances. + * + * If you don't want to share excludes use the addExcludes method of a + * specific FacetQuery instance instead. + * + * @param array|string $excludes array or string with comma separated exclude tags + * + * @return self Provides fluent interface + */ + public function addExcludes($excludes): self + { + if (\is_string($excludes)) { + $excludes = preg_split('/(?facetQueries as $facetQuery) { + $facetQuery->addExcludes($excludes); + } + + $this->getLocalParameters()->addExcludes($excludes); + + return $this; + } + + /** + * Set the list of exclude tags. + * + * Excludes added to the MultiQuery facet are shared by all underlying + * FacetQueries, so they must be forwarded to any existing instances. + * + * If you don't want to share excludes use the setExcludes method of a + * specific FacetQuery instance instead. + * + * @param array|string $excludes array or string with comma separated exclude tags + * + * @return self Provides fluent interface + */ + public function setExcludes($excludes): self + { + if (\is_string($excludes)) { + $excludes = preg_split('/(?facetQueries as $facetQuery) { + $facetQuery->setExcludes($excludes); + } + + $this->getLocalParameters()->setExcludes($excludes); + + return $this; + } + /** * Remove a single exclude tag. * - * Excludes added to the MultiQuery facet a shared by all underlying + * Excludes added to the MultiQuery facet are shared by all underlying * FacetQueries, so changes must be forwarded to any existing instances. * * If you don't want this use the removeExclude method of a @@ -237,14 +291,12 @@ public function addExclude(string $exclude): AbstractFacet * * @param string $exclude * - * @throws OutOfBoundsException - * * @return self Provides fluent interface */ - public function removeExclude(string $exclude): AbstractFacet + public function removeExclude(string $exclude): self { foreach ($this->facetQueries as $facetQuery) { - $facetQuery->getLocalParameters()->removeExclude($exclude); + $facetQuery->removeExclude($exclude); } $this->getLocalParameters()->removeExclude($exclude); @@ -253,22 +305,20 @@ public function removeExclude(string $exclude): AbstractFacet } /** - * Remove all excludes. + * Remove all exclude tags. * - * Excludes added to the MultiQuery facet a shared by all underlying + * Excludes added to the MultiQuery facet are shared by all underlying * FacetQueries, so changes must be forwarded to any existing instances. * * If you don't want this use the clearExcludes method of a * specific FacetQuery instance instead. * - * @throws OutOfBoundsException - * * @return self Provides fluent interface */ - public function clearExcludes(): AbstractFacet + public function clearExcludes(): self { foreach ($this->facetQueries as $facetQuery) { - $facetQuery->getLocalParameters()->clearExcludes(); + $facetQuery->clearExcludes(); } $this->getLocalParameters()->clearExcludes(); @@ -276,6 +326,22 @@ public function clearExcludes(): AbstractFacet return $this; } + /** + * Get the list of exclude tags. + * + * Excludes added to the MultiQuery facet are shared by all underlying + * FacetQueries, so they must be forwarded to any existing instances. + * + * If you don't want to share excludes use the getExcludes method of a + * specific FacetQuery instance instead. + * + * @return array + */ + public function getExcludes(): array + { + return $this->getLocalParameters()->getExcludes(); + } + /** * Initialize options. * diff --git a/src/Component/Facet/Pivot.php b/src/Component/Facet/Pivot.php index f0d1aa682..8773ccd27 100644 --- a/src/Component/Facet/Pivot.php +++ b/src/Component/Facet/Pivot.php @@ -158,9 +158,11 @@ public function getSort(): ?string * * @return self Provides fluent interface */ - public function setOverrequestCount($count): self + public function setOverrequestCount(int $count): self { - return $this->setOption('overrequest.count', $count); + $this->setOption('overrequest.count', $count); + + return $this; } /** @@ -180,9 +182,11 @@ public function getOverrequestCount(): ?int * * @return self Provides fluent interface */ - public function setOverrequestRatio($ratio): self + public function setOverrequestRatio(float $ratio): self { - return $this->setOption('overrequest.ratio', $ratio); + $this->setOption('overrequest.ratio', $ratio); + + return $this; } /** diff --git a/src/Component/Facet/Range.php b/src/Component/Facet/Range.php index 9ba8566e9..2cb11092d 100644 --- a/src/Component/Facet/Range.php +++ b/src/Component/Facet/Range.php @@ -65,10 +65,18 @@ protected function init() foreach ($this->options as $name => $value) { switch ($name) { case 'exclude': + if (!\is_array($value)) { + $value = preg_split('/(?getLocalParameters()->addExcludes($value); + unset($this->options[$name]); + + trigger_error('Setting local parameter using the "exclude" option is deprecated in Solarium 7 and will be removed in Solarium 8. Use "local_exclude" instead.', \E_USER_DEPRECATED); break; case 'pivot': $this->setPivot(new Pivot($value)); + break; } } } diff --git a/src/Component/FacetSetInterface.php b/src/Component/FacetSetInterface.php index 72ba39934..ac3eba5fc 100644 --- a/src/Component/FacetSetInterface.php +++ b/src/Component/FacetSetInterface.php @@ -75,18 +75,18 @@ interface FacetSetInterface * * @throws InvalidArgumentException * - * @return \Solarium\Component\FacetSet + * @return self Provides fluent interface */ - public function addFacet($facet): self; + public function addFacet($facet); /** * Add multiple facets. * * @param array $facets * - * @return \Solarium\Component\FacetSet + * @return self Provides fluent interface */ - public function addFacets(array $facets): self; + public function addFacets(array $facets); /** * Get a facet. @@ -111,16 +111,16 @@ public function getFacets(): array; * * @param string|FacetInterface $facet * - * @return \Solarium\Component\FacetSet + * @return self Provides fluent interface */ - public function removeFacet($facet): self; + public function removeFacet($facet); /** * Remove all facets. * - * @return \Solarium\Component\FacetSet + * @return self Provides fluent interface */ - public function clearFacets(): self; + public function clearFacets(); /** * Set multiple facets. @@ -129,9 +129,9 @@ public function clearFacets(): self; * * @param FacetInterface[] $facets * - * @return \Solarium\Component\FacetSet + * @return self Provides fluent interface */ - public function setFacets(array $facets): self; + public function setFacets(array $facets); /** * Create a facet instance. diff --git a/src/Component/RequestBuilder/FacetSet.php b/src/Component/RequestBuilder/FacetSet.php index c5bc68f82..52e9cdcfe 100644 --- a/src/Component/RequestBuilder/FacetSet.php +++ b/src/Component/RequestBuilder/FacetSet.php @@ -146,8 +146,10 @@ public function addFacetField(Request $request, FacetField $facet, bool $useLoca $field = $facet->getField(); if ($useLocalParams) { - $localParams = ['key' => $facet->getKey(), + $localParams = [ + 'key' => $facet->getKey(), 'ex' => $facet->getLocalParameters()->getExcludes(), + 'terms' => $facet->getLocalParameters()->getTerms(), 'facet.prefix' => $facet->getPrefix(), 'facet.contains' => $facet->getContains(), 'facet.contains.ignoreCase' => $facet->getContainsIgnoreCase(), diff --git a/src/Core/Query/AbstractRequestBuilder.php b/src/Core/Query/AbstractRequestBuilder.php index 7f84d68e6..7115e9f9e 100644 --- a/src/Core/Query/AbstractRequestBuilder.php +++ b/src/Core/Query/AbstractRequestBuilder.php @@ -10,6 +10,7 @@ namespace Solarium\Core\Query; use Solarium\Core\Client\Request; +use Solarium\Core\Query\LocalParameters\LocalParameter; use Solarium\QueryType\Server\AbstractServerQuery; /** @@ -88,7 +89,13 @@ public function renderLocalParams(string $value, array $localParams = []): strin $paramValue = $paramValue ? 'true' : 'false'; } - $params .= $paramName.'='.$helper->escapeLocalParamValue($paramValue).' '; + if (LocalParameter::isSplitSmart($paramName)) { + $paramValue = $helper->escapeLocalParamValue($paramValue, ','); + } else { + $paramValue = $helper->escapeLocalParamValue($paramValue); + } + + $params .= $paramName.'='.$paramValue.' '; } if ('' !== $params = trim($params)) { diff --git a/src/Core/Query/Helper.php b/src/Core/Query/Helper.php index 11d61697e..4746be5c5 100644 --- a/src/Core/Query/Helper.php +++ b/src/Core/Query/Helper.php @@ -108,18 +108,30 @@ public function escapePhrase(string $input): string * a single quote, a double quote, or a right curly bracket. It backslash * escapes single quotes and backslashes within that quoted string. * + * If an optional pre-escaped separator character is passed, a backslash + * preceding this character will not be escaped with another backslash. + * {@internal Based on splitSmart() in org.apache.solr.common.util.StrUtils} + * * A value that doesn't require quoting is returned as is. * * @see https://solr.apache.org/guide/local-parameters-in-queries.html#basic-syntax-of-local-parameters * * @param string $value + * @param string $preEscapedSeparator Separator character that is already escaped with a backslash * * @return string */ - public function escapeLocalParamValue(string $value): string + public function escapeLocalParamValue(string $value, string $preEscapedSeparator = null): string { if (preg_match('/[ \'"}]/', $value)) { - $value = "'".preg_replace("/('|\\\\)/", '\\\$1', $value)."'"; + $pattern = "/('|\\\\)/"; + + if (null !== $preEscapedSeparator) { + $char = preg_quote(substr($preEscapedSeparator, 0, 1), '/'); + $pattern = "/('|\\\\(?!$char))/"; + } + + $value = "'".preg_replace($pattern, '\\\$1', $value)."'"; } return $value; diff --git a/src/Core/Query/LocalParameters/LocalParameter.php b/src/Core/Query/LocalParameters/LocalParameter.php index 76c42637a..968f17cdf 100644 --- a/src/Core/Query/LocalParameters/LocalParameter.php +++ b/src/Core/Query/LocalParameters/LocalParameter.php @@ -69,6 +69,15 @@ class LocalParameter implements LocalParameterInterface self::TYPE_COST => 'local_cost', ]; + public const IS_SPLIT_SMART = [ + self::TYPE_EXCLUDE, + self::TYPE_TAG, + self::TYPE_RANGE, + self::TYPE_STAT, + self::TYPE_TERM, + self::TYPE_QUERY, + ]; + /** * @var string */ @@ -174,4 +183,21 @@ public function removeValue($value): LocalParameterInterface return $this; } + + /** + * Will a parameter of this type be "split smart" by Solr? + * + * A local parameter is "split smart" if a literal comma in a value can be escaped + * with backslash when a comma is normally used to separate multiple values. + * + * {@internal Method name inspired by splitSmart() in org.apache.solr.common.util.StrUtils} + * + * @param string $type + * + * @return bool + */ + public static function isSplitSmart(string $type): bool + { + return \in_array($type, self::IS_SPLIT_SMART); + } } diff --git a/src/Core/Query/LocalParameters/LocalParameters.php b/src/Core/Query/LocalParameters/LocalParameters.php index ec6eae92b..013353fdb 100644 --- a/src/Core/Query/LocalParameters/LocalParameters.php +++ b/src/Core/Query/LocalParameters/LocalParameters.php @@ -329,10 +329,24 @@ public function addTerms(array $terms): self * @throws OutOfBoundsException * * @return $this + * + * @deprecated Will be removed in Solarium 8. Use {@see removeTerm()} instead. */ public function removeTerms(string $terms): self { - return $this->removeValue(LocalParameter::TYPE_TERM, $terms); + return $this->removeTerm($terms); + } + + /** + * @param string $term + * + * @throws OutOfBoundsException + * + * @return $this + */ + public function removeTerm(string $term): self + { + return $this->removeValue(LocalParameter::TYPE_TERM, $term); } /** diff --git a/src/Core/Query/LocalParameters/LocalParametersTrait.php b/src/Core/Query/LocalParameters/LocalParametersTrait.php index d0ade9b7f..37587689e 100644 --- a/src/Core/Query/LocalParameters/LocalParametersTrait.php +++ b/src/Core/Query/LocalParameters/LocalParametersTrait.php @@ -48,7 +48,7 @@ protected function initLocalParameters(): void switch ($name) { case LocalParameter::PARAMETER_MAP[LocalParameter::TYPE_EXCLUDE]: if (!\is_array($value)) { - $value = explode(',', $value); + $value = preg_split('/(?getLocalParameters()->addExcludes($value); @@ -62,7 +62,7 @@ protected function initLocalParameters(): void break; case LocalParameter::PARAMETER_MAP[LocalParameter::TYPE_TAG]: if (!\is_array($value)) { - $value = explode(',', $value); + $value = preg_split('/(?getLocalParameters()->addTags($value); @@ -71,7 +71,7 @@ protected function initLocalParameters(): void break; case LocalParameter::PARAMETER_MAP[LocalParameter::TYPE_RANGE]: if (!\is_array($value)) { - $value = explode(',', $value); + $value = preg_split('/(?getLocalParameters()->addRanges($value); @@ -80,7 +80,7 @@ protected function initLocalParameters(): void break; case LocalParameter::PARAMETER_MAP[LocalParameter::TYPE_STAT]: if (!\is_array($value)) { - $value = explode(',', $value); + $value = preg_split('/(?getLocalParameters()->addStats($value); @@ -88,7 +88,7 @@ protected function initLocalParameters(): void break; case LocalParameter::PARAMETER_MAP[LocalParameter::TYPE_TERM]: if (!\is_array($value)) { - $value = explode(',', $value); + $value = preg_split('/(?getLocalParameters()->addTerms($value); diff --git a/tests/Component/Facet/FacetTest.php b/tests/Component/Facet/FacetTest.php index e59acb19b..cc7163556 100644 --- a/tests/Component/Facet/FacetTest.php +++ b/tests/Component/Facet/FacetTest.php @@ -22,12 +22,14 @@ public function testConfigMode() { $this->facet->setOptions(['local_key' => 'myKey', 'local_exclude' => ['e1', 'e2']]); $this->assertSame('myKey', $this->facet->getKey()); + $this->assertEquals(['e1', 'e2'], $this->facet->getExcludes()); $this->assertEquals(['e1', 'e2'], $this->facet->getLocalParameters()->getExcludes()); } public function testConfigModeWithSingleValueExclude() { $this->facet->setOptions(['local_exclude' => 'e1']); + $this->assertEquals(['e1'], $this->facet->getExcludes()); $this->assertEquals(['e1'], $this->facet->getLocalParameters()->getExcludes()); } @@ -39,27 +41,61 @@ public function testSetAndGetKey() public function testAddExclude() { - $this->facet->getLocalParameters()->setExclude('e1'); + $this->facet->addExclude('e1'); + $this->assertEquals(['e1'], $this->facet->getExcludes()); $this->assertEquals(['e1'], $this->facet->getLocalParameters()->getExcludes()); + + $this->facet->addExclude('e2'); + $this->assertEquals(['e1', 'e2'], $this->facet->getExcludes()); + $this->assertEquals(['e1', 'e2'], $this->facet->getLocalParameters()->getExcludes()); } public function testAddExcludes() { - $this->facet->getLocalParameters()->addExcludes(['e1', 'e2']); + $this->facet->addExcludes(['e1', 'e2']); + $this->assertEquals(['e1', 'e2'], $this->facet->getExcludes()); + $this->assertEquals(['e1', 'e2'], $this->facet->getLocalParameters()->getExcludes()); + + $this->facet->addExcludes('e3,e4'); + $this->assertEquals(['e1', 'e2', 'e3', 'e4'], $this->facet->getExcludes()); + $this->assertEquals(['e1', 'e2', 'e3', 'e4'], $this->facet->getLocalParameters()->getExcludes()); + } + + public function testSetExcludes() + { + $this->facet->setExcludes(['e1', 'e2']); + $this->assertEquals(['e1', 'e2'], $this->facet->getExcludes()); $this->assertEquals(['e1', 'e2'], $this->facet->getLocalParameters()->getExcludes()); + + $this->facet->setExcludes('e3,e4'); + $this->assertEquals(['e3', 'e4'], $this->facet->getExcludes()); + $this->assertEquals(['e3', 'e4'], $this->facet->getLocalParameters()->getExcludes()); + } + + public function testSetAndAddTermsWithEscapedSeparator() + { + $this->facet->setExcludes('e1\,e2,e3'); + $this->assertEquals(['e1\,e2', 'e3'], $this->facet->getExcludes()); + $this->assertEquals(['e1\,e2', 'e3'], $this->facet->getLocalParameters()->getExcludes()); + + $this->facet->addExcludes('e4\,e5,e6'); + $this->assertEquals(['e1\,e2', 'e3', 'e4\,e5', 'e6'], $this->facet->getExcludes()); + $this->assertEquals(['e1\,e2', 'e3', 'e4\,e5', 'e6'], $this->facet->getLocalParameters()->getExcludes()); } public function testRemoveExclude() { - $this->facet->getLocalParameters()->addExcludes(['e1', 'e2']); - $this->facet->getLocalParameters()->removeExclude('e1'); + $this->facet->setExcludes(['e1', 'e2']); + $this->facet->removeExclude('e1'); + $this->assertEquals(['e2'], $this->facet->getExcludes()); $this->assertEquals(['e2'], $this->facet->getLocalParameters()->getExcludes()); } public function testClearExcludes() { - $this->facet->getLocalParameters()->addExcludes(['e1', 'e2']); - $this->facet->getLocalParameters()->clearExcludes(); + $this->facet->setExcludes(['e1', 'e2']); + $this->facet->clearExcludes(); + $this->assertEquals([], $this->facet->getExcludes()); $this->assertEquals([], $this->facet->getLocalParameters()->getExcludes()); } } diff --git a/tests/Component/Facet/FieldTest.php b/tests/Component/Facet/FieldTest.php index 4c8c9e8ad..a2b8aa4e9 100644 --- a/tests/Component/Facet/FieldTest.php +++ b/tests/Component/Facet/FieldTest.php @@ -23,6 +23,7 @@ public function testConfigMode() $options = [ 'local_key' => 'myKey', 'local_exclude' => ['e1', 'e2'], + 'local_terms' => ['t1', 't2'], 'field' => 'text', 'prefix' => 'xyz', 'contains' => 'foobar', @@ -45,7 +46,8 @@ public function testConfigMode() $this->facet->setOptions($options); $this->assertSame($options['local_key'], $this->facet->getKey()); - $this->assertSame($options['local_exclude'], $this->facet->getLocalParameters()->getExcludes()); + $this->assertSame($options['local_exclude'], $this->facet->getExcludes()); + $this->assertSame($options['local_terms'], $this->facet->getTerms()); $this->assertSame($options['field'], $this->facet->getField()); $this->assertSame($options['prefix'], $this->facet->getPrefix()); $this->assertSame($options['contains'], $this->facet->getContains()); @@ -79,6 +81,66 @@ public function testSetAndGetField() $this->assertSame('category', $this->facet->getField()); } + public function testAddTerm() + { + $this->facet->addTerm('t1'); + $this->assertEquals(['t1'], $this->facet->getTerms()); + $this->assertEquals(['t1'], $this->facet->getLocalParameters()->getTerms()); + + $this->facet->addTerm('t2'); + $this->assertEquals(['t1', 't2'], $this->facet->getTerms()); + $this->assertEquals(['t1', 't2'], $this->facet->getLocalParameters()->getTerms()); + } + + public function testAddTerms() + { + $this->facet->addTerms(['t1', 't2']); + $this->assertEquals(['t1', 't2'], $this->facet->getTerms()); + $this->assertEquals(['t1', 't2'], $this->facet->getLocalParameters()->getTerms()); + + $this->facet->addTerms('t3,t4'); + $this->assertEquals(['t1', 't2', 't3', 't4'], $this->facet->getTerms()); + $this->assertEquals(['t1', 't2', 't3', 't4'], $this->facet->getLocalParameters()->getTerms()); + } + + public function testSetTerms() + { + $this->facet->setTerms(['t1', 't2']); + $this->assertEquals(['t1', 't2'], $this->facet->getTerms()); + $this->assertEquals(['t1', 't2'], $this->facet->getLocalParameters()->getTerms()); + + $this->facet->setTerms('t3,t4'); + $this->assertEquals(['t3', 't4'], $this->facet->getTerms()); + $this->assertEquals(['t3', 't4'], $this->facet->getLocalParameters()->getTerms()); + } + + public function testSetAndAddTermsWithEscapedSeparator() + { + $this->facet->setTerms('t1\,t2,t3'); + $this->assertEquals(['t1\,t2', 't3'], $this->facet->getTerms()); + $this->assertEquals(['t1\,t2', 't3'], $this->facet->getLocalParameters()->getTerms()); + + $this->facet->addTerms('t4\,t5,t6'); + $this->assertEquals(['t1\,t2', 't3', 't4\,t5', 't6'], $this->facet->getTerms()); + $this->assertEquals(['t1\,t2', 't3', 't4\,t5', 't6'], $this->facet->getLocalParameters()->getTerms()); + } + + public function testRemoveTerm() + { + $this->facet->setTerms(['t1', 't2']); + $this->facet->removeTerm('t1'); + $this->assertEquals(['t2'], $this->facet->getTerms()); + $this->assertEquals(['t2'], $this->facet->getLocalParameters()->getTerms()); + } + + public function testClearTerms() + { + $this->facet->setTerms(['t1', 't2']); + $this->facet->clearTerms(); + $this->assertEquals([], $this->facet->getTerms()); + $this->assertEquals([], $this->facet->getLocalParameters()->getTerms()); + } + public function testSetAndGetPrefix() { $this->facet->setPrefix('xyz'); diff --git a/tests/Component/Facet/MultiQueryTest.php b/tests/Component/Facet/MultiQueryTest.php index 5273b19c3..cd2028506 100644 --- a/tests/Component/Facet/MultiQueryTest.php +++ b/tests/Component/Facet/MultiQueryTest.php @@ -293,6 +293,66 @@ public function testSetQueries() ); } + public function testAddExclude() + { + $this->facet->addExclude('e1'); + $this->assertEquals(['e1'], $this->facet->getExcludes()); + $this->assertEquals(['e1'], $this->facet->getLocalParameters()->getExcludes()); + + $this->facet->addExclude('e2'); + $this->assertEquals(['e1', 'e2'], $this->facet->getExcludes()); + $this->assertEquals(['e1', 'e2'], $this->facet->getLocalParameters()->getExcludes()); + } + + public function testAddExcludes() + { + $this->facet->addExcludes(['e1', 'e2']); + $this->assertEquals(['e1', 'e2'], $this->facet->getExcludes()); + $this->assertEquals(['e1', 'e2'], $this->facet->getLocalParameters()->getExcludes()); + + $this->facet->addExcludes('e3,e4'); + $this->assertEquals(['e1', 'e2', 'e3', 'e4'], $this->facet->getExcludes()); + $this->assertEquals(['e1', 'e2', 'e3', 'e4'], $this->facet->getLocalParameters()->getExcludes()); + } + + public function testSetExcludes() + { + $this->facet->setExcludes(['e1', 'e2']); + $this->assertEquals(['e1', 'e2'], $this->facet->getExcludes()); + $this->assertEquals(['e1', 'e2'], $this->facet->getLocalParameters()->getExcludes()); + + $this->facet->setExcludes('e3,e4'); + $this->assertEquals(['e3', 'e4'], $this->facet->getExcludes()); + $this->assertEquals(['e3', 'e4'], $this->facet->getLocalParameters()->getExcludes()); + } + + public function testSetAndAddTermsWithEscapedSeparator() + { + $this->facet->setExcludes('e1\,e2,e3'); + $this->assertEquals(['e1\,e2', 'e3'], $this->facet->getExcludes()); + $this->assertEquals(['e1\,e2', 'e3'], $this->facet->getLocalParameters()->getExcludes()); + + $this->facet->addExcludes('e4\,e5,e6'); + $this->assertEquals(['e1\,e2', 'e3', 'e4\,e5', 'e6'], $this->facet->getExcludes()); + $this->assertEquals(['e1\,e2', 'e3', 'e4\,e5', 'e6'], $this->facet->getLocalParameters()->getExcludes()); + } + + public function testRemoveExclude() + { + $this->facet->setExcludes(['e1', 'e2']); + $this->facet->removeExclude('e1'); + $this->assertEquals(['e2'], $this->facet->getExcludes()); + $this->assertEquals(['e2'], $this->facet->getLocalParameters()->getExcludes()); + } + + public function testClearExcludes() + { + $this->facet->setExcludes(['e1', 'e2']); + $this->facet->clearExcludes(); + $this->assertEquals([], $this->facet->getExcludes()); + $this->assertEquals([], $this->facet->getLocalParameters()->getExcludes()); + } + public function testAddExcludeForwarding() { $facetQuery = new Query(); @@ -304,7 +364,37 @@ public function testAddExcludeForwarding() $this->assertSame( ['fq1'], - $facetQuery->getLocalParameters()->getExcludes() + $facetQuery->getExcludes() + ); + } + + public function testAddExcludesForwarding() + { + $facetQuery = new Query(); + $facetQuery->setKey('k1'); + $facetQuery->setQuery('category:1'); + $this->facet->addQuery($facetQuery); + + $this->facet->addExcludes(['fq1', 'fq2']); + + $this->assertSame( + ['fq1', 'fq2'], + $facetQuery->getExcludes() + ); + } + + public function testSetExcludesForwarding() + { + $facetQuery = new Query(); + $facetQuery->setKey('k1'); + $facetQuery->setQuery('category:1'); + $this->facet->addQuery($facetQuery); + + $this->facet->setExcludes(['fq1', 'fq2']); + + $this->assertSame( + ['fq1', 'fq2'], + $facetQuery->getExcludes() ); } @@ -319,14 +409,14 @@ public function testRemoveExcludeForwarding() $this->assertSame( ['fq1'], - $facetQuery->getLocalParameters()->getExcludes() + $facetQuery->getExcludes() ); $this->facet->removeExclude('fq1'); $this->assertSame( [], - $facetQuery->getLocalParameters()->getExcludes() + $facetQuery->getExcludes() ); } diff --git a/tests/Component/Facet/RangeTest.php b/tests/Component/Facet/RangeTest.php index 89e8705d3..fb4a32e3f 100644 --- a/tests/Component/Facet/RangeTest.php +++ b/tests/Component/Facet/RangeTest.php @@ -37,7 +37,7 @@ public function testConfigMode() $this->facet->setOptions($options); $this->assertSame($options['local_key'], $this->facet->getKey()); - $this->assertSame($options['local_exclude'], $this->facet->getLocalParameters()->getExcludes()); + $this->assertSame($options['local_exclude'], $this->facet->getExcludes()); $this->assertSame($options['field'], $this->facet->getField()); $this->assertSame((string) $options['start'], $this->facet->getStart()); $this->assertSame((string) $options['end'], $this->facet->getEnd()); @@ -47,6 +47,33 @@ public function testConfigMode() $this->assertSame([$options['include']], $this->facet->getInclude()); } + public function testConfigModeWithExclude() + { + $options = [ + 'exclude' => 'e1\,e2,e3', + ]; + + @$this->facet->setOptions($options); + + $this->assertSame(['e1\,e2', 'e3'], $this->facet->getExcludes()); + } + + public function testConfigModeWithExcludeThrowsDeprecation() + { + set_error_handler(static function (int $errno, string $errstr): never { + throw new \Exception($errstr, $errno); + }, \E_USER_DEPRECATED); + + $options = [ + 'exclude' => 'e1\,e2,e3', + ]; + + $this->expectExceptionCode(\E_USER_DEPRECATED); + $this->facet->setOptions($options); + + restore_error_handler(); + } + public function testGetType() { $this->assertSame( diff --git a/tests/Component/RequestBuilder/FacetSetTest.php b/tests/Component/RequestBuilder/FacetSetTest.php index e2bc9e8af..fda7f2074 100644 --- a/tests/Component/RequestBuilder/FacetSetTest.php +++ b/tests/Component/RequestBuilder/FacetSetTest.php @@ -47,7 +47,7 @@ public function testBuildEmptyFacetSet() { $request = $this->builder->buildComponent($this->component, $this->request); - static::assertEquals( + $this->assertEquals( [], $request->getParams() ); @@ -55,8 +55,8 @@ public function testBuildEmptyFacetSet() public function testBuildWithFacets() { - $this->component->addFacet(new FacetField(['local_key' => 'f1', 'field' => 'owner'])); - $this->component->addFacet(new FacetQuery(['local_key' => 'f2', 'query' => 'category:23'])); + $this->component->addFacet(new FacetField(['local_key' => 'f1', 'local_exclude' => 'e11,e12', 'local_terms' => 't1,t2', 'field' => 'owner'])); + $this->component->addFacet(new FacetQuery(['local_key' => 'f2', 'local_exclude' => 'e21,e22', 'query' => 'category:23'])); $this->component->addFacet( new FacetMultiQuery(['local_key' => 'f3', 'query' => ['f4' => ['query' => 'category:40']]]) ); @@ -65,7 +65,20 @@ public function testBuildWithFacets() $this->assertNull($request->getRawData()); $this->assertEquals( - '?facet.field={!key=f1}owner&facet.query={!key=f2}category:23&facet.query={!key=f4}category:40&facet=true', + '?facet.field={!key=f1 ex=e11,e12 terms=t1,t2}owner&facet.query={!key=f2 ex=e21,e22}category:23&facet.query={!key=f4}category:40&facet=true', + urldecode($request->getUri()) + ); + } + + public function testBuildWithFacetFieldWithCommaAndQuoteInTerm() + { + $this->component->addFacet(new FacetField(['local_key' => 'f1', 'local_terms' => ['yes\, it is', 'no\, it isn\'t'], 'field' => 'isit'])); + + $request = $this->builder->buildComponent($this->component, $this->request); + + $this->assertNull($request->getRawData()); + $this->assertEquals( + "?facet.field={!key=f1 terms='yes\\, it is,no\\, it isn\\'t'}isit&facet=true", urldecode($request->getUri()) ); } @@ -134,6 +147,21 @@ public function testBuildWithJsonFacetFilterQueryAndParams() ); } + public function testBuildWithJsonFacetFilterQueryWithPlaceholders() + { + $terms = new JsonTerms(['local_key' => 'f1', 'field' => 'owner']); + $terms->setDomainFilterQuery('popularity:[%1% TO %2%]', [5, 10]); + $this->component->addFacet($terms); + + $request = $this->builder->buildComponent($this->component, $this->request); + + $this->assertNull($request->getRawData()); + $this->assertEquals( + '?json.facet={"f1":{"field":"owner","domain":{"filter":"popularity:[5 TO 10]"},"type":"terms"}}', + urldecode($request->getUri()) + ); + } + public function testBuildWithJsonFacetFilterParamsAndQuery() { $terms = new JsonTerms(['local_key' => 'f1', 'field' => 'owner']); @@ -151,6 +179,24 @@ public function testBuildWithJsonFacetFilterParamsAndQuery() ); } + public function testBuildWithJsonFacetFilterParamsAndQueryOverwrite() + { + $terms = new JsonTerms(['local_key' => 'f1', 'field' => 'owner']); + $terms->setDomainFilterQuery('popularity:[5 TO 10]'); + $terms->addDomainFilterParameter('myparam1'); + $terms->addDomainFilterParameter('myparam2'); + $terms->setDomainFilterQuery('popularity:[15 TO 20]'); + $this->component->addFacet($terms); + + $request = $this->builder->buildComponent($this->component, $this->request); + + $this->assertNull($request->getRawData()); + $this->assertEquals( + '?json.facet={"f1":{"field":"owner","domain":{"filter":["popularity:[15 TO 20]",{"param":"myparam1"},{"param":"myparam2"}]},"type":"terms"}}', + urldecode($request->getUri()) + ); + } + public function testBuildWithFacetsAndJsonFacets() { $this->component->addFacet(new FacetField(['local_key' => 'f1', 'field' => 'owner'])); @@ -208,6 +254,44 @@ public function testBuildWithNestedJsonFacets() ); } + public function testBuildWithNestedJsonFacetRemoved() + { + $terms = new JsonTerms(['local_key' => 'f1', 'field' => 'owner']); + $query = new JsonQuery(['local_key' => 'f2', 'query' => 'category:23']); + $query->addFacet(new JsonAggregation(['local_key' => 'f1', 'function' => 'avg(mul(price,popularity))'])); + $query->addFacet(new JsonAggregation(['local_key' => 'f2', 'function' => 'unique(popularity)'])); + $query->removeFacet('f1'); + $terms->addFacet($query); + $this->component->addFacet($terms); + + $request = $this->builder->buildComponent($this->component, $this->request); + + $this->assertNull($request->getRawData()); + $this->assertEquals( + '?json.facet={"f1":{"field":"owner","type":"terms","facet":{"f2":{"type":"query","facet":{"f2":"unique(popularity)"},"q":"category:23"}}}}', + urldecode($request->getUri()) + ); + } + + public function testBuildWithNestedJsonFacetsCleared() + { + $terms = new JsonTerms(['local_key' => 'f1', 'field' => 'owner']); + $query = new JsonQuery(['local_key' => 'f2', 'query' => 'category:23']); + $query->addFacet(new JsonAggregation(['local_key' => 'f1', 'function' => 'avg(mul(price,popularity))'])); + $query->addFacet(new JsonAggregation(['local_key' => 'f2', 'function' => 'unique(popularity)'])); + $query->clearFacets(); + $terms->addFacet($query); + $this->component->addFacet($terms); + + $request = $this->builder->buildComponent($this->component, $this->request); + + $this->assertNull($request->getRawData()); + $this->assertEquals( + '?json.facet={"f1":{"field":"owner","type":"terms","facet":{"f2":{"type":"query","q":"category:23"}}}}', + urldecode($request->getUri()) + ); + } + public function testBuildWithRangeFacet() { $this->component->addFacet(new FacetRange( @@ -366,7 +450,7 @@ public function testBuildWithPivotFacet() 'overrequest.ratio' => 2.2, ] ); - $facet->getLocalParameters()->setExclude('owner'); + $facet->addExclude('owner'); $this->component->addFacet($facet); $this->component->setPivotMinCount(5); $this->component->setLimit(-1); diff --git a/tests/Core/Query/HelperTest.php b/tests/Core/Query/HelperTest.php index 1583309da..71178c961 100644 --- a/tests/Core/Query/HelperTest.php +++ b/tests/Core/Query/HelperTest.php @@ -496,6 +496,34 @@ public function escapeLocalParamValueProvider(): array ]; } + /** + * @dataProvider escapeLocalParamValuePreEscapedSeparatorProvider + */ + public function testEscapeLocalParamValuePreEscapedSeparator(string $value, string $separator, string $expectedWithoutSeparator, string $expectedWithSeparator) + { + $this->assertSame( + $expectedWithoutSeparator, + $this->helper->escapeLocalParamValue($value) + ); + + $this->assertSame( + $expectedWithSeparator, + $this->helper->escapeLocalParamValue($value, $separator) + ); + } + + public function escapeLocalParamValuePreEscapedSeparatorProvider(): array + { + return [ + 'no other escapes needed' => ['a\\,b', ',', 'a\\,b', 'a\\,b'], + 'other escapes needed' => ['a b\\,c', ',', "'a b\\\\,c'", "'a b\\,c'"], + 'unescaped separator left alone' => ['a b\\,c,d', ',', "'a b\\\\,c,d'", "'a b\\,c,d'"], + 'multiple escaped separators' => ['a b\\,c\\,d', ',', "'a b\\\\,c\\\\,d'", "'a b\\,c\\,d'"], + 'separator can be only 1 char' => ['a b\\,\\;c', ',;', "'a b\\\\,\\\\;c'", "'a b\\,\\\\;c'"], + 'separator is also regex syntax' => ['a b\\|c', '|', "'a b\\\\|c'", "'a b\\|c'"], + ]; + } + /** * @testWith ["ab"] * ["a\\b"] diff --git a/tests/Core/Query/LocalParameters/LocalParameterTest.php b/tests/Core/Query/LocalParameters/LocalParameterTest.php index debfc3166..ecb64477b 100644 --- a/tests/Core/Query/LocalParameters/LocalParameterTest.php +++ b/tests/Core/Query/LocalParameters/LocalParameterTest.php @@ -86,4 +86,10 @@ public function testToString(): void $this->parameter->setValues(['value1', 'value2']); $this->assertSame('key=value1,value2', (string) $this->parameter); } + + public function testIsSplitSmart(): void + { + $this->assertTrue(LocalParameter::isSplitSmart(LocalParameter::IS_SPLIT_SMART[0])); + $this->assertFalse(LocalParameter::isSplitSmart('other.type')); + } } diff --git a/tests/Core/Query/LocalParameters/LocalParametersTest.php b/tests/Core/Query/LocalParameters/LocalParametersTest.php index 79a4ba674..a9bb464df 100644 --- a/tests/Core/Query/LocalParameters/LocalParametersTest.php +++ b/tests/Core/Query/LocalParameters/LocalParametersTest.php @@ -37,19 +37,19 @@ public function testInitLocalParameters(): void { $options = [ 'local_key' => 'key', - 'local_exclude' => '', - 'local_tag' => '', - 'local_range' => '', - 'local_stats' => '', - 'local_terms' => '', - 'local_type' => '', - 'local_query' => '', - 'local_query_field' => '', - 'local_default_field' => '', - 'local_max' => '', - 'local_mean' => '', - 'local_min' => '', - 'local_value' => '', + 'local_exclude' => 'ex1\,ex2,ex3', + 'local_tag' => 'tag1\,tag2,tag3', + 'local_range' => 'r1\,r2,r3', + 'local_stats' => 'stat1\,stat2,stat3', + 'local_terms' => 't1\,t2,t3', + 'local_type' => 'mytype', + 'local_query' => 'myquery', + 'local_query_field' => 'myfield', + 'local_default_field' => 'deffield', + 'local_max' => 'max', + 'local_mean' => 'mean', + 'local_min' => 'min', + 'local_value' => 'value', 'local_cache' => true, 'local_cost' => 0, ]; @@ -59,21 +59,37 @@ public function testInitLocalParameters(): void $parameters = $query->getLocalParameters(); $this->assertArrayHasKey(LocalParameter::TYPE_KEY, $parameters); + $this->assertSame(['key'], $parameters->getKeys()); $this->assertArrayHasKey(LocalParameter::TYPE_EXCLUDE, $parameters); + $this->assertSame(['ex1\,ex2', 'ex3'], $parameters->getExcludes()); $this->assertArrayHasKey(LocalParameter::TYPE_TAG, $parameters); + $this->assertSame(['tag1\,tag2', 'tag3'], $parameters->getTags()); $this->assertArrayHasKey(LocalParameter::TYPE_RANGE, $parameters); + $this->assertSame(['r1\,r2', 'r3'], $parameters->getRanges()); $this->assertArrayHasKey(LocalParameter::TYPE_STAT, $parameters); + $this->assertSame(['stat1\,stat2', 'stat3'], $parameters->getStats()); $this->assertArrayHasKey(LocalParameter::TYPE_TERM, $parameters); + $this->assertSame(['t1\,t2', 't3'], $parameters->getTerms()); $this->assertArrayHasKey(LocalParameter::TYPE_TYPE, $parameters); + $this->assertSame(['mytype'], $parameters->getTypes()); $this->assertArrayHasKey(LocalParameter::TYPE_QUERY, $parameters); + $this->assertSame(['myquery'], $parameters->getQueries()); $this->assertArrayHasKey(LocalParameter::TYPE_QUERY_FIELD, $parameters); + $this->assertSame(['myfield'], $parameters->getQueryFields()); $this->assertArrayHasKey(LocalParameter::TYPE_DEFAULT_FIELD, $parameters); + $this->assertSame(['deffield'], $parameters->getDefaultFields()); $this->assertArrayHasKey(LocalParameter::TYPE_MAX, $parameters); - $this->assertArrayHasKey(LocalParameter::TYPE_MAX, $parameters); + $this->assertSame(['max'], $parameters->getMax()); + $this->assertArrayHasKey(LocalParameter::TYPE_MEAN, $parameters); + $this->assertSame(['mean'], $parameters->getMean()); $this->assertArrayHasKey(LocalParameter::TYPE_MIN, $parameters); + $this->assertSame(['min'], $parameters->getMin()); $this->assertArrayHasKey(LocalParameter::TYPE_VALUE, $parameters); + $this->assertSame(['value'], $parameters->getLocalValues()); $this->assertArrayHasKey(LocalParameter::TYPE_CACHE, $parameters); + $this->assertSame(['true'], $parameters->getCache()); $this->assertArrayHasKey(LocalParameter::TYPE_COST, $parameters); + $this->assertSame([0], $parameters->getCost()); $keys = $parameters[LocalParameter::TYPE_KEY]; $this->assertInstanceOf(LocalParameter::class, $keys); @@ -96,6 +112,12 @@ public function testIllegalParameterType(): void $parameters->getKeys(); } + public function testGetUninitedLocalParameters(): void + { + $localParameters = (new DummyTraitUse())->getLocalParameters(); + $this->assertEquals(new LocalParameters(), $localParameters); + } + /** * @throws \PHPUnit\Framework\ExpectationFailedException * @throws \Solarium\Exception\OutOfBoundsException @@ -132,6 +154,9 @@ public function testExclude(): void $this->parameters->removeExclude('excludeOne'); $this->assertSame(['excludeTwo'], $this->parameters->getExcludes()); + + $this->parameters->setExclude('excludeThree'); + $this->assertSame(['excludeTwo', 'excludeThree'], $this->parameters->getExcludes()); } /** @@ -164,6 +189,9 @@ public function testRange(): void $this->parameters->removeRange('rangeOne'); $this->assertSame(['rangeTwo'], $this->parameters->getRanges()); + + $this->parameters->setRange('rangeThree'); + $this->assertSame(['rangeTwo', 'rangeThree'], $this->parameters->getRanges()); } /** @@ -196,6 +224,9 @@ public function testTag(): void $this->parameters->removeTag('tagOne'); $this->assertSame(['tagTwo'], $this->parameters->getTags()); + + $this->parameters->setTag('tagThree'); + $this->assertSame(['tagTwo', 'tagThree'], $this->parameters->getTags()); } /** @@ -226,8 +257,11 @@ public function testTerms(): void $this->parameters->addTerms(['termsOne', 'termsTwo']); $this->assertSame(['termsOne', 'termsTwo'], $this->parameters->getTerms()); - $this->parameters->removeTerms('termsOne'); + $this->parameters->removeTerm('termsOne'); $this->assertSame(['termsTwo'], $this->parameters->getTerms()); + + $this->parameters->setTerm('termsThree'); + $this->assertSame(['termsTwo', 'termsThree'], $this->parameters->getTerms()); } /** @@ -243,6 +277,21 @@ public function testReplaceTerms(): void $this->assertSame(['termsOne', 'termsTwo'], $this->parameters->getTerms()); } + /** + * @throws \PHPUnit\Framework\ExpectationFailedException + * @throws \Solarium\Exception\OutOfBoundsException + * + * @deprecated Will be removed in Solarium 8 + */ + public function testRemoveTerms(): void + { + $this->parameters->setTerms(['termsOne', 'termsTwo']); + $this->assertSame(['termsOne', 'termsTwo'], $this->parameters->getTerms()); + + $this->parameters->removeTerms('termsOne'); + $this->assertSame(['termsTwo'], $this->parameters->getTerms()); + } + /** * @throws \PHPUnit\Framework\ExpectationFailedException * @throws \Solarium\Exception\OutOfBoundsException @@ -260,6 +309,9 @@ public function testQuery(): void $this->parameters->removeQuery('queryOne'); $this->assertSame(['queryTwo'], $this->parameters->getQueries()); + + $this->parameters->setQuery('queryThree'); + $this->assertSame(['queryTwo', 'queryThree'], $this->parameters->getQueries()); } /** @@ -292,6 +344,9 @@ public function testStats(): void $this->parameters->removeStat('statsOne'); $this->assertSame(['statsTwo'], $this->parameters->getStats()); + + $this->parameters->setStat('statsThree'); + $this->assertSame(['statsTwo', 'statsThree'], $this->parameters->getStats()); } /** @@ -477,3 +532,11 @@ class DummyQuery extends Configurable { use LocalParametersTrait; } + +class DummyTraitUse +{ + use LocalParametersTrait; + + // trait assumes this is inherited from Configurable + protected $options = []; +} diff --git a/tests/Core/Query/RequestBuilderTest.php b/tests/Core/Query/RequestBuilderTest.php index 6b0874655..bcea5cbfa 100644 --- a/tests/Core/Query/RequestBuilderTest.php +++ b/tests/Core/Query/RequestBuilderTest.php @@ -5,6 +5,7 @@ use PHPUnit\Framework\TestCase; use Solarium\Core\Query\AbstractRequestBuilder; use Solarium\Core\Query\Helper; +use Solarium\Core\Query\LocalParameters\LocalParameter; use Solarium\QueryType\Select\Query\Query as SelectQuery; class RequestBuilderTest extends TestCase @@ -175,6 +176,21 @@ public function testRenderLocalParamsWithEscapes() ); } + public function testRenderLocalParamsWithSplitSmart() + { + $splitSmartParam = LocalParameter::IS_SPLIT_SMART[0]; + + $myParams = [ + 'no.split.smart' => 'a\, b,c', + $splitSmartParam => 'd\, e,f', + ]; + + $this->assertSame( + "{!no.split.smart='a\\\\, b,c' $splitSmartParam='d\\, e,f'}myValue", + $this->builder->renderLocalParams('myValue', $myParams) + ); + } + public function testRenderLocalParamsWithoutParams() { $this->assertSame(