refactor(lib): fold lib/Cron into the lib/BackgroundJob this app already had

appinfo/info.xml

@@ -25,8 +25,8 @@ Versioniq gives Nextcloud administrators the ability to roll back apps to previo
 		<job>OCA\Versioniq\BackgroundJob\AdvisoryRefreshJob</job>
 		<job>OCA\Versioniq\BackgroundJob\PatExpiryWarningJob</job>
 		<job>OCA\Versioniq\BackgroundJob\AutoUpdateJob</job>
-		<job>OCA\Versioniq\Cron\PruneAuditJob</job>
-		<job>OCA\Versioniq\Cron\PinReconcileJob</job>
+		<job>OCA\Versioniq\BackgroundJob\PruneAuditJob</job>
+		<job>OCA\Versioniq\BackgroundJob\PinReconcileJob</job>
 	</background-jobs>
 	<repair-steps>
 		<!-- THE app_versions -> versioniq RENAME IS A DATA MIGRATION.
@@ -87,6 +87,22 @@ Versioniq gives Nextcloud administrators the ability to roll back apps to previo
 				— an escaping exception in <install> aborts the install.
 			-->
 			<step>OCA\Versioniq\Repair\MigrateSchemaApplicationId</step>
+			<!--
+				The background jobs moved from OCA\Versioniq\Cron to
+				OCA\Versioniq\BackgroundJob (ADR-100 Decision 3), and the <job>
+				entries below are a REGISTRATION instruction rather than a
+				description of state: on upgrade Nextcloud ADDS a job it does
+				not have, but never removes one whose class disappeared —
+				it cannot tell a renamed class from one that is merely
+				unavailable this boot.
+
+				So without this step the instance holds BOTH rows, and the dead
+				one cannot be instantiated on any cron tick. Measured on a live
+				instance during this move: oc_jobs carried
+				OCA\OpenCatalogi\Cron\DirectorySync alongside its BackgroundJob
+				replacement. Idempotent — a fresh install removes nothing.
+			-->
+			<step>OCA\Versioniq\Repair\RemoveRetiredCronJobs</step>
 		</post-migration>
 		<install>
 			<step>OCA\Versioniq\Repair\MigrateAppConfigKeys</step>

lib/Cron/PinReconcileJob.php → lib/BackgroundJob/PinReconcileJob.php

@@ -10,7 +10,7 @@
  */
 
 
-namespace OCA\Versioniq\Cron;
+namespace OCA\Versioniq\BackgroundJob;
 
 use OCA\Versioniq\Service\Pin\PinDriftHandler;
 use OCA\Versioniq\Service\Pin\PinStore;

lib/Cron/PruneAuditJob.php → lib/BackgroundJob/PruneAuditJob.php

@@ -10,7 +10,7 @@
  */
 
 
-namespace OCA\Versioniq\Cron;
+namespace OCA\Versioniq\BackgroundJob;
 
 use OCA\Versioniq\AppInfo\Application;
 use OCA\Versioniq\Db\AuditEntryMapper;

lib/Listener/AppUpdatedListener.php

@@ -24,7 +24,7 @@
  * tool built on `IAppManager`. `AppUpdateEvent` is post-hoc and
  * non-cancellable (Nextcloud core has no pre-update veto hook — see
  * design.md), so this listener can only detect and report; the daily
- * {@see \OCA\Versioniq\Cron\PinReconcileJob} is the safety net for updates
+ * {@see \OCA\Versioniq\BackgroundJob\PinReconcileJob} is the safety net for updates
  * that bypass the event entirely (e.g. performed while this app was
  * disabled).
  *

lib/Repair/RemoveRetiredCronJobs.php

