From 6157338e5972246e21e37db883a8ccb5cc2e392f Mon Sep 17 00:00:00 2001
From: Guy Sartorelli <guy.sartorelli@gmail.com>
Date: Thu, 26 Sep 2024 16:42:34 +1200
Subject: [PATCH] ENH Improve type safety to support refactored template layer

---
 code/Controllers/ContentController.php        | 24 +++-----
 code/Model/SiteTree.php                       |  4 +-
 tests/behat/src/ThemeContext.php              |  2 +-
 .../php/Controllers/ContentControllerTest.php | 31 ++++++----
 .../DummyTemplateContentController.php        | 15 +++++
 .../DummyTemplateEngine.php                   | 56 +++++++++++++++++++
 6 files changed, 101 insertions(+), 31 deletions(-)
 create mode 100644 tests/php/Controllers/ContentControllerTest/DummyTemplateContentController.php
 create mode 100644 tests/php/Controllers/ContentControllerTest/DummyTemplateEngine.php

diff --git a/code/Controllers/ContentController.php b/code/Controllers/ContentController.php
index 2e1a10e1..36ac4b97 100644
--- a/code/Controllers/ContentController.php
+++ b/code/Controllers/ContentController.php
@@ -289,16 +289,6 @@ class ContentController extends Controller
         return new ArrayList($visible);
     }
 
-    /**
-     * @return ArrayList<SiteTree>
-     * @deprecated 5.4.0 Use getMenu() instead. You can continue to use $Menu in templates.
-     */
-    public function Menu($level)
-    {
-        Deprecation::noticeWithNoReplacment('5.4.0', 'Use getMenu() instead. You can continue to use $Menu in templates.');
-        return $this->getMenu($level);
-    }
-
     /**
      * Returns the default log-in form.
      *
@@ -415,23 +405,23 @@ HTML;
         $action = $action === 'index' ? '' : '_' . $action;
 
         $templatesFound = [];
-        // Find templates for the record + action together - e.g. Page_action.ss
+        // Find templates for the record + action together - e.g. Page_action
         if ($this->dataRecord instanceof SiteTree) {
             $templatesFound[] = $this->dataRecord->getViewerTemplates($action);
         }
 
-        // Find templates for the controller + action together - e.g. PageController_action.ss
-        $templatesFound[] = SSViewer::get_templates_by_class(static::class, $action, Controller::class);
+        // Find templates for the controller + action together - e.g. PageController_action
+        $templatesFound[] = SSViewer::get_templates_by_class(static::class, $action ?? '', Controller::class);
 
-        // Find templates for the record without an action - e.g. Page.ss
+        // Find templates for the record without an action - e.g. Page
         if ($this->dataRecord instanceof SiteTree) {
             $templatesFound[] = $this->dataRecord->getViewerTemplates();
         }
 
-        // Find the templates for the controller without an action - e.g. PageController.ss
-        $templatesFound[] = SSViewer::get_templates_by_class(static::class, "", Controller::class);
+        // Find the templates for the controller without an action - e.g. PageController
+        $templatesFound[] = SSViewer::get_templates_by_class(static::class, '', Controller::class);
 
         $templates = array_merge(...$templatesFound);
-        return SSViewer::create($templates);
+        return SSViewer::create($templates, $this->getTemplateEngine());
     }
 }
diff --git a/code/Model/SiteTree.php b/code/Model/SiteTree.php
index 2ee8e6b1..4d053e5f 100755
--- a/code/Model/SiteTree.php
+++ b/code/Model/SiteTree.php
@@ -1454,7 +1454,7 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
 
         $tags['title'] = [
             'tag' => 'title',
-            'content' => $this->obj('Title')->forTemplate()
+            'content' => $this->obj('Title')?->forTemplate()
         ];
 
         $generator = $this->getGenerator();
@@ -1601,7 +1601,7 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
 
         $tagString = implode("\n", $tags);
         if ($this->ExtraMeta) {
-            $tagString .= $this->obj('ExtraMeta')->forTemplate();
+            $tagString .= $this->obj('ExtraMeta')?->forTemplate();
         }
 
         $this->extend('updateMetaTags', $tagString);
diff --git a/tests/behat/src/ThemeContext.php b/tests/behat/src/ThemeContext.php
index ead1f826..3da77ee2 100644
--- a/tests/behat/src/ThemeContext.php
+++ b/tests/behat/src/ThemeContext.php
@@ -33,7 +33,7 @@ class ThemeContext implements Context
     }
 
     /**
-     * Create a template within a test theme
+     * Create a template within a test theme. Only ss templates are supported.
      *
      * @Given /^a template "(?<template>[^"]+)" in theme "(?<theme>[^"]+)" with content "(?<content>[^"]+)"/
      * @param string $template
diff --git a/tests/php/Controllers/ContentControllerTest.php b/tests/php/Controllers/ContentControllerTest.php
index b661fd6d..51997c9a 100644
--- a/tests/php/Controllers/ContentControllerTest.php
+++ b/tests/php/Controllers/ContentControllerTest.php
@@ -5,6 +5,8 @@ namespace SilverStripe\CMS\Tests\Controllers;
 use SilverStripe\CMS\Controllers\ContentController;
 use SilverStripe\CMS\Controllers\RootURLController;
 use SilverStripe\CMS\Model\SiteTree;
+use SilverStripe\CMS\Tests\Controllers\ContentControllerTest\DummyTemplateContentController;
+use SilverStripe\CMS\Tests\Controllers\ContentControllerTest\DummyTemplateEngine;
 use SilverStripe\Control\Director;
 use SilverStripe\Control\HTTPResponse_Exception;
 use SilverStripe\Core\Config\Config;
@@ -162,7 +164,7 @@ class ContentControllerTest extends FunctionalTest
     {
         $this->useTestTheme(__DIR__, 'controllertest', function () {
 
-            // Test a page without a controller (ContentControllerTest_PageWithoutController.ss)
+            // Test a page without a controller (ContentControllerTest_PageWithoutController)
             $page = new ContentControllerTestPageWithoutController();
             $page->URLSegment = "test";
             $page->write();
@@ -171,7 +173,7 @@ class ContentControllerTest extends FunctionalTest
             $response = $this->get($page->RelativeLink());
             $this->assertEquals("ContentControllerTestPageWithoutController", trim($response->getBody() ?? ''));
 
-            // This should fall over to user Page.ss
+            // This should fall over to use Page
             $page = new ContentControllerTestPage();
             $page->URLSegment = "test";
             $page->write();
@@ -191,20 +193,27 @@ class ContentControllerTest extends FunctionalTest
             $this->assertEquals("ContentControllerTestPage_test", trim($response->getBody() ?? ''));
 
             // Test that an action without a template will default to the index template, which is
-            // to say the default Page.ss template
+            // to say the default Page template
             $response = $this->get($page->RelativeLink("testwithouttemplate"));
             $this->assertEquals("Page", trim($response->getBody() ?? ''));
 
-            // Test that an action with a template will render the both action template *and* the
+            // Test that an action with a template will render both the action template *and* the
             // correct parent template
-            $controller = new ContentController($page);
+            $controller = new DummyTemplateContentController($page);
             $viewer = $controller->getViewer('test');
-            $this->assertEquals(
-                __DIR__
-                . '/themes/controllertest/templates/SilverStripe/CMS/Tests/Controllers/'
-                . 'ContentControllerTestPage_test.ss',
-                $viewer->templates()['main']
-            );
+            /** @var DummyTemplateEngine $engine */
+            $engine = $viewer->getTemplateEngine();
+            $templateCandidates = $engine->getTemplates();
+            // Check for the action templates
+            $this->assertContains('SilverStripe\CMS\Tests\Controllers\ContentControllerTestPage_test', $templateCandidates);
+            $this->assertContains('SilverStripe\CMS\Model\SiteTree_test', $templateCandidates);
+            $this->assertContains('SilverStripe\CMS\Tests\Controllers\ContentControllerTest\DummyTemplateContentController_test', $templateCandidates);
+            $this->assertContains('SilverStripe\CMS\Controllers\ContentController_test', $templateCandidates);
+            // Check for the parent templates
+            $this->assertContains('SilverStripe\CMS\Tests\Controllers\ContentControllerTestPage', $templateCandidates);
+            $this->assertContains('SilverStripe\CMS\Model\SiteTree', $templateCandidates);
+            $this->assertContains('SilverStripe\CMS\Tests\Controllers\ContentControllerTest\DummyTemplateContentController', $templateCandidates);
+            $this->assertContains('SilverStripe\CMS\Controllers\ContentController', $templateCandidates);
         });
     }
 }
