Fix DeploymentConfig::getAllEnvOverrides memoization when no MAGENTO_…#40589
Fix DeploymentConfig::getAllEnvOverrides memoization when no MAGENTO_…#40589hryvinskyi wants to merge 1 commit intomagento:2.4-developfrom
Conversation
…DC_ env vars exist `DeploymentConfig::getAllEnvOverrides()` has a broken memoization pattern. It uses `empty($this->envOverrides)` to check if the env scan has already been performed, but when no `MAGENTO_DC_*` environment variables exist (which is the common case), `$this->envOverrides` remains `[]`, and `empty([])` returns `true` causing the full `getenv()` scan to execute on every call.
|
Hi @hryvinskyi. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
|
@magento run all tests |
Description (*)
DeploymentConfig::getAllEnvOverrides()has a broken memoization pattern that causes significant performance degradation on every request.The method uses
empty($this->envOverrides)to determine whether the environment variable scan has already been performed. However, when noMAGENTO_DC_*environment variables exist (which is the common case for most Magento installations),$this->envOverridesremains as[], andempty([])evaluates totrue— causing the fullgetenv()loop to execute on every single call.The fix changes the initialization of
$envOverridesfrom[]tonulland uses a strict=== nullcheck as the guard condition. This ensures thegetenv()scan runs exactly once per request lifecycle, regardless of whether anyMAGENTO_DC_*variables are found.Manual testing scenarios (*)
MAGENTO_DC_*environment variables set (default setup).getAllEnvOverridesin the profile — before the fix it will show a lot of callsgetAllEnvOverridesnow shows ~1-2 calls with negligible time.MAGENTO_DC_*environment variable (e.g.,MAGENTO_DC_DB__CONNECTION__DEFAULT__HOST=127.0.0.1).DeploymentConfig::get('db/connection/default/host').resetData()still works correctly (e.g., afterbin/magento app:config:import).Questions or comments
This is a one-line logic fix with meaningful performance impact. The root cause is that
empty()cannot distinguish between "never scanned" and "scanned but found nothing." Usingnullas the uninitialized sentinel resolves this cleanly.Contribution checklist (*)