r93047 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r93046‎ | r93047 | r93048 >
Date:10:16, 25 July 2011
Author:questpc
Status:deferred
Tags:
Comment:
Question randomizer now works with newly created pages. Value of poll randomize attribute is stored into poll description table, which is now used to distinguish non-randomized polls (no extra SQL query to load randomized questions list in such case anymore).
Modified paths:
  • /trunk/extensions/QPoll/archives/qpoll_interpretation.src (modified) (history)
  • /trunk/extensions/QPoll/ctrl/qp_poll.php (modified) (history)
  • /trunk/extensions/QPoll/qp_pollstore.php (modified) (history)
  • /trunk/extensions/QPoll/qp_results.php (modified) (history)
  • /trunk/extensions/QPoll/tables/qpoll_random_questions.src (modified) (history)
  • /trunk/extensions/QPoll/tables/qpoll_trunk.sql (modified) (history)

Diff [purge]

Index: trunk/extensions/QPoll/qp_results.php
——@@ -348,7 +348,6 @@
349349 $userTitle = Title::makeTitleSafe( NS_USER, $userName );
350350 $user_link = $this->qpLink( $userTitle, $userName );
351351 $pollStore->setLastUser( $userName, false );
352 - $pollStore->loadRandomQuestions();
353352 if ( !$pollStore->loadUserVote() ) {
354353 return '';
355354 }
Index: trunk/extensions/QPoll/archives/qpoll_interpretation.src
——@@ -1,8 +1,9 @@
 2+-- fields to store the interpretation template title and random questions count
23 ALTER TABLE /*$wgDBprefix*/qp_poll_desc
34 DROP INDEX article_poll,
45 ADD COLUMN interpretation_namespace int NOT NULL default 0,
56 ADD COLUMN interpretation_title varchar(255) binary NOT NULL default '',
 7+ ADD COLUMN random_question_count int NOT NULL default 0,
