ReplicaMasterAwareRecordIdsAcquirer::connectMaster() - Code Metrics - Inspection of "Merge pull request #26 from wmde/loadbalancer" - wmde/mediawiki-term-store - Measure and Improve Code Quality continuously with Scrutinizer

Passed

Push — master ( e35b6a...605339 )

by Alaa

created 2019-05-22 08:39 UTC

connectMaster() A

↳ Parent: ReplicaMasterAwareRecordIdsAcquirer

Complexity

Conditions	2
Paths	2

Size

Total Lines

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	5
rs	10
c	0
b	0
f	0
cc	2
nc	2
nop	0

<?php

namespace Wikibase\TermStore\MediaWiki\PackagePrivate\Util;

use AppendIterator;
use ArrayIterator;
use Psr\Log\LoggerInterface;
use Psr\Log\NullLogger;
use Wikimedia\Rdbms\DBQueryError;
use Wikimedia\Rdbms\IDatabase;
use Wikimedia\Rdbms\ILoadBalancer;

/**
 * Allows acquiring ids of records in database table,
 * by inspecting a given read-only replica database to initially
 * find existing records with their ids, and insert non-existing
 * records into a read-write master databas and getting those
 * ids as well from the master database after insertion.
 */
class ReplicaMasterAwareRecordIdsAcquirer {

	/**
	 * @var ILoadBalancer
	 */
	private $loadBalancer;

	/**
	 * @var IDatabase $dbMaster master database to insert non-existing records into
	 */
	private $dbMaster = null;

	/**
	 * @var IDatabase $dbReplica replica database to initially query existing records in
	 */
	private $dbReplica = null;

	/**
	 * @var string $table
	 */
	private $table;

	/**
	 * @var string $idColumn
	 */
	private $idColumn;

	/**
	 * @var LoggerInterface|null $logger
	 */
	private $logger;

