feat(UserStatsJobs): wire UserStatsJob to send stats to PSS via API client by Arsalanulhaq · Pull Request #18 · IONOS-Productivity/ncw-tools

Arsalanulhaq · 2026-05-05T16:09:40Z

Summary

Replaces the log-only payload in UserStatsJob with an actual HTTP PUT call to the NextcloudPSS service using ionos-ncpss-addons-api-client
Brand, extRef, and PSS credentials are read from system config keys (ncw_tools.pss.*)
Log statement is kept alongside the API call for observability
Errors are caught and logged without retrying (PSS has server-side idempotency)

Test plan

1. Set system config keys (inside the container)

occ config:system:set ncw_tools.pss.base_url --value='https://productivityqa.icaas.server.lan:10443/nextcloud'
occ config:system:set ncw_tools.pss.brand --value='IONOS'
occ config:system:set ncw_tools.pss.ext_ref --value='<extRef>'
occ config:system:set ncw_tools.pss.username --value='<username>'
occ config:system:set ncw_tools.pss.password --value='<password>'

2. Temporary code change required for local testing (self-signed cert)

PSS uses a self-signed certificate locally. Guzzle will reject it by default, so these two temporary changes are needed — do not commit:

lib/Service/PssConfigService.php — add:

public function getAllowInsecure(): bool {
    return $this->config->getSystemValueBool('ncw_tools.pss.allow_insecure', false);
}

lib/Service/ApiStatsClientService.php — change newClient() to:

public function newClient(bool $allowInsecure = false): Client {
    return new Client(['verify' => !$allowInsecure]);
}

lib/BackgroundJob/UserStatsJob.php — change the newClient() call to:

$client = $this->apiClientService->newClient($this->configService->getAllowInsecure());

Then set the config flag:

occ config:system:set ncw_tools.pss.allow_insecure --value=true --type=boolean

3. Trigger the flow

NC_PASS='MyLongPass99@dev' occ user:add --password-from-env testuser_$(date +%s)

4. Run the queued job

# Find the job ID (possibly the last created job-id)
occ background-job:list --output=json | jq

# Execute it
occ background-job:execute --force-execute <id>

5. Verify

# Check log for payload + no errors
grep 'User stats payload\|failed to push\|UserStatsJob' /var/www/html/data/nextcloud.log | tail -5

# Verify PSS received the stats
curl -k -u '<username>:<password>' -X GET \
  'https://productivityqa.icaas.server.lan:10443/nextcloud/config/IONOS?extRef=<extRef>' \
  -H 'accept: application/json'

The response should contain a currentUsers value matching the count from the log payload.

Uses configuration from feat(ncw_tools-app.config.php): add configuration for ncw_tools with … ncw-config#162

Copilot

Pull request overview

This PR updates NCW Tools’ UserStatsJob to push user-count stats to NextcloudPSS via the ionos-ncpss-addons-api-client, introducing small service wrappers for client creation and PSS config access.

Changes:

Replaced log-only behavior in UserStatsJob with an API call (updateStats) to PSS while keeping payload logging.
Added PssConfigService (system-config reader) and ApiStatsClientService (API client factory).
Added the new API client dependency to Composer and updated unit tests accordingly.

Reviewed changes

Copilot reviewed 7 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`lib/BackgroundJob/UserStatsJob.php`	Builds a PSS stats request and performs the `updateStats` API call with error logging.
`lib/Service/PssConfigService.php`	Reads PSS-related system config values used by the job.
`lib/Service/ApiStatsClientService.php`	Constructs the Guzzle client and Stats API wrapper from the generated client library.
`lib/AppInfo/Application.php`	Adds explicit `vendor/autoload.php` loading so the generated client classes resolve.
`tests/unit/BackgroundJob/UserStatsJobTest.php`	Updates/extends unit tests to verify the API call and error logging behavior.
`composer.json`	Adds the `ionos-ncpss-addons-api-client` repository + requirement; adjusts post-update bin behavior.
`composer.lock`	Locks new dependency set (including Guzzle + PSR packages and the IONOS client).
`psalm.xml`	Adds constructor references for Psalm’s container reflection setup.
`.gitignore`	Ignores a local directory related to the API client repo.

