r52972 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r52971‎ | r52972 | r52973 >
Date:13:27, 9 July 2009
Author:werdna
Status:deferred
Tags:
Comment:
* Add thread-splitting mechanism (TODO write merging functionality)
* When bulk-loading a thread, also bulk-load all subthreads.
* When renaming a thread, poke the subject in subthreads too (includes lazy update to fix corruption).
* When moving a thread, make sure the article info is fixed for all subthreads too (includes lazy update to fix corruption).
* Fix hasDistinctSubject, is useless and seems to have several bugs. Now just a wrapper for isTopmost()
* Refactor MoveThread to be a subclass of a new special-page type, ThreadActionPage (TODO convert DeleteThread).
* Fix other misc. bugs in bulk-loading of threads, to reduce number of queries on page view.
Modified paths:
  • /trunk/extensions/LiquidThreads/LiquidThreads.php (modified) (history)
  • /trunk/extensions/LiquidThreads/classes/LqtDispatch.php (modified) (history)
  • /trunk/extensions/LiquidThreads/classes/LqtThread.php (modified) (history)
  • /trunk/extensions/LiquidThreads/classes/LqtThreads.php (modified) (history)
  • /trunk/extensions/LiquidThreads/classes/LqtView.php (modified) (history)
  • /trunk/extensions/LiquidThreads/i18n/Lqt.alias.php (modified) (history)
  • /trunk/extensions/LiquidThreads/i18n/Lqt.i18n.php (modified) (history)
  • /trunk/extensions/LiquidThreads/pages/SpecialMoveThread.php (modified) (history)
  • /trunk/extensions/LiquidThreads/pages/ThreadPermalinkView.php (modified) (history)

Diff [purge]

Index: trunk/extensions/LiquidThreads/i18n/Lqt.i18n.php
——@@ -160,6 +160,12 @@
161161 'lqt_summarize_link' => 'Summarize',
162162 'lqt-summarize-intro' => 'Please summarize the below thread in the editing box.
163163 You may use any wikitext in your summary. When you finish, click "{{int:savearticle}}".',
 164+ 'lqt-thread-split' => 'Split to new thread',
 165+ 'lqt-split-success' => 'You have successfully split off the thread $1.',
 166+ 'lqt_split_thread' => 'Split a thread',
 167+ 'lqt-thread-split-subject'=> 'New thread subject',
 168+ 'lqt-split-submit' => 'Split',
 169+ 'lqt_split_badsubject' => 'The subject you entered is invalid.',
164170
165171 // Logging
166172 'lqt-log-name' => 'Threaded discussion log',
Index: trunk/extensions/LiquidThreads/i18n/Lqt.alias.php
——@@ -13,6 +13,7 @@
1414 'DeleteThread' => array( 'DeleteThread' ),
1515 'MoveThread' => array( 'MoveThread' ),
1616 'NewMessages' => array( 'NewMessages' ),
 17+ 'SplitThread' => array( 'SplitThread' ),
