BUG fix issues with search variants applying to more than one class

.travis.yml

@@ -1,36 +1,35 @@
 # See https://github.com/silverstripe-labs/silverstripe-travis-support for setup details
 
-sudo: false
-
 language: php
 
+sudo: false
+
 php:
-  - 5.3
   - 5.4
   - 5.5
   - 5.6
-  - 7.0
 
 env:
   - DB=MYSQL CORE_RELEASE=3.2
 
 matrix:
   include:
+    - php: 5.3
+      env: DB=PGSQL CORE_RELEASE=3.1
     - php: 5.6
-      env: DB=MYSQL CORE_RELEASE=3
+      env: DB=MYSQL CORE_RELEASE=3.2
     - php: 5.6
-      env: DB=MYSQL CORE_RELEASE=3.1
+      env: DB=MYSQL CORE_RELEASE=3.3 SUBSITES=1
     - php: 5.6
-      env: DB=PGSQL CORE_RELEASE=3.2
-  allow_failures:
-    - php: 7.0
+      env: DB=MYSQL CORE_RELEASE=3.3 QUEUEDJOBS=1
 
 before_script:
   - composer self-update || true
   - git clone git://github.com/silverstripe-labs/silverstripe-travis-support.git ~/travis-support
-  - php ~/travis-support/travis_setup.php --source `pwd` --target ~/builds/ss
+  - "if [ \"$SUBSITES\" = \"\" -a \"$QUEUEDJOBS\" = \"\" ]; then php ~/travis-support/travis_setup.php --source `pwd` --target ~/builds/ss; fi"
+  - "if [ \"$SUBSITES\" = \"1\" ]; then php ~/travis-support/travis_setup.php --source `pwd` --target ~/builds/ss --require silverstripe/subsites; fi"
+  - "if [ \"$QUEUEDJOBS\" = \"1\" ]; then php ~/travis-support/travis_setup.php --source `pwd` --target ~/builds/ss --require silverstripe/queuedjobs; fi"
   - cd ~/builds/ss
-  - composer install
 
 script:
-  - vendor/bin/phpunit fulltextsearch/tests
+  - vendor/bin/phpunit fulltextsearch/tests/

code/search/SearchIndex.php

@@ -25,7 +25,7 @@
  * - Specifying which classes and fields this index contains
  *
  * - Specifying update rules that are not extractable from metadata (because the values come from functions for instance)
