BUG Fix versioned writes where subtables have no fields key

BUG Remove unnecessary config nesting in tests which are now handled via core
Fixes some regressions in recent framework fixes to versioned

code/search/SearchUpdater.php

@@ -90,14 +90,14 @@ class SearchUpdater extends Object
         $writes = array();
 
         foreach ($manipulation as $table => $details) {
-            if (!isset($details['id']) || !isset($details['fields'])) {
+            if (!isset($details['id'])) {
                 continue;
             }
 
             $id = $details['id'];
             $state = $details['state'];
             $class = $details['class'];
-            $fields = $details['fields'];
+            $fields = isset($details['fields']) ? $details['fields'] : array();
 
             $base = ClassInfo::baseDataClass($class);
             $key = "$id:$base:".serialize($state);
@@ -125,6 +125,13 @@ class SearchUpdater extends Object
             }
         }
 
+        // Trim records without fields
+        foreach(array_keys($writes) as $key) {
+            if(empty($writes[$key]['fields'])) {
+                unset($writes[$key]);
+            }
+        }
+
         // Then extract any state that is needed for the writes
 
         SearchVariant::call('extractManipulationWriteState', $writes);
@@ -136,7 +143,7 @@ class SearchUpdater extends Object
 
     /**
      * Send updates to the current search processor for execution
-     * 
+     *
      * @param array $writes
      */
     public static function process_writes($writes)

tests/BatchedProcessorTest.php

@@ -42,7 +42,7 @@ class BatchedProcessor_QueuedJobService
 class BatchedProcessorTest extends SapphireTest
 {
     protected $oldProcessor;
-    
+
     protected $extraDataObjects = array(
         'BatchedProcessorTest_Object'
     );
@@ -66,8 +66,7 @@ class BatchedProcessorTest extends SapphireTest
     public function setUp()
     {
         parent::setUp();
-        Config::nest();
+
-        
         if (!interface_exists('QueuedJob')) {
             $this->skipTest = true;
             $this->markTestSkipped("These tests need the QueuedJobs module installed to run");
@@ -79,11 +78,11 @@ class BatchedProcessorTest extends SapphireTest
         }
 
         SS_Datetime::set_mock_now('2015-05-07 06:00:00');
-        
+
         Config::inst()->update('SearchUpdateBatchedProcessor', 'batch_size', 5);
         Config::inst()->update('SearchUpdateBatchedProcessor', 'batch_soft_cap', 0);
         Config::inst()->update('SearchUpdateCommitJobProcessor', 'cooldown', 600);
-        
+
         Versioned::reading_stage("Stage");
 
         Injector::inst()->registerService(new BatchedProcessor_QueuedJobService(), 'QueuedJobService');
@@ -92,7 +91,7 @@ class BatchedProcessorTest extends SapphireTest
 
         SearchUpdateCommitJobProcessor::$dirty_indexes = array();
         SearchUpdateCommitJobProcessor::$has_run = false;
-        
+
         $this->oldProcessor = SearchUpdater::$processor;
         SearchUpdater::$processor = new SearchUpdateQueuedJobProcessor();
     }
@@ -102,8 +101,6 @@ class BatchedProcessorTest extends SapphireTest
         if ($this->oldProcessor) {
             SearchUpdater::$processor = $this->oldProcessor;
         }
-        Config::unnest();
-        Injector::inst()->unregisterNamedObject('QueuedJobService');
         FullTextSearch::force_index_list();
         parent::tearDown();
     }
@@ -132,7 +129,7 @@ class BatchedProcessorTest extends SapphireTest
         $processor->batchData();
         return $processor;
     }
-    
+
     /**
      * Tests that large jobs are broken up into a suitable number of batches
      */
@@ -141,14 +138,14 @@ class BatchedProcessorTest extends SapphireTest
         $index = singleton('BatchedProcessorTest_Index');
         $index->reset();
         $processor = $this->generateDirtyIds();
-        
+
         // Check initial state
         $data = $processor->getJobData();
         $this->assertEquals(9, $data->totalSteps);
         $this->assertEquals(0, $data->currentStep);
         $this->assertEmpty($data->isComplete);
         $this->assertEquals(0, count($index->getAdded()));
-        
+
         // Advance state
         for ($pass = 1; $pass <= 8; $pass++) {
             $processor->process();
@@ -156,7 +153,7 @@ class BatchedProcessorTest extends SapphireTest
             $this->assertEquals($pass, $data->currentStep);
             $this->assertEquals($pass * 5, count($index->getAdded()));
         }
