Files
projectsend/tests/Feature/Files/HasUniqueSlugTest.php
T
ignacionelson 98597d462d Give a deleted folder's name back
Delete a folder called Test and you could never have a folder called Test
again. The deletion worked, the folder left the screen, and the name went
with it — permanently, with an error that named a collision against a row
the interface will not show you and offered nothing to do about it.

Files and groups had it too. All three carry a unique index on slug and
all three soft-delete, so the trashed row sat in the index holding a name
nothing could reach. A public one failed outright at the validator, which
checks the table and therefore sees rows the screen does not. A private
one failed more quietly: the derived slug stepped around the trashed row
into report-2, then report-3, once per deletion, climbing forever.

The reservation was deliberate — a trashed row's slug was kept so that
restoring it could not land on somebody else's URL. But nothing in this
application restores anything. There is no restore() call, no route, no
screen; File's own comment says as much. Soft deletes are here so rows can
outlive their delete for foreign keys, the activity log and the erasure
grace period, never so they can come back. The slug was being held for a
page that could not return, and route binding already 404s the trashed row
in the meantime.

So deleting now hands the slug back, and the database is what makes that a
rewrite rather than a gentler lookup: teaching the collision checks to skip
trashed rows would leave two rows holding "report", which the unique index
rejects whatever the application thinks. The slug moves to report__deleted-42
instead. Underscores are the whole trick — Str::slug() turns them into
hyphens and Rules::slug() refuses them outright, so no derived slug and no
hand-typed one can ever land on a vacated one. That is a guarantee about
the character class rather than a hope about collisions.

The format lives in VacatedSlug rather than on the trait because the
migration needs it too and a trait constant cannot be reached through the
trait's own name — the first version of this was a fatal error waiting for
whoever ran migrations. The migration matters as much as the hook: without
it the fix only helps installations that have never deleted anything, and
every name already buried stays buried.

The collision checks still count trashed rows. It costs nothing and keeps
them honest about what the index will accept if a row is ever soft-deleted
by something that bypasses model events.

previous_file_id had this same bug and was fixed this same way, in
File::detachOnDelete — a trashed row holding its predecessor's unique slot
so the chain could never be re-linked. This is that fix, for the other four
unique indexes' worth of the same mistake. users.email is the one left, and
is deliberately not in here: an email address is a login identity rather
than a URL handle, and freeing it silently is the wrong answer.

Fixes #1645

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-18 12:11:02 -03:00

154 lines
6.3 KiB
PHP

