From a2cfbb531bd67b413c2a73a465aa65298b5b6412 Mon Sep 17 00:00:00 2001 From: Darren Inwood Date: Tue, 25 Mar 2014 09:55:13 +1300 Subject: [PATCH] BUG Fix sold indexing storing against the incorrect class key --- _config/config.yml | 3 + code/search/SearchIndex.php | 7 +- code/search/SearchUpdater.php | 27 +++++ code/solr/SolrIndex.php | 3 +- docs/en/changelogs/1.0.3.md | 20 ++++ tests/SearchVariantVersionedTest.php | 12 ++ tests/SolrIndexVersionedTest.php | 159 +++++++++++++++++++++++++++ 7 files changed, 227 insertions(+), 4 deletions(-) create mode 100644 _config/config.yml create mode 100644 docs/en/changelogs/1.0.3.md create mode 100644 tests/SolrIndexVersionedTest.php diff --git a/_config/config.yml b/_config/config.yml new file mode 100644 index 0000000..6907361 --- /dev/null +++ b/_config/config.yml @@ -0,0 +1,3 @@ +DataObject: + extensions: + - 'SearchUpdater_DeleteHandler' diff --git a/code/search/SearchIndex.php b/code/search/SearchIndex.php index 1dcc40c..2a2527c 100644 --- a/code/search/SearchIndex.php +++ b/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; } } } diff --git a/code/search/SearchUpdater.php b/code/search/SearchUpdater.php index 7847170..dd55b31 100644 --- a/code/search/SearchUpdater.php +++ b/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); + } + +} diff --git a/code/solr/SolrIndex.php b/code/solr/SolrIndex.php index 45324ef..2c70ad5 100644 --- a/code/solr/SolrIndex.php +++ b/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); } } diff --git a/docs/en/changelogs/1.0.3.md b/docs/en/changelogs/1.0.3.md new file mode 100644 index 0000000..305f761 --- /dev/null +++ b/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) diff --git a/tests/SearchVariantVersionedTest.php b/tests/SearchVariantVersionedTest.php index 3c1e25c..408b275 100644 --- a/tests/SearchVariantVersionedTest.php +++ b/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 diff --git a/tests/SolrIndexVersionedTest.php b/tests/SolrIndexVersionedTest.php new file mode 100644 index 0000000..562576e --- /dev/null +++ b/tests/SolrIndexVersionedTest.php @@ -0,0 +1,159 @@ +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; + } + +}