kalugin66/InvoiceShelf
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor FileDisk system with per-disk unique names and disk assignments UI

Browse Source 
Major changes to the file disk subsystem:

- Each FileDisk now gets a unique Laravel disk name (disk_{id}) instead
  of temp_{driver}, fixing the bug where multiple local disks with
  different roots overwrote each other's config.

- Move disk registration logic from FileDisk model to FileDiskService
  (registerDisk, getDiskName). Model keeps only getDecodedCredentials
  and a deprecated setConfig() wrapper.

- Add Disk Assignments admin UI (File Disk tab) with three purpose
  dropdowns: Media Storage, PDF Storage, Backup Storage. Stored as
  settings (media_disk_id, pdf_disk_id, backup_disk_id).

- Backup tab now uses the assigned backup disk instead of a per-backup
  dropdown. BackupsController refactored to use BackupService which
  centralizes disk resolution. Removed stale 4-second cache.

- Add local_public disk to config/filesystems.php so system disks
  are properly defined.

- Local disk roots stored relative to storage/app/ with hint text
  in the admin modal explaining the convention.

- Fix BaseModal watchEffect -> watch to prevent infinite request
  loops on the File Disk page.

- Fix string/number comparison for disk purpose IDs from settings.

- Add safeguards: prevent deleting disks with files, warn on
  purpose change, prevent deleting system disks.
This commit is contained in:
Darko Gjorgjijoski
2026-04-07 02:04:57 +02:00
parent ea1fc9b799
commit 20085cab5d
20 changed files with 492 additions and 258 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
app/Console/Commands/MigrateMediaToPrivateDisk.php
View File