<?php
declare(strict_types=1);
use App\Models\User;
use App\Modules\Files\Models\File;
use App\Modules\Files\Models\Folder;
use App\Modules\Groups\Models\Group;
use App\Support\Rules;
use App\Support\VacatedSlug;
use Illuminate\Support\Facades\Storage;
use Illuminate\Support\Facades\Validator;
use Illuminate\Support\Str;
beforeEach(function () {
Storage::fake('files');
User::factory()->create();
});
// File, Folder and Group each carried their own byte-identical copy of this
// logic before it moved to the shared trait; the cases below are the ones
// that copy actually handled, asserted against all three so a future model
// picking up the trait inherits the same guarantees.
test('a slug is derived from the name when none is given', function () {
expect(File::factory()->create(['name' => 'Quarterly Report'])->slug)->toBe('quarterly-report')
->and(Folder::query()->create(['name' => 'Client Docs', 'path' => '/'])->slug)->toBe('client-docs')
->and(Group::query()->create(['name' => 'Board Members'])->slug)->toBe('board-members');
});
test('an explicitly given slug is left alone', function () {
expect(File::factory()->create(['name' => 'Report', 'slug' => 'custom-one'])->slug)->toBe('custom-one');
});
test('colliding names get a numeric suffix that keeps counting up', function () {
expect(File::factory()->create(['name' => 'Report'])->slug)->toBe('report')
->and(File::factory()->create(['name' => 'Report'])->slug)->toBe('report-2')
->and(File::factory()->create(['name' => 'Report'])->slug)->toBe('report-3');
});
// Deleting used to hold the slug forever, which burned the name: nothing can
// restore a trashed row, so it was reserved for a page that could never come
// back. See issue #1645.
test('deleting a row hands its slug back', function () {
$first = File::factory()->create(['name' => 'Report']);
$first->delete();
expect($first->trashed())->toBeTrue()
->and(File::factory()->create(['name' => 'Report'])->slug)->toBe('report');
});
test('the vacated slug is parked where nothing can collide with it', function () {
$file = File::factory()->create(['name' => 'Report']);
$file->delete();
$parked = File::withTrashed()->whereKey($file->id)->value('slug');
expect($parked)->toBe('report'.VacatedSlug::MARKER.$file->id)
// Neither route into a slug can produce that string, which is what
// makes parking there safe rather than merely unlikely.
->and(Str::slug($parked))->not->toContain('_')
->and(Validator::make(
['slug' => $parked, 'public' => true],
['slug' => Rules::slug('files')],
)->fails())->toBeTrue();
});
test('the name is reusable however many times it is deleted', function () {
foreach (range(1, 3) as $ignored) {
$file = File::factory()->create(['name' => 'Report']);
expect($file->slug)->toBe('report');
$file->delete();
}
// Three deleted rows, each parked under its own id rather than piling up
// as report-2, report-3, report-4 the way the suffix used to.
expect(File::withTrashed()->where('slug', 'like', 'report'.VacatedSlug::MARKER.'%')->count())->toBe(3);
});
test('all three models hand the slug back', function () {
$file = File::factory()->create(['name' => 'Shared']);
$folder = Folder::query()->create(['name' => 'Shared', 'path' => '/']);
$group = Group::query()->create(['name' => 'Shared']);
$file->delete();
$folder->delete();
$group->delete();
expect(File::factory()->create(['name' => 'Shared'])->slug)->toBe('shared')
->and(Folder::query()->create(['name' => 'Shared', 'path' => '/'])->slug)->toBe('shared')
->and(Group::query()->create(['name' => 'Shared'])->slug)->toBe('shared');
});
// The reported bug: a public folder's slug is user-supplied and validated
// against the table, so a trashed row holding it made the name unusable with
// an error naming a row the interface will not show.
test('a deleted public row does not block its slug at the validator', function () {
$folder = Folder::query()->create(['name' => 'Quarterly', 'path' => '/', 'public' => true, 'slug' => 'quarterly']);
$folder->delete();
expect(Validator::make(
['slug' => 'quarterly', 'public' => true],
['slug' => Rules::slug('folders')],
)->fails())->toBeFalse();
});
test('force-deleting leaves nothing behind to vacate', function () {
$file = File::factory()->create(['name' => 'Report']);
$file->forceDelete();
expect(File::withTrashed()->whereKey($file->id)->exists())->toBeFalse()
->and(File::factory()->create(['name' => 'Report'])->slug)->toBe('report');
});
test('vacating does not look like somebody edited the row', function () {
$file = File::factory()->create(['name' => 'Report']);
$this->travel(5)->minutes();
$file->delete();
// The soft delete moves updated_at itself; what matters is that vacating
// the slug afterwards is not a second write on top of it, so the two
// stamps the delete wrote still agree.
$row = File::withTrashed()->whereKey($file->id)->sole();
expect($row->slug)->toBe('report'.VacatedSlug::MARKER.$file->id)
->and($row->updated_at)->toEqual($row->deleted_at);
});
test('a name that slugs to nothing falls back to a per-model default', function () {
expect(File::factory()->create(['name' => '!!!'])->slug)->toBe('file')
->and(Folder::query()->create(['name' => '!!!', 'path' => '/'])->slug)->toBe('folder')
->and(Group::query()->create(['name' => '!!!'])->slug)->toBe('group');
});
test('ignoreId lets a row keep the slug it already holds', function () {
$file = File::factory()->create(['name' => 'Report']);
expect(File::uniqueSlugFrom('Report', $file->id))->toBe('report')
// Without the exemption the same name has to move out of its own way.
->and(File::uniqueSlugFrom('Report'))->toBe('report-2');
});
// The trait boots via bootHasUniqueSlug() precisely so it does not replace
// File::booted(), which is where the disk-cleanup hook lives.
test('the file disk-cleanup hook still fires now that the trait owns slug creation', function () {
$file = File::factory()->create(['name' => 'Report', 'path' => 'docs/report.pdf']);
Storage::disk('files')->put('docs/report.pdf', 'bytes');
$file->delete();
expect(Storage::disk('files')->exists('docs/report.pdf'))->toBeFalse();
});