Merge pull request #16390 from artfulrobot/tame-propbag-deprecation-logs

Prevent PropertBag from being so noisy about deprecation warnings
civicrm · Jan 28, 2020 · 7c786c4 · 7c786c4
2 parents d64bafa + afbfe83
commit 7c786c4
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 6 deletions.
diff --git a/Civi/Payment/PropertyBag.php b/Civi/Payment/PropertyBag.php
@@ -21,6 +21,10 @@
  *
  */
 class PropertyBag implements \ArrayAccess {
+  /**
+   * @var array - see legacyWarning */
+  public static $legacyWarnings = [];
+
   protected $props = ['default' => []];
 
   protected static $propMap = [
@@ -161,12 +165,34 @@ public function offsetUnset ($offset) {
   }
 
   /**
+   * Log legacy warnings info.
+   *
    * @param string $message
    */
   protected function legacyWarning($message) {
-    $message = "Deprecated code: $message";
+    if (empty(static::$legacyWarnings)) {
+      // First time we have been called.
+      register_shutdown_function([PropertyBag::class, 'writeLegacyWarnings']);
+    }
+    // Store warnings instead of logging immediately, as calls to Civi::log()
+    // can take over half a second to work in some hosting environments.
+    static::$legacyWarnings[$message] = TRUE;
+
+    // For unit tests:
     $this->lastWarning = $message;
-    Civi::log()->warning($message);
+  }
+
+  /**
+   * Save any legacy warnings to log.
+   *
+   * Called as a shutdown function.
+   */
+  public static function writeLegacyWarnings() {
+    if (!empty(static::$legacyWarnings)) {
+      $message = "Civi\\Payment\\PropertyBag related deprecation warnings:\n"
+        . implode("\n", array_keys(static::$legacyWarnings));
+      Civi::log()->warning($message, ['civi.tag' => 'deprecated']);
+    }
   }
 
   /**

diff --git a/tests/phpunit/Civi/Payment/PropertyBagTest.php b/tests/phpunit/Civi/Payment/PropertyBagTest.php
@@ -69,12 +69,12 @@ public function testSetContactIDLegacyWay() {
 
     // Now access via legacy name - should work but generate warning.
     $this->assertEquals(123, $propertyBag['contact_id']);
-    $this->assertEquals("Deprecated code: We have translated 'contact_id' to 'contactID' for you, but please update your code to use the propper setters and getters.", $propertyBag->lastWarning);
+    $this->assertEquals("We have translated 'contact_id' to 'contactID' for you, but please update your code to use the propper setters and getters.", $propertyBag->lastWarning);
 
     // Repeat but this time set the property using a legacy name, fetch by new name.
     $propertyBag = new PropertyBag();
     $propertyBag['contact_id'] = 123;
-    $this->assertEquals("Deprecated code: We have translated 'contact_id' to 'contactID' for you, but please update your code to use the propper setters and getters.", $propertyBag->lastWarning);
+    $this->assertEquals("We have translated 'contact_id' to 'contactID' for you, but please update your code to use the propper setters and getters.", $propertyBag->lastWarning);
     $this->assertEquals(123, $propertyBag->getContactID());
     $this->assertEquals(123, $propertyBag['contactID']);
     $this->assertEquals(123, $propertyBag['contact_id']);
@@ -88,7 +88,7 @@ public function testMergeInputs() {
       'contactID' => 123,
       'contributionRecurID' => 456,
     ]);
-    $this->assertEquals("Deprecated code: We have merged input params into the property bag for now but please rewrite code to not use this.", $propertyBag->lastWarning);
+    $this->assertEquals("We have merged input params into the property bag for now but please rewrite code to not use this.", $propertyBag->lastWarning);
     $this->assertEquals(123, $propertyBag->getContactID());
     $this->assertEquals(456, $propertyBag->getContributionRecurID());
   }
@@ -106,7 +106,7 @@ public function testSetCustomProp() {
     $propertyBag = new PropertyBag();
     $propertyBag['customThingForMyProcessor'] = 'fidget';
     $this->assertEquals('fidget', $propertyBag->getCustomProperty('customThingForMyProcessor'));
-    $this->assertEquals("Deprecated code: Unknown property 'customThingForMyProcessor'. We have merged this in for now as a custom property. Please rewrite your code to use PropertyBag->setCustomProperty if it is a genuinely custom property, or a standardised setter like PropertyBag->setContactID for standard properties", $propertyBag->lastWarning);
+    $this->assertEquals("Unknown property 'customThingForMyProcessor'. We have merged this in for now as a custom property. Please rewrite your code to use PropertyBag->setCustomProperty if it is a genuinely custom property, or a standardised setter like PropertyBag->setContactID for standard properties", $propertyBag->lastWarning);
   }
 
   /**