Feature #94995
open
Expose environment object through DI
0%
Description
The Environment static class currently has a lot of useful information, but being static it's bad for testability.
Solution, which will require PHP 8.1 and thus TYPO3 v12:
- Make all properties of the Environment class public and readonly.
- Make the initialize() static method just instantiate a singleton instance of Environment that is tracked internally.
- Have the constructor do whatever pre-processing and lookup is necessary (like computing some of the derived strings.)
- Change all the static methods to access the instance object and just return the appropriate property.
- Wire the Environment object into the DI system. (This will be the trickiest part; have to look up where it's initialized right now.)
- Services can now get Environment injected, and just read public properties off of it as they need.
- Deprecate (either via documentation, code, or both) using the static methods.
- Remove the static methods in v13, leaving behind a readonly struct class in DI for anyone to use as needed, or trivially override for testing.
Updated by Benni Mack over 2 years ago
just a side not for historical reasons.
The Environment class was introduced to replace actual PHP constants. Constants were really bad in terms of testing. However, back then we did not have DI, and we had (and still have) the Environment checks in static methods etc. where we did not have DI (yet). So I decided to go with static methods, but able to change them in tests with the initialize() method.
The Environment class contains lowlevel information that will not change during a PHP process (not backend/frontend subrequests), thus also not containing any Url/Request information.
Updated by Helmut Hummel over 2 years ago
Larry Garfield wrote:
The Environment static class currently has a lot of useful information, but being static it's bad for testability.
Yupp, that also bothers me for a while now.
Solution, which will require PHP 8.1 and thus TYPO3 v12:
Sounds like a reasonable plan.
I'd like to add the following details:
- SystemEnvironmentBuilder should be a factory of the environment instance
- The environment instance should be passed to Bootstrap::init()
- The instance should be added to a static property in Environment, so that for BC the static methods can read from the instance
Other than that the proposal looks good, thanks
Updated by Gerrit Code Review about 2 years ago
- Status changed from New to Under Review
Patch set 1 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/73110
Updated by Gerrit Code Review about 2 years ago
Patch set 2 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/73110
Updated by Larry Garfield about 2 years ago
- Blocked by Task #96612: Update to latest PHP Parser added