557 lines
No EOL
16 KiB
Markdown
557 lines
No EOL
16 KiB
Markdown
# 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 = '<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:**
|
|
```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
|
|
<?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
|
|
|
|
```bash
|
|
php artisan make:job ProcessUserBusinessDetails
|
|
```
|
|
|
|
```php
|
|
<?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
|
|
<?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:**
|
|
```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. |