API Allow controller discovery without underscore (PSR-2 compliance)

This commit is contained in:
Robbie Averill 2016-12-30 10:32:04 +13:00
parent 03a17029ef
commit 0927e54780
9 changed files with 102 additions and 26 deletions

View File

@ -35,9 +35,9 @@ mappings:
SiteTreeURLSegmentField_Readonly: SilverStripe\CMS\Forms\SiteTreeURLSegmentField_Readonly SiteTreeURLSegmentField_Readonly: SilverStripe\CMS\Forms\SiteTreeURLSegmentField_Readonly
CurrentPageIdentifier: SilverStripe\CMS\Model\CurrentPageIdentifier CurrentPageIdentifier: SilverStripe\CMS\Model\CurrentPageIdentifier
ErrorPage: SilverStripe\CMS\Model\ErrorPage ErrorPage: SilverStripe\CMS\Model\ErrorPage
ErrorPage_Controller: SilverStripe\CMS\Model\ErrorPage_Controller ErrorPage_Controller: SilverStripe\CMS\Model\ErrorPageController
RedirectorPage: SilverStripe\CMS\Model\RedirectorPage RedirectorPage: SilverStripe\CMS\Model\RedirectorPage
RedirectorPage_Controller: SilverStripe\CMS\Model\RedirectorPage_Controller RedirectorPage_Controller: SilverStripe\CMS\Model\RedirectorPageController
SiteTree: SilverStripe\CMS\Model\SiteTree SiteTree: SilverStripe\CMS\Model\SiteTree
SiteTreeExtension: SilverStripe\CMS\Model\SiteTreeExtension SiteTreeExtension: SilverStripe\CMS\Model\SiteTreeExtension
SiteTreeFileExtension: SilverStripe\CMS\Model\SiteTreeFileExtension SiteTreeFileExtension: SilverStripe\CMS\Model\SiteTreeFileExtension
@ -45,7 +45,6 @@ mappings:
SiteTreeLinkTracking: SilverStripe\CMS\Model\SiteTreeLinkTracking SiteTreeLinkTracking: SilverStripe\CMS\Model\SiteTreeLinkTracking
SiteTreeLinkTracking_Parser: SilverStripe\CMS\Model\SiteTreeLinkTracking_Parser SiteTreeLinkTracking_Parser: SilverStripe\CMS\Model\SiteTreeLinkTracking_Parser
VirtualPage: SilverStripe\CMS\Model\VirtualPage VirtualPage: SilverStripe\CMS\Model\VirtualPage
VirtualPage_Controller: SilverStripe\CMS\Model\VirtualPage_Controller
BrokenFilesReport: SilverStripe\CMS\Reports\BrokenFilesReport BrokenFilesReport: SilverStripe\CMS\Reports\BrokenFilesReport
SideReport_BrokenFiles: SilverStripe\CMS\Reports\SideReport_BrokenFiles SideReport_BrokenFiles: SilverStripe\CMS\Reports\SideReport_BrokenFiles
BrokenLinksReport: SilverStripe\CMS\Reports\BrokenLinksReport BrokenLinksReport: SilverStripe\CMS\Reports\BrokenLinksReport

View File

