[13.x] Throw exception when served disks share the same URI

[jackbayliss]

The default local disk ships with serve => true and no url configured, which falls back to /storage.

If a developer adds a second local disk with serve => true also without a url both disks silently resolve to the same URI

The actual failure only surfaces later (e.g. when using temporaryUploadUrl), making it difficult to debug, as you may have the wrong content, or missing files - or you just get straight up route missing exception.

This adds an early InvalidArgumentException when a URI collision is detected that tells the developer to use the url config.

This happened to me when moving s3 disks to local for testing and removing Minio, which is why I think it may be useful 🫡 - as I was a bit confused at first.

Targeted 13.x as may affect applications with multiple served disks using the same url- though already broken behavior I guess.

[github-actions]

Thanks for submitting a PR!

Note that draft PRs are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

[browner12]

I'm a little confused here. If the disks is configured without a URL, doesn't that mean we don't want a public URL for the asset?

It seems like we should be able to configure multiple "private" disks that don't have a public URL, which this code prevents.

[jackbayliss]

@browner12 From the top of my head, serve: true registers an HTTP route for that disk, which is what powers the new temporaryUploadUrl() support on the local driver.

If you have multiple local disks (with serve as true) without specifying a url, they all default to /storage and conflict, which is why I added the collision detection.

I think just setting serve => false could work as you can use the disk manually, unless you're using the temporaryUrl..

Alternatively I guess the right move is to set a url property... Though local could probably use a default

Granted the documentation is quite confusing, so apologies if I missed something.

[browner12]

Okay, after digging into it a little more I think you're right. If serve is true that route is being registered, and then it makes sense to require separate URLs.

I would agree the docs are not super clear for all of this stuff, so thanks for looking at it. I think some of the default naming makes it confusing as well, because the "local" disk stores its file at /app/private, but then the files are publicly available. IDK