Repository navigation
Add on-demand storage fakes and fix root signed URLs #619
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -137,6 +137,10 @@ public function disk(UnitEnum|string|null $name = null): Filesystem | |
| */ | ||
| public function build(array|string $config, ?string $name = null): Filesystem | ||
| { | ||
| if ($name === null && isset($this->disks[self::ON_DEMAND_DISK_NAME])) { | ||
| return $this->disks[self::ON_DEMAND_DISK_NAME]; | ||
| } | ||
|
|
||
| $config = is_array($config) ? $config : [ | ||
| 'driver' => 'local', | ||
| 'root' => $config, | ||
|
|
@@ -170,8 +174,8 @@ protected function resolve(string $name, ?array $config = null): Filesystem | |
| /** | ||
| * Resolve the given disk while preserving its logical construction name. | ||
| * | ||
| * The configured disk name "ondemand" is valid, so build() enters this | ||
| * method directly to carry anonymous construction as a separate value. | ||
| * Anonymous builds carry a null logical name through custom creators | ||
| * and pool identity, independently of their internal construction name. | ||
| */ | ||
| private function resolveWithLogicalName(string $name, ?array $config, ?string $logicalName): Filesystem | ||
| { | ||
|
|
@@ -834,7 +838,13 @@ public function set(string $name, mixed $disk): static | |
| */ | ||
| protected function getConfig(string $name): array | ||
| { | ||
| return $this->app->make('config')->get("filesystems.disks.{$name}") ?: []; | ||
| $config = $this->app->make('config')->get("filesystems.disks.{$name}") ?: []; | ||
|
|
||
| if ($name === self::ON_DEMAND_DISK_NAME && $config !== []) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: This makes serving URLs from Prompt for AI agents
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Configured ondemand disks are intentionally unsupported; the porting guide requires a different name. Local serving already requires a valid matching configured disk, and an explicit build does not register routes. Use a non-reserved name for both when serving files. Preserving a configured ondemand serving disk would undo the intended reservation; an additional special-case guard for this unsupported configuration is unnecessary. |
||
| throw new InvalidArgumentException('The disk name [ondemand] is reserved for on-demand disk fakes. Rename the configured disk.'); | ||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
|
||
| } | ||
|
Comment on lines
+843
to
+845
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a configured
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validation intentionally happens when the disk is resolved, as with other invalid disk configurations. File-serving route registration does not construct the driver. Using this reserved configuration then raises the explicit rename error. A second check during route registration would duplicate that validation; boot-time rejection is not part of this contract. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That’s fair. |
||
|
|
||
| return $config; | ||
| } | ||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: An explicitly configured empty
ondemanddisk bypasses this reservation check because$config !== []is false. Detect whether the configuration key exists separately from its value so every configuredondemanddisk receives the rename error.Prompt for AI agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An empty configuration cannot construct a disk: resolution already rejects it with the missing-driver error, and it cannot populate the cache entry that intercepts anonymous builds. The reservation check prevents actual configured disks from occupying that slot. Distinguishing an empty entry from an absent one only changes the error wording and does not fix a behavior or safety gap; absent configuration also remains valid for fake cleanup.