@ -4,13 +4,13 @@ namespace SilverStripe\CMS\Model;
use SilverStripe\ORM\DataModel; use SilverStripe\ORM\DataModel;
use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPRequest;
use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse;
use Page_Controller; use PageController;
/** /**
* Controller for ErrorPages. * Controller for ErrorPages.
*/ */
class ErrorPage_Controller extends Page_Controller class ErrorPageController extends PageController
{ {
/** /**

View File

@ -1,12 +1,12 @@
<?php <?php
namespace SilverStripe\CMS\Model; namespace SilverStripe\CMS\Model;
use Page_Controller; use PageController;
/** /**
* Controller for the {@link RedirectorPage}. * Controller for the {@link RedirectorPage}.
*/ */
class RedirectorPage_Controller extends Page_Controller class RedirectorPageController extends PageController
{ {
public function init() public function init()

View File

@ -2713,7 +2713,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
} }
/** /**
* Find the controller name by our convention of {$ModelClass}_Controller * Find the controller name by our convention of {$ModelClass}Controller
* *
* @return string * @return string
*/ */
@ -2729,8 +2729,17 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
if ($class == SiteTree::class) { if ($class == SiteTree::class) {
break; break;
} }
//if we have a class of "{$ClassName}_Controller" then we found our controller // If we have a class of "{$ClassName}Controller" then we found our controller
if (class_exists($candidate = sprintf('%s_Controller', $class))) { if (class_exists($candidate = sprintf('%sController', $class))) {
$controller = $candidate;
break;
} elseif (class_exists($candidate = sprintf('%s_Controller', $class))) {
// Support the legacy underscored filename, but raise a deprecation notice
Deprecation::notice(
'4.0',
'Underscored controller class names are deprecated. Use "MyController" instead of "My_Controller".',
Deprecation::SCOPE_GLOBAL
);
$controller = $candidate; $controller = $candidate;
break; break;
} }

View File

@ -7,6 +7,14 @@ class Page extends SiteTree
{ {
} }
class Page_Controller extends ContentController class PageController extends ContentController
{ {
} }
/**
* Deprecated: Used for backwards compatibility between modules
*/
class Page_Controller extends ContentController
{
}

View File

@ -198,7 +198,7 @@ class ContentControllerTest extends FunctionalTest {
class ContentControllerTest_Page extends Page { } class ContentControllerTest_Page extends Page { }
class ContentControllerTest_Page_Controller extends Page_Controller { class ContentControllerTest_PageController extends PageController {
private static $allowed_actions = array ( private static $allowed_actions = array (
'second_index' 'second_index'
@ -218,10 +218,10 @@ class ContentControllerTest_Page_Controller extends Page_Controller {
class ContentControllerTestPageWithoutController extends Page { } class ContentControllerTestPageWithoutController extends Page { }
class ContentControllerTestPage extends Page { } class ContentControllerTestPage extends Page { }
class ContentControllerTestPage_Controller extends Page_Controller { class ContentControllerTestPageController extends PageController {
private static $allowed_actions = array( private static $allowed_actions = array(
"test", 'test',
"testwithouttemplate" 'testwithouttemplate'
); );
function testwithouttemplate() { function testwithouttemplate() {

View File

@ -1,7 +1,7 @@
<?php <?php
use SilverStripe\CMS\Model\RedirectorPage; use SilverStripe\CMS\Model\RedirectorPage;
use SilverStripe\CMS\Model\RedirectorPage_Controller; use SilverStripe\CMS\Model\RedirectorPageController;
use SilverStripe\Control\Director; use SilverStripe\Control\Director;
use SilverStripe\Dev\FunctionalTest; use SilverStripe\Dev\FunctionalTest;
use SilverStripe\Core\Extension; use SilverStripe\Core\Extension;
@ -81,17 +81,17 @@ class RedirectorPageTest extends FunctionalTest {
} }
/** /**
* Test that we can trigger a redirection before RedirectorPage_Controller::init() is called * Test that we can trigger a redirection before RedirectorPageController::init() is called
*/ */
public function testRedirectRespectsFinishedResponse() { public function testRedirectRespectsFinishedResponse() {
$page = $this->objFromFixture('SilverStripe\\CMS\\Model\\RedirectorPage', 'goodinternal'); $page = $this->objFromFixture('SilverStripe\\CMS\\Model\\RedirectorPage', 'goodinternal');
RedirectorPage_Controller::add_extension('RedirectorPageTest_RedirectExtension'); RedirectorPageController::add_extension('RedirectorPageTest_RedirectExtension');
$response = $this->get($page->regularLink()); $response = $this->get($page->regularLink());
$this->assertEquals(302, $response->getStatusCode()); $this->assertEquals(302, $response->getStatusCode());
$this->assertEquals('/foo', $response->getHeader('Location')); $this->assertEquals('/foo', $response->getHeader('Location'));
RedirectorPage_Controller::remove_extension('RedirectorPageTest_RedirectExtension'); RedirectorPageController::remove_extension('RedirectorPageTest_RedirectExtension');
} }
} }

View File

@ -17,13 +17,12 @@ use SilverStripe\Control\Session;
use SilverStripe\View\Parsers\ShortcodeParser; use SilverStripe\View\Parsers\ShortcodeParser;
use SilverStripe\Control\Director; use SilverStripe\Control\Director;
use SilverStripe\i18n\i18n; use SilverStripe\i18n\i18n;
use SilverStripe\Dev\Deprecation;
use SilverStripe\Dev\SapphireTest; use SilverStripe\Dev\SapphireTest;
use SilverStripe\Dev\TestOnly; use SilverStripe\Dev\TestOnly;
use SilverStripe\View\Parsers\HTMLCleaner; use SilverStripe\View\Parsers\HTMLCleaner;
use SilverStripe\View\Parsers\Diff; use SilverStripe\View\Parsers\Diff;
/** /**
* @package cms * @package cms
* @subpackage tests * @subpackage tests
@ -1334,6 +1333,54 @@ class SiteTreeTest extends SapphireTest {
$this->assertFalse($page->canPublish()); $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 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 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 ( private static $allowed_actions = array (
'conflicted-action' 'conflicted-action'
); );
public function hasActionTemplate($template) { public function hasActionTemplate($template) {
if($template == 'conflicted-template') { if ($template == 'conflicted-template') {
return true; return true;
} else { } else {
return parent::hasActionTemplate($template); return parent::hasActionTemplate($template);
@ -1468,3 +1515,16 @@ class SiteTreeTest_AdminDeniedExtension extends DataExtension implements TestOnl
public function canAddChildren() { return false; } public function canAddChildren() { return false; }
public function canView() { 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
{
}

View File

@ -611,7 +611,7 @@ class VirtualPageTest extends FunctionalTest {
$virtualPage = $this->objFromFixture('SilverStripe\\CMS\\Model\\VirtualPage', 'vp4'); $virtualPage = $this->objFromFixture('SilverStripe\\CMS\\Model\\VirtualPage', 'vp4');
$controller = ModelAsController::controller_for($virtualPage); $controller = ModelAsController::controller_for($virtualPage);
$this->assertInstanceOf('VirtualPageTest_ClassA_Controller', $controller); $this->assertInstanceOf('VirtualPageTest_ClassAController', $controller);
$this->assertTrue($controller->hasMethod('testMethod')); $this->assertTrue($controller->hasMethod('testMethod'));
$this->assertEquals('hello', $controller->testMethod()); $this->assertEquals('hello', $controller->testMethod());
$this->assertTrue($controller->hasMethod('modelMethod')); $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 = [ private static $allowed_actions = [
'testaction' 'testaction'
]; ];