Circular Dependency Analyse & Oplossingsvoorstel
π DEEP DIVE: De Circular Dependency Chainβ
Huidige Situatieβ
Het Probleemβ
1. Softwarecatalog importeert en ziet dependency op opencatalogi
{
"dependencies": [{
"type": "nextcloud-app",
"app": "opencatalogi",
"required": true
}]
}
2. ImportHandler roept handleNextcloudAppDependencies() aan
- Checkt of opencatalogi enabled is
- Zo niet:
$appManager->enableApp('opencatalogi')
3. Nextcloud boot proces voor opencatalogi start
// opencatalogi/lib/AppInfo/Application.php:75
$settingsService->initialize();
4. SettingsService->initialize() roept loadSettings()
// opencatalogi/lib/Service/SettingsService.php:249
if ($this->shouldLoadSettings()) {
$this->loadSettings();
}
5. loadSettings() roept ConfigurationService->importFromApp()
// SettingsService.php:634
return $configurationService->importFromApp(
appId: 'opencatalogi',
data: $data,
version: $currentAppVersion,
force: $force
);
6. Dit triggert WEER ImportHandler->importFromJson()
- Die WEER
handleNextcloudAppDependencies()zou kunnen aanroepen - INFINITE LOOP als opencatalogi ook dependencies heeft
π― OPLOSSINGSVOORSTELLENβ
Optie 1: Guard Flag (Simpelst, Meest Robuust)β
Voeg een import context flag toe die voorkomt dat dependency checking recursief wordt aangeroepen.
Implementatie:β
// ImportHandler.php
private static bool $isDependencyCheckActive = false;
private function handleNextcloudAppDependencies(array $configData): void
{
// Prevent recursive dependency checking
if (self::$isDependencyCheckActive === true) {
$this->logger->debug('Skipping dependency check - already in dependency resolution context');
return;
}
$dependencies = $configData['x-openregister']['dependencies'] ?? [];
if (empty($dependencies) === true) {
return;
}
// Set guard flag
self::$isDependencyCheckActive = true;
try {
// ... existing dependency handling code ...
foreach ($dependencies as $dependency) {
// ... enable apps ...
}
} finally {
// Always reset flag, even on exception
self::$isDependencyCheckActive = false;
}
}
Voordelen:
- β Simpel te implementeren
- β Voorkomt alle recursie
- β Geen breaking changes
- β Works met elke dependency chain
Nadelen:
- β οΈ Global state (static property)
- β οΈ Bij parallel imports kan het problemen geven (maar Nextcloud is single-threaded)
Optie 2: Dependency Resolution Phase (Meest Clean)β
Splits de import in 2 fasen: dependency resolution en actual import.
Implementatie:β
// ImportHandler.php
public function importFromJson(
array $data,
?Configuration $configuration=null,
?string $owner=null,
?string $appId=null,
?string $version=null,
bool $force=false,
bool $resolveDependencies=true // NEW parameter
): array {
// PHASE 1: Resolve dependencies (if requested)
if ($resolveDependencies === true) {
$this->resolveDependenciesRecursive($data);
}
// PHASE 2: Import (dependencies already resolved)
// ... existing import logic ...
// importSeedData will NOT trigger dependency resolution again
}
private function resolveDependenciesRecursive(
array $configData,
array &$resolvedApps = []
): void {
$dependencies = $configData['x-openregister']['dependencies'] ?? [];
foreach ($dependencies as $dependency) {
if ($dependency['type'] !== 'nextcloud-app') {
continue;
}
$appId = $dependency['app'];
// Skip if already resolved
if (in_array($appId, $resolvedApps)) {
continue;
}
$appManager = \OC::$server->get(\OCP\App\IAppManager::class);
if (!$appManager->isEnabledForUser($appId)) {
$this->logger->info("Enabling dependency: {$appId}");
$appManager->enableApp($appId);
\OC_App::loadApp($appId);
// Mark as resolved BEFORE loading to prevent circular checks
$resolvedApps[] = $appId;
// Load the app's config and resolve ITS dependencies
$appConfig = $this->loadAppConfig($appId);
if ($appConfig) {
$this->resolveDependenciesRecursive($appConfig, $resolvedApps);
}
} else {
$resolvedApps[] = $appId;
}
}
}
Voordelen:
- β Clean separation of concerns
- β Explicit dependency tree resolution
- β Supports multi-level dependencies
- β No global state
Nadelen:
- β οΈ Meer code wijzigingen
- β οΈ Complexer om te testen
Optie 3: Lazy Dependency Resolution (Meest Flexibel)β
Controleer dependencies ALLEEN voor seedData, niet tijdens de algemene import.
Implementatie:β
// Move dependency check from importFromJson to importSeedData
private function importSeedData(
array $configData,
?string $owner,
?string $appId,
Configuration $configuration,
array &$result
): void {
$seedData = $configData['x-openregister']['seedData'] ?? null;
if ($seedData === null || empty($seedData['objects']) === true) {
return;
}
// Check dependencies ONLY when we actually need seedData
// At this point, schemas/registers are already imported
$this->ensureDependenciesForSeedData($configData);
// ... rest of seedData import ...
}
private function ensureDependenciesForSeedData(array $configData): void
{
$dependencies = $configData['x-openregister']['dependencies'] ?? [];
foreach ($dependencies as $dependency) {
if ($dependency['type'] !== 'nextcloud-app') {
continue;
}
$appId = $dependency['app'];
$appManager = \OC::$server->get(\OCP\App\IAppManager::class);
// Only enable if NOT already enabled
if (!$appManager->isEnabledForUser($appId)) {
$this->logger->info("Enabling app for seedData: {$appId}");
$appManager->enableApp($appId);
\OC_App::loadApp($appId);
// DO NOT recursively check this app's dependencies
// It will handle its own config loading independently
}
}
}
Voordelen:
- β Dependencies only checked when needed
- β Minimale impact op bestaande code
- β Apps kunnen hun eigen config laden zonder conflict
Nadelen:
- β οΈ Dependency niet gecheckt voor algemene imports (alleen seedData)
- β οΈ Kan nog steeds circular triggering hebben als app zelf import doet
Optie 4: Boot Hook System (Meest Elegant, Meeste Werk)β
Gebruik Nextcloud's boot hooks om dependencies te resolven VOOR app boot.
Implementatie:β
// Register een pre-boot hook
$context->registerBootstrap(\OCA\OpenRegister\Bootstrap\DependencyResolver::class);
// DependencyResolver.php
class DependencyResolver implements IBootstrap {
public function register(IRegistrationContext $context): void {
// Register before apps boot
}
public function boot(IBootContext $context): void {
// Check ALL app configs for dependencies
// Enable required apps BEFORE they boot
$this->resolveAllDependencies();
}
}
Voordelen:
- β Nextcloud-native oplossing
- β Dependencies resolved voor ALL apps
- β No circular issues mogelijk
Nadelen:
- β οΈ Veel werk om te implementeren
- β οΈ Vergt wijzigingen in app bootstrap
- β οΈ Kan performance impact hebben (alle configs checken bij boot)
π AANBEVELING: Combinatie van Optie 1 + 3β
Waarom?β
- Optie 1 (Guard Flag) voorkomt recursie - MUST HAVE
- Optie 3 (Lazy Resolution) checkt alleen voor seedData - EFFICIENT
- Samen geven ze maximale veiligheid met minimale overhead
Implementatie Plan:β
// ImportHandler.php
private static bool $isDependencyCheckActive = false;
public function importFromJson(...) {
// Normal import without dependency check
// ... schemas, registers import ...
// Seed data import WITH dependency check
if ($configuration !== null) {
$this->importSeedData(...);
}
}
private function importSeedData(...) {
$seedData = $configData['x-openregister']['seedData'] ?? null;
if (!$seedData) return;
// Check dependencies with guard
$this->ensureDependenciesForSeedData($configData);
// ... import objects ...
}
private function ensureDependenciesForSeedData(array $configData): void
{
// GUARD: Prevent recursive calls
if (self::$isDependencyCheckActive === true) {
$this->logger->debug('Skipping recursive dependency check');
return;
}
self::$isDependencyCheckActive = true;
try {
$dependencies = $configData['x-openregister']['dependencies'] ?? [];
foreach ($dependencies as $dependency) {
if ($dependency['type'] !== 'nextcloud-app') {
continue;
}
$appId = $dependency['app'];
$appManager = \OC::$server->get(\OCP\App\IAppManager::class);
if ($appManager->isInstalled($appId) === false) {
$this->logger->warning("Required app '{$appId}' not installed");
if ($dependency['required'] ?? false) {
throw new Exception("Required app '{$appId}' not installed");
}
continue;
}
if (!$appManager->isEnabledForUser($appId)) {
$this->logger->info("Enabling required app: {$appId}");
$appManager->enableApp($appId);
\OC_App::loadApp($appId);
}
}
} finally {
self::$isDependencyCheckActive = false;
}
}
Voordelen van deze combinatie:β
β
Veilig: Guard voorkomt alle recursie
β
EfficiΓ«nt: Alleen checken waar nodig (seedData)
β
Simpel: Minimale code changes
β
Robuust: Works met complexe dependency chains
β
Backwards compatible: Geen breaking changes
π IMPLEMENTATIE STAPPENβ
- β
Stap 1: Voeg guard flag toe aan
ImportHandler - β
Stap 2: Move
handleNextcloudAppDependenciescall naarimportSeedData - β
Stap 3: Hernoem naar
ensureDependenciesForSeedData - β Stap 4: Test met softwarecatalog -> opencatalogi dependency
- β Stap 5: Voeg openconnector dependency toe aan softwarecatalog
- β
Stap 6: Documenteer in
website/docs/development/
π§ͺ TEST SCENARIOβ
Clean Database
βββ Enable ONLY openregister
βββ Import softwarecatalog config
β βββ Has dependency: opencatalogi (required)
β βββ Has dependency: openconnector (optional)
β βββ Has seedData: pages, menus
βββ ImportHandler sees dependencies
βββ Enables opencatalogi
β βββ Opencatalogi boots
β βββ Loads own config (ImportHandler called AGAIN)
β βββ Guard flag PREVENTS recursive dependency check β
β βββ Import completes
βββ Enables openconnector (optional, skips if not installed)
βββ ImportSeedData finds page/menu schemas from opencatalogi β
βββ Creates 4 pages + 3 menus β
Expected Result:
- 21 softwarecatalog schemas
- 48 opencatalogi schemas
- 7 seed objects (4 pages + 3 menus)
- NO infinite loops
- NO hangs
π‘ BONUS: Future Enhancementβ
Voor de toekomst kunnen we dependency metadata toevoegen:
{
"dependencies": [
{
"type": "nextcloud-app",
"app": "opencatalogi",
"version": ">=0.7.0",
"required": true,
"provides": ["page", "menu", "glossary"],
"reason": "Provides page/menu schemas for seedData"
}
]
}
Dit maakt het mogelijk om:
- β Checken of de juiste versie geΓ―nstalleerd is
- β Weten welke schemas een app provides
- β Betere error messages bij missing dependencies
- β Dependency graph visualisatie in UI