Skip to content

Commit

Permalink
[TASK] Provide current content object as request attribute
Browse files Browse the repository at this point in the history
Extbase ConfigurationManager as stateful singleton
object is updated within extbase bootstrap for
each plugin call. This is ugly, but since
ConfigurationManager can be injected to other
extbase services, which can be injected itself,
it is very hard to get rid of.

However, ConfigurationManager is also abused to
"park" the current ContentObjectRenderer instance
in this "convenient" singleton. The current
content object renderer instance can then be retrieved
at extbase runtime using getContentObject().

The form extension uses this at a couple of places
to move the ContentObjectRenderer data around. The
ConfigurationManager should however not be a source
of such data, and our main "state" object in the
framework is the request (which ConfigurationManager
needs as well, but that's a different patch).

To allow a deprecation of getContentObject() in
ConfigurationManager, the patch changes the codebase
to attach the current content object as request attribute
instead. This is a bit fiddly as well since
ContentObjectRenderer can be recursive (cObj rendering other
cObj again), but the change at least makes this state more
obvious, and we're pretty confident the situation will
further relax and become more transparent with continued
ContentObjectRenderer refactorings. Some of these
have been prepared in v12 already and we'll see
more on this with v13.

The patch does the main refactoring, deprecating
getContentObject() will follow with another patch.
The patch does not add the attribute at all places
where ContentObjectRenderer instances are created,
more may follow later. The main goal of this patch
is to create the main infrastructure and to not break
the form extension.

Resolves: #100623
Releases: main
Change-Id: I3de7f53244c0b438ef54940d87a169068f1a832e
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/77252
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Benni Mack <benni@typo3.org>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: core-ci <typo3@b13.com>
  • Loading branch information
helhum authored and lolli42 committed Apr 18, 2023
1 parent cc8faad commit b193d25
Show file tree
Hide file tree
Showing 18 changed files with 101 additions and 90 deletions.
Expand Up @@ -67,6 +67,7 @@ public function getContentObject(): ?ContentObjectRenderer
* Note that this is a low level method and only makes sense to be used by Extbase internally.
*
* @param array $configuration The new configuration
* @internal
*/
public function setConfiguration(array $configuration = []): void
{
Expand Down
Expand Up @@ -55,6 +55,7 @@ public function getConfiguration(string $configurationType, ?string $extensionNa
* Note that this is a low level method and only makes sense to be used by Extbase internally.
*
* @param array $configuration The new configuration
* @internal
*/
public function setConfiguration(array $configuration = []): void;

Expand Down
Expand Up @@ -41,8 +41,8 @@ public function render($conf = [])
// Come here only if we are not called from $TSFE->processNonCacheableContentPartsAndSubstituteContentMarkers()!
$this->cObj->setUserObjectType(ContentObjectRenderer::OBJECTTYPE_USER);
}
$extbaseBootstrap->initialize($conf);
$content = $extbaseBootstrap->handleFrontendRequest($this->request);
$request = $extbaseBootstrap->initialize($conf, $this->request);
$content = $extbaseBootstrap->handleFrontendRequest($request);
// Rendering is deferred, as the action should not be cached, we pump this now to TSFE to be executed later-on
if ($this->cObj->doConvertToUserIntObject) {
$this->cObj->doConvertToUserIntObject = false;
Expand Down
20 changes: 12 additions & 8 deletions typo3/sysext/extbase/Classes/Core/Bootstrap.php
Expand Up @@ -79,14 +79,14 @@ public function setContentObjectRenderer(ContentObjectRenderer $cObj)
/**
* Explicitly initializes all necessary Extbase objects by invoking the various initialize* methods.
*
* Usually this method is only called from unit tests or other applications which need a more fine grained control over
* Usually this method is only called from unit tests or other applications which need a more fine-grained control over
* the initialization and request handling process. Most other applications just call the run() method.
*
* @param array $configuration The TS configuration array
* @throws \RuntimeException
* @see run()
*/
public function initialize(array $configuration): void
public function initialize(array $configuration, ServerRequestInterface $request): ServerRequestInterface
{
if (!Environment::isCli()) {
if (!isset($configuration['extensionName']) || $configuration['extensionName'] === '') {
Expand All @@ -96,7 +96,7 @@ public function initialize(array $configuration): void
throw new \RuntimeException('Invalid configuration: "pluginName" is not set', 1290623027);
}
}
$this->initializeConfiguration($configuration);
return $this->initializeConfiguration($configuration, $request);
}

/**
Expand All @@ -105,11 +105,16 @@ public function initialize(array $configuration): void
* @see initialize()
* @internal
*/
public function initializeConfiguration(array $configuration): void
public function initializeConfiguration(array $configuration, ServerRequestInterface $request): ServerRequestInterface
{
$this->cObj ??= $this->container->get(ContentObjectRenderer::class);
if ($this->cObj === null) {
$this->cObj = $this->container->get(ContentObjectRenderer::class);
$request = $request->withAttribute('currentContentObject', $this->cObj);
}
$this->cObj->setRequest($request);
$this->configurationManager->setContentObject($this->cObj);
$this->configurationManager->setConfiguration($configuration);
return $request;
// todo: Shouldn't the configuration manager object – which is a singleton – be stateless?
// todo: At this point we give the configuration manager a state, while we could directly pass the
// todo: configuration (i.e. controllerName, actionName and such), directly to the request
Expand All @@ -133,7 +138,7 @@ public function initializeConfiguration(array $configuration): void
*/
public function run(string $content, array $configuration, ServerRequestInterface $request): string
{
$this->initialize($configuration);
$request = $this->initialize($configuration, $request);
return $this->handleFrontendRequest($request);
}

Expand Down Expand Up @@ -204,7 +209,6 @@ public function handleFrontendRequest(ServerRequestInterface $request): string
*
* Creates an Extbase Request, dispatches it and then returns the Response
*
* @param ServerRequestInterface $request
* @internal
*/
public function handleBackendRequest(ServerRequestInterface $request): ResponseInterface
Expand All @@ -216,7 +220,7 @@ public function handleBackendRequest(ServerRequestInterface $request): ResponseI
'pluginName' => $module?->getIdentifier(),
];

$this->initialize($configuration);
$request = $this->initialize($configuration, $request);
$extbaseRequest = $this->extbaseRequestBuilder->build($request);
$response = $this->dispatcher->dispatch($extbaseRequest);
$this->resetSingletons();
Expand Down
38 changes: 13 additions & 25 deletions typo3/sysext/extbase/Classes/Mvc/Web/Routing/UriBuilder.php
Expand Up @@ -40,22 +40,11 @@
*/
class UriBuilder
{
/**
* @var \TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface
*/
protected $configurationManager;
protected ConfigurationManagerInterface $configurationManager;

/**
* @var \TYPO3\CMS\Extbase\Service\ExtensionService
*/
protected $extensionService;
protected ExtensionService $extensionService;

/**
* An instance of \TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer
*
* @var \TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer
*/
protected $contentObject;
protected ContentObjectRenderer $contentObject;

protected ?RequestInterface $request = null;

Expand Down Expand Up @@ -147,16 +136,6 @@ public function injectExtensionService(ExtensionService $extensionService): void
$this->extensionService = $extensionService;
}

/**
* Life-cycle method that is called by the DI container as soon as this object is completely built
* @internal only to be used within Extbase, not part of TYPO3 Core API.
*/
public function initializeObject(): void
{
$this->contentObject = $this->configurationManager->getContentObject()
?? GeneralUtility::makeInstance(ContentObjectRenderer::class);
}

/**
* Sets the current request
*
Expand All @@ -165,6 +144,15 @@ public function initializeObject(): void
public function setRequest(RequestInterface $request): UriBuilder
{
$this->request = $request;
$contentObject = $request->getAttribute('currentContentObject');
if ($contentObject === null) {
// @todo: Review this. This should never be the case since extbase
// bootstrap adds 'currentContentObject' to request. When this
// if() kicks in, it most likely indicates an "out of scope" usage.
$contentObject = GeneralUtility::makeInstance(ContentObjectRenderer::class);
$contentObject->setRequest($request->withAttribute('currentContentObject', $contentObject));
}
$this->contentObject = $contentObject;
return $this;
}

Expand Down Expand Up @@ -478,7 +466,7 @@ public function reset(): UriBuilder
/*
* $this->request MUST NOT be reset here because the request is actually a hard dependency and not part
* of the internal state of this object.
* todo: consider making the request a constructor dependency or get rid of it's usage
* todo: make the request a constructor dependency
*/
return $this;
}
Expand Down
Expand Up @@ -73,7 +73,6 @@ protected function setUp(): void
$this->subject->setRequest($this->mockRequest);
$this->subject->injectConfigurationManager($this->createMock(ConfigurationManagerInterface::class));
$this->subject->injectExtensionService($this->mockExtensionService);
$this->subject->initializeObject();
$this->subject->_set('contentObject', $this->mockContentObject);
$requestContextFactory = new RequestContextFactory(new BackendEntryPointResolver());
$router = new Router($requestContextFactory);
Expand Down
Expand Up @@ -41,7 +41,6 @@ protected function setUp(): void
$this->subject->setRequest($this->createMock(Request::class));
$this->subject->injectConfigurationManager($this->createMock(ConfigurationManagerInterface::class));
$this->subject->injectExtensionService($this->mockExtensionService);
$this->subject->initializeObject();
$this->subject->_set('contentObject', $this->createMock(ContentObjectRenderer::class));
}

Expand Down
Expand Up @@ -128,7 +128,7 @@ public static function renderStatic(array $arguments, \Closure $renderChildrenCl
/** @var RenderingContext $renderingContext */
$request = $renderingContext->getRequest();
$contentObjectRenderer = self::getContentObjectRenderer($request);
$contentObjectRenderer->setRequest($request);
$contentObjectRenderer->setRequest($request->withAttribute('currentContentObject', $contentObjectRenderer));
$tsfeBackup = null;
if (!isset($GLOBALS['TSFE']) || !($GLOBALS['TSFE'] instanceof TypoScriptFrontendController)) {
$tsfeBackup = self::simulateFrontendEnvironment();
Expand Down
Expand Up @@ -69,7 +69,7 @@ public function renderAction(): ResponseInterface
$formDefinition['persistenceIdentifier'] = $this->settings['persistenceIdentifier'];
$formDefinition = $this->overrideByFlexFormSettings($formDefinition);
$formDefinition = ArrayUtility::setValueByPath($formDefinition, 'renderingOptions._originalIdentifier', $formDefinition['identifier'], '.');
$formDefinition['identifier'] .= '-' . $this->configurationManager->getContentObject()->data['uid'];
$formDefinition['identifier'] .= '-' . $this->request->getAttribute('currentContentObject')->data['uid'];
}
$this->view->assign('formConfiguration', $formDefinition);

Expand All @@ -93,7 +93,7 @@ public function performAction(): ResponseInterface
*/
protected function overrideByFlexFormSettings(array $formDefinition): array
{
$flexFormData = GeneralUtility::xml2array($this->configurationManager->getContentObject()->data['pi_flexform'] ?? '');
$flexFormData = GeneralUtility::xml2array($this->request->getAttribute('currentContentObject')->data['pi_flexform'] ?? '');

if (!is_array($flexFormData)) {
return $formDefinition;
Expand Down
Expand Up @@ -57,20 +57,11 @@ class ConfirmationFinisher extends AbstractFinisher
'typoscriptObjectPath' => 'lib.tx_form.contentElementRendering',
];

/**
* @var array
*/
protected $typoScriptSetup = [];
protected array $typoScriptSetup = [];

/**
* @var ConfigurationManagerInterface
*/
protected $configurationManager;
protected ConfigurationManagerInterface $configurationManager;

/**
* @var ContentObjectRenderer
*/
protected $contentObjectRenderer;
protected ContentObjectRenderer $contentObjectRenderer;

public function injectConfigurationManager(ConfigurationManagerInterface $configurationManager)
{
Expand Down Expand Up @@ -109,7 +100,8 @@ protected function executeInternal()
}
$setup = $setup[$segment . '.'];
}
$this->contentObjectRenderer->start([$contentElementUid], '');
$this->contentObjectRenderer->setRequest($this->finisherContext->getRequest());
$this->contentObjectRenderer->start([$contentElementUid]);
$this->contentObjectRenderer->setCurrentVal((string)$contentElementUid);
$message = $this->contentObjectRenderer->cObjGetSingle($setup[$lastSegment], $setup[$lastSegment . '.'], $lastSegment);
} else {
Expand Down
6 changes: 3 additions & 3 deletions typo3/sysext/form/Classes/Domain/Runtime/FormRuntime.php
Expand Up @@ -504,7 +504,7 @@ protected function isPostRequest(): bool
*/
protected function isRenderedCached(): bool
{
$contentObject = $this->configurationManager->getContentObject();
$contentObject = $this->request->getAttribute('currentContentObject');
return $contentObject === null
? true
// @todo this does not work when rendering a cached `FLUIDTEMPLATE` (not nested in `COA_INT`)
Expand Down Expand Up @@ -1079,8 +1079,8 @@ protected function getConditionResolver(): Resolver
}

$contentObjectData = [];
if ($this->configurationManager->getContentObject() instanceof ContentObjectRenderer) {
$contentObjectData = $this->configurationManager->getContentObject()->data;
if ($this->request->getAttribute('currentContentObject') instanceof ContentObjectRenderer) {
$contentObjectData = $this->request->getAttribute('currentContentObject')->data;
}

return GeneralUtility::makeInstance(
Expand Down
Expand Up @@ -24,14 +24,14 @@ class FormCachingTestsController extends ActionController
{
public function someRenderAction(): ResponseInterface
{
$this->view->assign('formIdentifier', $this->request->getPluginName() . '-' . $this->configurationManager->getContentObject()->data['uid']);
$this->view->assign('formIdentifier', $this->request->getPluginName() . '-' . $this->request->getAttribute('currentContentObject')->data['uid']);
$this->view->assign('pluginName', $this->request->getPluginName());
return $this->htmlResponse();
}

public function somePerformAction(): ResponseInterface
{
$this->view->assign('formIdentifier', $this->request->getPluginName() . '-' . $this->configurationManager->getContentObject()->data['uid']);
$this->view->assign('formIdentifier', $this->request->getPluginName() . '-' . $this->request->getAttribute('currentContentObject')->data['uid']);
$this->view->assign('pluginName', $this->request->getPluginName());
return $this->htmlResponse();
}
Expand Down
Expand Up @@ -21,9 +21,11 @@
use TYPO3\CMS\Core\Cache\CacheManager;
use TYPO3\CMS\Core\Cache\Frontend\NullFrontend;
use TYPO3\CMS\Core\Configuration\FlexForm\FlexFormTools;
use TYPO3\CMS\Core\Http\ServerRequest;
use TYPO3\CMS\Core\Tests\Unit\Fixtures\EventDispatcher\MockEventDispatcher;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Extbase\Configuration\FrontendConfigurationManager;
use TYPO3\CMS\Extbase\Mvc\ExtbaseRequestParameters;
use TYPO3\CMS\Extbase\Mvc\Request;
use TYPO3\CMS\Form\Controller\FormFrontendController;
use TYPO3\CMS\Form\Domain\Configuration\ConfigurationService;
use TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer;
Expand Down Expand Up @@ -65,7 +67,12 @@ public function overrideByFlexFormSettingsReturnsNoOverriddenConfigurationIfFlex
);

$flexFormTools = new FlexFormTools(new MockEventDispatcher());
$request = (new ServerRequest())->withAttribute('extbase', new ExtbaseRequestParameters());
$request = (new Request($request));
$contentObject = new ContentObjectRenderer();
$request = $request->withAttribute('currentContentObject', $contentObject);
$contentObject->setRequest($request);
$mockController->_set('request', $request);
$contentObject->data = [
'pi_flexform' => $flexFormTools->flexArray2Xml([
'data' => [
Expand Down Expand Up @@ -99,13 +106,6 @@ public function overrideByFlexFormSettingsReturnsNoOverriddenConfigurationIfFlex
]),
];

$frontendConfigurationManager = $this->createMock(FrontendConfigurationManager::class);
$frontendConfigurationManager
->method('getContentObject')
->willReturn($contentObject);

$mockController->_set('configurationManager', $frontendConfigurationManager);

$configurationServiceMock->method('getPrototypeConfiguration')->with(self::anything())->willReturn([
'finishersDefinition' => [
'EmailToReceiver' => [
Expand Down Expand Up @@ -196,7 +196,12 @@ public function overrideByFlexFormSettingsReturnsOverriddenConfigurationIfFlexfo
);

$flexFormTools = new FlexFormTools(new MockEventDispatcher());
$request = (new ServerRequest())->withAttribute('extbase', new ExtbaseRequestParameters());
$request = (new Request($request));
$contentObject = new ContentObjectRenderer();
$request = $request->withAttribute('currentContentObject', $contentObject);
$contentObject->setRequest($request);
$mockController->_set('request', $request);
$contentObject->data = [
'pi_flexform' => $flexFormTools->flexArray2Xml([
'data' => [
Expand Down Expand Up @@ -230,13 +235,6 @@ public function overrideByFlexFormSettingsReturnsOverriddenConfigurationIfFlexfo
]),
];

$frontendConfigurationManager = $this->createMock(FrontendConfigurationManager::class);
$frontendConfigurationManager
->method('getContentObject')
->willReturn($contentObject);

$mockController->_set('configurationManager', $frontendConfigurationManager);

$configurationServiceMock->method('getPrototypeConfiguration')->with(self::anything())->willReturn([
'finishersDefinition' => [
'EmailToReceiver' => [
Expand Down Expand Up @@ -354,7 +352,12 @@ public function overrideByFlexFormSettingsReturnsNotOverriddenConfigurationKeyIf
GeneralUtility::addInstance(FlexFormTools::class, new FlexFormTools($eventDispatcher));

$flexFormTools = new FlexFormTools(new MockEventDispatcher());
$request = (new ServerRequest())->withAttribute('extbase', new ExtbaseRequestParameters());
$request = (new Request($request));
$contentObject = new ContentObjectRenderer();
$request = $request->withAttribute('currentContentObject', $contentObject);
$contentObject->setRequest($request);
$mockController->_set('request', $request);
$contentObject->data = [
'pi_flexform' => $flexFormTools->flexArray2Xml([
'data' => [
Expand Down Expand Up @@ -388,13 +391,6 @@ public function overrideByFlexFormSettingsReturnsNotOverriddenConfigurationKeyIf
]),
];

$frontendConfigurationManager = $this->createMock(FrontendConfigurationManager::class);
$frontendConfigurationManager
->method('getContentObject')
->willReturn($contentObject);

$mockController->_set('configurationManager', $frontendConfigurationManager);

$configurationServiceMock->method('getPrototypeConfiguration')->with(self::anything())->willReturn([
'finishersDefinition' => [
'EmailToReceiver' => [
Expand Down

0 comments on commit b193d25

Please sign in to comment.