Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WIP: Make state properties in controller readonly #3283

Closed
wants to merge 2 commits into from
Closed

WIP: Make state properties in controller readonly #3283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
72700b5
!!! TASK: Make `AbstractController` $request and $response readonly
mhsdesign Jan 26, 2024
f5e8497
TASK: Remove @Flow\Scope("singleton") from controllers
mhsdesign Jan 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions Neos.Flow/Classes/Mvc/Controller/AbstractController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,14 @@
* @var ActionRequest
* @api
*/
protected $request;
protected readonly ActionRequest $request;

Check failure on line 63 in Neos.Flow/Classes/Mvc/Controller/AbstractController.php

View workflow job for this annotation

GitHub Actions / PHP 8.2 Test static analysis (deps: highest) 

Class Neos\Flow\Mvc\Controller\AbstractController has an uninitialized readonly property $request. Assign it in the constructor.

Check failure on line 63 in Neos.Flow/Classes/Mvc/Controller/AbstractController.php

View workflow job for this annotation

GitHub Actions / PHP 8.2 Test static analysis (deps: highest) 

Class Neos\Flow\Mvc\Controller\AbstractController has an uninitialized readonly property $request. Assign it in the constructor.

/**
* The response which will be returned by this action controller
* @var ActionResponse
* @api
*/
protected $response;
protected readonly ActionResponse $response;

Check failure on line 70 in Neos.Flow/Classes/Mvc/Controller/AbstractController.php

View workflow job for this annotation

GitHub Actions / PHP 8.2 Test static analysis (deps: highest) 

Class Neos\Flow\Mvc\Controller\AbstractController has an uninitialized readonly property $response. Assign it in the constructor.

Check failure on line 70 in Neos.Flow/Classes/Mvc/Controller/AbstractController.php

View workflow job for this annotation

GitHub Actions / PHP 8.2 Test static analysis (deps: highest) 

Class Neos\Flow\Mvc\Controller\AbstractController has an uninitialized readonly property $response. Assign it in the constructor.

/**
* Arguments passed to the controller
Expand Down
2 changes: 1 addition & 1 deletion Neos.Flow/Classes/Mvc/Controller/ActionController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ protected function callActionMethod(ActionRequest $request, ActionResponse $resp
}

if ($actionResult === null && $this->view instanceof ViewInterface) {
$this->response = $this->renderView($this->response);
$this->renderView($this->response)->mergeIntoParentResponse($this->response);
} else {
$this->response->setContent($actionResult);
}
Expand Down
12 changes: 0 additions & 12 deletions Neos.Flow/Classes/Mvc/Controller/RestController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,6 @@
*/
class RestController extends ActionController
{
/**
* The current request
* @var ActionRequest
*/
protected $request;

/**
* The response which will be returned by this action controller
* @var ActionResponse
*/
protected $response;

/**
* Name of the action method argument which acts as the resource for the
* RESTful controller. If an argument with the specified name is passed
Expand Down
1 change: 0 additions & 1 deletion Neos.Flow/Tests/Functional/Mvc/Fixtures/Controller/ActionControllerTestAController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
/**
* A controller fixture
*
* @Flow\Scope("singleton")
*/
class ActionControllerTestAController extends ActionController
{
Expand Down
1 change: 0 additions & 1 deletion Neos.Flow/Tests/Functional/Mvc/Fixtures/Controller/ActionControllerTestBController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
/**
* An action controller test fixture
*
* @Flow\Scope("singleton")
*/
class ActionControllerTestBController extends ActionController
{
Expand Down
1 change: 0 additions & 1 deletion Neos.Flow/Tests/Functional/Mvc/Fixtures/Controller/RoutingTestAController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
/**
* A controller fixture
*
* @Flow\Scope("singleton")
*/
class RoutingTestAController extends ActionController
{
Expand Down
1 change: 0 additions & 1 deletion Neos.Flow/Tests/Functional/Mvc/Fixtures/Controller/StandardController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
/**
* A controller fixture
*
* @Flow\Scope("singleton")
*/
class StandardController extends ActionController
{
Expand Down
1 change: 0 additions & 1 deletion Neos.Flow/Tests/Functional/Mvc/Fixtures/Controller/UriBuilderController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
/**
* part of the test fixture for {@see UriBuilderTest}
*
* @Flow\Scope("singleton")
*/
class UriBuilderController extends ActionController
{
Expand Down
33 changes: 9 additions & 24 deletions Neos.Flow/Tests/Unit/Mvc/Controller/ActionControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,10 @@ protected function setUp(): void
$this->mockRequest->expects(self::any())->method('getFormat')->will(self::returnValue('theFormat'));
$this->mockRequest->expects(self::any())->method('getControllerName')->will(self::returnValue('TheController'));
$this->mockRequest->expects(self::any())->method('getControllerActionName')->will(self::returnValue('theAction'));
$this->inject($this->actionController, 'request', $this->mockRequest);

$this->mockObjectManager = $this->createMock(ObjectManagerInterface::class);
$this->inject($this->actionController, 'objectManager', $this->mockObjectManager);

$this->mockControllerContext = $this->getMockBuilder(Mvc\Controller\ControllerContext::class)->disableOriginalConstructor()->getMock();
$this->inject($this->actionController, 'controllerContext', $this->mockControllerContext);

$this->mockViewConfigurationManager = $this->createMock(Mvc\ViewConfigurationManager::class);
$this->inject($this->actionController, 'viewConfigurationManager', $this->mockViewConfigurationManager);
}
Expand Down Expand Up @@ -153,9 +149,7 @@ public function processRequestThrowsExceptionIfRequestedActionIsNotCallable()
$mockHttpRequest = $this->getMockBuilder(ServerRequestInterface::class)->disableOriginalConstructor()->getMock();
$mockRequest->expects(self::any())->method('getHttpRequest')->will(self::returnValue($mockHttpRequest));

$mockResponse = new Mvc\ActionResponse;

$this->actionController->processRequest($mockRequest, $mockResponse);
$this->actionController->processRequest($mockRequest);
}

/**
Expand Down Expand Up @@ -193,37 +187,30 @@ public function processRequestThrowsExceptionIfRequestedActionIsNotPublic()
$mockHttpRequest = $this->getMockBuilder(ServerRequestInterface::class)->disableOriginalConstructor()->getMock();
$mockRequest->expects(self::any())->method('getHttpRequest')->will(self::returnValue($mockHttpRequest));

$mockResponse = new Mvc\ActionResponse;

$this->actionController->processRequest($mockRequest, $mockResponse);
$this->actionController->processRequest($mockRequest);
}

/**
* @test
*/
public function processRequestInjectsControllerContextToView()
{
$this->actionController = $this->getAccessibleMock(ActionController::class, ['resolveActionMethodName', 'initializeActionMethodArguments', 'initializeActionMethodValidators', 'resolveView', 'callActionMethod', 'initializeController']);
$this->actionController = $this->getAccessibleMock(ActionController::class, ['resolveActionMethodName', 'initializeActionMethodArguments', 'initializeActionMethodValidators', 'resolveView', 'callActionMethod']);
$this->actionController->method('resolveActionMethodName')->willReturn('indexAction');

$this->inject($this->actionController, 'objectManager', $this->mockObjectManager);
$this->inject($this->actionController, 'controllerContext', $this->mockControllerContext);
$this->inject($this->actionController, 'request', $this->mockRequest);
$mockResponse = new Mvc\ActionResponse;
$mockResponse->setContentType('text/plain');

$this->inject($this->actionController, 'arguments', new Arguments([]));
$this->inject($this->actionController, 'objectManager', $this->mockObjectManager);

$mockMvcPropertyMappingConfigurationService = $this->createMock(Mvc\Controller\MvcPropertyMappingConfigurationService::class);
$this->inject($this->actionController, 'mvcPropertyMappingConfigurationService', $mockMvcPropertyMappingConfigurationService);

$mockHttpRequest = $this->getMockBuilder(ServerRequestInterface::class)->disableOriginalConstructor()->getMock();
$mockHttpRequest = $this->getMockBuilder(ServerRequestInterface::class)->getMock();
$this->mockRequest->expects(self::any())->method('getHttpRequest')->will(self::returnValue($mockHttpRequest));

$mockResponse = new Mvc\ActionResponse;
$mockResponse->setContentType('text/plain');
$this->inject($this->actionController, 'response', $mockResponse);

$mockView = $this->createMock(Mvc\View\ViewInterface::class);
$mockView->expects(self::once())->method('setControllerContext')->with($this->mockControllerContext);
$mockView->expects(self::once())->method('setControllerContext')->with(self::callback(fn(Mvc\Controller\ControllerContext $controllerContext) => $controllerContext->getRequest() === $this->mockRequest));
$this->actionController->expects(self::once())->method('resolveView')->with($this->mockRequest)->will(self::returnValue($mockView));
$this->actionController->expects(self::once())->method('callActionMethod')->willReturn($mockResponse);
$this->actionController->expects(self::once())->method('resolveActionMethodName')->with($this->mockRequest)->will(self::returnValue('someAction'));
Expand Down Expand Up @@ -342,13 +329,11 @@ public function processRequestUsesContentTypeFromRenderedView($supportedMediaTyp
$mockMvcPropertyMappingConfigurationService = $this->createMock(Mvc\Controller\MvcPropertyMappingConfigurationService::class);
$this->inject($this->actionController, 'mvcPropertyMappingConfigurationService', $mockMvcPropertyMappingConfigurationService);

$mockHttpRequest = $this->getMockBuilder(ServerRequestInterface::class)->disableOriginalConstructor()->getMock();
$mockHttpRequest = $this->getMockBuilder(ServerRequestInterface::class)->getMock();
$mockHttpRequest->method('getHeaderLine')->with('Accept')->willReturn('application/xml');
$mockHttpRequest->method('getHeaderLine')->with('Accept')->willReturn('application/xml');
$this->mockRequest->method('getHttpRequest')->willReturn($mockHttpRequest);

$mockResponse = new Mvc\ActionResponse;

$this->inject($this->actionController, 'supportedMediaTypes', ['application/xml']);

$mockView = $this->createMock(Mvc\View\ViewInterface::class);
Expand Down