@@ -0,0 +1,151 @@
+<?php
+
+/**
+ * Repair step for removing the background-job registrations left behind by the
+ * move out of the retired `OCA\Versioniq\Cron` namespace.
+ *
+ * @category Repair
+ * @package  OCA\Versioniq\Repair
+ *
+ * @author    Conduction Development Team <info@conduction.nl>
+ * @copyright 2026 Conduction B.V.
+ * @license   EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12
+ *
+ * SPDX-FileCopyrightText: 2026 Conduction B.V. <info@conduction.nl>
+ * SPDX-License-Identifier: EUPL-1.2
+ */
+
+declare(strict_types=1);
+
+
+namespace OCA\Versioniq\Repair;
+
+use OCP\BackgroundJob\IJobList;
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+use Psr\Log\LoggerInterface;
+use Throwable;
+
+/**
+ * Removes the `oc_jobs` rows left behind when this app's background jobs moved
+ * out of the retired `OCA\Versioniq\Cron` namespace into
+ * `OCA\Versioniq\BackgroundJob` (ADR-100 Decision 3).
+ *
+ * WHY A REPAIR STEP IS REQUIRED, AND NOT OPTIONAL TIDYING.
+ *
+ * `appinfo/info.xml`'s `<job>` entries are a REGISTRATION instruction, not a
+ * description of state. On upgrade Nextcloud ADDS any job it does not already
+ * have; it never removes one whose class disappeared, because it has no way to
+ * tell a renamed class from a class that is merely unavailable this boot.
+ *
+ * So a class rename leaves the instance holding BOTH: the new row, added from
+ * the updated `info.xml`, and the old row still naming a class that no longer
+ * exists. Measured on a live instance during the fleet-wide move — before this
+ * step existed, `oc_jobs` carried
+ * `OCA\OpenCatalogi\Cron\DirectorySync` and `…\Cron\RetentionEvaluation`
+ * alongside their `BackgroundJob` replacements.
+ *
+ * The orphan is not inert. `\OC\BackgroundJob\JobList::buildJob()` cannot
+ * instantiate a class that does not exist, so every cron tick that reaches the
+ * row fails to build it, and the failure is logged rather than raised — the
+ * quiet kind of broken. It also breaks anything that resolves a job by NAME
+ * rather than by fully-qualified class: this app's own e2e helper looks the job
+ * up with `class LIKE '%PinReconcileJob%' LIMIT 1`, which with two matching
+ * rows and no ordering may return the dead one and silently execute nothing.
+ * That is how the orphan was found.
+ *
+ * Idempotent: `IJobList::remove()` on an absent class is a no-op, so a fresh
+ * install — which never had the old rows — passes through without change, and
+ * re-running the step costs one DELETE that matches nothing.
+ *
+ * @psalm-suppress UnusedClass Nextcloud instantiates repair steps from
+ *  the `<repair-steps>` block in appinfo/info.xml, which is XML — psalm
+ *  reads PHP and therefore sees no caller. The sibling steps escape this
+ *  only because unrelated docblocks happen to `{@see}` them, which is a
+ *  coincidence rather than a contract.
+ */
+class RemoveRetiredCronJobs implements IRepairStep {
+
+	/**
+	 * The classes retired by the move, named in full and deliberately as
+	 * literals.
+	 *
+	 * They are string constants rather than `SomeClass::class` because these
+	 * classes NO LONGER EXIST — a `::class` reference would be a compile-time
+	 * error, and that is precisely the point of the list.
+	 *
+	 * @var string[]
+	 */
+	private const RETIRED_JOB_CLASSES = [
+		'OCA\Versioniq\Cron\PinReconcileJob',
+		'OCA\Versioniq\Cron\PruneAuditJob',
+	];
+
+	/**
+	 * @param IJobList        $jobList The background job list.
+	 * @param LoggerInterface $logger  The logger.
+	 */
+	public function __construct(
+		private IJobList $jobList,
+		private LoggerInterface $logger,
+	) {
+	}//end __construct()
+
+	/**
+	 * The step's name, as shown by `occ upgrade`.
+	 *
+	 * @return string The name.
+	 *
+	 * @spec exclude See the class docblock — no capability spec covers the
+	 *  namespace move this step cleans up after.
+	 */
+	public function getName(): string {
+		return 'Remove background-job registrations for the retired Versioniq\Cron namespace';
+	}//end getName()
+
+	/**
+	 * Remove each retired job registration.
+	 *
+	 * Never raises. A repair step that aborts the upgrade over a job row would
+	 * trade a dormant orphan for an instance that will not start, which is the
+	 * worse failure — so a removal that goes wrong is reported and the step
+	 * continues with the next class.
+	 *
+	 * @param IOutput $output The upgrade output.
+	 *
+	 * @return void
+	 *
+	 * @spec exclude See the class docblock — no capability spec covers the
+	 *  namespace move this step cleans up after.
+	 *
+	 * @psalm-suppress ArgumentTypeCoercion `IJobList::remove()` is typed for
+	 *  callers REGISTERING a job, which hold the class. This step RETIRES one
+	 *  and the class is gone by construction, so a class-string cannot exist.
+	 *  Declared here rather than at the call site: psalm needs a docblock, and
+	 *  phpcs forbids one before a statement.
+	 */
+	public function run(IOutput $output): void {
+		foreach (self::RETIRED_JOB_CLASSES as $class) {
+			try {
+				// PHPStan: remove() is typed `class-string<IJob>|IJob`, and a
+				// plain string is exactly what this step must pass — the
+				// classes are GONE, which is the whole reason the row has to be
+				// removed. A class-string is unobtainable by construction, and
+				// remove() only ever uses the value as the `class` column to
+				// delete on, so the narrower type is about callers registering
+				// jobs, not callers retiring them.
+				// @phpstan-ignore argument.type
+				$this->jobList->remove($class);
+				$output->info('Removed retired background job registration: ' . $class);
+			} catch (Throwable $e) {
+				// Reported, not raised — see the docblock above.
+				$this->logger->warning(
+					'[RemoveRetiredCronJobs] Could not remove ' . $class . ': ' . $e->getMessage(),
+					['app' => 'versioniq', 'exception' => $e]
+				);
+				$output->warning('Could not remove ' . $class . ': ' . $e->getMessage());
+			}
+		}
+
+	}//end run()
+}//end class

