refactor: improve error handling and type consistency in List and Reader controllers

Author: ericges

.audit/260719012-combined/10-architektur.md

@@ -61,46 +61,60 @@ Doku-Drift gegen den aktuellen Code: `ListBuilderFactory` heißt `ListSpecBuilde
 >
 > **Nutzer-Antwort: Angeglichen -- method: __METHOD__, source, wenn verfügbar: table.id -- übertragen auf gesamte Codebase**
 
-## A-09: `symfony/event-dispatcher` nicht direkt deklariert — Minor (claude, reduzierter Umfang)
-
-`FilterFormFactory` instanziiert direkt `new EventDispatcher()` (`src/Filter/Factory/FilterFormFactory.php:17,70`), deklariert ist aber nur `symfony/event-dispatcher-contracts` (`composer.json:17`); das konkrete Paket kommt nur transitiv über `contao/core-bundle`.
-
-## A-10: `ValidationLoader::executeQuery()` liefert `[]` statt `null` bei abgebrochenem Query-Aufbau — Minor (claude)
-
-Bei `!$qb` wird `[]` zurückgegeben — harmlos (falsy), aber semantisch schief gegenüber dem `?array`-Vertrag, in dem `null` „nicht gefunden" bedeutet (`:117`: `return $entry ?: null;`).
-
-- `src/Engine/Loader/ValidationLoader.php:107-109`
-
-## A-11: Query-Assemblierung lebt in Event-Listener-Prioritäten ohne zentrale Übersicht — Info (claude)
-
-Select@490, Conditions@470, Page@430, Order@420, Join@-450; Integrations-Listener dazwischen (250/220/200/190/100). Die Gesamtordnung ist nirgends zentral dokumentiert (kein Pipeline-Kommentar im `ListQueryDirector`).
-
-- `src/EventListener/QueryStructModifier/SelectModifierListener.php:13`, `ConditionsModifierListener.php:11`, `PageModifierListener.php:11`, `OrderModifierListener.php:12`, `JoinModifierListener.php:10` · `src/Integration/ContaoCalendar/EventListener/CountEventsModifierListener.php:14` u. a.
-
-## A-12: `ViewInterface` ist leerer Marker; Aufrufer müssen downcasten — Info (claude)
-
-Das Interface ist leer (`src/Engine/View/ViewInterface.php:7-9`); `ReaderController` downcastet auf `ValidationView` (`src/Controller/ContentElement/ReaderController.php:127`). Die `@template`-Annotationen sind nur mit dem `generics.noParent`-Ignore in PHPStan haltbar.
-
-## A-13: `#[TaggedIterator]` ist seit Symfony 7.1 deprecated — Info (claude)
-
-Genutzt in drei Registries; relevant für Deprecation-Logs bei Support-Matrix ^5.4|^6|^7. Nachfolger `AutowireIterator` existiert erst ab 6.3 → für die Matrix ggf. `!tagged_iterator` in YAML.
-
-- `src/Registry/EngineModRegistry.php:15` · `src/Registry/ProjectorRegistry.php:19` · `src/Registry/FilterTypeRegistry.php:18`
-
-## A-14: Statische Contao-Aufrufe in Context-DTOs — Info (claude, reduzierter Umfang)
-
-`PageModel::findByPk` in wertartigen Context-Objekten — DB-Zugriffe, testfeindlich, aber Contao-idiomatisch.
+> ## A-09: `symfony/event-dispatcher` nicht direkt deklariert — Minor (claude, reduzierter Umfang)
+>
+> `FilterFormFactory` instanziiert direkt `new EventDispatcher()` (`src/Filter/Factory/FilterFormFactory.php:17,70`), deklariert ist aber nur `symfony/event-dispatcher-contracts` (`composer.json:17`); das konkrete Paket kommt nur transitiv über `contao/core-bundle`.
+> 
+> **Nutzer-Antwort: Required in composer.json**
 
-- `src/Engine/Context/ReaderUrlConfigCreatorTrait.php:18` · `src/Engine/Context/ValidationContext.php:44`
+> ## A-10: `ValidationLoader::executeQuery()` liefert `[]` statt `null` bei abgebrochenem Query-Aufbau — Minor (claude)
+>
+> Bei `!$qb` wird `[]` zurückgegeben — harmlos (falsy), aber semantisch schief gegenüber dem `?array`-Vertrag, in dem `null` „nicht gefunden" bedeutet (`:117`: `return $entry ?: null;`).
+>
+> - `src/Engine/Loader/ValidationLoader.php:107-109`
+>
+> **Nutzer-Antwort: Return-type auf `array` angepasst.**
 