1718 );
1819
1920 /** Arabic (العربية)
Index: trunk/extensions/LiquidThreads/pages/SpecialMoveThread.php
——@@ -2,14 +2,8 @@
33
44 if ( !defined( 'MEDIAWIKI' ) ) die;
55
6 -class SpecialMoveThread extends UnlistedSpecialPage {
7 - private $user, $output, $request, $title, $thread;
 6+class SpecialMoveThread extends ThreadActionPage {
87
9 - function __construct() {
10 - parent::__construct( 'Movethread' );
11 - $this->includable( false );
12 - }
13 -
148 /**
159 * @see SpecialPage::getDescription
1610 */
——@@ -34,53 +28,36 @@
3529 ),
3630 );
3731 }
38 -
39 - function execute( $par ) {
 32+
 33+ function getPageName() { return 'MoveThread'; }
 34+
 35+ function getSubmitText() {
4036 wfLoadExtensionMessages( 'LiquidThreads' );
 37+ return wfMsg( 'lqt_move_move' );
 38+ }
 39+
 40+ function buildForm() {
 41+ $form = parent::buildForm();
4142
42 - global $wgOut;
43 -
44 - // Page title
45 - $wgOut->setPageTitle( wfMsg( 'lqt_movethread' ) );
46 -
47 - // Handle parameter
48 - $this->mTarget = $par;
49 - if ( $par === null || $par === "" ) {
50 - wfLoadExtensionMessages( 'LiquidThreads' );
51 - $this->output->addHTML( wfMsg( 'lqt_threadrequired' ) );
52 - return;
53 - }
54 - $thread = Threads::withRoot( new Article( Title::newFromURL( $par ) ) );
55 - if ( !$thread ) {
56 - $this->output->addHTML( wfMsg( 'lqt_nosuchthread' ) );
57 - return;
58 - }
59 - $this->mThread = $thread;
60 -
6143 // Generate introduction
6244 $intro = '';
6345
6446 global $wgUser;
6547 $sk = $wgUser->getSkin();
66 - $page = $article_name = $thread->article()->getTitle()->getPrefixedText();
 48+ $page = $article_name = $this->mThread->article()->getTitle()->getPrefixedText();
6749
6850 $edit_text = wfMsgExt( 'lqt_move_torename_edit', 'parseinline' );
69 - $edit_link = $sk->link( $thread->title(), $edit_text, array(),
70 - array( 'lqt_method' => 'edit', 'lqt_operand' => $thread->id() ) );
 51+ $edit_link = $sk->link( $this->mThread->title(), $edit_text, array(),
 52+ array( 'lqt_method' => 'edit', 'lqt_operand' => $this->mThread->id() ) );
7153
7254 $intro .= wfMsgExt( 'lqt_move_movingthread', 'parse',
7355 array('[['.$this->mTarget.']]', '[['.$page.']]') );
7456 $intro .= wfMsgExt( 'lqt_move_torename', array( 'parse', 'replaceafter' ),
7557 array( $edit_link ) );
76 -
77 - $form = new HTMLForm( $this->getFormFields(), 'lqt-move' );
78 -
79 - $form->setSubmitText( wfMsg('lqt_move_move') );
80 - $form->setTitle( SpecialPage::getTitleFor( 'MoveThread', $par ) );
81 - $form->setSubmitCallback( array( $this, 'trySubmit' ) );
 58+
8259 $form->setIntro( $intro );
8360
84 - $form->show();
 61+ return $form;
8562 }
8663
8764 function checkUserRights( $oldTitle, $newTitle ) {
Index: trunk/extensions/LiquidThreads/pages/ThreadPermalinkView.php
——@@ -157,6 +157,8 @@
158158
159159 if ( $this->methodApplies( 'summarize' ) )
160160 $this->showSummarizeForm( $this->thread );
 161+ elseif ( $this->methodApplies( 'split' ) )
 162+ $this->showSplitForm( $this->thread );
161163
162164 $this->showThread( $this->thread );
163165 return false;
Index: trunk/extensions/LiquidThreads/LiquidThreads.php
——@@ -57,9 +57,11 @@
5858 $wgHooks['LanguageGetMagic'][] = 'LiquidThreadsMagicWords::getMagicWords';
5959
6060 // Special pages
 61+#$wgSpecialPages['UndeleteThread'] = 'SpecialUndeleteThread';
6162 $wgSpecialPages['DeleteThread'] = 'SpecialDeleteThread';
6263 $wgSpecialPages['MoveThread'] = 'SpecialMoveThread';
6364 $wgSpecialPages['NewMessages'] = 'SpecialNewMessages';
 65+$wgSpecialPages['SplitThread'] = 'SpecialSplitThread';
6466 $wgSpecialPageGroups['NewMessages'] = 'wiki';
6567
6668 // Classes
——@@ -72,7 +74,7 @@
7375 $wgAutoloadClasses['Threads'] = $dir . 'classes/LqtThreads.php';
7476 $wgAutoloadClasses['NewMessages'] = $dir . 'classes/LqtNewMessages.php';
7577 $wgAutoloadClasses['LiquidThreadsMagicWords'] = $dir . 'i18n/LiquidThreads.magic.php';
76 -$wgAutoloadClasses['LqtParserFunctions'] = $dir . 'classes/LqtParserFunctions.php'; // File does not exist
 78+$wgAutoloadClasses['LqtParserFunctions'] = $dir . 'classes/LqtParserFunctions.php';
7779
7880 // Page classes
7981 $wgAutoloadClasses['TalkpageView'] = $dir . 'pages/TalkpageView.php';
——@@ -86,10 +88,15 @@
8789 $wgAutoloadClasses['ThreadHistoryListingView'] = $dir . 'pages/ThreadHistoryListingView.php';
8890 $wgAutoloadClasses['ThreadHistoricalRevisionView'] = $dir . 'pages/ThreadHistoricalRevisionView.php';
8991 $wgAutoloadClasses['SummaryPageView'] = $dir . 'pages/SummaryPageView.php';
 92+$wgAutoloadClasses['NewUserMessagesView'] = $dir . 'pages/NewUserMessagesView.php';
 93+
 94+$wgAutoloadClasses['ThreadActionPage'] = "$dir/pages/ThreadActionPage.php";
 95+
 96+#$wgAutoloadClasses['SpecialUndeleteThread'] = $dir . "pages/SpecialUndeleteThread.php";
9097 $wgAutoloadClasses['SpecialMoveThread'] = $dir . 'pages/SpecialMoveThread.php';
9198 $wgAutoloadClasses['SpecialDeleteThread'] = $dir . 'pages/SpecialDeleteThread.php';
92 -$wgAutoloadClasses['NewUserMessagesView'] = $dir . 'pages/NewUserMessagesView.php';
9399 $wgAutoloadClasses['SpecialNewMessages'] = $dir . 'pages/SpecialNewMessages.php';
 100+$wgAutoloadClasses['SpecialSplitThread'] = "$dir/pages/SpecialSplitThread.php";
94101
95102 // Logging
96103 $wgLogTypes[] = 'liquidthreads';
Index: trunk/extensions/LiquidThreads/classes/LqtThreads.php
——@@ -16,9 +16,12 @@
1717 const CHANGE_DELETED = 4;
1818 const CHANGE_UNDELETED = 5;
1919 const CHANGE_MOVED_TALKPAGE = 6;
 20+ const CHANGE_SPLIT = 7;
 21+ const CHANGE_EDITED_SUBJECT = 8;
 22+
2023 static $VALID_CHANGE_TYPES = array( self::CHANGE_EDITED_SUMMARY, self::CHANGE_EDITED_ROOT,
2124 self::CHANGE_REPLY_CREATED, self::CHANGE_NEW_THREAD, self::CHANGE_DELETED, self::CHANGE_UNDELETED,
22 - self::CHANGE_MOVED_TALKPAGE );
 25+ self::CHANGE_MOVED_TALKPAGE, self::CHANGE_SPLIT, self::CHANGE_EDITED_SUBJECT );
2326
2427 // Possible values of Thread->editedness.
2528 const EDITED_NEVER = 0;
——@@ -151,7 +154,6 @@
152155 $dbr = wfGetDB( DB_SLAVE );
153156
154157 $res = $dbr->select( 'thread', '*', $where, __METHOD__, $options );
155 -
156158 $threads = Threads::loadFromResult( $res, $dbr );
157159
158160 foreach ( $threads as $thread ) {
——@@ -201,7 +203,8 @@
202204 if ( array_key_exists( $id, self::$cache_by_id ) ) {
203205 return self::$cache_by_id[$id];
204206 }
205 - $ts = Threads::where( array( 'thread.thread_id' => $id ) );
 207+ $ts = Threads::where( array( 'thread_id' => $id ) );
 208+
206209 return self::assertSingularity( $ts, 'thread_id', $id );
207210 }
208211
Index: trunk/extensions/LiquidThreads/classes/LqtView.php
——@@ -562,6 +562,13 @@
563563 'href' => $delete_url,
564564 'enabled' => true );
565565 }
 566+
 567+ if ( !$thread->isTopmostThread() ) {
 568+ $splitUrl = SpecialPage::getTitleFor( 'SplitThread',
 569+ $thread->title()->getPrefixedText() )->getFullURL();
 570+ $commands['split'] = array( 'label' => wfMsgExt( 'lqt-thread-split', 'parseinline' ),
 571+ 'href' => $splitUrl, 'enabled' => true );
 572+ }
