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
Instead of handling just the default channel, make LogTenancyBootstrapper handle all the channels that should be affected (= $storagePathChannels, channels from $channelOverrides and the default channel in case 'stack' is the default)
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.
- 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).
This PR fixes the URL override example in TenancyServiceProvider stub
(the commented `overrideUrlInTenantContext()` segment). If the tenant
doesn't have any domain, set the root URL back to the original one.
This pull request adds improved PHPDoc type annotations to several
Eloquent relationship methods, enhancing static analysis and developer
experience. These changes clarify the expected return types for
relationships, making the codebase easier to understand and work with.
Relationship method type annotations:
* Added a detailed return type annotation to the `tenant` method in the
`BelongsToTenant` trait, specifying the related model and the current
class.
* Added a detailed return type annotation to the `domains` method in the
`HasDomains` trait, specifying the related model and the current class.
* Added a detailed return type annotation to the `tenants` method in the
`ResourceSyncing` class, specifying the related model and the current
class.
The old names of the class and method were misleading. We don't
actually need any relation. And we don't even need a model instance
as we were returning previously -- the only use of that method was
in TriggerSyncingEvents which would immediately use ::class on the
returned value. Therefore, all we are asking for in this interface
is just the central resource class.
Also make all resource syncing-related listener closures static.
Also correct return type for getGlobalIdentifierKey to string|int.
(We intentionally do not support returning null like many other
"get x key" methods would since such a case might break resource
syncing logic. This is also why we use inline getAttribute() in the
creating listener instead of calling the method.)
Also move pivot record deletion to that listener and improve tests
The 'tenant pivot records are deleted along with the tenants to which
they belong to' test is failing in this commit -- the listener
for deleting mappings when a *tenant* is deleted is only implemented
in the next commit. The only change done here is to re-add FKs
(necessary for passing *in this commit* in that specific dataset
variant) that were removed from the default test migration as we now
have the DeleteResourceMapping listener that's enabled by default.
This change was prompted by a phpstan failure after a recent update.
While making this change, I noticed we don't need the macro anymore
as useAssetOrigin() was added to the UrlGenerator earlier this year,
simplifying our implementation.
Recent Laravel installations often have http://localhost:8000 as
APP_URL, so we make sure to strip any port suffix from the default
central domain derived from APP_URL.