	/**
	 * @param ILoadBalancer $loadBalancer database connection accessor
	 * @param string $table the name of the table this acquirer is for
	 * @param string $idColumn the name of the column that contains the desired ids
	 * @param LoggerInterface $logger
	 */
	public function __construct(
		ILoadBalancer $loadBalancer,
		$table,
		$idColumn,
		LoggerInterface $logger = null
	) {
		$this->loadBalancer = $loadBalancer;
		$this->table = $table;
		$this->idColumn = $idColumn;
		$this->logger = $logger ?? new NullLogger();
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;
}
	}

	/**
	 * Acquire ids of needed records in the table, inserting non-existing
	 * ones into master database.
	 *
	 * Note 1: this function assumes that all records given in $neededRecords specify
	 * the same columns. If some records specify less, more or different columns than
	 * the first one does, the behavior is not defined.
	 *
	 * Note 2: this function assumes that all records given in $neededRecords have
	 * their values as strings. If some values are of different type (e.g. integer ids)
	 * this can cause infinite loops due to mismatch in identifying records selected in
	 * database with their corresponding needed records. The first element keys will be
	 * used as the set of columns to select in database and to provide back in the returned array.
	 *
	 * @param array $neededRecords array of records to be looked-up or inserted.
	 *	Each entry in this array should an associative array of column => value pairs.
	 *	Example:
	 *	[
	 *		[ 'columnA' => 'valueA1', 'columnB' => 'valueB1' ],
	 *		[ 'columnA' => 'valueA2', 'columnB' => 'valueB2' ],
	 *		...
	 *	]
	 *
	 * @return array the array of input recrods along with their ids
	 *	Example:
	 *	[
	 *		[ 'columnA' => 'valueA1', 'columnB' => 'valueB1', 'idColumn' => '1' ],
	 *		[ 'columnA' => 'valueA2', 'columnB' => 'valueB2', 'idColumn' => '2' ],
	 *		...
	 *	]
	 */
	public function acquireIds( array $neededRecords ) {
		$this->connectReplica();
		$existingRecords = $this->findExistingRecords( $this->dbReplica, $neededRecords );
		$neededRecords = $this->filterNonExistingRecords( $neededRecords, $existingRecords );

		while ( !empty( $neededRecords ) ) {
			$this->connectMaster();
			$this->insertNonExistingRecordsIntoMaster( $neededRecords );
			$existingRecords = array_merge(
				$existingRecords,
				$this->findExistingRecords( $this->dbMaster, $neededRecords )
			);
			$neededRecords = $this->filterNonExistingRecords( $neededRecords, $existingRecords );
		}

		return $existingRecords;
	}

	private function connectReplica() {
		if ( $this->dbReplica === null ) {
			$this->dbReplica = $this->loadBalancer->getConnection( ILoadBalancer::DB_REPLICA );
		}
	}

	private function connectMaster() {
		if ( $this->dbMaster === null ) {
			$this->dbMaster = $this->loadBalancer->getConnection( ILoadBalancer::DB_MASTER );
		}
	}

	private function findExistingRecords( IDatabase $db, array $neededRecords ): array {
		$recordsSelectConditions = array_map( function ( $record ) use ( $db ) {
			return $db->makeList( $record, IDatabase::LIST_AND );
		}, $neededRecords );

		/*
		 * Todo, related to Note 1 on self::acquireIds():
		 * this class can allow for specifying a different set of columns to select
		 * and return back from self::acquireIds(). This set of columns can be added as
		 * an optional argument to self::acquireIds() for instance, the current solution
		 * in here can be a fallback when that isn't given.
		 */
		$selectColumns = array_keys( $neededRecords[0] );
		$selectColumns[] = $this->idColumn;

		$existingRows = $db->select(
			$this->table,
			$selectColumns,
			$db->makeList( $recordsSelectConditions, IDatabase::LIST_OR )
		);

		$existingRecords = [];
		foreach ( $existingRows as $row ) {
			$existingRecord = [];
			foreach ( $selectColumns as $column ) {
				$existingRecord[$column] = $row->$column;
			}
			$existingRecords[] = $existingRecord;
		}

		return $existingRecords;
	}

	private function insertNonExistingRecordsIntoMaster( array $neededRecords ) {
		$uniqueRecords = [];
		foreach ( $neededRecords as $record ) {
			$recordHash = $this->calcRecordHash( $record );
			$uniqueRecords[$recordHash] = $record;
		}

		try {
			$this->dbMaster->insert( $this->table, array_values( $uniqueRecords ) );
		} catch ( DBQueryError $dbError ) {

			$this->logger->info(
				'{method}: Inserting records into {table} failed: {exception}',
				[
					'method' => __METHOD__,
					'exception' => $dbError,
					'table' => $this->table,
					'records' => $uniqueRecords
				]
			);
		}
	}

	private function filterNonExistingRecords( $neededRecords, $existingRecords ): array {
		$existingRecordsHashes = [];
		foreach ( $existingRecords as $record ) {
			unset( $record[$this->idColumn] );
			$recordHash = $this->calcRecordHash( $record );
			$existingRecordsHashes[$recordHash] = true;
		}

		$nonExistingRecords = [];
		foreach ( $neededRecords as $record ) {
			$recordHash = $this->calcRecordHash( $record );

			if ( !isset( $existingRecordsHashes[$recordHash] ) ) {
				$nonExistingRecords[] = $record;
			}
		}

		return $nonExistingRecords;
	}

	/**
	 * Implementation detail, related to Note 2 on self::acquireIds():
	 * this function relies on the fact that the given set of needed records will have
	 * all values as strings in order to produce hashes that match up correctly with
	 * selected records in database, because database selection will always return
	 * values as strings.
	 */
	private function calcRecordHash( array $record ) {
		ksort( $record );
		return md5( serialize( $record ) );
	}

}



