From 5dd0759bb603c590b0a6031c1a12e3cb4a1ea583 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Jun 2017 12:11:28 +0200 Subject: [PATCH 1/7] TASK: Fix file permissions --- Configuration/TypoScript/constants.txt | 0 Configuration/TypoScript/setup.txt | 0 2 files changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 Configuration/TypoScript/constants.txt mode change 100755 => 100644 Configuration/TypoScript/setup.txt diff --git a/Configuration/TypoScript/constants.txt b/Configuration/TypoScript/constants.txt old mode 100755 new mode 100644 diff --git a/Configuration/TypoScript/setup.txt b/Configuration/TypoScript/setup.txt old mode 100755 new mode 100644 From 3a2523e1d2e8e61350150a29c0f4ce5e098d2ee4 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Jun 2017 12:32:27 +0200 Subject: [PATCH 2/7] WIP|FEATURE: First basic implementation of filter * Working version without further architecture. * Manually tested. * Still need to move to new architecture and cover with tests. --- Classes/Connection/Elasticsearch.php | 27 ++++++++++++++++++++-- Classes/Domain/Model/SearchRequest.php | 31 +++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/Classes/Connection/Elasticsearch.php b/Classes/Connection/Elasticsearch.php index 560ca07..7f963ea 100644 --- a/Classes/Connection/Elasticsearch.php +++ b/Classes/Connection/Elasticsearch.php @@ -146,12 +146,35 @@ class Elasticsearch implements Singleton, ConnectionInterface { $this->logger->debug('Search for', [$searchRequest->getSearchTerm()]); + $query = [ + 'bool' => [ + 'must' => [ + [ + 'match' => [ + '_all' => $searchRequest->getSearchTerm() + ], + ], + ], + ], + ]; + + if ($searchRequest->hasFilter()) { + $queryFilter = []; + foreach ($searchRequest->getFilter() as $field => $value) { + $queryFilter[$field] = $value; + } + + $query['bool']['filter'] = [ + 'term' => $queryFilter, + ]; + } + $search = new \Elastica\Search($this->connection->getClient()); $search->addIndex('typo3content'); - + $search->setQuery(new \Elastica\Query(['query' => $query])); // TODO: Return wrapped result to implement our interface. // Also update php doc to reflect the change. - return $search->search('"' . $searchRequest->getSearchTerm() . '"'); + return $search->search(); } /** diff --git a/Classes/Domain/Model/SearchRequest.php b/Classes/Domain/Model/SearchRequest.php index 1de2f71..105cc45 100644 --- a/Classes/Domain/Model/SearchRequest.php +++ b/Classes/Domain/Model/SearchRequest.php @@ -32,7 +32,12 @@ class SearchRequest implements SearchRequestInterface * * @var string */ - protected $query; + protected $query = ''; + + /** + * @var array + */ + protected $filter = []; /** * @param string $query @@ -57,4 +62,28 @@ class SearchRequest implements SearchRequestInterface { return $this->query; } + + /** + * @param array $filter + */ + public function setFilter(array $filter) + { + $this->filter = $filter; + } + + /** + * @return bool + */ + public function hasFilter() + { + return count($this->filter); + } + + /** + * @return array + */ + public function getFilter() + { + return $this->filter; + } } From 1a41c5e237729026696034d04a67bcfca7384a71 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Jun 2017 13:53:08 +0200 Subject: [PATCH 3/7] FEATURE: Add filter to search * Allow to filter results by field values. --- .travis.yml | 4 +- Classes/Connection/Elasticsearch.php | 37 ++++------ Classes/Connection/SearchRequestInterface.php | 10 +++ Classes/Domain/Search/QueryFactory.php | 72 +++++++++++++++++++ Makefile | 5 ++ .../AbstractFunctionalTestCase.php | 7 ++ .../Connection/Elasticsearch/FilterTest.php | 61 ++++++++++++++++ .../Functional/Fixtures/Searching/Filter.xml | 42 +++++++++++ Tests/Unit/AbstractUnitTestCase.php | 27 +++++++ Tests/Unit/Domain/Search/QueryFactoryTest.php | 56 +++++++++++++++ Tests/Unit/UnitTests.xml | 28 ++++++++ 11 files changed, 323 insertions(+), 26 deletions(-) create mode 100644 Classes/Domain/Search/QueryFactory.php create mode 100644 Tests/Functional/Connection/Elasticsearch/FilterTest.php create mode 100644 Tests/Functional/Fixtures/Searching/Filter.xml create mode 100644 Tests/Unit/AbstractUnitTestCase.php create mode 100644 Tests/Unit/Domain/Search/QueryFactoryTest.php create mode 100644 Tests/Unit/UnitTests.xml diff --git a/.travis.yml b/.travis.yml index e583e51..123363a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -53,7 +53,9 @@ services: install: make install -script: make functionalTests +script: + - make unitTests + - make functionalTests after_script: - make uploadCodeCoverage diff --git a/Classes/Connection/Elasticsearch.php b/Classes/Connection/Elasticsearch.php index 7f963ea..386e19c 100644 --- a/Classes/Connection/Elasticsearch.php +++ b/Classes/Connection/Elasticsearch.php @@ -21,6 +21,7 @@ namespace Leonmrni\SearchCore\Connection; */ use TYPO3\CMS\Core\SingletonInterface as Singleton; +use Leonmrni\SearchCore\Domain\Search\QueryFactory; /** * Outer wrapper to elasticsearch. @@ -47,6 +48,11 @@ class Elasticsearch implements Singleton, ConnectionInterface */ protected $documentFactory; + /** + * @var QueryFactory + */ + protected $queryFactory; + /** * @var \TYPO3\CMS\Core\Log\Logger */ @@ -67,17 +73,20 @@ class Elasticsearch implements Singleton, ConnectionInterface * @param Elasticsearch\IndexFactory $indexFactory * @param Elasticsearch\TypeFactory $typeFactory * @param Elasticsearch\DocumentFactory $documentFactory + * @param QueryFactory $queryFactory */ public function __construct( Elasticsearch\Connection $connection, Elasticsearch\IndexFactory $indexFactory, Elasticsearch\TypeFactory $typeFactory, - Elasticsearch\DocumentFactory $documentFactory + Elasticsearch\DocumentFactory $documentFactory, + QueryFactory $queryFactory ) { $this->connection = $connection; $this->indexFactory = $indexFactory; $this->typeFactory = $typeFactory; $this->documentFactory = $documentFactory; + $this->queryFactory = $queryFactory; } public function addDocument($documentType, array $document) @@ -146,32 +155,10 @@ class Elasticsearch implements Singleton, ConnectionInterface { $this->logger->debug('Search for', [$searchRequest->getSearchTerm()]); - $query = [ - 'bool' => [ - 'must' => [ - [ - 'match' => [ - '_all' => $searchRequest->getSearchTerm() - ], - ], - ], - ], - ]; - - if ($searchRequest->hasFilter()) { - $queryFilter = []; - foreach ($searchRequest->getFilter() as $field => $value) { - $queryFilter[$field] = $value; - } - - $query['bool']['filter'] = [ - 'term' => $queryFilter, - ]; - } - $search = new \Elastica\Search($this->connection->getClient()); $search->addIndex('typo3content'); - $search->setQuery(new \Elastica\Query(['query' => $query])); + $search->setQuery($this->queryFactory->create($this, $searchRequest)); + // TODO: Return wrapped result to implement our interface. // Also update php doc to reflect the change. return $search->search(); diff --git a/Classes/Connection/SearchRequestInterface.php b/Classes/Connection/SearchRequestInterface.php index ecf7a74..603c02f 100644 --- a/Classes/Connection/SearchRequestInterface.php +++ b/Classes/Connection/SearchRequestInterface.php @@ -31,4 +31,14 @@ interface SearchRequestInterface * @return string */ public function getSearchTerm(); + + /** + * @return bool + */ + public function hasFilter(); + + /** + * @return array + */ + public function getFilter(); } diff --git a/Classes/Domain/Search/QueryFactory.php b/Classes/Domain/Search/QueryFactory.php new file mode 100644 index 0000000..2f3ac25 --- /dev/null +++ b/Classes/Domain/Search/QueryFactory.php @@ -0,0 +1,72 @@ + + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +use Leonmrni\SearchCore\Connection\ConnectionInterface; +use Leonmrni\SearchCore\Connection\Elasticsearch\Query; +use Leonmrni\SearchCore\Connection\SearchRequestInterface; + +class QueryFactory +{ + /** + * @param ConnectionInterface $connection + * @param SearchRequestInterface $searchRequest + * + * @return \Elastica\Query + */ + public function create( + ConnectionInterface $connection, + SearchRequestInterface $searchRequest + ) { + return $this->createElasticaQuery($searchRequest); + } + + /** + * @param SearchRequestInterface $searchRequest + * @return \Elastica\Query + */ + protected function createElasticaQuery(SearchRequestInterface $searchRequest) + { + $query = [ + 'bool' => [ + 'must' => [ + [ + 'match' => [ + '_all' => $searchRequest->getSearchTerm() + ], + ], + ], + ], + ]; + $queryFilter = []; + + if ($searchRequest->hasFilter()) { + foreach ($searchRequest->getFilter() as $field => $value) { + $queryFilter[$field] = $value; + } + $query['bool']['filter'] = [ + 'term' => $queryFilter, + ]; + } + + return new \Elastica\Query(['query' => $query]); + } +} diff --git a/Makefile b/Makefile index 2520ab0..6d058c0 100644 --- a/Makefile +++ b/Makefile @@ -23,6 +23,11 @@ functionalTests: .Build/bin/phpunit --colors --debug -v \ -c Tests/Functional/FunctionalTests.xml +unitTests: + TYPO3_PATH_WEB=$(TYPO3_WEB_DIR) \ + .Build/bin/phpunit --colors --debug -v \ + -c Tests/Unit/UnitTests.xml + uploadCodeCoverage: uploadCodeCoverageToScrutinizer uploadCodeCoverageToCodacy uploadCodeCoverageToScrutinizer: diff --git a/Tests/Functional/Connection/Elasticsearch/AbstractFunctionalTestCase.php b/Tests/Functional/Connection/Elasticsearch/AbstractFunctionalTestCase.php index cdd8ddf..363ad6d 100644 --- a/Tests/Functional/Connection/Elasticsearch/AbstractFunctionalTestCase.php +++ b/Tests/Functional/Connection/Elasticsearch/AbstractFunctionalTestCase.php @@ -43,11 +43,18 @@ abstract class AbstractFunctionalTestCase extends BaseFunctionalTestCase 'host' => getenv('ES_HOST') ?: \Elastica\Connection::DEFAULT_HOST, 'port' => getenv('ES_PORT') ?: \Elastica\Connection::DEFAULT_PORT, ]); + + $this->cleanUp(); } public function tearDown() { // Delete everything so next test starts clean. + $this->cleanUp(); + } + + protected function cleanUp() + { $this->client->getIndex('_all')->delete(); $this->client->getIndex('_all')->clearCache(); } diff --git a/Tests/Functional/Connection/Elasticsearch/FilterTest.php b/Tests/Functional/Connection/Elasticsearch/FilterTest.php new file mode 100644 index 0000000..88ae37f --- /dev/null +++ b/Tests/Functional/Connection/Elasticsearch/FilterTest.php @@ -0,0 +1,61 @@ + + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +use Leonmrni\SearchCore\Domain\Index\IndexerFactory; +use Leonmrni\SearchCore\Domain\Model\SearchRequest; +use Leonmrni\SearchCore\Domain\Search\SearchService; +use TYPO3\CMS\Extbase\Object\ObjectManager; + +class FilterTest extends AbstractFunctionalTestCase +{ + protected function getDataSets() + { + return array_merge( + parent::getDataSets(), + ['Tests/Functional/Fixtures/Searching/Filter.xml'] + ); + } + + /** + * @test + */ + public function itsPossibleToFilterResultsByASingleField() + { + \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class) + ->get(IndexerFactory::class) + ->getIndexer('tt_content') + ->indexAllDocuments() + ; + + $searchService = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class) + ->get(SearchService::class); + $searchRequest = new SearchRequest('Search Word'); + + $result = $searchService->search($searchRequest); + $this->assertSame(2, count($result), 'Did not receive both indexed elements without filter.'); + + $searchRequest->setFilter(['CType' => 'html']); + $result = $searchService->search($searchRequest); + $this->assertSame('5', $result[0]->getData()['uid'], 'Did not get the expected result entry.'); + $this->assertSame(1, count($result), 'Did not receive the single filtered element.'); + } +} diff --git a/Tests/Functional/Fixtures/Searching/Filter.xml b/Tests/Functional/Fixtures/Searching/Filter.xml new file mode 100644 index 0000000..103a4da --- /dev/null +++ b/Tests/Functional/Fixtures/Searching/Filter.xml @@ -0,0 +1,42 @@ + + + + 5 + 1 + 1480686370 + 1480686370 + 0 + 72 + html +
indexed content element with html ctype
+ Search Word + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + + + + 6 + 1 + 1480686370 + 1480686370 + 0 + 72 + header +
indexed content element with header ctype
+ Search Word + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + + diff --git a/Tests/Unit/AbstractUnitTestCase.php b/Tests/Unit/AbstractUnitTestCase.php new file mode 100644 index 0000000..f3281c1 --- /dev/null +++ b/Tests/Unit/AbstractUnitTestCase.php @@ -0,0 +1,27 @@ + + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +use TYPO3\CMS\Core\Tests\UnitTestCase as CoreTestCase; + +abstract class AbstractUnitTestCase extends CoreTestCase +{ +} diff --git a/Tests/Unit/Domain/Search/QueryFactoryTest.php b/Tests/Unit/Domain/Search/QueryFactoryTest.php new file mode 100644 index 0000000..7fc6b15 --- /dev/null +++ b/Tests/Unit/Domain/Search/QueryFactoryTest.php @@ -0,0 +1,56 @@ + + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +use Leonmrni\SearchCore\Connection; +use Leonmrni\SearchCore\Domain\Model\SearchRequest; +use Leonmrni\SearchCore\Domain\Search\QueryFactory; +use Leonmrni\SearchCore\Tests\Unit\AbstractUnitTestCase; + +class QueryFactoryTest extends AbstractUnitTestCase +{ + protected $subject; + + public function setUp() + { + parent::setUp(); + + $this->subject = new QueryFactory; + } + + /** + * @test + */ + public function creatonOfQueryWorksInGeneral() + { + $connection = $this->getMockBuilder(Connection\Elasticsearch::class) + ->disableOriginalConstructor() + ->getMock(); + $searchRequest = new SearchRequest('SearchWord'); + + $query = $this->subject->create($connection, $searchRequest); + $this->assertInstanceOf( + \Elastica\Query::class, + $query, + 'Factory did not create the expected instance.' + ); + } +} diff --git a/Tests/Unit/UnitTests.xml b/Tests/Unit/UnitTests.xml new file mode 100644 index 0000000..6456405 --- /dev/null +++ b/Tests/Unit/UnitTests.xml @@ -0,0 +1,28 @@ + + + + + . + + + + + + ../../Classes + + + From bb9e29574f84e16a81e2888a78fb60e41f63b8a4 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Jun 2017 14:25:03 +0200 Subject: [PATCH 4/7] TASK: Use elasticsearch 5.2 on travis * As we make use of newer features that are incompatible with travis own elasticsearch version 1.x. --- .travis.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 123363a..03a193c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,3 +1,12 @@ +sudo: true + +addons: + apt: + packages: + - oracle-java8-set-default +before_install: + - curl -O https://artifacts.elastic.co/downloads/elasticsearch/elasticsearch-5.2.0.deb && sudo dpkg -i --force-confnew elasticsearch-5.2.0.deb && sudo service elasticsearch start + language: php php: @@ -49,7 +58,6 @@ matrix: services: - mysql - - elasticsearch install: make install From f5729c276325e8d8696eb8a51bd0cc80b1f60bc5 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Jun 2017 15:33:06 +0200 Subject: [PATCH 5/7] BUGFIX: Keep return type * Return boolean type. --- Classes/Domain/Model/SearchRequest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Classes/Domain/Model/SearchRequest.php b/Classes/Domain/Model/SearchRequest.php index 105cc45..b88c866 100644 --- a/Classes/Domain/Model/SearchRequest.php +++ b/Classes/Domain/Model/SearchRequest.php @@ -76,7 +76,7 @@ class SearchRequest implements SearchRequestInterface */ public function hasFilter() { - return count($this->filter); + return count($this->filter) > 0; } /** From f4a9531fe59df1e40b6090342121e3b6664fee59 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 6 Jun 2017 16:25:35 +0200 Subject: [PATCH 6/7] TASK: Remove unnecessary code * As filter is already in the format we need, we can just use it instead of using a foreach. --- Classes/Domain/Search/QueryFactory.php | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/Classes/Domain/Search/QueryFactory.php b/Classes/Domain/Search/QueryFactory.php index 2f3ac25..31c50df 100644 --- a/Classes/Domain/Search/QueryFactory.php +++ b/Classes/Domain/Search/QueryFactory.php @@ -56,15 +56,9 @@ class QueryFactory ], ], ]; - $queryFilter = []; if ($searchRequest->hasFilter()) { - foreach ($searchRequest->getFilter() as $field => $value) { - $queryFilter[$field] = $value; - } - $query['bool']['filter'] = [ - 'term' => $queryFilter, - ]; + $query['bool']['filter'] = ['term' => $searchRequest->getFilter()]; } return new \Elastica\Query(['query' => $query]); From f453592b39158e41a2f46121253e0755de1b618c Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 8 Jun 2017 08:38:14 +0200 Subject: [PATCH 7/7] TASK: Add further tests and cast search input * Map user input to string in any case. * Add tests to check whether filter is added to query. * Add test to check whether input is casted to string. --- Classes/Domain/Model/SearchRequest.php | 4 +- Tests/Unit/Domain/Search/QueryFactoryTest.php | 43 +++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/Classes/Domain/Model/SearchRequest.php b/Classes/Domain/Model/SearchRequest.php index b88c866..f1ad18e 100644 --- a/Classes/Domain/Model/SearchRequest.php +++ b/Classes/Domain/Model/SearchRequest.php @@ -44,7 +44,7 @@ class SearchRequest implements SearchRequestInterface */ public function __construct($query) { - $this->query = $query; + $this->query = (string) $query; } /** @@ -68,7 +68,7 @@ class SearchRequest implements SearchRequestInterface */ public function setFilter(array $filter) { - $this->filter = $filter; + $this->filter = array_map('strval', $filter); } /** diff --git a/Tests/Unit/Domain/Search/QueryFactoryTest.php b/Tests/Unit/Domain/Search/QueryFactoryTest.php index 7fc6b15..83f031e 100644 --- a/Tests/Unit/Domain/Search/QueryFactoryTest.php +++ b/Tests/Unit/Domain/Search/QueryFactoryTest.php @@ -53,4 +53,47 @@ class QueryFactoryTest extends AbstractUnitTestCase 'Factory did not create the expected instance.' ); } + + /** + * @test + */ + public function filterIsAddedToQuery() + { + $connection = $this->getMockBuilder(Connection\Elasticsearch::class) + ->disableOriginalConstructor() + ->getMock(); + $searchRequest = new SearchRequest('SearchWord'); + $searchRequest->setFilter(['field' => 'content']); + + $query = $this->subject->create($connection, $searchRequest); + $this->assertSame( + ['field' => 'content'], + $query->toArray()['query']['bool']['filter']['term'], + 'Filter was not added to query.' + ); + } + + /** + * @test + */ + public function userInputIsAlwaysString() + { + $connection = $this->getMockBuilder(Connection\Elasticsearch::class) + ->disableOriginalConstructor() + ->getMock(); + $searchRequest = new SearchRequest(10); + $searchRequest->setFilter(['field' => 20]); + + $query = $this->subject->create($connection, $searchRequest); + $this->assertSame( + '10', + $query->toArray()['query']['bool']['must'][0]['match']['_all'], + 'Search word was not escaped as expected.' + ); + $this->assertSame( + '20', + $query->toArray()['query']['bool']['filter']['term']['field'], + 'Search word was not escaped as expected.' + ); + } }