From 8c7ee94d7f40ea31f2855aa2ac8e6a13f463e127 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 8 Dec 2023 14:19:01 +0100 Subject: [PATCH 01/12] fix(OCP): Migrate to OCP\DB\Types Signed-off-by: Joas Schilling --- lib/Migration/Version2011Date20210930134607.php | 2 +- psalm.xml | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Migration/Version2011Date20210930134607.php b/lib/Migration/Version2011Date20210930134607.php index 98833e720..eeac19639 100644 --- a/lib/Migration/Version2011Date20210930134607.php +++ b/lib/Migration/Version2011Date20210930134607.php @@ -27,8 +27,8 @@ namespace OCA\Notifications\Migration; use Closure; -use Doctrine\DBAL\Types\Types; use OCP\DB\ISchemaWrapper; +use OCP\DB\Types; use OCP\Migration\IOutput; use OCP\Migration\SimpleMigrationStep; diff --git a/psalm.xml b/psalm.xml index 0118c4046..14ac30ab7 100644 --- a/psalm.xml +++ b/psalm.xml @@ -29,7 +29,6 @@ - From d8ecfabcdeccff231b235534f1ab9d9f50728fb5 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 8 Dec 2023 14:19:32 +0100 Subject: [PATCH 02/12] fix(CI): Update scripts and fix versions Signed-off-by: Joas Schilling --- composer.json | 4 +++- psalm.xml | 1 + tests/psalm-baseline.xml | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index b21ad34cf..f8ca4f74d 100644 --- a/composer.json +++ b/composer.json @@ -37,8 +37,10 @@ "lint": "find . -name \\*.php -not -path './vendor/*' -not -path './build/*' -print0 | xargs -0 -n1 php -l", "cs:check": "php-cs-fixer fix --dry-run --diff", "cs:fix": "php-cs-fixer fix", + "openapi": "generate-spec", "psalm": "psalm --threads=1", - "psalm:update-baseline": "psalm --threads=1 --update-baseline --set-baseline=tests/psalm-baseline.xml", + "psalm:dev": "psalm --no-cache --threads=$(nproc)", + "psalm:update-baseline": "psalm --threads=1 --update-baseline", "psalm:clear": "psalm --clear-cache && psalm --clear-global-cache", "psalm:fix": "psalm --alter --issues=InvalidReturnType,InvalidNullableReturnType,MissingParamType,InvalidFalsableReturnType", "test:unit": "vendor/bin/phpunit --color -c tests/Unit/phpunit.xml", diff --git a/psalm.xml b/psalm.xml index 14ac30ab7..8680aa534 100644 --- a/psalm.xml +++ b/psalm.xml @@ -5,6 +5,7 @@ findUnusedBaselineEntry="true" findUnusedCode="false" resolveFromConfigFile="true" + phpVersion="8.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="https://getpsalm.org/schema/config" xsi:schemaLocation="https://getpsalm.org/schema/config vendor-bin/psalm/vendor/vimeo/psalm/config.xsd" diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 882b2a335..3c35c644e 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -1,5 +1,5 @@ - + IProvider From 89f28d6d5f0169026ac646a35c39cf709a35761c Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 8 Dec 2023 14:28:56 +0100 Subject: [PATCH 03/12] fix(CI): Remove unmaintained library Signed-off-by: Joas Schilling --- tests/Integration/composer.json | 1 - tests/Integration/composer.lock | 45 +----------------------------- tests/Integration/config/behat.yml | 11 ++++---- 3 files changed, 7 insertions(+), 50 deletions(-) diff --git a/tests/Integration/composer.json b/tests/Integration/composer.json index ac8fe0e09..bb4167100 100644 --- a/tests/Integration/composer.json +++ b/tests/Integration/composer.json @@ -8,7 +8,6 @@ "require-dev": { "behat/behat": "^3.12", "guzzlehttp/guzzle": "^7.5", - "jarnaiz/behat-junit-formatter": "^1.3", "phpunit/phpunit": "^9.6" } } diff --git a/tests/Integration/composer.lock b/tests/Integration/composer.lock index 9b70201b8..d6f94c146 100644 --- a/tests/Integration/composer.lock +++ b/tests/Integration/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "8aea43a4af45069a70b15339a976f9eb", + "content-hash": "a75ac6c5e2ccd633528be3daf92531d5", "packages": [], "packages-dev": [ { @@ -601,49 +601,6 @@ ], "time": "2023-08-27T10:13:57+00:00" }, - { - "name": "jarnaiz/behat-junit-formatter", - "version": "v1.3.2", - "source": { - "type": "git", - "url": "https://github.com/j-arnaiz/behat-junit-formatter.git", - "reference": "2f80b3881e04d3cf43e05ab821c0e80675a9846d" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/j-arnaiz/behat-junit-formatter/zipball/2f80b3881e04d3cf43e05ab821c0e80675a9846d", - "reference": "2f80b3881e04d3cf43e05ab821c0e80675a9846d", - "shasum": "" - }, - "require": { - "behat/behat": "~3.0", - "php": ">=5.3.0" - }, - "type": "library", - "extra": { - "branch-alias": { - "dev-master": "v1.0.x-dev" - } - }, - "autoload": { - "psr-4": { - "jarnaiz\\JUnitFormatter\\": "src/" - } - }, - "notification-url": "https://packagist.org/downloads/", - "authors": [ - { - "name": "Jesús Arnaiz", - "email": "j.arnaiz@gmail.com" - } - ], - "description": "Behat 3 JUnit xml formatter", - "support": { - "issues": "https://github.com/j-arnaiz/behat-junit-formatter/issues", - "source": "https://github.com/j-arnaiz/behat-junit-formatter/tree/master" - }, - "time": "2016-01-26T17:05:07+00:00" - }, { "name": "myclabs/deep-copy", "version": "1.11.1", diff --git a/tests/Integration/config/behat.yml b/tests/Integration/config/behat.yml index 425da1b76..d9aa6838a 100644 --- a/tests/Integration/config/behat.yml +++ b/tests/Integration/config/behat.yml @@ -1,4 +1,10 @@ default: + formatters: + junit: + output_path: '%paths.base%/../output' + pretty: + output_styles: + comment: [ 'bright-blue' ] autoload: '': '%paths.base%/../features/bootstrap' suites: @@ -7,8 +13,3 @@ default: - '%paths.base%/../features' contexts: - FeatureContext - - extensions: - jarnaiz\JUnitFormatter\JUnitFormatterExtension: - filename: report.xml - outputDir: '%paths.base%/../output/' From e2d06e2eaa88ea373dc5fd338289f83d2061c0d7 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 8 Dec 2023 14:51:33 +0100 Subject: [PATCH 04/12] fix(API): Move to public API for Authentication where possible Signed-off-by: Joas Schilling --- lib/AppInfo/Application.php | 6 ------ lib/Controller/PushController.php | 4 ++-- tests/psalm-baseline.xml | 12 ------------ 3 files changed, 2 insertions(+), 20 deletions(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 21a46a439..20b11986e 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -25,7 +25,6 @@ namespace OCA\Notifications\AppInfo; -use OC\Authentication\Token\IProvider; use OCA\Notifications\App; use OCA\Notifications\Capabilities; use OCA\Notifications\Listener\BeforeTemplateRenderedListener; @@ -38,7 +37,6 @@ use OCP\AppFramework\Bootstrap\IBootstrap; use OCP\AppFramework\Bootstrap\IRegistrationContext; use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent; -use OCP\AppFramework\IAppContainer; use OCP\Notification\IManager; use OCP\User\Events\PostLoginEvent; use OCP\User\Events\UserCreatedEvent; @@ -54,10 +52,6 @@ public function __construct() { public function register(IRegistrationContext $context): void { $context->registerCapability(Capabilities::class); - $context->registerService(IProvider::class, function (IAppContainer $c) { - return $c->getServer()->get(IProvider::class); - }); - $context->registerSetupCheck(SetupWarningOnRateLimitReached::class); $context->registerNotifierService(AdminNotifications::class); diff --git a/lib/Controller/PushController.php b/lib/Controller/PushController.php index e00d6abe6..33824a1ff 100644 --- a/lib/Controller/PushController.php +++ b/lib/Controller/PushController.php @@ -26,14 +26,14 @@ namespace OCA\Notifications\Controller; -use OC\Authentication\Exceptions\InvalidTokenException; use OC\Authentication\Token\IProvider; -use OC\Authentication\Token\IToken; use OC\Security\IdentityProof\Manager; use OCA\Notifications\ResponseDefinitions; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCSController; +use OCP\Authentication\Exceptions\InvalidTokenException; +use OCP\Authentication\Token\IToken; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; use OCP\IRequest; diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 3c35c644e..b75974c1e 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -1,20 +1,8 @@ - - - IProvider - IProvider - - IProvider - IToken - IToken - IToken - IToken - InvalidTokenException - InvalidTokenException Manager From 5479d707185c8c310296dac7e0b99435e8db7eed Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 8 Dec 2023 14:52:58 +0100 Subject: [PATCH 05/12] fix(API): Use typed execute*() methods Signed-off-by: Joas Schilling --- lib/Controller/PushController.php | 2 +- lib/Migration/Version2010Date20210218082811.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Controller/PushController.php b/lib/Controller/PushController.php index 33824a1ff..0126af5f4 100644 --- a/lib/Controller/PushController.php +++ b/lib/Controller/PushController.php @@ -208,7 +208,7 @@ protected function savePushToken(IUser $user, IToken $token, string $deviceIdent ->from('notifications_pushhash') ->where($query->expr()->eq('uid', $query->createNamedParameter($user->getUID()))) ->andWhere($query->expr()->eq('token', $query->createNamedParameter($token->getId()))); - $result = $query->execute(); + $result = $query->executeQuery(); $row = $result->fetch(); $result->closeCursor(); diff --git a/lib/Migration/Version2010Date20210218082811.php b/lib/Migration/Version2010Date20210218082811.php index e4ce0090c..6708653a6 100644 --- a/lib/Migration/Version2010Date20210218082811.php +++ b/lib/Migration/Version2010Date20210218082811.php @@ -128,7 +128,7 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $query = $this->connection->getQueryBuilder(); $query->select('*') ->from('notifications_pushtokens'); - $result = $query->execute(); + $result = $query->executeQuery(); while ($row = $result->fetch()) { $insert @@ -142,7 +142,7 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array ->setParameter('apptype', $row['apptype']) ; - $insert->execute(); + $insert->executeStatement(); } $result->closeCursor(); } From cfaea0db9773454a480bed9e9e56ff1838945409 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 8 Dec 2023 14:55:12 +0100 Subject: [PATCH 06/12] fix(listener): Help psalm understand that we always have a user Signed-off-by: Joas Schilling --- lib/Listener/BeforeTemplateRenderedListener.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/Listener/BeforeTemplateRenderedListener.php b/lib/Listener/BeforeTemplateRenderedListener.php index 1527c0316..e33bc5404 100644 --- a/lib/Listener/BeforeTemplateRenderedListener.php +++ b/lib/Listener/BeforeTemplateRenderedListener.php @@ -67,14 +67,15 @@ public function handle(Event $event): void { return; } - if (!$this->userSession->getUser() instanceof IUser) { + $user = $this->userSession->getUser(); + if (!$user instanceof IUser) { return; } $this->initialState->provideInitialState( 'sound_notification', $this->config->getUserValue( - $this->userSession->getUser()->getUID(), + $user->getUID(), Application::APP_ID, 'sound_notification', 'yes' @@ -84,7 +85,7 @@ public function handle(Event $event): void { $this->initialState->provideInitialState( 'sound_talk', $this->config->getUserValue( - $this->userSession->getUser()->getUID(), + $user->getUID(), Application::APP_ID, 'sound_talk', 'yes' From 282391d5f6a941f6617187042b864cdcde119642 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 8 Dec 2023 15:02:21 +0100 Subject: [PATCH 07/12] fix(docs): Fix returned notification list type Signed-off-by: Joas Schilling --- lib/Handler.php | 2 +- lib/MailNotifications.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Handler.php b/lib/Handler.php index a36a74b22..3753694dd 100644 --- a/lib/Handler.php +++ b/lib/Handler.php @@ -249,7 +249,7 @@ public function confirmIdsForUser(string $user, array $ids): array { * @param int $startAfterId * @param string $userId * @param int $limit - * @return array [notification_id => INotification] + * @return array [notification_id => INotification] */ public function getAfterId(int $startAfterId, string $userId, int $limit = 25): array { $sql = $this->connection->getQueryBuilder(); diff --git a/lib/MailNotifications.php b/lib/MailNotifications.php index ef7ff2488..e61f21389 100644 --- a/lib/MailNotifications.php +++ b/lib/MailNotifications.php @@ -160,7 +160,7 @@ public function sendEmails(int $batchSize, int $sendTime): void { $languageCode = $userLanguages[$settings->getUserId()] ?? $fallbackLang; $timezone = $userTimezones[$settings->getUserId()] ?? $fallbackTimeZone; - /** @var INotification[] $notifications */ + /** @var array $notifications */ $notifications = $this->handler->getAfterId($settings->getLastSendId(), $settings->getUserId()); if (!empty($notifications)) { $oldestNotification = end($notifications); @@ -183,10 +183,10 @@ public function sendEmails(int $batchSize, int $sendTime): void { } /** - * send an email to the user containing given list of notifications + * Send an email to the user containing given list of notifications * * @param Settings $settings - * @param INotification[] $notifications + * @param non-empty-array $notifications * @param string $language * @param string $timezone */ From 561d30cabe882abd1c300546913d725825283e3a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 8 Dec 2023 15:06:42 +0100 Subject: [PATCH 08/12] fix(mail): Make sure the timezone is not empty Signed-off-by: Joas Schilling --- lib/MailNotifications.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/MailNotifications.php b/lib/MailNotifications.php index e61f21389..eaf576418 100644 --- a/lib/MailNotifications.php +++ b/lib/MailNotifications.php @@ -131,6 +131,7 @@ public function sendEmails(int $batchSize, int $sendTime): void { // Batch-read settings $fallbackTimeZone = date_default_timezone_get(); + /** @psalm-var array $userTimezones */ $userTimezones = $this->config->getUserValueForUsers('core', 'timezone', $userIds); $userEnabled = $this->config->getUserValueForUsers('core', 'enabled', $userIds); @@ -280,7 +281,13 @@ protected function prepareEmailMessage(string $uid, array $notifications, string foreach ($notifications as $notification) { try { - $relativeDateTime = $this->dateFormatter->formatDateTimeRelativeDay($notification->getDateTime(), 'long', 'short', new \DateTimeZone($timezone), $l10n); + $relativeDateTime = $this->dateFormatter->formatDateTimeRelativeDay( + $notification->getDateTime(), + 'long', + 'short', + new \DateTimeZone($timezone ?: 'UTC'), + $l10n + ); $template->addBodyListItem($this->getHTMLContents($notification), $relativeDateTime, $notification->getIcon(), $notification->getParsedSubject()); // Buttons probably were not intended for this, but it works ok enough for showing the idea. From 02b51406c6828509280383012d71a201edfe7faa Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 8 Dec 2023 15:12:37 +0100 Subject: [PATCH 09/12] fix(push): Fix some type checks Signed-off-by: Joas Schilling --- lib/Push.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/Push.php b/lib/Push.php index b34984b45..bae3c4952 100644 --- a/lib/Push.php +++ b/lib/Push.php @@ -280,7 +280,9 @@ public function pushToDevice(int $id, INotification $notification, ?OutputInterf if (isset($this->userStatuses[$notification->getUser()])) { $userStatus = $this->userStatuses[$notification->getUser()]; - if ($userStatus->getStatus() === IUserStatus::DND && empty($this->allowedDNDPushList[$notification->getApp()])) { + if ($userStatus instanceof IUserStatus + && $userStatus->getStatus() === IUserStatus::DND + && empty($this->allowedDNDPushList[$notification->getApp()])) { $this->printInfo('User status is set to DND - no push notifications will be sent'); return; } @@ -498,14 +500,14 @@ protected function sendNotificationsToProxies(): void { $response = $client->post($proxyServer . '/notifications', $requestData); $status = $response->getStatusCode(); - $body = $response->getBody(); - $bodyData = json_decode($body, true); + $body = (string) $response->getBody(); + $bodyData = json_decode($body, true, flags: JSON_THROW_ON_ERROR); } catch (ClientException $e) { // Server responded with 4xx (400 Bad Request mostlikely) $response = $e->getResponse(); $status = $response->getStatusCode(); $body = $response->getBody()->getContents(); - $bodyData = json_decode($body, true); + $bodyData = json_decode($body, true, flags: JSON_THROW_ON_ERROR); } catch (ServerException $e) { // Server responded with 5xx $response = $e->getResponse(); From e33e5c3be07fe6c430b405ec81b7418c6a2d74c6 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 8 Dec 2023 15:12:57 +0100 Subject: [PATCH 10/12] fix(CI): Bump psalm error level to 3 Signed-off-by: Joas Schilling --- psalm.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/psalm.xml b/psalm.xml index 8680aa534..adfc77193 100644 --- a/psalm.xml +++ b/psalm.xml @@ -1,7 +1,7 @@ Date: Fri, 8 Dec 2023 15:19:08 +0100 Subject: [PATCH 11/12] fix(CI): Fix path to psalm config.xsd Signed-off-by: Joas Schilling --- psalm.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/psalm.xml b/psalm.xml index adfc77193..b10ab5db5 100644 --- a/psalm.xml +++ b/psalm.xml @@ -8,7 +8,7 @@ phpVersion="8.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="https://getpsalm.org/schema/config" - xsi:schemaLocation="https://getpsalm.org/schema/config vendor-bin/psalm/vendor/vimeo/psalm/config.xsd" + xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd" > From 91f911c5b2101b237b635a6cf9607b8dfa7762ac Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 8 Dec 2023 17:00:22 +0100 Subject: [PATCH 12/12] fix(tests): Handle JSON exceptions Signed-off-by: Joas Schilling --- lib/Push.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/Push.php b/lib/Push.php index bae3c4952..a0c323f65 100644 --- a/lib/Push.php +++ b/lib/Push.php @@ -501,13 +501,21 @@ protected function sendNotificationsToProxies(): void { $response = $client->post($proxyServer . '/notifications', $requestData); $status = $response->getStatusCode(); $body = (string) $response->getBody(); - $bodyData = json_decode($body, true, flags: JSON_THROW_ON_ERROR); + try { + $bodyData = json_decode($body, true); + } catch (\JsonException $e) { + $bodyData = null; + } } catch (ClientException $e) { // Server responded with 4xx (400 Bad Request mostlikely) $response = $e->getResponse(); $status = $response->getStatusCode(); $body = $response->getBody()->getContents(); - $bodyData = json_decode($body, true, flags: JSON_THROW_ON_ERROR); + try { + $bodyData = json_decode($body, true); + } catch (\JsonException $e) { + $bodyData = null; + } } catch (ServerException $e) { // Server responded with 5xx $response = $e->getResponse();