fix(nc35): register OpenRegister PSR-4 prefix without OC_App

lib/AppInfo/OpenRegisterAutoloader.php

@@ -27,8 +27,8 @@
  *
  * WHY THIS EXISTS
  * ---------------
- * `Coordinator::registerApps()` walks the SORTED app list, calling
- * `OC_App::registerAutoloading()` and then `register()` for one app at a time.
+ * `Coordinator::registerApps()` walks the SORTED app list, registering each
+ * app's autoloading and then calling `register()` for one app at a time.
  * Under the app's OLD id, `nldesign` sorted before `openregister`, so
  * `Application::register()` ran while the `OCA\OpenRegister\` PSR-4 prefix did
  * not yet exist — on a completely healthy instance with OpenRegister installed
@@ -42,21 +42,35 @@
  * normally closed — but app sort order is not a contract to rely on and this
  * call is idempotent and cheap, so the prelude stays.
  *
+ * PUBLIC API ONLY (NEXTCLOUD 35)
+ * ------------------------------
+ * This used to call `\OC_App::registerAutoloading()`. That is private API and
+ * Nextcloud 35 REMOVED it (moved to the equally private
+ * `OC\App\AppManager::registerAutoloading()`). The resulting `\Error` landed in
+ * the catch below, `ensure()` returned false and federated config sharing was
+ * switched off in silence. For an app shipping its own `vendor/autoload.php` —
+ * which OpenRegister does — Nextcloud's registration reduces to a PSR-4 prefix
+ * over `lib/`, so that is exactly what is registered here, with
+ * `spl_autoload_register()` and the public `IAppManager`. Same approach as
+ * keepiq#712.
+ *
+ * OpenRegister's `vendor/autoload.php` is deliberately NOT required: that would
+ * pull its whole dependency tree into this process, where versions differing
+ * from ours would win first-come. Deliberately NOT `IAppManager::loadApp()`
+ * either: that boots OpenRegister before its own `register()` has run.
+ *
  * WHY IT IS A CLASS AND NOT FOUR INLINE LINES
  * -------------------------------------------
- * Three reasons, all practical. It is independently unit-testable, which four
- * lines inside `register()` are not — `Application` cannot be constructed in a
- * unit test without a server container. It confines the one unavoidable
- * static call to a single method instead of spreading it through bootstrap
- * code. And `ensure()` is deliberately an INSTANCE method so the call site in
- * `register()` is an ordinary method call: the PHPMD StaticAccess suppression
- * below then covers exactly one line in one class, and the rule keeps its
+ * It is independently unit-testable, which inline lines inside `register()`
+ * are not — `Application` cannot be constructed in a unit test without a
+ * server container. And `ensure()` is deliberately an INSTANCE method so the
+ * call site in `register()` is an ordinary method call: the PHPMD StaticAccess
+ * suppression below then covers exactly one class, and the rule keeps its
  * teeth in the bootstrap and everywhere else.
  *
- * @SuppressWarnings(PHPMD.StaticAccess) - `OC_App::registerAutoloading()` is
- * the ONLY way to pull another app's PSR-4 prefix into the running process and
- * Nextcloud exposes no instance API for it; ADR-040 prescribes this exact
- * call.
+ * @SuppressWarnings(PHPMD.StaticAccess) - `\OCP\Server::get()` is the public
+ * service locator, and this runs at the composition root where no container
+ * is available to inject from.
  *
  * @spec openspec/specs/federated-config-sharing/spec.md
  */
@@ -70,39 +84,110 @@ final class OpenRegisterAutoloader {
 	public const OPENREGISTER_APP_ID = 'openregister';
 
 	/**
-	 * Register OpenRegister's PSR-4 prefix with the running process.
+	 * The PSR-4 namespace prefix OpenRegister's `lib/` serves.
 	 *
-	 * `registerAutoloading()` touches only the autoloader and is idempotent,
-	 * so calling it is safe regardless of which apps registered before this
-	 * one. Every failure is swallowed on purpose: OpenRegister is a SOFT
-	 * dependency, and "absent or disabled" is a supported state in which the
-	 * caller's `class_exists()` guard then answers FALSE truthfully.
+	 * @var string
+	 */
+	private const OPENREGISTER_NAMESPACE = 'OCA\\OpenRegister\\';
+
+	/**
+	 * The registered loader, or null when none is on the SPL chain.
 	 *
-	 * The path is resolved into a local BEFORE the static call, deliberately.
-	 * Written as one expression, PHP resolves `OC_App` first and raises its
-	 * Error before `getAppPath()` is ever evaluated — so in any environment
-	 * without the legacy class (a unit test, for one) the prelude would never
-	 * even ask which path it wanted, and a test asserting that it does would
-	 * fail for a reason unrelated to what it meant to check.
+	 * Static on purpose: the SPL chain is process-wide, so one loader per
+	 * process however many instances call `ensure()`.
+	 *
+	 * @var (\Closure(string): void)|null
+	 */
+	private static ?\Closure $loader = null;
+
+	/**
+	 * Register OpenRegister's PSR-4 prefix with the running process.
+	 *
+	 * Idempotent: a second call while registered is a no-op. Every failure is
+	 * swallowed on purpose: OpenRegister is a SOFT dependency, and "absent or
+	 * disabled" is a supported state in which the caller's `class_exists()`
+	 * guard then answers FALSE truthfully.
 	 *
 	 * @param IAppManager|null $appManager Injected for tests; resolved from the
 	 *                                     server container when omitted.
 	 *
-	 * @return bool True when the prefix was registered, false when OpenRegister
+	 * @return bool True when the prefix is registered, false when OpenRegister
 	 *              is absent, disabled, or otherwise unreachable.
 	 *
 	 * @spec openspec/specs/federated-config-sharing/spec.md
 	 */
 	public function ensure(?IAppManager $appManager = null): bool {
 		try {
 			$manager = ($appManager ?? Server::get(IAppManager::class));
-			$path = $manager->getAppPath(self::OPENREGISTER_APP_ID);
-			\OC_App::registerAutoloading(self::OPENREGISTER_APP_ID, $path);
+
+			// Checked before the short-circuit: under a long-lived worker the
+			// static loader outlives the request, and an OpenRegister disabled
+			// since must not stay wired.
+			if ($manager->isEnabledForAnyone(self::OPENREGISTER_APP_ID) === false) {
+				self::unregister();
+				return false;
+			}
+
+			if (self::$loader !== null) {
+				return true;
+			}
+
+			$path = rtrim($manager->getAppPath(self::OPENREGISTER_APP_ID), '/');
+			if (is_dir($path . '/lib') === false) {
+				return false;
+			}
+
+			self::$loader = static function (string $class) use ($path): void {
+				$file = self::classFile(appPath: $path, class: $class);
+				if ($file !== null && is_file($file) === true) {
+					include_once $file;
+				}
+			};
+			spl_autoload_register(self::$loader);
+
 			return true;
 		} catch (Throwable) {
 			// OpenRegister absent, disabled, or the server container is not
-			// available (unit tests). The caller degrades as designed.
+			// available (unit tests). The caller degrades as designed. Never
+			// rethrow: an exception escaping here would abort the caller's
+			// entire register().
 			return false;
-		}
+		}//end try
 	}//end ensure()
+
+	/**
+	 * Take the loader off the SPL chain (tests, and OpenRegister disabled).
+	 *
+	 * @return void
+	 */
+	public static function unregister(): void {
+		if (self::$loader !== null) {
+			spl_autoload_unregister(self::$loader);
+			self::$loader = null;
+		}
+	}//end unregister()
+
+	/**
+	 * Map an `OCA\OpenRegister\…` class name to its file under `lib/`.
+	 *
+	 * Returns null for any class that is not OpenRegister's, so this loader
+	 * never answers for (and shadows) names another loader owns.
+	 *
+	 * @param string $appPath Absolute path to the openregister app, no trailing slash.
+	 * @param string $class   The fully qualified class name being resolved.
+	 *
+	 * @return string|null The candidate file, or null when not OpenRegister's.
+	 */
+	public static function classFile(string $appPath, string $class): ?string {
+		if (str_starts_with($class, self::OPENREGISTER_NAMESPACE) === false) {
+			return null;
+		}
+
+		$relative = substr($class, strlen(self::OPENREGISTER_NAMESPACE));
+		if ($relative === '') {
+			return null;
+		}
+
+		return $appPath . '/lib/' . str_replace('\\', '/', $relative) . '.php';
+	}//end classFile()
 }//end class

lib/Service/ComplianceReportService.php

@@ -25,6 +25,7 @@
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\IConfig;
 use OCP\IURLGenerator;
+use OCP\ServerVersion;
 
 /**
  * Computes the active-configuration WCAG contrast compliance evidence report.
@@ -177,6 +178,15 @@ class ComplianceReportService {
 	 */
 	private ITimeFactory $timeFactory;
 
+	/**
+	 * The running server's version (public since NC 31, below this app's floor of 32).
+	 *
+	 * Null only when constructed without it (isolated unit tests).
+	 *
+	 * @var ServerVersion|null
+	 */
+	private ?ServerVersion $serverVersion;
+
 	/**
 	 * Constructor.
 	 *
@@ -189,6 +199,9 @@ class ComplianceReportService {
 	 * @param IConfig $config The config service.
 	 * @param IURLGenerator $urlGenerator The URL generator.
 	 * @param ITimeFactory $timeFactory The injectable clock.
+	 * @param ServerVersion|null $serverVersion The running server's version.
+	 *
+	 * @SuppressWarnings(PHPMD.ExcessiveParameterList) - each collaborator backs one part of the report.
 	 */
 	public function __construct(
 		ContrastService $contrast,
@@ -200,6 +213,7 @@ public function __construct(
 		IConfig $config,
 		IURLGenerator $urlGenerator,
 		ITimeFactory $timeFactory,
+		?ServerVersion $serverVersion = null,
 	) {
 		$this->contrast = $contrast;
 		$this->cssParser = $cssParser;
@@ -210,6 +224,7 @@ public function __construct(
 		$this->config = $config;
 		$this->urlGenerator = $urlGenerator;
 		$this->timeFactory = $timeFactory;
+		$this->serverVersion = $serverVersion;
 	}//end __construct()
 
 	/**
@@ -725,23 +740,24 @@ private function buildMetadata(string $tokenSetId, array $tokenSetMeta, string $
 	/**
 	 * Resolve the Nextcloud server version string.
 	 *
-	 * Uses the deprecated array-returning \OCP\Util::getVersion() rather than
-	 * the newer \OCP\ServerVersion (NC 31.0.0+) because this app supports NC
-	 * back to 28 (appinfo/info.xml) and a type-hinted ServerVersion
-	 * constructor dependency would break DI autowiring on 28-30. Falls back
-	 * to "unknown" when no live Nextcloud server container is present (e.g. a
-	 * standalone PHPUnit run without a full NC bootstrap) — that never
-	 * happens for the real endpoint/occ command, only in isolated unit tests.
+	 * Uses the injected \OCP\ServerVersion (public since NC 31.0.0, below
+	 * this app's floor of 32 in appinfo/info.xml) instead of the
+	 * \OCP\Util::getVersion() deprecated since 31. Falls back to "unknown"
+	 * when it was not injected (e.g. a standalone PHPUnit run without a full
+	 * NC bootstrap) — that never happens for the real endpoint/occ command,
+	 * only in isolated unit tests.
 	 *
 	 * @return string The Nextcloud version (e.g. "34.0.0.5"), or "unknown".
 	 *
-	 * @SuppressWarnings(PHPMD.StaticAccess) - see rationale above.
-	 *
 	 * @spec openspec/specs/compliance-evidence/spec.md
 	 */
 	private function resolveNextcloudVersion(): string {
+		if ($this->serverVersion === null) {
+			return 'unknown';
+		}
+
 		try {
-			return implode('.', \OCP\Util::getVersion());
+			return implode('.', $this->serverVersion->getVersion());
 		} catch (\Throwable $e) {
 			return 'unknown';
 		}

lib/Service/PlaygroundStateService.php

@@ -25,6 +25,7 @@
 use OCA\Thematiq\Service\RuntimeFile\SetFileReader;
 use OCP\App\IAppManager;
 use OCP\IL10N;
+use OCP\ServerVersion;
 use Throwable;
 
 /**
@@ -92,6 +93,7 @@ class PlaygroundStateService {
 	 * @param IL10N $l10n The app's translations.
 	 * @param RuntimeFileStore|null $store Where uploaded sets and their dark files live.
 	 * @param SetFileReader $files Reads a set file from the release or the store.
+	 * @param ServerVersion|null $serverVersion The running server's version.
 	 */
 	public function __construct(
 		IAppManager $appManager,
@@ -101,6 +103,7 @@ public function __construct(
 		IL10N $l10n,
 		private ?RuntimeFileStore $store = null,
 		private SetFileReader $files = new SetFileReader(),
+		private ?ServerVersion $serverVersion = null,
 	) {
 		$this->appManager = $appManager;
 		$this->previewValues = $previewValues;
@@ -218,28 +221,25 @@ private function getDarkTokens(string $tokenSetId): array {
 	 * they take it — and this is the one it opens on, because the version you
 	 * are running is the one you are asking about first.
 	 *
-	 * Resolved through the deprecated array-returning `\OCP\Util::getVersion()`
-	 * for the same reason `ComplianceReportService` does: a type-hinted
-	 * `\OCP\ServerVersion` constructor dependency is a DI problem on older
-	 * servers, and this value is read once to pick a default.
+	 * Read from the injected `\OCP\ServerVersion` (public since NC 31, below
+	 * this app's floor of 32), replacing the `\OCP\Util::getVersion()` call
+	 * deprecated since 31.
 	 *
 	 * @return int The major version, or 0 when it cannot be read.
 	 *
-	 * @SuppressWarnings(PHPMD.StaticAccess) - see rationale above.
-	 *
 	 * @spec openspec/changes/component-playground/specs/component-playground/spec.md
 	 */
 	protected function getServerMajor(): int {
-		try {
-			$version = \OCP\Util::getVersion();
+		if ($this->serverVersion === null) {
+			// Only without a full Nextcloud bootstrap, which in practice means
+			// an isolated unit test. Zero is not a version the switch offers,
+			// so the instrument falls back to the newest one it knows.
+			return 0;
+		}
 
-			return (int)($version[0] ?? 0);
+		try {
+			return (int)($this->serverVersion->getVersion()[0] ?? 0);
 		} catch (Throwable $e) {
-			// Only reachable without a full Nextcloud bootstrap, which in
-			// practice means an isolated unit test. Zero is not a version the
-			// switch offers, so the instrument falls back to the newest one it
-			// knows — a working switch either way, and nothing worth a
-			// dependency on a logger to say.
 			return 0;
 		}
 	}//end getServerMajor()