Symfony-Refactoring-Patterns: Eine laufende Codebasis verbessern, ohne die Feature-Arbeit zu stoppen

Die meisten Ratschläge zum Symfony-Refactoring setzen stillschweigend einen Luxus voraus, den niemand hat: einen ruhigen Sprint, einen Feature-Freeze und die Erlaubnis, Dinge kaputt zu machen, während man sie sortiert. Reale Projekte funktionieren so nicht. Die Codebasis, die die Arbeit braucht, ist die, die gerade zahlende Kunden bedient, und die Roadmap pausiert nicht, während Sie aufräumen.

In diesem Beitrag geht es um die andere Art von Refactoring. Kein Versions-Upgrade, keine Neuentwicklung, sondern strukturelle Verbesserung innerhalb einer stabilen Symfony-Version, angewandt auf Code, der weiter ausliefern muss. Fünf Patterns, jeweils mit dem Vorher, dem Nachher und dem Grund, warum die bestehende Form Probleme verursacht. Eine Bedingung zieht sich durch alle: Jeder Schritt muss die Anwendung deploybar lassen. Wenn ein Refactoring nicht an einem Dienstagnachmittag gemergt und released werden kann, ist es kein Refactoring, sondern ein Branch, der verrotten wird.

Pattern 1: Fat Controller in invokable Service-Klassen ziehen

Die häufigste Form in einer alternden Symfony-Anwendung ist eine Controller-Action, der ein Körper gewachsen ist. Sie lädt eine Entity, validiert ein paar Regeln, ruft ein Gateway auf, mutiert Zustand, flusht, verschickt eine E-Mail und leitet weiter.

#[Route('/orders/{id}/refund', methods: ['POST'])]
public function refund(
    int $id,
    Request $request,
    EntityManagerInterface $em,
    MailerInterface $mailer,
    PaymentGateway $gateway,
): Response {
    $order = $em->getRepository(Order::class)->find($id);
    if (!$order) {
        throw $this->createNotFoundException();
    }
    if ($order->getStatus() !== 'paid') {
        $this->addFlash('error', 'Only paid orders can be refunded.');

        return $this->redirectToRoute('order_show', ['id' => $id]);
    }

    $amount = (int) $request->request->get('amount');
    if ($amount <= 0 || $amount > $order->getTotal()) {
        $this->addFlash('error', 'Invalid refund amount.');

        return $this->redirectToRoute('order_show', ['id' => $id]);
    }

    $gateway->refund($order->getPaymentReference(), $amount);
    $order->setStatus($amount === $order->getTotal() ? 'refunded' : 'partially_refunded');
    $em->flush();
    $mailer->send($this->buildRefundMail($order, $amount));

    return $this->redirectToRoute('order_show', ['id' => $id]);
}

Das Problem ist nicht ästhetischer Natur. Es liegt darin, dass die Erstattungsregeln jetzt nur noch über einen HTTP-Request erreichbar sind. Sie zu testen bedeutet, den Kernel zu booten, einen Request zu bauen und eine Session bereitzustellen, damit addFlash nicht explodiert. Wenn dieselbe Erstattung aus einem Konsolenkommando oder einem Webhook des Zahlungsanbieters heraus passieren muss, entwirrt das niemand; es wird kopiert, und nun leben die Regeln an zwei Stellen, die auseinanderdriften.

Die Extraktion ist mechanisch. Verschieben Sie die Entscheidungslogik in einen invokable Service und lassen Sie ihn Fehler über Exceptions statt über Flash-Messages signalisieren.

final class RefundOrder
{
    public function __construct(
        private OrderRepository $orders,
        private PaymentGateway $gateway,
        private EntityManagerInterface $em,
    ) {
    }

    public function __invoke(int $orderId, int $amount): Order
    {
        $order = $this->orders->find($orderId) ?? throw new OrderNotFound($orderId);

        if (!$order->isRefundable()) {
            throw new OrderNotRefundable($orderId, $order->getStatus());
        }
        if ($amount <= 0 || $amount > $order->getTotal()) {
            throw new InvalidRefundAmount($amount);
        }

        $this->gateway->refund($order->getPaymentReference(), $amount);
        $order->recordRefund($amount);
        $this->em->flush();

        return $order;
    }
}

Der Controller behält nur, was wirklich HTTP ist: Eingaben lesen, Domain-Exceptions fangen und sie in Flashes und Redirects verwandeln. Der Service ist jetzt mit drei Mocks und ohne Kernel unit-testbar, was die Laufzeit dieses Tests üblicherweise von einer Sekunde auf eine Millisekunde senkt.

