Merge pull request #145 from dhensby/pull/144

Index now supports multiple relations with the same name.

code/search/SearchIndex.php

@@ -37,6 +37,27 @@ abstract class SearchIndex extends ViewableData
      */
     private static $hide_ancestor;
 
+    /**
+     * Used to separate class name and relation name in the sources array
+     * this string must not be present in class name
+     * @var string
+     * @config
+     */
+    private static $class_delimiter = '_|_';
+
+    /**
+     * This is used to clean the source name from suffix
+     * suffixes are needed to support multiple relations with the same name on different page types
+     * @param string $source
+     * @return string
+     */
+    protected function getSourceName($source)
+    {
+        $source = explode(self::config()->get('class_delimiter'), $source);
+
+        return $source[0];
+    }
+
     public function __construct()
     {
         parent::__construct();
@@ -57,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())
     {
@@ -77,19 +101,35 @@ abstract class SearchIndex extends ViewableData
             foreach ($lookups as $lookup) {
                 $next = array();
 
-                foreach ($sources as $source => $options) {
+                foreach ($sources as $source => $baseOptions) {
-                    $class = null;
+                    $source = $this->getSourceName($source);
 
-                    foreach (SearchIntrospection::hierarchy($source, $options['include_children']) as $dataclass) {
+                    foreach (SearchIntrospection::hierarchy($source, $baseOptions['include_children']) as $dataclass) {
+                        $class = null;
+                        $options = $baseOptions;
                         $singleton = singleton($dataclass);
 
                         if ($hasOne = $singleton->has_one($lookup)) {
+                            // we only want to include base class for relation, omit classes that inherited the relation
+                            $relationList = Config::inst()->get($dataclass, 'has_one', Config::UNINHERITED);
+                            $relationList = (!is_null($relationList)) ? $relationList : array();
+                            if (!array_key_exists($lookup, $relationList)) {
+                                continue;
+                            }
+
                             $class = $hasOne;
                             $options['lookup_chain'][] = array(
                                 'call' => 'method', 'method' => $lookup,
                                 'through' => 'has_one', 'class' => $dataclass, 'otherclass' => $class, 'foreignkey' => "{$lookup}ID"
                             );
                         } elseif ($hasMany = $singleton->has_many($lookup)) {
+                            // we only want to include base class for relation, omit classes that inherited the relation
+                            $relationList = Config::inst()->get($dataclass, 'has_many', Config::UNINHERITED);
+                            $relationList = (!is_null($relationList)) ? $relationList : array();
+                            if (!array_key_exists($lookup, $relationList)) {
+                                continue;
+                            }
+
                             $class = $hasMany;
                             $options['multi_valued'] = true;
                             $options['lookup_chain'][] = array(
@@ -97,6 +137,13 @@ abstract class SearchIndex extends ViewableData
                                 'through' => 'has_many', 'class' => $dataclass, 'otherclass' => $class, 'foreignkey' => $singleton->getRemoteJoinField($lookup, 'has_many')
                             );
                         } elseif ($manyMany = $singleton->many_many($lookup)) {
+                            // we only want to include base class for relation, omit classes that inherited the relation
+                            $relationList = Config::inst()->get($dataclass, 'many_many', Config::UNINHERITED);
+                            $relationList = (!is_null($relationList)) ? $relationList : array();
+                            if (!array_key_exists($lookup, $relationList)) {
+                                continue;
+                            }
+
                             $class = $manyMany[1];
                             $options['multi_valued'] = true;
                             $options['lookup_chain'][] = array(
@@ -109,8 +156,10 @@ abstract class SearchIndex extends ViewableData
                             if (!isset($options['origin'])) {
                                 $options['origin'] = $dataclass;
                             }
-                            $next[$class] = $options;
+
-                            continue 2;
+                            // we add suffix here to prevent the relation to be overwritten by other instances
+                            // all sources lookups must clean the source name before reading it via getSourceName()
+                            $next[$class . self::config()->get('class_delimiter') . $dataclass] = $options;
                         }
                     }
                 }
@@ -123,6 +172,7 @@ abstract class SearchIndex extends ViewableData
         }
 
         foreach ($sources as $class => $options) {
+            $class = $this->getSourceName($class);
             $dataclasses = SearchIntrospection::hierarchy($class, $options['include_children']);
 
             while (count($dataclasses)) {
@@ -557,7 +607,7 @@ abstract class SearchIndex extends ViewableData
 
                         $ids = $sql->execute()->column();
                     }
-                    
+
                     if (empty($ids)) {
                         break;
                     }

tests/SearchUpdaterTest.php

@@ -21,6 +21,30 @@ class SearchUpdaterTest_Container extends DataObject
     );
 }
 
+/**
+ * Used to test ambiguous relationships.
+ */
+class SearchUpdaterTest_OtherContainer extends DataObject
+{
+    private static $has_many = array(
+        'HasManyObjects' => 'SearchUpdaterTest_HasMany'
+    );
+
+    private static $many_many = array(
+        'ManyManyObjects' => 'SearchUpdaterTest_ManyMany'
+    );
+}
+
+/**
+ * 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(
@@ -41,7 +65,8 @@ class SearchUpdaterTest_HasMany extends DataObject
     );
 
     private static $has_one = array(
-        'HasManyContainer' => 'SearchUpdaterTest_Container'
+        'HasManyContainer' => 'SearchUpdaterTest_Container',
+        'HasManyOtherContainer' => 'SearchUpdaterTest_OtherContainer',
     );
 }
 
@@ -53,7 +78,8 @@ class SearchUpdaterTest_ManyMany extends DataObject
     );
 
     private static $belongs_many_many = array(
-        'ManyManyContainer' => 'SearchUpdaterTest_Container'
+        'ManyManyContainer' => 'SearchUpdaterTest_Container',
+        'ManyManyOtherContainer' => 'SearchUpdaterTest_OtherContainer',
     );
 }
 

tests/SolrIndexTest.php

@@ -53,6 +53,64 @@ class SolrIndexTest extends SapphireTest
         $this->assertEquals('SearchUpdaterTest_ManyMany', $data['class']);
     }
 
+    public function testFieldDataAmbiguousHasMany()
+    {
+        $index = new SolrIndexTest_AmbiguousRelationIndex();
+        $data = $index->fieldData('HasManyObjects.Field1');
+
+        $this->assertArrayHasKey('SearchUpdaterTest_Container_HasManyObjects_Field1', $data);
+        $this->assertArrayHasKey('SearchUpdaterTest_OtherContainer_HasManyObjects_Field1', $data);
+
+        $dataContainer = $data['SearchUpdaterTest_Container_HasManyObjects_Field1'];
+        $this->assertEquals($dataContainer['origin'], 'SearchUpdaterTest_Container');
+        $this->assertEquals($dataContainer['base'], 'SearchUpdaterTest_Container');
+        $this->assertEquals($dataContainer['class'], 'SearchUpdaterTest_HasMany');
+
+        $dataOtherContainer = $data['SearchUpdaterTest_OtherContainer_HasManyObjects_Field1'];
+        $this->assertEquals($dataOtherContainer['origin'], 'SearchUpdaterTest_OtherContainer');
+        $this->assertEquals($dataOtherContainer['base'], 'SearchUpdaterTest_OtherContainer');
+        $this->assertEquals($dataOtherContainer['class'], 'SearchUpdaterTest_HasMany');
+    }
+
+    public function testFieldDataAmbiguousManyMany()
+    {
+        $index = new SolrIndexTest_AmbiguousRelationIndex();
+        $data = $index->fieldData('ManyManyObjects.Field1');
+
+        $this->assertArrayHasKey('SearchUpdaterTest_Container_ManyManyObjects_Field1', $data);
+        $this->assertArrayHasKey('SearchUpdaterTest_OtherContainer_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');
+    }
+
+    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
      */
@@ -359,3 +417,43 @@ class SolrIndexTest_BoostedIndex extends SolrIndex
         $this->addBoostedField('Field2', null, array(), 2.1);
     }
 }
+
+class SolrIndexTest_AmbiguousRelationIndex extends SolrIndex
+{
+    protected function getStoredDefault()
+    {
+        // Override isDev defaulting to stored
+        return 'false';
+    }
+
+    public function init()
+    {
+        $this->addClass('SearchUpdaterTest_Container');
+        $this->addClass('SearchUpdaterTest_OtherContainer');
+
+        // These relationships exist on both classes
+        $this->addFilterField('HasManyObjects.Field1');
+        $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');
+    }
+}