566573
567574 return $commands;
568575 }
——@@ -873,7 +880,7 @@
874881 }
875882
876883 static function anchorName( $thread ) {
877 - return "lqt_thread_{$thread->id()}";
 884+ return $thread->getAnchorName();
878885 }
879886
880887 // Gets HTML for the 'in reply to' thing if warranted.
——@@ -1085,5 +1092,4 @@
10861093 function showSummary( $t ) {
10871094 $this->output->addHTML( $this->getSummary( $t ) );
10881095 }
1089 -
10901096 }
Index: trunk/extensions/LiquidThreads/classes/LqtDispatch.php
——@@ -80,8 +80,7 @@
8181 }
8282 else if ( $action == 'watch' || $action == 'unwatch' ) {
8383 $viewname = self::$views['ThreadWatchView'];
84 - }
85 - else {
 84+ } else {
8685 $viewname = self::$views['ThreadPermalinkView'];
8786 }
8887 $view = new $viewname( $output, $article, $title, $user, $request );
Index: trunk/extensions/LiquidThreads/classes/LqtThread.php
——@@ -52,6 +52,7 @@
5353 protected $replies;
5454
5555 static $titleCacheById = array();
 56+ static $replyCacheById = array();
5657
5758 function isHistorical() {
5859 return false;
——@@ -217,17 +218,18 @@
218219
219220 $new_articleNamespace = $title->getNamespace();
220221 $new_articleTitle = $title->getDBkey();
 222+
 223+ // Update on *all* subthreads.
 224+ $dbr->update( 'thread',
 225+ array(
 226+ 'thread_revision=thread_revision+1',
 227+ 'thread_article_namespace' => $new_articleNamespace,
 228+ 'thread_article_title' => $new_articleTitle,
 229+ 'thread_modified' => $dbr->timestamp( wfTimestampNow() ),
 230+ ),
 231+ array( 'thread_ancestor' => $this->id() ),
 232+ __METHOD__ );
221233
222 - foreach ( $this->replies() as $r ) {
223 - $res = $dbr->update( 'thread',
224 - /* SET */array(
225 - 'thread_revision' => $r->revisionNumber() + 1,
226 - 'thread_article_namespace' => $new_articleNamespace,
227 - 'thread_article_title' => $new_articleTitle ),
228 - /* WHERE */ array( 'thread_id' => $r->id(), ),
229 - __METHOD__ );
230 - }
231 -
232234 $this->articleNamespace = $new_articleNamespace;
233235 $this->articleTitle = $new_articleTitle;
234236 $this->revisionNumber += 1;
——@@ -270,7 +272,7 @@
271273 __METHOD__ );
272274
273275 $thread = Threads::newThread( $redirectArticle, $this->double->article(), null,
274 - Threads::TYPE_MOVED, $log );
 276+ Threads::TYPE_MOVED, $this->subject() );