-## A-15: Backend-Responses ohne Null-Check auf `$listModel` — Info (claude)
+> ## A-11: Query-Assemblierung lebt in Event-Listener-Prioritäten ohne zentrale Übersicht — Info (claude)
+> 
+> Select@490, Conditions@470, Page@430, Order@420, Join@-450; Integrations-Listener dazwischen (250/220/200/190/100). Die Gesamtordnung ist nirgends zentral dokumentiert (kein Pipeline-Kommentar im `ListQueryDirector`).
+> 
+> - `src/EventListener/QueryStructModifier/SelectModifierListener.php:13`, `ConditionsModifierListener.php:11`, `PageModifierListener.php:11`, `OrderModifierListener.php:12`, `JoinModifierListener.php:10` · `src/Integration/ContaoCalendar/EventListener/CountEventsModifierListener.php:14` u. a.
+> 
+> **Nutzer-Antwort: Das muss in einem zukünftigen PR nochmal überarbeitet werden.**
 
-`getRelated()` kann `null` liefern; der Catch deckt nur Exceptions ab. Danach werden `$listModel->title` / `trans($listModel->type)` ungeprüft dereferenziert — in beiden Controllern. (Gelöschte/fehlende Liste → Backend-Crash; siehe auch SEC-03 in [30-sicherheit.md](30-sicherheit.md).)
+> ## A-12: `ViewInterface` ist leerer Marker; Aufrufer müssen downcasten — Info (claude)
+>
+> Das Interface ist leer (`src/Engine/View/ViewInterface.php:7-9`); `ReaderController` downcastet auf `ValidationView` (`src/Controller/ContentElement/ReaderController.php:127`). Die `@template`-Annotationen sind nur mit dem `generics.noParent`-Ignore in PHPStan haltbar.
 
-- `src/Controller/ContentElement/ReaderController.php:220-236` (Zugriff `:232-233`) · `src/Controller/ContentElement/ListViewController.php:154-168` (Zugriff `:166-167`)
+> ## A-13: `#[TaggedIterator]` ist seit Symfony 7.1 deprecated — Info (claude)
+> 
+> Genutzt in drei Registries; relevant für Deprecation-Logs bei Support-Matrix ^5.4|^6|^7. Nachfolger `AutowireIterator` existiert erst ab 6.3 → für die Matrix ggf. `!tagged_iterator` in YAML.
+> 
+> - `src/Registry/EngineModRegistry.php:15` · `src/Registry/ProjectorRegistry.php:19` · `src/Registry/FilterTypeRegistry.php:18`
+>
+> **Nutzer-Antwort: Passt so.**
 
-## A-16: `Engine`-Mods-API mischt Semantiken — Info (claude)
+> ## A-14: Statische Contao-Aufrufe in Context-DTOs — Info (claude, reduzierter Umfang)
+> 
+> `PageModel::findByPk` in wertartigen Context-Objekten — DB-Zugriffe, testfeindlich, aber Contao-idiomatisch.
+> 
+> - `src/Engine/Context/ReaderUrlConfigCreatorTrait.php:18` · `src/Engine/Context/ValidationContext.php:44`
+> 
+> **Nutzer-Antwort: Weiterhin statische Aufrufe, aber nun besser gekapselt.**
 
-`addMod()` appendet numerisch, `setMod()`/`unsetMod()` arbeiten mit String-Keys im selben Array; `unsetMod()` kann appendete Mods nicht adressieren — öffentlicher `@api`-Punkt.
+> ## A-15: Backend-Responses ohne Null-Check auf `$listModel` — Info (claude)
+> 
+> `getRelated()` kann `null` liefern; der Catch deckt nur Exceptions ab. Danach werden `$listModel->title` / `trans($listModel->type)` ungeprüft dereferenziert — in beiden Controllern. (Gelöschte/fehlende Liste → Backend-Crash; siehe auch SEC-03 in [30-sicherheit.md](30-sicherheit.md).)
+> 
+> - `src/Controller/ContentElement/ReaderController.php:220-236` (Zugriff `:232-233`) · `src/Controller/ContentElement/ListViewController.php:154-168` (Zugriff `:166-167`)
+> 
+> **Nutzer-Antwort: Good Catch! Ist jetzt mit einer entsprechenden Warnung gesichert.**
 
