From 16fc54e1019f6b7f6e0e49ba05695f76f0e8fb83 Mon Sep 17 00:00:00 2001
From: Mojmir Fendek <mojmir@silverstripe.com>
Date: Fri, 16 Jun 2017 09:39:37 +1200
Subject: [PATCH] Index now supports multiple relations with the same name.

---
 code/search/SearchIndex.php |  7 +++++--
 tests/SearchUpdaterTest.php | 14 +++++++++++--
 tests/SolrIndexTest.php     | 41 +++++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/code/search/SearchIndex.php b/code/search/SearchIndex.php
index eba8096..30054e7 100644
--- a/code/search/SearchIndex.php
+++ b/code/search/SearchIndex.php
@@ -51,7 +51,7 @@ abstract class SearchIndex extends ViewableData
      * @param string $source
      * @return string
      */
-    private function getSourceName($source)
+    protected function getSourceName($source)
     {
         $source = explode(self::config()->get('class_delimiter'), $source);
 
@@ -78,6 +78,9 @@ abstract class SearchIndex extends ViewableData
     /**
      * Examines the classes this index is built on to try and find defined fields in the class hierarchy for those classes.
      * Looks for db and viewable-data fields, although can't nessecarily find type for viewable-data fields.
+     * If multiple classes have a relation with the same name all of these will be included in the search index
+     * Note that only classes that have the relations uninherited (defined in them) will be listed
+     * this is because inherited relations do not need to be processed by index explicitly
      */
     public function fieldData($field, $forceType = null, $extraOptions = array())
     {
@@ -604,7 +607,7 @@ abstract class SearchIndex extends ViewableData
 
                         $ids = $sql->execute()->column();
                     }
-                    
+
                     if (empty($ids)) {
                         break;
                     }
diff --git a/tests/SearchUpdaterTest.php b/tests/SearchUpdaterTest.php
index 3b42f11..9b662d4 100644
--- a/tests/SearchUpdaterTest.php
+++ b/tests/SearchUpdaterTest.php
@@ -35,6 +35,16 @@ class SearchUpdaterTest_OtherContainer extends DataObject
     );
 }
 
+/**
+ * Used to test inherited ambiguous relationships.
+ */
+class SearchUpdaterTest_ExtendedContainer extends SearchUpdaterTest_OtherContainer
+{
+    private static $db = array(
+        'SomeField' => 'Varchar',
+    );
+}
+
 class SearchUpdaterTest_HasOne extends DataObject
 {
     private static $db = array(
@@ -56,7 +66,7 @@ class SearchUpdaterTest_HasMany extends DataObject
 
     private static $has_one = array(
         'HasManyContainer' => 'SearchUpdaterTest_Container',
-        'HasManyOtherContainer' => 'SearchUpdaterTest_OtherContainer'
+        'HasManyOtherContainer' => 'SearchUpdaterTest_OtherContainer',
     );
 }
 
@@ -69,7 +79,7 @@ class SearchUpdaterTest_ManyMany extends DataObject
 
     private static $belongs_many_many = array(
         'ManyManyContainer' => 'SearchUpdaterTest_Container',
-        'ManyManyOtherContainer' => 'SearchUpdaterTest_OtherContainer'
+        'ManyManyOtherContainer' => 'SearchUpdaterTest_OtherContainer',
     );
 }
 
diff --git a/tests/SolrIndexTest.php b/tests/SolrIndexTest.php
index cc7e745..b01ce17 100644
--- a/tests/SolrIndexTest.php
+++ b/tests/SolrIndexTest.php
@@ -91,6 +91,26 @@ class SolrIndexTest extends SapphireTest
         $this->assertEquals($dataOtherContainer['class'], 'SearchUpdaterTest_ManyMany');
     }
 
+    public function testFieldDataAmbiguousManyManyInherited()
+    {
+        $index = new SolrIndexTest_AmbiguousRelationInheritedIndex();
+        $data = $index->fieldData('ManyManyObjects.Field1');
+
+        $this->assertArrayHasKey('SearchUpdaterTest_Container_ManyManyObjects_Field1', $data);
+        $this->assertArrayHasKey('SearchUpdaterTest_OtherContainer_ManyManyObjects_Field1', $data);
+        $this->assertArrayNotHasKey('SearchUpdaterTest_ExtendedContainer_ManyManyObjects_Field1', $data);
+
+        $dataContainer = $data['SearchUpdaterTest_Container_ManyManyObjects_Field1'];
+        $this->assertEquals($dataContainer['origin'], 'SearchUpdaterTest_Container');
+        $this->assertEquals($dataContainer['base'], 'SearchUpdaterTest_Container');
+        $this->assertEquals($dataContainer['class'], 'SearchUpdaterTest_ManyMany');
+
+        $dataOtherContainer = $data['SearchUpdaterTest_OtherContainer_ManyManyObjects_Field1'];
+        $this->assertEquals($dataOtherContainer['origin'], 'SearchUpdaterTest_OtherContainer');
+        $this->assertEquals($dataOtherContainer['base'], 'SearchUpdaterTest_OtherContainer');
+        $this->assertEquals($dataOtherContainer['class'], 'SearchUpdaterTest_ManyMany');
+    }
+
     /**
      * Test boosting on SearchQuery
      */
@@ -416,3 +436,24 @@ class SolrIndexTest_AmbiguousRelationIndex extends SolrIndex
         $this->addFilterField('ManyManyObjects.Field1');
     }
 }
+
+class SolrIndexTest_AmbiguousRelationInheritedIndex extends SolrIndex
+{
+    protected function getStoredDefault()
+    {
+        // Override isDev defaulting to stored
+        return 'false';
+    }
+
+    public function init()
+    {
+        $this->addClass('SearchUpdaterTest_Container');
+        // this one has not the relation defined in it's class but is rather inherited from parent
+        // note that even if we do not include it's parent class the fields will be properly added
+        $this->addClass('SearchUpdaterTest_ExtendedContainer');
+
+        // These relationships exist on both classes
+        $this->addFilterField('HasManyObjects.Field1');
+        $this->addFilterField('ManyManyObjects.Field1');
+    }
+}