From ab721de49cf295ec5ee5191116af56c00a857059 Mon Sep 17 00:00:00 2001 From: Andy Miller Date: Fri, 15 Sep 2017 09:07:46 -0600 Subject: [PATCH] Improved support for Assets with query strings #1451 --- CHANGELOG.md | 1 + system/src/Grav/Common/Assets.php | 70 ++++++++++++++++++++------- tests/unit/Grav/Common/AssetsTest.php | 35 +++++++++----- 3 files changed, 76 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 343479247..eb60a1ca5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ 1. [](#improved) * Implemented `Composer\CaBundle` for SSL Certs [#1241](https://github.com/getgrav/grav/issues/1241) * Refactored the Assets sorting logic + * Improved support for Assets with query strings [#1451](https://github.com/getgrav/grav/issues/1451) 1. [](#bugfix) * Fixed issue with Image query string not being fully URL encoded [#1622](https://github.com/getgrav/grav/issues/1622) * Fixed `Page::summary()` when using delimiter and multibyte UTF8 Characters [#1644](https://github.com/getgrav/grav/issues/1644) diff --git a/system/src/Grav/Common/Assets.php b/system/src/Grav/Common/Assets.php index bc3f19416..1f3aaa6f3 100644 --- a/system/src/Grav/Common/Assets.php +++ b/system/src/Grav/Common/Assets.php @@ -184,7 +184,7 @@ class Assets // Set timestamp if (isset($config['enable_asset_timestamp']) && $config['enable_asset_timestamp'] === true) { - $this->timestamp = '?' . Grav::instance()['cache']->getKey(); + $this->timestamp = Grav::instance()['cache']->getKey(); } return $this; @@ -285,9 +285,20 @@ class Assets return $this; } + $query = []; + $modified = false; $remote = $this->isRemoteLink($asset); if (!$remote) { + + $asset_parts = parse_url($asset); + if (isset($asset_parts['query'])) { + $query[] = $asset_parts['query']; + unset($asset_parts['query']); + $asset = Uri::buildUrl($asset_parts); + } + + $modified = $this->getLastModificationTime($asset); $asset = $this->buildLocalLink($asset); } @@ -305,7 +316,8 @@ class Assets 'pipeline' => (bool) $pipeline, 'loading' => $loading ?: '', 'group' => $group ?: 'head', - 'modified' => $modified + 'modified' => $modified, + 'query' => implode('&', $query), ]; // check for dynamic array and merge with defaults @@ -538,7 +550,7 @@ class Assets } else { $media = isset($file['media']) ? sprintf(' media="%s"', $file['media']) : ''; - $output .= '' . "\n"; + $output .= '' . "\n"; } } } @@ -558,7 +570,7 @@ class Assets } else { $media = isset($file['media']) ? sprintf(' media="%s"', $file['media']) : ''; - $output .= '' . "\n"; + $output .= '' . "\n"; } } } @@ -622,7 +634,7 @@ class Assets $inline_js .= $this->gatherLinks([$file], JS_ASSET) . "\n"; } else { - $output .= '' . "\n"; + $output .= '' . "\n"; } } } @@ -641,7 +653,7 @@ class Assets $inline_js .= $this->gatherLinks([$file], JS_ASSET) . "\n"; } else { - $output .= '' . "\n"; + $output .= '' . "\n"; } } } @@ -1353,22 +1365,46 @@ class Assets */ public function setTimestamp($value) { - $this->timestamp = '?' . $value; + $this->timestamp = $value; } - public function getTimestamp($asset = null) + /** + * Get the timestamp for assets + * + * @return string + */ + public function getTimestamp() { - if (is_array($asset)) { - if ($asset['remote'] === false) { - if (Utils::contains($asset['asset'], '?')) { - return str_replace('?', '&', $this->timestamp); - } else { - return $this->timestamp; - } + return $this->timestamp; + } + + /** + * Get the full query string including any query params and timestamp + * + * @param $asset + * @return string + */ + public function getQuerystring($asset) + { + $querystring = ''; + + if (!empty($asset['query'])) { + if (Utils::contains($asset['asset'], '?')) { + $querystring .= '&' . $asset['query']; + } else { + $querystring .= '?' . $asset['query']; } - } elseif (empty($asset)) { - return $this->timestamp; } + + if ($this->timestamp) { + if (Utils::contains($asset['asset'], '?') || $querystring) { + $querystring .= '&' . $this->timestamp; + } else { + $querystring .= '?' . $this->timestamp; + } + } + + return $querystring; } /** diff --git a/tests/unit/Grav/Common/AssetsTest.php b/tests/unit/Grav/Common/AssetsTest.php index e4b055cc6..3ca9be2f8 100644 --- a/tests/unit/Grav/Common/AssetsTest.php +++ b/tests/unit/Grav/Common/AssetsTest.php @@ -43,7 +43,8 @@ class AssetsTest extends \Codeception\TestCase\Test 'pipeline' => true, 'loading' => '', 'group' => 'head', - 'modified' => false + 'modified' => false, + 'query' => '' ], reset($array)); $this->assets->add('test.js'); @@ -59,7 +60,8 @@ class AssetsTest extends \Codeception\TestCase\Test 'pipeline' => true, 'loading' => '', 'group' => 'head', - 'modified' => false + 'modified' => false, + 'query' => '' ], reset($array)); //test addCss(). Test adding asset to a separate group @@ -77,7 +79,8 @@ class AssetsTest extends \Codeception\TestCase\Test 'pipeline' => true, 'loading' => '', 'group' => 'head', - 'modified' => false + 'modified' => false, + 'query' => '' ], reset($array)); //test addCss(). Testing with remote URL @@ -95,7 +98,8 @@ class AssetsTest extends \Codeception\TestCase\Test 'pipeline' => true, 'loading' => '', 'group' => 'head', - 'modified' => false + 'modified' => false, + 'query' => '' ], reset($array)); //test addCss() adding asset to a separate group, and with an alternate rel attribute @@ -119,7 +123,8 @@ class AssetsTest extends \Codeception\TestCase\Test 'pipeline' => true, 'loading' => '', 'group' => 'head', - 'modified' => false + 'modified' => false, + 'query' => '' ], reset($array)); //Test CSS Groups @@ -139,7 +144,8 @@ class AssetsTest extends \Codeception\TestCase\Test 'pipeline' => true, 'loading' => '', 'group' => 'footer', - 'modified' => false + 'modified' => false, + 'query' => '' ], reset($array)); //Test JS Groups @@ -159,7 +165,8 @@ class AssetsTest extends \Codeception\TestCase\Test 'pipeline' => true, 'loading' => '', 'group' => 'footer', - 'modified' => false + 'modified' => false, + 'query' => '' ], reset($array)); //Test async / defer @@ -177,7 +184,8 @@ class AssetsTest extends \Codeception\TestCase\Test 'pipeline' => true, 'loading' => 'async', 'group' => 'head', - 'modified' => false + 'modified' => false, + 'query' => '' ], reset($array)); $this->assets->reset(); @@ -194,7 +202,8 @@ class AssetsTest extends \Codeception\TestCase\Test 'pipeline' => true, 'loading' => 'defer', 'group' => 'head', - 'modified' => false + 'modified' => false, + 'query' => '' ], reset($array)); //Test inline @@ -412,14 +421,14 @@ class AssetsTest extends \Codeception\TestCase\Test $this->assets->setTimestamp('foo'); $this->assets->addCSS('http://somesite.com/test.css'); $css = $this->assets->css(); - $this->assertSame('' . PHP_EOL, $css); + $this->assertSame('' . PHP_EOL, $css); // external CSS already with param $this->assets->reset(); $this->assets->setTimestamp('foo'); $this->assets->addCSS('http://somesite.com/test.css?bar'); $css = $this->assets->css(); - $this->assertSame('' . PHP_EOL, $css); + $this->assertSame('' . PHP_EOL, $css); // local JS nothing extra $this->assets->reset(); @@ -440,14 +449,14 @@ class AssetsTest extends \Codeception\TestCase\Test $this->assets->setTimestamp('foo'); $this->assets->addJs('http://somesite.com/test.js'); $css = $this->assets->js(); - $this->assertSame('' . PHP_EOL, $css); + $this->assertSame('' . PHP_EOL, $css); // external JS already with param $this->assets->reset(); $this->assets->setTimestamp('foo'); $this->assets->addJs('http://somesite.com/test.js?bar'); $css = $this->assets->js(); - $this->assertSame('' . PHP_EOL, $css); + $this->assertSame('' . PHP_EOL, $css); } public function testAddInlineCss()