CRM-21568 - Move emptiness judgments from SettingsBag::setDb to InnoD…

…BIndexer The `SettingsBag::setDb()` function calls any `on_change` listeners. It originally used "dumb on change" behavior (where it calls the listeners without comparing values). CRM-19610 had an issue where the `InnoDBIndexer` was running a bit too often, so they tried to resolve it by making the `SettingsBag::setDb()` more clever. Unfortunately, that's been a bit bouncy, and the cleverness depends on one's particular interpretation of 0 vs '0' vs '' vs NULL vs FALSE. Before ------ All on-change listeners may be skipped if there's particular mix of emptiness in the old/new values. After ----- The on-change listeners always fire. However, the specific listener involved with CRM-19610 will now ignore some insignificant changes. Related discussion: #11417 ---------------------------------------- * CRM-21568: https://issues.civicrm.org/jira/browse/CRM-21568 * CRM-19610: https://issues.civicrm.org/jira/browse/CRM-19610
civicrm · Dec 16, 2017 · 9f0441b · 9f0441b
1 parent f560523
commit 9f0441b
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 3 deletions.
diff --git a/CRM/Core/InnoDBIndexer.php b/CRM/Core/InnoDBIndexer.php
@@ -91,6 +91,10 @@ public static function singleton($fresh = FALSE) {
    *   Specification of the setting (per *.settings.php).
    */
   public static function onToggleFts($oldValue, $newValue, $metadata) {
+    if (empty($oldValue) && empty($newValue)) {
+      return;
+    }
+
     $indexer = CRM_Core_InnoDBIndexer::singleton();
     $indexer->setActive($newValue);
     $indexer->fixSchemaDifferences();

diff --git a/Civi/Core/SettingsBag.php b/Civi/Core/SettingsBag.php
@@ -352,9 +352,11 @@ protected function setDb($name, $value) {
     }
     $dao->find(TRUE);
 
-    // string comparison with 0 always return true, so to be ensure the type use ===
-    // ref - https://stackoverflow.com/questions/8671942/php-string-comparasion-to-0-integer-returns-true
-    if (isset($metadata['on_change']) && !($value === 0 && ($dao->value === NULL || unserialize($dao->value) == 0))) {
+    // Call 'on_change' listeners. It would be nice to only fire when there's
+    // a genuine change in the data. However, PHP developers have mixed
+    // expectations about whether 0, '0', '', NULL, and FALSE represent the same
+    // value, so there's no universal way to determine if a change is genuine.
+    if (isset($metadata['on_change'])) {
       foreach ($metadata['on_change'] as $callback) {
         call_user_func(
           \Civi\Core\Resolver::singleton()->get($callback),