1			<?php
2
3			namespace Wikibase\TermStore\MediaWiki\PackagePrivate\Util;
4
5			use AppendIterator;
6			use ArrayIterator;
7			use Psr\Log\LoggerInterface;
8			use Psr\Log\NullLogger;
9			use Wikimedia\Rdbms\DBQueryError;
10			use Wikimedia\Rdbms\IDatabase;
11			use Wikimedia\Rdbms\ILoadBalancer;
12
13			/**
14			* Allows acquiring ids of records in database table,
15			* by inspecting a given read-only replica database to initially
16			* find existing records with their ids, and insert non-existing
17			* records into a read-write master databas and getting those
18			* ids as well from the master database after insertion.
19			*/
20			class ReplicaMasterAwareRecordIdsAcquirer {
21
22			/**
23			* @var ILoadBalancer
24			*/
25			private $loadBalancer;
26
27			/**
28			* @var IDatabase $dbMaster master database to insert non-existing records into
29			*/
30			private $dbMaster = null;
31
32			/**
33			* @var IDatabase $dbReplica replica database to initially query existing records in
34			*/
35			private $dbReplica = null;
36
37			/**
38			* @var string $table
39			*/
40			private $table;
41
42			/**
43			* @var string $idColumn
44			*/
45			private $idColumn;
46
47			/**
48			* @var LoggerInterface\|null $logger
49			*/
50			private $logger;
51
52			/**
53			* @param ILoadBalancer $loadBalancer database connection accessor
54			* @param string $table the name of the table this acquirer is for
55			* @param string $idColumn the name of the column that contains the desired ids
56			* @param LoggerInterface $logger
57			*/
58			public function __construct(
59			ILoadBalancer $loadBalancer,
60			$table,
61			$idColumn,
62			LoggerInterface $logger = null
63			) {
64			$this->loadBalancer = $loadBalancer;
65			$this->table = $table;
66			$this->idColumn = $idColumn;
67			$this->logger = $logger ?? new NullLogger();
			0 ignored issues – show Documentation Bug introduced 2019-05-15 23:31 UTC by Report Bug Copy Issue Report It seems like `$logger ?? new \Psr\Log\NullLogger()` can also be of type `object<Psr\Log\NullLogger>`. However, the property `$logger` is declared as type `object<Psr\Log\LoggerInterface>\|null`. 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...
68			}
69
70			/**
71			* Acquire ids of needed records in the table, inserting non-existing
72			* ones into master database.
73			*
74			* Note 1: this function assumes that all records given in $neededRecords specify
75			* the same columns. If some records specify less, more or different columns than
76			* the first one does, the behavior is not defined.
77			*
78			* Note 2: this function assumes that all records given in $neededRecords have
79			* their values as strings. If some values are of different type (e.g. integer ids)
80			* this can cause infinite loops due to mismatch in identifying records selected in
81			* database with their corresponding needed records. The first element keys will be
82			* used as the set of columns to select in database and to provide back in the returned array.
83			*
84			* @param array $neededRecords array of records to be looked-up or inserted.
85			* Each entry in this array should an associative array of column => value pairs.
86			* Example:
87			* [
88			* [ 'columnA' => 'valueA1', 'columnB' => 'valueB1' ],
89			* [ 'columnA' => 'valueA2', 'columnB' => 'valueB2' ],
90			* ...
91			* ]
92			*
93			* @return array the array of input recrods along with their ids
94			* Example:
95			* [
96			* [ 'columnA' => 'valueA1', 'columnB' => 'valueB1', 'idColumn' => '1' ],
97			* [ 'columnA' => 'valueA2', 'columnB' => 'valueB2', 'idColumn' => '2' ],
98			* ...
99			* ]
100			*/
101			public function acquireIds( array $neededRecords ) {
102			$this->connectReplica();
103			$existingRecords = $this->findExistingRecords( $this->dbReplica, $neededRecords );
104			$neededRecords = $this->filterNonExistingRecords( $neededRecords, $existingRecords );
105
106			while ( !empty( $neededRecords ) ) {
107			$this->connectMaster();
108			$this->insertNonExistingRecordsIntoMaster( $neededRecords );
109			$existingRecords = array_merge(
110			$existingRecords,
111			$this->findExistingRecords( $this->dbMaster, $neededRecords )
112			);
113			$neededRecords = $this->filterNonExistingRecords( $neededRecords, $existingRecords );
114			}
115
116			return $existingRecords;
117			}
118
119			private function connectReplica() {
120			if ( $this->dbReplica === null ) {
121			$this->dbReplica = $this->loadBalancer->getConnection( ILoadBalancer::DB_REPLICA );
122			}
123			}
124
125			private function connectMaster() {
126			if ( $this->dbMaster === null ) {
127			$this->dbMaster = $this->loadBalancer->getConnection( ILoadBalancer::DB_MASTER );
128			}
129			}
130
131			private function findExistingRecords( IDatabase $db, array $neededRecords ): array {
132			$recordsSelectConditions = array_map( function ( $record ) use ( $db ) {
133			return $db->makeList( $record, IDatabase::LIST_AND );
134			}, $neededRecords );
135
136			/*
137			* Todo, related to Note 1 on self::acquireIds():
138			* this class can allow for specifying a different set of columns to select
139			* and return back from self::acquireIds(). This set of columns can be added as
140			* an optional argument to self::acquireIds() for instance, the current solution
141			* in here can be a fallback when that isn't given.
142			*/
143			$selectColumns = array_keys( $neededRecords[0] );
144			$selectColumns[] = $this->idColumn;
145
146			$existingRows = $db->select(
147			$this->table,
148			$selectColumns,
149			$db->makeList( $recordsSelectConditions, IDatabase::LIST_OR )
150			);
151
152			$existingRecords = [];
153			foreach ( $existingRows as $row ) {
154			$existingRecord = [];
155			foreach ( $selectColumns as $column ) {
156			$existingRecord[$column] = $row->$column;
157			}
158			$existingRecords[] = $existingRecord;
159			}
160
161			return $existingRecords;
162			}
163
164			private function insertNonExistingRecordsIntoMaster( array $neededRecords ) {
165			$uniqueRecords = [];
166			foreach ( $neededRecords as $record ) {
167			$recordHash = $this->calcRecordHash( $record );
168			$uniqueRecords[$recordHash] = $record;
169			}
170
171			try {
172			$this->dbMaster->insert( $this->table, array_values( $uniqueRecords ) );
173			} catch ( DBQueryError $dbError ) {
			0 ignored issues – show Bug introduced 2019-05-15 07:42 UTC by Report Bug Copy Issue Report The class `Wikimedia\Rdbms\DBQueryError` does not exist. Did you forget a USE statement, or did you not list all dependencies? Scrutinizer analyzes your `composer.json`/`composer.lock` file if available to determine the classes, and functions that are defined by your dependencies. It seems like the listed class was neither found in your dependencies, nor was it found in the analyzed files in your repository. If you are using some other form of dependency management, you might want to disable this analysis. Loading history...
174			$this->logger->info(
175			'{method}: Inserting records into {table} failed: {exception}',
176			[
177			'method' => __METHOD__,
178			'exception' => $dbError,
179			'table' => $this->table,
180			'records' => $uniqueRecords
181			]
182			);
183			}
184			}
185
186			private function filterNonExistingRecords( $neededRecords, $existingRecords ): array {
187			$existingRecordsHashes = [];
188			foreach ( $existingRecords as $record ) {
189			unset( $record[$this->idColumn] );
190			$recordHash = $this->calcRecordHash( $record );
191			$existingRecordsHashes[$recordHash] = true;
192			}
193
194			$nonExistingRecords = [];
195			foreach ( $neededRecords as $record ) {
196			$recordHash = $this->calcRecordHash( $record );
197
198			if ( !isset( $existingRecordsHashes[$recordHash] ) ) {
199			$nonExistingRecords[] = $record;
200			}
201			}
202
203			return $nonExistingRecords;
204			}
205
206			/**
207			* Implementation detail, related to Note 2 on self::acquireIds():
208			* this function relies on the fact that the given set of needed records will have
209			* all values as strings in order to produce hashes that match up correctly with
210			* selected records in database, because database selection will always return
211			* values as strings.
212			*/
213			private function calcRecordHash( array $record ) {
214			ksort( $record );
215			return md5( serialize( $record ) );
216			}
217
218			}
219
220

wmde / mediawiki-term-store

Push — master ( e35b6a...605339 )

connectMaster() A

Complexity

Size

Duplication

Importance

Duplication Side-by-Side

Filter issues like