Skip to content
  • totten's avatar
    CRM_Core_Config_MagicMerge - Fix thread-local updates for aliased properties · cdf3884e
    totten authored
    Overview
    --------
    
    `CRM_Core_Config_MagicMerge` allows properties to be temporarily modified
    (for the duration the object's lifespan). However, this behavior does
    not work correctly if the property-name uses aliasing.
    
    The PR addresses a test-failure that became visible in #14718, and it adds
    test-coverage for some typical and atypical examples.
    
    Before
    ------
    
    * (Vast majority of properties) If a property has a simple name (e.g.
      `maxFileSize`), then `$cache`-reads and `$cache`-writes are consistent.
    * (Handful of properties) If a property has a split name (e.g.
      `maxAttachments`, `max_attachments`), then `$cache`-reads and
      `$cache`-writes are *not* consistent.  Writes basically don't work.
    
    After
    -----
    
    * Aliased properties work the same as normal/non-aliased.
    
    Technical Details
    -----------------
    
    To understand the change, you should skim `MagicMerge` and observe
    a few things:
    
    * In `getPropertyMap()`,  observe that:
        * (a) Most properties have a singular name -- for example, `maxFileSize`
          has one name, which will be the same in the high-level facade (`$config->maxFileSize`)
          and in the underlying data-store (`settings->get('maxFileSize')`).
        * (b) Some properties are aliased. Ex: `$config->maxAttachments`
          corresponds to underlying item `settings->get('max_attachments')`.
    * In `__get()` and `__set()`, note that properties are loaded initally from
      the underlying data-store once; thereafter, any reads or writes go to
      `$this->cache`. That's a thread-local place that stores temporary revisions.
    
    To see the cache consistency problem, specifically note that:
    
    * `__get()` consistently references `$this->cache[$k]`
    * `__set()` was confused; at the start, it used `$this->cache[$k]`,
      and later it used `$this->cache[$name]`.
    
    This PR adds a unit-test which ensures that the thread-local/cached value
    works consistently.
    cdf3884e