From a50cbf25a2c2b29f08536ac455301f915ca91acc Mon Sep 17 00:00:00 2001 From: Benjamin Serfhos Date: Wed, 24 Oct 2018 17:41:47 +0200 Subject: [PATCH] [TASK] Process feedback and fix unit-tests --- .../Elasticsearch/MappingFactory.php | 4 --- Classes/Domain/Model/SearchRequest.php | 3 +- Classes/Domain/Search/QueryFactory.php | 6 ++-- Classes/Utility/ArrayUtility.php | 35 +++++++++++++++++++ .../Elasticsearch/MappingFactoryTest.php | 14 +------- .../Unit/Controller/SearchControllerTest.php | 4 +-- .../Unit/Domain/Index/AbstractIndexerTest.php | 2 ++ Tests/Unit/Domain/Model/SearchRequestTest.php | 4 +++ Tests/Unit/Domain/Search/QueryFactoryTest.php | 6 ++-- 9 files changed, 54 insertions(+), 24 deletions(-) create mode 100644 Classes/Utility/ArrayUtility.php diff --git a/Classes/Connection/Elasticsearch/MappingFactory.php b/Classes/Connection/Elasticsearch/MappingFactory.php index 9f1f262..705f58f 100644 --- a/Classes/Connection/Elasticsearch/MappingFactory.php +++ b/Classes/Connection/Elasticsearch/MappingFactory.php @@ -55,10 +55,6 @@ class MappingFactory implements Singleton $mapping->setType($type); $configuration = $this->getConfiguration($type->getName()); - if (isset($configuration['_all'])) { - $mapping->setAllField($configuration['_all']); - unset($configuration['_all']); - } $mapping->setProperties($configuration); return $mapping; diff --git a/Classes/Domain/Model/SearchRequest.php b/Classes/Domain/Model/SearchRequest.php index 0c66365..787a936 100644 --- a/Classes/Domain/Model/SearchRequest.php +++ b/Classes/Domain/Model/SearchRequest.php @@ -25,6 +25,7 @@ use Codappix\SearchCore\Connection\ConnectionInterface; use Codappix\SearchCore\Connection\FacetRequestInterface; use Codappix\SearchCore\Connection\SearchRequestInterface; use Codappix\SearchCore\Domain\Search\SearchService; +use Codappix\SearchCore\Utility\ArrayUtility as CustomArrayUtility; use TYPO3\CMS\Core\Utility\ArrayUtility; /** @@ -101,7 +102,7 @@ class SearchRequest implements SearchRequestInterface public function setFilter(array $filter) { $filter = ArrayUtility::removeArrayEntryByValue($filter, ''); - $this->filter = ArrayUtility::removeNullValuesRecursive($filter); + $this->filter = CustomArrayUtility::removeEmptyElementsRecursively($filter); } /** diff --git a/Classes/Domain/Search/QueryFactory.php b/Classes/Domain/Search/QueryFactory.php index 4bbd22d..c297560 100644 --- a/Classes/Domain/Search/QueryFactory.php +++ b/Classes/Domain/Search/QueryFactory.php @@ -124,9 +124,11 @@ class QueryFactory 'query' => $searchRequest->getSearchTerm(), ]; - $fieldsToQuery = GeneralUtility::trimExplode(',', $this->configuration->getIfExists('searching.fields.query'), true); - if (!empty($fieldsToQuery)) { + try { + $fieldsToQuery = GeneralUtility::trimExplode(',', $this->configuration->get('searching.fields.query'), true); $matchExpression['fields'] = $fieldsToQuery; + } catch (InvalidArgumentException $e) { + // Nothing configured } $minimumShouldMatch = $this->configuration->getIfExists('searching.minimumShouldMatch'); diff --git a/Classes/Utility/ArrayUtility.php b/Classes/Utility/ArrayUtility.php new file mode 100644 index 0000000..facc6fd --- /dev/null +++ b/Classes/Utility/ArrayUtility.php @@ -0,0 +1,35 @@ + $value) { + if (is_array($value)) { + $result[$key] = self::removeEmptyElementsRecursively($value); + if ($result[$key] === []) { + unset($result[$key]); + } + } elseif ($value === null) { + unset($result[$key]); + } + } + return $result; + } + +} diff --git a/Tests/Unit/Connection/Elasticsearch/MappingFactoryTest.php b/Tests/Unit/Connection/Elasticsearch/MappingFactoryTest.php index 8dde1cb..094c5d0 100644 --- a/Tests/Unit/Connection/Elasticsearch/MappingFactoryTest.php +++ b/Tests/Unit/Connection/Elasticsearch/MappingFactoryTest.php @@ -46,10 +46,6 @@ class MappingFactoryTest extends AbstractUnitTestCase { $indexName = 'someIndex'; $configuration = [ - '_all' => [ - 'type' => 'text', - 'analyzer' => 'ngram4', - ], 'channel' => [ 'type' => 'keyword', ], @@ -64,16 +60,8 @@ class MappingFactoryTest extends AbstractUnitTestCase ->method('get') ->with('indexing.' . $indexName . '.mapping') ->willReturn($configuration); - $mapping = $this->subject->getMapping($type)->toArray()[$indexName]; - $this->assertArraySubset( - [ - '_all' => $configuration['_all'] - ], - $mapping, - true, - 'Configuration of _all field was not set for mapping.' - ); + $this->assertArraySubset( [ 'channel' => $configuration['channel'] diff --git a/Tests/Unit/Controller/SearchControllerTest.php b/Tests/Unit/Controller/SearchControllerTest.php index 67c6d98..8617f6c 100644 --- a/Tests/Unit/Controller/SearchControllerTest.php +++ b/Tests/Unit/Controller/SearchControllerTest.php @@ -22,7 +22,7 @@ namespace Codappix\Tests\Unit\Controller; use Codappix\SearchCore\Controller\SearchController; use Codappix\SearchCore\Domain\Model\SearchRequest; -use Codappix\SearchCore\Domain\Search\SearchService; +use Codappix\SearchCore\Domain\Search\CachedSearchService; use Codappix\SearchCore\Tests\Unit\AbstractUnitTestCase; use TYPO3\CMS\Extbase\Mvc\Web\Request; use TYPO3\CMS\Extbase\Object\ObjectManager; @@ -54,7 +54,7 @@ class SearchControllerTest extends AbstractUnitTestCase parent::setUp(); - $searchService = $this->getMockBuilder(SearchService::class) + $searchService = $this->getMockBuilder(CachedSearchService::class) ->disableOriginalConstructor() ->getMock(); $this->request = new Request(); diff --git a/Tests/Unit/Domain/Index/AbstractIndexerTest.php b/Tests/Unit/Domain/Index/AbstractIndexerTest.php index 411cc24..b7373da 100644 --- a/Tests/Unit/Domain/Index/AbstractIndexerTest.php +++ b/Tests/Unit/Domain/Index/AbstractIndexerTest.php @@ -82,6 +82,7 @@ class AbstractIndexerTest extends AbstractUnitTestCase $expectedRecord['new_test_field'] = 'test'; $expectedRecord['new_test_field2'] = 'test' . PHP_EOL . 'test'; $expectedRecord['search_abstract'] = ''; + $expectedRecord['search_document_type'] = 'testTable'; $this->dataProcessorService->expects($this->any()) ->method('executeDataProcessor') @@ -137,6 +138,7 @@ class AbstractIndexerTest extends AbstractUnitTestCase $record = ['field 1' => 'test']; $expectedRecord = $record; $expectedRecord['search_abstract'] = ''; + $expectedRecord['search_document_type'] = 'testTable'; $this->configuration->expects($this->exactly(2)) ->method('get') diff --git a/Tests/Unit/Domain/Model/SearchRequestTest.php b/Tests/Unit/Domain/Model/SearchRequestTest.php index 25ced4e..fee7a80 100644 --- a/Tests/Unit/Domain/Model/SearchRequestTest.php +++ b/Tests/Unit/Domain/Model/SearchRequestTest.php @@ -45,6 +45,10 @@ class SearchRequestTest extends AbstractUnitTestCase ); } + /** + * Data provider for emptyFilterWillNotBeSet() + * @return array + */ public function possibleEmptyFilter() { return [ diff --git a/Tests/Unit/Domain/Search/QueryFactoryTest.php b/Tests/Unit/Domain/Search/QueryFactoryTest.php index dc824c5..5603502 100644 --- a/Tests/Unit/Domain/Search/QueryFactoryTest.php +++ b/Tests/Unit/Domain/Search/QueryFactoryTest.php @@ -428,7 +428,7 @@ class QueryFactoryTest extends AbstractUnitTestCase ['searching.fieldValueFactor'] ) ->will($this->onConsecutiveCalls( - '_all, field1, field2', + 'field1, field2', $this->throwException(new InvalidArgumentException), $this->throwException(new InvalidArgumentException), $this->throwException(new InvalidArgumentException), @@ -445,7 +445,6 @@ class QueryFactoryTest extends AbstractUnitTestCase 'type' => 'most_fields', 'query' => 'SearchWord', 'fields' => [ - '_all', 'field1', 'field2', ], @@ -662,6 +661,9 @@ class QueryFactoryTest extends AbstractUnitTestCase ); } + /** + * @return void + */ protected function configureConfigurationMockWithDefault() { $this->configuration->expects($this->any())