Simplify tracking of URLs

In the prior commit, it needed to conceptually change the behavior of
`getTrackerURL($oldURL): $trackedURL`, but it danced around it (adding
more classes and similar-sounding-methods, but distributed elsewhere).

This re-consolidates to keep a cleaner separation of responsibilities
between `{Html,Text}ClickTracker.php` (*parsing an HTML or text message for URLS*)
and `TrackableURL.php` (*translating the URL*).

CRM/Mailing/BAO/TrackableURL.php

@@ -38,7 +38,15 @@ public function __construct() {
    *   The redirect/tracking url
    */
   public static function getTrackerURL($url, $mailing_id, $queue_id) {
+    if (strpos($url, '{') !== FALSE) {
+      return self::getTrackerURLForUrlWithTokens($url, $mailing_id, $queue_id);
+    }
+    else {
+      return self::getBasicTrackerURL($url, $mailing_id, $queue_id);
+    }
+  }
 
+  private static function getBasicTrackerURL($url, $mailing_id, $queue_id) {
     static $urlCache = [];
 
     if (array_key_exists($mailing_id . $url, $urlCache)) {
@@ -87,6 +95,66 @@ public static function getTrackerURL($url, $mailing_id, $queue_id) {
     return $returnUrl;
   }
 
+  /**
+   * Create a trackable URL for a URL with tokens.
+   *
+   * @param string $url
+   * @param int $mailing_id
+   * @param int|string $queue_id
+   *
+   * @return string
+   */
+  private static function getTrackerURLForUrlWithTokens($url, $mailing_id, $queue_id) {
+
+    // Parse the URL.
+    // (not using parse_url because it's messy to reassemble)
+    if (!preg_match('/^([^?#]+)([?][^#]*)?(#.*)?$/', $url, $parsed)) {
+      // Failed to parse it, give up and don't track it.
+      return $url;
+    }
+
+    // If we have a token in the URL + path section, we can't tokenise.
+    if (strpos($parsed[1], '{') !== FALSE) {
+      return $url;
+    }
+
+    $trackable_url = $parsed[1];
+
+    // Process the query parameters, if there are any.
+    $tokenised_params = [];
+    $static_params = [];
+    if (!empty($parsed[2])) {
+      $query_key_value_pairs = explode('&', substr($parsed[2], 1));
+
+      // Separate the tokenised from the static parts.
+      foreach ($query_key_value_pairs as $_) {
+        if (strpos($_, '{') === FALSE) {
+          $static_params[] = $_;
+        }
+        else {
+          $tokenised_params[] = $_;
+        }
+      }
+      // Add the static params to the trackable part.
+      if ($static_params) {
+        $trackable_url .= '?' . implode('&', $static_params);
+      }
+    }
+
+    // Get trackable URL.
+    $data = self::getBasicTrackerURL($trackable_url, $mailing_id, $queue_id);
+
+    // Append the tokenised bits and the fragment.
+    if ($tokenised_params) {
+      // We know the URL will already have the '?'
+      $data .= '&' . implode('&', $tokenised_params);
+    }
+    if (!empty($parsed[3])) {
+      $data .= $parsed[3];
+    }
+    return $data;
+  }
+
   /**
    * @param $url
    * @param $mailing_id

ext/flexmailer/src/ClickTracker/HtmlClickTracker.php

@@ -10,25 +10,13 @@
  */
 namespace Civi\FlexMailer\ClickTracker;
 
-class HtmlClickTracker extends BaseClickTracker implements ClickTrackerInterface {
+class HtmlClickTracker implements ClickTrackerInterface {
 
   public function filterContent($msg, $mailing_id, $queue_id) {
-
-    $getTrackerURL = BaseClickTracker::$getTrackerURL;
-
     return self::replaceHrefUrls($msg,
-      function ($url) use ($mailing_id, $queue_id, $getTrackerURL) {
-        if (strpos($url, '{') !== FALSE) {
-          // If there are tokens in the URL use special treatment.
-
-          // Since we're dealing with HTML let's strip out the entities in the URL
-          // so that we can add them back in later.
-          $originalUrlDecoded = html_entity_decode($url);
-          $data = BaseClickTracker::getTrackerURLForUrlWithTokens($originalUrlDecoded, $mailing_id, $queue_id);
-        }
-        else {
-          $data = $getTrackerURL($url, $mailing_id, $queue_id);
-        }
+      function ($url) use ($mailing_id, $queue_id) {
+        $data = \CRM_Mailing_BAO_TrackableURL::getTrackerURL(
+          html_entity_decode($url), $mailing_id, $queue_id);
         $data = htmlentities($data, ENT_NOQUOTES);
         return $data;
       }

ext/flexmailer/src/ClickTracker/TextClickTracker.php

@@ -10,21 +10,13 @@
  */
 namespace Civi\FlexMailer\ClickTracker;
 
-class TextClickTracker extends BaseClickTracker implements ClickTrackerInterface {
+class TextClickTracker implements ClickTrackerInterface {
 
   public function filterContent($msg, $mailing_id, $queue_id) {
-
-    $getTrackerURL = BaseClickTracker::$getTrackerURL;
-
     return self::replaceTextUrls($msg,
-      function ($url) use ($mailing_id, $queue_id, $getTrackerURL) {
-        if (strpos($url, '{') !== FALSE) {
-          $data = BaseClickTracker::getTrackerURLForUrlWithTokens($url, $mailing_id, $queue_id);
-        }
-        else {
-          $data = $getTrackerURL($url, $mailing_id, $queue_id);
-        }
-        return $data;
+      function ($url) use ($mailing_id, $queue_id) {
+        return \CRM_Mailing_BAO_TrackableURL::getTrackerURL($url, $mailing_id,
+          $queue_id);
       }
     );
   }

ext/flexmailer/tests/phpunit/Civi/FlexMailer/ClickTrackerTest.php

@@ -1,19 +1,19 @@
 <?php
+namespace Civi\FlexMailer;
 
 use Civi\Test\HeadlessInterface;
 use Civi\Test\HookInterface;
 use Civi\Test\TransactionalInterface;
 
 use Civi\FlexMailer\ClickTracker\TextClickTracker;
 use Civi\FlexMailer\ClickTracker\HtmlClickTracker;
-use Civi\FlexMailer\ClickTracker\BaseClickTracker;
 
 /**
  * Tests that URLs are converted to tracked ones if at all possible.
  *
  * @group headless
  */
-class Civi_FlexMailer_ClickTrackerTest extends \PHPUnit\Framework\TestCase implements HeadlessInterface, HookInterface, TransactionalInterface {
+class ClickTrackerTest extends \PHPUnit\Framework\TestCase implements HeadlessInterface, HookInterface, TransactionalInterface {
 
   protected $mailing_id;
 
@@ -27,16 +27,17 @@ public function setUpHeadless() {
 
   public function setUp() {
     // Mock the getTrackerURL call; we don't need to test creating a row in a table.
-    BaseClickTracker::$getTrackerURL = function($a, $b, $c) {
-      return 'http://example.com/extern?u=1&qid=1';
-    };
-
+    // If you want this to work without runkit, then either (a) make the dummy rows or (b) switch this to a hook/event that is runtime-configurable.
+    require_once 'CRM/Mailing/BAO/TrackableURL.php';
+    runkit7_method_rename('\CRM_Mailing_BAO_TrackableURL', 'getBasicTrackerURL', 'orig_getBasicTrackerURL');
+    runkit7_method_add('\CRM_Mailing_BAO_TrackableURL', 'getBasicTrackerURL', '$a, $b, $c', 'return \'http://example.com/extern?u=1&qid=1\';', RUNKIT7_ACC_STATIC | RUNKIT7_ACC_PRIVATE);
     parent::setUp();
   }
 
   public function tearDown() {
     // Reset the class.
-    BaseClickTracker::$getTrackerURL = ['CRM_Mailing_BAO_TrackableURL', 'getTrackerURL'];
+    runkit7_method_remove('\CRM_Mailing_BAO_TrackableURL', 'getBasicTrackerURL');
+    runkit7_method_rename('\CRM_Mailing_BAO_TrackableURL', 'orig_getBasicTrackerURL', 'getBasicTrackerURL');
     parent::tearDown();
   }
 

ext/flexmailer/tests/phpunit/Civi/FlexMailer/FlexMailerSystemTest.php

@@ -119,7 +119,16 @@ public function testUrlTracking(
    */
   public function urlTrackingExamples() {
     $cases = parent::urlTrackingExamples();
-    unset($cases[6]);
+
+    // When it comes to URLs with embedded tokens, support diverges - Flexmailer
+    // can track them, but BAO mailer cannot.
+    $cases[6] = [
+      '<p><a href="http://example.net/?id={contact.contact_id}">Foo</a></p>',
+      ';<p><a href=[\'"].*(extern/url.php|civicrm/mailing/url)(\?|&amp\\;)u=\d+.*&amp\\;id=\d+.*[\'"]>Foo</a></p>;',
+      ';\\[1\\] .*(extern/url.php|civicrm/mailing/url)[\?&]u=\d+.*&id=\d+.*;',
+      ['url_tracking' => 1],
+    ];
+
     return $cases;
   }