(REF) CRM_Core_Config - Simplify dependencies between MagicMerge and Runtime
authorTim Otten <totten@civicrm.org>
Wed, 3 May 2023 04:54:24 +0000 (21:54 -0700)
committerTim Otten <totten@civicrm.org>
Wed, 3 May 2023 19:23:33 +0000 (12:23 -0700)
commitf128a168194d871b669e4712798462502f3839c6
tree57b187e34bd494a72211394d47d37accf2191705
parent88b8503d923526b5645a8bc141622080efa88c94
(REF) CRM_Core_Config  - Simplify dependencies between MagicMerge and Runtime

Overview
--------

Simplify the dependencies between classes `CRM_Core_Config_MagicMerge` and `CRM_Core_Config_Runtime`.

Before
------

There is a sort of dependency loop:

```php
class CRM_Core_Config extends CRM_Core_Config_MagicMerge { ... }
class CRM_Core_Config_MagicMerge { ... \Civi\Core\Container::getBootService('runtime')->get() ... }
class CRM_Core_Config_Runtime extends CRM_Core_Config_MagicMerge { ... }
```

Thus, we have `$config` (an instance of `MagicMerge`) which uses `$runtime` (another
instance of `MagicMerge`).  While it works, it twists the brain to understand properties
that actually through each of these layers (e.g.  `$config->initialized`).

After
-----

There is no loop.

```php
class CRM_Core_Config extends CRM_Core_Config_MagicMerge { ... }
class CRM_Core_Config_MagicMerge { ... \Civi\Core\Container::getBootService('runtime')->get() ... }
class CRM_Core_Config_Runtime { ... }
```

Technical Details
-----------------

Removing the `extends MagicMerge` bit means that:

1. In `Runtime.php`, it shouldn't call `$this->getSettings()`.
    * I believe that this loop was added by 1b81ed503682ef2d88f65673b0dce9f112078000 in order to make use of `getSettings()`.
      However, `Civi::settings()` should be equally valid here. (After all, `getSettings()` calls out to `civi::settings()`...)
2. In `Runtime.php`, the property `$initialized` cannot be stored through inherited-magic.
    * By declaring the property, we make it similar to all the other properties in `Runtime.php` (which are also declared).

Unit-testing this change would be quite tricky.  For testing, I basically wanted to ensure
that `$config->initialized` behaved the same way before+after.  To do this, I hacked some log
statements (https://gist.github.com/totten/4038882da7a5a4014b73c0596057715d) and ran

```
cv ev 'return CRM_Core_Config::singleton()->initialized;'
```

This showed that `$initialized` starts out as `null` and becomes `1`.  The behavior is the
same before+after the patch.  (Actually, it's a little better - since it resolves some
warnings emitted by the first `var_export()` call.)
CRM/Core/Config/MagicMerge.php
CRM/Core/Config/Runtime.php