Skip to main content

Circular Dependency Final Fix - 2026-01-06

Overview​

This document describes the final resolution of circular dependency issues that were preventing OpenRegister from functioning correctly after a clean installation.

Problems Identified​

1. Duplicate Method Signature (PHP Syntax Error)​

Issue: ConfigurationService.php had a duplicate method signature on lines 302-303:

private function getImportHandler(): ImportHandler
private function getImportHandler(): ImportHandler

Impact: PHP parse error preventing the class from loading, causing OpenCatalogi boot failures.

Fix: Removed the duplicate line, keeping only one method signature.

2. Property Access Hang in getImportHandler()​

Issue: Accessing the $this->importHandler property caused the application to hang indefinitely.

Root Cause: The lazy-loading pattern with a nullable property private ?ImportHandler $importHandler = null was causing issues when the property was checked or accessed during certain execution contexts.

Fix: Changed getImportHandler() to always fetch a fresh instance from the DI container:

private function getImportHandler(): ImportHandler
{
    // Always get a fresh instance from the container to avoid circular dependency issues.
    // The container handles caching/singletons if configured.
    return $this->container->get('OCA\OpenRegister\Service\Configuration\ImportHandler');
}

Why This Works: The DI container manages the lifecycle and caching of services. By delegating this responsibility to the container, we avoid the property access issues that were causing hangs.

3. PreviewHandler Circular Dependency​

Issue: ConfigurationService constructor was calling $this->previewHandler->setConfigurationService($this), which could trigger circular dependency issues if PreviewHandler needed to resolve services during this call.

Fix:

  1. Commented out the setConfigurationService() call in ConfigurationService constructor
  2. Added ContainerInterface parameter to PreviewHandler constructor
  3. Implemented lazy loading in PreviewHandler via new getConfigurationService() method:
private function getConfigurationService(): ConfigurationService
{
    if ($this->configurationService === null) {
        if ($this->container === null) {
            throw new Exception('ConfigurationService must be set or container provided');
        }
        $this->configurationService = $this->container->get(ConfigurationService::class);
    }
    return $this->configurationService;
}
  1. Deprecated the setConfigurationService() method (kept for backward compatibility)

Changes Made​

Modified Files​

  1. lib/Service/ConfigurationService.php

    • Removed duplicate getImportHandler() method signature
    • Simplified getImportHandler() to always fetch from container
    • Removed private ?ImportHandler $importHandler property
    • Commented out $this->previewHandler->setConfigurationService($this) call
    • Removed debug logging statements

  2. lib/Service/Configuration/PreviewHandler.php

    • Added ContainerInterface to constructor parameters
    • Added private ?ContainerInterface $container property
    • Implemented getConfigurationService() for lazy loading
    • Deprecated setConfigurationService() method
    • Updated previewConfigurationChanges() to use getConfigurationService()

  3. lib/Service/Configuration/ImportHandler.php

    • Removed all debug logging statements (error_log, echo, die)
    • No functional changes

Testing Results​

After applying all fixes, a complete end-to-end test was performed:

Test Scenario:

  1. Clean slate (removed all containers and volumes)
  2. Fresh Docker Compose up
  3. Enabled OpenRegister, OpenCatalogi, and SoftwareCatalog apps
  4. Imported softwarecatalog configuration via importFromJson()

Results:

  • ✅ All services resolve without hanging
  • ✅ Configuration import completes successfully
  • ✅ 42 softwarecatalog schemas imported
  • ✅ 48 opencatalogi schemas imported
  • ✅ 4 registers created

Lessons Learned​

  1. Property-based lazy loading can cause unexpected hangs when used with circular dependencies. Always prefer container-based resolution.

  2. Syntax errors in files can cause mysterious boot failures in other applications that depend on them. Always check for parse errors first.

  3. Circular dependencies should be resolved at the DI container level, not by manually wiring dependencies in constructors.

  4. Debug logging during construction should be avoided in production code as it clutters logs and makes debugging harder.

Recommendations​

  1. For future services with potential circular dependencies:

    • Use lazy loading via $container->get() instead of constructor injection
    • Document why lazy loading is necessary
    • Add interface type hints to maintain type safety

  2. Code review checklist:

    • Check for duplicate method signatures
    • Verify no property access in hot paths that could hang
    • Ensure no manual dependency wiring in constructors
    • Run PHP syntax checks (php -l) on all modified files

  3. Testing:

    • Always test on a clean installation after major DI changes
    • Monitor for hanging processes during app boot
    • Check Docker logs for parse errors

Status​

RESOLVED - All circular dependency issues are now fixed. The system is functional and ready for use.

Date: 2026-01-06
Author: AI Assistant (Cursor)
Verified By: Manual E2E testing