lib/Service/Pin/PinDriftHandler.php

@@ -21,7 +21,7 @@
 
 /**
  * Shared drift-response path used by both {@see \OCA\Versioniq\Listener\AppUpdatedListener}
- * (immediate, event-driven) and {@see \OCA\Versioniq\Cron\PinReconcileJob}
+ * (immediate, event-driven) and {@see \OCA\Versioniq\BackgroundJob\PinReconcileJob}
  * (daily safety net): compares a pinned app's live installed version against
  * its pin, records drift on the pin (idempotently, via {@see PinStore::markDrift()}),
  * and — only on a genuinely new drift — notifies every admin-group member.

tests/bootstrap-unit-only.php

@@ -9,6 +9,21 @@
 
 require_once __DIR__ . '/../vendor/autoload.php';
 
+// Server-side classes an app never has in its own vendor tree, but which
+// nextcloud/ocp references at CLASS-DEFINITION time.
+//
+// This file existed and was never loaded here. It matters because
+// OCP\DB\QueryBuilder\IQueryBuilder derives its PARAM_* constants from
+// Doctrine\DBAL\ParameterType, and doctrine/dbal is a dependency of the
+// Nextcloud SERVER, not of an app — so any unit test that so much as
+// type-hints IQueryBuilder died with `Class "Doctrine\DBAL\ParameterType" not
+// found` before reaching an assertion.
+//
+// That stayed invisible because tests/unit/Repair was missing from the
+// testsuite list in phpunit-unit-only.xml: the tests never ran, so the
+// breakage never reported. Adding the directory surfaced seven errors at once.
+require_once __DIR__ . '/stubs/server-internals.php';
+
 // nextcloud/ocp ships interface stubs without composer autoload — register
 // them manually so PHPUnit can build mocks for OCP\* interfaces.
 spl_autoload_register(static function (string $class): void {

tests/phpunit-unit-only.xml

@@ -34,7 +34,12 @@
 		<directory suffix="Test.php">unit/BackgroundJob</directory>
 		<directory suffix="Test.php">unit/Notification</directory>
 		<directory suffix="Test.php">unit/Listener</directory>
-		<directory suffix="Test.php">unit/Cron</directory>
+		<!-- unit/Repair was missing from this list, so BOTH tests under it had
+		     never run: a testsuite that matches no directory is not an error,
+		     it just collects nothing and reports green. Found while adding
+		     RemoveRetiredCronJobsTest, whose first run reported
+		     "No tests executed!". -->
+		<directory suffix="Test.php">unit/Repair</directory>
 		<directory suffix="Test.php">unit/Sections</directory>
 		<directory suffix="Test.php">unit/Settings</directory>
 		<directory suffix="Test.php">unit/Command</directory>
@@ -47,7 +52,7 @@
 			<directory suffix=".php">../lib/BackgroundJob</directory>
 			<directory suffix=".php">../lib/Notification</directory>
 			<directory suffix=".php">../lib/Listener</directory>
-			<directory suffix=".php">../lib/Cron</directory>
+			<directory suffix=".php">../lib/Repair</directory>
 			<directory suffix=".php">../lib/Sections</directory>
 			<directory suffix=".php">../lib/Settings</directory>
 			<directory suffix=".php">../lib/Command</directory>

tests/stubs/server-internals.php

@@ -160,3 +160,44 @@ public function addUniqueIndex(array $columns, ?string $name = null): void {
 		}
 	}
 }
+
+namespace Doctrine\DBAL {
+	/**
+	 * The parameter-type constants `OCP\DB\QueryBuilder\IQueryBuilder` derives
+	 * its PARAM_* constants from.
+	 *
+	 * doctrine/dbal is a runtime dependency of the Nextcloud SERVER, not of an
+	 * app, so it is absent from this app's vendor tree — but `nextcloud/ocp`'s
+	 * IQueryBuilder references these constants at class-definition time. Any
+	 * unit test that so much as type-hints IQueryBuilder therefore dies with
+	 * `Class "Doctrine\DBAL\ParameterType" not found` before a single assertion
+	 * runs.
+	 *
+	 * That is exactly what had happened: tests/unit/Repair was missing from the
+	 * testsuite list in phpunit-unit-only.xml, so its tests never ran and this
+	 * gap stayed invisible. Adding the directory surfaced seven errors at once.
+	 *
+	 * The values mirror doctrine/dbal 3.x, where these are plain int constants.
+	 */
+	final class ParameterType {
+		public const NULL = 0;
+		public const INTEGER = 1;
+		public const STRING = 2;
+		public const LARGE_OBJECT = 3;
+		public const BOOLEAN = 5;
+		public const BINARY = 16;
+		public const ASCII = 17;
+	}
+
+	/**
+	 * The array-parameter counterpart, used by IQueryBuilder's PARAM_*_ARRAY.
+	 *
+	 * Same reason, same source: doctrine/dbal 3.x integer constants.
+	 */
+	final class ArrayParameterType {
+		public const INTEGER = 101;
+		public const STRING = 102;
+		public const ASCII = 117;
+		public const BINARY = 116;
+	}
+}

