# Code Review: TreeCalcBot.php ## Zusammenfassung der Analyse Die `TreeCalcBot` Klasse ist ein komplexes Service-Layer-Tool zur Berechnung und Darstellung von Multi-Level-Marketing (MLM) Geschäftsstrukturen. Die Klasse verarbeitet Benutzer-Hierarchien, berechnet Verkaufsvolumen und generiert HTML-Trees für die Frontend-Darstellung. **Hauptprobleme:** - Massive Performance-Probleme durch N+1 Abfragen und fehlende Lazy Loading - Vermischung von Datenzugriff, Geschäftslogik und Präsentationsschicht - Über 390 Zeilen Code in einer einzigen Klasse (Single Responsibility Principle verletzt) - Keine Fehlerbehandlung bei kritischen Operationen - HTML-Generierung direkt in der Service-Klasse - Ineffiziente rekursive Strukturverarbeitung ohne Memory-Management **Risiko-Level:** 🔴 HOCH - Potenzielle Systemausfälle bei großen Datenmengen ## Detaillierte Verbesserungsvorschläge ### 1. Performance-Optimierung: N+1 Problem beheben **IST-Zustand (Zeile 151-159):** ```php private function readRootUsers(){ $users = User::with('account')->select('users.*') ->where('users.deleted_at', '=', null) ->where('users.id', '!=', 1) ->where('users.admin', "<", 4) ->where('users.m_level', "!=", null) ->where('users.m_sponsor', "=", null) ->where('users.payment_account', "!=", null) ->where('users.active_date', "<=", $this->date->end_date) ->get(); if($users){ foreach($users as $user){ $BusinessUserItem = new BusinessUserItem($this->date); $BusinessUserItem->makeUser($user->id); // HIER: N+1 Problem! $BusinessUserItem->addUserID(); $this->business_users[] = $BusinessUserItem; } } } ``` **SOLL-Vorschlag:** ```php private function readRootUsers(){ $users = User::with([ 'account', 'userLevel', 'userBusiness' => function($query) { $query->where('month', $this->date->month) ->where('year', $this->date->year); } ])->select('users.*') ->where('users.deleted_at', '=', null) ->where('users.id', '!=', 1) ->where('users.admin', "<", 4) ->where('users.m_level', "!=", null) ->where('users.m_sponsor', "=", null) ->where('users.payment_account', "!=", null) ->where('users.active_date', "<=", $this->date->end_date) ->chunk(100, function($users) { foreach($users as $user){ $BusinessUserItem = new BusinessUserItem($this->date); $BusinessUserItem->makeUserFromModel($user); // Verwende bereits geladene Daten $BusinessUserItem->addUserID(); $this->business_users[] = $BusinessUserItem; } }); } ``` **Begründung:** Verhindert das N+1-Problem durch Eager Loading und reduziert Memory-Verbrauch durch Chunking. Anzahl der DB-Abfragen reduziert sich von N+1 auf 1 pro Chunk. ### 2. Single Responsibility Principle: HTML-Generierung auslagern **IST-Zustand (Zeile 250-391):** ```php public function makeHtmlTree(){ $deep = 0; $ret = '
    '; foreach($this->business_users as $business_user){ $ret .= $this->addItem($business_user, $deep); } $ret .= '
