Sollten die beiden BBCode-Umformungsansätze von Discourse eine gemeinsame Kern-API teilen?

Entwickler
1

Zwei offene PRs führen unterschiedliche Ansätze zum Bearbeiten von BBCode-Attributen im rohen Postinhalt ein:

Während der Review von #43187 schlug Martin vor, die generische BBCode-Manipulationsfunktionalität vom Event-spezifischen Service zu trennen.

Ich hatte ursprünglich einige damit verbundene tokenbasierte Funktionalitäten aus der anderen PR übernommen, was jedoch zwei Umschreibungsmechanismen innerhalb von #43187 einführt hätte. Da die tokenbasierte API in diesem Branch keinen Produktionsaufruf hatte, habe ich sie in Commit 6d13371 entfernt.

Die tokenbasierte API bleibt unabhängig davon für #42932 nützlich.

Ich fragte mich, ob diese beiden Ansätze irgendwann zu einer wiederverwendbaren Kern-API zusammengeführt werden sollten oder ob ihre unterschiedlichen Anforderungen es rechtfertigen, beide beizubehalten.

Vergleich der aktuellen Implementierungen

Die Tabelle beschreibt, was der Code derzeit implementiert, statt anzudeuten, dass einer der Ansätze grundsätzlich falsch ist.

