tonyair
/
silverstripe-cms
mirror of https://github.com/silverstripe/silverstripe-cms
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #2788 from creative-commoners/pulls/5/action-signature 

API Strongly-type action method signatures
This commit is contained in:
Guy Sartorelli 2022-10-19 10:08:22 +13:00 committed by GitHub
parent 9b64c7de24 6e19ae737f
commit 8320023526
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 55 additions and 105 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
code/BatchActions/CMSBatchAction_Archive.php
View File

@ -4,6 +4,7 @@ namespace SilverStripe\CMS\BatchActions;
use SilverStripe\ORM\SS_List;
use SilverStripe\Admin\CMSBatchAction;
use SilverStripe\Control\HTTPResponse;
/**
* Delete items batch action.
@ -15,7 +16,7 @@ class CMSBatchAction_Archive extends CMSBatchAction
return _t(__CLASS__ . '.TITLE', 'Unpublish and archive');
}
public function run(SS_List $pages)
public function run(SS_List $pages): HTTPResponse
{
return $this->batchaction(
$pages,

3
code/BatchActions/CMSBatchAction_Publish.php
View File

@ -3,6 +3,7 @@
namespace SilverStripe\CMS\BatchActions;
use SilverStripe\Admin\CMSBatchAction;
use SilverStripe\Control\HTTPResponse;
use SilverStripe\ORM\SS_List;
/**
@ -15,7 +16,7 @@ class CMSBatchAction_Publish extends CMSBatchAction
return _t(__CLASS__ . '.PUBLISH_PAGES', 'Publish');
}
public function run(SS_List $pages)
public function run(SS_List $pages): HTTPResponse
{
return $this->batchaction(
$pages,

3
code/BatchActions/CMSBatchAction_Restore.php
View File

@ -4,6 +4,7 @@ namespace SilverStripe\CMS\BatchActions;
use SilverStripe\Admin\CMSBatchAction;
use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\Control\HTTPResponse;
use SilverStripe\ORM\ArrayList;
use SilverStripe\ORM\SS_List;
use SilverStripe\Versioned\Versioned;
@ -20,7 +21,7 @@ class CMSBatchAction_Restore extends CMSBatchAction
return _t(__CLASS__ . '.RESTORE', 'Restore');
}
public function run(SS_List $pages)
public function run(SS_List $pages): HTTPResponse
{
// Sort pages by depth
$pageArray = $pages->toArray();

3
code/BatchActions/CMSBatchAction_Unpublish.php
View File

@ -3,6 +3,7 @@
namespace SilverStripe\CMS\BatchActions;
use SilverStripe\Admin\CMSBatchAction;
use SilverStripe\Control\HTTPResponse;
use SilverStripe\ORM\SS_List;
/**
@ -15,7 +16,7 @@ class CMSBatchAction_Unpublish extends CMSBatchAction
return _t(__CLASS__ . '.UNPUBLISH_PAGES', 'Unpublish');
}
public function run(SS_List $pages)
public function run(SS_List $pages): HTTPResponse
{
return $this->batchaction(
$pages,

84
code/Controllers/CMSMain.php
View File

@ -23,6 +23,7 @@ use SilverStripe\Control\Director;
use SilverStripe\Control\HTTPRequest;
use SilverStripe\Control\HTTPResponse;
use SilverStripe\Control\HTTPResponse_Exception;
use SilverStripe\Control\PjaxResponseNegotiator;
use SilverStripe\Core\Cache\MemberCacheFlusher;
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Convert;
@ -204,7 +205,7 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
CMSBatchActionHandler::register('publish', CMSBatchAction_Publish::class);
}
public function index($request)
public function index(HTTPRequest $request): HTTPResponse
{
// In case we're not showing a specific record, explicitly remove any session state,
// to avoid it being highlighted in the tree, and causing an edit form to show.
@ -215,7 +216,7 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
return parent::index($request);
}
public function getResponseNegotiator()
public function getResponseNegotiator(): PjaxResponseNegotiator
{
$negotiator = parent::getResponseNegotiator();
@ -663,11 +664,8 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
/**
* Get a subtree underneath the request param 'ID'.
* If ID = 0, then get the whole tree.
*
* @param HTTPRequest $request
* @return string
*/
public function getsubtree($request)
public function getsubtree(HTTPRequest $request): HTTPResponse
{
$html = $this->getSiteTreeFor(
$this->config()->get('tree_class'),
@ -682,7 +680,7 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
$html = preg_replace('/^[\s\t\r\n]*<ul[^>]*>/', '', $html ?? '');
$html = preg_replace('/<\/ul[^>]*>[\s\t\r\n]*$/', '', $html ?? '');
return $html;
return $this->getResponse()->setBody($html);
}
/**
@ -690,11 +688,8 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
* Similar to {@link getsubtree()}, but doesn't enforce loading
* all children with the node. Useful to refresh views after
* state modifications, e.g. saving a form.
*
* @param HTTPRequest $request
* @return HTTPResponse
*/
public function updatetreenodes($request)
public function updatetreenodes(HTTPRequest $request): HTTPResponse
{
$data = [];
$ids = explode(',', $request->getVar('ids') ?? '');
@ -762,17 +757,15 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
* - 'SiblingIDs': Array of all sibling nodes to the moved node (incl. the node itself).
* In case of a 'ParentID' change, relates to the new siblings under the new parent.
*
* @param HTTPRequest $request
* @return HTTPResponse JSON string with a
* @throws HTTPResponse_Exception
*/
public function savetreenode($request)
public function savetreenode(HTTPRequest $request): HTTPResponse
{
if (!SecurityToken::inst()->checkRequest($request)) {
return $this->httpError(400);
$this->httpError(400);
}
if (!$this->CanOrganiseSitetree()) {
return $this->httpError(
$this->httpError(
403,
_t(
__CLASS__.'.CANT_REORGANISE',
@ -785,14 +778,14 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
$id = $request->requestVar('ID');
$parentID = $request->requestVar('ParentID');
if (!is_numeric($id) || !is_numeric($parentID)) {
return $this->httpError(400);
$this->httpError(400);
}
// Check record exists in the DB
/** @var SiteTree $node */
$node = DataObject::get_by_id($className, $id);
if (!$node) {
return $this->httpError(
$this->httpError(
500,
_t(
__CLASS__.'.PLEASESAVE',
@ -804,7 +797,7 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
// Check top level permissions
$root = $node->getParentType();
if (($parentID == '0' || $root == 'root') && !SiteConfig::current_site_config()->canCreateTopLevel()) {
return $this->httpError(
$this->httpError(
403,
_t(
__CLASS__.'.CANT_REORGANISE',
@ -1036,7 +1029,7 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
return $pageTypes;
}
public function doSearch($data, $form)
public function doSearch(array $data, Form $form): HTTPResponse
{
return $this->getsubtree($this->getRequest());
}
@ -1561,17 +1554,14 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
/**
* Callback to request the list of page types allowed under a given page instance.
* Provides a slower but more precise response over SiteTreeHints
*
* @param HTTPRequest $request
* @return HTTPResponse
*/
public function childfilter($request)
public function childfilter(HTTPRequest $request): HTTPResponse
{
// Check valid parent specified
$parentID = $request->requestVar('ParentID');
$parent = SiteTree::get()->byID($parentID);
if (!$parent || !$parent->exists()) {
return $this->httpError(404);
$this->httpError(404);
}
// Build hints specific to this class
@ -1758,12 +1748,9 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
/**
* Save and Publish page handler
*
* @param array $data
* @param Form $form
* @return HTTPResponse
* @throws HTTPResponse_Exception
*/
public function save($data, $form)
public function save(array $data, Form $form): HTTPResponse
{
$className = $this->config()->get('tree_class');
@ -1906,12 +1893,9 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
*
* @uses SiteTree->doRevertToLive()
*
* @param array $data
* @param Form $form
* @return HTTPResponse
* @throws HTTPResponse_Exception
*/
public function revert($data, $form)
public function revert(array $data, Form $form): HTTPResponse
{
if (!isset($data['ID'])) {
throw new HTTPResponse_Exception("Please pass an ID in the form content", 400);
@ -1959,12 +1943,9 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
*
* @see deletefromlive()
*
* @param array $data
* @param Form $form
* @return HTTPResponse
* @throws HTTPResponse_Exception
*/
public function delete($data, $form)
public function delete(array $data, Form $form): HTTPResponse
{
$id = $data['ID'];
$record = SiteTree::get()->byID($id);
@ -1994,12 +1975,9 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
/**
* Delete this page from both live and stage
*
* @param array $data
* @param Form $form
* @return HTTPResponse
* @throws HTTPResponse_Exception
*/
public function archive($data, $form)
public function archive(array $data, Form $form): HTTPResponse
{
$id = $data['ID'];
/** @var SiteTree $record */
@ -2027,14 +2005,14 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
return $this->getResponseNegotiator()->respond($this->getRequest());
}
public function publish($data, $form)
public function publish(array $data, Form $form): HTTPResponse
{
$data['publish'] = '1';
return $this->save($data, $form);
}
public function unpublish($data, $form)
public function unpublish(array $data, Form $form): HTTPResponse
{
$className = $this->config()->get('tree_class');
/** @var SiteTree $record */
@ -2171,10 +2149,8 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
/**
* @deprecated 5.0 Please use custom logic for this
* @param $request
* @return HTTPResponse|string|void
*/
public function publishall($request)
public function publishall(HTTPRequest $request): HTTPResponse
{
if (!Permission::check('ADMIN')) {
return Security::permissionFailure($this);
@ -2188,7 +2164,7 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
if (isset($this->requestParams['confirm'])) {
// Protect against CSRF on destructive action
if (!SecurityToken::inst()->checkRequest($request)) {
return $this->httpError(400);
$this->httpError(400);
}
$start = 0;
@ -2238,17 +2214,13 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
'</form>';
}
return $response;
return HTTPResponse::create()->setBody($response);
}
/**
* Restore a completely deleted page from the SiteTree_versions table.
*
* @param array $data
* @param Form $form
* @return HTTPResponse
*/
public function restore($data, $form)
public function restore(array $data, Form $form): HTTPResponse
{
if (!isset($data['ID']) || !is_numeric($data['ID'])) {
return new HTTPResponse("Please pass an ID in the form content", 400);
@ -2275,7 +2247,7 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
return $this->getResponseNegotiator()->respond($this->getRequest());
}
public function duplicate($request)
public function duplicate(HTTPRequest $request): HTTPResponse
{
// Protect against CSRF on destructive action
if (!SecurityToken::inst()->checkRequest($request)) {
@ -2319,11 +2291,11 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
return new HTTPResponse("CMSMain::duplicate() Bad ID: '$id'", 400);
}
public function duplicatewithchildren($request)
public function duplicatewithchildren(HTTPRequest $request): HTTPResponse
{
// Protect against CSRF on destructive action
if (!SecurityToken::inst()->checkRequest($request)) {
return $this->httpError(400);
$this->httpError(400);
}
Environment::increaseTimeLimitTo();
if (($id = $this->urlParams['ID']) && is_numeric($id)) {

9
code/Controllers/CMSPageAddController.php
View File

@ -192,12 +192,7 @@ class CMSPageAddController extends CMSPageEditController
return $form;
}
/**
* @param array $data
* @param Form $form
* @return HTTPResponse
*/
public function doAdd($data, $form)
public function doAdd(array $data, Form $form): HTTPResponse
{
$className = isset($data['PageType']) ? $data['PageType'] : "Page";
$parentID = isset($data['ParentID']) ? (int)$data['ParentID'] : 0;
@ -241,7 +236,7 @@ class CMSPageAddController extends CMSPageEditController
return $this->redirect(Controller::join_links($editController->Link('show'), $record->ID));
}
public function doCancel($data, $form)
public function doCancel(array $data, Form $form): HTTPResponse
{
return $this->redirect(CMSMain::singleton()->Link());
}

17
code/Controllers/CMSPageEditController.php
View File

@ -53,30 +53,27 @@ class CMSPageEditController extends CMSMain
/**
* Action handler for adding pages to a campaign
*
* @param array $data
* @param Form $form
* @return DBHTMLText|HTTPResponse
*/
public function addtocampaign($data, $form)
public function addtocampaign(array $data, Form $form): HTTPResponse
{
$id = $data['ID'];
$record = \Page::get()->byID($id);
$handler = AddToCampaignHandler::create($this, $record);
$results = $handler->addToCampaign($record, $data);
if (is_null($results)) {
return null;
$response = $handler->addToCampaign($record, $data);
$message = $response->getBody();
if (empty($message)) {
return $response;
}
if ($this->getSchemaRequested()) {
// Send extra "message" data with schema response
$extraData = ['message' => $results];
$extraData = ['message' => $message];
$schemaId = Controller::join_links($this->Link('schema/AddToCampaignForm'), $id);
return $this->getSchemaResponse($schemaId, $form, null, $extraData);
}
return $results;
return $response;
}
/**

4
code/Controllers/ContentController.php
View File

@ -187,11 +187,9 @@ class ContentController extends Controller
* This acts the same as {@link Controller::handleRequest()}, but if an action cannot be found this will attempt to
* fall over to a child controller in order to provide functionality for nested URLs.
*
* @param HTTPRequest $request
* @return HTTPResponse
* @throws HTTPResponse_Exception
*/
public function handleRequest(HTTPRequest $request)
public function handleRequest(HTTPRequest $request): HTTPResponse
{
/** @var SiteTree $child */
$child = null;

21
code/Controllers/ModelAsController.php
View File

@ -33,11 +33,9 @@ class ModelAsController extends Controller implements NestedController
* Get the appropriate {@link ContentController} for handling a {@link SiteTree} object, link it to the object and
* return it.
*
* @param SiteTree $sitetree
* @param string $action
* @return ContentController
*/
public static function controller_for(SiteTree $sitetree, $action = null)
public static function controller_for(SiteTree $sitetree, $action = null): ContentController
{
$controller = $sitetree->getControllerName();
@ -72,10 +70,8 @@ class ModelAsController extends Controller implements NestedController
/**
* @uses ModelAsController::getNestedController()
* @param HTTPRequest $request
* @return HTTPResponse
*/
public function handleRequest(HTTPRequest $request)
public function handleRequest(HTTPRequest $request): HTTPResponse
{
$this->beforeHandleRequest($request);
@ -95,14 +91,8 @@ class ModelAsController extends Controller implements NestedController
}
try {
$result = $this->getNestedController();
if ($result instanceof RequestHandler) {
$result = $result->handleRequest($this->getRequest());
} elseif (!($result instanceof HTTPResponse)) {
user_error("ModelAsController::getNestedController() returned bad object type '" .
get_class($result)."'", E_USER_WARNING);
}
$result = $this->getNestedController()->handleRequest($this->getRequest());
$result = $result;
} catch (HTTPResponse_Exception $responseException) {
$result = $responseException->getResponse();
}
@ -112,10 +102,9 @@ class ModelAsController extends Controller implements NestedController
}
/**
* @return ContentController
* @throws Exception If URLSegment not passed in as a request parameter.
*/
public function getNestedController()
public function getNestedController(): ContentController
{
$request = $this->getRequest();

3
code/Controllers/OldPageRedirector.php
View File

@ -17,10 +17,9 @@ class OldPageRedirector extends Extension
* On every URL that generates a 404, we'll capture it here and see if we can
* find an old URL that it should be redirecting to.
*
* @param HTTPRequest $request The request object
* @throws HTTPResponse_Exception
*/
public function onBeforeHTTPError404($request)
public function onBeforeHTTPError404(HTTPRequest $request)
{
// We need to get the URL ourselves because $request->allParams() only has a max of 4 params
$params = preg_split('|/+|', $request->getURL() ?? '');

6
code/Controllers/RootURLController.php
View File

@ -85,11 +85,7 @@ class RootURLController extends Controller implements Resettable
}
}
/**
* @param HTTPRequest $request
* @return HTTPResponse
*/
public function handleRequest(HTTPRequest $request)
public function handleRequest(HTTPRequest $request): HTTPResponse
{
self::$is_at_root = true;
$this->beforeHandleRequest($request);

4
tests/php/Controllers/CMSBatchActionsTest.php
View File

@ -141,7 +141,7 @@ class CMSBatchActionsTest extends SapphireTest
$this->assertEquals($archivedID, $list->first()->ParentID);
// Run restore
$result = json_decode($action->run($list) ?? '', true);
$result = json_decode($action->run($list)->getBody(), true);
$this->assertEquals(
[
$archivedxID => $archivedxID,
@ -161,7 +161,7 @@ class CMSBatchActionsTest extends SapphireTest
$this->assertEquals(0, $list->last()->ParentID); // archived (parent)
// Run restore
$result = json_decode($action->run($list) ?? '', true);
$result = json_decode($action->run($list)->getBody(), true);
$this->assertEquals(
[
// Order of archived is opposite to order items are passed in, as