Merge "Rest: Fix unspecific type hints in REST handler/validators"

includes/Rest/Handler.php

@@ -48,7 +48,7 @@ abstract class Handler {
 	/** @var array|null */
 	private $validatedParams;
 
-	/** @var mixed */
+	/** @var mixed|null */
 	private $validatedBody;
 
 	/** @var ConditionalHeaderUtil */
@@ -212,10 +212,8 @@ abstract class Handler {
 	 * @throws HttpException On validation failure.
 	 */
 	public function validate( Validator $restValidator ) {
-		$validatedParams = $restValidator->validateParams( $this->getParamSettings() );
-		$validatedBody = $restValidator->validateBody( $this->request, $this );
-		$this->validatedParams = $validatedParams;
-		$this->validatedBody = $validatedBody;
+		$this->validatedParams = $restValidator->validateParams( $this->getParamSettings() );
+		$this->validatedBody = $restValidator->validateBody( $this->request, $this );
 		$this->postValidationSetup();
 	}
 
@@ -487,7 +485,9 @@ abstract class Handler {
 	 * @stable to override
 	 *
 	 * @param string $contentType Content type of the request.
-	 * @return BodyValidator
+	 * @return BodyValidator A {@see NullBodyValidator} in this default implementation
+	 * @throws HttpException It's possible to fail early here when e.g. $contentType is unsupported,
+	 *  or later when {@see BodyValidator::validateBody} is called
 	 */
 	public function getBodyValidator( $contentType ) {
 		// TODO: Create a JsonBodyValidator if getParamSettings() returns body params.
@@ -511,7 +511,7 @@ abstract class Handler {
 
 	/**
 	 * Fetch the validated body
-	 * @return mixed Value returned by the body validator, or null if validate() was
+	 * @return mixed|null Value returned by the body validator, or null if validate() was
 	 *  not called yet, validation failed, there was no body, or the body was form data.
 	 */
 	public function getValidatedBody() {

includes/Rest/Handler/CreationHandler.php

@@ -19,11 +19,14 @@ class CreationHandler extends EditHandler {
 	 * @inheritDoc
 	 */
 	protected function getTitleParameter() {
-		return $this->getValidatedBody()['title'];
+		$body = $this->getValidatedBody();
+		'@phan-var array $body';
+		return $body['title'];
 	}
 
 	/**
 	 * @inheritDoc
+	 * @return JsonBodyValidator
 	 */
 	public function getBodyValidator( $contentType ) {
 		if ( $contentType !== 'application/json' ) {
@@ -66,6 +69,7 @@ class CreationHandler extends EditHandler {
 	 */
 	protected function getActionModuleParameters() {
 		$body = $this->getValidatedBody();
+		'@phan-var array $body';
 
 		$title = $this->getTitleParameter();
 

includes/Rest/Handler/UpdateHandler.php

@@ -55,6 +55,7 @@ class UpdateHandler extends EditHandler {
 
 	/**
 	 * @inheritDoc
+	 * @return JsonBodyValidator
 	 */
 	public function getBodyValidator( $contentType ) {
 		if ( $contentType !== 'application/json' ) {
@@ -93,6 +94,7 @@ class UpdateHandler extends EditHandler {
 	 */
 	protected function getActionModuleParameters() {
 		$body = $this->getValidatedBody();
+		'@phan-var array $body';
 
 		$title = $this->getTitleParameter();
 		$baseRevId = $body['latest']['id'] ?? 0;
@@ -188,6 +190,7 @@ class UpdateHandler extends EditHandler {
 	 */
 	private function getConflictData() {
 		$body = $this->getValidatedBody();
+		'@phan-var array $body';
 		$baseRevId = $body['latest']['id'] ?? 0;
 		$title = $this->titleParser->parseTitle( $this->getValidatedParams()['title'] );
 

includes/Rest/Router.php

@@ -536,6 +536,7 @@ class Router {
 	 *
 	 * @param Handler $handler
 	 * @return ResponseInterface
+	 * @throws HttpException
 	 */
 	private function executeHandler( $handler ): ResponseInterface {
 		ProfilingContext::singleton()->init( MW_ENTRY_POINT, $handler->getPath() );

includes/Rest/Validator/BodyValidator.php

@@ -21,7 +21,7 @@ interface BodyValidator {
 	 * available to the handler via Handler::getValidatedBody().
 	 *
 	 * @param RequestInterface $request
-	 * @return mixed
+	 * @return mixed|null
 	 * @throws HttpException on validation failure
 	 */
 	public function validateBody( RequestInterface $request );

includes/Rest/Validator/JsonBodyValidator.php

@@ -27,6 +27,10 @@ class JsonBodyValidator implements BodyValidator {
 		$this->bodyParamSettings = $bodyParamSettings;
 	}
 
+	/**
+	 * @inheritDoc
+	 * @return array
+	 */
 	public function validateBody( RequestInterface $request ) {
 		$jsonStream = $request->getBody();
 		$status = FormatJson::parse( "$jsonStream", FormatJson::FORCE_ASSOC );

includes/Rest/Validator/NullBodyValidator.php

@@ -9,6 +9,10 @@ use MediaWiki\Rest\RequestInterface;
  */
 class NullBodyValidator implements BodyValidator {
 
+	/**
+	 * @inheritDoc
+	 * @return null
+	 */
 	public function validateBody( RequestInterface $request ) {
 		return null;
 	}

includes/Rest/Validator/Validator.php

@@ -146,8 +146,8 @@ class Validator {
 	 * available to the handler via Handler::getValidatedBody().
 	 *
 	 * @param RequestInterface $request
-	 * @param Handler $handler Used to call getBodyValidator()
-	 * @return mixed May be null
+	 * @param Handler $handler Used to call {@see Handler::getBodyValidator}
+	 * @return mixed|null Return value from {@see BodyValidator::validateBody}
 	 * @throws HttpException on validation failure
 	 */
 	public function validateBody( RequestInterface $request, Handler $handler ) {

tests/phpunit/unit/includes/Rest/Handler/HandlerTest.php

@@ -233,14 +233,14 @@ class HandlerTest extends \MediaWikiUnitTestCase {
 
 	public function testGetValidatedBody() {
 		$validator = $this->createMock( Validator::class );
-		$validator->method( 'validateBody' )->willReturn( 'VALIDATED BODY' );
+		$validator->method( 'validateBody' )->willReturn( [ 'VALIDATED BODY' ] );
 
 		$handler = $this->newHandler();
 		$this->initHandler( $handler, new RequestData() );
 		$handler->validate( $validator );
 
 		$body = $handler->getValidatedBody();
-		$this->assertSame( 'VALIDATED BODY', $body );
+		$this->assertSame( [ 'VALIDATED BODY' ], $body );
 	}
 
 	public function testGetRequest() {

tests/phpunit/unit/includes/Rest/Validator/JsonBodyValidatorTest.php

@@ -60,7 +60,7 @@ class JsonBodyValidatorTest extends \MediaWikiUnitTestCase {
 	/**
 	 * @dataProvider provideValidateBody
 	 */
-	public function testValidateBody( $settings, RequestData $requestData, $expected ) {
+	public function testValidateBody( array $settings, RequestData $requestData, array $expected ) {
 		$validator = new JsonBodyValidator( $settings );
 		$actual = $validator->validateBody( $requestData );
 		$this->assertArrayEquals( $expected, $actual, false, true );