wikimedia /
mediawiki
Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like ContribsPager often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use ContribsPager, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 26 | class ContribsPager extends ReverseChronologicalPager { |
||
| 27 | |||
| 28 | public $mDefaultDirection = IndexPager::DIR_DESCENDING; |
||
| 29 | public $messages; |
||
| 30 | public $target; |
||
| 31 | public $namespace = ''; |
||
| 32 | public $mDb; |
||
| 33 | public $preventClickjacking = false; |
||
| 34 | |||
| 35 | /** @var IDatabase */ |
||
| 36 | public $mDbSecondary; |
||
| 37 | |||
| 38 | /** |
||
| 39 | * @var array |
||
| 40 | */ |
||
| 41 | protected $mParentLens; |
||
| 42 | |||
| 43 | function __construct( IContextSource $context, array $options ) { |
||
| 44 | parent::__construct( $context ); |
||
| 45 | |||
| 46 | $msgs = [ |
||
| 47 | 'diff', |
||
| 48 | 'hist', |
||
| 49 | 'pipe-separator', |
||
| 50 | 'uctop' |
||
| 51 | ]; |
||
| 52 | |||
| 53 | foreach ( $msgs as $msg ) { |
||
| 54 | $this->messages[$msg] = $this->msg( $msg )->escaped(); |
||
| 55 | } |
||
| 56 | |||
| 57 | $this->target = isset( $options['target'] ) ? $options['target'] : ''; |
||
| 58 | $this->contribs = isset( $options['contribs'] ) ? $options['contribs'] : 'users'; |
||
|
|
|||
| 59 | $this->namespace = isset( $options['namespace'] ) ? $options['namespace'] : ''; |
||
| 60 | $this->tagFilter = isset( $options['tagfilter'] ) ? $options['tagfilter'] : false; |
||
| 61 | $this->nsInvert = isset( $options['nsInvert'] ) ? $options['nsInvert'] : false; |
||
| 62 | $this->associated = isset( $options['associated'] ) ? $options['associated'] : false; |
||
| 63 | |||
| 64 | $this->deletedOnly = !empty( $options['deletedOnly'] ); |
||
| 65 | $this->topOnly = !empty( $options['topOnly'] ); |
||
| 66 | $this->newOnly = !empty( $options['newOnly'] ); |
||
| 67 | $this->hideMinor = !empty( $options['hideMinor'] ); |
||
| 68 | |||
| 69 | $year = isset( $options['year'] ) ? $options['year'] : false; |
||
| 70 | $month = isset( $options['month'] ) ? $options['month'] : false; |
||
| 71 | $this->getDateCond( $year, $month ); |
||
| 72 | |||
| 73 | // Most of this code will use the 'contributions' group DB, which can map to replica DBs |
||
| 74 | // with extra user based indexes or partioning by user. The additional metadata |
||
| 75 | // queries should use a regular replica DB since the lookup pattern is not all by user. |
||
| 76 | $this->mDbSecondary = wfGetDB( DB_REPLICA ); // any random replica DB |
||
| 77 | $this->mDb = wfGetDB( DB_REPLICA, 'contributions' ); |
||
| 78 | } |
||
| 79 | |||
| 80 | function getDefaultQuery() { |
||
| 86 | |||
| 87 | /** |
||
| 88 | * This method basically executes the exact same code as the parent class, though with |
||
| 89 | * a hook added, to allow extensions to add additional queries. |
||
| 90 | * |
||
| 91 | * @param string $offset Index offset, inclusive |
||
| 92 | * @param int $limit Exact query limit |
||
| 93 | * @param bool $descending Query direction, false for ascending, true for descending |
||
| 94 | * @return ResultWrapper |
||
| 95 | */ |
||
| 96 | function reallyDoQuery( $offset, $limit, $descending ) { |
||
| 154 | |||
| 155 | function getQueryInfo() { |
||
| 205 | |||
| 206 | function getUserCond() { |
||
| 207 | $condition = []; |
||
| 208 | $join_conds = []; |
||
| 209 | $tables = [ 'revision', 'page', 'user' ]; |
||
| 210 | $index = false; |
||
| 211 | if ( $this->contribs == 'newbie' ) { |
||
| 212 | $max = $this->mDb->selectField( 'user', 'max(user_id)', false, __METHOD__ ); |
||
| 213 | $condition[] = 'rev_user >' . (int)( $max - $max / 100 ); |
||
| 214 | # ignore local groups with the bot right |
||
| 215 | # @todo FIXME: Global groups may have 'bot' rights |
||
| 216 | $groupsWithBotPermission = User::getGroupsWithPermission( 'bot' ); |
||
| 217 | if ( count( $groupsWithBotPermission ) ) { |
||
| 218 | $tables[] = 'user_groups'; |
||
| 219 | $condition[] = 'ug_group IS NULL'; |
||
| 220 | $join_conds['user_groups'] = [ |
||
| 221 | 'LEFT JOIN', [ |
||
| 222 | 'ug_user = rev_user', |
||
| 223 | 'ug_group' => $groupsWithBotPermission |
||
| 224 | ] |
||
| 225 | ]; |
||
| 226 | } |
||
| 227 | // (T140537) Disallow looking too far in the past for 'newbies' queries. If the user requested |
||
| 228 | // a timestamp offset far in the past such that there are no edits by users with user_ids in |
||
| 229 | // the range, we would end up scanning all revisions from that offset until start of time. |
||
| 230 | $condition[] = 'rev_timestamp > ' . |
||
| 231 | $this->mDb->addQuotes( $this->mDb->timestamp( wfTimestamp() - 30 * 24 * 60 * 60 ) ); |
||
| 232 | } else { |
||
| 233 | $uid = User::idFromName( $this->target ); |
||
| 234 | if ( $uid ) { |
||
| 235 | $condition['rev_user'] = $uid; |
||
| 236 | $index = 'user_timestamp'; |
||
| 237 | } else { |
||
| 238 | $condition['rev_user_text'] = $this->target; |
||
| 239 | $index = 'usertext_timestamp'; |
||
| 240 | } |
||
| 241 | } |
||
| 242 | |||
| 243 | if ( $this->deletedOnly ) { |
||
| 244 | $condition[] = 'rev_deleted != 0'; |
||
| 245 | } |
||
| 246 | |||
| 247 | if ( $this->topOnly ) { |
||
| 248 | $condition[] = 'rev_id = page_latest'; |
||
| 249 | } |
||
| 250 | |||
| 251 | if ( $this->newOnly ) { |
||
| 252 | $condition[] = 'rev_parent_id = 0'; |
||
| 253 | } |
||
| 254 | |||
| 255 | if ( $this->hideMinor ) { |
||
| 256 | $condition[] = 'rev_minor_edit = 0'; |
||
| 257 | } |
||
| 258 | |||
| 259 | return [ $tables, $index, $condition, $join_conds ]; |
||
| 260 | } |
||
| 261 | |||
| 262 | function getNamespaceCond() { |
||
| 285 | |||
| 286 | function getIndexField() { |
||
| 289 | |||
| 290 | function doBatchLookups() { |
||
| 318 | |||
| 319 | /** |
||
| 320 | * @return string |
||
| 321 | */ |
||
| 322 | function getStartBody() { |
||
| 325 | |||
| 326 | /** |
||
| 327 | * @return string |
||
| 328 | */ |
||
| 329 | function getEndBody() { |
||
| 332 | |||
| 333 | /** |
||
| 334 | * Generates each row in the contributions list. |
||
| 335 | * |
||
| 336 | * Contributions which are marked "top" are currently on top of the history. |
||
| 337 | * For these contributions, a [rollback] link is shown for users with roll- |
||
| 338 | * back privileges. The rollback link restores the most recent version that |
||
| 339 | * was not written by the target user. |
||
| 340 | * |
||
| 341 | * @todo This would probably look a lot nicer in a table. |
||
| 342 | * @param object $row |
||
| 343 | * @return string |
||
| 344 | */ |
||
| 345 | function formatRow( $row ) { |
||
| 346 | |||
| 347 | $ret = ''; |
||
| 348 | $classes = []; |
||
| 349 | |||
| 350 | /* |
||
| 351 | * There may be more than just revision rows. To make sure that we'll only be processing |
||
| 352 | * revisions here, let's _try_ to build a revision out of our row (without displaying |
||
| 353 | * notices though) and then trying to grab data from the built object. If we succeed, |
||
| 354 | * we're definitely dealing with revision data and we may proceed, if not, we'll leave it |
||
| 355 | * to extensions to subscribe to the hook to parse the row. |
||
| 356 | */ |
||
| 357 | MediaWiki\suppressWarnings(); |
||
| 358 | try { |
||
| 359 | $rev = new Revision( $row ); |
||
| 360 | $validRevision = (bool)$rev->getId(); |
||
| 361 | } catch ( Exception $e ) { |
||
| 362 | $validRevision = false; |
||
| 363 | } |
||
| 364 | MediaWiki\restoreWarnings(); |
||
| 365 | |||
| 366 | if ( $validRevision ) { |
||
| 367 | $classes = []; |
||
| 368 | |||
| 369 | $page = Title::newFromRow( $row ); |
||
| 370 | $link = Linker::link( |
||
| 371 | $page, |
||
| 372 | htmlspecialchars( $page->getPrefixedText() ), |
||
| 373 | [ 'class' => 'mw-contributions-title' ], |
||
| 374 | $page->isRedirect() ? [ 'redirect' => 'no' ] : [] |
||
| 375 | ); |
||
| 376 | # Mark current revisions |
||
| 377 | $topmarktext = ''; |
||
| 378 | $user = $this->getUser(); |
||
| 379 | if ( $row->rev_id === $row->page_latest ) { |
||
| 380 | $topmarktext .= '<span class="mw-uctop">' . $this->messages['uctop'] . '</span>'; |
||
| 381 | $classes[] = 'mw-contributions-current'; |
||
| 382 | # Add rollback link |
||
| 383 | if ( !$row->page_is_new && $page->quickUserCan( 'rollback', $user ) |
||
| 384 | && $page->quickUserCan( 'edit', $user ) |
||
| 385 | ) { |
||
| 386 | $this->preventClickjacking(); |
||
| 387 | $topmarktext .= ' ' . Linker::generateRollback( $rev, $this->getContext() ); |
||
| 388 | } |
||
| 389 | } |
||
| 390 | # Is there a visible previous revision? |
||
| 391 | View Code Duplication | if ( $rev->userCan( Revision::DELETED_TEXT, $user ) && $rev->getParentId() !== 0 ) { |
|
| 392 | $difftext = Linker::linkKnown( |
||
| 393 | $page, |
||
| 394 | $this->messages['diff'], |
||
| 395 | [], |
||
| 396 | [ |
||
| 397 | 'diff' => 'prev', |
||
| 398 | 'oldid' => $row->rev_id |
||
| 399 | ] |
||
| 400 | ); |
||
| 401 | } else { |
||
| 402 | $difftext = $this->messages['diff']; |
||
| 403 | } |
||
| 404 | $histlink = Linker::linkKnown( |
||
| 405 | $page, |
||
| 406 | $this->messages['hist'], |
||
| 407 | [], |
||
| 408 | [ 'action' => 'history' ] |
||
| 409 | ); |
||
| 410 | |||
| 411 | if ( $row->rev_parent_id === null ) { |
||
| 412 | // For some reason rev_parent_id isn't populated for this row. |
||
| 413 | // Its rumoured this is true on wikipedia for some revisions (bug 34922). |
||
| 414 | // Next best thing is to have the total number of bytes. |
||
| 415 | $chardiff = ' <span class="mw-changeslist-separator">. .</span> '; |
||
| 416 | $chardiff .= Linker::formatRevisionSize( $row->rev_len ); |
||
| 417 | $chardiff .= ' <span class="mw-changeslist-separator">. .</span> '; |
||
| 418 | } else { |
||
| 419 | $parentLen = 0; |
||
| 420 | if ( isset( $this->mParentLens[$row->rev_parent_id] ) ) { |
||
| 421 | $parentLen = $this->mParentLens[$row->rev_parent_id]; |
||
| 422 | } |
||
| 423 | |||
| 424 | $chardiff = ' <span class="mw-changeslist-separator">. .</span> '; |
||
| 425 | $chardiff .= ChangesList::showCharacterDifference( |
||
| 426 | $parentLen, |
||
| 427 | $row->rev_len, |
||
| 428 | $this->getContext() |
||
| 429 | ); |
||
| 430 | $chardiff .= ' <span class="mw-changeslist-separator">. .</span> '; |
||
| 431 | } |
||
| 432 | |||
| 433 | $lang = $this->getLanguage(); |
||
| 434 | $comment = $lang->getDirMark() . Linker::revComment( $rev, false, true ); |
||
| 435 | $date = $lang->userTimeAndDate( $row->rev_timestamp, $user ); |
||
| 436 | if ( $rev->userCan( Revision::DELETED_TEXT, $user ) ) { |
||
| 437 | $d = Linker::linkKnown( |
||
| 438 | $page, |
||
| 439 | htmlspecialchars( $date ), |
||
| 440 | [ 'class' => 'mw-changeslist-date' ], |
||
| 441 | [ 'oldid' => intval( $row->rev_id ) ] |
||
| 442 | ); |
||
| 443 | } else { |
||
| 444 | $d = htmlspecialchars( $date ); |
||
| 445 | } |
||
| 446 | if ( $rev->isDeleted( Revision::DELETED_TEXT ) ) { |
||
| 447 | $d = '<span class="history-deleted">' . $d . '</span>'; |
||
| 448 | } |
||
| 449 | |||
| 450 | # Show user names for /newbies as there may be different users. |
||
| 451 | # Note that we already excluded rows with hidden user names. |
||
| 452 | if ( $this->contribs == 'newbie' ) { |
||
| 453 | $userlink = ' . . ' . $lang->getDirMark() |
||
| 454 | . Linker::userLink( $rev->getUser(), $rev->getUserText() ); |
||
| 455 | $userlink .= ' ' . $this->msg( 'parentheses' )->rawParams( |
||
| 456 | Linker::userTalkLink( $rev->getUser(), $rev->getUserText() ) )->escaped() . ' '; |
||
| 457 | } else { |
||
| 458 | $userlink = ''; |
||
| 459 | } |
||
| 460 | |||
| 461 | $flags = []; |
||
| 462 | if ( $rev->getParentId() === 0 ) { |
||
| 463 | $flags[] = ChangesList::flag( 'newpage' ); |
||
| 464 | } |
||
| 465 | |||
| 466 | if ( $rev->isMinor() ) { |
||
| 467 | $flags[] = ChangesList::flag( 'minor' ); |
||
| 468 | } |
||
| 469 | |||
| 470 | $del = Linker::getRevDeleteLink( $user, $rev, $page ); |
||
| 471 | if ( $del !== '' ) { |
||
| 472 | $del .= ' '; |
||
| 473 | } |
||
| 474 | |||
| 475 | $diffHistLinks = $this->msg( 'parentheses' ) |
||
| 476 | ->rawParams( $difftext . $this->messages['pipe-separator'] . $histlink ) |
||
| 477 | ->escaped(); |
||
| 478 | |||
| 479 | # Tags, if any. |
||
| 480 | list( $tagSummary, $newClasses ) = ChangeTags::formatSummaryRow( |
||
| 481 | $row->ts_tags, |
||
| 482 | 'contributions', |
||
| 483 | $this->getContext() |
||
| 484 | ); |
||
| 485 | $classes = array_merge( $classes, $newClasses ); |
||
| 486 | |||
| 487 | Hooks::run( 'SpecialContributions::formatRow::flags', [ $this->getContext(), $row, &$flags ] ); |
||
| 488 | |||
| 489 | $templateParams = [ |
||
| 490 | 'del' => $del, |
||
| 491 | 'timestamp' => $d, |
||
| 492 | 'diffHistLinks' => $diffHistLinks, |
||
| 493 | 'charDifference' => $chardiff, |
||
| 494 | 'flags' => $flags, |
||
| 495 | 'articleLink' => $link, |
||
| 496 | 'userlink' => $userlink, |
||
| 497 | 'logText' => $comment, |
||
| 498 | 'topmarktext' => $topmarktext, |
||
| 499 | 'tagSummary' => $tagSummary, |
||
| 500 | ]; |
||
| 501 | |||
| 502 | # Denote if username is redacted for this edit |
||
| 503 | if ( $rev->isDeleted( Revision::DELETED_USER ) ) { |
||
| 504 | $templateParams['rev-deleted-user-contribs'] = |
||
| 505 | $this->msg( 'rev-deleted-user-contribs' )->escaped(); |
||
| 506 | } |
||
| 507 | |||
| 508 | $templateParser = new TemplateParser(); |
||
| 509 | $ret = $templateParser->processTemplate( |
||
| 510 | 'SpecialContributionsLine', |
||
| 511 | $templateParams |
||
| 512 | ); |
||
| 513 | } |
||
| 514 | |||
| 515 | // Let extensions add data |
||
| 516 | Hooks::run( 'ContributionsLineEnding', [ $this, &$ret, $row, &$classes ] ); |
||
| 517 | |||
| 518 | // TODO: Handle exceptions in the catch block above. Do any extensions rely on |
||
| 519 | // receiving empty rows? |
||
| 520 | |||
| 521 | if ( $classes === [] && $ret === '' ) { |
||
| 522 | wfDebug( "Dropping Special:Contribution row that could not be formatted\n" ); |
||
| 523 | return "<!-- Could not format Special:Contribution row. -->\n"; |
||
| 524 | } |
||
| 525 | |||
| 526 | // FIXME: The signature of the ContributionsLineEnding hook makes it |
||
| 527 | // very awkward to move this LI wrapper into the template. |
||
| 528 | return Html::rawElement( 'li', [ 'class' => $classes ], $ret ) . "\n"; |
||
| 529 | } |
||
| 530 | |||
| 531 | /** |
||
| 532 | * Overwrite Pager function and return a helpful comment |
||
| 533 | * @return string |
||
| 534 | */ |
||
| 535 | function getSqlComment() { |
||
| 543 | |||
| 544 | protected function preventClickjacking() { |
||
| 547 | |||
| 548 | /** |
||
| 549 | * @return bool |
||
| 550 | */ |
||
| 551 | public function getPreventClickjacking() { |
||
| 554 | } |
||
| 555 |
Loading history...
In PHP it is possible to write to properties without declaring them. For example, the following is perfectly valid PHP code:
Generally, it is a good practice to explictly declare properties to avoid accidental typos and provide IDE auto-completion: