LinksDeletionUpdate::doUpdate() - Code Metrics - Inspection of "Daily Inspection: Merge "Improve tableExists() for..." - wikimedia/mediawiki - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Branch — master (174b3a)

unknown

created 2016-09-20 17:56 UTC

LinksDeletionUpdate::doUpdate() C

↳ Parent: LinksDeletionUpdate

Complexity

Conditions	8
Paths	54

Size

Total Lines	133
Code Lines	88

Duplication

Lines	10
Ratio	7.52 %

Importance

Changes

Metric	Value
cc	8
eloc	88
nc	54
nop	0
dl	10
loc	133
rs	5.2676
c	0
b	0
f	0

How to fix Long Method

<?php
/**
 * Updater for link tracking tables after a page edit.
 *
 * This program is free software; you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation; either version 2 of the License, or
 * (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License along
 * with this program; if not, write to the Free Software Foundation, Inc.,
 * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 * http://www.gnu.org/copyleft/gpl.html
 *
 * @file
 */
use MediaWiki\MediaWikiServices;

/**
 * Update object handling the cleanup of links tables after a page was deleted.
 **/
class LinksDeletionUpdate extends DataUpdate implements EnqueueableDataUpdate {
	/** @var WikiPage */
	protected $page;
	/** @var integer */
	protected $pageId;
	/** @var string */
	protected $timestamp;

	/** @var IDatabase */
	private $db;

	/**
	 * @param WikiPage $page Page we are updating
	 * @param integer|null $pageId ID of the page we are updating [optional]
	 * @param string|null $timestamp TS_MW timestamp of deletion
	 * @throws MWException
	 */
	function __construct( WikiPage $page, $pageId = null, $timestamp = null ) {
		parent::__construct();

		$this->page = $page;
		if ( $pageId ) {
0   == false // true
0   == null  // true
123 == false // false
123 == null  // false

// It is often better to use strict comparison
0 === false // false
0 === null  // false
			$this->pageId = $pageId; // page ID at time of deletion
		} elseif ( $page->exists() ) {
			$this->pageId = $page->getId();
		} else {
			throw new InvalidArgumentException( "Page ID not known. Page doesn't exist?" );
		}

		$this->timestamp = $timestamp ?: wfTimestampNow();
class Id
{
    public $id;

    public function __construct($id)
    {
        $this->id = $id;
    }

}

class Account
{
    /** @var  Id $id */
    public $id;
}

$account_id = false;

if (starsAreRight()) {
    $account_id = new Id(42);
}

$account = new Account();
if ($account instanceof Id)
{
    $account->id = $account_id;
}
	}

	public function doUpdate() {
		$services = MediaWikiServices::getInstance();
		$config = $services->getMainConfig();
		$lbFactory = $services->getDBLoadBalancerFactory();
		$batchSize = $config->get( 'UpdateRowsPerQuery' );

		// Page may already be deleted, so don't just getId()
		$id = $this->pageId;

		if ( $this->ticket ) {
			// Make sure all links update threads see the changes of each other.
			// This handles the case when updates have to batched into several COMMITs.
			$scopedLock = LinksUpdate::acquirePageLock( $this->getDB(), $id );
		}

		$title = $this->page->getTitle();
		$dbw = $this->getDB(); // convenience

		// Delete restrictions for it
		$dbw->delete( 'page_restrictions', [ 'pr_page' => $id ], __METHOD__ );

		// Fix category table counts
		$cats = $dbw->selectFieldValues(
			'categorylinks',
			'cl_to',
			[ 'cl_from' => $id ],
			__METHOD__
		);
		$catBatches = array_chunk( $cats, $batchSize );
		foreach ( $catBatches as $catBatch ) {
			$this->page->updateCategoryCounts( [], $catBatch, $id );
			if ( count( $catBatches ) > 1 ) {
				$lbFactory->commitAndWaitForReplication(
					__METHOD__, $this->ticket, [ 'wiki' => $dbw->getWikiID() ]
				);
			}
		}

		// Refresh the category table entry if it seems to have no pages. Check
		// master for the most up-to-date cat_pages count.
		if ( $title->getNamespace() === NS_CATEGORY ) {
			$row = $dbw->selectRow(
				'category',
				[ 'cat_id', 'cat_title', 'cat_pages', 'cat_subcats', 'cat_files' ],
				[ 'cat_title' => $title->getDBkey(), 'cat_pages <= 0' ],
				__METHOD__
			);
			if ( $row ) {
				Category::newFromRow( $row, $title )->refreshCounts();
/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}
			}
		}

		$this->batchDeleteByPK(
			'pagelinks',
			[ 'pl_from' => $id ],
			[ 'pl_from', 'pl_namespace', 'pl_title' ],
			$batchSize
		);
		$this->batchDeleteByPK(
			'imagelinks',
			[ 'il_from' => $id ],
			[ 'il_from', 'il_to' ],
			$batchSize
		);
		$this->batchDeleteByPK(
			'categorylinks',
			[ 'cl_from' => $id ],
			[ 'cl_from', 'cl_to' ],
			$batchSize
		);
		$this->batchDeleteByPK(
			'templatelinks',
			[ 'tl_from' => $id ],
			[ 'tl_from', 'tl_namespace', 'tl_title' ],
			$batchSize
		);
		$this->batchDeleteByPK(
			'externallinks',
			[ 'el_from' => $id ],
			[ 'el_id' ],
			$batchSize
		);
		$this->batchDeleteByPK(
			'langlinks',
			[ 'll_from' => $id ],
			[ 'll_from', 'll_lang' ],
			$batchSize
		);
		$this->batchDeleteByPK(
			'iwlinks',
			[ 'iwl_from' => $id ],
			[ 'iwl_from', 'iwl_prefix', 'iwl_title' ],
			$batchSize
		);

		// Delete any redirect entry or page props entries
		$dbw->delete( 'redirect', [ 'rd_from' => $id ], __METHOD__ );
		$dbw->delete( 'page_props', [ 'pp_page' => $id ], __METHOD__ );

		// Find recentchanges entries to clean up...
		$rcIdsForTitle = $dbw->selectFieldValues(
			'recentchanges',
			'rc_id',
			[
				'rc_type != ' . RC_LOG,
				'rc_namespace' => $title->getNamespace(),
				'rc_title' => $title->getDBkey(),
				'rc_timestamp < ' .
					$dbw->addQuotes( $dbw->timestamp( $this->timestamp ) )
			],
			__METHOD__
		);
		$rcIdsForPage = $dbw->selectFieldValues(
			'recentchanges',
			'rc_id',
			[ 'rc_type != ' . RC_LOG, 'rc_cur_id' => $id ],
			__METHOD__
		);

		// T98706: delete by PK to avoid lock contention with RC delete log insertions
		$rcIdBatches = array_chunk( array_merge( $rcIdsForTitle, $rcIdsForPage ), $batchSize );
		foreach ( $rcIdBatches as $rcIdBatch ) {
			$dbw->delete( 'recentchanges', [ 'rc_id' => $rcIdBatch ], __METHOD__ );
			if ( count( $rcIdBatches ) > 1 ) {
				$lbFactory->commitAndWaitForReplication(
					__METHOD__, $this->ticket, [ 'wiki' => $dbw->getWikiID() ]
				);
			}
		}

		// Commit and release the lock (if set)
		ScopedCallback::consume( $scopedLock );
	}

	private function batchDeleteByPK( $table, array $conds, array $pk, $bSize ) {
		$services = MediaWikiServices::getInstance();
		$lbFactory = $services->getDBLoadBalancerFactory();
		$dbw = $this->getDB(); // convenience

		$res = $dbw->select( $table, $pk, $conds, __METHOD__ );

		$pkDeleteConds = [];
		foreach ( $res as $row ) {
$collection = json_decode($data, true);
if ( ! is_array($collection)) {
    throw new \RuntimeException('$collection must be an array.');
}

foreach ($collection as $item) { /** ... */ }
			$pkDeleteConds[] = $dbw->makeList( (array)$row, LIST_AND );
			if ( count( $pkDeleteConds ) >= $bSize ) {
				$dbw->delete( $table, $dbw->makeList( $pkDeleteConds, LIST_OR ), __METHOD__ );
				$lbFactory->commitAndWaitForReplication(
					__METHOD__, $this->ticket, [ 'wiki' => $dbw->getWikiID() ]
				);
				$pkDeleteConds = [];
			}
		}

		if ( $pkDeleteConds ) {
			$dbw->delete( $table, $dbw->makeList( $pkDeleteConds, LIST_OR ), __METHOD__ );
		}
	}

	protected function getDB() {
		if ( !$this->db ) {
			$this->db = wfGetDB( DB_MASTER );
		}

		return $this->db;
	}

	public function getAsJobSpecification() {
		return [
			'wiki' => $this->getDB()->getWikiID(),
			'job'  => new JobSpecification(
				'deleteLinks',
				[ 'pageId' => $this->pageId, 'timestamp' => $this->timestamp ],
				[ 'removeDuplicates' => true ],
				$this->page->getTitle()
			)
		];
	}
}


1		<?php
2		/**
3		* Updater for link tracking tables after a page edit.
4		*
5		* This program is free software; you can redistribute it and/or modify
6		* it under the terms of the GNU General Public License as published by
7		* the Free Software Foundation; either version 2 of the License, or
8		* (at your option) any later version.
9		*
10		* This program is distributed in the hope that it will be useful,
11		* but WITHOUT ANY WARRANTY; without even the implied warranty of
12		* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13		* GNU General Public License for more details.
14		*
15		* You should have received a copy of the GNU General Public License along
16		* with this program; if not, write to the Free Software Foundation, Inc.,
17		* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
18		* http://www.gnu.org/copyleft/gpl.html
19		*
20		* @file
21		*/
22		use MediaWiki\MediaWikiServices;
23
24		/**
25		* Update object handling the cleanup of links tables after a page was deleted.
26		**/
27		class LinksDeletionUpdate extends DataUpdate implements EnqueueableDataUpdate {
28		/** @var WikiPage */
29		protected $page;
30		/** @var integer */
31		protected $pageId;
32		/** @var string */
33		protected $timestamp;
34
35		/** @var IDatabase */
36		private $db;
37
38		/**
39		* @param WikiPage $page Page we are updating
40		* @param integer\|null $pageId ID of the page we are updating [optional]
41		* @param string\|null $timestamp TS_MW timestamp of deletion
42		* @throws MWException
43		*/
44		function __construct( WikiPage $page, $pageId = null, $timestamp = null ) {
45		parent::__construct();
46
47		$this->page = $page;
48		if ( $pageId ) {
		0 ignored issues – show Bug Best Practice introduced 2016-06-15 18:10 UTC by Report Bug Copy Issue Report The expression `$pageId` of type `integer\|null` is loosely compared to `true`; this is ambiguous if the integer can be zero. You might want to explicitly use `!== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `integer` values, zero is a special case, in particular the following results might be unexpected: 0 == false // true 0 == null // true 123 == false // false 123 == null // false // It is often better to use strict comparison 0 === false // false 0 === null // false Loading history...
49		$this->pageId = $pageId; // page ID at time of deletion
50		} elseif ( $page->exists() ) {
51		$this->pageId = $page->getId();
52		} else {
53		throw new InvalidArgumentException( "Page ID not known. Page doesn't exist?" );
54		}
55
56		$this->timestamp = $timestamp ?: wfTimestampNow();
		0 ignored issues – show Documentation Bug introduced 2016-06-15 18:10 UTC by Report Bug Copy Issue Report It seems like `$timestamp ?: wfTimestampNow()` can also be of type `false`. However, the property `$timestamp` is declared as type `string`. Maybe add an additional type check? Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly. For example, imagine you have a variable `$accountId` that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to the `id` property of an instance of the `Account` class. This class holds a proper account, so the id value must no longer be false. Either this assignment is in error or a type check should be added for that assignment. class Id { public $id; public function __construct($id) { $this->id = $id; } } class Account { /** @var Id $id */ public $id; } $account_id = false; if (starsAreRight()) { $account_id = new Id(42); } $account = new Account(); if ($account instanceof Id) { $account->id = $account_id; } Loading history...
57		}
58
59		public function doUpdate() {
60		$services = MediaWikiServices::getInstance();
61		$config = $services->getMainConfig();
62		$lbFactory = $services->getDBLoadBalancerFactory();
63		$batchSize = $config->get( 'UpdateRowsPerQuery' );
64
65		// Page may already be deleted, so don't just getId()
66		$id = $this->pageId;
67
68		if ( $this->ticket ) {
69		// Make sure all links update threads see the changes of each other.
70		// This handles the case when updates have to batched into several COMMITs.
71		$scopedLock = LinksUpdate::acquirePageLock( $this->getDB(), $id );
72		}
73
74		$title = $this->page->getTitle();
75		$dbw = $this->getDB(); // convenience
76
77		// Delete restrictions for it
78		$dbw->delete( 'page_restrictions', [ 'pr_page' => $id ], __METHOD__ );
79
80		// Fix category table counts
81		$cats = $dbw->selectFieldValues(
82		'categorylinks',
83		'cl_to',
84		[ 'cl_from' => $id ],
85		__METHOD__
86		);
87		$catBatches = array_chunk( $cats, $batchSize );
88		foreach ( $catBatches as $catBatch ) {
89		$this->page->updateCategoryCounts( [], $catBatch, $id );
90	View Code Duplication	if ( count( $catBatches ) > 1 ) {
91		$lbFactory->commitAndWaitForReplication(
92		__METHOD__, $this->ticket, [ 'wiki' => $dbw->getWikiID() ]
93		);
94		}
95		}
96
97		// Refresh the category table entry if it seems to have no pages. Check
98		// master for the most up-to-date cat_pages count.
99		if ( $title->getNamespace() === NS_CATEGORY ) {
100		$row = $dbw->selectRow(
101		'category',
102		[ 'cat_id', 'cat_title', 'cat_pages', 'cat_subcats', 'cat_files' ],
103		[ 'cat_title' => $title->getDBkey(), 'cat_pages <= 0' ],
104		__METHOD__
105		);
106		if ( $row ) {
107		Category::newFromRow( $row, $title )->refreshCounts();
		0 ignored issues – show Bug introduced 2016-09-01 18:04 UTC by Report Bug Copy Issue Report It seems like `$row` defined by `$dbw->selectRow('categor...ges <= 0'), __METHOD__)` on line `100` can also be of type `boolean`; however, `Category::newFromRow()` does only seem to accept `object`, maybe add an additional type check? If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check: /** * @return array\|string */ function returnsDifferentValues($x) { if ($x) { return 'foo'; } return array(); } $x = returnsDifferentValues($y); if (is_array($x)) { // $x is an array. } If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue. Loading history...
108		}
109		}
110
111		$this->batchDeleteByPK(
112		'pagelinks',
113		[ 'pl_from' => $id ],
114		[ 'pl_from', 'pl_namespace', 'pl_title' ],
115		$batchSize
116		);
117		$this->batchDeleteByPK(
118		'imagelinks',
119		[ 'il_from' => $id ],
120		[ 'il_from', 'il_to' ],
121		$batchSize
122		);
123		$this->batchDeleteByPK(
124		'categorylinks',
125		[ 'cl_from' => $id ],
126		[ 'cl_from', 'cl_to' ],
127		$batchSize
128		);
129		$this->batchDeleteByPK(
130		'templatelinks',
131		[ 'tl_from' => $id ],
132		[ 'tl_from', 'tl_namespace', 'tl_title' ],
133		$batchSize
134		);
135		$this->batchDeleteByPK(
136		'externallinks',
137		[ 'el_from' => $id ],
138		[ 'el_id' ],
139		$batchSize
140		);
141		$this->batchDeleteByPK(
142		'langlinks',
143		[ 'll_from' => $id ],
144		[ 'll_from', 'll_lang' ],
145		$batchSize
146		);
147		$this->batchDeleteByPK(
148		'iwlinks',
149		[ 'iwl_from' => $id ],
150		[ 'iwl_from', 'iwl_prefix', 'iwl_title' ],
151		$batchSize
152		);
153
154		// Delete any redirect entry or page props entries
155		$dbw->delete( 'redirect', [ 'rd_from' => $id ], __METHOD__ );
156		$dbw->delete( 'page_props', [ 'pp_page' => $id ], __METHOD__ );
157
158		// Find recentchanges entries to clean up...
159		$rcIdsForTitle = $dbw->selectFieldValues(
160		'recentchanges',
161		'rc_id',
162		[
163		'rc_type != ' . RC_LOG,
164		'rc_namespace' => $title->getNamespace(),
165		'rc_title' => $title->getDBkey(),
166		'rc_timestamp < ' .
167		$dbw->addQuotes( $dbw->timestamp( $this->timestamp ) )
168		],
169		__METHOD__
170		);
171		$rcIdsForPage = $dbw->selectFieldValues(
172		'recentchanges',
173		'rc_id',
174		[ 'rc_type != ' . RC_LOG, 'rc_cur_id' => $id ],
175		__METHOD__
176		);
177
178		// T98706: delete by PK to avoid lock contention with RC delete log insertions
179		$rcIdBatches = array_chunk( array_merge( $rcIdsForTitle, $rcIdsForPage ), $batchSize );
180		foreach ( $rcIdBatches as $rcIdBatch ) {
181		$dbw->delete( 'recentchanges', [ 'rc_id' => $rcIdBatch ], __METHOD__ );
182	View Code Duplication	if ( count( $rcIdBatches ) > 1 ) {
183		$lbFactory->commitAndWaitForReplication(
184		__METHOD__, $this->ticket, [ 'wiki' => $dbw->getWikiID() ]
185		);
186		}
187		}
188
189		// Commit and release the lock (if set)
190		ScopedCallback::consume( $scopedLock );
191		}
192
193		private function batchDeleteByPK( $table, array $conds, array $pk, $bSize ) {
194		$services = MediaWikiServices::getInstance();
195		$lbFactory = $services->getDBLoadBalancerFactory();
196		$dbw = $this->getDB(); // convenience
197
198		$res = $dbw->select( $table, $pk, $conds, __METHOD__ );
199
200		$pkDeleteConds = [];
201		foreach ( $res as $row ) {
		0 ignored issues – show Bug introduced 2016-06-03 18:03 UTC by Report Bug Copy Issue Report The expression `$res` of type `object<ResultWrapper>\|boolean` is not guaranteed to be traversable. How about adding an additional type check? There are different options of fixing this problem. If you want to be on the safe side, you can add an additional type-check: $collection = json_decode($data, true); if ( ! is_array($collection)) { throw new \RuntimeException('$collection must be an array.'); } foreach ($collection as $item) { /** ... / } 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: /** @var array $collection / $collection = json_decode($data, true); foreach ($collection as $item) { /* .. / } Mark the issue as a false-positive*: Just hover the remove button, in the top-right corner of this issue for more options. Loading history...
202		$pkDeleteConds[] = $dbw->makeList( (array)$row, LIST_AND );
203		if ( count( $pkDeleteConds ) >= $bSize ) {
204		$dbw->delete( $table, $dbw->makeList( $pkDeleteConds, LIST_OR ), __METHOD__ );
205		$lbFactory->commitAndWaitForReplication(
206		__METHOD__, $this->ticket, [ 'wiki' => $dbw->getWikiID() ]
207		);
208		$pkDeleteConds = [];
209		}
210		}
211
212		if ( $pkDeleteConds ) {
213		$dbw->delete( $table, $dbw->makeList( $pkDeleteConds, LIST_OR ), __METHOD__ );
214		}
215		}
216
217		protected function getDB() {
218		if ( !$this->db ) {
219		$this->db = wfGetDB( DB_MASTER );
220		}
221
222		return $this->db;
223		}
224
225		public function getAsJobSpecification() {
226		return [
227		'wiki' => $this->getDB()->getWikiID(),
228		'job' => new JobSpecification(
229		'deleteLinks',
230		[ 'pageId' => $this->pageId, 'timestamp' => $this->timestamp ],
231		[ 'removeDuplicates' => true ],
232		$this->page->getTitle()
233		)
234		];
235		}
236		}
237

wikimedia / mediawiki

Branch — master (174b3a)

LinksDeletionUpdate::doUpdate() C

Complexity

Size

Duplication

Importance

How to fix Long Method

Long Method

Duplication Side-by-Side

Filter issues like