@@ -4,6 +4,7 @@ namespace App\Console\Commands;
use App\Models\FileDisk;
use App\Models\Setting;
use App\Services\FileDiskService;
use Illuminate\Console\Command;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\File;
@@ -24,21 +25,8 @@ class MigrateMediaToPrivateDisk extends Command
return self::FAILURE;
}
$prefix = env('DYNAMIC_DISK_PREFIX', 'temp_');
$targetDiskName = $prefix.$targetDisk->driver;
// Register disk config without mutating filesystems.default
$credentials = collect(json_decode($targetDisk->credentials));
$baseConfig = config('filesystems.disks.'.$targetDisk->driver, []);
foreach ($baseConfig as $key => $value) {
if ($credentials->has($key)) {
$baseConfig[$key] = $credentials[$key];
}
}
config(['filesystems.disks.'.$targetDiskName => $baseConfig]);
$targetRoot = $baseConfig['root'] ?? null;
$targetDiskName = app(FileDiskService::class)->registerDisk($targetDisk);
$targetRoot = config('filesystems.disks.'.$targetDiskName.'.root');
if (! $targetRoot) {
$this->error('Could not resolve target disk root path.');

54
app/Http/Controllers/Admin/BackupsController.php
View File

@@ -4,61 +4,47 @@ namespace App\Http\Controllers\Admin;
use App\Http\Controllers\Controller;
use App\Jobs\CreateBackupJob;
use App\Models\FileDisk;
use App\Rules\Backup\PathToZip;
use App\Services\Backup\BackupService;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\Request;
use Illuminate\Http\Response;
use Illuminate\Support\Facades\Cache;
use Spatie\Backup\BackupDestination\Backup;
use Spatie\Backup\BackupDestination\BackupDestination;
use Spatie\Backup\Helpers\Format;
use Symfony\Component\HttpFoundation\StreamedResponse;
class BackupsController extends Controller
{
public function __construct(
private readonly BackupService $backupService,
) {}
public function index(Request $request): JsonResponse
{
$this->authorize('manage backups');
$configuredBackupDisks = config('backup.backup.destination.disks');
try {
if ($request->file_disk_id) {
$fileDisk = FileDisk::find($request->file_disk_id);
if ($fileDisk) {
$fileDisk->setConfig();
$prefix = env('DYNAMIC_DISK_PREFIX', 'temp_');
config(['backup.backup.destination.disks' => [$prefix.$fileDisk->driver]]);
$configuredBackupDisks = config('backup.backup.destination.disks');
}
}
$destination = $this->backupService->getDestination($request->file_disk_id);
$backupDestination = BackupDestination::create(config('filesystems.default'), config('backup.backup.name'));
$backups = Cache::remember("backups-{$request->file_disk_id}", now()->addSeconds(4), function () use ($backupDestination) {
return $backupDestination
->backups()
->map(function (Backup $backup) {
return [
'path' => $backup->path(),
'created_at' => $backup->date()->format('Y-m-d H:i:s'),
'size' => Format::humanReadableSize($backup->sizeInBytes()),
];
})
->toArray();
});
$backups = $destination
->backups()
->map(function (Backup $backup) {
return [
'path' => $backup->path(),
'created_at' => $backup->date()->format('Y-m-d H:i:s'),
'size' => Format::humanReadableSize($backup->sizeInBytes()),
];
})
->toArray();
return response()->json([
'backups' => $backups,
'disks' => $configuredBackupDisks,
]);
} catch (\Exception $e) {
return response()->json([
'backups' => [],
'error' => 'invalid_disk_credentials',
'error_message' => $e->getMessage(),
'disks' => $configuredBackupDisks,
]);
}
}
@@ -82,9 +68,9 @@ class BackupsController extends Controller
'path' => ['required', new PathToZip],
]);
$backupDestination = BackupDestination::create(config('filesystems.default'), config('backup.backup.name'));
$destination = $this->backupService->getDestination($request->file_disk_id);
$backupDestination
$destination
->backups()
->first(function (Backup $backup) use ($validated) {
return $backup->path() === $validated['path'];
@@ -102,9 +88,9 @@ class BackupsController extends Controller
'path' => ['required', new PathToZip],
]);
$backupDestination = BackupDestination::create(config('filesystems.default'), config('backup.backup.name'));
$destination = $this->backupService->getDestination($request->file_disk_id);
$backup = $backupDestination->backups()->first(function (Backup $backup) use ($validated) {
$backup = $destination->backups()->first(function (Backup $backup) use ($validated) {
return $backup->path() === $validated['path'];
});

48
app/Http/Controllers/Admin/Settings/DiskController.php
View File

@@ -6,6 +6,7 @@ use App\Http\Controllers\Controller;
use App\Http\Requests\DiskEnvironmentRequest;
use App\Http\Resources\FileDiskResource;
use App\Models\FileDisk;
use App\Models\Setting;
use App\Services\FileDiskService;
use Illuminate\Auth\Access\AuthorizationException;
use Illuminate\Http\JsonResponse;
@@ -91,8 +92,10 @@ class DiskController extends Controller
$diskData = [];
switch ($disk) {
case 'local':
// Path is relative to storage/app/.
// e.g., "backups" resolves to storage/app/backups/ at runtime.
$diskData = [
'root' => config('filesystems.disks.local.root'),
'root' => '',
];
break;
@@ -216,11 +219,50 @@ class DiskController extends Controller
],
];
$default = config('filesystems.default');
$defaultDisk = FileDisk::where('set_as_default', true)->first();
return response()->json([
'drivers' => $drivers,
'default' => $default,
'default' => $defaultDisk?->driver ?? 'local',
]);
}
public function getDiskPurposes(): JsonResponse
{
$this->authorize('manage file disk');
$defaultDisk = FileDisk::where('set_as_default', true)->first();
$defaultId = $defaultDisk?->id;
return response()->json([
'media_disk_id' => Setting::getSetting('media_disk_id') ?? $defaultId,
'pdf_disk_id' => Setting::getSetting('pdf_disk_id') ?? $defaultId,
'backup_disk_id' => Setting::getSetting('backup_disk_id') ?? $defaultId,
]);
}
public function updateDiskPurposes(Request $request): JsonResponse
{
$this->authorize('manage file disk');
$request->validate([
'media_disk_id' => 'nullable|exists:file_disks,id',
'pdf_disk_id' => 'nullable|exists:file_disks,id',
'backup_disk_id' => 'nullable|exists:file_disks,id',
]);
if ($request->has('media_disk_id')) {
Setting::setSetting('media_disk_id', $request->media_disk_id);
}
if ($request->has('pdf_disk_id')) {
Setting::setSetting('pdf_disk_id', $request->pdf_disk_id);
}
if ($request->has('backup_disk_id')) {
Setting::setSetting('backup_disk_id', $request->backup_disk_id);
}
return response()->json(['success' => true]);
}
}

32
app/Models/FileDisk.php
View File

