Skip to content

Commit

Permalink
API Strongly-type action method signatures
Browse files Browse the repository at this point in the history
  • Loading branch information
emteknetnz committed Oct 11, 2022
1 parent 83a09da commit 82eee17
Show file tree
Hide file tree
Showing 17 changed files with 69 additions and 95 deletions.
30 changes: 8 additions & 22 deletions src/Control/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,8 @@ class Controller extends RequestHandler implements TemplateGlobalProvider
* The response object that the controller returns.
*
* Set in {@link handleRequest()}.
*
* @var HTTPResponse
*/
protected $response;
protected HTTPResponse $response;

/**
* Default URL handlers.
Expand Down Expand Up @@ -142,12 +140,11 @@ public function doInit()
*
* Also set the URLParams
*/
public function setRequest($request)
public function setRequest(HTTPRequest $request): static
{
$return = parent::setRequest($request);
parent::setRequest($request);
$this->setURLParams($this->getRequest()->allParams());

return $return;
return $this;
}

/**
Expand Down Expand Up @@ -192,11 +189,8 @@ protected function afterHandleRequest()
* Important: If you are going to overload handleRequest,
* make sure that you start the method with $this->beforeHandleRequest()
* and end the method with $this->afterHandleRequest()
*
* @param HTTPRequest $request
* @return HTTPResponse
*/
public function handleRequest(HTTPRequest $request)
public function handleRequest(HTTPRequest $request): HTTPResponse
{
if (!$request) {
throw new \RuntimeException('Controller::handleRequest() not passed a request!');
Expand Down Expand Up @@ -332,14 +326,10 @@ public function getURLParams()
/**
* Returns the HTTPResponse object that this controller is building up. Can be used to set the
* status code and headers.
*
* @return HTTPResponse
*/
public function getResponse()
public function getResponse(): HTTPResponse
{
if (!$this->response) {
$this->setResponse(new HTTPResponse());
}
$this->response ??= HTTPResponse::create();
return $this->response;
}

Expand Down Expand Up @@ -628,12 +618,8 @@ public function popCurrent()

/**
* Redirect to the given URL.
*
* @param string $url
* @param int $code
* @return HTTPResponse
*/
public function redirect($url, $code = 302)
public function redirect(string $url, int $code = 302): HTTPResponse
{
if ($this->getResponse()->getHeader('Location') && $this->getResponse()->getHeader('Location') != $url) {
user_error("Already directed to " . $this->getResponse()->getHeader('Location')
Expand Down
8 changes: 1 addition & 7 deletions src/Control/HTTPResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -310,13 +310,7 @@ public function removeHeader($header)
return $this;
}

/**
* @param string $dest
* @param int $code
*
* @return $this
*/
public function redirect($dest, $code = 302)
public function redirect(string $dest, int $code = 302): static
{
if (!in_array($code, self::$redirect_codes)) {
trigger_error("Invalid HTTP redirect code {$code}", E_USER_WARNING);
Expand Down
16 changes: 6 additions & 10 deletions src/Control/PjaxResponseNegotiator.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class PjaxResponseNegotiator
*/
protected $callbacks = [];

protected $response = null;
protected HTTPResponse $response;

/**
* Overridden fragments (if any). Otherwise uses fragments from the request.
Expand All @@ -41,21 +41,18 @@ class PjaxResponseNegotiator
* @param array $callbacks
* @param HTTPResponse $response An existing response to reuse (optional)
*/
public function __construct($callbacks = [], $response = null)
public function __construct($callbacks = [], HTTPResponse $response = null)
{
$this->callbacks = $callbacks;
$this->response = $response;
$this->response = $response ?: HTTPResponse::create();
}

public function getResponse()
public function getResponse(): HTTPResponse
{
if (!$this->response) {
$this->response = new HTTPResponse();
}
return $this->response;
}

public function setResponse($response)
public function setResponse(HTTPResponse $response)
{
$this->response = $response;
}
Expand All @@ -68,10 +65,9 @@ public function setResponse($response)
* @param array $extraCallbacks List of anonymous functions or callables returning either a string
* or HTTPResponse, keyed by their fragment identifier. The 'default' key can
* be used as a fallback for non-ajax responses.
* @return HTTPResponse
* @throws HTTPResponse_Exception
*/
public function respond(HTTPRequest $request, $extraCallbacks = [])
public function respond(HTTPRequest $request, $extraCallbacks = []): HTTPResponse
{
// Prepare the default options and combine with the others
$callbacks = array_merge($this->callbacks, $extraCallbacks);
Expand Down
17 changes: 4 additions & 13 deletions src/Control/RequestHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class RequestHandler extends ViewableData
* @var HTTPRequest $request The request object that the controller was called with.
* Set in {@link handleRequest()}. Useful to generate the {}
*/
protected $request = null;
protected HTTPRequest $request;

/**
* The DataModel for this request
Expand Down Expand Up @@ -543,11 +543,8 @@ public function getRequest()
/**
* Typically the request is set through {@link handleAction()}
* or {@link handleRequest()}, but in some based we want to set it manually.
*
* @param HTTPRequest $request
* @return $this
*/
public function setRequest($request)
public function setRequest(HTTPRequest $request): static
{
$this->request = $request;
return $this;
Expand Down Expand Up @@ -581,12 +578,8 @@ public function Link($action = null)

/**
* Redirect to the given URL.
*
* @param string $url
* @param int $code
* @return HTTPResponse
*/
public function redirect($url, $code = 302)
public function redirect(string $url, int $code = 302): HTTPResponse
{
$url = Director::absoluteURL($url);
$response = new HTTPResponse();
Expand Down Expand Up @@ -655,10 +648,8 @@ public function getReferer()
* URL (see {@link Director::baseURL()}).
*
* @uses redirect()
*
* @return HTTPResponse
*/
public function redirectBack()
public function redirectBack(): HTTPResponse
{
// Prefer to redirect to ?BackURL, but fall back to Referer header
// As a last resort redirect to base url
Expand Down
4 changes: 3 additions & 1 deletion src/Dev/DevBuildController.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

use SilverStripe\Control\Controller;
use SilverStripe\Control\Director;
use SilverStripe\Control\HTTPRequest;
use SilverStripe\Control\HTTPResponse;
use SilverStripe\ORM\DatabaseAdmin;

class DevBuildController extends Controller
Expand All @@ -17,7 +19,7 @@ class DevBuildController extends Controller
'build'
];

public function build($request)
public function build(HTTPRequest $request): HTTPResponse
{
if (Director::is_cli()) {
$da = DatabaseAdmin::create();
Expand Down
8 changes: 5 additions & 3 deletions src/Forms/FileField.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,22 @@
* <code>
* class ExampleFormController extends PageController {
*
* function Form() {
* public function Form(): Form
* {
* $fields = new FieldList(
* new TextField('MyName'),
* new FileField('MyFile')
* );
* $actions = new FieldList(
* new FormAction('doUpload', 'Upload file')
* );
* $validator = new RequiredFields(['MyName', 'MyFile']);
* $validator = new RequiredFields(['MyName', 'MyFile']);
*
* return new Form($this, 'Form', $fields, $actions, $validator);
* }
*
* function doUpload($data, $form) {
* public function doUpload((array $data, Form $form): HTTPResponse
* {
* $file = $data['MyFile'];
* $content = file_get_contents($file['tmp_name']);
* // ... process content
Expand Down
20 changes: 7 additions & 13 deletions src/Forms/FormRequestHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public function Link($action = null)
* if the form is valid.
*
* @param HTTPRequest $request
* @return HTTPResponse
* @return mixed
* @throws HTTPResponse_Exception
*/
public function httpSubmission($request)
Expand Down Expand Up @@ -200,7 +200,7 @@ public function httpSubmission($request)
// buttonClicked() validates that the action set above is valid
&& !$this->buttonClicked()
) {
return $this->httpError(
$this->httpError(
403,
sprintf('Action "%s" not allowed on controller (Class: %s)', $funcName, get_class($controller))
);
Expand All @@ -209,7 +209,7 @@ public function httpSubmission($request)
$this->hasMethod($funcName)
&& !$this->checkAccessAction($funcName)
) {
return $this->httpError(
$this->httpError(
403,
sprintf('Action "%s" not allowed on form request handler (Class: "%s")', $funcName, static::class)
);
Expand Down Expand Up @@ -261,7 +261,7 @@ public function httpSubmission($request)
);
}

return $this->httpError(404, "Could not find a suitable form-action callback function");
$this->httpError(404, "Could not find a suitable form-action callback function");
}

/**
Expand Down Expand Up @@ -299,11 +299,8 @@ public function checkAccessAction($action)
* handles 'application/json' requests with a JSON object containing the error messages.
* Behaviour can be influenced by setting {@link $redirectToFormOnValidationError},
* and can be overruled by setting {@link $validationResponseCallback}.
*
* @param ValidationResult $result
* @return HTTPResponse
*/
protected function getValidationErrorResponse(ValidationResult $result)
protected function getValidationErrorResponse(ValidationResult $result): HTTPResponse
{
// Check for custom handling mechanism
$callback = $this->form->getValidationResponseCallback();
Expand All @@ -329,10 +326,8 @@ protected function getValidationErrorResponse(ValidationResult $result)

/**
* Redirect back to this form with an added #anchor link
*
* @return HTTPResponse
*/
public function redirectBackToForm()
public function redirectBackToForm(): HTTPResponse
{
$pageURL = $this->getReturnReferer();
if (!$pageURL) {
Expand Down Expand Up @@ -369,9 +364,8 @@ protected function addBackURLParam($link)
*
* @internal called from {@see Form::getValidationErrorResponse}
* @param ValidationResult $result
* @return HTTPResponse
*/
protected function getAjaxErrorResponse(ValidationResult $result)
protected function getAjaxErrorResponse(ValidationResult $result): HTTPResponse
{
// Ajax form submissions accept json encoded errors by default
$acceptType = $this->getRequest()->getHeader('Accept');
Expand Down
7 changes: 1 addition & 6 deletions src/Security/MemberAuthenticator/LostPasswordHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,8 @@ public function redirectToLostPassword()
* the logic, by returning FALSE. In this case, the user will be redirected back
* to the form without further action. It is recommended to set a message
* in the form detailing why the action was denied.
*
* @skipUpgrade
* @param array $data Submitted data
* @param LostPasswordForm $form
* @return HTTPResponse
*/
public function forgotPassword($data, $form)
public function forgotPassword(array $data, Form $form): HTTPResponse
{
// Run a first pass validation check on the data
$dataValidation = $this->validateForgotPasswordData($data, $form);
Expand Down
6 changes: 4 additions & 2 deletions tests/php/Control/Middleware/Control/TestController.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
namespace SilverStripe\Control\Tests\Middleware\Control;

use SilverStripe\Control\Controller;
use SilverStripe\Control\HTTPRequest;
use SilverStripe\Control\HTTPResponse;

class TestController extends Controller
{
public function index($request)
public function index(HTTPRequest $request): HTTPResponse
{
return "Success";
return HTTPResponse::create()->setBody("Success");
}

public function Link($action = null)
Expand Down
10 changes: 6 additions & 4 deletions tests/php/Control/RequestHandlingTest/TestFormHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

namespace SilverStripe\Control\Tests\RequestHandlingTest;

use SilverStripe\Control\HTTPRequest;
use SilverStripe\Control\HTTPResponse;
use SilverStripe\Forms\FormRequestHandler;

/**
Expand All @@ -29,13 +31,13 @@ public function handleField($request)
return $this->form->Fields()->dataFieldByName($request->param('FieldName'));
}

public function handleSubmission($request)
public function handleSubmission(HTTPRequest $request): HTTPResponse
{
return "Form posted";
return HTTPResponse::create()->setBody("Form posted");
}

public function handleGet($request)
public function handleGet(HTTPRequest $request): HTTPResponse
{
return "Get request on form";
return HTTPResponse::create()->setBody("Get request on form");
}
}
9 changes: 7 additions & 2 deletions tests/php/Forms/FormFactoryTest/ControllerExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@
namespace SilverStripe\Forms\Tests\FormFactoryTest;

use SilverStripe\Control\Controller;
use SilverStripe\Control\HTTPRequest;
use SilverStripe\Control\HTTPResponse;
use SilverStripe\Core\Convert;
use SilverStripe\Core\Extension;
use SilverStripe\Forms\FieldList;
use SilverStripe\Forms\Form;
use SilverStripe\Forms\FormAction;
use SilverStripe\Forms\LiteralField;
use SilverStripe\ORM\DataObject;
Expand Down Expand Up @@ -75,13 +78,15 @@ public function updateFormFields(FieldList &$fields, Controller $controller, $na
}
}

public function publish($data, $form)
public function publish(array $data, Form $form): HTTPResponse
{
// noop
return HTTPResponse::create();
}

public function preview()
public function preview(HTTPRequest $request): HTTPResponse
{
// noop
return HTTPResponse::create();
}
}
Loading

0 comments on commit 82eee17

Please sign in to comment.