68 ADD UNIQUE INDEX article_poll (article_id,poll_id(128));
79
810 -- fields to store the interpretation results
Index: trunk/extensions/QPoll/ctrl/qp_poll.php
——@@ -127,10 +127,11 @@
128128 }
129129 }
130130 $newPollStore = array(
131 - 'poll_id'=>$this->mPollId,
132 - 'order_id'=>$this->mOrderId,
133 - 'dependance'=>$this->dependsOn,
134 - 'interpretation'=>$this->interpretation
 131+ 'poll_id' => $this->mPollId,
 132+ 'order_id' => $this->mOrderId,
 133+ 'randomQuestionCount' => $this->randomQuestionCount,
 134+ 'dependance' => $this->dependsOn,
 135+ 'interpretation' => $this->interpretation
135136 );
136137 if ( ( $dependanceResult = $this->checkDependance( $this->dependsOn ) ) !== true ) {
137138 # return an error string
——@@ -173,34 +174,22 @@
174175 }
175176
176177 function setUsedQuestions() {
177 - # todo: make global settings to perform all of this conditionally
178 - # load random questions from DB (when available)
179 - # setLastUser will not load/store user data, when pid is null
180 - $this->pollStore->setLastUser( $this->username );
181 - # loadRandomQuestions will call setPid() and setLastUser() in such case
182 - $this->pollStore->loadRandomQuestions();
183 - if ( $this->randomQuestionCount > 0 ) {
184 - if ( $this->randomQuestionCount > $this->questions->totalCount() ) {
185 - $this->randomQuestionCount = $this->questions->totalCount();
186 - }
187 - if ( is_array( $this->pollStore->randomQuestions ) ) {
188 - if ( count( $this->pollStore->randomQuestions ) == $this->randomQuestionCount ) {
189 - # count of random questions was not changed, no need to regenerate seed
190 - $this->questions->setUsedQuestions( $this->pollStore->randomQuestions );
191 - return;
192 - }
193 - }
194 - # generate or regenerate random questions
195 - $this->questions->randomize( $this->randomQuestionCount );
196 - $this->pollStore->randomQuestions = $this->questions->getUsedQuestions();
197 - } else {
198 - if ( !is_array( $this->pollStore->randomQuestions ) ) {
199 - # random questions are disabled and no previous seed in DB
 178+ if ( $this->randomQuestionCount === 0 ) {
 179+ return;
 180+ }
 181+ if ( $this->randomQuestionCount > $this->questions->totalCount() ) {
 182+ $this->randomQuestionCount = $this->questions->totalCount();
 183+ }
 184+ if ( is_array( $this->pollStore->randomQuestions ) ) {
 185+ if ( count( $this->pollStore->randomQuestions ) == $this->randomQuestionCount ) {
 186+ # count of random questions was not changed, no need to regenerate seed
 187+ $this->questions->setUsedQuestions( $this->pollStore->randomQuestions );
200188 return;
201189 }
202 - # there was stored random seed, will remove it at the end of this function
203 - $this->pollStore->randomQuestions = false;
204190 }
 191+ # generate or regenerate random questions
 192+ $this->questions->randomize( $this->randomQuestionCount );
 193+ $this->pollStore->randomQuestions = $this->questions->getUsedQuestions();
205194 # store random questions into DB
206195 $this->pollStore->setRandomQuestions();
207196 }
Index: trunk/extensions/QPoll/qp_pollstore.php
——@@ -184,6 +184,10 @@
185185 class qp_PollStore {
186186
187187 static $db = null;
 188+ # indicates whether random questions must be erased / regenerated when the value of
 189+ # 'randomize' attribute is changed from non-zero to zero and back
 190+ static $purgeRandomQuestions = false;
 191+
188192 /// DB keys
189193 var $pid = null;
190194 var $last_uid = null;
——@@ -191,30 +195,37 @@
192196 # username is used for caching of setLastUser() method (which now may be called multiple times);
193197 # also used by randomizer
194198 var $username = '';
195 - /// common properties
 199+
 200+ /*** common properties ***/
196201 var $mArticleId = null;
197202 # unique id of poll, used for addressing, also with 'qp_' prefix as the fragment part of the link
198203 var $mPollId = null;
199204 # order of poll on the page
200205 var $mOrderId = null;
 206+
 207+ /*** optional attributes ***/
201208 # dependance from other poll address in the following format: "page#otherpollid"
202209 var $dependsOn = null;
203 - # attempts of voting (passing the quiz). number of resubmits
204 - # note: resubmits are counted for syntax-correct answer (the vote is stored),
205 - # yet the answer still might be logically incorrect (quiz is not passed / partially passed)
206 - var $attempts = 0;
207 -
208210 # NS & DBkey of Title object representing interpretation template for Special:Pollresults page
209211 var $interpNS = 0;
210212 var $interpDBkey = null;
211213 # interpretation of user answer
212214 var $interpResult;
 215+ # 1..n - number of random indexes from poll's header; 0 - poll questions are not randomized
 216+ # pollstore loads / saves random indexes for every user only when this property is NOT zero
 217+ # which improves performance of non-randomized polls
 218+ var $randomQuestionCount = null;
213219
214220 # array of QuestionData instances (data from/to DB)
215221 var $Questions = null;
216222 # array of random indexes of Questions[] array (optional)
217223 var $randomQuestions = false;
218224
 225+ # attempts of voting (passing the quiz). number of resubmits
 226+ # note: resubmits are counted for syntax-correct answer (when the vote is stored),
 227+ # yet the answer still might be logically incorrect (quiz is not passed / partially passed)
 228+ var $attempts = 0;
 229+
219230 # poll processing state, read with getState()
220231 #
221232 # 'NA' - object just was created
——@@ -277,29 +288,42 @@
278289 }
279290 }
280291 }
281 - if ( $is_post ) {
282 - $this->setPid();
283 - } else {
284 - $this->loadPid();
 292+ if ( array_key_exists( 'randomQuestionCount', $argv ) ) {
 293+ $this->randomQuestionCount = $argv['randomQuestionCount'];
285294 }
 295+ # do not load / create the poll when article id is unavailable
 296+ # (only during newly created page submission)
 297+ if ( $this->mArticleId != 0 ) {
 298+ if ( $is_post ) {
 299+ $this->setPid();
 300+ } else {
 301+ $this->loadPid();
 302+ if ( is_null( $this->pid ) ) {
 303+ # try to create poll description (DB state was incomplete)
 304+ $this->setPid();
 305+ }
 306+ }
 307+ }
286308 break;
287309 case 'pid' :
288310 if ( array_key_exists( 'pid', $argv ) ) {
289311 $pid = intval( $argv[ 'pid' ] );
290312 $res = self::$db->select( 'qp_poll_desc',
291 - array( 'article_id', 'poll_id','order_id', 'dependance', 'interpretation_namespace', 'interpretation_title' ),
 313+ array( 'article_id', 'poll_id','order_id', 'dependance', 'interpretation_namespace', 'interpretation_title', 'random_question_count' ),
292314 array( 'pid'=>$pid ),
293315 __METHOD__ . ":create from pid" );
294316 $row = self::$db->fetchObject( $res );
295 - if ( $row!=false ) {
296 - $this->pid = $pid;
297 - $this->mArticleId = $row->article_id;
298 - $this->mPollId = $row->poll_id;
299 - $this->mOrderId = $row->order_id;
300 - $this->dependsOn = $row->dependance;
301 - $this->interpNS = $row->interpretation_namespace;
302 - $this->interpDBkey = $row->interpretation_title;
 317+ if ( $row === false ) {
 318+ throw new MWException( 'Attempt to create poll from non-existent poll id in ' . __METHOD__ );
303319 }
 320+ $this->pid = $pid;
 321+ $this->mArticleId = $row->article_id;
 322+ $this->mPollId = $row->poll_id;
 323+ $this->mOrderId = $row->order_id;
 324+ $this->dependsOn = $row->dependance;
 325+ $this->interpNS = $row->interpretation_namespace;
 326+ $this->interpDBkey = $row->interpretation_title;
 327+ $this->randomQuestionCount = $row->random_question_count;
304328 }
305329 break;
306330 }
——@@ -338,11 +362,20 @@
339363
340364 # returns Title object, to get a URI path, use Title::getFullText()/getPrefixedText() on it
341365 function getTitle() {
342 - $res = null;
343 - if ( $this->mArticleId !==null && $this->mPollId !== null ) {
344 - $res = Title::newFromID( $this->mArticleId );
345 - $res->setFragment( qp_AbstractPoll::s_getPollTitleFragment( $this->mPollId ) );
 366+ if ( $this->mArticleId === 0 ) {
 367+ throw new MWException( __METHOD__ . ' cannot be called for unsaved new pages' );
346368 }
 369+ if ( is_null( $this->mArticleId ) ) {
 370+ throw new MWException( 'Unknown article id in ' . __METHOD__ );
 371+ }
 372+ if ( is_null( $this->mPollId ) ) {
 373+ throw new MWException( 'Unknown poll id in ' . __METHOD__ );
 374+ }
 375+ $res = Title::newFromID( $this->mArticleId );
 376+ $res->setFragment( qp_AbstractPoll::s_getPollTitleFragment( $this->mPollId ) );
 377+ if ( !( $res instanceof Title ) ) {
 378+ throw new MWException( 'Invalid title created in ' . __METHOD__ );
 379+ }
347380 return $res;
348381 }
349382
——@@ -699,9 +732,15 @@
700733 * Will be overriden in memory when number of random questions was changed
701734 */
702735 function loadRandomQuestions() {
 736+ if ( $this->mArticleId == 0 ) {
 737+ $this->randomQuestions = false;
 738+ return;
 739+ }
 740+ if ( is_null( $this->pid ) ) {
 741+ throw new MWException( __METHOD__ . ' cannot be called when pid was not set' );
 742+ }
703743 if ( is_null( $this->last_uid ) ) {
704 - $this->setPid();
705 - $this->setLastUser( $this->username );
 744+ throw new MWException( __METHOD__ . ' cannot be called when uid was not set' );
706745 }
707746 $res = self::$db->select( 'qp_random_questions', 'question_id', array( 'uid' => $this->last_uid, 'pid' => $this->pid ), __METHOD__ );
708747 $this->randomQuestions = array();
——@@ -710,6 +749,8 @@
711750 }
712751 if ( count( $this->randomQuestions ) === 0 ) {
713752 $this->randomQuestions = false;
 753+ } else {
 754+ sort( $this->randomQuestions, SORT_NUMERIC );
714755 }
715756 }
716757
——@@ -720,9 +761,15 @@
721762 * when number of random questions for poll was changed
722763 */
723764 function setRandomQuestions() {
724 - if ( is_null( $this->pid ) || is_null( $this->last_uid ) ) {
725 - throw new MWException( __METHOD__ . ' cannot be called when pid/uid was not set' );
 765+ if ( $this->mArticleId == 0 ) {
 766+ return;
726767 }
 768+ if ( is_null( $this->pid ) ) {
 769+ throw new MWException( __METHOD__ . ' cannot be called when pid was not set' );
 770+ }
 771+ if ( is_null( $this->last_uid ) ) {
 772+ throw new MWException( __METHOD__ . ' cannot be called when uid was not set' );
 773+ }
727774 if ( is_array( $this->randomQuestions ) ) {
728775 $data = array();
729776 foreach( $this->randomQuestions as $qidx ) {
——@@ -740,7 +787,7 @@
741788 }
742789 # this->randomQuestions === false; this poll is not randomized anymore
743790 self::$db->delete( 'qp_random_questions',
744 - array( 'pid'=>$this->pid ),
 791+ array( 'pid'=>$this->pid, 'uid'=>$this->last_uid ),
745792 __METHOD__ . ':remove question random seed'
746793 );
747794 }
——@@ -763,6 +810,7 @@
764811 $this->username = $username;
765812 } else {
766813 $this->last_uid = null;
 814+ return;
767815 }
768816 } else {
769817 $this->last_uid = intval( $row->uid );
——@@ -780,6 +828,10 @@
781829 $this->interpResult->short = $row->short_interpretation;
782830 $this->interpResult->long = $row->long_interpretation;
783831 }
 832+ $this->randomQuestions = false;
 833+ if ( $this->randomQuestionCount != 0 ) {
 834+ $this->loadRandomQuestions();
 835+ }
784836 // todo: change to "insert ... on duplicate key update ..." when last_insert_id() bugs will be fixed
785837 }
786838
——@@ -795,10 +847,12 @@
796848 }
797849
798850 private function loadPid() {
 851+ if ( $this->mArticleId === 0 ) {
 852+ return;
 853+ }
799854 $res = self::$db->select( 'qp_poll_desc',
800 - array( 'pid', 'order_id', 'dependance', 'interpretation_namespace', 'interpretation_title' ),
801 - 'article_id=' . self::$db->addQuotes( $this->mArticleId ) . ' and ' .
802 - 'poll_id=' . self::$db->addQuotes( $this->mPollId ),
 855+ array( 'pid', 'order_id', 'dependance', 'interpretation_namespace', 'interpretation_title', 'random_question_count' ),
 856+ array( 'article_id' => $this->mArticleId, 'poll_id' => $this->mPollId ),
803857 __METHOD__ );
804858 $row = self::$db->fetchObject( $res );
805859 if ( $row != false ) {
——@@ -814,19 +868,25 @@
815869 $this->interpNS = $row->interpretation_namespace;
816870 $this->interpDBkey = $row->interpretation_title;
817871 }
 872+ if ( is_null( $this->randomQuestionCount ) ) {
 873+ $this->randomQuestionCount = $row->random_question_count;
 874+ }
818875 $this->updatePollAttributes( $row );
819876 }
820877 }
821878
822879 private function setPid() {
 880+ if ( $this->mArticleId === 0 ) {
 881+ throw new MWException( 'Cannot save new poll description during new page preprocess in ' . __METHOD__ );
 882+ }
823883 $res = self::$db->select( 'qp_poll_desc',
824 - array( 'pid', 'order_id', 'dependance', 'interpretation_namespace', 'interpretation_title' ),
 884+ array( 'pid', 'order_id', 'dependance', 'interpretation_namespace', 'interpretation_title', 'random_question_count' ),
825885 'article_id=' . self::$db->addQuotes( $this->mArticleId ) . ' and ' .
826886 'poll_id=' . self::$db->addQuotes( $this->mPollId ) );
827887 $row = self::$db->fetchObject( $res );
828888 if ( $row == false ) {
829889 self::$db->insert( 'qp_poll_desc',
830 - array( 'article_id'=>$this->mArticleId, 'poll_id'=>$this->mPollId, 'order_id'=>$this->mOrderId, 'dependance'=>$this->dependsOn, 'interpretation_namespace'=>$this->interpNS, 'interpretation_title'=>$this->interpDBkey ),
 890+ array( 'article_id'=>$this->mArticleId, 'poll_id'=>$this->mPollId, 'order_id'=>$this->mOrderId, 'dependance'=>$this->dependsOn, 'interpretation_namespace'=>$this->interpNS, 'interpretation_title'=>$this->interpDBkey, 'random_question_count'=>$this->randomQuestionCount ),
831891 __METHOD__ . ':update poll' );
832892 $this->pid = self::$db->insertId();
833893 } else {
——@@ -837,16 +897,27 @@
838898 }
839899
840900 private function updatePollAttributes( $row ) {
 901+ self::$db->begin();
841902 if ( $this->mOrderId != $row->order_id ||
842903 $this->dependsOn != $row->dependance ||
843904 $this->interpNS != $row->interpretation_namespace ||
844 - $this->interpDBkey != $row->interpretation_title ) {
 905+ $this->interpDBkey != $row->interpretation_title ||
 906+ ( $rqcChanged = $this->randomQuestionCount != $row->random_question_count ) ) {
845907 $res = self::$db->replace( 'qp_poll_desc',
846908 array( 'poll', 'article_poll' ),
847 - array( 'pid'=>$this->pid, 'article_id'=>$this->mArticleId, 'poll_id'=>$this->mPollId, 'order_id'=>$this->mOrderId, 'dependance'=>$this->dependsOn, 'interpretation_namespace'=>$this->interpNS, 'interpretation_title'=>$this->interpDBkey ),
 909+ array( 'pid'=>$this->pid, 'article_id'=>$this->mArticleId, 'poll_id'=>$this->mPollId, 'order_id'=>$this->mOrderId, 'dependance'=>$this->dependsOn, 'interpretation_namespace'=>$this->interpNS, 'interpretation_title'=>$this->interpDBkey, 'random_question_count'=>$this->randomQuestionCount ),
848910 __METHOD__ . ':poll attributes update'
849911 );
850912 }
 913+ if ( $rqcChanged &&
 914+ $this->randomQuestionCount == 0 &&
 915+ self::$purgeRandomQuestions ) {
 916+ # the poll questions are not randomized anymore
 917+ self::$db->delete( 'qp_random_questions',
 918+ array( 'pid' => $this->pid ),
 919+ __METHOD__ . ':delete unused random seeds' );
 920+ }
 921+ self::$db->commit();
851922 }
852923
853924 private function setQuestionDesc() {
Index: trunk/extensions/QPoll/tables/qpoll_random_questions.src
——@@ -3,5 +3,6 @@
44 `pid` int unsigned NOT NULL,
55 `question_id` int unsigned NOT NULL,
66 PRIMARY KEY user_poll_question (uid,pid,question_id),
7 - INDEX user_seed (uid,pid)
 7+ INDEX user_seed (uid,pid),
 8+ INDEX poll (pid)
89 ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
Index: trunk/extensions/QPoll/tables/qpoll_trunk.sql
——@@ -12,6 +12,7 @@
1313 `dependance` mediumtext NOT NULL,
1414 interpretation_namespace int NOT NULL,
1515 interpretation_title varchar(255) binary NOT NULL,
 16+ random_question_count int NOT NULL default 0,
1617 PRIMARY KEY poll (pid),
1718 UNIQUE INDEX article_poll (article_id,poll_id(128))
1819 ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
——@@ -85,5 +86,6 @@
8687 `pid` int unsigned NOT NULL,
8788 `question_id` int unsigned NOT NULL,
8889 PRIMARY KEY user_poll_question (uid,pid,question_id),
89 - INDEX user_seed (uid,pid)
 90+ INDEX user_seed (uid,pid),
 91+ INDEX poll (pid)
9092 ) ENGINE=InnoDB DEFAULT CHARSET=utf8;

Status & tagging log