-        
+
         // Last run should have two hanging items
         $processor->process();
         $data = $processor->getJobData();
@@ -218,7 +215,7 @@ class BatchedProcessorTest extends SapphireTest
         );
     }
 
-    
+
     /**
      * Tests that the batch_soft_cap setting is properly respected
      */
@@ -227,26 +224,26 @@ class BatchedProcessorTest extends SapphireTest
         $index = singleton('BatchedProcessorTest_Index');
         $index->reset();
         $processor = $this->generateDirtyIds();
-        
+
         // Test that increasing the soft cap to 2 will reduce the number of batches
         Config::inst()->update('SearchUpdateBatchedProcessor', 'batch_soft_cap', 2);
         $processor->batchData();
         $data = $processor->getJobData();
         //Debug::dump($data);die;
         $this->assertEquals(8, $data->totalSteps);
-        
+
         // A soft cap of 1 should not fit in the hanging two items
         Config::inst()->update('SearchUpdateBatchedProcessor', 'batch_soft_cap', 1);
         $processor->batchData();
         $data = $processor->getJobData();
         $this->assertEquals(9, $data->totalSteps);
-        
+
         // Extra large soft cap should fit both items
         Config::inst()->update('SearchUpdateBatchedProcessor', 'batch_soft_cap', 4);
         $processor->batchData();
         $data = $processor->getJobData();
         $this->assertEquals(8, $data->totalSteps);