-- `src/Engine/Engine.php:66-93`
+> ## A-16: `Engine`-Mods-API mischt Semantiken — Info (claude)
+> 
+> `addMod()` appendet numerisch, `setMod()`/`unsetMod()` arbeiten mit String-Keys im selben Array; `unsetMod()` kann appendete Mods nicht adressieren — öffentlicher `@api`-Punkt.
+> 
+> - `src/Engine/Engine.php:66-93`
+>
+> **Nutzer-Antwort: Das ist kein Fehler sondern explizit so gewollt. Der Nutzer hat die Wahl, Filter für mehrfache veränderung überschreibbar zu machen, oder nicht. In den meisten Fällen wird das nicht gebraucht, daher reicht Listenindexierung ohne Möglichkeit zur Änderung.**

composer.json

@@ -14,6 +14,7 @@
         "psr/log": "^1.0 || ^2.0 || ^3.0",
         "symfony/config": "^5.4 || ^6.0 || ^7.0",
         "symfony/dependency-injection": "^5.4 || ^6.0 || ^7.0",
+        "symfony/event-dispatcher": "^5.4 || ^6.0 || ^7.0",
         "symfony/event-dispatcher-contracts": "^1.0 || ^2.0 || ^3.0",
         "symfony/filesystem": "^5.4 || ^6.0 || ^7.0",
         "symfony/form": "^5.4 || ^6.0 || ^7.0",
@@ -35,7 +36,6 @@
         "heimrichhannot/contao-test-utilities-bundle": "^0.1",
         "phpunit/phpunit": "^8.0 || ^9.0",
         "php-coveralls/php-coveralls": "^2.0",
-        "symfony/event-dispatcher": "^5.4 || ^6.0 || ^7.0",
         "symfony/phpunit-bridge": "^5.4 || ^6.0 || ^7.0",
         "phpstan/phpstan": "^1.10",
         "phpstan/phpstan-symfony": "^1.2"

src/Controller/ContentElement/ListViewController.php

@@ -215,6 +215,14 @@ protected function getBackendResponse(Template $template, ContentModel $model, R
             return new Response($e->getMessage());
         }
 
