From 24deb93b4f3b8d469c4c4e22e8390d6fa780d37d Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 12 Jan 2017 14:07:23 +0100 Subject: [PATCH 1/7] BUGFIX: Don't break type if settings are not available * E.g. if no TypoScript is included we still should have an array to not break method calls. --- Classes/Configuration/ConfigurationContainer.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Classes/Configuration/ConfigurationContainer.php b/Classes/Configuration/ConfigurationContainer.php index 56de674..fee781c 100644 --- a/Classes/Configuration/ConfigurationContainer.php +++ b/Classes/Configuration/ConfigurationContainer.php @@ -48,6 +48,9 @@ class ConfigurationContainer implements ConfigurationContainerInterface 'SearchCore', 'search' ); + if ($this->settings === null) { + $this->settings = []; + } } /** From 1878209b51ce88602de9e86d1d03963b1714d6c6 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 12 Jan 2017 14:21:41 +0100 Subject: [PATCH 2/7] BUGFIX: Handle missing configuration in hook * Don't break if no configuration exists, instead improve logging. --- .../Configuration/ConfigurationContainer.php | 3 +- Classes/Hook/DataHandler.php | 38 +++++++++++++++---- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/Classes/Configuration/ConfigurationContainer.php b/Classes/Configuration/ConfigurationContainer.php index fee781c..0ed0955 100644 --- a/Classes/Configuration/ConfigurationContainer.php +++ b/Classes/Configuration/ConfigurationContainer.php @@ -40,6 +40,7 @@ class ConfigurationContainer implements ConfigurationContainerInterface * Inject settings via ConfigurationManager. * * @param ConfigurationManagerInterface $configurationManager + * @throws NoConfigurationException */ public function injectConfigurationManager(ConfigurationManagerInterface $configurationManager) { @@ -49,7 +50,7 @@ class ConfigurationContainer implements ConfigurationContainerInterface 'search' ); if ($this->settings === null) { - $this->settings = []; + throw new NoConfigurationException('Could not fetch configuration.', 1484226842); } } diff --git a/Classes/Hook/DataHandler.php b/Classes/Hook/DataHandler.php index 0c102ab..1bcb4a5 100644 --- a/Classes/Hook/DataHandler.php +++ b/Classes/Hook/DataHandler.php @@ -20,6 +20,8 @@ namespace Leonmrni\SearchCore\Hook; * 02110-1301, USA. */ +use Leonmrni\SearchCore\Configuration\NoConfigurationException; +use Leonmrni\SearchCore\Domain\Service\DataHandler as OwnDataHandler; use TYPO3\CMS\Backend\Utility\BackendUtility; use TYPO3\CMS\Core\DataHandling\DataHandler as CoreDataHandler; use TYPO3\CMS\Core\Log\LogManager; @@ -27,7 +29,6 @@ use TYPO3\CMS\Core\Log\Logger; use TYPO3\CMS\Core\SingletonInterface as Singleton; use TYPO3\CMS\Core\Utility\GeneralUtility; use TYPO3\CMS\Extbase\Object\ObjectManager; -use Leonmrni\SearchCore\Domain\Service\DataHandler as OwnDataHandler; /** * Wrapper for TYPO3 Hooks to internal API. @@ -55,8 +56,13 @@ class DataHandler implements Singleton { $this->dataHandler = $dataHandler; if ($this->dataHandler === null) { - $this->dataHandler = GeneralUtility::makeInstance(ObjectManager::class) - ->get(OwnDataHandler::class); + try { + $this->dataHandler = GeneralUtility::makeInstance(ObjectManager::class) + ->get(OwnDataHandler::class); + } catch (NoConfigurationException $e) { + // We have no configuration. That's fine, hooks will not be + // executed due to check for existing DataHandler. + } } $this->logger = $logger; @@ -76,8 +82,8 @@ class DataHandler implements Singleton */ public function processCmdmap_deleteAction($table, $uid) { - if (! $this->shouldProcessTable($table)) { - $this->logger->debug('Delete not processed, cause table is not allowed.', [$table]); + if (! $this->shouldProcessHookForTable($table)) { + $this->logger->debug('Delete not processed.', [$table, $uid]); return false; } @@ -98,8 +104,8 @@ class DataHandler implements Singleton */ public function processDatamap_afterDatabaseOperations($status, $table, $uid, array $fieldArray, CoreDataHandler $dataHandler) { - if (! $this->shouldProcessTable($table)) { - $this->logger->debug('Database update not processed, cause table is not allowed.', [$table]); + if (! $this->shouldProcessHookForTable($table)) { + $this->logger->debug('Database update not processed.', [$table, $uid]); return false; } @@ -124,6 +130,24 @@ class DataHandler implements Singleton return false; } + /** + * @param string $table + * @return bool + */ + protected function shouldProcessHookForTable($table) + { + if ($this->dataHandler === null) { + $this->logger->debug('Datahandler could not be setup.'); + return false; + } + if (! $this->shouldProcessTable($table)) { + $this->logger->debug('Table is not allowed.', [$table]); + return false; + } + + return true; + } + /** * @param string $table * @return bool From d358714d0d5b89cbbc4a517f9dcf5a657e68df63 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 12 Jan 2017 14:26:09 +0100 Subject: [PATCH 3/7] BUGFIX: Handle non available records during indexing * E.g. a hidden record is edited. --- Classes/Domain/Index/TcaIndexer.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Classes/Domain/Index/TcaIndexer.php b/Classes/Domain/Index/TcaIndexer.php index 0d92abc..9592b65 100644 --- a/Classes/Domain/Index/TcaIndexer.php +++ b/Classes/Domain/Index/TcaIndexer.php @@ -82,7 +82,11 @@ class TcaIndexer implements IndexerInterface public function indexDocument($identifier) { $this->logger->info('Start indexing single record.', [$identifier]); - $this->connection->addDocument($this->tcaTableService->getTableName(), $this->getRecord($identifier)); + try { + $this->connection->addDocument($this->tcaTableService->getTableName(), $this->getRecord($identifier)); + } catch (NoRecordFoundException $e) { + $this->logger->info('Could not index document.', [$e->getMessage()]); + } $this->logger->info('Finish indexing'); } @@ -128,6 +132,7 @@ class TcaIndexer implements IndexerInterface /** * @param int $identifier * @return array + * @throws NoRecordFoundException If record could not be found. */ protected function getRecord($identifier) { @@ -137,6 +142,13 @@ class TcaIndexer implements IndexerInterface $this->tcaTableService->getWhereClause() . ' AND ' . $this->tcaTableService->getTableName() . '.uid = ' . (int) $identifier ); + + if ($record === false) { + throw new NoRecordFoundException( + 'Record could not be fetched from database: "' . $identifier . '". Perhaps record is not active.', + 1484225364 + ); + } $this->tcaTableService->prepareRecord($record); return $record; From 8448618f60756bcdc22952f93b6600c3c83b3f31 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Tue, 17 Jan 2017 09:22:41 +0100 Subject: [PATCH 4/7] FIX: Catch deletion of non existing documents * Catch exception raised during deletion of documents that are not already indexed. * Log failed deletion tries. * Add new necessary Exceptions. --- .../NoConfigurationException.php | 25 +++++++++++++++++++ Classes/Connection/Elasticsearch.php | 16 +++++++----- .../Domain/Index/NoRecordFoundException.php | 25 +++++++++++++++++++ 3 files changed, 60 insertions(+), 6 deletions(-) create mode 100755 Classes/Configuration/NoConfigurationException.php create mode 100755 Classes/Domain/Index/NoRecordFoundException.php diff --git a/Classes/Configuration/NoConfigurationException.php b/Classes/Configuration/NoConfigurationException.php new file mode 100755 index 0000000..bc6c1ba --- /dev/null +++ b/Classes/Configuration/NoConfigurationException.php @@ -0,0 +1,25 @@ + + * + * 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. + */ + +class NoConfigurationException extends \InvalidArgumentException +{ +} diff --git a/Classes/Connection/Elasticsearch.php b/Classes/Connection/Elasticsearch.php index 4cce11b..560ca07 100644 --- a/Classes/Connection/Elasticsearch.php +++ b/Classes/Connection/Elasticsearch.php @@ -92,12 +92,16 @@ class Elasticsearch implements Singleton, ConnectionInterface public function deleteDocument($documentType, $identifier) { - $this->withType( - $documentType, - function ($type) use ($identifier) { - $type->deleteById($identifier); - } - ); + try { + $this->withType( + $documentType, + function ($type) use ($identifier) { + $type->deleteById($identifier); + } + ); + } catch (\Elastica\Exception\NotFoundException $exception) { + $this->logger->debug('Tried to delete document in index, which does not exist.', [$documentType, $identifier]); + } } public function updateDocument($documentType, array $document) diff --git a/Classes/Domain/Index/NoRecordFoundException.php b/Classes/Domain/Index/NoRecordFoundException.php new file mode 100755 index 0000000..8a92408 --- /dev/null +++ b/Classes/Domain/Index/NoRecordFoundException.php @@ -0,0 +1,25 @@ + + * + * 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. + */ + +class NoRecordFoundException extends IndexingException +{ +} From 8e062d6e42813821f9ed1438665588df1abb83cf Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 26 Jan 2017 14:37:17 +0100 Subject: [PATCH 5/7] BUGFIX: Respect additionalWhereClause and rootline * Use constants instead of setup. * Grap configuration from path. * Add where clause. --- .../NoConfigurationException.php | 0 .../Domain/Index/NoRecordFoundException.php | 0 .../Index/TcaIndexer/TcaTableService.php | 24 +++++++++------ Configuration/TypoScript/constants.txt | 29 +++++++++++++++++++ Configuration/TypoScript/setup.txt | 12 +++----- Documentation/source/configuration.rst | 12 ++++---- 6 files changed, 55 insertions(+), 22 deletions(-) mode change 100755 => 100644 Classes/Configuration/NoConfigurationException.php mode change 100755 => 100644 Classes/Domain/Index/NoRecordFoundException.php mode change 100644 => 100755 Configuration/TypoScript/constants.txt mode change 100644 => 100755 Configuration/TypoScript/setup.txt diff --git a/Classes/Configuration/NoConfigurationException.php b/Classes/Configuration/NoConfigurationException.php old mode 100755 new mode 100644 diff --git a/Classes/Domain/Index/NoRecordFoundException.php b/Classes/Domain/Index/NoRecordFoundException.php old mode 100755 new mode 100644 diff --git a/Classes/Domain/Index/TcaIndexer/TcaTableService.php b/Classes/Domain/Index/TcaIndexer/TcaTableService.php index 3f2d697..936d425 100644 --- a/Classes/Domain/Index/TcaIndexer/TcaTableService.php +++ b/Classes/Domain/Index/TcaIndexer/TcaTableService.php @@ -151,9 +151,9 @@ class TcaTableService . ' AND pages.no_search = 0' ; - $userDefinedWhere = $this->configuration->getIfExists('indexer.tca.' . $this->tableName); + $userDefinedWhere = $this->configuration->getIfExists('indexer.tca.' . $this->tableName . '.additionalWhereClause'); if (is_string($userDefinedWhere)) { - $whereClause .= $userDefinedWhere; + $whereClause .= ' AND ' . $userDefinedWhere; } if ($this->isBlacklistedRootLineConfigured()) { @@ -236,20 +236,26 @@ class TcaTableService * Checks whether the given record was blacklisted by root line. * This can be configured by typoscript as whole root lines can be black listed. * + * NOTE: Does not support pages yet. We have to add a switch once we + * support them to use uid instead. + * * @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; + // If no rootline exists, the record is on a unreachable page and therefore blacklisted. + $rootline = BackendUtility::BEgetRootLine($record['pid']); + if (!isset($rootline[0])) { + return true; } - foreach (BackendUtility::BEgetRootLine($record['uid']) as $pageInRootLine) { - if (in_array($pageInRootLine['uid'], $this->getBlackListedRootLine())) { - return true; + // Check configured black list if present. + if ($this->isBlackListedRootLineConfigured()) { + foreach ($rootline as $pageInRootLine) { + if (in_array($pageInRootLine['uid'], $this->getBlackListedRootLine())) { + return true; + } } } diff --git a/Configuration/TypoScript/constants.txt b/Configuration/TypoScript/constants.txt old mode 100644 new mode 100755 index e69de29..50f57c3 --- a/Configuration/TypoScript/constants.txt +++ b/Configuration/TypoScript/constants.txt @@ -0,0 +1,29 @@ +plugin { + tx_searchcore { + settings { + connections { + elasticsearch { + host = localhost + port = 9200 + } + } + + indexer { + tca { + # Pages are not supported yet, see + # https://github.com/DanielSiepmann/search_core/issues/24 but + # should also be added, together with additionalWhereClause + # based on doktypes + allowedTables = tt_content + + tt_content { + additionalWhereClause ( + pages.doktype NOT IN (3, 199) + AND tt_content.CType NOT IN ('gridelements_pi1', 'list', 'div', 'menu', 'shortcut', 'search', 'login') + ) + } + } + } + } + } +} diff --git a/Configuration/TypoScript/setup.txt b/Configuration/TypoScript/setup.txt old mode 100644 new mode 100755 index 0e5bd20..afa15be --- a/Configuration/TypoScript/setup.txt +++ b/Configuration/TypoScript/setup.txt @@ -3,21 +3,17 @@ plugin { settings { connections { elasticsearch { - host = localhost - port = 9200 + host = {$plugin.tx_searchcore.settings.connections.elasticsearch.host} + port = {$plugin.tx_searchcore.settings.connections.elasticsearch.port} } } indexer { tca { - # Pages are not supported yet, see - # https://github.com/DanielSiepmann/search_core/issues/24 but - # should also be added, together with additionalWhereClause - # based on doktypes - allowedTables = tt_content + allowedTables = {$plugin.tx_searchcore.settings.indexer.tca.allowedTables} tt_content { - additionalWhereClause = tt_content.CType NOT IN ('gridelements_pi1', 'list', 'div', 'menu') + additionalWhereClause = {$plugin.tx_searchcore.settings.indexer.tca.tt_content.additionalWhereClause} } } } diff --git a/Documentation/source/configuration.rst b/Documentation/source/configuration.rst index b41a0d1..fb51d11 100644 --- a/Documentation/source/configuration.rst +++ b/Documentation/source/configuration.rst @@ -15,14 +15,16 @@ mode of TYPO3. Do so by placing the following line at the end:: We will use references inside the extension to make the above unnecessary in the future. -Currently no constants are available, but this will change in the near future to make configuration -easier. - The structure is following TYPO3 Extbase conventions. All settings are placed inside of:: plugin.tx_searchcore.settings -Here is the example default configuration that's provided through static setup: +Here is the example default configuration that's provided through static include: + +.. literalinclude:: ../../Configuration/TypoScript/constants.txt + :language: typoscript + :linenos: + :caption: Static TypoScript Constants .. literalinclude:: ../../Configuration/TypoScript/setup.txt :language: typoscript @@ -145,7 +147,7 @@ The following settings are available. For each setting its documented which inde The page attribute *No Search* is also taken into account to prevent indexing records from only one page without recursion. - Contains a comma separated list of table names. Spaces are trimmed. + Contains a comma separated list of page uids. Spaces are trimmed. Example:: From 8bea39217edad5ae337c9d514965696e026f4bd6 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 26 Jan 2017 14:57:23 +0100 Subject: [PATCH 6/7] BUGFIX: Don't break if no more records exist --- Classes/Domain/Index/TcaIndexer.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Classes/Domain/Index/TcaIndexer.php b/Classes/Domain/Index/TcaIndexer.php index 9592b65..5cf6988 100644 --- a/Classes/Domain/Index/TcaIndexer.php +++ b/Classes/Domain/Index/TcaIndexer.php @@ -120,8 +120,11 @@ class TcaIndexer implements IndexerInterface '', (int) $offset . ',' . (int) $limit ); - $this->tcaTableService->filterRecordsByRootLineBlacklist($records); + if ($records === null) { + return null; + } + $this->tcaTableService->filterRecordsByRootLineBlacklist($records); foreach ($records as &$record) { $this->tcaTableService->prepareRecord($record); } From 350f90601649d5a7a577ee7f472767b7d9619c1b Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Thu, 11 May 2017 09:16:24 +0200 Subject: [PATCH 7/7] BUGFIX: Don't index null records * Check null beside false for db result. --- Classes/Domain/Index/TcaIndexer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Classes/Domain/Index/TcaIndexer.php b/Classes/Domain/Index/TcaIndexer.php index 5cf6988..b50c6fa 100644 --- a/Classes/Domain/Index/TcaIndexer.php +++ b/Classes/Domain/Index/TcaIndexer.php @@ -146,7 +146,7 @@ class TcaIndexer implements IndexerInterface . ' AND ' . $this->tcaTableService->getTableName() . '.uid = ' . (int) $identifier ); - if ($record === false) { + if ($record === false || $record === null) { throw new NoRecordFoundException( 'Record could not be fetched from database: "' . $identifier . '". Perhaps record is not active.', 1484225364