@@ -2,6 +2,7 @@
namespace App\Models;
use App\Services\FileDiskService;
use Carbon\Carbon;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model;
@@ -31,6 +32,21 @@ class FileDisk extends Model
$this->attributes['credentials'] = json_encode($value);
}
/**
* Decode credentials, handling double-encoded JSON from legacy data.
*/
public function getDecodedCredentials(): Collection
{
$decoded = json_decode($this->credentials, true);
// Handle double-encoded JSON (string inside string)
if (is_string($decoded)) {
$decoded = json_decode($decoded, true);
}
return collect($decoded ?? []);
}
public function scopeWhereOrder($query, $orderByField, $orderBy)
{
$query->orderBy($orderByField, $orderBy);
@@ -83,14 +99,14 @@ class FileDisk extends Model
/**
* Apply this disk's credentials to the filesystem configuration at runtime.
*
* @deprecated Use FileDiskService::registerDisk() instead — setConfig() mutates filesystems.default.
*/
public function setConfig(): void
{
$driver = $this->driver;
$credentials = collect(json_decode($this['credentials']));
self::setFilesystem($credentials, $driver);
$service = app(FileDiskService::class);
$diskName = $service->registerDisk($this);
config(['filesystems.default' => $diskName]);
}
/**
@@ -103,6 +119,8 @@ class FileDisk extends Model
/**
* Register a dynamic filesystem disk in the runtime configuration using the given credentials.
*
* @deprecated Use FileDisk::find($id)->registerDisk() instead.
*/
public static function setFilesystem(Collection $credentials, string $driver): void
{
@@ -118,6 +136,10 @@ class FileDisk extends Model
}
}
if ($driver === 'local' && isset($disks['root']) && ! str_starts_with($disks['root'], '/')) {
$disks['root'] = storage_path('app/'.$disks['root']);
}
config(['filesystems.disks.'.$prefix.$driver => $disks]);
}

27
app/Providers/AppConfigProvider.php
View File

@@ -4,6 +4,7 @@ namespace App\Providers;
use App\Models\FileDisk;
use App\Models\Setting;
use App\Services\FileDiskService;
use App\Services\Setup\InstallUtils;
use Illuminate\Support\Facades\Config;
use Illuminate\Support\ServiceProvider;
@@ -163,28 +164,16 @@ class AppConfigProvider extends ServiceProvider
protected function configureFileSystemFromDatabase(): void
{
try {
// Register the default file disk config without changing filesystems.default.
// Calling setConfig() mutates the global default which causes side effects
// on pages that make multiple API requests (e.g., File Disk admin page).
$fileDisk = FileDisk::whereSetAsDefault(true)->first();
if ($fileDisk) {
$prefix = env('DYNAMIC_DISK_PREFIX', 'temp_');
$diskName = $prefix.$fileDisk->driver;
$credentials = collect(json_decode($fileDisk->credentials));
$baseConfig = config('filesystems.disks.'.$fileDisk->driver, []);
foreach ($baseConfig as $key => $value) {
if ($credentials->has($key)) {
$baseConfig[$key] = $credentials[$key];
}
}
config(['filesystems.disks.'.$diskName => $baseConfig]);
// Point Spatie Media Library at the same disk
config(['media-library.disk_name' => $diskName]);
if (! $fileDisk) {
return;
}
$diskName = app(FileDiskService::class)->registerDisk($fileDisk);
// Point Spatie Media Library at the resolved disk
config(['media-library.disk_name' => $diskName]);
} catch (\Exception $e) {
// Silently fail if database is not available (during installation, migrations, etc.)
}

11
app/Services/Backup/BackupConfigurationFactory.php
View File

@@ -3,6 +3,7 @@
namespace App\Services\Backup;
use App\Models\FileDisk;
use App\Services\FileDiskService;
use Exception;
use Spatie\Backup\Config\Config;
@@ -16,14 +17,10 @@ class BackupConfigurationFactory
$fileDisk = FileDisk::find($data['file_disk_id']);
$fileDisk->setConfig();
$diskName = app(FileDiskService::class)->registerDisk($fileDisk);
$prefix = env('DYNAMIC_DISK_PREFIX', 'temp_');
config(['backup.backup.destination.disks' => [$diskName]]);
config(['backup.backup.destination.disks' => [$prefix.$fileDisk->driver]]);
$config = Config::fromArray(config('backup'));
return $config;
return Config::fromArray(config('backup'));
}
}

52
app/Services/Backup/BackupService.php Normal file
View File

@@ -0,0 +1,52 @@
<?php
namespace App\Services\Backup;
use App\Models\FileDisk;
use App\Models\Setting;
use App\Services\FileDiskService;
use Spatie\Backup\BackupDestination\BackupDestination;
class BackupService
{
public function __construct(
private readonly FileDiskService $fileDiskService,
) {}
/**
* Resolve the backup FileDisk from the given ID, settings, or default.
*/
public function resolveBackupDisk(?int $fileDiskId = null): ?FileDisk
{
if ($fileDiskId) {
$disk = FileDisk::find($fileDiskId);
if ($disk) {
return $disk;
}
}
$backupDiskId = Setting::getSetting('backup_disk_id');
if ($backupDiskId) {
$disk = FileDisk::find($backupDiskId);
if ($disk) {
return $disk;
}
}
return FileDisk::where('set_as_default', true)->first();
}
/**
* Create a BackupDestination pointing at the resolved disk.
*/
public function getDestination(?int $fileDiskId = null): BackupDestination
{
$disk = $this->resolveBackupDisk($fileDiskId);
$diskName = $disk
? $this->fileDiskService->registerDisk($disk)
: config('filesystems.default');
return BackupDestination::create($diskName, config('backup.backup.name'));
}
}

