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 WatchedItemStore 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 WatchedItemStore, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 13 | class WatchedItemStore { |
||
| 14 | |||
| 15 | /** |
||
| 16 | * @var LoadBalancer |
||
| 17 | */ |
||
| 18 | private $loadBalancer; |
||
| 19 | |||
| 20 | /** |
||
| 21 | * @var HashBagOStuff |
||
| 22 | */ |
||
| 23 | private $cache; |
||
| 24 | |||
| 25 | /** |
||
| 26 | * @var array[] Looks like $cacheIndex[Namespace ID][Target DB Key][User Id] => 'key' |
||
| 27 | * The index is needed so that on mass changes all relevant items can be un-cached. |
||
| 28 | * For example: Clearing a users watchlist of all items or updating notification timestamps |
||
| 29 | * for all users watching a single target. |
||
| 30 | */ |
||
| 31 | private $cacheIndex = []; |
||
| 32 | |||
| 33 | /** |
||
| 34 | * @var callable|null |
||
| 35 | */ |
||
| 36 | private $deferredUpdatesAddCallableUpdateCallback; |
||
| 37 | |||
| 38 | /** |
||
| 39 | * @var callable|null |
||
| 40 | */ |
||
| 41 | private $revisionGetTimestampFromIdCallback; |
||
| 42 | |||
| 43 | /** |
||
| 44 | * @var self|null |
||
| 45 | */ |
||
| 46 | private static $instance; |
||
| 47 | |||
| 48 | /** |
||
| 49 | * @param LoadBalancer $loadBalancer |
||
| 50 | * @param HashBagOStuff $cache |
||
| 51 | */ |
||
| 52 | public function __construct( |
||
| 53 | LoadBalancer $loadBalancer, |
||
| 54 | HashBagOStuff $cache |
||
| 55 | ) { |
||
| 56 | $this->loadBalancer = $loadBalancer; |
||
| 57 | $this->cache = $cache; |
||
| 58 | $this->deferredUpdatesAddCallableUpdateCallback = [ 'DeferredUpdates', 'addCallableUpdate' ]; |
||
| 59 | $this->revisionGetTimestampFromIdCallback = [ 'Revision', 'getTimestampFromId' ]; |
||
| 60 | } |
||
| 61 | |||
| 62 | /** |
||
| 63 | * Overrides the DeferredUpdates::addCallableUpdate callback |
||
| 64 | * This is intended for use while testing and will fail if MW_PHPUNIT_TEST is not defined. |
||
| 65 | * |
||
| 66 | * @param callable $callback |
||
| 67 | * |
||
| 68 | * @see DeferredUpdates::addCallableUpdate for callback signiture |
||
| 69 | * |
||
| 70 | * @return ScopedCallback to reset the overridden value |
||
| 71 | * @throws MWException |
||
| 72 | */ |
||
| 73 | View Code Duplication | public function overrideDeferredUpdatesAddCallableUpdateCallback( $callback ) { |
|
| 74 | if ( !defined( 'MW_PHPUNIT_TEST' ) ) { |
||
| 75 | throw new MWException( |
||
| 76 | 'Cannot override DeferredUpdates::addCallableUpdate callback in operation.' |
||
| 77 | ); |
||
| 78 | } |
||
| 79 | Assert::parameterType( 'callable', $callback, '$callback' ); |
||
| 80 | |||
| 81 | $previousValue = $this->deferredUpdatesAddCallableUpdateCallback; |
||
| 82 | $this->deferredUpdatesAddCallableUpdateCallback = $callback; |
||
| 83 | return new ScopedCallback( function() use ( $previousValue ) { |
||
| 84 | $this->deferredUpdatesAddCallableUpdateCallback = $previousValue; |
||
| 85 | } ); |
||
| 86 | } |
||
| 87 | |||
| 88 | /** |
||
| 89 | * Overrides the Revision::getTimestampFromId callback |
||
| 90 | * This is intended for use while testing and will fail if MW_PHPUNIT_TEST is not defined. |
||
| 91 | * |
||
| 92 | * @param callable $callback |
||
| 93 | * @see Revision::getTimestampFromId for callback signiture |
||
| 94 | * |
||
| 95 | * @return ScopedCallback to reset the overridden value |
||
| 96 | * @throws MWException |
||
| 97 | */ |
||
| 98 | View Code Duplication | public function overrideRevisionGetTimestampFromIdCallback( $callback ) { |
|
| 99 | if ( !defined( 'MW_PHPUNIT_TEST' ) ) { |
||
| 100 | throw new MWException( |
||
| 101 | 'Cannot override Revision::getTimestampFromId callback in operation.' |
||
| 102 | ); |
||
| 103 | } |
||
| 104 | Assert::parameterType( 'callable', $callback, '$callback' ); |
||
| 105 | |||
| 106 | $previousValue = $this->revisionGetTimestampFromIdCallback; |
||
| 107 | $this->revisionGetTimestampFromIdCallback = $callback; |
||
| 108 | return new ScopedCallback( function() use ( $previousValue ) { |
||
| 109 | $this->revisionGetTimestampFromIdCallback = $previousValue; |
||
| 110 | } ); |
||
| 111 | } |
||
| 112 | |||
| 113 | /** |
||
| 114 | * Overrides the default instance of this class |
||
| 115 | * This is intended for use while testing and will fail if MW_PHPUNIT_TEST is not defined. |
||
| 116 | * |
||
| 117 | * If this method is used it MUST also be called with null after a test to ensure a new |
||
| 118 | * default instance is created next time getDefaultInstance is called. |
||
| 119 | * |
||
| 120 | * @param WatchedItemStore|null $store |
||
| 121 | * |
||
| 122 | * @return ScopedCallback to reset the overridden value |
||
| 123 | * @throws MWException |
||
| 124 | */ |
||
| 125 | public static function overrideDefaultInstance( WatchedItemStore $store = null ) { |
||
| 126 | if ( !defined( 'MW_PHPUNIT_TEST' ) ) { |
||
| 127 | throw new MWException( |
||
| 128 | 'Cannot override ' . __CLASS__ . 'default instance in operation.' |
||
| 129 | ); |
||
| 130 | } |
||
| 131 | |||
| 132 | $previousValue = self::$instance; |
||
| 133 | self::$instance = $store; |
||
| 134 | return new ScopedCallback( function() use ( $previousValue ) { |
||
| 135 | self::$instance = $previousValue; |
||
| 136 | } ); |
||
| 137 | } |
||
| 138 | |||
| 139 | /** |
||
| 140 | * @return self |
||
| 141 | */ |
||
| 142 | public static function getDefaultInstance() { |
||
| 143 | if ( !self::$instance ) { |
||
| 144 | self::$instance = new self( |
||
| 145 | wfGetLB(), |
||
| 146 | new HashBagOStuff( [ 'maxKeys' => 100 ] ) |
||
| 147 | ); |
||
| 148 | } |
||
| 149 | return self::$instance; |
||
| 150 | } |
||
| 151 | |||
| 152 | private function getCacheKey( User $user, LinkTarget $target ) { |
||
| 153 | return $this->cache->makeKey( |
||
| 154 | (string)$target->getNamespace(), |
||
| 155 | $target->getDBkey(), |
||
| 156 | (string)$user->getId() |
||
| 157 | ); |
||
| 158 | } |
||
| 159 | |||
| 160 | private function cache( WatchedItem $item ) { |
||
| 161 | $user = $item->getUser(); |
||
| 162 | $target = $item->getLinkTarget(); |
||
| 163 | $key = $this->getCacheKey( $user, $target ); |
||
| 164 | $this->cache->set( $key, $item ); |
||
| 165 | $this->cacheIndex[$target->getNamespace()][$target->getDBkey()][$user->getId()] = $key; |
||
| 166 | } |
||
| 167 | |||
| 168 | private function uncache( User $user, LinkTarget $target ) { |
||
| 169 | $this->cache->delete( $this->getCacheKey( $user, $target ) ); |
||
| 170 | unset( $this->cacheIndex[$target->getNamespace()][$target->getDBkey()][$user->getId()] ); |
||
| 171 | } |
||
| 172 | |||
| 173 | private function uncacheLinkTarget( LinkTarget $target ) { |
||
| 181 | |||
| 182 | /** |
||
| 183 | * @param User $user |
||
| 184 | * @param LinkTarget $target |
||
| 185 | * |
||
| 186 | * @return WatchedItem|null |
||
| 187 | */ |
||
| 188 | private function getCached( User $user, LinkTarget $target ) { |
||
| 189 | return $this->cache->get( $this->getCacheKey( $user, $target ) ); |
||
| 190 | } |
||
| 191 | |||
| 192 | /** |
||
| 193 | * Return an array of conditions to select or update the appropriate database |
||
| 194 | * row. |
||
| 195 | * |
||
| 196 | * @param User $user |
||
| 197 | * @param LinkTarget $target |
||
| 198 | * |
||
| 199 | * @return array |
||
| 200 | */ |
||
| 201 | private function dbCond( User $user, LinkTarget $target ) { |
||
| 202 | return [ |
||
| 203 | 'wl_user' => $user->getId(), |
||
| 204 | 'wl_namespace' => $target->getNamespace(), |
||
| 205 | 'wl_title' => $target->getDBkey(), |
||
| 206 | ]; |
||
| 207 | } |
||
| 208 | |||
| 209 | /** |
||
| 210 | * @param int $slaveOrMaster DB_MASTER or DB_SLAVE |
||
| 211 | * |
||
| 212 | * @return DatabaseBase |
||
| 213 | * @throws MWException |
||
| 214 | */ |
||
| 215 | private function getConnection( $slaveOrMaster ) { |
||
| 218 | |||
| 219 | /** |
||
| 220 | * @param DatabaseBase $connection |
||
| 221 | * |
||
| 222 | * @throws MWException |
||
| 223 | */ |
||
| 224 | private function reuseConnection( $connection ) { |
||
| 227 | |||
| 228 | /** |
||
| 229 | * Count the number of individual items that are watched by the user. |
||
| 230 | * If a subject and corresponding talk page are watched this will return 2. |
||
| 231 | * |
||
| 232 | * @param User $user |
||
| 233 | * |
||
| 234 | * @return int |
||
| 235 | */ |
||
| 236 | public function countWatchedItems( User $user ) { |
||
| 250 | |||
| 251 | /** |
||
| 252 | * @param LinkTarget $target |
||
| 253 | * |
||
| 254 | * @return int |
||
| 255 | */ |
||
| 256 | public function countWatchers( LinkTarget $target ) { |
||
| 271 | |||
| 272 | /** |
||
| 273 | * Number of page watchers who also visited a "recent" edit |
||
| 274 | * |
||
| 275 | * @param LinkTarget $target |
||
| 276 | * @param mixed $threshold timestamp accepted by wfTimestamp |
||
| 277 | * |
||
| 278 | * @return int |
||
| 279 | * @throws DBUnexpectedError |
||
| 280 | * @throws MWException |
||
| 281 | */ |
||
| 282 | public function countVisitingWatchers( LinkTarget $target, $threshold ) { |
||
| 300 | |||
| 301 | /** |
||
| 302 | * @param LinkTarget[] $targets |
||
| 303 | * @param array $options Allowed keys: |
||
| 304 | * 'minimumWatchers' => int |
||
| 305 | * |
||
| 306 | * @return array multi dimensional like $return[$namespaceId][$titleString] = int $watchers |
||
| 307 | * All targets will be present in the result. 0 either means no watchers or the number |
||
| 308 | * of watchers was below the minimumWatchers option if passed. |
||
| 309 | */ |
||
| 310 | public function countWatchersMultiple( array $targets, array $options = [] ) { |
||
| 341 | |||
| 342 | /** |
||
| 343 | * Number of watchers of each page who have visited recent edits to that page |
||
| 344 | * |
||
| 345 | * @param array $targetsWithVisitThresholds array of pairs (LinkTarget $target, mixed $threshold), |
||
| 346 | * $threshold is: |
||
| 347 | * - a timestamp of the recent edit if $target exists (format accepted by wfTimestamp) |
||
| 348 | * - null if $target doesn't exist |
||
| 349 | * @param int|null $minimumWatchers |
||
| 350 | * @return array multi-dimensional like $return[$namespaceId][$titleString] = $watchers, |
||
| 351 | * where $watchers is an int: |
||
| 352 | * - if the page exists, number of users watching who have visited the page recently |
||
| 353 | * - if the page doesn't exist, number of users that have the page on their watchlist |
||
| 354 | * - 0 means there are no visiting watchers or their number is below the minimumWatchers |
||
| 355 | * option (if passed). |
||
| 356 | */ |
||
| 357 | public function countVisitingWatchersMultiple( |
||
| 391 | |||
| 392 | /** |
||
| 393 | * Generates condition for the query used in a batch count visiting watchers. |
||
| 394 | * |
||
| 395 | * @param IDatabase $db |
||
| 396 | * @param array $targetsWithVisitThresholds array of pairs (LinkTarget, last visit threshold) |
||
| 397 | * @return string |
||
| 398 | */ |
||
| 399 | private function getVisitingWatchersCondition( |
||
| 435 | |||
| 436 | /** |
||
| 437 | * Get an item (may be cached) |
||
| 438 | * |
||
| 439 | * @param User $user |
||
| 440 | * @param LinkTarget $target |
||
| 441 | * |
||
| 442 | * @return WatchedItem|false |
||
| 443 | */ |
||
| 444 | public function getWatchedItem( User $user, LinkTarget $target ) { |
||
| 455 | |||
| 456 | /** |
||
| 457 | * Loads an item from the db |
||
| 458 | * |
||
| 459 | * @param User $user |
||
| 460 | * @param LinkTarget $target |
||
| 461 | * |
||
| 462 | * @return WatchedItem|false |
||
| 463 | */ |
||
| 464 | public function loadWatchedItem( User $user, LinkTarget $target ) { |
||
| 492 | |||
| 493 | /** |
||
| 494 | * Must be called separately for Subject & Talk namespaces |
||
| 495 | * |
||
| 496 | * @param User $user |
||
| 497 | * @param LinkTarget $target |
||
| 498 | * |
||
| 499 | * @return bool |
||
| 500 | */ |
||
| 501 | public function isWatched( User $user, LinkTarget $target ) { |
||
| 504 | |||
| 505 | /** |
||
| 506 | * Must be called separately for Subject & Talk namespaces |
||
| 507 | * |
||
| 508 | * @param User $user |
||
| 509 | * @param LinkTarget $target |
||
| 510 | */ |
||
| 511 | public function addWatch( User $user, LinkTarget $target ) { |
||
| 514 | |||
| 515 | /** |
||
| 516 | * @param array[] $userTargetCombinations array of arrays containing [0] => User [1] => LinkTarget |
||
| 517 | * |
||
| 518 | * @return bool success |
||
| 519 | */ |
||
| 520 | public function addWatchBatch( array $userTargetCombinations ) { |
||
| 559 | |||
| 560 | /** |
||
| 561 | * Removes the an entry for the User watching the LinkTarget |
||
| 562 | * Must be called separately for Subject & Talk namespaces |
||
| 563 | * |
||
| 564 | * @param User $user |
||
| 565 | * @param LinkTarget $target |
||
| 566 | * |
||
| 567 | * @return bool success |
||
| 568 | * @throws DBUnexpectedError |
||
| 569 | * @throws MWException |
||
| 570 | */ |
||
| 571 | public function removeWatch( User $user, LinkTarget $target ) { |
||
| 592 | |||
| 593 | /** |
||
| 594 | * @param User $editor The editor that triggered the update. Their notification |
||
| 595 | * timestamp will not be updated(they have already seen it) |
||
| 596 | * @param LinkTarget $target The target to update timestamps for |
||
| 597 | * @param string $timestamp Set the update timestamp to this value |
||
| 598 | * |
||
| 599 | * @return int[] Array of user IDs the timestamp has been updated for |
||
| 600 | */ |
||
| 601 | public function updateNotificationTimestamp( User $editor, LinkTarget $target, $timestamp ) { |
||
| 641 | |||
| 642 | /** |
||
| 643 | * Reset the notification timestamp of this entry |
||
| 644 | * |
||
| 645 | * @param User $user |
||
| 646 | * @param Title $title |
||
| 647 | * @param string $force Whether to force the write query to be executed even if the |
||
| 648 | * page is not watched or the notification timestamp is already NULL. |
||
| 649 | * 'force' in order to force |
||
| 650 | * @param int $oldid The revision id being viewed. If not given or 0, latest revision is assumed. |
||
| 651 | * |
||
| 652 | * @return bool success |
||
| 653 | */ |
||
| 654 | public function resetNotificationTimestamp( User $user, Title $title, $force = '', $oldid = 0 ) { |
||
| 692 | |||
| 693 | private function getNotificationTimestamp( User $user, Title $title, $item, $force, $oldid ) { |
||
| 739 | |||
| 740 | /** |
||
| 741 | * @param User $user |
||
| 742 | * @param int $unreadLimit |
||
| 743 | * |
||
| 744 | * @return int|bool The number of unread notifications |
||
| 745 | * true if greater than or equal to $unreadLimit |
||
| 746 | */ |
||
| 747 | public function countUnreadNotifications( User $user, $unreadLimit = null ) { |
||
| 777 | |||
| 778 | /** |
||
| 779 | * Check if the given title already is watched by the user, and if so |
||
| 780 | * add a watch for the new title. |
||
| 781 | * |
||
| 782 | * To be used for page renames and such. |
||
| 783 | * |
||
| 784 | * @param LinkTarget $oldTarget |
||
| 785 | * @param LinkTarget $newTarget |
||
| 786 | */ |
||
| 787 | public function duplicateAllAssociatedEntries( LinkTarget $oldTarget, LinkTarget $newTarget ) { |
||
| 798 | |||
| 799 | /** |
||
| 800 | * Check if the given title already is watched by the user, and if so |
||
| 801 | * add a watch for the new title. |
||
| 802 | * |
||
| 803 | * To be used for page renames and such. |
||
| 804 | * This must be called separately for Subject and Talk pages |
||
| 805 | * |
||
| 806 | * @param LinkTarget $oldTarget |
||
| 807 | * @param LinkTarget $newTarget |
||
| 808 | */ |
||
| 809 | public function duplicateEntry( LinkTarget $oldTarget, LinkTarget $newTarget ) { |
||
| 851 | |||
| 852 | } |
||
| 853 |
Loading history...
