From 0927e54780d2f5b7d240ffd7f1f39522c19610a0 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 30 Dec 2016 10:32:04 +1300 Subject: [PATCH] API Allow controller discovery without underscore (PSR-2 compliance) --- .upgrade.yml | 5 +- ...Controller.php => ErrorPageController.php} | 4 +- ...oller.php => RedirectorPageController.php} | 4 +- code/Model/SiteTree.php | 15 +++- tests/bootstrap/fixtures/Page.php.fixture | 10 ++- tests/controller/ContentControllerTest.php | 8 +-- tests/model/RedirectorPageTest.php | 8 +-- tests/model/SiteTreeTest.php | 70 +++++++++++++++++-- tests/model/VirtualPageTest.php | 4 +- 9 files changed, 102 insertions(+), 26 deletions(-) rename code/Model/{ErrorPage_Controller.php => ErrorPageController.php} (91%) rename code/Model/{RedirectorPage_Controller.php => RedirectorPageController.php} (89%) diff --git a/.upgrade.yml b/.upgrade.yml index 1d4cbe17..7cfb4b03 100644 --- a/.upgrade.yml +++ b/.upgrade.yml @@ -35,9 +35,9 @@ mappings: SiteTreeURLSegmentField_Readonly: SilverStripe\CMS\Forms\SiteTreeURLSegmentField_Readonly CurrentPageIdentifier: SilverStripe\CMS\Model\CurrentPageIdentifier ErrorPage: SilverStripe\CMS\Model\ErrorPage - ErrorPage_Controller: SilverStripe\CMS\Model\ErrorPage_Controller + ErrorPage_Controller: SilverStripe\CMS\Model\ErrorPageController RedirectorPage: SilverStripe\CMS\Model\RedirectorPage - RedirectorPage_Controller: SilverStripe\CMS\Model\RedirectorPage_Controller + RedirectorPage_Controller: SilverStripe\CMS\Model\RedirectorPageController SiteTree: SilverStripe\CMS\Model\SiteTree SiteTreeExtension: SilverStripe\CMS\Model\SiteTreeExtension SiteTreeFileExtension: SilverStripe\CMS\Model\SiteTreeFileExtension @@ -45,7 +45,6 @@ mappings: SiteTreeLinkTracking: SilverStripe\CMS\Model\SiteTreeLinkTracking SiteTreeLinkTracking_Parser: SilverStripe\CMS\Model\SiteTreeLinkTracking_Parser VirtualPage: SilverStripe\CMS\Model\VirtualPage - VirtualPage_Controller: SilverStripe\CMS\Model\VirtualPage_Controller BrokenFilesReport: SilverStripe\CMS\Reports\BrokenFilesReport SideReport_BrokenFiles: SilverStripe\CMS\Reports\SideReport_BrokenFiles BrokenLinksReport: SilverStripe\CMS\Reports\BrokenLinksReport diff --git a/code/Model/ErrorPage_Controller.php b/code/Model/ErrorPageController.php similarity index 91% rename from code/Model/ErrorPage_Controller.php rename to code/Model/ErrorPageController.php index aaacb3ad..167e1f43 100644 --- a/code/Model/ErrorPage_Controller.php +++ b/code/Model/ErrorPageController.php @@ -4,13 +4,13 @@ namespace SilverStripe\CMS\Model; use SilverStripe\ORM\DataModel; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; -use Page_Controller; +use PageController; /** * Controller for ErrorPages. */ -class ErrorPage_Controller extends Page_Controller +class ErrorPageController extends PageController { /** diff --git a/code/Model/RedirectorPage_Controller.php b/code/Model/RedirectorPageController.php similarity index 89% rename from code/Model/RedirectorPage_Controller.php rename to code/Model/RedirectorPageController.php index be18612e..a55e8173 100644 --- a/code/Model/RedirectorPage_Controller.php +++ b/code/Model/RedirectorPageController.php @@ -1,12 +1,12 @@ objFromFixture('SilverStripe\\CMS\\Model\\RedirectorPage', 'goodinternal'); - RedirectorPage_Controller::add_extension('RedirectorPageTest_RedirectExtension'); + RedirectorPageController::add_extension('RedirectorPageTest_RedirectExtension'); $response = $this->get($page->regularLink()); $this->assertEquals(302, $response->getStatusCode()); $this->assertEquals('/foo', $response->getHeader('Location')); - RedirectorPage_Controller::remove_extension('RedirectorPageTest_RedirectExtension'); + RedirectorPageController::remove_extension('RedirectorPageTest_RedirectExtension'); } } diff --git a/tests/model/SiteTreeTest.php b/tests/model/SiteTreeTest.php index 164c752b..cf3bb1e9 100644 --- a/tests/model/SiteTreeTest.php +++ b/tests/model/SiteTreeTest.php @@ -17,13 +17,12 @@ use SilverStripe\Control\Session; use SilverStripe\View\Parsers\ShortcodeParser; use SilverStripe\Control\Director; use SilverStripe\i18n\i18n; +use SilverStripe\Dev\Deprecation; use SilverStripe\Dev\SapphireTest; use SilverStripe\Dev\TestOnly; use SilverStripe\View\Parsers\HTMLCleaner; use SilverStripe\View\Parsers\Diff; - - /** * @package cms * @subpackage tests @@ -1334,6 +1333,54 @@ class SiteTreeTest extends SapphireTest { $this->assertFalse($page->canPublish()); } + /** + * Test that the controller name for a SiteTree instance can be gathered by appending "Controller" to the SiteTree + * class name in a PSR-2 compliant manner. + */ + public function testGetControllerName() + { + $class = new Page; + $this->assertSame('PageController', $class->getControllerName()); + } + + /** + * Test that underscored class names (legacy) are still supported, but raise a deprecation notice. The exception and + * manual catching is in place to ensure that we can (A) detect that a deprecation notice was raised and (B) that + * we can restore the old error handling after catching the exception we threw from this test. Simply asserting that + * an exception is thrown will not allow further logic to execute after the exception is thrown. + */ + public function testGetControllerNameWithUnderscoresIsSupported() + { + $defaultEnabled = Deprecation::get_enabled(); + Deprecation::set_enabled(true); + Deprecation::notification_version('4.0.0'); + + // Catch user_errors thrown by deprecation + set_error_handler( + function ($errno, $errstr) { + throw new \Exception($errstr); + }, + E_USER_DEPRECATED + ); + + $class = new SiteTreeTest_LegacyControllerName; + $notice = ''; + try { + $this->assertSame('SiteTreeTest_LegacyControllerName_Controller', $class->getControllerName()); + } catch (\Exception $ex) { + $notice = $ex->getMessage(); + } + restore_error_handler(); + + $this->assertContains( + 'Underscored controller class names are deprecated. Use "MyController" instead ' + . 'of "My_Controller".', + $notice, + 'A deprecation notice should have been raised.' + ); + Deprecation::set_enabled($defaultEnabled); + } + } /**#@+ @@ -1341,18 +1388,18 @@ class SiteTreeTest extends SapphireTest { */ class SiteTreeTest_PageNode extends Page implements TestOnly { } -class SiteTreeTest_PageNode_Controller extends Page_Controller implements TestOnly { +class SiteTreeTest_PageNodeController extends PageController implements TestOnly { } class SiteTreeTest_Conflicted extends Page implements TestOnly { } -class SiteTreeTest_Conflicted_Controller extends Page_Controller implements TestOnly { +class SiteTreeTest_ConflictedController extends PageController implements TestOnly { private static $allowed_actions = array ( 'conflicted-action' ); public function hasActionTemplate($template) { - if($template == 'conflicted-template') { + if ($template == 'conflicted-template') { return true; } else { return parent::hasActionTemplate($template); @@ -1468,3 +1515,16 @@ class SiteTreeTest_AdminDeniedExtension extends DataExtension implements TestOnl public function canAddChildren() { return false; } public function canView() { return false; } } + +/** + * An empty SiteTree instance with a controller to test that legacy controller names can still be loaded + */ +class SiteTreeTest_LegacyControllerName extends Page implements TestOnly +{ + +} + +class SiteTreeTest_LegacyControllerName_Controller extends PageController implements TestOnly +{ + +} diff --git a/tests/model/VirtualPageTest.php b/tests/model/VirtualPageTest.php index 581e536a..9155750a 100644 --- a/tests/model/VirtualPageTest.php +++ b/tests/model/VirtualPageTest.php @@ -611,7 +611,7 @@ class VirtualPageTest extends FunctionalTest { $virtualPage = $this->objFromFixture('SilverStripe\\CMS\\Model\\VirtualPage', 'vp4'); $controller = ModelAsController::controller_for($virtualPage); - $this->assertInstanceOf('VirtualPageTest_ClassA_Controller', $controller); + $this->assertInstanceOf('VirtualPageTest_ClassAController', $controller); $this->assertTrue($controller->hasMethod('testMethod')); $this->assertEquals('hello', $controller->testMethod()); $this->assertTrue($controller->hasMethod('modelMethod')); @@ -641,7 +641,7 @@ class VirtualPageTest_ClassA extends Page implements TestOnly { } } -class VirtualPageTest_ClassA_Controller extends Page_Controller implements TestOnly { +class VirtualPageTest_ClassAController extends PageController implements TestOnly { private static $allowed_actions = [ 'testaction' ];