mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-17 00:55:07 +00:00
ignacionelson
d7e639b7af
Follow-up to #1703, which made `exists:folders,id` mean what its ten readers already assumed. Two things it named and deliberately left. **The chunked upload is two requests.** store()'s rule only ever sees the first: POST /uploads records the resolved folder on the UploadSession and complete() reads it back from the session rather than from the caller, so deleting the folder while the bytes are in flight still files the assembled file into it -- the same orphan state #1703 removes, reached by a door a validation rule cannot watch. complete() now re-resolves through Folder::query() and files at the root when the folder has gone. Root rather than a refusal, because the two moments cost different things. At store() nothing has been sent, so refusing is free and honest, which is the call #1703 made. Here the bytes are already uploaded, and discarding somebody's finished transfer over a folder that vanished underneath them is the harsher of the two surprises. The file lands somewhere they can see it and move it. **The refusal now explains itself.** "The selected folder id is invalid" says nothing when the answer is that the folder has been deleted -- and that is the usual way to meet this rule, since a live id picked from a list is how anybody gets here. It matters most on the chunked path, the one place #1703 makes a previously-working request fail. A small ValidationRule object carries the message, which keeps the single definition Rules::folderId() exists for: a messages() array would have to be repeated at all ten call sites, and rules meaning different things in ten places is what went wrong in the first place. One note for whoever writes the next test here. Upload parts live in storage_path('app/uploads-tmp/{session_id}'), which is a real shared directory rather than a faked disk, and each parallel worker's database restarts session ids at 1 -- so two files writing parts on two workers collide, and ChunkedUploadsTest's afterEach deletes the whole tree for everybody. Six test files write parts today. These two cases live in ChunkedUploadsTest rather than beside the rest of their subject so this change does not add a seventh racer; the underlying isolation problem predates it and is worth its own fix.
55 lines
1.9 KiB
PHP
55 lines
1.9 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
namespace App\Modules\Files\Folders;
|
|
|
|
use App\Modules\Files\Models\Folder;
|
|
use Closure;
|
|
use Illuminate\Contracts\Validation\ValidationRule;
|
|
|
|
/**
|
|
* An id naming a folder that is actually there.
|
|
*
|
|
* A rule object rather than `Rule::exists(...)->whereNull('deleted_at')`
|
|
* for one reason: the message. The generic form says "The selected folder
|
|
* id is invalid", which tells somebody nothing when the real answer is
|
|
* that the folder they picked has since been deleted — and that is the
|
|
* usual way to meet this rule, since a live id they chose from a list is
|
|
* how they got here. It matters most on the chunked upload path, which is
|
|
* the one place a request that used to succeed now fails.
|
|
*
|
|
* Carrying the message on the rule keeps the single definition
|
|
* Rules::folderId() exists for: a `messages()` array would have to be
|
|
* repeated at every call site, which is how the plain `exists:folders,id`
|
|
* it replaced came to mean two different things in ten places.
|
|
*/
|
|
class FolderExistsRule implements ValidationRule
|
|
{
|
|
/**
|
|
* @param Closure(string, string|null=): \Illuminate\Translation\PotentiallyTranslatedString $fail
|
|
*/
|
|
public function validate(string $attribute, mixed $value, Closure $fail): void
|
|
{
|
|
if ($value === null || $value === '') {
|
|
return;
|
|
}
|
|
|
|
if (! is_numeric($value)) {
|
|
// Reached only when a caller drops `integer`; the message
|
|
// still has to make sense to whoever sees it.
|
|
$fail(__('That folder could not be found.'));
|
|
|
|
return;
|
|
}
|
|
|
|
// Folder::query() honours the soft delete, which is the whole
|
|
// point — the table-level `exists` rule this replaces does not.
|
|
if (Folder::query()->whereKey((int) $value)->exists()) {
|
|
return;
|
|
}
|
|
|
|
$fail(__('That folder no longer exists. Pick another one and try again.'));
|
|
}
|
|
}
|