Skip to content

Commit

Permalink
[TASK] Avoid usage of object manager in RequestBuilder
Browse files Browse the repository at this point in the history 
Replace ObjectManager->get() with GeneralUtility::makeInstance()

Releases: master
Resolves: #92243
Change-Id: Idcf8436699e5d6845cab95d5332a536c156748dd
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/65651
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Benni Mack <benni@typo3.org>
Tested-by: Oliver Bartsch <bo@cedev.de>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Benni Mack <benni@typo3.org>
Reviewed-by: Oliver Klee <typo3-coding@oliverklee.de>
Reviewed-by: Oliver Bartsch <bo@cedev.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
  • Loading branch information
@alexanderschnitzler @lolli42
alexanderschnitzler authored and lolli42 committed Sep 23, 2020
1 parent 006b542 commit a554dad
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 129 deletions.
16 changes: 1 addition & 15 deletions typo3/sysext/extbase/Classes/Mvc/Web/RequestBuilder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
use TYPO3\CMS\Extbase\Mvc\Exception\InvalidActionNameException;
use TYPO3\CMS\Extbase\Mvc\Exception\InvalidControllerNameException;
use TYPO3\CMS\Extbase\Mvc\Request;
use TYPO3\CMS\Extbase\Object\ObjectManagerInterface;
use TYPO3\CMS\Extbase\Service\EnvironmentService;
use TYPO3\CMS\Extbase\Service\ExtensionService;

Expand All @@ -37,11 +36,6 @@
*/
class RequestBuilder implements SingletonInterface
{
/**
* @var \TYPO3\CMS\Extbase\Object\ObjectManagerInterface
*/
protected $objectManager;

/**
* This is a unique key for a plugin (not the extension key!)
*
Expand Down Expand Up @@ -114,14 +108,6 @@ class RequestBuilder implements SingletonInterface
*/
private $allowedControllerAliases = [];

/**
* @param \TYPO3\CMS\Extbase\Object\ObjectManagerInterface $objectManager
*/
public function injectObjectManager(ObjectManagerInterface $objectManager)
{
$this->objectManager = $objectManager;
}

/**
* @param \TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface $configurationManager
*/
Expand Down Expand Up @@ -217,7 +203,7 @@ public function build()
}

/** @var \TYPO3\CMS\Extbase\Mvc\Request $request */
$request = $this->objectManager->get(Request::class);
$request = GeneralUtility::makeInstance(Request::class);
$request->setPluginName($this->pluginName);
$request->setControllerExtensionName($this->extensionName);
$request->setControllerAliasToClassNameMapping($this->controllerAliasToClassMapping);
Expand Down
2 changes: 1 addition & 1 deletion typo3/sysext/fluid/Classes/Core/Widget/WidgetRequestBuilder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public function build(): RequestInterface
}
$widgetContext = $this->ajaxWidgetContextHolder->get($rawGetArguments['fluid-widget-id']);

$request = $this->objectManager->get(WidgetRequest::class, $widgetContext->getControllerObjectName());
$request = GeneralUtility::makeInstance(WidgetRequest::class, $widgetContext->getControllerObjectName());
$request->setRequestUri(GeneralUtility::getIndpEnv('TYPO3_REQUEST_URL'));
$request->setBaseUri($baseUri);
$request->setMethod($_SERVER['REQUEST_METHOD'] ?? null);
Expand Down
163 changes: 50 additions & 113 deletions typo3/sysext/fluid/Tests/Unit/Core/Widget/WidgetRequestBuilderTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@

namespace TYPO3\CMS\Fluid\Tests\Unit\Core\Widget;

use Prophecy\Argument;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Extbase\Object\ObjectManagerInterface;
use TYPO3\CMS\Extbase\Service\EnvironmentService;
use TYPO3\CMS\Fluid\Core\Widget\AjaxWidgetContextHolder;
use TYPO3\CMS\Fluid\Core\Widget\WidgetContext;
Expand All @@ -34,40 +34,22 @@ class WidgetRequestBuilderTest extends UnitTestCase
*/
protected $widgetRequestBuilder;

/**
* @var ObjectManagerInterface
*/
protected $mockObjectManager;

/**
* @var WidgetRequest
*/
protected $mockWidgetRequest;

/**
* @var AjaxWidgetContextHolder
*/
protected $mockAjaxWidgetContextHolder;

/**
* @var WidgetContext
*/
protected $mockWidgetContext;

