Improved support for Assets with query strings #1451

2026-01-27 18:00:07 +01:00 · 2017-09-15 09:07:46 -06:00
parent 133e83f89d
commit ab721de49c
3 changed files with 76 additions and 30 deletions
--- 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)
--- 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 .= '<link href="' . $file['asset'] . $this->getTimestamp($file) . '"' . $attributes . $media . ' />' . "\n";
+                        $output .= '<link href="' . $file['asset'] . $this->getQuerystring($file) . '"' . $attributes . $media . ' />' . "\n";
                    }
                }
            }
@@ -558,7 +570,7 @@ class Assets
                    }
                    else {
                        $media = isset($file['media']) ? sprintf(' media="%s"', $file['media']) : '';
-                        $output .= '<link href="' . $file['asset'] . $this->getTimestamp($file) . '"' . $attributes . $media . ' />' . "\n";
+                        $output .= '<link href="' . $file['asset'] . $this->getQuerystring($file) . '"' . $attributes . $media . ' />' . "\n";
                    }
                }
            }
@@ -622,7 +634,7 @@ class Assets
                        $inline_js .= $this->gatherLinks([$file], JS_ASSET) . "\n";
                    }
                    else {
-                        $output .= '<script src="' . $file['asset'] . $this->getTimestamp($file) . '"' . $attributes . ' ' . $file['loading'] . '></script>' . "\n";
+                        $output .= '<script src="' . $file['asset'] . $this->getQuerystring($file) . '"' . $attributes . ' ' . $file['loading'] . '></script>' . "\n";
                    }
                }
            }
@@ -641,7 +653,7 @@ class Assets
                        $inline_js .= $this->gatherLinks([$file], JS_ASSET) . "\n";
                    }
                    else {
-                        $output .= '<script src="' . $file['asset'] . $this->getTimestamp($file) . '"' . $attributes . ' ' . $file['loading'] . '></script>' . "\n";
+                        $output .= '<script src="' . $file['asset'] . $this->getQuerystring($file) . '"' . $attributes . ' ' . $file['loading'] . '></script>' . "\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;
    }

    /**
--- 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('<link href="http://somesite.com/test.css" type="text/css" rel="stylesheet" />' . PHP_EOL, $css);
+        $this->assertSame('<link href="http://somesite.com/test.css?foo" type="text/css" rel="stylesheet" />' . 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('<link href="http://somesite.com/test.css?bar" type="text/css" rel="stylesheet" />' . PHP_EOL, $css);
+        $this->assertSame('<link href="http://somesite.com/test.css?bar&foo" type="text/css" rel="stylesheet" />' . 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('<script src="http://somesite.com/test.js" type="text/javascript" ></script>' . PHP_EOL, $css);
+        $this->assertSame('<script src="http://somesite.com/test.js?foo" type="text/javascript" ></script>' . 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('<script src="http://somesite.com/test.js?bar" type="text/javascript" ></script>' . PHP_EOL, $css);
+        $this->assertSame('<script src="http://somesite.com/test.js?bar&foo" type="text/javascript" ></script>' . PHP_EOL, $css);
    }

    public function testAddInlineCss()