- * 
+ *
  */
 abstract class SearchIndex extends ViewableData
 {
@@ -354,7 +354,7 @@ abstract class SearchIndex extends ViewableData
 
     /**
      * Returns an array where each member is all the fields and the classes that are at the end of some
-     * specific lookup chain from one of the base classes 
+     * specific lookup chain from one of the base classes
      */
     public function getDerivedFields()
     {
@@ -391,7 +391,7 @@ abstract class SearchIndex extends ViewableData
 
     /**
      * Get the "document ID" (a database & variant unique id) given some "Base" class, DataObject ID and state array
-     * 
+     *
      * @param String $base - The base class of the object
      * @param Integer $id - The ID of the object
      * @param Array $state - The variant state of the object
@@ -465,7 +465,7 @@ abstract class SearchIndex extends ViewableData
                         $method = $step['method'];
                         $object = $object->$method();
                     } elseif ($step['call'] == 'variant') {
-                        $variants = SearchVariant::variants($field['base'], true);
+                        $variants = SearchVariant::variants();
                         $variant = $variants[$step['variant']];
                         $method = $step['method'];
                         $object = $variant->$method($object);
@@ -476,6 +476,7 @@ abstract class SearchIndex extends ViewableData
                 }
             }
         } catch (Exception $e) {
+            static::warn($e);
             $object = null;
         }
 
@@ -483,6 +484,20 @@ abstract class SearchIndex extends ViewableData
         return $object;
     }
 
+    /**
+     * Log non-fatal errors
+     *
+     * @param Exception $e
+     * @throws Exception
+     */
+    public static function warn($e) {
+        // Noisy errors during testing
+        if(class_exists('SapphireTest', false) && SapphireTest::is_running_test()) {
+            throw $e;
+        }
+        SS_Log::log($e, SS_Log::WARN);
+    }
+
     /**
      * Given a class, object id, set of stateful ids and a list of changed fields (in a special format),
      * return what statefulids need updating in this index
@@ -620,7 +635,7 @@ abstract class SearchIndex_Recording extends SearchIndex
         $res = array();
 
         $res['ID'] = $object->ID;
-        
+
         foreach ($this->getFieldsIterator() as $name => $field) {
             $val = $this->_getFieldValue($object, $field);
             $res[$name] = $val;
@@ -655,7 +670,7 @@ abstract class SearchIndex_Recording extends SearchIndex
     {
         $this->committed = true;
     }
-    
+
     public function getIndexName()
     {
         return get_class($this);

code/search/SearchVariant.php

@@ -123,7 +123,7 @@ abstract class SearchVariant
      * @param bool $includeSubclasses - (Optional) If false, only variants that apply strictly to the passed class or its super-classes
      * will be checked. If true (the default), variants that apply to any sub-class of the passed class with also be checked
      *
-     * @return An object with one method, call()
+     * @return SearchVariant_Caller An object with one method, call()
      */
     public static function with($class = null, $includeSubclasses = true)
     {
@@ -197,6 +197,59 @@ abstract class SearchVariant
 
         return $allstates ? new CombinationsArrayIterator($allstates) : array(array());
     }
+
+
+    /**
+     * Add new filter field to index safely.
+     *
+     * This method will respect existing filters with the same field name that
+     * correspond to multiple classes
+     *
+     * @param SearchIndex $index
+     * @param string $name Field name
+     * @param array $field Field spec
+     */
+    protected function addFilterField($index, $name, $field) {
+        // If field already exists, make sure to merge origin / base fields
+        if(isset($index->filterFields[$name])) {
+            $field['base'] = $this->mergeClasses(
+                $index->filterFields[$name]['base'],
+                $field['base']
+            );
+            $field['origin'] = $this->mergeClasses(
+                $index->filterFields[$name]['origin'],
+                $field['origin']
+            );
+        }
+
+        $index->filterFields[$name] = $field;
+    }
+
+    /**
+     * Merge sets of (or individual) class names together for a search index field.
+     *
+     * If there is only one unique class name, then just return it as a string instead of array.
+     *
+     * @param array|string $left Left class(es)
+     * @param array|string $right Right class(es)
+     * @return array|string List of classes, or single class
+     */
+    protected function mergeClasses($left, $right) {
+        // Merge together and remove dupes
+        if(!is_array($left)) {
+            $left = array($left);
+        }
+        if(!is_array($right)) {
+            $right = array($right);
+        }
+        $merged = array_values(array_unique(array_merge($left, $right)));
+
+        // If there is only one item, return it as a single string
+        if(count($merged) === 1) {
+            return reset($merged);
+        }
+        return $merged;
+    }
 }
 
 /**

code/search/SearchVariantSiteTreeSubsitesPolyhome.php

@@ -40,19 +40,19 @@ class SearchVariantSiteTreeSubsitesPolyhome extends SearchVariant
         }
     }
 
-    public function alterDefinition($base, $index)
+    public function alterDefinition($class, $index)
     {
         $self = get_class($this);
-        
-        $index->filterFields['_subsite'] = array(
+
+        $this->addFilterField($index, '_subsite', array(
             'name' => '_subsite',
             'field' => '_subsite',
             'fullfield' => '_subsite',
-            'base' => $base,
-            'origin' => $base,
+            'base' => ClassInfo::baseDataClass($class),
+            'origin' => $class,
             'type' => 'Int',
             'lookup_chain' => array(array('call' => 'variant', 'variant' => $self, 'method' => 'currentState'))
-        );
+        ));
     }
 
     public function alterQuery($query, $index)

code/search/SearchVariantSubsites.php

@@ -44,29 +44,29 @@ class SearchVariantSubsites extends SearchVariant
         Permission::flush_permission_cache();
     }
 
-    public function alterDefinition($base, $index)
+    public function alterDefinition($class, $index)
     {
         $self = get_class($this);
-        
-        $index->filterFields['_subsite'] = array(
+
+        // Add field to root
+        $this->addFilterField($index, '_subsite', array(
             'name' => '_subsite',
             'field' => '_subsite',
             'fullfield' => '_subsite',
-            'base' => $base,
-            'origin' => $base,
+            'base' => ClassInfo::baseDataClass($class),
+            'origin' => $class,
             'type' => 'Int',
             'lookup_chain' => array(array('call' => 'variant', 'variant' => $self, 'method' => 'currentState'))
-        );
+        ));
     }
 
+
     public function alterQuery($query, $index)
     {
         $subsite = Subsite::currentSubsiteID();
         $query->filter('_subsite', array($subsite, SearchQuery::$missing));
     }
 
-    public static $subsites = null;
-
     /**
      * We need _really_ complicated logic to find just the changed subsites (because we use versions there's no explicit
      * deletes, just new versions with different members) so just always use all of them
@@ -74,25 +74,27 @@ class SearchVariantSubsites extends SearchVariant
     public function extractManipulationWriteState(&$writes)
     {
         $self = get_class($this);
+        $query = new SQLQuery('"ID"', '"Subsite"');
+        $subsites = array_merge(array('0'), $query->execute()->column());
 
         foreach ($writes as $key => $write) {
-            if (!$this->appliesTo($write['class'], true)) {
+            $applies = $this->appliesTo($write['class'], true);
+            if (!$applies) {
                 continue;
             }
 
-            if (self::$subsites === null) {
-                $query = new SQLQuery('"ID"', '"Subsite"');
-                self::$subsites = array_merge(array('0'), $query->execute()->column());
-            }
-
             $next = array();
-
             foreach ($write['statefulids'] as $i => $statefulid) {
-                foreach (self::$subsites as $subsiteID) {
-                    $next[] = array('id' => $statefulid['id'], 'state' => array_merge($statefulid['state'], array($self => (string)$subsiteID)));
+                foreach ($subsites as $subsiteID) {
+                    $next[] = array(
+                        'id' => $statefulid['id'],
+                        'state' => array_merge(
+                            $statefulid['state'],
+                            array($self => (string)$subsiteID)
+                        )
+                    );
                 }
             }
-
             $writes[$key]['statefulids'] = $next;
         }
     }

code/search/SearchVariantVersioned.php

@@ -25,19 +25,19 @@ class SearchVariantVersioned extends SearchVariant
         Versioned::reading_stage($state);
     }
 
-    public function alterDefinition($base, $index)
+    public function alterDefinition($class, $index)
     {
         $self = get_class($this);
 
-        $index->filterFields['_versionedstage'] = array(
+        $this->addFilterField($index, '_versionedstage', array(
             'name' => '_versionedstage',
             'field' => '_versionedstage',
             'fullfield' => '_versionedstage',
-            'base' => $base,
-            'origin' => $base,
+            'base' => ClassInfo::baseDataClass($class),
+            'origin' => $class,
             'type' => 'String',
             'lookup_chain' => array(array('call' => 'variant', 'variant' => $self, 'method' => 'currentState'))
-        );
+        ));
     }
 
     public function alterQuery($query, $index)
@@ -45,11 +45,11 @@ class SearchVariantVersioned extends SearchVariant
         $stage = Versioned::current_stage();
         $query->filter('_versionedstage', array($stage, SearchQuery::$missing));
     }
-    
+
     public function extractManipulationState(&$manipulation)
     {
         $self = get_class($this);
-        
+
         foreach ($manipulation as $table => $details) {
             $class = $details['class'];
             $stage = 'Stage';

code/solr/Solr.php

@@ -173,7 +173,7 @@ class Solr_BuildTask extends BuildTask
 
     /**
      * Get the current logger
-     * 
+     *
      * @return LoggerInterface
      */
     public function getLogger()
@@ -225,7 +225,7 @@ class Solr_Configure extends Solr_BuildTask
     public function run($request)
     {
         parent::run($request);
-        
+
         // Find the IndexStore handler, which will handle uploading config files to Solr
         $store = $this->getSolrConfigStore();
         $indexes = Solr::get_indexes();
@@ -240,10 +240,10 @@ class Solr_Configure extends Solr_BuildTask
             }
         }
     }
