From f128a168194d871b669e4712798462502f3839c6 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Tue, 2 May 2023 21:54:24 -0700
Subject: [PATCH] (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 | 1 -
 CRM/Core/Config/Runtime.php    | 9 +++++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/CRM/Core/Config/MagicMerge.php b/CRM/Core/Config/MagicMerge.php
index 1c7be169ea..3e5f3b197d 100644
--- a/CRM/Core/Config/MagicMerge.php
+++ b/CRM/Core/Config/MagicMerge.php
@@ -402,7 +402,6 @@ class CRM_Core_Config_MagicMerge {
         'inCiviCRM' => FALSE,
         'doNotResetCache' => 0,
         'keyDisable' => FALSE,
-        'initialized' => FALSE,
         'userFrameworkFrontend' => FALSE,
         'userPermissionTemp' => NULL,
       ];
diff --git a/CRM/Core/Config/Runtime.php b/CRM/Core/Config/Runtime.php
index 1fd76d270d..c642d699db 100644
--- a/CRM/Core/Config/Runtime.php
+++ b/CRM/Core/Config/Runtime.php
@@ -16,7 +16,7 @@
  * the DSN, CMS type, CMS URL, etc. Generally, runtime properties must be
  * determined externally (before loading CiviCRM).
  */
-class CRM_Core_Config_Runtime extends CRM_Core_Config_MagicMerge {
+class CRM_Core_Config_Runtime {
 
   public $dsn;
 
@@ -63,6 +63,11 @@ class CRM_Core_Config_Runtime extends CRM_Core_Config_MagicMerge {
    */
   public $templateDir;
 
+  /**
+   * @var bool
+   */
+  public $initialized;
+
   /**
    * @param bool $loadFromDB
    */
@@ -117,7 +122,7 @@ class CRM_Core_Config_Runtime extends CRM_Core_Config_MagicMerge {
   public function includeCustomPath() {
     $customProprtyName = ['customPHPPathDir', 'customTemplateDir'];
     foreach ($customProprtyName as $property) {
-      $value = $this->getSettings()->get($property);
+      $value = Civi::settings()->get($property);
       if (!empty($value)) {
         $customPath = Civi::paths()->getPath($value);
         set_include_path($customPath . PATH_SEPARATOR . get_include_path());
-- 
2.25.1