+        if (!$listModel instanceof ListModel) {
+            return new Response(\sprintf(
+                '<div><strong class="tl_red">%s</strong></div><div>%s</div>',
+                $this->translator->trans('reader.invalid_list', [], 'flare'),
+                Str::formatHeadline($model->headline, withTags: true),
+            ));
+        }
+
         return new Response(\sprintf(
             '<div>%s</div><span>%s</span> <span class="tl_gray">[%s, %s]</span>',
             (string) Str::formatHeadline($model->headline),

src/Controller/ContentElement/ReaderController.php

@@ -125,7 +125,7 @@ protected function getFrontendResponse(Template $template, ContentModel $content
             $validationView = $engine->createView();
 
             if (!$validationView instanceof ValidationView) {
-                throw ViewException::create(ValidationView::class, $validationView, __METHOD__);
+                throw ViewException::create(ValidationView::class, $validationView, method: __METHOD__);
             }
 
             if (!$autoItemModel = $validationView->getModelByAutoItem($autoItem)) {
@@ -226,6 +226,14 @@ protected function getBackendResponse(Template $template, ContentModel $model, R
             return new Response($e->getMessage());
         }
 
+        if (!$listModel instanceof ListModel) {
+            return new Response(\sprintf(
+                '<div><strong class="tl_red">%s</strong></div><div>%s</div>',
+                $this->translator->trans('reader.invalid_list', [], 'flare'),
+                Str::formatHeadline($model->headline, withTags: true),
+            ));
+        }
+
         return new Response(\sprintf(
             '%s%s <span class="tl_gray">[%s, %s]</span>',
             Str::formatHeadline($model->headline, withTags: true),

src/Engine/Context/Factory/InteractiveContextFactory.php

@@ -38,8 +38,8 @@ public function createFromContent(ContentModel $contentModel, ListSpec $list): I
 
         $config = new InteractiveContext(
             paginatorConfig: $paginatorConfig,
-            sortOrderSequence: $sortOrderSequence,
             formName: $filterFormName,
+            sortOrderSequence: $sortOrderSequence,
             contentModelId: (int) $contentModel->id,
             formActionPage: (int) $contentModel->{ContentContainer::FIELD_JUMP_TO},
             jumpToReaderPageId: $jumpToReaderPageId,

src/Engine/Context/Factory/ValidationContextFactory.php

@@ -7,7 +7,6 @@
 use Contao\ContentModel;
 use HeimrichHannot\FlareBundle\DataContainer\ContentContainer;
 use HeimrichHannot\FlareBundle\Engine\Context\ValidationContext;
-use HeimrichHannot\FlareBundle\Engine\View\InteractiveView;
 use HeimrichHannot\FlareBundle\List\ListSpec;
 use Symfony\Component\Validator\Exception\ValidationFailedException;
 use Symfony\Component\Validator\Validator\ValidatorInterface;

src/Engine/Context/InteractiveContext.php

@@ -23,14 +23,16 @@ public static function getContextType(): string
 
     public function __construct(
         public PaginatorConfig              $paginatorConfig,
-        public ?SortOrderSequence           $sortOrderSequence = null,
         #[Assert\NotBlank] public string    $formName,
+        public ?SortOrderSequence           $sortOrderSequence = null,
         #[Assert\PositiveOrZero] public int $contentModelId = 0,
         #[Assert\PositiveOrZero] public int $formActionPage = 0,
         #[Assert\PositiveOrZero] public int $jumpToReaderPageId = 0,
         #[Assert\NotBlank] public string    $autoItemField = 'id',
         public ?string                      $pageParam = null,
-    ) {}
+    ) {
+        $this->initJumpToReaderPage();
+    }
 
     public function getFormName(): string
     {

src/Engine/Context/ReaderUrlConfigCreatorTrait.php

@@ -9,16 +9,28 @@
 
 trait ReaderUrlConfigCreatorTrait
 {
-    public function createReaderUrlConfig(): ?ReaderUrlConfig
+    private \Closure $jumpToReaderPage;
+
+    final protected function initJumpToReaderPage(): void
     {
-        if (!$this->jumpToReaderPageId) {
-            return null;
-        }
+        $this->jumpToReaderPage = function (): ?PageModel {
+            $pageModel = PageModel::findByPk($this->jumpToReaderPageId);
+            $this->jumpToReaderPage = static fn (): ?PageModel => $pageModel;
+            return $pageModel;
+        };
+    }
 
-        if (!$pageModel = PageModel::findByPk($this->jumpToReaderPageId)) {
+    protected function getJumpToReaderPage(): ?PageModel
+    {
+        return ($this->jumpToReaderPage)();
+    }
+
+    public function createReaderUrlConfig(): ?ReaderUrlConfig
+    {
+        if (!$pageModel = $this->getJumpToReaderPage()) {
             return null;
         }
 
         return new ReaderUrlConfig(readerPage: $pageModel, autoItemField: $this->autoItemField);
     }
-}
+}

src/Engine/Context/ValidationContext.php

@@ -16,6 +16,8 @@
     use ReaderUrlConfigCreatorTrait;
 
     private PaginatorConfig $paginatorConfig;
+    private \Closure $jumpToListViewPage;
+    private \Closure $jumpToReaderPage;
 
     public static function getContextType(): string
     {
@@ -29,6 +31,14 @@ public function __construct(
         private array                       $filterValues = [],
     ) {
         $this->paginatorConfig = new PaginatorConfig(itemsPerPage: 1);
+
+        $this->jumpToListViewPage = function (): ?PageModel {
+            $pageModel = PageModel::findByPk($this->jumpToListViewPageId);
+            $this->jumpToListViewPage = static fn (): ?PageModel => $pageModel;
+            return $pageModel;
+        };
+
+        $this->initJumpToReaderPage();
     }
 
     public function createBackLink(): ?BackLink
@@ -37,7 +47,7 @@ public function createBackLink(): ?BackLink
             return null;
         }
 
-        if (!$pageModel = PageModel::findByPk($this->jumpToListViewPageId)) {
+        if (!$pageModel = ($this->jumpToListViewPage)()) {
             return null;
         }
 

src/Engine/Loader/ValidationLoader.php

@@ -127,7 +127,7 @@ public function fetchEntryByAutoItem(string $autoItem): ?array
     /**
      * @throws \Exception
      */
-    private function executeQuery(ListSpec $list, ValidationContext $context): ?array
+    private function executeQuery(ListSpec $list, ValidationContext $context): array
     {
         $qb = $this->listQueryDirector->createQueryBuilder(new ListQueryConfig(
             list: $list,
@@ -145,6 +145,6 @@ private function executeQuery(ListSpec $list, ValidationContext $context): ?arra
 
         $result->free();
 
-        return $entry ?: null;
+        return $entry ?: [];
     }
 }