Hi Masoud,

ich habe mal etwas gründlicher über den Code geschaut und fasse in diesem Issue ein paar Anmerkungen und Verbesserungsvorschläge zusammen. Wir können gern morgen zu allen Punkten ausführlich sprechen :)

REST-Controller

https://github.com/hedayati-m/Sitegeist.Nomenclator/blob/e7d868b556a53fe95b6683fc7560fb6c6182ae81/Classes/Application/Controller/GlossaryEntryController.php#L28

• Hier ist es besser JsonView::class zu referenzieren, anstatt den Klassennamen auszuschreiben

https://github.com/hedayati-m/Sitegeist.Nomenclator/blob/e7d868b556a53fe95b6683fc7560fb6c6182ae81/Classes/Application/Controller/GlossaryEntryController.php#L30

• Wenn eine Funktion oder Methode nichts zurück gibt, dann bitte nach Möglichkeit immer void als Return Type angeben

Glossary

Nach DDD-Begriffen ist die Klasse Glossary sowas wie Dein Repository. D.h. Methoden wie beforeGlossaryPublished, saveGlossaryInCache und readGlossaryIndexFromCache sind hier sehr gut aufgehoben.

Die Methoden extractGlossaryIndexFromNode und glossaryDuplicates hingegen gehören eher in eine Factory. Ich denke, dass das Domänenmodell von einer Entity GlossaryIndex profitieren würde, die quasi als AggregateRoot fungiert. GlossaryIndex könnte eine static factory method fromNode beinhalten, die die Funktion von extractGlossaryIndexFromNode übernimmt und zugleich den Integritätscheck über glossaryDuplicates durchführt.

GlossaryEntryFactory

Die GlossaryEntryFactory könnte durch eine static factory method auf der Klasse GlossaryEntry eingespart werden:

GlossaryEntry::fromNode($node);

// ...
final class GlossaryEntry
{
    // ...
    public static function fromNode(TraversableNodeInterface $node): self
    {
        // ...
    }
}

GlossaryEntry

https://github.com/hedayati-m/Sitegeist.Nomenclator/blob/e7d868b556a53fe95b6683fc7560fb6c6182ae81/Classes/Domain/GlossaryEntry.php#L11

• Value Objects sollten als final markiert werden

LinkTermsToGlossaryImplementation

https://github.com/hedayati-m/Sitegeist.Nomenclator/blob/e7d868b556a53fe95b6683fc7560fb6c6182ae81/Classes/Fusion/LinkTermsToGlossaryImplementation.php#L45-L51

Die FlowQuery Operation find ist relativ teuer. Lass uns hier morgen mal nach Möglichkeiten suchen, wie wir diesen Query-Algorithmus optimieren können, sodass der zweite find-Aufruf eingespart werden kann.

Darüber hinaus denke ich, dass das Auffinden dieser Nodes eine Aufgabe ist, die die Klasse Glossary als Repository übernehmen könnte.

https://github.com/hedayati-m/Sitegeist.Nomenclator/blob/e7d868b556a53fe95b6683fc7560fb6c6182ae81/Classes/Fusion/LinkTermsToGlossaryImplementation.php#L61

• ReturnType fehlt

https://github.com/hedayati-m/Sitegeist.Nomenclator/blob/e7d868b556a53fe95b6683fc7560fb6c6182ae81/Classes/Fusion/LinkTermsToGlossaryImplementation.php#L63

• Nullish Coalescing Fallback fehlt (also: return $this->fusionValue('value') ?? '';)

https://github.com/hedayati-m/Sitegeist.Nomenclator/blob/e7d868b556a53fe95b6683fc7560fb6c6182ae81/Classes/Fusion/LinkTermsToGlossaryImplementation.php#L69

• Das müsste @return null|TraversableNodeInterface sein - Context Values aus Fusion sind nicht null-safe

https://github.com/hedayati-m/Sitegeist.Nomenclator/blob/e7d868b556a53fe95b6683fc7560fb6c6182ae81/Classes/Fusion/LinkTermsToGlossaryImplementation.php#L71

• ReturnType fehlt

https://github.com/hedayati-m/Sitegeist.Nomenclator/blob/e7d868b556a53fe95b6683fc7560fb6c6182ae81/Classes/Fusion/LinkTermsToGlossaryImplementation.php#L100

Anstelle des custom HTML parsing Algorithmus, der in dieser Zeile anfängt, empfehle ich den Einsatz einer Third-Party Library. Hervorragend geeignet für dieses Problem ist z.B. diese hier: https://github.com/Masterminds/html5-php

Der Grund: Das Parsen von HTML ist nicht-trivial und es ist in der Tat unmöglich HTML korrekt mit Regular Expressions zu parsen (dieser Beitrag erklärt das Problem sehr gut: https://stackoverflow.com/a/6751339/15130211). Mit Regular Expressions kann das Problem nur sehr grob gelöst werden und zahlreiche Edge-Cases werden dann nicht beachtet. Was passiert z.B. wenn der Begriff bereits anderweitig verlinkt ist? Mit einem echten HTML-Parser können Probleme dieser Art ausgeschlossen werden.

Es würde sich außerdem lohnen, den Algorithmus in eine eigene Utility-Klasse auszulagern und mit reichlich Unit Tests zu versehen. Aber auch mit HTML-Parser wird der Algorithmus am Ende komplexer aussehen. Hier ist eine Skizze, wie ich mir das vorstellen könnte:

use Masterminds\HTML5;

// ...

class HTMLTermReplacer
{
    public static function replaceTerms(string $markup, array $terms, callable $onReplaceTerm): string
    {
        $html5 = new HTML5();
        $doc = $html5->loadHTML($markup);

        $xpath = new \DOMXPath($doc);
        // Namespace needs to be registered due to limitations in Masterminds\HTML5
        // see: https://github.com/Masterminds/html5-php/issues/57
        $xpath->registerNamespace('x', 'http://www.w3.org/1999/xhtml');

        // The XPath Query selects all text nodes that are not
        // descendants of links
        $nodes = $xpath->query('//text()[not(ancestor::x:a)]');
        foreach ($nodes as $node) {
            foreach ($terms as $term) {
                // Regex left out for simplicity's sake
                $matches = preg_split('/(...)/', $node->nodeValue, -1, PREG_SPLIT_DELIM_CAPTURE);

                if (count($matches) > 1) {
                    $fragment = $doc->createDocumentFragment();

                    foreach ($matches as $match) {
                        if ($match === $term) {
                            $link = $onReplaceNode($doc, $term);
                            $fragment->appendChild($link);
                        } else {
                            $text = $doc->createTextNode($match);
                            $fragment->appendChild($text);
                        }
                    }

                    $node->parentNode->replaceChild($fragment, $node);
                }
            }
        }

        return $html5->saveHTML($doc->childNodes[1]->childNodes);
    }
}

Lass uns gern morgen über die Umsetzung und Teststrategie sprechen :)