Don't change state AssetPublisher after publish bundle (#52)

composer.json

@@ -30,7 +30,7 @@
         "phpunit/phpunit": "^9.5",
         "roave/infection-static-analysis-plugin": "^1.7",
         "spatie/phpunit-watcher": "^1.23",
-        "vimeo/psalm": "^4.3",
+        "vimeo/psalm": "^4.4",
         "yiisoft/di": "^3.0@dev"
     },
     "autoload": {

package.json

@@ -9,6 +9,6 @@
     },
     "homepage": "https://github.com/yiisoft/assets#readme",
     "devDependencies": {
-        "sass": "^1.30.0"
+        "sass": "^1.32.4"
     }
 }

src/AssetPublisher.php

@@ -171,30 +171,34 @@ public function __construct(Aliases $aliases)
      */
     public function getAssetUrl(AssetBundle $bundle, string $assetPath): string
     {
+        $this->checkBundleData($bundle);
+
         $asset = AssetUtil::resolveAsset($bundle, $assetPath, $this->assetMap);
 
         if (!empty($asset)) {
             $assetPath = $asset;
         }
 
-        if (!$bundle->cdn) {
-            $this->checkBasePath($bundle->basePath);
-            $this->checkBaseUrl($bundle->baseUrl);
+        if ($bundle->cdn) {
+            return $this->baseUrl . '/' . $assetPath;
         }
 
         if (!AssetUtil::isRelative($assetPath) || strncmp($assetPath, '/', 1) === 0) {
             return $assetPath;
         }
 
-        if (!is_file("$this->basePath/$assetPath")) {
-            throw new InvalidConfigException("Asset files not found: '$this->basePath/$assetPath'.");
+        $path = $this->getBundleBasePath($bundle) . '/' . $assetPath;
+        $url = $this->getBundleBaseUrl($bundle) . '/' . $assetPath;
+
+        if (!is_file($path)) {
+            throw new InvalidConfigException("Asset files not found: '$path'.");
         }
 
-        if ($this->appendTimestamp  && ($timestamp = FileHelper::lastModifiedTime("$this->basePath/$assetPath")) > 0) {
-            return "$this->baseUrl/$assetPath?v=$timestamp";
+        if ($this->appendTimestamp && ($timestamp = FileHelper::lastModifiedTime("$path")) > 0) {
+            return $url . '?v=' . $timestamp;
         }
 
-        return "$this->baseUrl/$assetPath";
+        return $url;
     }
 
     /**
@@ -232,13 +236,12 @@ public function loadBundle(string $name, array $config = []): AssetBundle
         $bundle->cssOptions = array_merge($bundle->cssOptions, $this->cssDefaultOptions);
         $bundle->jsOptions = array_merge($bundle->jsOptions, $this->jsDefaultOptions);
 
-        if (!$bundle->cdn) {
-            $this->checkBasePath($bundle->basePath);
-            $this->checkBaseUrl($bundle->baseUrl);
+        if ($bundle->cdn) {
+            return $bundle;
         }
 
         if (!empty($bundle->sourcePath)) {
-            [$bundle->basePath, $bundle->baseUrl] = ($this->publish($bundle));
+            [$bundle->basePath, $bundle->baseUrl] = $this->publish($bundle);
         }
 
         return $bundle;
@@ -285,17 +288,13 @@ public function publish(AssetBundle $bundle): array
             return $this->published[$bundle->sourcePath];
         }
 
-        $this->checkBasePath($bundle->basePath);
-        $this->checkBaseUrl($bundle->baseUrl);
+        $this->checkBundleData($bundle);
 
         if (!file_exists($this->aliases->get($bundle->sourcePath))) {
             throw new InvalidConfigException("The sourcePath to be published does not exist: $bundle->sourcePath");
         }
 
-        return $this->published[$bundle->sourcePath] = $this->publishDirectory(
-            $bundle->sourcePath,
-            $bundle->publishOptions
-        );
+        return $this->published[$bundle->sourcePath] = $this->publishBundleDirectory($bundle);
     }
 
     /**
@@ -469,46 +468,38 @@ public function setLinkAssets(bool $value): void
         $this->linkAssets = $value;
     }
 
+    private function getBundleBasePath(AssetBundle $bundle): string
+    {
+        return $this->aliases->get(empty($bundle->basePath) ? $this->basePath : $bundle->basePath);
+    }
+
+    private function getBundleBaseUrl(AssetBundle $bundle): string
+    {
+        return $this->aliases->get($bundle->baseUrl === null ? $this->baseUrl : $bundle->baseUrl);
+    }
+
     /**
-     * Verify the {@see basePath} of AssetPublisher and AssetBundle is valid.
+     * Verify the {@see basePath} and the {@see baseUrl} of AssetPublisher and AssetBundle is valid.
      *
-     * @param string|null $basePath
+     * @param AssetBundle $bundle
      *
      * @throws InvalidConfigException
      */
-    private function checkBasePath(?string $basePath): void
+    private function checkBundleData(AssetBundle $bundle): void
     {
-        if (empty($this->basePath) && empty($basePath)) {
+        if (!$bundle->cdn && empty($this->basePath) && empty($bundle->basePath)) {
             throw new InvalidConfigException(
                 'basePath must be set in AssetPublisher->setBasePath($path) or ' .
                 'AssetBundle property public ?string $basePath = $path'
             );
         }
 
-        if (!empty($basePath)) {
-            $this->basePath = $this->aliases->get($basePath);
-        }
-    }
-
-    /**
-     * Verify the {@see baseUrl} of AssetPublisher and AssetBundle is valid.
-     *
-     * @param string|null $baseUrl
-     *
-     * @throws InvalidConfigException
-     */
-    private function checkBaseUrl(?string $baseUrl): void
-    {
-        if (!isset($this->baseUrl) && $baseUrl === null) {
+        if (!$bundle->cdn && !isset($this->baseUrl) && $bundle->baseUrl === null) {
             throw new InvalidConfigException(
                 'baseUrl must be set in AssetPublisher->setBaseUrl($path) or ' .
                 'AssetBundle property public ?string $baseUrl = $path'
             );
         }
-
-        if ($baseUrl !== null) {
-            $this->baseUrl = $this->aliases->get($baseUrl);
-        }
     }
 
     /**
@@ -531,23 +522,19 @@ private function hash(string $path): string
     }
 
     /**
-     * Publishes a directory.
-     *
-     * @param string $src the asset directory to be published
-     * @param array $options the options to be applied when publishing a directory. The following options are
-     * supported:
+     * Publishes a bundle directory.
      *
-     * - only: patterns that the file paths should match if they want to be copied.
+     * @param AssetBundle $bundle
      *
      * @throws Exception if the asset to be published does not exist.
      *
      * @return array the path directory and the URL that the asset is published as.
      */
-    private function publishDirectory(string $src, array $options): array
+    private function publishBundleDirectory(AssetBundle $bundle): array
     {
-        $src = $this->aliases->get($src);
+        $src = $this->aliases->get($bundle->sourcePath);
         $dir = $this->hash($src);
-        $dstDir = $this->basePath . '/' . $dir;
+        $dstDir = $this->getBundleBasePath($bundle) . '/' . $dir;
 
         if ($this->linkAssets) {
             if (!is_dir($dstDir)) {
@@ -561,12 +548,12 @@ private function publishDirectory(string $src, array $options): array
                 }
             }
         } elseif (
-            !empty($options['forceCopy']) ||
-            ($this->forceCopy && !isset($options['forceCopy'])) ||
+            !empty($bundle->publishOptions['forceCopy']) ||
+            ($this->forceCopy && !isset($bundle->publishOptions['forceCopy'])) ||
             !is_dir($dstDir)
         ) {
             $opts = array_merge(
-                $options,
+                $bundle->publishOptions,
                 [
                     'dirMode' => $this->dirMode,
                     'fileMode' => $this->fileMode,
@@ -577,6 +564,6 @@ private function publishDirectory(string $src, array $options): array
             FileHelper::copyDirectory($src, $dstDir, $opts);
         }
 
-        return [$dstDir, $this->baseUrl . '/' . $dir];
+        return [$dstDir, $this->getBundleBaseUrl($bundle) . '/' . $dir];
     }
 }

tests/AssetBundleTest.php

@@ -95,24 +95,6 @@ public function testBaseUrlEmptyString(): void
         $this->assetManager->register([RootAsset::class]);
     }
 
-    public function testBaseUrlEmptyStringChain(): void
-    {
-        $this->assetManager->setBundles(
-            [
-                RootAsset::class => [
-                    'depends' => [BaseAsset::class],
-                ],
-                BaseAsset::class => [
-                    'baseUrl' => null,
-                ],
-            ]
-        );
-
-        $this->assertEmpty($this->assetManager->getAssetBundles());
-
-        $this->assetManager->register([RootAsset::class]);
-    }
-
     public function testBaseUrlIsNotSetException(): void
     {
         $this->assetManager->setBundles(

tests/AssetPublisherTest.php

@@ -4,16 +4,15 @@
 
 namespace Yiisoft\Assets\Tests;
 
+use Yiisoft\Assets\AssetPublisher;
 use Yiisoft\Assets\Exception\InvalidConfigException;
 use Yiisoft\Assets\Tests\stubs\BaseAsset;
 use Yiisoft\Assets\Tests\stubs\JqueryAsset;
 use Yiisoft\Assets\Tests\stubs\SourceAsset;
+use Yiisoft\Assets\Tests\stubs\WithoutBaseAsset;
 use Yiisoft\Files\FileHelper;
 use Yiisoft\Files\PathMatcher\PathMatcher;
 
-/**
- * AssetPublisherTest.
- */
 final class AssetPublisherTest extends TestCase
 {
     protected function tearDown(): void
@@ -406,4 +405,14 @@ private function verifySourcesPublishedBySymlink(): SourceAsset
 
         return $bundle;
     }
+
+    public function testPublishWithAndWithoutBasePath(): void
+    {
+        $publisher = new AssetPublisher($this->aliases);
+        $publisher->publish(new SourceAsset());
+
+        $this->expectException(InvalidConfigException::class);
+        $this->expectExceptionMessageMatches('/^basePath must be set in AssetPublisher->setBasePath\(\$path\)/');
+        $publisher->publish(new WithoutBaseAsset());
+    }
 }

tests/stubs/WithoutBaseAsset.php

@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Yiisoft\Assets\Tests\stubs;
+
+use Yiisoft\Assets\AssetBundle;
+
+final class WithoutBaseAsset extends AssetBundle
+{
+    public ?string $sourcePath = '@root/tests/public/without-base';
+
+    public array $css = [
+        'stub.css',
+    ];
+}