From ede22c2c19e4b1901b5615b26b2cea5c7778e14e Mon Sep 17 00:00:00 2001 From: Ralf Lang Date: Mon, 6 Jul 2026 17:58:59 +0200 Subject: [PATCH 1/3] feat(config): compile registry to a single artifact with vhost slots and load it via a short-circuit --- src/Config/RegistryConfigCompiler.php | 268 +++++++++++++++ src/Config/RegistryConfigLoader.php | 90 ++++- src/Config/RegistryConfigWriter.php | 174 ++++++++++ .../Config/RegistryConfigCompilerTest.php | 313 ++++++++++++++++++ test/Unit/Config/RegistryConfigLoaderTest.php | 196 +++++++++++ test/Unit/Config/RegistryConfigWriterTest.php | 217 ++++++++++++ 6 files changed, 1255 insertions(+), 3 deletions(-) create mode 100644 src/Config/RegistryConfigCompiler.php create mode 100644 src/Config/RegistryConfigWriter.php create mode 100644 test/Unit/Config/RegistryConfigCompilerTest.php create mode 100644 test/Unit/Config/RegistryConfigWriterTest.php diff --git a/src/Config/RegistryConfigCompiler.php b/src/Config/RegistryConfigCompiler.php new file mode 100644 index 00000000..366e566a --- /dev/null +++ b/src/Config/RegistryConfigCompiler.php @@ -0,0 +1,268 @@ + [ ...merged default registry, per-app array ], + * 'foo.example.com' => [ ...delta relative to default ], + * 'bar.example.com' => [ ...delta relative to default ], + * ] + * + * The 'default' slot is the fully-merged result of the standard load + * chain minus vhost. Vendor defaults, deployment base, registry.d + * snippets and registry.local.php get merged with + * array_replace_recursive at each step. This matches what + * RegistryConfigLoader::load() returns to a caller with no vhost. + * + * Each vhost slot holds only the delta: the raw contents of the + * corresponding registry-{hostname}.php file. Storing the delta rather + * than a fully-merged view keeps the compiled artifact small. For a + * dozen vhosts that share the same base registry, the compiled file + * grows by roughly one delta per vhost rather than N full registries. + * RegistryConfigLoader recombines default + vhost-slot at read time. + * + * ## Vhost discovery + * + * If no explicit vhost list is supplied, the compiler discovers vhosts + * by globbing `{configBase}/horde/registry-*.php`. Extracts the vhost + * name from the filename. This is the common case. A deployment's + * vhost set is defined by which vhost files exist. + * + * Callers with an external source of truth for the vhost list (a + * deployment manifest, a database, a config array) can pass `$vhosts` + * explicitly. The compiler then only reads vhost files for the listed + * names, and silently skips names whose files don't exist. + * + * ## Merge semantics + * + * Uses `array_replace_recursive`, matching RegistryConfigLoader. Legacy + * Horde_Registry used bare `include` (which is shallow-replacement at + * the top level of $this->applications). Modern uses recursive. The + * compiler follows modern. + * + * ## Not written to disk by the compiler + * + * `compile()` returns an in-memory array. Callers that want to + * persist it use {@see RegistryConfigWriter}, which handles atomic + * write and opcache invalidation as separate concerns. The + * compiler stays a pure-compute class with no filesystem side + * effects. Deterministic, testable and safe to invoke from any + * context (CLI, HTTP request, unit test) without worrying about + * disk state. + * + * The split is intentional: serialization format, atomic write, + * permission handling and opcache invalidation are operational + * concerns that vary by caller (installer vs. hordectl vs. CI + * pipeline). Baking them into the compiler would force every + * caller through one policy. + * + * @category Horde + * @copyright 2026 The Horde Project + * @license http://www.horde.org/licenses/lgpl21 LGPL 2.1 + * @package Core + */ +class RegistryConfigCompiler +{ + /** + * @param string $configBase Deployment config root (HORDE_CONFIG_BASE). + * The compiler reads + * `{configBase}/horde/registry.php`, + * `registry.d/*.php`, + * `registry.local.php` and + * `registry-{vhost}.php` files. + * @param string $vendorBase Path to the vendor/horde/horde + * directory. The compiler reads + * `{vendorBase}/config/registry.php` as + * the factory defaults. + * @param string[]|null $vhosts Explicit vhost list, or null for + * auto-discovery via filesystem glob. + * Pass an empty array for a + * default-only compile. + */ + public function __construct( + private readonly string $configBase, + private readonly string $vendorBase, + private readonly ?array $vhosts = null, + ) {} + + /** + * Compile the full registry graph. + * + * @return array{ + * default: array>, + * } & array>> + * A map with the 'default' key holding the merged default + * registry, plus one key per compiled vhost holding that + * vhost's raw delta. Vhosts whose files don't exist are + * silently omitted from the result. + */ + public function compile(): array + { + $result = ['default' => $this->compileDefault()]; + + $vhosts = $this->vhosts ?? $this->discoverVhosts(); + foreach ($vhosts as $vhost) { + $delta = $this->loadVhostDelta($vhost); + if ($delta !== null) { + $result[$vhost] = $delta; + } + } + + return $result; + } + + /** + * Merge the four non-vhost layers into a single applications array. + * + * Order (each layer overrides the previous via + * array_replace_recursive): + * 1. vendor defaults: {vendorBase}/config/registry.php + * 2. deployment base: {configBase}/horde/registry.php + * 3. snippets: {configBase}/horde/registry.d/*.php (sorted) + * 4. local override: {configBase}/horde/registry.local.php + * + * @return array> + */ + private function compileDefault(): array + { + $confDir = $this->configBase . '/horde/'; + $vendorDir = $this->vendorBase . '/config/'; + + $files = []; + + // Layer 1: vendor defaults. + $files[] = $vendorDir . 'registry.php'; + + // Layer 2: deployment base. + $files[] = $confDir . 'registry.php'; + + // Layer 3: snippets. Sorted so numeric prefixes (01-, 02-, ...) + // load in the intended order across filesystems. + $snippetsDir = $confDir . 'registry.d'; + if (is_dir($snippetsDir)) { + $snippets = glob($snippetsDir . '/*.php'); + if ($snippets !== false) { + sort($snippets); + $files = array_merge($files, $snippets); + } + } + + // Layer 4: local override. + $files[] = $confDir . 'registry.local.php'; + + $applications = []; + foreach ($files as $file) { + if (file_exists($file)) { + $applications = array_replace_recursive( + $applications, + $this->includeFile($file) + ); + } + } + + return $applications; + } + + /** + * Read a single vhost file and return its raw applications delta. + * + * Returns null (not an empty array) when the file doesn't exist, + * so callers can distinguish "no vhost file" from "vhost file with + * no applications". The latter should still be recorded as a + * present-but-empty override. + * + * @return array>|null + */ + private function loadVhostDelta(string $vhost): ?array + { + $vhostFile = $this->configBase . '/horde/registry-' . $vhost . '.php'; + if (!file_exists($vhostFile)) { + return null; + } + return $this->includeFile($vhostFile); + } + + /** + * Discover vhosts by scanning for registry-*.php files. + * + * Returns hostnames extracted from filenames, unsorted. Sorting is + * not meaningful for vhost slots. They are keyed by hostname in + * the output, and no vhost merges onto another. + * + * @return string[] + */ + private function discoverVhosts(): array + { + $pattern = $this->configBase . '/horde/registry-*.php'; + $files = glob($pattern); + if ($files === false) { + return []; + } + + $vhosts = []; + foreach ($files as $file) { + $basename = basename($file); + // strip 'registry-' prefix (9 chars) and '.php' suffix. + $vhost = substr($basename, 9, -4); + // Guard against pathological empty match. Theoretically + // impossible from the glob, but a defensive skip is cheap. + if ($vhost !== '') { + $vhosts[] = $vhost; + } + } + + return $vhosts; + } + + /** + * Include a registry file with $this->applications context. + * + * Registry files assign to $this->applications['app'] = [...]. To + * make that work without polluting the compiler's own state, bind + * the include to a disposable object whose only public property is + * $applications, then read that property back after include + * returns. + * + * Duplicated from RegistryConfigLoader::includeFile intentionally. + * Both callers do the same thing, extraction would create a new + * public helper class for 22 lines of shared logic, and the + * duplication risk is low because the file-format contract is + * stable. + * + * @return array> + */ + private function includeFile(string $file): array + { + $mock = new class { + /** @var array> */ + public array $applications = []; + }; + + $executor = function () use ($file) { + include $file; + }; + $executor->bindTo($mock, $mock)(); + + return $mock->applications; + } +} diff --git a/src/Config/RegistryConfigLoader.php b/src/Config/RegistryConfigLoader.php index 048d5cac..91afa214 100644 --- a/src/Config/RegistryConfigLoader.php +++ b/src/Config/RegistryConfigLoader.php @@ -35,6 +35,21 @@ * * Registry is global - always loads from 'horde' app only. * + * ## Compiled-registry short-circuit + * + * If a compiled-registry file exists at $compiledRegistryPath, the + * loader loads it via `require` and returns a merged view without + * scanning any of the layer files above. The compiled file is + * produced by {@see RegistryConfigCompiler} and holds a + * two-level structure: a 'default' key with the merged default + * registry, and one key per compiled vhost with that vhost's raw + * delta. `load()` merges default + vhost-slot at read time. + * + * Callers who want to force layer reloading (e.g. after editing a + * registry.d snippet) should delete the compiled file. `clearCache()` + * only invalidates the in-memory RegistryState cache, not the on-disk + * compiled file. + * * @category Horde * @copyright 2026 The Horde Project * @license http://www.horde.org/licenses/lgpl21 LGPL 2.1 @@ -44,13 +59,21 @@ class RegistryConfigLoader { private ?RegistryState $cache = null; + private readonly string $compiledRegistryPath; public function __construct( private string $configBase, // HORDE_CONFIG_BASE constant private string $vendorBase, // Path to vendor/horde/horde directory - private Vhost|string $vhost = 'localhost' + private Vhost|string $vhost = 'localhost', + ?string $compiledRegistryPath = null, ) { $this->vhost = Vhost::from($this->vhost); + // The compiled registry lives outside the admin-authored + // var/config/horde/ tree. It's a derived cache, not a config + // source. Default: sibling var/cache/compiled/registry.php. + // Callers with non-standard layouts pass this explicitly. + $this->compiledRegistryPath = $compiledRegistryPath + ?? dirname($this->configBase) . '/cache/compiled/registry.php'; } /** @@ -64,7 +87,22 @@ public function __construct( public function load(string $file = 'registry.php'): RegistryState { if ($this->cache === null) { - $applications = $this->loadFiles($file); + // Fast path: if a compiled registry exists, use it and + // skip every layer file. The compiler wrote the same + // merge result the layer scan would produce, so callers + // observe identical output either way. + // + // Only the default filename is served from the compiled + // artifact. Callers passing an explicit non-default $file + // want a different registry (e.g. a test fixture) and + // must fall through to the layer scan. + if ($file === 'registry.php' + && file_exists($this->compiledRegistryPath) + ) { + $applications = $this->loadFromCompiled(); + } else { + $applications = $this->loadFiles($file); + } $this->cache = new RegistryState($applications); } @@ -202,7 +240,53 @@ private function loadFiles(string $file): array } /** - * Include PHP file in isolated scope with $this context + * Load the merged registry from a pre-compiled artifact. + * + * The compiled file is a plain PHP ` [ merged default registry ], + * '' => [ vhost-specific delta ], + * ] + * + * If the current vhost is present in the compiled artifact, its + * delta merges onto the default via array_replace_recursive (same + * semantics as the layer scan). Vhosts not present in the compiled + * artifact fall back to the default silently. This matches the + * layer scan's behavior when no registry-{vhost}.php file exists. + * + * @return array> + */ + private function loadFromCompiled(): array + { + $compiled = require $this->compiledRegistryPath; + + // Defensive: if the compiled file is somehow malformed (not an + // array, or missing 'default'), fall back to layers rather + // than propagate an unusable state. This preserves the "you + // can delete the compiled file to force a fresh scan" story + // even when the file is corrupt. + if (!is_array($compiled) || !isset($compiled['default'])) { + return $this->loadFiles('registry.php'); + } + + $applications = $compiled['default']; + + if ($this->vhost->isAvailable()) { + $hostname = $this->vhost->getHostname(); + if ($hostname !== null && isset($compiled[$hostname])) { + $applications = array_replace_recursive( + $applications, + $compiled[$hostname] + ); + } + } + + return $applications; + } + + /** + * Include PHP file in isolated scope with $this context. * * Registry files expect $this->applications to be available * diff --git a/src/Config/RegistryConfigWriter.php b/src/Config/RegistryConfigWriter.php new file mode 100644 index 00000000..193ceb24 --- /dev/null +++ b/src/Config/RegistryConfigWriter.php @@ -0,0 +1,174 @@ +>> $compiled + * The two-level structure produced by + * {@see RegistryConfigCompiler::compile()}. + * @param string $path Destination path (usually + * `{HORDE_VAR}/cache/compiled/registry.php`). + * @throws RuntimeException If the destination directory cannot + * be created, if serialization fails, + * or if the atomic install fails. + */ + public function write(array $compiled, string $path): void + { + $dir = dirname($path); + $this->ensureDirectory($dir); + + $code = "invalidateOpcache($path); + } + + /** + * Create the destination directory if it doesn't exist. + * + * The compiled-registry path canonically sits under + * `var/cache/compiled/`, which may not exist on a fresh install + * that hasn't yet run the compiler. Create it eagerly rather + * than error out. The caller almost never wants "directory + * missing" to be a hard failure mode. + */ + private function ensureDirectory(string $dir): void + { + if (is_dir($dir)) { + return; + } + // The race between the is_dir check and the mkdir call is + // benign: mkdir returns false with the EEXIST errno, and the + // second is_dir catches it. Only genuine failure to create + // the tree is reported. + if (!mkdir($dir, 0o755, true) && !is_dir($dir)) { + throw new RuntimeException( + "Cannot create compiled-registry directory: {$dir}" + ); + } + } + + /** + * Invalidate the opcache entry for the newly-written file. + * + * No-op when opcache isn't loaded (CLI without opcache, testing, + * older PHP builds). Errors from opcache itself are not fatal: + * the file is already on disk and will be re-cached on next + * request in the worst case. + */ + private function invalidateOpcache(string $path): void + { + if (function_exists('opcache_invalidate')) { + @opcache_invalidate($path, true); + } + } +} diff --git a/test/Unit/Config/RegistryConfigCompilerTest.php b/test/Unit/Config/RegistryConfigCompilerTest.php new file mode 100644 index 00000000..2fcdef72 --- /dev/null +++ b/test/Unit/Config/RegistryConfigCompilerTest.php @@ -0,0 +1,313 @@ +tempDir = sys_get_temp_dir() . '/horde-registry-compiler-test-' . uniqid(); + $this->vendorDir = $this->tempDir . '/vendor/horde/horde'; + $this->configDir = $this->tempDir . '/config'; + + mkdir($this->vendorDir . '/config', 0o755, true); + mkdir($this->configDir . '/horde', 0o755, true); + } + + protected function tearDown(): void + { + $this->removeDirectory($this->tempDir); + } + + private function removeDirectory(string $dir): void + { + if (!is_dir($dir)) { + return; + } + $files = array_diff(scandir($dir), ['.', '..']); + foreach ($files as $file) { + $path = $dir . '/' . $file; + is_dir($path) ? $this->removeDirectory($path) : unlink($path); + } + rmdir($dir); + } + + /** Convenience: write a registry file with $this->applications syntax. */ + private function writeRegistryFile(string $path, array $applications): void + { + $lines = [" $spec) { + $lines[] = '$this->applications[' . var_export($name, true) . '] = ' + . var_export($spec, true) . ';'; + } + file_put_contents($path, implode("\n", $lines) . "\n"); + } + + // =================================================================== + // Default layer merge + // =================================================================== + + public function testCompileWithOnlyVendorProducesDefault(): void + { + $this->writeRegistryFile( + $this->vendorDir . '/config/registry.php', + ['horde' => ['name' => 'Horde', 'status' => 'active']] + ); + + $compiler = new RegistryConfigCompiler($this->configDir, $this->vendorDir); + $result = $compiler->compile(); + + $this->assertArrayHasKey('default', $result); + $this->assertSame('Horde', $result['default']['horde']['name']); + $this->assertSame('active', $result['default']['horde']['status']); + } + + public function testCompileMergesAllFourDefaultLayers(): void + { + // Layer 1: vendor - sets everything on imp + $this->writeRegistryFile( + $this->vendorDir . '/config/registry.php', + ['imp' => [ + 'name' => 'IMP', + 'status' => 'inactive', + 'params' => ['timeout' => 30, 'ssl' => false], + ]] + ); + + // Layer 2: deployment base - overrides status + $this->writeRegistryFile( + $this->configDir . '/horde/registry.php', + ['imp' => ['status' => 'active']] + ); + + // Layer 3: snippet - overrides timeout + mkdir($this->configDir . '/horde/registry.d', 0o755, true); + $this->writeRegistryFile( + $this->configDir . '/horde/registry.d/01-timeouts.php', + ['imp' => ['params' => ['timeout' => 60]]] + ); + + // Layer 4: local - overrides ssl + $this->writeRegistryFile( + $this->configDir . '/horde/registry.local.php', + ['imp' => ['params' => ['ssl' => true]]] + ); + + $compiler = new RegistryConfigCompiler($this->configDir, $this->vendorDir); + $result = $compiler->compile(); + + // Layer precedence: local > snippet > base > vendor + $imp = $result['default']['imp']; + $this->assertSame('IMP', $imp['name'], 'name preserved from vendor'); + $this->assertSame('active', $imp['status'], 'status overridden by base'); + $this->assertSame(60, $imp['params']['timeout'], 'timeout overridden by snippet'); + $this->assertTrue($imp['params']['ssl'], 'ssl overridden by local'); + } + + public function testCompileLoadsSnippetsInSortedOrder(): void + { + // 01 sets the value; 02 overrides. If load order were reversed + // (or non-deterministic across filesystems), the test would flap. + $this->writeRegistryFile( + $this->vendorDir . '/config/registry.php', + ['imp' => ['name' => 'IMP']] + ); + + mkdir($this->configDir . '/horde/registry.d', 0o755, true); + $this->writeRegistryFile( + $this->configDir . '/horde/registry.d/01-set.php', + ['imp' => ['status' => 'first']] + ); + $this->writeRegistryFile( + $this->configDir . '/horde/registry.d/02-override.php', + ['imp' => ['status' => 'second']] + ); + + $compiler = new RegistryConfigCompiler($this->configDir, $this->vendorDir); + $result = $compiler->compile(); + + $this->assertSame('second', $result['default']['imp']['status']); + } + + public function testCompileWithNoFilesProducesEmptyDefault(): void + { + $compiler = new RegistryConfigCompiler($this->configDir, $this->vendorDir); + $result = $compiler->compile(); + + $this->assertSame(['default' => []], $result); + } + + // =================================================================== + // Vhost discovery and slots + // =================================================================== + + public function testCompileAutoDiscoversVhostFiles(): void + { + $this->writeRegistryFile( + $this->vendorDir . '/config/registry.php', + ['imp' => ['name' => 'IMP', 'webroot' => '/imp']] + ); + + $this->writeRegistryFile( + $this->configDir . '/horde/registry-foo.example.com.php', + ['imp' => ['webroot' => '/mail-foo']] + ); + + $this->writeRegistryFile( + $this->configDir . '/horde/registry-bar.example.com.php', + ['imp' => ['webroot' => '/mail-bar']] + ); + + $compiler = new RegistryConfigCompiler($this->configDir, $this->vendorDir); + $result = $compiler->compile(); + + $this->assertArrayHasKey('foo.example.com', $result); + $this->assertArrayHasKey('bar.example.com', $result); + $this->assertSame('/mail-foo', $result['foo.example.com']['imp']['webroot']); + $this->assertSame('/mail-bar', $result['bar.example.com']['imp']['webroot']); + } + + public function testCompileStoresRawVhostDeltaNotMergedRegistry(): void + { + // Vendor establishes lots of baseline properties. + $this->writeRegistryFile( + $this->vendorDir . '/config/registry.php', + ['imp' => [ + 'name' => 'IMP', + 'status' => 'active', + 'webroot' => '/imp', + 'fileroot' => '/var/www/imp', + ]] + ); + + // Vhost changes only webroot. + $this->writeRegistryFile( + $this->configDir . '/horde/registry-example.com.php', + ['imp' => ['webroot' => '/mail']] + ); + + $compiler = new RegistryConfigCompiler($this->configDir, $this->vendorDir); + $result = $compiler->compile(); + + // The vhost slot holds only the delta, not the full merged shape. + // RegistryConfigLoader recombines default + vhost at read time. + $this->assertSame( + ['imp' => ['webroot' => '/mail']], + $result['example.com'] + ); + } + + public function testCompileWithExplicitVhostList(): void + { + $this->writeRegistryFile( + $this->vendorDir . '/config/registry.php', + ['imp' => ['name' => 'IMP']] + ); + + // Two vhost files on disk. + $this->writeRegistryFile( + $this->configDir . '/horde/registry-included.example.com.php', + ['imp' => ['webroot' => '/mail-included']] + ); + $this->writeRegistryFile( + $this->configDir . '/horde/registry-skipped.example.com.php', + ['imp' => ['webroot' => '/mail-skipped']] + ); + + // Caller asks for only one of them plus one that doesn't exist. + $compiler = new RegistryConfigCompiler( + $this->configDir, + $this->vendorDir, + vhosts: ['included.example.com', 'never-existed.example.com'] + ); + $result = $compiler->compile(); + + $this->assertArrayHasKey('included.example.com', $result); + $this->assertArrayNotHasKey( + 'skipped.example.com', + $result, + 'explicit list should suppress auto-discovery' + ); + $this->assertArrayNotHasKey( + 'never-existed.example.com', + $result, + 'listed but missing vhost file should be silently skipped' + ); + } + + public function testCompileWithEmptyExplicitListYieldsDefaultOnly(): void + { + $this->writeRegistryFile( + $this->vendorDir . '/config/registry.php', + ['horde' => ['name' => 'Horde']] + ); + // Vhost file present, but explicit empty list overrides discovery. + $this->writeRegistryFile( + $this->configDir . '/horde/registry-ignored.example.com.php', + ['horde' => ['webroot' => '/nope']] + ); + + $compiler = new RegistryConfigCompiler( + $this->configDir, + $this->vendorDir, + vhosts: [] + ); + $result = $compiler->compile(); + + $this->assertSame(['default'], array_keys($result)); + } + + public function testCompileHandlesVhostWithHyphensInHostname(): void + { + // registry-.php filename parsing must survive vhost names + // that themselves contain hyphens (registry-my-app.example.com.php). + $this->writeRegistryFile( + $this->vendorDir . '/config/registry.php', + ['horde' => ['name' => 'Horde']] + ); + $this->writeRegistryFile( + $this->configDir . '/horde/registry-my-app.example.com.php', + ['horde' => ['webroot' => '/tenant']] + ); + + $compiler = new RegistryConfigCompiler($this->configDir, $this->vendorDir); + $result = $compiler->compile(); + + $this->assertArrayHasKey('my-app.example.com', $result); + } +} diff --git a/test/Unit/Config/RegistryConfigLoaderTest.php b/test/Unit/Config/RegistryConfigLoaderTest.php index 383a3680..d13d2496 100644 --- a/test/Unit/Config/RegistryConfigLoaderTest.php +++ b/test/Unit/Config/RegistryConfigLoaderTest.php @@ -523,4 +523,200 @@ public function testLoadLayerForIntrospection(): void $this->assertArrayHasKey('imp', $localLayer); $this->assertArrayNotHasKey('horde', $localLayer); } + + // =================================================================== + // F. Compiled-registry short-circuit + // =================================================================== + + /** + * Write a compiled-registry PHP file at the given path. + * + * Mirrors the format RegistryConfigWriter emits: a plain + * `configDir . '/horde/registry.php', + <<<'PHP' + applications['horde'] = ['name' => 'From base file']; + PHP + ); + + $compiledPath = $this->tempDir . '/cache/compiled/registry.php'; + $this->writeCompiledFile($compiledPath, [ + 'default' => [ + 'horde' => ['name' => 'From compiled file'], + ], + ]); + + $loader = new RegistryConfigLoader( + $this->configDir, + $this->vendorDir, + compiledRegistryPath: $compiledPath, + ); + + $state = $loader->load(); + $apps = $state->toArray(); + + // Compiled wins: the base file was not scanned. + $this->assertSame('From compiled file', $apps['horde']['name']); + } + + public function testCompiledFileMergesVhostDeltaOntoDefault(): void + { + $compiledPath = $this->tempDir . '/cache/compiled/registry.php'; + $this->writeCompiledFile($compiledPath, [ + 'default' => [ + 'imp' => [ + 'name' => 'IMP', + 'webroot' => '/imp', + 'fileroot' => '/var/www/imp', + 'jsuri' => '/imp/js', + 'themesuri' => '/imp/themes', + 'status' => 'active', + ], + ], + 'example.com' => [ + 'imp' => ['webroot' => '/mail'], + ], + ]); + + $loader = new RegistryConfigLoader( + $this->configDir, + $this->vendorDir, + 'example.com', + compiledRegistryPath: $compiledPath, + ); + + $apps = $loader->load()->toArray(); + + // Vhost delta overrides webroot. Default preserves the rest. + $this->assertSame('/mail', $apps['imp']['webroot']); + $this->assertSame('IMP', $apps['imp']['name']); + $this->assertSame('active', $apps['imp']['status']); + } + + public function testCompiledFileFallsBackToDefaultForUnknownVhost(): void + { + $compiledPath = $this->tempDir . '/cache/compiled/registry.php'; + $this->writeCompiledFile($compiledPath, [ + 'default' => [ + 'imp' => ['webroot' => '/imp'], + ], + 'known.example.com' => [ + 'imp' => ['webroot' => '/mail-known'], + ], + ]); + + // Loader configured for a vhost that has no slot in the + // compiled artifact. Should silently fall back to default. + $loader = new RegistryConfigLoader( + $this->configDir, + $this->vendorDir, + 'unknown.example.com', + compiledRegistryPath: $compiledPath, + ); + + $apps = $loader->load()->toArray(); + $this->assertSame('/imp', $apps['imp']['webroot']); + } + + public function testCompiledFileMissingFallsBackToLayerScan(): void + { + // Layer files present, compiled-registry path pointed at a + // path that doesn't exist. Loader must not raise and must + // return the same result the layer scan would produce. + file_put_contents( + $this->configDir . '/horde/registry.php', + <<<'PHP' + applications['horde'] = ['name' => 'Layer scan']; + PHP + ); + + $loader = new RegistryConfigLoader( + $this->configDir, + $this->vendorDir, + compiledRegistryPath: $this->tempDir . '/does-not-exist.php', + ); + + $apps = $loader->load()->toArray(); + $this->assertSame('Layer scan', $apps['horde']['name']); + } + + public function testMalformedCompiledFileFallsBackToLayerScan(): void + { + // A corrupt or wrong-shape compiled file (not an array, or + // missing 'default') should not brick the loader. The layer + // scan is the safe fallback. + file_put_contents( + $this->configDir . '/horde/registry.php', + <<<'PHP' + applications['horde'] = ['name' => 'Layer scan']; + PHP + ); + + $compiledPath = $this->tempDir . '/cache/compiled/registry.php'; + if (!is_dir(dirname($compiledPath))) { + mkdir(dirname($compiledPath), 0o755, true); + } + file_put_contents( + $compiledPath, + "configDir, + $this->vendorDir, + compiledRegistryPath: $compiledPath, + ); + + $apps = $loader->load()->toArray(); + $this->assertSame('Layer scan', $apps['horde']['name']); + } + + public function testDefaultCompiledPathIsSiblingCacheDir(): void + { + // When no explicit compiledRegistryPath is passed, the loader + // derives dirname($configBase) . '/cache/compiled/registry.php'. + // Simulate a bundle-shaped var/{config,cache}/ layout and + // check the loader picks up the file at the expected place. + $bundleVar = $this->tempDir . '/var'; + mkdir($bundleVar . '/config/horde', 0o755, true); + mkdir($bundleVar . '/cache/compiled', 0o755, true); + + $this->writeCompiledFile( + $bundleVar . '/cache/compiled/registry.php', + ['default' => ['horde' => ['name' => 'From default path']]] + ); + + // Layers under var/config/ would produce a different result + // Layers under var/config/ would produce a different result + // if scanned. Do not seed them. Silence is the assertion + // that layer scanning was skipped. + $loader = new RegistryConfigLoader( + $bundleVar . '/config', + $this->vendorDir, + ); + + $apps = $loader->load()->toArray(); + $this->assertSame('From default path', $apps['horde']['name']); + } } diff --git a/test/Unit/Config/RegistryConfigWriterTest.php b/test/Unit/Config/RegistryConfigWriterTest.php new file mode 100644 index 00000000..194b4f99 --- /dev/null +++ b/test/Unit/Config/RegistryConfigWriterTest.php @@ -0,0 +1,217 @@ +tempDir = sys_get_temp_dir() . '/horde-registry-writer-test-' . uniqid(); + mkdir($this->tempDir, 0o755, true); + } + + protected function tearDown(): void + { + $this->removeDirectory($this->tempDir); + } + + private function removeDirectory(string $dir): void + { + if (!is_dir($dir)) { + return; + } + // Restore write bits in case a test removed them. + @chmod($dir, 0o755); + $files = array_diff(scandir($dir), ['.', '..']); + foreach ($files as $file) { + $path = $dir . '/' . $file; + is_dir($path) ? $this->removeDirectory($path) : @unlink($path); + } + @rmdir($dir); + } + + // =================================================================== + // Round-trip + // =================================================================== + + public function testWriteProducesRequireableFile(): void + { + $compiled = [ + 'default' => [ + 'horde' => ['name' => 'Horde', 'status' => 'active'], + 'imp' => ['name' => 'IMP', 'params' => ['timeout' => 30]], + ], + 'example.com' => [ + 'imp' => ['webroot' => '/mail'], + ], + ]; + + $path = $this->tempDir . '/registry.php'; + (new RegistryConfigWriter())->write($compiled, $path); + + $this->assertFileExists($path); + + $loaded = require $path; + $this->assertSame($compiled, $loaded); + } + + public function testWriteHandlesEmptyDefaultOnly(): void + { + $compiled = ['default' => []]; + $path = $this->tempDir . '/registry.php'; + + (new RegistryConfigWriter())->write($compiled, $path); + + $this->assertSame($compiled, require $path); + } + + public function testWriteHandlesNestedStructures(): void + { + // Registry data can be arbitrarily deep. Verify var_export + // survives realistic depth without losing keys. + $compiled = [ + 'default' => [ + 'imp' => [ + 'name' => 'IMP', + 'params' => [ + 'nested' => [ + 'deeper' => [ + 'value' => 42, + 'list' => [1, 2, 3], + ], + ], + ], + ], + ], + ]; + $path = $this->tempDir . '/registry.php'; + + (new RegistryConfigWriter())->write($compiled, $path); + + $this->assertSame($compiled, require $path); + } + + // =================================================================== + // Directory handling + // =================================================================== + + public function testWriteCreatesMissingParentDirectory(): void + { + $path = $this->tempDir . '/cache/compiled/registry.php'; + $this->assertDirectoryDoesNotExist(dirname($path)); + + (new RegistryConfigWriter())->write(['default' => []], $path); + + $this->assertDirectoryExists(dirname($path)); + $this->assertFileExists($path); + } + + public function testWriteCreatesDeeplyNestedParentDirectories(): void + { + $path = $this->tempDir . '/a/b/c/d/registry.php'; + (new RegistryConfigWriter())->write(['default' => []], $path); + + $this->assertFileExists($path); + } + + public function testWriteWithExistingDirectoryIsNoop(): void + { + $dir = $this->tempDir . '/cache/compiled'; + mkdir($dir, 0o755, true); + // Sentinel file that must survive the write. + file_put_contents($dir . '/sibling.txt', 'do-not-touch'); + + (new RegistryConfigWriter()) + ->write(['default' => []], $dir . '/registry.php'); + + $this->assertSame('do-not-touch', file_get_contents($dir . '/sibling.txt')); + } + + // =================================================================== + // Atomicity + // =================================================================== + + public function testWriteLeavesNoStaleTempFiles(): void + { + $path = $this->tempDir . '/registry.php'; + (new RegistryConfigWriter())->write(['default' => []], $path); + + $entries = array_diff(scandir($this->tempDir), ['.', '..']); + // Only the final file should remain. The atomic-rename step + // moves the temp file into place. A successful write leaves + // nothing else behind. + $this->assertSame(['registry.php'], array_values($entries)); + } + + public function testWriteOverwritesExistingFile(): void + { + $path = $this->tempDir . '/registry.php'; + + $writer = new RegistryConfigWriter(); + $writer->write(['default' => ['horde' => ['name' => 'Old']]], $path); + $writer->write(['default' => ['horde' => ['name' => 'New']]], $path); + + $loaded = require $path; + $this->assertSame('New', $loaded['default']['horde']['name']); + } + + // =================================================================== + // Error handling + // =================================================================== + + public function testWriteFailsOnUnwritableParent(): void + { + // r-x on the parent: tempnam() can't create a temp file there, + // and the writer surfaces that as a RuntimeException rather + // than silently succeeding or leaving a broken state. + $parent = $this->tempDir . '/readonly'; + mkdir($parent, 0o755); + chmod($parent, 0o555); + + try { + $this->expectException(RuntimeException::class); + (new RegistryConfigWriter()) + ->write(['default' => []], $parent . '/registry.php'); + } finally { + chmod($parent, 0o755); + } + } +} From dd6900c6fc39e5159863749da4f3cff5e3e11be1 Mon Sep 17 00:00:00 2001 From: Ralf Lang Date: Mon, 6 Jul 2026 18:37:35 +0200 Subject: [PATCH 2/3] feat(config): expose RegistryConfigCompiler through injector factory --- src/Config/RegistryConfigCompiler.php | 4 ++ src/Factory/RegistryConfigCompilerFactory.php | 46 +++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 src/Factory/RegistryConfigCompilerFactory.php diff --git a/src/Config/RegistryConfigCompiler.php b/src/Config/RegistryConfigCompiler.php index 366e566a..a0b15053 100644 --- a/src/Config/RegistryConfigCompiler.php +++ b/src/Config/RegistryConfigCompiler.php @@ -16,6 +16,9 @@ namespace Horde\Core\Config; +use Horde\Core\Factory\RegistryConfigCompilerFactory; +use Horde\Injector\Attribute\Factory; + /** * Compile all registry sources into a single array graph. * @@ -81,6 +84,7 @@ * @license http://www.horde.org/licenses/lgpl21 LGPL 2.1 * @package Core */ +#[Factory(factory: RegistryConfigCompilerFactory::class, method: 'create')] class RegistryConfigCompiler { /** diff --git a/src/Factory/RegistryConfigCompilerFactory.php b/src/Factory/RegistryConfigCompilerFactory.php new file mode 100644 index 00000000..012fd519 --- /dev/null +++ b/src/Factory/RegistryConfigCompilerFactory.php @@ -0,0 +1,46 @@ + Date: Mon, 6 Jul 2026 19:12:08 +0200 Subject: [PATCH 3/3] fix(service): decouple group and permission factories from legacy $conf globals --- src/Factory/GroupServiceFactory.php | 74 +++++++++++++++++++++--- src/Factory/PermissionServiceFactory.php | 22 ++++--- 2 files changed, 80 insertions(+), 16 deletions(-) diff --git a/src/Factory/GroupServiceFactory.php b/src/Factory/GroupServiceFactory.php index a8a49c4a..f4a54a93 100644 --- a/src/Factory/GroupServiceFactory.php +++ b/src/Factory/GroupServiceFactory.php @@ -16,15 +16,28 @@ namespace Horde\Core\Factory; +use Horde\Core\Config\ConfigLoader; use Horde\Core\Service\GroupService; use Horde\Core\Service\SqlGroupService; use Horde\Injector\Injector; +use Horde_Group_Sql; +use RuntimeException; /** - * Factory for creating GroupService instances + * Factory for creating GroupService instances. * - * Creates the appropriate GroupService implementation based on the - * configured group driver (SQL, LDAP, etc.). + * Reads driver and params from the modern ConfigLoader rather than + * routing through the legacy Horde_Core_Factory_Group. The legacy + * factory depends on the `$conf` global being populated by + * Horde_Registry::appInit(), which the modern rampage-based request + * stack does not run. Modern requests (admin API) that need groups + * would fail with an obscure "array expected, null given" from + * Horde::getDriverConfig() otherwise. + * + * Mirrors PermissionServiceFactory's shape: modern config source, + * DbServiceFactory for pooled DB connections, direct construction + * of the legacy Horde_Group_* backend from typed config values, and + * a match statement per driver. * * @category Horde * @package Core @@ -34,17 +47,60 @@ class GroupServiceFactory { /** - * Create GroupService instance + * Create a GroupService instance. * * @param Injector $injector Dependency injector - * @return GroupService Group service instance + * @return GroupService Group service with configured backend + * @throws RuntimeException If the configured driver is unsupported */ public function create(Injector $injector): GroupService { - // Get the legacy Horde_Group instance (already configured via Horde_Core_Factory_Group) - $groupBackend = $injector->getInstance('Horde_Group'); + $loader = $injector->getInstance(ConfigLoader::class); + $state = $loader->load('horde'); + + $driver = strtolower($state->get('group.driver', 'sql')); + $params = $state->get('group.params', []); + + return match ($driver) { + 'sql' => $this->createSqlBackend($injector, $params), + default => throw new RuntimeException( + "Unsupported group driver: {$driver}. " + . "Modern GroupService currently supports 'sql' only. " + . "LDAP, File, and other legacy drivers are not yet ported." + ), + }; + } + + /** + * Construct the SQL-backed group service. + * + * Uses DbServiceFactory for connection pooling. The connection + * shares the main horde pool when params.driverconfig === 'horde' + * (the common case); an explicit db config in params yields a + * separate pool. + * + * Horde_Group_Sql only requires 'db' in its params. The base + * class treats 'cache' as optional (defaults to a Horde_Support_Stub) + * and never touches a logger. Deliberately omitting both here keeps + * this factory free of any legacy Horde_Core_Factory_* dependency + * that would need $GLOBALS['conf'] populated — the modern rampage + * request stack doesn't do that. + * + * @param Injector $injector Dependency injector + * @param array $params Group driver params from conf.php + * @return SqlGroupService SQL group service wrapping Horde_Group_Sql + */ + private function createSqlBackend( + Injector $injector, + array $params, + ): SqlGroupService { + $dbFactory = $injector->getInstance(DbServiceFactory::class); + $dbService = $dbFactory->create($injector, 'horde:group'); + + $backend = new Horde_Group_Sql([ + 'db' => $dbService->getAdapter(), + ]); - // Wrap in modern service - return new SqlGroupService($groupBackend); + return new SqlGroupService($backend); } } diff --git a/src/Factory/PermissionServiceFactory.php b/src/Factory/PermissionServiceFactory.php index 338306c3..7a9024cf 100644 --- a/src/Factory/PermissionServiceFactory.php +++ b/src/Factory/PermissionServiceFactory.php @@ -21,11 +21,11 @@ use Horde\Core\Service\NullPermissionService; use Horde\Core\Service\GroupService; use Horde\Core\Config\ConfigLoader; +use Horde_Cache; +use Horde_Cache_Storage_Null; use Horde_Perms_Sql; use Horde_Perms_Null; use Horde\Injector\Injector; -use Horde_Cache; -use Horde_Log_Logger; use RuntimeException; /** @@ -85,17 +85,25 @@ private function createSqlBackend( $dbFactory = $injector->getInstance(DbServiceFactory::class); $dbService = $dbFactory->create($injector, 'horde:perms'); - // Get dependencies - $cache = $injector->getInstance(Horde_Cache::class); - $logger = $injector->getInstance(Horde_Log_Logger::class); $groupService = $injector->getInstance(GroupService::class); - // Create legacy Horde_Perms_Sql backend + // Create legacy Horde_Perms_Sql backend. Horde_Perms_Sql calls + // $this->_cache->get() unconditionally in getPermission() and + // then passes the cache into Horde_Perms_Permission_Sql::setObs(), + // which strictly type-hints Horde_Cache. A no-op cache that + // still satisfies the type is what we want: a real Horde_Cache + // wired to Horde_Cache_Storage_Null. Both are standalone — + // no $GLOBALS['conf'] dependency, no external service. + // + // The legacy Horde_Core_Factory_Cache path would also produce + // a Horde_Cache, but reads driver and params from $conf, which + // the modern rampage stack doesn't populate. + $cache = new Horde_Cache(new Horde_Cache_Storage_Null()); + $permsParams = [ 'db' => $dbService->getAdapter(), 'table' => $params['table'] ?? 'horde_perms', 'cache' => $cache, - 'logger' => $logger, ]; $backend = new Horde_Perms_Sql($permsParams);