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 LinksUpdate 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 LinksUpdate, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 30 | class LinksUpdate extends SqlDataUpdate implements EnqueueableDataUpdate { |
||
| 31 | // @todo make members protected, but make sure extensions don't break |
||
| 32 | |||
| 33 | /** @var int Page ID of the article linked from */ |
||
| 34 | public $mId; |
||
| 35 | |||
| 36 | /** @var Title Title object of the article linked from */ |
||
| 37 | public $mTitle; |
||
| 38 | |||
| 39 | /** @var ParserOutput */ |
||
| 40 | public $mParserOutput; |
||
| 41 | |||
| 42 | /** @var array Map of title strings to IDs for the links in the document */ |
||
| 43 | public $mLinks; |
||
| 44 | |||
| 45 | /** @var array DB keys of the images used, in the array key only */ |
||
| 46 | public $mImages; |
||
| 47 | |||
| 48 | /** @var array Map of title strings to IDs for the template references, including broken ones */ |
||
| 49 | public $mTemplates; |
||
| 50 | |||
| 51 | /** @var array URLs of external links, array key only */ |
||
| 52 | public $mExternals; |
||
| 53 | |||
| 54 | /** @var array Map of category names to sort keys */ |
||
| 55 | public $mCategories; |
||
| 56 | |||
| 57 | /** @var array Map of language codes to titles */ |
||
| 58 | public $mInterlangs; |
||
| 59 | |||
| 60 | /** @var array 2-D map of (prefix => DBK => 1) */ |
||
| 61 | public $mInterwikis; |
||
| 62 | |||
| 63 | /** @var array Map of arbitrary name to value */ |
||
| 64 | public $mProperties; |
||
| 65 | |||
| 66 | /** @var bool Whether to queue jobs for recursive updates */ |
||
| 67 | public $mRecursive; |
||
| 68 | |||
| 69 | /** @var Revision Revision for which this update has been triggered */ |
||
| 70 | private $mRevision; |
||
| 71 | |||
| 72 | /** |
||
| 73 | * @var null|array Added links if calculated. |
||
| 74 | */ |
||
| 75 | private $linkInsertions = null; |
||
| 76 | |||
| 77 | /** |
||
| 78 | * @var null|array Deleted links if calculated. |
||
| 79 | */ |
||
| 80 | private $linkDeletions = null; |
||
| 81 | |||
| 82 | /** |
||
| 83 | * @var User|null |
||
| 84 | */ |
||
| 85 | private $user; |
||
| 86 | |||
| 87 | /** |
||
| 88 | * Constructor |
||
| 89 | * |
||
| 90 | * @param Title $title Title of the page we're updating |
||
| 91 | * @param ParserOutput $parserOutput Output from a full parse of this page |
||
| 92 | * @param bool $recursive Queue jobs for recursive updates? |
||
| 93 | * @throws MWException |
||
| 94 | */ |
||
| 95 | function __construct( Title $title, ParserOutput $parserOutput, $recursive = true ) { |
||
| 144 | |||
| 145 | /** |
||
| 146 | * Update link tables with outgoing links from an updated article |
||
| 147 | * |
||
| 148 | * @note: this is managed by DeferredUpdates::execute(). Do not run this in a transaction. |
||
| 149 | */ |
||
| 150 | public function doUpdate() { |
||
| 164 | |||
| 165 | /** |
||
| 166 | * Acquire a lock for performing link table updates for a page on a DB |
||
| 167 | * |
||
| 168 | * @param IDatabase $dbw |
||
| 169 | * @param integer $pageId |
||
| 170 | * @return ScopedCallback|null Returns null on failure |
||
| 171 | * @throws RuntimeException |
||
| 172 | * @since 1.27 |
||
| 173 | */ |
||
| 174 | public static function acquirePageLock( IDatabase $dbw, $pageId ) { |
||
| 186 | |||
| 187 | protected function doIncrementalUpdate() { |
||
| 256 | |||
| 257 | /** |
||
| 258 | * Queue recursive jobs for this page |
||
| 259 | * |
||
| 260 | * Which means do LinksUpdate on all pages that include the current page, |
||
| 261 | * using the job queue. |
||
| 262 | */ |
||
| 263 | protected function queueRecursiveJobs() { |
||
| 282 | |||
| 283 | /** |
||
| 284 | * Queue a RefreshLinks job for any table. |
||
| 285 | * |
||
| 286 | * @param Title $title Title to do job for |
||
| 287 | * @param string $table Table to use (e.g. 'templatelinks') |
||
| 288 | */ |
||
| 289 | public static function queueRecursiveJobsForTable( Title $title, $table ) { |
||
| 304 | |||
| 305 | /** |
||
| 306 | * @param array $cats |
||
| 307 | */ |
||
| 308 | function invalidateCategories( $cats ) { |
||
| 311 | |||
| 312 | /** |
||
| 313 | * Update all the appropriate counts in the category table. |
||
| 314 | * @param array $added Associative array of category name => sort key |
||
| 315 | * @param array $deleted Associative array of category name => sort key |
||
| 316 | */ |
||
| 317 | function updateCategoryCounts( $added, $deleted ) { |
||
| 323 | |||
| 324 | /** |
||
| 325 | * @param array $images |
||
| 326 | */ |
||
| 327 | function invalidateImageDescriptions( $images ) { |
||
| 330 | |||
| 331 | /** |
||
| 332 | * Update a table by doing a delete query then an insert query |
||
| 333 | * @param string $table Table name |
||
| 334 | * @param string $prefix Field name prefix |
||
| 335 | * @param array $deletions |
||
| 336 | * @param array $insertions Rows to insert |
||
| 337 | */ |
||
| 338 | private function incrTableUpdate( $table, $prefix, $deletions, $insertions ) { |
||
| 339 | $bSize = RequestContext::getMain()->getConfig()->get( 'UpdateRowsPerQuery' ); |
||
| 340 | |||
| 341 | if ( $table === 'page_props' ) { |
||
| 342 | $fromField = 'pp_page'; |
||
| 343 | } else { |
||
| 344 | $fromField = "{$prefix}_from"; |
||
| 345 | } |
||
| 346 | |||
| 347 | $deleteWheres = []; // list of WHERE clause arrays for each DB delete() call |
||
| 348 | if ( $table === 'pagelinks' || $table === 'templatelinks' || $table === 'iwlinks' ) { |
||
| 349 | $baseKey = ( $table === 'iwlinks' ) ? 'iwl_prefix' : "{$prefix}_namespace"; |
||
| 350 | |||
| 351 | $curBatchSize = 0; |
||
| 352 | $curDeletionBatch = []; |
||
| 353 | $deletionBatches = []; |
||
| 354 | foreach ( $deletions as $ns => $dbKeys ) { |
||
| 355 | foreach ( $dbKeys as $dbKey => $unused ) { |
||
| 356 | $curDeletionBatch[$ns][$dbKey] = 1; |
||
| 357 | if ( ++$curBatchSize >= $bSize ) { |
||
| 358 | $deletionBatches[] = $curDeletionBatch; |
||
| 359 | $curDeletionBatch = []; |
||
| 360 | $curBatchSize = 0; |
||
| 361 | } |
||
| 362 | } |
||
| 363 | } |
||
| 364 | if ( $curDeletionBatch ) { |
||
| 365 | $deletionBatches[] = $curDeletionBatch; |
||
| 366 | } |
||
| 367 | |||
| 368 | foreach ( $deletionBatches as $deletionBatch ) { |
||
| 369 | $deleteWheres[] = [ |
||
| 370 | $fromField => $this->mId, |
||
| 371 | $this->mDb->makeWhereFrom2d( $deletionBatch, $baseKey, "{$prefix}_title" ) |
||
| 372 | ]; |
||
| 373 | } |
||
| 374 | } else { |
||
| 375 | if ( $table === 'langlinks' ) { |
||
| 376 | $toField = 'll_lang'; |
||
| 377 | } elseif ( $table === 'page_props' ) { |
||
| 378 | $toField = 'pp_propname'; |
||
| 379 | } else { |
||
| 380 | $toField = $prefix . '_to'; |
||
| 381 | } |
||
| 382 | |||
| 383 | $deletionBatches = array_chunk( array_keys( $deletions ), $bSize ); |
||
| 384 | foreach ( $deletionBatches as $deletionBatch ) { |
||
| 385 | $deleteWheres[] = [ $fromField => $this->mId, $toField => $deletionBatch ]; |
||
| 386 | } |
||
| 387 | } |
||
| 388 | |||
| 389 | View Code Duplication | foreach ( $deleteWheres as $deleteWhere ) { |
|
| 390 | $this->mDb->delete( $table, $deleteWhere, __METHOD__ ); |
||
| 391 | $this->mDb->commit( __METHOD__, 'flush' ); |
||
| 392 | wfGetLBFactory()->waitForReplication( [ 'wiki' => $this->mDb->getWikiID() ] ); |
||
| 393 | } |
||
| 394 | |||
| 395 | $insertBatches = array_chunk( $insertions, $bSize ); |
||
| 396 | View Code Duplication | foreach ( $insertBatches as $insertBatch ) { |
|
| 397 | $this->mDb->insert( $table, $insertBatch, __METHOD__, 'IGNORE' ); |
||
| 398 | $this->mDb->commit( __METHOD__, 'flush' ); |
||
| 399 | wfGetLBFactory()->waitForReplication( [ 'wiki' => $this->mDb->getWikiID() ] ); |
||
| 400 | } |
||
| 401 | |||
| 402 | if ( count( $insertions ) ) { |
||
| 403 | Hooks::run( 'LinksUpdateAfterInsert', [ $this, $table, $insertions ] ); |
||
| 404 | } |
||
| 405 | } |
||
| 406 | |||
| 407 | /** |
||
| 408 | * Get an array of pagelinks insertions for passing to the DB |
||
| 409 | * Skips the titles specified by the 2-D array $existing |
||
| 410 | * @param array $existing |
||
| 411 | * @return array |
||
| 412 | */ |
||
| 413 | View Code Duplication | private function getLinkInsertions( $existing = [] ) { |
|
| 431 | |||
| 432 | /** |
||
| 433 | * Get an array of template insertions. Like getLinkInsertions() |
||
| 434 | * @param array $existing |
||
| 435 | * @return array |
||
| 436 | */ |
||
| 437 | View Code Duplication | private function getTemplateInsertions( $existing = [] ) { |
|
| 453 | |||
| 454 | /** |
||
| 455 | * Get an array of image insertions |
||
| 456 | * Skips the names specified in $existing |
||
| 457 | * @param array $existing |
||
| 458 | * @return array |
||
| 459 | */ |
||
| 460 | private function getImageInsertions( $existing = [] ) { |
||
| 473 | |||
| 474 | /** |
||
| 475 | * Get an array of externallinks insertions. Skips the names specified in $existing |
||
| 476 | * @param array $existing |
||
| 477 | * @return array |
||
| 478 | */ |
||
| 479 | private function getExternalInsertions( $existing = [] ) { |
||
| 495 | |||
| 496 | /** |
||
| 497 | * Get an array of category insertions |
||
| 498 | * |
||
| 499 | * @param array $existing Mapping existing category names to sort keys. If both |
||
| 500 | * match a link in $this, the link will be omitted from the output |
||
| 501 | * |
||
| 502 | * @return array |
||
| 503 | */ |
||
| 504 | private function getCategoryInsertions( $existing = [] ) { |
||
| 540 | |||
| 541 | /** |
||
| 542 | * Get an array of interlanguage link insertions |
||
| 543 | * |
||
| 544 | * @param array $existing Mapping existing language codes to titles |
||
| 545 | * |
||
| 546 | * @return array |
||
| 547 | */ |
||
| 548 | private function getInterlangInsertions( $existing = [] ) { |
||
| 561 | |||
| 562 | /** |
||
| 563 | * Get an array of page property insertions |
||
| 564 | * @param array $existing |
||
| 565 | * @return array |
||
| 566 | */ |
||
| 567 | function getPropertyInsertions( $existing = [] ) { |
||
| 577 | |||
| 578 | /** |
||
| 579 | * Returns an associative array to be used for inserting a row into |
||
| 580 | * the page_props table. Besides the given property name, this will |
||
| 581 | * include the page id from $this->mId and any property value from |
||
| 582 | * $this->mProperties. |
||
| 583 | * |
||
| 584 | * The array returned will include the pp_sortkey field if this |
||
| 585 | * is present in the database (as indicated by $wgPagePropsHaveSortkey). |
||
| 586 | * The sortkey value is currently determined by getPropertySortKeyValue(). |
||
| 587 | * |
||
| 588 | * @note this assumes that $this->mProperties[$prop] is defined. |
||
| 589 | * |
||
| 590 | * @param string $prop The name of the property. |
||
| 591 | * |
||
| 592 | * @return array |
||
| 593 | */ |
||
| 594 | private function getPagePropRowData( $prop ) { |
||
| 611 | |||
| 612 | /** |
||
| 613 | * Determines the sort key for the given property value. |
||
| 614 | * This will return $value if it is a float or int, |
||
| 615 | * 1 or resp. 0 if it is a bool, and null otherwise. |
||
| 616 | * |
||
| 617 | * @note In the future, we may allow the sortkey to be specified explicitly |
||
| 618 | * in ParserOutput::setProperty. |
||
| 619 | * |
||
| 620 | * @param mixed $value |
||
| 621 | * |
||
| 622 | * @return float|null |
||
| 623 | */ |
||
| 624 | private function getPropertySortKeyValue( $value ) { |
||
| 631 | |||
| 632 | /** |
||
| 633 | * Get an array of interwiki insertions for passing to the DB |
||
| 634 | * Skips the titles specified by the 2-D array $existing |
||
| 635 | * @param array $existing |
||
| 636 | * @return array |
||
| 637 | */ |
||
| 638 | private function getInterwikiInsertions( $existing = [] ) { |
||
| 656 | |||
| 657 | /** |
||
| 658 | * Given an array of existing links, returns those links which are not in $this |
||
| 659 | * and thus should be deleted. |
||
| 660 | * @param array $existing |
||
| 661 | * @return array |
||
| 662 | */ |
||
| 663 | View Code Duplication | private function getLinkDeletions( $existing ) { |
|
| 675 | |||
| 676 | /** |
||
| 677 | * Given an array of existing templates, returns those templates which are not in $this |
||
| 678 | * and thus should be deleted. |
||
| 679 | * @param array $existing |
||
| 680 | * @return array |
||
| 681 | */ |
||
| 682 | View Code Duplication | private function getTemplateDeletions( $existing ) { |
|
| 694 | |||
| 695 | /** |
||
| 696 | * Given an array of existing images, returns those images which are not in $this |
||
| 697 | * and thus should be deleted. |
||
| 698 | * @param array $existing |
||
| 699 | * @return array |
||
| 700 | */ |
||
| 701 | private function getImageDeletions( $existing ) { |
||
| 704 | |||
| 705 | /** |
||
| 706 | * Given an array of existing external links, returns those links which are not |
||
| 707 | * in $this and thus should be deleted. |
||
| 708 | * @param array $existing |
||
| 709 | * @return array |
||
| 710 | */ |
||
| 711 | private function getExternalDeletions( $existing ) { |
||
| 714 | |||
| 715 | /** |
||
| 716 | * Given an array of existing categories, returns those categories which are not in $this |
||
| 717 | * and thus should be deleted. |
||
| 718 | * @param array $existing |
||
| 719 | * @return array |
||
| 720 | */ |
||
| 721 | private function getCategoryDeletions( $existing ) { |
||
| 724 | |||
| 725 | /** |
||
| 726 | * Given an array of existing interlanguage links, returns those links which are not |
||
| 727 | * in $this and thus should be deleted. |
||
| 728 | * @param array $existing |
||
| 729 | * @return array |
||
| 730 | */ |
||
| 731 | private function getInterlangDeletions( $existing ) { |
||
| 734 | |||
| 735 | /** |
||
| 736 | * Get array of properties which should be deleted. |
||
| 737 | * @param array $existing |
||
| 738 | * @return array |
||
| 739 | */ |
||
| 740 | function getPropertyDeletions( $existing ) { |
||
| 743 | |||
| 744 | /** |
||
| 745 | * Given an array of existing interwiki links, returns those links which are not in $this |
||
| 746 | * and thus should be deleted. |
||
| 747 | * @param array $existing |
||
| 748 | * @return array |
||
| 749 | */ |
||
| 750 | View Code Duplication | private function getInterwikiDeletions( $existing ) { |
|
| 762 | |||
| 763 | /** |
||
| 764 | * Get an array of existing links, as a 2-D array |
||
| 765 | * |
||
| 766 | * @return array |
||
| 767 | */ |
||
| 768 | private function getExistingLinks() { |
||
| 781 | |||
| 782 | /** |
||
| 783 | * Get an array of existing templates, as a 2-D array |
||
| 784 | * |
||
| 785 | * @return array |
||
| 786 | */ |
||
| 787 | private function getExistingTemplates() { |
||
| 800 | |||
| 801 | /** |
||
| 802 | * Get an array of existing images, image names in the keys |
||
| 803 | * |
||
| 804 | * @return array |
||
| 805 | */ |
||
| 806 | private function getExistingImages() { |
||
| 816 | |||
| 817 | /** |
||
| 818 | * Get an array of existing external links, URLs in the keys |
||
| 819 | * |
||
| 820 | * @return array |
||
| 821 | */ |
||
| 822 | private function getExistingExternals() { |
||
| 832 | |||
| 833 | /** |
||
| 834 | * Get an array of existing categories, with the name in the key and sort key in the value. |
||
| 835 | * |
||
| 836 | * @return array |
||
| 837 | */ |
||
| 838 | private function getExistingCategories() { |
||
| 848 | |||
| 849 | /** |
||
| 850 | * Get an array of existing interlanguage links, with the language code in the key and the |
||
| 851 | * title in the value. |
||
| 852 | * |
||
| 853 | * @return array |
||
| 854 | */ |
||
| 855 | private function getExistingInterlangs() { |
||
| 865 | |||
| 866 | /** |
||
| 867 | * Get an array of existing inline interwiki links, as a 2-D array |
||
| 868 | * @return array (prefix => array(dbkey => 1)) |
||
| 869 | */ |
||
| 870 | protected function getExistingInterwikis() { |
||
| 883 | |||
| 884 | /** |
||
| 885 | * Get an array of existing categories, with the name in the key and sort key in the value. |
||
| 886 | * |
||
| 887 | * @return array Array of property names and values |
||
| 888 | */ |
||
| 889 | private function getExistingProperties() { |
||
| 899 | |||
| 900 | /** |
||
| 901 | * Return the title object of the page being updated |
||
| 902 | * @return Title |
||
| 903 | */ |
||
| 904 | public function getTitle() { |
||
| 907 | |||
| 908 | /** |
||
| 909 | * Returns parser output |
||
| 910 | * @since 1.19 |
||
| 911 | * @return ParserOutput |
||
| 912 | */ |
||
| 913 | public function getParserOutput() { |
||
| 916 | |||
| 917 | /** |
||
| 918 | * Return the list of images used as generated by the parser |
||
| 919 | * @return array |
||
| 920 | */ |
||
| 921 | public function getImages() { |
||
| 924 | |||
| 925 | /** |
||
| 926 | * Set the revision corresponding to this LinksUpdate |
||
| 927 | * |
||
| 928 | * @since 1.27 |
||
| 929 | * |
||
| 930 | * @param Revision $revision |
||
| 931 | */ |
||
| 932 | public function setRevision( Revision $revision ) { |
||
| 935 | |||
| 936 | /** |
||
| 937 | * @since 1.28 |
||
| 938 | * @return null|Revision |
||
| 939 | */ |
||
| 940 | public function getRevision() { |
||
| 943 | |||
| 944 | /** |
||
| 945 | * Set the User who triggered this LinksUpdate |
||
| 946 | * |
||
| 947 | * @since 1.27 |
||
| 948 | * @param User $user |
||
| 949 | */ |
||
| 950 | public function setTriggeringUser( User $user ) { |
||
| 953 | |||
| 954 | /** |
||
| 955 | * @since 1.27 |
||
| 956 | * @return null|User |
||
| 957 | */ |
||
| 958 | public function getTriggeringUser() { |
||
| 961 | |||
| 962 | /** |
||
| 963 | * Invalidate any necessary link lists related to page property changes |
||
| 964 | * @param array $changed |
||
| 965 | */ |
||
| 966 | private function invalidateProperties( $changed ) { |
||
| 981 | |||
| 982 | /** |
||
| 983 | * Fetch page links added by this LinksUpdate. Only available after the update is complete. |
||
| 984 | * @since 1.22 |
||
| 985 | * @return null|array Array of Titles |
||
| 986 | */ |
||
| 987 | public function getAddedLinks() { |
||
| 998 | |||
| 999 | /** |
||
| 1000 | * Fetch page links removed by this LinksUpdate. Only available after the update is complete. |
||
| 1001 | * @since 1.22 |
||
| 1002 | * @return null|array Array of Titles |
||
| 1003 | */ |
||
| 1004 | public function getRemovedLinks() { |
||
| 1017 | |||
| 1018 | /** |
||
| 1019 | * Update links table freshness |
||
| 1020 | */ |
||
| 1021 | protected function updateLinksTimestamp() { |
||
| 1032 | |||
| 1033 | public function getAsJobSpecification() { |
||
| 1065 | } |
||
| 1066 |
Loading history...
There are different options of fixing this problem.
If you want to be on the safe side, you can add an additional type-check:
If you are sure that the expression is traversable, you might want to add a doc comment cast to improve IDE auto-completion and static analysis:
Mark the issue as a false-positive: Just hover the remove button, in the top-right corner of this issue for more options.