From 203b70898be752f0f8cbd17f0b1b873a4c3e86cb Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 15 Dec 2016 11:31:48 +0100 Subject: [PATCH 1/3] TASK: Restructure tests * As introduces through review, the tests are working and have huge code coverage, but don't test what they say. Therefore we reorder them in new structure, to have new tests in clean structure. --- .../Functional/AbstractFunctionalTestCase.php | 29 +++++----- .../AbstractFunctionalTestCase.php | 54 +++++++++++++++++++ .../Elasticsearch}/IndexTcaTableTest.php | 5 +- Tests/Functional/Hooks/DataHandlerTest.php | 7 ++- 4 files changed, 75 insertions(+), 20 deletions(-) create mode 100644 Tests/Functional/Connection/Elasticsearch/AbstractFunctionalTestCase.php rename Tests/Functional/{Indexing => Connection/Elasticsearch}/IndexTcaTableTest.php (97%) diff --git a/Tests/Functional/AbstractFunctionalTestCase.php b/Tests/Functional/AbstractFunctionalTestCase.php index b9b6dfb..5c253be 100644 --- a/Tests/Functional/AbstractFunctionalTestCase.php +++ b/Tests/Functional/AbstractFunctionalTestCase.php @@ -24,18 +24,22 @@ use TYPO3\CMS\Core\Tests\FunctionalTestCase as CoreTestCase; /** * All functional tests should extend this base class. - * - * It will take care of leaving a clean environment for next test. - * TODO: this is in reality an "elastica" abstract case - not search_core ;) */ abstract class AbstractFunctionalTestCase extends CoreTestCase { protected $testExtensionsToLoad = ['typo3conf/ext/search_core']; /** - * @var \Elastica\Client + * Define whether to setup default typoscript on page 1. + * + * Set to false if you need to add further ts and use getDefaultPageTs to get the default one. + * + * This is necessary as setUpFrontendRootPage will allways add a new record + * and only the first one is used. + * + * @var bool */ - protected $client; + protected $loadDefaultTs = true; public function setUp() { @@ -46,19 +50,14 @@ abstract class AbstractFunctionalTestCase extends CoreTestCase // Provide necessary configuration for extension $this->importDataSet('Tests/Functional/Fixtures/BasicSetup.xml'); - $this->setUpFrontendRootPage(1, ['EXT:search_core/Tests/Functional/Fixtures/BasicSetup.ts']); - // Create client to make requests and assert something. - $this->client = new \Elastica\Client([ - 'host' => getenv('ES_HOST') ?: \Elastica\Connection::DEFAULT_HOST, - 'port' => getenv('ES_PORT') ?: \Elastica\Connection::DEFAULT_PORT, - ]); + if ($this->loadDefaultTs) { + $this->setUpFrontendRootPage(1, $this->getDefaultPageTs()); + } } - public function tearDown() + protected function getDefaultPageTs() { - // Delete everything so next test starts clean. - $this->client->getIndex('_all')->delete(); - $this->client->getIndex('_all')->clearCache(); + return ['EXT:search_core/Tests/Functional/Fixtures/BasicSetup.ts']; } } diff --git a/Tests/Functional/Connection/Elasticsearch/AbstractFunctionalTestCase.php b/Tests/Functional/Connection/Elasticsearch/AbstractFunctionalTestCase.php new file mode 100644 index 0000000..cdd8ddf --- /dev/null +++ b/Tests/Functional/Connection/Elasticsearch/AbstractFunctionalTestCase.php @@ -0,0 +1,54 @@ + + * + * 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\Tests\Functional\AbstractFunctionalTestCase as BaseFunctionalTestCase; + +/** + * All functional tests should extend this base class. + * + * It will take care of leaving a clean environment for next test. + */ +abstract class AbstractFunctionalTestCase extends BaseFunctionalTestCase +{ + /** + * @var \Elastica\Client + */ + protected $client; + + public function setUp() + { + parent::setUp(); + + // Create client to make requests and assert something. + $this->client = new \Elastica\Client([ + 'host' => getenv('ES_HOST') ?: \Elastica\Connection::DEFAULT_HOST, + 'port' => getenv('ES_PORT') ?: \Elastica\Connection::DEFAULT_PORT, + ]); + } + + public function tearDown() + { + // Delete everything so next test starts clean. + $this->client->getIndex('_all')->delete(); + $this->client->getIndex('_all')->clearCache(); + } +} diff --git a/Tests/Functional/Indexing/IndexTcaTableTest.php b/Tests/Functional/Connection/Elasticsearch/IndexTcaTableTest.php similarity index 97% rename from Tests/Functional/Indexing/IndexTcaTableTest.php rename to Tests/Functional/Connection/Elasticsearch/IndexTcaTableTest.php index 6c08a50..30d55f9 100644 --- a/Tests/Functional/Indexing/IndexTcaTableTest.php +++ b/Tests/Functional/Connection/Elasticsearch/IndexTcaTableTest.php @@ -1,5 +1,5 @@ @@ -21,11 +21,10 @@ namespace Leonmrni\SearchCore\Tests\Functional\Indexing; */ use Leonmrni\SearchCore\Domain\Index\IndexerFactory; -use Leonmrni\SearchCore\Tests\Functional\AbstractFunctionalTestCase; use TYPO3\CMS\Extbase\Object\ObjectManager; /** - * + * TODO: https://github.com/DanielSiepmann/search_core/issues/16 */ class IndexTcaTableTest extends AbstractFunctionalTestCase { diff --git a/Tests/Functional/Hooks/DataHandlerTest.php b/Tests/Functional/Hooks/DataHandlerTest.php index 60c0115..7139934 100644 --- a/Tests/Functional/Hooks/DataHandlerTest.php +++ b/Tests/Functional/Hooks/DataHandlerTest.php @@ -21,12 +21,15 @@ namespace Leonmrni\SearchCore\Tests\Functional\Hooks; */ use Leonmrni\SearchCore\Hook\DataHandler as Hook; -use Leonmrni\SearchCore\Tests\Functional\AbstractFunctionalTestCase; +use Leonmrni\SearchCore\Tests\Functional\Connection\Elasticsearch\AbstractFunctionalTestCase; use TYPO3\CMS\Core\DataHandling\DataHandler as CoreDataHandler; use TYPO3\CMS\Extbase\Object\ObjectManager; /** - * + * TODO: Rewrite as this test doesn't test what it should do. + * We have to split it up in two tests: + * 1. Test whether TYPO3 DataHandler will our hook as expected. + * 2. Test whether our hook will send the documents to connection as expected. */ class DataHandlerTest extends AbstractFunctionalTestCase { From 499b9d05004014df92ea90ff9afd98970057f8ca Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 15 Dec 2016 11:32:41 +0100 Subject: [PATCH 2/3] FEATURE: Add new configuration to blacklist records by root line * Allow configuration through TypoScript to exclude records for indexing based on their root line position. Page uids can be configured for exclusion and all records beneath will be excluded while indexing. --- Classes/Domain/Index/TcaIndexer.php | 1 + .../Index/TcaIndexer/TcaTableService.php | 71 ++++++++++++- .../TcaIndexer/RespectRootLineBlacklist.ts | 11 ++ .../TcaIndexer/RespectRootLineBlacklist.xml | 100 ++++++++++++++++++ Tests/Functional/Indexing/TcaIndexerTest.php | 86 +++++++++++++++ 5 files changed, 268 insertions(+), 1 deletion(-) create mode 100644 Tests/Functional/Fixtures/Indexing/TcaIndexer/RespectRootLineBlacklist.ts create mode 100644 Tests/Functional/Fixtures/Indexing/TcaIndexer/RespectRootLineBlacklist.xml create mode 100644 Tests/Functional/Indexing/TcaIndexerTest.php diff --git a/Classes/Domain/Index/TcaIndexer.php b/Classes/Domain/Index/TcaIndexer.php index 229f9e8..0d92abc 100644 --- a/Classes/Domain/Index/TcaIndexer.php +++ b/Classes/Domain/Index/TcaIndexer.php @@ -116,6 +116,7 @@ class TcaIndexer implements IndexerInterface '', (int) $offset . ',' . (int) $limit ); + $this->tcaTableService->filterRecordsByRootLineBlacklist($records); foreach ($records as &$record) { $this->tcaTableService->prepareRecord($record); diff --git a/Classes/Domain/Index/TcaIndexer/TcaTableService.php b/Classes/Domain/Index/TcaIndexer/TcaTableService.php index cc808dc..9540c55 100644 --- a/Classes/Domain/Index/TcaIndexer/TcaTableService.php +++ b/Classes/Domain/Index/TcaIndexer/TcaTableService.php @@ -23,6 +23,7 @@ namespace Leonmrni\SearchCore\Domain\Index\TcaIndexer; use Leonmrni\SearchCore\Configuration\ConfigurationContainerInterface; use Leonmrni\SearchCore\Domain\Index\IndexingException; use TYPO3\CMS\Backend\Utility\BackendUtility; +use TYPO3\CMS\Core\Utility\GeneralUtility; /** * Encapsulate logik related to TCA configuration. @@ -104,6 +105,22 @@ class TcaTableService return $this->tableName . ' LEFT JOIN pages on ' . $this->tableName . '.pid = pages.uid'; } + /** + * Filter the given records by root line blacklist settings. + * + * @param array &$records + * @return void + */ + public function filterRecordsByRootLineBlacklist(array &$records) + { + $records = array_filter( + $records, + function ($record) { + return ! $this->isRecordBlacklistedByRootline($record); + } + ); + } + /** * Adjust record accordingly to configuration. * @param array &$record @@ -125,7 +142,7 @@ class TcaTableService */ public function getWhereClause() { - $whereClause = '1=1 ' + $whereClause = '1=1' . BackendUtility::BEenableFields($this->tableName) . BackendUtility::deleteClause($this->tableName) @@ -139,6 +156,15 @@ class TcaTableService $whereClause .= $userDefinedWhere; } + if ($this->isBlacklistedRootLineConfigured()) { + $whereClause .= ' AND pages.uid NOT IN (' + . implode(',', $this->getBlacklistedRootLine()) + . ')' + . ' AND pages.pid NOT IN (' + . implode(',', $this->getBlacklistedRootLine()) + . ')'; + } + $this->logger->debug('Generated where clause.', [$this->tableName, $whereClause]); return $whereClause; } @@ -206,4 +232,47 @@ class TcaTableService return $this->tca['columns'][$columnName]['config']; } + /** + * Checks whether the given record was blacklisted by root line. + * This can be configured by typoscript as whole root lines can be black listed. + * + * @param array &$record + * @return bool + */ + protected function isRecordBlacklistedByRootline(array &$record) + { + // NOTE: Does not support pages yet. We have to add a switch once we + // support them to use uid instead. + if (! $this->isBlackListedRootLineConfigured()) { + return false; + } + + foreach (BackendUtility::BEgetRootLine($record['uid']) as $pageInRootLine) { + if (in_array($pageInRootLine['uid'], $this->getBlackListedRootLine())) { + return true; + } + } + + return false; + } + + /** + * Checks whether any page uids are black listed. + * + * @return bool + */ + protected function isBlackListedRootLineConfigured() + { + return (bool) $this->configuration->getIfExists('index', 'rootLineBlacklist'); + } + + /** + * Get the list of black listed root line page uids. + * + * @return array + */ + protected function getBlackListedRootLine() + { + return GeneralUtility::intExplode(',', $this->configuration->getIfExists('index', 'rootLineBlacklist')); + } } diff --git a/Tests/Functional/Fixtures/Indexing/TcaIndexer/RespectRootLineBlacklist.ts b/Tests/Functional/Fixtures/Indexing/TcaIndexer/RespectRootLineBlacklist.ts new file mode 100644 index 0000000..4c134e6 --- /dev/null +++ b/Tests/Functional/Fixtures/Indexing/TcaIndexer/RespectRootLineBlacklist.ts @@ -0,0 +1,11 @@ +plugin { + tx_searchcore { + settings { + index { + rootLineBlacklist = 3 + } + } + } +} + +module.tx_searchcore < plugin.tx_searchcore diff --git a/Tests/Functional/Fixtures/Indexing/TcaIndexer/RespectRootLineBlacklist.xml b/Tests/Functional/Fixtures/Indexing/TcaIndexer/RespectRootLineBlacklist.xml new file mode 100644 index 0000000..0e9cbda --- /dev/null +++ b/Tests/Functional/Fixtures/Indexing/TcaIndexer/RespectRootLineBlacklist.xml @@ -0,0 +1,100 @@ + + + + 2 + 1 + Content here will be indexed + + + + 3 + 2 + Content here will not be indexed + + + + 4 + 3 + Content here will not be indexed either + + + + 1 + 1 + 1480686370 + 1480686370 + 0 + 72 + textmedia +
indexed content element
+ this is the content of textmedia content element that should get indexed + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + + + + 2 + 2 + 1480686371 + 1480686370 + 0 + 72 + textmedia +
indexed content element
+ this is the content of textmedia content element that should get indexed + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + + + + 3 + 3 + 1480686371 + 1480686370 + 0 + 72 + textmedia +
This content should not be indexed due to root line contraints
+ + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + + + + 4 + 4 + 1480686371 + 1480686370 + 0 + 72 + textmedia +
This content should not be indexed due to root line contraints
+ + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + + diff --git a/Tests/Functional/Indexing/TcaIndexerTest.php b/Tests/Functional/Indexing/TcaIndexerTest.php new file mode 100644 index 0000000..1610cc0 --- /dev/null +++ b/Tests/Functional/Indexing/TcaIndexerTest.php @@ -0,0 +1,86 @@ + + * + * 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\Configuration\ConfigurationContainerInterface; +use Leonmrni\SearchCore\Connection\Elasticsearch; +use Leonmrni\SearchCore\Domain\Index\TcaIndexer; +use Leonmrni\SearchCore\Domain\Index\TcaIndexer\RelationResolver; +use Leonmrni\SearchCore\Domain\Index\TcaIndexer\TcaTableService; +use Leonmrni\SearchCore\Tests\Functional\AbstractFunctionalTestCase; +use TYPO3\CMS\Extbase\Object\ObjectManager; + +class TcaIndexerTest extends AbstractFunctionalTestCase +{ + protected $loadDefaultTs = false; + + /** + * @test + */ + public function respectRootLineBlacklist() + { + $this->importDataSet('Tests/Functional/Fixtures/Indexing/TcaIndexer/RespectRootLineBlacklist.xml'); + $this->setUpFrontendRootPage( + 1, + array_merge( + $this->getDefaultPageTs(), + ['EXT:search_core/Tests/Functional/Fixtures/Indexing/TcaIndexer/RespectRootLineBlacklist.ts'] + ) + ); + + $objectManager = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class); + $tableName = 'tt_content'; + $tableService = $objectManager->get( + TcaTableService::class, + $tableName, + $objectManager->get(RelationResolver::class), + $objectManager->get(ConfigurationContainerInterface::class) + ); + $connection = $this->getAccessibleMock( + Elasticsearch::class, + [ + 'addDocuments', + ], + [], + '', + false + ); + + $connection->expects($this->once()) + ->method('addDocuments') + ->with( + $this->stringContains('tt_content'), + $this->callback(function ($documents) { + foreach ($documents as $document) { + // Page uids 1 and 2 are allowed while 3 and 4 are not allowed. + // Therefore only documents with page uid 1 and 2 should exist. + if (! isset($document['pid']) || ! in_array($document['pid'], [1, 2])) { + return false; + } + } + + return true; + }) + ); + + $objectManager->get(TcaIndexer::class, $tableService, $connection)->indexAllDocuments(); + } +} From 1486799a024a05125351556ecc1f8b9b7f61d029 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 15 Dec 2016 11:41:45 +0100 Subject: [PATCH 3/3] CLEANUP: Code Style Issues --- Classes/Connection/Elasticsearch.php | 2 +- Classes/Domain/Index/TcaIndexer/TcaTableService.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Classes/Connection/Elasticsearch.php b/Classes/Connection/Elasticsearch.php index 42c4359..4cce11b 100644 --- a/Classes/Connection/Elasticsearch.php +++ b/Classes/Connection/Elasticsearch.php @@ -94,7 +94,7 @@ class Elasticsearch implements Singleton, ConnectionInterface { $this->withType( $documentType, - function ($type) use($identifier) { + function ($type) use ($identifier) { $type->deleteById($identifier); } ); diff --git a/Classes/Domain/Index/TcaIndexer/TcaTableService.php b/Classes/Domain/Index/TcaIndexer/TcaTableService.php index 9540c55..0f963c0 100644 --- a/Classes/Domain/Index/TcaIndexer/TcaTableService.php +++ b/Classes/Domain/Index/TcaIndexer/TcaTableService.php @@ -263,7 +263,7 @@ class TcaTableService */ protected function isBlackListedRootLineConfigured() { - return (bool) $this->configuration->getIfExists('index', 'rootLineBlacklist'); + return (bool) $this->configuration->getIfExists('index', 'rootLineBlacklist'); } /**