protected function setUp(): void
{
parent::setUp();
$this->widgetRequestBuilder = $this->getAccessibleMock(WidgetRequestBuilder::class, ['setArgumentsFromRawRequestData']);
$this->mockWidgetRequest = $this->createMock(WidgetRequest::class);
$this->mockObjectManager = $this->createMock(ObjectManagerInterface::class);
$this->mockObjectManager->expects(self::any())->method('get')->with(WidgetRequest::class)->willReturn($this->mockWidgetRequest);
$this->widgetRequestBuilder->_set('objectManager', $this->mockObjectManager);
$this->mockWidgetContext = $this->createMock(WidgetContext::class);
$this->mockAjaxWidgetContextHolder = $this->createMock(AjaxWidgetContextHolder::class);
$this->widgetRequestBuilder->injectAjaxWidgetContextHolder($this->mockAjaxWidgetContextHolder);

$mockWidgetContext = $this->createMock(WidgetContext::class);
$mockWidgetContext->method('getControllerObjectName')->willReturn('TYPO3\\CMS\\Core\\Controller\\FooController');

$ajaxWidgetContextHolderProphecy = $this->prophesize(AjaxWidgetContextHolder::class);
$ajaxWidgetContextHolderProphecy->get(Argument::cetera())->willReturn($mockWidgetContext);

$environmentServiceMock = $this->createMock(EnvironmentService::class);
$environmentServiceMock->expects(self::any())->method('isEnvironmentInFrontendMode')->willReturn(true);
$environmentServiceMock->expects(self::any())->method('isEnvironmentInBackendMode')->willReturn(false);

$this->widgetRequestBuilder = new WidgetRequestBuilder();
$this->widgetRequestBuilder->injectAjaxWidgetContextHolder($ajaxWidgetContextHolderProphecy->reveal());
$this->widgetRequestBuilder->injectEnvironmentService($environmentServiceMock);
}

Expand All @@ -76,16 +58,6 @@ protected function setUp(): void
*/
public function buildThrowsIfNoFluidWidgetIdWasSet()
{
$_SERVER = [
'REMOTE_ADDR' => 'foo',
'SSL_SESSION_ID' => 'foo',
'REQUEST_URI' => 'foo',
'ORIG_SCRIPT_NAME' => 'foo',
'REQUEST_METHOD' => 'foo'
];
$_GET = [
'not-the-fluid-widget-id' => 'foo'
];
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionCode(1521190675);
$this->widgetRequestBuilder->build();
Expand All @@ -97,19 +69,17 @@ public function buildThrowsIfNoFluidWidgetIdWasSet()
public function buildSetsRequestUri()
{
$_SERVER = [
'REMOTE_ADDR' => 'foo',
'SSL_SESSION_ID' => 'foo',
'REQUEST_URI' => 'foo',
'ORIG_SCRIPT_NAME' => 'foo',
'REQUEST_METHOD' => 'foo'
'REQUEST_METHOD' => 'GET'
];
$_GET = [
'fluid-widget-id' => 'foo'
];
$requestUri = GeneralUtility::getIndpEnv('TYPO3_REQUEST_URL');
$this->mockAjaxWidgetContextHolder->expects(self::once())->method('get')->willReturn($this->mockWidgetContext);
$this->mockWidgetRequest->expects(self::once())->method('setRequestURI')->with($requestUri);
$this->widgetRequestBuilder->build();
/** @var WidgetRequest $request */
$request = $this->widgetRequestBuilder->build();

self::assertInstanceOf(WidgetRequest::class, $request);
self::assertSame($requestUri, $request->getRequestUri());
}

/**
Expand All @@ -118,19 +88,17 @@ public function buildSetsRequestUri()
public function buildSetsBaseUri()
{
$_SERVER = [
'REMOTE_ADDR' => 'foo',
'SSL_SESSION_ID' => 'foo',
'REQUEST_URI' => 'foo',
'ORIG_SCRIPT_NAME' => 'foo',
'REQUEST_METHOD' => 'foo'
'REQUEST_METHOD' => 'GET'
];
$_GET = [
'fluid-widget-id' => 'foo'
];
$baseUri = GeneralUtility::getIndpEnv('TYPO3_SITE_URL');
$this->mockAjaxWidgetContextHolder->expects(self::once())->method('get')->willReturn($this->mockWidgetContext);
$this->mockWidgetRequest->expects(self::once())->method('setBaseURI')->with($baseUri);
$this->widgetRequestBuilder->build();
/** @var WidgetRequest $request */
$request = $this->widgetRequestBuilder->build();

self::assertInstanceOf(WidgetRequest::class, $request);
self::assertSame($baseUri, $request->getBaseUri());
}

/**
Expand All @@ -139,18 +107,16 @@ public function buildSetsBaseUri()
public function buildSetsRequestMethod()
{
$_SERVER = [
'REMOTE_ADDR' => 'foo',
'SSL_SESSION_ID' => 'foo',
'REQUEST_URI' => 'foo',
'ORIG_SCRIPT_NAME' => 'foo',
'REQUEST_METHOD' => 'POST'
];
$_GET = [
'fluid-widget-id' => 'foo'
];
$this->mockAjaxWidgetContextHolder->expects(self::once())->method('get')->willReturn($this->mockWidgetContext);
$this->mockWidgetRequest->expects(self::once())->method('setMethod')->with('POST');
$this->widgetRequestBuilder->build();
/** @var WidgetRequest $request */
$request = $this->widgetRequestBuilder->build();

self::assertInstanceOf(WidgetRequest::class, $request);
self::assertSame('POST', $request->getMethod());
}

