From 8cd15db1fcafcc97bcb968867785995d58c18169 Mon Sep 17 00:00:00 2001
From: lukinovec <lukinovec@gmail.com>
Date: Tue, 18 Mar 2025 21:27:27 +0100
Subject: [PATCH] [4.x] Make RemoveStorageSymlinksAction able to delete broken
 symlinks (#1323)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* 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   |  2 +-
 src/Concerns/DealsWithTenantSymlinks.php |  2 +-
 tests/ActionTest.php                     | 52 ++++++++++++++++++++++--
 3 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/assets/TenancyServiceProvider.stub.php b/assets/TenancyServiceProvider.stub.php
index 2f2f11f2..d49d96d0 100644
--- a/assets/TenancyServiceProvider.stub.php
+++ b/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.
diff --git a/src/Concerns/DealsWithTenantSymlinks.php b/src/Concerns/DealsWithTenantSymlinks.php
index 37984dc8..114eadb5 100644
--- a/src/Concerns/DealsWithTenantSymlinks.php
+++ b/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);
     }
 }
diff --git a/tests/ActionTest.php b/tests/ActionTest.php
index 1adcb32d..63b6b377 100644
--- a/tests/ActionTest.php
+++ b/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();
 });