feat(plus): disallow setting media path for cloud storages

app/Http/Requests/API/SettingRequest.php

@@ -2,6 +2,8 @@
 
 namespace App\Http\Requests\API;
 
+use App\Rules\MediaPath;
+
 /**
  * @property-read string $media_path
  */
@@ -11,7 +13,7 @@ class SettingRequest extends Request
     public function rules(): array
     {
         return [
-            'media_path' => 'string|required|path.valid',
+            'media_path' => ['string', new MediaPath()],
         ];
     }
 }

app/Models/Setting.php

@@ -33,7 +33,7 @@ class Setting extends Model
      * @param array|string $key the key of the setting, or an associative array of settings,
      *                            in which case $value will be discarded
      */
-    public static function set(array|string $key, $value = ''): void
+    public static function set(array|string $key, $value): void
     {
         if (is_array($key)) {
             foreach ($key as $k => $v) {
@@ -45,4 +45,9 @@ class Setting extends Model
 
         self::query()->updateOrCreate(compact('key'), compact('value'));
     }
+
+    public static function unset(string $key): void
+    {
+        static::set($key, null);
+    }
 }

app/Providers/AppServiceProvider.php

@@ -33,9 +33,6 @@ class AppServiceProvider extends ServiceProvider
             $db->statement($db->raw('PRAGMA foreign_keys = ON'));
         }
 
-        // Add some custom validation rules
-        $validator->extend('path.valid', static fn ($attribute, $value): bool => is_dir($value) && is_readable($value));
-
         // disable wrapping JSON resource in a `data` key
         JsonResource::withoutWrapping();
 

app/Rules/MediaPath.php

@@ -0,0 +1,28 @@
+<?php
+
+namespace App\Rules;
+
+use Illuminate\Contracts\Validation\Rule;
+use Illuminate\Support\Facades\File;
+
+class MediaPath implements Rule
+{
+    public function passes($attribute, $value): bool
+    {
+        if (config('koel.storage_driver') === 'local') {
+            return $value && File::isDirectory($value) && File::isReadable($value);
+        }
+
+        // Setting a media path is not required for non-local storage drivers.
+        return false;
+    }
+
+    public function message(): string
+    {
+        if (config('koel.storage_driver') === 'local') {
+            return 'Media path is required for local storage.';
+        }
+
+        return 'Media path is not required for non-local storage.';
+    }
+}

phpunit.xml.dist

@@ -35,6 +35,7 @@
         <env name="APP_ENV" value="testing"/>
         <env name="APP_URL" value="http://localhost/"/>
         <env name="APP_KEY" value="16efa6c23c2e8c705826b0e66778fbe7"/>
+        <env name="STORAGE_DRIVER" value="local"/>
         <env name="CACHE_DRIVER" value="array"/>
         <env name="SCOUT_DRIVER" value="tntsearch"/> <!-- "database" driver doesn't support SQLite -->
         <env name="SESSION_DRIVER" value="array"/>

resources/assets/js/components/screens/SettingsScreen.vue

@@ -2,7 +2,11 @@
   <section id="settingsWrapper">
     <ScreenHeader>Settings</ScreenHeader>
 
-    <form class="main-scroll-wrap" @submit.prevent="confirmThenSave">
+    <p v-if="storageDriver !== 'local'" class="cloud-warn text-secondary">
+      Since you’re not using a cloud storage, there’s no need to set a media path.
+    </p>
+
+    <form v-else class="main-scroll-wrap" @submit.prevent="confirmThenSave">
       <div class="form-row">
         <label for="inputSettingsPath">Media Path</label>
 
@@ -99,5 +103,9 @@ const confirmThenSave = async () => {
       width: 100%;
     }
   }
+
+  .cloud-warn {
+    padding: 1.8rem;
+  }
 }
 </style>

tests/Feature/SettingTest.php

@@ -37,4 +37,14 @@ class SettingTest extends TestCase
         $this->putAs('/api/settings', ['media_path' => __DIR__])
             ->assertForbidden();
     }
+
+    public function testMediaPathCannotBeSetForCloudStorage(): void
+    {
+        config(['koel.storage_driver' => 's3']);
+
+        $this->putAs('/api/settings', ['media_path' => __DIR__], create_admin())
+            ->assertUnprocessable();
+
+        config(['koel.storage_driver' => 'local']);
+    }
 }