Make $civicrm_paths less sensitive to trailing slashes. Add tests.

[totten]

Overview

Recall that the default value of imageUploadURL is [civicrm.files]/persist/contribute/. Now, suppose you have one of these two configurations in civicrm.settings.php:

// (1) Trailing slash configuration
$civicrm_paths['civicrm.files']['url'] = 'http://tmpd8-clean.bknix:8001/sites/default/files/civicrm/';
$civicrm_paths['civicrm.files']['path'] = '/Users/totten/bknix/build/tmpd8-clean/web/sites/default/files/civicrm/';

// (2) Trimmed slash configuration
$civicrm_paths['civicrm.files']['url'] = 'http://tmpd8-clean.bknix:8001/sites/default/files/civicrm';
$civicrm_paths['civicrm.files']['path'] = '/Users/totten/bknix/build/tmpd8-clean/web/sites/default/files/civicrm';

You could inspect to see if the URL is generated correctly by running these commands:

$ cv api setting.get return=imageUploadURL
$ cv url -c imageUploadURL
$ cv url -d "[civicrm.files]/persist/contribute/"
$ cv path -c imageUploadDir
$ cv path -d "[civicrm.files]/persist/contribute/"

Before

Under either configuration (trailing-slash or trimmed-slash), you'll find that paths are generated properly (cv path ... or Civi::paths()->getPath(...)).

For generating URLs (cv url ... or Civi::paths()->getUrl(...)), only the trailing-slash cfg works. The trimmed-slash cfg leads to a bad URL with a missing slash.

[bknix-max:~/bknix/build/tmpd8prj-clean/web] cv api setting.get return=imageUploadURL
{
    "is_error": 0,
    "version": 3,
    "count": 1,
    "id": 1,
    "values": {
        "1": {
            "imageUploadURL": "[civicrm.files]/persist/contribute/"
        }
    }
}

[bknix-max:~/bknix/build/tmpd8prj-clean/web] cv url -c imageUploadURL
"http://tmpd8prj-clean.bknix:8001/sites/default/files/civicrmpersist/contribute"

[bknix-max:~/bknix/build/tmpd8prj-clean/web] cv url -d "[civicrm.files]/persist/contribute/"
"http://tmpd8prj-clean.bknix:8001/sites/default/files/civicrmpersist/contribute/"

This is surprising because the path expression ([civicrm.files]/persist/contribute) includes a slash... but it disappears in the final computation.

After

Both configurations work, for paths and for URLs.

Comments

This fixes an issue in which the git-based Civi-D8 sites have a malfunctioning menu -- because the asset URLs are malformed.

This PR is an extracted subset of #16328, which was an exploratory branch aimed at supporting deployment of Civi git repos in D8 via composer. The changes are extracted to make the size of the review more manageable. It's probably best to use this smaller PR to consider topics like regression-risk and general code convention rather than trying to assess the fuller aims of #16328.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

Nice - makes sense & tests are really solid

[eileenmcnaughton]

This turns out to have caused a wp regreession & possibly Joomla!

IN WP it manifests when clean urls are not enabled - in this url the / after admin.php is a bug

https://www.yeswhangarei.co.nz/wp-admin/admin.php/?page=CiviCRM&q=civicrm%2Fupgrade%2Fqueue%2Fajax%2FrunNext

@kcristiano

@totten can we revert it in the release / rc as a short term fix?