'; return $ret; } private function addItem($item, $deep){ // 55 Zeilen HTML-String-Concatenation... } ``` **SOLL-Vorschlag:** ```php // Neue Klasse: App\Services\BusinessPlan\TreeHtmlRenderer.php class TreeHtmlRenderer { private $initFrom; public function __construct($initFrom = 'member') { $this->initFrom = $initFrom; } public function renderTree(array $businessUsers): string { return view('admin.business.components.tree', [ 'business_users' => $businessUsers, 'init_from' => $this->initFrom ])->render(); } public function renderParentless(array $parentless): string { return view('admin.business.components.parentless', [ 'parentless' => $parentless, 'init_from' => $this->initFrom ])->render(); } } // In TreeCalcBot.php: public function makeHtmlTree(): string { $renderer = new TreeHtmlRenderer($this->init_from); return $renderer->renderTree($this->business_users); } ``` **Begründung:** Trennt Darstellungslogik von Geschäftslogik, verbessert Testbarkeit und Wartbarkeit. HTML-Templates sind wiederverwendbar und können einfacher angepasst werden. ### 3. Memory-Management: Große Datenmengen effizienter verarbeiten **IST-Zustand (Zeile 176-192):** ```php private function readParentlessUser(){ $users = User::with('account')->select('users.*') ->where('users.deleted_at', '=', null) ->where('users.id', '!=', 1) ->where('users.admin', "<", 4) ->where('users.payment_account', "!=", null) ->where('users.active_date', "<=", $this->date->end_date) ->get(); // Lädt ALLE User in Memory! foreach($users as $user){ if(!isset(self::$userIDs[$user->id])){ $BusinessUserItem = new BusinessUserItem($this->date); $BusinessUserItem->makeUser($user->id); $this->parentless[] = $BusinessUserItem; } } } ``` **SOLL-Vorschlag:** ```php private function readParentlessUser(){ User::with('account')->select('users.*') ->where('users.deleted_at', '=', null) ->where('users.id', '!=', 1) ->where('users.admin', "<", 4) ->where('users.payment_account', "!=", null) ->where('users.active_date', "<=", $this->date->end_date) ->whereNotIn('users.id', array_keys(self::$userIDs)) ->lazy(100) // Lazy Loading für Memory-Effizienz ->each(function($user) { $BusinessUserItem = new BusinessUserItem($this->date); $BusinessUserItem->makeUserFromModel($user); $this->parentless[] = $BusinessUserItem; }); } ``` **Begründung:** Lazy Loading verhindert Memory-Overflow bei großen Datenmengen. `lazy()` lädt nur jeweils 100 Datensätze in Memory, anstatt alle auf einmal. ### 4. Robuste Fehlerbehandlung implementieren **IST-Zustand (Zeile 229-236):** ```php public function readSponsorUser($user_id){ $user = User::find($user_id); $userSponsor = User::find($user->m_sponsor); // Potenzielle null-pointer Exception! if($userSponsor){ $this->sponsor = new BusinessUserItem($this->date); $this->sponsor->makeUser($userSponsor->id); } } ``` **SOLL-Vorschlag:** ```php public function readSponsorUser($user_id){ try { $user = User::find($user_id); if (!$user) { \Log::warning("TreeCalcBot: User not found: {$user_id}"); return; } if (!$user->m_sponsor) { return; // Kein Sponsor definiert } $userSponsor = User::find($user->m_sponsor); if ($userSponsor) { $this->sponsor = new BusinessUserItem($this->date); $this->sponsor->makeUserFromModel($userSponsor); } else { \Log::warning("TreeCalcBot: Sponsor not found: {$user->m_sponsor} for user: {$user_id}"); } } catch (\Exception $e) { \Log::error("TreeCalcBot: Error reading sponsor for user {$user_id}: " . $e->getMessage()); } } ``` **Begründung:** Verhindert Systemabstürze durch null-pointer Exceptions und bietet aussagekräftige Logging-Informationen für Debugging. ### 5. Dependency Injection für bessere Testbarkeit **IST-Zustand (Zeile 25-35):** ```php public function __construct($month, $year, $init_from = 'member') { $this->date = new stdClass(); $date = Carbon::parse($year.'-'.$month.'-1'); $this->date->month = $month; $this->date->year = $year; $this->date->start_date = $date->format('Y-m-d H:i:s'); $this->date->end_date = $date->endOfMonth()->format('Y-m-d H:i:s'); $this->init_from = $init_from; } ``` **SOLL-Vorschlag:** ```php use Carbon\Carbon; use Illuminate\Contracts\Logging\Log; public function __construct( int $month, int $year, string $init_from = 'member', Log $logger = null ) { $this->validateInput($month, $year); $this->date = new stdClass(); $date = Carbon::parse($year.'-'.$month.'-1'); $this->date->month = $month; $this->date->year = $year; $this->date->start_date = $date->format('Y-m-d H:i:s'); $this->date->end_date = $date->endOfMonth()->format('Y-m-d H:i:s'); $this->init_from = $init_from; $this->logger = $logger ?? app(Log::class); } private function validateInput(int $month, int $year): void { if ($month < 1 || $month > 12) { throw new \InvalidArgumentException("Invalid month: {$month}"); } if ($year < 2020 || $year > date('Y') + 1) { throw new \InvalidArgumentException("Invalid year: {$year}"); } } ``` **Begründung:** Ermöglicht Unit-Testing durch Dependency Injection und validiert Eingabeparameter zur Laufzeit. ### 6. Static Property für bessere Performance **IST-Zustand (Zeile 19-23):** ```php private static $userIDs = []; public static function addUserID($id){ self::$userIDs[$id] = $id; } ``` **SOLL-Vorschlag:** ```php private array $processedUserIDs = []; public function addUserID(int $id): void { $this->processedUserIDs[$id] = true; // Boolean statt ID als Wert spart Memory } public function isUserProcessed(int $id): bool { return isset($this->processedUserIDs[$id]); } public function clearProcessedUsers(): void { $this->processedUserIDs = []; } ``` **Begründung:** Nicht-statische Properties vermeiden Memory-Leaks zwischen verschiedenen Instanzen und Boolean-Werte sparen Speicher. ## Lösung für langlaufende Cron-Jobs (Asynchrone Verarbeitung) ### Konzept Die aktuelle Implementierung läuft synchron und verarbeitet alle Business-User-Strukturen in einem einzigen Request, was bei großen Datenmengen zu Timeouts führt. Die beste Lösung ist die Auslagerung in Laravel Queues, die folgende Vorteile bietet: - **Asynchrone Verarbeitung:** Keine Timeout-Probleme - **Skalierbarkeit:** Mehrere Worker können parallel arbeiten - **Fehlerbehandlung:** Automatische Retry-Mechanismen bei Fehlern - **Monitoring:** Queue-Status kann überwacht werden - **Memory-Management:** Jeder Job hat eigene Memory-Grenzen ### Schritt-für-Schritt-Anleitung #### 1. Job-Klasse erstellen ```bash php artisan make:job ProcessBusinessStructure ``` #### 2. Job-Implementierung ```php month = $month; $this->year = $year; $this->step = $step; } public function handle() { Log::info("Processing Business Structure: Month {$this->month}, Year {$this->year}, Step: {$this->step}"); $businessStore = new BusinessUsersStore($this->month, $this->year); try { switch ($this->step) { case 'structure': $businessStore->storeUserBusinessStructure(); // Nächsten Schritt als neuen Job dispatchen ProcessBusinessStructure::dispatch($this->month, $this->year, 'details') ->delay(now()->addSeconds(10)); break; case 'details': $businessStore->storeBusinessUsersDetail(); // Finalisierung als neuen Job dispatchen ProcessBusinessStructure::dispatch($this->month, $this->year, 'complete') ->delay(now()->addSeconds(10)); break; case 'complete': $businessStore->storeBusinessCompleted(); Log::info("Business Structure processing completed for {$this->month}/{$this->year}"); break; } } catch (\Exception $e) { Log::error("Business Structure processing failed: " . $e->getMessage()); throw $e; // Re-throw für Queue Retry-Mechanismus } } public function failed(\Throwable $exception) { Log::error("Business Structure Job failed permanently: " . $exception->getMessage()); // Optional: E-Mail-Benachrichtigung an Admins } } ``` #### 3. User-spezifische Job-Klasse für Details ```bash php artisan make:job ProcessUserBusinessDetails ``` ```php userId = $userId; $this->month = $month; $this->year = $year; } public function handle() { $user = User::find($this->userId); if (!$user) { \Log::warning("User {$this->userId} not found for business details processing"); return; } $treeCalcBot = new TreeCalcBot($this->month, $this->year, 'admin'); $treeCalcBot->initBusinesslUserDetail($user); if ($treeCalcBot->business_user) { // Store logic hier implementieren \Log::info("Processed business details for user {$this->userId}"); } } } ``` #### 4. Artisan Command anpassen ```php argument('month') ?? date('n'); $year = $this->argument('year') ?? date('Y'); $this->info("Dispatching business structure processing for {$month}/{$year}"); // Job in Queue einreihen ProcessBusinessStructure::dispatch($month, $year); $this->info("Job dispatched successfully. Check queue:work for progress."); } } ``` #### 5. Queue-Konfiguration **config/queue.php anpassen:** ```php 'connections' => [ 'database' => [ 'driver' => 'database', 'table' => 'jobs', 'queue' => 'default', 'retry_after' => 600, 'after_commit' => false, ], ], ``` **Migration für Jobs-Tabelle:** ```bash php artisan queue:table php artisan migrate ``` #### 6. Queue Worker starten ```bash # Produktiv: php artisan queue:work --queue=default --tries=3 --timeout=300 # Mit Supervisor für automatischen Neustart: # /etc/supervisor/conf.d/laravel-worker.conf [program:laravel-worker] process_name=%(program_name)s_%(process_num)02d command=php /path/to/project/artisan queue:work --sleep=3 --tries=3 --timeout=300 autostart=true autorestart=true user=www-data numprocs=2 redirect_stderr=true stdout_logfile=/path/to/project/storage/logs/worker.log ``` #### 7. Cron-Integration ```php // app/Console/Kernel.php protected function schedule(Schedule $schedule) { // Einmal monatlich am 1. um 02:00 Uhr $schedule->command('business:store') ->monthlyOn(1, '02:00') ->withoutOverlapping() ->onOneServer(); } ``` ### Monitoring und Fehlerbehandlung ```bash # Queue Status überwachen php artisan queue:monitor database --max=100 # Failed Jobs anzeigen php artisan queue:failed # Failed Job erneut versuchen php artisan queue:retry all ``` Diese Lösung stellt sicher, dass auch bei großen Datenmengen die Verarbeitung erfolgreich abgeschlossen wird, ohne dass Timeout-Probleme auftreten.