tests/unit/Cron/PinReconcileJobTest.php → tests/unit/BackgroundJob/PinReconcileJobTest.php

@@ -2,9 +2,9 @@
 
 declare(strict_types=1);
 
-namespace OCA\Versioniq\Tests\Unit\Cron;
+namespace OCA\Versioniq\Tests\Unit\BackgroundJob;
 
-use OCA\Versioniq\Cron\PinReconcileJob;
+use OCA\Versioniq\BackgroundJob\PinReconcileJob;
 use OCA\Versioniq\Service\Pin\Pin;
 use OCA\Versioniq\Service\Pin\PinDriftHandler;
 use OCA\Versioniq\Service\Pin\PinStore;

tests/unit/Cron/PruneAuditJobTest.php → tests/unit/BackgroundJob/PruneAuditJobTest.php

@@ -2,10 +2,10 @@
 
 declare(strict_types=1);
 
-namespace OCA\Versioniq\Tests\Unit\Cron;
+namespace OCA\Versioniq\Tests\Unit\BackgroundJob;
 
 use OCA\Versioniq\AppInfo\Application;
-use OCA\Versioniq\Cron\PruneAuditJob;
+use OCA\Versioniq\BackgroundJob\PruneAuditJob;
 use OCA\Versioniq\Db\AuditEntryMapper;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\IAppConfig;