Support updating search index when page is deleted
authorChad Horohoe <chadh@wikimedia.org>
Tue, 18 Jun 2013 20:06:49 +0000 (16:06 -0400)
committerChad Horohoe <chadh@wikimedia.org>
Tue, 18 Jun 2013 21:41:13 +0000 (17:41 -0400)
Right now, if you delete a page then you end up with a stale
entry in the search index, this affects all core SQL-based
searches.

For extensions, this means they no longer have to implement
something like ArticleDelete, they can just add the delete()
method to their SearchEngine subclass.

Change-Id: I9d8a9878aeebc53f453ab1cbacc03fe73fcca949

includes/WikiPage.php
includes/search/SearchEngine.php
includes/search/SearchMySQL.php
includes/search/SearchUpdate.php
tests/phpunit/includes/search/SearchUpdateTest.php

index 5ed6c8d..9e6a0c8 100644 (file)
@@ -2648,7 +2648,7 @@ class WikiPage implements Page, IDBAccessObject {
        /**
         * Do some database updates after deletion
         *
-        * @param int $id page_id value of the page being deleted (B/C, currently unused)
+        * @param int $id page_id value of the page being deleted
         * @param $content Content: optional page content to be used when determining the required updates.
         *        This may be needed because $this->getContent() may already return null when the page proper was deleted.
         */
@@ -2665,6 +2665,9 @@ class WikiPage implements Page, IDBAccessObject {
 
                // Reset this object and the Title object
                $this->loadFromRow( false, self::READ_LATEST );
+
+               // Search engine
+               DeferredUpdates::addUpdate( new SearchUpdate( $id, $this->mTitle ) );
        }
 
        /**
index a975b03..00f7923 100644 (file)
@@ -494,6 +494,18 @@ class SearchEngine {
                // no-op
        }
 
+       /**
+        * Delete an indexed page
+        * Title should be pre-processed.
+        * STUB
+        *
+        * @param Integer $id Page id that was deleted
+        * @param String $title Title of page that was deleted
+        */
+       function delete( $id, $title ) {
+               // no-op
+       }
+
        /**
         * Get OpenSearch suggestion template
         *
index c219b92..d8c638e 100644 (file)
@@ -369,6 +369,19 @@ class SearchMySQL extends SearchEngine {
                        array( $dbw->lowPriorityOption() ) );
        }
 
+       /**
+        * Delete an indexed page
+        * Title should be pre-processed.
+        *
+        * @param Integer $id Page id that was deleted
+        * @param String $title Title of page that was deleted
+        */
+       function delete( $id, $title ) {
+               $dbw = wfGetDB( DB_MASTER );
+
+               $dbw->delete( 'searchindex', array( 'si_page' => $id ), __METHOD__ );
+       }
+
        /**
         * Converts some characters for MySQL's indexing to grok it correctly,
         * and pads short words to overcome limitations.
index 8c1bda2..759c7b9 100644 (file)
@@ -33,7 +33,7 @@ class SearchUpdate implements DeferrableUpdate {
        private $mId = 0, $mNamespace, $mTitle, $mText;
        private $mTitleWords;
 
-       function __construct( $id, $title, $text = false ) {
+       public function __construct( $id, $title, $text = false ) {
                if ( is_string( $title ) ) {
                        $nt = Title::newFromText( $title );
                } else {
@@ -53,8 +53,8 @@ class SearchUpdate implements DeferrableUpdate {
                }
        }
 
-       function doUpdate() {
-               global $wgContLang, $wgDisableSearchUpdate;
+       public function doUpdate() {
+               global $wgDisableSearchUpdate;
 
                if ( $wgDisableSearchUpdate || !$this->mId ) {
                        return;
@@ -63,17 +63,34 @@ class SearchUpdate implements DeferrableUpdate {
                wfProfileIn( __METHOD__ );
 
                $search = SearchEngine::create();
-               $lc = SearchEngine::legalSearchChars() . '&#;';
+               $normalTitle = $search->normalizeText( Title::indexTitle( $this->mNamespace, $this->mTitle ) );
 
-               if ( $this->mText === false ) {
-                       $search->updateTitle( $this->mId,
-                               $search->normalizeText( Title::indexTitle( $this->mNamespace, $this->mTitle ) ) );
+               if ( WikiPage::newFromId( $this->mId ) === null ) {
+                       $search->delete( $this->mId, $normalTitle );
+                       wfProfileOut( __METHOD__ );
+                       return;
+               } elseif ( $this->mText === false ) {
+                       $search->updateTitle( $this->mId, $normalTitle );
                        wfProfileOut( __METHOD__ );
                        return;
                }
 
+               $text = self::updateText( $this->mText );
+
+               wfRunHooks( 'SearchUpdate', array( $this->mId, $this->mNamespace, $this->mTitle, &$text ) );
+
+               # Perform the actual update
+               $search->update( $this->mId, $normalTitle, $search->normalizeText( $text ) );
+
+               wfProfileOut( __METHOD__ );
+       }
+
+       public static function updateText( $text ) {
+               global $wgContLang;
+
                # Language-specific strip/conversion
-               $text = $wgContLang->normalizeForSearch( $this->mText );
+               $text = $wgContLang->normalizeForSearch( $text );
+               $lc = SearchEngine::legalSearchChars() . '&#;';
 
                wfProfileIn( __METHOD__ . '-regexps' );
                $text = preg_replace( "/<\\/?\\s*[A-Za-z][^>]*?>/",
@@ -123,14 +140,7 @@ class SearchUpdate implements DeferrableUpdate {
                # Strip wiki '' and '''
                $text = preg_replace( "/''[']*/", " ", $text );
                wfProfileOut( __METHOD__ . '-regexps' );
-
-               wfRunHooks( 'SearchUpdate', array( $this->mId, $this->mNamespace, $this->mTitle, &$text ) );
-
-               # Perform the actual update
-               $search->update( $this->mId, $search->normalizeText( Title::indexTitle( $this->mNamespace, $this->mTitle ) ),
-                               $search->normalizeText( $text ) );
-
-               wfProfileOut( __METHOD__ );
+               return $text;
        }
 }
 
index d19cdd0..e947c34 100644 (file)
@@ -17,6 +17,7 @@ class MockSearch extends SearchEngine {
 
 /**
  * @group Search
+ * @group Database
  */
 class SearchUpdateTest extends MediaWikiTestCase {
 
@@ -25,17 +26,8 @@ class SearchUpdateTest extends MediaWikiTestCase {
                $this->setMwGlobals( 'wgSearchType', 'MockSearch' );
        }
 
-       function update( $text, $title = 'Test', $id = 1 ) {
-               $u = new SearchUpdate( $id, $title, $text );
-               $u->doUpdate();
-
-               return array( MockSearch::$title, MockSearch::$text );
-       }
-
        function updateText( $text ) {
-               list( , $resultText ) = $this->update( $text );
-               $resultText = trim( $resultText ); // abstract from some implementation details
-               return $resultText;
+               return trim( SearchUpdate::updateText( $text ) );
        }
 
        function testUpdateText() {