Files
michaelschiemer/docs/refactoring/discovery-registry-loading-analysis.md
2025-11-24 21:28:25 +01:00

318 lines
12 KiB
Markdown

# Discovery Registry Loading - Problem-Analyse und Refactoring-Vorschläge
## Problem-Identifikation
### Aktuelles Problem
Die `DiscoveryRegistry` wird über **zwei verschiedene Mechanismen** geladen, was zu Inkonsistenzen führt:
1. **`DiscoveryServiceBootstrapper::bootstrap()`** (Runtime)
- Wird in `ContainerBootstrapper::autowire()` aufgerufen
- Verwendet `DiscoveryCacheManager` mit `isStale()` Prüfung
- Registriert `DiscoveryRegistry` als Singleton im Container
- Führt Discovery durch, wenn Cache stale ist oder nicht existiert
2. **`DiscoveryRegistryInitializer`** (Build-Time)
- Ist ein `#[Initializer(ContextType::ALL)]`
- Lädt aus `storage/discovery/` Dateien (Build-Time Storage)
- Wird möglicherweise vor/nach `DiscoveryServiceBootstrapper` ausgeführt
- Kann eine alte Registry laden, die dann als Singleton registriert wird
### Konflikt-Szenario
**Szenario 1: DiscoveryRegistryInitializer läuft zuerst**
```
1. DiscoveryRegistryInitializer läuft (Initializer-System)
2. Lädt alte Registry aus storage/discovery/ (ohne DatabaseAssetGalleryDataProvider)
3. Registriert als Singleton im Container
4. DiscoveryServiceBootstrapper läuft
5. Prüft Cache -> findet alte Registry (nicht stale, weil Datei vor Cache-Erstellung geändert wurde)
6. Verwendet alte Registry -> Provider fehlt
```
**Szenario 2: DiscoveryServiceBootstrapper läuft zuerst**
```
1. DiscoveryServiceBootstrapper läuft (autowire)
2. Führt Discovery durch -> findet DatabaseAssetGalleryDataProvider
3. Registriert als Singleton
4. DiscoveryRegistryInitializer läuft später
5. Lädt alte Registry aus storage/discovery/
6. Überschreibt Singleton? -> Provider geht verloren
```
## Root Cause Analysis
### Problem 1: Zwei verschiedene Cache-Mechanismen
- **Runtime-Cache**: `DiscoveryCacheManager` mit `isStale()` Prüfung
- **Build-Time Storage**: `storage/discovery/` Dateien (veraltet?)
### Problem 2: Unklare Ausführungsreihenfolge
- `DiscoveryServiceBootstrapper` läuft in `autowire()` (explizit)
- `DiscoveryRegistryInitializer` läuft über Initializer-System (automatisch)
- Keine garantierte Reihenfolge
### Problem 3: isStale() Prüfung funktioniert nicht korrekt
- **BEHOBEN**: Verwendet jetzt Cache-Erstellungszeit statt Request-Zeit
- Aber: Wenn `DiscoveryRegistryInitializer` eine alte Registry lädt, wird diese nicht als stale erkannt
## Refactoring-Vorschläge
### Option 1: DiscoveryRegistryInitializer entfernen (Empfohlen)
**Vorteile:**
- Eliminiert Konflikt zwischen zwei Mechanismen
- Einheitliche Cache-Strategie über `DiscoveryCacheManager`
- `isStale()` Prüfung funktioniert korrekt
**Nachteile:**
- Build-Time Storage wird nicht mehr verwendet
- Möglicherweise Performance-Verlust beim ersten Request
**Implementierung:**
1. `DiscoveryRegistryInitializer` entfernen oder deaktivieren
2. Sicherstellen, dass `DiscoveryServiceBootstrapper` immer läuft
3. `DiscoveryCacheManager` als einzige Quelle für Discovery-Registry
### Option 2: DiscoveryRegistryInitializer als Fallback
**Vorteile:**
- Behält Build-Time Storage für Performance
- Fallback wenn Runtime-Cache nicht verfügbar
**Nachteile:**
- Komplexere Logik
- Mögliche Inkonsistenzen zwischen Build-Time und Runtime
**Implementierung:**
1. `DiscoveryServiceBootstrapper` prüft zuerst Runtime-Cache
2. Wenn nicht verfügbar/stale, prüft `DiscoveryRegistryInitializer`
3. Nur wenn beide fehlschlagen, führt Discovery durch
### Option 3: Einheitliche Cache-Strategie
**Vorteile:**
- Einheitliche Quelle für Discovery-Registry
- Klare Verantwortlichkeiten
**Nachteile:**
- Größere Refactoring-Arbeit
**Implementierung:**
1. `DiscoveryCacheManager` verwendet `storage/discovery/` als Backend
2. `DiscoveryRegistryInitializer` entfernen
3. `DiscoveryServiceBootstrapper` verwendet nur `DiscoveryCacheManager`
### Option 4: Priority-basierte Ausführungsreihenfolge
**Vorteile:**
- Beide Mechanismen bleiben erhalten
- Klare Ausführungsreihenfolge
**Nachteile:**
- Komplexere Initializer-Logik
- Mögliche Race-Conditions
**Implementierung:**
1. `DiscoveryServiceBootstrapper` mit höherer Priority
2. `DiscoveryRegistryInitializer` prüft ob Registry bereits existiert
3. Nur laden wenn nicht vorhanden
## Empfohlene Lösung: Option 1 + Verbesserungen
### Schritt 1: DiscoveryRegistryInitializer deaktivieren/entfernen
```php
// DiscoveryRegistryInitializer.php
#[Initializer(ContextType::ALL)]
public function __invoke(): DiscoveryRegistry
{
// DEPRECATED: This initializer is deprecated in favor of DiscoveryServiceBootstrapper
// Check if DiscoveryRegistry already exists (from DiscoveryServiceBootstrapper)
if ($this->container->has(DiscoveryRegistry::class)) {
return $this->container->get(DiscoveryRegistry::class);
}
// Fallback: Load from storage (for backward compatibility)
$attributes = $this->discoveryLoader->loadAttributes() ?? new AttributeRegistry();
$interfaces = $this->discoveryLoader->loadInterfaces() ?? new InterfaceRegistry();
$templates = $this->discoveryLoader->loadTemplates() ?? new TemplateRegistry();
return new DiscoveryRegistry(
attributes: $attributes,
interfaces: $interfaces,
templates: $templates
);
}
```
### Schritt 2: DiscoveryServiceBootstrapper als einzige Quelle
```php
// DiscoveryServiceBootstrapper.php
public function bootstrap(): DiscoveryRegistry
{
// Prüfe ob bereits registriert (z.B. durch DiscoveryRegistryInitializer)
if ($this->container->has(DiscoveryRegistry::class)) {
$existing = $this->container->get(DiscoveryRegistry::class);
// Prüfe ob stale
if ($this->isRegistryStale($existing)) {
// Force refresh
$this->container->forget(DiscoveryRegistry::class);
} else {
return $existing;
}
}
// ... rest of bootstrap logic
}
```
### Schritt 3: Verbesserte isStale() Prüfung
```php
// DiscoveryCacheManager.php
private function isStale(DiscoveryContext $context, DiscoveryRegistry $registry, ?\DateTimeInterface $cacheStartTime = null): bool
{
// ... existing logic
// ZUSÄTZLICH: Prüfe ob Dateien seit Cache-Erstellung geändert wurden
// Dies ist bereits implementiert, aber sollte auch für Build-Time Storage gelten
}
```
## Weitere Verbesserungen
### 1. Discovery-Registry Versionierung
- Jede Registry hat eine Version/Timestamp
- Vergleich zwischen Build-Time und Runtime-Cache
- Automatische Invalidation bei Versionsunterschieden
### 2. Unified Discovery Storage
- Einheitliche Storage-Strategie für Build-Time und Runtime
- `DiscoveryCacheManager` verwendet `storage/discovery/` als Backend
- Konsistente Cache-Invalidation
### 3. Discovery-Registry Factory
- Zentrale Factory für Discovery-Registry-Erstellung
- Einheitliche Logik für alle Quellen
- Klare Verantwortlichkeiten
### 4. Logging und Monitoring
- Besseres Logging für Discovery-Registry-Loading
- Metriken für Cache-Hits/Misses
- Warnungen bei Inkonsistenzen
## Migration-Plan
1. **Phase 1**: `DiscoveryRegistryInitializer` als Fallback behalten, aber prüfen ob Registry bereits existiert
2. **Phase 2**: `DiscoveryServiceBootstrapper` prüft ob Registry stale ist, auch wenn bereits registriert
3. **Phase 3**: `DiscoveryRegistryInitializer` vollständig entfernen
4. **Phase 4**: Build-Time Storage optional machen (nur für Performance-Optimierung)
## Testing-Strategie
1. Unit-Tests für `DiscoveryCacheManager::isStale()`
2. Integration-Tests für Discovery-Registry-Loading
3. E2E-Tests für Provider-Resolution
4. Performance-Tests für Cache-Hit-Rate
## Implementierte Änderungen
### 1. isStale() Prüfung korrigiert ✅
- **Datei**: `src/Framework/Discovery/Storage/DiscoveryCacheManager.php`
- **Änderung**: `isStale()` verwendet jetzt Cache-Erstellungszeit statt Request-Zeit
- **Implementierung**: `startTime` wird beim Speichern im Cache gespeichert und beim Laden verwendet
### 2. DiscoveryRegistryInitializer entfernt ✅
- **Datei**: `src/Framework/Discovery/DiscoveryRegistryInitializer.php` (entfernt)
- **Änderung**: Initializer wurde vollständig entfernt, da er nur noch als No-Op fungierte
- **Implementierung**: `DiscoveryServiceBootstrapper` ist jetzt die einzige Quelle für Discovery-Registry
### 3. DiscoveryServiceBootstrapper verbessert ✅
- **Datei**: `src/Framework/Discovery/DiscoveryServiceBootstrapper.php`
- **Änderung**: Prüft ob existierende Registry stale ist, bevor Discovery durchgeführt wird
- **Implementierung**: `isRegistryStale()` Methode hinzugefügt
### 4. Cache-Struktur Migration ✅
- **Datei**: `src/Framework/Discovery/Storage/DiscoveryCacheManager.php`
- **Änderung**: Automatische Migration von altem Cache-Format zu neuem Format mit `startTime` und `version`
- **Implementierung**: `upgradeCacheEntry()` und `validateCacheStructure()` Methoden hinzugefügt
### 5. Registry Versionierung ✅
- **Datei**: `src/Framework/Discovery/Storage/DiscoveryCacheManager.php`
- **Änderung**: Cache-Struktur erweitert um `version` Feld für Registry-Versionierung
- **Implementierung**: `getRegistryVersion()` Methode erstellt Version basierend auf Registry-Content-Hash
### 6. Unified Discovery Storage ✅
- **Datei**: `src/Framework/Discovery/Storage/DiscoveryCacheManager.php`
- **Änderung**: Build-Time Storage als optionales Backend hinzugefügt
- **Implementierung**: `tryGetFromBuildTimeStorage()` Methode prüft Build-Time Storage vor Runtime-Cache
### 7. Verbessertes Logging und Monitoring ✅
- **Dateien**:
- `src/Framework/Discovery/Storage/DiscoveryCacheManager.php`
- `src/Framework/Discovery/Results/DiscoveryRegistry.php`
- **Änderung**: Strukturiertes Logging, Metriken-Sammlung, Debug-Helpers hinzugefügt
- **Implementierung**:
- `getMetadata()` Methode in `DiscoveryRegistry`
- `getSource()` Methode in `DiscoveryRegistry`
- Erweiterte Logging-Statements mit Source-Informationen
## Weitere Refactoring-Vorschläge
### Refactoring 1: Einheitliche Cache-Struktur
**Problem**: Cache-Struktur wurde geändert (Array mit `registry` und `startTime`), aber alte Caches haben noch das alte Format.
**Lösung**:
- Migration-Script für alte Cache-Einträge
- Oder: Automatische Erkennung und Konvertierung beim Laden
### Refactoring 2: DiscoveryRegistryInitializer entfernen
**Problem**: Zwei Mechanismen für Discovery-Registry-Loading führen zu Konflikten.
**Lösung**:
- `DiscoveryRegistryInitializer` vollständig entfernen
- Oder: Nur als Fallback verwenden (bereits implementiert)
- `DiscoveryServiceBootstrapper` als einzige Quelle
### Refactoring 3: Build-Time Storage optional machen
**Problem**: Build-Time Storage (`storage/discovery/`) wird möglicherweise nicht mehr verwendet.
**Lösung**:
- Build-Time Storage als Performance-Optimierung optional machen
- Nur verwenden wenn verfügbar und aktuell
- Runtime-Cache als primäre Quelle
### Refactoring 4: Discovery-Registry Versionierung
**Problem**: Keine Möglichkeit, verschiedene Versionen der Registry zu vergleichen.
**Lösung**:
- Jede Registry hat eine Version/Timestamp
- Vergleich zwischen Build-Time und Runtime-Cache
- Automatische Invalidation bei Versionsunterschieden
### Refactoring 5: Unified Discovery Storage
**Problem**: Zwei verschiedene Storage-Mechanismen (Build-Time und Runtime).
**Lösung**:
- Einheitliche Storage-Strategie
- `DiscoveryCacheManager` verwendet `storage/discovery/` als Backend
- Konsistente Cache-Invalidation
### Refactoring 6: Discovery-Registry Factory
**Problem**: Discovery-Registry wird an mehreren Stellen erstellt.
**Lösung**:
- Zentrale Factory für Discovery-Registry-Erstellung
- Einheitliche Logik für alle Quellen
- Klare Verantwortlichkeiten
### Refactoring 7: Verbessertes Logging
**Problem**: Schwierig zu debuggen, welche Registry verwendet wird.
**Lösung**:
- Besseres Logging für Discovery-Registry-Loading
- Metriken für Cache-Hits/Misses
- Warnungen bei Inkonsistenzen