Copilot

Pull request overview

Copilot reviewed 7 out of 9 changed files in this pull request and generated 4 comments.

+		$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;
+		}


+if (file_exists(__DIR__ . '/../../vendor/autoload.php')) {
+	require_once __DIR__ . '/../../vendor/autoload.php';
+}


 		],
 		"post-update-cmd": [
-			"@composer bin all install --ansi"
+			"@composer bin all update --ansi"


Copilot

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

lib/BackgroundJob/UserStatsJob.php:76

The error log stores only $e->getMessage() under the exception key. This drops the exception type and stack trace, making production debugging harder. Prefer logging the throwable itself (or include class + trace) so the logger can capture full context.

		} catch (\Throwable $e) {
			$this->logger->error('UserStatsJob: failed to push stats to PSS', [
				'exception' => $e->getMessage(),
			]);

+if (file_exists(__DIR__ . '/../../vendor/autoload.php')) {
+	require_once __DIR__ . '/../../vendor/autoload.php';
+}
+
 use OCA\NcwTools\Capabilities;


+		$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(),
+			);


+		$value = $this->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;
+	}


+			"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"
+				},
+				"autoload": {
+					"psr-4": {
+						"IONOS\\NextcloudPSS\\AddonsAPI\\Client\\" : "lib/"
+					}
+				}
+			}


Copilot

Pull request overview

Copilot reviewed 15 out of 17 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

lib/Stats/PssStatsReporter.php:60

The log formats the timestamp with a literal Z suffix even if $at is not in UTC, which can produce misleading logs. Consider formatting from a UTC-normalized timestamp (or include the real offset) so observability matches what is sent to PSS.

		$this->logger->info('PssStatsReporter: pushed user stats', [
			'existingUsers' => $count,
			'timestamp' => $at->format('Y-m-d\TH:i:s.v\Z'),
		]);

+		$request = new StatsUpdateRequest();
+		$request->setTimestamp($at instanceof \DateTime ? $at : \DateTime::createFromInterface($at));
+		$request->setUsers($userStats);


+
+	private const DEFAULT_CONNECT_TIMEOUT_S = 5;
+	private const DEFAULT_TIMEOUT_S = 10;
+
+	private int $connectTimeout;
+	private int $timeout;
+	private bool $allowInsecure;
+
+	public function __construct(IConfig $config) {
+		$this->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);
+	}


 use OCA\NcwTools\Listeners\UserEventListener;
+use OCA\NcwTools\Stats\PssStatsReporter;
+use OCA\NcwTools\Stats\StatsReporter;


@@ -0,0 +1,122 @@
+# 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 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.


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.

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

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.

…ires 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 <kupriyanov@strato.de>

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 <kupriyanov@strato.de>

…ases 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 <kupriyanov@strato.de>

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 <kupriyanov@strato.de>

…S adapter 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 <kupriyanov@strato.de>

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 <kupriyanov@strato.de>

Copilot

Pull request overview

Copilot reviewed 16 out of 18 changed files in this pull request and generated 5 comments.

@@ -0,0 +1,122 @@
+# 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 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.


