Skip to content
Snippets Groups Projects
Commit 5ecafdb7 authored by Christian Kuhn's avatar Christian Kuhn
Browse files

[TASK] Do not use ConfigurationManager in FormPersistenceManager

The FormPersistenceManager is a central class of ext:form
that takes care of .form.yaml writing and loading. This
includes overloading with form specific TypoScript settings
and sanitation of storage locations based on module.tx_form
TypoScript.

Since ext:form is designed as an extbase extension,
FormPersistenceManager relies on extbase ConfigurationManager
to retrieve TypoScript. The ext:form internal own variant
of extbase ConfigurationManager has this dependency as well.

This wouldn't be a huge problem if FormPersistenceManager
is only used from within the extbase bootstrapped extension
context only. FormPersistenceManager however is used
'out-of-context' in three places: It is used within the
BE FormEngine related flex form data structure handling
hook, within the BE page module related content element
preview rendering, and by the formvh:render view helper.

These three places do not have an extbase context, so extbase,
and especially the extbase related stateful injectable singleton
ConfigurationManager, has not been configured at these points.
Extbase ConfigurationManager, and in turn the ext:form extended
variant as well, have a dependency to the request to work
properly and thus rely on the request being set up correctly.

To deal with this, FormPersistenceManager configures extbase
ConfigurationManager within __construct() by handing over the
request from $GLOBALS['TYPO3_REQUEST'].

This is bad for a series of reasons, one being obviously the
access to $GLOBALS['TYPO3_REQUEST'] within __construct() in
the first place. Also, FormPersistenceManager is quite often
used *within* properly configured extbase context as well,
for instance from within the extbase based ext:form
controllers, so FormPersistenceManager then reconfigures
extbase ConfigurationManager with the (non-extbase) global
request and thus messes up its state.

The problem is here that FormPersistenceManager does not and
can not know wether its used 'out-of-context'.

To deal with this, the patch removes use of ConfigurationManager
to retrieve TypoScript within FormPersistenceManager, and forces
consumers to hand it over to those methods that need it. The
idea is that only the callers know their context and can take
care of setting up ConfigurationManager as needed on their own.

This approach makes the dependency to the request object
explicit: Two of the three 'out-of-context' places are "ok" with
that: The BE page module preview rendering has the request
available, the formvh:render view helper as well. The third
one will be harder to resolve: The flex form related data
structure handling has not been designed to have the request,
there may or may not be CLI situations in this low level handling
where no request is available. This needs further analysis, the
patch outlines various options for now.

The patch forces FormPersistenceManagerInterface to be adapted,
which should not be a huge issue since the overall functionality
is ext:form internal, and there shouldn't be extensions that
use it directly. The patch is thus *not* considered breaking
and FormPersistenceManagerInterface is now declared @internal
along the way to reflect this.

Also, as usual when working in this area, the related tests
receive an overhaul to deal with the new signatures and
internal handling, and to be more easy to maintain.

Also, class ext:form ConfigurationService suffers from
similar issues as outlined above and will probably need
a revamp as well. This one however is only used in
combination with FormPersistenceManager when used
'out-of-context', so the changes for FormPersistenceManager
relax this situation as well. Still, and @todo is left in it.

Resolves: #104667
Related: #92408
Related: #102558
Releases: main
Change-Id: I555aa6445f1fad577253cf90065df540a5830bd7
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/85669


Reviewed-by: default avatarAnja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: default avatarChristian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: default avatarStefan Bürk <stefan@buerk.tech>
Tested-by: default avatarcore-ci <typo3@b13.com>
Tested-by: default avatarAnja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: default avatarStefan Bürk <stefan@buerk.tech>
Tested-by: default avatarChristian Kuhn <lolli@schwarzbu.ch>
parent e4cab4b6
Branches
Tags
No related merge requests found
Showing
with 1565 additions and 626 deletions
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment