Merge pull request #192 from creative-commoners/pulls/3.0/safer-index-names-together

NEW Replace backslashes in Solr index names with dashes

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);

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');
@@ -850,11 +874,12 @@ abstract class SolrIndex extends SearchIndex
      * Solr requires namespaced classes to have double escaped backslashes
      *
      * @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);
     }
 
     /**

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()

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 '));