From 73de31920fb611cb5d3c4868073afc78722a86f4 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:25:28 -0300 Subject: [PATCH 01/27] chore(deps): update 3rdparty pointer Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- 3rdparty | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/3rdparty b/3rdparty index e9a3446cfb..fb4e8a634e 160000 --- a/3rdparty +++ b/3rdparty @@ -1 +1 @@ -Subproject commit e9a3446cfb547be2eeb2a20e9ab6817955a60a54 +Subproject commit fb4e8a634e9cdc4c8893816d537bb337bbeeca16 From 59d410160abf5dd6f0fbdcf2c3508dbaf7a7697d Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:25:28 -0300 Subject: [PATCH 02/27] feat(process): add process manager service Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/Process/ProcessManager.php | 178 +++++++++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 lib/Service/Process/ProcessManager.php diff --git a/lib/Service/Process/ProcessManager.php b/lib/Service/Process/ProcessManager.php new file mode 100644 index 0000000000..b7864f39a8 --- /dev/null +++ b/lib/Service/Process/ProcessManager.php @@ -0,0 +1,178 @@ + $context + */ + public function register(string $source, int $pid, array $context = []): void { + if ($pid <= 0) { + return; + } + + $registry = $this->getRegistry(); + $registry[$source][(string)$pid] = [ + 'pid' => $pid, + 'context' => $context, + 'createdAt' => time(), + ]; + $this->saveRegistry($registry); + } + + public function unregister(string $source, int $pid): void { + $registry = $this->getRegistry(); + unset($registry[$source][(string)$pid]); + $this->saveRegistry($registry); + } + + /** + * @return array, createdAt: int}> + */ + public function listRunning(string $source): array { + $registry = $this->getRegistry(); + $entries = $registry[$source] ?? []; + $running = []; + $changed = false; + + foreach ($entries as $pidKey => $entry) { + $pid = (int)($entry['pid'] ?? 0); + if ($pid <= 0 || !$this->isRunning($pid)) { + unset($registry[$source][$pidKey]); + $changed = true; + continue; + } + + $running[] = [ + 'pid' => $pid, + 'context' => is_array($entry['context'] ?? null) ? $entry['context'] : [], + 'createdAt' => (int)($entry['createdAt'] ?? 0), + ]; + } + + if ($changed) { + $this->saveRegistry($registry); + } + + return $running; + } + + public function countRunning(string $source): int { + return count($this->listRunning($source)); + } + + /** + * @param null|callable(array{pid: int, context: array, createdAt: int}): bool $filter + */ + public function findRunningPid(string $source, ?callable $filter = null): int { + foreach ($this->listRunning($source) as $entry) { + if ($filter !== null && !$filter($entry)) { + continue; + } + return $entry['pid']; + } + + return 0; + } + + public function stopAll(string $source, int $signal = SIGTERM): int { + $stopped = 0; + foreach ($this->listRunning($source) as $entry) { + if ($this->terminate($entry['pid'], $signal)) { + $stopped++; + } + $this->unregister($source, $entry['pid']); + } + + return $stopped; + } + + public function isRunning(int $pid): bool { + if ($pid <= 0) { + return false; + } + + if (function_exists('posix_kill')) { + return @posix_kill($pid, 0); + } + + if (!function_exists('exec')) { + return false; + } + + try { + exec(sprintf('kill -0 %d 2>/dev/null', $pid), $output, $exitCode); + return $exitCode === 0; + } catch (\Throwable $e) { + $this->logger->debug('Failed to probe process status', [ + 'pid' => $pid, + 'error' => $e->getMessage(), + ]); + return false; + } + } + + protected function terminate(int $pid, int $signal): bool { + if ($pid <= 0) { + return false; + } + + if (function_exists('posix_kill')) { + return @posix_kill($pid, $signal); + } + + if (!function_exists('exec')) { + return false; + } + + try { + exec(sprintf('kill -%d %d 2>/dev/null', $signal, $pid), $output, $exitCode); + return $exitCode === 0; + } catch (\Throwable $e) { + $this->logger->debug('Failed to terminate process', [ + 'pid' => $pid, + 'signal' => $signal, + 'error' => $e->getMessage(), + ]); + return false; + } + } + + /** + * @return array, createdAt: int}>> + */ + private function getRegistry(): array { + $raw = $this->appConfig->getValueString(Application::APP_ID, self::APP_CONFIG_KEY, '{}'); + $decoded = json_decode($raw, true); + if (!is_array($decoded)) { + return []; + } + + return $decoded; + } + + /** + * @param array, createdAt: int}>> $registry + */ + private function saveRegistry(array $registry): void { + $this->appConfig->setValueString(Application::APP_ID, self::APP_CONFIG_KEY, json_encode($registry)); + } +} From 2859d1f5f5f0c82a1c5a467852a0c28098c682bb Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:25:28 -0300 Subject: [PATCH 03/27] test(process): add process manager unit tests Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../Service/Process/ProcessManagerTest.php | 223 ++++++++++++++++++ 1 file changed, 223 insertions(+) create mode 100644 tests/php/Unit/Service/Process/ProcessManagerTest.php diff --git a/tests/php/Unit/Service/Process/ProcessManagerTest.php b/tests/php/Unit/Service/Process/ProcessManagerTest.php new file mode 100644 index 0000000000..3c7f99412f --- /dev/null +++ b/tests/php/Unit/Service/Process/ProcessManagerTest.php @@ -0,0 +1,223 @@ +appConfig = $this->createMock(IAppConfig::class); + $this->logger = $this->createMock(LoggerInterface::class); + } + + /** + * @return array{0: ProcessManager, 1: string} + */ + private function makeManagerWithStoredRegistry(?callable $factory = null): array { + $stored = '{}'; + $this->appConfig->method('getValueString') + ->willReturnCallback(function (string $_appId, string $_key, string $default) use (&$stored): string { + return $stored ?: $default; + }); + + $this->appConfig->method('setValueString') + ->willReturnCallback(function (string $_appId, string $_key, string $value) use (&$stored): bool { + $stored = $value; + return true; + }); + + $manager = $factory + ? $factory($this->appConfig, $this->logger) + : new ProcessManager($this->appConfig, $this->logger); + + return [$manager, $stored]; + } + + public function testListRunningPurgesStoppedPids(): void { + $stored = '{}'; + $this->appConfig->method('getValueString') + ->willReturnCallback(function (string $appId, string $key, string $default) use (&$stored): string { + $this->assertSame(Application::APP_ID, $appId); + $this->assertSame('process_registry', $key); + return $stored ?: $default; + }); + + $this->appConfig->method('setValueString') + ->willReturnCallback(function (string $_appId, string $_key, string $value) use (&$stored): bool { + $stored = $value; + return true; + }); + + $manager = new class($this->appConfig, $this->logger) extends ProcessManager { + public function isRunning(int $pid): bool { + return $pid === 111; + } + }; + + $manager->register(self::INSTALL_SOURCE, 111, ['resource' => 'cfssl']); + $manager->register(self::INSTALL_SOURCE, 222, ['resource' => 'java']); + + $running = $manager->listRunning(self::INSTALL_SOURCE); + + $this->assertCount(1, $running); + $this->assertSame(111, $running[0]['pid']); + } + + public function testRegisterIgnoresInvalidPid(): void { + [$manager] = $this->makeManagerWithStoredRegistry(fn (IAppConfig $appConfig, LoggerInterface $logger): ProcessManager => new class($appConfig, $logger) extends ProcessManager { + public function isRunning(int $pid): bool { + return true; + } + }); + + $manager->register(self::WORKER_SOURCE, 0); + $manager->register(self::WORKER_SOURCE, -9); + + $this->assertSame(0, $manager->countRunning(self::WORKER_SOURCE)); + } + + public function testFindRunningPidReturnsFirstWhenNoFilterProvided(): void { + [$manager] = $this->makeManagerWithStoredRegistry(fn (IAppConfig $appConfig, LoggerInterface $logger): ProcessManager => new class($appConfig, $logger) extends ProcessManager { + public function isRunning(int $pid): bool { + return true; + } + }); + + $manager->register(self::INSTALL_SOURCE, 111, ['resource' => 'cfssl']); + $manager->register(self::INSTALL_SOURCE, 222, ['resource' => 'java']); + + $this->assertSame(111, $manager->findRunningPid(self::INSTALL_SOURCE)); + } + + public function testFindRunningPidAppliesFilterAgainstContext(): void { + [$manager] = $this->makeManagerWithStoredRegistry(fn (IAppConfig $appConfig, LoggerInterface $logger): ProcessManager => new class($appConfig, $logger) extends ProcessManager { + public function isRunning(int $pid): bool { + return true; + } + }); + + $manager->register(self::INSTALL_SOURCE, 111, ['resource' => 'cfssl']); + $manager->register(self::INSTALL_SOURCE, 222, ['resource' => 'java']); + + $actual = $manager->findRunningPid( + self::INSTALL_SOURCE, + fn (array $entry): bool => ($entry['context']['resource'] ?? '') === 'java', + ); + + $this->assertSame(222, $actual); + } + + public function testFindRunningPidReturnsZeroWhenNothingMatchesFilter(): void { + [$manager] = $this->makeManagerWithStoredRegistry(fn (IAppConfig $appConfig, LoggerInterface $logger): ProcessManager => new class($appConfig, $logger) extends ProcessManager { + public function isRunning(int $pid): bool { + return true; + } + }); + + $manager->register(self::INSTALL_SOURCE, 111, ['resource' => 'cfssl']); + + $actual = $manager->findRunningPid( + self::INSTALL_SOURCE, + fn (array $entry): bool => ($entry['context']['resource'] ?? '') === 'java', + ); + + $this->assertSame(0, $actual); + } + + public function testIsRunningReturnsFalseForInvalidPid(): void { + $manager = new ProcessManager($this->appConfig, $this->logger); + + $this->assertFalse($manager->isRunning(0)); + $this->assertFalse($manager->isRunning(-1)); + } + + public function testStopAllTerminatesAndUnregistersTrackedPids(): void { + $stored = '{}'; + $this->appConfig->method('getValueString') + ->willReturnCallback(function (string $_appId, string $_key, string $default) use (&$stored): string { + return $stored ?: $default; + }); + + $this->appConfig->method('setValueString') + ->willReturnCallback(function (string $_appId, string $_key, string $value) use (&$stored): bool { + $stored = $value; + return true; + }); + + $manager = new class($this->appConfig, $this->logger) extends ProcessManager { + /** @var int[] */ + public array $terminated = []; + + public function isRunning(int $pid): bool { + return true; + } + + protected function terminate(int $pid, int $signal): bool { + $this->terminated[] = $pid; + return true; + } + }; + + $manager->register(self::WORKER_SOURCE, 333); + $manager->register(self::WORKER_SOURCE, 444); + + $stopped = $manager->stopAll(self::WORKER_SOURCE); + + $this->assertSame(2, $stopped); + $this->assertSame([333, 444], $manager->terminated); + $this->assertSame(0, $manager->countRunning(self::WORKER_SOURCE)); + } + + public function testStopAllUsesProvidedSignal(): void { + $stored = '{}'; + $this->appConfig->method('getValueString') + ->willReturnCallback(function (string $_appId, string $_key, string $default) use (&$stored): string { + return $stored ?: $default; + }); + + $this->appConfig->method('setValueString') + ->willReturnCallback(function (string $_appId, string $_key, string $value) use (&$stored): bool { + $stored = $value; + return true; + }); + + $manager = new class($this->appConfig, $this->logger) extends ProcessManager { + /** @var int[] */ + public array $signals = []; + + public function isRunning(int $pid): bool { + return true; + } + + protected function terminate(int $pid, int $signal): bool { + $this->signals[] = $signal; + return true; + } + }; + + $manager->register(self::WORKER_SOURCE, 333); + $manager->register(self::WORKER_SOURCE, 444); + + $manager->stopAll(self::WORKER_SOURCE, SIGKILL); + + $this->assertSame([SIGKILL, SIGKILL], $manager->signals); + } +} From cba943062ae3cd7a863759b804df2fbcc0affcb0 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:25:28 -0300 Subject: [PATCH 04/27] refactor(cfssl): manage process lifecycle via process manager Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../CertificateEngine/CfsslHandler.php | 66 +++++++++++++------ 1 file changed, 45 insertions(+), 21 deletions(-) diff --git a/lib/Handler/CertificateEngine/CfsslHandler.php b/lib/Handler/CertificateEngine/CfsslHandler.php index fd78641d8a..61d2c9e3cb 100644 --- a/lib/Handler/CertificateEngine/CfsslHandler.php +++ b/lib/Handler/CertificateEngine/CfsslHandler.php @@ -22,6 +22,8 @@ use OCA\Libresign\Service\CertificatePolicyService; use OCA\Libresign\Service\Crl\CrlRevocationChecker; use OCA\Libresign\Service\Install\InstallService; +use OCA\Libresign\Service\Process\ProcessManager; +use OCA\Libresign\Vendor\Symfony\Component\Process\Process; use OCP\Files\AppData\IAppDataFactory; use OCP\IAppConfig; use OCP\IConfig; @@ -39,6 +41,7 @@ */ class CfsslHandler extends AEngineHandler implements IEngineHandler { public const CFSSL_URI = 'http://127.0.0.1:8888/api/v1/cfssl/'; + private const PROCESS_SOURCE = 'cfssl'; /** @var Client */ protected $client; @@ -58,6 +61,7 @@ public function __construct( protected CrlMapper $crlMapper, protected LoggerInterface $logger, CrlRevocationChecker $crlRevocationChecker, + private ProcessManager $processManager, ) { parent::__construct( $config, @@ -245,10 +249,11 @@ private function gencert(): void { $configPath = $this->getCurrentConfigPath(); $csrFile = $configPath . '/csr_server.json'; - $cmd = escapeshellcmd($binary) . ' gencert -initca ' . escapeshellarg($csrFile); - $output = shell_exec($cmd); + $process = $this->createProcess([$binary, 'gencert', '-initca', $csrFile]); + $process->run(); + $output = $process->getOutput(); - if (!$output) { + if (!$process->isSuccessful() || $output === '') { throw new \RuntimeException('cfssl without output.'); } @@ -320,11 +325,26 @@ private function wakeUp(): void { throw new LibresignException('CFSSL not configured.'); } $this->cfsslServerHandler->updateExpirity($this->getCaExpiryInDays()); - $cmd = 'nohup ' . $binary . ' serve -address=127.0.0.1 ' - . '-ca-key ' . $configPath . DIRECTORY_SEPARATOR . 'ca-key.pem ' - . '-ca ' . $configPath . DIRECTORY_SEPARATOR . 'ca.pem ' - . '-config ' . $configPath . DIRECTORY_SEPARATOR . 'config_server.json > /dev/null 2>&1 & echo $!'; - shell_exec($cmd); + $process = $this->createProcess([ + $binary, + 'serve', + '-address=127.0.0.1', + '-ca-key', $configPath . DIRECTORY_SEPARATOR . 'ca-key.pem', + '-ca', $configPath . DIRECTORY_SEPARATOR . 'ca.pem', + '-config', $configPath . DIRECTORY_SEPARATOR . 'config_server.json', + ]); + $process->setOptions(['create_new_console' => true]); + $process->setTimeout(null); + $process->disableOutput(); + $process->start(); + + $pid = (int)($process->getPid() ?? 0); + if ($pid > 0) { + $this->processManager->register(self::PROCESS_SOURCE, $pid, [ + 'uri' => $this->getCfsslUri(), + ]); + } + $loops = 0; while (!$this->portOpen() && $loops <= 4) { sleep(1); @@ -349,17 +369,11 @@ private function portOpen(): bool { } private function getServerPid(): int { - $cmd = 'ps -eo pid,command|'; - $cmd .= 'grep "cfssl.*serve.*-address"|' - . 'grep -v grep|' - . 'grep -v defunct|' - . 'sed -e "s/^[[:space:]]*//"|cut -d" " -f1'; - $output = shell_exec($cmd); - if (!is_string($output)) { - return 0; - } - $pid = trim($output); - return (int)$pid; + $uri = $this->getCfsslUri(); + return $this->processManager->findRunningPid( + self::PROCESS_SOURCE, + fn (array $entry): bool => ($entry['context']['uri'] ?? '') === $uri, + ); } /** @@ -381,6 +395,7 @@ private function stopIfRunning(): void { $pid = $this->getServerPid(); if ($pid > 0) { exec($this->parseCommand('kill -9 ' . $pid)); + $this->processManager->unregister(self::PROCESS_SOURCE, $pid); } } @@ -452,8 +467,10 @@ private function checkBinaries(): array { ->setTip('Run occ libresign:install --cfssl'), ]; } - $version = shell_exec("$binary version"); - if (!is_string($version) || empty($version)) { + $process = $this->createProcess([$binary, 'version']); + $process->run(); + $version = $process->getOutput(); + if (!$process->isSuccessful() || empty($version)) { return [ (new ConfigureCheckHelper()) ->setErrorMessage(sprintf( @@ -502,6 +519,13 @@ private function checkBinaries(): array { return $return; } + /** + * @param string[] $command + */ + protected function createProcess(array $command): Process { + return new Process($command); + } + /** * Get Authority Key Identifier from certificate (needed for CFSSL revocation) * From 8c565fcecb72367754abcd415f3ed5a3cc57a297 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:25:28 -0300 Subject: [PATCH 05/27] test(cfssl): cover binary checks and process integration Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../CertificateEngine/CfsslHandlerTest.php | 209 ++++++++++++++++++ 1 file changed, 209 insertions(+) create mode 100644 tests/php/Unit/Handler/CertificateEngine/CfsslHandlerTest.php diff --git a/tests/php/Unit/Handler/CertificateEngine/CfsslHandlerTest.php b/tests/php/Unit/Handler/CertificateEngine/CfsslHandlerTest.php new file mode 100644 index 0000000000..80fa0cc85b --- /dev/null +++ b/tests/php/Unit/Handler/CertificateEngine/CfsslHandlerTest.php @@ -0,0 +1,209 @@ +processManager = $this->createMock(ProcessManager::class); + } + + public function testGetServerPidReadsFromRegistry(): void { + $handler = $this->createHandler(); + + $this->processManager->expects($this->once()) + ->method('findRunningPid') + ->with(self::PROCESS_SOURCE, $this->callback('is_callable')) + ->willReturnCallback(function (string $_source, callable $filter): int { + return $filter([ + 'pid' => 302, + 'context' => ['uri' => CfsslHandler::CFSSL_URI], + 'createdAt' => 123, + ]) ? 302 : 0; + }); + + $actual = self::invokePrivate($handler, 'getServerPid'); + + $this->assertSame(302, $actual); + } + + public function testScopedProcessClassIsAvailable(): void { + $this->assertTrue(class_exists(Process::class)); + } + + public function testCheckBinariesReturnsErrorWhenProcessFails(): void { + $binary = tempnam(sys_get_temp_dir(), 'cfssl-bin-'); + $this->assertNotFalse($binary); + + $process = $this->createMock(Process::class); + $process->expects($this->once()) + ->method('run'); + $process->expects($this->once()) + ->method('getOutput') + ->willReturn(''); + $process->expects($this->once()) + ->method('isSuccessful') + ->willReturn(false); + + $handler = $this->createHandler($process, (string)$binary); + $result = self::invokePrivate($handler, 'checkBinaries'); + + $this->assertSame('error', $result[0]->jsonSerialize()['status']); + $this->assertStringContainsString('Failed to run the command', $result[0]->jsonSerialize()['message']); + + @unlink((string)$binary); + } + + public function testCheckBinariesReturnsSuccessWhenProcessOutputIsValid(): void { + $binary = tempnam(sys_get_temp_dir(), 'cfssl-bin-'); + $this->assertNotFalse($binary); + + $process = $this->createMock(Process::class); + $process->expects($this->once()) + ->method('run'); + $process->expects($this->once()) + ->method('isSuccessful') + ->willReturn(true); + $process->expects($this->once()) + ->method('getOutput') + ->willReturn('Version: ' . InstallService::CFSSL_VERSION . "\nRuntime: go1.22\n"); + + $handler = $this->createHandler($process, (string)$binary); + $result = self::invokePrivate($handler, 'checkBinaries'); + + $this->assertCount(3, $result); + $this->assertSame('success', $result[0]->jsonSerialize()['status']); + $this->assertSame('success', $result[1]->jsonSerialize()['status']); + $this->assertSame('success', $result[2]->jsonSerialize()['status']); + + @unlink((string)$binary); + } + + public function testCheckBinariesReturnsErrorWhenVersionOutputFormatIsInvalid(): void { + $binary = tempnam(sys_get_temp_dir(), 'cfssl-bin-'); + $this->assertNotFalse($binary); + + $process = $this->createMock(Process::class); + $process->expects($this->once()) + ->method('run'); + $process->expects($this->once()) + ->method('isSuccessful') + ->willReturn(true); + $process->expects($this->once()) + ->method('getOutput') + ->willReturn('cfssl version output without expected separators'); + + $handler = $this->createHandler($process, (string)$binary); + $result = self::invokePrivate($handler, 'checkBinaries'); + + $this->assertSame('error', $result[0]->jsonSerialize()['status']); + $this->assertStringContainsString('Failed to identify cfssl version', $result[0]->jsonSerialize()['message']); + + @unlink((string)$binary); + } + + public function testCheckBinariesReturnsErrorWhenCfsslVersionDoesNotMatchExpected(): void { + $binary = tempnam(sys_get_temp_dir(), 'cfssl-bin-'); + $this->assertNotFalse($binary); + + $process = $this->createMock(Process::class); + $process->expects($this->once()) + ->method('run'); + $process->expects($this->once()) + ->method('isSuccessful') + ->willReturn(true); + $process->expects($this->once()) + ->method('getOutput') + ->willReturn("Version: 0.0.1\nRuntime: go1.22\n"); + + $handler = $this->createHandler($process, (string)$binary); + $result = self::invokePrivate($handler, 'checkBinaries'); + + $this->assertSame('error', $result[0]->jsonSerialize()['status']); + $this->assertStringContainsString('Invalid version. Expected:', $result[0]->jsonSerialize()['message']); + + @unlink((string)$binary); + } + + private function createHandler(?Process $process = null, ?string $binary = null): CfsslHandler { + $config = \OCP\Server::get(IConfig::class); + $appConfig = $this->getMockAppConfigWithReset(); + if ($binary !== null) { + $appConfigMock = $this->createMock(IAppConfig::class); + $appConfigMock->method('getValueString') + ->willReturnCallback(function (string $appId, string $key, string $default = '') use ($binary): string { + if ($appId === 'libresign' && $key === 'cfssl_bin') { + return $binary; + } + return $default; + }); + $appConfig = $appConfigMock; + } + $appDataFactory = \OCP\Server::get(IAppDataFactory::class); + $dateTimeFormatter = \OCP\Server::get(IDateTimeFormatter::class); + $tempManager = \OCP\Server::get(ITempManager::class); + $certificatePolicyService = \OCP\Server::get(CertificatePolicyService::class); + $urlGenerator = \OCP\Server::get(IURLGenerator::class); + $caIdentifierService = \OCP\Server::get(CaIdentifierService::class); + $crlMapper = \OCP\Server::get(CrlMapper::class); + $logger = \OCP\Server::get(LoggerInterface::class); + $crlRevocationChecker = $this->createMock(CrlRevocationChecker::class); + $cfsslServerHandler = $this->createMock(CfsslServerHandler::class); + $cfsslServerHandler->expects($this->once()) + ->method('configCallback'); + $process ??= $this->createMock(Process::class); + + $handler = $this->getMockBuilder(CfsslHandler::class) + ->setConstructorArgs([ + $config, + $appConfig, + $appDataFactory, + $dateTimeFormatter, + $tempManager, + $cfsslServerHandler, + $certificatePolicyService, + $urlGenerator, + $caIdentifierService, + $crlMapper, + $logger, + $crlRevocationChecker, + $this->processManager, + ]) + ->onlyMethods(['createProcess']) + ->getMock(); + + $handler->method('createProcess') + ->willReturn($process); + + return $handler; + } +} From e4b6cbcf51f804f0a19cc0eecb691e623050f2c2 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:25:28 -0300 Subject: [PATCH 06/27] refactor(install): use process api for async install Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/Install/InstallService.php | 51 ++++++++++++++++---------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/lib/Service/Install/InstallService.php b/lib/Service/Install/InstallService.php index 0f76941858..2bf04671b6 100644 --- a/lib/Service/Install/InstallService.php +++ b/lib/Service/Install/InstallService.php @@ -21,7 +21,9 @@ use OCA\Libresign\Handler\CertificateEngine\CfsslHandler; use OCA\Libresign\Handler\CertificateEngine\IEngineHandler; use OCA\Libresign\Service\CaIdentifierService; +use OCA\Libresign\Service\Process\ProcessManager; use OCA\Libresign\Vendor\LibreSign\WhatOSAmI\OperatingSystem; +use OCA\Libresign\Vendor\Symfony\Component\Process\Process; use OCP\Files\AppData\IAppDataFactory; use OCP\Files\IAppData; use OCP\Files\NotFoundException; @@ -37,7 +39,6 @@ use RuntimeException; use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Output\OutputInterface; -use Symfony\Component\Process\Process; class InstallService { use TSimpleFile { @@ -52,6 +53,7 @@ class InstallService { public const JSIGNPDF_VERSION = '2.3.0'; /** @todo When update, verify the hash **/ private const JSIGNPDF_HASH = 'd239658ea50a39eb35169d8392feaffb'; public const CFSSL_VERSION = '1.6.5'; + private const PROCESS_SOURCE = 'install'; private ICache $cache; private ?OutputInterface $output = null; @@ -77,6 +79,7 @@ public function __construct( private SignSetupService $signSetupService, protected IAppDataFactory $appDataFactory, private CaIdentifierService $caIdentifierService, + private ProcessManager $processManager, ) { $this->cache = $cacheFactory->createDistributed('libresign-setup'); $this->appData = $appDataFactory->get('libresign'); @@ -145,18 +148,28 @@ private function getDataDir(): string { private function runAsync(): void { $resource = $this->resource; - $process = new Process([OC::$SERVERROOT . '/occ', 'libresign:install', '--' . $resource]); + $process = $this->createProcess([OC::$SERVERROOT . '/occ', 'libresign:install', '--' . $resource]); $process->setOptions(['create_new_console' => true]); $process->setTimeout(null); $process->start(); $data['pid'] = $process->getPid(); if ($data['pid']) { + $this->processManager->register(self::PROCESS_SOURCE, (int)$data['pid'], [ + 'resource' => $resource, + ]); $this->setCache($resource, $data); } else { $this->logger->error('Error to get PID of background install proccess. Command: ' . OC::$SERVERROOT . '/occ libresign:install --' . $resource); } } + /** + * @param string[] $command + */ + protected function createProcess(array $command): Process { + return new Process($command); + } + private function progressToDatabase(int $downloadSize, int $downloaded): void { $data = $this->getProressData(); $data['download_size'] = $downloadSize; @@ -295,27 +308,27 @@ public function isDownloadWip(): bool { } private function getInstallPid(int $pid = 0): int { + $resource = $this->resource; if ($pid > 0) { - if (shell_exec('which ps') === null) { - if (is_dir('/proc/' . $pid)) { - return $pid; - } - return 0; + $registeredPid = $this->processManager->findRunningPid( + self::PROCESS_SOURCE, + fn (array $entry): bool + => $entry['pid'] === $pid + && ($entry['context']['resource'] ?? '') === $resource, + ); + + if ($registeredPid > 0) { + return $registeredPid; } - $cmd = 'ps -p ' . $pid . ' -o pid,command|'; - } else { - $cmd = 'ps -eo pid,command|'; - } - $cmd .= 'grep "libresign:install --' . $this->resource . '"|' - . 'grep -v grep|' - . 'grep -v defunct|' - . 'sed -e "s/^[[:space:]]*//"|cut -d" " -f1'; - $output = shell_exec($cmd); - if (!is_string($output)) { + + $this->processManager->unregister(self::PROCESS_SOURCE, $pid); return 0; } - $pid = trim($output); - return (int)$pid; + + return $this->processManager->findRunningPid( + self::PROCESS_SOURCE, + fn (array $entry): bool => ($entry['context']['resource'] ?? '') === $resource, + ); } public function setResource(string $resource): self { From 6a6ae3782fe95e502065e29fe099286cd3140446 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:25:29 -0300 Subject: [PATCH 07/27] test(install): cover async process pid handling Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- tests/php/Unit/Service/InstallServiceTest.php | 150 ++++++++++++++++++ 1 file changed, 150 insertions(+) diff --git a/tests/php/Unit/Service/InstallServiceTest.php b/tests/php/Unit/Service/InstallServiceTest.php index b57349d50c..f331b2331e 100644 --- a/tests/php/Unit/Service/InstallServiceTest.php +++ b/tests/php/Unit/Service/InstallServiceTest.php @@ -13,6 +13,8 @@ use OCA\Libresign\Service\CaIdentifierService; use OCA\Libresign\Service\Install\InstallService; use OCA\Libresign\Service\Install\SignSetupService; +use OCA\Libresign\Service\Process\ProcessManager; +use OCA\Libresign\Vendor\Symfony\Component\Process\Process; use OCP\Files\AppData\IAppDataFactory; use OCP\Http\Client\IClientService; use OCP\IAppConfig; @@ -32,6 +34,7 @@ final class InstallServiceTest extends \OCA\Libresign\Tests\Unit\TestCase { private SignSetupService&MockObject $ignSetupService; private IAppDataFactory&MockObject $appDataFactory; private CaIdentifierService&MockObject $caIdentifierService; + private ProcessManager&MockObject $processManager; public function setUp(): void { parent::setUp(); @@ -47,6 +50,7 @@ protected function getInstallService(): InstallService { $this->ignSetupService = $this->createMock(SignSetupService::class); $this->appDataFactory = $this->createMock(IAppDataFactory::class); $this->caIdentifierService = $this->createMock(CaIdentifierService::class); + $this->processManager = $this->createMock(ProcessManager::class); return new InstallService( $this->cacheFactory, $this->clientService, @@ -57,6 +61,7 @@ protected function getInstallService(): InstallService { $this->ignSetupService, $this->appDataFactory, $this->caIdentifierService, + $this->processManager, ); } @@ -168,4 +173,149 @@ public static function providerGetFolder(): array { ['x86_64', 'test/folder1/folder2', 'folder2'], ]; } + + public function testGetInstallPidReadsMatchingPidFromRegistry(): void { + $installService = $this->getInstallService(); + $installService->setResource('cfssl'); + + $this->processManager->expects($this->once()) + ->method('findRunningPid') + ->with('install', $this->callback('is_callable')) + ->willReturnCallback(function (string $_source, callable $filter): int { + return $filter([ + 'pid' => 123, + 'context' => ['resource' => 'cfssl'], + 'createdAt' => 123, + ]) ? 123 : 0; + }); + + $actual = self::invokePrivate($installService, 'getInstallPid'); + + $this->assertSame(123, $actual); + } + + public function testGetInstallPidValidatesRequestedPidAgainstResource(): void { + $installService = $this->getInstallService(); + $installService->setResource('cfssl'); + + $this->processManager->expects($this->once()) + ->method('findRunningPid') + ->with('install', $this->callback('is_callable')) + ->willReturn(0); + + $this->processManager->expects($this->once()) + ->method('unregister') + ->with('install', 123); + + $actual = self::invokePrivate($installService, 'getInstallPid', [123]); + + $this->assertSame(0, $actual); + } + + public function testGetInstallPidKeepsRequestedPidWhenResourceMatches(): void { + $installService = $this->getInstallService(); + $installService->setResource('cfssl'); + + $this->processManager->expects($this->once()) + ->method('findRunningPid') + ->with('install', $this->callback('is_callable')) + ->willReturnCallback(function (string $_source, callable $filter): int { + return $filter([ + 'pid' => 321, + 'context' => ['resource' => 'cfssl'], + 'createdAt' => 123, + ]) ? 321 : 0; + }); + + $this->processManager->expects($this->never()) + ->method('unregister'); + + $actual = self::invokePrivate($installService, 'getInstallPid', [321]); + + $this->assertSame(321, $actual); + } + + public function testRunAsyncRegistersPidWhenProcessStarts(): void { + $process = $this->createMock(Process::class); + $process->expects($this->once()) + ->method('setOptions') + ->with(['create_new_console' => true]); + $process->expects($this->once()) + ->method('setTimeout') + ->with(null); + $process->expects($this->once()) + ->method('start'); + $process->expects($this->once()) + ->method('getPid') + ->willReturn(321); + + $installService = $this->getInstallServiceWithProcess($process); + $installService->setResource('cfssl'); + + $this->processManager->expects($this->once()) + ->method('register') + ->with('install', 321, ['resource' => 'cfssl']); + + self::invokePrivate($installService, 'runAsync'); + } + + public function testRunAsyncLogsErrorWhenPidIsMissing(): void { + $process = $this->createMock(Process::class); + $process->expects($this->once()) + ->method('setOptions') + ->with(['create_new_console' => true]); + $process->expects($this->once()) + ->method('setTimeout') + ->with(null); + $process->expects($this->once()) + ->method('start'); + $process->expects($this->once()) + ->method('getPid') + ->willReturn(null); + + $installService = $this->getInstallServiceWithProcess($process); + $installService->setResource('cfssl'); + + $this->processManager->expects($this->never()) + ->method('register'); + $this->logger->expects($this->once()) + ->method('error') + ->with($this->stringContains('Error to get PID of background install proccess')); + + self::invokePrivate($installService, 'runAsync'); + } + + private function getInstallServiceWithProcess(Process $process): InstallService&MockObject { + $this->cacheFactory = $this->createMock(ICacheFactory::class); + $this->clientService = $this->createMock(IClientService::class); + $this->certificateEngineFactory = $this->createMock(CertificateEngineFactory::class); + $this->config = $this->createMock(IConfig::class); + $this->appConfig = $this->createMock(IAppConfig::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->ignSetupService = $this->createMock(SignSetupService::class); + $this->appDataFactory = $this->createMock(IAppDataFactory::class); + $this->caIdentifierService = $this->createMock(CaIdentifierService::class); + $this->processManager = $this->createMock(ProcessManager::class); + + $installService = $this->getMockBuilder(InstallService::class) + ->setConstructorArgs([ + $this->cacheFactory, + $this->clientService, + $this->certificateEngineFactory, + $this->config, + $this->appConfig, + $this->logger, + $this->ignSetupService, + $this->appDataFactory, + $this->caIdentifierService, + $this->processManager, + ]) + ->onlyMethods(['createProcess']) + ->getMock(); + + $installService->method('createProcess') + ->willReturn($process); + + return $installService; + } } From d8abd81ca638134caf0e226103c00f37410c044a Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:25:29 -0300 Subject: [PATCH 08/27] refactor(migration): stop workers via process manager Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Migration/StopRunningWorkers.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/Migration/StopRunningWorkers.php b/lib/Migration/StopRunningWorkers.php index 1d931a6c12..bfa31ce5a5 100644 --- a/lib/Migration/StopRunningWorkers.php +++ b/lib/Migration/StopRunningWorkers.php @@ -8,14 +8,16 @@ namespace OCA\Libresign\Migration; -use OCA\Libresign\Service\Worker\WorkerStopper; +use OCA\Libresign\Service\Process\ProcessManager; use OCP\Migration\IOutput; use OCP\Migration\IRepairStep; use Psr\Log\LoggerInterface; class StopRunningWorkers implements IRepairStep { + private const PROCESS_SOURCE = 'worker'; + public function __construct( - private WorkerStopper $stopper, + private ProcessManager $processManager, private LoggerInterface $logger, ) { } @@ -28,7 +30,7 @@ public function getName(): string { #[\Override] public function run(IOutput $output): void { try { - $stopped = $this->stopper->stopAll(); + $stopped = $this->processManager->stopAll(self::PROCESS_SOURCE, SIGTERM); if ($stopped > 0) { $output->info('Stopped ' . $stopped . ' LibreSign worker(s).'); } From a54096871c5ee76365f6aa706c292de0569d68dd Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:25:29 -0300 Subject: [PATCH 09/27] refactor(worker): remove worker counter service Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/Worker/WorkerCounter.php | 39 ---------------------------- 1 file changed, 39 deletions(-) delete mode 100644 lib/Service/Worker/WorkerCounter.php diff --git a/lib/Service/Worker/WorkerCounter.php b/lib/Service/Worker/WorkerCounter.php deleted file mode 100644 index d8903aca8f..0000000000 --- a/lib/Service/Worker/WorkerCounter.php +++ /dev/null @@ -1,39 +0,0 @@ -buildCountCommand(); - $output = shell_exec($cmd); - return max(0, (int)trim((string)$output)); - } catch (\Throwable $e) { - $this->logger->debug('Failed to count running workers', [ - 'error' => $e->getMessage(), - ]); - return 0; - } - } - - private function buildCountCommand(): string { - $occPath = \OC::$SERVERROOT . '/occ'; - return sprintf( - "ps -eo args | grep -F %s | grep -F 'background-job:worker' | grep -E 'SignFileJob|SignSingleFileJob' | grep -v grep | wc -l", - escapeshellarg($occPath) - ); - } -} From b746f68e03a282dd12fd5346f43f84c7c2846f62 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:25:29 -0300 Subject: [PATCH 10/27] test(worker): remove worker counter tests Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../Unit/Service/Worker/WorkerCounterTest.php | 33 ------------------- 1 file changed, 33 deletions(-) delete mode 100644 tests/php/Unit/Service/Worker/WorkerCounterTest.php diff --git a/tests/php/Unit/Service/Worker/WorkerCounterTest.php b/tests/php/Unit/Service/Worker/WorkerCounterTest.php deleted file mode 100644 index 4d119a11ba..0000000000 --- a/tests/php/Unit/Service/Worker/WorkerCounterTest.php +++ /dev/null @@ -1,33 +0,0 @@ -logger = $this->createMock(LoggerInterface::class); - } - - private function makeCounter(): WorkerCounter { - return new WorkerCounter($this->logger); - } - - public function testCountRunningReturnsNeverNegative(): void { - $counter = $this->makeCounter(); - $result = $counter->countRunning(); - $this->assertGreaterThanOrEqual(0, $result); - } -} From 598dc46df9fb9ea66c0b8133344fe19eddf65de6 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:25:29 -0300 Subject: [PATCH 11/27] refactor(worker): remove worker stopper service Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/Worker/WorkerStopper.php | 121 --------------------------- 1 file changed, 121 deletions(-) delete mode 100644 lib/Service/Worker/WorkerStopper.php diff --git a/lib/Service/Worker/WorkerStopper.php b/lib/Service/Worker/WorkerStopper.php deleted file mode 100644 index 3ce9546fa3..0000000000 --- a/lib/Service/Worker/WorkerStopper.php +++ /dev/null @@ -1,121 +0,0 @@ -getProcessList(); - if ($processList === '') { - return 0; - } - - $stopped = 0; - foreach ($this->parseProcessList($processList) as [$pid, $args]) { - if ($pid === null || $args === null) { - continue; - } - - if (!$this->matchesWorkerCommand($args)) { - continue; - } - - if ($this->terminateProcess($pid)) { - $stopped++; - } - } - - return $stopped; - } - - private function getProcessList(): string { - try { - $output = shell_exec('ps -eo pid=,args='); - return $output === null ? '' : trim($output); - } catch (\Throwable $e) { - $this->logger->debug('Failed to read process list', [ - 'error' => $e->getMessage(), - ]); - return ''; - } - } - - private function parseProcessList(string $processList): array { - $lines = preg_split('/\R/', $processList) ?: []; - $entries = []; - - foreach ($lines as $line) { - $line = trim($line); - if ($line === '') { - continue; - } - - $parts = preg_split('/\s+/', $line, 2); - if (!$parts || count($parts) < 2) { - continue; - } - - $pid = (int)$parts[0]; - if ($pid <= 0) { - continue; - } - - $entries[] = [$pid, $parts[1]]; - } - - return $entries; - } - - private function matchesWorkerCommand(string $args): bool { - $occPath = \OC::$SERVERROOT . '/occ'; - if (strpos($args, $occPath) === false) { - return false; - } - if (strpos($args, 'background-job:worker') === false) { - return false; - } - - foreach (self::JOB_CLASSES as $jobClass) { - if (strpos($args, $jobClass) !== false) { - return true; - } - } - - return false; - } - - private function terminateProcess(int $pid): bool { - if (function_exists('posix_kill')) { - return @posix_kill($pid, SIGTERM); - } - - if (!function_exists('shell_exec')) { - $this->logger->debug('Cannot stop worker without posix_kill or shell_exec', [ - 'pid' => $pid, - ]); - return false; - } - - shell_exec(sprintf('kill -TERM %d 2>/dev/null', $pid)); - return true; - } -} From 128276686088c349a581fe232355787829e12153 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:25:29 -0300 Subject: [PATCH 12/27] refactor(worker): calculate workers from process manager Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/Worker/WorkerHealthService.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/Service/Worker/WorkerHealthService.php b/lib/Service/Worker/WorkerHealthService.php index 9376409a19..877446c4a7 100644 --- a/lib/Service/Worker/WorkerHealthService.php +++ b/lib/Service/Worker/WorkerHealthService.php @@ -8,12 +8,15 @@ namespace OCA\Libresign\Service\Worker; +use OCA\Libresign\Service\Process\ProcessManager; use Psr\Log\LoggerInterface; class WorkerHealthService { + private const PROCESS_SOURCE = 'worker'; + public function __construct( private WorkerConfiguration $workerConfiguration, - private WorkerCounter $workerCounter, + private ProcessManager $processManager, private WorkerJobCounter $workerJobCounter, private StartThrottlePolicy $startThrottlePolicy, private WorkerStarter $workerStarter, @@ -54,7 +57,7 @@ public function ensureWorkerRunning(): bool { private function calculateWorkersNeeded(): int { $desired = $this->workerConfiguration->getDesiredWorkerCount(); - $running = $this->workerCounter->countRunning(); + $running = $this->processManager->countRunning(self::PROCESS_SOURCE); $pendingJobs = $this->workerJobCounter->countPendingJobs(); if ($this->hasNoPendingWork($pendingJobs)) { From 1600b18bf3d4649d228f146507de57c7fa4560d7 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:25:29 -0300 Subject: [PATCH 13/27] refactor(worker): start workers via symfony process Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/Worker/WorkerStarter.php | 32 +++++++++++++++++++++------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/lib/Service/Worker/WorkerStarter.php b/lib/Service/Worker/WorkerStarter.php index d9ccf6829c..da385e68a5 100644 --- a/lib/Service/Worker/WorkerStarter.php +++ b/lib/Service/Worker/WorkerStarter.php @@ -10,13 +10,17 @@ use OCA\Libresign\BackgroundJob\SignFileJob; use OCA\Libresign\BackgroundJob\SignSingleFileJob; +use OCA\Libresign\Service\Process\ProcessManager; +use OCA\Libresign\Vendor\Symfony\Component\Process\Process; use OCP\IBinaryFinder; class WorkerStarter { private const MAX_WORKERS = 32; + private const PROCESS_SOURCE = 'worker'; public function __construct( private IBinaryFinder $binaryFinder, + private ProcessManager $processManager, ) { } @@ -41,13 +45,25 @@ private function clampWorkerCount(int $count): int { } private function executeCommand(string $phpPath, string $occPath, array $jobClasses): void { - $jobClassesArg = implode(' ', array_map('escapeshellarg', $jobClasses)); - $cmd = sprintf( - '%s %s background-job:worker %s --stop_after=30m >> /dev/null 2>&1 &', - escapeshellarg($phpPath), - escapeshellarg($occPath), - $jobClassesArg - ); - shell_exec($cmd); + $command = [ + $phpPath, + $occPath, + 'background-job:worker', + ...$jobClasses, + '--stop_after=30m', + ]; + $process = $this->createProcess($command); + $process->setOptions(['create_new_console' => true]); + $process->setTimeout(null); + $process->start(); + + $pid = $process->getPid() ?? 0; + if ($pid > 0) { + $this->processManager->register(self::PROCESS_SOURCE, $pid); + } + } + + protected function createProcess(array $command): Process { + return new Process($command); } } From 210ffbdcb2b85e9daca4feda0b141d9bb4d487da Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:25:29 -0300 Subject: [PATCH 14/27] test(worker): add worker starter unit tests Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../Unit/Service/Worker/WorkerStarterTest.php | 172 ++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 tests/php/Unit/Service/Worker/WorkerStarterTest.php diff --git a/tests/php/Unit/Service/Worker/WorkerStarterTest.php b/tests/php/Unit/Service/Worker/WorkerStarterTest.php new file mode 100644 index 0000000000..76ee062bcf --- /dev/null +++ b/tests/php/Unit/Service/Worker/WorkerStarterTest.php @@ -0,0 +1,172 @@ +binaryFinder = $this->createMock(IBinaryFinder::class); + $this->processManager = $this->createMock(ProcessManager::class); + } + + private function makeProcessMock(?int $pid): Process&MockObject { + $process = $this->createMock(Process::class); + $process->expects($this->once())->method('setOptions')->with(['create_new_console' => true]); + $process->expects($this->once())->method('setTimeout')->with(null); + $process->expects($this->once())->method('start'); + $process->expects($this->once())->method('getPid')->willReturn($pid); + return $process; + } + + /** @return array */ + private function makeProcessMocks(int $count, ?int $pid): array { + $processes = []; + for ($i = 0; $i < $count; $i++) { + $processes[] = $this->makeProcessMock($pid); + } + return $processes; + } + + public function testStartWorkersFallbacksToPhpAndRegistersPid(): void { + $this->binaryFinder->expects($this->once()) + ->method('findBinaryPath') + ->with('php') + ->willReturn(false); + + $callIndex = 0; + $this->processManager->expects($this->exactly(2)) + ->method('register') + ->willReturnCallback(function (string $source, int $pid) use (&$callIndex): void { + $callIndex++; + $this->assertSame('worker', $source); + $this->assertSame($callIndex === 1 ? 101 : 202, $pid); + }); + + $workerStarter = new class($this->binaryFinder, $this->processManager, [$this->makeProcessMock(101), $this->makeProcessMock(202)]) extends WorkerStarter { + /** @var Process[] */ + private array $processes; + /** @var array> */ + public array $commands = []; + + public function __construct(IBinaryFinder $binaryFinder, ProcessManager $processManager, array $processes) { + parent::__construct($binaryFinder, $processManager); + $this->processes = $processes; + } + + protected function createProcess(array $command): Process { + $this->commands[] = $command; + return array_shift($this->processes); + } + }; + + $workerStarter->startWorkers(2); + + $this->assertCount(2, $workerStarter->commands); + $this->assertSame('php', $workerStarter->commands[0][0]); + $this->assertSame('background-job:worker', $workerStarter->commands[0][2]); + } + + public function testStartWorkersClampsMinimumToOne(): void { + $this->binaryFinder->expects($this->once()) + ->method('findBinaryPath') + ->with('php') + ->willReturn('/usr/bin/php'); + + $this->processManager->expects($this->never()) + ->method('register'); + + $workerStarter = new class($this->binaryFinder, $this->processManager, [$this->makeProcessMock(0)]) extends WorkerStarter { + /** @var Process[] */ + private array $processes; + /** @var array> */ + public array $commands = []; + + public function __construct(IBinaryFinder $binaryFinder, ProcessManager $processManager, array $processes) { + parent::__construct($binaryFinder, $processManager); + $this->processes = $processes; + } + + protected function createProcess(array $command): Process { + $this->commands[] = $command; + return array_shift($this->processes); + } + }; + + $workerStarter->startWorkers(0); + + $this->assertCount(1, $workerStarter->commands); + } + + public function testStartWorkersClampsMaximumToThirtyTwo(): void { + $this->binaryFinder->expects($this->once()) + ->method('findBinaryPath') + ->with('php') + ->willReturn('/usr/bin/php'); + + $this->processManager->expects($this->never()) + ->method('register'); + + $workerStarter = new class($this->binaryFinder, $this->processManager, $this->makeProcessMocks(32, null)) extends WorkerStarter { + /** @var Process[] */ + private array $processes; + /** @var array> */ + public array $commands = []; + + public function __construct(IBinaryFinder $binaryFinder, ProcessManager $processManager, array $processes) { + parent::__construct($binaryFinder, $processManager); + $this->processes = $processes; + } + + protected function createProcess(array $command): Process { + $this->commands[] = $command; + return array_shift($this->processes); + } + }; + + $workerStarter->startWorkers(999); + + $this->assertCount(32, $workerStarter->commands); + } + + public function testStartWorkersDoesNotRegisterWhenPidIsInvalid(): void { + $this->binaryFinder->expects($this->once()) + ->method('findBinaryPath') + ->with('php') + ->willReturn('/usr/bin/php'); + + $this->processManager->expects($this->never()) + ->method('register'); + + $workerStarter = new class($this->binaryFinder, $this->processManager, [$this->makeProcessMock(0), $this->makeProcessMock(-1), $this->makeProcessMock(null)]) extends WorkerStarter { + /** @var Process[] */ + private array $processes; + + public function __construct(IBinaryFinder $binaryFinder, ProcessManager $processManager, array $processes) { + parent::__construct($binaryFinder, $processManager); + $this->processes = $processes; + } + + protected function createProcess(array $command): Process { + return array_shift($this->processes); + } + }; + + $workerStarter->startWorkers(3); + } +} From 69469731fc4e60dc72405383354de5393d7846b7 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:25:29 -0300 Subject: [PATCH 15/27] test(worker): improve worker health scenarios Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../Worker/WorkerHealthServiceTest.php | 206 ++++++------------ 1 file changed, 61 insertions(+), 145 deletions(-) diff --git a/tests/php/Unit/Service/Worker/WorkerHealthServiceTest.php b/tests/php/Unit/Service/Worker/WorkerHealthServiceTest.php index 003e43dbe7..5d507363a0 100644 --- a/tests/php/Unit/Service/Worker/WorkerHealthServiceTest.php +++ b/tests/php/Unit/Service/Worker/WorkerHealthServiceTest.php @@ -8,19 +8,20 @@ namespace OCA\Libresign\Tests\Unit\Service\Worker; +use OCA\Libresign\Service\Process\ProcessManager; use OCA\Libresign\Service\Worker\StartThrottlePolicy; use OCA\Libresign\Service\Worker\WorkerConfiguration; -use OCA\Libresign\Service\Worker\WorkerCounter; use OCA\Libresign\Service\Worker\WorkerHealthService; use OCA\Libresign\Service\Worker\WorkerJobCounter; use OCA\Libresign\Service\Worker\WorkerStarter; use OCA\Libresign\Tests\Unit\TestCase; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; class WorkerHealthServiceTest extends TestCase { private WorkerConfiguration&MockObject $workerConfiguration; - private WorkerCounter&MockObject $workerCounter; + private ProcessManager&MockObject $processManager; private WorkerJobCounter&MockObject $workerJobCounter; private StartThrottlePolicy&MockObject $startThrottlePolicy; private WorkerStarter&MockObject $workerStarter; @@ -29,7 +30,7 @@ class WorkerHealthServiceTest extends TestCase { public function setUp(): void { parent::setUp(); $this->workerConfiguration = $this->createMock(WorkerConfiguration::class); - $this->workerCounter = $this->createMock(WorkerCounter::class); + $this->processManager = $this->createMock(ProcessManager::class); $this->workerJobCounter = $this->createMock(WorkerJobCounter::class); $this->startThrottlePolicy = $this->createMock(StartThrottlePolicy::class); $this->workerStarter = $this->createMock(WorkerStarter::class); @@ -39,7 +40,7 @@ public function setUp(): void { private function makeService(): WorkerHealthService { return new WorkerHealthService( $this->workerConfiguration, - $this->workerCounter, + $this->processManager, $this->workerJobCounter, $this->startThrottlePolicy, $this->workerStarter, @@ -56,22 +57,24 @@ public function testEnsureWorkerRunningReturnsFalseWhenAsyncDisabled(): void { $this->assertFalse($service->ensureWorkerRunning()); } - public function testEnsureWorkerRunningNoStartWhenNoPendingJobs(): void { + #[DataProvider('providerNoStartScenarios')] + public function testEnsureWorkerRunningDoesNotStartWorkers(int $desired, int $running, int $pendingJobs): void { $this->workerConfiguration->expects($this->once()) ->method('isAsyncLocalEnabled') ->willReturn(true); $this->workerConfiguration->expects($this->once()) ->method('getDesiredWorkerCount') - ->willReturn(4); + ->willReturn($desired); - $this->workerCounter->expects($this->once()) + $this->processManager->expects($this->once()) ->method('countRunning') - ->willReturn(0); + ->with('worker') + ->willReturn($running); $this->workerJobCounter->expects($this->once()) ->method('countPendingJobs') - ->willReturn(0); + ->willReturn($pendingJobs); $this->startThrottlePolicy->expects($this->never()) ->method('isThrottled'); @@ -83,31 +86,16 @@ public function testEnsureWorkerRunningNoStartWhenNoPendingJobs(): void { $this->assertTrue($service->ensureWorkerRunning()); } - public function testEnsureWorkerRunningNoStartWhenEnoughWorkers(): void { - $this->workerConfiguration->expects($this->once()) - ->method('isAsyncLocalEnabled') - ->willReturn(true); - - $this->workerConfiguration->expects($this->once()) - ->method('getDesiredWorkerCount') - ->willReturn(4); - - $this->workerCounter->expects($this->once()) - ->method('countRunning') - ->willReturn(4); - - $this->workerJobCounter->expects($this->once()) - ->method('countPendingJobs') - ->willReturn(10); - - $this->startThrottlePolicy->expects($this->never()) - ->method('isThrottled'); - - $service = $this->makeService(); - $this->assertTrue($service->ensureWorkerRunning()); + public static function providerNoStartScenarios(): array { + return [ + 'no pending jobs' => [4, 0, 0], + 'enough workers already running' => [4, 4, 10], + 'running workers exceeds desired' => [2, 5, 10], + 'desired workers is zero' => [0, 0, 10], + ]; } - public function testEnsureWorkerRunningRespectThrottlePolicy(): void { + public function testEnsureWorkerRunningRespectsThrottlePolicy(): void { $this->workerConfiguration->expects($this->once()) ->method('isAsyncLocalEnabled') ->willReturn(true); @@ -116,8 +104,9 @@ public function testEnsureWorkerRunningRespectThrottlePolicy(): void { ->method('getDesiredWorkerCount') ->willReturn(4); - $this->workerCounter->expects($this->once()) + $this->processManager->expects($this->once()) ->method('countRunning') + ->with('worker') ->willReturn(0); $this->workerJobCounter->expects($this->once()) @@ -138,22 +127,24 @@ public function testEnsureWorkerRunningRespectThrottlePolicy(): void { $this->assertTrue($service->ensureWorkerRunning()); } - public function testEnsureWorkerRunningStartsWhenNotThrottled(): void { + #[DataProvider('providerStartWorkerScenarios')] + public function testEnsureWorkerRunningStartsExpectedNumberOfWorkers(int $desired, int $running, int $pendingJobs, int $expectedStarts): void { $this->workerConfiguration->expects($this->once()) ->method('isAsyncLocalEnabled') ->willReturn(true); $this->workerConfiguration->expects($this->once()) ->method('getDesiredWorkerCount') - ->willReturn(4); + ->willReturn($desired); - $this->workerCounter->expects($this->once()) + $this->processManager->expects($this->once()) ->method('countRunning') - ->willReturn(2); + ->with('worker') + ->willReturn($running); $this->workerJobCounter->expects($this->once()) ->method('countPendingJobs') - ->willReturn(10); + ->willReturn($pendingJobs); $this->startThrottlePolicy->expects($this->once()) ->method('isThrottled') @@ -164,130 +155,52 @@ public function testEnsureWorkerRunningStartsWhenNotThrottled(): void { $this->workerStarter->expects($this->once()) ->method('startWorkers') - ->with(2); + ->with($expectedStarts); $service = $this->makeService(); $this->assertTrue($service->ensureWorkerRunning()); } - public function testEnsureWorkerRunningStartsOnlyNeededWorkersBasedOnPendingJobs(): void { - $this->workerConfiguration->expects($this->once()) - ->method('isAsyncLocalEnabled') - ->willReturn(true); - - $this->workerConfiguration->expects($this->once()) - ->method('getDesiredWorkerCount') - ->willReturn(10); - - $this->workerCounter->expects($this->once()) - ->method('countRunning') - ->willReturn(0); - - $this->workerJobCounter->expects($this->once()) - ->method('countPendingJobs') - ->willReturn(3); - - $this->startThrottlePolicy->expects($this->once()) - ->method('isThrottled') - ->willReturn(false); - - $this->startThrottlePolicy->expects($this->once()) - ->method('recordAttempt'); - - // Should only start 3 workers because there are only 3 pending jobs - $this->workerStarter->expects($this->once()) - ->method('startWorkers') - ->with(3); - - $service = $this->makeService(); - $this->assertTrue($service->ensureWorkerRunning()); + public static function providerStartWorkerScenarios(): array { + return [ + 'fills remaining desired capacity' => [4, 2, 10, 2], + 'limited by pending jobs with no running' => [10, 0, 3, 3], + 'limited by pending jobs with running workers' => [10, 3, 5, 5], + 'pending jobs smaller than desired gap' => [10, 1, 2, 2], + 'single pending job starts one worker' => [4, 0, 1, 1], + ]; } - public function testEnsureWorkerRunningLimitsWorkersByPendingJobsAndRunningWorkers(): void { + public function testEnsureWorkerRunningHandlesExceptions(): void { $this->workerConfiguration->expects($this->once()) ->method('isAsyncLocalEnabled') - ->willReturn(true); - - $this->workerConfiguration->expects($this->once()) - ->method('getDesiredWorkerCount') - ->willReturn(10); - - $this->workerCounter->expects($this->once()) - ->method('countRunning') - ->willReturn(3); - - $this->workerJobCounter->expects($this->once()) - ->method('countPendingJobs') - ->willReturn(5); - - $this->startThrottlePolicy->expects($this->once()) - ->method('isThrottled') - ->willReturn(false); - - $this->startThrottlePolicy->expects($this->once()) - ->method('recordAttempt'); + ->will($this->throwException(new \RuntimeException('Config error'))); - // Pending jobs = 5, Running = 3, Desired = 10 - // Should start min(5, 10-3) = min(5, 7) = 5 workers - $this->workerStarter->expects($this->once()) - ->method('startWorkers') - ->with(5); + $this->logger->expects($this->once()) + ->method('error') + ->with($this->stringContains('Failed to ensure worker is running')); $service = $this->makeService(); - $this->assertTrue($service->ensureWorkerRunning()); + $this->assertFalse($service->ensureWorkerRunning()); } - public function testEnsureWorkerRunningStartsFewerWorkersWhenLessPendingJobsThanGap(): void { + public function testEnsureWorkerRunningReturnsFalseWhenWorkerStarterFails(): void { $this->workerConfiguration->expects($this->once()) ->method('isAsyncLocalEnabled') ->willReturn(true); $this->workerConfiguration->expects($this->once()) ->method('getDesiredWorkerCount') - ->willReturn(10); - - $this->workerCounter->expects($this->once()) - ->method('countRunning') - ->willReturn(1); - - $this->workerJobCounter->expects($this->once()) - ->method('countPendingJobs') ->willReturn(2); - $this->startThrottlePolicy->expects($this->once()) - ->method('isThrottled') - ->willReturn(false); - - $this->startThrottlePolicy->expects($this->once()) - ->method('recordAttempt'); - - // Pending jobs = 2, Running = 1, Desired = 10 - // Gap would be 9, but only 2 jobs pending - // Should start min(2, 9) = 2 workers - $this->workerStarter->expects($this->once()) - ->method('startWorkers') - ->with(2); - - $service = $this->makeService(); - $this->assertTrue($service->ensureWorkerRunning()); - } - - public function testEnsureWorkerRunningStartsOneWorkerWhenOnlyOneJobPending(): void { - $this->workerConfiguration->expects($this->once()) - ->method('isAsyncLocalEnabled') - ->willReturn(true); - - $this->workerConfiguration->expects($this->once()) - ->method('getDesiredWorkerCount') - ->willReturn(4); - - $this->workerCounter->expects($this->once()) + $this->processManager->expects($this->once()) ->method('countRunning') + ->with('worker') ->willReturn(0); $this->workerJobCounter->expects($this->once()) ->method('countPendingJobs') - ->willReturn(1); + ->willReturn(2); $this->startThrottlePolicy->expects($this->once()) ->method('isThrottled') @@ -296,25 +209,28 @@ public function testEnsureWorkerRunningStartsOneWorkerWhenOnlyOneJobPending(): v $this->startThrottlePolicy->expects($this->once()) ->method('recordAttempt'); - // Should only start 1 worker for 1 job $this->workerStarter->expects($this->once()) ->method('startWorkers') - ->with(1); + ->with(2) + ->will($this->throwException(new \RuntimeException('process launch failed'))); + + $this->logger->expects($this->once()) + ->method('error') + ->with( + $this->stringContains('Failed to ensure worker is running'), + $this->arrayHasKey('exception') + ); $service = $this->makeService(); - $this->assertTrue($service->ensureWorkerRunning()); + $this->assertFalse($service->ensureWorkerRunning()); } - public function testEnsureWorkerRunningHandlesExceptions(): void { + public function testIsAsyncLocalEnabledDelegatesToConfiguration(): void { $this->workerConfiguration->expects($this->once()) ->method('isAsyncLocalEnabled') - ->will($this->throwException(new \RuntimeException('Config error'))); - - $this->logger->expects($this->once()) - ->method('error') - ->with($this->stringContains('Failed to ensure worker is running')); + ->willReturn(true); $service = $this->makeService(); - $this->assertFalse($service->ensureWorkerRunning()); + $this->assertTrue($service->isAsyncLocalEnabled()); } } From cc30543b74bf016ab0a2dda92188c60c5c8a7acb Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:25:29 -0300 Subject: [PATCH 16/27] test(worker): improve worker job counter scenarios Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../Service/Worker/WorkerJobCounterTest.php | 175 ++++++++++++++++++ 1 file changed, 175 insertions(+) diff --git a/tests/php/Unit/Service/Worker/WorkerJobCounterTest.php b/tests/php/Unit/Service/Worker/WorkerJobCounterTest.php index 842eb77791..bd93bd439a 100644 --- a/tests/php/Unit/Service/Worker/WorkerJobCounterTest.php +++ b/tests/php/Unit/Service/Worker/WorkerJobCounterTest.php @@ -248,4 +248,179 @@ public function testCountPendingJobsHandlesLargeNumbers(): void { $counter = $this->makeCounter(); $this->assertSame(1003, $counter->countPendingJobs()); } + + public function testCountPendingJobsTreatsNonArraySignFileArgumentAsSingleWorkUnit(): void { + $signFileJob = $this->createMock(SignFileJob::class); + $signFileJob->method('getArgument') + ->willReturn('invalid'); + + $signFileJobs = [$signFileJob]; + $signSingleJobs = []; + $callIndex = 0; + $this->jobList->expects($this->exactly(2)) + ->method('getJobsIterator') + ->willReturnCallback(function (string $class) use (&$callIndex, $signFileJobs, $signSingleJobs) { + $callIndex++; + if ($callIndex === 1) { + $this->assertSame(SignFileJob::class, $class); + return $signFileJobs; + } + $this->assertSame(SignSingleFileJob::class, $class); + return $signSingleJobs; + }); + + $this->fileMapper->expects($this->never()) + ->method('getById'); + + $counter = $this->makeCounter(); + $this->assertSame(1, $counter->countPendingJobs()); + } + + public function testCountPendingJobsTreatsMissingFileIdAsSingleWorkUnit(): void { + $signFileJob = $this->makeSignFileJob([]); + + $signFileJobs = [$signFileJob]; + $signSingleJobs = []; + $callIndex = 0; + $this->jobList->expects($this->exactly(2)) + ->method('getJobsIterator') + ->willReturnCallback(function (string $class) use (&$callIndex, $signFileJobs, $signSingleJobs) { + $callIndex++; + if ($callIndex === 1) { + $this->assertSame(SignFileJob::class, $class); + return $signFileJobs; + } + $this->assertSame(SignSingleFileJob::class, $class); + return $signSingleJobs; + }); + + $this->fileMapper->expects($this->never()) + ->method('getById'); + + $counter = $this->makeCounter(); + $this->assertSame(1, $counter->countPendingJobs()); + } + + public function testCountPendingJobsTreatsMapperLookupFailureAsSingleWorkUnit(): void { + $signFileJob = $this->makeSignFileJob(['fileId' => 77]); + + $signFileJobs = [$signFileJob]; + $signSingleJobs = []; + $callIndex = 0; + $this->jobList->expects($this->exactly(2)) + ->method('getJobsIterator') + ->willReturnCallback(function (string $class) use (&$callIndex, $signFileJobs, $signSingleJobs) { + $callIndex++; + if ($callIndex === 1) { + $this->assertSame(SignFileJob::class, $class); + return $signFileJobs; + } + $this->assertSame(SignSingleFileJob::class, $class); + return $signSingleJobs; + }); + + $this->fileMapper->expects($this->once()) + ->method('getById') + ->with(77) + ->will($this->throwException(new \RuntimeException('not found'))); + + $counter = $this->makeCounter(); + $this->assertSame(1, $counter->countPendingJobs()); + } + + public function testCountPendingJobsKeepsAtLeastOneWorkUnitForEmptyEnvelope(): void { + $signFileJob = $this->makeSignFileJob(['fileId' => 33]); + $envelope = new FileEntity(); + $envelope->setId(33); + $envelope->setNodeTypeEnum(NodeType::ENVELOPE); + + $signFileJobs = [$signFileJob]; + $signSingleJobs = []; + $callIndex = 0; + $this->jobList->expects($this->exactly(2)) + ->method('getJobsIterator') + ->willReturnCallback(function (string $class) use (&$callIndex, $signFileJobs, $signSingleJobs) { + $callIndex++; + if ($callIndex === 1) { + $this->assertSame(SignFileJob::class, $class); + return $signFileJobs; + } + $this->assertSame(SignSingleFileJob::class, $class); + return $signSingleJobs; + }); + + $this->fileMapper->expects($this->once()) + ->method('getById') + ->with(33) + ->willReturn($envelope); + + $this->fileMapper->expects($this->once()) + ->method('countChildrenFiles') + ->with(33) + ->willReturn(0); + + $counter = $this->makeCounter(); + $this->assertSame(1, $counter->countPendingJobs()); + } + + public function testCountPendingJobsCastsStringFileIdToInt(): void { + $signFileJob = $this->makeSignFileJob(['fileId' => '123']); + $envelope = new FileEntity(); + $envelope->setId(123); + $envelope->setNodeTypeEnum(NodeType::ENVELOPE); + + $signFileJobs = [$signFileJob]; + $signSingleJobs = []; + $callIndex = 0; + $this->jobList->expects($this->exactly(2)) + ->method('getJobsIterator') + ->willReturnCallback(function (string $class) use (&$callIndex, $signFileJobs, $signSingleJobs) { + $callIndex++; + if ($callIndex === 1) { + $this->assertSame(SignFileJob::class, $class); + return $signFileJobs; + } + $this->assertSame(SignSingleFileJob::class, $class); + return $signSingleJobs; + }); + + $this->fileMapper->expects($this->once()) + ->method('getById') + ->with(123) + ->willReturn($envelope); + + $this->fileMapper->expects($this->once()) + ->method('countChildrenFiles') + ->with(123) + ->willReturn(2); + + $counter = $this->makeCounter(); + $this->assertSame(2, $counter->countPendingJobs()); + } + + public function testCountPendingJobsReturnsZeroWhenSecondIteratorFails(): void { + $signFileJob = $this->makeSignFileJob([]); + $callIndex = 0; + $this->jobList->expects($this->exactly(2)) + ->method('getJobsIterator') + ->willReturnCallback(function (string $class) use (&$callIndex, $signFileJob) { + $callIndex++; + if ($callIndex === 1) { + $this->assertSame(SignFileJob::class, $class); + return [$signFileJob]; + } + $this->assertSame(SignSingleFileJob::class, $class); + throw new \RuntimeException('queue unavailable'); + }); + + $this->logger->expects($this->once()) + ->method('debug') + ->with( + $this->stringContains('Failed to count pending jobs'), + $this->arrayHasKey('error') + ); + + $counter = $this->makeCounter(); + $this->assertSame(0, $counter->countPendingJobs()); + } } From 3bbd4e4c456430328608230f070117b5af5a7d29 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:25:29 -0300 Subject: [PATCH 17/27] chore(psalm): update baseline Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- tests/psalm-baseline.xml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 8b30f34b7b..4da4019583 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -139,9 +139,4 @@ - - - - - From ca0641e3f40e6b0fd0fab3bd163c2d0cf315f37a Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:30:37 -0300 Subject: [PATCH 18/27] chore(deps): update 3rdparty pointer after ci fix Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- 3rdparty | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/3rdparty b/3rdparty index fb4e8a634e..b03616033b 160000 --- a/3rdparty +++ b/3rdparty @@ -1 +1 @@ -Subproject commit fb4e8a634e9cdc4c8893816d537bb337bbeeca16 +Subproject commit b03616033b9b58b41e996372f624af3e18e201ba From 7709632d63ffa3c0b774d12e0fff7398388bc641 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 18:51:19 -0300 Subject: [PATCH 19/27] chore(deps): update 3rdparty submodule to main Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- 3rdparty | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/3rdparty b/3rdparty index b03616033b..15f2ec9272 160000 --- a/3rdparty +++ b/3rdparty @@ -1 +1 @@ -Subproject commit b03616033b9b58b41e996372f624af3e18e201ba +Subproject commit 15f2ec927260aa6b64667b0b39579968b7609e9e From 99b73a44c8673fe8db57124fc8715e594c523f88 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 18:51:25 -0300 Subject: [PATCH 20/27] refactor(process): orchestrate manager with collaborators Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/Process/ProcessManager.php | 140 +++++++++++++++++++------ 1 file changed, 107 insertions(+), 33 deletions(-) diff --git a/lib/Service/Process/ProcessManager.php b/lib/Service/Process/ProcessManager.php index b7864f39a8..b85658334b 100644 --- a/lib/Service/Process/ProcessManager.php +++ b/lib/Service/Process/ProcessManager.php @@ -14,11 +14,19 @@ class ProcessManager { private const APP_CONFIG_KEY = 'process_registry'; + private const APP_CONFIG_HINTS_KEY = 'process_registry_hints'; + + private ProcessSignaler $signaler; + private ListeningPidResolver $pidResolver; public function __construct( private IAppConfig $appConfig, private LoggerInterface $logger, + ?ProcessSignaler $signaler = null, + ?ListeningPidResolver $pidResolver = null, ) { + $this->signaler = $signaler ?? new ProcessSignaler($logger); + $this->pidResolver = $pidResolver ?? new ListeningPidResolver(); } /** @@ -36,6 +44,7 @@ public function register(string $source, int $pid, array $context = []): void { 'createdAt' => time(), ]; $this->saveRegistry($registry); + $this->setSourceHint($source, $context); } public function unregister(string $source, int $pid): void { @@ -83,6 +92,16 @@ public function countRunning(string $source): int { * @param null|callable(array{pid: int, context: array, createdAt: int}): bool $filter */ public function findRunningPid(string $source, ?callable $filter = null): int { + $entries = $this->listRunning($source); + foreach ($entries as $entry) { + if ($filter !== null && !$filter($entry)) { + continue; + } + return $entry['pid']; + } + + $this->hydrateFromFallback($source); + foreach ($this->listRunning($source) as $entry) { if ($filter !== null && !$filter($entry)) { continue; @@ -96,7 +115,7 @@ public function findRunningPid(string $source, ?callable $filter = null): int { public function stopAll(string $source, int $signal = SIGTERM): int { $stopped = 0; foreach ($this->listRunning($source) as $entry) { - if ($this->terminate($entry['pid'], $signal)) { + if ($this->stopPid($entry['pid'], $signal)) { $stopped++; } $this->unregister($source, $entry['pid']); @@ -105,55 +124,89 @@ public function stopAll(string $source, int $signal = SIGTERM): int { return $stopped; } - public function isRunning(int $pid): bool { - if ($pid <= 0) { - return false; + public function stopPid(int $pid, int $signal = SIGTERM): bool { + return $this->signaler->stopPid($pid, $signal); + } + + /** + * @param array $context + */ + public function setSourceHint(string $source, array $context): void { + if ($source === '' || $context === []) { + return; } - if (function_exists('posix_kill')) { - return @posix_kill($pid, 0); + $hints = $this->getHints(); + $hints[$source] = $context; + $this->saveHints($hints); + } + + /** + * @return int[] + */ + public function findListeningPids(int $port): array { + return $this->pidResolver->findListeningPids($port); + } + + public function isRunning(int $pid): bool { + return $this->signaler->isRunning($pid); + } + + private function hydrateFromFallback(string $source): void { + $hint = $this->getSourceHint($source); + if (!is_array($hint) || $hint === []) { + return; } - if (!function_exists('exec')) { - return false; + $port = $this->getPortFromHint($hint); + if ($port <= 0) { + return; } try { - exec(sprintf('kill -0 %d 2>/dev/null', $pid), $output, $exitCode); - return $exitCode === 0; - } catch (\Throwable $e) { - $this->logger->debug('Failed to probe process status', [ - 'pid' => $pid, + $pids = $this->findListeningPids($port); + } catch (\RuntimeException $e) { + $this->logger->debug('Unable to hydrate process registry from fallback', [ + 'source' => $source, + 'port' => $port, 'error' => $e->getMessage(), ]); - return false; + return; } - } - protected function terminate(int $pid, int $signal): bool { - if ($pid <= 0) { - return false; + foreach ($pids as $pid) { + if ($pid <= 0) { + continue; + } + $this->register($source, $pid, $hint); } + } - if (function_exists('posix_kill')) { - return @posix_kill($pid, $signal); + /** + * @param array $hint + */ + private function getPortFromHint(array $hint): int { + if (isset($hint['port']) && is_numeric((string)$hint['port'])) { + $port = (int)$hint['port']; + if ($port > 0) { + return $port; + } } - if (!function_exists('exec')) { - return false; + if (isset($hint['uri']) && is_string($hint['uri'])) { + return (int)(parse_url($hint['uri'], PHP_URL_PORT) ?? 0); } - try { - exec(sprintf('kill -%d %d 2>/dev/null', $signal, $pid), $output, $exitCode); - return $exitCode === 0; - } catch (\Throwable $e) { - $this->logger->debug('Failed to terminate process', [ - 'pid' => $pid, - 'signal' => $signal, - 'error' => $e->getMessage(), - ]); - return false; - } + return 0; + } + + /** + * @return array + */ + private function getSourceHint(string $source): array { + $hints = $this->getHints(); + $hint = $hints[$source] ?? []; + return is_array($hint) ? $hint : []; } /** @@ -175,4 +228,25 @@ private function getRegistry(): array { private function saveRegistry(array $registry): void { $this->appConfig->setValueString(Application::APP_ID, self::APP_CONFIG_KEY, json_encode($registry)); } + + /** + * @return array> + */ + private function getHints(): array { + $raw = $this->appConfig->getValueString(Application::APP_ID, self::APP_CONFIG_HINTS_KEY, '{}'); + $decoded = json_decode($raw, true); + if (!is_array($decoded)) { + return []; + } + + return $decoded; + } + + /** + * @param array> $hints + */ + private function saveHints(array $hints): void { + $this->appConfig->setValueString(Application::APP_ID, self::APP_CONFIG_HINTS_KEY, json_encode($hints)); + } + } From d6630156eb46169d5ca75302dd2852d809291695 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 18:51:25 -0300 Subject: [PATCH 21/27] feat(process): add listening pid resolver Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/Process/ListeningPidResolver.php | 211 +++++++++++++++++++ 1 file changed, 211 insertions(+) create mode 100644 lib/Service/Process/ListeningPidResolver.php diff --git a/lib/Service/Process/ListeningPidResolver.php b/lib/Service/Process/ListeningPidResolver.php new file mode 100644 index 0000000000..d6148c0a9c --- /dev/null +++ b/lib/Service/Process/ListeningPidResolver.php @@ -0,0 +1,211 @@ +findListeningPidsUsingSs($port); + if ($ss !== null) { + $usedStrategy = true; + $pids = array_merge($pids, $ss); + } + + $lsof = $this->findListeningPidsUsingLsof($port); + if ($lsof !== null) { + $usedStrategy = true; + $pids = array_merge($pids, $lsof); + } + + $proc = $this->findListeningPidsUsingProc($port); + if ($proc !== null) { + $usedStrategy = true; + $pids = array_merge($pids, $proc); + } + + if (!$usedStrategy) { + throw new \RuntimeException('Unable to inspect listening process PIDs: no strategy available.'); + } + + return array_values(array_filter( + array_unique(array_map('intval', $pids)), + static fn (int $pid): bool => $pid > 0, + )); + } + + /** + * @return int[]|null null means strategy unavailable + */ + protected function findListeningPidsUsingSs(int $port): ?array { + if (!$this->commandIsAvailable('ss')) { + return null; + } + + $process = $this->createProcess(['ss', '-ltnp', 'sport = :' . $port]); + $process->run(); + if (!$process->isSuccessful()) { + return []; + } + + $output = $process->getOutput(); + preg_match_all('/pid=(\d+)/', $output, $matches); + if (!isset($matches[1]) || !is_array($matches[1])) { + return []; + } + + return array_map('intval', $matches[1]); + } + + /** + * @return int[]|null null means strategy unavailable + */ + protected function findListeningPidsUsingLsof(int $port): ?array { + if (!$this->commandIsAvailable('lsof')) { + return null; + } + + $process = $this->createProcess(['lsof', '-ti', 'tcp:' . $port, '-sTCP:LISTEN']); + $process->run(); + if (!$process->isSuccessful()) { + return []; + } + + $lines = preg_split('/\R/', trim($process->getOutput())); + if (!is_array($lines)) { + return []; + } + + return array_map('intval', array_filter($lines, static fn (string $line): bool => $line !== '')); + } + + /** + * @return int[]|null null means strategy unavailable + */ + protected function findListeningPidsUsingProc(int $port): ?array { + if (PHP_OS_FAMILY !== 'Linux') { + return null; + } + + if (!is_readable('/proc/net/tcp') && !is_readable('/proc/net/tcp6')) { + return null; + } + + $inodesByPort = $this->getListeningSocketInodesByPort($port); + if (empty($inodesByPort)) { + return []; + } + + $fdPaths = glob('/proc/[0-9]*/fd/[0-9]*'); + if (!is_array($fdPaths)) { + return []; + } + + $pids = []; + foreach ($fdPaths as $fdPath) { + $target = @readlink($fdPath); + if (!is_string($target) || !preg_match('/^socket:\\[(\\d+)\\]$/', $target, $matches)) { + continue; + } + + $inode = $matches[1] ?? ''; + if ($inode === '' || !isset($inodesByPort[$inode])) { + continue; + } + + if (preg_match('#^/proc/([0-9]+)/fd/[0-9]+$#', $fdPath, $pidMatches)) { + $pids[] = (int)$pidMatches[1]; + } + } + + return $pids; + } + + /** + * @return array + */ + protected function getListeningSocketInodesByPort(int $port): array { + $portHex = strtoupper(str_pad(dechex($port), 4, '0', STR_PAD_LEFT)); + $inodes = []; + + foreach (['/proc/net/tcp', '/proc/net/tcp6'] as $path) { + foreach ($this->readListeningInodesFromProcTable($path, $portHex) as $inode) { + $inodes[$inode] = true; + } + } + + return $inodes; + } + + /** + * @return string[] + */ + protected function readListeningInodesFromProcTable(string $path, string $portHex): array { + if (!is_readable($path)) { + return []; + } + + $lines = file($path, FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES); + if (!is_array($lines) || count($lines) <= 1) { + return []; + } + + $inodes = []; + foreach (array_slice($lines, 1) as $line) { + $columns = preg_split('/\s+/', trim($line)); + if (!is_array($columns) || count($columns) < 10) { + continue; + } + + $localAddress = $columns[1]; + $state = $columns[3]; + $inode = $columns[9] ?? ''; + + if ($state !== '0A' || !is_string($inode) || $inode === '') { + continue; + } + + $addressParts = explode(':', $localAddress); + if (count($addressParts) !== 2) { + continue; + } + + if (strtoupper($addressParts[1]) !== $portHex) { + continue; + } + + $inodes[] = $inode; + } + + return $inodes; + } + + protected function commandIsAvailable(string $command): bool { + $process = $this->createProcess(['/bin/sh', '-lc', 'command -v ' . escapeshellarg($command) . ' >/dev/null 2>&1']); + $process->run(); + return $process->isSuccessful(); + } + + /** + * @param string[] $command + */ + protected function createProcess(array $command): Process { + return new Process($command); + } +} From d53ff8a6a6a94402f701d04f9b9e0cfb9e672ae4 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 18:51:31 -0300 Subject: [PATCH 22/27] feat(process): add process signaler service Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/Process/ProcessSignaler.php | 70 +++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 lib/Service/Process/ProcessSignaler.php diff --git a/lib/Service/Process/ProcessSignaler.php b/lib/Service/Process/ProcessSignaler.php new file mode 100644 index 0000000000..2e6274bd9f --- /dev/null +++ b/lib/Service/Process/ProcessSignaler.php @@ -0,0 +1,70 @@ +/dev/null', $pid), $output, $exitCode); + return $exitCode === 0; + } catch (\Throwable $e) { + $this->logger->debug('Failed to probe process status', [ + 'pid' => $pid, + 'error' => $e->getMessage(), + ]); + return false; + } + } + + public function stopPid(int $pid, int $signal = SIGTERM): bool { + if ($pid <= 0) { + return false; + } + + if (function_exists('posix_kill')) { + return @posix_kill($pid, $signal); + } + + if (!function_exists('exec')) { + return false; + } + + try { + exec(sprintf('kill -%d %d 2>/dev/null', $signal, $pid), $output, $exitCode); + return $exitCode === 0; + } catch (\Throwable $e) { + $this->logger->debug('Failed to terminate process', [ + 'pid' => $pid, + 'signal' => $signal, + 'error' => $e->getMessage(), + ]); + return false; + } + } + +} From 5db20e4127de8ba5fc3c66b384b3bfb5afdded02 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 18:51:36 -0300 Subject: [PATCH 23/27] refactor(cfssl): use process manager fallback orchestration Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../CertificateEngine/CfsslHandler.php | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/lib/Handler/CertificateEngine/CfsslHandler.php b/lib/Handler/CertificateEngine/CfsslHandler.php index 61d2c9e3cb..d55adf4074 100644 --- a/lib/Handler/CertificateEngine/CfsslHandler.php +++ b/lib/Handler/CertificateEngine/CfsslHandler.php @@ -376,25 +376,29 @@ private function getServerPid(): int { ); } - /** - * Parse command - * - * Have commands that need to be executed as sudo otherwise don't will work, - * by example the command runuser or kill. To prevent error when run in a - * GitHub Actions, these commands are executed prefixed by sudo when exists - * an environment called GITHUB_ACTIONS. - */ - private function parseCommand(string $command): string { - if (getenv('GITHUB_ACTIONS') !== false) { - $command = 'sudo ' . $command; - } - return $command; - } - private function stopIfRunning(): void { - $pid = $this->getServerPid(); - if ($pid > 0) { - exec($this->parseCommand('kill -9 ' . $pid)); + $uri = $this->getCfsslUri(); + $port = (int)(parse_url($uri, PHP_URL_PORT) ?? 0); + $this->processManager->setSourceHint(self::PROCESS_SOURCE, [ + 'uri' => $uri, + 'port' => $port, + ]); + + $this->processManager->findRunningPid( + self::PROCESS_SOURCE, + fn (array $entry): bool => ($entry['context']['uri'] ?? '') === $uri, + ); + + foreach ($this->processManager->listRunning(self::PROCESS_SOURCE) as $entry) { + if (($entry['context']['uri'] ?? '') !== $uri) { + continue; + } + + $pid = (int)($entry['pid'] ?? 0); + if ($pid <= 0) { + continue; + } + $this->processManager->stopPid($pid, SIGKILL); $this->processManager->unregister(self::PROCESS_SOURCE, $pid); } } From 04f3347968d62c6213d5424e2cda35a83c55fa6f Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 18:51:40 -0300 Subject: [PATCH 24/27] test(process): cover manager fallback business rules Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../Service/Process/ProcessManagerTest.php | 162 +++++++++++++++--- 1 file changed, 134 insertions(+), 28 deletions(-) diff --git a/tests/php/Unit/Service/Process/ProcessManagerTest.php b/tests/php/Unit/Service/Process/ProcessManagerTest.php index 3c7f99412f..8934ae4210 100644 --- a/tests/php/Unit/Service/Process/ProcessManagerTest.php +++ b/tests/php/Unit/Service/Process/ProcessManagerTest.php @@ -9,9 +9,12 @@ namespace OCA\Libresign\Tests\Unit\Service\Process; use OCA\Libresign\AppInfo\Application; +use OCA\Libresign\Service\Process\ListeningPidResolver; use OCA\Libresign\Service\Process\ProcessManager; +use OCA\Libresign\Service\Process\ProcessSignaler; use OCA\Libresign\Tests\Unit\TestCase; use OCP\IAppConfig; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -29,18 +32,21 @@ public function setUp(): void { } /** - * @return array{0: ProcessManager, 1: string} + * @return array{0: ProcessManager, 1: array} */ private function makeManagerWithStoredRegistry(?callable $factory = null): array { - $stored = '{}'; + $storedByKey = [ + 'process_registry' => '{}', + 'process_registry_hints' => '{}', + ]; $this->appConfig->method('getValueString') - ->willReturnCallback(function (string $_appId, string $_key, string $default) use (&$stored): string { - return $stored ?: $default; + ->willReturnCallback(function (string $_appId, string $key, string $default) use (&$storedByKey): string { + return $storedByKey[$key] ?? $default; }); $this->appConfig->method('setValueString') - ->willReturnCallback(function (string $_appId, string $_key, string $value) use (&$stored): bool { - $stored = $value; + ->willReturnCallback(function (string $_appId, string $key, string $value) use (&$storedByKey): bool { + $storedByKey[$key] = $value; return true; }); @@ -48,21 +54,24 @@ private function makeManagerWithStoredRegistry(?callable $factory = null): array ? $factory($this->appConfig, $this->logger) : new ProcessManager($this->appConfig, $this->logger); - return [$manager, $stored]; + return [$manager, $storedByKey]; } public function testListRunningPurgesStoppedPids(): void { - $stored = '{}'; + $storedByKey = [ + 'process_registry' => '{}', + 'process_registry_hints' => '{}', + ]; $this->appConfig->method('getValueString') - ->willReturnCallback(function (string $appId, string $key, string $default) use (&$stored): string { + ->willReturnCallback(function (string $appId, string $key, string $default) use (&$storedByKey): string { $this->assertSame(Application::APP_ID, $appId); - $this->assertSame('process_registry', $key); - return $stored ?: $default; + $this->assertContains($key, ['process_registry', 'process_registry_hints']); + return $storedByKey[$key] ?? $default; }); $this->appConfig->method('setValueString') - ->willReturnCallback(function (string $_appId, string $_key, string $value) use (&$stored): bool { - $stored = $value; + ->willReturnCallback(function (string $_appId, string $key, string $value) use (&$storedByKey): bool { + $storedByKey[$key] = $value; return true; }); @@ -81,19 +90,29 @@ public function isRunning(int $pid): bool { $this->assertSame(111, $running[0]['pid']); } - public function testRegisterIgnoresInvalidPid(): void { + #[DataProvider('provideInvalidPids')] + public function testRegisterIgnoresInvalidPid(int $invalidPid): void { [$manager] = $this->makeManagerWithStoredRegistry(fn (IAppConfig $appConfig, LoggerInterface $logger): ProcessManager => new class($appConfig, $logger) extends ProcessManager { public function isRunning(int $pid): bool { return true; } }); - $manager->register(self::WORKER_SOURCE, 0); - $manager->register(self::WORKER_SOURCE, -9); + $manager->register(self::WORKER_SOURCE, $invalidPid); $this->assertSame(0, $manager->countRunning(self::WORKER_SOURCE)); } + /** + * @return array + */ + public static function provideInvalidPids(): array { + return [ + 'zero' => [0], + 'negative' => [-9], + ]; + } + public function testFindRunningPidReturnsFirstWhenNoFilterProvided(): void { [$manager] = $this->makeManagerWithStoredRegistry(fn (IAppConfig $appConfig, LoggerInterface $logger): ProcessManager => new class($appConfig, $logger) extends ProcessManager { public function isRunning(int $pid): bool { @@ -142,6 +161,49 @@ public function isRunning(int $pid): bool { $this->assertSame(0, $actual); } + public function testFindRunningPidHydratesRegistryWhenHintExists(): void { + $resolver = $this->createMock(ListeningPidResolver::class); + $resolver->expects($this->once()) + ->method('findListeningPids') + ->with(8888) + ->willReturn([777]); + + $signaler = $this->createMock(ProcessSignaler::class); + $signaler->method('isRunning') + ->willReturn(true); + + [$manager] = $this->makeManagerWithStoredRegistry( + fn (IAppConfig $appConfig, LoggerInterface $logger): ProcessManager + => new ProcessManager($appConfig, $logger, $signaler, $resolver) + ); + + $manager->setSourceHint(self::INSTALL_SOURCE, [ + 'uri' => 'http://127.0.0.1:8888/api/v1/cfssl/', + 'port' => 8888, + ]); + + $actual = $manager->findRunningPid(self::INSTALL_SOURCE); + + $this->assertSame(777, $actual); + } + + public function testFindRunningPidSkipsFallbackWhenNoHintExists(): void { + $resolver = $this->createMock(ListeningPidResolver::class); + $resolver->expects($this->never()) + ->method('findListeningPids'); + + $signaler = $this->createMock(ProcessSignaler::class); + $signaler->method('isRunning') + ->willReturn(true); + + [$manager] = $this->makeManagerWithStoredRegistry( + fn (IAppConfig $appConfig, LoggerInterface $logger): ProcessManager + => new ProcessManager($appConfig, $logger, $signaler, $resolver) + ); + + $this->assertSame(0, $manager->findRunningPid(self::INSTALL_SOURCE)); + } + public function testIsRunningReturnsFalseForInvalidPid(): void { $manager = new ProcessManager($this->appConfig, $this->logger); @@ -150,15 +212,18 @@ public function testIsRunningReturnsFalseForInvalidPid(): void { } public function testStopAllTerminatesAndUnregistersTrackedPids(): void { - $stored = '{}'; + $storedByKey = [ + 'process_registry' => '{}', + 'process_registry_hints' => '{}', + ]; $this->appConfig->method('getValueString') - ->willReturnCallback(function (string $_appId, string $_key, string $default) use (&$stored): string { - return $stored ?: $default; + ->willReturnCallback(function (string $_appId, string $key, string $default) use (&$storedByKey): string { + return $storedByKey[$key] ?? $default; }); $this->appConfig->method('setValueString') - ->willReturnCallback(function (string $_appId, string $_key, string $value) use (&$stored): bool { - $stored = $value; + ->willReturnCallback(function (string $_appId, string $key, string $value) use (&$storedByKey): bool { + $storedByKey[$key] = $value; return true; }); @@ -170,7 +235,7 @@ public function isRunning(int $pid): bool { return true; } - protected function terminate(int $pid, int $signal): bool { + public function stopPid(int $pid, int $signal = SIGTERM): bool { $this->terminated[] = $pid; return true; } @@ -187,15 +252,18 @@ protected function terminate(int $pid, int $signal): bool { } public function testStopAllUsesProvidedSignal(): void { - $stored = '{}'; + $storedByKey = [ + 'process_registry' => '{}', + 'process_registry_hints' => '{}', + ]; $this->appConfig->method('getValueString') - ->willReturnCallback(function (string $_appId, string $_key, string $default) use (&$stored): string { - return $stored ?: $default; + ->willReturnCallback(function (string $_appId, string $key, string $default) use (&$storedByKey): string { + return $storedByKey[$key] ?? $default; }); $this->appConfig->method('setValueString') - ->willReturnCallback(function (string $_appId, string $_key, string $value) use (&$stored): bool { - $stored = $value; + ->willReturnCallback(function (string $_appId, string $key, string $value) use (&$storedByKey): bool { + $storedByKey[$key] = $value; return true; }); @@ -207,7 +275,7 @@ public function isRunning(int $pid): bool { return true; } - protected function terminate(int $pid, int $signal): bool { + public function stopPid(int $pid, int $signal = SIGTERM): bool { $this->signals[] = $signal; return true; } @@ -220,4 +288,42 @@ protected function terminate(int $pid, int $signal): bool { $this->assertSame([SIGKILL, SIGKILL], $manager->signals); } + + public function testFindListeningPidsDelegatesToResolver(): void { + $resolver = $this->createMock(ListeningPidResolver::class); + $resolver->expects($this->once()) + ->method('findListeningPids') + ->with(8888) + ->willReturn([111, 222]); + + $signaler = $this->createMock(ProcessSignaler::class); + + $manager = new ProcessManager($this->appConfig, $this->logger, $signaler, $resolver); + + $this->assertSame([111, 222], $manager->findListeningPids(8888)); + } + + #[DataProvider('provideIsRunningDelegation')] + public function testIsRunningDelegatesToSignaler(int $pid, bool $expected): void { + $resolver = $this->createMock(ListeningPidResolver::class); + $signaler = $this->createMock(ProcessSignaler::class); + $signaler->expects($this->once()) + ->method('isRunning') + ->with($pid) + ->willReturn($expected); + + $manager = new ProcessManager($this->appConfig, $this->logger, $signaler, $resolver); + + $this->assertSame($expected, $manager->isRunning($pid)); + } + + /** + * @return array + */ + public static function provideIsRunningDelegation(): array { + return [ + 'running pid' => [111, true], + 'not running pid' => [222, false], + ]; + } } From 06e10a62d3ef22b62f49061dab6c7b73314884a6 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 18:51:47 -0300 Subject: [PATCH 25/27] test(process): add listening resolver unit coverage Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../Process/ListeningPidResolverTest.php | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 tests/php/Unit/Service/Process/ListeningPidResolverTest.php diff --git a/tests/php/Unit/Service/Process/ListeningPidResolverTest.php b/tests/php/Unit/Service/Process/ListeningPidResolverTest.php new file mode 100644 index 0000000000..a704788388 --- /dev/null +++ b/tests/php/Unit/Service/Process/ListeningPidResolverTest.php @@ -0,0 +1,77 @@ +expectException(\RuntimeException::class); + $this->expectExceptionMessage('Invalid port'); + + $resolver->findListeningPids($port); + } + + /** + * @return array + */ + public static function provideInvalidPorts(): array { + return [ + 'zero' => [0], + 'negative' => [-1], + ]; + } + + public function testFindListeningPidsMergesUniqueFromAvailableStrategies(): void { + $resolver = new class() extends ListeningPidResolver { + protected function findListeningPidsUsingSs(int $port): ?array { + return [100, 101]; + } + + protected function findListeningPidsUsingLsof(int $port): ?array { + return [101, 102]; + } + + protected function findListeningPidsUsingProc(int $port): ?array { + return [102, 103]; + } + }; + + $actual = $resolver->findListeningPids(8888); + sort($actual); + + $this->assertSame([100, 101, 102, 103], $actual); + } + + public function testFindListeningPidsThrowsWhenNoStrategyIsAvailable(): void { + $resolver = new class() extends ListeningPidResolver { + protected function findListeningPidsUsingSs(int $port): ?array { + return null; + } + + protected function findListeningPidsUsingLsof(int $port): ?array { + return null; + } + + protected function findListeningPidsUsingProc(int $port): ?array { + return null; + } + }; + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('no strategy available'); + + $resolver->findListeningPids(8888); + } +} From 979f0d9b3e21678343d82c0e413d01aced5e637c Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 18:51:52 -0300 Subject: [PATCH 26/27] test(process): add process signaler unit coverage Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../Service/Process/ProcessSignalerTest.php | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 tests/php/Unit/Service/Process/ProcessSignalerTest.php diff --git a/tests/php/Unit/Service/Process/ProcessSignalerTest.php b/tests/php/Unit/Service/Process/ProcessSignalerTest.php new file mode 100644 index 0000000000..d4026b9d74 --- /dev/null +++ b/tests/php/Unit/Service/Process/ProcessSignalerTest.php @@ -0,0 +1,40 @@ +createMock(LoggerInterface::class)); + + $this->assertFalse($signaler->isRunning($pid)); + } + + #[DataProvider('provideInvalidPids')] + public function testStopPidReturnsFalseForInvalidPid(int $pid): void { + $signaler = new ProcessSignaler($this->createMock(LoggerInterface::class)); + + $this->assertFalse($signaler->stopPid($pid)); + } + + /** + * @return array + */ + public static function provideInvalidPids(): array { + return [ + 'zero' => [0], + 'negative' => [-1], + ]; + } +} From 37610cb4be99357ace573504fa5935c6eb7b55d8 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 8 Apr 2026 18:52:00 -0300 Subject: [PATCH 27/27] test(cfssl): improve handler unit scenarios Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../CertificateEngine/CfsslHandlerTest.php | 189 +++++++++++------- 1 file changed, 120 insertions(+), 69 deletions(-) diff --git a/tests/php/Unit/Handler/CertificateEngine/CfsslHandlerTest.php b/tests/php/Unit/Handler/CertificateEngine/CfsslHandlerTest.php index 80fa0cc85b..cb85e0f9cc 100644 --- a/tests/php/Unit/Handler/CertificateEngine/CfsslHandlerTest.php +++ b/tests/php/Unit/Handler/CertificateEngine/CfsslHandlerTest.php @@ -24,6 +24,7 @@ use OCP\IDateTimeFormatter; use OCP\ITempManager; use OCP\IURLGenerator; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -60,25 +61,88 @@ public function testScopedProcessClassIsAvailable(): void { $this->assertTrue(class_exists(Process::class)); } - public function testCheckBinariesReturnsErrorWhenProcessFails(): void { + public function testStopIfRunningKillsRegisteredAndPortListenerPids(): void { + $unregisteredPids = []; + $stoppedPids = []; + $uri = CfsslHandler::CFSSL_URI; + $port = 8888; + + $this->processManager->expects($this->once()) + ->method('setSourceHint') + ->with(self::PROCESS_SOURCE, [ + 'uri' => $uri, + 'port' => $port, + ]); + + $this->processManager->expects($this->once()) + ->method('findRunningPid') + ->with(self::PROCESS_SOURCE, $this->callback('is_callable')) + ->willReturn(321); + + $this->processManager->expects($this->once()) + ->method('listRunning') + ->with(self::PROCESS_SOURCE) + ->willReturn([ + ['pid' => 321, 'context' => ['uri' => $uri], 'createdAt' => 1], + ['pid' => 654, 'context' => ['uri' => $uri], 'createdAt' => 2], + ]); + + $this->processManager->expects($this->exactly(2)) + ->method('unregister') + ->willReturnCallback(function (string $source, int $pid) use (&$unregisteredPids): void { + $this->assertSame(self::PROCESS_SOURCE, $source); + $unregisteredPids[] = $pid; + }); + + $this->processManager->expects($this->exactly(2)) + ->method('stopPid') + ->withAnyParameters() + ->willReturnCallback(function (int $pid, int $signal) use (&$stoppedPids): bool { + $this->assertSame(SIGKILL, $signal); + $stoppedPids[] = $pid; + return true; + }); + + $handler = $this->getMockBuilder(CfsslHandler::class) + ->setConstructorArgs($this->getConstructorArgs()) + ->onlyMethods(['createProcess']) + ->getMock(); + + $handler->method('createProcess') + ->willReturn($this->createMock(Process::class)); + + self::invokePrivate($handler, 'stopIfRunning'); + + sort($unregisteredPids); + sort($stoppedPids); + $this->assertSame([321, 654], $unregisteredPids); + $this->assertSame([321, 654], $stoppedPids); + } + + #[DataProvider('provideCheckBinariesErrorCases')] + public function testCheckBinariesReturnsErrorForInvalidProcessState( + bool $isSuccessful, + string $output, + string $expectedMessage, + ): void { $binary = tempnam(sys_get_temp_dir(), 'cfssl-bin-'); $this->assertNotFalse($binary); $process = $this->createMock(Process::class); $process->expects($this->once()) ->method('run'); - $process->expects($this->once()) - ->method('getOutput') - ->willReturn(''); $process->expects($this->once()) ->method('isSuccessful') - ->willReturn(false); + ->willReturn($isSuccessful); + $process->expects($this->once()) + ->method('getOutput') + ->willReturn($output); $handler = $this->createHandler($process, (string)$binary); $result = self::invokePrivate($handler, 'checkBinaries'); $this->assertSame('error', $result[0]->jsonSerialize()['status']); - $this->assertStringContainsString('Failed to run the command', $result[0]->jsonSerialize()['message']); + $this->assertStringContainsString($expectedMessage, $result[0]->jsonSerialize()['message']); @unlink((string)$binary); } @@ -108,53 +172,49 @@ public function testCheckBinariesReturnsSuccessWhenProcessOutputIsValid(): void @unlink((string)$binary); } - public function testCheckBinariesReturnsErrorWhenVersionOutputFormatIsInvalid(): void { - $binary = tempnam(sys_get_temp_dir(), 'cfssl-bin-'); - $this->assertNotFalse($binary); - $process = $this->createMock(Process::class); - $process->expects($this->once()) - ->method('run'); - $process->expects($this->once()) - ->method('isSuccessful') - ->willReturn(true); - $process->expects($this->once()) - ->method('getOutput') - ->willReturn('cfssl version output without expected separators'); - - $handler = $this->createHandler($process, (string)$binary); - $result = self::invokePrivate($handler, 'checkBinaries'); - - $this->assertSame('error', $result[0]->jsonSerialize()['status']); - $this->assertStringContainsString('Failed to identify cfssl version', $result[0]->jsonSerialize()['message']); - - @unlink((string)$binary); + /** + * @return array + */ + public static function provideCheckBinariesErrorCases(): array { + return [ + 'process failure without output' => [ + false, + '', + 'Failed to run the command', + ], + 'invalid version output format' => [ + true, + 'cfssl version output without expected separators', + 'Failed to identify cfssl version', + ], + 'version mismatch' => [ + true, + "Version: 0.0.1\nRuntime: go1.22\n", + 'Invalid version. Expected:', + ], + ]; } - public function testCheckBinariesReturnsErrorWhenCfsslVersionDoesNotMatchExpected(): void { - $binary = tempnam(sys_get_temp_dir(), 'cfssl-bin-'); - $this->assertNotFalse($binary); - - $process = $this->createMock(Process::class); - $process->expects($this->once()) - ->method('run'); - $process->expects($this->once()) - ->method('isSuccessful') - ->willReturn(true); - $process->expects($this->once()) - ->method('getOutput') - ->willReturn("Version: 0.0.1\nRuntime: go1.22\n"); + private function createHandler(?Process $process = null, ?string $binary = null): CfsslHandler { + $constructorArgs = $this->getConstructorArgs($binary); + $process ??= $this->createMock(Process::class); - $handler = $this->createHandler($process, (string)$binary); - $result = self::invokePrivate($handler, 'checkBinaries'); + $handler = $this->getMockBuilder(CfsslHandler::class) + ->setConstructorArgs($constructorArgs) + ->onlyMethods(['createProcess']) + ->getMock(); - $this->assertSame('error', $result[0]->jsonSerialize()['status']); - $this->assertStringContainsString('Invalid version. Expected:', $result[0]->jsonSerialize()['message']); + $handler->method('createProcess') + ->willReturn($process); - @unlink((string)$binary); + return $handler; } - private function createHandler(?Process $process = null, ?string $binary = null): CfsslHandler { + /** + * @return array + */ + private function getConstructorArgs(?string $binary = null): array { $config = \OCP\Server::get(IConfig::class); $appConfig = $this->getMockAppConfigWithReset(); if ($binary !== null) { @@ -180,30 +240,21 @@ private function createHandler(?Process $process = null, ?string $binary = null) $cfsslServerHandler = $this->createMock(CfsslServerHandler::class); $cfsslServerHandler->expects($this->once()) ->method('configCallback'); - $process ??= $this->createMock(Process::class); - - $handler = $this->getMockBuilder(CfsslHandler::class) - ->setConstructorArgs([ - $config, - $appConfig, - $appDataFactory, - $dateTimeFormatter, - $tempManager, - $cfsslServerHandler, - $certificatePolicyService, - $urlGenerator, - $caIdentifierService, - $crlMapper, - $logger, - $crlRevocationChecker, - $this->processManager, - ]) - ->onlyMethods(['createProcess']) - ->getMock(); - $handler->method('createProcess') - ->willReturn($process); - - return $handler; + return [ + $config, + $appConfig, + $appDataFactory, + $dateTimeFormatter, + $tempManager, + $cfsslServerHandler, + $certificatePolicyService, + $urlGenerator, + $caIdentifierService, + $crlMapper, + $logger, + $crlRevocationChecker, + $this->processManager, + ]; } }