Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup psalm issues in DB/ContactsManager and Console #35539

Merged
merged 3 commits into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 5 additions & 12 deletions lib/private/Console/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,12 @@
use Symfony\Component\Console\Output\OutputInterface;

class Application {
/** @var IConfig */
private $config;
private IConfig $config;
private SymfonyApplication $application;
/** @var IEventDispatcher */
private $dispatcher;
/** @var IRequest */
private $request;
/** @var LoggerInterface */
private $logger;
/** @var MemoryInfo */
private $memoryInfo;
private IEventDispatcher $dispatcher;
private IRequest $request;
private LoggerInterface $logger;
private MemoryInfo $memoryInfo;

public function __construct(IConfig $config,
IEventDispatcher $dispatcher,
Expand All @@ -74,8 +69,6 @@ public function __construct(IConfig $config,
}

/**
* @param InputInterface $input
* @param ConsoleOutputInterface $output
* @throws \Exception
*/
public function loadCommands(
Expand Down
25 changes: 11 additions & 14 deletions lib/private/ContactsManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,32 +91,32 @@ public function search($pattern, $searchProperties = [], $options = []) {
* This function can be used to delete the contact identified by the given id
*
* @param int $id the unique identifier to a contact
* @param string $address_book_key identifier of the address book in which the contact shall be deleted
* @param string $addressBookKey identifier of the address book in which the contact shall be deleted
* @return bool successful or not
*/
public function delete($id, $address_book_key) {
$addressBook = $this->getAddressBook($address_book_key);
public function delete($id, $addressBookKey) {
$addressBook = $this->getAddressBook($addressBookKey);
if (!$addressBook) {
return null;
return false;
}

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

return null;
return false;
}

/**
* This function is used to create a new contact if 'id' is not given or not present.
* Otherwise the contact will be updated by replacing the entire data set.
*
* @param array $properties this array if key-value-pairs defines a contact
* @param string $address_book_key identifier of the address book in which the contact shall be created or updated
* @return array representing the contact just created or updated
* @param string $addressBookKey identifier of the address book in which the contact shall be created or updated
* @return ?array representing the contact just created or updated
*/
public function createOrUpdate($properties, $address_book_key) {
$addressBook = $this->getAddressBook($address_book_key);
public function createOrUpdate($properties, $addressBookKey) {
$addressBook = $this->getAddressBook($addressBookKey);
if (!$addressBook) {
return null;
}
Expand All @@ -133,7 +133,7 @@ public function createOrUpdate($properties, $address_book_key) {
*
* @return bool true if enabled, false if not
*/
public function isEnabled() {
public function isEnabled(): bool {
return !empty($this->addressBooks) || !empty($this->addressBookLoaders);
}

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

/**
* Get (and load when needed) the address book for $key
*
* @param string $addressBookKey
* @return IAddressBook
*/
protected function getAddressBook($addressBookKey) {
protected function getAddressBook(string $addressBookKey): ?IAddressBook {
$this->loadAddressBooks();
if (!array_key_exists($addressBookKey, $this->addressBooks)) {
return null;
Expand Down
10 changes: 7 additions & 3 deletions lib/private/DB/Adapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
use OC\DB\Exceptions\DbalException;

/**
* This handles the way we use to write queries, into something that can be
Expand Down Expand Up @@ -142,9 +143,12 @@ public function insertIgnoreConflict(string $table, array $values) : int {
foreach ($values as $key => $value) {
$builder->setValue($key, $builder->createNamedParameter($value));
}
return $builder->execute();
} catch (UniqueConstraintViolationException $e) {
return 0;
return $builder->executeStatement();
} catch (DbalException $e) {
if ($e->getReason() === \OCP\DB\Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
return 0;
}
throw $e;
}
}
}
16 changes: 8 additions & 8 deletions lib/private/DB/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ public function getPrefix() {
* @return Statement The prepared statement.
* @throws Exception
*/
public function prepare($statement, $limit = null, $offset = null): Statement {
public function prepare($sql, $limit = null, $offset = null): Statement {
if ($limit === -1 || $limit === null) {
$limit = null;
} else {
Expand All @@ -231,9 +231,9 @@ public function prepare($statement, $limit = null, $offset = null): Statement {
}
if (!is_null($limit)) {
$platform = $this->getDatabasePlatform();
$statement = $platform->modifyLimitQuery($statement, $limit, $offset);
$sql = $platform->modifyLimitQuery($sql, $limit, $offset);
}
$statement = $this->replaceTablePrefix($statement);
$statement = $this->replaceTablePrefix($sql);
$statement = $this->adapter->fixupStatement($statement);

return parent::prepare($statement);
Expand Down Expand Up @@ -321,14 +321,14 @@ protected function logQueryToFile(string $sql): void {
*
* @param string $seqName Name of the sequence object from which the ID should be returned.
*
* @return string the last inserted ID.
* @return int the last inserted ID.
* @throws Exception
*/
public function lastInsertId($seqName = null) {
if ($seqName) {
$seqName = $this->replaceTablePrefix($seqName);
public function lastInsertId($name = null): int {
if ($name) {
$name = $this->replaceTablePrefix($name);
}
return $this->adapter->lastInsertId($seqName);
return $this->adapter->lastInsertId($name);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/private/DB/ConnectionAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public function executeStatement($sql, array $params = [], array $types = []): i

public function lastInsertId(string $table): int {
try {
return (int)$this->inner->lastInsertId($table);
return $this->inner->lastInsertId($table);
} catch (Exception $e) {
throw DbalException::wrap($e);
}
Expand Down
58 changes: 20 additions & 38 deletions lib/private/DB/MigrationService.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class MigrationService {
/**
* @throws \Exception
*/
public function __construct($appName, Connection $connection, ?IOutput $output = null, ?AppLocator $appLocator = null) {
public function __construct(string $appName, Connection $connection, ?IOutput $output = null, ?AppLocator $appLocator = null) {
$this->appName = $appName;
$this->connection = $connection;
if ($output === null) {
Expand Down Expand Up @@ -100,18 +100,15 @@ public function __construct($appName, Connection $connection, ?IOutput $output =

/**
* Returns the name of the app for which this migration is executed
*
* @return string
*/
public function getApp() {
public function getApp(): string {
return $this->appName;
}

/**
* @return bool
* @codeCoverageIgnore - this will implicitly tested on installation
*/
private function createMigrationTable() {
private function createMigrationTable(): bool {
if ($this->migrationTableCreated) {
return false;
}
Expand Down Expand Up @@ -188,7 +185,7 @@ public function getMigratedVersions() {
->where($qb->expr()->eq('app', $qb->createNamedParameter($this->getApp())))
->orderBy('version');

$result = $qb->execute();
$result = $qb->executeQuery();
$rows = $result->fetchAll(\PDO::FETCH_COLUMN);
$result->closeCursor();

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

/**
* Returns all versions which are available in the migration folder
*
* @return array
* @return list<string>
*/
public function getAvailableVersions() {
public function getAvailableVersions(): array {
$this->ensureMigrationsAreLoaded();
return array_map('strval', array_keys($this->migrations));
}

protected function findMigrations() {
/**
* @return array<string, string>
*/
protected function findMigrations(): array {
$directory = realpath($this->migrationsPath);
if ($directory === false || !file_exists($directory) || !is_dir($directory)) {
return [];
Expand Down Expand Up @@ -322,10 +321,9 @@ public function getMigrationsDirectory() {
/**
* Return the explicit version for the aliases; current, next, prev, latest
*
* @param string $alias
* @return mixed|null|string
*/
public function getMigration($alias) {
public function getMigration(string $alias) {
switch ($alias) {
case 'current':
return $this->getCurrentVersion();
Expand All @@ -342,29 +340,22 @@ public function getMigration($alias) {
return '0';
}

/**
* @param string $version
* @param int $delta
* @return null|string
*/
private function getRelativeVersion($version, $delta) {
private function getRelativeVersion(string $version, int $delta): ?string {
$this->ensureMigrationsAreLoaded();

$versions = $this->getAvailableVersions();
array_unshift($versions, 0);
array_unshift($versions, '0');
/** @var int $offset */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this @var? Doesn’t that contradict the comparison with false below?

$offset = array_search($version, $versions, true);
if ($offset === false || !isset($versions[$offset + $delta])) {
// Unknown version or delta out of bounds.
return null;
}

return (string) $versions[$offset + $delta];
return (string)$versions[$offset + $delta];
}

/**
* @return string
*/
private function getCurrentVersion() {
private function getCurrentVersion(): string {
$m = $this->getMigratedVersions();
if (count($m) === 0) {
return '0';
Expand All @@ -374,11 +365,9 @@ private function getCurrentVersion() {
}

/**
* @param string $version
* @return string
* @throws \InvalidArgumentException
*/
private function getClass($version) {
private function getClass(string $version): string {
$this->ensureMigrationsAreLoaded();

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

/**
* Allows to set an IOutput implementation which is used for logging progress and messages
*
* @param IOutput $output
*/
public function setOutput(IOutput $output) {
public function setOutput(IOutput $output): void {
$this->output = $output;
}

/**
* Applies all not yet applied versions up to $to
*
* @param string $to
* @param bool $schemaOnly
* @throws \InvalidArgumentException
*/
public function migrate($to = 'latest', $schemaOnly = false) {
public function migrate(string $to = 'latest', bool $schemaOnly = false): void {
if ($schemaOnly) {
$this->migrateSchemaOnly($to);
return;
Expand All @@ -425,11 +409,9 @@ public function migrate($to = 'latest', $schemaOnly = false) {

/**
* Applies all not yet applied versions up to $to
*
* @param string $to
* @throws \InvalidArgumentException
*/
public function migrateSchemaOnly($to = 'latest') {
public function migrateSchemaOnly(string $to = 'latest'): void {
// read known migrations
$toBeExecuted = $this->getMigrationsToExecute($to);

Expand Down
2 changes: 2 additions & 0 deletions lib/private/DB/OracleConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
class OracleConnection extends Connection {
/**
* Quote the keys of the array
* @param array<string, string> $data
* @return array<string, string>
*/
private function quoteKeys(array $data) {
$return = [];
Expand Down
17 changes: 8 additions & 9 deletions lib/public/Contacts/IManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,22 +107,22 @@ public function search($pattern, $searchProperties = [], $options = []);
* This function can be used to delete the contact identified by the given id
*
* @param int $id the unique identifier to a contact
* @param string $address_book_key identifier of the address book in which the contact shall be deleted
* @param string $addressBookKey identifier of the address book in which the contact shall be deleted
* @return bool successful or not
* @since 6.0.0
*/
public function delete($id, $address_book_key);
public function delete($id, $addressBookKey);

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

/**
* Check if contacts are available (e.g. contacts app enabled)
Expand All @@ -135,20 +135,19 @@ public function isEnabled();
/**
* Registers an address book
*
* @param \OCP\IAddressBook $address_book
* @return void
* @since 6.0.0
*/
public function registerAddressBook(\OCP\IAddressBook $address_book);
public function registerAddressBook(\OCP\IAddressBook $addressBook);

/**
* Unregisters an address book
*
* @param \OCP\IAddressBook $address_book
* @param \OCP\IAddressBook $addressBook
* @return void
* @since 6.0.0
*/
public function unregisterAddressBook(\OCP\IAddressBook $address_book);
public function unregisterAddressBook(\OCP\IAddressBook $addressBook);

/**
* In order to improve lazy loading a closure can be registered which will be called in case
Expand Down
Loading
Loading