-        
+
         // Process all data and ensure that all are processed adequately
         for ($pass = 1; $pass <= 8; $pass++) {
             $processor->process();

tests/SearchUpdaterTest.php

@@ -74,7 +74,7 @@ class SearchUpdaterTest extends SapphireTest
     protected $usesDatabase = true;
 
     private static $index = null;
-    
+
     public function setUp()
     {
         parent::setUp();
@@ -87,8 +87,6 @@ class SearchUpdaterTest extends SapphireTest
 
         SearchUpdater::bind_manipulation_capture();
 
-        Config::nest();
-
         Config::inst()->update('Injector', 'SearchUpdateProcessor', array(
             'class' => 'SearchUpdateImmediateProcessor'
         ));
@@ -97,13 +95,6 @@ class SearchUpdaterTest extends SapphireTest
         SearchUpdater::clear_dirty_indexes();
     }
 
-    public function tearDown()
-    {
-        Config::unnest();
-
-        parent::tearDown();
-    }
-
     public function testBasic()
     {
         $item = new SearchUpdaterTest_Container();

tests/SearchVariantVersionedTest.php

@@ -2,6 +2,9 @@
 
 class SearchVariantVersionedTest extends SapphireTest
 {
+    /**
+     * @var SearchVariantVersionedTest_Index
+     */
     private static $index = null;
 
     protected $extraDataObjects = array(
@@ -23,8 +26,6 @@ class SearchVariantVersionedTest extends SapphireTest
 
         SearchUpdater::bind_manipulation_capture();
 
-        Config::nest();
-
         Config::inst()->update('Injector', 'SearchUpdateProcessor', array(
             'class' => 'SearchUpdateImmediateProcessor'
         ));
@@ -33,13 +34,6 @@ class SearchVariantVersionedTest extends SapphireTest
         SearchUpdater::clear_dirty_indexes();
     }
 
-    public function tearDown()
-    {
-        Config::unnest();
-
-        parent::tearDown();
-    }
-
     public function testPublishing()
     {
         // Check that write updates Stage
@@ -71,9 +65,13 @@ class SearchVariantVersionedTest extends SapphireTest
         $item->write();
 
         SearchUpdater::flush_dirty_indexes();
-        $this->assertEquals(self::$index->getAdded(array('ID', '_versionedstage')), array(
+
-            array('ID' => $item->ID, '_versionedstage' => 'Stage')
+        $expected = array(array(
+            'ID' => $item->ID,
+            '_versionedstage' => 'Stage'
         ));
+        $added = self::$index->getAdded(array('ID', '_versionedstage'));
+        $this->assertEquals($expected, $added);
     }
 
     public function testExcludeVariantState()

tests/SolrIndexTest.php

@@ -12,12 +12,12 @@ class SolrIndexTest extends SapphireTest
 
     public function setUp()
     {
+        parent::setUp();
+
         if (!class_exists('Phockito')) {
             $this->markTestSkipped("These tests need the Phockito module installed to run");
             $this->skipTest = true;
         }
-
-        parent::setUp();
     }
 
     public function testFieldDataHasOne()
@@ -88,7 +88,7 @@ class SolrIndexTest extends SapphireTest
                 \Hamcrest_Matchers::anything()
             );
     }
-    
+
     /**
      * Test boosting on field schema (via queried fields parameter)
      */

tests/SolrIndexVersionedTest.php

@@ -7,9 +7,9 @@ if (class_exists('Phockito')) {
 class SolrIndexVersionedTest extends SapphireTest
 {
     protected $oldMode = null;
-    
+
     protected static $index = null;
-    
+
     protected $extraDataObjects = array(
         'SearchVariantVersionedTest_Item'
     );
@@ -17,7 +17,7 @@ class SolrIndexVersionedTest extends SapphireTest
     public function setUp()
     {
         parent::setUp();
-        
+
         if (!class_exists('Phockito')) {
             $this->skipTest = true;
             return $this->markTestSkipped("These tests need the Phockito module installed to run");
@@ -35,23 +35,20 @@ class SolrIndexVersionedTest extends SapphireTest
 
         SearchUpdater::bind_manipulation_capture();
 
-        Config::nest();
-
         Config::inst()->update('Injector', 'SearchUpdateProcessor', array(
             'class' => 'SearchUpdateImmediateProcessor'
         ));
 
         FullTextSearch::force_index_list(self::$index);
         SearchUpdater::clear_dirty_indexes();
-        
+
         $this->oldMode = Versioned::get_reading_mode();
         Versioned::reading_stage('Stage');
     }
-    
+
     public function tearDown()
     {
         Versioned::set_reading_mode($this->oldMode);
-        Config::unnest();
         parent::tearDown();
     }
 
@@ -59,21 +56,21 @@ class SolrIndexVersionedTest extends SapphireTest
     {
         return Phockito::mock('Solr3Service');
     }
-    
+
     protected function getExpectedDocumentId($id, $stage)
     {
         // Prevent subsites from breaking tests
         $subsites = class_exists('Subsite') ? '"SearchVariantSubsites":"0",' : '';
         return $id.'-SiteTree-{'.$subsites.'"SearchVariantVersioned":"'.$stage.'"}';
     }
-    
+
     public function testPublishing()
     {
-        
+
         // Setup mocks
         $serviceMock = $this->getServiceMock();
         self::$index->setService($serviceMock);
-        
+
         // Check that write updates Stage
         Versioned::reading_stage('Stage');
         Phockito::reset($serviceMock);
@@ -85,7 +82,7 @@ class SolrIndexVersionedTest extends SapphireTest
             'ClassName' => 'SearchVariantVersionedTest_Item'
         ));
         Phockito::verify($serviceMock)->addDocument($doc);
-        
+
         // Check that write updates Live
         Versioned::reading_stage('Stage');
         Phockito::reset($serviceMock);
@@ -99,14 +96,14 @@ class SolrIndexVersionedTest extends SapphireTest
         ));
         Phockito::verify($serviceMock)->addDocument($doc);
     }
-    
+
     public function testDelete()
     {
-        
+
         // Setup mocks
         $serviceMock = $this->getServiceMock();
         self::$index->setService($serviceMock);
-        
+
         // Delete the live record (not the stage)
         Versioned::reading_stage('Stage');
         Phockito::reset($serviceMock);
@@ -121,7 +118,7 @@ class SolrIndexVersionedTest extends SapphireTest
             ->deleteById($this->getExpectedDocumentId($id, 'Live'));
         Phockito::verify($serviceMock, 0)
             ->deleteById($this->getExpectedDocumentId($id, 'Stage'));
-        
+
         // Delete the stage record
         Versioned::reading_stage('Stage');
         Phockito::reset($serviceMock);
@@ -155,7 +152,7 @@ if (!class_exists('Phockito')) {
 class SolrDocumentMatcher extends Hamcrest_BaseMatcher
 {
     protected $properties;
-    
+
     public function __construct($properties)
     {
         $this->properties = $properties;
@@ -171,13 +168,13 @@ class SolrDocumentMatcher extends Hamcrest_BaseMatcher
         if (! ($item instanceof Apache_Solr_Document)) {
             return false;
         }
-        
+
         foreach ($this->properties as $key => $value) {
             if ($item->{$key} != $value) {
                 return false;
             }
         }
-        
+
         return true;
     }
 }