From 65eb0bde6a25fbdd84a5c369eaa591df8ed9d523 Mon Sep 17 00:00:00 2001 From: Sam Minnee Date: Wed, 4 May 2016 19:19:34 +1200 Subject: [PATCH] API: Look for templates of namespaced classes in subfolders. This change will mean that SilverStripe\Control\Controller will look for its template in templates/SilverStripe/Control/Controller.ss. In order to preserve some backwards campatibility, non-namespaced classes can have the templates stored in any template subfolder, but once you add a namespace to a class, the namespaced path expression will need to be a subfolder of /templates or themes//templates. Layout and Content templates are stil supported as special template type, Includes still functions but is a no-op. Other template subfolders should not be used. --- core/manifest/TemplateManifest.php | 78 +++++++++++++++---- tests/core/manifest/TemplateManifestTest.php | 26 ++++++- .../templates/MyNamespace/Layout/MyClass.ss | 1 + .../module/templates/MyNamespace/MyClass.ss | 1 + .../MyNamespace/MySubnamespace/MySubclass.ss | 1 + .../theme/templates/MyNamespace/MyClass.ss | 0 tests/view/SSViewerTest.php | 26 +++---- .../TestNamespace/SSViewerTest_Controller.php | 8 ++ .../SSViewerTest_Controller.ss | 0 9 files changed, 111 insertions(+), 30 deletions(-) create mode 100644 tests/core/manifest/fixtures/templatemanifest/module/templates/MyNamespace/Layout/MyClass.ss create mode 100644 tests/core/manifest/fixtures/templatemanifest/module/templates/MyNamespace/MyClass.ss create mode 100644 tests/core/manifest/fixtures/templatemanifest/module/templates/MyNamespace/MySubnamespace/MySubclass.ss create mode 100644 tests/core/manifest/fixtures/templatemanifest/themes/theme/templates/MyNamespace/MyClass.ss create mode 100644 tests/view/TestNamespace/SSViewerTest_Controller.php rename tests/view/themes/layouttest/{ => TestNamespace}/SSViewerTest_Controller.ss (100%) diff --git a/core/manifest/TemplateManifest.php b/core/manifest/TemplateManifest.php index 12692e9f2..4839a29a8 100644 --- a/core/manifest/TemplateManifest.php +++ b/core/manifest/TemplateManifest.php @@ -178,34 +178,86 @@ class SS_TemplateManifest { $this->inited = true; } - public function handleFile($basename, $pathname, $depth) { + public function handleFile($basename, $pathname, $depth) + { $projectFile = false; $theme = null; - if (strpos($pathname, $this->base . '/' . THEMES_DIR) === 0) { - $start = strlen($this->base . '/' . THEMES_DIR) + 1; - $theme = substr($pathname, $start); - $theme = substr($theme, 0, strpos($theme, '/')); - $theme = strtok($theme, '_'); - } else if($this->project && (strpos($pathname, $this->base . '/' . $this->project .'/') === 0)) { + // Template in theme + if (preg_match( + '#'.preg_quote($this->base.'/'.THEMES_DIR).'/([^/_]+)(_[^/]+)?/(.*)$#', + $pathname, + $matches + )) { + $theme = $matches[1]; + $relPath = $matches[3]; + + // Template in project + } elseif (preg_match( + '#'.preg_quote($this->base.'/'.$this->project).'/(.*)$#', + $pathname, + $matches + )) { $projectFile = true; + $relPath = $matches[1]; + + // Template in module + } elseif (preg_match( + '#'.preg_quote($this->base).'/([^/]+)/(.*)$#', + $pathname, + $matches + )) { + $relPath = $matches[2]; + + } else { + throw new \LogicException("Can't determine meaning of path: $pathname"); } - $type = basename(dirname($pathname)); - $name = strtolower(substr($basename, 0, -3)); - - if ($type == self::TEMPLATES_DIR) { - $type = 'main'; + // If a templates subfolder is used, ignore that + if (preg_match('#'.preg_quote(self::TEMPLATES_DIR).'/(.*)$#', $relPath, $matches)) { + $relPath = $matches[1]; } + // Layout and Content folders have special meaning + if (preg_match('#^(.*/)?(Layout|Content|Includes)/([^/]+)$#', $relPath, $matches)) { + $type = $matches[2]; + $relPath = "$matches[1]$matches[3]"; + } else { + $type = "main"; + } + + $name = strtolower(substr($relPath, 0, -3)); + $name = str_replace('/', '\\', $name); + if ($theme) { $this->templates[$name]['themes'][$theme][$type] = $pathname; - } else if($projectFile) { + } else if ($projectFile) { $this->templates[$name][$this->project][$type] = $pathname; } else { $this->templates[$name][$type] = $pathname; } + // If we've found a template in a subdirectory, then allow its use for a non-namespaced class + // as well. This was a common SilverStripe 3 approach, where templates were placed into + // subfolders to suit the whim of the developer. + if (strpos($name, '\\') !== false) { + $name2 = substr($name, strrpos($name, '\\') + 1); + // In of these cases, the template will only be provided if it isn't already set. This + // matches SilverStripe 3 prioritisation. + if ($theme) { + if (!isset($this->templates[$name2]['themes'][$theme][$type])) { + $this->templates[$name2]['themes'][$theme][$type] = $pathname; + } + } else if ($projectFile) { + if (!isset($this->templates[$name2][$this->project][$type])) { + $this->templates[$name2][$this->project][$type] = $pathname; + } + } else { + if (!isset($this->templates[$name2][$type])) { + $this->templates[$name2][$type] = $pathname; + } + } + } } protected function init() { diff --git a/tests/core/manifest/TemplateManifestTest.php b/tests/core/manifest/TemplateManifestTest.php index a7a96cebd..61e9af45c 100644 --- a/tests/core/manifest/TemplateManifestTest.php +++ b/tests/core/manifest/TemplateManifestTest.php @@ -25,7 +25,7 @@ class TemplateManifestTest extends SapphireTest { public function testGetTemplates() { $expect = array( 'root' => array( - 'module' => "{$this->base}/module/Root.ss" + 'main' => "{$this->base}/module/Root.ss" ), 'page' => array( 'main' => "{$this->base}/module/templates/Page.ss", @@ -54,6 +54,30 @@ class TemplateManifestTest extends SapphireTest { 'theme' => array('main' => "{$this->base}/themes/theme/templates/CustomThemePage.ss",) ) ), + 'mynamespace\myclass' => array( + 'main' => "{$this->base}/module/templates/MyNamespace/MyClass.ss", + 'Layout' => "{$this->base}/module/templates/MyNamespace/Layout/MyClass.ss", + 'themes' => array( + 'theme' => array( + 'main' => "{$this->base}/themes/theme/templates/MyNamespace/MyClass.ss", + ) + ), + ), + 'mynamespace\mysubnamespace\mysubclass' => array( + 'main' => "{$this->base}/module/templates/MyNamespace/MySubnamespace/MySubclass.ss", + ), + 'myclass' => array( + 'main' => "{$this->base}/module/templates/MyNamespace/MyClass.ss", + 'Layout' => "{$this->base}/module/templates/MyNamespace/Layout/MyClass.ss", + 'themes' => array( + 'theme' => array( + 'main' => "{$this->base}/themes/theme/templates/MyNamespace/MyClass.ss", + ) + ), + ), + 'mysubclass' => array( + 'main' => "{$this->base}/module/templates/MyNamespace/MySubnamespace/MySubclass.ss", + ), 'include' => array('themes' => array( 'theme' => array( 'Includes' => "{$this->base}/themes/theme/templates/Includes/Include.ss" diff --git a/tests/core/manifest/fixtures/templatemanifest/module/templates/MyNamespace/Layout/MyClass.ss b/tests/core/manifest/fixtures/templatemanifest/module/templates/MyNamespace/Layout/MyClass.ss new file mode 100644 index 000000000..d1edecb79 --- /dev/null +++ b/tests/core/manifest/fixtures/templatemanifest/module/templates/MyNamespace/Layout/MyClass.ss @@ -0,0 +1 @@ +MyClass.ss \ No newline at end of file diff --git a/tests/core/manifest/fixtures/templatemanifest/module/templates/MyNamespace/MyClass.ss b/tests/core/manifest/fixtures/templatemanifest/module/templates/MyNamespace/MyClass.ss new file mode 100644 index 000000000..d1edecb79 --- /dev/null +++ b/tests/core/manifest/fixtures/templatemanifest/module/templates/MyNamespace/MyClass.ss @@ -0,0 +1 @@ +MyClass.ss \ No newline at end of file diff --git a/tests/core/manifest/fixtures/templatemanifest/module/templates/MyNamespace/MySubnamespace/MySubclass.ss b/tests/core/manifest/fixtures/templatemanifest/module/templates/MyNamespace/MySubnamespace/MySubclass.ss new file mode 100644 index 000000000..7d5e1028f --- /dev/null +++ b/tests/core/manifest/fixtures/templatemanifest/module/templates/MyNamespace/MySubnamespace/MySubclass.ss @@ -0,0 +1 @@ +MySubclass.ss \ No newline at end of file diff --git a/tests/core/manifest/fixtures/templatemanifest/themes/theme/templates/MyNamespace/MyClass.ss b/tests/core/manifest/fixtures/templatemanifest/themes/theme/templates/MyNamespace/MyClass.ss new file mode 100644 index 000000000..e69de29bb diff --git a/tests/view/SSViewerTest.php b/tests/view/SSViewerTest.php index a4e05408c..d42958a73 100644 --- a/tests/view/SSViewerTest.php +++ b/tests/view/SSViewerTest.php @@ -1124,24 +1124,24 @@ after') $self = $this; $this->useTestTheme(dirname(__FILE__), 'layouttest', function() use ($self) { // Test passing a string - $templates = SSViewer::get_templates_by_class('SSViewerTest_Controller', '', 'Controller'); - $self->assertCount(2, $templates); + $templates = SSViewer::get_templates_by_class( + 'TestNamespace\SSViewerTest_Controller', + '', + 'Controller' + ); + $self->assertEquals([ + 'TestNamespace\SSViewerTest_Controller', + 'Controller', + ], $templates); // Test to ensure we're stopping at the base class. - $templates = SSViewer::get_templates_by_class('SSViewerTest_Controller', '', 'SSViewerTest_Controller'); + $templates = SSViewer::get_templates_by_class('TestNamespace\SSViewerTest_Controller', '', 'TestNamespace\SSViewerTest_Controller'); $self->assertCount(1, $templates); // Make sure we can filter our templates by suffix. $templates = SSViewer::get_templates_by_class('SSViewerTest', '_Controller'); $self->assertCount(1, $templates); - // Test passing a valid object - $templates = SSViewer::get_templates_by_class("SSViewerTest_Controller", '', 'Controller'); - - // Test that templates are returned in the correct order - $self->assertEquals('SSViewerTest_Controller', array_shift($templates)); - $self->assertEquals('Controller', array_shift($templates)); - // Let's throw something random in there. $self->setExpectedException('InvalidArgumentException'); $templates = SSViewer::get_templates_by_class(array()); @@ -1593,11 +1593,6 @@ class SSViewerTest_ViewableData extends ViewableData implements TestOnly { } } - -class SSViewerTest_Controller extends Controller { - -} - class SSViewerTest_Object extends DataObject implements TestOnly { public $number = null; @@ -1687,4 +1682,3 @@ class SSViewerTest_LevelTest extends ViewableData implements TestOnly { return new self($number); } } - diff --git a/tests/view/TestNamespace/SSViewerTest_Controller.php b/tests/view/TestNamespace/SSViewerTest_Controller.php new file mode 100644 index 000000000..6814768b6 --- /dev/null +++ b/tests/view/TestNamespace/SSViewerTest_Controller.php @@ -0,0 +1,8 @@ +