275277
276278 # Purge old title from squid
277279 # The new title, and links to the new title, are purged in Article::onArticleCreate()
——@@ -329,22 +331,47 @@
330332 $this->doLazyUpdates( $line );
331333 }
332334
333 - // Load a list of threads in bulk.
334 - static function bulkLoad( $rows ) {
335 - $threads = array();
336 -
337 - // Preload page data in one swoop.
 335+ // Load a list of threads in bulk, including all subthreads.
 336+ static function bulkLoad( $rows ) {
 337+ // Preload subthreads
 338+ $thread_ids = array();
338339 $pageIds = array();
 340+ $replies_by_id = array();
339341
340342 foreach( $rows as $row ) {
 343+ $thread_ids[] = $row->thread_id;
 344+
 345+ // Grab page data while we're here.
341346 if ($row->thread_root)
342347 $pageIds[] = $row->thread_root;
343348 if ($row->thread_summary_page)
344349 $pageIds[] = $row->thread_summary_page;
345350 }
346351
 352+ if ( count($thread_ids) ) {
 353+ $dbr = wfGetDB( DB_SLAVE );
 354+ $res = $dbr->select( 'thread', '*', array( 'thread_ancestor' => $thread_ids ),
 355+ __METHOD__ );
 356+
 357+ while( $row = $dbr->fetchObject($res) ) {
 358+ // Grab page data while we're here.
 359+ if ($row->thread_root)
 360+ $pageIds[] = $row->thread_root;
 361+ if ($row->thread_summary_page)
 362+ $pageIds[] = $row->thread_summary_page;
 363+
 364+ if (!isset( $replies_by_id[$row->thread_parent] ) ) {
 365+ $replies_by_id[$row->thread_parent] = array();
 366+ }
 367+
 368+ $replies_by_id[$row->thread_parent][] = new Thread( $row, null );
 369+ }
 370+ }
 371+
 372+ self::$replyCacheById = array_merge( self::$replyCacheById, $replies_by_id );
 373+
 374+ // Preload page data in one swoop.
347375 if ( count($pageIds) ) {
348 - $dbr = wfGetDB( DB_SLAVE );
349376 $res = $dbr->select( 'page', '*', array( 'page_id' => $pageIds ), __METHOD__ );
350377 while( $row = $dbr->fetchObject( $res ) ) {
351378 $t = Title::newFromRow( $row );
——@@ -357,8 +384,11 @@
358385 }
359386 }
360387
 388+ $threads = array();
 389+
361390 foreach( $rows as $row ) {
362 - $threads[] = new Thread( $row, null );
 391+ $threads[] = $thread = new Thread( $row, null );
 392+ Threads::$cache_by_id[$row->thread_id] = $thread;
363393 }
364394
365395 return $threads;
——@@ -374,13 +404,17 @@
375405 $doingUpdates = true;
376406
377407 // Fix missing ancestry information.
 408+ // (there was a bug where this was not saved properly)
378409 if ($this->parentId &&!$this->ancestorId) {
379410 $this->fixMissingAncestor();
380411 }
381412
 413+ $ancestor = $this->topmostThread();
 414+
382415 $set = array();
383416
384417 // Fix missing subject information
 418+ // (this information only started to be added later)
385419 if (!$this->subject) {
386420 $detectedSubject = $this->root()->getTitle()->getText();
387421 $parts = self::splitIncrementFromSubject( $detectedSubject );
——@@ -391,7 +425,16 @@
392426 $set['thread_subject'] = $detectedSubject;
393427 }
394428
 429+ // Fix inconsistent subject information
 430+ // (in some intermediate versions this was not updated when the subject was changed)
 431+ if ($this->subject() != $ancestor->subject()) {
 432+ $set['thread_subject'] = $ancestor->subject();
 433+
 434+ $this->subject = $ancestor->subject();
 435+ }
 436+
395437 // Fix missing authorship information
 438+ // (this information only started to be added later)
396439 if ( !$this->authorName ) {
397440 $author = $this->root()->originalAuthor();
398441
——@@ -402,6 +445,19 @@
403446 $set['thread_author_id'] = $this->authorId;
404447 }
405448
 449+ // Check for article corruption from incomplete thread moves.
 450+ // (thread moves only updated this on immediate replies, not replies to replies etc)
 451+ if (! $ancestor->article()->getTitle()->equals( $this->article()->getTitle() ) ) {
 452+ $title = $ancestor->article()->getTitle();
 453+ $set['thread_article_namespace'] = $title->getNamespace();
 454+ $set['thread_article_title'] = $title->getDbKey();
 455+
 456+ $this->articleNamespace = $title->getNamespace();
 457+ $this->articleTitle = $title->getDbKey();
 458+
 459+ $this->article = $ancestor->article();
 460+ }
 461+
406462 if ( count($set) ) {
407463 $dbw = wfGetDB( DB_MASTER );
408464
——@@ -467,6 +523,12 @@
468524 return $this->replies;
469525 }
470526
 527+ // Check cache
 528+ if ( isset( self::$replyCacheById[$this->id()] ) ) {
 529+ $this->replies = self::$replyCacheById[$this->id()];
 530+ return $this->replies;
 531+ }
 532+
471533 $this->replies = array();
472534
473535 $dbr = wfGetDB( DB_SLAVE );
——@@ -485,6 +547,12 @@
486548 }
487549
488550 function setSuperthread( $thread ) {
 551+ if ($thread == null) {
 552+ $this->parentId = null;
 553+ $this->ancestorId = $this->id();
 554+ return;
 555+ }
 556+
489557 $this->parentId = $thread->id();
490558
491559 if ( $thread->isTopmostThread() ) {
——@@ -523,6 +591,14 @@
524592 return $thread;
525593 }
526594 }
 595+
 596+ function setAncestor( $newAncestor ) {
 597+ if ( is_object( $newAncestor ) ) {
 598+ $this->ancestorId = $newAncestor->id();
 599+ } else {
 600+ $this->ancestorId = $newAncestor;
 601+ }
 602+ }
527603
528604 // Due to a bug in earlier versions, the topmost thread sometimes isn't there.
529605 // Fix the corruption by repeatedly grabbing the parent until we hit the topmost thread.
——@@ -656,6 +732,11 @@
657733
658734 function setSubject( $subject ) {
659735 $this->subject = $subject;
 736+
 737+ foreach( $this->replies() as $reply ) {
 738+ $reply->setSubject( $subject );
 739+ $reply->commitRevision( CHANGE_EDITED_SUBJECT );
 740+ }
660741 }
661742
662743 function wikilinkWithoutIncrement() {
——@@ -667,12 +748,7 @@
668749 }
669750
670751 function hasDistinctSubject() {
671 - if ( $this->hasSuperthread() ) {
672 - return $this->superthread()->subjectWithoutIncrement()
673 - != $this->subjectWithoutIncrement();
674 - } else {
675 - return true;
676 - }
 752+ return $this->isTopmostThread();
677753 }
678754
679755 function hasSubthreads() {
——@@ -799,4 +875,8 @@
800876 function getArchiveStartDays() {
801877 return Threads::getArticleArchiveStartDays( $this->article() );
802878 }
 879+
 880+ function getAnchorName() {
 881+ return "lqt_thread_{$this->id()}";
 882+ }
803883 }

Status & tagging log