Funktion BbcodeRewriter (#43187) Tokenbasierte API (#42932) Bedeutung und Wichtigkeit
1. Verwendet Discourses BBCode-Parser Scannt Kandidaten-Eröffnungstags mit parseBBCodeTag, derselben Eröffnungstag-Syntax, die auch vom Markdown-Engine verwendet wird. Verwendet DiscourseMarkdownIt.parse, um Markdown-Tokens zu erhalten und einen passenden BBCode-Block zu finden. Beide Ansätze verstehen die BBCode-Syntax, anstatt sich ausschließlich auf einen unabhängigen regulären Ausdruck zu verlassen. Sie unterscheiden sich darin, wie sie den zu bearbeitenden Tag identifizieren.
2. Auswahl des Ziell-BBCode-Blocks Markiert Kandidaten-Eröffnungstags vorübergehend, rendert den Post und identifiziert den Kandidaten, der einem ausgewählten HTML-Element entspricht, z. B. div.discourse-post-event. Wählt das erste geparste BBCode-Token aus, dessen meta.bbcode dem angeforderten Tag-Namen entspricht. Die Identität des Ziels ist wichtig. #43187 muss den rohen BBCode mit dem Event verknüpfen, das Discourse tatsächlich gerendert hat. Der Workflow in #42932 verwendet den ersten passenden geparsten Event-Block.
3. Behandlung fehlender oder mehrdeutiger Ziele Löst TargetNotFound, AmbiguousTarget oder UnsupportedRenderer aus, wenn ein Ziel nicht zuverlässig identifiziert werden kann. Gibt null zurück, wenn kein passender geparster Block existiert; andernfalls wählt es das erste Match. Das Fehlverhalten unterscheidet sich. #43187 lehnt mehrdeutige automatische Bearbeitungen bewusst ab. Die Auswahl des ersten geparsten Blocks kann ausreichen, wenn der Aufrufer nur einen relevanten Block erwartet.
4. Verifizierung der Roh-zu-Rendern-Mapping Rendert markierten Inhalt, prüft den Marker am beabsichtigten HTML-Element und stellt sicher, dass Einfügen und Entfernen des Markers das gerenderte HTML unverändert lässt. Verwendet die Token-Metadaten des Markdown-Parsers, um den Eröffnungstag zu lokalisieren, ohne eine zusätzliche Identitätsprüfung des gerenderten HTML. Dies schützt vor dem Bearbeiten des falschen gerenderten Ziels. Dies ist besonders relevant, wenn ein Hintergrundjob ein bestehendes Event automatisch archiviert, bringt aber zusätzliches Rendering und Komplexität mit sich.
5. Prüfung des gewählten Eröffnungstags Scannt vor dem Umschreiben den übergebenen Rohinhalt erneut und prüft, ob der gewählte Kandidat weiterhin mit seinem erwarteten Eröffnungstext übereinstimmt. Lokalisiert den passenden Eröffnungstag während des Update-Aufrufs, ohne einen separaten Vergleich mit dem erwarteten Eröffnungstext. Dies ist ein interner Konsistenzcheck. Er hilft dabei, ein Umschreiben eines Kandidaten zu vermeiden, der nicht mehr der früheren Auswahl entspricht. Es ist an sich kein Schutz gegen gleichzeitige Datenbankbearbeitungen.
6. Handhabung von Attributnamen und bestehenden Werten Schreibt ausgewählte Attribute unter Verwendung der Attributgrenzen des Parsers um. Es normalisiert auch äquivalente gerenderte Attributnamen, sodass Varianten wie Recurrence und recurrence konsistent entfernt werden können. Verwendet Attribut-Token-Matches, um angeforderte Namen zu ersetzen oder zu entfernen, sowie den gemeinsamen BBCode-Attribut-Serialisierer, um Werte einzufügen. Beide zielen darauf ab, unverwandte Attribute zu erhalten. Die Normalisierung äquivalenter Namen ist besonders wichtig beim Archivieren eines Events, da das Zurücklassen eines Alias für die Wiederholung dazu führen könnte, dass das abgeschlossene Event wiederkehrend bleibt.
7. Validierung des umgeschriebenen Tags Parst den umgeschriebenen Eröffnungstag erneut und prüft, ob seine Attribute mit den beabsichtigten Werten übereinstimmen. Ein ungültiger Roundtrip löst einen Fehler aus. Erzeugt den umgeschriebenen Eröffnungstag direkt, ohne einen äquivalenten finalen Parse-und-Vergleichsschritt. Roundtrip-Validierung fängt Serialisierungsprobleme ab, wie geänderte Attributwerte oder fehlerhafte Grenzen. Der tokenbasierte Ansatz ist einfacher, verlässt sich aber auf seine Umschreiblogik und Tests anstelle dieser zusätzlichen Laufzeitprüfung.
8. Erhaltung unverwandter Postinhalte Ändert den gewählten Eröffnungstag, behält aber den umgebenden Rohinhalt bei. Es prüft auch, dass unverwandte Attributwerte das Umschreiben überstehen. Ersetzt den Unterstring des gewählten Eröffnungstags, behält aber den umgebenden Rohinhalt und Attribute, die nicht zur Änderung angefordert wurden, bei. Beide müssen unbeabsichtigte Poständerungen vermeiden. Eine Event-bezogene Bearbeitung sollte die umgebende Diskussion oder unverwandte BBCode-Attribute nicht stillschweigend ändern.
9. Implementierungskosten Erfordert Kandidaten-Scanning, temporäre Marker, Post-Rendering, HTML-Inspektion, Attributumschreibung und Validierung. Parst Markdown-Tokens, wählt den ersten passenden Eröffnungstag und schreibt ihn direkt um. Stärkere Laufzeitprüfungen haben ihren Preis. Die Frage ist, ob die zusätzlichen Schutzmaßnahmen allen Aufrufern zur Verfügung stehen sollten, optional sein sollten oder nur dort implementiert werden sollen, wo die Anwendung sie benötigt.

Warum sich die Anforderungen unterscheiden

Für #43187 läuft das Nachfolger-Rollover automatisch ab, wenn ein Vorkommen abgeschlossen ist. Das ursprüngliche Event wird als nicht wiederkehrendes Vorkommen archiviert, und ein neues Topic wird für das nächste Vorkommen erstellt.

Wenn der falsche [event]-Block geändert wird oder die umgeschriebenen Attribute das beabsichtigte Event nicht mehr darstellen, könnte der Fehler über mehrere automatische Rollovers hinweg bestehen bleiben.

Für #42932 fordert die Workflow-Aktion explizit an, dass ein Event geöffnet oder geschlossen wird. Seine aktuelle tokenbasierte Implementierung ändert den ersten passenden geparsten Event-Block.

Der tokenbasierte Ansatz ist daher für diesen Anwendungsfall nicht unbedingt ungeeignet. Er bietet lediglich andere Garantien hinsichtlich Auswahl und Validierung.

Beide Ansätze teilen bereits einige zugrunde liegende Infrastrukturen, einschließlich parseBBCodeTag und serializeBBCodeAttr.

Mögliche Richtung für eine gemeinsame Abstraktion

Eine Möglichkeit bestünde darin, drei Verantwortlichkeiten zu trennen:

  1. Finden eines BBCode-Eröffnungstags — unter Verwendung von Parser-Tokens, expliziter Kandidatenauswahl oder einer gerenderten Elementzuordnung.

  2. Umschreiben seiner Attribute — unter Verwendung einer gemeinsamen Implementierung für Attributgrenzen, Serialisierung und Erhaltung unverwandter Werte.

  3. Validierung des Ergebnisses — optionales Erzwingen der Zielidentität, Erkennung von Mehrdeutigkeiten, Äquivalenz des gerenderten Inhalts oder Attribut-Roundtrip-Prüfungen, je nach Aufrufer.

Das könnte es einfacheren Workflows ermöglichen, eine unkomplizierte Tokenauswahl zu verwenden, während empfindlichere automatische Bearbeitungsvorgänge stärkere Schutzmaßnahmen anfordern.

Es könnte jedoch auch unnötige Komplexität im Vergleich zur Wartung zweier kleinerer APIs einführen.

Frage an die Maintainer

Wäre es vorzuziehen, dass die beiden PRs ihre separaten Umschreibungs-APIs beibehalten, angesichts ihrer unterschiedlichen Anforderungen an die Zielauswahl?

Oder wäre ein gemeinsamer tiefer gelegener Attribut-Umschreiber mit konfigurierbarer Zielauswahl und Validierung eine bessere langfristige Richtung?

Mich interessiert insbesondere, ob die zusätzlichen Sicherheitsprüfungen, die derzeit von BbcodeRewriter implementiert werden, als allgemein nützliche Kernfunktionalität betrachtet werden oder ob sie spezifisch für Anwendungsfälle bleiben sollten, die stärkere Garantien benötigen.

Ich schlage nicht vor, dass eine der PRs die andere blockieren muss; das Ziel besteht darin, unnötige Duplikate in Discourses Kern-BBCode-Infrastruktur zu vermeiden, während sich beide Funktionen weiterentwickeln.

1 „Gefällt mir“