From 957469d770c555d5461ea476906fbaa64ac2bc02 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Tue, 29 Jan 2013 14:14:47 +0100 Subject: [PATCH] API Removed auto-routing of controller name Use custom routing rules to achieve this effect (see changelog) --- _config/routes.yml | 5 +- dev/SapphireTest.php | 5 ++ docs/en/changelogs/rc/3.2.0.md | 58 ++++++++++++++++++- docs/en/howto/csv-import.md | 5 +- docs/en/topics/javascript.md | 2 +- tests/control/RequestHandlingTest.php | 54 ++++++++--------- tests/forms/FormTest.php | 15 +++-- .../gridfield/GridFieldDetailFormTest.php | 2 +- 8 files changed, 107 insertions(+), 39 deletions(-) diff --git a/_config/routes.yml b/_config/routes.yml index 32161642e..df5b5ac33 100644 --- a/_config/routes.yml +++ b/_config/routes.yml @@ -15,12 +15,15 @@ After: Director: rules: 'Security//$Action/$ID/$OtherID': 'Security' - '$Controller//$Action/$ID/$OtherID': '*' 'api/v1/live': 'VersionedRestfulServer' 'api/v1': 'RestfulServer' 'soap/v1': 'SOAPModelAccess' 'dev': 'DevelopmentAdmin' 'interactive': 'SapphireREPL' + 'InstallerTest//$Action/$ID/$OtherID': 'InstallerTest' + 'JSTestRunner//$Action/$ID/$OtherID': 'JSTestRunner' + 'SapphireInfo//$Action/$ID/$OtherID': 'SapphireInfo' + 'SapphireREPL//$Action/$ID/$OtherID': 'SapphireREPL' --- Name: adminroutes Before: '*' diff --git a/dev/SapphireTest.php b/dev/SapphireTest.php index 13561c12b..be770bf55 100644 --- a/dev/SapphireTest.php +++ b/dev/SapphireTest.php @@ -197,6 +197,11 @@ class SapphireTest extends PHPUnit_Framework_TestCase { Security::$database_is_ready = null; $this->originalTheme = SSViewer::current_theme(); + + // Add controller-name auto-routing + Config::inst()->update('Director', 'rules', array( + '$Controller//$Action/$ID/$OtherID' => '*' + )); if(class_exists('SiteTree')) { // Save nested_urls state, so we can restore it later diff --git a/docs/en/changelogs/rc/3.2.0.md b/docs/en/changelogs/rc/3.2.0.md index fdd7f633b..86af6d011 100644 --- a/docs/en/changelogs/rc/3.2.0.md +++ b/docs/en/changelogs/rc/3.2.0.md @@ -1,11 +1,63 @@ # 3.2.0 (unreleased) -## Overview ## +## Overview -### CMS ### +### CMS -* Moved SS_Report and ReportAdmin out to a separate module. If you're using + * Moved SS_Report and ReportAdmin out to a separate module. If you're using composer or downloading a release, this module should be included for you. Otherwise, you'll need to include the module yourself (https://github.com/silverstripe-labs/silverstripe-reports) +### Framework + + * API: Removed URL routing by controller name + +## Details + +### API: Removed URL routing by controller name + +The auto-routing of controller class names to URL endpoints +has been removed (rule: `'$Controller//$Action/$ID/$OtherID': '*'`). +This increases clarity in routing since it makes URL entpoints explicit, +and thereby simplifies system and security reviews. + +Please access any custom controllers exclusively through self-defined +[routes](/reference/director). For controllers extending `Page_Controller`, +simply use the provided page URLs. + + :::php + class MyController extends Controller { + static $allowed_actions = array('myaction'); + public function myaction($request) { + // ... + } + } + +Create a new file `mysite/_config/routes.yml` +(read more about the [config format](/topics/configuration)). +Your controller is now available on `http://yourdomain.com/my-controller-endpoint`, +after refreshing the configuration cache through `?flush=all`. + + :::yaml + --- + Name: my-routes + After: framework/routes#coreroutes + --- + Director: + rules: + 'my-controller-endpoint//$Action' : 'MyController' + + +The auto-routing is still in place for unit tests, +since its a frequently used feature there. Although we advise against it, +you can reinstate the old behaviour through a director rule: + + :::yaml + --- + Name: my-routes + After: framework/routes#coreroutes + --- + Director: + rules: + '$Controller//$Action/$ID/$OtherID': '*' \ No newline at end of file diff --git a/docs/en/howto/csv-import.md b/docs/en/howto/csv-import.md index 5c8253397..3cef56c83 100644 --- a/docs/en/howto/csv-import.md +++ b/docs/en/howto/csv-import.md @@ -63,7 +63,10 @@ below the search form on the left. ## Import through a custom controller -You can have more customized logic and interface feedback through a custom controller. Let's create a simple upload form (which is used for `MyDataObject` instances). You can access it through `http://localhost/MyController/?flush=all`. +You can have more customized logic and interface feedback through a custom controller. +Let's create a simple upload form (which is used for `MyDataObject` instances). +You'll need to add a route to your controller to make it accessible via URL +(see [director](/reference/director)). :::php -
+
diff --git a/tests/control/RequestHandlingTest.php b/tests/control/RequestHandlingTest.php index b2db41446..95e83739d 100644 --- a/tests/control/RequestHandlingTest.php +++ b/tests/control/RequestHandlingTest.php @@ -5,7 +5,35 @@ * We've set up a simple URL handling model based on */ class RequestHandlingTest extends FunctionalTest { + static $fixture_file = null; + + public function setUp() { + parent::setUp(); + + Config::inst()->update('Director', 'rules', array( + // If we don't request any variables, then the whole URL will get shifted off. This is fine, but it means that the + // controller will have to parse the Action from the URL itself. + 'testGoodBase1' => "RequestHandlingTest_Controller", + + // The double-slash indicates how much of the URL should be shifted off the stack. This is important for dealing + // with nested request handlers appropriately. + 'testGoodBase2//$Action/$ID/$OtherID' => "RequestHandlingTest_Controller", + + // By default, the entire URL will be shifted off. This creates a bit of backward-incompatability, but makes the + // URL rules much more explicit. + 'testBadBase/$Action/$ID/$OtherID' => "RequestHandlingTest_Controller", + + // Rules with an extension always default to the index() action + 'testBaseWithExtension/virtualfile.xml' => "RequestHandlingTest_Controller", + + // Without the extension, the methodname should be matched + 'testBaseWithExtension//$Action/$ID/$OtherID' => "RequestHandlingTest_Controller", + + // Test nested base + 'testParentBase/testChildBase//$Action/$ID/$OtherID' => "RequestHandlingTest_Controller", + )); + } // public function testRequestHandlerChainingLatestParams() { // $c = new RequestHandlingTest_Controller(); @@ -265,32 +293,6 @@ class RequestHandlingTest extends FunctionalTest { } -/** - * Director rules for the test - */ -Config::inst()->update('Director', 'rules', array( - // If we don't request any variables, then the whole URL will get shifted off. This is fine, but it means that the - // controller will have to parse the Action from the URL itself. - 'testGoodBase1' => "RequestHandlingTest_Controller", - - // The double-slash indicates how much of the URL should be shifted off the stack. This is important for dealing - // with nested request handlers appropriately. - 'testGoodBase2//$Action/$ID/$OtherID' => "RequestHandlingTest_Controller", - - // By default, the entire URL will be shifted off. This creates a bit of backward-incompatability, but makes the - // URL rules much more explicit. - 'testBadBase/$Action/$ID/$OtherID' => "RequestHandlingTest_Controller", - - // Rules with an extension always default to the index() action - 'testBaseWithExtension/virtualfile.xml' => "RequestHandlingTest_Controller", - - // Without the extension, the methodname should be matched - 'testBaseWithExtension//$Action/$ID/$OtherID' => "RequestHandlingTest_Controller", - - // Test nested base - 'testParentBase/testChildBase//$Action/$ID/$OtherID' => "RequestHandlingTest_Controller", -)); - /** * Controller for the test */ diff --git a/tests/forms/FormTest.php b/tests/forms/FormTest.php index c4e31142c..717eb4311 100644 --- a/tests/forms/FormTest.php +++ b/tests/forms/FormTest.php @@ -11,6 +11,14 @@ class FormTest extends FunctionalTest { 'FormTest_Player', 'FormTest_Team', ); + + function setUp() { + parent::setUp(); + + Config::inst()->update('Director', 'rules', array( + 'FormTest_Controller' => 'FormTest_Controller' + )); + } public function testLoadDataFromRequest() { $form = new Form( @@ -536,9 +544,4 @@ class FormTest_ControllerWithSecurityToken extends Controller implements TestOnl public function getViewer($action = null) { return new SSViewer('BlankPage'); } -} - -Config::inst()->update('Director', 'rules', array( - 'FormTest_Controller' => 'FormTest_Controller' -)); - +} \ No newline at end of file diff --git a/tests/forms/gridfield/GridFieldDetailFormTest.php b/tests/forms/gridfield/GridFieldDetailFormTest.php index 6c50b2e5c..e3f766b39 100644 --- a/tests/forms/gridfield/GridFieldDetailFormTest.php +++ b/tests/forms/gridfield/GridFieldDetailFormTest.php @@ -327,7 +327,7 @@ class GridFieldDetailFormTest_Controller extends Controller implements TestOnly class GridFieldDetailFormTest_GroupController extends Controller implements TestOnly { protected $template = 'BlankPage'; - + public function Form() { $field = new GridField('testfield', 'testfield', GridFieldDetailFormTest_PeopleGroup::get()->sort('Name')); $field->getConfig()->addComponent($gridFieldForm = new GridFieldDetailForm($this, 'Form'));