-    
+
     /**
      * Update the index on the given store
-     * 
+     *
      * @param SolrIndex $instance Instance
      * @param SolrConfigStore $store
      */
@@ -251,11 +251,11 @@ class Solr_Configure extends Solr_BuildTask
     {
         $index = $instance->getIndexName();
         $this->getLogger()->info("Configuring $index.");
-        
+
         // Upload the config files for this index
         $this->getLogger()->info("Uploading configuration ...");
         $instance->uploadConfig($store);
-        
+
         // Then tell Solr to use those config files
         $service = Solr::service();
         if ($service->coreIsActive($index)) {
@@ -265,23 +265,23 @@ class Solr_Configure extends Solr_BuildTask
             $this->getLogger()->info("Creating core ...");
             $service->coreCreate($index, $store->instanceDir($index));
         }
-        
+
         $this->getLogger()->info("Done");
     }
 
     /**
      * Get config store
-     * 
+     *
      * @return SolrConfigStore
      */
     protected function getSolrConfigStore()
     {
         $options = Solr::solr_options();
-        
+
         if (!isset($options['indexstore']) || !($indexstore = $options['indexstore'])) {
             user_error('No index configuration for Solr provided', E_USER_ERROR);
         }
-        
+
         // Find the IndexStore handler, which will handle uploading config files to Solr
         $mode = $indexstore['mode'];
 
@@ -340,7 +340,7 @@ class Solr_Reindex extends Solr_BuildTask
     public function run($request)
     {
         parent::run($request);
-        
+
         // Reset state
         $originalState = SearchVariant::current_state();
         $this->doReindex($request);
@@ -411,7 +411,7 @@ class Solr_Reindex extends Solr_BuildTask
     protected function runFrom($index, $class, $start, $variantstate)
     {
         DeprecationTest_Deprecation::notice('2.0.0', 'Solr_Reindex now uses a new grouping mechanism');
-        
+
         // Set time limit and state
         increase_time_limit_to();
         SearchVariant::activate_state($variantstate);

code/solr/SolrIndex.php

@@ -31,7 +31,7 @@ abstract class SolrIndex extends SearchIndex
     protected $extrasPath = null;
 
     protected $templatesPath = null;
-    
+
     /**
      * List of boosted fields
      *
@@ -54,7 +54,7 @@ abstract class SolrIndex extends SearchIndex
      * @var array
      */
     private static $copy_fields = array();
-    
+
     /**
      * @return String Absolute path to the folder containing
      * templates which are used for generating the schema and field definitions.
@@ -93,11 +93,11 @@ abstract class SolrIndex extends SearchIndex
     /**
      * Index-time analyzer which is applied to a specific field.
      * Can be used to remove HTML tags, apply stemming, etc.
-     * 
+     *
      * @see http://wiki.apache.org/solr/AnalyzersTokenizersTokenFilters#solr.WhitespaceTokenizerFactory
-     * 
+     *
      * @param String $field
-     * @param String $type  
+     * @param String $type
      * @param Array $params Parameters for the analyzer, usually at least a "class"
      */
     public function addAnalyzer($field, $type, $params)
@@ -179,13 +179,13 @@ abstract class SolrIndex extends SearchIndex
             }
             $xml[] = $this->getFieldDefinition($name, $field);
         }
-        
+
         return implode("\n\t\t", $xml);
     }
