[4.x] Make RemoveStorageSymlinksAction able to delete broken symlinks (#1323)

* Add regression test for removing invalid symlinks

* Move commented RemoveStorageSymlinks job to the DeletingTenant pipeline (better default - the symlinks will be removed *before* deleting tenant storage)

* Remove symlink validity check from symlinkExists() (only check for the symlink's existence)

* Delete complete todo0

* Make the symlink assertions more explicit

* update test name

---------

Co-authored-by: Samuel Štancl <samuel@archte.ch>

assets/TenancyServiceProvider.stub.php

@@ -53,6 +53,7 @@ class TenancyServiceProvider extends ServiceProvider
             Events\DeletingTenant::class => [
                 JobPipeline::make([
                     Jobs\DeleteDomains::class,
+                    // Jobs\RemoveStorageSymlinks::class,
                 ])->send(function (Events\DeletingTenant $event) {
                     return $event->tenant;
                 })->shouldBeQueued(false),
@@ -62,7 +63,6 @@ class TenancyServiceProvider extends ServiceProvider
             Events\TenantDeleted::class => [
                 JobPipeline::make([
                     Jobs\DeleteDatabase::class,
-                    // Jobs\RemoveStorageSymlinks::class,
                 ])->send(function (Events\TenantDeleted $event) {
                     return $event->tenant;
                 })->shouldBeQueued(false), // `false` by default, but you probably want to make this `true` for production.

src/Concerns/DealsWithTenantSymlinks.php

@@ -56,6 +56,6 @@ trait DealsWithTenantSymlinks
     /** Determine if the provided path is an existing symlink. */
     protected function symlinkExists(string $link): bool
     {
-        return file_exists($link) && is_link($link);
+        return is_link($link);
     }
 }

tests/ActionTest.php

@@ -11,6 +11,7 @@ use Stancl\Tenancy\Listeners\RevertToCentralContext;
 use Stancl\Tenancy\Actions\CreateStorageSymlinksAction;
 use Stancl\Tenancy\Actions\RemoveStorageSymlinksAction;
 use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper;
+use Illuminate\Support\Facades\File;
 
 beforeEach(function () {
     Event::listen(TenancyInitialized::class, BootstrapTenancy::class);
@@ -35,11 +36,15 @@ test('create storage symlinks action works', function() {
 
     tenancy()->initialize($tenant);
 
-    $this->assertDirectoryDoesNotExist($publicPath = public_path("public-$tenantKey"));
+    // The symlink doesn't exist
+    expect(is_link($publicPath = public_path("public-$tenantKey")))->toBeFalse();
+    expect(file_exists($publicPath))->toBeFalse();
 
     (new CreateStorageSymlinksAction)($tenant);
 
-    $this->assertDirectoryExists($publicPath);
+    // The symlink exists and is valid
+    expect(is_link($publicPath = public_path("public-$tenantKey")))->toBeTrue();
+    expect(file_exists($publicPath))->toBeTrue();
     $this->assertEquals(storage_path("app/public/"), readlink($publicPath));
 });
 
@@ -61,9 +66,48 @@ test('remove storage symlinks action works', function() {
 
     (new CreateStorageSymlinksAction)($tenant);
 
-    $this->assertDirectoryExists($publicPath = public_path("public-$tenantKey"));
+    // The symlink exists and is valid
+    expect(is_link($publicPath = public_path("public-$tenantKey")))->toBeTrue();
+    expect(file_exists($publicPath))->toBeTrue();
 
     (new RemoveStorageSymlinksAction)($tenant);
 
-    $this->assertDirectoryDoesNotExist($publicPath);
+    // The symlink doesn't exist
+    expect(is_link($publicPath))->toBeFalse();
+    expect(file_exists($publicPath))->toBeFalse();
+});
+
+test('removing tenant symlinks works even if the symlinks are invalid', function() {
+    config([
+        'tenancy.bootstrappers' => [
+            FilesystemTenancyBootstrapper::class,
+        ],
+        'tenancy.filesystem.suffix_base' => 'tenant-',
+        'tenancy.filesystem.root_override.public' => '%storage_path%/app/public/',
+        'tenancy.filesystem.url_override.public' => 'public-%tenant%'
+    ]);
+
+    /** @var Tenant $tenant */
+    $tenant = Tenant::create();
+    $tenantKey = $tenant->getTenantKey();
+
+    tenancy()->initialize($tenant);
+
+    (new CreateStorageSymlinksAction)($tenant);
+
+    // The symlink exists and is valid
+    expect(is_link($publicPath = public_path("public-$tenantKey")))->toBeTrue();
+    expect(file_exists($publicPath))->toBeTrue();
+
+    // Make the symlink invalid by deleting the tenant storage directory
+    $storagePath = storage_path();
+    File::deleteDirectory($storagePath);
+
+    // The symlink still exists, but isn't valid
+    expect(is_link($publicPath))->toBeTrue();
+    expect(file_exists($publicPath))->toBeFalse();
+
+    (new RemoveStorageSymlinksAction)($tenant);
+
+    expect(is_link($publicPath))->toBeFalse();
 });