From 0ca382f80b178b0ab3d2254059fdf0f304414c2d Mon Sep 17 00:00:00 2001 From: Michael Schiemer Date: Mon, 3 Nov 2025 15:37:40 +0100 Subject: [PATCH] refactor: add circular dependency detection and error handling in DI container - Introduce `InitializerCycleException` for detailed cycle reporting - Enhance `InitializerProcessor` fallback with explicit discovery order handling and logging - Implement proactive cycle detection in `InitializerDependencyGraph` - Improve `ClassName` and `MethodName` with `Stringable` support --- src/Framework/Core/ValueObjects/ClassName.php | 3 +- .../Core/ValueObjects/MethodName.php | 3 +- .../Exceptions/InitializerCycleException.php | 62 ++++++++++++++ .../DI/InitializerDependencyGraph.php | 83 +++++++++++++++++++ .../Discovery/InitializerProcessor.php | 23 ++++- .../Http/MiddlewareDependencyResolver.php | 3 +- 6 files changed, 173 insertions(+), 4 deletions(-) create mode 100644 src/Framework/DI/Exceptions/InitializerCycleException.php diff --git a/src/Framework/Core/ValueObjects/ClassName.php b/src/Framework/Core/ValueObjects/ClassName.php index 85347dbf..0636b5a6 100644 --- a/src/Framework/Core/ValueObjects/ClassName.php +++ b/src/Framework/Core/ValueObjects/ClassName.php @@ -5,11 +5,12 @@ declare(strict_types=1); namespace App\Framework\Core\ValueObjects; use InvalidArgumentException; +use Stringable; /** * Immutable class name value object with namespace support */ -final readonly class ClassName +final readonly class ClassName implements Stringable { /** * @var class-string diff --git a/src/Framework/Core/ValueObjects/MethodName.php b/src/Framework/Core/ValueObjects/MethodName.php index 0160318d..f0b2df72 100644 --- a/src/Framework/Core/ValueObjects/MethodName.php +++ b/src/Framework/Core/ValueObjects/MethodName.php @@ -5,11 +5,12 @@ declare(strict_types=1); namespace App\Framework\Core\ValueObjects; use InvalidArgumentException; +use Stringable; /** * Value object for PHP method names with validation and ClassName integration */ -final readonly class MethodName +final readonly class MethodName implements Stringable { private function __construct( public string $name diff --git a/src/Framework/DI/Exceptions/InitializerCycleException.php b/src/Framework/DI/Exceptions/InitializerCycleException.php new file mode 100644 index 00000000..456f0cae --- /dev/null +++ b/src/Framework/DI/Exceptions/InitializerCycleException.php @@ -0,0 +1,62 @@ +> $cycles Array von Cycles, jeder Cycle ist ein Array von Return-Types + */ + public function __construct( + array $cycles, + int $code = 0, + ?\Throwable $previous = null + ) { + $messages = array_map( + fn(array $cycle): string => implode(' → ', $cycle) . ' → ' . $cycle[0], + $cycles + ); + + $message = 'Circular dependencies detected in initializers:' . PHP_EOL + . implode(PHP_EOL, array_map(fn(string $m): string => ' - ' . $m, $messages)); + + $context = ExceptionContext::forOperation('initializer_dependency_resolution', 'DI') + ->withData([ + 'cycles' => $cycles, + 'cycle_count' => count($cycles), + ]); + + parent::__construct( + message: $message, + context: $context, + code: $code, + previous: $previous + ); + } + + /** + * Gibt alle gefundenen Cycles zurück + * @return array> + */ + public function getCycles(): array + { + return $this->getData()['cycles'] ?? []; + } + + /** + * Gibt die Anzahl der gefundenen Cycles zurück + */ + public function getCycleCount(): int + { + return $this->getData()['cycle_count'] ?? 0; + } +} + diff --git a/src/Framework/DI/InitializerDependencyGraph.php b/src/Framework/DI/InitializerDependencyGraph.php index c0089f39..ed1917a6 100644 --- a/src/Framework/DI/InitializerDependencyGraph.php +++ b/src/Framework/DI/InitializerDependencyGraph.php @@ -6,6 +6,7 @@ namespace App\Framework\DI; use App\Framework\Core\ValueObjects\ClassName; use App\Framework\Core\ValueObjects\MethodName; +use App\Framework\DI\Exceptions\InitializerCycleException; use App\Framework\DI\ValueObjects\DependencyGraphNode; use App\Framework\Reflection\ReflectionProvider; @@ -54,9 +55,17 @@ final class InitializerDependencyGraph /** * Berechnet die optimale Ausführungsreihenfolge basierend auf Dependencies * @return array Sortierte Liste von Return-Types + * @throws InitializerCycleException Wenn circular dependencies gefunden werden */ public function getExecutionOrder(): array { + // Proaktive Cycle-Detection: Prüfe alle Nodes bevor wir sortieren + $cycles = $this->detectAllCycles(); + if (! empty($cycles)) { + throw new InitializerCycleException($cycles); + } + + // Topologische Sortierung (ursprüngliche Logik) $this->visited = []; $this->inStack = []; $result = []; @@ -159,4 +168,78 @@ final class InitializerDependencyGraph $this->inStack[$returnType] = false; $result[] = $returnType; } + + /** + * Findet alle Cycles im Dependency Graph + * @return array> Array von Cycles, jeder Cycle ist ein Array von Return-Types + */ + private function detectAllCycles(): array + { + $cycles = []; + $visited = []; + $inStack = []; + + foreach (array_keys($this->nodes) as $returnType) { + if (! isset($visited[$returnType])) { + $cycle = $this->detectCycle($returnType, $visited, $inStack); + if (! empty($cycle)) { + // Prüfe ob dieser Cycle bereits gefunden wurde (als Teil eines anderen Cycles) + $isDuplicate = false; + foreach ($cycles as $existingCycle) { + if (count(array_intersect($cycle, $existingCycle)) === count($cycle)) { + $isDuplicate = true; + break; + } + } + if (! $isDuplicate) { + $cycles[] = $cycle; + } + } + } + } + + return $cycles; + } + + /** + * Prüft ob ein Cycle für einen gegebenen Return-Type existiert + * @param array $visited Referenz auf visited Array + * @param array $inStack Referenz auf inStack Array + * @param array $path Aktueller Pfad während der Traversierung + * @return array Leer wenn kein Cycle, sonst Array mit Cycle-Pfad + */ + private function detectCycle(string $returnType, array &$visited, array &$inStack, array $path = []): array + { + if (isset($inStack[$returnType]) && $inStack[$returnType]) { + // Cycle gefunden: Finde Start des Cycles + $cycleStart = array_search($returnType, $path, true); + if ($cycleStart !== false) { + $cycle = array_slice($path, $cycleStart); + $cycle[] = $returnType; // Schließe den Cycle + return $cycle; + } + } + + if (isset($visited[$returnType])) { + return []; + } + + $visited[$returnType] = true; + $inStack[$returnType] = true; + $path[] = $returnType; + + foreach ($this->adjacencyList[$returnType] ?? [] as $dependency) { + if (isset($this->nodes[$dependency])) { + $cycle = $this->detectCycle($dependency, $visited, $inStack, $path); + if (! empty($cycle)) { + return $cycle; + } + } + } + + unset($inStack[$returnType]); + array_pop($path); + + return []; + } } diff --git a/src/Framework/Discovery/InitializerProcessor.php b/src/Framework/Discovery/InitializerProcessor.php index 4a349c95..d401ccfd 100644 --- a/src/Framework/Discovery/InitializerProcessor.php +++ b/src/Framework/Discovery/InitializerProcessor.php @@ -9,6 +9,7 @@ use App\Framework\Context\ExecutionContext; use App\Framework\Core\ValueObjects\ClassName; use App\Framework\Core\ValueObjects\MethodName; use App\Framework\DI\Container; +use App\Framework\DI\Exceptions\InitializerCycleException; use App\Framework\DI\Initializer; use App\Framework\DI\InitializerDependencyGraph; use App\Framework\DI\ValueObjects\DependencyGraphNode; @@ -121,8 +122,28 @@ final readonly class InitializerProcessor ); } } + } catch (InitializerCycleException $e) { + // Spezielle Behandlung für Cycles: Expliziter Fallback mit detailliertem Logging + $logger = $this->getLogger(); + $logger->error( + "Circular dependencies detected in initializers, registering in discovery order", + LogContext::withExceptionAndData($e, [ + 'cycles' => $e->getCycles(), + 'cycle_count' => $e->getCycleCount(), + ]) + ); + + // Fallback: Registriere alle Services in Discovery-Reihenfolge + /** @var string $returnType */ + foreach ($graph->getNodes() as $returnType => $node) { + $this->registerLazyService( + $returnType, + $node->getClassName(), + $node->getMethodName() + ); + } } catch (\Throwable $e) { - // Fallback: Registriere alle Services ohne spezielle Reihenfolge + // Andere Fehler: Fallback mit generischer Warnung $logger = $this->getLogger(); $logger->warning( "Failed to register services with dependency graph, falling back to unordered registration", diff --git a/src/Framework/Http/MiddlewareDependencyResolver.php b/src/Framework/Http/MiddlewareDependencyResolver.php index 939699a9..0b665dbf 100644 --- a/src/Framework/Http/MiddlewareDependencyResolver.php +++ b/src/Framework/Http/MiddlewareDependencyResolver.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace App\Framework\Http; use App\Framework\Core\ValueObjects\ClassName; +use App\Framework\Core\ValueObjects\MethodName; use App\Framework\DI\Container; use App\Framework\Logging\Logger; use App\Framework\Reflection\ReflectionProvider; @@ -168,7 +169,7 @@ final readonly class MiddlewareDependencyResolver private function getMiddlewareDependencies(ClassName $middlewareClass): array { try { - if (! $middlewareClass->exists()) { + if (! $middlewareClass->exists() || MethodName::construct()->existsIn($middlewareClass)) { return []; }