/**
Expand All @@ -159,10 +125,6 @@ public function buildSetsRequestMethod()
public function buildSetsPostArgumentsFromRequest()
{
$_SERVER = [
'REMOTE_ADDR' => 'foo',
'SSL_SESSION_ID' => 'foo',
'REQUEST_URI' => 'foo',
'ORIG_SCRIPT_NAME' => 'foo',
'REQUEST_METHOD' => 'POST'
];
$_GET = [
Expand All @@ -172,9 +134,11 @@ public function buildSetsPostArgumentsFromRequest()
$_POST = [
'post' => 'bar'
];
$this->mockAjaxWidgetContextHolder->expects(self::once())->method('get')->willReturn($this->mockWidgetContext);
$this->mockWidgetRequest->expects(self::once())->method('setArguments')->with($_POST);
$this->widgetRequestBuilder->build();
/** @var WidgetRequest $request */
$request = $this->widgetRequestBuilder->build();

self::assertInstanceOf(WidgetRequest::class, $request);
self::assertSame($_POST, $request->getArguments());
}

/**
Expand All @@ -183,10 +147,6 @@ public function buildSetsPostArgumentsFromRequest()
public function buildSetsGetArgumentsFromRequest()
{
$_SERVER = [
'REMOTE_ADDR' => 'foo',
'SSL_SESSION_ID' => 'foo',
'REQUEST_URI' => 'foo',
'ORIG_SCRIPT_NAME' => 'foo',
'REQUEST_METHOD' => 'GET'
];
$_GET = [
Expand All @@ -196,9 +156,11 @@ public function buildSetsGetArgumentsFromRequest()
$_POST = [
'post' => 'bar'
];
$this->mockAjaxWidgetContextHolder->expects(self::once())->method('get')->willReturn($this->mockWidgetContext);
$this->mockWidgetRequest->expects(self::once())->method('setArguments')->with($_GET);
$this->widgetRequestBuilder->build();
/** @var WidgetRequest $request */
$request = $this->widgetRequestBuilder->build();

self::assertInstanceOf(WidgetRequest::class, $request);
self::assertSame($_GET, $request->getArguments());
}

/**
Expand All @@ -207,19 +169,17 @@ public function buildSetsGetArgumentsFromRequest()
public function buildSetsControllerActionNameFromGetArguments()
{
$_SERVER = [
'REMOTE_ADDR' => 'foo',
'SSL_SESSION_ID' => 'foo',
'REQUEST_URI' => 'foo',
'ORIG_SCRIPT_NAME' => 'foo',
'REQUEST_METHOD' => 'foo'
'REQUEST_METHOD' => 'GET'
];
$_GET = [
'action' => 'myAction',
'fluid-widget-id' => 'foo'
];
$this->mockAjaxWidgetContextHolder->expects(self::once())->method('get')->willReturn($this->mockWidgetContext);
$this->mockWidgetRequest->expects(self::once())->method('setControllerActionName')->with('myAction');
$this->widgetRequestBuilder->build();
/** @var WidgetRequest $request */
$request = $this->widgetRequestBuilder->build();

self::assertInstanceOf(WidgetRequest::class, $request);
self::assertSame('myAction', $request->getControllerActionName());
}

/**
Expand All @@ -228,39 +188,16 @@ public function buildSetsControllerActionNameFromGetArguments()
public function buildSetsWidgetContext()
{
$_SERVER = [
'REMOTE_ADDR' => 'foo',
'SSL_SESSION_ID' => 'foo',
'REQUEST_URI' => 'foo',
'ORIG_SCRIPT_NAME' => 'foo',
'REQUEST_METHOD' => 'foo'
'REQUEST_METHOD' => 'GET'
];
$_GET = [
'fluid-widget-id' => '123'
];
$this->mockAjaxWidgetContextHolder->expects(self::once())->method('get')->willReturn($this->mockWidgetContext);
$this->mockAjaxWidgetContextHolder->expects(self::once())->method('get')->with('123')->willReturn($this->mockWidgetContext);
$this->mockWidgetRequest->expects(self::once())->method('setWidgetContext')->with($this->mockWidgetContext);
$this->widgetRequestBuilder->build();
}
/** @var WidgetRequest $request */
$request = $this->widgetRequestBuilder->build();

/**
* @test
*/
public function buildReturnsRequest()
{
$_SERVER = [
'REMOTE_ADDR' => 'foo',
'SSL_SESSION_ID' => 'foo',
'REQUEST_URI' => 'foo',
'ORIG_SCRIPT_NAME' => 'foo',
'REQUEST_METHOD' => 'foo'
];
$_GET = [
'fluid-widget-id' => 'foo'
];
$this->mockAjaxWidgetContextHolder->expects(self::once())->method('get')->willReturn($this->mockWidgetContext);
$expected = $this->mockWidgetRequest;
$actual = $this->widgetRequestBuilder->build();
self::assertSame($expected, $actual);
self::assertInstanceOf(WidgetRequest::class, $request);
self::assertSame('TYPO3\\CMS\\Core\\Controller\\FooController', $request->getControllerObjectName());
self::assertSame('[]', $request->getArgumentPrefix());
}
}

0 comments on commit a554dad

Please sign in to comment.