r44721 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r44720‎ | r44721 | r44722 >
Date:17:27, 17 December 2008
Author:ialex
Status:reverted (Comments)
Tags:
Comment:
* Short circuit EmailNotification::notify() to not call EmailNotification::commonMessageKeys() if there're no users to notify. This is a hack to work arround the following exception:
A database query syntax error has occurred.
The last attempted database query was:
"SELECT gu_id, lu_wiki, gu_salt, gu_password,gu_auth_token, gu_locked,gu_hidden,gu_registration,gu_email,gu_email_authenticated FROM `parsertest_globaluser` LEFT OUTER JOIN `parsertest_localuser` ON gu_name=lu_name AND lu_wiki='test2wiki-parsertest_' WHERE gu_name='127.0.0.1'"
from within function "Database::safeQuery".
MySQL returned error "1146: Table 'centralauth.parsertest_globaluser' doesn't exist (127.0.0.1)"
* Whitespaces fixes
Modified paths:
  • /trunk/phase3/includes/Hooks.php (modified) (history)
  • /trunk/phase3/includes/UserMailer.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/UserMailer.php
@@ -262,38 +262,41 @@
263263 * The recipient is appended to the arguments given to messageCompositionFunction.
264264 * Both callbacks are to be given in the same formats accepted by the hook system.
265265 */
266 - static function notify($editor, $timestamp, $userListFunction, $messageCompositionFunction) {
 266+ static function notify( $editor, $timestamp, $userListFunction, $messageCompositionFunction ) {
267267 global $wgEnotifUseRealName, $wgEnotifImpersonal;
268268 global $wgLang;
269269
270 - $common_keys = self::commonMessageKeys($editor);
271 - $users = wfInvoke("userList", $userListFunction);
272 - foreach($users as $u) {
273 - list($user_keys, $body_msg_name, $subj_msg_name) =
274 - wfInvoke("message", $messageCompositionFunction, array($u));
275 - $keys = array_merge($common_keys, $user_keys);
 270+ $users = wfInvoke( 'userList', $userListFunction );
 271+ if( !count( $users ) )
 272+ return;
276273
 274+ $common_keys = self::commonMessageKeys( $editor );
 275+ foreach( $users as $u ) {
 276+ list( $user_keys, $body_msg_name, $subj_msg_name ) =
 277+ wfInvoke( 'message', $messageCompositionFunction, array( $u ) );
 278+ $keys = array_merge( $common_keys, $user_keys );
 279+
277280 if( $wgEnotifImpersonal ) {
278 - $keys['$WATCHINGUSERNAME'] = wfMsgForContent('enotif_impersonal_salutation');
279 - $keys['$PAGEEDITDATE'] = $wgLang->timeanddate($timestamp, true, false, false);
 281+ $keys['$WATCHINGUSERNAME'] = wfMsgForContent( 'enotif_impersonal_salutation' );
 282+ $keys['$PAGEEDITDATE'] = $wgLang->timeanddate( $timestamp, true, false, false );
280283 } else {
281284 $keys['$WATCHINGUSERNAME'] = $wgEnotifUseRealName ? $u->getRealName() : $u->getName();
282 - $keys['$PAGEEDITDATE'] = $wgLang->timeAndDate($timestamp, true, false,
283 - $u->getOption('timecorrection'));
 285+ $keys['$PAGEEDITDATE'] = $wgLang->timeAndDate( $timestamp, true, false,
 286+ $u->getOption( 'timecorrection' ) );
284287 }
285288
286 - $subject = strtr(wfMsgForContent( $subj_msg_name ), $keys);
 289+ $subject = strtr( wfMsgForContent( $subj_msg_name ), $keys );
287290 $body = wordwrap( strtr( wfMsgForContent( $body_msg_name ), $keys ), 72 );
288291
289 - $to = new MailAddress($u);
 292+ $to = new MailAddress( $u );
290293 $from = $keys['$FROM_HEADER'];
291294 $replyto = $keys['$REPLYTO_HEADER'];
292 - UserMailer::send($to, $from, $subject, $body, $replyto);
 295+ UserMailer::send( $to, $from, $subject, $body, $replyto );
293296 }
294297 }
295298
296299
297 - static function commonMessageKeys($editor) {
 300+ static function commonMessageKeys( $editor ) {
298301 global $wgEnotifUseRealName, $wgEnotifRevealEditorAddress;
299302 global $wgNoReplyAddress, $wgPasswordSender;
300303
@@ -320,11 +323,11 @@
321324 $keys['$REPLYTO_HEADER'] = $replyto;
322325
323326 if( $editor->isAnon() ) {
324 - $keys['$PAGEEDITOR'] = wfMsgForContent('enotif_anon_editor', $name);
 327+ $keys['$PAGEEDITOR'] = wfMsgForContent( 'enotif_anon_editor', $name );
325328 $keys['$PAGEEDITOR_EMAIL'] = wfMsgForContent( 'noemailtitle' );
326329 } else{
327330 $keys['$PAGEEDITOR'] = $name;
328 - $keys['$PAGEEDITOR_EMAIL'] = SpecialPage::getSafeTitleFor('Emailuser', $name)->getFullUrl();
 331+ $keys['$PAGEEDITOR_EMAIL'] = SpecialPage::getSafeTitleFor( 'Emailuser', $name )->getFullUrl();
329332 }
330333 $keys['$PAGEEDITOR_WIKI'] = $editor->getUserPage()->getFullUrl();
331334
@@ -357,13 +360,13 @@
358361 * @param $minorEdit
359362 * @param $oldid (default: false)
360363 */
361 - static function notifyOnPageChange($editor, $title, $timestamp, $summary, $minorEdit, $oldid = false) {
 364+ static function notifyOnPageChange( $editor, $title, $timestamp, $summary, $minorEdit, $oldid = false ) {
362365 global $wgEnotifUseJobQ;
363366
364 - if( $title->getNamespace() < 0 )
 367+ if ( $title->getNamespace() < 0 )
365368 return;
366369
367 - if ($wgEnotifUseJobQ) {
 370+ if ( $wgEnotifUseJobQ ) {
368371 $params = array(
369372 "editor" => $editor->getName(),
370373 "editorID" => $editor->getID(),
@@ -374,7 +377,7 @@
375378 $job = new EnotifNotifyJob( $title, $params );
376379 $job->insert();
377380 } else {
378 - self::actuallyNotifyOnPageChange($editor, $title, $timestamp, $summary, $minorEdit, $oldid);
 381+ self::actuallyNotifyOnPageChange( $editor, $title, $timestamp, $summary, $minorEdit, $oldid );
379382 }
380383
381384 }
@@ -392,15 +395,15 @@
393396 * @param $minorEdit
394397 * @param $oldid (default: false)
395398 */
396 - static function actuallyNotifyOnPageChange($editor, $title, $timestamp,
397 - $summary, $minorEdit, $oldid=false) {
 399+ static function actuallyNotifyOnPageChange( $editor, $title, $timestamp,
 400+ $summary, $minorEdit, $oldid = false ) {
398401 global $wgShowUpdatedMarker, $wgEnotifWatchlist;
399402
400403 wfProfileIn( __METHOD__ );
401404
402 - EmailNotification::notify($editor, $timestamp,
403 - array('PageChangeNotification::usersList', array($editor, $title, $minorEdit)),
404 - array('PageChangeNotification::message', array($oldid, $minorEdit, $summary, $title, $editor) ) );
 405+ EmailNotification::notify( $editor, $timestamp,
 406+ array( 'PageChangeNotification::usersList', array( $editor, $title, $minorEdit ) ),
 407+ array( 'PageChangeNotification::message', array( $oldid, $minorEdit, $summary, $title, $editor ) ) );
405408
406409 $latestTimestamp = Revision::getTimestampFromId( $title, $title->getLatestRevID() );
407410 // Do not update watchlists if something else already did.
@@ -428,7 +431,7 @@
429432 static function message( $stuff ) {
430433 global $wgEnotifImpersonal;
431434
432 - list($oldid, $medit, $summary, $title, $user) = $stuff;
 435+ list( $oldid, $medit, $summary, $title, $user ) = $stuff;
433436 $keys = array();
434437
435438 # regarding the use of oldid as an indicator for the last visited version, see also
@@ -448,28 +451,28 @@
449452
450453 if ($wgEnotifImpersonal && $oldid) {
451454 # For impersonal mail, show a diff link to the last revision.
452 - $keys['$NEWPAGE'] = wfMsgForContent('enotif_lastdiff',
453 - $title->getFullURL("oldid={$oldid}&diff=prev"));
 455+ $keys['$NEWPAGE'] = wfMsgForContent( 'enotif_lastdiff',
 456+ $title->getFullURL( "oldid={$oldid}&diff=prev" ) );
454457 }
455458
456459 $keys['$PAGETITLE'] = $title->getPrefixedText();
457460 $keys['$PAGETITLE_URL'] = $title->getFullUrl();
458461 $keys['$PAGEMINOREDIT'] = $medit ? wfMsg( 'minoredit' ) : '';
459 - $keys['$PAGESUMMARY'] = ($summary == '') ? ' - ' : $summary;
 462+ $keys['$PAGESUMMARY'] = ( $summary == '' ) ? ' - ' : $summary;
460463
461 - return array($keys, 'enotif_body', 'enotif_subject');
 464+ return array( $keys, 'enotif_body', 'enotif_subject' );
462465 }
463466
464 - static function usersList($stuff) {
 467+ static function usersList( $stuff ) {
465468 global $wgEnotifWatchlist, $wgEnotifMinorEdits, $wgUsersNotifiedOnAllChanges;
466469
467 - list($editor, $title, $minorEdit) = $stuff;
 470+ list( $editor, $title, $minorEdit ) = $stuff;
468471 $recipients = array();
469472
470473 # User talk pages:
471474 $userTalkId = false;
472 - if( $title->getNamespace() == NS_USER_TALK && (!$minorEdit || $wgEnotifMinorEdits) ) {
473 - $targetUser = User::newFromName($title->getText());
 475+ if( $title->getNamespace() == NS_USER_TALK && ( !$minorEdit || $wgEnotifMinorEdits ) ) {
 476+ $targetUser = User::newFromName( $title->getText() );
474477
475478 if ( !$targetUser || $targetUser->isAnon() )
476479 $msg = "user talk page edited, but user does not exist";
@@ -477,7 +480,7 @@
478481 else if ( $targetUser->getId() == $editor->getId() )
479482 $msg = "user edited their own talk page, no notification sent";
480483
481 - else if ( !$targetUser->getOption('enotifusertalkpages') )
 484+ else if ( !$targetUser->getOption( 'enotifusertalkpages' ) )
482485 $msg = "talk page owner doesn't want notifications";
483486
484487 else if ( !$targetUser->isEmailConfirmed() )
@@ -488,11 +491,11 @@
489492 $recipients[] = $targetUser;
490493 $userTalkId = $targetUser->getId(); # won't be included in watchlist, below.
491494 }
492 - wfDebug( __METHOD__ .": ". $msg . "\n" );
 495+ wfDebug( __METHOD__ . ': ' . $msg . "\n" );
493496 }
494 - wfDebug("Did not send a user-talk notification.\n");
 497+ wfDebug( "Did not send a user-talk notification.\n" );
495498
496 - if( $wgEnotifWatchlist && (!$minorEdit || $wgEnotifMinorEdits) ) {
 499+ if( $wgEnotifWatchlist && ( !$minorEdit || $wgEnotifMinorEdits ) ) {
497500 // Send updates to watchers other than the current editor
498501 $userCondition = 'wl_user != ' . $editor->getID();
499502
@@ -525,7 +528,7 @@
526529 }
527530
528531 foreach ( $wgUsersNotifiedOnAllChanges as $name ) {
529 - $recipients[] = User::newFromName($name);
 532+ $recipients[] = User::newFromName( $name );
530533 }
531534
532535 return $recipients;
Index: trunk/phase3/includes/Hooks.php
@@ -23,40 +23,39 @@
2424 */
2525
2626 /* Private */
27 -function _wfInvokeInternalGoop($event, $hook) {
 27+function _wfInvokeInternalGoop( $event, $hook ) {
2828 $object = NULL;
2929 $method = NULL;
3030 $func = NULL;
3131 $data = NULL;
3232 $have_data = false;
3333
34 - if (is_array($hook)) {
35 - if (count($hook) < 1) {
36 - throw new MWException("Empty array in hooks for " . $event . "\n");
37 - } else if (is_object($hook[0])) {
 34+ if ( is_array( $hook ) ) {
 35+ if ( count( $hook ) < 1 ) {
 36+ throw new MWException( "Empty array in hooks for " . $event . "\n" );
 37+ } else if ( is_object( $hook[0] ) ) {
3838 $object = $hook[0];
39 - if (count($hook) < 2) {
 39+ if ( count( $hook ) < 2 ) {
4040 $method = "on" . $event;
4141 } else {
4242 $method = $hook[1];
43 - if (count($hook) > 2) {
 43+ if ( count( $hook ) > 2 ) {
4444 $data = $hook[2];
4545 $have_data = true;
4646 }
4747 }
48 - } else if (is_string($hook[0])) {
 48+ } else if ( is_string( $hook[0] ) ) {
4949 $func = $hook[0];
50 - if (count($hook) > 1) {
 50+ if ( count( $hook ) > 1 ) {
5151 $data = $hook[1];
5252 $have_data = true;
5353 }
5454 } else {
55 - var_dump( $wgHooks );
56 - throw new MWException("Unknown datatype in hooks for " . $event . "\n");
 55+ throw new MWException( "Unknown datatype in hooks for " . $event . "\n" );
5756 }
58 - } else if (is_string($hook)) { # functions look like strings, too
 57+ } else if ( is_string( $hook ) ) { # functions look like strings, too
5958 $func = $hook;
60 - } else if (is_object($hook)) {
 59+ } else if ( is_object( $hook ) ) {
6160 $object = $hook;
6261 $method = "on" . $event;
6362 } else {
@@ -76,8 +75,8 @@
7776 }
7877
7978 /* Return a string describing the hook for debugging purposes. */
80 -function wfFormatInvocation($event, $hook, $args = array()) {
81 - list($callback, $func, $data) = _wfInvokeInternalGoop($event, $hook, $args);
 79+function wfFormatInvocation( $event, $hook, $args = array() ) {
 80+ list( $callback, $func, $data ) = _wfInvokeInternalGoop( $event, $hook, $args );
8281
8382 if( is_array( $callback ) ) {
8483 if( is_object( $callback[0] ) ) {
@@ -103,12 +102,12 @@
104103 * If arguments are provided both as part of the hook itself, and when
105104 * calling wfCallFancyCallback, the two arrays are merged.
106105 */
107 -function wfInvoke($event, $hook, $args = array()) {
108 - list($callback, $func, $data) = _wfInvokeInternalGoop($event, $hook, $args);
 106+function wfInvoke( $event, $hook, $args = array() ) {
 107+ list( $callback, $func, $data ) = _wfInvokeInternalGoop( $event, $hook, $args );
109108
110109 /* We put the first data element on, if needed. */
111 - if ($data) {
112 - $hook_args = array_merge(array($data), $args);
 110+ if ( $data ) {
 111+ $hook_args = array_merge( array( $data ), $args );
113112 } else {
114113 $hook_args = $args;
115114 }
@@ -129,40 +128,40 @@
130129 * careful about its contents. So, there's a lot more error-checking
131130 * in here than would normally be necessary.
132131 */
133 -function wfRunHooks($event, $args = array()) {
 132+function wfRunHooks( $event, $args = array() ) {
134133
135134 global $wgHooks;
136135
137 - if (!is_array($wgHooks)) {
138 - throw new MWException("Global hooks array is not an array!\n");
 136+ if ( !is_array( $wgHooks ) ) {
 137+ throw new MWException( "Global hooks array is not an array!\n" );
139138 return false;
140139 }
141140
142 - if (!array_key_exists($event, $wgHooks)) {
 141+ if (!array_key_exists( $event, $wgHooks ) ) {
143142 return true;
144143 }
145144
146 - if (!is_array($wgHooks[$event])) {
147 - throw new MWException("Hooks array for event '$event' is not an array!\n");
 145+ if ( !is_array( $wgHooks[$event] ) ) {
 146+ throw new MWException( "Hooks array for event '$event' is not an array!\n" );
148147 return false;
149148 }
150149
151 - foreach ($wgHooks[$event] as $index => $hook) {
 150+ foreach ( $wgHooks[$event] as $index => $hook ) {
152151
153 - $retval = wfInvoke($event, $hook, $args);
 152+ $retval = wfInvoke( $event, $hook, $args );
154153
155154 /* String return is an error; false return means stop processing. */
156155
157 - if (is_string($retval)) {
 156+ if ( is_string( $retval ) ) {
158157 global $wgOut;
159 - $wgOut->showFatalError($retval);
 158+ $wgOut->showFatalError( $retval );
160159 return false;
161160 } elseif( $retval === null ) {
162 - $prettyFunc = wfFormatInvocation($event, $hook, $args);
 161+ $prettyFunc = wfFormatInvocation( $event, $hook, $args );
163162 throw new MWException( "Detected bug in an extension! " .
164163 "Hook $prettyFunc failed to return a value; " .
165164 "should return true to continue hook processing or false to abort." );
166 - } else if (!$retval) {
 165+ } else if ( !$retval ) {
167166 return false;
168167 }
169168 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r44960Revert r44702, r44703, r44704 (wfInvoke and UserMailer refactor based on it) ...brion18:08, 23 December 2008

Comments

#Comment by Brion VIBBER (talk | contribs)   18:09, 23 December 2008

Reverted in r44960

Status & tagging log