From 60ce92a697dd357465db65f11e10c85cc5404d70 Mon Sep 17 00:00:00 2001 From: Benjamin Gaussorgues Date: Wed, 3 Jun 2026 18:26:46 +0200 Subject: [PATCH] feat(utils): add getter for serverid with proper default Signed-off-by: Benjamin Gaussorgues --- .../CleanupBackgroundJobsJob.php | 62 +++++++++++++++++++ core/Command/Background/JobsHistory.php | 6 +- core/Command/Background/RunningJobs.php | 6 +- lib/composer/composer/autoload_classmap.php | 2 + lib/composer/composer/autoload_static.php | 2 + .../Repair/AddCleanupBackgroundJobsJob.php | 33 ++++++++++ lib/private/Server.php | 4 ++ lib/private/ServerInfo.php | 48 ++++++++++++++ lib/private/Snowflake/SnowflakeGenerator.php | 24 +------ lib/public/IServerInfo.php | 33 ++++++++++ lib/public/Util.php | 8 +-- tests/lib/Snowflake/GeneratorTest.php | 18 +++--- 12 files changed, 205 insertions(+), 41 deletions(-) create mode 100644 core/BackgroundJobs/CleanupBackgroundJobsJob.php create mode 100644 lib/private/Repair/AddCleanupBackgroundJobsJob.php create mode 100644 lib/private/ServerInfo.php create mode 100644 lib/public/IServerInfo.php diff --git a/core/BackgroundJobs/CleanupBackgroundJobsJob.php b/core/BackgroundJobs/CleanupBackgroundJobsJob.php new file mode 100644 index 00000000000..69f4eb8fed6 --- /dev/null +++ b/core/BackgroundJobs/CleanupBackgroundJobsJob.php @@ -0,0 +1,62 @@ +setInterval(60 * 60); + $this->setTimeSensitivity(IJob::TIME_INSENSITIVE); + } + + #[Override] + protected function run($argument): void { + $this->reapCrashedJobs(); + + // TODO Clean oldest jobs + } + + private function reapCrashedJobs(): void { + $currentServerId = $this->serverInfo->getServerId(); + + foreach ($this->jobRuns->runningJobs(1000) as $job) { + if ($job->serverId !== $currentServerId) { + continue; + } + exec('ps -p ' . escapeshellarg((string)$job->pid) . ' -o cmd', $output, $result); + if (count($output) === 1 && $output[0] === 'CMD' && $result === 1) { + // Process doesn't exists anymore + $maxDuration = (new DateTimeImmutable())->diff($job->startedAt); + $maxDuration = + ($maxDuration->format('%a') * 24 * 60 * 60 * 1000) + + ($maxDuration->format('%h') * 60 * 60 * 1000) + + ($maxDuration->format('%m') * 60 * 1000) + + ($maxDuration->format('%s') * 1000) + + (int)($maxDuration->format('%f') / 1000); + $this->jobRuns->finished($job->runId, $maxDuration, 0, JobStatus::CRASHED); + } + } + } +} diff --git a/core/Command/Background/JobsHistory.php b/core/Command/Background/JobsHistory.php index 8837bcc10ce..566dfc550dd 100644 --- a/core/Command/Background/JobsHistory.php +++ b/core/Command/Background/JobsHistory.php @@ -12,7 +12,7 @@ namespace OC\Core\Command\Background; use OC\BackgroundJob\JobRuns; use OC\Core\Command\Base; use OCP\BackgroundJob\JobStatus; -use OCP\IConfig; +use OCP\IServerInfo; use OCP\Util; use Override; use Symfony\Component\Console\Input\InputInterface; @@ -23,7 +23,7 @@ use ValueError; final class JobsHistory extends Base { public function __construct( private readonly JobRuns $jobRuns, - private IConfig $config, + private readonly IServerInfo $serverInfo, ) { parent::__construct(); } @@ -75,7 +75,7 @@ final class JobsHistory extends Base { private function formatLine(iterable $jobs): \Generator { $jobsInfo = []; $now = time(); - $currentServerId = $this->config->getSystemValueInt('serverid', -1); + $currentServerId = $this->serverInfo->getServerId(); foreach ($jobs as $job) { $status = match ($job->status) { JobStatus::RUNNING => 'Running', diff --git a/core/Command/Background/RunningJobs.php b/core/Command/Background/RunningJobs.php index 8b63ace09c5..01c375eb306 100644 --- a/core/Command/Background/RunningJobs.php +++ b/core/Command/Background/RunningJobs.php @@ -11,7 +11,7 @@ namespace OC\Core\Command\Background; use OC\BackgroundJob\JobRuns; use OC\Core\Command\Base; -use OCP\IConfig; +use OCP\IServerInfo; use Override; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -20,7 +20,7 @@ use Symfony\Component\Console\Output\OutputInterface; final class RunningJobs extends Base { public function __construct( private readonly JobRuns $jobRuns, - private IConfig $config, + private readonly IServerInfo $serverInfo, ) { parent::__construct(); } @@ -60,7 +60,7 @@ final class RunningJobs extends Base { private function formatLine(iterable $jobs): \Generator { $now = time(); - $currentServerId = $this->config->getSystemValueInt('serverid', -1); + $currentServerId = $this->serverInfo->getServerId(); foreach ($jobs as $job) { yield [ 'Run ID' => $job->runId, diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index d799c16bc8a..7f18f4d7cc3 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -651,6 +651,7 @@ return array( 'OCP\\IRequest' => $baseDir . '/lib/public/IRequest.php', 'OCP\\IRequestId' => $baseDir . '/lib/public/IRequestId.php', 'OCP\\IServerContainer' => $baseDir . '/lib/public/IServerContainer.php', + 'OCP\\IServerInfo' => $baseDir . '/lib/public/IServerInfo.php', 'OCP\\ISession' => $baseDir . '/lib/public/ISession.php', 'OCP\\IStreamImage' => $baseDir . '/lib/public/IStreamImage.php', 'OCP\\ITagManager' => $baseDir . '/lib/public/ITagManager.php', @@ -2173,6 +2174,7 @@ return array( 'OC\\Security\\VerificationToken\\VerificationToken' => $baseDir . '/lib/private/Security/VerificationToken/VerificationToken.php', 'OC\\Server' => $baseDir . '/lib/private/Server.php', 'OC\\ServerContainer' => $baseDir . '/lib/private/ServerContainer.php', + 'OC\\ServerInfo' => $baseDir . '/lib/private/ServerInfo.php', 'OC\\ServerNotAvailableException' => $baseDir . '/lib/private/ServerNotAvailableException.php', 'OC\\ServiceUnavailableException' => $baseDir . '/lib/private/ServiceUnavailableException.php', 'OC\\Session\\CryptoSessionData' => $baseDir . '/lib/private/Session/CryptoSessionData.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 0ec2e588ab1..9f739dbbca2 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -692,6 +692,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\IRequest' => __DIR__ . '/../../..' . '/lib/public/IRequest.php', 'OCP\\IRequestId' => __DIR__ . '/../../..' . '/lib/public/IRequestId.php', 'OCP\\IServerContainer' => __DIR__ . '/../../..' . '/lib/public/IServerContainer.php', + 'OCP\\IServerInfo' => __DIR__ . '/../../..' . '/lib/public/IServerInfo.php', 'OCP\\ISession' => __DIR__ . '/../../..' . '/lib/public/ISession.php', 'OCP\\IStreamImage' => __DIR__ . '/../../..' . '/lib/public/IStreamImage.php', 'OCP\\ITagManager' => __DIR__ . '/../../..' . '/lib/public/ITagManager.php', @@ -2214,6 +2215,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Security\\VerificationToken\\VerificationToken' => __DIR__ . '/../../..' . '/lib/private/Security/VerificationToken/VerificationToken.php', 'OC\\Server' => __DIR__ . '/../../..' . '/lib/private/Server.php', 'OC\\ServerContainer' => __DIR__ . '/../../..' . '/lib/private/ServerContainer.php', + 'OC\\ServerInfo' => __DIR__ . '/../../..' . '/lib/private/ServerInfo.php', 'OC\\ServerNotAvailableException' => __DIR__ . '/../../..' . '/lib/private/ServerNotAvailableException.php', 'OC\\ServiceUnavailableException' => __DIR__ . '/../../..' . '/lib/private/ServiceUnavailableException.php', 'OC\\Session\\CryptoSessionData' => __DIR__ . '/../../..' . '/lib/private/Session/CryptoSessionData.php', diff --git a/lib/private/Repair/AddCleanupBackgroundJobsJob.php b/lib/private/Repair/AddCleanupBackgroundJobsJob.php new file mode 100644 index 00000000000..41478db9900 --- /dev/null +++ b/lib/private/Repair/AddCleanupBackgroundJobsJob.php @@ -0,0 +1,33 @@ +jobList->add(CleanupBackgroundJobsJob::class); + } +} diff --git a/lib/private/Server.php b/lib/private/Server.php index 1a4e523147f..3d79fd87ce6 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -227,6 +227,7 @@ use OCP\IPreview; use OCP\IRequest; use OCP\IRequestId; use OCP\IServerContainer; +use OCP\IServerInfo; use OCP\ISession; use OCP\ITagManager; use OCP\ITempManager; @@ -1146,6 +1147,9 @@ class Server extends ServerContainer implements IServerContainer { return $c->get(FileSequence::class); }, false); + $this->registerAlias(ISnowflakeDecoder::class, SnowflakeDecoder::class); + $this->registerAlias(IJobRuns::class, JobRuns::class); + $this->registerAlias(IServerInfo::class, ServerInfo::class); $this->connectDispatcher(); } diff --git a/lib/private/ServerInfo.php b/lib/private/ServerInfo.php new file mode 100644 index 00000000000..bf5dec468f2 --- /dev/null +++ b/lib/private/ServerInfo.php @@ -0,0 +1,48 @@ +config->getSystemValueInt('serverid', -1); + if ($serverid < 1) { + // Fallback: generates a server ID based on hostname + /** @var int<0,max> */ + $serverid = PHP_INT_SIZE === 4 + ? hexdec(hash('xxh32', $this->getHostname())) + // Makes sure it doesn't overflow 32 bits int + : hexdec(substr(hash('xxh32', $this->getHostname()), -3)); + } + + /** @var int<0,511> */ + return $serverid & 0x1FF; + } + + #[Override] + public function getHostname(): string { + $hostname = gethostname(); + if ($hostname === false) { + // Use a random hostname + return bin2hex(random_bytes(8)); + } + + return $hostname; + } +} diff --git a/lib/private/Snowflake/SnowflakeGenerator.php b/lib/private/Snowflake/SnowflakeGenerator.php index fa07f398583..bad8a2288da 100644 --- a/lib/private/Snowflake/SnowflakeGenerator.php +++ b/lib/private/Snowflake/SnowflakeGenerator.php @@ -10,7 +10,7 @@ declare(strict_types=1); namespace OC\Snowflake; use OCP\AppFramework\Utility\ITimeFactory; -use OCP\IConfig; +use OCP\IServerInfo; use OCP\Snowflake\ISnowflakeGenerator; use Override; @@ -24,8 +24,8 @@ use Override; final readonly class SnowflakeGenerator implements ISnowflakeGenerator { public function __construct( private ITimeFactory $timeFactory, - private IConfig $config, private ISequence $sequenceGenerator, + private IServerInfo $serverInfo, ) { } @@ -34,7 +34,7 @@ final readonly class SnowflakeGenerator implements ISnowflakeGenerator { // Relative time [$seconds, $milliseconds] = $this->getCurrentTime(); - $serverId = $this->getServerId(); // Already 9 bits + $serverId = $this->serverInfo->getServerId(); $isCli = (int)$this->isCli(); // 1 bit $sequenceId = $this->sequenceGenerator->nextId($seconds, $milliseconds, $serverId); // 12 bits if ($sequenceId > 0xFFF || $sequenceId === false) { @@ -102,24 +102,6 @@ final readonly class SnowflakeGenerator implements ISnowflakeGenerator { ]; } - /** - * Return configured serverid or generate one if not set - * - * @return int<0,511> - */ - private function getServerId(): int { - $serverid = $this->config->getSystemValueInt('serverid', -1); - if ($serverid < 1) { - // Fallback: generates a server ID based on hostname - // or random bytes if hostname isn't available - /** @var int<0,max> */ - $serverid = hexdec(hash('xxh32', gethostname() ?: random_bytes(8))); - } - - /** @var int<0,511> */ - return $serverid & 0x1FF; - } - private function isCli(): bool { return PHP_SAPI === 'cli'; } diff --git a/lib/public/IServerInfo.php b/lib/public/IServerInfo.php new file mode 100644 index 00000000000..82e3cb7e8e3 --- /dev/null +++ b/lib/public/IServerInfo.php @@ -0,0 +1,33 @@ + + * @since 34.0.1 + */ + public function getServerId(): int; + + /** + * Returns current server hostname + * + * @since 34.0.1 + */ + public function getHostname(): string; +} diff --git a/lib/public/Util.php b/lib/public/Util.php index 9fc9b9e7597..e03712d2a82 100644 --- a/lib/public/Util.php +++ b/lib/public/Util.php @@ -246,9 +246,7 @@ class Util { */ public static function linkToAbsolute($app, $file, $args = []) { $urlGenerator = Server::get(IURLGenerator::class); - return $urlGenerator->getAbsoluteURL( - $urlGenerator->linkTo($app, $file, $args) - ); + return $urlGenerator->getAbsoluteURL($urlGenerator->linkTo($app, $file, $args)); } /** @@ -483,7 +481,7 @@ class Util { * @since 4.5.0 */ public static function mb_array_change_key_case($input, $case = MB_CASE_LOWER, $encoding = 'UTF-8') { - $case = ($case !== MB_CASE_UPPER) ? MB_CASE_LOWER : MB_CASE_UPPER; + $case = $case !== MB_CASE_UPPER ? MB_CASE_LOWER : MB_CASE_UPPER; $ret = []; foreach ($input as $k => $v) { $ret[mb_convert_case($k, $case, $encoding)] = $v; @@ -518,7 +516,7 @@ class Util { $freeSpace = max($freeSpace, 0); return $freeSpace; } else { - return (INF > 0)? INF: PHP_INT_MAX; // work around https://bugs.php.net/bug.php?id=69188 + return INF > 0 ? INF : PHP_INT_MAX; // work around https://bugs.php.net/bug.php?id=69188 } } diff --git a/tests/lib/Snowflake/GeneratorTest.php b/tests/lib/Snowflake/GeneratorTest.php index c8fbc25da5f..6eac3a41078 100644 --- a/tests/lib/Snowflake/GeneratorTest.php +++ b/tests/lib/Snowflake/GeneratorTest.php @@ -14,7 +14,8 @@ use OC\Snowflake\ISequence; use OC\Snowflake\SnowflakeDecoder; use OC\Snowflake\SnowflakeGenerator; use OCP\AppFramework\Utility\ITimeFactory; -use OCP\IConfig; +use OCP\IServerInfo; +use OCP\Server; use OCP\Snowflake\ISnowflakeGenerator; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; @@ -25,8 +26,8 @@ use Test\TestCase; */ class GeneratorTest extends TestCase { private SnowflakeDecoder $decoder; - private IConfig&MockObject $config; private ISequence&MockObject $sequence; + private IServerInfo $serverInfo; #[\Override] public function setUp(): void { @@ -34,16 +35,15 @@ class GeneratorTest extends TestCase { $this->decoder = new SnowflakeDecoder(); - $this->config = $this->createMock(IConfig::class); - $this->config->method('getSystemValueInt')->with('serverid')->willReturn(42); - $this->sequence = $this->createMock(ISequence::class); $this->sequence->method('isAvailable')->willReturn(true); $this->sequence->method('nextId')->willReturn(421); + + $this->serverInfo = Server::get(IServerInfo::class); } public function testGenerator(): void { - $generator = new SnowflakeGenerator(new TimeFactory(), $this->config, $this->sequence); + $generator = new SnowflakeGenerator(new TimeFactory(), $this->sequence, $this->serverInfo); $snowflakeId = $generator->nextId(); $data = $this->decoder->decode($generator->nextId()); @@ -63,7 +63,7 @@ class GeneratorTest extends TestCase { $this->assertTrue($data->isCli()); // Check serverId - $this->assertEquals(42, $data->getServerId()); + $this->assertEquals($this->serverInfo->getServerId(), $data->getServerId()); } #[DataProvider('provideSnowflakeData')] @@ -72,12 +72,12 @@ class GeneratorTest extends TestCase { $timeFactory = $this->createMock(ITimeFactory::class); $timeFactory->method('now')->willReturn($dt); - $generator = new SnowflakeGenerator($timeFactory, $this->config, $this->sequence); + $generator = new SnowflakeGenerator($timeFactory, $this->sequence, $this->serverInfo); $data = $this->decoder->decode($generator->nextId()); $this->assertEquals($expectedSeconds, $data->getCreatedAt()->format('U') - ISnowflakeGenerator::TS_OFFSET); $this->assertEquals($expectedMilliseconds, (int)$data->getCreatedAt()->format('v')); - $this->assertEquals(42, $data->getServerId()); + $this->assertEquals($this->serverInfo->getServerId(), $data->getServerId()); } public static function provideSnowflakeData(): array {