From 4d57f09230370f9e90e90a24a5f069639ef98760 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Wed, 6 Dec 2017 10:17:18 +1300 Subject: [PATCH] NEW Replace backslashes in Solr index names with dashes --- src/Solr/Services/SolrService.php | 12 ++++++++- src/Solr/SolrIndex.php | 35 ++++++++++++++++++++++---- tests/SolrIndexTest.php | 41 +++++++++++++++++++++++++++++-- tests/SolrReindexTest.php | 13 +++++++--- 4 files changed, 90 insertions(+), 11 deletions(-) diff --git a/src/Solr/Services/SolrService.php b/src/Solr/Services/SolrService.php index 5cff62d..c719e89 100644 --- a/src/Solr/Services/SolrService.php +++ b/src/Solr/Services/SolrService.php @@ -4,6 +4,7 @@ namespace SilverStripe\FullTextSearch\Solr\Services; use SilverStripe\Core\Config\Config; use SilverStripe\FullTextSearch\Solr\Solr; +use SilverStripe\FullTextSearch\Solr\SolrIndex; use Silverstripe\Core\ClassInfo; /** @@ -20,6 +21,9 @@ class SolrService extends SolrService_Core */ protected function coreCommand($command, $core, $params = array()) { + // Unencode the class name + $core = SolrIndex::getClassNameFromIndex($core); + $command = strtoupper($command); //get the non-namespaced name of the Solr core, since backslashes not valid characters $core = ClassInfo::shortName($core); @@ -31,11 +35,14 @@ class SolrService extends SolrService_Core /** * Is the passed core active? - * @param string $core The name of the core + * @param string $core The name of the core (an encoded class name) * @return boolean True if that core exists & is active */ public function coreIsActive($core) { + // Unencode the class name + $core = SolrIndex::getClassNameFromIndex($core); + // Request the status of the full core name $result = $this->coreCommand('STATUS', $core); @@ -88,6 +95,9 @@ class SolrService extends SolrService_Core */ public function serviceForCore($core) { + // Unencode the class name + $core = SolrIndex::getClassNameFromIndex($core); + $klass = Config::inst()->get(get_called_class(), 'core_class'); $coreName = ClassInfo::shortName($core); return new $klass($this->_host, $this->_port, $this->_path . $coreName, $this->_httpTransport); diff --git a/src/Solr/SolrIndex.php b/src/Solr/SolrIndex.php index 8e940a3..24fa5de 100644 --- a/src/Solr/SolrIndex.php +++ b/src/Solr/SolrIndex.php @@ -108,7 +108,8 @@ abstract class SolrIndex extends SearchIndex */ public function getIndexName() { - $name = get_class($this); + $name = $this->sanitiseClassName(get_class($this), '-'); + $indexParts = [$name]; if ($indexPrefix = Environment::getEnv('SS_SOLR_INDEX_PREFIX')) { @@ -122,6 +123,29 @@ abstract class SolrIndex extends SearchIndex return implode($indexParts); } + /** + * Helper for returning the indexer class name from an index name, encoded via {@link getIndexName()} + * + * @param string $indexName + * @return string + */ + public static function getClassNameFromIndex($indexName) + { + if (($indexPrefix = Environment::getEnv('SS_SOLR_INDEX_PREFIX')) + && (substr($indexName, 0, strlen($indexPrefix)) === $indexPrefix) + ) { + $indexName = substr($indexName, strlen($indexPrefix)); + } + + if (($indexSuffix = Environment::getEnv('SS_SOLR_INDEX_SUFFIX')) + && (substr($indexName, -strlen($indexSuffix)) === $indexSuffix) + ) { + $indexName = substr($indexName, 0, -strlen($indexSuffix)); + } + + return str_replace('-', '\\', $indexName); + } + public function getTypes() { return $this->renderWith($this->getTemplatesPath() . '/types.ss'); @@ -849,12 +873,13 @@ abstract class SolrIndex extends SearchIndex /** * Solr requires namespaced classes to have double escaped backslashes * - * @param string $className E.g. My\Object\Here - * @return string E.g. My\\Object\\Here + * @param string $className E.g. My\Object\Here + * @param string $replaceWith The replacement character(s) to use + * @return string E.g. My\\Object\\Here */ - public function sanitiseClassName($className) + public function sanitiseClassName($className, $replaceWith = '\\\\') { - return str_replace('\\', '\\\\', $className); + return str_replace('\\', $replaceWith, $className); } /** diff --git a/tests/SolrIndexTest.php b/tests/SolrIndexTest.php index 3b55387..73bf11d 100644 --- a/tests/SolrIndexTest.php +++ b/tests/SolrIndexTest.php @@ -9,6 +9,7 @@ use SilverStripe\Core\Kernel; use SilverStripe\Dev\SapphireTest; use SilverStripe\FullTextSearch\Search\Queries\SearchQuery; use SilverStripe\FullTextSearch\Search\Variants\SearchVariantSubsites; +use SilverStripe\FullTextSearch\Solr\SolrIndex; use SilverStripe\FullTextSearch\Solr\Services\Solr3Service; use SilverStripe\FullTextSearch\Tests\SearchUpdaterTest\SearchUpdaterTest_Container; use SilverStripe\FullTextSearch\Tests\SearchUpdaterTest\SearchUpdaterTest_HasOne; @@ -341,16 +342,25 @@ class SolrIndexTest extends SapphireTest public function testSanitiseClassName() { $index = new SolrIndexTest_FakeIndex2; + $this->assertSame( 'SilverStripe\\\\FullTextSearch\\\\Tests\\\\SolrIndexTest', $index->sanitiseClassName(static::class) ); + + $this->assertSame( + 'SilverStripe-FullTextSearch-Tests-SolrIndexTest', + $index->sanitiseClassName(static::class, '-') + ); } public function testGetIndexName() { $index = new SolrIndexTest_FakeIndex2; - $this->assertSame(SolrIndexTest_FakeIndex2::class, $index->getIndexName()); + $this->assertSame( + 'SilverStripe-FullTextSearch-Tests-SolrIndexTest-SolrIndexTest_FakeIndex2', + $index->getIndexName() + ); } public function testGetIndexNameWithPrefixAndSuffixFromEnvironment() @@ -360,7 +370,34 @@ class SolrIndexTest extends SapphireTest Environment::putEnv('SS_SOLR_INDEX_PREFIX="foo_"'); Environment::putEnv('SS_SOLR_INDEX_SUFFIX="_bar"'); - $this->assertSame('foo_' . SolrIndexTest_FakeIndex2::class . '_bar', $index->getIndexName()); + $this->assertSame( + 'foo_SilverStripe-FullTextSearch-Tests-SolrIndexTest-SolrIndexTest_FakeIndex2_bar', + $index->getIndexName() + ); + } + + /** + * @dataProvider indexNameProvider + * @param string $indexName + * @param string $expected + */ + public function testGetClassNameFromIndex($indexName, $expected) + { + Environment::putEnv('SS_SOLR_INDEX_PREFIX="foo_"'); + Environment::putEnv('SS_SOLR_INDEX_SUFFIX="_bar"'); + + $this->assertSame($expected, SolrIndex::getClassNameFromIndex($indexName)); + } + + /** + * @return array[] + */ + public function indexNameProvider() + { + return [ + ['foo_SilverStripe-FullTextSearch-Tests-SolrIndexTest_bar', __CLASS__], + ['SilverStripe-FullTextSearch-Tests-SolrIndexTest', __CLASS__], + ]; } protected function getFakeRawSolrResponse() diff --git a/tests/SolrReindexTest.php b/tests/SolrReindexTest.php index c88614e..0a34397 100644 --- a/tests/SolrReindexTest.php +++ b/tests/SolrReindexTest.php @@ -179,10 +179,17 @@ class SolrReindexTest extends SapphireTest $this->getHandler()->runReindex($logger, 21, Solr_Reindex::class); // Test that invalid classes are removed - $this->assertContains('Clearing obsolete classes from ' . SolrReindexTest_Index::class, $logger->getMessages()); - //var_dump($logger->getMessages()); + $this->assertContains( + 'Clearing obsolete classes from ' . str_replace('\\', '-', SolrReindexTest_Index::class), + $logger->getMessages() + ); + // Test that valid classes in invalid variants are removed - $this->assertContains('Clearing all records of type ' . SolrReindexTest_Item::class . ' in the current state: {' . json_encode(SolrReindexTest_Variant::class) . ':"2"}', $logger->getMessages()); + $this->assertContains( + 'Clearing all records of type ' . SolrReindexTest_Item::class . ' in the current state: {' + . json_encode(SolrReindexTest_Variant::class) . ':"2"}', + $logger->getMessages() + ); // 120x2 grouped into groups of 21 results in 12 groups $this->assertEquals(12, $logger->countMessages('Called processGroup with '));