Civi::url() - Split $fragment and $fragmentQuery. Define clear/consistent rule for...
authorTim Otten <totten@civicrm.org>
Tue, 25 Jul 2023 00:03:26 +0000 (17:03 -0700)
committerTim Otten <totten@civicrm.org>
Tue, 25 Jul 2023 01:12:35 +0000 (18:12 -0700)
Civi.php
Civi/Core/Url.php
tests/phpunit/Civi/Core/UrlTest.php

index c7c48001d90782e511c1fb11ba7607f4ddbdeb4c..1a23839e5872a2468940b2cf3280804999eb9a5c 100644 (file)
--- a/Civi.php
+++ b/Civi.php
@@ -219,12 +219,21 @@ class Civi {
   }
 
   /**
-   * Construct a URL based on a logical service address.
+   * Construct a URL based on a logical service address. URL building follows a few rules:
    *
-   * Ex: Link to constituent's dashboard (on frontend UI)
+   * 1. You should initialize with a baseline URL (e.g. 'frontend://civicrm/profile/view?id=123&gid=456').
+   * 2. The scheme indicates the general type of URL ('frontend://', 'backend://', 'asset://', 'assetBuilder://').
+   * 3. The URL object provides getters, setters, and adders (e.g. `getScheme()`, `setPath(...)`, `addQuery(...)`)
+   * 4. Strings are raw. Arrays are auto-encoded. (`addQuery('name=John+Doughnut')` or `addQuery(['name' => 'John Doughnut'])`)
+   * 5. You may use variable expressions (`id=[contact]&gid=[profile]`).
+   * 6. The URL can be cast to string (aka `__toString()`).
+   *
+   * Here are several examples:
+   *
+   * Ex: Link to constituent's dashboard (specifically on frontend UI)
    *   $url = Civi::url('frontend://civicrm/user?reset=1');
    *
-   * Ex: Link to constituent's dashboard (on frontend UI or backend UI -- whatever matches current page-view)
+   * Ex: Link to constituent's dashboard (on frontend UI or backend UI -- based on the active scheme of current page-view)
    *   $url = Civi::url('//civicrm/user?reset=1');
    *
    * Ex: Link to constituent's dashboard (with method calls - good for dynamic options)
index 91b20d81ce2630db136b3d6d01a939b1ad6351eb..da014967ff5087c6a32a4998d0dc704652823213 100644 (file)
@@ -12,6 +12,25 @@ namespace Civi\Core;
  * The typical way to construct a URL object is through `Civi::url()`, which features more
  * documentation and examples.
  *
+ * This class-model has several properties. Most properties follow one of two patterns:
+ *
+ *   - URL components (such as `path`, `query`, `fragment`, `fragmentQuery`).
+ *     These have getter/setter/adder methods. They are stored as raw URL substrings.
+ *   - Smart flags (such as `preferFormat`, `ssl`, `cacheCode`).
+ *     These have getter/setter methods. They are stored as simple types (booleans or strings).
+ *     They also have aliases via `__construct(...$flags)` and `useFlags($flags)`
+ *
+ * URI components (`path`, `query`, etc) can be understood as raw-strings or data-arrays. Compare:
+ *
+ *  - "Path":           "civicrm/foo+bar/whiz+bang" vs ['civicrm', 'foo bar', 'whiz bang']
+ *  - "Query:           "a=100&b=Hello+world" vs ["a" => 100, "b" => "Hello world"]
+ *  - "Fragment":       "#/mailing/new" vs ["/mailing", "/new"]
+ *  - "Fragment Query": "angularDebug=1" vs ["angularDebug" => 1]
+ *
+ * The raw-string is supported from all angles (storage+getters+setters+adders).
+ * Additionally, the setters+adders accept arrays.
+ *
+ * This cl
  * @see \Civi::url()
  */
 final class Url {
@@ -39,6 +58,11 @@ final class Url {
    */
   private $fragment;
 
+  /**
+   * @var string|null
+   */
+  private $fragmentQuery;
+
   /**
    * Whether to auto-append the cache-busting resource code.
    *
@@ -104,7 +128,9 @@ final class Url {
       $this->path .= $parsed['path'];
     }
     $this->query = $parsed['query'] ?? NULL;
-    $this->fragment = $parsed['fragment'] ?? NULL;
+    $fragmentParts = isset($parsed['fragment']) ? explode('?', $parsed['fragment'], 2) : [];
+    $this->fragment = $fragmentParts[0] ?? NULL;
+    $this->fragmentQuery = $fragmentParts[1] ?? NULL;
 
     if ($flags !== NULL) {
       $this->useFlags($flags);
@@ -113,6 +139,7 @@ final class Url {
 
   /**
    * @return string
+   *   Ex: 'frontend' or 'backend'
    */
   public function getScheme() {
     return $this->scheme;
@@ -120,6 +147,7 @@ final class Url {
 
   /**
    * @param string $scheme
+   *   Ex: 'frontend' or 'backend'
    */
   public function setScheme(string $scheme): Url {
     $this->scheme = $scheme;
@@ -127,90 +155,159 @@ final class Url {
   }
 
   /**
-   * @return mixed
+   * @return string|null
+   *   Ex: 'civicrm/event/info'
+   *   Ex: 'civicrm/hello+world%3F'
    */
   public function getPath() {
     return $this->path;
   }
 
   /**
-   * @param string $path
+   * @param string|string[]|null $path
+   *   Ex: 'civicrm/event/info'
+   *   Ex: 'civicrm/hello+world%3F'
+   *   Ex: ['civicrm', 'hello world?']
    */
-  public function setPath(string $path): Url {
-    $this->path = $path;
+  public function setPath($path): Url {
+    $this->path = static::encodePath($path);
     return $this;
   }
 
   /**
-   * @param string|string[] $pathParts
+   * Add new sections to the path.
+   *
+   * When adding new parts to the path, there is an implicit delimiter ('/') between parts.
+   *
+   * @param string|string[] $path
+   *   Ex: 'civicrm/event/info'
+   *   Ex: 'civicrm/hello+world%3F'
+   *   Ex: ['civicrm', 'hello world?']
    * @return $this
    */
-  public function addPath($pathParts): Url {
-    $suffix = implode('/', (array) $pathParts);
-    if ($this->path === NULL) {
-      $this->path = $suffix;
-    }
-    else {
-      $this->path = rtrim($this->path, '/') . '/' . $suffix;
-    }
+  public function addPath($path): Url {
+    static::appendString($this->path, '/', static::encodePath($path));
     return $this;
   }
 
   /**
    * @return string|null
+   *   Ex: 'name=John+Doughnut&id=9'
    */
   public function getQuery(): ?string {
     return $this->query;
   }
 
   /**
-   * @param string|array|null $query
+   * @param string|string[]|null $query
+   *   Ex: 'name=John+Doughnut&id=9'
+   *   Ex: ['name' => 'John Doughnut', 'id' => 9]
+   * @return $this
    */
   public function setQuery($query): Url {
-    $this->query = \CRM_Utils_System::makeQueryString($query);
+    if (is_array($query)) {
+      $query = \CRM_Utils_System::makeQueryString($query);
+    }
+    $this->query = $query;
     return $this;
   }
 
   /**
-   * @param string|array $query
+   * @param string|string[] $query
+   *   Ex: 'name=John+Doughnut&id=9'
+   *   Ex: ['name' => 'John Doughnut', 'id' => 9]
    * @return $this
    */
   public function addQuery($query): Url {
-    if ($this->query === NULL) {
-      $this->query = \CRM_Utils_System::makeQueryString($query);
-    }
-    else {
-      $this->query .= '&' . \CRM_Utils_System::makeQueryString($query);
+    if (is_array($query)) {
+      $query = \CRM_Utils_System::makeQueryString($query);
     }
+    static::appendString($this->query, '&', $query);
     return $this;
   }
 
   /**
+   * Get the primary fragment.
+   *
+   * NOTE: This is the primary fragment identifier (as in `#id` or `#/client/side/route`).
+   * and does not include fregment queries. (as in '#?angularDebug=1').
+   *
    * @return string|null
+   *   Ex: '/mailing/new'
+   *   Ex: '/foo+bar%3F/newish%3F'
+   * @see Url::getFragmentQuery()
+   * @see Url::composeFragment()
    */
   public function getFragment(): ?string {
     return $this->fragment;
   }
 
   /**
-   * @param string|null $fragment
+   * Replace the fragment.
+   *
+   * NOTE: This is the primary fragment identifier (as in `#id` or `#/client/side/route`).
+   * and does not include fregment queries. (as in '#?angularDebug=1').
+   *
+   * @param string|string[]|null $fragment
+   *   Ex: '/mailing/new'
+   *   Ex: '/foo+bar/newish%3F'
+   *   Ex: ['', 'foo bar', 'newish?']
+   * @return $this
+   * @see Url::setFragmentQuery()
+   * @see url::composeFragment()
    */
-  public function setFragment(?string $fragment): Url {
-    $this->fragment = \CRM_Utils_System::makeQueryString($fragment);
+  public function setFragment($fragment): Url {
+    $this->fragment = static::encodePath($fragment);
     return $this;
   }
 
   /**
-   * @param string|array $fragment
+   * Add to fragment.
+   *
+   * @param string|string[] $fragment
+   *   Ex: 'mailing/new'
+   *   Ex: 'foo+bar/newish%3F'
+   *   Ex: ['foo bar', 'newish?']
    * @return $this
    */
   public function addFragment($fragment): Url {
-    if ($this->fragment === NULL) {
-      $this->fragment = \CRM_Utils_System::makeQueryString($fragment);
+    static::appendString($this->fragment, '/', static::encodePath($fragment));
+    return $this;
+  }
+
+  /**
+   * @return string|null
+   *   Ex: 'name=John+Doughnut&id=9'
+   */
+  public function getFragmentQuery(): ?string {
+    return $this->fragmentQuery;
+  }
+
+  /**
+   * @param string|string[]|null $fragmentQuery
+   *   Ex: 'name=John+Doughnut&id=9'
+   *   Ex: ['name' => 'John Doughnut', 'id' => 9]
+   * @return $this
+   */
+  public function setFragmentQuery($fragmentQuery) {
+    if (is_array($fragmentQuery)) {
+      $fragmentQuery = \CRM_Utils_System::makeQueryString($fragmentQuery);
     }
-    else {
-      $this->fragment .= '&' . \CRM_Utils_System::makeQueryString($fragment);
+    $this->fragmentQuery = $fragmentQuery;
+    return $this;
+  }
+
+  /**
+   * @param string|array $fragmentQuery
+   *   Ex: 'name=John+Doughnut&id=9'
+   *   Ex: ['name' => 'John Doughnut', 'id' => 9]
+   * @return $this
+   */
+  public function addFragmentQuery($fragmentQuery): Url {
+    if (is_array($fragmentQuery)) {
+      $fragmentQuery = \CRM_Utils_System::makeQueryString($fragmentQuery);
     }
+    static::appendString($this->fragmentQuery, '&', $fragmentQuery);
     return $this;
   }
 
@@ -402,11 +499,11 @@ final class Url {
     switch ($scheme) {
       case 'frontend':
       case 'service':
-        $result = $userSystem->url($this->getPath(), $this->getQuery(), $preferFormat === 'absolute', $this->getFragment(), TRUE, FALSE, FALSE);
+        $result = $userSystem->url($this->getPath(), $this->getQuery(), $preferFormat === 'absolute', $this->composeFragment(), TRUE, FALSE, FALSE);
         break;
 
       case 'backend':
-        $result = $userSystem->url($this->getPath(), $this->getQuery(), $preferFormat === 'absolute', $this->getFragment(), FALSE, TRUE, FALSE);
+        $result = $userSystem->url($this->getPath(), $this->getQuery(), $preferFormat === 'absolute', $this->composeFragment(), FALSE, TRUE, FALSE);
         break;
 
       case 'assetBuilder':
@@ -465,15 +562,24 @@ final class Url {
 
   private function composeSuffix(): string {
     $result = '';
-    if ($this->query) {
+    if ($this->query !== NULL && $this->query !== '') {
       $result .= '?' . $this->query;
     }
-    if ($this->fragment) {
-      $result .= '#' . $this->fragment;
+    $fragment = $this->composeFragment();
+    if ($fragment !== NULL && $fragment !== '') {
+      $result .= '#' . $fragment;
     }
     return $result;
   }
 
+  private function composeFragment(): ?string {
+    $fragment = $this->fragment;
+    if ($this->fragmentQuery !== NULL && $this->fragmentQuery !== '') {
+      $fragment = ($fragment ?: '') . '?' . $this->fragmentQuery;
+    }
+    return $fragment;
+  }
+
   private static function detectFormat(): string {
     // Some environments may override default - e.g. cv-cli prefers absolute URLs
     // WISHLIST: If handling `Job.*`, then 'absolute'
@@ -502,4 +608,42 @@ final class Url {
     return \CRM_Core_Config::singleton()->userSystem->isFrontEndPage() ? 'frontend' : 'backend';
   }
 
+  /**
+   * @param string|string[]|null $path
+   *   Ex: 'greet/hello+world/en'
+   *   Ex: ['greet', 'hello world', 'en']
+   * @return string|null
+   *   Ex: 'greet/hello+world/en'
+   */
+  private static function encodePath($path): ?string {
+    if (is_array($path)) {
+      $encodedArray = array_map('urlencode', $path);
+      return implode('/', $encodedArray);
+    }
+    else {
+      return $path;
+    }
+  }
+
+  private static function appendString(?string &$var, string $separator, ?string $value): void {
+    if ($value === NULL) {
+      return;
+    }
+
+    if ($var === NULL) {
+      $var = $value;
+      return;
+    }
+
+    // Dedupe separators
+    if (str_ends_with($var, $separator)) {
+      $var = rtrim($var, $separator);
+    }
+    if ($value[0] === $separator) {
+      $value = ltrim($value, $separator);
+    }
+
+    $var = $var . $separator . $value;
+  }
+
 }
index 35c261f8afe77916ff09a55d11b32afed999479e..2fdcc779d6834a3d5bd943069b62f7c2d5857000 100644 (file)
@@ -51,15 +51,17 @@ class UrlTest extends \CiviUnitTestCase {
   public function testPath() {
     $examples = [];
     $examples[] = ['civicrm/ajax/api4', Civi::url('service://civicrm/ajax/api4')];
-    $examples[] = ['civicrm/ajax/api4/Contact/get', Civi::url('service://civicrm/ajax/api4')->addPath('Contact/get')];
-    $examples[] = ['civicrm/ajax/api4/Contact/get', Civi::url('service://civicrm/ajax/api4')->addPath('Contact')->addPath('get')];
+    $examples[] = ['civicrm/ajax/api4/Contact/get+stuff', Civi::url('service://civicrm/ajax/api4/Contact/get+stuff')];
+    $examples[] = ['civicrm/ajax/api4/Contact/get+stuff', Civi::url('service://civicrm/ajax/api4')->addPath(['Contact', 'get stuff'])];
+    $examples[] = ['civicrm/ajax/api4/Contact/get+stuff', Civi::url('service://civicrm/ajax/api4/Contact')->addPath('get+stuff')];
+    $examples[] = ['civicrm/ajax/api4/Contact/get+stuff', Civi::url('service://civicrm/ajax/api4/Contact')->addPath(['get stuff'])];
     $examples[] = ['civicrm/new-path', Civi::url('service://civicrm/old-path')->setPath('civicrm/new-path')];
 
     foreach ($examples as $key => $example) {
       /** @var \Civi\Core\Url $url */
       [$expected, $url] = $example;
-      $this->assertEquals($expected, $url->getPath());
-      $this->assertStringContainsString($expected, (string) $url);
+      $this->assertEquals($expected, $url->getPath(), sprintf("%s at %d should be have matching property", __FUNCTION__, $key));
+      $this->assertStringContainsString($expected, (string) $url, sprintf("%s at %d should be have matching output", __FUNCTION__, $key));
     }
   }
 
@@ -69,15 +71,59 @@ class UrlTest extends \CiviUnitTestCase {
     $examples[] = ['reset=1&id=9', Civi::url('frontend://civicrm/profile/view')->addQuery('reset=1&id=9')];
     $examples[] = ['reset=1&id=9', Civi::url('frontend://civicrm/profile/view')->addQuery(['reset' => 1, 'id' => 9])];
     $examples[] = ['noise=Hello+world%3F', Civi::url('frontend://civicrm/profile/view?noise=Hello+world%3F')];
+    $examples[] = ['noise=Hello+world%3F', Civi::url('frontend://civicrm/profile/view')->addQuery('noise=Hello+world%3F')];
     $examples[] = ['noise=Hello+world%3F', Civi::url('frontend://civicrm/profile/view')->addQuery(['noise' => 'Hello world?'])];
     $examples[] = ['reset=1&id=9', Civi::url('frontend://civicrm/profile/view?forget=this')->setQuery('reset=1&id=9')];
+    $examples[] = ['reset=1&id=9', Civi::url('frontend://civicrm/profile/view?forget=this')->setQuery(['reset' => 1, 'id' => 9])];
     $examples[] = ['reset=1&id=9', Civi::url('frontend://civicrm/profile/view?forget=this')->setQuery('reset=1')->addQuery('id=9')];
 
     foreach ($examples as $key => $example) {
       /** @var \Civi\Core\Url $url */
       [$expected, $url] = $example;
-      $this->assertEquals($expected, $url->getQuery());
-      $this->assertStringContainsString($expected, (string) $url);
+      $this->assertEquals($expected, $url->getQuery(), sprintf("%s at %d should be have matching property", __FUNCTION__, $key));
+      $this->assertStringContainsString($expected, (string) $url, sprintf("%s at %d should be have matching output", __FUNCTION__, $key));
+    }
+  }
+
+  public function testFragment() {
+    $examples = [];
+    $examples[] = ['/mailing/new', Civi::url('frontend://civicrm/a/#/mailing/new')];
+    $examples[] = ['/mailing/new', Civi::url('frontend://civicrm/a/#/')->addFragment('mailing/new')];
+    $examples[] = ['/mailing/new', Civi::url('frontend://civicrm/a/#/')->addFragment('/mailing/new')];
+    $examples[] = ['/mailing/new', Civi::url('frontend://civicrm/a/#/')->addFragment(['mailing', 'new'])];
+    $examples[] = [NULL, Civi::url('frontend://civicrm/a/#/mailing/new')->setFragment(NULL)];
+    $examples[] = ['/mailing/new+stuff', Civi::url('frontend://civicrm/a/#/mailing/new+stuff?extra=1')];
+    $examples[] = ['/mailing/new+stuff', Civi::url('frontend://civicrm/a/#/mailing?extra=1')->addFragment('new+stuff')];
+    $examples[] = ['/mailing/new+stuff', Civi::url('frontend://civicrm/a/#/mailing?extra=1')->addFragment(['new stuff'])];
+    $examples[] = ['/mailing/new+stuff', Civi::url('frontend://civicrm/a/#/ignore?extra=1')->setFragment('/mailing/new+stuff')];
+    $examples[] = ['/mailing/new+stuff', Civi::url('frontend://civicrm/a/#/ignore?extra=1')->setFragment(['', 'mailing', 'new stuff'])];
+
+    foreach ($examples as $key => $example) {
+      /** @var \Civi\Core\Url $url */
+      [$expected, $url] = $example;
+      $this->assertEquals($expected, $url->getFragment(), sprintf("%s at %d should be have matching property", __FUNCTION__, $key));
+      if ($expected !== NULL) {
+        $this->assertStringContainsString($expected, (string) $url, sprintf("%s at %d should be have matching output", __FUNCTION__, $key));
+      }
+    }
+  }
+
+  public function testFragmentQuery() {
+    $examples = [];
+    $examples[] = ['angularDebug=1&extra=hello+world%3F', Civi::url('frontend://civicrm/a/#/mailing/new?angularDebug=1&extra=hello+world%3F')];
+    $examples[] = ['angularDebug=1&extra=hello+world%3F', Civi::url('frontend://civicrm/a/#/mailing/new?angularDebug=1')->addFragmentQuery('extra=hello+world%3F')];
+    $examples[] = ['angularDebug=1&extra=hello+world%3F', Civi::url('frontend://civicrm/a/#/mailing/new')->addFragmentQuery('angularDebug=1&extra=hello+world%3F')];
+    $examples[] = ['angularDebug=1&extra=hello+world%3F', Civi::url('frontend://civicrm/a/#/mailing/new')->addFragmentQuery(['angularDebug' => 1, 'extra' => 'hello world?'])];
+    $examples[] = ['angularDebug=1&extra=hello+world%3F', Civi::url('frontend://civicrm/a/#/mailing/new')->setFragmentQuery('angularDebug=1&extra=hello+world%3F')];
+    $examples[] = ['angularDebug=1&extra=hello+world%3F', Civi::url('frontend://civicrm/a/#/mailing/new')->setFragmentQuery(['angularDebug' => 1, 'extra' => 'hello world?'])];
+
+    foreach ($examples as $key => $example) {
+      /** @var \Civi\Core\Url $url */
+      [$expected, $url] = $example;
+      $this->assertEquals($expected, $url->getFragmentQuery(), sprintf("%s at %d should be have matching property", __FUNCTION__, $key));
+      if ($expected !== NULL) {
+        $this->assertStringContainsString($expected, (string) $url, sprintf("%s at %d should be have matching output", __FUNCTION__, $key));
+      }
     }
   }