Skip to content

Commit f331d86

Browse files
come-nccome-nc
authored
Merge pull request #35539 from nextcloud/cleanup/carl/application-contactsmanager
Cleanup psalm issues in DB/ContactsManager and Console
2 parents 7ee34a3 + 637cafc commit f331d86

File tree

9 files changed

+89
-85
lines changed

9 files changed

+89
-85
lines changed

‎lib/private/Console/Application.php‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,12 @@
4747
use Symfony\Component\Console\Output\OutputInterface;
4848

4949
class Application {
50-
/** @var IConfig */
51-
private $config;
50+
private IConfig $config;
5251
private SymfonyApplication $application;
53-
/** @var IEventDispatcher */
54-
private $dispatcher;
55-
/** @var IRequest */
56-
private $request;
57-
/** @var LoggerInterface */
58-
private $logger;
59-
/** @var MemoryInfo */
60-
private $memoryInfo;
52+
private IEventDispatcher $dispatcher;
53+
private IRequest $request;
54+
private LoggerInterface $logger;
55+
private MemoryInfo $memoryInfo;
6156

6257
public function __construct(IConfig $config,
6358
IEventDispatcher $dispatcher,
@@ -74,8 +69,6 @@ public function __construct(IConfig $config,
7469
}
7570

7671
/**
77-
* @param InputInterface $input
78-
* @param ConsoleOutputInterface $output
7972
* @throws \Exception
8073
*/
8174
public function loadCommands(

‎lib/private/ContactsManager.php‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -91,32 +91,32 @@ public function search($pattern, $searchProperties = [], $options = []) {
9191
* This function can be used to delete the contact identified by the given id
9292
*
9393
* @param int $id the unique identifier to a contact
94-
* @param string $address_book_key identifier of the address book in which the contact shall be deleted
94+
* @param string $addressBookKey identifier of the address book in which the contact shall be deleted
9595
* @return bool successful or not
9696
*/
97-
public function delete($id, $address_book_key) {
98-
$addressBook = $this->getAddressBook($address_book_key);
97+
public function delete($id, $addressBookKey) {
98+
$addressBook = $this->getAddressBook($addressBookKey);
9999
if (!$addressBook) {
100-
return null;
100+
return false;
101101
}
102102

103103
if ($addressBook->getPermissions() & Constants::PERMISSION_DELETE) {
104104
return $addressBook->delete($id);
105105
}
106106

107-
return null;
107+
return false;
108108
}
109109

110110
/**
111111
* This function is used to create a new contact if 'id' is not given or not present.
112112
* Otherwise the contact will be updated by replacing the entire data set.
113113
*
114114
* @param array $properties this array if key-value-pairs defines a contact
115-
* @param string $address_book_key identifier of the address book in which the contact shall be created or updated
116-
* @return array representing the contact just created or updated
115+
* @param string $addressBookKey identifier of the address book in which the contact shall be created or updated
116+
* @return ?array representing the contact just created or updated
117117
*/
118-
public function createOrUpdate($properties, $address_book_key) {
119-
$addressBook = $this->getAddressBook($address_book_key);
118+
public function createOrUpdate($properties, $addressBookKey) {
119+
$addressBook = $this->getAddressBook($addressBookKey);
120120
if (!$addressBook) {
121121
return null;
122122
}
@@ -133,7 +133,7 @@ public function createOrUpdate($properties, $address_book_key) {
133133
*
134134
* @return bool true if enabled, false if not
135135
*/
136-
public function isEnabled() {
136+
public function isEnabled(): bool {
137137
return !empty($this->addressBooks) || !empty($this->addressBookLoaders);
138138
}
139139

@@ -192,11 +192,8 @@ public function register(\Closure $callable) {
192192

193193
/**
194194
* Get (and load when needed) the address book for $key
195-
*
196-
* @param string $addressBookKey
197-
* @return IAddressBook
198195
*/
199-
protected function getAddressBook($addressBookKey) {
196+
protected function getAddressBook(string $addressBookKey): ?IAddressBook {
200197
$this->loadAddressBooks();
201198
if (!array_key_exists($addressBookKey, $this->addressBooks)) {
202199
return null;

‎lib/private/DB/Adapter.php‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
use Doctrine\DBAL\Exception;
3232
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
33+
use OC\DB\Exceptions\DbalException;
3334

3435
/**
3536
* This handles the way we use to write queries, into something that can be
@@ -142,9 +143,12 @@ public function insertIgnoreConflict(string $table, array $values) : int {
142143
foreach ($values as $key => $value) {
143144
$builder->setValue($key, $builder->createNamedParameter($value));
144145
}
145-
return $builder->execute();
146-
} catch (UniqueConstraintViolationException $e) {
147-
return 0;
146+
return $builder->executeStatement();
147+
} catch (DbalException $e) {
148+
if ($e->getReason() === \OCP\DB\Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
149+
return 0;
150+
}
151+
throw $e;
148152
}
149153
}
150154
}

‎lib/private/DB/Connection.php‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ public function getPrefix() {
220220
* @return Statement The prepared statement.
221221
* @throws Exception
222222
*/
223-
public function prepare($statement, $limit = null, $offset = null): Statement {
223+
public function prepare($sql, $limit = null, $offset = null): Statement {
224224
if ($limit === -1 || $limit === null) {
225225
$limit = null;
226226
} else {
@@ -231,9 +231,9 @@ public function prepare($statement, $limit = null, $offset = null): Statement {
231231
}
232232
if (!is_null($limit)) {
233233
$platform = $this->getDatabasePlatform();
234-
$statement = $platform->modifyLimitQuery($statement, $limit, $offset);
234+
$sql = $platform->modifyLimitQuery($sql, $limit, $offset);
235235
}
236-
$statement = $this->replaceTablePrefix($statement);
236+
$statement = $this->replaceTablePrefix($sql);
237237
$statement = $this->adapter->fixupStatement($statement);
238238

239239
return parent::prepare($statement);
@@ -321,14 +321,14 @@ protected function logQueryToFile(string $sql): void {
321321
*
322322
* @param string $seqName Name of the sequence object from which the ID should be returned.
323323
*
324-
* @return string the last inserted ID.
324+
* @return int the last inserted ID.
325325
* @throws Exception
326326
*/
327-
public function lastInsertId($seqName = null) {
328-
if ($seqName) {
329-
$seqName = $this->replaceTablePrefix($seqName);
327+
public function lastInsertId($name = null): int {
328+
if ($name) {
329+
$name = $this->replaceTablePrefix($name);
330330
}
331-
return $this->adapter->lastInsertId($seqName);
331+
return $this->adapter->lastInsertId($name);
332332
}
333333

334334
/**

‎lib/private/DB/ConnectionAdapter.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public function executeStatement($sql, array $params = [], array $types = []): i
8787

8888
public function lastInsertId(string $table): int {
8989
try {
90-
return (int)$this->inner->lastInsertId($table);
90+
return $this->inner->lastInsertId($table);
9191
} catch (Exception $e) {
9292
throw DbalException::wrap($e);
9393
}

‎lib/private/DB/MigrationService.php‎

Lines changed: 20 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ class MigrationService {
5858
/**
5959
* @throws \Exception
6060
*/
61-
public function __construct($appName, Connection $connection, ?IOutput $output = null, ?AppLocator $appLocator = null) {
61+
public function __construct(string $appName, Connection $connection, ?IOutput $output = null, ?AppLocator $appLocator = null) {
6262
$this->appName = $appName;
6363
$this->connection = $connection;
6464
if ($output === null) {
@@ -100,18 +100,15 @@ public function __construct($appName, Connection $connection, ?IOutput $output =
100100

101101
/**
102102
* Returns the name of the app for which this migration is executed
103-
*
104-
* @return string
105103
*/
106-
public function getApp() {
104+
public function getApp(): string {
107105
return $this->appName;
108106
}
109107

110108
/**
111-
* @return bool
112109
* @codeCoverageIgnore - this will implicitly tested on installation
113110
*/
114-
private function createMigrationTable() {
111+
private function createMigrationTable(): bool {
115112
if ($this->migrationTableCreated) {
116113
return false;
117114
}
@@ -188,7 +185,7 @@ public function getMigratedVersions() {
188185
->where($qb->expr()->eq('app', $qb->createNamedParameter($this->getApp())))
189186
->orderBy('version');
190187

191-
$result = $qb->execute();
188+
$result = $qb->executeQuery();
192189
$rows = $result->fetchAll(\PDO::FETCH_COLUMN);
193190
$result->closeCursor();
194191

@@ -197,15 +194,17 @@ public function getMigratedVersions() {
197194

198195
/**
199196
* Returns all versions which are available in the migration folder
200-
*
201-
* @return array
197+
* @return list<string>
202198
*/
203-
public function getAvailableVersions() {
199+
public function getAvailableVersions(): array {
204200
$this->ensureMigrationsAreLoaded();
205201
return array_map('strval', array_keys($this->migrations));
206202
}
207203

208-
protected function findMigrations() {
204+
/**
205+
* @return array<string, string>
206+
*/
207+
protected function findMigrations(): array {
209208
$directory = realpath($this->migrationsPath);
210209
if ($directory === false || !file_exists($directory) || !is_dir($directory)) {
211210
return [];
@@ -322,10 +321,9 @@ public function getMigrationsDirectory() {
322321
/**
323322
* Return the explicit version for the aliases; current, next, prev, latest
324323
*
325-
* @param string $alias
326324
* @return mixed|null|string
327325
*/
328-
public function getMigration($alias) {
326+
public function getMigration(string $alias) {
329327
switch ($alias) {
330328
case 'current':
331329
return $this->getCurrentVersion();
@@ -342,29 +340,22 @@ public function getMigration($alias) {
342340
return '0';
343341
}
344342

345-
/**
346-
* @param string $version
347-
* @param int $delta
348-
* @return null|string
349-
*/
350-
private function getRelativeVersion($version, $delta) {
343+
private function getRelativeVersion(string $version, int $delta): ?string {
351344
$this->ensureMigrationsAreLoaded();
352345

353346
$versions = $this->getAvailableVersions();
354-
array_unshift($versions, 0);
347+
array_unshift($versions, '0');
348+
/** @var int $offset */
355349
$offset = array_search($version, $versions, true);
356350
if ($offset === false || !isset($versions[$offset + $delta])) {
357351
// Unknown version or delta out of bounds.
358352
return null;
359353
}
360354

361-
return (string) $versions[$offset + $delta];
355+
return (string)$versions[$offset + $delta];
362356
}
363357

364-
/**
365-
* @return string
366-
*/
367-
private function getCurrentVersion() {
358+
private function getCurrentVersion(): string {
368359
$m = $this->getMigratedVersions();
369360
if (count($m) === 0) {
370361
return '0';
@@ -374,11 +365,9 @@ private function getCurrentVersion() {
374365
}
375366

376367
/**
377-
* @param string $version
378-
* @return string
379368
* @throws \InvalidArgumentException
380369
*/
381-
private function getClass($version) {
370+
private function getClass(string $version): string {
382371
$this->ensureMigrationsAreLoaded();
383372

384373
if (isset($this->migrations[$version])) {
@@ -390,21 +379,16 @@ private function getClass($version) {
390379

391380
/**
392381
* Allows to set an IOutput implementation which is used for logging progress and messages
393-
*
394-
* @param IOutput $output
395382
*/
396-
public function setOutput(IOutput $output) {
383+
public function setOutput(IOutput $output): void {
397384
$this->output = $output;
398385
}
399386

400387
/**
401388
* Applies all not yet applied versions up to $to
402-
*
403-
* @param string $to
404-
* @param bool $schemaOnly
405389
* @throws \InvalidArgumentException
406390
*/
407-
public function migrate($to = 'latest', $schemaOnly = false) {
391+
public function migrate(string $to = 'latest', bool $schemaOnly = false): void {
408392
if ($schemaOnly) {
409393
$this->migrateSchemaOnly($to);
410394
return;
@@ -425,11 +409,9 @@ public function migrate($to = 'latest', $schemaOnly = false) {
425409

426410
/**
427411
* Applies all not yet applied versions up to $to
428-
*
429-
* @param string $to
430412
* @throws \InvalidArgumentException
431413
*/
432-
public function migrateSchemaOnly($to = 'latest') {
414+
public function migrateSchemaOnly(string $to = 'latest'): void {
433415
// read known migrations
434416
$toBeExecuted = $this->getMigrationsToExecute($to);
435417

‎lib/private/DB/OracleConnection.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
class OracleConnection extends Connection {
3030
/**
3131
* Quote the keys of the array
32+
* @param array<string, string> $data
33+
* @return array<string, string>
3234
*/
3335
private function quoteKeys(array $data) {
3436
$return = [];

‎lib/public/Contacts/IManager.php‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -107,22 +107,22 @@ public function search($pattern, $searchProperties = [], $options = []);
107107
* This function can be used to delete the contact identified by the given id
108108
*
109109
* @param int $id the unique identifier to a contact
110-
* @param string $address_book_key identifier of the address book in which the contact shall be deleted
110+
* @param string $addressBookKey identifier of the address book in which the contact shall be deleted
111111
* @return bool successful or not
112112
* @since 6.0.0
113113
*/
114-
public function delete($id, $address_book_key);
114+
public function delete($id, $addressBookKey);
115115

116116
/**
117117
* This function is used to create a new contact if 'id' is not given or not present.
118118
* Otherwise the contact will be updated by replacing the entire data set.
119119
*
120120
* @param array $properties this array if key-value-pairs defines a contact
121-
* @param string $address_book_key identifier of the address book in which the contact shall be created or updated
122-
* @return array an array representing the contact just created or updated
121+
* @param string $addressBookKey identifier of the address book in which the contact shall be created or updated
122+
* @return ?array an array representing the contact just created or updated
123123
* @since 6.0.0
124124
*/
125-
public function createOrUpdate($properties, $address_book_key);
125+
public function createOrUpdate($properties, $addressBookKey);
126126

127127
/**
128128
* Check if contacts are available (e.g. contacts app enabled)
@@ -135,20 +135,19 @@ public function isEnabled();
135135
/**
136136
* Registers an address book
137137
*
138-
* @param \OCP\IAddressBook $address_book
139138
* @return void
140139
* @since 6.0.0
141140
*/
142-
public function registerAddressBook(\OCP\IAddressBook $address_book);
141+
public function registerAddressBook(\OCP\IAddressBook $addressBook);
143142

144143
/**
145144
* Unregisters an address book
146145
*
147-
* @param \OCP\IAddressBook $address_book
146+
* @param \OCP\IAddressBook $addressBook
148147
* @return void
149148
* @since 6.0.0
150149
*/
151-
public function unregisterAddressBook(\OCP\IAddressBook $address_book);
150+
public function unregisterAddressBook(\OCP\IAddressBook $addressBook);
152151

153152
/**
154153
* In order to improve lazy loading a closure can be registered which will be called in case

0 commit comments

Comments
 (0)