97
app/Services/FileDiskService.php
View File

@@ -13,8 +13,10 @@ class FileDiskService
$this->clearDefaults();
}
$credentials = $this->normalizeCredentials($request->credentials, $request->driver);
return FileDisk::create([
'credentials' => $request->credentials,
'credentials' => $credentials,
'name' => $request->name,
'driver' => $request->driver,
'set_as_default' => $request->set_as_default,
@@ -24,8 +26,10 @@ class FileDiskService
public function update(FileDisk $disk, Request $request): FileDisk
{
$credentials = $this->normalizeCredentials($request->credentials, $request->driver);
$data = [
'credentials' => $request->credentials,
'credentials' => $credentials,
'name' => $request->name,
'driver' => $request->driver,
];
@@ -53,21 +57,77 @@ class FileDiskService
return $disk;
}
/**
* Get the unique Laravel filesystem disk name for a FileDisk.
*/
public function getDiskName(FileDisk $disk): string
{
if ($disk->isSystem()) {
return $disk->name === 'local_public' ? 'local_public' : 'local';
}
return 'disk_'.$disk->id;
}
/**
* Register a FileDisk in the runtime filesystem configuration.
* Returns the Laravel disk name. Does NOT change filesystems.default.
*/
public function registerDisk(FileDisk $disk): string
{
$diskName = $this->getDiskName($disk);
// System disks are already in config/filesystems.php
if ($disk->isSystem()) {
return $diskName;
}
$credentials = $disk->getDecodedCredentials();
$baseConfig = config('filesystems.disks.'.$disk->driver, []);
foreach ($baseConfig as $key => $value) {
if ($credentials->has($key)) {
$baseConfig[$key] = $credentials[$key];
}
}
// Resolve relative local roots to storage/app/{path}
if ($disk->driver === 'local' && isset($baseConfig['root']) && ! str_starts_with($baseConfig['root'], '/')) {
$baseConfig['root'] = storage_path('app/'.$baseConfig['root']);
}
config(['filesystems.disks.'.$diskName => $baseConfig]);
return $diskName;
}
public function validateCredentials(array $credentials, string $driver): bool
{
FileDisk::setFilesystem(collect($credentials), $driver);
// Create a temporary disk config for validation
$baseConfig = config('filesystems.disks.'.$driver, []);
$prefix = env('DYNAMIC_DISK_PREFIX', 'temp_');
foreach ($baseConfig as $key => $value) {
if (isset($credentials[$key])) {
$baseConfig[$key] = $credentials[$key];
}
}
if ($driver === 'local' && isset($baseConfig['root']) && ! str_starts_with($baseConfig['root'], '/')) {
$baseConfig['root'] = storage_path('app/'.$baseConfig['root']);
}
$tempDiskName = 'validation_temp';
config(['filesystems.disks.'.$tempDiskName => $baseConfig]);
try {
$root = '';
if ($driver == 'dropbox') {
$root = $credentials['root'].'/';
}
\Storage::disk($prefix.$driver)->put($root.'invoiceshelf_temp.text', 'Check Credentials');
\Storage::disk($tempDiskName)->put($root.'invoiceshelf_temp.text', 'Check Credentials');
if (\Storage::disk($prefix.$driver)->exists($root.'invoiceshelf_temp.text')) {
\Storage::disk($prefix.$driver)->delete($root.'invoiceshelf_temp.text');
if (\Storage::disk($tempDiskName)->exists($root.'invoiceshelf_temp.text')) {
\Storage::disk($tempDiskName)->delete($root.'invoiceshelf_temp.text');
return true;
}
@@ -78,6 +138,29 @@ class FileDiskService
return false;
}
/**
* For local disks, strip any absolute prefix and store the root
* as a path relative to storage/app. At runtime the path is
* resolved to an absolute path via storage_path().
*/
private function normalizeCredentials(array $credentials, string $driver): array
{
if ($driver === 'local' && isset($credentials['root'])) {
$root = $credentials['root'];
$storageApp = storage_path('app').'/';
if (str_starts_with($root, $storageApp)) {
$root = substr($root, strlen($storageApp));
}
$root = ltrim($root, '/');
$credentials['root'] = $root;
}
return $credentials;
}
private function clearDefaults(): void
{
FileDisk::query()->update(['set_as_default' => false]);