+		$this->reporter->reportUserCount(42, new DateTime('2026-01-01T00:00:00.000', new \DateTimeZone('UTC')));
+	}
+
+	public function testReportPostsAndLogsOnSuccess(): void {


+require_once __DIR__ . '/../../vendor/autoload.php';
+


+class PssApiFactory {
+	private const KEY_CONNECT_TIMEOUT = 'ncw_tools.pss.connect_timeout';
+	private const KEY_TIMEOUT = 'ncw_tools.pss.timeout';
+	private const KEY_ALLOW_INSECURE = 'ncw_tools.pss.allow_insecure';
+
+	private const DEFAULT_CONNECT_TIMEOUT_S = 5;
+	private const DEFAULT_TIMEOUT_S = 10;
+
+	private int $connectTimeout;
+	private int $timeout;
+	private bool $allowInsecure;
+
+	public function __construct(IConfig $config) {
+		$this->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,
+		]);


+		$this->logger->info('PssStatsReporter: pushed user stats', [
+			'existingUsers' => $count,
+			'timestamp' => $at->format('Y-m-d\TH:i:s.v\Z'),
+		]);


Arsalanulhaq force-pushed the as/dev/NSW-878-wire-user-stats-api branch 2 times, most recently from a273b83 to 8810d6e Compare May 6, 2026 10:34

Arsalanulhaq requested a review from Copilot May 6, 2026 10:49

Copilot started reviewing on behalf of Arsalanulhaq May 6, 2026 10:50 View session

Copilot AI reviewed May 6, 2026

View reviewed changes

Arsalanulhaq force-pushed the as/dev/NSW-876-user-event-listener branch from b88779b to f08c24c Compare May 6, 2026 14:23

Arsalanulhaq force-pushed the as/dev/NSW-878-wire-user-stats-api branch 2 times, most recently from e3d658d to d9f0fbb Compare May 6, 2026 14:36

bromiesTM force-pushed the as/dev/NSW-878-wire-user-stats-api branch from 82d8543 to acfcd5d Compare May 11, 2026 08:58

bromiesTM requested a review from Copilot May 11, 2026 08:59

Copilot started reviewing on behalf of bromiesTM May 11, 2026 09:00 View session

Copilot AI reviewed May 11, 2026

View reviewed changes

printminion-co force-pushed the as/dev/NSW-876-user-event-listener branch 3 times, most recently from 442ccb4 to 9434d93 Compare May 13, 2026 15:19

Base automatically changed from as/dev/NSW-876-user-event-listener to main May 13, 2026 15:22

printminion-co force-pushed the as/dev/NSW-878-wire-user-stats-api branch 2 times, most recently from d0d9820 to a8e541a Compare May 13, 2026 16:02

printminion-co requested a review from Copilot May 13, 2026 16:11

Copilot started reviewing on behalf of printminion-co May 13, 2026 16:12 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

chore(.gitignore): add .vscode to ignore list

bd9249b

printminion-co force-pushed the as/dev/NSW-878-wire-user-stats-api branch 2 times, most recently from 613e00f to 5c9cc9d Compare May 15, 2026 07:53

printminion-co requested a review from Copilot May 15, 2026 10:19

Copilot started reviewing on behalf of printminion-co May 15, 2026 10:19 View session

Copilot AI reviewed May 15, 2026

View reviewed changes

printminion-co force-pushed the as/dev/NSW-878-wire-user-stats-api branch 2 times, most recently from 221fbb0 to 05aa708 Compare May 15, 2026 11:25

Arsalanulhaq and others added 8 commits May 15, 2026 14:02

printminion-co force-pushed the as/dev/NSW-878-wire-user-stats-api branch from 638aa58 to fdee034 Compare May 15, 2026 12:03

printminion-co requested a review from Copilot May 15, 2026 12:04

Copilot started reviewing on behalf of printminion-co May 15, 2026 12:04 View session

Copilot AI reviewed May 15, 2026

View reviewed changes

printminion-co merged commit 162dd88 into main May 15, 2026
38 checks passed

printminion-co deleted the as/dev/NSW-878-wire-user-stats-api branch May 15, 2026 12:09

Navigation Menu

Uh oh!

feat(UserStatsJobs): wire UserStatsJob to send stats to PSS via API client - #18

feat(UserStatsJobs): wire UserStatsJob to send stats to PSS via API client#18
printminion-co merged 10 commits into
mainfrom
as/dev/NSW-878-wire-user-stats-api

Arsalanulhaq commented May 5, 2026 •

edited by printminion-co

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -0,0 +1,122 @@
		# 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 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.

Uh oh!

Conversation

Arsalanulhaq commented May 5, 2026 • edited by printminion-co Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

1. Set system config keys (inside the container)

2. Temporary code change required for local testing (self-signed cert)

3. Trigger the flow

4. Run the queued job

5. Verify

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Arsalanulhaq commented May 5, 2026 •

edited by printminion-co

Loading