"test asset controller returns a 404 when accessing a file outside the storage root" tested very similar things to the new test (which had some redundant config anyway). Merged these tests into one -- "tenant asset controller only serves files inside the asset root"
"adding a scoped disk to tenancy.filesystem.disks throws an exception if its base disk is not listed" doesn't use a dataset and deal with inline baes disks anymore.
Added a separate test for scoped disks with inline base ("adding a scoped disk with an inline base disk to tenancy.filesystem.disks throws an exception").
Removed the "adding a scoped disk to tenancy.filesystem.disks has no effect on the disk when its base disk is listed too" test, it was mostly redundant.
Use the *original* 'tenant storage gets deleted during tenant deletion when the DeletingTenant pipeline contains DeleteTenantStorage' test and remove what's not necessary anymore. Also make it clear that enabling FS bootstrapper is not required for the deletion to work -- the tenant directory just has to exist.
Delete the nonsensical 'DeleteTenantStorage does not delete the central storage directory when the filesystem bootstrapper is disabled' test. That one was there to test that the central dir never gets deleted, but it was wrong. Added 'DeleteTenantStorage never deletes the central storage directory' which actually makes the job's realpath() comparison check pass and the job just returns.
Refrain from dealing with the impossible "self-referencing" scoped disk case. Instead of that, test the inline parent behavior.
Also update the exception message in forgetDisks() so that it's a bit less vague.
In FSBootstrapper::forgetDisks():
- `tenancy.filesystem.disks => ['scoped']` throws
- `tenancy.filesystem.disks => ['scoped', 'parent']` does NOT throw
- `tenancy.filesystem.disks => ['scoped_with_scoped_parent', 'scoped_parent']` (invalid config where a scoped disk's base disk doesn't actually exist because the scoped disks just reference themselves) throws
At first glance, it could look weird that $attemptedPath uses "/" but the check in abortIf below uses DIRECTORY_SEPARATOR. Add comment that explains this.
If diskRoot() somehow ended up receiving a scoped disk (e.g. in case the scoped disk was listed in tenancy.filesystem.disks), its root would get configured, and it'd be completely redundant. It wouldn't break anything since scoped disk's configured root is ignored -- its parent's root is always used. Even though not adding this skipping code would essentially do no harm, it prevents the method from doing redundant work and defines the behavior a bit more clearly.
diskUrl() is similar in that regard, but that method already has a strict "disk driver has to be 'local'" -- scoped disks never made it through so nothing to change there.
This includes moving the TenantAssetController baseDiskName() method to FSBootstrapper and making it public static, since the same logic is used in two places now. Also cover the edge case where a scoped disk A has a scoped disk B as its parent, and B has A as its parent -- in that case, the method would be stuck in an infinite loop (also added separate test for this, commenting out the $visited-related code in baseDiskName will make the test fail).
Also updated the assetRoot's unnamed disk exception message.
The test fails with the nested disk dataset because we resolve a disk first, then initialize tenancy, and because the nested scoped disks aren't forgotten, so the disk config changes that the FS bootstrapper applies aren't reflected on the already-resolved disk instance.
Instead of just saying that the publicDisk *should* be listed in tenancy.filesystem.disks, enforce that -- if the disk isn't tenant-aware, throw an exception.
Also update comments accordingly. E.g. since scoped disks don't have to have a single "parent disk" (the parent can also be a scoped disk and have another parent, and so on), use "base disk".
Since the tenant storage path can now be grabbed using FilesystemTenancyBootstrapper::getBoundTenantStoragePath(), the log bootstrapper doesn't need to depend on the FSBootstrapper being enabled and storage_path() being suffixed.
Instead of adding a regression test, just delete FilesystemTenancyBootstrapper from the config settings in the log bootstrapper tests (and in tests that did use storage_path() in tenant context assertions, use explicitly "hardcoded" paths instead).
Also, instead of throwing the "no root path configured" exception, just throw an exception if the disk is not local (i.e. is not instanceof LocalFilesystemAdapter). A local disk HAS to have a string root, otherwise, Laravel throws an exception while instantiating that disk.
Test that tenant assets can be served from scoped disks, and that tenant asset roots respect the disk's configured prefix.
Currently, the tests fail because TenantAssetController grabs the root from the config instead of resolving it via Storage::disk(...)->path('').
The docblock said that the FSBootstrapper was required for this job to work at all, but that's not fully true since the job just uses the FSBootstrapper's public static methods to get the storage paths.
Correct misleading ones, add ones that were missing (e.g. the TenantAssetController's docblock, the FSBootstrapper dependency should be mentioned there)
Briefly document the root_override placeholders, make the links point to v4 docs instead of the v3 ones. Also in the url_override comments, mention that local disks must have a valid root in order for the override to work correctly.
Previously, we only skipped disks with `null` override. But an override with an empty string is also incorrect, and simply checking if $this->app['config']["tenancy.filesystem.url_override.{$disk}"]) is falsy instead of strictly null takes care of that.
Update the existing "create storage symlinks action skips disks with a null url_override" test so that it covers disks with NO url_override (unset/null and empty string). The test fails with the empty string override at the moment.
getBoundTenantStoragePath() and DeleteTenantStorage both claimed the tenant storage directory is where disks, cache and sessions are scoped to. That's only true when root_override points there and scope_cache/scope_sessions are enabled -- a root_override using %original_storage_path% puts the disk root outside it entirely.
The symlink target used to be built from the root_override template, so a disk without an entry there had nothing to resolve.
The symlink target is now the disk's tenant-context root, which the bootstrapper sets either way -- with a root_override it expands
the template, without one, it appends the suffix to the disk's own root. Disks that only have a url_override now get a working symlink
instead of being skipped while their URL was still overridden.
Skipping disks with a null url_override is now explicit. The root_override check used to cover that by accident, and without it str_replace() gets null and throws a TypeError.
The comment said that disks, cache and sessions are scoped ot the tenant's storage dir either way, but that's only true if the disks are included in tenancy.filesystem.disks, and for cache and sessions, scope_cache and scope_sessions have to be enabled. This might be obvious, but it'll be better to make this completely clear from the comment.
When a disk in url_override and root_override is absent from tenancy.filesystem.disks, FilesystemTenancyBootstrapper leaves its root unchanged, possibleTenantSymlinks allows creating a symlink for that unscoped disk (= a disk with a central root), which can expose shared files.
Fixed by throwing an exception in possibleTenantSymlinks saying that the disk should be tenant-aware (= included in the tenancy.filesystem.disks config).
The resolved path was compared to the asset root using a plain string prefix, so a
directory whose name just starts with the asset root's name passed the check. This
didn't matter while the asset root was hardcoded to app/public, but $publicDisk lets
it be any disk root.
Currently this fails because the controller checks that the requested file is inside the
asset root using a plain string prefix, so with the root set to
'%storage_path%/app/media/', a request for '../media-originals/photo.jpg' is served
from the sibling 'app/media-originals' directory.
Disabling the config doesn't break local disk tenancy -- it only affects the
storage_path() helper. Disks, cache and sessions are scoped either way, so the
tradeoff is that files accessed using storage_path() are shared by all tenants.
