Database deletion is now skipped by default if the tenant has the
`create_database` internal attribute set to false, meaning it was likely
created without a database. This skip can be opted out of by changing a
static property.
It also adds an opt-in static property for ignoring any other failures
during database deletion, to allow continuing execution of the delete
pipeline.
---------
Co-authored-by: Samuel Štancl <samuel@archte.ch>
The `CreateTenantStorage` and `DeleteTenantStorage` listeners were used
alongside JobPipelines. When the `TenantCreated` JobPipeline had
`shouldBeQueued(true)` and the `Listeners\CreateTenantStorage` was
uncommented, the listener would throw an exception
(`Stancl\Tenancy\Database\Exceptions\TenantDatabaseDoesNotExistException
Database tenantX.sqlite does not exist.`) because at the time of
executing the listener, the tenant DB wasn't created yet.
The same issue could likely also occur in the `DeleteTenantStorage`
listener as it uses `tenancy()->run()` to resolve the tenant's storage
path which wouldn't work if the tenant's database (or other resources)
was already deleted, making initialization impossible.
This PR changes `DeleteTenantStorage` into a job and puts it (commented)
into the job pipeline, so that it can be queued with the rest of the
jobs. It also removes `CreateTenantStorage` because it should be
redundant with the FilesystemTenancyBootstrapper creating the same paths
automatically when storage path is suffixed.
The old classes are kept but deprecated for backwards compatibility.
We've also added some edge case hardening to `DeleteTenantStorage` to
make sure it never deletes the central storage path directory, which
previously could in theory occur due to a misconfiguration if a user
enabled this job/listener but disabled storage path suffixing.
Co-authored-by: Samuel Štancl <samuel@archte.ch>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
At the moment, `where()` cannot be used correctly while using
`withoutPending()`. For example, if we have a single non-pending tenant
in our DB (with ID 'foo'), queries like
`Tenant::withoutPending()->where('id', 'nonexistent')->first()`will
incorrectly return the non-pending tenant ('foo').
This is because `withoutPending()` does
`$builder->whereNull('data->pending_since')->orWhereNull('data')`. These
two aren't grouped, so `withoutPending()->where('id', 'nonexistent')`
basically translates to "WHERE data->pending_since IS NULL **OR (data IS
NULL AND id = 'nonexistent')**". So the query will include all tenants
whose `pending_since` is null (= all non-pending tenants).
Grouping `->whereNull('data->pending_since')->orWhereNull('data')` in a
closure passed to a separate `where()` fixes this issue.
phpstan started failing with '... implements generic interface
Illuminate\Database\Eloquent\Scope but does not specify its types:
TModel'. We solve this by adding an implements docblock to the scopes
implementing that interface. They're fairly generic - we just use the
Model type itself in the code - so we use Model for the type parameter.
This prevents race conditions that may occur if there are two concurrent
processes trying to create the storage path for the tenant. The
storagePath() method runs during bootstrap() which can easily happen
in two places at once. The race condition specifically occurs in between
the is_dir() check and the mkdir() call, the latter producing an
exception if the dir already exist. We simply ignore any error coming
out of mkdir() and then check for success separately.
We could omit that success check since failure is unlikely and would
only occur due to a server misconfiguration that would manifest itself
in other ways as well, but this way the simple TOC/TOU race condition
is prevented while other errors are still reported.
We apply the same change to the mkdir() in scopeSessions() as the logic
is similar.
Resolves#1452
Some bootstrappers read attributes of the tenant during bootstrap() but
don't respond to changes made to the tenant afterwards.
Therefore, when making changes to the tenant that'd affect the behavior
of a bootstrapper, it's necessary to reinitialize tenancy (if it matters
that changes are reflected immediately). This adds a convenience helper
for that purpose.
Previously, credential mappings from `$mapPresets` overrode mappings defined in `$credentialsMap`. If someone used pusher/reverb/ably and wanted to override some of that preset's mappings, e.g. use 'pusher_app_key' instead of 'pusher_key' by specifying 'pusher_app_key' in `$credentialsMap`, the preset's mapping ('pusher_key') would still be used.
Tests now use datasets with all drivers that are in `TenancyBroadcastManager::$tenantBroadcasters` by default plus the custom driver. Also add assertions for updating the tenant properties/config in tenant context.
Tests from BroadcastingTest moved to the appropriate bootstrapper test files. The new tenant credentials test has assertions equal to both the original property -> config mapping test and the config -> credentials test.
After initializing tenancy, calls like `Broadcast::auth()` use the central `BroadcastManager`. Clearing the facade's resolved `Broadcasting\Factory` instance fixes that problem.
Test that BroadcastingConfigBootstrapper correctly maps tenant properties to broadcasting config/credentials, and that the credentials don't leak when switching contexts. Also add the `$config` property to `TestingBroadcaster` so that we can access the credentials used by the broadcaster.
- Update ci.yml and composer.json
- Wrap single database tenancy trait scopes in whenBooted()
- Update SessionSeparationTest to use laravel-cache- prefix in L13
and laravel_cache_ in <=L12. Our own prefix remains tenant_%tenant%_
(as configured in tenancy.cache.prefix). We could update this to be
tenant-%tenant%- from now on for consistency with Laravel's prefixes
(changed in https://github.com/laravel/framework/pull/56172) but I'm
not sure yet. _ seems to read a bit better but perhaps consistency
is more important. We may change this later and it can be adjusted
in userland easily (since it's just a config option).
Using `URL::temporarySignedRoute()` in tenant context with
`UrlGeneratorBootstrapper` enabled doesn't work the same as `route()`.
The bypass parameter doesn't actually bypass the route name prefixing.
`route()` is called in the `parent::temporarySignedRoute()` call, and
because the bypass parameter is removed before calling
`parent::temporarySignedRoute()`, the underlying `route()` call doesn't
get the bypass parameter and it ends up attempting to generate URL for a
route with the name prefixed with 'tenant.'.
This PR adds the bypass parameter back after `prepareRouteInputs()`, so
that `parent::temporarySignedRoute()` receives it, and the underlying
`route()` call respects it. Also added basic tests for the
`URL::temporarySignedRoute()` behavior (the new bypass parameter test
works as a regression test).
