Kevin Adametz 480fdc65ed commit 08-2025

2025-08-12 18:01:59 +02:00

16 KiB

Raw Blame History

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):

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:

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):

public function makeHtmlTree(){
    $deep = 0;
    $ret = '<ol class="dd-list">';
        foreach($this->business_users as $business_user){
            $ret .= $this->addItem($business_user, $deep);
        }
    $ret .= '</ol>';
    return $ret;
}

private function addItem($item, $deep){
    // 55 Zeilen HTML-String-Concatenation...
}

SOLL-Vorschlag:

// 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):

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:

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):

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:

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):

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:

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):

private static $userIDs = [];

public static function addUserID($id){
    self::$userIDs[$id] = $id;
}

SOLL-Vorschlag:

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

php artisan make:job ProcessBusinessStructure

2. Job-Implementierung

<?php
// app/Jobs/ProcessBusinessStructure.php

namespace App\Jobs;

use App\Cron\BusinessUsersStore;
use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;
use Illuminate\Support\Facades\Log;

class ProcessBusinessStructure implements ShouldQueue
{
    use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;

    public $timeout = 300; // 5 Minuten pro Job
    public $tries = 3;     // 3 Versuche bei Fehlern

    private $month;
    private $year;
    private $step;

    public function __construct(int $month, int $year, string $step = 'structure')
    {
        $this->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

php artisan make:job ProcessUserBusinessDetails

<?php
// app/Jobs/ProcessUserBusinessDetails.php

namespace App\Jobs;

use App\User;
use App\Services\BusinessPlan\TreeCalcBot;
use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;

class ProcessUserBusinessDetails implements ShouldQueue
{
    use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;

    public $timeout = 120; // 2 Minuten pro User
    public $tries = 2;

    private $userId;
    private $month;
    private $year;

    public function __construct(int $userId, int $month, int $year)
    {
        $this->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
// app/Console/Commands/BusinessStore.php

namespace App\Console\Commands;

use App\Jobs\ProcessBusinessStructure;
use Illuminate\Console\Command;

class BusinessStore extends Command
{
    protected $signature = 'business:store {month?} {year?}';
    protected $description = 'Process business structure asynchronously via queues';

    public function handle()
    {
        $month = $this->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:

'connections' => [
    'database' => [
        'driver' => 'database',
        'table' => 'jobs',
        'queue' => 'default',
        'retry_after' => 600,
        'after_commit' => false,
    ],
],

Migration für Jobs-Tabelle:

php artisan queue:table
php artisan migrate

6. Queue Worker starten

# 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

// 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

# 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.

16 KiB Raw Blame History

Code Review: TreeCalcBot.php

Zusammenfassung der Analyse

Detaillierte Verbesserungsvorschläge

1. Performance-Optimierung: N+1 Problem beheben

2. Single Responsibility Principle: HTML-Generierung auslagern

3. Memory-Management: Große Datenmengen effizienter verarbeiten

4. Robuste Fehlerbehandlung implementieren

5. Dependency Injection für bessere Testbarkeit

6. Static Property für bessere Performance

Lösung für langlaufende Cron-Jobs (Asynchrone Verarbeitung)

Konzept

Schritt-für-Schritt-Anleitung

1. Job-Klasse erstellen

2. Job-Implementierung

3. User-spezifische Job-Klasse für Details

4. Artisan Command anpassen

5. Queue-Konfiguration

6. Queue Worker starten

7. Cron-Integration

Monitoring und Fehlerbehandlung

16 KiB

Raw Blame History