-    
+
     /**
      * Extract first suggestion text from collated values
-     * 
+     *
      * @param mixed $collation
      * @return string
      */
@@ -246,10 +246,10 @@ abstract class SolrIndex extends SearchIndex
         $options = array_merge($extraOptions, array('stored' => 'true'));
         $this->addFulltextField($field, $forceType, $options);
     }
-    
+
     /**
      * Add a fulltext field with a boosted value
-     * 
+     *
      * @param string $field The field to add
      * @param string $forceType The type to force this field as (required in some cases, when not
      * detectable from metadata)
@@ -261,7 +261,7 @@ abstract class SolrIndex extends SearchIndex
         $options = array_merge($extraOptions, array('boost' => $boost));
         $this->addFulltextField($field, $forceType, $options);
     }
-    
+
 
     public function fieldData($field, $forceType = null, $extraOptions = array())
     {
@@ -272,7 +272,7 @@ abstract class SolrIndex extends SearchIndex
             unset($extraOptions['boost']);
         }
         $data = parent::fieldData($field, $forceType, $extraOptions);
-        
+
         // Boost all fields with this name
         if (isset($boost)) {
             foreach ($data as $fieldName => $fieldInfo) {
@@ -281,14 +281,14 @@ abstract class SolrIndex extends SearchIndex
         }
         return $data;
     }
-    
+
     /**
      * Set the default boosting level for a specific field.
-     * Will control the default value for qf param (Query Fields), but will not 
+     * Will control the default value for qf param (Query Fields), but will not
      * override a query-specific value.
-     * 
+     *
      * Fields must be added before having a field boosting specified
-     * 
+     *
      * @param string $field Full field key (Model_Field)
      * @param float|null $level Numeric boosting value. Set to null to clear boost
      */
@@ -303,20 +303,20 @@ abstract class SolrIndex extends SearchIndex
             $this->boostedFields[$field] = $level;
         }
     }
-    
+
     /**
      * Get all boosted fields
-     * 
+     *
      * @return array
      */
     public function getBoostedFields()
     {
         return $this->boostedFields;
     }
-    
+
     /**
      * Determine the best default value for the 'qf' parameter
-     * 
+     *
      * @return array|null List of query fields, or null if not specified
      */
     public function getQueryFields()
@@ -335,7 +335,7 @@ abstract class SolrIndex extends SearchIndex
         if ($queryFields && !isset($this->boostedFields[$df])) {
             $queryFields[] = $df;
         }
-        
+
         return $queryFields;
     }
 
