From bd9249b9920eaa845cc1dd661d81e540b24b383c Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Fri, 15 May 2026 09:47:34 +0200 Subject: [PATCH 01/10] chore(.gitignore): add .vscode to ignore list --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index d3d331b..9165828 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ /tests/.phpunit.cache /coverage/ +/.vscode/ From 3a829afa1eaffc30f74dc21bbbd7c1f3401d689e Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Fri, 15 May 2026 09:41:36 +0200 Subject: [PATCH 02/10] docs(post-setup): add event-flow doc for welcome mail Introduces a `docs/` tree describing the app's event flows for a public audience. This first entry covers the existing post-setup flow: InstallationCompletedEvent -> PostSetupJob -> welcome mail. The diagram documents the TIME_SENSITIVE retry loop, the `post_install` status state machine, and the configurable retry interval. A `docs/README.md` index lists the flow; further flows will be appended to it as separate docs. --- REUSE.toml | 4 +- docs/README.md | 5 ++ docs/events/post-setup.md | 100 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 docs/README.md create mode 100644 docs/events/post-setup.md diff --git a/REUSE.toml b/REUSE.toml index 9f6d292..3540d4b 100644 --- a/REUSE.toml +++ b/REUSE.toml @@ -41,7 +41,9 @@ SPDX-License-Identifier = "AGPL-3.0-or-later" path = [ "README.md", "CHANGELOG.md", - "CODE_OF_CONDUCT.md" + "CODE_OF_CONDUCT.md", + "docs/README.md", + "docs/events/post-setup.md" ] precedence = "aggregate" SPDX-FileCopyrightText = "2026 STRATO GmbH" diff --git a/docs/README.md b/docs/README.md new file mode 100644 index 0000000..703ce75 --- /dev/null +++ b/docs/README.md @@ -0,0 +1,5 @@ +# Event Flows + +NCW Tools reacts to Nextcloud lifecycle events by registering listeners that enqueue background jobs. Each document below walks through one end-to-end flow — the triggering event, the listener's synchronous work, the job's asynchronous behaviour, and the configuration it relies on. + +- [Post-setup welcome mail](events/post-setup.md) — sends the initial welcome email to the admin user once the system is reachable after installation. diff --git a/docs/events/post-setup.md b/docs/events/post-setup.md new file mode 100644 index 0000000..ecc7db1 --- /dev/null +++ b/docs/events/post-setup.md @@ -0,0 +1,100 @@ +# Post-setup welcome mail + +After Nextcloud installation completes, this app sends an initial welcome email to the admin user. A listener seeds an app-config status flag and schedules a time-sensitive background job; the job retries on a configurable interval until the system URL is reachable and the admin user exists, then sends the welcome mail via Nextcloud's `NewUserMailHelper` (with a fresh password-reset token) and marks itself done. + +## Trigger event + +`OCP\Install\Events\InstallationCompletedEvent` + +## Configuration + +| Key | Where | Type | Default | Purpose | +| --- | --- | --- | --- | --- | +| `overwrite.cli.url` | `config/config.php` | string | _required_ | Base URL the job probes via `…/status.php` to confirm the instance is reachable. | +| `ncw_tools.post_setup_job.retry_interval` | `config/config.php` | int (seconds) | `2` | Interval between job retries while waiting for the system to become ready. | +| `post_install` | App config (`ncw_tools`) | string | _set by listener_ | Observable status: `INIT` (work pending), `DONE` (welcome mail sent), `UNKNOWN` (listener has not run). | + +## Flow + +```mermaid +sequenceDiagram + autonumber + + participant INS as Nextcloud Installer + participant ED as EventDispatcher + participant ICEL as InstallationCompletedEventListener + participant CFG as IAppConfig + participant LOG as LoggerInterface + participant JL as IJobList + participant CRON as Nextcloud Cron + participant PSJ as PostSetupJob + participant HTTP as IClientService + participant UM as IUserManager + participant MAIL as WelcomeMailHelper + + Note over INS,JL: Synchronous phase — runs during installation + + INS->>ED: dispatch(InstallationCompletedEvent) + ED->>ICEL: handle(event) + ICEL->>CFG: setValueString(post_install, "INIT") + + alt admin username missing + ICEL->>LOG: warning("No admin user provided") + else admin username present + ICEL->>LOG: info("Scheduling welcome email job") + ICEL->>JL: add(PostSetupJob::class, adminUserId) + end + + Note over CRON,MAIL: Asynchronous phase — TIME_SENSITIVE,
retries every ncw_tools.post_setup_job.retry_interval seconds (default 2) + + CRON->>PSJ: run(adminUserId) + PSJ->>CFG: getValueString(post_install) + + alt status == "DONE" + PSJ->>JL: remove(this) + PSJ-->>CRON: return (already completed) + else status == "UNKNOWN" + PSJ->>LOG: warning("Job status unknown, waiting") + PSJ-->>CRON: return (retry) + else status == "INIT" + PSJ->>HTTP: GET {overwrite.cli.url}/status.php + + alt URL empty + PSJ->>LOG: warning("System URL not configured") + PSJ-->>CRON: return (retry) + else HTTP not 2xx + PSJ->>LOG: info("System not ready, will retry") + PSJ-->>CRON: return (retry) + else HTTP 2xx + PSJ->>UM: userExists(adminUserId) + + alt user not found + PSJ->>LOG: warning("Admin user not found") + PSJ-->>CRON: return (retry) + else user exists + PSJ->>UM: get(adminUserId) + PSJ->>MAIL: sendWelcomeMail(user, generateResetToken=true) + + alt exception thrown + PSJ->>LOG: error("Failed to send welcome email, will retry") + PSJ-->>CRON: return (retry) + else success + PSJ->>CFG: setValueString(post_install, "DONE") + PSJ->>JL: remove(this) + PSJ->>LOG: info("Post-installation job completed") + end + end + end + end +``` + +## Failure modes + +The job is `TIME_SENSITIVE` and re-runs on every cron tick until it succeeds. Any of the following keep it in the queue: + +- `overwrite.cli.url` is unset. +- `…/status.php` returns a non-2xx response, or the request throws. +- The admin user does not exist (or cannot be retrieved). +- `WelcomeMailHelper::sendWelcomeMail` throws. + +On success, the job sets `post_install = "DONE"` and removes itself from the job list. A subsequent install event would re-seed the status to `INIT` and re-schedule the job. From c2b4a265526762101b589c4cb58b67f9d9553443 Mon Sep 17 00:00:00 2001 From: Arsalan Ul Haq Sohni Date: Tue, 5 May 2026 17:53:45 +0200 Subject: [PATCH 03/10] feat(user-stats): wire UserStatsJob to send stats to PSS via API client Replaces the log-only payload with an actual HTTP PUT call to the NextcloudPSS service using the ionos-ncpss-addons-api-client. Follows the same factory + config-service pattern as the mail app's IONOS integration (ApiStatsClientService, PssConfigService). Brand, extRef, and PSS credentials are read from system config. The log statement is kept alongside the API call for observability. Errors are caught and logged without retrying. Ref-Id: NSW-878 --- composer.json | 29 +++++++ lib/AppInfo/Application.php | 4 + lib/BackgroundJob/UserStatsJob.php | 42 +++++++++- lib/Service/ApiStatsClientService.php | 33 ++++++++ lib/Service/PssConfigService.php | 62 +++++++++++++++ tests/unit/BackgroundJob/UserStatsJobTest.php | 79 +++++++++++++++++-- 6 files changed, 240 insertions(+), 9 deletions(-) create mode 100644 lib/Service/ApiStatsClientService.php create mode 100644 lib/Service/PssConfigService.php diff --git a/composer.json b/composer.json index 07d4ed9..c9c96d5 100644 --- a/composer.json +++ b/composer.json @@ -9,6 +9,34 @@ "homepage": "https://example.com" } ], + "repositories": [ + { + "type": "package", + "package": { + "name": "ionos-productivity/ionos-ncpss-addons-api-client", + "version": "2.0.0", + "source": { + "type": "git", + "url": "https://github.com/ionos-productivity/ionos-ncpss-addons-api-client.git", + "reference": "2.0.0-20260429080405" + }, + "dist": { + "type": "zip", + "url": "https://github.com/ionos-productivity/ionos-ncpss-addons-api-client/archive/refs/tags/2.0.0-20260429080405.zip", + "reference": "2.0.0-20260429080405" + }, + "autoload": { + "psr-4": { + "IONOS\\NextcloudPSS\\AddonsAPI\\Client\\": "lib/" + } + }, + "require": { + "guzzlehttp/guzzle": "^7.3", + "guzzlehttp/psr7": "^1.7 || ^2.0" + } + } + } + ], "autoload": { "psr-4": { "OCA\\NcwTools\\": "lib/" @@ -37,6 +65,7 @@ }, "require": { "bamarni/composer-bin-plugin": "^1.8", + "ionos-productivity/ionos-ncpss-addons-api-client": "2.0.0", "php": "^8.1" }, "require-dev": { diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index b0fc7ac..3400682 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -9,6 +9,10 @@ namespace OCA\NcwTools\AppInfo; +if (file_exists(__DIR__ . '/../../vendor/autoload.php')) { + require_once __DIR__ . '/../../vendor/autoload.php'; +} + use OCA\NcwTools\Capabilities; use OCA\NcwTools\Listeners\InstallationCompletedEventListener; use OCA\NcwTools\Listeners\UserEventListener; diff --git a/lib/BackgroundJob/UserStatsJob.php b/lib/BackgroundJob/UserStatsJob.php index a88d633..9447a5a 100644 --- a/lib/BackgroundJob/UserStatsJob.php +++ b/lib/BackgroundJob/UserStatsJob.php @@ -9,6 +9,10 @@ namespace OCA\NcwTools\BackgroundJob; +use IONOS\NextcloudPSS\AddonsAPI\Client\Model\StatsUpdateRequest; +use IONOS\NextcloudPSS\AddonsAPI\Client\Model\UserStats; +use OCA\NcwTools\Service\ApiStatsClientService; +use OCA\NcwTools\Service\PssConfigService; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\QueuedJob; use OCP\IUserManager; @@ -20,6 +24,8 @@ public function __construct( private LoggerInterface $logger, private ITimeFactory $timeFactory, private IUserManager $userManager, + private ApiStatsClientService $apiClientService, + private PssConfigService $configService, ) { parent::__construct($timeFactory); } @@ -31,11 +37,43 @@ protected function run(mixed $argument): void { return; } + $brand = $this->configService->getBrand(); + $extRef = $this->configService->getExtRef(); + $baseUrl = $this->configService->getBaseUrl(); + + if ($brand === '' || $extRef === '' || $baseUrl === '') { + $this->logger->error('UserStatsJob: missing required PSS configuration, aborting'); + return; + } + + $timestamp = $this->timeFactory->getDateTime('now', new \DateTimeZone('UTC'))->format('Y-m-d\TH:i:s.v\Z'); + $payload = [ - 'timestamp' => $this->timeFactory->getDateTime('now', new \DateTimeZone('UTC'))->format('Y-m-d\TH:i:s.v\Z'), + 'timestamp' => $timestamp, 'users' => ['existingUsers' => $userTotalCount], ]; - $this->logger->info('User stats payload', ['payload' => $payload]); + + $userStats = new UserStats(); + $userStats->setExistingUsers($userTotalCount); + + try { + $request = new StatsUpdateRequest(); + $request->setTimestamp(new \DateTime($timestamp)); + $request->setUsers($userStats); + + $client = $this->apiClientService->newClient(); + $api = $this->apiClientService->newStatsAPIApi( + $client, + $baseUrl, + $this->configService->getUsername(), + $this->configService->getPassword(), + ); + $api->updateStats($brand, $extRef, $request); + } catch (\Throwable $e) { + $this->logger->error('UserStatsJob: failed to push stats to PSS', [ + 'exception' => $e->getMessage(), + ]); + } } } diff --git a/lib/Service/ApiStatsClientService.php b/lib/Service/ApiStatsClientService.php new file mode 100644 index 0000000..e486059 --- /dev/null +++ b/lib/Service/ApiStatsClientService.php @@ -0,0 +1,33 @@ + 5, + 'timeout' => 10, + ]); + } + + public function newStatsAPIApi(ClientInterface $client, string $baseUrl, string $username, string $password): StatsAPIApi { + $config = new Configuration(); + $config->setHost($baseUrl); + $config->setUsername($username); + $config->setPassword($password); + return new StatsAPIApi($client, $config); + } +} diff --git a/lib/Service/PssConfigService.php b/lib/Service/PssConfigService.php new file mode 100644 index 0000000..b001d2b --- /dev/null +++ b/lib/Service/PssConfigService.php @@ -0,0 +1,62 @@ +config->getSystemValueString('ncw_tools.pss.brand'); + if ($value === '') { + $this->logger->error('PssConfigService: ncw_tools.pss.brand is not configured'); + } + return $value; + } + + public function getExtRef(): string { + $value = $this->config->getSystemValueString('ncw_tools.pss.ext_ref'); + if ($value === '') { + $this->logger->error('PssConfigService: ncw_tools.pss.ext_ref is not configured'); + } + return $value; + } + + public function getBaseUrl(): string { + $value = $this->config->getSystemValueString('ncw_tools.pss.base_url'); + if ($value === '') { + $this->logger->error('PssConfigService: ncw_tools.pss.base_url is not configured'); + } + return $value; + } + + public function getUsername(): string { + $value = $this->config->getSystemValueString('ncw_tools.pss.username'); + if ($value === '') { + $this->logger->error('PssConfigService: ncw_tools.pss.username is not configured'); + } + return $value; + } + + public function getPassword(): string { + $value = $this->config->getSystemValueString('ncw_tools.pss.password'); + if ($value === '') { + $this->logger->error('PssConfigService: ncw_tools.pss.password is not configured'); + } + return $value; + } +} diff --git a/tests/unit/BackgroundJob/UserStatsJobTest.php b/tests/unit/BackgroundJob/UserStatsJobTest.php index 839fc3b..6ddd9d3 100644 --- a/tests/unit/BackgroundJob/UserStatsJobTest.php +++ b/tests/unit/BackgroundJob/UserStatsJobTest.php @@ -10,7 +10,12 @@ namespace OCA\NcwTools\Tests\Unit\BackgroundJob; use DateTime; +use GuzzleHttp\Client; +use IONOS\NextcloudPSS\AddonsAPI\Client\Api\StatsAPIApi; +use IONOS\NextcloudPSS\AddonsAPI\Client\Model\StatsUpdateRequest; use OCA\NcwTools\BackgroundJob\UserStatsJob; +use OCA\NcwTools\Service\ApiStatsClientService; +use OCA\NcwTools\Service\PssConfigService; use OCP\AppFramework\Utility\ITimeFactory; use OCP\IUserManager; use PHPUnit\Framework\MockObject\MockObject; @@ -21,6 +26,9 @@ class UserStatsJobTest extends TestCase { private LoggerInterface&MockObject $logger; private ITimeFactory&MockObject $timeFactory; private IUserManager&MockObject $userManager; + private ApiStatsClientService&MockObject $apiClientService; + private PssConfigService&MockObject $configService; + private StatsAPIApi&MockObject $statsApi; private UserStatsJob $job; protected function setUp(): void { @@ -31,27 +39,84 @@ protected function setUp(): void { $this->timeFactory->method('getDateTime')->willReturn(new DateTime('2026-01-01T00:00:00.000 UTC')); $this->userManager = $this->createMock(IUserManager::class); + $this->statsApi = $this->createMock(StatsAPIApi::class); + $this->apiClientService = $this->createMock(ApiStatsClientService::class); + $this->apiClientService->method('newClient')->willReturn($this->createMock(Client::class)); + $this->apiClientService->method('newStatsAPIApi')->willReturn($this->statsApi); + + $this->configService = $this->createMock(PssConfigService::class); + $this->configService->method('getBrand')->willReturn('IONOS'); + $this->configService->method('getExtRef')->willReturn('test-ext-ref'); + $this->configService->method('getBaseUrl')->willReturn('https://pss.example.com'); + $this->configService->method('getUsername')->willReturn('user'); + $this->configService->method('getPassword')->willReturn('pass'); + $this->job = new UserStatsJob( $this->logger, $this->timeFactory, $this->userManager, + $this->apiClientService, + $this->configService, ); } - public function testRunLogsPayloadOnSuccess(): void { + public function testRunCallsApiWithCorrectPayload(): void { $this->userManager->method('countUsersTotal')->willReturn(42); $this->logger->expects($this->once()) ->method('info') + ->with('User stats payload', $this->callback(function (array $context): bool { + return $context['payload']['users']['existingUsers'] === 42 + && preg_match('/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/', $context['payload']['timestamp']) === 1; + })); + + $this->statsApi->expects($this->once()) + ->method('updateStats') ->with( - 'User stats payload', - $this->callback(function (array $context): bool { - return preg_match('/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/', $context['payload']['timestamp']) === 1 - && $context['payload']['users']['existingUsers'] === 42; + 'IONOS', + 'test-ext-ref', + $this->callback(function (StatsUpdateRequest $req): bool { + return $req->getUsers()?->getExistingUsers() === 42; }) ); - $this->logger->expects($this->never())->method('warning'); + $this->invokePrivate($this->job, 'run', [null]); + } + + public function testRunLogsErrorWhenApiThrows(): void { + $this->userManager->method('countUsersTotal')->willReturn(42); + $this->statsApi->method('updateStats')->willThrowException(new \Exception('connection refused')); + + $this->logger->expects($this->once()) + ->method('error') + ->with('UserStatsJob: failed to push stats to PSS', $this->callback(function (array $ctx): bool { + return str_contains($ctx['exception'], 'connection refused'); + })); + + $this->invokePrivate($this->job, 'run', [null]); + } + + public function testRunLogsErrorWhenRequiredConfigMissing(): void { + $this->userManager->method('countUsersTotal')->willReturn(5); + + $this->configService = $this->createMock(PssConfigService::class); + $this->configService->method('getBrand')->willReturn(''); + $this->configService->method('getExtRef')->willReturn('test-ext-ref'); + $this->configService->method('getBaseUrl')->willReturn('https://pss.example.com'); + + $this->job = new UserStatsJob( + $this->logger, + $this->timeFactory, + $this->userManager, + $this->apiClientService, + $this->configService, + ); + + $this->logger->expects($this->once()) + ->method('error') + ->with('UserStatsJob: missing required PSS configuration, aborting'); + + $this->statsApi->expects($this->never())->method('updateStats'); $this->invokePrivate($this->job, 'run', [null]); } @@ -63,7 +128,7 @@ public function testRunLogsWarningWhenCountFalse(): void { ->method('warning') ->with('UserStatsJob: could not retrieve user count'); - $this->logger->expects($this->never())->method('info'); + $this->statsApi->expects($this->never())->method('updateStats'); $this->invokePrivate($this->job, 'run', [null]); } From 746314680777c3315f87078d7a3decfe3da61266 Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Fri, 15 May 2026 09:43:58 +0200 Subject: [PATCH 04/10] docs(user-stats): add event-flow doc for stats reporting Documents the user-stats event flow that the recent UserStatsJob / ApiStatsClientService work implements: UserCreatedEvent / UserDeletedEvent -> UserEventListener -> deduplicated UserStatsJob -> PSS Stats API POST. The sequence diagram covers the abort branches (count unavailable, any PSS config value missing), the successful POST path, and the Throwable catch. Adds the flow to the docs index alongside post-setup. --- REUSE.toml | 3 +- docs/README.md | 1 + docs/events/user-stats.md | 100 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 docs/events/user-stats.md diff --git a/REUSE.toml b/REUSE.toml index 3540d4b..34d2636 100644 --- a/REUSE.toml +++ b/REUSE.toml @@ -43,7 +43,8 @@ path = [ "CHANGELOG.md", "CODE_OF_CONDUCT.md", "docs/README.md", - "docs/events/post-setup.md" + "docs/events/post-setup.md", + "docs/events/user-stats.md" ] precedence = "aggregate" SPDX-FileCopyrightText = "2026 STRATO GmbH" diff --git a/docs/README.md b/docs/README.md index 703ce75..6535d63 100644 --- a/docs/README.md +++ b/docs/README.md @@ -3,3 +3,4 @@ NCW Tools reacts to Nextcloud lifecycle events by registering listeners that enqueue background jobs. Each document below walks through one end-to-end flow — the triggering event, the listener's synchronous work, the job's asynchronous behaviour, and the configuration it relies on. - [Post-setup welcome mail](events/post-setup.md) — sends the initial welcome email to the admin user once the system is reachable after installation. +- [User stats reporting](events/user-stats.md) — reports the current total user count to the PSS Stats API after every user create or delete. diff --git a/docs/events/user-stats.md b/docs/events/user-stats.md new file mode 100644 index 0000000..572ea94 --- /dev/null +++ b/docs/events/user-stats.md @@ -0,0 +1,100 @@ +# User stats reporting + +On every user create or delete, this app reports the current total user count to the PSS Stats API. A listener logs the change and enqueues a single deduplicated background job; on the next cron tick the job reads the total user count, validates the PSS credentials, and POSTs a `StatsUpdateRequest` to the PSS Stats API. The job is queued — it does not retry on failure; the next user event re-enqueues it. + +## Trigger events + +- `OCP\User\Events\UserCreatedEvent` +- `OCP\User\Events\UserDeletedEvent` + +## Configuration + +All keys live in `config/config.php`. The job aborts (with a logged error) if any value is missing or empty. + +| Key | Type | Purpose | +| --- | --- | --- | +| `ncw_tools.pss.brand` | string | PSS brand identifier; first path segment of the stats endpoint. | +| `ncw_tools.pss.ext_ref` | string | External tenant reference; second path segment of the stats endpoint. | +| `ncw_tools.pss.base_url` | string | Base URL of the PSS API. | +| `ncw_tools.pss.username` | string | HTTP Basic auth username. | +| `ncw_tools.pss.password` | string | HTTP Basic auth password. | + +## Flow + +```mermaid +sequenceDiagram + autonumber + + participant OCC as occ user:add / user:delete + participant ED as EventDispatcher + participant UEL as UserEventListener + participant LOG as LoggerInterface + participant JL as IJobList + participant CRON as Nextcloud Cron + participant USJ as UserStatsJob + participant UM as IUserManager + participant CFG as PssConfigService + participant API as ApiStatsClientService + participant PSS as PSS Stats API + + Note over OCC,JL: Synchronous phase — runs during the OCC command + + OCC->>ED: dispatch(UserCreatedEvent | UserDeletedEvent) + ED->>UEL: handle(event) + + alt UserCreatedEvent + UEL->>LOG: info("User added", {uid}) + else UserDeletedEvent + UEL->>LOG: info("User deleted", {uid}) + end + + UEL->>JL: has(UserStatsJob::class, null) + + alt already queued + JL-->>UEL: true (skip) + else not queued + JL-->>UEL: false + UEL->>JL: add(UserStatsJob::class) + end + + Note over CRON,PSS: Asynchronous phase — next cron cycle + + CRON->>USJ: run() + USJ->>UM: countUsersTotal() + UM-->>USJ: int | false + + alt count === false + USJ->>LOG: warning("could not retrieve user count") + USJ-->>CRON: return + else count is int + USJ->>CFG: getBrand / getExtRef / getBaseUrl / getUsername / getPassword + + alt any value empty + USJ->>LOG: error("missing required PSS configuration, aborting") + USJ-->>CRON: return + else all values present + USJ->>USJ: build timestamp (UTC ISO-8601 ms)
build StatsUpdateRequest with UserStats(existingUsers) + USJ->>LOG: info("User stats payload", {payload}) + USJ->>API: newClient() + USJ->>API: newStatsAPIApi(client, baseUrl, username, password) + USJ->>PSS: updateStats(brand, extRef, request) + + alt Throwable + PSS-->>USJ: exception + USJ->>LOG: error("failed to push stats to PSS", {exception}) + else success + PSS-->>USJ: 2xx + end + end + end +``` + +## Failure modes + +The job is a `QueuedJob` — there is no automatic retry. Each of the following ends the run without reporting: + +- `IUserManager::countUsersTotal()` returns `false` (logged at warning). +- Any of the five `ncw_tools.pss.*` values is missing or empty (logged at error). +- The PSS API call throws (logged at error with the exception). + +The next `UserCreatedEvent` or `UserDeletedEvent` re-enqueues the job, so the count converges once the underlying problem is resolved. From c5f6cd5c23170cc121631f2111f9d8c4ef1c9123 Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Wed, 13 May 2026 17:53:29 +0200 Subject: [PATCH 05/10] chore(deps): pin api-client commit and declare transitive guzzle requires Pin the ionos-ncpss-addons-api-client repository entry to commit a5f09eacaa3139e50e43ced4a44137946a8bfe18 in both `source` and `dist` blocks. Declare guzzle and psr7 as transitive requires so composer can resolve the dependency graph without dereferencing the source tag. Installs become reproducible and don't require git access. Signed-off-by: Misha M.-Kupriyanov --- composer.json | 11 +- composer.lock | 622 +++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 624 insertions(+), 9 deletions(-) diff --git a/composer.json b/composer.json index c9c96d5..dc6c4cd 100644 --- a/composer.json +++ b/composer.json @@ -20,19 +20,14 @@ "url": "https://github.com/ionos-productivity/ionos-ncpss-addons-api-client.git", "reference": "2.0.0-20260429080405" }, - "dist": { - "type": "zip", - "url": "https://github.com/ionos-productivity/ionos-ncpss-addons-api-client/archive/refs/tags/2.0.0-20260429080405.zip", - "reference": "2.0.0-20260429080405" + "require": { + "guzzlehttp/guzzle": "^7.3", + "guzzlehttp/psr7": "^1.7 || ^2.0" }, "autoload": { "psr-4": { "IONOS\\NextcloudPSS\\AddonsAPI\\Client\\": "lib/" } - }, - "require": { - "guzzlehttp/guzzle": "^7.3", - "guzzlehttp/psr7": "^1.7 || ^2.0" } } } diff --git a/composer.lock b/composer.lock index 7ec3dc8..6db10c5 100644 --- a/composer.lock +++ b/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": "c429e219370ff3b6c87f4d2c835f43b7", + "content-hash": "aff3ddb2be52e4825714064f03c35ceb", "packages": [ { "name": "bamarni/composer-bin-plugin", @@ -62,6 +62,626 @@ "source": "https://github.com/bamarni/composer-bin-plugin/tree/1.9.1" }, "time": "2026-02-04T10:18:12+00:00" + }, + { + "name": "guzzlehttp/guzzle", + "version": "7.10.0", + "source": { + "type": "git", + "url": "https://github.com/guzzle/guzzle.git", + "reference": "b51ac707cfa420b7bfd4e4d5e510ba8008e822b4" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/guzzle/guzzle/zipball/b51ac707cfa420b7bfd4e4d5e510ba8008e822b4", + "reference": "b51ac707cfa420b7bfd4e4d5e510ba8008e822b4", + "shasum": "" + }, + "require": { + "ext-json": "*", + "guzzlehttp/promises": "^2.3", + "guzzlehttp/psr7": "^2.8", + "php": "^7.2.5 || ^8.0", + "psr/http-client": "^1.0", + "symfony/deprecation-contracts": "^2.2 || ^3.0" + }, + "provide": { + "psr/http-client-implementation": "1.0" + }, + "require-dev": { + "bamarni/composer-bin-plugin": "^1.8.2", + "ext-curl": "*", + "guzzle/client-integration-tests": "3.0.2", + "php-http/message-factory": "^1.1", + "phpunit/phpunit": "^8.5.39 || ^9.6.20", + "psr/log": "^1.1 || ^2.0 || ^3.0" + }, + "suggest": { + "ext-curl": "Required for CURL handler support", + "ext-intl": "Required for Internationalized Domain Name (IDN) support", + "psr/log": "Required for using the Log middleware" + }, + "type": "library", + "extra": { + "bamarni-bin": { + "bin-links": true, + "forward-command": false + } + }, + "autoload": { + "files": [ + "src/functions_include.php" + ], + "psr-4": { + "GuzzleHttp\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Graham Campbell", + "email": "hello@gjcampbell.co.uk", + "homepage": "https://github.com/GrahamCampbell" + }, + { + "name": "Michael Dowling", + "email": "mtdowling@gmail.com", + "homepage": "https://github.com/mtdowling" + }, + { + "name": "Jeremy Lindblom", + "email": "jeremeamia@gmail.com", + "homepage": "https://github.com/jeremeamia" + }, + { + "name": "George Mponos", + "email": "gmponos@gmail.com", + "homepage": "https://github.com/gmponos" + }, + { + "name": "Tobias Nyholm", + "email": "tobias.nyholm@gmail.com", + "homepage": "https://github.com/Nyholm" + }, + { + "name": "Márk Sági-Kazár", + "email": "mark.sagikazar@gmail.com", + "homepage": "https://github.com/sagikazarmark" + }, + { + "name": "Tobias Schultze", + "email": "webmaster@tubo-world.de", + "homepage": "https://github.com/Tobion" + } + ], + "description": "Guzzle is a PHP HTTP client library", + "keywords": [ + "client", + "curl", + "framework", + "http", + "http client", + "psr-18", + "psr-7", + "rest", + "web service" + ], + "support": { + "issues": "https://github.com/guzzle/guzzle/issues", + "source": "https://github.com/guzzle/guzzle/tree/7.10.0" + }, + "funding": [ + { + "url": "https://github.com/GrahamCampbell", + "type": "github" + }, + { + "url": "https://github.com/Nyholm", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/guzzlehttp/guzzle", + "type": "tidelift" + } + ], + "time": "2025-08-23T22:36:01+00:00" + }, + { + "name": "guzzlehttp/promises", + "version": "2.3.0", + "source": { + "type": "git", + "url": "https://github.com/guzzle/promises.git", + "reference": "481557b130ef3790cf82b713667b43030dc9c957" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/guzzle/promises/zipball/481557b130ef3790cf82b713667b43030dc9c957", + "reference": "481557b130ef3790cf82b713667b43030dc9c957", + "shasum": "" + }, + "require": { + "php": "^7.2.5 || ^8.0" + }, + "require-dev": { + "bamarni/composer-bin-plugin": "^1.8.2", + "phpunit/phpunit": "^8.5.44 || ^9.6.25" + }, + "type": "library", + "extra": { + "bamarni-bin": { + "bin-links": true, + "forward-command": false + } + }, + "autoload": { + "psr-4": { + "GuzzleHttp\\Promise\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Graham Campbell", + "email": "hello@gjcampbell.co.uk", + "homepage": "https://github.com/GrahamCampbell" + }, + { + "name": "Michael Dowling", + "email": "mtdowling@gmail.com", + "homepage": "https://github.com/mtdowling" + }, + { + "name": "Tobias Nyholm", + "email": "tobias.nyholm@gmail.com", + "homepage": "https://github.com/Nyholm" + }, + { + "name": "Tobias Schultze", + "email": "webmaster@tubo-world.de", + "homepage": "https://github.com/Tobion" + } + ], + "description": "Guzzle promises library", + "keywords": [ + "promise" + ], + "support": { + "issues": "https://github.com/guzzle/promises/issues", + "source": "https://github.com/guzzle/promises/tree/2.3.0" + }, + "funding": [ + { + "url": "https://github.com/GrahamCampbell", + "type": "github" + }, + { + "url": "https://github.com/Nyholm", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/guzzlehttp/promises", + "type": "tidelift" + } + ], + "time": "2025-08-22T14:34:08+00:00" + }, + { + "name": "guzzlehttp/psr7", + "version": "2.9.0", + "source": { + "type": "git", + "url": "https://github.com/guzzle/psr7.git", + "reference": "7d0ed42f28e42d61352a7a79de682e5e67fec884" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/guzzle/psr7/zipball/7d0ed42f28e42d61352a7a79de682e5e67fec884", + "reference": "7d0ed42f28e42d61352a7a79de682e5e67fec884", + "shasum": "" + }, + "require": { + "php": "^7.2.5 || ^8.0", + "psr/http-factory": "^1.0", + "psr/http-message": "^1.1 || ^2.0", + "ralouphie/getallheaders": "^3.0" + }, + "provide": { + "psr/http-factory-implementation": "1.0", + "psr/http-message-implementation": "1.0" + }, + "require-dev": { + "bamarni/composer-bin-plugin": "^1.8.2", + "http-interop/http-factory-tests": "0.9.0", + "jshttp/mime-db": "1.54.0.1", + "phpunit/phpunit": "^8.5.44 || ^9.6.25" + }, + "suggest": { + "laminas/laminas-httphandlerrunner": "Emit PSR-7 responses" + }, + "type": "library", + "extra": { + "bamarni-bin": { + "bin-links": true, + "forward-command": false + } + }, + "autoload": { + "psr-4": { + "GuzzleHttp\\Psr7\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Graham Campbell", + "email": "hello@gjcampbell.co.uk", + "homepage": "https://github.com/GrahamCampbell" + }, + { + "name": "Michael Dowling", + "email": "mtdowling@gmail.com", + "homepage": "https://github.com/mtdowling" + }, + { + "name": "George Mponos", + "email": "gmponos@gmail.com", + "homepage": "https://github.com/gmponos" + }, + { + "name": "Tobias Nyholm", + "email": "tobias.nyholm@gmail.com", + "homepage": "https://github.com/Nyholm" + }, + { + "name": "Márk Sági-Kazár", + "email": "mark.sagikazar@gmail.com", + "homepage": "https://github.com/sagikazarmark" + }, + { + "name": "Tobias Schultze", + "email": "webmaster@tubo-world.de", + "homepage": "https://github.com/Tobion" + }, + { + "name": "Márk Sági-Kazár", + "email": "mark.sagikazar@gmail.com", + "homepage": "https://sagikazarmark.hu" + } + ], + "description": "PSR-7 message implementation that also provides common utility methods", + "keywords": [ + "http", + "message", + "psr-7", + "request", + "response", + "stream", + "uri", + "url" + ], + "support": { + "issues": "https://github.com/guzzle/psr7/issues", + "source": "https://github.com/guzzle/psr7/tree/2.9.0" + }, + "funding": [ + { + "url": "https://github.com/GrahamCampbell", + "type": "github" + }, + { + "url": "https://github.com/Nyholm", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/guzzlehttp/psr7", + "type": "tidelift" + } + ], + "time": "2026-03-10T16:41:02+00:00" + }, + { + "name": "ionos-productivity/ionos-ncpss-addons-api-client", + "version": "2.0.0", + "source": { + "type": "git", + "url": "https://github.com/ionos-productivity/ionos-ncpss-addons-api-client.git", + "reference": "2.0.0-20260429080405" + }, + "require": { + "guzzlehttp/guzzle": "^7.3", + "guzzlehttp/psr7": "^1.7 || ^2.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "IONOS\\NextcloudPSS\\AddonsAPI\\Client\\": "lib/" + } + } + }, + { + "name": "psr/http-client", + "version": "1.0.3", + "source": { + "type": "git", + "url": "https://github.com/php-fig/http-client.git", + "reference": "bb5906edc1c324c9a05aa0873d40117941e5fa90" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-fig/http-client/zipball/bb5906edc1c324c9a05aa0873d40117941e5fa90", + "reference": "bb5906edc1c324c9a05aa0873d40117941e5fa90", + "shasum": "" + }, + "require": { + "php": "^7.0 || ^8.0", + "psr/http-message": "^1.0 || ^2.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.0.x-dev" + } + }, + "autoload": { + "psr-4": { + "Psr\\Http\\Client\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "PHP-FIG", + "homepage": "https://www.php-fig.org/" + } + ], + "description": "Common interface for HTTP clients", + "homepage": "https://github.com/php-fig/http-client", + "keywords": [ + "http", + "http-client", + "psr", + "psr-18" + ], + "support": { + "source": "https://github.com/php-fig/http-client" + }, + "time": "2023-09-23T14:17:50+00:00" + }, + { + "name": "psr/http-factory", + "version": "1.1.0", + "source": { + "type": "git", + "url": "https://github.com/php-fig/http-factory.git", + "reference": "2b4765fddfe3b508ac62f829e852b1501d3f6e8a" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-fig/http-factory/zipball/2b4765fddfe3b508ac62f829e852b1501d3f6e8a", + "reference": "2b4765fddfe3b508ac62f829e852b1501d3f6e8a", + "shasum": "" + }, + "require": { + "php": ">=7.1", + "psr/http-message": "^1.0 || ^2.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.0.x-dev" + } + }, + "autoload": { + "psr-4": { + "Psr\\Http\\Message\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "PHP-FIG", + "homepage": "https://www.php-fig.org/" + } + ], + "description": "PSR-17: Common interfaces for PSR-7 HTTP message factories", + "keywords": [ + "factory", + "http", + "message", + "psr", + "psr-17", + "psr-7", + "request", + "response" + ], + "support": { + "source": "https://github.com/php-fig/http-factory" + }, + "time": "2024-04-15T12:06:14+00:00" + }, + { + "name": "psr/http-message", + "version": "2.0", + "source": { + "type": "git", + "url": "https://github.com/php-fig/http-message.git", + "reference": "402d35bcb92c70c026d1a6a9883f06b2ead23d71" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-fig/http-message/zipball/402d35bcb92c70c026d1a6a9883f06b2ead23d71", + "reference": "402d35bcb92c70c026d1a6a9883f06b2ead23d71", + "shasum": "" + }, + "require": { + "php": "^7.2 || ^8.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "2.0.x-dev" + } + }, + "autoload": { + "psr-4": { + "Psr\\Http\\Message\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "PHP-FIG", + "homepage": "https://www.php-fig.org/" + } + ], + "description": "Common interface for HTTP messages", + "homepage": "https://github.com/php-fig/http-message", + "keywords": [ + "http", + "http-message", + "psr", + "psr-7", + "request", + "response" + ], + "support": { + "source": "https://github.com/php-fig/http-message/tree/2.0" + }, + "time": "2023-04-04T09:54:51+00:00" + }, + { + "name": "ralouphie/getallheaders", + "version": "3.0.3", + "source": { + "type": "git", + "url": "https://github.com/ralouphie/getallheaders.git", + "reference": "120b605dfeb996808c31b6477290a714d356e822" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/ralouphie/getallheaders/zipball/120b605dfeb996808c31b6477290a714d356e822", + "reference": "120b605dfeb996808c31b6477290a714d356e822", + "shasum": "" + }, + "require": { + "php": ">=5.6" + }, + "require-dev": { + "php-coveralls/php-coveralls": "^2.1", + "phpunit/phpunit": "^5 || ^6.5" + }, + "type": "library", + "autoload": { + "files": [ + "src/getallheaders.php" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Ralph Khattar", + "email": "ralph.khattar@gmail.com" + } + ], + "description": "A polyfill for getallheaders.", + "support": { + "issues": "https://github.com/ralouphie/getallheaders/issues", + "source": "https://github.com/ralouphie/getallheaders/tree/develop" + }, + "time": "2019-03-08T08:55:37+00:00" + }, + { + "name": "symfony/deprecation-contracts", + "version": "v3.7.0", + "source": { + "type": "git", + "url": "https://github.com/symfony/deprecation-contracts.git", + "reference": "50f59d1f3ca46d41ac911f97a78626b6756af35b" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/deprecation-contracts/zipball/50f59d1f3ca46d41ac911f97a78626b6756af35b", + "reference": "50f59d1f3ca46d41ac911f97a78626b6756af35b", + "shasum": "" + }, + "require": { + "php": ">=8.1" + }, + "type": "library", + "extra": { + "thanks": { + "url": "https://github.com/symfony/contracts", + "name": "symfony/contracts" + }, + "branch-alias": { + "dev-main": "3.7-dev" + } + }, + "autoload": { + "files": [ + "function.php" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Nicolas Grekas", + "email": "p@tchwork.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "A generic function and convention to trigger deprecation notices", + "homepage": "https://symfony.com", + "support": { + "source": "https://github.com/symfony/deprecation-contracts/tree/v3.7.0" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://github.com/nicolas-grekas", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2026-04-13T15:52:40+00:00" } ], "packages-dev": [ From 0503dec5aa1fb13529e4fdba4b0f19934a6bf324 Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Wed, 13 May 2026 18:19:57 +0200 Subject: [PATCH 06/10] fix(user-stats): apply review-driven hardening MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five small corrections to the original feat: - Require username + password in the pre-flight config guard (previously: silent 401 on the live API call). - Log the Throwable object instead of getMessage() so Nextcloud's logger preserves the serialized stack trace. - Drop the file_exists() fallback around vendor/autoload.php — a hard require failing at boot is louder than a "Class not found" at job-run time. - Drop the per-getter error logging in PssConfigService. With a fully unconfigured install each cron tick produced six error lines; the aggregate guard in UserStatsJob is enough. - Hold the timestamp as a single DateTime end-to-end instead of formatting it for the log then re-parsing it for the API request. Signed-off-by: Misha M.-Kupriyanov --- lib/AppInfo/Application.php | 4 +-- lib/BackgroundJob/UserStatsJob.php | 16 ++++++---- lib/Service/PssConfigService.php | 32 +++---------------- tests/unit/BackgroundJob/UserStatsJobTest.php | 32 ++++++++++++++++++- 4 files changed, 46 insertions(+), 38 deletions(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 3400682..68c290f 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -9,9 +9,7 @@ namespace OCA\NcwTools\AppInfo; -if (file_exists(__DIR__ . '/../../vendor/autoload.php')) { - require_once __DIR__ . '/../../vendor/autoload.php'; -} +require_once __DIR__ . '/../../vendor/autoload.php'; use OCA\NcwTools\Capabilities; use OCA\NcwTools\Listeners\InstallationCompletedEventListener; diff --git a/lib/BackgroundJob/UserStatsJob.php b/lib/BackgroundJob/UserStatsJob.php index 9447a5a..1a8de31 100644 --- a/lib/BackgroundJob/UserStatsJob.php +++ b/lib/BackgroundJob/UserStatsJob.php @@ -40,16 +40,18 @@ protected function run(mixed $argument): void { $brand = $this->configService->getBrand(); $extRef = $this->configService->getExtRef(); $baseUrl = $this->configService->getBaseUrl(); + $username = $this->configService->getUsername(); + $password = $this->configService->getPassword(); - if ($brand === '' || $extRef === '' || $baseUrl === '') { + if ($brand === '' || $extRef === '' || $baseUrl === '' || $username === '' || $password === '') { $this->logger->error('UserStatsJob: missing required PSS configuration, aborting'); return; } - $timestamp = $this->timeFactory->getDateTime('now', new \DateTimeZone('UTC'))->format('Y-m-d\TH:i:s.v\Z'); + $timestamp = $this->timeFactory->getDateTime('now', new \DateTimeZone('UTC')); $payload = [ - 'timestamp' => $timestamp, + 'timestamp' => $timestamp->format('Y-m-d\TH:i:s.v\Z'), 'users' => ['existingUsers' => $userTotalCount], ]; $this->logger->info('User stats payload', ['payload' => $payload]); @@ -59,20 +61,20 @@ protected function run(mixed $argument): void { try { $request = new StatsUpdateRequest(); - $request->setTimestamp(new \DateTime($timestamp)); + $request->setTimestamp($timestamp); $request->setUsers($userStats); $client = $this->apiClientService->newClient(); $api = $this->apiClientService->newStatsAPIApi( $client, $baseUrl, - $this->configService->getUsername(), - $this->configService->getPassword(), + $username, + $password, ); $api->updateStats($brand, $extRef, $request); } catch (\Throwable $e) { $this->logger->error('UserStatsJob: failed to push stats to PSS', [ - 'exception' => $e->getMessage(), + 'exception' => $e, ]); } } diff --git a/lib/Service/PssConfigService.php b/lib/Service/PssConfigService.php index b001d2b..ed65209 100644 --- a/lib/Service/PssConfigService.php +++ b/lib/Service/PssConfigService.php @@ -10,53 +10,31 @@ namespace OCA\NcwTools\Service; use OCP\IConfig; -use Psr\Log\LoggerInterface; class PssConfigService { public function __construct( private IConfig $config, - private LoggerInterface $logger, ) { } public function getBrand(): string { - $value = $this->config->getSystemValueString('ncw_tools.pss.brand'); - if ($value === '') { - $this->logger->error('PssConfigService: ncw_tools.pss.brand is not configured'); - } - return $value; + return $this->config->getSystemValueString('ncw_tools.pss.brand'); } public function getExtRef(): string { - $value = $this->config->getSystemValueString('ncw_tools.pss.ext_ref'); - if ($value === '') { - $this->logger->error('PssConfigService: ncw_tools.pss.ext_ref is not configured'); - } - return $value; + return $this->config->getSystemValueString('ncw_tools.pss.ext_ref'); } public function getBaseUrl(): string { - $value = $this->config->getSystemValueString('ncw_tools.pss.base_url'); - if ($value === '') { - $this->logger->error('PssConfigService: ncw_tools.pss.base_url is not configured'); - } - return $value; + return $this->config->getSystemValueString('ncw_tools.pss.base_url'); } public function getUsername(): string { - $value = $this->config->getSystemValueString('ncw_tools.pss.username'); - if ($value === '') { - $this->logger->error('PssConfigService: ncw_tools.pss.username is not configured'); - } - return $value; + return $this->config->getSystemValueString('ncw_tools.pss.username'); } public function getPassword(): string { - $value = $this->config->getSystemValueString('ncw_tools.pss.password'); - if ($value === '') { - $this->logger->error('PssConfigService: ncw_tools.pss.password is not configured'); - } - return $value; + return $this->config->getSystemValueString('ncw_tools.pss.password'); } } diff --git a/tests/unit/BackgroundJob/UserStatsJobTest.php b/tests/unit/BackgroundJob/UserStatsJobTest.php index 6ddd9d3..51e540a 100644 --- a/tests/unit/BackgroundJob/UserStatsJobTest.php +++ b/tests/unit/BackgroundJob/UserStatsJobTest.php @@ -90,7 +90,8 @@ public function testRunLogsErrorWhenApiThrows(): void { $this->logger->expects($this->once()) ->method('error') ->with('UserStatsJob: failed to push stats to PSS', $this->callback(function (array $ctx): bool { - return str_contains($ctx['exception'], 'connection refused'); + return $ctx['exception'] instanceof \Exception + && str_contains($ctx['exception']->getMessage(), 'connection refused'); })); $this->invokePrivate($this->job, 'run', [null]); @@ -103,6 +104,35 @@ public function testRunLogsErrorWhenRequiredConfigMissing(): void { $this->configService->method('getBrand')->willReturn(''); $this->configService->method('getExtRef')->willReturn('test-ext-ref'); $this->configService->method('getBaseUrl')->willReturn('https://pss.example.com'); + $this->configService->method('getUsername')->willReturn('user'); + $this->configService->method('getPassword')->willReturn('pass'); + + $this->job = new UserStatsJob( + $this->logger, + $this->timeFactory, + $this->userManager, + $this->apiClientService, + $this->configService, + ); + + $this->logger->expects($this->once()) + ->method('error') + ->with('UserStatsJob: missing required PSS configuration, aborting'); + + $this->statsApi->expects($this->never())->method('updateStats'); + + $this->invokePrivate($this->job, 'run', [null]); + } + + public function testRunLogsErrorWhenCredentialsMissing(): void { + $this->userManager->method('countUsersTotal')->willReturn(5); + + $this->configService = $this->createMock(PssConfigService::class); + $this->configService->method('getBrand')->willReturn('IONOS'); + $this->configService->method('getExtRef')->willReturn('test-ext-ref'); + $this->configService->method('getBaseUrl')->willReturn('https://pss.example.com'); + $this->configService->method('getUsername')->willReturn(''); + $this->configService->method('getPassword')->willReturn('pass'); $this->job = new UserStatsJob( $this->logger, From 563582a2537119c8dca15097a5ca587866998f4d Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Fri, 15 May 2026 10:01:49 +0200 Subject: [PATCH 07/10] test(user-stats): tighten happy-path and consolidate missing-config cases Assert that the success-path emits no error or warning so a regression that started logging spuriously on the happy path would fail the test. Consolidate the five one-per-missing-field test methods into a single DataProvider-driven test parameterised over every required key. Signed-off-by: Misha M.-Kupriyanov --- lib/BackgroundJob/UserStatsJob.php | 27 +++--- lib/Service/ApiStatsClientService.php | 8 +- lib/Service/PssConfigService.php | 15 ++-- tests/unit/BackgroundJob/UserStatsJobTest.php | 87 ++++++++----------- 4 files changed, 56 insertions(+), 81 deletions(-) diff --git a/lib/BackgroundJob/UserStatsJob.php b/lib/BackgroundJob/UserStatsJob.php index 1a8de31..c851acc 100644 --- a/lib/BackgroundJob/UserStatsJob.php +++ b/lib/BackgroundJob/UserStatsJob.php @@ -50,27 +50,20 @@ protected function run(mixed $argument): void { $timestamp = $this->timeFactory->getDateTime('now', new \DateTimeZone('UTC')); - $payload = [ - 'timestamp' => $timestamp->format('Y-m-d\TH:i:s.v\Z'), - 'users' => ['existingUsers' => $userTotalCount], - ]; - $this->logger->info('User stats payload', ['payload' => $payload]); - $userStats = new UserStats(); $userStats->setExistingUsers($userTotalCount); - try { - $request = new StatsUpdateRequest(); - $request->setTimestamp($timestamp); - $request->setUsers($userStats); + $request = new StatsUpdateRequest(); + $request->setTimestamp($timestamp); + $request->setUsers($userStats); - $client = $this->apiClientService->newClient(); - $api = $this->apiClientService->newStatsAPIApi( - $client, - $baseUrl, - $username, - $password, - ); + $this->logger->info('UserStatsJob: pushing user stats', [ + 'existingUsers' => $userTotalCount, + 'timestamp' => $timestamp->format('Y-m-d\TH:i:s.v\Z'), + ]); + + try { + $api = $this->apiClientService->newStatsAPIApi($baseUrl, $username, $password); $api->updateStats($brand, $extRef, $request); } catch (\Throwable $e) { $this->logger->error('UserStatsJob: failed to push stats to PSS', [ diff --git a/lib/Service/ApiStatsClientService.php b/lib/Service/ApiStatsClientService.php index e486059..09a76eb 100644 --- a/lib/Service/ApiStatsClientService.php +++ b/lib/Service/ApiStatsClientService.php @@ -10,20 +10,16 @@ namespace OCA\NcwTools\Service; use GuzzleHttp\Client; -use GuzzleHttp\ClientInterface; use IONOS\NextcloudPSS\AddonsAPI\Client\Api\StatsAPIApi; use IONOS\NextcloudPSS\AddonsAPI\Client\Configuration; class ApiStatsClientService { - public function newClient(): Client { - return new Client([ + public function newStatsAPIApi(string $baseUrl, string $username, string $password): StatsAPIApi { + $client = new Client([ 'connect_timeout' => 5, 'timeout' => 10, ]); - } - - public function newStatsAPIApi(ClientInterface $client, string $baseUrl, string $username, string $password): StatsAPIApi { $config = new Configuration(); $config->setHost($baseUrl); $config->setUsername($username); diff --git a/lib/Service/PssConfigService.php b/lib/Service/PssConfigService.php index ed65209..84adae3 100644 --- a/lib/Service/PssConfigService.php +++ b/lib/Service/PssConfigService.php @@ -12,6 +12,11 @@ use OCP\IConfig; class PssConfigService { + private const KEY_BRAND = 'ncw_tools.pss.brand'; + private const KEY_EXT_REF = 'ncw_tools.pss.ext_ref'; + private const KEY_BASE_URL = 'ncw_tools.pss.base_url'; + private const KEY_USERNAME = 'ncw_tools.pss.username'; + private const KEY_PASSWORD = 'ncw_tools.pss.password'; public function __construct( private IConfig $config, @@ -19,22 +24,22 @@ public function __construct( } public function getBrand(): string { - return $this->config->getSystemValueString('ncw_tools.pss.brand'); + return $this->config->getSystemValueString(self::KEY_BRAND); } public function getExtRef(): string { - return $this->config->getSystemValueString('ncw_tools.pss.ext_ref'); + return $this->config->getSystemValueString(self::KEY_EXT_REF); } public function getBaseUrl(): string { - return $this->config->getSystemValueString('ncw_tools.pss.base_url'); + return $this->config->getSystemValueString(self::KEY_BASE_URL); } public function getUsername(): string { - return $this->config->getSystemValueString('ncw_tools.pss.username'); + return $this->config->getSystemValueString(self::KEY_USERNAME); } public function getPassword(): string { - return $this->config->getSystemValueString('ncw_tools.pss.password'); + return $this->config->getSystemValueString(self::KEY_PASSWORD); } } diff --git a/tests/unit/BackgroundJob/UserStatsJobTest.php b/tests/unit/BackgroundJob/UserStatsJobTest.php index 51e540a..809ba0d 100644 --- a/tests/unit/BackgroundJob/UserStatsJobTest.php +++ b/tests/unit/BackgroundJob/UserStatsJobTest.php @@ -10,7 +10,6 @@ namespace OCA\NcwTools\Tests\Unit\BackgroundJob; use DateTime; -use GuzzleHttp\Client; use IONOS\NextcloudPSS\AddonsAPI\Client\Api\StatsAPIApi; use IONOS\NextcloudPSS\AddonsAPI\Client\Model\StatsUpdateRequest; use OCA\NcwTools\BackgroundJob\UserStatsJob; @@ -18,6 +17,7 @@ use OCA\NcwTools\Service\PssConfigService; use OCP\AppFramework\Utility\ITimeFactory; use OCP\IUserManager; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; use Test\TestCase; @@ -41,17 +41,25 @@ protected function setUp(): void { $this->statsApi = $this->createMock(StatsAPIApi::class); $this->apiClientService = $this->createMock(ApiStatsClientService::class); - $this->apiClientService->method('newClient')->willReturn($this->createMock(Client::class)); $this->apiClientService->method('newStatsAPIApi')->willReturn($this->statsApi); - $this->configService = $this->createMock(PssConfigService::class); - $this->configService->method('getBrand')->willReturn('IONOS'); - $this->configService->method('getExtRef')->willReturn('test-ext-ref'); - $this->configService->method('getBaseUrl')->willReturn('https://pss.example.com'); - $this->configService->method('getUsername')->willReturn('user'); - $this->configService->method('getPassword')->willReturn('pass'); + $this->configService = $this->createConfigService('IONOS', 'test-ext-ref', 'https://pss.example.com', 'user', 'pass'); - $this->job = new UserStatsJob( + $this->job = $this->buildJob(); + } + + private function createConfigService(string $brand, string $extRef, string $baseUrl, string $username, string $password): PssConfigService&MockObject { + $config = $this->createMock(PssConfigService::class); + $config->method('getBrand')->willReturn($brand); + $config->method('getExtRef')->willReturn($extRef); + $config->method('getBaseUrl')->willReturn($baseUrl); + $config->method('getUsername')->willReturn($username); + $config->method('getPassword')->willReturn($password); + return $config; + } + + private function buildJob(): UserStatsJob { + return new UserStatsJob( $this->logger, $this->timeFactory, $this->userManager, @@ -65,10 +73,12 @@ public function testRunCallsApiWithCorrectPayload(): void { $this->logger->expects($this->once()) ->method('info') - ->with('User stats payload', $this->callback(function (array $context): bool { - return $context['payload']['users']['existingUsers'] === 42 - && preg_match('/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/', $context['payload']['timestamp']) === 1; + ->with('UserStatsJob: pushing user stats', $this->callback(function (array $context): bool { + return $context['existingUsers'] === 42 + && preg_match('/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/', $context['timestamp']) === 1; })); + $this->logger->expects($this->never())->method('error'); + $this->logger->expects($this->never())->method('warning'); $this->statsApi->expects($this->once()) ->method('updateStats') @@ -97,50 +107,21 @@ public function testRunLogsErrorWhenApiThrows(): void { $this->invokePrivate($this->job, 'run', [null]); } - public function testRunLogsErrorWhenRequiredConfigMissing(): void { - $this->userManager->method('countUsersTotal')->willReturn(5); - - $this->configService = $this->createMock(PssConfigService::class); - $this->configService->method('getBrand')->willReturn(''); - $this->configService->method('getExtRef')->willReturn('test-ext-ref'); - $this->configService->method('getBaseUrl')->willReturn('https://pss.example.com'); - $this->configService->method('getUsername')->willReturn('user'); - $this->configService->method('getPassword')->willReturn('pass'); - - $this->job = new UserStatsJob( - $this->logger, - $this->timeFactory, - $this->userManager, - $this->apiClientService, - $this->configService, - ); - - $this->logger->expects($this->once()) - ->method('error') - ->with('UserStatsJob: missing required PSS configuration, aborting'); - - $this->statsApi->expects($this->never())->method('updateStats'); - - $this->invokePrivate($this->job, 'run', [null]); + public static function provideMissingConfig(): array { + return [ + 'missing brand' => ['', 'test-ext-ref', 'https://pss.example.com', 'user', 'pass'], + 'missing ext_ref' => ['IONOS', '', 'https://pss.example.com', 'user', 'pass'], + 'missing baseUrl' => ['IONOS', 'test-ext-ref', '', 'user', 'pass'], + 'missing username' => ['IONOS', 'test-ext-ref', 'https://pss.example.com', '', 'pass'], + 'missing password' => ['IONOS', 'test-ext-ref', 'https://pss.example.com', 'user', ''], + ]; } - public function testRunLogsErrorWhenCredentialsMissing(): void { + #[DataProvider('provideMissingConfig')] + public function testRunLogsErrorWhenConfigMissing(string $brand, string $extRef, string $baseUrl, string $username, string $password): void { $this->userManager->method('countUsersTotal')->willReturn(5); - - $this->configService = $this->createMock(PssConfigService::class); - $this->configService->method('getBrand')->willReturn('IONOS'); - $this->configService->method('getExtRef')->willReturn('test-ext-ref'); - $this->configService->method('getBaseUrl')->willReturn('https://pss.example.com'); - $this->configService->method('getUsername')->willReturn(''); - $this->configService->method('getPassword')->willReturn('pass'); - - $this->job = new UserStatsJob( - $this->logger, - $this->timeFactory, - $this->userManager, - $this->apiClientService, - $this->configService, - ); + $this->configService = $this->createConfigService($brand, $extRef, $baseUrl, $username, $password); + $this->job = $this->buildJob(); $this->logger->expects($this->once()) ->method('error') From 143cc48f1706075182e415ea5ee7e3c3a9f85ce7 Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Fri, 15 May 2026 10:34:58 +0200 Subject: [PATCH 08/10] style(app-info): move vendor autoload include below use statements PSR-12 expects `use` statements directly under the namespace block, with file-level side-effect statements (require_once) ordered after. Cosmetic only; no runtime effect. Signed-off-by: Misha M.-Kupriyanov --- lib/AppInfo/Application.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 68c290f..0b34896 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -9,8 +9,6 @@ namespace OCA\NcwTools\AppInfo; -require_once __DIR__ . '/../../vendor/autoload.php'; - use OCA\NcwTools\Capabilities; use OCA\NcwTools\Listeners\InstallationCompletedEventListener; use OCA\NcwTools\Listeners\UserEventListener; @@ -22,6 +20,8 @@ use OCP\User\Events\UserCreatedEvent; use OCP\User\Events\UserDeletedEvent; +require_once __DIR__ . '/../../vendor/autoload.php'; + class Application extends App implements IBootstrap { public const APP_ID = 'ncw_tools'; From 063ebf108bd9828084fd84e5758c8f121dd2ce3f Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Fri, 15 May 2026 11:56:54 +0200 Subject: [PATCH 09/10] refactor(user-stats): split UserStatsJob into StatsReporter port + PSS adapter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UserStatsJob currently mixes the domain concern ("on cron, report the user count") with PSS transport details: it reads 5 config keys, builds vendor DTOs, instantiates the API client, and catches transport errors. The job becomes hard to test in isolation and gains a new vendor coupling every time PSS changes. Introduce a port and adapter under lib/Stats/: - StatsReporter — interface: reportUserCount(int, DateTimeInterface). - PssStatsReporter — the sole place that knows about PSS. Reads config, builds StatsUpdateRequest, calls updateStats(). The catch is narrowed to just updateStats() and logs exceptionClass + message (not 'exception' => $e) so deep vendor frames cannot leak the password through a serialized trace. PHP 8.1 has no #[\SensitiveParameter], so this is the available mitigation. - PssConfig — frozen DTO. password is a private field with a getter; __debugInfo() redacts it from var_dump/print_r/json/log. - PssConfigReader — atomic read returning ?PssConfig. On miss, one log line per cron tick naming the missing keys. - PssApiFactory — Guzzle + Configuration factory. Timeouts read from ncw_tools.pss.{connect_timeout,timeout} (defaults 5/10); ncw_tools.pss.allow_insecure (default false) flips Guzzle's TLS verification off for local debug against self-signed certs. UserStatsJob is now a ~10-line delegate that depends only on the StatsReporter interface. lib/Service/ApiStatsClientService.php and lib/Service/PssConfigService.php are deleted. Tests rewritten: UserStatsJobTest mocks StatsReporter only; new PssConfigReaderTest covers each missing-key case + password redaction; new PssStatsReporterTest covers null-config / happy / throwable paths and asserts the exception object is not leaked. Signed-off-by: Misha M.-Kupriyanov --- composer.json | 7 +- composer.lock | 9 +- lib/AppInfo/Application.php | 3 + lib/BackgroundJob/UserStatsJob.php | 52 ++------ lib/Service/ApiStatsClientService.php | 29 ----- lib/Service/PssConfigService.php | 45 ------- lib/Stats/PssApiFactory.php | 47 ++++++++ lib/Stats/PssConfig.php | 35 ++++++ lib/Stats/PssConfigReader.php | 55 +++++++++ lib/Stats/PssStatsReporter.php | 62 ++++++++++ lib/Stats/StatsReporter.php | 14 +++ psalm.xml | 3 + tests/unit/BackgroundJob/UserStatsJobTest.php | 106 +++------------- tests/unit/Stats/PssConfigReaderTest.php | 107 +++++++++++++++++ tests/unit/Stats/PssStatsReporterTest.php | 113 ++++++++++++++++++ 15 files changed, 476 insertions(+), 211 deletions(-) delete mode 100644 lib/Service/ApiStatsClientService.php delete mode 100644 lib/Service/PssConfigService.php create mode 100644 lib/Stats/PssApiFactory.php create mode 100644 lib/Stats/PssConfig.php create mode 100644 lib/Stats/PssConfigReader.php create mode 100644 lib/Stats/PssStatsReporter.php create mode 100644 lib/Stats/StatsReporter.php create mode 100644 tests/unit/Stats/PssConfigReaderTest.php create mode 100644 tests/unit/Stats/PssStatsReporterTest.php diff --git a/composer.json b/composer.json index dc6c4cd..6c48070 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,12 @@ "source": { "type": "git", "url": "https://github.com/ionos-productivity/ionos-ncpss-addons-api-client.git", - "reference": "2.0.0-20260429080405" + "reference": "a5f09eacaa3139e50e43ced4a44137946a8bfe18" + }, + "dist": { + "type": "zip", + "url": "https://github.com/ionos-productivity/ionos-ncpss-addons-api-client/archive/refs/tags/2.0.0-20260429080405.zip", + "reference": "a5f09eacaa3139e50e43ced4a44137946a8bfe18" }, "require": { "guzzlehttp/guzzle": "^7.3", diff --git a/composer.lock b/composer.lock index 6db10c5..72f3dbd 100644 --- a/composer.lock +++ b/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": "aff3ddb2be52e4825714064f03c35ceb", + "content-hash": "7ac7adafbdcb64d2d37d9c9e42108378", "packages": [ { "name": "bamarni/composer-bin-plugin", @@ -395,7 +395,12 @@ "source": { "type": "git", "url": "https://github.com/ionos-productivity/ionos-ncpss-addons-api-client.git", - "reference": "2.0.0-20260429080405" + "reference": "a5f09eacaa3139e50e43ced4a44137946a8bfe18" + }, + "dist": { + "type": "zip", + "url": "https://github.com/ionos-productivity/ionos-ncpss-addons-api-client/archive/refs/tags/2.0.0-20260429080405.zip", + "reference": "a5f09eacaa3139e50e43ced4a44137946a8bfe18" }, "require": { "guzzlehttp/guzzle": "^7.3", diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 0b34896..5a34c44 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -12,6 +12,8 @@ use OCA\NcwTools\Capabilities; use OCA\NcwTools\Listeners\InstallationCompletedEventListener; use OCA\NcwTools\Listeners\UserEventListener; +use OCA\NcwTools\Stats\PssStatsReporter; +use OCA\NcwTools\Stats\StatsReporter; use OCP\AppFramework\App; use OCP\AppFramework\Bootstrap\IBootContext; use OCP\AppFramework\Bootstrap\IBootstrap; @@ -34,6 +36,7 @@ public function register(IRegistrationContext $context): void { $context->registerEventListener(UserCreatedEvent::class, UserEventListener::class); $context->registerEventListener(UserDeletedEvent::class, UserEventListener::class); $context->registerCapability(Capabilities::class); + $context->registerServiceAlias(StatsReporter::class, PssStatsReporter::class); } public function boot(IBootContext $context): void { diff --git a/lib/BackgroundJob/UserStatsJob.php b/lib/BackgroundJob/UserStatsJob.php index c851acc..c68a617 100644 --- a/lib/BackgroundJob/UserStatsJob.php +++ b/lib/BackgroundJob/UserStatsJob.php @@ -9,10 +9,7 @@ namespace OCA\NcwTools\BackgroundJob; -use IONOS\NextcloudPSS\AddonsAPI\Client\Model\StatsUpdateRequest; -use IONOS\NextcloudPSS\AddonsAPI\Client\Model\UserStats; -use OCA\NcwTools\Service\ApiStatsClientService; -use OCA\NcwTools\Service\PssConfigService; +use OCA\NcwTools\Stats\StatsReporter; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\QueuedJob; use OCP\IUserManager; @@ -22,53 +19,22 @@ class UserStatsJob extends QueuedJob { public function __construct( private LoggerInterface $logger, - private ITimeFactory $timeFactory, + ITimeFactory $timeFactory, private IUserManager $userManager, - private ApiStatsClientService $apiClientService, - private PssConfigService $configService, + private StatsReporter $reporter, ) { parent::__construct($timeFactory); } protected function run(mixed $argument): void { - $userTotalCount = $this->userManager->countUsersTotal(); - if ($userTotalCount === false) { + $count = $this->userManager->countUsersTotal(); + if ($count === false) { $this->logger->warning('UserStatsJob: could not retrieve user count'); return; } - - $brand = $this->configService->getBrand(); - $extRef = $this->configService->getExtRef(); - $baseUrl = $this->configService->getBaseUrl(); - $username = $this->configService->getUsername(); - $password = $this->configService->getPassword(); - - if ($brand === '' || $extRef === '' || $baseUrl === '' || $username === '' || $password === '') { - $this->logger->error('UserStatsJob: missing required PSS configuration, aborting'); - return; - } - - $timestamp = $this->timeFactory->getDateTime('now', new \DateTimeZone('UTC')); - - $userStats = new UserStats(); - $userStats->setExistingUsers($userTotalCount); - - $request = new StatsUpdateRequest(); - $request->setTimestamp($timestamp); - $request->setUsers($userStats); - - $this->logger->info('UserStatsJob: pushing user stats', [ - 'existingUsers' => $userTotalCount, - 'timestamp' => $timestamp->format('Y-m-d\TH:i:s.v\Z'), - ]); - - try { - $api = $this->apiClientService->newStatsAPIApi($baseUrl, $username, $password); - $api->updateStats($brand, $extRef, $request); - } catch (\Throwable $e) { - $this->logger->error('UserStatsJob: failed to push stats to PSS', [ - 'exception' => $e, - ]); - } + $this->reporter->reportUserCount( + $count, + $this->time->getDateTime('now', new \DateTimeZone('UTC')), + ); } } diff --git a/lib/Service/ApiStatsClientService.php b/lib/Service/ApiStatsClientService.php deleted file mode 100644 index 09a76eb..0000000 --- a/lib/Service/ApiStatsClientService.php +++ /dev/null @@ -1,29 +0,0 @@ - 5, - 'timeout' => 10, - ]); - $config = new Configuration(); - $config->setHost($baseUrl); - $config->setUsername($username); - $config->setPassword($password); - return new StatsAPIApi($client, $config); - } -} diff --git a/lib/Service/PssConfigService.php b/lib/Service/PssConfigService.php deleted file mode 100644 index 84adae3..0000000 --- a/lib/Service/PssConfigService.php +++ /dev/null @@ -1,45 +0,0 @@ -config->getSystemValueString(self::KEY_BRAND); - } - - public function getExtRef(): string { - return $this->config->getSystemValueString(self::KEY_EXT_REF); - } - - public function getBaseUrl(): string { - return $this->config->getSystemValueString(self::KEY_BASE_URL); - } - - public function getUsername(): string { - return $this->config->getSystemValueString(self::KEY_USERNAME); - } - - public function getPassword(): string { - return $this->config->getSystemValueString(self::KEY_PASSWORD); - } -} diff --git a/lib/Stats/PssApiFactory.php b/lib/Stats/PssApiFactory.php new file mode 100644 index 0000000..dbeab02 --- /dev/null +++ b/lib/Stats/PssApiFactory.php @@ -0,0 +1,47 @@ +connectTimeout = $config->getSystemValueInt(self::KEY_CONNECT_TIMEOUT, self::DEFAULT_CONNECT_TIMEOUT_S); + $this->timeout = $config->getSystemValueInt(self::KEY_TIMEOUT, self::DEFAULT_TIMEOUT_S); + $this->allowInsecure = $config->getSystemValueBool(self::KEY_ALLOW_INSECURE, false); + } + + public function newStatsApi(string $baseUrl, string $username, string $password): StatsAPIApi { + $client = new Client([ + 'connect_timeout' => $this->connectTimeout, + 'timeout' => $this->timeout, + 'verify' => !$this->allowInsecure, + ]); + $config = new Configuration(); + $config->setHost($baseUrl); + $config->setUsername($username); + $config->setPassword($password); + return new StatsAPIApi($client, $config); + } +} diff --git a/lib/Stats/PssConfig.php b/lib/Stats/PssConfig.php new file mode 100644 index 0000000..3b2181a --- /dev/null +++ b/lib/Stats/PssConfig.php @@ -0,0 +1,35 @@ +password; + } + + public function __debugInfo(): array { + return [ + 'brand' => $this->brand, + 'extRef' => $this->extRef, + 'baseUrl' => $this->baseUrl, + 'username' => $this->username, + 'password' => '***', + ]; + } +} diff --git a/lib/Stats/PssConfigReader.php b/lib/Stats/PssConfigReader.php new file mode 100644 index 0000000..f1aa6fb --- /dev/null +++ b/lib/Stats/PssConfigReader.php @@ -0,0 +1,55 @@ + 'ncw_tools.pss.brand', + 'extRef' => 'ncw_tools.pss.ext_ref', + 'baseUrl' => 'ncw_tools.pss.base_url', + 'username' => 'ncw_tools.pss.username', + 'password' => 'ncw_tools.pss.password', + ]; + + public function __construct( + private IConfig $config, + private LoggerInterface $logger, + ) { + } + + public function read(): ?PssConfig { + $values = []; + $missing = []; + foreach (self::KEYS as $field => $key) { + $value = $this->config->getSystemValueString($key); + if ($value === '') { + $missing[] = $key; + } + $values[$field] = $value; + } + if ($missing !== []) { + $this->logger->error( + 'PssConfigReader: missing required PSS configuration', + ['keys' => $missing], + ); + return null; + } + return new PssConfig( + $values['brand'], + $values['extRef'], + $values['baseUrl'], + $values['username'], + $values['password'], + ); + } +} diff --git a/lib/Stats/PssStatsReporter.php b/lib/Stats/PssStatsReporter.php new file mode 100644 index 0000000..ff428c4 --- /dev/null +++ b/lib/Stats/PssStatsReporter.php @@ -0,0 +1,62 @@ +configReader->read(); + if ($config === null) { + return; + } + + $userStats = new UserStats(); + $userStats->setExistingUsers($count); + + $request = new StatsUpdateRequest(); + $request->setTimestamp($at instanceof \DateTime ? $at : \DateTime::createFromInterface($at)); + $request->setUsers($userStats); + + $api = $this->apiFactory->newStatsApi( + $config->baseUrl, + $config->username, + $config->getPassword(), + ); + + // Narrow the catch to just updateStats() and log message-only (not the + // full exception). Deep vendor frames can carry credential strings in + // their stack-trace args; PHP 8.1 has no #[\SensitiveParameter] to + // scrub them. We trade trace richness for credential safety. + try { + $api->updateStats($config->brand, $config->extRef, $request); + } catch (\Throwable $e) { + $this->logger->error('PssStatsReporter: failed to push stats to PSS', [ + 'exceptionClass' => $e::class, + 'message' => $e->getMessage(), + ]); + return; + } + + $this->logger->info('PssStatsReporter: pushed user stats', [ + 'existingUsers' => $count, + 'timestamp' => $at->format('Y-m-d\TH:i:s.v\Z'), + ]); + } +} diff --git a/lib/Stats/StatsReporter.php b/lib/Stats/StatsReporter.php new file mode 100644 index 0000000..3f59f8b --- /dev/null +++ b/lib/Stats/StatsReporter.php @@ -0,0 +1,14 @@ + + + + diff --git a/tests/unit/BackgroundJob/UserStatsJobTest.php b/tests/unit/BackgroundJob/UserStatsJobTest.php index 809ba0d..f888767 100644 --- a/tests/unit/BackgroundJob/UserStatsJobTest.php +++ b/tests/unit/BackgroundJob/UserStatsJobTest.php @@ -10,14 +10,10 @@ namespace OCA\NcwTools\Tests\Unit\BackgroundJob; use DateTime; -use IONOS\NextcloudPSS\AddonsAPI\Client\Api\StatsAPIApi; -use IONOS\NextcloudPSS\AddonsAPI\Client\Model\StatsUpdateRequest; use OCA\NcwTools\BackgroundJob\UserStatsJob; -use OCA\NcwTools\Service\ApiStatsClientService; -use OCA\NcwTools\Service\PssConfigService; +use OCA\NcwTools\Stats\StatsReporter; use OCP\AppFramework\Utility\ITimeFactory; use OCP\IUserManager; -use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; use Test\TestCase; @@ -26,9 +22,7 @@ class UserStatsJobTest extends TestCase { private LoggerInterface&MockObject $logger; private ITimeFactory&MockObject $timeFactory; private IUserManager&MockObject $userManager; - private ApiStatsClientService&MockObject $apiClientService; - private PssConfigService&MockObject $configService; - private StatsAPIApi&MockObject $statsApi; + private StatsReporter&MockObject $reporter; private UserStatsJob $job; protected function setUp(): void { @@ -36,110 +30,40 @@ protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->timeFactory = $this->createMock(ITimeFactory::class); - $this->timeFactory->method('getDateTime')->willReturn(new DateTime('2026-01-01T00:00:00.000 UTC')); $this->userManager = $this->createMock(IUserManager::class); + $this->reporter = $this->createMock(StatsReporter::class); - $this->statsApi = $this->createMock(StatsAPIApi::class); - $this->apiClientService = $this->createMock(ApiStatsClientService::class); - $this->apiClientService->method('newStatsAPIApi')->willReturn($this->statsApi); - - $this->configService = $this->createConfigService('IONOS', 'test-ext-ref', 'https://pss.example.com', 'user', 'pass'); - - $this->job = $this->buildJob(); - } - - private function createConfigService(string $brand, string $extRef, string $baseUrl, string $username, string $password): PssConfigService&MockObject { - $config = $this->createMock(PssConfigService::class); - $config->method('getBrand')->willReturn($brand); - $config->method('getExtRef')->willReturn($extRef); - $config->method('getBaseUrl')->willReturn($baseUrl); - $config->method('getUsername')->willReturn($username); - $config->method('getPassword')->willReturn($password); - return $config; - } - - private function buildJob(): UserStatsJob { - return new UserStatsJob( + $this->job = new UserStatsJob( $this->logger, $this->timeFactory, $this->userManager, - $this->apiClientService, - $this->configService, + $this->reporter, ); } - public function testRunCallsApiWithCorrectPayload(): void { + public function testRunDelegatesToReporterOnSuccess(): void { + $now = new DateTime('2026-01-01T00:00:00.000', new \DateTimeZone('UTC')); $this->userManager->method('countUsersTotal')->willReturn(42); + $this->timeFactory->method('getDateTime')->willReturn($now); - $this->logger->expects($this->once()) - ->method('info') - ->with('UserStatsJob: pushing user stats', $this->callback(function (array $context): bool { - return $context['existingUsers'] === 42 - && preg_match('/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/', $context['timestamp']) === 1; - })); - $this->logger->expects($this->never())->method('error'); - $this->logger->expects($this->never())->method('warning'); - - $this->statsApi->expects($this->once()) - ->method('updateStats') - ->with( - 'IONOS', - 'test-ext-ref', - $this->callback(function (StatsUpdateRequest $req): bool { - return $req->getUsers()?->getExistingUsers() === 42; - }) - ); + $this->reporter->expects($this->once()) + ->method('reportUserCount') + ->with(42, $now); - $this->invokePrivate($this->job, 'run', [null]); - } - - public function testRunLogsErrorWhenApiThrows(): void { - $this->userManager->method('countUsersTotal')->willReturn(42); - $this->statsApi->method('updateStats')->willThrowException(new \Exception('connection refused')); - - $this->logger->expects($this->once()) - ->method('error') - ->with('UserStatsJob: failed to push stats to PSS', $this->callback(function (array $ctx): bool { - return $ctx['exception'] instanceof \Exception - && str_contains($ctx['exception']->getMessage(), 'connection refused'); - })); - - $this->invokePrivate($this->job, 'run', [null]); - } - - public static function provideMissingConfig(): array { - return [ - 'missing brand' => ['', 'test-ext-ref', 'https://pss.example.com', 'user', 'pass'], - 'missing ext_ref' => ['IONOS', '', 'https://pss.example.com', 'user', 'pass'], - 'missing baseUrl' => ['IONOS', 'test-ext-ref', '', 'user', 'pass'], - 'missing username' => ['IONOS', 'test-ext-ref', 'https://pss.example.com', '', 'pass'], - 'missing password' => ['IONOS', 'test-ext-ref', 'https://pss.example.com', 'user', ''], - ]; - } - - #[DataProvider('provideMissingConfig')] - public function testRunLogsErrorWhenConfigMissing(string $brand, string $extRef, string $baseUrl, string $username, string $password): void { - $this->userManager->method('countUsersTotal')->willReturn(5); - $this->configService = $this->createConfigService($brand, $extRef, $baseUrl, $username, $password); - $this->job = $this->buildJob(); - - $this->logger->expects($this->once()) - ->method('error') - ->with('UserStatsJob: missing required PSS configuration, aborting'); - - $this->statsApi->expects($this->never())->method('updateStats'); + $this->logger->expects($this->never())->method('warning'); + $this->logger->expects($this->never())->method('error'); $this->invokePrivate($this->job, 'run', [null]); } - public function testRunLogsWarningWhenCountFalse(): void { + public function testRunLogsWarningAndSkipsReporterWhenCountFalse(): void { $this->userManager->method('countUsersTotal')->willReturn(false); $this->logger->expects($this->once()) ->method('warning') ->with('UserStatsJob: could not retrieve user count'); - $this->statsApi->expects($this->never())->method('updateStats'); + $this->reporter->expects($this->never())->method('reportUserCount'); $this->invokePrivate($this->job, 'run', [null]); } diff --git a/tests/unit/Stats/PssConfigReaderTest.php b/tests/unit/Stats/PssConfigReaderTest.php new file mode 100644 index 0000000..19dee66 --- /dev/null +++ b/tests/unit/Stats/PssConfigReaderTest.php @@ -0,0 +1,107 @@ +config = $this->createMock(IConfig::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->reader = new PssConfigReader($this->config, $this->logger); + } + + public function testReadReturnsConfigWhenAllKeysPresent(): void { + $this->stubValues([ + 'ncw_tools.pss.brand' => 'IONOS', + 'ncw_tools.pss.ext_ref' => 'tenant-1', + 'ncw_tools.pss.base_url' => 'https://pss.example.com', + 'ncw_tools.pss.username' => 'alice', + 'ncw_tools.pss.password' => 'secret', + ]); + + $this->logger->expects($this->never())->method('error'); + + $result = $this->reader->read(); + $this->assertNotNull($result); + $this->assertSame('IONOS', $result->brand); + $this->assertSame('tenant-1', $result->extRef); + $this->assertSame('https://pss.example.com', $result->baseUrl); + $this->assertSame('alice', $result->username); + $this->assertSame('secret', $result->getPassword()); + } + + public static function provideMissingKey(): array { + $full = [ + 'ncw_tools.pss.brand' => 'IONOS', + 'ncw_tools.pss.ext_ref' => 'tenant-1', + 'ncw_tools.pss.base_url' => 'https://pss.example.com', + 'ncw_tools.pss.username' => 'alice', + 'ncw_tools.pss.password' => 'secret', + ]; + $cases = []; + foreach (array_keys($full) as $missing) { + $values = $full; + $values[$missing] = ''; + $cases[$missing] = [$values, [$missing]]; + } + $cases['all missing'] = [ + array_fill_keys(array_keys($full), ''), + array_keys($full), + ]; + return $cases; + } + + #[DataProvider('provideMissingKey')] + public function testReadReturnsNullAndLogsMissingKeys(array $values, array $expectedMissing): void { + $this->stubValues($values); + + $this->logger->expects($this->once()) + ->method('error') + ->with( + 'PssConfigReader: missing required PSS configuration', + $this->callback(fn (array $ctx): bool => $ctx['keys'] === $expectedMissing), + ); + + $this->assertNull($this->reader->read()); + } + + public function testPasswordRedactedFromDebugInfo(): void { + $this->stubValues([ + 'ncw_tools.pss.brand' => 'IONOS', + 'ncw_tools.pss.ext_ref' => 'tenant-1', + 'ncw_tools.pss.base_url' => 'https://pss.example.com', + 'ncw_tools.pss.username' => 'alice', + 'ncw_tools.pss.password' => 'super-secret-value', + ]); + $config = $this->reader->read(); + $this->assertNotNull($config); + + $dump = print_r($config, true); + $this->assertStringNotContainsString('super-secret-value', $dump); + $this->assertStringContainsString('***', $dump); + } + + private function stubValues(array $values): void { + $this->config + ->method('getSystemValueString') + ->willReturnCallback(fn (string $key, string $default = '') => $values[$key] ?? $default); + } +} diff --git a/tests/unit/Stats/PssStatsReporterTest.php b/tests/unit/Stats/PssStatsReporterTest.php new file mode 100644 index 0000000..4722311 --- /dev/null +++ b/tests/unit/Stats/PssStatsReporterTest.php @@ -0,0 +1,113 @@ +configReader = $this->createMock(PssConfigReader::class); + $this->apiFactory = $this->createMock(PssApiFactory::class); + $this->statsApi = $this->createMock(StatsAPIApi::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->apiFactory->method('newStatsApi')->willReturn($this->statsApi); + + $this->reporter = new PssStatsReporter( + $this->configReader, + $this->apiFactory, + $this->logger, + ); + } + + public function testReportSkipsWhenConfigNull(): void { + $this->configReader->method('read')->willReturn(null); + + $this->statsApi->expects($this->never())->method('updateStats'); + $this->logger->expects($this->never())->method('error'); + $this->logger->expects($this->never())->method('info'); + + $this->reporter->reportUserCount(42, new DateTime('2026-01-01T00:00:00.000', new \DateTimeZone('UTC'))); + } + + public function testReportPostsAndLogsOnSuccess(): void { + $at = new DateTime('2026-01-01T12:34:56.789', new \DateTimeZone('UTC')); + $this->configReader->method('read')->willReturn(new PssConfig( + 'IONOS', + 'tenant-1', + 'https://pss.example.com', + 'alice', + 'secret', + )); + + $this->apiFactory->expects($this->once()) + ->method('newStatsApi') + ->with('https://pss.example.com', 'alice', 'secret') + ->willReturn($this->statsApi); + + $this->statsApi->expects($this->once()) + ->method('updateStats') + ->with( + 'IONOS', + 'tenant-1', + $this->callback(function (StatsUpdateRequest $req) use ($at): bool { + return $req->getUsers()?->getExistingUsers() === 42 + && $req->getTimestamp() == $at; + }), + ); + + $this->logger->expects($this->once()) + ->method('info') + ->with('PssStatsReporter: pushed user stats', $this->callback(function (array $ctx): bool { + return $ctx['existingUsers'] === 42 + && $ctx['timestamp'] === '2026-01-01T12:34:56.789Z'; + })); + $this->logger->expects($this->never())->method('error'); + + $this->reporter->reportUserCount(42, $at); + } + + public function testReportLogsErrorOnThrowableWithoutLeakingException(): void { + $this->configReader->method('read')->willReturn(new PssConfig( + 'IONOS', + 'tenant-1', + 'https://pss.example.com', + 'alice', + 'secret', + )); + $this->statsApi->method('updateStats')->willThrowException(new \RuntimeException('connection refused')); + + $this->logger->expects($this->once()) + ->method('error') + ->with('PssStatsReporter: failed to push stats to PSS', $this->callback(function (array $ctx): bool { + return $ctx['exceptionClass'] === \RuntimeException::class + && $ctx['message'] === 'connection refused' + && !array_key_exists('exception', $ctx); + })); + $this->logger->expects($this->never())->method('info'); + + $this->reporter->reportUserCount(42, new DateTime('2026-01-01T00:00:00.000', new \DateTimeZone('UTC'))); + } +} From fdee03450476f85ab1c77376790853b8f58d130f Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Fri, 15 May 2026 10:18:15 +0200 Subject: [PATCH 10/10] docs(user-stats): update flow diagram for StatsReporter refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the old UserStatsJob → API mermaid with the new flow: UserStatsJob → StatsReporter → PssConfigReader / PssApiFactory → PSS. Add the optional configuration table (connect_timeout, timeout, allow_insecure) and a security-notes section covering credential exposure in config.php, shell-history hygiene for occ config:system:set, and the in-code mitigations (PssConfig redaction, narrowed catch). Signed-off-by: Misha M.-Kupriyanov --- docs/events/user-stats.md | 60 ++++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/docs/events/user-stats.md b/docs/events/user-stats.md index 572ea94..afcdee9 100644 --- a/docs/events/user-stats.md +++ b/docs/events/user-stats.md @@ -1,6 +1,6 @@ # User stats reporting -On every user create or delete, this app reports the current total user count to the PSS Stats API. A listener logs the change and enqueues a single deduplicated background job; on the next cron tick the job reads the total user count, validates the PSS credentials, and POSTs a `StatsUpdateRequest` to the PSS Stats API. The job is queued — it does not retry on failure; the next user event re-enqueues it. +On every user create or delete, this app reports the current total user count to the PSS Stats API. A listener logs the change and enqueues a single deduplicated background job; on the next cron tick the job reads the total user count and delegates to a `StatsReporter` (the PSS adapter validates configuration, builds the `StatsUpdateRequest`, and POSTs it to the PSS Stats API). The job is queued — it does not retry on failure; the next user event re-enqueues it. ## Trigger events @@ -9,7 +9,9 @@ On every user create or delete, this app reports the current total user count to ## Configuration -All keys live in `config/config.php`. The job aborts (with a logged error) if any value is missing or empty. +All keys live in `config/config.php`. The adapter aborts (with a logged error listing the missing keys) if any required value is missing or empty. + +### Required | Key | Type | Purpose | | --- | --- | --- | @@ -19,6 +21,23 @@ All keys live in `config/config.php`. The job aborts (with a logged error) if an | `ncw_tools.pss.username` | string | HTTP Basic auth username. | | `ncw_tools.pss.password` | string | HTTP Basic auth password. | +### Optional + +| Key | Type | Default | Purpose | +| --- | --- | --- | --- | +| `ncw_tools.pss.connect_timeout` | int (seconds) | `5` | Guzzle connect timeout. | +| `ncw_tools.pss.timeout` | int (seconds) | `10` | Guzzle overall timeout. | +| `ncw_tools.pss.allow_insecure` | bool | `false` | When `true`, disables TLS verification (`verify => false`). **Dev/debug only** — never enable in production. | + +### Security notes for `ncw_tools.pss.password` + +The password is stored in `config/config.php` in plaintext, same trust model as the DB password already in that file. Two operational considerations: + +- **Shell history / `ps` exposure.** `occ config:system:set` puts the value on the command line — visible in `ps` and shell history. Prefer a leading-space command (with `HISTCONTROL=ignorespace` set) or read the value from an env var so it does not appear in process listings. +- **File permissions.** `config/config.php` should be `0640 root:www-data`. Audit on deploy. + +Code-side, the `PssConfig` DTO redacts `password` via `__debugInfo()`, and the PSS error handler logs `exceptionClass + message` only (not the full trace), so accidental serialization or deep-vendor stack-traces will not leak the value. + ## Flow ```mermaid @@ -33,8 +52,9 @@ sequenceDiagram participant CRON as Nextcloud Cron participant USJ as UserStatsJob participant UM as IUserManager - participant CFG as PssConfigService - participant API as ApiStatsClientService + participant SR as StatsReporter
(PssStatsReporter) + participant CR as PssConfigReader + participant AF as PssApiFactory participant PSS as PSS Stats API Note over OCC,JL: Synchronous phase — runs during the OCC command @@ -67,23 +87,25 @@ sequenceDiagram USJ->>LOG: warning("could not retrieve user count") USJ-->>CRON: return else count is int - USJ->>CFG: getBrand / getExtRef / getBaseUrl / getUsername / getPassword + USJ->>SR: reportUserCount(count, now) + SR->>CR: read() - alt any value empty - USJ->>LOG: error("missing required PSS configuration, aborting") - USJ-->>CRON: return + alt any required key missing + CR->>LOG: error("missing required PSS configuration", {keys}) + CR-->>SR: null + SR-->>USJ: return else all values present - USJ->>USJ: build timestamp (UTC ISO-8601 ms)
build StatsUpdateRequest with UserStats(existingUsers) - USJ->>LOG: info("User stats payload", {payload}) - USJ->>API: newClient() - USJ->>API: newStatsAPIApi(client, baseUrl, username, password) - USJ->>PSS: updateStats(brand, extRef, request) + CR-->>SR: PssConfig + SR->>SR: build StatsUpdateRequest with UserStats(existingUsers) + SR->>AF: newStatsApi(baseUrl, username, password) + SR->>PSS: updateStats(brand, extRef, request) alt Throwable - PSS-->>USJ: exception - USJ->>LOG: error("failed to push stats to PSS", {exception}) + PSS-->>SR: exception + SR->>LOG: error("failed to push stats to PSS", {exceptionClass, message}) else success - PSS-->>USJ: 2xx + PSS-->>SR: 2xx + SR->>LOG: info("pushed user stats", {existingUsers, timestamp}) end end end @@ -93,8 +115,8 @@ sequenceDiagram The job is a `QueuedJob` — there is no automatic retry. Each of the following ends the run without reporting: -- `IUserManager::countUsersTotal()` returns `false` (logged at warning). -- Any of the five `ncw_tools.pss.*` values is missing or empty (logged at error). -- The PSS API call throws (logged at error with the exception). +- `IUserManager::countUsersTotal()` returns `false` (logged at warning by the job). +- Any of the five required `ncw_tools.pss.*` values is missing or empty (logged at error by `PssConfigReader`, naming the missing keys). +- The PSS API call throws (logged at error by `PssStatsReporter` with `exceptionClass` + `message`). The next `UserCreatedEvent` or `UserDeletedEvent` re-enqueues the job, so the count converges once the underlying problem is resolved.