Merge pull request #39 from tractorcow/pulls/trigger-deletes

BUG Fix old indexing storing against the incorrect class key

_config/config.yml

@@ -0,0 +1,3 @@
+DataObject:
+  extensions:
+    - 'SearchUpdater_DeleteHandler'

code/search/SearchIndex.php

@@ -52,7 +52,7 @@ abstract class SearchIndex extends ViewableData {
 		$sources = $this->getClasses();
 
 		foreach ($sources as $source => $options) {
-			$sources[$source]['base'] = $source;
+			$sources[$source]['base'] = ClassInfo::baseDataClass($source);
 			$sources[$source]['lookup_chain'] = array();
 		}
 
@@ -440,10 +440,11 @@ abstract class SearchIndex extends ViewableData {
 		foreach ($this->classes as $searchclass => $options) {
 			if ($searchclass == $class || ($options['include_children'] && is_subclass_of($class, $searchclass))) {
 
-				$dirty[$searchclass] = array();
+				$base = ClassInfo::baseDataClass($searchclass);
+				$dirty[$base] = array();
 				foreach ($statefulids as $statefulid) {
 					$key = serialize($statefulid);
-					$dirty[$searchclass][$key] = $statefulid;
+					$dirty[$base][$key] = $statefulid;
 				}
 			}
 		}

code/search/SearchUpdater.php

@@ -165,3 +165,30 @@ class SearchUpdater_BindManipulationCaptureFilter implements RequestFilter {
 		/* NOP */
 	}
 }
+
+/**
+ * Delete operations do not use database manipulations.
+ * 
+ * If a delete has been requested, force a write on objects that should be
+ * indexed.  This causes the object to be marked for deletion from the index.
+ */
+
+class SearchUpdater_DeleteHandler extends DataExtension {
+
+	public function onAfterDelete() {
+		// Calling delete() on empty objects does nothing
+		if (!$this->owner->ID) return;
+		
+		// Force SearchUpdater to mark this record as dirty
+		$manipulation = array(
+			$this->owner->ClassName => array(
+				'fields' => array(),
+				'id' => $this->owner->ID,
+				'command' => 'update'
+			)
+		);
+		$this->owner->extend('augmentWrite', $manipulation);
+		SearchUpdater::handle_manipulation($manipulation);
+	}
+
+}

code/solr/SolrIndex.php

@@ -269,7 +269,8 @@ abstract class SolrIndex extends SearchIndex {
 
 		foreach ($this->getClasses() as $searchclass => $options) {
 			if ($searchclass == $class || ($options['include_children'] && is_subclass_of($class, $searchclass))) {
-				$docs[] = $this->_addAs($object, $searchclass, $options);
+				$base = ClassInfo::baseDataClass($searchclass);
+				$docs[] = $this->_addAs($object, $base, $options);
 			}
 		}
 

docs/en/changelogs/1.0.3.md

@@ -0,0 +1,20 @@
+# 1.0.3
+
+## Upgrading
+
+Users upgrading from 1.0.2 or below will need to run the Solr_Reindex task to refresh
+each SolrIndex. This is due to a change in record IDs, which are now generated from
+the base class of each DataObject, rather than the instance class.
+
+Developers working locally should be aware that by default, all indexes will be updated
+in realtime when the environment is in dev mode, rather than attempting to queue these
+updates with the queued jobs module (if installed).
+
+## Bugfixes
+
+ * BUG Fix old indexing storing against the incorrect class key
+ * [Don't rely on MySQL ordering of index->getAdded()](https://github.com/silverstripe-labs/silverstripe-fulltextsearch/commit/4b51393e014fc4c0cc8e192c74eb4594acaca605)
+
+## API
+
+ * [API Disable queued processing for development environments](https://github.com/silverstripe-labs/silverstripe-fulltextsearch/commit/71fc359b3711cf5b9429d86da0f1e0b20bd43dee)

tests/SearchVariantVersionedTest.php

@@ -38,10 +38,22 @@ class SearchVariantVersionedTest 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();
 	}
 
+	function tearDown() {
+		Config::unnest();
+
+		parent::tearDown();
+	}
+
 	function testPublishing() {
 		// Check that write updates Stage
 		

tests/SolrIndexVersionedTest.php

@@ -0,0 +1,159 @@
+<?php
+
+if (class_exists('Phockito')) Phockito::include_hamcrest();
+
+class SolrIndexVersionedTest extends SapphireTest {
+	
+	protected $oldMode = null;
+	
+	protected static $index = null;
+	
+	protected $extraDataObjects = array(
+		'SearchVariantVersionedTest_Item'
+	);
+
+	public function setUp() {
+
+		parent::setUp();
+		
+		if (!class_exists('Phockito')) {
+			$this->skipTest = true;
+			return $this->markTestSkipped("These tests need the Phockito module installed to run");
+		}
+
+		// Check versioned available
+		if(!class_exists('Versioned')) {
+			$this->skipTest = true;
+			return $this->markTestSkipped('The versioned decorator is not installed');
+		}
+
+		if (self::$index === null) self::$index = singleton('SolrVersionedTest_Index');
+
+		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();
+	}
+
+	protected function getServiceMock() {
+		return Phockito::mock('Solr3Service');
+	}
+	
+	
+	public function testPublishing() {
+		
+		// Setup mocks
+		$serviceMock = $this->getServiceMock();
+		self::$index->setService($serviceMock);
+		
+		// Check that write updates Stage
+		Versioned::reading_stage('Stage');
+		Phockito::reset($serviceMock);
+		$item = new SearchVariantVersionedTest_Item(array('Title' => 'Foo'));
+		$item->write();
+		SearchUpdater::flush_dirty_indexes();
+		$doc = new SolrDocumentMatcher(array(
+			'_documentid' => $item->ID.'-SiteTree-{"SearchVariantVersioned":"Stage"}',
+			'ClassName' => 'SearchVariantVersionedTest_Item'
+		));
+		Phockito::verify($serviceMock)->addDocument($doc);
+		
+		// Check that write updates Live
+		Versioned::reading_stage('Stage');
+		Phockito::reset($serviceMock);
+		$item = new SearchVariantVersionedTest_Item(array('Title' => 'Bar'));
+		$item->write();
+		$item->publish('Stage', 'Live');
+		SearchUpdater::flush_dirty_indexes();
+		$doc = new SolrDocumentMatcher(array(
+			'_documentid' => $item->ID.'-SiteTree-{"SearchVariantVersioned":"Live"}',
+			'ClassName' => 'SearchVariantVersionedTest_Item'
+		));
+		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);
+		$item = new SearchVariantVersionedTest_Item(array('Title' => 'Too'));
+		$item->write();
+		$item->publish('Stage', 'Live');
+		Versioned::reading_stage('Live');
+		$id = $item->ID;
+		$item->delete();
+		SearchUpdater::flush_dirty_indexes();
+		Phockito::verify($serviceMock, 1)
+			->deleteById($id.'-SiteTree-{"SearchVariantVersioned":"Live"}');
+		Phockito::verify($serviceMock, 0)
+			->deleteById($id.'-SiteTree-{"SearchVariantVersioned":"Stage"}');
+		
+		// Delete the stage record
+		Versioned::reading_stage('Stage');
+		Phockito::reset($serviceMock);
+		$item = new SearchVariantVersionedTest_Item(array('Title' => 'Too'));
+		$item->write();
+		$item->publish('Stage', 'Live');
+		$id = $item->ID;
+		$item->delete();
+		SearchUpdater::flush_dirty_indexes();
+		Phockito::verify($serviceMock, 1)
+			->deleteById($id.'-SiteTree-{"SearchVariantVersioned":"Stage"}');
+		Phockito::verify($serviceMock, 0)
+			->deleteById($id.'-SiteTree-{"SearchVariantVersioned":"Live"}');
+	}
+}
+
+
+class SolrVersionedTest_Index extends SolrIndex {
+	function init() {
+		$this->addClass('SearchVariantVersionedTest_Item');
+		$this->addFilterField('TestText');
+	}
+}
+
+
+class SolrDocumentMatcher extends Hamcrest_BaseMatcher {
+	
+	protected $properties;
+	
+	public function __construct($properties) {
+		$this->properties = $properties;
+	}
+
+	public function describeTo(\Hamcrest_Description $description) {
+		$description->appendText('Apache_Solr_Document with properties '.var_export($this->properties, true));
+	}
+
+	public function matches($item) {
+		
+		if(! ($item instanceof Apache_Solr_Document)) return false;
+		
+		foreach($this->properties as $key => $value) {
+			if($item->{$key} != $value) return false;
+		}
+		
+		return true;
+	}
+
+}