Merge pull request #14353 from craftcms/bugfix/subpath-urls

Change generateUrl signature

CHANGELOG-WIP.md

@@ -439,6 +439,7 @@
 - `craft\fields\fieldlayoutelements\BaseUiElement::selectorIcon()` can now return a system icon name. ([#14169](https://github.com/craftcms/cms/pull/14169))
 - `craft\gql\mutations\Entry::createSaveMutations()` now accepts a `$section` argument.
 - `craft\helpers\App::parseEnv()` now returns `null` when a missing environment variable name is passed to it. ([#14253](https://github.com/craftcms/cms/pull/14253))
+- `craft\helpers\Assets::generateUrl()` no longer has an `$fs` argument. ([#14353](https://github.com/craftcms/cms/pull/14353))
 - `craft\helpers\Cp::fieldHtml()` now supports a `labelExtra` config value.
 - `craft\helpers\Db::parseParam()`, `parseDateParam()`, `parseMoneyParam()`, and `parseNumericParam()` now return `null` instead of an empty string if no condition should be applied.
 - `craft\helpers\Html::id()` and `Craft.formatInputId()` now retain colons and periods, and ensure the string begins with a letter.

CHANGELOG.md

@@ -15,6 +15,7 @@
 - Added `craft\services\Relations::deleteLeftoverRelations()`. ([#13956](https://github.com/craftcms/cms/issues/13956))
 - Added `craft\services\Search::shouldCallSearchElements()`. ([#14293](https://github.com/craftcms/cms/issues/14293))
 - `craft\behaviors\SessionBehavior::setSuccess()` and `getSuccess()` use the `success` flash key now, rather than `notice`. ([#14345](https://github.com/craftcms/cms/pull/14345))
+- `craft\helpers\Assets::generateUrl()` no longer has an `$fs` argument. ([#14353](https://github.com/craftcms/cms/pull/14353))
 - `craft\models\FieldLayout::getField()` and `isFieldIncluded()` now now have a `$filter` argument rather than `$attribute`, and it can be set to a callable.
 - Exception response data no longer includes an `error` key with the exception message. `message` should be used instead. ([#14346](https://github.com/craftcms/cms/pull/14346))
 - Relations for fields that are no longer included in an element’s field layout are now deleted after element save. ([#13956](https://github.com/craftcms/cms/issues/13956))

src/elements/Asset.php

@@ -2078,7 +2078,7 @@ private function _url(mixed $transform = null, ?bool $immediately = null): ?stri
             return null;
         }
 
-        return Html::encodeSpaces(Assets::generateUrl($volume, $this));
+        return Html::encodeSpaces(Assets::generateUrl($this));
     }
 
     /**

src/helpers/Assets.php

@@ -81,19 +81,19 @@ public static function tempFilePath(string $extension = 'tmp'): string
     /**
      * Generates the URL for an asset.
      *
-     * @param BaseFsInterface $fs
      * @param Asset $asset
      * @param string|null $uri Asset URI to use. Defaults to the filename.
      * @param DateTime|null $dateUpdated last datetime the target of the url was updated, if known
      * @return string
      * @throws InvalidConfigException if the asset doesn’t have a filename.
      */
-    public static function generateUrl(BaseFsInterface $fs, Asset $asset, ?string $uri = null, ?DateTime $dateUpdated = null): string
+    public static function generateUrl(Asset $asset, ?string $uri = null, ?DateTime $dateUpdated = null): string
     {
+        $volume = $asset->getVolume();
         $pathParts = explode('/', $asset->folderPath . ($uri ?? $asset->getFilename()));
         $path = implode('/', array_map('rawurlencode', $pathParts));
-        $rootUrl = $fs->getRootUrl() ?? '';
-        $url = ($rootUrl !== '' ? StringHelper::ensureRight($rootUrl, '/') : '') . $path;
+        $rootUrl = $volume->getRootUrl() ?? '';
+        $url = $rootUrl . $path;
 
         if (Craft::$app->getConfig()->getGeneral()->revAssetUrls) {
             return self::revUrl($url, $asset, $dateUpdated);

tests/fixtures/data/assets.php

@@ -7,11 +7,17 @@
 
 return [
     [
-        'tempFilePath' => dirname(__FILE__, 3) . '/_craft/storage/runtime/temp/product.jpg',
-        'filename' => 'product.jpg',
+        'tempFilePath' => dirname(__FILE__, 3) . '/_craft/storage/runtime/temp/shinybrad.png',
+        'filename' => 'shinybrad.png',
         'volumeId' => '1000',
         'folderId' => '1000',
         'imageDescription' => 'Paint me like one of your French girls',
         'volumeAndMass' => 'Almost as if this field is named to throw off some tests.',
     ],
+    [
+        'tempFilePath' => dirname(__FILE__, 3) . '/_craft/storage/runtime/temp/shinybrad2.png',
+        'filename' => 'shinybrad2.png',
+        'volumeId' => '1003',
+        'folderId' => '1001',
+    ],
 ];

tests/fixtures/data/volumefolder.php

@@ -16,4 +16,14 @@
         'dateUpdated' => '2018-08-08 20:00:00',
         'uid' => '09a48e85-2f12-44a8-b82c-45b14b13d8ce',
     ],
+    'root-volume-folder-subpath' => [
+        'id' => '1001',
+        'parentId' => null,
+        'volumeId' => '1003',
+        'name' => 'Test volume 4',
+        'path' => 'test volume 4/',
+        'dateCreated' => '2018-08-08 20:00:00',
+        'dateUpdated' => '2018-08-08 20:00:00',
+        'uid' => 'a4c84a44-f3e6-4a52-a805-102c351fc6b1',
+    ],
 ];

tests/fixtures/data/volumes.php

@@ -37,4 +37,16 @@
         'uid' => 'volume-1002----------------------uid',
         'dateDeleted' => (new DateTime('now'))->sub(new DateInterval('P3M5D'))->format('Y-m-d'),
     ],
+
+    'subpath' => [
+        'id' => '1003',
+        'name' => 'Test volume 4',
+        'handle' => 'testVolume4',
+        'subpath' => 'test-subpath',
+        'fs' => 'localFs',
+        'sortOrder' => 8,
+        'fieldLayoutUid' => 'field-layout-1003----------------uid',
+        'uid' => 'volume-1003----------------------uid',
+    ],
+
 ];

tests/unit/gql/TypeResolverTest.php

@@ -72,7 +72,7 @@ public function testRunGqlResolveTest(): void
         // Not using a data provider for this because of fixture load/unload on *every* iteration.
         $data = [
             // Assets
-            [Asset::class, ['filename' => 'product.jpg'], AssetResolver::class],
+            [Asset::class, ['filename' => 'shinybrad.png'], AssetResolver::class],
             [Asset::class, ['folderId' => 1000], AssetResolver::class],
             [Asset::class, ['filename' => StringHelper::randomString(128)], AssetResolver::class],
 

tests/unit/helpers/AssetsHelperTest.php

@@ -56,9 +56,8 @@ public function testGenerateUrl(string $expected, array $params): void
 
         /** @var Asset|null $asset */
         $asset = $assetQuery->one();
-        $fs = $asset->getVolume()->getFs();
 
-        self::assertSame($expected, Assets::generateUrl($fs, $asset));
+        self::assertSame($expected, Assets::generateUrl($asset));
     }
 
     /**
@@ -193,7 +192,8 @@ public function testParseSrcsetSize(array|false $expected, mixed $size): void
     public static function generateUrlDataProvider(): array
     {
         return [
-            ['https://cdn.test.craftcms.test/test%20volume%201/product.jpg', ['volumeId' => '1000', 'filename' => 'product.jpg']],
+            ['https://cdn.test.craftcms.test/test%20volume%201/shinybrad.png', ['volumeId' => '1000', 'filename' => 'shinybrad.png']],
+            ['https://cdn.test.craftcms.test/test-subpath/test%20volume%204/shinybrad2.png', ['volumeId' => '1003', 'filename' => 'shinybrad2.png']],
         ];
     }