diff --git a/tests/php/Controllers/ContentControllerTest/DummyTemplateContentController.php b/tests/php/Controllers/ContentControllerTest/DummyTemplateContentController.php
new file mode 100644
index 00000000..15078b66
--- /dev/null
+++ b/tests/php/Controllers/ContentControllerTest/DummyTemplateContentController.php
@@ -0,0 +1,15 @@
+<?php
+
+namespace SilverStripe\CMS\Tests\Controllers\ContentControllerTest;
+
+use SilverStripe\CMS\Controllers\ContentController;
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\View\TemplateEngine;
+
+class DummyTemplateContentController extends ContentController implements TestOnly
+{
+    protected function getTemplateEngine(): TemplateEngine
+    {
+        return new DummyTemplateEngine();
+    }
+}
diff --git a/tests/php/Controllers/ContentControllerTest/DummyTemplateEngine.php b/tests/php/Controllers/ContentControllerTest/DummyTemplateEngine.php
new file mode 100644
index 00000000..b16597bd
--- /dev/null
+++ b/tests/php/Controllers/ContentControllerTest/DummyTemplateEngine.php
@@ -0,0 +1,56 @@
+<?php
+
+namespace SilverStripe\CMS\Tests\Controllers\ContentControllerTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\View\TemplateEngine;
+use SilverStripe\View\ViewLayerData;
+
+/**
+ * A dummy template renderer that doesn't actually render any templates.
+ */
+class DummyTemplateEngine implements TemplateEngine, TestOnly
+{
+    private string $output = '<html><head></head><body></body></html>';
+
+    private string|array $templates;
+
+    public function __construct(string|array $templateCandidates = [])
+    {
+        $this->templates = $templateCandidates;
+    }
+
+    public function setTemplate(string|array $templateCandidates): static
+    {
+        $this->templates = $templateCandidates;
+        return $this;
+    }
+
+    public function hasTemplate(string|array $templateCandidates): bool
+    {
+        return true;
+    }
+
+    public function renderString(string $template, ViewLayerData $model, array $overlay = [], bool $cache = true): string
+    {
+        return $this->output;
+    }
+
+    public function render(ViewLayerData $model, array $overlay = []): string
+    {
+        return $this->output;
+    }
+
+    public function setOutput(string $output): void
+    {
+        $this->output = $output;
+    }
+
+    /**
+     * Returns the template candidates that were passed to the constructor or to setTemplate()
+     */
+    public function getTemplates(): array
+    {
+        return $this->templates;
+    }
+}