Machen Sie das eine Action nach der anderen. Jede andere Route funktioniert exakt wie zuvor, also ist die Änderung releasebar, sobald sie grün ist.

Pattern 2: Parametersuppe durch Value Objects ersetzen

Lange positionale Signaturen sind eine verlässliche Quelle von Produktionsvorfällen, denn der Compiler lässt Sie zwei Argumente desselben skalaren Typs klaglos vertauschen.

public function createInvoice(
    int $customerId,
    string $currency,
    int $netAmount,
    int $taxRate,
    ?string $vatId,
    bool $reverseCharge,
    \DateTimeImmutable $issuedAt,
): Invoice {

Jede Aufrufstelle wiederholt dieselbe Validierung, oder lässt sie weg. Die Regel, dass eine Reverse-Charge-Rechnung eine USt-IdNr. braucht, lebt nirgendwo im Besonderen, was heißt: Sie lebt in dem Aufrufer, der zufällig daran gedacht hat.

Drücken Sie die Invarianten in Typen, die sich nicht in einem ungültigen Zustand konstruieren lassen.

final readonly class Money
{
    public function __construct(
        public int $amount,
        public string $currency,
    ) {
        if ($amount < 0) {
            throw new \InvalidArgumentException('Money cannot be negative.');
        }
        if (!preg_match('/^[A-Z]{3}$/', $currency)) {
            throw new \InvalidArgumentException("Invalid currency: {$currency}");
        }
    }
}

final readonly class TaxTreatment
{
    public function __construct(
        public int $ratePercent,
        public ?string $vatId,
        public bool $reverseCharge,
    ) {
        if ($reverseCharge && null === $vatId) {
            throw new \InvalidArgumentException('Reverse charge requires a VAT ID.');
        }
    }
}

Die Signatur schrumpft auf createInvoice(CustomerId $customer, Money $net, TaxTreatment $tax, \DateTimeImmutable $issuedAt), und eine ganze Klasse von Bugs wird undarstellbar.

Was das auf einer laufenden Codebasis sicher macht, ist der Migrationspfad. Aktualisieren Sie nicht vierzig Aufrufstellen in einem Commit. Behalten Sie die alte Signatur als dünnen, als deprecated markierten Wrapper, der die Value Objects baut und delegiert.

/**
 * @deprecated Use createInvoice() with value objects instead.
 */
public function createInvoiceLegacy(int $customerId, string $currency, int $netAmount, /* ... */): Invoice
{
    return $this->createInvoice(
        new CustomerId($customerId),
        new Money($netAmount, $currency),
        new TaxTreatment($taxRate, $vatId, $reverseCharge),
        $issuedAt,
    );
}

Neuer Code nutzt die neue Signatur sofort. Alte Aufrufstellen wandern in den Commits mit, die sie ohnehin anfassen. Wenn ein Grep nach createInvoiceLegacy leer zurückkommt, löschen Sie den Wrapper. Drei separate Releases, keines davon riskant.

Pattern 3: Geschäftslogik aus Doctrine-Entities herausziehen

Entities, die nach Services greifen, sind der Grund, warum eine Testsuite elf Minuten dauert.

class Subscription
{
    public function calculateRenewalPrice(
        PriceListRepository $prices,
        DiscountService $discounts,
    ): int {
        $base = $prices->findForPlan($this->plan)->getAmount();

        return $discounts->apply($base, $this->customer->getTier());
    }
}

Eine Entity, der man ein Repository übergeben muss, damit sie eine Frage über sich selbst beantworten kann, ist keine echte Entity-Methode, sondern ein Service mit einer unbequemen Aufrufkonvention. Jeder Test, der die Preisberechnung anfasst, braucht jetzt die Persistenzschicht, und die Entity lässt sich in einem Test nicht konstruieren, ohne den halben Container mitzuschleppen.

Trennen Sie, indem Sie fragen, was das Objekt aus seinem eigenen Zustand beantworten kann. isRefundable(), recordRefund() und isWithinTrial() bleiben: Sie lesen und mutieren Felder, die der Entity bereits gehören. Alles, was auf andere Aggregate schauen muss, zieht aus.

final class RenewalPricer
{
    public function __construct(
        private PriceListRepository $prices,
        private DiscountCalculator $discounts,
    ) {
    }

    public function priceFor(Subscription $subscription): Money
    {
        $base = $this->prices->findForPlan($subscription->getPlan())->amount();

        return $this->discounts->apply($base, $subscription->getCustomerTier());
    }
}

Die bestehenden Tests sind hier das Hindernis, und es gibt einen konkreten Trick, um sie nicht zu brechen. Schreiben Sie vor der Extraktion einen Charakterisierungstest, der das aktuelle Ergebnis für eine Bandbreite von Eingaben festnagelt, einschließlich der hässlichen Randfälle, die niemand dokumentiert hat. Dann extrahieren Sie. Dann lassen Sie die alte Entity-Methode stehen und an den neuen Service delegieren, sodass jeder bestehende Aufrufer und jeder bestehende Test grün bleibt.

/**
 * @deprecated Use RenewalPricer::priceFor().
 */
public function calculateRenewalPrice(PriceListRepository $prices, DiscountService $discounts): int
{
    return (new RenewalPricer($prices, $discounts))->priceFor($this)->amount;
}

Die Delegation ist temporär und leicht hässlich, und das ist in Ordnung. Sie erkauft Ihnen einen grünen Build bei jedem Commit, und genau darum geht es. Diese Art chirurgischer Trennung ist der Kern der meisten Legacy-Code-Optimierung, die wir machen, und der Charakterisierungstest ist fast immer das Erste, was geschrieben wird.

Pattern 4: Command und Handler schrittweise mit Messenger einführen

Teams behandeln den Wechsel zu einem Command Bus üblicherweise als Architekturentscheidung, die ein großes Meeting erfordert. Das muss nicht sein. Symfony Messenger dispatcht standardmäßig synchron, also ist die Einführung einer Message und eines Handlers eine rein strukturelle Änderung mit identischem Laufzeitverhalten. Sie entscheiden später, pro Message-Klasse, ob sie asynchron wird.

Beginnen Sie mit den Operationen, die langsam sind oder die Sie wiederholen können wollen: E-Mails, Exporte, Aufrufe von Drittanbieter-APIs.

final readonly class SendRefundNotification
{
    public function __construct(
        public int $orderId,
        public int $amount,
    ) {
    }
}

#[AsMessageHandler]
final class SendRefundNotificationHandler
{
    public function __construct(
        private OrderRepository $orders,
        private MailerInterface $mailer,
    ) {
    }

    public function __invoke(SendRefundNotification $message): void
    {
        $order = $this->orders->find($message->orderId);
        if (null === $order) {
            return;
        }

        $this->mailer->send(RefundMail::for($order, $message->amount));
    }
}

Mergen Sie es synchron laufend. Das Verhalten ist unverändert, das Deployment ist langweilig, und der Handler ist jetzt unabhängig testbar. Wenn Sie bereit sind, nimmt eine Zeile Konfiguration ihn vom Request-Pfad:

framework:
    messenger:
        transports:
            async:
                dsn: '%env(MESSENGER_TRANSPORT_DSN)%'
                retry_strategy:
                    max_retries: 3
                    delay: 1000
                    multiplier: 2
        routing:
            App\Message\SendRefundNotification: async

Zwei Dinge müssen stimmen, bevor Sie diesen Schalter umlegen. Messages müssen Identifikatoren transportieren statt Entity-Objekte, denn eine detachte Entity überlebt die Serialisierung nicht. Und Handler müssen idempotent sein, denn ein Retry bedeutet, dass der Handler zweimal auf derselben Message läuft. Wenn der zweite Lauf eine zweite E-Mail verschicken oder ein zweites Mal erstatten würde, bauen Sie einen Guard mit einem stabilen Schlüssel ein, bevor Sie sie in eine Queue routen.

Pattern 5: Hartkodierte Abhängigkeiten durch getaggte Interfaces ersetzen

Ein Service, der jede Implementierung beim Namen kennt, muss jedes Mal editiert werden, wenn eine neue erscheint.

public function export(string $format, Report $report): string
{
    return match ($format) {
        'csv' => $this->csvExporter->export($report),
        'xlsx' => $this->xlsxExporter->export($report),
        'pdf' => $this->pdfExporter->export($report),
        default => throw new \InvalidArgumentException($format),
    };
}

Ein neues Format anfassen heißt: Konstruktor, Match, Service-Definition und jedes Test-Double, das den Konstruktor bedienen muss. Letzteres ist der Grund, warum niemand das Format hinzufügen will.

Definieren Sie den Vertrag, lassen Sie die Autokonfiguration die Implementierungen einsammeln und injizieren Sie sie als Iterator.

interface ReportExporter
{
    public function supports(string $format): bool;

    public function export(Report $report): string;
}

final class ReportExporterRegistry
{
    /**
     * @param iterable<ReportExporter> $exporters
     */
    public function __construct(
        #[AutowireIterator('app.report_exporter')]
        private iterable $exporters,
    ) {
    }

    public function get(string $format): ReportExporter
    {
        foreach ($this->exporters as $exporter) {
            if ($exporter->supports($format)) {
                return $exporter;
            }
        }

        throw new UnsupportedExportFormat($format);
    }
}

services:
    _instanceof:
        App\Export\ReportExporter:
            tags: ['app.report_exporter']

Der inkrementelle Weg: Lassen Sie zuerst die drei bestehenden Exporter das Interface implementieren, was nichts ändert, weil das alte match sie weiterhin direkt aufruft. Dann fügen Sie die Registry hinzu. Dann stellen Sie den einzigen Aufrufer um. Dann löschen Sie das match. Vier kleine Commits, jeder unabhängig auslieferbar, und ein neues Exportformat ist danach eine neue Klasse und null Änderungen anderswo.

Die Disziplin, die Symfony-Refactoring sicher macht

Die Patterns zählen weniger als die Regeln, die Sie beim Anwenden befolgen. Vier haben sich ihren Platz in jedem Engagement verdient:

Strukturelle und verhaltensändernde Commits trennen. Ein Commit verschiebt entweder Code oder ändert, was er tut, niemals beides. Wenn in Produktion etwas bricht, ist der Unterschied zwischen dem Review einer reinen Verschiebung und dem Review einer Verschiebung plus Logikanpassung der Unterschied zwischen einer Fünf-Minuten-Diagnose und einer Stunde davon.

Charakterisieren, bevor Sie extrahieren. Ungetesteter Code ist kein Grund, das Refactoring zu überspringen, sondern ein Grund, zuerst den Test zu schreiben, der das aktuelle Verhalten festnagelt, Bugs eingeschlossen. Sie konservieren Verhalten, Sie segnen es nicht ab. Beheben Sie den Bug in einem späteren, klar beschrifteten Commit.

Deprecaten, migrieren, löschen, als drei Releases. Nichts wird im selben Release entfernt, das es ersetzt. Das ist es, was Ihnen erlaubt, mitten in der Migration anzuhalten, ohne die Codebasis kaputt zu hinterlassen, und das zählt, denn Sie werden mitten in der Migration auf etwas Dringendes gezogen werden.

Mit statischer Analyse ratschen. Erzeugen Sie eine PHPStan-Baseline, committen Sie sie und lassen Sie die CI fehlschlagen, wenn sie wächst. Die bestehenden Schulden bleiben anerkannt statt auf einen Schlag behoben, und neue Schulden können nicht hinzukommen. Über ein paar Monate schrumpft die Baseline von selbst, während Leute Dateien anfassen. Ein Code-Quality-Review ist oft genau das: herauszufinden, wo die Ratsche gesetzt werden sollte und was zuerst zu beheben ist.

Wo Sie anfangen sollten

Nehmen Sie die Datei, über die sich Ihr Team am meisten beschwert, nicht die, die bei einer Metrik am schlechtesten abschneidet. Beschwerdehäufigkeit ist ein besserer Näherungswert für Kosten als zyklomatische Komplexität, denn sie sagt Ihnen, wo der Code die Leute tatsächlich ausbremst. Wenden Sie das kleinste passende Pattern an, liefern Sie es aus und schauen Sie, ob die nächste Änderung in diesem Bereich leichter fällt. Wenn nicht, haben Sie an der falschen Naht geschnitten, und Sie haben einen Nachmittag verloren statt eines Quartals.

Refactoring in dieser Granularität ist absichtlich undramatisch. Es gibt keinen Rewrite-Branch, kein Migrationswochenende und kein Foliendeck, das um einen Feature-Freeze bittet. Es gibt nur eine Codebasis, in der sich jede Woche etwas leichter arbeiten lässt, während die Roadmap weiterläuft.

Wenn Sie auf eine Symfony-Codebasis schauen, deren Änderungen teuer geworden sind, und eine zweite Meinung wollen, welche Nähte zuerst zu schneiden sind, ist das die Art von Einschätzung, die wir bei Wolf-Tech machen. Schreiben Sie an hello@wolf-tech.io oder schauen Sie auf wolf-tech.io vorbei, und wir sagen Ihnen, wo der Hebel liegt.