@@ -390,7 +390,7 @@ abstract class SolrIndex extends SearchIndex
 
     /**
      * Convert definition to XML tag
-     * 
+     *
      * @param String $tag
      * @param String $attrs Map of attributes
      * @param String $content Inner content
@@ -451,32 +451,53 @@ abstract class SolrIndex extends SearchIndex
         return implode("\n\t", $xml);
     }
 
+    /**
+     * Determine if the given object is one of the given type
+     *
+     * @param string $class
+     * @param array|string $base Class or list of base classes
+     * @return bool
+     */
+    protected function classIs($class, $base) {
+        if(is_array($base)) {
+            foreach($base as $nextBase) {
+                if($this->classIs($class, $nextBase)) {
+                    return true;
+                }
+            }
+            return false;
+        }
+
+        // Check single origin
+        return $class === $base || is_subclass_of($class, $base);
+    }
+
     protected function _addField($doc, $object, $field)
     {
         $class = get_class($object);
-        if ($class != $field['origin'] && !is_subclass_of($class, $field['origin'])) {
+        if(!$this->classIs($class, $field['origin'])) {
             return;
         }
 
         $value = $this->_getFieldValue($object, $field);
-        
+
         $type = isset(self::$filterTypeMap[$field['type']]) ? self::$filterTypeMap[$field['type']] : self::$filterTypeMap['*'];
 
         if (is_array($value)) {
             foreach ($value as $sub) {
                 /* Solr requires dates in the form 1995-12-31T23:59:59Z */
-            if ($type == 'tdate') {
-                if (!$sub) {
+                if ($type == 'tdate') {
+                    if (!$sub) {
+                        continue;
+                    }
+                    $sub = gmdate('Y-m-d\TH:i:s\Z', strtotime($sub));
+                }
+
+                /* Solr requires numbers to be valid if presented, not just empty */
+                if (($type == 'tint' || $type == 'tfloat' || $type == 'tdouble') && !is_numeric($sub)) {
                     continue;
                 }
-                $sub = gmdate('Y-m-d\TH:i:s\Z', strtotime($sub));
-            }
 
-            /* Solr requires numbers to be valid if presented, not just empty */
-            if (($type == 'tint' || $type == 'tfloat' || $type == 'tdouble') && !is_numeric($sub)) {
-                continue;
-            }
-            
                 $doc->addField($field['name'], $sub);
             }
         } else {
@@ -516,7 +537,7 @@ abstract class SolrIndex extends SearchIndex
         // Add the user-specified fields
 
         foreach ($this->getFieldsIterator() as $name => $field) {
-            if ($field['base'] == $base) {
+            if ($field['base'] === $base || (is_array($field['base']) && in_array($base, $field['base']))) {
                 $this->_addField($doc, $object, $field);
             }
         }
@@ -524,7 +545,7 @@ abstract class SolrIndex extends SearchIndex
         try {
             $this->getService()->addDocument($doc);
         } catch (Exception $e) {
-            SS_Log::log($e, SS_Log::WARN);
+            static::warn($e);
             return false;
         }
 
@@ -564,7 +585,7 @@ abstract class SolrIndex extends SearchIndex
         try {
             $this->getService()->deleteById($documentID);
         } catch (Exception $e) {
-            SS_Log::log($e, SS_Log::WARN);
+            static::warn($e);
             return false;
         }
     }
@@ -608,7 +629,7 @@ abstract class SolrIndex extends SearchIndex
         try {
             $this->getService()->commit(false, false, false);
         } catch (Exception $e) {
-            SS_Log::log($e, SS_Log::WARN);
+            static::warn($e);
             return false;
         }
     }
@@ -618,7 +639,7 @@ abstract class SolrIndex extends SearchIndex
      * @param integer $offset
      * @param integer $limit
      * @param array $params Extra request parameters passed through to Solr
-     * @return ArrayData Map with the following keys: 
+     * @return ArrayData Map with the following keys:
      *  - 'Matches': ArrayList of the matched object instances
      */
     public function search(SearchQuery $query, $offset = -1, $limit = -1, $params = array())
@@ -638,7 +659,7 @@ abstract class SolrIndex extends SearchIndex
 
         // Build the search itself
         $q = $this->getQueryComponent($query, $hlq);
-        
+
         // If using boosting, set the clean term separately for highlighting.
         // See https://issues.apache.org/jira/browse/SOLR-2632
         if (array_key_exists('hl', $params) && !array_key_exists('hl.q', $params)) {
@@ -657,10 +678,10 @@ abstract class SolrIndex extends SearchIndex
         if ($classq) {
             $fq[] = '+('.implode(' ', $classq).')';
         }
-        
+
         // Filter by filters
         $fq = array_merge($fq, $this->getFiltersComponent($query));
-        
+
         // Prepare query fields unless specified explicitly
         if (isset($params['qf'])) {
             $qf = $params['qf'];
@@ -697,7 +718,7 @@ abstract class SolrIndex extends SearchIndex
         }
 
         $params = array_merge($params, array('fq' => implode(' ', $fq)));
-        
+
         $res = $service->search(
             $q ? implode(' ', $q) : '*:*',
             $offset,
@@ -751,7 +772,7 @@ abstract class SolrIndex extends SearchIndex
         $ret['Matches']->setPageStart($offset);
         // Results per page
         $ret['Matches']->setPageLength($limit);
-        
+
         // Include spellcheck and suggestion data. Requires spellcheck=true in $params
         if (isset($res->spellcheck)) {
             // Expose all spellcheck data, for custom handling.
@@ -761,7 +782,7 @@ abstract class SolrIndex extends SearchIndex
             if (isset($res->spellcheck->suggestions->collation)) {
                 // Extract string suggestion
                 $suggestion = $this->getCollatedSuggestion($res->spellcheck->suggestions->collation);
-                
+
                 // The collation, including advanced query params (e.g. +), suitable for making another query programmatically.
                 $ret['Suggestion'] = $suggestion;
 
@@ -922,10 +943,10 @@ abstract class SolrIndex extends SearchIndex
         $this->service = $service;
         return $this;
     }
-    
+
     /**
      * Upload config for this index to the given store
-     * 
+     *
      * @param SolrConfigStore $store
      */
     public function uploadConfig($store)

tests/SolrIndexSubsitesTest.php

@@ -0,0 +1,145 @@
+<?php
+
+if (class_exists('Phockito')) {
+    Phockito::include_hamcrest(false);
+}
+
+/**
+ * Subsite specific solr testing
+ */
+class SolrIndexSubsitesTest extends SapphireTest {
+
+    public static $fixture_file = 'SolrIndexSubsitesTest.yml';
+
+    /**
+     * @var SolrIndexSubsitesTest_Index
+     */
+    private static $index = null;
+
+    protected $server = null;
+
+    public function setUp()
+    {
+        // Prevent parent::setUp() crashing on db build
+        if (!class_exists('Subsite')) {
+            $this->skipTest = true;
+        }
+
+        parent::setUp();
+
+        $this->server = $_SERVER;
+
+        if (!class_exists('Phockito')) {
+            $this->skipTest = true;
+            $this->markTestSkipped("These tests need the Phockito module installed to run");
+            return;
+        }
+
+        // Check versioned available
+        if (!class_exists('Subsite')) {
+            $this->skipTest = true;
+            $this->markTestSkipped('The subsite module is not installed');
+            return;
+        }
+
+        if (self::$index === null) {
+            self::$index = singleton('SolrIndexSubsitesTest_Index');
+        }
+
+        SearchUpdater::bind_manipulation_capture();
+
+        Config::inst()->update('Injector', 'SearchUpdateProcessor', array(
+            'class' => 'SearchUpdateImmediateProcessor'
+        ));
+
+        FullTextSearch::force_index_list(self::$index);
+        SearchUpdater::clear_dirty_indexes();
+    }
+
+    public function tearDown()
+    {
+        if($this->server) {
+            $_SERVER = $this->server;
+            $this->server = null;
+        }
+        parent::tearDown();
+    }
+
+    protected function getServiceMock()
+    {
+        return Phockito::mock('Solr4Service');
+    }
+
+    /**
+     * @param DataObject $object Item being added
+     * @param int $subsiteID
+     * @param string $stage
+     * @return string
+     */
+    protected function getExpectedDocumentId($object, $subsiteID, $stage = null)
+    {
+        $id = $object->ID;
+        $class = ClassInfo::baseDataClass($object);
+        $variants = array();
+
+        // Check subsite
+        if(class_exists('Subsite') && $object->hasOne('Subsite')) {
+            $variants[] = '"SearchVariantSubsites":"' . $subsiteID. '"';
+        }
+
+        // Check versioned
+        if($stage) {
+            $variants[] = '"SearchVariantVersioned":"' . $stage . '"';
+        }
+        return $id.'-'.$class.'-{'.implode(',',$variants).'}';
+    }
+
+    public function testPublishing()
+    {
+        // Setup mocks
+        $serviceMock = $this->getServiceMock();
+        self::$index->setService($serviceMock);
+
+        $subsite1 = $this->objFromFixture('Subsite', 'subsite1');
+
+        // Add records to first subsite
+        Versioned::reading_stage('Stage');
+        $_SERVER['HTTP_HOST'] = 'www.subsite1.com';
+        Phockito::reset($serviceMock);
+        $file = new File();
+        $file->Title = 'My File';
+        $file->SubsiteID = $subsite1->ID;
+        $file->write();
+        $page = new Page();
+        $page->Title = 'My Page';
+        $page->SubsiteID = $subsite1->ID;
+        $page->write();
+        SearchUpdater::flush_dirty_indexes();
+        $doc1 = new SolrDocumentMatcher(array(
+            '_documentid' => $this->getExpectedDocumentId($page, $subsite1->ID, 'Stage'),
+            'ClassName' => 'Page',
+            'SiteTree_Title' => 'My Page',
+            '_versionedstage' => 'Stage',
+            '_subsite' => $subsite1->ID
+        ));
+        $doc2 = new SolrDocumentMatcher(array(
+            '_documentid' => $this->getExpectedDocumentId($file, $subsite1->ID),
+            'ClassName' => 'File',
+            'File_Title' => 'My File',
+            '_subsite' => $subsite1->ID
+        ));
+        Phockito::verify($serviceMock)->addDocument($doc1);
+        Phockito::verify($serviceMock)->addDocument($doc2);
+
+    }
+}
+
+class SolrIndexSubsitesTest_Index extends SolrIndex
+{
+    public function init()
+    {
+        $this->addClass('File');
+        $this->addClass('SiteTree');
+        $this->addAllFulltextFields();
+    }
+}

tests/SolrIndexSubsitesTest.yml

@@ -0,0 +1,16 @@
+Subsite:
+  main:
+    Title: Template
+  subsite1:
+    Title: 'Subsite1 Template'
+  subsite2:
+    Title: 'Subsite2 Template'
+SubsiteDomain:
+  subsite1:
+    SubsiteID: =>Subsite.subsite1
+    Domain: www.subsite1.com
+    Protocol: automatic
+  subsite2:
+    SubsiteID: =>Subsite.subsite2
+    Domain: www.subsite2.com
+    Protocol: automatic

tests/SolrIndexVersionedTest.php

@@ -11,7 +11,8 @@ class SolrIndexVersionedTest extends SapphireTest
     protected static $index = null;
 
     protected $extraDataObjects = array(
-        'SearchVariantVersionedTest_Item'
+        'SearchVariantVersionedTest_Item',
+        'SolrIndexVersionedTest_Object',
     );
 
     public function setUp()
@@ -20,13 +21,15 @@ class SolrIndexVersionedTest extends SapphireTest
 
         if (!class_exists('Phockito')) {
             $this->skipTest = true;
-            return $this->markTestSkipped("These tests need the Phockito module installed to run");
+            $this->markTestSkipped("These tests need the Phockito module installed to run");
+            return;
         }
 
         // Check versioned available
         if (!class_exists('Versioned')) {
             $this->skipTest = true;
-            return $this->markTestSkipped('The versioned decorator is not installed');
+            $this->markTestSkipped('The versioned decorator is not installed');
+            return;
         }
 
         if (self::$index === null) {
@@ -57,11 +60,21 @@ class SolrIndexVersionedTest extends SapphireTest
         return Phockito::mock('Solr3Service');
     }
 
-    protected function getExpectedDocumentId($id, $stage)
+    /**
+     * @param DataObject $object Item being added
+     * @param string $stage
+     * @return string
+     */
+    protected function getExpectedDocumentId($object, $stage)
     {
+        $id = $object->ID;
+        $class = ClassInfo::baseDataClass($object);
         // Prevent subsites from breaking tests
-        $subsites = class_exists('Subsite') ? '"SearchVariantSubsites":"0",' : '';
-        return $id.'-SiteTree-{'.$subsites.'"SearchVariantVersioned":"'.$stage.'"}';
+        $subsites = '';
+        if(class_exists('Subsite') && $object->hasOne('Subsite')) {
+            $subsites = '"SearchVariantSubsites":"0",';
+        }
+        return $id.'-'.$class.'-{'.$subsites.'"SearchVariantVersioned":"'.$stage.'"}';
     }
 
     public function testPublishing()
@@ -74,32 +87,54 @@ class SolrIndexVersionedTest extends SapphireTest
         // Check that write updates Stage
         Versioned::reading_stage('Stage');
         Phockito::reset($serviceMock);
-        $item = new SearchVariantVersionedTest_Item(array('Title' => 'Foo'));
+        $item = new SearchVariantVersionedTest_Item(array('TestText' => 'Foo'));
         $item->write();
+        $object = new SolrIndexVersionedTest_Object(array('TestText' => 'Bar'));
+        $object->write();
         SearchUpdater::flush_dirty_indexes();
-        $doc = new SolrDocumentMatcher(array(
-            '_documentid' => $this->getExpectedDocumentId($item->ID, 'Stage'),
-            'ClassName' => 'SearchVariantVersionedTest_Item'
+        $doc1 = new SolrDocumentMatcher(array(
+            '_documentid' => $this->getExpectedDocumentId($item, 'Stage'),
+            'ClassName' => 'SearchVariantVersionedTest_Item',
+            'SearchVariantVersionedTest_Item_TestText' => 'Foo',
+            '_versionedstage' => 'Stage'
         ));
-        Phockito::verify($serviceMock)->addDocument($doc);
+        $doc2 = new SolrDocumentMatcher(array(
+            '_documentid' => $this->getExpectedDocumentId($object, 'Stage'),
+            'ClassName' => 'SolrIndexVersionedTest_Object',
+            'SolrIndexVersionedTest_Object_TestText' => 'Bar',
+            '_versionedstage' => 'Stage'
+        ));
+        Phockito::verify($serviceMock)->addDocument($doc1);
+        Phockito::verify($serviceMock)->addDocument($doc2);
 
         // Check that write updates Live
         Versioned::reading_stage('Stage');
         Phockito::reset($serviceMock);
-        $item = new SearchVariantVersionedTest_Item(array('Title' => 'Bar'));
+        $item = new SearchVariantVersionedTest_Item(array('TestText' => 'Foo'));
         $item->write();
         $item->publish('Stage', 'Live');
+        $object = new SolrIndexVersionedTest_Object(array('TestText' => 'Bar'));
+        $object->write();
+        $object->publish('Stage', 'Live');
         SearchUpdater::flush_dirty_indexes();
         $doc = new SolrDocumentMatcher(array(
-            '_documentid' => $this->getExpectedDocumentId($item->ID, 'Live'),
-            'ClassName' => 'SearchVariantVersionedTest_Item'
+            '_documentid' => $this->getExpectedDocumentId($item, 'Live'),
+            'ClassName' => 'SearchVariantVersionedTest_Item',
+            'SearchVariantVersionedTest_Item_TestText' => 'Foo',
+            '_versionedstage' => 'Live'
+        ));
+        $doc2 = new SolrDocumentMatcher(array(
+            '_documentid' => $this->getExpectedDocumentId($object, 'Live'),
+            'ClassName' => 'SolrIndexVersionedTest_Object',
+            'SolrIndexVersionedTest_Object_TestText' => 'Bar',
+            '_versionedstage' => 'Live'
         ));
         Phockito::verify($serviceMock)->addDocument($doc);
+        Phockito::verify($serviceMock)->addDocument($doc2);
     }
 
     public function testDelete()
     {
-
         // Setup mocks
         $serviceMock = $this->getServiceMock();
         self::$index->setService($serviceMock);
@@ -107,11 +142,11 @@ class SolrIndexVersionedTest extends SapphireTest
         // Delete the live record (not the stage)
         Versioned::reading_stage('Stage');
         Phockito::reset($serviceMock);
-        $item = new SearchVariantVersionedTest_Item(array('Title' => 'Too'));
+        $item = new SearchVariantVersionedTest_Item(array('TestText' => 'Too'));
         $item->write();
         $item->publish('Stage', 'Live');
         Versioned::reading_stage('Live');
-        $id = $item->ID;
+        $id = clone $item;
         $item->delete();
         SearchUpdater::flush_dirty_indexes();
         Phockito::verify($serviceMock, 1)
@@ -122,10 +157,10 @@ class SolrIndexVersionedTest extends SapphireTest
         // Delete the stage record
         Versioned::reading_stage('Stage');
         Phockito::reset($serviceMock);
-        $item = new SearchVariantVersionedTest_Item(array('Title' => 'Too'));
+        $item = new SearchVariantVersionedTest_Item(array('TestText' => 'Too'));
         $item->write();
         $item->publish('Stage', 'Live');
-        $id = $item->ID;
+        $id = clone $item;
         $item->delete();
         SearchUpdater::flush_dirty_indexes();
         Phockito::verify($serviceMock, 1)
@@ -141,7 +176,9 @@ class SolrVersionedTest_Index extends SolrIndex
     public function init()
     {
         $this->addClass('SearchVariantVersionedTest_Item');
+        $this->addClass('SolrIndexVersionedTest_Object');
         $this->addFilterField('TestText');
+        $this->addFulltextField('Content');
     }
 }
 
@@ -178,3 +215,19 @@ class SolrDocumentMatcher extends Hamcrest_BaseMatcher
         return true;
     }
 }
+
+/**
+ * Non-sitetree versioned dataobject
+ */
+class SolrIndexVersionedTest_Object extends DataObject implements TestOnly {
+
+    private static $extensions = array(
+        'Versioned'
+    );
+
+    private static $db = array(
+        'Title' => 'Varchar',
+        'Content' => 'Text',
+        'TestText' => 'Varchar',
+    );
+}

tests/SolrReindexTest.php

@@ -39,7 +39,7 @@ class SolrReindexTest extends SapphireTest
             $this->skipTest = true;
             return $this->markTestSkipped("These tests need the Phockito module installed to run");
         }
-        
+
         // Set test handler for reindex
         Config::inst()->update('Injector', 'SolrReindexHandler', array(
             'class' => 'SolrReindexTest_TestHandler'
@@ -416,19 +416,19 @@ class SolrReindexTest_Variant extends SearchVariant implements TestOnly
         }
     }
 
-    public function alterDefinition($base, $index)
+    public function alterDefinition($class, $index)
     {
         $self = get_class($this);
 
-        $index->filterFields['_testvariant'] = array(
+        $this->addFilterField($index, '_testvariant', array(
             'name' => '_testvariant',
             'field' => '_testvariant',
             'fullfield' => '_testvariant',
-            'base' => $base,
-            'origin' => $base,
+            'base' => ClassInfo::baseDataClass($class),
+            'origin' => $class,
             'type' => 'Int',
             'lookup_chain' => array(array('call' => 'variant', 'variant' => $self, 'method' => 'currentState'))
-        );
+        ));
     }
 
     public function alterQuery($query, $index)